这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ecthiender
Copy link
Contributor

@ecthiender ecthiender commented Nov 29, 2019

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-Control or Expires header to indicate when to next fetch the updated JWKs.

Currently Hasura reads Expires header to figure out when to refresh the JWKs. With this PR, it prioritizes Cache-Control header over Expires to refresh the JWKs.

Affected components

  • Server
  • Tests

Related Issues

#3301

Solution and Design

It checks for the presence of Cache-Control header in the JWK response. If it is present, it tries to parse the time out of it. If absent, it will try to check for Expires header 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-Control header is also read and takes priority over Expires header.

Server checklist

Catalog upgrade

Does this PR change Hasura Catalog version?

  • No

Metadata

Does this PR add a new Metadata feature?

  • No

GraphQL

  • No new GraphQL schema is generated

Breaking changes

  • No Breaking changes

Steps to test and verify

Run a server that can respond with a JWK and add a Cache-Control header with max-age=5 seconds. Run Hasura, with the jwk_url of this server in HASURA_GRAPHQL_JWT_SECRET. Observe, that Hasura makes request to this JWK server every 5 seconds.

Limitations, known bugs & workarounds

@ecthiender ecthiender added the c/server Related to server label Nov 29, 2019
@netlify
Copy link

netlify bot commented Nov 29, 2019

Deploy preview for hasura-docs ready!

Built with commit 8aad990

https://deploy-preview-3446--hasura-docs.netlify.com

@ecthiender ecthiender force-pushed the fix-3301-jwk-cache-control-header branch from 31c6058 to 2a90bf3 Compare November 29, 2019 08:04
@hasura-bot
Copy link
Contributor

Review app for commit 2a90bf3 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-2a90bf39

@hasura-bot
Copy link
Contributor

Review app for commit 55b0444 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-55b0444b

@ecthiender ecthiender force-pushed the fix-3301-jwk-cache-control-header branch from 55b0444 to 00511a1 Compare December 2, 2019 13:01
@hasura-bot
Copy link
Contributor

Review app for commit 00511a1 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-00511a1f

@ecthiender ecthiender added the s/wip Status: This issue is a work in progress label Dec 2, 2019
@hasura-bot
Copy link
Contributor

Review app for commit b5e3c1a deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-b5e3c1a6

@hasura-bot
Copy link
Contributor

Review app for commit c24a840 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-c24a840f

@ecthiender ecthiender force-pushed the fix-3301-jwk-cache-control-header branch from c24a840 to 5bbfd13 Compare December 3, 2019 07:43
@hasura-bot
Copy link
Contributor

Review app for commit 5bbfd13 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-5bbfd134

@ecthiender ecthiender force-pushed the fix-3301-jwk-cache-control-header branch from 5bbfd13 to 61188e3 Compare December 3, 2019 08:27
@hasura-bot
Copy link
Contributor

Review app for commit 61188e3 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-61188e3f

@ecthiender ecthiender marked this pull request as ready for review December 3, 2019 09:28
Copy link
Contributor

@lexi-lambda lexi-lambda left a 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. 😄

@hasura-bot
Copy link
Contributor

Review app for commit 8aad990 deployed to Heroku: https://hge-ci-pull-3446.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3446-8aad990f

@lexi-lambda lexi-lambda merged commit afd6f30 into hasura:master Dec 3, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-3446.herokuapp.com is deleted

@lexi-lambda lexi-lambda removed the s/wip Status: This issue is a work in progress label Dec 3, 2019
polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/server Related to server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants