Переглянути джерело

SEC-99/428/429/563: Various refactoring of method security metadata support.

Ben Alex 17 роки тому
батько
коміт
9a4977ebd1
33 змінених файлів з 935 додано та 1015 видалено
  1. 75 0
      core-tiger/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java
  2. 0 140
      core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributes.java
  3. 54 0
      core-tiger/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java
  4. 0 137
      core-tiger/src/main/java/org/springframework/security/annotation/SecurityAnnotationAttributes.java
  5. 104 0
      core-tiger/src/test/java/org/springframework/security/annotation/Jsr250MethodDefinitionSourceTests.java
  6. 0 114
      core-tiger/src/test/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributesTests.java
  7. 27 34
      core-tiger/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java
  8. 35 86
      core-tiger/src/test/java/org/springframework/security/annotation/SecuredMethodDefinitionSourceTests.java
  9. 32 6
      core/src/main/java/org/springframework/security/ConfigAttributeDefinition.java
  10. 15 14
      core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java
  11. 2 2
      core/src/main/java/org/springframework/security/config/BeanIds.java
  12. 28 22
      core/src/main/java/org/springframework/security/config/InterceptMethodsBeanDefinitionDecorator.java
  13. 200 0
      core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java
  14. 132 74
      core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java
  15. 21 86
      core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionAttributes.java
  16. 7 2
      core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java
  17. 3 3
      core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java
  18. 7 11
      core/src/main/java/org/springframework/security/intercept/method/aopalliance/MethodDefinitionSourceAdvisor.java
  19. 28 10
      core/src/main/java/org/springframework/security/util/MethodInvocationUtils.java
  20. 5 3
      core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java
  21. 12 6
      core/src/test/java/org/springframework/security/MockJoinPoint.java
  22. 0 4
      core/src/test/java/org/springframework/security/OtherTargetObject.java
  23. 1 1
      core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java
  24. 21 194
      core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionAttributesTests.java
  25. 47 34
      core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java
  26. 3 2
      core/src/test/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluatorTests.java
  27. 4 0
      core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java
  28. 6 16
      core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodDefinitionSourceAdvisorTests.java
  29. 5 1
      core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java
  30. 6 9
      core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java
  31. 2 2
      core/src/test/java/org/springframework/security/vote/BasicAclEntryVoterTests.java
  32. 2 2
      core/src/test/resources/org/springframework/security/config/method-security.xml
  33. 51 0
      sandbox/other/src/main/java/org/springframework/security/AspectJParsingTests.java

+ 75 - 0
core-tiger/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java

@@ -0,0 +1,75 @@
+/* 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.annotation;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.RolesAllowed;
+
+import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.intercept.method.AbstractFallbackMethodDefinitionSource;
+
+
+/**
+ * Sources method security metadata from major JSR 250 security annotations. 
+ *
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class Jsr250MethodDefinitionSource extends AbstractFallbackMethodDefinitionSource {
+
+	protected ConfigAttributeDefinition findAttributes(Class clazz) {
+		return processAnnotations(clazz.getAnnotations());
+	}
+
+	protected ConfigAttributeDefinition findAttributes(Method method, Class targetClass) {
+		return processAnnotations(AnnotationUtils.getAnnotations(method));
+	}
+	
+    public Collection getConfigAttributeDefinitions() {
+        return null;
+    }
+    
+	private ConfigAttributeDefinition processAnnotations(Annotation[] annotations) {
+		if (annotations == null || annotations.length == 0) {
+			return null;
+		}
+		for (Annotation a: annotations) {
+			if (a instanceof DenyAll) {
+				return new ConfigAttributeDefinition(Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE);
+			}
+			if (a instanceof PermitAll) {
+				return new ConfigAttributeDefinition(Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE);
+			}
+			if (a instanceof RolesAllowed) {
+				RolesAllowed ra = (RolesAllowed) a;
+				List attributes = new ArrayList();
+				for (String allowed : ra.value()) {
+					attributes.add(new Jsr250SecurityConfig(allowed));
+				}
+				return new ConfigAttributeDefinition(attributes);
+			}
+		}
+		return null;
+	}
+}

+ 0 - 140
core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributes.java

@@ -1,140 +0,0 @@
-package org.springframework.security.annotation;
-
-import org.springframework.security.SecurityConfig;
-import org.springframework.metadata.Attributes;
-import org.springframework.core.annotation.AnnotationUtils;
-
-import javax.annotation.security.PermitAll;
-import javax.annotation.security.RolesAllowed;
-import javax.annotation.security.DenyAll;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.ArrayList;
-import java.lang.reflect.Method;
-import java.lang.reflect.Field;
-import java.lang.annotation.Annotation;
-
-/**
- * Java 5 Annotation {@link Attributes} metadata implementation used for secure method interception using
- * the security anotations defined in JSR-250.
- * <p>
- * This <code>Attributes</code> implementation will return security configuration for classes described using the
- * Java JEE 5 annotations (<em>DenyAll</em>, <em>PermitAll</em> and <em>RolesAllowed</em>).
- * <p>
- *
- * @author Mark St.Godard
- * @author Usama Rashwan
- * @author Luke Taylor
- * @since 2.0
- *
- * @see javax.annotation.security.RolesAllowed
- */
-
-public class Jsr250SecurityAnnotationAttributes implements Attributes {
-    
-    //~ Methods ========================================================================================================
-
-    /**
-     * Get the <code>RolesAllowed</code> attributes for a given target class.
-     * This method will return an empty Collection because the call to getAttributes(method) will override the class
-     * annotation.
-     *
-     * @param target The target Object
-     * @return Empty Collection of <code>SecurityConfig</code>
-     *
-     * @see Attributes#getAttributes
-     */
-    public Collection<SecurityConfig> getAttributes(Class target) {
-        return new HashSet<SecurityConfig>();
-    }
-
-    /**
-     * Get the attributes for a given target method, acording to JSR-250 precedence rules.
-     *
-     * @param method The target method
-     * @return Collection of <code>SecurityConfig</code>
-     * @see Attributes#getAttributes
-     */
-    public Collection<SecurityConfig> getAttributes(Method method) {
-        ArrayList<SecurityConfig> attributes = new ArrayList<SecurityConfig>();
-
-        if (AnnotationUtils.getAnnotation(method, DenyAll.class) != null) {
-            attributes.add(Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE);
-
-            return attributes;
-        }
-
-        if (AnnotationUtils.getAnnotation(method, PermitAll.class) != null) {
-            attributes.add(Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE);
-
-            return attributes;
-        }
-
-        RolesAllowed rolesAllowed = AnnotationUtils.getAnnotation(method, RolesAllowed.class);
-        
-        if (rolesAllowed != null) {
-            for (String role : rolesAllowed.value()) {
-                attributes.add(new Jsr250SecurityConfig(role));
-            }
-
-            return attributes;
-        }
-
-        // Now check the class-level attributes:
-        if (method.getDeclaringClass().getAnnotation(DenyAll.class) != null) {
-            attributes.add(Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE);
-
-            return attributes;
-        }
-
-        if (method.getDeclaringClass().getAnnotation(PermitAll.class) != null) {
-            attributes.add(Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE);
-
-            return attributes;
-        }
-
-        rolesAllowed = method.getDeclaringClass().getAnnotation(RolesAllowed.class);
-
-        if (rolesAllowed != null) {
-            for (String role : rolesAllowed.value()) {
-                attributes.add(new Jsr250SecurityConfig(role));
-            }
-        }
-
-        return attributes;
-    }
-
-    protected Collection<SecurityConfig> populateSecurityConfigWithRolesAllowed (Annotation[] annotations) {
-        Set<SecurityConfig> attributes = new HashSet<SecurityConfig>();
-    	for (Annotation annotation : annotations) {
-            // check for RolesAllowed annotations
-            if (annotation instanceof RolesAllowed) {
-            	RolesAllowed attr = (RolesAllowed) annotation;
-
-                for (String auth : attr.value()) {
-                    attributes.add(new SecurityConfig(auth));
-                }
-
-                break;
-            }
-        }
-    	return attributes;
-    }
-
-    public Collection getAttributes(Class clazz, Class filter) {
-        throw new UnsupportedOperationException();
-    }
-
-    public Collection getAttributes(Method method, Class clazz) {
-        throw new UnsupportedOperationException();
-    }
-
-    public Collection getAttributes(Field field) {
-        throw new UnsupportedOperationException();
-    }
-
-    public Collection getAttributes(Field field, Class clazz) {
-        throw new UnsupportedOperationException();
-    }
-}

+ 54 - 0
core-tiger/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java

@@ -0,0 +1,54 @@
+/* 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.annotation;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.Collection;
+
+import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.intercept.method.AbstractFallbackMethodDefinitionSource;
+
+
+/**
+ * Sources method security metadata from Spring Security's {@link Secured} annotation. 
+ *
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class SecuredMethodDefinitionSource extends AbstractFallbackMethodDefinitionSource {
+
+	protected ConfigAttributeDefinition findAttributes(Class clazz) {
+		return processAnnotation(clazz.getAnnotation(Secured.class));
+	}
+
+	protected ConfigAttributeDefinition findAttributes(Method method, Class targetClass) {
+		return processAnnotation(AnnotationUtils.findAnnotation(method, Secured.class));
+	}
+	
+    public Collection getConfigAttributeDefinitions() {
+        return null;
+    }
+    
+	private ConfigAttributeDefinition processAnnotation(Annotation a) {
+		if (a == null || !(a instanceof Secured)) {
+			return null;
+		}
+		Secured secured = (Secured) a;
+		return new ConfigAttributeDefinition(secured.value());
+	}
+}

+ 0 - 137
core-tiger/src/main/java/org/springframework/security/annotation/SecurityAnnotationAttributes.java

@@ -1,137 +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.annotation;
-
-import org.springframework.security.SecurityConfig;
-
-import org.springframework.metadata.Attributes;
-
-import org.springframework.core.annotation.AnnotationUtils;
-
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
-
-
-/**
- * Java 5 Annotation <code>Attributes</code> metadata implementation used for  secure method interception.<p>This
- * <code>Attributes</code> implementation will return security  configuration for classes described using the
- * <code>Secured</code> Java 5 annotation.
- * <p>
- * The <code>SecurityAnnotationAttributes</code> implementation can be used to configure a
- * <code>MethodDefinitionAttributes</code> and  <code>MethodSecurityInterceptor</code> bean definition (see below).
- * <p>
- * For example:
- * <pre>
- * &lt;bean id="attributes" class="org.springframework.security.annotation.SecurityAnnotationAttributes"/>
- * &lt;bean id="objectDefinitionSource"
- *     class="org.springframework.security.intercept.method.MethodDefinitionAttributes">
- *         &lt;property name="attributes">&lt;ref local="attributes"/>&lt;/property>
- * &lt;/bean>
- * &lt;bean id="securityInterceptor"
- *     class="org.springframework.security.intercept.method.aopalliance.MethodSecurityInterceptor">
- *      . . .
- *      &lt;property name="objectDefinitionSource">&lt;ref local="objectDefinitionSource"/>&lt;/property>
- * &lt;/bean>
- * </pre>
- * <p>
- * These security annotations are similiar to the Commons Attributes approach, however they are using Java 5
- * language-level metadata support.
- *
- * @author Mark St.Godard
- * @version $Id$
- *
- * @see org.springframework.security.annotation.Secured
- */
-public class SecurityAnnotationAttributes implements Attributes {
-    //~ Methods ========================================================================================================
-
-    /**
-     * Get the <code>Secured</code> attributes for a given target class.
-     *
-     * @param target The target method
-     *
-     * @return Collection of <code>SecurityConfig</code>
-     *
-     * @see Attributes#getAttributes
-     */
-    public Collection getAttributes(Class target) {
-        Set<SecurityConfig> attributes = new HashSet<SecurityConfig>();
-
-        for (Annotation annotation : target.getAnnotations()) {
-            // check for Secured annotations
-            if (annotation instanceof Secured) {
-                Secured attr = (Secured) annotation;
-
-                for (String auth : attr.value()) {
-                    attributes.add(new SecurityConfig(auth));
-                }
-
-                break;
-            }
-        }
-
-        return attributes;
-    }
-
-    /**
-     * Get the <code>Secured</code> attributes for a given target method.
-     *
-     * @param method The target method
-     *
-     * @return Collection of <code>SecurityConfig</code>
-     *
-     * @see Attributes#getAttributes
-     */
-    public Collection getAttributes(Method method) {
-        Set<SecurityConfig> attributes = new HashSet<SecurityConfig>();
-        Annotation[] annotations = AnnotationUtils.getAnnotations(method);
-
-        for (Annotation annotation : annotations) {
-            // check for Secured annotations
-            if (annotation instanceof Secured) {
-                Secured attr = (Secured) annotation;
-
-                for (String auth : attr.value()) {
-                    attributes.add(new SecurityConfig(auth));
-                }
-
-                break;
-            }
-        }
-
-        return attributes;
-    }
-
-    public Collection getAttributes(Class clazz, Class filter) {
-        throw new UnsupportedOperationException();
-    }
-
-    public Collection getAttributes(Method method, Class clazz) {
-        throw new UnsupportedOperationException();
-    }
-
-    public Collection getAttributes(Field field) {
-        throw new UnsupportedOperationException();
-    }
-
-    public Collection getAttributes(Field field, Class clazz) {
-        throw new UnsupportedOperationException();
-    }
-}

+ 104 - 0
core-tiger/src/test/java/org/springframework/security/annotation/Jsr250MethodDefinitionSourceTests.java

@@ -0,0 +1,104 @@
+package org.springframework.security.annotation;
+
+import static org.junit.Assert.assertEquals;
+
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.RolesAllowed;
+
+import junit.framework.Assert;
+
+import org.junit.Test;
+import org.springframework.security.ConfigAttributeDefinition;
+
+/**
+ * @author Luke Taylor
+ * @author Ben Alex
+ * @version $Id$
+ */
+public class Jsr250MethodDefinitionSourceTests {
+	Jsr250MethodDefinitionSource mds = new Jsr250MethodDefinitionSource();
+    A a = new A();
+    UserAllowedClass userAllowed = new UserAllowedClass();
+    DenyAllClass denyAll = new DenyAllClass();
+
+    @Test
+    public void methodWithRolesAllowedHasCorrectAttribute() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(a.getClass().getMethod("adminMethod"), null);
+        assertEquals(1, accessAttributes.getConfigAttributes().size());
+        assertEquals("ADMIN", accessAttributes.getConfigAttributes().iterator().next().toString());
+    }
+
+    @Test
+    public void permitAllMethodHasPermitAllAttribute() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(a.getClass().getMethod("permitAllMethod"), null);
+        assertEquals(1, accessAttributes.getConfigAttributes().size());
+        assertEquals("javax.annotation.security.PermitAll", accessAttributes.getConfigAttributes().iterator().next().toString());
+    }
+
+    @Test
+    public void noRoleMethodHasDenyAllAttributeWithDenyAllClass() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(denyAll.getClass());
+        assertEquals(1, accessAttributes.getConfigAttributes().size());
+        assertEquals("javax.annotation.security.DenyAll", accessAttributes.getConfigAttributes().iterator().next().toString());
+    }
+
+    @Test
+    public void adminMethodHasAdminAttributeWithDenyAllClass() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(denyAll.getClass().getMethod("adminMethod"), null);
+        assertEquals(1, accessAttributes.getConfigAttributes().size());
+        assertEquals("ADMIN", accessAttributes.getConfigAttributes().iterator().next().toString());
+    }
+
+    @Test
+    public void noRoleMethodHasNoAttributes() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(a.getClass().getMethod("noRoleMethod"), null);
+        Assert.assertNull(accessAttributes);
+    }
+    
+    @Test
+    public void classRoleIsAppliedToNoRoleMethod() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(userAllowed.getClass().getMethod("noRoleMethod"), null);
+        Assert.assertNull(accessAttributes);
+    }
+
+    @Test
+    public void methodRoleOverridesClassRole() throws Exception {
+        ConfigAttributeDefinition accessAttributes = mds.findAttributes(userAllowed.getClass().getMethod("adminMethod"), null);
+        assertEquals(1, accessAttributes.getConfigAttributes().size());
+        assertEquals("ADMIN", accessAttributes.getConfigAttributes().iterator().next().toString());
+    }
+
+//~ Inner Classes ======================================================================================================
+
+    public static class A {
+
+        public void noRoleMethod() {}
+
+        @RolesAllowed("ADMIN")
+        public void adminMethod() {}
+
+        @PermitAll
+        public void permitAllMethod() {}
+    }
+
+    @RolesAllowed("USER")
+    public static class UserAllowedClass {
+        public void noRoleMethod() {}
+
+        @RolesAllowed("ADMIN")
+        public void adminMethod() {}        
+    }
+
+    @DenyAll
+    public static class DenyAllClass {
+
+        public void noRoleMethod()  {}
+
+        @RolesAllowed("ADMIN")
+        public void adminMethod() {}        
+    }
+
+
+
+}

+ 0 - 114
core-tiger/src/test/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributesTests.java

@@ -1,114 +0,0 @@
-package org.springframework.security.annotation;
-
-import org.springframework.security.SecurityConfig;
-
-import org.junit.Test;
-import static org.junit.Assert.*;
-
-import java.util.List;
-import java.util.ArrayList;
-
-import javax.annotation.security.RolesAllowed;
-import javax.annotation.security.PermitAll;
-import javax.annotation.security.DenyAll;
-
-/**
- * @author Luke Taylor
- * @version $Id$
- */
-public class Jsr250SecurityAnnotationAttributesTests {
-    Jsr250SecurityAnnotationAttributes attributes = new Jsr250SecurityAnnotationAttributes();
-    A a = new A();
-    UserAllowedClass userAllowed = new UserAllowedClass();
-    DenyAllClass denyAll = new DenyAllClass();
-
-    @Test
-    public void methodWithRolesAllowedHasCorrectAttribute() throws Exception {
-//        Method[] methods = a.getClass().getMethods();
-
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(a.getClass().getMethod("adminMethod")));
-        assertEquals(1, accessAttributes.size());
-        assertEquals("ADMIN", accessAttributes.get(0).getAttribute());
-    }
-
-    @Test
-    public void permitAllMethodHasPermitAllAttribute() throws Exception {
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(a.getClass().getMethod("permitAllMethod")));
-        assertEquals(1, accessAttributes.size());
-        assertEquals("javax.annotation.security.PermitAll", accessAttributes.get(0).getAttribute());
-    }
-
-    @Test
-    public void noRoleMethodHasDenyAllAttributeWithDenyAllClass() throws Exception {
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(denyAll.getClass().getMethod("noRoleMethod")));
-        assertEquals(1, accessAttributes.size());
-        assertEquals("javax.annotation.security.DenyAll", accessAttributes.get(0).getAttribute());
-    }
-
-    @Test
-    public void adminMethodHasAdminAttributeWithDenyAllClass() throws Exception {
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(denyAll.getClass().getMethod("adminMethod")));
-        assertEquals(1, accessAttributes.size());
-        assertEquals("ADMIN", accessAttributes.get(0).getAttribute());
-    }
-
-    @Test
-    public void noRoleMethodHasNoAttributes() throws Exception {
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(a.getClass().getMethod("noRoleMethod")));
-        assertEquals(0, accessAttributes.size());
-    }
-    
-    @Test
-    public void classRoleIsAppliedToNoRoleMethod() throws Exception {
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(userAllowed.getClass().getMethod("noRoleMethod")));
-        assertEquals(1, accessAttributes.size());
-        assertEquals("USER", accessAttributes.get(0).getAttribute());
-    }
-
-    @Test
-    public void methodRoleOverridesClassRole() throws Exception {
-        List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(userAllowed.getClass().getMethod("adminMethod")));
-        assertEquals(1, accessAttributes.size());
-        assertEquals("ADMIN", accessAttributes.get(0).getAttribute());
-    }
-
-//~ Inner Classes ======================================================================================================
-
-    public static class A {
-
-        public void noRoleMethod() {}
-
-        @RolesAllowed("ADMIN")
-        public void adminMethod() {}
-
-        @PermitAll
-        public void permitAllMethod() {}
-    }
-
-    @RolesAllowed("USER")
-    public static class UserAllowedClass {
-        public void noRoleMethod() {}
-
-        @RolesAllowed("ADMIN")
-        public void adminMethod() {}        
-    }
-
-    @DenyAll
-    public static class DenyAllClass {
-
-        public void noRoleMethod()  {}
-
-        @RolesAllowed("ADMIN")
-        public void adminMethod() {}        
-    }
-
-
-
-}

+ 27 - 34
core-tiger/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java

@@ -20,20 +20,17 @@ import java.lang.reflect.Method;
 
 import junit.framework.TestCase;
 
+import org.aopalliance.intercept.MethodInvocation;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.annotation.test.Entity;
-import org.springframework.security.annotation.test.OrganisationService;
-import org.springframework.security.annotation.test.PersonService;
 import org.springframework.security.annotation.test.PersonServiceImpl;
 import org.springframework.security.annotation.test.Service;
-import org.springframework.security.annotation.test.ServiceImpl;
-import org.springframework.security.intercept.method.MethodDefinitionMap;
+import org.springframework.security.intercept.method.MapBasedMethodDefinitionSource;
 import org.springframework.security.intercept.method.MethodDefinitionSourceEditor;
-import org.aopalliance.intercept.MethodInvocation;
 
 
 /**
- * Extra tests to demonstrate generics behaviour with <code>MethodDefinitionMap</code>.
+ * Extra tests to demonstrate generics behaviour with <code>MapBasedMethodDefinitionSource</code>.
  *
  * @author Ben Alex
  * @version $Id$
@@ -41,58 +38,53 @@ import org.aopalliance.intercept.MethodInvocation;
 public class MethodDefinitionSourceEditorTigerTests extends TestCase {
     //~ Methods ========================================================================================================
 
-    public void testConcreteClassInvocationsAlsoReturnDefinitionsAgainstInterface() throws Exception {
+    public void testConcreteClassInvocations() throws Exception {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText(
-                "org.springframework.security.annotation.test.Service.makeLower*=ROLE_FROM_INTERFACE\r\norg.springframework.security.annotation.test.Service.makeUpper*=ROLE_FROM_INTERFACE\r\norg.springframework.security.annotation.test.ServiceImpl.makeUpper*=ROLE_FROM_IMPLEMENTATION");
+                "org.springframework.security.annotation.test.Service.makeLower*=ROLE_FROM_INTERFACE\r\n" +
+                "org.springframework.security.annotation.test.Service.makeUpper*=ROLE_FROM_INTERFACE\r\n" +
+                "org.springframework.security.annotation.test.ServiceImpl.makeUpper*=ROLE_FROM_IMPLEMENTATION");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(3, map.getMethodMapSize());
 
         ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(Service.class,
-                "makeLowerCase", new Class[]{Entity.class}));
+                "makeLowerCase", new Class[]{Entity.class}, new PersonServiceImpl()));
         ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE");
         assertEquals(expectedMakeLower, returnedMakeLower);
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ServiceImpl.class,
-                "makeUpperCase", new Class[]{Entity.class}));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION", "ROLE_FROM_INTERFACE"});
+        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(Service.class,
+                "makeUpperCase", new Class[]{Entity.class}, new PersonServiceImpl()));
+        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION"});
         assertEquals(expectedMakeUpper, returnedMakeUpper);
     }
 
-    public void testGenericsSuperclassDeclarationsAreIncludedWhenSubclassesOverride() throws Exception {
+    public void testBridgeMethodResolution() throws Exception {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText(
-                "org.springframework.security.annotation.test.Service.makeLower*=ROLE_FROM_INTERFACE\r\norg.springframework.security.annotation.test.Service.makeUpper*=ROLE_FROM_INTERFACE\r\norg.springframework.security.annotation.test.ServiceImpl.makeUpper*=ROLE_FROM_IMPLEMENTATION");
+                "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");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(3, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(PersonService.class,
-                "makeLowerCase", new Class[]{Entity.class}));
-        ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE");
-        assertEquals(expectedMakeLower, returnedMakeLower);
-
-        ConfigAttributeDefinition returnedMakeLower2 = map.getAttributes(new MockMethodInvocation(
-                OrganisationService.class, "makeLowerCase", new Class[]{Entity.class}));
-        ConfigAttributeDefinition expectedMakeLower2 = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE");
-        assertEquals(expectedMakeLower2, returnedMakeLower2);
-
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(
-                PersonServiceImpl.class, "makeUpperCase", new Class[]{Entity.class}));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION", "ROLE_FROM_INTERFACE"});
+        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(Service.class,
+                "makeUpperCase", new Class[]{Entity.class}, new PersonServiceImpl()));
+        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_PSI"});
         assertEquals(expectedMakeUpper, returnedMakeUpper);
     }
 
     //~ Inner Classes ==================================================================================================
 
     private class MockMethodInvocation implements MethodInvocation {
-        Method method;
+        private Method method;
+        private Object targetObject;
 
-        public MockMethodInvocation(Class clazz, String methodName, Class[] parameterTypes)
+        public MockMethodInvocation(Class clazz, String methodName, Class[] parameterTypes, Object targetObject)
             throws NoSuchMethodException {
-            System.out.println(clazz + " " + methodName + " " + parameterTypes[0]);
-            method = clazz.getMethod(methodName, parameterTypes);
+            this.method = clazz.getMethod(methodName, parameterTypes);
+            this.targetObject = targetObject;
         }
 
         public Object[] getArguments() {
@@ -108,11 +100,12 @@ public class MethodDefinitionSourceEditorTigerTests extends TestCase {
         }
 
         public Object getThis() {
-            return null;
+            return targetObject;
         }
 
         public Object proceed() throws Throwable {
             return null;
         }
     }
+
 }

+ 35 - 86
core-tiger/src/test/java/org/springframework/security/annotation/SecurityAnnotationAttributesTests.java → core-tiger/src/test/java/org/springframework/security/annotation/SecuredMethodDefinitionSourceTests.java

@@ -14,41 +14,33 @@
  */
 package org.springframework.security.annotation;
 
-import junit.framework.TestCase;
+import java.lang.reflect.Method;
 
-import org.springframework.security.SecurityConfig;
+import junit.framework.TestCase;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-
-import org.springframework.metadata.Attributes;
-
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-
-import java.util.Collection;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.SecurityConfig;
+import org.springframework.util.StringUtils;
 
 
 /**
- * Tests for {@link org.springframework.security.annotation.SecurityAnnotationAttributes}
+ * Tests for {@link org.springframework.security.annotation.SecuredMethodDefinitionSource}
  *
  * @author Mark St.Godard
  * @author Joe Scalise
+ * @author Ben Alex
  * @version $Id$
  */
-public class SecurityAnnotationAttributesTests extends TestCase {
+public class SecuredMethodDefinitionSourceTests extends TestCase {
     //~ Instance fields ================================================================================================
 
-    private Attributes attributes;
-    private Log logger = LogFactory.getLog(SecurityAnnotationAttributesTests.class);
+    private SecuredMethodDefinitionSource mds = new SecuredMethodDefinitionSource();;
+    private Log logger = LogFactory.getLog(SecuredMethodDefinitionSourceTests.class);
 
     //~ Methods ========================================================================================================
 
-    protected void setUp() throws Exception {
-        // create the Annotations impl
-        this.attributes = new SecurityAnnotationAttributes();
-    }
-
     public void testGenericsSuperclassDeclarationsAreIncludedWhenSubclassesOverride() {
         Method method = null;
 
@@ -58,20 +50,19 @@ public class SecurityAnnotationAttributesTests extends TestCase {
             fail("Should be a superMethod called 'someUserMethod3' on class!");
         }
 
-        Collection attrs = this.attributes.getAttributes(method);
+        ConfigAttributeDefinition attrs = this.mds.findAttributes(method, DepartmentServiceImpl.class);
+
+        assertNotNull(attrs);
 
         if (logger.isDebugEnabled()) {
-            logger.debug("attrs: ");
-            logger.debug(attrs);
+            logger.debug("attrs: " + StringUtils.collectionToCommaDelimitedString(attrs.getConfigAttributes()));
         }
 
-        assertNotNull(attrs);
-
         // expect 1 attribute
-        assertTrue("Did not find 1 attribute", attrs.size() == 1);
+        assertTrue("Did not find 1 attribute", attrs.getConfigAttributes().size() == 1);
 
         // should have 1 SecurityConfig
-        for (Object obj : attrs) {
+        for (Object obj : attrs.getConfigAttributes()) {
             assertTrue(obj instanceof SecurityConfig);
 
             SecurityConfig sc = (SecurityConfig) obj;
@@ -86,67 +77,39 @@ public class SecurityAnnotationAttributesTests extends TestCase {
             fail("Should be a superMethod called 'someUserMethod3' on class!");
         }
 
-        System.out.println(superMethod);
+        ConfigAttributeDefinition superAttrs = this.mds.findAttributes(superMethod, DepartmentServiceImpl.class);
 
-        Collection superAttrs = this.attributes.getAttributes(superMethod);
+        assertNotNull(superAttrs);
 
         if (logger.isDebugEnabled()) {
-            logger.debug("superAttrs: ");
-            logger.debug(superAttrs);
+            logger.debug("superAttrs: " + StringUtils.collectionToCommaDelimitedString(superAttrs.getConfigAttributes()));
         }
 
-        assertNotNull(superAttrs);
-
-        // TODO: Enable this part of the test once we can build against Spring 2.0+ and above only (SEC-274)
-        /*
-           // expect 1 attribute
-           assertTrue("Did not find 1 attribute", superAttrs.size() == 1);
-           // should have 1 SecurityConfig
-           for (Object obj : superAttrs) {
-               assertTrue(obj instanceof SecurityConfig);
-               SecurityConfig sc = (SecurityConfig) obj;
-               assertEquals("Found an incorrect role", "ROLE_ADMIN", sc.getAttribute());
-           }
-         */
+        // This part of the test relates to SEC-274
+        // expect 1 attribute
+        assertTrue("Did not find 1 attribute", superAttrs.getConfigAttributes().size() == 1);
+        // should have 1 SecurityConfig
+        for (Object obj : superAttrs.getConfigAttributes()) {
+        	assertTrue(obj instanceof SecurityConfig);
+            SecurityConfig sc = (SecurityConfig) obj;
+            assertEquals("Found an incorrect role", "ROLE_ADMIN", sc.getAttribute());
+        }
     }
 
     public void testGetAttributesClass() {
-        Collection attrs = this.attributes.getAttributes(BusinessService.class);
+    	ConfigAttributeDefinition attrs = this.mds.findAttributes(BusinessService.class);
 
         assertNotNull(attrs);
 
         // expect 1 annotation
-        assertTrue(attrs.size() == 1);
+        assertTrue(attrs.getConfigAttributes().size() == 1);
 
         // should have 1 SecurityConfig
-        SecurityConfig sc = (SecurityConfig) attrs.iterator().next();
+        SecurityConfig sc = (SecurityConfig) attrs.getConfigAttributes().iterator().next();
 
         assertTrue(sc.getAttribute().equals("ROLE_USER"));
     }
 
-    public void testGetAttributesClassClass() {
-        try {
-            this.attributes.getAttributes(BusinessService.class, null);
-            fail("Unsupported method should have thrown an exception!");
-        } catch (UnsupportedOperationException expected) {}
-    }
-
-    public void testGetAttributesField() {
-        try {
-            Field field = null;
-            this.attributes.getAttributes(field);
-            fail("Unsupported method should have thrown an exception!");
-        } catch (UnsupportedOperationException expected) {}
-    }
-
-    public void testGetAttributesFieldClass() {
-        try {
-            Field field = null;
-            this.attributes.getAttributes(field, null);
-            fail("Unsupported method should have thrown an exception!");
-        } catch (UnsupportedOperationException expected) {}
-    }
-
     public void testGetAttributesMethod() {
         Method method = null;
 
@@ -156,18 +119,18 @@ public class SecurityAnnotationAttributesTests extends TestCase {
             fail("Should be a method called 'someUserAndAdminMethod' on class!");
         }
 
-        Collection attrs = this.attributes.getAttributes(method);
+        ConfigAttributeDefinition attrs = this.mds.findAttributes(method, BusinessService.class);
 
         assertNotNull(attrs);
 
         // expect 2 attributes
-        assertTrue(attrs.size() == 2);
+        assertTrue(attrs.getConfigAttributes().size() == 2);
 
         boolean user = false;
         boolean admin = false;
 
         // should have 2 SecurityConfigs
-        for (Object obj : attrs) {
+        for (Object obj : attrs.getConfigAttributes()) {
             assertTrue(obj instanceof SecurityConfig);
 
             SecurityConfig sc = (SecurityConfig) obj;
@@ -182,19 +145,5 @@ public class SecurityAnnotationAttributesTests extends TestCase {
         // expect to have ROLE_USER and ROLE_ADMIN
         assertTrue(user && admin);
     }
-
-    public void testGetAttributesMethodClass() {
-        Method method = null;
-
-        try {
-            method = BusinessService.class.getMethod("someUserAndAdminMethod", new Class[] {});
-        } catch (NoSuchMethodException unexpected) {
-            fail("Should be a method called 'someUserAndAdminMethod' on class!");
-        }
-
-        try {
-            this.attributes.getAttributes(method, null);
-            fail("Unsupported method should have thrown an exception!");
-        } catch (UnsupportedOperationException expected) {}
-    }
+    
 }

+ 32 - 6
core/src/main/java/org/springframework/security/ConfigAttributeDefinition.java

@@ -15,15 +15,14 @@
 
 package org.springframework.security;
 
-import org.springframework.util.Assert;
-
 import java.io.Serializable;
-
-import java.util.Iterator;
-import java.util.List;
-import java.util.Collections;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+import org.springframework.util.Assert;
 
 
 /**
@@ -99,6 +98,33 @@ public class ConfigAttributeDefinition implements Serializable {
 
         this.configAttributes = Collections.unmodifiableList(new ArrayList(configAttributes));
     }
+    
+    /**
+     * Creates a <tt>ConfigAttributeDefinition</tt> by including only those attributes which implement <tt>ConfigAttribute</tt>.
+     * 
+     * @param unfilteredInput a collection of various elements, zero or more which implement <tt>ConfigAttribute</tt> (can also be <tt>null</tt>)
+     * @return a ConfigAttributeDefinition if at least one <tt>ConfigAttribute</tt> was present, or <tt>null</tt> if none implemented it
+     */
+    public static ConfigAttributeDefinition createFiltered(Collection unfilteredInput) {
+    	if (unfilteredInput == null) {
+    		return null;
+    	}
+
+    	List configAttributes = new ArrayList();
+    	Iterator i = unfilteredInput.iterator();
+    	while (i.hasNext()) {
+    		Object element = i.next();
+    		if (element instanceof ConfigAttribute) {
+    			configAttributes.add(element);
+    		}
+    	}
+    	
+    	if (configAttributes.size() == 0) {
+    		return null;
+    	}
+    	
+    	return new ConfigAttributeDefinition(configAttributes);
+    }
 
     //~ Methods ========================================================================================================
 

+ 15 - 14
core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java

@@ -23,8 +23,8 @@ import org.w3c.dom.Element;
  * @version $Id$
  */
 class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser {
-    public static final String SECURITY_ANNOTATION_ATTRIBUTES_CLASS = "org.springframework.security.annotation.SecurityAnnotationAttributes";
-    public static final String JSR_250_SECURITY_ANNOTATION_ATTRIBUTES_CLASS = "org.springframework.security.annotation.Jsr250SecurityAnnotationAttributes";
+    public static final String SECURED_METHOD_DEFINITION_SOURCE_CLASS = "org.springframework.security.annotation.SecuredMethodDefinitionSource";
+    public static final String JSR_250_SECURITY_METHOD_DEFINITION_SOURCE_CLASS = "org.springframework.security.annotation.Jsr250MethodDefinitionSource";
     public static final String JSR_250_VOTER_CLASS = "org.springframework.security.annotation.Jsr250Voter";
     private static final String ATT_ACCESS_MGR = "access-decision-manager-ref";
     private static final String ATT_USE_JSR250 = "jsr250";
@@ -32,10 +32,12 @@ class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser {
     public BeanDefinition parse(Element element, ParserContext parserContext) {
         boolean useJsr250 = "true".equals(element.getAttribute(ATT_USE_JSR250));
         String className = useJsr250 ?
-                JSR_250_SECURITY_ANNOTATION_ATTRIBUTES_CLASS : SECURITY_ANNOTATION_ATTRIBUTES_CLASS;
+                JSR_250_SECURITY_METHOD_DEFINITION_SOURCE_CLASS : SECURED_METHOD_DEFINITION_SOURCE_CLASS;
 
+        String beanId = useJsr250 ? BeanIds.JSR_250_METHOD_DEFINITION_SOURCE : BeanIds.SECURED_METHOD_DEFINITION_SOURCE;
+        
         // Reflectively obtain the Annotation-based ObjectDefinitionSource.
-    	// Reflection is used to avoid a compile-time dependency on SECURITY_ANNOTATION_ATTRIBUTES_CLASS, as this parser is in the Java 4 project whereas the dependency is in the Tiger project.
+    	// Reflection is used to avoid a compile-time dependency on SECURED_METHOD_DEFINITION_SOURCE_CLASS, as this parser is in the Java 4 project whereas the dependency is in the Tiger project.
     	Assert.isTrue(ClassUtils.isPresent(className), "Could not locate class '" + className + "' - please ensure the spring-security-tiger-xxx.jar is in your classpath and you are running Java 5 or above.");
     	Class clazz = null;
 
@@ -47,15 +49,7 @@ class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser {
 
         RootBeanDefinition securityAnnotations = new RootBeanDefinition(clazz);
         securityAnnotations.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
-        parserContext.getRegistry().registerBeanDefinition(BeanIds.SECURITY_ANNOTATION_ATTRIBUTES, securityAnnotations);
-
-        RootBeanDefinition methodDefinitionAttributes = new RootBeanDefinition(MethodDefinitionAttributes.class);
-        methodDefinitionAttributes.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
-        methodDefinitionAttributes.getPropertyValues().addPropertyValue("attributes", new RuntimeBeanReference(BeanIds.SECURITY_ANNOTATION_ATTRIBUTES));
-        parserContext.getRegistry().registerBeanDefinition(BeanIds.METHOD_DEFINITION_ATTRIBUTES, methodDefinitionAttributes);
-
-        RootBeanDefinition interceptor = new RootBeanDefinition(MethodSecurityInterceptor.class);
-        interceptor.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
+        parserContext.getRegistry().registerBeanDefinition(beanId, securityAnnotations);
 
         String accessManagerId = element.getAttribute(ATT_ACCESS_MGR);
 
@@ -69,15 +63,22 @@ class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser {
             accessManagerId = BeanIds.ACCESS_MANAGER;
         }
 
+        // MethodSecurityInterceptor
+
+        RootBeanDefinition interceptor = new RootBeanDefinition(MethodSecurityInterceptor.class);
+        interceptor.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
+
         interceptor.getPropertyValues().addPropertyValue("accessDecisionManager",
                 new RuntimeBeanReference(accessManagerId));
 
         interceptor.getPropertyValues().addPropertyValue("authenticationManager",
                 new RuntimeBeanReference(BeanIds.AUTHENTICATION_MANAGER));
 
-        interceptor.getPropertyValues().addPropertyValue("objectDefinitionSource", new RuntimeBeanReference(BeanIds.METHOD_DEFINITION_ATTRIBUTES));
+        interceptor.getPropertyValues().addPropertyValue("objectDefinitionSource", new RuntimeBeanReference(beanId));
         parserContext.getRegistry().registerBeanDefinition(BeanIds.METHOD_SECURITY_INTERCEPTOR, interceptor);
 
+        // MethodDefinitionSourceAdvisor
+        
         RootBeanDefinition advisor = new RootBeanDefinition(MethodDefinitionSourceAdvisor.class);
         advisor.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
         advisor.getConstructorArgumentValues().addGenericArgumentValue(interceptor);

+ 2 - 2
core/src/main/java/org/springframework/security/config/BeanIds.java

@@ -48,8 +48,8 @@ public abstract class BeanIds {
 	public static final String SECURITY_CONTEXT_HOLDER_AWARE_REQUEST_FILTER = "_securityContextHolderAwareRequestFilter";
 	public static final String METHOD_SECURITY_INTERCEPTOR = "_methodSecurityInterceptor";
 	public static final String METHOD_DEFINITION_SOURCE_ADVISOR = "_methodDefinitionSourceAdvisor";
-	public static final String SECURITY_ANNOTATION_ATTRIBUTES = "_securityAnnotationAttributes";
-	public static final String METHOD_DEFINITION_ATTRIBUTES = "_methodDefinitionAttributes";
+	public static final String SECURED_METHOD_DEFINITION_SOURCE = "_securedMethodDefinitionSource";
+	public static final String JSR_250_METHOD_DEFINITION_SOURCE = "_jsr250MethodDefinitionSource";
     public static final String EMBEDDED_APACHE_DS = "_apacheDirectoryServerContainer";
     public static final String CONTEXT_SOURCE = "_securityContextSource";
     public static final String PORT_MAPPER = "_portMapper";

+ 28 - 22
core/src/main/java/org/springframework/security/config/InterceptMethodsBeanDefinitionDecorator.java

@@ -1,28 +1,24 @@
 package org.springframework.security.config;
 
+import java.util.Iterator;
+import java.util.List;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.aop.config.AbstractInterceptorDrivenBeanDefinitionDecorator;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.BeanDefinitionHolder;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
-import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.beans.factory.xml.BeanDefinitionDecorator;
 import org.springframework.beans.factory.xml.ParserContext;
-import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.ConfigAttributeEditor;
-import org.springframework.security.intercept.method.MethodDefinitionMap;
 import org.springframework.security.intercept.method.aopalliance.MethodSecurityInterceptor;
-import org.springframework.util.xml.DomUtils;
 import org.springframework.util.StringUtils;
-
+import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
-import java.util.Iterator;
-import java.util.List;
-
 /**
  * @author Luke Taylor
  * @author Ben Alex
@@ -35,17 +31,16 @@ public class InterceptMethodsBeanDefinitionDecorator implements BeanDefinitionDe
     public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder definition, ParserContext parserContext) {
         ConfigUtils.registerProviderManagerIfNecessary(parserContext);
         ConfigUtils.registerDefaultAccessManagerIfNecessary(parserContext);
-
+        
         return delegate.decorate(node, definition, parserContext);
     }
 }
 
 /**
- * This is the real class which does the work. We need acccess to the ParserContext in order to do bean
+ * This is the real class which does the work. We need access to the ParserContext in order to do bean
  * registration.
  */
 class InternalInterceptMethodsBeanDefinitionDecorator extends AbstractInterceptorDrivenBeanDefinitionDecorator {
-    static final String ATT_CLASS = "class";
 	static final String ATT_METHOD = "method";
 	static final String ATT_ACCESS = "access";
     private static final String ATT_ACCESS_MGR = "access-decision-manager-ref";
@@ -68,24 +63,35 @@ class InternalInterceptMethodsBeanDefinitionDecorator extends AbstractIntercepto
         interceptor.addPropertyValue("accessDecisionManager", new RuntimeBeanReference(accessManagerId));
         interceptor.addPropertyValue("authenticationManager", new RuntimeBeanReference(BeanIds.AUTHENTICATION_MANAGER));
 
+        // Lookup parent bean information
+    	Element parent = (Element) node.getParentNode();
+    	String parentBeanClass = parent.getAttribute("class");
+    	String parentBeanId = parent.getAttribute("id");
+        parent = null;
+        
         // Parse the included methods
         List methods = DomUtils.getChildElementsByTagName(interceptMethodsElt, Elements.PROTECT);
-        MethodDefinitionMap methodMap = new MethodDefinitionMap();
-        ConfigAttributeEditor attributeEditor = new ConfigAttributeEditor();
 
+        StringBuffer sb = new StringBuffer();
+        
         for (Iterator i = methods.iterator(); i.hasNext();) {
             Element protectmethodElt = (Element) i.next();
             String accessConfig = protectmethodElt.getAttribute(ATT_ACCESS);
-            attributeEditor.setAsText(accessConfig);
 
-// TODO: We want to use just the method names, but MethodDefinitionMap won't work that way.
-//            methodMap.addSecureMethod(targetClass, protectmethodElt.getAttribute("method"),
-//                    (ConfigAttributeDefinition) attributeEditor.getValue());
-            methodMap.addSecureMethod(protectmethodElt.getAttribute(ATT_METHOD),
-                    (ConfigAttributeDefinition) attributeEditor.getValue());
+            // Support inference of class names
+            String methodName = protectmethodElt.getAttribute(ATT_METHOD);
+            
+            if (methodName.lastIndexOf(".") == -1) {
+            	if (parentBeanClass != null && !"".equals(parentBeanClass)) {
+            		methodName = parentBeanClass + "." + methodName;
+            	}
+            }
+            
+            // Rely on the default property editor for MethodSecurityInterceptor.setObjectDefinitionSource to setup the MethodDefinitionSource
+            sb.append(methodName + "=" + accessConfig).append("\r\n");
         }
-
-        interceptor.addPropertyValue("objectDefinitionSource", methodMap);
+        
+        interceptor.addPropertyValue("objectDefinitionSource", sb.toString());
 
         return interceptor.getBeanDefinition();
     }

+ 200 - 0
core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java

@@ -0,0 +1,200 @@
+package org.springframework.security.intercept.method;
+
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.aopalliance.intercept.MethodInvocation;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.aspectj.lang.JoinPoint;
+import org.aspectj.lang.reflect.CodeSignature;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
+import org.springframework.util.ObjectUtils;
+
+/**
+ * Abstract implementation of {@link MethodDefinitionSource} that supports both Spring AOP and AspectJ and
+ * caches configuration attribute resolution from: 1. specific target method; 2. target class;  3. declaring method;
+ * 4. declaring class/interface.
+ * 
+ * <p>
+ * This class mimics the behaviour of Spring's AbstractFallbackTransactionAttributeSource class.
+ * </p>
+ * 
+ * <p>
+ * Note that this class cannot extract security metadata where that metadata is expressed by way of
+ * a target method/class (ie #1 and #2 above) AND the target method/class is encapsulated in another
+ * proxy object. Spring Security does not walk a proxy chain to locate the concrete/final target object.
+ * Consider making Spring Security your final advisor (so it advises the final target, as opposed to
+ * another proxy), move the metadata to declared methods or interfaces the proxy implements, or provide
+ * your own replacement <tt>MethodDefinitionSource</tt>.
+ * </p>
+ * 
+ * @author Ben Alex
+ * @version $Id$
+ */
+public abstract class AbstractFallbackMethodDefinitionSource implements MethodDefinitionSource {
+
+    private static final Log logger = LogFactory.getLog(AbstractFallbackMethodDefinitionSource.class);
+	private final static Object NULL_CONFIG_ATTRIBUTE = new Object();
+    private final Map attributeCache = new HashMap();
+
+    public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException {
+        Assert.notNull(object, "Object cannot be null");
+
+        if (object instanceof MethodInvocation) {
+        	MethodInvocation mi = (MethodInvocation) object;
+            return getAttributes(mi.getMethod(), mi.getThis().getClass());
+        }
+
+        if (object instanceof JoinPoint) {
+            JoinPoint jp = (JoinPoint) object;
+            Class targetClass = jp.getTarget().getClass();
+            String targetMethodName = jp.getStaticPart().getSignature().getName();
+            Class[] types = ((CodeSignature) jp.getStaticPart().getSignature()).getParameterTypes();
+            Class declaringType = ((CodeSignature) jp.getStaticPart().getSignature()).getDeclaringType();
+
+            Method method = ClassUtils.getMethodIfAvailable(declaringType, targetMethodName, types);
+            Assert.notNull(method, "Could not obtain target method from JoinPoint: '"+ jp + "'");
+            
+            return getAttributes(method, targetClass);
+        }
+
+        throw new IllegalArgumentException("Object must be a MethodInvocation or JoinPoint");
+    }
+    
+    public final boolean supports(Class clazz) {
+        return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz));
+    }
+    
+    public ConfigAttributeDefinition 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);
+			if (cached != null) {
+				// Value will either be canonical value indicating there is no config attribute,
+				// or an actual config attribute.
+				if (cached == NULL_CONFIG_ATTRIBUTE) {
+					return null;
+				}
+				else {
+					return (ConfigAttributeDefinition) cached;
+				}
+			}
+			else {
+				// We need to work it out.
+				ConfigAttributeDefinition cfgAtt = computeAttributes(method, targetClass);
+				// Put it in the cache.
+				if (cfgAtt == null) {
+					this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE);
+				}
+				else {
+					if (logger.isDebugEnabled()) {
+						logger.debug("Adding security method [" + cacheKey + "] with attribute [" + cfgAtt + "]");
+					}
+					this.attributeCache.put(cacheKey, cfgAtt);
+				}
+				return cfgAtt;
+			}
+		}
+    }
+    
+    /**
+     * 
+	 * @param method the method for the current invocation (never <code>null</code>)
+	 * @param targetClass the target class for this invocation (may be <code>null</code>)
+     * @return
+     */
+    private ConfigAttributeDefinition computeAttributes(Method method, Class targetClass) {
+    	// The method may be on an interface, but we need attributes from the target class.
+		// If the target class is null, the method will be unchanged.
+		Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
+		// First try is the method in the target class.
+		ConfigAttributeDefinition attr = findAttributes(specificMethod, targetClass);
+		if (attr != null) {
+			return attr;
+		}
+
+		// Second try is the config attribute on the target class.
+		attr = findAttributes(specificMethod.getDeclaringClass());
+		if (attr != null) {
+			return attr;
+		}
+
+		if (specificMethod != method) {
+			// Fallback is to look at the original method.
+			attr = findAttributes(method, method.getDeclaringClass());
+			if (attr != null) {
+				return attr;
+			}
+			// Last fallback is the class of the original method.
+			return findAttributes(method.getDeclaringClass());
+		}
+		return null;
+
+    }
+    
+    /**
+     * Obtains the security metadata applicable to the specified method invocation.
+     * 
+     * <p>
+     * Note that the {@link Method#getDeclaringClass()} may not equal the <code>targetClass</code>.
+     * Both parameters are provided to assist subclasses which may wish to provide advanced
+     * capabilities related to method metadata being "registered" against a method even if the
+     * target class does not declare the method (ie the subclass may only inherit the method).
+     * 
+     * @param method the method for the current invocation (never <code>null</code>)
+     * @param targetClass the target class for the invocation (may be <code>null</code>)
+     * @return the security metadata (or null if no metadata applies)
+     */
+    protected abstract ConfigAttributeDefinition findAttributes(Method method, Class targetClass);
+    
+    /**
+     * Obtains the security metadata registered against the specified class.
+     * 
+     * <p>
+     * Subclasses should only return metadata expressed at a class level. Subclasses should NOT
+     * aggregate metadata for each method registered against a class, as the abstract superclass
+     * will separate invoke {@link #findAttributes(Method, Class)} for individual methods as
+     * appropriate. 
+     * 
+     * @param clazz the target class for the invocation (never <code>null</code>)
+     * @return the security metadata (or null if no metadata applies)
+     */
+    protected abstract ConfigAttributeDefinition findAttributes(Class clazz);
+    
+	private static class DefaultCacheKey {
+
+		private final Method method;
+		private final Class targetClass;
+
+		public DefaultCacheKey(Method method, Class targetClass) {
+			this.method = method;
+			this.targetClass = targetClass;
+		}
+
+		public boolean equals(Object other) {
+			if (this == other) {
+				return true;
+			}
+			if (!(other instanceof DefaultCacheKey)) {
+				return false;
+			}
+			DefaultCacheKey otherKey = (DefaultCacheKey) other;
+			return (this.method.equals(otherKey.method) &&
+					ObjectUtils.nullSafeEquals(this.targetClass, otherKey.targetClass));
+		}
+
+		public int hashCode() {
+			return this.method.hashCode() * 21 + (this.targetClass != null ? this.targetClass.hashCode() : 0);
+		}
+
+		public String toString() {
+			return "CacheKey[" + (targetClass == null ? "-" : targetClass.getName()) + "; " + method + "]";
+		}
+}
+    
+}

+ 132 - 74
core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionMap.java → core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java

@@ -15,63 +15,59 @@
 
 package org.springframework.security.intercept.method;
 
-import org.springframework.security.ConfigAttributeDefinition;
-
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
 import java.lang.reflect.Method;
-
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Collection;
-import java.util.Collections;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.BeanClassLoaderAware;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 
 
 /**
- * Stores a {@link ConfigAttributeDefinition} for each method signature defined in a bean context.<p>For
- * consistency with {@link MethodDefinitionAttributes} as well as support for
- * <code>MethodDefinitionSourceAdvisor</code>, this implementation will return a
- * <code>ConfigAttributeDefinition</code> containing all configuration attributes defined against:
- *  <ul>
- *      <li>The method-specific attributes defined for the intercepted method of the intercepted class.</li>
- *      <li>The method-specific attributes defined by any explicitly implemented interface if that interface
- *      contains a method signature matching that of the intercepted method.</li>
- *  </ul>
- *  </p>
- *  <p>In general you should therefore define the <b>interface method</b>s of your secure objects, not the
- * implementations. For example, define <code>com.company.Foo.findAll=ROLE_TEST</code> but not
- * <code>com.company.FooImpl.findAll=ROLE_TEST</code>.</p>
- *
+ * Stores a {@link ConfigAttributeDefinition} for a method or class signature.
+ * 
+ * <p>
+ * This class is the preferred implementation of {@link MethodDefinitionSource} for XML-based
+ * definition of method security metadata. To assist in XML-based definition, wildcard support
+ * is provided.
+ * </p>
+ * 
  * @author Ben Alex
- * @version $Id$
+ * @version $Id: MethodDefinitionMap.java 2558 2008-01-30 15:43:40Z luke_t $
  */
-public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
+public class MapBasedMethodDefinitionSource extends AbstractFallbackMethodDefinitionSource implements BeanClassLoaderAware {
     //~ Static fields/initializers =====================================================================================
 
-    private static final Log logger = LogFactory.getLog(MethodDefinitionMap.class);
+    private static final Log logger = LogFactory.getLog(MapBasedMethodDefinitionSource.class);
 
     //~ Instance fields ================================================================================================
+	private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
 
-    /** Map from Method to ApplicationDefinition */
+	/** Map from RegisteredMethod to ConfigAttributeDefinition */
     protected Map methodMap = new HashMap();
 
-    /** Map from Method to name pattern used for registration */
+    /** Map from RegisteredMethod to name pattern used for registration */
     private Map nameMap = new HashMap();
 
     //~ Methods ========================================================================================================
 
-    public MethodDefinitionMap() {
+    public MapBasedMethodDefinitionSource() {
     }
 
     /**
-     * Creates the MethodDefinitionMap from a
+     * Creates the MapBasedMethodDefinitionSource from a
      * @param methodMap map of method names to <tt>ConfigAttributeDefinition</tt>s.
      */
-    public MethodDefinitionMap(Map methodMap) {
+    public MapBasedMethodDefinitionSource(Map methodMap) {
         Iterator iterator = methodMap.entrySet().iterator();
 
         while (iterator.hasNext()) {
@@ -80,15 +76,44 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
         }        
     }
 
+	/**
+	 * Implementation does not support class-level attributes.
+	 */
+	protected ConfigAttributeDefinition findAttributes(Class clazz) {
+		return null;
+	}
+
+	/**
+	 * Will walk the method inheritance tree to find the most specific declaration applicable.
+	 */
+	protected ConfigAttributeDefinition findAttributes(Method method, Class targetClass) {
+		return findAttributesSpecifiedAgainst(method, targetClass);
+	}
+	
+	private ConfigAttributeDefinition findAttributesSpecifiedAgainst(Method method, Class clazz) {
+		RegisteredMethod registeredMethod = new RegisteredMethod(method, clazz);
+		if (methodMap.containsKey(registeredMethod)) {
+			return (ConfigAttributeDefinition) methodMap.get(registeredMethod);
+		}
+		// Search superclass
+		if (clazz.getSuperclass() != null) {
+			return findAttributesSpecifiedAgainst(method, clazz.getSuperclass());
+		}
+		return null;
+	}
+
     /**
-     * Add configuration attributes for a secure method. Method names can end or start with <code>&#42</code>
-     * for matching multiple methods.
+     * Add configuration attributes for a secure method.
      *
      * @param method the method to be secured
      * @param attr required authorities associated with the method
      */
-    public void addSecureMethod(Method method, ConfigAttributeDefinition attr) {
-        logger.info("Adding secure method [" + method + "] with attributes [" + attr + "]");
+    private void addSecureMethod(RegisteredMethod method, ConfigAttributeDefinition attr) {
+    	Assert.notNull(method, "RegisteredMethod required");
+    	Assert.notNull(attr, "Configuration attribute required");
+    	if (logger.isInfoEnabled()) {
+            logger.info("Adding secure method [" + method + "] with attributes [" + attr + "]");
+    	}
         this.methodMap.put(method, attr);
     }
 
@@ -96,47 +121,41 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
      * Add configuration attributes for a secure method. Method names can end or start with <code>&#42</code>
      * for matching multiple methods.
      *
-     * @param name class and method name, separated by a dot
+     * @param name type and method name, separated by a dot
      * @param attr required authorities associated with the method
-     *
-     * @throws IllegalArgumentException DOCUMENT ME!
      */
     public void addSecureMethod(String name, ConfigAttributeDefinition attr) {
-        int lastDotIndex = name.lastIndexOf(".");
+    	int lastDotIndex = name.lastIndexOf(".");
 
         if (lastDotIndex == -1) {
             throw new IllegalArgumentException("'" + name + "' is not a valid method name: format is FQN.methodName");
         }
 
-        String className = name.substring(0, lastDotIndex);
         String methodName = name.substring(lastDotIndex + 1);
-
-        try {
-            Class clazz = Class.forName(className, true, Thread.currentThread().getContextClassLoader());
-            addSecureMethod(clazz, methodName, attr);
-        } catch (ClassNotFoundException ex) {
-            throw new IllegalArgumentException("Class '" + className + "' not found");
-        }
+        Assert.hasText(methodName, "Method not found for '" + name + "'");
+        
+        String typeName = name.substring(0, lastDotIndex);
+        Class type = ClassUtils.resolveClassName(typeName, this.beanClassLoader);
+        
+        addSecureMethod(type, methodName, attr);
     }
 
     /**
-     * Add configuration attributes for a secure method. Method names can end or start with <code>&#42</code>
+     * Add configuration attributes for a secure method. Mapped method names can end or start with <code>&#42</code>
      * for matching multiple methods.
-     *
-     * @param clazz target interface or class
-     * @param mappedName mapped method name
+     * 
+     * @param javaType target interface or class the security configuration attribute applies to
+     * @param mappedName mapped method name, which the javaType has declared or inherited
      * @param attr required authorities associated with the method
-     *
-     * @throws IllegalArgumentException DOCUMENT ME!
      */
-    public void addSecureMethod(Class clazz, String mappedName, ConfigAttributeDefinition attr) {
-        String name = clazz.getName() + '.' + mappedName;
+    public void addSecureMethod(Class javaType, String mappedName, ConfigAttributeDefinition attr) {
+        String name = javaType.getName() + '.' + mappedName;
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Adding secure method [" + name + "] with attributes [" + attr + "]");
+            logger.debug("Request to add secure method [" + name + "] with attributes [" + attr + "]");
         }
 
-        Method[] methods = clazz.getDeclaredMethods();
+        Method[] methods = javaType.getMethods();
         List matchingMethods = new ArrayList();
 
         for (int i = 0; i < methods.length; i++) {
@@ -146,13 +165,14 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
         }
 
         if (matchingMethods.isEmpty()) {
-            throw new IllegalArgumentException("Couldn't find method '" + mappedName + "' on " + clazz);
+            throw new IllegalArgumentException("Couldn't find method '" + mappedName + "' on '" + javaType + "'");
         }
 
         // register all matching methods
         for (Iterator it = matchingMethods.iterator(); it.hasNext();) {
             Method method = (Method) it.next();
-            String regMethodName = (String) this.nameMap.get(method);
+            RegisteredMethod registeredMethod = new RegisteredMethod(method, javaType);
+            String regMethodName = (String) this.nameMap.get(registeredMethod);
 
             if ((regMethodName == null) || (!regMethodName.equals(name) && (regMethodName.length() <= name.length()))) {
                 // no already registered method name, or more specific
@@ -162,8 +182,8 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
                         + "] is more specific than [" + regMethodName + "]");
                 }
 
-                this.nameMap.put(method, name);
-                addSecureMethod(method, attr);
+                this.nameMap.put(registeredMethod, name);
+                addSecureMethod(registeredMethod, attr);
             } else {
                 logger.debug("Keeping attributes for secure method [" + method + "]: current name [" + name
                     + "] is not more specific than [" + regMethodName + "]");
@@ -172,9 +192,7 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
     }
 
     /**
-     * Obtains the configuration attributes explicitly defined against this bean. This method will not return
-     * implicit configuration attributes that may be returned by {@link #lookupAttributes(Method)} as it does not have
-     * access to a method invocation at this time.
+     * Obtains the configuration attributes explicitly defined against this bean.
      *
      * @return the attributes explicitly defined against this bean
      */
@@ -182,17 +200,6 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
         return Collections.unmodifiableCollection(methodMap.values());
     }
 
-    /**
-     * Obtains the number of configuration attributes explicitly defined against this bean. This method will
-     * not return implicit configuration attributes that may be returned by {@link #lookupAttributes(Method)} as it
-     * does not have access to a method invocation at this time.
-     *
-     * @return the number of configuration attributes explicitly defined against this bean
-     */
-    public int getMethodMapSize() {
-        return this.methodMap.size();
-    }
-
     /**
      * Return if the given method name matches the mapped name. The default implementation checks for "xxx" and
      * "xxx" matches.
@@ -245,4 +252,55 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
 
         attributes.addAll(toMerge.getConfigAttributes());
     }
+
+	public void setBeanClassLoader(ClassLoader beanClassLoader) {
+		Assert.notNull(beanClassLoader, "Bean class loader required");
+		this.beanClassLoader = beanClassLoader;
+	}
+
+	/**
+	 * @return map size (for unit tests and diagnostics)
+	 */
+	public int getMethodMapSize() {
+		return methodMap.size();
+	}
+	
+	/**
+	 * Stores both the Java Method as well as the Class we obtained the Method from. This is necessary because Method only
+	 * provides us access to the declaring class. It doesn't provide a way for us to introspect which Class the Method
+	 * was registered against. If a given Class inherits and redeclares a method (ie calls super();) the registered Class
+	 * and delcaring Class are the same. If a given class merely inherits but does not redeclare a method, the registered
+	 * Class will be the Class we're invoking against and the Method will provide details of the declared class.
+	 */
+	private class RegisteredMethod {
+		private Method method;
+		private Class registeredJavaType;
+
+		public RegisteredMethod(Method method, Class registeredJavaType) {
+			Assert.notNull(method, "Method required");
+			Assert.notNull(registeredJavaType, "Registered Java Type required");
+			this.method = method;
+			this.registeredJavaType = registeredJavaType;
+		}
+
+		public boolean equals(Object obj) {
+			if (this == obj) {
+				return true;
+			}
+			if (obj != null && obj instanceof RegisteredMethod) {
+				RegisteredMethod rhs = (RegisteredMethod) obj;
+				return method.equals(rhs.method) && registeredJavaType.equals(rhs.registeredJavaType);
+			}
+			return false;
+		}
+
+		public int hashCode() {
+			return method.hashCode() * registeredJavaType.hashCode();
+		}
+
+		public String toString() {
+			return "RegisteredMethod[" + registeredJavaType.getName() + "; " + method + "]";
+		}
+	}
+    
 }

+ 21 - 86
core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionAttributes.java

@@ -15,122 +15,57 @@
 
 package org.springframework.security.intercept.method;
 
-import org.springframework.security.ConfigAttribute;
-import org.springframework.security.ConfigAttributeDefinition;
-
-import org.springframework.metadata.Attributes;
-
 import java.lang.reflect.Method;
-
 import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.ArrayList;
+
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.metadata.Attributes;
+import org.springframework.security.ConfigAttribute;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.util.Assert;
 
 
 /**
  * Provides {@link ConfigAttributeDefinition}s for a method signature (via the <tt>lookupAttributes</tt> method)
- * by delegating to a configured {@link Attributes} object. The latter may use Java 5 annotations, Commons attributes
+ * by delegating to a configured {@link Attributes} object. The latter may use Commons attributes
  * or some other approach to determine the <tt>ConfigAttribute</tt>s which apply. 
- * <p>
- * This class will only detect those attributes which are defined for:
- *  <ul>
- *      <li>The class-wide attributes defined for the intercepted class.</li>
- *      <li>The class-wide attributes defined for interfaces explicitly implemented by the intercepted class.</li>
- *      <li>The method-specific attributes defined for the intercepted method of the intercepted class.</li>
- *      <li>The method-specific attributes defined by any explicitly implemented interface if that interface
- *      contains a method signature matching that of the intercepted method.</li>
- *  </ul>
+ *
  * <p>
  * Note that attributes defined against parent classes (either for their methods or interfaces) are not
  * detected. The attributes must be defined against an explicit method or interface on the intercepted class.
  * <p>
+ * 
  * Attributes detected that do not implement {@link ConfigAttribute} will be ignored.
  *
  * @author Cameron Braid
  * @author Ben Alex
  * @version $Id$
  */
-public class MethodDefinitionAttributes extends AbstractMethodDefinitionSource {
+public class MethodDefinitionAttributes extends AbstractFallbackMethodDefinitionSource implements InitializingBean {
     //~ Instance fields ================================================================================================
 
     private Attributes attributes;
 
     //~ Methods ========================================================================================================
 
-    private void add(List definition, Collection attribs) {
-        for (Iterator iter = attribs.iterator(); iter.hasNext();) {
-            Object o = iter.next();
-
-            if (o instanceof ConfigAttribute) {
-                definition.add(o);
-            }
-        }
-    }
-
-    private void addClassAttributes(List definition, Class[] clazz) {
-        for (int i = 0; i < clazz.length; i++) {
-            Collection classAttributes = attributes.getAttributes(clazz[i]);
-
-            if (classAttributes != null) {
-                add(definition, classAttributes);
-            }
-        }
-    }
-
-    private void addInterfaceMethodAttributes(List definition, Method method) {
-        Class[] interfaces = method.getDeclaringClass().getInterfaces();
-
-        for (int i = 0; i < interfaces.length; i++) {
-            Class clazz = interfaces[i];
-
-            try {
-                Method m = clazz.getDeclaredMethod(method.getName(), (Class[]) method.getParameterTypes());
-                addMethodAttributes(definition, m);
-            } catch (Exception e) {
-                // this won't happen since we are getting a method from an interface that
-                // the declaring class implements
-            }
-        }
-    }
-
-    private void addMethodAttributes(List definition, Method method) {
-        // add the method level attributes
-        Collection methodAttributes = attributes.getAttributes(method);
-
-        if (methodAttributes != null) {
-            add(definition, methodAttributes);
-        }
-    }
+    public void afterPropertiesSet() throws Exception {
+    	Assert.notNull(attributes, "attributes required");
+	}
 
     public Collection getConfigAttributeDefinitions() {
         return null;
     }
+    
+	protected ConfigAttributeDefinition findAttributes(Class clazz) {
+        return ConfigAttributeDefinition.createFiltered(attributes.getAttributes(clazz));
+	}
 
-    protected ConfigAttributeDefinition lookupAttributes(Method method) {
-        Class interceptedClass = method.getDeclaringClass();
-        List attributes = new ArrayList();
-
-        // add the class level attributes for the implementing class
-        addClassAttributes(attributes, new Class[] {interceptedClass});
-
-        // add the class level attributes for the implemented interfaces
-        addClassAttributes(attributes, interceptedClass.getInterfaces());
-
-        // add the method level attributes for the implemented method
-        addMethodAttributes(attributes, method);
-
-        // add the method level attributes for the implemented intreface methods
-        addInterfaceMethodAttributes(attributes, method);
-
-        if (attributes.size() == 0) {
-            return null;
-        }
-
-        return new ConfigAttributeDefinition(attributes);
-    }
+	protected ConfigAttributeDefinition findAttributes(Method method, Class targetClass) {
+        return ConfigAttributeDefinition.createFiltered(attributes.getAttributes(method));
+	}
 
     public void setAttributes(Attributes attributes) {
+    	Assert.notNull(attributes, "Attributes required");
         this.attributes = attributes;
     }
 }

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

@@ -15,14 +15,19 @@
 
 package org.springframework.security.intercept.method;
 
+import java.lang.reflect.Method;
+
+import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.intercept.ObjectDefinitionSource;
 
 
 /**
- * Marker interface for <code>ObjectDefinitionSource</code> implementations
+ * Interface for <code>ObjectDefinitionSource</code> implementations
  * that are designed to perform lookups keyed on <code>Method</code>s.
  *
  * @author Ben Alex
  * @version $Id$
  */
-public interface MethodDefinitionSource extends ObjectDefinitionSource {}
+public interface MethodDefinitionSource extends ObjectDefinitionSource {
+    public ConfigAttributeDefinition getAttributes(Method method, Class targetClass);
+}

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

@@ -33,7 +33,7 @@ import java.util.LinkedHashMap;
 
 /**
  * Property editor to assist with the setup of a {@link MethodDefinitionSource}.
- * <p>The class creates and populates a {@link MethodDefinitionMap}.</p>
+ * <p>The class creates and populates a {@link MapBasedMethodDefinitionSource}.</p>
  *
  * @author Ben Alex
  * @version $Id$
@@ -47,7 +47,7 @@ public class MethodDefinitionSourceEditor extends PropertyEditorSupport {
 
     public void setAsText(String s) throws IllegalArgumentException {
         if ((s == null) || "".equals(s)) {
-            setValue(new MethodDefinitionMap());
+            setValue(new MapBasedMethodDefinitionSource());
             return;
         }
 
@@ -69,6 +69,6 @@ public class MethodDefinitionSourceEditor extends PropertyEditorSupport {
             mappings.put(name, new ConfigAttributeDefinition(tokens));
         }
 
-        setValue(new MethodDefinitionMap(mappings));
+        setValue(new MapBasedMethodDefinitionSource(mappings));
     }
 }

+ 7 - 11
core/src/main/java/org/springframework/security/intercept/method/aopalliance/MethodDefinitionSourceAdvisor.java

@@ -21,11 +21,10 @@ import java.lang.reflect.Method;
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInvocation;
 import org.springframework.aop.Pointcut;
-import org.springframework.aop.framework.AopConfigException;
 import org.springframework.aop.support.AbstractPointcutAdvisor;
 import org.springframework.aop.support.StaticMethodMatcherPointcut;
 import org.springframework.security.intercept.method.MethodDefinitionSource;
-
+import org.springframework.util.Assert;
 
 /**
  * Advisor driven by a {@link MethodDefinitionSource}, used to exclude a {@link MethodSecurityInterceptor} from
@@ -55,10 +54,7 @@ public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor {
     public MethodDefinitionSourceAdvisor(MethodSecurityInterceptor advice) {
     	this.interceptor = advice;
 
-        if (advice.getObjectDefinitionSource() == null) {
-            throw new AopConfigException("Cannot construct a MethodDefinitionSourceAdvisor using a "
-                + "MethodSecurityInterceptor that has no ObjectDefinitionSource configured");
-        }
+    	Assert.notNull(advice.getObjectDefinitionSource(), "Cannot construct a MethodDefinitionSourceAdvisor using a MethodSecurityInterceptor that has no ObjectDefinitionSource configured");
 
         this.attributeSource = advice.getObjectDefinitionSource();
         this.pointcut = new MethodDefinitionSourcePointcut();
@@ -78,9 +74,7 @@ public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor {
     
     class MethodDefinitionSourcePointcut extends StaticMethodMatcherPointcut {
         public boolean matches(Method m, Class targetClass) {
-            MethodInvocation methodInvocation = new InternalMethodInvocation(m);
-
-            return attributeSource.getAttributes(methodInvocation) != null;
+            return attributeSource.getAttributes(m, targetClass) != null;
         }
     }
     
@@ -92,9 +86,11 @@ public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor {
      */
     class InternalMethodInvocation implements MethodInvocation {
         private Method method;
+        private Class targetClass;
 
-        public InternalMethodInvocation(Method method) {
+        public InternalMethodInvocation(Method method, Class targetClass) {
             this.method = method;
+            this.targetClass = targetClass;
         }
 
         protected InternalMethodInvocation() {
@@ -114,7 +110,7 @@ public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor {
         }
 
         public Object getThis() {
-            throw new UnsupportedOperationException();
+        	return this.targetClass;
         }
 
         public Object proceed() throws Throwable {

+ 28 - 10
core/src/main/java/org/springframework/security/util/MethodInvocationUtils.java

@@ -15,15 +15,15 @@
 
 package org.springframework.security.util;
 
-import org.aopalliance.intercept.MethodInvocation;
-
-import org.springframework.util.Assert;
-
 import java.lang.reflect.Method;
-
 import java.util.ArrayList;
 import java.util.List;
 
+import org.aopalliance.intercept.MethodInvocation;
+import org.springframework.aop.framework.Advised;
+import org.springframework.aop.support.AopUtils;
+import org.springframework.util.Assert;
+
 
 /**
  * Static utility methods for creating <code>MethodInvocation</code>s usable within Spring Security.
@@ -76,8 +76,25 @@ public final class MethodInvocationUtils {
 
             classArgs = (Class[]) list.toArray(new Class[] {});
         }
+        
+        // Determine the type that declares the requested method, taking into account proxies
+        Class target = AopUtils.getTargetClass(object);
+        if (object instanceof Advised) {
+        	Advised a = (Advised) object;
+        	if (!a.isProxyTargetClass()) {
+        		Class[] possibleInterfaces = a.getProxiedInterfaces();
+        		for (int i = 0; i < possibleInterfaces.length; i++) {
+        			try {
+            			possibleInterfaces[i].getMethod(methodName, classArgs);
+            			// to get here means no exception happened
+            			target = possibleInterfaces[i];
+            			break;
+        			} catch (Exception tryTheNextOne) {}
+        		}
+        	}
+        }
 
-        return createFromClass(object.getClass(), methodName, classArgs, args);
+        return createFromClass(object, target, methodName, classArgs, args);
     }
 
     /**
@@ -89,21 +106,22 @@ public final class MethodInvocationUtils {
      * @return a <code>MethodInvocation</code>, or <code>null</code> if there was a problem
      */
     public static MethodInvocation createFromClass(Class clazz, String methodName) {
-        return createFromClass(clazz, methodName, null, null);
+        return createFromClass(null, clazz, methodName, null, null);
     }
 
     /**
      * Generates a <code>MethodInvocation</code> for specified <code>methodName</code> on the passed class,
      * using the <code>args</code> to locate the method.
      *
+     * @param targetObject the object being invoked
      * @param clazz the class of object that will be used to find the relevant <code>Method</code>
      * @param methodName the name of the method to find
      * @param classArgs arguments that are required to locate the relevant method signature
      * @param args the actual arguments that should be passed to SimpleMethodInvocation
      * @return a <code>MethodInvocation</code>, or <code>null</code> if there was a problem
      */
-    public static MethodInvocation createFromClass(Class clazz, String methodName, Class[] classArgs, Object[] args) {
-        Assert.notNull(clazz, "Class required");
+    public static MethodInvocation createFromClass(Object targetObject, Class clazz, String methodName, Class[] classArgs, Object[] args) {
+    	Assert.notNull(clazz, "Class required");
         Assert.hasText(methodName, "MethodName required");
 
         Method method;
@@ -114,6 +132,6 @@ public final class MethodInvocationUtils {
             return null;
         }
 
-        return new SimpleMethodInvocation(method, args);
+        return new SimpleMethodInvocation(targetObject, method, args);
     }
 }

+ 5 - 3
core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java

@@ -32,11 +32,13 @@ public class SimpleMethodInvocation implements MethodInvocation {
 
     private Method method;
     private Object[] arguments;
+    private Object targetObject;
 
     //~ Constructors ===================================================================================================
 
-    public SimpleMethodInvocation(Method method, Object[] arguments) {
-        this.method = method;
+    public SimpleMethodInvocation(Object targetObject, Method method, Object[] arguments) {
+        this.targetObject = targetObject;
+    	this.method = method;
         this.arguments = arguments;
     }
 
@@ -57,7 +59,7 @@ public class SimpleMethodInvocation implements MethodInvocation {
     }
 
     public Object getThis() {
-        throw new UnsupportedOperationException("mock method not implemented");
+        return targetObject;
     }
 
     public Object proceed() throws Throwable {

+ 12 - 6
core/src/test/java/org/springframework/security/MockJoinPoint.java

@@ -34,12 +34,14 @@ public class MockJoinPoint implements JoinPoint {
 
     private Method beingInvoked;
     private Object object;
+    private Class declaringType;
 
     //~ Constructors ===================================================================================================
 
     public MockJoinPoint(Object object, Method beingInvoked) {
         this.object = object;
         this.beingInvoked = beingInvoked;
+        this.declaringType = object.getClass();
     }
 
     //~ Methods ========================================================================================================
@@ -61,7 +63,7 @@ public class MockJoinPoint implements JoinPoint {
     }
 
     public StaticPart getStaticPart() {
-        return new MockStaticPart(beingInvoked);
+        return new MockStaticPart(beingInvoked, declaringType);
     }
 
     public Object getTarget() {
@@ -84,13 +86,15 @@ public class MockJoinPoint implements JoinPoint {
 
     private class MockCodeSignature implements CodeSignature {
         private Method beingInvoked;
+        private Class declaringType;
 
-        public MockCodeSignature(Method beingInvoked) {
+        public MockCodeSignature(Method beingInvoked, Class declaringType) {
             this.beingInvoked = beingInvoked;
+            this.declaringType = declaringType;
         }
 
         public Class getDeclaringType() {
-            throw new UnsupportedOperationException("mock not implemented");
+            return this.declaringType;
         }
 
         public String getDeclaringTypeName() {
@@ -128,9 +132,11 @@ public class MockJoinPoint implements JoinPoint {
 
     private class MockStaticPart implements StaticPart {
         private Method beingInvoked;
-
-        public MockStaticPart(Method beingInvoked) {
+        private Class declaringType;
+        
+        public MockStaticPart(Method beingInvoked, Class declaringType) {
             this.beingInvoked = beingInvoked;
+            this.declaringType = declaringType;
         }
 
         public String getKind() {
@@ -138,7 +144,7 @@ public class MockJoinPoint implements JoinPoint {
         }
 
         public Signature getSignature() {
-            return new MockCodeSignature(beingInvoked);
+            return new MockCodeSignature(beingInvoked, declaringType);
         }
 
         public SourceLocation getSourceLocation() {

+ 0 - 4
core/src/test/java/org/springframework/security/OtherTargetObject.java

@@ -29,10 +29,6 @@ package org.springframework.security;
 public class OtherTargetObject extends TargetObject implements ITargetObject {
     //~ Methods ========================================================================================================
 
-    public int countLength(String input) {
-        return super.countLength(input);
-    }
-
     public String makeLowerCase(String input) {
         return super.makeLowerCase(input);
     }

+ 1 - 1
core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java

@@ -59,7 +59,7 @@ public class ContextPropagatingRemoteInvocationTests extends TestCase {
         throws Exception {
         Class clazz = TargetObject.class;
         Method method = clazz.getMethod("makeLowerCase", new Class[] {String.class});
-        MethodInvocation mi = new SimpleMethodInvocation(method, new Object[] {"SOME_STRING"});
+        MethodInvocation mi = new SimpleMethodInvocation(new TargetObject(), method, new Object[] {"SOME_STRING"});
 
         ContextPropagatingRemoteInvocationFactory factory = new ContextPropagatingRemoteInvocationFactory();
 

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

@@ -15,33 +15,13 @@
 
 package org.springframework.security.intercept.method;
 
-import junit.framework.TestCase;
+import java.lang.reflect.Method;
+
+import junit.framework.Assert;
 
-import org.springframework.security.ConfigAttribute;
+import org.junit.Test;
 import org.springframework.security.ConfigAttributeDefinition;
-import org.springframework.security.GrantedAuthority;
-import org.springframework.security.GrantedAuthorityImpl;
 import org.springframework.security.ITargetObject;
-import org.springframework.security.OtherTargetObject;
-import org.springframework.security.SecurityConfig;
-import org.springframework.security.TargetObject;
-
-import org.springframework.security.acl.basic.SomeDomain;
-
-import org.springframework.security.context.SecurityContextHolder;
-
-import org.springframework.security.providers.UsernamePasswordAuthenticationToken;
-
-import org.springframework.security.util.SimpleMethodInvocation;
-
-import org.springframework.context.ApplicationContext;
-import org.springframework.context.support.ClassPathXmlApplicationContext;
-
-import java.lang.reflect.Method;
-
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Set;
 
 
 /**
@@ -51,180 +31,27 @@ import java.util.Set;
  * @author Ben Alex
  * @version $Id$
  */
-public class MethodDefinitionAttributesTests extends TestCase {
-    //~ Instance fields ================================================================================================
-
-    ClassPathXmlApplicationContext applicationContext;
-
-    //~ Constructors ===================================================================================================
-
-    public MethodDefinitionAttributesTests(String a) {
-        super(a);
-    }
-
-    //~ Methods ========================================================================================================
+public class MethodDefinitionAttributesTests {
 
-
-    protected void tearDown() throws Exception {
-        super.tearDown();
-        SecurityContextHolder.clearContext();
+	private MethodDefinitionAttributes build() {
+    	MethodDefinitionAttributes mda = new MethodDefinitionAttributes();
+    	mda.setAttributes(new MockAttributes());
+    	return mda;
     }
-
-    private ConfigAttributeDefinition getConfigAttributeDefinition(Class clazz, String methodName, Class[] args)
-            throws Exception {
-        
-        final Method method = clazz.getMethod(methodName, args);
-        MethodDefinitionAttributes source = new MethodDefinitionAttributes();
-        source.setAttributes(new MockAttributes());
-
-        ConfigAttributeDefinition config = source.getAttributes(new SimpleMethodInvocation() {
-                    public Method getMethod() {
-                        return method;
-                    }
-                });
-
-        return config;
+    
+    @Test
+    public void testMethodsReturned() throws Exception {
+        Class clazz = ITargetObject.class;
+        Method method = clazz.getMethod("countLength", new Class[] {String.class});
+    	ConfigAttributeDefinition result = build().findAttributes(method, ITargetObject.class);
+    	Assert.assertEquals(1, result.getConfigAttributes().size());
     }
 
-    private ITargetObject makeInterceptedTarget() {
-        ApplicationContext context = new ClassPathXmlApplicationContext(
-                "org/springframework/security/intercept/method/applicationContext.xml");
-
-        return (ITargetObject) context.getBean("target");
-    }
-
-    public final void setUp() throws Exception {
-        super.setUp();
-    }
-
-    public void testAttributesForInterfaceTargetObject() throws Exception {
-        ConfigAttributeDefinition def1 = getConfigAttributeDefinition(ITargetObject.class, "countLength",
-                new Class[] {String.class});
-        Set set1 = toSet(def1);
-        assertTrue(set1.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set1.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_COUNT_LENGTH")));
-
-        ConfigAttributeDefinition def2 = getConfigAttributeDefinition(ITargetObject.class, "makeLowerCase",
-                new Class[] {String.class});
-        Set set2 = toSet(def2);
-        assertTrue(set2.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set2.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_MAKE_LOWER_CASE")));
-
-        ConfigAttributeDefinition def3 = getConfigAttributeDefinition(ITargetObject.class, "makeUpperCase",
-                new Class[] {String.class});
-        Set set3 = toSet(def3);
-        assertTrue(set3.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set3.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_MAKE_UPPER_CASE")));
+    @Test
+    public void testClassesReturned() throws Exception {
+        Class clazz = ITargetObject.class;
+    	ConfigAttributeDefinition result = build().findAttributes(ITargetObject.class);
+    	Assert.assertEquals(1, result.getConfigAttributes().size());
     }
 
-    public void testAttributesForOtherTargetObject() throws Exception {
-        ConfigAttributeDefinition def1 = getConfigAttributeDefinition(OtherTargetObject.class, "countLength",
-                new Class[] {String.class});
-        Set set1 = toSet(def1);
-        assertTrue(set1.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set1.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_COUNT_LENGTH")));
-
-        // Confirm MOCK_CLASS_METHOD_COUNT_LENGTH not added, as it's a String (not a ConfigAttribute)
-        // Confirm also MOCK_CLASS not added, as we return null for class
-        assertEquals(2, set1.size());
-
-        ConfigAttributeDefinition def2 = getConfigAttributeDefinition(OtherTargetObject.class, "makeLowerCase",
-                new Class[] {String.class});
-        Set set2 = toSet(def2);
-        assertTrue(set2.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set2.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_MAKE_LOWER_CASE")));
-        assertTrue(set2.contains(new SecurityConfig("MOCK_CLASS_METHOD_MAKE_LOWER_CASE")));
-
-        // Confirm MOCK_CLASS not added, as we return null for class
-        assertEquals(3, set2.size());
-
-        ConfigAttributeDefinition def3 = getConfigAttributeDefinition(OtherTargetObject.class, "makeUpperCase",
-                new Class[] {String.class});
-        Set set3 = toSet(def3);
-        assertTrue(set3.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set3.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_MAKE_UPPER_CASE")));
-        assertTrue(set3.contains(new SecurityConfig("RUN_AS"))); // defined against interface
-
-        assertEquals(3, set3.size());
-    }
-
-    public void testAttributesForTargetObject() throws Exception {
-        ConfigAttributeDefinition def1 = getConfigAttributeDefinition(TargetObject.class, "countLength",
-                new Class[] {String.class});
-        Set set1 = toSet(def1);
-        assertTrue(set1.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set1.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_COUNT_LENGTH")));
-
-        assertTrue(set1.contains(new SecurityConfig("MOCK_CLASS")));
-
-        // Confirm the MOCK_CLASS_METHOD_COUNT_LENGTH was not added, as it's not a ConfigAttribute
-        assertEquals(3, set1.size());
-
-        ConfigAttributeDefinition def2 = getConfigAttributeDefinition(TargetObject.class, "makeLowerCase",
-                new Class[] {String.class});
-        Set set2 = toSet(def2);
-        assertTrue(set2.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set2.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_MAKE_LOWER_CASE")));
-        assertTrue(set2.contains(new SecurityConfig("MOCK_CLASS")));
-        assertTrue(set2.contains(new SecurityConfig("MOCK_CLASS_METHOD_MAKE_LOWER_CASE")));
-        assertEquals(4, set2.size());
-
-        ConfigAttributeDefinition def3 = getConfigAttributeDefinition(TargetObject.class, "makeUpperCase",
-                new Class[] {String.class});
-        Set set3 = toSet(def3);
-        assertTrue(set3.contains(new SecurityConfig("MOCK_INTERFACE")));
-        assertTrue(set3.contains(new SecurityConfig("MOCK_INTERFACE_METHOD_MAKE_UPPER_CASE")));
-        assertTrue(set3.contains(new SecurityConfig("MOCK_CLASS")));
-        assertTrue(set3.contains(new SecurityConfig("MOCK_CLASS_METHOD_MAKE_UPPER_CASE")));
-        assertTrue(set3.contains(new SecurityConfig("RUN_AS")));
-        assertEquals(5, set3.size());
-    }
-
-    public void testMethodCallWithRunAsReplacement() throws Exception {
-        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
-                new GrantedAuthority[] {new GrantedAuthorityImpl("MOCK_INTERFACE_METHOD_MAKE_UPPER_CASE")});
-        SecurityContextHolder.getContext().setAuthentication(token);
-
-        ITargetObject target = makeInterceptedTarget();
-        String result = target.makeUpperCase("hello");
-        assertEquals("HELLO org.springframework.security.MockRunAsAuthenticationToken true", result);
-    }
-
-    public void testMethodCallWithoutRunAsReplacement() throws Exception {
-        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
-                new GrantedAuthority[] {new GrantedAuthorityImpl("MOCK_INTERFACE_METHOD_MAKE_LOWER_CASE")});
-        SecurityContextHolder.getContext().setAuthentication(token);
-
-        ITargetObject target = makeInterceptedTarget();
-        String result = target.makeLowerCase("HELLO");
-
-        assertEquals("hello org.springframework.security.providers.UsernamePasswordAuthenticationToken true", result);
-    }
-
-    public void testNullReturnedIfZeroAttributesDefinedForMethodInvocation()
-        throws Exception {
-        // SomeDomain is not defined in the MockAttributes()
-        // (which getConfigAttributeDefinition refers to)
-        ConfigAttributeDefinition def = getConfigAttributeDefinition(SomeDomain.class, "getId", null);
-        assertNull(def);
-    }
-
-    /**
-     * convert a <code>ConfigAttributeDefinition</code> into a set of <code>ConfigAttribute</code>(s)
-     *
-     * @param def the <code>ConfigAttributeDefinition</code> to cover
-     *
-     * @return a Set of <code>ConfigAttributes</code>
-     */
-    private Set toSet(ConfigAttributeDefinition def) {
-        Set set = new HashSet();
-        Iterator i = def.getConfigAttributes().iterator();
-
-        while (i.hasNext()) {
-            ConfigAttribute a = (ConfigAttribute) i.next();
-            set.add(a);
-        }
-
-        return set;
-    }
 }

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

@@ -18,7 +18,9 @@ package org.springframework.security.intercept.method;
 import junit.framework.TestCase;
 
 import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.ITargetObject;
 import org.springframework.security.MockJoinPoint;
+import org.springframework.security.OtherTargetObject;
 import org.springframework.security.TargetObject;
 
 import org.aopalliance.intercept.MethodInvocation;
@@ -30,7 +32,7 @@ import java.util.Iterator;
 
 
 /**
- * Tests {@link MethodDefinitionSourceEditor} and its associated {@link MethodDefinitionMap}.
+ * Tests {@link MethodDefinitionSourceEditor} and its associated {@link MapBasedMethodDefinitionSource}.
  *
  * @author Ben Alex
  * @version $Id$
@@ -55,7 +57,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("org.springframework.security.TargetObject.countLength=ROLE_ONE,ROLE_TWO,RUN_AS_ENTRY");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
 
         Class clazz = TargetObject.class;
         Method method = clazz.getMethod("countLength", new Class[] {String.class});
@@ -101,32 +103,41 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         }
     }
 
-    public void testConcreteClassInvocationsAlsoReturnDefinitionsAgainstInterface()
-        throws Exception {
+    public void testConcreteClassInvocationsAlsoReturnDefinitionsAgainstInterface() throws Exception {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText(
-            "org.springframework.security.ITargetObject.makeLower*=ROLE_FROM_INTERFACE\r\norg.springframework.security.ITargetObject.makeUpper*=ROLE_FROM_INTERFACE\r\norg.springframework.security.TargetObject.makeUpper*=ROLE_FROM_IMPLEMENTATION");
+            "org.springframework.security.ITargetObject.computeHashCode*=ROLE_FROM_INTERFACE\r\n" +
+            "org.springframework.security.ITargetObject.makeLower*=ROLE_FROM_INTERFACE\r\n" +
+            "org.springframework.security.ITargetObject.makeUpper*=ROLE_FROM_INTERFACE\r\n" +
+            "org.springframework.security.TargetObject.computeHashCode*=ROLE_FROM_TO\r\n" +
+            "org.springframework.security.OtherTargetObject.computeHashCode*=ROLE_FROM_OTO\r\n" +
+            "org.springframework.security.OtherTargetObject.makeUpper*=ROLE_FROM_IMPLEMENTATION");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
-        assertEquals(3, map.getMethodMapSize());
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
+        assertEquals(6, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(TargetObject.class,
-                    "makeLowerCase", new Class[] {String.class}));
+        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new OtherTargetObject()));
         ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE");
         assertEquals(expectedMakeLower, returnedMakeLower);
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(TargetObject.class,
-                    "makeUpperCase", new Class[] {String.class}));
-        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(
-                new String[]{"ROLE_FROM_IMPLEMENTATION","ROLE_FROM_INTERFACE"});
+        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeUpperCase", new Class[] {String.class}, new OtherTargetObject()));
+        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"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"});
+        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"});
+        assertEquals(expectedComputeHashCode, returnedComputeHashCode);
     }
 
     public void testEmptyStringReturnsEmptyMap() {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(0, map.getMethodMapSize());
     }
 
@@ -135,7 +146,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         editor.setAsText(
             "org.springframework.security.TargetObject.countLength=ROLE_ONE,ROLE_TWO,RUN_AS_ENTRY\r\norg.springframework.security.TargetObject.make*=ROLE_NINE,ROLE_SUPERVISOR");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         Iterator iter = map.getConfigAttributeDefinitions().iterator();
         int counter = 0;
 
@@ -152,7 +163,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         editor.setAsText(
             "org.springframework.security.TargetObject.countLength=ROLE_ONE,ROLE_TWO,RUN_AS_ENTRY\r\norg.springframework.security.TargetObject.make*=ROLE_NINE,ROLE_SUPERVISOR");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(3, map.getMethodMapSize());
     }
 
@@ -161,21 +172,21 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         editor.setAsText(
             "org.springframework.security.TargetObject.*=ROLE_GENERAL\r\norg.springframework.security.TargetObject.makeLower*=ROLE_LOWER\r\norg.springframework.security.TargetObject.make*=ROLE_MAKE\r\norg.springframework.security.TargetObject.makeUpper*=ROLE_UPPER");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
-        assertEquals(5, map.getMethodMapSize());
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
+        assertEquals(14, map.getMethodMapSize());
 
-        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(TargetObject.class,
-                    "makeLowerCase", new Class[] {String.class}));
+        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+                    "makeLowerCase", new Class[] {String.class}, new TargetObject()));
         ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_LOWER");
         assertEquals(expectedMakeLower, returnedMakeLower);
 
-        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(TargetObject.class,
-                    "makeUpperCase", new Class[] {String.class}));
+        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+                    "makeUpperCase", new Class[] {String.class}, new TargetObject()));
         ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition("ROLE_UPPER");
         assertEquals(expectedMakeUpper, returnedMakeUpper);
 
-        ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(TargetObject.class,
-                    "countLength", new Class[] {String.class}));
+        ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class,
+                    "countLength", new Class[] {String.class}, new TargetObject()));
         ConfigAttributeDefinition expectedCountLength = new ConfigAttributeDefinition("ROLE_GENERAL");
         assertEquals(expectedCountLength, returnedCountLength);
     }
@@ -184,10 +195,10 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("org.springframework.security.TargetObject.countLength=ROLE_ONE,ROLE_TWO,RUN_AS_ENTRY");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
 
         ConfigAttributeDefinition configAttributeDefinition = map.getAttributes(new MockMethodInvocation(
-                    TargetObject.class, "makeLowerCase", new Class[] {String.class}));
+                    ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new TargetObject()));
         assertNull(configAttributeDefinition);
     }
 
@@ -195,7 +206,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText(null);
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         assertEquals(0, map.getMethodMapSize());
     }
 
@@ -203,10 +214,10 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("org.springframework.security.TargetObject.countLength=ROLE_ONE,ROLE_TWO,RUN_AS_ENTRY");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
 
-        ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(TargetObject.class,
-                    "countLength", new Class[] {String.class}));
+        ConfigAttributeDefinition 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);
@@ -215,11 +226,13 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
     //~ Inner Classes ==================================================================================================
 
     private class MockMethodInvocation implements MethodInvocation {
-        Method method;
+        private Method method;
+        private Object targetObject;
 
-        public MockMethodInvocation(Class clazz, String methodName, Class[] parameterTypes)
+        public MockMethodInvocation(Class clazz, String methodName, Class[] parameterTypes, Object targetObject)
             throws NoSuchMethodException {
-            method = clazz.getMethod(methodName, parameterTypes);
+            this.method = clazz.getMethod(methodName, parameterTypes);
+            this.targetObject = targetObject;
         }
 
         public Object[] getArguments() {
@@ -235,7 +248,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         }
 
         public Object getThis() {
-            return null;
+            return targetObject;
         }
 
         public Object proceed() throws Throwable {

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

@@ -20,6 +20,7 @@ import junit.framework.TestCase;
 import org.springframework.security.GrantedAuthority;
 import org.springframework.security.GrantedAuthorityImpl;
 import org.springframework.security.ITargetObject;
+import org.springframework.security.OtherTargetObject;
 
 import org.springframework.security.intercept.method.aopalliance.MethodSecurityInterceptor;
 
@@ -88,7 +89,7 @@ public class MethodInvocationPrivilegeEvaluatorTests extends TestCase {
         throws Exception {
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
                 new GrantedAuthority[] {new GrantedAuthorityImpl("MOCK_LOWER")});
-        MethodInvocation mi = MethodInvocationUtils.createFromClass(ITargetObject.class, "makeLowerCase",
+        MethodInvocation mi = MethodInvocationUtils.createFromClass(new OtherTargetObject(), ITargetObject.class, "makeLowerCase",
                 new Class[] {String.class}, new Object[] {"Hello world"});
         MethodSecurityInterceptor interceptor = makeSecurityInterceptor();
 
@@ -117,7 +118,7 @@ public class MethodInvocationPrivilegeEvaluatorTests extends TestCase {
         throws Exception {
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
                 new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_NOT_HELD")});
-        MethodInvocation mi = MethodInvocationUtils.createFromClass(ITargetObject.class, "makeLowerCase",
+        MethodInvocation mi = MethodInvocationUtils.createFromClass(new OtherTargetObject(), ITargetObject.class, "makeLowerCase",
                 new Class[] {String.class}, new Object[] {"helloWorld"});
         MethodSecurityInterceptor interceptor = makeSecurityInterceptor();
 

+ 4 - 0
core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java

@@ -71,4 +71,8 @@ public class MockMethodDefinitionSource extends AbstractMethodDefinitionSource {
     protected ConfigAttributeDefinition lookupAttributes(Method method) {
         throw new UnsupportedOperationException("mock method not implemented");
     }
+
+	public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) {
+        throw new UnsupportedOperationException("mock method not implemented");
+	}
 }

+ 6 - 16
core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodDefinitionSourceAdvisorTests.java

@@ -15,17 +15,14 @@
 
 package org.springframework.security.intercept.method.aopalliance;
 
+import java.lang.reflect.Method;
+
 import junit.framework.TestCase;
 
 import org.springframework.security.TargetObject;
-
-import org.springframework.security.intercept.method.MethodDefinitionMap;
+import org.springframework.security.intercept.method.MapBasedMethodDefinitionSource;
 import org.springframework.security.intercept.method.MethodDefinitionSourceEditor;
 
-import org.springframework.aop.framework.AopConfigException;
-
-import java.lang.reflect.Method;
-
 
 /**
  * Tests {@link MethodDefinitionSourceAdvisor}.
@@ -50,7 +47,7 @@ public class MethodDefinitionSourceAdvisorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("org.springframework.security.TargetObject.countLength=ROLE_NOT_USED");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
 
         MethodSecurityInterceptor msi = new MethodSecurityInterceptor();
         msi.setObjectDefinitionSource(map);
@@ -90,7 +87,7 @@ public class MethodDefinitionSourceAdvisorTests extends TestCase {
         try {
             new MethodDefinitionSourceAdvisor(msi);
             fail("Should have detected null ObjectDefinitionSource and thrown AopConfigException");
-        } catch (AopConfigException expected) {
+        } catch (IllegalArgumentException expected) {
             assertTrue(true);
         }
     }
@@ -99,7 +96,7 @@ public class MethodDefinitionSourceAdvisorTests extends TestCase {
         Class clazz = TargetObject.class;
         Method method = clazz.getMethod("countLength", new Class[] {String.class});
 
-        MethodDefinitionSourceAdvisor.InternalMethodInvocation imi = new MethodDefinitionSourceAdvisor(getInterceptor()).new InternalMethodInvocation(method);
+        MethodDefinitionSourceAdvisor.InternalMethodInvocation imi = new MethodDefinitionSourceAdvisor(getInterceptor()).new InternalMethodInvocation(method, clazz);
 
         try {
             imi.getArguments();
@@ -115,13 +112,6 @@ public class MethodDefinitionSourceAdvisorTests extends TestCase {
             assertTrue(true);
         }
 
-        try {
-            imi.getThis();
-            fail("Should have thrown UnsupportedOperationException");
-        } catch (UnsupportedOperationException expected) {
-            assertTrue(true);
-        }
-
         try {
             imi.proceed();
             fail("Should have thrown UnsupportedOperationException");

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

@@ -455,7 +455,11 @@ public class MethodSecurityInterceptorTests extends TestCase {
             throw new UnsupportedOperationException("mock method not implemented");
         }
 
-        public boolean supports(Class clazz) {
+        public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) {
+            throw new UnsupportedOperationException("mock method not implemented");
+		}
+
+		public boolean supports(Class clazz) {
             if (String.class.isAssignableFrom(clazz)) {
                 return true;
             } else {

+ 6 - 9
core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java

@@ -15,27 +15,24 @@
 
 package org.springframework.security.intercept.method.aspectj;
 
+import java.lang.reflect.Method;
+
 import junit.framework.TestCase;
 
 import org.springframework.security.AccessDeniedException;
 import org.springframework.security.GrantedAuthority;
 import org.springframework.security.GrantedAuthorityImpl;
 import org.springframework.security.MockAccessDecisionManager;
+import org.springframework.security.MockApplicationEventPublisher;
 import org.springframework.security.MockAuthenticationManager;
 import org.springframework.security.MockJoinPoint;
 import org.springframework.security.MockRunAsManager;
 import org.springframework.security.TargetObject;
-import org.springframework.security.MockApplicationEventPublisher;
-
 import org.springframework.security.context.SecurityContextHolder;
-
-import org.springframework.security.intercept.method.MethodDefinitionMap;
+import org.springframework.security.intercept.method.MapBasedMethodDefinitionSource;
 import org.springframework.security.intercept.method.MethodDefinitionSourceEditor;
-
 import org.springframework.security.providers.TestingAuthenticationToken;
 
-import java.lang.reflect.Method;
-
 
 /**
  * Tests {@link AspectJSecurityInterceptor}.
@@ -74,7 +71,7 @@ public class AspectJSecurityInterceptorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("org.springframework.security.TargetObject.countLength=MOCK_ONE,MOCK_TWO");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         si.setObjectDefinitionSource(map);
         assertEquals(map, si.getObjectDefinitionSource());
 
@@ -105,7 +102,7 @@ public class AspectJSecurityInterceptorTests extends TestCase {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("org.springframework.security.TargetObject.countLength=MOCK_ONE,MOCK_TWO");
 
-        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue();
         si.setObjectDefinitionSource(map);
 
         si.afterPropertiesSet();

+ 2 - 2
core/src/test/java/org/springframework/security/vote/BasicAclEntryVoterTests.java

@@ -56,7 +56,7 @@ public class BasicAclEntryVoterTests extends TestCase {
         Class clazz = SomeDomainObjectManager.class;
         Method method = clazz.getMethod("someServiceMethod", new Class[] {SomeDomainObject.class});
 
-        return new SimpleMethodInvocation(method, new Object[] {domainObject});
+        return new SimpleMethodInvocation(new SomeDomainObjectManager(), method, new Object[] {domainObject});
     }
 
     public static void main(String[] args) {
@@ -419,7 +419,7 @@ public class BasicAclEntryVoterTests extends TestCase {
         Class clazz = String.class;
         Method method = clazz.getMethod("toString", new Class[]{});
 
-        MethodInvocation mi = new SimpleMethodInvocation(method, new Object[]{domainObject});
+        MethodInvocation mi = new SimpleMethodInvocation(new String(), method, new Object[]{domainObject});
 
         try {
             voter.vote(new UsernamePasswordAuthenticationToken("rod", null), mi, attr);

+ 2 - 2
core/src/test/resources/org/springframework/security/config/method-security.xml

@@ -11,8 +11,8 @@ http://www.springframework.org/schema/security http://www.springframework.org/sc
         <intercept-methods>
             <!-- TODO: It would be better if we didn't need the package/interface names here -->
             <protect method="org.springframework.security.config.TestBusinessBean.set*" access="ROLE_ADMIN" />
-            <protect method="org.springframework.security.config.TestBusinessBean.get*" access="ROLE_ADMIN,ROLE_USER" />
-            <protect method="org.springframework.security.config.TestBusinessBean.doSomething" access="ROLE_USER" />
+            <protect method="get*" access="ROLE_ADMIN,ROLE_USER" />
+            <protect method="doSomething" access="ROLE_USER" />
         </intercept-methods>
     </b:bean>
 

+ 51 - 0
sandbox/other/src/main/java/org/springframework/security/AspectJParsingTests.java

@@ -0,0 +1,51 @@
+package org.springframework.security;
+
+import java.lang.reflect.Method;
+import java.util.HashSet;
+import java.util.Set;
+
+import junit.framework.Assert;
+
+import org.aspectj.lang.annotation.Pointcut;
+import org.aspectj.weaver.tools.PointcutExpression;
+import org.aspectj.weaver.tools.PointcutParser;
+import org.aspectj.weaver.tools.PointcutPrimitive;
+import org.junit.Test;
+
+/**
+ * A quick play with AspectJ pointcut parsing. Was contemplating using this for MapBasedMethodDefinitionSource refactoring,
+ * but decided to revisit at a future point. Requires aspectjweaver-1.5.3.jar in classpath.
+ * 
+ * @author Ben Alex
+ */
+
+public class AspectJParsingTests {
+	private static final Set DEFAULT_SUPPORTED_PRIMITIVES = new HashSet();
+
+	@Pointcut("execution(int TargetObject.countLength(String))")
+	public void goodPointcut() {}
+
+	static {
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.CALL);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.EXECUTION);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.ARGS);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.REFERENCE);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.THIS);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.TARGET);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.WITHIN);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.AT_ANNOTATION);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.AT_WITHIN);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.AT_ARGS);
+		DEFAULT_SUPPORTED_PRIMITIVES.add(PointcutPrimitive.AT_TARGET);
+	}
+
+	@Test
+	public void testMatches() throws Exception {
+		PointcutParser parser = PointcutParser.getPointcutParserSupportingSpecifiedPrimitivesAndUsingContextClassloaderForResolution(DEFAULT_SUPPORTED_PRIMITIVES);
+		PointcutExpression expression = parser.parsePointcutExpression("org.springframework.security.AspectJParsingTests.goodPointcut()");
+
+		Method exec = OtherTargetObject.class.getMethod("countLength", new Class[] {String.class});
+		Assert.assertTrue(expression.matchesMethodExecution(exec).alwaysMatches());
+	}
+
+}