scheb / 2fa

Two-factor authentication for Symfony applications 🔐
MIT License
495 stars 72 forks source link

Infinite redirection loop problem #213

Open secit-pl opened 8 months ago

secit-pl commented 8 months ago

Bundle version: 7.0.0 Symfony version: 7.0.1 PHP version: 8.2.10

Description

Throwing an exception in user_checker checkPostAuth() ends in infinite redirection loop.

To Reproduce

  1. Configure bundle using email tokens.
  2. Add user checker in security.yaml
    security:
    firewalls:
    website:
      lazy: true
      provider: app_user_provider
      user_checker: App\Security\UserChecker
      ...
  3. App\Security\UserChecker
    
    <?php

namespace App\Security;

use Symfony\Component\Security\Core\Exception\CredentialsExpiredException; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface;

class UserChecker implements UserCheckerInterface { public function checkPreAuth(UserInterface $user): void { }

public function checkPostAuth(UserInterface $user): void
{
    throw new CredentialsExpiredException();
}

}



3. Now login as usual, enter the code from the mail, submit form and you will get the infinite redirection loop.

<img width="666" alt="image" src="https://github.com/scheb/2fa/assets/24696884/60d7c12f-5ce2-4226-8143-c3c8f30c608d">
scheb commented 8 months ago

I can reproduce this behavior, but I don't yet understand where this is originating from.

My recommendation would be to move your check to the checkPreAuth phase, so it's already evaluated on the initial login. Due to 2fa, the checkPostAuth is delayed until the 2fa process has been complete and that's where it's causing the issues.

scheb commented 8 months ago

Checker's checkPostAuth check is executed on the AuthenticationSuccessEvent, which happens once the TwoFactorToken was unwrapped. The original authenticated token is about to become the new security token, though by that point it's not written back into the token storage yet.

When an exception happens on the AuthenticationSuccessEvent, the TwoFactorToken is never replaced in the token storage and stays there forever.

The exception causes the DefaultAuthenticationFailureHandler to trigger, redirects back to the 2fa form to display the error.

2fa form controller receives a TwoFactorToken without any 2fa providers available (all of them have been completed), so it responds with an AccessDeniedException. The AccessDeniedException in combination with a TwoFactorToken redirects back to the 2fa form. Repeat.


No idea how this can be properly handled. Most reasonable approach imo would be to move checks to checkPreAuth, to ensure the user is validated before 2fa even kicks in.