Skip to content

Fix to save all values for multi-valued request parameters #1252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Authentication convert(HttpServletRequest request) {
!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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public Authentication convert(HttpServletRequest request) {
!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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public Authentication convert(HttpServletRequest request) {
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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public Authentication convert(HttpServletRequest request) {
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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public Authentication convert(HttpServletRequest request) {
!key.equals(OAuth2ParameterNames.USER_CODE) &&
!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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public Authentication convert(HttpServletRequest request) {
parameters.forEach((key, value) -> {
if (!key.equals(OAuth2ParameterNames.CLIENT_ID) &&
!key.equals(OAuth2ParameterNames.SCOPE)) {
additionalParameters.put(key, value.get(0));
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public Authentication convert(HttpServletRequest request) {
if (!key.equals(OAuth2ParameterNames.GRANT_TYPE) &&
!key.equals(OAuth2ParameterNames.CLIENT_ID) &&
!key.equals(OAuth2ParameterNames.DEVICE_CODE)) {
additionalParameters.put(key, value.get(0));
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public Authentication convert(HttpServletRequest request) {
Map<String, Object> additionalParameters = new HashMap<>();
parameters.forEach((key, value) -> {
if (!key.equals(OAuth2ParameterNames.USER_CODE)) {
additionalParameters.put(key, value.get(0));
additionalParameters.put(key, value.size() == 1 ? value.get(0) : value.toArray(new String[0]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 jakarta.servlet.http.HttpServletRequest;

Expand Down Expand Up @@ -59,11 +59,13 @@ static Map<String, Object> getParametersIfMatchesAuthorizationCodeGrantRequest(H
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public Authentication convert(HttpServletRequest request) {
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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public Authentication convert(HttpServletRequest request) {
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]));
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.security.oauth2.server.authorization.web.authentication;

import java.util.HashMap;
import java.util.Map;

import jakarta.servlet.http.HttpServletRequest;

Expand Down Expand Up @@ -68,7 +69,12 @@ public Authentication convert(HttpServletRequest request) {

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,8 @@ public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationRespon
.thenReturn(authorizationCodeRequestAuthenticationResult);

MockHttpServletRequest request = createAuthorizationRequest(registeredClient);
request.addParameter("foo", "value1", "value2");

MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);

Expand All @@ -603,6 +605,12 @@ public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationRespon
.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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ public void doFilterWhenDeviceAuthorizationRequestThenDeviceAuthorizationRespons

MockHttpServletRequest request = createRequest();
request.addParameter("custom-param-1", "custom-value-1");
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
Expand All @@ -211,7 +212,8 @@ public void doFilterWhenDeviceAuthorizationRequestThenDeviceAuthorizationRespons
assertThat(deviceAuthorizationRequestAuthentication.getPrincipal()).isEqualTo(clientPrincipal);
assertThat(deviceAuthorizationRequestAuthentication.getScopes()).isEmpty();
assertThat(deviceAuthorizationRequestAuthentication.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" }));
// @formatter:off
assertThat(deviceAuthorizationRequestAuthentication.getDetails())
.asInstanceOf(type(WebAuthenticationDetails.class))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ public void doFilterWhenDeviceAuthorizationConsentRequestThenSuccess() throws Ex
request.addParameter(OAuth2ParameterNames.STATE, STATE);
request.addParameter(OAuth2ParameterNames.USER_CODE, USER_CODE);
request.addParameter("custom-param-1", "custom-value-1");
request.addParameter("custom-param-2", "custom-value-2a", "custom-value-2b");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = mock(FilterChain.class);
this.filter.doFilter(request, response, filterChain);
Expand All @@ -207,7 +208,7 @@ public void doFilterWhenDeviceAuthorizationConsentRequestThenSuccess() throws Ex
assertThat(deviceAuthorizationConsentAuthentication.getUserCode()).isEqualTo(USER_CODE);
assertThat(deviceAuthorizationConsentAuthentication.getScopes()).containsExactly("scope-1", "scope-2");
assertThat(deviceAuthorizationConsentAuthentication.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" }));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ public void doFilterWhenAuthorizationCodeTokenRequestThenAccessTokenResponse() t
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)
Expand Down Expand Up @@ -340,7 +341,8 @@ public void doFilterWhenClientCredentialsTokenRequestThenAccessTokenResponse() t
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)
Expand Down Expand Up @@ -430,7 +432,8 @@ public void doFilterWhenRefreshTokenRequestThenAccessTokenResponse() throws Exce
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)
Expand Down Expand Up @@ -613,6 +616,7 @@ private static MockHttpServletRequest createAuthorizationCodeTokenRequest(Regist
// 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;
}
Expand All @@ -627,6 +631,7 @@ private static MockHttpServletRequest createClientCredentialsTokenRequest(Regist
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;
}
Expand All @@ -642,6 +647,7 @@ private static MockHttpServletRequest createRefreshTokenTokenRequest(RegisteredC
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public void doFilterWhenTokenIntrospectionRequestValidThenSuccessResponse() thro
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);
Expand All @@ -236,7 +236,7 @@ public void doFilterWhenTokenIntrospectionRequestValidThenSuccessResponse() thro
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public void convertWhenAuthorizationHeaderBasicWithValidCredentialsThenReturnCli
@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");
Expand All @@ -115,7 +116,8 @@ public void convertWhenConfidentialClientWithPkceParametersThenAdditionalParamet
.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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public void convertWhenConfidentialClientWithPkceParametersThenAdditionalParamet
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");
Expand All @@ -103,7 +104,8 @@ public void convertWhenConfidentialClientWithPkceParametersThenAdditionalParamet
.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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ public void convertWhenJwtAssertionThenReturnClientAuthenticationToken() {
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");
assertThat(authentication.getClientAuthenticationMethod().getValue()).isEqualTo(JWT_BEARER_TYPE);
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public void convertWhenAllParametersThenReturnDeviceAuthorizationConsentAuthenti
request.addParameter(OAuth2ParameterNames.SCOPE, "message.read");
request.addParameter(OAuth2ParameterNames.SCOPE, "message.write");
request.addParameter("param-1", "value-1");
request.addParameter("param-2", "value-2");
request.addParameter("param-2", "value-2", "value-2b");

SecurityContextImpl securityContext = new SecurityContextImpl();
securityContext.setAuthentication(new TestingAuthenticationToken("user", null));
Expand All @@ -261,7 +261,8 @@ public void convertWhenAllParametersThenReturnDeviceAuthorizationConsentAuthenti
assertThat(authentication.getUserCode()).isEqualTo(USER_CODE);
assertThat(authentication.getScopes()).containsExactly("message.read", "message.write");
assertThat(authentication.getAdditionalParameters())
.containsExactly(entry("param-1", "value-1"), entry("param-2", "value-2"));
.containsExactly(entry("param-1", "value-1"),
entry("param-2", new String[]{"value-2", "value-2b"}));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void convertWhenAllParametersThenReturnDeviceAuthorizationRequestAuthenti
request.addParameter(OAuth2ParameterNames.CLIENT_ID, CLIENT_ID);
request.addParameter(OAuth2ParameterNames.SCOPE, "message.read message.write");
request.addParameter("param-1", "value-1");
request.addParameter("param-2", "value-2");
request.addParameter("param-2", "value-2", "value-2b");

SecurityContextImpl securityContext = new SecurityContextImpl();
securityContext.setAuthentication(new TestingAuthenticationToken(CLIENT_ID, null));
Expand All @@ -108,7 +108,8 @@ public void convertWhenAllParametersThenReturnDeviceAuthorizationRequestAuthenti
assertThat(authentication.getAuthorizationUri()).endsWith(AUTHORIZATION_URI);
assertThat(authentication.getScopes()).containsExactly("message.read", "message.write");
assertThat(authentication.getAdditionalParameters())
.containsExactly(entry("param-1", "value-1"), entry("param-2", "value-2"));
.containsExactly(entry("param-1", "value-1"),
entry("param-2", new String[]{"value-2", "value-2b"}));
}

private static MockHttpServletRequest createRequest() {
Expand Down
Loading