Fix to save all values for multi-valued request parameters
Fixes gh-1250
This commit is contained in:
committed by
Joe Grandja
parent
4bb741b0ba
commit
890b1ef0ed
@@ -84,7 +84,7 @@ public final class OAuth2AuthorizationCodeAuthenticationConverter implements Aut
|
||||
!key.equals(OAuth2ParameterNames.CLIENT_ID) &&
|
||||
!key.equals(OAuth2ParameterNames.CODE) &&
|
||||
!key.equals(OAuth2ParameterNames.REDIRECT_URI)) {
|
||||
additionalParameters.put(key, value.get(0));
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -138,7 +138,7 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationConverter impleme
|
||||
!key.equals(OAuth2ParameterNames.REDIRECT_URI) &&
|
||||
!key.equals(OAuth2ParameterNames.SCOPE) &&
|
||||
!key.equals(OAuth2ParameterNames.STATE)) {
|
||||
additionalParameters.put(key, value.get(0));
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -93,7 +93,7 @@ public final class OAuth2AuthorizationConsentAuthenticationConverter implements
|
||||
if (!key.equals(OAuth2ParameterNames.CLIENT_ID) &&
|
||||
!key.equals(OAuth2ParameterNames.STATE) &&
|
||||
!key.equals(OAuth2ParameterNames.SCOPE)) {
|
||||
additionalParameters.put(key, value.get(0));
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@ public final class OAuth2ClientCredentialsAuthenticationConverter implements Aut
|
||||
parameters.forEach((key, value) -> {
|
||||
if (!key.equals(OAuth2ParameterNames.GRANT_TYPE) &&
|
||||
!key.equals(OAuth2ParameterNames.SCOPE)) {
|
||||
additionalParameters.put(key, value.get(0));
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -16,8 +16,8 @@
|
||||
package org.springframework.security.oauth2.server.authorization.web.authentication;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
||||
@@ -58,11 +58,13 @@ final class OAuth2EndpointUtils {
|
||||
if (!matchesAuthorizationCodeGrantRequest(request)) {
|
||||
return Collections.emptyMap();
|
||||
}
|
||||
Map<String, Object> parameters = new HashMap<>(getParameters(request).toSingleValueMap());
|
||||
MultiValueMap<String, String> parameters = getParameters(request);
|
||||
for (String exclusion : exclusions) {
|
||||
parameters.remove(exclusion);
|
||||
}
|
||||
return parameters;
|
||||
return parameters.entrySet().stream()
|
||||
.collect(Collectors.toMap(Map.Entry::getKey,
|
||||
e -> e.getValue().size() == 1 ? e.getValue().get(0) : e.getValue().toArray(new String[0])));
|
||||
}
|
||||
|
||||
static boolean matchesAuthorizationCodeGrantRequest(HttpServletRequest request) {
|
||||
|
||||
@@ -90,7 +90,7 @@ public final class OAuth2RefreshTokenAuthenticationConverter implements Authenti
|
||||
if (!key.equals(OAuth2ParameterNames.GRANT_TYPE) &&
|
||||
!key.equals(OAuth2ParameterNames.REFRESH_TOKEN) &&
|
||||
!key.equals(OAuth2ParameterNames.SCOPE)) {
|
||||
additionalParameters.put(key, value.get(0));
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -69,7 +69,7 @@ public final class OAuth2TokenIntrospectionAuthenticationConverter implements Au
|
||||
parameters.forEach((key, value) -> {
|
||||
if (!key.equals(OAuth2ParameterNames.TOKEN) &&
|
||||
!key.equals(OAuth2ParameterNames.TOKEN_TYPE_HINT)) {
|
||||
additionalParameters.put(key, value.get(0));
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
package org.springframework.security.oauth2.server.authorization.web.authentication;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
||||
@@ -68,7 +69,12 @@ public final class PublicClientAuthenticationConverter implements Authentication
|
||||
|
||||
parameters.remove(OAuth2ParameterNames.CLIENT_ID);
|
||||
|
||||
Map<String, Object> additionalParameters = new HashMap<>();
|
||||
parameters.forEach((key, value) -> {
|
||||
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
|
||||
});
|
||||
|
||||
return new OAuth2ClientAuthenticationToken(clientId, ClientAuthenticationMethod.NONE, null,
|
||||
new HashMap<>(parameters.toSingleValueMap()));
|
||||
additionalParameters);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -556,6 +556,8 @@ public class OAuth2AuthorizationEndpointFilterTests {
|
||||
.thenReturn(authorizationCodeRequestAuthenticationResult);
|
||||
|
||||
MockHttpServletRequest request = createAuthorizationRequest(registeredClient);
|
||||
request.addParameter("foo", "value1", "value2");
|
||||
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
FilterChain filterChain = mock(FilterChain.class);
|
||||
|
||||
@@ -570,6 +572,12 @@ public class OAuth2AuthorizationEndpointFilterTests {
|
||||
.asInstanceOf(type(WebAuthenticationDetails.class))
|
||||
.extracting(WebAuthenticationDetails::getRemoteAddress)
|
||||
.isEqualTo(REMOTE_ADDRESS);
|
||||
|
||||
// Assert that multi-valued request parameters are preserved
|
||||
assertThat(authorizationCodeRequestAuthenticationCaptor.getValue().getAdditionalParameters())
|
||||
.extracting(ap -> ap.get("foo"))
|
||||
.asInstanceOf(type(String[].class))
|
||||
.isEqualTo(new String[] { "value1", "value2" });
|
||||
assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value());
|
||||
assertThat(response.getRedirectedUrl()).isEqualTo(
|
||||
"https://example.com?param=encoded%20parameter%20value&code=code&state=client%20state");
|
||||
|
||||
@@ -273,7 +273,8 @@ public class OAuth2TokenEndpointFilterTests {
|
||||
assertThat(authorizationCodeAuthentication.getRedirectUri()).isEqualTo(
|
||||
request.getParameter(OAuth2ParameterNames.REDIRECT_URI));
|
||||
assertThat(authorizationCodeAuthentication.getAdditionalParameters())
|
||||
.containsExactly(entry("custom-param-1", "custom-value-1"));
|
||||
.containsExactly(entry("custom-param-1", "custom-value-1"),
|
||||
entry("custom-param-2", new String[]{ "custom-value-2a", "custom-value-2b" }));
|
||||
assertThat(authorizationCodeAuthentication.getDetails())
|
||||
.asInstanceOf(type(WebAuthenticationDetails.class))
|
||||
.extracting(WebAuthenticationDetails::getRemoteAddress)
|
||||
@@ -340,7 +341,8 @@ public class OAuth2TokenEndpointFilterTests {
|
||||
assertThat(clientCredentialsAuthentication.getPrincipal()).isEqualTo(clientPrincipal);
|
||||
assertThat(clientCredentialsAuthentication.getScopes()).isEqualTo(registeredClient.getScopes());
|
||||
assertThat(clientCredentialsAuthentication.getAdditionalParameters())
|
||||
.containsExactly(entry("custom-param-1", "custom-value-1"));
|
||||
.containsExactly(entry("custom-param-1", "custom-value-1"),
|
||||
entry("custom-param-2", new String[]{ "custom-value-2a", "custom-value-2b" }));
|
||||
assertThat(clientCredentialsAuthentication.getDetails())
|
||||
.asInstanceOf(type(WebAuthenticationDetails.class))
|
||||
.extracting(WebAuthenticationDetails::getRemoteAddress)
|
||||
@@ -430,7 +432,8 @@ public class OAuth2TokenEndpointFilterTests {
|
||||
assertThat(refreshTokenAuthenticationToken.getPrincipal()).isEqualTo(clientPrincipal);
|
||||
assertThat(refreshTokenAuthenticationToken.getScopes()).isEqualTo(registeredClient.getScopes());
|
||||
assertThat(refreshTokenAuthenticationToken.getAdditionalParameters())
|
||||
.containsExactly(entry("custom-param-1", "custom-value-1"));
|
||||
.containsExactly(entry("custom-param-1", "custom-value-1"),
|
||||
entry("custom-param-2", new String[]{ "custom-value-2a", "custom-value-2b" }));
|
||||
assertThat(refreshTokenAuthenticationToken.getDetails())
|
||||
.asInstanceOf(type(WebAuthenticationDetails.class))
|
||||
.extracting(WebAuthenticationDetails::getRemoteAddress)
|
||||
@@ -613,6 +616,7 @@ public class OAuth2TokenEndpointFilterTests {
|
||||
// The client does not need to send the client ID param, but we are resilient in case they do
|
||||
request.addParameter(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId());
|
||||
request.addParameter("custom-param-1", "custom-value-1");
|
||||
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
|
||||
|
||||
return request;
|
||||
}
|
||||
@@ -627,6 +631,7 @@ public class OAuth2TokenEndpointFilterTests {
|
||||
request.addParameter(OAuth2ParameterNames.SCOPE,
|
||||
StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " "));
|
||||
request.addParameter("custom-param-1", "custom-value-1");
|
||||
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
|
||||
|
||||
return request;
|
||||
}
|
||||
@@ -642,6 +647,7 @@ public class OAuth2TokenEndpointFilterTests {
|
||||
request.addParameter(OAuth2ParameterNames.SCOPE,
|
||||
StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " "));
|
||||
request.addParameter("custom-param-1", "custom-value-1");
|
||||
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
|
||||
|
||||
return request;
|
||||
}
|
||||
|
||||
@@ -219,7 +219,7 @@ public class OAuth2TokenIntrospectionEndpointFilterTests {
|
||||
MockHttpServletRequest request = createTokenIntrospectionRequest(
|
||||
accessToken.getTokenValue(), OAuth2TokenType.ACCESS_TOKEN.getValue());
|
||||
request.addParameter("custom-param-1", "custom-value-1");
|
||||
request.addParameter("custom-param-2", "custom-value-2");
|
||||
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
|
||||
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
FilterChain filterChain = mock(FilterChain.class);
|
||||
@@ -236,7 +236,7 @@ public class OAuth2TokenIntrospectionEndpointFilterTests {
|
||||
assertThat(tokenIntrospectionAuthentication.getValue().getAdditionalParameters())
|
||||
.contains(
|
||||
entry("custom-param-1", "custom-value-1"),
|
||||
entry("custom-param-2", "custom-value-2"));
|
||||
entry("custom-param-2", new String[]{"custom-value-2a", "custom-value-2b"}));
|
||||
|
||||
OAuth2TokenIntrospection tokenIntrospectionResponse = readTokenIntrospectionResponse(response);
|
||||
assertThat(tokenIntrospectionResponse.isActive()).isEqualTo(tokenClaims.isActive());
|
||||
|
||||
@@ -106,6 +106,7 @@ public class ClientSecretBasicAuthenticationConverterTests {
|
||||
@Test
|
||||
public void convertWhenConfidentialClientWithPkceParametersThenAdditionalParametersIncluded() throws Exception {
|
||||
MockHttpServletRequest request = createPkceTokenRequest();
|
||||
request.addParameter("custom-param-1", "custom-value-1a", "custom-value-1b");
|
||||
request.addHeader(HttpHeaders.AUTHORIZATION, "Basic " + encodeBasicAuth("clientId", "secret"));
|
||||
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
|
||||
assertThat(authentication.getPrincipal()).isEqualTo("clientId");
|
||||
@@ -115,7 +116,8 @@ public class ClientSecretBasicAuthenticationConverterTests {
|
||||
.containsOnly(
|
||||
entry(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()),
|
||||
entry(OAuth2ParameterNames.CODE, "code"),
|
||||
entry(PkceParameterNames.CODE_VERIFIER, "code-verifier-1"));
|
||||
entry(PkceParameterNames.CODE_VERIFIER, "code-verifier-1"),
|
||||
entry("custom-param-1", new String[] { "custom-value-1a", "custom-value-1b" }));
|
||||
}
|
||||
|
||||
private static String encodeBasicAuth(String clientId, String secret) throws Exception {
|
||||
|
||||
@@ -95,6 +95,7 @@ public class ClientSecretPostAuthenticationConverterTests {
|
||||
MockHttpServletRequest request = createPkceTokenRequest();
|
||||
request.addParameter(OAuth2ParameterNames.CLIENT_ID, "client-1");
|
||||
request.addParameter(OAuth2ParameterNames.CLIENT_SECRET, "client-secret");
|
||||
request.addParameter("custom-param-1", "custom-value-1a", "custom-value-1b");
|
||||
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
|
||||
assertThat(authentication.getPrincipal()).isEqualTo("client-1");
|
||||
assertThat(authentication.getCredentials()).isEqualTo("client-secret");
|
||||
@@ -103,7 +104,8 @@ public class ClientSecretPostAuthenticationConverterTests {
|
||||
.containsOnly(
|
||||
entry(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()),
|
||||
entry(OAuth2ParameterNames.CODE, "code"),
|
||||
entry(PkceParameterNames.CODE_VERIFIER, "code-verifier-1"));
|
||||
entry(PkceParameterNames.CODE_VERIFIER, "code-verifier-1"),
|
||||
entry("custom-param-1", new String[] { "custom-value-1a", "custom-value-1b" }));
|
||||
}
|
||||
|
||||
private static MockHttpServletRequest createPkceTokenRequest() {
|
||||
|
||||
@@ -107,6 +107,8 @@ public class JwtClientAssertionAuthenticationConverterTests {
|
||||
request.addParameter(OAuth2ParameterNames.CLIENT_ID, "client-1");
|
||||
request.addParameter(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue());
|
||||
request.addParameter(OAuth2ParameterNames.CODE, "code");
|
||||
request.addParameter("custom-param-1", "custom-value-1");
|
||||
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
|
||||
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
|
||||
assertThat(authentication.getPrincipal()).isEqualTo("client-1");
|
||||
assertThat(authentication.getCredentials()).isEqualTo("jwt-assertion");
|
||||
@@ -114,7 +116,9 @@ public class JwtClientAssertionAuthenticationConverterTests {
|
||||
assertThat(authentication.getAdditionalParameters())
|
||||
.containsOnly(
|
||||
entry(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()),
|
||||
entry(OAuth2ParameterNames.CODE, "code"));
|
||||
entry(OAuth2ParameterNames.CODE, "code"),
|
||||
entry("custom-param-1", "custom-value-1"),
|
||||
entry("custom-param-2", new String[] {"custom-value-2a", "custom-value-2b"}));
|
||||
}
|
||||
|
||||
private void assertThrown(MockHttpServletRequest request, String errorCode) {
|
||||
|
||||
@@ -82,6 +82,8 @@ public class PublicClientAuthenticationConverterTests {
|
||||
@Test
|
||||
public void convertWhenPublicClientThenReturnClientAuthenticationToken() {
|
||||
MockHttpServletRequest request = createPkceTokenRequest();
|
||||
request.addParameter("param-1", "value-1");
|
||||
request.addParameter("param-2", "value-2", "value-2b");
|
||||
OAuth2ClientAuthenticationToken authentication = (OAuth2ClientAuthenticationToken) this.converter.convert(request);
|
||||
assertThat(authentication.getPrincipal()).isEqualTo("client-1");
|
||||
assertThat(authentication.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.NONE);
|
||||
@@ -89,7 +91,9 @@ public class PublicClientAuthenticationConverterTests {
|
||||
.containsOnly(
|
||||
entry(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()),
|
||||
entry(OAuth2ParameterNames.CODE, "code"),
|
||||
entry(PkceParameterNames.CODE_VERIFIER, "code-verifier-1"));
|
||||
entry(PkceParameterNames.CODE_VERIFIER, "code-verifier-1"),
|
||||
entry("param-1", "value-1"),
|
||||
entry("param-2", new String[] {"value-2", "value-2b"}));
|
||||
}
|
||||
|
||||
private static MockHttpServletRequest createPkceTokenRequest() {
|
||||
|
||||
Reference in New Issue
Block a user