From 48115faee730fa020b1e5a6481ed4fa49d21a2c0 Mon Sep 17 00:00:00 2001 From: aijaz2 Date: Fri, 26 Jul 2024 17:50:31 +0100 Subject: [PATCH] Fix empty code parameter in CodeVerifierAuthenticator Closes gh-1680 --- .../CodeVerifierAuthenticator.java | 15 +++++++---- ...ientSecretAuthenticationProviderTests.java | 1 + .../OAuth2AuthorizationCodeGrantTests.java | 25 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java index 71a293126..c20d0a0a2 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java @@ -137,11 +137,16 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio } private static boolean authorizationCodeGrant(Map parameters) { - // @formatter:off - return AuthorizationGrantType.AUTHORIZATION_CODE.getValue().equals( - parameters.get(OAuth2ParameterNames.GRANT_TYPE)) && - parameters.get(OAuth2ParameterNames.CODE) != null; - // @formatter:on + + if (!AuthorizationGrantType.AUTHORIZATION_CODE.getValue().equals(parameters.get(OAuth2ParameterNames.GRANT_TYPE))) { + return false; + } + + if (!StringUtils.hasText((String) parameters.get(OAuth2ParameterNames.CODE))) { + throwInvalidGrant(OAuth2ParameterNames.CODE); + } + + return true; } private boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) { diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java index 4d41cf99f..a91eeb4c7 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProviderTests.java @@ -46,6 +46,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java index d2795a5b2..ca5573e98 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java @@ -25,6 +25,7 @@ import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Base64; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -71,6 +72,7 @@ import org.springframework.security.crypto.password.NoOpPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.OAuth2RefreshToken; import org.springframework.security.oauth2.core.OAuth2Token; import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse; @@ -98,6 +100,7 @@ import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationContext; import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationProvider; import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationToken; +import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken; import org.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository; import org.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository.RegisteredClientParametersMapper; import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; @@ -515,6 +518,28 @@ public void requestWhenPublicClientWithPkceAndCustomRefreshTokenGeneratorThenRet .isEqualTo(true); } + @Test + public void requestWhenPublicClientWithPkceAndEmptyCodeThenBadRequest() throws Exception { + this.spring.register(AuthorizationServerConfiguration.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredPublicClient().build(); + this.registeredClientRepository.save(registeredClient); + + MultiValueMap tokenRequestParameters = new LinkedMultiValueMap<>(); + tokenRequestParameters.set(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()); + tokenRequestParameters.set(OAuth2ParameterNames.CODE, ""); + tokenRequestParameters.set(OAuth2ParameterNames.REDIRECT_URI, registeredClient.getRedirectUris().iterator().next()); + + this.mvc + .perform(post(DEFAULT_TOKEN_ENDPOINT_URI) + .params(tokenRequestParameters) + .param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId()) + .param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER)) + .andExpect(header().string(HttpHeaders.CACHE_CONTROL, containsString("no-store"))) + .andExpect(header().string(HttpHeaders.PRAGMA, containsString("no-cache"))) + .andExpect(status().isBadRequest()); + } + @Test public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRequest() throws Exception { this.spring.register(AuthorizationServerConfiguration.class).autowire();