Browse Source

Missing response_type in POST authorization request returns invalid_request

Closes gh-2226
Joe Grandja 4 weeks ago
parent
commit
9ebc52baba

+ 15 - 8
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java

@@ -39,6 +39,7 @@ import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
 import org.springframework.security.oauth2.core.OAuth2Error;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
+import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationException;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationProvider;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationToken;
@@ -150,18 +151,24 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte
 				HttpMethod.GET.name());
 		RequestMatcher authorizationRequestPostMatcher = new AntPathRequestMatcher(authorizationEndpointUri,
 				HttpMethod.POST.name());
-
-		RequestMatcher responseTypeParameterMatcher = (
-				request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null;
-
+		RequestMatcher authorizationConsentMatcher = createAuthorizationConsentMatcher(authorizationEndpointUri);
 		RequestMatcher authorizationRequestMatcher = new OrRequestMatcher(authorizationRequestGetMatcher,
-				new AndRequestMatcher(authorizationRequestPostMatcher, responseTypeParameterMatcher));
-		RequestMatcher authorizationConsentMatcher = new AndRequestMatcher(authorizationRequestPostMatcher,
-				new NegatedRequestMatcher(responseTypeParameterMatcher));
-
+				new AndRequestMatcher(authorizationRequestPostMatcher,
+						new NegatedRequestMatcher(authorizationConsentMatcher)));
 		return new OrRequestMatcher(authorizationRequestMatcher, authorizationConsentMatcher);
 	}
 
+	private static RequestMatcher createAuthorizationConsentMatcher(String authorizationEndpointUri) {
+		final RequestMatcher authorizationConsentPostMatcher = new AntPathRequestMatcher(authorizationEndpointUri,
+				HttpMethod.POST.name());
+		return (request) -> authorizationConsentPostMatcher.matches(request)
+				&& request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) == null
+				&& request.getParameter(OAuth2ParameterNames.REQUEST_URI) == null
+				&& request.getParameter(OAuth2ParameterNames.REDIRECT_URI) == null
+				&& request.getParameter(PkceParameterNames.CODE_CHALLENGE) == null
+				&& request.getParameter(PkceParameterNames.CODE_CHALLENGE_METHOD) == null;
+	}
+
 	@Override
 	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
 			throws ServletException, IOException {

+ 4 - 8
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java

@@ -43,8 +43,6 @@ import org.springframework.security.oauth2.server.authorization.settings.Authori
 import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter;
 import org.springframework.security.oauth2.server.authorization.web.OAuth2PushedAuthorizationRequestEndpointFilter;
 import org.springframework.security.web.authentication.AuthenticationConverter;
-import org.springframework.security.web.util.matcher.AndRequestMatcher;
-import org.springframework.security.web.util.matcher.OrRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.MultiValueMap;
@@ -197,12 +195,10 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationConverter impleme
 	}
 
 	private static RequestMatcher createDefaultRequestMatcher() {
-		RequestMatcher getMethodMatcher = (request) -> "GET".equals(request.getMethod());
-		RequestMatcher postMethodMatcher = (request) -> "POST".equals(request.getMethod());
-		RequestMatcher responseTypeParameterMatcher = (
-				request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null;
-		return new OrRequestMatcher(getMethodMatcher,
-				new AndRequestMatcher(postMethodMatcher, responseTypeParameterMatcher));
+		final RequestMatcher authorizationConsentMatcher = OAuth2AuthorizationConsentAuthenticationConverter
+			.createDefaultRequestMatcher();
+		return (request) -> "GET".equals(request.getMethod())
+				|| ("POST".equals(request.getMethod()) && !authorizationConsentMatcher.matches(request));
 	}
 
 	private static void throwError(String errorCode, String parameterName) {

+ 8 - 7
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java

@@ -29,12 +29,11 @@ import org.springframework.security.core.context.SecurityContextHolder;
 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.core.endpoint.PkceParameterNames;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationException;
 import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationToken;
 import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter;
 import org.springframework.security.web.authentication.AuthenticationConverter;
-import org.springframework.security.web.util.matcher.AndRequestMatcher;
-import org.springframework.security.web.util.matcher.NegatedRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
@@ -105,11 +104,13 @@ public final class OAuth2AuthorizationConsentAuthenticationConverter implements
 				additionalParameters);
 	}
 
-	private static RequestMatcher createDefaultRequestMatcher() {
-		RequestMatcher postMethodMatcher = (request) -> "POST".equals(request.getMethod());
-		RequestMatcher responseTypeParameterMatcher = (
-				request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null;
-		return new AndRequestMatcher(postMethodMatcher, new NegatedRequestMatcher(responseTypeParameterMatcher));
+	static RequestMatcher createDefaultRequestMatcher() {
+		return (request) -> "POST".equals(request.getMethod())
+				&& request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) == null
+				&& request.getParameter(OAuth2ParameterNames.REQUEST_URI) == null
+				&& request.getParameter(OAuth2ParameterNames.REDIRECT_URI) == null
+				&& request.getParameter(PkceParameterNames.CODE_CHALLENGE) == null
+				&& request.getParameter(PkceParameterNames.CODE_CHALLENGE_METHOD) == null;
 	}
 
 	private static void throwError(String errorCode, String parameterName) {

+ 13 - 2
oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java

@@ -206,6 +206,17 @@ public class OAuth2AuthorizationEndpointFilterTests {
 				});
 	}
 
+	// gh-2226
+	@Test
+	public void doFilterWhenPostAuthorizationRequestMissingResponseTypeThenInvalidRequestError() throws Exception {
+		doFilterWhenAuthorizationRequestInvalidParameterThenError(TestRegisteredClients.registeredClient().build(),
+				OAuth2ParameterNames.RESPONSE_TYPE, OAuth2ErrorCodes.INVALID_REQUEST, (request) -> {
+					request.setMethod("POST");
+					request.removeParameter(OAuth2ParameterNames.RESPONSE_TYPE);
+					request.setQueryString(null);
+				});
+	}
+
 	@Test
 	public void doFilterWhenAuthorizationRequestMultipleResponseTypeThenInvalidRequestError() throws Exception {
 		doFilterWhenAuthorizationRequestInvalidParameterThenError(TestRegisteredClients.registeredClient().build(),
@@ -645,7 +656,7 @@ public class OAuth2AuthorizationEndpointFilterTests {
 
 		this.filter.doFilter(request, response, filterChain);
 
-		verify(this.authenticationManager).authenticate(any());
+		verify(this.authenticationManager).authenticate(any(OAuth2AuthorizationCodeRequestAuthenticationToken.class));
 		verifyNoInteractions(filterChain);
 
 		assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());
@@ -674,7 +685,7 @@ public class OAuth2AuthorizationEndpointFilterTests {
 
 		this.filter.doFilter(request, response, filterChain);
 
-		verify(this.authenticationManager).authenticate(any());
+		verify(this.authenticationManager).authenticate(any(OAuth2AuthorizationCodeRequestAuthenticationToken.class));
 		verifyNoInteractions(filterChain);
 
 		assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());