Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Child roles not added #3

Open
weierophinney opened this issue Dec 31, 2019 · 5 comments
Open

Child roles not added #3

weierophinney opened this issue Dec 31, 2019 · 5 comments
Labels
Bug Something isn't working

Comments

@weierophinney
Copy link
Member

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 zendframework/zend-permissions-rbac#45

@weierophinney weierophinney added the Bug Something isn't working label Dec 31, 2019
@weierophinney
Copy link
Member Author

@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 zendframework/zend-permissions-rbac#45 (comment)

@weierophinney
Copy link
Member Author

Jus to link PR: #46


Originally posted by @michalbundyra at zendframework/zend-permissions-rbac#45 (comment)

@weierophinney
Copy link
Member Author

@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 zendframework/zend-permissions-rbac#45 (comment)

@weierophinney
Copy link
Member Author

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


Originally posted by @ezimuel at zendframework/zend-permissions-rbac#45 (comment)

@badmansan
Copy link

So, where is the fix commit?

gennadiylitvinyuk added a commit to gennadiylitvinyuk/laminas-permissions-rbac that referenced this issue Apr 3, 2021
Closes issue laminas#3. Original fix by @ezimuel

Signed-off-by: Gennadiy Litvinyuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants