From a6f09279faed1db11742e4d810497c3bb99677f8 Mon Sep 17 00:00:00 2001 From: Alexander Tarasov Date: Wed, 27 Sep 2023 10:57:42 +0200 Subject: [PATCH] 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. --- src/sentry/models/organizationmember.py | 4 +- .../sentry/models/test_organizationmember.py | 54 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/sentry/models/organizationmember.py b/src/sentry/models/organizationmember.py index 7fc443890deb78..07177304271ac4 100644 --- a/src/sentry/models/organizationmember.py +++ b/src/sentry/models/organizationmember.py @@ -649,8 +649,8 @@ def get_allowed_org_roles_to_invite(self): 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(): diff --git a/tests/sentry/models/test_organizationmember.py b/tests/sentry/models/test_organizationmember.py index 8e66fe2e0e1c7b..bb9e11de8018c0 100644 --- a/tests/sentry/models/test_organizationmember.py +++ b/tests/sentry/models/test_organizationmember.py @@ -28,6 +28,32 @@ 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 @@ def test_get_allowed_org_roles_to_invite(self): 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")