[Security] fixed issue where authentication listeners clear unrelated tokens

This commit fixes an issue where authentication listeners clear all security tokens in case of authentication failure.
This behavior makes it impossible to combine certain authentication mechanisms, notably x509 with form-based login.

7 files changed, 466 insertions, 6 deletions

diff --git a/Http/Firewall/AbstractAuthenticationListener.php b/Http/Firewall/AbstractAuthenticationListener.php
index d33c0f7..384f7ce 100644
--- a/Http/Firewall/AbstractAuthenticationListener.php
+++ b/Http/Firewall/AbstractAuthenticationListener.php
@@ -186,7 +186,10 @@ abstract class AbstractAuthenticationListener implements ListenerInterface
             $this->logger->info(sprintf('Authentication request failed: %s', $failed->getMessage()));
         }
 
-        $this->securityContext->setToken(null);
+        $token = $this->securityContext->getToken();
+        if ($token instanceof UsernamePasswordToken && $this->providerKey === $token->getProviderKey()) {
+            $this->securityContext->setToken(null);
+        }
 
         $response = $this->failureHandler->onAuthenticationFailure($request, $failed);
 
diff --git a/Http/Firewall/AbstractPreAuthenticatedListener.php b/Http/Firewall/AbstractPreAuthenticatedListener.php
index c6e47d0..28f6411 100644
--- a/Http/Firewall/AbstractPreAuthenticatedListener.php
+++ b/Http/Firewall/AbstractPreAuthenticatedListener.php
@@ -21,6 +21,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 
 /**
  * AbstractPreAuthenticatedListener is the base class for all listener that
@@ -59,7 +60,12 @@ abstract class AbstractPreAuthenticatedListener implements ListenerInterface
             $this->logger->debug(sprintf('Checking secure context token: %s', $this->securityContext->getToken()));
         }
 
-        list($user, $credentials) = $this->getPreAuthenticatedData($request);
+        try {
+            list($user, $credentials) = $this->getPreAuthenticatedData($request);
+        } catch (BadCredentialsException $exception) {
+            $this->clearToken();
+            return;
+        }
 
         if (null !== $token = $this->securityContext->getToken()) {
             if ($token instanceof PreAuthenticatedToken && $this->providerKey == $token->getProviderKey() && $token->isAuthenticated() && $token->getUsername() === $user) {
@@ -84,6 +90,17 @@ abstract class AbstractPreAuthenticatedListener implements ListenerInterface
                 $this->dispatcher->dispatch(SecurityEvents::INTERACTIVE_LOGIN, $loginEvent);
             }
         } catch (AuthenticationException $failed) {
+            $this->clearToken();
+        }
+    }
+
+    /**
+     * Clears a PreAuthenticatedToken for this provider (if present)
+     */
+    protected function clearToken()
+    {
+        $token = $this->securityContext->getToken();
+        if ($token instanceof PreAuthenticatedToken && $this->providerKey === $token->getProviderKey()) {
             $this->securityContext->setToken(null);
 
             if (null !== $this->logger) {
diff --git a/Http/Firewall/BasicAuthenticationListener.php b/Http/Firewall/BasicAuthenticationListener.php
index 5b1c8b3..bfc4abc 100644
--- a/Http/Firewall/BasicAuthenticationListener.php
+++ b/Http/Firewall/BasicAuthenticationListener.php
@@ -74,7 +74,10 @@ class BasicAuthenticationListener implements ListenerInterface
             $token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));
             $this->securityContext->setToken($token);
         } catch (AuthenticationException $failed) {
-            $this->securityContext->setToken(null);
+            $token = $this->securityContext->getToken();
+            if ($token instanceof UsernamePasswordToken && $this->providerKey === $token->getProviderKey()) {
+                $this->securityContext->setToken(null);
+            }
 
             if (null !== $this->logger) {
                 $this->logger->info(sprintf('Authentication request failed for user "%s": %s', $username, $failed->getMessage()));
diff --git a/Http/Firewall/DigestAuthenticationListener.php b/Http/Firewall/DigestAuthenticationListener.php
index 7ab3dcf..ea85e77 100644
--- a/Http/Firewall/DigestAuthenticationListener.php
+++ b/Http/Firewall/DigestAuthenticationListener.php
@@ -124,7 +124,10 @@ class DigestAuthenticationListener implements ListenerInterface
 
     private function fail(GetResponseEvent $event, Request $request, AuthenticationException $authException)
     {
-        $this->securityContext->setToken(null);
+        $token = $this->securityContext->getToken();
+        if ($token instanceof UsernamePasswordToken && $this->providerKey === $token->getProviderKey()) {
+            $this->securityContext->setToken(null);
+        }
 
         if (null !== $this->logger) {
             $this->logger->info($authException);
diff --git a/Tests/Http/Firewall/AbstractPreAuthenticatedListenerTest.php b/Tests/Http/Firewall/AbstractPreAuthenticatedListenerTest.php
new file mode 100644
index 0000000..76721ec
--- /dev/null
+++ b/Tests/Http/Firewall/AbstractPreAuthenticatedListenerTest.php
@@ -0,0 +1,267 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Tests\Http\Firewall;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
+
+class AbstractPreAuthenticatedListenerTest extends \PHPUnit_Framework_TestCase
+{
+    protected function setUp()
+    {
+        if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) {
+            $this->markTestSkipped('The "EventDispatcher" component is not available');
+        }
+
+        if (!class_exists('Symfony\Component\HttpFoundation\Request')) {
+            $this->markTestSkipped('The "HttpFoundation" component is not available');
+        }
+
+        if (!class_exists('Symfony\Component\HttpKernel\HttpKernel')) {
+            $this->markTestSkipped('The "HttpKernel" component is not available');
+        }
+    }
+
+    public function testHandleWithValidValues()
+    {
+        $userCredentials = array('TheUser', 'TheCredentials');
+
+        $request = new Request(array(), array(), array(), array(), array(), array());
+
+        $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context
+            ->expects($this->any())
+            ->method('getToken')
+            ->will($this->returnValue(null))
+        ;
+        $context
+            ->expects($this->once())
+            ->method('setToken')
+            ->with($this->equalTo($token))
+        ;
+
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+        $authenticationManager
+            ->expects($this->once())
+            ->method('authenticate')
+            ->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
+            ->will($this->returnValue($token))
+        ;
+
+        $listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        ));
+        $listener
+            ->expects($this->once())
+            ->method('getPreAuthenticatedData')
+            ->will($this->returnValue($userCredentials));
+
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $event
+            ->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+
+        $listener->handle($event);
+    }
+
+    public function testHandleWhenAuthenticationFails()
+    {
+        $userCredentials = array('TheUser', 'TheCredentials');
+
+        $request = new Request(array(), array(), array(), array(), array(), array());
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context
+            ->expects($this->any())
+            ->method('getToken')
+            ->will($this->returnValue(null))
+        ;
+        $context
+            ->expects($this->never())
+            ->method('setToken')
+        ;
+
+        $exception = new AuthenticationException('Authentication failed.');
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+        $authenticationManager
+            ->expects($this->once())
+            ->method('authenticate')
+            ->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
+            ->will($this->throwException($exception))
+        ;
+
+        $listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        ));
+        $listener
+            ->expects($this->once())
+            ->method('getPreAuthenticatedData')
+            ->will($this->returnValue($userCredentials));
+
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $event
+            ->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+
+        $listener->handle($event);
+    }
+
+    public function testHandleWhenAuthenticationFailsWithDifferentToken()
+    {
+        $userCredentials = array('TheUser', 'TheCredentials');
+
+        $token = new UsernamePasswordToken('TheUsername', 'ThePassword', 'TheProviderKey', array('ROLE_FOO'));
+
+        $request = new Request(array(), array(), array(), array(), array(), array());
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context
+            ->expects($this->any())
+            ->method('getToken')
+            ->will($this->returnValue($token))
+        ;
+        $context
+            ->expects($this->never())
+            ->method('setToken')
+        ;
+
+        $exception = new AuthenticationException('Authentication failed.');
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+        $authenticationManager
+            ->expects($this->once())
+            ->method('authenticate')
+            ->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
+            ->will($this->throwException($exception))
+        ;
+
+        $listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        ));
+        $listener
+            ->expects($this->once())
+            ->method('getPreAuthenticatedData')
+            ->will($this->returnValue($userCredentials));
+
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $event
+            ->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+
+        $listener->handle($event);
+    }
+
+    public function testHandleWithASimilarAuthenticatedToken()
+    {
+        $userCredentials = array('TheUser', 'TheCredentials');
+
+        $request = new Request(array(), array(), array(), array(), array(), array());
+
+        $token = new PreAuthenticatedToken('TheUser', 'TheCredentials', 'TheProviderKey', array('ROLE_FOO'));
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context
+            ->expects($this->any())
+            ->method('getToken')
+            ->will($this->returnValue($token))
+        ;
+
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+        $authenticationManager
+            ->expects($this->never())
+            ->method('authenticate')
+        ;
+
+        $listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        ));
+        $listener
+            ->expects($this->once())
+            ->method('getPreAuthenticatedData')
+            ->will($this->returnValue($userCredentials));
+
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $event
+            ->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+
+        $listener->handle($event);
+    }
+
+    public function testHandleWithAnInvalidSimilarToken()
+    {
+        $userCredentials = array('TheUser', 'TheCredentials');
+
+        $request = new Request(array(), array(), array(), array(), array(), array());
+
+        $token = new PreAuthenticatedToken('AnotherUser', 'TheCredentials', 'TheProviderKey', array('ROLE_FOO'));
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context
+            ->expects($this->any())
+            ->method('getToken')
+            ->will($this->returnValue($token))
+        ;
+        $context
+            ->expects($this->once())
+            ->method('setToken')
+            ->with($this->equalTo(null))
+        ;
+
+        $exception = new AuthenticationException('Authentication failed.');
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+        $authenticationManager
+            ->expects($this->once())
+            ->method('authenticate')
+            ->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken'))
+            ->will($this->throwException($exception))
+        ;
+
+        $listener = $this->getMockForAbstractClass('Symfony\Component\Security\Http\Firewall\AbstractPreAuthenticatedListener', array(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        ));
+        $listener
+            ->expects($this->once())
+            ->method('getPreAuthenticatedData')
+            ->will($this->returnValue($userCredentials));
+
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $event
+            ->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+
+        $listener->handle($event);
+    }
+}
diff --git a/Tests/Http/Firewall/BasicAuthenticationListenerTest.php b/Tests/Http/Firewall/BasicAuthenticationListenerTest.php
index 667869f..84564bf 100644
--- a/Tests/Http/Firewall/BasicAuthenticationListenerTest.php
+++ b/Tests/Http/Firewall/BasicAuthenticationListenerTest.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Tests\Http\Firewall;
 
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Http\Firewall\BasicAuthenticationListener;
@@ -96,9 +97,8 @@ class BasicAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
             ->will($this->returnValue(null))
         ;
         $context
-            ->expects($this->once())
+            ->expects($this->never())
             ->method('setToken')
-            ->with($this->equalTo(null))
         ;
 
         $response = new Response();
@@ -195,4 +195,56 @@ class BasicAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
 
         $listener->handle($event);
     }
+
+    public function testHandleWithADifferentAuthenticatedToken()
+    {
+        $request = new Request(array(), array(), array(), array(), array(), array(
+            'PHP_AUTH_USER' => 'TheUsername',
+            'PHP_AUTH_PW'   => 'ThePassword'
+        ));
+
+        $token = new PreAuthenticatedToken('TheUser', 'TheCredentials', 'TheProviderKey', array('ROLE_FOO'));
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context
+            ->expects($this->any())
+            ->method('getToken')
+            ->will($this->returnValue($token))
+        ;
+        $context
+            ->expects($this->never())
+            ->method('setToken')
+        ;
+
+        $response = new Response();
+
+        $authenticationEntryPoint = $this->getMock('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface');
+        $authenticationEntryPoint
+            ->expects($this->any())
+            ->method('start')
+            ->with($this->equalTo($request), $this->isInstanceOf('Symfony\Component\Security\Core\Exception\AuthenticationException'))
+            ->will($this->returnValue($response))
+        ;
+
+        $listener = new BasicAuthenticationListener(
+            $context,
+            new AuthenticationProviderManager(array($this->getMock('Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface'))),
+            'TheProviderKey',
+            $authenticationEntryPoint
+        );
+
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $event
+            ->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+        $event
+            ->expects($this->once())
+            ->method('setResponse')
+            ->with($this->equalTo($response))
+        ;
+
+        $listener->handle($event);
+    }
 }
diff --git a/Tests/Http/Firewall/X509AuthenticationListenerTest.php b/Tests/Http/Firewall/X509AuthenticationListenerTest.php
new file mode 100644
index 0000000..81ac0f7
--- /dev/null
+++ b/Tests/Http/Firewall/X509AuthenticationListenerTest.php
@@ -0,0 +1,115 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Tests\Http\Firewall;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Http\Firewall\X509AuthenticationListener;
+
+class X509AuthenticationListenerTest extends \PHPUnit_Framework_TestCase
+{
+    protected function setUp()
+    {
+        if (!class_exists('Symfony\Component\HttpFoundation\Request')) {
+            $this->markTestSkipped('The "HttpFoundation" component is not available');
+        }
+    }
+
+    /**
+     * @dataProvider dataProviderGetPreAuthenticatedData
+     */
+    public function testGetPreAuthenticatedData($user, $credentials)
+    {
+        $serverVars = array();
+        if ('' !== $user) {
+            $serverVars['SSL_CLIENT_S_DN_Email'] = $user;
+        }
+        if ('' !== $credentials) {
+            $serverVars['SSL_CLIENT_S_DN'] = $credentials;
+        }
+
+        $request = new Request(array(), array(), array(), array(), array(), $serverVars);
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+
+        $listener = new X509AuthenticationListener(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        );
+
+        $method = new \ReflectionMethod($listener, 'getPreAuthenticatedData');
+        $method->setAccessible(true);
+
+        $result = $method->invokeArgs($listener, array($request));
+        $this->assertSame($result, array($user, $credentials));
+    }
+
+    public static function dataProviderGetPreAuthenticatedData()
+    {
+        return array(
+            'validValues' => array('TheUser', 'TheCredentials'),
+            'noCredentials' => array('TheUser', ''),
+        );
+    }
+
+    /**
+     * @expectedException Symfony\Component\Security\Core\Exception\BadCredentialsException
+     */
+    public function testGetPreAuthenticatedDataNoUser()
+    {
+        $request = new Request(array(), array(), array(), array(), array(), array());
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+
+        $listener = new X509AuthenticationListener(
+            $context,
+            $authenticationManager,
+            'TheProviderKey'
+        );
+
+        $method = new \ReflectionMethod($listener, 'getPreAuthenticatedData');
+        $method->setAccessible(true);
+
+        $result = $method->invokeArgs($listener, array($request));
+    }
+
+    public function testGetPreAuthenticatedDataWithDifferentKeys()
+    {
+        $userCredentials = array('TheUser', 'TheCredentials');
+
+        $request = new Request(array(), array(), array(), array(), array(), array(
+            'TheUserKey' => 'TheUser',
+            'TheCredentialsKey' => 'TheCredentials'
+        ));
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+
+        $authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
+
+        $listener = new X509AuthenticationListener(
+            $context,
+            $authenticationManager,
+            'TheProviderKey',
+            'TheUserKey',
+            'TheCredentialsKey'
+        );
+
+        $method = new \ReflectionMethod($listener, 'getPreAuthenticatedData');
+        $method->setAccessible(true);
+
+        $result = $method->invokeArgs($listener, array($request));
+        $this->assertSame($result, $userCredentials);
+    }
+}