Extract a SecurityFilterChain interface and create a default implementation to facilitate other configuration options.

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

@@ -9,6 +9,7 @@ import org.apache.commons.logging.LogFactory;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.authentication.AnonymousAuthenticationToken;
+import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.FilterInvocation;
 import org.springframework.security.web.SecurityFilterChain;
@@ -24,6 +25,7 @@ import org.springframework.security.web.context.SecurityContextPersistenceFilter
 import org.springframework.security.web.jaasapi.JaasApiIntegrationFilter;
 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 final Log logger = LogFactory.getLog(getClass());
@@ -34,17 +36,34 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain
             checkFilterStack(filterChain.getFilters());
         }
 
+        checkPathOrder(new ArrayList<SecurityFilterChain>(fcp.getFilterChains()));
         checkForDuplicateMatchers(new ArrayList<SecurityFilterChain>(fcp.getFilterChains()));
     }
 
+    private void checkPathOrder(List<SecurityFilterChain> filterChains) {
+        // Check that the universal pattern is listed at the end, if at all
+        Iterator<SecurityFilterChain> chains = filterChains.iterator();
+
+        while (chains.hasNext()) {
+            if (((DefaultSecurityFilterChain)chains.next()).getRequestMatcher() instanceof AnyRequestMatcher && chains.hasNext()) {
+                throw new IllegalArgumentException("A universal match pattern ('/**') is defined " +
+                        " before other patterns in the filter chain, causing them to be ignored. Please check the " +
+                        "ordering in your <security:http> namespace or FilterChainProxy bean configuration");
+            }
+        }
+    }
+
     private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
-        SecurityFilterChain chain = chains.remove(0);
 
-        for (SecurityFilterChain test : chains) {
-            if (chain.getRequestMatcher().equals(test.getRequestMatcher())) {
-                throw new IllegalArgumentException("The FilterChainProxy contains two filter chains using the" +
-                        " matcher " + chain.getRequestMatcher() + ". If you are using multiple <http> namespace " +
-                        "elements, you must use a 'pattern' attribute to define the request patterns to which they apply.");
+        while (chains.size() > 1) {
+            DefaultSecurityFilterChain chain = (DefaultSecurityFilterChain)chains.remove(0);
+
+            for (SecurityFilterChain test : chains) {
+                if (chain.getRequestMatcher().equals(((DefaultSecurityFilterChain)test).getRequestMatcher())) {
+                    throw new IllegalArgumentException("The FilterChainProxy contains two filter chains using the" +
+                            " matcher " + chain.getRequestMatcher() + ". If you are using multiple <http> namespace " +
+                            "elements, you must use a 'pattern' attribute to define the request patterns to which they apply.");
+                }
             }
         }
     }

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

@@ -7,6 +7,7 @@ import org.springframework.beans.factory.support.ManagedList;
 import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
 import org.springframework.beans.factory.xml.BeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
+import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -26,7 +27,7 @@ public class FilterChainBeanDefinitionParser implements BeanDefinitionParser {
         String requestMatcher = elt.getAttribute(ATT_REQUEST_MATCHER_REF);
         String filters = elt.getAttribute(HttpSecurityBeanDefinitionParser.ATT_FILTERS);
 
-        BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(SecurityFilterChain.class);
+        BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(DefaultSecurityFilterChain.class);
 
         if (StringUtils.hasText(path)) {
             Assert.isTrue(!StringUtils.hasText(requestMatcher), "");

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

@@ -21,6 +21,7 @@ import org.springframework.security.authentication.ProviderManager;
 import org.springframework.security.config.BeanIds;
 import org.springframework.security.config.Elements;
 import org.springframework.security.config.authentication.AuthenticationManagerFactoryBean;
+import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.util.AnyRequestMatcher;
@@ -147,7 +148,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
             filterChainMatcher = new RootBeanDefinition(AnyRequestMatcher.class);
         }
 
-        BeanDefinitionBuilder filterChainBldr = BeanDefinitionBuilder.rootBeanDefinition(SecurityFilterChain.class);
+        BeanDefinitionBuilder filterChainBldr = BeanDefinitionBuilder.rootBeanDefinition(DefaultSecurityFilterChain.class);
         filterChainBldr.addConstructorArgValue(filterChainMatcher);
         filterChainBldr.addConstructorArgValue(filterChain);
 

config/src/test/groovy/org/springframework/security/config/http/MultiHttpBlockConfigTests.groovy

@@ -42,7 +42,7 @@ class MultiHttpBlockConfigTests extends AbstractHttpConfigTests {
         createAppContext()
         then:
         BeanCreationException e = thrown()
-        e.cause.cause instanceof IllegalArgumentException
+        e.cause instanceof IllegalArgumentException
     }
 
   def duplicatePatternsAreRejected () {

config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java

@@ -33,6 +33,7 @@ import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
@@ -67,11 +68,6 @@ public class FilterChainProxyConfigTests {
         }
     }
 
-    @Test(expected=BeanCreationException.class)
-    public void misplacedUniversalPathShouldBeDetected() throws Exception {
-        appCtx.getBean("newFilterChainProxyWrongPathOrder", FilterChainProxy.class);
-    }
-
     @Test
     public void normalOperation() throws Exception {
         FilterChainProxy filterChainProxy = appCtx.getBean("filterChain", FilterChainProxy.class);
@@ -111,9 +107,13 @@ public class FilterChainProxyConfigTests {
         FilterChainProxy fcp = appCtx.getBean("sec1235FilterChainProxy", FilterChainProxy.class);
 
         List<SecurityFilterChain> chains = fcp.getFilterChains();
-        assertEquals("/login*", ((AntPathRequestMatcher)chains.get(0).getRequestMatcher()).getPattern());
-        assertEquals("/logout", ((AntPathRequestMatcher)chains.get(1).getRequestMatcher()).getPattern());
-        assertTrue(chains.get(2).getRequestMatcher() instanceof AnyRequestMatcher);
+        assertEquals("/login*", getPattern(chains.get(0)));
+        assertEquals("/logout", getPattern(chains.get(1)));
+        assertTrue(((DefaultSecurityFilterChain)chains.get(2)).getRequestMatcher() instanceof AnyRequestMatcher);
+    }
+
+    private String getPattern(SecurityFilterChain chain) {
+        return ((AntPathRequestMatcher)((DefaultSecurityFilterChain)chain).getRequestMatcher()).getPattern();
     }
 
     private void checkPathAndFilterOrder(FilterChainProxy filterChainProxy) throws Exception {

config/src/test/resources/org/springframework/security/util/filtertest-valid.xml

@@ -98,8 +98,11 @@
             <sec:filter-chain pattern="/some/other/path/**" filters="sif,mockFilter,mockFilter2"/>
            </util:list>
         </constructor-arg>
+        <property name="filterChainValidator" ref="fcv" />
     </bean>
 
+    <bean id="fcv" class="org.springframework.security.config.http.DefaultFilterChainValidator" />
+
     <bean id="newFilterChainProxyRegex" class="org.springframework.security.web.FilterChainProxy">
         <sec:filter-chain-map path-type="regex">
             <sec:filter-chain pattern="\A/foo/.*\Z" filters="mockFilter"/>
@@ -124,7 +127,7 @@
     <bean id="newFilterChainProxyNonNamespace" class="org.springframework.security.web.FilterChainProxy">
         <constructor-arg>
             <list>
-                <bean class="org.springframework.security.web.SecurityFilterChain">
+                <bean class="org.springframework.security.web.DefaultSecurityFilterChain">
                     <constructor-arg ref="fooMatcher"/>
                     <constructor-arg>
                         <list>
@@ -132,7 +135,7 @@
                         </list>
                     </constructor-arg>
                 </bean>
-                <bean class="org.springframework.security.web.SecurityFilterChain">
+                <bean class="org.springframework.security.web.DefaultSecurityFilterChain">
                     <constructor-arg>
                         <bean class="org.springframework.security.web.util.AntPathRequestMatcher">
                             <constructor-arg value="/some/other/path/**"/>
@@ -146,7 +149,7 @@
                         </list>
                     </constructor-arg>
                 </bean>
-                <bean class="org.springframework.security.web.SecurityFilterChain">
+                <bean class="org.springframework.security.web.DefaultSecurityFilterChain">
                     <constructor-arg>
                         <bean class="org.springframework.security.web.util.AntPathRequestMatcher">
                             <constructor-arg value="/do/not/filter*"/>
@@ -156,7 +159,7 @@
                         <list />
                     </constructor-arg>
                 </bean>
-                <bean class="org.springframework.security.web.SecurityFilterChain">
+                <bean class="org.springframework.security.web.DefaultSecurityFilterChain">
                     <constructor-arg>
                         <bean class="org.springframework.security.web.util.AntPathRequestMatcher">
                             <constructor-arg value="/**"/>

web/src/main/java/org/springframework/security/web/DefaultSecurityFilterChain.java

@@ -0,0 +1,45 @@
+package org.springframework.security.web;
+
+import org.springframework.security.web.util.RequestMatcher;
+
+import javax.servlet.Filter;
+import javax.servlet.http.HttpServletRequest;
+import java.util.*;
+
+/**
+ * Standard implementation of {@code SecurityFilterChain}.
+ *
+ * @author Luke Taylor
+ *
+ * @since 3.1
+ */
+public final class DefaultSecurityFilterChain implements SecurityFilterChain {
+    private final RequestMatcher requestMatcher;
+    private final List<Filter> filters;
+
+    public DefaultSecurityFilterChain(RequestMatcher requestMatcher, Filter... filters) {
+        this(requestMatcher, Arrays.asList(filters));
+    }
+
+    public DefaultSecurityFilterChain(RequestMatcher requestMatcher, List<Filter> filters) {
+        this.requestMatcher = requestMatcher;
+        this.filters = new ArrayList<Filter>(filters);
+    }
+
+    public RequestMatcher getRequestMatcher() {
+        return requestMatcher;
+    }
+
+    public List<Filter> getFilters() {
+        return filters;
+    }
+
+    public boolean matches(HttpServletRequest request) {
+        return requestMatcher.matches(request);
+    }
+
+    @Override
+    public String toString() {
+        return "[ " + requestMatcher + ", " + filters + "]";
+    }
+}

web/src/main/java/org/springframework/security/web/FilterChainProxy.java

@@ -142,7 +142,6 @@ public class FilterChainProxy extends GenericFilterBean {
 
     public FilterChainProxy(List<SecurityFilterChain> filterChains) {
         this.filterChains = filterChains;
-        checkPathOrder();
     }
 
     @Override
@@ -219,10 +218,8 @@ public class FilterChainProxy extends GenericFilterBean {
         filterChains = new ArrayList<SecurityFilterChain>(filterChainMap.size());
 
         for (Map.Entry<RequestMatcher,List<Filter>> entry : filterChainMap.entrySet()) {
-            filterChains.add(new SecurityFilterChain(entry.getKey(), entry.getValue()));
+            filterChains.add(new DefaultSecurityFilterChain(entry.getKey(), entry.getValue()));
         }
-
-        checkPathOrder();
     }
 
     /**
@@ -238,25 +235,12 @@ public class FilterChainProxy extends GenericFilterBean {
         LinkedHashMap<RequestMatcher, List<Filter>> map =  new LinkedHashMap<RequestMatcher, List<Filter>>();
 
         for (SecurityFilterChain chain : filterChains) {
-            map.put(chain.getRequestMatcher(), chain.getFilters());
+            map.put(((DefaultSecurityFilterChain)chain).getRequestMatcher(), chain.getFilters());
         }
 
         return map;
     }
 
-    private void checkPathOrder() {
-        // Check that the universal pattern is listed at the end, if at all
-        Iterator<SecurityFilterChain> chains = filterChains.iterator();
-
-        while(chains.hasNext()) {
-            if ((chains.next().getRequestMatcher() instanceof AnyRequestMatcher && chains.hasNext())) {
-                throw new IllegalArgumentException("A universal match pattern ('/**') is defined " +
-                        " before other patterns in the filter chain, causing them to be ignored. Please check the " +
-                        "ordering in your <security:http> namespace or FilterChainProxy bean configuration");
-            }
-        }
-    }
-
     /**
      * @return the list of {@code SecurityFilterChain}s which will be matched against and
      *         applied to incoming requests.

web/src/main/java/org/springframework/security/web/SecurityFilterChain.java

@@ -1,54 +1,23 @@
 package org.springframework.security.web;
 
-import org.springframework.security.web.util.RequestMatcher;
-
 import javax.servlet.Filter;
-import javax.servlet.FilterChain;
-import javax.servlet.FilterConfig;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
-import java.io.IOException;
 import java.util.*;
 
 /**
- * Bean which defines a filter chain which is capable of being matched against an {@code HttpServletRequest}.
+ * Defines a filter chain which is capable of being matched against an {@code HttpServletRequest}.
  * in order to decide whether it applies to that request.
  * <p>
  * Used to configure a {@code FilterChainProxy}.
  *
+ *
  * @author Luke Taylor
  *
  * @since 3.1
  */
-public final class SecurityFilterChain {
-    private final RequestMatcher requestMatcher;
-    private final List<Filter> filters;
-
-    public SecurityFilterChain(RequestMatcher requestMatcher, Filter... filters) {
-        this(requestMatcher, Arrays.asList(filters));
-    }
-
-    public SecurityFilterChain(RequestMatcher requestMatcher, List<Filter> filters) {
-        this.requestMatcher = requestMatcher;
-        this.filters = new ArrayList<Filter>(filters);
-    }
-
-    public RequestMatcher getRequestMatcher() {
-        return requestMatcher;
-    }
-
-    public List<Filter> getFilters() {
-        return filters;
-    }
+public interface SecurityFilterChain {
 
-    public boolean matches(HttpServletRequest request) {
-        return requestMatcher.matches(request);
-    }
+    boolean matches(HttpServletRequest request);
 
-    @Override
-    public String toString() {
-        return "[ " + requestMatcher + ", " + filters + "]";
-    }
+    List<Filter> getFilters();
 }

web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java

@@ -47,7 +47,7 @@ public class FilterChainProxyTests {
                         return null;
                     }
                 }).when(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
-        fcp = new FilterChainProxy(new SecurityFilterChain(matcher, Arrays.asList(filter)));
+        fcp = new FilterChainProxy(new DefaultSecurityFilterChain(matcher, Arrays.asList(filter)));
         fcp.setFilterChainValidator(mock(FilterChainProxy.FilterChainValidator.class));
         request = new MockHttpServletRequest();
         request.setServletPath("/path");
@@ -94,7 +94,7 @@ public class FilterChainProxyTests {
     @Test
     public void originalFilterChainIsInvokedIfMatchingSecurityChainIsEmpty() throws Exception {
         List<Filter> noFilters = Collections.emptyList();
-        fcp = new FilterChainProxy(new SecurityFilterChain(matcher, noFilters));
+        fcp = new FilterChainProxy(new DefaultSecurityFilterChain(matcher, noFilters));
 
         when(matcher.matches(any(HttpServletRequest.class))).thenReturn(true);
         fcp.doFilter(request, response, chain);
@@ -137,7 +137,7 @@ public class FilterChainProxyTests {
     @Test
     public void bothWrappersAreResetWithNestedFcps() throws Exception {
         HttpFirewall fw = mock(HttpFirewall.class);
-        FilterChainProxy firstFcp = new FilterChainProxy(new SecurityFilterChain(matcher, fcp));
+        FilterChainProxy firstFcp = new FilterChainProxy(new DefaultSecurityFilterChain(matcher, fcp));
         firstFcp.setFirewall(fw);
         fcp.setFirewall(fw);
         FirewalledRequest firstFwr = mock(FirewalledRequest.class, "firstFwr");