Adds paramater name to JwtClientAssertion validation error Fixes spri…

…ng-projectsgh-1449
felix-hellman · Nov 17, 2023 · a19a3a1 · a19a3a1
1 parent 74058e5
commit a19a3a1
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 9 deletions.
diff --git a/...h2/server/authorization/web/authentication/JwtClientAssertionAuthenticationConverter.java b/...h2/server/authorization/web/authentication/JwtClientAssertionAuthenticationConverter.java
@@ -19,10 +19,13 @@
 
 import jakarta.servlet.http.HttpServletRequest;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.lang.Nullable;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
+import org.springframework.security.oauth2.core.OAuth2Error;
 import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken;
@@ -44,7 +47,6 @@
 public final class JwtClientAssertionAuthenticationConverter implements AuthenticationConverter {
 	private static final ClientAuthenticationMethod JWT_CLIENT_ASSERTION_AUTHENTICATION_METHOD =
 			new ClientAuthenticationMethod("urn:ietf:params:oauth:client-assertion-type:jwt-bearer");
-
 	@Nullable
 	@Override
 	public Authentication convert(HttpServletRequest request) {
@@ -58,7 +60,7 @@ public Authentication convert(HttpServletRequest request) {
 		// client_assertion_type (REQUIRED)
 		String clientAssertionType = parameters.getFirst(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE);
 		if (parameters.get(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE).size() != 1) {
-			throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST);
+			throwException(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE);
 		}
 		if (!JWT_CLIENT_ASSERTION_AUTHENTICATION_METHOD.getValue().equals(clientAssertionType)) {
 			return null;
@@ -67,14 +69,14 @@ public Authentication convert(HttpServletRequest request) {
 		// client_assertion (REQUIRED)
 		String jwtAssertion = parameters.getFirst(OAuth2ParameterNames.CLIENT_ASSERTION);
 		if (parameters.get(OAuth2ParameterNames.CLIENT_ASSERTION).size() != 1) {
-			throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST);
+			throwException(OAuth2ParameterNames.CLIENT_ASSERTION);
 		}
 
 		// client_id (OPTIONAL as per specification but REQUIRED by this implementation)
 		String clientId = parameters.getFirst(OAuth2ParameterNames.CLIENT_ID);
 		if (!StringUtils.hasText(clientId) ||
 				parameters.get(OAuth2ParameterNames.CLIENT_ID).size() != 1) {
-			throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST);
+			throwException(OAuth2ParameterNames.CLIENT_ID);
 		}
 
 		Map<String, Object> additionalParameters = OAuth2EndpointUtils.getParametersIfMatchesAuthorizationCodeGrantRequest(request,
@@ -86,4 +88,8 @@ public Authentication convert(HttpServletRequest request) {
 				jwtAssertion, additionalParameters);
 	}
 
+	public void throwException(String parameterName) {
+		throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST), "Required parameter %s is missing or invalid".formatted(parameterName));
+	}
+
 }
diff --git a/...rver/authorization/web/authentication/JwtClientAssertionAuthenticationConverterTests.java b/...rver/authorization/web/authentication/JwtClientAssertionAuthenticationConverterTests.java
@@ -60,7 +60,7 @@ public void convertWhenMultipleClientAssertionTypeThenInvalidRequestError() {
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE, JWT_BEARER_TYPE);
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE, "other-client-assertion-type");
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION, "jwt-assertion");
-		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST);
+		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.CLIENT_ASSERTION_TYPE);
 	}
 
 	@Test
@@ -78,15 +78,15 @@ public void convertWhenMultipleClientAssertionThenInvalidRequestError() {
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE, JWT_BEARER_TYPE);
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION, "jwt-assertion");
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION, "other-jwt-assertion");
-		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST);
+		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.CLIENT_ASSERTION);
 	}
 
 	@Test
 	public void convertWhenMissingClientIdThenInvalidRequestError() {
 		MockHttpServletRequest request = new MockHttpServletRequest();
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE, JWT_BEARER_TYPE);
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION, "jwt-assertion");
-		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST);
+		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.CLIENT_ID);
 	}
 
 	@Test
@@ -96,7 +96,7 @@ public void convertWhenMultipleClientIdThenInvalidRequestError() {
 		request.addParameter(OAuth2ParameterNames.CLIENT_ASSERTION, "jwt-assertion");
 		request.addParameter(OAuth2ParameterNames.CLIENT_ID, "client-1");
 		request.addParameter(OAuth2ParameterNames.CLIENT_ID, "client-2");
-		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST);
+		assertThrown(request, OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.CLIENT_ID);
 	}
 
 	@Test
@@ -121,9 +121,10 @@ public void convertWhenJwtAssertionThenReturnClientAuthenticationToken() {
 						entry("custom-param-2", new String[] {"custom-value-1", "custom-value-2"}));
 	}
 
-	private void assertThrown(MockHttpServletRequest request, String errorCode) {
+	private void assertThrown(MockHttpServletRequest request, String errorCode, String parameter) {
 		assertThatThrownBy(() -> this.converter.convert(request))
 				.isInstanceOf(OAuth2AuthenticationException.class)
+				.hasMessageContaining(parameter)
 				.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
 				.extracting("errorCode")
 				.isEqualTo(errorCode);