Skip to content

Commit

Permalink
fix(member-invite): prevent invites to teams that the inviter is not …
Browse files Browse the repository at this point in the history
…a member of when open team membership is off (#83250)

Fixing a bug with the "Let Members Invite Others" and "Open Team
Membership" settings.

The new behavior is as follows:

- If "Let Members Invite Others" is turned OFF (`disable_member_invite =
True`), members can never send invites
- If "Let Members Invite Others" is turned ON (`disable_member_invite =
False`), then there are 2 cases:
- If "Open Team Membership" is turned ON (`allow_joinleave = True`),
then members can invite members to any team in the org
- If "Open Team Membership is turned OFF (`allow_joinleave = False`)
then members can only invite members to **teams that they are in (NEW
BEHAVIOR)**
  • Loading branch information
ameliahsu authored Jan 10, 2025
1 parent 0123b4a commit d112246
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 14 deletions.
12 changes: 6 additions & 6 deletions src/sentry/api/endpoints/organization_member/details.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,25 +223,25 @@ def put(
status=403,
)

is_member = not (
request.access.has_scope("member:invite") and request.access.has_scope("member:admin")
is_member = not request.access.has_scope("member:admin") and (
request.access.has_scope("member:invite")
)
enable_member_invite = not organization.flags.disable_member_invite
members_can_invite = not organization.flags.disable_member_invite
# Members can only resend invites
reinvite_request_only = set(result.keys()).issubset({"reinvite", "regenerate"})
is_reinvite_request_only = set(result.keys()).issubset({"reinvite", "regenerate"})
# Members can only resend invites that they sent
is_invite_from_user = member.inviter_id == request.user.id

if is_member:
if not enable_member_invite or not member.is_pending:
if not (members_can_invite and member.is_pending and is_reinvite_request_only):
raise PermissionDenied
if not is_invite_from_user:
return Response({"detail": ERR_MEMBER_INVITE}, status=403)

# XXX(dcramer): if/when this expands beyond reinvite we need to check
# access level
if result.get("reinvite"):
if not reinvite_request_only:
if not is_reinvite_request_only:
return Response({"detail": ERR_EDIT_WHEN_REINVITING}, status=403)
if member.is_pending:
if ratelimits.for_organization_member_invite(
Expand Down
25 changes: 25 additions & 0 deletions src/sentry/api/endpoints/organization_member/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,31 @@ def post(self, request: Request, organization) -> Response:
)
return Response({"detail": ERR_RATE_LIMITED}, status=429)

is_member = not request.access.has_scope("member:admin") and (
request.access.has_scope("member:invite")
)
# if Open Team Membership is disabled and Member Invites are enabled, members can only invite members to teams they are in
members_can_only_invite_to_members_teams = (
not organization.flags.allow_joinleave and not organization.flags.disable_member_invite
)
has_team_roles = "teamRoles" in result and bool(result["teamRoles"])

if is_member and members_can_only_invite_to_members_teams and has_team_roles:
requester_teams = set(
OrganizationMember.objects.filter(
organization=organization,
user_id=request.user.id,
user_is_active=True,
).values_list("teams__slug", flat=True)
)
team_slugs = [team.slug for team, _ in result.get("teamRoles")]
# ensure that the requester is a member of all teams they are trying to assign
if not requester_teams.issuperset(team_slugs):
return Response(
{"detail": "You cannot assign members to teams you are not a member of."},
status=400,
)

if (
("teamRoles" in result and result["teamRoles"])
or ("teams" in result and result["teams"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ def test_member_can_only_reinvite(self, mock_send_invite_email):
response = self.get_error_response(
self.organization.slug,
self.curr_invite.id,
reinvite=1,
teams=[foo.slug],
status_code=403,
)
Expand All @@ -240,14 +239,10 @@ def test_member_can_only_reinvite(self, mock_send_invite_email):
response = self.get_error_response(
self.organization.slug,
self.curr_invite.id,
reinvite=1,
teams=[foo.slug],
status_code=403,
)
assert (
response.data.get("detail")
== "You cannot modify member details when resending an invitation. Separate requests are required."
)
assert response.data.get("detail") == "You do not have permission to perform this action."
assert not mock_send_invite_email.mock_calls

@patch("sentry.models.OrganizationMember.send_invite_email")
Expand Down
67 changes: 65 additions & 2 deletions tests/sentry/api/endpoints/test_organization_member_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,9 @@ def invite_all_helper(self, role):
data = {
"email": f"{invite_role}_1@localhost",
"role": invite_role,
"teams": [self.team.slug],
"teamRoles": [
{"teamSlug": self.team.slug, "role": "contributor"},
],
}
if role == "member" or role == "admin":
self.get_error_response(self.organization.slug, **data, status_code=403)
Expand All @@ -553,13 +555,72 @@ def invite_all_helper(self, role):
data = {
"email": f"{invite_role}_2@localhost",
"role": invite_role,
"teams": [self.team.slug],
"teamRoles": [
{"teamSlug": self.team.slug, "role": "contributor"},
],
}
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)

def invite_to_other_team_helper(self, role):
user = self.create_user("inviter@localhost")
self.create_member(user=user, organization=self.organization, role=role, teams=[self.team])
self.login_as(user=user)

other_team = self.create_team(organization=self.organization, name="Moo Deng's Team")

def get_data(email: str, other_team_invite: bool = False):
data = {
"email": f"{email}@localhost",
"role": "member",
"teamRoles": [
{
"teamSlug": other_team.slug if other_team_invite else self.team.slug,
"role": "contributor",
},
],
}
return data

# members can never invite members if disable_member_invite = True
self.organization.flags.allow_joinleave = True
self.organization.flags.disable_member_invite = True
self.organization.save()
response = self.get_error_response(
self.organization.slug, **get_data("foo1"), status_code=403
)
assert response.data.get("detail") == "You do not have permission to perform this action."

self.organization.flags.allow_joinleave = False
self.organization.flags.disable_member_invite = True
self.organization.save()
response = self.get_error_response(
self.organization.slug, **get_data("foo2"), status_code=403
)
assert response.data.get("detail") == "You do not have permission to perform this action."

# members can only invite members to teams they are in if allow_joinleave = False
self.organization.flags.allow_joinleave = False
self.organization.flags.disable_member_invite = False
self.organization.save()
self.get_success_response(self.organization.slug, **get_data("foo3"), status_code=201)
response = self.get_error_response(
self.organization.slug, **get_data("foo4", True), status_code=400
)
assert (
response.data.get("detail")
== "You cannot assign members to teams you are not a member of."
)

# members can invite member to any team if allow_joinleave = True
self.organization.flags.allow_joinleave = True
self.organization.flags.disable_member_invite = False
self.organization.save()
self.get_success_response(self.organization.slug, **get_data("foo5"), status_code=201)
self.get_success_response(self.organization.slug, **get_data("foo6", True), status_code=201)

def test_owner_invites(self):
self.invite_all_helper("owner")

Expand All @@ -568,9 +629,11 @@ def test_manager_invites(self):

def test_admin_invites(self):
self.invite_all_helper("admin")
self.invite_to_other_team_helper("admin")

def test_member_invites(self):
self.invite_all_helper("member")
self.invite_to_other_team_helper("member")

def test_respects_feature_flag(self):
user = self.create_user("[email protected]")
Expand Down

0 comments on commit d112246

Please sign in to comment.