SEC-507: Updated JSR-250 impl to include better support for PermitAll and DenyAll as suggested by Ryan Heaton. Includes JSR-250 voter which is now used by AnnotationDriverbeanDefinitionParser.

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

@@ -6,21 +6,22 @@ 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 <code>Attributes</code> metadata implementation used for secure method interception.
+ * 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
- * <code>RolesAllowed</code> Java JEE 5 annotation.
+ * Java JEE 5 annotations (<em>DenyAll</em>, <em>PermitAll</em> and <em>RolesAllowed</em>).
  * <p>
- * The <code>SecurityAnnotationAttributes</code> implementation can be used to configure a
- * <code>MethodDefinitionAttributes</code> and  <code>MethodSecurityInterceptor</code> bean definition.
  *
  * @author Mark St.Godard
  * @author Usama Rashwan
@@ -31,6 +32,7 @@ import java.lang.annotation.Annotation;
  */
 
 public class Jsr250SecurityAnnotationAttributes implements Attributes {
+    
     //~ Methods ========================================================================================================
 
     /**
@@ -48,19 +50,56 @@ public class Jsr250SecurityAnnotationAttributes implements Attributes {
     }
 
     /**
-     * Get the <code>RolesAllowed</code> attributes for a given target method.
+     * 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) {
-    	Annotation[] annotations = AnnotationUtils.getAnnotations(method);
-        Collection<SecurityConfig> attributes = populateSecurityConfigWithRolesAllowed(annotations);
-        // if there is no RolesAllowed defined on the Method then we will use the one defined on the class
-        // level , according to JSR 250
-        if (attributes.size()==0 && !method.isAnnotationPresent(PermitAll.class)) {
-        	attributes = populateSecurityConfigWithRolesAllowed(method.getDeclaringClass().getDeclaredAnnotations());
+        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;

core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityConfig.java

@@ -0,0 +1,22 @@
+package org.springframework.security.annotation;
+
+import org.springframework.security.SecurityConfig;
+
+import javax.annotation.security.PermitAll;
+import javax.annotation.security.DenyAll;
+
+/**
+ * Security config applicable as a JSR 250 annotation attribute.
+ *
+ * @author Ryan Heaton
+ * @since 2.0
+ */
+public class Jsr250SecurityConfig extends SecurityConfig {
+    public static final Jsr250SecurityConfig PERMIT_ALL_ATTRIBUTE = new Jsr250SecurityConfig(PermitAll.class.getName());
+    public static final Jsr250SecurityConfig DENY_ALL_ATTRIBUTE = new Jsr250SecurityConfig(DenyAll.class.getName());
+
+    public Jsr250SecurityConfig(String role) {
+        super(role);
+    }
+
+}

core-tiger/src/main/java/org/springframework/security/annotation/Jsr250Voter.java

@@ -0,0 +1,77 @@
+package org.springframework.security.annotation;
+
+import org.springframework.security.GrantedAuthority;
+import org.springframework.security.ConfigAttribute;
+import org.springframework.security.ConfigAttributeDefinition;
+import org.springframework.security.Authentication;
+import org.springframework.security.vote.AccessDecisionVoter;
+
+import java.util.Iterator;
+
+/**
+ * Voter on JSR-250 configuration attributes.
+ *
+ * @author Ryan Heaton
+ * @since 2.0
+ */
+public class Jsr250Voter implements AccessDecisionVoter {
+
+    /**
+     * The specified config attribute is supported if its an instance of a {@link Jsr250SecurityConfig}.
+     *
+     * @param configAttribute The config attribute.
+     * @return whether the config attribute is supported.
+     */
+    public boolean supports(ConfigAttribute configAttribute) {
+        return configAttribute instanceof Jsr250SecurityConfig;
+    }
+
+    /**
+     * All classes are supported.
+     *
+     * @param clazz the class.
+     * @return true
+     */
+    public boolean supports(Class clazz) {
+        return true;
+    }
+
+    /**
+     * Votes according to JSR 250.
+     *
+     * @param authentication The authentication object.
+     * @param object         The access object.
+     * @param definition     The configuration definition.
+     * @return The vote.
+     */
+    public int vote(Authentication authentication, Object object, ConfigAttributeDefinition definition) {
+        int result = ACCESS_ABSTAIN;
+        Iterator iter = definition.getConfigAttributes().iterator();
+
+        while (iter.hasNext()) {
+            ConfigAttribute attribute = (ConfigAttribute) iter.next();
+
+            if (Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE.equals(attribute)) {
+                return ACCESS_GRANTED;
+            }
+
+            if (Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE.equals(attribute)) {
+                return ACCESS_DENIED;
+            }
+
+            if (supports(attribute)) {
+                result = ACCESS_DENIED;
+
+                // Attempt to find a matching granted authority
+                for (GrantedAuthority authority : authentication.getAuthorities()) {
+                    if (attribute.getAttribute().equals(authority.getAuthority())) {
+                        return ACCESS_GRANTED;
+                    }
+                }
+            }
+        }
+
+        return result;
+    }
+}
+

core-tiger/src/test/java/org/springframework/security/annotation/BusinessService.java

@@ -16,11 +16,14 @@
 package org.springframework.security.annotation;
 
 import javax.annotation.security.RolesAllowed;
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.PermitAll;
 
 /**
  * @version $Id$
  */
 @Secured({"ROLE_USER"})
+@PermitAll
 public interface BusinessService {
     //~ Methods ========================================================================================================
 

core-tiger/src/test/java/org/springframework/security/annotation/Jsr250BusinessServiceImpl.java

@@ -1,12 +1,15 @@
 package org.springframework.security.annotation;
 
 import javax.annotation.security.RolesAllowed;
+import javax.annotation.security.DenyAll;
+import javax.annotation.security.PermitAll;
 
 /**
  *
  * @author Luke Taylor
  * @version $Id$
  */
+@PermitAll
 public class Jsr250BusinessServiceImpl implements BusinessService {
 
     @RolesAllowed({"ROLE_USER"})
@@ -25,7 +28,7 @@ public class Jsr250BusinessServiceImpl implements BusinessService {
     public void someAdminMethod() {
     }
 
-	public int someOther(int input) {
+    public int someOther(int input) {
 		return input;
 	}
 }

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

@@ -10,6 +10,7 @@ import java.util.ArrayList;
 
 import javax.annotation.security.RolesAllowed;
 import javax.annotation.security.PermitAll;
+import javax.annotation.security.DenyAll;
 
 /**
  * @author Luke Taylor
@@ -18,7 +19,8 @@ import javax.annotation.security.PermitAll;
 public class Jsr250SecurityAnnotationAttributesTests {
     Jsr250SecurityAnnotationAttributes attributes = new Jsr250SecurityAnnotationAttributes();
     A a = new A();
-    B b = new B();
+    UserAllowedClass userAllowed = new UserAllowedClass();
+    DenyAllClass denyAll = new DenyAllClass();
 
     @Test
     public void methodWithRolesAllowedHasCorrectAttribute() throws Exception {
@@ -31,10 +33,27 @@ public class Jsr250SecurityAnnotationAttributesTests {
     }
 
     @Test
-    public void permitAllMethodHasNoAttributes() throws Exception {
+    public void permitAllMethodHasPermitAllAttribute() throws Exception {
         List<SecurityConfig> accessAttributes =
                 new ArrayList<SecurityConfig>(attributes.getAttributes(a.getClass().getMethod("permitAllMethod")));
-        assertEquals(0, accessAttributes.size());
+        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
@@ -45,9 +64,9 @@ public class Jsr250SecurityAnnotationAttributesTests {
     }
     
     @Test
-    public void classRoleIsAppliedNoRoleMethod() throws Exception {
+    public void classRoleIsAppliedToNoRoleMethod() throws Exception {
         List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(b.getClass().getMethod("noRoleMethod")));
+                new ArrayList<SecurityConfig>(attributes.getAttributes(userAllowed.getClass().getMethod("noRoleMethod")));
         assertEquals(1, accessAttributes.size());
         assertEquals("USER", accessAttributes.get(0).getAttribute());
     }
@@ -55,7 +74,7 @@ public class Jsr250SecurityAnnotationAttributesTests {
     @Test
     public void methodRoleOverridesClassRole() throws Exception {
         List<SecurityConfig> accessAttributes =
-                new ArrayList<SecurityConfig>(attributes.getAttributes(b.getClass().getMethod("adminMethod")));
+                new ArrayList<SecurityConfig>(attributes.getAttributes(userAllowed.getClass().getMethod("adminMethod")));
         assertEquals(1, accessAttributes.size());
         assertEquals("ADMIN", accessAttributes.get(0).getAttribute());
     }
@@ -71,15 +90,25 @@ public class Jsr250SecurityAnnotationAttributesTests {
 
         @PermitAll
         public void permitAllMethod() {}
-
     }
 
     @RolesAllowed("USER")
-    public static class B {
+    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() {}        
+    }
+
+
+
 }

core-tiger/src/test/java/org/springframework/security/config/Jsr250AnnotationDrivenBeanDefinitionParserTests.java

@@ -40,6 +40,15 @@ public class Jsr250AnnotationDrivenBeanDefinitionParserTests {
         target.someUserMethod1();
     }
 
+    @Test
+    public void permitAllShouldBeDefaultAttribute() {
+        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
+                new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_USER")});
+        SecurityContextHolder.getContext().setAuthentication(token);
+
+        target.someOther(0);
+    }
+    
     @Test
     public void targetShouldAllowProtectedMethodInvocationWithCorrectRole() {
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",

core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java

@@ -25,11 +25,13 @@ import org.w3c.dom.Element;
 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 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";
 
     public BeanDefinition parse(Element element, ParserContext parserContext) {
-        String className = "true".equals(element.getAttribute(ATT_USE_JSR250)) ?
+        boolean useJsr250 = "true".equals(element.getAttribute(ATT_USE_JSR250));
+        String className = useJsr250 ?
                 JSR_250_SECURITY_ANNOTATION_ATTRIBUTES_CLASS : SECURITY_ANNOTATION_ATTRIBUTES_CLASS;
 
         // Reflectively obtain the Annotation-based ObjectDefinitionSource.
@@ -56,6 +58,11 @@ class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser {
 
         if (!StringUtils.hasText(accessManagerId)) {
             ConfigUtils.registerDefaultAccessManagerIfNecessary(parserContext);
+
+            if (useJsr250) {
+                ConfigUtils.addVoter(new RootBeanDefinition(JSR_250_VOTER_CLASS, null, null), parserContext);                
+            }
+
             accessManagerId = BeanIds.ACCESS_MANAGER;
         }
 

core/src/main/java/org/springframework/security/config/ConfigUtils.java

@@ -16,7 +16,6 @@ import org.springframework.security.vote.RoleVoter;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import java.util.Arrays;
 import java.util.Map;
 
 /**
@@ -32,15 +31,30 @@ public abstract class ConfigUtils {
     static void registerDefaultAccessManagerIfNecessary(ParserContext parserContext) {
 
         if (!parserContext.getRegistry().containsBeanDefinition(BeanIds.ACCESS_MANAGER)) {
+            ManagedList defaultVoters = new ManagedList(2);
+
+            defaultVoters.add(new RootBeanDefinition(RoleVoter.class));
+            defaultVoters.add(new RootBeanDefinition(AuthenticatedVoter.class));
+
             BeanDefinitionBuilder accessMgrBuilder = BeanDefinitionBuilder.rootBeanDefinition(AffirmativeBased.class);
-            accessMgrBuilder.addPropertyValue("decisionVoters",
-                            Arrays.asList(new Object[] {new RoleVoter(), new AuthenticatedVoter()}));
+            accessMgrBuilder.addPropertyValue("decisionVoters", defaultVoters);
             BeanDefinition accessMgr = accessMgrBuilder.getBeanDefinition();
 
             parserContext.getRegistry().registerBeanDefinition(BeanIds.ACCESS_MANAGER, accessMgr);
         }
     }
 
+    public static void addVoter(BeanDefinition voter, ParserContext parserContext) {
+        registerDefaultAccessManagerIfNecessary(parserContext);
+
+        BeanDefinition accessMgr = parserContext.getRegistry().getBeanDefinition(BeanIds.ACCESS_MANAGER);
+
+        ManagedList voters = (ManagedList) accessMgr.getPropertyValues().getPropertyValue("decisionVoters").getValue();
+        voters.add(voter);
+
+        accessMgr.getPropertyValues().addPropertyValue("decisionVoters", voter);
+    }
+
     /**
      * Creates and registers the bean definition for the default ProviderManager instance and returns
      * the BeanDefinition for it. This method will typically be called when registering authentication providers