8000 [Security] Fix clearing remember-me cookie after deauthentication · symfony/symfony@34907fd · GitHub
[go: up one dir, main page]

Skip to content
8000

Commit 34907fd

Browse files
author
Robin Chalas
committed
[Security] Fix clearing remember-me cookie after deauthentication
1 parent 5cacc5d commit 34907fd

File tree

9 files changed

+180
-8
lines changed

9 files changed

+180
-8
lines changed

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,15 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
7777
continue;
7878
}
7979

80-
if (!isset($attribute['provider'])) {
81-
throw new \RuntimeException('Each "security.remember_me_aware" tag must have a provider attribute.');
80+
// context listener only needs to clear the remember-me cookie
81+
if (0 !== strpos($serviceId, 'security.context_listener.')) {
82+
if (!isset($attribute['provider'])) {
83+
throw new \RuntimeException('Each "security.remember_me_aware" tag must have a provider attribute.');
84+
}
85+
86+
$userProviders[] = new Reference($attribute['provider']);
8287
}
8388

84-
$userProviders[] = new Reference($attribute['provider']);
8589
$container
8690
->getDefinition($serviceId)
8791
->addMethodCall('setRememberMeServices', [new Reference($rememberMeServicesId)])

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
374374
$listeners[] = new Reference('security.channel_listener');
375375

376376
$contextKey = null;
377+
$contextListenerId = null;
377378
// Context serializer listener
378379
if (false === $firewall['stateless']) {
379380
$contextKey = $id;
@@ -390,7 +391,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
390391
}
391392

392393
$this->logoutOnUserChangeByContextKey[$contextKey] = [$id, $logoutOnUserChange];
393-
$listeners[] = new Reference($this->createContextListener($container, $contextKey, $logoutOnUserChange));
394+
$listeners[] = new Reference($contextListenerId = $this->createContextListener($container, $contextKey, $logoutOnUserChange));
394395
$sessionStrategyId = 'security.authentication.session_strategy';
395396
} else {
396397
$this->statelessFirewallKeys[] = $id;
@@ -463,7 +464,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
463464
$configuredEntryPoint = isset($firewall['entry_point']) ? $firewall['entry_point'] : null;
464465

465466
// Authentication listeners
466-
list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $providerIds, $configuredEntryPoint);
467+
list($authListeners, $defaultEntryPoint) = $this->createAuthenticationListeners($container, $id, $firewall, $authenticationProviders, $defaultProvider, $providerIds, $configuredEntryPoint, $contextListenerId);
467468

468469
$config->replaceArgument(7, $configuredEntryPoint ?: $defaultEntryPoint);
469470

@@ -519,7 +520,7 @@ private function createContextListener($container, $contextKey, $logoutUserOnCha
519520
return $this->contextListeners[$contextKey] = $listenerId;
520521
}
521522

522-
private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider = null, array $providerIds, $defaultEntryPoint)
523+
private function createAuthenticationListeners($container, $id, $firewall, &$authenticationProviders, $defaultProvider = null, array $providerIds, $defaultEntryPoint, $contextListenerId = null)
523524
{
524525
$listeners = [];
525526
$hasListeners = false;
@@ -537,6 +538,9 @@ private function createAuthenticationListeners($container, $id, $firewall, &$aut
537538
} elseif ('remember_me' === $key) {
538539
// RememberMeFactory will use the firewall secret when created
539540
$userProvider = null;
541+
if ($contextListenerId) {
542+
$container->getDefinition($contextListenerId)->addTag('security.remember_me_aware', ['id' => $id]);
543+
}
540544
} else {
541545
$userProvider = $defaultProvider ?: $this->getFirstProvider($id, $key, $providerIds);
542546
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
13+
14+
use Symfony\Component\HttpFoundation\Response;
15+
use Symfony\Component\Security\Core\User\InMemoryUserProvider;
16+
use Symfony\Component\Security\Core\User\User;
17+
use Symfony\Component\Security\Core\User\UserInterface;
18+
use Symfony\Component\Security\Core\User\UserProviderInterface;
19+
20+
class RememberMeDeauthenticationTest extends AbstractWebTestCase
21+
{
22+
public function testRememberMeCookieGetsClearedOnUserChange()
23+
{
24+
$client = $this->createClient(['test_case' => 'RememberMeDeauthentication', 'root_config' => 'config.yml']);
25+
26+
$client->request('POST', '/login', [
27+
'_username' => 'johannes',
28+
'_password' => 'test',
29+
]);
30+
31+
$cookieJar = $client->getCookieJar();
32+
$this->assertNotNull($cookieJar->get('REMEMBERME'));
33+
34+
$client->request('GET', '/foo');
35+
$this->assertNull($cookieJar->get('REMEMBERME'));
36+
}
37+
}
38+
39+
class RememberMeFooController
40+
{
41+
public function __invoke(UserInterface $user)
42+
{
43+
return new Response($user->getUsername());
44+
}
45+
}
46+
47+
class RememberMeUserProvider implements UserProviderInterface
48+
{
49+
private $inner;
50+
51+
public function __construct(InMemoryUserProvider $inner)
52+
{
53+
$this->inner = $inner;
54+
}
55+
56+
public function loadUserByUsername($username)
57+
{
58+
return $this->inner->loadUserByUsername($username);
59+
}
60+
61+
public function refreshUser(UserInterface $user)
62+
{
63+
$user = $this->inner->refreshUser($user);
64+
65+
$alterUser = \Closure::bind(function (User $user) { $user->password = 'foo'; }, null, User::class);
66+
$alterUser($user);
67+
68+
return $user;
69+
}
70+
71+
public function supportsClass($class)
72+
{
73+
return $this->inner->supportsClass($class);
74+
}
75+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
13+
use Symfony\Bundle\SecurityBundle\SecurityBundle;
14+
15+
return [
16+
new FrameworkBundle(),
17+
new SecurityBundle(),
18+
];
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
imports:
2+
- { resource: ./../config/framework.yml }
3+
4+
security:
5+
encoders:
6+
Symfony\Component\Security\Core\User\User: plaintext
7+
8+
providers:
9+
in_memory:
10+
memory:
11+
users:
12+
johannes: { password: test, roles: [ROLE_USER] }
13+
14+
firewalls:
15+
default:
16+
form_login:
17+
check_path: login
18+
remember_me: true
19+
remember_me:
20+
always_remember_me: true
21+
secret: key
22+
anonymous: ~
23+
logout_on_user_change: true
24+
25+
access_control:
26+
- { path: ^/foo, roles: ROLE_USER }
27+
28+
services:
29+
Symfony\Bundle\SecurityBundle\Tests\Functional\RememberMeUserProvider:
30+
public: true
31+
decorates: security.user.provider.concrete.in_memory
32+
arguments: ['@Symfony\Bundle\SecurityBundle\Tests\Functional\RememberMeUserProvider.inner']
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
login:
2+
path: /login
3+
4+
foo:
5+
path: /foo
6+
defaults:
7+
_controller: Symfony\Bundle\SecurityBundle\Tests\Functional\RememberMeFooController

src/Symfony/Bundle/SecurityBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
"php": "^5.5.9|>=7.0.8",
2020
"ext-xml": "*",
2121
"symfony/config": "~3.4|~4.0",
22-
"symfony/security": "~3.4.15|~4.0.15|^4.1.4",
22+
"symfony/security": "~3.4.36|~4.3.9|^4.4.1",
2323
"symfony/dependency-injection": "^3.4.3|^4.0.3",
2424
"symfony/http-kernel": "~3.4|~4.0",
2525
"symfony/polyfill-php70": "~1.0"

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Symfony\Component\Security\Core\Role\SwitchUserRole;
2828
use Symfony\Component\Security\Core\User\UserInterface;
2929
use Symfony\Component\Security\Core\User\UserProviderInterface;
30+
use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
3031

3132
/**
3233
* ContextListener manages the SecurityContext persistence through a session.
@@ -44,6 +45,7 @@ class ContextListener implements ListenerInterface
4445
private $registered;
4546
private $trustResolver;
4647
private $logoutOnUserChange = false;
48+
private $rememberMeServices;
4749

4850
/**
4951
* @param iterable|UserProviderInterface[] $userProviders
@@ -103,6 +105,10 @@ public function handle(GetResponseEvent $event)
103105

104106
if ($token instanceof TokenInterface) {
105107
$token = $this->refreshUser($token);
108+
109+
if (!$token && $this->logoutOnUserChange && $this->rememberMeServices) {
110+
$this->rememberMeServices->loginFail($request);
111+
}
106112
} elseif (null !== $token) {
107113
if (null !== $this->logger) {
108114
$this->logger->warning('Expected a security token from the session, got something else.', ['key' => $this->sessionKey, 'received' => $token]);
@@ -268,4 +274,12 @@ public static function handleUnserializeCallback($class)
268274
{
269275
throw new \UnexpectedValueException('Class not found: '.$class, 0x37313bc);
270276
}
277+
278+
/**
279+
* @internal
280+
*/
281+
public function setRememberMeServices(RememberMeServicesInterface $rememberMeServices)
282+
{
283+
$this->rememberMeServices = $rememberMeServices;
284+
}
271285
}

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use Symfony\Component\Security\Core\User\UserInterface;
3232
use Symfony\Component\Security\Core\User\UserProviderInterface;
3333
use Symfony\Component\Security\Http\Firewall\ContextListener;
34+
use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
3435

3536
class ContextListenerTest extends TestCase
3637
{
@@ -278,6 +279,19 @@ public function testIfTokenIsNotDeauthenticated()
278279
$this->assertSame($goodRefreshedUser, $tokenStorage->getToken()->getUser());
279280
}
280281

282+
public function testRememberMeGetsCanceledIfTokenIsDeauthenticated()
283+
{
284+
$tokenStorage = new TokenStorage();
285+
$refreshedUser = new User('foobar', 'baz');
286+
287+
$rememberMeServices = $this->createMock(RememberMeServicesInterface::class);
288+
$rememberMeServices->expects($this->once())->method('loginFail');
289+
290+
$this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], null, true, $rememberMeServices);
291+
292+
$this->assertNull($tokenStorage->getToken());
293+
}
294+
281295
public function testTryAllUserProvidersUntilASupportingUserProviderIsFound()
282296
{
283297
$tokenStorage = new TokenStorage();
@@ -347,7 +361,7 @@ protected function runSessionOnKernelResponse($newToken, $original = null)
347361
return $session;
348362
}
349363

350-
private function handleEventWithPreviousSession(TokenStorageInterface $tokenStorage, $userProviders, UserInterface $user = null, $logoutOnUserChange = false)
364+
private function handleEventWithPreviousSession(TokenStorageInterface $tokenStorage, $userProviders, UserInterface $user = null, $logoutOnUserChange = false, RememberMeServicesInterface $rememberMeServices = null)
351365
{
352366
$user = $user ?: new User('foo', 'bar');
353367
$session = new Session(new MockArraySessionStorage());
@@ -359,6 +373,10 @@ private function handleEventWithPreviousSession(TokenStorageInterface $tokenStor
359373

360374
$listener = new ContextListener($tokenStorage, $userProviders, 'context_key');
361375
$listener->setLogoutOnUserChange($logoutOnUserChange);
376+
377+
if ($rememberMeServices) {
378+
$listener->setRememberMeServices($rememberMeServices);
379+
}
362380
$listener->handle(new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST));
363381
}
364382
}

0 commit comments

Comments
 (0)
0