feat: Add permission groups via UserRole (#30545)

Enables easier use of UserPermission by defining roles (e.g. "support") and clustering permissions.

migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0264_use_booleanfield_docintegration
+sentry: 0265_add_userrole
 social_auth: 0001_initial

src/sentry/auth/access.py

@@ -1,6 +1,7 @@
 __all__ = ["from_user", "from_member", "DEFAULT"]
 
 import warnings
+from typing import FrozenSet
 
 import sentry_sdk
 from django.conf import settings
@@ -18,6 +19,7 @@ from sentry.models import (
     SentryApp,
     Team,
     UserPermission,
+    UserRole,
 )
 from sentry.utils.request_cache import request_cache
 
@@ -27,6 +29,10 @@ def get_cached_organization_member(user_id, organization_id):
     return OrganizationMember.objects.get(user_id=user_id, organization_id=organization_id)
 
 
+def get_permissions_for_user(user_id: int) -> FrozenSet[str]:
+    return UserRole.permissions_for_user(user_id) | UserPermission.for_user(user_id)
+
+
 def _sso_params(member):
     """
     Return a tuple of (requires_sso, sso_is_valid) for a given member.
@@ -308,7 +314,7 @@ def from_request(request, organization=None, scopes=None):
             sso_is_valid=sso_is_valid,
             requires_sso=requires_sso,
             has_global_access=True,
-            permissions=UserPermission.for_user(request.user.id),
+            permissions=get_permissions_for_user(request.user.id),
             role=role,
         )
 
@@ -353,14 +359,14 @@ def from_user(user, organization=None, scopes=None, is_superuser=False):
 
     if not organization:
         return OrganizationlessAccess(
-            permissions=UserPermission.for_user(user.id) if is_superuser else ()
+            permissions=get_permissions_for_user(user.id) if is_superuser else ()
         )
 
     try:
         om = get_cached_organization_member(user.id, organization.id)
     except OrganizationMember.DoesNotExist:
         return OrganizationlessAccess(
-            permissions=UserPermission.for_user(user.id) if is_superuser else ()
+            permissions=get_permissions_for_user(user.id) if is_superuser else ()
         )
 
     # ensure cached relation
@@ -397,7 +403,7 @@ def from_member(member, scopes=None, is_superuser=False):
         projects=project_list,
         has_global_access=bool(member.organization.flags.allow_joinleave)
         or roles.get(member.role).is_global,
-        permissions=UserPermission.for_user(member.user_id) if is_superuser else (),
+        permissions=get_permissions_for_user(member.user_id) if is_superuser else (),
         role=member.role,
     )
 

src/sentry/migrations/0265_add_userrole.py

@@ -0,0 +1,92 @@
+# Generated by Django 2.2.24 on 2022-01-06 19:58
+
+import django.db.models.deletion
+import django.utils.timezone
+from django.conf import settings
+from django.db import migrations, models
+
+import sentry.db.models.fields.array
+import sentry.db.models.fields.bounded
+import sentry.db.models.fields.foreignkey
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = False
+
+    # This flag is used to decide whether to run this migration in a transaction or not.
+    # By default we prefer to run in a transaction, but for migrations where you want
+    # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
+    # want to create an index concurrently when adding one to an existing table.
+    # You'll also usually want to set this to `False` if you're writing a data
+    # migration, since we don't want the entire migration to run in one long-running
+    # transaction.
+    atomic = True
+
+    dependencies = [
+        ("sentry", "0264_use_booleanfield_docintegration"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="UserRole",
+            fields=[
+                (
+                    "id",
+                    sentry.db.models.fields.bounded.BoundedBigAutoField(
+                        primary_key=True, serialize=False
+                    ),
+                ),
+                ("date_updated", models.DateTimeField(default=django.utils.timezone.now)),
+                ("date_added", models.DateTimeField(default=django.utils.timezone.now, null=True)),
+                ("name", models.CharField(max_length=32, unique=True)),
+                ("permissions", sentry.db.models.fields.array.ArrayField(null=True)),
+            ],
+            options={
+                "db_table": "sentry_userrole",
+            },
+        ),
+        migrations.CreateModel(
+            name="UserRoleUser",
+            fields=[
+                (
+                    "id",
+                    sentry.db.models.fields.bounded.BoundedBigAutoField(
+                        primary_key=True, serialize=False
+                    ),
+                ),
+                ("date_updated", models.DateTimeField(default=django.utils.timezone.now)),
+                ("date_added", models.DateTimeField(default=django.utils.timezone.now, null=True)),
+                (
+                    "role",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE, to="sentry.UserRole"
+                    ),
+                ),
+                (
+                    "user",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL
+                    ),
+                ),
+            ],
+            options={
+                "db_table": "sentry_userrole_users",
+            },
+        ),
+        migrations.AddField(
+            model_name="userrole",
+            name="users",
+            field=models.ManyToManyField(
+                through="sentry.UserRoleUser", to=settings.AUTH_USER_MODEL
+            ),
+        ),
+    ]

src/sentry/models/__init__.py

@@ -124,3 +124,4 @@ from .userip import *  # NOQA
 from .useroption import *  # NOQA
 from .userpermission import *  # NOQA
 from .userreport import *  # NOQA
+from .userrole import *  # NOQA

src/sentry/models/userpermission.py

@@ -1,3 +1,5 @@
+from typing import FrozenSet
+
 from django.db import models
 
 from sentry.db.models import FlexibleForeignKey, Model, sane_repr
@@ -24,5 +26,8 @@ class UserPermission(Model):
     __repr__ = sane_repr("user_id", "permission")
 
     @classmethod
-    def for_user(cls, user_id):
+    def for_user(cls, user_id: int) -> FrozenSet[str]:
+        """
+        Return a set of permission for the given user ID.
+        """
         return frozenset(cls.objects.filter(user=user_id).values_list("permission", flat=True))

src/sentry/models/userrole.py

@@ -0,0 +1,48 @@
+from typing import FrozenSet
+
+from django.db import models
+
+from sentry.db.models import ArrayField, DefaultFieldsModel, sane_repr
+from sentry.db.models.fields.foreignkey import FlexibleForeignKey
+
+
+class UserRole(DefaultFieldsModel):
+    """
+    Roles are applied to administrative users and apply a set of `UserPermission`.
+    """
+
+    __include_in_export__ = True
+
+    name = models.CharField(max_length=32, unique=True)
+    permissions = ArrayField()
+    users = models.ManyToManyField("sentry.User", through="sentry.UserRoleUser")
+
+    class Meta:
+        app_label = "sentry"
+        db_table = "sentry_userrole"
+
+    __repr__ = sane_repr("name", "permissions")
+
+    @classmethod
+    def permissions_for_user(cls, user_id: int) -> FrozenSet[str]:
+        """
+        Return a set of permission for the given user ID scoped to roles.
+        """
+        return frozenset(
+            i
+            for sl in cls.objects.filter(users=user_id).values_list("permissions", flat=True)
+            for i in sl
+        )
+
+
+class UserRoleUser(DefaultFieldsModel):
+    __include_in_export__ = True
+
+    user = FlexibleForeignKey("sentry.User")
+    role = FlexibleForeignKey("sentry.UserRole")
+
+    class Meta:
+        app_label = "sentry"
+        db_table = "sentry_userrole_users"
+
+    __repr__ = sane_repr("user", "role")

tests/sentry/auth/test_access.py

@@ -3,7 +3,14 @@ from unittest.mock import Mock
 from django.contrib.auth.models import AnonymousUser
 
 from sentry.auth import access
-from sentry.models import AuthIdentity, AuthProvider, ObjectStatus, Organization, UserPermission
+from sentry.models import (
+    AuthIdentity,
+    AuthProvider,
+    ObjectStatus,
+    Organization,
+    UserPermission,
+    UserRole,
+)
 from sentry.testutils import TestCase
 
 
@@ -347,3 +354,16 @@ class DefaultAccessTest(TestCase):
         assert not result.has_project_scope(Mock(), "project:read")
         assert not result.has_project_membership(Mock())
         assert not result.permissions
+
+
+class GetPermissionsForUserTest(TestCase):
+    def test_combines_roles_and_perms(self):
+        user = self.user
+
+        UserPermission.objects.create(user=user, permission="test.permission")
+        role = UserRole.objects.create(name="test.role", permissions=["test.permission-role"])
+        role.users.add(user)
+
+        assert sorted(access.get_permissions_for_user(user.id)) == sorted(
+            ["test.permission", "test.permission-role"]
+        )

tests/sentry/models/test_userrole.py

@@ -0,0 +1,14 @@
+from sentry.models import UserRole
+from sentry.testutils import TestCase
+
+
+class UserRoleTest(TestCase):
+    def test_permissions_for_user(self):
+        user = self.create_user(email="a@example.com")
+        user2 = self.create_user(email="b@example.com")
+        role = UserRole.objects.create(name="test", permissions=["test1", "test2"])
+        role.users.add(user)
+        role2 = UserRole.objects.create(name="test2", permissions=["test2", "test3"])
+        role2.users.add(user)
+        assert sorted(UserRole.permissions_for_user(user.id)) == ["test1", "test2", "test3"]
+        assert sorted(UserRole.permissions_for_user(user2.id)) == []