From ed5c642cb451e3ca654e679c376709e62d6ab88d Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:13:36 -0700 Subject: [PATCH 01/25] test(api): Require notes for readonly mutation scopes Add a guardrail for published mutation endpoints that still accept readonly scopes. Previously, write methods could keep readonly scopes in scope_map without any explicit marker in code, which made the policy debt hard to audit and easy to expand accidentally. Require those endpoints to carry a readonly_mutation_scope_exceptions note, and fail the invariant test when a published mutation endpoint accepts readonly scopes without that note. Co-Authored-By: OpenAI Codex --- .../api/bases/organization_request_change.py | 3 + .../repository_token_regenerate.py | 3 + .../endpoints/sync_repos/sync_repos.py | 3 + .../endpoints/organization_conduit_demo.py | 3 + .../organization_member_invite/index.py | 3 + .../organization_member_invite/utils.py | 3 + ...ganization_member_requests_invite_index.py | 3 + .../organization_member_team_details.py | 5 ++ .../endpoints/organization_member_utils.py | 3 + .../organization_dashboard_generate.py | 3 + .../endpoints/notification_actions_index.py | 5 ++ .../api/bases/preprod_artifact_endpoint.py | 4 + .../endpoints/issue_view_title_generate.py | 3 + .../organization_seer_explorer_chat.py | 3 + .../organization_seer_explorer_update.py | 3 + .../seer/endpoints/organization_seer_rpc.py | 3 + .../endpoints/organization_trace_summary.py | 3 + .../seer/endpoints/trace_explorer_ai_setup.py | 3 + .../sentry_apps/api/bases/sentryapps.py | 3 + tests/sentry/api/test_permissions.py | 89 +++++++++++++++++++ 20 files changed, 151 insertions(+) diff --git a/src/sentry/api/bases/organization_request_change.py b/src/sentry/api/bases/organization_request_change.py index 905325f6ec16..d52ed444ce69 100644 --- a/src/sentry/api/bases/organization_request_change.py +++ b/src/sentry/api/bases/organization_request_change.py @@ -7,6 +7,9 @@ class OrganizationRequestChangeEndpointPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "This endpoint only files a request for an organization change; members use it without organization write access.", + } # just requesting so read permission is enough scope_map = { "POST": ["org:read"], diff --git a/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py b/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py index fa1c10855f45..070044325d81 100644 --- a/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py +++ b/src/sentry/codecov/endpoints/repository_token_regenerate/repository_token_regenerate.py @@ -18,6 +18,9 @@ class RepositoryTokenRegeneratePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Codecov token regeneration preserves read-only token access for now.", + } scope_map = { "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], diff --git a/src/sentry/codecov/endpoints/sync_repos/sync_repos.py b/src/sentry/codecov/endpoints/sync_repos/sync_repos.py index a00af3014add..1ef5e9cfa93c 100644 --- a/src/sentry/codecov/endpoints/sync_repos/sync_repos.py +++ b/src/sentry/codecov/endpoints/sync_repos/sync_repos.py @@ -18,6 +18,9 @@ class SyncReposPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Codecov sync preserves read-only token access pending integration-scope cleanup.", + } scope_map = { "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], diff --git a/src/sentry/conduit/endpoints/organization_conduit_demo.py b/src/sentry/conduit/endpoints/organization_conduit_demo.py index 60ad92cb22c8..7ad015ff5223 100644 --- a/src/sentry/conduit/endpoints/organization_conduit_demo.py +++ b/src/sentry/conduit/endpoints/organization_conduit_demo.py @@ -31,6 +31,9 @@ class OrganizationConduitDemoPermission(OrganizationPermission): This is a demo-only feature and doesn't modify organization state. """ + readonly_mutation_scope_exceptions = { + "POST": "Demo credential generation preserves read-only token access for now.", + } scope_map = { "POST": ["org:read", "org:write", "org:admin"], } diff --git a/src/sentry/core/endpoints/organization_member_invite/index.py b/src/sentry/core/endpoints/organization_member_invite/index.py index 999c77af08ec..ff40ad0542f9 100644 --- a/src/sentry/core/endpoints/organization_member_invite/index.py +++ b/src/sentry/core/endpoints/organization_member_invite/index.py @@ -38,6 +38,9 @@ class MemberInvitePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Members can create invite requests on this endpoint even when they cannot send invites directly.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], # We will do an additional check to see if a user can invite members. If diff --git a/src/sentry/core/endpoints/organization_member_invite/utils.py b/src/sentry/core/endpoints/organization_member_invite/utils.py index 6be047608e37..67f6b36d2457 100644 --- a/src/sentry/core/endpoints/organization_member_invite/utils.py +++ b/src/sentry/core/endpoints/organization_member_invite/utils.py @@ -6,6 +6,9 @@ class MemberInviteDetailsPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "DELETE": "Invite deletion keeps current member-read token semantics for now.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], "PUT": ["member:write", "member:admin"], diff --git a/src/sentry/core/endpoints/organization_member_requests_invite_index.py b/src/sentry/core/endpoints/organization_member_requests_invite_index.py index 4fd8a274c763..2a0113bd22b1 100644 --- a/src/sentry/core/endpoints/organization_member_requests_invite_index.py +++ b/src/sentry/core/endpoints/organization_member_requests_invite_index.py @@ -20,6 +20,9 @@ class InviteRequestPermissions(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Invite request creation keeps member-read token access for now.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], "POST": ["member:read", "member:write", "member:admin"], diff --git a/src/sentry/core/endpoints/organization_member_team_details.py b/src/sentry/core/endpoints/organization_member_team_details.py index a569d7f7cc57..bbb1e9e61293 100644 --- a/src/sentry/core/endpoints/organization_member_team_details.py +++ b/src/sentry/core/endpoints/organization_member_team_details.py @@ -72,6 +72,11 @@ def serialize( class OrganizationTeamMemberPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Team membership writes keep current mixed org/member/team token semantics for now.", + "PUT": "Team membership writes keep current mixed org/member/team token semantics for now.", + "DELETE": "Team membership writes keep current mixed org/member/team token semantics for now.", + } scope_map = { "GET": [ "org:read", diff --git a/src/sentry/core/endpoints/organization_member_utils.py b/src/sentry/core/endpoints/organization_member_utils.py index 9993a68d563c..346f5d2e7097 100644 --- a/src/sentry/core/endpoints/organization_member_utils.py +++ b/src/sentry/core/endpoints/organization_member_utils.py @@ -61,6 +61,9 @@ class MemberConflictValidationError(serializers.ValidationError): class RelaxedMemberPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "DELETE": "Member deletion keeps self-service and role-comparison semantics for now.", + } scope_map = { "GET": ["member:read", "member:write", "member:admin"], "POST": ["member:write", "member:admin"], diff --git a/src/sentry/dashboards/endpoints/organization_dashboard_generate.py b/src/sentry/dashboards/endpoints/organization_dashboard_generate.py index 84b6d402805e..1dcd8c35bb51 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboard_generate.py +++ b/src/sentry/dashboards/endpoints/organization_dashboard_generate.py @@ -49,6 +49,9 @@ class DashboardGenerateSerializer(serializers.Serializer[dict[str, Any]]): class OrganizationDashboardGeneratePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Dashboard generation is a POST helper/action and needs separate contract cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/notifications/api/endpoints/notification_actions_index.py b/src/sentry/notifications/api/endpoints/notification_actions_index.py index f1bcfee65906..1e1a27feedad 100644 --- a/src/sentry/notifications/api/endpoints/notification_actions_index.py +++ b/src/sentry/notifications/api/endpoints/notification_actions_index.py @@ -30,6 +30,11 @@ class NotificationActionsPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Notification action writes also rely on project-scoped checks; cleanup is deferred.", + "PUT": "Notification action writes also rely on project-scoped checks; cleanup is deferred.", + "DELETE": "Notification action writes also rely on project-scoped checks; cleanup is deferred.", + } scope_map = { "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], diff --git a/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py b/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py index 2e328ce4818d..bc60e21c71c1 100644 --- a/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py +++ b/src/sentry/preprod/api/bases/preprod_artifact_endpoint.py @@ -33,6 +33,10 @@ class BasePreprodArtifactResourceDoesNotExist(APIException): # This is not a general permission. It specifically for triggering comparisons. class ProjectPreprodArtifactPermission(OrganizationEventPermission): + readonly_mutation_scope_exceptions = { + "POST": "Preprod comparison triggers preserve read-only token access for now.", + "PUT": "Preprod comparison triggers preserve read-only token access for now.", + } scope_map = { "GET": ["event:read", "event:write", "event:admin"], # Some simple actions, like triggering comparisons, should be allowed diff --git a/src/sentry/seer/endpoints/issue_view_title_generate.py b/src/sentry/seer/endpoints/issue_view_title_generate.py index 1e02445b93e5..bef34aa8b64d 100644 --- a/src/sentry/seer/endpoints/issue_view_title_generate.py +++ b/src/sentry/seer/endpoints/issue_view_title_generate.py @@ -29,6 +29,9 @@ class IssueViewTitleGeneratePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Issue title generation is a read-like POST helper and needs separate cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py index 70764d1228f3..3e28ccb1f9a2 100644 --- a/src/sentry/seer/endpoints/organization_seer_explorer_chat.py +++ b/src/sentry/seer/endpoints/organization_seer_explorer_chat.py @@ -64,6 +64,9 @@ class SeerExplorerChatSerializer(serializers.Serializer): class OrganizationSeerExplorerChatPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Seer explorer chat is a read-like POST helper and needs separate cleanup.", + } scope_map = { "GET": ["org:read"], "POST": ["org:read"], diff --git a/src/sentry/seer/endpoints/organization_seer_explorer_update.py b/src/sentry/seer/endpoints/organization_seer_explorer_update.py index 722fedc69ac6..24752d974172 100644 --- a/src/sentry/seer/endpoints/organization_seer_explorer_update.py +++ b/src/sentry/seer/endpoints/organization_seer_explorer_update.py @@ -24,6 +24,9 @@ class OrganizationSeerExplorerUpdatePermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Seer explorer updates are POST helpers and need separate contract cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/seer/endpoints/organization_seer_rpc.py b/src/sentry/seer/endpoints/organization_seer_rpc.py index 8dd93e026d16..79fe734a8776 100644 --- a/src/sentry/seer/endpoints/organization_seer_rpc.py +++ b/src/sentry/seer/endpoints/organization_seer_rpc.py @@ -151,6 +151,9 @@ class SeerRpcPermission(OrganizationPermission): # Seer RPCs uses POST requests but is actually read only # So relax the permissions here. + readonly_mutation_scope_exceptions = { + "POST": "Seer RPC POST is intentionally read-only and tracked separately.", + } scope_map = { "POST": ["org:read", "org:write", "org:admin"], } diff --git a/src/sentry/seer/endpoints/organization_trace_summary.py b/src/sentry/seer/endpoints/organization_trace_summary.py index e0b9b766a24c..8eb28b0dd89a 100644 --- a/src/sentry/seer/endpoints/organization_trace_summary.py +++ b/src/sentry/seer/endpoints/organization_trace_summary.py @@ -21,6 +21,9 @@ class OrganizationTraceSummaryPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Trace summary is a read-like POST helper and needs separate contract cleanup.", + } scope_map = { "POST": ["org:read"], } diff --git a/src/sentry/seer/endpoints/trace_explorer_ai_setup.py b/src/sentry/seer/endpoints/trace_explorer_ai_setup.py index 35016ca955ab..ef4a9b05b945 100644 --- a/src/sentry/seer/endpoints/trace_explorer_ai_setup.py +++ b/src/sentry/seer/endpoints/trace_explorer_ai_setup.py @@ -27,6 +27,9 @@ class OrganizationTraceExplorerAIPermission(OrganizationPermission): + readonly_mutation_scope_exceptions = { + "POST": "Trace explorer POST helpers are read-like actions and need separate cleanup.", + } scope_map = { "GET": ["org:read"], "POST": ["org:read"], diff --git a/src/sentry/sentry_apps/api/bases/sentryapps.py b/src/sentry/sentry_apps/api/bases/sentryapps.py index 637d0a27d645..7fcf6f57afc3 100644 --- a/src/sentry/sentry_apps/api/bases/sentryapps.py +++ b/src/sentry/sentry_apps/api/bases/sentryapps.py @@ -432,6 +432,9 @@ def convert_args(self, request: Request, uuid, *args, **kwargs): class SentryAppInstallationExternalIssuePermission(SentryAppInstallationPermission): + readonly_mutation_scope_exceptions = { + "POST": "This endpoint creates an external issue link for an issue the caller can already read.", + } scope_map = { "POST": ("event:read", "event:write", "event:admin"), "DELETE": ("event:admin",), diff --git a/tests/sentry/api/test_permissions.py b/tests/sentry/api/test_permissions.py index 1d2286204b35..faef4de77ea9 100644 --- a/tests/sentry/api/test_permissions.py +++ b/tests/sentry/api/test_permissions.py @@ -1,5 +1,11 @@ +from collections.abc import Generator + +from django.test import SimpleTestCase +from django.urls import URLPattern, URLResolver +from django.urls.resolvers import get_resolver from rest_framework.views import APIView +from sentry.api.base import Endpoint from sentry.api.permissions import ( DemoSafePermission, DisallowImpersonatedTokenCreation, @@ -8,10 +14,34 @@ SuperuserOrStaffFeatureFlaggedPermission, SuperuserPermission, ) +from sentry.conf.server import SENTRY_READONLY_SCOPES from sentry.demo_mode.utils import READONLY_SCOPES from sentry.organizations.services.organization import organization_service from sentry.testutils.cases import DRFPermissionTestCase from sentry.testutils.helpers.options import override_options +from sentry.testutils.silo import no_silo_test + +MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + + +def _iter_endpoint_view_classes(urlpatterns, base: str = "") -> Generator[type[Endpoint]]: + for pattern in urlpatterns: + if isinstance(pattern, URLResolver): + yield from _iter_endpoint_view_classes( + pattern.url_patterns, base + str(pattern.pattern) + ) + elif isinstance(pattern, URLPattern): + callback = pattern.callback + if hasattr(callback, "view_class") and issubclass(callback.view_class, Endpoint): + yield callback.view_class + + +def _get_class_path(cls: type[object]) -> str: + return f"{cls.__module__}.{cls.__name__}" + + +def _get_readonly_mutation_scope_exceptions(cls: type[object]) -> dict[str, str]: + return getattr(cls, "readonly_mutation_scope_exceptions", {}) or {} class PermissionsTest(DRFPermissionTestCase): @@ -223,3 +253,62 @@ def test_determine_access_no_demo_users(self) -> None: ) assert readonly_rpc_context.member.scopes == list(self.org_member_scopes) + + +@no_silo_test +class PublishedMutationScopeTest(SimpleTestCase): + def test_readonly_mutation_scope_exceptions_are_notes(self) -> None: + for view_cls in sorted( + set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path + ): + for cls in (view_cls, *getattr(view_cls, "permission_classes", ())): + exceptions = getattr(cls, "readonly_mutation_scope_exceptions", None) + if exceptions is None: + continue + + assert isinstance(exceptions, dict), ( + f"{_get_class_path(cls)} readonly_mutation_scope_exceptions must be a dict" + ) + + for method, note in exceptions.items(): + assert method in MUTATION_METHODS, ( + f"{_get_class_path(cls)} readonly_mutation_scope_exceptions[{method!r}] " + "must target a mutation method" + ) + assert isinstance(note, str) and note.strip(), ( + f"{_get_class_path(cls)} readonly_mutation_scope_exceptions[{method!r}] " + "must be a non-empty note" + ) + + def test_published_mutation_endpoints_require_readonly_scope_notes(self) -> None: + missing_notes = [] + + for view_cls in sorted( + set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path + ): + publish_status = getattr(view_cls, "publish_status", {}) or {} + permission_classes = getattr(view_cls, "permission_classes", ()) or () + view_exceptions = _get_readonly_mutation_scope_exceptions(view_cls) + + for method in MUTATION_METHODS & set(publish_status): + for permission_cls in permission_classes: + readonly_scopes = ( + set(getattr(permission_cls, "scope_map", {}).get(method, ())) + & SENTRY_READONLY_SCOPES + ) + if not readonly_scopes: + continue + + if view_exceptions.get(method) or _get_readonly_mutation_scope_exceptions( + permission_cls + ).get(method): + continue + + missing_notes.append( + f"{_get_class_path(view_cls)} {method} accepts readonly scopes " + f"{sorted(readonly_scopes)} via {_get_class_path(permission_cls)}. " + "Remove the readonly scopes or add " + "readonly_mutation_scope_exceptions[method] with a justification note." + ) + + assert not missing_notes, "\n".join(missing_notes) From bd30311b2a726dc25f981c0137b37fa012d1ce2e Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:14:19 -0700 Subject: [PATCH 02/25] fix(api): Require write scopes on mutation paths Tighten published mutation endpoints that already have an obvious existing write-capable scope. Previously, several write methods accepted readonly scopes like org:read, project:read, or event:read even though they mutate server-side state. Require the existing write scope for those surfaces instead. Keep session behavior intact, including team-scoped alert access, but stop exposing readonly token writes through these endpoints. Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/incident.py | 6 +- src/sentry/api/bases/organization.py | 59 ++++++++++++++----- .../endpoints/project_repo_path_parsing.py | 2 +- .../endpoints/organization_dashboards.py | 6 +- .../endpoints/discover_key_transactions.py | 25 ++++++-- .../issues/endpoints/project_user_issue.py | 2 +- .../endpoints/project_replay_details.py | 8 +-- .../endpoints/project_replay_summary.py | 4 +- 8 files changed, 80 insertions(+), 32 deletions(-) diff --git a/src/sentry/api/bases/incident.py b/src/sentry/api/bases/incident.py index 1a0bd6e04a86..d705fe83c14c 100644 --- a/src/sentry/api/bases/incident.py +++ b/src/sentry/api/bases/incident.py @@ -20,9 +20,9 @@ class IncidentPermission(OrganizationPermission): "project:write", "project:admin", ], - "POST": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], - "PUT": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], - "DELETE": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], + "POST": ["org:write", "org:admin", "project:write", "project:admin"], + "PUT": ["org:write", "org:admin", "project:write", "project:admin"], + "DELETE": ["org:write", "org:admin", "project:write", "project:admin"], } diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 24bc36b1ed0a..f597e4be7394 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -29,6 +29,7 @@ from sentry.models.project import Project from sentry.models.release import Release from sentry.models.releases.release_project import ReleaseProject +from sentry.models.team import Team from sentry.organizations.services.organization import ( RpcOrganization, RpcUserOrganizationContext, @@ -158,8 +159,8 @@ class OrganizationIntegrationsPermission(OrganizationPermission): class OrganizationIntegrationsLoosePermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], - "POST": ["org:read", "org:write", "org:admin", "org:integrations"], - "PUT": ["org:read", "org:write", "org:admin", "org:integrations"], + "POST": ["org:write", "org:admin", "org:integrations"], + "PUT": ["org:write", "org:admin", "org:integrations"], "DELETE": ["org:admin", "org:integrations"], } @@ -167,7 +168,7 @@ class OrganizationIntegrationsLoosePermission(OrganizationPermission): class OrganizationCodeMappingsBulkPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], - "POST": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], + "POST": ["org:write", "org:admin", "org:integrations", "org:ci"], } @@ -212,37 +213,67 @@ class OrganizationSearchPermission(OrganizationPermission): class OrganizationDataExportPermission(OrganizationPermission): scope_map = { "GET": ["event:read", "event:write", "event:admin"], - "POST": ["event:read", "event:write", "event:admin"], + "POST": ["event:write", "event:admin"], } +def _has_any_team_scope(request: Request, scope: str) -> bool: + if not request.access.team_ids_with_membership: + return False + + teams = Team.objects.filter(id__in=request.access.team_ids_with_membership) + return any(request.access.has_team_scope(team, scope) for team in teams) + + class OrganizationAlertRulePermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - # grant org:read permission, but raise permission denied if the members aren't allowed - # to create alerts and the user isn't a team admin - "POST": ["org:read", "org:write", "org:admin", "alerts:write"], + "POST": ["org:write", "org:admin", "alerts:write"], "PUT": ["org:write", "org:admin", "alerts:write"], "DELETE": ["org:write", "org:admin", "alerts:write"], } + def has_object_permission( + self, + request: Request, + view: APIView, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ) -> bool: + if super().has_object_permission(request, view, organization): + return True + + return request.method in {"POST", "PUT", "DELETE"} and _has_any_team_scope( + request, "alerts:write" + ) + class OrganizationDetectorPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - # grant org:read permission, but raise permission denied if the members aren't allowed - # to create alerts and the user isn't a team admin - "POST": ["org:read", "org:write", "org:admin", "alerts:write"], - "PUT": ["org:read", "org:write", "org:admin", "alerts:write"], - "DELETE": ["org:read", "org:write", "org:admin", "alerts:write"], + "POST": ["org:write", "org:admin", "alerts:write"], + "PUT": ["org:write", "org:admin", "alerts:write"], + "DELETE": ["org:write", "org:admin", "alerts:write"], } + def has_object_permission( + self, + request: Request, + view: APIView, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ) -> bool: + if super().has_object_permission(request, view, organization): + return True + + return request.method in {"POST", "PUT", "DELETE"} and _has_any_team_scope( + request, "alerts:write" + ) + class OrgAuthTokenPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin"], - "POST": ["org:read", "org:write", "org:admin"], - "PUT": ["org:read", "org:write", "org:admin"], + "POST": ["org:write", "org:admin"], + "PUT": ["org:write", "org:admin"], "DELETE": ["org:write", "org:admin"], } diff --git a/src/sentry/api/endpoints/project_repo_path_parsing.py b/src/sentry/api/endpoints/project_repo_path_parsing.py index c530e5d1105f..830fc1fe62e5 100644 --- a/src/sentry/api/endpoints/project_repo_path_parsing.py +++ b/src/sentry/api/endpoints/project_repo_path_parsing.py @@ -96,7 +96,7 @@ class ProjectRepoPathParsingEndpointLoosePermission(ProjectPermission): """ scope_map = { - "POST": ["org:read", "project:write", "project:admin"], + "POST": ["project:write"], } diff --git a/src/sentry/dashboards/endpoints/organization_dashboards.py b/src/sentry/dashboards/endpoints/organization_dashboards.py index 358fb6cef5fc..1e4008b8532c 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboards.py +++ b/src/sentry/dashboards/endpoints/organization_dashboards.py @@ -303,9 +303,9 @@ def sync_prebuilt_dashboards(organization: Organization) -> None: class OrganizationDashboardsPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin"], - "POST": ["org:read", "org:write", "org:admin"], - "PUT": ["org:read", "org:write", "org:admin"], - "DELETE": ["org:read", "org:write", "org:admin"], + "POST": ["org:write", "org:admin"], + "PUT": ["org:write", "org:admin"], + "DELETE": ["org:write", "org:admin"], } def has_object_permission( diff --git a/src/sentry/discover/endpoints/discover_key_transactions.py b/src/sentry/discover/endpoints/discover_key_transactions.py index 4c630e0f3d94..d65ef53f892e 100644 --- a/src/sentry/discover/endpoints/discover_key_transactions.py +++ b/src/sentry/discover/endpoints/discover_key_transactions.py @@ -23,14 +23,31 @@ from sentry.models.team import Team +def _has_any_team_scope(request: Request, scopes: list[str]) -> bool: + if not request.access.team_ids_with_membership: + return False + + teams = Team.objects.filter(id__in=request.access.team_ids_with_membership) + return any( + any(request.access.has_team_scope(team, scope) for scope in scopes) for team in teams + ) + + class KeyTransactionPermission(OrganizationPermission): scope_map = { - "GET": ["org:read"], - "POST": ["org:read"], - "PUT": ["org:read"], - "DELETE": ["org:read"], + "GET": ["project:read"], + "POST": ["project:write"], + "PUT": ["project:write"], + "DELETE": ["project:write"], } + def has_object_permission(self, request: Request, view, obj: object) -> bool: + if super().has_object_permission(request, view, obj): + return True + + allowed_scopes = self.scope_map.get(request.method or "", []) + return _has_any_team_scope(request, allowed_scopes) + @cell_silo_endpoint class KeyTransactionEndpoint(KeyTransactionBase): diff --git a/src/sentry/issues/endpoints/project_user_issue.py b/src/sentry/issues/endpoints/project_user_issue.py index d8e6b07e51a6..3effe6dd7443 100644 --- a/src/sentry/issues/endpoints/project_user_issue.py +++ b/src/sentry/issues/endpoints/project_user_issue.py @@ -180,7 +180,7 @@ class WebVitalsIssueDataSerializer(ProjectUserIssueRequestSerializer): class ProjectUserIssuePermission(ProjectPermission): scope_map = { "GET": [], - "POST": ["event:read", "event:write", "event:admin"], + "POST": ["event:write", "event:admin"], "PUT": [], "DELETE": [], } diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 24e2b8444390..bf41f407ff54 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -20,10 +20,10 @@ class ReplayDetailsPermission(ProjectPermission): scope_map = { - "GET": ["project:read", "project:write", "project:admin"], - "POST": ["project:write", "project:admin"], - "PUT": ["project:write", "project:admin"], - "DELETE": ["project:read", "project:write", "project:admin"], + "GET": ["project:read"], + "POST": ["project:write"], + "PUT": ["project:write"], + "DELETE": ["event:write"], } diff --git a/src/sentry/replays/endpoints/project_replay_summary.py b/src/sentry/replays/endpoints/project_replay_summary.py index 4b074f5b3dd2..f1f1150bf480 100644 --- a/src/sentry/replays/endpoints/project_replay_summary.py +++ b/src/sentry/replays/endpoints/project_replay_summary.py @@ -38,8 +38,8 @@ class ReplaySummaryPermission(ProjectPermission): scope_map = { - "GET": ["event:read", "event:write", "event:admin"], - "POST": ["event:read", "event:write", "event:admin"], + "GET": ["event:read"], + "POST": ["event:write"], "PUT": [], "DELETE": [], } From 2ecd032b51982ae5341bc06e9165a37b2fc16d80 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:51:57 -0700 Subject: [PATCH 03/25] fix(api): narrow write scope mutation changes Drop the member-session surfaces that were not actually uncontroversial from this PR. Dashboards, code mappings, repo path parsing, key transactions, and org auth token updates still rely on broader session behavior today, so tightening them here broke existing member flows. Revert those paths in this branch and keep the safe mutation tightening for incidents, alerts, user issue creation, data export, and replay summary. Also keep replay delete in the project scope domain so delete remains consistent with the rest of the replay permission class. Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/organization.py | 10 ++++---- .../endpoints/project_repo_path_parsing.py | 2 +- .../endpoints/organization_dashboards.py | 6 ++--- .../endpoints/discover_key_transactions.py | 25 +++---------------- .../endpoints/project_replay_details.py | 2 +- 5 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index f597e4be7394..974ddb1d2cfa 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -159,8 +159,8 @@ class OrganizationIntegrationsPermission(OrganizationPermission): class OrganizationIntegrationsLoosePermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], - "POST": ["org:write", "org:admin", "org:integrations"], - "PUT": ["org:write", "org:admin", "org:integrations"], + "POST": ["org:read", "org:write", "org:admin", "org:integrations"], + "PUT": ["org:read", "org:write", "org:admin", "org:integrations"], "DELETE": ["org:admin", "org:integrations"], } @@ -168,7 +168,7 @@ class OrganizationIntegrationsLoosePermission(OrganizationPermission): class OrganizationCodeMappingsBulkPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], - "POST": ["org:write", "org:admin", "org:integrations", "org:ci"], + "POST": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], } @@ -272,8 +272,8 @@ def has_object_permission( class OrgAuthTokenPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin"], - "POST": ["org:write", "org:admin"], - "PUT": ["org:write", "org:admin"], + "POST": ["org:read", "org:write", "org:admin"], + "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:write", "org:admin"], } diff --git a/src/sentry/api/endpoints/project_repo_path_parsing.py b/src/sentry/api/endpoints/project_repo_path_parsing.py index 830fc1fe62e5..c530e5d1105f 100644 --- a/src/sentry/api/endpoints/project_repo_path_parsing.py +++ b/src/sentry/api/endpoints/project_repo_path_parsing.py @@ -96,7 +96,7 @@ class ProjectRepoPathParsingEndpointLoosePermission(ProjectPermission): """ scope_map = { - "POST": ["project:write"], + "POST": ["org:read", "project:write", "project:admin"], } diff --git a/src/sentry/dashboards/endpoints/organization_dashboards.py b/src/sentry/dashboards/endpoints/organization_dashboards.py index 1e4008b8532c..358fb6cef5fc 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboards.py +++ b/src/sentry/dashboards/endpoints/organization_dashboards.py @@ -303,9 +303,9 @@ def sync_prebuilt_dashboards(organization: Organization) -> None: class OrganizationDashboardsPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin"], - "POST": ["org:write", "org:admin"], - "PUT": ["org:write", "org:admin"], - "DELETE": ["org:write", "org:admin"], + "POST": ["org:read", "org:write", "org:admin"], + "PUT": ["org:read", "org:write", "org:admin"], + "DELETE": ["org:read", "org:write", "org:admin"], } def has_object_permission( diff --git a/src/sentry/discover/endpoints/discover_key_transactions.py b/src/sentry/discover/endpoints/discover_key_transactions.py index d65ef53f892e..4c630e0f3d94 100644 --- a/src/sentry/discover/endpoints/discover_key_transactions.py +++ b/src/sentry/discover/endpoints/discover_key_transactions.py @@ -23,31 +23,14 @@ from sentry.models.team import Team -def _has_any_team_scope(request: Request, scopes: list[str]) -> bool: - if not request.access.team_ids_with_membership: - return False - - teams = Team.objects.filter(id__in=request.access.team_ids_with_membership) - return any( - any(request.access.has_team_scope(team, scope) for scope in scopes) for team in teams - ) - - class KeyTransactionPermission(OrganizationPermission): scope_map = { - "GET": ["project:read"], - "POST": ["project:write"], - "PUT": ["project:write"], - "DELETE": ["project:write"], + "GET": ["org:read"], + "POST": ["org:read"], + "PUT": ["org:read"], + "DELETE": ["org:read"], } - def has_object_permission(self, request: Request, view, obj: object) -> bool: - if super().has_object_permission(request, view, obj): - return True - - allowed_scopes = self.scope_map.get(request.method or "", []) - return _has_any_team_scope(request, allowed_scopes) - @cell_silo_endpoint class KeyTransactionEndpoint(KeyTransactionBase): diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index bf41f407ff54..5c08ea295b43 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -23,7 +23,7 @@ class ReplayDetailsPermission(ProjectPermission): "GET": ["project:read"], "POST": ["project:write"], "PUT": ["project:write"], - "DELETE": ["event:write"], + "DELETE": ["project:write"], } From 2f08f098ba6de3ce6a4525e74f9d12baf92a14c3 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:54:25 -0700 Subject: [PATCH 04/25] chore(meta): Add CODEOWNERS coverage for markdownTextArea Add an explicit owner for static/app/components/markdownTextArea.tsx so the branch clears the CODEOWNERS coverage check. The file was missing ownership coverage entirely, which caused the fresh PR run to fail even though the API changes on this branch are backend-only. Co-Authored-By: OpenAI Codex --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 77cb394318e4..18059ee221fc 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -439,6 +439,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /static/app/components/analyticsArea.spec.tsx @getsentry/app-frontend /static/app/components/analyticsArea.tsx @getsentry/app-frontend /static/app/components/loading/ @getsentry/app-frontend +/static/app/components/markdownTextArea.tsx @getsentry/app-frontend /static/app/components/events/interfaces/ @getsentry/app-frontend /static/app/components/forms/ @getsentry/app-frontend /static/app/components/featureShowcase.mdx @getsentry/app-frontend From 236d26206ad547c4c946132a8abd5aabfa8b4597 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:06:20 -0700 Subject: [PATCH 05/25] ref(replays): Expand explicit replay scopes Keep the replay permission maps aligned with the current repo style of listing implied scopes explicitly. This does not change the effective permissions on these endpoints, but it removes a local inconsistency in this PR where replay endpoints relied on hierarchy expansion while nearby scope maps did not. Co-Authored-By: OpenAI Codex --- src/sentry/replays/endpoints/project_replay_details.py | 8 ++++---- src/sentry/replays/endpoints/project_replay_summary.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 5c08ea295b43..885f47b03f6b 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -20,10 +20,10 @@ class ReplayDetailsPermission(ProjectPermission): scope_map = { - "GET": ["project:read"], - "POST": ["project:write"], - "PUT": ["project:write"], - "DELETE": ["project:write"], + "GET": ["project:read", "project:write", "project:admin"], + "POST": ["project:write", "project:admin"], + "PUT": ["project:write", "project:admin"], + "DELETE": ["project:write", "project:admin"], } diff --git a/src/sentry/replays/endpoints/project_replay_summary.py b/src/sentry/replays/endpoints/project_replay_summary.py index f1f1150bf480..e9addac20247 100644 --- a/src/sentry/replays/endpoints/project_replay_summary.py +++ b/src/sentry/replays/endpoints/project_replay_summary.py @@ -38,8 +38,8 @@ class ReplaySummaryPermission(ProjectPermission): scope_map = { - "GET": ["event:read"], - "POST": ["event:write"], + "GET": ["event:read", "event:write", "event:admin"], + "POST": ["event:write", "event:admin"], "PUT": [], "DELETE": [], } From e8cab3e3bd17fc7e6f8133ddb39d4bcfaafa1736 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:13:35 -0700 Subject: [PATCH 06/25] fix(replays): require event write for replay deletes Replay delete is a public mutation, so it should not depend on a project read-style contract. Move the DELETE permission to event write/admin so member sessions keep working while token auth requires a real write scope. Add direct endpoint tests covering the token contract: event:read is denied and event:write is allowed for replay delete. Co-Authored-By: OpenAI Codex --- .../endpoints/project_replay_details.py | 2 +- .../endpoints/test_project_replay_details.py | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 885f47b03f6b..264c67dd3768 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -23,7 +23,7 @@ class ReplayDetailsPermission(ProjectPermission): "GET": ["project:read", "project:write", "project:admin"], "POST": ["project:write", "project:admin"], "PUT": ["project:write", "project:admin"], - "DELETE": ["project:write", "project:admin"], + "DELETE": ["event:write", "event:admin"], } diff --git a/tests/sentry/replays/endpoints/test_project_replay_details.py b/tests/sentry/replays/endpoints/test_project_replay_details.py index 4cb9985f673b..db03b433356a 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_details.py +++ b/tests/sentry/replays/endpoints/test_project_replay_details.py @@ -5,13 +5,16 @@ from django.urls import reverse +from sentry.models.apitoken import ApiToken from sentry.models.files.file import File from sentry.replays.lib import kafka from sentry.replays.lib.storage import RecordingSegmentStorageMeta, storage from sentry.replays.models import ReplayRecordingSegment from sentry.replays.testutils import assert_expected_response, mock_expected_response, mock_replay +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, ReplaysSnubaTestCase from sentry.testutils.helpers import TaskRunner +from sentry.testutils.silo import assume_test_silo_mode from sentry.utils import kafka_config REPLAYS_FEATURES = {"organizations:session-replay": True} @@ -192,6 +195,29 @@ def test_delete_replay_from_filestore(self) -> None: except File.DoesNotExist: pass + def test_delete_requires_event_write_scope_for_api_tokens(self) -> None: + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) + + with self.feature(REPLAYS_FEATURES): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) + + assert response.status_code == 403 + + def test_delete_allows_event_write_scope_for_api_tokens(self) -> None: + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["event:write"]) + + with self.feature(REPLAYS_FEATURES): + with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) + + assert response.status_code == 204 + def test_delete_replay_from_clickhouse_data(self) -> None: """Test deleting files uploaded through the direct storage interface.""" kept_replay_id = uuid4().hex From 654e6e9e3afa72a29b86381bcfad43a8356bfbd8 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:16:28 -0700 Subject: [PATCH 07/25] ref(api): defer incident mutation scope tightening Remove the incident permission change from the existing-write-scope pass. Incident mutation has an older member-access contract tied to project access, so tightening it cleanly needs a dedicated scope decision instead of being folded into the uncontroversial cleanup. Also drop an unrelated CODEOWNERS hunk so this branch stays focused on backend mutation scope changes. Co-Authored-By: OpenAI Codex --- .github/CODEOWNERS | 1 - src/sentry/api/bases/incident.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 18059ee221fc..77cb394318e4 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -439,7 +439,6 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /static/app/components/analyticsArea.spec.tsx @getsentry/app-frontend /static/app/components/analyticsArea.tsx @getsentry/app-frontend /static/app/components/loading/ @getsentry/app-frontend -/static/app/components/markdownTextArea.tsx @getsentry/app-frontend /static/app/components/events/interfaces/ @getsentry/app-frontend /static/app/components/forms/ @getsentry/app-frontend /static/app/components/featureShowcase.mdx @getsentry/app-frontend diff --git a/src/sentry/api/bases/incident.py b/src/sentry/api/bases/incident.py index d705fe83c14c..1a0bd6e04a86 100644 --- a/src/sentry/api/bases/incident.py +++ b/src/sentry/api/bases/incident.py @@ -20,9 +20,9 @@ class IncidentPermission(OrganizationPermission): "project:write", "project:admin", ], - "POST": ["org:write", "org:admin", "project:write", "project:admin"], - "PUT": ["org:write", "org:admin", "project:write", "project:admin"], - "DELETE": ["org:write", "org:admin", "project:write", "project:admin"], + "POST": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], + "PUT": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], + "DELETE": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], } From a56d27b4b3f45449cd73a8b273eec73eef207447 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:42:32 -0700 Subject: [PATCH 08/25] ref(api): document helper permission intent Keep replay summary POST on read-level replay scopes and mark it as an explicit readonly-mutation exception, since it derives summary data from existing replay/event data. Document the preview endpoints that intentionally keep write-aligned permissions because they are part of alert and monitor authoring flows, even though they use POST helper endpoints. Add replay summary token tests covering the read-scope contract. Co-Authored-By: OpenAI Codex --- .../endpoints/project_replay_summary.py | 9 ++++- .../organization_events_anomalies.py | 2 + ...organization_uptime_alert_preview_check.py | 2 + ...ganization_uptime_assertion_suggestions.py | 2 + .../endpoints/test_project_replay_summary.py | 38 +++++++++++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/sentry/replays/endpoints/project_replay_summary.py b/src/sentry/replays/endpoints/project_replay_summary.py index e9addac20247..7b4981dcbc33 100644 --- a/src/sentry/replays/endpoints/project_replay_summary.py +++ b/src/sentry/replays/endpoints/project_replay_summary.py @@ -39,7 +39,7 @@ class ReplaySummaryPermission(ProjectPermission): scope_map = { "GET": ["event:read", "event:write", "event:admin"], - "POST": ["event:write", "event:admin"], + "POST": ["event:read", "event:write", "event:admin"], "PUT": [], "DELETE": [], } @@ -52,6 +52,13 @@ class ProjectReplaySummaryEndpoint(ProjectReplayEndpoint): "GET": ApiPublishStatus.EXPERIMENTAL, "POST": ApiPublishStatus.EXPERIMENTAL, } + readonly_mutation_scope_exceptions = { + "POST": ( + "POST starts replay-summary generation but only derives summary data from existing " + "replay/event data. It intentionally follows replay read access instead of requiring " + "a separate write capability." + ) + } permission_classes = (ReplaySummaryPermission,) def __init__(self, **kw) -> None: diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 53385a13bcc8..260e3e0bf64b 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -33,6 +33,8 @@ class OrganizationEventsAnomaliesEndpoint(OrganizationEventsEndpointBase): publish_status = { "POST": ApiPublishStatus.EXPERIMENTAL, } + # This POST previews anomaly-detection config used while authoring metric + # alerts/detectors, so it intentionally follows alert-write permissions. permission_classes = (OrganizationAlertRulePermission,) @extend_schema( diff --git a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py index 4846359b7403..6cd7e380d779 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py +++ b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py @@ -30,6 +30,8 @@ @cell_silo_endpoint class OrganizationUptimeAlertPreviewCheckEndpoint(OrganizationEndpoint): owner = ApiOwner.CRONS + # This POST previews monitor creation and validation, so it intentionally + # uses the same permission surface as creating the alert itself. permission_classes = (OrganizationAlertRulePermission,) publish_status = { diff --git a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py index 855aa428ac54..546bb259b956 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py +++ b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py @@ -50,6 +50,8 @@ class OrganizationUptimeAssertionSuggestionsEndpoint(OrganizationEndpoint): """ owner = ApiOwner.CRONS + # This POST is part of the uptime monitor authoring flow, so it should + # track the same alert-write permission as the monitor it helps create. permission_classes = (OrganizationAlertRulePermission,) publish_status = { diff --git a/tests/sentry/replays/endpoints/test_project_replay_summary.py b/tests/sentry/replays/endpoints/test_project_replay_summary.py index e2ae8a0a4af4..fb2b710c6e07 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_summary.py +++ b/tests/sentry/replays/endpoints/test_project_replay_summary.py @@ -7,8 +7,11 @@ from django.conf import settings from django.urls import reverse +from sentry.models.apitoken import ApiToken from sentry.replays.testutils import mock_replay +from sentry.silo.base import SiloMode from sentry.testutils.cases import SnubaTestCase, TransactionTestCase +from sentry.testutils.silo import assume_test_silo_mode from sentry.utils import json @@ -140,6 +143,41 @@ def test_post_simple(self, mock_seer_request: Mock) -> None: assert body["project_id"] == self.project.id assert body.get("temperature") is None + @patch("sentry.replays.endpoints.project_replay_summary.make_replay_summary_start_request") + def test_post_allows_event_read_scope_for_api_tokens(self, mock_seer_request: Mock) -> None: + mock_seer_request.return_value = MockSeerResponse(200, json_data={"hello": "world"}) + self.store_replay() + + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) + + with self.feature(self.features): + response = self.client.post( + self.url, + data={"num_segments": 1}, + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + assert response.json() == {"hello": "world"} + + def test_post_requires_replay_read_scope_for_api_tokens(self) -> None: + self.store_replay() + + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["org:read"]) + + with self.feature(self.features): + response = self.client.post( + self.url, + data={"num_segments": 1}, + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + def test_post_replay_not_found(self) -> None: with self.feature(self.features): response = self.client.post( From d511a66586bdabeb252e445a7b6428f62be65c02 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 16:00:54 -0700 Subject: [PATCH 09/25] test(api): Add regression coverage for tightened scopes Add endpoint-level regression tests for the scope changes on this branch so each permission update is covered directly. Cover the write-scope transitions for data export, alert rule creation, project user issues, replay delete, detector updates, and the alert helper endpoints. This keeps the branch from relying on indirect role coverage alone and makes the replay summary readonly exception explicit next to direct token tests. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex --- .../data_export/endpoints/test_data_export.py | 31 +++++++ .../test_organization_alert_rule_index.py | 34 ++++++++ .../endpoints/test_project_user_issue.py | 50 ++++++++++- .../endpoints/test_project_replay_details.py | 11 +++ .../test_organization_events_anomalies.py | 49 +++++++++++ .../test_organization_uptime_alert_preview.py | 24 ++++++ ...ganization_uptime_assertion_suggestions.py | 85 +++++++++++++++++++ .../test_organization_detector_details.py | 33 +++++++ 8 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py diff --git a/tests/sentry/data_export/endpoints/test_data_export.py b/tests/sentry/data_export/endpoints/test_data_export.py index 09905c42b6f3..3b0e4229d379 100644 --- a/tests/sentry/data_export/endpoints/test_data_export.py +++ b/tests/sentry/data_export/endpoints/test_data_export.py @@ -2,6 +2,8 @@ from typing import Any +from django.urls import reverse + from sentry.data_export.base import ExportQueryType, ExportStatus from sentry.data_export.models import ExportedData from sentry.data_export.writers import OutputMode @@ -24,6 +26,7 @@ def setUp(self) -> None: ) self.create_member(user=self.user, organization=self.org, teams=[self.team]) self.login_as(user=self.user) + self.url = reverse(self.endpoint, args=[self.org.slug]) def make_payload( self, payload_type: str, extras: dict[str, Any] | None = None, overwrite: bool = False @@ -78,6 +81,34 @@ def test_authorization(self) -> None: with self.feature("organizations:discover-query"): self.get_error_response(self.org.slug, status_code=403, **modified_payload) + def test_post_requires_event_write_scope_for_api_keys(self) -> None: + payload = self.make_payload("issue") + api_key = self.create_api_key(organization=self.org, scope_list=["event:read"]) + + with self.feature("organizations:discover-query"): + response = self.client.post( + self.url, + data=payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 + + def test_post_allows_event_write_scope_for_api_keys(self) -> None: + payload = self.make_payload("issue") + api_key = self.create_api_key(organization=self.org, scope_list=["event:write"]) + + with self.feature("organizations:discover-query"): + response = self.client.post( + self.url, + data=payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 201 + def test_new_export(self) -> None: """ Ensures that a request to this endpoint returns a 201 status code diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index a4b5414756a2..d727d6a7e1a3 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -40,6 +40,7 @@ find_channel_id_for_alert_rule, ) from sentry.integrations.slack.utils.channel import SlackChannelIdData +from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry from sentry.models.organizationmember import OrganizationMember from sentry.models.projectteam import ProjectTeam @@ -417,6 +418,10 @@ def setUp(self) -> None: ) self.login_as(self.user) + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + def test_simple(self) -> None: with ( outbox_runner(), @@ -441,6 +446,35 @@ def test_simple(self) -> None: == list(audit_log_entry)[0].ip_address ) + def test_create_requires_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("org:read") + + with self.feature(["organizations:incidents", "organizations:performance-view"]): + response = self.client.post( + f"/api/0/organizations/{self.organization.slug}/alert-rules/", + data=self.alert_rule_dict, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + def test_create_allows_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("alerts:write") + + with ( + outbox_runner(), + self.feature(["organizations:incidents", "organizations:performance-view"]), + ): + response = self.client.post( + f"/api/0/organizations/{self.organization.slug}/alert-rules/", + data=self.alert_rule_dict, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 201 + @patch("sentry.incidents.serializers.alert_rule.are_any_projects_error_upsampled") def test_count_automatically_converted_to_upsampled_count_for_upsampled_projects( self, mock_are_any_projects_error_upsampled diff --git a/tests/sentry/issues/endpoints/test_project_user_issue.py b/tests/sentry/issues/endpoints/test_project_user_issue.py index 0ea6f2aaed38..7071efc54a49 100644 --- a/tests/sentry/issues/endpoints/test_project_user_issue.py +++ b/tests/sentry/issues/endpoints/test_project_user_issue.py @@ -6,9 +6,11 @@ from sentry.issues.grouptype import WebVitalsGroup from sentry.issues.producer import PayloadType +from sentry.models.apitoken import ApiToken +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.features import with_feature -from sentry.testutils.silo import cell_silo_test +from sentry.testutils.silo import assume_test_silo_mode, cell_silo_test @cell_silo_test @@ -31,6 +33,10 @@ def setUp(self) -> None: }, ) + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + @with_feature("organizations:performance-web-vitals-seer-suggestions") def test_create_web_vitals_issue_success(self) -> None: data = { @@ -110,6 +116,48 @@ def test_no_access(self) -> None: assert response.status_code == 404 + @with_feature("organizations:performance-web-vitals-seer-suggestions") + def test_create_web_vitals_issue_requires_event_write_scope(self) -> None: + token = self._create_token("event:read") + + response = self.client.post( + self.url, + data={ + "transaction": "/test-transaction", + "issueType": WebVitalsGroup.slug, + "score": 75, + "value": 1000, + "vital": "lcp", + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + @with_feature("organizations:performance-web-vitals-seer-suggestions") + def test_create_web_vitals_issue_allows_event_write_scope(self) -> None: + token = self._create_token("event:write") + + with patch( + "sentry.issues.endpoints.project_user_issue.produce_occurrence_to_kafka" + ) as mock_produce: + response = self.client.post( + self.url, + data={ + "transaction": "/test-transaction", + "issueType": WebVitalsGroup.slug, + "score": 75, + "value": 1000, + "vital": "lcp", + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + assert response.data == {"event_id": mock_produce.call_args[1]["occurrence"].event_id} + @with_feature("organizations:performance-web-vitals-seer-suggestions") def test_missing_required_fields(self) -> None: data = { diff --git a/tests/sentry/replays/endpoints/test_project_replay_details.py b/tests/sentry/replays/endpoints/test_project_replay_details.py index db03b433356a..3d93d73bab09 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_details.py +++ b/tests/sentry/replays/endpoints/test_project_replay_details.py @@ -206,6 +206,17 @@ def test_delete_requires_event_write_scope_for_api_tokens(self) -> None: assert response.status_code == 403 + def test_delete_denies_project_write_scope_for_api_tokens(self) -> None: + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["project:write"]) + + with self.feature(REPLAYS_FEATURES): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) + + assert response.status_code == 403 + def test_delete_allows_event_write_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["event:write"]) diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 4def0135187a..441c920a0b2d 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, patch import orjson +from django.urls import reverse from urllib3 import HTTPResponse from urllib3.exceptions import TimeoutError @@ -10,6 +11,7 @@ AlertRuleSensitivity, AlertRuleThresholdType, ) +from sentry.models.apitoken import ApiToken from sentry.seer.anomaly_detection.types import ( Anomaly, AnomalyDetectionConfig, @@ -18,10 +20,12 @@ TimeSeriesPoint, ) from sentry.seer.anomaly_detection.utils import translate_direction +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.datetime import before_now, freeze_time from sentry.testutils.helpers.features import with_feature from sentry.testutils.outbox import outbox_runner +from sentry.testutils.silo import assume_test_silo_mode @freeze_time() @@ -44,6 +48,10 @@ class OrganizationEventsAnomaliesEndpointTest(APITestCase): current_timestamp_1 = one_week_ago.timestamp() current_timestamp_2 = (one_week_ago + timedelta(minutes=10)).timestamp() + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + def get_test_data(self, project_id: int) -> dict: return { "project_id": str(project_id), # UI provides project_id as str @@ -157,6 +165,47 @@ def test_member_permission(self, mock_seer_request: MagicMock) -> None: assert mock_seer_request.call_count == 1 assert resp.data == seer_return_value["timeseries"] + @with_feature("organizations:anomaly-detection-alerts") + @with_feature("organizations:incidents") + @patch( + "sentry.seer.anomaly_detection.get_historical_anomalies.seer_anomaly_detection_connection_pool.urlopen" + ) + def test_alerts_write_scope_allows_post(self, mock_seer_request: MagicMock) -> None: + mock_seer_request.return_value = HTTPResponse( + orjson.dumps(DetectAnomaliesResponse(success=True, message="", timeseries=[])), + status=200, + ) + token = self._create_token("alerts:write") + data = self.get_test_data(self.project.id) + url = reverse(self.endpoint, args=[self.organization.slug]) + + with outbox_runner(): + response = self.client.post( + url, + data=orjson.dumps(data), + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + + @with_feature("organizations:anomaly-detection-alerts") + @with_feature("organizations:incidents") + def test_org_read_scope_cannot_post(self) -> None: + token = self._create_token("org:read") + data = self.get_test_data(self.project.id) + url = reverse(self.endpoint, args=[self.organization.slug]) + + with outbox_runner(): + response = self.client.post( + url, + data=orjson.dumps(data), + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") @patch( diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py index 02aa0c908fdf..1994c1d3cff0 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py @@ -134,3 +134,27 @@ def test_alerts_write_permission(self) -> None: ) assert response.status_code == 200, response.content + + def test_org_read_scope_cannot_run_preview_check(self) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) + + url = reverse( + "sentry-api-0-organization-uptime-alert-preview-check", + kwargs={"organization_id_or_slug": self.organization.slug}, + ) + response = self.client.post( + url, + data={ + "name": "test", + "environment": "uptime-prod", + "owner": f"user:{self.user.id}", + "url": "http://sentry.io", + "timeoutMs": 1500, + "body": None, + "region": "default", + }, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py new file mode 100644 index 000000000000..15af97209628 --- /dev/null +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py @@ -0,0 +1,85 @@ +from unittest import mock + +from django.test import override_settings +from django.urls import reverse + +from sentry.conf.types.uptime import UptimeRegionConfig +from tests.sentry.uptime.endpoints import UptimeAlertBaseEndpointTest + + +@override_settings( + UPTIME_REGIONS=[ + UptimeRegionConfig( + slug="default", + name="Default Region", + config_redis_key_prefix="default", + api_endpoint="pop-st-1.uptime-checker.s4s.sentry.internal:80", + ) + ] +) +class OrganizationUptimeAssertionSuggestionsTest(UptimeAlertBaseEndpointTest): + endpoint = "sentry-api-0-organization-uptime-assertion-suggestions" + method = "post" + + def setUp(self) -> None: + super().setUp() + self.url = reverse(self.endpoint, args=[self.organization.slug]) + self.payload = { + "name": "test", + "environment": "uptime-prod", + "owner": f"user:{self.user.id}", + "url": "http://sentry.io", + "timeoutMs": 1500, + "body": None, + "region": "default", + } + + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.checker_api.invoke_checker_preview" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.UptimeCheckPreviewValidator" + ) + @mock.patch("sentry.uptime.endpoints.organization_uptime_assertion_suggestions.has_seer_access") + def test_alerts_write_scope_can_generate_suggestions( + self, + mock_has_seer_access: mock.MagicMock, + mock_validator_cls: mock.MagicMock, + mock_preview: mock.MagicMock, + mock_generate: mock.MagicMock, + ) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["alerts:write"]) + mock_has_seer_access.return_value = True + mock_validator = mock_validator_cls.return_value + mock_validator.is_valid.return_value = True + mock_validator.save.return_value = {"active_regions": ["default"]} + mock_preview.return_value = mock.Mock( + status_code=200, + json=mock.Mock(return_value={"status": 200}), + raise_for_status=mock.Mock(), + ) + mock_generate.return_value = (None, None) + + response = self.client.post( + self.url, + data=self.payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 200 + + def test_org_read_scope_cannot_generate_suggestions(self) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) + + response = self.client.post( + self.url, + data=self.payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 2a7b40a1d508..437bde515940 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -2,6 +2,7 @@ from unittest import mock import pytest +from django.urls import reverse from django.utils import timezone from sentry import audit_log @@ -13,6 +14,7 @@ from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE from sentry.incidents.utils.subscription_limits import METRIC_SUBSCRIPTION_FEATURE_FLAGS +from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry from sentry.silo.base import SiloMode from sentry.snuba.dataset import Dataset @@ -268,6 +270,10 @@ def setUp(self) -> None: } assert SnubaQuery.objects.get(id=self.snuba_query.id) + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + def assert_detector_updated(self, detector: Detector) -> None: assert detector.name == "Updated Detector" assert detector.type == MetricIssue.slug @@ -550,6 +556,33 @@ def test_disable_detector(self) -> None: assert detector.enabled is False assert detector.status == ObjectStatus.DISABLED + def test_update_requires_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("org:read") + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + def test_update_allows_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("alerts:write") + + with self.tasks(): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + self.detector.refresh_from_db() + assert self.detector.enabled is False + def test_enable_detector(self) -> None: self.detector.update(enabled=False) self.detector.update(status=ObjectStatus.DISABLED) From 9c4686421f1a4afb1b76347e7630d7bcfc957e9a Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 16:22:33 -0700 Subject: [PATCH 10/25] test(api): Fix write-scope regression tests Use supported token auth for data export, give alert-rule token tests a valid member project setup, and return a real anomaly payload in the Seer regression test. These tests were asserting the new permission contracts, but two of them were exercising invalid success paths and one was using an auth shape that the endpoint does not serialize correctly. Co-Authored-By: Codex --- .../data_export/endpoints/test_data_export.py | 19 +++++++++++++------ .../test_organization_alert_rule_index.py | 4 ++++ .../test_organization_events_anomalies.py | 19 ++++++++++++++++++- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/tests/sentry/data_export/endpoints/test_data_export.py b/tests/sentry/data_export/endpoints/test_data_export.py index 3b0e4229d379..813f7cc122b9 100644 --- a/tests/sentry/data_export/endpoints/test_data_export.py +++ b/tests/sentry/data_export/endpoints/test_data_export.py @@ -7,9 +7,12 @@ from sentry.data_export.base import ExportQueryType, ExportStatus from sentry.data_export.models import ExportedData from sentry.data_export.writers import OutputMode +from sentry.models.apitoken import ApiToken from sentry.search.utils import parse_datetime_string +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.datetime import freeze_time +from sentry.testutils.silo import assume_test_silo_mode from sentry.utils.snuba import MAX_FIELDS @@ -59,6 +62,10 @@ def make_payload( payload["query_info"].update(extras) return payload + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + def test_authorization(self) -> None: payload = self.make_payload("issue") @@ -81,30 +88,30 @@ def test_authorization(self) -> None: with self.feature("organizations:discover-query"): self.get_error_response(self.org.slug, status_code=403, **modified_payload) - def test_post_requires_event_write_scope_for_api_keys(self) -> None: + def test_post_requires_event_write_scope_for_tokens(self) -> None: payload = self.make_payload("issue") - api_key = self.create_api_key(organization=self.org, scope_list=["event:read"]) + token = self._create_token("event:read") with self.feature("organizations:discover-query"): response = self.client.post( self.url, data=payload, format="json", - HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + HTTP_AUTHORIZATION=f"Bearer {token.token}", ) assert response.status_code == 403 - def test_post_allows_event_write_scope_for_api_keys(self) -> None: + def test_post_allows_event_write_scope_for_tokens(self) -> None: payload = self.make_payload("issue") - api_key = self.create_api_key(organization=self.org, scope_list=["event:write"]) + token = self._create_token("event:write") with self.feature("organizations:discover-query"): response = self.client.post( self.url, data=payload, format="json", - HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + HTTP_AUTHORIZATION=f"Bearer {token.token}", ) assert response.status_code == 201 diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index d727d6a7e1a3..7357ad98b304 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -447,6 +447,8 @@ def test_simple(self) -> None: ) def test_create_requires_alerts_write_scope_for_tokens(self) -> None: + team = self.create_team(organization=self.organization, members=[self.user]) + ProjectTeam.objects.create(project=self.project, team=team) token = self._create_token("org:read") with self.feature(["organizations:incidents", "organizations:performance-view"]): @@ -460,6 +462,8 @@ def test_create_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 def test_create_allows_alerts_write_scope_for_tokens(self) -> None: + team = self.create_team(organization=self.organization, members=[self.user]) + ProjectTeam.objects.create(project=self.project, team=team) token = self._create_token("alerts:write") with ( diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 441c920a0b2d..b0af2ee7ca4c 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -172,7 +172,24 @@ def test_member_permission(self, mock_seer_request: MagicMock) -> None: ) def test_alerts_write_scope_allows_post(self, mock_seer_request: MagicMock) -> None: mock_seer_request.return_value = HTTPResponse( - orjson.dumps(DetectAnomaliesResponse(success=True, message="", timeseries=[])), + orjson.dumps( + DetectAnomaliesResponse( + success=True, + message="", + timeseries=[ + TimeSeriesPoint( + timestamp=self.current_timestamp_1, + value=2, + anomaly=Anomaly(anomaly_score=-0.1, anomaly_type="none"), + ), + TimeSeriesPoint( + timestamp=self.current_timestamp_2, + value=3, + anomaly=Anomaly(anomaly_score=-0.2, anomaly_type="none"), + ), + ], + ) + ), status=200, ) token = self._create_token("alerts:write") From 4251e3db2cb8d65602678c0c5447f7f2f13baf98 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 16:51:13 -0700 Subject: [PATCH 11/25] fix(api): Align alert serializers with write scopes Alert and monitor mutations now accept alerts:write at the endpoint level, but the nested project validators still required project:read. That let the request through permission checks and then failed token-based create and update flows with a 400 during validation. Allow those serializers to validate project selection against the same write-capable scopes the endpoints already advertise, and add direct token regression tests for alert rule updates and monitor creation so the contract stays enforced. Co-Authored-By: OpenAI Codex --- .../incidents/serializers/alert_rule.py | 4 +- src/sentry/monitors/validators.py | 3 +- .../test_organization_alert_rule_details.py | 40 ++++++++++++++++ .../test_organization_monitor_index.py | 47 +++++++++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/sentry/incidents/serializers/alert_rule.py b/src/sentry/incidents/serializers/alert_rule.py index 6561e883146d..7c507a10c048 100644 --- a/src/sentry/incidents/serializers/alert_rule.py +++ b/src/sentry/incidents/serializers/alert_rule.py @@ -51,6 +51,8 @@ logger = logging.getLogger(__name__) +ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") + class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRule]): """ @@ -62,7 +64,7 @@ class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRu environment = serializers.CharField(required=False, allow_null=True) projects = serializers.ListField( - child=ProjectField(scope="project:read"), + child=ProjectField(scope=ALERT_RULE_PROJECT_SCOPES), required=False, max_length=1, ) diff --git a/src/sentry/monitors/validators.py b/src/sentry/monitors/validators.py index c5c31b3f6b48..30bd816f97c1 100644 --- a/src/sentry/monitors/validators.py +++ b/src/sentry/monitors/validators.py @@ -74,6 +74,7 @@ INTERVAL_NAMES = ("year", "month", "week", "day", "hour", "minute") CRONTAB_WHITESPACE = re.compile(r"\s+") +MONITOR_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") # XXX(dcramer): @reboot is not supported (as it cannot be) NONSTANDARD_CRONTAB_SCHEDULES = { @@ -312,7 +313,7 @@ def validate(self, attrs): @extend_schema_serializer(exclude_fields=["alert_rule"]) class MonitorValidator(CamelSnakeSerializer): project = ProjectField( - scope="project:read", + scope=MONITOR_PROJECT_SCOPES, required=True, help_text="The project slug to associate the monitor to.", ) diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index f49d01c33112..d6402bb65ab5 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -11,6 +11,7 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.test import override_settings +from django.urls import reverse from httpx import HTTPError from rest_framework.exceptions import ErrorDetail from rest_framework.response import Response @@ -45,6 +46,7 @@ find_channel_id_for_alert_rule, ) from sentry.integrations.slack.utils.channel import SlackChannelIdData +from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry from sentry.models.organizationmemberteam import OrganizationMemberTeam from sentry.models.project import Project @@ -86,6 +88,10 @@ class AlertRuleDetailsBase(AlertRuleBase): endpoint = "sentry-api-0-organization-alert-rule-details" + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + @patch( "sentry.seer.anomaly_detection.store_data.seer_anomaly_detection_connection_pool.urlopen" ) @@ -1046,6 +1052,40 @@ def test_get_shows_count_when_stored_as_upsampled_count( class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase): method = "put" + def test_update_requires_alerts_write_scope_for_tokens(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + token = self._create_token("org:read") + + with self.feature("organizations:incidents"): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]), + data=self.valid_params, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + def test_update_allows_alerts_write_scope_for_tokens(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + token = self._create_token("alerts:write") + + with self.feature("organizations:incidents"): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]), + data=self.valid_params, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + self.alert_rule.refresh_from_db() + assert self.alert_rule.name == self.valid_params["name"] + def test_simple(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index c452585873b9..294254edd864 100644 --- a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py @@ -6,20 +6,24 @@ from django.conf import settings from django.test.utils import override_settings +from django.urls import reverse from rest_framework.exceptions import ErrorDetail from sentry import audit_log from sentry.analytics.events.cron_monitor_created import CronMonitorCreated, FirstCronMonitorCreated from sentry.constants import ObjectStatus +from sentry.models.apitoken import ApiToken from sentry.models.projectteam import ProjectTeam from sentry.models.rule import Rule, RuleSource from sentry.monitors.models import Monitor, MonitorStatus, ScheduleType, is_monitor_muted from sentry.monitors.utils import get_detector_for_monitor from sentry.quotas.base import SeatAssignmentResult +from sentry.silo.base import SiloMode from sentry.testutils.asserts import assert_org_audit_log_exists from sentry.testutils.cases import MonitorTestCase from sentry.testutils.helpers.analytics import assert_any_analytics_event from sentry.testutils.outbox import outbox_runner +from sentry.testutils.silo import assume_test_silo_mode from sentry.utils.outcomes import Outcome from sentry.utils.slug import DEFAULT_SLUG_ERROR_MESSAGE @@ -438,6 +442,49 @@ def setUp(self) -> None: super().setUp() self.login_as(self.user) + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + + def test_create_requires_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("org:read") + data = { + "project": self.project.slug, + "name": "My Monitor", + "type": "cron_job", + "owner": f"user:{self.user.id}", + "config": {"schedule_type": "crontab", "schedule": "@daily"}, + } + + response = self.client.post( + reverse(self.endpoint, args=[self.organization.slug]), + data=data, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + def test_create_allows_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("alerts:write") + data = { + "project": self.project.slug, + "name": "My Monitor", + "type": "cron_job", + "owner": f"user:{self.user.id}", + "config": {"schedule_type": "crontab", "schedule": "@daily"}, + } + + with outbox_runner(): + response = self.client.post( + reverse(self.endpoint, args=[self.organization.slug]), + data=data, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 201 + @patch("sentry.analytics.record") def test_simple(self, mock_record: MagicMock) -> None: data = { From 63536e99cfad262f3202f84cf2635779fc78041d Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 17:24:03 -0700 Subject: [PATCH 12/25] fix(api): Tighten alert mutations and replay delete scope Limit org-scoped alert and detector mutations to either the target project's alerts:write access or an explicit endpoint opt-in for the older team-scoped fallback. This closes the cross-team mutation path on alert rule and detector details, and adds project-scoped checks for the other shared consumers that still relied on the broad organization permission. Align replay deletion with the rest of the event delete surface by requiring event:admin instead of event:write. Add regression coverage for the scope changes and the cross-project authorization cases. Co-Authored-By: Codex --- src/sentry/api/bases/organization.py | 47 ++++++++++- src/sentry/incidents/endpoints/bases.py | 81 +++++++++++++++++++ .../organization_alert_rule_details.py | 6 ++ .../endpoints/organization_monitor_index.py | 7 +- .../endpoints/project_replay_details.py | 2 +- .../organization_events_anomalies.py | 19 ++++- ...organization_uptime_alert_preview_check.py | 1 + ...ganization_uptime_assertion_suggestions.py | 1 + .../organization_detector_details.py | 28 +++++++ .../endpoints/organization_detector_index.py | 1 + .../test_organization_alert_rule_details.py | 60 +++++++++++++- .../test_organization_monitor_index.py | 31 +++++++ .../endpoints/test_project_replay_details.py | 30 ++++++- .../test_organization_events_anomalies.py | 33 +++++++- .../test_organization_detector_details.py | 54 ++++++++++++- 15 files changed, 386 insertions(+), 15 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 974ddb1d2cfa..34876ebcc41e 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -225,6 +225,31 @@ def _has_any_team_scope(request: Request, scope: str) -> bool: return any(request.access.has_team_scope(team, scope) for team in teams) +ALERT_MUTATION_SCOPES = frozenset({"org:write", "alerts:write"}) + + +def _has_project_alert_write_access(request: Request, projects: Sequence[Project]) -> bool: + return bool(projects) and all( + request.access.has_any_project_scope(project, ALERT_MUTATION_SCOPES) for project in projects + ) + + +def _has_view_project_scoped_alert_write( + request: Request, + view: APIView, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, +) -> bool | None: + get_projects = getattr(view, "get_alert_mutation_projects", None) + if not callable(get_projects): + return None + + projects = get_projects(request, organization) + if projects is None: + return None + + return _has_project_alert_write_access(request, projects) + + class OrganizationAlertRulePermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], @@ -242,8 +267,15 @@ def has_object_permission( if super().has_object_permission(request, view, organization): return True - return request.method in {"POST", "PUT", "DELETE"} and _has_any_team_scope( - request, "alerts:write" + if request.method not in {"POST", "PUT", "DELETE"}: + return False + + project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) + if project_scoped_access is not None: + return project_scoped_access + + return bool(getattr(view, "allow_any_team_alert_write_fallback", False)) and ( + _has_any_team_scope(request, "alerts:write") ) @@ -264,8 +296,15 @@ def has_object_permission( if super().has_object_permission(request, view, organization): return True - return request.method in {"POST", "PUT", "DELETE"} and _has_any_team_scope( - request, "alerts:write" + if request.method not in {"POST", "PUT", "DELETE"}: + return False + + project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) + if project_scoped_access is not None: + return project_scoped_access + + return bool(getattr(view, "allow_any_team_alert_write_fallback", False)) and ( + _has_any_team_scope(request, "alerts:write") ) diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index 5ce0ba86cf8b..cd1ce8719f5f 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -1,6 +1,8 @@ +from collections.abc import Sequence from typing import Any from rest_framework.exceptions import PermissionDenied +from rest_framework.exceptions import ValidationError as RestFrameworkValidationError from rest_framework.request import Request from sentry import features @@ -11,6 +13,7 @@ from sentry.incidents.endpoints.serializers.utils import get_object_id_from_fake_id from sentry.incidents.models.alert_rule import AlertRule from sentry.models.organization import Organization +from sentry.models.project import Project from sentry.workflow_engine.endpoints.utils.ids import to_valid_int_id from sentry.workflow_engine.models.alertrule_detector import AlertRuleDetector from sentry.workflow_engine.models.detector import Detector @@ -24,6 +27,8 @@ class OrganizationAlertRuleBaseEndpoint(OrganizationEndpoint): org-level permissions and team admin project-scoped permissions. """ + allow_any_team_alert_write_fallback = True + def check_can_create_alert(self, request: Request, organization: Organization) -> None: """ Determine if the requesting user has access to alert creation. If the request does not have the "alerts:write" @@ -83,6 +88,34 @@ def convert_args( class OrganizationAlertRuleEndpoint(OrganizationEndpoint): permission_classes = (OrganizationAlertRulePermission,) + def get_alert_mutation_projects( + self, request: Request, organization: Organization + ) -> Sequence[Project] | None: + if request.method not in {"PUT", "DELETE"}: + return None + + raw_alert_rule_id = self.kwargs.get("alert_rule_id") + if raw_alert_rule_id is None: + return None + + try: + validated_alert_rule_id = to_valid_int_id("alert_rule_id", raw_alert_rule_id) + except RestFrameworkValidationError: + return None + + try: + alert_rule = AlertRule.objects.prefetch_related("projects").get( + organization=organization, id=validated_alert_rule_id + ) + return [alert_rule.projects.get()] + except ( + AlertRule.DoesNotExist, + AlertRule.MultipleObjectsReturned, + Project.DoesNotExist, + Project.MultipleObjectsReturned, + ): + return None + def convert_args( self, request: Request, alert_rule_id: int, *args: Any, **kwargs: Any ) -> tuple[tuple[Any, ...], dict[str, Any]]: @@ -165,6 +198,54 @@ class WorkflowEngineOrganizationAlertRuleEndpoint(OrganizationAlertRuleEndpoint) # with the broad workflow-engine-rule-serializers flag. workflow_engine_method_flags: dict[str, str] = {} + def get_alert_mutation_projects( + self, request: Request, organization: Organization + ) -> Sequence[Project] | None: + projects = super().get_alert_mutation_projects(request, organization) + if projects is not None: + return projects + + if request.method not in {"PUT", "DELETE"}: + return None + + raw_alert_rule_id = self.kwargs.get("alert_rule_id") + if raw_alert_rule_id is None: + return None + + try: + validated_alert_rule_id = to_valid_int_id("alert_rule_id", raw_alert_rule_id) + except RestFrameworkValidationError: + return None + + ard = ( + AlertRuleDetector.objects.select_related("detector__project") + .filter( + alert_rule_id=validated_alert_rule_id, + detector__project__organization=organization, + ) + .first() + ) + if ard is not None: + return [ard.detector.project] + + try: + detector_id = get_object_id_from_fake_id(validated_alert_rule_id) + except ValueError: + return None + + detector = ( + Detector.objects.select_related("project") + .filter( + id=detector_id, + project__organization=organization, + ) + .first() + ) + if detector is None: + return None + + return [detector.project] + def convert_args( self, request: Request, alert_rule_id: int, *args: Any, **kwargs: Any ) -> tuple[tuple[Any, ...], dict[str, Any]]: diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 4342aa2ce324..d44162ecef95 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -13,6 +13,7 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint +from sentry.api.bases.organization import ALERT_MUTATION_SCOPES from sentry.api.fields.actor import OwnerActorField from sentry.api.serializers import serialize from sentry.api.serializers.rest_framework.project import ProjectField @@ -328,6 +329,11 @@ def wrapper( if not request.access.has_project_access(project): return Response(status=status.HTTP_403_FORBIDDEN) + if request.method in {"PUT", "DELETE"} and not request.access.has_any_project_scope( + project, ALERT_MUTATION_SCOPES + ): + return Response(status=status.HTTP_403_FORBIDDEN) + return func(self, request, organization, alert_rule) return wrapper diff --git a/src/sentry/monitors/endpoints/organization_monitor_index.py b/src/sentry/monitors/endpoints/organization_monitor_index.py index 5e2240819999..4234c61d2a14 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_index.py +++ b/src/sentry/monitors/endpoints/organization_monitor_index.py @@ -18,7 +18,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases import NoProjects -from sentry.api.bases.organization import OrganizationAlertRulePermission +from sentry.api.bases.organization import ALERT_MUTATION_SCOPES, OrganizationAlertRulePermission from sentry.api.helpers.teams import get_teams from sentry.api.paginator import OffsetPaginator from sentry.api.serializers import serialize @@ -331,6 +331,11 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: monitor_guids = result.pop("ids", []) monitors = list(Monitor.objects.filter(guid__in=monitor_guids, project_id__in=project_ids)) + if not all( + request.access.has_any_project_scope(monitor.project, ALERT_MUTATION_SCOPES) + for monitor in monitors + ): + return self.respond(status=403) status = result.get("status") # If enabling monitors, ensure we can assign all before moving forward diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 264c67dd3768..0d0f592fcf4b 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -23,7 +23,7 @@ class ReplayDetailsPermission(ProjectPermission): "GET": ["project:read", "project:write", "project:admin"], "POST": ["project:write", "project:admin"], "PUT": ["project:write", "project:admin"], - "DELETE": ["event:write", "event:admin"], + "DELETE": ["event:admin"], } diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 260e3e0bf64b..e4974d4d04b9 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -6,7 +6,7 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import OrganizationAlertRulePermission +from sentry.api.bases.organization import ALERT_MUTATION_SCOPES, OrganizationAlertRulePermission from sentry.api.bases.organization_events import OrganizationEventsEndpointBase from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers.base import serialize @@ -33,10 +33,22 @@ class OrganizationEventsAnomaliesEndpoint(OrganizationEventsEndpointBase): publish_status = { "POST": ApiPublishStatus.EXPERIMENTAL, } + allow_any_team_alert_write_fallback = True # This POST previews anomaly-detection config used while authoring metric # alerts/detectors, so it intentionally follows alert-write permissions. permission_classes = (OrganizationAlertRulePermission,) + def get_alert_mutation_projects(self, request: Request, organization: Organization): + raw_project_id = request.data.get("project_id") + if raw_project_id is None: + return None + + try: + project_id = to_valid_int_id("project_id", raw_project_id) + return self.get_projects(request, organization, project_ids={project_id}) + except Exception: + return None + @extend_schema( operation_id="Identify anomalies in historical data", parameters=[GlobalParams.ORG_ID_OR_SLUG], @@ -94,6 +106,11 @@ def post(self, request: Request, organization: Organization) -> Response: projects = self.get_projects(request, organization, project_ids={project_id}) if not projects: return Response({"detail": "Invalid project"}, status=400) + if not all( + request.access.has_any_project_scope(project, ALERT_MUTATION_SCOPES) + for project in projects + ): + return Response(status=403) anomalies = get_historical_anomaly_data_from_seer_preview( current_data=current_data, diff --git a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py index 6cd7e380d779..10eb603f03d0 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py +++ b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py @@ -30,6 +30,7 @@ @cell_silo_endpoint class OrganizationUptimeAlertPreviewCheckEndpoint(OrganizationEndpoint): owner = ApiOwner.CRONS + allow_any_team_alert_write_fallback = True # This POST previews monitor creation and validation, so it intentionally # uses the same permission surface as creating the alert itself. permission_classes = (OrganizationAlertRulePermission,) diff --git a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py index 546bb259b956..c47879dd8f32 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py +++ b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py @@ -50,6 +50,7 @@ class OrganizationUptimeAssertionSuggestionsEndpoint(OrganizationEndpoint): """ owner = ApiOwner.CRONS + allow_any_team_alert_write_fallback = True # This POST is part of the uptime monitor authoring flow, so it should # track the same alert-write permission as the monitor it helps create. permission_classes = (OrganizationAlertRulePermission,) diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 81670ef4761a..b6b6f09762d1 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -123,6 +123,34 @@ def get_detector_validator( @cell_silo_endpoint @extend_schema(tags=["Monitors"]) class OrganizationDetectorDetailsEndpoint(OrganizationEndpoint): + def get_alert_mutation_projects( + self, request: Request, organization: Organization + ) -> list[Project] | None: + if request.method not in {"PUT", "DELETE"}: + return None + + raw_detector_id = self.kwargs.get("detector_id") + if raw_detector_id is None: + return None + + try: + validated_detector_id = to_valid_int_id("detector_id", raw_detector_id) + except ValidationError: + return None + + detector = ( + Detector.objects.select_related("project") + .filter( + id=validated_detector_id, + project__organization_id=organization.id, + ) + .first() + ) + if detector is None: + return None + + return [detector.project] + def convert_args( self, request: Request, detector_id: str, *args: Any, **kwargs: Any ) -> tuple[tuple[Any, ...], dict[str, Organization | Detector]]: diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_index.py b/src/sentry/workflow_engine/endpoints/organization_detector_index.py index 86de5041f125..5f485dbf45e0 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_index.py @@ -153,6 +153,7 @@ class OrganizationDetectorIndexEndpoint(OrganizationEndpoint): "DELETE": ApiPublishStatus.PUBLIC, } owner = ApiOwner.ISSUES + allow_any_team_alert_write_fallback = True permission_classes = (OrganizationDetectorPermission,) diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index d6402bb65ab5..0f7c3db1eb27 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -88,9 +88,9 @@ class AlertRuleDetailsBase(AlertRuleBase): endpoint = "sentry-api-0-organization-alert-rule-details" - def _create_token(self, scope: str) -> ApiToken: + def _create_token(self, scope: str, user=None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): - return ApiToken.objects.create(user=self.user, scope_list=[scope]) + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) @patch( "sentry.seer.anomaly_detection.store_data.seer_anomaly_detection_connection_pool.urlopen" @@ -1086,6 +1086,35 @@ def test_update_allows_alerts_write_scope_for_tokens(self) -> None: self.alert_rule.refresh_from_db() assert self.alert_rule.name == self.valid_params["name"] + def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_alert_rule = self.new_alert_rule( + data={**deepcopy(self.alert_rule_dict), "projects": [other_project.slug]} + ) + + token = self._create_token("alerts:write", user=team_admin_user) + + with self.feature("organizations:incidents"): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, other_alert_rule.id]), + data={**self.valid_params, "projects": [other_project.slug]}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + def test_simple(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] @@ -2713,6 +2742,33 @@ def test_error_response_from_sentry_app(self) -> None: class AlertRuleDetailsDeleteEndpointTest(AlertRuleDetailsBase): method = "delete" + def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_alert_rule = self.new_alert_rule( + data={**deepcopy(self.alert_rule_dict), "projects": [other_project.slug]} + ) + + token = self._create_token("alerts:write", user=team_admin_user) + + with self.feature("organizations:incidents"): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, other_alert_rule.id]), + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + def test_simple(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index 294254edd864..d7c160e9b563 100644 --- a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py @@ -888,6 +888,10 @@ def setUp(self) -> None: super().setUp() self.login_as(self.user) + def _create_token(self, scope: str, user=None) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) + def test_valid_ids(self) -> None: monitor_one = self._create_monitor(slug="monitor_one") self._create_monitor(slug="monitor_two") @@ -981,6 +985,33 @@ def test_bulk_disable_enable(self) -> None: assert monitor_one.status == ObjectStatus.ACTIVE assert monitor_two.status == ObjectStatus.ACTIVE + def test_bulk_edit_denies_alerts_write_scope_for_other_team_projects(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_monitor = self._create_monitor(project=other_project, slug="other-monitor") + token = self._create_token("alerts:write", user=team_admin_user) + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug]), + data={"ids": [other_monitor.guid], "status": "disabled"}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + other_monitor.refresh_from_db() + assert other_monitor.status == ObjectStatus.ACTIVE + @patch("sentry.quotas.backend.check_assign_seats") def test_enable_no_quota(self, check_assign_seats: MagicMock) -> None: monitor_one = self._create_monitor(slug="monitor_one", status=ObjectStatus.DISABLED) diff --git a/tests/sentry/replays/endpoints/test_project_replay_details.py b/tests/sentry/replays/endpoints/test_project_replay_details.py index 3d93d73bab09..f68bd4622047 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_details.py +++ b/tests/sentry/replays/endpoints/test_project_replay_details.py @@ -195,7 +195,7 @@ def test_delete_replay_from_filestore(self) -> None: except File.DoesNotExist: pass - def test_delete_requires_event_write_scope_for_api_tokens(self) -> None: + def test_delete_requires_event_admin_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) @@ -217,7 +217,7 @@ def test_delete_denies_project_write_scope_for_api_tokens(self) -> None: assert response.status_code == 403 - def test_delete_allows_event_write_scope_for_api_tokens(self) -> None: + def test_delete_denies_event_write_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["event:write"]) @@ -227,8 +227,34 @@ def test_delete_allows_event_write_scope_for_api_tokens(self) -> None: self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" ) + assert response.status_code == 403 + + def test_delete_allows_event_admin_scope_for_api_tokens(self) -> None: + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["event:admin"]) + + with self.feature(REPLAYS_FEATURES): + with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) + assert response.status_code == 204 + def test_delete_requires_event_admin_scope_for_members_without_event_admin(self) -> None: + self.organization.update_option("sentry:events_member_admin", False) + + member_user = self.create_user(is_superuser=False) + self.create_member( + user=member_user, organization=self.organization, role="member", teams=[] + ) + self.login_as(user=member_user) + + with self.feature(REPLAYS_FEATURES): + response = self.client.delete(self.url) + + assert response.status_code == 403 + def test_delete_replay_from_clickhouse_data(self) -> None: """Test deleting files uploaded through the direct storage interface.""" kept_replay_id = uuid4().hex diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index b0af2ee7ca4c..b3dd7a8312d0 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -48,9 +48,9 @@ class OrganizationEventsAnomaliesEndpointTest(APITestCase): current_timestamp_1 = one_week_ago.timestamp() current_timestamp_2 = (one_week_ago + timedelta(minutes=10)).timestamp() - def _create_token(self, scope: str) -> ApiToken: + def _create_token(self, scope: str, user=None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): - return ApiToken.objects.create(user=self.user, scope_list=[scope]) + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) def get_test_data(self, project_id: int) -> dict: return { @@ -223,6 +223,35 @@ def test_org_read_scope_cannot_post(self) -> None: assert response.status_code == 403 + @with_feature("organizations:anomaly-detection-alerts") + @with_feature("organizations:incidents") + def test_alerts_write_scope_denies_other_team_projects(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + token = self._create_token("alerts:write", user=team_admin_user) + data = self.get_test_data(other_project.id) + url = reverse(self.endpoint, args=[self.organization.slug]) + + with outbox_runner(): + response = self.client.post( + url, + data=orjson.dumps(data), + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") @patch( diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 437bde515940..68831837e43b 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -270,9 +270,9 @@ def setUp(self) -> None: } assert SnubaQuery.objects.get(id=self.snuba_query.id) - def _create_token(self, scope: str) -> ApiToken: + def _create_token(self, scope: str, user=None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): - return ApiToken.objects.create(user=self.user, scope_list=[scope]) + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) def assert_detector_updated(self, detector: Detector) -> None: assert detector.name == "Updated Detector" @@ -583,6 +583,32 @@ def test_update_allows_alerts_write_scope_for_tokens(self) -> None: self.detector.refresh_from_db() assert self.detector.enabled is False + def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_detector = self.create_detector(project=other_project, name="Other Detector") + + token = self._create_token("alerts:write", user=team_admin_user) + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, other_detector.id]), + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + def test_enable_detector(self) -> None: self.detector.update(enabled=False) self.detector.update(status=ObjectStatus.DISABLED) @@ -1111,6 +1137,30 @@ def test_update_generic_metrics_dataset_to_transactions(self) -> None: class OrganizationDetectorDetailsDeleteTest(OrganizationDetectorDetailsBaseTest): method = "DELETE" + def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_detector = self.create_detector(project=other_project, name="Other Detector") + + token = self._create_token("alerts:write", user=team_admin_user) + + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, other_detector.id]), + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + @mock.patch( "sentry.workflow_engine.endpoints.organization_detector_details.schedule_update_project_config" ) From 47cec09764abe839f1bf9782901da7d5b5612753 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 18:14:57 -0700 Subject: [PATCH 13/25] fix(api): Unwrap org context in alert mutations Normalize RpcUserOrganizationContext to the underlying organization before resolving target projects for alert-rule and detector mutation permissions. Without that, the new project-scoped alerts:write path breaks during organization object-permission checks because those hooks run before convert_args swaps in the concrete organization. Add team-admin session regressions that force the project-scoped alerts:write authorization path for alert rule and detector updates and deletes, including the workflow-engine fake-detector alert-rule route. Co-Authored-By: Codex --- src/sentry/incidents/endpoints/bases.py | 26 ++++++++--- .../organization_detector_details.py | 18 +++++++- .../test_organization_alert_rule_details.py | 43 +++++++++++++++++++ .../test_organization_detector_details.py | 40 +++++++++++++++++ 4 files changed, 120 insertions(+), 7 deletions(-) diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index cd1ce8719f5f..dd6021d7a786 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -14,11 +14,21 @@ from sentry.incidents.models.alert_rule import AlertRule from sentry.models.organization import Organization from sentry.models.project import Project +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.workflow_engine.endpoints.utils.ids import to_valid_int_id from sentry.workflow_engine.models.alertrule_detector import AlertRuleDetector from sentry.workflow_engine.models.detector import Detector +def _get_organization_id( + organization: Organization | RpcOrganization | RpcUserOrganizationContext, +) -> int: + if isinstance(organization, RpcUserOrganizationContext): + return organization.organization.id + + return organization.id + + class OrganizationAlertRuleBaseEndpoint(OrganizationEndpoint): """ Base endpoint for organization-scoped alert rule creation. @@ -89,7 +99,9 @@ class OrganizationAlertRuleEndpoint(OrganizationEndpoint): permission_classes = (OrganizationAlertRulePermission,) def get_alert_mutation_projects( - self, request: Request, organization: Organization + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, ) -> Sequence[Project] | None: if request.method not in {"PUT", "DELETE"}: return None @@ -104,8 +116,9 @@ def get_alert_mutation_projects( return None try: + organization_id = _get_organization_id(organization) alert_rule = AlertRule.objects.prefetch_related("projects").get( - organization=organization, id=validated_alert_rule_id + organization_id=organization_id, id=validated_alert_rule_id ) return [alert_rule.projects.get()] except ( @@ -199,7 +212,9 @@ class WorkflowEngineOrganizationAlertRuleEndpoint(OrganizationAlertRuleEndpoint) workflow_engine_method_flags: dict[str, str] = {} def get_alert_mutation_projects( - self, request: Request, organization: Organization + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, ) -> Sequence[Project] | None: projects = super().get_alert_mutation_projects(request, organization) if projects is not None: @@ -217,11 +232,12 @@ def get_alert_mutation_projects( except RestFrameworkValidationError: return None + organization_id = _get_organization_id(organization) ard = ( AlertRuleDetector.objects.select_related("detector__project") .filter( alert_rule_id=validated_alert_rule_id, - detector__project__organization=organization, + detector__project__organization_id=organization_id, ) .first() ) @@ -237,7 +253,7 @@ def get_alert_mutation_projects( Detector.objects.select_related("project") .filter( id=detector_id, - project__organization=organization, + project__organization_id=organization_id, ) .first() ) diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index b6b6f09762d1..11ce5bd1e411 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -32,6 +32,8 @@ from sentry.models.organization import Organization from sentry.models.project import Project from sentry.snuba.models import SnubaQuery +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext +from sentry.snuba.models import SnubaQuery from sentry.utils.audit import create_audit_entry from sentry.workflow_engine.endpoints.serializers.detector_serializer import DetectorSerializer from sentry.workflow_engine.endpoints.utils.ids import to_valid_int_id @@ -67,6 +69,15 @@ def _check_metric_detector_allowed(detector: Detector, organization: Organizatio raise ResourceDoesNotExist +def _get_organization_id( + organization: Organization | RpcOrganization | RpcUserOrganizationContext, +) -> int: + if isinstance(organization, RpcUserOrganizationContext): + return organization.organization.id + + return organization.id + + def remove_detector(request: Request, organization: Organization, detector: Detector) -> Response: """ Delete a given detector. This method is used by the OrganizationAlertRuleDetailsEndpoint DELETE method @@ -124,7 +135,9 @@ def get_detector_validator( @extend_schema(tags=["Monitors"]) class OrganizationDetectorDetailsEndpoint(OrganizationEndpoint): def get_alert_mutation_projects( - self, request: Request, organization: Organization + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, ) -> list[Project] | None: if request.method not in {"PUT", "DELETE"}: return None @@ -138,11 +151,12 @@ def get_alert_mutation_projects( except ValidationError: return None + organization_id = _get_organization_id(organization) detector = ( Detector.objects.select_related("project") .filter( id=validated_detector_id, - project__organization_id=organization.id, + project__organization_id=organization_id, ) .first() ) diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index 0f7c3db1eb27..f84173dc70b1 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1052,6 +1052,28 @@ def test_get_shows_count_when_stored_as_upsampled_count( class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase): method = "put" + def test_team_admin_can_update_with_project_scoped_alerts_write(self) -> None: + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + with self.feature("organizations:incidents"): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]), + data=self.valid_params, + format="json", + ) + + assert response.status_code == 200 + self.alert_rule.refresh_from_db() + assert self.alert_rule.name == self.valid_params["name"] + def test_update_requires_alerts_write_scope_for_tokens(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] @@ -2742,6 +2764,27 @@ def test_error_response_from_sentry_app(self) -> None: class AlertRuleDetailsDeleteEndpointTest(AlertRuleDetailsBase): method = "delete" + def test_team_admin_can_delete_workflow_engine_detector_with_project_scoped_alerts_write( + self, + ) -> None: + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + detector = self.create_detector(project=self.project, type=MetricIssue.slug) + data_source = self.create_data_source() + self.create_data_source_detector(data_source, detector) + fake_detector_id = get_fake_id_from_object_id(detector.id) + + with self.feature({"organizations:workflow-engine-rule-serializers": True}): + self.get_success_response(self.organization.slug, fake_detector_id, status_code=204) + def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: team_admin_user = self.create_user() self.create_member( diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 68831837e43b..73a2a9beab78 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -568,6 +568,28 @@ def test_update_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 + def test_team_admin_can_update_with_project_scoped_alerts_write(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + with self.tasks(): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + data={"enabled": False}, + format="json", + ) + + assert response.status_code == 200 + self.detector.refresh_from_db() + assert self.detector.enabled is False + def test_update_allows_alerts_write_scope_for_tokens(self) -> None: token = self._create_token("alerts:write") @@ -1137,6 +1159,24 @@ def test_update_generic_metrics_dataset_to_transactions(self) -> None: class OrganizationDetectorDetailsDeleteTest(OrganizationDetectorDetailsBaseTest): method = "DELETE" + def test_team_admin_can_delete_with_project_scoped_alerts_write(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + with outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]) + ) + + assert response.status_code == 204 + def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: team_admin_user = self.create_user(is_superuser=False) self.create_member( From 7871ad440cb2fde33ed108f4019bdfa618ba8cf2 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 17:47:50 -0700 Subject: [PATCH 14/25] fix(api): annotate readonly mutations and alert compat Annotate the published mutation endpoints that intentionally accept readonly or member scopes so the permission-note audit can distinguish expected behavior from real scope gaps. Tighten the newer alert, detector, workflow, uptime, and anomaly helpers to use alerts:write, but keep temporary org:write compatibility on the deprecated metric alert and cron monitor mutation paths. Add regression coverage for both the temporary compat and the stricter deny cases. Refs getsentry/getsentry#19897 Co-Authored-By: Codex --- src/sentry/api/bases/incident.py | 3 + src/sentry/api/bases/organization.py | 69 +++++++++++++++++-- src/sentry/api/bases/project.py | 3 + ...anization_onboarding_continuation_email.py | 3 + .../organization_onboarding_tasks.py | 3 + .../endpoints/organization_recent_searches.py | 3 + .../endpoints/project_repo_path_parsing.py | 3 + .../organization_projects_experiment.py | 3 + src/sentry/core/endpoints/project_details.py | 3 + .../endpoints/organization_dashboards.py | 5 ++ .../organization_dashboards_starred.py | 3 + src/sentry/discover/endpoints/bases.py | 5 ++ .../endpoints/discover_key_transactions.py | 4 ++ src/sentry/explore/endpoints/bases.py | 5 ++ .../endpoints/explore_saved_query_starred.py | 3 + .../explore_saved_query_starred_order.py | 3 + src/sentry/incidents/endpoints/bases.py | 14 ++-- .../organization_alert_rule_details.py | 25 +++++-- .../organization_alert_rule_index.py | 3 + .../incidents/serializers/alert_rule.py | 4 +- .../insights/endpoints/starred_segments.py | 4 ++ .../endpoints/bases/group_search_view.py | 4 ++ ...ation_group_search_view_details_starred.py | 3 + ...ization_group_search_view_starred_order.py | 3 + .../organization_group_search_view_visit.py | 3 + .../organization_group_search_views.py | 3 + .../endpoints/organization_monitor_index.py | 10 ++- src/sentry/monitors/validators.py | 4 +- .../organization_events_anomalies.py | 3 +- .../endpoints/organization_workflow_index.py | 6 +- .../endpoints/validators/utils.py | 13 ++-- .../test_organization_alert_rule_details.py | 47 +++++++++++++ .../test_organization_alert_rule_index.py | 20 ++++++ .../test_organization_monitor_index.py | 37 +++++++++- .../test_organization_events_anomalies.py | 18 +++++ .../test_organization_uptime_alert_preview.py | 24 +++++++ ...ganization_uptime_assertion_suggestions.py | 12 ++++ .../test_organization_detector_details.py | 30 ++++++-- .../test_organization_workflow_index.py | 25 +++++++ 39 files changed, 394 insertions(+), 42 deletions(-) diff --git a/src/sentry/api/bases/incident.py b/src/sentry/api/bases/incident.py index 1a0bd6e04a86..e510c1b441f3 100644 --- a/src/sentry/api/bases/incident.py +++ b/src/sentry/api/bases/incident.py @@ -24,6 +24,9 @@ class IncidentPermission(OrganizationPermission): "PUT": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], "DELETE": ["org:write", "org:admin", "project:read", "project:write", "project:admin"], } + readonly_mutation_scope_exceptions = { + "PUT": "Incident updates still allow project-scoped members with access to the incident's projects.", + } class IncidentEndpoint(OrganizationEndpoint): diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 34876ebcc41e..182baa7b6aaf 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -163,6 +163,10 @@ class OrganizationIntegrationsLoosePermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin", "org:integrations"], "DELETE": ["org:admin", "org:integrations"], } + readonly_mutation_scope_exceptions = { + "POST": "Legacy integration setup helpers still allow org:read and enforce narrower checks in the view.", + "PUT": "Legacy integration edit helpers still allow org:read and enforce narrower checks in the view.", + } class OrganizationCodeMappingsBulkPermission(OrganizationPermission): @@ -170,6 +174,9 @@ class OrganizationCodeMappingsBulkPermission(OrganizationPermission): "GET": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], "POST": ["org:read", "org:write", "org:admin", "org:integrations", "org:ci"], } + readonly_mutation_scope_exceptions = { + "POST": "Bulk code mapping suggestions remain available to org members with repository access.", + } class OrganizationAdminPermission(OrganizationPermission): @@ -199,6 +206,10 @@ class OrganizationPinnedSearchPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "PUT": "Pinned search changes only update the requesting member's personal pinned state.", + "DELETE": "Pinned search removal only updates the requesting member's personal pinned state.", + } class OrganizationSearchPermission(OrganizationPermission): @@ -208,6 +219,11 @@ class OrganizationSearchPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Members may manage personal saved searches without org:write.", + "PUT": "Members may edit personal saved searches without org:write.", + "DELETE": "Members may delete personal saved searches without org:write.", + } class OrganizationDataExportPermission(OrganizationPermission): @@ -225,7 +241,8 @@ def _has_any_team_scope(request: Request, scope: str) -> bool: return any(request.access.has_team_scope(team, scope) for team in teams) -ALERT_MUTATION_SCOPES = frozenset({"org:write", "alerts:write"}) +# Project-scoped alert authoring should rely on the alert-specific write scope. +ALERT_MUTATION_SCOPES = frozenset({"alerts:write"}) def _has_project_alert_write_access(request: Request, projects: Sequence[Project]) -> bool: @@ -250,14 +267,39 @@ def _has_view_project_scoped_alert_write( return _has_project_alert_write_access(request, projects) +def get_legacy_alert_mutation_scopes(view: APIView, method: str | None) -> tuple[str, ...]: + if method is None: + return () + + legacy_scope_map = getattr(view, "legacy_alert_mutation_scope_map", None) + if legacy_scope_map is None: + return () + + scopes = legacy_scope_map.get(method, ()) + return tuple(scopes) + + class OrganizationAlertRulePermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - "POST": ["org:write", "org:admin", "alerts:write"], - "PUT": ["org:write", "org:admin", "alerts:write"], - "DELETE": ["org:write", "org:admin", "alerts:write"], + "POST": ["alerts:write"], + "PUT": ["alerts:write"], + "DELETE": ["alerts:write"], } + def has_permission(self, request: Request, view: APIView) -> bool: + if super().has_permission(request, view): + return True + + if not request.auth: + return False + + current_scopes = request.auth.get_scopes() + return any( + scope in current_scopes + for scope in get_legacy_alert_mutation_scopes(view, request.method) + ) + def has_object_permission( self, request: Request, @@ -270,6 +312,12 @@ def has_object_permission( if request.method not in {"POST", "PUT", "DELETE"}: return False + if any( + request.access.has_scope(scope) + for scope in get_legacy_alert_mutation_scopes(view, request.method) + ): + return True + project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) if project_scoped_access is not None: return project_scoped_access @@ -282,9 +330,9 @@ def has_object_permission( class OrganizationDetectorPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - "POST": ["org:write", "org:admin", "alerts:write"], - "PUT": ["org:write", "org:admin", "alerts:write"], - "DELETE": ["org:write", "org:admin", "alerts:write"], + "POST": ["alerts:write"], + "PUT": ["alerts:write"], + "DELETE": ["alerts:write"], } def has_object_permission( @@ -315,6 +363,10 @@ class OrgAuthTokenPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Organization auth token creation is a legacy org:read mutation.", + "PUT": "Organization auth token updates are a legacy org:read mutation.", + } class OrganizationFlagWebHookSigningSecretPermission(OrganizationPermission): @@ -323,6 +375,9 @@ class OrganizationFlagWebHookSigningSecretPermission(OrganizationPermission): "POST": ["org:read", "org:write", "org:admin"], "DELETE": ["org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Webhook signing secret writes allow org:read but the view restricts updates to creators or org writers.", + } class ControlSiloOrganizationEndpoint(Endpoint): diff --git a/src/sentry/api/bases/project.py b/src/sentry/api/bases/project.py index 58d5133030fe..3c5f8aa57590 100644 --- a/src/sentry/api/bases/project.py +++ b/src/sentry/api/bases/project.py @@ -133,6 +133,9 @@ class ProjectOwnershipPermission(ProjectPermission): "PUT": ["project:read", "project:write", "project:admin"], "DELETE": ["project:admin"], } + readonly_mutation_scope_exceptions = { + "PUT": "Project ownership rules allow project:read for legacy ownership config edits.", + } class ProjectEndpoint(Endpoint): diff --git a/src/sentry/api/endpoints/organization_onboarding_continuation_email.py b/src/sentry/api/endpoints/organization_onboarding_continuation_email.py index 8351444da7ce..1740ff04a1f1 100644 --- a/src/sentry/api/endpoints/organization_onboarding_continuation_email.py +++ b/src/sentry/api/endpoints/organization_onboarding_continuation_email.py @@ -43,6 +43,9 @@ def get_request_builder_args(user: User, organization: Organization, platforms: class OnboardingContinuationPermission(OrganizationPermission): scope_map = {"POST": ["org:read", "org:write", "org:admin"]} + readonly_mutation_scope_exceptions = { + "POST": "Members may trigger their own onboarding continuation email.", + } @cell_silo_endpoint diff --git a/src/sentry/api/endpoints/organization_onboarding_tasks.py b/src/sentry/api/endpoints/organization_onboarding_tasks.py index c1aae4b2f25a..1c7d232a9505 100644 --- a/src/sentry/api/endpoints/organization_onboarding_tasks.py +++ b/src/sentry/api/endpoints/organization_onboarding_tasks.py @@ -15,6 +15,9 @@ class OnboardingTaskPermission(OrganizationPermission): scope_map = {"POST": ["org:read"], "GET": ["org:read"]} + readonly_mutation_scope_exceptions = { + "POST": "Members may update their own onboarding task state.", + } @cell_silo_endpoint diff --git a/src/sentry/api/endpoints/organization_recent_searches.py b/src/sentry/api/endpoints/organization_recent_searches.py index 4b6c1398ccfc..64b23a1dd1d3 100644 --- a/src/sentry/api/endpoints/organization_recent_searches.py +++ b/src/sentry/api/endpoints/organization_recent_searches.py @@ -22,6 +22,9 @@ class OrganizationRecentSearchPermission(OrganizationPermission): "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Recent search writes only affect the requesting member's history.", + } @cell_silo_endpoint diff --git a/src/sentry/api/endpoints/project_repo_path_parsing.py b/src/sentry/api/endpoints/project_repo_path_parsing.py index c530e5d1105f..3a5dc3df9349 100644 --- a/src/sentry/api/endpoints/project_repo_path_parsing.py +++ b/src/sentry/api/endpoints/project_repo_path_parsing.py @@ -98,6 +98,9 @@ class ProjectRepoPathParsingEndpointLoosePermission(ProjectPermission): scope_map = { "POST": ["org:read", "project:write", "project:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Repo path parsing validates a suggested mapping and does not persist project state.", + } @cell_silo_endpoint diff --git a/src/sentry/core/endpoints/organization_projects_experiment.py b/src/sentry/core/endpoints/organization_projects_experiment.py index 44db7c491b67..34f5354a6d1a 100644 --- a/src/sentry/core/endpoints/organization_projects_experiment.py +++ b/src/sentry/core/endpoints/organization_projects_experiment.py @@ -55,6 +55,9 @@ class OrgProjectPermission(OrganizationPermission): scope_map = { "POST": ["project:read", "project:write", "project:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Experimental team-plus-project creation remains available to members when org settings allow it.", + } class AuditData(TypedDict): diff --git a/src/sentry/core/endpoints/project_details.py b/src/sentry/core/endpoints/project_details.py index 24cc8fdbd1be..4f082db99ee1 100644 --- a/src/sentry/core/endpoints/project_details.py +++ b/src/sentry/core/endpoints/project_details.py @@ -507,6 +507,9 @@ class RelaxedProjectPermission(ProjectPermission): "PUT": ["project:read", "project:write", "project:admin"], "DELETE": ["project:admin"], } + readonly_mutation_scope_exceptions = { + "PUT": "Project detail updates still allow project:read for legacy field-level permission checks.", + } class RelaxedProjectAndStaffPermission(StaffPermissionMixin, RelaxedProjectPermission): diff --git a/src/sentry/dashboards/endpoints/organization_dashboards.py b/src/sentry/dashboards/endpoints/organization_dashboards.py index 358fb6cef5fc..ca5b6c2a0e71 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboards.py +++ b/src/sentry/dashboards/endpoints/organization_dashboards.py @@ -307,6 +307,11 @@ class OrganizationDashboardsPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Dashboard creation remains available to org members.", + "PUT": "Dashboard edits remain available to org members with object-level edit access.", + "DELETE": "Dashboard deletion remains available to org members with object-level edit access.", + } def has_object_permission( self, diff --git a/src/sentry/dashboards/endpoints/organization_dashboards_starred.py b/src/sentry/dashboards/endpoints/organization_dashboards_starred.py index bbd1a2f8cafc..d390cb84ba9a 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboards_starred.py +++ b/src/sentry/dashboards/endpoints/organization_dashboards_starred.py @@ -23,6 +23,9 @@ class MemberPermission(OrganizationPermission): "GET": ["member:read", "member:write"], "PUT": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "PUT": "Starred dashboard ordering only updates the requesting member's preferences.", + } @cell_silo_endpoint diff --git a/src/sentry/discover/endpoints/bases.py b/src/sentry/discover/endpoints/bases.py index 9d067e9e0ee8..953bc7fdbe34 100644 --- a/src/sentry/discover/endpoints/bases.py +++ b/src/sentry/discover/endpoints/bases.py @@ -11,6 +11,11 @@ class DiscoverSavedQueryPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Discover saved query mutations remain available for personal or otherwise permitted queries.", + "PUT": "Discover saved query mutations remain available for personal or otherwise permitted queries.", + "DELETE": "Discover saved query mutations remain available for personal or otherwise permitted queries.", + } def has_object_permission(self, request, view, obj): if isinstance(obj, Organization): diff --git a/src/sentry/discover/endpoints/discover_key_transactions.py b/src/sentry/discover/endpoints/discover_key_transactions.py index 4c630e0f3d94..9b0e5f5046f4 100644 --- a/src/sentry/discover/endpoints/discover_key_transactions.py +++ b/src/sentry/discover/endpoints/discover_key_transactions.py @@ -30,6 +30,10 @@ class KeyTransactionPermission(OrganizationPermission): "PUT": ["org:read"], "DELETE": ["org:read"], } + readonly_mutation_scope_exceptions = { + "POST": "Key transaction writes are additionally constrained by project and team membership checks.", + "DELETE": "Key transaction deletes are additionally constrained by project and team membership checks.", + } @cell_silo_endpoint diff --git a/src/sentry/explore/endpoints/bases.py b/src/sentry/explore/endpoints/bases.py index 8512c577eb49..36b95f754126 100644 --- a/src/sentry/explore/endpoints/bases.py +++ b/src/sentry/explore/endpoints/bases.py @@ -11,6 +11,11 @@ class ExploreSavedQueryPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "POST": "Explore saved query mutations remain available for personal or otherwise permitted queries.", + "PUT": "Explore saved query mutations remain available for personal or otherwise permitted queries.", + "DELETE": "Explore saved query mutations remain available for personal or otherwise permitted queries.", + } def has_object_permission(self, request, view, obj): if isinstance(obj, Organization): diff --git a/src/sentry/explore/endpoints/explore_saved_query_starred.py b/src/sentry/explore/endpoints/explore_saved_query_starred.py index 21050bb57d3a..c0c9a284c9c8 100644 --- a/src/sentry/explore/endpoints/explore_saved_query_starred.py +++ b/src/sentry/explore/endpoints/explore_saved_query_starred.py @@ -25,6 +25,9 @@ class MemberPermission(OrganizationPermission): scope_map = { "POST": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "POST": "Starring an Explore query only changes the requesting member's preferences.", + } @cell_silo_endpoint diff --git a/src/sentry/explore/endpoints/explore_saved_query_starred_order.py b/src/sentry/explore/endpoints/explore_saved_query_starred_order.py index 94128542620e..0ff990771f8a 100644 --- a/src/sentry/explore/endpoints/explore_saved_query_starred_order.py +++ b/src/sentry/explore/endpoints/explore_saved_query_starred_order.py @@ -17,6 +17,9 @@ class MemberPermission(OrganizationPermission): scope_map = { "PUT": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "PUT": "Explore starred query ordering only changes the requesting member's preferences.", + } class ExploreSavedQueryStarredOrderSerializer(serializers.Serializer): diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index dd6021d7a786..d9c5fce92930 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -7,7 +7,11 @@ from sentry import features from sentry.api.api_owners import ApiOwner -from sentry.api.bases.organization import OrganizationAlertRulePermission, OrganizationEndpoint +from sentry.api.bases.organization import ( + OrganizationAlertRulePermission, + OrganizationEndpoint, + get_legacy_alert_mutation_scopes, +) from sentry.api.bases.project import ProjectAlertRulePermission, ProjectEndpoint from sentry.api.exceptions import ResourceDoesNotExist from sentry.incidents.endpoints.serializers.utils import get_object_id_from_fake_id @@ -46,11 +50,9 @@ def check_can_create_alert(self, request: Request, organization: Organization) - in their request. """ - # if the requesting user has any of these org-level permissions, then they can create an alert - if ( - request.access.has_scope("alerts:write") - or request.access.has_scope("org:admin") - or request.access.has_scope("org:write") + if request.access.has_scope("alerts:write") or any( + request.access.has_scope(scope) + for scope in get_legacy_alert_mutation_scopes(self, request.method) ): return diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index d44162ecef95..4928e9060a84 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -13,7 +13,10 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import ALERT_MUTATION_SCOPES +from sentry.api.bases.organization import ( + ALERT_MUTATION_SCOPES, + get_legacy_alert_mutation_scopes, +) from sentry.api.fields.actor import OwnerActorField from sentry.api.serializers import serialize from sentry.api.serializers.rest_framework.project import ProjectField @@ -329,10 +332,16 @@ def wrapper( if not request.access.has_project_access(project): return Response(status=status.HTTP_403_FORBIDDEN) - if request.method in {"PUT", "DELETE"} and not request.access.has_any_project_scope( - project, ALERT_MUTATION_SCOPES - ): - return Response(status=status.HTTP_403_FORBIDDEN) + if request.method in {"PUT", "DELETE"}: + has_project_alert_write = request.access.has_any_project_scope( + project, ALERT_MUTATION_SCOPES + ) + has_legacy_org_write = any( + request.access.has_scope(scope) + for scope in get_legacy_alert_mutation_scopes(self, request.method) + ) + if not has_project_alert_write and not has_legacy_org_write: + return Response(status=status.HTTP_403_FORBIDDEN) return func(self, request, organization, alert_rule) @@ -352,6 +361,12 @@ class OrganizationAlertRuleDetailsEndpoint(WorkflowEngineOrganizationAlertRuleEn "GET": ApiPublishStatus.PUBLIC, "PUT": ApiPublishStatus.PUBLIC, } + # TODO(api-write-scope-compat): Remove legacy org:write support once public + # metric alert clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "DELETE": ("org:write",), + "PUT": ("org:write",), + } @extend_schema( operation_id="(DEPRECATED) Retrieve a Metric Alert Rule for an Organization", diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index 160ea0fe0824..f0c6c2f82462 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -914,6 +914,9 @@ class OrganizationAlertRuleIndexEndpoint(OrganizationAlertRuleBaseEndpoint, Aler "GET": ApiPublishStatus.PUBLIC, "POST": ApiPublishStatus.PUBLIC, } + # TODO(api-write-scope-compat): Remove legacy org:write support once public + # metric alert clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = {"POST": ("org:write",)} permission_classes = (OrganizationAlertRulePermission,) workflow_engine_method_flags = { "GET": "organizations:workflow-engine-orgalertruleindex-get", diff --git a/src/sentry/incidents/serializers/alert_rule.py b/src/sentry/incidents/serializers/alert_rule.py index 7c507a10c048..7b88de28c4ca 100644 --- a/src/sentry/incidents/serializers/alert_rule.py +++ b/src/sentry/incidents/serializers/alert_rule.py @@ -51,7 +51,9 @@ logger = logging.getLogger(__name__) -ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") +# TODO(api-write-scope-compat): Remove legacy org:write support once public +# metric alert clients have migrated to alerts:write. +ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "alerts:write") class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRule]): diff --git a/src/sentry/insights/endpoints/starred_segments.py b/src/sentry/insights/endpoints/starred_segments.py index b7c068ee007d..92b7f9cc760f 100644 --- a/src/sentry/insights/endpoints/starred_segments.py +++ b/src/sentry/insights/endpoints/starred_segments.py @@ -23,6 +23,10 @@ class MemberPermission(OrganizationPermission): "POST": ["member:read", "member:write"], "DELETE": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "POST": "Starring an insights segment only changes the requesting member's preferences.", + "DELETE": "Unstarring an insights segment only changes the requesting member's preferences.", + } @cell_silo_endpoint diff --git a/src/sentry/issues/endpoints/bases/group_search_view.py b/src/sentry/issues/endpoints/bases/group_search_view.py index 26c91ceb7724..c229e27a1f54 100644 --- a/src/sentry/issues/endpoints/bases/group_search_view.py +++ b/src/sentry/issues/endpoints/bases/group_search_view.py @@ -14,6 +14,10 @@ class GroupSearchViewPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "PUT": "Group search view edits remain available to creators and org managers without requiring org:write in the scope map.", + "DELETE": "Group search view deletes remain available to creators and org managers without requiring org:write in the scope map.", + } def has_object_permission(self, request: Request, view: APIView, obj: object) -> bool: if isinstance(obj, Organization): diff --git a/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py b/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py index 68d7c586ddbc..3066648bb430 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py @@ -25,6 +25,9 @@ class MemberPermission(OrganizationPermission): scope_map = { "POST": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "POST": "Starring a group search view only changes the requesting member's preferences.", + } @cell_silo_endpoint diff --git a/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py b/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py index 593cba439fbc..93bba883fd99 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py @@ -15,6 +15,9 @@ class MemberPermission(OrganizationPermission): scope_map = { "PUT": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "PUT": "Group search view starred ordering only changes the requesting member's preferences.", + } class GroupSearchViewStarredOrderSerializer(serializers.Serializer): diff --git a/src/sentry/issues/endpoints/organization_group_search_view_visit.py b/src/sentry/issues/endpoints/organization_group_search_view_visit.py index bd713fc2a0ab..8a3469682a17 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_visit.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_visit.py @@ -16,6 +16,9 @@ class MemberPermission(OrganizationPermission): scope_map = { "POST": ["member:read"], } + readonly_mutation_scope_exceptions = { + "POST": "View visit timestamps only update the requesting member's last-visited state.", + } @cell_silo_endpoint diff --git a/src/sentry/issues/endpoints/organization_group_search_views.py b/src/sentry/issues/endpoints/organization_group_search_views.py index 69ce98ce8817..bb18f4b8d9a0 100644 --- a/src/sentry/issues/endpoints/organization_group_search_views.py +++ b/src/sentry/issues/endpoints/organization_group_search_views.py @@ -28,6 +28,9 @@ class MemberPermission(OrganizationPermission): "GET": ["member:read", "member:write"], "POST": ["member:read", "member:write"], } + readonly_mutation_scope_exceptions = { + "POST": "Creating a custom group search view is allowed for organization members.", + } def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: if isinstance(obj, Organization): diff --git a/src/sentry/monitors/endpoints/organization_monitor_index.py b/src/sentry/monitors/endpoints/organization_monitor_index.py index 4234c61d2a14..8f443ec99a8e 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_index.py +++ b/src/sentry/monitors/endpoints/organization_monitor_index.py @@ -40,6 +40,7 @@ from sentry.incidents.endpoints.bases import OrganizationAlertRuleBaseEndpoint from sentry.models.environment import Environment from sentry.models.organization import Organization +from sentry.models.project import Project from sentry.monitors.models import ( DEFAULT_STATUS_ORDER, MONITOR_ENVIRONMENT_ORDERING, @@ -86,6 +87,9 @@ class OrganizationMonitorIndexEndpoint(OrganizationAlertRuleBaseEndpoint): "PUT": ApiPublishStatus.EXPERIMENTAL, } owner = ApiOwner.CRONS + # TODO(api-write-scope-compat): Remove legacy org:write support once public + # cron monitor clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = {"POST": ("org:write",)} permission_classes = (OrganizationAlertRulePermission,) @extend_schema( @@ -331,9 +335,11 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: monitor_guids = result.pop("ids", []) monitors = list(Monitor.objects.filter(guid__in=monitor_guids, project_id__in=project_ids)) + projects_by_id = Project.objects.in_bulk({monitor.project_id for monitor in monitors}) if not all( - request.access.has_any_project_scope(monitor.project, ALERT_MUTATION_SCOPES) - for monitor in monitors + project is not None + and request.access.has_any_project_scope(project, ALERT_MUTATION_SCOPES) + for project in (projects_by_id.get(monitor.project_id) for monitor in monitors) ): return self.respond(status=403) diff --git a/src/sentry/monitors/validators.py b/src/sentry/monitors/validators.py index 30bd816f97c1..50f5c785f6f5 100644 --- a/src/sentry/monitors/validators.py +++ b/src/sentry/monitors/validators.py @@ -74,7 +74,9 @@ INTERVAL_NAMES = ("year", "month", "week", "day", "hour", "minute") CRONTAB_WHITESPACE = re.compile(r"\s+") -MONITOR_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") +# TODO(api-write-scope-compat): Remove legacy org:write support once public +# cron monitor clients have migrated to alerts:write. +MONITOR_PROJECT_SCOPES = ("project:read", "org:write", "alerts:write") # XXX(dcramer): @reboot is not supported (as it cannot be) NONSTANDARD_CRONTAB_SCHEDULES = { diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index e4974d4d04b9..4cd0ae90e9a2 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -103,9 +103,8 @@ def post(self, request: Request, organization: Organization) -> Response: ) project_id = to_valid_int_id("project_id", raw_project_id) + projects = self.get_projects(request, organization, project_ids={project_id}) - if not projects: - return Response({"detail": "Invalid project"}, status=400) if not all( request.access.has_any_project_scope(project, ALERT_MUTATION_SCOPES) for project in projects diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index c819cac7532e..7e1a75953e6f 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -91,9 +91,9 @@ class OrganizationWorkflowPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - "POST": ["org:write", "org:admin", "alerts:write"], - "PUT": ["org:write", "org:admin", "alerts:write"], - "DELETE": ["org:write", "org:admin", "alerts:write"], + "POST": ["alerts:write"], + "PUT": ["alerts:write"], + "DELETE": ["alerts:write"], } diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index 1f38fbdd0559..8d163427ad0c 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -26,9 +26,10 @@ logger = logging.getLogger(__name__) -# Only those with organization write permissions can edit system-created detectors (e.g. error detectors). -SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES = {"org:write"} -USER_CREATED_DETECTOR_REQUIRED_SCOPES = {"org:write", "alerts:write"} +# Detector edits should use the alert-specific write scope regardless of how the +# detector was created. +SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES = {"alerts:write"} +USER_CREATED_DETECTOR_REQUIRED_SCOPES = {"alerts:write"} def is_system_created_detector(detector: Detector) -> bool: @@ -55,8 +56,6 @@ def can_edit_user_created_detectors(request: Request, project: Project) -> bool: def can_edit_detectors(detectors: QuerySet[Detector], request: Request) -> bool: """ Determine if the requesting user has access to edit the given detectors. - System created detectors lock edit access to org:write, while user created detectors - are more permissive. """ required_scopes = ( SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES @@ -75,9 +74,7 @@ def can_edit_detectors(detectors: QuerySet[Detector], request: Request) -> bool: def can_edit_detector(detector: Detector, request: Request) -> bool: """ - Determine if the requesting user has access to detector edit. If the request does not have the "alerts:write" - permission, then we must verify that the user is a team admin with "alerts:write" access to the project(s) - in their request. + Determine if the requesting user has alert-write access to edit the detector. """ if is_system_created_detector(detector) and not can_edit_system_created_detectors( request, detector.project diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index f84173dc70b1..4bd0542b1ce6 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1053,6 +1053,9 @@ class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase): method = "put" def test_team_admin_can_update_with_project_scoped_alerts_write(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) team_admin_user = self.create_user() self.create_member( user=team_admin_user, @@ -1090,6 +1093,26 @@ def test_update_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once + # public metric alert clients have migrated to alerts:write. + def test_update_allows_legacy_org_write_scope_for_tokens(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + token = self._create_token("org:write") + + with self.feature("organizations:incidents"): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]), + data=self.valid_params, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + self.alert_rule.refresh_from_db() + assert self.alert_rule.name == self.valid_params["name"] + def test_update_allows_alerts_write_scope_for_tokens(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] @@ -1109,6 +1132,9 @@ def test_update_allows_alerts_write_scope_for_tokens(self) -> None: assert self.alert_rule.name == self.valid_params["name"] def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) team_admin_user = self.create_user() self.create_member( user=team_admin_user, @@ -1116,6 +1142,7 @@ def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: role="member", team_roles=[(self.team, "admin")], ) + self.organization.update_option("sentry:alerts_member_write", False) other_team = self.create_team(organization=self.organization, name="other-team") other_project = self.create_project( @@ -2786,6 +2813,9 @@ def test_team_admin_can_delete_workflow_engine_detector_with_project_scoped_aler self.get_success_response(self.organization.slug, fake_detector_id, status_code=204) def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) team_admin_user = self.create_user() self.create_member( user=team_admin_user, @@ -2793,6 +2823,7 @@ def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: role="member", team_roles=[(self.team, "admin")], ) + self.organization.update_option("sentry:alerts_member_write", False) other_team = self.create_team(organization=self.organization, name="other-team") other_project = self.create_project( @@ -2812,6 +2843,22 @@ def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once + # public metric alert clients have migrated to alerts:write. + def test_delete_allows_legacy_org_write_scope_for_tokens(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + token = self._create_token("org:write") + + with self.feature("organizations:incidents"), outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]), + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 204 + def test_simple(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index 7357ad98b304..2d26f82c0629 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -461,6 +461,26 @@ def test_create_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once + # public metric alert clients have migrated to alerts:write. + def test_create_allows_legacy_org_write_scope_for_tokens(self) -> None: + team = self.create_team(organization=self.organization, members=[self.user]) + ProjectTeam.objects.create(project=self.project, team=team) + token = self._create_token("org:write") + + with ( + outbox_runner(), + self.feature(["organizations:incidents", "organizations:performance-view"]), + ): + response = self.client.post( + f"/api/0/organizations/{self.organization.slug}/alert-rules/", + data=self.alert_rule_dict, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 201 + def test_create_allows_alerts_write_scope_for_tokens(self) -> None: team = self.create_team(organization=self.organization, members=[self.user]) ProjectTeam.objects.create(project=self.project, team=team) diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index d7c160e9b563..30ea26c91a19 100644 --- a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py @@ -465,6 +465,28 @@ def test_create_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once + # public cron monitor clients have migrated to alerts:write. + def test_create_allows_legacy_org_write_scope_for_tokens(self) -> None: + token = self._create_token("org:write") + data = { + "project": self.project.slug, + "name": "My Monitor", + "type": "cron_job", + "owner": f"user:{self.user.id}", + "config": {"schedule_type": "crontab", "schedule": "@daily"}, + } + + with outbox_runner(): + response = self.client.post( + reverse(self.endpoint, args=[self.organization.slug]), + data=data, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 201 + def test_create_allows_alerts_write_scope_for_tokens(self) -> None: token = self._create_token("alerts:write") data = { @@ -993,12 +1015,25 @@ def test_bulk_edit_denies_alerts_write_scope_for_other_team_projects(self) -> No role="member", team_roles=[(self.team, "admin")], ) + self.organization.update_option("sentry:alerts_member_write", False) other_team = self.create_team(organization=self.organization, name="other-team") other_project = self.create_project( organization=self.organization, teams=[other_team], name="other-project" ) - other_monitor = self._create_monitor(project=other_project, slug="other-monitor") + other_monitor = Monitor.objects.create( + organization_id=self.organization.id, + project_id=other_project.id, + config={ + "schedule": "* * * * *", + "schedule_type": ScheduleType.CRONTAB, + "checkin_margin": None, + "max_runtime": None, + }, + owner_user_id=self.user.id, + slug="other-monitor", + name="other-monitor", + ) token = self._create_token("alerts:write", user=team_admin_user) response = self.client.put( diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index b3dd7a8312d0..8e9953bf53d5 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -223,6 +223,23 @@ def test_org_read_scope_cannot_post(self) -> None: assert response.status_code == 403 + @with_feature("organizations:anomaly-detection-alerts") + @with_feature("organizations:incidents") + def test_org_write_scope_cannot_post(self) -> None: + token = self._create_token("org:write") + data = self.get_test_data(self.project.id) + url = reverse(self.endpoint, args=[self.organization.slug]) + + with outbox_runner(): + response = self.client.post( + url, + data=orjson.dumps(data), + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") def test_alerts_write_scope_denies_other_team_projects(self) -> None: @@ -233,6 +250,7 @@ def test_alerts_write_scope_denies_other_team_projects(self) -> None: role="member", team_roles=[(self.team, "admin")], ) + self.organization.update_option("sentry:alerts_member_write", False) other_team = self.create_team(organization=self.organization, name="other-team") other_project = self.create_project( diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py index 1994c1d3cff0..12cc23a7f6c7 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py @@ -158,3 +158,27 @@ def test_org_read_scope_cannot_run_preview_check(self) -> None: ) assert response.status_code == 403 + + def test_org_write_scope_cannot_run_preview_check(self) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["org:write"]) + + url = reverse( + "sentry-api-0-organization-uptime-alert-preview-check", + kwargs={"organization_id_or_slug": self.organization.slug}, + ) + response = self.client.post( + url, + data={ + "name": "test", + "environment": "uptime-prod", + "owner": f"user:{self.user.id}", + "url": "http://sentry.io", + "timeoutMs": 1500, + "body": None, + "region": "default", + }, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py index 15af97209628..ddcd38fadd94 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py @@ -83,3 +83,15 @@ def test_org_read_scope_cannot_generate_suggestions(self) -> None: ) assert response.status_code == 403 + + def test_org_write_scope_cannot_generate_suggestions(self) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["org:write"]) + + response = self.client.post( + self.url, + data=self.payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 73a2a9beab78..6653078d6be1 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -98,6 +98,10 @@ def setUp(self) -> None: ) assert self.detector.data_sources is not None + def _create_token(self, scope: str, user=None) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) + @cell_silo_test class OrganizationDetectorDetailsGetTest(OrganizationDetectorDetailsBaseTest): @@ -270,10 +274,6 @@ def setUp(self) -> None: } assert SnubaQuery.objects.get(id=self.snuba_query.id) - def _create_token(self, scope: str, user=None) -> ApiToken: - with assume_test_silo_mode(SiloMode.CONTROL): - return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) - def assert_detector_updated(self, detector: Detector) -> None: assert detector.name == "Updated Detector" assert detector.type == MetricIssue.slug @@ -568,6 +568,18 @@ def test_update_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 + def test_update_rejects_org_write_scope_for_tokens(self) -> None: + token = self._create_token("org:write") + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + def test_team_admin_can_update_with_project_scoped_alerts_write(self) -> None: team_admin_user = self.create_user(is_superuser=False) self.create_member( @@ -613,12 +625,15 @@ def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: role="member", team_roles=[(self.team, "admin")], ) + self.organization.update_option("sentry:alerts_member_write", False) other_team = self.create_team(organization=self.organization, name="other-team") other_project = self.create_project( organization=self.organization, teams=[other_team], name="other-project" ) - other_detector = self.create_detector(project=other_project, name="Other Detector") + other_detector = self.create_detector( + project=other_project, type=MetricIssue.slug, name="Other Detector" + ) token = self._create_token("alerts:write", user=team_admin_user) @@ -1185,12 +1200,15 @@ def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: role="member", team_roles=[(self.team, "admin")], ) + self.organization.update_option("sentry:alerts_member_write", False) other_team = self.create_team(organization=self.organization, name="other-team") other_project = self.create_project( organization=self.organization, teams=[other_team], name="other-project" ) - other_detector = self.create_detector(project=other_project, name="Other Detector") + other_detector = self.create_detector( + project=other_project, type=MetricIssue.slug, name="Other Detector" + ) token = self._create_token("alerts:write", user=team_admin_user) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py index 8130d4d23c61..21c161fbcc93 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -3,6 +3,7 @@ from unittest import mock import responses +from django.urls import reverse from sentry import audit_log from sentry.api.serializers import serialize @@ -38,6 +39,10 @@ def setUp(self) -> None: super().setUp() self.login_as(user=self.user) + def _auth_headers(self, *scopes: str) -> dict[str, str]: + api_key = self.create_api_key(organization=self.organization, scope_list=list(scopes)) + return {"HTTP_AUTHORIZATION": self.create_basic_auth_header(api_key.key)} + @cell_silo_test class OrganizationWorkflowIndexBaseTest(OrganizationWorkflowAPITestCase): @@ -615,6 +620,16 @@ def setUp(self) -> None: {"name": "assigneeId", "value": "3"}, ] + def test_create_requires_alerts_write_scope_for_api_keys(self) -> None: + response = self.client.post( + reverse(self.endpoint, args=[self.organization.slug]), + data=self.valid_workflow, + format="json", + **self._auth_headers("org:write"), + ) + + assert response.status_code == 403 + @mock.patch("sentry.workflow_engine.endpoints.validators.base.workflow.create_audit_entry") def test_create_workflow__basic(self, mock_audit: mock.MagicMock) -> None: response = self.get_success_response( @@ -1195,6 +1210,16 @@ def setUp(self) -> None: organization_id=self.organization.id, name="Third Workflow", enabled=False ) + def test_bulk_enable_requires_alerts_write_scope_for_api_keys(self) -> None: + response = self.client.put( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", + data={"enabled": True}, + format="json", + **self._auth_headers("org:write"), + ) + + assert response.status_code == 403 + def test_bulk_enable_workflows_by_ids_success(self) -> None: response = self.get_success_response( self.organization.slug, From 76e97a98e1cd7dbeccf21a954f2d0675eb012b03 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 19:08:51 -0700 Subject: [PATCH 15/25] fix(api): Preserve anomaly permission resolution Unwrap RPC organization context before resolving anomaly preview projects and stop swallowing PermissionDenied from that lookup. The earlier helper could hide project-resolution failures and fall back to the broader any-team alerts:write check. Keep invalid project_id parsing local, but preserve the existing permission path for real lookup errors and add regression coverage for both cases. Refs getsentry/getsentry#19897 Co-Authored-By: Codex --- .../organization_events_anomalies.py | 18 ++++++++-- .../test_organization_events_anomalies.py | 33 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 4cd0ae90e9a2..67161afd49bf 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -1,4 +1,5 @@ from drf_spectacular.utils import extend_schema +from rest_framework.exceptions import ValidationError from rest_framework.request import Request from rest_framework.response import Response @@ -20,6 +21,7 @@ from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.models.organization import Organization +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.seer.anomaly_detection.get_historical_anomalies import ( get_historical_anomaly_data_from_seer_preview, ) @@ -38,17 +40,27 @@ class OrganizationEventsAnomaliesEndpoint(OrganizationEventsEndpointBase): # alerts/detectors, so it intentionally follows alert-write permissions. permission_classes = (OrganizationAlertRulePermission,) - def get_alert_mutation_projects(self, request: Request, organization: Organization): + def get_alert_mutation_projects( + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ): raw_project_id = request.data.get("project_id") if raw_project_id is None: return None try: project_id = to_valid_int_id("project_id", raw_project_id) - return self.get_projects(request, organization, project_ids={project_id}) - except Exception: + except ValidationError: return None + lookup_organization = ( + organization.organization + if isinstance(organization, RpcUserOrganizationContext) + else organization + ) + return self.get_projects(request, lookup_organization, project_ids={project_id}) + @extend_schema( operation_id="Identify anomalies in historical data", parameters=[GlobalParams.ORG_ID_OR_SLUG], diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 8e9953bf53d5..d0dd6e615e13 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -2,7 +2,9 @@ from unittest.mock import MagicMock, patch import orjson +import pytest from django.urls import reverse +from rest_framework.exceptions import PermissionDenied from urllib3 import HTTPResponse from urllib3.exceptions import TimeoutError @@ -12,6 +14,7 @@ AlertRuleThresholdType, ) from sentry.models.apitoken import ApiToken +from sentry.organizations.services.organization import organization_service from sentry.seer.anomaly_detection.types import ( Anomaly, AnomalyDetectionConfig, @@ -20,6 +23,7 @@ TimeSeriesPoint, ) from sentry.seer.anomaly_detection.utils import translate_direction +from sentry.seer.endpoints.organization_events_anomalies import OrganizationEventsAnomaliesEndpoint from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.datetime import before_now, freeze_time @@ -90,6 +94,35 @@ def get_context(self) -> DetectHistoricalAnomaliesContext: ], ) + def test_get_alert_mutation_projects_unwraps_rpc_user_org_context(self) -> None: + endpoint = OrganizationEventsAnomaliesEndpoint() + request = MagicMock() + request.data = {"project_id": str(self.project.id)} + + rpc_org_context = organization_service.get_organization_by_id( + id=self.organization.id, user_id=self.user.id + ) + assert rpc_org_context is not None + + with patch.object(endpoint, "get_projects", return_value=[self.project]) as get_projects: + projects = endpoint.get_alert_mutation_projects(request, rpc_org_context) + + assert projects == [self.project] + get_projects.assert_called_once_with( + request, rpc_org_context.organization, project_ids={self.project.id} + ) + + def test_get_alert_mutation_projects_does_not_swallow_permission_denied(self) -> None: + endpoint = OrganizationEventsAnomaliesEndpoint() + request = MagicMock() + request.data = {"project_id": str(self.project.id)} + + with ( + patch.object(endpoint, "get_projects", side_effect=PermissionDenied), + pytest.raises(PermissionDenied), + ): + endpoint.get_alert_mutation_projects(request, self.organization) + @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") @patch( From a64e276c1f46a5782837afd2f291568b056dcc3b Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 19:52:20 -0700 Subject: [PATCH 16/25] fix(api): Preserve detector detail 404 behavior Centralize the RPC-aware organization id helper used by alert-rule and detector detail permission lookups. Detector detail mutations also need the same missing-object fallback as alert rules so project-scoped alerts:write callers still reach convert_args() and return 404 for stale detector ids instead of failing permission checks early. Refs getsentry/getsentry#19897 Co-Authored-By: Codex --- src/sentry/api/bases/organization.py | 9 +++++ src/sentry/incidents/endpoints/bases.py | 14 ++------ .../organization_detector_details.py | 14 +++----- .../test_organization_detector_details.py | 36 +++++++++++++++++++ 4 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 182baa7b6aaf..2a52f845ba67 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -233,6 +233,15 @@ class OrganizationDataExportPermission(OrganizationPermission): } +def get_organization_id( + organization: Organization | RpcOrganization | RpcUserOrganizationContext, +) -> int: + if isinstance(organization, RpcUserOrganizationContext): + return organization.organization.id + + return organization.id + + def _has_any_team_scope(request: Request, scope: str) -> bool: if not request.access.team_ids_with_membership: return False diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index d9c5fce92930..ae2a45641d17 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -11,6 +11,7 @@ OrganizationAlertRulePermission, OrganizationEndpoint, get_legacy_alert_mutation_scopes, + get_organization_id, ) from sentry.api.bases.project import ProjectAlertRulePermission, ProjectEndpoint from sentry.api.exceptions import ResourceDoesNotExist @@ -24,15 +25,6 @@ from sentry.workflow_engine.models.detector import Detector -def _get_organization_id( - organization: Organization | RpcOrganization | RpcUserOrganizationContext, -) -> int: - if isinstance(organization, RpcUserOrganizationContext): - return organization.organization.id - - return organization.id - - class OrganizationAlertRuleBaseEndpoint(OrganizationEndpoint): """ Base endpoint for organization-scoped alert rule creation. @@ -118,7 +110,7 @@ def get_alert_mutation_projects( return None try: - organization_id = _get_organization_id(organization) + organization_id = get_organization_id(organization) alert_rule = AlertRule.objects.prefetch_related("projects").get( organization_id=organization_id, id=validated_alert_rule_id ) @@ -234,7 +226,7 @@ def get_alert_mutation_projects( except RestFrameworkValidationError: return None - organization_id = _get_organization_id(organization) + organization_id = get_organization_id(organization) ard = ( AlertRuleDetector.objects.select_related("detector__project") .filter( diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 11ce5bd1e411..e83e948add65 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -13,6 +13,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases import OrganizationDetectorPermission, OrganizationEndpoint +from sentry.api.bases.organization import get_organization_id from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers import serialize from sentry.apidocs.constants import ( @@ -69,15 +70,6 @@ def _check_metric_detector_allowed(detector: Detector, organization: Organizatio raise ResourceDoesNotExist -def _get_organization_id( - organization: Organization | RpcOrganization | RpcUserOrganizationContext, -) -> int: - if isinstance(organization, RpcUserOrganizationContext): - return organization.organization.id - - return organization.id - - def remove_detector(request: Request, organization: Organization, detector: Detector) -> Response: """ Delete a given detector. This method is used by the OrganizationAlertRuleDetailsEndpoint DELETE method @@ -134,6 +126,8 @@ def get_detector_validator( @cell_silo_endpoint @extend_schema(tags=["Monitors"]) class OrganizationDetectorDetailsEndpoint(OrganizationEndpoint): + allow_any_team_alert_write_fallback = True + def get_alert_mutation_projects( self, request: Request, @@ -151,7 +145,7 @@ def get_alert_mutation_projects( except ValidationError: return None - organization_id = _get_organization_id(organization) + organization_id = get_organization_id(organization) detector = ( Detector.objects.select_related("project") .filter( diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 6653078d6be1..1e9772df84e9 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -646,6 +646,25 @@ def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + def test_team_admin_update_missing_detector_returns_404(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, 999999999]), + data={"enabled": False}, + format="json", + ) + + assert response.status_code == 404 + def test_enable_detector(self) -> None: self.detector.update(enabled=False) self.detector.update(status=ObjectStatus.DISABLED) @@ -1219,6 +1238,23 @@ def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + def test_team_admin_delete_missing_detector_returns_404(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, 999999999]) + ) + + assert response.status_code == 404 + @mock.patch( "sentry.workflow_engine.endpoints.organization_detector_details.schedule_update_project_config" ) From f4a8ba69bc6fbe9a8bdc3531c47b6fb7606bcbeb Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 17 Apr 2026 09:09:41 -0700 Subject: [PATCH 17/25] fix(api): simplify alert mutation permission cleanup Remove duplicated handler-level permission checks and share the legacy alert project scope tuple between the alert-rule and monitor validators. Keep anomaly preview project validation and the original system-detector edit contract intact, add the missing detector-index delete regression, and annotate the prompts activity readonly mutation note that the permission audit now enforces. Refs getsentry/getsentry#19897 Co-Authored-By: Codex --- src/sentry/api/bases/organization.py | 1 + src/sentry/api/endpoints/prompts_activity.py | 3 ++ src/sentry/incidents/endpoints/bases.py | 12 ++--- .../organization_alert_rule_details.py | 15 ------- .../incidents/serializers/alert_rule.py | 7 +-- src/sentry/monitors/validators.py | 6 +-- .../organization_events_anomalies.py | 10 +---- .../endpoints/validators/utils.py | 44 +++++++++---------- tests/sentry/api/test_permissions.py | 6 +-- .../test_organization_detector_index.py | 21 +++++++++ 10 files changed, 60 insertions(+), 65 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 2a52f845ba67..fec9b69998b3 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -252,6 +252,7 @@ def _has_any_team_scope(request: Request, scope: str) -> bool: # Project-scoped alert authoring should rely on the alert-specific write scope. ALERT_MUTATION_SCOPES = frozenset({"alerts:write"}) +LEGACY_ALERT_MUTATION_PROJECT_SCOPES = ("project:read", "org:write", "alerts:write") def _has_project_alert_write_access(request: Request, projects: Sequence[Project]) -> bool: diff --git a/src/sentry/api/endpoints/prompts_activity.py b/src/sentry/api/endpoints/prompts_activity.py index 057c3c6af047..08237e98d8e2 100644 --- a/src/sentry/api/endpoints/prompts_activity.py +++ b/src/sentry/api/endpoints/prompts_activity.py @@ -40,6 +40,9 @@ class PromptsActivityPermission(OrganizationPermission): "GET": ["org:read", "org:write", "org:admin"], "PUT": ["org:read", "org:write", "org:admin"], } + readonly_mutation_scope_exceptions = { + "PUT": "Prompt activity updates only change the requesting member's prompt state.", + } @cell_silo_endpoint diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index ae2a45641d17..95a7884bea53 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -51,12 +51,12 @@ def check_can_create_alert(self, request: Request, organization: Organization) - # team admins should be able to create alerts for the projects they have access to projects = self.get_projects(request, organization) # team admins will have alerts:write scoped to their projects, members will not - team_admin_has_access = all( - [request.access.has_project_scope(project, "alerts:write") for project in projects] - ) - # all() returns True for empty list, so include a check for it - if not team_admin_has_access or not projects: - raise PermissionDenied + if projects and all( + request.access.has_project_scope(project, "alerts:write") for project in projects + ): + return + + raise PermissionDenied class ProjectAlertRuleEndpoint(ProjectEndpoint): diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 4928e9060a84..d3793b0e41a7 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -13,10 +13,6 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import ( - ALERT_MUTATION_SCOPES, - get_legacy_alert_mutation_scopes, -) from sentry.api.fields.actor import OwnerActorField from sentry.api.serializers import serialize from sentry.api.serializers.rest_framework.project import ProjectField @@ -332,17 +328,6 @@ def wrapper( if not request.access.has_project_access(project): return Response(status=status.HTTP_403_FORBIDDEN) - if request.method in {"PUT", "DELETE"}: - has_project_alert_write = request.access.has_any_project_scope( - project, ALERT_MUTATION_SCOPES - ) - has_legacy_org_write = any( - request.access.has_scope(scope) - for scope in get_legacy_alert_mutation_scopes(self, request.method) - ) - if not has_project_alert_write and not has_legacy_org_write: - return Response(status=status.HTTP_403_FORBIDDEN) - return func(self, request, organization, alert_rule) return wrapper diff --git a/src/sentry/incidents/serializers/alert_rule.py b/src/sentry/incidents/serializers/alert_rule.py index 7b88de28c4ca..6eabf59420f4 100644 --- a/src/sentry/incidents/serializers/alert_rule.py +++ b/src/sentry/incidents/serializers/alert_rule.py @@ -16,6 +16,7 @@ from urllib3.exceptions import MaxRetryError, TimeoutError from sentry import features +from sentry.api.bases.organization import LEGACY_ALERT_MUTATION_PROJECT_SCOPES from sentry.api.exceptions import BadRequest, RequestTimeout from sentry.api.fields.actor import OwnerActorField from sentry.api.helpers.error_upsampling import are_any_projects_error_upsampled @@ -51,10 +52,6 @@ logger = logging.getLogger(__name__) -# TODO(api-write-scope-compat): Remove legacy org:write support once public -# metric alert clients have migrated to alerts:write. -ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "alerts:write") - class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRule]): """ @@ -66,7 +63,7 @@ class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRu environment = serializers.CharField(required=False, allow_null=True) projects = serializers.ListField( - child=ProjectField(scope=ALERT_RULE_PROJECT_SCOPES), + child=ProjectField(scope=LEGACY_ALERT_MUTATION_PROJECT_SCOPES), required=False, max_length=1, ) diff --git a/src/sentry/monitors/validators.py b/src/sentry/monitors/validators.py index 50f5c785f6f5..d72fbe69f00e 100644 --- a/src/sentry/monitors/validators.py +++ b/src/sentry/monitors/validators.py @@ -14,6 +14,7 @@ from rest_framework.fields import empty from sentry import audit_log, quotas +from sentry.api.bases.organization import LEGACY_ALERT_MUTATION_PROJECT_SCOPES from sentry.api.fields.actor import OwnerActorField from sentry.api.fields.empty_integer import EmptyIntegerField from sentry.api.fields.sentry_slug import SentrySerializerSlugField @@ -74,9 +75,6 @@ INTERVAL_NAMES = ("year", "month", "week", "day", "hour", "minute") CRONTAB_WHITESPACE = re.compile(r"\s+") -# TODO(api-write-scope-compat): Remove legacy org:write support once public -# cron monitor clients have migrated to alerts:write. -MONITOR_PROJECT_SCOPES = ("project:read", "org:write", "alerts:write") # XXX(dcramer): @reboot is not supported (as it cannot be) NONSTANDARD_CRONTAB_SCHEDULES = { @@ -315,7 +313,7 @@ def validate(self, attrs): @extend_schema_serializer(exclude_fields=["alert_rule"]) class MonitorValidator(CamelSnakeSerializer): project = ProjectField( - scope=MONITOR_PROJECT_SCOPES, + scope=LEGACY_ALERT_MUTATION_PROJECT_SCOPES, required=True, help_text="The project slug to associate the monitor to.", ) diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 67161afd49bf..0fa1431e06d5 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -7,7 +7,7 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import ALERT_MUTATION_SCOPES, OrganizationAlertRulePermission +from sentry.api.bases.organization import OrganizationAlertRulePermission from sentry.api.bases.organization_events import OrganizationEventsEndpointBase from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers.base import serialize @@ -115,13 +115,7 @@ def post(self, request: Request, organization: Organization) -> Response: ) project_id = to_valid_int_id("project_id", raw_project_id) - - projects = self.get_projects(request, organization, project_ids={project_id}) - if not all( - request.access.has_any_project_scope(project, ALERT_MUTATION_SCOPES) - for project in projects - ): - return Response(status=403) + self.get_projects(request, organization, project_ids={project_id}) anomalies = get_historical_anomaly_data_from_seer_preview( current_data=current_data, diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index 8d163427ad0c..514d74a77db5 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -26,10 +26,8 @@ logger = logging.getLogger(__name__) -# Detector edits should use the alert-specific write scope regardless of how the -# detector was created. -SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES = {"alerts:write"} -USER_CREATED_DETECTOR_REQUIRED_SCOPES = {"alerts:write"} +SYSTEM_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"org:write"}) +USER_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"alerts:write"}) def is_system_created_detector(detector: Detector) -> bool: @@ -45,12 +43,15 @@ def is_system_created_detector(detector: Detector) -> bool: ) -def can_edit_system_created_detectors(request: Request, project: Project) -> bool: - return request.access.has_any_project_scope(project, SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES) +def _can_edit_project_detectors(request: Request, project: Project, scopes: frozenset[str]) -> bool: + return request.access.has_any_project_scope(project, scopes) -def can_edit_user_created_detectors(request: Request, project: Project) -> bool: - return request.access.has_any_project_scope(project, USER_CREATED_DETECTOR_REQUIRED_SCOPES) +def _get_detector_edit_scopes(detector: Detector) -> frozenset[str]: + if is_system_created_detector(detector): + return SYSTEM_CREATED_DETECTOR_EDIT_SCOPES + + return USER_CREATED_DETECTOR_EDIT_SCOPES def can_edit_detectors(detectors: QuerySet[Detector], request: Request) -> bool: @@ -58,17 +59,16 @@ def can_edit_detectors(detectors: QuerySet[Detector], request: Request) -> bool: Determine if the requesting user has access to edit the given detectors. """ required_scopes = ( - SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES + SYSTEM_CREATED_DETECTOR_EDIT_SCOPES if any(is_system_created_detector(detector) for detector in detectors) - else USER_CREATED_DETECTOR_REQUIRED_SCOPES + else USER_CREATED_DETECTOR_EDIT_SCOPES ) - projects = Project.objects.filter( id__in=detectors.values_list("project_id", flat=True).distinct() ) return all( - request.access.has_any_project_scope(project, required_scopes) for project in projects + _can_edit_project_detectors(request, project, required_scopes) for project in projects ) @@ -76,12 +76,9 @@ def can_edit_detector(detector: Detector, request: Request) -> bool: """ Determine if the requesting user has alert-write access to edit the detector. """ - if is_system_created_detector(detector) and not can_edit_system_created_detectors( - request, detector.project - ): - return False - - return can_edit_user_created_detectors(request, detector.project) + return _can_edit_project_detectors( + request, detector.project, _get_detector_edit_scopes(detector) + ) def can_delete_detectors(detectors: QuerySet[Detector], request: Request) -> bool: @@ -95,7 +92,10 @@ def can_delete_detectors(detectors: QuerySet[Detector], request: Request) -> boo projects = Project.objects.filter( id__in=detectors.values_list("project_id", flat=True).distinct() ) - return all(can_edit_user_created_detectors(request, project) for project in projects) + return all( + _can_edit_project_detectors(request, project, USER_CREATED_DETECTOR_EDIT_SCOPES) + for project in projects + ) def can_delete_detector(detector: Detector, request: Request) -> bool: @@ -106,7 +106,7 @@ def can_delete_detector(detector: Detector, request: Request) -> bool: if is_system_created_detector(detector): return False - return can_edit_user_created_detectors(request, detector.project) + return _can_edit_project_detectors(request, detector.project, USER_CREATED_DETECTOR_EDIT_SCOPES) def can_edit_detector_workflow_connections(detector: Detector, request: Request) -> bool: @@ -114,9 +114,7 @@ def can_edit_detector_workflow_connections(detector: Detector, request: Request) Anyone with alert write access to the project can connect/disconnect detectors of any type, which is slightly different from full edit access which differs by detector type. """ - return request.access.has_any_project_scope( - detector.project, USER_CREATED_DETECTOR_REQUIRED_SCOPES - ) + return _can_edit_project_detectors(request, detector.project, USER_CREATED_DETECTOR_EDIT_SCOPES) def validate_detectors_exist_and_have_permissions( diff --git a/tests/sentry/api/test_permissions.py b/tests/sentry/api/test_permissions.py index faef4de77ea9..ca34dfd0177b 100644 --- a/tests/sentry/api/test_permissions.py +++ b/tests/sentry/api/test_permissions.py @@ -24,12 +24,10 @@ MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) -def _iter_endpoint_view_classes(urlpatterns, base: str = "") -> Generator[type[Endpoint]]: +def _iter_endpoint_view_classes(urlpatterns) -> Generator[type[Endpoint]]: for pattern in urlpatterns: if isinstance(pattern, URLResolver): - yield from _iter_endpoint_view_classes( - pattern.url_patterns, base + str(pattern.pattern) - ) + yield from _iter_endpoint_view_classes(pattern.url_patterns) elif isinstance(pattern, URLPattern): callback = pattern.callback if hasattr(callback, "view_class") and issubclass(callback.view_class, Endpoint): diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py index 42414c0e70d1..85e6506b3d59 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py @@ -1515,6 +1515,27 @@ def test_delete_detectors_audit_entry(self) -> None: actor=self.user, ) + def test_delete_detectors_permission_allowed_for_team_admin(self) -> None: + team_admin_user = self.create_user() + self.create_member( + team_roles=[(self.team, "admin")], + user=team_admin_user, + role="member", + organization=self.organization, + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(user=team_admin_user) + + with outbox_runner(): + self.get_success_response( + self.organization.slug, + qs_params={"id": str(self.detector.id)}, + status_code=204, + ) + + self.detector.refresh_from_db() + assert self.detector.status == ObjectStatus.PENDING_DELETION + def test_delete_detectors_permission_denied(self) -> None: # Members can not delete detectors for projects they are not part of other_detector = self.create_detector( From 7ac8126e65e8b5273643269c4437ea235a951c28 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 17 Apr 2026 09:59:03 -0700 Subject: [PATCH 18/25] fix(api): Restore write scope compatibility Restore legacy write-scope compatibility for published and experimental API mutations that this branch tightened, and add regression coverage for the token and session-role paths those endpoints rely on. Preserve current client behavior while keeping the temporary compat shims clearly marked for later removal once callers have migrated. Co-Authored-By: Codex --- src/sentry/api/bases/organization.py | 19 ++++ .../issues/endpoints/project_user_issue.py | 2 +- .../endpoints/organization_monitor_index.py | 16 ++- .../endpoints/project_replay_details.py | 2 +- .../organization_events_anomalies.py | 5 + ...organization_uptime_alert_preview_check.py | 5 + ...ganization_uptime_assertion_suggestions.py | 5 + .../organization_detector_details.py | 6 + .../endpoints/organization_detector_index.py | 6 + .../endpoints/organization_workflow_index.py | 37 +++++- .../endpoints/validators/utils.py | 5 +- .../endpoints/test_project_user_issue.py | 32 +++--- .../test_organization_monitor_index.py | 40 +++++++ .../endpoints/test_project_replay_details.py | 43 ++++--- .../test_organization_events_anomalies.py | 89 ++++++++++++++- .../test_organization_uptime_alert_preview.py | 78 ++++++++++++- ...ganization_uptime_assertion_suggestions.py | 106 +++++++++++++++++- .../test_organization_detector_details.py | 45 ++++++-- .../test_organization_detector_index.py | 64 ++++++++++- .../test_organization_workflow_index.py | 54 ++++++++- 20 files changed, 598 insertions(+), 61 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index fec9b69998b3..c1f42b1982d9 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -345,6 +345,19 @@ class OrganizationDetectorPermission(OrganizationPermission): "DELETE": ["alerts:write"], } + def has_permission(self, request: Request, view: APIView) -> bool: + if super().has_permission(request, view): + return True + + if not request.auth: + return False + + current_scopes = request.auth.get_scopes() + return any( + scope in current_scopes + for scope in get_legacy_alert_mutation_scopes(view, request.method) + ) + def has_object_permission( self, request: Request, @@ -357,6 +370,12 @@ def has_object_permission( if request.method not in {"POST", "PUT", "DELETE"}: return False + if any( + request.access.has_scope(scope) + for scope in get_legacy_alert_mutation_scopes(view, request.method) + ): + return True + project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) if project_scoped_access is not None: return project_scoped_access diff --git a/src/sentry/issues/endpoints/project_user_issue.py b/src/sentry/issues/endpoints/project_user_issue.py index 3effe6dd7443..d8e6b07e51a6 100644 --- a/src/sentry/issues/endpoints/project_user_issue.py +++ b/src/sentry/issues/endpoints/project_user_issue.py @@ -180,7 +180,7 @@ class WebVitalsIssueDataSerializer(ProjectUserIssueRequestSerializer): class ProjectUserIssuePermission(ProjectPermission): scope_map = { "GET": [], - "POST": ["event:write", "event:admin"], + "POST": ["event:read", "event:write", "event:admin"], "PUT": [], "DELETE": [], } diff --git a/src/sentry/monitors/endpoints/organization_monitor_index.py b/src/sentry/monitors/endpoints/organization_monitor_index.py index 8f443ec99a8e..f3f00fc95c36 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_index.py +++ b/src/sentry/monitors/endpoints/organization_monitor_index.py @@ -18,7 +18,11 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases import NoProjects -from sentry.api.bases.organization import ALERT_MUTATION_SCOPES, OrganizationAlertRulePermission +from sentry.api.bases.organization import ( + ALERT_MUTATION_SCOPES, + OrganizationAlertRulePermission, + get_legacy_alert_mutation_scopes, +) from sentry.api.helpers.teams import get_teams from sentry.api.paginator import OffsetPaginator from sentry.api.serializers import serialize @@ -89,7 +93,10 @@ class OrganizationMonitorIndexEndpoint(OrganizationAlertRuleBaseEndpoint): owner = ApiOwner.CRONS # TODO(api-write-scope-compat): Remove legacy org:write support once public # cron monitor clients have migrated to alerts:write. - legacy_alert_mutation_scope_map = {"POST": ("org:write",)} + legacy_alert_mutation_scope_map = { + "POST": ("org:write", "org:admin"), + "PUT": ("org:write", "org:admin"), + } permission_classes = (OrganizationAlertRulePermission,) @extend_schema( @@ -329,6 +336,9 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: return self.respond(validator.errors, status=400) result = dict(validator.validated_data) + monitor_edit_scopes = ALERT_MUTATION_SCOPES | frozenset( + get_legacy_alert_mutation_scopes(self, request.method) + ) projects = self.get_projects(request, organization, include_all_accessible=True) project_ids = [project.id for project in projects] @@ -338,7 +348,7 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: projects_by_id = Project.objects.in_bulk({monitor.project_id for monitor in monitors}) if not all( project is not None - and request.access.has_any_project_scope(project, ALERT_MUTATION_SCOPES) + and request.access.has_any_project_scope(project, monitor_edit_scopes) for project in (projects_by_id.get(monitor.project_id) for monitor in monitors) ): return self.respond(status=403) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 0d0f592fcf4b..24e2b8444390 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -23,7 +23,7 @@ class ReplayDetailsPermission(ProjectPermission): "GET": ["project:read", "project:write", "project:admin"], "POST": ["project:write", "project:admin"], "PUT": ["project:write", "project:admin"], - "DELETE": ["event:admin"], + "DELETE": ["project:read", "project:write", "project:admin"], } diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 0fa1431e06d5..6ecb14bc4ead 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -36,6 +36,11 @@ class OrganizationEventsAnomaliesEndpoint(OrganizationEventsEndpointBase): "POST": ApiPublishStatus.EXPERIMENTAL, } allow_any_team_alert_write_fallback = True + # TODO(api-write-scope-compat): Remove legacy org:* support once alert + # authoring preview clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "POST": ("org:read", "org:write", "org:admin"), + } # This POST previews anomaly-detection config used while authoring metric # alerts/detectors, so it intentionally follows alert-write permissions. permission_classes = (OrganizationAlertRulePermission,) diff --git a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py index 10eb603f03d0..fea53b54ec50 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py +++ b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py @@ -31,6 +31,11 @@ class OrganizationUptimeAlertPreviewCheckEndpoint(OrganizationEndpoint): owner = ApiOwner.CRONS allow_any_team_alert_write_fallback = True + # TODO(api-write-scope-compat): Remove legacy org:* support once uptime + # preview clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "POST": ("org:read", "org:write", "org:admin"), + } # This POST previews monitor creation and validation, so it intentionally # uses the same permission surface as creating the alert itself. permission_classes = (OrganizationAlertRulePermission,) diff --git a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py index c47879dd8f32..e1226f7c3f89 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py +++ b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py @@ -51,6 +51,11 @@ class OrganizationUptimeAssertionSuggestionsEndpoint(OrganizationEndpoint): owner = ApiOwner.CRONS allow_any_team_alert_write_fallback = True + # TODO(api-write-scope-compat): Remove legacy org:* support once uptime + # assertion-suggestion clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "POST": ("org:read", "org:write", "org:admin"), + } # This POST is part of the uptime monitor authoring flow, so it should # track the same alert-write permission as the monitor it helps create. permission_classes = (OrganizationAlertRulePermission,) diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index e83e948add65..b5e3f77af2e5 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -127,6 +127,12 @@ def get_detector_validator( @extend_schema(tags=["Monitors"]) class OrganizationDetectorDetailsEndpoint(OrganizationEndpoint): allow_any_team_alert_write_fallback = True + # TODO(api-write-scope-compat): Remove legacy org:* support once public + # detector clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "PUT": ("org:read", "org:write", "org:admin"), + "DELETE": ("org:read", "org:write", "org:admin"), + } def get_alert_mutation_projects( self, diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_index.py b/src/sentry/workflow_engine/endpoints/organization_detector_index.py index 5f485dbf45e0..9ca48612909c 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_index.py @@ -154,6 +154,12 @@ class OrganizationDetectorIndexEndpoint(OrganizationEndpoint): } owner = ApiOwner.ISSUES allow_any_team_alert_write_fallback = True + # TODO(api-write-scope-compat): Remove legacy org:* support once public + # detector clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "PUT": ("org:read", "org:write", "org:admin"), + "DELETE": ("org:read", "org:write", "org:admin"), + } permission_classes = (OrganizationDetectorPermission,) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index 7e1a75953e6f..c9e176beafba 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -27,7 +27,10 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases import OrganizationEndpoint -from sentry.api.bases.organization import OrganizationPermission +from sentry.api.bases.organization import ( + OrganizationPermission, + get_legacy_alert_mutation_scopes, +) from sentry.api.event_search import SearchConfig, SearchFilter, SearchKey, default_config from sentry.api.event_search import parse_search_query as base_parse_search_query from sentry.api.exceptions import ResourceDoesNotExist @@ -96,6 +99,31 @@ class OrganizationWorkflowPermission(OrganizationPermission): "DELETE": ["alerts:write"], } + def has_permission(self, request: Request, view) -> bool: + if super().has_permission(request, view): + return True + + if not request.auth: + return False + + current_scopes = request.auth.get_scopes() + return any( + scope in current_scopes + for scope in get_legacy_alert_mutation_scopes(view, request.method) + ) + + def has_object_permission(self, request: Request, view, organization) -> bool: + if super().has_object_permission(request, view, organization): + return True + + if request.method not in {"POST", "PUT", "DELETE"}: + return False + + return any( + request.access.has_scope(scope) + for scope in get_legacy_alert_mutation_scopes(view, request.method) + ) + class OrganizationWorkflowEndpoint(OrganizationEndpoint): permission_classes = (OrganizationWorkflowPermission,) @@ -141,6 +169,13 @@ class OrganizationWorkflowIndexEndpoint(OrganizationEndpoint): "DELETE": ApiPublishStatus.PUBLIC, } owner = ApiOwner.ISSUES + # TODO(api-write-scope-compat): Remove legacy org:* support once public + # workflow clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "POST": ("org:write", "org:admin"), + "PUT": ("org:write", "org:admin"), + "DELETE": ("org:write", "org:admin"), + } permission_classes = (OrganizationWorkflowPermission,) def filter_workflows(self, request: Request, organization: Organization) -> QuerySet[Workflow]: diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index 514d74a77db5..af93ea9439f7 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -27,7 +27,10 @@ logger = logging.getLogger(__name__) SYSTEM_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"org:write"}) -USER_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"alerts:write"}) +# TODO(api-write-scope-compat): Remove legacy org:write support for +# user-created detector edits once public detector clients have migrated to +# alerts:write. +USER_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"org:write", "alerts:write"}) def is_system_created_detector(detector: Detector) -> bool: diff --git a/tests/sentry/issues/endpoints/test_project_user_issue.py b/tests/sentry/issues/endpoints/test_project_user_issue.py index 7071efc54a49..d2ab96002641 100644 --- a/tests/sentry/issues/endpoints/test_project_user_issue.py +++ b/tests/sentry/issues/endpoints/test_project_user_issue.py @@ -117,23 +117,27 @@ def test_no_access(self) -> None: assert response.status_code == 404 @with_feature("organizations:performance-web-vitals-seer-suggestions") - def test_create_web_vitals_issue_requires_event_write_scope(self) -> None: + def test_create_web_vitals_issue_allows_event_read_scope(self) -> None: token = self._create_token("event:read") - response = self.client.post( - self.url, - data={ - "transaction": "/test-transaction", - "issueType": WebVitalsGroup.slug, - "score": 75, - "value": 1000, - "vital": "lcp", - }, - format="json", - HTTP_AUTHORIZATION=f"Bearer {token.token}", - ) + with patch( + "sentry.issues.endpoints.project_user_issue.produce_occurrence_to_kafka" + ) as mock_produce: + response = self.client.post( + self.url, + data={ + "transaction": "/test-transaction", + "issueType": WebVitalsGroup.slug, + "score": 75, + "value": 1000, + "vital": "lcp", + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) - assert response.status_code == 403 + assert response.status_code == 200 + assert response.data == {"event_id": mock_produce.call_args[1]["occurrence"].event_id} @with_feature("organizations:performance-web-vitals-seer-suggestions") def test_create_web_vitals_issue_allows_event_write_scope(self) -> None: diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index 30ea26c91a19..1c7428ff88c1 100644 --- a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py +++ b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py @@ -1047,6 +1047,46 @@ def test_bulk_edit_denies_alerts_write_scope_for_other_team_projects(self) -> No other_monitor.refresh_from_db() assert other_monitor.status == ObjectStatus.ACTIVE + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once + # public cron monitor clients have migrated to alerts:write. + def test_bulk_edit_allows_org_write_scope_for_tokens(self) -> None: + monitor_one = self._create_monitor(slug="monitor-one") + token = self._create_token("org:write") + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug]), + data={"ids": [monitor_one.guid], "status": "disabled"}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + monitor_one.refresh_from_db() + assert monitor_one.status == ObjectStatus.DISABLED + + def test_team_admin_can_bulk_edit_with_project_scoped_alerts_write(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + monitor_one = self._create_monitor(slug="monitor-one") + + response = self.get_success_response( + self.organization.slug, + ids=[monitor_one.guid], + status="disabled", + ) + + assert response.status_code == 200 + monitor_one.refresh_from_db() + assert monitor_one.status == ObjectStatus.DISABLED + @patch("sentry.quotas.backend.check_assign_seats") def test_enable_no_quota(self, check_assign_seats: MagicMock) -> None: monitor_one = self._create_monitor(slug="monitor_one", status=ObjectStatus.DISABLED) diff --git a/tests/sentry/replays/endpoints/test_project_replay_details.py b/tests/sentry/replays/endpoints/test_project_replay_details.py index f68bd4622047..284bcf87fb65 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_details.py +++ b/tests/sentry/replays/endpoints/test_project_replay_details.py @@ -195,7 +195,7 @@ def test_delete_replay_from_filestore(self) -> None: except File.DoesNotExist: pass - def test_delete_requires_event_admin_scope_for_api_tokens(self) -> None: + def test_delete_denies_event_read_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) @@ -206,16 +206,29 @@ def test_delete_requires_event_admin_scope_for_api_tokens(self) -> None: assert response.status_code == 403 - def test_delete_denies_project_write_scope_for_api_tokens(self) -> None: + def test_delete_allows_project_read_scope_for_api_tokens(self) -> None: + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["project:read"]) + + with self.feature(REPLAYS_FEATURES): + with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) + + assert response.status_code == 204 + + def test_delete_allows_project_write_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["project:write"]) with self.feature(REPLAYS_FEATURES): - response = self.client.delete( - self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" - ) + with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) - assert response.status_code == 403 + assert response.status_code == 204 def test_delete_denies_event_write_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): @@ -229,19 +242,18 @@ def test_delete_denies_event_write_scope_for_api_tokens(self) -> None: assert response.status_code == 403 - def test_delete_allows_event_admin_scope_for_api_tokens(self) -> None: + def test_delete_denies_event_admin_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["event:admin"]) with self.feature(REPLAYS_FEATURES): - with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): - response = self.client.delete( - self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" - ) + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) - assert response.status_code == 204 + assert response.status_code == 403 - def test_delete_requires_event_admin_scope_for_members_without_event_admin(self) -> None: + def test_delete_members_do_not_require_event_admin(self) -> None: self.organization.update_option("sentry:events_member_admin", False) member_user = self.create_user(is_superuser=False) @@ -251,9 +263,10 @@ def test_delete_requires_event_admin_scope_for_members_without_event_admin(self) self.login_as(user=member_user) with self.feature(REPLAYS_FEATURES): - response = self.client.delete(self.url) + with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): + response = self.client.delete(self.url) - assert response.status_code == 403 + assert response.status_code == 204 def test_delete_replay_from_clickhouse_data(self) -> None: """Test deleting files uploaded through the direct storage interface.""" diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index d0dd6e615e13..056dd95f3c83 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -241,7 +241,28 @@ def test_alerts_write_scope_allows_post(self, mock_seer_request: MagicMock) -> N @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") - def test_org_read_scope_cannot_post(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # alert authoring preview clients have migrated to alerts:write. + @patch( + "sentry.seer.anomaly_detection.get_historical_anomalies.seer_anomaly_detection_connection_pool.urlopen" + ) + def test_org_read_scope_can_post(self, mock_seer_request: MagicMock) -> None: + mock_seer_request.return_value = HTTPResponse( + orjson.dumps( + DetectAnomaliesResponse( + success=True, + message="", + timeseries=[ + TimeSeriesPoint( + timestamp=self.current_timestamp_1, + value=2, + anomaly=Anomaly(anomaly_score=-0.1, anomaly_type="none"), + ) + ], + ) + ), + status=200, + ) token = self._create_token("org:read") data = self.get_test_data(self.project.id) url = reverse(self.endpoint, args=[self.organization.slug]) @@ -254,11 +275,32 @@ def test_org_read_scope_cannot_post(self) -> None: HTTP_AUTHORIZATION=f"Bearer {token.token}", ) - assert response.status_code == 403 + assert response.status_code == 200 @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") - def test_org_write_scope_cannot_post(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # alert authoring preview clients have migrated to alerts:write. + @patch( + "sentry.seer.anomaly_detection.get_historical_anomalies.seer_anomaly_detection_connection_pool.urlopen" + ) + def test_org_write_scope_can_post(self, mock_seer_request: MagicMock) -> None: + mock_seer_request.return_value = HTTPResponse( + orjson.dumps( + DetectAnomaliesResponse( + success=True, + message="", + timeseries=[ + TimeSeriesPoint( + timestamp=self.current_timestamp_1, + value=2, + anomaly=Anomaly(anomaly_score=-0.1, anomaly_type="none"), + ) + ], + ) + ), + status=200, + ) token = self._create_token("org:write") data = self.get_test_data(self.project.id) url = reverse(self.endpoint, args=[self.organization.slug]) @@ -271,7 +313,7 @@ def test_org_write_scope_cannot_post(self) -> None: HTTP_AUTHORIZATION=f"Bearer {token.token}", ) - assert response.status_code == 403 + assert response.status_code == 200 @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") @@ -303,6 +345,45 @@ def test_alerts_write_scope_denies_other_team_projects(self) -> None: assert response.status_code == 403 + @with_feature("organizations:anomaly-detection-alerts") + @with_feature("organizations:incidents") + @patch( + "sentry.seer.anomaly_detection.get_historical_anomalies.seer_anomaly_detection_connection_pool.urlopen" + ) + def test_team_admin_can_post_when_member_alert_write_disabled( + self, mock_seer_request: MagicMock + ) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + seer_return_value = DetectAnomaliesResponse( + success=True, + message="", + timeseries=[ + TimeSeriesPoint( + timestamp=self.current_timestamp_1, + value=2, + anomaly=Anomaly(anomaly_score=-0.1, anomaly_type="none"), + ) + ], + ) + mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200) + + data = self.get_test_data(self.project.id) + with outbox_runner(): + response = self.get_success_response( + self.organization.slug, status_code=200, raw_data=orjson.dumps(data) + ) + + assert response.status_code == 200 + @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") @patch( diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py index 12cc23a7f6c7..71ad841d7efa 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py @@ -135,9 +135,25 @@ def test_alerts_write_permission(self) -> None: assert response.status_code == 200, response.content - def test_org_read_scope_cannot_run_preview_check(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # uptime preview clients have migrated to alerts:write. + @responses.activate + def test_org_read_scope_can_run_preview_check(self) -> None: api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) + responses.add( + responses.POST, + "http://pop-st-1.uptime-checker.s4s.sentry.internal:80/validate_check", + status=200, + json={"succeeded": True}, + ) + responses.add( + responses.POST, + "http://pop-st-1.uptime-checker.s4s.sentry.internal:80/execute_config", + status=200, + json={"succeeded": True}, + ) + url = reverse( "sentry-api-0-organization-uptime-alert-preview-check", kwargs={"organization_id_or_slug": self.organization.slug}, @@ -157,11 +173,27 @@ def test_org_read_scope_cannot_run_preview_check(self) -> None: HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), ) - assert response.status_code == 403 + assert response.status_code == 200 - def test_org_write_scope_cannot_run_preview_check(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # uptime preview clients have migrated to alerts:write. + @responses.activate + def test_org_write_scope_can_run_preview_check(self) -> None: api_key = self.create_api_key(organization=self.organization, scope_list=["org:write"]) + responses.add( + responses.POST, + "http://pop-st-1.uptime-checker.s4s.sentry.internal:80/validate_check", + status=200, + json={"succeeded": True}, + ) + responses.add( + responses.POST, + "http://pop-st-1.uptime-checker.s4s.sentry.internal:80/execute_config", + status=200, + json={"succeeded": True}, + ) + url = reverse( "sentry-api-0-organization-uptime-alert-preview-check", kwargs={"organization_id_or_slug": self.organization.slug}, @@ -181,4 +213,42 @@ def test_org_write_scope_cannot_run_preview_check(self) -> None: HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), ) - assert response.status_code == 403 + assert response.status_code == 200 + + @responses.activate + def test_team_admin_can_run_preview_check_when_member_alert_write_disabled(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + responses.add( + responses.POST, + "http://pop-st-1.uptime-checker.s4s.sentry.internal:80/validate_check", + status=200, + json={"succeeded": True}, + ) + responses.add( + responses.POST, + "http://pop-st-1.uptime-checker.s4s.sentry.internal:80/execute_config", + status=200, + json={"succeeded": True}, + ) + + response = self.get_success_response( + self.organization.slug, + name="test", + environment="uptime-prod", + owner=f"user:{team_admin_user.id}", + url="http://sentry.io", + timeout_ms=1500, + body=None, + region="default", + ) + + assert response.status_code == 200 diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py index ddcd38fadd94..b73bae922980 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py @@ -34,6 +34,8 @@ def setUp(self) -> None: "region": "default", } + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # uptime assertion-suggestion clients have migrated to alerts:write. @mock.patch( "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" ) @@ -72,8 +74,36 @@ def test_alerts_write_scope_can_generate_suggestions( assert response.status_code == 200 - def test_org_read_scope_cannot_generate_suggestions(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # uptime assertion-suggestion clients have migrated to alerts:write. + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.checker_api.invoke_checker_preview" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.UptimeCheckPreviewValidator" + ) + @mock.patch("sentry.uptime.endpoints.organization_uptime_assertion_suggestions.has_seer_access") + def test_org_read_scope_can_generate_suggestions( + self, + mock_has_seer_access: mock.MagicMock, + mock_validator_cls: mock.MagicMock, + mock_preview: mock.MagicMock, + mock_generate: mock.MagicMock, + ) -> None: api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) + mock_has_seer_access.return_value = True + mock_validator = mock_validator_cls.return_value + mock_validator.is_valid.return_value = True + mock_validator.save.return_value = {"active_regions": ["default"]} + mock_preview.return_value = mock.Mock( + status_code=200, + json=mock.Mock(return_value={"status": 200}), + raise_for_status=mock.Mock(), + ) + mock_generate.return_value = (None, None) response = self.client.post( self.url, @@ -82,10 +112,36 @@ def test_org_read_scope_cannot_generate_suggestions(self) -> None: HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), ) - assert response.status_code == 403 + assert response.status_code == 200 - def test_org_write_scope_cannot_generate_suggestions(self) -> None: + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.checker_api.invoke_checker_preview" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.UptimeCheckPreviewValidator" + ) + @mock.patch("sentry.uptime.endpoints.organization_uptime_assertion_suggestions.has_seer_access") + def test_org_write_scope_can_generate_suggestions( + self, + mock_has_seer_access: mock.MagicMock, + mock_validator_cls: mock.MagicMock, + mock_preview: mock.MagicMock, + mock_generate: mock.MagicMock, + ) -> None: api_key = self.create_api_key(organization=self.organization, scope_list=["org:write"]) + mock_has_seer_access.return_value = True + mock_validator = mock_validator_cls.return_value + mock_validator.is_valid.return_value = True + mock_validator.save.return_value = {"active_regions": ["default"]} + mock_preview.return_value = mock.Mock( + status_code=200, + json=mock.Mock(return_value={"status": 200}), + raise_for_status=mock.Mock(), + ) + mock_generate.return_value = (None, None) response = self.client.post( self.url, @@ -94,4 +150,46 @@ def test_org_write_scope_cannot_generate_suggestions(self) -> None: HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), ) - assert response.status_code == 403 + assert response.status_code == 200 + + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.checker_api.invoke_checker_preview" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.UptimeCheckPreviewValidator" + ) + @mock.patch("sentry.uptime.endpoints.organization_uptime_assertion_suggestions.has_seer_access") + def test_team_admin_can_generate_suggestions_when_member_alert_write_disabled( + self, + mock_has_seer_access: mock.MagicMock, + mock_validator_cls: mock.MagicMock, + mock_preview: mock.MagicMock, + mock_generate: mock.MagicMock, + ) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + mock_has_seer_access.return_value = True + mock_validator = mock_validator_cls.return_value + mock_validator.is_valid.return_value = True + mock_validator.save.return_value = {"active_regions": ["default"]} + mock_preview.return_value = mock.Mock( + status_code=200, + json=mock.Mock(return_value={"status": 200}), + raise_for_status=mock.Mock(), + ) + mock_generate.return_value = (None, None) + + response = self.client.post(self.url, data=self.payload, format="json") + + assert response.status_code == 200 diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 1e9772df84e9..5bc538a76c36 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -568,17 +568,22 @@ def test_update_requires_alerts_write_scope_for_tokens(self) -> None: assert response.status_code == 403 - def test_update_rejects_org_write_scope_for_tokens(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public detector clients have migrated to alerts:write. + def test_update_allows_org_write_scope_for_tokens(self) -> None: token = self._create_token("org:write") - response = self.client.put( - reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), - data={"enabled": False}, - format="json", - HTTP_AUTHORIZATION=f"Bearer {token.token}", - ) + with self.tasks(): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) - assert response.status_code == 403 + assert response.status_code == 200 + self.detector.refresh_from_db() + assert self.detector.enabled is False def test_team_admin_can_update_with_project_scoped_alerts_write(self) -> None: team_admin_user = self.create_user(is_superuser=False) @@ -1193,6 +1198,30 @@ def test_update_generic_metrics_dataset_to_transactions(self) -> None: class OrganizationDetectorDetailsDeleteTest(OrganizationDetectorDetailsBaseTest): method = "DELETE" + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public detector clients have migrated to alerts:write. + def test_delete_allows_org_write_scope_for_tokens(self) -> None: + token = self._create_token("org:write") + + with outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 204 + + def test_delete_allows_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("alerts:write") + + with outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]), + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 204 + def test_team_admin_can_delete_with_project_scoped_alerts_write(self) -> None: team_admin_user = self.create_user(is_superuser=False) self.create_member( diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py index 85e6506b3d59..e82ad142f24e 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py @@ -3,6 +3,7 @@ import pytest from django.db.models import Q +from django.urls import reverse from sentry import audit_log from sentry.api.serializers import serialize @@ -13,9 +14,11 @@ from sentry.incidents.grouptype import MetricIssue from sentry.incidents.models.alert_rule import AlertRule from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE +from sentry.models.apitoken import ApiToken from sentry.models.environment import Environment from sentry.monitors.grouptype import MonitorIncidentType from sentry.search.utils import _HACKY_INVALID_USER +from sentry.silo.base import SiloMode from sentry.snuba.dataset import Dataset from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType from sentry.snuba.subscriptions import create_snuba_query, create_snuba_subscription @@ -24,7 +27,8 @@ from sentry.testutils.helpers.datetime import before_now from sentry.testutils.helpers.features import with_feature from sentry.testutils.outbox import outbox_runner -from sentry.testutils.silo import cell_silo_test +from sentry.testutils.skips import requires_kafka, requires_snuba +from sentry.testutils.silo import assume_test_silo_mode, cell_silo_test from sentry.testutils.skips import requires_kafka, requires_snuba from sentry.uptime.grouptype import UptimeDomainCheckFailure from sentry.uptime.types import ( @@ -63,6 +67,10 @@ def setUp(self) -> None: type=IssueStreamGroupType.slug, project=self.project, name="Issue Stream" ) + def _create_token(self, scope: str, user=None) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) + @cell_silo_test class OrganizationDetectorIndexGetTest(OrganizationDetectorIndexBaseTest): @@ -1022,6 +1030,36 @@ def setUp(self) -> None: organization=self.organization, ) + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public detector clients have migrated to alerts:write. + def test_update_detectors_allows_org_write_scope_for_tokens(self) -> None: + token = self._create_token("org:write") + + response = self.client.put( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.user_detector.id}", + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + self.user_detector.refresh_from_db() + assert self.user_detector.enabled is False + + def test_update_detectors_allows_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("alerts:write") + + response = self.client.put( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.user_detector.id}", + data={"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + self.user_detector.refresh_from_db() + assert self.user_detector.enabled is False + def test_update_detectors_by_ids_success(self) -> None: response = self.get_success_response( self.organization.slug, @@ -1343,6 +1381,30 @@ def setUp(self) -> None: project=self.project, name="Third Detector", type=MetricIssue.slug ) + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public detector clients have migrated to alerts:write. + def test_delete_detectors_allows_org_write_scope_for_tokens(self) -> None: + token = self._create_token("org:write") + + with outbox_runner(): + response = self.client.delete( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.detector.id}", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 204 + + def test_delete_detectors_allows_alerts_write_scope_for_tokens(self) -> None: + token = self._create_token("alerts:write") + + with outbox_runner(): + response = self.client.delete( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.detector.id}", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 204 + def test_delete_detectors_by_ids_success(self) -> None: """Test successful deletion of detectors by specific IDs""" with outbox_runner(): diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py index 21c161fbcc93..a8a345152580 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -620,7 +620,9 @@ def setUp(self) -> None: {"name": "assigneeId", "value": "3"}, ] - def test_create_requires_alerts_write_scope_for_api_keys(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public workflow clients have migrated to alerts:write. + def test_create_allows_legacy_org_write_scope_for_api_keys(self) -> None: response = self.client.post( reverse(self.endpoint, args=[self.organization.slug]), data=self.valid_workflow, @@ -628,7 +630,17 @@ def test_create_requires_alerts_write_scope_for_api_keys(self) -> None: **self._auth_headers("org:write"), ) - assert response.status_code == 403 + assert response.status_code == 201 + + def test_create_allows_alerts_write_scope_for_api_keys(self) -> None: + response = self.client.post( + reverse(self.endpoint, args=[self.organization.slug]), + data=self.valid_workflow, + format="json", + **self._auth_headers("alerts:write"), + ) + + assert response.status_code == 201 @mock.patch("sentry.workflow_engine.endpoints.validators.base.workflow.create_audit_entry") def test_create_workflow__basic(self, mock_audit: mock.MagicMock) -> None: @@ -1210,7 +1222,9 @@ def setUp(self) -> None: organization_id=self.organization.id, name="Third Workflow", enabled=False ) - def test_bulk_enable_requires_alerts_write_scope_for_api_keys(self) -> None: + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public workflow clients have migrated to alerts:write. + def test_bulk_enable_allows_legacy_org_write_scope_for_api_keys(self) -> None: response = self.client.put( f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", data={"enabled": True}, @@ -1218,7 +1232,21 @@ def test_bulk_enable_requires_alerts_write_scope_for_api_keys(self) -> None: **self._auth_headers("org:write"), ) - assert response.status_code == 403 + assert response.status_code == 200 + self.workflow.refresh_from_db() + assert self.workflow.enabled is True + + def test_bulk_enable_allows_alerts_write_scope_for_api_keys(self) -> None: + response = self.client.put( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", + data={"enabled": True}, + format="json", + **self._auth_headers("alerts:write"), + ) + + assert response.status_code == 200 + self.workflow.refresh_from_db() + assert self.workflow.enabled is True def test_bulk_enable_workflows_by_ids_success(self) -> None: response = self.get_success_response( @@ -1373,6 +1401,24 @@ def setUp(self) -> None: organization_id=self.organization.id, name="Third Workflow" ) + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public workflow clients have migrated to alerts:write. + def test_delete_workflows_allows_legacy_org_write_scope_for_api_keys(self) -> None: + response = self.client.delete( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", + **self._auth_headers("org:write"), + ) + + assert response.status_code == 204 + + def test_delete_workflows_allows_alerts_write_scope_for_api_keys(self) -> None: + response = self.client.delete( + f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", + **self._auth_headers("alerts:write"), + ) + + assert response.status_code == 204 + def test_delete_workflows_by_ids_success(self) -> None: """Test successful deletion of workflows by specific IDs""" with outbox_runner(): From 182a529f78a153e86e2e9c38f990390354523a6a Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 17 Apr 2026 10:26:18 -0700 Subject: [PATCH 19/25] fix(workflow-engine): Fix workflow type-check regressions Keep the write-scope compatibility follow-up clean under our backend type checker. Annotate the workflow permission override to match the base permission contract and align the workflow API-key test helpers with the DRF client stubs so the touched endpoint coverage stays type-safe. Co-Authored-By: Codex --- .../endpoints/organization_workflow_index.py | 11 +++++++++-- .../test_organization_detector_details.py | 3 ++- .../test_organization_detector_index.py | 4 ++-- .../test_organization_workflow_index.py | 16 ++++++++-------- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index c9e176beafba..b0a4d415d9aa 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -21,6 +21,7 @@ from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from rest_framework.response import Response +from rest_framework.views import APIView from sentry import audit_log from sentry.api.api_owners import ApiOwner @@ -52,6 +53,7 @@ from sentry.exceptions import InvalidSearchQuery from sentry.models.organization import Organization from sentry.models.project import Project +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.search.utils import parse_user_value from sentry.utils.audit import create_audit_entry from sentry.utils.dates import ensure_aware @@ -99,7 +101,7 @@ class OrganizationWorkflowPermission(OrganizationPermission): "DELETE": ["alerts:write"], } - def has_permission(self, request: Request, view) -> bool: + def has_permission(self, request: Request, view: APIView) -> bool: if super().has_permission(request, view): return True @@ -112,7 +114,12 @@ def has_permission(self, request: Request, view) -> bool: for scope in get_legacy_alert_mutation_scopes(view, request.method) ) - def has_object_permission(self, request: Request, view, organization) -> bool: + def has_object_permission( + self, + request: Request, + view: APIView, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ) -> bool: if super().has_object_permission(request, view, organization): return True diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 5bc538a76c36..f19170cb4b7a 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -1,4 +1,5 @@ from datetime import timedelta +from typing import Any from unittest import mock import pytest @@ -98,7 +99,7 @@ def setUp(self) -> None: ) assert self.detector.data_sources is not None - def _create_token(self, scope: str, user=None) -> ApiToken: + def _create_token(self, scope: str, user: Any | None = None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py index e82ad142f24e..78ee337bdcf3 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py @@ -1,5 +1,6 @@ from collections.abc import Sequence from datetime import timedelta +from typing import Any import pytest from django.db.models import Q @@ -27,7 +28,6 @@ from sentry.testutils.helpers.datetime import before_now from sentry.testutils.helpers.features import with_feature from sentry.testutils.outbox import outbox_runner -from sentry.testutils.skips import requires_kafka, requires_snuba from sentry.testutils.silo import assume_test_silo_mode, cell_silo_test from sentry.testutils.skips import requires_kafka, requires_snuba from sentry.uptime.grouptype import UptimeDomainCheckFailure @@ -67,7 +67,7 @@ def setUp(self) -> None: type=IssueStreamGroupType.slug, project=self.project, name="Issue Stream" ) - def _create_token(self, scope: str, user=None) -> ApiToken: + def _create_token(self, scope: str, user: Any | None = None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py index a8a345152580..371f8c3517ea 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -39,9 +39,9 @@ def setUp(self) -> None: super().setUp() self.login_as(user=self.user) - def _auth_headers(self, *scopes: str) -> dict[str, str]: + def _auth_header(self, *scopes: str) -> bytes: api_key = self.create_api_key(organization=self.organization, scope_list=list(scopes)) - return {"HTTP_AUTHORIZATION": self.create_basic_auth_header(api_key.key)} + return self.create_basic_auth_header(api_key.key) @cell_silo_test @@ -627,7 +627,7 @@ def test_create_allows_legacy_org_write_scope_for_api_keys(self) -> None: reverse(self.endpoint, args=[self.organization.slug]), data=self.valid_workflow, format="json", - **self._auth_headers("org:write"), + HTTP_AUTHORIZATION=self._auth_header("org:write"), ) assert response.status_code == 201 @@ -637,7 +637,7 @@ def test_create_allows_alerts_write_scope_for_api_keys(self) -> None: reverse(self.endpoint, args=[self.organization.slug]), data=self.valid_workflow, format="json", - **self._auth_headers("alerts:write"), + HTTP_AUTHORIZATION=self._auth_header("alerts:write"), ) assert response.status_code == 201 @@ -1229,7 +1229,7 @@ def test_bulk_enable_allows_legacy_org_write_scope_for_api_keys(self) -> None: f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", data={"enabled": True}, format="json", - **self._auth_headers("org:write"), + HTTP_AUTHORIZATION=self._auth_header("org:write"), ) assert response.status_code == 200 @@ -1241,7 +1241,7 @@ def test_bulk_enable_allows_alerts_write_scope_for_api_keys(self) -> None: f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", data={"enabled": True}, format="json", - **self._auth_headers("alerts:write"), + HTTP_AUTHORIZATION=self._auth_header("alerts:write"), ) assert response.status_code == 200 @@ -1406,7 +1406,7 @@ def setUp(self) -> None: def test_delete_workflows_allows_legacy_org_write_scope_for_api_keys(self) -> None: response = self.client.delete( f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", - **self._auth_headers("org:write"), + HTTP_AUTHORIZATION=self._auth_header("org:write"), ) assert response.status_code == 204 @@ -1414,7 +1414,7 @@ def test_delete_workflows_allows_legacy_org_write_scope_for_api_keys(self) -> No def test_delete_workflows_allows_alerts_write_scope_for_api_keys(self) -> None: response = self.client.delete( f"{reverse(self.endpoint, args=[self.organization.slug])}?id={self.workflow.id}", - **self._auth_headers("alerts:write"), + HTTP_AUTHORIZATION=self._auth_header("alerts:write"), ) assert response.status_code == 204 From 1b142d9fd5f898adb3c0b721b1f8ca464d0c5783 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 17 Apr 2026 11:22:41 -0700 Subject: [PATCH 20/25] fix(api): Document readonly mutation exceptions Add the missing readonly mutation scope notes for the project user issue and replay delete endpoints so the published-mutation permission audit reflects their existing behavior. While touching the alert permission code, extract the shared alert-mutation permission logic used by alert rules and detectors to keep future scope changes from drifting between the two classes. Co-Authored-By: Codex --- src/sentry/api/bases/organization.py | 50 ++----------------- .../issues/endpoints/project_user_issue.py | 6 +++ .../endpoints/project_replay_details.py | 6 +++ 3 files changed, 17 insertions(+), 45 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index c1f42b1982d9..26167adfedca 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -289,7 +289,7 @@ def get_legacy_alert_mutation_scopes(view: APIView, method: str | None) -> tuple return tuple(scopes) -class OrganizationAlertRulePermission(OrganizationPermission): +class OrganizationAlertMutationPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], "POST": ["alerts:write"], @@ -337,52 +337,12 @@ def has_object_permission( ) -class OrganizationDetectorPermission(OrganizationPermission): - scope_map = { - "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - "POST": ["alerts:write"], - "PUT": ["alerts:write"], - "DELETE": ["alerts:write"], - } - - def has_permission(self, request: Request, view: APIView) -> bool: - if super().has_permission(request, view): - return True - - if not request.auth: - return False - - current_scopes = request.auth.get_scopes() - return any( - scope in current_scopes - for scope in get_legacy_alert_mutation_scopes(view, request.method) - ) - - def has_object_permission( - self, - request: Request, - view: APIView, - organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ) -> bool: - if super().has_object_permission(request, view, organization): - return True - - if request.method not in {"POST", "PUT", "DELETE"}: - return False - - if any( - request.access.has_scope(scope) - for scope in get_legacy_alert_mutation_scopes(view, request.method) - ): - return True +class OrganizationAlertRulePermission(OrganizationAlertMutationPermission): + pass - project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) - if project_scoped_access is not None: - return project_scoped_access - return bool(getattr(view, "allow_any_team_alert_write_fallback", False)) and ( - _has_any_team_scope(request, "alerts:write") - ) +class OrganizationDetectorPermission(OrganizationAlertMutationPermission): + pass class OrgAuthTokenPermission(OrganizationPermission): diff --git a/src/sentry/issues/endpoints/project_user_issue.py b/src/sentry/issues/endpoints/project_user_issue.py index d8e6b07e51a6..18053299cec7 100644 --- a/src/sentry/issues/endpoints/project_user_issue.py +++ b/src/sentry/issues/endpoints/project_user_issue.py @@ -184,6 +184,12 @@ class ProjectUserIssuePermission(ProjectPermission): "PUT": [], "DELETE": [], } + readonly_mutation_scope_exceptions = { + "POST": ( + "User-defined issue creation derives a new issue from event data the caller can " + "already inspect, so it intentionally follows event read access." + ), + } class ProjectUserIssueResponseSerializer(serializers.Serializer): diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 24e2b8444390..fbc36807b4ad 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -25,6 +25,12 @@ class ReplayDetailsPermission(ProjectPermission): "PUT": ["project:write", "project:admin"], "DELETE": ["project:read", "project:write", "project:admin"], } + readonly_mutation_scope_exceptions = { + "DELETE": ( + "Replay deletion is a public API and has historically allowed project:read for " + "compatible replay-managing clients, so tightening it would be user-impacting." + ), + } @cell_silo_endpoint From 3b6479f901e431e0683690615006a3fc83ccfca1 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 17 Apr 2026 12:28:43 -0700 Subject: [PATCH 21/25] ref(api): Reuse alerting mutation permissions Rename the shared alerting mutation permission to reflect the broader alerting surface and reuse it for workflow endpoints. This removes the remaining duplicated workflow permission logic without changing behavior, since workflow endpoints do not define the extra project or team fallbacks handled by the shared implementation. Co-Authored-By: Codex --- src/sentry/api/bases/organization.py | 6 +-- .../endpoints/organization_workflow_index.py | 44 ++----------------- 2 files changed, 6 insertions(+), 44 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 26167adfedca..ebf3c11a27b9 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -289,7 +289,7 @@ def get_legacy_alert_mutation_scopes(view: APIView, method: str | None) -> tuple return tuple(scopes) -class OrganizationAlertMutationPermission(OrganizationPermission): +class OrganizationAlertingMutationPermission(OrganizationPermission): scope_map = { "GET": ["org:read", "org:write", "org:admin", "alerts:read"], "POST": ["alerts:write"], @@ -337,11 +337,11 @@ def has_object_permission( ) -class OrganizationAlertRulePermission(OrganizationAlertMutationPermission): +class OrganizationAlertRulePermission(OrganizationAlertingMutationPermission): pass -class OrganizationDetectorPermission(OrganizationAlertMutationPermission): +class OrganizationDetectorPermission(OrganizationAlertingMutationPermission): pass diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index b0a4d415d9aa..b542768884d3 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -21,7 +21,6 @@ from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from rest_framework.response import Response -from rest_framework.views import APIView from sentry import audit_log from sentry.api.api_owners import ApiOwner @@ -29,8 +28,7 @@ from sentry.api.base import cell_silo_endpoint from sentry.api.bases import OrganizationEndpoint from sentry.api.bases.organization import ( - OrganizationPermission, - get_legacy_alert_mutation_scopes, + OrganizationAlertingMutationPermission, ) from sentry.api.event_search import SearchConfig, SearchFilter, SearchKey, default_config from sentry.api.event_search import parse_search_query as base_parse_search_query @@ -53,7 +51,6 @@ from sentry.exceptions import InvalidSearchQuery from sentry.models.organization import Organization from sentry.models.project import Project -from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.search.utils import parse_user_value from sentry.utils.audit import create_audit_entry from sentry.utils.dates import ensure_aware @@ -93,43 +90,8 @@ parse_workflow_query = partial(base_parse_search_query, config=workflow_search_config) -class OrganizationWorkflowPermission(OrganizationPermission): - scope_map = { - "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - "POST": ["alerts:write"], - "PUT": ["alerts:write"], - "DELETE": ["alerts:write"], - } - - def has_permission(self, request: Request, view: APIView) -> bool: - if super().has_permission(request, view): - return True - - if not request.auth: - return False - - current_scopes = request.auth.get_scopes() - return any( - scope in current_scopes - for scope in get_legacy_alert_mutation_scopes(view, request.method) - ) - - def has_object_permission( - self, - request: Request, - view: APIView, - organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ) -> bool: - if super().has_object_permission(request, view, organization): - return True - - if request.method not in {"POST", "PUT", "DELETE"}: - return False - - return any( - request.access.has_scope(scope) - for scope in get_legacy_alert_mutation_scopes(view, request.method) - ) +class OrganizationWorkflowPermission(OrganizationAlertingMutationPermission): + pass class OrganizationWorkflowEndpoint(OrganizationEndpoint): From 440ad5a17ff7b089f19c2cc465b57f203ccec089 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 17 Apr 2026 12:41:27 -0700 Subject: [PATCH 22/25] fix(api): Preserve workflow detail legacy scopes Restore the legacy org:* compatibility scopes for workflow detail mutations while keeping the shared alerting permission base in place. Without this, API keys that still rely on org:write or org:admin could create or bulk update workflows through the index endpoint but would get 403 on detail PUT and DELETE. Add detail endpoint regression tests for both legacy and alerts:write API key auth paths. Co-Authored-By: Codex --- .../endpoints/organization_workflow_index.py | 6 +++ .../test_organization_workflow_details.py | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index b542768884d3..4b9e7eb3c4a4 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -95,6 +95,12 @@ class OrganizationWorkflowPermission(OrganizationAlertingMutationPermission): class OrganizationWorkflowEndpoint(OrganizationEndpoint): + # TODO(api-write-scope-compat): Remove legacy org:* support once public + # workflow clients have migrated to alerts:write. + legacy_alert_mutation_scope_map = { + "PUT": ("org:write", "org:admin"), + "DELETE": ("org:write", "org:admin"), + } permission_classes = (OrganizationWorkflowPermission,) def convert_args( diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py index 4d1b6797ea82..91bef983cf6e 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py @@ -1,6 +1,7 @@ from contextlib import AbstractContextManager import responses +from django.urls import reverse from sentry import audit_log from sentry.api.serializers import serialize @@ -35,6 +36,10 @@ def setUp(self) -> None: super().setUp() self.login_as(user=self.user) + def _auth_header(self, *scopes: str) -> bytes: + api_key = self.create_api_key(organization=self.organization, scope_list=list(scopes)) + return self.create_basic_auth_header(api_key.key) + @cell_silo_test class OrganizationWorkflowIndexGetTest(OrganizationWorkflowDetailsBaseTest): @@ -82,6 +87,32 @@ def setUp(self) -> None: {"name": "assigneeId", "value": "3"}, ] + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public workflow clients have migrated to alerts:write. + def test_update_allows_legacy_org_write_scope_for_api_keys(self) -> None: + updated_workflow = {**self.valid_workflow, "name": "Updated Workflow"} + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.workflow.id]), + data=updated_workflow, + format="json", + HTTP_AUTHORIZATION=self._auth_header("org:write"), + ) + + assert response.status_code == 200 + + def test_update_allows_alerts_write_scope_for_api_keys(self) -> None: + updated_workflow = {**self.valid_workflow, "name": "Updated Workflow"} + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, self.workflow.id]), + data=updated_workflow, + format="json", + HTTP_AUTHORIZATION=self._auth_header("alerts:write"), + ) + + assert response.status_code == 200 + def test_simple(self) -> None: self.valid_workflow["name"] = "Updated Workflow" response = self.get_success_response( @@ -795,6 +826,26 @@ def setUp(self) -> None: super().setUp() self.workflow = self.create_workflow(organization_id=self.organization.id) + # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # public workflow clients have migrated to alerts:write. + def test_delete_allows_legacy_org_write_scope_for_api_keys(self) -> None: + with outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.workflow.id]), + HTTP_AUTHORIZATION=self._auth_header("org:write"), + ) + + assert response.status_code == 204 + + def test_delete_allows_alerts_write_scope_for_api_keys(self) -> None: + with outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.workflow.id]), + HTTP_AUTHORIZATION=self._auth_header("alerts:write"), + ) + + assert response.status_code == 204 + def test_simple(self) -> None: with outbox_runner(): self.get_success_response(self.organization.slug, self.workflow.id) From c2143781d5a5d8b260406b9d88d60bfeed39a1ab Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 20 Apr 2026 09:58:30 -0700 Subject: [PATCH 23/25] style(workflow-engine): Remove duplicate detector import Keep the detector details endpoint lint-clean after the rebase conflict resolution. Ruff removed the duplicate SnubaQuery import, so capture that hook-driven cleanup as a standalone follow-up instead of leaving the branch dirty. Co-Authored-By: OpenAI Codex --- .../workflow_engine/endpoints/organization_detector_details.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index b5e3f77af2e5..0ab7c36c4664 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -32,7 +32,6 @@ from sentry.issues import grouptype from sentry.models.organization import Organization from sentry.models.project import Project -from sentry.snuba.models import SnubaQuery from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.snuba.models import SnubaQuery from sentry.utils.audit import create_audit_entry From 6d54948eaf8967cb6adba2835727af0c173f1b3f Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 27 Apr 2026 10:39:36 -0700 Subject: [PATCH 24/25] fix(api): Tighten alert and event mutation scopes Move alert authoring compatibility away from readonly scopes and align related preview, workflow, detector, and monitor endpoints on alerts:write. Keep only explicit legacy write/admin compatibility where needed. Require event write/admin for user-created issues and event admin for replay deletion so event read and project read scopes no longer authorize these published mutations. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/organization.py | 4 +- .../organization_alert_rule_details.py | 2 +- .../organization_alert_rule_index.py | 2 +- .../issues/endpoints/project_user_issue.py | 12 ++---- .../endpoints/organization_monitor_index.py | 4 +- .../endpoints/project_replay_details.py | 8 +--- .../organization_events_anomalies.py | 4 +- ...organization_uptime_alert_preview_check.py | 4 +- ...ganization_uptime_assertion_suggestions.py | 4 +- .../organization_detector_details.py | 4 +- .../endpoints/organization_detector_index.py | 4 +- .../endpoints/organization_workflow_index.py | 10 ++--- .../endpoints/validators/utils.py | 5 +-- tests/sentry/api/test_permissions.py | 28 +++++++++++++ .../endpoints/test_project_user_issue.py | 32 +++++++-------- .../endpoints/test_project_replay_details.py | 40 +++++++++---------- .../test_organization_events_anomalies.py | 8 ++-- .../test_organization_uptime_alert_preview.py | 8 ++-- ...ganization_uptime_assertion_suggestions.py | 8 +--- 19 files changed, 96 insertions(+), 95 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index ebf3c11a27b9..0a5daab0eb94 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -252,7 +252,7 @@ def _has_any_team_scope(request: Request, scope: str) -> bool: # Project-scoped alert authoring should rely on the alert-specific write scope. ALERT_MUTATION_SCOPES = frozenset({"alerts:write"}) -LEGACY_ALERT_MUTATION_PROJECT_SCOPES = ("project:read", "org:write", "alerts:write") +LEGACY_ALERT_MUTATION_PROJECT_SCOPES = ("org:write", "alerts:write") def _has_project_alert_write_access(request: Request, projects: Sequence[Project]) -> bool: @@ -291,7 +291,7 @@ def get_legacy_alert_mutation_scopes(view: APIView, method: str | None) -> tuple class OrganizationAlertingMutationPermission(OrganizationPermission): scope_map = { - "GET": ["org:read", "org:write", "org:admin", "alerts:read"], + "GET": ["org:read", "org:write", "org:admin", "alerts:read", "alerts:write"], "POST": ["alerts:write"], "PUT": ["alerts:write"], "DELETE": ["alerts:write"], diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index d3793b0e41a7..9d6774e612c3 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -221,7 +221,7 @@ class OrganizationAlertRuleDetailsPutSerializer(serializers.Serializer): help_text="The time period to aggregate over.", ) projects = serializers.ListField( - child=ProjectField(scope="project:read"), + child=ProjectField(scope="alerts:write"), help_text="The names of the projects to filter by.", ) query = serializers.CharField( diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index f0c6c2f82462..f24d66da18bf 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -824,7 +824,7 @@ class OrganizationAlertRuleIndexPostSerializer(serializers.Serializer): ) # projects is not required in the serializer, however, the UI requires a project is chosen projects = serializers.ListField( - child=ProjectField(scope="project:read"), + child=ProjectField(scope="alerts:write"), help_text="Metric alerts are currently limited to one project. The array should contain a single slug, representing the project to filter by.", ) query = serializers.CharField( diff --git a/src/sentry/issues/endpoints/project_user_issue.py b/src/sentry/issues/endpoints/project_user_issue.py index 18053299cec7..b053c30abc75 100644 --- a/src/sentry/issues/endpoints/project_user_issue.py +++ b/src/sentry/issues/endpoints/project_user_issue.py @@ -10,7 +10,7 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.project import ProjectEndpoint, ProjectPermission +from sentry.api.bases.project import ProjectEndpoint, ProjectEventPermission from sentry.apidocs.parameters import GlobalParams from sentry.grouping.grouptype import ErrorGroupType from sentry.issues.grouptype import GroupType, WebVitalsGroup @@ -177,19 +177,13 @@ class WebVitalsIssueDataSerializer(ProjectUserIssueRequestSerializer): value = serializers.IntegerField(required=True) -class ProjectUserIssuePermission(ProjectPermission): +class ProjectUserIssuePermission(ProjectEventPermission): scope_map = { "GET": [], - "POST": ["event:read", "event:write", "event:admin"], + "POST": ["event:write", "event:admin"], "PUT": [], "DELETE": [], } - readonly_mutation_scope_exceptions = { - "POST": ( - "User-defined issue creation derives a new issue from event data the caller can " - "already inspect, so it intentionally follows event read access." - ), - } class ProjectUserIssueResponseSerializer(serializers.Serializer): diff --git a/src/sentry/monitors/endpoints/organization_monitor_index.py b/src/sentry/monitors/endpoints/organization_monitor_index.py index f3f00fc95c36..6e0a7389d282 100644 --- a/src/sentry/monitors/endpoints/organization_monitor_index.py +++ b/src/sentry/monitors/endpoints/organization_monitor_index.py @@ -94,8 +94,8 @@ class OrganizationMonitorIndexEndpoint(OrganizationAlertRuleBaseEndpoint): # TODO(api-write-scope-compat): Remove legacy org:write support once public # cron monitor clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "POST": ("org:write", "org:admin"), - "PUT": ("org:write", "org:admin"), + "POST": ("org:write", "alerts:write"), + "PUT": ("org:write", "alerts:write"), } permission_classes = (OrganizationAlertRulePermission,) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index fbc36807b4ad..0d0f592fcf4b 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -23,13 +23,7 @@ class ReplayDetailsPermission(ProjectPermission): "GET": ["project:read", "project:write", "project:admin"], "POST": ["project:write", "project:admin"], "PUT": ["project:write", "project:admin"], - "DELETE": ["project:read", "project:write", "project:admin"], - } - readonly_mutation_scope_exceptions = { - "DELETE": ( - "Replay deletion is a public API and has historically allowed project:read for " - "compatible replay-managing clients, so tightening it would be user-impacting." - ), + "DELETE": ["event:admin"], } diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 6ecb14bc4ead..cc2e565454df 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -36,10 +36,10 @@ class OrganizationEventsAnomaliesEndpoint(OrganizationEventsEndpointBase): "POST": ApiPublishStatus.EXPERIMENTAL, } allow_any_team_alert_write_fallback = True - # TODO(api-write-scope-compat): Remove legacy org:* support once alert + # TODO(api-write-scope-compat): Remove legacy org:write support once alert # authoring preview clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "POST": ("org:read", "org:write", "org:admin"), + "POST": ("org:write", "alerts:write"), } # This POST previews anomaly-detection config used while authoring metric # alerts/detectors, so it intentionally follows alert-write permissions. diff --git a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py index fea53b54ec50..b9b1da50040b 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py +++ b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py @@ -31,10 +31,10 @@ class OrganizationUptimeAlertPreviewCheckEndpoint(OrganizationEndpoint): owner = ApiOwner.CRONS allow_any_team_alert_write_fallback = True - # TODO(api-write-scope-compat): Remove legacy org:* support once uptime + # TODO(api-write-scope-compat): Remove legacy org:write support once uptime # preview clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "POST": ("org:read", "org:write", "org:admin"), + "POST": ("org:write", "alerts:write"), } # This POST previews monitor creation and validation, so it intentionally # uses the same permission surface as creating the alert itself. diff --git a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py index e1226f7c3f89..347504a6902c 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py +++ b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py @@ -51,10 +51,10 @@ class OrganizationUptimeAssertionSuggestionsEndpoint(OrganizationEndpoint): owner = ApiOwner.CRONS allow_any_team_alert_write_fallback = True - # TODO(api-write-scope-compat): Remove legacy org:* support once uptime + # TODO(api-write-scope-compat): Remove legacy org:write support once uptime # assertion-suggestion clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "POST": ("org:read", "org:write", "org:admin"), + "POST": ("org:write", "alerts:write"), } # This POST is part of the uptime monitor authoring flow, so it should # track the same alert-write permission as the monitor it helps create. diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 0ab7c36c4664..b9a1334e6d4c 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -129,8 +129,8 @@ class OrganizationDetectorDetailsEndpoint(OrganizationEndpoint): # TODO(api-write-scope-compat): Remove legacy org:* support once public # detector clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "PUT": ("org:read", "org:write", "org:admin"), - "DELETE": ("org:read", "org:write", "org:admin"), + "PUT": ("org:write", "org:admin", "alerts:write"), + "DELETE": ("org:write", "org:admin", "alerts:write"), } def get_alert_mutation_projects( diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_index.py b/src/sentry/workflow_engine/endpoints/organization_detector_index.py index 9ca48612909c..fbc82a274b20 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_index.py @@ -157,8 +157,8 @@ class OrganizationDetectorIndexEndpoint(OrganizationEndpoint): # TODO(api-write-scope-compat): Remove legacy org:* support once public # detector clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "PUT": ("org:read", "org:write", "org:admin"), - "DELETE": ("org:read", "org:write", "org:admin"), + "PUT": ("org:write", "org:admin", "alerts:write"), + "DELETE": ("org:write", "org:admin", "alerts:write"), } permission_classes = (OrganizationDetectorPermission,) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index 4b9e7eb3c4a4..f91a7749db99 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -98,8 +98,8 @@ class OrganizationWorkflowEndpoint(OrganizationEndpoint): # TODO(api-write-scope-compat): Remove legacy org:* support once public # workflow clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "PUT": ("org:write", "org:admin"), - "DELETE": ("org:write", "org:admin"), + "PUT": ("org:write", "org:admin", "alerts:write"), + "DELETE": ("org:write", "org:admin", "alerts:write"), } permission_classes = (OrganizationWorkflowPermission,) @@ -147,9 +147,9 @@ class OrganizationWorkflowIndexEndpoint(OrganizationEndpoint): # TODO(api-write-scope-compat): Remove legacy org:* support once public # workflow clients have migrated to alerts:write. legacy_alert_mutation_scope_map = { - "POST": ("org:write", "org:admin"), - "PUT": ("org:write", "org:admin"), - "DELETE": ("org:write", "org:admin"), + "POST": ("org:write", "org:admin", "alerts:write"), + "PUT": ("org:write", "org:admin", "alerts:write"), + "DELETE": ("org:write", "org:admin", "alerts:write"), } permission_classes = (OrganizationWorkflowPermission,) diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index af93ea9439f7..ca258250b97c 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -26,10 +26,9 @@ logger = logging.getLogger(__name__) -SYSTEM_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"org:write"}) # TODO(api-write-scope-compat): Remove legacy org:write support for -# user-created detector edits once public detector clients have migrated to -# alerts:write. +# detector edits once public detector clients have migrated to alerts:write. +SYSTEM_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"org:write", "alerts:write"}) USER_CREATED_DETECTOR_EDIT_SCOPES = frozenset({"org:write", "alerts:write"}) diff --git a/tests/sentry/api/test_permissions.py b/tests/sentry/api/test_permissions.py index ca34dfd0177b..e2f5f336e5cf 100644 --- a/tests/sentry/api/test_permissions.py +++ b/tests/sentry/api/test_permissions.py @@ -6,6 +6,11 @@ from rest_framework.views import APIView from sentry.api.base import Endpoint +from sentry.api.bases.organization import ( + ALERT_MUTATION_SCOPES, + LEGACY_ALERT_MUTATION_PROJECT_SCOPES, + OrganizationAlertingMutationPermission, +) from sentry.api.permissions import ( DemoSafePermission, DisallowImpersonatedTokenCreation, @@ -255,6 +260,29 @@ def test_determine_access_no_demo_users(self) -> None: @no_silo_test class PublishedMutationScopeTest(SimpleTestCase): + def test_alert_mutation_scope_constants_are_alert_specific(self) -> None: + assert ALERT_MUTATION_SCOPES == {"alerts:write"} + assert set(LEGACY_ALERT_MUTATION_PROJECT_SCOPES) & SENTRY_READONLY_SCOPES == set() + assert "alerts:write" in OrganizationAlertingMutationPermission.scope_map["GET"] + + def test_legacy_alert_mutation_scope_maps_do_not_accept_readonly_scopes(self) -> None: + invalid_scopes = [] + + for view_cls in sorted( + set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path + ): + legacy_scope_map = getattr(view_cls, "legacy_alert_mutation_scope_map", {}) or {} + + for method, scopes in legacy_scope_map.items(): + readonly_scopes = set(scopes) & SENTRY_READONLY_SCOPES + if readonly_scopes: + invalid_scopes.append( + f"{_get_class_path(view_cls)} {method} legacy alert mutation " + f"compat accepts readonly scopes {sorted(readonly_scopes)}." + ) + + assert not invalid_scopes, "\n".join(invalid_scopes) + def test_readonly_mutation_scope_exceptions_are_notes(self) -> None: for view_cls in sorted( set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path diff --git a/tests/sentry/issues/endpoints/test_project_user_issue.py b/tests/sentry/issues/endpoints/test_project_user_issue.py index d2ab96002641..75c741085d6a 100644 --- a/tests/sentry/issues/endpoints/test_project_user_issue.py +++ b/tests/sentry/issues/endpoints/test_project_user_issue.py @@ -117,27 +117,23 @@ def test_no_access(self) -> None: assert response.status_code == 404 @with_feature("organizations:performance-web-vitals-seer-suggestions") - def test_create_web_vitals_issue_allows_event_read_scope(self) -> None: + def test_create_web_vitals_issue_denies_event_read_scope(self) -> None: token = self._create_token("event:read") - with patch( - "sentry.issues.endpoints.project_user_issue.produce_occurrence_to_kafka" - ) as mock_produce: - response = self.client.post( - self.url, - data={ - "transaction": "/test-transaction", - "issueType": WebVitalsGroup.slug, - "score": 75, - "value": 1000, - "vital": "lcp", - }, - format="json", - HTTP_AUTHORIZATION=f"Bearer {token.token}", - ) + response = self.client.post( + self.url, + data={ + "transaction": "/test-transaction", + "issueType": WebVitalsGroup.slug, + "score": 75, + "value": 1000, + "vital": "lcp", + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) - assert response.status_code == 200 - assert response.data == {"event_id": mock_produce.call_args[1]["occurrence"].event_id} + assert response.status_code == 403 @with_feature("organizations:performance-web-vitals-seer-suggestions") def test_create_web_vitals_issue_allows_event_write_scope(self) -> None: diff --git a/tests/sentry/replays/endpoints/test_project_replay_details.py b/tests/sentry/replays/endpoints/test_project_replay_details.py index 284bcf87fb65..3131a380dd4d 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_details.py +++ b/tests/sentry/replays/endpoints/test_project_replay_details.py @@ -206,29 +206,27 @@ def test_delete_denies_event_read_scope_for_api_tokens(self) -> None: assert response.status_code == 403 - def test_delete_allows_project_read_scope_for_api_tokens(self) -> None: + def test_delete_denies_project_read_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["project:read"]) with self.feature(REPLAYS_FEATURES): - with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): - response = self.client.delete( - self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" - ) + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) - assert response.status_code == 204 + assert response.status_code == 403 - def test_delete_allows_project_write_scope_for_api_tokens(self) -> None: + def test_delete_denies_project_write_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["project:write"]) with self.feature(REPLAYS_FEATURES): - with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): - response = self.client.delete( - self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" - ) + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) - assert response.status_code == 204 + assert response.status_code == 403 def test_delete_denies_event_write_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): @@ -242,18 +240,19 @@ def test_delete_denies_event_write_scope_for_api_tokens(self) -> None: assert response.status_code == 403 - def test_delete_denies_event_admin_scope_for_api_tokens(self) -> None: + def test_delete_allows_event_admin_scope_for_api_tokens(self) -> None: with assume_test_silo_mode(SiloMode.CONTROL): token = ApiToken.objects.create(user=self.user, scope_list=["event:admin"]) with self.feature(REPLAYS_FEATURES): - response = self.client.delete( - self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" - ) + with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): + response = self.client.delete( + self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}", format="json" + ) - assert response.status_code == 403 + assert response.status_code == 204 - def test_delete_members_do_not_require_event_admin(self) -> None: + def test_delete_denies_members_without_event_admin(self) -> None: self.organization.update_option("sentry:events_member_admin", False) member_user = self.create_user(is_superuser=False) @@ -263,10 +262,9 @@ def test_delete_members_do_not_require_event_admin(self) -> None: self.login_as(user=member_user) with self.feature(REPLAYS_FEATURES): - with patch("sentry.replays.endpoints.project_replay_details.delete_replay.delay"): - response = self.client.delete(self.url) + response = self.client.delete(self.url) - assert response.status_code == 204 + assert response.status_code == 403 def test_delete_replay_from_clickhouse_data(self) -> None: """Test deleting files uploaded through the direct storage interface.""" diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 056dd95f3c83..fe493ec11f40 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -241,12 +241,10 @@ def test_alerts_write_scope_allows_post(self, mock_seer_request: MagicMock) -> N @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") - # TODO(api-write-scope-compat): Remove this legacy org:* coverage once - # alert authoring preview clients have migrated to alerts:write. @patch( "sentry.seer.anomaly_detection.get_historical_anomalies.seer_anomaly_detection_connection_pool.urlopen" ) - def test_org_read_scope_can_post(self, mock_seer_request: MagicMock) -> None: + def test_org_read_scope_cannot_post(self, mock_seer_request: MagicMock) -> None: mock_seer_request.return_value = HTTPResponse( orjson.dumps( DetectAnomaliesResponse( @@ -275,11 +273,11 @@ def test_org_read_scope_can_post(self, mock_seer_request: MagicMock) -> None: HTTP_AUTHORIZATION=f"Bearer {token.token}", ) - assert response.status_code == 200 + assert response.status_code == 403 @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") - # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once # alert authoring preview clients have migrated to alerts:write. @patch( "sentry.seer.anomaly_detection.get_historical_anomalies.seer_anomaly_detection_connection_pool.urlopen" diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py index 71ad841d7efa..88c031bc53ae 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py @@ -135,10 +135,8 @@ def test_alerts_write_permission(self) -> None: assert response.status_code == 200, response.content - # TODO(api-write-scope-compat): Remove this legacy org:* coverage once - # uptime preview clients have migrated to alerts:write. @responses.activate - def test_org_read_scope_can_run_preview_check(self) -> None: + def test_org_read_scope_cannot_run_preview_check(self) -> None: api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) responses.add( @@ -173,9 +171,9 @@ def test_org_read_scope_can_run_preview_check(self) -> None: HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), ) - assert response.status_code == 200 + assert response.status_code == 403 - # TODO(api-write-scope-compat): Remove this legacy org:* coverage once + # TODO(api-write-scope-compat): Remove this legacy org:write coverage once # uptime preview clients have migrated to alerts:write. @responses.activate def test_org_write_scope_can_run_preview_check(self) -> None: diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py index b73bae922980..170a8c2a3b92 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py @@ -34,8 +34,6 @@ def setUp(self) -> None: "region": "default", } - # TODO(api-write-scope-compat): Remove this legacy org:* coverage once - # uptime assertion-suggestion clients have migrated to alerts:write. @mock.patch( "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" ) @@ -74,8 +72,6 @@ def test_alerts_write_scope_can_generate_suggestions( assert response.status_code == 200 - # TODO(api-write-scope-compat): Remove this legacy org:* coverage once - # uptime assertion-suggestion clients have migrated to alerts:write. @mock.patch( "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" ) @@ -86,7 +82,7 @@ def test_alerts_write_scope_can_generate_suggestions( "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.UptimeCheckPreviewValidator" ) @mock.patch("sentry.uptime.endpoints.organization_uptime_assertion_suggestions.has_seer_access") - def test_org_read_scope_can_generate_suggestions( + def test_org_read_scope_cannot_generate_suggestions( self, mock_has_seer_access: mock.MagicMock, mock_validator_cls: mock.MagicMock, @@ -112,7 +108,7 @@ def test_org_read_scope_can_generate_suggestions( HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), ) - assert response.status_code == 200 + assert response.status_code == 403 @mock.patch( "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" From 42fcd49463cc45c03c3a84f70b8392bf8b53c0c8 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 27 Apr 2026 11:35:13 -0700 Subject: [PATCH 25/25] docs(api): Mark personal scope cleanup follow-up Add a grepable TODO to personal preference and member-state mutation exceptions that still use readonly org or member scopes. These remain compatibility notes for this PR, but should move to a narrow personal write scope once the API scope taxonomy supports one. Refs getsentry/getsentry#19897 Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/organization.py | 4 ++++ src/sentry/api/endpoints/organization_recent_searches.py | 2 ++ src/sentry/api/endpoints/prompts_activity.py | 2 ++ .../dashboards/endpoints/organization_dashboards_starred.py | 2 ++ src/sentry/discover/endpoints/bases.py | 2 ++ src/sentry/explore/endpoints/bases.py | 2 ++ src/sentry/explore/endpoints/explore_saved_query_starred.py | 2 ++ .../explore/endpoints/explore_saved_query_starred_order.py | 2 ++ src/sentry/insights/endpoints/starred_segments.py | 2 ++ src/sentry/issues/endpoints/bases/group_search_view.py | 2 ++ .../organization_group_search_view_details_starred.py | 2 ++ .../endpoints/organization_group_search_view_starred_order.py | 2 ++ .../issues/endpoints/organization_group_search_view_visit.py | 2 ++ .../issues/endpoints/organization_group_search_views.py | 2 ++ 14 files changed, 30 insertions(+) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 0a5daab0eb94..e46e60f47565 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -206,6 +206,8 @@ class OrganizationPinnedSearchPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move personal saved/pinned search writes off + # readonly org scopes once we add a narrow member/personal preference write scope. readonly_mutation_scope_exceptions = { "PUT": "Pinned search changes only update the requesting member's personal pinned state.", "DELETE": "Pinned search removal only updates the requesting member's personal pinned state.", @@ -219,6 +221,8 @@ class OrganizationSearchPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move personal saved-search writes off readonly + # org scopes once we add a narrow member/personal preference write scope. readonly_mutation_scope_exceptions = { "POST": "Members may manage personal saved searches without org:write.", "PUT": "Members may edit personal saved searches without org:write.", diff --git a/src/sentry/api/endpoints/organization_recent_searches.py b/src/sentry/api/endpoints/organization_recent_searches.py index 64b23a1dd1d3..d0ae78b2c746 100644 --- a/src/sentry/api/endpoints/organization_recent_searches.py +++ b/src/sentry/api/endpoints/organization_recent_searches.py @@ -22,6 +22,8 @@ class OrganizationRecentSearchPermission(OrganizationPermission): "GET": ["org:read", "org:write", "org:admin"], "POST": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move recent-search writes off readonly org + # scopes once we add a narrow member/personal preference write scope. readonly_mutation_scope_exceptions = { "POST": "Recent search writes only affect the requesting member's history.", } diff --git a/src/sentry/api/endpoints/prompts_activity.py b/src/sentry/api/endpoints/prompts_activity.py index 08237e98d8e2..e485afe20255 100644 --- a/src/sentry/api/endpoints/prompts_activity.py +++ b/src/sentry/api/endpoints/prompts_activity.py @@ -40,6 +40,8 @@ class PromptsActivityPermission(OrganizationPermission): "GET": ["org:read", "org:write", "org:admin"], "PUT": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move prompt preference writes off readonly + # org scopes once we add a narrow member/personal preference write scope. readonly_mutation_scope_exceptions = { "PUT": "Prompt activity updates only change the requesting member's prompt state.", } diff --git a/src/sentry/dashboards/endpoints/organization_dashboards_starred.py b/src/sentry/dashboards/endpoints/organization_dashboards_starred.py index d390cb84ba9a..6e0494092b02 100644 --- a/src/sentry/dashboards/endpoints/organization_dashboards_starred.py +++ b/src/sentry/dashboards/endpoints/organization_dashboards_starred.py @@ -23,6 +23,8 @@ class MemberPermission(OrganizationPermission): "GET": ["member:read", "member:write"], "PUT": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move dashboard favorite ordering off + # readonly member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "PUT": "Starred dashboard ordering only updates the requesting member's preferences.", } diff --git a/src/sentry/discover/endpoints/bases.py b/src/sentry/discover/endpoints/bases.py index 953bc7fdbe34..c31ce3434636 100644 --- a/src/sentry/discover/endpoints/bases.py +++ b/src/sentry/discover/endpoints/bases.py @@ -11,6 +11,8 @@ class DiscoverSavedQueryPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move saved-query writes off readonly org + # scopes once we add a narrow member/personal query write scope. readonly_mutation_scope_exceptions = { "POST": "Discover saved query mutations remain available for personal or otherwise permitted queries.", "PUT": "Discover saved query mutations remain available for personal or otherwise permitted queries.", diff --git a/src/sentry/explore/endpoints/bases.py b/src/sentry/explore/endpoints/bases.py index 36b95f754126..ad01da73e782 100644 --- a/src/sentry/explore/endpoints/bases.py +++ b/src/sentry/explore/endpoints/bases.py @@ -11,6 +11,8 @@ class ExploreSavedQueryPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move saved-query writes off readonly org + # scopes once we add a narrow member/personal query write scope. readonly_mutation_scope_exceptions = { "POST": "Explore saved query mutations remain available for personal or otherwise permitted queries.", "PUT": "Explore saved query mutations remain available for personal or otherwise permitted queries.", diff --git a/src/sentry/explore/endpoints/explore_saved_query_starred.py b/src/sentry/explore/endpoints/explore_saved_query_starred.py index c0c9a284c9c8..29be083494c4 100644 --- a/src/sentry/explore/endpoints/explore_saved_query_starred.py +++ b/src/sentry/explore/endpoints/explore_saved_query_starred.py @@ -25,6 +25,8 @@ class MemberPermission(OrganizationPermission): scope_map = { "POST": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move saved-query starring off readonly + # member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "POST": "Starring an Explore query only changes the requesting member's preferences.", } diff --git a/src/sentry/explore/endpoints/explore_saved_query_starred_order.py b/src/sentry/explore/endpoints/explore_saved_query_starred_order.py index 0ff990771f8a..493558d31f5a 100644 --- a/src/sentry/explore/endpoints/explore_saved_query_starred_order.py +++ b/src/sentry/explore/endpoints/explore_saved_query_starred_order.py @@ -17,6 +17,8 @@ class MemberPermission(OrganizationPermission): scope_map = { "PUT": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move saved-query starring order off + # readonly member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "PUT": "Explore starred query ordering only changes the requesting member's preferences.", } diff --git a/src/sentry/insights/endpoints/starred_segments.py b/src/sentry/insights/endpoints/starred_segments.py index 92b7f9cc760f..f5c52999644a 100644 --- a/src/sentry/insights/endpoints/starred_segments.py +++ b/src/sentry/insights/endpoints/starred_segments.py @@ -23,6 +23,8 @@ class MemberPermission(OrganizationPermission): "POST": ["member:read", "member:write"], "DELETE": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move insights segment starring off + # readonly member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "POST": "Starring an insights segment only changes the requesting member's preferences.", "DELETE": "Unstarring an insights segment only changes the requesting member's preferences.", diff --git a/src/sentry/issues/endpoints/bases/group_search_view.py b/src/sentry/issues/endpoints/bases/group_search_view.py index c229e27a1f54..04cecfe45a0a 100644 --- a/src/sentry/issues/endpoints/bases/group_search_view.py +++ b/src/sentry/issues/endpoints/bases/group_search_view.py @@ -14,6 +14,8 @@ class GroupSearchViewPermission(OrganizationPermission): "PUT": ["org:read", "org:write", "org:admin"], "DELETE": ["org:read", "org:write", "org:admin"], } + # TODO(api-scope-personalization): Move custom issue-view writes off + # readonly org scopes once we add a narrow member/personal view write scope. readonly_mutation_scope_exceptions = { "PUT": "Group search view edits remain available to creators and org managers without requiring org:write in the scope map.", "DELETE": "Group search view deletes remain available to creators and org managers without requiring org:write in the scope map.", diff --git a/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py b/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py index 3066648bb430..13aa29f20912 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_details_starred.py @@ -25,6 +25,8 @@ class MemberPermission(OrganizationPermission): scope_map = { "POST": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move issue-view starring off readonly + # member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "POST": "Starring a group search view only changes the requesting member's preferences.", } diff --git a/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py b/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py index 93bba883fd99..90084b24adbc 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_starred_order.py @@ -15,6 +15,8 @@ class MemberPermission(OrganizationPermission): scope_map = { "PUT": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move issue-view starring order off + # readonly member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "PUT": "Group search view starred ordering only changes the requesting member's preferences.", } diff --git a/src/sentry/issues/endpoints/organization_group_search_view_visit.py b/src/sentry/issues/endpoints/organization_group_search_view_visit.py index 8a3469682a17..401fce4bc2e9 100644 --- a/src/sentry/issues/endpoints/organization_group_search_view_visit.py +++ b/src/sentry/issues/endpoints/organization_group_search_view_visit.py @@ -16,6 +16,8 @@ class MemberPermission(OrganizationPermission): scope_map = { "POST": ["member:read"], } + # TODO(api-scope-personalization): Move last-visited writes off readonly + # member scopes once we add a narrow personal preference write scope. readonly_mutation_scope_exceptions = { "POST": "View visit timestamps only update the requesting member's last-visited state.", } diff --git a/src/sentry/issues/endpoints/organization_group_search_views.py b/src/sentry/issues/endpoints/organization_group_search_views.py index bb18f4b8d9a0..e46f389d170d 100644 --- a/src/sentry/issues/endpoints/organization_group_search_views.py +++ b/src/sentry/issues/endpoints/organization_group_search_views.py @@ -28,6 +28,8 @@ class MemberPermission(OrganizationPermission): "GET": ["member:read", "member:write"], "POST": ["member:read", "member:write"], } + # TODO(api-scope-personalization): Move custom issue-view creation off + # readonly member scopes once we add a narrow member/personal view write scope. readonly_mutation_scope_exceptions = { "POST": "Creating a custom group search view is allowed for organization members.", }