From 96394e06898b56e5dbfe98a9eef12eb3adc526d8 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:46:33 +0000 Subject: [PATCH 1/5] feat(service-accounts): Add an endpoint to create service account tokens POST /organizations/{org}/service-accounts/{id}/tokens/ mints a token that acts as the service account and returns its value once. The token's scopes must already be granted to the account. expiresAt defaults to 30 days from now. A set value must be in the future, and null creates a token that never expires. Seer agent tokens and impersonating admins can't create tokens. Each create writes a SERVICE_ACCOUNT_TOKEN_ADD audit log entry. --- src/sentry/audit_log/register.py | 8 + .../organization_service_account_tokens.py | 155 +++++++++++- .../organization_service_accounts.py | 3 +- ...est_organization_service_account_tokens.py | 223 +++++++++++++++++- 4 files changed, 381 insertions(+), 8 deletions(-) diff --git a/src/sentry/audit_log/register.py b/src/sentry/audit_log/register.py index eb09b0b3d848..e9d6c26f77ef 100644 --- a/src/sentry/audit_log/register.py +++ b/src/sentry/audit_log/register.py @@ -792,3 +792,11 @@ template="removed service account {name}", ) ) +default_manager.add( + AuditLogEvent( + event_id=226, + name="SERVICE_ACCOUNT_TOKEN_ADD", + api_name="service-account-token.create", + template="added token {name} to 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 39204da147e5..91800c55b6ae 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -7,29 +7,49 @@ from __future__ import annotations from collections.abc import Mapping -from datetime import datetime -from typing import Any, TypedDict +from datetime import datetime, timedelta +from typing import Any, NotRequired, TypedDict, cast +from django.utils import timezone from drf_spectacular.utils import extend_schema +from rest_framework import serializers +from rest_framework.permissions import SAFE_METHODS, BasePermission from rest_framework.request import Request from rest_framework.response import Response +from sentry import audit_log from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import control_silo_endpoint from sentry.api.paginator import OffsetPaginator +from sentry.api.permissions import DisallowImpersonatedTokenCreation from sentry.api.serializers import Serializer, serialize -from sentry.apidocs.constants import RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED +from sentry.api.utils import generate_locality_url +from sentry.apidocs.constants import ( + RESPONSE_BAD_REQUEST, + RESPONSE_FORBIDDEN, + RESPONSE_NOT_FOUND, + RESPONSE_UNAUTHORIZED, +) from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import acting_user_id +from sentry.models.organizationmapping import OrganizationMapping from sentry.organizations.services.organization import ( RpcOrganization, RpcUserOrganizationContext, ) +from sentry.seer.agent_token import is_agent_auth from sentry.service_accounts.endpoints.organization_service_accounts import ( SERVICE_ACCOUNT_ID, + ServiceAccountPermission, ServiceAccountScopedEndpointBase, ) -from sentry.service_accounts.models import ServiceAccount, ServiceAccountToken +from sentry.service_accounts.models import MAX_NAME_LENGTH, ServiceAccount, ServiceAccountToken +from sentry.service_accounts.tokens import generate_token, stored_token_fields +from sentry.types.cell import get_locality_name_for_cell +from sentry.utils.security.orgauthtoken_token import SystemUrlPrefixMissingException + +DEFAULT_TOKEN_LIFETIME = timedelta(days=30) class ServiceAccountTokenResponse(TypedDict): @@ -40,6 +60,14 @@ class ServiceAccountTokenResponse(TypedDict): dateCreated: datetime dateLastUsed: datetime | None dateExpires: datetime | None + # Only in the response to the request that creates the token. + token: NotRequired[str] + + +class ServiceAccountTokenCreateRequestData(TypedDict): + name: str + scopes: list[str] + expiresAt: NotRequired[datetime | None] class ServiceAccountTokenSerializer(Serializer[ServiceAccountTokenResponse]): @@ -57,16 +85,65 @@ def serialize( } +class ServiceAccountTokenCreateSerializer(serializers.Serializer): + name = serializers.CharField(max_length=MAX_NAME_LENGTH, help_text="The name of the token.") + scopes = serializers.ListField( + child=serializers.CharField(), + allow_empty=False, + help_text="Scopes granted to the token. Each must already be granted to the account.", + ) + expiresAt = serializers.DateTimeField( + required=False, + allow_null=True, + help_text="When the token stops working. Defaults to 30 days from now. Send null for a token that never expires.", + ) + + def validate_expiresAt(self, expires_at: datetime | None) -> datetime | None: + if expires_at is not None and expires_at <= timezone.now(): + raise serializers.ValidationError("The expiration date must be in the future.") + return expires_at + + def validate_scopes(self, scopes: list[str]) -> list[str]: + service_account: ServiceAccount = self.context["service_account"] + ungrantable = set(scopes) - set(service_account.scope_list) + if ungrantable: + raise serializers.ValidationError( + "The service account doesn't have scopes: " + ", ".join(sorted(ungrantable)) + ) + return sorted(set(scopes)) + + +class DisallowAgentTokenWrites(BasePermission): + def has_permission(self, request: Request, view: object) -> bool: + return request.method in SAFE_METHODS or not is_agent_auth(request.auth) + + +def _audit_log_data(service_account: ServiceAccount, token: ServiceAccountToken) -> dict[str, Any]: + # Never the token or its hash. + return { + "name": token.name, + "scopes": sorted(token.scope_list), + "service_account_id": service_account.id, + "service_account_name": service_account.name, + } + + class ServiceAccountTokenEndpointBase(ServiceAccountScopedEndpointBase): # A token outlives whoever minted it, so only a person signed in as themselves may - # manage tokens, not a service account. + # manage tokens: not a service account, a Seer agent, or an impersonating admin. requires_user = True + permission_classes = ( + ServiceAccountPermission, + DisallowAgentTokenWrites, + DisallowImpersonatedTokenCreation, + ) @control_silo_endpoint class OrganizationServiceAccountTokenIndexEndpoint(ServiceAccountTokenEndpointBase): publish_status = { "GET": ApiPublishStatus.PRIVATE, + "POST": ApiPublishStatus.PRIVATE, } @extend_schema( @@ -102,3 +179,71 @@ def get( results, request.user, serializer=ServiceAccountTokenSerializer() ), ) + + @extend_schema( + operation_id="Create a Service Account Token", + parameters=[GlobalParams.ORG_ID_OR_SLUG, SERVICE_ACCOUNT_ID], + request=ServiceAccountTokenCreateSerializer, + responses={ + 201: inline_sentry_response_serializer( + "CreateServiceAccountTokenResponse", ServiceAccountTokenResponse + ), + 400: RESPONSE_BAD_REQUEST, + 401: RESPONSE_UNAUTHORIZED, + 403: RESPONSE_FORBIDDEN, + 404: RESPONSE_NOT_FOUND, + }, + ) + def post( + self, + request: Request, + organization_context: RpcUserOrganizationContext, + organization: RpcOrganization, + service_account: ServiceAccount, + ) -> Response: + """ + Create a token that acts as the service account. The response includes the token + in `token`. It's the only time the token is returned. + """ + serializer = ServiceAccountTokenCreateSerializer( + data=request.data, context={"service_account": service_account} + ) + if not serializer.is_valid(): + return Response({"detail": serializer.errors}, status=400) + data = cast(ServiceAccountTokenCreateRequestData, serializer.validated_data) + + cell_name = OrganizationMapping.objects.get(organization_id=organization.id).cell_name + try: + token_str = generate_token( + organization.slug, generate_locality_url(get_locality_name_for_cell(cell_name)) + ) + except SystemUrlPrefixMissingException: + return Response( + {"detail": "system.url-prefix is not set. You need to set this to create a token."}, + status=400, + ) + stored = stored_token_fields(token_str) + + token = ServiceAccountToken.objects.create( + service_account=service_account, + organization_id=organization.id, + name=data["name"], + scope_list=data["scopes"], + token_hashed=stored.token_hashed, + token_last_characters=stored.token_last_characters, + created_by_id=acting_user_id(request.user), + date_expires=data.get("expiresAt", timezone.now() + DEFAULT_TOKEN_LIFETIME), + ) + + self.create_audit_entry( + request, + organization=organization, + target_object=token.id, + event=audit_log.get_event_id("SERVICE_ACCOUNT_TOKEN_ADD"), + data=_audit_log_data(service_account, token), + ) + response: ServiceAccountTokenResponse = serialize( + token, request.user, serializer=ServiceAccountTokenSerializer() + ) + response["token"] = token_str + return Response(response, status=201) diff --git a/src/sentry/service_accounts/endpoints/organization_service_accounts.py b/src/sentry/service_accounts/endpoints/organization_service_accounts.py index b0301ef71bcd..ee0ccb8a41b1 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_accounts.py +++ b/src/sentry/service_accounts/endpoints/organization_service_accounts.py @@ -15,6 +15,7 @@ from drf_spectacular.utils import OpenApiParameter, extend_schema from rest_framework import serializers from rest_framework.exceptions import PermissionDenied +from rest_framework.permissions import BasePermission from rest_framework.request import Request from rest_framework.response import Response @@ -231,7 +232,7 @@ class ServiceAccountPermission(OrganizationPermission): class OrganizationServiceAccountEndpointBase(ControlSiloOrganizationEndpoint): owner = ApiOwner.COMMUNITY - permission_classes = (ServiceAccountPermission,) + permission_classes: tuple[type[BasePermission], ...] = (ServiceAccountPermission,) def convert_args( self, request: Request, *args: Any, **kwargs: Any 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 18f4d0cc27bc..fee7dc8ab597 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 @@ -1,20 +1,31 @@ +from datetime import timedelta from typing import Any from unittest import mock from django.urls import reverse +from django.utils import timezone from rest_framework.authentication import BaseAuthentication from rest_framework.request import Request +from sentry import audit_log from sentry.auth.services.auth import AuthenticatedToken +from sentry.models.auditlogentry import AuditLogEntry +from sentry.seer.agent_token import AGENT_TOKEN_KIND from sentry.service_accounts.auth import SERVICE_ACCOUNT_TOKEN_KIND from sentry.service_accounts.endpoints.organization_service_account_tokens import ( + DisallowAgentTokenWrites, OrganizationServiceAccountTokenIndexEndpoint, ) from sentry.service_accounts.models import ServiceAccount, ServiceAccountToken from sentry.service_accounts.principal import ServiceAccountPrincipal -from sentry.service_accounts.tokens import stored_token_fields +from sentry.service_accounts.tokens import ( + SERVICE_ACCOUNT_TOKEN_PREFIX, + parse_token, + stored_token_fields, +) from sentry.silo.base import SiloMode -from sentry.testutils.cases import APITestCase +from sentry.testutils.cases import APITestCase, TestCase +from sentry.testutils.helpers.datetime import freeze_time from sentry.testutils.silo import assume_test_silo_mode, control_silo_test FEATURE_FLAG = "organizations:service-accounts" @@ -104,6 +115,163 @@ def test_member_can_list(self) -> None: assert len(response.data) == 1 + @freeze_time("2026-11-09 12:00:00") + def test_create(self) -> None: + with self.feature(FEATURE_FLAG): + response = self.get_success_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:releases", "project:read", "project:read"], + status_code=201, + ) + + token = ServiceAccountToken.objects.get(id=response.data["id"]) + plaintext = response.data["token"] + assert plaintext.startswith(SERVICE_ACCOUNT_TOKEN_PREFIX) + parsed = parse_token(plaintext) + assert parsed is not None + assert parsed["org"] == self.organization.slug + assert token.token_hashed == stored_token_fields(plaintext).token_hashed + assert token.token_hashed != plaintext + assert token.service_account_id == self.service_account.id + assert token.organization_id == self.organization.id + assert token.scope_list == ["project:read", "project:releases"] + assert token.created_by_id == self.user.id + assert token.date_expires == timezone.now() + timedelta(days=30) + assert token.date_deactivated is None + assert response.data == { + "id": str(token.id), + "name": "deploys", + "scopes": ["project:read", "project:releases"], + "tokenLastCharacters": plaintext[-4:], + "dateCreated": token.date_added, + "dateLastUsed": None, + "dateExpires": token.date_expires, + "token": plaintext, + } + + entry = AuditLogEntry.objects.get(event=audit_log.get_event_id("SERVICE_ACCOUNT_TOKEN_ADD")) + assert entry.organization_id == self.organization.id + assert entry.actor_id == self.user.id + assert entry.target_object == token.id + assert entry.data == { + "name": "deploys", + "scopes": ["project:read", "project:releases"], + "service_account_id": self.service_account.id, + "service_account_name": "ci", + } + + # The plaintext is never returned again. + with self.feature(FEATURE_FLAG): + response = self.get_success_response(self.organization.slug, self.service_account.id) + assert "token" not in response.data[0] + assert plaintext not in str(response.content) + + @freeze_time("2026-11-09 12:00:00") + def test_create_with_expiration(self) -> None: + expires_at = timezone.now() + timedelta(days=30) + with self.feature(FEATURE_FLAG): + response = self.get_success_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read"], + expiresAt=expires_at.isoformat(), + status_code=201, + ) + + token = ServiceAccountToken.objects.get(id=response.data["id"]) + assert token.date_expires == expires_at + assert response.data["dateExpires"] == expires_at + + def test_create_with_null_expiration_never_expires(self) -> None: + with self.feature(FEATURE_FLAG): + response = self.get_success_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read"], + expiresAt=None, + status_code=201, + ) + + assert ServiceAccountToken.objects.get(id=response.data["id"]).date_expires is None + + @freeze_time("2026-11-09 12:00:00") + def test_create_rejects_past_expiration(self) -> None: + with self.feature(FEATURE_FLAG): + response = self.get_error_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read"], + expiresAt=(timezone.now() - timedelta(minutes=1)).isoformat(), + status_code=400, + ) + + assert set(response.data["detail"]) == {"expiresAt"} + assert not ServiceAccountToken.objects.exists() + + def test_create_rejects_scopes_the_account_lacks(self) -> None: + with self.feature(FEATURE_FLAG): + response = self.get_error_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read", "org:write"], + status_code=400, + ) + + assert "scopes" in response.data["detail"] + assert not ServiceAccountToken.objects.exists() + + def test_create_requires_name_and_scopes(self) -> None: + with self.feature(FEATURE_FLAG): + response = self.get_error_response( + self.organization.slug, + self.service_account.id, + method="post", + scopes=[], + status_code=400, + ) + + assert set(response.data["detail"]) == {"name", "scopes"} + assert not ServiceAccountToken.objects.exists() + + def test_create_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, + method="post", + name="deploys", + scopes=["project:read"], + status_code=403, + ) + + assert not ServiceAccountToken.objects.exists() + + def test_manager_can_create(self) -> None: + self.login_as_role("manager") + + with self.feature(FEATURE_FLAG): + self.get_success_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read"], + status_code=201, + ) + def test_service_account_caller_cannot_list(self) -> None: url = reverse(self.endpoint, args=[self.organization.slug, self.service_account.id]) @@ -120,6 +288,25 @@ def test_service_account_caller_cannot_list(self) -> None: assert response.status_code == 403 assert response.data["detail"]["code"] == "user-required" + def test_service_account_caller_cannot_create(self) -> None: + url = reverse(self.endpoint, args=[self.organization.slug, self.service_account.id]) + + with ( + self.feature(FEATURE_FLAG), + mock.patch.object( + OrganizationServiceAccountTokenIndexEndpoint, + "authentication_classes", + (ServiceAccountAuthentication,), + ), + ): + response = self.client.post( + url, data={"name": "deploys", "scopes": ["project:read"]}, format="json" + ) + + assert response.status_code == 403 + assert response.data["detail"]["code"] == "user-required" + assert not ServiceAccountToken.objects.exists() + def test_other_org_returns_404(self) -> None: with assume_test_silo_mode(SiloMode.CELL): other_organization = self.create_organization() @@ -129,6 +316,23 @@ def test_other_org_returns_404(self) -> None: with self.feature(FEATURE_FLAG): self.get_error_response(self.organization.slug, other.id, status_code=404) + def test_create_for_other_org_returns_404(self) -> None: + with assume_test_silo_mode(SiloMode.CELL): + other_organization = self.create_organization() + other = self.create_service_account(organization=other_organization, name="other") + + with self.feature(FEATURE_FLAG): + self.get_error_response( + self.organization.slug, + other.id, + method="post", + name="deploys", + scopes=["org:read"], + status_code=404, + ) + + assert not ServiceAccountToken.objects.filter(service_account=other).exists() + def test_deleted_account_returns_404(self) -> None: self.service_account.deactivate() @@ -136,3 +340,18 @@ def test_deleted_account_returns_404(self) -> None: self.get_error_response( self.organization.slug, self.service_account.id, status_code=404 ) + + +@control_silo_test +class DisallowAgentTokenWritesTest(TestCase): + def check(self, method: str, kind: str) -> bool: + request = mock.Mock(method=method, auth=AuthenticatedToken(kind=kind, user_id=self.user.id)) + return DisallowAgentTokenWrites().has_permission(request, view=None) + + def test_rejects_agent_writes(self) -> None: + assert not self.check("POST", AGENT_TOKEN_KIND) + assert not self.check("DELETE", AGENT_TOKEN_KIND) + + def test_allows_agent_reads_and_other_writes(self) -> None: + assert self.check("GET", AGENT_TOKEN_KIND) + assert self.check("POST", "api_token") From d8da7feee0ae3282617d7ccb34198670bd6527eb Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:46:14 +0000 Subject: [PATCH 2/5] fix(service-accounts): Limit new token scopes to the caller's scopes A token is a bearer credential returned to whoever creates it, so a manager could mint a token carrying scopes they lack, such as org:admin, from an account an owner set up. Requested scopes must now be on the account and held by the caller. --- .../organization_service_account_tokens.py | 18 +++++++++++----- ...est_organization_service_account_tokens.py | 21 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 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 91800c55b6ae..1ecf6c9bba04 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -90,7 +90,7 @@ class ServiceAccountTokenCreateSerializer(serializers.Serializer): scopes = serializers.ListField( child=serializers.CharField(), allow_empty=False, - help_text="Scopes granted to the token. Each must already be granted to the account.", + help_text="Scopes granted to the token. Each must be granted to the account and held by you.", ) expiresAt = serializers.DateTimeField( required=False, @@ -105,10 +105,17 @@ def validate_expiresAt(self, expires_at: datetime | None) -> datetime | None: def validate_scopes(self, scopes: list[str]) -> list[str]: service_account: ServiceAccount = self.context["service_account"] - ungrantable = set(scopes) - set(service_account.scope_list) - if ungrantable: + missing_on_account = set(scopes) - set(service_account.scope_list) + if missing_on_account: raise serializers.ValidationError( - "The service account doesn't have scopes: " + ", ".join(sorted(ungrantable)) + "The service account doesn't have scopes: " + ", ".join(sorted(missing_on_account)) + ) + # A token is a bearer credential handed to the caller, so it must not carry + # scopes the caller couldn't use themselves. + missing_on_caller = set(scopes) - self.context["access"].scopes + if missing_on_caller: + raise serializers.ValidationError( + "You can't grant scopes you don't have: " + ", ".join(sorted(missing_on_caller)) ) return sorted(set(scopes)) @@ -206,7 +213,8 @@ def post( in `token`. It's the only time the token is returned. """ serializer = ServiceAccountTokenCreateSerializer( - data=request.data, context={"service_account": service_account} + data=request.data, + context={"service_account": service_account, "access": request.access}, ) if not serializer.is_valid(): return Response({"detail": serializer.errors}, status=400) 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 fee7dc8ab597..e39152ffb2e0 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 @@ -231,6 +231,27 @@ def test_create_rejects_scopes_the_account_lacks(self) -> None: assert "scopes" in response.data["detail"] assert not ServiceAccountToken.objects.exists() + def test_create_rejects_scopes_the_caller_lacks(self) -> None: + service_account = self.create_service_account( + organization=self.organization, name="admin-bot", scope_list=["org:admin", "org:read"] + ) + self.login_as_role("manager") + + with self.feature(FEATURE_FLAG): + response = self.get_error_response( + self.organization.slug, + service_account.id, + method="post", + name="deploys", + scopes=["org:read", "org:admin"], + status_code=400, + ) + + assert response.data["detail"]["scopes"] == [ + "You can't grant scopes you don't have: org:admin" + ] + assert not ServiceAccountToken.objects.exists() + def test_create_requires_name_and_scopes(self) -> None: with self.feature(FEATURE_FLAG): response = self.get_error_response( From 6741cd2375506df58d51bb89e8f0c0b276fae433 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:38:04 +0000 Subject: [PATCH 3/5] ref(service-accounts): Don't flush the outbox when creating a token The token is committed before replication, so a failed flush shouldn't turn a successful create into a 500 that drops the only copy of the plaintext token. The outbox drains later and the token reaches the cell then. --- .../organization_service_account_tokens.py | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 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 1ecf6c9bba04..9bbe2d3bbd2a 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -10,6 +10,7 @@ from datetime import datetime, timedelta from typing import Any, NotRequired, TypedDict, cast +from django.db import router, transaction from django.utils import timezone from drf_spectacular.utils import extend_schema from rest_framework import serializers @@ -33,6 +34,7 @@ from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.auth.principal import acting_user_id +from sentry.hybridcloud.models.outbox import outbox_context from sentry.models.organizationmapping import OrganizationMapping from sentry.organizations.services.organization import ( RpcOrganization, @@ -232,16 +234,22 @@ def post( ) stored = stored_token_fields(token_str) - token = ServiceAccountToken.objects.create( - service_account=service_account, - organization_id=organization.id, - name=data["name"], - scope_list=data["scopes"], - token_hashed=stored.token_hashed, - token_last_characters=stored.token_last_characters, - created_by_id=acting_user_id(request.user), - date_expires=data.get("expiresAt", timezone.now() + DEFAULT_TOKEN_LIFETIME), - ) + # The token is committed before replication, so a failed flush mustn't turn the + # request into a 500 that hides the only copy of the plaintext token. The outbox + # drains later and the token reaches the cell then. + with outbox_context( + transaction.atomic(using=router.db_for_write(ServiceAccountToken)), flush=False + ): + token = ServiceAccountToken.objects.create( + service_account=service_account, + organization_id=organization.id, + name=data["name"], + scope_list=data["scopes"], + token_hashed=stored.token_hashed, + token_last_characters=stored.token_last_characters, + created_by_id=acting_user_id(request.user), + date_expires=data.get("expiresAt", timezone.now() + DEFAULT_TOKEN_LIFETIME), + ) self.create_audit_entry( request, From bce2f29bb3ae18fd98804be11a231b014842deb1 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:51:02 +0000 Subject: [PATCH 4/5] fix(service-accounts): Flush a new token's outbox after commit Turning off the flush stopped a replication failure from returning a 500, but it also left the token waiting for the scheduled outbox drain before it reached the cell. Flush after the commit instead, and log a failure rather than raising it. --- .../organization_service_account_tokens.py | 25 ++++++++-- ...est_organization_service_account_tokens.py | 46 ++++++++++++++++++- 2 files changed, 67 insertions(+), 4 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 9bbe2d3bbd2a..ab2026549f42 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -6,6 +6,7 @@ from __future__ import annotations +import logging from collections.abc import Mapping from datetime import datetime, timedelta from typing import Any, NotRequired, TypedDict, cast @@ -51,9 +52,27 @@ from sentry.types.cell import get_locality_name_for_cell from sentry.utils.security.orgauthtoken_token import SystemUrlPrefixMissingException +logger = logging.getLogger(__name__) + DEFAULT_TOKEN_LIFETIME = timedelta(days=30) +def _flush_token_outboxes(token: ServiceAccountToken) -> None: + """Replicate the token to its cell now, but don't fail the request if that fails. + + The token and its outbox are already committed, so the scheduled outbox drain + replicates it later. + """ + try: + for outbox in token.outboxes_for_update(): + outbox.drain_shard() + except Exception: + logger.exception( + "service_accounts.token.outbox_flush_failed", + extra={"token_id": token.id, "organization_id": token.organization_id}, + ) + + class ServiceAccountTokenResponse(TypedDict): id: str name: str @@ -234,9 +253,8 @@ def post( ) stored = stored_token_fields(token_str) - # The token is committed before replication, so a failed flush mustn't turn the - # request into a 500 that hides the only copy of the plaintext token. The outbox - # drains later and the token reaches the cell then. + # Flushing inside the transaction would turn a replication failure into a 500 that + # hides the only copy of the plaintext token, so flush after commit instead. with outbox_context( transaction.atomic(using=router.db_for_write(ServiceAccountToken)), flush=False ): @@ -250,6 +268,7 @@ def post( created_by_id=acting_user_id(request.user), date_expires=data.get("expiresAt", timezone.now() + DEFAULT_TOKEN_LIFETIME), ) + _flush_token_outboxes(token) 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 e39152ffb2e0..0a77fc11acb9 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 @@ -16,7 +16,11 @@ DisallowAgentTokenWrites, OrganizationServiceAccountTokenIndexEndpoint, ) -from sentry.service_accounts.models import ServiceAccount, ServiceAccountToken +from sentry.service_accounts.models import ( + ServiceAccount, + ServiceAccountToken, + ServiceAccountTokenReplica, +) from sentry.service_accounts.principal import ServiceAccountPrincipal from sentry.service_accounts.tokens import ( SERVICE_ACCOUNT_TOKEN_PREFIX, @@ -170,6 +174,46 @@ def test_create(self) -> None: assert plaintext not in str(response.content) @freeze_time("2026-11-09 12:00:00") + def test_create_replicates_token_to_cell(self) -> None: + with self.feature(FEATURE_FLAG): + response = self.get_success_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read"], + status_code=201, + ) + + with assume_test_silo_mode(SiloMode.CELL): + assert ServiceAccountTokenReplica.objects.filter( + token_id=int(response.data["id"]) + ).exists() + + def test_create_returns_token_when_outbox_flush_fails(self) -> None: + with ( + self.feature(FEATURE_FLAG), + mock.patch( + "sentry.hybridcloud.models.outbox.ControlOutbox.drain_shard", + side_effect=Exception("cell unavailable"), + ), + ): + response = self.get_success_response( + self.organization.slug, + self.service_account.id, + method="post", + name="deploys", + scopes=["project:read"], + status_code=201, + ) + + assert response.data["token"].startswith(SERVICE_ACCOUNT_TOKEN_PREFIX) + assert ServiceAccountToken.objects.filter(id=response.data["id"]).exists() + with assume_test_silo_mode(SiloMode.CELL): + assert not ServiceAccountTokenReplica.objects.filter( + token_id=int(response.data["id"]) + ).exists() + def test_create_with_expiration(self) -> None: expires_at = timezone.now() + timedelta(days=30) with self.feature(FEATURE_FLAG): From 41c18dd01bc54725cd0f225cf0387ddf7baf95b7 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:56:13 +0000 Subject: [PATCH 5/5] ref(service-accounts): Catch outbox flush errors like ApiTokensEndpoint Flush in the transaction as usual and catch OutboxDatabaseError and OutboxFlushError once the token is bound, the same way ApiTokensEndpoint.post does, instead of a separate post-commit drain helper. --- .../organization_service_account_tokens.py | 60 +++++++++---------- ...est_organization_service_account_tokens.py | 2 +- 2 files changed, 29 insertions(+), 33 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 ab2026549f42..ebdc434d7828 100644 --- a/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py +++ b/src/sentry/service_accounts/endpoints/organization_service_account_tokens.py @@ -35,7 +35,11 @@ from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.auth.principal import acting_user_id -from sentry.hybridcloud.models.outbox import outbox_context +from sentry.hybridcloud.models.outbox import ( + OutboxDatabaseError, + OutboxFlushError, + outbox_context, +) from sentry.models.organizationmapping import OrganizationMapping from sentry.organizations.services.organization import ( RpcOrganization, @@ -57,22 +61,6 @@ DEFAULT_TOKEN_LIFETIME = timedelta(days=30) -def _flush_token_outboxes(token: ServiceAccountToken) -> None: - """Replicate the token to its cell now, but don't fail the request if that fails. - - The token and its outbox are already committed, so the scheduled outbox drain - replicates it later. - """ - try: - for outbox in token.outboxes_for_update(): - outbox.drain_shard() - except Exception: - logger.exception( - "service_accounts.token.outbox_flush_failed", - extra={"token_id": token.id, "organization_id": token.organization_id}, - ) - - class ServiceAccountTokenResponse(TypedDict): id: str name: str @@ -253,22 +241,30 @@ def post( ) stored = stored_token_fields(token_str) - # Flushing inside the transaction would turn a replication failure into a 500 that - # hides the only copy of the plaintext token, so flush after commit instead. - with outbox_context( - transaction.atomic(using=router.db_for_write(ServiceAccountToken)), flush=False - ): - token = ServiceAccountToken.objects.create( - service_account=service_account, - organization_id=organization.id, - name=data["name"], - scope_list=data["scopes"], - token_hashed=stored.token_hashed, - token_last_characters=stored.token_last_characters, - created_by_id=acting_user_id(request.user), - date_expires=data.get("expiresAt", timezone.now() + DEFAULT_TOKEN_LIFETIME), + token: ServiceAccountToken | None = None + try: + # Binds `token` before the flush runs, so it's set when only replication fails. + with outbox_context(transaction.atomic(using=router.db_for_write(ServiceAccountToken))): + token = ServiceAccountToken.objects.create( + service_account=service_account, + organization_id=organization.id, + name=data["name"], + scope_list=data["scopes"], + token_hashed=stored.token_hashed, + token_last_characters=stored.token_last_characters, + created_by_id=acting_user_id(request.user), + date_expires=data.get("expiresAt", timezone.now() + DEFAULT_TOKEN_LIFETIME), + ) + except (OutboxDatabaseError, OutboxFlushError): + if token is None: + raise + # The token and its outbox are committed, so it reaches the cell when the + # outbox drains. Failing here would lose the only copy of the plaintext token. + logger.warning( + "service_accounts.token.outbox-flush-failure", + extra={"token_id": token.id, "organization_id": organization.id}, + exc_info=True, ) - _flush_token_outboxes(token) 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 0a77fc11acb9..e02d5d3273e8 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 @@ -194,7 +194,7 @@ def test_create_returns_token_when_outbox_flush_fails(self) -> None: with ( self.feature(FEATURE_FLAG), mock.patch( - "sentry.hybridcloud.models.outbox.ControlOutbox.drain_shard", + "sentry.service_accounts.models.ServiceAccountToken.handle_async_replication", side_effect=Exception("cell unavailable"), ), ):