[Security] Refactor LogoutListener constructor to take options

This will facilitate adding additional options for CSRF protection. Additionally, a unit test for existing behavior was added.

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

@@ -276,8 +276,10 @@ class SecurityExtension extends Extension
         if (isset($firewall['logout'])) {
             $listenerId = 'security.logout_listener.'.$id;
             $listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.logout_listener'));
-            $listener->replaceArgument(2, $firewall['logout']['path']);
+            $listener->replaceArgument(2, array(
-            $listener->replaceArgument(3, $firewall['logout']['target']);
+                'logout_path' => $firewall['logout']['path'],
+                'target_url'  => $firewall['logout']['target'],
+            ));
             $listeners[] = new Reference($listenerId);
 
             // add logout success handler

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

@@ -78,8 +78,7 @@
         <service id="security.logout_listener" class="%security.logout_listener.class%" public="false" abstract="true">
             <argument type="service" id="security.context" />
             <argument type="service" id="security.http_utils" />
-            <argument /> <!-- Logout Path -->
+            <argument /> <!-- Options -->
-            <argument /> <!-- Target-URL Path -->
             <argument type="service" id="security.logout.success_handler" on-invalid="null" />
         </service>
         <service id="security.logout.handler.session" class="%security.logout.handler.session.class%" public="false" />

src/Symfony/Component/Security/Http/Firewall/LogoutListener.php

@@ -28,8 +28,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 class LogoutListener implements ListenerInterface
 {
     private $securityContext;
-    private $logoutPath;
+    private $options;
-    private $targetUrl;
     private $handlers;
     private $successHandler;
     private $httpUtils;
@@ -39,16 +38,17 @@ class LogoutListener implements ListenerInterface
      *
      * @param SecurityContextInterface      $securityContext
      * @param HttpUtils                     $httpUtils        An HttpUtilsInterface instance
-     * @param string                        $logoutPath       The path that starts the logout process
+     * @param array                         $options          An array of options for the processing of a logout attempt
-     * @param string                        $targetUrl        The URL to redirect to after logout
      * @param LogoutSuccessHandlerInterface $successHandler
      */
-    public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, $logoutPath, $targetUrl = '/', LogoutSuccessHandlerInterface $successHandler = null)
+    public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, array $options = array(), LogoutSuccessHandlerInterface $successHandler = null)
     {
         $this->securityContext = $securityContext;
         $this->httpUtils = $httpUtils;
-        $this->logoutPath = $logoutPath;
+        $this->options = array_merge(array(
-        $this->targetUrl = $targetUrl;
+            'logout_path' => '/logout',
+            'target_url'  => '/',
+        ), $options);
         $this->successHandler = $successHandler;
         $this->handlers = array();
     }
@@ -83,7 +83,7 @@ class LogoutListener implements ListenerInterface
                 throw new \RuntimeException('Logout Success Handler did not return a Response.');
             }
         } else {
-            $response = $this->httpUtils->createRedirectResponse($request, $this->targetUrl);
+            $response = $this->httpUtils->createRedirectResponse($request, $this->options['target_url']);
         }
 
         // handle multiple logout attempts gracefully
@@ -111,6 +111,6 @@ class LogoutListener implements ListenerInterface
      */
     protected function requiresLogout(Request $request)
     {
-        return $this->httpUtils->checkRequestPath($request, $this->logoutPath);
+        return $this->httpUtils->checkRequestPath($request, $this->options['logout_path']);
     }
 }

tests/Symfony/Tests/Component/Security/Http/Firewall/LogoutListenerTest.php

@@ -0,0 +1,195 @@
+<?php
+
+/*
+ * This file is part of the Symfony framework.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\Tests\Component\Security\Http\Firewall;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Security\Http\Firewall\LogoutListener;
+
+class LogoutListenerTest extends \PHPUnit_Framework_TestCase
+{
+    public function testHandleUnmatchedPath()
+    {
+        list($listener, $context, $httpUtils, $options) = $this->getListener();
+
+        list($event, $request) = $this->getGetResponseEvent();
+
+        $event->expects($this->never())
+            ->method('setResponse');
+
+        $httpUtils->expects($this->once())
+            ->method('checkRequestPath')
+            ->with($request, $options['logout_path'])
+            ->will($this->returnValue(false));
+
+        $listener->handle($event);
+    }
+
+    public function testHandleMatchedPathWithSuccessHandler()
+    {
+        $successHandler = $this->getSuccessHandler();
+
+        list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler);
+
+        list($event, $request) = $this->getGetResponseEvent();
+
+        $httpUtils->expects($this->once())
+            ->method('checkRequestPath')
+            ->with($request, $options['logout_path'])
+            ->will($this->returnValue(true));
+
+        $successHandler->expects($this->once())
+            ->method('onLogoutSuccess')
+            ->with($request)
+            ->will($this->returnValue($response = new Response()));
+
+        $context->expects($this->once())
+            ->method('getToken')
+            ->will($this->returnValue($token = $this->getToken()));
+
+        $handler = $this->getHandler();
+        $handler->expects($this->once())
+            ->method('logout')
+            ->with($request, $response, $token);
+
+        $context->expects($this->once())
+            ->method('setToken')
+            ->with(null);
+
+        $event->expects($this->once())
+            ->method('setResponse')
+            ->with($response);
+
+        $listener->addHandler($handler);
+
+        $listener->handle($event);
+    }
+
+    public function testHandleMatchedPathWithoutSuccessHandler()
+    {
+        list($listener, $context, $httpUtils, $options) = $this->getListener();
+
+        list($event, $request) = $this->getGetResponseEvent();
+
+        $httpUtils->expects($this->once())
+            ->method('checkRequestPath')
+            ->with($request, $options['logout_path'])
+            ->will($this->returnValue(true));
+
+        $httpUtils->expects($this->once())
+            ->method('createRedirectResponse')
+            ->with($request, $options['target_url'])
+            ->will($this->returnValue($response = new Response()));
+
+        $context->expects($this->once())
+            ->method('getToken')
+            ->will($this->returnValue($token = $this->getToken()));
+
+        $handler = $this->getHandler();
+        $handler->expects($this->once())
+            ->method('logout')
+            ->with($request, $response, $token);
+
+        $context->expects($this->once())
+            ->method('setToken')
+            ->with(null);
+
+        $event->expects($this->once())
+            ->method('setResponse')
+            ->with($response);
+
+        $listener->addHandler($handler);
+
+        $listener->handle($event);
+    }
+
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testSuccessHandlerReturnsNonResponse()
+    {
+        $successHandler = $this->getSuccessHandler();
+
+        list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler);
+
+        list($event, $request) = $this->getGetResponseEvent();
+
+        $httpUtils->expects($this->once())
+            ->method('checkRequestPath')
+            ->with($request, $options['logout_path'])
+            ->will($this->returnValue(true));
+
+        $successHandler->expects($this->once())
+            ->method('onLogoutSuccess')
+            ->with($request)
+            ->will($this->returnValue(null));
+
+        $listener->handle($event);
+    }
+
+    private function getContext()
+    {
+        return $this->getMockBuilder('Symfony\Component\Security\Core\SecurityContext')
+            ->disableOriginalConstructor()
+            ->getMock();
+    }
+
+    private function getGetResponseEvent()
+    {
+        $event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $event->expects($this->any())
+            ->method('getRequest')
+            ->will($this->returnValue($request = new Request()));
+
+        return array($event, $request);
+    }
+
+    private function getHandler()
+    {
+        return $this->getMock('Symfony\Component\Security\Http\Logout\LogoutHandlerInterface');
+    }
+
+    private function getHttpUtils()
+    {
+        return $this->getMockBuilder('Symfony\Component\Security\Http\HttpUtils')
+            ->disableOriginalConstructor()
+            ->getMock();
+    }
+
+    private function getListener($successHandler = null)
+    {
+        $listener = new LogoutListener(
+            $context = $this->getContext(),
+            $httpUtils = $this->getHttpUtils(),
+            $options = array(
+                'logout_path' => '/logout',
+                'target_url' => '/',
+            ),
+            $successHandler
+        );
+
+        return array($listener, $context, $httpUtils, $options);
+    }
+
+    private function getSuccessHandler()
+    {
+        return $this->getMock('Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface');
+    }
+
+    private function getToken()
+    {
+        return $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
+    }
+}