Make password not optional in PasswordUpdater (#602)

src/Admin/UserAdmin.php

@@ -183,7 +183,7 @@ protected function configureBatchActions($actions): array
      */
     public function prePersist($object): void
     {
-        $this->passwordUpdater->updatePassword($object);
+        $this->passwordUpdater->updatePassword($object, $object->getPlainPassword());
 
         if (null !== $object->hasMailCrypt()) {
             $this->mailCryptKeyHandler->create($object);
@@ -205,7 +205,7 @@ public function preUpdate($object): void
         }
 
         if (!empty($object->getPlainPassword())) {
-            $this->passwordUpdater->updatePassword($object);
+            $this->passwordUpdater->updatePassword($object, $object->getPlainPassword());
         } else {
             $object->updateUpdatedTime();
         }

src/Command/UsersMailCryptCommand.php

@@ -20,11 +20,12 @@ class UsersMailCryptCommand extends Command
     private readonly UserRepository $repository;
 
     public function __construct(
-        EntityManagerInterface $manager,
+        EntityManagerInterface                     $manager,
         private readonly UserAuthenticationHandler $handler,
-        private readonly MailCryptKeyHandler $mailCryptKeyHandler,
-        private readonly int $mailCrypt
-    ) {
+        private readonly MailCryptKeyHandler       $mailCryptKeyHandler,
+        private readonly int                       $mailCrypt
+    )
+    {
         $this->repository = $manager->getRepository(User::class);
         parent::__construct();
     }

src/Command/UsersResetCommand.php

@@ -98,9 +98,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
         $output->write(sprintf("\nResetting user %s ...\n\n", $email));
 
-        // Set new password
-        $user->setPlainPassword($password);
-        $this->passwordUpdater->updatePassword($user);
+        $this->passwordUpdater->updatePassword($user, $password);
 
         // Generate MailCrypt key with new password (overwrites old MailCrypt key)
         if ($user->hasMailCryptSecretBox()) {

src/Controller/AccountController.php

@@ -19,7 +19,8 @@ public function __construct(
         private readonly PasswordUpdater        $passwordUpdater,
         private readonly MailCryptKeyHandler    $mailCryptKeyHandler,
         private readonly EntityManagerInterface $manager,
-    ) {
+    )
+    {
     }
 
     /**
@@ -69,7 +70,7 @@ public function account(Request $request): Response
     private function changePassword(Request $request, User $user, PasswordChange $passwordChange): void
     {
         $user->setPlainPassword($passwordChange->getNewPassword());
-        $this->passwordUpdater->updatePassword($user);
+        $this->passwordUpdater->updatePassword($user, $passwordChange->getNewPassword());
         // Reencrypt the MailCrypt key with new password
         if ($user->hasMailCryptSecretBox()) {
             $this->mailCryptKeyHandler->update($user, $passwordChange->getPassword());

src/Controller/RecoveryController.php

@@ -372,7 +372,7 @@ private function renderResetPasswordForm(User $user, string $recoveryToken): Res
     private function resetPassword(User $user, string $password, string $recoveryToken): string
     {
         $user->setPlainPassword($password);
-        $this->passwordUpdater->updatePassword($user);
+        $this->passwordUpdater->updatePassword($user, $password);
 
         $mailCryptPrivateKey = $this->recoveryTokenHandler->decrypt($user, $recoveryToken);
 

src/DataFixtures/AbstractUserData.php

@@ -16,8 +16,7 @@ abstract class AbstractUserData extends Fixture
     public function __construct(readonly PasswordUpdater $passwordUpdater)
     {
         $user = new User();
-        $user->setPlainPassword(self::PASSWORD);
-        $passwordUpdater->updatePassword($user);
+        $passwordUpdater->updatePassword($user, self::PASSWORD);
         $this->passwordHash = $user->getPassword();
     }
 

src/Handler/DeleteHandler.php

@@ -41,8 +41,7 @@ public function deleteUser(User $user)
 
         // Set password to random new one
         $password = PasswordGenerator::generate();
-        $user->setPlainPassword($password);
-        $this->passwordUpdater->updatePassword($user);
+        $this->passwordUpdater->updatePassword($user, $password);
 
         // Erase recovery token and related fields
         $user->eraseRecoveryStartTime();

src/Handler/RegistrationHandler.php

@@ -15,12 +15,21 @@
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 
-class RegistrationHandler
+readonly class RegistrationHandler
 {
     /**
      * Constructor.
      */
-    public function __construct(private readonly EntityManagerInterface $manager, private readonly DomainGuesser $domainGuesser, private readonly EventDispatcherInterface $eventDispatcher, private readonly PasswordUpdater $passwordUpdater, private readonly MailCryptKeyHandler $mailCryptKeyHandler, private readonly RecoveryTokenHandler $recoveryTokenHandler, private readonly bool $registrationOpen, private readonly bool $mailCrypt)
+    public function __construct(
+        private EntityManagerInterface   $manager,
+        private DomainGuesser            $domainGuesser,
+        private EventDispatcherInterface $eventDispatcher,
+        private PasswordUpdater          $passwordUpdater,
+        private MailCryptKeyHandler      $mailCryptKeyHandler,
+        private RecoveryTokenHandler     $recoveryTokenHandler,
+        private bool                     $registrationOpen,
+        private bool                     $mailCrypt
+    )
     {
     }
 
@@ -37,7 +46,7 @@ public function handle(Registration $registration): void
         $user = $this->buildUser($registration);
 
         // Update password, generate MailCrypt keys, generate recovery token
-        $this->passwordUpdater->updatePassword($user);
+        $this->passwordUpdater->updatePassword($user, $registration->getPlainPassword());
         $this->mailCryptKeyHandler->create($user);
         $this->recoveryTokenHandler->create($user);
 
@@ -63,7 +72,7 @@ public function isRegistrationOpen(): bool
     private function buildUser(Registration $registration): User
     {
         $user = new User();
-        $user->setEmail(strtolower((string) $registration->getEmail()));
+        $user->setEmail(strtolower((string)$registration->getEmail()));
         $user->setPlainPassword($registration->getPlainPassword());
         $user->setRoles([Roles::USER]);
 

src/Helper/AdminPasswordUpdater.php

@@ -29,8 +29,7 @@ public function updateAdminPassword(string $password): void
             $admin->setRoles([Roles::ADMIN]);
             $admin->setDomain($domain);
         }
-        $admin->setPlainPassword($password);
-        $this->updater->updatePassword($admin);
+        $this->updater->updatePassword($admin, $password);
         $this->manager->persist($admin);
         $this->manager->flush();
     }

src/Helper/PasswordGenerator.php

@@ -2,14 +2,12 @@
 
 namespace App\Helper;
 
-use Exception;
-/**
- * Class PasswordGenerator.
- */
+use Random\RandomException;
+
 final class PasswordGenerator
 {
     /**
-     * @throws Exception
+     * @throws RandomException
      */
     public static function generate(int $length = 45): string
     {

src/Helper/PasswordUpdater.php

@@ -5,33 +5,15 @@
 use App\Entity\User;
 use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
 
-/**
- * Class PasswordUpdater.
- */
-class PasswordUpdater
+readonly class PasswordUpdater
 {
-    /**
-     * PasswordUpdater constructor.
-     */
-    public function __construct(private readonly PasswordHasherFactoryInterface $passwordHasherFactory)
+    public function __construct(private PasswordHasherFactoryInterface $passwordHasherFactory)
     {
     }
 
-    public function updatePassword(User $user, string $plainPassword = null): void
+    public function updatePassword(User $user, string $plainPassword): void
     {
-        if (null === $plainPassword) {
-            $plainPassword = $user->getPlainPassword();
-        }
-
-        if (!$plainPassword) {
-            return;
-        }
-
-        $hasher = $this->passwordHasherFactory->getPasswordHasher($user);
-        $hash = $hasher->hash($plainPassword);
-
-        $user->setPassword($hash);
-
+        $user->setPassword($this->passwordHasherFactory->getPasswordHasher($user)->hash($plainPassword));
         $user->updateUpdatedTime();
     }
 }

tests/Behat/FeatureContext.php

@@ -125,8 +125,7 @@ public function theFollowingUserExists(TableNode $table): void
 
                         break;
                     case 'password':
-                        $user->setPlainPassword($value);
-                        $this->passwordUpdater->updatePassword($user);
+                        $this->passwordUpdater->updatePassword($user, $value);
                         break;
                     case 'roles':
                         $roles = explode(',', (string)$value);

tests/Controller/RecoveryControllerTest.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace App\Tests\Controller;
+
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+class RecoveryControllerTest extends WebTestCase
+{
+    public function testVisitRecoveryUnauthenticated()
+    {
+        $client = static::createClient();
+        $client->request('GET', '/recovery');
+
+        $this->assertResponseIsSuccessful();
+    }
+
+    public function testVisitRecoveryWitInvalidRecoveryToken()
+    {
+        $client = static::createClient();
+        $crawler = $client->request('GET', '/recovery');
+
+        $form = $crawler->selectButton('Recover')->form();
+        $form['recovery_process[email]'] = 'user@example.com';
+        $form['recovery_process[recoveryToken]'] = 'invalid-token';
+
+        $client->submit($form);
+
+        $this->assertResponseIsSuccessful();
+        $this->assertSelectorTextContains('div.alert-danger', "This token has an invalid format.");
+    }
+}

tests/Handler/RegistrationHandlerTest.php

@@ -2,6 +2,13 @@
 
 namespace App\Tests\Handler;
 
+use App\Entity\Domain;
+use App\Entity\User;
+use App\Entity\Voucher;
+use App\Enum\Roles;
+use App\Event\Events;
+use App\Event\UserEvent;
+use App\Repository\VoucherRepository;
 use Exception;
 use App\Form\Model\Registration;
 use App\Guesser\DomainGuesser;
@@ -10,23 +17,71 @@
 use App\Handler\RegistrationHandler;
 use App\Helper\PasswordUpdater;
 use Doctrine\ORM\EntityManagerInterface;
-use PHPUnit\Framework\TestCase;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
-class RegistrationHandlerTest extends TestCase
+class RegistrationHandlerTest extends KernelTestCase
 {
     public function testHandleWithDisabledRegistration()
     {
-        $manager = $this->getMockBuilder(EntityManagerInterface::class)->disableOriginalConstructor()->getMock();
-        $domainGuesser = $this->getMockBuilder(DomainGuesser::class)->disableOriginalConstructor()->getMock();
-        $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->disableOriginalConstructor()->getMock();
-        $passwordUpdater = $this->getMockBuilder(PasswordUpdater::class)->disableOriginalConstructor()->getMock();
-        $mailCryptKeyHandler = $this->getMockBuilder(MailCryptKeyHandler::class)->disableOriginalConstructor()->getMock();
-        $recoveryTokenHandler = $this->getMockBuilder(RecoveryTokenHandler::class)->disableOriginalConstructor()->getMock();
-
-        $handler = new RegistrationHandler($manager, $domainGuesser, $eventDispatcher, $passwordUpdater, $mailCryptKeyHandler, $recoveryTokenHandler, false, 2);
+        $handler = new RegistrationHandler(
+            $this->createMock(EntityManagerInterface::class),
+            $this->createMock(DomainGuesser::class),
+            $this->createMock(EventDispatcherInterface::class),
+            $this->createMock(PasswordUpdater::class),
+            $this->createMock(MailCryptKeyHandler::class),
+            $this->createMock(RecoveryTokenHandler::class),
+            false,
+            false
+        );
 
         $this->expectException(Exception::class);
         $handler->handle(new Registration());
     }
+
+    public function testHandleWithEnabledRegistration()
+    {
+        $domain = new Domain();
+        $domainGuesser = $this->createMock(DomainGuesser::class);
+        $domainGuesser->method('guess')->willReturn($domain);
+
+        $voucher = new Voucher();
+        $voucherRepository = $this->createMock(VoucherRepository::class);
+        $voucherRepository->method('findByCode')->willReturn($voucher);
+
+        $manager = $this->createMock(EntityManagerInterface::class);
+        $manager->method('getRepository')->willReturnMap([
+            [Voucher::class, $voucherRepository],
+        ]);
+        $manager->method('persist')->willReturnCallback(function (User $user) use ($voucher, $domain): void {
+            $this->assertEquals("user@example.com", $user->getEmail());
+            $this->assertEquals([Roles::USER], $user->getRoles());
+            $this->assertEquals($domain, $user->getDomain());
+            $this->assertEquals($voucher, $user->getInvitationVoucher());
+            $this->assertFalse($user->getMailCrypt());
+        });
+
+        $eventDispatcher = $this->createMock(EventDispatcherInterface::class);
+        $eventDispatcher->expects($this->once())->method('dispatch')->with($this->isInstanceOf(UserEvent::class), Events::MAIL_ACCOUNT_CREATED);
+
+        $handler = new RegistrationHandler(
+            $manager,
+            $domainGuesser,
+            $eventDispatcher,
+            $this->createMock(PasswordUpdater::class),
+            $this->createMock(MailCryptKeyHandler::class),
+            $this->createMock(RecoveryTokenHandler::class),
+            true,
+            false
+        );
+
+        $registration = new Registration();
+        $registration->setPlainPassword('password');
+        $registration->setEmail('user@example.com');
+        $registration->setVoucher("voucher");
+
+        $handler->handle($registration);
+
+        $this->assertNotNull($voucher->getRedeemedTime());
+    }
 }

tests/Helper/AdminPasswordUpdaterTest.php

@@ -13,6 +13,7 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
+use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
 use Symfony\Component\PasswordHasher\PasswordHasherInterface;
 
 class AdminPasswordUpdaterTest extends TestCase
@@ -37,8 +38,7 @@ public function testUpdateAdminPassword(): void
 
         $adminPasswordUpdater->updateAdminPassword('newpassword');
 
-        self::assertEquals('newpassword', $admin->getPlainPassword());
-        self::assertNotEquals('impossible_login', $admin->getPassword());
+        self::assertEquals('newpassword', $admin->getPassword());
     }
 
     public function getManager($object): MockObject
@@ -65,8 +65,7 @@ public function getManager($object): MockObject
 
     public function getUpdater(): PasswordUpdater
     {
-        $hasher = $this->getMockBuilder(PasswordHasherInterface::class)
-            ->getMock();
+        $hasher = new PlaintextPasswordHasher();
         $passwordHasherFactory = $this->getMockBuilder(PasswordHasherFactoryInterface::class)
             ->getMock();
         $passwordHasherFactory->method('getPasswordHasher')->willReturn($hasher);