From 27450c0bb4827ea7ab354307f05d79b3f1365197 Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sat, 28 Nov 2020 16:34:29 +0100 Subject: [PATCH] [Security] [DX] Automatically add PasswordUpgradeBadge + default support() impl in AbstractFormLoginAuthenticator --- .../AbstractLoginFormAuthenticator.php | 14 ++++++ .../CheckCredentialsListener.php | 5 ++ .../CheckCredentialsListenerTest.php | 49 +++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/src/Symfony/Component/Security/Http/Authenticator/AbstractLoginFormAuthenticator.php b/src/Symfony/Component/Security/Http/Authenticator/AbstractLoginFormAuthenticator.php index 24c7405ea9..aeaf1d17cd 100644 --- a/src/Symfony/Component/Security/Http/Authenticator/AbstractLoginFormAuthenticator.php +++ b/src/Symfony/Component/Security/Http/Authenticator/AbstractLoginFormAuthenticator.php @@ -32,6 +32,20 @@ abstract class AbstractLoginFormAuthenticator extends AbstractAuthenticator impl */ abstract protected function getLoginUrl(Request $request): string; + /** + * {@inheritdoc} + * + * Override to change the request conditions that have to be + * matched in order to handle the login form submit. + * + * This default implementation handles all POST requests to the + * login path (@see getLoginUrl()). + */ + public function supports(Request $request): bool + { + return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getPathInfo(); + } + /** * Override to change what happens after a bad username/password is submitted. */ diff --git a/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php b/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php index e2d3f86b8f..c1f649b089 100644 --- a/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php +++ b/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Security\Http\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; @@ -65,6 +66,10 @@ class CheckCredentialsListener implements EventSubscriberInterface $badge->markResolved(); + if (!$passport->hasBadge(PasswordUpgradeBadge::class)) { + $passport->addBadge(new PasswordUpgradeBadge($presentedPassword)); + } + return; } diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/CheckCredentialsListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/CheckCredentialsListenerTest.php index 5dc411bef0..e903dcd22c 100644 --- a/src/Symfony/Component/Security/Http/Tests/EventListener/CheckCredentialsListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/CheckCredentialsListenerTest.php @@ -17,6 +17,7 @@ use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; @@ -113,6 +114,54 @@ class CheckCredentialsListenerTest extends TestCase $this->listener->checkPassport($event); } + public function testAddsPasswordUpgradeBadge() + { + $encoder = $this->createMock(PasswordEncoderInterface::class); + $encoder->expects($this->any())->method('isPasswordValid')->with('encoded-password', 'ThePa$$word')->willReturn(true); + + $this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->identicalTo($this->user))->willReturn($encoder); + + $passport = new Passport(new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word')); + $this->listener->checkPassport($this->createEvent($passport)); + + $this->assertTrue($passport->hasBadge(PasswordUpgradeBadge::class)); + $this->assertEquals('ThePa$$word', $passport->getBadge(PasswordUpgradeBadge::class)->getAndErasePlaintextPassword()); + } + + public function testAddsNoPasswordUpgradeBadgeIfItAlreadyExists() + { + $encoder = $this->createMock(PasswordEncoderInterface::class); + $encoder->expects($this->any())->method('isPasswordValid')->with('encoded-password', 'ThePa$$word')->willReturn(true); + + $this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->identicalTo($this->user))->willReturn($encoder); + + $passport = $this->getMockBuilder(Passport::class) + ->setMethods(['addBadge']) + ->setConstructorArgs([new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'), [new PasswordUpgradeBadge('ThePa$$word')]]) + ->getMock(); + + $passport->expects($this->never())->method('addBadge')->with($this->isInstanceOf(PasswordUpgradeBadge::class)); + + $this->listener->checkPassport($this->createEvent($passport)); + } + + public function testAddsNoPasswordUpgradeBadgeIfPasswordIsInvalid() + { + $encoder = $this->createMock(PasswordEncoderInterface::class); + $encoder->expects($this->any())->method('isPasswordValid')->with('encoded-password', 'ThePa$$word')->willReturn(false); + + $this->encoderFactory->expects($this->any())->method('getEncoder')->with($this->identicalTo($this->user))->willReturn($encoder); + + $passport = $this->getMockBuilder(Passport::class) + ->setMethods(['addBadge']) + ->setConstructorArgs([new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'), [new PasswordUpgradeBadge('ThePa$$word')]]) + ->getMock(); + + $passport->expects($this->never())->method('addBadge')->with($this->isInstanceOf(PasswordUpgradeBadge::class)); + + $this->listener->checkPassport($this->createEvent($passport)); + } + private function createEvent($passport) { return new CheckPassportEvent($this->createMock(AuthenticatorInterface::class), $passport);