Roles are evaluated in the wrong order

[TomHAnderson]

https://github.com/zendframework/zend-permissions-acl/blame/master/src/Acl.php#L903

The line above uses array_pop().
The manual reads

array_pop — Pop the element off the end of array

The function roleDFSVisitOnePrivilege loads the parent roles from the top down https://github.com/zendframework/zend-permissions-acl/blame/master/src/Acl.php#L956

        foreach ($this->getRoleRegistry()->getParents($role) as $roleParent) {
            $dfs['stack'][] = $roleParent;
        }

The result of roles being added to the stack from the most permissioned to the least then reading the roles from the least permissioned to the most causes the ACL to fail when a role hierarchy such as

Admin > User > Guest

where the permission allows a User.

[TomHAnderson]

I think this is a serious issue which has backwards breaking written all over it.
This line https://github.com/zendframework/zend-permissions-acl/blame/master/src/Acl.php#L905 loops until a true or false is returned. I think a valid way to correct this is to modify the loop to check all roles until a true is found and, if a false is found within them, return that if no true is found.

[TomHAnderson]

https://github.com/zendframework/zend-permissions-acl/pull/31/files

[TomHAnderson]

When roles are added as parents of a role their logical order in my mind should be most privileged to least. But when ordered this way the least privileged role is evaluated before any of the other parents.

[ezimuel]

@TomHAnderson can you provide a unit test to proof the issue? Thanks!

[TomHAnderson]

https://github.com/zendframework/zend-permissions-acl/pull/31/files

This pull request also contains a fix for the issue. If you do not include the fix the unit test will fail.

[TomHAnderson]

I've got a pending project which is waiting on a decision for this issue. The dark side easy way is to fix api-skeletons/zf-oauth2-doctrine-permissions-acl to load roles backwards and I'm not keen on the dark side. Please let me know how you'd like to proceed with this.

[TomHAnderson]

ping @ezimuel - any word on this?

[TomHAnderson]

Here is an alternative fix for the same problem: https://github.com/zendframework/zend-permissions-acl/pull/32

[TomHAnderson]

33 corrects the existing unit tests. Please look into these library updates asap, @ezimuel

[ezimuel]

@TomHAnderson I reviewed #33, waiting for your replies. Thanks!

[ezimuel]

Close because it's not an issue, see my comment here.