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 27a1cce2..8448bb8a 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 @@ -30,12 +30,15 @@ import java.util.UUID; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.annotation.Autowired; 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 +82,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 { @@ -90,6 +94,7 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe private final OAuth2TokenGenerator tokenGenerator; private final Converter clientRegistrationConverter; private Converter registeredClientConverter; + private PasswordEncoder passwordEncoder; /** * Constructs an {@code OidcClientRegistrationAuthenticationProvider} using the provided parameters. @@ -109,6 +114,7 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe this.tokenGenerator = tokenGenerator; this.clientRegistrationConverter = new RegisteredClientOidcClientRegistrationConverter(); this.registeredClientConverter = new OidcClientRegistrationRegisteredClientConverter(); + this.passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder(); } @Override @@ -167,6 +173,13 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe this.registeredClientConverter = registeredClientConverter; } + // gh-1056 + @Autowired(required = false) + void setPasswordEncoder(PasswordEncoder passwordEncoder) { + Assert.notNull(passwordEncoder, "passwordEncoder cannot be null"); + this.passwordEncoder = passwordEncoder; + } + private OidcClientRegistrationAuthenticationToken registerClient(OidcClientRegistrationAuthenticationToken clientRegistrationAuthentication, OAuth2Authorization authorization) { @@ -187,7 +200,16 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe } RegisteredClient registeredClient = this.registeredClientConverter.convert(clientRegistrationAuthentication.getClientRegistration()); - this.registeredClientRepository.save(registeredClient); + + if (StringUtils.hasText(registeredClient.getClientSecret())) { + // Encode the client secret + RegisteredClient updatedRegisteredClient = RegisteredClient.from(registeredClient) + .clientSecret(this.passwordEncoder.encode(registeredClient.getClientSecret())) + .build(); + this.registeredClientRepository.save(updatedRegisteredClient); + } else { + this.registeredClientRepository.save(registeredClient); + } 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 96c89c03..ca178ed6 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. @@ -57,7 +57,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; @@ -112,6 +112,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; @@ -290,7 +291,7 @@ public class OidcClientRegistrationTests { assertThat(clientConfigurationResponse.getClientId()).isEqualTo(clientRegistrationResponse.getClientId()); assertThat(clientConfigurationResponse.getClientIdIssuedAt()).isEqualTo(clientRegistrationResponse.getClientIdIssuedAt()); - assertThat(clientConfigurationResponse.getClientSecret()).isEqualTo(clientRegistrationResponse.getClientSecret()); + assertThat(clientConfigurationResponse.getClientSecret()).isNotNull(); assertThat(clientConfigurationResponse.getClientSecretExpiresAt()).isEqualTo(clientRegistrationResponse.getClientSecretExpiresAt()); assertThat(clientConfigurationResponse.getClientName()).isEqualTo(clientRegistrationResponse.getClientName()); assertThat(clientConfigurationResponse.getRedirectUris()) @@ -371,6 +372,34 @@ public class OidcClientRegistrationTests { verifyNoInteractions(authenticationSuccessHandler); } + // 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); + + 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(); + } + private OidcClientRegistration registerClient(OidcClientRegistration clientRegistration) throws Exception { // ***** (1) Obtain the "initial" access token used for registering the client @@ -566,7 +595,7 @@ 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 3ebde311..c7b04d74 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 @@ -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; @@ -71,9 +73,11 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -87,6 +91,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { private OAuth2AuthorizationService authorizationService; private JwtEncoder jwtEncoder; private OAuth2TokenGenerator tokenGenerator; + private PasswordEncoder passwordEncoder; private AuthorizationServerSettings authorizationServerSettings; private OidcClientRegistrationAuthenticationProvider authenticationProvider; @@ -102,10 +107,22 @@ public class OidcClientRegistrationAuthenticationProviderTests { return jwtGenerator.generate(context); } }); + 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.authorizationServerSettings = AuthorizationServerSettings.builder().issuer("https://provider.com").build(); AuthorizationServerContextHolder.setContext(new TestAuthorizationServerContext(this.authorizationServerSettings, null)); this.authenticationProvider = new OidcClientRegistrationAuthenticationProvider( this.registeredClientRepository, this.authorizationService, this.tokenGenerator); + this.authenticationProvider.setPasswordEncoder(this.passwordEncoder); } @AfterEach @@ -141,6 +158,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(); @@ -544,6 +568,8 @@ public class OidcClientRegistrationAuthenticationProviderTests { assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm()) .isEqualTo(MacAlgorithm.HS256.getName()); assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNotNull(); + verify(this.passwordEncoder).encode(any()); + reset(this.passwordEncoder); // @formatter:off builder @@ -555,6 +581,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm()) .isEqualTo(SignatureAlgorithm.RS256.getName()); assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNull(); + verifyNoInteractions(this.passwordEncoder); } @Test @@ -638,6 +665,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { verify(this.registeredClientRepository).save(registeredClientCaptor.capture()); verify(this.authorizationService, times(2)).save(authorizationCaptor.capture()); verify(this.jwtEncoder).encode(any()); + verify(this.passwordEncoder).encode(any()); // assert "registration" access token, which should be used for subsequent calls to client configuration endpoint OAuth2Authorization authorizationResult = authorizationCaptor.getAllValues().get(0);