From 7b8faac94db79531ed199aa31ab043c1e198609f Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 9 Sep 2026 19:43:10 +0200 Subject: [PATCH 1/2] fix(api): Allow unlinking external issues with event:write Accept write or admin for external issue unlink while preserving project and organization checks. Keep actual issue deletion restricted to event:admin. --- .../sentry-app-external-issue-details.json | 2 +- src/sentry/issues/endpoints/bases/group.py | 8 ++ .../endpoints/group_integration_details.py | 3 +- .../sentry_apps/api/bases/sentryapps.py | 2 +- .../endpoints/group_external_issue_details.py | 3 +- .../issues/endpoints/test_group_details.py | 24 +++++ .../test_group_integration_details.py | 99 ++++++++++++++++- .../test_group_external_issue_details.py | 63 +++++++++++ ...app_installation_external_issue_details.py | 101 +++++++++++++++++- 9 files changed, 298 insertions(+), 7 deletions(-) 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..632aec1523be 100644 --- a/src/sentry/issues/endpoints/bases/group.py +++ b/src/sentry/issues/endpoints/bases/group.py @@ -44,6 +44,14 @@ def has_object_permission(self, request: Request, view: APIView, group: Any) -> return super().has_object_permission(request, view, group.project) +class GroupLinkPermission(GroupPermission): + scope_map = { + **GroupPermission.scope_map, + # Unlinking removes an association, not the Sentry issue or external resource. + "DELETE": ["event:write", "event:admin"], + } + + class GroupEndpoint(Endpoint): owner = ApiOwner.ISSUES permission_classes = (GroupPermission,) diff --git a/src/sentry/issues/endpoints/group_integration_details.py b/src/sentry/issues/endpoints/group_integration_details.py index bf13dbbce800..89c9b42ee02e 100644 --- a/src/sentry/issues/endpoints/group_integration_details.py +++ b/src/sentry/issues/endpoints/group_integration_details.py @@ -53,7 +53,7 @@ LinkExternalIssueAction, UnlinkExternalIssueAction, ) -from sentry.issues.endpoints.bases.group import GroupEndpoint +from sentry.issues.endpoints.bases.group import GroupEndpoint, GroupLinkPermission from sentry.models.activity import Activity from sentry.models.group import Group from sentry.models.grouplink import GroupLink @@ -122,6 +122,7 @@ def serialize( @cell_silo_endpoint class GroupIntegrationDetailsEndpoint(GroupEndpoint): owner = ApiOwner.INTEGRATION_PLATFORM + permission_classes = (GroupLinkPermission,) publish_status = { "GET": ApiPublishStatus.PUBLIC, "POST": 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/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py index 8f478206a837..460d53fd21cb 100644 --- a/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py @@ -14,13 +14,14 @@ resolve_action_source, ) from sentry.issues.action_log.types import UnlinkPlatformExternalIssueAction -from sentry.issues.endpoints.bases.group import GroupEndpoint +from sentry.issues.endpoints.bases.group import GroupEndpoint, GroupLinkPermission from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue @cell_silo_endpoint class GroupExternalIssueDetailsEndpoint(GroupEndpoint): owner = ApiOwner.PROJECT_MANAGEMENT_INTEGRATIONS + permission_classes = (GroupLinkPermission,) publish_status = { "DELETE": ApiPublishStatus.PRIVATE, } diff --git a/tests/sentry/issues/endpoints/test_group_details.py b/tests/sentry/issues/endpoints/test_group_details.py index 010b104ee7e0..f8ce12a468b4 100644 --- a/tests/sentry/issues/endpoints/test_group_details.py +++ b/tests/sentry/issues/endpoints/test_group_details.py @@ -1103,6 +1103,30 @@ def test_ratelimit(self) -> None: class GroupDeleteTest(APITestCase): + 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_with_admin_only_token(self) -> None: + group = self.create_group() + token = self.create_user_auth_token(user=self.user, scope_list=["event:admin"]) + + response = self.client.delete( + f"/api/0/organizations/{self.organization.slug}/issues/{group.id}/", + HTTP_AUTHORIZATION=f"Bearer {token.token}", + ) + + assert response.status_code == 202 + assert Group.objects.get(id=group.id).status == GroupStatus.PENDING_DELETION + def test_delete_deferred(self) -> None: self.login_as(user=self.user) diff --git a/tests/sentry/issues/endpoints/test_group_integration_details.py b/tests/sentry/issues/endpoints/test_group_integration_details.py index 1ab4339c0baa..0d3e497f7a44 100644 --- a/tests/sentry/issues/endpoints/test_group_integration_details.py +++ b/tests/sentry/issues/endpoints/test_group_integration_details.py @@ -8,7 +8,7 @@ from sentry.integrations.models.external_issue import ExternalIssue from sentry.integrations.types import EventLifecycleOutcome from sentry.models.activity import Activity -from sentry.models.group import Group +from sentry.models.group import Group, GroupStatus from sentry.models.grouplink import GroupLink from sentry.models.organization import Organization from sentry.shared_integrations.exceptions import ( @@ -19,6 +19,7 @@ from sentry.testutils.cases import APITestCase from sentry.testutils.factories import EventType from sentry.testutils.helpers.datetime import before_now +from sentry.testutils.helpers.features import with_feature from sentry.testutils.skips import requires_snuba from sentry.types.activity import ActivityType from sentry.users.services.user_option import get_option_from_list, user_option_service @@ -39,6 +40,102 @@ def raise_integration_installation_configuration_error(*args: Any, **kwargs: Any raise IntegrationConfigurationError("Repository has no issue tracker.") +@with_feature("organizations:integrations-issue-basic") +class GroupIntegrationDetailsDeleteTest(APITestCase): + method = "delete" + + def setUp(self) -> None: + super().setUp() + self.group = self.create_group(project=self.project) + self.integration = self.create_integration( + organization=self.organization, provider="example", external_id="example:1" + ) + self.external_issue = self.create_integration_external_issue( + group=self.group, integration=self.integration, key="APP-123" + ) + + def reverse_url(self) -> str: + return f"/api/0/organizations/{self.organization.slug}/issues/{self.group.id}/integrations/{self.integration.id}/" + + def test_delete_with_write_only_token(self) -> None: + self.assert_unlink_allowed("event:write") + + def test_delete_with_admin_only_token(self) -> None: + self.assert_unlink_allowed("event:admin") + + def assert_unlink_allowed(self, scope: str) -> None: + token = self.create_user_auth_token(user=self.user, scope_list=[scope]) + + self.get_success_response( + qs_params={"externalIssue": self.external_issue.id}, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=204, + ) + + assert not GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() + assert not ExternalIssue.objects.filter(id=self.external_issue.id).exists() + assert Group.objects.get(id=self.group.id).status == GroupStatus.UNRESOLVED + + def test_delete_with_read_only_token(self) -> None: + token = self.create_user_auth_token(user=self.user, scope_list=["event:read"]) + + self.get_error_response( + qs_params={"externalIssue": self.external_issue.id}, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=403, + ) + + assert GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() + assert ExternalIssue.objects.filter(id=self.external_issue.id).exists() + + def test_delete_as_project_member_without_admin(self) -> None: + 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.get_success_response( + qs_params={"externalIssue": self.external_issue.id}, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=204, + ) + + assert not GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() + assert Group.objects.get(id=self.group.id).status == GroupStatus.UNRESOLVED + + def test_delete_without_project_access(self) -> None: + self.organization.flags.allow_joinleave = False + self.organization.save() + member = self.create_user() + self.create_member(user=member, organization=self.organization, role="member", teams=[]) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) + + self.get_error_response( + qs_params={"externalIssue": self.external_issue.id}, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=403, + ) + + assert GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() + assert ExternalIssue.objects.filter(id=self.external_issue.id).exists() + + def test_delete_without_organization_access(self) -> None: + other_user = self.create_user() + self.create_organization(owner=other_user) + token = self.create_user_auth_token(user=other_user, scope_list=["event:write"]) + + self.get_error_response( + qs_params={"externalIssue": self.external_issue.id}, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=403, + ) + + assert GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() + assert ExternalIssue.objects.filter(id=self.external_issue.id).exists() + + class GroupIntegrationDetailsTest(APITestCase): def setUp(self) -> None: super().setUp() 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..a9deedef465d 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 @@ -22,6 +23,66 @@ def test_deletes_external_issue(self) -> None: 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 _assert_unlink_with_token_scope( + self, scope: str, expected_status: int, link_exists: bool + ) -> None: + user = self.create_user() + self.create_member( + user=user, organization=self.organization, role="member", teams=[self.team] + ) + token = self.create_user_auth_token(user=user, scope_list=[scope]) + self.client.cookies.clear() + + response = self.client.delete( + self.url, format="json", HTTP_AUTHORIZATION=f"Bearer {token.token}" + ) + + assert response.status_code == expected_status, response.content + assert ( + PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() is link_exists + ) + assert Group.objects.get(id=self.group.id).status == self.group.status + + def test_token_with_event_write_can_unlink(self) -> None: + self._assert_unlink_with_token_scope("event:write", 204, False) + + def test_token_with_event_admin_can_unlink(self) -> None: + self._assert_unlink_with_token_scope("event:admin", 204, False) + + def test_token_with_event_read_cannot_unlink(self) -> None: + self._assert_unlink_with_token_scope("event:read", 403, True) + + def test_member_without_event_admin_can_unlink(self) -> None: + self.organization.update_option("sentry:events_member_admin", False) + user = self.create_user() + self.create_member( + user=user, organization=self.organization, role="member", teams=[self.team] + ) + self.login_as(user=user) + + response = self.client.delete(self.url, format="json") + + 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_token_cannot_unlink_without_project_access(self) -> None: + self.organization.flags.allow_joinleave = False + self.organization.save() + user = self.create_user() + self.create_member(user=user, organization=self.organization, role="member", teams=[]) + token = self.create_user_auth_token(user=user, scope_list=["event:write"]) + self.client.cookies.clear() + + response = self.client.delete( + self.url, format="json", HTTP_AUTHORIZATION=f"Bearer {token.token}" + ) + + assert response.status_code == 403, response.content + assert 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 +135,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..36df8ced03a1 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 @@ -1,3 +1,4 @@ +from sentry.models.apitoken import ApiToken from sentry.models.organization import Organization from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase @@ -39,6 +40,92 @@ def test_deletes_external_issue(self) -> None: with assume_test_silo_mode_of(PlatformExternalIssue): assert not PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + def _assert_token_unlink( + self, token: ApiToken, expected_status: int, link_exists: bool + ) -> None: + self.client.cookies.clear() + response = self.get_response( + self.install.uuid, + self.external_issue.id, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + ) + + assert response.status_code == expected_status, response.content + with assume_test_silo_mode_of(PlatformExternalIssue): + assert ( + PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + is link_exists + ) + + def test_deletes_external_issue_with_writer_token(self) -> None: + token = self.create_user_auth_token(user=self.user, scope_list=["event:write"]) + self._assert_token_unlink(token, 204, False) + + def test_deletes_external_issue_with_admin_token(self) -> None: + token = self.create_user_auth_token(user=self.user, scope_list=["event:admin"]) + self._assert_token_unlink(token, 204, False) + + def test_rejects_read_only_user_token(self) -> None: + token = self.create_user_auth_token(user=self.user, scope_list=["event:read"]) + self._assert_token_unlink(token, 403, True) + + def _create_app_token(self, scope: str) -> ApiToken: + self.sentry_app.update(scope_list=[scope]) + self.install.refresh_from_db() + token = self.create_internal_integration_token(install=self.install, user=self.user) + assert token.user_id == self.sentry_app.proxy_user_id + return token + + def test_deletes_external_issue_with_app_writer_token(self) -> None: + token = self._create_app_token("event:write") + assert token.scope_list == ["event:read", "event:write"] + self._assert_token_unlink(token, 204, False) + + def test_deletes_external_issue_with_app_admin_token(self) -> None: + token = self._create_app_token("event:admin") + assert token.scope_list == ["event:admin", "event:read", "event:write"] + self._assert_token_unlink(token, 204, False) + + def test_rejects_read_only_app_token(self) -> None: + token = self._create_app_token("event:read") + assert token.scope_list == ["event:read"] + self._assert_token_unlink(token, 403, True) + + def test_rejects_token_from_different_app(self) -> None: + other_app = self.create_sentry_app( + name="other-app", organization=self.org, scopes=["event:write"] + ) + other_install = self.create_sentry_app_installation( + organization=self.org, slug=other_app.slug, user=self.user + ) + token = self.create_internal_integration_token(install=other_install, user=self.user) + self._assert_token_unlink(token, 403, True) + + def test_rejects_issue_from_different_app(self) -> None: + other_app = self.create_sentry_app( + name="other-app", organization=self.org, scopes=["event:write"] + ) + other_install = self.create_sentry_app_installation( + organization=self.org, slug=other_app.slug, user=self.user + ) + + self.get_error_response(other_install.uuid, self.external_issue.id, status_code=404) + + with assume_test_silo_mode_of(PlatformExternalIssue): + assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() + + def test_member_with_write_scope_can_unlink(self) -> None: + team = self.create_team(organization=self.org) + 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) + 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._assert_token_unlink(token, 204, False) + def test_handles_non_existing_external_issue(self) -> None: self.get_error_response(self.install.uuid, 999999, status_code=404) @@ -71,6 +158,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]) @@ -78,9 +166,18 @@ def test_handles_member_without_project_access(self) -> None: self.create_member( organization=self.org, user=restricted_member, role="member", teams=[member_team] ) - self.login_as(restricted_member) + token = self.create_user_auth_token(user=restricted_member, scope_list=["event:write"]) + self.client.cookies.clear() - self.get_error_response(self.install.uuid, self.external_issue.id, status_code=403) + response = self.get_error_response( + self.install.uuid, + self.external_issue.id, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=403, + ) + assert ( + response.data["detail"] == "You do not have permission to delete this external issue." + ) with assume_test_silo_mode_of(PlatformExternalIssue): assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() From 3738c4346b23d30349fbff61782132d1adaabc55 Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 10 Sep 2026 15:42:31 +0200 Subject: [PATCH 2/2] fix(api): Allow write access for links and own comments Use event:write in GroupPermission while keeping issue deletion gated by event:admin. Preserve eventsMemberAdmin and fold scope regressions into existing endpoint tests. --- src/sentry/issues/endpoints/bases/group.py | 10 +- src/sentry/issues/endpoints/group_details.py | 11 +- .../endpoints/group_integration_details.py | 3 +- .../endpoints/group_external_issue_details.py | 3 +- .../issues/endpoints/test_group_details.py | 35 +++-- .../test_group_integration_details.py | 138 +++--------------- .../endpoints/test_group_notes_details.py | 36 ++++- .../test_group_external_issue_details.py | 61 +------- ...app_installation_external_issue_details.py | 120 ++++----------- 9 files changed, 122 insertions(+), 295 deletions(-) diff --git a/src/sentry/issues/endpoints/bases/group.py b/src/sentry/issues/endpoints/bases/group.py index 632aec1523be..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: @@ -44,14 +44,6 @@ def has_object_permission(self, request: Request, view: APIView, group: Any) -> return super().has_object_permission(request, view, group.project) -class GroupLinkPermission(GroupPermission): - scope_map = { - **GroupPermission.scope_map, - # Unlinking removes an association, not the Sentry issue or external resource. - "DELETE": ["event:write", "event:admin"], - } - - class GroupEndpoint(Endpoint): owner = ApiOwner.ISSUES permission_classes = (GroupPermission,) 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/issues/endpoints/group_integration_details.py b/src/sentry/issues/endpoints/group_integration_details.py index 89c9b42ee02e..bf13dbbce800 100644 --- a/src/sentry/issues/endpoints/group_integration_details.py +++ b/src/sentry/issues/endpoints/group_integration_details.py @@ -53,7 +53,7 @@ LinkExternalIssueAction, UnlinkExternalIssueAction, ) -from sentry.issues.endpoints.bases.group import GroupEndpoint, GroupLinkPermission +from sentry.issues.endpoints.bases.group import GroupEndpoint from sentry.models.activity import Activity from sentry.models.group import Group from sentry.models.grouplink import GroupLink @@ -122,7 +122,6 @@ def serialize( @cell_silo_endpoint class GroupIntegrationDetailsEndpoint(GroupEndpoint): owner = ApiOwner.INTEGRATION_PLATFORM - permission_classes = (GroupLinkPermission,) publish_status = { "GET": ApiPublishStatus.PUBLIC, "POST": ApiPublishStatus.PUBLIC, diff --git a/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py index 460d53fd21cb..8f478206a837 100644 --- a/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/group_external_issue_details.py @@ -14,14 +14,13 @@ resolve_action_source, ) from sentry.issues.action_log.types import UnlinkPlatformExternalIssueAction -from sentry.issues.endpoints.bases.group import GroupEndpoint, GroupLinkPermission +from sentry.issues.endpoints.bases.group import GroupEndpoint from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue @cell_silo_endpoint class GroupExternalIssueDetailsEndpoint(GroupEndpoint): owner = ApiOwner.PROJECT_MANAGEMENT_INTEGRATIONS - permission_classes = (GroupLinkPermission,) publish_status = { "DELETE": ApiPublishStatus.PRIVATE, } diff --git a/tests/sentry/issues/endpoints/test_group_details.py b/tests/sentry/issues/endpoints/test_group_details.py index f8ce12a468b4..9ec1ca895202 100644 --- a/tests/sentry/issues/endpoints/test_group_details.py +++ b/tests/sentry/issues/endpoints/test_group_details.py @@ -1103,32 +1103,41 @@ def test_ratelimit(self) -> None: class GroupDeleteTest(APITestCase): - def test_delete_with_write_only_token(self) -> None: + def test_delete_as_member_respects_organization_setting(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}", + 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}/" - assert response.status_code == 403 + 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 - def test_delete_with_admin_only_token(self) -> None: + 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:admin"]) + 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 == 202 - assert Group.objects.get(id=group.id).status == GroupStatus.PENDING_DELETION + 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 @@ -1136,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 0d3e497f7a44..a249ff0b5ca7 100644 --- a/tests/sentry/issues/endpoints/test_group_integration_details.py +++ b/tests/sentry/issues/endpoints/test_group_integration_details.py @@ -8,7 +8,7 @@ from sentry.integrations.models.external_issue import ExternalIssue from sentry.integrations.types import EventLifecycleOutcome from sentry.models.activity import Activity -from sentry.models.group import Group, GroupStatus +from sentry.models.group import Group from sentry.models.grouplink import GroupLink from sentry.models.organization import Organization from sentry.shared_integrations.exceptions import ( @@ -19,7 +19,6 @@ from sentry.testutils.cases import APITestCase from sentry.testutils.factories import EventType from sentry.testutils.helpers.datetime import before_now -from sentry.testutils.helpers.features import with_feature from sentry.testutils.skips import requires_snuba from sentry.types.activity import ActivityType from sentry.users.services.user_option import get_option_from_list, user_option_service @@ -40,102 +39,6 @@ def raise_integration_installation_configuration_error(*args: Any, **kwargs: Any raise IntegrationConfigurationError("Repository has no issue tracker.") -@with_feature("organizations:integrations-issue-basic") -class GroupIntegrationDetailsDeleteTest(APITestCase): - method = "delete" - - def setUp(self) -> None: - super().setUp() - self.group = self.create_group(project=self.project) - self.integration = self.create_integration( - organization=self.organization, provider="example", external_id="example:1" - ) - self.external_issue = self.create_integration_external_issue( - group=self.group, integration=self.integration, key="APP-123" - ) - - def reverse_url(self) -> str: - return f"/api/0/organizations/{self.organization.slug}/issues/{self.group.id}/integrations/{self.integration.id}/" - - def test_delete_with_write_only_token(self) -> None: - self.assert_unlink_allowed("event:write") - - def test_delete_with_admin_only_token(self) -> None: - self.assert_unlink_allowed("event:admin") - - def assert_unlink_allowed(self, scope: str) -> None: - token = self.create_user_auth_token(user=self.user, scope_list=[scope]) - - self.get_success_response( - qs_params={"externalIssue": self.external_issue.id}, - extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, - status_code=204, - ) - - assert not GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() - assert not ExternalIssue.objects.filter(id=self.external_issue.id).exists() - assert Group.objects.get(id=self.group.id).status == GroupStatus.UNRESOLVED - - def test_delete_with_read_only_token(self) -> None: - token = self.create_user_auth_token(user=self.user, scope_list=["event:read"]) - - self.get_error_response( - qs_params={"externalIssue": self.external_issue.id}, - extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, - status_code=403, - ) - - assert GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() - assert ExternalIssue.objects.filter(id=self.external_issue.id).exists() - - def test_delete_as_project_member_without_admin(self) -> None: - 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.get_success_response( - qs_params={"externalIssue": self.external_issue.id}, - extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, - status_code=204, - ) - - assert not GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() - assert Group.objects.get(id=self.group.id).status == GroupStatus.UNRESOLVED - - def test_delete_without_project_access(self) -> None: - self.organization.flags.allow_joinleave = False - self.organization.save() - member = self.create_user() - self.create_member(user=member, organization=self.organization, role="member", teams=[]) - token = self.create_user_auth_token(user=member, scope_list=["event:write"]) - - self.get_error_response( - qs_params={"externalIssue": self.external_issue.id}, - extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, - status_code=403, - ) - - assert GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() - assert ExternalIssue.objects.filter(id=self.external_issue.id).exists() - - def test_delete_without_organization_access(self) -> None: - other_user = self.create_user() - self.create_organization(owner=other_user) - token = self.create_user_auth_token(user=other_user, scope_list=["event:write"]) - - self.get_error_response( - qs_params={"externalIssue": self.external_issue.id}, - extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, - status_code=403, - ) - - assert GroupLink.objects.get_group_issues(self.group, self.external_issue.id).exists() - assert ExternalIssue.objects.filter(id=self.external_issue.id).exists() - - class GroupIntegrationDetailsTest(APITestCase): def setUp(self) -> None: super().setUp() @@ -605,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 a9deedef465d..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 @@ -19,71 +19,20 @@ 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") - - 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 _assert_unlink_with_token_scope( - self, scope: str, expected_status: int, link_exists: bool - ) -> None: - user = self.create_user() - self.create_member( - user=user, organization=self.organization, role="member", teams=[self.team] - ) - token = self.create_user_auth_token(user=user, scope_list=[scope]) - self.client.cookies.clear() - - response = self.client.delete( - self.url, format="json", HTTP_AUTHORIZATION=f"Bearer {token.token}" - ) - - assert response.status_code == expected_status, response.content - assert ( - PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() is link_exists - ) - assert Group.objects.get(id=self.group.id).status == self.group.status - - def test_token_with_event_write_can_unlink(self) -> None: - self._assert_unlink_with_token_scope("event:write", 204, False) - - def test_token_with_event_admin_can_unlink(self) -> None: - self._assert_unlink_with_token_scope("event:admin", 204, False) - - def test_token_with_event_read_cannot_unlink(self) -> None: - self._assert_unlink_with_token_scope("event:read", 403, True) - - def test_member_without_event_admin_can_unlink(self) -> None: self.organization.update_option("sentry:events_member_admin", False) - user = self.create_user() + member = self.create_user() self.create_member( - user=user, organization=self.organization, role="member", teams=[self.team] + user=member, organization=self.organization, role="member", teams=[self.team] ) - self.login_as(user=user) + token = self.create_user_auth_token(user=member, scope_list=["event:write"]) + self.client.cookies.clear() - response = self.client.delete(self.url, format="json") + 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_token_cannot_unlink_without_project_access(self) -> None: - self.organization.flags.allow_joinleave = False - self.organization.save() - user = self.create_user() - self.create_member(user=user, organization=self.organization, role="member", teams=[]) - token = self.create_user_auth_token(user=user, scope_list=["event:write"]) - self.client.cookies.clear() - - response = self.client.delete( - self.url, format="json", HTTP_AUTHORIZATION=f"Bearer {token.token}" - ) - - assert response.status_code == 403, response.content - assert 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: response = self.client.delete(self.url, format="json") 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 36df8ced03a1..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 @@ -1,4 +1,3 @@ -from sentry.models.apitoken import ApiToken from sentry.models.organization import Organization from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.testutils.cases import APITestCase @@ -20,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 @@ -34,98 +33,52 @@ def setUp(self) -> None: self.login_as(self.user) def test_deletes_external_issue(self) -> None: - 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) + 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 not PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() - def _assert_token_unlink( - self, token: ApiToken, expected_status: int, link_exists: bool - ) -> None: + 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() - response = self.get_response( + + self.get_success_response( self.install.uuid, self.external_issue.id, extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=204, ) - - assert response.status_code == expected_status, response.content with assume_test_silo_mode_of(PlatformExternalIssue): - assert ( - PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() - is link_exists - ) - - def test_deletes_external_issue_with_writer_token(self) -> None: - token = self.create_user_auth_token(user=self.user, scope_list=["event:write"]) - self._assert_token_unlink(token, 204, False) - - def test_deletes_external_issue_with_admin_token(self) -> None: - token = self.create_user_auth_token(user=self.user, scope_list=["event:admin"]) - self._assert_token_unlink(token, 204, False) - - def test_rejects_read_only_user_token(self) -> None: - token = self.create_user_auth_token(user=self.user, scope_list=["event:read"]) - self._assert_token_unlink(token, 403, True) + assert not PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() - def _create_app_token(self, scope: str) -> ApiToken: - self.sentry_app.update(scope_list=[scope]) + 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) - assert token.user_id == self.sentry_app.proxy_user_id - return token - - def test_deletes_external_issue_with_app_writer_token(self) -> None: - token = self._create_app_token("event:write") - assert token.scope_list == ["event:read", "event:write"] - self._assert_token_unlink(token, 204, False) - - def test_deletes_external_issue_with_app_admin_token(self) -> None: - token = self._create_app_token("event:admin") - assert token.scope_list == ["event:admin", "event:read", "event:write"] - self._assert_token_unlink(token, 204, False) - - def test_rejects_read_only_app_token(self) -> None: - token = self._create_app_token("event:read") - assert token.scope_list == ["event:read"] - self._assert_token_unlink(token, 403, True) - - def test_rejects_token_from_different_app(self) -> None: - other_app = self.create_sentry_app( - name="other-app", organization=self.org, scopes=["event:write"] - ) - other_install = self.create_sentry_app_installation( - organization=self.org, slug=other_app.slug, user=self.user - ) - token = self.create_internal_integration_token(install=other_install, user=self.user) - self._assert_token_unlink(token, 403, True) + self.client.cookies.clear() - def test_rejects_issue_from_different_app(self) -> None: - other_app = self.create_sentry_app( - name="other-app", organization=self.org, scopes=["event:write"] - ) - other_install = self.create_sentry_app_installation( - organization=self.org, slug=other_app.slug, user=self.user + self.get_error_response( + self.install.uuid, + self.external_issue.id, + extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, + status_code=403, ) - - self.get_error_response(other_install.uuid, self.external_issue.id, status_code=404) - with assume_test_silo_mode_of(PlatformExternalIssue): assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists() - def test_member_with_write_scope_can_unlink(self) -> None: - team = self.create_team(organization=self.org) - 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) - 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._assert_token_unlink(token, 204, False) - def test_handles_non_existing_external_issue(self) -> None: self.get_error_response(self.install.uuid, 999999, status_code=404) @@ -166,18 +119,9 @@ def test_handles_member_without_project_access(self) -> None: self.create_member( organization=self.org, user=restricted_member, role="member", teams=[member_team] ) - token = self.create_user_auth_token(user=restricted_member, scope_list=["event:write"]) - self.client.cookies.clear() + self.login_as(restricted_member) - response = self.get_error_response( - self.install.uuid, - self.external_issue.id, - extra_headers={"HTTP_AUTHORIZATION": f"Bearer {token.token}"}, - status_code=403, - ) - assert ( - response.data["detail"] == "You do not have permission to delete this external issue." - ) + self.get_error_response(self.install.uuid, self.external_issue.id, status_code=403) with assume_test_silo_mode_of(PlatformExternalIssue): assert PlatformExternalIssue.objects.filter(id=self.external_issue.id).exists()