fix(backup): Keep `OrganizationMember`s with external inviters (#71576)

There was previously a bug with how we exported `OrganizationMember`s:
we assumed that all `invited_id` would also be in the organization, and
therefore included in the filtering `pk_map`, but this is not actually
true. This is because it is possible for an instance admin who is not a
member of this (or really, any) org to invite the user. This creates an
exception in the general assumption that "all org scope models reference
user IDs already in the org".

To get around this, we simply wrap the exporting function for
`OrganizationMember` in such a way that we no longer filter by
`inviter_id`; we simply ignore the filed and take all values. The code
already has an allowance that enables it to ignore unknown `inviter_id`
values at import time, so doing this is okay.

To enable this change, a number of tests have been robustified with
different kinds of `OrganizationMember` instances: pending invites,
members invited by users in the org, members invited by users outside of
the org, admins, superadmins, etc. We should have much better coverage
of all of the edge cases associated with this model going forward.

fixtures/backup/fresh-install.json

@@ -55,6 +55,14 @@
     "date_added": "2023-06-22T22:59:56.531Z"
   }
 },
+{
+  "model": "sentry.email",
+  "pk": 3,
+  "fields": {
+    "email": "superadmin@example.com",
+    "date_added": "2023-06-22T22:59:57.531Z"
+  }
+},
 {
   "model": "sentry.organization",
   "pk": 1,
@@ -118,6 +126,31 @@
     "avatar_url": null
   }
 },
+{
+  "model": "sentry.user",
+  "pk": 3,
+  "fields": {
+    "password": "pbkdf2_sha256$720000$Qr71byRoUAtqx37gux7FZLx3Wa7mVU1gL2KbL8Iozk/Fw9dVd/93cwBk=",
+    "last_login": null,
+    "username": "superadmin@example.com",
+    "name": "",
+    "email": "superadmin@example.com",
+    "is_staff": true,
+    "is_active": true,
+    "is_superuser": true,
+    "is_managed": false,
+    "is_sentry_app": null,
+    "is_password_expired": false,
+    "is_unclaimed": false,
+    "last_password_change": "2023-06-22T22:59:59.023Z",
+    "flags": "0",
+    "session_nonce": null,
+    "date_joined": "2023-06-22T22:59:57.488Z",
+    "last_active": "2023-06-22T22:59:57.489Z",
+    "avatar_type": 0,
+    "avatar_url": null
+  }
+},
 {
   "model": "sentry.relayusage",
   "pk": 1,
@@ -184,6 +217,17 @@
     "is_verified": false
   }
 },
+{
+  "model": "sentry.useremail",
+  "pk": 3,
+  "fields": {
+    "user": 3,
+    "email": "superadmin@example.com",
+    "validation_hash": "VSj0hfPGiq1UdkHDg8p49sOP9saYvIwH",
+    "date_hash_added": "2023-06-22T22:59:57.521Z",
+    "is_verified": true
+  }
+},
 {
   "model": "sentry.userrole",
   "pk": 1,
@@ -204,6 +248,16 @@
     "role": 1
   }
 },
+{
+  "model": "sentry.userroleuser",
+  "pk": 2,
+  "fields": {
+    "date_updated": "2023-06-22T23:00:00.456Z",
+    "date_added": "2023-06-22T22:59:58.000Z",
+    "user": 3,
+    "role": 1
+  }
+},
 {
   "model": "sentry.team",
   "pk": 1,
@@ -250,13 +304,33 @@
     "date_added": "2023-06-22T22:59:56.561Z",
     "token_expires_at": null,
     "has_global_access": true,
-    "inviter_id": null,
+    "inviter_id": 3,
     "invite_status": 0,
     "type": 50,
     "user_is_active": true,
     "user_email": "member@example.com"
   }
 },
+{
+  "model": "sentry.organizationmember",
+  "pk": 3,
+  "fields": {
+    "organization": 1,
+    "user_id": null,
+    "email": "pending@example.com",
+    "role": "member",
+    "flags": "0",
+    "token": null,
+    "date_added": "2023-06-22T22:59:56.561Z",
+    "token_expires_at": null,
+    "has_global_access": true,
+    "inviter_id": 1,
+    "invite_status": 1,
+    "type": 50,
+    "user_is_active": true,
+    "user_email": null
+  }
+},
 {
   "model": "sentry.project",
   "pk": 1,

src/sentry/backup/comparators.py

@@ -817,7 +817,6 @@ def get_default_comparators() -> dict[str, list[JSONScrubbingComparator]]:
             "sentry.organizationintegration": [DateUpdatedComparator("date_updated")],
             "sentry.organizationmember": [
                 HashObfuscatingComparator("token"),
-                EqualOrRemovedComparator("inviter_id"),
             ],
             "sentry.projectkey": [
                 HashObfuscatingComparator("public_key", "secret_key"),

src/sentry/db/postgres/base.py

@@ -93,7 +93,7 @@ class CursorWrapper:
 
 class DatabaseWrapper(DjangoDatabaseWrapper):
     SchemaEditorClass = DatabaseSchemaEditorProxy
-    queries_limit = 10000
+    queries_limit = 12000
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)

src/sentry/models/organizationmember.py

@@ -644,6 +644,29 @@ class OrganizationMember(ReplicatedRegionModel):
             mapping=rpc_org_member_update,
         )
 
+    @classmethod
+    def query_for_relocation_export(cls, q: Q, pk_map: PrimaryKeyMap) -> Q:
+        q = super().query_for_relocation_export(q, pk_map)
+
+        # Manually avoid filtering on `inviter_id` when exporting. This ensures that
+        # `OrganizationMember`s that were invited by a user from a different organization are not
+        # filtered out when export in `Organization` scope.
+        new_q = Q()
+        for clause in q.children:
+            if not isinstance(clause, Q):
+                new_q.children.append(clause)
+                continue
+
+            mentioned_inviter = False
+            for subclause in clause.children:
+                if isinstance(subclause, tuple) and "inviter" in subclause[0]:
+                    mentioned_inviter = True
+                    break
+            if not mentioned_inviter:
+                new_q.children.append(clause)
+
+        return new_q
+
     def normalize_before_relocation_import(
         self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
     ) -> int | None:

src/sentry/testutils/helpers/backups.py

@@ -365,7 +365,9 @@ class BackupTestCase(TransactionTestCase):
         owner: User,
         member: User,
         other_members: list[User] | None = None,
-        invites: dict[User, str] | None = None,
+        pending_invites: dict[User, str] | None = None,
+        # A dictionary of a user to the other users they invited
+        accepted_invites: dict[User, list[User]] | None = None,
     ) -> Organization:
         org = self.create_organization(name=slug, owner=owner)
         owner_id: BoundedBigAutoField = owner.id
@@ -373,8 +375,8 @@ class BackupTestCase(TransactionTestCase):
         if other_members:
             for user in other_members:
                 self.create_member(organization=org, user=user, role="member")
-        if invites:
-            for inviter, email in invites.items():
+        if pending_invites:
+            for inviter, email in pending_invites.items():
                 OrganizationMember.objects.create(
                     organization_id=org.id,
                     role="member",
@@ -382,6 +384,12 @@ class BackupTestCase(TransactionTestCase):
                     inviter_id=inviter.id,
                     invite_status=InviteStatus.REQUESTED_TO_BE_INVITED.value,
                 )
+        if accepted_invites:
+            for inviter, users in accepted_invites.items():
+                for user in users:
+                    self.create_member(
+                        organization=org, user=user, role="member", inviter_id=inviter.id
+                    )
 
         OrganizationOption.objects.create(
             organization=org, key="sentry:account-rate-limit", value=0
@@ -644,15 +652,9 @@ class BackupTestCase(TransactionTestCase):
 
     @assume_test_silo_mode(SiloMode.CONTROL)
     def create_exhaustive_global_configs(self, owner: User):
+        self.create_exhaustive_api_keys_for_user(owner)
         self.create_exhaustive_global_configs_regional()
         ControlOption.objects.create(key="bar", value="b")
-        ApiAuthorization.objects.create(user=owner)
-        ApiToken.objects.create(
-            user=owner,
-            expires_at=None,
-            name="create_exhaustive_global_configs",
-            token_type=AuthTokenType.USER,
-        )
 
     @assume_test_silo_mode(SiloMode.REGION)
     def create_exhaustive_global_configs_regional(self):
@@ -669,14 +671,39 @@ class BackupTestCase(TransactionTestCase):
         extensions, and all global flags set.
         """
 
-        owner = self.create_exhaustive_user(
-            "owner", is_admin=is_superadmin, is_superuser=is_superadmin, is_staff=is_superadmin
+        superadmin = self.create_exhaustive_user(
+            "superadmin", is_admin=is_superadmin, is_superuser=is_superadmin, is_staff=is_superadmin
         )
+        owner = self.create_exhaustive_user("owner")
         member = self.create_exhaustive_user("member")
-        org = self.create_exhaustive_organization("test-org", owner, member)
+        org = self.create_exhaustive_organization(
+            "test-org",
+            owner,
+            member,
+            pending_invites={
+                superadmin: "invited-by-superadmin-not-in-org@example.com",
+                owner: "invited-by-org-owner@example.com",
+                member: "invited-by-org-member@example.com",
+            },
+            accepted_invites={
+                superadmin: [self.create_exhaustive_user("added-by-superadmin-not-in-org")],
+                owner: [self.create_exhaustive_user("added-by-org-owner")],
+                member: [self.create_exhaustive_user("added-by-org-member")],
+            },
+        )
         self.create_exhaustive_sentry_app("test app", owner, org)
         self.create_exhaustive_global_configs(owner)
 
+    @assume_test_silo_mode(SiloMode.CONTROL)
+    def create_exhaustive_api_keys_for_user(self, user: User):
+        ApiAuthorization.objects.create(user=user)
+        ApiToken.objects.create(
+            user=user,
+            expires_at=None,
+            name=f"create_exhaustive_global_configs_for_{user.name}",
+            token_type=AuthTokenType.USER,
+        )
+
     def import_export_then_validate(self, out_name, *, reset_pks: bool = True) -> Any:
         return import_export_then_validate(out_name, reset_pks=reset_pks)
 

tests/sentry/backup/snapshots/SanitizationExhaustiveTests/test_clean_pks.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2024-01-01T00:00:00+00:00'
+created: '2024-05-28T16:45:57.037010+00:00'
 creator: sentry
 source: tests/sentry/backup/test_sanitize.py
 ---
@@ -115,7 +115,10 @@ source: tests/sentry/backup/test_sanitize.py
   ordinal: 5
   sanitized_fields:
   - date_joined
+  - email
   - last_active
+  - last_password_change
+  - password
   - username
 - model_name: sentry.user
   ordinal: 6
@@ -126,6 +129,39 @@ source: tests/sentry/backup/test_sanitize.py
   - last_password_change
   - password
   - username
+- model_name: sentry.user
+  ordinal: 7
+  sanitized_fields:
+  - date_joined
+  - email
+  - last_active
+  - last_password_change
+  - password
+  - username
+- model_name: sentry.user
+  ordinal: 8
+  sanitized_fields:
+  - date_joined
+  - email
+  - last_active
+  - last_password_change
+  - password
+  - username
+- model_name: sentry.user
+  ordinal: 9
+  sanitized_fields:
+  - date_joined
+  - last_active
+  - username
+- model_name: sentry.user
+  ordinal: 10
+  sanitized_fields:
+  - date_joined
+  - email
+  - last_active
+  - last_password_change
+  - password
+  - username
 - model_name: sentry.userip
   ordinal: 1
   sanitized_fields:
@@ -138,6 +174,30 @@ source: tests/sentry/backup/test_sanitize.py
   - first_seen
   - ip_address
   - last_seen
+- model_name: sentry.userip
+  ordinal: 3
+  sanitized_fields:
+  - first_seen
+  - ip_address
+  - last_seen
+- model_name: sentry.userip
+  ordinal: 4
+  sanitized_fields:
+  - first_seen
+  - ip_address
+  - last_seen
+- model_name: sentry.userip
+  ordinal: 5
+  sanitized_fields:
+  - first_seen
+  - ip_address
+  - last_seen
+- model_name: sentry.userip
+  ordinal: 6
+  sanitized_fields:
+  - first_seen
+  - ip_address
+  - last_seen
 - model_name: sentry.userpermission
   ordinal: 1
   sanitized_fields: []
@@ -201,6 +261,33 @@ source: tests/sentry/backup/test_sanitize.py
   ordinal: 2
   sanitized_fields:
   - date_added
+- model_name: sentry.organizationmember
+  ordinal: 3
+  sanitized_fields:
+  - date_added
+  - email
+- model_name: sentry.organizationmember
+  ordinal: 4
+  sanitized_fields:
+  - date_added
+  - email
+- model_name: sentry.organizationmember
+  ordinal: 5
+  sanitized_fields:
+  - date_added
+  - email
+- model_name: sentry.organizationmember
+  ordinal: 6
+  sanitized_fields:
+  - date_added
+- model_name: sentry.organizationmember
+  ordinal: 7
+  sanitized_fields:
+  - date_added
+- model_name: sentry.organizationmember
+  ordinal: 8
+  sanitized_fields:
+  - date_added
 - model_name: sentry.organizationaccessrequest
   ordinal: 1
   sanitized_fields: []
@@ -239,11 +326,31 @@ source: tests/sentry/backup/test_sanitize.py
   ordinal: 5
   sanitized_fields:
   - date_added
+  - email
 - model_name: sentry.email
   ordinal: 6
   sanitized_fields:
   - date_added
   - email
+- model_name: sentry.email
+  ordinal: 7
+  sanitized_fields:
+  - date_added
+  - email
+- model_name: sentry.email
+  ordinal: 8
+  sanitized_fields:
+  - date_added
+  - email
+- model_name: sentry.email
+  ordinal: 9
+  sanitized_fields:
+  - date_added
+- model_name: sentry.email
+  ordinal: 10
+  sanitized_fields:
+  - date_added
+  - email
 - model_name: sentry.dashboardtombstone
   ordinal: 1
   sanitized_fields:
@@ -283,6 +390,22 @@ source: tests/sentry/backup/test_sanitize.py
   ordinal: 2
   sanitized_fields:
   - created_at
+- model_name: sentry.authenticator
+  ordinal: 3
+  sanitized_fields:
+  - created_at
+- model_name: sentry.authenticator
+  ordinal: 4
+  sanitized_fields:
+  - created_at
+- model_name: sentry.authenticator
+  ordinal: 5
+  sanitized_fields:
+  - created_at
+- model_name: sentry.authenticator
+  ordinal: 6
+  sanitized_fields:
+  - created_at
 - model_name: sentry.apikey
   ordinal: 1
   sanitized_fields:
@@ -302,6 +425,18 @@ source: tests/sentry/backup/test_sanitize.py
 - model_name: sentry.useroption
   ordinal: 2
   sanitized_fields: []
+- model_name: sentry.useroption
+  ordinal: 3
+  sanitized_fields: []
+- model_name: sentry.useroption
+  ordinal: 4
+  sanitized_fields: []
+- model_name: sentry.useroption
+  ordinal: 5
+  sanitized_fields: []
+- model_name: sentry.useroption
+  ordinal: 6
+  sanitized_fields: []
 - model_name: sentry.useremail
   ordinal: 1
   sanitized_fields:
@@ -330,6 +465,7 @@ source: tests/sentry/backup/test_sanitize.py
   ordinal: 5
   sanitized_fields:
   - date_hash_added
+  - email
   - validation_hash
 - model_name: sentry.useremail
   ordinal: 6
@@ -337,6 +473,29 @@ source: tests/sentry/backup/test_sanitize.py
   - date_hash_added
   - email
   - validation_hash
+- model_name: sentry.useremail
+  ordinal: 7
+  sanitized_fields:
+  - date_hash_added
+  - email
+  - validation_hash
+- model_name: sentry.useremail
+  ordinal: 8
+  sanitized_fields:
+  - date_hash_added
+  - email
+  - validation_hash
+- model_name: sentry.useremail
+  ordinal: 9
+  sanitized_fields:
+  - date_hash_added
+  - validation_hash
+- model_name: sentry.useremail
+  ordinal: 10
+  sanitized_fields:
+  - date_hash_added
+  - email
+  - validation_hash
 - model_name: sentry.snubaquery
   ordinal: 1
   sanitized_fields:

tests/sentry/backup/snapshots/SanitizationIntegrationTests/test_fresh_install.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2024-05-08T18:10:03.554472+00:00'
+created: '2024-05-28T16:45:51.971601+00:00'
 creator: sentry
 source: tests/sentry/backup/test_sanitize.py
 ---
@@ -29,6 +29,11 @@ source: tests/sentry/backup/test_sanitize.py
   sanitized_fields:
   - date_added
   - email
+- model_name: sentry.email
+  ordinal: 3
+  sanitized_fields:
+  - date_added
+  - email
 - model_name: sentry.organization
   ordinal: 1
   sanitized_fields:
@@ -53,6 +58,15 @@ source: tests/sentry/backup/test_sanitize.py
   - last_password_change
   - password
   - username
+- model_name: sentry.user
+  ordinal: 3
+  sanitized_fields:
+  - date_joined
+  - email
+  - last_active
+  - last_password_change
+  - password
+  - username
 - model_name: sentry.relayusage
   ordinal: 1
   sanitized_fields:
@@ -85,6 +99,12 @@ source: tests/sentry/backup/test_sanitize.py
   - date_hash_added
   - email
   - validation_hash
+- model_name: sentry.useremail
+  ordinal: 3
+  sanitized_fields:
+  - date_hash_added
+  - email
+  - validation_hash
 - model_name: sentry.userrole
   ordinal: 1
   sanitized_fields:
@@ -96,6 +116,11 @@ source: tests/sentry/backup/test_sanitize.py
   sanitized_fields:
   - date_added
   - date_updated
+- model_name: sentry.userroleuser
+  ordinal: 2
+  sanitized_fields:
+  - date_added
+  - date_updated
 - model_name: sentry.team
   ordinal: 1
   sanitized_fields:
@@ -110,6 +135,11 @@ source: tests/sentry/backup/test_sanitize.py
   ordinal: 2
   sanitized_fields:
   - date_added
+- model_name: sentry.organizationmember
+  ordinal: 3
+  sanitized_fields:
+  - date_added
+  - email
 - model_name: sentry.project
   ordinal: 1
   sanitized_fields:

tests/sentry/backup/snapshots/test_comparators/test_default_comparators.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2024-05-09T23:29:47.969852+00:00'
+created: '2024-05-28T16:54:51.531380+00:00'
 creator: sentry
 source: tests/sentry/backup/test_comparators.py
 ---
@@ -922,9 +922,6 @@ source: tests/sentry/backup/test_comparators.py
   - class: HashObfuscatingComparator
     fields:
     - token
-  - class: EqualOrRemovedComparator
-    fields:
-    - inviter_id
   - class: DatetimeEqualityComparator
     fields:
     - date_added

tests/sentry/backup/test_exports.py

@@ -130,9 +130,12 @@ class ScopingTests(ExportTestCase):
     @freeze_time("2023-10-11 18:00:00")
     def test_config_export_scoping(self):
         self.create_exhaustive_instance(is_superadmin=True)
-        self.create_exhaustive_user("admin", is_admin=True)
-        self.create_exhaustive_user("staff", is_staff=True)
-        self.create_exhaustive_user("superuser", is_superuser=True)
+        admin = self.create_exhaustive_user("admin", is_admin=True)
+        staff = self.create_exhaustive_user("staff", is_staff=True)
+        superuser = self.create_exhaustive_user("superuser", is_superuser=True)
+        self.create_exhaustive_api_keys_for_user(admin)
+        self.create_exhaustive_api_keys_for_user(staff)
+        self.create_exhaustive_api_keys_for_user(superuser)
         with tempfile.TemporaryDirectory() as tmp_dir:
             unencrypted = self.export(tmp_dir, scope=ExportScope.Config)
             self.verify_model_inclusion(unencrypted, ExportScope.Config)