Skip to content

Commit

Permalink
[Security] add support for opportunistic password migrations
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Aug 5, 2019
1 parent e5ab85d commit 231592f
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 9 deletions.
9 changes: 8 additions & 1 deletion Authentication/Provider/DaoAuthenticationProvider.php
Expand Up @@ -16,6 +16,7 @@
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;
Expand Down Expand Up @@ -54,9 +55,15 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
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()));
}
}
}

Expand Down
44 changes: 43 additions & 1 deletion Tests/Authentication/Provider/DaoAuthenticationProviderTest.php
Expand Up @@ -15,6 +15,10 @@
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
{
Expand Down Expand Up @@ -247,6 +251,44 @@ public function testCheckAuthentication()
$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();
Expand All @@ -261,7 +303,7 @@ protected function getSupportedToken()

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')
Expand Down
6 changes: 0 additions & 6 deletions Tests/Encoder/MigratingPasswordEncoderTest.php
Expand Up @@ -14,7 +14,6 @@
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
{
Expand Down Expand Up @@ -66,8 +65,3 @@ public function testFallback()
$this->assertTrue($encoder->isPasswordValid('abc', 'foo', 'salt'));
}
}

interface TestPasswordEncoderInterface extends PasswordEncoderInterface
{
public function needsRehash(string $encoded): bool;
}
19 changes: 19 additions & 0 deletions 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;
}
24 changes: 24 additions & 0 deletions Tests/User/ChainUserProviderTest.php
Expand Up @@ -15,6 +15,8 @@
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
{
Expand Down Expand Up @@ -188,6 +190,28 @@ public function testAcceptsTraversable()
$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();
Expand Down
18 changes: 17 additions & 1 deletion User/ChainUserProvider.php
Expand Up @@ -22,7 +22,7 @@
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class ChainUserProvider implements UserProviderInterface
class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterface
{
private $providers;

Expand Down Expand Up @@ -104,4 +104,20 @@ public function supportsClass($class)

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
}
}
}
}
}
27 changes: 27 additions & 0 deletions 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;
}

0 comments on commit 231592f

Please sign in to comment.