-
Notifications
You must be signed in to change notification settings - Fork 2.8k
read cache control header to refresh JWK (fix #3301) #3446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
read cache control header to refresh JWK (fix #3301) #3446
Conversation
|
Deploy preview for hasura-docs ready! Built with commit 8aad990 |
31c6058 to
2a90bf3
Compare
|
Review app for commit 2a90bf3 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
|
Review app for commit 55b0444 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
read cache-control header first and then expires header to figure out when to refresh the JWK
55b0444 to
00511a1
Compare
|
Review app for commit 00511a1 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
|
Review app for commit b5e3c1a deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
|
Review app for commit c24a840 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
c24a840 to
5bbfd13
Compare
|
Review app for commit 5bbfd13 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
5bbfd13 to
61188e3
Compare
|
Review app for commit 61188e3 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
lexi-lambda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks. 😄
|
Review app for commit 8aad990 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com |
|
Review app https://hge-ci-pull-3446.herokuapp.com is deleted |
Description
There is an option to pass a JWK URL in the JWT configuration with Hasura. A lot of providers rotate their JWKs, and hence add a
Cache-ControlorExpiresheader to indicate when to next fetch the updated JWKs.Currently Hasura reads
Expiresheader to figure out when to refresh the JWKs. With this PR, it prioritizesCache-Controlheader overExpiresto refresh the JWKs.Affected components
Related Issues
#3301
Solution and Design
It checks for the presence of
Cache-Controlheader in the JWK response. If it is present, it tries to parse the time out of it. If absent, it will try to check forExpiresheader and try to parse a timestamp out of it.If it is successful in parsing time from any of the above headers, it will refresh the JWKs after that time. If not found, it defaults to 1min to refresh the JWKs.
This behaviour is same as current, except now
Cache-Controlheader is also read and takes priority overExpiresheader.Server checklist
Catalog upgrade
Does this PR change Hasura Catalog version?
Metadata
Does this PR add a new Metadata feature?
GraphQL
Breaking changes
Steps to test and verify
Run a server that can respond with a JWK and add a
Cache-Controlheader withmax-age=5seconds. Run Hasura, with thejwk_urlof this server inHASURA_GRAPHQL_JWT_SECRET. Observe, that Hasura makes request to this JWK server every 5 seconds.Limitations, known bugs & workarounds