On password reset require 2FA code before storing new password (#1403)

* Two-Factor: extract code validation into constraint and add test to enable two-factor

* Two-Factor: password reset for a user with 2FA enabled should ask for the 2FA code

* Apply review feedback:
- better form options
- define listener event on method instead of class
- avoid initializing new totp secret on every request

* TwoFactor: move constraint and validator to existing classes

Author: glaubinix

.env.test

@@ -6,3 +6,4 @@ PANTHER_APP_ENV=panther
 DATABASE_URL="mysql://[email protected]:3306/packagist?serverVersion=8.0.28"
 MAILER_DSN=null://null
 REDIS_URL=redis://localhost/14
+APP_MAILER_FROM_EMAIL=[email protected]

config/services_test.yaml

@@ -13,3 +13,9 @@ services:
     Algolia\AlgoliaSearch\SearchClient:
         public: true
         factory: ['Algolia\AlgoliaSearch\SearchClient', create]
+
+    # stub to replace 2FA code generation
+    App\Tests\Mock\TotpAuthenticatorStub:
+        arguments:
+            $totpFactory: '@scheb_two_factor.security.totp_factory'
+    Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Totp\TotpAuthenticatorInterface: '@App\Tests\Mock\TotpAuthenticatorStub'

src/Controller/ResetPasswordController.php

@@ -16,6 +16,7 @@
 use App\Form\ResetPasswordFormType;
 use App\Form\ResetPasswordRequestFormType;
 use App\Security\BruteForceLoginFormAuthenticator;
+use App\Security\Passport\Badge\ResolvedTwoFactorCodeCredentials;
 use App\Security\UserChecker;
 use Beelab\Recaptcha2Bundle\Recaptcha\RecaptchaException;
 use Beelab\Recaptcha2Bundle\Recaptcha\RecaptchaVerifier;
@@ -98,7 +99,7 @@ public function reset(Request $request, UserPasswordHasherInterface $passwordHas
         }
 
         // The token is valid; allow the user to change their password.
-        $form = $this->createForm(ResetPasswordFormType::class);
+        $form = $this->createForm(ResetPasswordFormType::class, null, ['user' => $user]);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
@@ -123,7 +124,9 @@ public function reset(Request $request, UserPasswordHasherInterface $passwordHas
                 // skip authenticating if any pre-auth check does not pass
             }
 
-            if ($response = $userAuthenticator->authenticateUser($user, $authenticator, $request)) {
+            // A user resetting the password with 2FA enabled, should automatically be marked as 2FA complete
+            $badges = $user->isTotpAuthenticationEnabled() ? [new ResolvedTwoFactorCodeCredentials()] : [];
+            if ($response = $userAuthenticator->authenticateUser($user, $authenticator, $request, $badges)) {
                 return $response;
             }
 

src/Controller/UserController.php

@@ -265,39 +265,28 @@ public function enableTwoFactorAuthAction(Request $req, #[VarName('name')] User
             throw $this->createAccessDeniedException('You cannot change this user\'s two-factor authentication settings');
         }
 
-        $enableRequest = new EnableTwoFactorRequest();
-        $form = $this->createForm(EnableTwoFactorAuthType::class, $enableRequest)
-            ->handleRequest($req);
-
-        $secret = (string) $req->getSession()->get('2fa_secret');
-        if (!$form->isSubmitted() || '' === $secret) {
-            $secret = $authenticator->generateSecret();
-            $req->getSession()->set('2fa_secret', $secret);
-        }
-
+        $secret = (string) $req->getSession()->get('2fa_secret') ?: $authenticator->generateSecret();
         // 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);
 
-        if ($form->isSubmitted()) {
-            // Validate the code using the secret that was submitted in the form
-            if (!$authenticator->checkCode($user, $enableRequest->getCode() ?? '')) {
-                $form->get('code')->addError(new FormError('Invalid authenticator code'));
-            }
+        $enableRequest = new EnableTwoFactorRequest();
+        $form = $this->createForm(EnableTwoFactorAuthType::class, $enableRequest, ['user' => $user])
+            ->handleRequest($req);
 
-            if ($form->isValid()) {
-                $req->getSession()->remove('2fa_secret');
-                $authManager->enableTwoFactorAuth($user, $secret);
-                $backupCode = $authManager->generateAndSaveNewBackupCode($user);
+        if ($form->isSubmitted() && $form->isValid()) {
+            $req->getSession()->remove('2fa_secret');
+            $authManager->enableTwoFactorAuth($user, $secret);
+            $backupCode = $authManager->generateAndSaveNewBackupCode($user);
 
-                $this->addFlash('success', 'Two-factor authentication has been enabled.');
-                $req->getSession()->set('backup_code', $backupCode);
+            $this->addFlash('success', 'Two-factor authentication has been enabled.');
+            $req->getSession()->set('backup_code', $backupCode);
 
-                return $this->redirectToRoute('user_2fa_confirm', ['name' => $user->getUsername()]);
-            }
+            return $this->redirectToRoute('user_2fa_confirm', ['name' => $user->getUsername()]);
         }
 
+        $req->getSession()->set('2fa_secret', $secret);
         $qrContent = $authenticator->getQRContent($user);
 
         $qrCode = Builder::create()

src/EventListener/ResolvedTwoFactorCodeCredentialsListener.php

@@ -0,0 +1,29 @@
+<?php declare(strict_types=1);
+
+/*
+ * This file is part of Packagist.
+ *
+ * (c) Jordi Boggiano <[email protected]>
+ *     Nils Adermann <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\EventListener;
+
+use App\Security\Passport\Badge\ResolvedTwoFactorCodeCredentials;
+use Scheb\TwoFactorBundle\Security\Http\Authenticator\TwoFactorAuthenticator;
+use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
+use Symfony\Component\Security\Http\Event\AuthenticationTokenCreatedEvent;
+
+class ResolvedTwoFactorCodeCredentialsListener
+{
+    #[AsEventListener(event: AuthenticationTokenCreatedEvent::class, priority: 512)]
+    public function onAuthenticationTokenCreated(AuthenticationTokenCreatedEvent $event): void
+    {
+        if ($event->getPassport()->getBadge(ResolvedTwoFactorCodeCredentials::class)) {
+            $event->getAuthenticatedToken()->setAttribute(TwoFactorAuthenticator::FLAG_2FA_COMPLETE, true);
+        }
+    }
+}

src/Form/ResetPasswordFormType.php

@@ -12,13 +12,25 @@
 
 namespace App\Form;
 
+use App\Entity\User;
 use App\Validator\Password;
+use App\Validator\TwoFactorCode;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\PasswordType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
 use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\OptionsResolver\OptionsResolver;
 
 class ResetPasswordFormType extends AbstractType
 {
+    public function configureOptions(OptionsResolver $resolver): void
+    {
+        $resolver
+            ->define('user')
+            ->allowedTypes(User::class)
+            ->required();
+    }
+
     public function buildForm(FormBuilderInterface $builder, array $options): void
     {
         $builder
@@ -32,5 +44,17 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
                 ],
             ])
         ;
+
+        if ($options['user']->isTotpAuthenticationEnabled()) {
+            $builder
+                ->add('twoFactorCode', TextType::class, [
+                    'label' => 'Two-Factor Code',
+                    'required' => true,
+                    'mapped' => false,
+                    'constraints' => [
+                        new TwoFactorCode($options['user']),
+                    ],
+                ]);
+        }
     }
 }

src/Form/Type/EnableTwoFactorAuthType.php

@@ -12,7 +12,9 @@
 
 namespace App\Form\Type;
 
+use App\Entity\User;
 use App\Form\Model\EnableTwoFactorRequest;
+use App\Validator\TwoFactorCode;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\TextType;
 use Symfony\Component\Form\FormBuilderInterface;
@@ -25,13 +27,19 @@ class EnableTwoFactorAuthType extends AbstractType
 {
     public function buildForm(FormBuilderInterface $builder, array $options): void
     {
-        $builder->add('code', TextType::class);
+        $builder->add('code', TextType::class, [
+            'constraints' => [new TwoFactorCode($options['user'])]
+        ]);
     }
 
     public function configureOptions(OptionsResolver $resolver): void
     {
-        $resolver->setDefaults([
-            'data_class' => EnableTwoFactorRequest::class,
-        ]);
+        $resolver
+            ->setDefaults([
+                'data_class' => EnableTwoFactorRequest::class,
+            ])
+            ->define('user')
+            ->allowedTypes(User::class)
+            ->required();
     }
 }

src/Security/Passport/Badge/ResolvedTwoFactorCodeCredentials.php

@@ -0,0 +1,23 @@
+<?php declare(strict_types=1);
+
+/*
+ * This file is part of Packagist.
+ *
+ * (c) Jordi Boggiano <[email protected]>
+ *     Nils Adermann <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Security\Passport\Badge;
+
+use Symfony\Component\Security\Http\Authenticator\Passport\Badge\BadgeInterface;
+
+class ResolvedTwoFactorCodeCredentials implements BadgeInterface
+{
+    public function isResolved(): bool
+    {
+        return true;
+    }
+}

src/Validator/TwoFactorCode.php

@@ -0,0 +1,26 @@
+<?php declare(strict_types=1);
+
+/*
+ * This file is part of Packagist.
+ *
+ * (c) Jordi Boggiano <[email protected]>
+ *     Nils Adermann <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Validator;
+
+use App\Entity\User;
+use Symfony\Component\Validator\Constraint;
+
+class TwoFactorCode extends Constraint
+{
+    public function __construct(
+        public readonly User $user,
+        public readonly string $message = 'Invalid authenticator code',
+    ) {
+        parent::__construct();
+    }
+}

src/Validator/TwoFactorCodeValidator.php

@@ -0,0 +1,34 @@
+<?php declare(strict_types=1);
+
+/*
+ * This file is part of Packagist.
+ *
+ * (c) Jordi Boggiano <[email protected]>
+ *     Nils Adermann <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Validator;
+
+use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Totp\TotpAuthenticatorInterface;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+
+class TwoFactorCodeValidator extends ConstraintValidator
+{
+    public function __construct(
+        private readonly TotpAuthenticatorInterface $totpAuthenticator,
+    ) {}
+
+    /**
+     * @param TwoFactorCode $constraint
+     */
+    public function validate(mixed $value, Constraint $constraint): void
+    {
+        if (!$this->totpAuthenticator->checkCode($constraint->user, (string) $value)) {
+            $this->context->addViolation($constraint->message);
+        }
+    }
+}

templates/reset_password/reset.html.twig

@@ -7,6 +7,9 @@
 
     {{ form_start(resetForm) }}
         {{ form_row(resetForm.plainPassword) }}
+        {% if resetForm.twoFactorCode is defined %}
+            {{ form_row(resetForm.twoFactorCode) }}
+        {% endif %}
         <button class="btn btn-primary">Reset password</button>
     {{ form_end(resetForm) }}
 {% endblock %}