diff --git a/docs/superpowers/specs/2026-07-25-per-user-profile-design.md b/docs/superpowers/specs/2026-07-25-per-user-profile-design.md new file mode 100644 index 0000000..188bccb --- /dev/null +++ b/docs/superpowers/specs/2026-07-25-per-user-profile-design.md @@ -0,0 +1,129 @@ +# Per-user profile & data isolation — design + +Date: 2026-07-25 + +## Overview + +`docs/superpowers/specs/2026-07-24-oidc-authentication-design.md` deliberately scoped OIDC login as an access gate only: every authenticated+authorized user reaches the same singleton `profile` row and the same dataset. That doc explicitly called multi-tenancy "a separate, much larger project" and listed it as a non-goal. + +This is that project. Each OIDC-authenticated user now gets their own profile, their own Garmin credentials, and their own fully independent synced/classified dataset. There is no cross-user visibility and no admin/impersonation path — switching to a different user's data is only possible by logging out and back in as that user. + +This reverses the "single active profile" decision recorded in `docs/superpowers/specs/2026-07-17-profile-taxonomy-analysis-engine-design.md`. + +## Goals + +- Every table that currently holds a singleton or globally-shared row (`profile`, `workout_kinds`, `sync_state`) or shared rows (`activities`, `sync_runs`) becomes scoped to the logged-in user. +- A user's `user_id` is derived exclusively from their signed session cookie (the OIDC `sub`, resolved server-side) — never from any client-supplied path/query/body parameter. This is the entire isolation boundary and must hold in every handler without exception. +- New users get an explicit one-screen setup step (display name only) before they can use the rest of the app; nothing is auto-provisioned silently. +- The existing single-user dataset (today's "Kriss" profile and everything synced under it) becomes one specific user's data via a one-time migration step, with zero data loss and zero re-sync required. +- Each user brings their own Garmin account; Garmin sync, MFA, and classification all keep working exactly as they do today, just scoped per user. + +## Non-goals + +- Any admin UI, user list, or "view as" capability. There is deliberately no way to see or touch another user's data short of holding their session cookie. +- Self-service account deletion/merging. Not needed yet; can be a later, separate project. +- Changing the OIDC login flow itself (`/api/session/*`), the required-role gate, or the Garmin credential/MFA flow (`/api/auth/*`) — all of that is reused as-is, just now resolved per-user instead of globally. +- Any change to the classification rule engine (`internal/classify`) itself — it's already pure (`MetricContext` + `RuleKind`s in, verdict out) and has no notion of "whose" data it's operating on. + +## Data model + +### New table + +```sql +CREATE TABLE users ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + oidc_sub TEXT NOT NULL UNIQUE, + display_name TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (datetime('now')) +); +``` + +### Scoped tables + +| Table | Change | +|---|---| +| `profile` | add `user_id INTEGER NOT NULL REFERENCES users(id)`, `UNIQUE(user_id)`; drop `CHECK(id = 1)`. Queries move from `WHERE id = 1` to `WHERE user_id = ?`. | +| `workout_kinds` | add `user_id INTEGER NOT NULL REFERENCES users(id)`; unique constraint becomes `UNIQUE(user_id, name)` instead of `UNIQUE(name)`. | +| `activities` | add `user_id INTEGER NOT NULL REFERENCES users(id)`; unique constraint becomes `UNIQUE(user_id, garmin_activity_id)` instead of `UNIQUE(garmin_activity_id)`. | +| `sync_state` | add `user_id INTEGER NOT NULL REFERENCES users(id)`, `UNIQUE(user_id)`; drop `CHECK(id = 1)`. | +| `sync_runs` | add `user_id INTEGER NOT NULL REFERENCES users(id)`. | + +### Tables that stay unchanged + +`laps`, `activity_samples`, `kind_assignments`, `workout_type_paces` are never queried except joined off `activity_id` or `workout_kind_id`, whose owning row already carries `user_id`. Adding a redundant `user_id` there would just create a second source of truth to keep in sync — ownership is checked transitively (e.g. `SELECT ... FROM laps JOIN activities ON activities.id = laps.activity_id WHERE activities.user_id = ? AND laps.id = ?`). + +### Migration mechanics + +SQLite's `ALTER TABLE ADD COLUMN` can't add a `NOT NULL` column with no default to a table that already has rows without a full table rebuild. Since every environment this runs in (today, just this one deployment) is about to have its rows backfilled in the same upgrade anyway, the pragmatic sequence across two new numbered migration files is: + +1. `00XX_users_table.sql` — creates `users`. +2. `00XX_scope_tables_to_user.sql` — adds each `user_id` column as nullable (no `NOT NULL` yet, since SQLite requires a default for that on a non-empty table and there's no sensible default at schema-definition time), adds the new/changed unique constraints. The `NOT NULL` behavior is then enforced at the application layer from here on (every insert path is being rewritten anyway as part of this change), matching how this codebase already treats some invariants as store/API-layer contracts rather than DB-level constraints (e.g. `kind_assignments.status` values). + +### Legacy data binding + +Rather than a placeholder-user-then-manual-claim dance, a Go-level bootstrap step (not raw SQL, since migrations can't take parameters) runs once at `geniusrund` startup: + +- New required-at-this-one-time env var: `GENIUSRUN_LEGACY_OWNER_OIDC_SUB` — your Keycloak `sub` claim. +- On startup, if the `users` table is empty and any pre-existing `profile`/`activities`/`workout_kinds`/`sync_state`/`sync_runs` rows have `user_id IS NULL`: insert one `users` row (`oidc_sub = GENIUSRUN_LEGACY_OWNER_OIDC_SUB`, `display_name` = the existing `profile.name` column value), then `UPDATE` every such row to that new `user_id`, in one transaction. +- Once this has run, the env var is no longer read (the `users` table is no longer empty, so the bootstrap step is a no-op on every subsequent startup) and can be removed from the environment. +- This requires knowing your own `sub` ahead of time — it's already visible today via `GET /api/session/me` (`Claims.Sub`) after logging in once under the current single-tenant code, before this migration ships. + +## Auth & per-user runtime wiring + +### Session -> user resolution + +A new middleware step runs immediately after the existing `RequireSession` (role-checked) middleware: + +- Looks up `SELECT id, display_name FROM users WHERE oidc_sub = ?` using the already-verified session claims' `Sub`. +- Found: injects the resolved `user_id` (and `display_name`) into the request context via a new `UserFromContext(ctx)` helper, alongside the existing `ClaimsFromContext`. +- Not found: every route except `POST /api/setup` and the existing `/api/session/logout` returns `403` (or the frontend equivalent — see below) instead of reaching its handler. + +**Security invariant** (the entire isolation boundary): every handler that touches `profile`, `activities`, `workout_kinds`, `sync_state`, `sync_runs`, or anything joined off them must obtain `user_id` from `UserFromContext(ctx)` only — never from a URL param, query string, or request body field. This is a specific thing to check for in code review for every handler touched by this change. + +### `garmin.Client` / `sync.Service` become per-user + +`internal/garmin.Client` and `internal/sync.Service` already take their config/dependencies as constructor params with no hidden global state, so per-user instantiation is mechanical: + +- `api.Server`'s current single fixed `Garmin`/`Sync` fields and flat Garmin-auth-status fields (today: plain fields on `Server`, single-user assumption) become two mutex-guarded maps: `map[int64]*garmin.Client` and `map[int64]*sync.Service`, keyed by `user_id`, lazily constructed on first access for a given user. +- A per-user `garmin.Client` is built from that user's own `profile` row (`GarminEmail`/`GarminPassword`) and a per-user token store path: `filepath.Join(cfg.GarminTokenStoreRoot, strconv.FormatInt(userID, 10))`. `GarminTokenStore` in `internal/config` is renamed/repurposed to `GarminTokenStoreRoot` (a directory, not a single tokenstore path) to reflect this. +- `sync.Service.Progress()` becomes naturally per-user once each user has their own `Service` instance — no separate change needed there. +- `store.DB` stays a single shared connection against one SQLite file — this design keeps the single-shared-DB-with-`user_id`-columns approach, so no per-user DB file/connection is needed. + +## New-user setup flow + +On first login, the session->user middleware finds no `users` row for that `sub`. The frontend shows a "Create your profile" screen (structurally similar to how `LoginGate` already blocks unauthenticated users) asking only for **Display Name**. + +On submit, `POST /api/setup {display_name}` runs one transaction via a new `store.ProvisionUser(ctx, oidcSub, displayName string) (userID int64, err error)`: + +1. Insert the `users` row. +2. Insert a default `profile` row for that `user_id` (same defaults as today's fresh-install `profile` seed row). +3. Seed the same 8 default `workout_kinds` rows (same placeholder rules as migrations `0004`/`0007`) for that `user_id`. +4. Insert a default `sync_state` row (`earliest_synced_date = NULL`, `backfill_complete = 0`). + +This seeding logic must exist as reusable Go code rather than living only in migration SQL, since it now runs continuously (every new signup) rather than once at deploy time. Garmin credentials are filled in afterward via the existing Profile screen, unchanged from today's flow. + +## Frontend changes + +- New "Create your profile" screen/component, shown when `GET /api/session/me` (or a follow-up call) indicates an authorized session with no provisioned profile yet. Only field: Display Name. +- Existing Profile screen: the `profile.name` column (added in migration `0011` for exactly this eventual purpose) becomes the user-editable "Display Name" field — no schema change needed there, it already exists. +- No profile switcher, no user list, anywhere in the UI. + +## API changes summary + +Every existing handler in `internal/api` that reads/writes `profile`, `activities`, `workout_kinds`, `sync_state`, or `sync_runs` (directly or via `internal/sync`/`internal/store` calls) gets a `userID` parameter threaded from `UserFromContext(ctx)` down through the corresponding `store`/`sync` method signatures. This is a mechanical but wide-reaching signature change — most of `internal/store`'s non-classification methods, `internal/sync.Service`'s methods, and their `internal/api` callers are touched. + +## Testing plan + +- `internal/store`: a shared test helper provisions a `users` row (and typically a full `ProvisionUser` call) before exercising any scoped table, replacing the current "just open a temp DB" setup. New test cases: + - Two users' `profile`/`workout_kinds`/`activities`/`sync_state` rows never leak into each other's `Get*` calls. + - The new unique constraints (`UNIQUE(user_id, name)`, `UNIQUE(user_id, garmin_activity_id)`) allow the same name/Garmin ID across different users but reject duplicates within one user. +- `internal/api`: `httptest` setup logs in as a specific test user (sets a session cookie carrying that user's `sub`, matching a seeded `users` row) for existing tests; new tests assert that user A's session can never read or mutate user B's `profile`/`activities`/`workout_kinds` even when handed user B's internal IDs directly (the security invariant, tested adversarially). +- `internal/sync`/`internal/garmin`: verify the keyed-map lazy construction — two users produce two independent `Client`/`Service` instances, and one user's `Progress()` counters never reflect another user's in-flight sync. +- New-user flow: `POST /api/setup` with no prior `users` row succeeds and seeds all four tables in one transaction; a second call for the same `sub` is rejected (already provisioned). +- Legacy migration bootstrap: a test DB with pre-migration singleton rows (`user_id IS NULL`) plus `GENIUSRUN_LEGACY_OWNER_OIDC_SUB` set gets every row correctly attributed to one new user; running the bootstrap a second time (now that `users` is non-empty) is a no-op. + +## Rollout notes + +- Before deploying: log into the current single-tenant build once, note your `sub` from `GET /api/session/me`, and set `GENIUSRUN_LEGACY_OWNER_OIDC_SUB` to it for the first startup after upgrading. +- `GARMIN_TOKENSTORE` env var / `GarminTokenStore` config field is repurposed as a root directory (`GarminTokenStoreRoot`) rather than a single tokenstore path — existing deployments need to point it at a directory rather than mcp-garmin's old single-file/dir location, and any already-cached Garmin session under the old path is effectively invalidated (that one user will need to log into Garmin again post-migration, once, under their new per-user subdirectory). +- No frontend routing changes beyond the new setup screen — everything else (Dashboard, Review Queue, Plan stub, Profile) is unchanged in shape, just now reflecting whichever user is logged in.