Fix NullPointerException Handling and Enhance Logging in OAuth2 Authorization

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

@@ -1,18 +1,3 @@
-/*
- * Copyright 2020-2024 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.
- * You may obtain a copy of the License at
- *
- *      https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
 package org.springframework.security.oauth2.server.authorization.authentication;
 
 import java.nio.charset.StandardCharsets;
@@ -86,48 +71,53 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio
 			throwInvalidGrant(OAuth2ParameterNames.CODE);
 		}
 
-		if (this.logger.isTraceEnabled()) {
-			this.logger.trace("Retrieved authorization with authorization code");
+		if (logger.isTraceEnabled()) {
+			logger.trace("Retrieved authorization with authorization code");
 		}
 
-		OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(
+        assert authorization != null;
+        OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute(
 				OAuth2AuthorizationRequest.class.getName());
+		if (authorizationRequest == null) {
+			throwInvalidGrant("Invalid authorization request");
+		}
 
-		String codeChallenge = (String) authorizationRequest.getAdditionalParameters()
+        assert authorizationRequest != null;
+        String codeChallenge = (String) authorizationRequest.getAdditionalParameters()
 				.get(PkceParameterNames.CODE_CHALLENGE);
 		String codeVerifier = (String) parameters.get(PkceParameterNames.CODE_VERIFIER);
 		if (!StringUtils.hasText(codeChallenge)) {
 			if (registeredClient.getClientSettings().isRequireProofKey() ||
 					StringUtils.hasText(codeVerifier)) {
-				if (this.logger.isDebugEnabled()) {
-					this.logger.debug(LogMessage.format("Invalid request: code_challenge is required" +
+				if (logger.isDebugEnabled()) {
+					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()) {
-					this.logger.trace("Did not authenticate code verifier since requireProofKey=false");
+				if (logger.isTraceEnabled()) {
+					logger.trace("Did not authenticate code verifier since requireProofKey=false");
 				}
 				return false;
 			}
 		}
 
-		if (this.logger.isTraceEnabled()) {
-			this.logger.trace("Validated code verifier parameters");
+		if (logger.isTraceEnabled()) {
+			logger.trace("Validated code verifier parameters");
 		}
 
 		String codeChallengeMethod = (String) authorizationRequest.getAdditionalParameters()
 				.get(PkceParameterNames.CODE_CHALLENGE_METHOD);
 		if (!codeVerifierValid(codeVerifier, codeChallenge, codeChallengeMethod)) {
-			if (this.logger.isDebugEnabled()) {
-				this.logger.debug(LogMessage.format("Invalid request: code_verifier is missing or invalid" +
+			if (logger.isDebugEnabled()) {
+				logger.debug(LogMessage.format("Invalid request: code_verifier is missing or invalid" +
 						" for registered client '%s'", registeredClient.getId()));
 			}
 			throwInvalidGrant(PkceParameterNames.CODE_VERIFIER);
 		}
 
-		if (this.logger.isTraceEnabled()) {
-			this.logger.trace("Authenticated code verifier");
+		if (logger.isTraceEnabled()) {
+			logger.trace("Authenticated code verifier");
 		}
 
 		return true;
@@ -149,9 +139,9 @@ private boolean codeVerifierValid(String codeVerifier, String codeChallenge, Str
 				String encodedVerifier = Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
 				return encodedVerifier.equals(codeChallenge);
 			} catch (NoSuchAlgorithmException ex) {
-				// It is unlikely that SHA-256 is not available on the server. If it is not available,
-				// there will likely be bigger issues as well. We default to SERVER_ERROR.
-				throw new OAuth2AuthenticationException(OAuth2ErrorCodes.SERVER_ERROR);
+				throw new OAuth2AuthenticationException(
+						new OAuth2Error(OAuth2ErrorCodes.SERVER_ERROR,
+								"Failed to verify code verifier: SHA-256 algorithm not available", null));
 			}
 		}
 		return false;
@@ -165,5 +155,4 @@ private static void throwInvalidGrant(String parameterName) {
 		);
 		throw new OAuth2AuthenticationException(error);
 	}
-
 }

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

@@ -129,4 +129,21 @@ public Map<String, Object> getAdditionalParameters() {
 		return this.additionalParameters;
 	}
 
+	/**
+	 * Indicates if the client secret is needed for client authentication.
+	 * Checks if the authentication method is either 'client_secret_basic' or 'client_secret_post',
+	 * both requiring the client secret for authentication.
+	 * @return {@code true} if the client secret is required, {@code false} otherwise
+	 */
+	public boolean isClientSecretRequired() {
+		return getClientAuthenticationMethod() == ClientAuthenticationMethod.CLIENT_SECRET_BASIC ||
+				getClientAuthenticationMethod() == ClientAuthenticationMethod.CLIENT_SECRET_POST;}
+
+	/**
+	 * Returns the client secret.
+	 *
+	 * @return the client secret
+	 */
+	public Object getClientSecret() {
+		return getCredentials();}
 }

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java

@@ -16,6 +16,7 @@
 package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers;
 
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -250,7 +251,7 @@ public OAuth2AuthorizationServerConfigurer deviceVerificationEndpoint(Customizer
 	public OAuth2AuthorizationServerConfigurer oidc(Customizer<OidcConfigurer> oidcCustomizer) {
 		OidcConfigurer oidcConfigurer = getConfigurer(OidcConfigurer.class);
 		if (oidcConfigurer == null) {
-			addConfigurer(OidcConfigurer.class, new OidcConfigurer(this::postProcess));
+			addConfigurer(new OidcConfigurer(this::postProcess));
 			oidcConfigurer = getConfigurer(OidcConfigurer.class);
 		}
 		oidcCustomizer.customize(oidcConfigurer);
@@ -369,28 +370,28 @@ private <T> T getConfigurer(Class<T> type) {
 		return (T) this.configurers.get(type);
 	}
 
-	private <T extends AbstractOAuth2Configurer> void addConfigurer(Class<T> configurerType, T configurer) {
-		this.configurers.put(configurerType, configurer);
+	private <T extends AbstractOAuth2Configurer> void addConfigurer(T configurer) {
+		this.configurers.put(OidcConfigurer.class, configurer);
 	}
 
 	private <T extends AbstractOAuth2Configurer> RequestMatcher getRequestMatcher(Class<T> configurerType) {
 		T configurer = getConfigurer(configurerType);
 		return configurer != null ? configurer.getRequestMatcher() : null;
 	}
 
+
 	private static void validateAuthorizationServerSettings(AuthorizationServerSettings authorizationServerSettings) {
 		if (authorizationServerSettings.getIssuer() != null) {
 			URI issuerUri;
 			try {
 				issuerUri = new URI(authorizationServerSettings.getIssuer());
-				issuerUri.toURL();
-			} catch (Exception ex) {
+				// rfc8414 https://datatracker.ietf.org/doc/html/rfc8414#section-2
+				if (issuerUri.getRawQuery() != null || issuerUri.getRawFragment() != null) {
+					throw new IllegalArgumentException("issuer URL cannot contain query or fragment component");
+				}
+			} catch (URISyntaxException ex) {
 				throw new IllegalArgumentException("issuer must be a valid URL", ex);
 			}
-			// rfc8414 https://datatracker.ietf.org/doc/html/rfc8414#section-2
-			if (issuerUri.getQuery() != null || issuerUri.getFragment() != null) {
-				throw new IllegalArgumentException("issuer cannot contain query or fragment component");
-			}
 		}
 	}
 

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilter.java

@@ -119,6 +119,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
 			}
 			if (authenticationRequest != null) {
 				validateClientIdentifier(authenticationRequest);
+				validateClientAuthenticationMethod(authenticationRequest);
 				Authentication authenticationResult = this.authenticationManager.authenticate(authenticationRequest);
 				this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, authenticationResult);
 			}
@@ -222,5 +223,13 @@ private static void validateClientIdentifier(Authentication authentication) {
 			}
 		}
 	}
+	private static void validateClientAuthenticationMethod(Authentication authentication) {
+		if (!(authentication instanceof OAuth2ClientAuthenticationToken clientAuthentication)) {
+			return;
+		}
 
+		if (clientAuthentication.isClientSecretRequired() && clientAuthentication.getClientSecret() == null) {
+			throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST));
+		}
+	}
 }