Identity does not expose the upstream subject, so OAuth 2.0-only upstreams cannot have a correct Cedar principal
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Bug description
auth.Identity carries no record of an upstream provider's subject, so any consumer
that needs "who is this user at the upstream IdP" has to reconstruct it from a token —
and for a type: oauth2 upstream there is no token to reconstruct it from.
The concrete consequence, surfaced while fixing #6048: Cedar's principal on such a
deployment is Client::"<ToolHive internal user ID>" — a UserResolver UUID — rather
than the upstream subject. So a policy written as
forbid(principal == Client::"github|alice", action, resource);
cannot be made to match, on any configuration, for a GitHub OAuth app upstream. #6048
fixes this for OIDC upstreams with opaque access tokens (Google) by reading the stored
id_token, but an OAuth 2.0 upstream never asked for openid has no id_token by
construction, so that route does not exist for it. GitHub-as-oauth2 is the
multi-upstream example in our own operator guide, so this is a supported configuration
rather than a corner.
What Identity actually holds (verified, not inferred):
SubjectandPlatformUserID— both set from the AS-issuedsubin
claimsToIdentity(pkg/auth/context.go), i.e. ToolHive's internal user ID.Name/Email— the auth server's mirror of the first configured upstream, not
necessarily the pinned one.Groups— documented as intentionally never populated by the middleware.Claims— the AS-issued token's claims, minustsid.Metadata—map[string]stringwith no production writer at all; the only writes
are inidentity_test.go, so it is not a usable side channel either.UpstreamTokens/UpstreamIDTokens— the only upstream-derived fields, and the
id_token map has no entry for an OAuth 2.0-only provider.
Meanwhile the value does exist server-side. storage.UpstreamTokens.UpstreamSubject
(pkg/authserver/storage/types.go) is written at pkg/authserver/server/handlers/callback.go
alongside the tokens, and round-tripped through both the memory and Redis backends and
through refresher.go. It is simply never projected onto Identity: today it is
consumed only inside the auth server, for the refresh path's expected-subject check
and the account-linking guard in callback.go.
Steps to reproduce
- Configure a workload with the embedded auth server and a
type: oauth2upstream —
a GitHub OAuth app is the canonical case, and the one in
docs/operator/virtualmcpserver-kubernetes-guide.md. - Write any Cedar policy keyed on the principal, e.g.
forbid(principal == Client::"github|alice", action, resource); - Sign in as that user and call a tool.
The rule never matches, because the principal is Client::"<uuid>". There is no
configuration change that makes it match.
Expected behavior
The upstream subject should be reachable by consumers that need it, so a
principal-keyed policy behaves the same regardless of whether the upstream is OIDC or
OAuth 2.0 and regardless of its access-token format.
Actual behavior
It is unreachable outside the auth server, so the principal silently falls back to
ToolHive's internal user ID for OAuth 2.0-only upstreams.
Additional context
The natural fix is to project it onto Identity next to the two token maps — e.g. an
UpstreamSubjects map[string]string keyed by provider name, populated in
TokenValidator.Middleware from the same credential bundle that already yields
UpstreamTokens and UpstreamIDTokens (pkg/auth/token.go). That is a wider change
to pkg/auth's public surface than #6048 wanted to carry, hence this issue.
Points a design should settle:
- Trust and shape. It would be the only upstream-identity field on
Identitynot
derived from a token the consumer can inspect, so its doc needs to be explicit that
it is auth-server-asserted state rather than a parsed claim — and that it is not
redacted like the token fields, because it is an identifier rather than a credential. - Precedence for Cedar. With it available, should the principal prefer it over the
access token'ssub? They should agree for OIDC providers; if they ever disagree,
which wins needs deciding rather than discovering. Note #6048 deliberately keeps
subun-supplemented because the principal has nohas-style guard in Cedar, so
this interacts with that reasoning. - Whether it should be a map at all.
UpstreamTokens/UpstreamIDTokensare keyed
by provider and are documented as not guaranteed to share a key set; a subjects map
would add a third key set to keep consistent. A single value for the pinned provider
may be a better fit for the actual consumer. - The synthetic-identity path.
callback.gobypassesUserResolverfor
result.Syntheticupstreams and uses the synthesized subject directly, which
rotates per re-auth. Whatever is projected must have defined behaviour there.
Found while reviewing #6048; the Identity-shape audit above is that work's.
Related: #6048, #5916, #6022, #6036.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading pkg/auth/context.go and pkg/auth/token.go to trace how Identity is built, then inspect storage.UpstreamTokens in pkg/authserver/storage/types.go and its handling in callback.go and refresher.go. Review the related Cedar behavior from #6048. Done means the upstream subject is exposed with documented trust and synthetic-identity behavior, and OAuth 2.0-only upstream principals can be evaluated correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authentication, authorization, backend-api-design, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100