SEC-1407: Use RequestMatcher instances as the FilterInvocationSecurit…

…yMetadataSource keys and in the FilterChainMap use by FilterChainProxy.

This greatly simplifies the code and opens up possibilities for other matching strategies (e.g. EL). This also means that matching is now completely strict - the order of the matchers is all that matters (not whether an HTTP method is included or not). The first matcher that returns true will be used.

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

@@ -2,7 +2,6 @@
 
 import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 
 import javax.servlet.Filter;
 
@@ -11,6 +10,7 @@
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.authentication.AnonymousAuthenticationToken;
 import org.springframework.security.web.FilterChainProxy;
+import org.springframework.security.web.FilterInvocation;
 import org.springframework.security.web.access.ExceptionTranslationFilter;
 import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
@@ -22,18 +22,17 @@
 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) {
-        Map<String, List<Filter>> filterChainMap = fcp.getFilterChainMap();
-        for(String pattern : fcp.getFilterChainMap().keySet()) {
-            List<Filter> filters = filterChainMap.get(pattern);
+        for(List<Filter> filters : fcp.getFilterChainMap().values()) {
             checkFilterStack(filters);
         }
 
-        checkLoginPageIsntProtected(fcp, filterChainMap.get(fcp.getMatcher().getUniversalMatchPattern()));
+        checkLoginPageIsntProtected(fcp);
     }
 
     private Object getFilter(Class<?> type, List<Filter> filters) {
@@ -78,12 +77,14 @@ private void checkForDuplicates(Class<? extends Filter> clazz, List<Filter> filt
     }
 
     /* Checks for the common error of having a login page URL protected by the security interceptor */
-    private void checkLoginPageIsntProtected(FilterChainProxy fcp, List<Filter> defaultFilters) {
+    private void checkLoginPageIsntProtected(FilterChainProxy fcp) {
+        List<Filter> defaultFilters = fcp.getFilterChainMap().get(new AnyRequestMatcher());
         ExceptionTranslationFilter etf = (ExceptionTranslationFilter)getFilter(ExceptionTranslationFilter.class, defaultFilters);
 
         if (etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint) {
             String loginPage =
                 ((LoginUrlAuthenticationEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl();
+            FilterInvocation loginRequest = new FilterInvocation(loginPage, "POST");
             List<Filter> filters = fcp.getFilters(loginPage);
             logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration");
 
@@ -100,7 +101,8 @@ private void checkLoginPageIsntProtected(FilterChainProxy fcp, List<Filter> defa
             FilterSecurityInterceptor fsi = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class, filters);
             DefaultFilterInvocationSecurityMetadataSource fids =
                     (DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource();
-            Collection<ConfigAttribute> attributes = fids.lookupAttributes(loginPage, "POST");
+
+            Collection<ConfigAttribute> attributes = fids.getAttributes(loginRequest);
 
             if (attributes == null) {
                 logger.debug("No access attributes defined for login page URL");
@@ -122,7 +124,7 @@ private void checkLoginPageIsntProtected(FilterChainProxy fcp, List<Filter> defa
             AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(),
                             anonPF.getUserAttribute().getAuthorities());
             try {
-                fsi.getAccessDecisionManager().decide(token, new Object(), fids.lookupAttributes(loginPage, "POST"));
+                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 " +

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

@@ -1,5 +1,10 @@
 package org.springframework.security.config.http;
 
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.BeanDefinitionHolder;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
@@ -8,14 +13,11 @@
 import org.springframework.beans.factory.xml.BeanDefinitionDecorator;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.security.config.Elements;
-import org.springframework.security.web.util.RegexUrlPathMatcher;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
-import java.util.*;
-
 /**
  * Sets the filter chain Map for a FilterChainProxy bean declaration.
  *
@@ -30,11 +32,7 @@ public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder holder, Par
         Map filterChainMap = new LinkedHashMap();
         Element elt = (Element)node;
 
-        String pathType = elt.getAttribute(HttpSecurityBeanDefinitionParser.ATT_PATH_TYPE);
-
-        if (HttpSecurityBeanDefinitionParser.OPT_PATH_TYPE_REGEX.equals(pathType)) {
-            filterChainProxy.getPropertyValues().addPropertyValue("matcher", new RegexUrlPathMatcher());
-        }
+        MatcherType matcherType = MatcherType.fromElement(elt);
 
         List<Element> filterChainElts = DomUtils.getChildElementsByTagName(elt, Elements.FILTER_CHAIN);
 
@@ -52,8 +50,10 @@ public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder holder, Par
                     "'must not be empty", elt);
             }
 
+            BeanDefinition matcher = matcherType.createMatcher(path, null);
+
             if (filters.equals(HttpSecurityBeanDefinitionParser.OPT_FILTERS_NONE)) {
-                filterChainMap.put(path, Collections.EMPTY_LIST);
+                filterChainMap.put(matcher, Collections.EMPTY_LIST);
             } else {
                 String[] filterBeanNames = StringUtils.tokenizeToStringArray(filters, ",");
                 ManagedList filterChain = new ManagedList(filterBeanNames.length);
@@ -62,7 +62,7 @@ public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder holder, Par
                     filterChain.add(new RuntimeBeanReference(filterBeanNames[i]));
                 }
 
-                filterChainMap.put(path, filterChain);
+                filterChainMap.put(matcher, filterChain);
             }
         }
 

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

@@ -17,9 +17,6 @@
 import org.springframework.security.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
-import org.springframework.security.web.access.intercept.RequestKey;
-import org.springframework.security.web.util.AntUrlPathMatcher;
-import org.springframework.security.web.util.UrlMatcher;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
@@ -63,11 +60,11 @@ public BeanDefinition parse(Element element, ParserContext parserContext) {
     }
 
     static BeanDefinition createSecurityMetadataSource(List<Element> interceptUrls, Element elt, ParserContext pc) {
-        UrlMatcher matcher = HttpSecurityBeanDefinitionParser.createUrlMatcher(elt);
+        MatcherType matcherType = MatcherType.fromElement(elt);
         boolean useExpressions = isUseExpressions(elt);
 
         ManagedMap<BeanDefinition, BeanDefinition> requestToAttributesMap = parseInterceptUrlsForFilterInvocationRequestMap(
-                interceptUrls, useExpressions, pc);
+                matcherType, interceptUrls, useExpressions, pc);
         BeanDefinitionBuilder fidsBuilder;
 
         if (useExpressions) {
@@ -83,16 +80,14 @@ static BeanDefinition createSecurityMetadataSource(List<Element> interceptUrls,
             }
 
             fidsBuilder = BeanDefinitionBuilder.rootBeanDefinition(ExpressionBasedFilterInvocationSecurityMetadataSource.class);
-            fidsBuilder.addConstructorArgValue(matcher);
             fidsBuilder.addConstructorArgValue(requestToAttributesMap);
             fidsBuilder.addConstructorArgReference(expressionHandlerRef);
         } else {
             fidsBuilder = BeanDefinitionBuilder.rootBeanDefinition(DefaultFilterInvocationSecurityMetadataSource.class);
-            fidsBuilder.addConstructorArgValue(matcher);
             fidsBuilder.addConstructorArgValue(requestToAttributesMap);
         }
 
-        fidsBuilder.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher);
+//        fidsBuilder.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher);
         fidsBuilder.getRawBeanDefinition().setSource(pc.extractSource(elt));
 
         return fidsBuilder.getBeanDefinition();
@@ -102,8 +97,9 @@ static boolean isUseExpressions(Element elt) {
         return "true".equals(elt.getAttribute(ATT_USE_EXPRESSIONS));
     }
 
-    private static ManagedMap<BeanDefinition, BeanDefinition> parseInterceptUrlsForFilterInvocationRequestMap(List<Element> urlElts,
-            boolean useExpressions, ParserContext parserContext) {
+    private static ManagedMap<BeanDefinition, BeanDefinition>
+        parseInterceptUrlsForFilterInvocationRequestMap(MatcherType matcherType,
+                List<Element> urlElts, boolean useExpressions, ParserContext parserContext) {
 
         ManagedMap<BeanDefinition, BeanDefinition> filterInvocationDefinitionMap = new ManagedMap<BeanDefinition, BeanDefinition>();
 
@@ -124,10 +120,7 @@ private static ManagedMap<BeanDefinition, BeanDefinition> parseInterceptUrlsForF
                 method = null;
             }
 
-            BeanDefinitionBuilder keyBldr = BeanDefinitionBuilder.rootBeanDefinition(RequestKey.class);
-            keyBldr.addConstructorArgValue(path);
-            keyBldr.addConstructorArgValue(method);
-
+            BeanDefinition matcher = matcherType.createMatcher(path, method);
             BeanDefinitionBuilder attributeBuilder = BeanDefinitionBuilder.rootBeanDefinition(SecurityConfig.class);
             attributeBuilder.addConstructorArgValue(access);
 
@@ -140,13 +133,11 @@ private static ManagedMap<BeanDefinition, BeanDefinition> parseInterceptUrlsForF
                 attributeBuilder.setFactoryMethod("createListFromCommaDelimitedString");
             }
 
-            BeanDefinition key = keyBldr.getBeanDefinition();
-
-            if (filterInvocationDefinitionMap.containsKey(key)) {
+            if (filterInvocationDefinitionMap.containsKey(matcher)) {
                 logger.warn("Duplicate URL defined: " + path + ". The original attribute values will be overwritten");
             }
 
-            filterInvocationDefinitionMap.put(key, attributeBuilder.getBeanDefinition());
+            filterInvocationDefinitionMap.put(matcher, attributeBuilder.getBeanDefinition());
         }
 
         return filterInvocationDefinitionMap;

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

@@ -35,7 +35,6 @@
 import org.springframework.security.web.access.expression.WebExpressionVoter;
 import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
-import org.springframework.security.web.access.intercept.RequestKey;
 import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler;
 import org.springframework.security.web.authentication.session.ConcurrentSessionControlStrategy;
 import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy;
@@ -48,8 +47,6 @@
 import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
 import org.springframework.security.web.session.ConcurrentSessionFilter;
 import org.springframework.security.web.session.SessionManagementFilter;
-import org.springframework.security.web.util.AntUrlPathMatcher;
-import org.springframework.security.web.util.UrlMatcher;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
@@ -81,10 +78,9 @@ class HttpConfigurationBuilder {
 
     private final Element httpElt;
     private final ParserContext pc;
-    private final UrlMatcher matcher;
-    private final Boolean convertPathsToLowerCase;
     private final SessionCreationPolicy sessionPolicy;
     private final List<Element> interceptUrls;
+    private final MatcherType matcherType;
 
     // Use ManagedMap to allow placeholder resolution
     private ManagedMap<BeanDefinition, List<BeanMetadataElement>> filterChainMap;
@@ -102,15 +98,12 @@ class HttpConfigurationBuilder {
     private BeanReference fsi;
     private BeanReference requestCache;
 
-    public HttpConfigurationBuilder(Element element, ParserContext pc, UrlMatcher matcher,
+    public HttpConfigurationBuilder(Element element, ParserContext pc, MatcherType matcherType,
             String portMapperName, BeanReference authenticationManager) {
         this.httpElt = element;
         this.pc = pc;
         this.portMapperName = portMapperName;
-        this.matcher = matcher;
-        // SEC-501 - should paths stored in request maps be converted to lower case
-        // true if Ant path and using lower case
-        convertPathsToLowerCase = (matcher instanceof AntUrlPathMatcher) && matcher.requiresLowerCaseUrl();
+        this.matcherType = matcherType;
         interceptUrls = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL);
         String createSession = element.getAttribute(ATT_CREATE_SESSION);
 
@@ -139,10 +132,7 @@ private void parseInterceptUrlsForEmptyFilterChains() {
                 pc.getReaderContext().error("path attribute cannot be empty or null", urlElt);
             }
 
-            BeanDefinitionBuilder pathBean = BeanDefinitionBuilder.rootBeanDefinition(HttpConfigurationBuilder.class);
-            pathBean.setFactoryMethod("createPath");
-            pathBean.addConstructorArgValue(path);
-            pathBean.addConstructorArgValue(convertPathsToLowerCase);
+            BeanDefinition matcher = matcherType.createMatcher(path, null);
 
             String filters = urlElt.getAttribute(ATT_FILTERS);
 
@@ -153,7 +143,7 @@ private void parseInterceptUrlsForEmptyFilterChains() {
                 }
 
                 List<BeanMetadataElement> noFilters = Collections.emptyList();
-                filterChainMap.put(pathBean.getBeanDefinition(), noFilters);
+                filterChainMap.put(matcher, noFilters);
             }
         }
     }
@@ -378,9 +368,8 @@ private void createChannelProcessingFilter() {
 
         RootBeanDefinition channelFilter = new RootBeanDefinition(ChannelProcessingFilter.class);
         BeanDefinitionBuilder metadataSourceBldr = BeanDefinitionBuilder.rootBeanDefinition(DefaultFilterInvocationSecurityMetadataSource.class);
-        metadataSourceBldr.addConstructorArgValue(matcher);
         metadataSourceBldr.addConstructorArgValue(channelRequestMap);
-        metadataSourceBldr.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher);
+//        metadataSourceBldr.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher);
 
         channelFilter.getPropertyValues().addPropertyValue("securityMetadataSource", metadataSourceBldr.getBeanDefinition());
         RootBeanDefinition channelDecisionManager = new RootBeanDefinition(ChannelDecisionManagerImpl.class);
@@ -413,26 +402,22 @@ private ManagedMap<BeanDefinition,BeanDefinition> parseInterceptUrlsForChannelSe
 
         for (Element urlElt : interceptUrls) {
             String path = urlElt.getAttribute(ATT_PATH_PATTERN);
+            String method = urlElt.getAttribute(ATT_HTTP_METHOD);
 
             if(!StringUtils.hasText(path)) {
-                pc.getReaderContext().error("path attribute cannot be empty or null", urlElt);
-            }
-
-            if (convertPathsToLowerCase) {
-                path = path.toLowerCase();
+                pc.getReaderContext().error("pattern attribute cannot be empty or null", urlElt);
             }
 
             String requiredChannel = urlElt.getAttribute(ATT_REQUIRES_CHANNEL);
 
             if (StringUtils.hasText(requiredChannel)) {
-                BeanDefinition requestKey = new RootBeanDefinition(RequestKey.class);
-                requestKey.getConstructorArgumentValues().addGenericArgumentValue(path);
+                BeanDefinition matcher = matcherType.createMatcher(path, method);
 
                 RootBeanDefinition channelAttributes = new RootBeanDefinition(ChannelAttributeFactory.class);
                 channelAttributes.getConstructorArgumentValues().addGenericArgumentValue(requiredChannel);
                 channelAttributes.setFactoryMethodName("createChannelAttributes");
 
-                channelRequestMap.put(requestKey, channelAttributes);
+                channelRequestMap.put(matcher, channelAttributes);
             }
         }