Skip to content

Inability to update a user's roles after login #12025

Closed
@andyvenus

Description

@andyvenus

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 :)

Activity

linaori

linaori commented on Sep 25, 2014

@linaori
Contributor

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.

stof

stof commented on Sep 25, 2014

@stof
Member

@fabpot should we take roles into account in AbstractToken::hasUserChanged to force re-authenticating when roles are changed ?

linaori

linaori commented on Sep 25, 2014

@linaori
Contributor

@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 AbstractToken::hasUserChanged()? People can hook on to that and manually determine what they want to have done. For example: #11320 would really help in this case.

origaminal

origaminal commented on Aug 20, 2015

@origaminal
Contributor

@fabpot The problem is still actual.
http://stackoverflow.com/questions/31986324/symfony2-authentication-roles-not-updating-in-twig-template

+1 to @stof 's idea of adding to AbstractToken::hasUserChanged a roles comparison: getRoles is part on UserInterface and it looks logically.

Kobzol

Kobzol commented on May 1, 2016

@Kobzol

Is there any way to reload the roles dynamically in Symfony 3.x?
I load them from DB and they can change dynamically.
They change from an outside source, so I can't simply reload the user after the change, because I don't know when it happens.
I tried using both EquatableInterfaceand the always_authenticate_before_granting setting, but neither of them worked, when I used them, no roles were loaded at all (I load them using a User repository).

troymccabe

troymccabe commented on May 17, 2016

@troymccabe
Contributor

@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 KernelEvents::CONTROLLER event (I tried KernelEvents::REQUEST, though that seemed a bit early in the bootstrapping process--$container->get('security.token_storage')->getToken() was null.)

Your event subscriber should be injected with (at a minimum) @security.token_storage, though injecting things like Doctrine to load roles would be beneficial as well.

With this, the @Security annotation from extra bundle evaluates has_role properly after calling setToken, and the roles available in templates and such down the request pipeline are the ones set in the YourRoleListener.

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]];
    }
}
Kobzol

Kobzol commented on May 17, 2016

@Kobzol

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).
After a advice received elsewhere I solved it in a different way, which is way simpler and works so far, maybe it will be useful to others.
You can create your own voter and extract the roles to be checked not from the token, but from the user object (and reload the user's roles on each request in his repository or somewhere else).
services.yml:

login.user_voter:
        class: AppBundle\Security\UserVoter
        tags:
            - { name: security.voter, priority: 245 } # priority is important to override the default
        public: false         # small performance boost

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 Symfony\Component\Security\Core\Role\RoleInterface and returns the stored role string in the getRole method. This was required because the authentication system expected that I return Role objects (not strings) from the extractRoles method.

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.

troymccabe

troymccabe commented on May 17, 2016

@troymccabe
Contributor

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?

weaverryan

weaverryan commented on Feb 17, 2017

@weaverryan
Member

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 access_decision_manager.strategy configuration in security (i.e. make sure this configuration does not appear in your security.yml) - that's important for this to work.

You will still have a few weird issues, however. First, role_hierarchy probably won't work. And also, your web debug toolbar will still probably show the old roles.

vudaltsov

vudaltsov commented on Mar 3, 2017

@vudaltsov
Contributor

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 remember_me functionality enabled.

When I change the user's roles, he get's reauthenticated with the RememberMeToken and new roles instead of being logged out.

ThePeterMick

ThePeterMick commented on Mar 5, 2017

@ThePeterMick

@andyvenus one way you can trigger role reload is to implement the EquatableInterface and do a custom check for example:

  • Extend the User class with an updatedAt property.
  • Once you change user roles, update the updatedAt value at that time (you'd need to include updatedAt in the serialize / unserialize call)
  • Please note that writing your own isEqualTo (EquatableInterface) function will need to cover for any checks that typically would be covered by out of the box when / if you are using AdvancedUserInterface / UserInterface (those can be viewed here https://github.com/symfony/symfony/blob/3.2/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php#L244-L287 so you'd need to take of it yourself (please let me know if you need a snippet). On L250 you can see that if your user is an instance of EquatableInterface it will relay any checks to it.

For all this to work, you'd need to use access_control in security.yml or use is_granted call if using @Security annotation as has_role won't trigger a role reload as described here: #21861

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 isEnabled returns false) and this will actually log the user out in any session they might have as I've described here long time ago: #17023 and #13870

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 username on the 2 User objects doesn't match for some reason, then the user will be logged out for security reasons." ). There is a potential fix for this kindly made by @xaben here: #19033 but given many people already might rely on the exisiting functionality hopefully this can be made parameterized / optional in terms of which fields actually do trigger a logout.

ThePeterMick

ThePeterMick commented on Mar 5, 2017

@ThePeterMick

@vudaltsov what if you didn't have the remember_me functionality enabled? Would it actually log you out completely when you change the roles?

Or let's say if you were to not implement isEqualTo and just change username / password (whatever is serialized as "standard" as per the docs. Does it log you out completely? There is an issue for this at #17023 as this won't log you out, only disabling the account will log the user out completely i.e. changing the isEnabled to false.

33 remaining items

vudaltsov

vudaltsov commented on Apr 2, 2019

@vudaltsov
Contributor

requires_granted?

curry684

curry684 commented on Apr 6, 2019

@curry684
Contributor

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.

uginroot

uginroot commented on Dec 5, 2019

@uginroot

Update @Kobzol solution from Symfony 4.*

Add file '/src/App/Security/UserVoter.php'
Responsible for taking user roles from the database with each request.

<?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 UserInterface, EquatableInterface, Serializable from class App\Entity\User
Responsible for serialization, deserialization and verification of important user data for current token user authorization.
If the isEqualTo method returns false, then the user will have to login again.

<?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 UserProviderInterface from class \App\Repository\UserRepository
Responsible for updating user data and relocate user for relogin if important data has changed

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();
    }
}
carcabot

carcabot commented on Dec 21, 2019

@carcabot

If roles are changed dynamically, I think it's enough to extend RoleVoter.

Add file '/src/App/Security/UserVoter.php'
Responsible for taking user roles from the database with each request.

uginroot

uginroot commented on Dec 21, 2019

@uginroot

If roles are changed dynamically, I think it's enough to extend RoleVoter.

Add file '/src/App/Security/UserVoter.php'
Responsible for taking user roles from the database with each request.

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.

pcmishra22

pcmishra22 commented on Nov 12, 2020

@pcmishra22

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.

pcmishra22

pcmishra22 commented on Nov 12, 2020

@pcmishra22

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.

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

pcmishra22 commented on Nov 12, 2020

@pcmishra22

@fabpot should we take roles into account in AbstractToken::hasUserChanged to force re-authenticating when roles are changed ?

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

pcmishra22 commented on Nov 12, 2020

@pcmishra22

Is there any way to reload the roles dynamically in Symfony 3.x?
I load them from DB and they can change dynamically.
They change from an outside source, so I can't simply reload the user after the change, because I don't know when it happens.
I tried using both EquatableInterfaceand the always_authenticate_before_granting setting, but neither of them worked, when I used them, no roles were loaded at all (I load them using a User repository).

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

pcmishra22 commented on Nov 12, 2020

@pcmishra22

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).
After a advice received elsewhere I solved it in a different way, which is way simpler and works so far, maybe it will be useful to others.
You can create your own voter and extract the roles to be checked not from the token, but from the user object (and reload the user's roles on each request in his repository or somewhere else).
services.yml:

login.user_voter:
        class: AppBundle\Security\UserVoter
        tags:
            - { name: security.voter, priority: 245 } # priority is important to override the default
        public: false         # small performance boost

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 Symfony\Component\Security\Core\Role\RoleInterface and returns the stored role string in the getRole method. This was required because the authentication system expected that I return Role objects (not strings) from the extractRoles method.

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.

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.

linaori

linaori commented on Nov 12, 2020

@linaori
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @fabpot@javiereguiluz@weaverryan@mahono@gjuric

        Issue actions

          Inability to update a user's roles after login · Issue #12025 · symfony/symfony