From 37d0454fd71ba782e4083a29d468f753f5fe4646 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Sat, 23 Apr 2011 21:37:57 +0100 Subject: [PATCH] SEC-1657: Create SecurityFilterChain class for use in configuring FilterChinProxy. Encapsulates a RequestMatcher and List. --- .../security/web/FilterChainProxy.java | 106 ++++++++++-------- .../security/web/SecurityFilterChain.java | 54 +++++++++ .../security/web/FilterChainProxyTests.java | 32 +++--- 3 files changed, 132 insertions(+), 60 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/SecurityFilterChain.java diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java index 85b784d75f..1d07c49460 100644 --- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java @@ -23,7 +23,6 @@ import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.util.AnyRequestMatcher; import org.springframework.security.web.util.RequestMatcher; import org.springframework.security.web.util.UrlUtils; -import org.springframework.util.Assert; import org.springframework.web.filter.DelegatingFilterProxy; import org.springframework.web.filter.GenericFilterBean; @@ -49,21 +48,21 @@ import java.util.*; * *

Configuration

*

- * As of version 3.1, {@code FilterChainProxy} is configured using an ordered Map of {@link RequestMatcher} instances - * to {@code List}s of {@code Filter}s. The Map instance will normally be created while parsing the namespace - * configuration, so doesn't have to be set explicitly. Instead the {@code <filter-chain-map>} - * element should be used within the bean declaration. - * This in turn should have a list of child {@code <filter-chain>} elements which each define a URI pattern and - * the list of filters (as comma-separated bean names) which should be applied to requests which match the pattern. - * The default pattern matching strategy is to use {@link org.springframework.security.web.util.AntPathRequestMatcher - * Ant-style paths}. An example configuration might look like this: + * As of version 3.1, {@code FilterChainProxy} is configured using a list of {@link SecurityFilterChain} instances, + * each of which contains a {@link RequestMatcher} and a list of filters which should be applied to matching requests. + * Most applications will only contain a single filter chain, and if you are using the namespace, you don't have to + * set the chains explicitly. If you require finer-grained control, you can make use of the {@code <filter-chain>} + * namespace element. This defines a URI pattern and the list of filters (as comma-separated bean names) which should be + * applied to requests which match the pattern. An example configuration might look like this: * * 

- <bean id="myfilterChainProxy" class="org.springframework.security.util.FilterChjainProxy">
-     <security:filter-chain-map request-matcher="ant">
-         <security:filter-chain pattern="/do/not/filter*" filters="none"/>
-         <security:filter-chain pattern="/**" filters="filter1,filter2,filter3"/>
-     </security:filter-chain-map>
+ <bean id="myfilterChainProxy" class="org.springframework.security.util.FilterChainProxy">
+     <constructor-arg>
+         <util:list>
+             <security:filter-chain pattern="/do/not/filter*" filters="none"/>
+             <security:filter-chain pattern="/**" filters="filter1,filter2,filter3"/>
+         </util:list>
+     </constructor-arg>
  </bean>
  *
* @@ -126,7 +125,7 @@ public class FilterChainProxy extends GenericFilterBean { //~ Instance fields ================================================================================================ - private Map> filterChainMap; + private List filterChains; private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); @@ -134,9 +133,20 @@ public class FilterChainProxy extends GenericFilterBean { //~ Methods ======================================================================================================== + public FilterChainProxy() { + } + + public FilterChainProxy(SecurityFilterChain chain) { + this(Arrays.asList(chain)); + } + + public FilterChainProxy(List filterChains) { + this.filterChains = filterChains; + checkPathOrder(); + } + @Override public void afterPropertiesSet() { - Assert.notNull(filterChainMap, "filterChainMap must be set"); filterChainValidator.validate(this); } @@ -172,11 +182,9 @@ public class FilterChainProxy extends GenericFilterBean { * @return an ordered array of Filters defining the filter chain */ private List getFilters(HttpServletRequest request) { - for (Map.Entry> entry : filterChainMap.entrySet()) { - RequestMatcher matcher = entry.getKey(); - - if (matcher.matches(request)) { - return entry.getValue(); + for (SecurityFilterChain chain : filterChains) { + if (chain.matches(request)) { + return chain.getFilters(); } } @@ -204,34 +212,44 @@ public class FilterChainProxy extends GenericFilterBean { * example. * * @param filterChainMap the map of path Strings to {@code List<Filter>}s. + * @deprecated Use the constructor which takes a {@code List<SecurityFilterChain>} instead. */ - @SuppressWarnings("unchecked") - public void setFilterChainMap(Map filterChainMap) { - checkContents(filterChainMap); - this.filterChainMap = new LinkedHashMap>(filterChainMap); + @Deprecated + public void setFilterChainMap(Map> filterChainMap) { + filterChains = new ArrayList(filterChainMap.size()); + + for (Map.Entry> entry : filterChainMap.entrySet()) { + filterChains.add(new SecurityFilterChain(entry.getKey(), entry.getValue())); + } + checkPathOrder(); } - @SuppressWarnings("unchecked") - private void checkContents(Map filterChainMap) { - for (Object key : filterChainMap.keySet()) { - Assert.isInstanceOf(RequestMatcher.class, key, "Path key must be a RequestMatcher but found " + key); - Object filters = filterChainMap.get(key); - Assert.isInstanceOf(List.class, filters, "Value must be a filter list"); - // Check the contents + /** + * Returns a copy of the underlying filter chain map. Modifications to the map contents + * will not affect the FilterChainProxy state. + * + * @return the map of path pattern Strings to filter chain lists (with ordering guaranteed). + * + * @deprecated use the list of {@link SecurityFilterChain}s instead + */ + @Deprecated + public Map> getFilterChainMap() { + LinkedHashMap> map = new LinkedHashMap>(); - for (Object filter : ((List) filters)) { - Assert.isInstanceOf(Filter.class, filter, "Objects in filter chain must be of type Filter. "); - } + for (SecurityFilterChain chain : filterChains) { + map.put(chain.getRequestMatcher(), chain.getFilters()); } + + return map; } private void checkPathOrder() { // Check that the universal pattern is listed at the end, if at all - Iterator matchers = filterChainMap.keySet().iterator(); + Iterator chains = filterChains.iterator(); - while(matchers.hasNext()) { - if ((matchers.next() instanceof AnyRequestMatcher && matchers.hasNext())) { + 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 namespace or FilterChainProxy bean configuration"); @@ -240,13 +258,11 @@ public class FilterChainProxy extends GenericFilterBean { } /** - * Returns a copy of the underlying filter chain map. Modifications to the map contents - * will not affect the FilterChainProxy state - to change the map call {@code setFilterChainMap}. - * - * @return the map of path pattern Strings to filter chain lists (with ordering guaranteed). + * @return the list of {@code SecurityFilterChain}s which will be matched against and + * applied to incoming requests. */ - public Map> getFilterChainMap() { - return new LinkedHashMap>(filterChainMap); + public List getFilterChains() { + return Collections.unmodifiableList(filterChains); } /** @@ -273,7 +289,7 @@ public class FilterChainProxy extends GenericFilterBean { StringBuilder sb = new StringBuilder(); sb.append("FilterChainProxy["); sb.append("Filter Chains: "); - sb.append(filterChainMap); + sb.append(filterChains); sb.append("]"); return sb.toString(); diff --git a/web/src/main/java/org/springframework/security/web/SecurityFilterChain.java b/web/src/main/java/org/springframework/security/web/SecurityFilterChain.java new file mode 100644 index 0000000000..86e8605954 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/SecurityFilterChain.java @@ -0,0 +1,54 @@ +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}. + * in order to decide whether it applies to that request. + *

+ * Used to configure a {@code FilterChainProxy}. + * + * @author Luke Taylor + * + * @since 3.1 + */ +public final class SecurityFilterChain { + private final RequestMatcher requestMatcher; + private final List filters; + + public SecurityFilterChain(RequestMatcher requestMatcher, Filter... filters) { + this(requestMatcher, Arrays.asList(filters)); + } + + public SecurityFilterChain(RequestMatcher requestMatcher, List filters) { + this.requestMatcher = requestMatcher; + this.filters = filters; + } + + public RequestMatcher getRequestMatcher() { + return requestMatcher; + } + + public List getFilters() { + return filters; + } + + public boolean matches(HttpServletRequest request) { + return requestMatcher.matches(request); + } + + @Override + public String toString() { + return "[ " + requestMatcher + ", " + filters + "]"; + } +} diff --git a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java index 4e2e98e3ec..582368a679 100644 --- a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java +++ b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java @@ -35,8 +35,6 @@ public class FilterChainProxyTests { @Before public void setup() throws Exception { - fcp = new FilterChainProxy(); - fcp.setFilterChainValidator(mock(FilterChainProxy.FilterChainValidator.class)); matcher = mock(RequestMatcher.class); filter = mock(Filter.class); doAnswer(new Answer() { @@ -49,9 +47,8 @@ public class FilterChainProxyTests { return null; } }).when(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class)); - LinkedHashMap map = new LinkedHashMap(); - map.put(matcher, Arrays.asList(filter)); - fcp.setFilterChainMap(map); + fcp = new FilterChainProxy(new SecurityFilterChain(matcher, Arrays.asList(filter))); + fcp.setFilterChainValidator(mock(FilterChainProxy.FilterChainValidator.class)); request = new MockHttpServletRequest(); request.setServletPath("/path"); response = new MockHttpServletResponse(); @@ -68,14 +65,23 @@ public class FilterChainProxyTests { public void securityFilterChainIsNotInvokedIfMatchFails() throws Exception { when(matcher.matches(any(HttpServletRequest.class))).thenReturn(false); fcp.doFilter(request, response, chain); - assertEquals(1, fcp.getFilterChainMap().size()); - assertSame(filter, fcp.getFilterChainMap().get(matcher).get(0)); + assertEquals(1, fcp.getFilterChains().size()); + assertSame(filter, fcp.getFilterChains().get(0).getFilters().get(0)); verifyZeroInteractions(filter); // The actual filter chain should be invoked though verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class)); } + @Test + @Deprecated + public void filterChainMapIsCorrect() throws Exception { + fcp.setFilterChainMap(fcp.getFilterChainMap()); + Map> filterChainMap = fcp.getFilterChainMap(); + assertEquals(1, filterChainMap.size()); + assertSame(filter, filterChainMap.get(matcher).get(0)); + } + @Test public void originalChainIsInvokedAfterSecurityChainIfMatchSucceeds() throws Exception { when(matcher.matches(any(HttpServletRequest.class))).thenReturn(true); @@ -87,9 +93,8 @@ public class FilterChainProxyTests { @Test public void originalFilterChainIsInvokedIfMatchingSecurityChainIsEmpty() throws Exception { - LinkedHashMap map = new LinkedHashMap(); - map.put(matcher, Collections.emptyList()); - fcp.setFilterChainMap(map); + List noFilters = Collections.emptyList(); + fcp = new FilterChainProxy(new SecurityFilterChain(matcher, noFilters)); when(matcher.matches(any(HttpServletRequest.class))).thenReturn(true); fcp.doFilter(request, response, chain); @@ -132,10 +137,7 @@ public class FilterChainProxyTests { @Test public void bothWrappersAreResetWithNestedFcps() throws Exception { HttpFirewall fw = mock(HttpFirewall.class); - FilterChainProxy firstFcp = new FilterChainProxy(); - LinkedHashMap fcm = new LinkedHashMap(); - fcm.put(matcher, Arrays.asList(fcp)); - firstFcp.setFilterChainMap(fcm); + FilterChainProxy firstFcp = new FilterChainProxy(new SecurityFilterChain(matcher, fcp)); firstFcp.setFirewall(fw); fcp.setFirewall(fw); FirewalledRequest firstFwr = mock(FirewalledRequest.class, "firstFwr"); @@ -153,4 +155,4 @@ public class FilterChainProxyTests { verify(firstFwr).reset(); verify(fwr).reset(); } -} \ No newline at end of file +}