Browse Source

nit: add context about migrating identity thing (#55413)

Nathan Hsieh 1 year ago
parent
commit
fdc524a0ff

+ 4 - 0
src/sentry/auth/helper.py

@@ -817,6 +817,10 @@ class AuthHelper(Pipeline):
                 auth_identity = None
 
             # Handle migration of identity keys
+            # Context - when google oauth was initially created, the auth_identity key was simply
+            # the provider email. This can cause issues if the customer changes their domain name,
+            # and now their email is different and they're locked out of their account.
+            # This logic updates their id to the provider id instead.
             if not auth_identity and isinstance(user_id, MigratingIdentityId):
                 try:
                     auth_identity = AuthIdentity.objects.select_related("user").get(

+ 7 - 0
src/sentry/auth/provider.py

@@ -17,6 +17,13 @@ class MigratingIdentityId(namedtuple("MigratingIdentityId", ["id", "legacy_id"])
     MigratingIdentityId may be used in the ``id`` field of an identity
     dictionary to facilitate migrating user identities from one identifying id
     to another.
+
+    Context - when google oauth was initially created, the auth_identity key was simply
+    the provider email. This can cause issues if the customer changes their domain name,
+    and now their email is different and they're locked out of their account.
+    This logic updates their id to the provider id instead.
+
+    NOTE: this should _only_ really be relevant for google oauth implementation
     """
 
     __slots__ = ()

+ 1 - 7
src/sentry/auth/providers/fly/provider.py

@@ -2,7 +2,6 @@ from typing import Any, Dict, Optional, cast
 
 from sentry import options
 from sentry.auth.partnership_configs import SPONSOR_OAUTH_NAME, ChannelName
-from sentry.auth.provider import MigratingIdentityId
 from sentry.auth.providers.oauth2 import OAuth2Callback, OAuth2Provider
 
 from .constants import ACCESS_TOKEN_URL, AUTHORIZE_URL
@@ -76,13 +75,8 @@ class FlyOAuth2Provider(OAuth2Provider):
         data = state["data"]
         user_data = state["user"]
 
-        # XXX(epurkhiser): We initially were using the email as the id key.
-        # This caused account dupes on domain changes. Migrate to the
-        # account-unique sub key.
-        user_id = MigratingIdentityId(id=user_data["user_id"], legacy_id=user_data["email"])
-
         return {
-            "id": user_id,
+            "id": user_data["user_id"],
             "email": user_data["email"],
             "name": user_data["email"],
             "data": self.get_oauth_data(data),

+ 1 - 4
tests/sentry/auth/providers/fly/test_provider.py

@@ -2,7 +2,6 @@ import pytest
 
 from sentry.auth.exceptions import IdentityNotValid
 from sentry.auth.partnership_configs import ChannelName
-from sentry.auth.provider import MigratingIdentityId
 from sentry.models import AuthIdentity, AuthProvider
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import control_silo_test
@@ -61,9 +60,7 @@ class FlyOAuth2ProviderTest(TestCase):
             "data": data,
             "user": user_info,
         }
-        expected_user_id = MigratingIdentityId(
-            id=user_info["user_id"], legacy_id=user_info["email"]
-        )
+        expected_user_id = user_info["user_id"]
         result = provider.build_identity(state)
         assert result == {
             "id": expected_user_id,