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..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 @@ -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 @@ -12,22 +11,7 @@ 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 +from sentry.users.services.user.serial import serialize_generic_user class SentryAppInstallationExternalIssueActionsSerializer(serializers.Serializer): @@ -57,9 +41,9 @@ 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) + 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, @@ -68,7 +52,7 @@ def post(self, request: Request, installation) -> Response: action=action, fields=data, uri=uri, - user=user, + 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 b00b450121a5d9..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 @@ -25,6 +25,10 @@ def delete(self, request: Request, installation, external_issue_id) -> Response: status_code=400, ) + 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, 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..1ec0981996d370 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @@ -41,6 +41,10 @@ def post(self, request: Request, installation) -> Response: except Exception: return Response({"detail": "issueId is required, and must be an integer"}, status=400) + 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, 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..23a94374ae80e7 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -26,11 +26,23 @@ def get(self, request: Request, installation: RpcSentryAppInstallation) -> Respo if not uri: return Response({"detail": "uri query parameter is required"}, status=400) + if not request.user.is_authenticated: + 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) + + # 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"), - 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/src/sentry/sentry_apps/services/cell/impl.py b/src/sentry/sentry_apps/services/cell/impl.py index 48f42ea67e30c2..adb1038d0bfcad 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 | None = None, project_id: int | None = None, query: str | None = None, dependent_data: str | None = None, @@ -54,9 +55,32 @@ 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 + project = ( + Project.objects.select_related("organization") + .filter(id=project_id, organization_id=organization_id) + .first() + ) + if not project: + 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, + ) + 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 +173,13 @@ def create_external_issue( web_url: str, project: str, identifier: str, + user: RpcUser | None = None, ) -> RpcPlatformExternalIssueResult: """ Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @ POST """ try: - group = Group.objects.get( + group = Group.objects.select_related("project", "project__organization").get( id=group_id, project_id__in=Project.objects.filter(organization_id=organization_id), ) @@ -166,6 +191,28 @@ def create_external_issue( ) ) + if group.project.organization_id != organization_id: + return RpcPlatformExternalIssueResult( + error=RpcSentryAppError( + message="Could not find the corresponding issue for the given issueId", + status_code=404, + ) + ) + + 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( install=installation, @@ -187,14 +234,21 @@ def delete_external_issue( organization_id: int, installation: RpcSentryAppInstallation, external_issue_id: int, + user: RpcUser | None = None, ) -> 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", + "group__project__organization", + "project", + "project__organization", + ).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 +260,32 @@ def delete_external_issue( ), ) + issue_project = platform_external_issue.project or platform_external_issue.group.project + if issue_project.organization_id != organization_id: + return RpcEmptyResult( + success=False, + error=RpcSentryAppError( + message="Could not find the corresponding external issue from given external_issue_id", + status_code=404, + ), + ) + organization = issue_project.organization + + 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) return RpcEmptyResult() diff --git a/src/sentry/sentry_apps/services/cell/service.py b/src/sentry/sentry_apps/services/cell/service.py index 953e8a2c6beee3..c3775566397f9a 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 | None = None, 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 | None = None, ) -> 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 | None = None, ) -> RpcEmptyResult: """Deletes a PlatformExternalIssue.""" pass 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/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..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 @@ -66,7 +66,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}" 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..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 @@ -93,3 +93,10 @@ 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_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" diff --git a/tests/sentry/sentry_apps/services/test_cell_service.py b/tests/sentry/sentry_apps/services/test_cell_service.py index 7f3cc1ff835fea..58f574bfa63310 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,238 @@ 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_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 + 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 + + @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, + 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_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) 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