Skip to content

Remove use of deprecated ClientAuthenticationMethod's #350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ public RegisteredClient build() {
}
validateScopes();
validateRedirectUris();
upgradeClientAuthenticationMethods();
return create();
}

Expand Down Expand Up @@ -544,6 +545,17 @@ private void validateRedirectUris() {
}
}

private void upgradeClientAuthenticationMethods() {
if (this.clientAuthenticationMethods.contains(ClientAuthenticationMethod.BASIC)) {
this.clientAuthenticationMethods.remove(ClientAuthenticationMethod.BASIC);
this.clientAuthenticationMethods.add(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
}
if (this.clientAuthenticationMethods.contains(ClientAuthenticationMethod.POST)) {
this.clientAuthenticationMethods.remove(ClientAuthenticationMethod.POST);
this.clientAuthenticationMethods.add(ClientAuthenticationMethod.CLIENT_SECRET_POST);
}
}

private static boolean validateRedirectUri(String redirectUri) {
try {
URI validRedirectUri = new URI(redirectUri);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 the original author or authors.
* Copyright 2020-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -86,7 +86,7 @@ public Authentication convert(HttpServletRequest request) {
throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST), ex);
}

return new OAuth2ClientAuthenticationToken(clientID, clientSecret, ClientAuthenticationMethod.BASIC,
return new OAuth2ClientAuthenticationToken(clientID, clientSecret, ClientAuthenticationMethod.CLIENT_SECRET_BASIC,
extractAdditionalParameters(request));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 the original author or authors.
* Copyright 2020-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -67,7 +67,7 @@ public Authentication convert(HttpServletRequest request) {
throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST));
}

return new OAuth2ClientAuthenticationToken(clientId, clientSecret, ClientAuthenticationMethod.POST,
return new OAuth2ClientAuthenticationToken(clientId, clientSecret, ClientAuthenticationMethod.CLIENT_SECRET_POST,
extractAdditionalParameters(request));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public void requestWhenClientRegistrationRequestAuthorizedThenClientRegistration
assertThat(clientRegistrationResponse.getScopes())
.containsExactlyInAnyOrderElementsOf(clientRegistration.getScopes());
assertThat(clientRegistrationResponse.getTokenEndpointAuthenticationMethod())
.isEqualTo(ClientAuthenticationMethod.BASIC.getValue());
.isEqualTo(ClientAuthenticationMethod.CLIENT_SECRET_BASIC.getValue());
assertThat(clientRegistrationResponse.getIdTokenSignedResponseAlgorithm())
.isEqualTo(SignatureAlgorithm.RS256.getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void authenticateWhenClientPrincipalNotOAuth2ClientAuthenticationTokenThe
public void authenticateWhenClientPrincipalNotAuthenticatedThenThrowOAuth2AuthenticationException() {
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
OAuth2ClientAuthenticationToken clientPrincipal = new OAuth2ClientAuthenticationToken(
registeredClient.getClientId(), registeredClient.getClientSecret(), ClientAuthenticationMethod.BASIC, null);
registeredClient.getClientId(), registeredClient.getClientSecret(), ClientAuthenticationMethod.CLIENT_SECRET_BASIC, null);
OAuth2AuthorizationCodeAuthenticationToken authentication =
new OAuth2AuthorizationCodeAuthenticationToken(AUTHORIZATION_CODE, clientPrincipal, null, null);
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public void authenticateWhenInvalidClientIdThenThrowOAuth2AuthenticationExceptio
.thenReturn(registeredClient);

OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken(
registeredClient.getClientId() + "-invalid", registeredClient.getClientSecret(), ClientAuthenticationMethod.BASIC, null);
registeredClient.getClientId() + "-invalid", registeredClient.getClientSecret(), ClientAuthenticationMethod.CLIENT_SECRET_BASIC, null);
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
.isInstanceOf(OAuth2AuthenticationException.class)
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
Expand All @@ -139,7 +139,7 @@ public void authenticateWhenInvalidClientSecretThenThrowOAuth2AuthenticationExce
.thenReturn(registeredClient);

OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken(
registeredClient.getClientId(), registeredClient.getClientSecret() + "-invalid", ClientAuthenticationMethod.BASIC, null);
registeredClient.getClientId(), registeredClient.getClientSecret() + "-invalid", ClientAuthenticationMethod.CLIENT_SECRET_BASIC, null);
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
.isInstanceOf(OAuth2AuthenticationException.class)
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
Expand Down Expand Up @@ -170,7 +170,7 @@ public void authenticateWhenValidCredentialsThenAuthenticated() {
.thenReturn(registeredClient);

OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken(
registeredClient.getClientId(), registeredClient.getClientSecret(), ClientAuthenticationMethod.BASIC, null);
registeredClient.getClientId(), registeredClient.getClientSecret(), ClientAuthenticationMethod.CLIENT_SECRET_BASIC, null);
OAuth2ClientAuthenticationToken authenticationResult =
(OAuth2ClientAuthenticationToken) this.authenticationProvider.authenticate(authentication);

Expand Down Expand Up @@ -416,7 +416,7 @@ public void authenticateWhenClientAuthenticationMethodNotConfiguredThenThrowOAut
.thenReturn(registeredClient);

OAuth2ClientAuthenticationToken authentication = new OAuth2ClientAuthenticationToken(
registeredClient.getClientId(), registeredClient.getClientSecret(), ClientAuthenticationMethod.POST, null);
registeredClient.getClientId(), registeredClient.getClientSecret(), ClientAuthenticationMethod.CLIENT_SECRET_POST, null);
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
.isInstanceOf(OAuth2AuthenticationException.class)
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class RegisteredClientTests {
private static final Set<String> SCOPES = Collections.unmodifiableSet(
Stream.of("openid", "profile", "email").collect(Collectors.toSet()));
private static final Set<ClientAuthenticationMethod> CLIENT_AUTHENTICATION_METHODS =
Collections.singleton(ClientAuthenticationMethod.BASIC);
Collections.singleton(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);

@Test
public void buildWhenAuthorizationGrantTypesNotSetThenThrowIllegalArgumentException() {
Expand Down Expand Up @@ -146,7 +146,7 @@ public void buildWhenClientAuthenticationMethodNotProvidedThenDefaultToBasic() {
.build();

assertThat(registration.getClientAuthenticationMethods())
.isEqualTo(Collections.singleton(ClientAuthenticationMethod.BASIC));
.isEqualTo(Collections.singleton(ClientAuthenticationMethod.CLIENT_SECRET_BASIC));
}

@Test
Expand Down Expand Up @@ -280,6 +280,22 @@ public void buildWhenAuthorizationGrantTypesConsumerClearsSetThenThrowIllegalArg

@Test
public void buildWhenTwoClientAuthenticationMethodsAreProvidedThenBothAreRegistered() {
RegisteredClient registration = RegisteredClient.withId(ID)
.clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET)
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST)
.redirectUris(redirectUris -> redirectUris.addAll(REDIRECT_URIS))
.scopes(scopes -> scopes.addAll(SCOPES))
.build();

assertThat(registration.getClientAuthenticationMethods())
.containsExactlyInAnyOrder(ClientAuthenticationMethod.CLIENT_SECRET_BASIC, ClientAuthenticationMethod.CLIENT_SECRET_POST);
}

@Test
public void buildWhenBothDeprecatedClientAuthenticationMethodsAreProvidedThenBothNonDeprecatedAreRegistered() {
RegisteredClient registration = RegisteredClient.withId(ID)
.clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET)
Expand All @@ -291,11 +307,29 @@ public void buildWhenTwoClientAuthenticationMethodsAreProvidedThenBothAreRegiste
.build();

assertThat(registration.getClientAuthenticationMethods())
.containsExactlyInAnyOrder(ClientAuthenticationMethod.BASIC, ClientAuthenticationMethod.POST);
.containsExactlyInAnyOrder(ClientAuthenticationMethod.CLIENT_SECRET_BASIC, ClientAuthenticationMethod.CLIENT_SECRET_POST);
}

@Test
public void buildWhenClientAuthenticationMethodsConsumerIsProvidedThenConsumerAccepted() {
RegisteredClient registration = RegisteredClient.withId(ID)
.clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET)
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.clientAuthenticationMethods(clientAuthenticationMethods -> {
clientAuthenticationMethods.add(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
clientAuthenticationMethods.add(ClientAuthenticationMethod.CLIENT_SECRET_POST);
})
.redirectUris(redirectUris -> redirectUris.addAll(REDIRECT_URIS))
.scopes(scopes -> scopes.addAll(SCOPES))
.build();

assertThat(registration.getClientAuthenticationMethods())
.containsExactlyInAnyOrder(ClientAuthenticationMethod.CLIENT_SECRET_BASIC, ClientAuthenticationMethod.CLIENT_SECRET_POST);
}

@Test
public void buildWhenConsumerAddsDeprecatedClientAuthenticationMethodsThenNonDeprecatedAreRegistered() {
RegisteredClient registration = RegisteredClient.withId(ID)
.clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET)
Expand All @@ -309,7 +343,7 @@ public void buildWhenClientAuthenticationMethodsConsumerIsProvidedThenConsumerAc
.build();

assertThat(registration.getClientAuthenticationMethods())
.containsExactlyInAnyOrder(ClientAuthenticationMethod.BASIC, ClientAuthenticationMethod.POST);
.containsExactlyInAnyOrder(ClientAuthenticationMethod.CLIENT_SECRET_BASIC, ClientAuthenticationMethod.CLIENT_SECRET_POST);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static RegisteredClient.Builder registeredClient() {
.clientSecret("secret")
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN)
.clientAuthenticationMethod(ClientAuthenticationMethod.BASIC)
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
.redirectUri("https://example.com")
.scope("scope1");
}
Expand All @@ -46,8 +46,8 @@ public static RegisteredClient.Builder registeredClient2() {
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN)
.authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS)
.clientAuthenticationMethod(ClientAuthenticationMethod.BASIC)
.clientAuthenticationMethod(ClientAuthenticationMethod.POST)
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)
.clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_POST)
.redirectUri("https://example.com")
.scope("scope1")
.scope("scope2");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public void authenticateWhenValidAccessTokenThenReturnClientRegistration() {
assertThat(registeredClientResult.getClientIdIssuedAt()).isNotNull();
assertThat(registeredClientResult.getClientSecret()).isNotNull();
assertThat(registeredClientResult.getClientName()).isEqualTo(clientRegistration.getClientName());
assertThat(registeredClientResult.getClientAuthenticationMethods()).containsExactly(ClientAuthenticationMethod.BASIC);
assertThat(registeredClientResult.getClientAuthenticationMethods()).containsExactly(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
assertThat(registeredClientResult.getRedirectUris()).containsExactly("https://client.example.com");
assertThat(registeredClientResult.getAuthorizationGrantTypes())
.containsExactlyInAnyOrder(AuthorizationGrantType.AUTHORIZATION_CODE, AuthorizationGrantType.CLIENT_CREDENTIALS);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 the original author or authors.
* Copyright 2020-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -99,7 +99,7 @@ public void convertWhenAuthorizationHeaderBasicWithValidCredentialsThenReturnCli
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
assertThat(authentication.getPrincipal()).isEqualTo("clientId");
assertThat(authentication.getCredentials()).isEqualTo("secret");
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC);
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
}

@Test
Expand All @@ -109,7 +109,7 @@ public void convertWhenConfidentialClientWithPkceParametersThenAdditionalParamet
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
assertThat(authentication.getPrincipal()).isEqualTo("clientId");
assertThat(authentication.getCredentials()).isEqualTo("secret");
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC);
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
assertThat(authentication.getAdditionalParameters())
.containsOnly(
entry(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 the original author or authors.
* Copyright 2020-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -86,7 +86,7 @@ public void convertWhenPostWithValidCredentialsThenReturnClientAuthenticationTok
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
assertThat(authentication.getPrincipal()).isEqualTo("client-1");
assertThat(authentication.getCredentials()).isEqualTo("client-secret");
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.POST);
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.CLIENT_SECRET_POST);
}

@Test
Expand All @@ -97,7 +97,7 @@ public void convertWhenConfidentialClientWithPkceParametersThenAdditionalParamet
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
assertThat(authentication.getPrincipal()).isEqualTo("client-1");
assertThat(authentication.getCredentials()).isEqualTo("client-secret");
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.POST);
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.CLIENT_SECRET_POST);
assertThat(authentication.getAdditionalParameters())
.containsOnly(
entry(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()),
Expand Down