From fcff01e46eccbb546160edaf09585ac44c450e94 Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Thu, 20 Nov 2025 17:33:12 -0800 Subject: [PATCH 1/2] fix(aci): simplify DetectorWorkflow connection permission requirements --- .../endpoints/validators/detector_workflow.py | 2 +- ...st_organization_detector_workflow_index.py | 173 ++---------------- 2 files changed, 12 insertions(+), 163 deletions(-) diff --git a/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py b/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py index 7104a44768005a..aa03f99015ecad 100644 --- a/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py +++ b/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py @@ -74,7 +74,7 @@ def can_edit_detector_workflow_connections(detector: Detector, request: Request) Anyone with alert write access to the project can connect/disconnect detectors of any type, which is slightly different from full edit access which differs by detector type. """ - return can_edit_user_created_detectors(request, detector.project) + return request.access.has_scope("alerts:write") def validate_detectors_exist_and_have_permissions( diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_index.py index c0429f30fa05e5..e88513b0ec905b 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_index.py @@ -2,7 +2,6 @@ from unittest.mock import MagicMock, call from sentry import audit_log -from sentry.grouping.grouptype import ErrorGroupType from sentry.testutils.cases import APITestCase from sentry.testutils.silo import region_silo_test from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow @@ -194,40 +193,10 @@ def test_missing_body_params(self) -> None: self.organization.slug, ) - def test_team_admin_can_connect_detectors(self) -> None: - self.login_as(user=self.team_admin_user) - self.organization.update_option("sentry:alerts_member_write", False) - - detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[self.team]), - created_by_id=self.user.id, - ) - body_params = { - "detectorId": detector.id, - "workflowId": self.unconnected_workflow.id, - } - self.get_success_response( - self.organization.slug, - **body_params, - ) - - def test_team_admin_cannot_connect_detectors_for_other_projects(self) -> None: - self.login_as(user=self.team_admin_user) - self.organization.update_option("sentry:alerts_member_write", False) - - other_detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[]), - created_by_id=self.user.id, - ) - body_params = { - "detectorId": other_detector.id, - "workflowId": self.unconnected_workflow.id, - } - self.get_error_response(self.organization.slug, **body_params, status_code=403) - - def test_member_can_connect_user_created_detectors(self) -> None: + def test_member_can_connect_detectors_with_alerts_write(self) -> None: self.organization.flags.allow_joinleave = False self.organization.save() + self.organization.update_option("sentry:alerts_member_write", True) self.login_as(user=self.member_user) detector = self.create_detector( @@ -243,28 +212,10 @@ def test_member_can_connect_user_created_detectors(self) -> None: **body_params, ) - def test_member_can_connect_system_created_detectors(self) -> None: - self.organization.flags.allow_joinleave = False - self.organization.save() - self.login_as(user=self.member_user) - - detector = self.create_detector( - project=self.create_project(organization=self.organization), - created_by_id=None, - type=ErrorGroupType.slug, - ) - body_params = { - "detectorId": detector.id, - "workflowId": self.unconnected_workflow.id, - } - self.get_success_response( - self.organization.slug, - **body_params, - ) - - def test_member_cannot_connect_detectors_for_other_projects(self) -> None: + def test_member_can_connect_detectors_for_other_projects_with_alerts_write(self) -> None: self.organization.flags.allow_joinleave = False self.organization.save() + self.organization.update_option("sentry:alerts_member_write", True) self.login_as(user=self.member_user) other_detector = self.create_detector( @@ -275,9 +226,9 @@ def test_member_cannot_connect_detectors_for_other_projects(self) -> None: "detectorId": other_detector.id, "workflowId": self.unconnected_workflow.id, } - self.get_error_response(self.organization.slug, **body_params, status_code=403) + self.get_success_response(self.organization.slug, **body_params) - def test_member_cannot_connect_detectors_when_alerts_member_write_disabled(self) -> None: + def test_member_cannot_connect_detectors_without_alerts_write(self) -> None: self.organization.update_option("sentry:alerts_member_write", False) self.organization.flags.allow_joinleave = True self.organization.save() @@ -365,75 +316,10 @@ def test_missing_ids(self) -> None: self.organization.slug, ) - def test_team_admin_can_disconnect_user_detectors(self) -> None: - self.login_as(user=self.team_admin_user) - - detector = self.create_detector( - project=self.create_project(organization=self.organization), - created_by_id=self.user.id, - ) - self.create_detector_workflow( - detector=detector, - workflow=self.workflow_1, - ) - self.get_success_response( - self.organization.slug, - qs_params={"detector_id": detector.id, "workflow_id": self.workflow_1.id}, - ) - - def test_team_admin_can_disconnect_sentry_detectors(self) -> None: - self.login_as(user=self.team_admin_user) - - sentry_detector = self.create_detector( - project=self.create_project(organization=self.organization), - ) - self.create_detector_workflow( - detector=sentry_detector, - workflow=self.workflow_1, - ) - self.get_success_response( - self.organization.slug, - qs_params={"detector_id": sentry_detector.id, "workflow_id": self.workflow_1.id}, - ) - - def test_team_admin_can_disconnect_detectors_for_accessible_projects(self) -> None: - self.login_as(user=self.team_admin_user) - self.organization.update_option("sentry:alerts_member_write", False) - - project_detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[self.team]), - created_by_id=self.user.id, - ) - self.create_detector_workflow( - detector=project_detector, - workflow=self.workflow_1, - ) - self.get_success_response( - self.organization.slug, - qs_params={"detector_id": project_detector.id, "workflow_id": self.workflow_1.id}, - ) - - def test_team_admin_cannot_disconnect_detectors_for_other_projects(self) -> None: - self.login_as(user=self.team_admin_user) - self.organization.update_option("sentry:alerts_member_write", False) - - other_detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[]), - created_by_id=self.user.id, - ) - self.create_detector_workflow( - detector=other_detector, - workflow=self.workflow_1, - ) - self.get_error_response( - self.organization.slug, - qs_params={"detector_id": other_detector.id, "workflow_id": self.workflow_1.id}, - status_code=403, - ) - - def test_member_can_disconnect_user_detectors(self) -> None: + def test_member_can_disconnect_detectors_with_alerts_write(self) -> None: self.organization.flags.allow_joinleave = False self.organization.save() + self.organization.update_option("sentry:alerts_member_write", True) self.login_as(user=self.member_user) detector = self.create_detector( @@ -449,9 +335,10 @@ def test_member_can_disconnect_user_detectors(self) -> None: qs_params={"detector_id": detector.id, "workflow_id": self.workflow_1.id}, ) - def test_member_cannot_disconnect_detectors_for_other_projects(self) -> None: + def test_member_cannot_disconnect_detectors_without_alerts_write(self) -> None: self.organization.flags.allow_joinleave = False self.organization.save() + self.organization.update_option("sentry:alerts_member_write", False) self.login_as(user=self.member_user) other_detector = self.create_detector( @@ -468,45 +355,6 @@ def test_member_cannot_disconnect_detectors_for_other_projects(self) -> None: status_code=403, ) - def test_member_can_disconnect_system_created_detectors(self) -> None: - self.organization.flags.allow_joinleave = False - self.organization.save() - self.login_as(user=self.member_user) - - sentry_detector = self.create_detector( - project=self.create_project(organization=self.organization), - type=ErrorGroupType.slug, - created_by_id=None, - ) - self.create_detector_workflow( - detector=sentry_detector, - workflow=self.workflow_1, - ) - self.get_success_response( - self.organization.slug, - qs_params={"detector_id": sentry_detector.id, "workflow_id": self.workflow_1.id}, - ) - - def test_member_cannot_disconnect_detectors_when_alerts_member_write_disabled(self) -> None: - self.organization.update_option("sentry:alerts_member_write", False) - self.organization.flags.allow_joinleave = True - self.organization.save() - self.login_as(user=self.member_user) - - detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[self.team]), - created_by_id=self.user.id, - ) - self.create_detector_workflow( - detector=detector, - workflow=self.workflow_1, - ) - self.get_error_response( - self.organization.slug, - qs_params={"detector_id": detector.id, "workflow_id": self.workflow_1.id}, - status_code=403, - ) - def test_batch_delete_no_permission(self) -> None: self.organization.update_option("sentry:alerts_member_write", False) self.login_as(user=self.member_user) @@ -520,6 +368,7 @@ def test_batch_delete_no_permission(self) -> None: detector=detector, workflow=self.workflow_1, ) + # Member lacks alerts:write because alerts_member_write is disabled self.get_error_response( self.organization.slug, qs_params={"workflow_id": self.workflow_1.id}, From d3bb1daf3e9c5b1b0423e44bbc595915d877389e Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Fri, 21 Nov 2025 16:32:33 -0800 Subject: [PATCH 2/2] fix tests, remove unnecessary tests --- ..._organization_detector_workflow_details.py | 91 +------------------ .../test_organization_workflow_index.py | 20 +++- 2 files changed, 16 insertions(+), 95 deletions(-) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_details.py index 356a42f959e95e..c6aba543c3c52f 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_details.py @@ -92,77 +92,8 @@ def test_does_not_exist(self) -> None: object_id=self.detector_workflow.id, ).exists() - def test_team_admin_can_disconnect_user_detectors(self) -> None: - self.login_as(user=self.team_admin_user) - - detector = self.create_detector( - project=self.create_project(organization=self.organization), - created_by_id=self.user.id, - ) - detector_workflow = self.create_detector_workflow( - detector=detector, - workflow=self.workflow, - ) - with outbox_runner(): - self.get_success_response( - self.organization.slug, - detector_workflow.id, - ) - - def test_team_admin_can_disconnect_sentry_detectors(self) -> None: - self.login_as(user=self.team_admin_user) - - sentry_detector = self.create_detector( - project=self.create_project(organization=self.organization), - ) - detector_workflow = self.create_detector_workflow( - detector=sentry_detector, - workflow=self.workflow, - ) - with outbox_runner(): - self.get_success_response( - self.organization.slug, - detector_workflow.id, - ) - - def test_team_admin_can_disconnect_detectors_for_accessible_projects(self) -> None: - self.login_as(user=self.team_admin_user) - self.organization.update_option("sentry:alerts_member_write", False) - - project_detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[self.team]), - created_by_id=self.user.id, - ) - detector_workflow = self.create_detector_workflow( - detector=project_detector, - workflow=self.workflow, - ) - with outbox_runner(): - self.get_success_response( - self.organization.slug, - detector_workflow.id, - ) - - def test_team_admin_cannot_disconnect_detectors_for_other_projects(self) -> None: - self.login_as(user=self.team_admin_user) - self.organization.update_option("sentry:alerts_member_write", False) - - other_detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[]), - created_by_id=self.user.id, - ) - detector_workflow = self.create_detector_workflow( - detector=other_detector, - workflow=self.workflow, - ) - with outbox_runner(): - self.get_error_response( - self.organization.slug, - detector_workflow.id, - status_code=403, - ) - def test_member_can_disconnect_user_detectors(self) -> None: + self.organization.update_option("sentry:alerts_member_write", True) self.organization.flags.allow_joinleave = False self.organization.save() self.login_as(user=self.member_user) @@ -181,26 +112,6 @@ def test_member_can_disconnect_user_detectors(self) -> None: detector_workflow.id, ) - def test_member_cannot_disconnect_detectors_for_other_projects(self) -> None: - self.organization.flags.allow_joinleave = False - self.organization.save() - self.login_as(user=self.member_user) - - other_detector = self.create_detector( - project=self.create_project(organization=self.organization, teams=[]), - created_by_id=self.user.id, - ) - detector_workflow = self.create_detector_workflow( - detector=other_detector, - workflow=self.workflow, - ) - with outbox_runner(): - self.get_error_response( - self.organization.slug, - detector_workflow.id, - status_code=403, - ) - def test_member_cannot_disconnect_detectors_when_alerts_member_write_disabled(self) -> None: self.organization.update_option("sentry:alerts_member_write", False) self.organization.flags.allow_joinleave = True 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 58c2cad1d4fe04..87225e388a03f8 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -621,7 +621,8 @@ def test_create_workflow_with_invalid_detector_ids(self) -> None: assert Workflow.objects.count() == 0 - def test_create_workflow_with_unauthorized_detectors(self) -> None: + def test_create_workflow_with_other_project_detector(self) -> None: + self.organization.update_option("sentry:alerts_member_write", True) self.organization.flags.allow_joinleave = False self.organization.save() @@ -638,15 +639,24 @@ def test_create_workflow_with_unauthorized_detectors(self) -> None: "detectorIds": [other_detector.id], } - self.get_error_response( + self.get_success_response( self.organization.slug, raw_data=workflow_data, - status_code=403, ) - # Verify no detector-workflow connections were created + # Verify detector-workflow connections was created created_detector_workflows = DetectorWorkflow.objects.all() - assert created_detector_workflows.count() == 0 + assert created_detector_workflows.count() == 1 + + def test_cannot_create_workflow_without_alerts_write(self) -> None: + self.organization.update_option("sentry:alerts_member_write", False) + self.login_as(user=self.member_user) + + self.get_error_response( + self.organization.slug, + raw_data=self.valid_workflow, + status_code=403, + ) @region_silo_test