From da5c39ec2e65ed4e58b08577e8de29e06ef65a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Tue, 8 Dec 2020 18:02:24 +0100 Subject: [PATCH] Move AuthenticationSuccessEvent outside try/catch block --- .../Authentication/AuthenticatorManager.php | 24 +++++++++---------- .../EventListener/UserCheckerListener.php | 13 +++++----- .../EventListener/UserCheckerListenerTest.php | 18 ++++++-------- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php index 318fd7bd21193..d3afaacdd17b1 100644 --- a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php +++ b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php @@ -185,18 +185,6 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req if (null !== $this->logger) { $this->logger->info('Authenticator successful!', ['token' => $authenticatedToken, 'authenticator' => \get_class($authenticator)]); } - - // success! (sets the token on the token storage, etc) - $response = $this->handleAuthenticationSuccess($authenticatedToken, $passport, $request, $authenticator); - if ($response instanceof Response) { - return $response; - } - - if (null !== $this->logger) { - $this->logger->debug('Authenticator set no success response: request continues.', ['authenticator' => \get_class($authenticator)]); - } - - return null; } catch (AuthenticationException $e) { // oh no! Authentication failed! $response = $this->handleAuthenticationFailure($e, $request, $authenticator, $passport); @@ -206,6 +194,18 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req return null; } + + // success! (sets the token on the token storage, etc) + $response = $this->handleAuthenticationSuccess($authenticatedToken, $passport, $request, $authenticator); + if ($response instanceof Response) { + return $response; + } + + if (null !== $this->logger) { + $this->logger->debug('Authenticator set no success response: request continues.', ['authenticator' => \get_class($authenticator)]); + } + + return null; } private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, PassportInterface $passport, Request $request, AuthenticatorInterface $authenticator): ?Response diff --git a/src/Symfony/Component/Security/Http/EventListener/UserCheckerListener.php b/src/Symfony/Component/Security/Http/EventListener/UserCheckerListener.php index 62da75e91b84a..bc346b9ad837f 100644 --- a/src/Symfony/Component/Security/Http/EventListener/UserCheckerListener.php +++ b/src/Symfony/Component/Security/Http/EventListener/UserCheckerListener.php @@ -12,11 +12,12 @@ namespace Symfony\Component\Security\Http\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent; use Symfony\Component\Security\Core\User\UserCheckerInterface; +use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PreAuthenticatedUserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; use Symfony\Component\Security\Http\Event\CheckPassportEvent; -use Symfony\Component\Security\Http\Event\LoginSuccessEvent; /** * @author Wouter de Jong @@ -43,21 +44,21 @@ public function preCheckCredentials(CheckPassportEvent $event): void $this->userChecker->checkPreAuth($passport->getUser()); } - public function postCheckCredentials(LoginSuccessEvent $event): void + public function postCheckCredentials(AuthenticationSuccessEvent $event): void { - $passport = $event->getPassport(); - if (!$passport instanceof UserPassportInterface || null === $passport->getUser()) { + $user = $event->getAuthenticationToken()->getUser(); + if (!$user instanceof UserInterface) { return; } - $this->userChecker->checkPostAuth($passport->getUser()); + $this->userChecker->checkPostAuth($user); } public static function getSubscribedEvents(): array { return [ CheckPassportEvent::class => ['preCheckCredentials', 256], - LoginSuccessEvent::class => ['postCheckCredentials', 256], + AuthenticationSuccessEvent::class => ['postCheckCredentials', 256], ]; } } diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/UserCheckerListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/UserCheckerListenerTest.php index 5422abfe5db92..213cfbba68718 100644 --- a/src/Symfony/Component/Security/Http/Tests/EventListener/UserCheckerListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/UserCheckerListenerTest.php @@ -12,8 +12,8 @@ namespace Symfony\Component\Security\Http\Tests\EventListener; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken; +use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; @@ -21,8 +21,8 @@ use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; +use Symfony\Component\Security\Http\Authenticator\Token\PostAuthenticationToken; use Symfony\Component\Security\Http\Event\CheckPassportEvent; -use Symfony\Component\Security\Http\Event\LoginSuccessEvent; use Symfony\Component\Security\Http\EventListener\UserCheckerListener; class UserCheckerListenerTest extends TestCase @@ -63,14 +63,14 @@ public function testPostAuthValidCredentials() { $this->userChecker->expects($this->once())->method('checkPostAuth')->with($this->user); - $this->listener->postCheckCredentials($this->createLoginSuccessEvent()); + $this->listener->postCheckCredentials(new AuthenticationSuccessEvent(new PostAuthenticationToken($this->user, 'main', []))); } public function testPostAuthNoUser() { $this->userChecker->expects($this->never())->method('checkPostAuth'); - $this->listener->postCheckCredentials($this->createLoginSuccessEvent($this->createMock(PassportInterface::class))); + $this->listener->postCheckCredentials(new AuthenticationSuccessEvent(new PreAuthenticatedToken('nobody', null, 'main'))); } private function createCheckPassportEvent($passport = null) @@ -82,12 +82,8 @@ private function createCheckPassportEvent($passport = null) return new CheckPassportEvent($this->createMock(AuthenticatorInterface::class), $passport); } - private function createLoginSuccessEvent($passport = null) + private function createAuthenticationSuccessEvent() { - if (null === $passport) { - $passport = new SelfValidatingPassport(new UserBadge('test', function () { return $this->user; })); - } - - return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), $passport, $this->createMock(TokenInterface::class), new Request(), null, 'main'); + return new AuthenticationSuccessEvent(new PostAuthenticationToken($this->user, 'main', [])); } }