From 951ee9aedc0f4a513949fc16ca5d3b9da64d6fe1 Mon Sep 17 00:00:00 2001 From: Giorgio Premi Date: Fri, 15 May 2026 12:46:47 +0200 Subject: [PATCH 1/3] Unit tests: fixed expections for Token attributes --- .../TwoFactorAuthenticatorTest.php | 35 +++++++++++++++---- .../Event/AuthenticationTokenListenerTest.php | 2 +- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php b/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php index f8bc7481..0fe54237 100644 --- a/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php +++ b/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php @@ -4,6 +4,7 @@ namespace Scheb\TwoFactorBundle\Tests\Security\Http\Authenticator; +use InvalidArgumentException; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -30,8 +31,10 @@ use Symfony\Component\Security\Http\Authenticator\Passport\Badge\RememberMeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use function array_key_exists; use function assert; use function method_exists; +use function sprintf; class TwoFactorAuthenticatorTest extends TestCase { @@ -193,12 +196,30 @@ private function stubIsCheckPath(bool $isCheckPath): void ->willReturn($isCheckPath); } - private function expect2faCompleteFlagSet(MockObject $authenticatedToken): void + private function stubAttributes(MockObject $authenticatedToken): void { + $attributes = []; $authenticatedToken - ->expects($this->any()) ->method('setAttribute') - ->with(TwoFactorAuthenticator::FLAG_2FA_COMPLETE, true); + ->willReturnCallback(static function ($name, $value) use (&$attributes) { + $attributes[$name] = $value; + + return null; + }); + $authenticatedToken + ->method('getAttribute') + ->willReturnCallback(static function ($name) use (&$attributes) { + return array_key_exists($name, $attributes); + }); + $authenticatedToken + ->method('getAttribute') + ->willReturnCallback(static function ($name) use (&$attributes) { + if (!array_key_exists($name, $attributes)) { + throw new InvalidArgumentException(sprintf('This token has no "%s" attribute.', $name)); + } + + return $attributes[$name]; + }); } #[Test] @@ -347,10 +368,11 @@ public function createToken_multiFactorAuthenticationIsComplete_returnAuthentica $twoFactorToken = $this->createTwoFactorToken($authenticatedToken, true); $passport = $this->createPassportWithTwoFactorCredentials($twoFactorToken); - $this->expect2faCompleteFlagSet($authenticatedToken); - + $this->stubAttributes($authenticatedToken); $returnValue = $this->authenticator->createToken($passport, self::FIREWALL_NAME); + $this->assertSame($authenticatedToken, $returnValue); + $this->assertTrue($authenticatedToken->getAttribute(TwoFactorAuthenticator::FLAG_2FA_COMPLETE)); } #[Test] @@ -361,10 +383,11 @@ public function createToken_noMultiFactorAuthentication_returnAuthenticatedToken $twoFactorToken = $this->createTwoFactorToken($authenticatedToken, false); $passport = $this->createPassportWithTwoFactorCredentials($twoFactorToken); - $this->expect2faCompleteFlagSet($authenticatedToken); + $this->stubAttributes($authenticatedToken); $returnValue = $this->authenticator->createToken($passport, self::FIREWALL_NAME); $this->assertSame($authenticatedToken, $returnValue); + $this->assertTrue($authenticatedToken->getAttribute(TwoFactorAuthenticator::FLAG_2FA_COMPLETE)); } #[Test] diff --git a/tests/Security/TwoFactor/Event/AuthenticationTokenListenerTest.php b/tests/Security/TwoFactor/Event/AuthenticationTokenListenerTest.php index 7dc5b7a7..88c827d7 100644 --- a/tests/Security/TwoFactor/Event/AuthenticationTokenListenerTest.php +++ b/tests/Security/TwoFactor/Event/AuthenticationTokenListenerTest.php @@ -115,7 +115,7 @@ public function onAuthenticationTokenCreated_tokenFlagged2faComplete_notChangeTo $this->stubTwoFactorConditionsFulfilled(true); $authenticatedToken = $this->createMock(TokenInterface::class); $authenticatedToken - ->expects($this->any()) + ->expects($this->atLeastOnce()) ->method('hasAttribute') ->with(TwoFactorAuthenticator::FLAG_2FA_COMPLETE) ->willReturn(true); From 6adf4cb3e946bed895fdf79a6fd12a777fa78e5c Mon Sep 17 00:00:00 2001 From: Giorgio Premi Date: Fri, 15 May 2026 12:46:47 +0200 Subject: [PATCH 2/3] Close #213: honor post-success listeners that may block authentication A provider is marked as resolved only during TwoFactorAuthenticator::onAuthenticationSuccess that is called after the whole Symfony late authentication checks are completed. This will restore the functionality of UserCheckerInterface::checkPostAuth that may block the authentication based on post-login user constraints. --- .../Http/Authenticator/TwoFactorAuthenticator.php | 11 ++++++++++- .../Http/EventListener/AbstractCheckCodeListener.php | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php index 123b381c..9688a11f 100644 --- a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php +++ b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php @@ -31,6 +31,7 @@ use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; use function assert; use function class_exists; +use function count; /** * @final @@ -122,7 +123,10 @@ public function createToken(Passport $passport, string $firewallName): TokenInte private function isAuthenticationComplete(TwoFactorTokenInterface $token): bool { - return !$this->twoFactorFirewallConfig->isMultiFactor() || $token->allTwoFactorProvidersAuthenticated(); + return !$this->twoFactorFirewallConfig->isMultiFactor() + || $token->allTwoFactorProvidersAuthenticated() + // The current provider is the last provider. It will be completed in onAuthenticationSuccess. + || 1 === count($token->getTwoFactorProviders()); } public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): Response|null @@ -132,6 +136,11 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token, // When it's still a TwoFactorTokenInterface, keep showing the auth form if ($token instanceof TwoFactorTokenInterface) { + $currentProvider = $token->getCurrentTwoFactorProvider(); + if (null !== $currentProvider) { + $token->setTwoFactorProviderComplete($currentProvider); + } + $this->dispatchTwoFactorAuthenticationEvent(TwoFactorAuthenticationEvents::REQUIRE, $request, $token); return $this->authenticationRequiredHandler->onAuthenticationRequired($request, $token); diff --git a/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php b/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php index 5564520f..9b6d1486 100644 --- a/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php +++ b/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php @@ -50,7 +50,7 @@ public function checkPassport(CheckPassportEvent $event): void return; } - $token->setTwoFactorProviderComplete($providerName); + // Badge is resolved, but authentication for provider is not complete yet $credentialsBadge->markResolved(); } From a26ff7b8acf145c7058ae73584c0878139b02880 Mon Sep 17 00:00:00 2001 From: Giorgio Premi Date: Fri, 15 May 2026 12:46:47 +0200 Subject: [PATCH 3/3] Issue #213: set token as completed right before the COMPLETE event Use a temporary map on TwoFactorAuthenticator to remember the on completion TwoFactorToken, so that it can be set as completed. This does not stricly affect the login, however the COMPLETE event and some other services remembering the token will benefit of the update. --- .../Authenticator/TwoFactorAuthenticator.php | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php index 9688a11f..5516285e 100644 --- a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php +++ b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php @@ -29,18 +29,22 @@ use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Symfony\Contracts\Service\ResetInterface; use function assert; use function class_exists; use function count; +use function spl_object_hash; /** * @final */ -class TwoFactorAuthenticator implements AuthenticatorInterface, InteractiveAuthenticatorInterface +class TwoFactorAuthenticator implements AuthenticatorInterface, InteractiveAuthenticatorInterface, ResetInterface { public const FLAG_2FA_COMPLETE = '2fa_complete'; private readonly LoggerInterface $logger; + /** @var TwoFactorTokenInterface[] */ + private array $onCompleteTokens = []; public function __construct( private readonly TwoFactorFirewallConfig $twoFactorFirewallConfig, @@ -54,6 +58,11 @@ public function __construct( $this->logger = $logger ?? new NullLogger(); } + public function reset(): void + { + $this->onCompleteTokens = []; + } + public function supports(Request $request): bool|null { return $this->twoFactorFirewallConfig->isCheckPathRequest($request); @@ -114,6 +123,7 @@ public function createToken(Passport $passport, string $firewallName): TokenInte if ($this->isAuthenticationComplete($twoFactorToken)) { $authenticatedToken = $twoFactorToken->getAuthenticatedToken(); // Authentication complete, unwrap the token $authenticatedToken->setAttribute(self::FLAG_2FA_COMPLETE, true); + $this->onCompleteTokens[spl_object_hash($authenticatedToken)] = $twoFactorToken; return $authenticatedToken; } @@ -146,6 +156,15 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token, return $this->authenticationRequiredHandler->onAuthenticationRequired($request, $token); } + $twoFactorToken = $this->onCompleteTokens[spl_object_hash($token)] ?? null; + if (null !== $twoFactorToken) { + unset($this->onCompleteTokens[spl_object_hash($token)]); + $currentProvider = $twoFactorToken->getCurrentTwoFactorProvider(); + if (null !== $currentProvider) { + $twoFactorToken->setTwoFactorProviderComplete($currentProvider); + } + } + $this->dispatchTwoFactorAuthenticationEvent(TwoFactorAuthenticationEvents::COMPLETE, $request, $token); return $this->successHandler->onAuthenticationSuccess($request, $token);