From d1b5093aa838d3547e7d667a5d5cfb20647f2a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Terje=20Br=C3=A5ten?= Date: Sun, 18 Nov 2012 22:41:04 +0100 Subject: [PATCH] Try to make sure cookies get deleted from the TokenProvider when no longer in use --- .../RememberMe/AbstractRememberMeServices.php | 3 +++ .../PersistentTokenBasedRememberMeServices.php | 9 +++++---- .../AbstractRememberMeServicesTest.php | 16 ++++++++-------- .../TokenBasedRememberMeServicesTest.php | 2 +- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php index 4f7c5b989e..e7a78ce6f0 100644 --- a/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php +++ b/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php @@ -172,6 +172,9 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface */ final public function loginSuccess(Request $request, Response $response, TokenInterface $token) { + // Make sure any old remember-me cookies are cancelled + $this->cancelCookie($request); + if (!$token->getUser() instanceof UserInterface) { if (null !== $this->logger) { $this->logger->debug('Remember-me ignores token since it does not contain a UserInterface implementation.'); diff --git a/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php index dbb6429911..2ad47f8c5e 100644 --- a/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php +++ b/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php @@ -63,10 +63,12 @@ class PersistentTokenBasedRememberMeServices extends AbstractRememberMeServices /** * {@inheritDoc} */ - public function logout(Request $request, Response $response, TokenInterface $token) + protected function cancelCookie(Request $request) { - parent::logout($request, $response, $token); + // Delete cookie on the client + parent::cancelCookie($request); + // Delete cookie from the tokenProvider if (null !== ($cookie = $request->cookies->get($this->options['name'])) && count($parts = $this->decodeCookie($cookie)) === 2 ) { @@ -88,8 +90,6 @@ class PersistentTokenBasedRememberMeServices extends AbstractRememberMeServices $persistentToken = $this->tokenProvider->loadTokenBySeries($series); if ($persistentToken->getTokenValue() !== $tokenValue) { - $this->tokenProvider->deleteTokenBySeries($series); - throw new CookieTheftException('This token was already used. The account is possibly compromised.'); } @@ -133,6 +133,7 @@ class PersistentTokenBasedRememberMeServices extends AbstractRememberMeServices ) ); + $request->attributes->remove(self::COOKIE_ATTR_NAME); $response->headers->setCookie( new Cookie( $this->options['name'], diff --git a/src/Symfony/Component/Security/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php b/src/Symfony/Component/Security/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php index fc8dffb60a..857168643c 100644 --- a/src/Symfony/Component/Security/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php +++ b/src/Symfony/Component/Security/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php @@ -39,7 +39,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase public function testAutoLoginReturnsNullWhenNoCookie() { - $service = $this->getService(null, array('name' => 'foo')); + $service = $this->getService(null, array('name' => 'foo', 'path' => null, 'domain' => null)); $this->assertNull($service->autoLogin(new Request())); } @@ -49,7 +49,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase */ public function testAutoLoginThrowsExceptionWhenImplementationDoesNotReturnUserInterface() { - $service = $this->getService(null, array('name' => 'foo')); + $service = $this->getService(null, array('name' => 'foo', 'path' => null, 'domain' => null)); $request = new Request; $request->cookies->set('foo', 'foo'); @@ -64,7 +64,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase public function testAutoLogin() { - $service = $this->getService(null, array('name' => 'foo')); + $service = $this->getService(null, array('name' => 'foo', 'path' => null, 'domain' => null)); $request = new Request(); $request->cookies->set('foo', 'foo'); @@ -112,7 +112,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase public function testLoginSuccessIsNotProcessedWhenTokenDoesNotContainUserInterfaceImplementation() { - $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => true)); + $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => true, 'path' => null, 'domain' => null)); $request = new Request; $response = new Response; $account = $this->getMock('Symfony\Component\Security\Core\User\UserInterface'); @@ -135,7 +135,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase public function testLoginSuccessIsNotProcessedWhenRememberMeIsNotRequested() { - $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => false, 'remember_me_parameter' => 'foo')); + $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => false, 'remember_me_parameter' => 'foo', 'path' => null, 'domain' => null)); $request = new Request; $response = new Response; $account = $this->getMock('Symfony\Component\Security\Core\User\UserInterface'); @@ -159,7 +159,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase public function testLoginSuccessWhenRememberMeAlwaysIsTrue() { - $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => true)); + $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => true, 'path' => null, 'domain' => null)); $request = new Request; $response = new Response; $account = $this->getMock('Symfony\Component\Security\Core\User\UserInterface'); @@ -184,7 +184,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase */ public function testLoginSuccessWhenRememberMeParameterWithPathIsPositive($value) { - $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => false, 'remember_me_parameter' => 'foo[bar]')); + $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => false, 'remember_me_parameter' => 'foo[bar]', 'path' => null, 'domain' => null)); $request = new Request; $request->request->set('foo', array('bar' => $value)); @@ -211,7 +211,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase */ public function testLoginSuccessWhenRememberMeParameterIsPositive($value) { - $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => false, 'remember_me_parameter' => 'foo')); + $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => false, 'remember_me_parameter' => 'foo', 'path' => null, 'domain' => null)); $request = new Request; $request->request->set('foo', $value); diff --git a/src/Symfony/Component/Security/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php b/src/Symfony/Component/Security/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php index 407db02f9f..6de69f1d03 100644 --- a/src/Symfony/Component/Security/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php +++ b/src/Symfony/Component/Security/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php @@ -179,7 +179,7 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase public function testLoginSuccessIgnoresTokensWhichDoNotContainAnUserInterfaceImplementation() { - $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => true)); + $service = $this->getService(null, array('name' => 'foo', 'always_remember_me' => true, 'path' => null, 'domain' => null)); $request = new Request; $response = new Response; $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');