From 612e4faafe7b242fafcb29c9a7bd9a4388b4a3ee Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 30 Mar 2022 17:33:51 +0200 Subject: [PATCH 1/5] Implement PasswordAuthenticatedUserInterface when appropriate --- Model/User.php | 7 ++++++- Model/UserInterface.php | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Model/User.php b/Model/User.php index 2e8d8049c..c174d58f2 100644 --- a/Model/User.php +++ b/Model/User.php @@ -199,6 +199,11 @@ public function getId() return $this->id; } + public function getUserIdentifier(): string + { + return $this->username; + } + /** * {@inheritdoc} */ @@ -242,7 +247,7 @@ public function getEmailCanonical() /** * {@inheritdoc} */ - public function getPassword() + public function getPassword(): ?string { return $this->password; } diff --git a/Model/UserInterface.php b/Model/UserInterface.php index 48456d805..be19915f0 100644 --- a/Model/UserInterface.php +++ b/Model/UserInterface.php @@ -11,8 +11,25 @@ namespace FOS\UserBundle\Model; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface as BaseUserInterface; +if (interface_exists(PasswordAuthenticatedUserInterface::class)) { + /** + * @internal Only for back compatibility. Remove / merge when dropping support for Symfony 4 + */ + interface CompatUserInterface extends PasswordAuthenticatedUserInterface, BaseUserInterface + { + } +} else { + /** + * @internal Only for back compatibility. Remove / merge when dropping support for Symfony 4 + */ + interface CompatUserInterface extends BaseUserInterface + { + } +} + /** * Implementations of that interface must be serializable. The mechanism * being used to support serialization is up for the implementation. @@ -23,7 +40,7 @@ * @author Johannes M. Schmitt * @author Julian Finkler */ -interface UserInterface extends BaseUserInterface +interface UserInterface extends CompatUserInterface { public const ROLE_DEFAULT = 'ROLE_USER'; From 13cf3b1b7e19854a8c23f351721ef8ea8157f3ef Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 30 Mar 2022 17:35:20 +0200 Subject: [PATCH 2/5] Fix the testsuite --- Tests/Mailer/MailerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Mailer/MailerTest.php b/Tests/Mailer/MailerTest.php index 4dadbe0a4..8b23437c6 100644 --- a/Tests/Mailer/MailerTest.php +++ b/Tests/Mailer/MailerTest.php @@ -18,7 +18,7 @@ class MailerTest extends TestCase { - protected function setUp() + protected function setUp(): void { // skip test for Symfony > 5 if (!interface_exists('Symfony\Bundle\FrameworkBundle\Templating\EngineInterface')) { From 84dedcb4f7448bd716e5c0d07d6c4aba90fa280e Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 30 Mar 2022 17:35:38 +0200 Subject: [PATCH 3/5] Add support for the password-hasher component --- .../Compiler/ConfigurePasswordHasherPass.php | 39 +++++++ FOSUserBundle.php | 2 + Resources/config/util.xml | 4 +- Tests/Util/HashingPasswordUpdaterTest.php | 106 ++++++++++++++++++ Tests/Util/PasswordUpdaterTest.php | 13 ++- Util/HashingPasswordUpdater.php | 53 +++++++++ Util/PasswordUpdater.php | 2 +- 7 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 DependencyInjection/Compiler/ConfigurePasswordHasherPass.php create mode 100644 Tests/Util/HashingPasswordUpdaterTest.php create mode 100644 Util/HashingPasswordUpdater.php diff --git a/DependencyInjection/Compiler/ConfigurePasswordHasherPass.php b/DependencyInjection/Compiler/ConfigurePasswordHasherPass.php new file mode 100644 index 000000000..4d83b76d5 --- /dev/null +++ b/DependencyInjection/Compiler/ConfigurePasswordHasherPass.php @@ -0,0 +1,39 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\UserBundle\DependencyInjection\Compiler; + +use FOS\UserBundle\Util\PasswordUpdater; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; + +/** + * @internal + */ +final class ConfigurePasswordHasherPass implements CompilerPassInterface +{ + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + if ($container->has('security.password_hasher_factory')) { + return; + } + + // If we don't have the new service for password-hasher, use the old implementation based on the EncoderFactoryInterface + $def = $container->getDefinition('fos_user.util.password_updater'); + + $def->setClass(PasswordUpdater::class); + $def->setArgument(0, new Reference('security.encoder_factory')); + } +} diff --git a/FOSUserBundle.php b/FOSUserBundle.php index 56f387129..9bbeb0892 100644 --- a/FOSUserBundle.php +++ b/FOSUserBundle.php @@ -16,6 +16,7 @@ use Doctrine\Bundle\MongoDBBundle\DependencyInjection\Compiler\DoctrineMongoDBMappingsPass; use FOS\UserBundle\DependencyInjection\Compiler\CheckForMailerPass; use FOS\UserBundle\DependencyInjection\Compiler\CheckForSessionPass; +use FOS\UserBundle\DependencyInjection\Compiler\ConfigurePasswordHasherPass; use FOS\UserBundle\DependencyInjection\Compiler\InjectRememberMeServicesPass; use FOS\UserBundle\DependencyInjection\Compiler\InjectUserCheckerPass; use FOS\UserBundle\DependencyInjection\Compiler\ValidationPass; @@ -33,6 +34,7 @@ class FOSUserBundle extends Bundle public function build(ContainerBuilder $container) { parent::build($container); + $container->addCompilerPass(new ConfigurePasswordHasherPass()); $container->addCompilerPass(new ValidationPass()); $container->addCompilerPass(new InjectUserCheckerPass()); $container->addCompilerPass(new InjectRememberMeServicesPass()); diff --git a/Resources/config/util.xml b/Resources/config/util.xml index 07f4686c7..585a28b43 100644 --- a/Resources/config/util.xml +++ b/Resources/config/util.xml @@ -18,8 +18,8 @@ - - + + diff --git a/Tests/Util/HashingPasswordUpdaterTest.php b/Tests/Util/HashingPasswordUpdaterTest.php new file mode 100644 index 000000000..66d722443 --- /dev/null +++ b/Tests/Util/HashingPasswordUpdaterTest.php @@ -0,0 +1,106 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\UserBundle\Tests\Util; + +use FOS\UserBundle\Tests\TestUser; +use FOS\UserBundle\Util\HashingPasswordUpdater; +use PHPUnit\Framework\TestCase; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; +use Symfony\Component\PasswordHasher\LegacyPasswordHasherInterface; +use Symfony\Component\PasswordHasher\PasswordHasherInterface; + +class HashingPasswordUpdaterTest extends TestCase +{ + /** + * @var HashingPasswordUpdater + */ + private $updater; + private $passwordHasherFactory; + + protected function setUp(): void + { + if (!interface_exists(PasswordHasherFactoryInterface::class)) { + self::markTestSkipped('This test requires having the password-hasher component.'); + } + + $this->passwordHasherFactory = $this->getMockBuilder(PasswordHasherFactoryInterface::class)->getMock(); + + $this->updater = new HashingPasswordUpdater($this->passwordHasherFactory); + } + + public function testUpdatePassword() + { + $hasher = $this->getMockBuilder(PasswordHasherInterface::class)->getMock(); + $user = new TestUser(); + $user->setPlainPassword('password'); + + $this->passwordHasherFactory->expects($this->once()) + ->method('getPasswordHasher') + ->with($user) + ->will($this->returnValue($hasher)); + + $hasher->expects($this->once()) + ->method('hash') + ->with('password', $this->identicalTo(null)) + ->will($this->returnValue('hashedPassword')); + + $this->updater->hashPassword($user); + $this->assertSame('hashedPassword', $user->getPassword(), '->updatePassword() sets hashed password'); + $this->assertNull($user->getSalt()); + $this->assertNull($user->getPlainPassword(), '->updatePassword() erases credentials'); + } + + public function testUpdatePasswordWithLegacyHasher() + { + $hasher = $this->getMockBuilder(LegacyPasswordHasherInterface::class)->getMock(); + $user = new TestUser(); + $user->setPlainPassword('password'); + $user->setSalt('old_salt'); + + $this->passwordHasherFactory->expects($this->once()) + ->method('getPasswordHasher') + ->with($user) + ->will($this->returnValue($hasher)); + + $hasher->expects($this->once()) + ->method('hash') + ->with('password', $this->isType('string')) + ->will($this->returnValue('hashedPassword')); + + $this->updater->hashPassword($user); + $this->assertSame('hashedPassword', $user->getPassword(), '->updatePassword() sets hashed password'); + $this->assertNotNull($user->getSalt()); + $this->assertNull($user->getPlainPassword(), '->updatePassword() erases credentials'); + } + + public function testDoesNotUpdateWithEmptyPlainPassword() + { + $user = new TestUser(); + $user->setPassword('hash'); + + $user->setPlainPassword(''); + + $this->updater->hashPassword($user); + $this->assertSame('hash', $user->getPassword()); + } + + public function testDoesNotUpdateWithoutPlainPassword() + { + $user = new TestUser(); + $user->setPassword('hash'); + + $user->setPlainPassword(null); + + $this->updater->hashPassword($user); + $this->assertSame('hash', $user->getPassword()); + } +} diff --git a/Tests/Util/PasswordUpdaterTest.php b/Tests/Util/PasswordUpdaterTest.php index ccbf8f00b..dc145c0a2 100644 --- a/Tests/Util/PasswordUpdaterTest.php +++ b/Tests/Util/PasswordUpdaterTest.php @@ -81,7 +81,7 @@ public function testUpdatePasswordWithBCrypt() $this->assertNull($user->getPlainPassword(), '->updatePassword() erases credentials'); } - public function testDoesNotUpdateWithoutPlainPassword() + public function testDoesNotUpdateWithEmptyPlainPassword() { $user = new TestUser(); $user->setPassword('hash'); @@ -92,6 +92,17 @@ public function testDoesNotUpdateWithoutPlainPassword() $this->assertSame('hash', $user->getPassword()); } + public function testDoesNotUpdateWithoutPlainPassword() + { + $user = new TestUser(); + $user->setPassword('hash'); + + $user->setPlainPassword(null); + + $this->updater->hashPassword($user); + $this->assertSame('hash', $user->getPassword()); + } + private function getMockPasswordEncoder() { return $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface')->getMock(); diff --git a/Util/HashingPasswordUpdater.php b/Util/HashingPasswordUpdater.php new file mode 100644 index 000000000..6dcde7207 --- /dev/null +++ b/Util/HashingPasswordUpdater.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\UserBundle\Util; + +use FOS\UserBundle\Model\UserInterface; +use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; +use Symfony\Component\PasswordHasher\LegacyPasswordHasherInterface; + +/** + * Class updating the hashed password in the user when there is a new password. + * + * @author Christophe Coevoet + */ +class HashingPasswordUpdater implements PasswordUpdaterInterface +{ + private $passwordHasherFactory; + + public function __construct(PasswordHasherFactoryInterface $passwordHasherFactory) + { + $this->passwordHasherFactory = $passwordHasherFactory; + } + + public function hashPassword(UserInterface $user) + { + $plainPassword = $user->getPlainPassword(); + + if (null === $plainPassword || '' === $plainPassword) { + return; + } + + $hasher = $this->passwordHasherFactory->getPasswordHasher($user); + + if (!$hasher instanceof LegacyPasswordHasherInterface) { + $user->setSalt(null); + } else { + $salt = rtrim(str_replace('+', '.', base64_encode(random_bytes(32))), '='); + $user->setSalt($salt); + } + + $hashedPassword = $hasher->hash($plainPassword, $user->getSalt()); + $user->setPassword($hashedPassword); + $user->eraseCredentials(); + } +} diff --git a/Util/PasswordUpdater.php b/Util/PasswordUpdater.php index 460ca5feb..f07ee27fb 100644 --- a/Util/PasswordUpdater.php +++ b/Util/PasswordUpdater.php @@ -34,7 +34,7 @@ public function hashPassword(UserInterface $user) { $plainPassword = $user->getPlainPassword(); - if (0 === strlen($plainPassword)) { + if (null === $plainPassword || '' === $plainPassword) { return; } From e9c8a3af81758d85d00631a2457482b54dbaa734 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 30 Mar 2022 17:36:03 +0200 Subject: [PATCH 4/5] Implement the new API for the user provider --- Security/UserProvider.php | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/Security/UserProvider.php b/Security/UserProvider.php index 1e9bd914c..88a36f3ad 100644 --- a/Security/UserProvider.php +++ b/Security/UserProvider.php @@ -15,6 +15,7 @@ use FOS\UserBundle\Model\UserManagerInterface; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\Exception\UserNotFoundException; use Symfony\Component\Security\Core\User\UserInterface as SecurityUserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; @@ -33,6 +34,17 @@ public function __construct(UserManagerInterface $userManager) $this->userManager = $userManager; } + public function loadUserByIdentifier(string $identifier): SecurityUserInterface + { + $user = $this->findUser($identifier); + + if (!$user) { + throw new UserNotFoundException(sprintf('Username "%s" does not exist.', $identifier)); + } + + return $user; + } + /** * {@inheritdoc} */ @@ -41,6 +53,10 @@ public function loadUserByUsername($username) $user = $this->findUser($username); if (!$user) { + if (class_exists(UserNotFoundException::class)) { + throw new UserNotFoundException(sprintf('Username "%s" does not exist.', $username)); + } + throw new UsernameNotFoundException(sprintf('Username "%s" does not exist.', $username)); } @@ -50,7 +66,7 @@ public function loadUserByUsername($username) /** * {@inheritdoc} */ - public function refreshUser(SecurityUserInterface $user) + public function refreshUser(SecurityUserInterface $user): SecurityUserInterface { if (!$user instanceof UserInterface) { throw new UnsupportedUserException(sprintf('Expected an instance of FOS\UserBundle\Model\UserInterface, but got "%s".', get_class($user))); @@ -61,6 +77,10 @@ public function refreshUser(SecurityUserInterface $user) } if (null === $reloadedUser = $this->userManager->findUserBy(['id' => $user->getId()])) { + if (class_exists(UserNotFoundException::class)) { + throw new UserNotFoundException(sprintf('User with ID "%s" could not be reloaded.', $user->getId())); + } + throw new UsernameNotFoundException(sprintf('User with ID "%s" could not be reloaded.', $user->getId())); } @@ -70,7 +90,7 @@ public function refreshUser(SecurityUserInterface $user) /** * {@inheritdoc} */ - public function supportsClass($class) + public function supportsClass($class): bool { $userClass = $this->userManager->getClass(); From a9f9661f777a859f894e9c46e77e3d15935769e1 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 30 Mar 2022 17:36:22 +0200 Subject: [PATCH 5/5] Bump the min PHP version to PHP 7.4 Supporting newer Symfony versions requires supporting variance rules, which are not available in PHP 7.1. --- .github/workflows/ci.yaml | 7 ++----- composer.json | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 753b0a57e..e29daefc5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,16 +9,14 @@ on: jobs: run: - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest strategy: fail-fast: false matrix: php: - - '7.1' - - '7.2' - - '7.3' - '7.4' - '8.0' + - '8.1' symfony-versions: [false] include: - description: 'Symfony 4.*' @@ -48,5 +46,4 @@ jobs: - name: Run PHPUnit tests run: | - export SYMFONY_PHPUNIT_REMOVE_RETURN_TYPEHINT=1 ./vendor/bin/simple-phpunit diff --git a/composer.json b/composer.json index b8c4e82df..4f291e356 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ } ], "require": { - "php": "^7.1.3 || ^8.0", + "php": "^7.4 || ^8.0", "ext-dom": "*", "ext-json": "*", "symfony/config": "^4.4 || ^5.0",