fix(Session): send SameSite attribute in BaseHandler::destroyCookie()

docs/changelogs/v4.7.7.md

@@ -19,6 +19,7 @@ Release Date: Unreleased
 - **CORS:** Fixed a bug where `Cors::hasResponseHeaders()` matched header substrings via `str_contains()`, causing false positives when existing exposed headers shared a prefix with configured exposed headers.
 - **HTTP:** Fixed a bug where `UploadedFile::move()` and `UploadedFile::store()` on Windows could store a file under a different name than `getName()` reported, or write its contents to an NTFS alternate data stream. Filenames ending in a period or space or containing a colon are now rejected, including explicitly supplied names.
 - **Security:** Fixed a bug where `Security::__construct()` threw a `TypeError` when the CSRF cookie was an array, causing an HTTP 500 response for affected requests.
+- **Session:** Fixed a bug where `BaseHandler::destroyCookie()` did not send the `SameSite` attribute in the `Set-Cookie` header, which could prevent browsers from properly clearing the session cookie.
 - **Worker Mode:** Fixed a bug where every request that ended with an uncaught exception, including a `404 Page Not Found` without a 404 override, called `exit()` in the exception handler. This ended the worker script, so FrankenPHP booted a new one, and the worker loop skipped its cleanup after the request. The exception handler now returns to the worker loop.
 
 See the repo's [CHANGELOG.md](https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md) for a complete list of bugs fixed.

system/Session/Handlers/BaseHandler.php

@@ -13,6 +13,7 @@
 
 namespace CodeIgniter\Session\Handlers;
 
+use CodeIgniter\Cookie\Cookie;
 use Config\Cookie as CookieConfig;
 use Config\Session as SessionConfig;
 use Psr\Log\LoggerAwareTrait;
@@ -70,6 +71,13 @@ abstract class BaseHandler implements SessionHandlerInterface
      */
     protected $cookieSecure = false;
 
+    /**
+     * Cookie SameSite setting.
+     *
+     * @var ''|'Lax'|'None'|'Strict'
+     */
+    protected string $cookieSameSite = '';
+
     /**
      * Cookie name to use.
      *
@@ -116,9 +124,10 @@ public function __construct(SessionConfig $config, string $ipAddress)
         $cookie = config(CookieConfig::class);
 
         // Session cookies have no prefix.
-        $this->cookieDomain = $cookie->domain;
-        $this->cookiePath   = $cookie->path;
-        $this->cookieSecure = $cookie->secure;
+        $this->cookieDomain   = $cookie->domain;
+        $this->cookiePath     = $cookie->path;
+        $this->cookieSecure   = $cookie->secure;
+        $this->cookieSameSite = $cookie->samesite;
 
         $this->ipAddress = $ipAddress;
     }
@@ -129,10 +138,21 @@ public function __construct(SessionConfig $config, string $ipAddress)
      */
     protected function destroyCookie(): bool
     {
+        $samesite = $this->cookieSameSite === ''
+            ? Cookie::SAMESITE_LAX
+            : ucfirst(strtolower($this->cookieSameSite));
+
         return setcookie(
             $this->cookieName,
             '',
-            ['expires' => 1, 'path' => $this->cookiePath, 'domain' => $this->cookieDomain, 'secure' => $this->cookieSecure, 'httponly' => true],
+            [
+                'expires'  => 1,
+                'path'     => $this->cookiePath,
+                'domain'   => $this->cookieDomain,
+                'secure'   => $this->cookieSecure,
+                'httponly' => true,
+                'samesite' => $samesite,
+            ],
         );
     }
 

tests/system/Session/Handlers/BaseHandlerTest.php

@@ -0,0 +1,198 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <admin@codeigniter.com>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace CodeIgniter\Session\Handlers;
+
+use CodeIgniter\Config\Factories;
+use CodeIgniter\Test\CIUnitTestCase;
+use Config\Cookie as CookieConfig;
+use Config\Session as SessionConfig;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Group;
+
+/**
+ * Mock setcookie function in CodeIgniter\Session\Handlers namespace.
+ *
+ * @param array<string, mixed> $options
+ */
+function setcookie(string $name, string $value = '', array $options = []): bool
+{
+    BaseHandlerTest::$lastSetCookieArgs = [
+        'name'    => $name,
+        'value'   => $value,
+        'options' => $options,
+    ];
+
+    return true;
+}
+
+/**
+ * @internal
+ */
+#[Group('Others')]
+final class BaseHandlerTest extends CIUnitTestCase
+{
+    /**
+     * @var array{name: string, value: string, options: array<string, mixed>}|null
+     */
+    public static ?array $lastSetCookieArgs = null;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        self::$lastSetCookieArgs = null;
+        Factories::reset('config');
+    }
+
+    protected function tearDown(): void
+    {
+        parent::tearDown();
+
+        self::$lastSetCookieArgs = null;
+        Factories::reset('config');
+    }
+
+    private function createHandler(?CookieConfig $cookieConfig = null): BaseHandler
+    {
+        if ($cookieConfig instanceof CookieConfig) {
+            Factories::injectMock('config', CookieConfig::class, $cookieConfig);
+        }
+
+        $sessionConfig             = new SessionConfig();
+        $sessionConfig->cookieName = 'ci_session';
+
+        return new class ($sessionConfig, '127.0.0.1') extends BaseHandler {
+            public function open(string $path, string $name): bool
+            {
+                return true;
+            }
+
+            public function close(): bool
+            {
+                return true;
+            }
+
+            public function read(string $id): string
+            {
+                return '';
+            }
+
+            public function write(string $id, string $data): bool
+            {
+                return true;
+            }
+
+            public function destroy(string $id): bool
+            {
+                return $this->destroyCookie();
+            }
+
+            public function gc(int $maxLifetime): int
+            {
+                return 1;
+            }
+        };
+    }
+
+    public function testConstructorInitializesCookieSameSiteFromConfig(): void
+    {
+        $cookieConfig           = new CookieConfig();
+        $cookieConfig->samesite = 'Strict';
+
+        $handler = $this->createHandler($cookieConfig);
+
+        $this->assertSame('Strict', $this->getPrivateProperty($handler, 'cookieSameSite'));
+    }
+
+    public function testConstructorDefaultsToCookieConfigSameSite(): void
+    {
+        $handler = $this->createHandler();
+
+        $this->assertSame('Lax', $this->getPrivateProperty($handler, 'cookieSameSite'));
+    }
+
+    public function testDestroyCookiePassesSameSiteInOptions(): void
+    {
+        $cookieConfig           = new CookieConfig();
+        $cookieConfig->domain   = 'example.com';
+        $cookieConfig->path     = '/app';
+        $cookieConfig->secure   = true;
+        $cookieConfig->samesite = 'Strict';
+
+        $handler = $this->createHandler($cookieConfig);
+        $result  = $handler->destroy('ci_session');
+
+        $this->assertTrue($result);
+        $this->assertNotNull(self::$lastSetCookieArgs);
+        $this->assertSame('ci_session', self::$lastSetCookieArgs['name']);
+        $this->assertSame('', self::$lastSetCookieArgs['value']);
+
+        $options = self::$lastSetCookieArgs['options'];
+        $this->assertSame(1, $options['expires']);
+        $this->assertSame('/app', $options['path']);
+        $this->assertSame('example.com', $options['domain']);
+        $this->assertTrue($options['secure']);
+        $this->assertTrue($options['httponly']);
+        $this->assertSame('Strict', $options['samesite']);
+    }
+
+    public function testDestroyCookieNormalizesEmptySameSiteToLax(): void
+    {
+        $cookieConfig           = new CookieConfig();
+        $cookieConfig->samesite = '';
+
+        $handler = $this->createHandler($cookieConfig);
+        $handler->destroy('ci_session');
+
+        $this->assertNotNull(self::$lastSetCookieArgs);
+        $this->assertSame('Lax', self::$lastSetCookieArgs['options']['samesite']);
+    }
+
+    public function testDestroyCookieNormalizesLowercaseSameSite(): void
+    {
+        $handler = $this->createHandler();
+        $this->setPrivateProperty($handler, 'cookieSameSite', 'none');
+        $handler->destroy('ci_session');
+
+        $this->assertNotNull(self::$lastSetCookieArgs);
+        $this->assertSame('None', self::$lastSetCookieArgs['options']['samesite']);
+    }
+
+    #[DataProvider('provideDestroyCookieNormalizesSameSiteCase')]
+    public function testDestroyCookieNormalizesSameSiteCase(string $input, string $expected): void
+    {
+        $handler = $this->createHandler();
+        $this->setPrivateProperty($handler, 'cookieSameSite', $input);
+        $handler->destroy('ci_session');
+
+        $this->assertNotNull(self::$lastSetCookieArgs);
+        $this->assertSame($expected, self::$lastSetCookieArgs['options']['samesite']);
+    }
+
+    /**
+     * @return iterable<string, array{string, string}>
+     */
+    public static function provideDestroyCookieNormalizesSameSiteCase(): iterable
+    {
+        yield 'uppercase LAX' => ['LAX', 'Lax'];
+
+        yield 'uppercase STRICT' => ['STRICT', 'Strict'];
+
+        yield 'uppercase NONE' => ['NONE', 'None'];
+
+        yield 'mixed case sTrIcT' => ['sTrIcT', 'Strict'];
+
+        yield 'empty string' => ['', 'Lax'];
+    }
+}