[Security] Fix clearing remember-me cookie after deauthentication

symfony · Nov 29, 2019 · 1f57f19 · 1f57f19
1 parent 9eafff5
commit 1f57f19
Show file tree

Hide file tree

Showing 9 changed files with 177 additions and 6 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php
@@ -81,7 +81,11 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
                     throw new \RuntimeException('Each "security.remember_me_aware" tag must have a provider attribute.');
                 }
 
-                $userProviders[] = new Reference($attribute['provider']);
+                // context listeners don't need a provider
+                if ('none' !== $attribute['provider']) {
+                    $userProviders[] = new Reference($attribute['provider']);
+                }
+
                 $container
                     ->getDefinition($serviceId)
                     ->addMethodCall('setRememberMeServices', [new Reference($rememberMeServicesId)])

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -374,6 +374,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
         $listeners[] = new Reference('security.channel_listener');
 
         $contextKey = null;
+        $contextListenerId = null;
         // Context serializer listener
         if (false === $firewall['stateless']) {
             $contextKey = $id;
@@ -390,7 +391,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
             }
 
             $this->logoutOnUserChangeByContextKey[$contextKey] = [$id, $logoutOnUserChange];
-            $listeners[] = new Reference($this->createContextListener($container, $contextKey, $logoutOnUserChange));
+            $listeners[] = new Reference($contextListenerId = $this->createContextListener($container, $contextKey, $logoutOnUserChange));
             $sessionStrategyId = 'security.authentication.session_strategy';
         } else {
             $this->statelessFirewallKeys[] = $id;
@@ -463,7 +464,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
         $configuredEntryPoint = isset($firewall['entry_point']) ? $firewall['entry_point'] : null;
 
         // Authentication listeners
-        list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $providerIds, $configuredEntryPoint);
+        list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $providerIds, $configuredEntryPoint, $contextListenerId);
 
         $config->replaceArgument(7, $configuredEntryPoint ?: $defaultEntryPoint);
 
@@ -519,7 +520,7 @@ private function createContextListener($container, $contextKey, $logoutUserOnCha
         return $this->contextListeners[$contextKey] = $listenerId;
     }
 
-    private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider = null, array $providerIds, $defaultEntryPoint)
+    private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider = null, array $providerIds, $defaultEntryPoint, $contextListenerId = null)
     {
         $listeners = [];
         $hasListeners = false;
@@ -537,6 +538,9 @@ private function createAuthenticationListeners($container, $id, $firewall, &$aut
                     } elseif ('remember_me' === $key) {
                         // RememberMeFactory will use the firewall secret when created
                         $userProvider = null;
+                        if ($contextListenerId) {
+                            $container->getDefinition($contextListenerId)->addTag('security.remember_me_aware', ['id' => $id, 'provider' => 'none']);
+                        }
                     } else {
                         $userProvider = $defaultProvider ?: $this->getFirstProvider($id, $key, $providerIds);
                     }

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/RememberMeDeauthenticationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/RememberMeDeauthenticationTest.php
@@ -0,0 +1,77 @@
+<?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\Tests\Functional;
+
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Security\Core\User\InMemoryUserProvider;
+use Symfony\Component\Security\Core\User\User;
+use Symfony\Component\Security\Core\User\UserInterface;
+use Symfony\Component\Security\Core\User\UserProviderInterface;
+
+class RememberMeDeauthenticationTest extends AbstractWebTestCase
+{
+    public function testRememberMeCookieGetsClearedOnUserChange()
+    {
+        $client = $this->createClient(['test_case' => 'RememberMeDeauthentication', 'root_config' => 'config.yml']);
+
+        $client->request('POST', '/login', [
+            '_username' => 'johannes',
+            '_password' => 'test',
+        ]);
+
+        $this->assertSame(302, $client->getResponse()->getStatusCode());
+        $cookieJar = $client->getCookieJar();
+        $this->assertNotNull($cookieJar->get('REMEMBERME'));
+
+        $client->request('GET', '/foo');
+        $this->assertSame(200, $client->getResponse()->getStatusCode());
+        $this->assertNull($cookieJar->get('REMEMBERME'));
+    }
+}
+
+class RememberMeFooController
+{
+    public function __invoke(UserInterface $user)
+    {
+        return new Response($user->getUsername());
+    }
+}
+
+class RememberMeUserProvider implements UserProviderInterface
+{
+    private $inner;
+
+    public function __construct(InMemoryUserProvider $inner)
+    {
+        $this->inner = $inner;
+    }
+
+    public function loadUserByUsername($username)
+    {
+        return $this->inner->loadUserByUsername($username);
+    }
+
+    public function refreshUser(UserInterface $user)
+    {
+        $user = $this->inner->refreshUser($user);
+
+        $alterUser = \Closure::bind(function (User $user) { $user->password = 'foo'; }, null, User::class);
+        $alterUser($user);
+
+        return $user;
+    }
+
+    public function supportsClass($class)
+    {
+        return $this->inner->supportsClass($class);
+    }
+}
diff --git a/...Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/bundles.php b/...Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/bundles.php
@@ -0,0 +1,18 @@
+<?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.
+ */
+
+use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
+use Symfony\Bundle\SecurityBundle\SecurityBundle;
+
+return [
+    new FrameworkBundle(),
+    new SecurityBundle(),
+];
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/config.yml
@@ -0,0 +1,32 @@
+imports:
+    - { resource: ./../config/framework.yml }
+
+security:
+    encoders:
+        Symfony\Component\Security\Core\User\User: plaintext
+
+    providers:
+        in_memory:
+            memory:
+                users:
+                    johannes: { password: test, roles: [ROLE_USER] }
+
+    firewalls:
+        default:
+            form_login:
+                check_path: login
+                remember_me: true
+            remember_me:
+                always_remember_me: true
+                secret: key
+            anonymous: ~
+            logout_on_user_change: true
+
+    access_control:
+        - { path: ^/foo, roles: ROLE_USER }
+
+services:
+    Symfony\Bundle\SecurityBundle\Tests\Functional\RememberMeUserProvider:
+        public: true
+        decorates: security.user.provider.concrete.in_memory
+        arguments: ['@Symfony\Bundle\SecurityBundle\Tests\Functional\RememberMeUserProvider.inner']
diff --git a/...Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/routing.yml b/...Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/routing.yml
@@ -0,0 +1,7 @@
+login:
+    path: /login
+
+foo:
+    path: /foo
+    defaults:
+        _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\RememberMeFooController
diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json
@@ -19,7 +19,7 @@
         "php": "^5.5.9|>=7.0.8",
         "ext-xml": "*",
         "symfony/config": "~3.4|~4.0",
-        "symfony/security": "~3.4.15|~4.0.15|^4.1.4",
+        "symfony/security": "~3.4.36|~4.3.9|^4.4.1",
         "symfony/dependency-injection": "^3.4.3|^4.0.3",
         "symfony/http-kernel": "~3.4|~4.0",
         "symfony/polyfill-php70": "~1.0"

diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php
@@ -27,6 +27,7 @@
 use Symfony\Component\Security\Core\Role\SwitchUserRole;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
+use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
 
 /**
  * ContextListener manages the SecurityContext persistence through a session.
@@ -44,6 +45,7 @@ class ContextListener implements ListenerInterface
     private $registered;
     private $trustResolver;
     private $logoutOnUserChange = false;
+    private $rememberMeServices;
 
     /**
      * @param iterable|UserProviderInterface[] $userProviders
@@ -103,6 +105,10 @@ public function handle(GetResponseEvent $event)
 
         if ($token instanceof TokenInterface) {
             $token = $this->refreshUser($token);
+
+            if (!$token && $this->logoutOnUserChange && $this->rememberMeServices) {
+                $this->rememberMeServices->loginFail($request);
+            }
         } elseif (null !== $token) {
             if (null !== $this->logger) {
                 $this->logger->warning('Expected a security token from the session, got something else.', ['key' => $this->sessionKey, 'received' => $token]);
@@ -268,4 +274,9 @@ public static function handleUnserializeCallback($class)
     {
         throw new \UnexpectedValueException('Class not found: '.$class, 0x37313bc);
     }
+
+    public function setRememberMeServices(RememberMeServicesInterface $rememberMeServices)
+    {
+        $this->rememberMeServices = $rememberMeServices;
+    }
 }
diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php
@@ -31,6 +31,7 @@
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 use Symfony\Component\Security\Http\Firewall\ContextListener;
+use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
 
 class ContextListenerTest extends TestCase
 {
@@ -278,6 +279,19 @@ public function testIfTokenIsNotDeauthenticated()
         $this->assertSame($goodRefreshedUser, $tokenStorage->getToken()->getUser());
     }
 
+    public function testRememberMeGetsCanceledIfTokenIsDeauthenticated()
+    {
+        $tokenStorage = new TokenStorage();
+        $refreshedUser = new User('foobar', 'baz');
+
+        $rememberMeServices = $this->createMock(RememberMeServicesInterface::class);
+        $rememberMeServices->expects($this->once())->method('loginFail');
+
+        $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], null, true, $rememberMeServices);
+
+        $this->assertNull($tokenStorage->getToken());
+    }
+
     public function testTryAllUserProvidersUntilASupportingUserProviderIsFound()
     {
         $tokenStorage = new TokenStorage();
@@ -347,7 +361,7 @@ protected function runSessionOnKernelResponse($newToken, $original = null)
         return $session;
     }
 
-    private function handleEventWithPreviousSession(TokenStorageInterface $tokenStorage, $userProviders, UserInterface $user = null, $logoutOnUserChange = false)
+    private function handleEventWithPreviousSession(TokenStorageInterface $tokenStorage, $userProviders, UserInterface $user = null, $logoutOnUserChange = false, RememberMeServicesInterface $rememberMeServices = null)
     {
         $user = $user ?: new User('foo', 'bar');
         $session = new Session(new MockArraySessionStorage());
@@ -359,6 +373,10 @@ private function handleEventWithPreviousSession(TokenStorageInterface $tokenStor
 
         $listener = new ContextListener($tokenStorage, $userProviders, 'context_key');
         $listener->setLogoutOnUserChange($logoutOnUserChange);
+
+        if ($rememberMeServices) {
+            $listener->setRememberMeServices($rememberMeServices);
+        }
         $listener->handle(new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST));
     }
 }