Inability to update a user's roles after login #12025
Comments
|
If your roles can dynamically change, you can create a listener that (somehow decides to) reload the token in your security component. You can implement this in your refresh user as well. If there's a solution for this, build-in, I would like to find out too. When your roles change, your token should change. |
|
@fabpot should we take roles into account in |
|
@stof I don't think that's desired in every situation. We have the same case but we don't want to have the user login again, that's why manually re-authenticate the user in a listener. Is it possible to add an event that's fired every time the |
|
@fabpot The problem is still actual. +1 to @stof 's idea of adding to |
|
Is there any way to reload the roles dynamically in Symfony 3.x? |
|
@Kobzol RE: "Is there any way to reload the roles dynamically in Symfony 3.x?" I think so? Here's how I went about it & it seems to be working, though maybe @stof or someone more involved would be able to shed some light on how much of a nasty bastardization it may be. You're able to register an subscriber listening to the Your event subscriber should be injected with (at a minimum) With this, the Long story short--you're able to modify the stored token before your controller action is executed (& annotations are evaluated) with an event subscriber. So, in terms of code, here's what an abstract version of what we've got: services.yml app.security.role_change_listener:
class: YourRoleListener
tags:
- { name: kernel.event_subscriber }
arguments:
- '@doctrine.orm.default_entity_manager'
- '@security.token_storage'YourRoleListener.php class YourRoleListener implements EventSubscriberInterface
{
/**
* @var EntityManager
*/
protected $entityManager;
/**
* @var TokenStorage
*/
protected $tokenStorage;
/**
* YourRoleListener constructor
*
* @param EntityManager $entityManager The entity manager to use to load roles
* @param TokenStorage $tokenStorage The token storage for the user
*/
public function __construct(EntityManager $entityManager, TokenStorage $tokenStorage)
{
$this->entityManager = $entityManager;
$this->tokenStorage = $tokenStorage;
}
/**
* Handles the kernel event
*
* @param FilterControllerEvent $event The dispatched event
*/
public function onKernelController(FilterControllerEvent $event)
{
if ($this->tokenStorage && $this->tokenStorage->getToken()) {
$token = $this->tokenStorage->getToken();
$user = $token->getUser();
// This check can be just `is_object` like in symfony core
// we're explicit about the class used
if ($user instanceof YourUserClass) {
// there didn't seem to be an easier way to grab the provider key,
// so using bound closure to retrieve it
$providerKeyGetter = function(TokenInterface $token) {
return $token->providerKey;
};
$boundProviderKeyGetter = Closure::bind($providerKeyGetter, null, $token);
// check & load roles for user here if necessary
$this->tokenStorage->setToken(
new UsernamePasswordToken(
$user,
$token->getCredentials(),
$boundProviderKeyGetter($token),
// loaded roles array goes here
)
);
}
}
}
/** {@inheritdoc} */
public static function getSubscribedEvents()
{
return [KernelEvents::CONTROLLER => ['onKernelController', 99]];
}
} |
|
I did something like that, but it is indeed a big hack and I didn't like it. It also caused some issues with authentication and other parts of the app (it also didn't show the right roles in the Symfony debug toolbar, but that's not that important). and the UserVoter looks like this: class UserVoter extends RoleVoter
{
protected function extractRoles(TokenInterface $token)
{
$user = $token->getUser();
return $user instanceof UserInterface ? array_map(function(string $role)
{
return new Role($role);
}, $user->getRoles()) : [];
}
}I had to create a simple Role class that implements the With this the roles are extracted from the user and thus are dynamically refreshed on every request. So far it seems that it works and doesn't interfere with the authentication system in any way. |
|
Ah, nice! Good to have another approach--thanks! @stof -- would the custom voter be the preferred approach, or is there some officially sanctioned way to achieve this? |
|
For the most part, I think the voter approach will work (but definitely test to make sure I'm not missing some edge case). So for now, I'd say this is the way to go. Make sure you haven't changed the You will still have a few weird issues, however. First, |
|
I used a different approach. First, I made Token think that User changes when roles change: class User implements \Serializable, \Symfony\Component\Security\Core\User\EquatableInterface
{
// ...
public function serialize()
{
return serialize([
// ...
$this->roles,
]);
}
public function unserialize($serialized)
{
list (
// ...
$this->roles
) = unserialize($serialized);
}
public function isEqualTo(UserInterface $user)
{
// ...
if (array_diff($this->getRoles(), $user->getRoles())) {
return false;
}
return true;
}
}I also have the When I change the user's roles, he get's reauthenticated with the |
|
@andyvenus one way you can trigger role reload is to implement the
For all this to work, you'd need to use If you follow all this any sessions that a user might have will be updated with the new roles. Alternatively you can serialize the roles and compare them as in the comment above. If you want to ban a user, you could consider disabling their account (so that Changing username / password or any other serialized properties will not log the user out (contrary to what the docs state: http://symfony.com/doc/current/security/entity_provider.html#security-serialize-equatable "For example, if the |
|
@vudaltsov what if you didn't have the Or let's say if you were to not implement |
|
It should really be part of |
|
We really need a "hasUserChanged" and "doesUserNeedReloading". Using hasUserChanged for changes that should invalidate an entire session (and potentially case a logout when not using remember_me), and doesUserNeedReloading that allows modifying the user in the security token, without having to reauthenticate (remember_me). |
|
PR open to address this issue: #24331 |
|
Can someone point me into the right direction to get a better understanding of the issue. I would like to help out. What exactly changed between v3.3 and v3.4 that caused this? In my case, Symfony 3.3 and previous versions always call the With 3.4 this is not executed unless I turn on security.always_authenticate_before_granting but even then it is being called for all IS_GRANTED calls but not before the Firewall denies access because the User still has the roles from the old UsernamePasswordToken that was loaded from the session. |
|
I have the same issue: my UserProvider currently retrieves a fresh user on each request, but RoleVoter seems to do its checks against "old" roles list obtained from token. @vudaltsov solution not working for me. public function isEqualTo(UserInterface $user)
{
// ...
if (array_diff($this->getRoles(), $user->getRoles())) {
return false;
}
return true;
}because: Second: when I added a proper check to public function authenticateToken(TokenInterface $token, UserProviderInterface $userProvider, $providerKey)
{
try {
$user = $userProvider->loadUserByUsername($token->getUsername());
} catch (UsernameNotFoundException $exception) {
throw new CustomUserMessageAuthenticationException('Invalid login or password');
}
$isPasswordValid = $this->encoder->isPasswordValid($user, $token->getCredentials());
if ($isPasswordValid) {
return new UsernamePasswordToken(
$user,
$user->getPassword(),
$providerKey,
$user->getRoles()
);
}
throw new CustomUserMessageAuthenticationException('Invalid login or password');
}but this time @Kobzol solution seems to work, however:
|
|
Upd, so finally it works:
|
|
The easiest solution would be to leave the identity alone until the next login. There doesn't seem to be a nice solution to reload roles, but that's okay in my opinion. Roles are part of your identity and if your identity changes, you should either be logged out or not updated til your next login. This ensures the visitor is going through the proper identification (authentication) process when it changes and possibly gets blocked by a user checker (or not). @RealJTG that won't work in 4.0 indeed and sounds like a step in the wrong direction. |
|
@iltar agree roles are part of identity but from the UX point of view it is not so okay to reload roles by logging out/back in. Let's say you're logged in and on your free plan, you upgrade to premium, then it logs you out and asks you to log back in because you've just upgraded. |
|
I would argue against the "roles are part of identity". If the administrator chooses to disable an account or restrict it while the user is logged in, I have no way to detect it. $this->guardHandler->authenticateUserAndHandleSuccess(
$user,
$request,
$this->authenticator,
'main'
);I don't get why user infos are refreshed but not the roles on every request. |
|
This does not make sense to me as well. I don't see a reason why this was changed in v3.4 and not sure how other people are handling this. A few of us can't be the only ones with this issue. It looks to me like the only sane solution at the moment (since it has been over 4 years since this issue was filed) is to not rely on Symfony roles, firewall or security component but to roll your own. |
|
Security used to be taken somewhat lightly in symfony, it used to do the complete opposite from what was in the docs (symfony/symfony-docs#5419 it used to say it will log you out but it didn't etc., now that section has been removed so try for yourself how it behaves in each scenario), then you've got the almost 5 year old roles issues and recently the symfony-checker's security.sensiolabs.org host was decommissioned with very little comms - afaik just twitter - because who isn't always on twitter ;-) I'd say use with caution, don't roll your own (do people still do that?). Test, test and test (you're doing automated tests, right?) Other components including Guard are superb (however Guard still needs to deal with the legacy issues, Ryan has done a tremendous job to make things better with Guard). |
Creating a new UsernamePasswordTokes is re-authentication AFAIK.
I don't mind that happening.
Not true. You could have generated a new UsernamePasswordToken with the data provided by the UserProvider if Equatable says that they are not the same.
That doesn't work in the firewall since it happens early in the request. Actually, the firewall blocking of stuff based on the role that was stored in the session is the only issue. The other issue being that I am working on a 7 year old app :) We do use voters for other stuff, but firewall is causing issues for us. |
|
What I understand is that for now Symfony Security has a binary approach: you can be white (identity is OK) or black (identity is not OK). And the response is you can stay or you are kicked out. What we (?) need is some gray area : identity has changed but meh you can stay in (authentication stays true) ! But symfony should trigger again post authentication security stuff (roles, voters, etc).
If user But if EDIT: since it's working with |
Is there any solution for this post? |
|
@arturslogins I've no solution if it's what you mean. but as said it's working if you allow |
For me, i'm searching solution for multiple user profiles for one single symfony app (Session) When i change profile, i would like to update user session with selected user data object. |
@linaori what I meant there is that the behaviour of being logged out and asked to log back in after upgrading to premium is wrong - they should be part of authorization but as the roles are not reloaded hence the need for re-authentication. Guess if you injected a repository in a voter to check the live data that would work, but still in an ideal world (?) shouldn't need to do that on each request post-upgrade to "premium". |
|
Injecting a repository in the voter would work, but not with the firewall. In the end, we managed to solve our issue by implementing Guard and extending the AbstractFormLoginAuthenticator. I still think this could be an easy fix in Symfony, after checking if the stored Token is equal to the freshly obtained User information Authentication did not change (it is still the same user) and authorization should be handled from the freshly obtained user information. After all, that is why we have the Equatable interface, to be able to handle ourselves if the user should be logged out or not. Although this was probably made to fix some other issues what we have now is pretty broken. The object you get when calling I could make a pull request for this in a week or so (it's a simple change), but not sure if it would be accepted. |
Next weekend we have a hackathon and we will be having a brainstorm session related to the UserInterface in Symfony and the Security token, so it might be a conflict on this part. My point of view remains though, the security token should be immutable, thus no roles change. If you need to have authorization, use voters, which query if the user can do something. Don't rely on the identity for this. |
|
How does one implement something like that with the Symfony firewall? |
@gjuric what's the problem with firewalls? They don't support voters? |
|
You don't implement it in the firewall, you implement it in a voter so you can call |
|
Are voters supported in |
|
Yes, it's confusing as it's called "roles", they are actually the "attributes" that are passed to voters. |
|
Thanks @linaori - I've searched for this in the docs but no mention of |
|
Correct! I've proposed to change it to "attributes" before, but isn't much clearer. Perhaps we should name it "is_granted" instead? |
|
|
|
We are centralizing discussions on authentication at #30914. As in the proposed changes the Roles would no longer be an integral part of the UserInterface it would be easier to handle situations like this, and this issue itself is no longer a structural problem. As fixing it in 4.x would be a potentially breaking change in authorization I don't think we need to keep this issue open separately. |
|
Update @Kobzol solution from Symfony 4.* Add file '/src/App/Security/UserVoter.php' <?php
namespace App\Security;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\RoleVoter;
use App\Entity\User;
class UserVoter extends RoleVoter{
/**
* @param TokenInterface $token
* @return array|string[]
*/
protected function extractRoles(TokenInterface $token)
{
$user = $token->getUser();
if(!$user instanceof User){
return parent::extractRoles($token);
}
return $user->getRoles();
}
}Add interfaces <?php
use Serializable;
use Symfony\Component\Security\Core\User\EquatableInterface;
use Symfony\Component\Security\Core\User\UserInterface;
class User implements UserInterface, EquatableInterface, Serializable
{
// ...
public function isEqualTo(UserInterface $user):bool
{
if(!$user instanceof static){
return false;
}
if($user->id !== $this->id){
return false;
}
if($user->email != $this->email){
return false;
}
if($user->password !== $this->password){
return false;
}
if($user->salt !== $this->salt){
return false;
}
// Check isActive, isBanned, isRemove, etc.
if($this->isActive){
return false;
}
return true;
}
public function serialize()
{
return serialize([
$this->id,
// isActive, isBanned, isRemove, etc.
$this->isActive,
$this->roles,
$this->password,
$this->email,
$this->salt,
]);
}
public function unserialize($serialized)
{
[
$this->id,
// isActive, isBanned, isRemove, etc.
$this->isActive,
$this->roles,
$this->password,
$this->email,
$this->salt,
] = unserialize($serialized);
}
}Add interface use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
class UserRepository implements UserProviderInterface
{
// ...
public function loadUserByUsername($username)
{
return $this->findOneBy(['email' => $username]);
}
public function refreshUser(UserInterface $user)
{
if(!$user instanceof User){
throw new UnsupportedUserException("Expected {$this->getClassName()} class");
}
$refreshUser = $this->getById($user->getId());
// If change important user fields
if(!$refreshUser->isEqualTo($user)){
throw new UsernameNotFoundException('Authentication required again');
}
return $refreshUser;
}
public function supportsClass($class)
{
return $class === $this->getClassName();
}
} |
|
If roles are changed dynamically, I think it's enough to extend RoleVoter.
|
This is true, but if the user is banned, then the user should not have any access, even if the roles field remains unchanged. In order for everything to work correctly, the rest of the code is needed. |
|
Still not able to update user roles after login. user login with limited rights and in between admin give user full rights but user not able to get all feature untill next login. Please give me complete solution. i am using symfony 5.1. This is urgent . Thanks. |
Still not able to update user roles after login. user login with limited rights and in between admin give user full rights but user not able to get all feature untill next login. Please give me complete solution. i am using symfony 5.1. This is urgent . Thanks. All the solution given i tried but not working. PLease help me on this. Thanks. |
Still not able to update user roles after login. user login with limited rights and in between admin give user full rights but user not able to get all feature untill next login. Please give me complete solution. i am using symfony 5.1. This is urgent . Thanks. All the solution given i tried but not working. PLease help me on this. Thanks. |
Still not able to update user roles after login. user login with limited rights and in between admin give user full rights but user not able to get all feature untill next login. Please give me complete solution. i am using symfony 5.1. This is urgent . Thanks. All the solution given i tried but not working. PLease help me on this. Thanks. |
Still not able to update user roles after login. user login with limited rights and in between admin give user full rights but user not able to get all feature untill next login. Please give me complete solution. i am using symfony 5.1. This is urgent . Thanks. All the solution given i tried but not working. PLease help me on this. Thanks. |
|
@pcmishra22 this is not a support channel, and please don't spam on github. Telling people it's urgent doesn't yield you more help, if anything people will just get annoyed and ignore you. For the proper support channels please visit https://symfony.com/support |
and others
I'm implementing the security component in my app and was surprised to find that a user's roles are decided at login time and cannot be easily updated after that. While the chosen UserInterface object can be refreshed on each load to check for changes (like the password changing or any other details), there is no way to check to see if the user's roles have changed since login. My roles are coming from the UserInterface getRoles method.
This means if I have an admin user for example and I remove their admin abilities, they remain an admin until they decide to log out. Or moving a user to a "banned" role would have no effect.
Ideally, a UsernamePasswordToken or any other token that contains a UserInterface object would check the UserInterface object's getRoles method to see if the roles have changed. But even just a setRoles method on AbstractToken would be useful so that the option is there to keep user's roles fresh.
Of course I may be missing something that allows you to make sure a user's roles are kept fresh so I'd love to hear about it :)
The text was updated successfully, but these errors were encountered: