Skip to content
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.

Sign up for GitHub

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

Jump to bottom

server regression: JWK is rejected due to Cache-Control header parsing #3655

Closed
lexi-lambda opened this issue Jan 7, 2020 · 3 comments
Closed

server regression: JWK is rejected due to Cache-Control header parsing #3655

lexi-lambda opened this issue Jan 7, 2020 · 3 comments
Assignees
@ecthiender
Labels
c/server Related to server k/bug Something isn't working p/urgent Immediate action required

Comments

@lexi-lambda
Copy link
Contributor

As reported on Discord, the changes in #3446 do not always properly parse the value of the Cache-Control header, causing the server to fail to start. For example, the response returned by Google Cloud IAM includes a header of the shape

cache-control: public, max-age=18341, must-revalidate, no-transform

and this is rejected by graphql-engine on startup, causing it to exit with the following somewhat unhelpful message:

Failed parsing Cache-Control header from JWK response. Could not find max-age or s-maxage

It would be nice if we could use some existing, off-the-shelf parser to parse this information, not roll our own.
@lexi-lambda lexi-lambda added k/bug Something isn't working c/server Related to server p/urgent Immediate action required labels Jan 7, 2020
@lexi-lambda lexi-lambda mentioned this issue Jan 9, 2020
Closed
@ecthiender ecthiender mentioned this issue Jan 10, 2020
Merged
26 tasks
@plouh
Copy link

plouh commented Jan 16, 2020

The problem still persists when server replies with cache-control: no-cache -header

@lexi-lambda lexi-lambda reopened this Jan 17, 2020
@ecthiender
Copy link
Member

@plouh thanks for reporting and sorry for the inconvenience! My bad I should have tested that case. I will try to push a fix by today.

@plouh
Copy link

plouh commented Jan 17, 2020

No problem at all, it's been a great pleasure to use hasura. We use keycloak as our identity provider, if it helps to reproduce the issue.

ecthiender added a commit to ecthiender/graphql-engine that referenced this issue Jan 27, 2020
@ecthiender
fix JWK cache-control header regression (fix hasura#3655)
dd7a9c2 
- fix when header parsing fails on startup, do not exit
- change qualified import of aeson in Auth.JWT from A to J to be consistent with the rest of the
  codebase
@ecthiender ecthiender mentioned this issue Jan 27, 2020
Merged
7 tasks
@0x777 0x777 closed this as completed in c6e34ba Feb 5, 2020
@ecthiender ecthiender mentioned this issue Feb 5, 2020
Closed
polRk pushed a commit to polRk/graphql-engine that referenced this issue Feb 12, 2020
@ecthiender @polRk
fix cache-control header parsing for JWK (fix hasura#3655) (hasura#3676)
879b365 
write a proper parser according to the RFC
https://tools.ietf.org/html/rfc7234#section-5.2
polRk pushed a commit to polRk/graphql-engine that referenced this issue Feb 12, 2020
@ecthiender @polRk
fix parsing JWK expiry time from headers on startup (fix hasura#3655) (…
08f2bec 
…hasura#3779)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ecthiender ecthiender

Labels
c/server Related to server k/bug Something isn't working p/urgent Immediate action required
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@plouh @ecthiender @lexi-lambda