feature #28447 [HttpFoundation] make cookies auto-secure when passing…

… them $secure=null + plan to make it and samesite=lax the defaults in 5.0 (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

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

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #26731
| License       | MIT
| Doc PR        | -

By creating Cookie instances using `null` for the `$secure` argument, this PR allows making cookies inherit their "secure" attribute from the request.

This PR also adds a forward to make $secure=null and samesite=lax the defaults in Symfony 5.0:
- either define all constructor's arguments explicitly
- or use the new `Cookie::create()` factory

Commits
-------

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

UPGRADE-4.2.md

@@ -75,6 +75,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.
+
 FrameworkBundle
 ---------------
 

UPGRADE-5.0.md

@@ -123,6 +123,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
 -------

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -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'])) {

src/Symfony/Component/HttpFoundation/CHANGELOG.md

@@ -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
 -----

src/Symfony/Component/HttpFoundation/Cookie.php

@@ -27,6 +27,7 @@ class Cookie
     protected $httpOnly;
     private $raw;
     private $sameSite;
+    private $secureDefault = false;
 
     const SAMESITE_LAX = 'lax';
     const SAMESITE_STRICT = 'strict';
@@ -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));
@@ -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);
         }
 
@@ -232,7 +244,7 @@ public function getPath()
      */
     public function isSecure()
     {
-        return $this->secure;
+        return $this->secure ?? $this->secureDefault;
     }
 
     /**
@@ -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;
+    }
 }

src/Symfony/Component/HttpFoundation/Response.php

@@ -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;
     }
 

src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php

@@ -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));
     }
 
     /**

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/AbstractSessionHandler.php

@@ -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);
                 }
             }
         }

src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php

@@ -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));
             }
         }
 

src/Symfony/Component/HttpFoundation/Tests/CookieTest.php

@@ -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()
@@ -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());
     }
 }