authz:authorize ifNotGranted not behaving as expected

[rawdave]

Hi I'm using Acegi Security on a web app using Spring with a STRUTS UI. I think my filter configuration is OK as using the <authz:authorize> tag with the ifAllGranted attribute works fine.

However, when I use the ifNotGranted attribute I don't get the behaviour I expected. For example:

<!-- Only directors/administrators can change who placed a property once it has been created -->
<authz&#58;authorize ifAllGranted="ROLE_ADMINISTRATOR">
&#91;i&#93;STUFF ONLY FOR ADMINISTRATORS&#91;/i&#93;
</authz&#58;authorize>
				
<!-- everyone else only gets to see who placed the property -->
<authz&#58;authorize ifNotGranted="ROLE_ADMINISTRATOR">
&#91;i&#93;STUFF FOR ORDINARY PEOPLE&#91;/i&#93;
</authz&#58;authorize>

What I would expect to happen is that administrators see the adminstrator only content and everyone else sees the ordinary content. What actually happens is that ordinary people see only the content contained by <authz:authorize ifNotGranted...
but administrators see the content contained by both <authz> tags.

According to the docs anyone with the ROLE_ADMINISTRATOR should not see the contents of the second tag.

Are there any common mistakes that exhibit these symptoms?

thanks

Dave

[Ben Alex]

Would you mind re-checking your configuration? AFAICS this situation is correctly tested for in AuthorizeTagTests:

    public void testSkipsBodyWhenNotGrantedUnsatisfied()
        throws JspException {
        authorizeTag.setIfNotGranted("ROLE_TELLER");
        assertEquals("prevents request - principal has ROLE_TELLER",
            Tag.SKIP_BODY, authorizeTag.doStartTag());
    }

[rawdave]

Thanks for the reply.
OK, I'll double check the configuration.
Is there anything in particular that you think I should check?
thanks
Dave

[Ben Alex]

I'd output the Authentication on the ContextHolder before your call to the taglib. See if the actual Authentication contains the role you're testing for, pointing you to either look more closely at the taglib or look more closely at your ContextHolder/Authentication management.

[Rexxe]

Ben,

I see the problem with this tag library, as I am running into the same issue. In your AuthorizeTag class you call either retainAll or containsAll from the Collection interface. As a result, the hashCode and equals of the user's GrantedAuthority object must be implemented AND equal the hashCode you implemented in your GrantedAuthorityImpl class (which is used for comparison in the tag's code). Let's say I have a Role object which implements GrantedAuthority but its hashCode (and equals) requires the use of multiple fields which would not be used by the hashCode() method of the GAimpl (or in my case I use Jakarta Lang's HashCodeBuilder and pass it 2 prime numbers to use for generation). Also, the equals() method must use only the GA.getAuthority() method to check for equality otherwise you cannot compare the GAImpl and user's GA class. Consequently, the way it is now, the retainAll method will never retain any objects since the hashCode of the user's GA will never equal the hashCode of the GAImpl class unless the hashCodes are equal and the user's GA's equals() is written correctly.

A temporary solution to use the tag right now is for the user's GA to just implement the same hashCode that is used in the GAImpl class which is simply the String representation hashCode of the role (e.g. "ROLE_ADMIN".hashCode()). Also, the equals method in the user's GA must look something like:

if (!(obj instanceof GrantedAuthority))
            return false;

        GrantedAuthority role = (GrantedAuthority) obj;
        return new EqualsBuilder().append(role.getAuthority(), this.getAuthority())
                .isEquals();

Perhaps a more permanent solution should be to simply loop through the collections and manually test, calling getAuthority() on both your GAImpl class and the user's GA class and checking for equality. This would allow custom hashCodes and equals on the user's GA object.

--Rexxe

[rawdave]

Hi Rexxe
Thanks for the code - implementing equals() and hashCode() as you suggested fixed the problem I was having too.
cheers
Dave

PS Ben, sorry for not following up your message earlier, "he who pays my salary" gave me a bunch of other stuff to fix first.

[fbos]

Rexxe, I'm the maintainer for the authz taglib. I need some help to reproduce the problem.

I currently have the following test case, which passes:

    public void testAllowsAccessToCustomGrantedAuthorityImplementation()
            throws Exception {
        currentUser = new TestingAuthenticationToken("abc", "123",
                new GrantedAuthority[] {
                    new CustomGrantedAuthority("ROLE_ADMIN")});

        authorizeTag.setIfAllGranted("ROLE_ADMIN");
        assertEquals("allows request - principal has ROLE_ADMIN",
            Tag.EVAL_BODY_INCLUDE, authorizeTag.doStartTag());
    }

    public static final class CustomGrantedAuthority
        extends GrantedAuthorityImpl {
        public CustomGrantedAuthority(String role) {
            super(role);
        }

        public boolean equals(Object other) {
            return getAuthority().equals(((GrantedAuthority)other).getAuthority());
        }
        
        public int hashCode() {
            return 1 + super.hashCode();
        }
    }

Notice hashCode() is returning a different value than GrantedAuthorityImpl. equals() is a bit different. I'm having a hard time reproducing your error, following your description. Could you post your equals() and hashCode() methods ? That would help me determine how to test the taglib.

Thanks !
François

[rawdave]

Hi Francois
These are the equals and hashCode methods that I implemented following Rexxe's instructions. Perhaps that will help. I can tell you that adding these methods fixed the problem I was having.

    public boolean equals(Object obj) {
    	if (!(obj instanceof GrantedAuthority))
            return false;

        GrantedAuthority role = (GrantedAuthority) obj;
        return new EqualsBuilder().append(role.getAuthority(), this.getAuthority())
                .isEquals();
    }
    
    public int hashCode() {
    	return getAuthority().hashCode();
    }

[fbos]

Thank you, rawdave, but I want the original code - before you implemented the new methods.

Were you extending GrantedAuthorityImpl, or were implementing GrantedAuthority ?

Thanks,
François

[rawdave]

Sorry, I misunderstood. we (Bryan Hunt in this case) implement GrantedAuthority. I've posted the complete class since it is not large.

The original version was exactly the same as below but without the equals() and hashCode() methods.

--Dave

package ie.jestate.bo.user;

import java.io.Serializable;

import org.apache.commons.lang.builder.EqualsBuilder;

import net.sf.acegisecurity.GrantedAuthority;

/**
 * @author bryan hunt
 * @hibernate.class table = "jestate_authorities"
 */
public class Authority implements GrantedAuthority, Serializable {

    protected String authority;

    protected Long id;

    public String toString() {
        return authority;
    }

    public Authority() {
        super();
    }

    public Authority(String authority) {
        super();
        this.authority = authority;
    }

    /**
     * @see net.sf.acegisecurity.GrantedAuthority#getAuthority()
     * @hibernate.property column = "TEXT" not-null = "true"
     */
    public String getAuthority() {
        return authority;
    }

    /**
     * @hibernate.id generator-class = "native"
     */
    public Long getId() {
        return id;
    }

    public void setAuthority(String authority) {
        this.authority = authority;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public boolean equals(Object obj) {
    	if (!(obj instanceof GrantedAuthority))
            return false;

        GrantedAuthority role = (GrantedAuthority) obj;
        return new EqualsBuilder().append(role.getAuthority(), this.getAuthority())
                .isEquals();
    }
    
    public int hashCode() {
    	return getAuthority().hashCode();
    }
}