keith/BattleForge
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: detect stale CSRF cookie pair and auto-rotate
CI / php (push) Failing after 1m17s
Details

Browse Source 
When a browser session predates a server restart with a fresh
secret.key, the old __csrf HMAC cookie is no longer verifiable
under the new secret. Every action POST returns 403 until the user
manually clears cookies, because the existing-pair check on the
front controller was 'is the cookie present?' rather than 'does
the cookie verify under the current secret?'.

The new check at the top of the front controller computes the
expected HMAC of the raw-token cookie under the current secret and
constant-time-compares it to the __csrf cookie. If they don't match,
the controller re-mints a fresh pair on the response, replacing the
browser's stale cookies with a working set. The next click of 'Play
bundled' then succeeds without user intervention.

Test coverage in tests/Integration/StaleCookieRotationTest.php:
  - testStaleCookiesAreReplacedOnNextRequest
  - testValidCookiesAreNotReIssuedOnNextRequest
  - testMissingTokenCookieIsFilledInOnNextRequest
This commit is contained in:
Keith Solomon
2026-07-26 16:07:51 -05:00
parent 2db141efc6
commit 3185bc3602
2 changed files with 194 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/index.php
+19 -7
View File
@@ -43,16 +43,28 @@ if ($secret === false || $secret === '') {
}
}
// 2. Ensure the __csrf cookie is set. If absent, issue a fresh token.
// The double-submit pattern stores the raw token in a separate readable
// cookie so client code (meta tag, JS) can echo it back as the
// X-CSRF-Token header. The HMAC cookie is the verification oracle;
// the raw-token cookie rides parallel and is readable by document.cookie
// or the equivalent server-side read.
// 2. Ensure the __csrf cookie is set. If absent OR stale, issue a fresh
// token. A cookie pair is "stale" when both cookies are present but
// the HMAC (__csrf) does not match the raw token (__csrf_token) under
// the current secret. This can happen when a browser session predates
// a server restart with a fresh secret.key: the browser still has the
// old __csrf cookie, the new server's secret does not match it, and
// every CSRF check fails. By re-minting on first request, we recover
// the session without requiring the user to clear cookies manually.
// The double-submit pattern stores the raw token in a separate
// readable cookie so client code (meta tag, JS) can echo it back as
// the X-CSRF-Token header. The HMAC cookie is the verification
// oracle; the raw-token cookie rides parallel and is readable by
// document.cookie or the equivalent server-side read.
$cookies = $_COOKIE;
$existingCookie = (string) ($cookies['__csrf'] ?? '');
$existingRawToken = (string) ($cookies['__csrf_token'] ?? '');
if ($existingCookie === '' || $existingRawToken === '') {
$pairIsFresh = false;
if ($existingCookie !== '' && $existingRawToken !== '') {
$expectedHmac = hash_hmac('sha256', $existingRawToken, $secret);
$pairIsFresh = hash_equals($expectedHmac, $existingCookie);
}
if ($existingCookie === '' || $existingRawToken === '' || !$pairIsFresh) {
[$token, $cookie] = CsrfToken::issue($secret);
setcookie('__csrf', $cookie, [
'expires' => time() + 86400,
tests/Integration/StaleCookieRotationTest.php
+175
View File
@@ -0,0 +1,175 @@
<?php
declare(strict_types=1);
namespace BattleForge\Tests\Integration;
use BattleForge\Http\CsrfToken;
use PHPUnit\Framework\TestCase;
/**
* Verifies that the front controller detects a stale CSRF cookie pair
* (one minted under a different secret.key) and re-mints a fresh pair on
* the next request. Without this, a browser session that predates a
* server restart with a fresh secret would have every action return 403
* until the user manually cleared cookies.
*
* The test boots the front controller in-process (matching the pattern
* in FullFlowTest) and reads the re-minted raw token from the meta tag
* in the response body. The HMAC verification of the new pair against
* the current SECRET can then be asserted in pure PHP without needing
* to inspect response Set-Cookie headers.
*/
final class StaleCookieRotationTest extends TestCase
{
private const SECRET_A = 'unit-test-secret-A';
private const SECRET_B = 'unit-test-secret-B';
private int $lastStatus = 0;
protected function setUp(): void
{
// Reset superglobals between requests.
$_GET = [];
$_POST = [];
$_FILES = [];
$_COOKIE = [];
$_SERVER = [];
}
protected function tearDown(): void
{
// Clear the env override set by individual tests so other tests
// do not see a polluted environment.
putenv('BATTLEFORGE_SECRET');
}
public function testStaleCookiesAreReplacedOnNextRequest(): void
{
// 1. Boot the front controller under SECRET_A to mint a cookie pair.
// We don't pre-mint a token via CsrfToken::issue because the test
// would then have to align it with what the controller mints; the
// controller's mint uses random_bytes(32) so the test's mint and
// the controller's mint would differ. Instead, we just observe
// that the controller minted *something* under SECRET_A.
putenv('BATTLEFORGE_SECRET=' . self::SECRET_A);
$this->primeGet('/');
$bodyA = $this->runFrontController();
self::assertSame(200, $this->lastStatus);
$tokenA = $this->extractMetaToken($bodyA);
// 2. Now simulate a server restart with SECRET_B. The browser
// still has the old cookies (minted under SECRET_A). We capture
// them from the first request's response by re-running a request
// with the same env: but since we cannot read Set-Cookie headers
// from a captured PHP test, we re-derive them using the same
// secret + the raw token the controller issued.
$cookieA = hash_hmac('sha256', $tokenA, self::SECRET_A);
// 3. Send a second request with the stale cookies but SECRET_B
// active. The front controller should detect the mismatch and
// re-mint a fresh pair.
putenv('BATTLEFORGE_SECRET=' . self::SECRET_B);
$_COOKIE['__csrf'] = $cookieA;
$_COOKIE['__csrf_token'] = $tokenA;
$this->primeGet('/');
$bodyB = $this->runFrontController();
// 4. The response should now carry a meta tag with a different
// raw token, and that token should HMAC under SECRET_B.
$tokenB = $this->extractMetaToken($bodyB);
self::assertNotSame($tokenA, $tokenB, 'raw token should be re-minted under SECRET_B');
$hmacUnderB = hash_hmac('sha256', $tokenB, self::SECRET_B);
self::assertSame(64, strlen($hmacUnderB), 'HMAC under SECRET_B should be 64 hex chars');
}
public function testValidCookiesAreNotReIssuedOnNextRequest(): void
{
// Mint a fresh pair under SECRET_A and capture both the raw token
// and the corresponding HMAC. The test sends these as the browser's
// cookie jar on the second request; the controller should recognise
// them as fresh and re-use the existing pair (no re-mint).
putenv('BATTLEFORGE_SECRET=' . self::SECRET_A);
[$tokenA, $cookieA] = CsrfToken::issue(self::SECRET_A);
// 1. First request: no cookies, controller mints a fresh pair.
$this->primeGet('/');
$bodyA = $this->runFrontController();
self::assertSame(200, $this->lastStatus);
$tokenAController = $this->extractMetaToken($bodyA);
$cookieAController = hash_hmac('sha256', $tokenAController, self::SECRET_A);
// 2. Second request: pre-mint the cookies that match the test's
// expectation. The controller should recognise the pair is fresh
// and re-use it (meta tag carries the same token, no re-mint).
$_COOKIE['__csrf'] = $cookieA;
$_COOKIE['__csrf_token'] = $tokenA;
$this->primeGet('/');
$bodyB = $this->runFrontController();
$tokenB = $this->extractMetaToken($bodyB);
// Both bodies have valid tokens; the test's "no re-mint" claim is
// that $tokenA (the test's mint) is the same as $tokenB (the
// controller's reuse). The controller's mint and the test's mint
// are different only when the controller re-mints, so the
// identity proves the controller re-used the existing pair.
self::assertSame($tokenA, $tokenB, 'controller should re-use the existing token when cookies are valid');
}
public function testMissingTokenCookieIsFilledInOnNextRequest(): void
{
// 1. Boot under SECRET_A so the controller mints a pair.
putenv('BATTLEFORGE_SECRET=' . self::SECRET_A);
$this->primeGet('/');
$bodyA = $this->runFrontController();
$tokenA = $this->extractMetaToken($bodyA);
// 2. Browser has the __csrf cookie but is missing __csrf_token
// (a partial-clear scenario, e.g. user cleared just one).
putenv('BATTLEFORGE_SECRET=' . self::SECRET_B);
$_COOKIE['__csrf'] = hash_hmac('sha256', $tokenA, self::SECRET_A);
// __csrf_token is intentionally missing.
$this->primeGet('/');
$bodyB = $this->runFrontController();
$tokenB = $this->extractMetaToken($bodyB);
self::assertNotSame($tokenA, $tokenB, 'missing __csrf_token should trigger re-mint');
}
private function primeGet(string $path): void
{
$_SERVER['REQUEST_METHOD'] = 'GET';
$_SERVER['REQUEST_URI'] = $path;
$_SERVER['CONTENT_TYPE'] = null;
}
private function extractMetaToken(string $body): string
{
$regex = '/<meta name="csrf-token" content="([a-f0-9]{64})">/';
if (!preg_match($regex, $body, $m)) {
self::fail('response body did not contain a 64-char hex <meta name="csrf-token">; first 500 chars: ' . substr($body, 0, 500));
}
return $m[1];
}
private function runFrontController(): string
{
$this->lastStatus = 0;
$body = $this->captureOutput(function (): void {
$this->lastStatus = (require __DIR__ . '/../../public/index.php') ?? http_response_code();
});
return $body;
}
/** @param callable(): void $fn */
private function captureOutput(callable $fn): string
{
ob_start();
try {
$fn();
} finally {
$output = (string) ob_get_clean();
}
return $output;
}
}