Repository navigation
Conversation
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
force-pushed
the
fix/sa-control-silo-auth
branch
3 times, most recently
from
November 2, 2026 23:43
df4afb1 to
a862919
Compare
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
force-pushed
the
fix/sa-control-silo-auth
branch
from
November 3, 2026 01:25
a862919 to
fd077ab
Compare
Contributor
Backend Test FailuresFailures on
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Service account viewer contexts now authenticate on control silo endpoints.
authenticate_service_accountreturned None in control because it read the cell-onlyServiceAccountReplica, 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) readsall_projectsfrom the cell replica, which raisesAvailabilityError(500) on control. On control it now readsServiceAccounttoo.Getting the Sentry App endpoints to work, and keeping the scope ceiling intact, took these changes:
request.user.idand required org membership. They now useacting_user_id, and a service account counts as part of its own organization, the same checkOrganizationReleasesBaseEndpointuses.SentryAppSerializerlooked up the caller's orgs byrequest.user.id. A service account now gets the non-member view, with noowner,clientIdorclientSecret.SentryAppDetailsPUT asserted aUser. Its analytics, and those of DELETE andSentryAppUpdater, now record no user for a service account, souser_idbecomes optional.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 raisedTypeError, 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 noemail, so it got a 500 (AttributeError). The warning now checkshas_user, and a service account gets the config with no warning./api/0/) looked the caller up as aUserwhenever it was authenticated. A service account has no user, so it got 403user-required. It now checkshas_user, and a service account getsuser: nullwith its scopes, like an API key.requires_user. They're gated onorg:admin, and until now only cell-only auth kept service accounts out. Without the guard, anorg:adminservice account could read legacy API keys' secrets and set a key's scopes toservice_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 needservice_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:adminit can delete integrations and unpublished or internal Sentry Apps. Withorg:writeit 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 toSERVICE_ACCOUNT_USER_REQUIRED. The remaining xfails are theNO_MEMBERSHIPandNO_TEAM_ACCESSgaps, which this PR doesn't change.Master
src/with this PR's tests:/auth-config/<org>/with a service account viewer context, on this PR without thehas_userchange:With it:
The endpoint's test file: 19 passed.
/api/0/with a service account viewer context, on this PR without thehas_userchange:With it:
This PR without the two API key
requires_userguards:This PR:
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.