Skip to content

Commit

Permalink
Allow member to invite other roles based on scopes not priority (#56648)
Browse files Browse the repository at this point in the history
Alternative to
getsentry/getsentry@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.
  • Loading branch information
oioki authored Sep 27, 2023
1 parent b320f46 commit a6f0927
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/sentry/models/organizationmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
54 changes: 54 additions & 0 deletions tests/sentry/models/test_organizationmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit a6f0927

Please sign in to comment.