Skip to content

fix(jwt): expire the JWKS host cache after poll_interval - #210

Open
ralph-sandstone wants to merge 1 commit into
grafbase:mainfrom
ralph-sandstone:jwks-cache-ttl
Open

ralph-sandstone wants to merge 1 commit into
grafbase:mainfrom
ralph-sandstone:jwks-cache-ttl

Conversation

@ralph-sandstone

Copy link
Copy Markdown

Problem

Jwt::new builds the JWKS host cache as

Cache::builder("jwks", 1).timeout(config.poll_interval).build()

CacheBuilder::timeout is the wait for a concurrent fetch, not the entry lifetime, and time_to_live defaults to None. So the JWKS bytes fetched at the first request are served for the life of the process. decoder() does re-enter the cache every poll_interval, but it gets the same bytes back every time, so poll_interval never triggers a real refetch.

When the identity provider rotates its signing key, every token carrying the new kid is rejected with a 401 until the gateway restarts. We hit this in production with Stytch (rotation roughly every six months, both keys published for one month, 5-minute tokens): 24 hours of 401s for every session, with no log line from the extension. Restarting the pods fixed it, which is what pointed at the frozen cache.

Fix

Set time_to_live(Some(config.poll_interval)) on the cache. The README already documents poll_interval as "How long the JWKS will be cached", so this makes the code do what the docs say.

The .timeout(config.poll_interval) override is dropped rather than kept: a 60-second wait on the cache lock is longer than any JWKS fetch should take, and the SDK default of 5 seconds is a better bound for a concurrent fetch.

Changelog entry added under Unreleased. I have not bumped extension.toml; happy to if you prefer that in the PR rather than via the extension-version-bump-needed label.

Verification

  • cargo fmt --check, cargo clippy -p jwt --locked, cargo check -p jwt --locked --target wasm32-wasip2 all clean on 1.89.0.
  • Behaviour checked against a forked build running under Gateway 0.53.2: with a local JWKS server, rotating the key set after poll_interval elapses now flips a token with the new kid from 401 to 200 on the next request; before this change it stayed 401 until restart.

Follow-up, not in this PR

A token whose kid is not in the cached set could trigger one forced refetch instead of waiting out poll_interval, and a rejection could log the kid and the cached kids at warn level. Both would have made this failure visible in minutes. I can open a second PR for either if there is interest.

The cache was built with .timeout(poll_interval) and no time_to_live. timeout is
the wait for a concurrent fetch, not the entry lifetime, and the SDK's default TTL
is None, so the JWKS fetched at startup was served for the life of the process.
After a provider key rotation every token signed by the new key was rejected with
a 401 until the gateway restarted.

Set time_to_live(poll_interval), which is what the README already documents the
setting to mean, and drop the timeout override so the 5s SDK default applies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant