Announcement Announcement Module
Collapse
No announcement yet.
authz:authorize ifNotGranted not behaving as expected Page Title Module
Move Remove Collapse
This topic is closed
X
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • authz:authorize ifNotGranted not behaving as expected

    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:

    Code:
    <!-- 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

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

    Code:
        public void testSkipsBodyWhenNotGrantedUnsatisfied&#40;&#41;
            throws JspException &#123;
            authorizeTag.setIfNotGranted&#40;"ROLE_TELLER"&#41;;
            assertEquals&#40;"prevents request - principal has ROLE_TELLER",
                Tag.SKIP_BODY, authorizeTag.doStartTag&#40;&#41;&#41;;
        &#125;

    Comment


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

      Comment


      • #4
        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.

        Comment


        • #5
          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:

          Code:
          if &#40;!&#40;obj instanceof GrantedAuthority&#41;&#41;
                      return false;
          
                  GrantedAuthority role = &#40;GrantedAuthority&#41; obj;
                  return new EqualsBuilder&#40;&#41;.append&#40;role.getAuthority&#40;&#41;, this.getAuthority&#40;&#41;&#41;
                          .isEquals&#40;&#41;;
          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

          Comment


          • #6
            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.

            Comment


            • #7
              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:
              Code:
                  public void testAllowsAccessToCustomGrantedAuthorityImplementation&#40;&#41;
                          throws Exception &#123;
                      currentUser = new TestingAuthenticationToken&#40;"abc", "123",
                              new GrantedAuthority&#91;&#93; &#123;
                                  new CustomGrantedAuthority&#40;"ROLE_ADMIN"&#41;&#125;&#41;;
              
                      authorizeTag.setIfAllGranted&#40;"ROLE_ADMIN"&#41;;
                      assertEquals&#40;"allows request - principal has ROLE_ADMIN",
                          Tag.EVAL_BODY_INCLUDE, authorizeTag.doStartTag&#40;&#41;&#41;;
                  &#125;
              
                  public static final class CustomGrantedAuthority
                      extends GrantedAuthorityImpl &#123;
                      public CustomGrantedAuthority&#40;String role&#41; &#123;
                          super&#40;role&#41;;
                      &#125;
              
                      public boolean equals&#40;Object other&#41; &#123;
                          return getAuthority&#40;&#41;.equals&#40;&#40;&#40;GrantedAuthority&#41;other&#41;.getAuthority&#40;&#41;&#41;;
                      &#125;
                      
                      public int hashCode&#40;&#41; &#123;
                          return 1 + super.hashCode&#40;&#41;;
                      &#125;
                  &#125;

              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

              Comment


              • #8
                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.

                Code:
                    public boolean equals&#40;Object obj&#41; &#123;
                    	if &#40;!&#40;obj instanceof GrantedAuthority&#41;&#41;
                            return false;
                
                        GrantedAuthority role = &#40;GrantedAuthority&#41; obj;
                        return new EqualsBuilder&#40;&#41;.append&#40;role.getAuthority&#40;&#41;, this.getAuthority&#40;&#41;&#41;
                                .isEquals&#40;&#41;;
                    &#125;
                    
                    public int hashCode&#40;&#41; &#123;
                    	return getAuthority&#40;&#41;.hashCode&#40;&#41;;
                    &#125;

                Comment


                • #9
                  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

                  Comment


                  • #10
                    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

                    Code:
                    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 &#123;
                    
                        protected String authority;
                    
                        protected Long id;
                    
                        public String toString&#40;&#41; &#123;
                            return authority;
                        &#125;
                    
                        public Authority&#40;&#41; &#123;
                            super&#40;&#41;;
                        &#125;
                    
                        public Authority&#40;String authority&#41; &#123;
                            super&#40;&#41;;
                            this.authority = authority;
                        &#125;
                    
                        /**
                         * @see net.sf.acegisecurity.GrantedAuthority#getAuthority&#40;&#41;
                         * @hibernate.property column = "TEXT" not-null = "true"
                         */
                        public String getAuthority&#40;&#41; &#123;
                            return authority;
                        &#125;
                    
                        /**
                         * @hibernate.id generator-class = "native"
                         */
                        public Long getId&#40;&#41; &#123;
                            return id;
                        &#125;
                    
                        public void setAuthority&#40;String authority&#41; &#123;
                            this.authority = authority;
                        &#125;
                    
                        public void setId&#40;Long id&#41; &#123;
                            this.id = id;
                        &#125;
                    
                        public boolean equals&#40;Object obj&#41; &#123;
                        	if &#40;!&#40;obj instanceof GrantedAuthority&#41;&#41;
                                return false;
                    
                            GrantedAuthority role = &#40;GrantedAuthority&#41; obj;
                            return new EqualsBuilder&#40;&#41;.append&#40;role.getAuthority&#40;&#41;, this.getAuthority&#40;&#41;&#41;
                                    .isEquals&#40;&#41;;
                        &#125;
                        
                        public int hashCode&#40;&#41; &#123;
                        	return getAuthority&#40;&#41;.hashCode&#40;&#41;;
                        &#125;
                    &#125;

                    Comment


                    • #11
                      François,

                      Your test is incorrect. Even your CustomGrantedAuthority shows the problem. Your equals() method only checks on the authority property. I have a custom GrantedAuthority object which not only looks at the authority property for equality but another property as well. As a result, my hashCode() method was different as well because it took into account this other property.

                      In the tag's code you use methods of the Set interface: retainAll and containsAll. Both of those methods are going to use the equals and hashCode methods of my custom GA which is different than the equals and hashCode of the GAImpl class.

                      For example:

                      Code:
                      public boolean equals&#40;Object obj&#41; &#123;
                              if &#40;!&#40;obj instanceof CustomGrantedAuthority&#41;&#41;  
                                  return false; <-- this will be hit when comparing the GAImpl to the Custom one here &#40;you make an assumption that the only thing that should be used for comparison is whatever is in the GA interface.&#41; 
                      
                              CustomGrantedAuthority role = &#40;CustomGrantedAuthority&#41; obj;
                              return new EqualsBuilder&#40;&#41;.append&#40;role.getAuthority&#40;&#41;, this.getAuthority&#40;&#41;&#41;.append&#40;role.getCode&#40;&#41;, this.getCode&#40;&#41;&#41;.isEquals&#40;&#41;;
                          &#125;
                      
                          /**
                           * @return
                           */
                          public int hashCode&#40;&#41; &#123;
                              return new HashCodeBuilder&#40;13,23&#41;.append&#40;this.authority&#41;.append&#40;this.code&#41;.toHashCode&#40;&#41;
                          &#125;
                      So, essentially what it boils down to is that by using the Set methods you are forcing the user to check only the authority property when creating the equals and hashCode methods. The fix is to get rid of the use of retainAll and containsAll and use a loop instead and pulling out and comparing the authority properties directly (or you can use CollectionUtils in Jakarta-Collections (though I don't remember if you had that dependency already or not)).

                      Here is one of the problem areas in the code (lines 150-56):

                      Code:
                      private Set retainAll&#40;final Collection granted,
                                                final Set requiredAuthorities&#41; &#123;
                              Set grantedCopy = new HashSet&#40;granted&#41;; <-- granted is a collection of CustomGA
                              grantedCopy.retainAll&#40;requiredAuthorities&#41;; <-- requiredAuthorities is  a set of GAImpl, which you build on lines 137-148&#41;.  This is where the disconnect is and as a result, nothing will be retained because the requiredAuthority never equals a CustomGA
                      
                              return grantedCopy;
                          &#125;
                      And the other problem area (lines 96-101):

                      Code:
                      if &#40;&#40;null != evaledIfAllGranted&#41; && !"".equals&#40;evaledIfAllGranted&#41;&#41; &#123;
                                  if &#40;!granted.containsAll&#40; <-- granted is a collection of CustomGA, but you are asking if it contains all of the GAImpls.
                                          parseAuthoritiesString&#40;evaledIfAllGranted&#41;&#41;&#41; &#123;
                                      return Tag.SKIP_BODY;
                                  &#125;
                              &#125;
                      Hope this helps,

                      Rexxe

                      Comment

                      Working...
                      X