diff --git a/api-docs/paths/integration-platform/sentry-app-external-issue-details.json b/api-docs/paths/integration-platform/sentry-app-external-issue-details.json index cc021cdf1416..c86deed36b30 100644 --- a/api-docs/paths/integration-platform/sentry-app-external-issue-details.json +++ b/api-docs/paths/integration-platform/sentry-app-external-issue-details.json @@ -36,7 +36,7 @@ }, "security": [ { - "auth_token": ["event:admin"] + "auth_token": ["event:write", "event:admin"] } ] } diff --git a/src/sentry/issues/endpoints/bases/group.py b/src/sentry/issues/endpoints/bases/group.py index 0f312c5459ec..9d0944ddd96a 100644 --- a/src/sentry/issues/endpoints/bases/group.py +++ b/src/sentry/issues/endpoints/bases/group.py @@ -36,7 +36,7 @@ class GroupPermission(ProjectPermission): "GET": ["event:read", "event:write", "event:admin"], "POST": ["event:write", "event:admin"], "PUT": ["event:write", "event:admin"], - "DELETE": ["event:admin"], + "DELETE": ["event:write", "event:admin"], } def has_object_permission(self, request: Request, view: APIView, group: Any) -> bool: diff --git a/src/sentry/issues/endpoints/group_details.py b/src/sentry/issues/endpoints/group_details.py index 7b5878064166..4f70c1fd3ede 100644 --- a/src/sentry/issues/endpoints/group_details.py +++ b/src/sentry/issues/endpoints/group_details.py @@ -55,7 +55,7 @@ ) from sentry.issues.derived.check import record_status_consistency from sentry.issues.derived.gate import derived_should_be_correct, should_serve_action_log_activity -from sentry.issues.endpoints.bases.group import GroupEndpoint +from sentry.issues.endpoints.bases.group import GroupEndpoint, GroupPermission from sentry.issues.escalating.escalating_group_forecast import EscalatingGroupForecast from sentry.issues.models.groupactionlogentry import GroupActionLogEntry from sentry.issues.models.groupderiveddata import GroupDerivedData @@ -88,10 +88,19 @@ def get_group_global_count(group: Group) -> str: return str(group.times_seen_with_pending) +class GroupDetailsPermission(GroupPermission): + scope_map = { + **GroupPermission.scope_map, + # Preserve the organization's "Let Members Delete Events" restriction. + "DELETE": ["event:admin"], + } + + @extend_schema(tags=["Events"]) @cell_silo_endpoint class GroupDetailsEndpoint(GroupEndpoint): owner = ApiOwner.ISSUES + permission_classes = (GroupDetailsPermission,) publish_status = { "DELETE": ApiPublishStatus.PUBLIC, "GET": ApiPublishStatus.PUBLIC, diff --git a/src/sentry/sentry_apps/api/bases/sentryapps.py b/src/sentry/sentry_apps/api/bases/sentryapps.py index 2120b5727c7f..eb57487b92c4 100644 --- a/src/sentry/sentry_apps/api/bases/sentryapps.py +++ b/src/sentry/sentry_apps/api/bases/sentryapps.py @@ -470,7 +470,7 @@ def convert_args(self, request: Request, uuid, *args, **kwargs): class SentryAppInstallationExternalIssuePermission(SentryAppInstallationPermission): scope_map = { "POST": ("event:write", "event:admin"), - "DELETE": ("event:admin",), + "DELETE": ("event:write", "event:admin"), } diff --git a/tests/sentry/issues/endpoints/test_group_details.py b/tests/sentry/issues/endpoints/test_group_details.py index 010b104ee7e0..9ec1ca895202 100644 --- a/tests/sentry/issues/endpoints/test_group_details.py +++ b/tests/sentry/issues/endpoints/test_group_details.py @@ -1103,8 +1103,41 @@ def test_ratelimit(self) -> None: class GroupDeleteTest(APITestCase): + def test_delete_as_member_respects_organization_setting(self) -> None: + group = self.create_group() + member = self.create_user() + self.create_member( + user=member, organization=self.organization, role="member", teams=[self.team] + ) + self.login_as(user=member) + url = f"/api/0/organizations/{self.organization.slug}/issues/{group.id}/" + + self.organization.update_option("sentry:events_member_admin", False) + response = self.client.delete(url) + + assert response.status_code == 403, response.content + assert Group.objects.get(id=group.id).status == GroupStatus.UNRESOLVED + + self.organization.update_option("sentry:events_member_admin", True) + response = self.client.delete(url) + + assert response.status_code == 202, response.content + assert Group.objects.get(id=group.id).status == GroupStatus.PENDING_DELETION + + def test_delete_with_write_only_token(self) -> None: + group = self.create_group() + token = self.create_user_auth_token(user=self.user, scope_list=["event:write"]) + + response = self.client.delete( + f"/api/0/organizations/{self.organization.slug}/issues/{group.id}/", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 403 + assert Group.objects.get(id=group.id).status == GroupStatus.UNRESOLVED + def test_delete_deferred(self) -> None: - self.login_as(user=self.user) + token = self.create_user_auth_token(user=self.user, scope_list=["event:admin"]) group = self.create_group() hash = "x" * 32 @@ -1112,7 +1145,7 @@ def test_delete_deferred(self) -> None: url = f"/api/0/organizations/{group.organization.slug}/issues/{group.id}/" - response = self.client.delete(url, format="json") + response = self.client.delete(url, HTTP_AUTHORIZATION=f"Bearer {token.token}") assert response.status_code == 202, response.content # Deletion was deferred, so it should still exist diff --git a/tests/sentry/issues/endpoints/test_group_integration_details.py b/tests/sentry/issues/endpoints/test_group_integration_details.py index 1ab4339c0baa..a249ff0b5ca7 100644 --- a/tests/sentry/issues/endpoints/test_group_integration_details.py +++ b/tests/sentry/issues/endpoints/test_group_integration_details.py @@ -508,33 +508,28 @@ def test_post_feature_disabled(self) -> None: assert response.data["detail"] == "Your organization does not have access to this feature." def test_simple_delete(self) -> None: - self.login_as(user=self.user) - org = self.organization - group = self.create_group() + self.organization.update_option("sentry:events_member_admin", False) + member = self.create_user() + self.create_member( + user=member, organization=self.organization, role="member", teams=[self.team] + ) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) + group = self.group integration = self.create_integration( - organization=org, provider="example", name="Example", external_id="example:1" + organization=self.organization, provider="example", external_id="example:1" ) - - external_issue = ExternalIssue.objects.get_or_create( - organization_id=org.id, integration_id=integration.id, key="APP-123" - )[0] - - group_link = GroupLink.objects.get_or_create( - group_id=group.id, - project_id=group.project_id, - linked_type=GroupLink.LinkedType.issue, - linked_id=external_issue.id, - relationship=GroupLink.Relationship.references, - )[0] - - path = f"/api/0/organizations/{org.slug}/issues/{group.id}/integrations/{integration.id}/?externalIssue={external_issue.id}" + external_issue = self.create_integration_external_issue( + group=group, integration=integration, key="APP-123" + ) + path = f"/api/0/organizations/{self.organization.slug}/issues/{group.id}/integrations/{integration.id}/?externalIssue={external_issue.id}" with self.feature("organizations:integrations-issue-basic"): - response = self.client.delete(path) + response = self.client.delete(path, HTTP_AUTHORIZATION=f"Bearer {token.token}") - assert response.status_code == 204 - assert not ExternalIssue.objects.filter(id=external_issue.id).exists() - assert not GroupLink.objects.filter(id=group_link.id).exists() + assert response.status_code == 204, response.content + assert not ExternalIssue.objects.filter(id=external_issue.id).exists() + assert not GroupLink.objects.get_group_issues(group, external_issue.id).exists() + assert Group.objects.get(id=group.id).status == group.status def test_delete_feature_disabled(self) -> None: self.login_as(user=self.user) diff --git a/tests/sentry/issues/endpoints/test_group_notes_details.py b/tests/sentry/issues/endpoints/test_group_notes_details.py index ac6d688b4618..10e95f719b85 100644 --- a/tests/sentry/issues/endpoints/test_group_notes_details.py +++ b/tests/sentry/issues/endpoints/test_group_notes_details.py @@ -63,18 +63,46 @@ def test_put_invalid_note_id(self) -> None: assert response.status_code == 404 def test_delete(self) -> None: - self.login_as(user=self.user) - - url = self.url + self.organization.update_option("sentry:events_member_admin", False) + member = self.create_user() + self.create_member( + user=member, organization=self.organization, role="member", teams=[self.team] + ) + self.activity.update(user_id=member.id) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) assert Group.objects.get(id=self.group.id).num_comments == 1 - response = self.client.delete(url, format="json") + response = self.client.delete(self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}") assert response.status_code == 204, response.status_code assert not Activity.objects.filter(id=self.activity.id).exists() assert Group.objects.get(id=self.group.id).num_comments == 0 + def test_delete_with_read_only_token(self) -> None: + token = self.create_user_auth_token(user=self.user, scope_list=["event:read"]) + + response = self.client.delete( + self.url, format="json", HTTP_AUTHORIZATION=f"Bearer {token.token}" + ) + + assert response.status_code == 403, response.content + assert Activity.objects.filter(id=self.activity.id).exists() + + def test_delete_another_users_comment_with_write_token(self) -> None: + member = self.create_user() + self.create_member( + user=member, organization=self.organization, role="member", teams=[self.team] + ) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) + + response = self.client.delete( + self.url, format="json", HTTP_AUTHORIZATION=f"Bearer {token.token}" + ) + + assert response.status_code == 404, response.content + assert Activity.objects.filter(id=self.activity.id).exists() + def test_delete_comment_and_subscription(self) -> None: """Test that if a user deletes their comment on an issue, we delete the subscription too""" self.login_as(user=self.user) diff --git a/tests/sentry/sentry_apps/api/endpoints/test_group_external_issue_details.py b/tests/sentry/sentry_apps/api/endpoints/test_group_external_issue_details.py index d7668ef52a83..d4a3188928f0 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_group_external_issue_details.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_group_external_issue_details.py @@ -1,4 +1,5 @@ from sentry.issues.action_log import ActionSource +from sentry.models.group import Group from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase @@ -18,10 +19,19 @@ def setUp(self) -> None: self.url = f"/api/0/organizations/{self.organization.slug}/issues/{self.group.id}/external-issues/{self.external_issue.id}/" def test_deletes_external_issue(self) -> None: - response = self.client.delete(self.url, format="json") + self.organization.update_option("sentry:events_member_admin", False) + member = self.create_user() + self.create_member( + user=member, organization=self.organization, role="member", teams=[self.team] + ) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) + self.client.cookies.clear() + + response = self.client.delete(self.url, HTTP_AUTHORIZATION=f"Bearer {token.token}") assert response.status_code == 204, response.content assert not PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + assert Group.objects.get(id=self.group.id).status == self.group.status def test_deletes_external_issue_records_action_log(self) -> None: with self.assertLogs("sentry.issues.action_log", level="INFO") as logs: @@ -74,3 +84,5 @@ def test_forbids_deleting_an_inaccessible_issue(self) -> None: response = self.client.delete(url, format="json") assert response.status_code == 403, response.content + assert PlatformExternalIssue.objects.filter(id=external_issue.id).exists() + assert Group.objects.get(id=group.id).status == group.status diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py index 3eba1c8aed69..3bd02a1ccf3a 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_details.py @@ -19,7 +19,7 @@ def setUp(self) -> None: name="testin", organization=self.org, webhook_url="https://example.com", - scopes=["event:admin"], + scopes=["event:write"], ) self.install = self.create_sentry_app_installation( organization=self.org, slug=self.sentry_app.slug, user=self.user @@ -33,12 +33,52 @@ def setUp(self) -> None: self.login_as(self.user) def test_deletes_external_issue(self) -> None: + team = self.create_team(organization=self.org) + with assume_test_silo_mode_of(Organization): + self.org.update_option("sentry:events_member_admin", False) + self.project.add_team(team) + member = self.create_user() + self.create_member(organization=self.org, user=member, role="member", teams=[team]) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) + self.client.cookies.clear() + + self.get_success_response( + self.install.uuid, + self.external_issue.id, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=204, + ) with assume_test_silo_mode_of(PlatformExternalIssue): - assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() - self.get_success_response(self.install.uuid, self.external_issue.id, status_code=204) + assert not PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + + def test_deletes_external_issue_with_app_writer_token(self) -> None: + token = self.create_internal_integration_token(install=self.install, user=self.user) + self.client.cookies.clear() + + self.get_success_response( + self.install.uuid, + self.external_issue.id, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=204, + ) with assume_test_silo_mode_of(PlatformExternalIssue): assert not PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + def test_rejects_read_only_app_token(self) -> None: + self.sentry_app.update(scope_list=["event:read"]) + self.install.refresh_from_db() + token = self.create_internal_integration_token(install=self.install, user=self.user) + self.client.cookies.clear() + + self.get_error_response( + self.install.uuid, + self.external_issue.id, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=403, + ) + with assume_test_silo_mode_of(PlatformExternalIssue): + assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + def test_handles_non_existing_external_issue(self) -> None: self.get_error_response(self.install.uuid, 999999, status_code=404) @@ -71,6 +111,7 @@ def test_handles_member_without_project_access(self) -> None: with assume_test_silo_mode_of(Organization): self.org.flags.allow_joinleave = False self.org.save() + self.org.update_option("sentry:events_member_admin", False) member_team = self.create_team(organization=self.org) self.create_project(organization=self.org, teams=[member_team])