From 287358298e67663eee57d4a7f47a72c073967f29 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 10:35:51 -0700 Subject: [PATCH 1/5] fix(api): Tighten alert mutation write scopes Require project-scoped write access for alert-style mutation endpoints across alerts, monitors, workflow engine, seer, uptime, replays, and related API surfaces. This keeps the intended write-scope behavior while rebuilding the change directly on master instead of depending on the readonly-mutation-notes stack. Add regression coverage for token and team-admin scope checks, plus the endpoint-specific edge cases that surfaced while unstacking the branch. Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/organization.py | 88 ++++++++-- src/sentry/incidents/endpoints/bases.py | 97 +++++++++++ .../organization_alert_rule_details.py | 6 + .../incidents/serializers/alert_rule.py | 4 +- .../issues/endpoints/project_user_issue.py | 2 +- .../endpoints/organization_monitor_index.py | 23 ++- src/sentry/monitors/validators.py | 3 +- .../endpoints/project_replay_details.py | 2 +- .../endpoints/project_replay_summary.py | 7 + .../organization_events_anomalies.py | 29 +++- ...organization_uptime_alert_preview_check.py | 3 + ...ganization_uptime_assertion_suggestions.py | 3 + .../organization_detector_details.py | 41 +++++ .../endpoints/organization_detector_index.py | 1 + .../data_export/endpoints/test_data_export.py | 38 +++++ .../test_organization_alert_rule_details.py | 150 ++++++++++++++++++ .../test_organization_alert_rule_index.py | 38 +++++ .../endpoints/test_project_user_issue.py | 50 +++++- .../test_organization_monitor_index.py | 84 +++++++++- .../endpoints/test_project_replay_details.py | 63 ++++++++ .../endpoints/test_project_replay_summary.py | 38 +++++ .../test_organization_events_anomalies.py | 96 +++++++++++ .../test_organization_uptime_alert_preview.py | 24 +++ ...ganization_uptime_assertion_suggestions.py | 85 ++++++++++ .../test_organization_detector_details.py | 129 +++++++++++++++ 25 files changed, 1079 insertions(+), 25 deletions(-) create mode 100644 tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 24bc36b1ed0a..34876ebcc41e 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, @@ -212,31 +213,100 @@ 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) + + +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"], - # 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 + + 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") + ) + 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 + + 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") + ) + class OrgAuthTokenPermission(OrganizationPermission): scope_map = { diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index 5ce0ba86cf8b..dd6021d7a786 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,11 +13,22 @@ 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.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. @@ -24,6 +37,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 +98,37 @@ def convert_args( class OrganizationAlertRuleEndpoint(OrganizationEndpoint): permission_classes = (OrganizationAlertRulePermission,) + def get_alert_mutation_projects( + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ) -> 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: + organization_id = _get_organization_id(organization) + alert_rule = AlertRule.objects.prefetch_related("projects").get( + organization_id=organization_id, id=validated_alert_rule_id + ) + return [alert_rule.projects.get()] + except ( + AlertRule.DoesNotExist, + AlertRule.MultipleObjectsReturned, + Project.DoesNotExist, + Project.MultipleObjectsReturned, + ): + return None + def convert_args( self, request: Request, alert_rule_id: int, *args: Any, **kwargs: Any ) -> tuple[tuple[Any, ...], dict[str, Any]]: @@ -165,6 +211,57 @@ 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 | RpcOrganization | RpcUserOrganizationContext, + ) -> 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 + + organization_id = _get_organization_id(organization) + ard = ( + AlertRuleDetector.objects.select_related("detector__project") + .filter( + alert_rule_id=validated_alert_rule_id, + detector__project__organization_id=organization_id, + ) + .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_id=organization_id, + ) + .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 bedc2c6b86ec..522e8c349b95 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 @@ -323,6 +324,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/incidents/serializers/alert_rule.py b/src/sentry/incidents/serializers/alert_rule.py index 6561e883146d..7c507a10c048 100644 --- a/src/sentry/incidents/serializers/alert_rule.py +++ b/src/sentry/incidents/serializers/alert_rule.py @@ -51,6 +51,8 @@ logger = logging.getLogger(__name__) +ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") + class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRule]): """ @@ -62,7 +64,7 @@ class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRu environment = serializers.CharField(required=False, allow_null=True) projects = serializers.ListField( - child=ProjectField(scope="project:read"), + child=ProjectField(scope=ALERT_RULE_PROJECT_SCOPES), required=False, max_length=1, ) diff --git a/src/sentry/issues/endpoints/project_user_issue.py b/src/sentry/issues/endpoints/project_user_issue.py index d8e6b07e51a6..3effe6dd7443 100644 --- a/src/sentry/issues/endpoints/project_user_issue.py +++ b/src/sentry/issues/endpoints/project_user_issue.py @@ -180,7 +180,7 @@ class WebVitalsIssueDataSerializer(ProjectUserIssueRequestSerializer): class ProjectUserIssuePermission(ProjectPermission): scope_map = { "GET": [], - "POST": ["event:read", "event:write", "event:admin"], + "POST": ["event:write", "event:admin"], "PUT": [], "DELETE": [], } diff --git a/src/sentry/monitors/endpoints/organization_monitor_index.py b/src/sentry/monitors/endpoints/organization_monitor_index.py index 5e2240819999..645a268f23cb 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 @@ -40,6 +40,7 @@ from sentry.incidents.endpoints.bases import OrganizationAlertRuleBaseEndpoint from sentry.models.environment import Environment from sentry.models.organization import Organization +from sentry.models.project import Project from sentry.monitors.models import ( DEFAULT_STATUS_ORDER, MONITOR_ENVIRONMENT_ORDERING, @@ -326,11 +327,23 @@ def put(self, request: AuthenticatedHttpRequest, organization) -> Response: result = dict(validator.validated_data) - projects = self.get_projects(request, organization, include_all_accessible=True) - project_ids = [project.id for project in projects] - monitor_guids = result.pop("ids", []) - monitors = list(Monitor.objects.filter(guid__in=monitor_guids, project_id__in=project_ids)) + monitors = list( + Monitor.objects.filter(guid__in=monitor_guids, organization_id=organization.id) + ) + projects_by_id = { + project.id: project + for project in Project.objects.filter( + id__in={monitor.project_id for monitor in monitors}, + organization_id=organization.id, + ) + } + if not all( + (project := projects_by_id.get(monitor.project_id)) + and request.access.has_any_project_scope(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/monitors/validators.py b/src/sentry/monitors/validators.py index c5c31b3f6b48..30bd816f97c1 100644 --- a/src/sentry/monitors/validators.py +++ b/src/sentry/monitors/validators.py @@ -74,6 +74,7 @@ INTERVAL_NAMES = ("year", "month", "week", "day", "hour", "minute") CRONTAB_WHITESPACE = re.compile(r"\s+") +MONITOR_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write") # XXX(dcramer): @reboot is not supported (as it cannot be) NONSTANDARD_CRONTAB_SCHEDULES = { @@ -312,7 +313,7 @@ def validate(self, attrs): @extend_schema_serializer(exclude_fields=["alert_rule"]) class MonitorValidator(CamelSnakeSerializer): project = ProjectField( - scope="project:read", + scope=MONITOR_PROJECT_SCOPES, required=True, help_text="The project slug to associate the monitor to.", ) diff --git a/src/sentry/replays/endpoints/project_replay_details.py b/src/sentry/replays/endpoints/project_replay_details.py index 24e2b8444390..0d0f592fcf4b 100644 --- a/src/sentry/replays/endpoints/project_replay_details.py +++ b/src/sentry/replays/endpoints/project_replay_details.py @@ -23,7 +23,7 @@ class ReplayDetailsPermission(ProjectPermission): "GET": ["project:read", "project:write", "project:admin"], "POST": ["project:write", "project:admin"], "PUT": ["project:write", "project:admin"], - "DELETE": ["project:read", "project:write", "project:admin"], + "DELETE": ["event:admin"], } diff --git a/src/sentry/replays/endpoints/project_replay_summary.py b/src/sentry/replays/endpoints/project_replay_summary.py index 4b074f5b3dd2..7b4981dcbc33 100644 --- a/src/sentry/replays/endpoints/project_replay_summary.py +++ b/src/sentry/replays/endpoints/project_replay_summary.py @@ -52,6 +52,13 @@ class ProjectReplaySummaryEndpoint(ProjectReplayEndpoint): "GET": ApiPublishStatus.EXPERIMENTAL, "POST": ApiPublishStatus.EXPERIMENTAL, } + readonly_mutation_scope_exceptions = { + "POST": ( + "POST starts replay-summary generation but only derives summary data from existing " + "replay/event data. It intentionally follows replay read access instead of requiring " + "a separate write capability." + ) + } permission_classes = (ReplaySummaryPermission,) def __init__(self, **kw) -> None: diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 53385a13bcc8..6bdeda7d1d27 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 @@ -20,6 +20,7 @@ from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.models.organization import Organization +from sentry.models.project import Project from sentry.seer.anomaly_detection.get_historical_anomalies import ( get_historical_anomaly_data_from_seer_preview, ) @@ -33,8 +34,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 list(Project.objects.filter(id=project_id, organization_id=organization.id)) + except Exception: + return None + @extend_schema( operation_id="Identify anomalies in historical data", parameters=[GlobalParams.ORG_ID_OR_SLUG], @@ -89,9 +104,17 @@ def post(self, request: Request, organization: Organization) -> Response: ) project_id = to_valid_int_id("project_id", raw_project_id) - projects = self.get_projects(request, organization, project_ids={project_id}) + projects = list(Project.objects.filter(id=project_id, organization_id=organization.id)) if not projects: - return Response({"detail": "Invalid project"}, status=400) + return Response( + {"detail": "You do not have permission to perform this action."}, + status=403, + ) + 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 4846359b7403..10eb603f03d0 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py +++ b/src/sentry/uptime/endpoints/organization_uptime_alert_preview_check.py @@ -30,6 +30,9 @@ @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,) publish_status = { diff --git a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py index 855aa428ac54..c47879dd8f32 100644 --- a/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py +++ b/src/sentry/uptime/endpoints/organization_uptime_assertion_suggestions.py @@ -50,6 +50,9 @@ 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,) publish_status = { diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index a399c2238980..8e535444dbe3 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 @@ -95,6 +105,37 @@ def get_detector_validator( @cell_silo_endpoint @extend_schema(tags=["Monitors"]) class OrganizationDetectorDetailsEndpoint(OrganizationEndpoint): + def get_alert_mutation_projects( + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ) -> 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 + + organization_id = _get_organization_id(organization) + 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 fa83134f2b01..c0b5ba60ee8c 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/data_export/endpoints/test_data_export.py b/tests/sentry/data_export/endpoints/test_data_export.py index 09905c42b6f3..813f7cc122b9 100644 --- a/tests/sentry/data_export/endpoints/test_data_export.py +++ b/tests/sentry/data_export/endpoints/test_data_export.py @@ -2,12 +2,17 @@ from typing import Any +from django.urls import reverse + from sentry.data_export.base import ExportQueryType, ExportStatus from sentry.data_export.models import ExportedData from sentry.data_export.writers import OutputMode +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 @@ -24,6 +29,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 @@ -56,6 +62,10 @@ def make_payload( payload["query_info"].update(extras) return payload + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + def test_authorization(self) -> None: payload = self.make_payload("issue") @@ -78,6 +88,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_tokens(self) -> None: + payload = self.make_payload("issue") + token = self._create_token("event:read") + + with self.feature("organizations:discover-query"): + response = self.client.post( + self.url, + data=payload, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + def test_post_allows_event_write_scope_for_tokens(self) -> None: + payload = self.make_payload("issue") + token = self._create_token("event:write") + + with self.feature("organizations:discover-query"): + response = self.client.post( + self.url, + data=payload, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + 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_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index f49d01c33112..77e8e76e7d0d 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -11,6 +11,7 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.test import override_settings +from django.urls import reverse from httpx import HTTPError from rest_framework.exceptions import ErrorDetail from rest_framework.response import Response @@ -45,6 +46,7 @@ find_channel_id_for_alert_rule, ) from sentry.integrations.slack.utils.channel import SlackChannelIdData +from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry from sentry.models.organizationmemberteam import OrganizationMemberTeam from sentry.models.project import Project @@ -86,6 +88,10 @@ class AlertRuleDetailsBase(AlertRuleBase): endpoint = "sentry-api-0-organization-alert-rule-details" + def _create_token(self, scope: str, user=None) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + 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" ) @@ -1046,6 +1052,98 @@ 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: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + 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] + ) + 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_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_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] @@ -2673,6 +2771,58 @@ 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: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_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/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index a4b5414756a2..7357ad98b304 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -40,6 +40,7 @@ find_channel_id_for_alert_rule, ) from sentry.integrations.slack.utils.channel import SlackChannelIdData +from sentry.models.apitoken import ApiToken from sentry.models.auditlogentry import AuditLogEntry from sentry.models.organizationmember import OrganizationMember from sentry.models.projectteam import ProjectTeam @@ -417,6 +418,10 @@ def setUp(self) -> None: ) self.login_as(self.user) + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + def test_simple(self) -> None: with ( outbox_runner(), @@ -441,6 +446,39 @@ def test_simple(self) -> None: == list(audit_log_entry)[0].ip_address ) + 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"]): + 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: + 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 ( + outbox_runner(), + self.feature(["organizations:incidents", "organizations:performance-view"]), + ): + response = self.client.post( + f"/api/0/organizations/{self.organization.slug}/alert-rules/", + data=self.alert_rule_dict, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 201 + @patch("sentry.incidents.serializers.alert_rule.are_any_projects_error_upsampled") def test_count_automatically_converted_to_upsampled_count_for_upsampled_projects( self, mock_are_any_projects_error_upsampled diff --git a/tests/sentry/issues/endpoints/test_project_user_issue.py b/tests/sentry/issues/endpoints/test_project_user_issue.py index 0ea6f2aaed38..7071efc54a49 100644 --- a/tests/sentry/issues/endpoints/test_project_user_issue.py +++ b/tests/sentry/issues/endpoints/test_project_user_issue.py @@ -6,9 +6,11 @@ from sentry.issues.grouptype import WebVitalsGroup from sentry.issues.producer import PayloadType +from sentry.models.apitoken import ApiToken +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.features import with_feature -from sentry.testutils.silo import cell_silo_test +from sentry.testutils.silo import assume_test_silo_mode, cell_silo_test @cell_silo_test @@ -31,6 +33,10 @@ def setUp(self) -> None: }, ) + def _create_token(self, scope: str) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=self.user, scope_list=[scope]) + @with_feature("organizations:performance-web-vitals-seer-suggestions") def test_create_web_vitals_issue_success(self) -> None: data = { @@ -110,6 +116,48 @@ def test_no_access(self) -> None: assert response.status_code == 404 + @with_feature("organizations:performance-web-vitals-seer-suggestions") + def test_create_web_vitals_issue_requires_event_write_scope(self) -> None: + token = self._create_token("event:read") + + response = self.client.post( + self.url, + data={ + "transaction": "/test-transaction", + "issueType": WebVitalsGroup.slug, + "score": 75, + "value": 1000, + "vital": "lcp", + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + + @with_feature("organizations:performance-web-vitals-seer-suggestions") + def test_create_web_vitals_issue_allows_event_write_scope(self) -> None: + token = self._create_token("event:write") + + with patch( + "sentry.issues.endpoints.project_user_issue.produce_occurrence_to_kafka" + ) as mock_produce: + response = self.client.post( + self.url, + data={ + "transaction": "/test-transaction", + "issueType": WebVitalsGroup.slug, + "score": 75, + "value": 1000, + "vital": "lcp", + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + assert response.data == {"event_id": mock_produce.call_args[1]["occurrence"].event_id} + @with_feature("organizations:performance-web-vitals-seer-suggestions") def test_missing_required_fields(self) -> None: data = { diff --git a/tests/sentry/monitors/endpoints/test_organization_monitor_index.py b/tests/sentry/monitors/endpoints/test_organization_monitor_index.py index c452585873b9..fdc5ccea6530 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 = { @@ -841,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") @@ -934,6 +985,35 @@ 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")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_monitor = self._create_monitor(slug="other-monitor") + other_monitor.update(project_id=other_project.id) + 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) @@ -977,9 +1057,7 @@ def test_disallow_when_no_open_membership(self) -> None: "ids": [monitor.guid], "isMuted": True, } - response = self.get_success_response(self.organization.slug, **data) - assert response.status_code == 200 - assert response.data == {"updated": [], "errored": []} + self.get_error_response(self.organization.slug, status_code=403, **data) monitor.refresh_from_db() env.refresh_from_db() diff --git a/tests/sentry/replays/endpoints/test_project_replay_details.py b/tests/sentry/replays/endpoints/test_project_replay_details.py index 4cb9985f673b..f68bd4622047 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,66 @@ def test_delete_replay_from_filestore(self) -> None: except File.DoesNotExist: pass + 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"]) + + 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_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_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"]) + + 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 == 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/replays/endpoints/test_project_replay_summary.py b/tests/sentry/replays/endpoints/test_project_replay_summary.py index e2ae8a0a4af4..fb2b710c6e07 100644 --- a/tests/sentry/replays/endpoints/test_project_replay_summary.py +++ b/tests/sentry/replays/endpoints/test_project_replay_summary.py @@ -7,8 +7,11 @@ from django.conf import settings from django.urls import reverse +from sentry.models.apitoken import ApiToken from sentry.replays.testutils import mock_replay +from sentry.silo.base import SiloMode from sentry.testutils.cases import SnubaTestCase, TransactionTestCase +from sentry.testutils.silo import assume_test_silo_mode from sentry.utils import json @@ -140,6 +143,41 @@ def test_post_simple(self, mock_seer_request: Mock) -> None: assert body["project_id"] == self.project.id assert body.get("temperature") is None + @patch("sentry.replays.endpoints.project_replay_summary.make_replay_summary_start_request") + def test_post_allows_event_read_scope_for_api_tokens(self, mock_seer_request: Mock) -> None: + mock_seer_request.return_value = MockSeerResponse(200, json_data={"hello": "world"}) + self.store_replay() + + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) + + with self.feature(self.features): + response = self.client.post( + self.url, + data={"num_segments": 1}, + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 200 + assert response.json() == {"hello": "world"} + + def test_post_requires_replay_read_scope_for_api_tokens(self) -> None: + self.store_replay() + + with assume_test_silo_mode(SiloMode.CONTROL): + token = ApiToken.objects.create(user=self.user, scope_list=["org:read"]) + + with self.feature(self.features): + response = self.client.post( + self.url, + data={"num_segments": 1}, + content_type="application/json", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + def test_post_replay_not_found(self) -> None: with self.feature(self.features): response = self.client.post( diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 4def0135187a..9a505a7611bc 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, user=None) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=user or 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,94 @@ 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=[ + 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") + 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") + 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")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + 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/uptime/endpoints/test_organization_uptime_alert_preview.py b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py index 02aa0c908fdf..1994c1d3cff0 100644 --- a/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_alert_preview.py @@ -134,3 +134,27 @@ def test_alerts_write_permission(self) -> None: ) assert response.status_code == 200, response.content + + def test_org_read_scope_cannot_run_preview_check(self) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) + + url = reverse( + "sentry-api-0-organization-uptime-alert-preview-check", + kwargs={"organization_id_or_slug": self.organization.slug}, + ) + response = self.client.post( + url, + data={ + "name": "test", + "environment": "uptime-prod", + "owner": f"user:{self.user.id}", + "url": "http://sentry.io", + "timeoutMs": 1500, + "body": None, + "region": "default", + }, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 diff --git a/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py new file mode 100644 index 000000000000..15af97209628 --- /dev/null +++ b/tests/sentry/uptime/endpoints/test_organization_uptime_assertion_suggestions.py @@ -0,0 +1,85 @@ +from unittest import mock + +from django.test import override_settings +from django.urls import reverse + +from sentry.conf.types.uptime import UptimeRegionConfig +from tests.sentry.uptime.endpoints import UptimeAlertBaseEndpointTest + + +@override_settings( + UPTIME_REGIONS=[ + UptimeRegionConfig( + slug="default", + name="Default Region", + config_redis_key_prefix="default", + api_endpoint="pop-st-1.uptime-checker.s4s.sentry.internal:80", + ) + ] +) +class OrganizationUptimeAssertionSuggestionsTest(UptimeAlertBaseEndpointTest): + endpoint = "sentry-api-0-organization-uptime-assertion-suggestions" + method = "post" + + def setUp(self) -> None: + super().setUp() + self.url = reverse(self.endpoint, args=[self.organization.slug]) + self.payload = { + "name": "test", + "environment": "uptime-prod", + "owner": f"user:{self.user.id}", + "url": "http://sentry.io", + "timeoutMs": 1500, + "body": None, + "region": "default", + } + + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.generate_assertion_suggestions" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.checker_api.invoke_checker_preview" + ) + @mock.patch( + "sentry.uptime.endpoints.organization_uptime_assertion_suggestions.UptimeCheckPreviewValidator" + ) + @mock.patch("sentry.uptime.endpoints.organization_uptime_assertion_suggestions.has_seer_access") + def test_alerts_write_scope_can_generate_suggestions( + self, + mock_has_seer_access: mock.MagicMock, + mock_validator_cls: mock.MagicMock, + mock_preview: mock.MagicMock, + mock_generate: mock.MagicMock, + ) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["alerts:write"]) + mock_has_seer_access.return_value = True + mock_validator = mock_validator_cls.return_value + mock_validator.is_valid.return_value = True + mock_validator.save.return_value = {"active_regions": ["default"]} + mock_preview.return_value = mock.Mock( + status_code=200, + json=mock.Mock(return_value={"status": 200}), + raise_for_status=mock.Mock(), + ) + mock_generate.return_value = (None, None) + + response = self.client.post( + self.url, + data=self.payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 200 + + def test_org_read_scope_cannot_generate_suggestions(self) -> None: + api_key = self.create_api_key(organization=self.organization, scope_list=["org:read"]) + + response = self.client.post( + self.url, + data=self.payload, + format="json", + HTTP_AUTHORIZATION=self.create_basic_auth_header(api_key.key), + ) + + assert response.status_code == 403 diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 29be7584c555..aaf7dd8e2f7b 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 @@ -94,6 +96,10 @@ def setUp(self) -> None: ) assert self.detector.data_sources is not None + def _create_token(self, scope: str, user=None) -> ApiToken: + with assume_test_silo_mode(SiloMode.CONTROL): + return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) + @cell_silo_test class OrganizationDetectorDetailsGetTest(OrganizationDetectorDetailsBaseTest): @@ -527,6 +533,84 @@ 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_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") + + 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_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")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_detector = self.create_detector( + project=other_project, type=MetricIssue.slug, 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) @@ -1055,6 +1139,51 @@ def test_update_generic_metrics_dataset_to_transactions(self) -> None: class OrganizationDetectorDetailsDeleteTest(OrganizationDetectorDetailsBaseTest): method = "DELETE" + def test_team_admin_can_delete_with_project_scoped_alerts_write(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(team_admin_user) + + with outbox_runner(): + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, self.detector.id]) + ) + + assert response.status_code == 204 + + def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: + team_admin_user = self.create_user(is_superuser=False) + self.create_member( + user=team_admin_user, + organization=self.organization, + role="member", + team_roles=[(self.team, "admin")], + ) + self.organization.update_option("sentry:alerts_member_write", False) + + other_team = self.create_team(organization=self.organization, name="other-team") + other_project = self.create_project( + organization=self.organization, teams=[other_team], name="other-project" + ) + other_detector = self.create_detector( + project=other_project, type=MetricIssue.slug, 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 f3db5618196be4009438154399618d7e35195601 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 11:08:14 -0700 Subject: [PATCH 2/5] fix(api): Preserve 404s for missing alert targets Keep missing alert-rule and detector detail lookups on the existing 404 path for project-scoped alerts:write callers. The permission helper now distinguishes a lookup miss from a view that does not participate in project-scoped alert checks, and the detail endpoint tests cover both the missing-target 404 and cross-project 403 behaviors. Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/organization.py | 22 ++++++++++- src/sentry/incidents/endpoints/bases.py | 22 ++++++----- .../organization_detector_details.py | 7 ++-- .../test_organization_alert_rule_details.py | 38 +++++++++++++++++++ .../test_organization_detector_details.py | 36 ++++++++++++++++++ 5 files changed, 111 insertions(+), 14 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 34876ebcc41e..1d8195b22c1d 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -2,7 +2,7 @@ from collections.abc import Sequence from datetime import datetime -from typing import Any, Literal, NotRequired, TypedDict, overload +from typing import Any, Literal, NotRequired, TypeAlias, TypedDict, overload import sentry_sdk from django.core.cache import cache @@ -228,6 +228,14 @@ def _has_any_team_scope(request: Request, scope: str) -> bool: ALERT_MUTATION_SCOPES = frozenset({"org:write", "alerts:write"}) +class _AlertMutationLookupMiss: + pass + + +ALERT_MUTATION_LOOKUP_MISS = _AlertMutationLookupMiss() +AlertMutationProjectLookup: TypeAlias = Sequence[Project] | None | _AlertMutationLookupMiss + + 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 @@ -238,12 +246,14 @@ def _has_view_project_scoped_alert_write( request: Request, view: APIView, organization: Organization | RpcOrganization | RpcUserOrganizationContext, -) -> bool | None: +) -> bool | None | _AlertMutationLookupMiss: get_projects = getattr(view, "get_alert_mutation_projects", None) if not callable(get_projects): return None projects = get_projects(request, organization) + if projects is ALERT_MUTATION_LOOKUP_MISS: + return ALERT_MUTATION_LOOKUP_MISS if projects is None: return None @@ -271,6 +281,10 @@ def has_object_permission( return False project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) + if project_scoped_access is ALERT_MUTATION_LOOKUP_MISS: + if _has_any_team_scope(request, "alerts:write"): + raise ResourceDoesNotExist + return False if project_scoped_access is not None: return project_scoped_access @@ -300,6 +314,10 @@ def has_object_permission( return False project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) + if project_scoped_access is ALERT_MUTATION_LOOKUP_MISS: + if _has_any_team_scope(request, "alerts:write"): + raise ResourceDoesNotExist + return False if project_scoped_access is not None: return project_scoped_access diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index dd6021d7a786..fcf71c21a4f0 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -1,4 +1,3 @@ -from collections.abc import Sequence from typing import Any from rest_framework.exceptions import PermissionDenied @@ -7,7 +6,12 @@ from sentry import features from sentry.api.api_owners import ApiOwner -from sentry.api.bases.organization import OrganizationAlertRulePermission, OrganizationEndpoint +from sentry.api.bases.organization import ( + ALERT_MUTATION_LOOKUP_MISS, + AlertMutationProjectLookup, + OrganizationAlertRulePermission, + OrganizationEndpoint, +) from sentry.api.bases.project import ProjectAlertRulePermission, ProjectEndpoint from sentry.api.exceptions import ResourceDoesNotExist from sentry.incidents.endpoints.serializers.utils import get_object_id_from_fake_id @@ -102,7 +106,7 @@ def get_alert_mutation_projects( self, request: Request, organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ) -> Sequence[Project] | None: + ) -> AlertMutationProjectLookup: if request.method not in {"PUT", "DELETE"}: return None @@ -113,7 +117,7 @@ def get_alert_mutation_projects( try: validated_alert_rule_id = to_valid_int_id("alert_rule_id", raw_alert_rule_id) except RestFrameworkValidationError: - return None + return ALERT_MUTATION_LOOKUP_MISS try: organization_id = _get_organization_id(organization) @@ -127,7 +131,7 @@ def get_alert_mutation_projects( Project.DoesNotExist, Project.MultipleObjectsReturned, ): - return None + return ALERT_MUTATION_LOOKUP_MISS def convert_args( self, request: Request, alert_rule_id: int, *args: Any, **kwargs: Any @@ -215,7 +219,7 @@ def get_alert_mutation_projects( self, request: Request, organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ) -> Sequence[Project] | None: + ) -> AlertMutationProjectLookup: projects = super().get_alert_mutation_projects(request, organization) if projects is not None: return projects @@ -230,7 +234,7 @@ def get_alert_mutation_projects( try: validated_alert_rule_id = to_valid_int_id("alert_rule_id", raw_alert_rule_id) except RestFrameworkValidationError: - return None + return ALERT_MUTATION_LOOKUP_MISS organization_id = _get_organization_id(organization) ard = ( @@ -247,7 +251,7 @@ def get_alert_mutation_projects( try: detector_id = get_object_id_from_fake_id(validated_alert_rule_id) except ValueError: - return None + return ALERT_MUTATION_LOOKUP_MISS detector = ( Detector.objects.select_related("project") @@ -258,7 +262,7 @@ def get_alert_mutation_projects( .first() ) if detector is None: - return None + return ALERT_MUTATION_LOOKUP_MISS return [detector.project] diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 8e535444dbe3..c68d5e8533f6 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -11,6 +11,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases import OrganizationDetectorPermission, OrganizationEndpoint +from sentry.api.bases.organization import ALERT_MUTATION_LOOKUP_MISS, AlertMutationProjectLookup from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers import serialize from sentry.apidocs.constants import ( @@ -109,7 +110,7 @@ def get_alert_mutation_projects( self, request: Request, organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ) -> list[Project] | None: + ) -> AlertMutationProjectLookup: if request.method not in {"PUT", "DELETE"}: return None @@ -120,7 +121,7 @@ def get_alert_mutation_projects( try: validated_detector_id = to_valid_int_id("detector_id", raw_detector_id) except ValidationError: - return None + return ALERT_MUTATION_LOOKUP_MISS organization_id = _get_organization_id(organization) detector = ( @@ -132,7 +133,7 @@ def get_alert_mutation_projects( .first() ) if detector is None: - return None + return ALERT_MUTATION_LOOKUP_MISS return [detector.project] diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index 77e8e76e7d0d..d5163dbdde0a 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1144,6 +1144,26 @@ def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + def test_update_missing_target_preserves_404_for_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, 999999999]), + data=self.valid_params, + format="json", + ) + + assert response.status_code == 404 + def test_simple(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] @@ -2823,6 +2843,24 @@ def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + def test_delete_missing_target_preserves_404_for_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.delete( + reverse(self.endpoint, args=[self.organization.slug, 999999999]), + ) + + assert response.status_code == 404 + def test_simple(self) -> None: self.create_member( user=self.user, organization=self.organization, role="owner", teams=[self.team] 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 aaf7dd8e2f7b..e04b66302416 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -611,6 +611,25 @@ def test_update_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + def test_update_missing_target_preserves_404_for_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) + + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, 999999999]), + data={"enabled": False}, + format="json", + ) + + assert response.status_code == 404 + def test_enable_detector(self) -> None: self.detector.update(enabled=False) self.detector.update(status=ObjectStatus.DISABLED) @@ -1184,6 +1203,23 @@ def test_delete_denies_alerts_write_scope_for_other_team_projects(self) -> None: assert response.status_code == 403 + def test_delete_missing_target_preserves_404_for_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) + + response = self.client.delete( + reverse(self.endpoint, args=[self.organization.slug, 999999999]), + ) + + assert response.status_code == 404 + @mock.patch( "sentry.workflow_engine.endpoints.organization_detector_details.schedule_update_project_config" ) From aec2a3d4fee0827dd6c308d923e9983630a3c89e Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 12:39:12 -0700 Subject: [PATCH 3/5] fix(api): Restore workflow engine alert target resolution Continue the feedback pass by keeping workflow-engine detector lookups reachable from the organization alert-rule detail endpoint. The project-scoped permission helper now preserves the 404 behavior for real lookup misses without short-circuiting the detector fallback path, and the alert-rule detail tests cover the team-admin workflow-engine detector cases explicitly. Also fold the duplicated organization-id helper into a shared location and reuse the alert-rule permission implementation for detector endpoints to avoid the same drift in future changes. Co-Authored-By: OpenAI Codex --- src/sentry/api/bases/organization.py | 42 +++++-------------- src/sentry/incidents/endpoints/bases.py | 12 +----- .../organization_detector_details.py | 15 +++---- .../test_organization_alert_rule_details.py | 30 +++++++++++++ 4 files changed, 48 insertions(+), 51 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 1d8195b22c1d..034029cbe90b 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -46,6 +46,15 @@ class NoProjects(Exception): pass +def _get_organization_id( + organization: Organization | RpcOrganization | RpcUserOrganizationContext, +) -> int: + if isinstance(organization, RpcUserOrganizationContext): + return organization.organization.id + + return organization.id + + class OrganizationPermission(DemoSafePermission): scope_map = { "GET": ["org:read", "org:write", "org:admin"], @@ -293,37 +302,8 @@ def has_object_permission( ) -class OrganizationDetectorPermission(OrganizationPermission): - scope_map = { - "GET": ["org:read", "org:write", "org:admin", "alerts:read"], - "POST": ["org:write", "org:admin", "alerts:write"], - "PUT": ["org:write", "org:admin", "alerts:write"], - "DELETE": ["org:write", "org:admin", "alerts:write"], - } - - def has_object_permission( - self, - request: Request, - view: APIView, - organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ) -> bool: - if super().has_object_permission(request, view, organization): - return True - - if request.method not in {"POST", "PUT", "DELETE"}: - return False - - project_scoped_access = _has_view_project_scoped_alert_write(request, view, organization) - if project_scoped_access is ALERT_MUTATION_LOOKUP_MISS: - if _has_any_team_scope(request, "alerts:write"): - raise ResourceDoesNotExist - return False - if project_scoped_access is not None: - return project_scoped_access - - return bool(getattr(view, "allow_any_team_alert_write_fallback", False)) and ( - _has_any_team_scope(request, "alerts:write") - ) +class OrganizationDetectorPermission(OrganizationAlertRulePermission): + pass class OrgAuthTokenPermission(OrganizationPermission): diff --git a/src/sentry/incidents/endpoints/bases.py b/src/sentry/incidents/endpoints/bases.py index fcf71c21a4f0..ed85281b2130 100644 --- a/src/sentry/incidents/endpoints/bases.py +++ b/src/sentry/incidents/endpoints/bases.py @@ -11,6 +11,7 @@ AlertMutationProjectLookup, OrganizationAlertRulePermission, OrganizationEndpoint, + _get_organization_id, ) from sentry.api.bases.project import ProjectAlertRulePermission, ProjectEndpoint from sentry.api.exceptions import ResourceDoesNotExist @@ -24,15 +25,6 @@ from sentry.workflow_engine.models.detector import Detector -def _get_organization_id( - organization: Organization | RpcOrganization | RpcUserOrganizationContext, -) -> int: - if isinstance(organization, RpcUserOrganizationContext): - return organization.organization.id - - return organization.id - - class OrganizationAlertRuleBaseEndpoint(OrganizationEndpoint): """ Base endpoint for organization-scoped alert rule creation. @@ -221,7 +213,7 @@ def get_alert_mutation_projects( organization: Organization | RpcOrganization | RpcUserOrganizationContext, ) -> AlertMutationProjectLookup: projects = super().get_alert_mutation_projects(request, organization) - if projects is not None: + if projects is not None and projects is not ALERT_MUTATION_LOOKUP_MISS: return projects if request.method not in {"PUT", "DELETE"}: diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index c68d5e8533f6..14486068aa30 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -11,7 +11,11 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases import OrganizationDetectorPermission, OrganizationEndpoint -from sentry.api.bases.organization import ALERT_MUTATION_LOOKUP_MISS, AlertMutationProjectLookup +from sentry.api.bases.organization import ( + ALERT_MUTATION_LOOKUP_MISS, + AlertMutationProjectLookup, + _get_organization_id, +) from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers import serialize from sentry.apidocs.constants import ( @@ -41,15 +45,6 @@ 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 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 d5163dbdde0a..5fa13560d688 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1077,6 +1077,36 @@ def test_team_admin_can_update_with_project_scoped_alerts_write(self) -> None: self.alert_rule.refresh_from_db() assert self.alert_rule.name == self.valid_params["name"] + @with_feature("organizations:workflow-engine-rule-serializers") + def test_team_admin_can_reach_workflow_engine_detector_update_with_project_scoped_alerts_write( + self, + ) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + team_admin_user = self.create_user() + self.create_member( + user=team_admin_user, + 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) + + ard = AlertRuleDetector.objects.get(alert_rule_id=self.alert_rule.id) + detector = Detector.objects.get(id=ard.detector_id) + fake_detector_id = get_fake_id_from_object_id(detector.id) + + with self.feature("organizations:incidents"), outbox_runner(): + response = self.client.put( + reverse(self.endpoint, args=[self.organization.slug, fake_detector_id]), + data=self.valid_params, + format="json", + ) + + assert response.status_code == 400 + 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] From 0ce9e9dcffd340b10b7caeb96c0c22fdd626b844 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 13:24:58 -0700 Subject: [PATCH 4/5] fix(api): Use RPC org context in anomaly preview permissions Route get_alert_mutation_projects through _get_organization_id so the\norganization-scoped alert permission check still works when auth token\nevaluation passes an RpcUserOrganizationContext.\n\nAdd a regression test for the RPC wrapper path so the mutation lookup\ndoes not silently return None again.\n\nCo-Authored-By: Codex --- .../organization_events_anomalies.py | 19 ++++++++++++++++--- .../test_organization_events_anomalies.py | 12 ++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 6bdeda7d1d27..3d75756f7a26 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -6,7 +6,11 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import ALERT_MUTATION_SCOPES, OrganizationAlertRulePermission +from sentry.api.bases.organization import ( + ALERT_MUTATION_SCOPES, + OrganizationAlertRulePermission, + _get_organization_id, +) from sentry.api.bases.organization_events import OrganizationEventsEndpointBase from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers.base import serialize @@ -21,6 +25,7 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.models.organization import Organization from sentry.models.project import Project +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.seer.anomaly_detection.get_historical_anomalies import ( get_historical_anomaly_data_from_seer_preview, ) @@ -39,14 +44,22 @@ class OrganizationEventsAnomaliesEndpoint(OrganizationEventsEndpointBase): # alerts/detectors, so it intentionally follows alert-write permissions. permission_classes = (OrganizationAlertRulePermission,) - def get_alert_mutation_projects(self, request: Request, organization: Organization): + def get_alert_mutation_projects( + self, + request: Request, + organization: Organization | RpcOrganization | RpcUserOrganizationContext, + ): raw_project_id = request.data.get("project_id") if raw_project_id is None: return None try: project_id = to_valid_int_id("project_id", raw_project_id) - return list(Project.objects.filter(id=project_id, organization_id=organization.id)) + return list( + Project.objects.filter( + id=project_id, organization_id=_get_organization_id(organization) + ) + ) except Exception: return None diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 9a505a7611bc..7b2c19041446 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -1,4 +1,5 @@ from datetime import timedelta +from types import SimpleNamespace from unittest.mock import MagicMock, patch import orjson @@ -12,6 +13,7 @@ AlertRuleThresholdType, ) from sentry.models.apitoken import ApiToken +from sentry.organizations.services.organization import RpcOrganization, RpcUserOrganizationContext from sentry.seer.anomaly_detection.types import ( Anomaly, AnomalyDetectionConfig, @@ -20,6 +22,7 @@ TimeSeriesPoint, ) from sentry.seer.anomaly_detection.utils import translate_direction +from sentry.seer.endpoints.organization_events_anomalies import OrganizationEventsAnomaliesEndpoint from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.datetime import before_now, freeze_time @@ -90,6 +93,15 @@ def get_context(self) -> DetectHistoricalAnomaliesContext: ], ) + def test_get_alert_mutation_projects_accepts_rpc_org_context(self) -> None: + endpoint = OrganizationEventsAnomaliesEndpoint() + request = SimpleNamespace(data={"project_id": str(self.project.id)}) + organization = RpcUserOrganizationContext( + organization=RpcOrganization(id=self.organization.id) + ) + + assert endpoint.get_alert_mutation_projects(request, organization) == [self.project] + @with_feature("organizations:anomaly-detection-alerts") @with_feature("organizations:incidents") @patch( From eafc40f9cf5839c26a8ca75221292f5fa7d0e4de Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 16 Apr 2026 13:48:53 -0700 Subject: [PATCH 5/5] fix(api): Preserve anomaly preview lookup misses Return ALERT_MUTATION_LOOKUP_MISS for invalid and missing anomaly\npreview project IDs so project-scoped alerts:write callers follow the\nmissing-target path instead of falling through to team fallback.\n\nTighten the related permission narrowing and test annotations so the\nbackend typing job matches the runtime behavior again.\n\nCo-Authored-By: Codex --- src/sentry/api/bases/organization.py | 2 +- .../organization_events_anomalies.py | 22 +++++++++---- .../test_organization_events_anomalies.py | 32 +++++++++++++++++-- .../test_organization_detector_details.py | 3 +- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 034029cbe90b..4df6d241c84e 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -294,7 +294,7 @@ def has_object_permission( if _has_any_team_scope(request, "alerts:write"): raise ResourceDoesNotExist return False - if project_scoped_access is not None: + if isinstance(project_scoped_access, bool): return project_scoped_access return bool(getattr(view, "allow_any_team_alert_write_fallback", False)) and ( diff --git a/src/sentry/seer/endpoints/organization_events_anomalies.py b/src/sentry/seer/endpoints/organization_events_anomalies.py index 3d75756f7a26..1dc09d247cc8 100644 --- a/src/sentry/seer/endpoints/organization_events_anomalies.py +++ b/src/sentry/seer/endpoints/organization_events_anomalies.py @@ -1,4 +1,5 @@ from drf_spectacular.utils import extend_schema +from rest_framework.exceptions import ValidationError as RestFrameworkValidationError from rest_framework.request import Request from rest_framework.response import Response @@ -7,7 +8,9 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases.organization import ( + ALERT_MUTATION_LOOKUP_MISS, ALERT_MUTATION_SCOPES, + AlertMutationProjectLookup, OrganizationAlertRulePermission, _get_organization_id, ) @@ -48,20 +51,25 @@ def get_alert_mutation_projects( self, request: Request, organization: Organization | RpcOrganization | RpcUserOrganizationContext, - ): + ) -> AlertMutationProjectLookup: 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 list( - Project.objects.filter( - id=project_id, organization_id=_get_organization_id(organization) - ) + except RestFrameworkValidationError: + return ALERT_MUTATION_LOOKUP_MISS + + projects = list( + Project.objects.filter( + id=project_id, organization_id=_get_organization_id(organization) ) - except Exception: - return None + ) + if not projects: + return ALERT_MUTATION_LOOKUP_MISS + + return projects @extend_schema( operation_id="Identify anomalies in historical data", diff --git a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py index 7b2c19041446..d4c2fbf42e2d 100644 --- a/tests/sentry/seer/endpoints/test_organization_events_anomalies.py +++ b/tests/sentry/seer/endpoints/test_organization_events_anomalies.py @@ -1,12 +1,15 @@ from datetime import timedelta from types import SimpleNamespace +from typing import cast from unittest.mock import MagicMock, patch import orjson from django.urls import reverse +from rest_framework.request import Request from urllib3 import HTTPResponse from urllib3.exceptions import TimeoutError +from sentry.api.bases.organization import ALERT_MUTATION_LOOKUP_MISS from sentry.incidents.models.alert_rule import ( AlertRuleSeasonality, AlertRuleSensitivity, @@ -29,6 +32,7 @@ from sentry.testutils.helpers.features import with_feature from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode +from sentry.users.models.user import User @freeze_time() @@ -51,7 +55,7 @@ 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, user=None) -> ApiToken: + def _create_token(self, scope: str, user: User | None = None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): return ApiToken.objects.create(user=user or self.user, scope_list=[scope]) @@ -95,13 +99,37 @@ def get_context(self) -> DetectHistoricalAnomaliesContext: def test_get_alert_mutation_projects_accepts_rpc_org_context(self) -> None: endpoint = OrganizationEventsAnomaliesEndpoint() - request = SimpleNamespace(data={"project_id": str(self.project.id)}) + request = cast(Request, SimpleNamespace(data={"project_id": str(self.project.id)})) organization = RpcUserOrganizationContext( organization=RpcOrganization(id=self.organization.id) ) assert endpoint.get_alert_mutation_projects(request, organization) == [self.project] + def test_get_alert_mutation_projects_returns_lookup_miss_for_invalid_project_id(self) -> None: + endpoint = OrganizationEventsAnomaliesEndpoint() + request = cast(Request, SimpleNamespace(data={"project_id": "not-a-project"})) + organization = RpcUserOrganizationContext( + organization=RpcOrganization(id=self.organization.id) + ) + + assert ( + endpoint.get_alert_mutation_projects(request, organization) + is ALERT_MUTATION_LOOKUP_MISS + ) + + def test_get_alert_mutation_projects_returns_lookup_miss_for_missing_project(self) -> None: + endpoint = OrganizationEventsAnomaliesEndpoint() + request = cast(Request, SimpleNamespace(data={"project_id": "999999999"})) + organization = RpcUserOrganizationContext( + organization=RpcOrganization(id=self.organization.id) + ) + + assert ( + endpoint.get_alert_mutation_projects(request, organization) + is ALERT_MUTATION_LOOKUP_MISS + ) + @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 e04b66302416..e95ff6e07130 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -25,6 +25,7 @@ from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, cell_silo_test from sentry.testutils.skips import requires_kafka, requires_snuba +from sentry.users.models.user import User from sentry.workflow_engine.models import ( AlertRuleDetector, DataCondition, @@ -96,7 +97,7 @@ def setUp(self) -> None: ) assert self.detector.data_sources is not None - def _create_token(self, scope: str, user=None) -> ApiToken: + def _create_token(self, scope: str, user: User | None = None) -> ApiToken: with assume_test_silo_mode(SiloMode.CONTROL): return ApiToken.objects.create(user=user or self.user, scope_list=[scope])