[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.
2013-06-14 20:11:03 +02:00 · 2013-06-14 20:11:03 +02:00 · 2317443708
parent 5fde0b00c6
commit 2317443708
7 changed files with 466 additions and 6 deletions
--- a/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php
+++ b/src/Symfony/Component/Security/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);

--- a/src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php
+++ b/src/Symfony/Component/Security/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) {
--- a/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php
+++ b/src/Symfony/Component/Security/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()));
--- a/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php
+++ b/src/Symfony/Component/Security/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);
--- a/src/Symfony/Component/Security/Tests/Http/Firewall/AbstractPreAuthenticatedListenerTest.php
+++ b/src/Symfony/Component/Security/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);
+    }
+}
--- a/src/Symfony/Component/Security/Tests/Http/Firewall/BasicAuthenticationListenerTest.php
+++ b/src/Symfony/Component/Security/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);
+    }
 }
--- a/src/Symfony/Component/Security/Tests/Http/Firewall/X509AuthenticationListenerTest.php
+++ b/src/Symfony/Component/Security/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);
+    }
+}