From f2020662fd60e5ecf168e3bd08d51598fe6f6975 Mon Sep 17 00:00:00 2001 From: Samanta <149822405+Shabbir7890@users.noreply.github.com> Date: Sat, 26 Sep 2026 12:41:41 +0100 Subject: [PATCH] fix(panelSessionGate): correct the Lua user-path match and execute the gate in tests (Refs #963) panel_user() used the PCRE idiom "($|/)" inside a Lua string.match, where it is the literal text "$|/" and matches no real path. The gate therefore returned nil on every request and delegated to Basic auth, so the opt-in cookie login was inert wherever enabled. Match the two real shapes explicitly: a trailing-slash prefix or the bare /user-. PanelSessionGateSourceTest only asserts source strings, which is how this shipped green. Add PanelSessionGateExecutionTest, which runs the real .lua under a stubbed lighty global and asserts the decision reached (login handler resolves the user, html visitor without a session gets 302, a request with Basic creds delegates, a non-panel path never sets REMOTE_USER). The dev container gets lua5.4 so the test runs; it skips cleanly where no Lua is present. --- Dockerfile | 1 + scripts/lib/lighttpd/panelSessionGate.lua | 5 +- .../PanelSessionGateExecutionTest.php | 170 ++++++++++++++++++ scripts/testing/check-tools.sh | 8 + 4 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 scripts/lib/tests/development/PanelSessionGateExecutionTest.php diff --git a/Dockerfile b/Dockerfile index 87eb720d6..2c24aba66 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,6 +10,7 @@ RUN apt-get update \ ca-certificates \ file \ git \ + lua5.4 \ php-cli \ ripgrep \ shellcheck \ diff --git a/scripts/lib/lighttpd/panelSessionGate.lua b/scripts/lib/lighttpd/panelSessionGate.lua index a09bae74e..db91a832d 100644 --- a/scripts/lib/lighttpd/panelSessionGate.lua +++ b/scripts/lib/lighttpd/panelSessionGate.lua @@ -22,7 +22,10 @@ local function request_path() end local function panel_user(path) - local user = path:match("^/user%-([a-z][a-z0-9]*)($|/)") + -- Lua patterns have no alternation and $ anchors only as the final character, so the + -- old "($|/)" group matched the literal text "$|/" and never a real path. Match the + -- two real shapes explicitly: a trailing-slash prefix, or the bare "/user-". + local user = path:match("^/user%-([a-z][a-z0-9]*)/") or path:match("^/user%-([a-z][a-z0-9]*)$") if not user or #user > 8 then return nil end diff --git a/scripts/lib/tests/development/PanelSessionGateExecutionTest.php b/scripts/lib/tests/development/PanelSessionGateExecutionTest.php new file mode 100644 index 000000000..80201aa73 --- /dev/null +++ b/scripts/lib/tests/development/PanelSessionGateExecutionTest.php @@ -0,0 +1,170 @@ +/dev/null')) !== '') { + return $candidate; + } + } + return null; + } + + /** Emit a Lua-safe double-quoted string literal for a test-controlled value. */ + private function luaQuote(string $value): string + { + $out = ''; + $length = strlen($value); + for ($i = 0; $i < $length; $i++) { + $char = $value[$i]; + if ($char === '\\') { + $out .= '\\\\'; + } elseif ($char === '"') { + $out .= '\\"'; + } elseif ($char === "\n") { + $out .= '\\n'; + } elseif ($char === "\r") { + $out .= '\\r'; + } else { + $out .= $char; + } + } + return '"'.$out.'"'; + } + + /** + * Run the gate for one request and return its outcome. + * + * @param array $headers + * @return array{rc:string,remote_user:string,auth:string,err:string} + */ + private function runGate(string $luaBinary, string $path, array $headers): array + { + $gate = $this->pmssRepoPath('scripts/lib/lighttpd/panelSessionGate.lua'); + + $requestLines = ''; + foreach ($headers as $name => $value) { + $requestLines .= ' ['.$this->luaQuote((string) $name).'] = '.$this->luaQuote((string) $value).",\n"; + } + + // Stub only what the gate reads; silence its log_event print() so stdout + // carries just our markers. dofile() returns the chunk's `return` value. + $driver = "print = function() end\n" + ."local reqEnv = {}\n" + ."lighty = {\n" + ." req_env = reqEnv,\n" + ." request = {\n".$requestLines." },\n" + ." env = { [\"uri.path\"] = ".$this->luaQuote($path)." },\n" + ." header = {},\n" + ."}\n" + ."local ok, rc = pcall(dofile, ".$this->luaQuote($gate).")\n" + ."if not ok then io.write(\"ERR=\"..tostring(rc)..\"\\n\") os.exit(0) end\n" + ."io.write(\"RC=\"..tostring(rc)..\"\\n\")\n" + ."io.write(\"REMOTE_USER=\"..tostring(reqEnv[\"REMOTE_USER\"])..\"\\n\")\n" + ."io.write(\"AUTH=\"..tostring(reqEnv[\"PMSS_PANEL_AUTH\"])..\"\\n\")\n"; + + $dir = $this->pmssMakeTempDir('panelgate'); + $driverPath = $dir.'/driver.lua'; + file_put_contents($driverPath, $driver); + + $output = (string) @shell_exec($luaBinary.' '.escapeshellarg($driverPath).' 2>&1'); + $result = ['rc' => '', 'remote_user' => '', 'auth' => '', 'err' => '']; + foreach (preg_split('/\r?\n/', $output) ?: [] as $line) { + if (strpos($line, 'RC=') === 0) { + $result['rc'] = substr($line, 3); + } elseif (strpos($line, 'REMOTE_USER=') === 0) { + $result['remote_user'] = substr($line, 12); + } elseif (strpos($line, 'AUTH=') === 0) { + $result['auth'] = substr($line, 5); + } elseif (strpos($line, 'ERR=') === 0) { + $result['err'] = substr($line, 4); + } + } + return $result; + } + + public function testGateResolvesPanelUserAndReachesTheLoginHandler(): void + { + $lua = $this->luaBinary(); + if ($lua === null) { + throw new SkipTest('no Lua interpreter (lua5.4/lua5.3/lua5.1/lua) available'); + } + + // The public login handler under a matched user: the gate resolves the + // user and hands the request on. Under the old pattern panel_user() was + // nil here and REMOTE_USER was never set. + $result = $this->runGate($lua, '/user-bob/panelSessionLogin.php', []); + $this->assertSame('', $result['err'], 'gate raised a Lua error: '.$result['err']); + $this->assertSame('bob', $result['remote_user'], 'login handler must resolve the panel user'); + $this->assertSame('login', $result['auth']); + } + + public function testGateRedirectsHtmlVisitorWithoutSessionToLogin(): void + { + $lua = $this->luaBinary(); + if ($lua === null) { + throw new SkipTest('no Lua interpreter (lua5.4/lua5.3/lua5.1/lua) available'); + } + + // No cookie, no Authorization, Accept: text/html -> redirect to login (302). + $result = $this->runGate($lua, '/user-bob/', ['Accept' => 'text/html']); + $this->assertSame('302', $result['rc'], 'html visitor with no session must be redirected to login'); + + // The bare "/user-bob" form (no trailing slash) must match too — this is the + // end-of-string case the original "$" anchor was meant to cover. + $result = $this->runGate($lua, '/user-bob', ['Accept' => 'text/html']); + $this->assertSame('302', $result['rc'], 'bare /user- must also be recognised'); + } + + public function testGateDelegatesWhenBasicCredentialsArePresent(): void + { + $lua = $this->luaBinary(); + if ($lua === null) { + throw new SkipTest('no Lua interpreter (lua5.4/lua5.3/lua5.1/lua) available'); + } + + // A request already carrying Basic credentials is handed to the htpasswd + // path (return nil), not redirected. + $result = $this->runGate($lua, '/user-bob/', ['Authorization' => 'Basic Zm9vOmJhcg==']); + $this->assertSame('nil', $result['rc'], 'a request with Basic credentials must delegate'); + } + + public function testGateIgnoresNonPanelPaths(): void + { + $lua = $this->luaBinary(); + if ($lua === null) { + throw new SkipTest('no Lua interpreter (lua5.4/lua5.3/lua5.1/lua) available'); + } + + // A path that is not /user- must delegate untouched and never set + // REMOTE_USER, so the gate cannot authenticate the wrong account. + $result = $this->runGate($lua, '/webdav-bob/file', ['Accept' => 'text/html']); + $this->assertSame('nil', $result['rc']); + $this->assertSame('nil', $result['remote_user'], 'non-panel paths must not set REMOTE_USER'); + } +} diff --git a/scripts/testing/check-tools.sh b/scripts/testing/check-tools.sh index e7171de6d..9df50fac8 100755 --- a/scripts/testing/check-tools.sh +++ b/scripts/testing/check-tools.sh @@ -17,4 +17,12 @@ for t in rg shellcheck shfmt phpstan; do if have "$t"; then echo " • $t (optional)"; else echo " • $t (missing, optional)"; fi done +# Lua interpreter: needed to execute the mod_magnet panel session gate in tests +# (any of lua5.4/lua5.3/lua5.1/lua). Without it PanelSessionGateExecutionTest skips. +if have lua5.4 || have lua5.3 || have lua5.1 || have lua; then + echo " • lua (present; gate-execution test will run)" +else + echo " • lua (missing; gate-execution test will skip)" +fi + exit 0