feature #20516 [Security][SecurityBundle] Enhance automatic logout ur…

…l generation (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Security][SecurityBundle] Enhance automatic logout url generation

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

This should help whenever:

- [the token does not implement the `getProviderKey` method](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Logout/LogoutUrlGenerator.php#L89-L99)
- you've got multiple firewalls sharing a same context but a logout listener only define on one of them.

##### Behavior:

> When not providing the firewall key:
>
>- Try to find the key from the token (unless it's an anonymous token)
>- If found, try to get the listener from the key. If the listener is found, stop there.
>- Try from the injected firewall key. If the listener is found, stop there.
>- Try from the injected firewall context. If the listener is found, stop there.
>
>The behavior remains unchanged when providing explicitly the firewall key. No fallback.

Commits
-------

5b7fe85 [Security][SecurityBundle] Enhance automatic logout url generation

UPGRADE-3.3.md

@@ -246,6 +246,9 @@ Security
  * The `RoleInterface` has been deprecated. Extend the `Symfony\Component\Security\Core\Role\Role`
    class in your custom role implementations instead.
 
+ * The `LogoutUrlGenerator::registerListener()` method will expect a 6th `$context = null` argument in 4.0.
+   Define the argument when overriding this method.
+
 SecurityBundle
 --------------
 

UPGRADE-4.0.md

@@ -373,6 +373,8 @@ Security
 
  * The `RoleInterface` has been removed. Extend the `Symfony\Component\Security\Core\Role\Role`
    class instead.
+
+ * The `LogoutUrlGenerator::registerListener()` method expects a 6th `$context = null` argument.
 
 SecurityBundle
 --------------

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

@@ -389,6 +389,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
                     $firewall['logout']['csrf_token_id'],
                     $firewall['logout']['csrf_parameter'],
                     isset($firewall['logout']['csrf_token_generator']) ? new Reference($firewall['logout']['csrf_token_generator']) : null,
+                    false === $firewall['stateless'] && isset($firewall['context']) ? $firewall['context'] : null,
                 ))
             ;
         }

src/Symfony/Bundle/SecurityBundle/EventListener/FirewallListener.php

@@ -0,0 +1,59 @@
+<?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\Bundle\SecurityBundle\EventListener;
+
+use Symfony\Bundle\SecurityBundle\Security\FirewallMap;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\Security\Http\Firewall;
+use Symfony\Component\Security\Http\FirewallMapInterface;
+use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
+
+/**
+ * @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
+ */
+class FirewallListener extends Firewall
+{
+    private $map;
+    private $logoutUrlGenerator;
+
+    public function __construct(FirewallMapInterface $map, EventDispatcherInterface $dispatcher, LogoutUrlGenerator $logoutUrlGenerator)
+    {
+        $this->map = $map;
+        $this->logoutUrlGenerator = $logoutUrlGenerator;
+
+        parent::__construct($map, $dispatcher);
+    }
+
+    public function onKernelRequest(GetResponseEvent $event)
+    {
+        if (!$event->isMasterRequest()) {
+            return;
+        }
+
+        if ($this->map instanceof FirewallMap && $config = $this->map->getFirewallConfig($event->getRequest())) {
+            $this->logoutUrlGenerator->setCurrentFirewall($config->getName(), $config->getContext());
+        }
+
+        parent::onKernelRequest($event);
+    }
+
+    public function onKernelFinishRequest(FinishRequestEvent $event)
+    {
+        if ($event->isMasterRequest()) {
+            $this->logoutUrlGenerator->setCurrentFirewall(null);
+        }
+
+        parent::onKernelFinishRequest($event);
+    }
+}

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

@@ -102,10 +102,11 @@
 
 
         <!-- Firewall related services -->
-        <service id="security.firewall" class="Symfony\Component\Security\Http\Firewall">
+        <service id="security.firewall" class="Symfony\Bundle\SecurityBundle\EventListener\FirewallListener">
             <tag name="kernel.event_subscriber" />
             <argument type="service" id="security.firewall.map" />
             <argument type="service" id="event_dispatcher" />
+            <argument type="service" id="security.logout_url_generator" />
         </service>
         <service id="Symfony\Component\Security\Http\Firewall" alias="security.firewall" public="false" />
 

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

@@ -13,6 +13,7 @@
 
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 
@@ -28,6 +29,7 @@ class LogoutUrlGenerator
     private $router;
     private $tokenStorage;
     private $listeners = array();
+    private $currentFirewall;
 
     public function __construct(RequestStack $requestStack = null, UrlGeneratorInterface $router = null, TokenStorageInterface $tokenStorage = null)
     {
@@ -39,15 +41,29 @@ public function __construct(RequestStack $requestStack = null, UrlGeneratorInter
     /**
      * Registers a firewall's LogoutListener, allowing its URL to be generated.
      *
-     * @param string                    $key              The firewall key
-     * @param string                    $logoutPath       The path that starts the logout process
-     * @param string                    $csrfTokenId      The ID of the CSRF token
-     * @param string                    $csrfParameter    The CSRF token parameter name
-     * @param CsrfTokenManagerInterface $csrfTokenManager A CsrfTokenManagerInterface instance
+     * @param string                         $key              The firewall key
+     * @param string                         $logoutPath       The path that starts the logout process
+     * @param string                         $csrfTokenId      The ID of the CSRF token
+     * @param string                         $csrfParameter    The CSRF token parameter name
+     * @param CsrfTokenManagerInterface|null $csrfTokenManager A CsrfTokenManagerInterface instance
+     * @param string|null                    $context          The listener context
      */
-    public function registerListener($key, $logoutPath, $csrfTokenId, $csrfParameter, CsrfTokenManagerInterface $csrfTokenManager = null)
+    public function registerListener($key, $logoutPath, $csrfTokenId, $csrfParameter, CsrfTokenManagerInterface $csrfTokenManager = null/*, $context = null*/)
     {
-        $this->listeners[$key] = array($logoutPath, $csrfTokenId, $csrfParameter, $csrfTokenManager);
+        if (func_num_args() >= 6) {
+            $context = func_get_arg(5);
+        } else {
+            if (__CLASS__ !== get_class($this)) {
+                $r = new \ReflectionMethod($this, __FUNCTION__);
+                if (__CLASS__ !== $r->getDeclaringClass()->getName()) {
+                    @trigger_error(sprintf('Method %s() will have a sixth `$context = null` argument in version 4.0. Not defining it is deprecated since 3.3.', get_class($this), __FUNCTION__), E_USER_DEPRECATED);
+                }
+            }
+
+            $context = null;
+        }
+
+        $this->listeners[$key] = array($logoutPath, $csrfTokenId, $csrfParameter, $csrfTokenManager, $context);
     }
 
     /**
@@ -74,35 +90,26 @@ public function getLogoutUrl($key = null)
         return $this->generateLogoutUrl($key, UrlGeneratorInterface::ABSOLUTE_URL);
     }
 
+    /**
+     * @param string|null $key     The current firewall key
+     * @param string|null $context The current firewall context
+     */
+    public function setCurrentFirewall($key, $context = null)
+    {
+        $this->currentFirewall = array($key, $context);
+    }
+
     /**
      * Generates the logout URL for the firewall.
      *
      * @param string|null $key           The firewall key or null to use the current firewall key
      * @param int         $referenceType The type of reference (one of the constants in UrlGeneratorInterface)
      *
      * @return string The logout URL
-     *
-     * @throws \InvalidArgumentException if no LogoutListener is registered for the key or the key could not be found automatically.
      */
     private function generateLogoutUrl($key, $referenceType)
     {
-        // Fetch the current provider key from token, if possible
-        if (null === $key && null !== $this->tokenStorage) {
-            $token = $this->tokenStorage->getToken();
-            if (null !== $token && method_exists($token, 'getProviderKey')) {
-                $key = $token->getProviderKey();
-            }
-        }
-
-        if (null === $key) {
-            throw new \InvalidArgumentException('Unable to find the current firewall LogoutListener, please provide the provider key manually.');
-        }
-
-        if (!array_key_exists($key, $this->listeners)) {
-            throw new \InvalidArgumentException(sprintf('No LogoutListener found for firewall key "%s".', $key));
-        }
-
-        list($logoutPath, $csrfTokenId, $csrfParameter, $csrfTokenManager) = $this->listeners[$key];
+        list($logoutPath, $csrfTokenId, $csrfParameter, $csrfTokenManager) = $this->getListener($key);
 
         $parameters = null !== $csrfTokenManager ? array($csrfParameter => (string) $csrfTokenManager->getToken($csrfTokenId)) : array();
 
@@ -128,4 +135,54 @@ private function generateLogoutUrl($key, $referenceType)
 
         return $url;
     }
+
+    /**
+     * @param string|null $key The firewall key or null use the current firewall key
+     *
+     * @return array The logout listener found
+     *
+     * @throws \InvalidArgumentException if no LogoutListener is registered for the key or could not be found automatically.
+     */
+    private function getListener($key)
+    {
+        if (null !== $key) {
+            if (isset($this->listeners[$key])) {
+                return $this->listeners[$key];
+            }
+
+            throw new \InvalidArgumentException(sprintf('No LogoutListener found for firewall key "%s".', $key));
+        }
+
+        // Fetch the current provider key from token, if possible
+        if (null !== $this->tokenStorage) {
+            $token = $this->tokenStorage->getToken();
+
+            if ($token instanceof AnonymousToken) {
+                throw new \InvalidArgumentException('Unable to generate a logout url for an anonymous token.');
+            }
+
+            if (null !== $token && method_exists($token, 'getProviderKey')) {
+                $key = $token->getProviderKey();
+
+                if (isset($this->listeners[$key])) {
+                    return $this->listeners[$key];
+                }
+            }
+        }
+
+        // Fetch from injected current firewall information, if possible
+        list($key, $context) = $this->currentFirewall;
+
+        if (isset($this->listeners[$key])) {
+            return $this->listeners[$key];
+        }
+
+        foreach ($this->listeners as $listener) {
+            if (isset($listener[4]) && $context === $listener[4]) {
+                return $listener;
+            }
+        }
+
+        throw new \InvalidArgumentException('Unable to find the current firewall LogoutListener, please provide the provider key manually.');
+    }
 }

src/Symfony/Component/Security/Http/Tests/Logout/LogoutUrlGeneratorTest.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\Http\Tests\Logout;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
+use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
+
+/**
+ * @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
+ */
+class LogoutUrlGeneratorTest extends TestCase
+{
+    /** @var TokenStorage */
+    private $tokenStorage;
+    /** @var LogoutUrlGenerator */
+    private $generator;
+
+    protected function setUp()
+    {
+        $requestStack = $this->getMockBuilder(RequestStack::class)->getMock();
+        $request = $this->getMockBuilder(Request::class)->getMock();
+        $requestStack->method('getCurrentRequest')->willReturn($request);
+
+        $this->tokenStorage = new TokenStorage();
+        $this->generator = new LogoutUrlGenerator($requestStack, null, $this->tokenStorage);
+    }
+
+    public function testGetLogoutPath()
+    {
+        $this->generator->registerListener('secured_area', '/logout', null, null);
+
+        $this->assertSame('/logout', $this->generator->getLogoutPath('secured_area'));
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage No LogoutListener found for firewall key "unregistered_key".
+     */
+    public function testGetLogoutPathWithoutLogoutListenerRegisteredForKeyThrowsException()
+    {
+        $this->generator->registerListener('secured_area', '/logout', null, null, null);
+
+        $this->generator->getLogoutPath('unregistered_key');
+    }
+
+    public function testGuessFromToken()
+    {
+        $this->tokenStorage->setToken(new UsernamePasswordToken('user', 'password', 'secured_area'));
+        $this->generator->registerListener('secured_area', '/logout', null, null);
+
+        $this->assertSame('/logout', $this->generator->getLogoutPath());
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage Unable to generate a logout url for an anonymous token.
+     */
+    public function testGuessFromAnonymousTokenThrowsException()
+    {
+        $this->tokenStorage->setToken(new AnonymousToken('default', 'anon.'));
+
+        $this->generator->getLogoutPath();
+    }
+
+    public function testGuessFromCurrentFirewallKey()
+    {
+        $this->generator->registerListener('secured_area', '/logout', null, null);
+        $this->generator->setCurrentFirewall('secured_area');
+
+        $this->assertSame('/logout', $this->generator->getLogoutPath());
+    }
+
+    public function testGuessFromCurrentFirewallContext()
+    {
+        $this->generator->registerListener('secured_area', '/logout', null, null, null, 'secured');
+        $this->generator->setCurrentFirewall('admin', 'secured');
+
+        $this->assertSame('/logout', $this->generator->getLogoutPath());
+    }
+
+    public function testGuessFromTokenWithoutProviderKeyFallbacksToCurrentFirewall()
+    {
+        $this->tokenStorage->setToken($this->getMockBuilder(TokenInterface::class)->getMock());
+        $this->generator->registerListener('secured_area', '/logout', null, null);
+        $this->generator->setCurrentFirewall('secured_area');
+
+        $this->assertSame('/logout', $this->generator->getLogoutPath());
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage Unable to find the current firewall LogoutListener, please provide the provider key manually
+     */
+    public function testUnableToGuessThrowsException()
+    {
+        $this->generator->registerListener('secured_area', '/logout', null, null);
+
+        $this->generator->getLogoutPath();
+    }
+}