Problem
The multi-issuer subject-token validator (MultiIssuerTokenValidator, pkg/authserver/server/tokenexchange/multi_issuer_validator.go) can validate subject tokens from trusted external OIDC issuers, but it is not wired into factory.go (which still builds NewSelfIssuedTokenValidator), and external tokens carry no client_id claim, so checkDelegationConsent (handler.go:302) fails them closed. External token exchange is unusable until a consent model exists.
Decision (from OAuth review)
"Trusted issuer + audience == us + valid signature/subject" authorizes ToolHive as a resource, not a specific client. Trusting such a token wholesale is a confused-deputy / CWE-863 risk.
Adopt a hybrid, fail-closed consent policy for external subject tokens:
- If the subject token carries
may_act (RFC 8693 §4.4), it is authoritative: may_act.sub must equal the authenticated actor (actorID = the ToolHive confidential client performing the exchange, handler.go:104). Same rule as the self-issued path.
- Else read a per-issuer actor claim (default
azp; operators may set appid for Entra v1 or cid for Okta) and require its value to appear in a per-issuer operator allowlist. This is what authorizes a specific external client's tokens to be exchanged here.
- Else reject.
The self-issued path (client_id binding) is unchanged.
Scope — what ships in THIS PR
Per the sequencing constraint below, all of the following land together:
If the diff approaches the repo's 400-line limit, split the operator CRD/converter config surface into a follow-up PR and land the pkg/authserver side (RunConfig/Config/server config + validator + factory + consent) first — but the validator must NOT be wired into factory.go in any PR that lacks the consent policy.
Design (recommended)
Only MultiIssuerTokenValidator holds per-issuer config (ActorClaim/AllowedActors), and only the handler knows actorID. Split the policy accordingly:
- In
MultiIssuerTokenValidator.validateExternalToken (which already has issuerConfig): when the token has no may_act, resolve the actor claim from ValidatedClaims.Extra[issuerConfig.ActorClaim] (default "azp" when ActorClaim is empty), require it to be a non-empty string present in issuerConfig.AllowedActors, and fail closed otherwise. Empty AllowedActors ⇒ reject every token (mirror the existing empty-AllowedAudiences convention). When may_act is present, skip the allowlist check (may_act is the authoritative signal) and let the handler enforce it.
- Do not populate
ValidatedClaims.ClientID from the external actor claim (namespace-collision footgun). If the handler needs the resolved value, add a new field (e.g. ExternalActor string) — but the recommended split needs no new field, since the allowlist decision is made in the validator and may_act is enforced by the existing handler code.
checkDelegationConsent (handler.go:302) stays responsible for the may_act↔actorID comparison (already implemented, applies to both paths) and the self-issued client_id binding. External tokens that reach it have either passed the validator's allowlist or carry may_act.
Read the actor claim from Extra because azp/appid/cid are not well-known fields on ValidatedClaims (see assignClaim, validator.go:255).
Config plumbing
Mirror how AllowedAudiences / DelegationTokenLifespan already flow (grep those names to trace the exact pattern):
- Serializable layer:
RunConfig (pkg/authserver/config.go:36, JSON/YAML tags).
- Resolved layer:
Config (pkg/authserver/config.go:587).
- Factory-visible layer:
server.AuthorizationServerConfig (read by factory.go; already exposes AllowedAudiences and PublicJWKS()).
- Operator:
cmd/thv-operator/pkg/controllerutil/authserver.go and cmd/thv-operator/pkg/vmcpconfig/converter.go.
Add a TrustedIssuers list (issuer URL, expected audience, JWKS URL, ActorClaim, AllowedActors) through those same layers, then have factory.go construct NewMultiIssuerTokenValidator(selfValidator, issuer, trustedIssuers) when trusted issuers are configured, else keep the self-issued validator.
Clock-skew leeway
External IdP tokens are validated with zero leeway today (ValidateWithLeeway(expected, 0), multi_issuer_validator.go:217). Add ~30–60s leeway on the external path to tolerate clock skew between the external IdP and ToolHive. Keep zero leeway for the self-issued path (shares ToolHive's clock).
Sequencing constraint
The consent gate MUST ship in the same PR that first wires MultiIssuerTokenValidator into factory.go. Until that PR merges, the validator is inert; do not enable external tokens in production without the consent policy.
Out of scope (follow-up)
Error-code taxonomy: once external tokens are reachable, grant-level failures (untrusted issuer, wrong audience, expired) should map to invalid_grant per RFC 6749 §5.2 rather than the blanket invalid_request at handler.go:118. This needs typed/sentinel validator errors the handler can distinguish and touches both validators — track it as a separate PR to keep this one reviewable. Leave the existing TODO note (retargeted to the follow-up issue) in place.
Problem
The multi-issuer subject-token validator (
MultiIssuerTokenValidator,pkg/authserver/server/tokenexchange/multi_issuer_validator.go) can validate subject tokens from trusted external OIDC issuers, but it is not wired intofactory.go(which still buildsNewSelfIssuedTokenValidator), and external tokens carry noclient_idclaim, socheckDelegationConsent(handler.go:302) fails them closed. External token exchange is unusable until a consent model exists.Decision (from OAuth review)
"Trusted issuer + audience == us + valid signature/subject" authorizes ToolHive as a resource, not a specific client. Trusting such a token wholesale is a confused-deputy / CWE-863 risk.
Adopt a hybrid, fail-closed consent policy for external subject tokens:
may_act(RFC 8693 §4.4), it is authoritative:may_act.submust equal the authenticated actor (actorID= the ToolHive confidential client performing the exchange,handler.go:104). Same rule as the self-issued path.azp; operators may setappidfor Entra v1 orcidfor Okta) and require its value to appear in a per-issuer operator allowlist. This is what authorizes a specific external client's tokens to be exchanged here.The self-issued path (
client_idbinding) is unchanged.Scope — what ships in THIS PR
Per the sequencing constraint below, all of the following land together:
TrustedIssuerfields (multi_issuer_validator.go:49): addActorClaim stringandAllowedActors []string.ActorClaim/AllowedActors(see Config below).MultiIssuerTokenValidatorintofactory.goand delete theTODO(#5989)on the type. This is the step that makes external tokens reachable — it MUST be in the same diff as the consent policy.If the diff approaches the repo's 400-line limit, split the operator CRD/converter config surface into a follow-up PR and land the
pkg/authserverside (RunConfig/Config/server config + validator + factory + consent) first — but the validator must NOT be wired intofactory.goin any PR that lacks the consent policy.Design (recommended)
Only
MultiIssuerTokenValidatorholds per-issuer config (ActorClaim/AllowedActors), and only the handler knowsactorID. Split the policy accordingly:MultiIssuerTokenValidator.validateExternalToken(which already hasissuerConfig): when the token has nomay_act, resolve the actor claim fromValidatedClaims.Extra[issuerConfig.ActorClaim](default"azp"whenActorClaimis empty), require it to be a non-empty string present inissuerConfig.AllowedActors, and fail closed otherwise. EmptyAllowedActors⇒ reject every token (mirror the existing empty-AllowedAudiencesconvention). Whenmay_actis present, skip the allowlist check (may_act is the authoritative signal) and let the handler enforce it.ValidatedClaims.ClientIDfrom the external actor claim (namespace-collision footgun). If the handler needs the resolved value, add a new field (e.g.ExternalActor string) — but the recommended split needs no new field, since the allowlist decision is made in the validator andmay_actis enforced by the existing handler code.checkDelegationConsent(handler.go:302) stays responsible for themay_act↔actorIDcomparison (already implemented, applies to both paths) and the self-issuedclient_idbinding. External tokens that reach it have either passed the validator's allowlist or carrymay_act.Read the actor claim from
Extrabecauseazp/appid/cidare not well-known fields onValidatedClaims(seeassignClaim,validator.go:255).Config plumbing
Mirror how
AllowedAudiences/DelegationTokenLifespanalready flow (grep those names to trace the exact pattern):RunConfig(pkg/authserver/config.go:36, JSON/YAML tags).Config(pkg/authserver/config.go:587).server.AuthorizationServerConfig(read byfactory.go; already exposesAllowedAudiencesandPublicJWKS()).cmd/thv-operator/pkg/controllerutil/authserver.goandcmd/thv-operator/pkg/vmcpconfig/converter.go.Add a
TrustedIssuerslist (issuer URL, expected audience, JWKS URL,ActorClaim,AllowedActors) through those same layers, then havefactory.goconstructNewMultiIssuerTokenValidator(selfValidator, issuer, trustedIssuers)when trusted issuers are configured, else keep the self-issued validator.Clock-skew leeway
External IdP tokens are validated with zero leeway today (
ValidateWithLeeway(expected, 0),multi_issuer_validator.go:217). Add ~30–60s leeway on the external path to tolerate clock skew between the external IdP and ToolHive. Keep zero leeway for the self-issued path (shares ToolHive's clock).Sequencing constraint
The consent gate MUST ship in the same PR that first wires
MultiIssuerTokenValidatorintofactory.go. Until that PR merges, the validator is inert; do not enable external tokens in production without the consent policy.Out of scope (follow-up)
Error-code taxonomy: once external tokens are reachable, grant-level failures (untrusted issuer, wrong audience, expired) should map to
invalid_grantper RFC 6749 §5.2 rather than the blanketinvalid_requestathandler.go:118. This needs typed/sentinel validator errors the handler can distinguish and touches both validators — track it as a separate PR to keep this one reviewable. Leave the existingTODOnote (retargeted to the follow-up issue) in place.