From a2b9392c93efb4e5b80d3bfa72b35cbcbe0960d1 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:14:19 -0700 Subject: [PATCH 01/12] 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 1a0bd6e04a865c..d705fe83c14c54 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 24bc36b1ed0a3d..f597e4be73944b 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 c530e5d1105fea..830fc1fe62e52e 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 358fb6cef5fc1c..1e4008b8532ce5 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 4c630e0f3d94c7..d65ef53f892e48 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 d8e6b07e51a660..3effe6dd744393 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 24e2b844439038..bf41f407ff5402 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 4b074f5b3dd295..f1f1150bf4803c 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 eed5963af1fb1060b46f7f61fabad578ed0e0649 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:51:57 -0700 Subject: [PATCH 02/12] 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 f597e4be73944b..974ddb1d2cfaef 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 830fc1fe62e52e..c530e5d1105fea 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 1e4008b8532ce5..358fb6cef5fc1c 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 d65ef53f892e48..4c630e0f3d94c7 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 bf41f407ff5402..5c08ea295b43ac 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 1366c6a567d6081a7b7daec4b5b5327c09707a24 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 14:54:25 -0700 Subject: [PATCH 03/12] 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 5baea3ba65fdcb..f012164aec686c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -437,6 +437,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/locale.tsx @getsentry/app-frontend From bccb7ae734a7f957147f510b636a9d7e1aa01155 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:06:20 -0700 Subject: [PATCH 04/12] 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 5c08ea295b43ac..885f47b03f6bf4 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 f1f1150bf4803c..e9addac202475f 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 6144c109d40cc1bcbd5d40cb92e462669104350a Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:13:35 -0700 Subject: [PATCH 05/12] 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 885f47b03f6bf4..264c67dd376806 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 4cb9985f673bab..db03b433356a07 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 9b80874ec8afe6b2dd9262e54cd518f1da0b4afe Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:16:28 -0700 Subject: [PATCH 06/12] 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 f012164aec686c..5baea3ba65fdcb 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -437,7 +437,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/locale.tsx @getsentry/app-frontend diff --git a/src/sentry/api/bases/incident.py b/src/sentry/api/bases/incident.py index d705fe83c14c54..1a0bd6e04a865c 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 a2d879bd6c3f7c5f0a8333332593fc50ab0acfd8 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 15:42:32 -0700 Subject: [PATCH 07/12] 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 e9addac202475f..7b4981dcbc3306 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 53385a13bcc87b..260e3e0bf64bca 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 4846359b74032e..6cd7e380d7791f 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 855aa428ac54cc..546bb259b956ba 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 e2ae8a0a4af4e1..fb2b710c6e07aa 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 1e1234036dd1ce41d5f1a9e17abd3e3102eacf28 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 16:00:54 -0700 Subject: [PATCH 08/12] 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 af482ea5eb330d..c6397fd43da082 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.search.utils import parse_datetime_string @@ -23,6 +25,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 @@ -77,6 +80,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 7a60b6c1c2227f..867eb154c4b49a 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 @@ -425,6 +426,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(), @@ -449,6 +454,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 0ea6f2aaed381c..7071efc54a49f3 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 db03b433356a07..3d93d73bab0987 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 4def0135187add..441c920a0b2d4c 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 02aa0c908fdfa6..1994c1d3cff077 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 00000000000000..15af972096289e --- /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 1ffb89af8f308b..86fa7433266653 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 @@ -12,6 +13,7 @@ from sentry.incidents.grouptype import MetricIssue from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE +from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry from sentry.silo.base import SiloMode from sentry.snuba.dataset import Dataset @@ -258,6 +260,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 @@ -486,6 +492,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 2f9b9e414d978151b16c3736fd3357c6bad2d9e5 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 16:22:33 -0700 Subject: [PATCH 09/12] 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 c6397fd43da082..fb382d04c62702 100644 --- a/tests/sentry/data_export/endpoints/test_data_export.py +++ b/tests/sentry/data_export/endpoints/test_data_export.py @@ -6,9 +6,12 @@ from sentry.data_export.base import ExportQueryType, ExportStatus from sentry.data_export.models import ExportedData +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 @@ -58,6 +61,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") @@ -80,30 +87,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 867eb154c4b49a..12f2be8c3f4c42 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -455,6 +455,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"]): @@ -468,6 +470,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 441c920a0b2d4c..b0af2ee7ca4c22 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 604f7297ac90de27a9039ba3c9fea12e06c17720 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 16:51:13 -0700 Subject: [PATCH 10/12] 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 38098ea1551521..d3e178235d52f7 100644 --- a/src/sentry/incidents/serializers/alert_rule.py +++ b/src/sentry/incidents/serializers/alert_rule.py @@ -45,6 +45,8 @@ logger = logging.getLogger(__name__) +ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") + class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRule]): """ @@ -56,7 +58,7 @@ class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRu environment = EnvironmentField(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 c5c31b3f6b4809..30bd816f97c14e 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 ef2bc6ce6ff061..76e736961a6250 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 @@ -44,6 +45,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 @@ -82,6 +84,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" ) @@ -1000,6 +1006,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 c452585873b9b5..294254edd8645f 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 2a711cc2753537bba1498aaf83129f3a0bc2cb8f Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 17:24:03 -0700 Subject: [PATCH 11/12] 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 974ddb1d2cfaef..34876ebcc41e01 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 6e57d9d01047e6..41d53275b04446 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]]: @@ -158,6 +191,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 764fd14e030e0c..4bfb10c05f3710 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 @@ -325,6 +326,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 5e22408199994f..4234c61d2a143b 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 264c67dd376806..0d0f592fcf4b3a 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 260e3e0bf64bca..e4974d4d04b9c0 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 6cd7e380d7791f..10eb603f03d07f 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 546bb259b956ba..c47879dd8f3248 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 a399c2238980a0..f063a540768847 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -95,6 +95,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 9029d0505ecb8d..465f4c851b7844 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_index.py @@ -150,6 +150,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 76e736961a6250..2254cc7e160814 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -84,9 +84,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" @@ -1040,6 +1040,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] @@ -2618,6 +2647,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 294254edd8645f..d7c160e9b563e2 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 3d93d73bab0987..f68bd462204729 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 b0af2ee7ca4c22..b3dd7a8312d035 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 86fa7433266653..63dbf3406395ca 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -260,9 +260,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" @@ -519,6 +519,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) @@ -1003,6 +1029,30 @@ def test_update_data_source_marks_user_updated_when_snapshot_exists( 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 34ef4d04793996b514b11cd9d90d9e9b9617cc04 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 15 Apr 2026 18:14:57 -0700 Subject: [PATCH 12/12] 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 | 17 +++++++- .../test_organization_alert_rule_details.py | 43 +++++++++++++++++++ .../test_organization_detector_details.py | 40 +++++++++++++++++ 4 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index 41d53275b04446..a11dee57b0363c 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 ( @@ -192,7 +205,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: @@ -210,11 +225,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() ) @@ -230,7 +246,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 f063a540768847..8e535444dbe30e 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -27,6 +27,7 @@ from sentry.issues import grouptype from sentry.models.organization import Organization from sentry.models.project import Project +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext 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 @@ -39,6 +40,15 @@ from sentry.workflow_engine.models import Detector +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 @@ -96,7 +106,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 @@ -110,11 +122,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 2254cc7e160814..c967fcf083632f 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1006,6 +1006,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] @@ -2647,6 +2669,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 63dbf3406395ca..101dc7f22f7673 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -504,6 +504,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") @@ -1029,6 +1051,24 @@ def test_update_data_source_marks_user_updated_when_snapshot_exists( 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(