diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java index 36a15a925a7..f549db11bdd 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java @@ -37,6 +37,7 @@ import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.web.PathMappedEndpoints; import org.springframework.boot.autoconfigure.security.servlet.RequestMatcherProvider; import org.springframework.boot.security.servlet.ApplicationContextRequestMatcher; +import org.springframework.boot.web.context.WebServerApplicationContext; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; @@ -44,7 +45,6 @@ import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.context.WebApplicationContext; -import org.springframework.web.context.support.WebApplicationContextUtils; /** * Factory that can be used to create a {@link RequestMatcher} for actuator endpoint @@ -127,6 +127,13 @@ public final class EndpointRequest { super(WebApplicationContext.class); } + @Override + protected boolean ignoreApplicationContext(WebApplicationContext applicationContext) { + ManagementPortType type = ManagementPortType.get(applicationContext.getEnvironment()); + return type == ManagementPortType.DIFFERENT + && WebServerApplicationContext.hasServerNamespace(applicationContext, "management"); + } + @Override protected final void initialized(Supplier context) { this.delegate = createDelegate(context.get()); @@ -134,17 +141,6 @@ public final class EndpointRequest { @Override protected final boolean matches(HttpServletRequest request, Supplier context) { - WebApplicationContext applicationContext = WebApplicationContextUtils - .getRequiredWebApplicationContext(request.getServletContext()); - if (ManagementPortType.get(applicationContext.getEnvironment()) == ManagementPortType.DIFFERENT) { - if (applicationContext.getParent() == null) { - return false; - } - String managementContextId = applicationContext.getParent().getId() + ":management"; - if (!managementContextId.equals(applicationContext.getId())) { - return false; - } - } return this.delegate.matches(request); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/PathRequest.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/PathRequest.java index 1dee5f9ee57..6817bbef8b4 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/PathRequest.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/PathRequest.java @@ -23,8 +23,10 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.boot.autoconfigure.h2.H2ConsoleProperties; import org.springframework.boot.autoconfigure.security.StaticResourceLocation; import org.springframework.boot.security.servlet.ApplicationContextRequestMatcher; +import org.springframework.boot.web.context.WebServerApplicationContext; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.web.context.WebApplicationContext; /** * Factory that can be used to create a {@link RequestMatcher} for commonly used paths. @@ -69,6 +71,11 @@ public final class PathRequest { super(H2ConsoleProperties.class); } + @Override + protected boolean ignoreApplicationContext(WebApplicationContext applicationContext) { + return WebServerApplicationContext.hasServerNamespace(applicationContext, "management"); + } + @Override protected void initialized(Supplier h2ConsoleProperties) { this.delegate = new AntPathRequestMatcher(h2ConsoleProperties.get().getPath() + "/**"); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequest.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequest.java index b37deb1dd0f..c6f38ed14c5 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequest.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequest.java @@ -29,10 +29,12 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.boot.autoconfigure.security.StaticResourceLocation; import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletPath; import org.springframework.boot.security.servlet.ApplicationContextRequestMatcher; +import org.springframework.boot.web.context.WebServerApplicationContext; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; +import org.springframework.web.context.WebApplicationContext; /** * Used to create a {@link RequestMatcher} for static resources in commonly used @@ -144,6 +146,11 @@ public final class StaticResourceRequest { .map(dispatcherServletPath::getRelativePath); } + @Override + protected boolean ignoreApplicationContext(WebApplicationContext applicationContext) { + return WebServerApplicationContext.hasServerNamespace(applicationContext, "management"); + } + @Override protected boolean matches(HttpServletRequest request, Supplier context) { return this.delegate.matches(request); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/PathRequestTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/PathRequestTests.java index 9a421af6084..88522396859 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/PathRequestTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/PathRequestTests.java @@ -27,7 +27,6 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.context.WebApplicationContext; -import org.springframework.web.context.support.StaticWebApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -51,8 +50,20 @@ public class PathRequestTests { assertMatcher(matcher).doesNotMatch("/js/file.js"); } + @Test + public void toH2ConsoleWhenManagementContextShouldNeverMatch() { + RequestMatcher matcher = PathRequest.toH2Console(); + assertMatcher(matcher, "management").doesNotMatch("/h2-console"); + assertMatcher(matcher, "management").doesNotMatch("/h2-console/subpath"); + assertMatcher(matcher, "management").doesNotMatch("/js/file.js"); + } + private RequestMatcherAssert assertMatcher(RequestMatcher matcher) { - StaticWebApplicationContext context = new StaticWebApplicationContext(); + return assertMatcher(matcher, null); + } + + private RequestMatcherAssert assertMatcher(RequestMatcher matcher, String serverNamespace) { + TestWebApplicationContext context = new TestWebApplicationContext(serverNamespace); context.registerBean(ServerProperties.class); context.registerBean(H2ConsoleProperties.class); return assertThat(new RequestMatcherAssert(context, matcher)); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequestTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequestTests.java index 77840b4f278..0e8a3862091 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequestTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequestTests.java @@ -27,7 +27,6 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.context.WebApplicationContext; -import org.springframework.web.context.support.StaticWebApplicationContext; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -53,6 +52,16 @@ public class StaticResourceRequestTests { assertMatcher(matcher).doesNotMatch("/bar"); } + @Test + public void atCommonLocationsWhenManagementContextShouldNeverMatch() { + RequestMatcher matcher = this.resourceRequest.atCommonLocations(); + assertMatcher(matcher, "management").doesNotMatch("/css/file.css"); + assertMatcher(matcher, "management").doesNotMatch("/js/file.js"); + assertMatcher(matcher, "management").doesNotMatch("/images/file.css"); + assertMatcher(matcher, "management").doesNotMatch("/webjars/file.css"); + assertMatcher(matcher, "management").doesNotMatch("/foo/favicon.ico"); + } + @Test public void atCommonLocationsWithExcludeShouldNotMatchExcluded() { RequestMatcher matcher = this.resourceRequest.atCommonLocations().excluding(StaticResourceLocation.CSS); @@ -70,8 +79,8 @@ public class StaticResourceRequestTests { @Test public void atLocationWhenHasServletPathShouldMatchLocation() { RequestMatcher matcher = this.resourceRequest.at(StaticResourceLocation.CSS); - assertMatcher(matcher, "/foo").matches("/foo", "/css/file.css"); - assertMatcher(matcher, "/foo").doesNotMatch("/foo", "/js/file.js"); + assertMatcher(matcher, null, "/foo").matches("/foo", "/css/file.css"); + assertMatcher(matcher, null, "/foo").doesNotMatch("/foo", "/js/file.js"); } @Test @@ -87,15 +96,16 @@ public class StaticResourceRequestTests { } private RequestMatcherAssert assertMatcher(RequestMatcher matcher) { - DispatcherServletPath dispatcherServletPath = () -> ""; - StaticWebApplicationContext context = new StaticWebApplicationContext(); - context.registerBean(DispatcherServletPath.class, () -> dispatcherServletPath); - return assertThat(new RequestMatcherAssert(context, matcher)); + return assertMatcher(matcher, null, ""); } - private RequestMatcherAssert assertMatcher(RequestMatcher matcher, String path) { + private RequestMatcherAssert assertMatcher(RequestMatcher matcher, String serverNamespace) { + return assertMatcher(matcher, serverNamespace, ""); + } + + private RequestMatcherAssert assertMatcher(RequestMatcher matcher, String serverNamespace, String path) { DispatcherServletPath dispatcherServletPath = () -> path; - StaticWebApplicationContext context = new StaticWebApplicationContext(); + TestWebApplicationContext context = new TestWebApplicationContext(serverNamespace); context.registerBean(DispatcherServletPath.class, () -> dispatcherServletPath); return assertThat(new RequestMatcherAssert(context, matcher)); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/TestWebApplicationContext.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/TestWebApplicationContext.java new file mode 100644 index 00000000000..4e551f508c5 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/TestWebApplicationContext.java @@ -0,0 +1,47 @@ +/* + * Copyright 2012-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.boot.autoconfigure.security.servlet; + +import org.springframework.boot.web.context.WebServerApplicationContext; +import org.springframework.boot.web.server.WebServer; +import org.springframework.web.context.support.StaticWebApplicationContext; + +/** + * Test {@link StaticWebApplicationContext} that also implements + * {@link WebServerApplicationContext}. + * + * @author Phillip Webb + */ +class TestWebApplicationContext extends StaticWebApplicationContext implements WebServerApplicationContext { + + private final String serverNamespace; + + TestWebApplicationContext(String serverNamespace) { + this.serverNamespace = serverNamespace; + } + + @Override + public WebServer getWebServer() { + return null; + } + + @Override + public String getServerNamespace() { + return this.serverNamespace; + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java index 3de519891bf..1e2b661b968 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java @@ -16,6 +16,7 @@ package org.springframework.boot.security.servlet; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import javax.servlet.http.HttpServletRequest; @@ -43,9 +44,7 @@ public abstract class ApplicationContextRequestMatcher implements RequestMatc private final Class contextClass; - private volatile Supplier context; - - private final Object contextLock = new Object(); + private final AtomicBoolean initialized = new AtomicBoolean(false); public ApplicationContextRequestMatcher(Class contextClass) { Assert.notNull(contextClass, "Context class must not be null"); @@ -54,7 +53,48 @@ public abstract class ApplicationContextRequestMatcher implements RequestMatc @Override public final boolean matches(HttpServletRequest request) { - return matches(request, getContext(request)); + WebApplicationContext webApplicationContext = WebApplicationContextUtils + .getRequiredWebApplicationContext(request.getServletContext()); + if (ignoreApplicationContext(webApplicationContext)) { + return false; + } + Supplier context = () -> getContext(webApplicationContext); + if (this.initialized.compareAndSet(false, true)) { + initialized(context); + } + return matches(request, context); + } + + @SuppressWarnings("unchecked") + private C getContext(WebApplicationContext webApplicationContext) { + if (this.contextClass.isInstance(webApplicationContext)) { + return (C) webApplicationContext; + } + return webApplicationContext.getBean(this.contextClass); + } + + /** + * Returns if the {@link WebApplicationContext} should be ignored and not used for + * matching. If this method returns {@code true} then the context will not be used and + * the {@link #matches(HttpServletRequest) matches} method will return {@code false}. + * @param webApplicationContext the candidate web application context + * @return if the application context should be ignored + * @since 2.1.8 + */ + protected boolean ignoreApplicationContext(WebApplicationContext webApplicationContext) { + return false; + } + + /** + * Method that can be implemented by subclasses that wish to initialize items the + * first time that the matcher is called. This method will be called only once and + * only if {@link #ignoreApplicationContext(WebApplicationContext)} returns + * {@code true}. Note that the supplied context will be based on the + * first request sent to the matcher. + * @param context a supplier for the initialized context (may throw an exception) + * @see #ignoreApplicationContext(WebApplicationContext) + */ + protected void initialized(Supplier context) { } /** @@ -65,34 +105,4 @@ public abstract class ApplicationContextRequestMatcher implements RequestMatc */ protected abstract boolean matches(HttpServletRequest request, Supplier context); - private Supplier getContext(HttpServletRequest request) { - if (this.context == null) { - synchronized (this.contextLock) { - if (this.context == null) { - Supplier createdContext = createContext(request); - initialized(createdContext); - this.context = createdContext; - } - } - } - return this.context; - } - - /** - * Called once the context has been initialized. - * @param context a supplier for the initialized context (may throw an exception) - */ - protected void initialized(Supplier context) { - } - - @SuppressWarnings("unchecked") - private Supplier createContext(HttpServletRequest request) { - WebApplicationContext context = WebApplicationContextUtils - .getRequiredWebApplicationContext(request.getServletContext()); - if (this.contextClass.isInstance(context)) { - return () -> (C) context; - } - return () -> context.getBean(this.contextClass); - } - } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/context/WebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/context/WebServerApplicationContext.java index 6e91956dc6e..7d0e9902240 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/context/WebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/context/WebServerApplicationContext.java @@ -18,6 +18,7 @@ package org.springframework.boot.web.context; import org.springframework.boot.web.server.WebServer; import org.springframework.context.ApplicationContext; +import org.springframework.util.ObjectUtils; /** * Interface to be implemented by {@link ApplicationContext application contexts} that @@ -44,4 +45,17 @@ public interface WebServerApplicationContext extends ApplicationContext { */ String getServerNamespace(); + /** + * Returns {@code true} if the specified context is a + * {@link WebServerApplicationContext} with a matching server namespace. + * @param context the context to check + * @param serverNamespace the server namespace to match against + * @return {@code true} if the server namespace of the context matches + * @since 2.1.8 + */ + static boolean hasServerNamespace(ApplicationContext context, String serverNamespace) { + return (context instanceof WebServerApplicationContext) && ObjectUtils + .nullSafeEquals(((WebServerApplicationContext) context).getServerNamespace(), serverNamespace); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcherTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcherTests.java index bbb63f06665..c08bc5681f6 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcherTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcherTests.java @@ -69,6 +69,42 @@ public class ApplicationContextRequestMatcherTests { assertThatExceptionOfType(NoSuchBeanDefinitionException.class).isThrownBy(supplier::get); } + @Test // gh-18012 + public void machesWhenCalledWithDifferentApplicationContextDoesNotCache() { + StaticWebApplicationContext context1 = createWebApplicationContext(); + StaticWebApplicationContext context2 = createWebApplicationContext(); + TestApplicationContextRequestMatcher matcher = new TestApplicationContextRequestMatcher<>( + ApplicationContext.class); + assertThat(matcher.callMatchesAndReturnProvidedContext(context1).get()).isEqualTo(context1); + assertThat(matcher.callMatchesAndReturnProvidedContext(context2).get()).isEqualTo(context2); + } + + @Test + public void initializeAndMatchesAreNotCalledIfContextIsIgnored() { + StaticWebApplicationContext context = createWebApplicationContext(); + TestApplicationContextRequestMatcher matcher = new TestApplicationContextRequestMatcher( + ApplicationContext.class) { + + @Override + protected boolean ignoreApplicationContext(WebApplicationContext webApplicationContext) { + return true; + } + + @Override + protected void initialized(Supplier context) { + throw new IllegalStateException(); + } + + @Override + protected boolean matches(HttpServletRequest request, Supplier context) { + throw new IllegalStateException(); + } + + }; + MockHttpServletRequest request = new MockHttpServletRequest(context.getServletContext()); + assertThat(matcher.matches(request)).isFalse(); + } + private StaticWebApplicationContext createWebApplicationContext() { StaticWebApplicationContext context = new StaticWebApplicationContext(); MockServletContext servletContext = new MockServletContext(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/context/WebServerApplicationContextTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/context/WebServerApplicationContextTests.java new file mode 100644 index 00000000000..6b1254dbe8b --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/context/WebServerApplicationContextTests.java @@ -0,0 +1,53 @@ +/* + * Copyright 2012-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.boot.web.context; + +import org.junit.Test; + +import org.springframework.context.ApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link WebServerApplicationContext}. + * + * @author Phillip Webb + */ +public class WebServerApplicationContextTests { + + @Test + public void hasServerNamespaceWhenContextIsNotWebServerApplicationContextReturnsFalse() { + ApplicationContext context = mock(ApplicationContext.class); + assertThat(WebServerApplicationContext.hasServerNamespace(context, "test")).isFalse(); + } + + @Test + public void hasServerNamespaceWhenContextIsWebServerApplicationContextAndNamespaceDoesNotMatchReturnsFalse() { + ApplicationContext context = mock(WebServerApplicationContext.class); + assertThat(WebServerApplicationContext.hasServerNamespace(context, "test")).isFalse(); + } + + @Test + public void hasServerNamespaceWhenContextIsWebServerApplicationContextAndNamespaceMatchesReturnsTrue() { + WebServerApplicationContext context = mock(WebServerApplicationContext.class); + given(context.getServerNamespace()).willReturn("test"); + assertThat(WebServerApplicationContext.hasServerNamespace(context, "test")).isTrue(); + } + +}