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 f6ea704

Browse filesBrowse files
author
Robin Chalas
committed
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 ------- 2cfc5c7 [Security] add support for opportunistic password migrations
2 parents d076d90 + 2cfc5c7 commit f6ea704
Copy full SHA for f6ea704

File tree

15 files changed

+242
-16
lines changed
Filter options

15 files changed

+242
-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
{
@@ -175,6 +176,23 @@ public function testLoadUserByUserNameShouldDeclineInvalidInterface()
175176
$provider->loadUserByUsername('name');
176177
}
177178

179+
public function testPasswordUpgrades()
180+
{
181+
$user = new User(1, 1, 'user1');
182+
183+
$repository = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
184+
$repository->expects($this->once())
185+
->method('upgradePassword')
186+
->with($user, 'foobar');
187+
188+
$provider = new EntityUserProvider(
189+
$this->getManager($this->getObjectManager($repository)),
190+
'Symfony\Bridge\Doctrine\Tests\Fixtures\User'
191+
);
192+
193+
$provider->upgradePassword($user, 'foobar');
194+
}
195+
178196
private function getManager($em, $name = null)
179197
{
180198
$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.4",
52-
"symfony/messenger": "<4.3"
52+
"symfony/messenger": "<4.3",
53+
"symfony/security-core": "<4.4"
5354
},
5455
"suggest": {
5556
"symfony/form": "",

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
<argument /> <!-- User Provider -->
3030
<argument /> <!-- Provider-shared Key -->
3131
<argument /> <!-- User Checker -->
32+
<argument type="service" id="security.password_encoder" />
3233
</service>
3334

3435
<service id="security.authentication.listener.guard"

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Ldap/Security/LdapUserProvider.php
+27-1Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
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;
1920
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
21+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
2022
use Symfony\Component\Security\Core\User\UserInterface;
2123
use Symfony\Component\Security\Core\User\UserProviderInterface;
2224

@@ -27,7 +29,7 @@
2729
* @author Charles Sarrazin <charles@sarraz.in>
2830
* @author Robin Chalas <robin.chalas@gmail.com>
2931
*/
30-
class LdapUserProvider implements UserProviderInterface
32+
class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterface
3133
{
3234
private $ldap;
3335
private $baseDn;
@@ -109,6 +111,30 @@ public function refreshUser(UserInterface $user)
109111
return new LdapUser($user->getEntry(), $user->getUsername(), $user->getPassword(), $user->getRoles());
110112
}
111113

114+
/**
115+
* {@inheritdoc}
116+
*/
117+
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
118+
{
119+
if (!$user instanceof LdapUser) {
120+
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
121+
}
122+
123+
if (null === $this->passwordAttribute) {
124+
return;
125+
}
126+
127+
try {
128+
if ($user->isEqualTo($this->loadUserByUsername($user->getUsername()))) {
129+
$user->getEntry()->setAttribute($this->passwordAttribute, [$newEncodedPassword]);
130+
$this->ldap->getEntryManager()->update($user->getEntry());
131+
$user->setPassword($newEncodedPassword);
132+
}
133+
} catch (ExceptionInterface $e) {
134+
// ignore failed password upgrades
135+
}
136+
}
137+
112138
/**
113139
* {@inheritdoc}
114140
*/

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/CHANGELOG.md
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ CHANGELOG
77
* Deprecated class `LdapUserProvider`, use `Symfony\Component\Ldap\Security\LdapUserProvider` instead
88
* Added method `needsRehash()` to `PasswordEncoderInterface` and `UserPasswordEncoderInterface`
99
* Added `MigratingPasswordEncoder`
10+
* Added and implemented `PasswordUpgraderInterface`, for opportunistic password migrations
11+
* Added `Guard\PasswordAuthenticatedInterface`, an optional interface
12+
for "guard" authenticators that deal with user passwords
1013

1114
4.3.0
1215
-----

‎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
{
@@ -247,6 +251,44 @@ public function testCheckAuthentication()
247251
$method->invoke($provider, $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserInterface')->getMock(), $token);
248252
}
249253

254+
public function testPasswordUpgrades()
255+
{
256+
$user = new User('user', 'pwd');
257+
258+
$encoder = $this->getMockBuilder(TestPasswordEncoderInterface::class)->getMock();
259+
$encoder->expects($this->once())
260+
->method('isPasswordValid')
261+
->willReturn(true)
262+
;
263+
$encoder->expects($this->once())
264+
->method('encodePassword')
265+
->willReturn('foobar')
266+
;
267+
$encoder->expects($this->once())
268+
->method('needsRehash')
269+
->willReturn(true)
270+
;
271+
272+
$provider = $this->getProvider(null, null, $encoder);
273+
274+
$userProvider = ((array) $provider)[sprintf("\0%s\0userProvider", DaoAuthenticationProvider::class)];
275+
$userProvider->expects($this->once())
276+
->method('upgradePassword')
277+
->with($user, 'foobar')
278+
;
279+
280+
$method = new \ReflectionMethod($provider, 'checkAuthentication');
281+
$method->setAccessible(true);
282+
283+
$token = $this->getSupportedToken();
284+
$token->expects($this->once())
285+
->method('getCredentials')
286+
->willReturn('foo')
287+
;
288+
289+
$method->invoke($provider, $user, $token);
290+
}
291+
250292
protected function getSupportedToken()
251293
{
252294
$mock = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Authentication\\Token\\UsernamePasswordToken')->setMethods(['getCredentials', 'getUser', 'getProviderKey'])->disableOriginalConstructor()->getMock();
@@ -261,7 +303,7 @@ protected function getSupportedToken()
261303

262304
protected function getProvider($user = null, $userChecker = null, $passwordEncoder = null)
263305
{
264-
$userProvider = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserProviderInterface')->getMock();
306+
$userProvider = $this->getMockBuilder([UserProviderInterface::class, PasswordUpgraderInterface::class])->getMock();
265307
if (null !== $user) {
266308
$userProvider->expects($this->once())
267309
->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
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Security\Core\Encoder\MigratingPasswordEncoder;
1616
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
17-
use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;
1817

1918
class MigratingPasswordEncoderTest extends TestCase
2019
{
@@ -66,8 +65,3 @@ public function testFallback()
6665
$this->assertTrue($encoder->isPasswordValid('abc', 'foo', 'salt'));
6766
}
6867
}
69-
70-
interface TestPasswordEncoderInterface extends PasswordEncoderInterface
71-
{
72-
public function needsRehash(string $encoded): bool;
73-
}
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
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+
use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;
15+
16+
interface TestPasswordEncoderInterface extends PasswordEncoderInterface
17+
{
18+
public function needsRehash(string $encoded): bool;
19+
}

‎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
{
@@ -188,6 +190,28 @@ public function testAcceptsTraversable()
188190
$this->assertSame($account, $provider->refreshUser($this->getAccount()));
189191
}
190192

193+
public function testPasswordUpgrades()
194+
{
195+
$user = new User('user', 'pwd');
196+
197+
$provider1 = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
198+
$provider1
199+
->expects($this->once())
200+
->method('upgradePassword')
201+
->willThrowException(new UnsupportedUserException('unsupported'))
202+
;
203+
204+
$provider2 = $this->getMockBuilder(PasswordUpgraderInterface::class)->getMock();
205+
$provider2
206+
->expects($this->once())
207+
->method('upgradePassword')
208+
->with($user, 'foobar')
209+
;
210+
211+
$provider = new ChainUserProvider([$provider1, $provider2]);
212+
$provider->upgradePassword($user, 'foobar');
213+
}
214+
191215
protected function getAccount()
192216
{
193217
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
}
+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+
}

0 commit comments

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