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.

1 file changed, 49 insertions, 77 deletions

diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.luaindex 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, "<form class='login' method='post' action='/?p=login'>") 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, 	"<p class='login-error' role='alert'>Wrong username or password.</p>")-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()
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, "<form class='login' method='post' action='/?p=login'>")
 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,
 	"<p class='login-error' role='alert'>Wrong username or password.</p>")
-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()