Skip to content

Commit

Permalink
Apply review feedback:
Browse files Browse the repository at this point in the history
- better form options
- define listener event on method instead of class
- avoid initializing new totp secret on every request
  • Loading branch information
glaubinix committed Nov 17, 2023
1 parent 4e70cd5 commit 7c266b1
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/Controller/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public function enableTwoFactorAuthAction(Request $req, #[VarName('name')] User
throw $this->createAccessDeniedException('You cannot change this user\'s two-factor authentication settings');
}

$secret = (string) $req->getSession()->get('2fa_secret', $authenticator->generateSecret());
$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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
use Symfony\Component\Security\Http\Event\AuthenticationTokenCreatedEvent;

#[AsEventListener(event: AuthenticationTokenCreatedEvent::class, method: 'onAuthenticationTokenCreated', priority: 512)]
class ResolvedTwoFactorCodeCredentialsListener
{
#[AsEventListener(event: AuthenticationTokenCreatedEvent::class, priority: 512)]
public function onAuthenticationTokenCreated(AuthenticationTokenCreatedEvent $event): void
{
if ($event->getPassport()->getBadge(ResolvedTwoFactorCodeCredentials::class)) {
Expand Down
5 changes: 3 additions & 2 deletions src/Form/ResetPasswordFormType.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ class ResetPasswordFormType extends AbstractType
public function configureOptions(OptionsResolver $resolver): void
{
$resolver
->setDefault('user', null)
->setAllowedTypes('user', User::class);
->define('user')
->allowedTypes(User::class)
->required();
}

public function buildForm(FormBuilderInterface $builder, array $options): void
Expand Down
5 changes: 3 additions & 2 deletions src/Form/Type/EnableTwoFactorAuthType.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ public function configureOptions(OptionsResolver $resolver): void
$resolver
->setDefaults([
'data_class' => EnableTwoFactorRequest::class,
'user' => null,
])
->setAllowedTypes('user', User::class);
->define('user')
->allowedTypes(User::class)
->required();
}
}

0 comments on commit 7c266b1

Please sign in to comment.