summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFabien Potencier <fabien.potencier@gmail.com>2015-10-05 16:40:32 +0200
committerFabien Potencier <fabien.potencier@gmail.com>2015-10-05 16:40:32 +0200
commit545528ff1e7a50a50b8e91f27bd667d66a140b14 (patch)
tree04ad1c0b4e70ae470b055d9f06bc88fc49a3b1bb
parentdc6bf51f8c3febd6a5fa0708e2a020d98daca79d (diff)
parent135b1b5bb942c97ec1f1d5e811063a7be3cae35e (diff)
downloadsymfony-security-545528ff1e7a50a50b8e91f27bd667d66a140b14.zip
symfony-security-545528ff1e7a50a50b8e91f27bd667d66a140b14.tar.gz
symfony-security-545528ff1e7a50a50b8e91f27bd667d66a140b14.tar.bz2
bug #14842 [Security][bugfix] "Remember me" cookie cleared on logout with custom "secure"/"httponly" config options [1] (MacDada)
This PR was squashed before being merged into the 2.3 branch (closes #14842). Discussion ---------- [Security][bugfix] "Remember me" cookie cleared on logout with custom "secure"/"httponly" config options [1] | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #14822 | License | MIT | Doc PR | ~ * test now always pass "secure" and "httponly" options, as they are required * could be considered BC, but [`RememberMeFactory` passes them](https://github.com/symfony/symfony/blob/2.3/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php#L21), so they should've always been treated as required * I can squash the commits before merging * Alternative solution: #14843 Commits ------- 18b1c6a [Security][bugfix] "Remember me" cookie cleared on logout with custom "secure"/"httponly" config options [1]
-rw-r--r--Http/RememberMe/AbstractRememberMeServices.php2
-rw-r--r--Tests/Http/RememberMe/AbstractRememberMeServicesTest.php32
-rw-r--r--Tests/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php11
-rw-r--r--Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php11
4 files changed, 50 insertions, 6 deletions
diff --git a/Http/RememberMe/AbstractRememberMeServices.php b/Http/RememberMe/AbstractRememberMeServices.php
index 1ba2df6..51eddb6 100644
--- a/Http/RememberMe/AbstractRememberMeServices.php
+++ b/Http/RememberMe/AbstractRememberMeServices.php
@@ -293,7 +293,7 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface
$this->logger->debug(sprintf('Clearing remember-me cookie "%s"', $this->options['name']));
}
- $request->attributes->set(self::COOKIE_ATTR_NAME, new Cookie($this->options['name'], null, 1, $this->options['path'], $this->options['domain']));
+ $request->attributes->set(self::COOKIE_ATTR_NAME, new Cookie($this->options['name'], null, 1, $this->options['path'], $this->options['domain'], $this->options['secure'], $this->options['httponly']));
}
/**
diff --git a/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php b/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php
index 70ff6a0..9dbcf3f 100644
--- a/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php
+++ b/Tests/Http/RememberMe/AbstractRememberMeServicesTest.php
@@ -82,16 +82,35 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$this->assertSame('fookey', $returnedToken->getProviderKey());
}
- public function testLogout()
+ /**
+ * @dataProvider provideOptionsForLogout
+ */
+ public function testLogout(array $options)
{
- $service = $this->getService(null, array('name' => 'foo', 'path' => null, 'domain' => null));
+ $service = $this->getService(null, $options);
$request = new Request();
$response = new Response();
$token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
$service->logout($request, $response, $token);
- $this->assertTrue($request->attributes->get(RememberMeServicesInterface::COOKIE_ATTR_NAME)->isCleared());
+ $cookie = $request->attributes->get(RememberMeServicesInterface::COOKIE_ATTR_NAME);
+
+ $this->assertInstanceOf('Symfony\Component\HttpFoundation\Cookie', $cookie);
+ $this->assertTrue($cookie->isCleared());
+ $this->assertSame($options['name'], $cookie->getName());
+ $this->assertSame($options['path'], $cookie->getPath());
+ $this->assertSame($options['domain'], $cookie->getDomain());
+ $this->assertSame($options['secure'], $cookie->isSecure());
+ $this->assertSame($options['httponly'], $cookie->isHttpOnly());
+ }
+
+ public function provideOptionsForLogout()
+ {
+ return array(
+ array(array('name' => 'foo', 'path' => '/', 'domain' => null, 'secure' => false, 'httponly' => true)),
+ array(array('name' => 'foo', 'path' => '/bar', 'domain' => 'baz.com', 'secure' => true, 'httponly' => false)),
+ );
}
public function testLoginFail()
@@ -267,6 +286,13 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$userProvider = $this->getProvider();
}
+ if (!isset($options['secure'])) {
+ $options['secure'] = false;
+ }
+ if (!isset($options['httponly'])) {
+ $options['httponly'] = true;
+ }
+
return $this->getMockForAbstractClass('Symfony\Component\Security\Http\RememberMe\AbstractRememberMeServices', array(
array($userProvider), 'fookey', 'fookey', $options, $logger,
));
diff --git a/Tests/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php b/Tests/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php
index 591828f..fe64abc 100644
--- a/Tests/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php
+++ b/Tests/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php
@@ -180,7 +180,7 @@ class PersistentTokenBasedRememberMeServicesTest extends \PHPUnit_Framework_Test
public function testLogout()
{
- $service = $this->getService(null, array('name' => 'foo', 'path' => '/foo', 'domain' => 'foodomain.foo'));
+ $service = $this->getService(null, array('name' => 'foo', 'path' => '/foo', 'domain' => 'foodomain.foo', 'secure' => true, 'httponly' => false));
$request = new Request();
$request->cookies->set('foo', $this->encodeCookie(array('fooseries', 'foovalue')));
$response = new Response();
@@ -201,6 +201,8 @@ class PersistentTokenBasedRememberMeServicesTest extends \PHPUnit_Framework_Test
$this->assertTrue($cookie->isCleared());
$this->assertEquals('/foo', $cookie->getPath());
$this->assertEquals('foodomain.foo', $cookie->getDomain());
+ $this->assertTrue($cookie->isSecure());
+ $this->assertFalse($cookie->isHttpOnly());
}
public function testLogoutSimplyIgnoresNonSetRequestCookie()
@@ -311,6 +313,13 @@ class PersistentTokenBasedRememberMeServicesTest extends \PHPUnit_Framework_Test
$userProvider = $this->getProvider();
}
+ if (!isset($options['secure'])) {
+ $options['secure'] = false;
+ }
+ if (!isset($options['httponly'])) {
+ $options['httponly'] = true;
+ }
+
return new PersistentTokenBasedRememberMeServices(array($userProvider), 'fookey', 'fookey', $options, $logger, new SecureRandom(sys_get_temp_dir().'/_sf2.seed'));
}
diff --git a/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php b/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php
index 511ddcc..929680d 100644
--- a/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php
+++ b/Tests/Http/RememberMe/TokenBasedRememberMeServicesTest.php
@@ -153,7 +153,7 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase
public function testLogout()
{
- $service = $this->getService(null, array('name' => 'foo', 'path' => null, 'domain' => null));
+ $service = $this->getService(null, array('name' => 'foo', 'path' => null, 'domain' => null, 'secure' => true, 'httponly' => false));
$request = new Request();
$response = new Response();
$token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
@@ -164,6 +164,8 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$this->assertTrue($cookie->isCleared());
$this->assertEquals('/', $cookie->getPath());
$this->assertNull($cookie->getDomain());
+ $this->assertTrue($cookie->isSecure());
+ $this->assertFalse($cookie->isHttpOnly());
}
public function testLoginFail()
@@ -264,6 +266,13 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$userProvider = $this->getProvider();
}
+ if (!isset($options['secure'])) {
+ $options['secure'] = false;
+ }
+ if (!isset($options['httponly'])) {
+ $options['httponly'] = true;
+ }
+
$service = new TokenBasedRememberMeServices(array($userProvider), 'fookey', 'fookey', $options, $logger);
return $service;