Skip to content

Commit f0b19f3

Browse files
jgrandjajgrandja
committed
Deprecate PasswordEncoder in JdbcRegisteredClientRepository
Closes gh-496
1 parent 666d569 commit f0b19f3

File tree

4 files changed

+9
-66
lines changed

4 files changed

+9
-66
lines changed

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.springframework.jdbc.core.PreparedStatementSetter;
3737
import org.springframework.jdbc.core.RowMapper;
3838
import org.springframework.jdbc.core.SqlParameterValue;
39-
import org.springframework.security.crypto.factory.PasswordEncoderFactories;
4039
import org.springframework.security.crypto.password.PasswordEncoder;
4140
import org.springframework.security.jackson2.SecurityJackson2Modules;
4241
import org.springframework.security.oauth2.core.AuthorizationGrantType;
@@ -294,7 +293,6 @@ private static ClientAuthenticationMethod resolveClientAuthenticationMethod(Stri
294293
*/
295294
public static class RegisteredClientParametersMapper implements Function<RegisteredClient, List<SqlParameterValue>> {
296295
private ObjectMapper objectMapper = new ObjectMapper();
297-
private PasswordEncoder passwordEncoder = PasswordEncoderFactories.createDelegatingPasswordEncoder();
298296

299297
public RegisteredClientParametersMapper() {
300298
ClassLoader classLoader = JdbcRegisteredClientRepository.class.getClassLoader();
@@ -323,7 +321,7 @@ public List<SqlParameterValue> apply(RegisteredClient registeredClient) {
323321
new SqlParameterValue(Types.VARCHAR, registeredClient.getId()),
324322
new SqlParameterValue(Types.VARCHAR, registeredClient.getClientId()),
325323
new SqlParameterValue(Types.TIMESTAMP, clientIdIssuedAt),
326-
new SqlParameterValue(Types.VARCHAR, encode(registeredClient.getClientSecret())),
324+
new SqlParameterValue(Types.VARCHAR, registeredClient.getClientSecret()),
327325
new SqlParameterValue(Types.TIMESTAMP, clientSecretExpiresAt),
328326
new SqlParameterValue(Types.VARCHAR, registeredClient.getClientName()),
329327
new SqlParameterValue(Types.VARCHAR, StringUtils.collectionToCommaDelimitedString(clientAuthenticationMethods)),
@@ -339,10 +337,11 @@ public final void setObjectMapper(ObjectMapper objectMapper) {
339337
this.objectMapper = objectMapper;
340338
}
341339

342-
340+
/**
341+
* @deprecated See javadoc {@link RegisteredClientRepository#save(RegisteredClient)}
342+
*/
343+
@Deprecated
343344
public final void setPasswordEncoder(PasswordEncoder passwordEncoder) {
344-
Assert.notNull(passwordEncoder, "passwordEncoder cannot be null");
345-
this.passwordEncoder = passwordEncoder;
346345
}
347346

348347
protected final ObjectMapper getObjectMapper() {
@@ -357,13 +356,6 @@ private String writeMap(Map<String, Object> data) {
357356
}
358357
}
359358

360-
private String encode(String value) {
361-
if (value != null) {
362-
return this.passwordEncoder.encode(value);
363-
}
364-
return null;
365-
}
366-
367359
}
368360

369361
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public interface RegisteredClientRepository {
3131
/**
3232
* Saves the registered client.
3333
*
34+
* <p>
35+
* IMPORTANT: Sensitive information should be encoded externally from the implementation, e.g. {@link RegisteredClient#getClientSecret()}
36+
*
3437
* @param registeredClient the {@link RegisteredClient}
3538
*/
3639
void save(RegisteredClient registeredClient);

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/JdbcRegisteredClientRepositoryTests.java‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@
4040
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
4141
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
4242
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
43-
import org.springframework.security.crypto.factory.PasswordEncoderFactories;
44-
import org.springframework.security.crypto.password.NoOpPasswordEncoder;
45-
import org.springframework.security.crypto.password.PasswordEncoder;
4643
import org.springframework.security.jackson2.SecurityJackson2Modules;
4744
import org.springframework.security.oauth2.core.AuthorizationGrantType;
4845
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
@@ -57,11 +54,8 @@
5754
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
5855
import static org.mockito.ArgumentMatchers.any;
5956
import static org.mockito.ArgumentMatchers.anyInt;
60-
import static org.mockito.ArgumentMatchers.anyString;
61-
import static org.mockito.ArgumentMatchers.eq;
6257
import static org.mockito.Mockito.spy;
6358
import static org.mockito.Mockito.verify;
64-
import static org.mockito.Mockito.verifyNoInteractions;
6559

6660
/**
6761
* Tests for {@link JdbcRegisteredClientRepository}.
@@ -77,27 +71,12 @@ public class JdbcRegisteredClientRepositoryTests {
7771
private EmbeddedDatabase db;
7872
private JdbcOperations jdbcOperations;
7973
private JdbcRegisteredClientRepository registeredClientRepository;
80-
private PasswordEncoder passwordEncoder;
8174

8275
@Before
8376
public void setUp() {
8477
this.db = createDb(OAUTH2_REGISTERED_CLIENT_SCHEMA_SQL_RESOURCE);
8578
this.jdbcOperations = new JdbcTemplate(this.db);
8679
this.registeredClientRepository = new JdbcRegisteredClientRepository(this.jdbcOperations);
87-
this.passwordEncoder = spy(new PasswordEncoder() {
88-
@Override
89-
public String encode(CharSequence rawPassword) {
90-
return NoOpPasswordEncoder.getInstance().encode(rawPassword);
91-
}
92-
93-
@Override
94-
public boolean matches(CharSequence rawPassword, String encodedPassword) {
95-
return NoOpPasswordEncoder.getInstance().matches(rawPassword, encodedPassword);
96-
}
97-
});
98-
RegisteredClientParametersMapper registeredClientParametersMapper = new RegisteredClientParametersMapper();
99-
registeredClientParametersMapper.setPasswordEncoder(this.passwordEncoder);
100-
this.registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
10180
}
10281

10382
@After
@@ -167,7 +146,6 @@ public void saveWhenNewThenSaved() {
167146
this.registeredClientRepository.save(expectedRegisteredClient);
168147
RegisteredClient registeredClient = this.registeredClientRepository.findById(expectedRegisteredClient.getId());
169148
assertThat(registeredClient).isEqualTo(expectedRegisteredClient);
170-
verify(this.passwordEncoder).encode(anyString());
171149
}
172150

173151
@Test
@@ -177,15 +155,13 @@ public void saveWhenClientSecretNullThenSaved() {
177155
this.registeredClientRepository.save(expectedRegisteredClient);
178156
RegisteredClient registeredClient = this.registeredClientRepository.findById(expectedRegisteredClient.getId());
179157
assertThat(registeredClient).isEqualTo(expectedRegisteredClient);
180-
verifyNoInteractions(this.passwordEncoder);
181158
}
182159

183160
@Test
184161
public void saveLoadRegisteredClientWhenCustomStrategiesSetThenCalled() throws Exception {
185162
RowMapper<RegisteredClient> registeredClientRowMapper = spy(new RegisteredClientRowMapper());
186163
this.registeredClientRepository.setRegisteredClientRowMapper(registeredClientRowMapper);
187164
RegisteredClientParametersMapper clientParametersMapper = new RegisteredClientParametersMapper();
188-
clientParametersMapper.setPasswordEncoder(this.passwordEncoder);
189165
Function<RegisteredClient, List<SqlParameterValue>> registeredClientParametersMapper = spy(clientParametersMapper);
190166
this.registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
191167

@@ -195,30 +171,6 @@ public void saveLoadRegisteredClientWhenCustomStrategiesSetThenCalled() throws E
195171
assertThat(result).isEqualTo(registeredClient);
196172
verify(registeredClientRowMapper).mapRow(any(), anyInt());
197173
verify(registeredClientParametersMapper).apply(any());
198-
verify(this.passwordEncoder).encode(anyString());
199-
}
200-
201-
// gh-389
202-
@Test
203-
public void saveWhenClientSecretAlreadyEncodedThenNotUpdated() {
204-
PasswordEncoder passwordEncoder = spy(PasswordEncoderFactories.createDelegatingPasswordEncoder());
205-
RegisteredClientParametersMapper registeredClientParametersMapper = new RegisteredClientParametersMapper();
206-
registeredClientParametersMapper.setPasswordEncoder(passwordEncoder);
207-
this.registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
208-
209-
RegisteredClient originalRegisteredClient = TestRegisteredClients.registeredClient().build();
210-
this.registeredClientRepository.save(originalRegisteredClient);
211-
verify(passwordEncoder).encode(eq(originalRegisteredClient.getClientSecret()));
212-
213-
RegisteredClient registeredClient = this.registeredClientRepository.findById(originalRegisteredClient.getId());
214-
assertThat(registeredClient).isNotNull();
215-
assertThat(passwordEncoder.matches(originalRegisteredClient.getClientSecret(), registeredClient.getClientSecret())).isTrue();
216-
217-
RegisteredClient updatedRegisteredClient = RegisteredClient.from(registeredClient).clientSecret("updated-client-secret").build();
218-
this.registeredClientRepository.save(updatedRegisteredClient);
219-
updatedRegisteredClient = this.registeredClientRepository.findById(originalRegisteredClient.getId());
220-
assertThat(updatedRegisteredClient).isNotNull();
221-
assertThat(passwordEncoder.matches(originalRegisteredClient.getClientSecret(), updatedRegisteredClient.getClientSecret())).isTrue();
222174
}
223175

224176
@Test
@@ -270,17 +222,13 @@ public void findByClientIdWhenNotExistsThenNotFound() {
270222
@Test
271223
public void tableDefinitionWhenCustomThenAbleToOverride() {
272224
EmbeddedDatabase db = createDb(OAUTH2_CUSTOM_REGISTERED_CLIENT_SCHEMA_SQL_RESOURCE);
273-
RegisteredClientParametersMapper registeredClientParametersMapper = new RegisteredClientParametersMapper();
274-
registeredClientParametersMapper.setPasswordEncoder(this.passwordEncoder);
275225
CustomJdbcRegisteredClientRepository registeredClientRepository = new CustomJdbcRegisteredClientRepository(new JdbcTemplate(db));
276-
registeredClientRepository.setRegisteredClientParametersMapper(registeredClientParametersMapper);
277226
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
278227
registeredClientRepository.save(registeredClient);
279228
RegisteredClient foundRegisteredClient1 = registeredClientRepository.findById(registeredClient.getId());
280229
assertThat(foundRegisteredClient1).isEqualTo(registeredClient);
281230
RegisteredClient foundRegisteredClient2 = registeredClientRepository.findByClientId(registeredClient.getClientId());
282231
assertThat(foundRegisteredClient2).isEqualTo(registeredClient);
283-
verify(this.passwordEncoder).encode(anyString());
284232
db.shutdown();
285233
}
286234

‎samples/default-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h
6666
public RegisteredClientRepository registeredClientRepository(JdbcTemplate jdbcTemplate) {
6767
RegisteredClient registeredClient = RegisteredClient.withId(UUID.randomUUID().toString())
6868
.clientId("messaging-client")
69-
.clientSecret("secret")
69+
.clientSecret("{noop}secret")
7070
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
7171
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
7272
.authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN)

0 commit comments

Comments
 (0)