From fe3de3fad8be8cdca18ba1dbd4ff85a27322fe0c Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Mon, 27 Apr 2020 14:04:29 +0200 Subject: [PATCH] [Security] Require entry_point to be configured with multiple authenticators --- CHANGELOG.md | 2 ++ .../Security/Factory/AbstractFactory.php | 6 ++++-- .../Security/Factory/AnonymousFactory.php | 2 ++ .../Factory/CustomAuthenticatorFactory.php | 6 ++++++ .../Factory/EntryPointFactoryInterface.php | 2 +- .../Security/Factory/FormLoginFactory.php | 9 ++++++++- .../Security/Factory/FormLoginLdapFactory.php | 2 ++ .../Factory/GuardAuthenticationFactory.php | 12 ++++++++++-- .../Security/Factory/HttpBasicFactory.php | 15 ++++++++------- .../Security/Factory/HttpBasicLdapFactory.php | 2 ++ .../Security/Factory/JsonLoginFactory.php | 2 ++ .../Security/Factory/JsonLoginLdapFactory.php | 2 ++ .../Security/Factory/RememberMeFactory.php | 3 +++ .../Security/Factory/RemoteUserFactory.php | 2 ++ .../Security/Factory/X509Factory.php | 2 ++ DependencyInjection/SecurityExtension.php | 19 +++++++++++++++++-- 16 files changed, 73 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5995cb18..615aceb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ CHANGELOG * Added XSD for configuration * Added security configuration for priority-based access decision strategy + * Marked the `AbstractFactory`, `AnonymousFactory`, `FormLoginFactory`, `FormLoginLdapFactory`, `GuardAuthenticationFactory`, `HttpBasicFactory`, `HttpBasicLdapFactory`, `JsonLoginFactory`, `JsonLoginLdapFactory`, `RememberMeFactory`, `RemoteUserFactory` and `X509Factory` as `@internal` + * Renamed method `AbstractFactory#createEntryPoint()` to `AbstractFactory#createDefaultEntryPoint()` 5.0.0 ----- diff --git a/DependencyInjection/Security/Factory/AbstractFactory.php b/DependencyInjection/Security/Factory/AbstractFactory.php index a5d6f7e4..c31e08ba 100644 --- a/DependencyInjection/Security/Factory/AbstractFactory.php +++ b/DependencyInjection/Security/Factory/AbstractFactory.php @@ -23,6 +23,8 @@ * @author Fabien Potencier * @author Lukas Kahwe Smith * @author Johannes M. Schmitt + * + * @internal */ abstract class AbstractFactory implements SecurityFactoryInterface { @@ -65,7 +67,7 @@ public function create(ContainerBuilder $container, string $id, array $config, s } // create entry point if applicable (optional) - $entryPointId = $this->createEntryPoint($container, $id, $config, $defaultEntryPointId); + $entryPointId = $this->createDefaultEntryPoint($container, $id, $config, $defaultEntryPointId); return [$authProviderId, $listenerId, $entryPointId]; } @@ -126,7 +128,7 @@ abstract protected function getListenerId(); * * @return string|null the entry point id */ - protected function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId) + protected function createDefaultEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId) { return $defaultEntryPointId; } diff --git a/DependencyInjection/Security/Factory/AnonymousFactory.php b/DependencyInjection/Security/Factory/AnonymousFactory.php index 53a6b503..7caff9fa 100644 --- a/DependencyInjection/Security/Factory/AnonymousFactory.php +++ b/DependencyInjection/Security/Factory/AnonymousFactory.php @@ -18,6 +18,8 @@ /** * @author Wouter de Jong + * + * @internal */ class AnonymousFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { diff --git a/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php b/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php index 95fa3c05..35984ca8 100644 --- a/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php +++ b/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php @@ -15,6 +15,12 @@ use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; +/** + * @author Wouter de Jong + * + * @internal + * @experimental in Symfony 5.1 + */ class CustomAuthenticatorFactory implements AuthenticatorFactoryInterface, SecurityFactoryInterface { public function create(ContainerBuilder $container, string $id, array $config, string $userProvider, ?string $defaultEntryPoint) diff --git a/DependencyInjection/Security/Factory/EntryPointFactoryInterface.php b/DependencyInjection/Security/Factory/EntryPointFactoryInterface.php index bf0e625f..0b56e309 100644 --- a/DependencyInjection/Security/Factory/EntryPointFactoryInterface.php +++ b/DependencyInjection/Security/Factory/EntryPointFactoryInterface.php @@ -23,5 +23,5 @@ interface EntryPointFactoryInterface /** * Creates the entry point and returns the service ID. */ - public function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId): string; + public function createEntryPoint(ContainerBuilder $container, string $id, array $config): ?string; } diff --git a/DependencyInjection/Security/Factory/FormLoginFactory.php b/DependencyInjection/Security/Factory/FormLoginFactory.php index c5f247c3..92ce5052 100644 --- a/DependencyInjection/Security/Factory/FormLoginFactory.php +++ b/DependencyInjection/Security/Factory/FormLoginFactory.php @@ -22,6 +22,8 @@ * * @author Fabien Potencier * @author Johannes M. Schmitt + * + * @internal */ class FormLoginFactory extends AbstractFactory implements AuthenticatorFactoryInterface, EntryPointFactoryInterface { @@ -90,7 +92,12 @@ protected function createListener(ContainerBuilder $container, string $id, array return $listenerId; } - public function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPoint): string + protected function createDefaultEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId) + { + return $this->createEntryPoint($container, $id, $config); + } + + public function createEntryPoint(ContainerBuilder $container, string $id, array $config): string { $entryPointId = 'security.authentication.form_entry_point.'.$id; $container diff --git a/DependencyInjection/Security/Factory/FormLoginLdapFactory.php b/DependencyInjection/Security/Factory/FormLoginLdapFactory.php index b2136c50..3d6d119b 100644 --- a/DependencyInjection/Security/Factory/FormLoginLdapFactory.php +++ b/DependencyInjection/Security/Factory/FormLoginLdapFactory.php @@ -22,6 +22,8 @@ * * @author Grégoire Pineau * @author Charles Sarrazin + * + * @internal */ class FormLoginLdapFactory extends FormLoginFactory { diff --git a/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php b/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php index a18dfefa..283da743 100644 --- a/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php +++ b/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php @@ -23,6 +23,8 @@ * Configures the "guard" authentication provider key under a firewall. * * @author Ryan Weaver + * + * @internal */ class GuardAuthenticationFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface, EntryPointFactoryInterface { @@ -111,9 +113,15 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal return $authenticatorIds; } - public function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPointId): string + public function createEntryPoint(ContainerBuilder $container, string $id, array $config): ?string { - return $this->determineEntryPoint($defaultEntryPointId, $config); + try { + return $this->determineEntryPoint(null, $config); + } catch (\LogicException $e) { + // ignore the exception, the new system prefers setting "entry_point" over "guard.entry_point" + } + + return null; } private function determineEntryPoint(?string $defaultEntryPointId, array $config): string diff --git a/DependencyInjection/Security/Factory/HttpBasicFactory.php b/DependencyInjection/Security/Factory/HttpBasicFactory.php index a698d2a1..5dfe0747 100644 --- a/DependencyInjection/Security/Factory/HttpBasicFactory.php +++ b/DependencyInjection/Security/Factory/HttpBasicFactory.php @@ -20,8 +20,10 @@ * HttpBasicFactory creates services for HTTP basic authentication. * * @author Fabien Potencier + * + * @internal */ -class HttpBasicFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface +class HttpBasicFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface, EntryPointFactoryInterface { public function create(ContainerBuilder $container, string $id, array $config, string $userProvider, ?string $defaultEntryPoint) { @@ -34,7 +36,10 @@ public function create(ContainerBuilder $container, string $id, array $config, s ; // entry point - $entryPointId = $this->createEntryPoint($container, $id, $config, $defaultEntryPoint); + $entryPointId = $defaultEntryPoint; + if (null === $entryPointId) { + $entryPointId = $this->createEntryPoint($container, $id, $config); + } // listener $listenerId = 'security.authentication.listener.basic.'.$id; @@ -77,12 +82,8 @@ public function addConfiguration(NodeDefinition $node) ; } - protected function createEntryPoint(ContainerBuilder $container, string $id, array $config, ?string $defaultEntryPoint) + public function createEntryPoint(ContainerBuilder $container, string $id, array $config): string { - if (null !== $defaultEntryPoint) { - return $defaultEntryPoint; - } - $entryPointId = 'security.authentication.basic_entry_point.'.$id; $container ->setDefinition($entryPointId, new ChildDefinition('security.authentication.basic_entry_point')) diff --git a/DependencyInjection/Security/Factory/HttpBasicLdapFactory.php b/DependencyInjection/Security/Factory/HttpBasicLdapFactory.php index 630e0b75..3e0bf5b0 100644 --- a/DependencyInjection/Security/Factory/HttpBasicLdapFactory.php +++ b/DependencyInjection/Security/Factory/HttpBasicLdapFactory.php @@ -23,6 +23,8 @@ * @author Fabien Potencier * @author Grégoire Pineau * @author Charles Sarrazin + * + * @internal */ class HttpBasicLdapFactory extends HttpBasicFactory { diff --git a/DependencyInjection/Security/Factory/JsonLoginFactory.php b/DependencyInjection/Security/Factory/JsonLoginFactory.php index 7aa90405..393c5539 100644 --- a/DependencyInjection/Security/Factory/JsonLoginFactory.php +++ b/DependencyInjection/Security/Factory/JsonLoginFactory.php @@ -19,6 +19,8 @@ * JsonLoginFactory creates services for JSON login authentication. * * @author Kévin Dunglas + * + * @internal */ class JsonLoginFactory extends AbstractFactory implements AuthenticatorFactoryInterface { diff --git a/DependencyInjection/Security/Factory/JsonLoginLdapFactory.php b/DependencyInjection/Security/Factory/JsonLoginLdapFactory.php index 6428f61c..ba0d7136 100644 --- a/DependencyInjection/Security/Factory/JsonLoginLdapFactory.php +++ b/DependencyInjection/Security/Factory/JsonLoginLdapFactory.php @@ -19,6 +19,8 @@ /** * JsonLoginLdapFactory creates services for json login ldap authentication. + * + * @internal */ class JsonLoginLdapFactory extends JsonLoginFactory { diff --git a/DependencyInjection/Security/Factory/RememberMeFactory.php b/DependencyInjection/Security/Factory/RememberMeFactory.php index 4b29db1a..884c7b57 100644 --- a/DependencyInjection/Security/Factory/RememberMeFactory.php +++ b/DependencyInjection/Security/Factory/RememberMeFactory.php @@ -20,6 +20,9 @@ use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\Security\Http\EventListener\RememberMeLogoutListener; +/** + * @internal + */ class RememberMeFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { protected $options = [ diff --git a/DependencyInjection/Security/Factory/RemoteUserFactory.php b/DependencyInjection/Security/Factory/RemoteUserFactory.php index e25c3c7d..fc2e49f6 100644 --- a/DependencyInjection/Security/Factory/RemoteUserFactory.php +++ b/DependencyInjection/Security/Factory/RemoteUserFactory.php @@ -21,6 +21,8 @@ * * @author Fabien Potencier * @author Maxime Douailin + * + * @internal */ class RemoteUserFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { diff --git a/DependencyInjection/Security/Factory/X509Factory.php b/DependencyInjection/Security/Factory/X509Factory.php index f966302a..56a25653 100644 --- a/DependencyInjection/Security/Factory/X509Factory.php +++ b/DependencyInjection/Security/Factory/X509Factory.php @@ -20,6 +20,8 @@ * X509Factory creates services for X509 certificate authentication. * * @author Fabien Potencier + * + * @internal */ class X509Factory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { diff --git a/DependencyInjection/SecurityExtension.php b/DependencyInjection/SecurityExtension.php index ac089d1e..5d65aea6 100644 --- a/DependencyInjection/SecurityExtension.php +++ b/DependencyInjection/SecurityExtension.php @@ -39,6 +39,7 @@ use Symfony\Component\Security\Core\User\ChainUserProvider; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Http\Controller\UserValueResolver; +use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; use Twig\Extension\AbstractExtension; /** @@ -519,6 +520,7 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri { $listeners = []; $hasListeners = false; + $entryPoints = []; foreach ($this->listenerPositions as $position) { foreach ($this->factories[$position] as $factory) { @@ -541,8 +543,8 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri $authenticationProviders[] = $authenticators; } - if ($factory instanceof EntryPointFactoryInterface) { - $defaultEntryPoint = $factory->createEntryPoint($container, $id, $firewall[$key], $defaultEntryPoint); + if ($factory instanceof EntryPointFactoryInterface && ($entryPoint = $factory->createEntryPoint($container, $id, $firewall[$key], null))) { + $entryPoints[$key] = $entryPoint; } } else { list($provider, $listenerId, $defaultEntryPoint) = $factory->create($container, $id, $firewall[$key], $userProvider, $defaultEntryPoint); @@ -555,6 +557,19 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri } } + if ($entryPoints) { + // we can be sure the authenticator system is enabled + if (null !== $defaultEntryPoint) { + return $entryPoints[$defaultEntryPoint] ?? $defaultEntryPoint; + } + + if (1 === \count($entryPoints)) { + return current($entryPoints); + } + + throw new InvalidConfigurationException(sprintf('Because you have multiple authenticators in firewall "%s", you need to set the "entry_point" key to one of your authenticators (%s) or a service ID implementing "%s". The "entry_point" determines what should happen (e.g. redirect to "/login") when an anonymous user tries to access a protected page.', $id, implode(', ', $entryPoints), AuthenticationEntryPointInterface::class)); + } + if (false === $hasListeners) { throw new InvalidConfigurationException(sprintf('No authentication listener registered for firewall "%s".', $id)); }