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