Tests etc #2
Tests etc #2
Conversation
# Conflicts: # src/Repository/RoleRepository.php
|
|
||
| try { | ||
| return $this->rbacManager->hasPermission($attribute, $user->getId()); | ||
| return $this->rbacManager->hasPermission($attribute, $user->getUserIdentifier()); |
There was a problem hiding this comment.
I think this could break things because subsequent queries rely on the user id, not the public-facing user identifier (email, username, etc). It uses the id for subsequent authorization queries.
But as you know, there is no UserInterface::id:
https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Security/Core/User/UserInterface.php
For me, static analysis has highlighted that this bundle is designed to the wrong interface (UserInterface instead of User). I think that the fix here would be to depend on User.
There was a problem hiding this comment.
We could also keep it the way it is, but change the SQL queries
There was a problem hiding this comment.
yea I think the issue is that the User entity comes from the config and we're not sure that id will exist either.
in our case, I think userIdentifier works better?
There was a problem hiding this comment.
userIdentifier works, I think we'd just need to update Rbac::hasPermission() to transform the userIdentifier string into an int for subsequent queries. We could also modify PermissionRepository::hasPermission(). Downside is more lookups/joins as opposed to original implementation that relies only on ids.
No description provided.