PUGX / PUGXMultiUserBundle

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

Auth Listener & Other providers #63

Open ogizanagi opened 10 years ago

ogizanagi commented 10 years ago

Hi,

Thank you for your great work.

I think there is a problem with the AutenticationListener and the setClass method of the UserDiscriminator. Indeed, in case of a configured chained provider, with a mix of in_memory users, PUGXUsers and other custom providers, login fails because of this listener, or even for the use of another simple provider. Example for in_memory user:

Impossible to set the class discriminator, because the class "Symfony\Component\Security\Core\User\User" is not present in the entities list

with the following security conf :

providers:
        fos_users:
            id: fos_user.user_provider.username_email
        admin:
            memory:
                users:
                    - { name: %main_admin_login%, password: %main_admin_password%, roles: ['ROLE_ADMIN', 'ROLE_SUPER_ADMIN'] }
        application:
            id: acme.application.provider
        chained_provider:
            chain:
                providers:
                    - admin
                    - fos_users
                    - application

The incriminated method is :

protected function discriminate($user)
{
    $this->userDiscriminator->setClass(get_class($user), true);
}

This method is called on every login & switch user events, and generates an exception if the user type isn't one of the configured PUGX users, with a discriminator. Basically, this means it will fail on every login attempt with another user type, with a chained or any other provider, which I consider isn't a good way to achieve what this bundle is meant for : allow users inheritance. This should not alter the basic symfony's functionnalities.

Maybe the LogicException thrown by the UserDiscriminator::setClass method should be catch in this listener ? Is this LogicException really usefull ? For which cases ? Is this AutenticationListener even necessary ? Isn't the discriminator set by the UserManager ?

ogizanagi commented 10 years ago

Currently I by-pass the listener, overriding the it by this one:

services:

    #Overrides AuthenticationListener from PUGXMultiUserBundel
    pugx_multi_user.listener.authentication:
      class:     Acme\UserBundle\Listeners\Security\AuthenticationListener
      arguments: ["@pugx_user.manager.user_discriminator", %fos_user.model.user.class%]
      tags:
            - { name: kernel.event_subscriber }
namespace Acme\UserBundle\Listeners\Security;

use PUGX\MultiUserBundle\Listener\AuthenticationListener as PUGXAuthenticationListener;
use PUGX\MultiUserBundle\Model\UserDiscriminator;

class AuthenticationListener extends PUGXAuthenticationListener
{
    /** @var String */
    protected $userClass;

    /**
     *
     * @param UserDiscriminator $userDiscriminator
     * @param $userClass
     */
    public function __construct(UserDiscriminator $userDiscriminator, $userClass)
    {
        parent::__construct($userDiscriminator);
        $this->userClass = $userClass;
    }

    protected function discriminate($user)
    {
        if($user instanceof $this->userClass) parent::discriminate($user);
    }
} 
Farshadi73 commented 6 years ago

I have same problem with fos user bundle chain_provider (static users)... I do your advice, but still have problem in check_login route :(


    providers:
        chain_provider:
            chain:
                providers: [in_memory, fos_userbundle]

        in_memory:
            memory:
                users:
                    user:  { name: user, password: user, roles: [ 'ROLE_USER' ] }
                    admin: { name: admin, password: admin, roles: [ 'ROLE_ADMIN' ] }

        fos_userbundle:
            id: fos_user.user_provider.username