diff --git a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php index 123b381c..5516285e 100644 --- a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php +++ b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php @@ -29,17 +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, @@ -53,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); @@ -113,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; } @@ -122,7 +133,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,11 +146,25 @@ 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); } + $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); 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(); } 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);