Consolidate duplicate JWKS fetch and cache machinery
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
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
InsecureAllowHTTPandAllowPrivateIPssurvive; neither becomes
a process-wide switch. - The response-body cap applies to both call sites.
- Refresh on unknown
kidstays rate-limited, so an attacker supplying random
kidvalues 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
- #6149 rebuilt the token-exchange validator's JWKS handling; this consolidates
what that left duplicated. - Not a dependency of the SPIFFE client-auth epic (#6199). SPIFFE JWT-SVID
authorities come from the trust bundle, not from a JWKS URI, so
jwtsvid.ParseAndValidatemakes no HTTP request. #6203's stated dependency on
a shared JWKS fetcher is mistaken and is being corrected separately.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading pkg/auth/token.go at ensureJWKSRegistered and getKeyFromJWKS, then compare pkg/authserver/server/tokenexchange/multi_issuer_validator.go at ensureRegistered, registerOrRefresh, lookupJWKS, refreshOnUnknownKid, and discoverJWKSURL. Trace the existing validation tests and verify that both call sites use one shared implementation while preserving per-issuer protections, bounded responses, rate-limited refresh, stale-on-error behavior, key rotation, and unchanged validation behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authentication, backend, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100