drwily/petal
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Multi-user groundwork: request-scoped user identity

Browse Source 
Petal ran as a single hardcoded user, with db.LocalUserID named directly
at ~35 query sites. That made the caller's identity a compile-time
constant scattered across every package — nothing a real login could
replace without touching all of them.

New internal/auth moves it into the request context:

  - Middleware(Resolver) resolves the caller once per API request
  - handlers read auth.UserID(r.Context()) instead of naming a user
  - Resolver is the seam an Authentik session check drops into
  - StaticResolver(db.LocalUserID) keeps Petal single-user today

Behavior is unchanged. UserID returns "" rather than panicking when the
middleware is absent, so a mis-wired route fails closed: every query is
WHERE user_id = ?, which then matches nothing.

main.go splits /api into a public group (/health, /version) and an
authenticated group for everything else — a monitoring probe must not
need a session.

Two pre-existing access-control gaps fixed while threading, both
harmless with one user and not with two:

  - setStatus (accept/dismiss) updated a suggestion by bare id with no
    ownership check at all
  - listForDoc/fetchPending read a document's suggestions by doc_id
    alone; a suggestion quotes the sentence it corrects, so that leaked
    the source prose

Both now scope through documents.user_id.

Tests: internal/auth covers the context round-trip, the absent-context
case, and both 401 paths. Two-user isolation suites in docs and
suggestions mount the same routers twice behind two resolvers over one
database and assert a stranger gets 404 on every id-taking path, sees
nothing in list/search, and leaves the owner's data untouched.

Those suites earned their keep immediately: docs.fetch gained a userID
parameter but kept binding db.LocalUserID in the query. Unused
parameters are legal Go, so it compiled clean, vet was silent, and every
existing test passed while the lookup stayed unscoped.

Still global, out of scope and flagged in BUILD_PLAN.md: the image store
has no per-user association, and frontend localStorage keys are
per-browser rather than per-account.
This commit is contained in:
prosolis
2026-07-26 21:42:37 -07:00
parent 61b3c6cd62
commit 6901cdbbe4
21 changed files with 692 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/suggestions/chat.go
+2 -2
View File
@@ -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")
internal/suggestions/handlers.go
+29 -14
View File
@@ -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)
internal/suggestions/handlers_test.go
+6 -1
View File
@@ -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 {
internal/suggestions/isolation_test.go
+119
View File
@@ -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)
}
}
internal/suggestions/rewrite.go
+2 -2
View File
@@ -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")
internal/suggestions/translate.go
+2 -2
View File
@@ -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")