From 720c5c6576cc53482a16ed9e13fe61102ef5d8d1 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Mon, 5 Feb 2024 15:43:32 +0100 Subject: [PATCH] PKCE should return error if code_verifier sent but no code_challenge in the authorization request Closes #26430 Signed-off-by: rmartinc --- .../protocol/oidc/utils/PkceUtils.java | 6 +++++ .../testsuite/client/AbstractFAPITest.java | 4 ++- .../OAuthProofKeyForCodeExchangeTest.java | 26 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/PkceUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/PkceUtils.java index cf4b75b397..62081ed7e0 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/PkceUtils.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/PkceUtils.java @@ -84,6 +84,12 @@ public class PkceUtils { throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "PKCE code verifier not specified", Response.Status.BAD_REQUEST); } + if (codeChallenge == null && codeVerifier != null) { + logger.warnf("PKCE code verifier specified but challenge not present in authorization, authUserId = %s, authUsername = %s", authUserId, authUsername); + event.error(Errors.INVALID_CODE_VERIFIER); + throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "PKCE code verifier specified but challenge not present in authorization", Response.Status.BAD_REQUEST); + } + if (codeChallenge != null) { verifyCodeVerifier(codeVerifier, codeChallenge, codeChallengeMethod, authUserId, authUsername, event, cors); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractFAPITest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractFAPITest.java index 1b5587ce3c..32b33761ef 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractFAPITest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractFAPITest.java @@ -194,7 +194,9 @@ public abstract class AbstractFAPITest extends AbstractClientPoliciesTest { List parameters = new LinkedList<>(); parameters.add(new BasicNameValuePair(OAuth2Constants.GRANT_TYPE, OAuth2Constants.AUTHORIZATION_CODE)); parameters.add(new BasicNameValuePair(OAuth2Constants.CODE, code)); - parameters.add(new BasicNameValuePair(OAuth2Constants.CODE_VERIFIER, codeVerifier)); + if (codeVerifier != null) { + parameters.add(new BasicNameValuePair(OAuth2Constants.CODE_VERIFIER, codeVerifier)); + } parameters.add(new BasicNameValuePair(OAuth2Constants.REDIRECT_URI, oauth.getRedirectUri())); parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_ASSERTION_TYPE, OAuth2Constants.CLIENT_ASSERTION_TYPE_JWT)); parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_ASSERTION, signedJwt)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthProofKeyForCodeExchangeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthProofKeyForCodeExchangeTest.java index c927568a72..4bd58442a2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthProofKeyForCodeExchangeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthProofKeyForCodeExchangeTest.java @@ -404,6 +404,32 @@ public class OAuthProofKeyForCodeExchangeTest extends AbstractKeycloakTest { events.expectCodeToToken(codeId, sessionId).error(Errors.INVALID_CODE_VERIFIER).clearDetails().assertEvent(); } + + @Test + public void accessTokenRequestInPKCECodeVerifierWithNoCodeChallenge() throws Exception { + String codeVerifier = "12345678e01234567890g2345678h012a4567j90123"; // 43 + + // send oauth request without code_challenge because intercepted + oauth.codeChallenge(null); + oauth.codeChallengeMethod(null); + oauth.doLogin("test-user@localhost", "password"); + + EventRepresentation loginEvent = events.expectLogin().assertEvent(); + String sessionId = loginEvent.getSessionId(); + String codeId = loginEvent.getDetails().get(Details.CODE_ID); + + // get the code and add codeVerifier + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + oauth.codeVerifier(codeVerifier); + OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, "password"); + + // assert invalid code because no challenge in authorization + assertEquals(400, response.getStatusCode()); + assertEquals(OAuthErrorException.INVALID_GRANT, response.getError()); + assertEquals("PKCE code verifier specified but challenge not present in authorization", response.getErrorDescription()); + + events.expectCodeToToken(codeId, sessionId).error(Errors.INVALID_CODE_VERIFIER).clearDetails().assertEvent(); + } private String generateS256CodeChallenge(String codeVerifier) throws Exception { MessageDigest md = MessageDigest.getInstance("SHA-256");