Skip to content

Commit ac055f5

Browse files
azaslavskyc298lee
authored andcommitted
fix(relocation): Ensure the correct email gets verified (#68721)
In github.com//pull/68586, we verified AN email that matches the user's. Now, we filter on user_id, to ensure that we verify that specific user's email.
1 parent 8fc9b8e commit ac055f5

File tree

4 files changed

+31
-13
lines changed

4 files changed

+31
-13
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,15 @@ def get_user_by_email(
237237
return serialize_rpc_user(user)
238238
return None
239239

240+
def verify_user_email(self, *, email: str, user_id: int) -> bool:
241+
user_email = UserEmail.objects.filter(email__iexact=email, user_id=user_id).first()
242+
if user_email is None:
243+
return False
244+
if not user_email.is_verified:
245+
user_email.update(is_verified=True)
246+
return True
247+
return False
248+
240249
def verify_any_email(self, *, email: str) -> bool:
241250
user_email = UserEmail.objects.filter(email__iexact=email).first()
242251
if user_email is None:

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ def get_user_by_email(
156156
) -> RpcUser | None:
157157
pass
158158

159+
@rpc_method
160+
@abstractmethod
161+
def verify_user_email(self, *, email: str, user_id: int) -> bool:
162+
pass
163+
159164
@rpc_method
160165
@abstractmethod
161166
def verify_any_email(self, *, email: str) -> bool:

src/sentry/web/frontend/accounts.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ def recover_confirm(request, user_id, hash, mode="recover"):
205205
# associated with this account in particular, since that is the only one the user
206206
# claiming email could have been sent to.
207207
rpc_user = user_service.get_user(user_id=user.id)
208-
user_service.verify_any_email(email=user.email)
208+
user_service.verify_user_email(email=user.email, user_id=user.id)
209209
orgs = organization_service.get_organizations_by_user_and_scope(
210210
region_name=mapping.region_name, user=rpc_user
211211
)

tests/sentry/web/frontend/test_accounts.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,7 @@ def test_relocate_recovery_post_multiple_orgs(self, terms_accepted_signal_mock):
170170
self.create_member(user=user, organization=org1)
171171
self.create_member(user=user, organization=org2)
172172

173-
resp = self.client.post(self.path, {"user": user.email})
174-
assert resp.status_code == 200
175-
176-
lost_password = LostPasswordHash.objects.get(user=user)
173+
lost_password = LostPasswordHash.objects.create(user=user)
177174
user.is_unclaimed = True
178175
user.save()
179176
old_password = user.password
@@ -219,19 +216,23 @@ def test_relocate_recovery_post_multiple_orgs(self, terms_accepted_signal_mock):
219216
assert UserEmail.objects.get(email=user.email).is_verified
220217

221218
@patch("sentry.signals.terms_accepted.send_robust")
222-
def test_relocate_recovery_post(self, terms_accepted_signal_mock):
223-
user = self.create_user(email="test@example.com")
224-
user_email = UserEmail.objects.get(email=user.email)
219+
def test_relocate_recovery_post_another_user_with_same_email(self, terms_accepted_signal_mock):
220+
same_email_user = self.create_user(username="same_email_as_first", email="test@example.com")
221+
same_email_user_email = UserEmail.objects.get(
222+
email=same_email_user.email, user_id=same_email_user.id
223+
)
224+
same_email_user_email.is_verified = False
225+
same_email_user_email.save()
226+
227+
user = self.create_user(username="first", email="test@example.com")
228+
user_email = UserEmail.objects.get(email=user.email, user_id=user.id)
225229
user_email.is_verified = False
226230
user_email.save()
227231

228232
org = self.create_organization()
229233
self.create_member(user=user, organization=org)
230234

231-
resp = self.client.post(self.path, {"user": user.email})
232-
assert resp.status_code == 200
233-
234-
lost_password = LostPasswordHash.objects.get(user=user)
235+
lost_password = LostPasswordHash.objects.create(user=user)
235236
user.is_unclaimed = True
236237
user.save()
237238
old_password = user.password
@@ -262,7 +263,10 @@ def test_relocate_recovery_post(self, terms_accepted_signal_mock):
262263
sender=recover_confirm,
263264
)
264265

265-
assert UserEmail.objects.get(email=user.email).is_verified
266+
assert UserEmail.objects.get(email=user.email, user_id=user.id).is_verified
267+
assert not UserEmail.objects.get(
268+
email=same_email_user_email.email, user_id=same_email_user.id
269+
).is_verified
266270

267271
def test_relocate_recovery_unchecked_tos(self):
268272
user = self.create_user()

0 commit comments

Comments
 (0)