2FA changes should invalidate session (#1417)

* 2FA changes should invalidate session

* User: invalidate remember me cookie if session buster increases

* Fix 2FA validation on code setup

config/packages/security.yaml

@@ -33,6 +33,7 @@ security:
                 always_remember_me: false
                 secure: '%force_ssl%'
                 lifetime: 31104000 # 1y
+                signature_properties: ['password', 'sessionBuster']
             logout:
                 enable_csrf: true
                 clear_site_data:

src/Controller/UserController.php

@@ -13,6 +13,7 @@
 namespace App\Controller;
 
 use App\Attribute\VarName;
+use App\Entity\TemporaryTwoFactorUser;
 use App\Model\FavoriteManager;
 use App\Entity\Package;
 use App\Entity\Version;
@@ -269,13 +270,14 @@ public function enableTwoFactorAuthAction(Request $req, #[VarName('name')] User
         // Temporarily store this code on the user, as we'll need it there to generate the
         // QR code and to check the confirmation code.  We won't actually save this change
         // until we've confirmed the code
-        $user->setTotpSecret($secret);
+        $temporary2faUser = new TemporaryTwoFactorUser($user, $secret);
 
         $enableRequest = new EnableTwoFactorRequest();
-        $form = $this->createForm(EnableTwoFactorAuthType::class, $enableRequest, ['user' => $user])
+        $form = $this->createForm(EnableTwoFactorAuthType::class, $enableRequest, ['user' => $temporary2faUser])
             ->handleRequest($req);
 
         if ($form->isSubmitted() && $form->isValid()) {
+            $user->setTotpSecret($secret);
             $req->getSession()->remove('2fa_secret');
             $authManager->enableTwoFactorAuth($user, $secret);
             $backupCode = $authManager->generateAndSaveNewBackupCode($user);
@@ -287,7 +289,7 @@ public function enableTwoFactorAuthAction(Request $req, #[VarName('name')] User
         }
 
         $req->getSession()->set('2fa_secret', $secret);
-        $qrContent = $authenticator->getQRContent($user);
+        $qrContent = $authenticator->getQRContent($temporary2faUser);
 
         $qrCode = Builder::create()
             ->writer(new SvgWriter())
@@ -305,6 +307,7 @@ public function enableTwoFactorAuthAction(Request $req, #[VarName('name')] User
             [
                 'user' => $user,
                 'form' => $form,
+                'tfaConfig' => $temporary2faUser->getTotpAuthenticationConfiguration(),
                 'qrCode' => $qrCode->getDataUri(),
             ]
         );

src/Entity/TemporaryTwoFactorUser.php

@@ -0,0 +1,31 @@
+<?php declare(strict_types=1);
+
+namespace App\Entity;
+
+use Scheb\TwoFactorBundle\Model\Totp\TotpConfiguration;
+use Scheb\TwoFactorBundle\Model\Totp\TotpConfigurationInterface;
+use Scheb\TwoFactorBundle\Model\Totp\TwoFactorInterface;
+
+class TemporaryTwoFactorUser implements TwoFactorInterface
+{
+    public function __construct(
+        private readonly User $user,
+        private readonly string $totpSecret,
+    ) {
+    }
+
+    public function isTotpAuthenticationEnabled(): bool
+    {
+        return true;
+    }
+
+    public function getTotpAuthenticationUsername(): string
+    {
+        return $this->user->getTotpAuthenticationUsername();
+    }
+
+    public function getTotpAuthenticationConfiguration(): TotpConfigurationInterface
+    {
+        return new TotpConfiguration($this->totpSecret, TotpConfiguration::ALGORITHM_SHA1, 30, 6);
+    }
+}

src/Entity/User.php

@@ -120,6 +120,9 @@ class User implements UserInterface, TwoFactorInterface, BackupCodeInterface, Eq
     #[ORM\Column(type: 'string', length: 8, nullable: true)]
     private string|null $backupCode = null;
 
+    #[ORM\Column(type: 'smallint', options: ['unsigned' => true, 'default' => 0])]
+    private int $sessionBuster = 0;
+
     public function __construct()
     {
         $this->packages = new ArrayCollection();
@@ -242,6 +245,7 @@ public function getGravatarUrl(): string
     public function setTotpSecret(string|null $secret): void
     {
         $this->totpSecret = $secret;
+        $this->invalidateAllSessions();
     }
 
     public function isTotpAuthenticationEnabled(): bool
@@ -307,6 +311,7 @@ public function __serialize(): array
             $this->id,
             $this->email,
             $this->emailCanonical,
+            'session_buster' => $this->sessionBuster,
         ];
     }
 
@@ -325,6 +330,9 @@ public function __unserialize(array $data): void
             $this->email,
             $this->emailCanonical
         ] = $data;
+
+        // session_buster might not be available yet in the session
+        $this->sessionBuster = $data['session_buster'] ?? 0;
     }
 
     public function eraseCredentials(): void
@@ -502,6 +510,10 @@ public function isEqualTo(UserInterface $user): bool
             return false;
         }
 
+        if ($this->getSessionBuster() !== $user->getSessionBuster()) {
+            return false;
+        }
+
         return (!$this->getPassword() && !$user->getPassword()) || ($this->getPassword() && $user->getPassword() && hash_equals($this->getPassword(), $user->getPassword()));
     }
 
@@ -529,4 +541,14 @@ public function isPasswordRequestExpired(int $ttl): bool
     {
         return !$this->getPasswordRequestedAt() instanceof \DateTime || $this->getPasswordRequestedAt()->getTimestamp() + $ttl <= time();
     }
+
+    public function getSessionBuster(): int
+    {
+        return $this->sessionBuster;
+    }
+
+    public function invalidateAllSessions(): void
+    {
+        $this->sessionBuster = ($this->sessionBuster + 1) % 65535;
+    }
 }

src/Form/Type/EnableTwoFactorAuthType.php

@@ -12,9 +12,9 @@
 
 namespace App\Form\Type;
 
-use App\Entity\User;
 use App\Form\Model\EnableTwoFactorRequest;
 use App\Validator\TwoFactorCode;
+use Scheb\TwoFactorBundle\Model\Totp\TwoFactorInterface;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\TextType;
 use Symfony\Component\Form\FormBuilderInterface;
@@ -39,7 +39,7 @@ public function configureOptions(OptionsResolver $resolver): void
                 'data_class' => EnableTwoFactorRequest::class,
             ])
             ->define('user')
-            ->allowedTypes(User::class)
+            ->allowedTypes(TwoFactorInterface::class)
             ->required();
     }
 }

src/Validator/TwoFactorCode.php

@@ -12,13 +12,13 @@
 
 namespace App\Validator;
 
-use App\Entity\User;
+use Scheb\TwoFactorBundle\Model\Totp\TwoFactorInterface;
 use Symfony\Component\Validator\Constraint;
 
 class TwoFactorCode extends Constraint
 {
     public function __construct(
-        public readonly User $user,
+        public readonly TwoFactorInterface $user,
         public readonly string $message = 'Invalid authenticator code',
     ) {
         parent::__construct();

templates/user/enable_two_factor_auth.html.twig

@@ -38,7 +38,7 @@
             <p class="pull-right two-factor-key">
                 <img src="{{ qrCode }}" height="200" />
                 <br>
-                <small>TOTP Key: <code style="user-select: all">{{ user.totpAuthenticationConfiguration.secret }}</code></small>
+                <small>TOTP Key: <code style="user-select: all">{{ tfaConfig.secret }}</code></small>
             </p>
 
             <h3>Enabling Two-Factor Authentication</h3>