merged branch TerjeBr/persistent-token-provider (PR #6055)
This PR was merged into the master branch.
Commits
-------
d1b5093
Try to make sure cookies get deleted from the TokenProvider when no longer in use
Discussion
----------
Delete cookies from the TokenProvider that is no longer in use
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Todo: -
License of the code: MIT
When the user logs in, or login fails for some reason, the old "remember me" cookie should be deleted from the TokenProvider if you are using the PersistentTokenBasedRememberMeServices.
As the code is now, the token is only deleted on logout.
---------------------------------------------------------------------------
by TerjeBr at 2012-11-20T13:45:54Z
So, anything else that needs to be done before this is merged?
---------------------------------------------------------------------------
by TerjeBr at 2012-11-21T10:30:53Z
Ok, I have corrected the typo in the comment and squashed the commit.
---------------------------------------------------------------------------
by schmittjoh at 2012-11-21T10:36:29Z
btw, ``canceled`` (more American) and ``cancelled`` (more British) are both
correct English forms.
On Wed, Nov 21, 2012 at 11:30 AM, Terje Bråten <notifications@github.com>wrote:
> Ok, I have corrected the typo in the comment and squashed the commit.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/6055#issuecomment-10592112>.
>
>
---------------------------------------------------------------------------
by schmittjoh at 2012-11-21T10:40:24Z
As a side-note have you verified that this does not break the cookie theft protection?
---------------------------------------------------------------------------
by TerjeBr at 2012-11-21T10:51:10Z
Yes, cookie theft protection is still there and is functioning well.
---------------------------------------------------------------------------
by TerjeBr at 2012-11-21T11:14:04Z
I am using this together with the DoctrineTokenProvider in issue #6057 in my own project and done some extensive testing on it.
---------------------------------------------------------------------------
by TerjeBr at 2012-11-23T10:30:34Z
Is this ready to be merged now?
This commit is contained in:
commit
d5ff2388cb
@ -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.');
|
||||
|
@ -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'],
|
||||
|
@ -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);
|
||||
|
@ -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');
|
||||
|
Reference in New Issue
Block a user