From d33026f56ba5c6eeb412180c2ba1d5895b32271e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 15 Sep 2026 09:00:56 +0200 Subject: [PATCH 1/2] Support the authentication proofs of Symfony 8.2 --- src/bundle/Resources/config/security.php | 1 + .../AuthenticationTrustResolver.php | 35 +++++++++++ .../Authentication/Token/TwoFactorToken.php | 31 ++++++++++ .../Authenticator/TwoFactorAuthenticator.php | 24 ++++++++ .../AuthenticationMethodProviderInterface.php | 21 +++++++ .../Provider/Email/EmailTwoFactorProvider.php | 9 ++- .../GoogleAuthenticatorTwoFactorProvider.php | 9 ++- .../TotpAuthenticatorTwoFactorProvider.php | 9 ++- .../AuthenticationTrustResolverTest.php | 50 ++++++++++++++++ .../RecencyAwareTrustResolver.php | 46 +++++++++++++++ .../Authentication/Token/ProofsAwareToken.php | 28 +++++++++ .../Token/TwoFactorTokenTest.php | 39 ++++++++++++ ...cationMethodTwoFactorProviderInterface.php | 12 ++++ .../TwoFactorAuthenticatorTest.php | 59 +++++++++++++++++++ .../Email/EmailTwoFactorProviderTest.php | 6 ++ ...ogleAuthenticatorTwoFactorProviderTest.php | 6 ++ ...TotpAuthenticatorTwoFactorProviderTest.php | 6 ++ 17 files changed, 388 insertions(+), 3 deletions(-) create mode 100644 src/bundle/Security/TwoFactor/Provider/AuthenticationMethodProviderInterface.php create mode 100644 tests/Security/Authentication/RecencyAwareTrustResolver.php create mode 100644 tests/Security/Authentication/Token/ProofsAwareToken.php create mode 100644 tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php diff --git a/src/bundle/Resources/config/security.php b/src/bundle/Resources/config/security.php index 9574cc2e..42a67523 100644 --- a/src/bundle/Resources/config/security.php +++ b/src/bundle/Resources/config/security.php @@ -38,6 +38,7 @@ abstract_arg('Authentication required handler'), service('event_dispatcher'), service('logger')->nullOnInvalid(), + service('scheb_two_factor.provider_registry'), ]) ->set('scheb_two_factor.security.authentication.trust_resolver', AuthenticationTrustResolver::class) diff --git a/src/bundle/Security/Authentication/AuthenticationTrustResolver.php b/src/bundle/Security/Authentication/AuthenticationTrustResolver.php index 4e41411d..e64c3ec2 100644 --- a/src/bundle/Security/Authentication/AuthenticationTrustResolver.php +++ b/src/bundle/Security/Authentication/AuthenticationTrustResolver.php @@ -7,6 +7,7 @@ use Scheb\TwoFactorBundle\Security\Authentication\Token\TwoFactorTokenInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use function method_exists; /** * @final @@ -32,6 +33,40 @@ public function isAuthenticated(TokenInterface|null $token = null): bool return $this->decoratedTrustResolver->isAuthenticated($token); } + /** + * Declared on the interface since Symfony 8.2, where not implementing it is deprecated. + */ + public function isAuthenticatedRecently(TokenInterface|null $token = null): bool + { + return $this->isAuthenticatedRecentlyEnough(__FUNCTION__, $token); + } + + /** + * Declared on the interface since Symfony 8.2, where not implementing it is deprecated. + */ + public function isAuthenticatedVeryRecently(TokenInterface|null $token = null): bool + { + return $this->isAuthenticatedRecentlyEnough(__FUNCTION__, $token); + } + + private function isAuthenticatedRecentlyEnough(string $method, TokenInterface|null $token): bool + { + // A pending two-factor authentication is no proof of anything yet + if ($this->isTwoFactorToken($token)) { + return false; + } + + // The decorated resolver only has the method on Symfony 8.2+ + if (!method_exists($this->decoratedTrustResolver, $method)) { + return false; + } + + /** @psalm-suppress MixedAssignment, MixedMethodCall */ + $result = $this->decoratedTrustResolver->$method($token); + + return true === $result; + } + private function isTwoFactorToken(TokenInterface|null $token): bool { return $token instanceof TwoFactorTokenInterface; diff --git a/src/bundle/Security/Authentication/Token/TwoFactorToken.php b/src/bundle/Security/Authentication/Token/TwoFactorToken.php index 3e706864..d1c3f2f9 100644 --- a/src/bundle/Security/Authentication/Token/TwoFactorToken.php +++ b/src/bundle/Security/Authentication/Token/TwoFactorToken.php @@ -17,6 +17,7 @@ use function array_search; use function array_unshift; use function count; +use function method_exists; use function reset; use function sprintf; @@ -73,6 +74,36 @@ public function getRoleNames(): array return []; } + /** + * Symfony 8.2 records on the token which authentication methods were proven and when. + * The proofs belong to the token that is authenticated once 2fa completes, so they are + * delegated to it: the first factor is recorded while this token is the current one. + * + * @return array + */ + public function getAuthenticationProofs(): array + { + if (!method_exists($this->authenticatedToken, 'getAuthenticationProofs')) { + return []; + } + + /** @psalm-suppress MixedAssignment, MixedMethodCall */ + return $this->authenticatedToken->getAuthenticationProofs(); + } + + /** + * @param array $proofs + */ + public function setAuthenticationProofs(array $proofs): void + { + if (!method_exists($this->authenticatedToken, 'setAuthenticationProofs')) { + return; + } + + /** @psalm-suppress MixedMethodCall */ + $this->authenticatedToken->setAuthenticationProofs($proofs); + } + public function createWithCredentials(string $credentials): TwoFactorTokenInterface { $credentialsToken = new self($this->authenticatedToken, $credentials, $this->firewallName, $this->twoFactorProviders); diff --git a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php index c423ef4b..a4636a68 100644 --- a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php +++ b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php @@ -12,6 +12,8 @@ use Scheb\TwoFactorBundle\Security\Http\Authenticator\Passport\Credentials\TwoFactorCodeCredentials; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvent; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvents; +use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; +use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Scheb\TwoFactorBundle\Security\TwoFactor\TwoFactorFirewallConfig; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -24,6 +26,7 @@ use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\AuthenticationMethodBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\RememberMeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; @@ -54,6 +57,7 @@ public function __construct( private readonly AuthenticationRequiredHandlerInterface $authenticationRequiredHandler, private readonly EventDispatcherInterface $eventDispatcher, LoggerInterface|null $logger = null, + private readonly TwoFactorProviderRegistry|null $providerRegistry = null, ) { $this->logger = $logger ?? new NullLogger(); } @@ -102,9 +106,29 @@ public function authenticate(Request $request): Passport $passport->addBadge(new TrustedDeviceBadge()); } + // Symfony 8.2 records on the token which authentication methods were proven, from that badge + $authenticationMethod = $this->getAuthenticationMethod($currentToken); + /** @psalm-suppress UndefinedClass */ + if (null !== $authenticationMethod && class_exists(AuthenticationMethodBadge::class)) { + /** @psalm-suppress UndefinedClass, InvalidArgument, MixedMethodCall, MixedArgument */ + $passport->addBadge(new AuthenticationMethodBadge($authenticationMethod)); + } + return $passport; } + private function getAuthenticationMethod(TwoFactorTokenInterface $token): string|null + { + $providerName = $token->getCurrentTwoFactorProvider(); + if (null === $providerName || null === $this->providerRegistry) { + return null; + } + + $provider = $this->providerRegistry->getProvider($providerName); + + return $provider instanceof AuthenticationMethodProviderInterface ? $provider->getAuthenticationMethod() : null; + } + private function shouldSetTrustedDevice(Request $request, Passport $passport): bool { return $this->twoFactorFirewallConfig->hasTrustedDeviceParameterInRequest($request) diff --git a/src/bundle/Security/TwoFactor/Provider/AuthenticationMethodProviderInterface.php b/src/bundle/Security/TwoFactor/Provider/AuthenticationMethodProviderInterface.php new file mode 100644 index 00000000..a34bfd60 --- /dev/null +++ b/src/bundle/Security/TwoFactor/Provider/AuthenticationMethodProviderInterface.php @@ -0,0 +1,21 @@ +formRenderer; } + + public function getAuthenticationMethod(): string + { + // AuthenticationMethod::ONE_TIME_PASSWORD of Symfony 8.2, which cannot be referenced on older versions + return 'otp'; + } } diff --git a/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php b/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php index fd39d594..60537a46 100644 --- a/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php +++ b/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProvider.php @@ -6,6 +6,7 @@ use Scheb\TwoFactorBundle\Model\Google\TwoFactorInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\AuthenticationContextInterface; +use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Exception\TwoFactorProviderLogicException; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorFormRendererInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface; @@ -14,7 +15,7 @@ /** * @final */ -class GoogleAuthenticatorTwoFactorProvider implements TwoFactorProviderInterface +class GoogleAuthenticatorTwoFactorProvider implements TwoFactorProviderInterface, AuthenticationMethodProviderInterface { public function __construct( private readonly GoogleAuthenticatorInterface $authenticator, @@ -60,4 +61,10 @@ public function getFormRenderer(): TwoFactorFormRendererInterface { return $this->formRenderer; } + + public function getAuthenticationMethod(): string + { + // AuthenticationMethod::ONE_TIME_PASSWORD of Symfony 8.2, which cannot be referenced on older versions + return 'otp'; + } } diff --git a/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php b/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php index 3ab17649..6ff422ab 100644 --- a/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php +++ b/src/totp/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProvider.php @@ -6,6 +6,7 @@ use Scheb\TwoFactorBundle\Model\Totp\TwoFactorInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\AuthenticationContextInterface; +use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Exception\TwoFactorProviderLogicException; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorFormRendererInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface; @@ -14,7 +15,7 @@ /** * @final */ -class TotpAuthenticatorTwoFactorProvider implements TwoFactorProviderInterface +class TotpAuthenticatorTwoFactorProvider implements TwoFactorProviderInterface, AuthenticationMethodProviderInterface { public function __construct( private readonly TotpAuthenticatorInterface $authenticator, @@ -64,4 +65,10 @@ public function getFormRenderer(): TwoFactorFormRendererInterface { return $this->formRenderer; } + + public function getAuthenticationMethod(): string + { + // AuthenticationMethod::ONE_TIME_PASSWORD of Symfony 8.2, which cannot be referenced on older versions + return 'otp'; + } } diff --git a/tests/Security/Authentication/AuthenticationTrustResolverTest.php b/tests/Security/Authentication/AuthenticationTrustResolverTest.php index cfb4c9a8..73cebc05 100644 --- a/tests/Security/Authentication/AuthenticationTrustResolverTest.php +++ b/tests/Security/Authentication/AuthenticationTrustResolverTest.php @@ -48,6 +48,56 @@ public function isRememberMe_tokenGiven_returnResultFromDecoratedTrustResolver(b $this->assertEquals($returnedResult, $returnValue); } + /** + * @return array> + */ + public static function provideRecencyMethods(): array + { + return [ + ['isAuthenticatedRecently'], + ['isAuthenticatedVeryRecently'], + ]; + } + + #[Test] + #[DataProvider('provideRecencyMethods')] + public function isAuthenticatedRecently_twoFactorToken_returnFalse(string $method): void + { + $decoratedTrustResolver = new RecencyAwareTrustResolver(); + $decoratedTrustResolver->result = true; + $trustResolver = new AuthenticationTrustResolver($decoratedTrustResolver); + + $returnValue = $trustResolver->$method($this->createMock(TwoFactorTokenInterface::class)); + $this->assertFalse($returnValue); + $this->assertNull($decoratedTrustResolver->calledMethod); + } + + #[Test] + #[DataProvider('provideRecencyMethods')] + public function isAuthenticatedRecently_decoratedTrustResolverWithoutTheMethod_returnFalse(string $method): void + { + // The method exists on the interface since Symfony 8.2 only + $this->decoratedTrustResolver + ->expects($this->never()) + ->method($this->anything()); + + $returnValue = $this->trustResolver->$method($this->createMock(TokenInterface::class)); + $this->assertFalse($returnValue); + } + + #[Test] + #[DataProvider('provideRecencyMethods')] + public function isAuthenticatedRecently_notTwoFactorToken_returnResultFromDecoratedTrustResolver(string $method): void + { + $decoratedTrustResolver = new RecencyAwareTrustResolver(); + $decoratedTrustResolver->result = true; + $trustResolver = new AuthenticationTrustResolver($decoratedTrustResolver); + + $returnValue = $trustResolver->$method($this->createMock(TokenInterface::class)); + $this->assertTrue($returnValue); + $this->assertEquals($method, $decoratedTrustResolver->calledMethod); + } + #[Test] public function isFullFledged_twoFactorToken_returnFalse(): void { diff --git a/tests/Security/Authentication/RecencyAwareTrustResolver.php b/tests/Security/Authentication/RecencyAwareTrustResolver.php new file mode 100644 index 00000000..5f44c031 --- /dev/null +++ b/tests/Security/Authentication/RecencyAwareTrustResolver.php @@ -0,0 +1,46 @@ +calledMethod = __FUNCTION__; + + return $this->result; + } + + public function isAuthenticatedVeryRecently(TokenInterface|null $token = null): bool + { + $this->calledMethod = __FUNCTION__; + + return $this->result; + } +} diff --git a/tests/Security/Authentication/Token/ProofsAwareToken.php b/tests/Security/Authentication/Token/ProofsAwareToken.php new file mode 100644 index 00000000..a600dd7a --- /dev/null +++ b/tests/Security/Authentication/Token/ProofsAwareToken.php @@ -0,0 +1,28 @@ + */ + private array $proofs = []; + + /** @return array */ + public function getAuthenticationProofs(): array + { + return $this->proofs; + } + + /** @param array $proofs */ + public function setAuthenticationProofs(array $proofs): void + { + $this->proofs = $proofs; + } +} diff --git a/tests/Security/Authentication/Token/TwoFactorTokenTest.php b/tests/Security/Authentication/Token/TwoFactorTokenTest.php index 43b131e8..13956006 100644 --- a/tests/Security/Authentication/Token/TwoFactorTokenTest.php +++ b/tests/Security/Authentication/Token/TwoFactorTokenTest.php @@ -12,6 +12,7 @@ use Scheb\TwoFactorBundle\Tests\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; use function serialize; use function unserialize; @@ -196,4 +197,42 @@ public function serialize_tokenGiven_unserializeIdenticalToken(): void $this->assertEquals($twoFactorToken, $unserializedToken); } + + #[Test] + public function getAuthenticationProofs_authenticatedTokenHoldsProofs_returnProofsOfAuthenticatedToken(): void + { + $authenticatedToken = $this->createProofsAwareToken(); + $authenticatedToken->setAuthenticationProofs(['pwd' => 100]); + $twoFactorToken = new TwoFactorToken($authenticatedToken, null, self::FIREWALL_NAME, ['provider1']); + + $this->assertEquals(['pwd' => 100], $twoFactorToken->getAuthenticationProofs()); + } + + #[Test] + public function setAuthenticationProofs_authenticatedTokenHoldsProofs_recordOnAuthenticatedToken(): void + { + // The proofs belong to the token that is authenticated once 2fa completes, and + // Symfony records the ones of the first factor while the TwoFactorToken is current + $authenticatedToken = $this->createProofsAwareToken(); + $twoFactorToken = new TwoFactorToken($authenticatedToken, null, self::FIREWALL_NAME, ['provider1']); + + $twoFactorToken->setAuthenticationProofs(['pwd' => 100]); + + $this->assertEquals(['pwd' => 100], $authenticatedToken->getAuthenticationProofs()); + } + + #[Test] + public function getAuthenticationProofs_authenticatedTokenWithoutProofs_returnEmptyArray(): void + { + // A token of Symfony below 8.2 has no proofs + $this->twoFactorToken->setAuthenticationProofs(['pwd' => 100]); + + $this->assertEquals([], $this->twoFactorToken->getAuthenticationProofs()); + } + + private function createProofsAwareToken(): UsernamePasswordToken + { + // A token of Symfony 8.2, where the methods are declared on the interface + return new ProofsAwareToken(new InMemoryUser('username', null), self::FIREWALL_NAME); + } } diff --git a/tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php b/tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php new file mode 100644 index 00000000..d9d98098 --- /dev/null +++ b/tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php @@ -0,0 +1,12 @@ +authenticationRequiredHandler = $this->createMock(AuthenticationRequiredHandlerInterface::class); $this->eventDispatcher = $this->createMock(EventDispatcherInterface::class); $this->request = $this->createMock(Request::class); + $this->providerRegistry = $this->createMock(TwoFactorProviderRegistry::class); $this->twoFactorFirewallConfig ->expects($this->any()) @@ -85,6 +92,7 @@ protected function setUp(): void $this->authenticationRequiredHandler, $this->eventDispatcher, $this->createMock(LoggerInterface::class), + $this->providerRegistry, ); } @@ -245,6 +253,57 @@ public function authenticate_onRequest_createTwoFactorPassportWithCredentials(): $this->assertEquals(self::CODE, $credentials->getCode()); } + #[Test] + public function authenticate_providerDeclaresItsAuthenticationMethod_createTwoFactorPassportWithAuthenticationMethodBadge(): void + { + if (!class_exists(AuthenticationMethodBadge::class)) { + $this->markTestSkipped('Requires the AuthenticationMethodBadge of Symfony 8.2'); + } + + $this->stubCurrentProviderIs('totp', $this->createProviderWithAuthenticationMethod('otp')); + + $returnValue = $this->authenticator->authenticate($this->request); + + $badge = $returnValue->getBadge(AuthenticationMethodBadge::class); + $this->assertInstanceOf(AuthenticationMethodBadge::class, $badge); + $this->assertEquals(['otp'], $badge->methods); + } + + #[Test] + public function authenticate_providerWithoutAuthenticationMethod_noAuthenticationMethodBadge(): void + { + $this->stubCurrentProviderIs('custom', $this->createMock(TwoFactorProviderInterface::class)); + + $returnValue = $this->authenticator->authenticate($this->request); + + $this->assertFalse($returnValue->hasBadge(AuthenticationMethodBadge::class)); + } + + private function stubCurrentProviderIs(string $providerName, TwoFactorProviderInterface $provider): void + { + $twoFactorToken = $this->stubTokenStorageHasTwoFactorToken(); + $twoFactorToken + ->expects($this->any()) + ->method('getCurrentTwoFactorProvider') + ->willReturn($providerName); + $this->providerRegistry + ->expects($this->any()) + ->method('getProvider') + ->with($providerName) + ->willReturn($provider); + } + + private function createProviderWithAuthenticationMethod(string $method): TwoFactorProviderInterface&AuthenticationMethodProviderInterface + { + $provider = $this->createMock(AuthenticationMethodTwoFactorProviderInterface::class); + $provider + ->expects($this->any()) + ->method('getAuthenticationMethod') + ->willReturn($method); + + return $provider; + } + #[Test] public function authenticate_tokenHasRememberMeAttribute_createTwoFactorPassportWithRememberMeBadge(): void { diff --git a/tests/Security/TwoFactor/Provider/Email/EmailTwoFactorProviderTest.php b/tests/Security/TwoFactor/Provider/Email/EmailTwoFactorProviderTest.php index 3a3b143b..cc51dd97 100644 --- a/tests/Security/TwoFactor/Provider/Email/EmailTwoFactorProviderTest.php +++ b/tests/Security/TwoFactor/Provider/Email/EmailTwoFactorProviderTest.php @@ -194,4 +194,10 @@ public function validateAuthenticationCode_invalidCode_dispatchCheckAndInvalidEv $this->provider->validateAuthenticationCode($user, self::INVALID_AUTH_CODE); } + + #[Test] + public function getAuthenticationMethod_always_returnOtp(): void + { + $this->assertEquals('otp', $this->provider->getAuthenticationMethod()); + } } diff --git a/tests/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProviderTest.php b/tests/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProviderTest.php index 08a2d1ec..0ea520c3 100644 --- a/tests/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProviderTest.php +++ b/tests/Security/TwoFactor/Provider/Google/GoogleAuthenticatorTwoFactorProviderTest.php @@ -145,4 +145,10 @@ public static function provideValidationResult(): array [false], ]; } + + #[Test] + public function getAuthenticationMethod_always_returnOtp(): void + { + $this->assertEquals('otp', $this->provider->getAuthenticationMethod()); + } } diff --git a/tests/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProviderTest.php b/tests/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProviderTest.php index ee654f84..61f223b4 100644 --- a/tests/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProviderTest.php +++ b/tests/Security/TwoFactor/Provider/Totp/TotpAuthenticatorTwoFactorProviderTest.php @@ -156,4 +156,10 @@ public static function provideValidationResult(): array [false], ]; } + + #[Test] + public function getAuthenticationMethod_always_returnOtp(): void + { + $this->assertEquals('otp', $this->provider->getAuthenticationMethod()); + } } From 17a208a2028b28ec6010fe9837e2b738a9c74e1a Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 9 Oct 2026 19:12:59 +0200 Subject: [PATCH 2/2] Add the AuthenticationMethodBadge from the code check listeners --- .../EventListener/CheckBackupCodeListener.php | 6 ++ src/bundle/Resources/config/security.php | 1 - .../Authenticator/TwoFactorAuthenticator.php | 24 -------- .../AbstractCheckCodeListener.php | 15 +++++ .../CheckTwoFactorCodeListener.php | 8 +++ .../TwoFactorAuthenticatorTest.php | 59 ------------------- .../AbstractCheckCodeListenerTestSetup.php | 4 +- ...cationMethodTwoFactorProviderInterface.php | 2 +- .../CheckBackupCodeListenerTest.php | 41 +++++++++++++ .../CheckTwoFactorCodeListenerTest.php | 52 ++++++++++++++++ 10 files changed, 126 insertions(+), 86 deletions(-) rename tests/Security/Http/{Authenticator => EventListener}/AuthenticationMethodTwoFactorProviderInterface.php (84%) diff --git a/src/backup-code/Security/Http/EventListener/CheckBackupCodeListener.php b/src/backup-code/Security/Http/EventListener/CheckBackupCodeListener.php index 72a835c7..06bd5ec0 100644 --- a/src/backup-code/Security/Http/EventListener/CheckBackupCodeListener.php +++ b/src/backup-code/Security/Http/EventListener/CheckBackupCodeListener.php @@ -45,6 +45,12 @@ protected function isValidCode(string $providerName, object $user, string $code) return false; } + protected function getAuthenticationMethod(string $providerName): string + { + // AuthenticationMethod::ONE_TIME_PASSWORD of Symfony 8.2, which cannot be referenced on older versions + return 'otp'; + } + /** * {@inheritDoc} */ diff --git a/src/bundle/Resources/config/security.php b/src/bundle/Resources/config/security.php index 42a67523..9574cc2e 100644 --- a/src/bundle/Resources/config/security.php +++ b/src/bundle/Resources/config/security.php @@ -38,7 +38,6 @@ abstract_arg('Authentication required handler'), service('event_dispatcher'), service('logger')->nullOnInvalid(), - service('scheb_two_factor.provider_registry'), ]) ->set('scheb_two_factor.security.authentication.trust_resolver', AuthenticationTrustResolver::class) diff --git a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php index a4636a68..c423ef4b 100644 --- a/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php +++ b/src/bundle/Security/Http/Authenticator/TwoFactorAuthenticator.php @@ -12,8 +12,6 @@ use Scheb\TwoFactorBundle\Security\Http\Authenticator\Passport\Credentials\TwoFactorCodeCredentials; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvent; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvents; -use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; -use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Scheb\TwoFactorBundle\Security\TwoFactor\TwoFactorFirewallConfig; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -26,7 +24,6 @@ use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface; -use Symfony\Component\Security\Http\Authenticator\Passport\Badge\AuthenticationMethodBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\RememberMeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; @@ -57,7 +54,6 @@ public function __construct( private readonly AuthenticationRequiredHandlerInterface $authenticationRequiredHandler, private readonly EventDispatcherInterface $eventDispatcher, LoggerInterface|null $logger = null, - private readonly TwoFactorProviderRegistry|null $providerRegistry = null, ) { $this->logger = $logger ?? new NullLogger(); } @@ -106,29 +102,9 @@ public function authenticate(Request $request): Passport $passport->addBadge(new TrustedDeviceBadge()); } - // Symfony 8.2 records on the token which authentication methods were proven, from that badge - $authenticationMethod = $this->getAuthenticationMethod($currentToken); - /** @psalm-suppress UndefinedClass */ - if (null !== $authenticationMethod && class_exists(AuthenticationMethodBadge::class)) { - /** @psalm-suppress UndefinedClass, InvalidArgument, MixedMethodCall, MixedArgument */ - $passport->addBadge(new AuthenticationMethodBadge($authenticationMethod)); - } - return $passport; } - private function getAuthenticationMethod(TwoFactorTokenInterface $token): string|null - { - $providerName = $token->getCurrentTwoFactorProvider(); - if (null === $providerName || null === $this->providerRegistry) { - return null; - } - - $provider = $this->providerRegistry->getProvider($providerName); - - return $provider instanceof AuthenticationMethodProviderInterface ? $provider->getAuthenticationMethod() : null; - } - private function shouldSetTrustedDevice(Request $request, Passport $passport): bool { return $this->twoFactorFirewallConfig->hasTrustedDeviceParameterInRequest($request) diff --git a/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php b/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php index 9b6d1486..b62863d6 100644 --- a/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php +++ b/src/bundle/Security/Http/EventListener/AbstractCheckCodeListener.php @@ -8,8 +8,10 @@ use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\PreparationRecorderInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\AuthenticationMethodBadge; use Symfony\Component\Security\Http\Event\CheckPassportEvent; use function assert; +use function class_exists; use function sprintf; /** @@ -50,9 +52,22 @@ public function checkPassport(CheckPassportEvent $event): void return; } + // Symfony 8.2 records on the token which authentication methods were proven, from that badge + $authenticationMethod = $this->getAuthenticationMethod($providerName); + /** @psalm-suppress UndefinedClass */ + if (null !== $authenticationMethod && class_exists(AuthenticationMethodBadge::class)) { + /** @psalm-suppress UndefinedClass, InvalidArgument, MixedMethodCall, MixedArgument */ + $passport->addBadge(new AuthenticationMethodBadge($authenticationMethod)); + } + // Badge is resolved, but authentication for provider is not complete yet $credentialsBadge->markResolved(); } abstract protected function isValidCode(string $providerName, object $user, string $code): bool; + + /** + * Returns the "amr" value of RFC 8176 that a valid code proves, if known. + */ + abstract protected function getAuthenticationMethod(string $providerName): string|null; } diff --git a/src/bundle/Security/Http/EventListener/CheckTwoFactorCodeListener.php b/src/bundle/Security/Http/EventListener/CheckTwoFactorCodeListener.php index c49940a4..68307098 100644 --- a/src/bundle/Security/Http/EventListener/CheckTwoFactorCodeListener.php +++ b/src/bundle/Security/Http/EventListener/CheckTwoFactorCodeListener.php @@ -9,6 +9,7 @@ use Scheb\TwoFactorBundle\Security\Authentication\Exception\TwoFactorProviderNotFoundException; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvents; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorCodeEvent; +use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\PreparationRecorderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Symfony\Component\Security\Http\Event\CheckPassportEvent; @@ -62,6 +63,13 @@ protected function isValidCode(string $providerName, object $user, string $code) throw new InvalidTwoFactorCodeException(InvalidTwoFactorCodeException::MESSAGE); } + protected function getAuthenticationMethod(string $providerName): string|null + { + $authenticationProvider = $this->providerRegistry->getProvider($providerName); + + return $authenticationProvider instanceof AuthenticationMethodProviderInterface ? $authenticationProvider->getAuthenticationMethod() : null; + } + /** * {@inheritDoc} */ diff --git a/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php b/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php index 098fd25f..1e7c4831 100644 --- a/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php +++ b/tests/Security/Http/Authenticator/TwoFactorAuthenticatorTest.php @@ -14,9 +14,6 @@ use Scheb\TwoFactorBundle\Security\Http\Authenticator\TwoFactorAuthenticator; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvent; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorAuthenticationEvents; -use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; -use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface; -use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Scheb\TwoFactorBundle\Security\TwoFactor\TwoFactorFirewallConfig; use Scheb\TwoFactorBundle\Tests\EventDispatcherTestHelper; use Scheb\TwoFactorBundle\Tests\TestCase; @@ -29,13 +26,11 @@ use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface; use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface; -use Symfony\Component\Security\Http\Authenticator\Passport\Badge\AuthenticationMethodBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\RememberMeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; use function assert; -use function class_exists; class TwoFactorAuthenticatorTest extends TestCase { @@ -53,7 +48,6 @@ class TwoFactorAuthenticatorTest extends TestCase private MockObject&AuthenticationFailureHandlerInterface $failureHandler; private MockObject&AuthenticationRequiredHandlerInterface $authenticationRequiredHandler; private MockObject&Request $request; - private MockObject&TwoFactorProviderRegistry $providerRegistry; private TwoFactorAuthenticator $authenticator; protected function setUp(): void @@ -65,7 +59,6 @@ protected function setUp(): void $this->authenticationRequiredHandler = $this->createMock(AuthenticationRequiredHandlerInterface::class); $this->eventDispatcher = $this->createMock(EventDispatcherInterface::class); $this->request = $this->createMock(Request::class); - $this->providerRegistry = $this->createMock(TwoFactorProviderRegistry::class); $this->twoFactorFirewallConfig ->expects($this->any()) @@ -92,7 +85,6 @@ protected function setUp(): void $this->authenticationRequiredHandler, $this->eventDispatcher, $this->createMock(LoggerInterface::class), - $this->providerRegistry, ); } @@ -253,57 +245,6 @@ public function authenticate_onRequest_createTwoFactorPassportWithCredentials(): $this->assertEquals(self::CODE, $credentials->getCode()); } - #[Test] - public function authenticate_providerDeclaresItsAuthenticationMethod_createTwoFactorPassportWithAuthenticationMethodBadge(): void - { - if (!class_exists(AuthenticationMethodBadge::class)) { - $this->markTestSkipped('Requires the AuthenticationMethodBadge of Symfony 8.2'); - } - - $this->stubCurrentProviderIs('totp', $this->createProviderWithAuthenticationMethod('otp')); - - $returnValue = $this->authenticator->authenticate($this->request); - - $badge = $returnValue->getBadge(AuthenticationMethodBadge::class); - $this->assertInstanceOf(AuthenticationMethodBadge::class, $badge); - $this->assertEquals(['otp'], $badge->methods); - } - - #[Test] - public function authenticate_providerWithoutAuthenticationMethod_noAuthenticationMethodBadge(): void - { - $this->stubCurrentProviderIs('custom', $this->createMock(TwoFactorProviderInterface::class)); - - $returnValue = $this->authenticator->authenticate($this->request); - - $this->assertFalse($returnValue->hasBadge(AuthenticationMethodBadge::class)); - } - - private function stubCurrentProviderIs(string $providerName, TwoFactorProviderInterface $provider): void - { - $twoFactorToken = $this->stubTokenStorageHasTwoFactorToken(); - $twoFactorToken - ->expects($this->any()) - ->method('getCurrentTwoFactorProvider') - ->willReturn($providerName); - $this->providerRegistry - ->expects($this->any()) - ->method('getProvider') - ->with($providerName) - ->willReturn($provider); - } - - private function createProviderWithAuthenticationMethod(string $method): TwoFactorProviderInterface&AuthenticationMethodProviderInterface - { - $provider = $this->createMock(AuthenticationMethodTwoFactorProviderInterface::class); - $provider - ->expects($this->any()) - ->method('getAuthenticationMethod') - ->willReturn($method); - - return $provider; - } - #[Test] public function authenticate_tokenHasRememberMeAttribute_createTwoFactorPassportWithRememberMeBadge(): void { diff --git a/tests/Security/Http/EventListener/AbstractCheckCodeListenerTestSetup.php b/tests/Security/Http/EventListener/AbstractCheckCodeListenerTestSetup.php index 49d4aebf..4e1d3957 100644 --- a/tests/Security/Http/EventListener/AbstractCheckCodeListenerTestSetup.php +++ b/tests/Security/Http/EventListener/AbstractCheckCodeListenerTestSetup.php @@ -52,7 +52,7 @@ protected function expectMarkCredentialsResolved(): void ->method('markResolved'); } - protected function stubAllPreconditionsFulfilled(): void + protected function stubAllPreconditionsFulfilled(): MockObject&Passport { $passport = $this->createMock(Passport::class); $token = $this->createTwoFactorToken(self::TWO_FACTOR_PROVIDER_ID); @@ -60,6 +60,8 @@ protected function stubAllPreconditionsFulfilled(): void $this->stubPassport($passport); $this->stubPassportHasCredentialsBadge($passport, $token, false); $this->stubPreparationPrepared(true); + + return $passport; } private function createTwoFactorToken(string|null $currentProvider): MockObject&TwoFactorTokenInterface diff --git a/tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php b/tests/Security/Http/EventListener/AuthenticationMethodTwoFactorProviderInterface.php similarity index 84% rename from tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php rename to tests/Security/Http/EventListener/AuthenticationMethodTwoFactorProviderInterface.php index d9d98098..4401949c 100644 --- a/tests/Security/Http/Authenticator/AuthenticationMethodTwoFactorProviderInterface.php +++ b/tests/Security/Http/EventListener/AuthenticationMethodTwoFactorProviderInterface.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Scheb\TwoFactorBundle\Tests\Security\Http\Authenticator; +namespace Scheb\TwoFactorBundle\Tests\Security\Http\EventListener; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\AuthenticationMethodProviderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface; diff --git a/tests/Security/Http/EventListener/CheckBackupCodeListenerTest.php b/tests/Security/Http/EventListener/CheckBackupCodeListenerTest.php index 824d8cdd..ef8cc0ea 100644 --- a/tests/Security/Http/EventListener/CheckBackupCodeListenerTest.php +++ b/tests/Security/Http/EventListener/CheckBackupCodeListenerTest.php @@ -11,7 +11,9 @@ use Scheb\TwoFactorBundle\Security\TwoFactor\Event\BackupCodeEvents; use Scheb\TwoFactorBundle\Security\TwoFactor\Event\TwoFactorCodeEvent; use Scheb\TwoFactorBundle\Tests\EventDispatcherTestHelper; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\AuthenticationMethodBadge; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use function class_exists; /** * @property CheckBackupCodeListener $listener @@ -116,4 +118,43 @@ public function checkPassport_invalidBackupCode_dispatchCheckEvent(): void $this->listener->checkPassport($this->checkPassportEvent); } + + #[Test] + public function checkPassport_validBackupCode_addOtpAuthenticationMethodBadge(): void + { + if (!class_exists(AuthenticationMethodBadge::class)) { + $this->markTestSkipped('Requires the AuthenticationMethodBadge of Symfony 8.2'); + } + + $passport = $this->stubAllPreconditionsFulfilled(); + + $this->backupCodeManager + ->expects($this->any()) + ->method('isBackupCode') + ->willReturn(true); + + $passport + ->expects($this->once()) + ->method('addBadge') + ->with(new AuthenticationMethodBadge('otp')); + + $this->listener->checkPassport($this->checkPassportEvent); + } + + #[Test] + public function checkPassport_invalidBackupCode_noAuthenticationMethodBadge(): void + { + $passport = $this->stubAllPreconditionsFulfilled(); + + $this->backupCodeManager + ->expects($this->any()) + ->method('isBackupCode') + ->willReturn(false); + + $passport + ->expects($this->never()) + ->method('addBadge'); + + $this->listener->checkPassport($this->checkPassportEvent); + } } diff --git a/tests/Security/Http/EventListener/CheckTwoFactorCodeListenerTest.php b/tests/Security/Http/EventListener/CheckTwoFactorCodeListenerTest.php index e16f82f3..1ff26b64 100644 --- a/tests/Security/Http/EventListener/CheckTwoFactorCodeListenerTest.php +++ b/tests/Security/Http/EventListener/CheckTwoFactorCodeListenerTest.php @@ -15,7 +15,9 @@ use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInterface; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Scheb\TwoFactorBundle\Tests\EventDispatcherTestHelper; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\AuthenticationMethodBadge; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use function class_exists; /** * @property CheckTwoFactorCodeListener $listener @@ -126,4 +128,54 @@ public function checkPassport_invalidCode_unresolvedCredentials(): void $this->listener->checkPassport($this->checkPassportEvent); } + + #[Test] + public function checkPassport_validCodeOfProviderWithAuthenticationMethod_addAuthenticationMethodBadge(): void + { + if (!class_exists(AuthenticationMethodBadge::class)) { + $this->markTestSkipped('Requires the AuthenticationMethodBadge of Symfony 8.2'); + } + + $passport = $this->stubAllPreconditionsFulfilled(); + + $authenticationProvider = $this->createMock(AuthenticationMethodTwoFactorProviderInterface::class); + $authenticationProvider + ->expects($this->any()) + ->method('validateAuthenticationCode') + ->willReturn(true); + $authenticationProvider + ->expects($this->any()) + ->method('getAuthenticationMethod') + ->willReturn('otp'); + $this->providerRegistry + ->expects($this->any()) + ->method('getProvider') + ->with(self::TWO_FACTOR_PROVIDER_ID) + ->willReturn($authenticationProvider); + + $passport + ->expects($this->once()) + ->method('addBadge') + ->with(new AuthenticationMethodBadge('otp')); + + $this->listener->checkPassport($this->checkPassportEvent); + } + + #[Test] + public function checkPassport_validCodeOfProviderWithoutAuthenticationMethod_noAuthenticationMethodBadge(): void + { + $passport = $this->stubAllPreconditionsFulfilled(); + + $authenticationProvider = $this->stubTwoFactorAuthenticationProvider(); + $authenticationProvider + ->expects($this->any()) + ->method('validateAuthenticationCode') + ->willReturn(true); + + $passport + ->expects($this->never()) + ->method('addBadge'); + + $this->listener->checkPassport($this->checkPassportEvent); + } }