Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Child roles not added #45

Open
jmaybury opened this issue Aug 2, 2019 · 5 comments · May be fixed by #48
Open

Child roles not added #45

jmaybury opened this issue Aug 2, 2019 · 5 comments · May be fixed by #48
Labels
bug

Comments

@jmaybury
Copy link

jmaybury commented Aug 2, 2019

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
@michalbundyra
Copy link
Member

@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
Copy link
Member

Jus to link PR: #46

@ezimuel
Copy link
Contributor

ezimuel commented Aug 6, 2019

@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 added a commit to ezimuel/zend-permissions-rbac that referenced this issue Aug 6, 2019
@ezimuel
Fix zendframework#45
4ebc5b2
@ezimuel ezimuel linked a pull request Aug 6, 2019 that will close this issue
Open
@ezimuel
Copy link
Contributor

ezimuel commented Aug 6, 2019

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

@ezimuel ezimuel added the bug label Aug 6, 2019
@weierophinney weierophinney mentioned this issue Dec 31, 2019
Open
@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-permissions-rbac; a new issue has been opened at laminas/laminas-permissions-rbac#3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #45 ezimuel/zend-permissions-rbac
4 participants
@weierophinney @ezimuel @jmaybury @michalbundyra