stacklok / stacklok/toolhive

Identity does not expose the upstream subject, so OAuth 2.0-only upstreams cannot have a correct Cedar principal

Open
#6,053 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-triage
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):

  • Subject and PlatformUserID — both set from the AS-issued sub in
    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, minus tsid.
  • Metadatamap[string]string with no production writer at all; the only writes
    are in identity_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

  1. Configure a workload with the embedded auth server and a type: oauth2 upstream —
    a GitHub OAuth app is the canonical case, and the one in
    docs/operator/virtualmcpserver-kubernetes-guide.md.
  2. Write any Cedar policy keyed on the principal, e.g.
    forbid(principal == Client::"github|alice", action, resource);
  3. 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 Identity not
    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's sub? They should agree for OIDC providers; if they ever disagree,
    which wins needs deciding rather than discovering. Note #6048 deliberately keeps
    sub un-supplemented because the principal has no has-style guard in Cedar, so
    this interacts with that reasoning.
  • Whether it should be a map at all. UpstreamTokens/UpstreamIDTokens are 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.go bypasses UserResolver for
    result.Synthetic upstreams 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.