Two-Factor: extract code validation into constraint and add test to e…

…nable two-factor

.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/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/Form/Type/EnableTwoFactorAuthType.php

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

src/Form/Validation/TwoFactorCodeConstraint.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\Form\Validation;
+
+use App\Entity\User;
+use Symfony\Component\Validator\Constraint;
+
+class TwoFactorCodeConstraint extends Constraint
+{
+    public function __construct(
+        public readonly User $user,
+        public readonly string $message = 'Invalid authenticator code',
+    ) {
+        parent::__construct();
+    }
+}

src/Form/Validation/TwoFactorCodeConstraintValidator.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\Form\Validation;
+
+use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Totp\TotpAuthenticatorInterface;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+
+class TwoFactorCodeConstraintValidator extends ConstraintValidator
+{
+    public function __construct(
+        private readonly TotpAuthenticatorInterface $totpAuthenticator,
+    ) {}
+
+    /**
+     * @param TwoFactorCodeConstraint $constraint
+     */
+    public function validate(mixed $value, Constraint $constraint): void
+    {
+        if (!$this->totpAuthenticator->checkCode($constraint->user, (string) $value)) {
+            $this->context->addViolation($constraint->message);
+        }
+    }
+}

tests/Controller/UserControllerTest.php

@@ -0,0 +1,77 @@
+<?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\Tests\Controller;
+
+use App\Entity\User;
+use App\Tests\Mock\TotpAuthenticatorStub;
+use Doctrine\DBAL\Connection;
+use Doctrine\Persistence\ManagerRegistry;
+use Symfony\Bundle\FrameworkBundle\KernelBrowser;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+class UserControllerTest extends WebTestCase
+{
+    private KernelBrowser $client;
+
+    public function setUp(): void
+    {
+        $this->client = self::createClient();
+        $this->client->disableReboot(); // Prevent reboot between requests
+        static::getContainer()->get(Connection::class)->beginTransaction();
+
+        parent::setUp();
+    }
+
+    public function tearDown(): void
+    {
+        static::getContainer()->get(Connection::class)->rollBack();
+
+        parent::tearDown();
+    }
+
+    public function testEnableTwoFactoCode(): void
+    {
+        $user = new User;
+        $user->setEnabled(true);
+        $user->setUsername('test');
+        $user->setEmail('[email protected]');
+        $user->setPassword('testtest');
+        $user->setApiToken('token');
+
+        $em = static::getContainer()->get(ManagerRegistry::class)->getManager();
+        $em->persist($user);
+        $em->flush();
+
+        $this->client->loginUser($user);
+
+        $crawler = $this->client->request('GET', sprintf('/users/%s/2fa/enable', $user->getUsername()));
+        $form = $crawler->selectButton('Enable Two-Factor Authentication')->form();
+        $form->setValues([
+            'enable_two_factor_auth[code]' => 123456,
+        ]);
+
+        $crawler = $this->client->submit($form);
+        $this->assertResponseStatusCodeSame(422);
+
+        $form = $crawler->selectButton('Enable Two-Factor Authentication')->form();
+        $form->setValues([
+            'enable_two_factor_auth[code]' => TotpAuthenticatorStub::MOCKED_VALID_CODE,
+        ]);
+
+        $this->client->submit($form);
+        $this->assertResponseStatusCodeSame(302);
+
+        $em->clear();
+        $this->assertTrue($em->getRepository(User::class)->find($user->getId())->isTotpAuthenticationEnabled());
+    }
+}

tests/Mock/TotpAuthenticatorStub.php

@@ -0,0 +1,42 @@
+<?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\Tests\Mock;
+
+use Scheb\TwoFactorBundle\Model\Totp\TwoFactorInterface;
+use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Totp\TotpAuthenticatorInterface;
+use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Totp\TotpFactory;
+use ParagonIE\ConstantTime\Base32;
+
+class TotpAuthenticatorStub implements TotpAuthenticatorInterface
+{
+    public const MOCKED_VALID_CODE = '999999';
+
+    public function __construct(
+        private readonly TotpFactory $totpFactory,
+    ) {}
+
+    public function checkCode(TwoFactorInterface $user, string $code): bool
+    {
+        return $code === self::MOCKED_VALID_CODE;
+    }
+
+    public function getQRContent(TwoFactorInterface $user): string
+    {
+        return $this->totpFactory->createTotpForUser($user)->getProvisioningUri();
+    }
+
+    public function generateSecret(): string
+    {
+        return Base32::encodeUpperUnpadded(random_bytes(32));
+    }
+}