Skip to content

Commit 90855aa

Browse files
jgrandjajgrandja
committed
Missing response_type in POST authorization request returns invalid_request
Issue spring-projects/spring-authorization-server#2226
1 parent 9f7e92d commit 90855aa

File tree

4 files changed

+40
-25
lines changed

4 files changed

+40
-25
lines changed

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.springframework.security.oauth2.core.OAuth2Error;
4141
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse;
4242
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
43+
import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
4344
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationException;
4445
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationProvider;
4546
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationToken;
@@ -151,18 +152,24 @@ private static RequestMatcher createDefaultRequestMatcher(String authorizationEn
151152
.matcher(HttpMethod.GET, authorizationEndpointUri);
152153
RequestMatcher authorizationRequestPostMatcher = PathPatternRequestMatcher.withDefaults()
153154
.matcher(HttpMethod.POST, authorizationEndpointUri);
154-
155-
RequestMatcher responseTypeParameterMatcher = (
156-
request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null;
157-
155+
RequestMatcher authorizationConsentMatcher = createAuthorizationConsentMatcher(authorizationEndpointUri);
158156
RequestMatcher authorizationRequestMatcher = new OrRequestMatcher(authorizationRequestGetMatcher,
159-
new AndRequestMatcher(authorizationRequestPostMatcher, responseTypeParameterMatcher));
160-
RequestMatcher authorizationConsentMatcher = new AndRequestMatcher(authorizationRequestPostMatcher,
161-
new NegatedRequestMatcher(responseTypeParameterMatcher));
162-
157+
new AndRequestMatcher(authorizationRequestPostMatcher,
158+
new NegatedRequestMatcher(authorizationConsentMatcher)));
163159
return new OrRequestMatcher(authorizationRequestMatcher, authorizationConsentMatcher);
164160
}
165161

162+
private static RequestMatcher createAuthorizationConsentMatcher(String authorizationEndpointUri) {
163+
final RequestMatcher authorizationConsentPostMatcher = PathPatternRequestMatcher.withDefaults()
164+
.matcher(HttpMethod.POST, authorizationEndpointUri);
165+
return (request) -> authorizationConsentPostMatcher.matches(request)
166+
&& request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) == null
167+
&& request.getParameter(OAuth2ParameterNames.REQUEST_URI) == null
168+
&& request.getParameter(OAuth2ParameterNames.REDIRECT_URI) == null
169+
&& request.getParameter(PkceParameterNames.CODE_CHALLENGE) == null
170+
&& request.getParameter(PkceParameterNames.CODE_CHALLENGE_METHOD) == null;
171+
}
172+
166173
@Override
167174
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
168175
throws ServletException, IOException {

‎oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter;
4545
import org.springframework.security.oauth2.server.authorization.web.OAuth2PushedAuthorizationRequestEndpointFilter;
4646
import org.springframework.security.web.authentication.AuthenticationConverter;
47-
import org.springframework.security.web.util.matcher.AndRequestMatcher;
48-
import org.springframework.security.web.util.matcher.OrRequestMatcher;
4947
import org.springframework.security.web.util.matcher.RequestMatcher;
5048
import org.springframework.util.CollectionUtils;
5149
import org.springframework.util.MultiValueMap;
@@ -198,12 +196,10 @@ private boolean isPushedAuthorizationRequest(HttpServletRequest request) {
198196
}
199197

200198
private static RequestMatcher createDefaultRequestMatcher() {
201-
RequestMatcher getMethodMatcher = (request) -> "GET".equals(request.getMethod());
202-
RequestMatcher postMethodMatcher = (request) -> "POST".equals(request.getMethod());
203-
RequestMatcher responseTypeParameterMatcher = (
204-
request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null;
205-
return new OrRequestMatcher(getMethodMatcher,
206-
new AndRequestMatcher(postMethodMatcher, responseTypeParameterMatcher));
199+
final RequestMatcher authorizationConsentMatcher = OAuth2AuthorizationConsentAuthenticationConverter
200+
.createDefaultRequestMatcher();
201+
return (request) -> "GET".equals(request.getMethod())
202+
|| ("POST".equals(request.getMethod()) && !authorizationConsentMatcher.matches(request));
207203
}
208204

209205
private static void throwError(String errorCode, String parameterName) {

‎oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationConsentAuthenticationConverter.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@
3030
import org.springframework.security.oauth2.core.OAuth2Error;
3131
import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
3232
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
33+
import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
3334
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationException;
3435
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationConsentAuthenticationToken;
3536
import org.springframework.security.oauth2.server.authorization.web.OAuth2AuthorizationEndpointFilter;
3637
import org.springframework.security.web.authentication.AuthenticationConverter;
37-
import org.springframework.security.web.util.matcher.AndRequestMatcher;
38-
import org.springframework.security.web.util.matcher.NegatedRequestMatcher;
3938
import org.springframework.security.web.util.matcher.RequestMatcher;
4039
import org.springframework.util.MultiValueMap;
4140
import org.springframework.util.StringUtils;
@@ -106,11 +105,13 @@ public Authentication convert(HttpServletRequest request) {
106105
additionalParameters);
107106
}
108107

109-
private static RequestMatcher createDefaultRequestMatcher() {
110-
RequestMatcher postMethodMatcher = (request) -> "POST".equals(request.getMethod());
111-
RequestMatcher responseTypeParameterMatcher = (
112-
request) -> request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null;
113-
return new AndRequestMatcher(postMethodMatcher, new NegatedRequestMatcher(responseTypeParameterMatcher));
108+
static RequestMatcher createDefaultRequestMatcher() {
109+
return (request) -> "POST".equals(request.getMethod())
110+
&& request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) == null
111+
&& request.getParameter(OAuth2ParameterNames.REQUEST_URI) == null
112+
&& request.getParameter(OAuth2ParameterNames.REDIRECT_URI) == null
113+
&& request.getParameter(PkceParameterNames.CODE_CHALLENGE) == null
114+
&& request.getParameter(PkceParameterNames.CODE_CHALLENGE_METHOD) == null;
114115
}
115116

116117
private static void throwError(String errorCode, String parameterName) {

‎oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,17 @@ public void doFilterWhenAuthorizationRequestMissingResponseTypeThenInvalidReques
207207
});
208208
}
209209

210+
// gh-2226
211+
@Test
212+
public void doFilterWhenPostAuthorizationRequestMissingResponseTypeThenInvalidRequestError() throws Exception {
213+
doFilterWhenAuthorizationRequestInvalidParameterThenError(TestRegisteredClients.registeredClient().build(),
214+
OAuth2ParameterNames.RESPONSE_TYPE, OAuth2ErrorCodes.INVALID_REQUEST, (request) -> {
215+
request.setMethod("POST");
216+
request.removeParameter(OAuth2ParameterNames.RESPONSE_TYPE);
217+
request.setQueryString(null);
218+
});
219+
}
220+
210221
@Test
211222
public void doFilterWhenAuthorizationRequestMultipleResponseTypeThenInvalidRequestError() throws Exception {
212223
doFilterWhenAuthorizationRequestInvalidParameterThenError(TestRegisteredClients.registeredClient().build(),
@@ -646,7 +657,7 @@ public void doFilterWhenPostAuthorizationRequestAuthenticatedThenAuthorizationRe
646657

647658
this.filter.doFilter(request, response, filterChain);
648659

649-
verify(this.authenticationManager).authenticate(any());
660+
verify(this.authenticationManager).authenticate(any(OAuth2AuthorizationCodeRequestAuthenticationToken.class));
650661
verifyNoInteractions(filterChain);
651662

652663
assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());
@@ -675,7 +686,7 @@ public void doFilterWhenAuthenticationRequestAuthenticatedThenAuthorizationRespo
675686

676687
this.filter.doFilter(request, response, filterChain);
677688

678-
verify(this.authenticationManager).authenticate(any());
689+
verify(this.authenticationManager).authenticate(any(OAuth2AuthorizationCodeRequestAuthenticationToken.class));
679690
verifyNoInteractions(filterChain);
680691

681692
assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());

0 commit comments

Comments
 (0)