-
Notifications
You must be signed in to change notification settings - Fork 215
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
Add JWKFetcher::getFormatted() method and switch validator to use #369
Conversation
98f0e95
to
3498808
Compare
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.
LGTM! Just had more of a question around the getFormatted
naming but didn't want to hold up the PR.
src/Helpers/JWKFetcher.php
Outdated
* | ||
* @return array | ||
*/ | ||
public function getFormatted($jwks_url) |
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.
Just a bit confused by the name. What does it mean to format the JWKS for JWT::decode()
?
The naming points to an internal transform process, just wondering if the developer needs to understand that detail. Could this be called getKeysFromJwks
or something similar?
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.
JWT::decode()
is an external library that validates JWT signatures (and a few other things). It takes public key signatures in the form of $keys[$kid] = $x5c_pem
.
Good point about the method name, that is a little vague. I don't think we need to say "from JWKS" since we're in the JWKS
class but I'll rename.
3498808
to
1c29dba
Compare
1c29dba
to
8718eb6
Compare
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Changes
JWKFetcher::getFormatted()
method to pull entire JWKS instead of specific keys. This makes for more clear caching and allows us to deprecate several unnecessary methods in this class.MockApi
class used in test suites to allow new mocked API responses to be added more easily.Testing