laminas-permissions-rbac icon indicating copy to clipboard operation
laminas-permissions-rbac copied to clipboard

Child roles not added

Open weierophinney opened this issue 5 years ago • 5 comments

In v3.x, if you create a role, add a child role too it, then add the parent role to the Rbac instance, the child role is not added. Using your own example code:

use Zend\Permissions\Rbac\Rbac;
use Zend\Permissions\Rbac\Role;

$rbac = new Rbac();
$foo  = new Role('foo');
$bar  = new Role('bar');

$foo->addChild($bar);
$rbac->addRole($foo);

echo $rbac->hasRole("bar") ? "works" : "doesn't work";

This displays "doesn't work". Similarly, attempting a permission check against role "bar" raises a "No role with name 'bar' could be found" exception.

This worked in 2.x


Originally posted by @jmaybury at https://github.com/zendframework/zend-permissions-rbac/issues/45

weierophinney avatar Dec 31 '19 22:12 weierophinney

@jmaybury I've checked the issue and I think it is a valid one. I mean I am not sure if it is going to be added back to v3 or not, but if not - the change is not documented. The problem was introduced in PR #34 and that change was never noted. I was looking also on changes in the documentation: https://github.com/zendframework/zend-permissions-rbac/pull/34/files#diff-fb6293f79cf977d58bdcbddfe2ab7c94 and there is said:

hasRole - Recursively queries the RBAC for the given role, returning `true` if found, `false` otherwise.
getRole - Get the role specified by name, raising an exception if not found.

Before getRole was also recursive. Of course it has no sense, that one method is recursive and the other not, because we can finish up with case when: hasRole(xyz)==true and getRole(xyz) does not return the role (this is not the implementation either, thankfully).

I would ping here the author of the change - @ezimuel, so maybe he can explain if it was desired change for v3, or not. If so, probably we would need update the migration guide and the documentation, also add some tests to cover that behaviour (as now there is no any).


Originally posted by @michalbundyra at https://github.com/zendframework/zend-permissions-rbac/issues/45#issuecomment-517913807

weierophinney avatar Dec 31 '19 22:12 weierophinney

Jus to link PR: #46


Originally posted by @michalbundyra at https://github.com/zendframework/zend-permissions-rbac/issues/45#issuecomment-518509070

weierophinney avatar Dec 31 '19 22:12 weierophinney

@webimpress I agree this is a bug. The #46 it doesn't fix the issue in my opinion. We need to fix this is addRole() and not in hasRole(). I'll send a PR in a while.


Originally posted by @ezimuel at https://github.com/zendframework/zend-permissions-rbac/issues/45#issuecomment-518560827

weierophinney avatar Dec 31 '19 22:12 weierophinney

@webimpress, @jmaybury Just send the PR #48 to fix this issue.


Originally posted by @ezimuel at https://github.com/zendframework/zend-permissions-rbac/issues/45#issuecomment-518571779

weierophinney avatar Dec 31 '19 22:12 weierophinney

So, where is the fix commit?

badmansan avatar Jul 20 '20 12:07 badmansan