Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Drop auth guards for input cgit never sends
cgit passes an empty string, never nil, when a request names no
repository, so the check that the index stays up proved nothing, and
auth-file in fact refuses the index too while its repos file cannot be
read. A session_seconds of 0 signs a session that has expired by the
time it comes back, so it needs no branch of its own in set_cookie.
| -rw-r--r-- | custom/extensions/auth-file.lua | 14 | |
| -rw-r--r-- | custom/extensions/auth-inline.lua | 10 | |
| -rw-r--r-- | tests/extensions/test-auth.lua | 7 |
3 files changed, 10 insertions, 21 deletions
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua index 96c2074..c0341e5 100644 --- a/custom/extensions/auth-file.lua +++ b/custom/extensions/auth-file.lua @@ -79,8 +79,8 @@ local groups_filename = "/etc/cgit-auth/groups" -- Per-repository access lives one per line as reponame:group1,group2 and so -- on. A repository named here is protected and one that is not named is -- public. The repository name has to match exactly, while group and user names --- match regardless of case. While this file cannot be read every repository --- is refused, since nothing then tells the protected ones apart. +-- match regardless of case. While this file cannot be read every page, the +-- index too, is refused, since nothing then tells the protected ones apart. local repos_filename = "/etc/cgit-auth/repos" -- Where the cookie-signing secret is stored, created on first use. It must be @@ -152,9 +152,6 @@ end -- a failed login can spend the same work on a hash of the same kind and -- cost, and its timing does not tell whether the name exists. function account_hash(user) - if user == nil then - return nil - end local wanted = user:lower() local users_file = io.open(users_filename, "r") if users_file == nil then @@ -180,9 +177,6 @@ end -- repository whose groups resolve to nobody comes back as an empty table -- instead, so it denies everyone rather than falling through to public. function repo_userset(repo) - if repo == nil then - return nil - end local groups = nil local repos_file, err = io.open(repos_filename, "r") if repos_file == nil then @@ -415,7 +409,7 @@ function set_cookie(cookie, value) end if value == "" then attributes = attributes .. "; Max-Age=0" - elseif session_seconds > 0 then + else attributes = attributes .. "; Max-Age=" .. tostring(session_seconds) end html("Set-Cookie: " .. cookie .. "=" .. value .. attributes .. "\n") @@ -454,7 +448,7 @@ end function authenticate_post() local redirect = validate_value("redirect", post["redirect"]) - if redirect == nil or not is_safe_redirect(redirect) then + if not is_safe_redirect(redirect) then not_found() return 0 end diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua index 6c0ad3b..b3ecaf9 100644 --- a/custom/extensions/auth-inline.lua +++ b/custom/extensions/auth-inline.lua @@ -141,9 +141,6 @@ end -- a failed login can spend the same work on a hash of the same kind and -- cost, and its timing does not tell whether the name exists. function account_hash(user) - if user == nil then - return nil - end local hash = users[user:lower()] if hash ~= nil then return hash @@ -155,9 +152,6 @@ end -- The users allowed into a repository come back keyed by lowercased name, and -- a repository the table does not name comes back as nil. function repo_userset(repo) - if repo == nil then - return nil - end return protected_repos[repo] end @@ -376,7 +370,7 @@ function set_cookie(cookie, value) end if value == "" then attributes = attributes .. "; Max-Age=0" - elseif session_seconds > 0 then + else attributes = attributes .. "; Max-Age=" .. tostring(session_seconds) end html("Set-Cookie: " .. cookie .. "=" .. value .. attributes .. "\n") @@ -415,7 +409,7 @@ end function authenticate_post() local redirect = validate_value("redirect", post["redirect"]) - if redirect == nil or not is_safe_redirect(redirect) then + if not is_safe_redirect(redirect) then not_found() return 0 end diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua index 07c7341..77cc605 100644 --- a/tests/extensions/test-auth.lua +++ b/tests/extensions/test-auth.lua @@ -94,7 +94,6 @@ h.equals("an account is found whatever its case", hash, stored_hash) h.equals("an unknown account is not", account_hash("nobody"), nil) local _, stand_in = account_hash("nobody") h.equals("it brings another account's hash to spend instead", stand_in, stored_hash) -h.equals("a nil user is not", account_hash(nil), nil) local userset = repo_userset("secret-repo") h.check("a protected repository lists its users", @@ -113,12 +112,14 @@ if variant == "file" then h.equals("a missing users file turns every login down", account_hash("alice"), nil) h.redirect_file("/etc/cgit-auth/users", "auth-users") -- Without the repos file nothing tells a protected repository from a - -- public one, so every repository is refused while the index stays up. + -- public one, so every page is refused, the index too, which cgit asks + -- about with an empty repository name. h.redirect_file("/etc/cgit-auth/repos", "auth-repos-missing") local refused = repo_userset("unlisted") h.check("a missing repos file refuses every repository", refused ~= nil and next(refused) == nil) - h.equals("the index stays public all the same", repo_userset(nil), nil) + local index = repo_userset("") + h.check("and the index as well", index ~= nil and next(index) == nil) h.redirect_file("/etc/cgit-auth/repos", "auth-repos") end |