Allow exception bubbling in RememberMeListener

lstrojny · lstrojny · commit fcb7f74af76c · 2014-07-24T16:03:29.000+02:00
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php
@@ -102,6 +102,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
         $listenerId = 'security.authentication.listener.rememberme.'.$id;
         $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.rememberme'));
         $listener->replaceArgument(1, new Reference($rememberMeServicesId));
+        $listener->replaceArgument(4, $config['catch_exceptions']);
 
         return array($authProviderId, $listenerId, $defaultEntryPoint);
     }
@@ -130,6 +131,7 @@ public function addConfiguration(NodeDefinition $node)
                 ->end()
                 ->prototype('scalar')->end()
             ->end()
+            ->scalarNode('catch_exceptions')->defaultTrue()->end()
         ;
 
         foreach ($this->options as $name => $value) {
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_rememberme.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_rememberme.xml
@@ -24,13 +24,14 @@
             <argument type="service" id="security.authentication.manager" />
             <argument type="service" id="logger" on-invalid="null" />
             <argument type="service" id="event_dispatcher" on-invalid="null"/>
+            <argument /> <!-- Catch exception flag set in RememberMeFactory -->
         </service>
 
         <service id="security.authentication.provider.rememberme" class="%security.authentication.provider.rememberme.class%" abstract="true" public="false">
             <argument type="service" id="security.user_checker" />
         </service>
 
-        <service id="security.rememberme.token.provider.in_memory" class="%security.rememberme.token.provider.in_memory.class%" public="false"></service>
+        <service id="security.rememberme.token.provider.in_memory" class="%security.rememberme.token.provider.in_memory.class%" public="false"/>
 
         <service id="security.authentication.rememberme.services.abstract" abstract="true" public="false">
             <tag name="monolog.logger" channel="security" />
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php
@@ -82,6 +82,7 @@ public function testFirewalls()
                 'security.authentication.listener.form.secure',
                 'security.authentication.listener.basic.secure',
                 'security.authentication.listener.digest.secure',
+                'security.authentication.listener.rememberme.secure',
                 'security.authentication.listener.anonymous.secure',
                 'security.access_listener',
                 'security.authentication.switchuser_listener.secure',
@@ -219,6 +220,20 @@ public function testCustomAclProvider()
         $this->assertEquals('foo', (string) $container->getAlias('security.acl.provider'));
     }
 
+    public function testRememberMeThrowExceptionsDefault()
+    {
+        $container = $this->getContainer('container1');
+        $this->assertTrue($container->getDefinition('security.authentication.listener.rememberme.secure')->getArgument(4));
+    }
+
+    public function testRememberMeThrowExceptions()
+    {
+        $container = $this->getContainer('remember_me_options');
+        $service = $container->getDefinition('security.authentication.listener.rememberme.main');
+        $this->assertEquals('security.authentication.rememberme.services.persistent.main', $service->getArgument(1));
+        $this->assertFalse($service->getArgument(4));
+    }
+
     protected function getContainer($file)
     {
         $container = new ContainerBuilder();
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php
@@ -70,6 +70,7 @@
             'switch_user' => true,
             'x509' => true,
             'logout' => true,
+            'remember_me' => array('key' => 'TheKey')
         ),
         'host' => array(
             'pattern' => '/test',
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php
@@ -0,0 +1,17 @@
+<?php
+$container->loadFromExtension('security', array(
+    'providers' => array(
+        'default' => array('id' => 'foo'),
+    ),
+
+    'firewalls' => array(
+        'main' => array(
+            'form_login' => true,
+            'remember_me' => array(
+                'key' => 'TheyKey',
+                'catch_exceptions' => false,
+                'token_provider' => 'token_provider_id',
+            )
+        )
+    ),
+));
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml
@@ -55,6 +55,7 @@
             <switch-user />
             <x509 />
             <logout />
+            <remember-me key="TheyKey"/>
         </firewall>
 
         <firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST">
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sec="http://symfony.com/schema/dic/security"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <sec:config>
+        <sec:providers>
+            <sec:default id="foo"/>
+        </sec:providers>
+        <sec:firewall name="main">
+            <sec:form-login/>
+            <sec:remember-me key="TheKey" catch-exceptions="false" token-provider="token_provider_id" />
+        </sec:firewall>
+    </sec:config>
+
+</container>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml
@@ -53,6 +53,8 @@ security:
             switch_user: true
             x509: true
             logout: true
+            remember_me:
+                key: TheKey
         host:
             pattern: /test
             host: foo\.example\.org
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml
@@ -0,0 +1,12 @@
+security:
+    providers:
+        default:
+            id: foo
+
+    firewalls:
+        main:
+            form_login: true
+            remember_me:
+                key: TheKey
+                catch_exceptions: false
+                token_provider: token_provider_id
diff --git a/src/Symfony/Component/Security/Http/Firewall/RememberMeListener.php b/src/Symfony/Component/Security/Http/Firewall/RememberMeListener.php
@@ -33,6 +33,7 @@ class RememberMeListener implements ListenerInterface
     private $authenticationManager;
     private $logger;
     private $dispatcher;
+    private $catchExceptions = true;
 
     /**
      * Constructor.
@@ -42,14 +43,16 @@ class RememberMeListener implements ListenerInterface
      * @param AuthenticationManagerInterface $authenticationManager
      * @param LoggerInterface                $logger
      * @param EventDispatcherInterface       $dispatcher
+     * @param bool                           $catchExceptions
      */
-    public function __construct(SecurityContextInterface $securityContext, RememberMeServicesInterface $rememberMeServices, AuthenticationManagerInterface $authenticationManager, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)
+    public function __construct(SecurityContextInterface $securityContext, RememberMeServicesInterface $rememberMeServices, AuthenticationManagerInterface $authenticationManager, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, $catchExceptions = true)
     {
         $this->securityContext = $securityContext;
         $this->rememberMeServices = $rememberMeServices;
         $this->authenticationManager = $authenticationManager;
         $this->logger = $logger;
         $this->dispatcher = $dispatcher;
+        $this->catchExceptions = $catchExceptions;
     }
 
     /**
@@ -90,6 +93,10 @@ public function handle(GetResponseEvent $event)
             }
 
             $this->rememberMeServices->loginFail($request);
+
+            if (!$this->catchExceptions) {
+                throw $failed;
+            }
         }
     }
 }
diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/RememberMeListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/RememberMeListenerTest.php
@@ -14,12 +14,13 @@
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Http\Firewall\RememberMeListener;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Http\SecurityEvents;
 
 class RememberMeListenerTest extends \PHPUnit_Framework_TestCase
 {
     public function testOnCoreSecurityDoesNotTryToPopulateNonEmptySecurityContext()
     {
-        list($listener, $context, $service,,) = $this->getListener();
+        list($listener, $context,,,,) = $this->getListener();
 
         $context
             ->expects($this->once())
@@ -99,6 +100,48 @@ public function testOnCoreSecurityIgnoresAuthenticationExceptionThrownByAuthenti
         $listener->handle($event);
     }
 
+    /**
+     * @expectedException Symfony\Component\Security\Core\Exception\AuthenticationException
+     * @expectedExceptionMessage Authentication failed.
+     */
+    public function testOnCoreSecurityIgnoresAuthenticationOptionallyRethrowsExceptionThrownAuthenticationManagerImplementation()
+    {
+        list($listener, $context, $service, $manager,) = $this->getListener(false, false);
+
+        $context
+            ->expects($this->once())
+            ->method('getToken')
+            ->will($this->returnValue(null))
+        ;
+
+        $service
+            ->expects($this->once())
+            ->method('autoLogin')
+            ->will($this->returnValue($this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')))
+        ;
+
+        $service
+            ->expects($this->once())
+            ->method('loginFail')
+        ;
+
+        $exception = new AuthenticationException('Authentication failed.');
+        $manager
+            ->expects($this->once())
+            ->method('authenticate')
+            ->will($this->throwException($exception))
+        ;
+
+        $event = $this->getGetResponseEvent();
+        $event
+            ->expects($this->once())
+            ->method('getRequest')
+            ->will($this->returnValue(new Request()))
+        ;
+
+        $listener->handle($event);
+    }
+
     public function testOnCoreSecurity()
     {
         list($listener, $context, $service, $manager,) = $this->getListener();
@@ -138,6 +181,55 @@ public function testOnCoreSecurity()
         $listener->handle($event);
     }
 
+    public function testOnCoreSecurityInteractiveLoginEventIsDispatchedIfDispatcherIsPresent()
+    {
+        list($listener, $context, $service, $manager,, $dispatcher) = $this->getListener(true);
+
+        $context
+            ->expects($this->once())
+            ->method('getToken')
+            ->will($this->returnValue(null))
+        ;
+
+        $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
+        $service
+            ->expects($this->once())
+            ->method('autoLogin')
+            ->will($this->returnValue($token))
+        ;
+
+        $context
+            ->expects($this->once())
+            ->method('setToken')
+            ->with($this->equalTo($token))
+        ;
+
+        $manager
+            ->expects($this->once())
+            ->method('authenticate')
+            ->will($this->returnValue($token))
+        ;
+
+        $event = $this->getGetResponseEvent();
+        $request = new Request();
+        $event
+            ->expects($this->once())
+            ->method('getRequest')
+            ->will($this->returnValue($request))
+        ;
+
+        $dispatcher
+            ->expects($this->once())
+            ->method('dispatch')
+            ->with(
+                SecurityEvents::INTERACTIVE_LOGIN,
+                $this->isInstanceOf('Symfony\Component\Security\Http\Event\InteractiveLoginEvent')
+            )
+        ;
+
+        $listener->handle($event);
+    }
+
     protected function getGetResponseEvent()
     {
         return $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
@@ -148,16 +240,18 @@ protected function getFilterResponseEvent()
         return $this->getMock('Symfony\Component\HttpKernel\Event\FilterResponseEvent', array(), array(), '', false);
     }
 
-    protected function getListener()
+    protected function getListener($withDispatcher = false, $catchExceptions = true)
     {
         $listener = new RememberMeListener(
             $context = $this->getContext(),
             $service = $this->getService(),
             $manager = $this->getManager(),
-            $logger = $this->getLogger()
+            $logger = $this->getLogger(),
+            $dispatcher = ($withDispatcher ? $this->getDispatcher() : null),
+            $catchExceptions
         );
 
-        return array($listener, $context, $service, $manager, $logger);
+        return array($listener, $context, $service, $manager, $logger, $dispatcher);
     }
 
     protected function getLogger()
@@ -177,8 +271,11 @@ protected function getService()
 
     protected function getContext()
     {
-        return $this->getMockBuilder('Symfony\Component\Security\Core\SecurityContext')
-                    ->disableOriginalConstructor()
-                    ->getMock();
+        return $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+    }
+
+    protected function getDispatcher()
+    {
+        return $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -102,6 +102,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,`
`102`	`102`	`$listenerId = 'security.authentication.listener.rememberme.'.$id;`
`103`	`103`	`$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.rememberme'));`
`104`	`104`	`$listener->replaceArgument(1, new Reference($rememberMeServicesId));`
	`105`	`+ $listener->replaceArgument(4, $config['catch_exceptions']);`
`105`	`106`
`106`	`107`	`return array($authProviderId, $listenerId, $defaultEntryPoint);`
`107`	`108`	`}`
`@@ -130,6 +131,7 @@ public function addConfiguration(NodeDefinition $node)`
`130`	`131`	`->end()`
`131`	`132`	`->prototype('scalar')->end()`
`132`	`133`	`->end()`
	`134`	`+ ->scalarNode('catch_exceptions')->defaultTrue()->end()`
`133`	`135`	`;`
`134`	`136`
`135`	`137`	`foreach ($this->options as $name => $value) {`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@ class RememberMeListener implements ListenerInterface`
`33`	`33`	`private $authenticationManager;`
`34`	`34`	`private $logger;`
`35`	`35`	`private $dispatcher;`
	`36`	`+ private $catchExceptions = true;`
`36`	`37`
`37`	`38`	`/**`
`38`	`39`	`* Constructor.`
`@@ -42,14 +43,16 @@ class RememberMeListener implements ListenerInterface`
`42`	`43`	`* @param AuthenticationManagerInterface $authenticationManager`
`43`	`44`	`* @param LoggerInterface $logger`
`44`	`45`	`* @param EventDispatcherInterface $dispatcher`
	`46`	`+ * @param bool $catchExceptions`
`45`	`47`	`*/`
`46`		`- public function __construct(SecurityContextInterface $securityContext, RememberMeServicesInterface $rememberMeServices, AuthenticationManagerInterface $authenticationManager, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)`
	`48`	`+ public function __construct(SecurityContextInterface $securityContext, RememberMeServicesInterface $rememberMeServices, AuthenticationManagerInterface $authenticationManager, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, $catchExceptions = true)`
`47`	`49`	`{`
`48`	`50`	`$this->securityContext = $securityContext;`
`49`	`51`	`$this->rememberMeServices = $rememberMeServices;`
`50`	`52`	`$this->authenticationManager = $authenticationManager;`
`51`	`53`	`$this->logger = $logger;`
`52`	`54`	`$this->dispatcher = $dispatcher;`
	`55`	`+ $this->catchExceptions = $catchExceptions;`
`53`	`56`	`}`
`54`	`57`
`55`	`58`	`/**`
`@@ -90,6 +93,10 @@ public function handle(GetResponseEvent $event)`
`90`	`93`	`}`
`91`	`94`
`92`	`95`	`$this->rememberMeServices->loginFail($request);`
	`96`	`+`
	`97`	`+ if (!$this->catchExceptions) {`
	`98`	`+ throw $failed;`
	`99`	`+ }`
`93`	`100`	`}`
`94`	`101`	`}`
`95`	`102`	`}`