Skip to content

feat(api): Let endpoints and code require a user - #129143

Merged
gricha merged 1 commit into
ref/request-actor-adoptionfrom
ref/requires-user
Oct 30, 2026
Merged

gricha merged 1 commit into
ref/request-actor-adoptionfrom
ref/requires-user

Conversation

@gricha

@gricha gricha commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Endpoints accept every caller by default: users, agents and service accounts. One that only works for a user sets requires_user = True, and a ServiceAccountPrincipal (authenticated, no user) gets a 403 user-required before the handler runs. UserEndpoint and RegionSiloUserEndpoint set it, so most account routes are covered by inheritance.

Which one to use:

  • Browser-only endpoints use SessionNoAuthTokenAuthentication, which refuses every token, so a service account never authenticates there.
  • requires_user is for endpoints people may call with any credential, API tokens included, but a caller with no user may not.
  • require_user(request) is for one user-only path inside an otherwise shared endpoint. It returns the user, or raises NotAuthenticated (anonymous) or UserRequired (no user). Creating a project calls it, since the new project's default team is the creator's own.

Every UserRequired increments api.user_required, tagged by endpoint class (the same tag record_deprecated_scope_translation uses). That gives a burn-down list of endpoints service accounts still can't use.

Matrix: fixes 1 service-account row (known gaps 89 → 88). The row is project creation, which now answers user-required instead of a 500 reading the principal's email.

Stack (each PR is based on the one above it):

  1. ref(auth): Add a shared request.user interface #128982 Shared request.user interface
  2. ref(auth): Set request.user and request.auth through one helper #129133 Set request.user / request.auth through one helper
  3. feat(auth): Put service accounts on request.user as a principal #129146 Service accounts on request.user
  4. ref(auth): Ask has_user() in access, org lookups and users/me #129136 has_user() in access, org lookups and users/me
  5. feat(api): Let endpoints and code require a user #129143 requires_user / require_user()
  6. ref(auth): Lint request.user patterns that break for callers with no user #129144 Lints for patterns that break for a service account
  7. ref(orgs): Ask has_user() for the acting user in org and team endpoints #129154 Orgs, teams, projects
  8. ref(issues): Ask has_user() for the acting user on issue actions #129155 Issues
  9. ref(explore): Ask has_user() for the acting user on saved queries and events #129149 Discover, Explore
  10. ref(dashboards): Use acting_user_id() for the dashboard creator #129156 Dashboards
  11. ref(workflow_engine): Ask has_user() for the detector and workflow creator #129150 Workflow engine
  12. ref(replays): Use acting_user_id() for the viewer #129151 Replays
  13. ref(preprod): Use acting_user_id() for the uploader #129152 Preprod
  14. ref(auth): Ask has_user() for the acting user in the remaining endpoints #129153 Remaining endpoints
  15. ref(audit): Attribute audit entries only to a user #129253 Audit entries
  16. ref(api): Skip per-user serializer lookups when no user is calling #129254 Per-user serializer lookups
  17. ref(users): Serialize no user when no user is behind the caller #129255 serialize_generic_user
  18. ref(features): Give flagpole no user context without a user #129256 Flagpole user context

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 29, 2026
@gricha
gricha added this pull request to stack #129145 October 29, 2026 06:46
@gricha
gricha force-pushed the ref/requires-user branch from d2a9022 to 4d886d1 Compare October 29, 2026 16:25
@gricha
gricha force-pushed the ref/requires-user branch 2 times, most recently from 0035828 to b72671c Compare October 29, 2026 18:49
@gricha
gricha removed this pull request from stack #129145 October 29, 2026 18:54
@gricha
gricha added this pull request to stack #129233 October 29, 2026 18:54
@gricha
gricha removed this pull request from stack #129233 October 29, 2026 20:47
@gricha
gricha force-pushed the ref/requires-user branch from b72671c to 7134e43 Compare October 29, 2026 20:48
@gricha
gricha marked this pull request as ready for review October 29, 2026 21:04
@gricha
gricha requested review from a team as code owners October 29, 2026 21:04
@gricha
gricha requested review from azulus and trevor-e and removed request for a team October 29, 2026 21:05
@gricha
gricha force-pushed the ref/requires-user branch 2 times, most recently from 2e7d750 to 048e97b Compare October 30, 2026 04:09
@gricha gricha changed the title feat(api): Let endpoints and code require a human caller feat(api): Let endpoints and code require a user Oct 30, 2026
@gricha
gricha force-pushed the ref/requires-user branch from 048e97b to 4994e49 Compare October 30, 2026 05:06
@sentry

sentry Bot commented Oct 30, 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 force-pushed the ref/requires-user branch 2 times, most recently from efd4c4f to 8aea04f Compare October 30, 2026 06:37
@gricha
gricha force-pushed the ref/requires-user branch from 8aea04f to debfd0c Compare October 30, 2026 08:56

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit debfd0c. Configure here.

Comment thread src/sentry/auth/principal.py Resolved
@gricha
gricha removed this pull request from stack #129262 October 30, 2026 17:56
@gricha
gricha added this pull request to stack #129336 October 30, 2026 17:57
@gricha
gricha force-pushed the ref/requires-user branch from debfd0c to 112658a Compare October 30, 2026 18:10
gricha added a commit that referenced this pull request Oct 30, 2026
Adds `acting_user_id(principal)` in `sentry.auth.principal`: the user
behind whatever sits on `request.user`, or None. A user gives its own
id, an agent the user it acts for, anyone else None. `RequestPrincipal`
is the interface every caller shares; Django's `AnonymousUser` is
patched at startup with the two members it lacks (`is_sentry_app`,
`get_display_name`), and the mypy plugin declares them.

The goal of the stack is that endpoints work for users, agents and
service accounts by default, without each one checking which it got. A
service account must never carry a user id, so `acting_user_id()` is the
one question every caller can answer.

It also starts a short "Request Principals" section in `src/AGENTS.md`;
each PR above extends it with what it adds.

It also adds the rest of the vocabulary, so every PR above can use it:
`has_user(principal)` narrows to `User | RpcUser` when a user is behind
the caller (the user, or an agent acting for one), and `UserRequired` is
the 403 `user-required` for code that needs one.

Nothing reads them yet.

**Stack** (each PR is based on the one above it):
1. #128982 Shared `request.user` interface
2. #129133 Set `request.user` / `request.auth` through one helper
3. #129146 Service accounts on `request.user`
4. #129136 `has_user()` in access, org lookups and `users/me`
5. #129143 `requires_user` / `require_user()`
6. #129144 Lints for patterns that break for a service account
7. #129154 Orgs, teams, projects
8. #129155 Issues
9. #129149 Discover, Explore
10. #129156 Dashboards
11. #129150 Workflow engine
12. #129151 Replays
13. #129152 Preprod
14. #129153 Remaining endpoints
15. #129253 Audit entries
16. #129254 Per-user serializer lookups
17. #129255 `serialize_generic_user`
18. #129256 Flagpole user context
@gricha
gricha force-pushed the ref/requires-user branch from 112658a to 7197457 Compare October 30, 2026 18:23
Endpoints accept every caller by default. One that only works for a person
sets requires_user, and a ServiceAccountPrincipal, which is authenticated with
user_id=None, gets a 403 user-required before the handler runs. The user API
bases (UserEndpoint, RegionSiloUserEndpoint) set it.

Code paths that need a person call require_user(request), which returns the
user or raises NotAuthenticated / UserRequired. Creating a project calls it,
since the new project's default team is the creator's own.

Every UserRequired increments api.user_required tagged by endpoint, so we can
see which endpoints service accounts still can't use.
gricha added a commit that referenced this pull request Oct 30, 2026
)

Adds `set_request_principal(request, user, auth)` and routes every place
in `src` that sets `request.user` or `request.auth` through it. That
covers the auth middleware, DRF authentication (`Request._authenticate`
is wrapped), the login/logout re-syncs, the cross-silo auth swap, the
internal API client (the request it builds and the DRF request
`initialize_request` stitches back together) and the test helpers. A
test fails on new direct assignments. Django's own `login()`/`logout()`
still set `request.user` internally; they only ever set a real or
anonymous user.

When DRF authentication finds a caller, the viewer context's actor is
updated to match. Endpoints with their own `authentication_classes` run
them after the viewer-context middleware, so the context would otherwise
describe the middleware's result. A credential with no caller of its
own, such as a signed seer-rpc call, keeps the viewer from the signed
header. So does a user that `ViewerContextAuthentication` loaded from a
signed `X-Viewer-Context`: the header already set its actor, e.g. an
agent acting for that user. The actor now also carries superuser
elevation from the credential, as the middleware already did.
`ApiClient` runs the view in its own viewer-context scope, so the view's
caller doesn't replace ours.

**Stack** (each PR is based on the one above it):
1. #128982 Shared `request.user` interface
2. #129133 Set `request.user` / `request.auth` through one helper
3. #129146 Service accounts on `request.user`
4. #129136 `has_user()` in access, org lookups and `users/me`
5. #129143 `requires_user` / `require_user()`
6. #129144 Lints for patterns that break for a service account
7. #129154 Orgs, teams, projects
8. #129155 Issues
9. #129149 Discover, Explore
10. #129156 Dashboards
11. #129150 Workflow engine
12. #129151 Replays
13. #129152 Preprod
14. #129153 Remaining endpoints
15. #129253 Audit entries
16. #129254 Per-user serializer lookups
17. #129255 `serialize_generic_user`
18. #129256 Flagpole user context
gricha added a commit that referenced this pull request Oct 30, 2026
)

On a DRF request, a service account's `request.user` is a
`ServiceAccountPrincipal` instead of `AnonymousUser`. Service accounts
are gated by `organizations:service-accounts`, which is now checked at
authentication too (`authenticate_service_account`), so that flag is the
one switch. Before, only service-account creation and the token endpoint
checked it, and anything holding the shared secret could sign a
service-account viewer context.

The principal has no user, so `has_user()` is False and
`acting_user_id()` is None, and its actor is `SERVICE_ACCOUNT`. Its `id`
is a `ServiceAccountId` that never equals an int and raises
`UserRequired` (403 `user-required`) when converted to one, so no
`request.user.id` call site can reach the user with the same number.

**Temporary:** `ServiceAccountId` is a runtime guard, because about 490
`request.user.id` reads still assume a user. It's deleted, along with
the RPC pass-through for `UserRequired`, once mypy types `request.user`
to include `ServiceAccountPrincipal` (plugin, shrinking per-module
allowlist) and those reads are migrated.

This PR sits right above the chokepoint on purpose: every PR after it is
written and tested against the real principal. Endpoints aren't blocked
up front. What breaks for a service account shows up in the auth matrix,
and the PRs above fix it or mark the endpoint `requires_user`.

What else this changes:
- **Web views:** `set_request_principal` sets a service account only on
a DRF `Request` (its setter also copies it to Django's `request.user`).
Until DRF authenticates it, Django's `request.user` is anonymous, and
session auth never sees it, since `ViewerContextAuthentication` runs
before `SessionAuthentication`. The check reads the credential
(`is_service_account_auth(auth)`), not `isinstance(user, ...)`: the
middleware passes Django's lazy user, and loading it reads the session,
which adds `Vary: Cookie` to every response.
`test_leaves_lazy_user_unloaded` guards it.
- **Access:** `SentryPermission.determine_access` asks `has_user()` and
`acting_user_id()`, so a service account keeps the credential-based
access (`from_rpc_auth`) it had as `AnonymousUser`, instead of its
`ServiceAccountId` reaching the org lookup.
- **Sudo:** neither the decorator nor the sudo middleware treats a
service account as sudo; both ask `has_user()` before
`has_usable_password()`. Sudo re-authenticates a person, so
`require_sudo()` answers a caller with no user with a 403
`user-required` instead of a sudo prompt, from `@sudo_required` and from
project delete. Org, team and project delete and project transfer stay
user-only.
- **Rate limits:** service accounts get their own bucket.
- **RPC:** `UserRequired` passes through as an API error.

The auth matrix enables the flag for its service-account rows and
records what still fails at this point in the stack. Rows that don't
match the oracle are strict xfails, so the PR that fixes a row has to
remove it. At this point 89 service-account rows are known gaps: 28
audit actors, 21 per-user serializer lookups, 20 `request.user.id`
reads, 2 `serialize_generic_user`, 1 project creation that needs
`require_user`, 12 control-silo endpoints and 5 that check the caller's
own membership or org role. The PRs above remove them. At the top of the
stack, 17 remain: the control-silo and membership ones.

This replaces #128983, split per review. It builds on #128779, now
landed.

**Stack** (each PR is based on the one above it):
1. #128982 Shared `request.user` interface
2. #129133 Set `request.user` / `request.auth` through one helper
3. #129146 Service accounts on `request.user`
4. #129136 `has_user()` in access, org lookups and `users/me`
5. #129143 `requires_user` / `require_user()`
6. #129144 Lints for patterns that break for a service account
7. #129154 Orgs, teams, projects
8. #129155 Issues
9. #129149 Discover, Explore
10. #129156 Dashboards
11. #129150 Workflow engine
12. #129151 Replays
13. #129152 Preprod
14. #129153 Remaining endpoints
15. #129253 Audit entries
16. #129254 Per-user serializer lookups
17. #129255 `serialize_generic_user`
18. #129256 Flagpole user context
@gricha
gricha force-pushed the ref/requires-user branch from 7197457 to 712b1d0 Compare October 30, 2026 18:40
@gricha
gricha merged commit 6c5a02b into master Oct 30, 2026
97 checks passed
@gricha
gricha deleted the ref/requires-user branch October 30, 2026 19:18
gricha added a commit that referenced this pull request Oct 30, 2026
Checks that meant "is there a user" asked "is logged in"
(`is_authenticated`) or read `request.user.id`. They now ask
`has_user()` or call `acting_user_id(request.user)`. Every existing
caller gets the same answer; only a `ServiceAccountPrincipal`, which is
authenticated but has no user, changes.

Nothing here is the runtime fix for service accounts: that's
`determine_access` in #129146. These change for consistency and defense:
- **Control-silo org lookups** use `acting_user_id()`.
- **`users/me`** is a defensive 404 for a service account.
- **`normalize_valid_user`** returns None unless `has_user()`, so
`from_request` with no org gives a service account no access instead of
an `OrganizationlessAccess`.
- **Rate limits** ask `has_user` instead of `isinstance(AnonymousUser)`.

The viewer-context middleware now builds its actor with `resolve_actor`
instead of a copy of it. That's deduplication: the only difference is
that a service-account credential gets `SERVICE_ACCOUNT` instead of
`UNKNOWN`, and the signed header overrides that anyway.

Service accounts are gated by `organizations:service-accounts`.

**Matrix:** fixes no service-account rows (known gaps stay at 89).

**Stack** (each PR is based on the one above it):
1. #128982 Shared `request.user` interface
2. #129133 Set `request.user` / `request.auth` through one helper
3. #129146 Service accounts on `request.user`
4. #129136 `has_user()` in access, org lookups and `users/me`
5. #129143 `requires_user` / `require_user()`
6. #129144 Lints for patterns that break for a service account
7. #129154 Orgs, teams, projects
8. #129155 Issues
9. #129149 Discover, Explore
10. #129156 Dashboards
11. #129150 Workflow engine
12. #129151 Replays
13. #129152 Preprod
14. #129153 Remaining endpoints
15. #129253 Audit entries
16. #129254 Per-user serializer lookups
17. #129255 `serialize_generic_user`
18. #129256 Flagpole user context
gricha added a commit that referenced this pull request Oct 30, 2026
…user (#129144)

A `ServiceAccountPrincipal` on `request.user` has no user
(`acting_user_id()` is None), and its `id` is a `ServiceAccountId`, not
a user id. These checks keep new code from relying on either:

- **S032** (flake8, enforced; no existing hits): `acting_user_id()` used
as a queryset lookup (`filter`, `exclude`, `get`, `get_or_create`,
`update_or_create`). It's None for a service account, so the lookup
matches rows with no user. Writing None, e.g. to `created_by_id`, is
allowed, because that's how a caller with no user is recorded.
- **S033** (flake8, enforced; no existing hits): assigning
`request.user` or `request.auth`, plainly, annotated, augmented, by
tuple unpacking or with `setattr`, anywhere but `set_request_principal`.
It replaces the AST test from #129133, so the check lives in one place.
Tests may still assign them.
- **`request.user.id` / `.pk`** (test, exact baseline 492): not a user
id for a service account. New uses fail, and the baseline only goes
down.
- **`isinstance` / `hasattr` / `getattr` on `request.user`** (test,
exact baseline 31): sniffing the class instead of branching on
`has_user()`.

The two baselines are exact, so a fix also has to lower them and they
can't drift back up.

**Matrix:** fixes no service-account rows (known gaps stay at 88).

**Stack** (each PR is based on the one above it):
1. #128982 Shared `request.user` interface
2. #129133 Set `request.user` / `request.auth` through one helper
3. #129146 Service accounts on `request.user`
4. #129136 `has_user()` in access, org lookups and `users/me`
5. #129143 `requires_user` / `require_user()`
6. #129144 Lints for patterns that break for a service account
7. #129154 Orgs, teams, projects
8. #129155 Issues
9. #129149 Discover, Explore
10. #129156 Dashboards
11. #129150 Workflow engine
12. #129151 Replays
13. #129152 Preprod
14. #129153 Remaining endpoints
15. #129253 Audit entries
16. #129254 Per-user serializer lookups
17. #129255 `serialize_generic_user`
18. #129256 Flagpole user context
gricha added a commit that referenced this pull request Oct 30, 2026
…ts (#129154)

A `ServiceAccountPrincipal` is authenticated but has no user, and its
`id` is a `ServiceAccountId` that raises `UserRequired` when used as a
user id. Org details, org index, teams, project index, organization
projects, `Team` lookups, the org deletion actor and
`auth_organization_config` treated `is_authenticated` as "is there a
user" or read `request.user.id`. They now ask `has_user(request.user)`,
or call `acting_user_id(request.user)` where they need the id, so a
service account records no user instead of getting a 403
`user-required`. The organization projects checks moved here from the
core PR. `RpcAuditLogEntryActor.actor_id` becomes optional to match.

Nothing changes for users and agents.

Split out so the owning team can review its slice.

`organizations/{org}/projects/` lists a service account's projects the
way it lists an agent's: every project in the org that its access
reaches. One without open membership sees only its linked projects, and
projects with no team are included, which the org-token branch it used
to share left out. `projects/` scopes a credential without global access
to `accessible_project_ids`; org tokens have global access, so they're
unaffected. Per warden and Bugbot; the tests land in #129254, where
service accounts first get past the project serializer.

Deleting an organization now calls `require_user(request)`, as its docs
say it needs a user. Sudo already keeps a service account out; this
keeps the rule in place if sudo ever opens to service accounts. Per
warden.

**Matrix:** fixes 1 service-account row (known gaps 88 → 87).

**Stack** (each PR is based on the one above it):
1. #128982 Shared `request.user` interface
2. #129133 Set `request.user` / `request.auth` through one helper
3. #129146 Service accounts on `request.user`
4. #129136 `has_user()` in access, org lookups and `users/me`
5. #129143 `requires_user` / `require_user()`
6. #129144 Lints for patterns that break for a service account
7. #129154 Orgs, teams, projects
8. #129155 Issues
9. #129149 Discover, Explore
10. #129156 Dashboards
11. #129150 Workflow engine
12. #129151 Replays
13. #129152 Preprod
14. #129153 Remaining endpoints
15. #129253 Audit entries
16. #129254 Per-user serializer lookups
17. #129255 `serialize_generic_user`
18. #129256 Flagpole user context
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.

2 participants