From 1f57f19ba86922a3b9d6efad5ce681b941a03364 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 28 Nov 2019 04:41:34 +0100 Subject: [PATCH] [Security] Fix clearing remember-me cookie after deauthentication --- .../Security/Factory/RememberMeFactory.php | 6 +- .../DependencyInjection/SecurityExtension.php | 10 ++- .../RememberMeDeauthenticationTest.php | 77 +++++++++++++++++++ .../RememberMeDeauthentication/bundles.php | 18 +++++ .../app/RememberMeDeauthentication/config.yml | 32 ++++++++ .../RememberMeDeauthentication/routing.yml | 7 ++ .../Bundle/SecurityBundle/composer.json | 2 +- .../Http/Firewall/ContextListener.php | 11 +++ .../Tests/Firewall/ContextListenerTest.php | 20 ++++- 9 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/RememberMeDeauthenticationTest.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/bundles.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/config.yml create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/routing.yml diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php index 34de3d6701470..8d419c0edd8f0 100644 --- 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 index 795fe053e66e4..19e9beb1e37e6 100644 --- 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 new file mode 100644 index 0000000000000..4dc2e35c18643 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/RememberMeDeauthenticationTest.php @@ -0,0 +1,77 @@ + + * + * 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/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/bundles.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/bundles.php new file mode 100644 index 0000000000000..9a26fb163a77d --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/bundles.php @@ -0,0 +1,18 @@ + + * + * 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 new file mode 100644 index 0000000000000..e5cefd37df76d --- /dev/null +++ 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/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/RememberMeDeauthentication/routing.yml new file mode 100644 index 0000000000000..08975bdcb3832 --- /dev/null +++ b/src/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 index 1f0e56e6eedde..1a8057b6fbd08 100644 --- 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 index c6b89793e66e7..ea9f51f9224ad 100644 --- 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 index 25915f212a4c0..acab7087cb92f 100644 --- 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)); } }