Add multi-user plan for review
Phase 0 (identity plumbing) is done; this writes down what real multi-user still needs and, more importantly, which decisions are genuinely open. Six OPEN items are flagged for a reviewer to push back on — chief among them whether Petal authenticates via Traefik forward-auth or becomes an OIDC client itself, which everything else depends on.
This commit is contained in:
prosolis
@@ -0,0 +1,285 @@
|
||||
# Petal multi-user plan
|
||||
|
||||
**Status:** Phase 0 (identity plumbing) landed 2026-07-26 in `6901cdb`. Everything
|
||||
below it is proposed, not agreed.
|
||||
|
||||
This document exists to be argued with. Sections marked **OPEN** are real
|
||||
decisions with real trade-offs, not rhetorical questions — a reviewer should
|
||||
push back on them. Where I have a recommendation I say so and say why.
|
||||
|
||||
---
|
||||
|
||||
## 1. Where Petal is today
|
||||
|
||||
Single user, by construction. `db.Open` seeds one row (`users.id = 'local'`) and,
|
||||
until this week, every query named that constant directly.
|
||||
|
||||
What was already right: **the schema has been multi-user-shaped from day one.**
|
||||
`documents`, `tags`, and `vocab_words` all carry `user_id`; `document_versions`
|
||||
and `suggestions` scope through their parent document. No migration is needed to
|
||||
support a second user — only a way to know which user is asking.
|
||||
|
||||
### What Phase 0 changed
|
||||
|
||||
- New `internal/auth`: `Middleware(Resolver)` resolves the caller once per API
|
||||
request and stores the id in the request context. Handlers read
|
||||
`auth.UserID(r.Context())`.
|
||||
- `Resolver` is a one-method interface — `Resolve(*http.Request) (string, error)`
|
||||
— and is the only thing a real identity provider has to implement.
|
||||
- `StaticResolver(db.LocalUserID)` supplies today's single user, so behavior is
|
||||
unchanged.
|
||||
- `/api` is split into a public group (`/health`, `/version`) and an
|
||||
authenticated group (everything else).
|
||||
- Two pre-existing access-control gaps fixed: `setStatus` (accept/dismiss) had
|
||||
**no ownership check at all**, and `fetchPending` read suggestions by `doc_id`
|
||||
alone — which leaked the quoted source sentences.
|
||||
- Two-user isolation test suites (`docs`, `suggestions`) mount the same routers
|
||||
twice behind two resolvers over one database.
|
||||
|
||||
**The remaining work is not "make Petal multi-user."** It is "authenticate
|
||||
someone, provision them, and clean up the three places where data is still
|
||||
global."
|
||||
|
||||
---
|
||||
|
||||
## 2. Goals and non-goals
|
||||
|
||||
**Goals**
|
||||
- Two or more people use one Petal instance without seeing each other's writing.
|
||||
- The existing local user's data survives, attached to a real account.
|
||||
- Adding a user is an operator action, not a code change.
|
||||
|
||||
**Non-goals (explicitly out, unless a reviewer argues otherwise)**
|
||||
- Sharing, collaboration, or multi-author documents. Petal is a private writing
|
||||
space; every feature to date assumes one reader. Sharing would change the
|
||||
passport's meaning (authorship evidence) and is a product decision, not an
|
||||
auth one.
|
||||
- Roles, permissions, or an admin UI.
|
||||
- Public signup. Accounts are provisioned deliberately.
|
||||
|
||||
---
|
||||
|
||||
## 3. Phase A — authentication
|
||||
|
||||
### OPEN #1: forward-auth vs. in-app OIDC
|
||||
|
||||
This is the biggest fork and it should be settled first, because everything in
|
||||
Phase B depends on it.
|
||||
|
||||
**Option A — Traefik forward-auth to an Authentik outpost.**
|
||||
Traefik is already in the deferred deploy bucket. Authentik's outpost terminates
|
||||
the login, and Petal receives a trusted header (`X-authentik-uid`, plus email and
|
||||
name). The `Resolver` becomes ~20 lines: read the header, map to a user id.
|
||||
|
||||
- *For:* no OIDC library, no session store, no cookie handling, no redirect
|
||||
plumbing, no token refresh, no logout endpoint. Petal keeps zero auth code.
|
||||
Login/MFA/password reset are entirely Authentik's problem.
|
||||
- *Against:* Petal is only secure if it is **never reachable except through
|
||||
Traefik**. Anyone who can hit the container directly can forge the header and
|
||||
become any user. That's a deployment invariant enforced by network config, not
|
||||
by code — and it is exactly the kind of invariant that quietly breaks. It also
|
||||
makes local development awkward (no proxy → no identity), though
|
||||
`StaticResolver` covers that.
|
||||
|
||||
**Option B — Petal is an OIDC client itself.**
|
||||
`github.com/coreos/go-oidc` + `golang.org/x/oauth2`, a `/auth/callback` route,
|
||||
and a signed session cookie. The config fields already exist
|
||||
(`AUTHENTIK_URL`, `AUTHENTIK_CLIENT_ID`, `AUTHENTIK_CLIENT_SECRET`,
|
||||
`SESSION_SECRET`).
|
||||
|
||||
- *For:* self-contained and safe to expose directly. No trust-the-proxy
|
||||
invariant. Works the same in dev and prod.
|
||||
- *Against:* meaningfully more code — a session table or signed-cookie scheme,
|
||||
CSRF on the callback, token expiry, logout. Two new dependencies in a project
|
||||
that currently has exactly two.
|
||||
|
||||
**My recommendation: Option B**, but not confidently. The deciding factor for me
|
||||
is that "must never be reachable directly" is a footgun that survives long after
|
||||
whoever set it up has forgotten, and Petal already holds someone's private
|
||||
journals. But if the deployment is definitively a single Traefik-fronted box on a
|
||||
LAN and will stay that way, Option A is *much* less code and I'd not object.
|
||||
|
||||
A reviewer should weigh: how likely is this instance ever exposed beyond the LAN?
|
||||
Does the operator want Petal to be independently deployable?
|
||||
|
||||
### Session handling (assumes Option B)
|
||||
|
||||
- Server-side sessions in a `sessions` table (id, user_id, expires_at,
|
||||
created_at, user_agent), cookie holds an opaque random id.
|
||||
Preferred over signed stateless cookies because it makes logout and
|
||||
revocation actually work — worth the one table.
|
||||
- Cookie: `HttpOnly`, `SameSite=Lax`, `Secure` when `BASE_URL` is https.
|
||||
- **OPEN #2:** session lifetime. This is a personal writing tool used daily on
|
||||
trusted devices; a 30-day sliding expiry is kind, an 8-hour one is
|
||||
conventional. I lean long — an editor that logs you out mid-draft is hostile,
|
||||
and the auto-save makes a surprise 401 genuinely costly. Needs a decision.
|
||||
|
||||
### The 401 problem
|
||||
|
||||
Every frontend fetch currently assumes success. Once a session can expire,
|
||||
**any** call can return 401 mid-session — including the 1.5s auto-save, which is
|
||||
the one that must not fail silently.
|
||||
|
||||
Proposal: a single interceptor in `web/src/api/client.ts` that, on 401, halts
|
||||
auto-save, surfaces a warm bilingual "请重新登录 / Please sign in again" state
|
||||
rather than a raw error, and preserves unsaved editor content across the
|
||||
re-login (localStorage draft keyed by doc id). This is small but easy to forget,
|
||||
and getting it wrong means lost writing.
|
||||
|
||||
---
|
||||
|
||||
## 4. Phase B — user provisioning and migration
|
||||
|
||||
### Provisioning
|
||||
|
||||
On first successful login, upsert a `users` row from the OIDC claims
|
||||
(`sub` → `users.id`, plus email and name). No signup flow; whoever Authentik lets
|
||||
in gets an account.
|
||||
|
||||
**OPEN #3:** should there be an allowlist? Authentik may host other applications
|
||||
with a broader user set than Petal should have. A `PETAL_ALLOWED_SUBS` env var,
|
||||
or an Authentik group check on a claim, would gate it. Probably yes, cheaply.
|
||||
|
||||
### Migrating the existing `local` user
|
||||
|
||||
The live database on millenia holds real writing under `user_id = 'local'`. That
|
||||
data must end up owned by the wife's real account.
|
||||
|
||||
Recommended: a migration that **renames** rather than copies — update the
|
||||
`users.id` and let `ON UPDATE CASCADE`… except SQLite FKs here are declared
|
||||
without `ON UPDATE`, so this needs either a deliberate multi-table update inside
|
||||
one transaction (`documents`, `tags`, `vocab_words` — versions and suggestions
|
||||
follow their parents) with `PRAGMA foreign_keys=OFF` around it, or an explicit
|
||||
one-off admin command.
|
||||
|
||||
I'd rather do this as a **documented one-off script run with the app stopped and
|
||||
a copy of the DB taken first** than as an automatic startup migration, because it
|
||||
depends on knowing the new OIDC subject id, which isn't available until that
|
||||
person logs in once. Sequence: deploy auth → she logs in → new empty account is
|
||||
created → stop app, back up, run script to move `local`'s rows onto her real id,
|
||||
delete the empty row → restart.
|
||||
|
||||
**OPEN #4:** is that acceptable, or is a small admin endpoint preferable to a
|
||||
script? The existing project convention (`scripts/`, hand-run Python) suggests a
|
||||
script is in keeping.
|
||||
|
||||
---
|
||||
|
||||
## 5. Phase C — the data that is still global
|
||||
|
||||
Found during the Phase 0 audit. None of these break with two users; all of them
|
||||
leak or bleed.
|
||||
|
||||
### Image store — the real one
|
||||
|
||||
`internal/images` is a flat content-addressed directory. There is no per-user
|
||||
association and no database row at all. Any authenticated user who knows a
|
||||
sha256 can fetch any other user's image.
|
||||
|
||||
That is capability-URL security. Hashes aren't guessable, so this is not an
|
||||
emergency — but "unguessable filename" is not access control, and images pasted
|
||||
into a private journal are exactly the content that shouldn't rely on it.
|
||||
|
||||
Proposal: an `images` table (hash, user_id, content_type, created_at, size) with
|
||||
the fetch handler joining on the caller. Content addressing is kept — the same
|
||||
image uploaded by two users is stored once on disk and simply has two rows, so
|
||||
deduplication survives. Deleting the last row referencing a hash removes the
|
||||
file.
|
||||
|
||||
**OPEN #5:** is this worth doing before real multi-user, or is it acceptable to
|
||||
ship auth first and treat this as a known limitation? I lean toward doing it *in
|
||||
the same phase as auth*, since the moment a second account exists the exposure is
|
||||
real and the fix requires a migration either way.
|
||||
|
||||
### Frontend `localStorage`
|
||||
|
||||
`petal.spell.personal` (personal dictionary), `petal.companion` (chosen mascot),
|
||||
plus sound and petal-effect preferences are all per-browser. Two users on one
|
||||
device share them — and the personal dictionary is the one that matters, since
|
||||
it's built from someone's own writing.
|
||||
|
||||
Cheapest fix: namespace every key by user id once the client knows who it is.
|
||||
The honest fix for the dictionary is to move it server-side into a table, which
|
||||
also means it follows a user between devices — arguably a feature.
|
||||
|
||||
### Not affected
|
||||
|
||||
`export-all` is correctly scoped. The TTS cache is content-addressed audio of
|
||||
text the requester supplied, no cross-user inference. The lexicon is a static
|
||||
dataset identical for everyone.
|
||||
|
||||
---
|
||||
|
||||
## 6. Phase D — per-user language
|
||||
|
||||
Tracked here because it lands on the same `users` row and shouldn't be designed
|
||||
twice.
|
||||
|
||||
**English is always the target language.** What varies is the user's *native*
|
||||
language — the one glosses and explanations are written in. Mandarin ships today;
|
||||
European Portuguese (pt-PT, explicitly not pt-BR) is wanted; French is possible.
|
||||
|
||||
That makes native language a `users` column and turns these into per-user
|
||||
lookups:
|
||||
|
||||
| Piece | Today | Notes |
|
||||
|---|---|---|
|
||||
| Gloss / lexicon data | ECDICT, English↔Chinese | pt-PT needs its own source — **this is the hard part**, not the code |
|
||||
| LLM prompt copy | Mandarin-first bilingual | `internal/llm/prompts.go` |
|
||||
| Companion tips | bilingual `tips.ts` | per-language copy |
|
||||
| TTS L1 voice | Piper zh_CN-huayan | Piper has pt-PT voices |
|
||||
| Font stacks | CJK fallbacks | not needed for Latin-script L1 |
|
||||
|
||||
English-side machinery (nspell en-US, the IPA/phonetic dataset, the EN Piper
|
||||
voice) is unaffected and stays shared.
|
||||
|
||||
**OPEN #6:** the gloss dataset is the blocker, not the plumbing. Is there an
|
||||
open English↔Portuguese dictionary of ECDICT's quality and license? If not, this
|
||||
phase is gated on data sourcing and the code work is comparatively trivial. Worth
|
||||
answering before scheduling it.
|
||||
|
||||
---
|
||||
|
||||
## 7. Suggested sequence
|
||||
|
||||
1. **Settle OPEN #1** (forward-auth vs. in-app OIDC). Everything else follows.
|
||||
2. Deploy plumbing: Dockerfile, Traefik, real hostname, HTTPS. Auth needs a
|
||||
stable `BASE_URL` and a redirect URI regardless of which option wins.
|
||||
3. Auth itself: `Resolver` implementation, sessions if applicable, frontend 401
|
||||
handling.
|
||||
4. Image store table + migration (same phase, per OPEN #5).
|
||||
5. Provision the second real account; migrate `local`'s data.
|
||||
6. `localStorage` namespacing.
|
||||
7. Per-user language, gated on the dataset question.
|
||||
|
||||
---
|
||||
|
||||
## 8. Risks
|
||||
|
||||
- **Silent unscoping.** Phase 0 hit this exactly once: `docs.fetch` took a
|
||||
`userID` parameter and kept binding `db.LocalUserID` in the query. Unused
|
||||
parameters are legal Go — it compiled, `vet` was silent, and every existing
|
||||
test passed while the lookup stayed unscoped. Only the two-user isolation test
|
||||
caught it. **Every new user-scoped endpoint should get an isolation case in the
|
||||
same commit**; the existing suites are the template.
|
||||
- **Migrating live data.** The wife's real writing is the thing being moved.
|
||||
Back up first, run with the app stopped, verify counts before deleting
|
||||
anything.
|
||||
- **A 401 mid-draft losing work.** See Phase A.
|
||||
- **Scope creep into sharing.** Multi-user and collaboration are different
|
||||
products. Adding accounts should not quietly become adding sharing.
|
||||
|
||||
---
|
||||
|
||||
## 9. Questions for the reviewer
|
||||
|
||||
1. Forward-auth or in-app OIDC? (OPEN #1 — the one that matters most)
|
||||
2. Session lifetime, given a daily-use editor with auto-save? (OPEN #2)
|
||||
3. Allowlist Petal accounts separately from Authentik's user set? (OPEN #3)
|
||||
4. Migration script vs. admin endpoint for moving `local`'s data? (OPEN #4)
|
||||
5. Fix the image store alongside auth, or ship auth with it as a known
|
||||
limitation? (OPEN #5)
|
||||
6. Is there a usable open English↔European-Portuguese dictionary dataset?
|
||||
(OPEN #6 — gates Phase D entirely)
|
||||
|
||||
Anything above that reads as settled but shouldn't be is also fair game.
|
||||
Reference in New Issue
Block a user