From 7d74b7c87e0b7e418ee7230c44c67bc1c7b418d1 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 6 Apr 2010 20:50:47 +0100 Subject: [PATCH] SEC-1171: Allow multiple http elements and add pattern attribute to specify filter chain mapping. --- .../http/DefaultFilterChainValidator.java | 102 +++++++++--------- .../HttpSecurityBeanDefinitionParser.java | 49 +++++++-- .../security/config/spring-security-3.1.rnc | 3 + .../security/config/spring-security-3.1.xsd | 5 + .../config/http/MiscHttpConfigTests.groovy | 10 -- .../http/MultiHttpBlockConfigTests.groovy | 47 ++++++++ .../web/util/AntPathRequestMatcher.java | 10 ++ 7 files changed, 153 insertions(+), 73 deletions(-) create mode 100644 config/src/test/groovy/org/springframework/security/config/http/MultiHttpBlockConfigTests.groovy diff --git a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java index 38450244d8..d078d73bd1 100644 --- a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java +++ b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java @@ -22,21 +22,18 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationFi import org.springframework.security.web.context.SecurityContextPersistenceFilter; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.session.SessionManagementFilter; -import org.springframework.security.web.util.AnyRequestMatcher; public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator { private Log logger = LogFactory.getLog(getClass()); public void validate(FilterChainProxy fcp) { for(List filters : fcp.getFilterChainMap().values()) { + checkLoginPageIsntProtected(fcp, filters); checkFilterStack(filters); } - - checkLoginPageIsntProtected(fcp); } private Object getFilter(Class type, List filters) { - for (Filter f : filters) { if (type.isAssignableFrom(f.getClass())) { return f; @@ -77,59 +74,60 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain } /* Checks for the common error of having a login page URL protected by the security interceptor */ - private void checkLoginPageIsntProtected(FilterChainProxy fcp) { - List defaultFilters = fcp.getFilterChainMap().get(new AnyRequestMatcher()); - ExceptionTranslationFilter etf = (ExceptionTranslationFilter)getFilter(ExceptionTranslationFilter.class, defaultFilters); + private void checkLoginPageIsntProtected(FilterChainProxy fcp, List filterStack) { + ExceptionTranslationFilter etf = (ExceptionTranslationFilter)getFilter(ExceptionTranslationFilter.class, filterStack); - if (etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint) { - String loginPage = - ((LoginUrlAuthenticationEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl(); - FilterInvocation loginRequest = new FilterInvocation(loginPage, "POST"); - List filters = fcp.getFilters(loginPage); - logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration"); + if(etf == null || !(etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint)) { + return; + } - if (filters == null || filters.isEmpty()) { - logger.debug("Filter chain is empty for the login page"); - return; + String loginPage = ((LoginUrlAuthenticationEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl(); + FilterInvocation loginRequest = new FilterInvocation(loginPage, "POST"); + List filters = fcp.getFilters(loginPage); + logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration"); + + if (filters == null || filters.isEmpty()) { + logger.debug("Filter chain is empty for the login page"); + return; + } + + if (getFilter(DefaultLoginPageGeneratingFilter.class, filters) != null) { + logger.debug("Default generated login page is in use"); + return; + } + + FilterSecurityInterceptor fsi = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class, filters); + DefaultFilterInvocationSecurityMetadataSource fids = + (DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource(); + + Collection attributes = fids.getAttributes(loginRequest); + + if (attributes == null) { + logger.debug("No access attributes defined for login page URL"); + if (fsi.isRejectPublicInvocations()) { + logger.warn("FilterSecurityInterceptor is configured to reject public invocations." + + " Your login page may not be accessible."); } + return; + } - if (getFilter(DefaultLoginPageGeneratingFilter.class, filters) != null) { - logger.debug("Default generated login page is in use"); - return; - } + AnonymousAuthenticationFilter anonPF = (AnonymousAuthenticationFilter) getFilter(AnonymousAuthenticationFilter.class, filters); + if (anonPF == null) { + logger.warn("The login page is being protected by the filter chain, but you don't appear to have" + + " anonymous authentication enabled. This is almost certainly an error."); + return; + } - FilterSecurityInterceptor fsi = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class, filters); - DefaultFilterInvocationSecurityMetadataSource fids = - (DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource(); - - Collection attributes = fids.getAttributes(loginRequest); - - if (attributes == null) { - logger.debug("No access attributes defined for login page URL"); - if (fsi.isRejectPublicInvocations()) { - logger.warn("FilterSecurityInterceptor is configured to reject public invocations." + - " Your login page may not be accessible."); - } - return; - } - - AnonymousAuthenticationFilter anonPF = (AnonymousAuthenticationFilter) getFilter(AnonymousAuthenticationFilter.class, filters); - if (anonPF == null) { - logger.warn("The login page is being protected by the filter chain, but you don't appear to have" + - " anonymous authentication enabled. This is almost certainly an error."); - return; - } - - // Simulate an anonymous access with the supplied attributes. - AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), - anonPF.getUserAttribute().getAuthorities()); - try { - fsi.getAccessDecisionManager().decide(token, new Object(), attributes); - } catch (Exception e) { - logger.warn("Anonymous access to the login page doesn't appear to be enabled. This is almost certainly " + - "an error. Please check your configuration allows unauthenticated access to the configured " + - "login page. (Simulated access was rejected: " + e + ")"); - } + // Simulate an anonymous access with the supplied attributes. + AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), + anonPF.getUserAttribute().getAuthorities()); + try { + fsi.getAccessDecisionManager().decide(token, new Object(), attributes); + } catch (Exception e) { + logger.warn("Anonymous access to the login page doesn't appear to be enabled. This is almost certainly " + + "an error. Please check your configuration allows unauthenticated access to the configured " + + "login page. (Simulated access was rejected: " + e + ")"); } } + } diff --git a/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java index 0ea95e8de5..ff11cf35f6 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java @@ -11,6 +11,7 @@ import org.springframework.beans.BeanMetadataElement; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanReference; import org.springframework.beans.factory.config.RuntimeBeanReference; +import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder; import org.springframework.beans.factory.parsing.BeanComponentDefinition; import org.springframework.beans.factory.parsing.CompositeComponentDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; @@ -104,9 +105,20 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { filterChain.add(od.bean); } + // Get the map of empty filter chains (if any) ManagedMap> filterChainMap = httpBldr.getFilterChainMap(); - BeanDefinition universalMatch = new RootBeanDefinition(AnyRequestMatcher.class); - filterChainMap.put(universalMatch, filterChain); + + String filterChainPattern = element.getAttribute(ATT_PATH_PATTERN); + + BeanDefinition filterChainMatcher; + + if (StringUtils.hasText(filterChainPattern)) { + filterChainMatcher = matcherType.createMatcher(filterChainPattern, null); + } else { + filterChainMatcher = new RootBeanDefinition(AnyRequestMatcher.class); + } + + filterChainMap.put(filterChainMatcher, filterChain); registerFilterChainProxy(pc, filterChainMap, source); @@ -215,18 +227,33 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { return customFilters; } + @SuppressWarnings("unchecked") private void registerFilterChainProxy(ParserContext pc, Map> filterChainMap, Object source) { if (pc.getRegistry().containsBeanDefinition(BeanIds.FILTER_CHAIN_PROXY)) { - pc.getReaderContext().error("Duplicate element detected", source); - } + // Already registered. Obtain the filter chain map and add the new entries to it + // pc.getReaderContext().error("Duplicate element detected", source); - BeanDefinitionBuilder fcpBldr = BeanDefinitionBuilder.rootBeanDefinition(FilterChainProxy.class); - fcpBldr.getRawBeanDefinition().setSource(source); -// fcpBldr.addPropertyValue("stripQueryStringFromUrls", Boolean.valueOf(matcher instanceof AntUrlPathMatcher)); - fcpBldr.addPropertyValue("filterChainMap", filterChainMap); - BeanDefinition fcpBean = fcpBldr.getBeanDefinition(); - pc.registerBeanComponent(new BeanComponentDefinition(fcpBean, BeanIds.FILTER_CHAIN_PROXY)); - pc.getRegistry().registerAlias(BeanIds.FILTER_CHAIN_PROXY, BeanIds.SPRING_SECURITY_FILTER_CHAIN); + BeanDefinition fcp = pc.getRegistry().getBeanDefinition(BeanIds.FILTER_CHAIN_PROXY); + Map existingFilterChainMap = (Map) fcp.getPropertyValues().getPropertyValue("filterChainMap").getValue(); + + for (BeanDefinition matcherBean : filterChainMap.keySet()) { + if (existingFilterChainMap.containsKey(matcherBean)) { + Map args = matcherBean.getConstructorArgumentValues().getIndexedArgumentValues(); + pc.getReaderContext().error("The filter chain map already contains this request matcher [" + + args.get(0).getValue() + ", " +args.get(1).getValue() + "]", source); + } + } + existingFilterChainMap.putAll(filterChainMap); + } else { + // Not already registered, so register it + BeanDefinitionBuilder fcpBldr = BeanDefinitionBuilder.rootBeanDefinition(FilterChainProxy.class); + fcpBldr.getRawBeanDefinition().setSource(source); + fcpBldr.addPropertyValue("filterChainMap", filterChainMap); + fcpBldr.addPropertyValue("filterChainValidator", new RootBeanDefinition(DefaultFilterChainValidator.class)); + BeanDefinition fcpBean = fcpBldr.getBeanDefinition(); + pc.registerBeanComponent(new BeanComponentDefinition(fcpBean, BeanIds.FILTER_CHAIN_PROXY)); + pc.getRegistry().registerAlias(BeanIds.FILTER_CHAIN_PROXY, BeanIds.SPRING_SECURITY_FILTER_CHAIN); + } } } diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc index 98769e6ac1..25bb958470 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc @@ -257,6 +257,9 @@ protect-pointcut.attlist &= http = ## Container element for HTTP security configuration element http {http.attlist, (intercept-url+ & access-denied-handler? & form-login? & openid-login? & x509? & http-basic? & logout? & session-management & remember-me? & anonymous? & port-mappings & custom-filter* & request-cache?) } +http.attlist &= + ## The request URL pattern which will be mapped to the filter chain created by this element. If omitted, the filter chain will match all requests. + attribute pattern {xsd:token}? http.attlist &= ## Automatically registers a login form, BASIC authentication, anonymous authentication, logout services, remember-me and servlet-api-integration. If set to "true", all of these capabilities are added (although you can still customize the configuration of each by providing the respective element). If unspecified, defaults to "false". attribute auto-config {boolean}? diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd index bda5964874..db30fb3116 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd @@ -687,6 +687,11 @@ + + + The request URL pattern which will be mapped to the filter chain created by this <http> element. If omitted, the filter chain will match all requests. + + Automatically registers a login form, BASIC authentication, anonymous authentication, logout services, remember-me and servlet-api-integration. If set to "true", all of these capabilities are added (although you can still customize the configuration of each by providing the respective element). If unspecified, defaults to "false". diff --git a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy index 042c95cb10..0110c780d9 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy @@ -82,16 +82,6 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { assert fsi.isObserveOncePerRequest() } - def duplicateElementCausesError() { - when: "Two http blocks are defined" - xml.http('auto-config': 'true') - xml.http('auto-config': 'true') - createAppContext() - - then: - BeanDefinitionParsingException e = thrown(); - } - def filterListShouldBeEmptyForPatternWithNoFilters() { httpAutoConfig() { interceptUrlNoFilters('/unprotected') diff --git a/config/src/test/groovy/org/springframework/security/config/http/MultiHttpBlockConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/MultiHttpBlockConfigTests.groovy new file mode 100644 index 0000000000..08a63188ae --- /dev/null +++ b/config/src/test/groovy/org/springframework/security/config/http/MultiHttpBlockConfigTests.groovy @@ -0,0 +1,47 @@ +package org.springframework.security.config.http + +import java.util.Map; +import java.util.List; + +import org.springframework.security.web.FilterChainProxy; +import org.springframework.security.config.BeanIds; + +import org.springframework.beans.factory.parsing.BeanDefinitionParsingException + +/** + * Tests scenarios with multiple <http> elements. + * + * @author Luke Taylor + */ +class MultiHttpBlockConfigTests extends AbstractHttpConfigTests { + + def multipleHttpElementsAreSupported () { + when: "Two elements are used" + xml.http(pattern: '/stateless/**', 'create-session': 'stateless') { + 'http-basic'() + } + xml.http(pattern: '/stateful/**') { + 'form-login'() + } + createAppContext() + FilterChainProxy fcp = appContext.getBean(BeanIds.FILTER_CHAIN_PROXY) + Map filterChains = fcp.getFilterChainMap(); + + then: + filterChains.size() == 2 + (filterChains.keySet() as List)[0].pattern == '/stateless/**' + } + + def duplicatePatternsAreRejected () { + when: "Two elements are used" + xml.http(pattern: '/stateless/**', 'create-session': 'stateless') { + 'http-basic'() + } + xml.http(pattern: '/stateless/**') { + 'form-login'() + } + createAppContext() + then: + thrown(BeanDefinitionParsingException) + } +} diff --git a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java index b88d41beae..9a394ceb20 100644 --- a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java @@ -80,6 +80,16 @@ public final class AntPathRequestMatcher implements RequestMatcher { return pattern; } + @Override + public boolean equals(Object obj) { + if (!(obj instanceof AntPathRequestMatcher)) { + return false; + } + AntPathRequestMatcher other = (AntPathRequestMatcher)obj; + return this.pattern.equals(other.pattern) && + this.httpMethod == other.httpMethod; + } + @Override public String toString() { StringBuilder sb = new StringBuilder();