Remove Support for legacy Password Hashers

CHANGELOG.md

@@ -1,4 +1,8 @@
-# 3.2.0 (UNRELEASED)
+# 3.3.0 (UNRELEASED)
+
+* Remove legacy password hash support (drop field `password_version` from `virtual_users` table)
+
+# 3.2.0 (2023.03.30)
 
 * Add Command to export metrics to Prometheus
 

config/doctrine/User.orm.yml

@@ -33,8 +33,6 @@ App\Entity\User:
     lastLoginTime:
       type: datetime
       nullable: true
-    passwordVersion:
-      type: integer
     recoverySecretBox:
       type: text
       nullable: true

config/packages/security.yaml

@@ -5,8 +5,6 @@ security:
     password_hashers:
         App\Entity\User:
             algorithm: sodium
-        legacy:
-            id: 'App\Security\Encoder\LegacyPasswordHasher'
 
     # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
     providers:
@@ -124,19 +122,31 @@ security:
     # Easy way to control access for large sections of your site
     # Note: Only the *first* access control that matches will be used
     access_control:
-      - { path: ^/login, roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: ^/logout, roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: "^/[a-z]{2}/init", roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: "^/[a-z]{2}/login", roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: "^/[a-z]{2}/logout", roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: "^/[a-z]{2}/recovery", roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: "^/[a-z]{2}/register", roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: "^/[a-z]{2}/$", roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: ^/$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
-      - { path: ^/2fa, roles: IS_AUTHENTICATED_2FA_IN_PROGRESS }
-      - { path: "^/[a-z]{2}/2fa", roles: IS_AUTHENTICATED_2FA_IN_PROGRESS }
-      - { path: ^/admin, roles: ROLE_DOMAIN_ADMIN }
-      - { path: "^/[a-z]{2}/voucher", roles: ROLE_USER, allow_if: "!is_granted('ROLE_SUSPICIOUS')"}
-      - { path: "^/[a-z]{2}/alias", roles: ROLE_USER, allow_if: "!is_granted('ROLE_SPAM')"}
-      - { path: "^/[a-z]{2}/account", roles: ROLE_USER, allow_if: "!is_granted('ROLE_SPAM')"}
-      - { path: ^/, roles: ROLE_USER }
+        - { path: ^/login, roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: ^/logout, roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "^/[a-z]{2}/init", roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "^/[a-z]{2}/login", roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "^/[a-z]{2}/logout", roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "^/[a-z]{2}/recovery", roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "^/[a-z]{2}/register", roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: "^/[a-z]{2}/$", roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: ^/$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
+        - { path: ^/2fa, roles: IS_AUTHENTICATED_2FA_IN_PROGRESS }
+        - { path: "^/[a-z]{2}/2fa", roles: IS_AUTHENTICATED_2FA_IN_PROGRESS }
+        - { path: ^/admin, roles: ROLE_DOMAIN_ADMIN }
+        - {
+              path: "^/[a-z]{2}/voucher",
+              roles: ROLE_USER,
+              allow_if: "!is_granted('ROLE_SUSPICIOUS')",
+          }
+        - {
+              path: "^/[a-z]{2}/alias",
+              roles: ROLE_USER,
+              allow_if: "!is_granted('ROLE_SPAM')",
+          }
+        - {
+              path: "^/[a-z]{2}/account",
+              roles: ROLE_USER,
+              allow_if: "!is_granted('ROLE_SPAM')",
+          }
+        - { path: ^/, roles: ROLE_USER }

src/Entity/User.php

@@ -15,7 +15,6 @@
 use App\Traits\MailCryptTrait;
 use App\Traits\OpenPgpKeyTrait;
 use App\Traits\PasswordTrait;
-use App\Traits\PasswordVersionTrait;
 use App\Traits\PlainMailCryptPrivateKeyTrait;
 use App\Traits\PlainPasswordTrait;
 use App\Traits\PlainRecoveryTokenTrait;
@@ -46,7 +45,6 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface, Passwor
     use PlainPasswordTrait;
     use DomainAwareTrait;
     use LastLoginTimeTrait;
-    use PasswordVersionTrait;
     use RecoverySecretBoxTrait;
     use PlainRecoveryTokenTrait;
     use RecoveryStartTimeTrait;
@@ -58,8 +56,6 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface, Passwor
     use TwofactorTrait;
     use TwofactorBackupCodeTrait;
 
-    public const CURRENT_PASSWORD_VERSION = 2;
-
     private array $roles = [];
 
     /**
@@ -68,7 +64,6 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface, Passwor
     public function __construct()
     {
         $this->deleted = false;
-        $this->passwordVersion = self::CURRENT_PASSWORD_VERSION;
         $currentDateTime = new \DateTime();
         $this->creationTime = $currentDateTime;
         $this->updatedTime = $currentDateTime;
@@ -113,7 +108,8 @@ public function getUsername(): ?string
     /**
      * @return string
      */
-    public function getUserIdentifier(): string {
+    public function getUserIdentifier(): string
+    {
         return $this->email;
     }
 
@@ -122,10 +118,6 @@ public function getUserIdentifier(): string {
      */
     public function getPasswordHasherName(): ?string
     {
-        if ($this->getPasswordVersion() < self::CURRENT_PASSWORD_VERSION) {
-            return 'legacy';
-        }
-
         // use default encoder
         return null;
     }

src/EventListener/LoginListener.php

@@ -4,7 +4,6 @@
 
 use App\Entity\User;
 use App\Event\LoginEvent;
-use App\Helper\PasswordUpdater;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
@@ -13,24 +12,24 @@
 class LoginListener implements EventSubscriberInterface
 {
     private EntityManagerInterface $manager;
-    private PasswordUpdater $passwordUpdater;
 
     /**
      * LoginListener constructor.
      */
-    public function __construct(EntityManagerInterface $manager, PasswordUpdater $passwordUpdater)
+    public function __construct(EntityManagerInterface $manager)
     {
         $this->manager = $manager;
-        $this->passwordUpdater = $passwordUpdater;
     }
 
     public function onSecurityInteractiveLogin(InteractiveLoginEvent $event): void
     {
         $request = $event->getRequest();
-        /** @var User $user */
+        /** @var User|null $user */
         $user = $event->getAuthenticationToken()->getUser();
-        $user->setPlainPassword($request->get('_password'));
-        $this->handleLogin($user);
+        if ($user !== null && $request->get('_password') !== null) {
+            $user->setPlainPassword($request->get('_password'));
+            $this->handleLogin($user);
+        }
     }
 
     public function onLogin(LoginEvent $event): void
@@ -40,11 +39,6 @@ public function onLogin(LoginEvent $event): void
 
     private function handleLogin(User $user): void
     {
-        // update password hash if necessary
-        if (($user->getPasswordVersion() < User::CURRENT_PASSWORD_VERSION) && null !== $plainPassword = $user->getPlainPassword()) {
-            $user->setPasswordVersion(User::CURRENT_PASSWORD_VERSION);
-            $this->passwordUpdater->updatePassword($user, $plainPassword);
-        }
         $this->updateLastLogin($user);
     }
 

tests/Entity/UserTest.php

@@ -50,8 +50,6 @@ public function testGetPasswordHasherName(): void
     {
         $user = new User();
         self::assertEquals(null, $user->getPasswordHasherName());
-        $user->setPasswordVersion(1);
-        self::assertEquals('legacy', $user->getPasswordHasherName());
     }
 
     public function testPlainPassword(): void

tests/EventListener/LoginListenerTest.php

@@ -31,74 +31,14 @@ public function setUp(): void
         $this->listener = new LoginListener($this->manager, $this->passwordUpdater);
     }
 
-    /**
-     * @dataProvider provider
-     */
-    public function testOnSecurityInteractiveLogin(User $user, bool $update): void
-    {
-        $this->manager->expects($this->once())->method('flush');
-
-        if ($update) {
-            $this->passwordUpdater->expects($this->once())->method('updatePassword');
-        } else {
-            $this->passwordUpdater->expects($this->never())->method('updatePassword');
-        }
-
-        $event = $this->getEvent($user);
-
-        $this->listener->onSecurityInteractiveLogin($event);
-    }
-
-    /**
-     * @return \PHPUnit_Framework_MockObject_MockObject|InteractiveLoginEvent
-     */
-    private function getEvent(User $user): InteractiveLoginEvent
-    {
-        $request = $this->getMockBuilder(Request::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $request->method('get')->willReturn('password');
-
-        $token = $this->getMockBuilder(TokenInterface::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $token->method('getUser')->willReturn($user);
-
-        $event = $this->getMockBuilder(InteractiveLoginEvent::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        $event->method('getRequest')->willReturn($request);
-        $event->method('getAuthenticationToken')->willReturn($token);
-
-        return $event;
-    }
-
-    public function provider(): array
-    {
-        return [
-            [$this->getUser(null), true],
-            [$this->getUser(0), true],
-            [$this->getUser(1), true],
-            [$this->getUser(2), false],
-            [$this->getUser(3), false],
-        ];
-    }
-
-    public function getUser(?int $passwordVersion): User
-    {
-        $user = new User();
-        $user->setPasswordVersion($passwordVersion);
-
-        return $user;
-    }
-
     public function testGetSubscribedEvents(): void
     {
-        $this->assertEquals([
-            SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
-            LoginEvent::class => 'onLogin',
-        ],
-            $this->listener::getSubscribedEvents());
+        $this->assertEquals(
+            [
+                SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
+                LoginEvent::class => 'onLogin',
+            ],
+            $this->listener::getSubscribedEvents()
+        );
     }
 }

tests/Helper/PasswordUpdaterTest.php

@@ -4,7 +4,6 @@
 
 use App\Entity\User;
 use App\Helper\PasswordUpdater;
-use App\Security\Encoder\LegacyPasswordHasher;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
 use Symfony\Component\PasswordHasher\PasswordHasherInterface;