refactor: Simplified LockedUserSubscriber logic

src/EventSubscriber/LockedUserSubscriber.php

@@ -16,12 +16,12 @@
 use Doctrine\ORM\ORMException;
 use Lexik\Bundle\JWTAuthenticationBundle\Event\AuthenticationFailureEvent;
 use Lexik\Bundle\JWTAuthenticationBundle\Event\AuthenticationSuccessEvent;
-use Lexik\Bundle\JWTAuthenticationBundle\Event\Event as LexikBaseEvent;
 use Lexik\Bundle\JWTAuthenticationBundle\Events;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\Security\Core\User\UserInterface;
-use Symfony\Contracts\EventDispatcher\Event;
+use Symfony\Component\Security\Core\Exception\LockedException;
+use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
 use Throwable;
+use function count;
 use function is_string;
 
 /**
@@ -42,11 +42,6 @@ class LockedUserSubscriber implements EventSubscriberInterface
      */
     private $logLoginFailureResource;
 
-    /**
-     * @var bool
-     */
-    private $reset = true;
-
     /**
      * LockedUserSubscriber constructor.
      *
@@ -80,116 +75,71 @@ public function __construct(UserRepository $userRepository, LogLoginFailureResou
     public static function getSubscribedEvents(): array
     {
         return [
-            Events::AUTHENTICATION_SUCCESS => 'onAuthenticationSuccess',
+            Events::AUTHENTICATION_SUCCESS => [
+                'onAuthenticationSuccess',
+                128,
+            ],
             Events::AUTHENTICATION_FAILURE => 'onAuthenticationFailure',
         ];
     }
 
     /**
-     * Method to increase
-     *
-     * This method is called when '\Lexik\Bundle\JWTAuthenticationBundle\Events::AUTHENTICATION_FAILURE'
-     * event is broadcast.
-     *
-     * @psalm-suppress MissingDependency
-     *
-     * @param AuthenticationFailureEvent $event
+     * @param AuthenticationSuccessEvent $event
      *
      * @throws ORMException
-     * @throws Throwable
      */
-    public function onAuthenticationFailure(AuthenticationFailureEvent $event): void
+    public function onAuthenticationSuccess(AuthenticationSuccessEvent $event): void
     {
-        $token = $event->getException()->getToken();
+        $user = $this->getUser($event->getUser());
 
-        if ($token === null) {
-            return;
+        if ($user === null) {
+            throw new UnsupportedUserException('Unsupported user.');
         }
 
-        $user = $this->getUser($token->getUser());
-
-        if ($user instanceof User) {
-            $this->checkLockedAccount($user, $event);
+        if (count($user->getLogsLoginFailure()) > 10) {
+            throw new LockedException('Locked account.');
         }
-    }
-
-    /**
-     * Method to reset login failures for current user.
-     *
-     * This method is called when '\Symfony\Component\Security\Core\Event\AuthenticationEvent::AUTHENTICATION_SUCCESS'
-     * event is broadcast.
-     *
-     * @psalm-suppress MissingDependency
-     *
-     * @param Event $event
-     *
-     * @throws ORMException
-     * @throws Throwable
-     */
-    public function onAuthenticationSuccess(Event $event): void
-    {
-        if ($event instanceof AuthenticationSuccessEvent) {
-            $user = $this->getUser($event->getUser());
 
-            if ($user instanceof User) {
-                $this->checkLockedAccount($user, $event);
-            }
-        }
+        $this->logLoginFailureResource->reset($user);
     }
 
     /**
-     * @psalm-suppress MissingDependency
-     * @psalm-suppress MismatchingDocblockParamType
-     *
-     * @param User                                                                 $user
-     * @param LexikBaseEvent|AuthenticationFailureEvent|AuthenticationSuccessEvent $event
+     * @param AuthenticationFailureEvent $event
      *
      * @throws Throwable
      */
-    private function checkLockedAccount(User $user, LexikBaseEvent $event): void
+    public function onAuthenticationFailure(AuthenticationFailureEvent $event): void
     {
-        if ($event instanceof AuthenticationFailureEvent) {
-            $this->onAuthenticationFailureEvent($user);
-        } elseif ($event instanceof AuthenticationSuccessEvent) {
-            $this->onAuthenticationSuccessEvent($user);
-        }
-    }
+        $token = $event->getException()->getToken();
 
-    /**
-     * @param User $user
-     *
-     * @throws Throwable
-     */
-    private function onAuthenticationFailureEvent(User $user): void
-    {
-        $this->logLoginFailureResource->save(new LogLoginFailure($user));
-    }
+        if ($token !== null) {
+            $user = $this->getUser($token->getUser());
 
-    /**
-     * @param User $user
-     */
-    private function onAuthenticationSuccessEvent(User $user): void
-    {
-        if ($this->reset === true) {
-            $this->logLoginFailureResource->reset($user);
+            if ($user !== null) {
+                $this->logLoginFailureResource->save(new LogLoginFailure($user), true);
+            }
+
+            $token->setAuthenticated(false);
         }
     }
 
     /**
-     * @param string|UserInterface|mixed $user
+     * @param string|object $user
      *
      * @return User|null
      *
      * @throws ORMException
      */
     private function getUser($user): ?User
     {
+        $output = null;
+
         if (is_string($user)) {
-            $user = $this->userRepository->loadUserByUsername($user);
+            $output = $this->userRepository->loadUserByUsername($user);
         } elseif ($user instanceof SecurityUser) {
-            $user = $this->userRepository->loadUserByUsername($user->getUsername());
+            $output = $this->userRepository->loadUserByUsername($user->getUsername());
         }
 
-        return $user instanceof User ? $user : null;
+        return $output;
     }
 }

tests/Integration/EventSubscriber/LockedUserSubscriberTest.php

@@ -5,22 +5,26 @@
  *
  * @author TLe, Tarmo Leppänen <tarmo.leppanen@protacon.com>
  */
-
 namespace App\Tests\Integration\EventSubscriber;
 
-use App\Entity\User;
+use App\Entity\User as UserEntity;
 use App\EventSubscriber\LockedUserSubscriber;
 use App\Repository\UserRepository;
 use App\Resource\LogLoginFailureResource;
 use App\Security\SecurityUser;
+use Doctrine\Common\Collections\ArrayCollection;
 use Lexik\Bundle\JWTAuthenticationBundle\Event\AuthenticationFailureEvent;
 use Lexik\Bundle\JWTAuthenticationBundle\Event\AuthenticationSuccessEvent;
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Core\Exception\LockedException;
+use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
+use Symfony\Component\Security\Core\User\User as CoreUser;
 use Throwable;
+use function range;
 
 /**
  * Class LockedUserSubscriberTest
@@ -33,50 +37,64 @@ class LockedUserSubscriberTest extends KernelTestCase
     /**
      * @throws Throwable
      */
-    public function testThatOnAuthenticationFailureCallsExpectedServiceMethod(): void
+    public function testThatOnAuthenticationSuccessThrowsUserNotFoundException(): void
     {
-        $user = new User();
-        $user->setUsername('test-user');
+        $this->expectException(UnsupportedUserException::class);
+        $this->expectExceptionMessage('Unsupported user.');
 
-        $token = new UsernamePasswordToken('test-user', 'password', 'providerKey');
-        $token->setUser(new SecurityUser($user));
+        /**
+         * @var MockObject|UserRepository          $userRepository
+         * @var MockObject|LogLoginFailureResource $logLoginFailureResource
+         */
+        $userRepository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock();
+        $logLoginFailureResource = $this->getMockBuilder(LogLoginFailureResource::class)
+            ->disableOriginalConstructor()->getMock();
+
+        $event = new AuthenticationSuccessEvent([], new CoreUser('username', 'password'), new Response());
 
-        $authenticationException = new AuthenticationException();
-        $authenticationException->setToken($token);
+        (new LockedUserSubscriber($userRepository, $logLoginFailureResource))->onAuthenticationSuccess($event);
+    }
+
+    /**
+     * @throws Throwable
+     */
+    public function testThatOnAuthenticationSuccessThrowsLockedException(): void
+    {
+        $this->expectException(LockedException::class);
+        $this->expectExceptionMessage('Locked account.');
 
         /**
          * @var MockObject|UserRepository          $userRepository
          * @var MockObject|LogLoginFailureResource $logLoginFailureResource
+         * @var MockObject|UserEntity              $user
          */
         $userRepository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock();
         $logLoginFailureResource = $this->getMockBuilder(LogLoginFailureResource::class)
             ->disableOriginalConstructor()->getMock();
+        $user = $this->getMockBuilder(UserEntity::class)->getMock();
 
         $userRepository
             ->expects(static::once())
             ->method('loadUserByUsername')
             ->with($user->getId())
             ->willReturn($user);
 
-        $logLoginFailureResource
+        $user
             ->expects(static::once())
-            ->method('save');
+            ->method('getLogsLoginFailure')
+            ->willReturn(new ArrayCollection(range(0, 11)));
+
+        $securityUser = new SecurityUser($user);
+        $event = new AuthenticationSuccessEvent([], $securityUser, new Response());
 
-        $subscriber = new LockedUserSubscriber($userRepository, $logLoginFailureResource);
-        $subscriber->onAuthenticationFailure(new AuthenticationFailureEvent($authenticationException, new Response()));
+        (new LockedUserSubscriber($userRepository, $logLoginFailureResource))->onAuthenticationSuccess($event);
     }
 
     /**
      * @throws Throwable
      */
-    public function testThatOnAuthenticationSuccessCallsExpectedServiceMethod(): void
+    public function testThatOnAuthenticationSuccessResourceResetMethodIsCalled(): void
     {
-        $user = new User();
-        $user->setUsername('test-user');
-
-        $securityUser = new SecurityUser($user);
-        $event = new AuthenticationSuccessEvent([], $securityUser, new Response());
-
         /**
          * @var MockObject|UserRepository          $userRepository
          * @var MockObject|LogLoginFailureResource $logLoginFailureResource
@@ -85,6 +103,8 @@ public function testThatOnAuthenticationSuccessCallsExpectedServiceMethod(): voi
         $logLoginFailureResource = $this->getMockBuilder(LogLoginFailureResource::class)
             ->disableOriginalConstructor()->getMock();
 
+        $user = new UserEntity();
+
         $userRepository
             ->expects(static::once())
             ->method('loadUserByUsername')
@@ -96,7 +116,101 @@ public function testThatOnAuthenticationSuccessCallsExpectedServiceMethod(): voi
             ->method('reset')
             ->with($user);
 
-        $subscriber = new LockedUserSubscriber($userRepository, $logLoginFailureResource);
-        $subscriber->onAuthenticationSuccess($event);
+        $securityUser = new SecurityUser($user);
+        $event = new AuthenticationSuccessEvent([], $securityUser, new Response());
+
+        (new LockedUserSubscriber($userRepository, $logLoginFailureResource))->onAuthenticationSuccess($event);
+    }
+
+    /**
+     * @throws Throwable
+     */
+    public function testThatOnAuthenticationFailureRepositoryAndResourceMethodsAreNotCalledWhenTokenIsNull(): void
+    {
+        /**
+         * @var MockObject|UserRepository          $userRepository
+         * @var MockObject|LogLoginFailureResource $logLoginFailureResource
+         */
+        $userRepository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock();
+        $logLoginFailureResource = $this->getMockBuilder(LogLoginFailureResource::class)
+            ->disableOriginalConstructor()->getMock();
+
+        $userRepository
+            ->expects(static::never())
+            ->method(static::anything());
+
+        $logLoginFailureResource
+            ->expects(static::never())
+            ->method(static::anything());
+
+        $event = new AuthenticationFailureEvent(new AuthenticationException(), new Response());
+
+        (new LockedUserSubscriber($userRepository, $logLoginFailureResource))->onAuthenticationFailure($event);
+    }
+
+    /**
+     * @throws Throwable
+     */
+    public function testThatOnAuthenticationFailureTestThatResourceMethodsAreNotCalledWhenWrongUser(): void
+    {
+        /**
+         * @var MockObject|UserRepository          $userRepository
+         * @var MockObject|LogLoginFailureResource $logLoginFailureResource
+         */
+        $userRepository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock();
+        $logLoginFailureResource = $this->getMockBuilder(LogLoginFailureResource::class)
+            ->disableOriginalConstructor()->getMock();
+
+        $token = new UsernamePasswordToken('test-user', 'password', 'providerKey');
+
+        $exception = new AuthenticationException();
+        $exception->setToken($token);
+
+        $userRepository
+            ->expects(static::once())
+            ->method('loadUserByUsername')
+            ->with('test-user')
+            ->willReturn(null);
+
+        $logLoginFailureResource
+            ->expects(static::never())
+            ->method(static::anything());
+
+        $event = new AuthenticationFailureEvent($exception, new Response());
+
+        (new LockedUserSubscriber($userRepository, $logLoginFailureResource))->onAuthenticationFailure($event);
+    }
+
+    /**
+     * @throws Throwable
+     */
+    public function testThatOnAuthenticationFailureTestThatResourceSaveMethodIsCalled(): void
+    {
+        /**
+         * @var MockObject|UserRepository          $userRepository
+         * @var MockObject|LogLoginFailureResource $logLoginFailureResource
+         */
+        $userRepository = $this->getMockBuilder(UserRepository::class)->disableOriginalConstructor()->getMock();
+        $logLoginFailureResource = $this->getMockBuilder(LogLoginFailureResource::class)
+            ->disableOriginalConstructor()->getMock();
+
+        $token = new UsernamePasswordToken('test-user', 'password', 'providerKey');
+
+        $exception = new AuthenticationException();
+        $exception->setToken($token);
+
+        $userRepository
+            ->expects(static::once())
+            ->method('loadUserByUsername')
+            ->with('test-user')
+            ->willReturn(new UserEntity());
+
+        $logLoginFailureResource
+            ->expects(static::once())
+            ->method('save');
+
+        $event = new AuthenticationFailureEvent($exception, new Response());
+
+        (new LockedUserSubscriber($userRepository, $logLoginFailureResource))->onAuthenticationFailure($event);
     }
 }