changed: BASIC or POST to CLIENT_SECRET_BASIC or CLIENT_SECRET_POST r…

…espectively when a client is registered. Closes gh-346
spring-projects · Jul 17, 2021 · 01e58d8 · 01e58d8
1 parent 687f03f
commit 01e58d8
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 34 deletions.
diff --git a/...ava/org/springframework/security/oauth2/server/authorization/client/RegisteredClient.java b/...ava/org/springframework/security/oauth2/server/authorization/client/RegisteredClient.java
@@ -15,16 +15,6 @@
  */
 package org.springframework.security.oauth2.server.authorization.client;
 
-import java.io.Serializable;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.time.Instant;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Objects;
-import java.util.Set;
-import java.util.function.Consumer;
-
 import org.springframework.lang.Nullable;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
@@ -35,6 +25,16 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
+import java.io.Serializable;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Consumer;
+
 /**
  * A representation of a client registration with an OAuth 2.0 Authorization Server.
  *
@@ -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/...rk/security/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverter.java b/...rk/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/...ork/security/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverter.java b/...ork/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/...g/annotation/web/configurers/oauth2/server/authorization/OidcClientRegistrationTests.java b/...g/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/...rver/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java b/...rver/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/...y/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java b/...y/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/...rg/springframework/security/oauth2/server/authorization/client/RegisteredClientTests.java b/...rg/springframework/security/oauth2/server/authorization/client/RegisteredClientTests.java
@@ -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() {
@@ -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/...rg/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java b/...rg/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/.../authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java b/.../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/...curity/oauth2/server/authorization/web/ClientSecretBasicAuthenticationConverterTests.java b/...curity/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/...ecurity/oauth2/server/authorization/web/ClientSecretPostAuthenticationConverterTests.java b/...ecurity/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()),