From a25149ed349971923675e0af15611b816db3e1ab Mon Sep 17 00:00:00 2001 From: Mia Hsu <55610339+ameliahsu@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:00:41 -0700 Subject: [PATCH] feat(onboarding): create `member:invite` scope (#76364) created `member:invite` scope to allow members without `member:write` access to add new members to an organization without being able to edit them --- src/sentry/api/bases/organizationmember.py | 28 ++++++- .../endpoints/organization_member/__init__.py | 7 +- .../endpoints/organization_member/index.py | 2 +- src/sentry/conf/server.py | 17 ++-- .../api/bases/test_organizationmember.py | 19 +++++ .../test_organization_member_index.py | 82 ++++++++++--------- 6 files changed, 110 insertions(+), 45 deletions(-) diff --git a/src/sentry/api/bases/organizationmember.py b/src/sentry/api/bases/organizationmember.py index b03d38ab3d9905..4f322056d3a701 100644 --- a/src/sentry/api/bases/organizationmember.py +++ b/src/sentry/api/bases/organizationmember.py @@ -5,11 +5,16 @@ from rest_framework import serializers from rest_framework.request import Request +from sentry import features from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.permissions import StaffPermissionMixin from sentry.db.models.fields.bounded import BoundedAutoField from sentry.models.organization import Organization from sentry.models.organizationmember import InviteStatus, OrganizationMember +from sentry.organizations.services.organization.model import ( + RpcOrganization, + RpcUserOrganizationContext, +) from .organization import OrganizationEndpoint, OrganizationPermission @@ -17,11 +22,32 @@ class MemberPermission(OrganizationPermission): scope_map = { "GET": ["member:read", "member:write", "member:admin"], - "POST": ["member:write", "member:admin"], + "POST": ["member:write", "member:admin", "member:invite"], "PUT": ["member:write", "member:admin"], "DELETE": ["member:admin"], } + def has_object_permission( + self, + request: Request, + view: object, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ) -> bool: + if not super().has_object_permission(request, view, organization): + return False + + if request.method != "POST": + return True + + scopes = request.access.scopes + is_role_above_member = "member:admin" in scopes or "member:write" in scopes + if isinstance(organization, RpcUserOrganizationContext): + organization = organization.organization + return is_role_above_member or ( + features.has("organizations:members-invite-teammates", organization) + and not organization.flags.disable_member_invite + ) + class MemberAndStaffPermission(StaffPermissionMixin, MemberPermission): """Allows staff to access member endpoints.""" diff --git a/src/sentry/api/endpoints/organization_member/__init__.py b/src/sentry/api/endpoints/organization_member/__init__.py index 7ca89130b044f8..6bdbc201e145ad 100644 --- a/src/sentry/api/endpoints/organization_member/__init__.py +++ b/src/sentry/api/endpoints/organization_member/__init__.py @@ -94,6 +94,7 @@ def get_allowed_org_roles( request: Request, organization: Organization, member: OrganizationMember | None = None, + creating_org_invite: bool = False, ) -> Collection[Role]: """ Get the set of org-level roles that the request is allowed to manage. @@ -101,11 +102,15 @@ def get_allowed_org_roles( In order to change another member's role, the returned set must include both the starting role and the new role. That is, the set contains the roles that the request is allowed to promote someone to and to demote someone from. + + If the request is to invite a new member, the member:admin scope is not required. """ if is_active_superuser(request): return roles.get_all() - if not request.access.has_scope("member:admin"): + + # The member:admin scope is not required to invite a new member (when creating_org_invite is True). + if not request.access.has_scope("member:admin") and not creating_org_invite: return () if member is None: diff --git a/src/sentry/api/endpoints/organization_member/index.py b/src/sentry/api/endpoints/organization_member/index.py index c73317ac1132f0..fb2821d23c41d3 100644 --- a/src/sentry/api/endpoints/organization_member/index.py +++ b/src/sentry/api/endpoints/organization_member/index.py @@ -319,7 +319,7 @@ def post(self, request: Request, organization) -> Response: {"organization": "Your organization is not allowed to invite members"}, status=403 ) - allowed_roles = get_allowed_org_roles(request, organization) + allowed_roles = get_allowed_org_roles(request, organization, creating_org_invite=True) assigned_org_role = request.data.get("orgRole") or request.data.get("role") # We allow requests from integration tokens to invite new members as the member role only diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index aee7e4c41ab0b7..6fc095c5c2acc4 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -1834,6 +1834,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "org:ci", # "org:superuser", Do not use for any type of superuser permission/access checks # Assigned to active SU sessions in src/sentry/auth/access.py to enable UI elements + "member:invite", "member:read", "member:write", "member:admin", @@ -1871,9 +1872,10 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "org:admin": {"org:read", "org:write", "org:admin", "org:integrations"}, "org:integrations": {"org:integrations"}, "org:ci": {"org:ci"}, + "member:invite": {"member:read", "member:invite"}, "member:read": {"member:read"}, - "member:write": {"member:read", "member:write"}, - "member:admin": {"member:read", "member:write", "member:admin"}, + "member:write": {"member:read", "member:invite", "member:write"}, + "member:admin": {"member:read", "member:invite", "member:write", "member:admin"}, "team:read": {"team:read"}, "team:write": {"team:read", "team:write"}, "team:admin": {"team:read", "team:write", "team:admin"}, @@ -1902,6 +1904,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: ("member:admin", "Read, write, and admin access to organization members."), ("member:write", "Read and write access to organization members."), ("member:read", "Read access to organization members."), + ("member:invite", "Member invite access to organization members."), ), ( ("team:admin", "Read, write, and admin access to teams."), @@ -1945,7 +1948,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: { "id": "member", "name": "Member", - "desc": "Members can view and act on events, as well as view most other data within the organization.", + "desc": "Members can view and act on events, as well as view most other data within the organization. By default, they can invite members to the organization unless the organization has disabled this feature.", "scopes": { "event:read", "event:write", @@ -1953,6 +1956,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "project:releases", "project:read", "org:read", + "member:invite", "member:read", "team:read", "alerts:read", @@ -1968,8 +1972,8 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: create new teams and projects, as well as remove teams and projects on which they already hold membership (or all teams, if open membership is enabled). Additionally, they can manage memberships of - teams that they are members of. They cannot invite members to the - organization. + teams that they are members of. By default, they can invite members + to the organization unless the organization has disabled this feature. """ ), "scopes": { @@ -1978,6 +1982,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "event:admin", "org:read", "member:read", + "member:invite", "project:read", "project:write", "project:admin", @@ -1999,6 +2004,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "event:read", "event:write", "event:admin", + "member:invite", "member:read", "member:write", "member:admin", @@ -2032,6 +2038,7 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "org:write", "org:admin", "org:integrations", + "member:invite", "member:read", "member:write", "member:admin", diff --git a/tests/sentry/api/bases/test_organizationmember.py b/tests/sentry/api/bases/test_organizationmember.py index af0f53631cbb61..45a64f9b4f978c 100644 --- a/tests/sentry/api/bases/test_organizationmember.py +++ b/tests/sentry/api/bases/test_organizationmember.py @@ -1,4 +1,5 @@ from sentry.api.bases.organizationmember import MemberAndStaffPermission, MemberPermission +from sentry.testutils.helpers import Feature from tests.sentry.api.bases.test_organization import PermissionBaseTestCase @@ -29,6 +30,15 @@ def test_org_member(self): assert not self.has_object_perm("POST", self.org, user=member_user) assert not self.has_object_perm("DELETE", self.org, user=member_user) + with Feature({"organizations:members-invite-teammates": True}): + self.org.flags.disable_member_invite = False + self.org.save() + assert self.has_object_perm("POST", self.org, user=member_user) + + self.org.flags.disable_member_invite = True + self.org.save() + assert not self.has_object_perm("POST", self.org, user=member_user) + def test_org_admin(self): admin_user = self.create_user() self.create_member(user=admin_user, organization=self.org, role="admin") @@ -37,6 +47,15 @@ def test_org_admin(self): assert not self.has_object_perm("POST", self.org, user=admin_user) assert not self.has_object_perm("DELETE", self.org, user=admin_user) + with Feature({"organizations:members-invite-teammates": True}): + self.org.flags.disable_member_invite = False + self.org.save() + assert self.has_object_perm("POST", self.org, user=admin_user) + + self.org.flags.disable_member_invite = True + self.org.save() + assert not self.has_object_perm("POST", self.org, user=admin_user) + def test_org_manager(self): manager_user = self.create_user() self.create_member(user=manager_user, organization=self.org, role="manager") diff --git a/tests/sentry/api/endpoints/test_organization_member_index.py b/tests/sentry/api/endpoints/test_organization_member_index.py index 0b31ff54c13fb5..3ece837da375cb 100644 --- a/tests/sentry/api/endpoints/test_organization_member_index.py +++ b/tests/sentry/api/endpoints/test_organization_member_index.py @@ -498,49 +498,57 @@ def test_cannot_invite_retired_role_with_flag(self): class OrganizationMemberPermissionRoleTest(OrganizationMemberListTestBase, HybridCloudTestMixin): method = "post" - def test_manager_invites(self): - manager_user = self.create_user("manager@localhost") - self.manager = self.create_member( - user=manager_user, organization=self.organization, role="manager" - ) - self.login_as(user=manager_user) - - data = {"email": "eric1@localhost", "role": "owner", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=400) + @with_feature("organizations:members-invite-teammates") + def invite_all_helper(self, role): + invite_roles = ["owner", "manager", "member"] + + user = self.create_user("user@localhost") + member = self.create_member(user=user, organization=self.organization, role=role) + self.login_as(user=user) + + self.organization.flags.disable_member_invite = True + self.organization.save() + + allowed_roles = member.get_allowed_org_roles_to_invite() + + for invite_role in invite_roles: + data = { + "email": f"{invite_role}_1@localhost", + "role": invite_role, + "teams": [self.team.slug], + } + if role == "member" or role == "admin": + self.get_error_response(self.organization.slug, **data, status_code=403) + elif any(invite_role == allowed_role.id for allowed_role in allowed_roles): + self.get_success_response(self.organization.slug, **data, status_code=201) + else: + self.get_error_response(self.organization.slug, **data, status_code=400) + + self.organization.flags.disable_member_invite = False + self.organization.save() + + for invite_role in invite_roles: + data = { + "email": f"{invite_role}_2@localhost", + "role": invite_role, + "teams": [self.team.slug], + } + if any(invite_role == allowed_role.id for allowed_role in allowed_roles): + self.get_success_response(self.organization.slug, **data, status_code=201) + else: + self.get_error_response(self.organization.slug, **data, status_code=400) - data = {"email": "eric2@localhost", "role": "manager", "teams": [self.team.slug]} - self.get_success_response(self.organization.slug, **data, status_code=201) + def test_owner_invites(self): + self.invite_all_helper("owner") - data = {"email": "eric3@localhost", "role": "member", "teams": [self.team.slug]} - self.get_success_response(self.organization.slug, **data, status_code=201) + def test_manager_invites(self): + self.invite_all_helper("manager") def test_admin_invites(self): - admin_user = self.create_user("admin22@localhost") - self.create_member(user=admin_user, organization=self.organization, role="admin") - self.login_as(user=admin_user) - - data = {"email": "eric1@localhost", "role": "owner", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=403) - - data = {"email": "eric2@localhost", "role": "manager", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=403) - - data = {"email": "eric3@localhost", "role": "member", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=403) + self.invite_all_helper("admin") def test_member_invites(self): - member_user = self.create_user("member@localhost") - self.create_member(user=member_user, organization=self.organization, role="member") - self.login_as(user=member_user) - - data = {"email": "eric1@localhost", "role": "owner", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=403) - - data = {"email": "eric2@localhost", "role": "manager", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=403) - - data = {"email": "eric3@localhost", "role": "member", "teams": [self.team.slug]} - self.get_error_response(self.organization.slug, **data, status_code=403) + self.invite_all_helper("member") def test_respects_feature_flag(self): user = self.create_user("baz@example.com")