Skip to content

Commit c86ddac

Browse files
jgrandjajgrandja
committed
Additional polish gh-1468 
1 parent d915f0a commit c86ddac

File tree

12 files changed

+133
-50
lines changed

12 files changed

+133
-50
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public String submitCode(String userCode) throws Exception {
117117
parameters.set(OAuth2ParameterNames.USER_CODE, userCode);
118118

119119
MvcResult mvcResult = this.mockMvc.perform(get("/oauth2/device_verification")
120-
.params(parameters)
120+
.queryParams(parameters)
121121
.with(user(this.username).roles("USER")))
122122
.andExpect(status().isOk())
123123
.andExpect(header().string(HttpHeaders.CONTENT_TYPE, containsString(MediaType.TEXT_HTML_VALUE)))

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright 2020-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.security.oauth2.server.authorization.oidc.web.authentication;
17+
18+
import java.util.Map;
19+
20+
import jakarta.servlet.http.HttpServletRequest;
21+
22+
import org.springframework.util.LinkedMultiValueMap;
23+
import org.springframework.util.MultiValueMap;
24+
import org.springframework.util.StringUtils;
25+
26+
/**
27+
* Utility methods for the OAuth 2.0 Protocol Endpoints.
28+
*
29+
* @author Joe Grandja
30+
* @author Greg Li
31+
* @since 1.1.4
32+
*/
33+
final class OAuth2EndpointUtils {
34+
35+
private OAuth2EndpointUtils() {
36+
}
37+
38+
static MultiValueMap<String, String> getFormParameters(HttpServletRequest request) {
39+
Map<String, String[]> parameterMap = request.getParameterMap();
40+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
41+
parameterMap.forEach((key, values) -> {
42+
String queryString = StringUtils.hasText(request.getQueryString()) ? request.getQueryString() : "";
43+
// If not query parameter then it's a form parameter
44+
if (!queryString.contains(key) && values.length > 0) {
45+
for (String value : values) {
46+
parameters.add(key, value);
47+
}
48+
}
49+
});
50+
return parameters;
51+
}
52+
53+
static MultiValueMap<String, String> getQueryParameters(HttpServletRequest request) {
54+
Map<String, String[]> parameterMap = request.getParameterMap();
55+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
56+
parameterMap.forEach((key, values) -> {
57+
String queryString = StringUtils.hasText(request.getQueryString()) ? request.getQueryString() : "";
58+
if (queryString.contains(key) && values.length > 0) {
59+
for (String value : values) {
60+
parameters.add(key, value);
61+
}
62+
}
63+
});
64+
return parameters;
65+
}
66+
67+
}

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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616
package org.springframework.security.oauth2.server.authorization.oidc.web.authentication;
1717

18-
import java.util.Map;
19-
2018
import jakarta.servlet.http.HttpServletRequest;
2119

2220
import org.springframework.http.converter.HttpMessageConverter;
@@ -32,7 +30,6 @@
3230
import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcClientRegistrationHttpMessageConverter;
3331
import org.springframework.security.oauth2.server.authorization.oidc.web.OidcClientRegistrationEndpointFilter;
3432
import org.springframework.security.web.authentication.AuthenticationConverter;
35-
import org.springframework.util.LinkedMultiValueMap;
3633
import org.springframework.util.MultiValueMap;
3734
import org.springframework.util.StringUtils;
3835

@@ -69,7 +66,7 @@ public Authentication convert(HttpServletRequest request) {
6966
return new OidcClientRegistrationAuthenticationToken(principal, clientRegistration);
7067
}
7168

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

7471
// client_id (REQUIRED)
7572
String clientId = parameters.getFirst(OAuth2ParameterNames.CLIENT_ID);
@@ -81,18 +78,4 @@ public Authentication convert(HttpServletRequest request) {
8178
return new OidcClientRegistrationAuthenticationToken(principal, clientId);
8279
}
8380

84-
private static MultiValueMap<String, String> getQueryParameters(HttpServletRequest request) {
85-
Map<String, String[]> parameterMap = request.getParameterMap();
86-
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
87-
parameterMap.forEach((key, values) -> {
88-
String queryString = StringUtils.hasText(request.getQueryString()) ? request.getQueryString() : "";
89-
if (queryString.contains(key) && values.length > 0) {
90-
for (String value : values) {
91-
parameters.add(key, value);
92-
}
93-
}
94-
});
95-
return parameters;
96-
}
97-
9881
}

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

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616
package org.springframework.security.oauth2.server.authorization.oidc.web.authentication;
1717

18-
import java.util.Map;
19-
2018
import jakarta.servlet.http.HttpServletRequest;
2119
import jakarta.servlet.http.HttpSession;
2220

@@ -31,7 +29,6 @@
3129
import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcLogoutAuthenticationToken;
3230
import org.springframework.security.oauth2.server.authorization.oidc.web.OidcLogoutEndpointFilter;
3331
import org.springframework.security.web.authentication.AuthenticationConverter;
34-
import org.springframework.util.LinkedMultiValueMap;
3532
import org.springframework.util.MultiValueMap;
3633
import org.springframework.util.StringUtils;
3734

@@ -51,12 +48,15 @@ public final class OidcLogoutAuthenticationConverter implements AuthenticationCo
5148

5249
@Override
5350
public Authentication convert(HttpServletRequest request) {
54-
MultiValueMap<String, String> parameters = getParameters(request);
51+
MultiValueMap<String, String> parameters =
52+
"GET".equals(request.getMethod()) ?
53+
OAuth2EndpointUtils.getQueryParameters(request) :
54+
OAuth2EndpointUtils.getFormParameters(request);
5555

5656
// id_token_hint (REQUIRED) // RECOMMENDED as per spec
57-
String idTokenHint = request.getParameter("id_token_hint");
57+
String idTokenHint = parameters.getFirst("id_token_hint");
5858
if (!StringUtils.hasText(idTokenHint) ||
59-
request.getParameterValues("id_token_hint").length != 1) {
59+
parameters.get("id_token_hint").size() != 1) {
6060
throwError(OAuth2ErrorCodes.INVALID_REQUEST, "id_token_hint");
6161
}
6262

@@ -96,19 +96,6 @@ public Authentication convert(HttpServletRequest request) {
9696
sessionId, clientId, postLogoutRedirectUri, state);
9797
}
9898

99-
private static MultiValueMap<String, String> getParameters(HttpServletRequest request) {
100-
Map<String, String[]> parameterMap = request.getParameterMap();
101-
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>(parameterMap.size());
102-
parameterMap.forEach((key, values) -> {
103-
if (values.length > 0) {
104-
for (String value : values) {
105-
parameters.add(key, value);
106-
}
107-
}
108-
});
109-
return parameters;
110-
}
111-
11299
private static void throwError(String errorCode, String parameterName) {
113100
OAuth2Error error = new OAuth2Error(
114101
errorCode,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public Authentication convert(HttpServletRequest request) {
5959
return null;
6060
}
6161

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

6464
String authorizationUri = request.getRequestURL().toString();
6565

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public final class OAuth2DeviceAuthorizationRequestAuthenticationConverter imple
5353
public Authentication convert(HttpServletRequest request) {
5454
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
5555

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

5858
String authorizationUri = request.getRequestURL().toString();
5959

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,16 @@ public final class OAuth2DeviceCodeAuthenticationConverter implements Authentica
4949
@Nullable
5050
@Override
5151
public Authentication convert(HttpServletRequest request) {
52+
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
53+
5254
// grant_type (REQUIRED)
53-
String grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE);
55+
String grantType = parameters.getFirst(OAuth2ParameterNames.GRANT_TYPE);
5456
if (!AuthorizationGrantType.DEVICE_CODE.getValue().equals(grantType)) {
5557
return null;
5658
}
5759

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

60-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
61-
6262
// device_code (REQUIRED)
6363
String deviceCode = parameters.getFirst(OAuth2ParameterNames.DEVICE_CODE);
6464
if (!StringUtils.hasText(deviceCode) ||

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ public Authentication convert(HttpServletRequest request) {
5959
return null;
6060
}
6161

62-
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
62+
MultiValueMap<String, String> parameters =
63+
"GET".equals(request.getMethod()) ?
64+
OAuth2EndpointUtils.getQueryParameters(request) :
65+
OAuth2EndpointUtils.getFormParameters(request);
6366

6467
// user_code (REQUIRED)
6568
String userCode = parameters.getFirst(OAuth2ParameterNames.USER_CODE);

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2DeviceCodeGrantTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ public void requestWhenDeviceVerificationRequestUnauthenticatedThenUnauthorized(
279279

280280
// @formatter:off
281281
this.mvc.perform(get(DEFAULT_DEVICE_VERIFICATION_ENDPOINT_URI)
282-
.params(parameters))
282+
.queryParams(parameters))
283283
.andExpect(status().isUnauthorized());
284284
// @formatter:on
285285
}
@@ -313,7 +313,7 @@ public void requestWhenDeviceVerificationRequestValidThenDisplaysConsentPage() t
313313

314314
// @formatter:off
315315
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_DEVICE_VERIFICATION_ENDPOINT_URI)
316-
.params(parameters)
316+
.queryParams(parameters)
317317
.with(user("user")))
318318
.andExpect(status().isOk())
319319
.andExpect(content().contentTypeCompatibleWith(MediaType.TEXT_HTML))

‎oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ public void requestWhenRefreshTokenRequestThenIdTokenContainsSidClaim() throws E
247247

248248
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient);
249249
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
250-
.params(authorizationRequestParameters)
250+
.queryParams(authorizationRequestParameters)
251251
.with(user("user").roles("A", "B")))
252252
.andExpect(status().is3xxRedirection())
253253
.andReturn();
@@ -304,7 +304,7 @@ public void requestWhenLogoutRequestThenLogout() throws Exception {
304304
// Login
305305
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient);
306306
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
307-
.params(authorizationRequestParameters)
307+
.queryParams(authorizationRequestParameters)
308308
.with(user("user")))
309309
.andExpect(status().is3xxRedirection())
310310
.andReturn();
@@ -353,7 +353,7 @@ public void requestWhenLogoutRequestWithOtherUsersIdTokenThenNotLogout() throws
353353

354354
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient1);
355355
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
356-
.params(authorizationRequestParameters)
356+
.queryParams(authorizationRequestParameters)
357357
.with(user("user1")))
358358
.andExpect(status().is3xxRedirection())
359359
.andReturn();
@@ -385,7 +385,7 @@ public void requestWhenLogoutRequestWithOtherUsersIdTokenThenNotLogout() throws
385385

386386
authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient2);
387387
mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
388-
.params(authorizationRequestParameters)
388+
.queryParams(authorizationRequestParameters)
389389
.with(user("user2")))
390390
.andExpect(status().is3xxRedirection())
391391
.andReturn();

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import jakarta.servlet.FilterChain;
2525
import jakarta.servlet.http.HttpServletRequest;
26+
2627
import org.junit.jupiter.api.AfterEach;
2728
import org.junit.jupiter.api.BeforeEach;
2829
import org.junit.jupiter.api.Test;
@@ -164,6 +165,7 @@ public void doFilterWhenUnauthenticatedThenPassThrough() throws Exception {
164165

165166
MockHttpServletRequest request = createRequest();
166167
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
168+
updateQueryString(request);
167169
MockHttpServletResponse response = new MockHttpServletResponse();
168170
FilterChain filterChain = mock(FilterChain.class);
169171
this.filter.doFilter(request, response, filterChain);
@@ -223,6 +225,7 @@ public void doFilterWhenDeviceVerificationRequestAndConsentNotRequiredThenSucces
223225
MockHttpServletRequest request = createRequest();
224226
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
225227
request.addParameter("custom-param-1", "custom-value-1");
228+
updateQueryString(request);
226229
MockHttpServletResponse response = new MockHttpServletResponse();
227230
FilterChain filterChain = mock(FilterChain.class);
228231
this.filter.doFilter(request, response, filterChain);
@@ -248,6 +251,7 @@ public void doFilterWhenDeviceVerificationRequestAndConsentRequiredThenConsentSc
248251

249252
MockHttpServletRequest request = createRequest();
250253
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
254+
updateQueryString(request);
251255
MockHttpServletResponse response = new MockHttpServletResponse();
252256
FilterChain filterChain = mock(FilterChain.class);
253257
this.filter.doFilter(request, response, filterChain);
@@ -268,6 +272,7 @@ public void doFilterWhenDeviceVerificationRequestAndConsentRequiredWithPreviousl
268272

269273
MockHttpServletRequest request = createRequest();
270274
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
275+
updateQueryString(request);
271276
MockHttpServletResponse response = new MockHttpServletResponse();
272277
FilterChain filterChain = mock(FilterChain.class);
273278
this.filter.doFilter(request, response, filterChain);
@@ -291,6 +296,7 @@ public void doFilterWhenDeviceVerificationRequestAndConsentRequiredAndConsentPag
291296
request.setServerPort(443);
292297
request.setServerName("provider.com");
293298
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
299+
updateQueryString(request);
294300
MockHttpServletResponse response = new MockHttpServletResponse();
295301
FilterChain filterChain = mock(FilterChain.class);
296302
this.filter.setConsentPage("/consent");
@@ -322,6 +328,7 @@ public void doFilterWhenAuthenticationConverterSetThenUsed() throws Exception {
322328

323329
MockHttpServletRequest request = createRequest();
324330
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
331+
updateQueryString(request);
325332
MockHttpServletResponse response = new MockHttpServletResponse();
326333
FilterChain filterChain = mock(FilterChain.class);
327334
this.filter.doFilter(request, response, filterChain);
@@ -340,6 +347,7 @@ public void doFilterWhenAuthenticationDetailsSourceSetThenUsed() throws Exceptio
340347

341348
MockHttpServletRequest request = createRequest();
342349
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
350+
updateQueryString(request);
343351
MockHttpServletResponse response = new MockHttpServletResponse();
344352
FilterChain filterChain = mock(FilterChain.class);
345353

@@ -367,6 +375,7 @@ public void doFilterWhenAuthenticationSuccessHandlerSetThenUsed() throws Excepti
367375

368376
MockHttpServletRequest request = createRequest();
369377
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
378+
updateQueryString(request);
370379
MockHttpServletResponse response = new MockHttpServletResponse();
371380
FilterChain filterChain = mock(FilterChain.class);
372381
this.filter.doFilter(request, response, filterChain);
@@ -388,6 +397,7 @@ public void doFilterWhenAuthenticationFailureHandlerSetThenUsed() throws Excepti
388397

389398
MockHttpServletRequest request = createRequest();
390399
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
400+
updateQueryString(request);
391401
MockHttpServletResponse response = new MockHttpServletResponse();
392402
FilterChain filterChain = mock(FilterChain.class);
393403
this.filter.doFilter(request, response, filterChain);
@@ -445,6 +455,18 @@ private static MockHttpServletRequest createRequest() {
445455
return request;
446456
}
447457

458+
private static void updateQueryString(MockHttpServletRequest request) {
459+
UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(request.getRequestURI());
460+
request.getParameterMap().forEach((key, values) -> {
461+
if (values.length > 0) {
462+
for (String value : values) {
463+
uriBuilder.queryParam(key, value);
464+
}
465+
}
466+
});
467+
request.setQueryString(uriBuilder.build().getQuery());
468+
}
469+
448470
private static String scopeCheckbox(String scope) {
449471
return MessageFormat.format(
450472
"<input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">",

0 commit comments

Comments
 (0)