Polish additional logging

Issue gh-1245, gh-1246, gh-1247, gh-1248

Author: Steve Riesenberg

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/ClientSecretAuthenticationProvider.java

@@ -20,6 +20,7 @@
 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 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");
+			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);
 		}

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 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 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio
 				.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 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio
 				.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 @@ private static boolean authorizationCodeGrant(Map<String, Object> parameters) {
 
 	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 @@ private static void throwInvalidGrant(String parameterName) {
 		throw new OAuth2AuthenticationException(error);
 	}
 
-	private void logDebugMessage(String logMessage){
-		if(this.logger.isDebugEnabled()){
-			this.logger.debug(logMessage);
-		}
-	}
 }

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java

@@ -20,6 +20,8 @@
 
 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 @@
  */
 public final class OAuth2AuthorizationCodeRequestAuthenticationValidator implements Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> {
 	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<OAuth2AuthorizationCodeRequestAuthenticationContext> DEFAULT_SCOPE_VALIDATOR =
-			this::validateScope;
+	public static final Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> DEFAULT_SCOPE_VALIDATOR =
+			OAuth2AuthorizationCodeRequestAuthenticationValidator::validateScope;
 
 	/**
 	 * The default validator for {@link OAuth2AuthorizationCodeRequestAuthenticationToken#getRedirectUri()}.
 	 */
-	public final Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> DEFAULT_REDIRECT_URI_VALIDATOR =
-			this::validateRedirectUri;
+	public static final Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> DEFAULT_REDIRECT_URI_VALIDATOR =
+			OAuth2AuthorizationCodeRequestAuthenticationValidator::validateRedirectUri;
 
 	private final Consumer<OAuth2AuthorizationCodeRequestAuthenticationContext> authenticationValidator =
 			DEFAULT_REDIRECT_URI_VALIDATOR.andThen(DEFAULT_SCOPE_VALIDATOR);
@@ -71,21 +73,24 @@ public void accept(OAuth2AuthorizationCodeRequestAuthenticationContext authentic
 		this.authenticationValidator.accept(authenticationContext);
 	}
 
-	private void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) {
+	private static void validateScope(OAuth2AuthorizationCodeRequestAuthenticationContext authenticationContext) {
 		OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication =
 				authenticationContext.getAuthentication();
 		RegisteredClient registeredClient = authenticationContext.getRegisteredClient();
 
 		Set<String> requestedScopes = authorizationCodeRequestAuthentication.getScopes();
 		Set<String> 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 @@ private void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationCon
 				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 @@ private void validateRedirectUri(OAuth2AuthorizationCodeRequestAuthenticationCon
 					}
 				}
 				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 @@ 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);
-		}
-	}
-
 }