Invite code reuse via cookie manipulation in sentry

@@ -4,7 +4,11 @@
 from rest_framework.response import Response
 
 from sentry.api.base import Endpoint, region_silo_endpoint
-from sentry.api.invite_helper import ApiInviteHelper, add_invite_cookie, remove_invite_cookie
+from sentry.api.invite_helper import (
+    ApiInviteHelper,
+    add_invite_details_to_session,
+    remove_invite_details_from_session,
+)
 from sentry.models import AuthProvider, OrganizationMember
 from sentry.utils import auth
 
@@ -14,26 +18,31 @@ class AcceptOrganizationInvite(Endpoint):
     # Disable authentication and permission requirements.
     permission_classes = []
 
-    def respond_invalid(self, request: Request) -> Response:
+    @staticmethod
+    def respond_invalid() -> Response:
         return Response(status=status.HTTP_400_BAD_REQUEST, data={"details": "Invalid invite code"})
 
-    def get_helper(self, request: Request, member_id, token):
+    def get_helper(self, request: Request, member_id: int, token: str) -> ApiInviteHelper:
         return ApiInviteHelper(request=request, member_id=member_id, instance=self, token=token)
 
-    def get(self, request: Request, member_id, token) -> Response:
+    def get(self, request: Request, member_id: int, token: str) -> Response:
         try:
             helper = self.get_helper(request, member_id, token)
         except OrganizationMember.DoesNotExist:
-            return self.respond_invalid(request)
+            return self.respond_invalid()
 
-        om = helper.om
-        organization = om.organization
+        organization_member = helper.om
+        organization = organization_member.organization
 
-        if not helper.member_pending or not helper.valid_token or not om.invite_approved:
-            return self.respond_invalid(request)
+        if (
+            not helper.member_pending
+            or not helper.valid_token
+            or not organization_member.invite_approved
+        ):
+            return self.respond_invalid()
 
-        # Keep track of the invite email for when we land back on the login page
-        request.session["invite_email"] = om.email
+        # Keep track of the invite details in the request session
+        request.session["invite_email"] = organization_member.email
 
         try:
             auth_provider = AuthProvider.objects.get(organization=organization)
@@ -57,7 +66,9 @@ def get(self, request: Request, member_id, token) -> Response:
         # Allow users to register an account when accepting an invite
         if not helper.user_authenticated:
             request.session["can_register"] = True
-            add_invite_cookie(request, response, member_id, token)
+            add_invite_details_to_session(
+                request, organization_member.id, organization_member.token
+            )
 
             # When SSO is required do *not* set a next_url to return to accept
             # invite. The invite will be accepted after SSO is completed.
@@ -73,24 +84,29 @@ def get(self, request: Request, member_id, token) -> Response:
         # to come back to the accept invite page since 2FA will *not* be
         # required if SSO is required.
         if auth_provider is not None:
-            add_invite_cookie(request, response, member_id, token)
+            add_invite_details_to_session(
+                request, organization_member.id, organization_member.token
+            )
+
             provider = auth_provider.get_provider()
             data["ssoProvider"] = provider.name
 
         onboarding_steps = helper.get_onboarding_steps()
         data.update(onboarding_steps)
         if any(onboarding_steps.values()):
-            add_invite_cookie(request, response, member_id, token)
+            add_invite_details_to_session(
+                request, organization_member.id, organization_member.token
+            )
 
         response.data = data
 
         return response
 
-    def post(self, request: Request, member_id, token) -> Response:
+    def post(self, request: Request, member_id: int, token: str) -> Response:
         try:
             helper = self.get_helper(request, member_id, token)
         except OrganizationMember.DoesNotExist:
-            return self.respond_invalid(request)
+            return self.respond_invalid()
 
         if not helper.valid_request:
             return Response(
@@ -106,6 +122,6 @@ def post(self, request: Request, member_id, token) -> Response:
             response = Response(status=status.HTTP_204_NO_CONTENT)
 
         helper.accept_invite()
-        remove_invite_cookie(request, response)
+        remove_invite_details_from_session(request)
 
         return response

@@ -12,7 +12,7 @@
 from sentry.api.base import control_silo_endpoint
 from sentry.api.bases.user import UserEndpoint
 from sentry.api.decorators import email_verification_required, sudo_required
-from sentry.api.invite_helper import ApiInviteHelper, remove_invite_cookie
+from sentry.api.invite_helper import ApiInviteHelper, remove_invite_details_from_session
 from sentry.api.serializers import serialize
 from sentry.auth.authenticators.base import EnrollmentStatus, NewEnrollmentDisallowed
 from sentry.auth.authenticators.sms import SMSRateLimitExceeded
@@ -281,10 +281,10 @@ def post(self, request: Request, user, interface_id) -> Response:
 
         # If there is a pending organization invite accept after the
         # authenticator has been configured.
-        invite_helper = ApiInviteHelper.from_cookie(request=request, instance=self, logger=logger)
+        invite_helper = ApiInviteHelper.from_session(request=request, instance=self, logger=logger)
 
         if invite_helper and invite_helper.valid_request:
             invite_helper.accept_invite()
-            remove_invite_cookie(request, response)
+            remove_invite_details_from_session(request)
 
         return response

@@ -1,8 +1,7 @@
-from typing import Dict
-from urllib.parse import parse_qsl, urlencode
+from typing import Dict, Tuple
 
-from django.urls import reverse
 from django.utils.crypto import constant_time_compare
+from rest_framework.request import Request
 
 from sentry import audit_log, features
 from sentry.models import (
@@ -17,50 +16,37 @@
 from sentry.utils import metrics
 from sentry.utils.audit import create_audit_entry
 
-INVITE_COOKIE = "pending-invite"
-COOKIE_MAX_AGE = 60 * 60 * 24 * 7  # 7 days
 
+def add_invite_details_to_session(request: Request, member_id: int, token: str):
+    """Add member ID and token to the request session"""
+    request.session["invite_token"] = token
+    request.session["invite_member_id"] = member_id
 
-def add_invite_cookie(request, response, member_id, token):
-    url = reverse("sentry-accept-invite", args=[member_id, token])
-    response.set_cookie(
-        INVITE_COOKIE,
-        urlencode({"memberId": member_id, "token": token, "url": url}),
-        max_age=COOKIE_MAX_AGE,
-    )
 
+def remove_invite_details_from_session(request):
+    """Deletes invite details from the request session"""
+    request.session.pop("invite_member_id", None)
+    request.session.pop("invite_token", None)
 
-def remove_invite_cookie(request, response):
-    if INVITE_COOKIE in request.COOKIES:
-        response.delete_cookie(INVITE_COOKIE)
 
-
-def get_invite_cookie(request):
-    if INVITE_COOKIE not in request.COOKIES:
-        return None
-
-    # memberId should be coerced back to an integer
-    invite_data = dict(parse_qsl(request.COOKIES.get(INVITE_COOKIE)))
-    invite_data["memberId"] = int(invite_data["memberId"])
-
-    return invite_data
+def get_invite_details(request) -> Tuple[str, int]:
+    """Returns tuple of (token, member_id) from request session"""
+    return request.session.get("invite_token", None), request.session.get("invite_member_id", None)
 
 
 class ApiInviteHelper:
     @classmethod
-    def from_cookie_or_email(cls, request, organization, email, instance=None, logger=None):
+    def from_session_or_email(cls, request, organization, email, instance=None, logger=None):
         """
         Initializes the ApiInviteHelper by locating the pending organization
-        member via the currently set pending invite cookie, or via the passed
-        email if no cookie is currently set.
+        member via the currently set pending invite details in the session, or
+        via the passed email if no cookie is currently set.
         """
-        pending_invite = get_invite_cookie(request)
+        invite_token, invite_member_id = get_invite_details(request)
 
         try:
-            if pending_invite is not None:
-                om = OrganizationMember.objects.get(
-                    id=pending_invite["memberId"], token=pending_invite["token"]
-                )
+            if invite_token and invite_member_id:
+                om = OrganizationMember.objects.get(token=invite_token, id=invite_member_id)
             else:
                 om = OrganizationMember.objects.get(
                     email=email, organization=organization, user=None
@@ -75,17 +61,17 @@ def from_cookie_or_email(cls, request, organization, email, instance=None, logge
         )
 
     @classmethod
-    def from_cookie(cls, request, instance=None, logger=None):
-        org_invite = get_invite_cookie(request)
+    def from_session(cls, request, instance=None, logger=None):
+        invite_token, invite_member_id = get_invite_details(request)
 
-        if not org_invite:
+        if not invite_token or not invite_member_id:
             return None
 
         try:
             return ApiInviteHelper(
                 request=request,
-                member_id=org_invite["memberId"],
-                token=org_invite["token"],
+                member_id=invite_member_id,
+                token=invite_token,
                 instance=instance,
                 logger=logger,
             )
@@ -95,12 +81,12 @@ def from_cookie(cls, request, instance=None, logger=None):
             return None
 
     def __init__(self, request, member_id, token, instance=None, logger=None):
-        self.request = request
-        self.member_id = member_id
-        self.token = token
+        self.request: Request = request
+        self.member_id: int = member_id
+        self.token: str = token
         self.instance = instance
         self.logger = logger
-        self.om = self.organization_member
+        self.om: OrganizationMember = self.organization_member
 
     def handle_success(self):
         member_joined.send_robust(
@@ -128,7 +114,7 @@ def handle_invite_not_approved(self):
             self.om.delete()
 
     @property
-    def organization_member(self):
+    def organization_member(self) -> OrganizationMember:
         return OrganizationMember.objects.select_related("organization").get(pk=self.member_id)
 
     @property

@@ -19,7 +19,7 @@
 from django.views import View
 
 from sentry import audit_log, features
-from sentry.api.invite_helper import ApiInviteHelper, remove_invite_cookie
+from sentry.api.invite_helper import ApiInviteHelper, remove_invite_details_from_session
 from sentry.api.utils import generate_organization_url
 from sentry.auth.email import AmbiguousUserFromEmail, resolve_email_to_user
 from sentry.auth.exceptions import IdentityNotValid
@@ -219,9 +219,9 @@ def _handle_new_membership(self, auth_identity: AuthIdentity) -> Optional[Organi
         user = auth_identity.user
 
         # If the user is either currently *pending* invite acceptance (as indicated
-        # from the pending-invite cookie) OR an existing invite exists on this
+        # from the invite token and member id in the session) OR an existing invite exists on this
         # organization for the email provided by the identity provider.
-        invite_helper = ApiInviteHelper.from_cookie_or_email(
+        invite_helper = ApiInviteHelper.from_session_or_email(
             request=self.request, organization=self.organization, email=user.email
         )
 
@@ -406,7 +406,7 @@ def _post_login_redirect(self) -> HttpResponseRedirect:
 
         # Always remove any pending invite cookies, pending invites will have been
         # accepted during the SSO flow.
-        remove_invite_cookie(self.request, response)
+        remove_invite_details_from_session(self.request)
 
         return response
 

@@ -11,7 +11,7 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry.api.invite_helper import ApiInviteHelper, remove_invite_cookie
+from sentry.api.invite_helper import ApiInviteHelper, remove_invite_details_from_session
 from sentry.auth.superuser import is_active_superuser
 from sentry.constants import WARN_SESSION_EXPIRED
 from sentry.http import get_server_hostname
@@ -166,7 +166,7 @@ def handle_basic_auth(self, request: Request, **kwargs):
             request.session.pop("invite_email", None)
 
             # Attempt to directly accept any pending invites
-            invite_helper = ApiInviteHelper.from_cookie(request=request, instance=self)
+            invite_helper = ApiInviteHelper.from_session(request=request, instance=self)
 
             # In single org mode, associate the user to the only organization.
             #
@@ -182,7 +182,7 @@ def handle_basic_auth(self, request: Request, **kwargs):
             if invite_helper and invite_helper.valid_request:
                 invite_helper.accept_invite()
                 response = self.redirect_to_org(request)
-                remove_invite_cookie(request, response)
+                remove_invite_details_from_session(request)
 
                 return response
 

@@ -1,5 +1,4 @@
 from datetime import timedelta
-from urllib.parse import parse_qsl
 
 from django.db.models import F
 from django.urls import reverse
@@ -27,14 +26,15 @@ def _require_2fa_for_organization(self):
         self.organization.update(flags=F("flags").bitor(Organization.flags.require_2fa))
         assert self.organization.flags.require_2fa.is_set
 
-    def _assert_pending_invite_cookie_set(self, response, om):
-        invite_link = om.get_invite_link()
-        invite_data = dict(parse_qsl(response.client.cookies["pending-invite"].value))
+    def _assert_pending_invite_details_in_session(self, om):
+        assert self.client.session["invite_token"] == om.token
+        assert self.client.session["invite_member_id"] == om.id
 
-        assert invite_data.get("url") in invite_link
-
-    def _assert_pending_invite_cookie_not_set(self, response):
-        self.assertNotIn("pending-invite", response.client.cookies)
+    def _assert_pending_invite_details_not_in_session(self, response):
+        session_invite_token = self.client.session.get("invite_token", None)
+        session_invite_member_id = self.client.session.get("invite_member_id", None)
+        assert session_invite_token is None
+        assert session_invite_member_id is None
 
     def _enroll_user_in_2fa(self):
         interface = TotpInterface()
@@ -111,7 +111,7 @@ def test_user_needs_2fa(self):
         assert resp.status_code == 200
         assert resp.data["needs2fa"]
 
-        self._assert_pending_invite_cookie_set(resp, om)
+        self._assert_pending_invite_details_in_session(om)
 
     def test_user_has_2fa(self):
         self._require_2fa_for_organization()
@@ -128,7 +128,7 @@ def test_user_has_2fa(self):
         assert resp.status_code == 200
         assert not resp.data["needs2fa"]
 
-        self._assert_pending_invite_cookie_not_set(resp)
+        self._assert_pending_invite_details_not_in_session(resp)
 
     def test_user_can_use_sso(self):
         AuthProvider.objects.create(organization=self.organization, provider="google")
@@ -249,7 +249,7 @@ def test_can_accept_when_user_has_2fa(self):
         )
         assert resp.status_code == 204
 
-        self._assert_pending_invite_cookie_not_set(resp)
+        self._assert_pending_invite_details_not_in_session(resp)
 
         om = OrganizationMember.objects.get(id=om.id)
         assert om.email is None
@@ -291,13 +291,12 @@ def test_2fa_cookie_deleted_after_accept(self):
             reverse("sentry-api-0-accept-organization-invite", args=[om.id, om.token])
         )
         assert resp.status_code == 200
-        self._assert_pending_invite_cookie_set(resp, om)
+        self._assert_pending_invite_details_in_session(om)
 
         self._enroll_user_in_2fa()
         resp = self.client.post(
             reverse("sentry-api-0-accept-organization-invite", args=[om.id, om.token])
         )
         assert resp.status_code == 204
 
-        # value set to empty string on deletion
-        assert not resp.client.cookies["pending-invite"].value
+        self._assert_pending_invite_details_not_in_session(resp)

@@ -1,5 +1,4 @@
 from unittest import mock
-from urllib.parse import parse_qsl
 
 from django.conf import settings
 from django.core import mail
@@ -277,10 +276,9 @@ def require_2fa_for_organization(self):
         self.organization.update(flags=F("flags").bitor(Organization.flags.require_2fa))
         self.assertTrue(self.organization.flags.require_2fa.is_set)
 
-    def _assert_pending_invite_cookie_set(self, response, om):
-        invite_link = om.get_invite_link()
-        invite_data = dict(parse_qsl(response.client.cookies["pending-invite"].value))
-        assert invite_data.get("url") in invite_link
+    def _assert_pending_invite_details_in_session(self, om):
+        assert self.client.session["invite_token"] == om.token
+        assert self.client.session["invite_member_id"] == om.id
 
     def create_existing_om(self):
         OrganizationMember.objects.create(
@@ -296,7 +294,7 @@ def get_om_and_init_invite(self):
             reverse("sentry-api-0-accept-organization-invite", args=[om.id, om.token])
         )
         assert resp.status_code == 200
-        self._assert_pending_invite_cookie_set(resp, om)
+        self._assert_pending_invite_details_in_session(om)
 
         return om
 
@@ -313,23 +311,26 @@ def assert_invite_accepted(self, response, member_id: int) -> None:
             data=om.get_audit_log_data(),
         )
 
-        self.assertFalse(response.client.cookies["pending-invite"].value)
+        assert not self.client.session.get("invite_token")
+        assert not self.client.session.get("invite_member_id")
 
-    def setup_u2f(self):
+    def setup_u2f(self, om):
         new_options = settings.SENTRY_OPTIONS.copy()
         new_options["system.url-prefix"] = "https://testserver"
         with self.settings(SENTRY_OPTIONS=new_options):
+            # We have to add the invite details back in to the session
+            # prior to .save_session() since this re-creates the session property
+            # when under test. See here for more details:
+            # https://docs.djangoproject.com/en/2.2/topics/testing/tools/#django.test.Client.session
             self.session["webauthn_register_state"] = "state"
+            self.session["invite_token"] = self.client.session["invite_token"]
+            self.session["invite_member_id"] = self.client.session["invite_member_id"]
             self.save_session()
             return self.get_success_response(
                 "me",
                 "u2f",
                 method="post",
-                **{
-                    "deviceName": "device name",
-                    "challenge": "challenge",
-                    "response": "response",
-                },
+                **{"deviceName": "device name", "challenge": "challenge", "response": "response"},
             )
 
     def test_cannot_accept_invite_pending_invite__2fa_required(self):
@@ -342,7 +343,7 @@ def test_cannot_accept_invite_pending_invite__2fa_required(self):
     @mock.patch("sentry.auth.authenticators.U2fInterface.try_enroll", return_value=True)
     def test_accept_pending_invite__u2f_enroll(self, try_enroll):
         om = self.get_om_and_init_invite()
-        resp = self.setup_u2f()
+        resp = self.setup_u2f(om)
 
         self.assert_invite_accepted(resp, om.id)
 
@@ -409,7 +410,7 @@ def test_accept_pending_invite__totp_enroll(self, validate_otp):
     def test_user_already_org_member(self, try_enroll, log):
         om = self.get_om_and_init_invite()
         self.create_existing_om()
-        self.setup_u2f()
+        self.setup_u2f(om)
 
         assert not OrganizationMember.objects.filter(id=om.id).exists()
 
@@ -427,7 +428,7 @@ def test_org_member_does_not_exist(self, try_enroll, log):
         # pending member cookie.
         om.update(id=om.id + 1)
 
-        self.setup_u2f()
+        self.setup_u2f(om)
 
         om = OrganizationMember.objects.get(id=om.id)
         assert om.user is None
@@ -445,7 +446,7 @@ def test_invalid_token(self, try_enroll, log):
         # pending member cookie.
         om.update(token="123")
 
-        self.setup_u2f()
+        self.setup_u2f(om)
 
         om = OrganizationMember.objects.get(id=om.id)
         assert om.user is None

@@ -1,5 +1,4 @@
 from unittest import mock
-from urllib.parse import urlencode
 
 from django.contrib import messages
 from django.contrib.auth.models import AnonymousUser
@@ -138,14 +137,14 @@ def test_associated_existing_member_invite_request(self):
 
     def test_associate_pending_invite(self):
         # The org member invite should have a non matching email, but the
-        # member id and token will match from the cookie, allowing association
+        # member id and token will match from the session, allowing association
         member = OrganizationMember.objects.create(
             organization=self.organization, email="different.email@example.com", token="abc"
         )
 
-        self.request.COOKIES["pending-invite"] = urlencode(
-            {"memberId": member.id, "token": member.token, "url": ""}
-        )
+        self.request.session["invite_member_id"] = member.id
+        self.request.session["invite_token"] = member.token
+        self.save_session()
 
         auth_identity = self.handler.handle_new_user()
 

@@ -176,15 +176,15 @@ def test_registration_single_org(self):
         assert OrganizationMember.objects.filter(user=user).exists()
 
     @override_settings(SENTRY_SINGLE_ORGANIZATION=True)
-    @mock.patch("sentry.web.frontend.auth_login.ApiInviteHelper.from_cookie")
-    def test_registration_single_org_with_invite(self, from_cookie):
+    @mock.patch("sentry.web.frontend.auth_login.ApiInviteHelper.from_session")
+    def test_registration_single_org_with_invite(self, from_session):
         self.session["can_register"] = True
         self.save_session()
 
         self.client.get(self.path)
 
         invite_helper = mock.Mock(valid_request=True)
-        from_cookie.return_value = invite_helper
+        from_session.return_value = invite_helper
 
         resp = self.client.post(
             self.path,
@@ -229,15 +229,15 @@ def test_register_prefills_invite_email(self):
         assert resp.context["register_form"].initial["username"] == "foo@example.com"
         self.assertTemplateUsed("sentry/login.html")
 
-    @mock.patch("sentry.web.frontend.auth_login.ApiInviteHelper.from_cookie")
-    def test_register_accepts_invite(self, from_cookie):
+    @mock.patch("sentry.web.frontend.auth_login.ApiInviteHelper.from_session")
+    def test_register_accepts_invite(self, from_session):
         self.session["can_register"] = True
         self.save_session()
 
         self.client.get(self.path)
 
         invite_helper = mock.Mock(valid_request=True)
-        from_cookie.return_value = invite_helper
+        from_session.return_value = invite_helper
 
         resp = self.client.post(
             self.path,