From 6ae05ef0d5336ab8a9926f10754647e13d5e758b Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:48:42 +0000 Subject: [PATCH 1/5] feat(service-accounts): Add an endpoint to revoke service account tokens DELETE /organizations/{org}/service-accounts/{id}/tokens/{token_id}/ marks the token deactivated. It stops working once the change replicates to the org's cell. Each revoke writes a SERVICE_ACCOUNT_TOKEN_REMOVE audit log entry. --- src/sentry/api/urls.py | 6 + src/sentry/audit_log/register.py | 8 ++ .../organization_service_account_tokens.py | 65 ++++++++++- .../utils/api/knownSentryApiUrls.generated.ts | 1 + ...est_organization_service_account_tokens.py | 103 ++++++++++++++++++ 5 files changed, 181 insertions(+), 2 deletions(-) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index dddd61982e0c..33944ffee047 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -706,6 +706,7 @@ SentryInternalAppTokensEndpoint, ) from sentry.service_accounts.endpoints.organization_service_account_tokens import ( + OrganizationServiceAccountTokenDetailsEndpoint, OrganizationServiceAccountTokenIndexEndpoint, ) from sentry.service_accounts.endpoints.organization_service_account_viewer_context import ( @@ -1848,6 +1849,11 @@ def create_group_urls( OrganizationServiceAccountTokenIndexEndpoint.as_view(), name="sentry-api-0-organization-service-account-tokens", ), + re_path( + r"^(?P[^/]+)/service-accounts/(?P\d+)/tokens/(?P\d+)/$", + OrganizationServiceAccountTokenDetailsEndpoint.as_view(), + name="sentry-api-0-organization-service-account-token-details", + ), re_path( r"^(?P[^/]+)/service-accounts/(?P\d+)/viewer-context/$", OrganizationServiceAccountViewerContextEndpoint.as_view(), diff --git a/src/sentry/audit_log/register.py b/src/sentry/audit_log/register.py index e9d6c26f77ef..16f4ece5a6aa 100644 --- a/src/sentry/audit_log/register.py +++ b/src/sentry/audit_log/register.py @@ -800,3 +800,11 @@ template="added token {name} to service account {service_account_name}", ) ) +default_manager.add( + AuditLogEvent( + event_id=227, + name="SERVICE_ACCOUNT_TOKEN_REMOVE", + api_name="service-account-token.remove", + template="revoked token {name} of service account {service_account_name}", + ) +) diff --git a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py index ebdc434d7828..384936fa95bc 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -13,7 +13,7 @@ from django.db import router, transaction from django.utils import timezone -from drf_spectacular.utils import extend_schema +from drf_spectacular.utils import OpenApiParameter, extend_schema from rest_framework import serializers from rest_framework.permissions import SAFE_METHODS, BasePermission from rest_framework.request import Request @@ -22,13 +22,15 @@ from sentry import audit_log from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint +from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.paginator import OffsetPaginator from sentry.api.permissions import DisallowImpersonatedTokenCreation from sentry.api.serializers import Serializer, serialize -from sentry.api.utils import generate_locality_url +from sentry.api.utils import generate_locality_url, to_valid_int_id from sentry.apidocs.constants import ( RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, + RESPONSE_NO_CONTENT, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED, ) @@ -60,6 +62,14 @@ DEFAULT_TOKEN_LIFETIME = timedelta(days=30) +TOKEN_ID = OpenApiParameter( + name="token_id", + description="The ID of the service account token.", + required=True, + type=str, + location="path", +) + class ServiceAccountTokenResponse(TypedDict): id: str @@ -278,3 +288,54 @@ def post( ) response["token"] = token_str return Response(response, status=201) + + +@control_silo_endpoint +class OrganizationServiceAccountTokenDetailsEndpoint(ServiceAccountTokenEndpointBase): + publish_status = { + "DELETE": ApiPublishStatus.PRIVATE, + } + + @extend_schema( + operation_id="Revoke a Service Account Token", + parameters=[GlobalParams.ORG_ID_OR_SLUG, SERVICE_ACCOUNT_ID, TOKEN_ID], + responses={ + 204: RESPONSE_NO_CONTENT, + 401: RESPONSE_UNAUTHORIZED, + 403: RESPONSE_FORBIDDEN, + 404: RESPONSE_NOT_FOUND, + }, + ) + def delete( + self, + request: Request, + organization_context: RpcUserOrganizationContext, + organization: RpcOrganization, + service_account: ServiceAccount, + token_id: str, + ) -> Response: + """Revoke a token. It stops working once the revocation reaches the org's cell.""" + with outbox_context(transaction.atomic(using=router.db_for_write(ServiceAccountToken))): + token = ( + ServiceAccountToken.objects.select_for_update() + .filter( + id=to_valid_int_id("token_id", token_id, raise_404=True), + service_account=service_account, + organization_id=organization.id, + date_deactivated__isnull=True, + ) + .first() + ) + if token is None: + raise ResourceDoesNotExist + token.date_deactivated = timezone.now() + token.save() + + self.create_audit_entry( + request, + organization=organization, + target_object=token.id, + event=audit_log.get_event_id("SERVICE_ACCOUNT_TOKEN_REMOVE"), + data=_audit_log_data(service_account, token), + ) + return Response(status=204) diff --git a/static/app/utils/api/knownSentryApiUrls.generated.ts b/static/app/utils/api/knownSentryApiUrls.generated.ts index ef412f36ff16..a15a664aa213 100644 --- a/static/app/utils/api/knownSentryApiUrls.generated.ts +++ b/static/app/utils/api/knownSentryApiUrls.generated.ts @@ -404,6 +404,7 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/service-accounts/' | '/organizations/$organizationIdOrSlug/service-accounts/$serviceAccountId/' | '/organizations/$organizationIdOrSlug/service-accounts/$serviceAccountId/tokens/' + | '/organizations/$organizationIdOrSlug/service-accounts/$serviceAccountId/tokens/$tokenId/' | '/organizations/$organizationIdOrSlug/service-accounts/$serviceAccountId/viewer-context/' | '/organizations/$organizationIdOrSlug/sessions/' | '/organizations/$organizationIdOrSlug/shared/groups/$shareId/' diff --git a/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py b/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py index e02d5d3273e8..d8cfd852b034 100644 --- a/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py +++ b/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py @@ -30,6 +30,7 @@ from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, TestCase from sentry.testutils.helpers.datetime import freeze_time +from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, control_silo_test FEATURE_FLAG = "organizations:service-accounts" @@ -407,6 +408,108 @@ def test_deleted_account_returns_404(self) -> None: ) +@control_silo_test +class OrganizationServiceAccountTokenDetailsTest(ServiceAccountTokenTestCase): + endpoint = "sentry-api-0-organization-service-account-token-details" + method = "delete" + + def setUp(self) -> None: + super().setUp() + self.token = self.create_token() + + def test_revoke(self) -> None: + with ( + self.feature(FEATURE_FLAG), + outbox_runner(), + mock.patch.object(ServiceAccountToken, "handle_async_replication") as replicate, + ): + self.get_success_response( + self.organization.slug, self.service_account.id, self.token.id, status_code=204 + ) + + self.token.refresh_from_db() + assert self.token.date_deactivated is not None + # Revoking keeps the row, and its outbox carries the revocation to the cell. + replicate.assert_called_once() + + entry = AuditLogEntry.objects.get( + event=audit_log.get_event_id("SERVICE_ACCOUNT_TOKEN_REMOVE") + ) + assert entry.actor_id == self.user.id + assert entry.target_object == self.token.id + assert entry.data == { + "name": "deploys", + "scopes": ["project:read"], + "service_account_id": self.service_account.id, + "service_account_name": "ci", + } + + def test_revoked_token_returns_404(self) -> None: + with self.feature(FEATURE_FLAG): + self.get_success_response( + self.organization.slug, self.service_account.id, self.token.id, status_code=204 + ) + self.get_error_response( + self.organization.slug, self.service_account.id, self.token.id, status_code=404 + ) + + assert ( + AuditLogEntry.objects.filter( + event=audit_log.get_event_id("SERVICE_ACCOUNT_TOKEN_REMOVE") + ).count() + == 1 + ) + + def test_token_of_other_account_returns_404(self) -> None: + other_account = self.create_service_account(organization=self.organization, name="other") + + with self.feature(FEATURE_FLAG): + self.get_error_response( + self.organization.slug, other_account.id, self.token.id, status_code=404 + ) + + self.token.refresh_from_db() + assert self.token.date_deactivated is None + + def test_token_of_other_org_returns_404(self) -> None: + with assume_test_silo_mode(SiloMode.CELL): + other_organization = self.create_organization(owner=self.user) + other_account = self.create_service_account(organization=other_organization, name="ci") + other_token = self.create_token(service_account=other_account) + + with self.feature(FEATURE_FLAG): + # The URL's org owns neither the account nor the token. + self.get_error_response( + self.organization.slug, other_account.id, other_token.id, status_code=404 + ) + # The URL's account is in the org, but the token isn't. + self.get_error_response( + self.organization.slug, self.service_account.id, other_token.id, status_code=404 + ) + + other_token.refresh_from_db() + assert other_token.date_deactivated is None + + def test_revoke_requires_service_account_write(self) -> None: + self.login_as_role("member") + + with self.feature(FEATURE_FLAG): + self.get_error_response( + self.organization.slug, self.service_account.id, self.token.id, status_code=403 + ) + + self.token.refresh_from_db() + assert self.token.date_deactivated is None + + def test_manager_can_revoke(self) -> None: + self.login_as_role("manager") + + with self.feature(FEATURE_FLAG): + self.get_success_response( + self.organization.slug, self.service_account.id, self.token.id, status_code=204 + ) + + @control_silo_test class DisallowAgentTokenWritesTest(TestCase): def check(self, method: str, kind: str) -> bool: From c2f1f0de784d6b97ceaf1e1abf40e150cceaf991 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:00:53 +0000 Subject: [PATCH 2/5] fix(service-accounts): Add the token details URL to the control silo patterns --- static/app/data/controlsiloUrlPatterns.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/data/controlsiloUrlPatterns.ts b/static/app/data/controlsiloUrlPatterns.ts index 5c68325fc333..633579350154 100644 --- a/static/app/data/controlsiloUrlPatterns.ts +++ b/static/app/data/controlsiloUrlPatterns.ts @@ -96,6 +96,7 @@ export const controlsiloUrlPatterns: RegExp[] = [ new RegExp('^api/0/organizations/[^/]+/service-accounts/$'), new RegExp('^api/0/organizations/[^/]+/service-accounts/[^/]+/$'), new RegExp('^api/0/organizations/[^/]+/service-accounts/[^/]+/tokens/$'), + new RegExp('^api/0/organizations/[^/]+/service-accounts/[^/]+/tokens/[^/]+/$'), new RegExp('^api/0/organizations/[^/]+/monitoring-providers/$'), new RegExp('^api/0/organizations/[^/]+/monitoring-providers/[^/]+/$'), new RegExp('^api/0/organizations/[^/]+/integrations/$'), From af4f9f5cf70df920635203723259fdec8f1029e7 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 14:38:49 +0000 Subject: [PATCH 3/5] test(service-accounts): Check the token replica after a revoke #130265 replicates service account tokens through their outbox, so the revoke test can check the cell replica's date_deactivated instead of asserting that the replication handler ran. Co-Authored-By: Shaun Kaasten <900809+skaasten@users.noreply.github.com> --- .../test_organization_service_account_tokens.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py b/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py index d8cfd852b034..1478072fc149 100644 --- a/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py +++ b/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py @@ -418,19 +418,16 @@ def setUp(self) -> None: self.token = self.create_token() def test_revoke(self) -> None: - with ( - self.feature(FEATURE_FLAG), - outbox_runner(), - mock.patch.object(ServiceAccountToken, "handle_async_replication") as replicate, - ): + with self.feature(FEATURE_FLAG), outbox_runner(): self.get_success_response( self.organization.slug, self.service_account.id, self.token.id, status_code=204 ) self.token.refresh_from_db() assert self.token.date_deactivated is not None - # Revoking keeps the row, and its outbox carries the revocation to the cell. - replicate.assert_called_once() + with assume_test_silo_mode(SiloMode.CELL): + replica = ServiceAccountTokenReplica.objects.get(token_id=self.token.id) + assert replica.date_deactivated == self.token.date_deactivated entry = AuditLogEntry.objects.get( event=audit_log.get_event_id("SERVICE_ACCOUNT_TOKEN_REMOVE") From 0c647b84d2dd456c45b0657e0c73ab869b14dcae Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:34:52 +0000 Subject: [PATCH 4/5] ref(service-accounts): Don't flush the outbox when revoking a token The revocation is committed before replication, so a failed flush shouldn't turn a successful request into a 500; the outbox drains later. Also use get() instead of first() to avoid an ORDER BY on the lookup. --- .../organization_service_account_tokens.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py index 384936fa95bc..99c0244306f5 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -315,18 +315,19 @@ def delete( token_id: str, ) -> Response: """Revoke a token. It stops working once the revocation reaches the org's cell.""" - with outbox_context(transaction.atomic(using=router.db_for_write(ServiceAccountToken))): - token = ( - ServiceAccountToken.objects.select_for_update() - .filter( + # The revocation commits before replication, so a failed flush shouldn't fail the + # request; the outbox is drained later. + with outbox_context( + transaction.atomic(using=router.db_for_write(ServiceAccountToken)), flush=False + ): + try: + token = ServiceAccountToken.objects.select_for_update().get( id=to_valid_int_id("token_id", token_id, raise_404=True), service_account=service_account, organization_id=organization.id, date_deactivated__isnull=True, ) - .first() - ) - if token is None: + except ServiceAccountToken.DoesNotExist: raise ResourceDoesNotExist token.date_deactivated = timezone.now() token.save() From 5871e01b3119a68da29306ad8bdb0ad7adabf052 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:53:13 +0000 Subject: [PATCH 5/5] fix(service-accounts): Catch outbox flush errors when revoking a token Revocation should still flush to the cell right away, but a replication failure shouldn't fail a revocation that's already committed. Catch OutboxDatabaseError and OutboxFlushError after the save, like ApiTokensEndpoint.post. The revoke test no longer wraps the request in outbox_runner, so it checks the flush. --- .../organization_service_account_tokens.py | 42 ++++++++++++------- ...est_organization_service_account_tokens.py | 18 +++++++- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py index 99c0244306f5..709ba56e4d40 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -315,22 +315,32 @@ def delete( token_id: str, ) -> Response: """Revoke a token. It stops working once the revocation reaches the org's cell.""" - # The revocation commits before replication, so a failed flush shouldn't fail the - # request; the outbox is drained later. - with outbox_context( - transaction.atomic(using=router.db_for_write(ServiceAccountToken)), flush=False - ): - try: - token = ServiceAccountToken.objects.select_for_update().get( - id=to_valid_int_id("token_id", token_id, raise_404=True), - service_account=service_account, - organization_id=organization.id, - date_deactivated__isnull=True, - ) - except ServiceAccountToken.DoesNotExist: - raise ResourceDoesNotExist - token.date_deactivated = timezone.now() - token.save() + # Set only once the save succeeds, so a flush failure is told apart from a failed save. + revoked: ServiceAccountToken | None = None + try: + with outbox_context(transaction.atomic(using=router.db_for_write(ServiceAccountToken))): + try: + token = ServiceAccountToken.objects.select_for_update().get( + id=to_valid_int_id("token_id", token_id, raise_404=True), + service_account=service_account, + organization_id=organization.id, + date_deactivated__isnull=True, + ) + except ServiceAccountToken.DoesNotExist: + raise ResourceDoesNotExist + token.date_deactivated = timezone.now() + token.save() + revoked = token + except (OutboxDatabaseError, OutboxFlushError): + if revoked is None: + raise + # The revocation and its outbox are committed, so it reaches the cell when the + # outbox drains. + logger.warning( + "service_accounts.token.outbox-flush-failure", + extra={"token_id": revoked.id, "organization_id": organization.id}, + exc_info=True, + ) self.create_audit_entry( request, diff --git a/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py b/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py index 1478072fc149..c9f4a1e660fa 100644 --- a/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py +++ b/tests/sentry/service_accounts/endpoints/test_organization_service_account_tokens.py @@ -30,7 +30,6 @@ from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, TestCase from sentry.testutils.helpers.datetime import freeze_time -from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, control_silo_test FEATURE_FLAG = "organizations:service-accounts" @@ -418,7 +417,7 @@ def setUp(self) -> None: self.token = self.create_token() def test_revoke(self) -> None: - with self.feature(FEATURE_FLAG), outbox_runner(): + with self.feature(FEATURE_FLAG): self.get_success_response( self.organization.slug, self.service_account.id, self.token.id, status_code=204 ) @@ -441,6 +440,21 @@ def test_revoke(self) -> None: "service_account_name": "ci", } + def test_revoke_succeeds_when_outbox_flush_fails(self) -> None: + with ( + self.feature(FEATURE_FLAG), + mock.patch( + "sentry.service_accounts.models.ServiceAccountToken.handle_async_replication", + side_effect=Exception("cell unavailable"), + ), + ): + self.get_success_response( + self.organization.slug, self.service_account.id, self.token.id, status_code=204 + ) + + self.token.refresh_from_db() + assert self.token.date_deactivated is not None + def test_revoked_token_returns_404(self) -> None: with self.feature(FEATURE_FLAG): self.get_success_response(