From 4d65f302bf0928fe2140f565091949ce986c4dc3 Mon Sep 17 00:00:00 2001 From: Maxime Steinhausser Date: Mon, 18 Sep 2023 17:40:31 +0200 Subject: [PATCH] [SecurityBundle][PasswordHasher] Fix password migration with custom hasher service with security bundle config --- .../DependencyInjection/SecurityExtension.php | 5 +- .../SecurityExtensionTest.php | 27 ++++++++++ .../Hasher/PasswordHasherFactory.php | 50 ++++++++++++------- .../Hasher/PasswordHasherFactoryTest.php | 33 ++++++++++++ 4 files changed, 96 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index d64b2c38ac7e7..c165024b68d0d 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -848,7 +848,10 @@ private function createHasher(array $config) { // a custom hasher service if (isset($config['id'])) { - return new Reference($config['id']); + return $config['migrate_from'] ?? false ? [ + 'instance' => new Reference($config['id']), + 'migrate_from' => $config['migrate_from'], + ] : new Reference($config['id']); } if ($config['migrate_from'] ?? false) { diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php index 71ca327ca40c6..eef68e4c3de46 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php @@ -881,6 +881,33 @@ public function testLegacyAuthorizationManagerSignature() $this->assertEquals('%security.access.always_authenticate_before_granting%', (string) $args[3]); } + public function testCustomHasherWithMigrateFrom() + { + $container = $this->getRawContainer(); + + $container->loadFromExtension('security', [ + 'enable_authenticator_manager' => true, + 'password_hashers' => [ + 'legacy' => 'md5', + 'App\User' => [ + 'id' => 'App\Security\CustomHasher', + 'migrate_from' => 'legacy', + ], + ], + 'firewalls' => ['main' => ['http_basic' => true]], + ]); + + $container->compile(); + + $hashersMap = $container->getDefinition('security.password_hasher_factory')->getArgument(0); + + $this->assertArrayHasKey('App\User', $hashersMap); + $this->assertEquals($hashersMap['App\User'], [ + 'instance' => new Reference('App\Security\CustomHasher'), + 'migrate_from' => ['legacy'], + ]); + } + protected function getRawContainer() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactory.php b/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactory.php index dd7e015c1ecd2..116301c04a441 100644 --- a/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactory.php +++ b/src/Symfony/Component/PasswordHasher/Hasher/PasswordHasherFactory.php @@ -71,6 +71,14 @@ public function getPasswordHasher($user): PasswordHasherInterface */ private function createHasher(array $config, bool $isExtra = false): PasswordHasherInterface { + if (isset($config['instance'])) { + if (!isset($config['migrate_from'])) { + return $config['instance']; + } + + $config = $this->getMigratingPasswordConfig($config); + } + if (isset($config['algorithm'])) { $rawConfig = $config; $config = $this->getHasherConfigFromAlgorithm($config); @@ -142,24 +150,8 @@ private function getHasherConfigFromAlgorithm(array $config): array ]; } - if ($frompasswordHashers = ($config['migrate_from'] ?? false)) { - unset($config['migrate_from']); - $hasherChain = [$this->createHasher($config, true)]; - - foreach ($frompasswordHashers as $name) { - if (isset($this->passwordHashers[$name])) { - $hasher = $this->createHasherUsingAdapter($name); - } else { - $hasher = $this->createHasher(['algorithm' => $name], true); - } - - $hasherChain[] = $hasher; - } - - return [ - 'class' => MigratingPasswordHasher::class, - 'arguments' => $hasherChain, - ]; + if ($config['migrate_from'] ?? false) { + return $this->getMigratingPasswordConfig($config); } switch ($config['algorithm']) { @@ -239,4 +231,26 @@ private function getHasherConfigFromAlgorithm(array $config): array ], ]; } + + private function getMigratingPasswordConfig(array $config): array + { + $frompasswordHashers = $config['migrate_from']; + unset($config['migrate_from']); + $hasherChain = [$this->createHasher($config, true)]; + + foreach ($frompasswordHashers as $name) { + if ($this->passwordHashers[$name] ?? false) { + $hasher = $this->createHasherUsingAdapter($name); + } else { + $hasher = $this->createHasher(['algorithm' => $name], true); + } + + $hasherChain[] = $hasher; + } + + return [ + 'class' => MigratingPasswordHasher::class, + 'arguments' => $hasherChain, + ]; + } } diff --git a/src/Symfony/Component/PasswordHasher/Tests/Hasher/PasswordHasherFactoryTest.php b/src/Symfony/Component/PasswordHasher/Tests/Hasher/PasswordHasherFactoryTest.php index 1f24a0d3cace2..1b97eedcdac48 100644 --- a/src/Symfony/Component/PasswordHasher/Tests/Hasher/PasswordHasherFactoryTest.php +++ b/src/Symfony/Component/PasswordHasher/Tests/Hasher/PasswordHasherFactoryTest.php @@ -49,6 +49,17 @@ public function testGetHasherWithService() $this->assertEquals($expectedHasher->hash('foo', ''), $hasher->hash('foo', '')); } + public function testGetHasherWithInstance() + { + $factory = new PasswordHasherFactory([ + PasswordAuthenticatedUserInterface::class => ['instance' => new MessageDigestPasswordHasher('sha1')], + ]); + + $hasher = $factory->getPasswordHasher($this->createMock(PasswordAuthenticatedUserInterface::class)); + $expectedHasher = new MessageDigestPasswordHasher('sha1'); + $this->assertEquals($expectedHasher->hash('foo', ''), $hasher->hash('foo', '')); + } + public function testGetHasherWithClassName() { $factory = new PasswordHasherFactory([ @@ -163,6 +174,28 @@ public function testMigrateFrom() $this->assertStringStartsWith(\SODIUM_CRYPTO_PWHASH_STRPREFIX, $hasher->hash('foo', null)); } + public function testMigrateFromWithCustomInstance() + { + if (!SodiumPasswordHasher::isSupported()) { + $this->markTestSkipped('Sodium is not available'); + } + + $sodium = new SodiumPasswordHasher(); + + $factory = new PasswordHasherFactory([ + 'digest_hasher' => $digest = new MessageDigestPasswordHasher('sha256'), + SomeUser::class => ['instance' => $sodium, 'migrate_from' => ['bcrypt', 'digest_hasher']], + ]); + + $hasher = $factory->getPasswordHasher(SomeUser::class); + $this->assertInstanceOf(MigratingPasswordHasher::class, $hasher); + + $this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo', null), 'foo', null)); + $this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo', null), 'foo', null)); + $this->assertTrue($hasher->verify($digest->hash('foo', null), 'foo', null)); + $this->assertStringStartsWith(\SODIUM_CRYPTO_PWHASH_STRPREFIX, $hasher->hash('foo', null)); + } + /** * @group legacy */