Close #213: honor post-success listeners that may block authentication

[giosh94mhz]

Description

Symfony’s standard authentication includes several "post-checks" that run immediately after a successful login, most notably TwoFactorAuthenticator::onAuthenticationSuccess.

Starting with Symfony 7.x and 2fa-bundle 7.x (and possibly others), these listeners can generate a loop-redirect, as described in #213.

The Issue

The root cause is that TwoFactorToken marks a provider as completed during badge verification. If an error occurs after this stage, the entire login process is left in an inconsistent state for several reasons:

1. There is no "current provider," even if the system is still processing a provider (e.g., TOTP).
2. At this point, the stored providers (i.e., getTwoFactorProviders) are empty, so the active token in TokenStorage is inconsistent
3. If authentication fails, the current authentication token is already switched and the previous inconsistent token remains in storage.
4. Subsequent requests then generate a new "prepared providers" list, but no providers are available, triggering the failure loop.

The Solution

During badge verification, providers must not be marked as complete in the TwoFactorToken, as this happens too early in the process. This is critical to keep TwoFactorTokenInterface::getCurrentTwoFactorProvider stable throughout the request and avoid the loop issue.

Instead, we need to identify another point to mark the TwoFactorToken provider as complete—ideally, at the very end of the authentication process.

By examining the Symfony\Component\Security\Http\Authentication\AuthenticatorManager::executeAuthenticator class, I see that the AuthenticationEvents::AUTHENTICATION_SUCCESS event (where UserCheckers and other checks run) is considered part of the standard process, as it is within the try-catch block. Immediately after this event, $authenticator->onAuthenticationSuccess is called, making it the right place to mark the flow as completed.

Therefore, the solution is to mark the current provider for any TwoFactorToken that reaches TwoFactorAuthenticator::onAuthenticationSuccess as completed—not before.

Tests

I ran many tests in my application, which uses both stateful and stateless authenticators, as well as multiple post-authentication locks, and everything seems to work. However, we only have one OTP method, so the code for handling multiple OTP methods and $this->dispatchTwoFactorAuthenticationEvent(TwoFactorAuthenticationEvents::REQUIRE, $request, $token) still needs some testing.

Minor Breaking Changes

I foresee two minor breaking changes:

1. Updating users may experience changes in TwoFactorTokenInterface::getCurrentTwoFactorProvider, e.g. from null to totp (though this is likely a fix rather than an issue, IMO).
2. The TwoFactorTokenInterface::allTwoFactorProvidersAuthenticated method will likely never return true, since the previous token forgot after onSuccessfulAuthentication.

The main struggle with TwoFactorTokenInterface is that the setTwoFactorProviderComplete method actually removes a provider from the token, which can cause glitches. My patch addresses this by requiring the authenticator to estimate the "on complete" status by comparing getTwoFactorProviders and the active providers.

Regarding point 2, I may update the TwoFactorToken inside the authenticator success so that TwoFactorTokenInterface::allTwoFactorProvidersAuthenticated is updated for potential external classes still referencing the token and triggering in post-success events, like TwoFactorAuthenticationEvents::COMPLETE.

In a future release, TwoFactorTokenInterface may need a cleanup or an improvements to avoid this edge cases.

[scheb]

I'm wondering where in the process the provider is marked as completed, if it's no longer happening in AbstractCheckCodeListener.

Should this line actually be un-commented?