feature #31843 [Security] add support for opportunistic password migrations (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security] add support for opportunistic password migrations

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #31139
| License       | MIT
| Doc PR        | -
| Maker PR | symfony/maker-bundle#389

With this last piece, we'll provide opportunistic password migrations out of the box.
This finishes the story drafted in #31153, see there for more info.

Commits
-------

2cfc5c7dd6 [Security] add support for opportunistic password migrations

src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php

@@ -14,6 +14,7 @@ namespace Symfony\Bridge\Doctrine\Security\User;
 use Doctrine\Common\Persistence\ManagerRegistry;
 use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 
@@ -25,7 +26,7 @@ use Symfony\Component\Security\Core\User\UserProviderInterface;
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
-class EntityUserProvider implements UserProviderInterface
+class EntityUserProvider implements UserProviderInterface, PasswordUpgraderInterface
 {
     private $registry;
     private $managerName;
@@ -107,6 +108,22 @@ class EntityUserProvider implements UserProviderInterface
         return $class === $this->getClass() || is_subclass_of($class, $this->getClass());
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
+    {
+        $class = $this->getClass();
+        if (!$user instanceof $class) {
+            throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
+        }
+
+        $repository = $this->getRepository();
+        if ($repository instanceof PasswordUpgraderInterface) {
+            $repository->upgradePassword($user, $newEncodedPassword);
+        }
+    }
+
     private function getObjectManager()
     {
         return $this->registry->getManager($this->managerName);

src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php

@@ -16,6 +16,7 @@ use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\Doctrine\Security\User\EntityUserProvider;
 use Symfony\Bridge\Doctrine\Test\DoctrineTestHelper;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\User;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
 
 class EntityUserProviderTest extends TestCase
 {
@@ -175,6 +176,23 @@ class EntityUserProviderTest extends TestCase
         $provider->loadUserByUsername('name');
     }
 
+    public function testPasswordUpgrades()
+    {
+        $user = new User(1, 1, 'user1');
+
+        $repository = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
+        $repository->expects($this->once())
+            ->method('upgradePassword')
+            ->with($user, 'foobar');
+
+        $provider = new EntityUserProvider(
+            $this->getManager($this->getObjectManager($repository)),
+            'Symfony\Bridge\Doctrine\Tests\Fixtures\User'
+        );
+
+        $provider->upgradePassword($user, 'foobar');
+    }
+
     private function getManager($em, $name = null)
     {
         $manager = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')->getMock();

src/Symfony/Bridge/Doctrine/composer.json

@@ -33,7 +33,7 @@
         "symfony/property-access": "^3.4|^4.0|^5.0",
         "symfony/property-info": "^3.4|^4.0|^5.0",
         "symfony/proxy-manager-bridge": "^3.4|^4.0|^5.0",
-        "symfony/security-core": "^3.4|^4.0|^5.0",
+        "symfony/security-core": "^4.4|^5.0",
         "symfony/expression-language": "^3.4|^4.0|^5.0",
         "symfony/validator": "^3.4|^4.0|^5.0",
         "symfony/translation": "^3.4|^4.0|^5.0",
@@ -49,7 +49,8 @@
         "phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0",
         "symfony/dependency-injection": "<3.4",
         "symfony/form": "<4.4",
-        "symfony/messenger": "<4.3"
+        "symfony/messenger": "<4.3",
+        "symfony/security-core": "<4.4"
     },
     "suggest": {
         "symfony/form": "",

src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml

@@ -29,6 +29,7 @@
             <argument /> <!-- User Provider -->
             <argument /> <!-- Provider-shared Key -->
             <argument /> <!-- User Checker -->
+            <argument type="service" id="security.password_encoder" />
         </service>
 
         <service id="security.authentication.listener.guard"

src/Symfony/Component/Ldap/Security/LdapUserProvider.php

@@ -13,10 +13,12 @@ namespace Symfony\Component\Ldap\Security;
 
 use Symfony\Component\Ldap\Entry;
 use Symfony\Component\Ldap\Exception\ConnectionException;
+use Symfony\Component\Ldap\Exception\ExceptionInterface;
 use Symfony\Component\Ldap\LdapInterface;
 use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
 use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 
@@ -27,7 +29,7 @@ use Symfony\Component\Security\Core\User\UserProviderInterface;
  * @author Charles Sarrazin <charles@sarraz.in>
  * @author Robin Chalas <robin.chalas@gmail.com>
  */
-class LdapUserProvider implements UserProviderInterface
+class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterface
 {
     private $ldap;
     private $baseDn;
@@ -109,6 +111,30 @@ class LdapUserProvider implements UserProviderInterface
         return new LdapUser($user->getEntry(), $user->getUsername(), $user->getPassword(), $user->getRoles());
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
+    {
+        if (!$user instanceof LdapUser) {
+            throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
+        }
+
+        if (null === $this->passwordAttribute) {
+            return;
+        }
+
+        try {
+            if ($user->isEqualTo($this->loadUserByUsername($user->getUsername()))) {
+                $user->getEntry()->setAttribute($this->passwordAttribute, [$newEncodedPassword]);
+                $this->ldap->getEntryManager()->update($user->getEntry());
+                $user->setPassword($newEncodedPassword);
+            }
+        } catch (ExceptionInterface $e) {
+            // ignore failed password upgrades
+        }
+    }
+
     /**
      * {@inheritdoc}
      */

src/Symfony/Component/Security/CHANGELOG.md

@@ -7,6 +7,9 @@ CHANGELOG
  * Deprecated class `LdapUserProvider`, use `Symfony\Component\Ldap\Security\LdapUserProvider` instead
  * Added method `needsRehash()` to `PasswordEncoderInterface` and `UserPasswordEncoderInterface`
  * Added `MigratingPasswordEncoder`
+ * Added and implemented `PasswordUpgraderInterface`, for opportunistic password migrations
+ * Added `Guard\PasswordAuthenticatedInterface`, an optional interface
+   for "guard" authenticators that deal with user passwords
 
 4.3.0
 -----

src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php

@@ -16,6 +16,7 @@ use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
 use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
 use Symfony\Component\Security\Core\User\UserCheckerInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
@@ -54,9 +55,15 @@ class DaoAuthenticationProvider extends UserAuthenticationProvider
                 throw new BadCredentialsException('The presented password cannot be empty.');
             }
 
-            if (!$this->encoderFactory->getEncoder($user)->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
+            $encoder = $this->encoderFactory->getEncoder($user);
+
+            if (!$encoder->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
                 throw new BadCredentialsException('The presented password is invalid.');
             }
+
+            if ($this->userProvider instanceof PasswordUpgraderInterface && method_exists($encoder, 'needsRehash') && $encoder->needsRehash($user->getPassword())) {
+                $this->userProvider->upgradePassword($user, $encoder->encodePassword($presentedPassword, $user->getSalt()));
+            }
         }
     }
 

src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php

@@ -15,6 +15,10 @@ use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Core\Authentication\Provider\DaoAuthenticationProvider;
 use Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
+use Symfony\Component\Security\Core\Tests\Encoder\TestPasswordEncoderInterface;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
+use Symfony\Component\Security\Core\User\User;
+use Symfony\Component\Security\Core\User\UserProviderInterface;
 
 class DaoAuthenticationProviderTest extends TestCase
 {
@@ -247,6 +251,44 @@ class DaoAuthenticationProviderTest extends TestCase
         $method->invoke($provider, $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(), $token);
     }
 
+    public function testPasswordUpgrades()
+    {
+        $user = new User('user', 'pwd');
+
+        $encoder = $this->getMockBuilder(TestPasswordEncoderInterface::class)->getMock();
+        $encoder->expects($this->once())
+                ->method('isPasswordValid')
+                ->willReturn(true)
+        ;
+        $encoder->expects($this->once())
+                ->method('encodePassword')
+                ->willReturn('foobar')
+        ;
+        $encoder->expects($this->once())
+                ->method('needsRehash')
+                ->willReturn(true)
+        ;
+
+        $provider = $this->getProvider(null, null, $encoder);
+
+        $userProvider = ((array) $provider)[sprintf("\0%s\0userProvider", DaoAuthenticationProvider::class)];
+        $userProvider->expects($this->once())
+            ->method('upgradePassword')
+            ->with($user, 'foobar')
+        ;
+
+        $method = new \ReflectionMethod($provider, 'checkAuthentication');
+        $method->setAccessible(true);
+
+        $token = $this->getSupportedToken();
+        $token->expects($this->once())
+              ->method('getCredentials')
+              ->willReturn('foo')
+        ;
+
+        $method->invoke($provider, $user, $token);
+    }
+
     protected function getSupportedToken()
     {
         $mock = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Authentication\\Token\\UsernamePasswordToken')->setMethods(['getCredentials', 'getUser', 'getProviderKey'])->disableOriginalConstructor()->getMock();
@@ -261,7 +303,7 @@ class DaoAuthenticationProviderTest extends TestCase
 
     protected function getProvider($user = null, $userChecker = null, $passwordEncoder = null)
     {
-        $userProvider = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserProviderInterface')->getMock();
+        $userProvider = $this->getMockBuilder([UserProviderInterface::class, PasswordUpgraderInterface::class])->getMock();
         if (null !== $user) {
             $userProvider->expects($this->once())
                          ->method('loadUserByUsername')

src/Symfony/Component/Security/Core/Tests/Encoder/MigratingPasswordEncoderTest.php

@@ -14,7 +14,6 @@ namespace Symfony\Component\Security\Core\Tests\Encoder;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Core\Encoder\MigratingPasswordEncoder;
 use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
-use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;
 
 class MigratingPasswordEncoderTest extends TestCase
 {
@@ -66,8 +65,3 @@ class MigratingPasswordEncoderTest extends TestCase
         $this->assertTrue($encoder->isPasswordValid('abc', 'foo', 'salt'));
     }
 }
-
-interface TestPasswordEncoderInterface extends PasswordEncoderInterface
-{
-    public function needsRehash(string $encoded): bool;
-}

src/Symfony/Component/Security/Core/Tests/Encoder/TestPasswordEncoderInterface.php

@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Tests\Encoder;
+
+use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;
+
+interface TestPasswordEncoderInterface extends PasswordEncoderInterface
+{
+    public function needsRehash(string $encoded): bool;
+}

src/Symfony/Component/Security/Core/Tests/User/ChainUserProviderTest.php

@@ -15,6 +15,8 @@ use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
 use Symfony\Component\Security\Core\User\ChainUserProvider;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
+use Symfony\Component\Security\Core\User\User;
 
 class ChainUserProviderTest extends TestCase
 {
@@ -188,6 +190,28 @@ class ChainUserProviderTest extends TestCase
         $this->assertSame($account, $provider->refreshUser($this->getAccount()));
     }
 
+    public function testPasswordUpgrades()
+    {
+        $user = new User('user', 'pwd');
+
+        $provider1 = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
+        $provider1
+            ->expects($this->once())
+            ->method('upgradePassword')
+            ->willThrowException(new UnsupportedUserException('unsupported'))
+        ;
+
+        $provider2 = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
+        $provider2
+            ->expects($this->once())
+            ->method('upgradePassword')
+            ->with($user, 'foobar')
+        ;
+
+        $provider = new ChainUserProvider([$provider1, $provider2]);
+        $provider->upgradePassword($user, 'foobar');
+    }
+
     protected function getAccount()
     {
         return $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();

src/Symfony/Component/Security/Core/User/ChainUserProvider.php

@@ -22,7 +22,7 @@ use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
  *
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
-class ChainUserProvider implements UserProviderInterface
+class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterface
 {
     private $providers;
 
@@ -104,4 +104,20 @@ class ChainUserProvider implements UserProviderInterface
 
         return false;
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
+    {
+        foreach ($this->providers as $provider) {
+            if ($provider instanceof PasswordUpgraderInterface) {
+                try {
+                    $provider->upgradePassword($user, $newEncodedPassword);
+                } catch (UnsupportedUserException $e) {
+                    // ignore: password upgrades are opportunistic
+                }
+            }
+        }
+    }
 }

src/Symfony/Component/Security/Core/User/PasswordUpgraderInterface.php

@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\User;
+
+/**
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+interface PasswordUpgraderInterface
+{
+    /**
+     * Upgrades the encoded password of a user, typically for using a better hash algorithm.
+     *
+     * This method should persist the new password in the user storage and update the $user object accordingly.
+     * Because you don't want your users not being able to log in, this method should be opportunistic:
+     * it's fine if it does nothing or if it fails without throwing any exception.
+     */
+    public function upgradePassword(UserInterface $user, string $newEncodedPassword): void;
+}

src/Symfony/Component/Security/Guard/PasswordAuthenticatedInterface.php

@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Guard;
+
+/**
+ * An optional interface for "guard" authenticators that deal with user passwords.
+ */
+interface PasswordAuthenticatedInterface
+{
+    /**
+     * Returns the clear-text password contained in credentials if any.
+     *
+     * @param mixed The user credentials
+     */
+    public function getPassword($credentials): ?string;
+}

src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php

@@ -13,14 +13,17 @@ namespace Symfony\Component\Security\Guard\Provider;
 
 use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\Exception\AuthenticationExpiredException;
 use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
+use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
 use Symfony\Component\Security\Core\User\UserCheckerInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 use Symfony\Component\Security\Guard\AuthenticatorInterface;
+use Symfony\Component\Security\Guard\PasswordAuthenticatedInterface;
 use Symfony\Component\Security\Guard\Token\GuardTokenInterface;
 use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
 
@@ -39,19 +42,19 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
     private $userProvider;
     private $providerKey;
     private $userChecker;
+    private $passwordEncoder;
 
     /**
      * @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationListener
-     * @param UserProviderInterface             $userProvider        The user provider
      * @param string                            $providerKey         The provider (i.e. firewall) key
-     * @param UserCheckerInterface              $userChecker
      */
-    public function __construct($guardAuthenticators, UserProviderInterface $userProvider, string $providerKey, UserCheckerInterface $userChecker)
+    public function __construct($guardAuthenticators, UserProviderInterface $userProvider, string $providerKey, UserCheckerInterface $userChecker, UserPasswordEncoderInterface $passwordEncoder = null)
     {
         $this->guardAuthenticators = $guardAuthenticators;
         $this->userProvider = $userProvider;
         $this->providerKey = $providerKey;
         $this->userChecker = $userChecker;
+        $this->passwordEncoder = $passwordEncoder;
     }
 
     /**
@@ -113,6 +116,9 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
         if (true !== $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
             throw new BadCredentialsException(sprintf('Authentication failed because %s::checkCredentials() did not return true.', \get_class($guardAuthenticator)));
         }
+        if ($this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordEncoder && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && method_exists($this->passwordEncoder, 'needsRehash') && $this->passwordEncoder->needsRehash($user)) {
+            $this->userProvider->upgradePassword($user, $this->passwordEncoder->encodePassword($user, $password));
+        }
         $this->userChecker->checkPostAuth($user);
 
         // turn the UserInterface into a TokenInterface