Possible bug?

[steel]

File: ACLEntryVoter.java
Function: public int vote(Authentication, Object, Config){}

Issue:

return authorityService.getAuthorities().contains(cad.authority) ? AccessDecisionVoter.ACCESS_GRANTED
                            : AccessDecisionVoter.ACCESS_DENIED;‍‍

Multiple acl parameters of type ACL_METHOD are not all evaluated. The first first one is evaluated and the entire function fails if the first comparison fails.
e.g. personService.createPerson=ACL_METHOD.Permission1, ACL_METHOD.ROLE_ADMINISTRATOR.

Code block:

for (ConfigAttributeDefintion cad : supportedDefinitions)
        {
            NodeRef testNodeRef = null;

            if (cad.typeString.equals(ACL_ALLOW))
            {
                return AccessDecisionVoter.ACCESS_GRANTED;
            }
            else if (cad.typeString.equals(ACL_METHOD))
            {
                if (authenticationService.getCurrentUserName().equals(cad.authority))
                {
                    return AccessDecisionVoter.ACCESS_GRANTED;
                }
                else
                {
                    return authorityService.getAuthorities().contains(cad.authority) ? AccessDecisionVoter.ACCESS_GRANTED
                            : AccessDecisionVoter.ACCESS_DENIED;
                }
            }
            else if (cad.parameter >= invocation.getArguments().length)
            {
                continue;
            }
            else if (cad.typeString.equals(ACL_NODE))
            {
                if (StoreRef.class.isAssignableFrom(params[cad.parameter]))
                {
                    if (invocation.getArguments()[cad.parameter] != null)
                    {
                        if (log.isDebugEnabled())
                        {
                            log.debug("\tPermission test against the store - using permissions on the root node");
                        }
                        StoreRef storeRef = (StoreRef) invocation.getArguments()[cad.parameter];
                        if (nodeService.exists(storeRef))
                        {
                            testNodeRef = nodeService.getRootNode(storeRef);
                        }
                    }
                }
                else if (NodeRef.class.isAssignableFrom(params[cad.parameter]))
                {
                    testNodeRef = (NodeRef) invocation.getArguments()[cad.parameter];
                    if (log.isDebugEnabled())
                    {
                        log.debug("\tPermission test on node " + nodeService.getPath(testNodeRef));
                    }
                }
                else if (ChildAssociationRef.class.isAssignableFrom(params[cad.parameter]))
                {
                    if (invocation.getArguments()[cad.parameter] != null)
                    {
                        testNodeRef = ((ChildAssociationRef) invocation.getArguments()[cad.parameter]).getChildRef();
                        if (log.isDebugEnabled())
                        {
                            log.debug("\tPermission test on node " + nodeService.getPath(testNodeRef));
                        }
                    }
                }
                else
                {
                    throw new ACLEntryVoterException("The specified parameter is not a NodeRef or ChildAssociationRef");
                }
            }
            else if (cad.typeString.equals(ACL_PARENT))
            {
                // There is no point having parent permissions for store
                // refs
                if (NodeRef.class.isAssignableFrom(params[cad.parameter]))
                {
                    NodeRef child = (NodeRef) invocation.getArguments()[cad.parameter];
                    if (child != null)
                    {
                        testNodeRef = nodeService.getPrimaryParent(child).getParentRef();
                        if (log.isDebugEnabled())
                        {
                            log.debug("\tPermission test for parent on node " + nodeService.getPath(testNodeRef));
                        }
                    }
                }
                else if (ChildAssociationRef.class.isAssignableFrom(params[cad.parameter]))
                {
                    if (invocation.getArguments()[cad.parameter] != null)
                    {
                        testNodeRef = ((ChildAssociationRef) invocation.getArguments()[cad.parameter]).getParentRef();
                        if (log.isDebugEnabled())
                        {
                            log.debug("\tPermission test for parent on child assoc ref for node "
                                    + nodeService.getPath(testNodeRef));
                        }
                    }

                }
                else
                {
                    throw new ACLEntryVoterException("The specified parameter is not a ChildAssociationRef");
                }
            }‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

Possible fix:
Add a pass boolean toggle outside of for loop to store any access_granted value and returns at the end after all have been evaluated.

Please let me know if I am incorrect here.

thanks!

[andy]

Hi

You are correct. Only one ACL_METHOD.<AUTHORITY> entry is expected.
You could avoid the issue by creating a group of users allowed to execute the method. "GROUP_User Managers" and include admins as things stand!

The other forms ACL_NODE for example are all "anded" together.

I will raise this as an issue.

However, the control is against the user having an *authority* membership (username, group) NOT a permission or group of permissions. The ROLE_ADMINISTRATOR is an authority used for global administrators.

The code could also check if a user has a global permission. It does not.
I will add this as a request. Apologies, for some reason I thought that it did.

http://www.alfresco.org/jira/browse/AR-473
http://www.alfresco.org/jira/browse/AR-474

Regards

Andy

[steel]

Great.
I do understand that the ACL is for authority membership, I just typed it incorrectly.

[andy]

Hi

More than one ACL_METHOD entry is now supported.

One or more authorities specified by the ACL_METHOD entries must be present for the user trying to invoke the method.

Global permissions are not supported as part of ACL_METHOD and on reflection this does not make much sense.

Regards

Andy

[steel]

Hi

More than one ACL_METHOD entry is now supported.

One or more authorities specified by the ACL_METHOD entries must be present for the user trying to invoke the method.

Global permissions are not supported as part of ACL_METHOD and on reflection this does not make much sense.

Regards

Andy

Excellent! Thank You.