Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 51 additions & 14 deletions docs/operator/virtualmcpserver-kubernetes-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -671,28 +671,65 @@ 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, so a rule keyed on `Client::"<internal-user-id>"` stops matching and one keyed
on `Client::"<upstream-subject>"` starts;
* in a multi-upstream chain `claim_email` now names the pinned provider's email
rather than the first configured upstream's;
* claims that only the ToolHive-issued token carried — `claim_client_id`, plus its
own `claim_iss`, `claim_aud` and `claim_exp` — are no longer visible, so a policy
gating tool access on a specific OAuth client via `claim_client_id` silently stops
matching. Those claims were never available when the pinned upstream issued a JWT
access token either, so this makes the two behave alike; gate on the upstream
subject or a claim the upstream asserts instead.

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
Expand Down
158 changes: 131 additions & 27 deletions pkg/authz/authorizers/cedar/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -578,7 +583,42 @@ 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 consequence worth stating: the claims the ToolHive-issued token carries in its
// own right — `client_id` (see session.New in pkg/authserver/server/session) plus
// fosite's `iss`/`aud`/`exp`/`iat`/`jti` — are no longer visible to policies on
// this path, so a rule gating on `claim_client_id` stops matching. That removes an
// asymmetry rather than creating one: the JWT branch never had those either, since
// neither the upstream access token nor its id_token carries this auth server's
// client_id, and any `client_id` an upstream access token does assert is the auth
// server's own registration at that upstream, not the calling MCP client. The loss
// also fails toward deny, since `principal has claim_client_id` becomes false.
//
// 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:
Expand All @@ -594,7 +634,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
Expand Down Expand Up @@ -634,14 +676,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)
Expand All @@ -652,6 +687,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.
Expand Down Expand Up @@ -704,16 +802,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
Expand Down
Loading
Loading