From 49478e1e35bcfe6ef79b9489087246bb7938aa3b Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Sat, 31 Oct 2026 13:33:04 -0700 Subject: Share the request helpers in the auth checks The two identical posts for an unknown user become one, and no two checks share a name, so a failure report points at a single check. --- tests/extensions/test-auth.lua | 126 ++++++++++++++++------------------------- 1 file changed, 49 insertions(+), 77 deletions(-) diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua index 77cc605..a45b1e9 100644 --- a/tests/extensions/test-auth.lua +++ b/tests/extensions/test-auth.lua @@ -142,7 +142,7 @@ h.equals("no header yields no cookie", get_cookie(nil, "cgitauth"), nil) h.equals("a longer name does not match", get_cookie("xcgitauth=z", "cgitauth"), nil) h.equals("a magic character in the name is taken literally", get_cookie("a-b=z", "a-b"), "z") h.equals("spaces around a pair are ignored", get_cookie(" foo=1 ; cgitauth=abc ", "cgitauth"), "abc") -h.equals("a value keeps its own equals signs", get_cookie("cgitauth=a=b", "cgitauth"), "a=b") +h.equals("a cookie value keeps its own equals signs", get_cookie("cgitauth=a=b", "cgitauth"), "a=b") -- The Cookie header is the visitor's to write, so a run of spaces in it -- must not cost the square of its length. @@ -237,106 +237,96 @@ local function run_action(action, opts) return h.output(), ret end -local ret +-- A login posts the form, and a visit asks about a repository, with a +-- session cookie when it is given one, and comes back with the answer. +local function login(user, pass, redirect) + local body = "username=" .. url_encode(user) .. "&password=" .. url_encode(pass) + if redirect then + body = body .. "&redirect=" .. url_encode(redirect) + end + return run_action("authenticate-post", { method = "POST", body = body }) +end -out, ret = run_action("authenticate-cookie", { repo = "unlisted" }) -h.equals("a public repository needs no cookie", ret, 1) +local function visit(repo, token) + local _, ret = run_action("authenticate-cookie", + { repo = repo, cookie = token and session_name .. "=" .. token }) + return ret +end + +-- Where the signed way back in a login form leads. +local function way_back_in(form) + local token = form:match("name='redirect' value='([^']*)'") + return token and validate_value("redirect", token) +end -out, ret = run_action("authenticate-cookie", { repo = "secret-repo" }) -h.equals("a protected repository turns a bare request away", ret, 0) +h.equals("a public repository needs no cookie", visit("unlisted"), 1) +h.equals("a protected repository turns a bare request away", visit("secret-repo"), 0) local session = secure_value("username", "Alice", now + 3600, stored_hash) -out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. session }) -h.equals("a signed session for a member is let through", ret, 1) +h.equals("a signed session for a member is let through", visit("secret-repo", session), 1) -- A session is signed with its account's hash, so a new password or a -- removed account ends it. local stale = secure_value("username", "Alice", now + 3600, "$6$rounds=300000$old$hash") -out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. stale }) -h.equals("a session from before a password change is turned away", ret, 0) +h.equals("a session from before a password change is turned away", visit("secret-repo", stale), 0) local orphan = secure_value("username", "bob", now + 3600, stored_hash) -out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. orphan }) -h.equals("a session for a member with no account is turned away", ret, 0) +h.equals("a session for a member with no account is turned away", visit("secret-repo", orphan), 0) local unbound = secure_value("username", "Alice", now + 3600) h.equals("a session signed without its binding does not verify", validate_value("username", unbound, account_hash), nil) -out, ret = run_action("authenticate-cookie", { repo = "empty-repo", cookie = session_name .. "=" .. session }) -h.equals("a memberless repository denies even a valid session", ret, 0) +h.equals("a memberless repository denies even a valid session", visit("empty-repo", session), 0) local outsider = secure_value("username", "carol", now + 3600, stored_hash) -out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. outsider }) -h.equals("a signed session for an outsider is turned away", ret, 0) +h.equals("a signed session for an outsider is turned away", visit("secret-repo", outsider), 0) local forged = session:sub(1, -2) .. (session:sub(-1) == "0" and "1" or "0") -out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. forged }) -h.equals("a forged session is turned away", ret, 0) +h.equals("a forged session is turned away", visit("secret-repo", forged), 0) -out, ret = run_action("no-such-action", {}) +local _, ret = run_action("no-such-action", {}) h.equals("an unknown action denies rather than raising", ret, 0) -out, ret = run_action("body", { url = "/repo/log/?q=x" }) +out = run_action("body", { url = "/repo/log/?q=x" }) h.contains("the login form posts to the login url", out, "
") h.equals("a first visit says nothing failed", out:find("login-error", 1, true), nil) -local token = out:match("name='redirect' value='([^']*)'") -h.equals("the form carries a signed way back", token and validate_value("redirect", token), "/repo/log/?q=x") +h.equals("the form carries a signed way back", way_back_in(out), "/repo/log/?q=x") -out, ret = run_action("body", { url = "//evil.example/x" }) -token = out:match("name='redirect' value='([^']*)'") -h.equals("an unsafe destination is swapped for the login page", - token and validate_value("redirect", token), "/?p=login") +out = run_action("body", { url = "//evil.example/x" }) +h.equals("an unsafe destination is swapped for the login page", way_back_in(out), "/?p=login") h.equals("and is not taken for a failed login", out:find("login-error", 1, true), nil) -out, ret = run_action("body", { url = "/repo/log/?q=x&login=failed" }) +out = run_action("body", { url = "/repo/log/?q=x&login=failed" }) h.contains("a failed login says so", out, "") -token = out:match("name='redirect' value='([^']*)'") -h.equals("and leaves the flag out of the way back", token and validate_value("redirect", token), "/repo/log/?q=x") +h.equals("and leaves the flag out of the way back", way_back_in(out), "/repo/log/?q=x") local way_back = secure_value("redirect", "/repo/", 0) -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=" .. url_encode(password) .. "&redirect=" .. url_encode(way_back), -}) +out = login("Alice", password, way_back) h.contains("a good login redirects back", out, "Status: 302") h.contains("to where the form said", out, "Location: /repo/\n") local granted = out:match("Set%-Cookie: __Host%-cgitauth=([^;]*);") h.equals("and grants a session that verifies", granted and validate_value("username", granted, account_hash), "Alice") -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=wrong&redirect=" .. url_encode(way_back), -}) +out = login("Alice", "wrong", way_back) h.contains("a bad password redirects the same way", out, "Status: 302") h.contains("back to the page with the failure flagged", out, "Location: /repo/?login=failed\n") h.contains("but clears the session cookie", out, "Set-Cookie: " .. session_name .. "=; ") -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=nobody&password=x&redirect=" .. url_encode(way_back), -}) -h.contains("an unknown user is told nothing different", out, "Set-Cookie: " .. session_name .. "=; ") -h.contains("and lands on the same flagged page", out, "Location: /repo/?login=failed\n") - -- A failed login for a name with no account spends crypt on a configured -- hash, the same work a real check costs, so timing tells nothing. crypt_settings = {} -run_action("authenticate-post", { - method = "POST", - body = "username=nobody&password=x&redirect=" .. url_encode(way_back), -}) +out = login("nobody", "x", way_back) +h.contains("an unknown user is told nothing different", out, "Set-Cookie: " .. session_name .. "=; ") +h.contains("and lands on the same flagged page", out, "Location: /repo/?login=failed\n") h.equals("a missing account spends crypt on a configured hash", crypt_settings[1], stored_hash) if variant == "file" then h.redirect_file("/etc/cgit-auth/users", "auth-users-missing") crypt_settings = {} - run_action("authenticate-post", { - method = "POST", - body = "username=nobody&password=x&redirect=" .. url_encode(way_back), - }) + login("nobody", "x", way_back) h.equals("with no accounts at all nothing is spent", #crypt_settings, 0) h.redirect_file("/etc/cgit-auth/users", "auth-users") end @@ -344,37 +334,21 @@ end -- crypt's work grows with the password, so one past the cap is turned down -- before crypt sees it. crypt_settings = {} -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=" .. string.rep("a", 257) .. "&redirect=" .. url_encode(way_back), -}) +out = login("Alice", string.rep("a", 257), way_back) h.contains("an overlong password is turned down", out, "Set-Cookie: " .. session_name .. "=; ") h.equals("without spending crypt on it", #crypt_settings, 0) -run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=" .. string.rep("a", 256) .. "&redirect=" .. url_encode(way_back), -}) +login("Alice", string.rep("a", 256), way_back) h.equals("a password at the cap is still checked", #crypt_settings, 1) -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=" .. url_encode(password), -}) +out = login("Alice", password) h.contains("a post without the signed token is not served", out, "Status: 404") -local hijack = secure_value("redirect", "//evil.example/", 0) -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=" .. url_encode(password) .. "&redirect=" .. url_encode(hijack), -}) +out = login("Alice", password, secure_value("redirect", "//evil.example/", 0)) h.contains("even a signed unsafe destination is not followed", out, "Status: 404") -- Over plain http the cookie cannot be Secure, and so loses the __Host- name. load_script({ { "local cookie_insecure = false", "local cookie_insecure = true" } }) -out, ret = run_action("authenticate-post", { - method = "POST", - body = "username=Alice&password=" .. url_encode(password) .. "&redirect=" .. url_encode(way_back), -}) +out = login("Alice", password, way_back) h.contains("an insecure session cookie keeps its plain name", out, "Set-Cookie: cgitauth=") h.excludes("and is not marked Secure", out, "; Secure") load_script() @@ -386,10 +360,8 @@ for _, name in ipairs({ "openssl.rand", "openssl.hmac", "posix.sys.stat", "posix package.loaded[name] = nil h.preload_failure(name) end -out, ret = run_action("authenticate-cookie", { repo = "unlisted" }) -h.equals("without the libraries a public repository is served", ret, 1) -h.check("while a protected one fails closed", not pcall(run_action, - "authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. session })) +h.equals("without the libraries a public repository is served", visit("unlisted"), 1) +h.check("while a protected one fails closed", not pcall(visit, "secret-repo", session)) h.check("and so does its login form", not pcall(run_action, "body", {})) h.finish() -- cgit