diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java index a277ae1e82..610edb8fd3 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java @@ -15,15 +15,6 @@ */ package org.springframework.security.config.annotation.web; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; - -import javax.servlet.ServletContext; -import javax.servlet.ServletRegistration; - -import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.context.ApplicationContext; import org.springframework.http.HttpMethod; import org.springframework.security.config.annotation.ObjectPostProcessor; @@ -34,9 +25,12 @@ import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.ClassUtils; -import org.springframework.web.context.ServletContextAware; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + /** * A base class for registering {@link RequestMatcher}'s. For example, it might allow for * specifying which {@link RequestMatcher} require a certain level of authorization. @@ -171,12 +165,9 @@ public abstract class AbstractRequestMatcherRegistry { List matchers = new ArrayList( mvcPatterns.length); for (String mvcPattern : mvcPatterns) { - MvcRequestMatcher matcher; - if(isServlet30) { - matcher = new ServletPathValidatingtMvcRequestMatcher(introspector, mvcPattern); + MvcRequestMatcher matcher = new MvcRequestMatcher(introspector, mvcPattern); + if (isServlet30) { opp.postProcess(matcher); - } else { - matcher = new MvcRequestMatcher(introspector, mvcPattern); } if (method != null) { matcher.setMethod(method); @@ -316,48 +307,4 @@ public abstract class AbstractRequestMatcherRegistry { } } - static class ServletPathValidatingtMvcRequestMatcher extends MvcRequestMatcher implements SmartInitializingSingleton, ServletContextAware { - private ServletContext servletContext; - - /** - * @param introspector - * @param pattern - */ - public ServletPathValidatingtMvcRequestMatcher(HandlerMappingIntrospector introspector, - String pattern) { - super(introspector, pattern); - } - - /* (non-Javadoc) - * @see org.springframework.beans.factory.SmartInitializingSingleton#afterSingletonsInstantiated() - */ - @Override - public void afterSingletonsInstantiated() { - if(getServletPath() != null) { - return; - } - Collection registrations = servletContext.getServletRegistrations().values(); - for(ServletRegistration registration : registrations) { - Collection mappings = registration.getMappings(); - for(String mapping : mappings) { - if(mapping.startsWith("/") && mapping.length() > 1) { - throw new IllegalStateException( - "servletPath must not be null for mvcPattern \"" + getMvcPattern() - + "\" when providing a servlet mapping of " - + mapping + " for servlet " - + registration.getClassName()); - } - } - } - } - - /* (non-Javadoc) - * @see org.springframework.web.context.ServletContextAware#setServletContext(javax.servlet.ServletContext) - */ - @Override - public void setServletContext(ServletContext servletContext) { - this.servletContext = servletContext; - } - - } } diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd index 7b3e1e59db..4175b9599a 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd @@ -1351,7 +1351,7 @@ 'mvc'. In addition, the value is only required in the following 2 use cases: 1) There are 2 or more HttpServlet's registered in the ServletContext that have mappings starting with '/' and are different; 2) The pattern starts with the same value of a registered - HttpServlet path, excluding the default (root) HttpServlet '/' + HttpServlet path, excluding the default (root) HttpServlet '/'. diff --git a/config/src/test/groovy/org/springframework/security/config/AbstractXmlConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/AbstractXmlConfigTests.groovy index fddaa1da15..4bc4f59ffa 100644 --- a/config/src/test/groovy/org/springframework/security/config/AbstractXmlConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/AbstractXmlConfigTests.groovy @@ -1,26 +1,25 @@ package org.springframework.security.config import groovy.xml.MarkupBuilder - -import org.mockito.Mockito; -import org.springframework.context.support.AbstractXmlApplicationContext +import org.mockito.Mockito +import org.springframework.context.ApplicationListener +import org.springframework.context.support.AbstractRefreshableApplicationContext +import org.springframework.mock.web.MockServletContext +import org.springframework.security.CollectingAppListener import org.springframework.security.config.util.InMemoryXmlApplicationContext +import org.springframework.security.config.util.InMemoryXmlWebApplicationContext import org.springframework.security.core.context.SecurityContextHolder import spock.lang.Specification -import static org.springframework.security.config.ConfigTestUtils.AUTH_PROVIDER_XML -import org.springframework.context.ApplicationListener -import org.springframework.context.ApplicationEvent -import org.springframework.security.authentication.event.AbstractAuthenticationEvent -import org.springframework.security.authentication.event.AbstractAuthenticationFailureEvent -import org.springframework.security.access.event.AbstractAuthorizationEvent -import org.springframework.security.CollectingAppListener +import javax.servlet.ServletContext + +import static org.springframework.security.config.ConfigTestUtils.AUTH_PROVIDER_XML /** * * @author Luke Taylor */ abstract class AbstractXmlConfigTests extends Specification { - AbstractXmlApplicationContext appContext; + AbstractRefreshableApplicationContext appContext; Writer writer; MarkupBuilder xml; ApplicationListener appListener; @@ -81,4 +80,27 @@ abstract class AbstractXmlConfigTests extends Specification { appContext = new InMemoryXmlApplicationContext(writer.toString() + extraXml); appContext.addApplicationListener(appListener); } + + def createWebAppContext() { + createWebAppContext(AUTH_PROVIDER_XML); + } + + def createWebAppContext(ServletContext servletContext) { + createWebAppContext(AUTH_PROVIDER_XML, servletContext); + } + + def createWebAppContext(String extraXml) { + createWebAppContext(extraXml, null); + } + + def createWebAppContext(String extraXml, ServletContext servletContext) { + appContext = new InMemoryXmlWebApplicationContext(writer.toString() + extraXml); + appContext.addApplicationListener(appListener); + if (servletContext != null) { + appContext.setServletContext(servletContext); + } else { + appContext.setServletContext(new MockServletContext()); + } + appContext.refresh(); + } } diff --git a/config/src/test/groovy/org/springframework/security/config/doc/XsdDocumentedTests.groovy b/config/src/test/groovy/org/springframework/security/config/doc/XsdDocumentedTests.groovy index 7a67e110a5..a52a0adacd 100644 --- a/config/src/test/groovy/org/springframework/security/config/doc/XsdDocumentedTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/doc/XsdDocumentedTests.groovy @@ -16,7 +16,6 @@ package org.springframework.security.config.doc import groovy.util.slurpersupport.GPathResult; -import groovy.util.slurpersupport.NodeChild import org.springframework.security.config.http.SecurityFilters @@ -89,7 +88,7 @@ class XsdDocumentedTests extends Specification { def 'the latest schema is being validated'() { when: 'all the schemas are found' def schemas = schemaDocument.getParentFile().list().findAll { it.endsWith('.xsd') } - then: 'the count is equal to 8, if not then schemaDocument needs updated' + then: 'the count is equal to 10, if not then schemaDocument needs updated' schemas.size() == 10 } diff --git a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy index 7c71c2d901..7df41dcbfe 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy @@ -15,17 +15,25 @@ */ package org.springframework.security.config.http +import org.mockito.invocation.InvocationOnMock +import org.mockito.stubbing.Answer +import org.springframework.beans.factory.BeanCreationException import org.springframework.beans.factory.parsing.BeanDefinitionParsingException import org.springframework.mock.web.MockFilterChain import org.springframework.mock.web.MockHttpServletRequest import org.springframework.mock.web.MockHttpServletResponse +import org.springframework.mock.web.MockServletContext import org.springframework.security.access.SecurityConfig import org.springframework.security.crypto.codec.Base64 import org.springframework.security.web.access.intercept.FilterSecurityInterceptor import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController +import javax.servlet.ServletContext +import javax.servlet.ServletRegistration import javax.servlet.http.HttpServletResponse + +import static org.mockito.Mockito.* /** * * @author Rob Winch @@ -199,6 +207,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { def "intercept-url supports mvc matchers"() { setup: + MockServletContext servletContext = mockServletContext(); MockHttpServletRequest request = new MockHttpServletRequest(method:'GET') MockHttpServletResponse response = new MockHttpServletResponse() MockFilterChain chain = new MockFilterChain() @@ -209,7 +218,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { bean('pathController',PathController) xml.'mvc:annotation-driven'() - createAppContext() + createWebAppContext(servletContext) when: request.servletPath = "/path" springSecurityFilterChain.doFilter(request, response, chain) @@ -235,6 +244,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { def "intercept-url mvc supports path variables"() { setup: + MockServletContext servletContext = mockServletContext(); MockHttpServletRequest request = new MockHttpServletRequest(method:'GET') MockHttpServletResponse response = new MockHttpServletResponse() MockFilterChain chain = new MockFilterChain() @@ -242,7 +252,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { 'http-basic'() 'intercept-url'(pattern: '/user/{un}/**', access: "#un == 'user'") } - createAppContext() + createWebAppContext(servletContext) when: 'user can access' request.servletPath = '/user/user/abc' springSecurityFilterChain.doFilter(request,response,chain) @@ -266,6 +276,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { def "intercept-url mvc matchers with servlet path"() { setup: + MockServletContext servletContext = mockServletContext("/spring"); MockHttpServletRequest request = new MockHttpServletRequest(method:'GET') MockHttpServletResponse response = new MockHttpServletResponse() MockFilterChain chain = new MockFilterChain() @@ -275,7 +286,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { } bean('pathController',PathController) xml.'mvc:annotation-driven'() - createAppContext() + createWebAppContext(servletContext) when: request.servletPath = "/spring" request.requestURI = "/spring/path" @@ -302,6 +313,30 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { response.status == HttpServletResponse.SC_UNAUTHORIZED } + def "intercept-url mvc matchers servlet path required"() { + when: + MockServletContext servletContext = mockServletContext("/spring"); + xml.http('request-matcher':'mvc') { + 'http-basic'() + 'intercept-url'(pattern: '/path', access: "denyAll") + } + createWebAppContext(servletContext) + then: + thrown(BeanCreationException) + } + + def "intercept-url mvc matchers servlet path NOT required"() { + when: + MockServletContext servletContext = mockServletContext(); + xml.http('request-matcher':'mvc') { + 'http-basic'() + 'intercept-url'(pattern: '/path', access: "denyAll") + } + createWebAppContext(servletContext) + then: + noExceptionThrown() + } + def "intercept-url ant matcher with servlet path fails"() { when: xml.http('request-matcher':'ant') { @@ -352,6 +387,24 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { } } + private ServletContext mockServletContext() { + return mockServletContext("/"); + } + + private ServletContext mockServletContext(String servletPath) { + MockServletContext servletContext = spy(new MockServletContext()); + final ServletRegistration registration = mock(ServletRegistration.class); + when(registration.getMappings()).thenReturn(Collections.singleton(servletPath)); + Answer> answer = new Answer>() { + @Override + public Map answer(InvocationOnMock invocation) throws Throwable { + return Collections.singletonMap("spring", registration); + } + }; + when(servletContext.getServletRegistrations()).thenAnswer(answer); + return servletContext; + } + def login(MockHttpServletRequest request, String username, String password) { String toEncode = username + ':' + password request.addHeader('Authorization','Basic ' + new String(Base64.encode(toEncode.getBytes('UTF-8')))) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java similarity index 73% rename from config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java rename to config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java index 691d67e4ab..e20bee96d3 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java @@ -16,14 +16,6 @@ package org.springframework.security.config.annotation.web; -import java.util.Arrays; -import java.util.Collections; -import java.util.Map; - -import javax.servlet.Registration; -import javax.servlet.ServletContext; -import javax.servlet.ServletRegistration; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,14 +23,17 @@ import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; import org.mockito.stubbing.Answer; - import org.springframework.mock.web.MockServletContext; -import org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.ServletPathValidatingtMvcRequestMatcher; +import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; +import javax.servlet.ServletContext; +import javax.servlet.ServletRegistration; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; + +import static org.mockito.Mockito.*; /** * @author Rob Winch @@ -49,34 +44,34 @@ public class AbstractRequestMatcherRegistryTests { @Mock HandlerMappingIntrospector introspector; - ServletPathValidatingtMvcRequestMatcher matcher; + MvcRequestMatcher matcher; ServletContext servletContext; @Before public void setup() { servletContext = spy(new MockServletContext()); - matcher = new ServletPathValidatingtMvcRequestMatcher(introspector, "/foo"); + matcher = new MvcRequestMatcher(introspector, "/foo"); matcher.setServletContext(servletContext); } @Test(expected = IllegalStateException.class) - public void servletPathValidatingtMvcRequestMatcherAfterSingletonsIntantiatedFailsWithSpringServlet() { + public void servletPathValidatingMvcRequestMatcherAfterPropertiesSetFailsWithSpringServlet() throws Exception { setMappings("/spring"); - matcher.afterSingletonsInstantiated(); + matcher.afterPropertiesSet(); } @Test - public void servletPathValidatingtMvcRequestMatcherAfterSingletonsIntantiatedWithSpringServlet() { + public void servletPathValidatingMvcRequestMatcherAfterPropertiesSetWithSpringServlet() throws Exception { matcher.setServletPath("/spring"); setMappings("/spring"); - matcher.afterSingletonsInstantiated(); + matcher.afterPropertiesSet(); } @Test - public void servletPathValidatingtMvcRequestMatcherAfterSingletonsIntantiatedDefaultServlet() { + public void servletPathValidatingMvcRequestMatcherAfterPropertiesSetDefaultServlet() throws Exception { setMappings("/"); - matcher.afterSingletonsInstantiated(); + matcher.afterPropertiesSet(); } private void setMappings(String... mappings) { diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java index 32c2b5a3e0..f6363423ea 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java @@ -27,6 +27,7 @@ import org.junit.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -449,7 +450,7 @@ public class AuthorizeRequestsTests { } } - @Test(expected = IllegalStateException.class) + @Test(expected = BeanCreationException.class) public void mvcMatcherServletPathRequired() throws Exception { final ServletRegistration registration = mock(ServletRegistration.class); when(registration.getMappings()).thenReturn(Collections.singleton("/spring")); @@ -502,4 +503,4 @@ public class AuthorizeRequestsTests { this.context.getAutowireCapableBeanFactory().autowireBean(this); } -} \ No newline at end of file +} diff --git a/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java b/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java index 1b530eb4b9..dc1b716ec3 100644 --- a/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java +++ b/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java @@ -25,7 +25,7 @@ import org.springframework.security.util.InMemoryResource; * @author Luke Taylor */ public class InMemoryXmlApplicationContext extends AbstractXmlApplicationContext { - private static final String BEANS_OPENING = "\n" + xml + BEANS_CLOSE; + inMemoryXml = new InMemoryResource(fullXml); + setAllowBeanDefinitionOverriding(true); + setParent(parent); + } + + @Override + protected void loadBeanDefinitions(DefaultListableBeanFactory beanFactory) throws BeansException, IOException { + XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(beanFactory); + reader.loadBeanDefinitions(new Resource[] { inMemoryXml }); + } + +} diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java index 5cb659cec7..75a5c7cf3a 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java @@ -16,17 +16,24 @@ package org.springframework.security.web.servlet.util.matcher; +import org.springframework.beans.factory.InitializingBean; import org.springframework.http.HttpMethod; +import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestVariablesExtractor; import org.springframework.util.AntPathMatcher; +import org.springframework.util.ClassUtils; import org.springframework.util.PathMatcher; +import org.springframework.web.context.ServletContextAware; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; import org.springframework.web.servlet.handler.MatchableHandlerMapping; import org.springframework.web.servlet.handler.RequestMatchResult; import org.springframework.web.util.UrlPathHelper; +import javax.servlet.ServletContext; +import javax.servlet.ServletRegistration; import javax.servlet.http.HttpServletRequest; +import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -45,13 +52,18 @@ import java.util.Map; * @since 4.1.1 */ public class MvcRequestMatcher - implements RequestMatcher, RequestVariablesExtractor { + implements RequestMatcher, RequestVariablesExtractor, InitializingBean, ServletContextAware { + + private static final boolean isServlet30 = ClassUtils.isPresent( + "javax.servlet.ServletRegistration", MvcRequestMatcher.class.getClassLoader()); + private final DefaultMatcher defaultMatcher = new DefaultMatcher(); private final HandlerMappingIntrospector introspector; private final String pattern; private HttpMethod method; private String servletPath; + private ServletContext servletContext; public MvcRequestMatcher(HandlerMappingIntrospector introspector, String pattern) { this.introspector = introspector; @@ -100,6 +112,40 @@ public class MvcRequestMatcher : result.extractUriTemplateVariables(); } + /* (non-Javadoc) + * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet() + */ + @Override + public void afterPropertiesSet() throws Exception { + // servletPath is required when at least one registered Servlet + // is mapped to a path other than the default (root) path '/' + if (this.servletContext == null || !isServlet30) { + return; + } + if (this.getServletPath() != null) { + return; + } + for (ServletRegistration registration : this.servletContext.getServletRegistrations().values()) { + for (String mapping : registration.getMappings()) { + if (mapping.startsWith("/") && mapping.length() > 1) { + throw new IllegalStateException( + "servletPath must not be null for mvcPattern \"" + this.getMvcPattern() + + "\" when providing a servlet mapping of " + + mapping + " for servlet " + + registration.getClassName()); + } + } + } + } + + /* (non-Javadoc) + * @see org.springframework.web.context.ServletContextAware#setServletContext(javax.servlet.ServletContext) + */ + @Override + public void setServletContext(ServletContext servletContext) { + this.servletContext = servletContext; + } + /** * @param method the method to set */