Repository navigation
Conversation
Contributor
Sentry Snapshot Testing
|
gricha
force-pushed
the
ref/sa-narrow-users
branch
from
November 1, 2026 04:24
1680b0e to
27d5e3b
Compare
User, auth, auth_v2 and relocation code reads the caller through `sentry.auth.principal`: person-only paths (passwords, 2FA, account merge, relocation creator) use `require_user()`, self checks in user permissions use `is_acting_user()`, "mine" lookups (relocation list, `id:me`) use `owned_by_acting_user()`, and logs use `acting_user_id()`. `access.from_user()`, `normalize_valid_user()` and `SessionSerializer.serialize()` take a service account (type-only). - A service account gets an empty relocation list instead of 403 (new test); creating or retrying one is 403 `user-required`. User and auth_v2 endpoints are control-silo. - An API key deleting a user's last primary authenticator with backup codes gets 401 instead of a 500. - Users and agents see no change.
gricha
force-pushed
the
ref/sa-narrow-users
branch
from
November 1, 2026 04:57
27d5e3b to
f761ad0
Compare
Comment on lines
+236
to
+238
| account=require_user(request), | ||
| type="mfa-removed", | ||
| actor=request.user, | ||
| actor=require_user(request), |
Contributor
There was a problem hiding this comment.
Backup MFA removal records the caller as the account owner
When an admin removes another user's last primary authenticator, this call passes the admin from require_user(request) as account, so capture_security_activity records the backup removal under the admin's user_id rather than the authenticator owner's. Pass account=user and actor=request.user, matching the other removal calls; the helper accepts AnonymousUser as an actor, so this also avoids rejecting an API-key caller at this point.
Evidence
- In the last-primary branch of
UserAuthenticatorDetailsEndpoint.delete(), the backup authenticators belong touser, but the call passesrequire_user(request)asaccount. capture_security_activitylogsaccount.idasuser_id, so when an admin removes another user's authenticator, the audit event is attributed to the admin rather than the target.UserEndpoint.requires_userrejects authenticated principals without a user, but an API key'sAnonymousUseris unauthenticated; this endpoint's organization permission path can allow the request to reach the branch.- For that API-key request,
require_user(request)raisesNotAuthenticatedinside the atomic block, rolling back the deletions. The helper acceptsAnonymousUserasactor, and sibling removal calls passaccount=userandactor=request.user.
Identified by Warden · sentry-backend-bugs · T5J-3YH
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.
User, auth, auth_v2 and relocation code reads the caller through
sentry.auth.principal: person-only paths (passwords, 2FA, account merge, relocation creator) userequire_user(), self checks in user permissions useis_acting_user(), "mine" lookups (relocation list,id:me) useowned_by_acting_user(), and logs useacting_user_id().access.from_user(),normalize_valid_user()andSessionSerializer.serialize()take a service account (type-only).user-required. User and auth_v2 endpoints are control-silo.Left:
user_authenticator_details.pypasses the caller tocapture_security_activityon a path API keys reach, so it stays on #129402's mypy allowlist.