diff --git a/src/sentry/api/permissions.py b/src/sentry/api/permissions.py index e6078edbc1af..9c97359e8446 100644 --- a/src/sentry/api/permissions.py +++ b/src/sentry/api/permissions.py @@ -5,11 +5,13 @@ from typing import TYPE_CHECKING, Any from django.conf import settings +from rest_framework.exceptions import PermissionDenied from rest_framework.permissions import SAFE_METHODS, BasePermission, IsAuthenticated # noqa: S012 from rest_framework.request import Request from sentry.api.exceptions import ( INSUFFICIENT_SCOPE_ATTR, + InsufficientScope, MemberDisabledOverLimit, SsoRequired, SuperuserRequired, @@ -21,6 +23,7 @@ from sentry.auth.system import is_system_auth from sentry.demo_mode.utils import get_readonly_scopes, is_demo_mode_enabled, is_demo_user from sentry.hybridcloud.rpc import extract_id_from +from sentry.models.apiscopes import add_scope_hierarchy from sentry.models.orgauthtoken import is_org_auth_token_auth, update_org_auth_token_last_used from sentry.organizations.services.organization import ( RpcOrganization, @@ -48,6 +51,21 @@ def _least_privileged_scope(allowed_scopes: set[str]) -> str | None: return min(grantable_scopes) if grantable_scopes else None +def enforce_scope(request: Request, required_scope: str) -> None: + """Require a scope and distinguish token failures from other denials.""" + if request.access.has_scope(required_scope): + return + if required_scope in add_scope_hierarchy(list(request.access.scopes)): + return + if ( + agent_token.is_agent_auth(request.auth) + and required_scope not in settings.SENTRY_TOKEN_ONLY_SCOPES + and request.access.would_have_scope_with_added_auth_scope(required_scope) + ): + raise InsufficientScope([required_scope]) + raise PermissionDenied + + class RelayPermission(BasePermission): def has_permission(self, request: Request, view: object) -> bool: return getattr(request, "relay", None) is not None diff --git a/src/sentry/auth/access.py b/src/sentry/auth/access.py index 7ff06f2f7799..c47ecbf333f6 100644 --- a/src/sentry/auth/access.py +++ b/src/sentry/auth/access.py @@ -21,6 +21,7 @@ from sentry.auth.system import is_system_auth from sentry.constants import ObjectStatus from sentry.data_secrecy.logic import should_allow_superuser_access +from sentry.models.apiscopes import add_scope_hierarchy from sentry.models.organization import Organization from sentry.models.organizationmember import OrganizationMember from sentry.models.organizationmemberteam import OrganizationMemberTeam @@ -132,6 +133,11 @@ def has_scope(self, scope: str) -> bool: check_scope_declaration(scope) return scope in self.scopes + def would_have_scope_with_added_auth_scope(self, scope: str) -> bool: + """Whether adding ``scope`` to the current auth scope cap would grant it.""" + check_scope_declaration(scope) + return False + def get_organization_role(self) -> OrganizationRole | None: if self.role is not None: return organization_roles.get(self.role) @@ -236,6 +242,15 @@ class DbAccess(Access): def role(self) -> str | None: return self._member.role if self._member else None + def would_have_scope_with_added_auth_scope(self, scope: str) -> bool: + check_scope_declaration(scope) + if self._member is None or self.scopes_upper_bound is None: + return False + candidate_scopes = _intersect_member_and_token_scopes( + self._member.get_scopes(), self.scopes_upper_bound | {scope} + ) + return scope in add_scope_hierarchy(list(candidate_scopes)) + @cached_property def _team_memberships(self) -> Mapping[Team, OrganizationMemberTeam]: if self._member is None: @@ -467,6 +482,16 @@ def scopes(self) -> frozenset[str]: self.scopes_upper_bound, ) + def would_have_scope_with_added_auth_scope(self, scope: str) -> bool: + check_scope_declaration(scope) + member = self.rpc_user_organization_context.member + if member is None or self.scopes_upper_bound is None: + return False + candidate_scopes = _intersect_member_and_token_scopes( + member.scopes, self.scopes_upper_bound | {scope} + ) + return scope in add_scope_hierarchy(list(candidate_scopes)) + # TODO(cathy): remove this @property def role(self) -> str | None: diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py index d560220e9847..c7fa4decdcbf 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_index.py @@ -77,6 +77,7 @@ from sentry.workflow_engine.endpoints.validators.utils import ( is_workflow_connected_to_all_projects_detector, should_include_all_projects_detector_workflows, + should_include_all_projects_detector_workflows_or_raise, ) from sentry.workflow_engine.models import DetectorWorkflow, Workflow from sentry.workflow_engine.models.workflow_fire_history import WorkflowFireHistory @@ -136,7 +137,7 @@ def convert_args( workflow = kwargs["workflow"] organization = kwargs["organization"] if is_workflow_connected_to_all_projects_detector(workflow): - if not should_include_all_projects_detector_workflows(request, organization): + if not should_include_all_projects_detector_workflows_or_raise(request, organization): raise PermissionDenied return args, kwargs @@ -261,14 +262,26 @@ def _get_workflows_for_mutation( queryset = self.filter_workflows(request, organization) workflows = list(queryset) - if not workflows: - return queryset, workflows - if raw_idlist := request.GET.getlist("id"): requested_ids = set(to_valid_int_id_list("id", raw_idlist)) - if requested_ids != {workflow.id for workflow in workflows}: + missing_workflow_ids = requested_ids - {workflow.id for workflow in workflows} + if missing_workflow_ids: + all_projects_detector = get_all_projects_detector(organization.id) + if ( + all_projects_detector + and DetectorWorkflow.objects.filter( + detector_id=all_projects_detector.id, + workflow_id__in=missing_workflow_ids, + ).exists() + ): + should_include_all_projects_detector_workflows_or_raise(request, organization) + if not workflows: + return queryset, workflows raise PermissionDenied + if not workflows: + return queryset, workflows + if not can_edit_workflows(workflows, request): raise PermissionDenied diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index 8d2ff7b74fd5..c1cdefe9515f 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -12,6 +12,7 @@ from rest_framework.request import Request from sentry import audit_log, features +from sentry.api.permissions import enforce_scope from sentry.issues import grouptype from sentry.models.organization import Organization from sentry.models.project import Project @@ -414,11 +415,22 @@ def should_include_all_projects_detector(request: Request, organization: Organiz def should_include_all_projects_detector_workflows( request: Request, organization: Organization ) -> bool: - """ - The flag is always required to show these workflows, but if it isn't a GET request, also check - that the caller has org:write. alerts:write is not sufficient to connect an all projects detector. - """ return features.has("organizations:workflow-engine-all-projects-detector", organization) and ( request.method == "GET" or can_edit_all_project_detector_workflow_connections(request=request) ) + + +def should_include_all_projects_detector_workflows_or_raise( + request: Request, organization: Organization +) -> bool: + """ + The flag is always required to show these workflows, but if it isn't a GET request, also check + that the caller has org:write. alerts:write is not sufficient to connect an all projects detector. + """ + if not features.has("organizations:workflow-engine-all-projects-detector", organization): + return False + if request.method == "GET": + return True + enforce_scope(request, "org:write") + return True diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py index 4339670a6c2c..09d675ad5ceb 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py @@ -2,6 +2,8 @@ from unittest import mock import responses +from django.test import override_settings +from rest_framework.test import APIClient from sentry import audit_log from sentry.api.serializers import serialize @@ -12,6 +14,7 @@ from sentry.incidents.grouptype import MetricIssue from sentry.models.auditlogentry import AuditLogEntry from sentry.models.rule import Rule +from sentry.seer import agent_token from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase from sentry.testutils.helpers import TaskRunner @@ -37,6 +40,8 @@ ProjectAccessTestMixin, ) +AGENT_TOKEN_SECRET = "test-seer-api-shared-secret-thirty-two-bytes!" + class OrganizationWorkflowDetailsBaseTest(APITestCase): endpoint = "sentry-api-0-organization-workflow-details" @@ -235,6 +240,7 @@ def test_update_rejects_non_object_actions(self) -> None: status_code=400, ) + @with_feature("organizations:workflow-engine-all-projects-detector") def test_all_projects_workflow_requires_org_write(self) -> None: detector = ensure_default_all_projects_detector(self.organization.id) self.create_detector_workflow(workflow=self.workflow, detector=detector) @@ -255,6 +261,60 @@ def test_all_projects_workflow_requires_org_write(self) -> None: self.workflow.refresh_from_db() assert self.workflow.name != "Unauthorized update" + @with_feature("organizations:workflow-engine-all-projects-detector") + @override_settings(SEER_API_SHARED_SECRET=AGENT_TOKEN_SECRET) + def test_all_projects_workflow_agent_token_advertises_org_write(self) -> None: + detector = ensure_default_all_projects_detector(self.organization.id) + self.create_detector_workflow(workflow=self.workflow, detector=detector) + token, _ = agent_token.encode_agent_token( + user_id=self.user.id, + organization_id=self.organization.id, + scopes=["org:read"], + session_id="workflow-update", + ) + client = APIClient() + + with self.feature(agent_token.FEATURE_FLAG): + response = client.put( + f"/api/0/organizations/{self.organization.slug}/workflows/{self.workflow.id}/", + data={**self.valid_workflow, "name": "Unauthorized update"}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token}", + ) + + assert response.status_code == 403, response.content + assert ( + response["WWW-Authenticate"] == 'Bearer error="insufficient_scope", scope="org:write"' + ) + + @with_feature("organizations:workflow-engine-all-projects-detector") + @override_settings(SEER_API_SHARED_SECRET=AGENT_TOKEN_SECRET) + def test_all_projects_workflow_agent_token_does_not_advertise_ungrantable_scope(self) -> None: + detector = ensure_default_all_projects_detector(self.organization.id) + self.create_detector_workflow(workflow=self.workflow, detector=detector) + user = self.create_user() + self.create_member( + user=user, organization=self.organization, role="member", teams=[self.team] + ) + token, _ = agent_token.encode_agent_token( + user_id=user.id, + organization_id=self.organization.id, + scopes=["org:read"], + session_id="workflow-update", + ) + client = APIClient() + + with self.feature(agent_token.FEATURE_FLAG): + response = client.put( + f"/api/0/organizations/{self.organization.slug}/workflows/{self.workflow.id}/", + data={**self.valid_workflow, "name": "Unauthorized update"}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {token}", + ) + + assert response.status_code == 403, response.content + assert "insufficient_scope" not in response.get("WWW-Authenticate", "") + def test_update_action_filter_with_string_encoded_id(self) -> None: dcg = DataConditionGroup.objects.create( organization=self.organization, diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py index d6a6fdc4200e..4a1ede826dca 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -3,6 +3,8 @@ from unittest import mock import responses +from django.test import override_settings +from rest_framework.test import APIClient from sentry import audit_log from sentry.api.serializers import serialize @@ -11,6 +13,7 @@ from sentry.deletions.tasks.scheduled import run_scheduled_deletions from sentry.grouping.grouptype import ErrorGroupType from sentry.incidents.grouptype import MetricIssue +from sentry.seer import agent_token from sentry.testutils.asserts import assert_org_audit_log_exists from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.features import with_feature @@ -35,6 +38,8 @@ ProjectAccessTestMixin, ) +AGENT_TOKEN_SECRET = "test-seer-api-shared-secret-thirty-two-bytes!" + class OrganizationWorkflowAPITestCase(APITestCase): endpoint = "sentry-api-0-organization-workflow-index" @@ -1536,6 +1541,17 @@ def setUp(self) -> None: organization_id=self.organization.id, name="Third Workflow", enabled=False ) + def _create_alerts_write_agent_client(self) -> APIClient: + token, _ = agent_token.encode_agent_token( + user_id=self.user.id, + organization_id=self.organization.id, + scopes=["alerts:write"], + session_id="workflow-update", + ) + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + return client + def test_team_admin_can_update_project_scoped_workflow(self) -> None: detector = self.create_detector(project=self.project) self.create_detector_workflow(workflow=self.workflow, detector=detector) @@ -1658,10 +1674,151 @@ def test_bulk_enable_workflows_by_ids_success(self) -> None: self.workflow_three.refresh_from_db() assert self.workflow_three.enabled is False + @with_feature("organizations:workflow-engine-all-projects-detector") + def test_bulk_enable_workflows_by_ids_including_all_projects(self) -> None: + all_projects_workflow = self.create_workflow( + organization_id=self.organization.id, enabled=False + ) + self.create_detector_workflow( + workflow=all_projects_workflow, + detector=ensure_default_all_projects_detector(self.organization.id), + ) + + response = self.get_success_response( + self.organization.slug, + qs_params=[ + ("id", str(self.workflow.id)), + ("id", str(all_projects_workflow.id)), + ], + raw_data={"enabled": True}, + ) + + self.workflow.refresh_from_db() + all_projects_workflow.refresh_from_db() + assert self.workflow.enabled is True + assert all_projects_workflow.enabled is True + assert {workflow["id"] for workflow in response.data} == { + str(self.workflow.id), + str(all_projects_workflow.id), + } + + @with_feature("organizations:workflow-engine-all-projects-detector") + @override_settings(SEER_API_SHARED_SECRET=AGENT_TOKEN_SECRET) + def test_agent_token_can_update_ordinary_workflow_when_all_projects_workflow_exists( + self, + ) -> None: + all_projects_workflow = self.create_workflow(organization_id=self.organization.id) + self.create_detector_workflow( + workflow=all_projects_workflow, + detector=ensure_default_all_projects_detector(self.organization.id), + ) + client = self._create_alerts_write_agent_client() + + with self.feature(agent_token.FEATURE_FLAG): + response = client.put( + f"/api/0/organizations/{self.organization.slug}/workflows/", + data={"enabled": True}, + format="json", + query_params={"id": str(self.workflow.id)}, + ) + + assert response.status_code == 200, response.content + self.workflow.refresh_from_db() + assert self.workflow.enabled is True + + @with_feature("organizations:workflow-engine-all-projects-detector") + @override_settings(SEER_API_SHARED_SECRET=AGENT_TOKEN_SECRET) + def test_agent_token_can_update_by_project_when_all_projects_workflow_exists(self) -> None: + self.create_detector_workflow( + workflow=self.workflow, + detector=self.create_detector(project=self.project), + ) + all_projects_workflow = self.create_workflow( + organization_id=self.organization.id, enabled=False + ) + self.create_detector_workflow( + workflow=all_projects_workflow, + detector=ensure_default_all_projects_detector(self.organization.id), + ) + client = self._create_alerts_write_agent_client() + + with self.feature(agent_token.FEATURE_FLAG): + response = client.put( + f"/api/0/organizations/{self.organization.slug}/workflows/", + data={"enabled": True}, + format="json", + query_params={"project": str(self.project.id)}, + ) + + assert response.status_code == 200, response.content + self.workflow.refresh_from_db() + all_projects_workflow.refresh_from_db() + assert self.workflow.enabled is True + assert all_projects_workflow.enabled is False + + @with_feature("organizations:workflow-engine-all-projects-detector") + @override_settings(SEER_API_SHARED_SECRET=AGENT_TOKEN_SECRET) + def test_agent_token_can_update_all_accessible_when_all_projects_workflow_exists(self) -> None: + all_projects_workflow = self.create_workflow( + organization_id=self.organization.id, enabled=False + ) + self.create_detector_workflow( + workflow=all_projects_workflow, + detector=ensure_default_all_projects_detector(self.organization.id), + ) + client = self._create_alerts_write_agent_client() + + with self.feature(agent_token.FEATURE_FLAG): + response = client.put( + f"/api/0/organizations/{self.organization.slug}/workflows/", + data={"enabled": True}, + format="json", + query_params={"projectSlug": "$all"}, + ) + + assert response.status_code == 200, response.content + self.workflow.refresh_from_db() + all_projects_workflow.refresh_from_db() + assert self.workflow.enabled is True + assert all_projects_workflow.enabled is False + + @with_feature("organizations:workflow-engine-all-projects-detector") + @override_settings(SEER_API_SHARED_SECRET=AGENT_TOKEN_SECRET) + def test_all_projects_workflow_agent_token_advertises_org_write(self) -> None: + all_projects_workflow = self.create_workflow( + organization_id=self.organization.id, enabled=False + ) + self.create_detector_workflow( + workflow=all_projects_workflow, + detector=ensure_default_all_projects_detector(self.organization.id), + ) + client = self._create_alerts_write_agent_client() + + with self.feature(agent_token.FEATURE_FLAG): + response = client.put( + f"/api/0/organizations/{self.organization.slug}/workflows/", + data={"enabled": True}, + format="json", + query_params={"id": str(all_projects_workflow.id)}, + ) + + assert response.status_code == 403, response.content + assert ( + response["WWW-Authenticate"] == 'Bearer error="insufficient_scope", scope="org:write"' + ) + + @with_feature("organizations:workflow-engine-all-projects-detector") def test_bulk_enable_all_projects_slug_sentinel_includes_detached_workflows(self) -> None: self.create_detector_workflow( workflow=self.workflow, detector=self.create_detector(project=self.project) ) + all_projects_workflow = self.create_workflow( + organization_id=self.organization.id, enabled=False + ) + self.create_detector_workflow( + workflow=all_projects_workflow, + detector=ensure_default_all_projects_detector(self.organization.id), + ) response = self.get_success_response( self.organization.slug, @@ -1672,13 +1829,16 @@ def test_bulk_enable_all_projects_slug_sentinel_includes_detached_workflows(self self.workflow.refresh_from_db() self.workflow_two.refresh_from_db() self.workflow_three.refresh_from_db() + all_projects_workflow.refresh_from_db() assert self.workflow.enabled is True assert self.workflow_two.enabled is True assert self.workflow_three.enabled is True + assert all_projects_workflow.enabled is True assert {workflow["id"] for workflow in response.data} == { str(self.workflow.id), str(self.workflow_two.id), str(self.workflow_three.id), + str(all_projects_workflow.id), } def test_bulk_disable_workflows_by_ids_success(self) -> None: