Browse Source

fix(auth) Add unique index for org (#80996)

This is re-write of https://github.com/getsentry/sentry/pull/80566/files

We want ApiAuthorization to be unique for each user and application if
org is null. That means if application is not scoped to specific org.

But also unique per org, user, application in case application is org
scoped. For these applications all ApiAuthorization objects will have
non null organization id.
Athena Moghaddam 3 months ago
parent
commit
43a617ad50

+ 5 - 0
fixtures/backup/model_dependencies/detailed.json

@@ -668,6 +668,11 @@
     ],
     "table_name": "sentry_apiauthorization",
     "uniques": [
+      [
+        "application",
+        "organization_id",
+        "user"
+      ],
       [
         "application",
         "user"

+ 1 - 1
migrations_lockfile.txt

@@ -10,7 +10,7 @@ hybridcloud: 0016_add_control_cacheversion
 nodestore: 0002_nodestore_no_dictfield
 remote_subscriptions: 0003_drop_remote_subscription
 replays: 0004_index_together
-sentry: 0791_add_hashing_metadata_to_grouphash_metadata
+sentry: 0792_add_unique_index_apiauthorization
 social_auth: 0002_default_auto_field
 uptime: 0018_add_trace_sampling_field_to_uptime
 workflow_engine: 0012_data_source_type_change

+ 48 - 0
src/sentry/migrations/0792_add_unique_index_apiauthorization.py

@@ -0,0 +1,48 @@
+# Generated by Django 5.1.1 on 2024-11-19 17:37
+
+from django.db import migrations, models
+
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production.
+    # This should only be used for operations where it's safe to run the migration after your
+    # code has deployed. So this should not be used for most operations that alter the schema
+    # of a table.
+    # Here are some things that make sense to mark as post deployment:
+    # - Large data migrations. Typically we want these to be run manually so that they can be
+    #   monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = False
+
+    dependencies = [
+        ("sentry", "0791_add_hashing_metadata_to_grouphash_metadata"),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="apiauthorization",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("organization_id__isnull", True)),
+                fields=("user", "application"),
+                name="apiauthorization_user_app",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="apiauthorization",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("organization_id__isnull", False)),
+                fields=("user", "application", "organization_id"),
+                name="apiauthorization_user_app_org",
+            ),
+        ),
+        migrations.AlterUniqueTogether(
+            name="apiauthorization",
+            unique_together=set(),
+        ),
+    ]

+ 13 - 1
src/sentry/models/apiauthorization.py

@@ -1,4 +1,5 @@
 from django.db import models
+from django.db.models import Q
 from django.utils import timezone
 
 from sentry.backup.scopes import RelocationScope
@@ -35,7 +36,18 @@ class ApiAuthorization(Model, HasApiScopes):
     class Meta:
         app_label = "sentry"
         db_table = "sentry_apiauthorization"
-        unique_together = (("user", "application"),)
+        constraints = [
+            models.UniqueConstraint(
+                fields=["user", "application"],
+                name="apiauthorization_user_app",
+                condition=Q(organization_id__isnull=True),
+            ),
+            models.UniqueConstraint(
+                fields=["user", "application", "organization_id"],
+                name="apiauthorization_user_app_org",
+                condition=Q(organization_id__isnull=False),
+            ),
+        ]
 
     __repr__ = sane_repr("user_id", "application_id")