From 8cea840af93072a2b22a3104755f544e00f7c252 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Sat, 31 Oct 2026 21:23:42 -0700 Subject: [PATCH] ref(api): Narrow request.user in flags, preprod, replays, releases and investigations Flags, preprod, replays, releases, feedback, data export, data secrecy, investigations, workflow engine and superuser-middleware logging read the caller through `sentry.auth.principal`. - A service account can create flag webhook secrets and approve preprod artifacts (recorded with no creator/approver) and gets 404 for data exports it doesn't own; span stars, replay viewed-by and test-fire actions are 403 `user-required`. New tests cover the first three. - Token callers: no longer count as the creator of a flag secret with no creator (updating it now needs `org:write`), no longer treat a null-approver approval as their own, can't read null-user exports, and starring a span returns 401 instead of a 500. - Users and agents see no change. Left: `data_export.py` (`get_or_create` keyed on the user), `notebooks/base.py` (`user_id()` helper used by about 15 endpoints) and `web/client_config.py` (Django views typed as DRF requests). --- .../endpoints/data_export_details.py | 7 ++-- .../data_secrecy/api/waive_data_secrecy.py | 10 ++++-- .../organization_feedback_categories.py | 5 ++- .../organization_feedback_summary.py | 5 ++- src/sentry/flags/endpoints/secrets.py | 5 +-- .../insights/endpoints/starred_segments.py | 5 +-- .../organization_investigation_details.py | 5 +-- src/sentry/middleware/superuser.py | 5 +-- .../api/endpoints/preprod_artifact_approve.py | 5 +-- .../endpoints/organization_release_details.py | 3 +- .../endpoints/project_replay_summary.py | 5 +-- .../endpoints/project_replay_viewed_by.py | 7 ++-- .../organization_service_accounts.py | 3 +- .../organization_test_fire_action.py | 9 +++--- tests/sentry/auth/test_principal.py | 2 +- .../endpoints/test_data_export_details.py | 21 ++++++++++++ tests/sentry/flags/endpoints/test_secrets.py | 29 +++++++++++++++++ tests/sentry/middleware/test_auth.py | 13 ++++---- .../test_preprod_artifact_approve.py | 32 ++++++++++++++++++- tests/sentry/seer/test_agent_token.py | 4 ++- 20 files changed, 140 insertions(+), 40 deletions(-) diff --git a/src/sentry/data_export/endpoints/data_export_details.py b/src/sentry/data_export/endpoints/data_export_details.py index c666ca37464a..4657496baf7b 100644 --- a/src/sentry/data_export/endpoints/data_export_details.py +++ b/src/sentry/data_export/endpoints/data_export_details.py @@ -16,6 +16,7 @@ from sentry.apidocs.constants import RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import owned_by_acting_user from sentry.data_export.models import ExportedData from sentry.models.organization import Organization from sentry.models.project import Project @@ -76,11 +77,7 @@ def get( """ try: data_export = ExportedData.objects.get( - id=data_export_id, - organization=organization, - user_id=request.user.id, - # A caller with no user (id None) would otherwise match exports whose user is gone. - user_id__isnull=False, + owned_by_acting_user(request.user), id=data_export_id, organization=organization ) except ExportedData.DoesNotExist: if ExportedData.objects.filter(id=data_export_id, organization=organization).exists(): diff --git a/src/sentry/data_secrecy/api/waive_data_secrecy.py b/src/sentry/data_secrecy/api/waive_data_secrecy.py index f93fd58331e1..d4a07d179265 100644 --- a/src/sentry/data_secrecy/api/waive_data_secrecy.py +++ b/src/sentry/data_secrecy/api/waive_data_secrecy.py @@ -12,6 +12,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint from sentry.api.bases.organization import ControlSiloOrganizationEndpoint, OrganizationPermission +from sentry.auth.principal import acting_user_id from sentry.data_secrecy.cache import effective_grant_status_cache from sentry.data_secrecy.logic import cache_effective_grant_status, data_access_grant_exists from sentry.data_secrecy.models.data_access_grant import ( @@ -98,7 +99,10 @@ def post( result = validator.validated_data DataAccessGrant.create_data_access_grant( - organization.id, request.user.id, DataAccessGrant.GrantType.MANUAL, result["access_end"] + organization.id, + acting_user_id(request.user), + DataAccessGrant.GrantType.MANUAL, + result["access_end"], ) cache_effective_grant_status(organization_id=organization.id) @@ -123,7 +127,9 @@ def delete( Revoke all active data secrecy waivers for an organization. """ DataAccessGrant.revoke_active_data_access_grants( - organization.id, request.user.id, DataAccessGrant.RevocationReason.MANUAL_REVOCATION + organization.id, + acting_user_id(request.user), + DataAccessGrant.RevocationReason.MANUAL_REVOCATION, ) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/sentry/feedback/endpoints/organization_feedback_categories.py b/src/sentry/feedback/endpoints/organization_feedback_categories.py index 13cab166f826..e3d46b3306df 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_categories.py +++ b/src/sentry/feedback/endpoints/organization_feedback_categories.py @@ -12,6 +12,7 @@ from sentry.api.base import cell_silo_endpoint from sentry.api.bases.organization import OrganizationEndpoint, OrganizationUserReportsPermission from sentry.api.utils import get_date_range_from_stats_period +from sentry.auth.principal import acting_user_id from sentry.exceptions import InvalidParams from sentry.feedback.lib.label_query import ( query_label_group_counts, @@ -108,7 +109,9 @@ def get(self, request: Request, organization: Organization) -> Response: {"detail": "AI categorization is not available for this organization."}, status=403 ) - viewer_context = SeerViewerContext(organization_id=organization.id, user_id=request.user.id) + viewer_context = SeerViewerContext( + organization_id=organization.id, user_id=acting_user_id(request.user) + ) try: start, end = get_date_range_from_stats_period( diff --git a/src/sentry/feedback/endpoints/organization_feedback_summary.py b/src/sentry/feedback/endpoints/organization_feedback_summary.py index ad5c8096ee69..f9dcff7c2713 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_summary.py +++ b/src/sentry/feedback/endpoints/organization_feedback_summary.py @@ -11,6 +11,7 @@ from sentry.api.base import cell_silo_endpoint from sentry.api.bases.organization import OrganizationEndpoint, OrganizationUserReportsPermission from sentry.api.utils import get_date_range_from_stats_period +from sentry.auth.principal import acting_user_id from sentry.exceptions import InvalidParams from sentry.feedback.lib.seer_api import SummarizeFeedbacksRequest, make_summarize_feedbacks_request from sentry.grouping.utils import hash_from_values @@ -95,7 +96,9 @@ def get(self, request: Request, organization: Organization) -> Response: {"detail": "AI summaries are not available for this organization."}, status=403 ) - viewer_context = SeerViewerContext(organization_id=organization.id, user_id=request.user.id) + viewer_context = SeerViewerContext( + organization_id=organization.id, user_id=acting_user_id(request.user) + ) try: start, end = get_date_range_from_stats_period( diff --git a/src/sentry/flags/endpoints/secrets.py b/src/sentry/flags/endpoints/secrets.py index 194f81a5d100..1600419e83f6 100644 --- a/src/sentry/flags/endpoints/secrets.py +++ b/src/sentry/flags/endpoints/secrets.py @@ -16,6 +16,7 @@ ) from sentry.api.paginator import OffsetPaginator from sentry.api.serializers import Serializer, register, serialize +from sentry.auth.principal import acting_user_id, is_acting_user from sentry.flags.models import FlagWebHookSigningSecretModel from sentry.models.organization import Organization @@ -95,7 +96,7 @@ def post(self, request: Request, organization: Organization) -> Response: organization_id=organization.id ).get(provider=validator.validated_data["provider"]) # allow update access if the user created the secret - is_creator = request.user.id == secret.created_by + is_creator = is_acting_user(request.user, secret.created_by) except FlagWebHookSigningSecretModel.DoesNotExist: # anyone can post a new secret is_creator = True @@ -105,7 +106,7 @@ def post(self, request: Request, organization: Organization) -> Response: organization=organization, provider=validator.validated_data["provider"], defaults={ - "created_by": request.user.id, + "created_by": acting_user_id(request.user), "date_added": datetime.now(tz=timezone.utc), "provider": validator.validated_data["provider"], "secret": validator.validated_data["secret"], diff --git a/src/sentry/insights/endpoints/starred_segments.py b/src/sentry/insights/endpoints/starred_segments.py index 3325c7b49a56..e2f5f60689a8 100644 --- a/src/sentry/insights/endpoints/starred_segments.py +++ b/src/sentry/insights/endpoints/starred_segments.py @@ -18,6 +18,7 @@ ) from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.response_types import ValidationErrorResponse, as_validation_errors +from sentry.auth.principal import require_user from sentry.insights.models import InsightsStarredSegment from sentry.models.organization import Organization from sentry.utils.db import atomic_transaction @@ -98,7 +99,7 @@ def post( _, created = InsightsStarredSegment.objects.get_or_create( organization=organization, project_id=project.id, - user_id=request.user.id, + user_id=require_user(request).id, segment_name=service_span_name, ) @@ -163,7 +164,7 @@ def delete( InsightsStarredSegment.objects.filter( organization=organization, - user_id=request.user.id, + user_id=require_user(request).id, project_id=project.id, segment_name=service_span_name, ).delete() diff --git a/src/sentry/investigations/endpoints/organization_investigation_details.py b/src/sentry/investigations/endpoints/organization_investigation_details.py index 050299541a8a..c0ea98293bbf 100644 --- a/src/sentry/investigations/endpoints/organization_investigation_details.py +++ b/src/sentry/investigations/endpoints/organization_investigation_details.py @@ -12,6 +12,7 @@ from sentry.apidocs import constants from sentry.apidocs.parameters import GlobalParams, InvestigationParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import acting_user_id from sentry.investigations.endpoints.serializers import ( InvestigationDetailsSerializer, InvestigationDetailsSerializerResponse, @@ -138,7 +139,7 @@ def put( archived = archive_investigation_with_orchestration( investigation=investigation, expected_version=expected_version, - actor_id=request.user.id, + actor_id=acting_user_id(request.user), ) except Exception as error: response = service_error(error) @@ -204,7 +205,7 @@ def delete( archive_investigation_with_orchestration( investigation=investigation, expected_version=validator.validated_data["investigation_version"], - actor_id=request.user.id, + actor_id=acting_user_id(request.user), ) except Exception as error: response = service_error(error) diff --git a/src/sentry/middleware/superuser.py b/src/sentry/middleware/superuser.py index db794056ca8f..1783cdd608ff 100644 --- a/src/sentry/middleware/superuser.py +++ b/src/sentry/middleware/superuser.py @@ -3,6 +3,7 @@ from rest_framework.request import Request from rest_framework.response import Response +from sentry.auth.principal import acting_user_id from sentry.auth.superuser import Superuser, logger @@ -27,7 +28,7 @@ def process_request(self, request: Request) -> None: "url": request.build_absolute_uri(), "method": request.method, "ip_address": request.META["REMOTE_ADDR"], - "user_id": request.user.id, + "user_id": acting_user_id(request.user), "user_email": self._extract_email(request), }, ) @@ -47,7 +48,7 @@ def process_response(self, request: Request, response: Response) -> Response: "superuser.superuser_access", extra={ "superuser_token_id": su.token, - "user_id": request.user.id, + "user_id": acting_user_id(request.user), "user_email": self._extract_email(request), "su_org_accessed": org_slug, }, diff --git a/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py b/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py index d2ecfbc6d6a9..f12c6aae1131 100644 --- a/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py +++ b/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py @@ -15,6 +15,7 @@ OrganizationEndpoint, OrganizationReleasePermission, ) +from sentry.auth.principal import acting_user_id, owned_by_acting_user from sentry.auth.staff import is_active_staff from sentry.models.organization import Organization from sentry.preprod.analytics import PreprodStatusCheckApprovalCreatedEvent @@ -75,9 +76,9 @@ def post(self, request: Request, organization: Organization, artifact_id: str) - # exists()+create() instead of get_or_create — no unique constraint on this model # (see snapshots/tasks.py for rationale) already_approved = PreprodComparisonApproval.objects.filter( + owned_by_acting_user(request.user, "approved_by_id"), preprod_artifact=artifact, preprod_feature_type=feature_type, - approved_by_id=request.user.id, approval_status=PreprodComparisonApproval.ApprovalStatus.APPROVED, ).exists() @@ -88,7 +89,7 @@ def post(self, request: Request, organization: Organization, artifact_id: str) - PreprodComparisonApproval.objects.create( preprod_artifact=artifact, preprod_feature_type=feature_type, - approved_by_id=request.user.id, + approved_by_id=acting_user_id(request.user), approval_status=PreprodComparisonApproval.ApprovalStatus.APPROVED, approved_at=datetime.now(timezone.utc), ) diff --git a/src/sentry/releases/endpoints/organization_release_details.py b/src/sentry/releases/endpoints/organization_release_details.py index 343216c0762c..58643a4f6a2c 100644 --- a/src/sentry/releases/endpoints/organization_release_details.py +++ b/src/sentry/releases/endpoints/organization_release_details.py @@ -42,6 +42,7 @@ as_validation_errors, ) from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import acting_user_id from sentry.constants import ALL_ACCESS_PROJECT_ID, ALL_ACCESS_PROJECTS_SLUG from sentry.issues.action_log import ( action_context_scope, @@ -587,7 +588,7 @@ def put( ) fetch_commits = not commit_list try: - release.set_refs(refs, request.user.id, fetch=fetch_commits) + release.set_refs(refs, acting_user_id(request.user), fetch=fetch_commits) except InvalidRepository as e: scope.set_tag("failure_reason", "InvalidRepository") scope.set_attribute("failure_reason", "InvalidRepository") diff --git a/src/sentry/replays/endpoints/project_replay_summary.py b/src/sentry/replays/endpoints/project_replay_summary.py index d8f29a429a51..96e38eea6d92 100644 --- a/src/sentry/replays/endpoints/project_replay_summary.py +++ b/src/sentry/replays/endpoints/project_replay_summary.py @@ -14,6 +14,7 @@ from sentry.api.base import cell_silo_endpoint from sentry.api.bases.project import ProjectPermission from sentry.api.utils import default_start_end_dates, handle_query_errors +from sentry.auth.principal import acting_user_id from sentry.models.project import Project from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.seer_api import ( @@ -197,7 +198,7 @@ def get(self, request: Request, project: Project, replay_id: str) -> Response: # Both the frontend and summary generation are gated by the same permissions. viewer_context = SeerViewerContext( - organization_id=project.organization_id, user_id=request.user.id + organization_id=project.organization_id, user_id=acting_user_id(request.user) ) # Request Seer for the state of the summary task. @@ -321,7 +322,7 @@ def validate_iso_timestamp(timestamp: str | None) -> str | None: start_request["temperature"] = temperature viewer_context = SeerViewerContext( - organization_id=project.organization_id, user_id=request.user.id + organization_id=project.organization_id, user_id=acting_user_id(request.user) ) return self._make_seer_start_request(start_request, viewer_context=viewer_context) diff --git a/src/sentry/replays/endpoints/project_replay_viewed_by.py b/src/sentry/replays/endpoints/project_replay_viewed_by.py index 91d44711eb24..8bf4a7ac25ea 100644 --- a/src/sentry/replays/endpoints/project_replay_viewed_by.py +++ b/src/sentry/replays/endpoints/project_replay_viewed_by.py @@ -19,7 +19,7 @@ from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import GlobalParams, ReplayParams from sentry.apidocs.utils import inline_sentry_response_serializer -from sentry.auth.principal import acting_user_id +from sentry.auth.principal import acting_user_id, require_user from sentry.models.project import Project from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.lib.kafka import publish_replay_event @@ -127,7 +127,8 @@ def post(self, request: Request, project: Project, replay_id: str) -> Response: replay_id = str(uuid.UUID(replay_id)) - user_orgs = user_service.get_organizations(user_id=request.user.id) + user = require_user(request) + user_orgs = user_service.get_organizations(user_id=user.id) if project.organization.id not in [org.id for org in user_orgs]: # If the user is not in the same organization as the replay, we don't need to do anything. return Response(status=204) @@ -158,7 +159,7 @@ def post(self, request: Request, project: Project, replay_id: str) -> Response: message = viewed_event( project.id, replay_id, - request.user.id, + user.id, finished_at_ts, ) publish_replay_event(message) diff --git a/src/sentry/service_accounts/endpoints/organization_service_accounts.py b/src/sentry/service_accounts/endpoints/organization_service_accounts.py index 1906c54323a2..3f9a4593ce05 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_accounts.py +++ b/src/sentry/service_accounts/endpoints/organization_service_accounts.py @@ -38,6 +38,7 @@ ) from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import acting_user_id from sentry.hybridcloud.models.outbox import outbox_context from sentry.organizations.services.organization import ( RpcOrganization, @@ -324,7 +325,7 @@ def post( scope_list=data["scopes"], is_active=data.get("isActive", True), all_projects=all_projects, - created_by_id=request.user.id, + created_by_id=acting_user_id(request.user), ) ServiceAccountProject.objects.bulk_create( ServiceAccountProject(service_account=service_account, project_id=project_id) diff --git a/src/sentry/workflow_engine/endpoints/organization_test_fire_action.py b/src/sentry/workflow_engine/endpoints/organization_test_fire_action.py index e7fde41aae08..e696ea92b2e6 100644 --- a/src/sentry/workflow_engine/endpoints/organization_test_fire_action.py +++ b/src/sentry/workflow_engine/endpoints/organization_test_fire_action.py @@ -5,7 +5,7 @@ from rest_framework import serializers from rest_framework.request import Request from rest_framework.response import Response -from rest_framework.status import HTTP_400_BAD_REQUEST, HTTP_401_UNAUTHORIZED +from rest_framework.status import HTTP_400_BAD_REQUEST from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus @@ -14,6 +14,7 @@ from sentry.api.serializers.rest_framework import CamelSnakeSerializer from sentry.apidocs.constants import RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED from sentry.apidocs.parameters import GlobalParams +from sentry.auth.principal import require_user from sentry.constants import ObjectStatus from sentry.issues.action_log import ( action_context_scope, @@ -99,12 +100,10 @@ def post(self, request: Request, organization: Organization) -> Response: data = serializer.validated_data - if not request.user.is_authenticated: - return Response(status=HTTP_401_UNAUTHORIZED) - + user_id = require_user(request).id qs = Project.objects.filter( organization=organization, - teams__organizationmember__user_id=request.user.id, + teams__organizationmember__user_id=user_id, status=ObjectStatus.ACTIVE, ).order_by("slug") diff --git a/tests/sentry/auth/test_principal.py b/tests/sentry/auth/test_principal.py index 5e896c4cbd1d..cbb9ccf9df5f 100644 --- a/tests/sentry/auth/test_principal.py +++ b/tests/sentry/auth/test_principal.py @@ -274,7 +274,7 @@ def _is_request_user_type_check(node: ast.AST) -> bool: # Lower these as callers move off. Exact, so the baseline can't drift back up. -REQUEST_USER_ID_BASELINE = 198 +REQUEST_USER_ID_BASELINE = 176 REQUEST_USER_TYPE_CHECK_BASELINE = 31 diff --git a/tests/sentry/data_export/endpoints/test_data_export_details.py b/tests/sentry/data_export/endpoints/test_data_export_details.py index a25f019e048e..f75359f339ec 100644 --- a/tests/sentry/data_export/endpoints/test_data_export_details.py +++ b/tests/sentry/data_export/endpoints/test_data_export_details.py @@ -3,6 +3,7 @@ from io import BytesIO from unittest.mock import patch +from django.test import override_settings from django.urls import reverse from django.utils import timezone @@ -10,6 +11,8 @@ from sentry.data_export.models import ExportedData from sentry.models.files.file import File from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers.features import with_feature +from sentry.viewer_context import Actor, ActorType, ViewerContext, encode_viewer_context class DataExportDetailsTest(APITestCase): @@ -155,6 +158,24 @@ def test_cannot_download_another_members_export(self) -> None: response = self.client.get(f"{url}?download") assert response.status_code == 404 + @override_settings(SEER_API_SHARED_SECRET="service-account-secret") + @with_feature("organizations:service-accounts") + def test_service_account_cannot_access_members_export(self) -> None: + service_account = self.create_service_account( + organization=self.organization, scope_list=["event:read"] + ) + viewer_context = encode_viewer_context( + ViewerContext( + actor=Actor(type=ActorType.SERVICE_ACCOUNT, actor_id=service_account.id), + organization_id=self.organization.id, + ), + key="service-account-secret", + ) + + url = reverse(self.endpoint, args=[self.organization.slug, self.data_export.id]) + response = self.client.get(url, HTTP_X_VIEWER_CONTEXT=viewer_context) + assert response.status_code == 404, response.content + def test_content_errors(self) -> None: self.data_export = ExportedData.objects.create( user_id=self.user.id, diff --git a/tests/sentry/flags/endpoints/test_secrets.py b/tests/sentry/flags/endpoints/test_secrets.py index 15d76b61f4f2..defc99fecd25 100644 --- a/tests/sentry/flags/endpoints/test_secrets.py +++ b/tests/sentry/flags/endpoints/test_secrets.py @@ -1,7 +1,10 @@ +from django.test import override_settings from django.urls import reverse from sentry.flags.models import FlagWebHookSigningSecretModel from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers.features import with_feature +from sentry.viewer_context import Actor, ActorType, ViewerContext, encode_viewer_context class OrganizationFlagsWebHookSigningSecretsEndpointTestCase(APITestCase): @@ -270,6 +273,32 @@ def test_update_has_scope(self) -> None: assert len(models) == 1 assert models[0].secret == "31271af8b9804cd99a4c787a28274993" + @override_settings(SEER_API_SHARED_SECRET="service-account-secret") + @with_feature("organizations:service-accounts") + def test_service_account_updates_secret_with_no_creator(self) -> None: + service_account = self.create_service_account( + organization=self.organization, scope_list=["org:write"] + ) + viewer_context = encode_viewer_context( + ViewerContext( + actor=Actor(type=ActorType.SERVICE_ACCOUNT, actor_id=service_account.id), + organization_id=self.organization.id, + ), + key="service-account-secret", + ) + + with self.feature(self.features): + response = self.client.post( + self.url, + data={"secret": "31271af8b9804cd99a4c787a28274993", "provider": "launchdarkly"}, + HTTP_X_VIEWER_CONTEXT=viewer_context, + ) + assert response.status_code == 201, response.content + + self.obj.refresh_from_db() + assert self.obj.secret == "31271af8b9804cd99a4c787a28274993" + assert self.obj.created_by is None + class OrganizationFlagsWebHookSigningSecretEndpointTestCase(APITestCase): endpoint = "sentry-api-0-organization-flag-hooks-signing-secret" diff --git a/tests/sentry/middleware/test_auth.py b/tests/sentry/middleware/test_auth.py index b6662e5e3150..c553bfafc1dd 100644 --- a/tests/sentry/middleware/test_auth.py +++ b/tests/sentry/middleware/test_auth.py @@ -5,6 +5,7 @@ from django.test import RequestFactory, override_settings from rest_framework.request import Request +from sentry.auth.principal import acting_user_id from sentry.auth.services.auth import AuthenticatedToken from sentry.middleware.auth import AuthenticationMiddleware from sentry.models.apikey import ApiKey @@ -49,7 +50,7 @@ def test_process_request_user(self) -> None: with outbox_runner(): self.middleware.process_request(request) # Force the user object to materialize - request.user.id + acting_user_id(request.user) with assume_test_silo_mode(SiloMode.CONTROL): self.user.refresh_from_db() @@ -131,7 +132,7 @@ def test_process_request_valid_agent_token(self) -> None: request.META["HTTP_AUTHORIZATION"] = f"Bearer {self._agent_token(self.user)}" self.middleware.process_request(request) assert request.user.is_authenticated - assert request.user.id == self.user.id + assert acting_user_id(request.user) == self.user.id assert request.auth is not None assert request.auth.kind == agent_token.AGENT_TOKEN_KIND assert request.auth.user_id == self.user.id @@ -163,7 +164,7 @@ def test_process_request_agent_token_supplies_compatibility_user_on_non_api_path request.META["HTTP_AUTHORIZATION"] = f"Bearer {self._agent_token(self.user)}" self.middleware.process_request(request) assert request.user.is_authenticated - assert request.user.id == self.user.id + assert acting_user_id(request.user) == self.user.id assert request.auth is not None assert request.auth.user_id == self.user.id @@ -179,8 +180,8 @@ def test_process_request_agent_token_wins_over_session(self) -> None: request.META["HTTP_AUTHORIZATION"] = f"Bearer {self._agent_token(self.user)}" self.middleware.process_request(request) assert request.user.is_authenticated - assert request.user.id == self.user.id - assert request.user.id != session_user.id + assert acting_user_id(request.user) == self.user.id + assert acting_user_id(request.user) != session_user.id assert request.auth is not None assert request.auth.user_id == self.user.id @@ -234,7 +235,7 @@ def test_process_request_log_userip(self, mock_geo_by_addr: MagicMock) -> None: with outbox_runner(): self.middleware.process_request(request) # Should be logged in and have logged a UserIp record. - assert request.user.id == self.user.id + assert acting_user_id(request.user) == self.user.id assert mock_geo_by_addr.call_count == 1 with assume_test_silo_mode(SiloMode.CONTROL): diff --git a/tests/sentry/preprod/api/endpoints/test_preprod_artifact_approve.py b/tests/sentry/preprod/api/endpoints/test_preprod_artifact_approve.py index 459a9f71a794..dcd2702430d5 100644 --- a/tests/sentry/preprod/api/endpoints/test_preprod_artifact_approve.py +++ b/tests/sentry/preprod/api/endpoints/test_preprod_artifact_approve.py @@ -1,5 +1,6 @@ -from unittest.mock import patch +from unittest.mock import MagicMock, patch +from django.test import override_settings from django.urls import reverse from sentry.preprod.analytics import PreprodStatusCheckApprovalCreatedEvent @@ -9,6 +10,8 @@ assert_any_analytics_event, assert_not_analytics_event, ) +from sentry.testutils.helpers.features import with_feature +from sentry.viewer_context import Actor, ActorType, ViewerContext, encode_viewer_context class OrganizationPreprodArtifactApproveTest(APITestCase): @@ -72,6 +75,33 @@ def test_approve_size_records_analytics(self, mock_analytics, mock_task) -> None ), ) + @patch("sentry.preprod.api.endpoints.preprod_artifact_approve.create_preprod_status_check_task") + @override_settings(SEER_API_SHARED_SECRET="service-account-secret") + @with_feature("organizations:service-accounts") + def test_service_account_approves_with_no_approver(self, mock_task: MagicMock) -> None: + service_account = self.create_service_account( + organization=self.org, scope_list=["project:releases"], project_ids=[self.project_b.id] + ) + viewer_context = encode_viewer_context( + ViewerContext( + actor=Actor(type=ActorType.SERVICE_ACCOUNT, actor_id=service_account.id), + organization_id=self.org.id, + ), + key="service-account-secret", + ) + + response = self.client.post( + self._approve_url(self.artifact.id), + data={"feature_type": "size"}, + format="json", + HTTP_X_VIEWER_CONTEXT=viewer_context, + ) + + assert response.status_code == 201, response.content + approval = PreprodComparisonApproval.objects.get(preprod_artifact=self.artifact) + assert approval.approval_status == PreprodComparisonApproval.ApprovalStatus.APPROVED + assert approval.approved_by_id is None + @patch("sentry.preprod.api.endpoints.preprod_artifact_approve.update_preprod_snapshot_vcs") @patch("sentry.analytics.record") def test_approve_snapshots_records_analytics(self, mock_analytics, mock_task) -> None: diff --git a/tests/sentry/seer/test_agent_token.py b/tests/sentry/seer/test_agent_token.py index aa651b2e06e3..6872c9dd9900 100644 --- a/tests/sentry/seer/test_agent_token.py +++ b/tests/sentry/seer/test_agent_token.py @@ -18,6 +18,7 @@ from sentry.api.authentication import AgentTokenAuthentication, UserAuthTokenAuthentication from sentry.api.bases.organization import OrganizationPermission, OrganizationReleasesBaseEndpoint from sentry.auth.access import DEFAULT, Access, from_auth, from_request, from_request_org_and_scopes +from sentry.auth.principal import acting_user_id, has_user from sentry.auth.services.auth import AuthenticatedToken from sentry.models.organizationmember import OrganizationMember from sentry.organizations.services.organization import organization_service @@ -132,6 +133,7 @@ def test_principal_subject_rejects_invalid_or_unsupported_types(self) -> None: def test_valid_token_supplies_compatibility_user_and_preserves_agent_auth(self) -> None: request = self._agent_request(self.owner, ["org:read"], method="GET") assert request.user.is_authenticated + assert has_user(request.user) assert request.user.id == self.owner.id assert request.user.email == self.owner.email assert request.auth is not None @@ -145,7 +147,7 @@ def test_compatibility_user_strips_platform_elevation(self) -> None: request = self._agent_request(elevated_user, ["org:read"], method="GET") - assert request.user.id == elevated_user.id + assert acting_user_id(request.user) == elevated_user.id assert not request.user.is_staff assert not request.user.is_superuser assert getattr(request.user, "permissions", None) == frozenset()