From f123e9c3339c1b25aa0c9447ff4b3d1403d05c8f Mon Sep 17 00:00:00 2001 From: Ben Alex Date: Fri, 15 Oct 2004 03:47:53 +0000 Subject: [PATCH] Make MethodDefinitionMap query interfaces defined by secure objects, to properly support MethodDefinitionSourceAdvisor. --- changelog.txt | 2 + .../intercept/method/MethodDefinitionMap.java | 91 ++++++++++++++++++- .../MethodDefinitionSourceEditorTests.java | 28 ++++++ upgrade-06-07.txt | 29 ++++++ 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 upgrade-06-07.txt diff --git a/changelog.txt b/changelog.txt index 902b08b125..b51e64f483 100644 --- a/changelog.txt +++ b/changelog.txt @@ -2,6 +2,8 @@ Changes in version 0.7 (2004-xx-xx) ----------------------------------- * Added MethodDefinitionSourceAdvisor for performance and autoproxying +* Added MethodDefinitionMap querying of interfaces defined by secure objects +* Documentation improvements Changes in version 0.6.1 (2004-09-25) ------------------------------------- diff --git a/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java b/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java index 206139014e..51e66a1df9 100644 --- a/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java +++ b/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java @@ -15,6 +15,7 @@ package net.sf.acegisecurity.intercept.method; +import net.sf.acegisecurity.ConfigAttribute; import net.sf.acegisecurity.ConfigAttributeDefinition; import org.aopalliance.intercept.MethodInvocation; @@ -34,6 +35,32 @@ import java.util.Map; /** * Stores a {@link ConfigAttributeDefinition} for each method signature defined * in a bean context. + * + *

+ * For consistency with {@link MethodDefinitionAttributes} as well as support + * for {@link MethodDefinitionSourceAdvisor}, this implementation will return + * a ConfigAttributeDefinition containing all configuration + * attributes defined against: + * + *

+ *

+ * + *

+ * In general you should therefore define the interface methods of your + * secure objects, not the implementations. For example, define + * com.company.Foo.findAll=ROLE_TEST but not + * com.company.FooImpl.findAll=ROLE_TEST. + *

* * @author Ben Alex * @version $Id$ @@ -53,10 +80,28 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource { //~ Methods ================================================================ + /** + * Obtains the configuration attributes explicitly defined against this + * bean. This method will not return implicit configuration attributes + * that may be returned by {@link #lookupAttributes(MethodInvocation)} as + * it does not have access to a method invocation at this time. + * + * @return the attributes explicitly defined against this bean + */ public Iterator getConfigAttributeDefinitions() { return methodMap.values().iterator(); } + /** + * 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(MethodInvocation)} 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(); } @@ -165,7 +210,38 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource { } protected ConfigAttributeDefinition lookupAttributes(MethodInvocation mi) { - return (ConfigAttributeDefinition) this.methodMap.get(mi.getMethod()); + ConfigAttributeDefinition definition = new ConfigAttributeDefinition(); + + // Add attributes explictly defined for this method invocation + ConfigAttributeDefinition directlyAssigned = (ConfigAttributeDefinition) this.methodMap + .get(mi.getMethod()); + merge(definition, directlyAssigned); + + // Add attributes explicitly defined for this method invocation's interfaces + Class[] interfaces = mi.getMethod().getDeclaringClass().getInterfaces(); + + for (int i = 0; i < interfaces.length; i++) { + Class clazz = interfaces[i]; + + try { + // Look for the method on the current interface + Method interfaceMethod = clazz.getDeclaredMethod(mi.getMethod() + .getName(), + mi.getMethod().getParameterTypes()); + ConfigAttributeDefinition interfaceAssigned = (ConfigAttributeDefinition) this.methodMap + .get(interfaceMethod); + merge(definition, interfaceAssigned); + } catch (Exception e) { + // skip this interface + } + } + + // Return null if empty, as per abstract superclass contract + if (definition.size() == 0) { + return null; + } else { + return definition; + } } /** @@ -184,4 +260,17 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource { || (mappedName.startsWith("*") && methodName.endsWith(mappedName.substring(1, mappedName.length()))); } + + private void merge(ConfigAttributeDefinition definition, + ConfigAttributeDefinition toMerge) { + if (toMerge == null) { + return; + } + + Iterator attribs = toMerge.getConfigAttributes(); + + while (attribs.hasNext()) { + definition.addConfigAttribute((ConfigAttribute) attribs.next()); + } + } } diff --git a/core/src/test/java/org/acegisecurity/intercept/method/MethodDefinitionSourceEditorTests.java b/core/src/test/java/org/acegisecurity/intercept/method/MethodDefinitionSourceEditorTests.java index ca57e2e0fb..3da03fde5f 100644 --- a/core/src/test/java/org/acegisecurity/intercept/method/MethodDefinitionSourceEditorTests.java +++ b/core/src/test/java/org/acegisecurity/intercept/method/MethodDefinitionSourceEditorTests.java @@ -91,6 +91,34 @@ public class MethodDefinitionSourceEditorTests extends TestCase { } } + public void testConcreteClassInvocationsAlsoReturnDefinitionsAgainstInterface() + throws Exception { + MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor(); + editor.setAsText( + "net.sf.acegisecurity.ITargetObject.makeLower*=ROLE_FROM_INTERFACE\r\nnet.sf.acegisecurity.ITargetObject.makeUpper*=ROLE_FROM_INTERFACE\r\nnet.sf.acegisecurity.TargetObject.makeUpper*=ROLE_FROM_IMPLEMENTATION"); + + MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue(); + assertEquals(3, map.getMethodMapSize()); + + ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation( + TargetObject.class, "makeLowerCase", + new Class[] {String.class})); + ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition(); + expectedMakeLower.addConfigAttribute(new SecurityConfig( + "ROLE_FROM_INTERFACE")); + assertEquals(expectedMakeLower, returnedMakeLower); + + ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation( + TargetObject.class, "makeUpperCase", + new Class[] {String.class})); + ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(); + expectedMakeUpper.addConfigAttribute(new SecurityConfig( + "ROLE_FROM_IMPLEMENTATION")); + expectedMakeUpper.addConfigAttribute(new SecurityConfig( + "ROLE_FROM_INTERFACE")); + assertEquals(expectedMakeUpper, returnedMakeUpper); + } + public void testEmptyStringReturnsEmptyMap() { MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor(); editor.setAsText(""); diff --git a/upgrade-06-07.txt b/upgrade-06-07.txt new file mode 100644 index 0000000000..c9e9e9a2ba --- /dev/null +++ b/upgrade-06-07.txt @@ -0,0 +1,29 @@ +=============================================================================== + ACEGI SECURITY SYSTEM FOR SPRING - UPGRADING FROM 0.6 TO 0.7 +=============================================================================== + +The following should help most casual users of the project update their +applications: + +- MethodDefinitionMap, which is usually used by MethodSecurityInterceptor + for its objectDefinitionSource property, has been changed. From 0.7, when + MethodDefinitionMap is queried for configuration attributes associated with + secure MethodInvocations, it will use any method matching in the method + invocation class (as it always has) plus any method matching any interface + the MethodInvocation class directly implements. So consider a PersonManager + interface, a PersonManagerImpl class that implements it, and a definition of + PersonManager.findAll=ROLE_FOO. In this example, any query for either + PersonManager.findAll OR PersonManagerImpl.findAll will return ROLE_FOO. + As we have always encouraged definition against the interface names (as per + this example), this change should not adversely impact users. This change + was necessary because of the new MethodDefinitionSourceAdvisor (see below). + Refer to the MethodDefinitionMap JavaDocs for further clarification. + +- MethodDefinitionSourceAdvisor can now be used instead of defining proxies + for secure business objects. The advisor is fully compatible with both + MethodDefinitionMap and MethodDefinitionAttributes. Using an advisor allows + caching of which methods the MethodSecurityInterceptor should handle, thus + providing a performance benefit as MethodSecurityInterceptor is not called + for public (non-secure) objects. It also simplifies configuration. + +$Id$