Prevent log spamming when using a non-cached JWKS

[lovasoa]

Multiple auth providers (keycloak, firebase, ...) do not allow caching on their JWK endpoints. In this case, Hasura requests the endpoint every second. This change avoids spamming the logs when this happens.

Description

Changelog

• [ ] CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

• [x] Server
• [ ] Console
• [ ] CLI
• [ ] Docs
• [ ] Community Content
• [ ] Build System
• [ ] Tests
• [ ] Other (list it)

Related Issues

https://github.com/hasura/graphql-engine/issues/8299

Limitations, known bugs & workarounds

Does this PR add a new Metadata feature?

• [x] No
• [ ] Yes
  • Does run_sql auto manages the new metadata through schema diffing?
    • [ ] Yes
    • [ ] Not required
  • Does run_sql auto manages the definitions of metadata on renaming?
    • [ ] Yes
    • [ ] Not required
  • Does export_metadata/replace_metadata supports the new metadata added?
    • [ ] Yes
    • [ ] Not required

GraphQL

• [ ] No new GraphQL schema is generated
• [ ] New GraphQL schema is being generated:
  • [ ] New types and typenames are correlated

Breaking changes

• [x] No Breaking changes

[hasura-bot]

Beep boop! :robot:

Hey @lovasoa, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible.

Stay awesome! :sunglasses:

[CLAassistant]

All committers have signed the CLA.

[SamirTalwar]

Hi @lovasoa, thanks for the patch!

This seems benign but there are definitely use cases where users might want to see these logs at INFO level too. We think it might be more prudent to make this configurable, so that you can customize it for yourself as needed, as outlined in #8611.

I see you've already spotted this issue. Would you be interested in submitting a patch along those lines?

[lovasoa]

Hi, and thanks for the response :) Sorry, but I don't have the bandwidth for that now. Are you sure it makes sense to log this as INFO by default for everyone ? I agree it should be made configurable, but by default, given that hasura will refresh keys every second for most jwks providers, I think it doesn't make sense to display something on every refresh.

[SamirTalwar]

It really depends on the caching proposed by the auth provider. I took a look at a few (Auth0, AWS Cognito, Firebase) and they all seem to encourage some caching.

The Firebase documentation I found suggests that keys are cached, and uses a central, public keyset. I hit the URL, https://www.googleapis.com/robot/v1/metadata/x509/[email protected], and got the following headers back:

cache-control: public, max-age=24087, must-revalidate, no-transform
date: Fri, 05 Aug 2022 13:40:48 GMT
expires: Fri, 05 Aug 2022 20:22:15 GMT

Can you explain your situation in greater detail so I can understand why it's different?

[lovasoa]

Hey @SamirTalwar , I'm sorry I missed your message when you posted it. Thank you for investigating on this ! Our auth provider is the open-source Keycloak. It serves JWKs with the following headers :

< HTTP/2 200 
< date: Fri, 28 Oct 2022 13:48:49 GMT
< content-type: application/json
< content-length: 1466
< cache-control: no-cache
< x-xss-protection: 1; mode=block
< x-frame-options: SAMEORIGIN
< referrer-policy: no-referrer
< strict-transport-security: max-age=15724800; includeSubDomains
< x-content-type-options: nosniff
< x-forwarded-for: keycloak.***.com
< x-forwarded-proto: https

Unfortunately, that does not seem to be configurable, and we end up with very large amounts of logs from hasura, which are expensive to store and make debugging more difficult.

[SamirTalwar]

That's interesting. My first thought was that we should probably let the auth provider specify the expiry, but a default of 1 second seems very arbitrary. I'll bring this up and see if we can make that configurable.

[lovasoa]

Thank you @SamirTalwar !

[pranshi06]

Hi @lovasoa , thanks for taking the initiative to resolve this issue. We have fixed it by making jwk-refresh-log type configurable as outlined in #8611 via fd6aa0a2662632c867fcf16c1fca4e1636cd5cf2 and 7b7845f697d1b0014b75533e03a0958e5c41530b.

[hasura-bot]

Beep boop! :robot:

Hey @lovasoa!

Sorry that your PR wasn’t merged.

Do take a look at any of the other open issues to see if you’d like to take something up! We’re around on Discord if you have any questions :smile:

[lovasoa]

Great, thank you @pranshi06 !