diff --git a/config/packages/_sylius.yaml b/config/packages/_sylius.yaml index 2130e4552ee..89674accaa6 100644 --- a/config/packages/_sylius.yaml +++ b/config/packages/_sylius.yaml @@ -12,6 +12,3 @@ parameters: sylius_shop: product_grid: include_all_descendants: true - -sylius_user: - encoder: argon2i diff --git a/config/packages/security.yaml b/config/packages/security.yaml index e3fb4e3e6cf..4381c070663 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -10,8 +10,7 @@ security: sylius_shop_user_provider: id: sylius.shop_user_provider.email_or_name_based encoders: - Sylius\Component\User\Model\UserInterface: sha512 - argon2i: argon2i + Sylius\Component\User\Model\UserInterface: argon2i firewalls: admin: switch_user: true diff --git a/config/packages/test/security.yaml b/config/packages/test/security.yaml new file mode 100644 index 00000000000..21cc3772599 --- /dev/null +++ b/config/packages/test/security.yaml @@ -0,0 +1,3 @@ +security: + encoders: + sha512: sha512 diff --git a/src/Sylius/Bundle/UserBundle/DependencyInjection/SyliusUserExtension.php b/src/Sylius/Bundle/UserBundle/DependencyInjection/SyliusUserExtension.php index 6a6c101f621..248f950dd04 100644 --- a/src/Sylius/Bundle/UserBundle/DependencyInjection/SyliusUserExtension.php +++ b/src/Sylius/Bundle/UserBundle/DependencyInjection/SyliusUserExtension.php @@ -14,6 +14,7 @@ namespace Sylius\Bundle\UserBundle\DependencyInjection; use Sylius\Bundle\ResourceBundle\DependencyInjection\Extension\AbstractResourceExtension; +use Sylius\Bundle\UserBundle\EventListener\UpdateUserEncoderListener; use Sylius\Bundle\UserBundle\EventListener\UserDeleteListener; use Sylius\Bundle\UserBundle\EventListener\UserLastLoginSubscriber; use Sylius\Bundle\UserBundle\EventListener\UserReloaderListener; @@ -32,6 +33,7 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\Security\Http\SecurityEvents; final class SyliusUserExtension extends AbstractResourceExtension { @@ -50,7 +52,7 @@ public function load(array $config, ContainerBuilder $container): void $loader->load('services.xml'); $this->createServices($config['resources'], $container); - $this->createFactories($config['encoder'], $config['resources'], $container); + $this->loadEncodersAwareServices($config['encoder'], $config['resources'], $container); } private function resolveResources(array $resources, ContainerBuilder $container): array @@ -82,7 +84,7 @@ private function createServices(array $resources, ContainerBuilder $container): } } - private function createFactories(?string $globalEncoder, array $resources, ContainerBuilder $container): void + private function loadEncodersAwareServices(?string $globalEncoder, array $resources, ContainerBuilder $container): void { foreach ($resources as $userType => $config) { $encoder = $config['user']['encoder'] ?? $globalEncoder; @@ -91,18 +93,8 @@ private function createFactories(?string $globalEncoder, array $resources, Conta continue; } - $factoryServiceId = sprintf('sylius.factory.%s_user', $userType); - - $factoryDefinition = new Definition( - UserWithEncoderFactory::class, - [ - $container->getDefinition($factoryServiceId), - $encoder, - ] - ); - $factoryDefinition->setPublic(true); - - $container->setDefinition($factoryServiceId, $factoryDefinition); + $this->overwriteResourceFactoryWithEncoderAwareFactory($container, $userType, $encoder); + $this->registerUpdateUserEncoderListener($container, $userType, $encoder, $config); } } @@ -252,4 +244,37 @@ private function createProviders(string $userType, string $userModel, ContainerB $emailOrNameBasedProviderDefinition->setClass(UsernameOrEmailProvider::class); $container->setDefinition($providerEmailOrNameBasedServiceId, $emailOrNameBasedProviderDefinition); } + + private function overwriteResourceFactoryWithEncoderAwareFactory(ContainerBuilder $container, string $userType, string $encoder): void + { + $factoryServiceId = sprintf('sylius.factory.%s_user', $userType); + + $factoryDefinition = new Definition( + UserWithEncoderFactory::class, + [ + $container->getDefinition($factoryServiceId), + $encoder, + ] + ); + $factoryDefinition->setPublic(true); + + $container->setDefinition($factoryServiceId, $factoryDefinition); + } + + private function registerUpdateUserEncoderListener(ContainerBuilder $container, string $userType, string $encoder, array $resourceConfig): void + { + $updateUserEncoderListenerDefinition = new Definition(UpdateUserEncoderListener::class, [ + new Reference(sprintf('sylius.manager.%s_user', $userType)), + $encoder, + $resourceConfig['user']['classes']['model'], + $resourceConfig['user']['classes']['interface'], + '_password', + ]); + $updateUserEncoderListenerDefinition->addTag('kernel.event_listener', ['event' => SecurityEvents::INTERACTIVE_LOGIN]); + + $container->setDefinition( + sprintf('sylius.%s_user.listener.update_user_encoder', $userType), + $updateUserEncoderListenerDefinition + ); + } } diff --git a/src/Sylius/Bundle/UserBundle/EventListener/UpdateUserEncoderListener.php b/src/Sylius/Bundle/UserBundle/EventListener/UpdateUserEncoderListener.php new file mode 100644 index 00000000000..c228a3d6a0f --- /dev/null +++ b/src/Sylius/Bundle/UserBundle/EventListener/UpdateUserEncoderListener.php @@ -0,0 +1,80 @@ +objectManager = $objectManager; + $this->recommendedEncoderName = $recommendedEncoderName; + $this->className = $className; + $this->interfaceName = $interfaceName; + $this->passwordParameter = $passwordParameter; + } + + public function onSecurityInteractiveLogin(InteractiveLoginEvent $event): void + { + $user = $event->getAuthenticationToken()->getUser(); + + if (!$user instanceof UserInterface) { + return; + } + + if (!$user instanceof $this->className || !$user instanceof $this->interfaceName) { + return; + } + + if ($user->getEncoderName() === $this->recommendedEncoderName) { + return; + } + + $request = $event->getRequest(); + + $plainPassword = $request->request->get($this->passwordParameter); + if (null === $plainPassword || '' === $plainPassword) { + return; + } + + $user->setEncoderName($this->recommendedEncoderName); + $user->setPlainPassword($plainPassword); + + $this->objectManager->persist($user); + $this->objectManager->flush(); + } +} diff --git a/src/Sylius/Bundle/UserBundle/Resources/config/app/config.yml b/src/Sylius/Bundle/UserBundle/Resources/config/app/config.yml index 9fbeca50bcf..16437727532 100644 --- a/src/Sylius/Bundle/UserBundle/Resources/config/app/config.yml +++ b/src/Sylius/Bundle/UserBundle/Resources/config/app/config.yml @@ -1,6 +1,13 @@ # This file is part of the Sylius package. # (c) Paweł Jędrzejewski +security: + encoders: + argon2i: argon2i + +sylius_user: + encoder: argon2i + jms_serializer: metadata: directories: diff --git a/src/Sylius/Bundle/UserBundle/Tests/DependencyInjection/SyliusUserExtensionTest.php b/src/Sylius/Bundle/UserBundle/Tests/DependencyInjection/SyliusUserExtensionTest.php index 41e312ad8e7..5d520998b0b 100644 --- a/src/Sylius/Bundle/UserBundle/Tests/DependencyInjection/SyliusUserExtensionTest.php +++ b/src/Sylius/Bundle/UserBundle/Tests/DependencyInjection/SyliusUserExtensionTest.php @@ -7,8 +7,11 @@ use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; use PHPUnit\Framework\Assert; use Sylius\Bundle\UserBundle\DependencyInjection\SyliusUserExtension; +use Sylius\Bundle\UserBundle\EventListener\UpdateUserEncoderListener; use Sylius\Bundle\UserBundle\Factory\UserWithEncoderFactory; use Sylius\Component\Resource\Factory\Factory; +use Sylius\Component\User\Model\User; +use Sylius\Component\User\Model\UserInterface; final class SyliusUserExtensionTest extends AbstractExtensionTestCase { @@ -85,6 +88,42 @@ public function it_decorates_user_factory_using_the_most_specific_encoder_config Assert::assertSame('evenmorecustomencoder', $factoryDefinition->getArgument(1)); } + /** @test */ + public function it_creates_a_update_user_encoder_listener_for_each_user_type(): void + { + $this->load([ + 'encoder' => 'customencoder', + 'resources' => [ + 'admin' => [ + 'user' => [ + 'encoder' => 'evenmorecustomencoder', + 'classes' => [ + 'model' => 'AdminUserClass', + 'interface' => 'AdminUserInterface', + ], + ], + ], + 'shop' => [], + ], + ]); + + $adminUserListenerDefinition = $this->container->getDefinition('sylius.admin_user.listener.update_user_encoder'); + + Assert::assertSame(UpdateUserEncoderListener::class, $adminUserListenerDefinition->getClass()); + Assert::assertSame('evenmorecustomencoder', $adminUserListenerDefinition->getArgument(1)); + Assert::assertSame('AdminUserClass', $adminUserListenerDefinition->getArgument(2)); + Assert::assertSame('AdminUserInterface', $adminUserListenerDefinition->getArgument(3)); + Assert::assertSame('_password', $adminUserListenerDefinition->getArgument(4)); + + $shopUserListenerDefinition = $this->container->getDefinition('sylius.shop_user.listener.update_user_encoder'); + + Assert::assertSame(UpdateUserEncoderListener::class, $shopUserListenerDefinition->getClass()); + Assert::assertSame('customencoder', $shopUserListenerDefinition->getArgument(1)); + Assert::assertSame(User::class, $shopUserListenerDefinition->getArgument(2)); + Assert::assertSame(UserInterface::class, $shopUserListenerDefinition->getArgument(3)); + Assert::assertSame('_password', $shopUserListenerDefinition->getArgument(4)); + } + protected function getContainerExtensions(): iterable { return [ diff --git a/src/Sylius/Bundle/UserBundle/spec/EventListener/UpdateUserEncoderListenerSpec.php b/src/Sylius/Bundle/UserBundle/spec/EventListener/UpdateUserEncoderListenerSpec.php new file mode 100644 index 00000000000..77ddf540cc7 --- /dev/null +++ b/src/Sylius/Bundle/UserBundle/spec/EventListener/UpdateUserEncoderListenerSpec.php @@ -0,0 +1,201 @@ +beConstructedWith($objectManager, 'newalgo', FixtureUser::class, FixtureUserInterface::class, '_password'); + } + + function it_does_nothing_if_user_does_not_implement_user_interface( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token + ): void { + $event->getAuthenticationToken()->willReturn($token); + + $user = new \stdClass(); + + $token->getUser()->willReturn($user); + + $objectManager->persist($user)->shouldNotBeCalled(); + $objectManager->flush()->shouldNotBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_does_nothing_if_user_does_not_implement_specified_class_or_interface( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + User $user + ): void { + $event->getAuthenticationToken()->willReturn($token); + + $token->getUser()->willReturn($user); + + $objectManager->persist($user)->shouldNotBeCalled(); + $objectManager->flush()->shouldNotBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_does_nothing_if_user_uses_the_recommended_encoder( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + FixtureUser $user + ): void { + $event->getAuthenticationToken()->willReturn($token); + + $token->getUser()->willReturn($user); + + $user->getEncoderName()->willReturn('newalgo'); + + $user->setEncoderName(Argument::any())->shouldNotBeCalled(); + $user->setPlainPassword(Argument::any())->shouldNotBeCalled(); + + $objectManager->persist($user)->shouldNotBeCalled(); + $objectManager->flush()->shouldNotBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_does_nothing_if_plain_password_could_not_be_resolved( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + FixtureUser $user + ): void { + $request = new Request(); + + $event->getAuthenticationToken()->willReturn($token); + $event->getRequest()->willReturn($request); + + $token->getUser()->willReturn($user); + + $user->getEncoderName()->willReturn('oldalgo'); + + $user->setEncoderName(Argument::any())->shouldNotBeCalled(); + $user->setPlainPassword(Argument::any())->shouldNotBeCalled(); + + $objectManager->persist($user)->shouldNotBeCalled(); + $objectManager->flush()->shouldNotBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_does_nothing_if_resolved_plain_password_is_null( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + FixtureUser $user + ): void { + $request = new Request(); + $request->request->set('_password', null); + + $event->getAuthenticationToken()->willReturn($token); + $event->getRequest()->willReturn($request); + + $token->getUser()->willReturn($user); + + $user->getEncoderName()->willReturn('oldalgo'); + + $user->setEncoderName(Argument::any())->shouldNotBeCalled(); + $user->setPlainPassword(Argument::any())->shouldNotBeCalled(); + + $objectManager->persist($user)->shouldNotBeCalled(); + $objectManager->flush()->shouldNotBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_does_nothing_if_resolved_plain_password_is_empty( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + FixtureUser $user + ): void { + $request = new Request(); + $request->request->set('_password', ''); + + $event->getAuthenticationToken()->willReturn($token); + $event->getRequest()->willReturn($request); + + $token->getUser()->willReturn($user); + + $user->getEncoderName()->willReturn('oldalgo'); + + $user->setEncoderName(Argument::any())->shouldNotBeCalled(); + $user->setPlainPassword(Argument::any())->shouldNotBeCalled(); + + $objectManager->persist($user)->shouldNotBeCalled(); + $objectManager->flush()->shouldNotBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_updates_the_encoder_and_plain_password_if_using_old_encoder_and_plain_password_could_be_resolved( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + FixtureUser $user + ): void { + $request = new Request(); + $request->request->set('_password', 'plainpassword'); + + $event->getAuthenticationToken()->willReturn($token); + $event->getRequest()->willReturn($request); + + $token->getUser()->willReturn($user); + + $user->getEncoderName()->willReturn('oldalgo'); + + $user->setEncoderName('newalgo')->shouldBeCalled(); + $user->setPlainPassword('plainpassword')->shouldBeCalled(); + + $objectManager->persist($user)->shouldBeCalled(); + $objectManager->flush()->shouldBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } + + function it_updates_the_encoder_and_plain_password_if_using_default_null_encoder_and_plain_password_could_be_resolved( + ObjectManager $objectManager, + InteractiveLoginEvent $event, + TokenInterface $token, + FixtureUser $user + ): void { + $request = new Request(); + $request->request->set('_password', 'plainpassword'); + + $event->getAuthenticationToken()->willReturn($token); + $event->getRequest()->willReturn($request); + + $token->getUser()->willReturn($user); + + $user->getEncoderName()->willReturn(null); + + $user->setEncoderName('newalgo')->shouldBeCalled(); + $user->setPlainPassword('plainpassword')->shouldBeCalled(); + + $objectManager->persist($user)->shouldBeCalled(); + $objectManager->flush()->shouldBeCalled(); + + $this->onSecurityInteractiveLogin($event); + } +} diff --git a/src/Sylius/Bundle/UserBundle/spec/Fixture/FixtureUser.php b/src/Sylius/Bundle/UserBundle/spec/Fixture/FixtureUser.php new file mode 100644 index 00000000000..0e16ea79a15 --- /dev/null +++ b/src/Sylius/Bundle/UserBundle/spec/Fixture/FixtureUser.php @@ -0,0 +1,20 @@ +locked, $this->enabled, $this->id, + $this->encoderName, ]); } @@ -535,7 +536,8 @@ public function unserialize($serialized): void $this->username, $this->locked, $this->enabled, - $this->id + $this->id, + $this->encoderName, ] = $data; } diff --git a/tests/Functional/UpdatingUserPasswordEncoderTest.php b/tests/Functional/UpdatingUserPasswordEncoderTest.php new file mode 100644 index 00000000000..d0bbf72c6d0 --- /dev/null +++ b/tests/Functional/UpdatingUserPasswordEncoderTest.php @@ -0,0 +1,104 @@ +client = static::createClient(); + $this->client->followRedirects(true); + + /** @var LoaderInterface $fixtureLoader */ + $fixtureLoader = $this->client->getContainer()->get('fidry_alice_data_fixtures.loader.doctrine'); + + $fixtureLoader->load( + [ + __DIR__ . '/../DataFixtures/ORM/resources/channels.yml', + __DIR__ . '/../DataFixtures/ORM/resources/customers.yml', + __DIR__ . '/../DataFixtures/ORM/resources/admin_users.yml', + ], + [], + [], + PurgeMode::createDeleteMode() + ); + } + + /** @test */ + public function it_updates_the_encoder_when_the_shop_user_logs_in(): void + { + /** @var UserRepositoryInterface $shopUserRepository */ + $shopUserRepository = $this->client->getContainer()->get('sylius.repository.shop_user'); + + /** @var ObjectManager $shopUserManager */ + $shopUserManager = $this->client->getContainer()->get('sylius.manager.shop_user'); + + $shopUser = $shopUserRepository->findOneByEmail('Oliver@doe.com'); + $shopUser->setPlainPassword('testpassword'); + $shopUser->setEncoderName('sha512'); + + $shopUserManager->persist($shopUser); + $shopUserManager->flush(); + + $this->client->request('GET', '/en_US/login'); + + $this->submitForm('Login', [ + '_username' => 'Oliver@doe.com', + '_password' => 'testpassword' + ]); + + Assert::assertSame(200, $this->client->getResponse()->getStatusCode()); + Assert::assertSame('/en_US/', parse_url($this->client->getCrawler()->getUri(), \PHP_URL_PATH)); + Assert::assertSame('argon2i', $shopUserRepository->findOneByEmail('Oliver@doe.com')->getEncoderName()); + } + + /** @test */ + public function it_updates_the_encoder_when_the_admin_user_logs_in(): void + { + /** @var UserRepositoryInterface $adminUserRepository */ + $adminUserRepository = $this->client->getContainer()->get('sylius.repository.admin_user'); + + /** @var ObjectManager $adminUserManager */ + $adminUserManager = $this->client->getContainer()->get('sylius.manager.admin_user'); + + $adminUser = $adminUserRepository->findOneByEmail('user@example.com'); + $adminUser->setPlainPassword('testpassword'); + $adminUser->setEncoderName('sha512'); + + $adminUserManager->persist($adminUser); + $adminUserManager->flush(); + + $this->client->request('GET', '/admin/login'); + + $this->submitForm('Login', [ + '_username' => 'user@example.com', + '_password' => 'testpassword' + ]); + + Assert::assertSame(200, $this->client->getResponse()->getStatusCode()); + Assert::assertSame('/admin/', parse_url($this->client->getCrawler()->getUri(), \PHP_URL_PATH)); + Assert::assertSame('argon2i', $adminUserRepository->findOneByEmail('user@example.com')->getEncoderName()); + } + + private function submitForm(string $button, array $fieldValues = []): void + { + $buttonNode = $this->client->getCrawler()->selectButton($button); + + $form = $buttonNode->form($fieldValues); + + $this->client->submit($form); + } +}