From 7dfdcf3a27214fc9423776fe566ee65bfac611c6 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Wed, 15 Jun 2022 08:24:49 -0400 Subject: [PATCH] Validate code_challenge_method parameter Issue gh-756 Closes gh-770 --- ...tionCodeRequestAuthenticationProvider.java | 8 +++----- ...odeRequestAuthenticationProviderTests.java | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java index 9c82e80f..0abbab6c 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java @@ -209,11 +209,9 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen String codeChallenge = (String) authorizationCodeRequestAuthentication.getAdditionalParameters().get(PkceParameterNames.CODE_CHALLENGE); if (StringUtils.hasText(codeChallenge)) { String codeChallengeMethod = (String) authorizationCodeRequestAuthentication.getAdditionalParameters().get(PkceParameterNames.CODE_CHALLENGE_METHOD); - if (StringUtils.hasText(codeChallengeMethod)) { - if (!"S256".equals(codeChallengeMethod)) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE_METHOD, PKCE_ERROR_URI, - authorizationCodeRequestAuthentication, registeredClient, null); - } + if (!StringUtils.hasText(codeChallengeMethod) || !"S256".equals(codeChallengeMethod)) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE_METHOD, PKCE_ERROR_URI, + authorizationCodeRequestAuthentication, registeredClient, null); } } else if (registeredClient.getClientSettings().isRequireProofKey()) { throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE, PKCE_ERROR_URI, diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java index b96560d1..bf40785e 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java @@ -377,6 +377,26 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests { ); } + // gh-770 + @Test + public void authenticateWhenPkceMissingCodeChallengeMethodThenThrowOAuth2AuthorizationCodeRequestAuthenticationException() { + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + when(this.registeredClientRepository.findByClientId(eq(registeredClient.getClientId()))) + .thenReturn(registeredClient); + Map additionalParameters = new HashMap<>(); + additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, "code-challenge"); + OAuth2AuthorizationCodeRequestAuthenticationToken authentication = + authorizationCodeRequestAuthentication(registeredClient, this.principal) + .additionalParameters(additionalParameters) + .build(); + assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .isInstanceOf(OAuth2AuthorizationCodeRequestAuthenticationException.class) + .satisfies(ex -> + assertAuthenticationException((OAuth2AuthorizationCodeRequestAuthenticationException) ex, + OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE_METHOD, authentication.getRedirectUri()) + ); + } + @Test public void authenticateWhenPrincipalNotAuthenticatedThenReturnAuthorizationCodeRequest() { RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();