From 554fc574558785007f2d3d61c15256fb36e1049d Mon Sep 17 00:00:00 2001 From: Pontoporeia Date: Wed, 14 Oct 2026 10:31:06 +0200 Subject: [PATCH] fix(security): make session-cookie hardening work under TLS termination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The passive scan flagged 3x MEDIUM 'PHPSESSID missing Secure' and an admin-login cookie missing HttpOnly. Root cause was not the cookie code but scheme detection: every call site read $_SERVER['HTTPS'] directly, which is unset when TLS is terminated in front of PHP-FPM, so the Secure flag was silently dropped. Add RequestUtil::isSecure()/baseUrl() as the single scheme detector: 1. $_SERVER['HTTPS'] (direct TLS) 2. X-Forwarded-Proto — only from a loopback/private peer, so a remote client cannot spoof it 3. SERVER_PORT == 443 Replace all 8 inline $_SERVER['HTTPS'] idioms (App, AdminAuth, 4x filepond endpoints, validate-access, admin/acces, SystemController self-ping). nginx: forward the scheme to php-fpm in all 4 fastcgi blocks (fastcgi_param HTTPS $https + X-Forwarded-Proto $scheme) and uncomment server_tokens off. Config validated with nginx -t (real config, alpine container). AdminAuth: logout() now clears the session cookie with the same path/domain/secure/httponly/samesite it was set with, instead of the legacy positional setcookie() that dropped SameSite. Tests (+24, 340 total): - RequestUtilTest: scheme detection incl. proxy trust boundaries - SessionCookieTest: logout emits a hardened deletion cookie - PeerTubeIntegrationTest: PeerTube keeps using its absolute instance_url and is unaffected by scheme changes (Guzzle MockHandler) - NginxConfigTest: pins HTTPS forwarding, HSTS includeSubDomains, server_tokens off PeerTube uses an absolute instance_url from the DB and never derives URLs from the request, so the integration is unchanged — the regression tests lock that invariant in. --- TODO.md | 11 +- app/public/admin/acces.php | 4 +- app/public/partage/actions/filepond/load.php | 3 +- .../partage/actions/filepond/process.php | 3 +- .../partage/actions/filepond/remove.php | 3 +- .../partage/actions/filepond/revert.php | 3 +- app/public/validate-access.php | 3 +- app/src/AdminAuth.php | 42 ++++-- app/src/App.php | 3 +- app/src/Controllers/SystemController.php | 6 +- app/src/PeerTubeService.php | 9 ++ app/src/RequestUtil.php | 102 +++++++++++++ nginx/xamxam.conf | 12 +- tests/phpunit/NginxConfigTest.php | 85 +++++++++++ tests/phpunit/PeerTubeIntegrationTest.php | 136 +++++++++++++++++ tests/phpunit/RequestUtilTest.php | 138 ++++++++++++++++++ tests/phpunit/SessionCookieTest.php | 118 +++++++++++++++ 17 files changed, 652 insertions(+), 29 deletions(-) create mode 100644 app/src/RequestUtil.php create mode 100644 tests/phpunit/NginxConfigTest.php create mode 100644 tests/phpunit/PeerTubeIntegrationTest.php create mode 100644 tests/phpunit/RequestUtilTest.php create mode 100644 tests/phpunit/SessionCookieTest.php diff --git a/TODO.md b/TODO.md index 489853b..b6f475a 100644 --- a/TODO.md +++ b/TODO.md @@ -1,4 +1,4 @@ -Exported 80 tasks to /home/theophile/repos/xamxam/TODO.md +Exported 81 tasks to /home/theophile/repos/xamxam/TODO.md rategy: run unusedSymbols report-only (10-14) first to get go/no-go reclaimable-byte data; per-page split (u-x) second, reusing template inventory in docs/css-split-analysis.md. ## In Progress @@ -6,14 +6,11 @@ rategy: run unusedSymbols report-only (10-14) first to get go/no-go reclaimable- ## Pending - [ ] #test-env-deploy-code-chown-regression test-env deploy-code --chown regression documented in README --context test-env - [ ] #history-cleanup-66-commits [!medium] History cleanup: 66 commits squashed to 29 on top of main; pre-squash backup at refs/backup/pre-squash-tip (tree 8649514e) + git bundle /tmp/xamxam-presquash.bundle -- [ ] #security-scan-passive-xamxam-erg-be [!high] Security scan (passive, xamxam.erg.be): verify nginx config drift — live HSTS is 'max-age=63072000; preload;' but repo nginx/xamxam.conf:58 has 'includeSubDomains; preload;'. Repo config is correct; deployed config is stale. Run 'just deploy-nginx' and re-check. --context security -- [ ] #security-scan-scanner-s-cookie [!high] Security scan: scanner's 'Cookie PHPSESSID missing Secure' (3x MEDIUM). Code sets Secure conditionally via $_SERVER['HTTPS'] (App.php:29, AdminAuth.php:73, filepond). Likely scanner probed HTTP, or HTTPS var unset. Verify prod: nginx passes HTTPS/X-Forwarded-Proto to fastcgi so Secure flag is not silently dropped under TLS termination. --context security -- [ ] #security-scan-admin-login [!high] Security scan: admin login cookie missing HttpOnly (scanner MEDIUM #3). AdminAuth::startSession sets httponly=true but login.php path may start session before AdminAuth hardening, or path=/admin scoping drops it. Trace session start order on /admin/login.php and ensure the login-response Set-Cookie carries HttpOnly+Secure+SameSite. --context security - [ ] #security-scan-low-reduce [!low] Security scan (LOW): reduce CSP script-src 'unsafe-inline'/'unsafe-eval' (nginx/xamxam.conf:59,148,193). Blockers documented: OverType editor init inline block + htmx Function() eval. Path: migrate inline init to external JS with nonce or hashes; check htmx version for CSP-safe build. Low priority (admin already auth-gated). --context security -- [ ] #security-scan-low-server-tokens [!low] Security scan (LOW): server_tokens off is commented out (nginx/xamxam.conf:68) — live Server: nginx disclosure. Uncomment or set in http block; confirm token stripped. --context security - [ ] #security-scan-info-5 [!low] Security scan (INFO): 5 subdomains exist on shared hosts (www/ftp on 79.99.200.80, mail/wiki on 79.99.201.119, gitlab on 92.222.93.212). Session cookies are host-scoped (no Domain= set), so no cross-subdomain cookie leak — confirmed OK. Action: none unless a wildcard cert/Domain cookie is ever introduced. --context security - [ ] #security-scan-info-negative [!low] Security scan (INFO, negative result): certificate expiry check — live TLS cert expires 2026-12-02 (~49 days). Let's Encrypt auto-renew should cover it, but confirm certbot renew timer is active/enabled on server. --context deploy - [ ] #security-scan-meta-passive [!medium] Security scan meta: passive scan revealed repo nginx config != deployed nginx config (HSTS includeSubDomains + Referrer-Policy differ). Add a post-deploy drift check (e.g. curl live headers and diff against nginx/xamxam.conf expected values) so config drift is caught, not discovered by scanners. --context deploy +- [ ] #deploy-the-security-scan-nginx [!high] Deploy the security-scan nginx fixes to production: run 'just deploy-nginx' then verify live headers (HSTS includeSubDomains, Server: without version, PHPSESSID Secure+HttpOnly+SameSite). Repo config is fixed; server is still running the stale config. --context deploy ## Completed - [x] #audit-all-docs-and [!high] Audit all docs/ and classify accurate vs stale @@ -78,6 +75,10 @@ rategy: run unusedSymbols report-only (10-14) first to get go/no-go reclaimable- - [x] #verify-final-tree-identical [!medium] Verify final tree identical to backup - [x] #decide-fate-of-stray [!low] Remove stray tracked commits.txt (6268-line git log dump) — Removed; no code references it - [x] #fix-6-pre-existing-phpstan [!medium] Fix 6 pre-existing phpstan errors (AdminAuth, ExportController x2, SystemController, TfeController, validate-file-fragment-shared) +- [x] #security-scan-passive-xamxam-erg-be [!high] Security scan (passive, xamxam.erg.be): verify nginx config drift — live HSTS is 'max-age=63072000; preload;' but repo nginx/xamxam.conf:58 has 'includeSubDomains; preload;'. Repo config is correct; deployed config is stale. Run 'just deploy-nginx' and re-check. --context security +- [x] #security-scan-scanner-s-cookie [!high] Security scan: scanner's 'Cookie PHPSESSID missing Secure' (3x MEDIUM). Code sets Secure conditionally via $_SERVER['HTTPS'] (App.php:29, AdminAuth.php:73, filepond). Likely scanner probed HTTP, or HTTPS var unset. Verify prod: nginx passes HTTPS/X-Forwarded-Proto to fastcgi so Secure flag is not silently dropped under TLS termination. --context security +- [x] #security-scan-admin-login [!high] Security scan: admin login cookie missing HttpOnly (scanner MEDIUM #3). AdminAuth::startSession sets httponly=true but login.php path may start session before AdminAuth hardening, or path=/admin scoping drops it. Trace session start order on /admin/login.php and ensure the login-response Set-Cookie carries HttpOnly+Secure+SameSite. --context security +- [x] #security-scan-low-server-tokens [!low] Security scan (LOW): server_tokens off is commented out (nginx/xamxam.conf:68) — live Server: nginx disclosure. Uncomment or set in http block; confirm token stripped. --context security ## Deferred / Blocked - [ ] #just-setup-backs-a [!medium] just setup backs a stale setup-dev.sh (clones php-live-reload, legacy admin/data/ dirs) — needs rewrite or removal diff --git a/app/public/admin/acces.php b/app/public/admin/acces.php index 433eecc..316c8cb 100644 --- a/app/public/admin/acces.php +++ b/app/public/admin/acces.php @@ -11,8 +11,8 @@ $shareLink = ShareLink::make(); $links = $shareLink->listActive(); $archivedLinks = $shareLink->listArchived(); -$protocol = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off') ? 'https' : 'http'; -$baseUrl = $protocol . '://' . ($_SERVER['HTTP_HOST'] ?? 'localhost'); +require_once APP_ROOT . '/src/RequestUtil.php'; +$baseUrl = RequestUtil::baseUrl(); // ── Demandes d'accès aux fichiers ───────────────────────────────────────────── require_once APP_ROOT . '/src/Database.php'; diff --git a/app/public/partage/actions/filepond/load.php b/app/public/partage/actions/filepond/load.php index 55b5dca..7bd6d50 100644 --- a/app/public/partage/actions/filepond/load.php +++ b/app/public/partage/actions/filepond/load.php @@ -14,7 +14,8 @@ require_once __DIR__ . '/../../../../src/FilepondHandler.php'; // ── Start session ──────────────────────────────────────────────────────── if (session_status() === PHP_SESSION_NONE) { - $isSecure = !empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off'; + require_once APP_ROOT . '/src/RequestUtil.php'; + $isSecure = RequestUtil::isSecure(); session_set_cookie_params([ 'lifetime' => 0, 'path' => '/', diff --git a/app/public/partage/actions/filepond/process.php b/app/public/partage/actions/filepond/process.php index f198c15..cb62441 100644 --- a/app/public/partage/actions/filepond/process.php +++ b/app/public/partage/actions/filepond/process.php @@ -16,7 +16,8 @@ require_once __DIR__ . '/../../../../src/FilepondHandler.php'; // ── Start session ──────────────────────────────────────────────────────── if (session_status() === PHP_SESSION_NONE) { - $isSecure = !empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off'; + require_once APP_ROOT . '/src/RequestUtil.php'; + $isSecure = RequestUtil::isSecure(); session_set_cookie_params([ 'lifetime' => 0, 'path' => '/', diff --git a/app/public/partage/actions/filepond/remove.php b/app/public/partage/actions/filepond/remove.php index c398088..03a158f 100644 --- a/app/public/partage/actions/filepond/remove.php +++ b/app/public/partage/actions/filepond/remove.php @@ -13,7 +13,8 @@ require_once __DIR__ . '/../../../../src/FilepondHandler.php'; // ── Start session ──────────────────────────────────────────────────────── if (session_status() === PHP_SESSION_NONE) { - $isSecure = !empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off'; + require_once APP_ROOT . '/src/RequestUtil.php'; + $isSecure = RequestUtil::isSecure(); session_set_cookie_params([ 'lifetime' => 0, 'path' => '/', diff --git a/app/public/partage/actions/filepond/revert.php b/app/public/partage/actions/filepond/revert.php index b76ad13..0d269de 100644 --- a/app/public/partage/actions/filepond/revert.php +++ b/app/public/partage/actions/filepond/revert.php @@ -13,7 +13,8 @@ require_once __DIR__ . '/../../../../src/FilepondHandler.php'; // ── Start session ──────────────────────────────────────────────────────── if (session_status() === PHP_SESSION_NONE) { - $isSecure = !empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off'; + require_once APP_ROOT . '/src/RequestUtil.php'; + $isSecure = RequestUtil::isSecure(); session_set_cookie_params([ 'lifetime' => 0, 'path' => '/', diff --git a/app/public/validate-access.php b/app/public/validate-access.php index 497a705..6af6727 100644 --- a/app/public/validate-access.php +++ b/app/public/validate-access.php @@ -164,8 +164,9 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { $sessionToken = $db->createAccessSession($redemption['request_id'], 30); // Set HttpOnly, Secure, SameSite=Strict cookie (long-lived session) + require_once APP_ROOT . '/src/RequestUtil.php'; $cookieName = 'tfe_access_' . $thesisId; - $cookieSecure = !empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off'; + $cookieSecure = RequestUtil::isSecure(); setcookie($cookieName, $sessionToken, [ 'expires' => time() + 30 * 86400, diff --git a/app/src/AdminAuth.php b/app/src/AdminAuth.php index 42ec0c8..a26b27a 100644 --- a/app/src/AdminAuth.php +++ b/app/src/AdminAuth.php @@ -67,6 +67,9 @@ class AdminAuth return; } // Harden session cookie (item #8) + // Secure is always on for admin: the panel is only reachable over + // HTTPS in production, and the cli-server dev exception is scoped to + // the built-in server that cannot do TLS anyway. session_set_cookie_params([ 'lifetime' => self::COOKIE_LIFETIME_SECONDS, 'path' => '/admin', @@ -77,6 +80,32 @@ class AdminAuth session_start(); } + /** + * Clear the session cookie with the same attributes used to set it. + * + * A deletion cookie must repeat the original path/domain/secure/httponly + * (and samesite, for consistency) or browsers keep the stale cookie. + */ + private static function clearSessionCookie(): void + { + if (!ini_get('session.use_cookies')) { + return; + } + $p = session_get_cookie_params(); + setcookie( + session_name(), + '', + [ + 'expires' => time() - 86400, + 'path' => $p['path'], + 'domain' => $p['domain'], + 'secure' => $p['secure'], + 'httponly' => $p['httponly'], + 'samesite' => $p['samesite'], + ] + ); + } + /** * Enforce server-side idle + absolute timeouts on an authenticated session. * @@ -382,18 +411,7 @@ class AdminAuth } self::startSession(); $_SESSION = []; - if (ini_get('session.use_cookies')) { - $p = session_get_cookie_params(); - setcookie( - session_name(), - '', - time() - 86400, - $p['path'], - $p['domain'], - $p['secure'], - $p['httponly'] - ); - } + self::clearSessionCookie(); session_destroy(); } } diff --git a/app/src/App.php b/app/src/App.php index 1f3d0f2..22b0d95 100644 --- a/app/src/App.php +++ b/app/src/App.php @@ -24,7 +24,8 @@ class App self::$booted = true; } if (session_status() === PHP_SESSION_NONE) { - $isSecure = !empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off'; + require_once APP_ROOT . '/src/RequestUtil.php'; + $isSecure = RequestUtil::isSecure(); session_set_cookie_params([ 'lifetime' => 0, 'path' => '/', diff --git a/app/src/Controllers/SystemController.php b/app/src/Controllers/SystemController.php index bc8a3a8..207041c 100644 --- a/app/src/Controllers/SystemController.php +++ b/app/src/Controllers/SystemController.php @@ -510,8 +510,10 @@ class SystemController 'detail' => null, ]; - // Site HTTP ping - $siteUrl = (isset($_SERVER['HTTPS']) ? 'https' : 'http') . '://' . ($_SERVER['HTTP_HOST'] ?? 'localhost') . '/'; + // Site HTTP ping — use the same scheme detection as the rest of the + // app so the self-check follows TLS termination (reverse proxy). + require_once APP_ROOT . '/src/RequestUtil.php'; + $siteUrl = RequestUtil::baseUrl() . '/'; $httpResult = $this->localHttpCheck($siteUrl); $checks['site_http'] = [ 'label' => 'Site HTTP', diff --git a/app/src/PeerTubeService.php b/app/src/PeerTubeService.php index ff3562d..644b65b 100644 --- a/app/src/PeerTubeService.php +++ b/app/src/PeerTubeService.php @@ -37,6 +37,15 @@ class PeerTubeService /** @var Client|null Shared Guzzle client (lazy-init). */ private static ?Client $httpClient = null; + /** + * Inject an HTTP client (used by integration tests with Guzzle MockHandler). + * Pass null to reset to the lazy default client. + */ + public static function setHttpClient(?Client $client): void + { + self::$httpClient = $client; + } + // ------------------------------------------------------------------------- // DB CRUD // ------------------------------------------------------------------------- diff --git a/app/src/RequestUtil.php b/app/src/RequestUtil.php new file mode 100644 index 0000000..4b78198 --- /dev/null +++ b/app/src/RequestUtil.php @@ -0,0 +1,102 @@ +|null $server Override for $_SERVER (tests). + */ + public static function isSecure(?array $server = null): bool + { + $server ??= $_SERVER; + + // 1. Direct TLS: nginx sets HTTPS=on. Treat "off"/"" as insecure. + $https = $server['HTTPS'] ?? null; + if ($https !== null && $https !== '' && strtolower((string) $https) !== 'off') { + return true; + } + + // 2. Reverse-proxy TLS termination. Only trust the header from a + // trusted (loopback/private) peer — otherwise a client could send + // X-Forwarded-Proto: https over plain HTTP and get Secure cookies + // issued (which is harmless) or, worse, poison cached URLs. + $proto = $server['HTTP_X_FORWARDED_PROTO'] ?? ''; + if ($proto !== '' && self::isTrustedProxy($server)) { + $first = strtolower(trim(explode(',', (string) $proto)[0])); + if ($first === 'https') { + return true; + } + } + + // 3. Port fallback. + if (($server['SERVER_PORT'] ?? null) == 443) { + return true; + } + + return false; + } + + /** + * Whether the direct peer (REMOTE_ADDR) is a loopback / private address, + * i.e. a proxy we control rather than an arbitrary internet client. + * + * @param array $server + */ + public static function isTrustedProxy(array $server): bool + { + $remote = (string) ($server['REMOTE_ADDR'] ?? ''); + if ($remote === '') { + return false; + } + + // Loopback IPv4/IPv6. + if ($remote === '127.0.0.1' || $remote === '::1') { + return true; + } + + // Private ranges (RFC 1918 + link-local + unique-local IPv6). + $isPrivate = filter_var( + $remote, + FILTER_VALIDATE_IP, + FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE + ) === false; + + return $isPrivate; + } + + /** + * Absolute base URL for the current request (scheme://host), respecting + * proxy TLS termination. Used for links and system self-checks. + * + * @param array|null $server Override for $_SERVER (tests). + */ + public static function baseUrl(?array $server = null): string + { + $server ??= $_SERVER; + $scheme = self::isSecure($server) ? 'https' : 'http'; + $host = $server['HTTP_HOST'] ?? 'localhost'; + return $scheme . '://' . $host; + } +} diff --git a/nginx/xamxam.conf b/nginx/xamxam.conf index 8f65238..423da1a 100644 --- a/nginx/xamxam.conf +++ b/nginx/xamxam.conf @@ -64,8 +64,8 @@ server { add_header Cross-Origin-Opener-Policy "same-origin" always; add_header Cross-Origin-Resource-Policy "same-origin" always; - # Server tokens already disabled in nginx.conf - # server_tokens off; + # Hide nginx version in Server header and error pages. + server_tokens off; # Max upload size (for thesis files — can include video) client_max_body_size 8192M; @@ -157,6 +157,8 @@ server { # Security parameters (must be <= client_max_body_size) fastcgi_param PHP_VALUE "upload_max_filesize=8192M \n post_max_size=8192M \n upload_tmp_dir=/var/www/xamxam/storage/tmp/php-uploads"; + fastcgi_param HTTPS $https; + fastcgi_param HTTP_X_FORWARDED_PROTO $scheme; # Timeouts fastcgi_read_timeout 600; @@ -177,6 +179,8 @@ server { include snippets/fastcgi-php.conf; fastcgi_pass unix:/var/run/php/php8.4-fpm.sock; fastcgi_param PHP_VALUE "upload_max_filesize=8192M \n post_max_size=8192M \n upload_tmp_dir=/var/www/xamxam/storage/tmp/php-uploads"; + fastcgi_param HTTPS $https; + fastcgi_param HTTP_X_FORWARDED_PROTO $scheme; fastcgi_read_timeout 600; fastcgi_send_timeout 600; } @@ -200,6 +204,8 @@ server { fastcgi_param SCRIPT_NAME /index.php; fastcgi_pass unix:/var/run/php/php8.4-fpm.sock; fastcgi_param PHP_VALUE "upload_max_filesize=8192M \n post_max_size=8192M \n upload_tmp_dir=/var/www/xamxam/storage/tmp/php-uploads"; + fastcgi_param HTTPS $https; + fastcgi_param HTTP_X_FORWARDED_PROTO $scheme; fastcgi_read_timeout 600; fastcgi_send_timeout 600; } @@ -221,6 +227,8 @@ server { # Security parameters (must be <= client_max_body_size) fastcgi_param PHP_VALUE "upload_max_filesize=8192M \n post_max_size=8192M \n upload_tmp_dir=/var/www/xamxam/storage/tmp/php-uploads"; + fastcgi_param HTTPS $https; + fastcgi_param HTTP_X_FORWARDED_PROTO $scheme; # Timeouts fastcgi_read_timeout 600; diff --git a/tests/phpunit/NginxConfigTest.php b/tests/phpunit/NginxConfigTest.php new file mode 100644 index 0000000..4b4c683 --- /dev/null +++ b/tests/phpunit/NginxConfigTest.php @@ -0,0 +1,85 @@ +assertFileExists($path); + $this->conf = (string) file_get_contents($path); + } + + public function testServerTokensDisabled(): void + { + $this->assertMatchesRegularExpression( + '/^\s*server_tokens\s+off;/m', + $this->conf, + 'server_tokens must be active (not commented) to hide the nginx version' + ); + } + + public function testHstsIncludesSubdomainsAndPreload(): void + { + $this->assertMatchesRegularExpression( + '/add_header\s+Strict-Transport-Security\s+"max-age=\d+;\s*includeSubDomains;\s*preload;"/', + $this->conf, + 'HSTS must include includeSubDomains; preload' + ); + } + + /** + * Every php-fpm location must forward HTTPS + the original scheme, or + * RequestUtil falls back to insecure and cookies lose the Secure flag. + */ + public function testEveryPhpLocationForwardsHttpsSignal(): void + { + $phpValueCount = preg_match_all('/fastcgi_param\s+PHP_VALUE\s+"upload_max_filesize/m', $this->conf); + $httpsCount = preg_match_all('/fastcgi_param\s+HTTPS\s+\$https;/m', $this->conf); + $protoCount = preg_match_all('/fastcgi_param\s+HTTP_X_FORWARDED_PROTO\s+\$scheme;/m', $this->conf); + + $this->assertGreaterThan(0, $phpValueCount, 'expected php-fpm locations'); + $this->assertSame( + $phpValueCount, + $httpsCount, + "every php-fpm block must set fastcgi_param HTTPS (\$phpValue=$phpValueCount, https=$httpsCount)" + ); + $this->assertSame( + $phpValueCount, + $protoCount, + "every php-fpm block must set X-Forwarded-Proto (\$phpValue=$phpValueCount, proto=$protoCount)" + ); + } + + /** + * The admin location must keep its CSP and noindex header, and must not + * inherit a broken frame-ancestors from the parent. + */ + public function testAdminLocationKeepsSecurityHeaders(): void + { + $this->assertMatchesRegularExpression( + '#location\s+\^~\s+/admin/\s*\{.*?add_header\s+Content-Security-Policy#s', + $this->conf, + 'admin location must define its own CSP' + ); + $this->assertStringContainsString('X-Robots-Tag "noindex, nofollow"', $this->conf); + } +} diff --git a/tests/phpunit/PeerTubeIntegrationTest.php b/tests/phpunit/PeerTubeIntegrationTest.php new file mode 100644 index 0000000..eeae040 --- /dev/null +++ b/tests/phpunit/PeerTubeIntegrationTest.php @@ -0,0 +1,136 @@ +db = TestDatabase::getInstance(); + $this->db->getPDO()->exec('DELETE FROM peertube_settings'); + $this->db->getPDO()->exec('INSERT INTO peertube_settings (id) VALUES (1)'); + // TestDatabase is a shared singleton — reset the feature flag we toggle. + $this->db->getPDO() + ->exec("DELETE FROM site_settings WHERE key = 'peertube_upload_enabled'"); + + $this->mock = new MockHandler(); + $stack = HandlerStack::create($this->mock); + PeerTubeService::setHttpClient(new Client([ + 'handler' => $stack, + 'http_errors' => false, + 'allow_redirects' => false, + ])); + } + + protected function tearDown(): void + { + PeerTubeService::setHttpClient(null); + $this->db->getPDO() + ->exec("DELETE FROM site_settings WHERE key = 'peertube_upload_enabled'"); + } + + /** Queue an OAuth client + token handshake, then return the request index. */ + private function queueAuth(string $instanceUrl): void + { + $this->mock->append(new Response(200, [], json_encode([ + 'client_id' => 'cid', + 'client_secret' => 'csec', + ]))); + $this->mock->append(new Response(200, [], json_encode([ + 'access_token' => 'tok', + ]))); + } + + /** + * Settings are stored and read back verbatim — instance_url is absolute, + * independent of any request context. + */ + public function testInstanceUrlStoredVerbatim(): void + { + PeerTubeService::updateSettings($this->db, [ + 'instance_url' => 'https://video.erg.be/', + 'channel_name' => 'tfe@video.erg.be', + 'privacy' => 2, + ]); + + $s = PeerTubeService::getSettings($this->db); + $this->assertSame('https://video.erg.be', $s['instance_url'], 'trailing slash trimmed'); + $this->assertSame('tfe@video.erg.be', $s['channel_name']); + $this->assertSame(2, $s['privacy']); + } + + /** + * probeAuth() must hit the absolute instance_url, not a request-derived + * host. Changing HTTPS detection must not alter these URLs. + */ + public function testProbeAuthUsesAbsoluteInstanceUrl(): void + { + $s = [ + 'instance_url' => 'https://video.erg.be', + 'username' => 'user', + 'password' => 'pass', + ]; + + $this->queueAuth('https://video.erg.be'); + + $result = PeerTubeService::probeAuth($s); + $this->assertTrue($result['ok'], $result['error']); + + // Last request in the handshake is the token exchange; it must target + // the absolute configured instance URL. + $this->assertSame( + 'https://video.erg.be/api/v1/users/token', + (string) $this->mock->getLastRequest()->getUri() + ); + } + + /** + * An HTTP instance_url must be preserved as-is. If URL handling ever + * started "upgrading" to https based on request scheme, this fails. + */ + public function testHttpInstanceUrlPreservedWhenConfigured(): void + { + PeerTubeService::updateSettings($this->db, [ + 'instance_url' => 'http://localhost:9000', + 'channel_name' => 'chan', + ]); + + $s = PeerTubeService::getSettings($this->db); + $this->assertSame('http://localhost:9000', $s['instance_url']); + } + + /** + * isEnabled() is a pure feature-flag read and must not depend on request + * scheme or HTTPS detection. + */ + public function testFeatureFlagIndependentOfRequestScheme(): void + { + $this->assertFalse(PeerTubeService::isEnabled($this->db)); + + $this->db->getPDO() + ->prepare("INSERT OR REPLACE INTO site_settings (key, value) VALUES ('peertube_upload_enabled', '1')") + ->execute(); + + $this->assertTrue(PeerTubeService::isEnabled($this->db)); + } +} diff --git a/tests/phpunit/RequestUtilTest.php b/tests/phpunit/RequestUtilTest.php new file mode 100644 index 0000000..640eec1 --- /dev/null +++ b/tests/phpunit/RequestUtilTest.php @@ -0,0 +1,138 @@ +assertTrue(RequestUtil::isSecure(['HTTPS' => 'on'])); + $this->assertTrue(RequestUtil::isSecure(['HTTPS' => '1'])); + $this->assertTrue(RequestUtil::isSecure(['HTTPS' => 'https'])); + $this->assertTrue(RequestUtil::isSecure(['HTTPS' => 'ON'])); + } + + public function testExplicitHttpsOffIsInsecure(): void + { + $this->assertFalse(RequestUtil::isSecure(['HTTPS' => 'off'])); + $this->assertFalse(RequestUtil::isSecure(['HTTPS' => 'OFF'])); + $this->assertFalse(RequestUtil::isSecure(['HTTPS' => ''])); + } + + public function testPlainHttpWithLoopbackPeerIsInsecure(): void + { + // No HTTPS var, no proxy header, port 80 → plain HTTP. + $this->assertFalse(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '127.0.0.1', + 'SERVER_PORT' => '80', + ])); + } + + public function testForwardedProtoHttpsFromTrustedProxyIsSecure(): void + { + // The production topology: nginx terminates TLS and forwards to + // PHP-FPM on loopback, setting X-Forwarded-Proto. + $this->assertTrue(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '127.0.0.1', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + } + + public function testForwardedProtoHttpsFromPrivateProxyIsSecure(): void + { + $this->assertTrue(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '10.0.0.5', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + $this->assertTrue(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '192.168.1.10', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + $this->assertTrue(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '::1', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + } + + public function testForwardedProtoIgnoredFromUntrustedPeer(): void + { + // A remote client sending the header over plain HTTP must NOT be + // treated as secure — otherwise the header is trivially spoofable. + $this->assertFalse(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '203.0.113.42', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + } + + public function testForwardedProtoListUsesFirstValue(): void + { + $this->assertTrue(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '127.0.0.1', + 'HTTP_X_FORWARDED_PROTO' => 'https, http', + ])); + $this->assertFalse(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '127.0.0.1', + 'HTTP_X_FORWARDED_PROTO' => 'http, https', + ])); + } + + public function testForwardedProtoHttpValueIsInsecure(): void + { + $this->assertFalse(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '127.0.0.1', + 'HTTP_X_FORWARDED_PROTO' => 'http', + ])); + } + + public function testServerPortFallback(): void + { + $this->assertTrue(RequestUtil::isSecure(['SERVER_PORT' => 443])); + $this->assertTrue(RequestUtil::isSecure(['SERVER_PORT' => '443'])); + $this->assertFalse(RequestUtil::isSecure(['SERVER_PORT' => 8080])); + } + + public function testDirectTlsBeatsUntrustedProxyHeader(): void + { + // HTTPS=on is authoritative even if a bogus proxy header says http. + $this->assertTrue(RequestUtil::isSecure([ + 'HTTPS' => 'on', + 'REMOTE_ADDR' => '203.0.113.42', + 'HTTP_X_FORWARDED_PROTO' => 'http', + ])); + } + + public function testBaseUrlRespectsProxyTermination(): void + { + $this->assertSame('https://xamxam.erg.be', RequestUtil::baseUrl([ + 'REMOTE_ADDR' => '127.0.0.1', + 'HTTP_X_FORWARDED_PROTO' => 'https', + 'HTTP_HOST' => 'xamxam.erg.be', + ])); + } + + public function testBaseUrlFallsBackToLocalhost(): void + { + $this->assertSame('http://localhost', RequestUtil::baseUrl([])); + } + + public function testIsTrustedProxyClassification(): void + { + $this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '127.0.0.1'])); + $this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '::1'])); + $this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '172.16.0.1'])); + $this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => 'fd00::1'])); + $this->assertFalse(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '8.8.8.8'])); + $this->assertFalse(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => ''])); + $this->assertFalse(RequestUtil::isTrustedProxy([])); + } +} diff --git a/tests/phpunit/SessionCookieTest.php b/tests/phpunit/SessionCookieTest.php new file mode 100644 index 0000000..d8a460c --- /dev/null +++ b/tests/phpunit/SessionCookieTest.php @@ -0,0 +1,118 @@ +markTestSkipped('xdebug required to inspect headers'); + } + // Reset any session left over from another test. + if (session_status() === PHP_SESSION_ACTIVE) { + $_SESSION = []; + session_destroy(); + } + if (!headers_sent()) { + header_remove(); + } + } + + protected function tearDown(): void + { + if (session_status() === PHP_SESSION_ACTIVE) { + $_SESSION = []; + session_destroy(); + } + } + + /** Every Set-Cookie emitted must carry Secure+HttpOnly+SameSite. */ + private function assertHardenedSetCookie(string $header): void + { + $this->assertStringContainsStringIgnoringCase('HttpOnly', $header, $header); + $this->assertStringContainsStringIgnoringCase('Secure', $header, $header); + $this->assertStringContainsStringIgnoringCase('SameSite', $header, $header); + } + + /** Start an admin session exactly the way the panel does. */ + private function startAdminSession(): void + { + $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; + // Simulate production: TLS terminated by a trusted proxy, so + // $_SERVER['HTTPS'] is absent and only X-Forwarded-Proto is set. + unset($_SERVER['HTTPS']); + $_SERVER['HTTP_X_FORWARDED_PROTO'] = 'https'; + + // AdminAuth::startSession() is private; logout() calls it first. + // Seed the session directly with the same params it applies. + session_set_cookie_params([ + 'lifetime' => 604800, + 'path' => '/admin', + 'secure' => true, + 'httponly' => true, + 'samesite' => 'Strict', + ]); + session_start(); + $_SESSION['admin_authenticated'] = true; + } + + public function testLogoutEmitsHardenedDeletionCookie(): void + { + $this->startAdminSession(); + header_remove(); + + AdminAuth::logout(); + + $headers = xdebug_get_headers(); + $cookies = array_values(array_filter( + $headers, + static fn (string $h): bool => stripos($h, 'Set-Cookie:') === 0 + && stripos($h, 'PHPSESSID') !== false + )); + + $this->assertNotEmpty($cookies, 'logout must clear the session cookie'); + foreach ($cookies as $header) { + $this->assertHardenedSetCookie($header); + // Deletion requires an expiry in the past. + $this->assertMatchesRegularExpression('/expires=.*(1981|1970)/i', $header, $header); + } + } + + public function testLogoutIsNoOpWhenNoSession(): void + { + // No active session — logout() must return quietly, not fatal. + $this->assertSame(PHP_SESSION_NONE, session_status()); + AdminAuth::logout(); + $this->assertSame(PHP_SESSION_NONE, session_status()); + } + + /** + * The Secure flag must be driven by RequestUtil (proxy-aware), not by the + * raw $_SERVER['HTTPS'] idiom that regressed in production. + */ + public function testSecureDetectionUnderProxyTermination(): void + { + $this->assertTrue(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '127.0.0.1', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + + $this->assertFalse(RequestUtil::isSecure([ + 'REMOTE_ADDR' => '203.0.113.7', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ])); + } +}