-
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
Add Jwt Client Authentication support #9520
Add Jwt Client Authentication support #9520
Conversation
480c576
to
943f1da
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.
I've left feedback inline
...springframework/security/oauth2/client/endpoint/NimbusJwtClientAuthenticationCustomizer.java
Outdated
Show resolved
Hide resolved
...springframework/security/oauth2/client/endpoint/NimbusJwtClientAuthenticationCustomizer.java
Outdated
Show resolved
Hide resolved
...ramework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityConverter.java
Outdated
Show resolved
Hide resolved
...ramework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityConverter.java
Outdated
Show resolved
Hide resolved
...ramework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityConverter.java
Outdated
Show resolved
Hide resolved
...nt/src/main/java/org/springframework/security/oauth2/client/endpoint/JwsHeaderConverter.java
Outdated
Show resolved
Hide resolved
...th2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/JoseHeader.java
Show resolved
Hide resolved
...nt/src/main/java/org/springframework/security/oauth2/client/endpoint/JwsHeaderConverter.java
Outdated
Show resolved
Hide resolved
...ient/src/main/java/org/springframework/security/oauth2/client/endpoint/NimbusJwsEncoder.java
Outdated
Show resolved
Hide resolved
...mework/security/oauth2/client/endpoint/NimbusJwtClientAuthenticationParametersConverter.java
Show resolved
Hide resolved
@rwinch I have addressed all feedback. Ready for next review. |
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. I think it is getting very close. I have responded to some of your replies.
...th2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/JoseHeader.java
Show resolved
Hide resolved
...th2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/JoseHeader.java
Show resolved
Hide resolved
...mework/security/oauth2/client/endpoint/NimbusJwtClientAuthenticationParametersConverter.java
Show resolved
Hide resolved
...ingframework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityUtils.java
Show resolved
Hide resolved
* | ||
* @param <T> the type of {@link AbstractOAuth2AuthorizationGrantRequest} | ||
*/ | ||
public static final class JwtClientAuthenticationContext<T extends AbstractOAuth2AuthorizationGrantRequest> { |
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.
Is there a reason you went with creating JwtClientAuthenticationContext
vs two converters like was done with the headers and claims in AbstractOAuth2AuthorizationGrantRequestEntityConverter
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.
See comment
Thanks for all the feedback @rwinch ! I applied the last bit of feedback and just merged to master. |
@jgrandja do you have an working example by configuring using application.yml, trying to get this working for apple but no luck so far. Also went through the test but still cant able to get it working |
Please look at this test as it demonstrates the explicit code configuration. |
@jgrandja It is still hard to understand how this can be done in a real world that is not a Test. Do you have an working example |
I will try to find time over the next week or so to put a sample together. |
@sabareeshkkanan This branch has a working sample that uses Spring Authorization Server. |
Add JWT client authentication support for
private_key_jwt
andclient_secret_jwt
.Related specifications
RFC 7521
4.2. Using Assertions for Client Authentication
RFC 7523
2.2. Using JWTs for Client Authentication
OpenID Connect Core 1.0
9. Client Authentication
The best place to start to see how to configure and customize JWT client authentication support are the following tests:
DefaultAuthorizationCodeTokenResponseClientTests
:getTokenResponseWhenAuthenticationPrivateKeyJwtThenFormParametersAreSent()
getTokenResponseWhenAuthenticationClientSecretJwtThenFormParametersAreSent()
DefaultClientCredentialsTokenResponseClientTests
:getTokenResponseWhenAuthenticationPrivateKeyJwtThenFormParametersAreSent()
getTokenResponseWhenAuthenticationClientSecretJwtThenFormParametersAreSent()
Closes gh-8175