diff --git a/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java b/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java index c6acd8fee0..98940d11c2 100644 --- a/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java +++ b/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java @@ -13,6 +13,13 @@ public class SecurityExpressionRoot { private Object filterObject; private Object returnObject; + /** Allows "permitAll" expression */ + public final boolean permitAll = true; + + /** Allows "denyAll" expression */ + public final boolean denyAll = false; + + public SecurityExpressionRoot(Authentication a) { this.authentication = a; } diff --git a/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java index bc9f930ff5..460b60419e 100644 --- a/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java @@ -1,5 +1,6 @@ package org.springframework.security.expression.support; +import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; @@ -13,7 +14,8 @@ import org.springframework.security.expression.annotation.PostAuthorize; import org.springframework.security.expression.annotation.PostFilter; import org.springframework.security.expression.annotation.PreAuthorize; import org.springframework.security.expression.annotation.PreFilter; -import org.springframework.security.intercept.method.AbstractFallbackMethodDefinitionSource; +import org.springframework.security.intercept.method.AbstractMethodDefinitionSource; +import org.springframework.util.ClassUtils; /** * MethodDefinitionSource which extracts metadata from the @PreFilter and @PreAuthorize annotations @@ -22,6 +24,9 @@ import org.springframework.security.intercept.method.AbstractFallbackMethodDefin * Annotations may be specified on classes or methods, and method-specific annotations will take precedence. * If you use any annotation and do not specify a pre-authorization condition, then the method will be * allowed as if a @PreAuthorize("permitAll") were present. + *

+ * Since we are handling multiple annotations here, it's possible that we may have to combine annotations defined in + * multiple locations for a single method - they may be defined on the method itself, or at interface or class level. * * @see MethodExpressionVoter * @@ -29,56 +34,71 @@ import org.springframework.security.intercept.method.AbstractFallbackMethodDefin * @since 2.5 * @version $Id$ */ -public class ExpressionAnnotationMethodDefinitionSource extends AbstractFallbackMethodDefinitionSource { +public class ExpressionAnnotationMethodDefinitionSource extends AbstractMethodDefinitionSource { - @Override - protected List findAttributes(Method method, Class targetClass) { - PreFilter preFilter = AnnotationUtils.findAnnotation(method, PreFilter.class); - PreAuthorize preAuthorize = AnnotationUtils.findAnnotation(method, PreAuthorize.class); - PostFilter postFilter = AnnotationUtils.findAnnotation(method, PostFilter.class); - PostAuthorize postAuthorize = AnnotationUtils.findAnnotation(method, PostAuthorize.class); + public List getAttributes(Method method, Class targetClass) { + logger.debug("Looking for expression annotations for method '" + + method.getName() + "' on target class '" + targetClass + "'"); + PreFilter preFilter = findAnnotation(method, targetClass, PreFilter.class); + PreAuthorize preAuthorize = findAnnotation(method, targetClass, PreAuthorize.class); + PostFilter postFilter = findAnnotation(method, targetClass, PostFilter.class); + // TODO: Can we check for void methods and throw an exception here? + PostAuthorize postAuthorize = findAnnotation(method, targetClass, PostAuthorize.class); if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null ) { - // There is no method level meta-data so return and allow parent to query at class-level + // There is no meta-data so return + logger.debug("No expression annotations found"); return null; } - // There is at least one non-null value, so the parent class will not query for class-specific annotations - // and we have to locate them here as appropriate. - - if (preAuthorize == null) { - preAuthorize = (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class); - } - - if (preFilter == null) { - preFilter = (PreFilter)targetClass.getAnnotation(PreFilter.class); - } - - if (postFilter == null) { - // TODO: Can we check for void methods and throw an exception here? - postFilter = (PostFilter)targetClass.getAnnotation(PostFilter.class); - } - - if (postAuthorize == null) { - postAuthorize = (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class); - } - return createAttributeList(preFilter, preAuthorize, postFilter, postAuthorize); } - @Override - protected List findAttributes(Class targetClass) { - PreFilter preFilter = (PreFilter)targetClass.getAnnotation(PreFilter.class); - PreAuthorize preAuthorize = (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class); - PostFilter postFilter = (PostFilter)targetClass.getAnnotation(PostFilter.class); - PostAuthorize postAuthorize = (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class); + /** + * See {@link org.springframework.security.intercept.method.AbstractFallbackMethodDefinitionSource#getAttributes(Method, Class)} + * for the logic of this method. The ordering here is slightly different in that we consider method-specific + * annotations on an interface before class-level ones. + */ + private A findAnnotation(Method method, Class targetClass, Class annotationClass) { + // 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); + A annotation = AnnotationUtils.findAnnotation(specificMethod, annotationClass); - if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null ) { - // There is no class level meta-data (and by implication no meta-data at all) - return null; + if (annotation != null) { + logger.debug(annotation + " found on specific method: " + specificMethod); + return annotation; } - return createAttributeList(preFilter, preAuthorize, postFilter, postAuthorize); + // Check the original (e.g. interface) method + if (specificMethod != method) { + annotation = AnnotationUtils.findAnnotation(method, annotationClass); + + if (annotation != null) { + logger.debug(annotation + " found on: " + method); + return annotation; + } + } + + // Check the class-level (note declaringClass, not targetClass, which may not actually implement the method) + annotation = specificMethod.getDeclaringClass().getAnnotation(annotationClass); + + if (annotation != null) { + logger.debug(annotation + " found on: " + specificMethod.getDeclaringClass().getName()); + return annotation; + } + + // Check for a possible interface annotation which would not be inherited by the declaring class + if (specificMethod != method) { + annotation = method.getDeclaringClass().getAnnotation(annotationClass); + + if (annotation != null) { + logger.debug(annotation + " found on: " + method.getDeclaringClass().getName()); + return annotation; + } + } + + return null; } public Collection> getConfigAttributeDefinitions() { @@ -91,7 +111,7 @@ public class ExpressionAnnotationMethodDefinitionSource extends AbstractFallback ConfigAttribute post = null; // TODO: Optimization of permitAll - String preAuthorizeExpression = preAuthorize == null ? "permitAll()" : preAuthorize.value(); + String preAuthorizeExpression = preAuthorize == null ? "permitAll" : preAuthorize.value(); String preFilterExpression = preFilter == null ? null : preFilter.value(); String filterObject = preFilter == null ? null : preFilter.filterTarget(); String postAuthorizeExpression = postAuthorize == null ? null : postAuthorize.value(); @@ -106,7 +126,6 @@ public class ExpressionAnnotationMethodDefinitionSource extends AbstractFallback throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e); } - List attrs = new ArrayList(2); if (pre != null) { attrs.add(pre); diff --git a/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java index 763c0f0f01..7f4337fd02 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java @@ -1,117 +1,33 @@ package org.springframework.security.intercept.method; import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; -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.ConfigAttribute; -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. - * + * performs attribute resolution from: 1. specific target method; 2. target class; 3. declaring method; + * 4. declaring class/interface. Use with {@link DelegatingMethodDefinitionSource} for caching support. *

* This class mimics the behaviour of Spring's AbstractFallbackTransactionAttributeSource class. - *

- * *

* 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 + * a target method/class (i.e. #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 MethodDefinitionSource. - *

* * @author Ben Alex + * @author Luke taylor * @version $Id$ * @since 2.0 */ -public abstract class AbstractFallbackMethodDefinitionSource implements MethodDefinitionSource { - protected final Log logger = LogFactory.getLog(getClass()); - - private final static List NULL_CONFIG_ATTRIBUTE = new ArrayList(0); - private final Map> attributeCache = new HashMap(); - - public List getAttributes(Object object) throws IllegalArgumentException { - Assert.notNull(object, "Object cannot be null"); - - if (object instanceof MethodInvocation) { - MethodInvocation mi = (MethodInvocation) object; - Object target = mi.getThis(); - return getAttributes(mi.getMethod(), target == null ? null : target.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 abstract class AbstractFallbackMethodDefinitionSource extends AbstractMethodDefinitionSource { public List getAttributes(Method method, Class targetClass) { - // First, see if we have a cached value. - DefaultCacheKey cacheKey = new DefaultCacheKey(method, targetClass); - synchronized (attributeCache) { - List cached = 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 cached; - } - } - else { - // We need to work it out. - List attributes = computeAttributes(method, targetClass); - // Put it in the cache. - if (attributes == null) { - this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE); - } else { - if (logger.isDebugEnabled()) { - logger.debug("Adding security method [" + cacheKey + "] with attributes " + attributes); - } - this.attributeCache.put(cacheKey, attributes); - } - return attributes; - } - } - } - - /** - * - * @param method the method for the current invocation (never null) - * @param targetClass the target class for this invocation (may be null) - * @return - */ - private List 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); @@ -137,7 +53,6 @@ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDe return findAttributes(method.getDeclaringClass()); } return null; - } /** @@ -169,35 +84,5 @@ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDe */ protected abstract List 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 + "]"; - } - } } diff --git a/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java index 5a1648ba78..8e9f9a2b40 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java @@ -26,73 +26,50 @@ import org.aspectj.lang.JoinPoint; import org.aspectj.lang.reflect.CodeSignature; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import java.lang.reflect.Method; import java.util.List; /** - * Abstract implementation of MethodDefinitionSource. + * Abstract implementation of MethodDefinitionSource which resolves the secured object type to + * either a MethodInvocation or a JoinPoint. * * @author Ben Alex - * @deprecated No longer used within the framework and will be removed + * @author Luke Taylor * @version $Id$ */ public abstract class AbstractMethodDefinitionSource implements MethodDefinitionSource { - //~ Static fields/initializers ===================================================================================== - private static final Log logger = LogFactory.getLog(AbstractMethodDefinitionSource.class); + protected final Log logger = LogFactory.getLog(getClass()); //~ Methods ======================================================================================================== - public List getAttributes(Object object) - throws IllegalArgumentException { - Assert.notNull(object, "Object cannot be null"); - + public final List getAttributes(Object object) { if (object instanceof MethodInvocation) { - return this.lookupAttributes(((MethodInvocation) object).getMethod()); + MethodInvocation mi = (MethodInvocation) object; + Object target = mi.getThis(); + return getAttributes(mi.getMethod(), target == null ? null : target.getClass()); } if (object instanceof JoinPoint) { JoinPoint jp = (JoinPoint) object; - Class targetClazz = jp.getTarget().getClass(); + 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(); - if (logger.isDebugEnabled()) { - logger.debug("Target Class: " + targetClazz); - logger.debug("Target Method Name: " + targetMethodName); + Method method = ClassUtils.getMethodIfAvailable(declaringType, targetMethodName, types); + Assert.notNull(method, "Could not obtain target method from JoinPoint: '"+ jp + "'"); - for (int i = 0; i < types.length; i++) { - if (logger.isDebugEnabled()) { - logger.debug("Target Method Arg #" + i + ": " + types[i]); - } - } - } - - try { - return this.lookupAttributes(targetClazz.getMethod(targetMethodName, types)); - } catch (NoSuchMethodException nsme) { - throw new IllegalArgumentException("Could not obtain target method from JoinPoint: " + jp); - } + return getAttributes(method, targetClass); } - throw new IllegalArgumentException("Object must be a MethodInvocation or JoinPoint"); + throw new IllegalArgumentException("Object must be a non-null MethodInvocation or JoinPoint"); } - /** - * Performs the actual lookup of the relevant ConfigAttributeDefinition for the specified - * Method which is subject of the method invocation.

Provided so subclasses need only to - * provide one basic method to properly interface with the MethodDefinitionSource.

- *

Returns null if there are no matching attributes for the method.

- * - * @param method the method being invoked for which configuration attributes should be looked up - * - * @return the ConfigAttributeDefinition that applies to the specified Method - */ - protected abstract List lookupAttributes(Method method); - - public boolean supports(Class clazz) { + public final boolean supports(Class clazz) { return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz)); } } diff --git a/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java index 66bea4e503..c6e1057d3e 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java @@ -2,52 +2,75 @@ package org.springframework.security.intercept.method; import java.lang.reflect.Method; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import org.springframework.beans.factory.InitializingBean; import org.springframework.security.ConfigAttribute; -import org.springframework.security.ConfigAttributeDefinition; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * Automatically tries a series of method definition sources, relying on the first source of metadata - * that provides a non-null response. + * that provides a non-null response. Provides automatic caching of the retrieved metadata. * * @author Ben Alex + * @author Luke Taylor * @version $Id$ */ -public final class DelegatingMethodDefinitionSource implements MethodDefinitionSource, InitializingBean { - private List methodDefinitionSources; +public final class DelegatingMethodDefinitionSource extends AbstractMethodDefinitionSource implements InitializingBean { + private final static List NULL_CONFIG_ATTRIBUTE = Collections.emptyList(); + + private List methodDefinitionSources; + private final Map> attributeCache = new HashMap(); + + //~ Methods ======================================================================================================== public void afterPropertiesSet() throws Exception { Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required"); } public List getAttributes(Method method, Class targetClass) { - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - List result = s.getAttributes(method, targetClass); - if (result != null) { - return result; + DefaultCacheKey cacheKey = new DefaultCacheKey(method, targetClass); + synchronized (attributeCache) { + List cached = attributeCache.get(cacheKey); + // Check for canonical value indicating there is no config attribute, + if (cached == NULL_CONFIG_ATTRIBUTE) { + return null; } - } - return null; - } - public List getAttributes(Object object) throws IllegalArgumentException { - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - List result = s.getAttributes(object); - if (result != null) { - return result; + if (cached != null) { + return cached; } + + // No cached value, so query the sources to find a result + List attributes = null; + for (MethodDefinitionSource s : methodDefinitionSources) { + attributes = s.getAttributes(method, targetClass); + if (attributes != null) { + break; + } + } + + // Put it in the cache. + if (attributes == null) { + this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE); + return null; + } + + if (logger.isDebugEnabled()) { + logger.debug("Adding security method [" + cacheKey + "] with attributes " + attributes); + } + + this.attributeCache.put(cacheKey, attributes); + + return attributes; } - return null; } public Collection> getConfigAttributeDefinitions() { @@ -63,19 +86,40 @@ public final class DelegatingMethodDefinitionSource implements MethodDefinitionS return set; } - public boolean supports(Class clazz) { - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - if (s.supports(clazz)) { - return true; - } - } - return false; - } - public void setMethodDefinitionSources(List methodDefinitionSources) { Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required"); this.methodDefinitionSources = methodDefinitionSources; } + + //~ Inner Classes ================================================================================================== + + 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 + "]"; + } + } } diff --git a/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java index 17feea7f62..1cd24ea9f4 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java @@ -18,14 +18,11 @@ package org.springframework.security.intercept.method; 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 org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; @@ -47,9 +44,6 @@ import org.springframework.util.ClassUtils; * @since 2.0 */ public class MapBasedMethodDefinitionSource extends AbstractFallbackMethodDefinitionSource implements BeanClassLoaderAware { - //~ Static fields/initializers ===================================================================================== - - private static final Log logger = LogFactory.getLog(MapBasedMethodDefinitionSource.class); //~ Instance fields ================================================================================================ private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader(); diff --git a/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java b/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java new file mode 100644 index 0000000000..c20805e7cc --- /dev/null +++ b/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java @@ -0,0 +1,169 @@ +package org.springframework.security.expression.support; + +import static org.junit.Assert.*; + +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.security.ConfigAttribute; +import org.springframework.security.expression.annotation.PostAuthorize; +import org.springframework.security.expression.annotation.PostFilter; +import org.springframework.security.expression.annotation.PreAuthorize; +import org.springframework.security.expression.annotation.PreFilter; +import org.springframework.security.intercept.method.MockMethodInvocation; + + +public class ExpressionAnnotationMethodDefinitionSourceTests { + private ExpressionAnnotationMethodDefinitionSource mds = new ExpressionAnnotationMethodDefinitionSource(); + + private MockMethodInvocation voidImpl1; + private MockMethodInvocation voidImpl2; + private MockMethodInvocation voidImpl3; + private MockMethodInvocation listImpl1; + private MockMethodInvocation notherListImpl1; + private MockMethodInvocation notherListImpl2; + + @Before + public void setUpData() throws Exception { + voidImpl1 = new MockMethodInvocation(new ReturnVoidImpl1(), ReturnVoid.class, "doSomething", List.class); + voidImpl2 = new MockMethodInvocation(new ReturnVoidImpl2(), ReturnVoid.class, "doSomething", List.class); + voidImpl3 = new MockMethodInvocation(new ReturnVoidImpl3(), ReturnVoid.class, "doSomething", List.class); + listImpl1 = new MockMethodInvocation(new ReturnAListImpl1(), ReturnAList.class, "doSomething", List.class); + notherListImpl1 = new MockMethodInvocation(new ReturnAnotherListImpl1(), ReturnAnotherList.class, "doSomething", List.class); + notherListImpl2 = new MockMethodInvocation(new ReturnAnotherListImpl2(), ReturnAnotherList.class, "doSomething", List.class); + } + + @Test + public void classLevelPreAnnotationIsPickedUpWhenNoMethodLevelExists() throws Exception { + List attrs = mds.getAttributes(voidImpl1); + + assertEquals(1, attrs.size()); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); + PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute) attrs.get(0); + assertNotNull(pre.getAuthorizeExpression()); + assertEquals("someExpression", pre.getAuthorizeExpression().getExpressionString()); + assertNull(pre.getFilterExpression()); + } + + @Test + public void mixedClassAndMethodPreAnnotationsAreBothIncluded() { + List attrs = mds.getAttributes(voidImpl2); + + assertEquals(1, attrs.size()); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); + PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertEquals("someExpression", pre.getAuthorizeExpression().getExpressionString()); + assertNotNull(pre.getFilterExpression()); + assertEquals("somePreFilterExpression", pre.getFilterExpression().getExpressionString()); + } + + @Test + public void methodWithPreFilterOnlyIsAllowed() { + List attrs = mds.getAttributes(voidImpl3); + + assertEquals(1, attrs.size()); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); + PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertEquals("permitAll", pre.getAuthorizeExpression().getExpressionString()); + assertNotNull(pre.getFilterExpression()); + assertEquals("somePreFilterExpression", pre.getFilterExpression().getExpressionString()); + } + + @Test + public void methodWithPostFilterOnlyIsAllowed() { + List attrs = mds.getAttributes(listImpl1); + + assertEquals(2, attrs.size()); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); + assertTrue(attrs.get(1) instanceof PostInvocationExpressionConfigAttribute); + PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + PostInvocationExpressionConfigAttribute post = (PostInvocationExpressionConfigAttribute)attrs.get(1); + assertEquals("permitAll", pre.getAuthorizeExpression().getExpressionString()); + assertNotNull(post.getFilterExpression()); + assertEquals("somePostFilterExpression", post.getFilterExpression().getExpressionString()); + } + + @Test + public void interfaceAttributesAreIncluded() { + List attrs = mds.getAttributes(notherListImpl1); + + assertEquals(1, attrs.size()); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); + PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertNotNull(pre.getFilterExpression()); + assertNotNull(pre.getAuthorizeExpression()); + assertEquals("interfaceMethodAuthzExpression", pre.getAuthorizeExpression().getExpressionString()); + assertEquals("interfacePreFilterExpression", pre.getFilterExpression().getExpressionString()); + } + + @Test + public void classAttributesTakesPrecedeceOverInterfaceAttributes() { + List attrs = mds.getAttributes(notherListImpl2); + + assertEquals(1, attrs.size()); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); + PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertNotNull(pre.getFilterExpression()); + assertNotNull(pre.getAuthorizeExpression()); + assertEquals("interfaceMethodAuthzExpression", pre.getAuthorizeExpression().getExpressionString()); + assertEquals("classMethodPreFilterExpression", pre.getFilterExpression().getExpressionString()); + } + + //~ Inner Classes ================================================================================================== + + public static interface ReturnVoid { + public void doSomething(List param); + } + + public static interface ReturnAList { + public List doSomething(List param); + } + + @PreAuthorize("interfaceAuthzExpression") + public static interface ReturnAnotherList { + @PreAuthorize("interfaceMethodAuthzExpression") + @PreFilter(filterTarget="param", value="interfacePreFilterExpression") + public List doSomething(List param); + } + + + @PreAuthorize("someExpression") + public static class ReturnVoidImpl1 implements ReturnVoid { + public void doSomething(List param) {} + } + + @PreAuthorize("someExpression") + public static class ReturnVoidImpl2 implements ReturnVoid { + @PreFilter(filterTarget="param", value="somePreFilterExpression") + public void doSomething(List param) {} + } + + public static class ReturnVoidImpl3 implements ReturnVoid { + @PreFilter(filterTarget="param", value="somePreFilterExpression") + public void doSomething(List param) {} + } + + public static class ReturnAListImpl1 implements ReturnAList { + @PostFilter("somePostFilterExpression") + public List doSomething(List param) {return param;} + } + + public static class ReturnAListImpl2 implements ReturnAList { + @PreAuthorize("someExpression") + @PreFilter(filterTarget="param", value="somePreFilterExpression") + @PostFilter("somePostFilterExpression") + @PostAuthorize("somePostAuthorizeExpression") + public List doSomething(List param) {return param;} + } + + public static class ReturnAnotherListImpl1 implements ReturnAnotherList { + public List doSomething(List param) {return param;} + } + + public static class ReturnAnotherListImpl2 implements ReturnAnotherList { + @PreFilter(filterTarget="param", value="classMethodPreFilterExpression") + public List doSomething(List param) {return param;} + } + +} diff --git a/core/src/test/java/org/springframework/security/intercept/method/MockMethodInvocation.java b/core/src/test/java/org/springframework/security/intercept/method/MockMethodInvocation.java index 842e4c9ab4..475dabb2c6 100644 --- a/core/src/test/java/org/springframework/security/intercept/method/MockMethodInvocation.java +++ b/core/src/test/java/org/springframework/security/intercept/method/MockMethodInvocation.java @@ -35,5 +35,3 @@ public class MockMethodInvocation implements MethodInvocation { return null; } } - - diff --git a/core/src/test/resources/log4j.properties b/core/src/test/resources/log4j.properties index 065156f6fe..9157b44c8e 100644 --- a/core/src/test/resources/log4j.properties +++ b/core/src/test/resources/log4j.properties @@ -6,7 +6,7 @@ log4j.rootCategory=INFO, stdout log4j.appender.stdout=org.apache.log4j.ConsoleAppender log4j.appender.stdout.layout=org.apache.log4j.PatternLayout -log4j.appender.stdout.layout.ConversionPattern=%d %p %c - %m%n +log4j.appender.stdout.layout.ConversionPattern=%d %p %c{1} - %m%n log4j.category.org.springframework.security=DEBUG log4j.category.org.springframework.ldap=DEBUG