Skip to content

Commit 99681f1

Browse files
sjohnrsjohnr
committed
Polish gh-15533 
1 parent d40b242 commit 99681f1

File tree

2 files changed

+63
-38
lines changed

2 files changed

+63
-38
lines changed

‎oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,13 +23,11 @@
2323
import java.util.Collections;
2424
import java.util.LinkedHashMap;
2525
import java.util.LinkedHashSet;
26-
import java.util.List;
2726
import java.util.Map;
2827
import java.util.Set;
2928
import java.util.function.Consumer;
3029
import java.util.function.Function;
31-
import java.util.stream.Stream;
32-
import java.util.stream.StreamSupport;
30+
3331
import org.springframework.security.core.SpringSecurityCoreVersion;
3432
import org.springframework.security.oauth2.core.AuthorizationGrantType;
3533
import org.springframework.util.Assert;
@@ -442,12 +440,20 @@ private String buildAuthorizationRequestUri() {
442440
Map<String, Object> parameters = getParameters(); // Not encoded
443441
this.parametersConsumer.accept(parameters);
444442
MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();
445-
parameters.forEach((key1, value) -> {
446-
String key = encodeQueryParam(key1);
447-
List<String> values = queryValues(value)
448-
.map(o -> encodeQueryParam(String.valueOf(o)))
449-
.toList();
450-
queryParams.put(key, values);
443+
parameters.forEach((k, v) -> {
444+
String key = encodeQueryParam(k);
445+
if (v instanceof Iterable) {
446+
((Iterable<?>) v).forEach((value) -> queryParams.add(key, encodeQueryParam(String.valueOf(value))));
447+
}
448+
else if (v != null && v.getClass().isArray()) {
449+
Object[] values = (Object[]) v;
450+
for (Object value : values) {
451+
queryParams.add(key, encodeQueryParam(String.valueOf(value)));
452+
}
453+
}
454+
else {
455+
queryParams.set(key, encodeQueryParam(String.valueOf(v)));
456+
}
451457
});
452458
UriBuilder uriBuilder = this.uriBuilderFactory.uriString(this.authorizationUri).queryParams(queryParams);
453459
return this.authorizationRequestUriFunction.apply(uriBuilder).toString();
@@ -475,20 +481,6 @@ private static String encodeQueryParam(String value) {
475481
return UriUtils.encodeQueryParam(value, StandardCharsets.UTF_8);
476482
}
477483

478-
// Query value as a stream
479-
// If the value is an Iterable or an array it will be converted to a stream
480-
private static Stream<?> queryValues(Object value) {
481-
if (value instanceof Iterable) {
482-
return StreamSupport.stream(((Iterable<?>) value).spliterator(), false);
483-
484-
} else if (value.getClass().isArray()) {
485-
return Arrays.stream((Object[]) value);
486-
487-
} else {
488-
return Stream.of(value);
489-
}
490-
}
491-
492484
}
493485

494486
}

‎oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,18 +16,21 @@
1616

1717
package org.springframework.security.oauth2.core.endpoint;
1818

19-
import static org.assertj.core.api.Assertions.assertThat;
20-
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
21-
2219
import java.net.URI;
2320
import java.util.Arrays;
2421
import java.util.HashMap;
22+
import java.util.LinkedHashMap;
2523
import java.util.LinkedHashSet;
2624
import java.util.Map;
2725
import java.util.Set;
26+
2827
import org.junit.jupiter.api.Test;
28+
2929
import org.springframework.security.oauth2.core.AuthorizationGrantType;
3030

31+
import static org.assertj.core.api.Assertions.assertThat;
32+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
33+
3134
/**
3235
* Tests for {@link OAuth2AuthorizationRequest}.
3336
*
@@ -318,18 +321,48 @@ public void buildWhenNonAsciiAdditionalParametersThenProperlyEncoded() {
318321
}
319322

320323
@Test
321-
public void additionalParametersArrayValueOrIterableEncoded() {
322-
Map<String, Object> additionalParameters = new HashMap<>();
323-
additionalParameters.put("item", new String[] { "1", "2" });
324-
additionalParameters.put("item2", Arrays.asList("H" + '\u00c5' + "M" + '\u00d6', "H" + '\u00c5' + "M" + '\u00d6'));
324+
public void buildWhenAdditionalParametersContainsArrayThenProperlyEncoded() {
325+
Map<String, Object> additionalParameters = new LinkedHashMap<>();
326+
additionalParameters.put("item1", new String[] { "1", "2" });
327+
additionalParameters.put("item2", "value2");
325328
OAuth2AuthorizationRequest authorizationRequest = TestOAuth2AuthorizationRequests.request()
326-
.additionalParameters(additionalParameters)
327-
.build();
329+
.additionalParameters(additionalParameters)
330+
.build();
328331
assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
329-
assertThat(authorizationRequest.getAuthorizationRequestUri()).isEqualTo(
330-
"https://example.com/login/oauth/authorize?" + "response_type=code&client_id=client-id&state=state&"
331-
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
332-
+ "item=1&item=2&item2=H%C3%85M%C3%96&item2=H%C3%85M%C3%96");
332+
assertThat(authorizationRequest.getAuthorizationRequestUri())
333+
.isEqualTo("https://example.com/login/oauth/authorize?response_type=code&client_id=client-id&state=state&"
334+
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
335+
+ "item1=1&item1=2&item2=value2");
336+
}
337+
338+
@Test
339+
public void buildWhenAdditionalParametersContainsIterableThenProperlyEncoded() {
340+
Map<String, Object> additionalParameters = new LinkedHashMap<>();
341+
additionalParameters.put("item1", Arrays.asList("1", "2"));
342+
additionalParameters.put("item2", "value2");
343+
OAuth2AuthorizationRequest authorizationRequest = TestOAuth2AuthorizationRequests.request()
344+
.additionalParameters(additionalParameters)
345+
.build();
346+
assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
347+
assertThat(authorizationRequest.getAuthorizationRequestUri())
348+
.isEqualTo("https://example.com/login/oauth/authorize?response_type=code&client_id=client-id&state=state&"
349+
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
350+
+ "item1=1&item1=2&item2=value2");
351+
}
352+
353+
@Test
354+
public void buildWhenAdditionalParametersContainsNullThenAuthorizationRequestUriContainsNull() {
355+
Map<String, Object> additionalParameters = new LinkedHashMap<>();
356+
additionalParameters.put("item1", null);
357+
additionalParameters.put("item2", "value2");
358+
OAuth2AuthorizationRequest authorizationRequest = TestOAuth2AuthorizationRequests.request()
359+
.additionalParameters(additionalParameters)
360+
.build();
361+
assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
362+
assertThat(authorizationRequest.getAuthorizationRequestUri())
363+
.isEqualTo("https://example.com/login/oauth/authorize?response_type=code&client_id=client-id&state=state&"
364+
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
365+
+ "item1=null&item2=value2");
333366
}
334367

335368
}

0 commit comments

Comments
 (0)