From 8354aaa3ccd45fd2d15c8c64644a1e01ff0a803d Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Thu, 24 Mar 2022 06:41:11 -0400 Subject: [PATCH] Dynamic client registration does not generate client_secret for private_key_jwt Closes gh-657 --- ...entRegistrationAuthenticationProvider.java | 24 ++++++++++++------- ...gistrationAuthenticationProviderTests.java | 6 +++-- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java index 1e312619..9d9d1b59 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java @@ -48,12 +48,9 @@ import org.springframework.security.oauth2.core.oidc.OidcClientRegistration; import org.springframework.security.oauth2.jose.jws.MacAlgorithm; import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm; import org.springframework.security.oauth2.jwt.JwtEncoder; -import org.springframework.security.oauth2.server.authorization.token.DefaultOAuth2TokenContext; -import org.springframework.security.oauth2.server.authorization.token.JwtGenerator; import org.springframework.security.oauth2.server.authorization.OAuth2Authorization; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.OAuth2TokenContext; -import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator; import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken; import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; @@ -62,6 +59,9 @@ import org.springframework.security.oauth2.server.authorization.config.ProviderS import org.springframework.security.oauth2.server.authorization.config.TokenSettings; import org.springframework.security.oauth2.server.authorization.context.ProviderContext; import org.springframework.security.oauth2.server.authorization.context.ProviderContextHolder; +import org.springframework.security.oauth2.server.authorization.token.DefaultOAuth2TokenContext; +import org.springframework.security.oauth2.server.authorization.token.JwtGenerator; +import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator; import org.springframework.security.oauth2.server.resource.authentication.AbstractOAuth2TokenAuthenticationToken; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -305,9 +305,12 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe OidcClientRegistration.Builder builder = OidcClientRegistration.builder() .clientId(registeredClient.getClientId()) .clientIdIssuedAt(registeredClient.getClientIdIssuedAt()) - .clientSecret(registeredClient.getClientSecret()) .clientName(registeredClient.getClientName()); + if (registeredClient.getClientSecret() != null) { + builder.clientSecret(registeredClient.getClientSecret()); + } + builder.redirectUris(redirectUris -> redirectUris.addAll(registeredClient.getRedirectUris())); @@ -419,17 +422,22 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe RegisteredClient.Builder builder = RegisteredClient.withId(UUID.randomUUID().toString()) .clientId(CLIENT_ID_GENERATOR.generateKey()) .clientIdIssuedAt(Instant.now()) - .clientSecret(CLIENT_SECRET_GENERATOR.generateKey()) .clientName(clientRegistration.getClientName()); if (ClientAuthenticationMethod.CLIENT_SECRET_POST.getValue().equals(clientRegistration.getTokenEndpointAuthenticationMethod())) { - builder.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST); + builder + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST) + .clientSecret(CLIENT_SECRET_GENERATOR.generateKey()); } else if (ClientAuthenticationMethod.CLIENT_SECRET_JWT.getValue().equals(clientRegistration.getTokenEndpointAuthenticationMethod())) { - builder.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_JWT); + builder + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_JWT) + .clientSecret(CLIENT_SECRET_GENERATOR.generateKey()); } else if (ClientAuthenticationMethod.PRIVATE_KEY_JWT.getValue().equals(clientRegistration.getTokenEndpointAuthenticationMethod())) { builder.clientAuthenticationMethod(ClientAuthenticationMethod.PRIVATE_KEY_JWT); } else { - builder.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC); + builder + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .clientSecret(CLIENT_SECRET_GENERATOR.generateKey()); } builder.redirectUris(redirectUris -> diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java index 227d8f9a..6bc70cf2 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java @@ -48,11 +48,9 @@ import org.springframework.security.oauth2.jwt.JwtClaimsSet; import org.springframework.security.oauth2.jwt.JwtEncoder; import org.springframework.security.oauth2.jwt.TestJoseHeaders; import org.springframework.security.oauth2.jwt.TestJwtClaimsSets; -import org.springframework.security.oauth2.server.authorization.token.JwtGenerator; import org.springframework.security.oauth2.server.authorization.OAuth2Authorization; import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService; import org.springframework.security.oauth2.server.authorization.OAuth2TokenContext; -import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator; import org.springframework.security.oauth2.server.authorization.TestOAuth2Authorizations; import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; @@ -61,6 +59,8 @@ import org.springframework.security.oauth2.server.authorization.config.ClientSet import org.springframework.security.oauth2.server.authorization.config.ProviderSettings; import org.springframework.security.oauth2.server.authorization.context.ProviderContext; import org.springframework.security.oauth2.server.authorization.context.ProviderContextHolder; +import org.springframework.security.oauth2.server.authorization.token.JwtGenerator; +import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; import org.springframework.web.util.UriComponentsBuilder; @@ -472,6 +472,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { (OidcClientRegistrationAuthenticationToken) this.authenticationProvider.authenticate(authentication); assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm()) .isEqualTo(MacAlgorithm.HS256.getName()); + assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNotNull(); // @formatter:off builder @@ -482,6 +483,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { authenticationResult = (OidcClientRegistrationAuthenticationToken) this.authenticationProvider.authenticate(authentication); assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm()) .isEqualTo(SignatureAlgorithm.RS256.getName()); + assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNull(); } @Test