Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d79e915

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[Security] add PasswordUpgraderInterface for opportunistic password migrations
1 parent ec9159e commit d79e915
Copy full SHA for d79e915

15 files changed

+224
-16
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php
+18-1Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Doctrine\Common\Persistence\ManagerRegistry;
1515
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
1616
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
17+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
1718
use Symfony\Component\Security\Core\User\UserInterface;
1819
use Symfony\Component\Security\Core\User\UserProviderInterface;
1920

@@ -25,7 +26,7 @@
2526
* @author Fabien Potencier <fabien@symfony.com>
2627
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
2728
*/
28-
class EntityUserProvider implements UserProviderInterface
29+
class EntityUserProvider implements UserProviderInterface, PasswordUpgraderInterface
2930
{
3031
private $registry;
3132
private $managerName;
@@ -107,6 +108,22 @@ public function supportsClass($class)
107108
return $class === $this->getClass() || is_subclass_of($class, $this->getClass());
108109
}
109110

111+
/**
112+
* {@inheritdoc}
113+
*/
114+
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
115+
{
116+
$class = $this->getClass();
117+
if (!$user instanceof $class) {
118+
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
119+
}
120+
121+
$repository = $this->getRepository();
122+
if ($repository instanceof PasswordUpgraderInterface) {
123+
$repository->upgradePassword($user, $newEncodedPassword);
124+
}
125+
}
126+
110127
private function getObjectManager()
111128
{
112129
return $this->registry->getManager($this->managerName);

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Bridge\Doctrine\Security\User\EntityUserProvider;
1717
use Symfony\Bridge\Doctrine\Test\DoctrineTestHelper;
1818
use Symfony\Bridge\Doctrine\Tests\Fixtures\User;
19+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
1920

2021
class EntityUserProviderTest extends TestCase
2122
{
@@ -182,6 +183,23 @@ public function testLoadUserByUserNameShouldDeclineInvalidInterface()
182183
$provider->loadUserByUsername('name');
183184
}
184185

186+
public function testPasswordUpgrades()
187+
{
188+
$user = new User(1, 1, 'user1');
189+
190+
$repository = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
191+
$repository->expects($this->once())
192+
->method('upgradePassword')
193+
->with($user, 'foobar');
194+
195+
$provider = new EntityUserProvider(
196+
$this->getManager($this->getObjectManager($repository)),
197+
'Symfony\Bridge\Doctrine\Tests\Fixtures\User'
198+
);
199+
200+
$provider->upgradePassword($user, 'foobar');
201+
}
202+
185203
private function getManager($em, $name = null)
186204
{
187205
$manager = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')->getMock();

‎src/Symfony/Bridge/Doctrine/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/composer.json
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"symfony/property-access": "^3.4|^4.0|^5.0",
3434
"symfony/property-info": "^3.4|^4.0|^5.0",
3535
"symfony/proxy-manager-bridge": "^3.4|^4.0|^5.0",
36-
"symfony/security-core": "^3.4|^4.0|^5.0",
36+
"symfony/security-core": "^4.4|^5.0",
3737
"symfony/expression-language": "^3.4|^4.0|^5.0",
3838
"symfony/validator": "^3.4|^4.0|^5.0",
3939
"symfony/translation": "^3.4|^4.0|^5.0",
@@ -49,7 +49,8 @@
4949
"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0",
5050
"symfony/dependency-injection": "<3.4",
5151
"symfony/form": "<4.3",
52-
"symfony/messenger": "<4.3"
52+
"symfony/messenger": "<4.3",
53+
"symfony/security-core": "<4.4"
5354
},
5455
"suggest": {
5556
"symfony/form": "",

‎src/Symfony/Component/Security/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ CHANGELOG
66

77
* Added method `needsRehash()` to `PasswordEncoderInterface` and `UserPasswordEncoderInterface`
88
* Added `MigratingPasswordEncoder`
9+
* Added and implemented `PasswordUpgraderInterface`
910

1011
4.3.0
1112
-----

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
1717
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1818
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
19+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
1920
use Symfony\Component\Security\Core\User\UserCheckerInterface;
2021
use Symfony\Component\Security\Core\User\UserInterface;
2122
use Symfony\Component\Security\Core\User\UserProviderInterface;
@@ -54,9 +55,15 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
5455
throw new BadCredentialsException('The presented password cannot be empty.');
5556
}
5657

57-
if (!$this->encoderFactory->getEncoder($user)->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
58+
$encoder = $this->encoderFactory->getEncoder($user);
59+
60+
if (!$encoder->isPasswordValid($user->getPassword(), $presentedPassword, $user->getSalt())) {
5861
throw new BadCredentialsException('The presented password is invalid.');
5962
}
63+
64+
if ($this->userProvider instanceof PasswordUpgraderInterface && method_exists($encoder, 'needsRehash') && $encoder->needsRehash($user->getPassword())) {
65+
$this->userProvider->upgradePassword($user, $encoder->encodePassword($presentedPassword, $user->getSalt()));
66+
}
6067
}
6168
}
6269

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Authentication/Provider/DaoAuthenticationProviderTest.php
+43-1Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
use Symfony\Component\Security\Core\Authentication\Provider\DaoAuthenticationProvider;
1616
use Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder;
1717
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
18+
use Symfony\Component\Security\Core\Tests\Encoder\TestPasswordEncoderInterface;
19+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
20+
use Symfony\Component\Security\Core\User\User;
21+
use Symfony\Component\Security\Core\User\UserProviderInterface;
1822

1923
class DaoAuthenticationProviderTest extends TestCase
2024
{
@@ -259,6 +263,44 @@ public function testCheckAuthentication()
259263
$method->invoke($provider, $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(), $token);
260264
}
261265

266+
public function testPasswordUpgrades()
267+
{
268+
$user = new User('user', 'pwd');
269+
270+
$encoder = $this->getMockBuilder(TestPasswordEncoderInterface::class)->getMock();
271+
$encoder->expects($this->once())
272+
->method('isPasswordValid')
273+
->willReturn(true)
274+
;
275+
$encoder->expects($this->once())
276+
->method('encodePassword')
277+
->willReturn('foobar')
278+
;
279+
$encoder->expects($this->once())
280+
->method('needsRehash')
281+
->willReturn(true)
282+
;
283+
284+
$provider = $this->getProvider(null, null, $encoder);
285+
286+
$userProvider = ((array) $provider)[sprintf("\0%s\0userProvider", DaoAuthenticationProvider::class)];
287+
$userProvider->expects($this->once())
288+
->method('upgradePassword')
289+
->with($user, 'foobar')
290+
;
291+
292+
$method = new \ReflectionMethod($provider, 'checkAuthentication');
293+
$method->setAccessible(true);
294+
295+
$token = $this->getSupportedToken();
296+
$token->expects($this->once())
297+
->method('getCredentials')
298+
->willReturn('foo')
299+
;
300+
301+
$method->invoke($provider, $user, $token);
302+
}
303+
262304
protected function getSupportedToken()
263305
{
264306
$mock = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Authentication\\Token\\UsernamePasswordToken')->setMethods(['getCredentials', 'getUser', 'getProviderKey'])->disableOriginalConstructor()->getMock();
@@ -273,7 +315,7 @@ protected function getSupportedToken()
273315

274316
protected function getProvider($user = null, $userChecker = null, $passwordEncoder = null)
275317
{
276-
$userProvider = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserProviderInterface')->getMock();
318+
$userProvider = $this->getMockBuilder([UserProviderInterface::class, PasswordUpgraderInterface::class])->getMock();
277319
if (null !== $user) {
278320
$userProvider->expects($this->once())
279321
->method('loadUserByUsername')

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Encoder/MigratingPasswordEncoderTest.php
-5Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,3 @@ public function testFallback()
6666
$this->assertTrue($encoder->isPasswordValid('abc', 'foo', 'salt'));
6767
}
6868
}
69-
70-
interface TestPasswordEncoderInterface extends PasswordEncoderInterface
71-
{
72-
public function needsRehash(string $encoded): bool;
73-
}

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

Copy file name to clipboard
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Tests\Encoder;
13+
14+
interface TestPasswordEncoderInterface extends PasswordEncoderInterface
15+
{
16+
public function needsRehash(string $encoded): bool;
17+
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/User/ChainUserProviderTest.php
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
1616
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
1717
use Symfony\Component\Security\Core\User\ChainUserProvider;
18+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
19+
use Symfony\Component\Security\Core\User\User;
1820

1921
class ChainUserProviderTest extends TestCase
2022
{
@@ -192,6 +194,28 @@ public function testAcceptsTraversable()
192194
$this->assertSame($account, $provider->refreshUser($this->getAccount()));
193195
}
194196

197+
public function testPasswordUpgrades()
198+
{
199+
$user = new User('user', 'pwd');
200+
201+
$provider1 = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
202+
$provider1
203+
->expects($this->once())
204+
->method('upgradePassword')
205+
->willThrowException(new UnsupportedUserException('unsupported'))
206+
;
207+
208+
$provider2 = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
209+
$provider2
210+
->expects($this->once())
211+
->method('upgradePassword')
212+
->with($user, 'foobar')
213+
;
214+
215+
$provider = new ChainUserProvider([$provider1, $provider2]);
216+
$provider->upgradePassword($user, 'foobar');
217+
}
218+
195219
protected function getAccount()
196220
{
197221
return $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/ChainUserProvider.php
+17-1Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
2424
*/
25-
class ChainUserProvider implements UserProviderInterface
25+
class ChainUserProvider implements UserProviderInterface, PasswordUpgraderInterface
2626
{
2727
private $providers;
2828

@@ -104,4 +104,20 @@ public function supportsClass($class)
104104

105105
return false;
106106
}
107+
108+
/**
109+
* {@inheritdoc}
110+
*/
111+
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
112+
{
113+
foreach ($this->providers as $provider) {
114+
if ($provider instanceof PasswordUpgraderInterface) {
115+
try {
116+
$provider->upgradePassword($user, $newEncodedPassword);
117+
} catch (UnsupportedUserException $e) {
118+
// ignore: password upgrades are opportunistic
119+
}
120+
}
121+
}
122+
}
107123
}

‎src/Symfony/Component/Security/Core/User/LdapUserProvider.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/LdapUserProvider.php
+37-1Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Ldap\Entry;
1515
use Symfony\Component\Ldap\Exception\ConnectionException;
16+
use Symfony\Component\Ldap\Exception\ExceptionInterface;
1617
use Symfony\Component\Ldap\LdapInterface;
1718
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1819
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
@@ -24,7 +25,7 @@
2425
* @author Grégoire Pineau <lyrixx@lyrixx.info>
2526
* @author Charles Sarrazin <charles@sarraz.in>
2627
*/
27-
class LdapUserProvider implements UserProviderInterface
28+
class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterface
2829
{
2930
private $ldap;
3031
private $baseDn;
@@ -34,6 +35,7 @@ class LdapUserProvider implements UserProviderInterface
3435
private $uidKey;
3536
private $defaultSearch;
3637
private $passwordAttribute;
38+
private $entry;
3739

3840
public function __construct(LdapInterface $ldap, string $baseDn, string $searchDn = null, string $searchPassword = null, array $defaultRoles = [], string $uidKey = null, string $filter = null, string $passwordAttribute = null)
3941
{
@@ -89,6 +91,11 @@ public function loadUserByUsername($username)
8991
} catch (InvalidArgumentException $e) {
9092
}
9193

94+
if (null !== $this->entry) {
95+
// Keep $entry around when called from upgradePassword()
96+
$this->entry = $entry;
97+
}
98+
9299
return $this->loadUser($username, $entry);
93100
}
94101

@@ -112,6 +119,35 @@ public function supportsClass($class)
112119
return 'Symfony\Component\Security\Core\User\User' === $class;
113120
}
114121

122+
/**
123+
* {@inheritdoc}
124+
*/
125+
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
126+
{
127+
if (!$user instanceof User) {
128+
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
129+
}
130+
131+
if (null === $this->passwordAttribute) {
132+
return;
133+
}
134+
135+
try {
136+
// Tell loadUserByUsername() to keep the $entry around
137+
$this->entry = true;
138+
139+
if ($user->isEqualTo($this->loadUserByUsername($user->getUsername())) && \is_object($this->entry)) {
140+
$this->entry->setAttribute($this->passwordAttribute, [$newEncodedPassword]);
141+
$this->ldap->getEntryManager()->update($this->entry);
142+
$user->setPassword($newEncodedPassword);
143+
}
144+
} catch (ExceptionInterface $e) {
145+
// ignore failed password upgrades
146+
} finally {
147+
$this->entry = null;
148+
}
149+
}
150+
115151
/**
116152
* Loads a user from an LDAP entry.
117153
*

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

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\User;
13+
14+
/**
15+
* @author Nicolas Grekas <p@tchwork.com>
16+
*/
17+
interface PasswordUpgraderInterface
18+
{
19+
/**
20+
* Upgrades the encoded password of a user, typically for using a better hash algorithm.
21+
*
22+
* This method should persist the new password in the user storage and update the $user object accordingly.
23+
* Because you don't want your users not being able to log in, this method should be opportunistic:
24+
* it's fine if it does nothing or if it fails without throwing any exception.
25+
*/
26+
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void;
27+
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/User.php
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,9 @@ public function isEqualTo(UserInterface $user)
157157

158158
return true;
159159
}
160+
161+
public function setPassword(string $password)
162+
{
163+
$this->password = $password;
164+
}
160165
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.