From 1b74ad26a4a9c4c2a27d7e47cbecab66909acd59 Mon Sep 17 00:00:00 2001 From: Daniel Burger <48986191+danielburger1337@users.noreply.github.com> Date: Thu, 18 Jan 2024 21:08:24 +0100 Subject: [PATCH] Add TwoFactorProviderDecider (#215) --- doc/configuration.rst | 4 + .../DependencyInjection/Configuration.php | 1 + .../SchebTwoFactorExtension.php | 9 ++ src/bundle/Resources/config/two_factor.php | 4 + .../Provider/TwoFactorProviderDecider.php | 26 ++++++ .../TwoFactorProviderDeciderInterface.php | 18 ++++ .../Provider/TwoFactorProviderInitiator.php | 31 +++---- .../SchebTwoFactorExtensionTest.php | 12 +++ .../Provider/TwoFactorProviderDeciderTest.php | 84 +++++++++++++++++++ .../TwoFactorProviderInitiatorTest.php | 15 +++- 10 files changed, 183 insertions(+), 21 deletions(-) create mode 100644 src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDecider.php create mode 100644 src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDeciderInterface.php create mode 100644 tests/Security/TwoFactor/Provider/TwoFactorProviderDeciderTest.php diff --git a/doc/configuration.rst b/doc/configuration.rst index 7488a7e7..599e9f14 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -100,6 +100,10 @@ Bundle Configuration # Must implement Scheb\TwoFactorBundle\Security\TwoFactor\Condition\TwoFactorConditionInterface two_factor_condition: acme.custom_two_factor_condition + # If you need custom conditions to decide what two factor provider to prefer. + # Must implement Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderDeciderInterface + two_factor_provider_decider: acme.custom_two_factor_provider_decider + Firewall Configuration ---------------------- diff --git a/src/bundle/DependencyInjection/Configuration.php b/src/bundle/DependencyInjection/Configuration.php index b3603b55..1e9dc405 100644 --- a/src/bundle/DependencyInjection/Configuration.php +++ b/src/bundle/DependencyInjection/Configuration.php @@ -47,6 +47,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->scalarNode('ip_whitelist_provider')->defaultValue('scheb_two_factor.default_ip_whitelist_provider')->end() ->scalarNode('two_factor_token_factory')->defaultValue('scheb_two_factor.default_token_factory')->end() + ->scalarNode('two_factor_provider_decider')->defaultValue('scheb_two_factor.default_provider_decider')->end() ->scalarNode('two_factor_condition')->defaultNull()->end() ->end(); diff --git a/src/bundle/DependencyInjection/SchebTwoFactorExtension.php b/src/bundle/DependencyInjection/SchebTwoFactorExtension.php index 3153e81d..971bb830 100644 --- a/src/bundle/DependencyInjection/SchebTwoFactorExtension.php +++ b/src/bundle/DependencyInjection/SchebTwoFactorExtension.php @@ -55,6 +55,7 @@ public function load(array $configs, ContainerBuilder $container): void $this->configureTwoFactorConditions($container, $config); $this->configureIpWhitelistProvider($container, $config); $this->configureTokenFactory($container, $config); + $this->configureProviderDecider($container, $config); if (isset($config['trusted_device']['enabled']) && $this->resolveFeatureFlag($container, $config['trusted_device']['enabled'])) { $this->configureTrustedDeviceManager($container, $config); @@ -158,6 +159,14 @@ private function configureTokenFactory(ContainerBuilder $container, array $confi $container->setAlias('scheb_two_factor.token_factory', $config['two_factor_token_factory']); } + /** + * @param array $config + */ + private function configureProviderDecider(ContainerBuilder $container, array $config): void + { + $container->setAlias('scheb_two_factor.provider_decider', $config['two_factor_provider_decider']); + } + /** * @param array $config */ diff --git a/src/bundle/Resources/config/two_factor.php b/src/bundle/Resources/config/two_factor.php index bc9da974..5767b79c 100644 --- a/src/bundle/Resources/config/two_factor.php +++ b/src/bundle/Resources/config/two_factor.php @@ -13,6 +13,7 @@ use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\DefaultTwoFactorFormRenderer; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TokenPreparationRecorder; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorFormRendererInterface; +use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderDecider; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderInitiator; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\TwoFactorProviderRegistry; use Scheb\TwoFactorBundle\Security\TwoFactor\TwoFactorFirewallContext; @@ -29,6 +30,8 @@ ->set('scheb_two_factor.default_token_factory', TwoFactorTokenFactory::class) + ->set('scheb_two_factor.default_provider_decider', TwoFactorProviderDecider::class) + ->set('scheb_two_factor.authentication_context_factory', AuthenticationContextFactory::class) ->args([AuthenticationContext::class]) @@ -56,6 +59,7 @@ ->args([ service('scheb_two_factor.provider_registry'), service('scheb_two_factor.token_factory'), + service('scheb_two_factor.provider_decider'), ]) ->set('scheb_two_factor.firewall_context', TwoFactorFirewallContext::class) diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDecider.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDecider.php new file mode 100644 index 00000000..ef9622c3 --- /dev/null +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDecider.php @@ -0,0 +1,26 @@ +getUser(); + + if ($user instanceof PreferredProviderInterface) { + return $user->getPreferredTwoFactorProvider(); + } + + return null; + } +} diff --git a/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDeciderInterface.php b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDeciderInterface.php new file mode 100644 index 00000000..9faf4650 --- /dev/null +++ b/src/bundle/Security/TwoFactor/Provider/TwoFactorProviderDeciderInterface.php @@ -0,0 +1,18 @@ +getToken(); if ($activeTwoFactorProviders) { $twoFactorToken = $this->twoFactorTokenFactory->create($authenticatedToken, $context->getFirewallName(), $activeTwoFactorProviders); - $this->setPreferredProvider($twoFactorToken, $context->getUser()); // Prioritize the user's preferred provider - return $twoFactorToken; - } - - return null; - } + $preferredProvider = $this->twoFactorProviderDecider->getPreferredTwoFactorProvider($activeTwoFactorProviders, $twoFactorToken, $context); - private function setPreferredProvider(TwoFactorTokenInterface $token, object $user): void - { - if (!($user instanceof PreferredProviderInterface)) { - return; - } + if (null !== $preferredProvider) { + try { + $twoFactorToken->preferTwoFactorProvider($preferredProvider); + } catch (UnknownTwoFactorProviderException) { + // Bad user input + } + } - $preferredProvider = $user->getPreferredTwoFactorProvider(); - if (!$preferredProvider) { - return; + return $twoFactorToken; } - try { - $token->preferTwoFactorProvider($preferredProvider); - } catch (UnknownTwoFactorProviderException) { - // Bad user input - } + return null; } } diff --git a/tests/DependencyInjection/SchebTwoFactorExtensionTest.php b/tests/DependencyInjection/SchebTwoFactorExtensionTest.php index efe13be7..feb66f1e 100644 --- a/tests/DependencyInjection/SchebTwoFactorExtensionTest.php +++ b/tests/DependencyInjection/SchebTwoFactorExtensionTest.php @@ -613,6 +613,17 @@ public function load_alternativeTokenFactory_replaceAlias(): void $this->assertHasAlias('scheb_two_factor.token_factory', 'acme_test.two_factor_token_factory'); } + /** + * @test + */ + public function load_alternativeProviderDecider_replaceAlias(): void + { + $config = $this->getFullConfig(); + $this->extension->load([$config], $this->container); + + $this->assertHasAlias('scheb_two_factor.provider_decider', 'acme_test.two_factor_provider_decider'); + } + /** * @return array|null */ @@ -638,6 +649,7 @@ private function getFullConfig(): array - 127.0.0.1 ip_whitelist_provider: acme_test.ip_whitelist_provider two_factor_token_factory: acme_test.two_factor_token_factory +two_factor_provider_decider: acme_test.two_factor_provider_decider two_factor_condition: acme_test.two_factor_condition trusted_device: enabled: true diff --git a/tests/Security/TwoFactor/Provider/TwoFactorProviderDeciderTest.php b/tests/Security/TwoFactor/Provider/TwoFactorProviderDeciderTest.php new file mode 100644 index 00000000..54228aa5 --- /dev/null +++ b/tests/Security/TwoFactor/Provider/TwoFactorProviderDeciderTest.php @@ -0,0 +1,84 @@ +twoFactorToken = $this->createMock(TwoFactorTokenInterface::class); + $this->twoFactorProviderDecider = new TwoFactorProviderDecider(); + } + + /** + * @test + */ + public function getPreferredTwoFactorProvider_implementsPreferredProvider_returnsPreferredProvider(): void + { + $user = $this->createUserWithPreferredProvider('preferredProvider'); + + $this->assertEquals( + 'preferredProvider', + $this->twoFactorProviderDecider->getPreferredTwoFactorProvider([], $this->twoFactorToken, $this->createAuthContext($user)), + ); + } + + /** + * @test + */ + public function getPreferredTwoFactorProvider_implementsPreferredProvider_returnsNullPreferredProvider(): void + { + $user = $this->createUserWithPreferredProvider(null); + + $this->assertNull( + $this->twoFactorProviderDecider->getPreferredTwoFactorProvider([], $this->twoFactorToken, $this->createAuthContext($user)), + ); + } + + /** + * @test + */ + public function getPreferredTwoFactorProvider_unexpectedUserObject_returnsNull(): void + { + $user = $this->createMock(UserInterface::class); + + $this->assertNull( + $this->twoFactorProviderDecider->getPreferredTwoFactorProvider([], $this->twoFactorToken, $this->createAuthContext($user)), + ); + } + + private function createUserWithPreferredProvider(string|null $preferredProvider): MockObject|UserWithPreferredProviderInterface + { + $user = $this->createMock(UserWithPreferredProviderInterface::class); + $user + ->expects($this->any()) + ->method('getPreferredTwoFactorProvider') + ->willReturn($preferredProvider); + + return $user; + } + + private function createAuthContext(object $user): MockObject|AuthenticationContextInterface + { + $authContext = $this->createMock(AuthenticationContextInterface::class); + $authContext + ->expects($this->any()) + ->method('getUser') + ->willReturn($user); + + return $authContext; + } +} diff --git a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php index 87bd9234..98ce7be4 100644 --- a/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php +++ b/tests/Security/TwoFactor/Provider/TwoFactorProviderInitiatorTest.php @@ -8,6 +8,7 @@ use Scheb\TwoFactorBundle\Security\Authentication\Token\TwoFactorTokenFactory; use Scheb\TwoFactorBundle\Security\Authentication\Token\TwoFactorTokenFactoryInterface; 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; @@ -18,6 +19,7 @@ class TwoFactorProviderInitiatorTest extends AbstractAuthenticationContextTestCa private MockObject|TwoFactorTokenFactoryInterface $twoFactorTokenFactory; private MockObject|TwoFactorProviderInterface $provider1; private MockObject|TwoFactorProviderInterface $provider2; + private MockObject|TwoFactorProviderDeciderInterface $providerDecider; private TwoFactorProviderInitiator $initiator; protected function setUp(): void @@ -36,7 +38,9 @@ protected function setUp(): void $this->twoFactorTokenFactory = $this->createMock(TwoFactorTokenFactory::class); - $this->initiator = new TwoFactorProviderInitiator($providerRegistry, $this->twoFactorTokenFactory); + $this->providerDecider = $this->createMock(TwoFactorProviderDeciderInterface::class); + + $this->initiator = new TwoFactorProviderInitiator($providerRegistry, $this->twoFactorTokenFactory, $this->providerDecider); } private function createTwoFactorToken(): MockObject|TwoFactorTokenInterface @@ -76,6 +80,14 @@ private function stubTwoFactorTokenFactoryReturns(MockObject $token): void ->willReturn($token); } + private function stubTwoFactorProviderDeciderReturns(string|null $preferredProvider): void + { + $this->providerDecider + ->expects($this->once()) + ->method('getPreferredTwoFactorProvider') + ->willReturn($preferredProvider); + } + /** * @test */ @@ -141,6 +153,7 @@ public function beginAuthentication_hasPreferredProvider_setThatProviderPreferre $context = $this->createAuthenticationContext(null, $originalToken, $user); $this->stubProvidersReturn(true, true); $this->stubTwoFactorTokenFactoryReturns($twoFactorToken); + $this->stubTwoFactorProviderDeciderReturns('preferredProvider'); $twoFactorToken ->expects($this->once())