From a1e513b35d2989da96b91d71c844c5b9dd0c0465 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Mon, 17 Jan 2022 14:37:48 +0100 Subject: [PATCH] Throw invalid_grant when invalid token request with PKCE Closes gh-581 --- .../OAuth2ClientAuthenticationProvider.java | 18 ++++++++++++++---- .../OAuth2AuthorizationCodeGrantTests.java | 6 +++--- ...Auth2ClientAuthenticationProviderTests.java | 12 ++++++------ 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProvider.java index b92fe008..258a88de 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/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 @@ public final class OAuth2ClientAuthenticationProvider implements AuthenticationP (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 @@ public final class OAuth2ClientAuthenticationProvider implements AuthenticationP .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 @@ public final class OAuth2ClientAuthenticationProvider implements AuthenticationP .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 @@ public final class OAuth2ClientAuthenticationProvider implements AuthenticationP 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 { private static final String JWT_CLIENT_AUTHENTICATION_ERROR_URI = "https://datatracker.ietf.org/doc/html/rfc7523#section-3"; private static final Map JCA_ALGORITHM_MAPPINGS; + static { Map mappings = new HashMap<>(); mappings.put(MacAlgorithm.HS256, "HmacSHA256"); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java index 49307b3f..a3dd59e4 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/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 class OAuth2AuthorizationCodeGrantTests { } @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 class OAuth2AuthorizationCodeGrantTests { .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/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java index 5d19403a..ddb862ed 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/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 class OAuth2ClientAuthenticationProviderTests { .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 class OAuth2ClientAuthenticationProviderTests { .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 class OAuth2ClientAuthenticationProviderTests { .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 class OAuth2ClientAuthenticationProviderTests { .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 class OAuth2ClientAuthenticationProviderTests { .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); }); }