diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java index 4e5d3f06..c5c33893 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationEndpointConfigurer.java +++ b/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.AuthenticationManager; 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 @@ public final class OidcClientRegistrationEndpointConfigurer extends AbstractOAut 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 = 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 97166d0d..c47040da 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 @@ -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.core.convert.converter.Converter; 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 @@ import org.springframework.util.StringUtils; * @see OAuth2TokenGenerator * @see OidcClientRegistrationAuthenticationToken * @see OidcClientConfigurationAuthenticationProvider + * @see PasswordEncoder * @see 3. Client Registration Endpoint */ public final class OidcClientRegistrationAuthenticationProvider implements AuthenticationProvider { @@ -91,6 +94,8 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe private final Converter clientRegistrationConverter; private Converter registeredClientConverter; + private PasswordEncoder passwordEncoder; + /** * Constructs an {@code OidcClientRegistrationAuthenticationProvider} using the provided parameters. * @@ -109,6 +114,21 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe 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 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe } 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"); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java index 16395f30..5f6b949b 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java +++ b/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.Customizer; 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.reset; 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 class OidcClientRegistrationTests { 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 class OidcClientRegistrationTests { 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 @@ public class OidcClientRegistrationTests { @Bean PasswordEncoder passwordEncoder() { - return NoOpPasswordEncoder.getInstance(); + return PasswordEncoderFactories.createDelegatingPasswordEncoder(); } } - } 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 3c51c8a6..bbf23604 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 @@ -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.mockito.ArgumentCaptor; 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 class OidcClientRegistrationAuthenticationProviderTests { 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 class OidcClientRegistrationAuthenticationProviderTests { .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 class OidcClientRegistrationAuthenticationProviderTests { assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm()) .isEqualTo(MacAlgorithm.HS256.getName()); assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNotNull(); + verify(this.passwordEncoder).encode(any()); // @formatter:off builder