diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java index 60e824859..de673b65e 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java @@ -122,6 +122,13 @@ public Authentication authenticate(Authentication authentication) throws Authent throwInvalidClient("client_secret_expires_at"); } + if (this.passwordEncoder.upgradeEncoding(registeredClient.getClientSecret())) { + registeredClient = RegisteredClient.from(registeredClient) + .clientSecret(this.passwordEncoder.encode(clientSecret)) + .build(); + registeredClientRepository.save(registeredClient); + } + if (this.logger.isTraceEnabled()) { this.logger.trace("Validated client authentication parameters"); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepository.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepository.java index 38edf7dff..6c6a18869 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepository.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepository.java @@ -72,9 +72,14 @@ public InMemoryRegisteredClientRepository(List registrations) @Override public void save(RegisteredClient registeredClient) { Assert.notNull(registeredClient, "registeredClient cannot be null"); - assertUniqueIdentifiers(registeredClient, this.idRegistrationMap); - this.idRegistrationMap.put(registeredClient.getId(), registeredClient); - this.clientIdRegistrationMap.put(registeredClient.getClientId(), registeredClient); + if (this.idRegistrationMap.containsKey(registeredClient.getId())) { + this.idRegistrationMap.put(registeredClient.getId(), registeredClient); + this.clientIdRegistrationMap.put(registeredClient.getClientId(), registeredClient); + } else { + assertUniqueIdentifiers(registeredClient, this.idRegistrationMap); + this.idRegistrationMap.put(registeredClient.getId(), registeredClient); + this.clientIdRegistrationMap.put(registeredClient.getClientId(), registeredClient); + } } @Nullable diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/JdbcRegisteredClientRepository.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/JdbcRegisteredClientRepository.java index 3da2d3703..37a5cb69e 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/JdbcRegisteredClientRepository.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/JdbcRegisteredClientRepository.java @@ -96,8 +96,9 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor // @formatter:off private static final String UPDATE_REGISTERED_CLIENT_SQL = "UPDATE " + TABLE_NAME - + " SET client_name = ?, client_authentication_methods = ?, authorization_grant_types = ?," - + " redirect_uris = ?, post_logout_redirect_uris = ?, scopes = ?, client_settings = ?, token_settings = ?" + + " SET client_secret = ?, client_secret_expires_at = ?, client_name = ?, client_authentication_methods = ?," + + " authorization_grant_types = ?, redirect_uris = ?, post_logout_redirect_uris = ?, scopes = ?," + + " client_settings = ?, token_settings = ?" + " WHERE " + PK_FILTER; // @formatter:on @@ -136,8 +137,6 @@ private void updateRegisteredClient(RegisteredClient registeredClient) { SqlParameterValue id = parameters.remove(0); parameters.remove(0); // remove client_id parameters.remove(0); // remove client_id_issued_at - parameters.remove(0); // remove client_secret - parameters.remove(0); // remove client_secret_expires_at parameters.add(id); PreparedStatementSetter pss = new ArgumentPreparedStatementSetter(parameters.toArray()); this.jdbcOperations.update(UPDATE_REGISTERED_CLIENT_SQL, pss); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java index 90a25e9be..17f757ebe 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java @@ -85,6 +85,11 @@ public String encode(CharSequence rawPassword) { public boolean matches(CharSequence rawPassword, String encodedPassword) { return NoOpPasswordEncoder.getInstance().matches(rawPassword, encodedPassword); } + + @Override + public boolean upgradeEncoding(String encodedPassword) { + return true; + } }); this.authenticationProvider.setPasswordEncoder(this.passwordEncoder); } @@ -222,6 +227,28 @@ public void authenticateWhenValidCredentialsThenAuthenticated() { assertThat(authenticationResult.getRegisteredClient()).isEqualTo(registeredClient); } + @Test + public void authenticateWhenValidCredentialsAndNonExpiredThenPasswordUpgraded() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) + .thenReturn(registeredClient); + + OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken( + registeredClient.getClientId(), ClientAuthenticationMethod.CLIENT_SECRET_BASIC, registeredClient.getClientSecret(), null); + OAuth2ClientAuthenticationToken authenticationResult = + (OAuth2ClientAuthenticationToken) this.authenticationProvider.authenticate(authentication); + + verify(this.passwordEncoder).matches(any(), any()); + verify(this.passwordEncoder).upgradeEncoding(any()); + verify(this.passwordEncoder).encode(any()); + verify(this.registeredClientRepository).save(any()); + assertThat(authenticationResult.isAuthenticated()).isTrue(); + assertThat(registeredClient).isNotSameAs(authenticationResult.getPrincipal()); + assertThat(authenticationResult.getPrincipal().toString()).isEqualTo(registeredClient.getClientId()); + assertThat(authenticationResult.getCredentials().toString()).isEqualTo(registeredClient.getClientSecret()); + assertThat(authenticationResult.getRegisteredClient()).isEqualTo(registeredClient); + } + @Test public void authenticateWhenAuthorizationCodeGrantAndValidCredentialsThenAuthenticated() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepositoryTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepositoryTests.java index d7638ee8c..94048a28e 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepositoryTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepositoryTests.java @@ -153,12 +153,12 @@ public void saveWhenNullThenThrowIllegalArgumentException() { } @Test - public void saveWhenExistingIdThenThrowIllegalArgumentException() { + public void saveWhenExistingIdThenUpdate() { RegisteredClient registeredClient = createRegisteredClient( - this.registration.getId(), "client-id-2", "client-secret-2"); - assertThatIllegalArgumentException() - .isThrownBy(() -> this.clients.save(registeredClient)) - .withMessage("Registered client must be unique. Found duplicate identifier: " + registeredClient.getId()); + this.registration.getId(), "client-id", "client-secret-2"); + this.clients.save(registeredClient); + RegisteredClient savedClient = this.clients.findByClientId(registeredClient.getClientId()); + assertThat(savedClient.getClientSecret()).isEqualTo("client-secret-2"); } @Test diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java index d20060ed0..65c16c337 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java @@ -55,6 +55,7 @@ import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.crypto.factory.PasswordEncoderFactories; import org.springframework.security.crypto.password.NoOpPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.oauth2.core.AuthorizationGrantType; @@ -231,6 +232,27 @@ public void requestWhenTokenRequestPostsClientCredentialsThenTokenResponse() thr verify(jwtCustomizer).customize(any()); } + @Test + public void requestWhenTokenRequestPostsClientCredentialsThenTokenResponseAndSecretUpgraded() throws Exception { + this.spring.register(AuthorizationServerConfigurationCustomPasswordEncoder.class).autowire(); + + String clientSecret = "secret-2"; + RegisteredClient registeredClient = TestRegisteredClients.registeredClient2().clientSecret("{noop}" + clientSecret).build(); + this.registeredClientRepository.save(registeredClient); + + this.mvc.perform(post(DEFAULT_TOKEN_ENDPOINT_URI) + .param(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) + .param(OAuth2ParameterNames.SCOPE, "scope1 scope2") + .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) + .param(OAuth2ParameterNames.CLIENT_SECRET, clientSecret)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.access_token").isNotEmpty()) + .andExpect(jsonPath("$.scope").value("scope1 scope2")); + + verify(jwtCustomizer).customize(any()); + assertThat(this.registeredClientRepository.findByClientId(registeredClient.getClientId()).getClientSecret()).startsWith("{bcrypt}"); + } + @Test public void requestWhenTokenEndpointCustomizedThenUsed() throws Exception { this.spring.register(AuthorizationServerConfigurationCustomTokenEndpoint.class).autowire(); @@ -429,6 +451,15 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h // @formatter:on } + @EnableWebSecurity + @Configuration(proxyBeanMethods = false) + static class AuthorizationServerConfigurationCustomPasswordEncoder extends AuthorizationServerConfiguration { + @Override + PasswordEncoder passwordEncoder() { + return PasswordEncoderFactories.createDelegatingPasswordEncoder(); + } + } + @EnableWebSecurity @Configuration(proxyBeanMethods = false) static class AuthorizationServerConfigurationCustomClientAuthentication extends AuthorizationServerConfiguration {