Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert Map constructor for InMemoryReactiveClientRegistrationRepository 

This commit reverts f6414e9a52 and
partial revert of e1b095df32.
NOTE: InMemoryReactiveClientRegistrationRepository should not expose a
Map constructor as it would allow the caller to pass in a 'distributed' (remote) Map,
which would result in a blocking I/O operation.
This commit is contained in:
Joe Grandja 2019-07-08 15:18:37 -04:00
parent 23d61d43e5
commit e554547593
4 changed files with 48 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java
View File

@ -18,13 +18,16 @@ package org.springframework.security.oauth2.client.registration;
import org.springframework.util.Assert;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Collector;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toConcurrentMap;
/**
* A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory.
@ -37,7 +40,6 @@ import java.util.stream.Collectors;
* @see ClientRegistration
*/
public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository, Iterable<ClientRegistration> {
private final Map<String, ClientRegistration> registrations;
/**
@ -46,8 +48,7 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr
* @param registrations the client registration(s)
*/
public InMemoryClientRegistrationRepository(ClientRegistration... registrations) {
Assert.notEmpty(registrations, "registrations cannot be empty");
this.registrations = createClientRegistrationIdToClientRegistration(Arrays.asList(registrations));
this(Arrays.asList(registrations));
}
/**
@ -56,7 +57,14 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr
* @param registrations the client registration(s)
*/
public InMemoryClientRegistrationRepository(List<ClientRegistration> registrations) {
this(createClientRegistrationIdToClientRegistration(registrations));
this(createRegistrationsMap(registrations));
}
private static Map<String, ClientRegistration> createRegistrationsMap(List<ClientRegistration> registrations) {
Assert.notEmpty(registrations, "registrations cannot be empty");
Collector<ClientRegistration, ?, ConcurrentMap<String, ClientRegistration>> collector =
toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity());
return registrations.stream().collect(collectingAndThen(collector, Collections::unmodifiableMap));
}
/**
@ -71,13 +79,6 @@ public final class InMemoryClientRegistrationRepository implements ClientRegistr
this.registrations = registrations;
}
private static Map<String, ClientRegistration> createClientRegistrationIdToClientRegistration(Collection<ClientRegistration> registrations) {
Assert.notNull(registrations, "registrations cannot be null");
return Collections.unmodifiableMap(registrations.stream()
.peek(registration -> Assert.notNull(registration, "registrations cannot contain null values"))
.collect(Collectors.toMap(ClientRegistration::getRegistrationId, Function.identity())));
}
@Override
public ClientRegistration findByRegistrationId(String registrationId) {
Assert.hasText(registrationId, "registrationId cannot be empty");

36
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepository.java
View File

@ -18,6 +18,11 @@ package org.springframework.security.oauth2.client.registration;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.springframework.util.Assert;
import org.springframework.util.ConcurrentReferenceHashMap;
import reactor.core.publisher.Mono;
@ -25,7 +30,6 @@ import reactor.core.publisher.Mono;
* A Reactive {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory.
*
* @author Rob Winch
* @author Vedran Pavic
* @since 5.1
* @see ClientRegistrationRepository
* @see ClientRegistration
@ -33,7 +37,7 @@ import reactor.core.publisher.Mono;
public final class InMemoryReactiveClientRegistrationRepository
implements ReactiveClientRegistrationRepository, Iterable<ClientRegistration> {
private final InMemoryClientRegistrationRepository delegate;
private final Map<String, ClientRegistration> clientIdToClientRegistration;
/**
* Constructs an {@code InMemoryReactiveClientRegistrationRepository} using the provided parameters.
@ -41,7 +45,12 @@ public final class InMemoryReactiveClientRegistrationRepository
* @param registrations the client registration(s)
*/
public InMemoryReactiveClientRegistrationRepository(ClientRegistration... registrations) {
this.delegate = new InMemoryClientRegistrationRepository(registrations);
Assert.notEmpty(registrations, "registrations cannot be empty");
this.clientIdToClientRegistration = new ConcurrentReferenceHashMap<>();
for (ClientRegistration registration : registrations) {
Assert.notNull(registration, "registrations cannot contain null values");
this.clientIdToClientRegistration.put(registration.getRegistrationId(), registration);
}
}
/**
@ -50,24 +59,15 @@ public final class InMemoryReactiveClientRegistrationRepository
* @param registrations the client registration(s)
*/
public InMemoryReactiveClientRegistrationRepository(List<ClientRegistration> registrations) {
this.delegate = new InMemoryClientRegistrationRepository(registrations);
Assert.notEmpty(registrations, "registrations cannot be null or empty");
this.clientIdToClientRegistration = registrations.stream()
.collect(Collectors.toConcurrentMap(ClientRegistration::getRegistrationId, Function.identity()));
}
/**
* Constructs an {@code InMemoryReactiveClientRegistrationRepository} using the provided {@code Map}
* of {@link ClientRegistration#getRegistrationId() registration id} to {@link ClientRegistration}.
* <b>NOTE:</b> The supplied {@code Map} must be a non-blocking {@code Map}.
*
* @since 5.2
* @param registrations the {@code Map} of client registration(s)
*/
public InMemoryReactiveClientRegistrationRepository(
Map<String, ClientRegistration> registrations) {
this.delegate = new InMemoryClientRegistrationRepository(registrations);
}
@Override
public Mono<ClientRegistration> findByRegistrationId(String registrationId) {
return Mono.justOrEmpty(this.delegate.findByRegistrationId(registrationId));
return Mono.justOrEmpty(this.clientIdToClientRegistration.get(registrationId));
}
/**
@ -77,6 +77,6 @@ public final class InMemoryReactiveClientRegistrationRepository
*/
@Override
public Iterator<ClientRegistration> iterator() {
return delegate.iterator();
return this.clientIdToClientRegistration.values().iterator();
}
}

11
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepositoryTests.java
View File

@ -19,6 +19,7 @@ package org.springframework.security.oauth2.client.registration;
import org.junit.Test;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -38,14 +39,14 @@ public class InMemoryClientRegistrationRepositoryTests {
private InMemoryClientRegistrationRepository clients = new InMemoryClientRegistrationRepository(this.registration);
@Test(expected = IllegalArgumentException.class)
public void constructorVarArgsListClientRegistrationWhenNullThenIllegalArgumentException() {
ClientRegistration nullRegistration = null;
new InMemoryClientRegistrationRepository(nullRegistration);
public void constructorListClientRegistrationWhenNullThenIllegalArgumentException() {
List<ClientRegistration> registrations = null;
new InMemoryClientRegistrationRepository(registrations);
}
@Test(expected = IllegalArgumentException.class)
public void constructorListClientRegistrationWhenNullThenIllegalArgumentException() {
List<ClientRegistration> registrations = null;
public void constructorListClientRegistrationWhenEmptyThenIllegalArgumentException() {
List<ClientRegistration> registrations = Collections.emptyList();
new InMemoryClientRegistrationRepository(registrations);
}

43
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/InMemoryReactiveClientRegistrationRepositoryTests.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -16,21 +16,18 @@
package org.springframework.security.oauth2.client.registration;
import org.junit.Before;
import org.junit.Test;
import reactor.test.StepVerifier;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
import reactor.test.StepVerifier;
/**
* @author Rob Winch
* @author Vedran Pavic
* @since 5.1
*/
public class InMemoryReactiveClientRegistrationRepositoryTests {
@ -64,33 +61,13 @@ public class InMemoryReactiveClientRegistrationRepositoryTests {
.isInstanceOf(IllegalArgumentException.class);
}
@Test
public void constructorWhenClientRegistrationListHasNullThenIllegalArgumentException() {
List<ClientRegistration> registrations = Arrays.asList(null, registration);
assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registrations))
.isInstanceOf(IllegalArgumentException.class);
}
@Test
public void constructorWhenClientRegistrationIsNullThenIllegalArgumentException() {
assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registration, null))
ClientRegistration registration = null;
assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registration))
.isInstanceOf(IllegalArgumentException.class);
}
@Test
public void constructorWhenClientRegistrationMapIsNullThenIllegalArgumentException() {
Map<String, ClientRegistration> registrations = null;
assertThatThrownBy(() -> new InMemoryReactiveClientRegistrationRepository(registrations))
.isInstanceOf(IllegalArgumentException.class);
}
@Test
public void constructorWhenClientRegistrationMapIsEmptyThenRepositoryIsEmpty() {
InMemoryReactiveClientRegistrationRepository repository = new InMemoryReactiveClientRegistrationRepository(
new HashMap<>());
assertThat(repository).isEmpty();
}
@Test
public void findByRegistrationIdWhenValidIdThenFound() {
StepVerifier.create(this.repository.findByRegistrationId(this.registration.getRegistrationId()))