diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php index 26499c28..053514e7 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,46 +23,41 @@ public function __construct( ) { } - /** - * @return string[] - */ - private function getActiveTwoFactorProviders(AuthenticationContextInterface $context): array + public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null { - $activeTwoFactorProviders = []; - // Iterate over two-factor providers and begin the two-factor authentication process. + $activeTwoFactorProviders = $statelessProviders = []; foreach ($this->providerRegistry->getAllProviders() as $providerName => $provider) { if (!$provider->beginAuthentication($context)) { continue; } $activeTwoFactorProviders[] = $providerName; + if ($provider->needsPreparation()) { + continue; + } + + $statelessProviders[] = $providerName; } - return $activeTwoFactorProviders; - } + if (0 === count($activeTwoFactorProviders)) { + return null; + } - public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null - { - $activeTwoFactorProviders = $this->getActiveTwoFactorProviders($context); + $twoFactorToken = $this->twoFactorTokenFactory->create($context->getToken(), $context->getFirewallName(), $activeTwoFactorProviders); - $authenticatedToken = $context->getToken(); - if ($activeTwoFactorProviders) { - $twoFactorToken = $this->twoFactorTokenFactory->create($authenticatedToken, $context->getFirewallName(), $activeTwoFactorProviders); + array_walk($statelessProviders, static fn (string $providerName) => $twoFactorToken->setTwoFactorProviderPrepared($providerName)); - $preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context); + $preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context); - if (null !== $preferredProvider) { - try { - $twoFactorToken->preferTwoFactorProvider($preferredProvider); - } catch (UnknownTwoFactorProviderException) { - // Bad user input - } + if (null !== $preferredProvider) { + try { + $twoFactorToken->preferTwoFactorProvider($preferredProvider); + } catch (UnknownTwoFactorProviderException) { + // Bad user input } - - return $twoFactorToken; } - return null; + return $twoFactorToken; } } diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php index fddb7e7e..59e735cc 100644 --- a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php @@ -14,7 +14,12 @@ interface TwoFactorProviderInterface public function beginAuthentication(AuthenticationContextInterface $context): bool; /** - * Do all steps necessary to prepare authentication, e.g. generate & send a code. + * 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. Will only be called when needsPreparation() returns true. */ public function prepareAuthentication(object $user): void; diff --git a/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php b/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php index a5276505..84dc34b0 100644 --- a/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php +++ b/src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php @@ -30,6 +30,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 98ce7be4..206eeb03 100644 --- a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php +++ b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php @@ -25,7 +25,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 @@ -162,4 +164,23 @@ 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(); + $twoFactorToken + ->expects($this->once()) + ->method('setTwoFactorProviderPrepared') + ->with('test2'); + $this->stubTwoFactorTokenFactoryReturns($twoFactorToken); + + $this->initiator->beginTwoFactorAuthentication($context); + } }