Skip to content

Commit 7b7a304

Browse files
sjohnrsjohnr
committed
Polish gh-15533 
1 parent 8a79102 commit 7b7a304

File tree

2 files changed

+63
-38
lines changed

2 files changed

+63
-38
lines changed

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

+16-24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 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;
@@ -465,12 +463,20 @@ private String buildAuthorizationRequestUri() {
465463
Map<String, Object> parameters = getParameters(); // Not encoded
466464
this.parametersConsumer.accept(parameters);
467465
MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();
468-
parameters.forEach((key1, value) -> {
469-
String key = encodeQueryParam(key1);
470-
List<String> values = queryValues(value)
471-
.map(o -> encodeQueryParam(String.valueOf(o)))
472-
.toList();
473-
queryParams.put(key, values);
466+
parameters.forEach((k, v) -> {
467+
String key = encodeQueryParam(k);
468+
if (v instanceof Iterable) {
469+
((Iterable<?>) v).forEach((value) -> queryParams.add(key, encodeQueryParam(String.valueOf(value))));
470+
}
471+
else if (v != null && v.getClass().isArray()) {
472+
Object[] values = (Object[]) v;
473+
for (Object value : values) {
474+
queryParams.add(key, encodeQueryParam(String.valueOf(value)));
475+
}
476+
}
477+
else {
478+
queryParams.set(key, encodeQueryParam(String.valueOf(v)));
479+
}
474480
});
475481
UriBuilder uriBuilder = this.uriBuilderFactory.uriString(this.authorizationUri).queryParams(queryParams);
476482
return this.authorizationRequestUriFunction.apply(uriBuilder).toString();
@@ -498,20 +504,6 @@ private static String encodeQueryParam(String value) {
498504
return UriUtils.encodeQueryParam(value, StandardCharsets.UTF_8);
499505
}
500506

501-
// Query value as a stream
502-
// If the value is an Iterable or an array it will be converted to a stream
503-
private static Stream<?> queryValues(Object value) {
504-
if (value instanceof Iterable) {
505-
return StreamSupport.stream(((Iterable<?>) value).spliterator(), false);
506-
507-
} else if (value.getClass().isArray()) {
508-
return Arrays.stream((Object[]) value);
509-
510-
} else {
511-
return Stream.of(value);
512-
}
513-
}
514-
515507
}
516508

517509
}

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

+47-14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 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
*
@@ -363,18 +366,48 @@ public void buildWhenNonAsciiAdditionalParametersThenProperlyEncoded() {
363366
}
364367

365368
@Test
366-
public void additionalParametersArrayValueOrIterableEncoded() {
367-
Map<String, Object> additionalParameters = new HashMap<>();
368-
additionalParameters.put("item", new String[] { "1", "2" });
369-
additionalParameters.put("item2", Arrays.asList("H" + '\u00c5' + "M" + '\u00d6', "H" + '\u00c5' + "M" + '\u00d6'));
369+
public void buildWhenAdditionalParametersContainsArrayThenProperlyEncoded() {
370+
Map<String, Object> additionalParameters = new LinkedHashMap<>();
371+
additionalParameters.put("item1", new String[] { "1", "2" });
372+
additionalParameters.put("item2", "value2");
370373
OAuth2AuthorizationRequest authorizationRequest = TestOAuth2AuthorizationRequests.request()
371-
.additionalParameters(additionalParameters)
372-
.build();
374+
.additionalParameters(additionalParameters)
375+
.build();
373376
assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
374-
assertThat(authorizationRequest.getAuthorizationRequestUri()).isEqualTo(
375-
"https://example.com/login/oauth/authorize?" + "response_type=code&client_id=client-id&state=state&"
376-
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
377-
+ "item=1&item=2&item2=H%C3%85M%C3%96&item2=H%C3%85M%C3%96");
377+
assertThat(authorizationRequest.getAuthorizationRequestUri())
378+
.isEqualTo("https://example.com/login/oauth/authorize?response_type=code&client_id=client-id&state=state&"
379+
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
380+
+ "item1=1&item1=2&item2=value2");
381+
}
382+
383+
@Test
384+
public void buildWhenAdditionalParametersContainsIterableThenProperlyEncoded() {
385+
Map<String, Object> additionalParameters = new LinkedHashMap<>();
386+
additionalParameters.put("item1", Arrays.asList("1", "2"));
387+
additionalParameters.put("item2", "value2");
388+
OAuth2AuthorizationRequest authorizationRequest = TestOAuth2AuthorizationRequests.request()
389+
.additionalParameters(additionalParameters)
390+
.build();
391+
assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
392+
assertThat(authorizationRequest.getAuthorizationRequestUri())
393+
.isEqualTo("https://example.com/login/oauth/authorize?response_type=code&client_id=client-id&state=state&"
394+
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
395+
+ "item1=1&item1=2&item2=value2");
396+
}
397+
398+
@Test
399+
public void buildWhenAdditionalParametersContainsNullThenAuthorizationRequestUriContainsNull() {
400+
Map<String, Object> additionalParameters = new LinkedHashMap<>();
401+
additionalParameters.put("item1", null);
402+
additionalParameters.put("item2", "value2");
403+
OAuth2AuthorizationRequest authorizationRequest = TestOAuth2AuthorizationRequests.request()
404+
.additionalParameters(additionalParameters)
405+
.build();
406+
assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
407+
assertThat(authorizationRequest.getAuthorizationRequestUri())
408+
.isEqualTo("https://example.com/login/oauth/authorize?response_type=code&client_id=client-id&state=state&"
409+
+ "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&"
410+
+ "item1=null&item2=value2");
378411
}
379412

380413
}

0 commit comments

Comments
 (0)