Skip to content

Commit 427e63e

Browse files
authored
fix(aci): simplify DetectorWorkflow connection permission requirements (#103799)
we've chosen to simplify permissions so that users only need `alerts:write` to connect any Detector-Workflows
1 parent 8a3d936 commit 427e63e

File tree

4 files changed

+28
-258
lines changed

4 files changed

+28
-258
lines changed

src/sentry/workflow_engine/endpoints/validators/detector_workflow.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ def can_edit_detector_workflow_connections(detector: Detector, request: Request)
9999
Anyone with alert write access to the project can connect/disconnect detectors of any type,
100100
which is slightly different from full edit access which differs by detector type.
101101
"""
102-
return can_edit_user_created_detectors(request, detector.project)
102+
return request.access.has_scope("alerts:write")
103103

104104

105105
def validate_detectors_exist_and_have_permissions(

tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_details.py

Lines changed: 1 addition & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -92,77 +92,8 @@ def test_does_not_exist(self) -> None:
9292
object_id=self.detector_workflow.id,
9393
).exists()
9494

95-
def test_team_admin_can_disconnect_user_detectors(self) -> None:
96-
self.login_as(user=self.team_admin_user)
97-
98-
detector = self.create_detector(
99-
project=self.create_project(organization=self.organization),
100-
created_by_id=self.user.id,
101-
)
102-
detector_workflow = self.create_detector_workflow(
103-
detector=detector,
104-
workflow=self.workflow,
105-
)
106-
with outbox_runner():
107-
self.get_success_response(
108-
self.organization.slug,
109-
detector_workflow.id,
110-
)
111-
112-
def test_team_admin_can_disconnect_sentry_detectors(self) -> None:
113-
self.login_as(user=self.team_admin_user)
114-
115-
sentry_detector = self.create_detector(
116-
project=self.create_project(organization=self.organization),
117-
)
118-
detector_workflow = self.create_detector_workflow(
119-
detector=sentry_detector,
120-
workflow=self.workflow,
121-
)
122-
with outbox_runner():
123-
self.get_success_response(
124-
self.organization.slug,
125-
detector_workflow.id,
126-
)
127-
128-
def test_team_admin_can_disconnect_detectors_for_accessible_projects(self) -> None:
129-
self.login_as(user=self.team_admin_user)
130-
self.organization.update_option("sentry:alerts_member_write", False)
131-
132-
project_detector = self.create_detector(
133-
project=self.create_project(organization=self.organization, teams=[self.team]),
134-
created_by_id=self.user.id,
135-
)
136-
detector_workflow = self.create_detector_workflow(
137-
detector=project_detector,
138-
workflow=self.workflow,
139-
)
140-
with outbox_runner():
141-
self.get_success_response(
142-
self.organization.slug,
143-
detector_workflow.id,
144-
)
145-
146-
def test_team_admin_cannot_disconnect_detectors_for_other_projects(self) -> None:
147-
self.login_as(user=self.team_admin_user)
148-
self.organization.update_option("sentry:alerts_member_write", False)
149-
150-
other_detector = self.create_detector(
151-
project=self.create_project(organization=self.organization, teams=[]),
152-
created_by_id=self.user.id,
153-
)
154-
detector_workflow = self.create_detector_workflow(
155-
detector=other_detector,
156-
workflow=self.workflow,
157-
)
158-
with outbox_runner():
159-
self.get_error_response(
160-
self.organization.slug,
161-
detector_workflow.id,
162-
status_code=403,
163-
)
164-
16595
def test_member_can_disconnect_user_detectors(self) -> None:
96+
self.organization.update_option("sentry:alerts_member_write", True)
16697
self.organization.flags.allow_joinleave = False
16798
self.organization.save()
16899
self.login_as(user=self.member_user)
@@ -181,26 +112,6 @@ def test_member_can_disconnect_user_detectors(self) -> None:
181112
detector_workflow.id,
182113
)
183114

184-
def test_member_cannot_disconnect_detectors_for_other_projects(self) -> None:
185-
self.organization.flags.allow_joinleave = False
186-
self.organization.save()
187-
self.login_as(user=self.member_user)
188-
189-
other_detector = self.create_detector(
190-
project=self.create_project(organization=self.organization, teams=[]),
191-
created_by_id=self.user.id,
192-
)
193-
detector_workflow = self.create_detector_workflow(
194-
detector=other_detector,
195-
workflow=self.workflow,
196-
)
197-
with outbox_runner():
198-
self.get_error_response(
199-
self.organization.slug,
200-
detector_workflow.id,
201-
status_code=403,
202-
)
203-
204115
def test_member_cannot_disconnect_detectors_when_alerts_member_write_disabled(self) -> None:
205116
self.organization.update_option("sentry:alerts_member_write", False)
206117
self.organization.flags.allow_joinleave = True

tests/sentry/workflow_engine/endpoints/test_organization_detector_workflow_index.py

Lines changed: 11 additions & 162 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
from unittest.mock import MagicMock, call
33

44
from sentry import audit_log
5-
from sentry.grouping.grouptype import ErrorGroupType
65
from sentry.testutils.cases import APITestCase
76
from sentry.testutils.silo import region_silo_test
87
from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow
@@ -194,40 +193,10 @@ def test_missing_body_params(self) -> None:
194193
self.organization.slug,
195194
)
196195

197-
def test_team_admin_can_connect_detectors(self) -> None:
198-
self.login_as(user=self.team_admin_user)
199-
self.organization.update_option("sentry:alerts_member_write", False)
200-
201-
detector = self.create_detector(
202-
project=self.create_project(organization=self.organization, teams=[self.team]),
203-
created_by_id=self.user.id,
204-
)
205-
body_params = {
206-
"detectorId": detector.id,
207-
"workflowId": self.unconnected_workflow.id,
208-
}
209-
self.get_success_response(
210-
self.organization.slug,
211-
**body_params,
212-
)
213-
214-
def test_team_admin_cannot_connect_detectors_for_other_projects(self) -> None:
215-
self.login_as(user=self.team_admin_user)
216-
self.organization.update_option("sentry:alerts_member_write", False)
217-
218-
other_detector = self.create_detector(
219-
project=self.create_project(organization=self.organization, teams=[]),
220-
created_by_id=self.user.id,
221-
)
222-
body_params = {
223-
"detectorId": other_detector.id,
224-
"workflowId": self.unconnected_workflow.id,
225-
}
226-
self.get_error_response(self.organization.slug, **body_params, status_code=403)
227-
228-
def test_member_can_connect_user_created_detectors(self) -> None:
196+
def test_member_can_connect_detectors_with_alerts_write(self) -> None:
229197
self.organization.flags.allow_joinleave = False
230198
self.organization.save()
199+
self.organization.update_option("sentry:alerts_member_write", True)
231200
self.login_as(user=self.member_user)
232201

233202
detector = self.create_detector(
@@ -243,28 +212,10 @@ def test_member_can_connect_user_created_detectors(self) -> None:
243212
**body_params,
244213
)
245214

246-
def test_member_can_connect_system_created_detectors(self) -> None:
247-
self.organization.flags.allow_joinleave = False
248-
self.organization.save()
249-
self.login_as(user=self.member_user)
250-
251-
detector = self.create_detector(
252-
project=self.create_project(organization=self.organization),
253-
created_by_id=None,
254-
type=ErrorGroupType.slug,
255-
)
256-
body_params = {
257-
"detectorId": detector.id,
258-
"workflowId": self.unconnected_workflow.id,
259-
}
260-
self.get_success_response(
261-
self.organization.slug,
262-
**body_params,
263-
)
264-
265-
def test_member_cannot_connect_detectors_for_other_projects(self) -> None:
215+
def test_member_can_connect_detectors_for_other_projects_with_alerts_write(self) -> None:
266216
self.organization.flags.allow_joinleave = False
267217
self.organization.save()
218+
self.organization.update_option("sentry:alerts_member_write", True)
268219
self.login_as(user=self.member_user)
269220

270221
other_detector = self.create_detector(
@@ -275,9 +226,9 @@ def test_member_cannot_connect_detectors_for_other_projects(self) -> None:
275226
"detectorId": other_detector.id,
276227
"workflowId": self.unconnected_workflow.id,
277228
}
278-
self.get_error_response(self.organization.slug, **body_params, status_code=403)
229+
self.get_success_response(self.organization.slug, **body_params)
279230

280-
def test_member_cannot_connect_detectors_when_alerts_member_write_disabled(self) -> None:
231+
def test_member_cannot_connect_detectors_without_alerts_write(self) -> None:
281232
self.organization.update_option("sentry:alerts_member_write", False)
282233
self.organization.flags.allow_joinleave = True
283234
self.organization.save()
@@ -365,75 +316,10 @@ def test_missing_ids(self) -> None:
365316
self.organization.slug,
366317
)
367318

368-
def test_team_admin_can_disconnect_user_detectors(self) -> None:
369-
self.login_as(user=self.team_admin_user)
370-
371-
detector = self.create_detector(
372-
project=self.create_project(organization=self.organization),
373-
created_by_id=self.user.id,
374-
)
375-
self.create_detector_workflow(
376-
detector=detector,
377-
workflow=self.workflow_1,
378-
)
379-
self.get_success_response(
380-
self.organization.slug,
381-
qs_params={"detector_id": detector.id, "workflow_id": self.workflow_1.id},
382-
)
383-
384-
def test_team_admin_can_disconnect_sentry_detectors(self) -> None:
385-
self.login_as(user=self.team_admin_user)
386-
387-
sentry_detector = self.create_detector(
388-
project=self.create_project(organization=self.organization),
389-
)
390-
self.create_detector_workflow(
391-
detector=sentry_detector,
392-
workflow=self.workflow_1,
393-
)
394-
self.get_success_response(
395-
self.organization.slug,
396-
qs_params={"detector_id": sentry_detector.id, "workflow_id": self.workflow_1.id},
397-
)
398-
399-
def test_team_admin_can_disconnect_detectors_for_accessible_projects(self) -> None:
400-
self.login_as(user=self.team_admin_user)
401-
self.organization.update_option("sentry:alerts_member_write", False)
402-
403-
project_detector = self.create_detector(
404-
project=self.create_project(organization=self.organization, teams=[self.team]),
405-
created_by_id=self.user.id,
406-
)
407-
self.create_detector_workflow(
408-
detector=project_detector,
409-
workflow=self.workflow_1,
410-
)
411-
self.get_success_response(
412-
self.organization.slug,
413-
qs_params={"detector_id": project_detector.id, "workflow_id": self.workflow_1.id},
414-
)
415-
416-
def test_team_admin_cannot_disconnect_detectors_for_other_projects(self) -> None:
417-
self.login_as(user=self.team_admin_user)
418-
self.organization.update_option("sentry:alerts_member_write", False)
419-
420-
other_detector = self.create_detector(
421-
project=self.create_project(organization=self.organization, teams=[]),
422-
created_by_id=self.user.id,
423-
)
424-
self.create_detector_workflow(
425-
detector=other_detector,
426-
workflow=self.workflow_1,
427-
)
428-
self.get_error_response(
429-
self.organization.slug,
430-
qs_params={"detector_id": other_detector.id, "workflow_id": self.workflow_1.id},
431-
status_code=403,
432-
)
433-
434-
def test_member_can_disconnect_user_detectors(self) -> None:
319+
def test_member_can_disconnect_detectors_with_alerts_write(self) -> None:
435320
self.organization.flags.allow_joinleave = False
436321
self.organization.save()
322+
self.organization.update_option("sentry:alerts_member_write", True)
437323
self.login_as(user=self.member_user)
438324

439325
detector = self.create_detector(
@@ -449,9 +335,10 @@ def test_member_can_disconnect_user_detectors(self) -> None:
449335
qs_params={"detector_id": detector.id, "workflow_id": self.workflow_1.id},
450336
)
451337

452-
def test_member_cannot_disconnect_detectors_for_other_projects(self) -> None:
338+
def test_member_cannot_disconnect_detectors_without_alerts_write(self) -> None:
453339
self.organization.flags.allow_joinleave = False
454340
self.organization.save()
341+
self.organization.update_option("sentry:alerts_member_write", False)
455342
self.login_as(user=self.member_user)
456343

457344
other_detector = self.create_detector(
@@ -468,45 +355,6 @@ def test_member_cannot_disconnect_detectors_for_other_projects(self) -> None:
468355
status_code=403,
469356
)
470357

471-
def test_member_can_disconnect_system_created_detectors(self) -> None:
472-
self.organization.flags.allow_joinleave = False
473-
self.organization.save()
474-
self.login_as(user=self.member_user)
475-
476-
sentry_detector = self.create_detector(
477-
project=self.create_project(organization=self.organization),
478-
type=ErrorGroupType.slug,
479-
created_by_id=None,
480-
)
481-
self.create_detector_workflow(
482-
detector=sentry_detector,
483-
workflow=self.workflow_1,
484-
)
485-
self.get_success_response(
486-
self.organization.slug,
487-
qs_params={"detector_id": sentry_detector.id, "workflow_id": self.workflow_1.id},
488-
)
489-
490-
def test_member_cannot_disconnect_detectors_when_alerts_member_write_disabled(self) -> None:
491-
self.organization.update_option("sentry:alerts_member_write", False)
492-
self.organization.flags.allow_joinleave = True
493-
self.organization.save()
494-
self.login_as(user=self.member_user)
495-
496-
detector = self.create_detector(
497-
project=self.create_project(organization=self.organization, teams=[self.team]),
498-
created_by_id=self.user.id,
499-
)
500-
self.create_detector_workflow(
501-
detector=detector,
502-
workflow=self.workflow_1,
503-
)
504-
self.get_error_response(
505-
self.organization.slug,
506-
qs_params={"detector_id": detector.id, "workflow_id": self.workflow_1.id},
507-
status_code=403,
508-
)
509-
510358
def test_batch_delete_no_permission(self) -> None:
511359
self.organization.update_option("sentry:alerts_member_write", False)
512360
self.login_as(user=self.member_user)
@@ -520,6 +368,7 @@ def test_batch_delete_no_permission(self) -> None:
520368
detector=detector,
521369
workflow=self.workflow_1,
522370
)
371+
# Member lacks alerts:write because alerts_member_write is disabled
523372
self.get_error_response(
524373
self.organization.slug,
525374
qs_params={"workflow_id": self.workflow_1.id},

tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,8 @@ def test_create_workflow_with_invalid_detector_ids(self) -> None:
668668

669669
assert Workflow.objects.count() == 0
670670

671-
def test_create_workflow_with_unauthorized_detectors(self) -> None:
671+
def test_create_workflow_with_other_project_detector(self) -> None:
672+
self.organization.update_option("sentry:alerts_member_write", True)
672673
self.organization.flags.allow_joinleave = False
673674
self.organization.save()
674675

@@ -685,15 +686,24 @@ def test_create_workflow_with_unauthorized_detectors(self) -> None:
685686
"detectorIds": [other_detector.id],
686687
}
687688

688-
self.get_error_response(
689+
self.get_success_response(
689690
self.organization.slug,
690691
raw_data=workflow_data,
691-
status_code=403,
692692
)
693693

694-
# Verify no detector-workflow connections were created
694+
# Verify detector-workflow connections was created
695695
created_detector_workflows = DetectorWorkflow.objects.all()
696-
assert created_detector_workflows.count() == 0
696+
assert created_detector_workflows.count() == 1
697+
698+
def test_cannot_create_workflow_without_alerts_write(self) -> None:
699+
self.organization.update_option("sentry:alerts_member_write", False)
700+
self.login_as(user=self.member_user)
701+
702+
self.get_error_response(
703+
self.organization.slug,
704+
raw_data=self.valid_workflow,
705+
status_code=403,
706+
)
697707

698708

699709
@region_silo_test

0 commit comments

Comments
 (0)