From 8d54f16a13f8049841f4b6877dbb31b92e833632 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Tue, 13 Jun 2023 15:44:36 -0500 Subject: [PATCH] Polish additional logging Issue gh-1245, gh-1246, gh-1247, gh-1248 --- .../ClientSecretAuthenticationProvider.java | 6 ++-- .../CodeVerifierAuthenticator.java | 18 +++++----- ...ionCodeRequestAuthenticationValidator.java | 36 +++++++++++-------- 3 files changed, 35 insertions(+), 25 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 d19e92a0..50f0afb8 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 @@ -20,6 +20,7 @@ import java.time.Instant; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -114,8 +115,9 @@ public final class ClientSecretAuthenticationProvider implements AuthenticationP String clientSecret = clientAuthentication.getCredentials().toString(); if (!this.passwordEncoder.matches(clientSecret, registeredClient.getClientSecret())) { - if(this.logger.isDebugEnabled()){ - this.logger.debug("Invalid client_secret"); + if (this.logger.isDebugEnabled()) { + this.logger.debug(LogMessage.format("Invalid request: client_secret does not match" + + " for registered client '%s'", registeredClient.getId())); } 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 61f769d2..a5664ee1 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-2023 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. @@ -24,6 +24,7 @@ import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogMessage; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; @@ -96,7 +97,10 @@ final class CodeVerifierAuthenticator { .get(PkceParameterNames.CODE_CHALLENGE); if (!StringUtils.hasText(codeChallenge)) { if (registeredClient.getClientSettings().isRequireProofKey()) { - logDebugMessage("Missing code_challenge"); + if (this.logger.isDebugEnabled()) { + this.logger.debug(LogMessage.format("Invalid request: code_challenge is required" + + " for registered client '%s'", registeredClient.getId())); + } throwInvalidGrant(PkceParameterNames.CODE_CHALLENGE); } else { if (this.logger.isTraceEnabled()) { @@ -114,6 +118,10 @@ final class CodeVerifierAuthenticator { .get(PkceParameterNames.CODE_CHALLENGE_METHOD); String codeVerifier = (String) parameters.get(PkceParameterNames.CODE_VERIFIER); if (!codeVerifierValid(codeVerifier, codeChallenge, codeChallengeMethod)) { + if (this.logger.isDebugEnabled()) { + this.logger.debug(LogMessage.format("Invalid request: code_verifier is missing or invalid" + + " for registered client '%s'", registeredClient.getId())); + } throwInvalidGrant(PkceParameterNames.CODE_VERIFIER); } @@ -132,7 +140,6 @@ final class CodeVerifierAuthenticator { private boolean codeVerifierValid(String codeVerifier, String codeChallenge, String codeChallengeMethod) { if (!StringUtils.hasText(codeVerifier)) { - logDebugMessage("Missing code_verifier"); return false; } else if ("S256".equals(codeChallengeMethod)) { try { @@ -158,9 +165,4 @@ final class CodeVerifierAuthenticator { 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 168106e5..5ad554e3 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 @@ -20,6 +20,8 @@ import java.util.function.Consumer; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + +import org.springframework.core.log.LogMessage; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.OAuth2ErrorCodes; @@ -49,19 +51,19 @@ import org.springframework.web.util.UriComponentsBuilder; */ 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 static final Log LOGGER = LogFactory.getLog(OAuth2AuthorizationCodeRequestAuthenticationValidator.class); - private final Log logger = LogFactory.getLog(getClass()); /** * The default validator for {@link OAuth2AuthorizationCodeRequestAuthenticationToken#getScopes()}. */ - public final Consumer DEFAULT_SCOPE_VALIDATOR = - this::validateScope; + public static final Consumer DEFAULT_SCOPE_VALIDATOR = + OAuth2AuthorizationCodeRequestAuthenticationValidator::validateScope; /** * The default validator for {@link OAuth2AuthorizationCodeRequestAuthenticationToken#getRedirectUri()}. */ - public final Consumer DEFAULT_REDIRECT_URI_VALIDATOR = - this::validateRedirectUri; + public static final Consumer DEFAULT_REDIRECT_URI_VALIDATOR = + OAuth2AuthorizationCodeRequestAuthenticationValidator::validateRedirectUri; private final Consumer authenticationValidator = DEFAULT_REDIRECT_URI_VALIDATOR.andThen(DEFAULT_SCOPE_VALIDATOR); @@ -71,7 +73,7 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator impleme this.authenticationValidator.accept(authenticationContext); } - private void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { + private static void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = authenticationContext.getAuthentication(); RegisteredClient registeredClient = authenticationContext.getRegisteredClient(); @@ -79,13 +81,16 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator impleme Set requestedScopes = authorizationCodeRequestAuthentication.getScopes(); Set allowedScopes = registeredClient.getScopes(); if (!requestedScopes.isEmpty() && !allowedScopes.containsAll(requestedScopes)) { - logDebugMessage("Invalid scope"); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug(LogMessage.format("Invalid request: requested scope is not allowed" + + " for registered client '%s'", registeredClient.getId())); + } throwError(OAuth2ErrorCodes.INVALID_SCOPE, OAuth2ParameterNames.SCOPE, authorizationCodeRequestAuthentication, registeredClient); } } - private void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { + private static void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) { OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = authenticationContext.getAuthentication(); RegisteredClient registeredClient = authenticationContext.getRegisteredClient(); @@ -100,6 +105,10 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator impleme requestedRedirect = UriComponentsBuilder.fromUriString(requestedRedirectUri).build(); } catch (Exception ex) { } if (requestedRedirect == null || requestedRedirect.getFragment() != null) { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug(LogMessage.format("Invalid request: redirect_uri is missing or contains a fragment" + + " for registered client '%s'", registeredClient.getId())); + } throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI, authorizationCodeRequestAuthentication, registeredClient); } @@ -128,7 +137,10 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator impleme } } if (!validRedirectUri) { - logDebugMessage("Invalid redirect_uri"); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug(LogMessage.format("Invalid request: redirect_uri does not match" + + " for registered client '%s'", registeredClient.getId())); + } throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI, authorizationCodeRequestAuthentication, registeredClient); } @@ -201,10 +213,4 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator impleme throw new OAuth2AuthorizationCodeRequestAuthenticationException(error, authorizationCodeRequestAuthenticationResult); } - private void logDebugMessage(String logMessage){ - if(this.logger.isDebugEnabled()){ - this.logger.debug(logMessage); - } - } - }