Spring Security
  1. Spring Security
  2. SEC-1560

AccessControlListTag should use PermissionEvaluator

    Details

    • Type: Refactoring Refactoring
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.0, 3.0.1, 3.0.2, 3.0.3, 3.1.0.M1
    • Fix Version/s: 3.1.0.RC3
    • Component/s: Taglibs
    • Labels:
      None

      Description

      With the version 3.0, I think AccessControlListTag should use the new interface PermissionEvaluator to check the permission on an object.

      I join a new AccessControlListTag that gets the permissionEvaluator from spring context. It is based on the current code.

        Issue Links

          Activity

          Hide
          Thomas Champagne added a comment -

          Do you think it is possible to integrate this feature in version 3.1 ?

          Show
          Thomas Champagne added a comment - Do you think it is possible to integrate this feature in version 3.1 ?
          Hide
          Luke Taylor added a comment -

          Ok .

          I think it makes sense, since the code is effectively duplicated in PermissionEvaluator and it makes the tag implementation much simpler.

          Show
          Luke Taylor added a comment - Ok . I think it makes sense, since the code is effectively duplicated in PermissionEvaluator and it makes the tag implementation much simpler.
          Hide
          Thomas Champagne added a comment -

          Thank you Luke.

          If you don't want to change the AccessControlListTag, you can add the possiblity to use the hasPermission method in the AuthorizeTag. Because the final problem is to check the permission on an object in a JSP page.
          For example, it is not possible to call this with the 3.0 version : <sec:authorize access="hasPermission(#book, 'write')">
          where the book variable is provided from the page context.
          I found this approch in the forum : http://forum.springsource.org/showthread.php?104518-hasPermission-in-sec-authorize

          I saw the code of the version 3.1 and I think there is not a lot of changes to do this :
          In the DefaultWebSecurityExpressionHandler, override the createEvaluationContextInternal method and create a WebSecurityEvaluationContext.
          In this WebSecurityEvaluationContext, override the lookupVariable method and lookup variables in the page context.

          Tell me your opinion about last solution and if I have time today, I will create a patch.

          Show
          Thomas Champagne added a comment - Thank you Luke. If you don't want to change the AccessControlListTag, you can add the possiblity to use the hasPermission method in the AuthorizeTag. Because the final problem is to check the permission on an object in a JSP page. For example, it is not possible to call this with the 3.0 version : <sec:authorize access="hasPermission(#book, 'write')"> where the book variable is provided from the page context. I found this approch in the forum : http://forum.springsource.org/showthread.php?104518-hasPermission-in-sec-authorize I saw the code of the version 3.1 and I think there is not a lot of changes to do this : In the DefaultWebSecurityExpressionHandler, override the createEvaluationContextInternal method and create a WebSecurityEvaluationContext. In this WebSecurityEvaluationContext, override the lookupVariable method and lookup variables in the page context. Tell me your opinion about last solution and if I have time today, I will create a patch.
          Hide
          Luke Taylor added a comment -

          Yes, it occurred to me that the AccessControlTag is now essentially a subset of the EL functionality. I will have a look, but it will probably require some other changes since it isn't currently possible to use a custom SecurityExpressionHandler for web access.

          Show
          Luke Taylor added a comment - Yes, it occurred to me that the AccessControlTag is now essentially a subset of the EL functionality. I will have a look, but it will probably require some other changes since it isn't currently possible to use a custom SecurityExpressionHandler for web access.
          Hide
          Eric Rath added a comment -

          I'm trying to upgrade from 3.0.5 to 3.1.1, and ran into trouble with accesscontrollist's hasPermission attribute. It previously accepted integer bitmasks, or string values consisting of single permission names or a comma-delimited list of permission names. After upgrading, it now only accepts single permission names. Is this expected? It appears the change to AccessControlListTag.java after the change for this ticket removed the call that resolves the hasPermission attribute to a collection of Permission objects. I tried to view that ticket (#1798), but Jira returned a permission-denied response, so I'm posting a comment here.

          I examined the 3.1.1 code, and can't find any potential execution path that would allow the hasPermission attribute value to be parsed into a collection of Permission objects without providing my own PermissionEvaluator instance. The current combination of AccessControlListTag and AclPermissionEvaluator only interprets the value as single permission name.

          If I want to allow single permission names, comma-delimited lists of permission names, and integer bitmasks, is a custom implementation of PermissionEvaluator the appropriate and intended solution?

          Show
          Eric Rath added a comment - I'm trying to upgrade from 3.0.5 to 3.1.1, and ran into trouble with accesscontrollist's hasPermission attribute. It previously accepted integer bitmasks, or string values consisting of single permission names or a comma-delimited list of permission names. After upgrading, it now only accepts single permission names. Is this expected? It appears the change to AccessControlListTag.java after the change for this ticket removed the call that resolves the hasPermission attribute to a collection of Permission objects. I tried to view that ticket (#1798), but Jira returned a permission-denied response, so I'm posting a comment here. I examined the 3.1.1 code, and can't find any potential execution path that would allow the hasPermission attribute value to be parsed into a collection of Permission objects without providing my own PermissionEvaluator instance. The current combination of AccessControlListTag and AclPermissionEvaluator only interprets the value as single permission name. If I want to allow single permission names, comma-delimited lists of permission names, and integer bitmasks, is a custom implementation of PermissionEvaluator the appropriate and intended solution?
          Hide
          Rob Winch added a comment -

          I have logged SEC-2022 and SEC-2023. You can get around the issue by using the work posted on your forum post

          Show
          Rob Winch added a comment - I have logged SEC-2022 and SEC-2023 . You can get around the issue by using the work posted on your forum post

            People

            • Assignee:
              Luke Taylor
              Reporter:
              Thomas Champagne
            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: