Skip to content

Commit 92f4192

Browse files
authored
fix(hybridcloud) Use celery for unlink delivery (#67838)
Move sso unlink operations and email delivery to a celery task that is triggered by the existing RPC method.
1 parent a2200ee commit 92f4192

File tree

3 files changed

+30
-23
lines changed

3 files changed

+30
-23
lines changed

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
from sentry.services.hybrid_cloud.user import RpcUser
6262
from sentry.services.hybrid_cloud.util import flags_to_bits
6363
from sentry.silo import unguarded_write
64+
from sentry.tasks.auth import email_unlink_notifications
6465
from sentry.types.region import find_regions_for_orgs
6566
from sentry.utils.audit import create_org_delete_log
6667

@@ -606,24 +607,14 @@ def send_sso_unlink_emails(
606607
from sentry.auth.exceptions import ProviderNotRegistered
607608

608609
try:
609-
provider = manager.get(provider_key)
610+
manager.get(provider_key)
610611
except ProviderNotRegistered as e:
611612
logger.warning("Could not send SSO unlink emails: %s", e)
612613
return
613614

614-
with unguarded_write(using=router.db_for_write(OrganizationMember)):
615-
# Flags are not replicated -- these updates are safe to skip outboxes
616-
OrganizationMember.objects.filter(organization_id=organization_id).update(
617-
flags=F("flags")
618-
.bitand(~OrganizationMember.flags["sso:linked"])
619-
.bitand(~OrganizationMember.flags["sso:invalid"])
620-
)
621-
622-
member_list = OrganizationMember.objects.filter(
623-
organization_id=organization_id,
615+
email_unlink_notifications.delay(
616+
org_id=organization_id, sending_user_email=sending_user_email, provider_key=provider_key
624617
)
625-
for member in member_list:
626-
member.send_sso_unlink_email(sending_user_email, provider)
627618

628619
def count_members_without_sso(self, *, organization_id: int) -> int:
629620
return OrganizationMember.objects.filter(

src/sentry/tasks/auth.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import abc
44
import logging
55

6+
from django.db import router
7+
from django.db.models import F
68
from django.urls import reverse
79

810
from sentry import audit_log, features, options
@@ -16,6 +18,7 @@
1618
from sentry.services.hybrid_cloud.user import RpcUser
1719
from sentry.services.hybrid_cloud.user.service import user_service
1820
from sentry.silo import SiloMode
21+
from sentry.silo.safety import unguarded_write
1922
from sentry.tasks.base import instrumented_task, retry
2023
from sentry.types.region import RegionMappingNotFound
2124
from sentry.utils.audit import create_audit_entry_from_user
@@ -60,18 +63,23 @@ def _email_missing_links(org_id: int, sending_user_id: int, provider_key: str) -
6063
@instrumented_task(
6164
name="sentry.tasks.email_unlink_notifications", queue="auth", silo_mode=SiloMode.REGION
6265
)
63-
def email_unlink_notifications(org_id: int, actor_id: int, provider_key: str):
66+
def email_unlink_notifications(
67+
org_id: int, sending_user_email: str, provider_key: str, actor_id: int | None = None
68+
):
6469
try:
6570
org = Organization.objects.get(id=org_id)
6671
provider = manager.get(provider_key)
6772
except (Organization.DoesNotExist, ProviderNotRegistered) as e:
6873
logger.warning("Could not send SSO unlink emails: %s", e)
6974
return
7075

71-
user = user_service.get_user(user_id=actor_id)
72-
if not user:
73-
logger.warning("sso.unlink.email_failure.could_not_find_user", extra={"user_id": actor_id})
74-
return
76+
with unguarded_write(using=router.db_for_write(OrganizationMember)):
77+
# Flags are not replicated -- these updates are safe to skip outboxes
78+
OrganizationMember.objects.filter(organization_id=org_id).update(
79+
flags=F("flags")
80+
.bitand(~OrganizationMember.flags["sso:linked"])
81+
.bitand(~OrganizationMember.flags["sso:invalid"])
82+
)
7583

7684
# Email all organization users, even if they never linked their accounts.
7785
# This provides a better experience in the case where SSO is enabled and
@@ -80,7 +88,7 @@ def email_unlink_notifications(org_id: int, actor_id: int, provider_key: str):
8088
# intermittently -- force an ordering in your test!
8189
members = OrganizationMember.objects.filter(organization=org, user_id__isnull=False)
8290
for member in members:
83-
member.send_sso_unlink_email(user, provider)
91+
member.send_sso_unlink_email(sending_user_email, provider)
8492

8593

8694
class OrganizationComplianceTask(abc.ABC):

tests/sentry/tasks/test_auth.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,13 @@ def setUp(self):
102102
self.provider = AuthProvider.objects.create(
103103
organization_id=self.organization.id, provider="dummy"
104104
)
105-
om = self.create_member(
105+
self.om = self.create_member(
106106
user_id=self.user.id,
107107
organization=self.organization,
108108
flags=OrganizationMember.flags["sso:linked"],
109109
)
110110

111-
assert om.flags["sso:linked"]
111+
assert self.om.flags["sso:linked"]
112112
self.user2 = self.create_user(email="baz@example.com")
113113

114114
om2 = self.create_member(user_id=self.user2.id, organization=self.organization, flags=0)
@@ -119,23 +119,31 @@ def setUp(self):
119119

120120
def test_email_unlink_notifications_with_password(self):
121121
with self.tasks():
122-
email_unlink_notifications(self.organization.id, self.user.id, self.provider.provider)
122+
email_unlink_notifications(
123+
self.organization.id, self.user.email, self.provider.provider
124+
)
123125

124126
emails = sorted(message.body for message in mail.outbox)
125127
assert len(emails) == 2
126128
assert f"can now login using your email {self.user.email}, and password" in emails[0]
127129
assert "you'll first have to set a password" not in emails[0]
130+
self.om.refresh_from_db()
131+
assert not self.om.flags["sso:linked"]
128132

129133
def test_email_unlink_notifications_without_password(self):
130134
with assume_test_silo_mode(SiloMode.CONTROL):
131135
self.user.password = ""
132136
self.user.save()
133137

134138
with self.tasks():
135-
email_unlink_notifications(self.organization.id, self.user.id, self.provider.provider)
139+
email_unlink_notifications(
140+
self.organization.id, self.user.email, self.provider.provider
141+
)
136142

137143
emails = sorted(message.body for message in mail.outbox)
138144
assert len(emails) == 2
139145
assert "you'll first have to set a password" in emails[0]
140146
assert f"can now login using your email {self.user.email}, and password" not in emails[0]
141147
assert f"can now login using your email {self.user2.email}, and password" in emails[1]
148+
self.om.refresh_from_db()
149+
assert not self.om.flags["sso:linked"]

0 commit comments

Comments
 (0)