10000 feature #39213 [Security] [DX] Automatically add PasswordUpgradeBadge… · symfony/symfony@ffd365b · GitHub
[go: up one dir, main page]
Skip to content

Commit ffd365b

Browse files
fabpotfabpot
committed
feature #39213 [Security] [DX] Automatically add PasswordUpgradeBadge + default support() impl in AbstractFormLoginAuthenticator (wouterj)
This PR was squashed before being merged into the 5.2 branch. Discussion ---------- [Security] [DX] Automatically add PasswordUpgradeBadge + default support() impl in AbstractFormLoginAuthenticator | Q | A | ------------- | --- | Branch? | 5.2 (hopefully? sorry to keep pushing the barrier here) | Bug fix? | no | New feature? | yes (sort of) | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - These are 2 suggestions we found while implementing `make:auth` for the new system (symfony/maker-bundle#736): Impact on a custom login form authenticator ([as generated by the new maker](https://github.com/symfony/maker-bundle/pull/736/files#diff-528164b6c24778d5e81fa3819b0552f0e68a9fea33c7d3446a012f3da7d0af60)): * **Automatically add `PasswordUpgradeBadge`** if there is a user password with valid password credentials. ```diff // ... return new Passport( new UserBadge($userIdentifier), new PasswordCredentials($password), [ - new PasswordUpgradeBadge($password), new CsrfTokenBadge('authenticate', $csrf), ] ) ``` Note that this does not automatically migrate all passwords: it still relies on `PasswordUpgraderInterface` to be implemented on the user loader/provider. * **Add default implementation of `AbstractFormLoginAuthenticator::support()`** ```diff - public function supports(Request $request): ?bool - { - return self::LOGIN_ROUTE === $request->attributes->get('_route') - && $request->isMethod('POST'); - } ``` cc @weaverryan @jrushlow Commits ------- 27450c0 [Security] [DX] Automatically add PasswordUpgradeBadge + default support() impl in AbstractFormLoginAuthenticator
2 parents 681b75c + 27450c0 commit ffd365b

File tree

3 files changed

+68
-0
lines changed

3 files changed

+68
-0
lines changed

‎src/Symfony/Component/Security/Http/Authenticator/AbstractLoginFormAuthenticator.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@ abstract class AbstractLoginFormAuthenticator extends AbstractAuthenticator impl
3232
*/
3333
abstract protected function getLoginUrl(Request $request): string;
3434

35+
/**
36+
* {@inheritdoc}
37+
*
38+
* Override to change the request conditions that have to be
39+
* matched in order to handle the login form submit.
40+
*
41+
* This default implementation handles all POST requests to the
42+
* login path (@see getLoginUrl()).
43+
*/
44+
public function supports(Request $request): bool
45+
{
46+
return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getPathInfo();
47+
}
48+
3549
/**
3650
* Override to change what happens after a bad username/password is submitted.
3751
*/

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1515
use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
1616
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
17+
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge;
1718
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials;
1819
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
1920
use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface;
@@ -65,6 +66,10 @@ public function checkPassport(CheckPassportEvent $event): void
6566

6667
$badge->markResolved();
6768

69+
if (!$passport->hasBadge(PasswordUpgradeBadge::class)) {
70+
$passport->addBadge(new PasswordUpgradeBadge($presentedPassword));
71+
}
72+
6873
return;
6974
}
7075

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1818
use Symfony\Component\Security\Core\User\User;
1919
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
20+
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge;
2021
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2122
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials;
2223
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
@@ -113,6 +114,54 @@ public function testNoCredentialsBadgeProvided()
113114
$this->listener->checkPassport($event);
114115
}
115116

117+
public function testAddsPasswordUpgradeBadge()
118+
{
119+
$encoder = $this->createMock(PasswordEncoderInterface::class);
120+
$encoder->expects($this->any())->method('isPasswordValid')->with('encoded-password', 'ThePa$$word')->willReturn(true);
121+
122+
$this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->identicalTo($this->user))->willReturn($encoder);
123+
124+
$passport = new Passport(new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'));
125+
$this->listener->checkPassport($this->createEvent($passport));
126+
127+
$this->assertTrue($passport->hasBadge(PasswordUpgradeBadge::class));
128+
$this->assertEquals('ThePa$$word', $passport->getBadge(PasswordUpgradeBadge::class)->getAndErasePlaintextPassword());
129+
}
130+
131+
public function testAddsNoPasswordUpgradeBadgeIfItAlreadyExists()
132+
{
133+
$encoder = $this->createMock(PasswordEncoderInterface::class);
134+
$encoder->expects($this->any())->method('isPasswordValid')->with('encoded-password', 'ThePa$$word')->willReturn(true);
135+
136+
$this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->identicalTo($this->user))->willReturn($encoder);
137+
138+
$passport = $this->getMockBuilder(Passport::class)
139+
->setMethods(['addBadge'])
140+
->setConstructorArgs([new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'), [new PasswordUpgradeBadge('ThePa$$word')]])
141+
->getMock();
142+
143+
$passport->expects($this->never())->method('addBadge')->with($this->isInstanceOf(PasswordUpgradeBadge::class));
144+
145+
$this->listener->checkPassport($this->createEvent($passport));
146+
}
147+
148+
public function testAddsNoPasswordUpgradeBadgeIfPasswordIsInvalid()
149+
{
150+
$encoder = $this->createMock(PasswordEncoderInterface::class);
151+
$encoder->expects($this->any())->method('isPasswordValid')->with('encoded-password', 'ThePa$$word')->willReturn(false);
152+
153+
$this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->identicalTo($this->user))->willReturn($encoder);
154+
155+
$passport = $this->getMockBuilder(Passport::class)
156+
->setMethods(['addBadge'])
157+
->setConstructorArgs([new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'), [new PasswordUpgradeBadge('ThePa$$word')]])
158+
->getMock();
159+
160+
$passport->expects($this->never())->method('addBadge')->with($this->isInstanceOf(PasswordUpgradeBadge::class));
161+
162+
$this->listener->checkPassport($this->createEvent($passport));
163+
}
164+
116165
private function createEvent($passport)
117166
{
118167
return new CheckPassportEvent($this->createMock(AuthenticatorInterface::class), $passport);

0 commit comments

Comments
 (0)
0