Spring/spring-authorization-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Additional polish gh-1468

Browse Source
This commit is contained in:
Joe Grandja
2023-12-18 12:48:11 -05:00
parent d915f0ab79
commit c86ddacbca
12 changed files with 133 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/src/docs/asciidoc/examples/src/test/java/sample/DeviceAuthorizationGrantFlow.java
View File

@@ -117,7 +117,7 @@ public class DeviceAuthorizationGrantFlow {
parameters.set(OAuth2ParameterNames.USER_CODE, userCode);
MvcResult mvcResult = this.mockMvc.perform(get("/oauth2/device_verification")
.params(parameters)
.queryParams(parameters)
.with(user(this.username).roles("USER")))
.andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.CONTENT_TYPE, containsString(MediaType.TEXT_HTML_VALUE)))

67
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/web/authentication/OAuth2EndpointUtils.java Normal file
View File

@@ -0,0 +1,67 @@
/*
* Copyright 2020-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.security.oauth2.server.authorization.oidc.web.authentication;
import java.util.Map;
import jakarta.servlet.http.HttpServletRequest;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
/**
* Utility methods for the OAuth 2.0 Protocol Endpoints.
*
* @author Joe Grandja
* @author Greg Li
* @since 1.1.4
*/
final class OAuth2EndpointUtils {
private OAuth2EndpointUtils() {
}
static MultiValueMap<String, String> getFormParameters(HttpServletRequest request) {
Map<String, String[]> parameterMap = request.getParameterMap();
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
parameterMap.forEach((key, values) -> {
String queryString = StringUtils.hasText(request.getQueryString()) ? request.getQueryString() : "";
// If not query parameter then it's a form parameter
if (!queryString.contains(key) && values.length > 0) {
for (String value : values) {
parameters.add(key, value);
}
}
});
return parameters;
}
static MultiValueMap<String, String> getQueryParameters(HttpServletRequest request) {
Map<String, String[]> parameterMap = request.getParameterMap();
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
parameterMap.forEach((key, values) -> {
String queryString = StringUtils.hasText(request.getQueryString()) ? request.getQueryString() : "";
if (queryString.contains(key) && values.length > 0) {
for (String value : values) {
parameters.add(key, value);
}
}
});
return parameters;
}
}

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

@@ -15,8 +15,6 @@
*/
package org.springframework.security.oauth2.server.authorization.oidc.web.authentication;
import java.util.Map;
import jakarta.servlet.http.HttpServletRequest;
import org.springframework.http.converter.HttpMessageConverter;
@@ -32,7 +30,6 @@ import org.springframework.security.oauth2.server.authorization.oidc.authenticat
import org.springframework.security.oauth2.server.authorization.oidc.http.converter.OidcClientRegistrationHttpMessageConverter;
import org.springframework.security.oauth2.server.authorization.oidc.web.OidcClientRegistrationEndpointFilter;
import org.springframework.security.web.authentication.AuthenticationConverter;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
@@ -69,7 +66,7 @@ public final class OidcClientRegistrationAuthenticationConverter implements Auth
return new OidcClientRegistrationAuthenticationToken(principal, clientRegistration);
}
MultiValueMap<String, String> parameters = getQueryParameters(request);
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getQueryParameters(request);
// client_id (REQUIRED)
String clientId = parameters.getFirst(OAuth2ParameterNames.CLIENT_ID);
@@ -81,18 +78,4 @@ public final class OidcClientRegistrationAuthenticationConverter implements Auth
return new OidcClientRegistrationAuthenticationToken(principal, clientId);
}
private static MultiValueMap<String, String> getQueryParameters(HttpServletRequest request) {
Map<String, String[]> parameterMap = request.getParameterMap();
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
parameterMap.forEach((key, values) -> {
String queryString = StringUtils.hasText(request.getQueryString()) ? request.getQueryString() : "";
if (queryString.contains(key) && values.length > 0) {
for (String value : values) {
parameters.add(key, value);
}
}
});
return parameters;
}
}

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

@@ -15,8 +15,6 @@
*/
package org.springframework.security.oauth2.server.authorization.oidc.web.authentication;
import java.util.Map;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpSession;
@@ -31,7 +29,6 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
import org.springframework.security.oauth2.server.authorization.oidc.authentication.OidcLogoutAuthenticationToken;
import org.springframework.security.oauth2.server.authorization.oidc.web.OidcLogoutEndpointFilter;
import org.springframework.security.web.authentication.AuthenticationConverter;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
@@ -51,12 +48,15 @@ public final class OidcLogoutAuthenticationConverter implements AuthenticationCo
@Override
public Authentication convert(HttpServletRequest request) {
MultiValueMap<String, String> parameters = getParameters(request);
MultiValueMap<String, String> parameters =
"GET".equals(request.getMethod()) ?
OAuth2EndpointUtils.getQueryParameters(request) :
OAuth2EndpointUtils.getFormParameters(request);
// id_token_hint (REQUIRED) // RECOMMENDED as per spec
String idTokenHint = request.getParameter("id_token_hint");
String idTokenHint = parameters.getFirst("id_token_hint");
if (!StringUtils.hasText(idTokenHint) ||
request.getParameterValues("id_token_hint").length != 1) {
parameters.get("id_token_hint").size() != 1) {
throwError(OAuth2ErrorCodes.INVALID_REQUEST, "id_token_hint");
}
@@ -96,19 +96,6 @@ public final class OidcLogoutAuthenticationConverter implements AuthenticationCo
sessionId, clientId, postLogoutRedirectUri, state);
}
private static MultiValueMap<String, String> getParameters(HttpServletRequest request) {
Map<String, String[]> parameterMap = request.getParameterMap();
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>(parameterMap.size());
parameterMap.forEach((key, values) -> {
if (values.length > 0) {
for (String value : values) {
parameters.add(key, value);
}
}
});
return parameters;
}
private static void throwError(String errorCode, String parameterName) {
OAuth2Error error = new OAuth2Error(
errorCode,

2
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationConsentAuthenticationConverter.java
View File

@@ -59,7 +59,7 @@ public final class OAuth2DeviceAuthorizationConsentAuthenticationConverter imple
return null;
}
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
String authorizationUri = request.getRequestURL().toString();

2
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceAuthorizationRequestAuthenticationConverter.java
View File

@@ -53,7 +53,7 @@ public final class OAuth2DeviceAuthorizationRequestAuthenticationConverter imple
public Authentication convert(HttpServletRequest request) {
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
String authorizationUri = request.getRequestURL().toString();

6
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceCodeAuthenticationConverter.java
View File

@@ -49,16 +49,16 @@ public final class OAuth2DeviceCodeAuthenticationConverter implements Authentica
@Nullable
@Override
public Authentication convert(HttpServletRequest request) {
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getFormParameters(request);
// grant_type (REQUIRED)
String grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE);
String grantType = parameters.getFirst(OAuth2ParameterNames.GRANT_TYPE);
if (!AuthorizationGrantType.DEVICE_CODE.getValue().equals(grantType)) {
return null;
}
Authentication clientPrincipal = SecurityContextHolder.getContext().getAuthentication();
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
// device_code (REQUIRED)
String deviceCode = parameters.getFirst(OAuth2ParameterNames.DEVICE_CODE);
if (!StringUtils.hasText(deviceCode) ||

5
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverter.java
View File

@@ -59,7 +59,10 @@ public final class OAuth2DeviceVerificationAuthenticationConverter implements Au
return null;
}
MultiValueMap<String, String> parameters = OAuth2EndpointUtils.getParameters(request);
MultiValueMap<String, String> parameters =
"GET".equals(request.getMethod()) ?
OAuth2EndpointUtils.getQueryParameters(request) :
OAuth2EndpointUtils.getFormParameters(request);
// user_code (REQUIRED)
String userCode = parameters.getFirst(OAuth2ParameterNames.USER_CODE);

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

@@ -279,7 +279,7 @@ public class OAuth2DeviceCodeGrantTests {
// @formatter:off
this.mvc.perform(get(DEFAULT_DEVICE_VERIFICATION_ENDPOINT_URI)
.params(parameters))
.queryParams(parameters))
.andExpect(status().isUnauthorized());
// @formatter:on
}
@@ -313,7 +313,7 @@ public class OAuth2DeviceCodeGrantTests {
// @formatter:off
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_DEVICE_VERIFICATION_ENDPOINT_URI)
.params(parameters)
.queryParams(parameters)
.with(user("user")))
.andExpect(status().isOk())
.andExpect(content().contentTypeCompatibleWith(MediaType.TEXT_HTML))

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

@@ -247,7 +247,7 @@ public class OidcTests {
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient);
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
.params(authorizationRequestParameters)
.queryParams(authorizationRequestParameters)
.with(user("user").roles("A", "B")))
.andExpect(status().is3xxRedirection())
.andReturn();
@@ -304,7 +304,7 @@ public class OidcTests {
// Login
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient);
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
.params(authorizationRequestParameters)
.queryParams(authorizationRequestParameters)
.with(user("user")))
.andExpect(status().is3xxRedirection())
.andReturn();
@@ -353,7 +353,7 @@ public class OidcTests {
MultiValueMap<String, String> authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient1);
MvcResult mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
.params(authorizationRequestParameters)
.queryParams(authorizationRequestParameters)
.with(user("user1")))
.andExpect(status().is3xxRedirection())
.andReturn();
@@ -385,7 +385,7 @@ public class OidcTests {
authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient2);
mvcResult = this.mvc.perform(get(DEFAULT_AUTHORIZATION_ENDPOINT_URI)
.params(authorizationRequestParameters)
.queryParams(authorizationRequestParameters)
.with(user("user2")))
.andExpect(status().is3xxRedirection())
.andReturn();

22
oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2DeviceVerificationEndpointFilterTests.java
View File

@@ -23,6 +23,7 @@ import java.util.Set;
import jakarta.servlet.FilterChain;
import jakarta.servlet.http.HttpServletRequest;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -164,6 +165,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -223,6 +225,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
request.addParameter("custom-param-1", "custom-value-1");
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -248,6 +251,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -268,6 +272,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -291,6 +296,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
request.setServerPort(443);
request.setServerName("provider.com");
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.setConsentPage("/consent");
@@ -322,6 +328,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -340,6 +347,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
@@ -367,6 +375,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -388,6 +397,7 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
updateQueryString(request);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
@@ -445,6 +455,18 @@ public class OAuth2DeviceVerificationEndpointFilterTests {
return request;
}
private static void updateQueryString(MockHttpServletRequest request) {
UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(request.getRequestURI());
request.getParameterMap().forEach((key, values) -> {
if (values.length > 0) {
for (String value : values) {
uriBuilder.queryParam(key, value);
}
}
});
request.setQueryString(uriBuilder.build().getQuery());
}
private static String scopeCheckbox(String scope) {
return MessageFormat.format(
"<input class=\"form-check-input\" type=\"checkbox\" name=\"scope\" value=\"{0}\" id=\"{0}\">",

21
oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2DeviceVerificationAuthenticationConverterTests.java
View File

@@ -31,6 +31,7 @@ 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.server.authorization.authentication.OAuth2DeviceVerificationAuthenticationToken;
import org.springframework.web.util.UriComponentsBuilder;
import static java.util.Map.entry;
import static org.assertj.core.api.Assertions.assertThat;
@@ -69,6 +70,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
public void convertWhenStateThenReturnNull() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.STATE, "abc123");
updateQueryString(request);
Authentication authentication = this.converter.convert(request);
assertThat(authentication).isNull();
}
@@ -84,6 +86,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
public void convertWhenEmptyUserCodeParameterThenInvalidRequestError() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, "");
updateQueryString(request);
// @formatter:off
assertThatExceptionOfType(OAuth2AuthenticationException.class)
.isThrownBy(() -> this.converter.convert(request))
@@ -98,6 +101,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
public void convertWhenInvalidUserCodeParameterThenInvalidRequestError() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, "LONG-USER-CODE");
updateQueryString(request);
// @formatter:off
assertThatExceptionOfType(OAuth2AuthenticationException.class)
.isThrownBy(() -> this.converter.convert(request))
@@ -113,6 +117,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
request.addParameter(OAuth2ParameterNames.USER_CODE, "another");
updateQueryString(request);
// @formatter:off
assertThatExceptionOfType(OAuth2AuthenticationException.class)
.isThrownBy(() -> this.converter.convert(request))
@@ -127,6 +132,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
public void convertWhenMissingPrincipalThenReturnDeviceVerificationAuthentication() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE.toLowerCase().replace("-", " . "));
updateQueryString(request);
OAuth2DeviceVerificationAuthenticationToken authentication =
(OAuth2DeviceVerificationAuthenticationToken) this.converter.convert(request);
@@ -140,6 +146,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
public void convertWhenNonNormalizedUserCodeThenReturnDeviceVerificationAuthentication() {
MockHttpServletRequest request = createRequest();
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE.toLowerCase().replace("-", " . "));
updateQueryString(request);
SecurityContextImpl securityContext = new SecurityContextImpl();
securityContext.setAuthentication(new TestingAuthenticationToken("user", null));
@@ -159,6 +166,7 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
request.addParameter("param-1", "value-1");
request.addParameter("param-2", "value-1", "value-2");
updateQueryString(request);
SecurityContextImpl securityContext = new SecurityContextImpl();
securityContext.setAuthentication(new TestingAuthenticationToken("user", null));
@@ -180,4 +188,17 @@ public class OAuth2DeviceVerificationAuthenticationConverterTests {
request.setRequestURI(VERIFICATION_URI);
return request;
}
private static void updateQueryString(MockHttpServletRequest request) {
UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromUriString(request.getRequestURI());
request.getParameterMap().forEach((key, values) -> {
if (values.length > 0) {
for (String value : values) {
uriBuilder.queryParam(key, value);
}
}
});
request.setQueryString(uriBuilder.build().getQuery());
}
}