merged branch asm89/default-logout-success-handler (PR #4921)

Commits
-------

df2406f [Security] Add note to changelog about BC break
01b2e39 [Security] Extract default logout success handling logic

Discussion
----------

[Security] Extract default logout success handling logic

Bug fix: no
Feature addition: no
Backwards compatibility break: yes, small one for people using the component
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=default-logout-success-handler)](http://travis-ci.org/asm89/symfony)
License of the code: MIT

As discussed earlier with @fabpot and @schmittjoh. This PR extracts the default logout success handling logic to a separate class that users can extend.

Note: build status is red, but that is because of a failing performance test in the form component? ..

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

@@ -279,18 +279,22 @@ 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, array(
+            $listener->replaceArgument(3, array(
                 'csrf_parameter' => $firewall['logout']['csrf_parameter'],
                 'intention'      => $firewall['logout']['intention'],
                 'logout_path'    => $firewall['logout']['path'],
-                'target_url'     => $firewall['logout']['target'],
             ));
             $listeners[] = new Reference($listenerId);
 
             // add logout success handler
             if (isset($firewall['logout']['success_handler'])) {
-                $listener->replaceArgument(3, new Reference($firewall['logout']['success_handler']));
+                $logoutSuccessHandlerId = $firewall['logout']['success_handler'];
+            } else {
+                $logoutSuccessHandlerId = 'security.logout.success_handler.'.$id;
+                $logoutSuccessHandler = $container->setDefinition($logoutSuccessHandlerId, new DefinitionDecorator('security.logout.success_handler'));
+                $logoutSuccessHandler->replaceArgument(1, $firewall['logout']['target']);
             }
+            $listener->replaceArgument(2, new Reference($logoutSuccessHandlerId));
 
             // add CSRF provider
             if (isset($firewall['logout']['csrf_provider'])) {

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

@@ -27,6 +27,7 @@
         <parameter key="security.logout_listener.class">Symfony\Component\Security\Http\Firewall\LogoutListener</parameter>
         <parameter key="security.logout.handler.session.class">Symfony\Component\Security\Http\Logout\SessionLogoutHandler</parameter>
         <parameter key="security.logout.handler.cookie_clearing.class">Symfony\Component\Security\Http\Logout\CookieClearingLogoutHandler</parameter>
+        <parameter key="security.logout.success_handler.class">Symfony\Component\Security\Http\Logout\DefaultLogoutSuccessHandler</parameter>
 
         <parameter key="security.access_listener.class">Symfony\Component\Security\Http\Firewall\AccessListener</parameter>
         <parameter key="security.access_map.class">Symfony\Component\Security\Http\AccessMap</parameter>
@@ -84,12 +85,17 @@
         <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 type="service" id="security.logout.success_handler" />
             <argument /> <!-- Options -->
-            <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" />
         <service id="security.logout.handler.cookie_clearing" class="%security.logout.handler.cookie_clearing.class%" public="false" abstract="true" />
 
+        <service id="security.logout.success_handler" class="%security.logout.success_handler.class%" public="false" abstract="true">
+            <argument type="service" id="security.http_utils" />
+            <argument>/</argument>
+        </service>
+
         <service id="security.authentication.form_entry_point" class="%security.authentication.form_entry_point.class%" public="false" abstract="true">
             <argument type="service" id="http_kernel" />
         </service>

src/Symfony/Component/Security/CHANGELOG.md

@@ -23,3 +23,5 @@ CHANGELOG
  * [BC BREAK] moved the default authentication success and failure handling to
    seperate classes. The order of arguments in the constructor of the
    `AbstractAuthenticationListener` has changed.
+ * [BC BREAK] moved the default logout sucess handling to a seperate class. The
+   order of arguments in the constructor of `LogoutListener` has changed.

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

@@ -40,11 +40,11 @@ class LogoutListener implements ListenerInterface
      *
      * @param SecurityContextInterface      $securityContext
      * @param HttpUtils                     $httpUtils       An HttpUtilsInterface instance
-     * @param array                         $options         An array of options to process a logout attempt
      * @param LogoutSuccessHandlerInterface $successHandler  A LogoutSuccessHandlerInterface instance
+     * @param array                         $options         An array of options to process a logout attempt
      * @param CsrfProviderInterface         $csrfProvider    A CsrfProviderInterface instance
      */
-    public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, array $options = array(), LogoutSuccessHandlerInterface $successHandler = null, CsrfProviderInterface $csrfProvider = null)
+    public function __construct(SecurityContextInterface $securityContext, HttpUtils $httpUtils, LogoutSuccessHandlerInterface $successHandler, array $options = array(), CsrfProviderInterface $csrfProvider = null)
     {
         $this->securityContext = $securityContext;
         $this->httpUtils = $httpUtils;
@@ -52,7 +52,6 @@ class LogoutListener implements ListenerInterface
             'csrf_parameter' => '_csrf_token',
             'intention'      => 'logout',
             'logout_path'    => '/logout',
-            'target_url'     => '/',
         ), $options);
         $this->successHandler = $successHandler;
         $this->csrfProvider = $csrfProvider;
@@ -95,14 +94,9 @@ class LogoutListener implements ListenerInterface
             }
         }
 
-        if (null !== $this->successHandler) {
+        $response = $this->successHandler->onLogoutSuccess($request);
-            $response = $this->successHandler->onLogoutSuccess($request);
+        if (!$response instanceof Response) {
-
+            throw new \RuntimeException('Logout Success Handler did not return a Response.');
-            if (!$response instanceof Response) {
-                throw new \RuntimeException('Logout Success Handler did not return a Response.');
-            }
-        } else {
-            $response = $this->httpUtils->createRedirectResponse($request, $this->options['target_url']);
         }
 
         // handle multiple logout attempts gracefully

src/Symfony/Component/Security/Http/Logout/DefaultLogoutSuccessHandler.php

@@ -0,0 +1,47 @@
+<?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\Http\Logout;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Http\HttpUtils;
+use Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface;
+
+/**
+ * Default logout success handler will redirect users to a configured path.
+ *
+ * @author Fabien Potencier <fabien@symfony.com>
+ * @author Alexander <iam.asm89@gmail.com>
+ */
+class DefaultLogoutSuccessHandler implements LogoutSuccessHandlerInterface
+{
+    protected $httpUtils;
+    protected $targetUrl;
+
+    /**
+     * @param HttpUtils $httpUtils
+     * @param string    $targetUrl
+     */
+    public function __construct(HttpUtils $httpUtils, $targetUrl = '/')
+    {
+        $this->httpUtils = $httpUtils;
+
+        $this->targetUrl = $targetUrl;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function onLogoutSuccess(Request $request)
+    {
+        return $this->httpUtils->createRedirectResponse($request, $this->targetUrl);
+    }
+}

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

@@ -103,7 +103,9 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase
 
     public function testHandleMatchedPathWithoutSuccessHandlerAndCsrfValidation()
     {
-        list($listener, $context, $httpUtils, $options) = $this->getListener();
+        $successHandler = $this->getSuccessHandler();
+
+        list($listener, $context, $httpUtils, $options) = $this->getListener($successHandler);
 
         list($event, $request) = $this->getGetResponseEvent();
 
@@ -112,9 +114,9 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase
             ->with($request, $options['logout_path'])
             ->will($this->returnValue(true));
 
-        $httpUtils->expects($this->once())
+        $successHandler->expects($this->once())
-            ->method('createRedirectResponse')
+            ->method('onLogoutSuccess')
-            ->with($request, $options['target_url'])
+            ->with($request)
             ->will($this->returnValue($response = new Response()));
 
         $context->expects($this->once())
@@ -231,13 +233,13 @@ class LogoutListenerTest extends \PHPUnit_Framework_TestCase
         $listener = new LogoutListener(
             $context = $this->getContext(),
             $httpUtils = $this->getHttpUtils(),
+            $successHandler ?: $this->getSuccessHandler(),
             $options = array(
                 'csrf_parameter' => '_csrf_token',
                 'intention'      => 'logout',
                 'logout_path'    => '/logout',
                 'target_url'     => '/',
             ),
-            $successHandler,
             $csrfProvider
         );