bug #27581 Fix bad method call with guard authentication + session migration (weaverryan)

This PR was squashed before being merged into the 2.8 branch (closes #27581).

Discussion
----------

Fix bad method call with guard authentication + session migration

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no (but there needs to be on master)
| Tests pass?   | yes
| Fixed tickets | #27577
| License       | MIT
| Doc PR        | n/a

I messed up #27452 :/. Guard is the one class where the session migration is not on the listener, it's on the handler. The tricky part is that there is only ONE handler (unlike listeners where there is 1 listener per firewall). That means that implementing a session migration strategy that avoids stateless firewalls was a bit more tricky: I could only think to inject a map into `GuardAuthenticationHandler`. On the bright side, this also fixes session migration (not happening) when people call the `authenticateUserAndHandleSuccess()` method directly.

On master, we'll need to add a deprecation to make the 3rd argument of `authenticateWithToken()` required - it's optional now for BC. We may also need to re-order the constructor args.

I DID test this in a real 2.8 project, to make sure that things were properly wired up. Apologies for not doing that for the other PR.

Cheers!

Commits
-------

2c0ac93 Fix bad method call with guard authentication + session migration

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php

@@ -77,7 +77,6 @@ class GuardAuthenticationFactory implements SecurityFactoryInterface
         $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.guard'));
         $listener->replaceArgument(2, $id);
         $listener->replaceArgument(3, $authenticatorReferences);
-        $listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));
 
         // determine the entryPointId to use
         $entryPointId = $this->determineEntryPoint($defaultEntryPoint, $config);

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

@@ -39,6 +39,7 @@ class SecurityExtension extends Extension
     private $factories = array();
     private $userProviderFactories = array();
     private $expressionLanguage;
+    private $statelessFirewallKeys = array();
 
     public function __construct()
     {
@@ -89,6 +90,9 @@ class SecurityExtension extends Extension
         $this->createAuthorization($config, $container);
         $this->createRoleHierarchy($config, $container);
 
+        $container->getDefinition('security.authentication.guard_handler')
+            ->replaceArgument(2, $this->statelessFirewallKeys);
+
         if ($config['encoders']) {
             $this->createEncoders($config['encoders'], $container);
         }
@@ -287,6 +291,7 @@ class SecurityExtension extends Extension
             $listeners[] = new Reference($this->createContextListener($container, $contextKey));
             $sessionStrategyId = 'security.authentication.session_strategy';
         } else {
+            $this->statelessFirewallKeys[] = $id;
             $sessionStrategyId = 'security.authentication.session_strategy_noop';
         }
         $container->setAlias(new Alias('security.authentication.session_strategy.'.$id, false), $sessionStrategyId);

src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml

@@ -10,6 +10,10 @@
             >
             <argument type="service" id="security.token_storage" />
             <argument type="service" id="event_dispatcher" on-invalid="null" />
+            <argument /> <!-- stateless firewall keys -->
+            <call method="setSessionAuthenticationStrategy">
+                <argument type="service" id="security.authentication.session_strategy" />
+            </call>
         </service>
 
         <!-- See GuardAuthenticationFactory -->

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php

@@ -119,6 +119,33 @@ class SecurityExtensionTest extends TestCase
         $this->assertFalse($container->hasDefinition('security.access.role_hierarchy_voter'));
     }
 
+    public function testGuardHandlerIsPassedStatelessFirewalls()
+    {
+        $container = $this->getRawContainer();
+
+        $container->loadFromExtension('security', array(
+            'providers' => array(
+                'default' => array('id' => 'foo'),
+            ),
+
+            'firewalls' => array(
+                'some_firewall' => array(
+                    'pattern' => '^/admin',
+                    'http_basic' => null,
+                ),
+                'stateless_firewall' => array(
+                    'pattern' => '/.*',
+                    'stateless' => true,
+                    'http_basic' => null,
+                ),
+            ),
+        ));
+
+        $container->compile();
+        $definition = $container->getDefinition('security.authentication.guard_handler');
+        $this->assertSame(array('stateless_firewall'), $definition->getArgument(2));
+    }
+
     protected function getRawContainer()
     {
         $container = new ContainerBuilder();

src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php

@@ -117,7 +117,7 @@ class GuardAuthenticationListener implements ListenerInterface
             }
 
             // sets the token on the token storage, etc
-            $this->guardHandler->authenticateWithToken($token, $request);
+            $this->guardHandler->authenticateWithToken($token, $request, $this->providerKey);
         } catch (AuthenticationException $e) {
             // oh no! Authentication failed!
 

src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php

@@ -35,19 +35,28 @@ class GuardAuthenticatorHandler
     private $tokenStorage;
     private $dispatcher;
     private $sessionStrategy;
+    private $statelessProviderKeys;
 
-    public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null)
+    /**
+     * @param array $statelessProviderKeys An array of provider/firewall keys that are "stateless" and so do not need the session migrated on success
+     */
+    public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null, array $statelessProviderKeys = array())
     {
         $this->tokenStorage = $tokenStorage;
         $this->dispatcher = $eventDispatcher;
+        $this->statelessProviderKeys = $statelessProviderKeys;
     }
 
     /**
      * Authenticates the given token in the system.
+     *
+     * @param string $providerKey The name of the provider/firewall being used for authentication
      */
-    public function authenticateWithToken(TokenInterface $token, Request $request)
+    public function authenticateWithToken(TokenInterface $token, Request $request/*, string $providerKey */)
     {
-        $this->migrateSession($request, $token);
+        $providerKey = \func_num_args() > 2 ? func_get_arg(2) : null;
+
+        $this->migrateSession($request, $token, $providerKey);
         $this->tokenStorage->setToken($token);
 
         if (null !== $this->dispatcher) {
@@ -98,7 +107,7 @@ class GuardAuthenticatorHandler
         // create an authenticated token for the User
         $token = $authenticator->createAuthenticatedToken($user, $providerKey);
         // authenticate this in the system
-        $this->authenticateWithToken($token, $request);
+        $this->authenticateWithToken($token, $request, $providerKey);
 
         // return the success metric
         return $this->handleAuthenticationSuccess($token, $request, $authenticator, $providerKey);
@@ -140,9 +149,9 @@ class GuardAuthenticatorHandler
         $this->sessionStrategy = $sessionStrategy;
     }
 
-    private function migrateSession(Request $request, TokenInterface $token)
+    private function migrateSession(Request $request, TokenInterface $token, $providerKey)
     {
-        if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
+        if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession() || \in_array($providerKey, $this->statelessProviderKeys, true)) {
             return;
         }
 

src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php

@@ -143,6 +143,18 @@ class GuardAuthenticatorHandlerTest extends TestCase
         $handler->authenticateWithToken($this->token, $this->request);
     }
 
+    public function testSessionStrategyIsNotCalledWhenStateless()
+    {
+        $this->configurePreviousSession();
+
+        $this->sessionStrategy->expects($this->never())
+            ->method('onAuthentication');
+
+        $handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher, array('some_provider_key'));
+        $handler->setSessionAuthenticationStrategy($this->sessionStrategy);
+        $handler->authenticateWithToken($this->token, $this->request, 'some_provider_key');
+    }
+
     protected function setUp()
     {
         $this->tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();