浏览代码

SEC-999,SEC-1013: removed ConfigAtributeDefinition from ObjectDefinitionSource and implementations. Modified el-authz to allow methods which use an annotation without explicitly specifying a PreAuthorize condition

Luke Taylor 17 年之前
父节点
当前提交
f592357c27
共有 47 个文件被更改,包括 526 次插入651 次删除
  1. 1 1
      core/src/main/java/org/springframework/security/ConfigAttributeEditor.java
  2. 15 1
      core/src/main/java/org/springframework/security/SecurityConfig.java
  3. 1 1
      core/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java
  4. 1 1
      core/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java
  5. 45 42
      core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java
  6. 1 1
      core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java
  7. 18 6
      core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java
  8. 8 1
      core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java
  9. 55 45
      core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java
  10. 5 5
      core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java
  11. 14 8
      core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java
  12. 0 12
      core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionBasedMethodConfigAttribute.java
  13. 12 0
      core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java
  14. 0 18
      core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionBasedMethodConfigAttribute.java
  15. 23 0
      core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java
  16. 12 11
      core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java
  17. 7 5
      core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java
  18. 13 16
      core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java
  19. 4 3
      core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java
  20. 55 54
      core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java
  21. 2 8
      core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java
  22. 3 2
      core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java
  23. 3 1
      core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java
  24. 5 2
      core/src/main/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluator.java
  25. 24 26
      core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java
  26. 2 5
      core/src/main/java/org/springframework/security/intercept/web/FIDSToFilterChainMapConverter.java
  27. 2 2
      core/src/main/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditor.java
  28. 5 2
      core/src/main/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluator.java
  29. 7 12
      core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java
  30. 0 138
      core/src/test/java/org/springframework/security/ConfigAttributeEditorTests.java
  31. 1 2
      core/src/test/java/org/springframework/security/annotation/ExpressionProtectedBusinessServiceImpl.java
  32. 26 49
      core/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java
  33. 4 1
      core/src/test/java/org/springframework/security/config/FilterInvocationDefinitionSourceParserTests.java
  34. 1 1
      core/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java
  35. 8 7
      core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java
  36. 26 15
      core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java
  37. 3 3
      core/src/test/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSourceTests.java
  38. 21 21
      core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java
  39. 11 9
      core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java
  40. 5 5
      core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java
  41. 33 37
      core/src/test/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSourceTests.java
  42. 14 17
      core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java
  43. 10 11
      core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java
  44. 4 3
      core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java
  45. 0 4
      core/src/test/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluatorTests.java
  46. 14 26
      core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java
  47. 2 11
      core/src/test/java/org/springframework/security/util/FilterChainProxyTests.java

+ 1 - 1
core/src/main/java/org/springframework/security/ConfigAttributeEditor.java

@@ -34,7 +34,7 @@ public class ConfigAttributeEditor extends PropertyEditorSupport {
 
     public void setAsText(String s) throws IllegalArgumentException {
         if (StringUtils.hasText(s)) {
-            setValue(new ConfigAttributeDefinition(StringUtils.commaDelimitedListToStringArray(s)));
+            setValue(SecurityConfig.createList(StringUtils.commaDelimitedListToStringArray(s)));
         } else {
             setValue(null);
         }

+ 15 - 1
core/src/main/java/org/springframework/security/SecurityConfig.java

@@ -15,6 +15,9 @@
 
 package org.springframework.security;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.springframework.util.Assert;
 
 /**
@@ -31,7 +34,7 @@ public class SecurityConfig implements ConfigAttribute {
     //~ Constructors ===================================================================================================
 
     public SecurityConfig(String config) {
-    	Assert.hasText(config, "You must provide a configuration attribute");
+        Assert.hasText(config, "You must provide a configuration attribute");
         this.attrib = config;
     }
 
@@ -58,4 +61,15 @@ public class SecurityConfig implements ConfigAttribute {
     public String toString() {
         return this.attrib;
     }
+
+    public 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);
+
+        for (String attribute : attributeNames) {
+            attributes.add(new SecurityConfig(attribute));
+        }
+
+        return attributes;
+    }
 }

+ 1 - 1
core/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java

@@ -48,7 +48,7 @@ public class Jsr250MethodDefinitionSource extends AbstractFallbackMethodDefiniti
         return processAnnotations(AnnotationUtils.getAnnotations(method));
     }
 
-    public Collection getConfigAttributeDefinitions() {
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
         return null;
     }
 

+ 1 - 1
core/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java

@@ -43,7 +43,7 @@ public class SecuredMethodDefinitionSource extends AbstractFallbackMethodDefinit
         return processAnnotation(AnnotationUtils.findAnnotation(method, Secured.class));
     }
 
-    public Collection getConfigAttributeDefinitions() {
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
         return null;
     }
 

+ 45 - 42
core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java

@@ -16,6 +16,7 @@ import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.core.OrderComparator;
 import org.springframework.core.Ordered;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.config.ConfigUtils.FilterChainList;
 import org.springframework.security.context.HttpSessionContextIntegrationFilter;
@@ -33,66 +34,66 @@ import org.springframework.security.util.FilterChainProxy;
 import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter;
 
 /**
- * 
+ *
  * @author Luke Taylor
  * @version $Id$
  * @since 2.0
  */
 public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFactoryAware {
     private Log logger = LogFactory.getLog(getClass());
-    
+
     private ListableBeanFactory beanFactory;
-    
+
     public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
         if(!BeanIds.FILTER_CHAIN_PROXY.equals(beanName)) {
             return bean;
         }
-        
+
         FilterChainProxy filterChainProxy = (FilterChainProxy) bean;
         FilterChainList filterList = (FilterChainList) beanFactory.getBean(BeanIds.FILTER_LIST);
-        
+
         List filters = new ArrayList(filterList.getFilters());
         Collections.sort(filters, new OrderComparator());
-        
+
         logger.info("Checking sorted filter chain: " + filters);
-        
+
         for(int i=0; i < filters.size(); i++) {
             Ordered filter = (Ordered)filters.get(i);
 
             if (i > 0) {
                 Ordered previous = (Ordered)filters.get(i-1);
                 if (filter.getOrder() == previous.getOrder()) {
-                    throw new SecurityConfigurationException("Filters '" + unwrapFilter(filter) + "' and '" + 
+                    throw new SecurityConfigurationException("Filters '" + unwrapFilter(filter) + "' and '" +
                             unwrapFilter(previous) + "' have the same 'order' value. When using custom filters, " +
-                            		"please make sure the positions do not conflict with default filters. " +
-                            		"Alternatively you can disable the default filters by removing the corresponding " +
-                            		"child elements from <http> and avoiding the use of <http auto-config='true'>.");
+                                    "please make sure the positions do not conflict with default filters. " +
+                                    "Alternatively you can disable the default filters by removing the corresponding " +
+                                    "child elements from <http> and avoiding the use of <http auto-config='true'>.");
                 }
             }
         }
 
-        logger.info("Filter chain...");        
+        logger.info("Filter chain...");
         for (int i=0; i < filters.size(); i++) {
         // Remove the ordered wrapper from the filter and put it back in the chain at the same position.
             Filter filter = unwrapFilter(filters.get(i));
-            logger.info("[" + i + "] - " + filter);            
+            logger.info("[" + i + "] - " + filter);
             filters.set(i, filter);
         }
-        
+
         checkFilterStack(filters);
-        
+
         // Note that this returns a copy
         Map filterMap = filterChainProxy.getFilterChainMap();
         filterMap.put(filterChainProxy.getMatcher().getUniversalMatchPattern(), filters);
         filterChainProxy.setFilterChainMap(filterMap);
-        
+
         checkLoginPageIsntProtected(filterChainProxy);
-        
+
         logger.info("FilterChainProxy: " + filterChainProxy);
 
         return bean;
     }
-    
+
     /**
      * Checks the filter list for possible errors and logs them
      */
@@ -105,7 +106,7 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac
         checkForDuplicates(ExceptionTranslationFilter.class, filters);
         checkForDuplicates(FilterSecurityInterceptor.class, filters);
     }
-    
+
     private void checkForDuplicates(Class clazz, List filters) {
         for (int i=0; i < filters.size(); i++) {
             Filter f1 = (Filter)filters.get(i);
@@ -122,53 +123,55 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac
             }
         }
     }
-    
+
     /* Checks for the common error of having a login page URL protected by the security interceptor */
     private void checkLoginPageIsntProtected(FilterChainProxy fcp) {
         ExceptionTranslationFilter etf = (ExceptionTranslationFilter) beanFactory.getBean(BeanIds.EXCEPTION_TRANSLATION_FILTER);
-        
+
         if (etf.getAuthenticationEntryPoint() instanceof AuthenticationProcessingFilterEntryPoint) {
-            String loginPage = 
+            String loginPage =
                 ((AuthenticationProcessingFilterEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl();
             List filters = fcp.getFilters(loginPage);
             logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration");
-                        
+
             if (filters == null || filters.isEmpty()) {
                 logger.debug("Filter chain is empty for the login page");
                 return;
             }
-            
+
             if (loginPage.equals(DefaultLoginPageGeneratingFilter.DEFAULT_LOGIN_PAGE_URL) &&
                     beanFactory.containsBean(BeanIds.DEFAULT_LOGIN_PAGE_GENERATING_FILTER)) {
                 logger.debug("Default generated login page is in use");
                 return;
             }
-            
-            FilterSecurityInterceptor fsi = 
+
+            FilterSecurityInterceptor fsi =
                     ((FilterSecurityInterceptor)beanFactory.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR));
-            DefaultFilterInvocationDefinitionSource fids = 
+            DefaultFilterInvocationDefinitionSource fids =
                     (DefaultFilterInvocationDefinitionSource) fsi.getObjectDefinitionSource();
-            ConfigAttributeDefinition cad = fids.lookupAttributes(loginPage, "POST");
-            
-            if (cad == null) {
+            List<? extends ConfigAttribute> attributes = fids.lookupAttributes(loginPage, "POST");
+
+            if (attributes == null) {
                 logger.debug("No access attributes defined for login page URL");
                 if (fsi.isRejectPublicInvocations()) {
                     logger.warn("FilterSecurityInterceptor is configured to reject public invocations." +
-                    		" Your login page may not be accessible.");
+                            " Your login page may not be accessible.");
                 }
                 return;
             }
 
+            ConfigAttributeDefinition cad = new ConfigAttributeDefinition(fids.lookupAttributes(loginPage, "POST"));
+
             if (!beanFactory.containsBean(BeanIds.ANONYMOUS_PROCESSING_FILTER)) {
                 logger.warn("The login page is being protected by the filter chain, but you don't appear to have" +
-                		" anonymous authentication enabled. This is almost certainly an error.");
+                        " anonymous authentication enabled. This is almost certainly an error.");
                 return;
             }
-            
+
             // Simulate an anonymous access with the supplied attributes.
             AnonymousProcessingFilter anonPF = (AnonymousProcessingFilter) beanFactory.getBean(BeanIds.ANONYMOUS_PROCESSING_FILTER);
-            AnonymousAuthenticationToken token = 
-                    new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), 
+            AnonymousAuthenticationToken token =
+                    new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(),
                             anonPF.getUserAttribute().getAuthorities());
             try {
                 fsi.getAccessDecisionManager().decide(token, new Object(), cad);
@@ -179,15 +182,15 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac
             }
         }
     }
-    
-    /** 
-     * Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped. 
+
+    /**
+     * Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped.
      */
     private Filter unwrapFilter(Object filter) {
         if (filter instanceof OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator) {
             return ((OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator)filter).getDelegate();
         }
-        
+
         return (Filter) filter;
     }
 
@@ -195,7 +198,7 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac
         return bean;
     }
 
-	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
-		this.beanFactory = (ListableBeanFactory) beanFactory;
-	}
+    public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
+        this.beanFactory = (ListableBeanFactory) beanFactory;
+    }
 }

+ 1 - 1
core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java

@@ -561,7 +561,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
                 }
 
                 editor.setAsText(channelConfigAttribute);
-                channelRequestMap.put(new RequestKey(path), (ConfigAttributeDefinition) editor.getValue());
+                channelRequestMap.put(new RequestKey(path), editor.getValue());
             }
 
             String filters = urlElt.getAttribute(ATT_FILTERS);

+ 18 - 6
core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java

@@ -17,7 +17,7 @@ public class SecurityExpressionRoot {
         this.authentication = a;
     }
 
-    public boolean hasRole(String role) {
+    public final boolean hasRole(String role) {
         return hasAnyRole(role);
     }
 
@@ -33,19 +33,27 @@ public class SecurityExpressionRoot {
         return false;
     }
 
-    public boolean isAnonymous() {
+    public final boolean permitAll() {
+        return true;
+    }
+
+    public final boolean denyAll() {
+        return false;
+    }
+
+    public final boolean isAnonymous() {
         return trustResolver.isAnonymous(authentication);
     }
 
-    public boolean isRememberMe() {
+    public final boolean isRememberMe() {
         return trustResolver.isRememberMe(authentication);
     }
 
-    public String getName() {
-        return authentication.getName();
+    public Authentication getAuthentication() {
+        return authentication;
     }
 
-    public boolean isFullyAuthenticated() {
+    public final boolean isFullyAuthenticated() {
         return !trustResolver.isAnonymous(authentication) && !trustResolver.isRememberMe(authentication);
     }
 
@@ -64,4 +72,8 @@ public class SecurityExpressionRoot {
     public Object getReturnObject() {
         return returnObject;
     }
+
+    public Object getPrincipal() {
+        return authentication.getPrincipal();
+    }
 }

+ 8 - 1
core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java

@@ -8,7 +8,14 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Annotation for specifying a method filtering expression which will be evaluated after a method has been invoked.
+ * Annotation for specifying a method filtering expression which will be evaluated before a method has been invoked.
+ * The name of the argument to be filtered is specified using the <tt>filterTarget</tt> attribute. This must be a
+ * Java Collection implementation which supports the {@link java.util.Collection#remove(Object) remove} method.
+ * Pre-filtering isn't supported on array types.
+ * <p>
+ * The annotation value contains the expression which will be evaluated for each element in the collection. If the
+ * expression evaluates to false, the element will be removed. The reserved name "filterObject" can be used within the
+ * expression to refer to the current object which is being evaluated.
  *
  * @author Luke Taylor
  * @version $Id$

+ 55 - 45
core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java

@@ -8,7 +8,6 @@ import java.util.List;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.expression.ParseException;
 import org.springframework.security.ConfigAttribute;
-import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.config.SecurityConfigurationException;
 import org.springframework.security.expression.annotation.PostAuthorize;
 import org.springframework.security.expression.annotation.PostFilter;
@@ -19,6 +18,10 @@ import org.springframework.security.intercept.method.AbstractFallbackMethodDefin
 /**
  * MethodDefinitionSource which extracts metadata from the @PreFilter and @PreAuthorize annotations
  * placed on a method. The metadata is encapsulated in a {@link AbstractExpressionBasedMethodConfigAttribute} instance.
+ * <p>
+ * Annotations may be specified on classes or methods, and method-specific annotations will take precedence.
+ * If you use any annotation and do not specify a pre-authorization condition, then the method will be
+ * allowed as if a @PreAuthorize("permitAll") were present.
  *
  * @see MethodExpressionVoter
  *
@@ -30,82 +33,89 @@ public class ExpressionAnnotationMethodDefinitionSource extends AbstractFallback
 
     @Override
     protected List<ConfigAttribute> findAttributes(Method method, Class targetClass) {
-        ConfigAttribute pre = processPreInvocationAnnotations(AnnotationUtils.findAnnotation(method, PreFilter.class),
-                AnnotationUtils.findAnnotation(method, PreAuthorize.class));
-        ConfigAttribute post = processPostInvocationAnnotations(AnnotationUtils.findAnnotation(method, PostFilter.class),
-                AnnotationUtils.findAnnotation(method, PostAuthorize.class));
+        PreFilter preFilter = AnnotationUtils.findAnnotation(method, PreFilter.class);
+        PreAuthorize preAuthorize = AnnotationUtils.findAnnotation(method, PreAuthorize.class);
+        PostFilter postFilter = AnnotationUtils.findAnnotation(method, PostFilter.class);
+        PostAuthorize postAuthorize = AnnotationUtils.findAnnotation(method, PostAuthorize.class);
 
-        if (pre == null && post == null) {
+        if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null ) {
+            // There is no method level meta-data so return and allow parent to query at class-level
             return null;
         }
 
-        List<ConfigAttribute> attrs = new ArrayList<ConfigAttribute>(2);
-        if (pre != null) {
-            attrs.add(pre);
+        // There is at least one non-null value, so the parent class will not query for class-specific annotations
+        // and we have to locate them here as appropriate.
+
+        if (preAuthorize == null) {
+            preAuthorize = (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class);
         }
 
-        if (post != null) {
-            attrs.add(post);
+        if (preFilter == null) {
+            preFilter = (PreFilter)targetClass.getAnnotation(PreFilter.class);
         }
 
-        return attrs;
+        if (postFilter == null) {
+            // TODO: Can we check for void methods and throw an exception here?
+            postFilter = (PostFilter)targetClass.getAnnotation(PostFilter.class);
+        }
+
+        if (postAuthorize == null) {
+            postAuthorize = (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class);
+        }
+
+        return createAttributeList(preFilter, preAuthorize, postFilter, postAuthorize);
     }
 
     @Override
     protected List<ConfigAttribute> findAttributes(Class targetClass) {
-        ConfigAttribute pre = processPreInvocationAnnotations((PreFilter)targetClass.getAnnotation(PreFilter.class),
-                (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class));
-        ConfigAttribute post = processPostInvocationAnnotations((PostFilter)targetClass.getAnnotation(PostFilter.class),
-                (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class));
+        PreFilter preFilter = (PreFilter)targetClass.getAnnotation(PreFilter.class);
+        PreAuthorize preAuthorize = (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class);
+        PostFilter postFilter = (PostFilter)targetClass.getAnnotation(PostFilter.class);
+        PostAuthorize postAuthorize = (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class);
 
-        if (pre == null && post == null) {
+        if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null ) {
+            // There is no class level meta-data (and by implication no meta-data at all)
             return null;
         }
 
-        List<ConfigAttribute> attrs = new ArrayList<ConfigAttribute>(2);
-        if (pre != null) {
-            attrs.add(pre);
-        }
-
-        if (post != null) {
-            attrs.add(post);
-        }
-
-        return attrs;
+        return createAttributeList(preFilter, preAuthorize, postFilter, postAuthorize);
     }
 
-    public Collection getConfigAttributeDefinitions() {
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
         return null;
     }
 
-    private ConfigAttribute processPreInvocationAnnotations(PreFilter preFilter, PreAuthorize preAuthz) {
-        if (preFilter == null && preAuthz == null) {
-            return null;
-        }
+    private List<ConfigAttribute> createAttributeList(PreFilter preFilter, PreAuthorize preAuthorize,
+            PostFilter postFilter, PostAuthorize postAuthorize) {
+        ConfigAttribute pre = null;
+        ConfigAttribute post = null;
 
-        String preAuthorizeExpression = preAuthz == null ? null : preAuthz.value();
+        // TODO: Optimization of permitAll
+        String preAuthorizeExpression = preAuthorize == null ? "permitAll()" : preAuthorize.value();
         String preFilterExpression = preFilter == null ? null : preFilter.value();
         String filterObject = preFilter == null ? null : preFilter.filterTarget();
+        String postAuthorizeExpression = postAuthorize == null ? null : postAuthorize.value();
+        String postFilterExpression = postFilter == null ? null : postFilter.value();
 
         try {
-            return new PreInvocationExpressionBasedMethodConfigAttribute(preFilterExpression, filterObject, preAuthorizeExpression);
+            pre = new PreInvocationExpressionConfigAttribute(preFilterExpression, filterObject, preAuthorizeExpression);
+            if (postFilterExpression != null || postAuthorizeExpression != null) {
+                post = new PostInvocationExpressionConfigAttribute(postFilterExpression, postAuthorizeExpression);
+            }
         } catch (ParseException e) {
             throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e);
         }
-    }
 
-    private ConfigAttribute processPostInvocationAnnotations(PostFilter postFilter, PostAuthorize postAuthz) {
-        if (postFilter == null && postAuthz == null) {
-            return null;
-        }
 
-        String postAuthorizeExpression = postAuthz == null ? null : postAuthz.value();
-        String postFilterExpression = postFilter == null ? null : postFilter.value();
+        List<ConfigAttribute> attrs = new ArrayList<ConfigAttribute>(2);
+        if (pre != null) {
+            attrs.add(pre);
+        }
 
-        try {
-            return new PostInvocationExpressionBasedMethodConfigAttribute(postFilterExpression, postAuthorizeExpression);
-        } catch (ParseException e) {
-            throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e);
+        if (post != null) {
+            attrs.add(post);
         }
+
+        return attrs;
     }
 }

+ 5 - 5
core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java

@@ -35,7 +35,7 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP
     public Object decide(Authentication authentication, Object object, ConfigAttributeDefinition config, Object returnedObject)
             throws AccessDeniedException {
 
-        PostInvocationExpressionBasedMethodConfigAttribute mca = findMethodAccessControlExpression(config);
+        PostInvocationExpressionConfigAttribute mca = findMethodAccessControlExpression(config);
 
         if (mca == null) {
             return returnedObject;
@@ -86,11 +86,11 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP
         }
     }
 
-    private PostInvocationExpressionBasedMethodConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) {
+    private PostInvocationExpressionConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) {
         // Find the MethodAccessControlExpression attribute
         for (ConfigAttribute attribute : config.getConfigAttributes()) {
-            if (attribute instanceof PostInvocationExpressionBasedMethodConfigAttribute) {
-                return (PostInvocationExpressionBasedMethodConfigAttribute)attribute;
+            if (attribute instanceof PostInvocationExpressionConfigAttribute) {
+                return (PostInvocationExpressionConfigAttribute)attribute;
             }
         }
 
@@ -98,7 +98,7 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP
     }
 
     public boolean supports(ConfigAttribute attribute) {
-        return attribute instanceof PostInvocationExpressionBasedMethodConfigAttribute;
+        return attribute instanceof PostInvocationExpressionConfigAttribute;
     }
 
     public boolean supports(Class clazz) {

+ 14 - 8
core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java

@@ -44,7 +44,7 @@ public class MethodExpressionVoter implements AccessDecisionVoter {
     }
 
     public int vote(Authentication authentication, Object object, ConfigAttributeDefinition config) {
-        PreInvocationExpressionBasedMethodConfigAttribute mace = findMethodAccessControlExpression(config);
+        PreInvocationExpressionConfigAttribute mace = findMethodAccessControlExpression(config);
 
         if (mace == null) {
             // No expression based metadata, so abstain
@@ -62,7 +62,7 @@ public class MethodExpressionVoter implements AccessDecisionVoter {
 
         if (preFilter != null) {
             // TODO: Allow null target if only single parameter, or single collection/array?
-            Object filtered = ExpressionUtils.doFilter(filterTarget, preFilter, ctx);
+            ExpressionUtils.doFilter(filterTarget, preFilter, ctx);
         }
 
         if (preAuthorize == null) {
@@ -88,19 +88,25 @@ public class MethodExpressionVoter implements AccessDecisionVoter {
             }
         }
 
-        if (filterTargetName != null && filterTarget == null) {
-            throw new IllegalArgumentException("No filter target argument with name " + filterTargetName +
-                    " found in method: " + method.getName());
+        if (filterTargetName != null) {
+            if (filterTarget == null) {
+                throw new IllegalArgumentException("No filter target argument with name " + filterTargetName +
+                        " found in method: " + method.getName());
+            }
+            if (filterTarget.getClass().isArray()) {
+                throw new IllegalArgumentException("Pre-filtering on array types is not supported. Changing '" +
+                        filterTargetName +"' to a collection will solve this problem");
+            }
         }
 
         return filterTarget;
     }
 
-    private PreInvocationExpressionBasedMethodConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) {
+    private PreInvocationExpressionConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) {
         // Find the MethodAccessControlExpression attribute
         for (ConfigAttribute attribute : config.getConfigAttributes()) {
-            if (attribute instanceof AbstractExpressionBasedMethodConfigAttribute) {
-                return (PreInvocationExpressionBasedMethodConfigAttribute)attribute;
+            if (attribute instanceof PreInvocationExpressionConfigAttribute) {
+                return (PreInvocationExpressionConfigAttribute)attribute;
             }
         }
 

+ 0 - 12
core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionBasedMethodConfigAttribute.java

@@ -1,12 +0,0 @@
-package org.springframework.security.expression.support;
-
-import org.springframework.expression.ParseException;
-
-class PostInvocationExpressionBasedMethodConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute {
-
-    PostInvocationExpressionBasedMethodConfigAttribute(String filterExpression, String authorizeExpression)
-            throws ParseException {
-        super(filterExpression, authorizeExpression);
-    }
-
-}

+ 12 - 0
core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java

@@ -0,0 +1,12 @@
+package org.springframework.security.expression.support;
+
+import org.springframework.expression.ParseException;
+
+class PostInvocationExpressionConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute {
+
+    PostInvocationExpressionConfigAttribute(String filterExpression, String authorizeExpression)
+            throws ParseException {
+        super(filterExpression, authorizeExpression);
+    }
+
+}

+ 0 - 18
core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionBasedMethodConfigAttribute.java

@@ -1,18 +0,0 @@
-package org.springframework.security.expression.support;
-
-import org.springframework.expression.ParseException;
-
-class PreInvocationExpressionBasedMethodConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute {
-    private final String filterTarget;
-
-    PreInvocationExpressionBasedMethodConfigAttribute(String filterExpression, String filterTarget,
-            String authorizeExpression) throws ParseException {
-        super(filterExpression, authorizeExpression);
-
-        this.filterTarget = filterTarget;
-    }
-
-    String getFilterTarget() {
-        return filterTarget;
-    }
-}

+ 23 - 0
core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java

@@ -0,0 +1,23 @@
+package org.springframework.security.expression.support;
+
+import org.springframework.expression.ParseException;
+
+class PreInvocationExpressionConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute {
+    private final String filterTarget;
+
+    PreInvocationExpressionConfigAttribute(String filterExpression, String filterTarget, String authorizeExpression)
+            throws ParseException {
+        super(filterExpression, authorizeExpression);
+
+        this.filterTarget = filterTarget;
+    }
+
+    /**
+     * The parameter name of the target argument (must be a Collection) to which filtering will be applied.
+     *
+     * @return the method parameter name
+     */
+    String getFilterTarget() {
+        return filterTarget;
+    }
+}

+ 12 - 11
core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java

@@ -51,6 +51,7 @@ import org.springframework.util.Assert;
 
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Set;
 import java.util.Collection;
 
@@ -110,7 +111,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
     protected static final Log logger = LogFactory.getLog(AbstractSecurityInterceptor.class);
 
     //~ Instance fields ================================================================================================
-    
+
     protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor();
     private ApplicationEventPublisher eventPublisher;
     private AccessDecisionManager accessDecisionManager;
@@ -184,7 +185,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
         }
 
         if (this.validateConfigAttributes) {
-            Collection attributeDefs = this.obtainObjectDefinitionSource().getConfigAttributeDefinitions();
+            Collection<List<? extends ConfigAttribute>> attributeDefs = this.obtainObjectDefinitionSource().getConfigAttributeDefinitions();
 
             if (attributeDefs == null) {
                 logger.warn("Could not validate configuration attributes as the ObjectDefinitionSource did not return "
@@ -192,16 +193,10 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
                 return;
             }
 
-            Iterator iter = attributeDefs.iterator();
             Set unsupportedAttrs = new HashSet();
 
-            while (iter.hasNext()) {
-                ConfigAttributeDefinition def = (ConfigAttributeDefinition) iter.next();
-                Iterator attributes = def.getConfigAttributes().iterator();
-
-                while (attributes.hasNext()) {
-                    ConfigAttribute attr = (ConfigAttribute) attributes.next();
-
+            for (List<? extends ConfigAttribute> def : attributeDefs) {
+                for (ConfigAttribute attr : def) {
                     if (!this.runAsManager.supports(attr) && !this.accessDecisionManager.supports(attr)
                             && ((this.afterInvocationManager == null) || !this.afterInvocationManager.supports(attr))) {
                         unsupportedAttrs.add(attr);
@@ -227,7 +222,13 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
                     + getSecureObjectClass());
         }
 
-        ConfigAttributeDefinition attr = this.obtainObjectDefinitionSource().getAttributes(object);
+        List<? extends ConfigAttribute> attributes = this.obtainObjectDefinitionSource().getAttributes(object);
+        ConfigAttributeDefinition attr = null;
+
+        // TODO: temporary until refactor security interceptor and AccessManager
+        if (attributes != null) {
+            attr = new ConfigAttributeDefinition(attributes);
+        }
 
         if (attr == null) {
             if (rejectPublicInvocations) {

+ 7 - 5
core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java

@@ -15,9 +15,11 @@
 
 package org.springframework.security.intercept;
 
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 
 import java.util.Collection;
+import java.util.List;
 
 
 /**
@@ -32,17 +34,17 @@ public interface ObjectDefinitionSource {
     //~ Methods ========================================================================================================
 
     /**
-     * Accesses the <code>ConfigAttributeDefinition</code> that applies to a given secure object.<P>Returns
-     * <code>null</code> if no <code>ConfigAttribiteDefinition</code> applies.</p>
+     * Accesses the <code>ConfigAttributeDefinition</code> that applies to a given secure object.
+     * <p>Returns <code>null</code> if no attributes apply.
      *
      * @param object the object being secured
      *
-     * @return the <code>ConfigAttributeDefinition</code> that applies to the passed object
+     * @return the attributes that apply to the passed in secured object
      *
      * @throws IllegalArgumentException if the passed object is not of a type supported by the
      *         <code>ObjectDefinitionSource</code> implementation
      */
-    ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException;
+    List<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException;
 
     /**
      * If available, returns all of the <code>ConfigAttributeDefinition</code>s defined by the implementing class.
@@ -52,7 +54,7 @@ public interface ObjectDefinitionSource {
      *
      * @return the <code>ConfigAttributeDefinition</code>s or <code>null</code> if unsupported
      */
-    Collection getConfigAttributeDefinitions();
+    Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions();
 
     /**
      * Indicates whether the <code>ObjectDefinitionSource</code> implementation is able to provide

+ 13 - 16
core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java

@@ -1,6 +1,7 @@
 package org.springframework.security.intercept.method;
 
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -11,7 +12,6 @@ import org.apache.commons.logging.LogFactory;
 import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.reflect.CodeSignature;
 import org.springframework.security.ConfigAttribute;
-import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
@@ -39,12 +39,12 @@ import org.springframework.util.ObjectUtils;
  * @since 2.0
  */
 public abstract class AbstractFallbackMethodDefinitionSource implements MethodDefinitionSource {
+    protected final Log logger = LogFactory.getLog(getClass());
 
-    private static final Log logger = LogFactory.getLog(AbstractFallbackMethodDefinitionSource.class);
-    private final static Object NULL_CONFIG_ATTRIBUTE = new Object();
-    private final Map attributeCache = new HashMap();
+    private final static List<ConfigAttribute> NULL_CONFIG_ATTRIBUTE = new ArrayList<ConfigAttribute>(0);
+    private final Map<DefaultCacheKey, List<ConfigAttribute>> attributeCache = new HashMap();
 
-    public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException {
+    public List<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException {
         Assert.notNull(object, "Object cannot be null");
 
         if (object instanceof MethodInvocation) {
@@ -73,11 +73,11 @@ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDe
         return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz));
     }
 
-    public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) {
+    public List<ConfigAttribute> getAttributes(Method method, Class targetClass) {
         // First, see if we have a cached value.
-        Object cacheKey = new DefaultCacheKey(method, targetClass);
-        synchronized (this.attributeCache) {
-            Object cached = this.attributeCache.get(cacheKey);
+        DefaultCacheKey cacheKey = new DefaultCacheKey(method, targetClass);
+        synchronized (attributeCache) {
+            List<ConfigAttribute> cached = attributeCache.get(cacheKey);
             if (cached != null) {
                 // Value will either be canonical value indicating there is no config attribute,
                 // or an actual config attribute.
@@ -85,25 +85,22 @@ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDe
                     return null;
                 }
                 else {
-                    return (ConfigAttributeDefinition) cached;
+                    return cached;
                 }
             }
             else {
                 // We need to work it out.
                 List<ConfigAttribute> attributes = computeAttributes(method, targetClass);
-                ConfigAttributeDefinition cfgAtt = null;
                 // Put it in the cache.
                 if (attributes == null) {
                     this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE);
                 } else {
-                    cfgAtt = new ConfigAttributeDefinition(attributes);
-
                     if (logger.isDebugEnabled()) {
-                        logger.debug("Adding security method [" + cacheKey + "] with attribute [" + cfgAtt + "]");
+                        logger.debug("Adding security method [" + cacheKey + "] with attributes " + attributes);
                     }
-                    this.attributeCache.put(cacheKey, cfgAtt);
+                    this.attributeCache.put(cacheKey, attributes);
                 }
-                return cfgAtt;
+                return attributes;
             }
         }
     }

+ 4 - 3
core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java

@@ -15,7 +15,7 @@
 
 package org.springframework.security.intercept.method;
 
-import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.ConfigAttribute;
 
 import org.aopalliance.intercept.MethodInvocation;
 
@@ -28,6 +28,7 @@ import org.aspectj.lang.reflect.CodeSignature;
 import org.springframework.util.Assert;
 
 import java.lang.reflect.Method;
+import java.util.List;
 
 
 /**
@@ -44,7 +45,7 @@ public abstract class AbstractMethodDefinitionSource implements MethodDefinition
 
     //~ Methods ========================================================================================================
 
-    public ConfigAttributeDefinition getAttributes(Object object)
+    public List<ConfigAttribute> getAttributes(Object object)
         throws IllegalArgumentException {
         Assert.notNull(object, "Object cannot be null");
 
@@ -89,7 +90,7 @@ public abstract class AbstractMethodDefinitionSource implements MethodDefinition
      *
      * @return the <code>ConfigAttributeDefinition</code> that applies to the specified <code>Method</code>
      */
-    protected abstract ConfigAttributeDefinition lookupAttributes(Method method);
+    protected abstract List<ConfigAttribute> lookupAttributes(Method method);
 
     public boolean supports(Class clazz) {
         return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz));

+ 55 - 54
core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java

@@ -8,73 +8,74 @@ import java.util.List;
 import java.util.Set;
 
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.util.Assert;
 
 /**
  * Automatically tries a series of method definition sources, relying on the first source of metadata
  * that provides a non-null response.
- * 
+ *
  * @author Ben Alex
  * @version $Id$
  */
 public final class DelegatingMethodDefinitionSource implements MethodDefinitionSource, InitializingBean {
-	private List methodDefinitionSources;
-	
-	public void afterPropertiesSet() throws Exception {
-		Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required");
-	}
+    private List methodDefinitionSources;
 
-	public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) {
-		Iterator i = methodDefinitionSources.iterator();
-		while (i.hasNext()) {
-			MethodDefinitionSource s = (MethodDefinitionSource) i.next();
-			ConfigAttributeDefinition result = s.getAttributes(method, targetClass);
-			if (result != null) {
-				return result;
-			}
-		}
-		return null;
-	}
+    public void afterPropertiesSet() throws Exception {
+        Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required");
+    }
 
-	public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException {
-		Iterator i = methodDefinitionSources.iterator();
-		while (i.hasNext()) {
-			MethodDefinitionSource s = (MethodDefinitionSource) i.next();
-			ConfigAttributeDefinition result = s.getAttributes(object);
-			if (result != null) {
-				return result;
-			}
-		}
-		return null;
-	}
+    public List<ConfigAttribute> getAttributes(Method method, Class targetClass) {
+        Iterator i = methodDefinitionSources.iterator();
+        while (i.hasNext()) {
+            MethodDefinitionSource s = (MethodDefinitionSource) i.next();
+            List<ConfigAttribute> result = s.getAttributes(method, targetClass);
+            if (result != null) {
+                return result;
+            }
+        }
+        return null;
+    }
 
-	public Collection getConfigAttributeDefinitions() {
-		Set set = new HashSet();
-		Iterator i = methodDefinitionSources.iterator();
-		while (i.hasNext()) {
-			MethodDefinitionSource s = (MethodDefinitionSource) i.next();
-			Collection attrs = s.getConfigAttributeDefinitions();
-			if (attrs != null) {
-				set.addAll(attrs);
-			}
-		}
-		return set;
-	}
+    public List<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException {
+        Iterator i = methodDefinitionSources.iterator();
+        while (i.hasNext()) {
+            MethodDefinitionSource s = (MethodDefinitionSource) i.next();
+            List<ConfigAttribute> result = s.getAttributes(object);
+            if (result != null) {
+                return result;
+            }
+        }
+        return null;
+    }
 
-	public boolean supports(Class clazz) {
-		Iterator i = methodDefinitionSources.iterator();
-		while (i.hasNext()) {
-			MethodDefinitionSource s = (MethodDefinitionSource) i.next();
-			if (s.supports(clazz)) {
-				return true;
-			}
-		}
-		return false;
-	}
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
+        Set set = new HashSet();
+        Iterator i = methodDefinitionSources.iterator();
+        while (i.hasNext()) {
+            MethodDefinitionSource s = (MethodDefinitionSource) i.next();
+            Collection<List<? extends ConfigAttribute>> attrs = s.getConfigAttributeDefinitions();
+            if (attrs != null) {
+                set.addAll(attrs);
+            }
+        }
+        return set;
+    }
 
-	public void setMethodDefinitionSources(List methodDefinitionSources) {
-		Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required");
-		this.methodDefinitionSources = methodDefinitionSources;
-	}
+    public boolean supports(Class clazz) {
+        Iterator i = methodDefinitionSources.iterator();
+        while (i.hasNext()) {
+            MethodDefinitionSource s = (MethodDefinitionSource) i.next();
+            if (s.supports(clazz)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public void setMethodDefinitionSources(List methodDefinitionSources) {
+        Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required");
+        this.methodDefinitionSources = methodDefinitionSources;
+    }
 }

+ 2 - 8
core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java

@@ -219,14 +219,8 @@ public class MapBasedMethodDefinitionSource extends AbstractFallbackMethodDefini
      *
      * @return the attributes explicitly defined against this bean
      */
-    public Collection getConfigAttributeDefinitions() {
-        List<ConfigAttributeDefinition> configAttrs = new ArrayList<ConfigAttributeDefinition>(methodMap.values().size());
-
-        for(List<? extends ConfigAttribute> attrList : methodMap.values()) {
-            configAttrs.add(new ConfigAttributeDefinition(attrList));
-        }
-
-        return configAttrs;
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
+        return methodMap.values();
     }
 
     /**

+ 3 - 2
core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java

@@ -16,8 +16,9 @@
 package org.springframework.security.intercept.method;
 
 import java.lang.reflect.Method;
+import java.util.List;
 
-import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.intercept.ObjectDefinitionSource;
 
 
@@ -29,5 +30,5 @@ import org.springframework.security.intercept.ObjectDefinitionSource;
  * @version $Id$
  */
 public interface MethodDefinitionSource extends ObjectDefinitionSource {
-    public ConfigAttributeDefinition getAttributes(Method method, Class targetClass);
+    public List<ConfigAttribute> getAttributes(Method method, Class targetClass);
 }

+ 3 - 1
core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java

@@ -36,9 +36,11 @@ import java.util.LinkedHashMap;
 
 /**
  * Property editor to assist with the setup of a {@link MethodDefinitionSource}.
- * <p>The class creates and populates a {@link MapBasedMethodDefinitionSource}.</p>
+ * <p>
+ * The class creates and populates a {@link MapBasedMethodDefinitionSource}.
  *
  * @author Ben Alex
+ * @deprecated use method annotations or the protect-pointcut support from the namespace
  * @version $Id$
  */
 public class MethodDefinitionSourceEditor extends PropertyEditorSupport {

+ 5 - 2
core/src/main/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluator.java

@@ -15,8 +15,11 @@
 
 package org.springframework.security.intercept.method;
 
+import java.util.List;
+
 import org.springframework.security.AccessDeniedException;
 import org.springframework.security.Authentication;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 
 import org.springframework.security.intercept.AbstractSecurityInterceptor;
@@ -60,7 +63,7 @@ public class MethodInvocationPrivilegeEvaluator implements InitializingBean {
         Assert.notNull(mi, "MethodInvocation required");
         Assert.notNull(mi.getMethod(), "MethodInvocation must provide a non-null getMethod()");
 
-        ConfigAttributeDefinition attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(mi);
+        List<? extends ConfigAttribute> attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(mi);
 
         if (attrs == null) {
             if (securityInterceptor.isRejectPublicInvocations()) {
@@ -76,7 +79,7 @@ public class MethodInvocationPrivilegeEvaluator implements InitializingBean {
         }
 
         try {
-            securityInterceptor.getAccessDecisionManager().decide(authentication, mi, attrs);
+            securityInterceptor.getAccessDecisionManager().decide(authentication, mi, new ConfigAttributeDefinition(attrs));
         } catch (AccessDeniedException unauthorized) {
             if (logger.isDebugEnabled()) {
                 logger.debug(mi.toString() + " denied for " + authentication.toString(), unauthorized);

+ 24 - 26
core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java

@@ -15,12 +15,15 @@
 
 package org.springframework.security.intercept.web;
 
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.util.UrlMatcher;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.LinkedHashMap;
 import java.util.Iterator;
@@ -28,7 +31,6 @@ import java.util.HashMap;
 import java.util.Set;
 import java.util.HashSet;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 
 
@@ -47,7 +49,7 @@ import java.util.Collections;
  * <p>
  * If URLs are registered for a particular HTTP method using
  * {@link #addSecureUrl(String, String, ConfigAttributeDefinition)}, then the method-specific matches will take
- * precedence over any URLs which are registered without an HTTP method. 
+ * precedence over any URLs which are registered without an HTTP method.
  *
  * @author Ben Alex
  * @author Luke Taylor
@@ -78,30 +80,27 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
     DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher) {
         this.urlMatcher = urlMatcher;
     }
-    
+
     /**
      * Builds the internal request map from the supplied map. The key elements should be of type {@link RequestKey},
-     * which contains a URL path and an optional HTTP method (may be null). The path stored in the key will depend on 
+     * which contains a URL path and an optional HTTP method (may be null). The path stored in the key will depend on
      * the type of the supplied UrlMatcher.
-     * 
+     *
      * @param urlMatcher typically an ant or regular expression matcher.
      * @param requestMap order-preserving map of <RequestKey, ConfigAttributeDefinition>.
      */
-    public DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher, LinkedHashMap requestMap) {
+    public DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher,
+            LinkedHashMap<RequestKey, List<? extends ConfigAttribute>> requestMap) {
         this.urlMatcher = urlMatcher;
 
-        Iterator iterator = requestMap.entrySet().iterator();
-
-        while (iterator.hasNext()) {
-            Map.Entry entry = (Map.Entry) iterator.next();
-            RequestKey reqKey = (RequestKey) entry.getKey();
-            addSecureUrl(reqKey.getUrl(), reqKey.getMethod(), (ConfigAttributeDefinition) entry.getValue());
+        for (Map.Entry<RequestKey, List<? extends ConfigAttribute>> entry : requestMap.entrySet()) {
+            addSecureUrl(entry.getKey().getUrl(), entry.getKey().getMethod(), entry.getValue());
         }
     }
 
     //~ Methods ========================================================================================================
 
-    void addSecureUrl(String pattern, ConfigAttributeDefinition attr) {
+    void addSecureUrl(String pattern, List<? extends ConfigAttribute> attr) {
         addSecureUrl(pattern, null, attr);
     }
 
@@ -111,7 +110,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
      * to the request map and will be passed back to the <tt>UrlMatcher</tt> when iterating through the map to find
      * a match for a particular URL.
      */
-    void addSecureUrl(String pattern, String method, ConfigAttributeDefinition attr) {
+    void addSecureUrl(String pattern, String method, List<? extends ConfigAttribute> attr) {
         Map mapToUse = getRequestMapForHttpMethod(method);
 
         mapToUse.put(urlMatcher.compile(pattern), attr);
@@ -145,11 +144,11 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
         return methodRequestmap;
     }
 
-    public Collection getConfigAttributeDefinitions() {
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
         return Collections.unmodifiableCollection(getRequestMap().values());
     }
 
-    public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException {
+    public List<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException {
         if ((object == null) || !this.supports(object.getClass())) {
             throw new IllegalArgumentException("Object must be a FilterInvocation");
         }
@@ -160,7 +159,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
         return lookupAttributes(url, method);
     }
 
-    protected ConfigAttributeDefinition lookupAttributes(String url) {
+    protected List<? extends ConfigAttribute> lookupAttributes(String url) {
         return lookupAttributes(url, null);
     }
 
@@ -179,14 +178,14 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
      * @return the <code>ConfigAttributeDefinition</code> that applies to the specified <code>FilterInvocation</code>
      * or null if no match is foud
      */
-    public ConfigAttributeDefinition lookupAttributes(String url, String method) {
+    public List<ConfigAttribute> 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("?");
 
             if (firstQuestionMarkIndex != -1) {
                 url = url.substring(0, firstQuestionMarkIndex);
-            }            
+            }
         }
 
         if (urlMatcher.requiresLowerCaseUrl()) {
@@ -197,7 +196,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
             }
         }
 
-        ConfigAttributeDefinition attributes = null;
+        List<ConfigAttribute> attributes = null;
 
         Map methodSpecificMap = (Map) httpMethodMap.get(method);
 
@@ -212,20 +211,19 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation
         return attributes;
     }
 
-    private ConfigAttributeDefinition lookupUrlInMap(Map requestMap, String url) {
-        Iterator entries = requestMap.entrySet().iterator();
+    private List<ConfigAttribute> lookupUrlInMap(Map<RequestKey, List<ConfigAttribute>> requestMap,
+            String url) {
 
-        while (entries.hasNext()) {
-            Map.Entry entry = (Map.Entry) entries.next();
+        for (Map.Entry<RequestKey, List<ConfigAttribute>> entry : requestMap.entrySet()) {
             Object p = entry.getKey();
-            boolean matched = urlMatcher.pathMatchesUrl(p, url);
+            boolean matched = urlMatcher.pathMatchesUrl(entry.getKey(), url);
 
             if (logger.isDebugEnabled()) {
                 logger.debug("Candidate is: '" + url + "'; pattern is " + p + "; matched=" + matched);
             }
 
             if (matched) {
-                return (ConfigAttributeDefinition) entry.getValue();
+                return entry.getValue();
             }
         }
 

+ 2 - 5
core/src/main/java/org/springframework/security/intercept/web/FIDSToFilterChainMapConverter.java

@@ -3,7 +3,6 @@ package org.springframework.security.intercept.web;
 import org.springframework.context.ApplicationContext;
 import org.springframework.util.Assert;
 import org.springframework.security.ConfigAttribute;
-import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.util.FilterChainProxy;
 import org.springframework.security.util.UrlMatcher;
 
@@ -38,13 +37,11 @@ public class FIDSToFilterChainMapConverter {
         while (paths.hasNext()) {
             Object entry = paths.next();
             String path = entry instanceof Pattern ? ((Pattern)entry).pattern() : (String)entry;
-            ConfigAttributeDefinition configAttributeDefinition = (ConfigAttributeDefinition) requestMap.get(entry);
+            List<? extends ConfigAttribute> configAttributeDefinition = (List<? extends ConfigAttribute>) requestMap.get(entry);
 
             List filters = new ArrayList();
-            Iterator attributes = configAttributeDefinition.getConfigAttributes().iterator();
 
-            while (attributes.hasNext()) {
-                ConfigAttribute attr = (ConfigAttribute) attributes.next();
+            for(ConfigAttribute attr : configAttributeDefinition) {
                 String filterName = attr.getAttribute();
 
                 Assert.notNull(filterName, "Configuration attribute: '" + attr + "' returned null to the getAttribute() " +

+ 2 - 2
core/src/main/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditor.java

@@ -25,7 +25,7 @@ import org.springframework.security.util.StringSplitUtils;
 import org.springframework.security.util.RegexUrlPathMatcher;
 import org.springframework.security.util.UrlMatcher;
 import org.springframework.security.util.AntUrlPathMatcher;
-import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.SecurityConfig;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -167,7 +167,7 @@ public class FilterInvocationDefinitionSourceEditor extends PropertyEditorSuppor
 
             String[] tokens = StringUtils.commaDelimitedListToStringArray(value);
 
-            urlMap.put(new RequestKey(name), new ConfigAttributeDefinition(tokens));
+            urlMap.put(new RequestKey(name), SecurityConfig.createList(tokens));
         }
 
         DefaultFilterInvocationDefinitionSource fids =

+ 5 - 2
core/src/main/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluator.java

@@ -15,8 +15,11 @@
 
 package org.springframework.security.intercept.web;
 
+import java.util.List;
+
 import org.springframework.security.AccessDeniedException;
 import org.springframework.security.Authentication;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 
 import org.springframework.security.intercept.AbstractSecurityInterceptor;
@@ -53,7 +56,7 @@ public class WebInvocationPrivilegeEvaluator implements InitializingBean {
     public boolean isAllowed(FilterInvocation fi, Authentication authentication) {
         Assert.notNull(fi, "FilterInvocation required");
 
-        ConfigAttributeDefinition attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(fi);
+        List<? extends ConfigAttribute> attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(fi);
 
         if (attrs == null) {
             if (securityInterceptor.isRejectPublicInvocations()) {
@@ -69,7 +72,7 @@ public class WebInvocationPrivilegeEvaluator implements InitializingBean {
         }
 
         try {
-            securityInterceptor.getAccessDecisionManager().decide(authentication, fi, attrs);
+            securityInterceptor.getAccessDecisionManager().decide(authentication, fi, new ConfigAttributeDefinition(attrs));
         } catch (AccessDeniedException unauthorized) {
             if (logger.isDebugEnabled()) {
                 logger.debug(fi.toString() + " denied for " + authentication.toString(), unauthorized);

+ 7 - 12
core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java

@@ -31,6 +31,7 @@ import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Set;
 import java.util.Collection;
 
@@ -59,7 +60,7 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini
         Assert.notNull(filterInvocationDefinitionSource, "filterInvocationDefinitionSource must be specified");
         Assert.notNull(channelDecisionManager, "channelDecisionManager must be specified");
 
-        Collection attrDefs = this.filterInvocationDefinitionSource.getConfigAttributeDefinitions();
+        Collection<List<? extends ConfigAttribute>> attrDefs = this.filterInvocationDefinitionSource.getConfigAttributeDefinitions();
 
         if (attrDefs == null) {
             if (logger.isWarnEnabled()) {
@@ -70,16 +71,10 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini
             return;
         }
 
-        Iterator iter = attrDefs.iterator();
         Set set = new HashSet();
 
-        while (iter.hasNext()) {
-            ConfigAttributeDefinition def = (ConfigAttributeDefinition) iter.next();
-            Iterator attributes = def.getConfigAttributes().iterator();
-
-            while (attributes.hasNext()) {
-                ConfigAttribute attr = (ConfigAttribute) attributes.next();
-
+        for (List<? extends ConfigAttribute> def : attrDefs) {
+            for (ConfigAttribute attr : def) {
                 if (!this.channelDecisionManager.supports(attr)) {
                     set.add(attr);
                 }
@@ -99,14 +94,14 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini
         throws IOException, ServletException {
 
         FilterInvocation fi = new FilterInvocation(request, response, chain);
-        ConfigAttributeDefinition attr = this.filterInvocationDefinitionSource.getAttributes(fi);
+        List<? extends ConfigAttribute> attr = this.filterInvocationDefinitionSource.getAttributes(fi);
 
         if (attr != null) {
             if (logger.isDebugEnabled()) {
-                logger.debug("Request: " + fi.toString() + "; ConfigAttributes: " + attr.toString());
+                logger.debug("Request: " + fi.toString() + "; ConfigAttributes: " + attr);
             }
 
-            channelDecisionManager.decide(fi, attr);
+            channelDecisionManager.decide(fi, new ConfigAttributeDefinition(attr));
 
             if (fi.getResponse().isCommitted()) {
                 return;

+ 0 - 138
core/src/test/java/org/springframework/security/ConfigAttributeEditorTests.java

@@ -1,138 +0,0 @@
-/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.security;
-
-import junit.framework.TestCase;
-
-import java.util.ArrayList;
-import java.util.Iterator;
-
-
-/**
- * Tests {@link ConfigAttributeEditor} and associated {@link ConfigAttributeDefinition}.
- *
- * @author Ben Alex
- * @version $Id$
- */
-public class ConfigAttributeEditorTests extends TestCase {
-    private static final String[] ATTRIBUTES = new String[] {"A", "B"};
-
-    //~ Constructors ===================================================================================================
-
-    public ConfigAttributeEditorTests() {
-        super();
-    }
-
-    public ConfigAttributeEditorTests(String arg0) {
-        super(arg0);
-    }
-
-    //~ Methods ========================================================================================================
-
-    public void testCorrectOperation() {
-        ConfigAttributeEditor editor = new ConfigAttributeEditor();
-        editor.setAsText("HELLO,DOCTOR,NAME,YESTERDAY,TOMORROW");
-
-        ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue();
-        Iterator iter = result.getConfigAttributes().iterator();
-        int position = 0;
-
-        while (iter.hasNext()) {
-            position++;
-            iter.next();
-        }
-
-        assertEquals(5, position);
-
-        assertEquals(5, result.getConfigAttributes().size());
-
-        assertTrue(result.contains(new SecurityConfig("HELLO")));
-        assertTrue(result.contains(new SecurityConfig("TOMORROW")));
-        assertFalse(result.contains(new SecurityConfig("FOOBAR")));
-    }
-
-    public void testEmptyStringReturnsNull() {
-        ConfigAttributeEditor editor = new ConfigAttributeEditor();
-        editor.setAsText("");
-
-        ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue();
-        assertTrue(result == null);
-    }
-
-    public void testEqualsHandlingWhenDifferentObjectTypes() {
-        ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES);
-
-        assertTrue(!def1.equals("A_STRING"));
-    }
-
-    public void testEqualsHandlingWhenExactlyEqual() {
-        ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES);
-        ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(ATTRIBUTES);
-
-        assertEquals(def1, def2);
-    }
-
-    public void testEqualsHandlingWhenOrderingNotEqual() {
-        ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES);
-        ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(new String[] {"B", "A"});
-
-        assertFalse(def1.equals(def2));
-    }
-
-    public void testEqualsHandlingWhenTestObjectHasNoAttributes() {
-        ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES);
-        ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(new String[] {});
-
-        assertFalse(def1.equals(def2));
-        assertFalse(def2.equals(def1));
-    }
-
-    public void testNullReturnsNull() {
-        ConfigAttributeEditor editor = new ConfigAttributeEditor();
-        editor.setAsText(null);
-
-        ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue();
-        assertTrue(result == null);
-    }
-
-    public void testStripsTrailingAndLeadingSpaces() {
-        ConfigAttributeEditor editor = new ConfigAttributeEditor();
-        editor.setAsText("  HELLO, DOCTOR,NAME,  YESTERDAY ,TOMORROW ");
-
-        ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue();
-        Iterator iter = result.getConfigAttributes().iterator();
-
-        ArrayList list = new ArrayList();
-
-        while (iter.hasNext()) {
-            list.add(iter.next());
-        }
-
-        assertEquals("HELLO", ((ConfigAttribute) list.get(0)).getAttribute());
-        assertEquals("DOCTOR", ((ConfigAttribute) list.get(1)).getAttribute());
-        assertEquals("NAME", ((ConfigAttribute) list.get(2)).getAttribute());
-        assertEquals("YESTERDAY", ((ConfigAttribute) list.get(3)).getAttribute());
-        assertEquals("TOMORROW", ((ConfigAttribute) list.get(4)).getAttribute());
-    }
-
-    public void testToString() {
-        ConfigAttributeEditor editor = new ConfigAttributeEditor();
-        editor.setAsText("KOALA,KANGAROO,EMU,WOMBAT");
-
-        ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue();
-        assertEquals("[KOALA, KANGAROO, EMU, WOMBAT]", result.toString());
-    }
-}

+ 1 - 2
core/src/test/java/org/springframework/security/annotation/ExpressionProtectedBusinessServiceImpl.java

@@ -28,7 +28,7 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService {
     public void someUserMethod2() {
     }
 
-    @PreFilter(filterTarget="someList", value="filterObject == name or filterObject == 'sam'")
+    @PreFilter(filterTarget="someList", value="filterObject == authentication.name or filterObject == 'sam'")
     @PostFilter("filterObject == 'bob'")
     public List methodReturningAList(List someList) {
         return someList;
@@ -38,7 +38,6 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService {
         return new ArrayList();
     }
 
-    @PreFilter(filterTarget="someArray", value="filterObject == name or filterObject == 'sam'")
     @PostFilter("filterObject == 'bob'")
     public Object[] methodReturningAnArray(Object[] someArray) {
         return someArray;

+ 26 - 49
core/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java

@@ -15,18 +15,22 @@
 
 package org.springframework.security.annotation;
 
-import java.lang.reflect.AccessibleObject;
-import java.lang.reflect.Method;
+import static org.junit.Assert.*;
+
+import java.util.List;
 
 import junit.framework.TestCase;
 
-import org.aopalliance.intercept.MethodInvocation;
-import org.springframework.security.ConfigAttributeDefinition;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.security.ConfigAttribute;
+import org.springframework.security.SecurityConfig;
 import org.springframework.security.annotation.test.Entity;
 import org.springframework.security.annotation.test.PersonServiceImpl;
 import org.springframework.security.annotation.test.Service;
 import org.springframework.security.intercept.method.MapBasedMethodDefinitionSource;
 import org.springframework.security.intercept.method.MethodDefinitionSourceEditor;
+import org.springframework.security.intercept.method.MockMethodInvocation;
 
 
 /**
@@ -35,9 +39,17 @@ import org.springframework.security.intercept.method.MethodDefinitionSourceEdito
  * @author Ben Alex
  * @version $Id$
  */
-public class MethodDefinitionSourceEditorTigerTests extends TestCase {
-    //~ Methods ========================================================================================================
+public class MethodDefinitionSourceEditorTigerTests {
+    private MockMethodInvocation makeUpper;
+    private MockMethodInvocation makeLower;
+
+    @Before
+    public void createMethodInvocations() throws Exception {
+        makeUpper = new MockMethodInvocation(new PersonServiceImpl(), Service.class,"makeUpperCase", Entity.class);
+        makeLower = new MockMethodInvocation(new PersonServiceImpl(), Service.class,"makeLowerCase", Entity.class);
+    }
 
+    @Test
     public void testConcreteClassInvocations() throws Exception {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText(
@@ -48,64 +60,29 @@ public class MethodDefinitionSourceEditorTigerTests extends TestCase {
         MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(3, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(Service.class,
-                "makeLowerCase", new Class[]{Entity.class}, new PersonServiceImpl()));
-        ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE");
+        List<? extends ConfigAttribute> returnedMakeLower = map.getAttributes(makeLower);
+        List<? extends ConfigAttribute> expectedMakeLower = SecurityConfig.createList("ROLE_FROM_INTERFACE");
         assertEquals(expectedMakeLower, returnedMakeLower);
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(Service.class,
-                "makeUpperCase", new Class[]{Entity.class}, new PersonServiceImpl()));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION"});
+        List<? extends ConfigAttribute> returnedMakeUpper = map.getAttributes(makeUpper);
+        List<? extends ConfigAttribute> expectedMakeUpper = SecurityConfig.createList(new String[]{"ROLE_FROM_IMPLEMENTATION"});
         assertEquals(expectedMakeUpper, returnedMakeUpper);
     }
 
+    @Test
     public void testBridgeMethodResolution() throws Exception {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText(
                 "org.springframework.security.annotation.test.PersonService.makeUpper*=ROLE_FROM_INTERFACE\r\n" +
                 "org.springframework.security.annotation.test.ServiceImpl.makeUpper*=ROLE_FROM_ABSTRACT\r\n" +
-        		"org.springframework.security.annotation.test.PersonServiceImpl.makeUpper*=ROLE_FROM_PSI");
+                "org.springframework.security.annotation.test.PersonServiceImpl.makeUpper*=ROLE_FROM_PSI");
 
         MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(3, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(Service.class,
-                "makeUpperCase", new Class[]{Entity.class}, new PersonServiceImpl()));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_PSI"});
+        List<? extends ConfigAttribute> returnedMakeUpper = map.getAttributes(makeUpper);
+        List<? extends ConfigAttribute> expectedMakeUpper = SecurityConfig.createList("ROLE_FROM_PSI");
         assertEquals(expectedMakeUpper, returnedMakeUpper);
     }
 
-    //~ Inner Classes ==================================================================================================
-
-    private class MockMethodInvocation implements MethodInvocation {
-        private Method method;
-        private Object targetObject;
-
-        public MockMethodInvocation(Class clazz, String methodName, Class[] parameterTypes, Object targetObject)
-            throws NoSuchMethodException {
-            this.method = clazz.getMethod(methodName, parameterTypes);
-            this.targetObject = targetObject;
-        }
-
-        public Object[] getArguments() {
-            return null;
-        }
-
-        public Method getMethod() {
-            return method;
-        }
-
-        public AccessibleObject getStaticPart() {
-            return null;
-        }
-
-        public Object getThis() {
-            return targetObject;
-        }
-
-        public Object proceed() throws Throwable {
-            return null;
-        }
-    }
-
 }

+ 4 - 1
core/src/test/java/org/springframework/security/config/FilterInvocationDefinitionSourceParserTests.java

@@ -2,12 +2,15 @@ package org.springframework.security.config;
 
 import static org.junit.Assert.*;
 
+import java.util.List;
+
 import org.junit.After;
 import org.junit.Test;
 import org.springframework.context.support.AbstractXmlApplicationContext;
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.SecurityConfig;
 import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource;
@@ -41,7 +44,7 @@ public class FilterInvocationDefinitionSourceParserTests {
                 "   <intercept-url pattern='/**' access='ROLE_A'/>" +
                 "</filter-invocation-definition-source>");
         DefaultFilterInvocationDefinitionSource fids = (DefaultFilterInvocationDefinitionSource) appContext.getBean("fids");
-        ConfigAttributeDefinition cad = fids.getAttributes(createFilterInvocation("/anything", "GET"));
+        List<? extends ConfigAttribute> cad = fids.getAttributes(createFilterInvocation("/anything", "GET"));
         assertTrue(cad.contains(new SecurityConfig("ROLE_A")));
     }
 

+ 1 - 1
core/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java

@@ -214,7 +214,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
     }
 
     @Test
-    public void preAndPostFilterAnnotationsWorkWithArrays() {
+    public void prePostFilterAnnotationWorksWithArrays() {
         setContext(
                 "<global-method-security spel-annotations='enabled'/>" +
                 "<b:bean id='target' class='org.springframework.security.annotation.ExpressionProtectedBusinessServiceImpl'/>" +

+ 8 - 7
core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java

@@ -16,6 +16,7 @@ import org.springframework.context.support.AbstractXmlApplicationContext;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockHttpSession;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.MockAuthenticationEntryPoint;
 import org.springframework.security.MockFilterChain;
@@ -221,12 +222,12 @@ public class HttpSecurityBeanDefinitionParserTests {
         FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR);
 
         FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource();
-        ConfigAttributeDefinition attrDef = fids.getAttributes(createFilterinvocation("/Secure", null));
-        assertEquals(2, attrDef.getConfigAttributes().size());
+        List<? extends 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")));
         attrDef = fids.getAttributes(createFilterinvocation("/secure", null));
-        assertEquals(1, attrDef.getConfigAttributes().size());
+        assertEquals(1, attrDef.size());
         assertTrue(attrDef.contains(new SecurityConfig("ROLE_C")));
     }
 
@@ -241,8 +242,8 @@ public class HttpSecurityBeanDefinitionParserTests {
 
         FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR);
         FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource();
-        ConfigAttributeDefinition attrs = fids.getAttributes(createFilterinvocation("/secure", "POST"));
-        assertEquals(2, attrs.getConfigAttributes().size());
+        List<? extends ConfigAttribute> attrs = fids.getAttributes(createFilterinvocation("/secure", "POST"));
+        assertEquals(2, attrs.size());
         assertTrue(attrs.contains(new SecurityConfig("ROLE_A")));
         assertTrue(attrs.contains(new SecurityConfig("ROLE_B")));
     }
@@ -656,8 +657,8 @@ public class HttpSecurityBeanDefinitionParserTests {
         FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR);
 
         FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource();
-        ConfigAttributeDefinition attrDef = fids.getAttributes(createFilterinvocation("/someurl", null));
-        assertEquals(1, attrDef.getConfigAttributes().size());
+        List<? extends ConfigAttribute> attrDef = fids.getAttributes(createFilterinvocation("/someurl", null));
+        assertEquals(1, attrDef.size());
         assertTrue(attrDef.contains(new SecurityConfig("ROLE_B")));
     }
 

+ 26 - 15
core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java

@@ -12,8 +12,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.annotation.ExpressionProtectedBusinessServiceImpl;
-import org.springframework.security.expression.support.AbstractExpressionBasedMethodConfigAttribute;
-import org.springframework.security.expression.support.MethodExpressionVoter;
 import org.springframework.security.providers.TestingAuthenticationToken;
 import org.springframework.security.util.SimpleMethodInvocation;
 import org.springframework.security.vote.AccessDecisionVoter;
@@ -22,7 +20,10 @@ public class MethodExpressionVoterTests {
     private TestingAuthenticationToken joe = new TestingAuthenticationToken("joe", "joespass", "blah");
     private MethodInvocation miStringArgs;
     private MethodInvocation miListArg;
+    private MethodInvocation miArrayArg;
     private List listArg;
+    private Object[] arrayArg;
+    private MethodExpressionVoter am = new MethodExpressionVoter();
 
     @Before
     public void setUp() throws Exception {
@@ -30,42 +31,52 @@ public class MethodExpressionVoterTests {
                 String.class, String.class);
         miStringArgs = new SimpleMethodInvocation(new Object(), m, new String[] {"joe", "arg2Value"});
         m = ExpressionProtectedBusinessServiceImpl.class.getMethod("methodReturningAList", List.class);
-        listArg = new ArrayList(Arrays.asList("joe", "bob"));
+        listArg = new ArrayList(Arrays.asList("joe", "bob", "sam"));
         miListArg = new SimpleMethodInvocation(new Object(), m, new Object[] {listArg});
+        m = ExpressionProtectedBusinessServiceImpl.class.getMethod("methodReturningAnArray", Object[].class);
+        arrayArg = new Object[] {"joe", "bob", "sam"};
+        miArrayArg = new SimpleMethodInvocation(new Object(), m, new Object[] {arrayArg});
     }
 
     @Test
     public void hasRoleExpressionAllowsUserWithRole() throws Exception {
-        MethodExpressionVoter am = new MethodExpressionVoter();
-        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute(null, null, "hasRole('blah')"));
-
+        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute(null, null, "hasRole('blah')"));
         assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miStringArgs, cad));
     }
 
     @Test
     public void hasRoleExpressionDeniesUserWithoutRole() throws Exception {
-        MethodExpressionVoter am = new MethodExpressionVoter();
-        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute(null, null, "hasRole('joedoesnt')"));
-
+        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute(null, null, "hasRole('joedoesnt')"));
         assertEquals(AccessDecisionVoter.ACCESS_DENIED, am.vote(joe, miStringArgs, cad));
     }
 
     @Test
     public void matchingArgAgainstAuthenticationNameIsSuccessful() throws Exception {
-        MethodExpressionVoter am = new MethodExpressionVoter();
-        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute(null, null, "(#userName == name) and (name == 'joe')"));
-
+        // userName is an argument name of this method
+        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute(null, null, "(#userName == principal) and (principal == 'joe')"));
         assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miStringArgs, cad));
     }
 
     @Test
     public void accessIsGrantedIfNoPreAuthorizeAttributeIsUsed() throws Exception {
-        MethodExpressionVoter am = new MethodExpressionVoter();
-        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute("(name == 'jim')", "someList", null));
-
+        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute("(filterObject == 'jim')", "someList", null));
         assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miListArg, cad));
         // All objects should have been removed, because the expression is always false
         assertEquals(0, listArg.size());
     }
 
+    @Test(expected=IllegalArgumentException.class)
+    public void arraysCannotBePrefiltered() throws Exception {
+        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute("(filterObject == 'jim')", "someArray", null));
+        am.vote(joe, miArrayArg, cad);
+    }
+
+    @Test
+    public void listPreFilteringIsSuccessful() throws Exception {
+        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute("(filterObject == 'joe' or filterObject == 'sam')", "someList", null));
+        am.vote(joe, miListArg, cad);
+        assertEquals("joe and sam should still be in the list", 2, listArg.size());
+        assertEquals("joe", listArg.get(0));
+        assertEquals("sam", listArg.get(1));
+    }
 }

+ 3 - 3
core/src/test/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSourceTests.java

@@ -35,7 +35,7 @@ public class MapBasedMethodDefinitionSourceTests {
     public void wildcardedMatchIsOverwrittenByMoreSpecificMatch() {
         mds.addSecureMethod(MockService.class, "some*", ROLE_A);
         mds.addSecureMethod(MockService.class, "someMethod*", ROLE_B);
-        assertEquals(ROLE_B, mds.getAttributes(someMethodInteger, MockService.class).getConfigAttributes());
+        assertEquals(ROLE_B, mds.getAttributes(someMethodInteger, MockService.class));
     }
 
     @Test
@@ -43,8 +43,8 @@ public class MapBasedMethodDefinitionSourceTests {
         mds.addSecureMethod(MockService.class, someMethodInteger, ROLE_A);
         mds.addSecureMethod(MockService.class, someMethodString, ROLE_B);
 
-        assertEquals(ROLE_A, mds.getAttributes(someMethodInteger, MockService.class).getConfigAttributes());
-        assertEquals(ROLE_B, mds.getAttributes(someMethodString, MockService.class).getConfigAttributes());
+        assertEquals(ROLE_A, mds.getAttributes(someMethodInteger, MockService.class));
+        assertEquals(ROLE_B, mds.getAttributes(someMethodString, MockService.class));
     }
 
     private class MockService {

+ 21 - 21
core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java

@@ -17,10 +17,12 @@ package org.springframework.security.intercept.method;
 
 import junit.framework.TestCase;
 
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.ITargetObject;
 import org.springframework.security.MockJoinPoint;
 import org.springframework.security.OtherTargetObject;
+import org.springframework.security.SecurityConfig;
 import org.springframework.security.TargetObject;
 
 import org.aopalliance.intercept.MethodInvocation;
@@ -29,6 +31,7 @@ import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Method;
 
 import java.util.Iterator;
+import java.util.List;
 
 
 /**
@@ -63,10 +66,9 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         Method method = clazz.getMethod("countLength", new Class[] {String.class});
         MockJoinPoint joinPoint = new MockJoinPoint(new TargetObject(), method);
 
-        ConfigAttributeDefinition returnedCountLength = map.getAttributes(joinPoint);
+        List<? extends ConfigAttribute> returnedCountLength = map.getAttributes(joinPoint);
 
-        ConfigAttributeDefinition expectedCountLength =
-                new ConfigAttributeDefinition(new String[] {"ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"});
+        List<? extends ConfigAttribute> expectedCountLength = SecurityConfig.createList("ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY");
         assertEquals(expectedCountLength, returnedCountLength);
     }
 
@@ -116,20 +118,20 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(6, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new OtherTargetObject()));
-        ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE");
+        List<? extends ConfigAttribute> returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new OtherTargetObject()));
+        List<? extends ConfigAttribute> expectedMakeLower = SecurityConfig.createList("ROLE_FROM_INTERFACE");
         assertEquals(expectedMakeLower, returnedMakeLower);
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeUpperCase", new Class[] {String.class}, new OtherTargetObject()));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION"});
+        List<? extends ConfigAttribute> returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeUpperCase", new Class[] {String.class}, new OtherTargetObject()));
+        List<? extends ConfigAttribute> expectedMakeUpper = SecurityConfig.createList("ROLE_FROM_IMPLEMENTATION");
         assertEquals(expectedMakeUpper, returnedMakeUpper);
 
-        ConfigAttributeDefinition returnedComputeHashCode = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "computeHashCode", new Class[] {String.class}, new OtherTargetObject()));
-        ConfigAttributeDefinition expectedComputeHashCode = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_OTO"});
+        List<? extends ConfigAttribute> returnedComputeHashCode = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "computeHashCode", new Class[] {String.class}, new OtherTargetObject()));
+        List<? extends ConfigAttribute> expectedComputeHashCode = SecurityConfig.createList("ROLE_FROM_OTO");
         assertEquals(expectedComputeHashCode, returnedComputeHashCode);
 
         returnedComputeHashCode = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "computeHashCode", new Class[] {String.class}, new TargetObject()));
-        expectedComputeHashCode = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_TO"});
+        expectedComputeHashCode = SecurityConfig.createList("ROLE_FROM_TO");
         assertEquals(expectedComputeHashCode, returnedComputeHashCode);
     }
 
@@ -175,19 +177,19 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(14, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+        List<? extends ConfigAttribute> returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
                     "makeLowerCase", new Class[] {String.class}, new TargetObject()));
-        ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_LOWER");
+        List<? extends ConfigAttribute> expectedMakeLower = SecurityConfig.createList("ROLE_LOWER");
         assertEquals(expectedMakeLower, returnedMakeLower);
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+        List<? extends ConfigAttribute> returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
                     "makeUpperCase", new Class[] {String.class}, new TargetObject()));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition("ROLE_UPPER");
+        List<? extends ConfigAttribute> expectedMakeUpper = SecurityConfig.createList("ROLE_UPPER");
         assertEquals(expectedMakeUpper, returnedMakeUpper);
 
-        ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+        List<? extends ConfigAttribute> returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
                     "countLength", new Class[] {String.class}, new TargetObject()));
-        ConfigAttributeDefinition expectedCountLength = new ConfigAttributeDefinition("ROLE_GENERAL");
+        List<? extends ConfigAttribute> expectedCountLength = SecurityConfig.createList("ROLE_GENERAL");
         assertEquals(expectedCountLength, returnedCountLength);
     }
 
@@ -197,7 +199,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
 
         MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
 
-        ConfigAttributeDefinition configAttributeDefinition = map.getAttributes(new MockMethodInvocation(
+        List<? extends ConfigAttribute> configAttributeDefinition = map.getAttributes(new MockMethodInvocation(
                     ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new TargetObject()));
         assertNull(configAttributeDefinition);
     }
@@ -216,11 +218,9 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
 
         MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
 
-        ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+        List<? extends ConfigAttribute> returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
                     "countLength", new Class[] {String.class}, new TargetObject()));
-        ConfigAttributeDefinition expectedCountLength = new ConfigAttributeDefinition(
-                new String[] {"ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"});
-        assertEquals(expectedCountLength, returnedCountLength);
+        assertEquals(SecurityConfig.createList("ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"), returnedCountLength);
     }
 
     //~ Inner Classes ==================================================================================================

+ 11 - 9
core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java

@@ -17,13 +17,15 @@ package org.springframework.security.intercept.method;
 
 import org.aopalliance.intercept.MethodInvocation;
 import org.aspectj.lang.JoinPoint;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.SecurityConfig;
 
 import java.lang.reflect.Method;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Vector;
-import java.util.Collection;
 
 
 /**
@@ -43,38 +45,38 @@ public class MockMethodDefinitionSource implements MethodDefinitionSource {
         returnACollection = returnACollectionWhenRequested;
         list = new Vector();
 
-        ConfigAttributeDefinition def1 = new ConfigAttributeDefinition("MOCK_LOWER");
+        List<? extends ConfigAttribute> def1 = SecurityConfig.createList("MOCK_LOWER");
         list.add(def1);
 
         if (includeInvalidAttributes) {
-            ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(new String[] {"MOCK_LOWER","INVALID_ATTRIBUTE"});
+            List<? extends ConfigAttribute> def2 = SecurityConfig.createList("MOCK_LOWER","INVALID_ATTRIBUTE");
             list.add(def2);
         }
 
-        ConfigAttributeDefinition def3 = new ConfigAttributeDefinition(new String[] {"MOCK_UPPER", "RUN_AS_"});
+        List<? extends ConfigAttribute> def3 = SecurityConfig.createList("MOCK_UPPER", "RUN_AS_");
         list.add(def3);
 
         if (includeInvalidAttributes) {
-            ConfigAttributeDefinition def4 = new ConfigAttributeDefinition(new String[] {"MOCK_SOMETHING", "ANOTHER_INVALID"});
+            List<? extends ConfigAttribute> def4 = SecurityConfig.createList("MOCK_SOMETHING", "ANOTHER_INVALID");
             list.add(def4);
         }
     }
 
     //~ Methods ========================================================================================================
 
-    public Collection getConfigAttributeDefinitions() {
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
         if (returnACollection) {
             return list;
         } else {
             return null;
-        }	
+        }
     }
 
-    public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException {
+    public List<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException {
         throw new UnsupportedOperationException("mock method not implemented");
     }
 
-    public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) {
+    public List<ConfigAttribute> getAttributes(Method method, Class targetClass) {
         throw new UnsupportedOperationException("mock method not implemented");
     }
 

+ 5 - 5
core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java

@@ -49,6 +49,7 @@ import org.springframework.context.support.ClassPathXmlApplicationContext;
 import java.lang.reflect.Method;
 
 import java.util.Collection;
+import java.util.List;
 
 
 /**
@@ -359,8 +360,7 @@ public class MethodSecurityInterceptorTests extends TestCase {
         }
     }
 
-    public void testValidationFailsIfInvalidAttributePresented()
-        throws Exception {
+    public void testValidationFailsIfInvalidAttributePresented() throws Exception {
         MethodSecurityInterceptor si = new MethodSecurityInterceptor();
         si.setAccessDecisionManager(new MockAccessDecisionManager());
         si.setAuthenticationManager(new MockAuthenticationManager());
@@ -447,11 +447,11 @@ public class MethodSecurityInterceptorTests extends TestCase {
     }
 
     private class MockObjectDefinitionSourceWhichOnlySupportsStrings implements MethodDefinitionSource {
-        public Collection getConfigAttributeDefinitions() {
+        public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
             return null;
         }
 
-        public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) {
+        public List<ConfigAttribute> getAttributes(Method method, Class targetClass) {
             throw new UnsupportedOperationException("mock method not implemented");
         }
 
@@ -463,7 +463,7 @@ public class MethodSecurityInterceptorTests extends TestCase {
             }
         }
 
-        public ConfigAttributeDefinition getAttributes(Object object) {
+        public List<ConfigAttribute> getAttributes(Object object) {
             throw new UnsupportedOperationException("mock method not implemented");
         }
     }

+ 33 - 37
core/src/test/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSourceTests.java

@@ -15,17 +15,22 @@
 
 package org.springframework.security.intercept.web;
 
-import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.MockFilterChain;
-import org.springframework.security.util.AntUrlPathMatcher;
-import org.springframework.security.util.InMemoryXmlApplicationContext;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
-import org.springframework.mock.web.MockHttpServletRequest;
-import org.springframework.mock.web.MockHttpServletResponse;
+import java.util.List;
 
-import org.junit.Test;
 import org.junit.Before;
-import static org.junit.Assert.*;
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.ConfigAttribute;
+import org.springframework.security.MockFilterChain;
+import org.springframework.security.SecurityConfig;
+import org.springframework.security.util.AntUrlPathMatcher;
+import org.springframework.security.util.InMemoryXmlApplicationContext;
 
 /**
  * Tests parts of {@link DefaultFilterInvocationDefinitionSource} not tested by {@link
@@ -35,7 +40,8 @@ import static org.junit.Assert.*;
  * @version $Id$
  */
 public class DefaultFilterInvocationDefinitionSourceTests {
-    DefaultFilterInvocationDefinitionSource map;
+    private DefaultFilterInvocationDefinitionSource map;
+    private List<ConfigAttribute> def = SecurityConfig.createList("ROLE_ONE");
 
     //~ Methods ========================================================================================================
     @Before
@@ -51,13 +57,11 @@ public class DefaultFilterInvocationDefinitionSourceTests {
 
     @Test
     public void lookupNotRequiringExactMatchSuccessIfNotMatching() {
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/secure/super/**", def);
 
         FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null);
 
-        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
-        assertEquals(def, response);
+        assertEquals(def, map.lookupAttributes(fi.getRequestUrl()));
     }
 
     /**
@@ -65,12 +69,11 @@ public class DefaultFilterInvocationDefinitionSourceTests {
      */
     @Test
     public void lookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() {
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/SeCuRE/super/**", def);
 
         FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null);
 
-        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
+        List<? extends ConfigAttribute> response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(def, response);
     }
 
@@ -78,75 +81,69 @@ public class DefaultFilterInvocationDefinitionSourceTests {
     @Test
     public void lookupRequiringExactMatchFailsIfNotMatching() {
         map = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher(false));
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/secure/super/**", def);
 
         FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null);
 
-        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
+        List<? extends ConfigAttribute> response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(null, response);
     }
 
     @Test
     public void lookupRequiringExactMatchIsSuccessful() {
         map = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher(false));
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/SeCurE/super/**", def);
 
         FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null);
 
-        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
+        List<? extends ConfigAttribute> response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(def, response);
     }
 
     @Test
     public void lookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() {
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/someAdminPage.html**", def);
 
         FilterInvocation fi = createFilterInvocation("/someAdminPage.html?a=/test", null);
 
-        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
+        List<? extends ConfigAttribute> response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(def, response); // see SEC-161 (it should truncate after ? sign)
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void unknownHttpMethodIsRejected() {
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/someAdminPage.html**", "UNKNOWN", def);
     }
 
     @Test
     public void httpMethodLookupSucceeds() {
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/somepage**", "GET", def);
 
         FilterInvocation fi = createFilterInvocation("/somepage", "GET");
-        ConfigAttributeDefinition attrs = map.getAttributes(fi);
+        List<? extends ConfigAttribute> attrs = map.getAttributes(fi);
         assertEquals(def, attrs);
     }
 
     @Test
     public void requestWithDifferentHttpMethodDoesntMatch() {
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
         map.addSecureUrl("/somepage**", "GET", def);
 
         FilterInvocation fi = createFilterInvocation("/somepage", null);
-        ConfigAttributeDefinition attrs = map.getAttributes(fi);
+        List<? extends ConfigAttribute> attrs = map.getAttributes(fi);
         assertNull(attrs);
     }
 
     @Test
     public void httpMethodSpecificUrlTakesPrecedence() {
         // Even though this is added before the method-specific def, the latter should match
-        ConfigAttributeDefinition allMethodDef = new ConfigAttributeDefinition("ROLE_ONE");
-        map.addSecureUrl("/**", null, allMethodDef);
+        List<? extends ConfigAttribute> allMethodDef = def;
+        map.addSecureUrl("/**", null, def);
 
-        ConfigAttributeDefinition postOnlyDef = new ConfigAttributeDefinition("ROLE_TWO");
+        List<? extends ConfigAttribute> postOnlyDef = SecurityConfig.createList("ROLE_TWO");
         map.addSecureUrl("/somepage**", "POST", postOnlyDef);
 
         FilterInvocation fi = createFilterInvocation("/somepage", "POST");
-        ConfigAttributeDefinition attrs = map.getAttributes(fi);
+        List<? extends ConfigAttribute> attrs = map.getAttributes(fi);
         assertEquals(postOnlyDef, attrs);
     }
 
@@ -154,13 +151,12 @@ public class DefaultFilterInvocationDefinitionSourceTests {
      * Check fixes for SEC-321
      */
     @Test
-    public void extraQuestionMarkStillMatches() {        
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE");
+    public void extraQuestionMarkStillMatches() {
         map.addSecureUrl("/someAdminPage.html*", def);
 
         FilterInvocation fi = createFilterInvocation("/someAdminPage.html?x=2/aa?y=3", null);
 
-        ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl());
+        List<? extends ConfigAttribute> response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(def, response);
 
         fi = createFilterInvocation("/someAdminPage.html??", null);
@@ -168,13 +164,13 @@ public class DefaultFilterInvocationDefinitionSourceTests {
         response = map.lookupAttributes(fi.getRequestUrl());
         assertEquals(def, response);
     }
-    
+
     @Test
     public void xmlMapConfigurationIsSuccessful() {
         InMemoryXmlApplicationContext context = new InMemoryXmlApplicationContext(
         "<b:bean id='fids' class='org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource'>" +
         "    <b:constructor-arg>" +
-        "        <b:bean class='org.springframework.security.util.AntUrlPathMatcher'/>" +        
+        "        <b:bean class='org.springframework.security.util.AntUrlPathMatcher'/>" +
         "    </b:constructor-arg>" +
         "    <b:constructor-arg>" +
         "        <b:map>" +
@@ -193,11 +189,11 @@ public class DefaultFilterInvocationDefinitionSourceTests {
         "    </b:constructor-arg>" +
         "</b:bean>"
         );
-        
+
         DefaultFilterInvocationDefinitionSource fids = (DefaultFilterInvocationDefinitionSource) context.getBean("fids");
-        ConfigAttributeDefinition cad = fids.lookupAttributes("/anything", "GET");
+        List<? extends ConfigAttribute> cad = fids.lookupAttributes("/anything", "GET");
         assertNotNull(cad);
-        assertEquals(1, cad.getConfigAttributes().size());
+        assertEquals(1, cad.size());
         context.close();
     }
 

+ 14 - 17
core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java

@@ -17,8 +17,9 @@ package org.springframework.security.intercept.web;
 
 import junit.framework.TestCase;
 
-import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.MockFilterChain;
+import org.springframework.security.SecurityConfig;
 import org.springframework.security.util.RegexUrlPathMatcher;
 import org.springframework.security.util.AntUrlPathMatcher;
 
@@ -26,6 +27,7 @@ import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 
 import java.util.Iterator;
+import java.util.List;
 import java.util.regex.PatternSyntaxException;
 
 
@@ -164,7 +166,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null);
         httpRequest.setServletPath("/totally/different/path/index.html");
 
-        ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest,
+        List<? extends ConfigAttribute> returned = map.getAttributes(new FilterInvocation(httpRequest,
                     new MockHttpServletResponse(), new MockFilterChain()));
 
         assertEquals(null, returned);
@@ -197,13 +199,10 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null);
         httpRequest.setServletPath("/secure/super/very_secret.html");
 
-        ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest,
+        List<? extends ConfigAttribute> returned = map.getAttributes(new FilterInvocation(httpRequest,
                     new MockHttpServletResponse(), new MockFilterChain()));
 
-        ConfigAttributeDefinition expected = new ConfigAttributeDefinition(
-                new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"});
-
-        assertEquals(expected, returned);
+        assertEquals(SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"), returned);
     }
 
     public void testOrderOfEntriesIsPreservedOrderB() {
@@ -216,11 +215,10 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null);
         httpRequest.setServletPath("/secure/super/very_secret.html");
 
-        ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest,
+        List<? extends ConfigAttribute> returned = map.getAttributes(new FilterInvocation(httpRequest,
                     new MockHttpServletResponse(), new MockFilterChain()));
-        ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_SUPERVISOR", "ROLE_TELLER"});
 
-        assertEquals(expected, returned);
+        assertEquals(SecurityConfig.createList("ROLE_SUPERVISOR", "ROLE_TELLER"), returned);
     }
 
     public void testSingleUrlParsingWithRegularExpressions() throws Exception {
@@ -232,9 +230,9 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null);
         httpRequest.setServletPath("/secure/super/very_secret.html");
 
-        ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest,
+        List<? extends ConfigAttribute> returned = map.getAttributes(new FilterInvocation(httpRequest,
                     new MockHttpServletResponse(), new MockFilterChain()));
-        ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"});
+        List<? extends ConfigAttribute> expected = SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE");
 
         assertEquals(expected, returned);
     }
@@ -248,11 +246,10 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null);
         httpRequest.setServletPath("/secure/super/very_secret.html");
 
-        ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest,
+        List<? extends ConfigAttribute> returned = map.getAttributes(new FilterInvocation(httpRequest,
                     new MockHttpServletResponse(), new MockFilterChain()));
-        ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"});
 
-        assertEquals(expected, returned);
+        assertEquals(SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"), returned);
     }
 
     public void testWhitespaceAndCommentsAndLinesWithoutEqualsSignsAreIgnored() {
@@ -292,9 +289,9 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase {
         MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null);
         httpRequest.setServletPath("/secure/super/very_secret.html");
 
-        ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest,
+        List<? extends ConfigAttribute> returned = map.getAttributes(new FilterInvocation(httpRequest,
                     new MockHttpServletResponse(), new MockFilterChain()));
-        ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"});
+        List<? extends ConfigAttribute> expected = SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE");
 
         assertEquals(expected, returned);
     }

+ 10 - 11
core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java

@@ -29,6 +29,7 @@ import org.springframework.security.MockAuthenticationManager;
 import org.springframework.security.MockRunAsManager;
 import org.springframework.security.RunAsManager;
 import org.springframework.security.MockApplicationEventPublisher;
+import org.springframework.security.SecurityConfig;
 import org.springframework.security.util.AntUrlPathMatcher;
 import org.springframework.security.util.RegexUrlPathMatcher;
 import org.springframework.security.context.SecurityContextHolder;
@@ -40,6 +41,7 @@ import java.io.IOException;
 
 import java.util.Collection;
 import java.util.LinkedHashMap;
+import java.util.List;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -145,8 +147,7 @@ public class FilterSecurityInterceptorTests extends TestCase {
         interceptor.setApplicationEventPublisher(new MockApplicationEventPublisher(true));
 
         // Setup a mock config attribute definition
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("MOCK_OK");
-        MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", def);
+        MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", "MOCK_OK");
         interceptor.setObjectDefinitionSource(mockSource);
 
         // Setup our expectation that the filter chain will be invoked, as access is granted
@@ -187,7 +188,6 @@ public class FilterSecurityInterceptorTests extends TestCase {
      * We just test invocation works in a success event. There is no need to test  access denied events as the
      * abstract parent enforces that logic, which is extensively tested separately.
      *
-     * @throws Throwable DOCUMENT ME!
      */
     public void testSuccessfulInvocation() throws Throwable {
         // Setup the FilterSecurityInterceptor
@@ -198,8 +198,7 @@ public class FilterSecurityInterceptorTests extends TestCase {
         interceptor.setApplicationEventPublisher(new MockApplicationEventPublisher(true));
 
         // Setup a mock config attribute definition
-        ConfigAttributeDefinition def = new ConfigAttributeDefinition("MOCK_OK");
-        MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", def);
+        MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", "MOCK_OK");
         interceptor.setObjectDefinitionSource(mockSource);
 
         // Setup our expectation that the filter chain will be invoked, as access is granted
@@ -224,7 +223,7 @@ public class FilterSecurityInterceptorTests extends TestCase {
         LinkedHashMap reqMap = new LinkedHashMap();
         reqMap.put(new RequestKey("/secure/**", null), new ConfigAttributeDefinition(new String[] {"ROLE_USER"}));
         DefaultFilterInvocationDefinitionSource fids
-                = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher());        
+                = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher());
 
         FilterSecurityInterceptor filter = new FilterSecurityInterceptor();
         filter.setObjectDefinitionSource(fids);
@@ -260,15 +259,15 @@ public class FilterSecurityInterceptorTests extends TestCase {
     }
 
     private class MockFilterInvocationDefinitionMap implements FilterInvocationDefinitionSource {
-        private ConfigAttributeDefinition toReturn;
+        private List<ConfigAttribute> toReturn;
         private String servletPath;
 
-        public MockFilterInvocationDefinitionMap(String servletPath, ConfigAttributeDefinition toReturn) {
+        public MockFilterInvocationDefinitionMap(String servletPath, String... toReturn) {
             this.servletPath = servletPath;
-            this.toReturn = toReturn;
+            this.toReturn = SecurityConfig.createList(toReturn);
         }
 
-        public ConfigAttributeDefinition getAttributes(Object object)
+        public List<ConfigAttribute> getAttributes(Object object)
             throws IllegalArgumentException {
             FilterInvocation fi = (FilterInvocation) object;
 
@@ -279,7 +278,7 @@ public class FilterSecurityInterceptorTests extends TestCase {
             }
         }
 
-        public Collection getConfigAttributeDefinitions() {
+        public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
             return null;
         }
 

+ 4 - 3
core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java

@@ -15,12 +15,13 @@
 
 package org.springframework.security.intercept.web;
 
+import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.util.AntUrlPathMatcher;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Vector;
-import java.util.Collection;
 
 
 /**
@@ -61,7 +62,7 @@ public class MockFilterInvocationDefinitionSource extends DefaultFilterInvocatio
 
     //~ Methods ========================================================================================================
 
-    public Collection getConfigAttributeDefinitions() {
+    public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
         if (returnAnIterator) {
             return list;
         } else {
@@ -69,7 +70,7 @@ public class MockFilterInvocationDefinitionSource extends DefaultFilterInvocatio
         }
     }
 
-    public ConfigAttributeDefinition lookupAttributes(String url, String method) {
+    public List<ConfigAttribute> lookupAttributes(String url, String method) {
         throw new UnsupportedOperationException("mock method not implemented");
     }
 }

+ 0 - 4
core/src/test/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluatorTests.java

@@ -47,10 +47,6 @@ public class WebInvocationPrivilegeEvaluatorTests extends TestCase {
 
     //~ Methods ========================================================================================================
 
-    public static void main(String[] args) {
-        junit.textui.TestRunner.run(WebInvocationPrivilegeEvaluatorTests.class);
-    }
-
     private FilterSecurityInterceptor makeFilterSecurityInterceptor() {
         ApplicationContext context = new ClassPathXmlApplicationContext(
                 "org/springframework/security/intercept/web/applicationContext.xml");

+ 14 - 26
core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java

@@ -19,6 +19,7 @@ import junit.framework.TestCase;
 
 import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.SecurityConfig;
 
 import org.springframework.security.intercept.web.FilterInvocation;
 import org.springframework.security.intercept.web.FilterInvocationDefinitionSource;
@@ -28,9 +29,9 @@ import org.springframework.mock.web.MockHttpServletResponse;
 
 import java.io.IOException;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Vector;
-import java.util.Collection;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -51,8 +52,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition("MOCK");
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "MOCK");
         filter.setFilterInvocationDefinitionSource(fids);
 
         try {
@@ -80,9 +80,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SUPPORTS_MOCK_ONLY"));
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition("SUPPORTS_MOCK_ONLY");
-
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SUPPORTS_MOCK_ONLY");
 
         filter.setFilterInvocationDefinitionSource(fids);
 
@@ -94,8 +92,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SUPPORTS_MOCK_ONLY"));
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition(new String[] {"SUPPORTS_MOCK_ONLY", "INVALID_ATTRIBUTE"});
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SUPPORTS_MOCK_ONLY", "INVALID_ATTRIBUTE");
 
         filter.setFilterInvocationDefinitionSource(fids);
 
@@ -111,9 +108,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(true, "SOME_ATTRIBUTE"));
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition("SOME_ATTRIBUTE");
-
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SOME_ATTRIBUTE");
 
         filter.setFilterInvocationDefinitionSource(fids);
 
@@ -132,9 +127,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SOME_ATTRIBUTE"));
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition("SOME_ATTRIBUTE");
-
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SOME_ATTRIBUTE");
 
         filter.setFilterInvocationDefinitionSource(fids);
 
@@ -154,9 +147,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "NOT_USED"));
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition("NOT_USED");
-
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "NOT_USED");
 
         filter.setFilterInvocationDefinitionSource(fids);
 
@@ -196,9 +187,7 @@ public class ChannelProcessingFilterTests extends TestCase {
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "MOCK"));
         assertTrue(filter.getChannelDecisionManager() != null);
 
-        ConfigAttributeDefinition attr = new ConfigAttributeDefinition("MOCK");
-
-        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, false);
+        MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", false, "MOCK");
 
         filter.setFilterInvocationDefinitionSource(fids);
         assertTrue(filter.getFilterInvocationDefinitionSource() != null);
@@ -257,18 +246,17 @@ public class ChannelProcessingFilterTests extends TestCase {
     }
 
     private class MockFilterInvocationDefinitionMap implements FilterInvocationDefinitionSource {
-        private ConfigAttributeDefinition toReturn;
+        private List<ConfigAttribute> toReturn;
         private String servletPath;
         private boolean provideIterator;
 
-        public MockFilterInvocationDefinitionMap(String servletPath, ConfigAttributeDefinition toReturn,
-            boolean provideIterator) {
+        public MockFilterInvocationDefinitionMap(String servletPath, boolean provideIterator, String... toReturn) {
             this.servletPath = servletPath;
-            this.toReturn = toReturn;
+            this.toReturn = SecurityConfig.createList(toReturn);
             this.provideIterator = provideIterator;
         }
 
-        public ConfigAttributeDefinition getAttributes(Object object)
+        public List<ConfigAttribute> getAttributes(Object object)
             throws IllegalArgumentException {
             FilterInvocation fi = (FilterInvocation) object;
 
@@ -279,7 +267,7 @@ public class ChannelProcessingFilterTests extends TestCase {
             }
         }
 
-        public Collection getConfigAttributeDefinitions() {
+        public Collection<List<? extends ConfigAttribute>> getConfigAttributeDefinitions() {
             if (!provideIterator) {
                 return null;
             }

+ 2 - 11
core/src/test/java/org/springframework/security/util/FilterChainProxyTests.java

@@ -18,6 +18,7 @@ package org.springframework.security.util;
 import org.springframework.security.ConfigAttribute;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.MockFilterConfig;
+import org.springframework.security.SecurityConfig;
 import org.springframework.security.context.HttpSessionContextIntegrationFilter;
 import org.springframework.security.intercept.web.MockFilterInvocationDefinitionSource;
 import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource;
@@ -76,10 +77,8 @@ public class FilterChainProxyTests {
         FilterChainProxy filterChainProxy = new FilterChainProxy();
         filterChainProxy.setApplicationContext(new StaticApplicationContext());
 
-        ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new MockConfigAttribute());
-
         LinkedHashMap map = new LinkedHashMap();
-        map.put(new RequestKey("/**"), cad);
+        map.put(new RequestKey("/**"), SecurityConfig.createList(null));
         DefaultFilterInvocationDefinitionSource fids =
                 new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher(), map);
 
@@ -226,12 +225,4 @@ public class FilterChainProxyTests {
         assertTrue(filter.isWasDoFiltered());
         assertTrue(filter.isWasDestroyed());
     }
-
-    //~ Inner Classes ==================================================================================================
-
-    private class MockConfigAttribute implements ConfigAttribute {
-        public String getAttribute() {
-            return null;
-        }
-    }
 }