InMemoryClientRegistrationRepository -> enforce unique ClientRegistration's

Fixes gh-4562

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

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2012-2017 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.security.oauth2.client.registration;
+
+import org.springframework.util.Assert;
+
+/**
+ * A {@link ClientRegistrationIdentifierStrategy} that identifies a {@link ClientRegistration}
+ * using the {@link ClientRegistration#getClientAlias()}.
+ *
+ * @author Joe Grandja
+ * @since 5.0
+ * @see ClientRegistration
+ */
+public class ClientAliasIdentifierStrategy implements ClientRegistrationIdentifierStrategy<String> {
+
+	@Override
+	public String getIdentifier(ClientRegistration clientRegistration) {
+		Assert.notNull(clientRegistration, "clientRegistration cannot be null");
+		return clientRegistration.getClientAlias();
+	}
+}

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

@@ -26,7 +26,7 @@ import java.util.LinkedHashSet;
 import java.util.Set;
 
 /**
- * A representation of a client registration with an <i>OAuth 2.0 Authorization Server</i>.
+ * A representation of a client registration with an OAuth 2.0 / OpenID Connect 1.0 <i>Authorization Server</i>.
  *
  * @author Joe Grandja
  * @since 5.0

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

@@ -18,14 +18,14 @@ package org.springframework.security.oauth2.client.registration;
 import java.util.List;
 
 /**
- * Implementations of this interface are responsible for the management of {@link ClientRegistration}'s.
+ * A repository for OAuth 2.0 / OpenID Connect 1.0 {@link ClientRegistration}'s.
  *
  * <p>
- * The <i>primary</i> client registration information is stored with the associated <i>Authorization Server</i>.
- * However, there may be uses cases where <i>secondary</i> information may need to be managed
- * that is not supported (or provided) by the <i>Authorization Server</i>.
- * This interface provides this capability for managing the <i>primary</i> and <i>secondary</i>
- * information of a client registration.
+ * <b>NOTE:</b> The client registration information is ultimately stored and owned
+ * by the associated <i>Authorization Server</i>.
+ * Therefore, this repository provides the capability to store a sub-set copy
+ * of the <i>primary</i> client registration information
+ * externally from the <i>Authorization Server</i>.
  *
  * @author Joe Grandja
  * @since 5.0
@@ -33,7 +33,7 @@ import java.util.List;
  */
 public interface ClientRegistrationRepository {
 
-	ClientRegistration getRegistrationByClientId(String clientId);
+	List<ClientRegistration> getRegistrationsByClientId(String clientId);
 
 	ClientRegistration getRegistrationByClientAlias(String clientAlias);
 

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

@@ -17,46 +17,56 @@ package org.springframework.security.oauth2.client.registration;
 
 import org.springframework.util.Assert;
 
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
-import java.util.Optional;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 /**
- * A basic implementation of a {@link ClientRegistrationRepository} that accepts
- * a <code>List</code> of {@link ClientRegistration}(s) via it's constructor and stores it <i>in-memory</i>.
+ * A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) <i>in-memory</i>.
  *
  * @author Joe Grandja
  * @since 5.0
  * @see ClientRegistration
  */
 public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository {
-	private final List<ClientRegistration> clientRegistrations;
+	private final ClientRegistrationIdentifierStrategy<String> identifierStrategy = new ClientAliasIdentifierStrategy();
+	private final Map<String, ClientRegistration> registrations;
 
-	public InMemoryClientRegistrationRepository(List<ClientRegistration> clientRegistrations) {
-		Assert.notEmpty(clientRegistrations, "clientRegistrations cannot be empty");
-		this.clientRegistrations = Collections.unmodifiableList(clientRegistrations);
+	public InMemoryClientRegistrationRepository(List<ClientRegistration> registrations) {
+		Assert.notEmpty(registrations, "registrations cannot be empty");
+		Map<String, ClientRegistration> registrationsMap = new HashMap<>();
+		registrations.forEach(registration -> {
+			String identifier = this.identifierStrategy.getIdentifier(registration);
+			if (registrationsMap.containsKey(identifier)) {
+				throw new IllegalArgumentException("ClientRegistration must be unique. Found duplicate identifier: " + identifier);
+			}
+			registrationsMap.put(identifier, registration);
+		});
+		this.registrations = Collections.unmodifiableMap(registrationsMap);
 	}
 
 	@Override
-	public ClientRegistration getRegistrationByClientId(String clientId) {
-		Optional<ClientRegistration> clientRegistration =
-				this.clientRegistrations.stream()
-				.filter(c -> c.getClientId().equals(clientId))
-				.findFirst();
-		return clientRegistration.isPresent() ? clientRegistration.get() : null;
+	public List<ClientRegistration> getRegistrationsByClientId(String clientId) {
+		Assert.hasText(clientId, "clientId cannot be empty");
+		return this.registrations.values().stream()
+			.filter(registration -> registration.getClientId().equals(clientId))
+			.collect(Collectors.toList());
 	}
 
 	@Override
 	public ClientRegistration getRegistrationByClientAlias(String clientAlias) {
-		Optional<ClientRegistration> clientRegistration =
-				this.clientRegistrations.stream()
-						.filter(c -> c.getClientAlias().equals(clientAlias))
-						.findFirst();
-		return clientRegistration.isPresent() ? clientRegistration.get() : null;
+		Assert.hasText(clientAlias, "clientAlias cannot be empty");
+		return this.registrations.values().stream()
+			.filter(registration -> registration.getClientAlias().equals(clientAlias))
+			.findFirst()
+			.orElseGet(null);
 	}
 
 	@Override
 	public List<ClientRegistration> getRegistrations() {
-		return this.clientRegistrations;
+		return new ArrayList<>(this.registrations.values());
 	}
 }

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationProcessingFilter.java

@@ -41,6 +41,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.security.web.util.matcher.RequestVariablesExtractor;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -140,8 +141,16 @@ public class AuthorizationCodeAuthenticationProcessingFilter extends AbstractAut
 
 		AuthorizationRequestAttributes matchingAuthorizationRequest = this.resolveAuthorizationRequest(request);
 
-		ClientRegistration clientRegistration = this.getClientRegistrationRepository().getRegistrationByClientId(
-				matchingAuthorizationRequest.getClientId());
+		String clientAlias = ((RequestVariablesExtractor)this.getAuthorizationResponseMatcher())
+			.extractUriTemplateVariables(request).get(CLIENT_ALIAS_URI_VARIABLE_NAME);
+		ClientRegistration clientRegistration = null;
+		if (!StringUtils.isEmpty(clientAlias)) {
+			clientRegistration = this.getClientRegistrationRepository().getRegistrationByClientAlias(clientAlias);
+		}
+		if (clientRegistration == null || !matchingAuthorizationRequest.getClientId().equals(clientRegistration.getClientId())) {
+			OAuth2Error oauth2Error = new OAuth2Error(OAuth2Error.INVALID_REQUEST_ERROR_CODE);
+			throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
+		}
 
 		// The clientRegistration.redirectUri may contain Uri template variables, whether it's configured by
 		// the user or configured by default. In these cases, the redirectUri will be expanded and ultimately changed