5 files changed, 77 insertions, 17 deletions
diff --git a/README.txt b/README.txt
index ceba98c..db55ec9 100644
--- a/README.txt
+++ b/README.txt
@@ -101,6 +101,8 @@ script added through the css and js options has stable hooks to work with.
   A character in a file that a browser would start a new line at, though git
   does not, such as a carriage return with no line feed after it, is shown as
   an escape sequence in span.escape.
+* The bundled auth filters write their login form as form.login, with
+  p.login-error above it after a failed login.
 * Colours, fonts and metrics are custom properties on div#cgit, so a theme can
   redefine those alone. Every rule in cgit.css starts with div#cgit, apart from
   the background and margins it sets on html.cgit-standalone, the class of the
diff --git a/assets/cgit.css b/assets/cgit.css
index 9c0cce8..cf62df8 100644
--- a/assets/cgit.css
+++ b/assets/cgit.css
@@ -1103,6 +1103,26 @@ div#cgit span.deletions {
   color: var(--del);
 }
 
+div#cgit p.login-error {
+  color: var(--del);
+  font-weight: bold;
+}
+
+div#cgit form.login table {
+  border-collapse: separate;
+  border-spacing: 0 0.5em;
+}
+
+div#cgit form.login td:first-child {
+  padding-right: 0.75em;
+}
+
+div#cgit form.login input[type="text"],
+div#cgit form.login input[type="password"] {
+  width: 16em;
+  max-width: 100%;
+}
+
 div#cgit .footer {
   padding: 0.85rem var(--gutter);
   text-align: center;
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index 354adac..a03e1ea 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -476,6 +476,18 @@ function redirect_to(url)
 	html("Location: " .. strip_controls(url) .. "\n")
 end
 
+-- A failed login lands back on the page it came from with this flag, which
+-- the form reads to say so and leaves out of the way back it signs.
+local failed_flag = "login=failed"
+
+local function flag_failure(url)
+	return url .. (url:find("?", 1, true) and "&" or "?") .. failed_flag
+end
+
+local function drop_failure_flag(url)
+	return (url:gsub("[?&]" .. failed_flag .. "$", ""))
+end
+
 function not_found()
 	html("Status: 404 Not Found\n")
 	html("Cache-Control: no-cache, no-store\n\n")
@@ -484,9 +496,8 @@ end
 -- The three actions cgit can ask for follow, and they are the same in both
 -- variants, so a change to one belongs in the other.
 
--- The redirect goes out before the password is checked, so a wrong password
--- and a right one answer with the same status and location and differ only in
--- the cookie.
+-- A wrong password and an unknown user answer alike, so the flag on the way
+-- back says only that the login failed.
 function authenticate_post()
 	local redirect = validate_value("redirect", post["redirect"])
 
@@ -495,8 +506,6 @@ function authenticate_post()
 		return 0
 	end
 
-	redirect_to(redirect)
-
 	local username = post["username"]
 	local password = post["password"]
 	local session = ""
@@ -514,6 +523,7 @@ function authenticate_post()
 	end
 
 	-- A failed login answers with an empty session, which clears the cookie.
+	redirect_to(session == "" and flag_failure(redirect) or redirect)
 	set_cookie(session_cookie, session)
 
 	html("\n")
@@ -541,13 +551,17 @@ end
 -- The form carries a signed redirect token, so the browser lands back on the
 -- page that was asked for.
 function body()
-	local target = cgit["url"]
+	local target = drop_failure_flag(cgit["url"])
+	local failed = target ~= cgit["url"]
 	if not is_safe_redirect(target) then
 		target = cgit["login"]
 	end
 
 	html("<h2>Authentication Required</h2>")
-	html("<form method='post' action='")
+	if failed then
+		html("<p class='login-error' role='alert'>Wrong username or password.</p>")
+	end
+	html("<form class='login' method='post' action='")
 	html_attr(cgit["login"])
 	html("'>")
 	html("<input type='hidden' name='redirect' value='")
@@ -560,7 +574,7 @@ function body()
 	html("<tr><td><label for='password'>Password:</label></td><td>")
 	html("<input id='password' name='password' type='password'")
 	html(" autocomplete='current-password'></td></tr>")
-	html("<tr><td colspan='2'>")
+	html("<tr><td></td><td>")
 	html("<input value='Login' type='submit'></td></tr>")
 	html("</table></form>")
 
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua
index e380733..c4b87f6 100644
--- a/custom/extensions/auth-inline.lua
+++ b/custom/extensions/auth-inline.lua
@@ -437,6 +437,18 @@ function redirect_to(url)
 	html("Location: " .. strip_controls(url) .. "\n")
 end
 
+-- A failed login lands back on the page it came from with this flag, which
+-- the form reads to say so and leaves out of the way back it signs.
+local failed_flag = "login=failed"
+
+local function flag_failure(url)
+	return url .. (url:find("?", 1, true) and "&" or "?") .. failed_flag
+end
+
+local function drop_failure_flag(url)
+	return (url:gsub("[?&]" .. failed_flag .. "$", ""))
+end
+
 function not_found()
 	html("Status: 404 Not Found\n")
 	html("Cache-Control: no-cache, no-store\n\n")
@@ -445,9 +457,8 @@ end
 -- The three actions cgit can ask for follow, and they are the same in both
 -- variants, so a change to one belongs in the other.
 
--- The redirect goes out before the password is checked, so a wrong password
--- and a right one answer with the same status and location and differ only in
--- the cookie.
+-- A wrong password and an unknown user answer alike, so the flag on the way
+-- back says only that the login failed.
 function authenticate_post()
 	local redirect = validate_value("redirect", post["redirect"])
 
@@ -456,8 +467,6 @@ function authenticate_post()
 		return 0
 	end
 
-	redirect_to(redirect)
-
 	local username = post["username"]
 	local password = post["password"]
 	local session = ""
@@ -475,6 +484,7 @@ function authenticate_post()
 	end
 
 	-- A failed login answers with an empty session, which clears the cookie.
+	redirect_to(session == "" and flag_failure(redirect) or redirect)
 	set_cookie(session_cookie, session)
 
 	html("\n")
@@ -502,13 +512,17 @@ end
 -- The form carries a signed redirect token, so the browser lands back on the
 -- page that was asked for.
 function body()
-	local target = cgit["url"]
+	local target = drop_failure_flag(cgit["url"])
+	local failed = target ~= cgit["url"]
 	if not is_safe_redirect(target) then
 		target = cgit["login"]
 	end
 
 	html("<h2>Authentication Required</h2>")
-	html("<form method='post' action='")
+	if failed then
+		html("<p class='login-error' role='alert'>Wrong username or password.</p>")
+	end
+	html("<form class='login' method='post' action='")
 	html_attr(cgit["login"])
 	html("'>")
 	html("<input type='hidden' name='redirect' value='")
@@ -521,7 +535,7 @@ function body()
 	html("<tr><td><label for='password'>Password:</label></td><td>")
 	html("<input id='password' name='password' type='password'")
 	html(" autocomplete='current-password'></td></tr>")
-	html("<tr><td colspan='2'>")
+	html("<tr><td></td><td>")
 	html("<input value='Login' type='submit'></td></tr>")
 	html("</table></form>")
 
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua
index 0115716..742eabe 100644
--- a/tests/extensions/test-auth.lua
+++ b/tests/extensions/test-auth.lua
@@ -278,7 +278,8 @@ out, 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" })
-h.contains("the login form posts to the login url", out, "<form method='post' action='/?p=login'>")
+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")
 
@@ -286,6 +287,13 @@ 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")
+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" })
+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")
 
 local way_back = secure_value("redirect", "/repo/", 0)
 
@@ -303,6 +311,7 @@ out, ret = run_action("authenticate-post", {
 	body = "username=Alice&password=wrong&redirect=" .. url_encode(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", {
@@ -310,6 +319,7 @@ out, ret = run_action("authenticate-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.
-rw-r--r--	README.txt	2
-rw-r--r--	assets/cgit.css	20
-rw-r--r--	custom/extensions/auth-file.lua	30
-rw-r--r--	custom/extensions/auth-inline.lua	30
-rw-r--r--	tests/extensions/test-auth.lua	12