From 1936d844acc9e44afb87bf87f4a264af89a18d04 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 22:53:36 +0000 Subject: [PATCH 1/3] fix(teams): Allow team admins to review access requests Co-Authored-By: Ben Henry --- .../organization_access_request_details.py | 11 ++++-- ...est_organization_access_request_details.py | 33 +++++++++++++++++ .../test_organization_access_requests.py | 37 ++++++++++++++++++- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/sentry/api/endpoints/organization_access_request_details.py b/src/sentry/api/endpoints/organization_access_request_details.py index 6b5d89394ee0..c101f3ecc44f 100644 --- a/src/sentry/api/endpoints/organization_access_request_details.py +++ b/src/sentry/api/endpoints/organization_access_request_details.py @@ -35,6 +35,7 @@ class AccessRequestPermission(OrganizationPermission): ], "POST": [], "PUT": [ + "org:read", "org:write", "org:admin", "team:write", @@ -89,14 +90,16 @@ def get(self, request: Request, organization: Organization) -> Response: ).select_related("team", "member") ) - elif request.access.has_scope("team:write") and request.access.team_ids_with_membership: - access_requests = list( - OrganizationAccessRequest.objects.filter( + elif request.access.team_ids_with_membership: + access_requests = [ + access_request + for access_request in OrganizationAccessRequest.objects.filter( member__user_is_active=True, member__user_id__isnull=False, team__id__in=request.access.team_ids_with_membership, ).select_related("team", "member") - ) + if self._can_access(request, access_request) + ] else: # Return empty response if user does not have access return Response([]) diff --git a/tests/sentry/api/endpoints/test_organization_access_request_details.py b/tests/sentry/api/endpoints/test_organization_access_request_details.py index 200a81180375..5a35766e4862 100644 --- a/tests/sentry/api/endpoints/test_organization_access_request_details.py +++ b/tests/sentry/api/endpoints/test_organization_access_request_details.py @@ -3,6 +3,7 @@ from sentry.models.organizationaccessrequest import OrganizationAccessRequest from sentry.models.organizationmemberteam import OrganizationMemberTeam from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers import Feature class GetOrganizationAccessRequestTest(APITestCase): @@ -102,6 +103,38 @@ def test_deny_request(self) -> None: assert not OrganizationAccessRequest.objects.filter(id=access_request.id).exists() def test_team_admin_can_approve(self) -> None: + organization = self.create_organization( + name="foo", + owner=self.user, + flags=0, + ) + user = self.create_user("bar@example.com") + member = self.create_member(organization=organization, user=user, role="member") + team = self.create_team(name="foo", organization=organization) + access_request = OrganizationAccessRequest.objects.create(member=member, team=team) + admin_user = self.create_user("admin@example.com") + self.create_member( + organization=organization, + user=admin_user, + role="member", + teams=[team], + teamRole="admin", + ) + path = reverse( + "sentry-api-0-organization-access-request-details", + args=[organization.slug, access_request.id], + ) + + self.login_as(admin_user) + with Feature({"organizations:team-roles": True}): + resp = self.client.put(path, data={"isApproved": 1}) + + assert resp.status_code == 204 + assert OrganizationMemberTeam.objects.filter( + organizationmember=member, team=team, is_active=True + ).exists() + + def test_legacy_org_admin_can_approve(self) -> None: self.login_as(user=self.user) organization = self.create_organization(name="foo", owner=self.user) diff --git a/tests/sentry/api/endpoints/test_organization_access_requests.py b/tests/sentry/api/endpoints/test_organization_access_requests.py index 64675e87f32f..eac9cf879dd2 100644 --- a/tests/sentry/api/endpoints/test_organization_access_requests.py +++ b/tests/sentry/api/endpoints/test_organization_access_requests.py @@ -2,6 +2,7 @@ from sentry.models.organizationaccessrequest import OrganizationAccessRequest from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers import Feature class UpdateOrganizationAccessRequestTest(APITestCase): @@ -59,6 +60,40 @@ def test_admin_can_list_access_requests(self) -> None: assert resp.data[0]["member"]["id"] == str(request_1.member_id) assert resp.data[0]["team"]["id"] == str(request_1.team_id) + def test_team_admin_only_lists_requests_for_managed_teams(self) -> None: + organization = self.create_organization( + name="foo", + owner=self.user, + flags=0, + ) + managed_team = self.create_team(name="managed", organization=organization) + other_team = self.create_team(name="other", organization=organization) + team_admin = self.create_user("admin@example.com") + self.create_member( + organization=organization, + user=team_admin, + role="member", + teams=[managed_team], + teamRole="admin", + ) + requester = self.create_member( + organization=organization, + user=self.create_user("requester@example.com"), + role="member", + ) + managed_request = OrganizationAccessRequest.objects.create( + member=requester, team=managed_team + ) + OrganizationAccessRequest.objects.create(member=requester, team=other_team) + path = reverse("sentry-api-0-organization-access-requests", args=[organization.slug]) + + self.login_as(team_admin) + with Feature({"organizations:team-roles": True}): + resp = self.client.get(path) + + assert resp.status_code == 200 + assert [item["id"] for item in resp.data] == [str(managed_request.id)] + def test_member_empty_results(self) -> None: self.login_as(user=self.user) @@ -70,7 +105,7 @@ def test_member_empty_results(self) -> None: OrganizationAccessRequest.objects.create(member=member, team=team) user = self.create_user("foo@example.com") - member = self.create_member(organization=organization, user=user, role="member") + self.create_member(organization=organization, user=user, role="member") path = reverse("sentry-api-0-organization-access-requests", args=[organization.slug]) From fff8a784c7ddc27e1de4ca55da89753ab2ea81d3 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 00:31:59 +0000 Subject: [PATCH 2/3] perf(teams): Limit access request membership queries --- .../organization_access_request_details.py | 41 ++++++++++++------- .../test_organization_access_requests.py | 3 +- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/sentry/api/endpoints/organization_access_request_details.py b/src/sentry/api/endpoints/organization_access_request_details.py index c101f3ecc44f..7619bd4e246c 100644 --- a/src/sentry/api/endpoints/organization_access_request_details.py +++ b/src/sentry/api/endpoints/organization_access_request_details.py @@ -14,8 +14,8 @@ from sentry.api.serializers import serialize from sentry.models.organization import Organization from sentry.models.organizationaccessrequest import OrganizationAccessRequest -from sentry.models.organizationmember import OrganizationMember from sentry.models.organizationmemberteam import OrganizationMemberTeam +from sentry.models.team import Team logger = logging.getLogger(__name__) @@ -90,28 +90,39 @@ def get(self, request: Request, organization: Organization) -> Response: ).select_related("team", "member") ) - elif request.access.team_ids_with_membership: - access_requests = [ - access_request - for access_request in OrganizationAccessRequest.objects.filter( + else: + authorized_team_ids = [ + team.id + for team in Team.objects.filter(id__in=request.access.team_ids_with_membership) + if request.access.has_team_scope(team, "team:write") + ] + if not authorized_team_ids: + return Response([]) + + access_requests = list( + OrganizationAccessRequest.objects.filter( member__user_is_active=True, member__user_id__isnull=False, - team__id__in=request.access.team_ids_with_membership, + team_id__in=authorized_team_ids, ).select_related("team", "member") - if self._can_access(request, access_request) - ] - else: - # Return empty response if user does not have access - return Response([]) + ) - teams_by_user = OrganizationMember.objects.get_teams_by_user(organization=organization) + if not access_requests: + return Response([]) - # We omit any requests which are now redundant (i.e. the user joined that team some other way) + member_ids = {access_request.member_id for access_request in access_requests} + team_ids = {access_request.team_id for access_request in access_requests} + existing_memberships = set( + OrganizationMemberTeam.objects.filter( + organizationmember_id__in=member_ids, + team_id__in=team_ids, + is_active=True, + ).values_list("organizationmember_id", "team_id") + ) valid_access_requests = [ access_request for access_request in access_requests - if access_request.member.user_id is not None - and access_request.team_id not in teams_by_user[access_request.member.user_id] + if (access_request.member_id, access_request.team_id) not in existing_memberships ] return Response(serialize(valid_access_requests, request.user)) diff --git a/tests/sentry/api/endpoints/test_organization_access_requests.py b/tests/sentry/api/endpoints/test_organization_access_requests.py index eac9cf879dd2..5e5904b43d21 100644 --- a/tests/sentry/api/endpoints/test_organization_access_requests.py +++ b/tests/sentry/api/endpoints/test_organization_access_requests.py @@ -69,13 +69,14 @@ def test_team_admin_only_lists_requests_for_managed_teams(self) -> None: managed_team = self.create_team(name="managed", organization=organization) other_team = self.create_team(name="other", organization=organization) team_admin = self.create_user("admin@example.com") - self.create_member( + team_admin_member = self.create_member( organization=organization, user=team_admin, role="member", teams=[managed_team], teamRole="admin", ) + self.create_team_membership(team=other_team, member=team_admin_member, role="contributor") requester = self.create_member( organization=organization, user=self.create_user("requester@example.com"), From 15b7ca749a826d008e93852c29e38b66837b49fa Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:32:10 +0000 Subject: [PATCH 3/3] fix(teams): Exclude inactive access memberships --- .../organization_access_request_details.py | 1 - .../test_organization_access_requests.py | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/sentry/api/endpoints/organization_access_request_details.py b/src/sentry/api/endpoints/organization_access_request_details.py index 7619bd4e246c..87262b5ed286 100644 --- a/src/sentry/api/endpoints/organization_access_request_details.py +++ b/src/sentry/api/endpoints/organization_access_request_details.py @@ -116,7 +116,6 @@ def get(self, request: Request, organization: Organization) -> Response: OrganizationMemberTeam.objects.filter( organizationmember_id__in=member_ids, team_id__in=team_ids, - is_active=True, ).values_list("organizationmember_id", "team_id") ) valid_access_requests = [ diff --git a/tests/sentry/api/endpoints/test_organization_access_requests.py b/tests/sentry/api/endpoints/test_organization_access_requests.py index 5e5904b43d21..1320c68b12cc 100644 --- a/tests/sentry/api/endpoints/test_organization_access_requests.py +++ b/tests/sentry/api/endpoints/test_organization_access_requests.py @@ -24,6 +24,25 @@ def test_owner_can_list_access_requests(self) -> None: assert len(resp.data) == 1 assert resp.data[0]["member"]["email"] == "bar@example.com" + def test_inactive_team_membership_is_not_listed_as_pending(self) -> None: + organization = self.create_organization(name="foo", owner=self.user) + requester = self.create_member( + organization=organization, + user=self.create_user("requester@example.com"), + role="member", + ) + team = self.create_team(name="foo", organization=organization) + membership = self.create_team_membership(team=team, member=requester) + membership.update(is_active=False) + OrganizationAccessRequest.objects.create(member=requester, team=team) + path = reverse("sentry-api-0-organization-access-requests", args=[organization.slug]) + + self.login_as(self.user) + resp = self.client.get(path) + + assert resp.status_code == 200 + assert resp.data == [] + def test_admin_can_list_access_requests(self) -> None: organization = self.create_organization( name="foo",