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..709ba56e4d40 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,65 @@ 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.""" + # 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, + 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/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/$'), 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..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 @@ -407,6 +407,120 @@ 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): + 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 + 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") + ) + 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_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( + 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: