From 7385fd5c441fb63df7f3112967e1857076cba9c9 Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sun, 20 Jun 2021 14:15:35 +0200 Subject: [PATCH] [SecurityBundle] Create a smooth upgrade path for security factories --- UPGRADE-5.4.md | 15 ++ UPGRADE-6.0.md | 15 ++ .../Bundle/SecurityBundle/CHANGELOG.md | 5 + .../DependencyInjection/MainConfiguration.php | 33 ++-- .../Security/Factory/AnonymousFactory.php | 5 + .../Factory/AuthenticatorFactoryInterface.php | 13 ++ .../Factory/CustomAuthenticatorFactory.php | 5 + .../Security/Factory/FormLoginFactory.php | 7 + .../Factory/GuardAuthenticationFactory.php | 5 + .../Security/Factory/HttpBasicFactory.php | 7 + .../Security/Factory/JsonLoginFactory.php | 7 + .../Security/Factory/LoginLinkFactory.php | 7 + .../Factory/LoginThrottlingFactory.php | 6 + .../Security/Factory/RememberMeFactory.php | 36 ++++- .../Security/Factory/RemoteUserFactory.php | 7 + .../Factory/SecurityFactoryInterface.php | 2 + .../Security/Factory/X509Factory.php | 7 + .../DependencyInjection/SecurityExtension.php | 144 ++++++++++-------- .../Bundle/SecurityBundle/SecurityBundle.php | 30 ++-- .../MainConfigurationTest.php | 27 ++++ 20 files changed, 290 insertions(+), 93 deletions(-) diff --git a/UPGRADE-5.4.md b/UPGRADE-5.4.md index af00fea8aa356..e547f86b1e05f 100644 --- a/UPGRADE-5.4.md +++ b/UPGRADE-5.4.md @@ -37,6 +37,21 @@ Messenger SecurityBundle -------------- + * Deprecate `SecurityFactoryInterface` and `SecurityExtension::addSecurityListenerFactory()` in favor of + `AuthenticatorFactoryInterface` and `SecurityExtension::addAuthenticatorFactory()` + * Add `AuthenticatorFactoryInterface::getPriority()` which replaces `SecurityFactoryInterface::getPosition()`. + Previous positions are mapped to the following priorities: + + | Position | Constant | Priority | + | ----------- | ----------------------------------------------------- | -------- | + | pre_auth | `RemoteUserFactory::PRIORITY`/`X509Factory::PRIORITY` | -10 | + | form | `FormLoginFactory::PRIORITY` | -30 | + | http | `HttpBasicFactory::PRIORITY` | -50 | + | remember_me | `RememberMeFactory::PRIORITY` | -60 | + | anonymous | n/a | -70 | + + * Deprecate passing an array of arrays as 1st argument to `MainConfiguration`, pass a sorted flat array of + factories instead. * Deprecate the `always_authenticate_before_granting` option Security diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 4b5d579533d71..bb5863fb304f2 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -355,6 +355,21 @@ Security SecurityBundle -------------- + * Remove `SecurityFactoryInterface` and `SecurityExtension::addSecurityListenerFactory()` in favor of + `AuthenticatorFactoryInterface` and `SecurityExtension::addAuthenticatorFactory()` + * Add `AuthenticatorFactoryInterface::getPriority()` which replaces `SecurityFactoryInterface::getPosition()`. + Previous positions are mapped to the following priorities: + + | Position | Constant | Priority | + | ----------- | ----------------------------------------------------- | -------- | + | pre_auth | `RemoteUserFactory::PRIORITY`/`X509Factory::PRIORITY` | -10 | + | form | `FormLoginFactory::PRIORITY` | -30 | + | http | `HttpBasicFactory::PRIORITY` | -50 | + | remember_me | `RememberMeFactory::PRIORITY` | -60 | + | anonymous | n/a | -70 | + + * Remove passing an array of arrays as 1st argument to `MainConfiguration`, pass a sorted flat array of + factories instead. * Remove the `always_authenticate_before_granting` option * Remove the `UserPasswordEncoderCommand` class and the corresponding `user:encode-password` command, use `UserPasswordHashCommand` and `user:hash-password` instead diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index 807e75536a005..6bbccb8b81e53 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -4,6 +4,11 @@ CHANGELOG 5.4 --- + * Deprecate `SecurityFactoryInterface` and `SecurityExtension::addSecurityListenerFactory()` in favor of + `AuthenticatorFactoryInterface` and `SecurityExtension::addAuthenticatorFactory()` + * Add `AuthenticatorFactoryInterface::getPriority()` which replaces `SecurityFactoryInterface::getPosition()` + * Deprecate passing an array of arrays as 1st argument to `MainConfiguration`, pass a sorted flat array of + factories instead. * Deprecate the `always_authenticate_before_granting` option 5.3 diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index a389ee873704d..6bdea1de8819d 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -12,6 +12,8 @@ namespace Symfony\Bundle\SecurityBundle\DependencyInjection; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -31,8 +33,17 @@ class MainConfiguration implements ConfigurationInterface private $factories; private $userProviderFactories; + /** + * @param (SecurityFactoryInterface|AuthenticatorFactoryInterface)[] $factories + */ public function __construct(array $factories, array $userProviderFactories) { + if (\is_array(current($factories))) { + trigger_deprecation('symfony/security-bundle', '5.4', 'Passing an array of arrays as 1st argument to "%s" is deprecated, pass a sorted array of factories instead.', __METHOD__); + + $factories = array_merge(...array_values($factories)); + } + $this->factories = $factories; $this->userProviderFactories = $userProviderFactories; } @@ -297,19 +308,17 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto ; $abstractFactoryKeys = []; - foreach ($factories as $factoriesAtPosition) { - foreach ($factoriesAtPosition as $factory) { - $name = str_replace('-', '_', $factory->getKey()); - $factoryNode = $firewallNodeBuilder->arrayNode($name) - ->canBeUnset() - ; - - if ($factory instanceof AbstractFactory) { - $abstractFactoryKeys[] = $name; - } - - $factory->addConfiguration($factoryNode); + foreach ($factories as $factory) { + $name = str_replace('-', '_', $factory->getKey()); + $factoryNode = $firewallNodeBuilder->arrayNode($name) + ->canBeUnset() + ; + + if ($factory instanceof AbstractFactory) { + $abstractFactoryKeys[] = $name; } + + $factory->addConfiguration($factoryNode); } // check for unreachable check paths diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php index ded4a61740d53..54128f43b6d96 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php @@ -52,6 +52,11 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal throw new InvalidConfigurationException(sprintf('The authenticator manager no longer has "anonymous" security. Please remove this option under the "%s" firewall'.($config['lazy'] ? ' and add "lazy: true"' : '').'.', $firewallName)); } + public function getPriority() + { + return -60; + } + public function getPosition() { return 'anonymous'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AuthenticatorFactoryInterface.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AuthenticatorFactoryInterface.php index d4fef81e247b4..6ecec3e281ae2 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AuthenticatorFactoryInterface.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AuthenticatorFactoryInterface.php @@ -11,9 +11,12 @@ namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory; +use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; /** + * @method int getPriority() defines the position at which the authenticator is called + * * @author Wouter de Jong */ interface AuthenticatorFactoryInterface @@ -24,4 +27,14 @@ interface AuthenticatorFactoryInterface * @return string|string[] The authenticator service ID(s) to be used by the firewall */ public function createAuthenticator(ContainerBuilder $container, string $firewallName, array $config, string $userProviderId); + + /** + * Defines the configuration key used to reference the authenticator + * in the firewall configuration. + * + * @return string + */ + public function getKey(); + + public function addConfiguration(NodeDefinition $builder); } diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php index a478de2c8d8a4..da8aba9f52ce3 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/CustomAuthenticatorFactory.php @@ -27,6 +27,11 @@ public function create(ContainerBuilder $container, string $id, array $config, s throw new \LogicException('Custom authenticators are not supported when "security.enable_authenticator_manager" is not set to true.'); } + public function getPriority(): int + { + return 0; + } + public function getPosition(): string { return 'pre_auth'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FormLoginFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FormLoginFactory.php index 3f4f6a16909b1..6da9514cc1db6 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FormLoginFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FormLoginFactory.php @@ -27,6 +27,8 @@ */ class FormLoginFactory extends AbstractFactory implements AuthenticatorFactoryInterface { + public const PRIORITY = -30; + public function __construct() { $this->addOption('username_parameter', '_username'); @@ -37,6 +39,11 @@ public function __construct() $this->addOption('post_only', true); } + public function getPriority(): int + { + return self::PRIORITY; + } + public function getPosition() { return 'form'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php index f60666e9dc772..ffae89ba9d4d0 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php @@ -34,6 +34,11 @@ public function getPosition() return 'pre_auth'; } + public function getPriority(): int + { + return 0; + } + public function getKey() { return 'guard'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicFactory.php index 784878b9ed775..629bc0a0430ca 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicFactory.php @@ -25,6 +25,8 @@ */ class HttpBasicFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { + public const PRIORITY = -50; + public function create(ContainerBuilder $container, string $id, array $config, string $userProvider, ?string $defaultEntryPoint) { $provider = 'security.authentication.provider.dao.'.$id; @@ -66,6 +68,11 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal return $authenticatorId; } + public function getPriority(): int + { + return self::PRIORITY; + } + public function getPosition() { return 'http'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php index 7458a35b0e6be..285197ce03fc9 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php @@ -24,6 +24,8 @@ */ class JsonLoginFactory extends AbstractFactory implements AuthenticatorFactoryInterface { + public const PRIORITY = -40; + public function __construct() { $this->addOption('username_path', 'username'); @@ -32,6 +34,11 @@ public function __construct() $this->defaultSuccessHandlerOptions = []; } + public function getPriority(): int + { + return self::PRIORITY; + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginLinkFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginLinkFactory.php index de426df457c5b..d702ce2893bce 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginLinkFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginLinkFactory.php @@ -27,6 +27,8 @@ */ class LoginLinkFactory extends AbstractFactory implements AuthenticatorFactoryInterface { + public const PRIORITY = -20; + public function addConfiguration(NodeDefinition $node) { /** @var NodeBuilder $builder */ @@ -147,6 +149,11 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal return $authenticatorId; } + public function getPriority(): int + { + return self::PRIORITY; + } + public function getPosition() { return 'form'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php index 111a2a062e1b5..160040e45ee67 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php @@ -34,6 +34,12 @@ public function create(ContainerBuilder $container, string $id, array $config, s throw new \LogicException('Login throttling is not supported when "security.enable_authenticator_manager" is not set to true.'); } + public function getPriority(): int + { + // this factory doesn't register any authenticators, this priority doesn't matter + return 0; + } + public function getPosition(): string { // this factory doesn't register any authenticators, this position doesn't matter diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php index de19f488454f2..8416d1591acaa 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php @@ -21,6 +21,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpFoundation\Cookie; @@ -30,8 +31,10 @@ /** * @internal */ -class RememberMeFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface +class RememberMeFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface, PrependExtensionInterface { + public const PRIORITY = -50; + protected $options = [ 'name' => 'REMEMBERME', 'lifetime' => 31536000, @@ -181,6 +184,14 @@ public function getPosition() return 'remember_me'; } + /** + * {@inheritDoc} + */ + public function getPriority(): int + { + return self::PRIORITY; + } + public function getKey() { return 'remember-me'; @@ -331,4 +342,27 @@ private function createTokenVerifier(ContainerBuilder $container, string $firewa return new Reference($tokenVerifierId, ContainerInterface::NULL_ON_INVALID_REFERENCE); } + + /** + * {@inheritdoc} + */ + public function prepend(ContainerBuilder $container) + { + $rememberMeSecureDefault = false; + $rememberMeSameSiteDefault = null; + + if (!isset($container->getExtensions()['framework'])) { + return; + } + + foreach ($container->getExtensionConfig('framework') as $config) { + if (isset($config['session']) && \is_array($config['session'])) { + $rememberMeSecureDefault = $config['session']['cookie_secure'] ?? $rememberMeSecureDefault; + $rememberMeSameSiteDefault = \array_key_exists('cookie_samesite', $config['session']) ? $config['session']['cookie_samesite'] : $rememberMeSameSiteDefault; + } + } + + $this->options['secure'] = $rememberMeSecureDefault; + $this->options['samesite'] = $rememberMeSameSiteDefault; + } } diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RemoteUserFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RemoteUserFactory.php index fc2e49f6f0819..0e002651ff0f1 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RemoteUserFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RemoteUserFactory.php @@ -26,6 +26,8 @@ */ class RemoteUserFactory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { + public const PRIORITY = -10; + public function create(ContainerBuilder $container, string $id, array $config, string $userProvider, ?string $defaultEntryPoint) { $providerId = 'security.authentication.provider.pre_authenticated.'.$id; @@ -58,6 +60,11 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal return $authenticatorId; } + public function getPriority(): int + { + return self::PRIORITY; + } + public function getPosition() { return 'pre_auth'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/SecurityFactoryInterface.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/SecurityFactoryInterface.php index 4a1497aec1640..4551a6cbcc11b 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/SecurityFactoryInterface.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/SecurityFactoryInterface.php @@ -18,6 +18,8 @@ * SecurityFactoryInterface is the interface for all security authentication listener. * * @author Fabien Potencier + * + * @deprecated since Symfony 5.3, use AuthenticatorFactoryInterface instead. */ interface SecurityFactoryInterface { diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/X509Factory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/X509Factory.php index 56a25653af9b5..8f06cbb20df0d 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/X509Factory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/X509Factory.php @@ -25,6 +25,8 @@ */ class X509Factory implements SecurityFactoryInterface, AuthenticatorFactoryInterface { + public const PRIORITY = -10; + public function create(ContainerBuilder $container, string $id, array $config, string $userProvider, ?string $defaultEntryPoint) { $providerId = 'security.authentication.provider.pre_authenticated.'.$id; @@ -60,6 +62,11 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal return $authenticatorId; } + public function getPriority(): int + { + return self::PRIORITY; + } + public function getPosition() { return 'pre_auth'; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index f1ce0a9aabef2..5d8267a249b84 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -14,7 +14,6 @@ use Symfony\Bridge\Twig\Extension\LogoutUrlExtension; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FirewallListenerFactoryInterface; -use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\RememberMeFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\UserProviderFactoryInterface; use Symfony\Bundle\SecurityBundle\Security\LegacyLogoutHandlerListener; @@ -58,42 +57,20 @@ class SecurityExtension extends Extension implements PrependExtensionInterface private $requestMatchers = []; private $expressions = []; private $contextListeners = []; - private $listenerPositions = ['pre_auth', 'form', 'http', 'remember_me', 'anonymous']; + /** @var array */ private $factories = []; + /** @var (AuthenticatorFactoryInterface|SecurityFactoryInterface)[] */ + private $sortedFactories = []; private $userProviderFactories = []; private $statelessFirewallKeys = []; private $authenticatorManagerEnabled = false; - public function __construct() - { - foreach ($this->listenerPositions as $position) { - $this->factories[$position] = []; - } - } - public function prepend(ContainerBuilder $container) { - $rememberMeSecureDefault = false; - $rememberMeSameSiteDefault = null; - - if (!isset($container->getExtensions()['framework'])) { - return; - } - foreach ($container->getExtensionConfig('framework') as $config) { - if (isset($config['session']) && \is_array($config['session'])) { - $rememberMeSecureDefault = $config['session']['cookie_secure'] ?? $rememberMeSecureDefault; - $rememberMeSameSiteDefault = \array_key_exists('cookie_samesite', $config['session']) ? $config['session']['cookie_samesite'] : $rememberMeSameSiteDefault; - } - } - foreach ($this->listenerPositions as $position) { - foreach ($this->factories[$position] as $factory) { - if ($factory instanceof RememberMeFactory) { - \Closure::bind(function () use ($rememberMeSecureDefault, $rememberMeSameSiteDefault) { - $this->options['secure'] = $rememberMeSecureDefault; - $this->options['samesite'] = $rememberMeSameSiteDefault; - }, $factory, $factory)(); - } + foreach ($this->getSortedFactories() as $factory) { + if ($factory instanceof PrependExtensionInterface) { + $factory->prepend($container); } } } @@ -556,12 +533,10 @@ private function createFirewall(ContainerBuilder $container, string $id, array $ $container->setAlias('security.user_checker.'.$id, new Alias($firewall['user_checker'], false)); - foreach ($this->factories as $position) { - foreach ($position as $factory) { - $key = str_replace('-', '_', $factory->getKey()); - if (\array_key_exists($key, $firewall)) { - $listenerKeys[] = $key; - } + foreach ($this->getSortedFactories() as $factory) { + $key = str_replace('-', '_', $factory->getKey()); + if (\array_key_exists($key, $firewall)) { + $listenerKeys[] = $key; } } @@ -594,44 +569,42 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri $hasListeners = false; $entryPoints = []; - foreach ($this->listenerPositions as $position) { - foreach ($this->factories[$position] as $factory) { - $key = str_replace('-', '_', $factory->getKey()); + foreach ($this->getSortedFactories() as $factory) { + $key = str_replace('-', '_', $factory->getKey()); - if (isset($firewall[$key])) { - $userProvider = $this->getUserProvider($container, $id, $firewall, $key, $defaultProvider, $providerIds, $contextListenerId); + if (isset($firewall[$key])) { + $userProvider = $this->getUserProvider($container, $id, $firewall, $key, $defaultProvider, $providerIds, $contextListenerId); - if ($this->authenticatorManagerEnabled) { - if (!$factory instanceof AuthenticatorFactoryInterface) { - throw new InvalidConfigurationException(sprintf('Cannot configure AuthenticatorManager as "%s" authentication does not support it, set "security.enable_authenticator_manager" to `false`.', $key)); - } + if ($this->authenticatorManagerEnabled) { + if (!$factory instanceof AuthenticatorFactoryInterface) { + throw new InvalidConfigurationException(sprintf('Cannot configure AuthenticatorManager as "%s" authentication does not support it, set "security.enable_authenticator_manager" to `false`.', $key)); + } - $authenticators = $factory->createAuthenticator($container, $id, $firewall[$key], $userProvider); - if (\is_array($authenticators)) { - foreach ($authenticators as $authenticator) { - $authenticationProviders[] = $authenticator; - $entryPoints[] = $authenticator; - } - } else { - $authenticationProviders[] = $authenticators; - $entryPoints[$key] = $authenticators; + $authenticators = $factory->createAuthenticator($container, $id, $firewall[$key], $userProvider); + if (\is_array($authenticators)) { + foreach ($authenticators as $authenticator) { + $authenticationProviders[] = $authenticator; + $entryPoints[] = $authenticator; } } else { - [$provider, $listenerId, $defaultEntryPoint] = $factory->create($container, $id, $firewall[$key], $userProvider, $defaultEntryPoint); - - $listeners[] = new Reference($listenerId); - $authenticationProviders[] = $provider; + $authenticationProviders[] = $authenticators; + $entryPoints[$key] = $authenticators; } + } else { + [$provider, $listenerId, $defaultEntryPoint] = $factory->create($container, $id, $firewall[$key], $userProvider, $defaultEntryPoint); - if ($factory instanceof FirewallListenerFactoryInterface) { - $firewallListenerIds = $factory->createListeners($container, $id, $firewall[$key]); - foreach ($firewallListenerIds as $firewallListenerId) { - $listeners[] = new Reference($firewallListenerId); - } - } + $listeners[] = new Reference($listenerId); + $authenticationProviders[] = $provider; + } - $hasListeners = true; + if ($factory instanceof FirewallListenerFactoryInterface) { + $firewallListenerIds = $factory->createListeners($container, $id, $firewall[$key]); + foreach ($firewallListenerIds as $firewallListenerId) { + $listeners[] = new Reference($firewallListenerId); + } } + + $hasListeners = true; } } @@ -1062,9 +1035,27 @@ private function createRequestMatcher(ContainerBuilder $container, string $path return $this->requestMatchers[$id] = new Reference($id); } + /** + * @deprecated since 5.4, use "addAuthenticatorFactory()" instead + */ public function addSecurityListenerFactory(SecurityFactoryInterface $factory) { - $this->factories[$factory->getPosition()][] = $factory; + trigger_deprecation('symfony/security-bundle', '5.4', 'Method "%s()" is deprecated, use "addAuthenticatorFactory()" instead.', __METHOD__); + + $this->factories[] = [[ + 'pre_auth' => -10, + 'form' => -30, + 'http' => -40, + 'remember_me' => -50, + 'anonymous' => -60, + ][$factory->getPosition()], $factory]; + $this->sortedFactories = []; + } + + public function addAuthenticatorFactory(AuthenticatorFactoryInterface $factory) + { + $this->factories[] = [method_exists($factory, 'getPriority') ? $factory->getPriority() : 0, $factory]; + $this->sortedFactories = []; } public function addUserProviderFactory(UserProviderFactoryInterface $factory) @@ -1088,7 +1079,7 @@ public function getNamespace() public function getConfiguration(array $config, ContainerBuilder $container) { // first assemble the factories - return new MainConfiguration($this->factories, $this->userProviderFactories); + return new MainConfiguration($this->getSortedFactories(), $this->userProviderFactories); } private function isValidIps($ips): bool @@ -1135,4 +1126,25 @@ private function isValidIp(string $cidr): bool return false; } + + /** + * @return (AuthenticatorFactoryInterface|SecurityFactoryInterface)[] + */ + private function getSortedFactories(): array + { + if (!$this->sortedFactories) { + $factories = []; + foreach ($this->factories as $i => $factory) { + $factories[] = array_merge($factory, [$i]); + } + + usort($factories, function ($a, $b) { + return $b[0] <=> $a[0] ?: $a[2] <=> $b[2]; + }); + + $this->sortedFactories = array_column($factories, 1); + } + + return $this->sortedFactories; + } } diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index 0798a3627da7f..4c2c3046f004f 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -38,6 +38,7 @@ use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\X509Factory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\InMemoryFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\LdapFactory; +use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\EventDispatcher\DependencyInjection\AddEventAliasesPass; @@ -56,21 +57,22 @@ public function build(ContainerBuilder $container) { parent::build($container); + /** @var SecurityExtension $extension */ $extension = $container->getExtension('security'); - $extension->addSecurityListenerFactory(new FormLoginFactory()); - $extension->addSecurityListenerFactory(new FormLoginLdapFactory()); - $extension->addSecurityListenerFactory(new JsonLoginFactory()); - $extension->addSecurityListenerFactory(new JsonLoginLdapFactory()); - $extension->addSecurityListenerFactory(new HttpBasicFactory()); - $extension->addSecurityListenerFactory(new HttpBasicLdapFactory()); - $extension->addSecurityListenerFactory(new RememberMeFactory()); - $extension->addSecurityListenerFactory(new X509Factory()); - $extension->addSecurityListenerFactory(new RemoteUserFactory()); - $extension->addSecurityListenerFactory(new GuardAuthenticationFactory()); - $extension->addSecurityListenerFactory(new AnonymousFactory()); - $extension->addSecurityListenerFactory(new CustomAuthenticatorFactory()); - $extension->addSecurityListenerFactory(new LoginThrottlingFactory()); - $extension->addSecurityListenerFactory(new LoginLinkFactory()); + $extension->addAuthenticatorFactory(new FormLoginFactory()); + $extension->addAuthenticatorFactory(new FormLoginLdapFactory()); + $extension->addAuthenticatorFactory(new JsonLoginFactory()); + $extension->addAuthenticatorFactory(new JsonLoginLdapFactory()); + $extension->addAuthenticatorFactory(new HttpBasicFactory()); + $extension->addAuthenticatorFactory(new HttpBasicLdapFactory()); + $extension->addAuthenticatorFactory(new RememberMeFactory()); + $extension->addAuthenticatorFactory(new X509Factory()); + $extension->addAuthenticatorFactory(new RemoteUserFactory()); + $extension->addAuthenticatorFactory(new GuardAuthenticationFactory()); + $extension->addAuthenticatorFactory(new AnonymousFactory()); + $extension->addAuthenticatorFactory(new CustomAuthenticatorFactory()); + $extension->addAuthenticatorFactory(new LoginThrottlingFactory()); + $extension->addAuthenticatorFactory(new LoginLinkFactory()); $extension->addUserProviderFactory(new InMemoryFactory()); $extension->addUserProviderFactory(new LdapFactory()); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php index acdfff8d1639a..8ebb389beff79 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php @@ -12,12 +12,16 @@ namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Bundle\SecurityBundle\DependencyInjection\MainConfiguration; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\Definition\Processor; class MainConfigurationTest extends TestCase { + use ExpectDeprecationTrait; + /** * The minimal, required config needed to not have any required validation * issues. @@ -113,4 +117,27 @@ public function testUserCheckers() $this->assertEquals('app.henk_checker', $processedConfig['firewalls']['stub']['user_checker']); } + + public function testFirewalls() + { + $factory = $this->createMock(AuthenticatorFactoryInterface::class); + $factory->expects($this->once())->method('addConfiguration'); + + $configuration = new MainConfiguration(['stub' => $factory], []); + $configuration->getConfigTreeBuilder(); + } + + /** + * @group legacy + */ + public function testLegacyFirewalls() + { + $factory = $this->createMock(AuthenticatorFactoryInterface::class); + $factory->expects($this->once())->method('addConfiguration'); + + $this->expectDeprecation('Since symfony/security-bundle 5.4: Passing an array of arrays as 1st argument to "Symfony\Bundle\SecurityBundle\DependencyInjection\MainConfiguration::__construct" is deprecated, pass a sorted array of factories instead.'); + + $configuration = new MainConfiguration(['http_basic' => ['stub' => $factory]], []); + $configuration->getConfigTreeBuilder(); + } }