From 6870a188038ba0dc6cd7b293ae119b3f6b9491f1 Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sat, 2 May 2020 13:48:24 +0200 Subject: [PATCH] Fixed entry point resolving and guard entry point configuration --- .../Factory/GuardAuthenticationFactory.php | 5 +- .../DependencyInjection/SecurityExtension.php | 6 +- .../SecurityExtensionTest.php | 131 ++++++++++++++++++ .../Bundle/SecurityBundle/composer.json | 2 +- 4 files changed, 139 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php index 283da74373..1dbe1a786f 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php @@ -12,6 +12,7 @@ namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory; use Symfony\Component\Config\Definition\Builder\NodeDefinition; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -118,10 +119,8 @@ class GuardAuthenticationFactory implements SecurityFactoryInterface, Authentica try { return $this->determineEntryPoint(null, $config); } catch (\LogicException $e) { - // ignore the exception, the new system prefers setting "entry_point" over "guard.entry_point" + throw new InvalidConfigurationException(sprintf('Because you have multiple guard authenticators, you need to set the "entry_point" key to one of your authenticators (%s).', implode(', ', $config['authenticators']))); } - - return null; } private function determineEntryPoint(?string $defaultEntryPointId, array $config): string diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index ca4a2abc25..e401b65883 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -33,6 +33,7 @@ use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpKernel\DependencyInjection\Extension; +use Symfony\Component\Ldap\Entry; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder; use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder; @@ -443,6 +444,9 @@ class SecurityExtension extends Extension implements PrependExtensionInterface if (!$this->authenticatorManagerEnabled) { $authenticationProviders = array_merge($authenticationProviders, $firewallAuthenticationProviders); } else { + // $configuredEntryPoint is resolved into a service ID and stored in $defaultEntryPoint + $configuredEntryPoint = $defaultEntryPoint; + // authenticator manager $authenticators = array_map(function ($id) { return new Reference($id); @@ -543,7 +547,7 @@ class SecurityExtension extends Extension implements PrependExtensionInterface $authenticationProviders[] = $authenticators; } - if ($factory instanceof EntryPointFactoryInterface && ($entryPoint = $factory->createEntryPoint($container, $id, $firewall[$key], null))) { + if ($factory instanceof EntryPointFactoryInterface && ($entryPoint = $factory->createEntryPoint($container, $id, $firewall[$key]))) { $entryPoints[$key] = $entryPoint; } } else { diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php index c395ed1b52..da09e432a0 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php @@ -16,11 +16,19 @@ use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension; use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Fixtures\UserProvider\DummyProvider; +use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FirewallEntryPointBundle\Security\EntryPointStub; +use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\GuardedBundle\AppCustomAuthenticator; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\ExpressionLanguage\Expression; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; +use Symfony\Component\Security\Guard\AuthenticatorInterface; class SecurityExtensionTest extends TestCase { @@ -413,6 +421,90 @@ class SecurityExtensionTest extends TestCase $this->assertEquals(new Reference('security.user.provider.concrete.second'), $container->getDefinition('security.authentication.switchuser_listener.foobar')->getArgument(1)); } + /** + * @dataProvider provideEntryPointFirewalls + */ + public function testAuthenticatorManagerEnabledEntryPoint(array $firewall, $entryPointId) + { + $container = $this->getRawContainer(); + $container->loadFromExtension('security', [ + 'enable_authenticator_manager' => true, + 'providers' => [ + 'first' => ['id' => 'users'], + ], + + 'firewalls' => [ + 'main' => $firewall, + ], + ]); + + $container->compile(); + + $this->assertEquals($entryPointId, (string) $container->getDefinition('security.firewall.map.config.main')->getArgument(7)); + $this->assertEquals($entryPointId, (string) $container->getDefinition('security.exception_listener.main')->getArgument(4)); + } + + public function provideEntryPointFirewalls() + { + // only one entry point available + yield [['http_basic' => true], 'security.authentication.basic_entry_point.main']; + // explicitly configured by authenticator key + yield [['form_login' => true, 'http_basic' => true, 'entry_point' => 'form_login'], 'security.authentication.form_entry_point.main']; + // explicitly configured another service + yield [['form_login' => true, 'entry_point' => EntryPointStub::class], EntryPointStub::class]; + // no entry point required + yield [['json_login' => true], null]; + + // only one guard authenticator entry point available + yield [[ + 'guard' => ['authenticators' => [AppCustomAuthenticator::class]], + ], AppCustomAuthenticator::class]; + // explicitly configured guard authenticator entry point + yield [[ + 'guard' => [ + 'authenticators' => [AppCustomAuthenticator::class, NullAuthenticator::class], + 'entry_point' => NullAuthenticator::class, + ], + ], NullAuthenticator::class]; + } + + /** + * @dataProvider provideEntryPointRequiredData + */ + public function testEntryPointRequired(array $firewall, $messageRegex) + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessageMatches($messageRegex); + + $container = $this->getRawContainer(); + $container->loadFromExtension('security', [ + 'enable_authenticator_manager' => true, + 'providers' => [ + 'first' => ['id' => 'users'], + ], + + 'firewalls' => [ + 'main' => $firewall, + ], + ]); + + $container->compile(); + } + + public function provideEntryPointRequiredData() + { + // more than one entry point available and not explicitly set + yield [ + ['http_basic' => true, 'form_login' => true], + '/^Because you have multiple authenticators in firewall "main", you need to set the "entry_point" key to one of your authenticators/', + ]; + // more than one guard entry point available and not explicitly set + yield [ + ['guard' => ['authenticators' => [AppCustomAuthenticator::class, NullAuthenticator::class]]], + '/^Because you have multiple guard authenticators, you need to set the "entry_point" key to one of your authenticators/', + ]; + } + protected function getRawContainer() { $container = new ContainerBuilder(); @@ -439,3 +531,42 @@ class SecurityExtensionTest extends TestCase return $container; } } + +class NullAuthenticator implements AuthenticatorInterface +{ + public function start(Request $request, AuthenticationException $authException = null) + { + } + + public function supports(Request $request) + { + } + + public function getCredentials(Request $request) + { + } + + public function getUser($credentials, UserProviderInterface $userProvider) + { + } + + public function checkCredentials($credentials, UserInterface $user) + { + } + + public function createAuthenticatedToken(UserInterface $user, string $providerKey) + { + } + + public function onAuthenticationFailure(Request $request, AuthenticationException $exception) + { + } + + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $providerKey) + { + } + + public function supportsRememberMe() + { + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index b06d8b4c3a..6ef832935e 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -19,7 +19,7 @@ "php": "^7.2.5", "ext-xml": "*", "symfony/config": "^4.4|^5.0", - "symfony/dependency-injection": "^4.4|^5.0", + "symfony/dependency-injection": "^5.1", "symfony/event-dispatcher": "^5.1", "symfony/http-kernel": "^5.0", "symfony/polyfill-php80": "^1.15",