Use configured ID Token signature algorithm #787
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
jgrandja
changed the title
jgrandja
added
type: enhancement
| @@ -125,7 +125,7 @@ public Jwt generate(OAuth2TokenContext context) { | |||
| } | |||
| // @formatter:on | |||
|
|
|||
| JwsHeader.Builder headersBuilder = JwsHeader.with(SignatureAlgorithm.RS256); | |||
| JwsHeader.Builder headersBuilder = JwsHeader.with(registeredClient.getTokenSettings().getIdTokenSignatureAlgorithm()); | |||
There was a problem hiding this comment.
registeredClient.getTokenSettings().getIdTokenSignatureAlgorithm() should only be used when minting an ID Token. The existing default should be preserved for access tokens.
jgrandja
changed the title
There was a problem hiding this comment.
Thanks for the updates @721806280. Please see review comments.
Also, please rebase off of 0.4.x and squash the commits on next push. Thanks.
| @@ -15,10 +15,6 @@ | |||
| */ | |||
| package org.springframework.security.oauth2.server.authorization.token; | |||
|
|
|||
| import java.time.Instant; | |||
There was a problem hiding this comment.
Please revert this change. Only apply changes to required code.
| @@ -30,29 +26,29 @@ | |||
| import org.springframework.security.oauth2.core.oidc.OidcIdToken; | |||
| import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames; | |||
| import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm; | |||
| import org.springframework.security.oauth2.jwt.JwsHeader; | |||
There was a problem hiding this comment.
Please revert this change. Only apply changes to required code.
| /** | ||
| * An {@link OAuth2TokenGenerator} that generates a {@link Jwt} | ||
| * used for an {@link OAuth2AccessToken} or {@link OidcIdToken}. | ||
| * | ||
| * @author Joe Grandja | ||
| * @since 0.2.3 |
There was a problem hiding this comment.
Please revert this change. Only apply changes to required code.
| } else { | ||
| expiresAt = issuedAt.plus(registeredClient.getTokenSettings().getAccessTokenTimeToLive()); | ||
| headersBuilder = JwsHeader.with(registeredClient.getTokenSettings().getIdTokenSignatureAlgorithm()); |
There was a problem hiding this comment.
This line should be under the if statement -> if (OidcParameterNames.ID_TOKEN.equals(context.getTokenType().getValue()))
Also, please add a test to ensure the defaults are set as expected.
There was a problem hiding this comment.
Thanks for the updates @721806280. Please see the one review comment.
In addition to the one requested change, please add a test in JwtGeneratorTests to ensure the defaults are set as expected. Also, please rebase this PR off of 0.4.x branch and squash the commits.
Thanks.
| @@ -89,9 +89,11 @@ public Jwt generate(OAuth2TokenContext context) { | |||
|
|
|||
| Instant issuedAt = Instant.now(); | |||
| Instant expiresAt; | |||
| JwsHeader.Builder headersBuilder = JwsHeader.with(SignatureAlgorithm.RS256); | |||
There was a problem hiding this comment.
To prevent JwsHeader.Builder from being instantiated twice, please add JwsHeader.with(SignatureAlgorithm.RS256) under the else
|
Thanks for the PR @721806280. This is now merged. FYI, I added a polish commit with some minor updates. |
The following commits are merged using the default merge strategy. 70d433a Update ref-doc with OAuth2Authorization.getAuthorizedScopes() 0994a1e Allow customizing OIDC Provider Configuration Response 8043b8c Allow customizing Authorization Server Metadata Response 4466cbe Use configured ID Token signature algorithm 502fa24 Polish gh-787 07d69cb Validate client secret not expired 2cc603c Improve configurability for AuthenticationConverter and AuthenticationProvider 1db0599 Make OAuth2AuthenticationContext an interface c326b1a Remove OAuth2AuthenticationValidator
The following commits are merged using the default merge strategy. adfb603 Update ref-doc with OAuth2Authorization.getAuthorizedScopes() 3f1d4c8 Allow customizing OIDC Provider Configuration Response 2be31fb Allow customizing Authorization Server Metadata Response bf1b85b Use configured ID Token signature algorithm 98c4d1c Polish spring-projectsgh-787 b4a6ba8 Validate client secret not expired 525c084 Improve configurability for AuthenticationConverter and AuthenticationProvider f545816 Make OAuth2AuthenticationContext an interface 333837d Remove OAuth2AuthenticationValidator
No description provided.