fix(backend): bind session key to both secrets and guard no-op chapter saves
Final-review fix wave over the web UI branch. - sessionKey now derives from API_TOKEN and WEB_PASSWORD with a \x00 separator, so rotating the password logs every browser out too. - uiChapter only clears last_chapter_url when the number actually changes. The form is pre-filled, so a bare tap of Save resubmits the same value; that used to destroy the chapter URL silently while updated_at stayed put, degrading Continue to the series index page. - MANGA_WEB_HOST is now required by the prod override rather than falling back to manga.example.com, matching MANGA_API_HOST. - Comment fixes: static cache rationale, pruneLocked aliasing invariant, and the stale "3 routes" line in CLAUDE.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+4
-3
@@ -23,6 +23,7 @@ ALLOWED_ORIGINS=https://asuracomic.net,https://asurascans.com,https://demonicsca
|
|||||||
# Generate one: openssl rand -base64 18
|
# Generate one: openssl rand -base64 18
|
||||||
WEB_PASSWORD=
|
WEB_PASSWORD=
|
||||||
|
|
||||||
# Subdomain Traefik routes to the browser UI (prod override only). The same
|
# Subdomain Traefik routes to the browser UI (required by the prod override
|
||||||
# container also answers on MANGA_API_HOST for the userscript's API.
|
# whenever the web UI is enabled). The same container also answers on
|
||||||
# MANGA_WEB_HOST=manga.example.com
|
# MANGA_API_HOST for the userscript's API.
|
||||||
|
MANGA_WEB_HOST=manga.example.com
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ Bromite userscript (isolated world, per-site adapters, localStorage cache)
|
|||||||
-- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> SQLite (volume)
|
-- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> SQLite (volume)
|
||||||
```
|
```
|
||||||
|
|
||||||
- **Backend** (`backend/`): stdlib `net/http` (3 routes, no framework) + `modernc.org/sqlite` (pure Go, `CGO_ENABLED=0` -> static binary -> distroless/scratch image). The reverse proxy terminates TLS; the Go service listens plain `:8080`.
|
- **Backend** (`backend/`): stdlib `net/http` (a handful of routes, no framework) + `modernc.org/sqlite` (pure Go, `CGO_ENABLED=0` -> static binary -> distroless/scratch image). The reverse proxy terminates TLS; the Go service listens plain `:8080`.
|
||||||
- **Single-user store.** One `bookmarks` table keyed `<site>:<series_id>` (`asura`|`demonic`). Sync is **last-write-wins**. Schema and endpoint list are in the plan.
|
- **Single-user store.** One `bookmarks` table keyed `<site>:<series_id>` (`asura`|`demonic`). Sync is **last-write-wins**. Schema and endpoint list are in the plan.
|
||||||
- **Endpoints:** `GET /bookmarks`, `PUT /bookmarks/{key}` (upsert; see `updated_at` rule below), `DELETE /bookmarks/{key}`, `GET /healthz` (no auth).
|
- **Endpoints:** `GET /bookmarks`, `PUT /bookmarks/{key}` (upsert; see `updated_at` rule below), `DELETE /bookmarks/{key}`, `GET /healthz` (no auth).
|
||||||
- **Web UI:** the same binary serves a password-gated browser UI on a second
|
- **Web UI:** the same binary serves a password-gated browser UI on a second
|
||||||
|
|||||||
@@ -93,8 +93,13 @@ The browser UI is served by the same container on a second hostname.
|
|||||||
Leaving `WEB_PASSWORD` unset is safe: the web routes are not registered and `/`
|
Leaving `WEB_PASSWORD` unset is safe: the web routes are not registered and `/`
|
||||||
returns 404. The userscript's API on `MANGA_API_HOST` is unaffected either way.
|
returns 404. The userscript's API on `MANGA_API_HOST` is unaffected either way.
|
||||||
|
|
||||||
Sessions are signed with a key derived from `API_TOKEN`, so rotating the token
|
`MANGA_WEB_HOST` itself is required by the prod override regardless — like
|
||||||
logs every browser out. The session cookie lasts 60 days.
|
`MANGA_API_HOST`, its Traefik label has no fallback, so `docker compose up`
|
||||||
|
refuses to start without it even if `WEB_PASSWORD` is unset and the web UI is
|
||||||
|
otherwise dormant.
|
||||||
|
|
||||||
|
Sessions are signed with a key derived from `API_TOKEN` and `WEB_PASSWORD`, so
|
||||||
|
rotating either one logs every browser out. The session cookie lasts 60 days.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
+11
-5
@@ -22,11 +22,13 @@ const (
|
|||||||
sessionKeyPurpose = "mangabm-web-session-v1"
|
sessionKeyPurpose = "mangabm-web-session-v1"
|
||||||
)
|
)
|
||||||
|
|
||||||
// sessionKey derives the cookie-signing key from the API token. Sessions are
|
// sessionKey derives the cookie-signing key from both secrets. Sessions are
|
||||||
// stateless — there is no session table — so rotating API_TOKEN invalidates
|
// stateless — there is no session table — so rotating either API_TOKEN or
|
||||||
// every outstanding cookie at once.
|
// WEB_PASSWORD invalidates every outstanding cookie at once. The \x00
|
||||||
func sessionKey(apiToken string) []byte {
|
// separator prevents the concatenation ambiguity a bare apiToken+webPassword
|
||||||
sum := sha256.Sum256([]byte(apiToken + sessionKeyPurpose))
|
// would have (e.g. "ab"+"c" colliding with "a"+"bc").
|
||||||
|
func sessionKey(apiToken, webPassword string) []byte {
|
||||||
|
sum := sha256.Sum256([]byte(apiToken + "\x00" + webPassword + sessionKeyPurpose))
|
||||||
return sum[:]
|
return sum[:]
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -166,6 +168,10 @@ func (l *loginLimiter) reset(ip string) {
|
|||||||
// The caller must hold l.mu.
|
// The caller must hold l.mu.
|
||||||
func (l *loginLimiter) pruneLocked(ip string, now time.Time) []time.Time {
|
func (l *loginLimiter) pruneLocked(ip string, now time.Time) []time.Time {
|
||||||
cutoff := now.Add(-loginWindow)
|
cutoff := now.Add(-loginWindow)
|
||||||
|
// In-place filter: kept reuses the backing array of the slice being
|
||||||
|
// ranged over. The range expression captures the slice header once at
|
||||||
|
// the start, so the append cursor (kept) can never outrun the read
|
||||||
|
// cursor (the range index) — safe to alias.
|
||||||
kept := l.failures[ip][:0]
|
kept := l.failures[ip][:0]
|
||||||
for _, at := range l.failures[ip] {
|
for _, at := range l.failures[ip] {
|
||||||
if at.After(cutoff) {
|
if at.After(cutoff) {
|
||||||
|
|||||||
+14
-6
@@ -10,7 +10,7 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
func TestSessionRoundTrip(t *testing.T) {
|
func TestSessionRoundTrip(t *testing.T) {
|
||||||
key := sessionKey("token-abc")
|
key := sessionKey("token-abc", "pw-abc")
|
||||||
now := time.Now().UnixMilli()
|
now := time.Now().UnixMilli()
|
||||||
value := signSession(key, now+60_000)
|
value := signSession(key, now+60_000)
|
||||||
if !verifySession(key, value, now) {
|
if !verifySession(key, value, now) {
|
||||||
@@ -19,7 +19,7 @@ func TestSessionRoundTrip(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestSessionRejects(t *testing.T) {
|
func TestSessionRejects(t *testing.T) {
|
||||||
key := sessionKey("token-abc")
|
key := sessionKey("token-abc", "pw-abc")
|
||||||
now := time.Now().UnixMilli()
|
now := time.Now().UnixMilli()
|
||||||
valid := signSession(key, now+60_000)
|
valid := signSession(key, now+60_000)
|
||||||
payload, sig, _ := strings.Cut(valid, ".")
|
payload, sig, _ := strings.Cut(valid, ".")
|
||||||
@@ -34,7 +34,7 @@ func TestSessionRejects(t *testing.T) {
|
|||||||
{"expired", signSession(key, now-1)},
|
{"expired", signSession(key, now-1)},
|
||||||
{"tampered signature", payload + "." + flipLastChar(sig)},
|
{"tampered signature", payload + "." + flipLastChar(sig)},
|
||||||
{"tampered expiry", "99999999999999." + sig},
|
{"tampered expiry", "99999999999999." + sig},
|
||||||
{"signed with another key", signSession(sessionKey("other-token"), now+60_000)},
|
{"signed with another key", signSession(sessionKey("other-token", "pw-abc"), now+60_000)},
|
||||||
}
|
}
|
||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
@@ -57,13 +57,21 @@ func flipLastChar(s string) string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestSessionKeyDependsOnToken(t *testing.T) {
|
func TestSessionKeyDependsOnToken(t *testing.T) {
|
||||||
a := sessionKey("token-a")
|
a := sessionKey("token-a", "pw-abc")
|
||||||
b := sessionKey("token-b")
|
b := sessionKey("token-b", "pw-abc")
|
||||||
if string(a) == string(b) {
|
if string(a) == string(b) {
|
||||||
t.Fatal("sessionKey collided for different API tokens")
|
t.Fatal("sessionKey collided for different API tokens")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSessionKeyDependsOnWebPassword(t *testing.T) {
|
||||||
|
a := sessionKey("token-abc", "pw-a")
|
||||||
|
b := sessionKey("token-abc", "pw-b")
|
||||||
|
if string(a) == string(b) {
|
||||||
|
t.Fatal("sessionKey collided for different web passwords with the same API token")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestSetSessionCookieAttributes(t *testing.T) {
|
func TestSetSessionCookieAttributes(t *testing.T) {
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -86,7 +94,7 @@ func TestSetSessionCookieAttributes(t *testing.T) {
|
|||||||
r.Header.Set("X-Forwarded-Proto", tc.forwarded)
|
r.Header.Set("X-Forwarded-Proto", tc.forwarded)
|
||||||
}
|
}
|
||||||
rr := httptest.NewRecorder()
|
rr := httptest.NewRecorder()
|
||||||
setSessionCookie(rr, r, sessionKey("token-abc"))
|
setSessionCookie(rr, r, sessionKey("token-abc", "pw-abc"))
|
||||||
|
|
||||||
cookies := rr.Result().Cookies()
|
cookies := rr.Result().Cookies()
|
||||||
if len(cookies) != 1 {
|
if len(cookies) != 1 {
|
||||||
|
|||||||
+14
-8
@@ -55,7 +55,7 @@ func newWebHandler(store *Store, cfg Config) (*webHandler, error) {
|
|||||||
return &webHandler{
|
return &webHandler{
|
||||||
store: store,
|
store: store,
|
||||||
tmpl: tmpl,
|
tmpl: tmpl,
|
||||||
key: sessionKey(cfg.Token),
|
key: sessionKey(cfg.Token, cfg.WebPassword),
|
||||||
password: cfg.WebPassword,
|
password: cfg.WebPassword,
|
||||||
limiter: newLoginLimiter(),
|
limiter: newLoginLimiter(),
|
||||||
}, nil
|
}, nil
|
||||||
@@ -73,9 +73,10 @@ func (h *webHandler) register(mux *http.ServeMux) {
|
|||||||
mux.HandleFunc("DELETE /ui/bookmarks/{key}", h.requireSession(h.uiDelete))
|
mux.HandleFunc("DELETE /ui/bookmarks/{key}", h.requireSession(h.uiDelete))
|
||||||
}
|
}
|
||||||
|
|
||||||
// staticHandler serves the embedded assets. The vendored htmx build and the
|
// staticHandler serves the embedded assets. An hour, not longer: assets are
|
||||||
// stylesheet change only on deploy, so a long max-age is safe; a redeploy
|
// not fingerprinted, and embed.FS reports a zero ModTime, so http.FileServer
|
||||||
// changes the binary and the browser revalidates on its own schedule.
|
// emits no Last-Modified or ETag and a client has no way to revalidate a
|
||||||
|
// cached copy after a deploy short of waiting out max-age.
|
||||||
func staticHandler() http.Handler {
|
func staticHandler() http.Handler {
|
||||||
sub, err := fs.Sub(staticFS, "static")
|
sub, err := fs.Sub(staticFS, "static")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -253,9 +254,12 @@ func (h *webHandler) uiFavorite(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
// uiChapter forces the read chapter to a value the user typed.
|
// uiChapter forces the read chapter to a value the user typed.
|
||||||
//
|
//
|
||||||
// It clears last_chapter_url: that URL points at the chapter actually read, and
|
// When that value actually changes the number, it also clears last_chapter_url:
|
||||||
// once the number is forced elsewhere it would send the reader backwards.
|
// that URL points at the chapter actually read, and once the number is forced
|
||||||
// ContinueURL then falls back to the series page, which is always right.
|
// elsewhere it would send the reader backwards. ContinueURL then falls back to
|
||||||
|
// the series page, which is always right. Resubmitting the same number — the
|
||||||
|
// form is pre-filled, so a bare tap of Save is an easy accidental submit —
|
||||||
|
// leaves last_chapter_url untouched instead of destroying it for no reason.
|
||||||
func (h *webHandler) uiChapter(w http.ResponseWriter, r *http.Request) {
|
func (h *webHandler) uiChapter(w http.ResponseWriter, r *http.Request) {
|
||||||
b, ok := h.loadForMutation(w, r)
|
b, ok := h.loadForMutation(w, r)
|
||||||
if !ok {
|
if !ok {
|
||||||
@@ -272,9 +276,11 @@ func (h *webHandler) uiChapter(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if num != b.LastChapterNum {
|
||||||
|
b.LastChapterURL = ""
|
||||||
|
}
|
||||||
b.LastChapter = raw
|
b.LastChapter = raw
|
||||||
b.LastChapterNum = num
|
b.LastChapterNum = num
|
||||||
b.LastChapterURL = ""
|
|
||||||
b.UpdatedAt = time.Now().UnixMilli()
|
b.UpdatedAt = time.Now().UnixMilli()
|
||||||
h.saveAndRenderCard(w, b)
|
h.saveAndRenderCard(w, b)
|
||||||
}
|
}
|
||||||
|
|||||||
+35
-1
@@ -36,7 +36,7 @@ func sessionCookie(t *testing.T, cfg Config) *http.Cookie {
|
|||||||
t.Helper()
|
t.Helper()
|
||||||
return &http.Cookie{
|
return &http.Cookie{
|
||||||
Name: sessionCookieName,
|
Name: sessionCookieName,
|
||||||
Value: signSession(sessionKey(cfg.Token), time.Now().Add(time.Hour).UnixMilli()),
|
Value: signSession(sessionKey(cfg.Token, cfg.WebPassword), time.Now().Add(time.Hour).UnixMilli()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -352,6 +352,40 @@ func TestChapterOverrideMovesUpdatedAt(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestChapterOverrideNoOpPreservesURLAndUpdatedAt(t *testing.T) {
|
||||||
|
cfg := webConfig()
|
||||||
|
srv, store := newWebTestServer(t, cfg)
|
||||||
|
before := seed(t, store, Bookmark{
|
||||||
|
Key: "asura:solo", Site: "asura", SeriesID: "solo",
|
||||||
|
Title: "Solo Leveling", LastChapter: "45", LastChapterNum: 45,
|
||||||
|
LastChapterURL: "https://example.test/ch/45", SeriesURL: "https://example.test/solo",
|
||||||
|
UpdatedAt: 1_000_000,
|
||||||
|
})
|
||||||
|
|
||||||
|
// The chapter form is pre-filled with the current value, so tapping Save
|
||||||
|
// without editing resubmits the unchanged number. That must be a no-op:
|
||||||
|
// it must not silently clear last_chapter_url or move updated_at.
|
||||||
|
rr := httptest.NewRecorder()
|
||||||
|
srv.ServeHTTP(rr, uiRequest(t, cfg, http.MethodPost,
|
||||||
|
"/ui/bookmarks/asura:solo/chapter", url.Values{"chapter": {"45"}}))
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("chapter no-op status = %d, want 200", rr.Code)
|
||||||
|
}
|
||||||
|
|
||||||
|
after, ok, err := store.Get("asura:solo")
|
||||||
|
if err != nil || !ok {
|
||||||
|
t.Fatalf("Get after no-op override: %v ok=%v", err, ok)
|
||||||
|
}
|
||||||
|
if after.LastChapterURL != before.LastChapterURL {
|
||||||
|
t.Fatalf("LastChapterURL = %q, want preserved %q on a no-op save",
|
||||||
|
after.LastChapterURL, before.LastChapterURL)
|
||||||
|
}
|
||||||
|
if after.UpdatedAt != before.UpdatedAt {
|
||||||
|
t.Fatalf("UpdatedAt = %d, want unchanged %d on a no-op save",
|
||||||
|
after.UpdatedAt, before.UpdatedAt)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestChapterOverrideRejectsBadInput(t *testing.T) {
|
func TestChapterOverrideRejectsBadInput(t *testing.T) {
|
||||||
cfg := webConfig()
|
cfg := webConfig()
|
||||||
srv, store := newWebTestServer(t, cfg)
|
srv, store := newWebTestServer(t, cfg)
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
#
|
#
|
||||||
# Set in .env:
|
# Set in .env:
|
||||||
# MANGA_API_HOST=manga-api.example.com # your subdomain (required)
|
# MANGA_API_HOST=manga-api.example.com # your subdomain (required)
|
||||||
# MANGA_WEB_HOST=manga.example.com # browser UI subdomain, same container
|
# MANGA_WEB_HOST=manga.example.com # browser UI subdomain, same container (required)
|
||||||
# PROXY_NETWORK=proxy # Traefik's network name, if not "proxy"
|
# PROXY_NETWORK=proxy # Traefik's network name, if not "proxy"
|
||||||
# TRAEFIK_ENTRYPOINT=websecure # your HTTPS entrypoint name
|
# TRAEFIK_ENTRYPOINT=websecure # your HTTPS entrypoint name
|
||||||
# TRAEFIK_CERTRESOLVER=le # your ACME/cert resolver name
|
# TRAEFIK_CERTRESOLVER=le # your ACME/cert resolver name
|
||||||
@@ -30,7 +30,7 @@ services:
|
|||||||
# Second hostname for the browser UI, same container. Traefik needs the
|
# Second hostname for the browser UI, same container. Traefik needs the
|
||||||
# service named explicitly once more than one router targets it.
|
# service named explicitly once more than one router targets it.
|
||||||
- "traefik.http.routers.mangabm.service=mangabm"
|
- "traefik.http.routers.mangabm.service=mangabm"
|
||||||
- "traefik.http.routers.mangaweb.rule=Host(`${MANGA_WEB_HOST:-manga.example.com}`)"
|
- "traefik.http.routers.mangaweb.rule=Host(`${MANGA_WEB_HOST:?set MANGA_WEB_HOST in .env}`)"
|
||||||
- "traefik.http.routers.mangaweb.entrypoints=${TRAEFIK_ENTRYPOINT:-websecure}"
|
- "traefik.http.routers.mangaweb.entrypoints=${TRAEFIK_ENTRYPOINT:-websecure}"
|
||||||
- "traefik.http.routers.mangaweb.tls=true"
|
- "traefik.http.routers.mangaweb.tls=true"
|
||||||
- "traefik.http.routers.mangaweb.tls.certresolver=${TRAEFIK_CERTRESOLVER:-le}"
|
- "traefik.http.routers.mangaweb.tls.certresolver=${TRAEFIK_CERTRESOLVER:-le}"
|
||||||
|
|||||||
Reference in New Issue
Block a user