diff --git a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java index 1d67337dd3..72b9117a66 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java @@ -1,8 +1,8 @@ package org.springframework.security.web.access.expression; import java.util.ArrayList; +import java.util.Collection; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import org.apache.commons.logging.Log; @@ -26,23 +26,23 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource extends private final static Log logger = LogFactory.getLog(ExpressionBasedFilterInvocationSecurityMetadataSource.class); public ExpressionBasedFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher, - LinkedHashMap> requestMap, WebSecurityExpressionHandler expressionHandler) { + LinkedHashMap> requestMap, WebSecurityExpressionHandler expressionHandler) { super(urlMatcher, processMap(requestMap, expressionHandler.getExpressionParser())); Assert.notNull(expressionHandler, "A non-null SecurityExpressionHandler is required"); } - private static LinkedHashMap> processMap( - LinkedHashMap> requestMap, ExpressionParser parser) { + private static LinkedHashMap> processMap( + LinkedHashMap> requestMap, ExpressionParser parser) { Assert.notNull(parser, "SecurityExpressionHandler returned a null parser object"); - LinkedHashMap> requestToExpressionAttributesMap = - new LinkedHashMap>(requestMap); + LinkedHashMap> requestToExpressionAttributesMap = + new LinkedHashMap>(requestMap); - for (Map.Entry> entry : requestMap.entrySet()) { + for (Map.Entry> entry : requestMap.entrySet()) { RequestKey request = entry.getKey(); Assert.isTrue(entry.getValue().size() == 1, "Expected a single expression attribute for " + request); ArrayList attributes = new ArrayList(1); - String expression = entry.getValue().get(0).getAttribute(); + String expression = entry.getValue().toArray(new ConfigAttribute[1])[0].getAttribute(); logger.debug("Adding web access control expression '" + expression + "', for " + request); try { attributes.add(new WebExpressionConfigAttribute(parser.parseExpression(expression))); diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java index c91af45313..4fb4e1b769 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java @@ -20,7 +20,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Set; @@ -60,8 +59,8 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo //private Map> requestMap = new LinkedHashMap>(); /** Stores request maps keyed by specific HTTP methods. A null key matches any method */ - private Map>> httpMethodMap = - new HashMap>>(); + private Map>> httpMethodMap = + new HashMap>>(); private UrlMatcher urlMatcher; @@ -78,10 +77,10 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo * @param requestMap order-preserving map of request definitions to attribute lists */ public DefaultFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher, - LinkedHashMap> requestMap) { + LinkedHashMap> requestMap) { this.urlMatcher = urlMatcher; - for (Map.Entry> entry : requestMap.entrySet()) { + for (Map.Entry> entry : requestMap.entrySet()) { addSecureUrl(entry.getKey().getUrl(), entry.getKey().getMethod(), entry.getValue()); } } @@ -94,13 +93,13 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo * to the request map and will be passed back to the UrlMatcher when iterating through the map to find * a match for a particular URL. */ - private void addSecureUrl(String pattern, String method, List attr) { - Map> mapToUse = getRequestMapForHttpMethod(method); + private void addSecureUrl(String pattern, String method, Collection attrs) { + Map> mapToUse = getRequestMapForHttpMethod(method); - mapToUse.put(urlMatcher.compile(pattern), attr); + mapToUse.put(urlMatcher.compile(pattern), attrs); if (logger.isDebugEnabled()) { - logger.debug("Added URL pattern: " + pattern + "; attributes: " + attr + + logger.debug("Added URL pattern: " + pattern + "; attributes: " + attrs + (method == null ? "" : " for HTTP method '" + method + "'")); } } @@ -110,15 +109,15 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo * @param method GET, POST etc * @return map of URL patterns to ConfigAttributes for this method. */ - private Map> getRequestMapForHttpMethod(String method) { + private Map> getRequestMapForHttpMethod(String method) { if (method != null && !HTTP_METHODS.contains(method)) { throw new IllegalArgumentException("Unrecognised HTTP method: '" + method + "'"); } - Map> methodRequestMap = httpMethodMap.get(method); + Map> methodRequestMap = httpMethodMap.get(method); if (methodRequestMap == null) { - methodRequestMap = new LinkedHashMap>(); + methodRequestMap = new LinkedHashMap>(); httpMethodMap.put(method, methodRequestMap); } @@ -128,8 +127,8 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo public Collection getAllConfigAttributes() { Set allAttributes = new HashSet(); - for (Map.Entry>> entry : httpMethodMap.entrySet()) { - for (List attrs : entry.getValue().values()) { + for (Map.Entry>> entry : httpMethodMap.entrySet()) { + for (Collection attrs : entry.getValue().values()) { allAttributes.addAll(attrs); } } @@ -161,7 +160,7 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo * @return the ConfigAttributes that apply to the specified FilterInvocation * or null if no match is found */ - public final List lookupAttributes(String url, String method) { + public final Collection lookupAttributes(String url, String method) { if (stripQueryStringFromUrls) { // Strip anything after a question mark symbol, as per SEC-161. See also SEC-321 int firstQuestionMarkIndex = url.indexOf("?"); @@ -180,7 +179,7 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo } // Obtain the map of request patterns to attributes for this method and lookup the url. - List attributes = extractMatchingAttributes(url, httpMethodMap.get(method)); + Collection attributes = extractMatchingAttributes(url, httpMethodMap.get(method)); // If no attributes found in method-specific map, use the general one stored under the null key if (attributes == null) { @@ -190,14 +189,14 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo return attributes; } - private List extractMatchingAttributes(String url, Map> requestMap) { - if (requestMap == null) { + private Collection extractMatchingAttributes(String url, Map> map) { + if (map == null) { return null; } final boolean debug = logger.isDebugEnabled(); - for (Map.Entry> entry : requestMap.entrySet()) { + for (Map.Entry> entry : map.entrySet()) { Object p = entry.getKey(); boolean matched = urlMatcher.pathMatchesUrl(entry.getKey(), url); diff --git a/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java b/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java index 8e7a9afc9e..8a122bdcfb 100644 --- a/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java @@ -20,7 +20,6 @@ import static org.mockito.Mockito.mock; import java.io.IOException; import java.util.Collection; -import java.util.List; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -120,8 +119,7 @@ public class ChannelProcessingFilterTests { } @Test - public void testDoFilterWhenNullConfigAttributeReturned() - throws Exception { + public void testDoFilterWhenNullConfigAttributeReturned() throws Exception { ChannelProcessingFilter filter = new ChannelProcessingFilter(); filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "NOT_USED")); @@ -180,7 +178,7 @@ public class ChannelProcessingFilterTests { } private class MockFilterInvocationDefinitionMap implements FilterInvocationSecurityMetadataSource { - private List toReturn; + private Collection toReturn; private String servletPath; private boolean provideIterator; @@ -190,7 +188,7 @@ public class ChannelProcessingFilterTests { this.provideIterator = provideIterator; } - public List getAttributes(Object object) + public Collection getAttributes(Object object) throws IllegalArgumentException { FilterInvocation fi = (FilterInvocation) object; diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java b/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java index 7f70edd9eb..5da376fda6 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java +++ b/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java @@ -20,7 +20,6 @@ import static org.mockito.Mockito.mock; import java.util.Collection; import java.util.LinkedHashMap; -import java.util.List; import javax.servlet.FilterChain; @@ -44,7 +43,7 @@ import org.springframework.security.web.util.AntUrlPathMatcher; @SuppressWarnings("unchecked") public class DefaultFilterInvocationSecurityMetadataSourceTests { private DefaultFilterInvocationSecurityMetadataSource fids; - private List def = SecurityConfig.createList("ROLE_ONE"); + private Collection def = SecurityConfig.createList("ROLE_ONE"); //~ Methods ======================================================================================================== private void createFids(String url, String method) { @@ -87,7 +86,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null); - List response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); assertEquals(def, response); } @@ -97,7 +96,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null); - List response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); assertEquals(null, response); } @@ -107,7 +106,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null); - List response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); assertEquals(def, response); } @@ -117,7 +116,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { FilterInvocation fi = createFilterInvocation("/someAdminPage.html?a=/test", null); - List response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); assertEquals(def, response); // see SEC-161 (it should truncate after ? sign) } @@ -155,10 +154,10 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { @Test public void httpMethodSpecificUrlTakesPrecedence() { - LinkedHashMap> requestMap = new LinkedHashMap>(); + LinkedHashMap> requestMap = new LinkedHashMap>(); // Even though this is added before the Http method-specific def, the latter should match requestMap.put(new RequestKey("/**"), def); - List postOnlyDef = SecurityConfig.createList("ROLE_TWO"); + Collection postOnlyDef = SecurityConfig.createList("ROLE_TWO"); requestMap.put(new RequestKey("/somepage**", "POST"), postOnlyDef); fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); @@ -170,7 +169,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { @Test public void mixingPatternsWithAndWithoutHttpMethodsIsSupported() throws Exception { LinkedHashMap requestMap = new LinkedHashMap(); - List userAttrs = SecurityConfig.createList("A"); + Collection userAttrs = SecurityConfig.createList("A"); requestMap.put(new RequestKey("/user/**", null), userAttrs); requestMap.put(new RequestKey("/teller/**", "GET"), SecurityConfig.createList("B")); fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); @@ -190,7 +189,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { FilterInvocation fi = createFilterInvocation("/someAdminPage.html?x=2/aa?y=3", null); - List response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); assertEquals(def, response); fi = createFilterInvocation("/someAdminPage.html??", null);