diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClient.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClient.java index 1823b5432..9ab9edc0b 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClient.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClient.java @@ -486,6 +486,7 @@ public RegisteredClient build() { } validateScopes(); validateRedirectUris(); + upgradeClientAuthenticationMethods(); return create(); } @@ -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); diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverter.java index aa8180871..dcfd20142 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverter.java @@ -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. @@ -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)); } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverter.java index 1b82d3f1b..8070f7c4c 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverter.java @@ -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. @@ -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)); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcClientRegistrationTests.java index f480efd5a..9fa7d81a0 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcClientRegistrationTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcClientRegistrationTests.java @@ -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()); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java index 087637091..1f8d9d750 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java @@ -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)) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java index e7dafac13..33263339d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java @@ -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()) @@ -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()) @@ -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); @@ -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()) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClientTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClientTests.java index d913b0850..bfbbb80aa 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClientTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/RegisteredClientTests.java @@ -43,7 +43,7 @@ public class RegisteredClientTests { private static final Set SCOPES = Collections.unmodifiableSet( Stream.of("openid", "profile", "email").collect(Collectors.toSet())); private static final Set CLIENT_AUTHENTICATION_METHODS = - Collections.singleton(ClientAuthenticationMethod.BASIC); + Collections.singleton(ClientAuthenticationMethod.CLIENT_SECRET_BASIC); @Test public void buildWhenAuthorizationGrantTypesNotSetThenThrowIllegalArgumentException() { @@ -146,7 +146,7 @@ public void buildWhenClientAuthenticationMethodNotProvidedThenDefaultToBasic() { .build(); assertThat(registration.getClientAuthenticationMethods()) - .isEqualTo(Collections.singleton(ClientAuthenticationMethod.BASIC)); + .isEqualTo(Collections.singleton(ClientAuthenticationMethod.CLIENT_SECRET_BASIC)); } @Test @@ -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) @@ -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) @@ -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 diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java index 908c6d058..514433b59 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java @@ -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"); } @@ -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"); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java index 70d2ca599..a6986920d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java @@ -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); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverterTests.java index 863b5c002..a1790b483 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverterTests.java @@ -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. @@ -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 @@ -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()), diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverterTests.java index c2b3ddf8e..863dd118c 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverterTests.java @@ -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. @@ -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 @@ -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()),