DaveC49 / SimpleUser-Silex2

This is an mod to JasonGrimes SImpleUser updated to work with SIlex v2.0
BSD 2-Clause "Simplified" License
13 stars 7 forks source link

Wrong code in UserManager #3

Open sintro opened 7 years ago

sintro commented 7 years ago

As I noticed here https://github.com/DaveC49/SimpleUser-Silex2/issues/2#issuecomment-309645777, UserManager.php has wrong code. Silex now has another interface with Security provider, and we can not use app['security']->getToken() anymore like here https://github.com/DaveC49/SimpleUser-Silex2/blob/master/src/SimpleUser/UserManager.php#L741 The correct way now is app['security.token_storage']->getToken() and app['security.token_storage']->setToken() Btw, the next line in this method is also broken, as it tries to call 'getKey' from AnonymousToken class, which does not have one, as it was renamed to getSecret. https://github.com/symfony/security-core/commit/5835d04319b29f20dc3e73167d046ccafcdf9d57 So, the correct code here will be:

    public function loginAsUser(User $user)
    {
        if (null !== ($current_token = $this->app['security.token_storage']->getToken())) {
            $providerKey = method_exists($current_token, 'getProviderKey') ? $current_token->getProviderKey() : $current_token->getSecret();
            $token = new UsernamePasswordToken($user, null, $providerKey);
            $this->app['security.token_storage']->setToken($token);

            $this->app['user'] = $user;
        }
    }

Also, nothing will work and web-profiler will crash after registration, because there is wrong configuration sample in README to connect this UserManager as custom UserProvider. Wrong part: 'users' => $app->share(function($app) { return $app['user.manager']; }) Correct:

'users' => function () use ($app) {
    return $app['user.manager'];
},
DaveC49 commented 7 years ago

Thanks Dmitry, I will incorporate the changes you have suggested and test them out. It may take me a few days as I am in the final throes of publishing a book for my wife and there is a hard deadline on that I can't avoid. Alternatively, if you have the time to patch it yourself, I am happy to add you as a collaborator. I am no longer using SimpleUser in any current projects so I only return to it if an issue comes up.

Best wishes

David

sintro commented 7 years ago

Great to hear that, dont waste your time now on this, I wil prepare PR soon for you (but without tests).

Keirodev commented 7 years ago

Hi there, funny thing, I discover this post after doing exactly same corrections. Think about updating ;)

DaveC49 commented 7 years ago

Hi Dmitry and Keirodev,

The changes that Dmitry pointed out have now been incorporated. Please let me know if you encounter any more problems. My apologies for taking so long to get around to it.

sintro commented 7 years ago

@DaveC49, nice, already wrote comment for your last commit. Also, I've introduced upgrade for custom voter (take a look here https://github.com/DaveC49/SimpleUser-Silex2/blob/silex2.1/src/SimpleUser/EditUserVoter.php), which uses last recomendations for Symfony voter interface. It also needed some changes in its injecting mechanism, which I also made in UserServiceProvider.php. Should work fine, but before merging to master would be great to run some tests (didn't check it).