diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java index 86e4b4e3c41..2714175c5b3 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java @@ -138,7 +138,9 @@ public void configureWhenAuthorizationCodeRequestThenRedirectForAuthorization() assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&" + "scope=user&state=.{15,}&" + - "redirect_uri=http://localhost/client-1"); + "redirect_uri=http://localhost/client-1&" + + "code_challenge_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -151,7 +153,9 @@ public void configureWhenOauth2ClientInLambdaThenRedirectForAuthorization() thro assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&" + "scope=user&state=.{15,}&" + - "redirect_uri=http://localhost/client-1"); + "redirect_uri=http://localhost/client-1&" + + "code_challenge_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -203,7 +207,9 @@ public void configureWhenRequestCacheProvidedAndClientAuthorizationRequiredExcep assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&" + "scope=user&state=.{15,}&" + - "redirect_uri=http://localhost/client-1"); + "redirect_uri=http://localhost/client-1&" + + "code_challenge_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); verify(requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class)); } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index b1047966877..d34cfe3463a 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java @@ -20,7 +20,6 @@ import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.security.oauth2.core.AuthorizationGrantType; -import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.PkceParameterNames; @@ -130,9 +129,7 @@ private OAuth2AuthorizationRequest resolve(HttpServletRequest request, String re // REQUIRED. OpenID Connect requests MUST contain the "openid" scope value. addNonceParameters(attributes, additionalParameters); } - if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) { - addPkceParameters(attributes, additionalParameters); - } + addPkceParameters(attributes, additionalParameters); builder.additionalParameters(additionalParameters); } else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) { builder = OAuth2AuthorizationRequest.implicit(); diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java index 1410099d934..b8d3a71d219 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java @@ -23,7 +23,6 @@ import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository; import org.springframework.security.oauth2.core.AuthorizationGrantType; -import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.PkceParameterNames; @@ -144,9 +143,7 @@ private OAuth2AuthorizationRequest authorizationRequest(ServerWebExchange exchan // REQUIRED. OpenID Connect requests MUST contain the "openid" scope value. addNonceParameters(attributes, additionalParameters); } - if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) { - addPkceParameters(attributes, additionalParameters); - } + addPkceParameters(attributes, additionalParameters); builder.additionalParameters(additionalParameters); } else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) { builder = OAuth2AuthorizationRequest.implicit(); diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java index 1fd1ccb34dc..c83c79e5a7e 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java @@ -123,12 +123,15 @@ public void resolveWhenAuthorizationRequestWithValidClientThenResolves() { assertThat(authorizationRequest.getState()).isNotNull(); assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OAuth2ParameterNames.REGISTRATION_ID); assertThat(authorizationRequest.getAttributes()) - .containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId())); + .containsOnlyKeys(OAuth2ParameterNames.REGISTRATION_ID, PkceParameterNames.CODE_VERIFIER) + .containsEntry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()); 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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -141,7 +144,8 @@ public void resolveWhenClientAuthorizationRequiredExceptionAvailableThenResolves OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request, clientRegistration.getRegistrationId()); assertThat(authorizationRequest).isNotNull(); assertThat(authorizationRequest.getAttributes()) - .containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId())); + .containsOnlyKeys(OAuth2ParameterNames.REGISTRATION_ID, PkceParameterNames.CODE_VERIFIER) + .containsEntry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()); } @Test @@ -263,7 +267,9 @@ public void resolveWhenAuthorizationRequestIncludesPort80ThenExpandedRedirectUri .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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -281,7 +287,9 @@ public void resolveWhenAuthorizationRequestIncludesPort443ThenExpandedRedirectUr .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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -296,7 +304,9 @@ public void resolveWhenClientAuthorizationRequiredExceptionAvailableThenRedirect .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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -311,7 +321,9 @@ public void resolveWhenAuthorizationRequestOAuth2LoginThenRedirectUriIsLogin() { .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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -327,7 +339,9 @@ public void resolveWhenAuthorizationRequestHasActionParameterAuthorizeThenRedire .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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -343,7 +357,9 @@ public void resolveWhenAuthorizationRequestHasActionParameterLoginThenRedirectUr .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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -411,7 +427,9 @@ public void resolveWhenAuthenticationRequestWithValidOidcClientThenResolves() { "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}"); + "code_challenge_method=S256&" + + "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } private static ClientRegistration.Builder fineRedirectUriTemplateClientRegistration() { diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java index bbc47c9a56e..18a04ecbede 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java @@ -154,7 +154,9 @@ public void doFilterWhenAuthorizationRequestOAuth2LoginThenRedirectForAuthorizat assertThat(response.getRedirectedUrl()).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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -234,7 +236,9 @@ public void doFilterWhenCustomAuthorizationRequestBaseUriThenRedirectForAuthoriz assertThat(response.getRedirectedUrl()).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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -255,7 +259,9 @@ public void doFilterWhenNotAuthorizationRequestAndClientAuthorizationRequiredExc assertThat(response.getRedirectedUrl()).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_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); verify(this.requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class)); } @@ -359,6 +365,8 @@ public void doFilterWhenAuthorizationRequestAndCustomAuthorizationRequestUriSetT "response_type=code&client_id=client-id&" + "scope=read:user&state=.{15,}&" + "redirect_uri=http://localhost/login/oauth2/code/registration-id&" + + "code_challenge_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "login_hint=user@provider\\.com"); } } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java index 0febd5025d8..dee8349307b 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java @@ -84,7 +84,9 @@ public void resolveWhenClientRegistrationFoundThenWorks() { assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&" + "scope=read:user&state=.*?&" + - "redirect_uri=/login/oauth2/code/registration-id"); + "redirect_uri=/login/oauth2/code/registration-id&" + + "code_challenge_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -98,7 +100,9 @@ public void resolveWhenForwardedHeadersClientRegistrationFoundThenWorks() { assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&" + "scope=read:user&state=.*?&" + - "redirect_uri=/login/oauth2/code/registration-id"); + "redirect_uri=/login/oauth2/code/registration-id&" + + "code_challenge_method=S256&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } @Test @@ -136,7 +140,9 @@ public void resolveWhenAuthenticationRequestWithValidOidcClientThenResolves() { "response_type=code&client_id=client-id&" + "scope=openid&state=.*?&" + "redirect_uri=/login/oauth2/code/registration-id&" + - "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); + "code_challenge_method=S256&" + + "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}"); } private OAuth2AuthorizationRequest resolve(String path) {