Skip to content

ref(users): Narrow request.user in user, auth and relocation endpoints - #129418

Draft
gricha wants to merge 1 commit into
ref/sa-narrow-appsfrom
ref/sa-narrow-users
Draft

gricha wants to merge 1 commit into
ref/sa-narrow-appsfrom
ref/sa-narrow-users

Conversation

@gricha

@gricha gricha commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

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.

Left: user_authenticator_details.py passes the caller to capture_security_activity on a path API keys reach, so it stays on #129402's mypy allowlist.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 31, 2026
@gricha
gricha added this pull request to stack #129364 October 31, 2026 02:27
@sentry

sentry Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Sentry Snapshot Testing

Name Added Removed Changed Renamed Unchanged Skipped Status
sentry-frontend
sentry-frontend
0 0 0 0 780 0 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

@gricha
gricha removed this pull request from stack #129364 November 1, 2026 04:24
@gricha
gricha force-pushed the ref/sa-narrow-users branch from 1680b0e to 27d5e3b Compare November 1, 2026 04:24
@gricha
gricha added this pull request to stack #129440 November 1, 2026 04:24
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
gricha force-pushed the ref/sa-narrow-users branch from 27d5e3b to f761ad0 Compare November 1, 2026 04:57
Comment on lines +236 to +238
account=require_user(request),
type="mfa-removed",
actor=request.user,
actor=require_user(request),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to user, but the call passes require_user(request) as account.
  • capture_security_activity logs account.id as user_id, so when an admin removes another user's authenticator, the audit event is attributed to the admin rather than the target.
  • UserEndpoint.requires_user rejects authenticated principals without a user, but an API key's AnonymousUser is unauthenticated; this endpoint's organization permission path can allow the request to reach the branch.
  • For that API-key request, require_user(request) raises NotAuthenticated inside the atomic block, rolling back the deletions. The helper accepts AnonymousUser as actor, and sibling removal calls pass account=user and actor=request.user.

Identified by Warden · sentry-backend-bugs · T5J-3YH

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