Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/sentry/api/bases/organization_request_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@


class OrganizationRequestChangeEndpointPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "This endpoint only files a request for an organization change; members use it without organization write access.",
}
# just requesting so read permission is enough
scope_map = {
"POST": ["org:read"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@


class RepositoryTokenRegeneratePermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Codecov token regeneration preserves read-only token access for now.",
}
scope_map = {
"GET": ["org:read", "org:write", "org:admin"],
"POST": ["org:read", "org:write", "org:admin"],
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/codecov/endpoints/sync_repos/sync_repos.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@


class SyncReposPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Codecov sync preserves read-only token access pending integration-scope cleanup.",
}
scope_map = {
"GET": ["org:read", "org:write", "org:admin"],
"POST": ["org:read", "org:write", "org:admin"],
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/conduit/endpoints/organization_conduit_demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ class OrganizationConduitDemoPermission(OrganizationPermission):
This is a demo-only feature and doesn't modify organization state.
"""

readonly_mutation_scope_exceptions = {
"POST": "Demo credential generation preserves read-only token access for now.",
}
scope_map = {
"POST": ["org:read", "org:write", "org:admin"],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@


class MemberInvitePermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Members can create invite requests on this endpoint even when they cannot send invites directly.",
}
scope_map = {
"GET": ["member:read", "member:write", "member:admin"],
# We will do an additional check to see if a user can invite members. If
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@


class MemberInviteDetailsPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"DELETE": "Invite deletion keeps current member-read token semantics for now.",
}
scope_map = {
"GET": ["member:read", "member:write", "member:admin"],
"PUT": ["member:write", "member:admin"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@


class InviteRequestPermissions(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Invite request creation keeps member-read token access for now.",
}
scope_map = {
"GET": ["member:read", "member:write", "member:admin"],
"POST": ["member:read", "member:write", "member:admin"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ def serialize(


class OrganizationTeamMemberPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Team membership writes keep current mixed org/member/team token semantics for now.",
"PUT": "Team membership writes keep current mixed org/member/team token semantics for now.",
"DELETE": "Team membership writes keep current mixed org/member/team token semantics for now.",
}
scope_map = {
"GET": [
"org:read",
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/core/endpoints/organization_member_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ class MemberConflictValidationError(serializers.ValidationError):


class RelaxedMemberPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"DELETE": "Member deletion keeps self-service and role-comparison semantics for now.",
}
scope_map = {
"GET": ["member:read", "member:write", "member:admin"],
"POST": ["member:write", "member:admin"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class DashboardGenerateSerializer(serializers.Serializer[dict[str, Any]]):


class OrganizationDashboardGeneratePermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Dashboard generation is a POST helper/action and needs separate contract cleanup.",
}
scope_map = {
"POST": ["org:read"],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@


class NotificationActionsPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Notification action writes also rely on project-scoped checks; cleanup is deferred.",
"PUT": "Notification action writes also rely on project-scoped checks; cleanup is deferred.",
"DELETE": "Notification action writes also rely on project-scoped checks; cleanup is deferred.",
}
scope_map = {
"GET": ["org:read", "org:write", "org:admin"],
"POST": ["org:read", "org:write", "org:admin"],
Expand Down
4 changes: 4 additions & 0 deletions src/sentry/preprod/api/bases/preprod_artifact_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class BasePreprodArtifactResourceDoesNotExist(APIException):

# This is not a general permission. It specifically for triggering comparisons.
class ProjectPreprodArtifactPermission(OrganizationEventPermission):
readonly_mutation_scope_exceptions = {
"POST": "Preprod comparison triggers preserve read-only token access for now.",
"PUT": "Preprod comparison triggers preserve read-only token access for now.",
}
scope_map = {
"GET": ["event:read", "event:write", "event:admin"],
# Some simple actions, like triggering comparisons, should be allowed
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/seer/endpoints/issue_view_title_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@


class IssueViewTitleGeneratePermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Issue title generation is a read-like POST helper and needs separate cleanup.",
}
scope_map = {
"POST": ["org:read"],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class SeerExplorerChatSerializer(serializers.Serializer):


class OrganizationSeerExplorerChatPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Seer explorer chat is a read-like POST helper and needs separate cleanup.",
}
scope_map = {
"GET": ["org:read"],
"POST": ["org:read"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@


class OrganizationSeerExplorerUpdatePermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Seer explorer updates are POST helpers and need separate contract cleanup.",
}
scope_map = {
"POST": ["org:read"],
}
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/seer/endpoints/organization_seer_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@
class SeerRpcPermission(OrganizationPermission):
# Seer RPCs uses POST requests but is actually read only
# So relax the permissions here.
readonly_mutation_scope_exceptions = {
"POST": "Seer RPC POST is intentionally read-only and tracked separately.",
}
scope_map = {
"POST": ["org:read", "org:write", "org:admin"],
}
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/seer/endpoints/organization_trace_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@


class OrganizationTraceSummaryPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Trace summary is a read-like POST helper and needs separate contract cleanup.",
}
scope_map = {
"POST": ["org:read"],
}
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/seer/endpoints/trace_explorer_ai_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@


class OrganizationTraceExplorerAIPermission(OrganizationPermission):
readonly_mutation_scope_exceptions = {
"POST": "Trace explorer POST helpers are read-like actions and need separate cleanup.",
}
scope_map = {
"GET": ["org:read"],
"POST": ["org:read"],
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/sentry_apps/api/bases/sentryapps.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,9 @@ def convert_args(self, request: Request, uuid, *args, **kwargs):


class SentryAppInstallationExternalIssuePermission(SentryAppInstallationPermission):
readonly_mutation_scope_exceptions = {
"POST": "This endpoint creates an external issue link for an issue the caller can already read.",
}
scope_map = {
"POST": ("event:read", "event:write", "event:admin"),
"DELETE": ("event:admin",),
Expand Down
89 changes: 89 additions & 0 deletions tests/sentry/api/test_permissions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
from collections.abc import Generator

from django.test import SimpleTestCase
from django.urls import URLPattern, URLResolver
from django.urls.resolvers import get_resolver
from rest_framework.views import APIView

from sentry.api.base import Endpoint
from sentry.api.permissions import (
DemoSafePermission,
DisallowImpersonatedTokenCreation,
Expand All @@ -8,10 +14,34 @@
SuperuserOrStaffFeatureFlaggedPermission,
SuperuserPermission,
)
from sentry.conf.server import SENTRY_READONLY_SCOPES
from sentry.demo_mode.utils import READONLY_SCOPES
from sentry.organizations.services.organization import organization_service
from sentry.testutils.cases import DRFPermissionTestCase
from sentry.testutils.helpers.options import override_options
from sentry.testutils.silo import no_silo_test

MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"})


def _iter_endpoint_view_classes(urlpatterns, base: str = "") -> Generator[type[Endpoint]]:
for pattern in urlpatterns:
if isinstance(pattern, URLResolver):
yield from _iter_endpoint_view_classes(
pattern.url_patterns, base + str(pattern.pattern)
)
elif isinstance(pattern, URLPattern):
callback = pattern.callback
if hasattr(callback, "view_class") and issubclass(callback.view_class, Endpoint):
yield callback.view_class


def _get_class_path(cls: type[object]) -> str:
return f"{cls.__module__}.{cls.__name__}"


def _get_readonly_mutation_scope_exceptions(cls: type[object]) -> dict[str, str]:
return getattr(cls, "readonly_mutation_scope_exceptions", {}) or {}


class PermissionsTest(DRFPermissionTestCase):
Expand Down Expand Up @@ -223,3 +253,62 @@ def test_determine_access_no_demo_users(self) -> None:
)

assert readonly_rpc_context.member.scopes == list(self.org_member_scopes)


@no_silo_test
class PublishedMutationScopeTest(SimpleTestCase):
def test_readonly_mutation_scope_exceptions_are_notes(self) -> None:
for view_cls in sorted(
set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path
):
for cls in (view_cls, *getattr(view_cls, "permission_classes", ())):
exceptions = getattr(cls, "readonly_mutation_scope_exceptions", None)
if exceptions is None:
continue

assert isinstance(exceptions, dict), (
f"{_get_class_path(cls)} readonly_mutation_scope_exceptions must be a dict"
)

for method, note in exceptions.items():
assert method in MUTATION_METHODS, (
f"{_get_class_path(cls)} readonly_mutation_scope_exceptions[{method!r}] "
"must target a mutation method"
)
assert isinstance(note, str) and note.strip(), (
f"{_get_class_path(cls)} readonly_mutation_scope_exceptions[{method!r}] "
"must be a non-empty note"
)

def test_published_mutation_endpoints_require_readonly_scope_notes(self) -> None:
missing_notes = []

for view_cls in sorted(
set(_iter_endpoint_view_classes(get_resolver().url_patterns)), key=_get_class_path
):
publish_status = getattr(view_cls, "publish_status", {}) or {}
permission_classes = getattr(view_cls, "permission_classes", ()) or ()
view_exceptions = _get_readonly_mutation_scope_exceptions(view_cls)

for method in MUTATION_METHODS & set(publish_status):
for permission_cls in permission_classes:
readonly_scopes = (
set(getattr(permission_cls, "scope_map", {}).get(method, ()))
& SENTRY_READONLY_SCOPES
)
if not readonly_scopes:
continue

if view_exceptions.get(method) or _get_readonly_mutation_scope_exceptions(
permission_cls
).get(method):
continue

missing_notes.append(
f"{_get_class_path(view_cls)} {method} accepts readonly scopes "
f"{sorted(readonly_scopes)} via {_get_class_path(permission_cls)}. "
"Remove the readonly scopes or add "
"readonly_mutation_scope_exceptions[method] with a justification note."
)

assert not missing_notes, "\n".join(missing_notes)
Loading