Skip to content

Commit c050431

Browse files
GabeVillalobosMichaelSun48
authored andcommitted
fix(hybrid-cloud): User deletion outboxes always fan out to all regions (#69363)
1 parent dd99628 commit c050431

File tree

2 files changed

+51
-12
lines changed

2 files changed

+51
-12
lines changed

src/sentry/models/user.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
from sentry.services.hybrid_cloud.organization import RpcRegionUser, organization_service
4343
from sentry.services.hybrid_cloud.user import RpcUser
4444
from sentry.types.integrations import EXTERNAL_PROVIDERS, ExternalProviders
45-
from sentry.types.region import find_regions_for_user
45+
from sentry.types.region import find_all_region_names, find_regions_for_user
4646
from sentry.utils.http import absolute_uri
4747
from sentry.utils.retries import TimedRetryPolicy
4848

@@ -202,7 +202,7 @@ def delete(self):
202202
avatar = self.avatar.first()
203203
if avatar:
204204
avatar.delete()
205-
for outbox in self.outboxes_for_update():
205+
for outbox in self.outboxes_for_update(is_user_delete=True):
206206
outbox.save()
207207
return super().delete()
208208

@@ -305,13 +305,23 @@ def send_confirm_emails(self, is_new_user=False):
305305
for email in email_list:
306306
self.send_confirm_email_singular(email, is_new_user)
307307

308-
def outboxes_for_update(self) -> list[ControlOutboxBase]:
309-
return User.outboxes_for_user_update(self.id)
308+
def outboxes_for_update(self, is_user_delete: bool = False) -> list[ControlOutboxBase]:
309+
return User.outboxes_for_user_update(self.id, is_user_delete=is_user_delete)
310310

311311
@staticmethod
312-
def outboxes_for_user_update(identifier: int) -> list[ControlOutboxBase]:
312+
def outboxes_for_user_update(
313+
identifier: int, is_user_delete: bool = False
314+
) -> list[ControlOutboxBase]:
315+
# User deletions must fan out to all regions to ensure cascade behavior
316+
# of anything with a HybridCloudForeignKey, even if the user is no longer
317+
# a member of any organizations in that region.
318+
if is_user_delete:
319+
user_regions = set(find_all_region_names())
320+
else:
321+
user_regions = find_regions_for_user(identifier)
322+
313323
return OutboxCategory.USER_UPDATE.as_control_outboxes(
314-
region_names=find_regions_for_user(identifier),
324+
region_names=user_regions,
315325
object_identifier=identifier,
316326
shard_identifier=identifier,
317327
)

tests/sentry/models/test_user.py

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
from sentry.testutils.cases import TestCase
1010
from sentry.testutils.hybrid_cloud import HybridCloudTestMixin
1111
from sentry.testutils.outbox import outbox_runner
12-
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
13-
from sentry.types.region import Region, RegionCategory
12+
from sentry.testutils.silo import assume_test_silo_mode, assume_test_silo_mode_of, control_silo_test
13+
from sentry.types.region import Region, RegionCategory, find_regions_for_user
1414

1515
_TEST_REGIONS = (
1616
Region("na", 1, "http://eu.testserver", RegionCategory.MULTI_TENANT),
@@ -34,21 +34,21 @@ def setUp(self):
3434
)
3535

3636
@assume_test_silo_mode(SiloMode.REGION)
37-
def user_tombstone_exists(self) -> bool:
37+
def user_tombstone_exists(self, user_id: int) -> bool:
3838
return RegionTombstone.objects.filter(
39-
table_name="auth_user", object_identifier=self.user_id
39+
table_name="auth_user", object_identifier=user_id
4040
).exists()
4141

4242
@assume_test_silo_mode(SiloMode.REGION)
4343
def get_user_saved_search_count(self) -> int:
4444
return SavedSearch.objects.filter(owner_id=self.user_id).count()
4545

4646
def test_simple(self):
47-
assert not self.user_tombstone_exists()
47+
assert not self.user_tombstone_exists(user_id=self.user_id)
4848
with outbox_runner():
4949
self.user.delete()
5050
assert not User.objects.filter(id=self.user_id).exists()
51-
assert self.user_tombstone_exists()
51+
assert self.user_tombstone_exists(user_id=self.user_id)
5252

5353
# cascade is asynchronous, ensure there is still related search,
5454
assert self.get_user_saved_search_count() == 1
@@ -87,6 +87,35 @@ def test_cascades_to_multiple_regions(self):
8787
schedule_hybrid_cloud_foreign_key_jobs()
8888
assert self.get_user_saved_search_count() == 0
8989

90+
def test_deletions_create_tombstones_in_regions_for_user_with_no_orgs(self):
91+
# Create a user with no org memberships
92+
user_to_delete = self.create_user("foo@example.com")
93+
user_id = user_to_delete.id
94+
with outbox_runner():
95+
user_to_delete.delete()
96+
97+
assert self.user_tombstone_exists(user_id=user_id)
98+
99+
def test_cascades_to_regions_even_if_user_ownership_revoked(self):
100+
eu_org = self.create_organization(region=_TEST_REGIONS[1])
101+
self.create_member(user=self.user, organization=eu_org)
102+
self.create_saved_search(name="eu-search", owner=self.user, organization=eu_org)
103+
assert self.get_user_saved_search_count() == 2
104+
105+
with outbox_runner(), assume_test_silo_mode_of(OrganizationMember):
106+
for member in OrganizationMember.objects.filter(user_id=self.user.id):
107+
member.delete()
108+
109+
assert find_regions_for_user(self.user.id) == set()
110+
111+
with outbox_runner():
112+
self.user.delete()
113+
114+
assert self.get_user_saved_search_count() == 2
115+
with assume_test_silo_mode(SiloMode.REGION), self.tasks():
116+
schedule_hybrid_cloud_foreign_key_jobs()
117+
assert self.get_user_saved_search_count() == 0
118+
90119

91120
@control_silo_test
92121
class UserDetailsTest(TestCase):

0 commit comments

Comments
 (0)