From 34401416b061300bdb2e077674726c2b17232699 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 5 May 2010 21:18:52 +0100 Subject: [PATCH] SEC-1171: Implement parsing of empty filter chain patters via http 'secured' attribute and remove filters='none' support. --- .../config/http/HttpConfigurationBuilder.java | 45 +++---------- .../HttpSecurityBeanDefinitionParser.java | 67 ++++++++++++------- .../security/config/spring-security-3.1.rnc | 8 ++- .../security/config/spring-security-3.1.xsd | 7 +- .../http/AbstractHttpConfigTests.groovy | 5 -- .../config/http/MiscHttpConfigTests.groovy | 15 ++--- .../http/PlaceHolderAndELConfigTests.groovy | 8 +-- ...HttpSecurityBeanDefinitionParserTests.java | 23 +++---- .../http-security-custom-concurrency.xml | 5 +- .../src/main/webapp/WEB-INF/http-security.xml | 4 +- 10 files changed, 91 insertions(+), 96 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java index 1cc6bae9f4..2b4375baa4 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java @@ -4,10 +4,8 @@ import static org.springframework.security.config.http.HttpSecurityBeanDefinitio import static org.springframework.security.config.http.SecurityFilters.*; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -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; @@ -82,9 +80,6 @@ class HttpConfigurationBuilder { private final List interceptUrls; private final MatcherType matcherType; - // Use ManagedMap to allow placeholder resolution - private ManagedMap> filterChainMap; - private BeanDefinition cpf; private BeanDefinition securityContextPersistenceFilter; private BeanReference contextRepoRef; @@ -105,6 +100,15 @@ class HttpConfigurationBuilder { this.portMapperName = portMapperName; this.matcherType = matcherType; interceptUrls = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL); + + for (Element urlElt : interceptUrls) { + if (StringUtils.hasText(urlElt.getAttribute(ATT_FILTERS))) { + pc.getReaderContext().error("The use of \"filters='none'\" is no longer supported. Please define a" + + " separate element for the pattern you want to exclude and use the attribute" + + " \"secured='false'\".", pc.extractSource(urlElt)); + } + } + String createSession = element.getAttribute(ATT_CREATE_SESSION); if (StringUtils.hasText(createSession)) { @@ -113,7 +117,6 @@ class HttpConfigurationBuilder { sessionPolicy = SessionCreationPolicy.ifRequired; } - parseInterceptUrlsForEmptyFilterChains(); createSecurityContextPersistenceFilter(); createSessionManagementFilters(); createRequestCacheFilter(); @@ -122,32 +125,6 @@ class HttpConfigurationBuilder { createFilterSecurityInterceptor(authenticationManager); } - private void parseInterceptUrlsForEmptyFilterChains() { - filterChainMap = new ManagedMap>(); - - for (Element urlElt : interceptUrls) { - String path = urlElt.getAttribute(ATT_PATH_PATTERN); - - if(!StringUtils.hasText(path)) { - pc.getReaderContext().error("path attribute cannot be empty or null", urlElt); - } - - BeanDefinition matcher = matcherType.createMatcher(path, null); - - String filters = urlElt.getAttribute(ATT_FILTERS); - - if (StringUtils.hasText(filters)) { - if (!filters.equals(OPT_FILTERS_NONE)) { - pc.getReaderContext().error("Currently only 'none' is supported as the custom " + - "filters attribute", urlElt); - } - - List noFilters = Collections.emptyList(); - filterChainMap.put(matcher, noFilters); - } - } - } - // Needed to account for placeholders static String createPath(String path, boolean lowerCase) { return lowerCase ? path.toLowerCase() : path; @@ -518,10 +495,6 @@ class HttpConfigurationBuilder { return requestCache; } - ManagedMap> getFilterChainMap() { - return filterChainMap; - } - List getFilters() { List filters = new ArrayList(); 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 ff11cf35f6..9b76ca2629 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 @@ -51,6 +51,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { static final String ATT_REQUIRES_CHANNEL = "requires-channel"; private static final String ATT_REF = "ref"; + private static final String ATT_SECURED = "secured"; static final String EXPRESSION_FIMDS_CLASS = "org.springframework.security.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource"; static final String EXPRESSION_HANDLER_CLASS = "org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler"; @@ -72,11 +73,48 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { CompositeComponentDefinition compositeDef = new CompositeComponentDefinition(element.getTagName(), pc.extractSource(element)); pc.pushContainingComponent(compositeDef); - final Object source = pc.extractSource(element); - - final String portMapperName = createPortMapper(element, pc); MatcherType matcherType = MatcherType.fromElement(element); + ManagedMap> filterChainMap = new ManagedMap>(); + + 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, createFilterChain(element, pc, matcherType)); + + registerFilterChainProxy(pc, filterChainMap, pc.extractSource(element)); + + pc.popAndRegisterContainingComponent(); + return null; + } + + List createFilterChain(Element element, ParserContext pc, MatcherType matcherType) { + boolean unSecured = "false".equals(element.getAttribute(ATT_SECURED)); + + if (unSecured) { + if (!StringUtils.hasText(element.getAttribute(ATT_PATH_PATTERN))) { + pc.getReaderContext().error("The '" + ATT_SECURED + "' attribute must be used in combination with" + + " the '" + ATT_PATH_PATTERN +"' attribute.", pc.extractSource(element)); + } + + for (int n=0; n < element.getChildNodes().getLength(); n ++) { + if (element.getChildNodes().item(n) instanceof Element) { + pc.getReaderContext().error("If you are using to define an unsecured pattern, " + + "it cannot contain child elements.", pc.extractSource(element)); + } + } + + return Collections.emptyList(); + } + + final String portMapperName = createPortMapper(element, pc); ManagedList authenticationProviders = new ManagedList(); BeanReference authenticationManager = createAuthenticationManager(element, pc, authenticationProviders, null); @@ -97,7 +135,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { unorderedFilterChain.addAll(buildCustomFilterList(element, pc)); Collections.sort(unorderedFilterChain, new OrderComparator()); - checkFilterChainOrder(unorderedFilterChain, pc, source); + checkFilterChainOrder(unorderedFilterChain, pc, pc.extractSource(element)); List filterChain = new ManagedList(); @@ -105,27 +143,10 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { filterChain.add(od.bean); } - // Get the map of empty filter chains (if any) - ManagedMap> filterChainMap = httpBldr.getFilterChainMap(); - - 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); - - pc.popAndRegisterContainingComponent(); - return null; + return filterChain; } + private String createPortMapper(Element elt, ParserContext pc) { // Register the portMapper. A default will always be created, even if no element exists. BeanDefinition portMapper = new PortMappingsBeanDefinitionParser().parse( 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 25bb958470..c4132c5f39 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 @@ -255,11 +255,15 @@ 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?) } + ## Container element for HTTP security configuration. Multiple elements can now be defined, each with a specific pattern to which the enclosed security configuration applies. A pattern can also be configured to bypass Spring Security's filters completely by setting the "secured" attribute to "false". + 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 &= + ## When set to 'false', requests matching the pattern attribute will be ignored by Spring Security. No security filters will be applied and no SecurityContext will be available. If set, the element must be empty, with no children. + attribute secured {boolean}? + 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 db30fb3116..0165a610cf 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 @@ -605,7 +605,7 @@ - Container element for HTTP security configuration + Container element for HTTP security configuration. Multiple elements can now be defined, each with a specific pattern to which the enclosed security configuration applies. A pattern can also be configured to bypass Spring Security's filters completely by setting the "secured" attribute to "false". @@ -692,6 +692,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. + + + When set to 'false', requests matching the pattern attribute will be ignored by Spring Security. No security filters will be applied and no SecurityContext will be available. If set, the <http> element must be empty, with no children. + + 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/AbstractHttpConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/AbstractHttpConfigTests.groovy index 843bf275a0..df25edc7de 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/AbstractHttpConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/AbstractHttpConfigTests.groovy @@ -35,11 +35,6 @@ abstract class AbstractHttpConfigTests extends AbstractXmlConfigTests { xml.'intercept-url'(pattern: path, method: httpMethod, access: authz) } - - def interceptUrlNoFilters(String path) { - xml.'intercept-url'(pattern: path, filters: 'none') - } - Filter getFilter(Class type) { List filters = getFilters("/any"); 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 0110c780d9..862c5932ca 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 @@ -83,9 +83,8 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { } def filterListShouldBeEmptyForPatternWithNoFilters() { - httpAutoConfig() { - interceptUrlNoFilters('/unprotected') - } + xml.http(pattern: '/unprotected', secured: 'false') + httpAutoConfig() {} createAppContext() expect: @@ -93,9 +92,8 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { } def regexPathsWorkCorrectly() { - httpAutoConfig('regex') { - interceptUrlNoFilters('\\A\\/[a-z]+') - } + xml.http(pattern: '\\A\\/[a-z]+', secured: 'false', 'request-matcher': 'regex') + httpAutoConfig() {} createAppContext() expect: @@ -105,9 +103,8 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests { def ciRegexPathsWorkCorrectly() { when: - httpAutoConfig('ciRegex') { - interceptUrlNoFilters('\\A\\/[a-z]+') - } + xml.http(pattern: '\\A\\/[a-z]+', secured: 'false', 'request-matcher': 'ciRegex') + httpAutoConfig() {} createAppContext() then: diff --git a/config/src/test/groovy/org/springframework/security/config/http/PlaceHolderAndELConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/PlaceHolderAndELConfigTests.groovy index 2d281d368f..242b0221b3 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/PlaceHolderAndELConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/PlaceHolderAndELConfigTests.groovy @@ -16,16 +16,16 @@ import org.springframework.security.web.authentication.UsernamePasswordAuthentic class PlaceHolderAndELConfigTests extends AbstractHttpConfigTests { - void setup() { + def setup() { // Add a PropertyPlaceholderConfigurer to the context for all the tests xml.'b:bean'('class': PropertyPlaceholderConfigurer.class.name) } - def filtersEqualsNoneSupportsPlaceholderForPattern() { + def unsecuredPatternSupportsPlaceholderForPattern() { System.setProperty("pattern.nofilters", "/unprotected"); + xml.http(pattern: '${pattern.nofilters}', secured: 'false') httpAutoConfig() { - interceptUrlNoFilters('${pattern.nofilters}') interceptUrl('/**', 'ROLE_A') } createAppContext() @@ -44,9 +44,9 @@ class PlaceHolderAndELConfigTests extends AbstractHttpConfigTests { System.setProperty("default.target", "/defaultTarget"); System.setProperty("auth.failure", "/authFailure"); + xml.http(pattern: '${login.page}', secured: 'false') xml.http { interceptUrl('${secure.Url}', '${secure.role}') - interceptUrlNoFilters('${login.page}'); 'form-login'('login-page':'${login.page}', 'default-target-url': '${default.target}', 'authentication-failure-url':'${auth.failure}'); } diff --git a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java index 7961da9942..51a5965806 100644 --- a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java @@ -151,9 +151,8 @@ public class HttpSecurityBeanDefinitionParserTests { @Test public void filterListShouldBeEmptyForPatternWithNoFilters() throws Exception { setContext( - " " + - " " + - " " + AUTH_PROVIDER_XML); + " " + + " " + AUTH_PROVIDER_XML); List filters = getFilters("/unprotected"); @@ -165,8 +164,8 @@ public class HttpSecurityBeanDefinitionParserTests { System.setProperty("pattern.nofilters", "/unprotected"); setContext( " " + + " " + " " + - " " + " " + " " + AUTH_PROVIDER_XML); @@ -178,9 +177,9 @@ public class HttpSecurityBeanDefinitionParserTests { @Test public void regexPathsWorkCorrectly() throws Exception { setContext( - " " + - " " + - " " + AUTH_PROVIDER_XML); + " " + + " " + + AUTH_PROVIDER_XML); assertEquals(0, getFilters("/imlowercase").size()); List allFilters = getFilters("/ImCaughtByTheAnyRequestMatcher"); checkAutoConfigFilters(allFilters); @@ -189,9 +188,9 @@ public class HttpSecurityBeanDefinitionParserTests { @Test public void ciRegexPathsWorkCorrectly() throws Exception { setContext( - " " + - " " + - " " + AUTH_PROVIDER_XML); + " " + + " " + + AUTH_PROVIDER_XML); assertEquals(0, getFilters("/imMixedCase").size()); // This will be matched by the default pattern ".*" List allFilters = getFilters("/Im_Caught_By_The_AnyRequestMatcher"); @@ -313,9 +312,9 @@ public class HttpSecurityBeanDefinitionParserTests { System.setProperty("auth.failure", "/authFailure"); setContext( "" + + "" + "" + " " + - " " + " " + "" + AUTH_PROVIDER_XML); @@ -883,8 +882,8 @@ public class HttpSecurityBeanDefinitionParserTests { closeAppContext(); // No filters applied to login page setContext( + " " + " " + - " " + " " + " " + " " + diff --git a/itest/web/src/main/webapp/WEB-INF/http-security-custom-concurrency.xml b/itest/web/src/main/webapp/WEB-INF/http-security-custom-concurrency.xml index c6e1092187..68e53ab9a1 100644 --- a/itest/web/src/main/webapp/WEB-INF/http-security-custom-concurrency.xml +++ b/itest/web/src/main/webapp/WEB-INF/http-security-custom-concurrency.xml @@ -4,10 +4,11 @@ xmlns:beans="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd - http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.0.xsd"> + http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.1.xsd"> + + - diff --git a/itest/web/src/main/webapp/WEB-INF/http-security.xml b/itest/web/src/main/webapp/WEB-INF/http-security.xml index e68da241d2..555fce6bc5 100644 --- a/itest/web/src/main/webapp/WEB-INF/http-security.xml +++ b/itest/web/src/main/webapp/WEB-INF/http-security.xml @@ -4,15 +4,15 @@ xmlns:beans="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd - http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.0.xsd"> + http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.1.xsd"> + -