Issue Details (XML | Word | Printable)

Key: ZF-1721
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Ralph Schindler
Reporter: Ralph Schindler
Votes: 12
Watchers: 10
Operations

If you were logged in you would be able to see more operations.
Google issue summary
Zend Framework

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

Created: 14/Jul/07 04:52 PM   Updated: 14/Aug/09 02:38 PM   Resolved: 14/Aug/09 02:38 PM
Component/s: Zend_Acl
Affects Version/s: 1.0.0
Fix Version/s: 1.9.1

Time Tracking:
Original Estimate: 1 week
Original Estimate - 1 week
Remaining Estimate: 1 week
Remaining Estimate - 1 week
Time Spent: Not Specified
Time Spent - Not Specified

Issue Links:
Related


 Description  « Hide

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
        }


Ralph Schindler added a comment - 14/Jul/07 06:47 PM

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;
            }
        }

Darby Felton added a comment - 26/Jul/07 11:54 AM

Postponing to after 1.0.1.


Darby Felton added a comment - 01/Aug/07 01:17 PM

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.


Ralph Schindler added a comment - 01/Aug/07 01:41 PM

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.


Ralph Schindler added a comment - 03/Aug/07 12:37 PM

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


Martijn Korse added a comment - 08/Aug/08 06:08 AM

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


Wil Sinclair added a comment - 19/Dec/08 11:32 AM

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.


Avi Block added a comment - 02/May/09 08:46 PM

Any update on this? 1.8 has been released!


Stefan Gehrig added a comment - 15/Jun/09 01:55 AM

Anything new on this one?


James Ellis added a comment - 16/Jun/09 06:41 AM

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


Ralph Schindler added a comment - 30/Jul/09 06:40 PM

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