From a7035d22bd2de6c24e7125623d38fb83d8f659a9 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Fri, 1 Mar 2024 11:40:59 -0500 Subject: [PATCH] Update PKCE validation --- .../CodeVerifierAuthenticator.java | 7 +++-- .../OAuth2AuthorizationCodeGrantTests.java | 31 ++++++++++++++++++- 2 files changed, 34 insertions(+), 4 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 98a577ee..dc063bea 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2022 the original author or authors. + * Copyright 2020-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -94,8 +94,10 @@ final class CodeVerifierAuthenticator { String codeChallenge = (String) authorizationRequest.getAdditionalParameters() .get(PkceParameterNames.CODE_CHALLENGE); + String codeVerifier = (String) parameters.get(PkceParameterNames.CODE_VERIFIER); if (!StringUtils.hasText(codeChallenge)) { - if (registeredClient.getClientSettings().isRequireProofKey()) { + if (registeredClient.getClientSettings().isRequireProofKey() || + StringUtils.hasText(codeVerifier)) { throwInvalidGrant(PkceParameterNames.CODE_CHALLENGE); } else { if (this.logger.isTraceEnabled()) { @@ -111,7 +113,6 @@ final class CodeVerifierAuthenticator { String codeChallengeMethod = (String) authorizationRequest.getAdditionalParameters() .get(PkceParameterNames.CODE_CHALLENGE_METHOD); - String codeVerifier = (String) parameters.get(PkceParameterNames.CODE_VERIFIER); if (!codeVerifierValid(codeVerifier, codeChallenge, codeChallengeMethod)) { throwInvalidGrant(PkceParameterNames.CODE_VERIFIER); } 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 5112cb4c..822f2bc4 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 the original author or authors. + * Copyright 2020-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -488,6 +488,35 @@ public class OAuth2AuthorizationCodeGrantTests { assertThat(redirectedUrl).isEqualTo(expectedRedirectUri); } + @Test + public void requestWhenConfidentialClientWithPkceAndMissingCodeChallengeButCodeVerifierProvidedThenBadRequest() throws Exception { + this.spring.register(AuthorizationServerConfiguration.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + this.registeredClientRepository.save(registeredClient); + + MultiValueMap authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient); + MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI) + .queryParams(authorizationRequestParameters) + .with(user("user"))) + .andExpect(status().is3xxRedirection()) + .andReturn(); + String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); + String expectedRedirectUri = authorizationRequestParameters.getFirst(OAuth2ParameterNames.REDIRECT_URI); + assertThat(redirectedUrl).matches(expectedRedirectUri + "\\?code=.{15,}&state=" + STATE_URL_ENCODED); + + String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); + OAuth2Authorization authorizationCodeAuthorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE); + assertThat(authorizationCodeAuthorization).isNotNull(); + assertThat(authorizationCodeAuthorization.getAuthorizationGrantType()).isEqualTo(AuthorizationGrantType.AUTHORIZATION_CODE); + + this.mvc.perform(post(DEFAULT_TOKEN_ENDPOINT_URI) + .params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization)) + .param(PkceParameterNames.CODE_VERIFIER, S256_CODE_VERIFIER) + .header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient))) + .andExpect(status().isBadRequest()); + } + @Test public void requestWhenCustomTokenGeneratorThenUsed() throws Exception { this.spring.register(AuthorizationServerConfigurationWithTokenGenerator.class).autowire();