Skip to content

Commit

Permalink
Two-Factor: password reset for a user with 2FA enabled should ask for…
Browse files Browse the repository at this point in the history
… the 2FA code
  • Loading branch information
glaubinix committed Nov 17, 2023
1 parent c8e3a66 commit 4e70cd5
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/Controller/ResetPasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand All @@ -124,7 +125,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;
}

Expand Down
29 changes: 29 additions & 0 deletions src/EventListener/ResolvedTwoFactorCodeCredentialsListener.php
Original file line number Diff line number Diff line change
@@ -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;

#[AsEventListener(event: AuthenticationTokenCreatedEvent::class, method: 'onAuthenticationTokenCreated', priority: 512)]
class ResolvedTwoFactorCodeCredentialsListener
{
public function onAuthenticationTokenCreated(AuthenticationTokenCreatedEvent $event): void
{
if ($event->getPassport()->getBadge(ResolvedTwoFactorCodeCredentials::class)) {
$event->getAuthenticatedToken()->setAttribute(TwoFactorAuthenticator::FLAG_2FA_COMPLETE, true);
}
}
}
23 changes: 23 additions & 0 deletions src/Form/ResetPasswordFormType.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,24 @@

namespace App\Form;

use App\Entity\User;
use App\Form\Validation\TwoFactorCodeConstraint;
use App\Validator\Password;
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
->setDefault('user', null)
->setAllowedTypes('user', User::class);
}

public function buildForm(FormBuilderInterface $builder, array $options): void
{
$builder
Expand All @@ -32,5 +43,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 TwoFactorCodeConstraint($options['user']),
],
]);
}
}
}
23 changes: 23 additions & 0 deletions src/Security/Passport/Badge/ResolvedTwoFactorCodeCredentials.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
3 changes: 3 additions & 0 deletions templates/reset_password/reset.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
115 changes: 115 additions & 0 deletions tests/Controller/ResetPasswordControllerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?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 Scheb\TwoFactorBundle\Security\Http\Authenticator\TwoFactorAuthenticator;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\DomCrawler\Crawler;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;

class ResetPasswordControllerTest 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 testResetPassword(): void
{
$user = $this->setupUserWithPasswordResetRequest(false);
$oldPassword = $user->getPassword();

$crawler = $this->client->request('GET', '/reset-password/reset/' . $user->getConfirmationToken());
$this->assertResponseStatusCodeSame(200);

$this->submitPasswordResetFormAndAsserStatusCode($crawler, 'new-password', 302);
$this->assertUserHasNewPassword($user, $oldPassword);
}

public function testResetPasswordWithTwoFactor(): void
{
$user = $this->setupUserWithPasswordResetRequest(true);
$oldPassword = $user->getPassword();

$crawler = $this->client->request('GET', '/reset-password/reset/' . $user->getConfirmationToken());
$this->assertResponseStatusCodeSame(200);

$this->submitPasswordResetFormAndAsserStatusCode($crawler, 'new-password', 422);
$this->submitPasswordResetFormAndAsserStatusCode($crawler, 'new-password', 302, TotpAuthenticatorStub::MOCKED_VALID_CODE);
$this->assertUserHasNewPassword($user, $oldPassword);

$this->assertTrue(self::getContainer()->get(TokenStorageInterface::class)->getToken()?->getAttribute(TwoFactorAuthenticator::FLAG_2FA_COMPLETE));
}

private function setupUserWithPasswordResetRequest(bool $withTwoFactor): User
{
$user = new User;
$user->setEnabled(true);
$user->setUsername('test');
$user->setEmail('[email protected]');
$user->setPassword('testtest');
$user->setApiToken('token');
$user->initializeConfirmationToken();
$user->setPasswordRequestedAt(new \DateTime());

if ($withTwoFactor) {
$user->setTotpSecret('secret');
}

$em = static::getContainer()->get(ManagerRegistry::class)->getManager();
$em->persist($user);
$em->flush();

return $user;
}

private function submitPasswordResetFormAndAsserStatusCode(Crawler $crawler, string $newPassword, int $expectedStatusCode, ?string $mfaCode = null): void
{
$form = $crawler->selectButton('Reset password')->form();
$form->setValues(array_filter([
'reset_password_form[plainPassword]' => $newPassword,
'reset_password_form[twoFactorCode]' => $mfaCode,
]));

$this->client->submit($form);
$this->assertResponseStatusCodeSame($expectedStatusCode);
}

private function assertUserHasNewPassword(User $user, ?string $oldPassword): void
{
$em = static::getContainer()->get(ManagerRegistry::class)->getManager();
$em->clear();

$user = $em->getRepository(User::class)->find($user->getId());
$this->assertNotNull($user);
$this->assertNotSame($oldPassword, $user->getPassword());
}
}

0 comments on commit 4e70cd5

Please sign in to comment.