Skip to content

Commit f6c650d

Browse files
kostya05983jzheaux
kostya05983
authored and
jzheaux
committed
Replace Streams with Loops
First version of replacing streams fix wwwAuthenticate and codestyle fix errors in implementation to pass tests Fix review notes Remove uneccessary final to align with cb Short circuit way to authorize Simplify error message, make code readably Return error while duplicate key found Delete check for duplicate, checkstyle issues Return duplicate error Fixes gh-7154
1 parent d6d0d89 commit f6c650d

File tree

30 files changed

+282
-183
lines changed

30 files changed

+282
-183
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java

+16-6
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,11 @@
4343
import org.springframework.security.crypto.password.PasswordEncoder;
4444
import org.springframework.util.Assert;
4545

46-
import java.util.Arrays;
4746
import java.util.Collections;
4847
import java.util.List;
4948
import java.util.Map;
49+
import java.util.ArrayList;
5050
import java.util.concurrent.atomic.AtomicBoolean;
51-
import java.util.stream.Collectors;
5251

5352
/**
5453
* Exports the authentication {@link Configuration}
@@ -153,10 +152,7 @@ private <T> T lazyBean(Class<T> interfaceName) {
153152
}
154153
String beanName;
155154
if (beanNamesForType.length > 1) {
156-
List<String> primaryBeanNames = Arrays.stream(beanNamesForType)
157-
.filter(i -> applicationContext instanceof ConfigurableApplicationContext)
158-
.filter(n -> ((ConfigurableApplicationContext) applicationContext).getBeanFactory().getBeanDefinition(n).isPrimary())
159-
.collect(Collectors.toList());
155+
List<String> primaryBeanNames = getPrimaryBeanNames(beanNamesForType);
160156

161157
Assert.isTrue(primaryBeanNames.size() != 0, () -> "Found " + beanNamesForType.length
162158
+ " beans for type " + interfaceName + ", but none marked as primary");
@@ -175,6 +171,20 @@ private <T> T lazyBean(Class<T> interfaceName) {
175171
return (T) proxyFactory.getObject();
176172
}
177173

174+
private List<String> getPrimaryBeanNames(String[] beanNamesForType) {
175+
List<String> list = new ArrayList<>();
176+
if (!(applicationContext instanceof ConfigurableApplicationContext)) {
177+
return Collections.emptyList();
178+
}
179+
for (String beanName : beanNamesForType) {
180+
if (((ConfigurableApplicationContext) applicationContext).getBeanFactory()
181+
.getBeanDefinition(beanName).isPrimary()) {
182+
list.add(beanName);
183+
}
184+
}
185+
return list;
186+
}
187+
178188
private AuthenticationManager getAuthenticationManagerBean() {
179189
return lazyBean(AuthenticationManager.class);
180190
}

‎core/src/main/java/org/springframework/security/authorization/AuthorityReactiveAuthorizationManager.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import java.util.Arrays;
2424
import java.util.List;
25-
import java.util.stream.Stream;
2625

2726
/**
2827
* A {@link ReactiveAuthorizationManager} that determines if the current user is
@@ -109,9 +108,14 @@ public static <T> AuthorityReactiveAuthorizationManager<T> hasAnyRole(String...
109108
Assert.notNull(role, "role cannot be null");
110109
}
111110

112-
return hasAnyAuthority(Stream.of(roles)
113-
.map(r -> "ROLE_" + r)
114-
.toArray(String[]::new)
115-
);
111+
return hasAnyAuthority(toNamedRolesArray(roles));
112+
}
113+
114+
private static String[] toNamedRolesArray(String... roles) {
115+
String[] result = new String[roles.length];
116+
for (int i=0; i < roles.length; i++) {
117+
result[i] = "ROLE_" + roles[i];
118+
}
119+
return result;
116120
}
117121
}

‎core/src/main/java/org/springframework/security/converter/RsaKeyConverters.java

+16-10
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@
1616

1717
package org.springframework.security.converter;
1818

19-
import java.io.BufferedReader;
2019
import java.io.InputStream;
20+
import java.io.BufferedReader;
2121
import java.io.InputStreamReader;
2222
import java.security.KeyFactory;
2323
import java.security.NoSuchAlgorithmException;
2424
import java.security.interfaces.RSAPrivateKey;
2525
import java.security.interfaces.RSAPublicKey;
2626
import java.security.spec.PKCS8EncodedKeySpec;
2727
import java.security.spec.X509EncodedKeySpec;
28-
import java.util.Base64;
2928
import java.util.List;
29+
import java.util.Base64;
3030
import java.util.stream.Collectors;
3131

3232
import org.springframework.core.convert.converter.Converter;
@@ -66,10 +66,13 @@ public static Converter<InputStream, RSAPrivateKey> pkcs8() {
6666
Assert.isTrue(!lines.isEmpty() && lines.get(0).startsWith(PKCS8_PEM_HEADER),
6767
"Key is not in PEM-encoded PKCS#8 format, " +
6868
"please check that the header begins with -----" + PKCS8_PEM_HEADER + "-----");
69-
String base64Encoded = lines.stream()
70-
.filter(RsaKeyConverters::isNotPkcs8Wrapper)
71-
.collect(Collectors.joining());
72-
byte[] pkcs8 = Base64.getDecoder().decode(base64Encoded);
69+
StringBuilder base64Encoded = new StringBuilder();
70+
for (String line : lines) {
71+
if (RsaKeyConverters.isNotPkcs8Wrapper(line)) {
72+
base64Encoded.append(line);
73+
}
74+
}
75+
byte[] pkcs8 = Base64.getDecoder().decode(base64Encoded.toString());
7376

7477
try {
7578
return (RSAPrivateKey) keyFactory.generatePrivate(
@@ -97,10 +100,13 @@ public static Converter<InputStream, RSAPublicKey> x509() {
97100
Assert.isTrue(!lines.isEmpty() && lines.get(0).startsWith(X509_PEM_HEADER),
98101
"Key is not in PEM-encoded X.509 format, " +
99102
"please check that the header begins with -----" + X509_PEM_HEADER + "-----");
100-
String base64Encoded = lines.stream()
101-
.filter(RsaKeyConverters::isNotX509Wrapper)
102-
.collect(Collectors.joining());
103-
byte[] x509 = Base64.getDecoder().decode(base64Encoded);
103+
StringBuilder base64Encoded = new StringBuilder();
104+
for (String line : lines) {
105+
if (RsaKeyConverters.isNotX509Wrapper(line)) {
106+
base64Encoded.append(line);
107+
}
108+
}
109+
byte[] x509 = Base64.getDecoder().decode(base64Encoded.toString());
104110

105111
try {
106112
return (RSAPublicKey) keyFactory.generatePublic(

‎core/src/main/java/org/springframework/security/core/userdetails/MapReactiveUserDetailsService.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919
import java.util.Arrays;
2020
import java.util.Collection;
2121
import java.util.Map;
22-
import java.util.function.Function;
23-
import java.util.stream.Collectors;
22+
import java.util.concurrent.ConcurrentHashMap;
2423

2524
import org.springframework.util.Assert;
2625
import reactor.core.publisher.Mono;
@@ -56,7 +55,10 @@ public MapReactiveUserDetailsService(UserDetails... users) {
5655
*/
5756
public MapReactiveUserDetailsService(Collection<UserDetails> users) {
5857
Assert.notEmpty(users, "users cannot be null or empty");
59-
this.users = users.stream().collect(Collectors.toConcurrentMap( u -> getKey(u.getUsername()), Function.identity()));
58+
this.users = new ConcurrentHashMap<>();
59+
for (UserDetails user : users) {
60+
this.users.put(getKey(user.getUsername()), user);
61+
}
6062
}
6163

6264
@Override

‎core/src/test/java/org/springframework/security/core/userdetails/MapReactiveUserDetailsServiceTests.java

+7
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ public void constructorEmptyUsers() {
4646
new MapReactiveUserDetailsService(users);
4747
}
4848

49+
@Test
50+
public void constructorCaseIntensiveKey() {
51+
UserDetails userDetails = User.withUsername("USER").password("password").roles("USER").build();
52+
MapReactiveUserDetailsService userDetailsService = new MapReactiveUserDetailsService(userDetails);
53+
assertThat(userDetailsService.findByUsername("user").block()).isEqualTo(userDetails);
54+
}
55+
4956
@Test
5057
public void findByUsernameWhenFoundThenReturns() {
5158
assertThat((users.findByUsername(USER_DETAILS.getUsername()).block())).isEqualTo(USER_DETAILS);

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/DelegatingOAuth2AuthorizedClientProvider.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import java.util.Arrays;
2323
import java.util.Collections;
2424
import java.util.List;
25-
import java.util.Objects;
2625

2726
/**
2827
* An implementation of an {@link OAuth2AuthorizedClientProvider} that simply delegates
@@ -64,10 +63,12 @@ public DelegatingOAuth2AuthorizedClientProvider(List<OAuth2AuthorizedClientProvi
6463
@Nullable
6564
public OAuth2AuthorizedClient authorize(OAuth2AuthorizationContext context) {
6665
Assert.notNull(context, "context cannot be null");
67-
return this.authorizedClientProviders.stream()
68-
.map(authorizedClientProvider -> authorizedClientProvider.authorize(context))
69-
.filter(Objects::nonNull)
70-
.findFirst()
71-
.orElse(null);
66+
for (OAuth2AuthorizedClientProvider authorizedClientProvider : authorizedClientProviders) {
67+
OAuth2AuthorizedClient oauth2AuthorizedClient = authorizedClientProvider.authorize(context);
68+
if (oauth2AuthorizedClient != null) {
69+
return oauth2AuthorizedClient;
70+
}
71+
}
72+
return null;
7273
}
7374
}

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/OAuth2AuthorizedClientProviderBuilder.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@
2323
import java.time.Clock;
2424
import java.time.Duration;
2525
import java.time.Instant;
26-
import java.util.LinkedHashMap;
27-
import java.util.List;
2826
import java.util.Map;
27+
import java.util.List;
28+
import java.util.LinkedHashMap;
29+
import java.util.ArrayList;
2930
import java.util.function.Consumer;
30-
import java.util.stream.Collectors;
3131

3232
/**
3333
* A builder that builds a {@link DelegatingOAuth2AuthorizedClientProvider} composed of
@@ -286,10 +286,10 @@ public OAuth2AuthorizedClientProvider build() {
286286
* @return the {@link DelegatingOAuth2AuthorizedClientProvider}
287287
*/
288288
public OAuth2AuthorizedClientProvider build() {
289-
List<OAuth2AuthorizedClientProvider> authorizedClientProviders =
290-
this.builders.values().stream()
291-
.map(Builder::build)
292-
.collect(Collectors.toList());
289+
List<OAuth2AuthorizedClientProvider> authorizedClientProviders = new ArrayList<>();
290+
for (Builder builder : this.builders.values()) {
291+
authorizedClientProviders.add(builder.build());
292+
}
293293
return new DelegatingOAuth2AuthorizedClientProvider(authorizedClientProviders);
294294
}
295295

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenValidator.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import java.util.HashMap;
3333
import java.util.List;
3434
import java.util.Map;
35-
import java.util.stream.Collectors;
3635

3736
/**
3837
* An {@link OAuth2TokenValidator} responsible for
@@ -137,11 +136,8 @@ public void setClockSkew(Duration clockSkew) {
137136
}
138137

139138
private static OAuth2Error invalidIdToken(Map<String, Object> invalidClaims) {
140-
String claimsDetail = invalidClaims.entrySet().stream()
141-
.map(it -> it.getKey() + " (" + it.getValue() + ")")
142-
.collect(Collectors.joining(", "));
143139
return new OAuth2Error("invalid_id_token",
144-
"The ID Token contains invalid claims: " + claimsDetail,
140+
"The ID Token contains invalid claims: " + invalidClaims,
145141
"https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation");
146142
}
147143

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java

+14-9
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,7 @@
2222
import java.util.Iterator;
2323
import java.util.List;
2424
import java.util.Map;
25-
import java.util.concurrent.ConcurrentMap;
26-
import java.util.function.Function;
27-
import java.util.stream.Collector;
28-
29-
import static java.util.stream.Collectors.collectingAndThen;
30-
import static java.util.stream.Collectors.toConcurrentMap;
25+
import java.util.concurrent.ConcurrentHashMap;
3126

3227
/**
3328
* A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory.
@@ -62,9 +57,19 @@ public InMemoryClientRegistrationRepository(List<ClientRegistration> registratio
6257

6358
private static Map<String, ClientRegistration> createRegistrationsMap(List<ClientRegistration> registrations) {
6459
Assert.notEmpty(registrations, "registrations cannot be empty");
65-
Collector<ClientRegistration, ?, ConcurrentMap<String, ClientRegistration>> collector =
66-
toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity());
67-
return registrations.stream().collect(collectingAndThen(collector, Collections::unmodifiableMap));
60+
return toUnmodifiableConcurrentMap(registrations);
61+
}
62+
63+
private static Map<String, ClientRegistration> toUnmodifiableConcurrentMap(List<ClientRegistration> registrations) {
64+
ConcurrentHashMap<String, ClientRegistration> result = new ConcurrentHashMap<>();
65+
for (ClientRegistration registration : registrations) {
66+
if (result.containsKey(registration.getRegistrationId())) {
67+
throw new IllegalStateException(String.format("Duplicate key %s",
68+
registration.getRegistrationId()));
69+
}
70+
result.put(registration.getRegistrationId(), registration);
71+
}
72+
return Collections.unmodifiableMap(result);
6873
}
6974

7075
/**

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepository.java

+10-5
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919
import java.util.List;
2020
import java.util.Map;
2121
import java.util.concurrent.ConcurrentHashMap;
22-
import java.util.function.Function;
23-
import java.util.stream.Collectors;
22+
import java.util.concurrent.ConcurrentHashMap;
2423

2524
import org.springframework.util.Assert;
2625

@@ -61,11 +60,9 @@ public InMemoryReactiveClientRegistrationRepository(ClientRegistration... regist
6160
*/
6261
public InMemoryReactiveClientRegistrationRepository(List<ClientRegistration> registrations) {
6362
Assert.notEmpty(registrations, "registrations cannot be null or empty");
64-
this.clientIdToClientRegistration = registrations.stream()
65-
.collect(Collectors.toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity()));
63+
this.clientIdToClientRegistration = toConcurrentMap(registrations);
6664
}
6765

68-
6966
@Override
7067
public Mono<ClientRegistration> findByRegistrationId(String registrationId) {
7168
return Mono.justOrEmpty(this.clientIdToClientRegistration.get(registrationId));
@@ -80,4 +77,12 @@ public Mono<ClientRegistration> findByRegistrationId(String registrationId) {
8077
public Iterator<ClientRegistration> iterator() {
8178
return this.clientIdToClientRegistration.values().iterator();
8279
}
80+
81+
private ConcurrentHashMap<String, ClientRegistration> toConcurrentMap(List<ClientRegistration> registrations) {
82+
ConcurrentHashMap<String, ClientRegistration> result = new ConcurrentHashMap<>();
83+
for (ClientRegistration registration : registrations) {
84+
result.put(registration.getRegistrationId(), registration);
85+
}
86+
return result;
87+
}
8388
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenValidatorTests.java

+10
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,16 @@ public void validateWhenMissingClaimsThenHasErrors() {
229229
.allMatch(msg -> msg.contains(IdTokenClaimNames.EXP));
230230
}
231231

232+
@Test
233+
public void validateFormatError() {
234+
this.claims.remove(IdTokenClaimNames.SUB);
235+
this.claims.remove(IdTokenClaimNames.AUD);
236+
assertThat(this.validateIdToken())
237+
.hasSize(1)
238+
.extracting(OAuth2Error::getDescription)
239+
.allMatch(msg -> msg.equals("The ID Token contains invalid claims: {sub=null, aud=null}"));
240+
}
241+
232242
private Collection<OAuth2Error> validateIdToken() {
233243
Jwt idToken = new Jwt("token123", this.issuedAt, this.expiresAt, this.headers, this.claims);
234244
OidcIdTokenValidator validator = new OidcIdTokenValidator(this.registration.build());

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepositoryTests.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,6 @@ public void constructorListClientRegistrationWhenEmptyThenIllegalArgumentExcepti
5050
new InMemoryClientRegistrationRepository(registrations);
5151
}
5252

53-
@Test(expected = IllegalStateException.class)
54-
public void constructorListClientRegistrationWhenDuplicateIdThenIllegalArgumentException() {
55-
List<ClientRegistration> registrations = Arrays.asList(this.registration, this.registration);
56-
new InMemoryClientRegistrationRepository(registrations);
57-
}
58-
5953
@Test(expected = IllegalArgumentException.class)
6054
public void constructorMapClientRegistrationWhenNullThenIllegalArgumentException() {
6155
new InMemoryClientRegistrationRepository((Map<String, ClientRegistration>) null);
@@ -67,6 +61,12 @@ public void constructorMapClientRegistrationWhenEmptyMapThenRepositoryIsEmpty()
6761
assertThat(clients).isEmpty();
6862
}
6963

64+
@Test(expected = IllegalStateException.class)
65+
public void constructorListClientRegistrationWhenDuplicateIdThenIllegalArgumentException() {
66+
List<ClientRegistration> registrations = Arrays.asList(this.registration, this.registration);
67+
new InMemoryClientRegistrationRepository(registrations);
68+
}
69+
7070
@Test
7171
public void findByRegistrationIdWhenFoundThenFound() {
7272
String id = this.registration.getRegistrationId();

‎oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java

+8-6
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@
2323
import java.util.Collections;
2424
import java.util.LinkedHashSet;
2525
import java.util.List;
26-
import java.util.Objects;
2726
import java.util.Set;
28-
import java.util.stream.Collectors;
27+
import java.util.ArrayList;
2928

3029
/**
3130
* @author Joe Grandja
@@ -64,10 +63,13 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
6463
}
6564
}
6665
if (source instanceof Collection) {
67-
return ((Collection<?>) source).stream()
68-
.filter(Objects::nonNull)
69-
.map(Objects::toString)
70-
.collect(Collectors.toList());
66+
Collection<String> results = new ArrayList<>();
67+
for (Object object : ((Collection<?>) source)) {
68+
if (object != null) {
69+
results.add(object.toString());
70+
}
71+
}
72+
return results;
7173
}
7274
return Collections.singletonList(source.toString());
7375
}

0 commit comments

Comments
 (0)