From fb09d0a444022018ce17274a76e7e9226a1c7205 Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Mon, 12 Jan 2026 11:36:22 +0100 Subject: [PATCH 1/2] Allow TwoFactor providers to be stateless Prior to this change, there was no way to determine if a two-factor provider needed preparation before authentication. This change introduces the needsPreparation method in the TwoFactorProviderInterface and its implementations, allowing the system to skip the preparation process for providers that do not require it. The preparation process requires state. For example: Prior to this change, if no state was available, the Totp and Google authenticators would fail, even if they are stateless. Co-authored-by: Tjeerd --- .../Provider/TwoFactorProviderInitiator.php | 45 +++++++++---------- .../Provider/TwoFactorProviderInterface.php | 5 +++ .../Provider/Email/EmailTwoFactorProvider.php | 5 +++ .../GoogleAuthenticatorTwoFactorProvider.php | 5 +++ .../TotpAuthenticatorTwoFactorProvider.php | 5 +++ .../TwoFactorProviderInitiatorTest.php | 29 ++++++++++++ 6 files changed, 71 insertions(+), 23 deletions(-) diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php index 26499c28..12698c08 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php @@ -8,6 +8,8 @@ use Scheb\TwoFactorBundle\Security\Authentication\Token\TwoFactorTokenInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\AuthenticationContextInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Exception\UnknownTwoFactorProviderException; +use function array_walk; +use function count; /** * @final @@ -21,12 +23,9 @@ public function __construct( ) { } - /** - * @return string[] - */ - private function getActiveTwoFactorProviders(AuthenticationContextInterface $context): array + public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null { - $activeTwoFactorProviders = []; + $activeTwoFactorProviders = $statelessProviders = []; // Iterate over two-factor providers and begin the two-factor authentication process. foreach ($this->providerRegistry->getAllProviders() as $providerName => $provider) { @@ -35,32 +34,32 @@ private function getActiveTwoFactorProviders(AuthenticationContextInterface $con } $activeTwoFactorProviders[] = $providerName; - } + if ($provider->needsPreparation()) { + continue; + } - return $activeTwoFactorProviders; - } + $statelessProviders[] = $providerName; + } - public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null - { - $activeTwoFactorProviders = $this->getActiveTwoFactorProviders($context); + if (0 === count($activeTwoFactorProviders)) { + return null; + } $authenticatedToken = $context->getToken(); - if ($activeTwoFactorProviders) { - $twoFactorToken = $this->twoFactorTokenFactory->create($authenticatedToken, $context->getFirewallName(), $activeTwoFactorProviders); + $twoFactorToken = $this->twoFactorTokenFactory->create($authenticatedToken, $context->getFirewallName(), $activeTwoFactorProviders); - $preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context); + array_walk($statelessProviders, static fn (string $providerName) => $twoFactorToken->setTwoFactorProviderPrepared($providerName)); - if (null !== $preferredProvider) { - try { - $twoFactorToken->preferTwoFactorProvider($preferredProvider); - } catch (UnknownTwoFactorProviderException) { - // Bad user input - } - } + $preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context); - return $twoFactorToken; + if (null !== $preferredProvider) { + try { + $twoFactorToken->preferTwoFactorProvider($preferredProvider); + } catch (UnknownTwoFactorProviderException) { + // Bad user input + } } - return null; + return $twoFactorToken; } } diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php index fddb7e7e..e8262a4f 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php @@ -13,6 +13,11 @@ interface TwoFactorProviderInterface */ public function beginAuthentication(AuthenticationContextInterface $context): bool; + /** + * Determine whether this Provider needs to be prepared (if the prepareAuthentication method needs to be called). + */ + public function needsPreparation(): bool; + /** * Do all steps necessary to prepare authentication, e.g. generate & send a code. */ diff --git a/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php b/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php index 73a0104d..984a1d16 100644 --- a/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php +++ b/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php @@ -34,6 +34,11 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo return $user instanceof TwoFactorInterface && $user->isEmailAuthEnabled(); } + public function needsPreparation(): bool + { + return true; + } + public function prepareAuthentication(object $user): void { if (!($user instanceof TwoFactorInterface)) { diff --git a/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php b/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php index f70c1aa4..1e6c9787 100644 --- a/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php +++ b/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php @@ -38,6 +38,11 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo return true; } + public function needsPreparation(): bool + { + return false; + } + public function prepareAuthentication(object $user): void { } diff --git a/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php b/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php index ded02f00..b4ea991a 100644 --- a/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php +++ b/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php @@ -42,6 +42,11 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo return true; } + public function needsPreparation(): bool + { + return false; + } + public function prepareAuthentication(object $user): void { } diff --git a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php index 1e7c1f41..91de2e63 100644 --- a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php +++ b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php @@ -26,7 +26,9 @@ class TwoFactorProviderInitiatorTest extends AbstractAuthenticationContextTestCa protected function setUp(): void { $this->provider1 = $this->createMock(TwoFactorProviderInterface::class); + $this->provider1->method('needsPreparation')->willReturn(true); $this->provider2 = $this->createMock(TwoFactorProviderInterface::class); + $this->provider2->method('needsPreparation')->willReturn(false); $providerRegistry = $this->createMock(TwoFactorProviderRegistry::class); $providerRegistry @@ -36,6 +38,16 @@ protected function setUp(): void 'test1' => $this->provider1, 'test2' => $this->provider2, ]); + $providerRegistry + ->expects($this->any()) + ->method('getProvider') + ->willReturnCallback(function (string $name) { + return match ($name) { + 'test1' => $this->provider1, + 'test2' => $this->provider2, + default => null, + }; + }); $this->twoFactorTokenFactory = $this->createMock(TwoFactorTokenFactory::class); @@ -155,4 +167,21 @@ public function beginAuthentication_hasPreferredProvider_setThatProviderPreferre $this->initiator->beginTwoFactorAuthentication($context); } + + #[Test] + public function beginAuthentication_statelessProviderPrepared_setThatProviderIsPrepared(): void + { + $originalToken = $this->createToken(); + $context = $this->createAuthenticationContext(null, $originalToken); + $this->stubProvidersReturn(true, true); + + $twoFactorToken = $this->createTwoFactorToken(); + $this->stubTwoFactorTokenFactoryReturns($twoFactorToken); + $twoFactorToken + ->expects($this->once()) + ->method('setTwoFactorProviderPrepared') + ->with('test2'); + + $this->initiator->beginTwoFactorAuthentication($context); + } } From 81f603999c19174d3d09674ad4e47cc38e50a17f Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Tue, 13 Jan 2026 08:39:01 +0100 Subject: [PATCH 2/2] Make `needsPreparation` a soft requirement to prevent b/c break Prior to this change, the needsPreparation method would break existing implementations. This change makes the change not break existing TwoFactorProviders by not actually defining the `needsPreparation` in the interface. This is to be implemented in version 9 of the 2fa package. --- .../Provider/TwoFactorProviderInitiator.php | 13 +++- .../Provider/TwoFactorProviderInterface.php | 6 +- .../Provider/MockTwoFactorProvider.php | 64 +++++++++++++++++++ .../TwoFactorProviderInitiatorTest.php | 37 ++++------- 4 files changed, 92 insertions(+), 28 deletions(-) create mode 100644 tests/Security/TwoFactor/Provider/MockTwoFactorProvider.php diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php index 12698c08..7ea5d98e 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php @@ -10,6 +10,9 @@ use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Exception\UnknownTwoFactorProviderException; use function array_walk; use function count; +use function method_exists; +use function trigger_error; +use const E_USER_DEPRECATED; /** * @final @@ -34,7 +37,15 @@ public function beginTwoFactorAuthentication(AuthenticationContextInterface $con } $activeTwoFactorProviders[] = $providerName; - if ($provider->needsPreparation()) { + + if (!method_exists($provider, 'needsPreparation')) { + @trigger_error( + 'Two-factor provider "'.$providerName.'" does not implement needsPreparation() method. This method will be required in the next major version.', + E_USER_DEPRECATED, + ); + } + + if (!method_exists($provider, 'needsPreparation') || $provider->needsPreparation()) { continue; } diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php index e8262a4f..44854bd6 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php @@ -15,8 +15,12 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo /** * Determine whether this Provider needs to be prepared (if the prepareAuthentication method needs to be called). + * + * In version 9, this method will be introduced, and all providers will need to implement it. + * Currently, it will be called, but is not required. + * + * public function needsPreparation(): bool; */ - public function needsPreparation(): bool; /** * Do all steps necessary to prepare authentication, e.g. generate & send a code. diff --git a/tests/Security/TwoFactor/Provider/MockTwoFactorProvider.php b/tests/Security/TwoFactor/Provider/MockTwoFactorProvider.php new file mode 100644 index 00000000..29445dfd --- /dev/null +++ b/tests/Security/TwoFactor/Provider/MockTwoFactorProvider.php @@ -0,0 +1,64 @@ +beginAuthenticationResult = $result; + } + + public function getBeginAuthenticationCallCount(): int + { + return $this->beginAuthenticationCallCount; + } + + public function getLastContext(): AuthenticationContextInterface|null + { + return $this->lastContext; + } + + public function beginAuthentication(AuthenticationContextInterface $context): bool + { + ++$this->beginAuthenticationCallCount; + $this->lastContext = $context; + + return $this->beginAuthenticationResult; + } + + public function needsPreparation(): bool + { + return $this->needsPreparation; + } + + public function prepareAuthentication(object $user): void + { + // Mock implementation + } + + public function validateAuthenticationCode(object $user, string $authenticationCode): bool + { + return false; + } + + public function getFormRenderer(): TwoFactorFormRendererInterface + { + throw new RuntimeException('Not implemented in mock'); + } +} diff --git a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php index 91de2e63..b6a33c33 100644 --- a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php +++ b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php @@ -11,24 +11,21 @@ use Scheb\TwoFactorBundle\Security\Authentication\Token\TwoFactorTokenInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderDeciderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInitiator; -use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Scheb\TwoFactorBundle\Tests\Security\TwoFactor\Condition\AbstractAuthenticationContextTestCase; class TwoFactorProviderInitiatorTest extends AbstractAuthenticationContextTestCase { private MockObject&TwoFactorTokenFactoryInterface $twoFactorTokenFactory; - private MockObject&TwoFactorProviderInterface $provider1; - private MockObject&TwoFactorProviderInterface $provider2; + private MockTwoFactorProvider $provider1; + private MockTwoFactorProvider $provider2; private MockObject&TwoFactorProviderDeciderInterface $providerDecider; private TwoFactorProviderInitiator $initiator; protected function setUp(): void { - $this->provider1 = $this->createMock(TwoFactorProviderInterface::class); - $this->provider1->method('needsPreparation')->willReturn(true); - $this->provider2 = $this->createMock(TwoFactorProviderInterface::class); - $this->provider2->method('needsPreparation')->willReturn(false); + $this->provider1 = new MockTwoFactorProvider(needsPreparation: true); + $this->provider2 = new MockTwoFactorProvider(needsPreparation: false); $providerRegistry = $this->createMock(TwoFactorProviderRegistry::class); $providerRegistry @@ -74,15 +71,8 @@ private function createUserWithPreferredProvider(string $preferredProvider): Moc private function stubProvidersReturn(bool $provider1Returns, bool $provider2Returns): void { - $this->provider1 - ->expects($this->any()) - ->method('beginAuthentication') - ->willReturn($provider1Returns); - - $this->provider2 - ->expects($this->any()) - ->method('beginAuthentication') - ->willReturn($provider2Returns); + $this->provider1->setBeginAuthenticationResult($provider1Returns); + $this->provider2->setBeginAuthenticationResult($provider2Returns); } private function stubTwoFactorTokenFactoryReturns(MockObject $token): void @@ -106,17 +96,12 @@ public function beginAuthentication_multipleProviders_beginAuthenticationOnEachT { $context = $this->createAuthenticationContext(); - $this->provider1 - ->expects($this->once()) - ->method('beginAuthentication') - ->with($context); - - $this->provider2 - ->expects($this->once()) - ->method('beginAuthentication') - ->with($context); - $this->initiator->beginTwoFactorAuthentication($context); + + $this->assertSame(1, $this->provider1->getBeginAuthenticationCallCount()); + $this->assertSame($context, $this->provider1->getLastContext()); + $this->assertSame(1, $this->provider2->getBeginAuthenticationCallCount()); + $this->assertSame($context, $this->provider2->getLastContext()); } #[Test]