diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php index 26499c28..7ea5d98e 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php @@ -8,6 +8,11 @@ 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; +use function method_exists; +use function trigger_error; +use const E_USER_DEPRECATED; /** * @final @@ -21,12 +26,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 +37,40 @@ private function getActiveTwoFactorProviders(AuthenticationContextInterface $con } $activeTwoFactorProviders[] = $providerName; - } - return $activeTwoFactorProviders; - } + 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, + ); + } - public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null - { - $activeTwoFactorProviders = $this->getActiveTwoFactorProviders($context); + if (!method_exists($provider, 'needsPreparation') || $provider->needsPreparation()) { + continue; + } + + $statelessProviders[] = $providerName; + } + + 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..44854bd6 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php @@ -13,6 +13,15 @@ interface TwoFactorProviderInterface */ public function beginAuthentication(AuthenticationContextInterface $context): bool; + /** + * 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; + */ + /** * 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/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 1e7c1f41..b6a33c33 100644 --- a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php +++ b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php @@ -11,22 +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->provider2 = $this->createMock(TwoFactorProviderInterface::class); + $this->provider1 = new MockTwoFactorProvider(needsPreparation: true); + $this->provider2 = new MockTwoFactorProvider(needsPreparation: false); $providerRegistry = $this->createMock(TwoFactorProviderRegistry::class); $providerRegistry @@ -36,6 +35,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); @@ -62,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 @@ -94,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] @@ -155,4 +152,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); + } }