stacklok / stacklok/toolhive

Cedar claim source and principal differ between JWT and opaque upstream access tokens

Open
#6,048 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

When a workload pins an upstream provider (primaryUpstreamProvider, forced by the
runner whenever the embedded auth server is active), Cedar resolves policy claims
along two different code paths depending on whether the upstream's access token
happens to be JWT-shaped. The two paths disagree about both the claim source and the
Cedar principal, so the same policy can behave differently for two providers that
are otherwise configured identically.

resolveClaims in pkg/authz/authorizers/cedar/core.go:

  • JWT access token (the branch fixed in #6022 / #6036): claims come from that
    provider's access token, supplemented for name/email from the same provider's
    id_token. The Cedar principal is the upstream subject.
  • Opaque access token (Google ya29.*, GitHub gho_* — the !looksLikeJWT
    fallback added in #5147): claims come from the ToolHive-issued token the client
    presented. The Cedar principal is Client::"<ToolHive internal user ID>".

Two consequences, both silent:

1. The principal is not the upstream subject on the opaque path. The AS-issued
sub is an internal user ID — a fresh UUID from UserResolver
(pkg/authserver/server/handlers/user.go), not the upstream subject, which travels
separately as UpstreamSubject (pkg/authserver/server/handlers/callback.go). So a
rule written as

forbid(principal == Client::"okta|alice", action, resource);

matches with a JWT-issuing upstream and silently does not match with an
opaque-token upstream. Cedar has no has-style guard in the principal position, so
there is no diagnostic — the request simply carries a different principal, which a
broad permit may legitimately match.

2. Claim provenance is wrong on the opaque path in multi-upstream chains. The
profile claims the auth server mirrors into its own token always come from the
first configured upstream: validateChain
(pkg/authserver/server/handlers/handler.go) requires chain[0] to be
upstreams[0], and only the first leg resolves identity from its provider
(userEmail = result.Email, callback.go), with later legs carrying those values
forward. primaryUpstreamProvider may name any upstream — the operator validates
membership, not position, and pinning a non-first provider is documented in the
vMCP guide. So with upstreamProviders: [okta, github] and
primaryUpstreamProvider: github, the opaque path feeds okta's email into a
claim set the policy reads as github's. verifyChainIdentity guarantees it is the
same canonical user, so this is not another person's identity — but a gate
distinguishing a corporate IdP from a personal one is exactly the policy it
subverts.

This is the same defect #6036 removed from the JWT path; #5147 predates that
reasoning, so the opaque branch still has it.

Steps to reproduce

For (1), the shortest demonstration is a unit test rather than a deployment: give
TestAuthorizeWithJWTClaims_PrincipalStaysUpstreamSourced
(pkg/authz/authorizers/cedar/core_test.go) a case whose UpstreamTokens entry is
an opaque string such as ya29.opaque instead of a JWT. The
forbid(principal == Client::"hub|banned-alice", ...) rule stops matching and the
broad permit applies.

End to end: run two workloads with the embedded auth server, one behind an upstream
that issues JWT access tokens and one behind Google or GitHub, with the same Cedar
policy keyed on Client::"<upstream-subject>". The rule applies on the first and
not the second.

Expected behavior

The claim source and the principal should not depend on the wire format of the
upstream access token. Both paths should read the pinned provider's own credentials,
so the principal is the upstream subject and profile claims carry that provider's
provenance regardless of token shape.

Actual behavior

The opaque path evaluates the ToolHive-issued token's claims and derives the
principal from ToolHive's internal user ID.

Additional context

The fix direction, given what #6036 established: the opaque branch should supplement
from the pinned provider's id_token (identity.UpstreamIDTokens[provider]) rather
than from identity.Claims. An OIDC upstream always has one — upstream/oidc.go
rejects a login without an id_token and force-adds the openid scope — and it
survives refresh (upstreamtoken/service.go carries the original forward when a
refresh returns none). exp should deliberately not be enforced, for the reasons
now documented on Identity.UpstreamIDTokens.

Two things make this more than a tidy-up, and also mean it needs its own change
rather than riding along with #6036:

  • It is a behaviour change on a shipped path. Google and GitHub deployments would
    see the Cedar principal change from Client::"<uuid>" to
    Client::"<upstream-subject>". Any policy keyed on the UUID form would stop
    matching. That is unlikely to be deliberate — nobody sensibly writes a policy
    against a random UUID — but it is a real migration consideration and deserves a
    release note.
  • A pure OAuth 2.0 upstream never asked for openid has no stored id_token, so it
    has no provenance-correct source for name/email at all. That case should fall
    back to today's behaviour or fail closed, deliberately chosen and documented, not
    left implicit.

Current behaviour is pinned by opaque_upstream_token_still_falls_back in
pkg/authz/integration_test.go, with a comment recording that the inconsistency is
intentional-for-now — that test will need updating as part of the fix.

Related: #5916, #5147, #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 with resolveClaims in pkg/authz/authorizers/cedar/core.go and compare its JWT and opaque-token paths with TestAuthorizeWithJWTClaims_PrincipalStaysUpstreamSourced in pkg/authz/authorizers/cedar/core_test.go. Then run opaque_upstream_token_still_falls_back in pkg/authz/integration_test.go and inspect the upstream identity handling named in the issue. Done means both token shapes use the pinned provider's provenance, with the OAuth-without-id_token behavior and migration impact deliberately documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
authentication, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.