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

Child roles not added

Open jmaybury opened this issue 6 years ago • 5 comments
trafficstars

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

jmaybury avatar Aug 02 '19 19:08 jmaybury

@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).

michalbundyra avatar Aug 03 '19 10:08 michalbundyra

Jus to link PR: #46

michalbundyra avatar Aug 06 '19 05:08 michalbundyra

@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.

ezimuel avatar Aug 06 '19 08:08 ezimuel

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

ezimuel avatar Aug 06 '19 08:08 ezimuel

This repository has been closed and moved to laminas/laminas-permissions-rbac; a new issue has been opened at https://github.com/laminas/laminas-permissions-rbac/issues/3.

weierophinney avatar Dec 31 '19 22:12 weierophinney