Upgrade client secret when available
Closes gh-1099
This commit is contained in:
committed by
Joe Grandja
parent
d197c188ba
commit
0255a24849
@@ -122,6 +122,13 @@ public final class ClientSecretAuthenticationProvider implements AuthenticationP
|
||||
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");
|
||||
}
|
||||
|
||||
@@ -72,9 +72,14 @@ public final class InMemoryRegisteredClientRepository implements RegisteredClien
|
||||
@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
|
||||
|
||||
@@ -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 @@ public class JdbcRegisteredClientRepository implements RegisteredClientRepositor
|
||||
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);
|
||||
|
||||
@@ -85,6 +85,11 @@ public class ClientSecretAuthenticationProviderTests {
|
||||
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 class ClientSecretAuthenticationProviderTests {
|
||||
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();
|
||||
|
||||
@@ -153,12 +153,12 @@ public class InMemoryRegisteredClientRepositoryTests {
|
||||
}
|
||||
|
||||
@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
|
||||
|
||||
@@ -55,6 +55,7 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity;
|
||||
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 class OAuth2ClientCredentialsGrantTests {
|
||||
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 class OAuth2ClientCredentialsGrantTests {
|
||||
// @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 {
|
||||
|
||||
Reference in New Issue
Block a user