Skip to content

fix(service-accounts): Accept service accounts on control silo endpoints - #129460

Draft
gricha wants to merge 1 commit into
masterfrom
fix/sa-control-silo-auth
Draft

gricha wants to merge 1 commit into
masterfrom
fix/sa-control-silo-auth

Conversation

@gricha

@gricha gricha commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Service account viewer contexts now authenticate on control silo endpoints. authenticate_service_account returned None in control because it read the cell-only ServiceAccountReplica, so in production every control endpoint treated a service account as anonymous (401). Single-process local runs hide this. Seer code mode already offers the integration and Sentry App endpoints to agents.

On control it now reads ServiceAccount, the source record, with the cell's rules: active, same org, organizations:service-accounts, and scopes from the account. Changes to an account apply on control right away, not after its outbox replicates. Each service account request on control costs one org RPC for the flag check.

from_service_account_auth (#129343) reads all_projects from the cell replica, which raises AvailabilityError (500) on control. On control it now reads ServiceAccount too.

Getting the Sentry App endpoints to work, and keeping the scope ceiling intact, took these changes:

  • Sentry App permissions listed the caller's orgs by request.user.id and required org membership. They now use acting_user_id, and a service account counts as part of its own organization, the same check OrganizationReleasesBaseEndpoint uses.
  • SentryAppSerializer looked up the caller's orgs by request.user.id. A service account now gets the non-member view, with no owner, clientId or clientSecret.
  • SentryAppDetails PUT asserted a User. Its analytics, and those of DELETE and SentryAppUpdater, now record no user for a service account, so user_id becomes optional.
  • Requests that name the acting user to the app now need a user and return 403 user-required. This covers external requests, external issue actions, and deleting an unpublished app that has installations. With a service account, building the uninstall webhook raised TypeError, because its actor is a user.
  • /auth-config/<org>/ warns a logged-in non-member with their email. A service account is authenticated but has no email, so it got a 500 (AttributeError). The warning now checks has_user, and a service account gets the config with no warning.
  • The API index (/api/0/) looked the caller up as a User whenever it was authenticated. A service account has no user, so it got 403 user-required. It now checks has_user, and a service account gets user: null with its scopes, like an API key.
  • Legacy API keys now set requires_user. They're gated on org:admin, and until now only cell-only auth kept service accounts out. Without the guard, an org:admin service account could read legacy API keys' secrets and set a key's scopes to service_account:write, past its own ceiling. The run below shows this.

Service account management needs no guard: writes need service_account:write (#129481) and reads need service_account:read (#129497), and a service account can't hold either.

Reach this adds. On control, a service account now does what a user token with the same scopes does, except as listed above. With org:admin it can delete integrations and unpublished or internal Sentry Apps. With org:write it can change an internal integration's webhook URL. These were already refused, and I probed them: org auth tokens, internal integration tokens, rotating an app secret, creating or installing apps, and waiving data secrecy.

ServiceAccountAccess's project-link lookups still read cell tables. No control endpoint calls them today, so one that starts to will fail loud.

Evidence

The agent-token matrix had 12 control rows as strict xfails (ServiceAccountGap.CONTROL_SILO). 10 now match a user token with the same scopes. External requests and external issue actions move to SERVICE_ACCOUNT_USER_REQUIRED. The remaining xfails are the NO_MEMBERSHIP and NO_TEAM_ACCESS gaps, which this PR doesn't change.

Master src/ with this PR's tests:

FAILED test_public_get_155_OrganizationIntegrations_service_account_viewer_context
FAILED test_public_get_161_OrganizationIntegrationDetails_service_account_viewer_context
FAILED test_public_get_167_OrganizationIntegrationChannelValidate_service_account_viewer_context
FAILED test_public_get_173_OrganizationIntegrationChannels_service_account_viewer_context
FAILED test_public_get_419_OrganizationSentryAppComponents_service_account_viewer_context
FAILED test_public_get_425_OrganizationSentryApps_service_account_viewer_context
FAILED test_public_get_839_SentryAppInstallationExternalRequests_service_account_viewer_context
FAILED test_public_get_845_SentryAppDetails_service_account_viewer_context
FAILED test_public_mutation_174_OrganizationIntegrationDetails_delete_service_account_viewer_context
FAILED test_public_mutation_741_SentryAppInstallationExternalIssueActions_post_service_account_viewer_context
FAILED test_public_mutation_748_SentryAppDetails_delete_service_account_viewer_context
FAILED test_public_mutation_755_SentryAppDetails_put_service_account_viewer_context
12 failed, 1 passed, 1714 deselected in 9.42s

FAILED tests/sentry/api/test_authentication.py::TestViewerContextAuthentication::test_service_account_authenticates_in_control_silo
FAILED tests/sentry/auth/test_access.py::ServiceAccountAccessTest::test_control_silo_reads_the_account
FAILED tests/sentry/auth/test_access.py::ServiceAccountAccessTest::test_control_silo_inactive_account_has_no_access
    (both: SiloLimit.AvailabilityError: Called `ServiceAccountReplica.get_queryset` on server in CONTROL mode)
FAILED tests/sentry/service_accounts/endpoints/test_organization_service_accounts.py::OrganizationServiceAccountDetailsTest::test_service_account_cannot_update_service_accounts
FAILED tests/sentry/api/endpoints/test_organization_api_key_index.py::OrganizationApiKeyIndex::test_service_account_cannot_list_keys
FAILED tests/sentry/api/endpoints/test_organization_api_key_details.py::OrganizationApiKeyDetailsPut::test_service_account_cannot_update_key
FAILED tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py::DeleteSentryAppDetailsTest::test_service_account_delete_unpublished_app
FAILED tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py::DeleteSentryAppDetailsTest::test_service_account_cannot_delete_unpublished_app_with_installs
8 failed in 6.73s

/auth-config/<org>/ with a service account viewer context, on this PR without the has_user change:

AttributeError: 'ServiceAccountPrincipal' object has no attribute 'email'
test_auth_organization_config.py:165: AssertionError: b'{"detail":"Internal Error","errorId":null}'
FAILED tests/sentry/api/endpoints/test_auth_organization_config.py::AuthOrganizationConfigEndpointTest::test_service_account_has_no_member_warning
1 failed in 3.72s

With it:

PASSED tests/sentry/api/endpoints/test_auth_organization_config.py::AuthOrganizationConfigEndpointTest::test_service_account_has_no_member_warning
1 passed in 3.79s

The endpoint's test file: 19 passed.

/api/0/ with a service account viewer context, on this PR without the has_user change:

E   AssertionError: b'{"detail":{"code":"user-required","message":"This action requires a user.","extra":{}}}'
E   assert 403 == 200
FAILED tests/sentry/api/endpoints/test_index.py::ApiIndexTest::test_service_account_has_no_user
1 failed in 27.78s

With it:

PASSED tests/sentry/api/endpoints/test_index.py::ApiIndexTest::test_service_account_has_no_user
6 passed in 258.74s (the endpoint's test file)

This PR without the two API key requires_user guards:

test_service_account_cannot_list_keys: 200
test_service_account_cannot_update_key: 200
    {"scope_list":["org:admin","org:integrations","org:read","org:write","service_account:write"],...}
2 failed in 3.92s

This PR:

PASSED test_public_get_155_OrganizationIntegrations_service_account_viewer_context
PASSED test_public_get_161_OrganizationIntegrationDetails_service_account_viewer_context
PASSED test_public_get_167_OrganizationIntegrationChannelValidate_service_account_viewer_context
PASSED test_public_get_173_OrganizationIntegrationChannels_service_account_viewer_context
PASSED test_public_get_419_OrganizationSentryAppComponents_service_account_viewer_context
PASSED test_public_get_425_OrganizationSentryApps_service_account_viewer_context
PASSED test_public_get_839_SentryAppInstallationExternalRequests_service_account_viewer_context
PASSED test_public_get_845_SentryAppDetails_service_account_viewer_context
PASSED test_public_mutation_174_OrganizationIntegrationDetails_delete_service_account_viewer_context
PASSED test_public_mutation_741_SentryAppInstallationExternalIssueActions_post_service_account_viewer_context
PASSED test_public_mutation_748_SentryAppDetails_delete_service_account_viewer_context
PASSED test_public_mutation_755_SentryAppDetails_put_service_account_viewer_context
PASSED test_user_global_get_001_UserOrganizationIntegrations_service_account_viewer_context
13 passed, 1714 deselected in 9.82s

PASSED tests/sentry/api/test_authentication.py::TestViewerContextAuthentication::test_service_account_authenticates_in_control_silo
PASSED tests/sentry/auth/test_access.py::ServiceAccountAccessTest::test_control_silo_reads_the_account
PASSED tests/sentry/auth/test_access.py::ServiceAccountAccessTest::test_control_silo_inactive_account_has_no_access
PASSED tests/sentry/service_accounts/endpoints/test_organization_service_accounts.py::OrganizationServiceAccountDetailsTest::test_service_account_cannot_update_service_accounts
PASSED tests/sentry/api/endpoints/test_organization_api_key_index.py::OrganizationApiKeyIndex::test_service_account_cannot_list_keys
PASSED tests/sentry/api/endpoints/test_organization_api_key_details.py::OrganizationApiKeyDetailsPut::test_service_account_cannot_update_key
PASSED tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py::DeleteSentryAppDetailsTest::test_service_account_delete_unpublished_app
PASSED tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py::DeleteSentryAppDetailsTest::test_service_account_cannot_delete_unpublished_app_with_installs
8 passed in 6.24s

Rebased on current master, where the matrix runs one test per endpoint with each credential as a subtest: the affected rows (-k "OrganizationIntegration or SentryApp") give 15 passed and 82 subtests passed. The test files this PR touches, plus the Sentry App permission, serializer, updater and external request/issue action tests, give 483 passed.

Before the rebase, the full matrix run serially: 1710 passed and 15 xfailed. The 2 failures were OrganizationReplayDetails (the user token and the service account), which fails locally with "Your query failed to run" for a varying set of credentials, on master too. Related suites (tests/sentry/sentry_apps, service_accounts, auth, access, permissions, API keys, analytics): 1458 passed.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

No findings on the latest commit.

skaasten added a commit that referenced this pull request Nov 2, 2026
…ount writes (#129481)

Step 6 of the service accounts open-membership vs `allProjects` plan.
Create, update and delete on the service account endpoints now require
`service_account:write` instead of `org:admin`. The scope landed in
#129204 and is granted to Manager and Owner.

This is a contract change for token callers: a token or org API key with
only `org:admin` now gets a 403 on writes and must request
`service_account:write`. The retired Admin role loses write access,
which matches the plan. Reads stay on `org:read`. The move to
`service_account:read` is a separate follow-up.

This PR does not depend on #129343 (access based on `allProjects`). It
touches the same endpoint file as #129460, but not the same lines.

<!-- junior-request-attribution:start -->
via **shaun.kaasten**.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:slack%3AC0CA89MBDQB%3A1793636454.171459 -->

--

[View
Session](https://junior-prod.sentry.dev/conversations/slack%3AC0CA89MBDQB%3A1793636454.171459)
[[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC0CA89MBDQB%3A1793636454.171459/?project=4510944073809921)

<!-- junior-session-footer:end -->

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: Shaun Kaasten <shaun.kaasten@sentry.io>
@gricha
gricha force-pushed the fix/sa-control-silo-auth branch 3 times, most recently from df4afb1 to a862919 Compare November 2, 2026 23:43
authenticate_service_account returned None in control, because it read the
cell-only ServiceAccountReplica, so control endpoints treated every service
account as anonymous. Control now reads ServiceAccount itself, with the
same rules as the cell: active, same org, the feature flag, and the
account's scopes.

The Sentry App endpoints read request.user.id and required membership.
They now use acting_user_id, and a service account counts as part of its
own organization. Requests that name the acting user to the app require
a user.

Service account management and legacy API keys now require a user. Both
gate on org:admin, so an org:admin service account could otherwise widen
other accounts or mint a key past its own scopes.
@gricha
gricha force-pushed the fix/sa-control-silo-auth branch from a862919 to fd077ab Compare November 3, 2026 01:25
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on ee09f68 in this run:

tests/sentry/auth/test_principal.py::test_request_user_id_uses_only_go_down — log
[gw3] linux -- Python 3.13.17 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/auth/test_principal.py:288: in test_request_user_id_uses_only_go_down
    assert count == REQUEST_USER_ID_BASELINE, (
E   AssertionError: request.user.id uses dropped to 345; lower REQUEST_USER_ID_BASELINE to match.
E   assert 345 == 347
tests/sentry/auth/test_principal.py::test_request_user_type_checks_only_go_down — log
[gw3] linux -- Python 3.13.17 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/auth/test_principal.py:300: in test_request_user_type_checks_only_go_down
    assert count == REQUEST_USER_TYPE_CHECK_BASELINE, (
E   AssertionError: request.user type checks dropped to 30; lower REQUEST_USER_TYPE_CHECK_BASELINE to match.
E   assert 30 == 31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant