Refactor MailCrypt key creation on login

y3n4 · y3n4 · commit 920f794200ff · 2025-04-13T10:45:46.000+02:00
diff --git a/config/services.yaml b/config/services.yaml
@@ -67,8 +67,8 @@ services:
       $mailLocation: '%env(DOVECOT_MAIL_LOCATION)%'
 
   App\Command\VoucherCreateCommand:
-      arguments:
-          $appUrl: "%env(APP_URL)%"
+    arguments:
+      $appUrl: '%env(APP_URL)%'
 
   App\Controller\DovecotController:
     arguments:
@@ -82,6 +82,10 @@ services:
     tags:
       - { name: kernel.event_subscriber }
 
+  App\EventListener\LoginListener:
+    arguments:
+      $mailCryptEnv: '%env(MAIL_CRYPT)%'
+
   App\EventListener\LocaleListener:
     arguments:
       $defaultLocale: '%locale%'
@@ -122,14 +126,14 @@ services:
       $to: '%env(NOTIFICATION_ADDRESS)%'
 
   App\Handler\UserRestoreHandler:
-      arguments:
-          $mailCryptEnv: "%env(MAIL_CRYPT)%"
+    arguments:
+      $mailCryptEnv: '%env(MAIL_CRYPT)%'
 
   App\Handler\RegistrationHandler:
-      public: true
-      arguments:
-          $registrationOpen: "%env(REGISTRATION_OPEN)%"
-          $mailCrypt: '%env(MAIL_CRYPT)%'
+    public: true
+    arguments:
+      $registrationOpen: '%env(REGISTRATION_OPEN)%'
+      $mailCrypt: '%env(MAIL_CRYPT)%'
 
   App\Handler\SuspiciousChildrenHandler:
     arguments:
diff --git a/src/Controller/DovecotController.php b/src/Controller/DovecotController.php
@@ -95,19 +95,6 @@ public function authenticate(
             return $this->json(['message' => self::MESSAGE_AUTHENTICATION_FAILED], Response::HTTP_UNAUTHORIZED);
         }
 
-        // If mailCrypt is enforced for all users, optionally create mailCrypt keypair for user
-        if (
-            $this->mailCrypt === MailCrypt::ENABLED_ENFORCE_ALL_USERS &&
-            false === $user->getMailCryptEnabled() &&
-            null === $user->getMailCryptPublicKey()
-        ) {
-            try {
-                $this->mailCryptKeyHandler->create($user, $request->getPassword(), true);
-            } catch (Exception $exception) {
-                return $this->json(['error' => $exception->getMessage()], Response::HTTP_INTERNAL_SERVER_ERROR);
-            }
-        }
-
         // If mailCrypt is enabled and enabled for user, derive mailCryptPrivateKey
         if ($this->mailCrypt->isAtLeast(MailCrypt::ENABLED_OPTIONAL) && $user->getMailCryptEnabled()) {
             try {
diff --git a/src/Event/LoginEvent.php b/src/Event/LoginEvent.php
@@ -6,20 +6,27 @@
 use App\Traits\UserAwareTrait;
 use Symfony\Contracts\EventDispatcher\Event;
 
-/**
- * Class LoginEvent.
- */
 class LoginEvent extends Event
 {
     use UserAwareTrait;
 
     public const NAME = 'user.login';
 
-    /**
-     * Constructor.
-     */
-    public function __construct(User $user)
+    private string $plainPassword;
+
+    public function getPlainPassword(): string
+    {
+        return $this->plainPassword;
+    }
+
+    public function setPlainPassword(string $plainPassword): void
+    {
+        $this->plainPassword = $plainPassword;
+    }
+
+    public function __construct(User $user, string $plainPassword)
     {
         $this->user = $user;
+        $this->plainPassword = $plainPassword;
     }
 }
diff --git a/src/EventListener/LoginListener.php b/src/EventListener/LoginListener.php
@@ -3,33 +3,59 @@
 namespace App\EventListener;
 
 use App\Entity\User;
+use App\Enum\MailCrypt;
 use App\Event\LoginEvent;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
 use Symfony\Component\Security\Http\SecurityEvents;
+use App\Handler\MailCryptKeyHandler;
 
 readonly class LoginListener implements EventSubscriberInterface
 {
-    public function __construct(private EntityManagerInterface $manager)
-    {
+    private readonly MailCrypt $mailCrypt;
+
+    public function __construct(
+        private EntityManagerInterface $manager,
+        private readonly MailCryptKeyHandler $mailCryptKeyHandler,
+        private readonly int $mailCryptEnv,
+    ) {
+        $this->mailCrypt = MailCrypt::from($this->mailCryptEnv);
     }
 
     public function onSecurityInteractiveLogin(InteractiveLoginEvent $event): void
     {
         $user = $event->getAuthenticationToken()->getUser();
 
         if ($user instanceof User) {
-            $this->handleLogin($user);
+            $password = $event->getRequest()->get('_password');
+            $this->handleLogin($user, $password);
         }
     }
 
-    public function onLogin(LoginEvent $event): void
+    public function onAuthenticationHandlerSuccess(LoginEvent $event): void
+    {
+        $this->handleLogin($event->getUser(), $event->getPlainPassword());
+    }
+
+    private function handleLogin(User $user, ?string $password): void
     {
-        $this->handleLogin($event->getUser());
+        if ($this->mailCrypt === MailCrypt::ENABLED_ENFORCE_ALL_USERS && null !== $password) {
+            $this->enableMailCrypt($user, $password);
+        }
+
+        $this->updateLastLogin($user);
+    }
+
+    private function enableMailCrypt(User $user, string $password): void
+    {
+        if ($user->getMailCryptEnabled() || null !== $user->getMailCryptPublicKey()) {
+            return;
+        }
+        $this->mailCryptKeyHandler->create($user, $password, true);
     }
 
-    private function handleLogin(User $user): void
+    private function updateLastLogin(User $user): void
     {
         $user->updateLastLoginTime();
         $this->manager->persist($user);
@@ -40,7 +66,7 @@ public static function getSubscribedEvents(): array
     {
         return [
             SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
-            LoginEvent::NAME => 'onLogin',
+            LoginEvent::NAME => 'onAuthenticationHandlerSuccess',
         ];
     }
 }
diff --git a/src/Handler/UserAuthenticationHandler.php b/src/Handler/UserAuthenticationHandler.php
@@ -7,26 +7,25 @@
 use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 
-/**
- * Class UserAuthenticationHandler.
- */
 class UserAuthenticationHandler
 {
-    /**
-     * UserAuthenticationHandler constructor.
-     */
-    public function __construct(private readonly PasswordHasherFactoryInterface $passwordHasherFactory, protected EventDispatcherInterface $eventDispatcher)
-    {
-    }
+    public function __construct(
+        private readonly PasswordHasherFactoryInterface
+        $passwordHasherFactory,
+        protected EventDispatcherInterface $eventDispatcher
+    ) {}
 
     public function authenticate(User $user, string $password): ?User
     {
         $hasher = $this->passwordHasherFactory->getPasswordHasher($user);
         if (!$hasher->verify($user->getPassword(), $password)) {
             return null;
         }
-        $this->eventDispatcher->dispatch(new LoginEvent($user), LoginEvent::NAME);
 
+        $this->eventDispatcher->dispatch(
+            new LoginEvent($user, $password),
+            LoginEvent::NAME
+        );
         return $user;
     }
 }
diff --git a/tests/EventListener/LoginListenerTest.php b/tests/EventListener/LoginListenerTest.php
@@ -5,40 +5,84 @@
 use App\Entity\User;
 use App\Event\LoginEvent;
 use App\EventListener\LoginListener;
+use App\Handler\MailCryptKeyHandler;
 use App\Helper\PasswordUpdater;
 use Doctrine\ORM\EntityManagerInterface;
+use PHPUnit\Framework\MockObject\MockBuilder;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
 use Symfony\Component\Security\Http\SecurityEvents;
+use function PHPUnit\Framework\assertCount;
+use function PHPUnit\Framework\assertEquals;
 
 class LoginListenerTest extends TestCase
 {
     private EntityManagerInterface $manager;
     private LoginListener $listener;
+    private LoginListener $listenerMailCrypt;
+    private MailCryptKeyHandler $mailCryptKeyHandler;
 
     public function setUp(): void
     {
         $this->manager = $this->getMockBuilder(EntityManagerInterface::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->listener = new LoginListener($this->manager);
+        $this->passwordUpdater = $this->getMockBuilder(PasswordUpdater::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->mailCryptKeyHandler = $this->getMockBuilder(MailCryptKeyHandler::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->listener = new LoginListener(
+            $this->manager,
+            $this->mailCryptKeyHandler,
+            2
+        );
+        // Enforces creation of mailCrypt on Login
+        $this->listenerMailCrypt = new LoginListener(
+            $this->manager,
+            $this->mailCryptKeyHandler,
+            3
+        );
     }
 
-    public function testOnSecurityInteractiveLogin(): void
+    /**
+     * @dataProvider provider
+     */
+    public function testOnSecurityInteractiveLogin(User $user, bool $createMailCrypt): void
     {
-        $user = new User();
         $this->manager->expects($this->once())->method('flush');
-        $event = $this->getEvent($user);
+        $this->mailCryptKeyHandler->expects($this->never())->method('create');
+
+        $event = $this->getInteractiveEvent($user);
 
         $this->listener->onSecurityInteractiveLogin($event);
     }
 
+    /**
+     * @dataProvider provider
+     */
+    public function testOnSecurityInteractiveLoginMailCrypt(User $user, bool $createMailCrypt): void
+    {
+        $this->manager->expects($this->once())->method('flush');
+
+        if ($createMailCrypt) {
+            $this->mailCryptKeyHandler->expects($this->once())->method('create');
+        } else {
+            $this->mailCryptKeyHandler->expects($this->never())->method('create');
+        }
+
+        $event = $this->getInteractiveEvent($user);
+
+        $this->listenerMailCrypt->onSecurityInteractiveLogin($event);
+    }
+
     /**
      * @return \PHPUnit_Framework_MockObject_MockObject|InteractiveLoginEvent
      */
-    private function getEvent(User $user): InteractiveLoginEvent
+    private function getInteractiveEvent(User $user): InteractiveLoginEvent
     {
         $request = $this->getMockBuilder(Request::class)
             ->disableOriginalConstructor()
@@ -60,12 +104,82 @@ private function getEvent(User $user): InteractiveLoginEvent
         return $event;
     }
 
+
+    /**
+     * @dataProvider provider
+     */
+    public function testOnAuthenticationHandlerSuccess(User $user, bool $createMailCrypt): void
+    {
+        $this->manager->expects($this->once())->method('flush');
+        $this->mailCryptKeyHandler->expects($this->never())->method('create');
+
+        $event = $this->getLoginEvent($user);
+
+        $this->listener->onAuthenticationHandlerSuccess($event);
+    }
+
+    /**
+     * @dataProvider provider
+     */
+    public function testOnAuthenticationHandlerSuccessMailCrypt(User $user, bool $createMailCrypt): void
+    {
+        $this->manager->expects($this->once())->method('flush');
+
+        if ($createMailCrypt) {
+            $this->mailCryptKeyHandler->expects($this->once())->method('create');
+        } else {
+            $this->mailCryptKeyHandler->expects($this->never())->method('create');
+        }
+
+        $event = $this->getLoginEvent($user);
+
+        $this->listenerMailCrypt->onAuthenticationHandlerSuccess($event);
+    }
+
+    /**
+     * @return \PHPUnit_Framework_MockObject_MockObject|LoginEvent
+     */
+    private function getLoginEvent(User $user): LoginEvent
+    {
+        $event = $this->getMockBuilder(LoginEvent::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $event->method('getUser')->willReturn($user);
+        $event->method('getPlainPassword')->willReturn('password');
+
+        return $event;
+    }
+
+    public function provider(): array
+    {
+        return [
+            [$this->getUser(null), true],
+            [$this->getUser(false), true],
+            [$this->getUser(true), false],
+        ];
+    }
+
+    public function getUser(?bool $mailCryptEnabled): User
+    {
+        $user = new User();
+        if ($mailCryptEnabled === false || $mailCryptEnabled === true) {
+            $user->setMailCryptEnabled($mailCryptEnabled);
+        }
+
+        return $user;
+    }
+
+
+
     public function testGetSubscribedEvents(): void
     {
-        $this->assertEquals([
-            SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
-            LoginEvent::NAME => 'onLogin',
-        ],
-            $this->listener::getSubscribedEvents());
+        $this->assertEquals(
+            [
+                SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
+                LoginEvent::NAME => 'onAuthenticationHandlerSuccess',
+            ],
+            $this->listener::getSubscribedEvents()
+        );
     }
 }
