Merge remote-tracking branch 'origin/pkce-default-config-gh-16391' into pkce-default-config-gh-16391

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

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

Author: rohan-naik07

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

@@ -756,9 +756,7 @@ public static final class ClientSettings implements Serializable {
 
 		private boolean requireProofKey;
 
-		private ClientSettings() {
-			this.requireProofKey = true;
-		}
+		private ClientSettings() {}
 
 		public boolean isRequireProofKey() {
 			return this.requireProofKey;
@@ -794,6 +792,7 @@ public static final class Builder {
 			private boolean requireProofKey;
 
 			private Builder() {
+				this.requireProofKey = true;
 			}
 
 			/**

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

@@ -64,7 +64,7 @@ public static ClientRegistration.Builder clientRegistration2() {
 		// @formatter:on
 	}
 
-	private static ClientRegistration.Builder publicClientRegistrationWithNoPkce() {
+	public static ClientRegistration.Builder publicClientRegistrationWithNoPkce() {
 		return ClientRegistration.withRegistrationId("no-pkce")
 				.redirectUri("{baseUrl}/{action}/oauth2/code/{registrationId}")
 				.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
@@ -76,6 +76,7 @@ private static ClientRegistration.Builder publicClientRegistrationWithNoPkce() {
 				.userInfoUri("https://api.example.com/user")
 				.userNameAttributeName("id")
 				.clientName("Client Name")
+				.clientId("client-id")
 				.clientSecret(null);
 	}
 

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

@@ -78,7 +78,7 @@ public void setUp() {
 		this.registration2 = TestClientRegistrations.clientRegistration2().build();
 
 		this.pkceClientRegistration = pkceClientRegistration().build();
-
+		this.nonProofKeyPublicClientRegistration = TestClientRegistrations.publicClientRegistrationWithNoPkce().build();
 		this.fineRedirectUriTemplateRegistration = fineRedirectUriTemplateClientRegistration().build();
 		// @formatter:off
 		this.publicClientRegistration = TestClientRegistrations.clientRegistration()
@@ -94,7 +94,7 @@ public void setUp() {
 		// @formatter:on
 		this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(this.registration1,
 				this.registration2, this.pkceClientRegistration, this.fineRedirectUriTemplateRegistration,
-				this.publicClientRegistration, this.oidcRegistration);
+				this.publicClientRegistration, this.oidcRegistration,nonProofKeyPublicClientRegistration);
 		this.resolver = new DefaultOAuth2AuthorizationRequestResolver(this.clientRegistrationRepository,
 				this.authorizationRequestBaseUri);
 	}
@@ -176,11 +176,14 @@ public void resolveWhenAuthorizationRequestWithValidClientThenResolves() {
 		assertThat(authorizationRequest.getAdditionalParameters())
 			.doesNotContainKey(OAuth2ParameterNames.REGISTRATION_ID);
 		assertThat(authorizationRequest.getAttributes())
-			.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+			.containsExactly(
+					entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()),
+					entry(PkceParameterNames.CODE_VERIFIER, authorizationRequest.getAttributes().get(PkceParameterNames.CODE_VERIFIER))
+			);
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -193,7 +196,10 @@ public void resolveWhenClientAuthorizationRequiredExceptionAvailableThenResolves
 				clientRegistration.getRegistrationId());
 		assertThat(authorizationRequest).isNotNull();
 		assertThat(authorizationRequest.getAttributes())
-			.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+			.containsExactly(
+					entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()),
+					entry(PkceParameterNames.CODE_VERIFIER, authorizationRequest.getAttributes().get(PkceParameterNames.CODE_VERIFIER))
+			);
 	}
 
 	@Test
@@ -301,7 +307,7 @@ public void resolveWhenAuthorizationRequestIncludesPort80ThenExpandedRedirectUri
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id"+ "&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -317,7 +323,7 @@ public void resolveWhenAuthorizationRequestIncludesPort443ThenExpandedRedirectUr
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=https://example.com/login/oauth2/code/registration-id");
+					+ "redirect_uri=https://example.com/login/oauth2/code/registration-id" + "&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -331,7 +337,7 @@ public void resolveWhenClientAuthorizationRequiredExceptionAvailableThenRedirect
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -344,7 +350,7 @@ public void resolveWhenAuthorizationRequestOAuth2LoginThenRedirectUriIsLogin() {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id-2&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2" + "&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -358,7 +364,7 @@ public void resolveWhenAuthorizationRequestHasActionParameterAuthorizeThenRedire
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -372,7 +378,7 @@ public void resolveWhenAuthorizationRequestHasActionParameterLoginThenRedirectUr
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id-2&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -449,32 +455,6 @@ public void resolveWhenAuthorizationRequestApplyPkceToPublicClientWithRequirePro
 	}
 
 	// gh-6548
-	@Test
-	public void resolveWhenAuthorizationRequestApplyPkceToSpecificConfidentialClientThenApplied() {
-		this.resolver.setAuthorizationRequestCustomizer((builder) -> {
-			builder.attributes((attrs) -> {
-				String registrationId = (String) attrs.get(OAuth2ParameterNames.REGISTRATION_ID);
-				if (this.registration1.getRegistrationId().equals(registrationId)) {
-					OAuth2AuthorizationRequestCustomizers.withPkce().accept(builder);
-				}
-			});
-		});
-
-		ClientRegistration clientRegistration = this.registration1;
-		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
-		request.setServletPath(requestUri);
-		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
-		assertPkceApplied(authorizationRequest, clientRegistration);
-
-		clientRegistration = this.registration2;
-		requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
-		request = new MockHttpServletRequest("GET", requestUri);
-		request.setServletPath(requestUri);
-		authorizationRequest = this.resolver.resolve(request);
-		assertPkceNotApplied(authorizationRequest, clientRegistration);
-	}
-
 	private void assertPkceApplied(OAuth2AuthorizationRequest authorizationRequest,
 			ClientRegistration clientRegistration) {
 		assertThat(authorizationRequest.getAdditionalParameters()).containsKey(PkceParameterNames.CODE_CHALLENGE);
@@ -531,7 +511,7 @@ public void resolveWhenAuthenticationRequestWithValidOidcClientThenResolves() {
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=openid&state=.{15,}&"
 					+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&"
-					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	// gh-7696
@@ -551,7 +531,8 @@ public void resolveWhenAuthorizationRequestCustomizerRemovesNonceThenQueryExclud
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=openid&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id");
+					+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&" +
+					"code_challenge=.{15,}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -569,7 +550,7 @@ public void resolveWhenAuthorizationRequestCustomizerAddsParameterThenQueryInclu
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=openid&state=.{15,}&"
 					+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&"
-					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "param1=value1");
+					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}" + "&code_challenge=.{15,}&code_challenge_method=S256&param1=value1");
 	}
 
 	@Test
@@ -586,7 +567,7 @@ public void resolveWhenAuthorizationRequestCustomizerOverridesParameterThenQuery
 		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches(
 				"https://example.com/login/oauth/authorize\\?" + "response_type=code&" + "scope=openid&state=.{15,}&"
 						+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&"
-						+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "appid=client-id");
+						+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}" + "&code_challenge=.{15,}&code_challenge_method=S256&appid=client-id");
 	}
 
 	@Test