diff --git a/BUILD_PLAN.md b/BUILD_PLAN.md index 3c27394..fbb61e2 100644 --- a/BUILD_PLAN.md +++ b/BUILD_PLAN.md @@ -135,7 +135,8 @@ Multi-session build. **Source of truth for what's done and what's next.** Update - [x] Verified: tsc clean, vite build OK, companion vitest 45/45. **Real-browser screenshots** (local Playwright + Chromium, clock mocked to 23:30): day = warm cream + pink sakura petals; night = dark plum-indigo + twinkling stars + glowing sleepy kitten. Both pretty (acceptance criterion). ### Deferred (post-v1-local) -- [ ] Authentik OIDC auth + session middleware ← **on hold: user doing foundational work first** +- [x] **Multi-user groundwork** (2026-07-26) — request-scoped identity. New `internal/auth`: `Middleware(Resolver)` resolves the caller once per API request and stores the id in the context; handlers read it via `auth.UserID(r.Context())` instead of naming `db.LocalUserID`. `StaticResolver(db.LocalUserID)` keeps Petal single-user today. **Auth itself is still deferred** — but every query is now scoped to whoever the resolver says is calling, so landing Authentik is a one-line change in `main.go` plus a `Resolver` implementation. +- [ ] Authentik OIDC auth + session middleware ← **next step: write a `Resolver` that validates a session cookie; the plumbing is in place** - [ ] Copyleaks Tier-2 + webhook HMAC - [ ] Dockerfile, docker-compose, Traefik, deploy to write.parodia.dev @@ -147,6 +148,7 @@ Multi-session build. **Source of truth for what's done and what's next.** Update - [x] **Phase 14 — companion warmth + bedtime nag + night mode**: more encouraging phrases, a gentle "go to bed" nudge after 11pm, and a calm dark theme + falling stars at night. ✅ (see Phase 14 above) ## Session log +- 2026-07-26: **Multi-user groundwork** (user: "let's start preparing Petal for multi-user support"; scope agreed as plumbing-only, aimed at Authentik). New **`internal/auth`** package — context-carried identity (`WithUser`/`UserID`), a `Resolver` seam (`Resolve(*http.Request) (string, error)`), `StaticResolver` for today's single user, and `Middleware` that 401s anything unresolved. `main.go` splits `/api` into a **public group** (`/health`, `/version` — a monitoring probe must not need a session) and an **authenticated group** carrying everything else. All ~35 `db.LocalUserID` call sites across `docs`/`suggestions`/`vocab` now read the caller from the request; helpers that had no request in scope (`fetch`, `ownsDoc`, `ownsTag`, `tagsByDoc`, `fetchVersion`, `passportVersions`, `fetchPending`, vocab `fetch`) take an explicit `userID` param. `UserID` returns `""` rather than panicking when middleware is absent, so a mis-wired route **fails closed** (every query is `WHERE user_id = ?` → matches nothing). **Two real access-control gaps found and fixed while threading**: `setStatus` (accept/dismiss) updated a suggestion by bare id with **no ownership check at all**, and `fetchPending`/`listForDoc` read a document's suggestions by `doc_id` alone — a leak of the quoted source sentences. Both now scope through `documents.user_id`. **A third bug was caught by the new tests, not by the compiler**: `docs.fetch` gained a `userID` parameter but kept binding `db.LocalUserID` in the query — legal Go (unused params compile), silently unscoped, and it would have shipped. New tests: `internal/auth/auth_test.go` (round-trip, absent-context, both 401 paths) and **two-user isolation suites** (`docs/isolation_test.go`, `suggestions/isolation_test.go`) that mount the same routers twice behind two resolvers over one DB and assert a stranger gets 404 on get/update/delete/export/passport/snapshot/version-preview/restore/tag-assign/tag-rename/tag-delete/suggestion-accept/dismiss, sees nothing in list/search/version-list, and leaves the owner's data untouched. go build/vet/test all clean. **Still global, deliberately out of scope** (flagged for the auth phase): the image store is content-addressed with no per-user association or DB row — any authenticated user holding a hash can fetch any image (capability-URL security, needs a table + migration to fix); `export-all` is correctly scoped; frontend `localStorage` keys (`petal.spell.personal`, `petal.companion`, sound/petals prefs) are per-browser, not per-account, so they'd bleed across users sharing a device. - 2026-06-26: **Phases 12 + 13 complete** (collocation coach + vocabulary garden — "finish the rest of the build plan except Authentik/Traefik"). **Phase 12**: collocation drops in as a third suggestion family reusing the whole `runPass`/`pendingScope` machinery — `llm/collocation.go` (`RunCollocation`, 25s floor, reuses `ParseCheckpoint`), `collocationSystemPrompt`/`CollocationMessages` (warm "Natives usually say…" + Mandarin gloss, defers grammar elsewhere), migration `0005` **rebuilds** the suggestions table to extend the `type` CHECK (SQLite can't ALTER a CHECK), `collocationScope` + `CollocationLimit` + `POST /{id}/collocation`. **Caught a latent bug**: `grammarScope` was `type != 'voice'` → would wipe collocation flags; fixed to `type NOT IN ('voice','collocation')`. Frontend: `--color-blossom` pink, "Make it sound natural 🌸" toolbar pill, `collocating`/`runCollocation` in `useCheckpoint`, StatusBar dot — all into the existing rail/card. **Phase 13**: new `internal/vocab` package — migration `0006_vocab_garden` (`vocab_words`, SM-2-lite columns, doc_id `ON DELETE SET NULL`, `UNIQUE(user_id,word)`), `scheduler.go` (Leitner ladder 1/3/7/16/35 → geometric; gentle "again", no streak-shaming), `handlers.go` (capture-upsert/list/due/review/delete, all owner-scoped, time math via SQLite `datetime()` so stored values stay canonical-UTC). Auto-capture wired into `EditorCore.openWordLookup` (only dictionary-known words, captures the surrounding sentence + doc_id) + a 🤍/💚 toggle on `WordCard`. `GardenPanel` slide-over: blossom grid (bloom stage by reps), flashcard review (sentence blanked, flip, again/good/easy, direction alternates recognition↔production), sleepy-kitten footer; opened from a global 🌷 header button. Tests: `TestCollocationPassCoexists`, vocab `scheduler_test.go` + `handlers_test.go`, db CHECK test extended. go build/vet/test + tsc + vite + vitest (51/51) all clean; migration verified against a copy of the live DB; live backend smoke (throwaway DB) walked the full vocab lifecycle + the warm-502 collocation path. **Remaining: only the deferred infra bucket** — Authentik auth, Copyleaks Tier-2 (needs a public webhook), Docker/Traefik/deploy — all on hold per the user's "except Authentik/Traefik". - 2026-06-26: **Phase 14 complete** (companion warmth + bedtime nag + night mode). `tips.ts`: `ENCOURAGEMENTS` 5→10 lines; new `BEDTIME` array (4 lines, user-supplied English wit + gentle Mandarin leads). `useCompanion.ts`: bedtime branch in the 10s heartbeat (after idle-return + break, before the generic tip); only nudges while actively writing; own `lastBedtime` ref + 30min `BEDTIME_GAP`, respects `PROACTIVE_GAP`; new `'bedtime'` `BubbleTone` lingers ~4s longer. **Night mode** (added same session, user request): `lib/night.ts` centralizes `isBedtime()` + window (now shared by the nag too); `hooks/useNightMode.ts` toggles `petal-night` on `` (60s re-check); `index.css` `html.petal-night` re-points only the palette tokens → whole UI flips via `var()` (no component edits), 600ms dusk fade, print stays white; `PetalFall` gains a `night` prop → chunky cartoon power stars (`makeCartoonStar`, Mario/Kirby-style, 5 candy colors) mixed ~70/30 with small twinkle sparkles, gentle spin + shallow shimmer, effect re-inits on flip; App: `useNightMode()` → ``. tsc + vite clean, companion vitest 45/45; verified with real-browser Playwright screenshots (clock mocked to 23:30) — day petals/cream vs night stars/dark-plum, both pretty. Bedtime window is `BEDTIME_FROM`/`BEDTIME_TO` (local clock) for easy retune. - 2026-06-26: **Phase 11 complete** (writer power-ups, batch requested as "do it all"). Seven features: (1) in-doc **Find & Replace** — `SearchHighlight` decoration extension + `FindReplace` bar (Ctrl/Cmd+F, match-case, replace-all back-to-front, DOM scroll that doesn't trigger the selection bubble); (2) **read-aloud** Web Speech util + 🔊 in WordCard & selection bubble; (3) **keyboard/touch access** — Ctrl/Cmd+D caret lookup, Ctrl/Cmd+J rewrite, touch long-press (refactored `handleContextMenu` → shared `openWordLookup(pos)`); (4) **export-all** backup zip (`GET /api/docs/export-all`, `TestExportAll`, sidebar download links); (5) **smart typography** input-rules extension (curly quotes/em-dash/ellipsis, ASCII-only so CJK untouched); (6) **duplicate doc + sidebar sort + outline popover**; (7) **English phonetic** (pivoted from pinyin — IPA is what an English learner needs; pinyin annotates Chinese she already reads) via `scripts/build_phonetic.py` + embedded `phonetic.json.gz` + `Result.Phonetic` + WordCard `/ˈrɪvər/` line — **full 46,579-word dataset built from ECDICT** (the csv re-download worked; `--seed` mode kept as a csv-free fallback). Also folded in this session: the **selection-bubble vs copy/paste fix** (bubble deferred to pointer-up + container `pointer-events:none` so it never sits where you click). go build/vet/test + tsc + vite all clean; live smoke verified word-phonetic (incl. de-inflection) + export-all zip (de-duped CJK names, route priority). Next: deferred bucket (auth/Copyleaks/deploy), still on hold per user. diff --git a/cmd/server/main.go b/cmd/server/main.go index fcf75b5..c1739da 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -12,6 +12,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/config" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/docs" @@ -65,48 +66,65 @@ func main() { _, _ = w.Write([]byte(`{"version":"` + version + `"}`)) }) - llmClient := llm.NewLLMClient(cfg) - sug := suggestions.New(database, llmClient) + // Everything below serves or mutates a particular user's data, so it sits + // behind the auth middleware. /health and /version deliberately stay + // outside it: they carry no user data, and a monitoring probe (or the + // client's update poll) must not need a session to reach them. + // + // The middleware resolves the caller once and hands handlers the answer via + // auth.UserID(r.Context()), replacing the db.LocalUserID constant those + // queries used to name directly. Petal is still single-user — StaticResolver + // returns that same local user for every request — but the identity now + // travels the same path a real one will. Swapping this line for an Authentik + // session resolver is the whole remaining change; no handler or query moves. + api.Group(func(pr chi.Router) { + pr.Use(auth.Middleware(auth.StaticResolver(db.LocalUserID))) - // Document CRUD plus the doc-scoped checkpoint/list suggestion routes, - // both under /api/docs. - docsHandler := docs.New(database) - docsRouter := docsHandler.Routes() - sug.RegisterDocRoutes(docsRouter) - api.Mount("/docs", docsRouter) + llmClient := llm.NewLLMClient(cfg) + sug := suggestions.New(database, llmClient) - // Tag management (the roster) and cross-document full-text search. - api.Mount("/tags", docsHandler.TagRoutes()) - api.Mount("/search", docsHandler.SearchRoutes()) + // Document CRUD plus the doc-scoped checkpoint/list suggestion routes, + // both under /api/docs. + docsHandler := docs.New(database) + docsRouter := docsHandler.Routes() + sug.RegisterDocRoutes(docsRouter) + pr.Mount("/docs", docsRouter) - // Per-suggestion actions (accept/dismiss) under /api/suggestions. - api.Mount("/suggestions", sug.Routes()) + // Tag management (the roster) and cross-document full-text search. + pr.Mount("/tags", docsHandler.TagRoutes()) + pr.Mount("/search", docsHandler.SearchRoutes()) - // Offline lexicon: full word lookups (gloss + definition + synonyms) for - // the right-click popover, and the lightweight Chinese-only gloss for the - // inline hover/select tooltip. One handler so the datasets load once. - lex := lexicon.NewHandler() - api.Mount("/word", lex.Routes()) - api.Mount("/gloss", lex.GlossRoutes()) + // Per-suggestion actions (accept/dismiss) under /api/suggestions. + pr.Mount("/suggestions", sug.Routes()) - // Vocabulary garden: words the writer looks up are captured here and - // surfaced for gentle spaced-repetition review. - api.Mount("/vocab", vocab.New(database).Routes()) + // Offline lexicon: full word lookups (gloss + definition + synonyms) for + // the right-click popover, and the lightweight Chinese-only gloss for the + // inline hover/select tooltip. One handler so the datasets load once. + // The dataset is static and identical for everyone, but it stays behind + // auth so the API surface has no unauthenticated read holes. + lex := lexicon.NewHandler() + pr.Mount("/word", lex.Routes()) + pr.Mount("/gloss", lex.GlossRoutes()) - // Editor image uploads, stored on disk and served back by content hash. - imgHandler, err := images.New(cfg.ImageDir) - if err != nil { - log.Fatalf("image store: %v", err) - } - api.Mount("/images", imgHandler.Routes()) + // Vocabulary garden: words the writer looks up are captured here and + // surfaced for gentle spaced-repetition review. + pr.Mount("/vocab", vocab.New(database).Routes()) - // Read-aloud: proxy short passages to a local Piper TTS server. Only - // mounted when TTS_ENDPOINT is configured; otherwise the frontend falls - // back to the browser's Web Speech API on its own. - if ttsHandler, ok := tts.New(cfg); ok { - api.Mount("/tts", ttsHandler.Routes()) - log.Printf("read-aloud enabled (TTS endpoint=%s)", cfg.TTSEndpoint) - } + // Editor image uploads, stored on disk and served back by content hash. + imgHandler, err := images.New(cfg.ImageDir) + if err != nil { + log.Fatalf("image store: %v", err) + } + pr.Mount("/images", imgHandler.Routes()) + + // Read-aloud: proxy short passages to a local Piper TTS server. Only + // mounted when TTS_ENDPOINT is configured; otherwise the frontend falls + // back to the browser's Web Speech API on its own. + if ttsHandler, ok := tts.New(cfg); ok { + pr.Mount("/tts", ttsHandler.Routes()) + log.Printf("read-aloud enabled (TTS endpoint=%s)", cfg.TTSEndpoint) + } + }) }) // Everything else: serve the embedded SPA (with index.html fallback for client routing). diff --git a/internal/auth/auth.go b/internal/auth/auth.go new file mode 100644 index 0000000..915f4e0 --- /dev/null +++ b/internal/auth/auth.go @@ -0,0 +1,77 @@ +// Package auth answers one question for every API request: who is asking? +// +// Until now Petal ran as a single hardcoded user and every query passed +// db.LocalUserID directly. That made the identity of the caller a compile-time +// constant scattered across ~35 call sites — nothing a real login could ever +// replace without touching all of them. This package moves that identity into +// the request context, resolved once by [Middleware], so handlers read the +// current user instead of naming one. +// +// The identity itself still comes from [StaticResolver] today, which returns +// the same local user for everyone. Swapping in Authentik later means writing +// one Resolver (validate the session cookie → user id) and changing the single +// line in main.go that constructs it. No handler changes. +package auth + +import ( + "context" + "net/http" + + "gitea.parodia.dev/drwily/petal/internal/httputil" +) + +// ctxKey is unexported so no other package can plant a user id in the context +// without going through [WithUser]. +type ctxKey struct{} + +// WithUser returns a copy of ctx carrying userID as the authenticated caller. +// Handlers never call this; [Middleware] does, and tests use it to build a +// request that looks authenticated. +func WithUser(ctx context.Context, userID string) context.Context { + return context.WithValue(ctx, ctxKey{}, userID) +} + +// UserID returns the authenticated user id carried by ctx, or "" if the request +// never passed through [Middleware]. +// +// Returning "" rather than panicking keeps an unauthenticated request failing +// *closed*: every query in Petal is scoped `WHERE user_id = ?`, so an empty id +// matches no rows — a missing middleware leaks nothing, it just returns empty +// results. Handlers may therefore use the value directly without checking it. +func UserID(ctx context.Context) string { + id, _ := ctx.Value(ctxKey{}).(string) + return id +} + +// Resolver maps an inbound request to the id of the user making it. Returning +// an error, or an empty id, rejects the request with a 401. +// +// This is the seam a real identity provider drops into: an Authentik resolver +// validates the session cookie and returns the user id it maps to. +type Resolver interface { + Resolve(r *http.Request) (string, error) +} + +// StaticResolver resolves every request to the same user id, ignoring the +// request entirely. It is how Petal runs today — a single-user app whose one +// user now arrives through the same path a logged-in user eventually will. +type StaticResolver string + +// Resolve implements [Resolver]. +func (s StaticResolver) Resolve(*http.Request) (string, error) { return string(s), nil } + +// Middleware resolves the caller with res and stores the result in the request +// context for [UserID]. Requests the resolver rejects — or resolves to an empty +// id — never reach the handler; they get a 401 instead. +func Middleware(res Resolver) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + userID, err := res.Resolve(r) + if err != nil || userID == "" { + httputil.ErrorJSON(w, http.StatusUnauthorized, "not signed in") + return + } + next.ServeHTTP(w, r.WithContext(WithUser(r.Context(), userID))) + }) + } +} diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go new file mode 100644 index 0000000..83abecc --- /dev/null +++ b/internal/auth/auth_test.go @@ -0,0 +1,75 @@ +package auth + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" +) + +// errResolver rejects every request, standing in for a real resolver that finds +// no valid session. +type errResolver struct{ err error } + +func (e errResolver) Resolve(*http.Request) (string, error) { return "", e.err } + +func TestUserIDRoundTrip(t *testing.T) { + ctx := WithUser(context.Background(), "alice") + if got := UserID(ctx); got != "alice" { + t.Fatalf("UserID = %q, want alice", got) + } +} + +// A request that never passed through the middleware must report no user rather +// than panicking — every query is `WHERE user_id = ?`, so an empty id fails +// closed (matches nothing) instead of falling back to some default account. +func TestUserIDAbsentIsEmpty(t *testing.T) { + if got := UserID(context.Background()); got != "" { + t.Fatalf("UserID on bare context = %q, want empty", got) + } +} + +func TestMiddlewareInjectsResolvedUser(t *testing.T) { + var seen string + h := Middleware(StaticResolver("local"))(http.HandlerFunc( + func(_ http.ResponseWriter, r *http.Request) { seen = UserID(r.Context()) }, + )) + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + + if seen != "local" { + t.Fatalf("handler saw user %q, want local", seen) + } + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } +} + +// Both rejection paths — an explicit error and a silent empty id — must 401 +// without ever entering the handler. The empty case matters most: a resolver +// that returns ("", nil) by mistake would otherwise hand handlers an empty user +// id, and while that fails closed at the SQL layer, it should never get there. +func TestMiddlewareRejectsUnresolved(t *testing.T) { + for name, res := range map[string]Resolver{ + "resolver error": errResolver{err: http.ErrNoCookie}, + "empty user id": StaticResolver(""), + } { + t.Run(name, func(t *testing.T) { + called := false + h := Middleware(res)(http.HandlerFunc( + func(http.ResponseWriter, *http.Request) { called = true }, + )) + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + + if called { + t.Fatal("handler ran for an unauthenticated request") + } + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } + }) + } +} diff --git a/internal/docs/export.go b/internal/docs/export.go index 8eafdd9..f2c49e7 100644 --- a/internal/docs/export.go +++ b/internal/docs/export.go @@ -13,6 +13,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -46,7 +47,7 @@ func (h *Handler) exportAll(w http.ResponseWriter, r *http.Request) { FROM documents WHERE user_id = ? ORDER BY updated_at DESC`, - db.LocalUserID, + auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) @@ -143,7 +144,7 @@ func (h *Handler) export(w http.ResponseWriter, r *http.Request) { return } - doc, err := h.fetch(chi.URLParam(r, "id")) + doc, err := h.fetch(auth.UserID(r.Context()), chi.URLParam(r, "id")) if errors.Is(err, sql.ErrNoRows) { notFound(w) return diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go index b528837..42f5a28 100644 --- a/internal/docs/handlers.go +++ b/internal/docs/handlers.go @@ -1,6 +1,7 @@ // Package docs implements the document CRUD HTTP handlers — the create / list / // read / update / delete surface that backs the editor and its 1.5s auto-save. -// All access is scoped to the single hardcoded local user while auth is deferred. +// Every query is scoped to the caller resolved by the auth middleware, so a +// document is only ever reachable by the user who owns it. package docs import ( @@ -12,6 +13,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -52,15 +54,16 @@ type docSummary struct { Tags []db.Tag `json:"tags"` } -// list returns the local user's documents, most-recently-updated first, each +// list returns the caller's documents, most-recently-updated first, each // decorated with its tags. func (h *Handler) list(w http.ResponseWriter, r *http.Request) { + userID := auth.UserID(r.Context()) rows, err := h.DB.Query( `SELECT id, title, word_count, updated_at FROM documents WHERE user_id = ? ORDER BY updated_at DESC`, - db.LocalUserID, + userID, ) if err != nil { httputil.ServerError(w, err) @@ -84,7 +87,7 @@ func (h *Handler) list(w http.ResponseWriter, r *http.Request) { return } - byDoc, err := h.tagsByDoc(ids) + byDoc, err := h.tagsByDoc(userID, ids) if err != nil { httputil.ServerError(w, err) return @@ -104,7 +107,7 @@ func (h *Handler) create(w http.ResponseWriter, r *http.Request) { err := h.DB.QueryRow( `INSERT INTO documents (user_id) VALUES (?) RETURNING id, user_id, title, content, content_text, tone, word_count, created_at, updated_at`, - db.LocalUserID, + auth.UserID(r.Context()), ).Scan( &doc.ID, &doc.UserID, &doc.Title, &doc.Content, &doc.ContentText, &doc.Tone, &doc.WordCount, &doc.CreatedAt, &doc.UpdatedAt, @@ -118,7 +121,7 @@ func (h *Handler) create(w http.ResponseWriter, r *http.Request) { // get returns a single full document by id. func (h *Handler) get(w http.ResponseWriter, r *http.Request) { - doc, err := h.fetch(chi.URLParam(r, "id")) + doc, err := h.fetch(auth.UserID(r.Context()), chi.URLParam(r, "id")) if errors.Is(err, sql.ErrNoRows) { notFound(w) return @@ -149,6 +152,7 @@ type updateRequest struct { // content and content_text are kept in sync by the client and written together. func (h *Handler) update(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") + userID := auth.UserID(r.Context()) var req updateRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -167,7 +171,7 @@ func (h *Handler) update(w http.ResponseWriter, r *http.Request) { updated_at = CURRENT_TIMESTAMP WHERE id = ? AND user_id = ?`, req.Title, req.Content, req.ContentText, req.Tone, req.WordCount, - req.PreserveHistory, id, db.LocalUserID, + req.PreserveHistory, id, userID, ) if err != nil { httputil.ServerError(w, err) @@ -178,7 +182,7 @@ func (h *Handler) update(w http.ResponseWriter, r *http.Request) { return } - doc, err := h.fetch(id) + doc, err := h.fetch(userID, id) if err != nil { httputil.ServerError(w, err) return @@ -200,7 +204,7 @@ func (h *Handler) update(w http.ResponseWriter, r *http.Request) { func (h *Handler) delete(w http.ResponseWriter, r *http.Request) { res, err := h.DB.Exec( `DELETE FROM documents WHERE id = ? AND user_id = ?`, - chi.URLParam(r, "id"), db.LocalUserID, + chi.URLParam(r, "id"), auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) @@ -213,15 +217,18 @@ func (h *Handler) delete(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -// fetch loads one full document scoped to the local user. -func (h *Handler) fetch(id string) (db.Document, error) { +// fetch loads one full document, scoped to its owner. Callers pass the id from +// [auth.UserID]; a document belonging to anyone else comes back as +// sql.ErrNoRows, which handlers surface as a 404 rather than a 403 (a stranger's +// document should be indistinguishable from one that doesn't exist). +func (h *Handler) fetch(userID, id string) (db.Document, error) { var doc db.Document err := h.DB.QueryRow( `SELECT id, user_id, title, content, content_text, tone, word_count, created_at, updated_at, preserve_history FROM documents WHERE id = ? AND user_id = ?`, - id, db.LocalUserID, + id, userID, ).Scan( &doc.ID, &doc.UserID, &doc.Title, &doc.Content, &doc.ContentText, &doc.Tone, &doc.WordCount, &doc.CreatedAt, &doc.UpdatedAt, &doc.PreserveHistory, diff --git a/internal/docs/handlers_test.go b/internal/docs/handlers_test.go index 1d0e35e..d80c9e7 100644 --- a/internal/docs/handlers_test.go +++ b/internal/docs/handlers_test.go @@ -8,10 +8,14 @@ import ( "path/filepath" "testing" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" ) -// newTestServer spins up an isolated on-disk database and the docs router. +// newTestServer spins up an isolated on-disk database and the docs router, +// behind the same auth middleware main.go installs. Tests must go through it: +// handlers read the caller from the request context, so a router mounted bare +// would see an empty user id and match no rows. func newTestServer(t *testing.T) http.Handler { t.Helper() database, err := db.Open(filepath.Join(t.TempDir(), "test.db")) @@ -19,7 +23,13 @@ func newTestServer(t *testing.T) http.Handler { t.Fatalf("open db: %v", err) } t.Cleanup(func() { database.Close() }) - return New(database).Routes() + return withAuth(New(database).Routes()) +} + +// withAuth wraps a router so every test request arrives authenticated as the +// seeded local user — the stand-in for a real session until Authentik lands. +func withAuth(h http.Handler) http.Handler { + return auth.Middleware(auth.StaticResolver(db.LocalUserID))(h) } func do(t *testing.T, srv http.Handler, method, path, body string) *httptest.ResponseRecorder { diff --git a/internal/docs/isolation_test.go b/internal/docs/isolation_test.go new file mode 100644 index 0000000..8622cda --- /dev/null +++ b/internal/docs/isolation_test.go @@ -0,0 +1,222 @@ +package docs + +import ( + "encoding/json" + "net/http" + "path/filepath" + "testing" + + "github.com/go-chi/chi/v5" + + "gitea.parodia.dev/drwily/petal/internal/auth" + "gitea.parodia.dev/drwily/petal/internal/db" +) + +// This file is the point of the auth plumbing: it proves that swapping the +// hardcoded user for a request-scoped one actually isolates accounts. Every +// handler resolves its user from the request, so mounting the same routers twice +// behind two different resolvers gives us two "logged-in" users over one +// database — which is exactly the situation a real login will create. + +// newTwoUserServer opens one database holding two users and returns a router for +// each, identical but for who the auth middleware says is calling. +func newTwoUserServer(t *testing.T) (alice, bob http.Handler) { + t.Helper() + database, err := db.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { database.Close() }) + + // db.Open seeds the local user; add a second so both sides have a valid FK. + if _, err := database.Exec( + `INSERT INTO users (id, email, display_name) VALUES (?, ?, ?)`, + "bob", "bob@petal.local", "Bob", + ); err != nil { + t.Fatalf("seed second user: %v", err) + } + + mount := func(userID string) http.Handler { + h := New(database) + r := chi.NewRouter() + r.Mount("/docs", h.Routes()) + r.Mount("/tags", h.TagRoutes()) + r.Mount("/search", h.SearchRoutes()) + return auth.Middleware(auth.StaticResolver(userID))(r) + } + return mount(db.LocalUserID), mount("bob") +} + +// TestDocumentIsolation walks every read and write path that takes a document id +// and asserts Bob cannot reach Alice's document through any of them. A stranger's +// document must be indistinguishable from a nonexistent one — 404, never 403. +func TestDocumentIsolation(t *testing.T) { + alice, bob := newTwoUserServer(t) + + docID := createDoc(t, alice, "Alice's diary", "a private sentence about my day") + + t.Run("not in list", func(t *testing.T) { + rec := do(t, bob, http.MethodGet, "/docs", "") + var out []docSummary + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode list: %v", err) + } + if len(out) != 0 { + t.Fatalf("bob sees %d of alice's documents, want 0", len(out)) + } + }) + + t.Run("not in search", func(t *testing.T) { + rec := do(t, bob, http.MethodGet, "/search?q=private", "") + var out []searchResult + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode search: %v", err) + } + if len(out) != 0 { + t.Fatalf("search leaked %d of alice's documents", len(out)) + } + }) + + // The FTS index is a separate table joined back to documents; a missing + // user_id filter there would leak content even though the list query is + // scoped, so assert the owner still finds her own document. + t.Run("owner still finds it", func(t *testing.T) { + rec := do(t, alice, http.MethodGet, "/search?q=private", "") + var out []searchResult + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode search: %v", err) + } + if len(out) != 1 { + t.Fatalf("alice found %d results for her own document, want 1", len(out)) + } + }) + + for _, tc := range []struct { + name, method, path, body string + }{ + {"get", http.MethodGet, "/docs/" + docID, ""}, + {"update", http.MethodPut, "/docs/" + docID, `{"title":"defaced"}`}, + {"delete", http.MethodDelete, "/docs/" + docID, ""}, + {"export", http.MethodGet, "/docs/" + docID + "/export?format=md", ""}, + {"passport", http.MethodGet, "/docs/" + docID + "/passport", ""}, + {"snapshot", http.MethodPost, "/docs/" + docID + "/versions", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + rec := do(t, bob, tc.method, tc.path, tc.body) + if rec.Code != http.StatusNotFound { + t.Fatalf("%s %s as bob = %d, want 404 (body: %s)", + tc.method, tc.path, rec.Code, rec.Body) + } + }) + } + + // The document must have survived every attempt above unchanged. + rec := do(t, alice, http.MethodGet, "/docs/"+docID, "") + if rec.Code != http.StatusOK { + t.Fatalf("alice lost access to her own document: %d %s", rec.Code, rec.Body) + } + var doc db.Document + if err := json.Unmarshal(rec.Body.Bytes(), &doc); err != nil { + t.Fatalf("decode doc: %v", err) + } + if doc.Title != "Alice's diary" { + t.Fatalf("title = %q, want %q — bob's update went through", doc.Title, "Alice's diary") + } +} + +// TestVersionIsolation covers the history endpoints, which scope through a join +// to documents rather than a direct user_id column — an easy place to forget the +// filter, and one where the leak would be the full text of every draft. +func TestVersionIsolation(t *testing.T) { + alice, bob := newTwoUserServer(t) + + docID := createDoc(t, alice, "Draft", "the first version of my essay") + rec := do(t, alice, http.MethodPost, "/docs/"+docID+"/versions", "") + if rec.Code != http.StatusCreated { + t.Fatalf("snapshot: %d %s", rec.Code, rec.Body) + } + var v db.DocumentVersion + if err := json.Unmarshal(rec.Body.Bytes(), &v); err != nil { + t.Fatalf("decode version: %v", err) + } + + t.Run("list is empty for stranger", func(t *testing.T) { + rec := do(t, bob, http.MethodGet, "/docs/"+docID+"/versions", "") + var out []db.DocumentVersion + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if len(out) != 0 { + t.Fatalf("bob sees %d of alice's snapshots, want 0", len(out)) + } + }) + + for _, tc := range []struct{ name, method, path string }{ + {"preview", http.MethodGet, "/docs/" + docID + "/versions/" + v.ID}, + {"restore", http.MethodPost, "/docs/" + docID + "/versions/" + v.ID + "/restore"}, + } { + t.Run(tc.name, func(t *testing.T) { + rec := do(t, bob, tc.method, tc.path, "") + if rec.Code != http.StatusNotFound { + t.Fatalf("%s as bob = %d, want 404 (body: %s)", tc.name, rec.Code, rec.Body) + } + }) + } +} + +// TestTagIsolation checks the tag roster and, more importantly, that a document +// and a tag can't be cross-linked across accounts — the assignment endpoint takes +// two ids from different tables and must own-check both. +func TestTagIsolation(t *testing.T) { + alice, bob := newTwoUserServer(t) + + docID := createDoc(t, alice, "Essay", "some words") + + rec := do(t, alice, http.MethodPost, "/tags", `{"name":"school","color":"mint"}`) + if rec.Code != http.StatusCreated { + t.Fatalf("create tag: %d %s", rec.Code, rec.Body) + } + var aliceTag db.Tag + if err := json.Unmarshal(rec.Body.Bytes(), &aliceTag); err != nil { + t.Fatalf("decode tag: %v", err) + } + + rec = do(t, bob, http.MethodPost, "/tags", `{"name":"bobs","color":"sky"}`) + var bobTag db.Tag + if err := json.Unmarshal(rec.Body.Bytes(), &bobTag); err != nil { + t.Fatalf("decode bob tag: %v", err) + } + + t.Run("roster is per user", func(t *testing.T) { + rec := do(t, bob, http.MethodGet, "/tags", "") + var out []db.Tag + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if len(out) != 1 || out[0].Name != "bobs" { + t.Fatalf("bob's roster = %+v, want just his own tag", out) + } + }) + + t.Run("cannot tag a stranger's document", func(t *testing.T) { + body, _ := json.Marshal(map[string]string{"tag_id": bobTag.ID}) + rec := do(t, bob, http.MethodPost, "/docs/"+docID+"/tags", string(body)) + if rec.Code != http.StatusNotFound { + t.Fatalf("bob tagging alice's doc = %d, want 404", rec.Code) + } + }) + + t.Run("cannot rename a stranger's tag", func(t *testing.T) { + rec := do(t, bob, http.MethodPatch, "/tags/"+aliceTag.ID, `{"name":"stolen"}`) + if rec.Code != http.StatusNotFound { + t.Fatalf("bob renaming alice's tag = %d, want 404", rec.Code) + } + }) + + t.Run("cannot delete a stranger's tag", func(t *testing.T) { + rec := do(t, bob, http.MethodDelete, "/tags/"+aliceTag.ID, "") + if rec.Code != http.StatusNotFound { + t.Fatalf("bob deleting alice's tag = %d, want 404", rec.Code) + } + }) +} diff --git a/internal/docs/passport.go b/internal/docs/passport.go index 8490a23..d5fe298 100644 --- a/internal/docs/passport.go +++ b/internal/docs/passport.go @@ -20,6 +20,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -216,8 +217,9 @@ func verifyChain(doc db.Document, versions []db.DocumentVersion) (status string, // printing, so "Save as PDF" in the browser produces the handoff artifact. func (h *Handler) passport(w http.ResponseWriter, r *http.Request) { docID := chi.URLParam(r, "id") + userID := auth.UserID(r.Context()) - doc, err := h.fetch(docID) + doc, err := h.fetch(userID, docID) if errors.Is(err, sql.ErrNoRows) { notFound(w) return @@ -227,7 +229,7 @@ func (h *Handler) passport(w http.ResponseWriter, r *http.Request) { return } - versions, err := h.passportVersions(docID) + versions, err := h.passportVersions(userID, docID) if err != nil { httputil.ServerError(w, err) return @@ -251,7 +253,7 @@ func (h *Handler) passport(w http.ResponseWriter, r *http.Request) { // passportVersions loads every snapshot oldest-first with the fields the report // and the chain check need — including content_text, which the list endpoint // omits as too heavy but verification cannot do without. -func (h *Handler) passportVersions(docID string) ([]db.DocumentVersion, error) { +func (h *Handler) passportVersions(userID, docID string) ([]db.DocumentVersion, error) { rows, err := h.DB.Query( `SELECT v.id, v.doc_id, v.title, v.content_text, v.word_count, v.kind, v.created_at, v.content_hash, v.prev_hash @@ -259,7 +261,7 @@ func (h *Handler) passportVersions(docID string) ([]db.DocumentVersion, error) { JOIN documents d ON d.id = v.doc_id WHERE v.doc_id = ? AND d.user_id = ? ORDER BY v.created_at ASC, v.rowid ASC`, - docID, db.LocalUserID, + docID, userID, ) if err != nil { return nil, err diff --git a/internal/docs/search.go b/internal/docs/search.go index c928086..8dc700e 100644 --- a/internal/docs/search.go +++ b/internal/docs/search.go @@ -7,6 +7,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -50,12 +51,13 @@ func (h *Handler) SearchRoutes() chi.Router { return r } -// search runs a cross-document full-text search for the local user. Queries of +// search runs a cross-document full-text search for the caller. Queries of // three or more runes use the trigram FTS index (fast, ranked); shorter queries // fall back to a LIKE scan so 2-character Chinese words still resolve. Either way // the snippet is built in Go from the original text, for clean word boundaries // and a uniform highlight format. func (h *Handler) search(w http.ResponseWriter, r *http.Request) { + userID := auth.UserID(r.Context()) q := strings.TrimSpace(r.URL.Query().Get("q")) if q == "" { httputil.WriteJSON(w, http.StatusOK, []searchResult{}) @@ -80,7 +82,7 @@ func (h *Handler) search(w http.ResponseWriter, r *http.Request) { WHERE documents_fts MATCH ? AND d.user_id = ? ORDER BY rank LIMIT ?`, - phrase, db.LocalUserID, maxSearchResults, + phrase, userID, maxSearchResults, ) if err != nil { httputil.ServerError(w, err) @@ -110,7 +112,7 @@ func (h *Handler) search(w http.ResponseWriter, r *http.Request) { AND (title LIKE ? ESCAPE '\' OR content_text LIKE ? ESCAPE '\') ORDER BY updated_at DESC LIMIT ?`, - db.LocalUserID, like, like, maxSearchResults, + userID, like, like, maxSearchResults, ) if err != nil { httputil.ServerError(w, err) @@ -144,7 +146,7 @@ func (h *Handler) search(w http.ResponseWriter, r *http.Request) { ids = append(ids, rw.id) } - byDoc, err := h.tagsByDoc(ids) + byDoc, err := h.tagsByDoc(userID, ids) if err != nil { httputil.ServerError(w, err) return diff --git a/internal/docs/tags.go b/internal/docs/tags.go index f87b4f8..5090318 100644 --- a/internal/docs/tags.go +++ b/internal/docs/tags.go @@ -7,6 +7,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -55,7 +56,7 @@ func (h *Handler) listTags(w http.ResponseWriter, r *http.Request) { WHERE t.user_id = ? GROUP BY t.id ORDER BY t.name COLLATE NOCASE`, - db.LocalUserID, + auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) @@ -104,7 +105,7 @@ func (h *Handler) createTag(w http.ResponseWriter, r *http.Request) { `INSERT INTO tags (user_id, name, color) VALUES (?, ?, ?) ON CONFLICT(user_id, name) DO UPDATE SET name = excluded.name RETURNING id, name, color`, - db.LocalUserID, name, normalizeColor(req.Color), + auth.UserID(r.Context()), name, normalizeColor(req.Color), ).Scan(&t.ID, &t.Name, &t.Color) if err != nil { httputil.ServerError(w, err) @@ -117,6 +118,7 @@ func (h *Handler) createTag(w http.ResponseWriter, r *http.Request) { // a recolor needn't resend the name. func (h *Handler) updateTag(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") + userID := auth.UserID(r.Context()) var req struct { Name *string `json:"name"` @@ -145,7 +147,7 @@ func (h *Handler) updateTag(w http.ResponseWriter, r *http.Request) { SET name = COALESCE(?, name), color = COALESCE(?, color) WHERE id = ? AND user_id = ?`, - namePtr, colorPtr, id, db.LocalUserID, + namePtr, colorPtr, id, userID, ) if err != nil { httputil.ServerError(w, err) @@ -159,7 +161,7 @@ func (h *Handler) updateTag(w http.ResponseWriter, r *http.Request) { var t db.Tag if err := h.DB.QueryRow( `SELECT id, name, color FROM tags WHERE id = ? AND user_id = ?`, - id, db.LocalUserID, + id, userID, ).Scan(&t.ID, &t.Name, &t.Color); err != nil { httputil.ServerError(w, err) return @@ -171,7 +173,7 @@ func (h *Handler) updateTag(w http.ResponseWriter, r *http.Request) { func (h *Handler) deleteTag(w http.ResponseWriter, r *http.Request) { res, err := h.DB.Exec( `DELETE FROM tags WHERE id = ? AND user_id = ?`, - chi.URLParam(r, "id"), db.LocalUserID, + chi.URLParam(r, "id"), auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) @@ -184,10 +186,11 @@ func (h *Handler) deleteTag(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -// assignTag attaches a tag to a document. Both must belong to the local user; +// assignTag attaches a tag to a document. Both must belong to the caller; // the assignment is idempotent (re-assigning is a no-op, not an error). func (h *Handler) assignTag(w http.ResponseWriter, r *http.Request) { docID := chi.URLParam(r, "id") + userID := auth.UserID(r.Context()) var req struct { TagID string `json:"tag_id"` @@ -203,11 +206,11 @@ func (h *Handler) assignTag(w http.ResponseWriter, r *http.Request) { // Verify both the doc and the tag belong to the user before linking, so a // stray id can't cross-link another account's rows. - if !h.ownsDoc(docID) { + if !h.ownsDoc(userID, docID) { notFound(w) return } - if !h.ownsTag(req.TagID) { + if !h.ownsTag(userID, req.TagID) { notFoundMsg(w, "tag not found") return } @@ -228,7 +231,7 @@ func (h *Handler) unassignTag(w http.ResponseWriter, r *http.Request) { docID := chi.URLParam(r, "id") tagID := chi.URLParam(r, "tagId") - if !h.ownsDoc(docID) { + if !h.ownsDoc(auth.UserID(r.Context()), docID) { notFound(w) return } @@ -242,22 +245,22 @@ func (h *Handler) unassignTag(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -// ownsDoc reports whether a document belongs to the local user. -func (h *Handler) ownsDoc(docID string) bool { +// ownsDoc reports whether a document belongs to the given user. +func (h *Handler) ownsDoc(userID, docID string) bool { var exists bool _ = h.DB.QueryRow( `SELECT EXISTS(SELECT 1 FROM documents WHERE id = ? AND user_id = ?)`, - docID, db.LocalUserID, + docID, userID, ).Scan(&exists) return exists } -// ownsTag reports whether a tag belongs to the local user. -func (h *Handler) ownsTag(tagID string) bool { +// ownsTag reports whether a tag belongs to the given user. +func (h *Handler) ownsTag(userID, tagID string) bool { var exists bool _ = h.DB.QueryRow( `SELECT EXISTS(SELECT 1 FROM tags WHERE id = ? AND user_id = ?)`, - tagID, db.LocalUserID, + tagID, userID, ).Scan(&exists) return exists } @@ -265,7 +268,7 @@ func (h *Handler) ownsTag(tagID string) bool { // tagsByDoc loads the tags for a set of documents in one query and groups them // by doc id. Used to decorate the document list and search results without an // N+1 of per-doc queries. Returns an empty (non-nil) map when ids is empty. -func (h *Handler) tagsByDoc(ids []string) (map[string][]db.Tag, error) { +func (h *Handler) tagsByDoc(userID string, ids []string) (map[string][]db.Tag, error) { out := map[string][]db.Tag{} if len(ids) == 0 { return out, nil @@ -277,7 +280,7 @@ func (h *Handler) tagsByDoc(ids []string) (map[string][]db.Tag, error) { for _, id := range ids { args = append(args, id) } - args = append(args, db.LocalUserID) + args = append(args, userID) rows, err := h.DB.Query( `SELECT dt.doc_id, t.id, t.name, t.color diff --git a/internal/docs/tags_test.go b/internal/docs/tags_test.go index 52c4aa5..e768e31 100644 --- a/internal/docs/tags_test.go +++ b/internal/docs/tags_test.go @@ -27,7 +27,7 @@ func newFullServer(t *testing.T) http.Handler { r.Mount("/docs", h.Routes()) r.Mount("/tags", h.TagRoutes()) r.Mount("/search", h.SearchRoutes()) - return r + return withAuth(r) } // createDoc makes a document with the given title/body and returns its id. diff --git a/internal/docs/versions.go b/internal/docs/versions.go index 3fe955e..b52295c 100644 --- a/internal/docs/versions.go +++ b/internal/docs/versions.go @@ -8,6 +8,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -49,7 +50,7 @@ func (h *Handler) listVersions(w http.ResponseWriter, r *http.Request) { JOIN documents d ON d.id = v.doc_id WHERE v.doc_id = ? AND d.user_id = ? ORDER BY v.created_at DESC`, - docID, db.LocalUserID, + docID, auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) @@ -75,7 +76,7 @@ func (h *Handler) listVersions(w http.ResponseWriter, r *http.Request) { // getVersion returns one snapshot in full (including content) for preview. func (h *Handler) getVersion(w http.ResponseWriter, r *http.Request) { - v, err := h.fetchVersion(chi.URLParam(r, "id"), chi.URLParam(r, "vid")) + v, err := h.fetchVersion(auth.UserID(r.Context()), chi.URLParam(r, "id"), chi.URLParam(r, "vid")) if errors.Is(err, sql.ErrNoRows) { notFoundMsg(w, "version not found") return @@ -92,7 +93,7 @@ func (h *Handler) getVersion(w http.ResponseWriter, r *http.Request) { func (h *Handler) createVersion(w http.ResponseWriter, r *http.Request) { docID := chi.URLParam(r, "id") - doc, err := h.fetch(docID) + doc, err := h.fetch(auth.UserID(r.Context()), docID) if errors.Is(err, sql.ErrNoRows) { notFound(w) return @@ -116,8 +117,9 @@ func (h *Handler) createVersion(w http.ResponseWriter, r *http.Request) { func (h *Handler) restoreVersion(w http.ResponseWriter, r *http.Request) { docID := chi.URLParam(r, "id") vid := chi.URLParam(r, "vid") + userID := auth.UserID(r.Context()) - v, err := h.fetchVersion(docID, vid) + v, err := h.fetchVersion(userID, docID, vid) if errors.Is(err, sql.ErrNoRows) { notFoundMsg(w, "version not found") return @@ -127,7 +129,7 @@ func (h *Handler) restoreVersion(w http.ResponseWriter, r *http.Request) { return } - current, err := h.fetch(docID) + current, err := h.fetch(userID, docID) if err != nil { httputil.ServerError(w, err) return @@ -142,7 +144,7 @@ func (h *Handler) restoreVersion(w http.ResponseWriter, r *http.Request) { SET title = ?, content = ?, content_text = ?, word_count = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ? AND user_id = ?`, - v.Title, v.Content, v.ContentText, v.WordCount, docID, db.LocalUserID, + v.Title, v.Content, v.ContentText, v.WordCount, docID, userID, ) if err != nil { httputil.ServerError(w, err) @@ -153,7 +155,7 @@ func (h *Handler) restoreVersion(w http.ResponseWriter, r *http.Request) { return } - doc, err := h.fetch(docID) + doc, err := h.fetch(userID, docID) if err != nil { httputil.ServerError(w, err) return @@ -284,14 +286,14 @@ func (h *Handler) pruneAutoVersions(docID string) error { } // fetchVersion loads one full snapshot, scoped to its owner via the parent doc. -func (h *Handler) fetchVersion(docID, vid string) (db.DocumentVersion, error) { +func (h *Handler) fetchVersion(userID, docID, vid string) (db.DocumentVersion, error) { var v db.DocumentVersion err := h.DB.QueryRow( `SELECT v.id, v.doc_id, v.title, v.content, v.content_text, v.word_count, v.kind, v.created_at FROM document_versions v JOIN documents d ON d.id = v.doc_id WHERE v.id = ? AND v.doc_id = ? AND d.user_id = ?`, - vid, docID, db.LocalUserID, + vid, docID, userID, ).Scan( &v.ID, &v.DocID, &v.Title, &v.Content, &v.ContentText, &v.WordCount, &v.Kind, &v.CreatedAt, diff --git a/internal/suggestions/chat.go b/internal/suggestions/chat.go index c992b68..5a33074 100644 --- a/internal/suggestions/chat.go +++ b/internal/suggestions/chat.go @@ -9,7 +9,7 @@ import ( "github.com/go-chi/chi/v5" - "gitea.parodia.dev/drwily/petal/internal/db" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/httputil" "gitea.parodia.dev/drwily/petal/internal/llm" ) @@ -46,7 +46,7 @@ func (h *Handler) chat(w http.ResponseWriter, r *http.Request) { FROM suggestions s JOIN documents d ON d.id = s.doc_id WHERE s.id = ? AND d.user_id = ?`, - sugID, db.LocalUserID, + sugID, auth.UserID(r.Context()), ).Scan(&original, &replacement, &explanation, &typ, &fromPos, &contentText) if errors.Is(err, sql.ErrNoRows) { httputil.ErrorJSON(w, http.StatusNotFound, "suggestion not found") diff --git a/internal/suggestions/handlers.go b/internal/suggestions/handlers.go index 2d078d1..7633d68 100644 --- a/internal/suggestions/handlers.go +++ b/internal/suggestions/handlers.go @@ -16,6 +16,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" "gitea.parodia.dev/drwily/petal/internal/llm" @@ -98,11 +99,11 @@ const maxMechanicsFindings = 500 func (h *Handler) mechanics(w http.ResponseWriter, r *http.Request) { docID := chi.URLParam(r, "id") - // Confirm the document exists (and is the local user's) for clean 404s. + // Confirm the document exists (and belongs to the caller) for clean 404s. var exists bool err := h.DB.QueryRow( `SELECT EXISTS(SELECT 1 FROM documents WHERE id = ? AND user_id = ?)`, - docID, db.LocalUserID, + docID, auth.UserID(r.Context()), ).Scan(&exists) if err != nil { httputil.ServerError(w, err) @@ -129,7 +130,7 @@ func (h *Handler) mechanics(w http.ResponseWriter, r *http.Request) { return } - out, err := h.fetchPending(docID) + out, err := h.fetchPending(auth.UserID(r.Context()), docID) if err != nil { httputil.ServerError(w, err) return @@ -203,11 +204,12 @@ type pass func(ctx context.Context, client llm.LLMClient, contentText, tone stri // (both families) so the client always renders a unified picture. func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.RateLimiter, run pass, scope pendingScope) { docID := chi.URLParam(r, "id") + userID := auth.UserID(r.Context()) var contentText, tone string err := h.DB.QueryRow( `SELECT content_text, tone FROM documents WHERE id = ? AND user_id = ?`, - docID, db.LocalUserID, + docID, userID, ).Scan(&contentText, &tone) if errors.Is(err, sql.ErrNoRows) { httputil.ErrorJSON(w, http.StatusNotFound, "document not found") @@ -228,7 +230,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R if !ok { // Throttled: return the existing pending set unchanged rather than an // error, so the frontend keeps showing current suggestions. - existing, err := h.fetchPending(docID) + existing, err := h.fetchPending(userID, docID) if err != nil { httputil.ServerError(w, err) return @@ -255,7 +257,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R // Return the unified pending set (grammar + voice), not just this batch, so // a grammar check never drops the voice highlights from the client and the // throttle path above stays consistent with the success path. - out, err := h.fetchPending(docID) + out, err := h.fetchPending(userID, docID) if err != nil { httputil.ServerError(w, err) return @@ -465,7 +467,7 @@ func buildSuppressor(tx *sql.Tx, docID string) (suppressor, error) { // listForDoc returns the document's current pending suggestions (used when the // editor loads a document, before any new checkpoint fires). func (h *Handler) listForDoc(w http.ResponseWriter, r *http.Request) { - out, err := h.fetchPending(chi.URLParam(r, "id")) + out, err := h.fetchPending(auth.UserID(r.Context()), chi.URLParam(r, "id")) if err != nil { httputil.ServerError(w, err) return @@ -473,13 +475,19 @@ func (h *Handler) listForDoc(w http.ResponseWriter, r *http.Request) { httputil.WriteJSON(w, http.StatusOK, out) } -func (h *Handler) fetchPending(docID string) ([]db.Suggestion, error) { +// fetchPending loads a document's pending suggestions, joined through documents +// so the rows are only reachable by the document's owner. A suggestion quotes the +// sentence it corrects, so an unscoped read here would leak document text to +// anyone holding a doc id. +func (h *Handler) fetchPending(userID, docID string) ([]db.Suggestion, error) { rows, err := h.DB.Query( - `SELECT id, doc_id, from_pos, to_pos, original, replacement, explanation, type, status, created_at - FROM suggestions - WHERE doc_id = ? AND status = ? - ORDER BY from_pos ASC, created_at ASC`, - docID, db.SuggestionStatusPending, + `SELECT s.id, s.doc_id, s.from_pos, s.to_pos, s.original, s.replacement, + s.explanation, s.type, s.status, s.created_at + FROM suggestions s + JOIN documents d ON d.id = s.doc_id + WHERE s.doc_id = ? AND d.user_id = ? AND s.status = ? + ORDER BY s.from_pos ASC, s.created_at ASC`, + docID, userID, db.SuggestionStatusPending, ) if err != nil { return nil, err @@ -554,10 +562,17 @@ func (h *Handler) dismiss(w http.ResponseWriter, r *http.Request) { h.setStatus(w, r, db.SuggestionStatusRejected) } +// setStatus accepts or dismisses one suggestion. The doc_id subquery scopes the +// write to the caller's own documents, so a stray (or guessed) suggestion id +// can't action a row belonging to another account; an unowned id simply affects +// no rows and surfaces as a 404. func (h *Handler) setStatus(w http.ResponseWriter, r *http.Request, status string) { res, err := h.DB.Exec( - `UPDATE suggestions SET status = ? WHERE id = ? AND status = ?`, + `UPDATE suggestions SET status = ? + WHERE id = ? AND status = ? + AND doc_id IN (SELECT id FROM documents WHERE user_id = ?)`, status, chi.URLParam(r, "id"), db.SuggestionStatusPending, + auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) diff --git a/internal/suggestions/handlers_test.go b/internal/suggestions/handlers_test.go index 6a00c4e..ca2a289 100644 --- a/internal/suggestions/handlers_test.go +++ b/internal/suggestions/handlers_test.go @@ -11,6 +11,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/llm" ) @@ -56,7 +57,11 @@ func newTestServer(t *testing.T, client llm.LLMClient) (http.Handler, string, *H r := chi.NewRouter() r.Route("/docs", func(dr chi.Router) { h.RegisterDocRoutes(dr) }) r.Mount("/suggestions", h.Routes()) - return r, docID, h + + // Behind the same auth middleware main.go installs: handlers resolve the + // caller from the request context, so a bare router would see no user. + authed := auth.Middleware(auth.StaticResolver(db.LocalUserID))(r) + return authed, docID, h } func do(t *testing.T, srv http.Handler, method, path, body string) *httptest.ResponseRecorder { diff --git a/internal/suggestions/isolation_test.go b/internal/suggestions/isolation_test.go new file mode 100644 index 0000000..932a3a1 --- /dev/null +++ b/internal/suggestions/isolation_test.go @@ -0,0 +1,119 @@ +package suggestions + +import ( + "encoding/json" + "net/http" + "path/filepath" + "testing" + + "github.com/go-chi/chi/v5" + + "gitea.parodia.dev/drwily/petal/internal/auth" + "gitea.parodia.dev/drwily/petal/internal/db" + "gitea.parodia.dev/drwily/petal/internal/llm" +) + +// Suggestions are scoped indirectly: the table has no user_id of its own, only a +// doc_id, so every access has to reach the owner through the parent document. A +// forgotten join here is worse than it sounds — a suggestion quotes the sentence +// it corrects, so listing another account's suggestions leaks their prose. + +// newTwoUserSuggestionServer seeds one document owned by the local user and +// returns routers for its owner and for a second, unrelated user. +func newTwoUserSuggestionServer(t *testing.T, client llm.LLMClient) (owner, stranger http.Handler, docID string) { + t.Helper() + database, err := db.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { database.Close() }) + + if _, err := database.Exec( + `INSERT INTO users (id, email, display_name) VALUES (?, ?, ?)`, + "bob", "bob@petal.local", "Bob", + ); err != nil { + t.Fatalf("seed second user: %v", err) + } + + if err := database.QueryRow( + `INSERT INTO documents (user_id, content_text) VALUES (?, ?) RETURNING id`, + db.LocalUserID, "I has two apple.", + ).Scan(&docID); err != nil { + t.Fatalf("seed doc: %v", err) + } + + mount := func(userID string) http.Handler { + h := New(database, client) + r := chi.NewRouter() + r.Route("/docs", func(dr chi.Router) { h.RegisterDocRoutes(dr) }) + r.Mount("/suggestions", h.Routes()) + return auth.Middleware(auth.StaticResolver(userID))(r) + } + return mount(db.LocalUserID), mount("bob"), docID +} + +func TestSuggestionIsolation(t *testing.T) { + client := &stubClient{response: `{"suggestions":[ + {"original":"I has","replacement":"I have","explanation":"subject-verb agreement","type":"grammar"} + ]}`} + owner, stranger, docID := newTwoUserSuggestionServer(t, client) + + // The owner runs a checkpoint so there is a real pending suggestion to guard. + rec := do(t, owner, http.MethodPost, "/docs/"+docID+"/check", "") + if rec.Code != http.StatusOK { + t.Fatalf("check: %d %s", rec.Code, rec.Body) + } + var pending []db.Suggestion + if err := json.Unmarshal(rec.Body.Bytes(), &pending); err != nil { + t.Fatalf("decode: %v", err) + } + if len(pending) != 1 { + t.Fatalf("owner has %d suggestions, want 1", len(pending)) + } + sugID := pending[0].ID + + t.Run("cannot list a stranger's suggestions", func(t *testing.T) { + rec := do(t, stranger, http.MethodGet, "/docs/"+docID+"/suggestions", "") + var out []db.Suggestion + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if len(out) != 0 { + t.Fatalf("stranger read %d suggestions (leaking %q)", len(out), out[0].Original) + } + }) + + t.Run("cannot run a pass on a stranger's document", func(t *testing.T) { + rec := do(t, stranger, http.MethodPost, "/docs/"+docID+"/check", "") + if rec.Code != http.StatusNotFound { + t.Fatalf("stranger check = %d, want 404", rec.Code) + } + }) + + // accept/dismiss take a bare suggestion id with no document in the path, so + // the write has to scope itself through doc_id → documents.user_id. + for _, action := range []string{"accept", "dismiss"} { + t.Run("cannot "+action+" a stranger's suggestion", func(t *testing.T) { + rec := do(t, stranger, http.MethodPost, "/suggestions/"+sugID+"/"+action, "") + if rec.Code != http.StatusNotFound { + t.Fatalf("stranger %s = %d, want 404 (body: %s)", action, rec.Code, rec.Body) + } + }) + } + + // After every attempt the suggestion must still be pending for its owner. + rec = do(t, owner, http.MethodGet, "/docs/"+docID+"/suggestions", "") + var after []db.Suggestion + if err := json.Unmarshal(rec.Body.Bytes(), &after); err != nil { + t.Fatalf("decode: %v", err) + } + if len(after) != 1 || after[0].Status != db.SuggestionStatusPending { + t.Fatalf("owner's suggestion was altered by the stranger: %+v", after) + } + + // And the owner can still action it — the scoping guards, it doesn't block. + rec = do(t, owner, http.MethodPost, "/suggestions/"+sugID+"/accept", "") + if rec.Code != http.StatusNoContent { + t.Fatalf("owner accept = %d, want 204 (body: %s)", rec.Code, rec.Body) + } +} diff --git a/internal/suggestions/rewrite.go b/internal/suggestions/rewrite.go index 55e2efa..6104214 100644 --- a/internal/suggestions/rewrite.go +++ b/internal/suggestions/rewrite.go @@ -9,7 +9,7 @@ import ( "github.com/go-chi/chi/v5" - "gitea.parodia.dev/drwily/petal/internal/db" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/httputil" "gitea.parodia.dev/drwily/petal/internal/llm" ) @@ -56,7 +56,7 @@ func (h *Handler) rewrite(w http.ResponseWriter, r *http.Request) { var exists int err := h.DB.QueryRow( `SELECT 1 FROM documents WHERE id = ? AND user_id = ?`, - docID, db.LocalUserID, + docID, auth.UserID(r.Context()), ).Scan(&exists) if errors.Is(err, sql.ErrNoRows) { httputil.ErrorJSON(w, http.StatusNotFound, "document not found") diff --git a/internal/suggestions/translate.go b/internal/suggestions/translate.go index 5add0b0..6045c60 100644 --- a/internal/suggestions/translate.go +++ b/internal/suggestions/translate.go @@ -8,7 +8,7 @@ import ( "github.com/go-chi/chi/v5" - "gitea.parodia.dev/drwily/petal/internal/db" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/httputil" "gitea.parodia.dev/drwily/petal/internal/llm" ) @@ -31,7 +31,7 @@ func (h *Handler) translate(w http.ResponseWriter, r *http.Request) { FROM suggestions s JOIN documents d ON d.id = s.doc_id WHERE s.id = ? AND d.user_id = ?`, - sugID, db.LocalUserID, + sugID, auth.UserID(r.Context()), ).Scan(&explanation) if errors.Is(err, sql.ErrNoRows) { httputil.ErrorJSON(w, http.StatusNotFound, "suggestion not found") diff --git a/internal/vocab/handlers.go b/internal/vocab/handlers.go index 6ff2a70..e205f49 100644 --- a/internal/vocab/handlers.go +++ b/internal/vocab/handlers.go @@ -11,6 +11,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" "gitea.parodia.dev/drwily/petal/internal/httputil" ) @@ -68,15 +69,15 @@ func scanWord(s interface { } // list returns the full garden, newest blossoms first. -func (h *Handler) list(w http.ResponseWriter, _ *http.Request) { +func (h *Handler) list(w http.ResponseWriter, r *http.Request) { h.queryList(w, `SELECT `+vocabColumns+` FROM vocab_words - WHERE user_id = ? ORDER BY created_at DESC`, db.LocalUserID) + WHERE user_id = ? ORDER BY created_at DESC`, auth.UserID(r.Context())) } // due returns only the cards whose review time has arrived, soonest first. -func (h *Handler) due(w http.ResponseWriter, _ *http.Request) { +func (h *Handler) due(w http.ResponseWriter, r *http.Request) { h.queryList(w, `SELECT `+vocabColumns+` FROM vocab_words - WHERE user_id = ? AND due_at <= datetime('now') ORDER BY due_at ASC`, db.LocalUserID) + WHERE user_id = ? AND due_at <= datetime('now') ORDER BY due_at ASC`, auth.UserID(r.Context())) } func (h *Handler) queryList(w http.ResponseWriter, query string, args ...any) { @@ -138,6 +139,8 @@ func clamp(s string, max int) string { // schedule untouched but refreshes its gloss/phonetic/example/doc_id so the most // recent context wins. Looking words up IS the data source — no extra effort. func (h *Handler) capture(w http.ResponseWriter, r *http.Request) { + userID := auth.UserID(r.Context()) + var req captureRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { httputil.ErrorJSON(w, http.StatusBadRequest, "invalid body") @@ -169,7 +172,7 @@ func (h *Handler) capture(w http.ResponseWriter, r *http.Request) { var ok int err := h.DB.QueryRow( `SELECT 1 FROM documents WHERE id = ? AND user_id = ?`, - *req.DocID, db.LocalUserID, + *req.DocID, userID, ).Scan(&ok) if errors.Is(err, sql.ErrNoRows) { httputil.ErrorJSON(w, http.StatusBadRequest, "unknown doc_id") @@ -194,14 +197,14 @@ func (h *Handler) capture(w http.ResponseWriter, r *http.Request) { phonetic = excluded.phonetic, example = CASE WHEN excluded.example != '' THEN excluded.example ELSE vocab_words.example END, doc_id = COALESCE(excluded.doc_id, vocab_words.doc_id)`, - db.LocalUserID, word, req.Gloss, req.Definition, req.Phonetic, req.Example, req.DocID, + userID, word, req.Gloss, req.Definition, req.Phonetic, req.Example, req.DocID, ) if err != nil { httputil.ServerError(w, err) return } - out, err := h.fetch(word) + out, err := h.fetch(userID, word) if err != nil { httputil.ServerError(w, err) return @@ -210,10 +213,10 @@ func (h *Handler) capture(w http.ResponseWriter, r *http.Request) { } // fetch loads one word row by its (user, word) key. -func (h *Handler) fetch(word string) (Word, error) { +func (h *Handler) fetch(userID, word string) (Word, error) { return scanWord(h.DB.QueryRow( `SELECT `+vocabColumns+` FROM vocab_words WHERE user_id = ? AND word = ?`, - db.LocalUserID, word, + userID, word, )) } @@ -225,6 +228,7 @@ type reviewRequest struct { // scheduler; the new interval is applied as `due_at = now + interval days`. func (h *Handler) review(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") + userID := auth.UserID(r.Context()) var req reviewRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { httputil.ErrorJSON(w, http.StatusBadRequest, "invalid body") @@ -249,7 +253,7 @@ func (h *Handler) review(w http.ResponseWriter, r *http.Request) { var cur State err = tx.QueryRow( `SELECT reps, interval_days, ease, lapses FROM vocab_words WHERE id = ? AND user_id = ?`, - id, db.LocalUserID, + id, userID, ).Scan(&cur.Reps, &cur.Interval, &cur.Ease, &cur.Lapses) if errors.Is(err, sql.ErrNoRows) { httputil.ErrorJSON(w, http.StatusNotFound, "word not found") @@ -269,14 +273,14 @@ func (h *Handler) review(w http.ResponseWriter, r *http.Request) { reps = ?, interval_days = ?, ease = ?, lapses = ?, last_reviewed = datetime('now'), due_at = datetime('now', ?) WHERE id = ? AND user_id = ?`, - nxt.Reps, nxt.Interval, nxt.Ease, nxt.Lapses, offset, id, db.LocalUserID, + nxt.Reps, nxt.Interval, nxt.Ease, nxt.Lapses, offset, id, userID, ); err != nil { httputil.ServerError(w, err) return } out, err := scanWord(tx.QueryRow( - `SELECT `+vocabColumns+` FROM vocab_words WHERE id = ? AND user_id = ?`, id, db.LocalUserID, + `SELECT `+vocabColumns+` FROM vocab_words WHERE id = ? AND user_id = ?`, id, userID, )) if err != nil { httputil.ServerError(w, err) @@ -293,7 +297,7 @@ func (h *Handler) review(w http.ResponseWriter, r *http.Request) { func (h *Handler) remove(w http.ResponseWriter, r *http.Request) { res, err := h.DB.Exec( `DELETE FROM vocab_words WHERE id = ? AND user_id = ?`, - chi.URLParam(r, "id"), db.LocalUserID, + chi.URLParam(r, "id"), auth.UserID(r.Context()), ) if err != nil { httputil.ServerError(w, err) diff --git a/internal/vocab/handlers_test.go b/internal/vocab/handlers_test.go index 7657841..54d8c2e 100644 --- a/internal/vocab/handlers_test.go +++ b/internal/vocab/handlers_test.go @@ -11,6 +11,7 @@ import ( "github.com/go-chi/chi/v5" + "gitea.parodia.dev/drwily/petal/internal/auth" "gitea.parodia.dev/drwily/petal/internal/db" ) @@ -23,7 +24,12 @@ func newTestServer(t *testing.T) (http.Handler, *db.DB) { t.Cleanup(func() { database.Close() }) r := chi.NewRouter() r.Mount("/vocab", New(database).Routes()) - return r, database + + // Behind the same auth middleware main.go installs: handlers resolve the + // caller from the request context, so a bare router would see no user and + // every user-scoped query would match nothing. + authed := auth.Middleware(auth.StaticResolver(db.LocalUserID))(r) + return authed, database } func do(t *testing.T, srv http.Handler, method, path, body string) *httptest.ResponseRecorder {