Zend Framework

Zend_Acl::isAllowed does not support Role/Resource Inheritance down to Assertions

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.0.0
  • Fix Version/s: 1.9.1
  • Component/s: Zend_Acl
  • Labels:
    None

Description

Zend_Acl::isAllowed currently transforms any is_a Zend_Acl_Role_Interface and Zend_Acl_Resource_Interface based objects to that of Zend_Acl_Role, and Zend_Acl_Resource.

This diminishes any benefit a developer might see in using assertions in a dynamic manner (ie, accessing properties of an object that has implemented a Role or Resource Interface) inside the assertion.

my fix is supplied below:

public function isAllowed($role = null, $resource = null, $privilege = null)
    {
        // make sure $role is a valid role
        if (null !== $role) {
            $this->_getRoleRegistry()->get($role); // throws exception if not found
        }

        if (null !== $resource) {
            $this->get($resource); // throws exception if not found
        }

Issue Links

This issue is related to:
ZF-1722 Zend_Acl assertions broken when inheritance is required (ie DepthFirstSearch) Major Resolved
This issue is related to:
ZF-4460 Resource objects passed in to ACL query methods are not passed through to registered assert()'s Major Resolved

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ralph Schindler added a comment -

Scratch that, this is a safer approach:

public function isAllowed($role = null, $resource = null, $privilege = null)
    {
        // make sure $role is a valid role
        if (null !== $role) {
            $registryRole = $this->_getRoleRegistry()->get($role); // throws exception if not found

            if (!$role instanceof Zend_Acl_Role_Interface) {
                $role = $registryRole;
            }
        }
        
        if (null !== $resource) {
            $registryResource = $this->get($resource); // throws exception if not found
            if (!$resource instanceof Zend_Acl_Resource_Interface) {
                $resource = $registryResource;
            }
        }
Show
Ralph Schindler added a comment - Scratch that, this is a safer approach: 
public function isAllowed($role = null, $resource = null, $privilege = null)
    {
        // make sure $role is a valid role
        if (null !== $role) {
            $registryRole = $this->_getRoleRegistry()->get($role); // throws exception if not found

            if (!$role instanceof Zend_Acl_Role_Interface) {
                $role = $registryRole;
            }
        }
        
        if (null !== $resource) {
            $registryResource = $this->get($resource); // throws exception if not found
            if (!$resource instanceof Zend_Acl_Resource_Interface) {
                $resource = $registryResource;
            }
        }
Hide
Permalink
Darby Felton added a comment -

Postponing to after 1.0.1.

Show
Darby Felton added a comment - Postponing to after 1.0.1.
Hide
Permalink
Darby Felton added a comment -

The issue description shows the same code as Zend_Acl already has at the top of isAllowed().

The proposed fix has no effect since:

  • Zend_Acl_Role_Registry::get() always returns an instance of Zend_Acl_Role_Interface unless an exception is thrown for a role not found, and
  • Zend_Acl::get() always returns an instance of Zend_Acl_Resource_Interface unless an exception is thrown for a resource not found.

Resolving as incomplete. See also ZF-1722 and ZF-1765.

Show
Darby Felton added a comment - The issue description shows the same code as Zend_Acl already has at the top of isAllowed(). The proposed fix has no effect since:
  • Zend_Acl_Role_Registry::get() always returns an instance of Zend_Acl_Role_Interface unless an exception is thrown for a role not found, and
  • Zend_Acl::get() always returns an instance of Zend_Acl_Resource_Interface unless an exception is thrown for a resource not found.
Resolving as incomplete. See also ZF-1722 and ZF-1765.
Hide
Permalink
Ralph Schindler added a comment -

Darby, the problem with ACL at current is its goals and architecture. The impression is that by using a is-a (instanceof) architecture, you are implying that you can implement Base Roles and Resoures and expect them to persist through the isAllowed, which is not the case. The initial problem is that isAllowed will take any supplied object that implements the interface, and turn it into the object presented at ACL creation time.

This is not what I want, and I think that moving forward, as more people start to use ACL for runtime resource access checking, this will be a need.

That being said, the current code:

if (null !== $role) {
            $role = $this->_getRoleRegistry()->get($role);
        }

        if (null !== $resource) {
            $resource = $this->get($resource);
        }

will always produce the same object supplied at ACL creation time.

Whereas, this code:

public function isAllowed($role = null, $resource = null, $privilege = null)
    {
        // make sure $role is a valid role
        if (null !== $role) {
            $registryRole = $this->_getRoleRegistry()->get($role); // throws exception if not found

            if (!$role instanceof Zend_Acl_Role_Interface) {
                $role = $registryRole;
            }
        }
        
        if (null !== $resource) {
            $registryResource = $this->get($resource); // throws exception if not found
            if (!$resource instanceof Zend_Acl_Resource_Interface) {
                $resource = $registryResource;
            }
        }

Will allow an object that implements the interface to persist through the isAllowed, as long as the ID (object type) matches somethign that is in the ACL registry.

Again, its an architectual issue, but one that I will address in a few days with code.

Show
Ralph Schindler added a comment - Darby, the problem with ACL at current is its goals and architecture. The impression is that by using a is-a (instanceof) architecture, you are implying that you can implement Base Roles and Resoures and expect them to persist through the isAllowed, which is not the case. The initial problem is that isAllowed will take any supplied object that implements the interface, and turn it into the object presented at ACL creation time. This is not what I want, and I think that moving forward, as more people start to use ACL for runtime resource access checking, this will be a need. That being said, the current code: 
if (null !== $role) {
            $role = $this->_getRoleRegistry()->get($role);
        }

        if (null !== $resource) {
            $resource = $this->get($resource);
        }
will always produce the same object supplied at ACL creation time. Whereas, this code: 
public function isAllowed($role = null, $resource = null, $privilege = null)
    {
        // make sure $role is a valid role
        if (null !== $role) {
            $registryRole = $this->_getRoleRegistry()->get($role); // throws exception if not found

            if (!$role instanceof Zend_Acl_Role_Interface) {
                $role = $registryRole;
            }
        }
        
        if (null !== $resource) {
            $registryResource = $this->get($resource); // throws exception if not found
            if (!$resource instanceof Zend_Acl_Resource_Interface) {
                $resource = $registryResource;
            }
        }
Will allow an object that implements the interface to persist through the isAllowed, as long as the ID (object type) matches somethign that is in the ACL registry. Again, its an architectual issue, but one that I will address in a few days with code.
Hide
Permalink
Ralph Schindler added a comment -

After discussion with darby, this is still an issue. (linked to ZF-1722 as well)

Show
Ralph Schindler added a comment - After discussion with darby, this is still an issue. (linked to ZF-1722 as well)
Hide
Permalink
Martijn Korse added a comment -

If i understand the problem correctly i think this shouldn't be solved here, but by overloading the _getRoleRegistry() and get() method.

I've created something in the past where i extended the Role Registry class and overloaded the getRoleRegistry() method (in an extended version of the Zend_Acl class), so it would return an object of my own registry class. and had no problem accessing the the properties of my custom role objects in assertions

Show
Martijn Korse added a comment - If i understand the problem correctly i think this shouldn't be solved here, but by overloading the _getRoleRegistry() and get() method. I've created something in the past where i extended the Role Registry class and overloaded the getRoleRegistry() method (in an extended version of the Zend_Acl class), so it would return an object of my own registry class. and had no problem accessing the the properties of my custom role objects in assertions
Hide
Permalink
Wil Sinclair added a comment -

I think you're the right person to assign this to, Ralph. We don't need to address it immediately, but I'd like closure on this soon after 1.8.

Show
Wil Sinclair added a comment - I think you're the right person to assign this to, Ralph. We don't need to address it immediately, but I'd like closure on this soon after 1.8.
Hide
Permalink
Avi Block added a comment -

Any update on this? 1.8 has been released!

Show
Avi Block added a comment - Any update on this? 1.8 has been released!
Hide
Permalink
Stefan Gehrig added a comment -

Anything new on this one?

Show
Stefan Gehrig added a comment - Anything new on this one?
Hide
Permalink
James Ellis added a comment -

Is Ralph actually working on this issue actively or should we not hold our breath?

Show
James Ellis added a comment - Is Ralph actually working on this issue actively or should we not hold our breath?
Hide
Permalink
Ralph Schindler added a comment -

Fix in place in trunk as of r17317, please test.

Show
Ralph Schindler added a comment - Fix in place in trunk as of r17317, please test.

People

Vote (12)
Watch (10)

Dates

  • Created:
    Updated:
    Resolved:

Time Tracking

Estimated:
1w
Original Estimate - 1 week
Remaining:
1w
Remaining Estimate - 1 week
Logged:
Not Specified
Time Spent - Not Specified
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.