diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index f8b44d755f..dddc84336c 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -40,6 +40,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; @@ -151,18 +152,24 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte .matcher(HttpMethod.GET, authorizationEndpointUri); RequestMatcher authorizationRequestPostMatcher = PathPatternRequestMatcher.withDefaults() .matcher(HttpMethod.POST, authorizationEndpointUri); - - 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 = PathPatternRequestMatcher.withDefaults() + .matcher(HttpMethod.POST, authorizationEndpointUri); + 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 { diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java index 2d6e53cd64..f5aac6dac6 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java @@ -44,8 +44,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; @@ -198,12 +196,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) { diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java index 01250a7267..e64872580b 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java @@ -30,12 +30,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; @@ -106,11 +105,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) { diff --git a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index a1ffda8dcb..063ecaa759 100644 --- a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -207,6 +207,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(), @@ -646,7 +657,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()); @@ -675,7 +686,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());