Skip to content

Consolidate duplicate JWKS fetch and cache machinery #6319

Description

@jhrozek

Description

ToolHive fetches and caches remote JWKS in two places, built independently on
the same libraries and converging on the same design. Neither knows about the
other.

pkg/auth/token.go — the inbound TokenValidator used by every proxy and CLI
path. Holds a jwk.Cache (jwx v3) created over
httprc.NewClient(httprc.WithHTTPClient(...)), registers the JWKS URL in
ensureJWKSRegistered (:698), and resolves keys in getKeyFromJWKS (:896) via
Lookup then LookupKeyID. Its HTTP client comes from
networking.NewHttpClientBuilder(), gated by AllowPrivateIP and
InsecureAllowHTTP. Discovery retries with cenkalti/backoff.

pkg/authserver/server/tokenexchange/multi_issuer_validator.go — the
token-exchange subject-token validator. Holds a jwk.Cache per issuer
(externalIssuerConfig.jwksCache, :212), registered in registerOrRefresh
(:631) and reached through ensureRegistered (:589) and lookupJWKS (:700).
Adds a rate-limited refreshOnUnknownKid (:757), a fetch-failure backoff,
discoverJWKSURL (:803), ValidateJWKSURL (:860), a limitedBodyTransport
response cap, and per-issuer InsecureAllowHTTP / AllowPrivateIPs.

Same problem, same libraries, two solutions. The second is the more complete
one, and its extra machinery — the response cap, the rate-limited kid refresh,
per-issuer rather than global HTTP and private-IP flags — is exactly the kind
of hardening the first would want and does not have.

Scope

Extract one shared JWKS fetch-and-cache type and move both call sites onto it.

The API surface both implementations already imply:

  • resolve a key set for an issuer or JWKS URL, with caching
  • look up by kid, refreshing once on a miss, rate-limited
  • serve the last known good set when a fetch fails
  • an SSRF-guarded HTTP client from networking, with a bounded response body
  • HTTP and private-IP relaxations declared per issuer, never globally

Take the extraction from the two implementations on main. Do not regress
anything the token-exchange validator already does.

Explicitly out of scope

pkg/authserver/upstream/oidc.go is a third mechanism: it uses
coreos/go-oidc, whose IDTokenVerifier carries its own RemoteKeySet with
its own cache and refresh policy. It already receives an SSRF-guarded client
through oidc.ClientContext (:200). Replacing it means reimplementing ID token
verification, nonce handling and the rest of the verifier, which is a much
larger change with real risk for no clear gain. Leave it alone and say so in
the code.

Prior art, and what not to lift

The unpushed xaa-spike-1 branch has pkg/oauthproto/jwks.go (commit
6fc952750), a JWKSFetcher with per-issuer caching, refresh on kid miss,
HTTPS enforcement, a dial-time private-address guard and a stale-on-error path.

Do not cherry-pick it. It was written on 2026-08-04, before #6149 rebuilt
the token-exchange validator, and it is hand-rolled sync.Mutex caching over
go-jose where both live implementations use jwx jwk.Cache + httprc.
Adopting it converts working code backwards: it would drop httprc's background
refresh, the response cap, and the validator's per-issuer flags in favour of a
single global one.

What it did get right is the API shape — resolve by issuer, look up by kid,
refresh once on miss, serve stale on error. Use it as a sketch of the interface
and nothing else.

Acceptance criteria

  • One shared type, both call sites converted, no third implementation left.
  • Per-issuer InsecureAllowHTTP and AllowPrivateIPs survive; neither becomes
    a process-wide switch.
  • The response-body cap applies to both call sites.
  • Refresh on unknown kid stays rate-limited, so an attacker supplying random
    kid values cannot drive unbounded fetches.
  • A fetch failure serves the last known good key set rather than failing all
    validation.
  • Key rotation still works end to end for both call sites.
  • No key material, token, or assertion is logged.
  • Existing token validation behaviour is unchanged; this is a refactor, not a
    behaviour change.

Why separately

pkg/auth/token.go is on the request path for every proxied workload, so this
wants its own review and its own test story rather than riding along inside a
feature branch.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    authenticationenhancementNew feature or requestgoPull requests that update go codeneeds-triageIssue needs initial triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions