Fix silent login loop caused by strict cookie parsing
The admin UI accepted the password, issued a valid session cookie, then
bounced straight back to the login page with no error shown.
http.cookies.SimpleCookie.load() silently discards the remainder of a Cookie
header the moment it meets a value it considers illegal, rather than raising
or skipping just that entry. A neighbouring cookie with a JSON-ish value such
as prefs={"a":1} is enough. Everything after it — including our session —
becomes invisible, so a perfectly good login looked like a failed one.
curl never showed it because curl only sends the one cookie. A browser sends
every cookie on the domain, and tube.jihakuz.xyz previously served
TubeArchivist alongside several sibling services on jihakuz.xyz.
Replaced with a tolerant hand-rolled parser and covered it with regression
tests for nine hostile neighbour values.
Also log successful logins at INFO. Only failures were logged, which made
"password rejected" and "session did not stick" indistinguishable from the
journal and sent the diagnosis down the wrong path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
18bb2e420b
commit
96f8d89442
@@ -129,6 +129,24 @@ def clear_cookie_header() -> str:
|
||||
return f"{COOKIE_NAME}=; Path=/; HttpOnly; SameSite=Lax; Max-Age=0"
|
||||
|
||||
|
||||
def cookie_value(header: str, name: str = COOKIE_NAME) -> str:
|
||||
"""Pull one cookie out of a request's Cookie header.
|
||||
|
||||
Deliberately hand-rolled rather than using http.cookies.SimpleCookie. That
|
||||
parser silently discards the remainder of the header the moment it meets a
|
||||
value it considers illegal — a JSON-ish value such as `prefs={"a":1}` is
|
||||
enough — so any cookie appearing after it becomes invisible. A browser sends
|
||||
us every cookie on the domain, including ones set by unrelated services, so
|
||||
one stray value would otherwise make a perfectly valid session vanish and
|
||||
bounce the user back to the login page with no error shown.
|
||||
"""
|
||||
for part in (header or "").split(";"):
|
||||
candidate, separator, value = part.strip().partition("=")
|
||||
if separator and candidate.strip() == name:
|
||||
return value.strip().strip('"')
|
||||
return ""
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# CSRF
|
||||
|
||||
|
||||
@@ -8,7 +8,6 @@ throttling.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import http.cookies
|
||||
import json
|
||||
import logging
|
||||
import subprocess
|
||||
@@ -89,16 +88,7 @@ class Handler(BaseHTTPRequestHandler):
|
||||
}
|
||||
|
||||
def _cookie_token(self) -> str:
|
||||
header = self.headers.get("Cookie")
|
||||
if not header:
|
||||
return ""
|
||||
jar = http.cookies.SimpleCookie()
|
||||
try:
|
||||
jar.load(header)
|
||||
except http.cookies.CookieError:
|
||||
return ""
|
||||
morsel = jar.get(auth.COOKIE_NAME)
|
||||
return morsel.value if morsel else ""
|
||||
return auth.cookie_value(self.headers.get("Cookie") or "")
|
||||
|
||||
# ------------------------------------------------------------- session
|
||||
|
||||
@@ -237,8 +227,15 @@ class Handler(BaseHTTPRequestHandler):
|
||||
)
|
||||
|
||||
stored = settings.raw("admin_password_hash")
|
||||
if not stored:
|
||||
log.warning("login attempted from %s but no password is set", key)
|
||||
|
||||
if stored and auth.verify_password(stored, form.get("password", "")):
|
||||
self.server.throttle.record_success(key)
|
||||
# Logged at INFO so "did my login work?" is answerable from the
|
||||
# journal. A rejected login only ever re-renders the form, which
|
||||
# looks identical to a session that failed to stick.
|
||||
log.info("successful login from %s", key)
|
||||
token = auth.issue_session(self._secret(settings))
|
||||
return self._redirect(
|
||||
"/",
|
||||
|
||||
Reference in New Issue
Block a user