feature #21450 [Security] Lazy load guard authenticators and authenti…

…cation providers (chalasr) This PR was squashed before being merged into the 3.3-dev branch (closes #21450). Discussion ---------- [Security] Lazy load guard authenticators and authentication providers | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Authentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed. Commits ------- cd6422a [SecurityBundle] Lazy load authentication providers b8a23de [Security][Guard] Lazy load authenticators
symfony · Feb 16, 2017 · b056d40 · b056d40
2 parents 7c9a5c1 + cd6422a
commit b056d40
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 29 deletions.
diff --git a/...Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php b/...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\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
@@ -62,11 +63,13 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
             $authenticatorReferences[] = new Reference($authenticatorId);
         }
 
+        $authenticators = new IteratorArgument($authenticatorReferences);
+
         // configure the GuardAuthenticationFactory to have the dynamic constructor arguments
         $providerId = 'security.authentication.provider.guard.'.$id;
         $container
             ->setDefinition($providerId, new ChildDefinition('security.authentication.provider.guard'))
-            ->replaceArgument(0, $authenticatorReferences)
+            ->replaceArgument(0, $authenticators)
             ->replaceArgument(1, new Reference($userProvider))
             ->replaceArgument(2, $id)
             ->replaceArgument(3, new Reference('security.user_checker.'.$id))
@@ -76,7 +79,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
         $listenerId = 'security.authentication.listener.guard.'.$id;
         $listener = $container->setDefinition($listenerId, new ChildDefinition('security.authentication.listener.guard'));
         $listener->replaceArgument(2, $id);
-        $listener->replaceArgument(3, $authenticatorReferences);
+        $listener->replaceArgument(3, $authenticators);
 
         // determine the entryPointId to use
         $entryPointId = $this->determineEntryPoint($defaultEntryPoint, $config);

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -266,7 +266,7 @@ private function createFirewalls($config, ContainerBuilder $container)
         }, array_values(array_unique($authenticationProviders)));
         $container
             ->getDefinition('security.authentication.manager')
-            ->replaceArgument(0, $authenticationProviders)
+            ->replaceArgument(0, new IteratorArgument($authenticationProviders))
         ;
     }
 

diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
@@ -27,7 +27,7 @@
 
         <!-- Authentication related services -->
         <service id="security.authentication.manager" class="Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager" public="false">
-            <argument type="collection" />
+            <argument /> <!-- providers -->
             <argument>%security.authentication.manager.erase_credentials%</argument>
             <call method="setEventDispatcher">
                 <argument type="service" id="event_dispatcher" />

diff --git a/...urityBundle/Tests/DependencyInjection/Security/Factory/GuardAuthenticationFactoryTest.php b/...urityBundle/Tests/DependencyInjection/Security/Factory/GuardAuthenticationFactoryTest.php
@@ -13,6 +13,7 @@
 
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\GuardAuthenticationFactory;
 use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
+use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
@@ -106,15 +107,15 @@ public function testBasicCreate()
 
         $providerDefinition = $container->getDefinition('security.authentication.provider.guard.my_firewall');
         $this->assertEquals(array(
-            'index_0' => array(new Reference('authenticator123')),
+            'index_0' => new IteratorArgument(array(new Reference('authenticator123'))),
             'index_1' => new Reference('my_user_provider'),
             'index_2' => 'my_firewall',
             'index_3' => new Reference('security.user_checker.my_firewall'),
         ), $providerDefinition->getArguments());
 
         $listenerDefinition = $container->getDefinition('security.authentication.listener.guard.my_firewall');
         $this->assertEquals('my_firewall', $listenerDefinition->getArgument(2));
-        $this->assertEquals(array(new Reference('authenticator123')), $listenerDefinition->getArgument(3));
+        $this->assertEquals(array(new Reference('authenticator123')), $listenerDefinition->getArgument(3)->getValues());
     }
 
     public function testExistingDefaultEntryPointUsed()

diff --git a/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php b/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php
@@ -37,23 +37,17 @@ class AuthenticationProviderManager implements AuthenticationManagerInterface
     /**
      * Constructor.
      *
-     * @param AuthenticationProviderInterface[] $providers        An array of AuthenticationProviderInterface instances
-     * @param bool                              $eraseCredentials Whether to erase credentials after authentication or not
+     * @param iterable|AuthenticationProviderInterface[] $providers        An iterable with AuthenticationProviderInterface instances as values
+     * @param bool                                       $eraseCredentials Whether to erase credentials after authentication or not
      *
      * @throws \InvalidArgumentException
      */
-    public function __construct(array $providers, $eraseCredentials = true)
+    public function __construct($providers, $eraseCredentials = true)
     {
         if (!$providers) {
             throw new \InvalidArgumentException('You must at least add one authentication provider.');
         }
 
-        foreach ($providers as $provider) {
-            if (!$provider instanceof AuthenticationProviderInterface) {
-                throw new \InvalidArgumentException(sprintf('Provider "%s" must implement the AuthenticationProviderInterface.', get_class($provider)));
-            }
-        }
-
         $this->providers = $providers;
         $this->eraseCredentials = (bool) $eraseCredentials;
     }
@@ -72,6 +66,10 @@ public function authenticate(TokenInterface $token)
         $result = null;
 
         foreach ($this->providers as $provider) {
+            if (!$provider instanceof AuthenticationProviderInterface) {
+                throw new \InvalidArgumentException(sprintf('Provider "%s" must implement the AuthenticationProviderInterface.', get_class($provider)));
+            }
+
             if (!$provider->supports($token)) {
                 continue;
             }

diff --git a/...ymfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php b/...ymfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php
@@ -15,6 +15,7 @@
 use Symfony\Component\Security\Core\Exception\ProviderNotFoundException;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\Exception\AccountStatusException;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 
 class AuthenticationProviderManagerTest extends \PHPUnit_Framework_TestCase
@@ -32,9 +33,9 @@ public function testAuthenticateWithoutProviders()
      */
     public function testAuthenticateWithProvidersWithIncorrectInterface()
     {
-        new AuthenticationProviderManager(array(
+        (new AuthenticationProviderManager(array(
             new \stdClass(),
-        ));
+        )))->authenticate($this->getMockBuilder(TokenInterface::class)->getMock());
     }
 
     public function testAuthenticateWhenNoProviderSupportsToken()

diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php
@@ -39,13 +39,13 @@ class GuardAuthenticationListener implements ListenerInterface
     private $rememberMeServices;
 
     /**
-     * @param GuardAuthenticatorHandler      $guardHandler          The Guard handler
-     * @param AuthenticationManagerInterface $authenticationManager An AuthenticationManagerInterface instance
-     * @param string                         $providerKey           The provider (i.e. firewall) key
-     * @param GuardAuthenticatorInterface[]  $guardAuthenticators   The authenticators, with keys that match what's passed to GuardAuthenticationProvider
-     * @param LoggerInterface                $logger                A LoggerInterface instance
+     * @param GuardAuthenticatorHandler              $guardHandler          The Guard handler
+     * @param AuthenticationManagerInterface         $authenticationManager An AuthenticationManagerInterface instance
+     * @param string                                 $providerKey           The provider (i.e. firewall) key
+     * @param iterable|GuardAuthenticatorInterface[] $guardAuthenticators   The authenticators, with keys that match what's passed to GuardAuthenticationProvider
+     * @param LoggerInterface                        $logger                A LoggerInterface instance
      */
-    public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, array $guardAuthenticators, LoggerInterface $logger = null)
+    public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null)
     {
         if (empty($providerKey)) {
             throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -66,7 +66,13 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
     public function handle(GetResponseEvent $event)
     {
         if (null !== $this->logger) {
-            $this->logger->debug('Checking for guard authentication credentials.', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
+            $context = array('firewall_key' => $this->providerKey);
+
+            if ($this->guardAuthenticators instanceof \Countable || is_array($this->guardAuthenticators)) {
+                $context['authenticators'] = count($this->guardAuthenticators);
+            }
+
+            $this->logger->debug('Checking for guard authentication credentials.', $context);
         }
 
         foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {

diff --git a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php
@@ -40,12 +40,12 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
     private $userChecker;
 
     /**
-     * @param GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationListener
-     * @param UserProviderInterface         $userProvider        The user provider
-     * @param string                        $providerKey         The provider (i.e. firewall) key
-     * @param UserCheckerInterface          $userChecker
+     * @param iterable|GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationListener
+     * @param UserProviderInterface                  $userProvider        The user provider
+     * @param string                                 $providerKey         The provider (i.e. firewall) key
+     * @param UserCheckerInterface                   $userChecker
      */
-    public function __construct(array $guardAuthenticators, UserProviderInterface $userProvider, $providerKey, UserCheckerInterface $userChecker)
+    public function __construct($guardAuthenticators, UserProviderInterface $userProvider, $providerKey, UserCheckerInterface $userChecker)
     {
         $this->guardAuthenticators = $guardAuthenticators;
         $this->userProvider = $userProvider;