Skip to content

Commit fb09d0a

Browse files
johanibTjeerd
andcommitted
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 <tjeerd@ibuildings.nl>
1 parent c90c685 commit fb09d0a

6 files changed

Lines changed: 71 additions & 23 deletions

File tree

src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInitiator.php

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use Scheb\TwoFactorBundle\Security\Authentication\Token\TwoFactorTokenInterface;
99
use Scheb\TwoFactorBundle\Security\TwoFactor\AuthenticationContextInterface;
1010
use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Exception\UnknownTwoFactorProviderException;
11+
use function array_walk;
12+
use function count;
1113

1214
/**
1315
* @final
@@ -21,12 +23,9 @@ public function __construct(
2123
) {
2224
}
2325

24-
/**
25-
* @return string[]
26-
*/
27-
private function getActiveTwoFactorProviders(AuthenticationContextInterface $context): array
26+
public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null
2827
{
29-
$activeTwoFactorProviders = [];
28+
$activeTwoFactorProviders = $statelessProviders = [];
3029

3130
// Iterate over two-factor providers and begin the two-factor authentication process.
3231
foreach ($this->providerRegistry->getAllProviders() as $providerName => $provider) {
@@ -35,32 +34,32 @@ private function getActiveTwoFactorProviders(AuthenticationContextInterface $con
3534
}
3635

3736
$activeTwoFactorProviders[] = $providerName;
38-
}
37+
if ($provider->needsPreparation()) {
38+
continue;
39+
}
3940

40-
return $activeTwoFactorProviders;
41-
}
41+
$statelessProviders[] = $providerName;
42+
}
4243

43-
public function beginTwoFactorAuthentication(AuthenticationContextInterface $context): TwoFactorTokenInterface|null
44-
{
45-
$activeTwoFactorProviders = $this->getActiveTwoFactorProviders($context);
44+
if (0 === count($activeTwoFactorProviders)) {
45+
return null;
46+
}
4647

4748
$authenticatedToken = $context->getToken();
48-
if ($activeTwoFactorProviders) {
49-
$twoFactorToken = $this->twoFactorTokenFactory->create($authenticatedToken, $context->getFirewallName(), $activeTwoFactorProviders);
49+
$twoFactorToken = $this->twoFactorTokenFactory->create($authenticatedToken, $context->getFirewallName(), $activeTwoFactorProviders);
5050

51-
$preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context);
51+
array_walk($statelessProviders, static fn (string $providerName) => $twoFactorToken->setTwoFactorProviderPrepared($providerName));
5252

53-
if (null !== $preferredProvider) {
54-
try {
55-
$twoFactorToken->preferTwoFactorProvider($preferredProvider);
56-
} catch (UnknownTwoFactorProviderException) {
57-
// Bad user input
58-
}
59-
}
53+
$preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context);
6054

61-
return $twoFactorToken;
55+
if (null !== $preferredProvider) {
56+
try {
57+
$twoFactorToken->preferTwoFactorProvider($preferredProvider);
58+
} catch (UnknownTwoFactorProviderException) {
59+
// Bad user input
60+
}
6261
}
6362

64-
return null;
63+
return $twoFactorToken;
6564
}
6665
}

src/bundle/Security/TwoFactor/Provider/TwoFactorProviderInterface.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ interface TwoFactorProviderInterface
1313
*/
1414
public function beginAuthentication(AuthenticationContextInterface $context): bool;
1515

16+
/**
17+
* Determine whether this Provider needs to be prepared (if the prepareAuthentication method needs to be called).
18+
*/
19+
public function needsPreparation(): bool;
20+
1621
/**
1722
* Do all steps necessary to prepare authentication, e.g. generate & send a code.
1823
*/

src/email/Security/TwoFactor/Provider/Email/EmailTwoFactorProvider.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo
3434
return $user instanceof TwoFactorInterface && $user->isEmailAuthEnabled();
3535
}
3636

37+
public function needsPreparation(): bool
38+
{
39+
return true;
40+
}
41+
3742
public function prepareAuthentication(object $user): void
3843
{
3944
if (!($user instanceof TwoFactorInterface)) {

src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo
3838
return true;
3939
}
4040

41+
public function needsPreparation(): bool
42+
{
43+
return false;
44+
}
45+
4146
public function prepareAuthentication(object $user): void
4247
{
4348
}

src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ public function beginAuthentication(AuthenticationContextInterface $context): bo
4242
return true;
4343
}
4444

45+
public function needsPreparation(): bool
46+
{
47+
return false;
48+
}
49+
4550
public function prepareAuthentication(object $user): void
4651
{
4752
}

tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ class TwoFactorProviderInitiatorTest extends AbstractAuthenticationContextTestCa
2626
protected function setUp(): void
2727
{
2828
$this->provider1 = $this->createMock(TwoFactorProviderInterface::class);
29+
$this->provider1->method('needsPreparation')->willReturn(true);
2930
$this->provider2 = $this->createMock(TwoFactorProviderInterface::class);
31+
$this->provider2->method('needsPreparation')->willReturn(false);
3032

3133
$providerRegistry = $this->createMock(TwoFactorProviderRegistry::class);
3234
$providerRegistry
@@ -36,6 +38,16 @@ protected function setUp(): void
3638
'test1' => $this->provider1,
3739
'test2' => $this->provider2,
3840
]);
41+
$providerRegistry
42+
->expects($this->any())
43+
->method('getProvider')
44+
->willReturnCallback(function (string $name) {
45+
return match ($name) {
46+
'test1' => $this->provider1,
47+
'test2' => $this->provider2,
48+
default => null,
49+
};
50+
});
3951

4052
$this->twoFactorTokenFactory = $this->createMock(TwoFactorTokenFactory::class);
4153

@@ -155,4 +167,21 @@ public function beginAuthentication_hasPreferredProvider_setThatProviderPreferre
155167

156168
$this->initiator->beginTwoFactorAuthentication($context);
157169
}
170+
171+
#[Test]
172+
public function beginAuthentication_statelessProviderPrepared_setThatProviderIsPrepared(): void
173+
{
174+
$originalToken = $this->createToken();
175+
$context = $this->createAuthenticationContext(null, $originalToken);
176+
$this->stubProvidersReturn(true, true);
177+
178+
$twoFactorToken = $this->createTwoFactorToken();
179+
$this->stubTwoFactorTokenFactoryReturns($twoFactorToken);
180+
$twoFactorToken
181+
->expects($this->once())
182+
->method('setTwoFactorProviderPrepared')
183+
->with('test2');
184+
185+
$this->initiator->beginTwoFactorAuthentication($context);
186+
}
158187
}

0 commit comments

Comments
 (0)