From 9735a718cc9750fa8c617c0104d8c38b74d6d07c Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 14 Aug 2019 11:14:47 -0600 Subject: [PATCH] Remove MultiTenantAuthenticationManagerResolver Fixes gh-7259 --- ...h2ResourceServerSecurityConfiguration.java | 11 +- ...tiTenantAuthenticationManagerResolver.java | 174 ------------------ ...antAuthenticationManagerResolverTests.java | 159 ---------------- 3 files changed, 8 insertions(+), 336 deletions(-) delete mode 100644 web/src/main/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolver.java delete mode 100644 web/src/test/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolverTests.java diff --git a/samples/boot/oauth2resourceserver-multitenancy/src/main/java/sample/OAuth2ResourceServerSecurityConfiguration.java b/samples/boot/oauth2resourceserver-multitenancy/src/main/java/sample/OAuth2ResourceServerSecurityConfiguration.java index 9bfd6e639e..a43e217e98 100644 --- a/samples/boot/oauth2resourceserver-multitenancy/src/main/java/sample/OAuth2ResourceServerSecurityConfiguration.java +++ b/samples/boot/oauth2resourceserver-multitenancy/src/main/java/sample/OAuth2ResourceServerSecurityConfiguration.java @@ -17,6 +17,7 @@ package sample; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; import org.springframework.beans.factory.annotation.Value; @@ -33,8 +34,6 @@ import org.springframework.security.oauth2.server.resource.authentication.OAuth2 import org.springframework.security.oauth2.server.resource.introspection.NimbusOpaqueTokenIntrospector; import org.springframework.security.oauth2.server.resource.introspection.OpaqueTokenIntrospector; -import static org.springframework.security.web.authentication.MultiTenantAuthenticationManagerResolver.resolveFromPath; - /** * @author Josh Cummings */ @@ -68,7 +67,13 @@ public class OAuth2ResourceServerSecurityConfiguration extends WebSecurityConfig Map authenticationManagers = new HashMap<>(); authenticationManagers.put("tenantOne", jwt()); authenticationManagers.put("tenantTwo", opaque()); - return resolveFromPath(authenticationManagers::get); + return request -> { + String[] pathParts = request.getRequestURI().split("/"); + String tenantId = pathParts.length > 0 ? pathParts[1] : null; + return Optional.ofNullable(tenantId) + .map(authenticationManagers::get) + .orElseThrow(() -> new IllegalArgumentException("unknown tenant")); + }; } AuthenticationManager jwt() { diff --git a/web/src/main/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolver.java b/web/src/main/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolver.java deleted file mode 100644 index 4056d35b9f..0000000000 --- a/web/src/main/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolver.java +++ /dev/null @@ -1,174 +0,0 @@ -/* - * Copyright 2002-2019 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 - * - * https://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.web.authentication; - -import java.util.Optional; -import javax.servlet.http.HttpServletRequest; - -import org.springframework.core.convert.converter.Converter; -import org.springframework.security.authentication.AuthenticationManager; -import org.springframework.security.authentication.AuthenticationManagerResolver; -import org.springframework.util.Assert; -import org.springframework.web.util.UriComponents; -import org.springframework.web.util.UriComponentsBuilder; - -/** - * An implementation of {@link AuthenticationManagerResolver} that separates the tasks of - * extracting the request's tenant identifier and looking up an {@link AuthenticationManager} - * by that tenant identifier. - * - * @author Josh Cummings - * @since 5.2 - * @see AuthenticationManagerResolver - */ -public final class MultiTenantAuthenticationManagerResolver implements AuthenticationManagerResolver { - - private final Converter authenticationManagerResolver; - - /** - * Constructs a {@link MultiTenantAuthenticationManagerResolver} with the provided parameters - * - * @param tenantResolver - * @param authenticationManagerResolver - */ - public MultiTenantAuthenticationManagerResolver - (Converter tenantResolver, - Converter authenticationManagerResolver) { - - Assert.notNull(tenantResolver, "tenantResolver cannot be null"); - Assert.notNull(authenticationManagerResolver, "authenticationManagerResolver cannot be null"); - - this.authenticationManagerResolver = request -> { - Optional context = Optional.ofNullable(tenantResolver.convert(request)); - return context.map(authenticationManagerResolver::convert) - .orElseThrow(() -> new IllegalArgumentException - ("Could not resolve AuthenticationManager by reference " + context.orElse(null))); - }; - } - - @Override - public AuthenticationManager resolve(HttpServletRequest context) { - return this.authenticationManagerResolver.convert(context); - } - - /** - * Creates an {@link AuthenticationManagerResolver} that will use a hostname's first label as - * the resolution key for the underlying {@link AuthenticationManagerResolver}. - * - * For example, you might have a set of {@link AuthenticationManager}s defined like so: - * - * 
-	 * 	Map authenticationManagers = new HashMap<>();
-	 *  authenticationManagers.put("tenantOne", managerOne());
-	 *  authenticationManagers.put("tenantTwo", managerTwo());
-	 *
- * - * And that your system serves hostnames like 
https://tenantOne.example.org
. - * - * Then, you could create an {@link AuthenticationManagerResolver} that uses the "tenantOne" value from - * the hostname to resolve Tenant One's {@link AuthenticationManager} like so: - * - * 
-	 *	AuthenticationManagerResolver resolver =
-	 *			resolveFromSubdomain(authenticationManagers::get);
-	 *
- * - * {@link HttpServletRequest} - * @param resolver A {@link String}-resolving {@link AuthenticationManagerResolver} - * @return A hostname-resolving {@link AuthenticationManagerResolver} - */ - public static AuthenticationManagerResolver - resolveFromSubdomain(Converter resolver) { - - return new MultiTenantAuthenticationManagerResolver<>(request -> - Optional.ofNullable(request.getServerName()) - .map(host -> host.split("\\.")) - .filter(segments -> segments.length > 0) - .map(segments -> segments[0]).orElse(null), resolver); - } - - /** - * Creates an {@link AuthenticationManagerResolver} that will use a request path's first segment as - * the resolution key for the underlying {@link AuthenticationManagerResolver}. - * - * For example, you might have a set of {@link AuthenticationManager}s defined like so: - * - * 
-	 * 	Map authenticationManagers = new HashMap<>();
-	 *  authenticationManagers.put("tenantOne", managerOne());
-	 *  authenticationManagers.put("tenantTwo", managerTwo());
-	 *
- * - * And that your system serves requests like 
https://example.org/tenantOne
. - * - * Then, you could create an {@link AuthenticationManagerResolver} that uses the "tenantOne" value from - * the request to resolve Tenant One's {@link AuthenticationManager} like so: - * - * 
-	 *	AuthenticationManagerResolver resolver =
-	 *			resolveFromPath(authenticationManagers::get);
-	 *
- * - * {@link HttpServletRequest} - * @param resolver A {@link String}-resolving {@link AuthenticationManagerResolver} - * @return A path-resolving {@link AuthenticationManagerResolver} - */ - public static AuthenticationManagerResolver - resolveFromPath(Converter resolver) { - - return new MultiTenantAuthenticationManagerResolver<>(request -> - Optional.ofNullable(request.getRequestURI()) - .map(UriComponentsBuilder::fromUriString) - .map(UriComponentsBuilder::build) - .map(UriComponents::getPathSegments) - .filter(segments -> !segments.isEmpty()) - .map(segments -> segments.get(0)).orElse(null), resolver); - } - - /** - * Creates an {@link AuthenticationManagerResolver} that will use a request headers's value as - * the resolution key for the underlying {@link AuthenticationManagerResolver}. - * - * For example, you might have a set of {@link AuthenticationManager}s defined like so: - * - * 
-	 * 	Map authenticationManagers = new HashMap<>();
-	 *  authenticationManagers.put("tenantOne", managerOne());
-	 *  authenticationManagers.put("tenantTwo", managerTwo());
-	 *
- * - * And that your system serves requests with a header like 
X-Tenant-Id: tenantOne
. - * - * Then, you could create an {@link AuthenticationManagerResolver} that uses the "tenantOne" value from - * the request to resolve Tenant One's {@link AuthenticationManager} like so: - * - * 
-	 *	AuthenticationManagerResolver resolver =
-	 *			resolveFromHeader("X-Tenant-Id", authenticationManagers::get);
-	 *
- * - * {@link HttpServletRequest} - * @param resolver A {@link String}-resolving {@link AuthenticationManagerResolver} - * @return A header-resolving {@link AuthenticationManagerResolver} - */ - public static AuthenticationManagerResolver - resolveFromHeader(String headerName, Converter resolver) { - - return new MultiTenantAuthenticationManagerResolver<> - (request -> request.getHeader(headerName), resolver); - } -} diff --git a/web/src/test/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolverTests.java b/web/src/test/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolverTests.java deleted file mode 100644 index 902e6b72be..0000000000 --- a/web/src/test/java/org/springframework/security/web/authentication/MultiTenantAuthenticationManagerResolverTests.java +++ /dev/null @@ -1,159 +0,0 @@ -/* - * Copyright 2002-2019 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 - * - * https://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.web.authentication; - -import java.util.Collections; -import java.util.Map; -import javax.servlet.http.HttpServletRequest; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - -import org.springframework.core.convert.converter.Converter; -import org.springframework.security.authentication.AuthenticationManager; -import org.springframework.security.authentication.AuthenticationManagerResolver; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.springframework.security.web.authentication.MultiTenantAuthenticationManagerResolver.resolveFromSubdomain; -import static org.springframework.security.web.authentication.MultiTenantAuthenticationManagerResolver.resolveFromPath; -import static org.springframework.security.web.authentication.MultiTenantAuthenticationManagerResolver.resolveFromHeader; - -/** - * Tests for {@link MultiTenantAuthenticationManagerResolver} - */ -@RunWith(MockitoJUnitRunner.class) -public class MultiTenantAuthenticationManagerResolverTests { - private static final String TENANT = "tenant"; - - @Mock - AuthenticationManager authenticationManager; - - @Mock - HttpServletRequest request; - - Map authenticationManagers; - - @Before - public void setup() { - this.authenticationManagers = Collections.singletonMap(TENANT, this.authenticationManager); - } - - @Test - public void resolveFromSubdomainWhenGivenResolverThenReturnsSubdomainParsingResolver() { - AuthenticationManagerResolver fromSubdomain = - resolveFromSubdomain(this.authenticationManagers::get); - - when(this.request.getServerName()).thenReturn(TENANT + ".example.org"); - - AuthenticationManager authenticationManager = fromSubdomain.resolve(this.request); - assertThat(authenticationManager).isEqualTo(this.authenticationManager); - - when(this.request.getServerName()).thenReturn("wrong.example.org"); - - assertThatCode(() -> fromSubdomain.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - - when(this.request.getServerName()).thenReturn("example"); - - assertThatCode(() -> fromSubdomain.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void resolveFromPathWhenGivenResolverThenReturnsPathParsingResolver() { - AuthenticationManagerResolver fromPath = - resolveFromPath(this.authenticationManagers::get); - - when(this.request.getRequestURI()).thenReturn("/" + TENANT + "/otherthings"); - - AuthenticationManager authenticationManager = fromPath.resolve(this.request); - assertThat(authenticationManager).isEqualTo(this.authenticationManager); - - when(this.request.getRequestURI()).thenReturn("/otherthings"); - - assertThatCode(() -> fromPath.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - - when(this.request.getRequestURI()).thenReturn("/"); - - assertThatCode(() -> fromPath.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void resolveFromHeaderWhenGivenResolverTheReturnsHeaderParsingResolver() { - AuthenticationManagerResolver fromHeader = - resolveFromHeader("X-Tenant-Id", this.authenticationManagers::get); - - when(this.request.getHeader("X-Tenant-Id")).thenReturn(TENANT); - - AuthenticationManager authenticationManager = fromHeader.resolve(this.request); - assertThat(authenticationManager).isEqualTo(this.authenticationManager); - - when(this.request.getHeader("X-Tenant-Id")).thenReturn("wrong"); - - assertThatCode(() -> fromHeader.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - - when(this.request.getHeader("X-Tenant-Id")).thenReturn(null); - - assertThatCode(() -> fromHeader.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void resolveWhenGivenTenantResolverThenResolves() { - AuthenticationManagerResolver byRequestConverter = - new MultiTenantAuthenticationManagerResolver<>(HttpServletRequest::getQueryString, - this.authenticationManagers::get); - - when(this.request.getQueryString()).thenReturn(TENANT); - - AuthenticationManager authenticationManager = byRequestConverter.resolve(this.request); - assertThat(authenticationManager).isEqualTo(this.authenticationManager); - - when(this.request.getQueryString()).thenReturn("wrong"); - - assertThatCode(() -> byRequestConverter.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - - when(this.request.getQueryString()).thenReturn(null); - - assertThatCode(() -> byRequestConverter.resolve(this.request)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void constructorWhenUsingNullTenantResolverThenException() { - assertThatCode(() -> new MultiTenantAuthenticationManagerResolver - (null, mock(Converter.class))) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - public void constructorWhenUsingNullAuthenticationManagerResolverThenException() { - assertThatCode(() -> new MultiTenantAuthenticationManagerResolver - (mock(Converter.class), null)) - .isInstanceOf(IllegalArgumentException.class); - } -}