From 2ec1f8af42d3c5466eaa5a5785d266e5faf58c35 Mon Sep 17 00:00:00 2001 From: geoffg-sentry <165922362+geoffg-sentry@users.noreply.github.com> Date: Fri, 10 Apr 2026 14:34:04 -0400 Subject: [PATCH 1/5] (fix): idors in external issue linking and some hardening --- .../sentry_apps/api/bases/sentryapps.py | 19 ++- .../installation_external_issue_actions.py | 28 +--- .../installation_external_issue_details.py | 4 + .../endpoints/installation_external_issues.py | 4 + .../installation_external_requests.py | 6 +- src/sentry/sentry_apps/services/cell/impl.py | 92 +++++++++++- .../sentry_apps/services/cell/service.py | 3 + ...app_installation_external_issue_details.py | 41 ++++++ ...sentry_app_installation_external_issues.py | 39 +++++- ...ntry_app_installation_external_requests.py | 31 +++- .../sentry_apps/services/test_cell_service.py | 132 ++++++++++++++++++ 11 files changed, 367 insertions(+), 32 deletions(-) diff --git a/src/sentry/sentry_apps/api/bases/sentryapps.py b/src/sentry/sentry_apps/api/bases/sentryapps.py index 637d0a27d64515..5565ee9bd09a38 100644 --- a/src/sentry/sentry_apps/api/bases/sentryapps.py +++ b/src/sentry/sentry_apps/api/bases/sentryapps.py @@ -2,7 +2,7 @@ import logging from collections.abc import Sequence -from typing import Any +from typing import Any, cast import sentry_sdk from rest_framework.permissions import BasePermission @@ -30,6 +30,7 @@ ) from sentry.users.models.user import User from sentry.users.services.user import RpcUser +from sentry.users.services.user.serial import serialize_rpc_user from sentry.users.services.user.service import user_service from sentry.utils.strings import to_single_line_str @@ -38,6 +39,22 @@ logger = logging.getLogger(__name__) +def rpc_user_from_request(request: Request) -> RpcUser: + """ + Unwrap LazyObject request.user and serialize Django User to RpcUser for cell RPC calls. + """ + from django.utils.functional import empty + + user: Any = request.user + if hasattr(user, "_wrapped"): + if user._wrapped is empty: + user._setup() + user = user._wrapped + if isinstance(user, User): + return serialize_rpc_user(user) + return cast(RpcUser, user) + + def ensure_scoped_permission(request: Request, allowed_scopes: Sequence[str] | None) -> bool: """ Verifies the User making the request has at least one required scope for diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py index 73c95280274c68..5dd06e1c76782d 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py @@ -1,4 +1,3 @@ -from django.utils.functional import empty from rest_framework import serializers from rest_framework.request import Request from rest_framework.response import Response @@ -7,27 +6,14 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint from sentry.api.serializers import serialize -from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint +from sentry.sentry_apps.api.bases.sentryapps import ( + SentryAppInstallationBaseEndpoint, + rpc_user_from_request, +) from sentry.sentry_apps.api.serializers.platform_external_issue import ( PlatformExternalIssueSerializer, ) from sentry.sentry_apps.services.cell import sentry_app_cell_service -from sentry.users.models.user import User -from sentry.users.services.user.serial import serialize_rpc_user - - -def _extract_lazy_object(lo): - """ - Unwrap a LazyObject and return the inner object. Whatever that may be. - - ProTip: This is relying on `django.utils.functional.empty`, which may - or may not be removed in the future. It's 100% undocumented. - """ - if not hasattr(lo, "_wrapped"): - return lo - if lo._wrapped is empty: - lo._setup() - return lo._wrapped class SentryAppInstallationExternalIssueActionsSerializer(serializers.Serializer): @@ -57,10 +43,6 @@ def post(self, request: Request, installation) -> Response: action = data.pop("action") uri = data.pop("uri") - user = _extract_lazy_object(request.user) - if isinstance(user, User): - user = serialize_rpc_user(user) - result = sentry_app_cell_service.create_issue_link( organization_id=installation.organization_id, installation=installation, @@ -68,7 +50,7 @@ def post(self, request: Request, installation) -> Response: action=action, fields=data, uri=uri, - user=user, + user=rpc_user_from_request(request), ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py index b00b450121a5d9..3afe2b18011d3f 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @@ -6,6 +6,9 @@ from sentry.sentry_apps.api.bases.sentryapps import ( SentryAppInstallationExternalIssueBaseEndpoint as ExternalIssueBaseEndpoint, ) +from sentry.sentry_apps.api.bases.sentryapps import ( + rpc_user_from_request, +) from sentry.sentry_apps.services.cell import sentry_app_cell_service from sentry.sentry_apps.utils.errors import SentryAppError @@ -29,6 +32,7 @@ def delete(self, request: Request, installation, external_issue_id) -> Response: organization_id=installation.organization_id, installation=installation, external_issue_id=external_issue_id, + user=rpc_user_from_request(request), ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py index e2ebf39ad904b5..27c4bcda005b1b 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @@ -9,6 +9,9 @@ from sentry.sentry_apps.api.bases.sentryapps import ( SentryAppInstallationExternalIssueBaseEndpoint as ExternalIssueBaseEndpoint, ) +from sentry.sentry_apps.api.bases.sentryapps import ( + rpc_user_from_request, +) from sentry.sentry_apps.api.parsers.sentry_app import URLField from sentry.sentry_apps.api.serializers.platform_external_issue import ( PlatformExternalIssueSerializer as ResponsePlatformExternalIssueSerializer, @@ -48,6 +51,7 @@ def post(self, request: Request, installation) -> Response: web_url=data["webUrl"], project=data["project"], identifier=data["identifier"], + user=rpc_user_from_request(request), ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index 8ac4f0c23999d5..fd1173ccc6684a 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -7,7 +7,10 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint -from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint +from sentry.sentry_apps.api.bases.sentryapps import ( + SentryAppInstallationBaseEndpoint, + rpc_user_from_request, +) from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation from sentry.sentry_apps.services.cell import sentry_app_cell_service @@ -30,6 +33,7 @@ def get(self, request: Request, installation: RpcSentryAppInstallation) -> Respo organization_id=installation.organization_id, installation=installation, uri=request.GET.get("uri"), + user=rpc_user_from_request(request), project_id=int(request.GET["projectId"]) if request.GET.get("projectId") else None, query=request.GET.get("query"), dependent_data=request.GET.get("dependentData"), diff --git a/src/sentry/sentry_apps/services/cell/impl.py b/src/sentry/sentry_apps/services/cell/impl.py index 48f42ea67e30c2..2bbadf51a1a8b7 100644 --- a/src/sentry/sentry_apps/services/cell/impl.py +++ b/src/sentry/sentry_apps/services/cell/impl.py @@ -44,6 +44,7 @@ def get_select_options( organization_id: int, installation: RpcSentryAppInstallation, uri: str, + user: RpcUser, project_id: int | None = None, query: str | None = None, dependent_data: str | None = None, @@ -55,8 +56,35 @@ def get_select_options( project_slug: str | None = None if project_id is not None: project = Project.objects.filter(id=project_id, organization_id=organization_id).first() - if project: - project_slug = project.slug + if not project: + return RpcSelectRequesterResult( + error=RpcSentryAppError( + message="Could not find the given project for this organization.", + status_code=404, + ) + ) + try: + organization = Organization.objects.get(id=organization_id) + except Organization.DoesNotExist: + return RpcSelectRequesterResult( + error=RpcSentryAppError( + message="Could not find the given project for this organization.", + status_code=404, + ) + ) + access = self._access_for_installation_user( + organization=organization, + installation=installation, + user=user, + ) + if not access.has_project_access(project): + return RpcSelectRequesterResult( + error=RpcSentryAppError( + message="You do not have permission to access this project.", + status_code=403, + ) + ) + project_slug = project.slug try: result = SelectRequester( @@ -149,12 +177,13 @@ def create_external_issue( web_url: str, project: str, identifier: str, + user: RpcUser, ) -> RpcPlatformExternalIssueResult: """ Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @ POST """ try: - group = Group.objects.get( + group = Group.objects.select_related("project").get( id=group_id, project_id__in=Project.objects.filter(organization_id=organization_id), ) @@ -166,6 +195,29 @@ def create_external_issue( ) ) + try: + organization = Organization.objects.get(id=organization_id) + except Organization.DoesNotExist: + return RpcPlatformExternalIssueResult( + error=RpcSentryAppError( + message="Could not find the corresponding issue for the given issueId", + status_code=404, + ) + ) + + access = self._access_for_installation_user( + organization=organization, + installation=installation, + user=user, + ) + if not access.has_project_access(group.project): + return RpcPlatformExternalIssueResult( + error=RpcSentryAppError( + message="You do not have permission to create an external issue for this issue.", + status_code=403, + ) + ) + try: external_issue = ExternalIssueCreator( install=installation, @@ -187,14 +239,17 @@ def delete_external_issue( organization_id: int, installation: RpcSentryAppInstallation, external_issue_id: int, + user: RpcUser, ) -> RpcEmptyResult: """ Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @ DELETE """ try: - platform_external_issue = PlatformExternalIssue.objects.get( + platform_external_issue = PlatformExternalIssue.objects.select_related( + "group", "group__project", "project" + ).get( id=external_issue_id, - project__organization_id=organization_id, + group__project__organization_id=organization_id, service_type=installation.sentry_app.slug, ) except PlatformExternalIssue.DoesNotExist: @@ -206,6 +261,33 @@ def delete_external_issue( ), ) + issue_project = platform_external_issue.project or platform_external_issue.group.project + + try: + organization = Organization.objects.get(id=organization_id) + except Organization.DoesNotExist: + return RpcEmptyResult( + success=False, + error=RpcSentryAppError( + message="Could not find the corresponding external issue from given external_issue_id", + status_code=404, + ), + ) + + access = self._access_for_installation_user( + organization=organization, + installation=installation, + user=user, + ) + if not access.has_project_access(issue_project): + return RpcEmptyResult( + success=False, + error=RpcSentryAppError( + message="You do not have permission to delete this external issue.", + status_code=403, + ), + ) + deletions.exec_sync(platform_external_issue) return RpcEmptyResult() diff --git a/src/sentry/sentry_apps/services/cell/service.py b/src/sentry/sentry_apps/services/cell/service.py index 953e8a2c6beee3..73cca91e2a1d17 100644 --- a/src/sentry/sentry_apps/services/cell/service.py +++ b/src/sentry/sentry_apps/services/cell/service.py @@ -51,6 +51,7 @@ def get_select_options( organization_id: int, installation: RpcSentryAppInstallation, uri: str, + user: RpcUser, project_id: int | None = None, query: str | None = None, dependent_data: str | None = None, @@ -85,6 +86,7 @@ def create_external_issue( web_url: str, project: str, identifier: str, + user: RpcUser, ) -> RpcPlatformExternalIssueResult: """Invokes ExternalIssueCreator to create an external issue.""" pass @@ -97,6 +99,7 @@ def delete_external_issue( organization_id: int, installation: RpcSentryAppInstallation, external_issue_id: int, + user: RpcUser, ) -> RpcEmptyResult: """Deletes a PlatformExternalIssue.""" pass diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py index 2065d5e475c024..1966e0d60e173f 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py @@ -1,3 +1,4 @@ +from sentry.models.organization import Organization from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @@ -71,3 +72,43 @@ def test_handles_invalid_external_issue_id_format(self) -> None: # Ensure the external issue still exists after failed attempts with assume_test_silo_mode_of(PlatformExternalIssue): assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + + def test_rejects_delete_without_project_access(self) -> None: + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + user_team = self.create_team(organization=self.org, name="sed-user-team") + other_team = self.create_team(organization=self.org, name="sed-other-team") + self.create_project(organization=self.org, teams=[user_team], name="sed-user-proj") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="sed-other-proj" + ) + other_group = self.create_group(project=other_project) + + limited_user = self.create_user() + self.create_member( + organization=self.org, + user=limited_user, + role="member", + teams=[user_team], + teamRole="admin", + ) + + with assume_test_silo_mode_of(PlatformExternalIssue): + other_external_issue = PlatformExternalIssue.objects.create( + group_id=other_group.id, + project_id=other_project.id, + service_type=self.sentry_app.slug, + display_name="Other#1", + web_url="https://example.com/o/1", + ) + + self.login_as(user=limited_user) + self.get_error_response( + self.install.uuid, + other_external_issue.id, + status_code=403, + ) + with assume_test_silo_mode_of(PlatformExternalIssue): + assert PlatformExternalIssue.objects.filter(id=other_external_issue.id).exists() diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py index 4fa34eac7faf01..c7f47d5d420d8b 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py @@ -2,6 +2,7 @@ from django.urls import reverse +from sentry.models.organization import Organization from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @@ -66,7 +67,9 @@ def test_creates_external_issue(self) -> None: def test_invalid_group_id(self) -> None: self._set_up_sentry_app("Testin", ["event:write"]) data = self._post_data() - data["issueId"] = self.create_group(project=self.create_project()).id + other_org = self.create_organization() + other_project = self.create_project(organization=other_org) + data["issueId"] = self.create_group(project=other_project).id response = self.client.post( self.url, data=data, HTTP_AUTHORIZATION=f"Bearer {self.api_token.token}" @@ -74,6 +77,40 @@ def test_invalid_group_id(self) -> None: assert response.status_code == 404 + def test_rejects_issue_from_inaccessible_project(self) -> None: + self._set_up_sentry_app("Testin", ["event:write"]) + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + user_team = self.create_team(organization=self.org, name="sei-user-team") + other_team = self.create_team(organization=self.org, name="sei-other-team") + self.create_project(organization=self.org, teams=[user_team], name="sei-user-proj") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="sei-other-proj" + ) + other_group = self.create_group(project=other_project) + + limited_user = self.create_user() + self.create_member( + organization=self.org, + user=limited_user, + role="member", + teams=[user_team], + teamRole="admin", + ) + + data = self._post_data() + data["issueId"] = other_group.id + + self.login_as(user=limited_user) + response = self.client.post(self.url, data=data) + + assert response.status_code == 403 + assert response.data["detail"] == ( + "You do not have permission to create an external issue for this issue." + ) + def test_invalid_scopes(self) -> None: self._set_up_sentry_app("Testin", ["project:read"]) data = self._post_data() diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py index 2e469a1b34bcf2..86bce5b8353e8f 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py @@ -4,8 +4,9 @@ from django.utils.http import urlencode from responses.matchers import query_string_matcher +from sentry.models.organization import Organization from sentry.testutils.cases import APITestCase -from sentry.testutils.silo import control_silo_test +from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @control_silo_test @@ -93,3 +94,31 @@ def test_external_request_fails(self) -> None: url = self.url + f"?uri={self.project.id}" response = self.client.get(url, format="json") assert response.status_code == 500 + + def test_rejects_project_id_without_access(self) -> None: + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + user_team = self.create_team(organization=self.org, name="ser-user-team") + other_team = self.create_team(organization=self.org, name="ser-other-team") + self.create_project(organization=self.org, teams=[user_team], name="ser-user-proj") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="ser-other-proj" + ) + + limited_user = self.create_user() + self.create_member( + organization=self.org, + user=limited_user, + role="member", + teams=[user_team], + teamRole="admin", + ) + + self.login_as(user=limited_user) + url = self.url + f"?projectId={other_project.id}&uri=/get-projects&query=proj" + response = self.client.get(url, format="json") + + assert response.status_code == 403 + assert response.data["detail"] == "You do not have permission to access this project." diff --git a/tests/sentry/sentry_apps/services/test_cell_service.py b/tests/sentry/sentry_apps/services/test_cell_service.py index 7f3cc1ff835fea..ee8115ffdd376a 100644 --- a/tests/sentry/sentry_apps/services/test_cell_service.py +++ b/tests/sentry/sentry_apps/services/test_cell_service.py @@ -60,6 +60,7 @@ def test_get_select_options(self) -> None: organization_id=self.org.id, installation=self.rpc_installation, uri="/get-projects", + user=serialize_rpc_user(self.user), project_id=self.project.id, query="proj", dependent_data=dependent_data, @@ -81,6 +82,7 @@ def test_get_select_options_error(self) -> None: organization_id=self.org.id, installation=self.rpc_installation, uri="/get-projects", + user=serialize_rpc_user(self.user), ) assert result.error is not None assert result.error.message.startswith( @@ -245,6 +247,7 @@ def test_create_external_issue(self) -> None: web_url="https://example.com/project/issue-1", project="ProjectName", identifier="issue-1", + user=serialize_rpc_user(self.user), ) with assume_test_silo_mode_of(PlatformExternalIssue): @@ -264,6 +267,7 @@ def test_create_external_issue_group_not_found(self) -> None: web_url="https://example.com/project/issue-1", project="ProjectName", identifier="issue-1", + user=serialize_rpc_user(self.user), ) assert result.error is not None @@ -284,6 +288,7 @@ def test_delete_external_issue(self) -> None: organization_id=self.org.id, installation=self.rpc_installation, external_issue_id=external_issue.id, + user=serialize_rpc_user(self.user), ) assert result.success is True @@ -296,12 +301,139 @@ def test_delete_external_issue_not_found(self) -> None: organization_id=self.org.id, installation=self.rpc_installation, external_issue_id=99999999, + user=serialize_rpc_user(self.user), ) assert result.success is False assert result.error is not None assert result.error.status_code == 404 + def test_create_external_issue_denied_without_project_access(self) -> None: + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + user_team = self.create_team(organization=self.org, name="cei-user-team") + other_team = self.create_team(organization=self.org, name="cei-other-team") + self.create_project(organization=self.org, teams=[user_team], name="cei-user-proj") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="cei-other-proj" + ) + other_group = self.create_group(project=other_project) + + limited_user = self.create_user() + self.create_member( + organization=self.org, + user=limited_user, + role="member", + teams=[user_team], + teamRole="admin", + ) + + result = sentry_app_cell_service.create_external_issue( + organization_id=self.org.id, + installation=self.rpc_installation, + group_id=other_group.id, + web_url="https://example.com/p/i", + project="P", + identifier="1", + user=serialize_rpc_user(limited_user), + ) + + assert result.error is not None + assert result.error.status_code == 403 + assert result.external_issue is None + + def test_get_select_options_denied_without_project_access(self) -> None: + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + user_team = self.create_team(organization=self.org, name="gso-user-team") + other_team = self.create_team(organization=self.org, name="gso-other-team") + self.create_project(organization=self.org, teams=[user_team], name="gso-user-proj") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="gso-other-proj" + ) + + limited_user = self.create_user() + self.create_member( + organization=self.org, + user=limited_user, + role="member", + teams=[user_team], + teamRole="admin", + ) + + result = sentry_app_cell_service.get_select_options( + organization_id=self.org.id, + installation=self.rpc_installation, + uri="/get-projects", + user=serialize_rpc_user(limited_user), + project_id=other_project.id, + query="q", + ) + + assert result.error is not None + assert result.error.status_code == 403 + + def test_get_select_options_unknown_project_id_returns_404(self) -> None: + result = sentry_app_cell_service.get_select_options( + organization_id=self.org.id, + installation=self.rpc_installation, + uri="/get-projects", + user=serialize_rpc_user(self.user), + project_id=999999999, + query="q", + ) + + assert result.error is not None + assert result.error.status_code == 404 + + def test_delete_external_issue_denied_without_project_access(self) -> None: + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + user_team = self.create_team(organization=self.org, name="dei-user-team") + other_team = self.create_team(organization=self.org, name="dei-other-team") + self.create_project(organization=self.org, teams=[user_team], name="dei-user-proj") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="dei-other-proj" + ) + other_group = self.create_group(project=other_project) + + limited_user = self.create_user() + self.create_member( + organization=self.org, + user=limited_user, + role="member", + teams=[user_team], + teamRole="admin", + ) + + with assume_test_silo_mode_of(PlatformExternalIssue): + external_issue = PlatformExternalIssue.objects.create( + group_id=other_group.id, + project_id=other_project.id, + service_type=self.sentry_app.slug, + display_name="X#1", + web_url="https://example.com/issue/1", + ) + + result = sentry_app_cell_service.delete_external_issue( + organization_id=self.org.id, + installation=self.rpc_installation, + external_issue_id=external_issue.id, + user=serialize_rpc_user(limited_user), + ) + + assert result.success is False + assert result.error is not None + assert result.error.status_code == 403 + with assume_test_silo_mode_of(PlatformExternalIssue): + assert PlatformExternalIssue.objects.filter(id=external_issue.id).exists() + def test_get_service_hook_projects(self) -> None: project2 = self.create_project(organization=self.org) From c6bda2f10e962725341111d09e317a6c6d409a26 Mon Sep 17 00:00:00 2001 From: geoffg-sentry <165922362+geoffg-sentry@users.noreply.github.com> Date: Tue, 14 Apr 2026 10:56:22 -0400 Subject: [PATCH 2/5] make User optional so I dont break everything --- .../sentry_apps/api/bases/sentryapps.py | 19 +-- .../installation_external_issue_actions.py | 12 +- .../installation_external_issue_details.py | 10 +- .../endpoints/installation_external_issues.py | 10 +- .../installation_external_requests.py | 12 +- src/sentry/sentry_apps/services/cell/impl.py | 108 +++++++++--------- .../sentry_apps/services/cell/service.py | 6 +- .../sentry_apps/services/test_cell_service.py | 99 ++++++++++++++++ 8 files changed, 182 insertions(+), 94 deletions(-) diff --git a/src/sentry/sentry_apps/api/bases/sentryapps.py b/src/sentry/sentry_apps/api/bases/sentryapps.py index 5565ee9bd09a38..637d0a27d64515 100644 --- a/src/sentry/sentry_apps/api/bases/sentryapps.py +++ b/src/sentry/sentry_apps/api/bases/sentryapps.py @@ -2,7 +2,7 @@ import logging from collections.abc import Sequence -from typing import Any, cast +from typing import Any import sentry_sdk from rest_framework.permissions import BasePermission @@ -30,7 +30,6 @@ ) from sentry.users.models.user import User from sentry.users.services.user import RpcUser -from sentry.users.services.user.serial import serialize_rpc_user from sentry.users.services.user.service import user_service from sentry.utils.strings import to_single_line_str @@ -39,22 +38,6 @@ logger = logging.getLogger(__name__) -def rpc_user_from_request(request: Request) -> RpcUser: - """ - Unwrap LazyObject request.user and serialize Django User to RpcUser for cell RPC calls. - """ - from django.utils.functional import empty - - user: Any = request.user - if hasattr(user, "_wrapped"): - if user._wrapped is empty: - user._setup() - user = user._wrapped - if isinstance(user, User): - return serialize_rpc_user(user) - return cast(RpcUser, user) - - def ensure_scoped_permission(request: Request, allowed_scopes: Sequence[str] | None) -> bool: """ Verifies the User making the request has at least one required scope for diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py index 5dd06e1c76782d..cd708ba483c19e 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py @@ -6,14 +6,12 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint from sentry.api.serializers import serialize -from sentry.sentry_apps.api.bases.sentryapps import ( - SentryAppInstallationBaseEndpoint, - rpc_user_from_request, -) +from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint from sentry.sentry_apps.api.serializers.platform_external_issue import ( PlatformExternalIssueSerializer, ) from sentry.sentry_apps.services.cell import sentry_app_cell_service +from sentry.users.services.user.serial import serialize_generic_user class SentryAppInstallationExternalIssueActionsSerializer(serializers.Serializer): @@ -43,6 +41,10 @@ def post(self, request: Request, installation) -> Response: action = data.pop("action") uri = data.pop("uri") + rpc_user = serialize_generic_user(request.user) + if rpc_user is None: + return Response({"detail": "Authentication credentials were not provided."}, status=401) + result = sentry_app_cell_service.create_issue_link( organization_id=installation.organization_id, installation=installation, @@ -50,7 +52,7 @@ def post(self, request: Request, installation) -> Response: action=action, fields=data, uri=uri, - user=rpc_user_from_request(request), + user=rpc_user, ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py index 3afe2b18011d3f..5c502abf9507e5 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @@ -6,11 +6,9 @@ from sentry.sentry_apps.api.bases.sentryapps import ( SentryAppInstallationExternalIssueBaseEndpoint as ExternalIssueBaseEndpoint, ) -from sentry.sentry_apps.api.bases.sentryapps import ( - rpc_user_from_request, -) from sentry.sentry_apps.services.cell import sentry_app_cell_service from sentry.sentry_apps.utils.errors import SentryAppError +from sentry.users.services.user.serial import serialize_generic_user @control_silo_endpoint @@ -28,11 +26,15 @@ def delete(self, request: Request, installation, external_issue_id) -> Response: status_code=400, ) + rpc_user = serialize_generic_user(request.user) + if rpc_user is None: + return Response({"detail": "Authentication credentials were not provided."}, status=401) + result = sentry_app_cell_service.delete_external_issue( organization_id=installation.organization_id, installation=installation, external_issue_id=external_issue_id, - user=rpc_user_from_request(request), + user=rpc_user, ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py index 27c4bcda005b1b..c2af7fa01e72c8 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @@ -9,14 +9,12 @@ from sentry.sentry_apps.api.bases.sentryapps import ( SentryAppInstallationExternalIssueBaseEndpoint as ExternalIssueBaseEndpoint, ) -from sentry.sentry_apps.api.bases.sentryapps import ( - rpc_user_from_request, -) from sentry.sentry_apps.api.parsers.sentry_app import URLField from sentry.sentry_apps.api.serializers.platform_external_issue import ( PlatformExternalIssueSerializer as ResponsePlatformExternalIssueSerializer, ) from sentry.sentry_apps.services.cell import sentry_app_cell_service +from sentry.users.services.user.serial import serialize_generic_user class PlatformExternalIssueSerializer(serializers.Serializer): @@ -44,6 +42,10 @@ def post(self, request: Request, installation) -> Response: except Exception: return Response({"detail": "issueId is required, and must be an integer"}, status=400) + rpc_user = serialize_generic_user(request.user) + if rpc_user is None: + return Response({"detail": "Authentication credentials were not provided."}, status=401) + result = sentry_app_cell_service.create_external_issue( organization_id=installation.organization_id, installation=installation, @@ -51,7 +53,7 @@ def post(self, request: Request, installation) -> Response: web_url=data["webUrl"], project=data["project"], identifier=data["identifier"], - user=rpc_user_from_request(request), + user=rpc_user, ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index fd1173ccc6684a..371057c33df526 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -7,12 +7,10 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint -from sentry.sentry_apps.api.bases.sentryapps import ( - SentryAppInstallationBaseEndpoint, - rpc_user_from_request, -) +from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation from sentry.sentry_apps.services.cell import sentry_app_cell_service +from sentry.users.services.user.serial import serialize_generic_user logger = logging.getLogger("sentry.sentry-apps") @@ -29,11 +27,15 @@ def get(self, request: Request, installation: RpcSentryAppInstallation) -> Respo if not uri: return Response({"detail": "uri query parameter is required"}, status=400) + rpc_user = serialize_generic_user(request.user) + if rpc_user is None: + return Response({"detail": "Authentication credentials were not provided."}, status=401) + result = sentry_app_cell_service.get_select_options( organization_id=installation.organization_id, installation=installation, uri=request.GET.get("uri"), - user=rpc_user_from_request(request), + user=rpc_user, project_id=int(request.GET["projectId"]) if request.GET.get("projectId") else None, query=request.GET.get("query"), dependent_data=request.GET.get("dependentData"), diff --git a/src/sentry/sentry_apps/services/cell/impl.py b/src/sentry/sentry_apps/services/cell/impl.py index 2bbadf51a1a8b7..adb1038d0bfcad 100644 --- a/src/sentry/sentry_apps/services/cell/impl.py +++ b/src/sentry/sentry_apps/services/cell/impl.py @@ -44,7 +44,7 @@ def get_select_options( organization_id: int, installation: RpcSentryAppInstallation, uri: str, - user: RpcUser, + user: RpcUser | None = None, project_id: int | None = None, query: str | None = None, dependent_data: str | None = None, @@ -55,7 +55,11 @@ def get_select_options( project_slug: str | None = None if project_id is not None: - project = Project.objects.filter(id=project_id, organization_id=organization_id).first() + project = ( + Project.objects.select_related("organization") + .filter(id=project_id, organization_id=organization_id) + .first() + ) if not project: return RpcSelectRequesterResult( error=RpcSentryAppError( @@ -63,27 +67,19 @@ def get_select_options( status_code=404, ) ) - try: - organization = Organization.objects.get(id=organization_id) - except Organization.DoesNotExist: - return RpcSelectRequesterResult( - error=RpcSentryAppError( - message="Could not find the given project for this organization.", - status_code=404, - ) + if user is not None: + access = self._access_for_installation_user( + organization=project.organization, + installation=installation, + user=user, ) - access = self._access_for_installation_user( - organization=organization, - installation=installation, - user=user, - ) - if not access.has_project_access(project): - return RpcSelectRequesterResult( - error=RpcSentryAppError( - message="You do not have permission to access this project.", - status_code=403, + if not access.has_project_access(project): + return RpcSelectRequesterResult( + error=RpcSentryAppError( + message="You do not have permission to access this project.", + status_code=403, + ) ) - ) project_slug = project.slug try: @@ -177,13 +173,13 @@ def create_external_issue( web_url: str, project: str, identifier: str, - user: RpcUser, + user: RpcUser | None = None, ) -> RpcPlatformExternalIssueResult: """ Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @ POST """ try: - group = Group.objects.select_related("project").get( + group = Group.objects.select_related("project", "project__organization").get( id=group_id, project_id__in=Project.objects.filter(organization_id=organization_id), ) @@ -195,9 +191,7 @@ def create_external_issue( ) ) - try: - organization = Organization.objects.get(id=organization_id) - except Organization.DoesNotExist: + if group.project.organization_id != organization_id: return RpcPlatformExternalIssueResult( error=RpcSentryAppError( message="Could not find the corresponding issue for the given issueId", @@ -205,18 +199,19 @@ def create_external_issue( ) ) - access = self._access_for_installation_user( - organization=organization, - installation=installation, - user=user, - ) - if not access.has_project_access(group.project): - return RpcPlatformExternalIssueResult( - error=RpcSentryAppError( - message="You do not have permission to create an external issue for this issue.", - status_code=403, - ) + if user is not None: + access = self._access_for_installation_user( + organization=group.project.organization, + installation=installation, + user=user, ) + if not access.has_project_access(group.project): + return RpcPlatformExternalIssueResult( + error=RpcSentryAppError( + message="You do not have permission to create an external issue for this issue.", + status_code=403, + ) + ) try: external_issue = ExternalIssueCreator( @@ -239,14 +234,18 @@ def delete_external_issue( organization_id: int, installation: RpcSentryAppInstallation, external_issue_id: int, - user: RpcUser, + user: RpcUser | None = None, ) -> RpcEmptyResult: """ Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @ DELETE """ try: platform_external_issue = PlatformExternalIssue.objects.select_related( - "group", "group__project", "project" + "group", + "group__project", + "group__project__organization", + "project", + "project__organization", ).get( id=external_issue_id, group__project__organization_id=organization_id, @@ -262,10 +261,7 @@ def delete_external_issue( ) issue_project = platform_external_issue.project or platform_external_issue.group.project - - try: - organization = Organization.objects.get(id=organization_id) - except Organization.DoesNotExist: + if issue_project.organization_id != organization_id: return RpcEmptyResult( success=False, error=RpcSentryAppError( @@ -273,20 +269,22 @@ def delete_external_issue( status_code=404, ), ) + organization = issue_project.organization - access = self._access_for_installation_user( - organization=organization, - installation=installation, - user=user, - ) - if not access.has_project_access(issue_project): - return RpcEmptyResult( - success=False, - error=RpcSentryAppError( - message="You do not have permission to delete this external issue.", - status_code=403, - ), + if user is not None: + access = self._access_for_installation_user( + organization=organization, + installation=installation, + user=user, ) + if not access.has_project_access(issue_project): + return RpcEmptyResult( + success=False, + error=RpcSentryAppError( + message="You do not have permission to delete this external issue.", + status_code=403, + ), + ) deletions.exec_sync(platform_external_issue) diff --git a/src/sentry/sentry_apps/services/cell/service.py b/src/sentry/sentry_apps/services/cell/service.py index 73cca91e2a1d17..c3775566397f9a 100644 --- a/src/sentry/sentry_apps/services/cell/service.py +++ b/src/sentry/sentry_apps/services/cell/service.py @@ -51,7 +51,7 @@ def get_select_options( organization_id: int, installation: RpcSentryAppInstallation, uri: str, - user: RpcUser, + user: RpcUser | None = None, project_id: int | None = None, query: str | None = None, dependent_data: str | None = None, @@ -86,7 +86,7 @@ def create_external_issue( web_url: str, project: str, identifier: str, - user: RpcUser, + user: RpcUser | None = None, ) -> RpcPlatformExternalIssueResult: """Invokes ExternalIssueCreator to create an external issue.""" pass @@ -99,7 +99,7 @@ def delete_external_issue( organization_id: int, installation: RpcSentryAppInstallation, external_issue_id: int, - user: RpcUser, + user: RpcUser | None = None, ) -> RpcEmptyResult: """Deletes a PlatformExternalIssue.""" pass diff --git a/tests/sentry/sentry_apps/services/test_cell_service.py b/tests/sentry/sentry_apps/services/test_cell_service.py index ee8115ffdd376a..58f574bfa63310 100644 --- a/tests/sentry/sentry_apps/services/test_cell_service.py +++ b/tests/sentry/sentry_apps/services/test_cell_service.py @@ -344,6 +344,31 @@ def test_create_external_issue_denied_without_project_access(self) -> None: assert result.error.status_code == 403 assert result.external_issue is None + def test_create_external_issue_legacy_skips_project_access_without_user(self) -> None: + """RPC user is optional during rollout; None skips has_project_access (temporary).""" + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + other_team = self.create_team(organization=self.org, name="cei-legacy-other-team") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="cei-legacy-other-proj" + ) + other_group = self.create_group(project=other_project) + + result = sentry_app_cell_service.create_external_issue( + organization_id=self.org.id, + installation=self.rpc_installation, + group_id=other_group.id, + web_url="https://example.com/p/i", + project="P", + identifier="1", + user=None, + ) + + assert result.error is None + assert result.external_issue is not None + def test_get_select_options_denied_without_project_access(self) -> None: with assume_test_silo_mode_of(Organization): self.org.flags.allow_joinleave = False @@ -377,6 +402,47 @@ def test_get_select_options_denied_without_project_access(self) -> None: assert result.error is not None assert result.error.status_code == 403 + @responses.activate + def test_get_select_options_legacy_skips_project_access_without_user(self) -> None: + """RPC user is optional during rollout; None skips has_project_access (temporary).""" + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + other_team = self.create_team(organization=self.org, name="gso-legacy-other-team") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="gso-legacy-other-proj" + ) + + options = [{"label": "Other", "value": "99"}] + qs = urlencode( + { + "projectSlug": other_project.slug, + "installationId": self.install.uuid, + "query": "q", + } + ) + responses.add( + method=responses.GET, + url="https://example.com/get-projects", + match=[query_string_matcher(qs)], + json=options, + status=200, + content_type="application/json", + ) + + result = sentry_app_cell_service.get_select_options( + organization_id=self.org.id, + installation=self.rpc_installation, + uri="/get-projects", + user=None, + project_id=other_project.id, + query="q", + ) + + assert result.error is None + assert result.choices == [["99", "Other"]] + def test_get_select_options_unknown_project_id_returns_404(self) -> None: result = sentry_app_cell_service.get_select_options( organization_id=self.org.id, @@ -434,6 +500,39 @@ def test_delete_external_issue_denied_without_project_access(self) -> None: with assume_test_silo_mode_of(PlatformExternalIssue): assert PlatformExternalIssue.objects.filter(id=external_issue.id).exists() + def test_delete_external_issue_legacy_skips_project_access_without_user(self) -> None: + """RPC user is optional during rollout; None skips has_project_access (temporary).""" + with assume_test_silo_mode_of(Organization): + self.org.flags.allow_joinleave = False + self.org.save() + + other_team = self.create_team(organization=self.org, name="dei-legacy-other-team") + other_project = self.create_project( + organization=self.org, teams=[other_team], name="dei-legacy-other-proj" + ) + other_group = self.create_group(project=other_project) + + with assume_test_silo_mode_of(PlatformExternalIssue): + external_issue = PlatformExternalIssue.objects.create( + group_id=other_group.id, + project_id=other_project.id, + service_type=self.sentry_app.slug, + display_name="X#1", + web_url="https://example.com/issue/1", + ) + + result = sentry_app_cell_service.delete_external_issue( + organization_id=self.org.id, + installation=self.rpc_installation, + external_issue_id=external_issue.id, + user=None, + ) + + assert result.success is True + assert result.error is None + with assume_test_silo_mode_of(PlatformExternalIssue): + assert not PlatformExternalIssue.objects.filter(id=external_issue.id).exists() + def test_get_service_hook_projects(self) -> None: project2 = self.create_project(organization=self.org) From cd489826642dc8d853b849be7c394208fd40ff77 Mon Sep 17 00:00:00 2001 From: geoffg-sentry <165922362+geoffg-sentry@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:08:47 -0400 Subject: [PATCH 3/5] serialize_generic_user unwrap LazyObject after setup() --- src/sentry/users/services/user/serial.py | 7 +++++-- tests/sentry/users/services/test_user_serial.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 tests/sentry/users/services/test_user_serial.py diff --git a/src/sentry/users/services/user/serial.py b/src/sentry/users/services/user/serial.py index bedfeef5db89c6..22e6ca24250649 100644 --- a/src/sentry/users/services/user/serial.py +++ b/src/sentry/users/services/user/serial.py @@ -3,7 +3,7 @@ from collections.abc import Iterable from typing import Any -from django.utils.functional import LazyObject +from django.utils.functional import LazyObject, empty from sentry.db.models.manager.base_query_set import BaseQuerySet from sentry.users.models.user import User @@ -23,7 +23,10 @@ def serialize_generic_user(user: Any) -> RpcUser | None: Return None if the user is anonymous (not logged in). """ if isinstance(user, LazyObject): # from auth middleware - user = getattr(user, "_wrapped") + lazy: Any = user + if lazy._wrapped is empty: + lazy._setup() + user = lazy._wrapped if user is None or user.id is None: return None if isinstance(user, RpcUser): diff --git a/tests/sentry/users/services/test_user_serial.py b/tests/sentry/users/services/test_user_serial.py new file mode 100644 index 00000000000000..a14053e221b874 --- /dev/null +++ b/tests/sentry/users/services/test_user_serial.py @@ -0,0 +1,14 @@ +from django.utils.functional import SimpleLazyObject + +from sentry.testutils.cases import TestCase +from sentry.users.services.user.serial import serialize_generic_user + + +class SerializeGenericUserTest(TestCase): + def test_simple_lazy_object_not_yet_evaluated(self) -> None: + """Lazy user from middleware must not raise when _wrapped is still empty.""" + user = self.create_user() + lazy_user = SimpleLazyObject(lambda: user) + rpc = serialize_generic_user(lazy_user) + assert rpc is not None + assert rpc.id == user.id From 1fb026edf2f0a42b60a8d7b97f6b3484a15c6cb1 Mon Sep 17 00:00:00 2001 From: geoffg-sentry <165922362+geoffg-sentry@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:16:54 -0400 Subject: [PATCH 4/5] fix truthy check that led to ValueError --- .../api/endpoints/installation_external_requests.py | 10 +++++++++- .../test_sentry_app_installation_external_requests.py | 7 +++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index 371057c33df526..7c47509e5b4162 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -31,12 +31,20 @@ def get(self, request: Request, installation: RpcSentryAppInstallation) -> Respo if rpc_user is None: return Response({"detail": "Authentication credentials were not provided."}, status=401) + project_id: int | None = None + project_id_raw = request.GET.get("projectId") + if project_id_raw: + try: + project_id = int(project_id_raw) + except (TypeError, ValueError): + return Response({"detail": "projectId must be an integer"}, status=400) + result = sentry_app_cell_service.get_select_options( organization_id=installation.organization_id, installation=installation, uri=request.GET.get("uri"), user=rpc_user, - project_id=int(request.GET["projectId"]) if request.GET.get("projectId") else None, + project_id=project_id, query=request.GET.get("query"), dependent_data=request.GET.get("dependentData"), ) diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py index 86bce5b8353e8f..f001a2df4e949f 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py @@ -122,3 +122,10 @@ def test_rejects_project_id_without_access(self) -> None: assert response.status_code == 403 assert response.data["detail"] == "You do not have permission to access this project." + + def test_invalid_project_id_returns_400(self) -> None: + self.login_as(user=self.user) + url = self.url + "?projectId=not-an-int&uri=/get-projects&query=proj" + response = self.client.get(url, format="json") + assert response.status_code == 400 + assert response.data["detail"] == "projectId must be an integer" From 961dd07818aa1647ebf3b04edd263b54c0025354 Mon Sep 17 00:00:00 2001 From: geoffg-sentry <165922362+geoffg-sentry@users.noreply.github.com> Date: Wed, 15 Apr 2026 12:32:02 -0400 Subject: [PATCH 5/5] postpone rpc_user param until next change --- .../installation_external_issue_details.py | 6 +-- .../endpoints/installation_external_issues.py | 6 +-- .../installation_external_requests.py | 6 +-- ...app_installation_external_issue_details.py | 41 ------------------- ...sentry_app_installation_external_issues.py | 35 ---------------- ...ntry_app_installation_external_requests.py | 31 +------------- 6 files changed, 7 insertions(+), 118 deletions(-) diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py index 5c502abf9507e5..4e6e3d87fa9ac1 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @@ -8,7 +8,6 @@ ) from sentry.sentry_apps.services.cell import sentry_app_cell_service from sentry.sentry_apps.utils.errors import SentryAppError -from sentry.users.services.user.serial import serialize_generic_user @control_silo_endpoint @@ -26,15 +25,14 @@ def delete(self, request: Request, installation, external_issue_id) -> Response: status_code=400, ) - rpc_user = serialize_generic_user(request.user) - if rpc_user is None: + if not request.user.is_authenticated: return Response({"detail": "Authentication credentials were not provided."}, status=401) + # Do not pass `user` until cells accept the new RPC arg everywhere (deploy phase 2). result = sentry_app_cell_service.delete_external_issue( organization_id=installation.organization_id, installation=installation, external_issue_id=external_issue_id, - user=rpc_user, ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py index c2af7fa01e72c8..1ec0981996d370 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @@ -14,7 +14,6 @@ PlatformExternalIssueSerializer as ResponsePlatformExternalIssueSerializer, ) from sentry.sentry_apps.services.cell import sentry_app_cell_service -from sentry.users.services.user.serial import serialize_generic_user class PlatformExternalIssueSerializer(serializers.Serializer): @@ -42,10 +41,10 @@ def post(self, request: Request, installation) -> Response: except Exception: return Response({"detail": "issueId is required, and must be an integer"}, status=400) - rpc_user = serialize_generic_user(request.user) - if rpc_user is None: + if not request.user.is_authenticated: return Response({"detail": "Authentication credentials were not provided."}, status=401) + # Do not pass `user` until cells accept the new RPC arg everywhere (deploy phase 2). result = sentry_app_cell_service.create_external_issue( organization_id=installation.organization_id, installation=installation, @@ -53,7 +52,6 @@ def post(self, request: Request, installation) -> Response: web_url=data["webUrl"], project=data["project"], identifier=data["identifier"], - user=rpc_user, ) if result.error: diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index 7c47509e5b4162..23a94374ae80e7 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -10,7 +10,6 @@ from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation from sentry.sentry_apps.services.cell import sentry_app_cell_service -from sentry.users.services.user.serial import serialize_generic_user logger = logging.getLogger("sentry.sentry-apps") @@ -27,8 +26,7 @@ def get(self, request: Request, installation: RpcSentryAppInstallation) -> Respo if not uri: return Response({"detail": "uri query parameter is required"}, status=400) - rpc_user = serialize_generic_user(request.user) - if rpc_user is None: + if not request.user.is_authenticated: return Response({"detail": "Authentication credentials were not provided."}, status=401) project_id: int | None = None @@ -39,11 +37,11 @@ def get(self, request: Request, installation: RpcSentryAppInstallation) -> Respo except (TypeError, ValueError): return Response({"detail": "projectId must be an integer"}, status=400) + # Do not pass `user` until cells accept the new RPC arg everywhere (deploy phase 2). result = sentry_app_cell_service.get_select_options( organization_id=installation.organization_id, installation=installation, uri=request.GET.get("uri"), - user=rpc_user, project_id=project_id, query=request.GET.get("query"), dependent_data=request.GET.get("dependentData"), diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py index 1966e0d60e173f..2065d5e475c024 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py @@ -1,4 +1,3 @@ -from sentry.models.organization import Organization from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @@ -72,43 +71,3 @@ def test_handles_invalid_external_issue_id_format(self) -> None: # Ensure the external issue still exists after failed attempts with assume_test_silo_mode_of(PlatformExternalIssue): assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() - - def test_rejects_delete_without_project_access(self) -> None: - with assume_test_silo_mode_of(Organization): - self.org.flags.allow_joinleave = False - self.org.save() - - user_team = self.create_team(organization=self.org, name="sed-user-team") - other_team = self.create_team(organization=self.org, name="sed-other-team") - self.create_project(organization=self.org, teams=[user_team], name="sed-user-proj") - other_project = self.create_project( - organization=self.org, teams=[other_team], name="sed-other-proj" - ) - other_group = self.create_group(project=other_project) - - limited_user = self.create_user() - self.create_member( - organization=self.org, - user=limited_user, - role="member", - teams=[user_team], - teamRole="admin", - ) - - with assume_test_silo_mode_of(PlatformExternalIssue): - other_external_issue = PlatformExternalIssue.objects.create( - group_id=other_group.id, - project_id=other_project.id, - service_type=self.sentry_app.slug, - display_name="Other#1", - web_url="https://example.com/o/1", - ) - - self.login_as(user=limited_user) - self.get_error_response( - self.install.uuid, - other_external_issue.id, - status_code=403, - ) - with assume_test_silo_mode_of(PlatformExternalIssue): - assert PlatformExternalIssue.objects.filter(id=other_external_issue.id).exists() diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py index c7f47d5d420d8b..3362dac8bec919 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issues.py @@ -2,7 +2,6 @@ from django.urls import reverse -from sentry.models.organization import Organization from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test @@ -77,40 +76,6 @@ def test_invalid_group_id(self) -> None: assert response.status_code == 404 - def test_rejects_issue_from_inaccessible_project(self) -> None: - self._set_up_sentry_app("Testin", ["event:write"]) - with assume_test_silo_mode_of(Organization): - self.org.flags.allow_joinleave = False - self.org.save() - - user_team = self.create_team(organization=self.org, name="sei-user-team") - other_team = self.create_team(organization=self.org, name="sei-other-team") - self.create_project(organization=self.org, teams=[user_team], name="sei-user-proj") - other_project = self.create_project( - organization=self.org, teams=[other_team], name="sei-other-proj" - ) - other_group = self.create_group(project=other_project) - - limited_user = self.create_user() - self.create_member( - organization=self.org, - user=limited_user, - role="member", - teams=[user_team], - teamRole="admin", - ) - - data = self._post_data() - data["issueId"] = other_group.id - - self.login_as(user=limited_user) - response = self.client.post(self.url, data=data) - - assert response.status_code == 403 - assert response.data["detail"] == ( - "You do not have permission to create an external issue for this issue." - ) - def test_invalid_scopes(self) -> None: self._set_up_sentry_app("Testin", ["project:read"]) data = self._post_data() diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py index f001a2df4e949f..3e7f22bde49250 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py @@ -4,9 +4,8 @@ from django.utils.http import urlencode from responses.matchers import query_string_matcher -from sentry.models.organization import Organization from sentry.testutils.cases import APITestCase -from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test +from sentry.testutils.silo import control_silo_test @control_silo_test @@ -95,34 +94,6 @@ def test_external_request_fails(self) -> None: response = self.client.get(url, format="json") assert response.status_code == 500 - def test_rejects_project_id_without_access(self) -> None: - with assume_test_silo_mode_of(Organization): - self.org.flags.allow_joinleave = False - self.org.save() - - user_team = self.create_team(organization=self.org, name="ser-user-team") - other_team = self.create_team(organization=self.org, name="ser-other-team") - self.create_project(organization=self.org, teams=[user_team], name="ser-user-proj") - other_project = self.create_project( - organization=self.org, teams=[other_team], name="ser-other-proj" - ) - - limited_user = self.create_user() - self.create_member( - organization=self.org, - user=limited_user, - role="member", - teams=[user_team], - teamRole="admin", - ) - - self.login_as(user=limited_user) - url = self.url + f"?projectId={other_project.id}&uri=/get-projects&query=proj" - response = self.client.get(url, format="json") - - assert response.status_code == 403 - assert response.data["detail"] == "You do not have permission to access this project." - def test_invalid_project_id_returns_400(self) -> None: self.login_as(user=self.user) url = self.url + "?projectId=not-an-int&uri=/get-projects&query=proj"