PUGX / PUGXMultiUserBundle

An extension for FOSUserBundle to handle users of different types. Compatible with Doctrine ORM.
163 stars 96 forks source link

Logging in wrong user type #76

Open garybutton opened 9 years ago

garybutton commented 9 years ago

Not sure if this is a bug or something I have done wrong with my set ups.

Using PUGXMultiUserBundle with FOSUserBundle to create 3 different user types (admin, User A, User B). I started encountering errors where when I logged in as User A, I found myself logged in the admin area. When I logged in as User B, I found myself in User A area. Not good!

After hours of tracing what was going wrong I found it...

FOS\UserBundle\Security\UserProvider.php

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

        if (!$this->supportsClass(get_class($user))) {
            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;
    }

What was going wrong is that where the user manager queries a user by ID - I had users across the 3 different database tables with the same ID, so although the correct user entity was being searched for, it was finding a user with the same ID in a different table before it got to the correct one.

Im not sure the best way to fix this but for now I have just had to changed the user query line to this (all my users will have a unique username):

if (null === $reloadedUser = $this->userManager->findUserBy(array('usernameCanonical' => $user->getUsernameCanonical()))) {

Any ideas how we can fix this permanently?

Thanks

giorrrgio commented 9 years ago

although the correct user entity was being searched for, it was finding a user with the same ID in a different table before it got to the correct one

Can you clarify this point? If the correct entity is searched for, than the correct repository should be called, than the correct table should be queried. What am I missing?

garybutton commented 9 years ago

The $user passed to refreshUser() in UserProvider.php is the user I have attempted to log in with. Correct at this point. But when the findUserBy() function is called in PUGX\MultiUserBundle\Doctrine\UserManager.php, the incorrect user is returned:

public function findUserBy(array $criteria)
    {
        $classes = $this->userDiscriminator->getClasses();

        foreach ($classes as $class) {

            $repo = $this->om->getRepository($class);

            if (!$repo) {
                throw new \LogicException(sprintf('Repository "%s" not found', $class));
            }

            // Some models does not have the property associated to the criteria
            try {
                $user = $repo->findOneBy($criteria);
            } catch (ORMException $e) {
                $user = null;
            }

            if ($user) {
                $this->userDiscriminator->setClass($class);
                return $user;
            }
        }

        return null;
    }

The $classes variable holds each of my user entities. Correct at this point. But, as an example, the first class it loops over here is User A, querying user ID 2. The result is successful and User A with ID 2 is returned to the refreshUser() function. But actually, I was attempting to log in as a User B type. But because my User B that im attempting to log in as also has an ID of 2, the foreach loop does not get queried beacuse user ID 2 was found in the first class of the loop and returned before reaching the next class.

Hope that makes more sense?

Thanks