-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Provide possibility to use custom cache to store JWK Set #8332
Conversation
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.
Thanks, @20fps! I've left some feedback inline.
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Outdated
Show resolved
Hide resolved
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Outdated
Show resolved
Hide resolved
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Outdated
Show resolved
Hide resolved
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Show resolved
Hide resolved
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.
Thanks, @20fps! I've left a bit more feedback inline.
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Outdated
Show resolved
Hide resolved
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Outdated
Show resolved
Hide resolved
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.
I've left just one more comment.
In light of that comment, would you also add a test that confirms that a JwtException
, not a BadJwtException
is thrown when using a faulty RestOperations
in conjunction with a Cache
?
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java
Outdated
Show resolved
Hide resolved
In preparation for merging, @20fps, would you please squash your commits and format the remaining one to look something like:
|
np, done |
- Follow convention to prefix member variable references with "this." - Reduce stack trace when IOException is thrown - Name tests to follow conventions Issue gh-8332
Update according to the changes requested here #7639 by @jzheaux to have a possibility to use spring Cache to store JWK Set.