SEC-1201: PropertyPlaceholderConfigurer does not work for intercept-url attributes. Added use of ManagedMaps and BeanDefinitions to support placeholders in the pattern and access attributes.

config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceBeanDefinitionParser.java

@@ -1,12 +1,15 @@
 package org.springframework.security.config.http;
 
-import java.util.LinkedHashMap;
 import java.util.List;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.ManagedMap;
 import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
-import org.springframework.security.access.ConfigAttribute;
+import org.springframework.security.access.SecurityConfig;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.RequestKey;
 import org.springframework.security.web.util.AntUrlPathMatcher;
@@ -23,6 +26,11 @@ import org.w3c.dom.Element;
  */
 public class FilterInvocationSecurityMetadataSourceBeanDefinitionParser extends AbstractSingleBeanDefinitionParser {
 
+    private static final String ATT_HTTP_METHOD = "method";
+    private static final String ATT_PATTERN = "pattern";
+    private static final String ATT_ACCESS = "access";
+    private static final Log logger = LogFactory.getLog(FilterInvocationSecurityMetadataSourceBeanDefinitionParser.class);
+
     protected String getBeanClassName(Element element) {
         return "org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource";
     }
@@ -44,11 +52,66 @@ public class FilterInvocationSecurityMetadataSourceBeanDefinitionParser extends
         UrlMatcher matcher = HttpSecurityBeanDefinitionParser.createUrlMatcher(element);
         boolean convertPathsToLowerCase = (matcher instanceof AntUrlPathMatcher) && matcher.requiresLowerCaseUrl();
 
-        LinkedHashMap<RequestKey, List<ConfigAttribute>> requestMap =
-        HttpSecurityBeanDefinitionParser.parseInterceptUrlsForFilterInvocationRequestMap(interceptUrls,
-                convertPathsToLowerCase, false, parserContext);
+        ManagedMap<BeanDefinition, BeanDefinition> requestMap = parseInterceptUrlsForFilterInvocationRequestMap(
+                interceptUrls, convertPathsToLowerCase, false, parserContext);
 
         builder.addConstructorArgValue(matcher);
         builder.addConstructorArgValue(requestMap);
     }
+
+    static ManagedMap<BeanDefinition, BeanDefinition> parseInterceptUrlsForFilterInvocationRequestMap(List<Element> urlElts,
+            boolean useLowerCasePaths, boolean useExpressions, ParserContext parserContext) {
+
+        ManagedMap<BeanDefinition, BeanDefinition> filterInvocationDefinitionMap = new ManagedMap<BeanDefinition, BeanDefinition>();
+
+        for (Element urlElt : urlElts) {
+            String access = urlElt.getAttribute(ATT_ACCESS);
+            if (!StringUtils.hasText(access)) {
+                continue;
+            }
+
+            String path = urlElt.getAttribute(ATT_PATTERN);
+
+            if(!StringUtils.hasText(path)) {
+                parserContext.getReaderContext().error("path attribute cannot be empty or null", urlElt);
+            }
+
+            if (useLowerCasePaths) {
+                path = path.toLowerCase();
+            }
+
+            String method = urlElt.getAttribute(ATT_HTTP_METHOD);
+            if (!StringUtils.hasText(method)) {
+                method = null;
+            }
+
+            // Use beans to
+
+            BeanDefinitionBuilder keyBldr = BeanDefinitionBuilder.rootBeanDefinition(RequestKey.class);
+            keyBldr.addConstructorArgValue(path);
+            keyBldr.addConstructorArgValue(method);
+
+            BeanDefinitionBuilder attributeBuilder = BeanDefinitionBuilder.rootBeanDefinition(SecurityConfig.class);
+            attributeBuilder.addConstructorArgValue(access);
+
+            if (useExpressions) {
+                logger.info("Creating access control expression attribute '" + access + "' for " + path);
+                // The expression will be parsed later by the ExpressionFilterInvocationSecurityMetadataSource
+                attributeBuilder.setFactoryMethod("createList");
+
+            } else {
+                attributeBuilder.setFactoryMethod("createListFromCommaDelimitedString");
+            }
+
+            BeanDefinition key = keyBldr.getBeanDefinition();
+
+            if (filterInvocationDefinitionMap.containsKey(key)) {
+                logger.warn("Duplicate URL defined: " + path + ". The original attribute values will be overwritten");
+            }
+
+            filterInvocationDefinitionMap.put(key, attributeBuilder.getBeanDefinition());
+        }
+
+        return filterInvocationDefinitionMap;
+    }
 }

config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java

@@ -102,14 +102,11 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
     private static final String OPT_SESSION_FIXATION_NO_PROTECTION = "none";
     private static final String OPT_SESSION_FIXATION_MIGRATE_SESSION = "migrateSession";
 
-    private static final String ATT_ACCESS_CONFIG = "access";
     static final String ATT_REQUIRES_CHANNEL = "requires-channel";
     private static final String OPT_REQUIRES_HTTP = "http";
     private static final String OPT_REQUIRES_HTTPS = "https";
     private static final String OPT_ANY_CHANNEL = "any";
 
-    private static final String ATT_HTTP_METHOD = "method";
-
     private static final String ATT_CREATE_SESSION = "create-session";
     private static final String DEF_CREATE_SESSION_IF_REQUIRED = "ifRequired";
     private static final String OPT_CREATE_SESSION_ALWAYS = "always";
@@ -169,7 +166,6 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
      * the map of filter chains defined, with the "universal" match pattern mapped to the list of beans which have been parsed here.
      */
     public BeanDefinition parse(Element element, ParserContext pc) {
-//        WebConfigUtils.registerProviderManagerIfNecessary(pc, element);
         CompositeComponentDefinition compositeDef =
             new CompositeComponentDefinition(element.getTagName(), pc.extractSource(element));
         pc.pushContainingComponent(compositeDef);
@@ -181,12 +177,12 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
         final boolean convertPathsToLowerCase = (matcher instanceof AntUrlPathMatcher) && matcher.requiresLowerCaseUrl();
         final boolean allowSessionCreation = !OPT_CREATE_SESSION_NEVER.equals(element.getAttribute(ATT_CREATE_SESSION));
         final boolean autoConfig = "true".equals(element.getAttribute(ATT_AUTO_CONFIG));
-        final Map<String, List<BeanMetadataElement>> filterChainMap =  new ManagedMap<String, List<BeanMetadataElement>>();
-        final LinkedHashMap<RequestKey, List<ConfigAttribute>> channelRequestMap = new LinkedHashMap<RequestKey, List<ConfigAttribute>>();
-
-        // filterChainMap and channelRequestMap are populated by this call
-        parseInterceptUrlsForChannelSecurityAndEmptyFilterChains(DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL),
-                filterChainMap, channelRequestMap, convertPathsToLowerCase, pc);
+        final List<Element> interceptUrls = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL);
+        // Use ManagedMap to allow placeholder resolution
+        final ManagedMap<String, List<BeanMetadataElement>> filterChainMap =
+            parseInterceptUrlsForEmptyFilterChains(interceptUrls, convertPathsToLowerCase, pc);
+        final LinkedHashMap<RequestKey, List<ConfigAttribute>> channelRequestMap =
+                parseInterceptUrlsForChannelSecurity(interceptUrls, convertPathsToLowerCase, pc);
 
         BeanDefinition cpf = null;
         BeanReference sessionRegistryRef = null;
@@ -840,11 +836,10 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
 
         boolean useExpressions = "true".equals(element.getAttribute(ATT_USE_EXPRESSIONS));
 
-        LinkedHashMap<RequestKey, List<ConfigAttribute>> requestToAttributesMap =
+        ManagedMap<BeanDefinition,BeanDefinition> requestToAttributesMap =
             parseInterceptUrlsForFilterInvocationRequestMap(DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL),
                     convertPathsToLowerCase, useExpressions, pc);
 
-
         RootBeanDefinition accessDecisionMgr;
         ManagedList<BeanDefinition> voters =  new ManagedList<BeanDefinition>(2);
 
@@ -1180,7 +1175,6 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
             lowercaseComparisons = null;
         }
 
-
         // Only change from the defaults if the attribute has been set
         if ("true".equals(lowercaseComparisons)) {
             if (useRegex) {
@@ -1200,10 +1194,13 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
     /**
      * Parses the intercept-url elements and populates the FilterChainProxy's filter chain Map and the
      * map used to create the FilterInvocationDefintionSource for the FilterSecurityInterceptor.
+     * @return
      */
-    void parseInterceptUrlsForChannelSecurityAndEmptyFilterChains(List<Element> urlElts, Map<String, List<BeanMetadataElement>> filterChainMap,  Map<RequestKey, List<ConfigAttribute>> channelRequestMap,
+    LinkedHashMap<RequestKey, List<ConfigAttribute>> parseInterceptUrlsForChannelSecurity(List<Element> urlElts,
             boolean useLowerCasePaths, ParserContext parserContext) {
 
+        LinkedHashMap<RequestKey, List<ConfigAttribute>> channelRequestMap = new ManagedMap<RequestKey, List<ConfigAttribute>>();
+
         for (Element urlElt : urlElts) {
             String path = urlElt.getAttribute(ATT_PATH_PATTERN);
 
@@ -1233,6 +1230,25 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
                 channelRequestMap.put(new RequestKey(path),
                         SecurityConfig.createList((StringUtils.commaDelimitedListToStringArray(channelConfigAttribute))));
             }
+        }
+
+        return channelRequestMap;
+    }
+
+    private ManagedMap<String, List<BeanMetadataElement>> parseInterceptUrlsForEmptyFilterChains(List<Element> urlElts,
+            boolean useLowerCasePaths, ParserContext parserContext) {
+        ManagedMap<String, List<BeanMetadataElement>> filterChainMap = new ManagedMap<String, List<BeanMetadataElement>>();
+
+        for (Element urlElt : urlElts) {
+            String path = urlElt.getAttribute(ATT_PATH_PATTERN);
+
+            if(!StringUtils.hasText(path)) {
+                parserContext.getReaderContext().error("path attribute cannot be empty or null", urlElt);
+            }
+
+            if (useLowerCasePaths) {
+                path = path.toLowerCase();
+            }
 
             String filters = urlElt.getAttribute(ATT_FILTERS);
 
@@ -1246,62 +1262,20 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
                 filterChainMap.put(path, noFilters);
             }
         }
+
+        return filterChainMap;
     }
 
     /**
      * Parses the filter invocation map which will be used to configure the FilterInvocationSecurityMetadataSource
      * used in the security interceptor.
      */
-    static LinkedHashMap<RequestKey, List<ConfigAttribute>>
+    private static ManagedMap<BeanDefinition,BeanDefinition>
     parseInterceptUrlsForFilterInvocationRequestMap(List<Element> urlElts,  boolean useLowerCasePaths,
             boolean useExpressions, ParserContext parserContext) {
 
-        LinkedHashMap<RequestKey, List<ConfigAttribute>> filterInvocationDefinitionMap = new LinkedHashMap<RequestKey, List<ConfigAttribute>>();
-
-        for (Element urlElt : urlElts) {
-            String access = urlElt.getAttribute(ATT_ACCESS_CONFIG);
-            if (!StringUtils.hasText(access)) {
-                continue;
-            }
-
-            String path = urlElt.getAttribute(ATT_PATH_PATTERN);
-
-            if(!StringUtils.hasText(path)) {
-                parserContext.getReaderContext().error("path attribute cannot be empty or null", urlElt);
-            }
-
-            if (useLowerCasePaths) {
-                path = path.toLowerCase();
-            }
-
-            String method = urlElt.getAttribute(ATT_HTTP_METHOD);
-            if (!StringUtils.hasText(method)) {
-                method = null;
-            }
-
-            // Convert the comma-separated list of access attributes to a List<ConfigAttribute>
-
-            RequestKey key = new RequestKey(path, method);
-            List<ConfigAttribute> attributes = null;
-
-            if (useExpressions) {
-                logger.info("Creating access control expression attribute '" + access + "' for " + key);
-                attributes = new ArrayList<ConfigAttribute>(1);
-                // The expression will be parsed later by the ExpressionFilterInvocationSecurityMetadataSource
-                attributes.add(new SecurityConfig(access));
-
-            } else {
-                attributes = SecurityConfig.createList(StringUtils.commaDelimitedListToStringArray(access));
-            }
-
-            if (filterInvocationDefinitionMap.containsKey(key)) {
-                logger.warn("Duplicate URL defined: " + key + ". The original attribute values will be overwritten");
-            }
-
-            filterInvocationDefinitionMap.put(key, attributes);
-        }
+        return FilterInvocationSecurityMetadataSourceBeanDefinitionParser.parseInterceptUrlsForFilterInvocationRequestMap(urlElts, useLowerCasePaths, useExpressions, parserContext);
 
-        return filterInvocationDefinitionMap;
     }
 
 }

config/src/test/java/org/springframework/security/config/http/FilterSecurityMetadataSourceBeanDefinitionParserTests.java

@@ -58,6 +58,23 @@ public class FilterSecurityMetadataSourceBeanDefinitionParserTests {
         assertTrue(cad.contains(new SecurityConfig("ROLE_A")));
     }
 
+    // SEC-1201
+    @Test
+    public void interceptUrlsSupportPropertyPlaceholders() {
+        System.setProperty("secure.url", "/secure");
+        System.setProperty("secure.role", "ROLE_A");
+        setContext(
+                "<b:bean class='org.springframework.beans.factory.config.PropertyPlaceholderConfigurer'/>" +
+                "<filter-security-metadata-source id='fids'>" +
+                "   <intercept-url pattern='${secure.url}' access='${secure.role}'/>" +
+                "</filter-security-metadata-source>");
+        DefaultFilterInvocationSecurityMetadataSource fids = (DefaultFilterInvocationSecurityMetadataSource) appContext.getBean("fids");
+        List<ConfigAttribute> cad = fids.getAttributes(createFilterInvocation("/secure", "GET"));
+        assertNotNull(cad);
+        assertEquals(1, cad.size());
+        assertEquals("ROLE_A", cad.get(0).getAttribute());
+    }
+
     @Test
     public void parsingWithinFilterSecurityInterceptorIsSuccessful() {
         setContext(
@@ -72,11 +89,8 @@ public class FilterSecurityMetadataSourceBeanDefinitionParserTests {
                 "   </b:property>" +
                 "   <b:property name='authenticationManager' ref='" + BeanIds.AUTHENTICATION_MANAGER +"'/>"+
                 "</b:bean>" + ConfigTestUtils.AUTH_PROVIDER_XML);
-
-
     }
 
-
     private FilterInvocation createFilterInvocation(String path, String method) {
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setRequestURI(null);

config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java

@@ -149,7 +149,7 @@ public class HttpSecurityBeanDefinitionParserTests {
     }
 
     @Test
-    public void filterListShouldBeEmptyForUnprotectedUrl() throws Exception {
+    public void filterListShouldBeEmptyForPatternWithNoFilters() throws Exception {
         setContext(
                 "    <http auto-config='true'>" +
                 "        <intercept-url pattern='/unprotected' filters='none' />" +
@@ -160,6 +160,22 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertTrue(filters.size() == 0);
     }
 
+    @Test
+    public void filtersEqualsNoneSupportsPlaceholderForPattern() throws Exception {
+        System.setProperty("pattern.nofilters", "/unprotected");
+        setContext(
+                "    <b:bean class='org.springframework.beans.factory.config.PropertyPlaceholderConfigurer'/>" +
+                "    <http auto-config='true'>" +
+                "        <intercept-url pattern='${pattern.nofilters}' filters='none' />" +
+                "        <intercept-url pattern='/**' access='ROLE_A' />" +
+                "    </http>" + AUTH_PROVIDER_XML);
+
+        List<Filter> filters = getFilters("/unprotected");
+
+        assertTrue(filters.size() == 0);
+    }
+
+
     @Test
     public void regexPathsWorkCorrectly() throws Exception {
         setContext(
@@ -274,7 +290,7 @@ public class HttpSecurityBeanDefinitionParserTests {
         FilterSecurityInterceptor fis = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class);
 
         FilterInvocationSecurityMetadataSource fids = fis.getSecurityMetadataSource();
-        List<? extends ConfigAttribute> attrDef = fids.getAttributes(createFilterinvocation("/Secure", null));
+        List<ConfigAttribute> attrDef = fids.getAttributes(createFilterinvocation("/Secure", null));
         assertEquals(2, attrDef.size());
         assertTrue(attrDef.contains(new SecurityConfig("ROLE_A")));
         assertTrue(attrDef.contains(new SecurityConfig("ROLE_B")));
@@ -283,6 +299,40 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertTrue(attrDef.contains(new SecurityConfig("ROLE_C")));
     }
 
+    // SEC-1201
+    @Test
+    public void interceptUrlsAndFormLoginSupportPropertyPlaceholders() throws Exception {
+        System.setProperty("secure.url", "/secure");
+        System.setProperty("secure.role", "ROLE_A");
+        System.setProperty("login.page", "/loginPage");
+        System.setProperty("default.target", "/defaultTarget");
+        System.setProperty("auth.failure", "/authFailure");
+        setContext(
+                "<b:bean class='org.springframework.beans.factory.config.PropertyPlaceholderConfigurer'/>" +
+                "<http>" +
+                "    <intercept-url pattern='${secure.url}' access='${secure.role}' />" +
+                "    <form-login login-page='${login.page}' default-target-url='${default.target}' " +
+                "        authentication-failure-url='${auth.failure}' />" +
+                "</http>" + AUTH_PROVIDER_XML);
+
+        // Check the security attribute
+        FilterSecurityInterceptor fis = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class);
+        FilterInvocationSecurityMetadataSource fids = fis.getSecurityMetadataSource();
+        List<ConfigAttribute> attrs = fids.getAttributes(createFilterinvocation("/secure", null));
+        assertNotNull(attrs);
+        assertEquals(1, attrs.size());
+        assertEquals("ROLE_A",attrs.get(0).getAttribute());
+
+        // Check the form login properties are set
+        UsernamePasswordAuthenticationProcessingFilter apf = (UsernamePasswordAuthenticationProcessingFilter)
+                getFilter(UsernamePasswordAuthenticationProcessingFilter.class);
+        assertEquals("/defaultTarget", FieldUtils.getFieldValue(apf, "successHandler.defaultTargetUrl"));
+        assertEquals("/authFailure", FieldUtils.getFieldValue(apf, "failureHandler.defaultFailureUrl"));
+
+        ExceptionTranslationFilter etf = (ExceptionTranslationFilter) getFilter(ExceptionTranslationFilter.class);
+        assertEquals("/loginPage", FieldUtils.getFieldValue(etf, "authenticationEntryPoint.loginFormUrl"));
+    }
+
     @Test
     public void httpMethodMatchIsSupported() throws Exception {
         setContext(
@@ -338,6 +388,19 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertTrue(filters.get(0) instanceof ChannelProcessingFilter);
     }
 
+    @Test
+    public void requiresChannelSupportsPlaceholder() throws Exception {
+        setContext(
+                "    <http auto-config='true'>" +
+                "        <intercept-url pattern='/**' requires-channel='https' />" +
+                "    </http>" + AUTH_PROVIDER_XML);
+        List<Filter> filters = getFilters("/someurl");
+
+        assertEquals("Expected " + (AUTO_CONFIG_FILTERS + 1) +"  filters in chain", AUTO_CONFIG_FILTERS + 1, filters.size());
+
+        assertTrue(filters.get(0) instanceof ChannelProcessingFilter);
+    }    
+    
     @Test
     public void portMappingsAreParsedCorrectly() throws Exception {
         setContext(

core/src/main/java/org/springframework/security/access/SecurityConfig.java

@@ -19,6 +19,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 /**
  * Stores a {@link ConfigAttribute} as a <code>String</code>.
@@ -62,7 +63,11 @@ public class SecurityConfig implements ConfigAttribute {
         return this.attrib;
     }
 
-    public static List<ConfigAttribute> createList(String... attributeNames) {
+    public final static List<ConfigAttribute> createListFromCommaDelimitedString(String access) {
+        return createList(StringUtils.commaDelimitedListToStringArray(access));
+    }
+
+    public final static List<ConfigAttribute> createList(String... attributeNames) {
         Assert.notNull(attributeNames, "You must supply a list of argument names");
         List<ConfigAttribute> attributes = new ArrayList<ConfigAttribute>(attributeNames.length);