From 0b946ee2c4b194252ccc17763beac0d1641f144d Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sun, 16 Aug 2026 14:17:36 +0700 Subject: [PATCH] test, docs: admin page gate tests and the page's written rules (#102) - web_test.go walks web.AdminPatterns() rather than naming routes by hand, so a new administrative route that forgets requireOwner fails the gate test instead of shipping open. - The harnesses take a LaneReporter; a fake one keeps the page's tests free of a poller and a Site. - backend/AGENTS.md records the adminRoutes/requireOwner rule and the nil-poller trap; design-system.md records --patina and the admin page's shape. --- backend/AGENTS.md | 16 ++- backend/api_test.go | 8 +- backend/cover_test.go | 2 +- backend/reader_credential_test.go | 6 +- backend/web_test.go | 197 +++++++++++++++++++++++++++--- docs/design-system.md | 20 ++- 6 files changed, 218 insertions(+), 31 deletions(-) diff --git a/backend/AGENTS.md b/backend/AGENTS.md index f365f31..8315b44 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -211,6 +211,16 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN `POST /rotate-token` (atomic epoch bump + hash rewrite; invalidates every installed copy, so the panel warns to reinstall on all devices). - Owner-only `POST /readers/{id}/revoke` (drops one Reader's session rows and - re-renders the `readers` panel; 404 for any non-owner) is the only route that - reaches across Readers. +- **Owner-only admin page (`internal/web/admin.go`, issue #102):** `GET /admin` + carries the Reader roster (sessions, Sighting counters, `POST + /readers/{id}/revoke` and `POST /readers/{id}/clear-marks`) and Poll Lane + status (`GET /ui/admin/lanes`, self-refreshing every 30s). Every route that + reaches past the acting Reader is listed in `adminRoutes()` and wrapped in + `requireOwner` at registration — add a route there, not a check inside a + handler; `web.AdminPatterns()` is what the gate test walks. A non-owner gets + 404, never 403. Lane figures come from the running poller through the + `web.LaneReporter` seam (`latest.Poller.LaneStatus`), never from a table: a + nil reporter or a Lane that has not finished a pass renders "no data yet" + rather than zeroes. `main.newRouter` takes the reporter as an interface and + converts a nil `*Poller` to a nil interface — a typed nil would make the page + claim a poller exists. diff --git a/backend/api_test.go b/backend/api_test.go index 44880d4..d2458db 100644 --- a/backend/api_test.go +++ b/backend/api_test.go @@ -48,7 +48,7 @@ func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) } func newTestServer(t *testing.T) http.Handler { t.Helper() - return newRouter(newTestStore(t), testConfig()) + return newRouter(newTestStore(t), testConfig(), nil) } func newTestStore(t *testing.T) *store.Store { @@ -599,7 +599,7 @@ func TestLoadConfigDiscord(t *testing.T) { // cooldown and the poller would re-fetch that series on every single tick. func TestPutDoesNotClobberLatestCheckedAt(t *testing.T) { s := newTestStore(t) - srv := newRouter(s, testConfig()) + srv := newRouter(s, testConfig(), nil) seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 777) @@ -637,7 +637,7 @@ func TestUserscriptServedWithWebUIDisabled(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/u/"+ownerCredential()+"/manga-bookmark.user.js", nil) - newRouter(s, cfg).ServeHTTP(rr, req) + newRouter(s, cfg, nil).ServeHTTP(rr, req) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200", rr.Code) } @@ -659,7 +659,7 @@ func TestNovelUserscriptServed(t *testing.T) { cfg := testConfig() cfg.NovelUserscriptPath = novelPath - srv := newRouter(s, cfg) + srv := newRouter(s, cfg, nil) rr := httptest.NewRecorder() srv.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, diff --git a/backend/cover_test.go b/backend/cover_test.go index 9c3c93f..77ea269 100644 --- a/backend/cover_test.go +++ b/backend/cover_test.go @@ -98,7 +98,7 @@ func TestPublicCoverNeverEchoesNonImage(t *testing.T) { if _, err := db.Exec(`UPDATE covers SET content_type = 'text/html' WHERE address = $1`, address); err != nil { t.Fatalf("poison row: %v", err) } - rr := getCover(t, newRouter(st, testConfig()), "/covers/"+address, nil) + rr := getCover(t, newRouter(st, testConfig(), nil), "/covers/"+address, nil) if rr.Code == http.StatusOK { t.Fatalf("status = 200, want a refusal for a non-image row (body %q)", rr.Body.String()) } diff --git a/backend/reader_credential_test.go b/backend/reader_credential_test.go index e66970c..03d2a85 100644 --- a/backend/reader_credential_test.go +++ b/backend/reader_credential_test.go @@ -49,7 +49,7 @@ func withBody(req *http.Request, body string) *http.Request { // A refused credential is refused however plausible it looks: only a hash the // readers table holds authenticates anything. func TestUnknownCredentialRejected(t *testing.T) { - srv := newRouter(newTestStore(t), testConfig()) + srv := newRouter(newTestStore(t), testConfig(), nil) rr := httptest.NewRecorder() srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", readerCredential("never-registered"))) @@ -69,7 +69,7 @@ func TestUnknownCredentialRejected(t *testing.T) { func TestPerReaderIsolation(t *testing.T) { s := newTestStore(t) registerReader(t, s, "other-reader") - srv := newRouter(s, testConfig()) + srv := newRouter(s, testConfig(), nil) ownerKey := "asura:solo" putBookmark(t, srv, ownerKey, store.Bookmark{ @@ -267,7 +267,7 @@ func TestRotateCredentialViaWebUI(t *testing.T) { } cfg := testConfig() cfg.UserscriptPath = path - srv := newRouter(s, cfg) + srv := newRouter(s, cfg, nil) oldCred := ownerCredential() rr := httptest.NewRecorder() diff --git a/backend/web_test.go b/backend/web_test.go index eb3c704..4e0b9e8 100644 --- a/backend/web_test.go +++ b/backend/web_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "bookmarkmanager/backend/internal/latest" "bookmarkmanager/backend/internal/session" "bookmarkmanager/backend/internal/store" "bookmarkmanager/backend/internal/web" @@ -26,11 +27,17 @@ import ( const testOwnerID = "owner-snowflake" // newWebTestServer returns the full router plus the store behind it, so tests -// can seed rows and assert on what the handlers wrote back. -func newWebTestServer(t *testing.T, cfg Config) (http.Handler, *store.Store) { +// can seed rows and assert on what the handlers wrote back. An optional lane +// reporter stands in for the running poller; omitted means none is running, +// which is what every test that is not about the admin page wants. +func newWebTestServer(t *testing.T, cfg Config, lanes ...web.LaneReporter) (http.Handler, *store.Store) { t.Helper() st := newTestStore(t) - return newRouter(st, cfg), st + var reporter web.LaneReporter + if len(lanes) > 0 { + reporter = lanes[0] + } + return newRouter(st, cfg, reporter), st } // sessionCookie mints a live session row for the owner and returns the cookie @@ -141,12 +148,12 @@ func discordConfig(stubURL string) web.DiscordConfig { // oauthWebTestServer returns the full router, its store, and a Discord stub // wired as the configured API — the starting point for sign-in tests. -func oauthWebTestServer(t *testing.T) (http.Handler, *store.Store, *discordStub) { +func oauthWebTestServer(t *testing.T, lanes ...web.LaneReporter) (http.Handler, *store.Store, *discordStub) { t.Helper() stub, srv := newDiscordStub(t) cfg := testConfig() cfg.Discord = discordConfig(srv.URL) - router, st := newWebTestServer(t, cfg) + router, st := newWebTestServer(t, cfg, lanes...) return router, st, stub } @@ -410,7 +417,7 @@ func TestDiscordLoginRefusesNonMember(t *testing.T) { cfg.Discord = discordConfig(srv.URL) cfg.Discord.RequiredRole = tc.require st := newTestStore(t) - router := newRouter(st, cfg) + router := newRouter(st, cfg, nil) rr := completeSignIn(t, router, startSignIn(t, router)) if rr.Code != http.StatusForbidden { @@ -626,24 +633,52 @@ func TestOwnerRevokesAnotherReadersSessions(t *testing.T) { } } -// The owner's own page carries the roster; nobody else's does. -func TestOwnerSeesReadersPanel(t *testing.T) { +// fakeLanes is the admin page's poller stand-in: one fixed snapshot, so the +// page's tests need neither a poller nor a Site. +type fakeLanes struct{ status latest.Status } + +func (f fakeLanes) LaneStatus() latest.Status { return f.status } + +// The roster moved off the reading page onto its own address: the owner gets a +// link, everyone else gets nothing, and the page itself lists every Reader with +// the counters and the two controls. +func TestAdminPageCarriesRosterAndOwnerLink(t *testing.T) { router, st, _ := oauthWebTestServer(t) - signInCookie(t, router) + theirCookie := signInCookie(t, router) + ownerCookie := sessionCookie(t, st) req := httptest.NewRequest(http.MethodGet, "/", nil) - req.AddCookie(sessionCookie(t, st)) + req.AddCookie(ownerCookie) rr := httptest.NewRecorder() router.ServeHTTP(rr, req) body := rr.Body.String() - if !strings.Contains(body, `id="readers"`) { - t.Fatal("the owner's page lacks the Readers panel") + if strings.Contains(body, `id="readers"`) { + t.Error("the reading page still carries the roster; it belongs on /admin") } - if !strings.Contains(body, testOwnerID) { - t.Fatalf("the roster does not list the registered Reader:\n%s", body) + if !strings.Contains(body, `href="/admin"`) { + t.Error("the owner's reading page offers no link to the admin page") } - if !strings.Contains(body, "Revoke sessions") { - t.Fatal("the roster offers no revocation control for a signed-in Reader") + + req = httptest.NewRequest(http.MethodGet, "/", nil) + req.AddCookie(theirCookie) + rr = httptest.NewRecorder() + router.ServeHTTP(rr, req) + if strings.Contains(rr.Body.String(), `href="/admin"`) { + t.Error("a non-owner was offered the admin link") + } + + req = httptest.NewRequest(http.MethodGet, "/admin", nil) + req.AddCookie(ownerCookie) + rr = httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("GET /admin status = %d, want 200", rr.Code) + } + body = rr.Body.String() + for _, want := range []string{`id="readers"`, testOwnerID, "Revoke sessions", "Clear marks", "confirmed"} { + if !strings.Contains(body, want) { + t.Errorf("admin page lacks %q:\n%s", want, body) + } } // Exactly one revocable row: the other Reader's. The owner's own row carries // the same session count and no button. @@ -652,6 +687,136 @@ func TestOwnerSeesReadersPanel(t *testing.T) { } } +// Every administrative route is gated the same way, so the test walks the list +// the router registers rather than naming routes by hand: no session is 401, +// a signed-in non-owner is 404, and the address is not confirmed to either. +func TestAdminRoutesAreOwnerOnly(t *testing.T) { + router, st, _ := oauthWebTestServer(t) + theirCookie := signInCookie(t, router) + ownerCookie := sessionCookie(t, st) + target := strconv.FormatInt(st.OwnerID(), 10) + + patterns := web.AdminPatterns() + if len(patterns) == 0 { + t.Fatal("no administrative routes to test") + } + for _, pattern := range patterns { + method, path, ok := strings.Cut(pattern, " ") + if !ok { + t.Fatalf("route pattern %q has no method", pattern) + } + path = strings.Replace(path, "{id}", target, 1) + + for _, tc := range []struct { + name string + cookie *http.Cookie + want int + }{ + {"no session", nil, http.StatusUnauthorized}, + {"non-owner", theirCookie, http.StatusNotFound}, + } { + req := httptest.NewRequest(method, path, nil) + if tc.cookie != nil { + req.AddCookie(tc.cookie) + } + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != tc.want { + t.Errorf("%s %s as %s: status = %d, want %d", method, path, tc.name, rr.Code, tc.want) + } + } + + req := httptest.NewRequest(method, path, nil) + req.AddCookie(ownerCookie) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code == http.StatusUnauthorized { + t.Errorf("%s %s as the owner: status = 401, the gate rejects the owner", method, path) + } + } +} + +// The Lane block reports what the poller says, and marks the Lanes that need +// attention — a clamped gap, a refusal, or a Site whose pages can only be read +// through a sidecar that is not there. +func TestAdminPageShowsLaneStatus(t *testing.T) { + lanes := fakeLanes{latest.Status{ + Lanes: []latest.LaneState{ + {Site: "asura", Due: 12, LastRun: time.Now().Add(-90 * time.Second), Gap: 40 * time.Second}, + {Site: "kagane", Due: 3, LastRun: time.Now().Add(-time.Minute), Gap: time.Minute, Browser: true}, + {Site: "demonic", Due: 400, LastRun: time.Now(), Gap: 8 * time.Second, Clamped: true}, + }, + BrowserConfigured: true, + }} + router, st, _ := oauthWebTestServer(t, lanes) + + req := httptest.NewRequest(http.MethodGet, "/ui/admin/lanes", nil) + req.AddCookie(sessionCookie(t, st)) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("GET /ui/admin/lanes status = %d, want 200", rr.Code) + } + body := rr.Body.String() + for _, want := range []string{"asura", "kagane", "12 due", "gap 40s", "ran 1m30s ago", "gap at floor", "no browser", "unreachable"} { + if !strings.Contains(body, want) { + t.Errorf("lane status lacks %q:\n%s", want, body) + } + } + // Two Lanes need attention: the clamped one and the one cut off from the + // sidecar. The healthy Lane must not be marked. + if n := strings.Count(body, `class="attention"`); n != 2 { + t.Errorf("attention rows = %d, want 2:\n%s", n, body) + } +} + +// No poller and a poller that has not finished a pass are the same to the page: +// it says so rather than drawing zeroes that read as a stopped backend. +func TestAdminPageWithoutAPollerSaysSo(t *testing.T) { + router, st, _ := oauthWebTestServer(t) + req := httptest.NewRequest(http.MethodGet, "/admin", nil) + req.AddCookie(sessionCookie(t, st)) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + body := rr.Body.String() + if !strings.Contains(body, "No data yet") { + t.Errorf("admin page with no poller does not say so:\n%s", body) + } + if !strings.Contains(body, "not configured") { + t.Errorf("admin page does not report the missing browser sidecar:\n%s", body) + } +} + +// Clearing a Reader's marks answers with the whole roster, so the page cannot +// keep showing the record that was just wiped. +func TestOwnerClearsReaderMarks(t *testing.T) { + router, st, _ := oauthWebTestServer(t) + theirCookie := signInCookie(t, router) + their, _, err := st.GetSession(theirCookie.Value, time.Now()) + if err != nil { + t.Fatalf("GetSession: %v", err) + } + + req := httptest.NewRequest(http.MethodPost, + "/readers/"+strconv.FormatInt(their.ReaderID, 10)+"/clear-marks", nil) + req.AddCookie(sessionCookie(t, st)) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("clear marks: status = %d, want 200 (body %s)", rr.Code, rr.Body.String()) + } + body := rr.Body.String() + if !strings.Contains(body, `id="readers"`) { + t.Fatalf("clear marks did not re-render the roster:\n%s", body) + } + if !strings.Contains(body, "0 confirmed / 0 contradicted") { + t.Errorf("roster does not report the cleared counters:\n%s", body) + } + if strings.Contains(body, "deferral blocked") { + t.Errorf("a cleared Reader is still marked blocked:\n%s", body) + } +} + func TestDiscordLoginTokenEndpointDown(t *testing.T) { stub, srv := newDiscordStub(t) stub.tokenStatus = http.StatusInternalServerError diff --git a/docs/design-system.md b/docs/design-system.md index 9f4aeb7..1fca74b 100644 --- a/docs/design-system.md +++ b/docs/design-system.md @@ -10,7 +10,7 @@ Implemented in: | Surface | Files | | --- | --- | -| Web UI (login, list, card, empty, errors) | `backend/internal/web/static/style.css`, `backend/internal/web/templates/{app,card,list,login,chrome,icons}.html`, `backend/internal/web/static/filter.js` | +| Web UI (login, list, card, empty, errors, admin) | `backend/internal/web/static/style.css`, `backend/internal/web/templates/{app,admin,lanes,readers,card,list,login,chrome,icons}.html`, `backend/internal/web/static/filter.js` | | Userscript panel (Shadow DOM) | `userscript/manga-bookmark.user.js` — `TEMPLATE` and `CSS` at the bottom of the IIFE | ## 1. The one idea @@ -73,6 +73,7 @@ Defined once in `backend/internal/web/static/style.css` `:root`, mirrored in the | `--moss` | `#7fae86` | `#3d6c46` | finished accent | | `--clay` | `#b5906f` | `#7c5533` | set-chapter accent | | `--trash` | `#977671` | `#8c6558` | remove, at rest — icons need 3:1, not 4.5:1 | +| `--patina` | `#b08a4a` | `#7a5a1e` | admin page only — a Poll Lane needing attention, a Reader whose reports are blocked | | `--play-hot-line` | `#3a1d18` | `#f0cfc6` | desktop cell border, play when `.is-new` | | `--fav-line` | `#332b14` | `#e3d3a4` | desktop cell border, favourite when on | | `--asura` | `#7d93a5` | `#4f6b80` | site tag | @@ -81,9 +82,12 @@ Defined once in `backend/internal/web/static/style.css` `:root`, mirrored in the | `--kagane` | `#9a8aa5` | `#6f5f7d` | site tag | | `--hatch` / `--hatch-dim` | 135° 5px stripe | paper stripe | missing-cover slot | -`--slate`/`--moss`/`--clay`/`--brass` are held at the same weight deliberately: -one accent per action, so a press says which lane it belongs to, with none of -them competing with ember. Dark is the default (`color-scheme: dark light`); +`--slate`/`--moss`/`--clay`/`--brass`/`--patina` are held at the same weight +deliberately: one accent per meaning, so a press says which lane it belongs to, +with none of them competing with ember. `--patina` is the admin page's only +colour — system health is neither a new chapter nor destruction, so it borrows +neither `--ember` nor `--danger`. +Dark is the default (`color-scheme: dark light`); light is a `@media (prefers-color-scheme: light)` override of the same names. **Any new colour must be added in both branches** — light is not a filter over dark, the hues are re-tuned. @@ -140,6 +144,14 @@ Recurring specs (copy these rather than inventing sizes): main#list article.card … | .empty ``` +The owner's admin page (`admin.html`) is the same sheet with two sections in +place of the list — `.lanes` (Poll Lane rows) and `.readers` (the roster) — +and no library switch: it belongs to neither library, so its topbar carries a +plain `.ghost.back` link home. Both sections are eyebrow + hairline-separated +rows, the shape the roster already had as a fold-out. `.lanes` refreshes itself +every 30s via `hx-get="/ui/admin/lanes"` with `hx-swap="outerHTML"`; the roster +re-renders only in answer to an action. + **Brand mark**: an inline `` (`viewBox="0 0 200 172"`), defined once in `chrome.html`'s `mark` template and reused by `app.html` and `login.html` so it takes the page's `--ink`/`currentColor`/`--ember` rather