PUGX / PUGXMultiUserBundle

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

Switch User _exit does not work anymore. #91

Closed Blackskyliner closed 4 years ago

Blackskyliner commented 9 years ago

With a recent change (https://github.com/symfony/symfony/commit/01ee3f6cdae036caa839375f8854b41118e68ffd) the https://github.com/PUGX/PUGXMultiUserBundle/blob/master/Listener/AuthenticationListener.php#L74 does not get called anymore, because the Userprovider gets called beforehand and throws an exception.

This Authentication Exception in https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Security/UserProvider.php#L64 gets thrown as mentioned above, because the discriminator was not called and thus it will try to check the impersonated user class with the admin (ROLE_SWITCH_USER) user class.

Therefore we either need to check the discrimination within the userprovider (if possible) or find another way to get this check working.

Current workaround is stick to one of the following version constraints < v2.7.2,< v2.6.11,< v2.6.10,< v2.3.31.

If you think it's not this bundles responsibility one may open an appropriate issue in the symfony repository.

leopro commented 9 years ago

@Blackskyliner can you provide a sandbox with a test that shows the problem? I'm not using the bundle since a while. Thanks.

Blackskyliner commented 9 years ago

I created a sandboxed Project: https://github.com/Blackskyliner/pugxmultiuserbundle_issue91 It contains the necessary steps to recreate the problem.

zimzim62000 commented 9 years ago

@Blackskyliner thx

I'am waiting for this fix.

Thx more

leopro commented 9 years ago

Thanks @Blackskyliner I'll have a look as soon I can.

sstok commented 9 years ago

You are switching to a completely separate user-system/provider! The Switch-user functionality is about switching to another user within the same firewall (and thus the same user-provider).

I believe this is possible by user a chain user-provider or something, but you need to make sure the usernames are unique in trough out both systems or else this can't work reliable.

Blackskyliner commented 9 years ago

I configured the bundle as stated in the configuration documentation of this repostory.

As far as I got it (through debugging) the PUGX does rely on the EventListener to Discriminate the user's object class correctly. This is in fact, because the provided UserManager of PUGX does not replace the FOS, espacially not in the case of the refreshUser function, which leads to the error, mentioned in the issue.

The change on SF2 changed the old behavior of calling the SecurityEvent::SwitchUser listener, it will now refresh the user before calling any of them. Thus PUGX does not discriminate the users class correctly, befor the FOS Usermanager function refreshUser gets called, leading to the mentioned error within the FOS Manager.

The usernames in my example ARE unique e.G. "Blackskyliner" and "Administrator". So I den't get your last point correctly.

If your answer was not directed at me, then sorry. But I hope I could help you understanding the issue/problem a bit better.

antoiner90 commented 8 years ago

Hi @Blackskyliner I had the same problem today. I made a quick temporary fix by extending the FosUserBundle UserProvider in my bundle. See : https://gist.github.com/antoiner90/6dc7049eb0e43a16487a

lowwa132 commented 8 years ago

Same problem here.

brunonic commented 8 years ago

Any news ?

Blackskyliner commented 7 years ago

@antoiner90 this does not seem to fix the problem for my case, excerpt of my log:

[2017-02-14 12:25:30] security.INFO: Authentication exception occurred; redirecting to authentication entry point (Expected an instance of X, but got "Y".) [] []

EDIT: Never mind - it works, I just used an other UserProvider and had to switch the security configuration accordingly.

Blackskyliner commented 7 years ago

@leopro I guess there will be no official "fix" for this as this bundle just assumes up-to-date stuff and I guess the problem is already handled otherwise on master?

It could also be an documentation fix like "If you want to use impersonation you have to define your own UserProvider handling discrimination" - or something like that?