Skip to content

Commit c9edcf3

Browse files
fix(fly): return user with correct auth (#56523)
When we find 2 users for one email we return the first one so the user experience doesn't break. This is causing issue for fly because one of the users is the one that's setup with fly auth. This fix returns the user who has the auth setup or the first user if ident is None
1 parent 26cd230 commit c9edcf3

File tree

3 files changed

+45
-4
lines changed

3 files changed

+45
-4
lines changed

src/sentry/services/hybrid_cloud/user/impl.py

+23-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
OrganizationStatus,
2222
UserEmail,
2323
)
24+
from sentry.models.authidentity import AuthIdentity
2425
from sentry.models.user import User
2526
from sentry.services.hybrid_cloud.auth import AuthenticationContext
2627
from sentry.services.hybrid_cloud.filter_query import (
@@ -37,6 +38,7 @@
3738
)
3839
from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user
3940
from sentry.services.hybrid_cloud.user.service import UserService
41+
from sentry.signals import user_signup
4042

4143
logger = logging.getLogger("user:provisioning")
4244

@@ -172,7 +174,9 @@ def get_first_superuser(self) -> Optional[RpcUser]:
172174
return None
173175
return serialize_rpc_user(user)
174176

175-
def get_or_create_user_by_email(self, *, email: str) -> RpcUser:
177+
def get_or_create_user_by_email(
178+
self, *, email: str, ident: Optional[str] = None, referrer: Optional[str] = None
179+
) -> RpcUser:
176180
with transaction.atomic(router.db_for_write(User)):
177181
user_query = User.objects.filter(email__iexact=email, is_active=True)
178182
# Create User if it doesn't exist
@@ -182,12 +186,28 @@ def get_or_create_user_by_email(self, *, email: str) -> RpcUser:
182186
email=email,
183187
name=email,
184188
)
189+
user_signup.send_robust(
190+
sender=self, user=user, source="api", referrer=referrer or "unknown"
191+
)
185192
else:
186193
# Users are not supposed to have the same email but right now our auth pipeline let this happen
187-
# So let's not break the user experience
194+
# So let's not break the user experience. Instead return the user with auth identity of ident or
195+
# the first user if ident is None
196+
user = user_query[0]
188197
if user_query.count() > 1:
189198
logger.warning("Email has multiple users", extra={"email": email})
190-
user = user_query[0]
199+
if ident:
200+
identity_query = AuthIdentity.objects.filter(
201+
user__in=user_query, ident=ident
202+
)
203+
if identity_query.exists():
204+
user = identity_query[0].user
205+
if identity_query.count() > 1:
206+
logger.warning(
207+
"Email has two auth identity for the same ident",
208+
extra={"email": email},
209+
)
210+
191211
return serialize_rpc_user(user)
192212

193213
def verify_any_email(self, *, email: str) -> bool:

src/sentry/services/hybrid_cloud/user/service.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ def get_first_superuser(self) -> Optional[RpcUser]:
139139

140140
@rpc_method
141141
@abstractmethod
142-
def get_or_create_user_by_email(self, *, email: str) -> RpcUser:
142+
def get_or_create_user_by_email(
143+
self, *, email: str, ident: Optional[str] = None, referrer: Optional[str] = None
144+
) -> RpcUser:
143145
pass
144146

145147
@rpc_method

tests/sentry/services/hybrid_cloud/user/test_impl.py

+19
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
from sentry.auth.providers.fly.provider import FlyOAuth2Provider
2+
from sentry.models.authidentity import AuthIdentity
3+
from sentry.models.authprovider import AuthProvider
14
from sentry.services.hybrid_cloud.user.service import user_service
25
from sentry.testutils.cases import TestCase
36
from sentry.testutils.silo import control_silo_test
@@ -28,3 +31,19 @@ def test_get_user_ci(self):
2831
user = self.create_user(email="[email protected]")
2932
fetched_user = user_service.get_or_create_user_by_email(email="[email protected]")
3033
assert user.id == fetched_user.id
34+
35+
def test_get_user_with_ident(self):
36+
user1 = self.create_user(email="[email protected]", username="1")
37+
user2 = self.create_user(email="[email protected]", username="2")
38+
org = self.create_organization(slug="test")
39+
config_data = FlyOAuth2Provider.build_config(resource={"id": "x1234x"})
40+
partner_user_id = "u4567u"
41+
provider = AuthProvider.objects.create(
42+
organization_id=org.id, provider="fly", config=config_data
43+
)
44+
AuthIdentity.objects.create(auth_provider=provider, user=user2, ident=partner_user_id)
45+
fetched_user = user_service.get_or_create_user_by_email(
46+
email="[email protected]", ident=partner_user_id
47+
)
48+
assert user2.id == fetched_user.id
49+
assert user1.id != fetched_user.id

0 commit comments

Comments
 (0)