Skip to content

Commit 0113aed

Browse files
committed
ref(api): Narrow request.user in prompts and dashboard edit checks
- Prompt dismissal is a personal write, so it uses require_user(). A legacy API key used to get 201 while nothing was saved; it now gets 401. No production PUT without a user in the last 30 days. - Dashboard edit permission reads the caller through acting_user_id(), which is the same value for users and token callers. Drops both files from the request.user allowlist.
1 parent 547cc8b commit 0113aed

5 files changed

Lines changed: 49 additions & 7 deletions

File tree

‎src/sentry/api/endpoints/prompts_activity.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from sentry.apidocs.parameters import GlobalParams
2424
from sentry.apidocs.response_types import DetailResponse
2525
from sentry.apidocs.utils import inline_sentry_response_serializer
26-
from sentry.auth.principal import owned_by_acting_user
26+
from sentry.auth.principal import owned_by_acting_user, require_user
2727
from sentry.models.organization import Organization
2828
from sentry.models.project import Project
2929
from sentry.models.promptsactivity import PromptsActivity
@@ -223,10 +223,11 @@ def put(self, request: Request, organization: Organization, **kwargs: object) ->
223223
data["snoozed_ts"] = None
224224
data["dismissed_ts"] = None
225225

226+
user_id = require_user(request).id
226227
try:
227228
with transaction.atomic(router.db_for_write(PromptsActivity)):
228229
PromptsActivity.objects.update_or_create(
229-
feature=feature, user_id=request.user.id, defaults={"data": data}, **fields
230+
feature=feature, user_id=user_id, defaults={"data": data}, **fields
230231
)
231232
except IntegrityError:
232233
pass

‎src/sentry/dashboards/endpoints/organization_dashboards.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ def has_object_permission(
406406

407407
# check if user is restricted from editing dashboard
408408
if hasattr(obj, "permissions"):
409-
return obj.permissions.has_edit_permissions(request.user.id)
409+
return obj.permissions.has_edit_permissions(acting_user_id(request.user))
410410

411411
# if no permissions are assigned, it is considered accessible to all users
412412
return True

‎tests/sentry/api/endpoints/test_prompts_activity.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from django.urls import reverse
33
from rest_framework.response import Response
44

5+
from sentry.models.apikey import ApiKey
56
from sentry.models.promptsactivity import PromptsActivity
67
from sentry.silo.base import SiloMode
78
from sentry.testutils.cases import APITestCase
@@ -90,6 +91,48 @@ def test_service_account_sees_no_prompt_state(self) -> None:
9091
class PutPromptsActivityTest(PromptsActivityTestBase):
9192
method = "put"
9293

94+
def _dismiss(self, **headers: str | bytes) -> Response:
95+
return self.client.put(
96+
self.path,
97+
{
98+
"organization_id": self.org.id,
99+
"project_id": self.project.id,
100+
"feature": "releases",
101+
"status": "dismissed",
102+
},
103+
**headers,
104+
)
105+
106+
def test_api_key_cannot_dismiss(self) -> None:
107+
with assume_test_silo_mode(SiloMode.CONTROL):
108+
key = ApiKey.objects.create(organization_id=self.org.id, scope_list=["org:read"])
109+
self.client.logout()
110+
111+
resp = self._dismiss(HTTP_AUTHORIZATION=self.create_basic_auth_header(key.key))
112+
113+
assert resp.status_code == 401, resp.content
114+
assert not PromptsActivity.objects.filter(feature="releases").exists()
115+
116+
@override_settings(SEER_API_SHARED_SECRET="service-account-secret")
117+
@with_feature("organizations:service-accounts")
118+
def test_service_account_cannot_dismiss(self) -> None:
119+
service_account = self.create_service_account(
120+
organization=self.org, scope_list=["org:read"]
121+
)
122+
viewer_context = encode_viewer_context(
123+
ViewerContext(
124+
actor=Actor(type=ActorType.SERVICE_ACCOUNT, actor_id=service_account.id),
125+
organization_id=self.org.id,
126+
),
127+
key="service-account-secret",
128+
)
129+
130+
resp = self._dismiss(HTTP_X_VIEWER_CONTEXT=viewer_context)
131+
132+
assert resp.status_code == 403, resp.content
133+
assert resp.data["detail"]["code"] == "user-required"
134+
assert not PromptsActivity.objects.filter(feature="releases").exists()
135+
93136
def test_regular_org_member_can_dismiss(self) -> None:
94137
member_user = self.create_user()
95138
self.create_member(user=member_user, organization=self.org, role="member")

‎tests/sentry/auth/test_principal.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ def _is_request_user_type_check(node: ast.AST) -> bool:
275275

276276

277277
# Lower these as callers move off. Exact, so the baseline can't drift back up.
278-
REQUEST_USER_ID_BASELINE = 133
278+
REQUEST_USER_ID_BASELINE = 131
279279
REQUEST_USER_TYPE_CHECK_BASELINE = 29
280-
REQUEST_USER_ALLOWLIST_BASELINE = 12
280+
REQUEST_USER_ALLOWLIST_BASELINE = 10
281281

282282

283283
def test_request_user_id_uses_only_go_down() -> None:

‎tools/mypy_helpers/request_user_allowlist.txt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
src/sentry/api/bases/organizationmember.py
22
src/sentry/api/endpoints/organization_releases.py
3-
src/sentry/api/endpoints/prompts_activity.py
43
src/sentry/core/endpoints/organization_member_details.py
54
src/sentry/core/endpoints/organization_member_index.py
65
src/sentry/core/endpoints/organization_member_requests_invite_index.py
7-
src/sentry/dashboards/endpoints/organization_dashboards.py
86
src/sentry/data_export/endpoints/data_export.py
97
src/sentry/issues/endpoints/group_integration_details.py
108
src/sentry/notebooks/endpoints/base.py

0 commit comments

Comments
 (0)