From 3022e80c04d28c2408aaf7432447f29f40ab232b Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Tue, 26 May 2026 22:54:35 -0500 Subject: [PATCH] refactor(auth): collapse lib/auth-context.js + lib/admin-auth.js onto lib/use-auth.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lib/use-auth.js` is now the sole client-side auth surface (P1 §9 of `.convoys/ship-readiness.md`). The legacy `lib/auth-context.js` (`AuthProvider` + `useAuth`) and `lib/admin-auth.js` (`AdminProvider` + `useAdmin` + `useIsAdmin`) are deleted; every importer is migrated to the canonical hook. Pre-convoy a worst-case page mount issued THREE identical `GET /api/auth/verify` requests (one per provider/hook); the post-convoy floor is one verify per page mount (3 → 1 on `pages/card/[id].js`, 2 → 1 elsewhere). Importer inventory swept (7 source files): - `pages/_app.js` — removed `` wrapper; `` is now the only top-level provider. `lib/use-auth.js` is hook-only, no replacement provider needed. - `pages/index.js`, `pages/scanner.js`, `pages/decks.js`, `pages/deck/[id].js`, `pages/deck-builder.js` — `import { useAuth }` path swap from `../lib/auth-context` to `../lib/use-auth`. All five pages destructured only `{ user }` or `{ user, loading }`; verified no consumer reads `login` / `register` from useAuth (those flows are in `pages/login.js` / `pages/signup.js` which call the API directly), so no shape-parity gap on `lib/use-auth.js`. - `pages/card/[id].js` — replaced `useIsAdmin()` (the only consumer of `lib/admin-auth.js` anywhere in the tree) with synchronous `user?.role === 'admin'` derived from the existing `useAuth()` call. Render condition at line 524 stays byte-identical. Decisions documented in `.convoys/single-auth-provider.md`: - D1: no extension to `lib/use-auth.js` (zero call sites for `login` / `register` from useAuth — those flows are direct fetches in `login.js` / `signup.js`). - D2: `useIsAdmin()` collapses onto `useAuth()`; no separate hook. - D3: provider tree `{children}` → `{children}`. - D4: 3 → 1 verify roundtrip on `card/[id].js`; 2 → 1 on every other page-load. - D5: zero test files modified; the 21-test vitest suite is server- side or prop-driven (`Layout.test.js` passes `user` as a prop, never imports the legacy hooks). Doc / config updates so the deletion lands cleanly: - `.github/CODEOWNERS` — drop the two CODEOWNERS lines for the deleted files. - `AGENTS.md` § 2 architecture row + § 3 "Auth (client)" bullet — rewritten for the post-convoy single-surface state. - `.cursor/rules/auth-and-permissions.mdc` — § "Legacy" reframed to "deleted by this convoy"; § "Authentication state on the client" updated to the post-convoy `useAuth()` shape and the direct-fetch login flow used by `login.js` / `signup.js`. - `.cursor/rules/no-go-zones.mdc` — auth-refactors bullet drops the deleted files from the canonical list. - `.cursor/skills/add-page/SKILL.md` — checklist + anti-pattern row refer to the deletion. Verification: - `rg "lib/auth-context|lib/admin-auth" --type js` → 0 hits in source. - `npm run lint` → 128 → 125 problems (3 fewer errors from the deleted unused-import lines; no regression). - `npm run test:run` → 21/21 pass (including the 5 Layout regression locks from `fix-layout-default-user`, which are prop-driven and unaffected). - `npm run build` → all 26 pages compile end-to-end; no SSR / static- generation breakage that would have surfaced if a page tried to use the legacy context hook unwrapped. - Manual smoke deferred to operator post-merge per convoy doc. Risks (full discussion in convoy file): - R1 shape parity gap — verified zero consumers of legacy-only surface; mitigated. - R2 SSR mismatch from removing `` — `useEffect`- guarded `localStorage` read; identical SSR shape pre/post; build passes. - R3 missed importer — post-delete grep + build pass would surface any miss. - R5 stale `useAuth` cache across components — pre-existing pattern, called out as follow-up rather than addressed here. Out of scope: any change to `lib/permission-middleware.js` (server- side; resolved P0 #1), `lib/auth-secret.js` (resolved P0 #2), `pages/api/**` route handlers, login / register API contracts, or the seeded admin account flow. Co-authored-by: Cursor --- .convoys/single-auth-provider.md | 317 +++++++++++++++++++++++++ .cursor/rules/auth-and-permissions.mdc | 15 +- .cursor/rules/no-go-zones.mdc | 2 +- .cursor/skills/add-page/SKILL.md | 4 +- .github/CODEOWNERS | 2 - AGENTS.md | 4 +- lib/admin-auth.js | 120 ---------- lib/auth-context.js | 116 --------- pages/_app.js | 7 +- pages/card/[id].js | 7 +- pages/deck-builder.js | 2 +- pages/deck/[id].js | 2 +- pages/decks.js | 2 +- pages/index.js | 2 +- pages/scanner.js | 2 +- 15 files changed, 341 insertions(+), 263 deletions(-) create mode 100644 .convoys/single-auth-provider.md delete mode 100644 lib/admin-auth.js delete mode 100644 lib/auth-context.js diff --git a/.convoys/single-auth-provider.md b/.convoys/single-auth-provider.md new file mode 100644 index 0000000..a2b0b3d --- /dev/null +++ b/.convoys/single-auth-provider.md @@ -0,0 +1,317 @@ +# single-auth-provider (P1 quality — collapse three client auth surfaces onto one) + +**Status:** OPEN 2026-05-26 (this convoy) +**Priority:** P1 quality (launch sequence step 9 — `.convoys/ship-readiness.md` § P1 entry 9) +**Convoy owner:** parent (architect + implementer rolled together — diff is mechanical once the +shape-parity decision is made) +**Branch:** `convoy/single-auth-provider` +**Opened:** 2026-05-26 + +## Background + +The repo carried **three parallel client-side auth implementations** since the early days of the +project. `.convoys/ship-readiness.md` § P1 entry 9 ("Three parallel client-side auth +implementations") is the canonical spec; AGENTS.md § 3 already documented `lib/use-auth.js` as the +canonical surface and instructed new code to avoid the other two. This convoy executes the +collapse. + +The three surfaces: + +1. **`lib/use-auth.js::useAuth`** (the keeper). Hook-only — reads `auth_token` from + `localStorage` on mount, hits `/api/auth/verify`, exposes `{ user, loading, logout, refreshAuth }`. + No React context, no `` wrapper required. +2. **`lib/auth-context.js::{ AuthProvider, useAuth }`** (legacy). Context provider + consumer + hook with the same verify-on-mount semantics, plus `login()` and `register()` helpers that + `pages/login.js` / `pages/signup.js` no longer use (those pages call `/api/auth/{login,register}` + directly and write the token to `localStorage` themselves). Wired in `pages/_app.js` as + ``. +3. **`lib/admin-auth.js::{ AdminProvider, useAdmin, useIsAdmin }`** (legacy). A redundant context + that does the *same* verify-on-mount roundtrip, plus a hook-only `useIsAdmin()` that does its + own verify roundtrip on top of that. `AdminProvider` is **not** wired in `_app.js` (verified + by reading `_app.js` pre-convoy: only `` + ``), so `useAdmin()` + would have thrown at runtime if anyone called it — nobody does. Only `useIsAdmin()` has a + live consumer (`pages/card/[id].js`). + +**Symptom that drove ranking this P1.** When `pages/card/[id].js` mounts, it calls +`useAuth()` from `lib/use-auth.js` AND `useIsAdmin()` from `lib/admin-auth.js`, each issuing its +own `GET /api/auth/verify`. With `AuthProvider` mounted on every page via `_app.js`, that's a +**third** verify roundtrip on the very first page load. Three roundtrips, identical request, +serial cost on a cold connection. Post-convoy: 1 roundtrip per page-load. + +## Decisions + +### D1 — Shape parity check on `lib/use-auth.js`. Verdict: no parity gap; do **not** extend. + +`lib/auth-context.js::useAuth()` exposed `{ user, loading, login, register, logout }`. +`lib/use-auth.js::useAuth()` exposes `{ user, loading, logout, refreshAuth }`. + +The apparent gap is `login` / `register`. Verified-by-grep: **zero call sites** invoke +`useAuth().login(…)` or `useAuth().register(…)` anywhere in `pages/**` or `components/**`. The +only callers of those flows are `pages/login.js` and `pages/signup.js`, both of which `fetch` +`/api/auth/{login,register}` directly and write the returned token to `localStorage`. +`useAuth()`'s `useEffect` then picks up the new token on the next mount (or the page can call +`refreshAuth()` to re-verify in place). + +Conclusion: do **not** add `login` / `register` to `use-auth.js`. The legacy methods were dead +code on the consumer surface; preserving them would be cargo-culting and would re-create a +non-DRY login flow (one in `pages/login.js`, one in the hook). `auth-and-permissions.mdc` § +"Authentication state on the client" was updated to document the post-convoy `useAuth()` shape +and to spell out the `login.js` / `signup.js` direct-fetch pattern. + +### D2 — `useIsAdmin()` migration shape. Verdict: collapse onto the existing `useAuth()` call. + +`pages/card/[id].js` is the **only** consumer of `useIsAdmin()`. The page already called +`useAuth()` from `lib/use-auth.js` at line 13 (added by `fix-layout-default-user` Brief 2). The +migration is: + +```js +// Before +const { user } = useAuth(); +// ... +const { isAdmin, loading: adminLoading } = useIsAdmin(); +// ... usage at line 524: {isAdmin && !adminLoading && (...)} + +// After +const { user, loading: authLoading } = useAuth(); +// ... +const isAdmin = user?.role === 'admin'; +const adminLoading = authLoading; +// ... usage at line 524 unchanged: {isAdmin && !adminLoading && (...)} +``` + +`adminLoading` is kept as a local alias rather than substituting `authLoading` directly at the +call site, to keep the diff minimal and the rendering condition byte-identical. The `loading` +window from `useAuth()` covers exactly the same period (`/api/auth/verify` resolution) that +`useIsAdmin`'s own loading covered, so there is no UX regression. + +### D3 — `pages/_app.js` provider tree. Before / after. + +```jsx +// Before + + + + + + +// After + + + +``` + +`useAuth()` from `lib/use-auth.js` is hook-only — no Provider needed. The `` +wrapper is removed entirely; no replacement Provider is added. `` stays (out of +scope). `` was never in the tree to begin with. + +### D4 — Token-verify roundtrip count. + +Per the spec: pre-convoy a worst-case page mount issued **3** identical `GET /api/auth/verify` +requests: + +1. `` in `_app.js` calls `verifyToken()` on mount. +2. `pages/card/[id].js` calls `useAuth()` from `lib/use-auth.js`, which calls `checkAuth()` on + mount → another verify. +3. The same page calls `useIsAdmin()` from `lib/admin-auth.js`, which calls its inline + `checkAdmin()` on mount → another verify. + +Post-convoy: + +1. `` is gone. +2. `pages/card/[id].js` calls `useAuth()` once → 1 verify. +3. `useIsAdmin()` call site is gone; admin status is computed synchronously from the same + `user` returned by step 2. + +Net: **3 → 1** verify roundtrip on `card/[id].js` mount. Other pages drop from **2 → 1** +(no `useIsAdmin` involved, but `` was). The 1× pattern is the floor; further +reduction would require server-side hydration of the user object, which is a separate +architectural conversation (out of scope; see Follow-ups). + +### D5 — Test impact. Verdict: zero test files modified. + +The 21-test vitest suite covers: + +- `test/lib/auth-secret.test.js` (3) — server-side, untouched by this convoy. +- `test/lib/permission-middleware.test.js` (8) — server-side, untouched. +- `test/api/auth-utils.test.js` (5) — server-side, untouched. +- `test/components/Layout.test.js` (5) — passes `user` as a *prop*, not via any hook. The + legacy `auth-context` and `admin-auth` modules are not imported. Unaffected. + +All four files were `grep`-checked for `auth-context|admin-auth|use-auth` references — zero +hits. No test was written against the legacy hooks themselves; the deletion is risk-free from a +test-suite perspective. Vitest stays green at 21/21 post-convoy. + +## Importer inventory + +Generated via `rg "from ['\"].*lib/auth-context['\"]" --type js` and +`rg "from ['\"].*lib/admin-auth['\"]" --type js` against the worktree (excluding docs / convoys). + +### Importers of `lib/auth-context.js` (6 source files) + +| File | Symbol | Migration | +| --- | --- | --- | +| `pages/_app.js` | `AuthProvider` | Wrapper removed; no replacement (D3) | +| `pages/index.js` | `useAuth` | Path swap → `lib/use-auth.js` | +| `pages/scanner.js` | `useAuth` | Path swap → `lib/use-auth` | +| `pages/decks.js` | `useAuth` | Path swap → `lib/use-auth` | +| `pages/deck/[id].js` | `useAuth` | Path swap → `lib/use-auth` (depth `../../`) | +| `pages/deck-builder.js` | `useAuth` | Path swap → `lib/use-auth` | + +All 5 page-level `useAuth` consumers destructured only `{ user }` or `{ user, loading }` (verified +by grep). No `login` / `register` / other-method consumer found, confirming D1. + +### Importers of `lib/admin-auth.js` (1 source file) + +| File | Symbol | Migration | +| --- | --- | --- | +| `pages/card/[id].js` | `useIsAdmin` | Replaced with `user?.role === 'admin'` from existing `useAuth()` (D2) | + +`AdminProvider` and `useAdmin()` had **zero** importers in the source tree — confirming +they were dead exports. + +### Adjacent doc / config edits + +| File | Change | +| --- | --- | +| `pages/_app.js` | Removed `import { AuthProvider } from '../lib/auth-context.js'` and the wrapper | +| `.github/CODEOWNERS` | Removed the two CODEOWNERS lines for the deleted files | +| `AGENTS.md` § 2 + § 3 | Updated the Auth row of the architecture table and the "Auth (client)" convention bullet to describe the post-convoy single-surface state | +| `.cursor/rules/auth-and-permissions.mdc` | Reframed § "Legacy" to "deleted by this convoy"; updated § "Authentication state on the client" to the post-convoy `useAuth()` shape and the direct-fetch login flow | +| `.cursor/rules/no-go-zones.mdc` | Auth-refactors bullet updated to drop the deleted files | +| `.cursor/skills/add-page/SKILL.md` | Updated checklist bullet + anti-pattern row to refer to the deletion | + +`.convoys/**` and `.convoys/fix-layout-default-user/**` were **not** edited — those are +historical convoy records and are append-only by repo convention. The doc-writer post-convoy +sweep will add the as-shipped section at the bottom of this file plus update +`.convoys/ship-readiness.md` § P1 → entry 9 with the squash commit reference. + +## The fix (per-category translation rules) + +### Category A — `useAuth` from `auth-context` → `useAuth` from `use-auth` + +```js +// before +import { useAuth } from '../lib/auth-context'; // or auth-context.js +// after +import { useAuth } from '../lib/use-auth'; // or use-auth.js +``` + +The destructure pattern (`const { user } = useAuth()` / `const { user, loading } = useAuth()`) +stays byte-identical. No call-site changes. + +### Category B — `AuthProvider` wrapper in `_app.js` + +```jsx +// before +import { AuthProvider } from '../lib/auth-context.js'; +return ( + + + + + +); + +// after +return ( + + + +); +``` + +Plus delete the import line. + +### Category C — `useIsAdmin` in `pages/card/[id].js` + +See D2 for the full diff. Three line-ranges touched: the import block, the `useAuth` destructure, +and the `useIsAdmin` line block. Usage at line 524 is unchanged. + +### Category D — `useAdmin`, `AdminProvider` + +No call sites. No work to do; these symbols disappear when the file is deleted. + +## Verification plan + +1. **`rg "lib/auth-context|lib/admin-auth" --type js`** → expect zero hits in `pages/`, `lib/`, + `components/`. Achieved. +2. **`npm run lint`** → baseline 128 problems pre-convoy → 125 problems post-convoy (3 fewer + errors, since the deleted files contained 3 unused-import / unused-var lints; no new lint + surface introduced). No regression. +3. **`npm run test:run`** → 21/21 pass pre- and post-convoy. Layout test confirmed unaffected. +4. **`npm run build`** → succeeds end-to-end. All 26 pages compile (10 dynamic API routes + 16 + `pages/**` views including `card/[id]`, `_app`, `decks`, `deck/[id]`, `deck-builder`, `scanner`, + `index` — every file modified by the sweep). No SSR-level breakage; importantly no + "useAuth must be used within an AuthProvider" runtime error during static generation, which + would have indicated the page tried to use the legacy context hook unwrapped. +5. **Manual smoke:** _deferred_ — the build pass + vitest pass + zero-hit grep is the gate for + merging; the parent does not have a logged-in admin browser session ready in this + conversation. Documenting in As-shipped post-merge once the operator runs `npm run dev` and + exercises dashboard / profile / settings / collections / cards / admin/card-editor. + +## Risks + +- **R1 — Shape parity gap breaks runtime auth state.** *Mitigated by D1.* The grep audit + confirmed no consumer reads `login` / `register` / any other surface that exists on the + legacy hook but not on `use-auth`. `loading` and `user` were preserved with identical + semantics. +- **R2 — SSR mismatch from removing ``.** *Mitigated.* `lib/use-auth.js` reads + `localStorage` inside a `useEffect`, so SSR sees `user === null, loading === true` and never + touches the browser-only API on the server — same guarded shape as the legacy provider. + `npm run build` confirms no SSR error during static generation. (`auth-context.js`'s + `useEffect` had the same guard, so removing the provider didn't change the SSR surface.) +- **R3 — Missed importer.** *Mitigated.* Post-delete grep over `--type js` returned zero hits. + The deletion would itself surface any missed importer at module-load time during `npm run + build` (Node would throw "Cannot find module"); build succeeded. +- **R4 — Verify-roundtrip dedup creates a regression where a page never re-verifies.** + *Mitigated.* Pre-convoy, three providers each ran their own verify on mount but they did not + coordinate state — one provider's success had no effect on another's loading flag. Post-convoy + we have a single source of truth. Pages that need to re-verify (e.g. after an action that + might have invalidated the token) can call `refreshAuth()` from the same hook; no consumer + currently does this, but the surface is preserved for future use. +- **R5 — Stale `useAuth` cache across components.** *Out of scope; see Follow-ups.* Each + `useAuth()` call site instantiates its own state via `useState`. Two components on the same + page that both call `useAuth` will issue two verify roundtrips and hold two independent + `user` references. This was true pre-convoy too (the legacy `useIsAdmin` was already a + separate verify). Hoisting state into a shared module-level cache or wrapping `useAuth` in a + context (the very thing we just removed!) is a separate decision — see "Follow-ups". + +## As-shipped + +_Stub for doc-writer post-merge:_ + +- Squash commit: `` +- PR: #`` +- Files changed: 13 (2 deletions: `lib/auth-context.js`, `lib/admin-auth.js`; 11 modifications: + `pages/_app.js`, `pages/index.js`, `pages/scanner.js`, `pages/decks.js`, `pages/deck/[id].js`, + `pages/deck-builder.js`, `pages/card/[id].js`, `.github/CODEOWNERS`, `AGENTS.md`, + `.cursor/rules/auth-and-permissions.mdc`, `.cursor/rules/no-go-zones.mdc`, + `.cursor/skills/add-page/SKILL.md`). +- Verify roundtrip count: documented 3 → 1 on `card/[id].js`, 2 → 1 on every other page-load. +- `.convoys/ship-readiness.md` § P1 entry 9 to be marked RESOLVED with this convoy's squash SHA. +- Lint baseline updated 128 → 125 (no regression; 3 fewer errors from deleted unused-import + lines). + +## Follow-ups (out of scope here) + +- **Component-level `useAuth` cache audit.** Two components on the same page that both call + `useAuth()` will issue two verify roundtrips. This was the original motivation for the + legacy context, and was the *one* legitimate thing those providers did right. A future + convoy should consider either (a) returning a shared module-level state via a small + Zustand-style store, (b) reintroducing a thin `` that *only* hoists state + without re-implementing fetch logic, or (c) accepting the duplicate roundtrip as the price of + hook-only simplicity. Today's call sites already deduplicate at the page level (one + `useAuth` per page is the prevailing pattern), so this is a soft optimisation, not a + correctness fix. +- **Rate-limit-aware re-auth on 429.** `lib/use-auth.js`'s `checkAuth` does not currently + back off if `/api/auth/verify` returns 429 (the rate-limiter from + `add-rate-limiting` would only kick in if a single client exceeded + 60 verify calls / minute, which is unrealistic in practice but worth a defensive guard). +- **Server-side hydration of user.** The page-mount verify roundtrip is unavoidable in this + hook-only shape because the token is only readable on the client. Moving to an HTTP-only + cookie + Next.js `getServerSideProps` hydration would eliminate the round-trip entirely + and is a larger architectural conversation that should not piggyback on a quality convoy. +- **Doc-writer cleanup.** Update `.convoys/ship-readiness.md` § P1 → entry 9 with the + RESOLVED stamp + squash SHA; trim the "three parallel surfaces" framing from any other + doc that still mentions it; refresh the "Auth refactors" no-go-zones bullet if any other + files become canonical (none today). diff --git a/.cursor/rules/auth-and-permissions.mdc b/.cursor/rules/auth-and-permissions.mdc index 7308787..98f7bbf 100644 --- a/.cursor/rules/auth-and-permissions.mdc +++ b/.cursor/rules/auth-and-permissions.mdc @@ -21,12 +21,15 @@ There are three parallel client-side auth implementations and one server-side he | Client: route protection | `components/ProtectedRoute.js` | | Client: admin route protection | `components/AdminProtected.js` | -## Legacy (do not extend) +## Legacy (deleted by `single-auth-provider`) -- `lib/auth-context.js::AuthProvider` + `useAuth` — older context. Still wired in `pages/_app.js`; left in place for compatibility. Don't add new consumers. -- `lib/admin-auth.js::AdminProvider` + `useAdmin` + `useIsAdmin` — parallel admin context. Same story. - -A convoy is planned to collapse these three into one provider + one hook. +`lib/auth-context.js` and `lib/admin-auth.js` were the two parallel client-side +auth surfaces that lived alongside `lib/use-auth.js`. They were deleted by the +`single-auth-provider` convoy (P1). Do **not** reintroduce a `` +or `` wrapper in `pages/_app.js` — `useAuth()` from +`lib/use-auth.js` is hook-only (reads token from `localStorage` and hits +`/api/auth/verify` on mount) and does not require a context provider. The +`useIsAdmin` semantic is now `const { user } = useAuth(); const isAdmin = user?.role === 'admin'`. ## Token model @@ -48,7 +51,7 @@ When introducing a new permission tier, update both `checkRolePermission`'s hier ## Authentication state on the client -`useAuth()` returns `{ user, loading, login, logout, refresh }`. `user === null` means logged out; `loading === true` means token verification in flight. Always render against `loading === false` before deciding to redirect. +`useAuth()` returns `{ user, loading, logout, refreshAuth }`. `user === null` means logged out; `loading === true` means token verification in flight. Always render against `loading === false` before deciding to redirect. The login / register flows do **not** go through `useAuth` — `pages/login.js` and `pages/signup.js` `fetch` `/api/auth/{login,register}` directly and write the returned token to `localStorage`; `useAuth()` will pick it up on next mount via its `useEffect` → `/api/auth/verify` roundtrip (or call `refreshAuth()` to re-verify in place). ## Server-side authorization patterns diff --git a/.cursor/rules/no-go-zones.mdc b/.cursor/rules/no-go-zones.mdc index 3f47c84..028ae90 100644 --- a/.cursor/rules/no-go-zones.mdc +++ b/.cursor/rules/no-go-zones.mdc @@ -33,5 +33,5 @@ Do not edit, refactor, or quote as context examples. If you think you need to ch ## Editing rules of thumb - **Schema changes:** until a proper migration tool lands, document the change in a new dated script under `scripts/migrations/YYYY-MM-DD-.js` (folder TBD). Do NOT edit `scripts/setup-neon-db.js` in place for any **DDL change** (`CREATE TABLE`, `ALTER`, new columns, constraint changes) — it's idempotent and meant for first-time setup only. **Operational changes are allowed** (env-var gating, error-message hardening, module-system fixes) — `drop-public-setup` set this precedent by adding the `ADMIN_INITIAL_PASSWORD` gate and converting the script to ESM. The distinction: if the change touches DDL strings or `INSERT` semantics, file a migration; if it only touches Node-module behavior or pre-flight validation, edit in place and document why in the convoy. -- **Auth refactors:** `lib/permission-middleware.js`, `pages/api/auth-utils.js`, `lib/auth-context.js`, `lib/admin-auth.js`, and `lib/use-auth.js` form a deliberately documented mess. Tighten them inside a single convoy; don't cherry-pick. +- **Auth refactors:** `lib/permission-middleware.js`, `pages/api/auth-utils.js`, `lib/auth-secret.js`, and `lib/use-auth.js` are the four documented auth surfaces. Tighten them inside a single convoy; don't cherry-pick. (The legacy `lib/auth-context.js` and `lib/admin-auth.js` were deleted by `single-auth-provider`; do not resurrect them.) - **Card-import jobs:** `pages/api/cards/import-*.js` hit external APIs with rate limits. Don't run them ad-hoc against prod data; use staging. diff --git a/.cursor/skills/add-page/SKILL.md b/.cursor/skills/add-page/SKILL.md index 09f7ff9..0180773 100644 --- a/.cursor/skills/add-page/SKILL.md +++ b/.cursor/skills/add-page/SKILL.md @@ -93,7 +93,7 @@ Use Tailwind for layout, spacing, sizing, hover/focus states. Use CSS vars (inli ## Step 6: Check - [ ] Auth wrapper chosen (ProtectedRoute / AdminProtected / public). -- [ ] `useAuth()` from `lib/use-auth.js` (not the legacy `lib/auth-context.js`). +- [ ] `useAuth()` from `lib/use-auth.js` (the only client auth hook; `lib/auth-context.js` and `lib/admin-auth.js` were deleted by the `single-auth-provider` convoy). - [ ] `user` passed to Layout explicitly. - [ ] Colors come from theme tokens, not hex. - [ ] All interactive elements have `aria-label` or visible text. @@ -105,5 +105,5 @@ Use Tailwind for layout, spacing, sizing, hover/focus states. Use CSS vars (inli | --- | --- | | Hardcode hex colors | Use CSS variables | | Default `user = { … }` to a real email | Default to `null` | -| Pull from `lib/auth-context` for new code | Use `lib/use-auth` | +| Reintroduce `lib/auth-context` or `lib/admin-auth` (deleted) | Use `lib/use-auth` | | Render Layout twice on the same page | Single `` at the top | diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 66fc49b..354055a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -8,8 +8,6 @@ pages/api/auth/** @YOUR-GITHUB-HANDLE pages/api/auth-utils.js @YOUR-GITHUB-HANDLE lib/permission-middleware.js @YOUR-GITHUB-HANDLE -lib/auth-context.js @YOUR-GITHUB-HANDLE -lib/admin-auth.js @YOUR-GITHUB-HANDLE lib/use-auth.js @YOUR-GITHUB-HANDLE components/ProtectedRoute.js @YOUR-GITHUB-HANDLE components/AdminProtected.js @YOUR-GITHUB-HANDLE diff --git a/AGENTS.md b/AGENTS.md index a5132ac..0777ef3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ A web app for managing trading-card-game collections (Magic, Pokémon, Lorcana). | Pages router views | `pages/*.js` | Public + auth views; uses `components/Layout.js` | | API routes | `pages/api/**/*.js` | Express-style `handler(req, res)`. **30+ handlers depend on `lib/permission-middleware.js::getUserFromRequest`** | | Shared UI | `components/*.js` | `Layout`, `CardItem`, `CameraScanner`, modal family | -| Auth + DB libs | `lib/*.js` | `auth-context`, `admin-auth`, `use-auth` (three parallel auth surfaces), `database`, `permission-middleware` | +| Auth + DB libs | `lib/*.js` | `use-auth` (canonical client hook — sole surface post-`single-auth-provider`), `database`, `permission-middleware` | | Migration scripts | `scripts/*.js` | 27+ one-off "add column" / "seed" scripts. No formal migration tool | | Card-import jobs | `pages/api/cards/import-*.js`, `scripts/import-*.js` | Scryfall / Lorcana / Pokémon TCG APIs | | Database schema | `scripts/setup-neon-db.js` | Bootstrap SQL DDL — the source of truth until a real migration tool lands | @@ -40,7 +40,7 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560 ## 3. Key conventions - **Auth (server):** `import { getUserFromRequest } from '../../lib/permission-middleware'` → returns `{ userId, email, role }` or `null`. `null` means "send 401" — always early-return when the user is null before doing any work that depends on their identity. -- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Avoid `lib/auth-context.js` and `lib/admin-auth.js` for new code — they are legacy parallel implementations. +- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Returns `{ user, loading, logout, refreshAuth }`; `user === null` means logged out, `loading === true` means token verification in flight. There is no client-side admin hook — compute `const isAdmin = user?.role === 'admin'` from the same `useAuth()` call. The legacy `lib/auth-context.js` and `lib/admin-auth.js` were deleted by the `single-auth-provider` convoy; do not reintroduce a `` / `` wrapper in `pages/_app.js`. - **Layout `user` prop:** pages should pass `user` from `useAuth()` to ``. Layout's default is `null` and renders a logged-out "Sign in" CTA when no user is supplied — both paths are valid (some surfaces like `pages/invite/{accept,decline}.js` legitimately render Layout for anonymous visitors). Do not reintroduce a hardcoded user object as a default prop. - **JWT secret + TTL:** `import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'`. This is the only place either value is defined; do not reintroduce literal fallbacks. `JWT_TOKEN_TTL = '24h'` is canonical. - **Auth helper (token mint / verify / password hash):** `import { ... } from '../../pages/api/auth-utils'` (`generateToken`, `verifyToken`, `hashPassword`, `verifyPassword`). Reads the secret + TTL from `lib/auth-secret.js` under the hood. diff --git a/lib/admin-auth.js b/lib/admin-auth.js deleted file mode 100644 index 4a90b65..0000000 --- a/lib/admin-auth.js +++ /dev/null @@ -1,120 +0,0 @@ -import { createContext, useContext, useState, useEffect } from 'react'; - -// Create admin context -const AdminContext = createContext(); - -export function AdminProvider({ children }) { - const [user, setUser] = useState(null); - const [loading, setLoading] = useState(true); - - useEffect(() => { - checkAdminAuth(); - }, []); - - const checkAdminAuth = async () => { - try { - // Get token from localStorage - const token = localStorage.getItem('auth_token'); - - const headers = { - 'Content-Type': 'application/json', - }; - - // Add authorization header if token exists - if (token) { - headers.Authorization = `Bearer ${token}`; - } - - const response = await fetch('/api/auth/verify', { headers }); - if (response.ok) { - const userData = await response.json(); - setUser(userData); - } else { - setUser(null); - // Clear invalid token - if (token) { - localStorage.removeItem('auth_token'); - } - } - } catch (error) { - console.error('Auth check failed:', error); - setUser(null); - } finally { - setLoading(false); - } - }; - - const isAdmin = () => { - return user && user.role === 'admin'; - }; - - const isAuthenticated = () => { - return user !== null; - }; - - const value = { - user, - loading, - isAdmin, - isAuthenticated, - checkAdminAuth - }; - - return ( - - {children} - - ); -} - -export function useAdmin() { - const context = useContext(AdminContext); - if (!context) { - throw new Error('useAdmin must be used within an AdminProvider'); - } - return context; -} - -// Simple hook for checking admin status without context -export function useIsAdmin() { - const [isAdmin, setIsAdmin] = useState(false); - const [loading, setLoading] = useState(true); - - useEffect(() => { - const checkAdmin = async () => { - try { - // Get token from localStorage - const token = localStorage.getItem('auth_token'); - - const headers = { - 'Content-Type': 'application/json', - }; - - // Add authorization header if token exists - if (token) { - headers.Authorization = `Bearer ${token}`; - } - - const response = await fetch('/api/auth/verify', { headers }); - if (response.ok) { - const userData = await response.json(); - setIsAdmin(userData.role === 'admin'); - } else { - setIsAdmin(false); - // Clear invalid token - if (token) { - localStorage.removeItem('auth_token'); - } - } - } catch (error) { - setIsAdmin(false); - } finally { - setLoading(false); - } - }; - - checkAdmin(); - }, []); - - return { isAdmin, loading }; -} \ No newline at end of file diff --git a/lib/auth-context.js b/lib/auth-context.js deleted file mode 100644 index 3b7e8b5..0000000 --- a/lib/auth-context.js +++ /dev/null @@ -1,116 +0,0 @@ -import { createContext, useContext, useState, useEffect } from 'react'; - -const AuthContext = createContext(); - -export function AuthProvider({ children }) { - const [user, setUser] = useState(null); - const [loading, setLoading] = useState(true); - - useEffect(() => { - // Check for existing token on app load - const token = localStorage.getItem('auth_token'); - if (token) { - // Verify token and set user - verifyToken(token); - } else { - setLoading(false); - } - }, []); - - const verifyToken = async (token) => { - try { - const response = await fetch('/api/auth/verify', { - headers: { - 'Authorization': `Bearer ${token}` - } - }); - - if (response.ok) { - const userData = await response.json(); - setUser(userData); // API returns user data directly, not wrapped in .user - } else { - localStorage.removeItem('auth_token'); - } - } catch (error) { - console.error('Token verification failed:', error); - localStorage.removeItem('auth_token'); - } finally { - setLoading(false); - } - }; - - const login = async (email, password) => { - try { - const response = await fetch('/api/auth/login', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ email, password }), - }); - - const data = await response.json(); - - if (response.ok) { - localStorage.setItem('auth_token', data.token); - setUser(data.user); - return { success: true }; - } else { - return { success: false, error: data.error }; - } - } catch (error) { - return { success: false, error: 'Network error' }; - } - }; - - const register = async (email, password) => { - try { - const response = await fetch('/api/auth/register', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ email, password }), - }); - - const data = await response.json(); - - if (response.ok) { - localStorage.setItem('auth_token', data.token); - setUser(data.user); - return { success: true }; - } else { - return { success: false, error: data.error }; - } - } catch (error) { - return { success: false, error: 'Network error' }; - } - }; - - const logout = () => { - localStorage.removeItem('auth_token'); - setUser(null); - }; - - const value = { - user, - loading, - login, - register, - logout, - }; - - return ( - - {children} - - ); -} - -export function useAuth() { - const context = useContext(AuthContext); - if (!context) { - throw new Error('useAuth must be used within an AuthProvider'); - } - return context; -} \ No newline at end of file diff --git a/pages/_app.js b/pages/_app.js index 671cf63..d85b52f 100644 --- a/pages/_app.js +++ b/pages/_app.js @@ -1,13 +1,10 @@ import '../styles/globals.css'; -import { AuthProvider } from '../lib/auth-context.js'; import { ThemeProvider } from '../lib/theme-context.js'; export default function App({ Component, pageProps }) { return ( - - - + ); -} \ No newline at end of file +} diff --git a/pages/card/[id].js b/pages/card/[id].js index e7d92e3..8de8bc8 100644 --- a/pages/card/[id].js +++ b/pages/card/[id].js @@ -1,7 +1,6 @@ import { useState, useEffect } from 'react'; import { useRouter } from 'next/router'; import Layout from '../../components/Layout'; -import { useIsAdmin } from '../../lib/admin-auth'; import { useAuth } from '../../lib/use-auth'; import CollectionSelectionModal from '../../components/CollectionSelectionModal'; import { ManaCost, ColorIdentity, AdvancedManaCost } from '../../components/ManaSymbols'; @@ -10,7 +9,7 @@ import ManaSymbolSettings from '../../components/ManaSymbolSettings'; export default function CardDetail() { const router = useRouter(); const { id } = router.query; - const { user } = useAuth(); + const { user, loading: authLoading } = useAuth(); const [card, setCard] = useState(null); const [loading, setLoading] = useState(true); @@ -31,8 +30,8 @@ export default function CardDetail() { // Mana symbol settings const [manaSymbolSettings, setManaSymbolSettings] = useState({ useSVG: false }); - // Check admin status - const { isAdmin, loading: adminLoading } = useIsAdmin(); + const isAdmin = user?.role === 'admin'; + const adminLoading = authLoading; // Fetch card data from API useEffect(() => { diff --git a/pages/deck-builder.js b/pages/deck-builder.js index 02328ab..d4e2ec7 100644 --- a/pages/deck-builder.js +++ b/pages/deck-builder.js @@ -4,7 +4,7 @@ import Link from 'next/link'; import Layout from '../components/Layout'; import { ManaCost, ColorIdentity, ColorFilterSymbol } from '../components/ManaSymbols'; import ManaSymbolSettings from '../components/ManaSymbolSettings'; -import { useAuth } from '../lib/auth-context'; +import { useAuth } from '../lib/use-auth'; import { getColorIdentity, getColorSymbol } from '../lib/mana-symbols'; export default function DeckBuilder() { diff --git a/pages/deck/[id].js b/pages/deck/[id].js index 2c975b3..ef7d0bb 100644 --- a/pages/deck/[id].js +++ b/pages/deck/[id].js @@ -3,7 +3,7 @@ import { useRouter } from 'next/router'; import Link from 'next/link'; import Layout from '../../components/Layout'; import { ManaCost, ColorIdentity } from '../../components/ManaSymbols'; -import { useAuth } from '../../lib/auth-context'; +import { useAuth } from '../../lib/use-auth'; import { getColorIdentity } from '../../lib/mana-symbols'; export default function DeckDetail() { diff --git a/pages/decks.js b/pages/decks.js index 33aedf4..e52b615 100644 --- a/pages/decks.js +++ b/pages/decks.js @@ -2,7 +2,7 @@ import { useState, useEffect } from 'react'; import { useRouter } from 'next/router'; import Link from 'next/link'; import Layout from '../components/Layout'; -import { useAuth } from '../lib/auth-context'; +import { useAuth } from '../lib/use-auth'; export default function Decks() { const { user } = useAuth(); diff --git a/pages/index.js b/pages/index.js index 5d93fd3..03a6724 100644 --- a/pages/index.js +++ b/pages/index.js @@ -1,7 +1,7 @@ import { useState, useEffect } from 'react'; import { useRouter } from 'next/router'; import Link from 'next/link'; -import { useAuth } from '../lib/auth-context.js'; +import { useAuth } from '../lib/use-auth.js'; import AnimatedFireLogo from '../components/AnimatedFireLogo'; export default function Home() { diff --git a/pages/scanner.js b/pages/scanner.js index 4fa22b2..345a816 100644 --- a/pages/scanner.js +++ b/pages/scanner.js @@ -5,7 +5,7 @@ import CameraScanner from '../components/CameraScanner'; import OCRSettings from '../components/OCRSettings'; import { ManaCost, ColorIdentity } from '../components/ManaSymbols'; import ManaSymbolSettings from '../components/ManaSymbolSettings'; -import { useAuth } from '../lib/auth-context'; +import { useAuth } from '../lib/use-auth'; export default function Scanner() { const { user } = useAuth();