Fix client secret encoding when client dynamically registered

Closes gh-1056

Author: uc4w6c

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.config.annotation.ObjectPostProcessor;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
 import org.springframework.security.oauth2.core.OAuth2Error;
 import org.springframework.security.oauth2.server.authorization.oidc.OidcClientRegistration;
@@ -221,6 +222,10 @@ private static List<AuthenticationProvider> createDefaultAuthenticationProviders
 						OAuth2ConfigurerUtils.getRegisteredClientRepository(httpSecurity),
 						OAuth2ConfigurerUtils.getAuthorizationService(httpSecurity),
 						OAuth2ConfigurerUtils.getTokenGenerator(httpSecurity));
+		PasswordEncoder passwordEncoder = OAuth2ConfigurerUtils.getOptionalBean(httpSecurity, PasswordEncoder.class);
+		if (passwordEncoder != null) {
+			oidcClientRegistrationAuthenticationProvider.setPasswordEncoder(passwordEncoder);
+		}
 		authenticationProviders.add(oidcClientRegistrationAuthenticationProvider);
 
 		OidcClientConfigurationAuthenticationProvider oidcClientConfigurationAuthenticationProvider =

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,8 +34,10 @@
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.crypto.factory.PasswordEncoderFactories;
 import org.springframework.security.crypto.keygen.Base64StringKeyGenerator;
 import org.springframework.security.crypto.keygen.StringKeyGenerator;
+import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClaimAccessor;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
@@ -79,6 +81,7 @@
  * @see OAuth2TokenGenerator
  * @see OidcClientRegistrationAuthenticationToken
  * @see OidcClientConfigurationAuthenticationProvider
+ * @see PasswordEncoder
  * @see <a href="https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration">3. Client Registration Endpoint</a>
  */
 public final class OidcClientRegistrationAuthenticationProvider implements AuthenticationProvider {
@@ -91,6 +94,8 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe
 	private final Converter<RegisteredClient, OidcClientRegistration> clientRegistrationConverter;
 	private Converter<OidcClientRegistration, RegisteredClient> registeredClientConverter;
 
+	private PasswordEncoder passwordEncoder;
+
 	/**
 	 * Constructs an {@code OidcClientRegistrationAuthenticationProvider} using the provided parameters.
 	 *
@@ -109,6 +114,21 @@ public OidcClientRegistrationAuthenticationProvider(RegisteredClientRepository r
 		this.tokenGenerator = tokenGenerator;
 		this.clientRegistrationConverter = new RegisteredClientOidcClientRegistrationConverter();
 		this.registeredClientConverter = new OidcClientRegistrationRegisteredClientConverter();
+		this.passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder();
+	}
+
+	/**
+	 * Sets the {@link PasswordEncoder} used to encode the clientSecret
+	 * the {@link RegisteredClient#getClientSecret() client secret}.
+	 * If not set, the client secret will be encoded using
+	 * {@link PasswordEncoderFactories#createDelegatingPasswordEncoder()}.
+	 *
+	 * @param passwordEncoder the {@link PasswordEncoder} used to encode the clientSecret
+	 * @since 1.1.0
+	 */
+	public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
+		Assert.notNull(passwordEncoder, "passwordEncoder cannot be null");
+		this.passwordEncoder = passwordEncoder;
 	}
 
 	@Override
@@ -183,7 +203,21 @@ private OidcClientRegistrationAuthenticationToken registerClient(OidcClientRegis
 		}
 
 		RegisteredClient registeredClient = this.registeredClientConverter.convert(clientRegistrationAuthentication.getClientRegistration());
-		this.registeredClientRepository.save(registeredClient);
+
+		// When secret exists, copy RegisteredClient and encode only secret
+		String rawClientSecret = registeredClient.getClientSecret();
+		String clientSecret = null;
+		RegisteredClient saveRegisteredClient = null;
+		if (rawClientSecret != null) {
+			clientSecret = passwordEncoder.encode(rawClientSecret);
+			saveRegisteredClient = RegisteredClient.from(registeredClient)
+					.clientSecret(clientSecret)
+					.build();
+		} else {
+			saveRegisteredClient = registeredClient;
+		}
+
+		this.registeredClientRepository.save(saveRegisteredClient);
 
 		if (this.logger.isTraceEnabled()) {
 			this.logger.trace("Saved registered client");

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -56,7 +56,7 @@
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configurers.oauth2.server.resource.OAuth2ResourceServerConfigurer;
-import org.springframework.security.crypto.password.NoOpPasswordEncoder;
+import org.springframework.security.crypto.factory.PasswordEncoderFactories;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
@@ -111,6 +111,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
+import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.jwt;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
@@ -289,7 +290,6 @@ public void requestWhenClientConfigurationRequestAuthorizedThenClientRegistratio
 
 		assertThat(clientConfigurationResponse.getClientId()).isEqualTo(clientRegistrationResponse.getClientId());
 		assertThat(clientConfigurationResponse.getClientIdIssuedAt()).isEqualTo(clientRegistrationResponse.getClientIdIssuedAt());
-		assertThat(clientConfigurationResponse.getClientSecret()).isEqualTo(clientRegistrationResponse.getClientSecret());
 		assertThat(clientConfigurationResponse.getClientSecretExpiresAt()).isEqualTo(clientRegistrationResponse.getClientSecretExpiresAt());
 		assertThat(clientConfigurationResponse.getClientName()).isEqualTo(clientRegistrationResponse.getClientName());
 		assertThat(clientConfigurationResponse.getRedirectUris())
@@ -357,6 +357,34 @@ public void requestWhenClientRegistrationEndpointCustomizedThenUsed() throws Exc
 		verifyNoInteractions(authenticationFailureHandler);
 	}
 
+	// gh-1056
+	@Test
+	public void requestWhenClientRegistersWithSecretThenClientAuthenticationSuccess() throws Exception {
+		this.spring.register(AuthorizationServerConfiguration.class).autowire();
+
+		// @formatter:off
+		OidcClientRegistration clientRegistration = OidcClientRegistration.builder()
+				.clientName("client-name")
+				.redirectUri("https://client.example.com")
+				.grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue())
+				.grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue())
+				.scope("scope1")
+				.scope("scope2")
+				.build();
+		// @formatter:on
+
+		OidcClientRegistration clientRegistrationResponse = registerClient(clientRegistration);
+
+		MvcResult mvcResult = this.mvc.perform(post(DEFAULT_TOKEN_ENDPOINT_URI)
+						.param(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.CLIENT_CREDENTIALS.getValue())
+						.param(OAuth2ParameterNames.SCOPE, "scope1")
+						.with(httpBasic(clientRegistrationResponse.getClientId(), clientRegistrationResponse.getClientSecret())))
+				.andExpect(status().isOk())
+				.andExpect(jsonPath("$.access_token").isNotEmpty())
+				.andExpect(jsonPath("$.scope").value("scope1"))
+				.andReturn();
+	}
+
 	@Test
 	public void requestWhenClientRegistrationEndpointCustomizedWithAuthenticationFailureHandlerThenUsed() throws Exception {
 		this.spring.register(CustomClientRegistrationConfiguration.class).autowire();
@@ -563,9 +591,8 @@ AuthorizationServerSettings authorizationServerSettings() {
 
 		@Bean
 		PasswordEncoder passwordEncoder() {
-			return NoOpPasswordEncoder.getInstance();
+			return PasswordEncoderFactories.createDelegatingPasswordEncoder();
 		}
 
 	}
-
 }

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -30,6 +30,8 @@
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.AuthorityUtils;
+import org.springframework.security.crypto.password.NoOpPasswordEncoder;
+import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.OAuth2AccessToken;
@@ -90,6 +92,8 @@ public class OidcClientRegistrationAuthenticationProviderTests {
 	private AuthorizationServerSettings authorizationServerSettings;
 	private OidcClientRegistrationAuthenticationProvider authenticationProvider;
 
+	private PasswordEncoder passwordEncoder;
+
 	@BeforeEach
 	public void setUp() {
 		this.registeredClientRepository = mock(RegisteredClientRepository.class);
@@ -106,6 +110,18 @@ public Jwt generate(OAuth2TokenContext context) {
 		AuthorizationServerContextHolder.setContext(new TestAuthorizationServerContext(this.authorizationServerSettings, null));
 		this.authenticationProvider = new OidcClientRegistrationAuthenticationProvider(
 				this.registeredClientRepository, this.authorizationService, this.tokenGenerator);
+		this.passwordEncoder = spy(new PasswordEncoder() {
+			@Override
+			public String encode(CharSequence rawPassword) {
+				return NoOpPasswordEncoder.getInstance().encode(rawPassword);
+			}
+
+			@Override
+			public boolean matches(CharSequence rawPassword, String encodedPassword) {
+				return NoOpPasswordEncoder.getInstance().matches(rawPassword, encodedPassword);
+			}
+		});
+		this.authenticationProvider.setPasswordEncoder(this.passwordEncoder);
 	}
 
 	@AfterEach
@@ -141,6 +157,13 @@ public void setRegisteredClientConverterWhenNullThenThrowIllegalArgumentExceptio
 				.withMessage("registeredClientConverter cannot be null");
 	}
 
+	@Test
+	public void setPasswordEncoderWhenNullThenThrowIllegalArgumentException() {
+		assertThatThrownBy(() -> this.authenticationProvider.setPasswordEncoder(null))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("passwordEncoder cannot be null");
+	}
+
 	@Test
 	public void supportsWhenTypeOidcClientRegistrationAuthenticationTokenThenReturnTrue() {
 		assertThat(this.authenticationProvider.supports(OidcClientRegistrationAuthenticationToken.class)).isTrue();
@@ -472,6 +495,7 @@ public void authenticateWhenTokenEndpointAuthenticationSigningAlgorithmNotProvid
 		assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm())
 				.isEqualTo(MacAlgorithm.HS256.getName());
 		assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNotNull();
+		verify(this.passwordEncoder).encode(any());
 
 		// @formatter:off
 		builder