Review — Auth expansion Phase 1 (feature/auth-expansion-phase1)

Date: 2026-07-15 · Scope: master (9dfa444)..HEAD — 25 commits Spec: docs/superpowers/specs/2026-07-14-auth-expansion-design.md · Plan: docs/superpowers/plans/2026-07-14-auth-expansion-phase1.md · Tracker: docs/tracking/auth-expansion-phase1.mdProcess: subagent-driven execution — fresh implementer per task, per-task spec+quality review, adversarial final whole-branch review, fix rounds re-reviewed.

Deterministic gate

Check	Result
nx lint/test/build web	✅ green
nx test web --configuration=coverage	✅ 83.95% lines (baseline 83.83%)
nx lint/test/build api (live-DB router tests incl. new fail-closed cases)	✅ green
nx e2e web-e2e (live stack: signup→mail→verify→pending, member sign-in, full reset)	✅ 5/5
nx lint web-e2e	✅ green
Live RLS probe (psql, JWT claims)	✅ Pending = 0 rows on member tables; member = full read

Findings and resolutions

🔴 Blocking — all fixed before this record

1. Pending users could reach member data via the Go API's non-CRUD routes (router.go — pulse/okrs/skills/votes/notifications mounted with only RequireAuth; services query as table owner, bypassing RLS). A self-served Pending JWT could read GET /pulse/:id/results (aggregate member sentiment), write pulse responses/OKRs/skills. → Fixed (db107fe): member-role RequireRole group on those five mounts; /me and admins unchanged by design. Fail-closed tests: Pending→403, no-claim→401, member→200 on /pulse/active and /okrs.
2. (found mid-branch, Task 14) The new is_member() restrictive RLS silently emptied all CRUD reads through the API — the engine's RLS context carried only sub, not membership_role. → Fixed (8f512f1): verified JWT claim threaded into the same request.jwt.claims GUC, default Pending (fail closed), regression tests for explicit-Pending and empty-claim callers (proven load-bearing by reverting the fix).

🟡 Should-fix — all fixed before this record

3. memberGuard failed open on a null role (profile fetch failure ⇒ full member access). → fail-closed: null role parks on /pending (35f3225).
4. Spec §4.3 drift: signed-in users weren't redirected off /login, /signup, /reset-password. → authPagesGuard added (not on /reset-password/update — recovery links must reach the form). Redirect-graph traced across all role×MFA states: no cycles, ≤2 hops.
5. MFA was advisory (URL-bar bypass during the challenge). → client-side AAL enforcement in authGuard/memberGuard via AuthService.mfaSatisfied(); /login?mfa=1 shows the challenge panel directly. Server-side AAL is NOT enforced (API/RLS ignore aal) — recorded as an explicit Phase 2 gap in the tracker.
6. docs/auth-setup.md still documented the Workspace domain gate. → rewritten (External consent screen, no hd, Pending profiles, GitHub OAuth app setup, email confirmations).
7. Task-7/8/12 per-task review rounds: login a11y labels + submit re-entrancy; signup resend silent-failure; MFA QR returned unusable raw SVG (now a proper data:image/svg+xml URI) and silent factor-removal failures. All fixed and re-reviewed in their rounds.

🟢 Follow-ups (accepted, not blocking)

• /settings/security has no nav entry (scaffold; reachable by URL).
• Signup with an already-confirmed email shows "check your email" and nothing arrives (GoTrue anti-enumeration; consider hedged copy).
• handle_new_user accepts an empty-string metadata name (cosmetic; nullif(trim(…),'') later).
• pendingGuard doesn't itself check AAL (no member data on /pending; API fail-closes independently).
• MFA panel has no back-to-password escape; a few AuthService helpers lack direct unit tests (e2e-covered).

Verdict

Green — ready to push/PR after user sign-off. Every blocking and should-fix finding is fixed, re-reviewed, and covered by tests; the deterministic gate is fully green; remaining items are recorded follow-ups. Reminder of the deliberate scope seams: org create/join/invites, per-org LoginPolicy/SAML, and server-side AAL enforcement are Phase 2 (see spec §5 and the tracker's Known gaps). Ops prerequisites before the first deploy of this branch: Google consent screen → External, GitHub OAuth apps (+ hosted provider config), SMTP on hosted Supabase (tracker ops checklist).