Incorrect user in security token

[joshbuckley182]

I've got a test project with two users, each with their own firewall and necessary config. Both users seem to be working (logging in as a user gives access to user routes, admin gives access to admin routes).

However; the user in the security token seems to always be of the user type. I checked the Doctrine log, and can see that it's always loading the user type. As I have one user of type user, and one of type admin, both users have an ID in the db of 1. When I log in as an admin, it's loading user 1.

I'm not sure at the moment how the security token is populated with the user. I'm thinking it could possibly be the priority of an event handler like a similar issue I've submitted a pull request for.

Any ideas?

[sstok]

Are you using the correct user-provider service? Each user-system has its own user-provider which only uses the user-manager of that user-system.

[joshbuckley182]

I've found the problem: FOS\UserBundle\Security\UserProvider::refreshUser() only checks that the given user to refresh is of type FOS\UserBundle\Model\User (or FOS\UserBundle\Propel\User). So, with two user providers, whenever refreshUser() is called, the first UserProvider matches, resulting in the refreshed user being incorrect.

I've changed the UserProvider class so that it can tell the difference between user types:

public function refreshUser(SecurityUserInterface $user)
    {
        if (!$user instanceof User && !$user instanceof PropelUser) {
            throw new UnsupportedUserException(sprintf('Expected an instance of FOS\UserBundle\Model\User, but got "%s".', get_class($user)));
        }

        // Add this
        if (get_class($user) != $this->userManager->getClass()) {
            throw new UnsupportedUserException(sprintf('Expected an instance of %s, but got "%s".', $this->userManager->getClass(), get_class($user)));
        }

        if (null === $reloadedUser = $this->userManager->findUserBy(array('id' => $user->getId()))) {
            throw new UsernameNotFoundException(sprintf('User with ID "%d" could not be reloaded.', $user->getId()));
        }

        return $reloadedUser;
    }

I'm going to submit a pull request to FOSUserBundle, but if they don't accept it, I'll submit a pull request to this bundle.

[joshbuckley182]

Wrote that before I saw your reply. I think so, everything seems to be working fine with that check. Unless you think I might have done something wrong in the configuration?

[sstok]

What does the configuration of your security looks like?

[joshbuckley182]

security.yml: https://github.com/joshbuckley182/rmub-test/blob/RollerworksMultiUserBundle/app/config/security.yml

[sstok]

Ah nice this will make testing/debugging allot easier :+1:

[sstok]

Wait I just remembered something, the user-discriminator also uses a session to remember the current user-system. If you previously used 'user' and then go to admin it will use 'user' instead.

I'm not sure its the cause of this issue but it just might. I will look into this tomorrow.

[joshbuckley182]

Thanks for looking into it.

I've been trying out my test project with the above change, and although it seems to be working ok once logged in, the registration seems to be messed up. I'm going to see if I can get anywhere with it myself; I'll probably be able to work on it tomorrow too.

[sstok]

Finally! I got it, its indeed a problem with the FOSUserBundle. The Symfony\Component\Security\Http\Firewall\ContextListener::refreshUser() always uses the UserProvider given to it, the user-provider should throw an UnsupportedUserException exception then.

If the FOSUserBundle developers can not fix this we can provide our own user-provider with this bundle, but luckily we can extend the base user-provider then.

I will open a PR at the FOSUserBundle for this.

[joshbuckley182]

Cool, glad we've come to the same conclusion. If it helps/is quicker, I've got the change locally ready to push...

[sstok]

You mean for this bundle or the FOSUserBundle?

[joshbuckley182]

FOSUserBundle. I've just cloned the repo so I can push the change.

Would it be worth fixing in this bundle too until fix is merged into FOSUserBundle?

[sstok]

https://github.com/FriendsOfSymfony/FOSUserBundle/pull/1239

[joshbuckley182]

Cool, hopefully they'll merge it. Thanks!

[joshbuckley182]

Fixed by FriendsOfSymfony/FOSUserBundle#1239

Thanks!