From 63aa5d8933b46bb0157a03b095cf79eb3acbd604 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Tue, 31 Jan 2023 10:39:30 +0900 Subject: [PATCH 1/2] Fix client secret encoding when client dynamically registered Closes gh-1056 --- ...cClientRegistrationEndpointConfigurer.java | 7 +++- ...entRegistrationAuthenticationProvider.java | 38 ++++++++++++++++++- .../OidcClientRegistrationTests.java | 37 +++++++++++++++--- ...gistrationAuthenticationProviderTests.java | 26 ++++++++++++- 4 files changed, 99 insertions(+), 9 deletions(-) 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 From addd6e13d57eb1567f951c9ac1102998faac7b90 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 6 Mar 2023 16:31:19 -0500 Subject: [PATCH 2/2] Polish gh-1056 --- ...cClientRegistrationEndpointConfigurer.java | 7 +--- ...entRegistrationAuthenticationProvider.java | 40 +++++++------------ .../OidcClientRegistrationTests.java | 30 +++++++------- ...gistrationAuthenticationProviderTests.java | 16 +++++--- 4 files changed, 41 insertions(+), 52 deletions(-) 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 c5c33893..4e5d3f06 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-2023 the original author or authors. + * Copyright 2020-2022 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,7 +26,6 @@ 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; @@ -222,10 +221,6 @@ 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 c47040da..a2ddbe4a 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,6 +30,7 @@ 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; @@ -93,7 +94,6 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe private final OAuth2TokenGenerator tokenGenerator; private final Converter clientRegistrationConverter; private Converter registeredClientConverter; - private PasswordEncoder passwordEncoder; /** @@ -117,20 +117,6 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe 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 public Authentication authenticate(Authentication authentication) throws AuthenticationException { OidcClientRegistrationAuthenticationToken clientRegistrationAuthentication = @@ -187,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) { @@ -204,21 +197,16 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe RegisteredClient registeredClient = this.registeredClientConverter.convert(clientRegistrationAuthentication.getClientRegistration()); - // 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) + 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 { - saveRegisteredClient = registeredClient; + this.registeredClientRepository.save(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 5f6b949b..d6122ebb 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 @@ -290,6 +290,7 @@ public class OidcClientRegistrationTests { assertThat(clientConfigurationResponse.getClientId()).isEqualTo(clientRegistrationResponse.getClientId()); assertThat(clientConfigurationResponse.getClientIdIssuedAt()).isEqualTo(clientRegistrationResponse.getClientIdIssuedAt()); + assertThat(clientConfigurationResponse.getClientSecret()).isNotNull(); assertThat(clientConfigurationResponse.getClientSecretExpiresAt()).isEqualTo(clientRegistrationResponse.getClientSecretExpiresAt()); assertThat(clientConfigurationResponse.getClientName()).isEqualTo(clientRegistrationResponse.getClientName()); assertThat(clientConfigurationResponse.getRedirectUris()) @@ -357,6 +358,19 @@ public class OidcClientRegistrationTests { verifyNoInteractions(authenticationFailureHandler); } + @Test + public void requestWhenClientRegistrationEndpointCustomizedWithAuthenticationFailureHandlerThenUsed() throws Exception { + this.spring.register(CustomClientRegistrationConfiguration.class).autowire(); + + when(authenticationProvider.authenticate(any())).thenThrow(new OAuth2AuthenticationException("error")); + + this.mvc.perform(get(DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI) + .param(OAuth2ParameterNames.CLIENT_ID, "invalid").with(jwt())); + + verify(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); + verifyNoInteractions(authenticationSuccessHandler); + } + // gh-1056 @Test public void requestWhenClientRegistersWithSecretThenClientAuthenticationSuccess() throws Exception { @@ -375,7 +389,7 @@ public class OidcClientRegistrationTests { OidcClientRegistration clientRegistrationResponse = registerClient(clientRegistration); - MvcResult mvcResult = this.mvc.perform(post(DEFAULT_TOKEN_ENDPOINT_URI) + 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()))) @@ -385,19 +399,6 @@ public class OidcClientRegistrationTests { .andReturn(); } - @Test - public void requestWhenClientRegistrationEndpointCustomizedWithAuthenticationFailureHandlerThenUsed() throws Exception { - this.spring.register(CustomClientRegistrationConfiguration.class).autowire(); - - when(authenticationProvider.authenticate(any())).thenThrow(new OAuth2AuthenticationException("error")); - - this.mvc.perform(get(DEFAULT_OIDC_CLIENT_REGISTRATION_ENDPOINT_URI) - .param(OAuth2ParameterNames.CLIENT_ID, "invalid").with(jwt())); - - verify(authenticationFailureHandler).onAuthenticationFailure(any(), any(), any()); - verifyNoInteractions(authenticationSuccessHandler); - } - private OidcClientRegistration registerClient(OidcClientRegistration clientRegistration) throws Exception { // ***** (1) Obtain the "initial" access token used for registering the client @@ -595,4 +596,5 @@ public class OidcClientRegistrationTests { } } + } 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 bbf23604..73c2ad4d 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 @@ -73,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; /** @@ -89,11 +91,10 @@ public class OidcClientRegistrationAuthenticationProviderTests { private OAuth2AuthorizationService authorizationService; private JwtEncoder jwtEncoder; private OAuth2TokenGenerator tokenGenerator; + private PasswordEncoder passwordEncoder; private AuthorizationServerSettings authorizationServerSettings; private OidcClientRegistrationAuthenticationProvider authenticationProvider; - private PasswordEncoder passwordEncoder; - @BeforeEach public void setUp() { this.registeredClientRepository = mock(RegisteredClientRepository.class); @@ -106,10 +107,6 @@ public class OidcClientRegistrationAuthenticationProviderTests { return jwtGenerator.generate(context); } }); - 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.passwordEncoder = spy(new PasswordEncoder() { @Override public String encode(CharSequence rawPassword) { @@ -121,6 +118,10 @@ public class OidcClientRegistrationAuthenticationProviderTests { 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); } @@ -496,6 +497,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { .isEqualTo(MacAlgorithm.HS256.getName()); assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNotNull(); verify(this.passwordEncoder).encode(any()); + reset(this.passwordEncoder); // @formatter:off builder @@ -507,6 +509,7 @@ public class OidcClientRegistrationAuthenticationProviderTests { assertThat(authenticationResult.getClientRegistration().getTokenEndpointAuthenticationSigningAlgorithm()) .isEqualTo(SignatureAlgorithm.RS256.getName()); assertThat(authenticationResult.getClientRegistration().getClientSecret()).isNull(); + verifyNoInteractions(this.passwordEncoder); } @Test @@ -589,6 +592,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);