Skip to content

Commit 604f729

Browse files
dcramercodex
andcommitted
fix(api): Align alert serializers with write scopes
Alert and monitor mutations now accept alerts:write at the endpoint level, but the nested project validators still required project:read. That let the request through permission checks and then failed token-based create and update flows with a 400 during validation. Allow those serializers to validate project selection against the same write-capable scopes the endpoints already advertise, and add direct token regression tests for alert rule updates and monitor creation so the contract stays enforced. Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 parent 2f9b9e4 commit 604f729

File tree

4 files changed

+92
-2
lines changed

4 files changed

+92
-2
lines changed

src/sentry/incidents/serializers/alert_rule.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@
4545

4646
logger = logging.getLogger(__name__)
4747

48+
ALERT_RULE_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write")
49+
4850

4951
class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRule]):
5052
"""
@@ -56,7 +58,7 @@ class AlertRuleSerializer(SnubaQueryValidator, CamelSnakeModelSerializer[AlertRu
5658

5759
environment = EnvironmentField(required=False, allow_null=True)
5860
projects = serializers.ListField(
59-
child=ProjectField(scope="project:read"),
61+
child=ProjectField(scope=ALERT_RULE_PROJECT_SCOPES),
6062
required=False,
6163
max_length=1,
6264
)

src/sentry/monitors/validators.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
INTERVAL_NAMES = ("year", "month", "week", "day", "hour", "minute")
7575

7676
CRONTAB_WHITESPACE = re.compile(r"\s+")
77+
MONITOR_PROJECT_SCOPES = ("project:read", "org:write", "org:admin", "alerts:write")
7778

7879
# XXX(dcramer): @reboot is not supported (as it cannot be)
7980
NONSTANDARD_CRONTAB_SCHEDULES = {
@@ -312,7 +313,7 @@ def validate(self, attrs):
312313
@extend_schema_serializer(exclude_fields=["alert_rule"])
313314
class MonitorValidator(CamelSnakeSerializer):
314315
project = ProjectField(
315-
scope="project:read",
316+
scope=MONITOR_PROJECT_SCOPES,
316317
required=True,
317318
help_text="The project slug to associate the monitor to.",
318319
)

tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from django.conf import settings
1212
from django.core.exceptions import ValidationError
1313
from django.test import override_settings
14+
from django.urls import reverse
1415
from httpx import HTTPError
1516
from rest_framework.exceptions import ErrorDetail
1617
from rest_framework.response import Response
@@ -44,6 +45,7 @@
4445
find_channel_id_for_alert_rule,
4546
)
4647
from sentry.integrations.slack.utils.channel import SlackChannelIdData
48+
from sentry.models.apitoken import ApiToken
4749
from sentry.models.auditlogentry import AuditLogEntry
4850
from sentry.models.organizationmemberteam import OrganizationMemberTeam
4951
from sentry.models.project import Project
@@ -82,6 +84,10 @@ class AlertRuleDetailsBase(AlertRuleBase):
8284

8385
endpoint = "sentry-api-0-organization-alert-rule-details"
8486

87+
def _create_token(self, scope: str) -> ApiToken:
88+
with assume_test_silo_mode(SiloMode.CONTROL):
89+
return ApiToken.objects.create(user=self.user, scope_list=[scope])
90+
8591
@patch(
8692
"sentry.seer.anomaly_detection.store_data.seer_anomaly_detection_connection_pool.urlopen"
8793
)
@@ -1000,6 +1006,40 @@ def test_get_shows_count_when_stored_as_upsampled_count(
10001006
class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase):
10011007
method = "put"
10021008

1009+
def test_update_requires_alerts_write_scope_for_tokens(self) -> None:
1010+
self.create_member(
1011+
user=self.user, organization=self.organization, role="owner", teams=[self.team]
1012+
)
1013+
token = self._create_token("org:read")
1014+
1015+
with self.feature("organizations:incidents"):
1016+
response = self.client.put(
1017+
reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]),
1018+
data=self.valid_params,
1019+
format="json",
1020+
HTTP_AUTHORIZATION=f"Bearer {token.token}",
1021+
)
1022+
1023+
assert response.status_code == 403
1024+
1025+
def test_update_allows_alerts_write_scope_for_tokens(self) -> None:
1026+
self.create_member(
1027+
user=self.user, organization=self.organization, role="owner", teams=[self.team]
1028+
)
1029+
token = self._create_token("alerts:write")
1030+
1031+
with self.feature("organizations:incidents"):
1032+
response = self.client.put(
1033+
reverse(self.endpoint, args=[self.organization.slug, self.alert_rule.id]),
1034+
data=self.valid_params,
1035+
format="json",
1036+
HTTP_AUTHORIZATION=f"Bearer {token.token}",
1037+
)
1038+
1039+
assert response.status_code == 200
1040+
self.alert_rule.refresh_from_db()
1041+
assert self.alert_rule.name == self.valid_params["name"]
1042+
10031043
def test_simple(self) -> None:
10041044
self.create_member(
10051045
user=self.user, organization=self.organization, role="owner", teams=[self.team]

tests/sentry/monitors/endpoints/test_organization_monitor_index.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,24 @@
66

77
from django.conf import settings
88
from django.test.utils import override_settings
9+
from django.urls import reverse
910
from rest_framework.exceptions import ErrorDetail
1011

1112
from sentry import audit_log
1213
from sentry.analytics.events.cron_monitor_created import CronMonitorCreated, FirstCronMonitorCreated
1314
from sentry.constants import ObjectStatus
15+
from sentry.models.apitoken import ApiToken
1416
from sentry.models.projectteam import ProjectTeam
1517
from sentry.models.rule import Rule, RuleSource
1618
from sentry.monitors.models import Monitor, MonitorStatus, ScheduleType, is_monitor_muted
1719
from sentry.monitors.utils import get_detector_for_monitor
1820
from sentry.quotas.base import SeatAssignmentResult
21+
from sentry.silo.base import SiloMode
1922
from sentry.testutils.asserts import assert_org_audit_log_exists
2023
from sentry.testutils.cases import MonitorTestCase
2124
from sentry.testutils.helpers.analytics import assert_any_analytics_event
2225
from sentry.testutils.outbox import outbox_runner
26+
from sentry.testutils.silo import assume_test_silo_mode
2327
from sentry.utils.outcomes import Outcome
2428
from sentry.utils.slug import DEFAULT_SLUG_ERROR_MESSAGE
2529

@@ -438,6 +442,49 @@ def setUp(self) -> None:
438442
super().setUp()
439443
self.login_as(self.user)
440444

445+
def _create_token(self, scope: str) -> ApiToken:
446+
with assume_test_silo_mode(SiloMode.CONTROL):
447+
return ApiToken.objects.create(user=self.user, scope_list=[scope])
448+
449+
def test_create_requires_alerts_write_scope_for_tokens(self) -> None:
450+
token = self._create_token("org:read")
451+
data = {
452+
"project": self.project.slug,
453+
"name": "My Monitor",
454+
"type": "cron_job",
455+
"owner": f"user:{self.user.id}",
456+
"config": {"schedule_type": "crontab", "schedule": "@daily"},
457+
}
458+
459+
response = self.client.post(
460+
reverse(self.endpoint, args=[self.organization.slug]),
461+
data=data,
462+
format="json",
463+
HTTP_AUTHORIZATION=f"Bearer {token.token}",
464+
)
465+
466+
assert response.status_code == 403
467+
468+
def test_create_allows_alerts_write_scope_for_tokens(self) -> None:
469+
token = self._create_token("alerts:write")
470+
data = {
471+
"project": self.project.slug,
472+
"name": "My Monitor",
473+
"type": "cron_job",
474+
"owner": f"user:{self.user.id}",
475+
"config": {"schedule_type": "crontab", "schedule": "@daily"},
476+
}
477+
478+
with outbox_runner():
479+
response = self.client.post(
480+
reverse(self.endpoint, args=[self.organization.slug]),
481+
data=data,
482+
format="json",
483+
HTTP_AUTHORIZATION=f"Bearer {token.token}",
484+
)
485+
486+
assert response.status_code == 201
487+
441488
@patch("sentry.analytics.record")
442489
def test_simple(self, mock_record: MagicMock) -> None:
443490
data = {

0 commit comments

Comments
 (0)