From e518adbef178589487f48885ff3ae758461283ef Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Mon, 22 Mar 2010 16:26:04 +0000
Subject: [PATCH] SEC-1443: Modify Jsr250Voter to handle multiple
 "RolesAllowed" roles.

It now votes to abstain if there are no Jsr250 attributes present. If any are found, it will either deny or grant access. For multiple "RoleAllowed" attributes, access will be granted if any user authority matches or denied if no match is found.
---
 .../access/annotation/Jsr250Voter.java        | 10 +++--
 .../access/annotation/Jsr250VoterTests.java   | 44 +++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 core/src/test/java/org/springframework/security/access/annotation/Jsr250VoterTests.java

diff --git a/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java b/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java
index 697cffba65..7e21b481bb 100644
--- a/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java
+++ b/core/src/main/java/org/springframework/security/access/annotation/Jsr250Voter.java
@@ -37,6 +37,9 @@ public class Jsr250Voter implements AccessDecisionVoter {
 
     /**
      * Votes according to JSR 250.
+     * <p>
+     * If no JSR-250 attributes are found, it will abstain, otherwise it will grant or deny access
+     * based on the attributes that are found.
      *
      * @param authentication The authentication object.
      * @param object         The access object.
@@ -44,6 +47,8 @@ public class Jsr250Voter implements AccessDecisionVoter {
      * @return The vote.
      */
     public int vote(Authentication authentication, Object object, Collection<ConfigAttribute> definition) {
+        boolean jsr250AttributeFound = false;
+
         for (ConfigAttribute attribute : definition) {
             if (Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE.equals(attribute)) {
                 return ACCESS_GRANTED;
@@ -54,18 +59,17 @@ public class Jsr250Voter implements AccessDecisionVoter {
             }
 
             if (supports(attribute)) {
+                jsr250AttributeFound = true;
                 // Attempt to find a matching granted authority
                 for (GrantedAuthority authority : authentication.getAuthorities()) {
                     if (attribute.getAttribute().equals(authority.getAuthority())) {
                         return ACCESS_GRANTED;
                     }
                 }
-                // No match - deny access
-                return ACCESS_DENIED;
             }
         }
 
-        return ACCESS_ABSTAIN;
+        return jsr250AttributeFound ? ACCESS_DENIED : ACCESS_ABSTAIN;
     }
 }
 
diff --git a/core/src/test/java/org/springframework/security/access/annotation/Jsr250VoterTests.java b/core/src/test/java/org/springframework/security/access/annotation/Jsr250VoterTests.java
new file mode 100644
index 0000000000..96004cd040
--- /dev/null
+++ b/core/src/test/java/org/springframework/security/access/annotation/Jsr250VoterTests.java
@@ -0,0 +1,44 @@
+package org.springframework.security.access.annotation;
+
+import static org.junit.Assert.*;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Test;
+import org.springframework.security.access.AccessDecisionVoter;
+import org.springframework.security.access.ConfigAttribute;
+import org.springframework.security.access.SecurityConfig;
+import org.springframework.security.authentication.TestingAuthenticationToken;
+
+/**
+ *
+ * @author Luke Taylor
+ */
+public class Jsr250VoterTests {
+
+    // SEC-1443
+    @Test
+    public void supportsMultipleRolesCorrectly() throws Exception {
+        List<ConfigAttribute> attrs = new ArrayList<ConfigAttribute>();
+        Jsr250Voter voter = new Jsr250Voter();
+
+        attrs.add(new Jsr250SecurityConfig("A"));
+        attrs.add(new Jsr250SecurityConfig("B"));
+        attrs.add(new Jsr250SecurityConfig("C"));
+
+        assertEquals(AccessDecisionVoter.ACCESS_GRANTED,
+                voter.vote(new TestingAuthenticationToken("user", "pwd", "A"), new Object(), attrs));
+        assertEquals(AccessDecisionVoter.ACCESS_GRANTED,
+                voter.vote(new TestingAuthenticationToken("user", "pwd", "B"), new Object(), attrs));
+        assertEquals(AccessDecisionVoter.ACCESS_GRANTED,
+                voter.vote(new TestingAuthenticationToken("user", "pwd", "C"), new Object(), attrs));
+
+        assertEquals(AccessDecisionVoter.ACCESS_DENIED,
+                voter.vote(new TestingAuthenticationToken("user", "pwd", "NONE"), new Object(), attrs));
+
+        assertEquals(AccessDecisionVoter.ACCESS_ABSTAIN,
+                voter.vote(new TestingAuthenticationToken("user", "pwd", "A"), new Object(),
+                        SecurityConfig.createList("A","B","C")));
+    }
+}