Repository navigation
fix(jwt): expire the JWKS host cache after poll_interval - #210
Open
ralph-sandstone wants to merge 1 commit into
Open
ralph-sandstone wants to merge 1 commit into
ralph-sandstone wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Jwt::newbuilds the JWKS host cache asCacheBuilder::timeoutis the wait for a concurrent fetch, not the entry lifetime, andtime_to_livedefaults toNone. So the JWKS bytes fetched at the first request are served for the life of the process.decoder()does re-enter the cache everypoll_interval, but it gets the same bytes back every time, sopoll_intervalnever triggers a real refetch.When the identity provider rotates its signing key, every token carrying the new
kidis 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 documentspoll_intervalas "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 theextension-version-bump-neededlabel.Verification
cargo fmt --check,cargo clippy -p jwt --locked,cargo check -p jwt --locked --target wasm32-wasip2all clean on 1.89.0.poll_intervalelapses now flips a token with the newkidfrom 401 to 200 on the next request; before this change it stayed 401 until restart.Follow-up, not in this PR
A token whose
kidis not in the cached set could trigger one forced refetch instead of waiting outpoll_interval, and a rejection could log thekidand 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.