Upgrade client secret when available

Closes gh-1099

Author: shanman190

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");
 		}

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/InMemoryRegisteredClientRepository.java

@@ -72,9 +72,14 @@ public InMemoryRegisteredClientRepository(List<RegisteredClient> 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

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);

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();

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

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 {