Deprecate PasswordEncoder in JdbcRegisteredClientRepository

Closes gh-496
This commit is contained in:
Joe Grandja
2021-11-16 03:52:54 -05:00
parent 666d569b48
commit f0b19f30d1
4 changed files with 9 additions and 66 deletions

View File

@@ -36,7 +36,6 @@ import org.springframework.jdbc.core.JdbcOperations;
import org.springframework.jdbc.core.PreparedStatementSetter;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.SqlParameterValue;
import org.springframework.security.crypto.factory.PasswordEncoderFactories;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.jackson2.SecurityJackson2Modules;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
@@ -294,7 +293,6 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor
*/
public static class RegisteredClientParametersMapper implements Function<RegisteredClient, List<SqlParameterValue>> {
private ObjectMapper objectMapper = new ObjectMapper();
private PasswordEncoder passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder();
public RegisteredClientParametersMapper() {
ClassLoader classLoader = JdbcRegisteredClientRepository.class.getClassLoader();
@@ -323,7 +321,7 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor
new SqlParameterValue(Types.VARCHAR, registeredClient.getId()),
new SqlParameterValue(Types.VARCHAR, registeredClient.getClientId()),
new SqlParameterValue(Types.TIMESTAMP, clientIdIssuedAt),
new SqlParameterValue(Types.VARCHAR, encode(registeredClient.getClientSecret())),
new SqlParameterValue(Types.VARCHAR, registeredClient.getClientSecret()),
new SqlParameterValue(Types.TIMESTAMP, clientSecretExpiresAt),
new SqlParameterValue(Types.VARCHAR, registeredClient.getClientName()),
new SqlParameterValue(Types.VARCHAR, StringUtils.collectionToCommaDelimitedString(clientAuthenticationMethods)),
@@ -339,10 +337,11 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor
this.objectMapper = objectMapper;
}
/**
* @deprecated See javadoc {@link RegisteredClientRepository#save(RegisteredClient)}
*/
@Deprecated
public final void setPasswordEncoder(PasswordEncoder passwordEncoder) {
Assert.notNull(passwordEncoder, "passwordEncoder cannot be null");
this.passwordEncoder = passwordEncoder;
}
protected final ObjectMapper getObjectMapper() {
@@ -357,13 +356,6 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor
}
}
private String encode(String value) {
if (value != null) {
return this.passwordEncoder.encode(value);
}
return null;
}
}
}

View File

@@ -31,6 +31,9 @@ public interface RegisteredClientRepository {
/**
* Saves the registered client.
*
* <p>
* IMPORTANT: Sensitive information should be encoded externally from the implementation, e.g. {@link RegisteredClient#getClientSecret()}
*
* @param registeredClient the {@link RegisteredClient}
*/
void save(RegisteredClient registeredClient);

View File

@@ -40,9 +40,6 @@ import org.springframework.jdbc.core.SqlParameterValue;
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;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
@@ -57,11 +54,8 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
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;
/**
* Tests for {@link JdbcRegisteredClientRepository}.
@@ -77,27 +71,12 @@ public class JdbcRegisteredClientRepositoryTests {
private EmbeddedDatabase db;
private JdbcOperations jdbcOperations;
private JdbcRegisteredClientRepository registeredClientRepository;
private PasswordEncoder passwordEncoder;
@Before
public void setUp() {
this.db = createDb(OAUTH2_REGISTERED_CLIENT_SCHEMA_SQL_RESOURCE);
this.jdbcOperations = new JdbcTemplate(this.db);
this.registeredClientRepository = new JdbcRegisteredClientRepository(this.jdbcOperations);
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);
}
});
RegisteredClientParametersMapper registeredClientParametersMapper = new RegisteredClientParametersMapper();
registeredClientParametersMapper.setPasswordEncoder(this.passwordEncoder);
this.registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
}
@After
@@ -167,7 +146,6 @@ public class JdbcRegisteredClientRepositoryTests {
this.registeredClientRepository.save(expectedRegisteredClient);
RegisteredClient registeredClient = this.registeredClientRepository.findById(expectedRegisteredClient.getId());
assertThat(registeredClient).isEqualTo(expectedRegisteredClient);
verify(this.passwordEncoder).encode(anyString());
}
@Test
@@ -177,7 +155,6 @@ public class JdbcRegisteredClientRepositoryTests {
this.registeredClientRepository.save(expectedRegisteredClient);
RegisteredClient registeredClient = this.registeredClientRepository.findById(expectedRegisteredClient.getId());
assertThat(registeredClient).isEqualTo(expectedRegisteredClient);
verifyNoInteractions(this.passwordEncoder);
}
@Test
@@ -185,7 +162,6 @@ public class JdbcRegisteredClientRepositoryTests {
RowMapper<RegisteredClient> registeredClientRowMapper = spy(new RegisteredClientRowMapper());
this.registeredClientRepository.setRegisteredClientRowMapper(registeredClientRowMapper);
RegisteredClientParametersMapper clientParametersMapper = new RegisteredClientParametersMapper();
clientParametersMapper.setPasswordEncoder(this.passwordEncoder);
Function<RegisteredClient, List<SqlParameterValue>> registeredClientParametersMapper = spy(clientParametersMapper);
this.registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
@@ -195,30 +171,6 @@ public class JdbcRegisteredClientRepositoryTests {
assertThat(result).isEqualTo(registeredClient);
verify(registeredClientRowMapper).mapRow(any(), anyInt());
verify(registeredClientParametersMapper).apply(any());
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
@@ -270,17 +222,13 @@ public class JdbcRegisteredClientRepositoryTests {
@Test
public void tableDefinitionWhenCustomThenAbleToOverride() {
EmbeddedDatabase db = createDb(OAUTH2_CUSTOM_REGISTERED_CLIENT_SCHEMA_SQL_RESOURCE);
RegisteredClientParametersMapper registeredClientParametersMapper = new RegisteredClientParametersMapper();
registeredClientParametersMapper.setPasswordEncoder(this.passwordEncoder);
CustomJdbcRegisteredClientRepository registeredClientRepository = new CustomJdbcRegisteredClientRepository(new JdbcTemplate(db));
registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
registeredClientRepository.save(registeredClient);
RegisteredClient foundRegisteredClient1 = registeredClientRepository.findById(registeredClient.getId());
assertThat(foundRegisteredClient1).isEqualTo(registeredClient);
RegisteredClient foundRegisteredClient2 = registeredClientRepository.findByClientId(registeredClient.getClientId());
assertThat(foundRegisteredClient2).isEqualTo(registeredClient);
verify(this.passwordEncoder).encode(anyString());
db.shutdown();
}

View File

@@ -66,7 +66,7 @@ public class AuthorizationServerConfig {
public RegisteredClientRepository registeredClientRepository(JdbcTemplate jdbcTemplate) {
RegisteredClient registeredClient = RegisteredClient.withId(UUID.randomUUID().toString())
.clientId("messaging-client")
.clientSecret("secret")
.clientSecret("{noop}secret")
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN)