Skip to content

Commit 4bc0df5

Browse files
Greg Lijgrandja
Greg Li
authored and
jgrandja
committed
Fix to ensure endpoints distinguish between form and query parameters
Closes spring-projectsgh-1451
1 parent 639fe93 commit 4bc0df5

15 files changed

+105
-67
lines changed

‎docs/src/docs/asciidoc/examples/src/test/java/sample/AuthorizationCodeGrantFlow.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public String authorize(RegisteredClient registeredClient) throws Exception {
9494
parameters.set(OAuth2ParameterNames.STATE, "state");
9595

9696
MvcResult mvcResult = this.mockMvc.perform(get("/oauth2/authorize")
97-
.params(parameters)
97+
.queryParams(parameters)
9898
.with(user(this.username).roles("USER")))
9999
.andExpect(status().isOk())
100100
.andExpect(header().string("content-type", containsString(MediaType.TEXT_HTML_VALUE)))

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,18 @@ public final class ClientSecretPostAuthenticationConverter implements Authentica
4848
@Nullable
4949
@Override
5050
public Authentication convert(HttpServletRequest request) {
51-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
51+
String queryString = request.getQueryString();
52+
if (StringUtils.hasText(queryString) &&
53+
(queryString.contains(OAuth2ParameterNames.CLIENT_ID) ||
54+
queryString.contains(OAuth2ParameterNames.CLIENT_SECRET))) {
55+
OAuth2Error error = new OAuth2Error(
56+
OAuth2ErrorCodes.INVALID_REQUEST,
57+
"Client credentials MUST NOT be included in the request URI.",
58+
null);
59+
throw new OAuth2AuthenticationException(error);
60+
}
61+
62+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
5263

5364
// client_id (REQUIRED)
5465
String clientId = parameters.getFirst(OAuth2ParameterNames.CLIENT_ID);
@@ -70,17 +81,6 @@ public Authentication convert(HttpServletRequest request) {
7081
throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST);
7182
}
7283

73-
String queryString = request.getQueryString();
74-
if (StringUtils.hasText(queryString) &&
75-
(queryString.contains(OAuth2ParameterNames.CLIENT_ID) ||
76-
queryString.contains(OAuth2ParameterNames.CLIENT_SECRET))) {
77-
OAuth2Error error = new OAuth2Error(
78-
OAuth2ErrorCodes.INVALID_REQUEST,
79-
"Client credentials MUST NOT be included in the request URI.",
80-
null);
81-
throw new OAuth2AuthenticationException(error);
82-
}
83-
8484
Map<String, Object> additionalParameters = OAuth2EndpointUtils.getParametersIfMatchesAuthorizationCodeGrantRequest(request,
8585
OAuth2ParameterNames.CLIENT_ID,
8686
OAuth2ParameterNames.CLIENT_SECRET);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ public final class JwtClientAssertionAuthenticationConverter implements Authenti
4848
@Nullable
4949
@Override
5050
public Authentication convert(HttpServletRequest request) {
51-
if (request.getParameter(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE) == null ||
52-
request.getParameter(OAuth2ParameterNames.CLIENT_ASSERTION) == null) {
51+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
52+
53+
if (parameters.getFirst(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE) == null ||
54+
parameters.getFirst(OAuth2ParameterNames.CLIENT_ASSERTION) == null) {
5355
return null;
5456
}
5557

56-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
57-
5858
// client_assertion_type (REQUIRED)
5959
String clientAssertionType = parameters.getFirst(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE);
6060
if (parameters.get(OAuth2ParameterNames.CLIENT_ASSERTION_TYPE).size() != 1) {

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,15 @@ public final class OAuth2AuthorizationCodeAuthenticationConverter implements Aut
4747
@Nullable
4848
@Override
4949
public Authentication convert(HttpServletRequest request) {
50+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
5051
// grant_type (REQUIRED)
51-
String grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE);
52+
String grantType = parameters.getFirst(OAuth2ParameterNames.GRANT_TYPE);
5253
if (!AuthorizationGrantType.AUTHORIZATION_CODE.getValue().equals(grantType)) {
5354
return null;
5455
}
5556

5657
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
5758

58-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
59-
6059
// code (REQUIRED)
6160
String code = parameters.getFirst(OAuth2ParameterNames.CODE);
6261
if (!StringUtils.hasText(code) ||

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ public Authentication convert(HttpServletRequest request) {
6666
return null;
6767
}
6868

69-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
69+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getQueryParameters(request);
7070

7171
// response_type (REQUIRED)
72-
String responseType = request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE);
72+
String responseType = parameters.getFirst(OAuth2ParameterNames.RESPONSE_TYPE);
7373
if (!StringUtils.hasText(responseType) ||
7474
parameters.get(OAuth2ParameterNames.RESPONSE_TYPE).size() != 1) {
7575
throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.RESPONSE_TYPE);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,13 @@ public final class OAuth2AuthorizationConsentAuthenticationConverter implements
5454

5555
@Override
5656
public Authentication convert(HttpServletRequest request) {
57+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
58+
5759
if (!"POST".equals(request.getMethod()) ||
58-
request.getParameter(OAuth2ParameterNames.RESPONSE_TYPE) != null) {
60+
parameters.getFirst(OAuth2ParameterNames.RESPONSE_TYPE) != null) {
5961
return null;
6062
}
6163

62-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
63-
6464
String authorizationUri = request.getRequestURL().toString();
6565

6666
// client_id (REQUIRED)

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,16 @@ public final class OAuth2ClientCredentialsAuthenticationConverter implements Aut
5050
@Nullable
5151
@Override
5252
public Authentication convert(HttpServletRequest request) {
53+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
54+
5355
// grant_type (REQUIRED)
54-
String grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE);
56+
String grantType = parameters.getFirst(OAuth2ParameterNames.GRANT_TYPE);
5557
if (!AuthorizationGrantType.CLIENT_CREDENTIALS.getValue().equals(grantType)) {
5658
return null;
5759
}
5860

5961
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
6062

61-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
62-
6363
// scope (OPTIONAL)
6464
String scope = parameters.getFirst(OAuth2ParameterNames.SCOPE);
6565
if (StringUtils.hasText(scope) &&

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

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,41 @@
2828
import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
2929
import org.springframework.util.LinkedMultiValueMap;
3030
import org.springframework.util.MultiValueMap;
31+
import org.springframework.util.StringUtils;
3132

3233
/**
3334
* Utility methods for the OAuth 2.0 Protocol Endpoints.
3435
*
3536
* @author Joe Grandja
37+
* @author Greg Li
3638
* @since 0.1.2
3739
*/
3840
final class OAuth2EndpointUtils {
3941
static final String ACCESS_TOKEN_REQUEST_ERROR_URI = "https://datatracker.ietf.org/doc/html/rfc6749#section-5.2";
40-
4142
private OAuth2EndpointUtils() {
4243
}
4344

44-
static MultiValueMap<String, String> getParameters(HttpServletRequest request) {
45+
static MultiValueMap<String, String> getFormParameters(HttpServletRequest request) {
46+
Map<String, String[]> parameterMap = request.getParameterMap();
47+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
48+
parameterMap.forEach((key, values) -> {
49+
// If not query parameter then it's a form parameter
50+
if ((!StringUtils.hasText(request.getQueryString()) && values.length > 0)
51+
|| (!request.getQueryString().contains(key) && values.length > 0)) {
52+
for (String value : values) {
53+
parameters.add(key, value);
54+
}
55+
}
56+
});
57+
return parameters;
58+
}
59+
60+
static MultiValueMap<String, String> getQueryParameters(HttpServletRequest request) {
4561
Map<String, String[]> parameterMap = request.getParameterMap();
46-
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>(parameterMap.size());
62+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
4763
parameterMap.forEach((key, values) -> {
48-
if (values.length > 0) {
64+
if (StringUtils.hasText(request.getQueryString())
65+
&& request.getQueryString().contains(key) && values.length > 0) {
4966
for (String value : values) {
5067
parameters.add(key, value);
5168
}
@@ -58,7 +75,7 @@ static Map<String, Object> getParametersIfMatchesAuthorizationCodeGrantRequest(H
5875
if (!matchesAuthorizationCodeGrantRequest(request)) {
5976
return Collections.emptyMap();
6077
}
61-
MultiValueMap<String, String> multiValueParameters = getParameters(request);
78+
MultiValueMap<String, String> multiValueParameters = getFormParameters(request);
6279
for (String exclusion : exclusions) {
6380
multiValueParameters.remove(exclusion);
6481
}
@@ -71,14 +88,16 @@ static Map<String, Object> getParametersIfMatchesAuthorizationCodeGrantRequest(H
7188
}
7289

7390
static boolean matchesAuthorizationCodeGrantRequest(HttpServletRequest request) {
91+
MultiValueMap<String, String> parameters = getFormParameters(request);
7492
return AuthorizationGrantType.AUTHORIZATION_CODE.getValue().equals(
75-
request.getParameter(OAuth2ParameterNames.GRANT_TYPE)) &&
76-
request.getParameter(OAuth2ParameterNames.CODE) != null;
93+
parameters.getFirst(OAuth2ParameterNames.GRANT_TYPE)) &&
94+
parameters.getFirst(OAuth2ParameterNames.CODE) != null;
7795
}
7896

7997
static boolean matchesPkceTokenRequest(HttpServletRequest request) {
98+
MultiValueMap<String, String> parameters = getFormParameters(request);
8099
return matchesAuthorizationCodeGrantRequest(request) &&
81-
request.getParameter(PkceParameterNames.CODE_VERIFIER) != null;
100+
parameters.getFirst(PkceParameterNames.CODE_VERIFIER) != null;
82101
}
83102

84103
static void throwError(String errorCode, String parameterName, String errorUri) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,16 @@ public final class OAuth2RefreshTokenAuthenticationConverter implements Authenti
5050
@Nullable
5151
@Override
5252
public Authentication convert(HttpServletRequest request) {
53+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
54+
5355
// grant_type (REQUIRED)
54-
String grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE);
56+
String grantType = parameters.getFirst(OAuth2ParameterNames.GRANT_TYPE);
5557
if (!AuthorizationGrantType.REFRESH_TOKEN.getValue().equals(grantType)) {
5658
return null;
5759
}
5860

5961
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
6062

61-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
62-
6363
// refresh_token (REQUIRED)
6464
String refreshToken = parameters.getFirst(OAuth2ParameterNames.REFRESH_TOKEN);
6565
if (!StringUtils.hasText(refreshToken) ||

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public final class OAuth2TokenIntrospectionAuthenticationConverter implements Au
4949
public Authentication convert(HttpServletRequest request) {
5050
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
5151

52-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
52+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
5353

5454
// token (REQUIRED)
5555
String token = parameters.getFirst(OAuth2ParameterNames.TOKEN);

0 commit comments

Comments
 (0)