Browse Source

Allow member to invite other roles based on scopes not priority (#56648)

Alternative to
https://github.com/getsentry/getsentry/compare/master...mdtro/h1/billing-user-fix

Here we don't rely on total order between all roles (member < admin <
manager < owner)
but figure out allowed to invite role based on whether scopes of invitee
is a subset of scopes of inviter.

The idea is to not allow privilege escalation via invites.
Alexander Tarasov 1 year ago
parent
commit
a6f09279fa

+ 2 - 2
src/sentry/models/organizationmember.py

@@ -649,8 +649,8 @@ class OrganizationMember(Model):
         Return a list of org-level roles which that member could invite
         Must check if member member has member:admin first before checking
         """
-        highest_role_priority = self.get_all_org_roles_sorted()[0].priority
-        return [r for r in organization_roles.get_all() if r.priority <= highest_role_priority]
+        member_scopes = self.get_scopes()
+        return [r for r in organization_roles.get_all() if r.scopes.issubset(member_scopes)]
 
     def is_only_owner(self) -> bool:
         if organization_roles.get_top_dog().id not in self.get_all_org_roles():

+ 54 - 0
tests/sentry/models/test_organizationmember.py

@@ -28,6 +28,32 @@ from sentry.testutils.outbox import outbox_runner
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
 
 
+class MockOrganizationRoles:
+    TEST_ORG_ROLES = [
+        {"id": "alice", "name": "Alice", "scopes": ["project:read", "project:write"]},
+        {"id": "bob", "name": "Bob", "scopes": ["project:read"]},
+        {"id": "carol", "name": "Carol", "scopes": ["project:write"]},
+    ]
+
+    TEST_TEAM_ROLES = [
+        {"id": "alice", "name": "Alice"},
+        {"id": "bob", "name": "Bob"},
+        {"id": "carol", "name": "Carol"},
+    ]
+
+    def __init__(self):
+        from sentry.roles.manager import RoleManager
+
+        self.default_manager = RoleManager(self.TEST_ORG_ROLES, self.TEST_TEAM_ROLES)
+        self.organization_roles = self.default_manager.organization_roles
+
+    def get_all(self):
+        return self.organization_roles.get_all()
+
+    def get(self, x):
+        return self.organization_roles.get(x)
+
+
 @region_silo_test(stable=True)
 class OrganizationMemberTest(TestCase, HybridCloudTestMixin):
     def test_legacy_token_generation(self):
@@ -505,6 +531,34 @@ class OrganizationMemberTest(TestCase, HybridCloudTestMixin):
             roles.get("manager"),
         ]
 
+    def test_get_allowed_org_roles_to_invite_subset_logic(self):
+        mock_org_roles = MockOrganizationRoles()
+        with patch("sentry.roles.organization_roles.get", mock_org_roles.get), patch(
+            "sentry.roles.organization_roles.get_all", mock_org_roles.get_all
+        ):
+            alice = self.create_member(
+                user=self.create_user(), organization=self.organization, role="alice"
+            )
+            assert alice.get_allowed_org_roles_to_invite() == [
+                roles.get("alice"),
+                roles.get("bob"),
+                roles.get("carol"),
+            ]
+
+            bob = self.create_member(
+                user=self.create_user(), organization=self.organization, role="bob"
+            )
+            assert bob.get_allowed_org_roles_to_invite() == [
+                roles.get("bob"),
+            ]
+
+            carol = self.create_member(
+                user=self.create_user(), organization=self.organization, role="carol"
+            )
+            assert carol.get_allowed_org_roles_to_invite() == [
+                roles.get("carol"),
+            ]
+
     def test_org_roles_by_source(self):
         manager_team = self.create_team(organization=self.organization, org_role="manager")
         owner_team = self.create_team(organization=self.organization, org_role="owner")