8000 feature #39111 [Security] Update password upgrader listener to work w… · symfony/symfony@de09329 · GitHub
[go: up one dir, main page]
Skip to content

Commit de09329

Browse files
chalasrchalasr
committed
feature #39111 [Security] Update password upgrader listener to work with the new UserBadge (wouterj)
This PR was merged into the 5.2 branch. Discussion -- 8000 -------- [Security] Update password upgrader listener to work with the new UserBadge | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - While working on a new amazing `make:auth` maker, @jrushlow discovered that we forgot to update the `PasswordUpgradeBadge` with the 5.2 `UserBadge` changes (ref #37846). This PR fixes it, by making the password upgrader optional and falling back to the user provider instead. Without these changes, each authenticator still needs to know the user repository/user provider in order to pass it to `PasswordUpgradeBadge`. I'm sorry for catching this soo late in the release cycle. There is a BC break involved here, but it's (a) very unlikely to impact application code and (b) in an experimental class. Commits ------- e39e844 Default to user provider, if available, in password upgrader
2 parents 0302332 + e39e844 commit de09329

File tree

5 files changed

+49
-15
lines changed

5 files changed

+49
-15
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ CHANGELOG
1515
* Added `LoginThrottlingListener`.
1616
* Added `LoginLinkAuthenticator`.
1717
* Moved methods `supports()` and `authenticate()` from `AbstractListener` to `FirewallListenerInterface`.
18+
* [BC break] `PasswordUpgradeBadge::getPasswordUpgrader()` changed its return type to return null or a `PasswordUpgraderInterface` implementation.
1819

1920
5.1.0
2021
-----

‎src/Symfony/Component/Security/Http/Authenticator/Passport/Badge/PasswordUpgradeBadge.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ class PasswordUpgradeBadge implements BadgeInterface
3030
private $passwordUpgrader;
3131

3232
/**
33-
* @param string $plaintextPassword The presented password, used in the rehash
34-
* @param PasswordUpgraderInterface $passwordUpgrader The password upgrader, usually the UserProvider
33+
* @param string $plaintextPassword The presented password, used in the rehash
34+
* @param PasswordUpgraderInterface|null $passwordUpgrader The password upgrader, defaults to the UserProvider if null
3535
*/
36-
public function __construct(string $plaintextPassword, PasswordUpgraderInterface $passwordUpgrader)
36+
public function __construct(string $plaintextPassword, ?PasswordUpgraderInterface $passwordUpgrader = null)
3737
{
3838
$this->plaintextPassword = $plaintextPassword;
3939
$this->passwordUpgrader = $passwordUpgrader;
@@ -51,7 +51,7 @@ public function getAndErasePlaintextPassword(): string
5151
return $password;
5252
}
5353

54-
public function getPasswordUpgrader(): PasswordUpgraderInterface
54+
public function getPasswordUpgrader(): ?PasswordUpgraderInterface
5555
{
5656
return $this->passwordUpgrader;
5757
}

‎src/Symfony/Component/Security/Http/EventListener/PasswordMigratingListener.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1515
use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
16+
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
1617
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge;
18+
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
1719
use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface;
1820
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
1921

@@ -53,7 +55,19 @@ public function onLoginSuccess(LoginSuccessEvent $event): void
5355
return;
5456
}
5557

56-
$badge->getPasswordUpgrader()->upgradePassword($user, $passwordEncoder->encodePassword($plaintextPassword, $user->getSalt()));
58+
$passwordUpgrader = $badge->getPasswordUpgrader();
59+
if (null === $passwordUpgrader) {
60+
/** @var UserBadge $userBadge */
61+
$userBadge = $passport->getBadge(UserBadge::class);
62+
$userLoader = $userBadge->getUserLoader();
63+
if (\is_array($userLoader) && $userLoader[0] instanceof PasswordUpgraderInterface) {
64+
$passwordUpgrader = $userLoader[0];
65+
} else {
66+
return;
67+
}
68+
}
69+
70+
$passwordUpgrader->upgradePassword($user, $passwordEncoder->encodePassword($plaintextPassword, $user->getSalt()));
5771
}
5872

5973
public static function getSubscribedEvents(): array

‎src/Symfony/Component/Security/Http/EventListener/UserProviderListener.php

Lines changed: 3 additions & 0 deletions
9E88
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
1717

1818
/**
19+
* Configures the user provider as user loader, if no user load
20+
* has been explicitly set.
21+
*
1922
* @author Wouter de Jong <wouter@wouterj.nl>
2023
*
2124
* @final

‎src/Symfony/Component/Security/Http/Tests/EventListener/PasswordMigratingListenerTest.php

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;
1919
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
2020
use Symfony\Component\Security\Core\User\UserInterface;
21+
use Symfony\Component\Security\Core\User\UserProviderInterface;
2122
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
2223
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge;
2324
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
@@ -34,9 +35,14 @@ class PasswordMigratingListenerTest extends TestCase
3435

3536
protected function setUp(): void
3637
{
38+
$this->user = $this->createMock(UserInterface::class);
39+
$this->user->expects($this->any())->method('getPassword')->willReturn('old-encoded-password');
40+
$encoder = $this->createMock(PasswordEncoderInterface::class);
41+
$encoder->expects($this->any())->method('needsRehash')->willReturn(true);
42+
$encoder->expects($this->any())->method('encodePassword')->with('pa$$word', null)->willReturn('new-encoded-password');
3743
$this->encoderFactory = $this->createMock(EncoderFactoryInterface::class);
44+
$this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->user)->willReturn($encoder);
3845
$this->listener = new PasswordMigratingListener($this->encoderFactory);
39-
$this->user = $this->createMock(UserInterface::class);
4046
}
4147

4248
/**
@@ -61,16 +67,8 @@ public function provideUnsupportedEvents()
6167
yield [$this->createEvent($this->createMock(PassportInterface::class))];
6268
}
6369

64-
public function testUpgrade()
70+
public function testUpgradeWithUpgrader()
6571
{
66-
$encoder = $this->createMock(PasswordEncoderInterface::class);
67-
$encoder->expects($this->any())->method('needsRehash')->willReturn(true);
68-
$encoder->expects($this->any())->method('encodePassword')->with('pa$$word', null)->willReturn('new-encoded-password');
69-
70-
$this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->user)->willReturn($encoder);
71-
72-
$this->user->expects($this->any())->method('getPassword')->willReturn('old-encoded-password');
73-
7472
$passwordUpgrader = $this->createPasswordUpgrader();
7573
$passwordUpgrader->expects($this->once())
7674
->method('upgradePassword')
@@ -81,6 +79,20 @@ public function testUpgrade()
8179
$this->listener->onLoginSuccess($event);
8280
}
8381

82+
public function testUpgradeWithoutUpgrader()
83+
{
84+
$userLoader = $this->createMock(MigratingUserProvider::class);
85+
$userLoader->expects($this->any())->method('loadUserByUsername')->willReturn($this->user);
86+
87+
$userLoader->expects($this->once())
88+
->method('upgradePassword')
89+
->with($this->user, 'new-encoded-password')
90+
;
91+
92+
$event = $this->createEvent(new SelfValidatingPassport(new UserBadge('test', [$userLoader, 'loadUserByUsername']), [new PasswordUpgradeBadge('pa$$word')]));
93+
$this->listener->onLoginSuccess($event);
94+
}
95+
8496
private function createPasswordUpgrader()
8597
{
8698
return $this->createMock(PasswordUpgraderInterface::class);
@@ -91,3 +103,7 @@ private function createEvent(PassportInterface $passport)
91103
return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), $passport, $this->createMock(TokenInterface::class), new Request(), null, 'main');
92104
}
93105
}
106+
107+
abstract class MigratingUserProvider implements UserProviderInterface, PasswordUpgraderInterface
108+
{
109+
}

0 commit comments

Comments
 (0)
0