diff --git a/core/src/main/java/org/acegisecurity/taglibs/authz/AuthorizeTag.java b/core/src/main/java/org/acegisecurity/taglibs/authz/AuthorizeTag.java index 05911c1f20..b749895888 100644 --- a/core/src/main/java/org/acegisecurity/taglibs/authz/AuthorizeTag.java +++ b/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. + * + *
+ * 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. + *
+ * + *
+ * CAVEAT: This method will not work if
+ * the granted authorities returns a null string as the
+ * return value of {@link
+ * net.sf.acegisecurity.GrantedAuthority#getAuthority()}.
+ *
+ * Reported by rawdave, on Fri Feb 04, 2005 2:11 pm in the Acegi Security + * System for Spring forums. + *
+ * + * @param granted The authorities granted by the authentication. May be any + * implementation of {@link GrantedAuthority} that does + * not returnnull 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
+ * granted and required.
+ *
+ * @see authz:authorize
+ * ifNotGranted not behaving as expected
+ */
+ private Set retainAll(final Collection granted, final Set required) {
+ Set grantedRoles = authoritiesToRoles(granted);
+ Set requiredRoles = authoritiesToRoles(required);
+ grantedRoles.retainAll(requiredRoles);
- return grantedCopy;
+ 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 target;
}
}
diff --git a/core/src/test/java/org/acegisecurity/taglibs/authz/AuthorizeTagCustomGrantedAuthorityTests.java b/core/src/test/java/org/acegisecurity/taglibs/authz/AuthorizeTagCustomGrantedAuthorityTests.java
new file mode 100644
index 0000000000..40c77f1dd2
--- /dev/null
+++ b/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;
+ }
+ }
+}
diff --git a/doc/xdocs/changes.xml b/doc/xdocs/changes.xml
index 6563626af0..8cc04352d9 100644
--- a/doc/xdocs/changes.xml
+++ b/doc/xdocs/changes.xml
@@ -28,6 +28,7 @@