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.

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

feat: AccessToken respecting certificate expiration #463

Closed
ynikitin-etsy opened this issue Jul 13, 2023 · 2 comments
Closed

feat: AccessToken respecting certificate expiration #463

ynikitin-etsy opened this issue Jul 13, 2023 · 2 comments

Comments

@ynikitin-etsy
Copy link

ynikitin-etsy commented Jul 13, 2023

Is your feature request related to a problem? Please describe.

I am using AccessToken to validate Google Service Account JWTs and that's it. When given the cache, AccessToken caches the public certificates so not to fetch them from Google very time. Currently, AccessToken.php expires the fetched public certificates after one hour. However, Google and other providers often rotate their keys. Thus, it might be possible to cache public certs that are stale for almost an hour (~59min, if fetched right before rotation) given specific timing requirements.

Describe the solution you'd like

Google recommends respecting the Cache-Control header, which it seems this library currently doesn't.

https://developers.google.com/identity/gsi/web/guides/verify-google-id-token

Describe alternatives you've considered

It might be possible to catch the UnexpectedValueException with message "kid" invalid, unable to lookup correct key and clear the cache so to force a refetch on revalidation. But, this would happen if you are given a JWT that's signed by another provider public keys' and you would refetch certs all the time.

Additional context

Not sure if my understanding is correct here, or there is a better way to make sure certs are always fresh and fetched at appropriate times. Are PR's welcomed for this?

@ynikitin-etsy ynikitin-etsy changed the title AccessToken respecting certificate expiration feat: AccessToken respecting certificate expiration Jul 13, 2023
@bshaffer
Copy link
Contributor

bshaffer commented Sep 8, 2023

Hello! Thank you for submitting this issue. You are correct that the Cache-Control header which comes back with the response for the certs is currently not being used. I've submitted a PR (#479) to add this behavior. Please take a look and leave a review if you'd like. And to answer your questions, PR's are ALWAYS welcome!

@ynikitin-etsy
Copy link
Author

@bshaffer Thank you, this is amazing! I will review asap!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants