Throw invalid_grant when invalid token request with PKCE

Closes gh-581
spring-projects · Jan 24, 2022 · a1e513b · a1e513b
1 parent 4d5b288
commit a1e513b
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 13 deletions.
diff --git a/...curity/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProvider.java b/...curity/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProvider.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2021 the original author or authors.
+ * Copyright 2020-2022 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.
@@ -228,7 +228,7 @@ private boolean authenticatePkceIfAvailable(OAuth2ClientAuthenticationToken clie
 				(String) parameters.get(OAuth2ParameterNames.CODE),
 				AUTHORIZATION_CODE_TOKEN_TYPE);
 		if (authorization == null) {
-			throwInvalidClient(OAuth2ParameterNames.CODE);
+			throwInvalidGrant(OAuth2ParameterNames.CODE);
 		}
 
 		OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(
@@ -238,7 +238,7 @@ private boolean authenticatePkceIfAvailable(OAuth2ClientAuthenticationToken clie
 				.get(PkceParameterNames.CODE_CHALLENGE);
 		if (!StringUtils.hasText(codeChallenge)) {
 			if (registeredClient.getClientSettings().isRequireProofKey()) {
-				throwInvalidClient(PkceParameterNames.CODE_CHALLENGE);
+				throwInvalidGrant(PkceParameterNames.CODE_CHALLENGE);
 			} else {
 				return false;
 			}
@@ -248,7 +248,7 @@ private boolean authenticatePkceIfAvailable(OAuth2ClientAuthenticationToken clie
 				.get(PkceParameterNames.CODE_CHALLENGE_METHOD);
 		String codeVerifier = (String) parameters.get(PkceParameterNames.CODE_VERIFIER);
 		if (!codeVerifierValid(codeVerifier, codeChallenge, codeChallengeMethod)) {
-			throwInvalidClient(PkceParameterNames.CODE_VERIFIER);
+			throwInvalidGrant(PkceParameterNames.CODE_VERIFIER);
 		}
 
 		return true;
@@ -291,10 +291,20 @@ private static void throwInvalidClient(String parameterName, Throwable cause) {
 		throw new OAuth2AuthenticationException(error, error.toString(), cause);
 	}
 
+	private static void throwInvalidGrant(String parameterName) {
+		OAuth2Error error = new OAuth2Error(
+				OAuth2ErrorCodes.INVALID_GRANT,
+				"Client authentication failed: " + parameterName,
+				null
+		);
+		throw new OAuth2AuthenticationException(error);
+	}
+
 	private static class JwtClientAssertionDecoderFactory implements JwtDecoderFactory<RegisteredClient> {
 		private static final String JWT_CLIENT_AUTHENTICATION_ERROR_URI = "https://datatracker.ietf.org/doc/html/rfc7523#section-3";
 
 		private static final Map<JwsAlgorithm, String> JCA_ALGORITHM_MAPPINGS;
+
 		static {
 			Map<JwsAlgorithm, String> mappings = new HashMap<>();
 			mappings.put(MacAlgorithm.HS256, "HmacSHA256");

diff --git a/...tation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java b/...tation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2021 the original author or authors.
+ * Copyright 2020-2022 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.
@@ -396,7 +396,7 @@ public void requestWhenPublicClientWithPkceThenReturnAccessTokenResponse() throw
 	}
 
 	@Test
-	public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenUnauthorized() throws Exception {
+	public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenBadRequest() throws Exception {
 		this.spring.register(AuthorizationServerConfiguration.class).autowire();
 
 		RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
@@ -421,7 +421,7 @@ public void requestWhenConfidentialClientWithPkceAndMissingCodeVerifierThenUnaut
 				.params(getTokenRequestParameters(registeredClient, authorizationCodeAuthorization))
 				.param(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId())
 				.header(HttpHeaders.AUTHORIZATION, getAuthorizationHeader(registeredClient)))
-				.andExpect(status().isUnauthorized());
+				.andExpect(status().isBadRequest());
 	}
 
 	@Test

diff --git a/...y/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java b/...y/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2021 the original author or authors.
+ * Copyright 2020-2022 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.
@@ -279,7 +279,7 @@ public void authenticateWhenPkceAndInvalidCodeThenThrowOAuth2AuthenticationExcep
 				.isInstanceOf(OAuth2AuthenticationException.class)
 				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
 				.satisfies(error -> {
-					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_GRANT);
 					assertThat(error.getDescription()).contains(OAuth2ParameterNames.CODE);
 				});
 	}
@@ -305,7 +305,7 @@ public void authenticateWhenPkceAndPublicClientAndMissingCodeVerifierThenThrowOA
 				.isInstanceOf(OAuth2AuthenticationException.class)
 				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
 				.satisfies(error -> {
-					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_GRANT);
 					assertThat(error.getDescription()).contains(PkceParameterNames.CODE_VERIFIER);
 				});
 	}
@@ -331,7 +331,7 @@ public void authenticateWhenPkceAndConfidentialClientAndMissingCodeVerifierThenT
 				.isInstanceOf(OAuth2AuthenticationException.class)
 				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
 				.satisfies(error -> {
-					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_GRANT);
 					assertThat(error.getDescription()).contains(PkceParameterNames.CODE_VERIFIER);
 				});
 	}
@@ -357,7 +357,7 @@ public void authenticateWhenPkceAndPlainMethodAndInvalidCodeVerifierThenThrowOAu
 				.isInstanceOf(OAuth2AuthenticationException.class)
 				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
 				.satisfies(error -> {
-					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_GRANT);
 					assertThat(error.getDescription()).contains(PkceParameterNames.CODE_VERIFIER);
 				});
 	}
@@ -383,7 +383,7 @@ public void authenticateWhenPkceAndS256MethodAndInvalidCodeVerifierThenThrowOAut
 				.isInstanceOf(OAuth2AuthenticationException.class)
 				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
 				.satisfies(error -> {
-					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_CLIENT);
+					assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_GRANT);
 					assertThat(error.getDescription()).contains(PkceParameterNames.CODE_VERIFIER);
 				});
 	}