Announcement Announcement Module
No announcement yet.
New ACL module issues / questions Page Title Module
Move Remove Collapse
This topic is closed
Conversation Detail Module
  • Filter
  • Time
  • Show
Clear All
new posts

  • New ACL module issues / questions

    Hi, Ive been trying out the new ACL module and have found some issues with it in addition to the ones reported in other threads (the sql does not work on oracle and you have to have an owner defined to get the sql to work even though the documentation states this is optional)

    So after fixing these issues(using a hacked version of BasicLookupStrategy and dummy owner sid) I have been able to control access to objects (voters and filters) with the new acls, however I have run into a few more issues that Im not sure if they are bugs or the module has been designed like that for some reason:

    1) In the ACLImpl.isGranted() method the permission check is done via an exact match on the masks ie

    if ((ace.getPermission().getMask() == permission[i].getMask()) && .....

    surely this should just check the required permission is set in the ace ie something like

    (ace.getPermission().getMask() & permission[i].getMask()) == permission[i].getMask())

    this is what the old acl pacakge did and seems the correct approach ie i want one ACE to specifiy the combination of permissions for a sid , and this should grant a permission if that is set in the ACE mask.

    Currently I have to define a separate ACE for each base permission which seems to defeat the purpose of using a bit mask.

    2) Again in the ACLImpl.isGranted() the logic is stated as being:

    'If the ACE specifies to grant access, the method will return
    true. If the ACE specifies to deny access, the loop will stop and the next permission iteration will be performed'

    This implies that any granting ACE will allow the operation, I would have thought the denys would take precedence? Ie you use denys as a specific block, so if there are any denys the action is not allowed. I think this is more what we need in our application, and obviously we can write our own ACL implementation with different logic, but I'm trying to understand the reasons for this design? Is this based on standard ACL grant / deny practices / what is behind doing it like this?

    Thanks for any help on this


  • #2
    I've just started working with the new ACL package, and am wondering if you ever figured out the answers to your questions? I'm half tempted to write my own "isGranted" implementation, but before I do, I want to know what the reasoning is behind the current implementation.


    • #3
      No, haven't got any answers yet - as it stands we would have to write our own implementation of this as well.

      I've raised an issue on the JIRA about this and a few other things (SEC-479).


      • #4
        I've done some further work on this and realise that in the current implementation any grant will allow access, and any deny will deny that permission (eg write) but then check any other permissions (eg adminsistrate - which may be allowed. This way of checking the permissions is not a big issue (we may only use one anyway).

        The main issue is that for each permission the current implementation just uses the first match it finds. I cant see how this would work in most cases.

        We need to implement an order of precedence in an acl, ie permissions override each other in this order

        user(principal) denies
        user(principal) grants
        group(granted authority) denies
        group(granted authority) grants
        then check parent.


        1) A user is two groups - one with read access granted and another with read access denied. Read access should be denied.

        2) A user is in a group that has read access granted, however that user has read access denied. Read access should be denied.

        3) A user is in a group that has read access denied, however that user has read access granted. Read access should be granted.

        These seem like sensible rules (comments if not!) but there seems no way to implement them without writing our own AclImpl that checks all sids (so sid order is less important?) as the the last sid could deny something that was granted. Additionally the aclimpl needs to be able to apply the principalsid has more weight than grantedauthoritysid logic.

        I think the supplied AclImpl either needs to be able to support this sort of logic, or be more extensible, currently we are having to copy the whole class and alter it instead of just overriding methods. eg could the ace list be accessible in subclasses?


        • #5
          Use Old ACL Package?

          This thread brings up a question that I really should answer right away:

          I am beginning work today on a user story that requires the ability for some domain objects to be publicly readable and for other to be only readable by their owner.

          Should I use the new or old ACL package?