Corrected Authz parsing of whitespace in GrantedAuthoritys. Contributed by Francois Beausoleil.

core/src/main/java/org/acegisecurity/taglibs/authz/AuthorizeTag.java

@@ -16,10 +16,13 @@
 package net.sf.acegisecurity.taglibs.authz;
 
 import net.sf.acegisecurity.Authentication;
+import net.sf.acegisecurity.GrantedAuthority;
 import net.sf.acegisecurity.GrantedAuthorityImpl;
 import net.sf.acegisecurity.context.ContextHolder;
 import net.sf.acegisecurity.context.security.SecureContext;
 
+import org.springframework.util.StringUtils;
+
 import org.springframework.web.util.ExpressionEvaluationUtils;
 
 import java.util.*;
@@ -132,24 +135,104 @@ public class AuthorizeTag extends TagSupport {
         return granted;
     }
 
+    private Set authoritiesToRoles(Collection c) {
+        Set target = new HashSet();
+
+        for (Iterator iterator = c.iterator(); iterator.hasNext();) {
+            GrantedAuthority authority = (GrantedAuthority) iterator.next();
+
+            if (null == authority.getAuthority()) {
+                throw new IllegalArgumentException(
+                    "Cannot process GrantedAuthority objects which return null from getAuthority() - attempting to process "
+                    + authority.toString());
+            }
+
+            target.add(authority.getAuthority());
+        }
+
+        return target;
+    }
+
     private Set parseAuthoritiesString(String authorizationsString) {
         final Set requiredAuthorities = new HashSet();
-        final StringTokenizer tokenizer;
-        tokenizer = new StringTokenizer(authorizationsString, ",", false);
+        final String[] authorities = StringUtils
+            .commaDelimitedListToStringArray(authorizationsString);
 
-        while (tokenizer.hasMoreTokens()) {
-            String role = tokenizer.nextToken();
-            requiredAuthorities.add(new GrantedAuthorityImpl(role.trim()));
+        for (int i = 0; i < authorities.length; i++) {
+            String authority = authorities[i];
+            String role = authority.replaceAll("\\s+", "");
+            requiredAuthorities.add(new GrantedAuthorityImpl(role));
         }
 
         return requiredAuthorities;
     }
 
-    private Set retainAll(final Collection granted,
-        final Set requiredAuthorities) {
-        Set grantedCopy = new HashSet(granted);
-        grantedCopy.retainAll(requiredAuthorities);
+    /**
+     * Find the common authorities between the current authentication's {@link
+     * GrantedAuthority} and the ones that have been specified in the tag's
+     * ifAny, ifNot or ifAllGranted attributes.
+     * 
+     * <p>
+     * We need to manually iterate over both collections, because the granted
+     * authorities might not implement {@link Object#equals(Object)} and
+     * {@link Object#hashCode()} in the same way as {@link
+     * GrantedAuthorityImpl}, thereby invalidating {@link
+     * Collection#retainAll(java.util.Collection)} results.
+     * </p>
+     * 
+     * <p>
+     * <strong>CAVEAT</strong>:  This method <strong>will not</strong> work if
+     * the granted authorities returns a <code>null</code> string as the
+     * return value of {@link
+     * net.sf.acegisecurity.GrantedAuthority#getAuthority()}.
+     * </p>
+     * 
+     * <p>
+     * Reported by rawdave, on Fri Feb 04, 2005 2:11 pm in the Acegi Security
+     * System for Spring forums.
+     * </p>
+     *
+     * @param granted The authorities granted by the authentication. May be any
+     *        implementation of {@link GrantedAuthority} that does
+     *        <strong>not</strong> return <code>null</code> from {@link
+     *        net.sf.acegisecurity.GrantedAuthority#getAuthority()}.
+     * @param required A {@link Set} of {@link GrantedAuthorityImpl}s that have
+     *        been built using ifAny, ifAll or ifNotGranted.
+     *
+     * @return A set containing only the common authorities between
+     *         <var>granted</var> and <var>required</var>.
+     *
+     * @see <a
+     *      href="http://forum.springframework.org/viewtopic.php?t=3367">authz:authorize
+     *      ifNotGranted not behaving as expected</a>
+     */
+    private Set retainAll(final Collection granted, final Set required) {
+        Set grantedRoles = authoritiesToRoles(granted);
+        Set requiredRoles = authoritiesToRoles(required);
+        grantedRoles.retainAll(requiredRoles);
+
+        return rolesToAuthorities(grantedRoles, granted);
+    }
+
+    private Set rolesToAuthorities(Set grantedRoles, Collection granted) {
+        Set target = new HashSet();
+
+        for (Iterator iterator = grantedRoles.iterator(); iterator.hasNext();) {
+            String role = (String) iterator.next();
+
+            for (Iterator grantedIterator = granted.iterator();
+                grantedIterator.hasNext();) {
+                GrantedAuthority authority = (GrantedAuthority) grantedIterator
+                    .next();
+
+                if (authority.getAuthority().equals(role)) {
+                    target.add(authority);
+
+                    break;
+                }
+            }
+        }
 
-        return grantedCopy;
+        return target;
     }
 }

core/src/test/java/org/acegisecurity/taglibs/authz/AuthorizeTagCustomGrantedAuthorityTests.java

@@ -0,0 +1,95 @@
+/* Copyright 2004, 2005 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 net.sf.acegisecurity.taglibs.authz;
+
+import junit.framework.TestCase;
+
+import net.sf.acegisecurity.GrantedAuthority;
+import net.sf.acegisecurity.context.ContextHolder;
+import net.sf.acegisecurity.context.security.SecureContextImpl;
+import net.sf.acegisecurity.providers.TestingAuthenticationToken;
+
+import javax.servlet.jsp.JspException;
+import javax.servlet.jsp.tagext.Tag;
+
+
+/**
+ * DOCUMENT ME!
+ *
+ * @author Francois Beausoleil
+ * @version $Id$
+ */
+public class AuthorizeTagCustomGrantedAuthorityTests extends TestCase {
+    //~ Instance fields ========================================================
+
+    private final AuthorizeTag authorizeTag = new AuthorizeTag();
+    private SecureContextImpl context;
+    private TestingAuthenticationToken currentUser;
+
+    //~ Methods ================================================================
+
+    public void testAllowsRequestWhenCustomAuthorityPresentsCorrectRole()
+        throws JspException {
+        authorizeTag.setIfAnyGranted("ROLE_TELLER");
+        assertEquals("authorized - ROLE_TELLER in both sets",
+            Tag.EVAL_BODY_INCLUDE, authorizeTag.doStartTag());
+    }
+
+    public void testRejectsRequestWhenCustomAuthorityReturnsNull()
+        throws JspException {
+        authorizeTag.setIfAnyGranted("ROLE_TELLER");
+        context.setAuthentication(new TestingAuthenticationToken("abc", "123",
+                new GrantedAuthority[] {new CustomGrantedAuthority(null)}));
+
+        try {
+            authorizeTag.doStartTag();
+            fail("Failed to reject GrantedAuthority with NULL getAuthority()");
+        } catch (IllegalArgumentException expected) {
+            assertTrue("expected", true);
+        }
+    }
+
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        currentUser = new TestingAuthenticationToken("abc", "123",
+                new GrantedAuthority[] {new CustomGrantedAuthority(
+                        "ROLE_TELLER")});
+
+        context = new SecureContextImpl();
+        context.setAuthentication(currentUser);
+
+        ContextHolder.setContext(context);
+    }
+
+    protected void tearDown() throws Exception {
+        ContextHolder.setContext(null);
+    }
+
+    //~ Inner Classes ==========================================================
+
+    private static class CustomGrantedAuthority implements GrantedAuthority {
+        private final String authority;
+
+        public CustomGrantedAuthority(String authority) {
+            this.authority = authority;
+        }
+
+        public String getAuthority() {
+            return authority;
+        }
+    }
+}

doc/xdocs/changes.xml

@@ -28,6 +28,7 @@
     <release version="0.8.1" date="In CVS">
       <action dev="benalex" type="fix">SecurityEnforcementFilter caused NullPointerException when anonymous authentication used with BasicProcessingFilterEntryPoint</action>
       <action dev="benalex" type="fix">FilterChainProxy now supports replacement of ServletRequest and ServetResponse by Filter beans</action>
+      <action dev="benalex" type="fix">Corrected Authz parsing of whitespace in GrantedAuthoritys</action>
       <action dev="benalex" type="update">ContextHolderAwareRequestWrapper methods returns null if user is anonymous</action>
       <action dev="benalex" type="update">AbstractBasicAclEntry improved compatibility with Hibernate</action>
     </release>