Avoid client secret double encoding when updating a registered client

This might have to be revisited at a later point, but to check if a value is encoded or not is quite tricky. The decision was to remove client_secret and client_secret_expires_at from the update statement

Closes gh-389

Author: ovidiupopa07

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

@@ -95,8 +95,7 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor
 
 	// @formatter:off
 	private static final String UPDATE_REGISTERED_CLIENT_SQL = "UPDATE " + TABLE_NAME
-			+ " SET client_secret = ?, client_secret_expires_at = ?,"
-			+ " client_name = ?, client_authentication_methods = ?, authorization_grant_types = ?,"
+			+ " SET client_name = ?, client_authentication_methods = ?, authorization_grant_types = ?,"
 			+ " redirect_uris = ?, scopes = ?, client_settings = ?, token_settings = ?"
 			+ " WHERE " + PK_FILTER;
 	// @formatter:on
@@ -134,6 +133,8 @@ 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/client/JdbcRegisteredClientRepositoryTests.java

@@ -40,6 +40,7 @@
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+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.jackson2.SecurityJackson2Modules;
@@ -57,6 +58,7 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
@@ -196,6 +198,29 @@ public void saveLoadRegisteredClientWhenCustomStrategiesSetThenCalled() throws E
 		verify(this.passwordEncoder).encode(anyString());
 	}
 
+	// gh-389
+	@Test
+	public void saveWhenClientSecretAlreadyEncodedThenNotUpdated() {
+		PasswordEncoder passwordEncoder = spy(PasswordEncoderFactories.createDelegatingPasswordEncoder());
+		RegisteredClientParametersMapper registeredClientParametersMapper = new RegisteredClientParametersMapper();
+		registeredClientParametersMapper.setPasswordEncoder(passwordEncoder);
+		this.registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
+
+		RegisteredClient originalRegisteredClient = TestRegisteredClients.registeredClient().build();
+		this.registeredClientRepository.save(originalRegisteredClient);
+		verify(passwordEncoder).encode(eq(originalRegisteredClient.getClientSecret()));
+
+		RegisteredClient registeredClient = this.registeredClientRepository.findById(originalRegisteredClient.getId());
+		assertThat(registeredClient).isNotNull();
+		assertThat(passwordEncoder.matches(originalRegisteredClient.getClientSecret(), registeredClient.getClientSecret())).isTrue();
+
+		RegisteredClient updatedRegisteredClient = RegisteredClient.from(registeredClient).clientSecret("updated-client-secret").build();
+		this.registeredClientRepository.save(updatedRegisteredClient);
+		updatedRegisteredClient = this.registeredClientRepository.findById(originalRegisteredClient.getId());
+		assertThat(updatedRegisteredClient).isNotNull();
+		assertThat(passwordEncoder.matches(originalRegisteredClient.getClientSecret(), updatedRegisteredClient.getClientSecret())).isTrue();
+	}
+
 	@Test
 	public void findByIdWhenIdNullThenThrowIllegalArgumentException() {
 		// @formatter:off