feat(admin): Impove user account merging (#72692)

We now ensure that user account merges are much more complete. All
exportable (and even some non-exportable!) data associated with the
user-being-merged is now properly pointed at the newly combined `User`
and `OrganizationMember` models, and awkward collisions (what if the
users had invited one another to a certain team?) are handled
gracefully.

Author: azaslavsky

src/sentry/backup/dependencies.py

@@ -7,7 +7,7 @@
 from typing import NamedTuple
 
 from django.db import models
-from django.db.models import UniqueConstraint
+from django.db.models import Q, UniqueConstraint
 from django.db.models.fields.related import ForeignKey, OneToOneField
 
 from sentry.backup.helpers import EXCLUDED_APPS
@@ -701,3 +701,28 @@ def get_exportable_sentry_models() -> set[type[models.base.Model]]:
             get_final_derivations_of(BaseModel),
         )
     )
+
+
+def merge_users_for_model_in_org(
+    model: type[models.base.Model], *, organization_id: int, from_user_id: int, to_user_id: int
+) -> None:
+    """
+    All instances of this model in a certain organization that reference both the organization and
+    user in question will be pointed at the new user instead.
+    """
+
+    from sentry.models.organization import Organization
+    from sentry.models.user import User
+
+    model_relations = dependencies()[get_model_name(model)]
+    user_refs = {k for k, v in model_relations.foreign_keys.items() if v.model == User}
+    org_refs = {
+        k if k.endswith("_id") else f"{k}_id"
+        for k, v in model_relations.foreign_keys.items()
+        if v.model == Organization
+    }
+    for_this_org = Q(**{field_name: organization_id for field_name in org_refs})
+    for user_ref in user_refs:
+        q = for_this_org & Q(**{user_ref: from_user_id})
+        obj = model.objects.filter(q)
+        obj.update(**{user_ref: to_user_id})

src/sentry/models/user.py

@@ -26,6 +26,7 @@
     NormalizedModelName,
     PrimaryKeyMap,
     get_model_name,
+    merge_users_for_model_in_org,
 )
 from sentry.backup.helpers import ImportFlags
 from sentry.backup.sanitize import SanitizableField, Sanitizer
@@ -38,13 +39,15 @@
     sane_repr,
 )
 from sentry.db.models.utils import unique_db_instance
+from sentry.db.postgres.transactions import enforce_constraints
 from sentry.integrations.types import EXTERNAL_PROVIDERS, ExternalProviders
 from sentry.locks import locks
 from sentry.models.authenticator import Authenticator
 from sentry.models.avatars import UserAvatar
 from sentry.models.lostpasswordhash import LostPasswordHash
 from sentry.models.organizationmapping import OrganizationMapping
 from sentry.models.organizationmembermapping import OrganizationMemberMapping
+from sentry.models.orgauthtoken import OrgAuthToken
 from sentry.models.outbox import ControlOutboxBase, OutboxCategory, outbox_context
 from sentry.services.hybrid_cloud.organization import RpcRegionUser, organization_service
 from sentry.services.hybrid_cloud.user import RpcUser
@@ -332,7 +335,7 @@ def outboxes_for_user_update(
             shard_identifier=identifier,
         )
 
-    def merge_to(from_user, to_user):
+    def merge_to(from_user: User, to_user: User) -> None:
         # TODO: we could discover relations automatically and make this useful
         from sentry.models.auditlogentry import AuditLogEntry
         from sentry.models.authenticator import Authenticator
@@ -343,33 +346,56 @@ def merge_to(from_user, to_user):
         from sentry.models.organizationmembermapping import OrganizationMemberMapping
         from sentry.models.useremail import UserEmail
 
+        from_user_id = from_user.id
+        to_user_id = to_user.id
+
         audit_logger.info(
-            "user.merge", extra={"from_user_id": from_user.id, "to_user_id": to_user.id}
+            "user.merge", extra={"from_user_id": from_user_id, "to_user_id": to_user_id}
         )
 
         organization_ids: list[int]
         organization_ids = OrganizationMemberMapping.objects.filter(
-            user_id=from_user.id
+            user_id=from_user_id
         ).values_list("organization_id", flat=True)
 
         for organization_id in organization_ids:
             organization_service.merge_users(
-                organization_id=organization_id, from_user_id=from_user.id, to_user_id=to_user.id
+                organization_id=organization_id, from_user_id=from_user_id, to_user_id=to_user_id
             )
 
-        model_list: tuple[type[BaseModel], ...] = (
+            # Update all organization control models to only use the new user id.
+            #
+            # TODO: in the future, proactively update `OrganizationMemberTeamReplica` as well.
+            with enforce_constraints(
+                transaction.atomic(using=router.db_for_write(OrganizationMemberMapping))
+            ):
+                control_side_org_models: tuple[type[BaseModel], ...] = (
+                    OrgAuthToken,
+                    OrganizationMemberMapping,
+                )
+                for model in control_side_org_models:
+                    merge_users_for_model_in_org(
+                        model,
+                        organization_id=organization_id,
+                        from_user_id=from_user_id,
+                        to_user_id=to_user_id,
+                    )
+
+        # While it would be nice to make the following changes in a transaction, there are too many
+        # unique constraints to make this feasible. Instead, we just do it sequentially and ignore
+        # the `IntegrityError`s.
+        user_related_models: tuple[type[BaseModel], ...] = (
             Authenticator,
             Identity,
             UserAvatar,
             UserEmail,
             UserOption,
         )
-
-        for model in model_list:
-            for obj in model.objects.filter(user_id=from_user.id):
+        for model in user_related_models:
+            for obj in model.objects.filter(user_id=from_user_id):
                 try:
                     with transaction.atomic(using=router.db_for_write(User)):
-                        obj.update(user_id=to_user.id)
+                        obj.update(user_id=to_user_id)
                 except IntegrityError:
                     pass
 
@@ -385,7 +411,7 @@ def merge_to(from_user, to_user):
             for ai in AuthIdentity.objects.filter(
                 user=from_user,
                 auth_provider__organization_id__in=AuthIdentity.objects.filter(
-                    user_id=to_user.id
+                    user_id=to_user_id
                 ).values("auth_provider__organization_id"),
             ):
                 ai.delete()

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

@@ -3,26 +3,39 @@
 from collections.abc import Mapping
 from typing import Any
 
-from django.db import IntegrityError, models, router, transaction
+from django.db import models, router, transaction
 from django.db.models.expressions import CombinedExpression, F
 from django.dispatch import Signal
 
 from sentry import roles
 from sentry.api.serializers import serialize
+from sentry.backup.dependencies import merge_users_for_model_in_org
 from sentry.db.postgres.transactions import enforce_constraints
+from sentry.incidents.models.alert_rule import AlertRule, AlertRuleActivity
+from sentry.incidents.models.incident import IncidentActivity, IncidentSubscription
 from sentry.models.activity import Activity
+from sentry.models.dashboard import Dashboard
+from sentry.models.dynamicsampling import CustomDynamicSamplingRule
 from sentry.models.groupassignee import GroupAssignee
 from sentry.models.groupbookmark import GroupBookmark
+from sentry.models.groupsearchview import GroupSearchView
 from sentry.models.groupseen import GroupSeen
 from sentry.models.groupshare import GroupShare
 from sentry.models.groupsubscription import GroupSubscription
 from sentry.models.organization import Organization, OrganizationStatus
+from sentry.models.organizationaccessrequest import OrganizationAccessRequest
 from sentry.models.organizationmapping import OrganizationMapping
 from sentry.models.organizationmember import InviteStatus, OrganizationMember
 from sentry.models.organizationmemberteam import OrganizationMemberTeam
 from sentry.models.outbox import ControlOutbox, OutboxCategory, OutboxScope, outbox_context
+from sentry.models.projectbookmark import ProjectBookmark
+from sentry.models.recentsearch import RecentSearch
+from sentry.models.rule import Rule, RuleActivity
+from sentry.models.rulesnooze import RuleSnooze
+from sentry.models.savedsearch import SavedSearch
 from sentry.models.scheduledeletion import RegionScheduledDeletion
 from sentry.models.team import Team, TeamStatus
+from sentry.monitors.models import Monitor
 from sentry.services.hybrid_cloud import OptionValue, logger
 from sentry.services.hybrid_cloud.app import app_service
 from sentry.services.hybrid_cloud.organization import (
@@ -515,33 +528,75 @@ def merge_users(self, *, organization_id: int, from_user_id: int, to_user_id: in
 
         assert to_member
 
-        for team in from_member.teams.all():
-            try:
-                with enforce_constraints(
-                    transaction.atomic(router.db_for_write(OrganizationMemberTeam))
-                ):
+        with enforce_constraints(transaction.atomic(using=router.db_for_write(OrganizationMember))):
+            # Delete all org access requests between the two now-merged users.
+            OrganizationAccessRequest.objects.filter(
+                member=from_member, requester_id=to_user_id
+            ).delete()
+            OrganizationAccessRequest.objects.filter(
+                member=to_member, requester_id=from_user_id
+            ).delete()
+
+            # All other org access requests should be pointed from the old member to the new
+            # one.
+            reqs = OrganizationAccessRequest.objects.filter(member=from_member)
+            for req in reqs:
+                req.member = to_member
+                req.save()
+
+            # Move all old team memberships to the newly merged `OrganizationMember`.
+            for team in from_member.teams.all():
+                OrganizationMemberTeam.objects.filter(
+                    organizationmember=from_member, team=team
+                ).delete()
+                to_member_team = OrganizationMemberTeam.objects.filter(
+                    organizationmember=to_member, team=team
+                ).first()
+                if to_member_team is None:
                     OrganizationMemberTeam.objects.create(organizationmember=to_member, team=team)
-            except IntegrityError:
-                pass
-
-        model_list = (
-            GroupAssignee,
-            GroupBookmark,
-            GroupSeen,
-            GroupShare,
-            GroupSubscription,
-            Activity,
-        )
 
-        for model in model_list:
-            for obj in model.objects.filter(
-                user_id=from_user_id, project__organization_id=organization_id
-            ):
-                try:
-                    with enforce_constraints(transaction.atomic(router.db_for_write(model))):
-                        obj.update(user_id=to_user_id)
-                except IntegrityError:
-                    pass
+            # Update all organization region models to only use the new user id.
+            model_list = [
+                Activity,
+                AlertRule,
+                AlertRuleActivity,
+                CustomDynamicSamplingRule,
+                Dashboard,
+                GroupAssignee,
+                GroupBookmark,
+                GroupSeen,
+                GroupShare,
+                GroupSearchView,
+                GroupSubscription,
+                IncidentActivity,
+                IncidentSubscription,
+                OrganizationAccessRequest,
+                ProjectBookmark,
+                RecentSearch,
+                Rule,
+                RuleActivity,
+                RuleSnooze,
+                SavedSearch,
+            ]
+            for model in model_list:
+                merge_users_for_model_in_org(
+                    model,
+                    organization_id=organization_id,
+                    from_user_id=from_user_id,
+                    to_user_id=to_user_id,
+                )
+
+            # Finally, delete the old member.
+            from_member.delete()
+
+        # TODO: for some reason, `Monitor` insists on being updated outside of the transaction, even
+        # though it's also not region siloed?
+        merge_users_for_model_in_org(
+            Monitor,
+            organization_id=organization_id,
+            from_user_id=from_user_id,
+            to_user_id=to_user_id,
+        )
 
     def reset_idp_flags(self, *, organization_id: int) -> None:
         with unguarded_write(using=router.db_for_write(OrganizationMember)):