PKCE configuration - enabled by default

Signed-off-by: Rohan Naik <[email protected]>

Author: rohan-naik07

docs/modules/ROOT/pages/servlet/oauth2/client/client-authentication.adoc

@@ -293,6 +293,6 @@ spring:
 
 [NOTE]
 ====
-Public Clients are supported using https://tools.ietf.org/html/rfc7636[Proof Key for Code Exchange] (PKCE).
-PKCE will automatically be used when `client-authentication-method` is set to "none" (`ClientAuthenticationMethod.NONE`).
+https://tools.ietf.org/html/rfc7636[Proof Key for Code Exchange] (PKCE) will be enabled by default for public as well as confidential clients. Refer https://docs.spring.io/spring-security/reference/servlet/oauth2/client/client-authentication.html#oauth2-client-authentication-public[this
+section] to disable PKCE.
 ====

docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc

@@ -69,7 +69,8 @@ This information is available only if the Spring Boot property `spring.security.
 <15> `(userInfoEndpoint)authenticationMethod`: The authentication method used when sending the access token to the UserInfo Endpoint.
 The supported values are *header*, *form*, and *query*.
 <16> `userNameAttributeName`: The name of the attribute returned in the UserInfo Response that references the Name or Identifier of the end-user.
-<17> [[oauth2Client-client-registration-requireProofKey]]`requireProofKey`: If `true` or if `authorizationGrantType` is `none`, then PKCE will be enabled by default.
+<17> [[oauth2Client-client-registration-requireProofKey]]`requireProofKey`: This will by default be `true`, as PKCE will be enabled by default.
+If set to `false`,then PKCE will be disabled for public as well as confidential clients.
 
 You can initially configure a `ClientRegistration` by using discovery of an OpenID Connect Provider's https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig[Configuration endpoint] or an Authorization Server's https://tools.ietf.org/html/rfc8414#section-3[Metadata endpoint].
 

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java

@@ -757,7 +757,7 @@ public static final class ClientSettings implements Serializable {
 		private boolean requireProofKey;
 
 		private ClientSettings() {
-
+			this.requireProofKey = true;
 		}
 
 		public boolean isRequireProofKey() {

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java

@@ -184,8 +184,7 @@ private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientR
 				// value.
 				applyNonce(builder);
 			}
-			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())
-					|| clientRegistration.getClientSettings().isRequireProofKey()) {
+			if (clientRegistration.getClientSettings().isRequireProofKey()) {
 				DEFAULT_PKCE_APPLIER.accept(builder);
 			}
 			return builder;

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java

@@ -196,8 +196,7 @@ private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientR
 				// value.
 				applyNonce(builder);
 			}
-			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())
-					|| clientRegistration.getClientSettings().isRequireProofKey()) {
+			if (clientRegistration.getClientSettings().isRequireProofKey()) {
 				DEFAULT_PKCE_APPLIER.accept(builder);
 			}
 			return builder;

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/TestClientRegistrations.java

@@ -64,6 +64,21 @@ public static ClientRegistration.Builder clientRegistration2() {
 		// @formatter:on
 	}
 
+	private static ClientRegistration.Builder publicClientRegistrationWithNoPkce() {
+		return ClientRegistration.withRegistrationId("no-pkce")
+				.redirectUri("{baseUrl}/{action}/oauth2/code/{registrationId}")
+				.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
+				.clientSettings(ClientRegistration.ClientSettings.builder().requireProofKey(false).build())
+				.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
+				.scope("read:user")
+				.authorizationUri("https://example.com/login/oauth/authorize")
+				.tokenUri("https://example.com/login/oauth/access_token")
+				.userInfoUri("https://api.example.com/user")
+				.userNameAttributeName("id")
+				.clientName("Client Name")
+				.clientSecret(null);
+	}
+
 	public static ClientRegistration.Builder clientCredentials() {
 		// @formatter:off
 		return clientRegistration()

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java

@@ -58,6 +58,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 
 	private ClientRegistration pkceClientRegistration;
 
+	private ClientRegistration nonProofKeyPublicClientRegistration;
+
 	private ClientRegistration fineRedirectUriTemplateRegistration;
 
 	private ClientRegistration publicClientRegistration;
@@ -373,9 +375,14 @@ public void resolveWhenAuthorizationRequestHasActionParameterLoginThenRedirectUr
 					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
 	}
 
+	// private client with proof key
+	// public client with proof key
+	// private client without proof key
+	// public client without proof key
+
 	@Test
 	public void resolveWhenAuthorizationRequestWithValidPublicClientThenResolves() {
-		ClientRegistration clientRegistration = this.publicClientRegistration;
+		ClientRegistration clientRegistration = this.publicClientRegistration; // change to private client registration
 		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
 		request.setServletPath(requestUri);
@@ -427,6 +434,25 @@ public void resolveWhenAuthorizationRequestApplyPkceToConfidentialClientsThenApp
 		assertPkceApplied(authorizationRequest, clientRegistration);
 	}
 
+	@Test
+	public void resolveWhenAuthorizationRequestApplyPkceToConfidentialClientThenApplied() {
+		// pkce enabled by default for private clients
+		ClientRegistration clientRegistration = this.registration1;
+		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		MockHttpServletRequest request = get(requestUri).build();
+		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
+		assertPkceApplied(authorizationRequest, clientRegistration);
+	}
+
+	@Test
+	public void resolveWhenAuthorizationRequestApplyPkceToPublicClientWithRequireProofKeyFalseThenApplied() {
+		ClientRegistration clientRegistration = this.nonProofKeyPublicClientRegistration; // change to non proof key public client
+		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		MockHttpServletRequest request = get(requestUri).build();
+		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
+		assertPkceNotApplied(authorizationRequest, clientRegistration);
+	}
+
 	// gh-6548
 	@Test
 	public void resolveWhenAuthorizationRequestApplyPkceToSpecificConfidentialClientThenApplied() {
@@ -593,7 +619,6 @@ private static ClientRegistration.Builder pkceClientRegistration() {
 			.clientId("client-id-3")
 			.clientSecret("client-secret");
 	}
-
 	private static ClientRegistration.Builder fineRedirectUriTemplateClientRegistration() {
 		// @formatter:off
 		return ClientRegistration.withRegistrationId("fine-redirect-uri-template-client-registration")