From 7fb386669f30b33580039120942434e9eb4c3431 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Thu, 21 Sep 2017 12:56:39 -0400 Subject: [PATCH] InMemoryClientRegistrationRepository -> enforce unique ClientRegistration's Fixes gh-4562 --- .../ClientAliasIdentifierStrategy.java | 35 ++++++++++++++ .../registration/ClientRegistration.java | 2 +- .../ClientRegistrationRepository.java | 14 +++--- .../InMemoryClientRegistrationRepository.java | 48 +++++++++++-------- ...ionCodeAuthenticationProcessingFilter.java | 13 ++++- 5 files changed, 83 insertions(+), 29 deletions(-) create mode 100644 oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientAliasIdentifierStrategy.java diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientAliasIdentifierStrategy.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientAliasIdentifierStrategy.java new file mode 100644 index 0000000000..6894e9a52d --- /dev/null +++ b/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 { + + @Override + public String getIdentifier(ClientRegistration clientRegistration) { + Assert.notNull(clientRegistration, "clientRegistration cannot be null"); + return clientRegistration.getClientAlias(); + } +} diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index c7c17c55a8..77488d4c50 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/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 OAuth 2.0 Authorization Server. + * A representation of a client registration with an OAuth 2.0 / OpenID Connect 1.0 Authorization Server. * * @author Joe Grandja * @since 5.0 diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistrationRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistrationRepository.java index f924719857..ea4b406479 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistrationRepository.java +++ b/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. * *

- * The primary client registration information is stored with the associated Authorization Server. - * However, there may be uses cases where secondary information may need to be managed - * that is not supported (or provided) by the Authorization Server. - * This interface provides this capability for managing the primary and secondary - * information of a client registration. + * NOTE: The client registration information is ultimately stored and owned + * by the associated Authorization Server. + * Therefore, this repository provides the capability to store a sub-set copy + * of the primary client registration information + * externally from the Authorization Server. * * @author Joe Grandja * @since 5.0 @@ -33,7 +33,7 @@ import java.util.List; */ public interface ClientRegistrationRepository { - ClientRegistration getRegistrationByClientId(String clientId); + List getRegistrationsByClientId(String clientId); ClientRegistration getRegistrationByClientAlias(String clientAlias); diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java index afabd923b3..a4dfa74ee2 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/InMemoryClientRegistrationRepository.java +++ b/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 List of {@link ClientRegistration}(s) via it's constructor and stores it in-memory. + * A {@link ClientRegistrationRepository} that stores {@link ClientRegistration}(s) in-memory. * * @author Joe Grandja * @since 5.0 * @see ClientRegistration */ public final class InMemoryClientRegistrationRepository implements ClientRegistrationRepository { - private final List clientRegistrations; + private final ClientRegistrationIdentifierStrategy identifierStrategy = new ClientAliasIdentifierStrategy(); + private final Map registrations; - public InMemoryClientRegistrationRepository(List clientRegistrations) { - Assert.notEmpty(clientRegistrations, "clientRegistrations cannot be empty"); - this.clientRegistrations = Collections.unmodifiableList(clientRegistrations); + public InMemoryClientRegistrationRepository(List registrations) { + Assert.notEmpty(registrations, "registrations cannot be empty"); + Map 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 = - this.clientRegistrations.stream() - .filter(c -> c.getClientId().equals(clientId)) - .findFirst(); - return clientRegistration.isPresent() ? clientRegistration.get() : null; + public List 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 = - 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 getRegistrations() { - return this.clientRegistrations; + return new ArrayList<>(this.registrations.values()); } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationProcessingFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationProcessingFilter.java index 3717309020..1864ed73ce 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationProcessingFilter.java +++ b/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