Skip to content

[HttpFoundation] make cookies auto-secure when passing them $secure=null + plan to make it and samesite=lax the defaults in 5.0 #28447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions UPGRADE-4.2.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ Form
{% endfor %}
```

HttpFoundation
--------------

* The default value of the "$secure" and "$samesite" arguments of Cookie's constructor
will respectively change from "false" to "null" and from "null" to "lax" in Symfony
5.0, you should define their values explicitly or use "Cookie::create()" instead.

Process
-------

Expand Down
2 changes: 2 additions & 0 deletions UPGRADE-5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ HttpFoundation
* The `$size` argument of the `UploadedFile` constructor has been removed.
* The `getClientSize()` method of the `UploadedFile` class has been removed.
* The `getSession()` method of the `Request` class throws an exception when session is null.
* The default value of the "$secure" and "$samesite" arguments of Cookie's constructor
changed respectively from "false" to "null" and from "null" to "lax".

Monolog
-------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ public function load(array $configs, ContainerBuilder $container)
if ($this->isConfigEnabled($container, $config['session'])) {
$this->sessionConfigEnabled = true;
$this->registerSessionConfiguration($config['session'], $container, $loader);
if (!empty($config['test'])) {
$container->getDefinition('test.session.listener')->setArgument(1, '%session.storage.options%');
}
}

if ($this->isConfigEnabled($container, $config['request'])) {
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ CHANGELOG
-----

* added `getAcceptableFormats()` for reading acceptable formats based on Accept header
* the default value of the "$secure" and "$samesite" arguments of Cookie's constructor
will respectively change from "false" to "null" and from "null" to "lax" in Symfony
5.0, you should define their values explicitly or use "Cookie::create()" instead.

4.1.3
-----
Expand Down
28 changes: 24 additions & 4 deletions src/Symfony/Component/HttpFoundation/Cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class Cookie
protected $httpOnly;
private $raw;
private $sameSite;
private $secureDefault = false;

const SAMESITE_LAX = 'lax';
const SAMESITE_STRICT = 'strict';
Expand Down Expand Up @@ -66,21 +67,30 @@ public static function fromString($cookie, $decode = false)
return new static($name, $value, $data['expires'], $data['path'], $data['domain'], $data['secure'], $data['httponly'], $data['raw'], $data['samesite']);
}

public static function create(string $name, string $value = null, $expire = 0, ?string $path = '/', string $domain = null, bool $secure = null, bool $httpOnly = true, bool $raw = false, ?string $sameSite = self::SAMESITE_LAX): self
{
return new self($name, $value, $expire, $path, $domain, $secure, $httpOnly, $raw, $sameSite);
}

/**
* @param string $name The name of the cookie
* @param string|null $value The value of the cookie
* @param int|string|\DateTimeInterface $expire The time the cookie expires
* @param string $path The path on the server in which the cookie will be available on
* @param string|null $domain The domain that the cookie is available to
* @param bool $secure Whether the cookie should only be transmitted over a secure HTTPS connection from the client
* @param bool|null $secure Whether the client should send back the cookie only over HTTPS or null to auto-enable this when the request is already using HTTPS
* @param bool $httpOnly Whether the cookie will be made accessible only through the HTTP protocol
* @param bool $raw Whether the cookie value should be sent with no url encoding
* @param string|null $sameSite Whether the cookie will be available for cross-site requests
*
* @throws \InvalidArgumentException
*/
public function __construct(string $name, string $value = null, $expire = 0, ?string $path = '/', string $domain = null, bool $secure = false, bool $httpOnly = true, bool $raw = false, string $sameSite = null)
public function __construct(string $name, string $value = null, $expire = 0, ?string $path = '/', string $domain = null, ?bool $secure = false, bool $httpOnly = true, bool $raw = false, string $sameSite = null)
{
if (9 > \func_num_args()) {
@trigger_error(sprintf('The default value of the "$secure" and "$samesite" arguments of "%s"\'s constructor will respectively change from "false" to "null" and from "null" to "lax" in Symfony 5.0, you should define their values explicitly or use "Cookie::create()" instead.', __METHOD__), E_USER_DEPRECATED);
}

// from PHP source code
if (preg_match("/[=,; \t\r\n\013\014]/", $name)) {
throw new \InvalidArgumentException(sprintf('The cookie name "%s" contains invalid characters.', $name));
Expand Down Expand Up @@ -110,7 +120,9 @@ public function __construct(string $name, string $value = null, $expire = 0, ?st
$this->httpOnly = $httpOnly;
$this->raw = $raw;

if (null !== $sameSite) {
if ('' === $sameSite) {
$sameSite = null;
} elseif (null !== $sameSite) {
$sameSite = strtolower($sameSite);
}

Expand Down Expand Up @@ -232,7 +244,7 @@ public function getPath()
*/
public function isSecure()
{
return $this->secure;
return $this->secure ?? $this->secureDefault;
}

/**
Expand Down Expand Up @@ -274,4 +286,12 @@ public function getSameSite()
{
return $this->sameSite;
}

/**
* @param bool $default The default value of the "secure" flag when it is set to null
*/
public function setSecureDefault(bool $default): void
{
$this->secureDefault = $default;
}
}
6 changes: 6 additions & 0 deletions src/Symfony/Component/HttpFoundation/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,12 @@ public function prepare(Request $request)

$this->ensureIEOverSSLCompatibility($request);

if ($request->isSecure()) {
foreach ($headers->getCookies() as $cookie) {
$cookie->setSecureDefault(true);
}
}

return $this;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public function getCookies($format = self::COOKIES_FLAT)
*/
public function clearCookie($name, $path = '/', $domain = null, $secure = false, $httpOnly = true)
{
$this->setCookie(new Cookie($name, null, 1, $path, $domain, $secure, $httpOnly));
$this->setCookie(new Cookie($name, null, 1, $path, $domain, $secure, $httpOnly, false, null));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ public function destroy($sessionId)
if (\PHP_VERSION_ID < 70300) {
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure'), ini_get('session.cookie_httponly'));
} else {
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure'), ini_get('session.cookie_httponly'), ini_get('session.cookie_samesite'));
$params = session_get_cookie_params();
unset($params['lifetime']);
setcookie($this->sessionName, '', $params);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public function start()
if (null !== $this->emulateSameSite) {
$originalCookie = SessionUtils::popSessionCookie(session_name(), session_id());
if (null !== $originalCookie) {
header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite));
header(sprintf('%s; samesite=%s', $originalCookie, $this->emulateSameSite));
}
}

Expand Down
84 changes: 51 additions & 33 deletions src/Symfony/Component/HttpFoundation/Tests/CookieTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,177 +45,177 @@ public function invalidNames()
*/
public function testInstantiationThrowsExceptionIfCookieNameContainsInvalidCharacters($name)
{
new Cookie($name);
Cookie::create($name);
}

/**
* @expectedException \InvalidArgumentException
*/
public function testInvalidExpiration()
{
new Cookie('MyCookie', 'foo', 'bar');
Cookie::create('MyCookie', 'foo', 'bar');
}

public function testNegativeExpirationIsNotPossible()
{
$cookie = new Cookie('foo', 'bar', -100);
$cookie = Cookie::create('foo', 'bar', -100);

$this->assertSame(0, $cookie->getExpiresTime());
}

public function testGetValue()
{
$value = 'MyValue';
$cookie = new Cookie('MyCookie', $value);
$cookie = Cookie::create('MyCookie', $value);

$this->assertSame($value, $cookie->getValue(), '->getValue() returns the proper value');
}

public function testGetPath()
{
$cookie = new Cookie('foo', 'bar');
$cookie = Cookie::create('foo', 'bar');

$this->assertSame('/', $cookie->getPath(), '->getPath() returns / as the default path');
}

public function testGetExpiresTime()
{
$cookie = new Cookie('foo', 'bar');
$cookie = Cookie::create('foo', 'bar');

$this->assertEquals(0, $cookie->getExpiresTime(), '->getExpiresTime() returns the default expire date');

$cookie = new Cookie('foo', 'bar', $expire = time() + 3600);
$cookie = Cookie::create('foo', 'bar', $expire = time() + 3600);

$this->assertEquals($expire, $cookie->getExpiresTime(), '->getExpiresTime() returns the expire date');
}

public function testGetExpiresTimeIsCastToInt()
{
$cookie = new Cookie('foo', 'bar', 3600.9);
$cookie = Cookie::create('foo', 'bar', 3600.9);

$this->assertSame(3600, $cookie->getExpiresTime(), '->getExpiresTime() returns the expire date as an integer');
}

public function testConstructorWithDateTime()
{
$expire = new \DateTime();
$cookie = new Cookie('foo', 'bar', $expire);
$cookie = Cookie::create('foo', 'bar', $expire);

$this->assertEquals($expire->format('U'), $cookie->getExpiresTime(), '->getExpiresTime() returns the expire date');
}

public function testConstructorWithDateTimeImmutable()
{
$expire = new \DateTimeImmutable();
$cookie = new Cookie('foo', 'bar', $expire);
$cookie = Cookie::create('foo', 'bar', $expire);

$this->assertEquals($expire->format('U'), $cookie->getExpiresTime(), '->getExpiresTime() returns the expire date');
}

public function testGetExpiresTimeWithStringValue()
{
$value = '+1 day';
$cookie = new Cookie('foo', 'bar', $value);
$cookie = Cookie::create('foo', 'bar', $value);
$expire = strtotime($value);

$this->assertEquals($expire, $cookie->getExpiresTime(), '->getExpiresTime() returns the expire date', 1);
}

public function testGetDomain()
{
$cookie = new Cookie('foo', 'bar', 0, '/', '.myfoodomain.com');
$cookie = Cookie::create('foo', 'bar', 0, '/', '.myfoodomain.com');

$this->assertEquals('.myfoodomain.com', $cookie->getDomain(), '->getDomain() returns the domain name on which the cookie is valid');
}

public function testIsSecure()
{
$cookie = new Cookie('foo', 'bar', 0, '/', '.myfoodomain.com', true);
$cookie = Cookie::create('foo', 'bar', 0, '/', '.myfoodomain.com', true);

$this->assertTrue($cookie->isSecure(), '->isSecure() returns whether the cookie is transmitted over HTTPS');
}

public function testIsHttpOnly()
{
$cookie = new Cookie('foo', 'bar', 0, '/', '.myfoodomain.com', false, true);
$cookie = Cookie::create('foo', 'bar', 0, '/', '.myfoodomain.com', false, true);

$this->assertTrue($cookie->isHttpOnly(), '->isHttpOnly() returns whether the cookie is only transmitted over HTTP');
}

public function testCookieIsNotCleared()
{
$cookie = new Cookie('foo', 'bar', time() + 3600 * 24);
$cookie = Cookie::create('foo', 'bar', time() + 3600 * 24);

$this->assertFalse($cookie->isCleared(), '->isCleared() returns false if the cookie did not expire yet');
}

public function testCookieIsCleared()
{
$cookie = new Cookie('foo', 'bar', time() - 20);
$cookie = Cookie::create('foo', 'bar', time() - 20);

$this->assertTrue($cookie->isCleared(), '->isCleared() returns true if the cookie has expired');

$cookie = new Cookie('foo', 'bar');
$cookie = Cookie::create('foo', 'bar');

$this->assertFalse($cookie->isCleared());

$cookie = new Cookie('foo', 'bar', 0);
$cookie = Cookie::create('foo', 'bar');

$this->assertFalse($cookie->isCleared());

$cookie = new Cookie('foo', 'bar', -1);
$cookie = Cookie::create('foo', 'bar', -1);

$this->assertFalse($cookie->isCleared());
}

public function testToString()
{
$cookie = new Cookie('foo', 'bar', $expire = strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true);
$cookie = Cookie::create('foo', 'bar', $expire = strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true, true, false, null);
$this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; Max-Age=0; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() returns string representation of the cookie');

$cookie = new Cookie('foo', 'bar with white spaces', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true);
$cookie = Cookie::create('foo', 'bar with white spaces', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true, true, false, null);
$this->assertEquals('foo=bar%20with%20white%20spaces; expires=Fri, 20-May-2011 15:25:52 GMT; Max-Age=0; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() encodes the value of the cookie according to RFC 3986 (white space = %20)');

$cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com');
$cookie = Cookie::create('foo', null, 1, '/admin/', '.myfoodomain.com', false, true, false, null);
$this->assertEquals('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', $expire = time() - 31536001).'; Max-Age=0; path=/admin/; domain=.myfoodomain.com; httponly', (string) $cookie, '->__toString() returns string representation of a cleared cookie if value is NULL');

$cookie = new Cookie('foo', 'bar', 0, '/', '');
$this->assertEquals('foo=bar; path=/; httponly', (string) $cookie);
$cookie = Cookie::create('foo', 'bar');
$this->assertEquals('foo=bar; path=/; httponly; samesite=lax', (string) $cookie);
}

public function testRawCookie()
{
$cookie = new Cookie('foo', 'b a r', 0, '/', null, false, false);
$cookie = Cookie::create('foo', 'b a r', 0, '/', null, false, false, false, null);
$this->assertFalse($cookie->isRaw());
$this->assertEquals('foo=b%20a%20r; path=/', (string) $cookie);

$cookie = new Cookie('foo', 'b+a+r', 0, '/', null, false, false, true);
$cookie = Cookie::create('foo', 'b+a+r', 0, '/', null, false, false, true, null);
$this->assertTrue($cookie->isRaw());
$this->assertEquals('foo=b+a+r; path=/', (string) $cookie);
}

public function testGetMaxAge()
{
$cookie = new Cookie('foo', 'bar');
$cookie = Cookie::create('foo', 'bar');
$this->assertEquals(0, $cookie->getMaxAge());

$cookie = new Cookie('foo', 'bar', $expire = time() + 100);
$cookie = Cookie::create('foo', 'bar', $expire = time() + 100);
$this->assertEquals($expire - time(), $cookie->getMaxAge());

$cookie = new Cookie('foo', 'bar', $expire = time() - 100);
$cookie = Cookie::create('foo', 'bar', $expire = time() - 100);
$this->assertEquals(0, $cookie->getMaxAge());
}

public function testFromString()
{
$cookie = Cookie::fromString('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; path=/; domain=.myfoodomain.com; secure; httponly');
$this->assertEquals(new Cookie('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true, true, true), $cookie);
$this->assertEquals(Cookie::create('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true, true, true, null), $cookie);

$cookie = Cookie::fromString('foo=bar', true);
$this->assertEquals(new Cookie('foo', 'bar', 0, '/', null, false, false), $cookie);
$this->assertEquals(Cookie::create('foo', 'bar', 0, '/', null, false, false, false, null), $cookie);

$cookie = Cookie::fromString('foo', true);
$this->assertEquals(new Cookie('foo', null, 0, '/', null, false, false), $cookie);
$this->assertEquals(Cookie::create('foo', null, 0, '/', null, false, false, false, null), $cookie);
}

public function testFromStringWithHttpOnly()
Expand All @@ -227,9 +227,27 @@ public function testFromStringWithHttpOnly()
$this->assertFalse($cookie->isHttpOnly());
}

public function testSameSiteAttributeIsCaseInsensitive()
public function testSameSiteAttribute()
{
$cookie = new Cookie('foo', 'bar', 0, '/', null, false, true, false, 'Lax');
$this->assertEquals('lax', $cookie->getSameSite());

$cookie = new Cookie('foo', 'bar', 0, '/', null, false, true, false, '');
$this->assertNull($cookie->getSameSite());
}

public function testSetSecureDefault()
{
$cookie = Cookie::create('foo', 'bar');

$this->assertFalse($cookie->isSecure());

$cookie->setSecureDefault(true);

$this->assertTrue($cookie->isSecure());

$cookie->setSecureDefault(false);

$this->assertFalse($cookie->isSecure());
}
}
Loading