From 23e12d9168e8d17fcc6f910470589c6e1bb8a4ff Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Wed, 20 May 2026 02:23:45 +0200 Subject: [PATCH 1/5] feat(security): add Fetch Metadata CSRF protection - Add Fetch Metadata-first CSRF verification with token fallback. - Keep upgraded apps token-only when the new config is missing. - Add Vary handling for unsafe Fetch Metadata-protected requests. - Document configuration, fallback behavior, and same-site caveats. - Cover same-origin, same-site, cross-site, fallback, and filter behavior. Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- app/Config/Security.php | 19 +++ system/Filters/CSRF.php | 22 ++- system/Security/Security.php | 64 +++++++- tests/system/Filters/CSRFTest.php | 91 +++++++++++ tests/system/Security/SecurityTest.php | 148 ++++++++++++++++++ user_guide_src/source/changelogs/v4.8.0.rst | 1 + .../source/installation/upgrade_480.rst | 2 + user_guide_src/source/libraries/security.rst | 43 +++++ .../source/libraries/security/011.php | 12 ++ .../source/libraries/security/012.php | 12 ++ 10 files changed, 409 insertions(+), 5 deletions(-) create mode 100644 user_guide_src/source/libraries/security/011.php create mode 100644 user_guide_src/source/libraries/security/012.php diff --git a/app/Config/Security.php b/app/Config/Security.php index 635f8b77b9b7..d596174a3c78 100644 --- a/app/Config/Security.php +++ b/app/Config/Security.php @@ -17,6 +17,25 @@ class Security extends BaseConfig */ public string $csrfProtection = 'cookie'; + /** + * -------------------------------------------------------------------------- + * CSRF Fetch Metadata + * -------------------------------------------------------------------------- + * + * Whether to use Fetch Metadata request headers as a first-line CSRF check. + */ + public bool $csrfUseFetchMetadata = true; + + /** + * -------------------------------------------------------------------------- + * CSRF Allow Same Site + * -------------------------------------------------------------------------- + * + * Whether requests with the Sec-Fetch-Site: same-site header should pass + * Fetch Metadata verification. + */ + public bool $csrfAllowSameSite = false; + /** * -------------------------------------------------------------------------- * CSRF Token Randomization diff --git a/system/Filters/CSRF.php b/system/Filters/CSRF.php index ea9a9939f2de..0480d5e24f63 100644 --- a/system/Filters/CSRF.php +++ b/system/Filters/CSRF.php @@ -14,11 +14,13 @@ namespace CodeIgniter\Filters; use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\Method; use CodeIgniter\HTTP\RedirectResponse; use CodeIgniter\HTTP\RequestInterface; use CodeIgniter\HTTP\ResponseInterface; use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Security\Security; +use Config\Security as SecurityConfig; /** * CSRF filter. @@ -52,12 +54,19 @@ public function before(RequestInterface $request, $arguments = null) $security->verify($request); } catch (SecurityException $e) { if ($security->shouldRedirect() && ! $request->isAJAX()) { - return redirect()->back()->with('error', $e->getMessage()); + $response = redirect()->back()->with('error', $e->getMessage()); + $this->addFetchMetadataVaryHeader($request, $response); + + return $response; } + $this->addFetchMetadataVaryHeader($request, service('response')); + throw $e; } + $this->addFetchMetadataVaryHeader($request, service('response')); + return null; } @@ -70,4 +79,15 @@ public function after(RequestInterface $request, ResponseInterface $response, $a { return null; } + + private function addFetchMetadataVaryHeader(IncomingRequest $request, ResponseInterface $response): void + { + $config = get_object_vars(config(SecurityConfig::class)); + $useFetchMetadata = ($config['csrfUseFetchMetadata'] ?? false) === true; + $isUnsafeMethod = in_array($request->getMethod(), [Method::POST, Method::PUT, Method::DELETE, Method::PATCH], true); + + if ($useFetchMetadata && $isUnsafeMethod) { + $response->appendHeader('Vary', 'Sec-Fetch-Site'); + } + } } diff --git a/system/Security/Security.php b/system/Security/Security.php index c1bfb856995a..dddd095a35cb 100644 --- a/system/Security/Security.php +++ b/system/Security/Security.php @@ -35,8 +35,11 @@ */ class Security implements SecurityInterface { - public const CSRF_PROTECTION_COOKIE = 'cookie'; - public const CSRF_PROTECTION_SESSION = 'session'; + public const CSRF_PROTECTION_COOKIE = 'cookie'; + public const CSRF_PROTECTION_SESSION = 'session'; + private const FETCH_METADATA_ALLOW = 'allow'; + private const FETCH_METADATA_FALLBACK = 'fallback'; + private const FETCH_METADATA_REJECT = 'reject'; /** * CSRF hash length in bytes. @@ -118,6 +121,27 @@ public function verify(RequestInterface $request): static assert($request instanceof IncomingRequest); + $decision = $this->fetchMetadataDecision($request); + + if ($decision === self::FETCH_METADATA_ALLOW) { + $this->removeTokenInRequest($request); + + log_message('info', 'CSRF Fetch Metadata verified.'); + + return $this; + } + + if ($decision === self::FETCH_METADATA_REJECT) { + throw SecurityException::forDisallowedAction(); + } + + $this->verifyToken($request); + + return $this; + } + + private function verifyToken(IncomingRequest $request): void + { $postedToken = $this->getPostedToken($request); try { @@ -139,8 +163,6 @@ public function verify(RequestInterface $request): static } log_message('info', 'CSRF token verified.'); - - return $this; } public function getHash(): ?string @@ -229,6 +251,36 @@ private function isCsrfCookie(): bool return $this->config->csrfProtection === self::CSRF_PROTECTION_COOKIE; } + /** + * @return self::FETCH_METADATA_* + */ + private function fetchMetadataDecision(IncomingRequest $request): string + { + $config = get_object_vars($this->config); + + if (($config['csrfUseFetchMetadata'] ?? false) !== true) { + return self::FETCH_METADATA_FALLBACK; + } + + $fetchSite = strtolower($request->getHeaderLine('Sec-Fetch-Site')); + + if ($fetchSite === 'same-origin') { + return self::FETCH_METADATA_ALLOW; + } + + if ($fetchSite === 'cross-site') { + return self::FETCH_METADATA_REJECT; + } + + if ($fetchSite === 'same-site') { + return ($config['csrfAllowSameSite'] ?? false) === true + ? self::FETCH_METADATA_ALLOW + : self::FETCH_METADATA_REJECT; + } + + return self::FETCH_METADATA_FALLBACK; + } + /** * @phpstan-assert SessionInterface $this->session */ @@ -285,6 +337,10 @@ private function removeTokenInRequest(IncomingRequest $request): void // If the token is found in form-encoded data, we can safely remove it. parse_str($body, $result); + if (! array_key_exists($tokenName, $result)) { + return; + } + unset($result[$tokenName]); $request->setBody(http_build_query($result)); } diff --git a/tests/system/Filters/CSRFTest.php b/tests/system/Filters/CSRFTest.php index ce9d3fdfe263..42b3e7bde2a1 100644 --- a/tests/system/Filters/CSRFTest.php +++ b/tests/system/Filters/CSRFTest.php @@ -13,10 +13,14 @@ namespace CodeIgniter\Filters; +use CodeIgniter\Config\Factories; use CodeIgniter\HTTP\CLIRequest; use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\RedirectResponse; use CodeIgniter\HTTP\Response; +use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Test\CIUnitTestCase; +use Config\Security as SecurityConfig; use PHPUnit\Framework\Attributes\BackupGlobals; use PHPUnit\Framework\Attributes\Group; @@ -37,6 +41,14 @@ protected function setUp(): void $this->config = new \Config\Filters(); } + protected function tearDown(): void + { + parent::tearDown(); + + $this->resetServices(); + Factories::reset('config'); + } + public function testDoNotCheckCliRequest(): void { $this->config->globals = [ @@ -73,4 +85,83 @@ public function testPassGetRequest(): void // GET request is not protected, so no SecurityException will be thrown. $this->assertSame($this->request, $request); } + + public function testBeforeAddsVaryHeaderForFetchMetadataVerification(): void + { + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'same-origin'); + + $filter->before($request); + + $this->assertSame('Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + + public function testBeforeAppendsVaryHeaderForFetchMetadataVerification(): void + { + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'same-origin'); + service('response')->setHeader('Vary', 'Accept-Language'); + + $filter->before($request); + + $this->assertSame('Accept-Language, Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + + public function testBeforeAddsVaryHeaderToRedirectResponseForFetchMetadataVerification(): void + { + $config = new SecurityConfig(); + $config->redirect = true; + Factories::injectMock('config', 'Security', $config); + + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'cross-site'); + + $response = $filter->before($request); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('Sec-Fetch-Site', $response->getHeaderLine('Vary')); + } + + public function testBeforeThrowsExceptionForRejectedFetchMetadataVerification(): void + { + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'cross-site'); + + try { + $filter->before($request); + + $this->fail('Expected SecurityException was not thrown.'); + } catch (SecurityException) { + $this->assertSame('Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + } + + public function testBeforeDoesNotAddVaryHeaderForTokenVerification(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', '8b9218a55906f9dcc1dc263dce7f005a') + ->setCookie('csrf_cookie_name', '8b9218a55906f9dcc1dc263dce7f005a'); + + $config = new SecurityConfig(); + $config->csrfUseFetchMetadata = false; + Factories::injectMock('config', 'Security', $config); + + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'same-origin'); + + $filter->before($request); + + $this->assertSame('', service('response')->getHeaderLine('Vary')); + } } diff --git a/tests/system/Security/SecurityTest.php b/tests/system/Security/SecurityTest.php index bc3f13148f2f..f20488d27286 100644 --- a/tests/system/Security/SecurityTest.php +++ b/tests/system/Security/SecurityTest.php @@ -237,6 +237,154 @@ public function testCsrfVerifyHeaderWithJsonBodyStripsTokenFromBody(): void $this->assertSame('{"foo":"bar"}', $request->getBody()); } + public function testCsrfVerifyFetchMetadataSameOriginReturnsSelf(): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-origin'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF Fetch Metadata verified.'); + } + + public function testCsrfVerifyFetchMetadataRemovesTokenButDoesNotRegenerate(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('foo', 'bar') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-origin'); + + $oldHash = $security->getHash(); + $security->verify($request); + $newHash = $security->getHash(); + + $this->assertSame($oldHash, $newHash); + $this->assertSame(['foo' => 'bar'], service('superglobals')->getPostArray()); + } + + public function testCsrfVerifyFetchMetadataPreservesRawBodyWithoutToken(): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest() + ->setHeader('Sec-Fetch-Site', 'same-origin') + ->setBody('unchanged'); + + $security->verify($request); + + $this->assertSame('unchanged', $request->getBody()); + } + + public function testCsrfVerifyFetchMetadataSameSiteThrowsExceptionByDefault(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + + $this->expectException(SecurityException::class); + $security->verify($request); + } + + public function testCsrfVerifyFetchMetadataSameSiteReturnsSelfWhenAllowed(): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + $config = new SecurityConfig(); + $config->csrfAllowSameSite = true; + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + } + + public function testCsrfVerifyFetchMetadataCrossSiteThrowsExceptionEvenWithValidToken(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + + $this->expectException(SecurityException::class); + $security->verify($request); + } + + #[DataProvider('provideCsrfVerifyFetchMetadataFallsBackToToken')] + public function testCsrfVerifyFetchMetadataFallsBackToToken(?string $fetchSite): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest(); + + if ($fetchSite !== null) { + $request->setHeader('Sec-Fetch-Site', $fetchSite); + } + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + + /** + * @return iterable + */ + public static function provideCsrfVerifyFetchMetadataFallsBackToToken(): iterable + { + yield 'missing' => [null]; + + yield 'none' => ['none']; + + yield 'unknown' => ['future-value']; + } + + public function testCsrfVerifyTokenOnlyIgnoresFetchMetadata(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $config = new SecurityConfig(); + $config->csrfUseFetchMetadata = false; + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + + public function testCsrfVerifyMissingFetchMetadataConfigFallsBackToTokenOnly(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $config = new SecurityConfig(); + unset($config->csrfUseFetchMetadata); + + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + public function testCsrfVerifyPutBodyThrowsExceptionOnNoMatch(): void { service('superglobals') diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 7670598cde1f..90d6c9543f2a 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -248,6 +248,7 @@ Libraries - **Locks:** Added :doc:`Atomic Locks ` for owner-aware, cross-process mutual exclusion backed by supported cache handlers: **File**, **Redis**, **Predis**, and **Memcached**. Memcached support has driver-specific release limitations because Memcached has no atomic compare-and-delete command. - **Logging:** Log handlers now receive the full context array as a third argument to ``handle()``. When ``$logGlobalContext`` is enabled, the CI global context is available under the ``HandlerInterface::GLOBAL_CONTEXT_KEY`` key. Built-in handlers append it to the log output; custom handlers can use it for structured logging. - **Logging:** Added :ref:`per-call context logging ` with three new ``Config\Logger`` options (``$logContext``, ``$logContextTrace``, ``$logContextUsedKeys``). Per PSR-3, a ``Throwable`` in the ``exception`` context key is automatically normalized to a meaningful array. All options default to ``false``. +- **Security:** Added :ref:`Fetch Metadata based CSRF protection ` with token fallback. Helpers and Functions ===================== diff --git a/user_guide_src/source/installation/upgrade_480.rst b/user_guide_src/source/installation/upgrade_480.rst index d2738db27158..de5d728c0e2b 100644 --- a/user_guide_src/source/installation/upgrade_480.rst +++ b/user_guide_src/source/installation/upgrade_480.rst @@ -75,6 +75,8 @@ Config - app/Config/Mimes.php - ``Config\Mimes::$mimes`` added a new key ``md`` for Markdown files. +- app/Config/Security.php + - ``Config\Security::$csrfUseFetchMetadata`` and ``Config\Security::$csrfAllowSameSite`` were added for Fetch Metadata based CSRF protection. All Changes =========== diff --git a/user_guide_src/source/libraries/security.rst b/user_guide_src/source/libraries/security.rst index 8fc49d646b1a..1252e0f2c7f9 100644 --- a/user_guide_src/source/libraries/security.rst +++ b/user_guide_src/source/libraries/security.rst @@ -89,6 +89,47 @@ You can set to use the Session based CSRF protection by editing the following co .. literalinclude:: security/002.php +.. _csrf-fetch-metadata: + +Fetch Metadata CSRF Protection +------------------------------ + +.. versionadded:: 4.8.0 + +CodeIgniter can use Fetch Metadata request headers as a first-line CSRF check for unsafe browser requests. +Fetch Metadata is a browser-supplied signal that helps the framework allow same-origin requests and reject +cross-site requests before checking the CSRF token. + +When CSRF protection is enabled, new applications use Fetch Metadata first by default. You can configure +this behavior in **app/Config/Security.php**: + +.. literalinclude:: security/011.php + +When it is enabled, unsafe requests with ``Sec-Fetch-Site: same-origin`` are allowed without a token. +Requests with ``Sec-Fetch-Site: cross-site`` are rejected. Requests with a missing ``Sec-Fetch-Site`` header, +``Sec-Fetch-Site: none``, or an unknown value fall back to token verification. This keeps protection working +for browsers or clients that do not send Fetch Metadata headers. Requests with ``Sec-Fetch-Site: same-site`` +are rejected unless same-site requests are explicitly allowed. + +Upgraded applications without this config value continue to use token verification. You may also disable +Fetch Metadata protection by setting ``$csrfUseFetchMetadata`` to ``false``. + +When an unsafe request passes with Fetch Metadata, the CSRF token is not regenerated. Token regeneration only +runs when token verification is used. + +.. warning:: Fetch Metadata protects browser-based requests. Browsers only send these headers for + potentially trustworthy URLs, and non-browser clients can send their own headers. It is not API + authentication. + +.. warning:: Same-site is not the same as same-origin, because sibling subdomains are considered same-site. + Same-site requests are rejected by default. If all same-site origins are trusted, you may allow them + in **app/Config/Security.php**: + + .. literalinclude:: security/012.php + +If your application receives legitimate cross-origin POST requests, such as webhooks, callbacks, or +OAuth/SAML responses, use :ref:`CSRF route exclusions ` and verify those routes another way. + Token Randomization ------------------- @@ -162,6 +203,8 @@ and enabling the `csrf` filter globally: .. literalinclude:: security/006.php +.. _csrf-exclude-uris: + Select URIs can be whitelisted from CSRF protection (for example API endpoints expecting externally POSTed content). You can add these URIs by adding them as exceptions in the filter: diff --git a/user_guide_src/source/libraries/security/011.php b/user_guide_src/source/libraries/security/011.php new file mode 100644 index 000000000000..9718501446f7 --- /dev/null +++ b/user_guide_src/source/libraries/security/011.php @@ -0,0 +1,12 @@ + Date: Wed, 20 May 2026 12:06:30 +0200 Subject: [PATCH 2/5] refactor(security): use config fallback style for fetch metadata Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- system/Filters/CSRF.php | 4 ++-- system/Security/Security.php | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/system/Filters/CSRF.php b/system/Filters/CSRF.php index 0480d5e24f63..e4a2035bec82 100644 --- a/system/Filters/CSRF.php +++ b/system/Filters/CSRF.php @@ -82,8 +82,8 @@ public function after(RequestInterface $request, ResponseInterface $response, $a private function addFetchMetadataVaryHeader(IncomingRequest $request, ResponseInterface $response): void { - $config = get_object_vars(config(SecurityConfig::class)); - $useFetchMetadata = ($config['csrfUseFetchMetadata'] ?? false) === true; + $config = config(SecurityConfig::class); + $useFetchMetadata = ($config->csrfUseFetchMetadata ?? false) === true; // @phpstan-ignore nullCoalesce.property $isUnsafeMethod = in_array($request->getMethod(), [Method::POST, Method::PUT, Method::DELETE, Method::PATCH], true); if ($useFetchMetadata && $isUnsafeMethod) { diff --git a/system/Security/Security.php b/system/Security/Security.php index dddd095a35cb..9a2dff36e795 100644 --- a/system/Security/Security.php +++ b/system/Security/Security.php @@ -256,9 +256,7 @@ private function isCsrfCookie(): bool */ private function fetchMetadataDecision(IncomingRequest $request): string { - $config = get_object_vars($this->config); - - if (($config['csrfUseFetchMetadata'] ?? false) !== true) { + if (! ($this->config->csrfUseFetchMetadata ?? false)) { // @phpstan-ignore nullCoalesce.initializedProperty return self::FETCH_METADATA_FALLBACK; } @@ -273,7 +271,7 @@ private function fetchMetadataDecision(IncomingRequest $request): string } if ($fetchSite === 'same-site') { - return ($config['csrfAllowSameSite'] ?? false) === true + return $this->config->csrfAllowSameSite ?? false // @phpstan-ignore nullCoalesce.initializedProperty ? self::FETCH_METADATA_ALLOW : self::FETCH_METADATA_REJECT; } From 74f96930c76336eaa526d6c30dd6a62defe9f89f Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Fri, 22 May 2026 10:27:00 +0200 Subject: [PATCH 3/5] trigger CI Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> From 771c15e80441261634063069e813d4c332e70859 Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Mon, 25 May 2026 22:48:12 +0200 Subject: [PATCH 4/5] refactor(security): address review feedbacks Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- app/Config/Security.php | 10 ++-- system/Filters/CSRF.php | 15 ++--- system/Security/Security.php | 13 ++-- tests/system/Filters/CSRFTest.php | 10 ++-- tests/system/Security/SecurityTest.php | 60 +++++++++++++++---- .../source/installation/upgrade_480.rst | 2 +- user_guide_src/source/libraries/security.rst | 8 +-- .../source/libraries/security/011.php | 2 +- .../source/libraries/security/012.php | 2 +- 9 files changed, 80 insertions(+), 42 deletions(-) diff --git a/app/Config/Security.php b/app/Config/Security.php index d596174a3c78..bc8c42b2de9c 100644 --- a/app/Config/Security.php +++ b/app/Config/Security.php @@ -24,17 +24,17 @@ class Security extends BaseConfig * * Whether to use Fetch Metadata request headers as a first-line CSRF check. */ - public bool $csrfUseFetchMetadata = true; + public bool $csrfFetchMetadata = true; /** * -------------------------------------------------------------------------- - * CSRF Allow Same Site + * CSRF Fetch Metadata Reject Same Site * -------------------------------------------------------------------------- * - * Whether requests with the Sec-Fetch-Site: same-site header should pass - * Fetch Metadata verification. + * Whether requests with the Sec-Fetch-Site: same-site header should be + * rejected instead of falling back to token verification. */ - public bool $csrfAllowSameSite = false; + public bool $csrfFetchMetadataRejectSameSite = false; /** * -------------------------------------------------------------------------- diff --git a/system/Filters/CSRF.php b/system/Filters/CSRF.php index e4a2035bec82..b2bf1d53b25b 100644 --- a/system/Filters/CSRF.php +++ b/system/Filters/CSRF.php @@ -20,7 +20,6 @@ use CodeIgniter\HTTP\ResponseInterface; use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Security\Security; -use Config\Security as SecurityConfig; /** * CSRF filter. @@ -55,17 +54,17 @@ public function before(RequestInterface $request, $arguments = null) } catch (SecurityException $e) { if ($security->shouldRedirect() && ! $request->isAJAX()) { $response = redirect()->back()->with('error', $e->getMessage()); - $this->addFetchMetadataVaryHeader($request, $response); + $this->addFetchMetadataVaryHeader($request, $response, $security); return $response; } - $this->addFetchMetadataVaryHeader($request, service('response')); + $this->addFetchMetadataVaryHeader($request, service('response'), $security); throw $e; } - $this->addFetchMetadataVaryHeader($request, service('response')); + $this->addFetchMetadataVaryHeader($request, service('response'), $security); return null; } @@ -80,13 +79,11 @@ public function after(RequestInterface $request, ResponseInterface $response, $a return null; } - private function addFetchMetadataVaryHeader(IncomingRequest $request, ResponseInterface $response): void + private function addFetchMetadataVaryHeader(IncomingRequest $request, ResponseInterface $response, Security $security): void { - $config = config(SecurityConfig::class); - $useFetchMetadata = ($config->csrfUseFetchMetadata ?? false) === true; // @phpstan-ignore nullCoalesce.property - $isUnsafeMethod = in_array($request->getMethod(), [Method::POST, Method::PUT, Method::DELETE, Method::PATCH], true); + $isUnsafeMethod = in_array($request->getMethod(), [Method::POST, Method::PUT, Method::DELETE, Method::PATCH], true); - if ($useFetchMetadata && $isUnsafeMethod) { + if ($security->shouldUseFetchMetadata() && $isUnsafeMethod) { $response->appendHeader('Vary', 'Sec-Fetch-Site'); } } diff --git a/system/Security/Security.php b/system/Security/Security.php index 9a2dff36e795..3e8d919a6cbf 100644 --- a/system/Security/Security.php +++ b/system/Security/Security.php @@ -192,6 +192,11 @@ public function shouldRedirect(): bool return $this->config->redirect; } + public function shouldUseFetchMetadata(): bool + { + return $this->config->csrfFetchMetadata ?? false; // @phpstan-ignore nullCoalesce.initializedProperty + } + /** * @phpstan-assert string $this->hash */ @@ -256,7 +261,7 @@ private function isCsrfCookie(): bool */ private function fetchMetadataDecision(IncomingRequest $request): string { - if (! ($this->config->csrfUseFetchMetadata ?? false)) { // @phpstan-ignore nullCoalesce.initializedProperty + if (! $this->shouldUseFetchMetadata()) { return self::FETCH_METADATA_FALLBACK; } @@ -271,9 +276,9 @@ private function fetchMetadataDecision(IncomingRequest $request): string } if ($fetchSite === 'same-site') { - return $this->config->csrfAllowSameSite ?? false // @phpstan-ignore nullCoalesce.initializedProperty - ? self::FETCH_METADATA_ALLOW - : self::FETCH_METADATA_REJECT; + return ($this->config->csrfFetchMetadataRejectSameSite ?? false) // @phpstan-ignore nullCoalesce.initializedProperty + ? self::FETCH_METADATA_REJECT + : self::FETCH_METADATA_FALLBACK; } return self::FETCH_METADATA_FALLBACK; diff --git a/tests/system/Filters/CSRFTest.php b/tests/system/Filters/CSRFTest.php index 42b3e7bde2a1..2d6a67066021 100644 --- a/tests/system/Filters/CSRFTest.php +++ b/tests/system/Filters/CSRFTest.php @@ -14,12 +14,14 @@ namespace CodeIgniter\Filters; use CodeIgniter\Config\Factories; +use CodeIgniter\Config\Services; use CodeIgniter\HTTP\CLIRequest; use CodeIgniter\HTTP\IncomingRequest; use CodeIgniter\HTTP\RedirectResponse; use CodeIgniter\HTTP\Response; use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Test\CIUnitTestCase; +use CodeIgniter\Test\Mock\MockSecurity; use Config\Security as SecurityConfig; use PHPUnit\Framework\Attributes\BackupGlobals; use PHPUnit\Framework\Attributes\Group; @@ -144,16 +146,16 @@ public function testBeforeThrowsExceptionForRejectedFetchMetadataVerification(): } } - public function testBeforeDoesNotAddVaryHeaderForTokenVerification(): void + public function testBeforeUsesSecurityServiceConfigForVaryHeader(): void { service('superglobals') ->setServer('REQUEST_METHOD', 'POST') ->setPost('csrf_test_name', '8b9218a55906f9dcc1dc263dce7f005a') ->setCookie('csrf_cookie_name', '8b9218a55906f9dcc1dc263dce7f005a'); - $config = new SecurityConfig(); - $config->csrfUseFetchMetadata = false; - Factories::injectMock('config', 'Security', $config); + $config = new SecurityConfig(); + $config->csrfFetchMetadata = false; + Services::injectMock('security', new MockSecurity($config)); $filter = new CSRF(); $request = single_service('incomingrequest', null) diff --git a/tests/system/Security/SecurityTest.php b/tests/system/Security/SecurityTest.php index f20488d27286..1785341531c1 100644 --- a/tests/system/Security/SecurityTest.php +++ b/tests/system/Security/SecurityTest.php @@ -281,7 +281,7 @@ public function testCsrfVerifyFetchMetadataPreservesRawBodyWithoutToken(): void $this->assertSame('unchanged', $request->getBody()); } - public function testCsrfVerifyFetchMetadataSameSiteThrowsExceptionByDefault(): void + public function testCsrfVerifyFetchMetadataSameSiteFallsBackToTokenByDefault(): void { service('superglobals') ->setServer('REQUEST_METHOD', 'POST') @@ -291,20 +291,54 @@ public function testCsrfVerifyFetchMetadataSameSiteThrowsExceptionByDefault(): v $security = $this->createMockSecurity(); $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + + #[DataProvider('provideCsrfVerifyFetchMetadataSameSiteFallsBackToTokenFailureByDefault')] + public function testCsrfVerifyFetchMetadataSameSiteFallsBackToTokenFailureByDefault(?string $token, ?string $cookie): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + if ($token !== null) { + service('superglobals')->setPost('csrf_test_name', $token); + } + + if ($cookie !== null) { + service('superglobals')->setCookie('csrf_cookie_name', $cookie); + } + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + $this->expectException(SecurityException::class); $security->verify($request); } - public function testCsrfVerifyFetchMetadataSameSiteReturnsSelfWhenAllowed(): void + /** + * @return iterable + */ + public static function provideCsrfVerifyFetchMetadataSameSiteFallsBackToTokenFailureByDefault(): iterable + { + yield 'missing' => [null, null]; + + yield 'invalid' => [self::INVALID_CSRF_HASH, self::CORRECT_CSRF_HASH]; + } + + public function testCsrfVerifyFetchMetadataSameSiteThrowsExceptionWhenRejected(): void { - service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); - $config = new SecurityConfig(); - $config->csrfAllowSameSite = true; - $security = $this->createMockSecurity($config); - $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + $config = new SecurityConfig(); + $config->csrfFetchMetadataRejectSameSite = true; + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); - $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->expectException(SecurityException::class); + $security->verify($request); } public function testCsrfVerifyFetchMetadataCrossSiteThrowsExceptionEvenWithValidToken(): void @@ -359,10 +393,10 @@ public function testCsrfVerifyTokenOnlyIgnoresFetchMetadata(): void ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); - $config = new SecurityConfig(); - $config->csrfUseFetchMetadata = false; - $security = $this->createMockSecurity($config); - $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + $config = new SecurityConfig(); + $config->csrfFetchMetadata = false; + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); $this->assertInstanceOf(Security::class, $security->verify($request)); $this->assertLogged('info', 'CSRF token verified.'); @@ -376,7 +410,7 @@ public function testCsrfVerifyMissingFetchMetadataConfigFallsBackToTokenOnly(): ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); $config = new SecurityConfig(); - unset($config->csrfUseFetchMetadata); + unset($config->csrfFetchMetadata); $security = $this->createMockSecurity($config); $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); diff --git a/user_guide_src/source/installation/upgrade_480.rst b/user_guide_src/source/installation/upgrade_480.rst index de5d728c0e2b..069982eabdfa 100644 --- a/user_guide_src/source/installation/upgrade_480.rst +++ b/user_guide_src/source/installation/upgrade_480.rst @@ -76,7 +76,7 @@ Config - app/Config/Mimes.php - ``Config\Mimes::$mimes`` added a new key ``md`` for Markdown files. - app/Config/Security.php - - ``Config\Security::$csrfUseFetchMetadata`` and ``Config\Security::$csrfAllowSameSite`` were added for Fetch Metadata based CSRF protection. + - ``Config\Security::$csrfFetchMetadata`` and ``Config\Security::$csrfFetchMetadataRejectSameSite`` were added for Fetch Metadata based CSRF protection. All Changes =========== diff --git a/user_guide_src/source/libraries/security.rst b/user_guide_src/source/libraries/security.rst index 1252e0f2c7f9..d79d3c095345 100644 --- a/user_guide_src/source/libraries/security.rst +++ b/user_guide_src/source/libraries/security.rst @@ -109,10 +109,10 @@ When it is enabled, unsafe requests with ``Sec-Fetch-Site: same-origin`` are all Requests with ``Sec-Fetch-Site: cross-site`` are rejected. Requests with a missing ``Sec-Fetch-Site`` header, ``Sec-Fetch-Site: none``, or an unknown value fall back to token verification. This keeps protection working for browsers or clients that do not send Fetch Metadata headers. Requests with ``Sec-Fetch-Site: same-site`` -are rejected unless same-site requests are explicitly allowed. +also fall back to token verification by default. Upgraded applications without this config value continue to use token verification. You may also disable -Fetch Metadata protection by setting ``$csrfUseFetchMetadata`` to ``false``. +Fetch Metadata protection by setting ``$csrfFetchMetadata`` to ``false``. When an unsafe request passes with Fetch Metadata, the CSRF token is not regenerated. Token regeneration only runs when token verification is used. @@ -122,8 +122,8 @@ runs when token verification is used. authentication. .. warning:: Same-site is not the same as same-origin, because sibling subdomains are considered same-site. - Same-site requests are rejected by default. If all same-site origins are trusted, you may allow them - in **app/Config/Security.php**: + Same-site requests fall back to token verification by default. If sibling subdomains are not trusted, + you may reject same-site requests before token verification in **app/Config/Security.php**: .. literalinclude:: security/012.php diff --git a/user_guide_src/source/libraries/security/011.php b/user_guide_src/source/libraries/security/011.php index 9718501446f7..7b76ed96d72a 100644 --- a/user_guide_src/source/libraries/security/011.php +++ b/user_guide_src/source/libraries/security/011.php @@ -6,7 +6,7 @@ class Security extends BaseConfig { - public bool $csrfUseFetchMetadata = true; + public bool $csrfFetchMetadata = true; // ... } diff --git a/user_guide_src/source/libraries/security/012.php b/user_guide_src/source/libraries/security/012.php index 096418e5b6f6..f4decac720f1 100644 --- a/user_guide_src/source/libraries/security/012.php +++ b/user_guide_src/source/libraries/security/012.php @@ -6,7 +6,7 @@ class Security extends BaseConfig { - public bool $csrfAllowSameSite = true; + public bool $csrfFetchMetadataRejectSameSite = true; // ... } From 22087e1e279a0ac0bf1ba1831f901824c1490326 Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Tue, 26 May 2026 12:29:07 +0200 Subject: [PATCH 5/5] test(security): cover fetch metadata vary fallback Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- tests/system/Filters/CSRFTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/system/Filters/CSRFTest.php b/tests/system/Filters/CSRFTest.php index 2d6a67066021..f8d594a93ad8 100644 --- a/tests/system/Filters/CSRFTest.php +++ b/tests/system/Filters/CSRFTest.php @@ -100,6 +100,22 @@ public function testBeforeAddsVaryHeaderForFetchMetadataVerification(): void $this->assertSame('Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); } + public function testBeforeAddsVaryHeaderWhenFetchMetadataFallsBackToToken(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', '8b9218a55906f9dcc1dc263dce7f005a') + ->setCookie('csrf_cookie_name', '8b9218a55906f9dcc1dc263dce7f005a'); + + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST'); + + $filter->before($request); + + $this->assertSame('Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + public function testBeforeAppendsVaryHeaderForFetchMetadataVerification(): void { $filter = new CSRF();