From b8a63d3e1c905928a351f2e8b144de7614de9035 Mon Sep 17 00:00:00 2001 From: Dejan Varmedja <114813331+fndejan@users.noreply.github.com> Date: Sat, 10 Jun 2023 23:10:23 +0200 Subject: [PATCH 1/5] Add debug lob entry --- .../ClientSecretAuthenticationProvider.java | 3 +++ .../CodeVerifierAuthenticator.java | 8 ++++++- ...ionCodeRequestAuthenticationValidator.java | 22 ++++++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java index b71065210..38ee0a453 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java @@ -114,6 +114,9 @@ public Authentication authenticate(Authentication authentication) throws Authent String clientSecret = clientAuthentication.getCredentials().toString(); if (!this.passwordEncoder.matches(clientSecret, registeredClient.getClientSecret())) { + if(this.logger.isDebugEnabled()){ + this.logger.debug("Invalid client secret"); + } throwInvalidClient(OAuth2ParameterNames.CLIENT_SECRET); } 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 98a577ee6..9412f4518 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 @@ -96,6 +96,9 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio .get(PkceParameterNames.CODE_CHALLENGE); if (!StringUtils.hasText(codeChallenge)) { if (registeredClient.getClientSettings().isRequireProofKey()) { + if(this.logger.isDebugEnabled()){ + this.logger.debug("Missing code_challenge"); + } throwInvalidGrant(PkceParameterNames.CODE_CHALLENGE); } else { if (this.logger.isTraceEnabled()) { @@ -129,8 +132,11 @@ private static boolean authorizationCodeGrant(Map parameters) { parameters.get(OAuth2ParameterNames.CODE) != null; } - private static boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) { + private boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) { if (!StringUtils.hasText(codeVerifier)) { + if(this.logger.isDebugEnabled()){ + this.logger.debug("Missing code_verifier"); + } return false; } else if ("S256".equals(codeChallengeMethod)) { try { diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java index 2c8dc2ad0..88310f99e 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java @@ -18,6 +18,8 @@ import java.util.Set; import java.util.function.Consumer; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2ErrorCodes; @@ -48,17 +50,19 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator implements Consumer { private static final String ERROR_URI = "https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1"; + private final Log logger = LogFactory.getLog(getClass()); + /** * The default validator for {@link OAuth2AuthorizationCodeRequestAuthenticationToken#getScopes()}. */ - public static final Consumer DEFAULT_SCOPE_VALIDATOR = - OAuth2AuthorizationCodeRequestAuthenticationValidator::validateScope; + public final Consumer DEFAULT_SCOPE_VALIDATOR = + this::validateScope; /** * The default validator for {@link OAuth2AuthorizationCodeRequestAuthenticationToken#getRedirectUri()}. */ - public static final Consumer DEFAULT_REDIRECT_URI_VALIDATOR = - OAuth2AuthorizationCodeRequestAuthenticationValidator::validateRedirectUri; + public final Consumer DEFAULT_REDIRECT_URI_VALIDATOR = + this::validateRedirectUri; private final Consumer authenticationValidator = DEFAULT_REDIRECT_URI_VALIDATOR.andThen(DEFAULT_SCOPE_VALIDATOR); @@ -68,7 +72,7 @@ public void accept(OAuth2AuthorizationCodeRequestAuthenticationContext authentic this.authenticationValidator.accept(authenticationContext); } - private static void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { + private void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = authenticationContext.getAuthentication(); RegisteredClient registeredClient = authenticationContext.getRegisteredClient(); @@ -76,12 +80,15 @@ private static void validateScope(OAuth2AuthorizationCodeRequestAuthenticationCo Set requestedScopes = authorizationCodeRequestAuthentication.getScopes(); Set allowedScopes = registeredClient.getScopes(); if (!requestedScopes.isEmpty() && !allowedScopes.containsAll(requestedScopes)) { + if(this.logger.isDebugEnabled()){ + this.logger.debug("Invalid scope"); + } throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE, authorizationCodeRequestAuthentication, registeredClient); } } - private static void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { + private void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = authenticationContext.getAuthentication(); RegisteredClient registeredClient = authenticationContext.getRegisteredClient(); @@ -124,6 +131,9 @@ private static void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthentica } } if (!validRedirectUri) { + if(this.logger.isDebugEnabled()){ + this.logger.debug("Invalid redirect_uri"); + } throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI, authorizationCodeRequestAuthentication, registeredClient); } From 66be2505cd797b7d94bb44814a7583a643205ced Mon Sep 17 00:00:00 2001 From: Dejan Varmedja <114813331+fndejan@users.noreply.github.com> Date: Sat, 10 Jun 2023 23:15:03 +0200 Subject: [PATCH 2/5] Update debug message --- .../authentication/ClientSecretAuthenticationProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java index 38ee0a453..d19e92a03 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java @@ -115,7 +115,7 @@ public Authentication authenticate(Authentication authentication) throws Authent String clientSecret = clientAuthentication.getCredentials().toString(); if (!this.passwordEncoder.matches(clientSecret, registeredClient.getClientSecret())) { if(this.logger.isDebugEnabled()){ - this.logger.debug("Invalid client secret"); + this.logger.debug("Invalid client_secret"); } throwInvalidClient(OAuth2ParameterNames.CLIENT_SECRET); } From c1eb8745f4666e09eac335e26852dbd90ce4ced6 Mon Sep 17 00:00:00 2001 From: Dejan Varmedja <114813331+fndejan@users.noreply.github.com> Date: Sat, 10 Jun 2023 23:29:57 +0200 Subject: [PATCH 3/5] Extract to private method --- .../authentication/CodeVerifierAuthenticator.java | 13 +++++++------ ...rizationCodeRequestAuthenticationValidator.java | 14 ++++++++------ 2 files changed, 15 insertions(+), 12 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 9412f4518..b1dafc464 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 @@ -96,9 +96,7 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio .get(PkceParameterNames.CODE_CHALLENGE); if (!StringUtils.hasText(codeChallenge)) { if (registeredClient.getClientSettings().isRequireProofKey()) { - if(this.logger.isDebugEnabled()){ - this.logger.debug("Missing code_challenge"); - } + logDebugMessage("Missing code_challenge"); throwInvalidGrant(PkceParameterNames.CODE_CHALLENGE); } else { if (this.logger.isTraceEnabled()) { @@ -134,9 +132,7 @@ private static boolean authorizationCodeGrant(Map parameters) { private boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) { if (!StringUtils.hasText(codeVerifier)) { - if(this.logger.isDebugEnabled()){ - this.logger.debug("Missing code_verifier"); - } + logDebugMessage("Missing code_challenge"); return false; } else if ("S256".equals(codeChallengeMethod)) { try { @@ -162,4 +158,9 @@ private static void throwInvalidGrant(String parameterName) { throw new OAuth2AuthenticationException(error); } + private void logDebugMessage(String logMessage){ + if(this.logger.isDebugEnabled()){ + this.logger.debug(logMessage); + } + } } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java index 88310f99e..607cb9c72 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java @@ -80,9 +80,7 @@ private void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext a Set requestedScopes = authorizationCodeRequestAuthentication.getScopes(); Set allowedScopes = registeredClient.getScopes(); if (!requestedScopes.isEmpty() && !allowedScopes.containsAll(requestedScopes)) { - if(this.logger.isDebugEnabled()){ - this.logger.debug("Invalid scope"); - } + logDebugMessage("Invalid scope"); throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE, authorizationCodeRequestAuthentication, registeredClient); } @@ -131,9 +129,7 @@ private void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationCon } } if (!validRedirectUri) { - if(this.logger.isDebugEnabled()){ - this.logger.debug("Invalid redirect_uri"); - } + logDebugMessage("Invalid redirect_uri"); throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI, authorizationCodeRequestAuthentication, registeredClient); } @@ -206,4 +202,10 @@ private static void throwError(OAuth2Error error, String parameterName, throw new OAuth2AuthorizationCodeRequestAuthenticationException(error, authorizationCodeRequestAuthenticationResult); } + private void logDebugMessage(String logMessage){ + if(this.logger.isDebugEnabled()){ + this.logger.debug(logMessage); + } + } + } From c8f08455cfcd1ba848bd35db83cf4068792070bd Mon Sep 17 00:00:00 2001 From: Dejan Varmedja <114813331+fndejan@users.noreply.github.com> Date: Sat, 10 Jun 2023 23:32:25 +0200 Subject: [PATCH 4/5] Remove extra space --- .../OAuth2AuthorizationCodeRequestAuthenticationValidator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java index 607cb9c72..168106e56 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java @@ -51,7 +51,6 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator impleme private static final String ERROR_URI = "https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1"; private final Log logger = LogFactory.getLog(getClass()); - /** * The default validator for {@link OAuth2AuthorizationCodeRequestAuthenticationToken#getScopes()}. */ From ee342938b6ddfc5145b889302216ba10136d4828 Mon Sep 17 00:00:00 2001 From: Dejan Varmedja <114813331+fndejan@users.noreply.github.com> Date: Sat, 10 Jun 2023 23:36:28 +0200 Subject: [PATCH 5/5] Fix duplicate debug message --- .../authorization/authentication/CodeVerifierAuthenticator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b1dafc464..61f769d25 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 @@ -132,7 +132,7 @@ private static boolean authorizationCodeGrant(Map parameters) { private boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) { if (!StringUtils.hasText(codeVerifier)) { - logDebugMessage("Missing code_challenge"); + logDebugMessage("Missing code_verifier"); return false; } else if ("S256".equals(codeChallengeMethod)) { try {