Source Cedar claims from id_token on opaque path

With the embedded auth server active, Cedar resolved claims from two
sources depending on an accident of the pinned upstream's access token
format. A JWT access token gave claims from that upstream, so the Cedar
principal was the upstream subject. An opaque one (Google's ya29.*,
GitHub's gho_*) hit the #5147 fallback and gave the claims of the token
ToolHive itself issued, so the principal became ToolHive's internal user
UUID and `forbid(principal == Client::"okta|alice", ...)` silently stopped
matching. In a multi-upstream chain the same fallback attributed the FIRST
configured upstream's profile to the pinned provider, since that is whose
name/email the auth server mirrors.

Read the pinned provider's own id_token on that branch instead. It supplies
the principal `sub` plus the profileClaimsFromIDToken allowlist that already
governs the JWT path, so both paths now admit the same claim names from the
same provider and session, and widening the allowlist widens them together.
Claims outside it stay out: an id_token's iss/aud/nonce describe the token
rather than the user, and admitting more here than on the JWT path would
recreate the inconsistency being removed.

`sub` is not being supplemented in the sense the JWT path forbids: an opaque
token asserts nothing, so there is no access-token `sub` to override, and an
id_token that omits `sub` still fails closed with ErrMissingPrincipal rather
than borrowing the auth server's subject. `exp` is not enforced, as for the
existing supplement — the token records what the upstream asserted at login,
and rejecting it once expired would flip a policy from permit to deny
mid-session.

A pure OAuth 2.0 upstream never asked for the `openid` scope has no id_token
at all (GitHub OAuth apps are the common case) and no other record of that
provider's subject exists in the identity. For that configuration alone the
ToolHive-issued token's claims are still used, which is what shipped before:
failing closed would deny every request on those deployments with nothing an
operator could configure to recover, so the weaker attribution is the lesser
harm. A rate-limited WARN names the provider and the consequence.

Fixes #6048

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n

Author: JAORMX

docs/operator/virtualmcpserver-kubernetes-guide.md

@@ -671,28 +671,55 @@ days, expiring it out of policy evaluation would let the same user be permitted
 early in a session and denied later, with no configuration change.
 
 The token the client presented — the one ToolHive's auth server issued — is
-never a claim source here, even though it mirrors a `name` and `email`. In a
-multi-upstream chain those mirrored values come from the **first** configured
-upstream (the identity provider), which need not be the provider
-`primaryUpstreamProvider` names, so using them could attribute one IdP's email to
-another.
+never a claim source here, even though it mirrors a `name` and `email`, with the
+single exception noted below. In a multi-upstream chain those mirrored values come
+from the **first** configured upstream (the identity provider), which need not be
+the provider `primaryUpstreamProvider` names, so using them could attribute one
+IdP's email to another.
+
+**Opaque upstream access tokens**: some providers (Google's `ya29.…`, GitHub's
+`gho_…`) issue access tokens that are not JWTs at all, so there are no claims in
+them to read. For those, that same provider's `id_token` is the claim source
+instead: it supplies the principal (`sub`) and the same `name`/`email` profile
+claims, so a rule keyed on `Client::"<upstream-subject>"` matches the upstream
+identity here exactly as it does for a JWT access token. Claims the `id_token`
+carries beyond those — `iss`, `aud`, `nonce`, groups, or provider-specific ones
+like Google's `hd` — are not admitted, the same restriction that applies when a
+JWT access token is supplemented.
+
+Earlier releases evaluated the ToolHive-issued token on this path instead, so a
+policy written against that token's values needs re-checking against the pinned
+provider's `id_token`: the principal is now the upstream subject rather than
+ToolHive's internal user ID, and in a multi-upstream chain `claim_email` now names
+the pinned provider's email rather than the first configured upstream's.
 
 An OAuth 2.0 upstream that was never asked for the `openid` scope has no stored
-`id_token`, so nothing is available to fall back to and the claim stays absent.
-The proxy says so once per 30s:
+`id_token`. With a JWT access token, nothing is available to fall back to and the
+profile claim stays absent. With an opaque access token there is no upstream claim
+source at all, and only in that case does Cedar evaluate the claims of the token
+the client presented — so the principal is ToolHive's internal user ID rather than
+the upstream subject, and a rule keyed on the upstream subject will not match.
+Requesting the `openid` scope for that upstream is what fixes it. Either way the
+proxy says so once per 30s:
 
 ```
 WARN no upstream ID token stored for provider; policies referencing profile
      claims the access token omits will deny provider=okta
+
+WARN no usable upstream ID token for provider with an opaque access token;
+     falling back to the claims of the ToolHive-issued token, so the Cedar
+     principal is ToolHive's internal user ID rather than the upstream subject
+     provider=github
 ```
 
-Every other claim is upstream-access-token-only. In particular, group, role and
-scope claims are not substituted from anywhere, so a policy such as `principal in
-THVGroup::"platform-eng"` only ever matches groups the upstream access token
-asserts. The same holds for the principal: `sub` is never substituted, so a rule
-keyed on `Client::"<upstream-subject>"` keeps matching the upstream identity, and
-an access token with no `sub` is rejected outright rather than having a principal
-chosen for it.
+Every other claim comes from the upstream access token or not at all. In
+particular, group, role and scope claims are not substituted from anywhere, so a
+policy such as `principal in THVGroup::"platform-eng"` only ever matches groups the
+pinned upstream asserts. The principal is always that upstream's subject too: a JWT
+access token's `sub` is never substituted, so a JWT that carries no `sub` is
+rejected outright rather than having a principal chosen for it, and an opaque access
+token takes the subject from the `id_token` of the same provider and session — never
+from the token the client presented.
 
 If a policy references a claim that neither the access token nor the `id_token`
 carries, `principal has claim_x` is false and the policy denies — author domain

pkg/authz/authorizers/cedar/core.go

@@ -172,7 +172,8 @@ type Authorizer struct {
 	mu sync.RWMutex
 	// primaryUpstreamProvider names the upstream IDP provider whose access token
 	// is the source of JWT claims for Cedar evaluation, aside from the narrow
-	// user-profile supplement described in resolveClaims.
+	// user-profile supplement and the opaque-access-token case described in
+	// resolveClaims — both of which read the same provider's id_token.
 	// When empty, claims from the token on the original client request are used,
 	// which may be a ToolHive-issued token or any other bearer token.
 	primaryUpstreamProvider string
@@ -197,9 +198,10 @@ type Authorizer struct {
 	// the window, hiding the claim-key dump on exactly the deployments that need it.
 	supplementLog *syncutil.AtMost
 	// missingIDTokenLog rate-limits the warning that the primary provider has no
-	// usable stored id_token, so profile claims cannot be supplemented at all.
-	// Separate from supplementLog because the two are mutually exclusive per
-	// request and describe opposite conditions.
+	// usable stored id_token — meaning either that profile claims cannot be
+	// supplemented (JWT access token) or that there is no upstream claim source at
+	// all (opaque access token). Separate from supplementLog because the two are
+	// mutually exclusive per request and describe opposite conditions.
 	missingIDTokenLog *syncutil.AtMost
 	// multiValuedClaims lists JWT claim names normalized to a canonical unpadded
 	// space-delimited string, plus a companion Cedar Set, before Cedar evaluation.
@@ -223,10 +225,13 @@ type ConfigOptions struct {
 	// The profile claims in profileClaimsFromIDToken fall back to the SAME
 	// provider's id_token when its access token omits them (many OIDC providers
 	// assert `email` only in the id_token), so `principal has claim_email` keeps
-	// meaning "this upstream asserted an email". Every other claim, including all
-	// group/role/scope claims, comes from the upstream access token or not at all,
-	// and the ToolHive-issued token the client presented is never a claim source on
-	// this path. See resolveClaims for the full contract.
+	// meaning "this upstream asserted an email". An opaque access token has no
+	// claims at all, so that provider's id_token supplies the principal and those
+	// same profile claims instead. Every other claim, including all group/role/scope
+	// claims, comes from the upstream access token or not at all, and the
+	// ToolHive-issued token the client presented is a claim source only for an
+	// opaque-access-token provider with no stored id_token, which has no upstream
+	// claim source whatsoever. See resolveClaims for the full contract.
 	PrimaryUpstreamProvider string `json:"primary_upstream_provider,omitempty" yaml:"primary_upstream_provider,omitempty"`
 
 	// GroupClaimName is the JWT claim key that contains group membership for the
@@ -578,7 +583,32 @@ func (a *Authorizer) IsAuthorized(
 // The ToolHive-issued token the client presented is deliberately NOT a claim
 // source here, even though it mirrors a name/email — in a multi-upstream chain
 // those belong to the first configured upstream, which need not be the pinned
-// provider, so using them could attribute one IdP's email to another.
+// provider, so using them could attribute one IdP's email to another. The single
+// exception is the last-resort case at the end of this comment, where the pinned
+// provider offers no claim source at all.
+//
+// When the pinned provider's access token is OPAQUE rather than JWT-shaped
+// (Google's ya29.*, GitHub's gho_*) it carries no claims to read, so the same
+// provider's id_token stands in as the claim carrier: it supplies the Cedar
+// principal (`sub`) plus the profileClaimsFromIDToken it asserts, and nothing
+// else. This keeps the principal on the upstream subject and the profile claims
+// attributed to the pinned provider — what the two branches used to disagree
+// about (#6048). `sub` is not "supplemented" here in the sense the bullets below
+// forbid: it is the subject of the only claim source there is, for the same
+// provider and session, and no access-token `sub` exists to be overridden.
+//
+// A pinned provider whose access token is opaque AND that has no usable stored
+// id_token has no upstream claim source at all. That is a pure OAuth 2.0 upstream
+// (pkg/authserver/upstream/oauth2.go) never asked for the `openid` scope, whose
+// identity is resolved from a userinfo endpoint and never lands in an id_token;
+// an OIDC upstream always has one. For that configuration ALONE the claims of the
+// token on the original client request are used, exactly as they were before
+// #6048 — so the Cedar principal is ToolHive's internal user ID and the profile
+// claims are the auth server's mirror of the FIRST configured upstream. Failing
+// closed instead would deny every request on those deployments with nothing an
+// operator could configure to recover, which is the deny-all trap #5916 was; the
+// weaker attribution is the lesser harm, and a rate-limited WARN names the
+// provider so the condition is diagnosable from logs alone.
 //
 // The supplement is restricted to profileClaimsFromIDToken rather than merging
 // the two token bodies. Read this before widening it:
@@ -594,7 +624,9 @@ func (a *Authorizer) IsAuthorized(
 //   - `sub` is excluded because it becomes the Cedar principal entity ID rather
 //     than an attribute, and Cedar has no `has`-style guard in the principal
 //     position. An access token without `sub` violates RFC 9068; failing closed
-//     with ErrMissingPrincipal beats silently choosing a principal.
+//     with ErrMissingPrincipal beats silently choosing a principal. An opaque
+//     access token is a different case, not an exception to this one: it asserts
+//     nothing, so there is no access-token `sub` to override.
 //
 // An access-token value always wins, so a claim the access token does assert can
 // never be shadowed. A claim absent from both tokens stays absent, so `has`-guarded
@@ -634,14 +666,7 @@ func (a *Authorizer) resolveClaims(identity *auth.Identity) (jwt.MapClaims, erro
 		// namespaced claims) see those attributes as absent on this branch and
 		// must be authored defensively (`principal has claim_groups && ...`).
 		if !looksLikeJWT(upstreamToken) {
-			// The Warn shares claimKeyLog with logClaimKeys below so a busy
-			// Google/GitHub deployment does not emit one line per tool call.
-			a.claimKeyLog.Do(func() {
-				slog.Warn("upstream token is not a JWT; falling back to request-token claims for Cedar evaluation",
-					"provider", a.primaryUpstreamProvider)
-			})
-			a.logClaimKeys("token-fallback", requestClaims)
-			return requestClaims, nil
+			return a.claimsForOpaqueUpstreamToken(identity, requestClaims), nil
 		}
 		return nil, fmt.Errorf("failed to parse upstream token for provider %q: %w",
 			a.primaryUpstreamProvider, err)
@@ -652,6 +677,69 @@ func (a *Authorizer) resolveClaims(identity *auth.Identity) (jwt.MapClaims, erro
 	return merged, nil
 }
 
+// errNoUpstreamIDToken reports that no id_token is stored for the pinned provider,
+// so that "nothing to read" and "stored but unparsable" reach one log line.
+var errNoUpstreamIDToken = errors.New("no ID token stored for provider")
+
+// claimsForOpaqueUpstreamToken returns the claim set to evaluate when the pinned
+// provider's access token is opaque, so it carries no claims of its own.
+//
+// The same provider's id_token stands in as the claim carrier. Only its `sub` and
+// the profileClaimsFromIDToken it asserts are admitted: `sub` because the Cedar
+// principal has to come from somewhere and this is the pinned provider's own
+// subject, and the profile claims through the same allowlist that governs the
+// JWT-access-token path, so both paths admit exactly the same claim names and
+// widening the allowlist (see #6049 for group claims) widens them together. The
+// id_token's remaining claims stay out: an `iss`/`aud`/`nonce`/`at_hash` describes
+// the token rather than the user, and admitting the rest here but not on the JWT
+// path would recreate the very inconsistency this resolves.
+//
+// `exp` is not checked, as elsewhere in this file — see Identity.UpstreamIDTokens.
+//
+// requestClaims (the ToolHive-issued token's claims) is the last resort for a
+// provider with no usable id_token, which is a pure OAuth 2.0 upstream; see
+// resolveClaims for why that keeps working rather than failing closed.
+func (a *Authorizer) claimsForOpaqueUpstreamToken(identity *auth.Identity, requestClaims jwt.MapClaims) jwt.MapClaims {
+	idToken := identity.UpstreamIDTokens[a.primaryUpstreamProvider] // nil map safe in Go
+
+	idTokenClaims, err := func() (jwt.MapClaims, error) {
+		if idToken == "" {
+			return nil, errNoUpstreamIDToken
+		}
+		return parseUpstreamJWTClaims(idToken)
+	}()
+	if err != nil {
+		// Shares missingIDTokenLog with supplementFromIDToken: both report that the
+		// provider has no usable id_token, and a request takes only one of the two
+		// paths, so neither starves the other.
+		a.missingIDTokenLog.Do(func() {
+			slog.Warn("no usable upstream ID token for provider with an opaque access token; "+
+				"falling back to the claims of the ToolHive-issued token, so the Cedar principal is "+
+				"ToolHive's internal user ID rather than the upstream subject",
+				"provider", a.primaryUpstreamProvider,
+				"error", err)
+		})
+		a.logClaimKeys("token-fallback", requestClaims)
+		return requestClaims
+	}
+
+	claims := make(jwt.MapClaims, len(profileClaimsFromIDToken)+1)
+	// An id_token without `sub` violates OIDC Core 1.0 §2, and leaving the key
+	// absent makes AuthorizeWithJWTClaims fail closed with ErrMissingPrincipal
+	// rather than pick a principal from the ToolHive-issued token.
+	if sub, ok := idTokenClaims[oidcSubjectClaim]; ok {
+		claims[oidcSubjectClaim] = sub
+	}
+	for _, name := range profileClaimsFromIDToken {
+		if v, ok := idTokenClaims[name]; ok {
+			claims[name] = v
+		}
+	}
+
+	a.logClaimKeys("upstream-id-token", claims)
+	return claims
+}
+
 // supplementFromIDToken returns upstreamClaims with the profileClaimsFromIDToken
 // it lacks filled in from the primary provider's stored id_token, logging what it
 // did. upstreamClaims is not mutated.
@@ -704,16 +792,22 @@ func (a *Authorizer) supplementFromIDToken(identity *auth.Identity, upstreamClai
 	return merged
 }
 
-// OIDC Core 1.0 §5.1 standard claim names, declared here rather than borrowed
-// from another package. What this code reads is an upstream provider's id_token,
-// so these are wire names fixed by the OIDC spec — not names ToolHive chooses.
-// Anchoring them to a ToolHive constant would invert the dependency: renaming that
-// constant would silently change which claim Cedar reads out of a third party's
-// token. Two literals also do not justify pulling an authorization-server
-// dependency tree into the authorizer.
+// OIDC Core 1.0 claim names — `sub` from §2, the profile claims from §5.1 —
+// declared here rather than borrowed from another package. What this code reads is
+// an upstream provider's id_token, so these are wire names fixed by the OIDC spec,
+// not names ToolHive chooses. Anchoring them to a ToolHive constant would invert
+// the dependency: renaming that constant would silently change which claim Cedar
+// reads out of a third party's token. Three literals also do not justify pulling
+// an authorization-server dependency tree into the authorizer.
+//
+// oidcSubjectClaim is deliberately NOT in profileClaimsFromIDToken: it is never
+// supplemented into a claim set an access token already produced. It is read from
+// the id_token only when the access token is opaque and the id_token is therefore
+// the whole claim source — see claimsForOpaqueUpstreamToken.
 const (
-	oidcNameClaim  = "name"
-	oidcEmailClaim = "email"
+	oidcSubjectClaim = "sub"
+	oidcNameClaim    = "name"
+	oidcEmailClaim   = "email"
 )
 
 // profileClaimsFromIDToken are the OIDC Core §5.1 profile claims that may be read