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

.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 %}