From b02bb5519ce75f449809e57e9c2c00b37d722861 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 9 May 2026 14:09:24 -0500 Subject: [PATCH 01/25] docs(plans): add email confirmation signup plan for #164 Plans a 3-commit sequence to require email confirmation on signup: foundation (PKCE flow, callback route, OTP verification), resend support, and friendly handling of unconfirmed sign-in attempts. --- docs/plans/164-email-confirmation-signup.md | 213 ++++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 docs/plans/164-email-confirmation-signup.md diff --git a/docs/plans/164-email-confirmation-signup.md b/docs/plans/164-email-confirmation-signup.md new file mode 100644 index 0000000..a3ffab2 --- /dev/null +++ b/docs/plans/164-email-confirmation-signup.md @@ -0,0 +1,213 @@ +# Plan — Issue #164: Require email confirmation on signup + +## Context + +Email confirmations are currently disabled in Supabase (`enable_confirmations = false` in `api/supabase/config.toml:176`). Anyone can sign up with any email and immediately be logged in — there's no proof of address ownership. The issue asks us to: + +1. Enable email confirmations +2. Show a "pending confirmation" state after signup (since `signUp()` will no longer return a session) +3. Allow the confirmation email to be resent +4. Complete verification when the user clicks the email link +5. Handle the case where an unconfirmed user later tries to sign in + +**Cross-issue ordering** — this issue should ship before #167 (change-email re-verification, which reuses the same `/auth/callback` route and OTP entry UI) and before #165 (sign-in/redirect audit, which standardizes destinations across all auth flows). 164 preserves today's destination logic; 165 will standardize it later. + +**Out of scope:** +- Branded email template content (deferred to #22) +- Standardizing post-auth redirect destinations (deferred to #165) +- Production Supabase dashboard configuration (must be done manually after deploy: enable Confirm Email in Authentication → Providers → Email, add `https:///auth/callback` to redirect allowlist, upload custom template) + +## Decisions + +| Decision | Choice | Rationale | +|---|---|---| +| Confirmation method | **Both magic link AND OTP code** in the email; user can use whichever | Resilient to corporate email scanners (Defender, Barracuda, Mimecast) that pre-fetch links and consume the token before users click. Supabase auth issue [#1214](https://github.com/supabase/auth/issues/1214) is open and unfixed; no clean recovery for affected users without an OTP fallback. Default Supabase template already emits both, so the email-side cost is near zero. | +| OTP input component | shadcn/ui `` (uses `input-otp` library) | Renders single invisible `` behind visual digit boxes — combines accessible single input (WCAG 1.3.5) with visual digit-box UX. Consistent with project's existing shadcn/ui usage. | +| Pending-UI surface | Inline state + shared `` component | No URL params (OWASP: PII like email shouldn't appear in URLs — leaks via browser history, server logs, Referer to Sentry). Refresh degrades benignly: link in inbox still works, resend reachable via signin path. | +| Email passing | React state within form components | User never retypes — Supabase's confirmation token uniquely identifies the user. Email passed as prop from parent (`SignUpForm`, `SignInForm`) into ``. | +| Sign-in unconfirmed handling | In scope, same issue (commit 3) | Completes the verification story: signup → pending → resend → confirm AND signin → unconfirmed → resend. | +| Post-confirmation destination | Preserve current logic per parent (`SignUpForm` → redirect-or-`/create-team`; `SignInForm` → redirect-or-`/leagues`; magic link callback uses URL `redirect` search param, else `/create-team`) | Standardization deferred to #165. | +| Resend with redirect override | `AuthContext.resendConfirmation(email, { redirect? })` accepts the current redirect param so the resent email's link encodes the *current* destination (e.g., signin's `/leagues/123`) rather than whatever was baked in at signup time | Helps a user who hits the unconfirmed-signin path with a deep link, triggers Resend, and uses the new email's link. The original signup email's link still routes to `/create-team` (can't fix retroactively); the OTP path always routes correctly via the callsite. | +| E2E approach | Existing admin-API fixture keeps `email_confirm: true` (auto-confirms); add ONE new test per new flow path (magic link in commit 1, OTP in commit 2) | Existing tests stay fast; targeted e2e covers the new wiring | +| Email template | Custom template at `api/supabase/templates/confirmation.html` includes both `{{ .ConfirmationURL }}` and `{{ .Token }}` from commit 1 | Default Supabase template already has both; we're shipping our own to control wording and prep for #22's branding | +| Cross-issue ordering | 164 → 167 → 165 (locked) | 164 builds verification primitives; 167 reuses callback route + OTP entry pattern for change-email re-verification; 165 audits all destinations after both new flows exist. | + +--- + +## Commits + +Each commit is a gate. Each must independently build, lint, test, and format. Wait for approval before moving on. Note: these commits are sized for review/approval — they don't necessarily map 1:1 to production deploys. Recommendation is to land all three before flipping production confirmations on, so users always have the OTP fallback path. + +### Commit 1 — Enable confirmations + PKCE flow + signup pending state + magic link callback + OTP verification + +**Goal:** Signup goes end-to-end via either path: submit → "check your email, click link or enter code" UI → user clicks link OR types code → land back in app, logged in. (Resend deferred to commit 2; signin-unconfirmed deferred to commit 3.) + +**Supabase client flow change** (`web/src/lib/supabase.ts`): +- Today: `createClient(url, key)` with no options → defaults to `flowType: 'implicit'` (verified in `auth-js` source — `DEFAULT_OPTIONS.flowType: 'implicit'`) +- Required: pass `{ auth: { flowType: 'pkce' } }` so the magic link returns `?code=` (which `exchangeCodeForSession` consumes) instead of `#access_token=` (implicit hash flow) +- This is a small but app-wide auth behavior change — covered by existing AuthContext tests + +**Supabase config** (`api/supabase/config.toml` and `e2e/supabase/config.toml`): +- `[auth] site_url` → set to local web URL (`http://localhost:5173` for dev, `http://localhost:5273` for e2e). Existing `http://127.0.0.1:3000` is dead config; nothing in the codebase references it. +- `[auth] additional_redirect_urls` → `["http://localhost:5173/auth/callback"]` for dev (and `5273` for e2e). Existing `https://127.0.0.1:3000` is HTTPS on a non-running port — wrong on multiple counts. +- `[auth.email] enable_confirmations = true` (currently `false` at line 176) +- `[auth.email.template.confirmation]` block pointing at `./templates/confirmation.html`. Both configs can reference the same file via relative path; mirrors how `e2e/supabase/migrations/` is symlinked to `api/supabase/migrations/`. + +**New email template** (`api/supabase/templates/confirmation.html`): +- Includes both the magic link (`{{ .ConfirmationURL }}`) AND the 6-digit OTP code (`{{ .Token }}`) +- Wording explicitly mentions "Click the link OR enter the 6-digit code in the app" +- Plain HTML; references app name placeholder; #22 will brand it + +**New components:** +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` — shared pending UI. Props: + - `email: string` — displayed in message + - `onVerified: () => void` — called after `verifyOtp` resolves successfully so parent can navigate + - `onResend?: () => Promise` — wired in commit 2 (placeholder/no-op for commit 1) + - Renders: instructional copy ("We sent a link to . Click the link in the email, or enter the 6-digit code below."), a shadcn/ui `` component (renders 6 visual digit slots with a single accessible input + `autocomplete="one-time-code"` underneath), a `` that calls `supabase.auth.verifyOtp({ email, token: code, type: 'signup' })` on submit. On error, render ``. On success, call `onVerified`. (`type: 'signup'` is the verified value from `auth-js` `EmailOtpType` for initial-signup confirmation.) + - Adds dependency: `input-otp` package via shadcn CLI (`npx shadcn@latest add input-otp` — installs to `web/src/components/ui/input-otp.tsx`, matching the project's existing shadcn vendoring per `web/components.json` aliases). Per `web/CLAUDE.md`, shadcn primitives are vendored — do not modify after install. +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.test.tsx` +- `web/src/components/auth/AuthCallback/AuthCallback.tsx` — handles the magic-link return: on mount, reads `code` and optional `redirect` from URL search params, calls `supabase.auth.exchangeCodeForSession(code)`, on success navigates to `redirect` if present else `/create-team`, on error renders `` with link back to `/sign-in`. Reuses the existing `redirectSearchSchema` Zod validator at `web/src/router.tsx:87-93` (validates `redirect` starts with `/` to prevent open redirects). +- `web/src/components/auth/AuthCallback/AuthCallback.test.tsx` + +**New helper** (`web/src/lib/auth-destination.ts`): +- `getPostSignupDestination(redirectParam?: string): string` — returns `redirectParam` if present, else `/create-team`. Search-param validation is already enforced upstream by `redirectSearchSchema` (router.tsx:87-93) when the route validates its search params, so this helper just chooses between two valid values. Used by `SignUpForm` (post-success when session is returned), `AuthCallback`, and ``'s `onVerified` from the signup callsite. (`SignInForm`'s default destination stays as today's `/leagues`; this helper is the signup-flow default.) + +**Wiring:** +- `web/src/contexts/AuthContext.tsx`: + - Update `signUp()` to pass `options: { data: { displayName }, emailRedirectTo: ${window.location.origin}/auth/callback?redirect= }`. The `redirect` is read from the current location and forwarded so deep-links survive the email gap on the magic-link path. + - No new context method this commit (resend in commit 2). +- `web/src/components/auth/SignUpForm/SignUpForm.tsx`: + - After successful `signUp()`, branch on `data.session`: + - If `session` is non-null (auto-confirm fallback if confirmations are ever disabled): existing navigate-to-destination logic at lines 68-72. + - If `session` is null: set local `pending` state, render `` instead of the form. `handleVerified` runs the same destination logic via `getPostSignupDestination`. +- `web/src/router.tsx`: + - Add `/auth/callback` as a public child of root (sibling to `/sign-up`, `/sign-in`). Accepts `code` and optional `redirect` search params validated via Zod. + +**Tests:** +- `SignUpForm.test.tsx`: + - When mocked `signUp` returns no session, `` is rendered and no navigation occurs + - When mocked `signUp` returns a session (auto-confirm fallback), existing navigation behavior preserved +- `CheckEmailNotice.test.tsx`: + - Renders email and code input + - Submitting valid code calls `supabase.auth.verifyOtp` with expected args + - On verify success, calls `onVerified` prop + - On verify error, renders `` + - Code input enforces 6 digits / numeric only (via `inputMode` and `pattern` — server is the source of truth) +- `AuthCallback.test.tsx`: + - On mount with `code` param, calls `exchangeCodeForSession` and navigates to `redirect` or default + - On exchange error, renders `` with `/sign-in` link + - With no `code` param, renders error state +- `AuthContext.test.tsx`: + - `signUp` calls Supabase with the expected `emailRedirectTo` (including the forwarded `redirect` query) +- E2E (`e2e/tests/auth.spec.ts`): TWO new tests using Inbucket's REST API: + - List mailbox: `GET http://127.0.0.1:54424/api/v1/mailbox/` returns JSON array of message metadata + - Fetch message: `GET http://127.0.0.1:54424/api/v1/mailbox//` returns body + - Port 54424 = Inbucket dev port 54324 (`api/supabase/config.toml:96`) + 100 per the e2e port-shift rule + - Magic-link path: fill signup form via UI → assert `` appears → fetch the latest mail → extract the magic link from the body → navigate Playwright to it → assert the app shows `/create-team` + - OTP path: fill signup form → assert `` appears → fetch the same mail → extract the 6-digit token → type it into the OTP input → assert the app shows `/create-team` + +**Verification:** +1. `cd e2e/supabase && supabase stop && supabase start` to pick up config changes (also `cd api/supabase && supabase stop && supabase start`) +2. `npm run web:dev` + `npm run api:watch`; sign up with a fresh email; confirm `` appears; open Inbucket at `http://127.0.0.1:54324`; both pathways work — clicking the link AND typing the code each result in landing on `/create-team` +3. `npm run web:test` + `npm run api:test` + `npm run e2e` all green + +--- + +### Commit 2 — Resend confirmation email + +**Goal:** "Resend" button on the pending UI re-sends the confirmation email (which contains both link and OTP). + +**Wiring:** +- `web/src/contexts/AuthContext.ts` — add `resendConfirmation(email: string, options?: { redirect?: string }): Promise` to interface +- `web/src/contexts/AuthContext.tsx` — implement: calls `supabase.auth.resend({ type: 'signup', email, options: { emailRedirectTo: ${origin}/auth/callback${redirect ? '?redirect=' + encodeURIComponent(redirect) : ''} } })`; throws on error +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` — add Resend button below the OTP form. On click, call `onResend()` (parent passes `auth.resendConfirmation(email, { redirect })`). Use existing `` for loading state. Use existing `LiveRegion` to announce success ("New confirmation email sent"). Use `` for errors, with friendly handling for Supabase's rate-limit error code (`over_email_send_rate_limit` → "Please wait a moment before requesting another email"). +- `SignUpForm.tsx` — pass `onResend={() => auth.resendConfirmation(email, { redirect })}` to ``, where `redirect` is read from the current route's search params (same source as the existing `redirect` handling at `SignUpForm.tsx:68-72`) + +**Tests:** +- `AuthContext.test.tsx`: `resendConfirmation` calls `supabase.auth.resend` with the correct args (type, email, emailRedirectTo); also tests the `redirect` option is encoded into emailRedirectTo correctly +- `CheckEmailNotice.test.tsx`: + - Clicking Resend calls `onResend` + - Loading state visible during await + - Success message announced via `LiveRegion` + - Generic error rendered via `InlineError` + - Rate-limit error renders friendly text +- `SignUpForm.test.tsx`: Resend button passes the current `redirect` param to `resendConfirmation` + +**Verification:** +- Manual: sign up, click Resend, verify a second email appears in Inbucket; either email's link OR either email's most-recent OTP completes verification +- Manual: rapid-fire Resend → rate-limit message +- All test commands green + +--- + +### Commit 3 — Friendly handling of unconfirmed-email sign-in + +**Goal:** If a user signs up but never confirms, then later tries to sign in, they see the same `` (with OTP entry + Resend) instead of a generic auth error. + +**Wiring:** +- `web/src/components/auth/SignInForm/SignInForm.tsx` — after `signIn()` throws, inspect the error: if it's Supabase's `email_not_confirmed` (check `error.code === 'email_not_confirmed'` — verified against `auth-js/src/lib/error-codes.ts`; `AuthApiError` has a string `code` property per `auth-js/src/lib/errors.ts`), set local `pending` state and render ` auth.resendConfirmation(email, { redirect })} />`, where `redirect` is the current route's `redirect` search param (same source as today's signin redirect handling at `SignInForm.tsx:33-37`). `handleVerified` here navigates to redirect-or-`/leagues`. For any other error, fall through to the existing `` at `SignInForm.tsx:39-43`. The Resend's `redirect` override means the resent email's magic link will encode the signin-time destination rather than the (potentially different) signup-time one. + +**Tests:** +- `SignInForm.test.tsx`: + - Mocked `signIn` rejects with `email_not_confirmed` → `` rendered, no navigation + - Mocked `signIn` rejects with any other error → existing `` behavior preserved + - On `` `onVerified`, navigation runs with signin destination logic + - Resend from this context passes the signin-time `redirect` param to `resendConfirmation` + +**Verification:** +- Manual: sign up, do NOT click the email link / enter the OTP, navigate to `/sign-in`, attempt sign-in with the unconfirmed credentials, verify pending UI appears with working OTP entry + Resend +- All test commands green + +--- + +## Critical files + +**Modified:** +- `api/supabase/config.toml` (commit 1) +- `e2e/supabase/config.toml` (commit 1) +- `web/src/lib/supabase.ts` (commit 1 — add `flowType: 'pkce'`) +- `web/src/contexts/AuthContext.tsx` (commits 1, 2) +- `web/src/contexts/AuthContext.ts` (commit 2) +- `web/src/components/auth/SignUpForm/SignUpForm.tsx` (commit 1, minor in commit 2) +- `web/src/components/auth/SignInForm/SignInForm.tsx` (commit 3) +- `web/src/router.tsx` (commit 1 — add `/auth/callback` route) + +**New:** +- `api/supabase/templates/confirmation.html` (commit 1; includes both link and OTP from the start) +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` + `.test.tsx` (commit 1; expanded in commit 2) +- `web/src/components/auth/AuthCallback/AuthCallback.tsx` + `.test.tsx` (commit 1) +- `web/src/lib/auth-destination.ts` (commit 1) +- `web/src/components/ui/input-otp.tsx` (commit 1; added via shadcn workflow — exact path matches existing shadcn components in the project) +- `web/package.json` — adds `input-otp` dependency (commit 1) + +**Reused (existing components, no changes):** +- `web/src/components/InlineError/InlineError.tsx` — error display in ``, ``, existing forms +- `web/src/components/LiveRegion/LiveRegion.tsx` — screen-reader announcements +- `web/src/components/LoadingButton/LoadingButton.tsx` — Verify and Resend button loading states +- `web/src/lib/supabase.ts` — Supabase client singleton +- `web/src/hooks/useAuth.ts` — auth hook (gains `resendConfirmation` in commit 2) + +--- + +## End-to-end verification (after all 3 commits) + +1. **Magic-link happy path:** Sign up with new email → see `` → check Inbucket → click link → land on `/create-team` logged in +2. **OTP happy path:** Sign up with new email → see `` → check Inbucket → type 6-digit code into OTP field → land on `/create-team` logged in +3. **Resend:** Sign up → click Resend → second email arrives in Inbucket → either email's link OR the latest OTP completes verification (older OTP is invalidated by the new send per Supabase's behavior) +4. **Unconfirmed sign-in:** Sign up → DON'T confirm → go to `/sign-in` → submit credentials → see `` → resend or OTP entry both work → land on `/leagues` (or wherever `redirect` param pointed) +5. **Existing e2e tests:** `npm run e2e` all green (admin fixture continues working via `email_confirm: true`) +6. **All tests + format + lint:** `npm run test:all` + `npm run web:lint` + `npm run web:format:check` + `npm run api:format:check` all green + +--- + +## Production deployment notes (post-merge, manual) + +- Enable Confirm Email in Supabase dashboard: Authentication → Providers → Email +- Add `https:///auth/callback` to the redirect URL allowlist +- Configure SMTP for outbound email (currently commented out in `api/supabase/config.toml:186-194`; production must use a real provider) +- Upload the custom confirmation template (Supabase dashboard reads templates from the dashboard in production, not from the local config file) +- **Raise email rate limit**: `[auth.rate_limit] email_sent = 2` (per hour) at `api/supabase/config.toml:149` is sane for local dev but far too low for production. Set in the Supabase dashboard to a value appropriate for expected signup volume. + +## Known preexisting concern (out of scope) + +- `on_auth_user_created` trigger in `api/supabase/migrations/20260108000000_create_user_profile_trigger.sql` fires on every `INSERT` into `auth.users`, regardless of whether the user has confirmed their email. With confirmations enabled, this means `Accounts` and `UserProfiles` rows are created at signup time even if the user never confirms. Result: orphan rows accumulate from abandoned signups. Cleanup (e.g., a periodic job to remove unconfirmed users older than N days, or moving the trigger to fire only after confirmation) is out of 164's scope but should be tracked as follow-up. From 4c2ddd573774670b0451ec59b8a37d4fd18a1881 Mon Sep 17 00:00:00 2001 From: Mark Date: Mon, 11 May 2026 08:01:10 -0500 Subject: [PATCH 02/25] docs(plans): split #164 plan into six commits; correct Mailpit API usage - Re-sequence Commit 1 into four self-contained commits: PKCE+AuthCallback, CheckEmailNotice vendoring, config flip + SignUpForm wiring, e2e. - Replace Inbucket REST paths with verified Mailpit endpoints (search?query=to:..., /message/{ID}, DELETE /messages) and embed real response shapes captured from the running container. - Note Supabase kept the [inbucket] config block and container name for back-compat even though the image is now supabase/mailpit. --- docs/plans/164-email-confirmation-signup.md | 188 +++++++++++++------- 1 file changed, 127 insertions(+), 61 deletions(-) diff --git a/docs/plans/164-email-confirmation-signup.md b/docs/plans/164-email-confirmation-signup.md index a3ffab2..42a194d 100644 --- a/docs/plans/164-email-confirmation-signup.md +++ b/docs/plans/164-email-confirmation-signup.md @@ -25,28 +25,88 @@ Email confirmations are currently disabled in Supabase (`enable_confirmations = | OTP input component | shadcn/ui `` (uses `input-otp` library) | Renders single invisible `` behind visual digit boxes — combines accessible single input (WCAG 1.3.5) with visual digit-box UX. Consistent with project's existing shadcn/ui usage. | | Pending-UI surface | Inline state + shared `` component | No URL params (OWASP: PII like email shouldn't appear in URLs — leaks via browser history, server logs, Referer to Sentry). Refresh degrades benignly: link in inbox still works, resend reachable via signin path. | | Email passing | React state within form components | User never retypes — Supabase's confirmation token uniquely identifies the user. Email passed as prop from parent (`SignUpForm`, `SignInForm`) into ``. | -| Sign-in unconfirmed handling | In scope, same issue (commit 3) | Completes the verification story: signup → pending → resend → confirm AND signin → unconfirmed → resend. | +| Sign-in unconfirmed handling | In scope, same issue (commit 6) | Completes the verification story: signup → pending → resend → confirm AND signin → unconfirmed → resend. | | Post-confirmation destination | Preserve current logic per parent (`SignUpForm` → redirect-or-`/create-team`; `SignInForm` → redirect-or-`/leagues`; magic link callback uses URL `redirect` search param, else `/create-team`) | Standardization deferred to #165. | | Resend with redirect override | `AuthContext.resendConfirmation(email, { redirect? })` accepts the current redirect param so the resent email's link encodes the *current* destination (e.g., signin's `/leagues/123`) rather than whatever was baked in at signup time | Helps a user who hits the unconfirmed-signin path with a deep link, triggers Resend, and uses the new email's link. The original signup email's link still routes to `/create-team` (can't fix retroactively); the OTP path always routes correctly via the callsite. | -| E2E approach | Existing admin-API fixture keeps `email_confirm: true` (auto-confirms); add ONE new test per new flow path (magic link in commit 1, OTP in commit 2) | Existing tests stay fast; targeted e2e covers the new wiring | -| Email template | Custom template at `api/supabase/templates/confirmation.html` includes both `{{ .ConfirmationURL }}` and `{{ .Token }}` from commit 1 | Default Supabase template already has both; we're shipping our own to control wording and prep for #22's branding | +| E2E approach | Existing admin-API fixture keeps `email_confirm: true` (auto-confirms); add two new tests in commit 4 (magic-link path + OTP path) | Existing tests stay fast; targeted e2e covers the new wiring | +| Email template | Custom template at `api/supabase/templates/confirmation.html` includes both `{{ .ConfirmationURL }}` and `{{ .Token }}`, lands in commit 3 alongside the config flip | Default Supabase template already has both; we're shipping our own to control wording and prep for #22's branding | | Cross-issue ordering | 164 → 167 → 165 (locked) | 164 builds verification primitives; 167 reuses callback route + OTP entry pattern for change-email re-verification; 165 audits all destinations after both new flows exist. | --- ## Commits -Each commit is a gate. Each must independently build, lint, test, and format. Wait for approval before moving on. Note: these commits are sized for review/approval — they don't necessarily map 1:1 to production deploys. Recommendation is to land all three before flipping production confirmations on, so users always have the OTP fallback path. +Each commit is a gate. Each must independently build, lint, test, and format. Wait for approval before moving on. Note: these commits are sized for review/approval — they don't necessarily map 1:1 to production deploys. Recommendation is to land all six before flipping production confirmations on, so users always have the OTP fallback path. The local-dev confirmation flip happens in commit 3; commits 1–2 are preparatory and safe under `enable_confirmations = false`. -### Commit 1 — Enable confirmations + PKCE flow + signup pending state + magic link callback + OTP verification +### Commit 1 — PKCE flow + `/auth/callback` route + AuthCallback handler -**Goal:** Signup goes end-to-end via either path: submit → "check your email, click link or enter code" UI → user clicks link OR types code → land back in app, logged in. (Resend deferred to commit 2; signin-unconfirmed deferred to commit 3.) +**Goal:** Stand up the magic-link landing page and switch the Supabase client to PKCE so the link returns `?code=` instead of an implicit hash. Safe to land while `enable_confirmations` is still `false`: the route exists but nothing emits a magic link yet, and PKCE is fully compatible with the existing implicit-flow happy path. **Supabase client flow change** (`web/src/lib/supabase.ts`): - Today: `createClient(url, key)` with no options → defaults to `flowType: 'implicit'` (verified in `auth-js` source — `DEFAULT_OPTIONS.flowType: 'implicit'`) - Required: pass `{ auth: { flowType: 'pkce' } }` so the magic link returns `?code=` (which `exchangeCodeForSession` consumes) instead of `#access_token=` (implicit hash flow) - This is a small but app-wide auth behavior change — covered by existing AuthContext tests +**New components:** +- `web/src/components/auth/AuthCallback/AuthCallback.tsx` — handles the magic-link return: on mount, reads `code` and optional `redirect` from URL search params, calls `supabase.auth.exchangeCodeForSession(code)`, on success navigates to `redirect` if present else `/create-team`, on error renders `` with link back to `/sign-in`. Reuses the existing `redirectSearchSchema` Zod validator at `web/src/router.tsx:87-93` (validates `redirect` starts with `/` to prevent open redirects). +- `web/src/components/auth/AuthCallback/AuthCallback.test.tsx` + +**New helper** (`web/src/lib/auth-destination.ts`): +- `getPostSignupDestination(redirectParam?: string): string` — returns `redirectParam` if present, else `/create-team`. Search-param validation is already enforced upstream by `redirectSearchSchema` (router.tsx:87-93) when the route validates its search params, so this helper just chooses between two valid values. Used by `AuthCallback` here, and later by `SignUpForm` and ``'s `onVerified` from the signup callsite (commit 3). Lands here because AuthCallback is its first caller. + +**Wiring:** +- `web/src/router.tsx` — add `/auth/callback` as a public child of root (sibling to `/sign-up`, `/sign-in`). Accepts `code` and optional `redirect` search params validated via Zod. + +**Tests:** +- `AuthCallback.test.tsx`: + - On mount with `code` param, calls `exchangeCodeForSession` and navigates to `redirect` or default + - On exchange error, renders `` with `/sign-in` link + - With no `code` param, renders error state +- `auth-destination.test.ts`: + - Returns the redirect param when provided + - Falls back to `/create-team` when redirect is missing/empty +- Existing `AuthContext.test.tsx` keeps passing under PKCE (no behavioral change to the public API). + +**Verification:** +1. `npm run web:test` green +2. `npm run web:lint` + `npm run web:format:check` green +3. Manual: existing signup/signin flow still works (auto-confirm path, no email sent because `enable_confirmations` is still `false`) + +--- + +### Commit 2 — `` component + `input-otp` vendoring + +**Goal:** Add the pending-state UI as a self-contained, tested component. Not yet wired into any form — that happens in commit 3. + +**New dependency:** +- `input-otp` package via shadcn CLI: `npx shadcn@latest add input-otp` — installs to `web/src/components/ui/input-otp.tsx`, matching the project's existing shadcn vendoring per `web/components.json` aliases. Per `web/CLAUDE.md`, shadcn primitives are vendored — do not modify after install. Adds `input-otp` to `web/package.json`. + +**New components:** +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` — shared pending UI. Props: + - `email: string` — displayed in message + - `onVerified: () => void` — called after `verifyOtp` resolves successfully so parent can navigate + - `onResend?: () => Promise` — wired in commit 5 (placeholder/no-op for commits 2–4) + - Renders: instructional copy ("We sent a link to . Click the link in the email, or enter the 6-digit code below."), a shadcn/ui `` component (renders 6 visual digit slots with a single accessible input + `autocomplete="one-time-code"` underneath), a `` that calls `supabase.auth.verifyOtp({ email, token: code, type: 'signup' })` on submit. On error, render ``. On success, call `onVerified`. (`type: 'signup'` is the verified value from `auth-js` `EmailOtpType` for initial-signup confirmation.) +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.test.tsx` + +**Tests:** +- `CheckEmailNotice.test.tsx`: + - Renders email and code input + - Submitting valid code calls `supabase.auth.verifyOtp` with expected args + - On verify success, calls `onVerified` prop + - On verify error, renders `` + - Code input enforces 6 digits / numeric only (via `inputMode` and `pattern` — server is the source of truth) + +**Verification:** +1. `npm run web:test` + `npm run web:lint` + `npm run web:format:check` green +2. Manual: nothing to verify in-app yet (component unwired); spot-check Storybook-style by importing into a scratch page if desired + +--- + +### Commit 3 — Enable confirmations + wire SignUpForm into the pending UI + +**Goal:** Flip the feature on. Signup goes end-to-end via either path: submit → "check your email, click link or enter code" UI → user clicks link OR types code → land back in app, logged in. (Resend deferred to commit 5; signin-unconfirmed deferred to commit 6.) This is the load-bearing commit; commits 1–2 are preparatory. + **Supabase config** (`api/supabase/config.toml` and `e2e/supabase/config.toml`): - `[auth] site_url` → set to local web URL (`http://localhost:5173` for dev, `http://localhost:5273` for e2e). Existing `http://127.0.0.1:3000` is dead config; nothing in the codebase references it. - `[auth] additional_redirect_urls` → `["http://localhost:5173/auth/callback"]` for dev (and `5273` for e2e). Existing `https://127.0.0.1:3000` is HTTPS on a non-running port — wrong on multiple counts. @@ -58,62 +118,67 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa - Wording explicitly mentions "Click the link OR enter the 6-digit code in the app" - Plain HTML; references app name placeholder; #22 will brand it -**New components:** -- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` — shared pending UI. Props: - - `email: string` — displayed in message - - `onVerified: () => void` — called after `verifyOtp` resolves successfully so parent can navigate - - `onResend?: () => Promise` — wired in commit 2 (placeholder/no-op for commit 1) - - Renders: instructional copy ("We sent a link to . Click the link in the email, or enter the 6-digit code below."), a shadcn/ui `` component (renders 6 visual digit slots with a single accessible input + `autocomplete="one-time-code"` underneath), a `` that calls `supabase.auth.verifyOtp({ email, token: code, type: 'signup' })` on submit. On error, render ``. On success, call `onVerified`. (`type: 'signup'` is the verified value from `auth-js` `EmailOtpType` for initial-signup confirmation.) - - Adds dependency: `input-otp` package via shadcn CLI (`npx shadcn@latest add input-otp` — installs to `web/src/components/ui/input-otp.tsx`, matching the project's existing shadcn vendoring per `web/components.json` aliases). Per `web/CLAUDE.md`, shadcn primitives are vendored — do not modify after install. -- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.test.tsx` -- `web/src/components/auth/AuthCallback/AuthCallback.tsx` — handles the magic-link return: on mount, reads `code` and optional `redirect` from URL search params, calls `supabase.auth.exchangeCodeForSession(code)`, on success navigates to `redirect` if present else `/create-team`, on error renders `` with link back to `/sign-in`. Reuses the existing `redirectSearchSchema` Zod validator at `web/src/router.tsx:87-93` (validates `redirect` starts with `/` to prevent open redirects). -- `web/src/components/auth/AuthCallback/AuthCallback.test.tsx` - -**New helper** (`web/src/lib/auth-destination.ts`): -- `getPostSignupDestination(redirectParam?: string): string` — returns `redirectParam` if present, else `/create-team`. Search-param validation is already enforced upstream by `redirectSearchSchema` (router.tsx:87-93) when the route validates its search params, so this helper just chooses between two valid values. Used by `SignUpForm` (post-success when session is returned), `AuthCallback`, and ``'s `onVerified` from the signup callsite. (`SignInForm`'s default destination stays as today's `/leagues`; this helper is the signup-flow default.) - **Wiring:** - `web/src/contexts/AuthContext.tsx`: - Update `signUp()` to pass `options: { data: { displayName }, emailRedirectTo: ${window.location.origin}/auth/callback?redirect= }`. The `redirect` is read from the current location and forwarded so deep-links survive the email gap on the magic-link path. - - No new context method this commit (resend in commit 2). + - No new context method this commit (resend in commit 5). - `web/src/components/auth/SignUpForm/SignUpForm.tsx`: - After successful `signUp()`, branch on `data.session`: - If `session` is non-null (auto-confirm fallback if confirmations are ever disabled): existing navigate-to-destination logic at lines 68-72. - If `session` is null: set local `pending` state, render `` instead of the form. `handleVerified` runs the same destination logic via `getPostSignupDestination`. -- `web/src/router.tsx`: - - Add `/auth/callback` as a public child of root (sibling to `/sign-up`, `/sign-in`). Accepts `code` and optional `redirect` search params validated via Zod. **Tests:** - `SignUpForm.test.tsx`: - When mocked `signUp` returns no session, `` is rendered and no navigation occurs - When mocked `signUp` returns a session (auto-confirm fallback), existing navigation behavior preserved -- `CheckEmailNotice.test.tsx`: - - Renders email and code input - - Submitting valid code calls `supabase.auth.verifyOtp` with expected args - - On verify success, calls `onVerified` prop - - On verify error, renders `` - - Code input enforces 6 digits / numeric only (via `inputMode` and `pattern` — server is the source of truth) -- `AuthCallback.test.tsx`: - - On mount with `code` param, calls `exchangeCodeForSession` and navigates to `redirect` or default - - On exchange error, renders `` with `/sign-in` link - - With no `code` param, renders error state - `AuthContext.test.tsx`: - `signUp` calls Supabase with the expected `emailRedirectTo` (including the forwarded `redirect` query) -- E2E (`e2e/tests/auth.spec.ts`): TWO new tests using Inbucket's REST API: - - List mailbox: `GET http://127.0.0.1:54424/api/v1/mailbox/` returns JSON array of message metadata - - Fetch message: `GET http://127.0.0.1:54424/api/v1/mailbox//` returns body - - Port 54424 = Inbucket dev port 54324 (`api/supabase/config.toml:96`) + 100 per the e2e port-shift rule - - Magic-link path: fill signup form via UI → assert `` appears → fetch the latest mail → extract the magic link from the body → navigate Playwright to it → assert the app shows `/create-team` - - OTP path: fill signup form → assert `` appears → fetch the same mail → extract the 6-digit token → type it into the OTP input → assert the app shows `/create-team` **Verification:** 1. `cd e2e/supabase && supabase stop && supabase start` to pick up config changes (also `cd api/supabase && supabase stop && supabase start`) -2. `npm run web:dev` + `npm run api:watch`; sign up with a fresh email; confirm `` appears; open Inbucket at `http://127.0.0.1:54324`; both pathways work — clicking the link AND typing the code each result in landing on `/create-team` -3. `npm run web:test` + `npm run api:test` + `npm run e2e` all green +2. `npm run web:dev` + `npm run api:watch`; sign up with a fresh email; confirm `` appears; open the Mailpit UI at `http://127.0.0.1:54324` (served by the container Supabase still names `supabase_inbucket_*`); both pathways work — clicking the link AND typing the code each result in landing on `/create-team` +3. `npm run web:test` + `npm run api:test` green +4. `npm run e2e` green — existing tests continue working via the admin-API fixture's `email_confirm: true` (auto-confirms, bypassing the email step) + +--- + +### Commit 4 — E2E coverage for magic-link and OTP signup paths + +**Goal:** Cross-system assertion that the wired-up flow from commit 3 actually works end-to-end through a real browser, against the local Supabase + Mailpit stack. + +**New fixture** (`e2e/fixtures/mailpit.ts`): +- Helper functions: `searchByRecipient(email)`, `getMessage(id)`, `clearAll()`. Targets `http://127.0.0.1:54424` (e2e Mailpit). +- **Note on naming:** Supabase CLI replaced Inbucket with Mailpit but kept the `[inbucket]` config block and the container name `supabase_inbucket_*` for backward compatibility. The actual image is `public.ecr.aws/supabase/mailpit` (verified via `docker inspect`), and the API surface is Mailpit's, not Inbucket's. +- **Search by recipient:** `GET /api/v1/search?query=to:` — returns newest-first. Verified shape: + ```json + { "total": 15, "count": 1, "messages_count": 1, + "messages": [ { "ID": "Vqp696v5jB9384dNn4y8Pr", + "From": { "Name": "Admin", "Address": "admin@email.com" }, + "To": [ { "Name": "", "Address": "bob2@test.com" } ], + "Subject": "Confirm your F1 Fantasy email", + "Created": "2026-05-10T04:29:00.864Z", + "Snippet": "Confirm your email Welcome to F1 Fantasy!..." } ] } + ``` +- **Fetch message body:** `GET /api/v1/message/{ID}` — note singular `message`. Returns `{ ID, Text, HTML, ... }`. The `Text` body of a Supabase signup email contains both the magic link and the OTP. Verified excerpt: + ``` + Confirm your email ( http://127.0.0.1:54421/auth/v1/verify?token=pkce_49af69736d98ab8581cfb35402e76a84b69db32b81ec70fd4f78af8b&type=signup&redirect_to=http://localhost:5273/auth/callback ) + + Or enter this code in the app: *765877* + ``` + The verify URL is *Supabase's* endpoint (port 54421 in e2e); hitting it 302s to the app callback (`http://localhost:5273/auth/callback?code=...`) which `` then exchanges for a session. The OTP is wrapped in `*…*` (markdown bold). +- **Per-test isolation:** `DELETE /api/v1/messages` (no body) clears all mailboxes — call from a per-test `beforeEach` so prior tests' emails don't bleed in. +- **Port arithmetic:** 54424 = Mailpit dev port 54324 (`api/supabase/config.toml:96`, under the still-named `[inbucket]` block) + 100 per the e2e port-shift rule. + +**New e2e tests** (`e2e/tests/auth.spec.ts`): +- **Magic-link path:** fill signup form via UI → assert `` appears → search Mailpit for the address → GET the message → regex `Text` for the verify URL → `page.goto(verifyUrl)` → assert the app shows `/create-team`. +- **OTP path:** fill signup form → assert `` appears → search Mailpit → GET the same message → regex `Text` for `/Or enter this code in the app: \*(\d{6})\*/` → type the 6 digits into the OTP input → assert the app shows `/create-team`. + +**Verification:** +- `npm run e2e` green (existing suite + the two new tests) --- -### Commit 2 — Resend confirmation email +### Commit 5 — Resend confirmation email **Goal:** "Resend" button on the pending UI re-sends the confirmation email (which contains both link and OTP). @@ -134,13 +199,13 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa - `SignUpForm.test.tsx`: Resend button passes the current `redirect` param to `resendConfirmation` **Verification:** -- Manual: sign up, click Resend, verify a second email appears in Inbucket; either email's link OR either email's most-recent OTP completes verification +- Manual: sign up, click Resend, verify a second email appears in Mailpit; either email's link OR either email's most-recent OTP completes verification - Manual: rapid-fire Resend → rate-limit message - All test commands green --- -### Commit 3 — Friendly handling of unconfirmed-email sign-in +### Commit 6 — Friendly handling of unconfirmed-email sign-in **Goal:** If a user signs up but never confirms, then later tries to sign in, they see the same `` (with OTP entry + Resend) instead of a generic auth error. @@ -163,37 +228,38 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa ## Critical files **Modified:** -- `api/supabase/config.toml` (commit 1) -- `e2e/supabase/config.toml` (commit 1) - `web/src/lib/supabase.ts` (commit 1 — add `flowType: 'pkce'`) -- `web/src/contexts/AuthContext.tsx` (commits 1, 2) -- `web/src/contexts/AuthContext.ts` (commit 2) -- `web/src/components/auth/SignUpForm/SignUpForm.tsx` (commit 1, minor in commit 2) -- `web/src/components/auth/SignInForm/SignInForm.tsx` (commit 3) - `web/src/router.tsx` (commit 1 — add `/auth/callback` route) +- `api/supabase/config.toml` (commit 3) +- `e2e/supabase/config.toml` (commit 3) +- `web/src/contexts/AuthContext.tsx` (commits 3, 5) +- `web/src/contexts/AuthContext.ts` (commit 5) +- `web/src/components/auth/SignUpForm/SignUpForm.tsx` (commit 3, minor in commit 5) +- `web/src/components/auth/SignInForm/SignInForm.tsx` (commit 6) **New:** -- `api/supabase/templates/confirmation.html` (commit 1; includes both link and OTP from the start) -- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` + `.test.tsx` (commit 1; expanded in commit 2) - `web/src/components/auth/AuthCallback/AuthCallback.tsx` + `.test.tsx` (commit 1) -- `web/src/lib/auth-destination.ts` (commit 1) -- `web/src/components/ui/input-otp.tsx` (commit 1; added via shadcn workflow — exact path matches existing shadcn components in the project) -- `web/package.json` — adds `input-otp` dependency (commit 1) +- `web/src/lib/auth-destination.ts` + `auth-destination.test.ts` (commit 1) +- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` + `.test.tsx` (commit 2; expanded in commit 5) +- `web/src/components/ui/input-otp.tsx` (commit 2; added via shadcn workflow — exact path matches existing shadcn components in the project) +- `web/package.json` — adds `input-otp` dependency (commit 2) +- `api/supabase/templates/confirmation.html` (commit 3; includes both link and OTP from the start) +- `e2e/fixtures/mailpit.ts` (commit 4) +- New tests in `e2e/tests/auth.spec.ts` (commit 4) **Reused (existing components, no changes):** - `web/src/components/InlineError/InlineError.tsx` — error display in ``, ``, existing forms - `web/src/components/LiveRegion/LiveRegion.tsx` — screen-reader announcements - `web/src/components/LoadingButton/LoadingButton.tsx` — Verify and Resend button loading states -- `web/src/lib/supabase.ts` — Supabase client singleton -- `web/src/hooks/useAuth.ts` — auth hook (gains `resendConfirmation` in commit 2) +- `web/src/hooks/useAuth.ts` — auth hook (gains `resendConfirmation` in commit 5) --- -## End-to-end verification (after all 3 commits) +## End-to-end verification (after all 6 commits) -1. **Magic-link happy path:** Sign up with new email → see `` → check Inbucket → click link → land on `/create-team` logged in -2. **OTP happy path:** Sign up with new email → see `` → check Inbucket → type 6-digit code into OTP field → land on `/create-team` logged in -3. **Resend:** Sign up → click Resend → second email arrives in Inbucket → either email's link OR the latest OTP completes verification (older OTP is invalidated by the new send per Supabase's behavior) +1. **Magic-link happy path:** Sign up with new email → see `` → check Mailpit → click link → land on `/create-team` logged in +2. **OTP happy path:** Sign up with new email → see `` → check Mailpit → type 6-digit code into OTP field → land on `/create-team` logged in +3. **Resend:** Sign up → click Resend → second email arrives in Mailpit → either email's link OR the latest OTP completes verification (older OTP is invalidated by the new send per Supabase's behavior) 4. **Unconfirmed sign-in:** Sign up → DON'T confirm → go to `/sign-in` → submit credentials → see `` → resend or OTP entry both work → land on `/leagues` (or wherever `redirect` param pointed) 5. **Existing e2e tests:** `npm run e2e` all green (admin fixture continues working via `email_confirm: true`) 6. **All tests + format + lint:** `npm run test:all` + `npm run web:lint` + `npm run web:format:check` + `npm run api:format:check` all green From 8a4a2149575733463213182aa519187c776d4850 Mon Sep 17 00:00:00 2001 From: Mark Date: Mon, 11 May 2026 08:18:19 -0500 Subject: [PATCH 03/25] docs: rename Inbucket label to Mailpit; link CLAUDE.md topology pointer Supabase swapped the local email-testing binary to Mailpit while leaving the [inbucket] config block name and supabase_inbucket_* container name in place for back-compat. The runtime, REST API, and UI are 100% Mailpit, so the README port table now labels the column accordingly. Convert the CLAUDE.md pointer into a proper markdown link to the README section. --- CLAUDE.md | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c5908a5..231ec8b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -212,7 +212,7 @@ Open this folder in VSCode and use: ## Local Services Topology -See `README.md` → "Local Services Topology" for the dev vs e2e stack layout, port-shift rule, and migration-sharing details. Quick recall: dev processes use Supabase `54321–54324` / web `5173` / API `5077`; e2e processes are all shifted by +100; `api/supabase/migrations/` is the source of truth and e2e symlinks to it. +See [Local Services Topology](README.md#local-services-topology) in the README for the dev vs e2e stack layout, port-shift rule, and migration-sharing details. Quick recall: dev processes use Supabase `54321–54324` / web `5173` / API `5077`; e2e processes are all shifted by +100; `api/supabase/migrations/` is the source of truth and e2e symlinks to it. ## Production Infrastructure diff --git a/README.md b/README.md index fb99aee..62c36ec 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,7 @@ npm run test:all # Frontend + backend (unit + integration). Does n Two Supabase CLI stacks and two sets of web/API servers coexist on one machine so dev work and the e2e suite never touch the same state. -| Stack | Config | Supabase ports (API/DB/Studio/Inbucket) | Web port | API port | +| Stack | Config | Supabase ports (API/DB/Studio/Mailpit) | Web port | API port | | ----- | --------------- | --------------------------------------- | -------- | -------- | | Dev | `api/supabase/` | `54321 / 54322 / 54323 / 54324` | `5173` | `5077` | | E2E | `e2e/supabase/` | `54421 / 54422 / 54423 / 54424` | `5273` | `5177` | From 23ca5ec8069eb19742d967065985f96ea7a230fd Mon Sep 17 00:00:00 2001 From: Mark Date: Mon, 11 May 2026 18:14:45 -0500 Subject: [PATCH 04/25] feat(web): add PKCE flow and /auth/callback loader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switches the Supabase client to `flowType: 'pkce'` so confirmation emails return `?code=` instead of `#access_token=`, and adds a new `/auth/callback` route that exchanges the code for a session via a route loader. On success the loader redirects to the `redirect` search param (else `/create-team`); on failure it captures to Sentry and rethrows so the inline `errorComponent` renders the back-to-sign-in affordance. Safe to land while `enable_confirmations` remains `false` — the route exists but nothing emits a magic link yet, and PKCE is fully compatible with the existing implicit-flow happy path. Adds `getPostSignupDestination` helper, integration coverage over loader branches (default destination, redirect param, missing code, supabase error, supabase rejection), and unit coverage for the helper. Refs #164 --- web/src/lib/auth-destination.test.ts | 25 ++ web/src/lib/auth-destination.ts | 8 + web/src/lib/supabase.test.ts | 6 +- web/src/lib/supabase.ts | 6 +- web/src/router.tsx | 78 ++++++ .../auth-callback.integration.test.tsx | 225 ++++++++++++++++++ 6 files changed, 346 insertions(+), 2 deletions(-) create mode 100644 web/src/lib/auth-destination.test.ts create mode 100644 web/src/lib/auth-destination.ts create mode 100644 web/src/tests/integration/auth-callback.integration.test.tsx diff --git a/web/src/lib/auth-destination.test.ts b/web/src/lib/auth-destination.test.ts new file mode 100644 index 0000000..873b384 --- /dev/null +++ b/web/src/lib/auth-destination.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from 'vitest'; + +import { getPostSignupDestination } from './auth-destination'; + +describe('getPostSignupDestination', () => { + it('returns the redirect param when provided', () => { + expect(getPostSignupDestination('/leagues')).toBe('/leagues'); + }); + + it('returns the redirect param for nested paths', () => { + expect(getPostSignupDestination('/leagues/123')).toBe('/leagues/123'); + }); + + it('falls back to /create-team when redirect is undefined', () => { + expect(getPostSignupDestination(undefined)).toBe('/create-team'); + }); + + it('falls back to /create-team when redirect is omitted', () => { + expect(getPostSignupDestination()).toBe('/create-team'); + }); + + it('falls back to /create-team when redirect is an empty string', () => { + expect(getPostSignupDestination('')).toBe('/create-team'); + }); +}); diff --git a/web/src/lib/auth-destination.ts b/web/src/lib/auth-destination.ts new file mode 100644 index 0000000..68d8401 --- /dev/null +++ b/web/src/lib/auth-destination.ts @@ -0,0 +1,8 @@ +const DEFAULT_POST_SIGNUP_DESTINATION = '/create-team'; + +export function getPostSignupDestination(redirectParam?: string): string { + if (redirectParam) { + return redirectParam; + } + return DEFAULT_POST_SIGNUP_DESTINATION; +} diff --git a/web/src/lib/supabase.test.ts b/web/src/lib/supabase.test.ts index af07111..3a2a51f 100644 --- a/web/src/lib/supabase.test.ts +++ b/web/src/lib/supabase.test.ts @@ -77,6 +77,10 @@ describe('supabase module initialization', () => { await import('./supabase'); - expect(createClient).toHaveBeenCalledWith('https://myproject.supabase.co', 'my-anon-key'); + expect(createClient).toHaveBeenCalledWith('https://myproject.supabase.co', 'my-anon-key', { + auth: { + flowType: 'pkce', + }, + }); }); }); diff --git a/web/src/lib/supabase.ts b/web/src/lib/supabase.ts index 48eab95..23b6b80 100644 --- a/web/src/lib/supabase.ts +++ b/web/src/lib/supabase.ts @@ -11,4 +11,8 @@ if (!supabaseAnonKey) { throw new Error('Missing required environment variable: VITE_SUPABASE_ANON_KEY'); } -export const supabase = createClient(supabaseUrl, supabaseAnonKey); +export const supabase = createClient(supabaseUrl, supabaseAnonKey, { + auth: { + flowType: 'pkce', + }, +}); diff --git a/web/src/router.tsx b/web/src/router.tsx index cdac3aa..5576977 100644 --- a/web/src/router.tsx +++ b/web/src/router.tsx @@ -2,6 +2,7 @@ import { Account } from '@/components/Account/Account'; import { CreateTeam } from '@/components/CreateTeam/CreateTeam'; import { ErrorBoundary } from '@/components/ErrorBoundary/ErrorBoundary'; import { ErrorFallback } from '@/components/ErrorBoundary/ErrorFallback'; +import { InlineError } from '@/components/InlineError/InlineError'; import { LandingPage } from '@/components/LandingPage/LandingPage'; import { Layout } from '@/components/Layout/Layout'; import { League } from '@/components/League/League'; @@ -9,10 +10,13 @@ import { LeagueList } from '@/components/LeagueList/LeagueList'; import { MyTeamRoute, TeamRoute } from '@/components/Team/Team'; import { SignInForm } from '@/components/auth/SignInForm/SignInForm'; import { SignUpForm } from '@/components/auth/SignUpForm/SignUpForm'; +import { Button } from '@/components/ui/button'; import type { Team as TeamType } from '@/contracts/Team'; import type { UserProfile } from '@/contracts/UserProfile'; +import { getPostSignupDestination } from '@/lib/auth-destination'; import { requireAuth, requireNoTeam, requireTeam } from '@/lib/route-guards'; import type { RouterContext } from '@/lib/router-context'; +import { supabase } from '@/lib/supabase'; import { getAvailableLeagues, getLeagueById, getMyLeagues } from '@/services/leagueService'; import { getLeagueStandings } from '@/services/standingsService'; import { getMyTeam, getTeamById } from '@/services/teamService'; @@ -20,6 +24,7 @@ import { userProfileService } from '@/services/userProfileService'; import * as Sentry from '@sentry/react'; import { ErrorComponent, + Link, Outlet, createRootRouteWithContext, createRoute, @@ -92,6 +97,22 @@ const redirectSearchSchema = z.object({ .catch(undefined), }); +/** + * Zod schema for validating the /auth/callback search params. + * + * The PKCE flow returns a `code` param from Supabase that must be exchanged for + * a session. `redirect` is forwarded through the email link so the user lands + * on the intended destination after confirmation. + */ +const authCallbackSearchSchema = z.object({ + code: z.string().optional().catch(undefined), + redirect: z + .string() + .refine((url) => url.startsWith('/'), 'Redirect must be an internal path') + .optional() + .catch(undefined), +}); + /** * Root route with context - wraps all routes in the application. * @@ -223,6 +244,62 @@ const signUpRoute = createRoute({ errorComponent: ({ error }) => , }); +/** + * Auth callback route - public landing page for PKCE / magic-link returns. + * + * Supabase appends `code` (and optionally the forwarded `redirect`) to this URL + * when the user clicks a confirmation link. The loader exchanges the code for a + * session and throws a `redirect` to the post-confirmation destination. On any + * failure (missing code, expired/consumed code, network error) the loader + * captures to Sentry and rethrows so the `errorComponent` can render the + * back-to-sign-in affordance. + * + * @type {import('@tanstack/react-router').Route} + */ +const authCallbackRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/auth/callback', + validateSearch: authCallbackSearchSchema, + loaderDeps: ({ search }) => ({ code: search.code, redirect: search.redirect }), + loader: async ({ deps }) => { + try { + if (!deps.code) { + throw new Error('Missing confirmation code'); + } + const { error } = await supabase.auth.exchangeCodeForSession(deps.code); + if (error) throw error; + } catch (err) { + const captured = err instanceof Error ? err : new Error('Auth callback failed'); + Sentry.captureException(captured, { + tags: { component: 'authCallbackRoute', operation: 'exchangeCodeForSession' }, + }); + throw captured; + } + + throw redirect({ to: getPostSignupDestination(deps.redirect) }); + }, + pendingComponent: () => ( +
+
+
+

Confirming your email...

+
+
+ ), + errorComponent: () => ( +
+
+ +
+ +
+
+
+ ), +}); + /** * Join invite route - public route for previewing and joining leagues via invite link. * @@ -708,6 +785,7 @@ const routeTree = rootRoute.addChildren([ indexRoute, signInRoute, signUpRoute, + authCallbackRoute, joinInviteRoute, authenticatedLayoutRoute.addChildren([ accountRoute, diff --git a/web/src/tests/integration/auth-callback.integration.test.tsx b/web/src/tests/integration/auth-callback.integration.test.tsx new file mode 100644 index 0000000..79cad33 --- /dev/null +++ b/web/src/tests/integration/auth-callback.integration.test.tsx @@ -0,0 +1,225 @@ +import { InlineError } from '@/components/InlineError/InlineError'; +import { Button } from '@/components/ui/button'; +import { getPostSignupDestination } from '@/lib/auth-destination'; +import type { RouterContext } from '@/lib/router-context'; +import { supabase } from '@/lib/supabase'; +import { + buildStubRoute, + createBaseRouterContext, + createUnauthAuth, + renderWithRouter, +} from '@/tests/test-utils'; +import * as Sentry from '@sentry/react'; +import { + Link, + Outlet, + createRootRouteWithContext, + createRoute, + redirect, +} from '@tanstack/react-router'; +import { screen } from '@testing-library/react'; +import type { MockedFunction } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { z } from 'zod'; + +// Supabase is a third party we don't own — stub its client per the testing +// strategy ("Stub third parties you don't own"). The auth.exchangeCodeForSession +// call talks to Supabase's auth server, not our API, so MSW can't intercept it. +vi.mock('@/lib/supabase', () => ({ + supabase: { + auth: { + exchangeCodeForSession: vi.fn(), + }, + }, +})); + +vi.mock('@sentry/react', () => ({ + captureException: vi.fn(), +})); + +const authCallbackSearchSchema = z.object({ + code: z.string().optional().catch(undefined), + redirect: z + .string() + .refine((url) => url.startsWith('/'), 'Redirect must be an internal path') + .optional() + .catch(undefined), +}); + +// Mirrors `authCallbackRoute` from `router.tsx`. Per the integration-test +// convention, production routes aren't exported so the route under test is +// rebuilt inline — only what the test needs to mount. +function buildAuthCallbackRouteTree() { + const rootRoute = createRootRouteWithContext()({ + component: () => , + }); + + const authCallbackRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/auth/callback', + validateSearch: authCallbackSearchSchema, + loaderDeps: ({ search }) => ({ code: search.code, redirect: search.redirect }), + loader: async ({ deps }) => { + try { + if (!deps.code) { + throw new Error('Missing confirmation code'); + } + const { error } = await supabase.auth.exchangeCodeForSession(deps.code); + if (error) throw error; + } catch (err) { + const captured = err instanceof Error ? err : new Error('Auth callback failed'); + Sentry.captureException(captured, { + tags: { component: 'authCallbackRoute', operation: 'exchangeCodeForSession' }, + }); + throw captured; + } + throw redirect({ to: getPostSignupDestination(deps.redirect) }); + }, + errorComponent: () => ( +
+
+ +
+ +
+
+
+ ), + }); + + const createTeamStub = buildStubRoute(rootRoute, { + path: 'create-team', + heading: 'Create Team', + }); + + const leaguesStub = buildStubRoute(rootRoute, { + path: 'leagues', + heading: 'My Leagues', + }); + + const signInStub = buildStubRoute(rootRoute, { + path: 'sign-in', + heading: 'Sign In', + }); + + return rootRoute.addChildren([authCallbackRoute, createTeamStub, leaguesStub, signInStub]); +} + +describe('/auth/callback', () => { + const exchangeMock = supabase.auth.exchangeCodeForSession as unknown as MockedFunction< + typeof supabase.auth.exchangeCodeForSession + >; + const captureExceptionMock = Sentry.captureException as unknown as MockedFunction< + typeof Sentry.captureException + >; + + beforeEach(() => { + exchangeMock.mockReset(); + captureExceptionMock.mockReset(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('exchanges the code and lands on /create-team by default', async () => { + exchangeMock.mockResolvedValueOnce({ + data: { session: null, user: null }, + error: null, + } as unknown as Awaited>); + + renderWithRouter({ + routeTree: buildAuthCallbackRouteTree(), + initialEntry: '/auth/callback?code=abc123', + auth: createUnauthAuth(), + routerContext: createBaseRouterContext(), + }); + + expect(await screen.findByRole('heading', { name: /create team/i })).toBeInTheDocument(); + expect(exchangeMock).toHaveBeenCalledWith('abc123'); + expect(captureExceptionMock).not.toHaveBeenCalled(); + }); + + it('honors the redirect search param when present', async () => { + exchangeMock.mockResolvedValueOnce({ + data: { session: null, user: null }, + error: null, + } as unknown as Awaited>); + + renderWithRouter({ + routeTree: buildAuthCallbackRouteTree(), + initialEntry: '/auth/callback?code=abc123&redirect=%2Fleagues', + auth: createUnauthAuth(), + routerContext: createBaseRouterContext(), + }); + + expect(await screen.findByRole('heading', { name: /my leagues/i })).toBeInTheDocument(); + }); + + it('renders the error UI and captures to Sentry when no code is present', async () => { + renderWithRouter({ + routeTree: buildAuthCallbackRouteTree(), + initialEntry: '/auth/callback', + auth: createUnauthAuth(), + routerContext: createBaseRouterContext(), + }); + + expect(await screen.findByRole('alert')).toHaveTextContent(/couldn't confirm your email/i); + expect(screen.getByRole('link', { name: /back to sign in/i })).toHaveAttribute( + 'href', + '/sign-in', + ); + expect(exchangeMock).not.toHaveBeenCalled(); + expect(captureExceptionMock).toHaveBeenCalledWith( + expect.objectContaining({ message: 'Missing confirmation code' }), + expect.objectContaining({ + tags: expect.objectContaining({ component: 'authCallbackRoute' }), + }), + ); + }); + + it('renders the error UI and captures to Sentry when supabase returns an error', async () => { + const supabaseError = new Error('invalid code'); + exchangeMock.mockResolvedValueOnce({ + data: { session: null, user: null }, + error: supabaseError, + } as unknown as Awaited>); + + renderWithRouter({ + routeTree: buildAuthCallbackRouteTree(), + initialEntry: '/auth/callback?code=abc123', + auth: createUnauthAuth(), + routerContext: createBaseRouterContext(), + }); + + expect(await screen.findByRole('alert')).toHaveTextContent(/couldn't confirm your email/i); + expect(captureExceptionMock).toHaveBeenCalledWith( + supabaseError, + expect.objectContaining({ + tags: expect.objectContaining({ component: 'authCallbackRoute' }), + }), + ); + }); + + it('renders the error UI and captures to Sentry when supabase rejects', async () => { + const rejection = new Error('network down'); + exchangeMock.mockRejectedValueOnce(rejection); + + renderWithRouter({ + routeTree: buildAuthCallbackRouteTree(), + initialEntry: '/auth/callback?code=abc123', + auth: createUnauthAuth(), + routerContext: createBaseRouterContext(), + }); + + expect(await screen.findByRole('alert')).toHaveTextContent(/couldn't confirm your email/i); + expect(captureExceptionMock).toHaveBeenCalledWith( + rejection, + expect.objectContaining({ + tags: expect.objectContaining({ component: 'authCallbackRoute' }), + }), + ); + }); +}); From a02cf2d4d840d00f7bc1c6ec75859cc373ded2f2 Mon Sep 17 00:00:00 2001 From: Mark Date: Mon, 11 May 2026 18:15:23 -0500 Subject: [PATCH 05/25] docs(plans): reflect loader-based auth callback design Updates the #164 plan to match what actually landed in commit 1: the auth-callback work uses a route loader and inline `errorComponent` rather than a dedicated `AuthCallback` component with `useEffect`. - New Decisions row documents the loader+inline-errorComponent choice (idiomatic for this codebase, avoids `useEffect` + `cancelled`-flag anti-pattern, `ErrorFallback` doesn't fit). - New Decisions row records the manual Sentry-capture pattern in the loader and points at #180 for the broader observability gap. - Commit 1 section rewritten: drops the AuthCallback component bullets, describes the loader/pendingComponent/errorComponent wiring directly, and replaces unit-test cases with integration-test coverage. - Critical files updated to remove `AuthCallback/` and add the new integration test. - Removes the redundant cross-issue ordering paragraph from Context (already captured in the Decisions table). - Adds the loader-error observability gap to "Known preexisting concerns" with a pointer to #180. Refs #164 --- docs/plans/164-email-confirmation-signup.md | 46 ++++++++++++--------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/docs/plans/164-email-confirmation-signup.md b/docs/plans/164-email-confirmation-signup.md index 42a194d..e94216a 100644 --- a/docs/plans/164-email-confirmation-signup.md +++ b/docs/plans/164-email-confirmation-signup.md @@ -10,8 +10,6 @@ Email confirmations are currently disabled in Supabase (`enable_confirmations = 4. Complete verification when the user clicks the email link 5. Handle the case where an unconfirmed user later tries to sign in -**Cross-issue ordering** — this issue should ship before #167 (change-email re-verification, which reuses the same `/auth/callback` route and OTP entry UI) and before #165 (sign-in/redirect audit, which standardizes destinations across all auth flows). 164 preserves today's destination logic; 165 will standardize it later. - **Out of scope:** - Branded email template content (deferred to #22) - Standardizing post-auth redirect destinations (deferred to #165) @@ -31,6 +29,8 @@ Email confirmations are currently disabled in Supabase (`enable_confirmations = | E2E approach | Existing admin-API fixture keeps `email_confirm: true` (auto-confirms); add two new tests in commit 4 (magic-link path + OTP path) | Existing tests stay fast; targeted e2e covers the new wiring | | Email template | Custom template at `api/supabase/templates/confirmation.html` includes both `{{ .ConfirmationURL }}` and `{{ .Token }}`, lands in commit 3 alongside the config flip | Default Supabase template already has both; we're shipping our own to control wording and prep for #22's branding | | Cross-issue ordering | 164 → 167 → 165 (locked) | 164 builds verification primitives; 167 reuses callback route + OTP entry pattern for change-email re-verification; 165 audits all destinations after both new flows exist. | +| Auth callback implementation | **Route loader + inline `errorComponent`, no separate component** | Async work belongs in a TanStack Router `loader` (matches every other async route in the codebase — `joinInviteRoute`, `accountRoute`, etc.); avoids the `useEffect` + `cancelled`-flag anti-pattern. Side effects (Sentry capture) live at the failure site inside the loader's `try/catch`, not in a render-time hook. The shared `ErrorFallback` doesn't fit (hardcoded copy + "Try again" button; reloading can't fix a consumed PKCE code), and the bespoke error UI is small enough to inline in `errorComponent` rather than warrant a dedicated component file. | +| Sentry coverage for loader failures | Manual `Sentry.captureException` inside the loader's `try/catch` | The auth-callback failure mode is a Supabase SDK error that bypasses `apiClient`'s built-in HTTP-error capture, so without explicit capture in the loader, Sentry never sees PKCE failures. This is consistent with the existing pattern in `rootRoute.beforeLoad`. The codebase-wide observability gap (loader failures outside `apiClient` are silently rendered) is tracked separately in #180; commit 1 doesn't try to solve it here. | --- @@ -38,7 +38,7 @@ Email confirmations are currently disabled in Supabase (`enable_confirmations = Each commit is a gate. Each must independently build, lint, test, and format. Wait for approval before moving on. Note: these commits are sized for review/approval — they don't necessarily map 1:1 to production deploys. Recommendation is to land all six before flipping production confirmations on, so users always have the OTP fallback path. The local-dev confirmation flip happens in commit 3; commits 1–2 are preparatory and safe under `enable_confirmations = false`. -### Commit 1 — PKCE flow + `/auth/callback` route + AuthCallback handler +### Commit 1 — PKCE flow + `/auth/callback` loader **Goal:** Stand up the magic-link landing page and switch the Supabase client to PKCE so the link returns `?code=` instead of an implicit hash. Safe to land while `enable_confirmations` is still `false`: the route exists but nothing emits a magic link yet, and PKCE is fully compatible with the existing implicit-flow happy path. @@ -47,30 +47,35 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa - Required: pass `{ auth: { flowType: 'pkce' } }` so the magic link returns `?code=` (which `exchangeCodeForSession` consumes) instead of `#access_token=` (implicit hash flow) - This is a small but app-wide auth behavior change — covered by existing AuthContext tests -**New components:** -- `web/src/components/auth/AuthCallback/AuthCallback.tsx` — handles the magic-link return: on mount, reads `code` and optional `redirect` from URL search params, calls `supabase.auth.exchangeCodeForSession(code)`, on success navigates to `redirect` if present else `/create-team`, on error renders `` with link back to `/sign-in`. Reuses the existing `redirectSearchSchema` Zod validator at `web/src/router.tsx:87-93` (validates `redirect` starts with `/` to prevent open redirects). -- `web/src/components/auth/AuthCallback/AuthCallback.test.tsx` - **New helper** (`web/src/lib/auth-destination.ts`): -- `getPostSignupDestination(redirectParam?: string): string` — returns `redirectParam` if present, else `/create-team`. Search-param validation is already enforced upstream by `redirectSearchSchema` (router.tsx:87-93) when the route validates its search params, so this helper just chooses between two valid values. Used by `AuthCallback` here, and later by `SignUpForm` and ``'s `onVerified` from the signup callsite (commit 3). Lands here because AuthCallback is its first caller. +- `getPostSignupDestination(redirectParam?: string): string` — returns `redirectParam` if present, else `/create-team`. Search-param validation is already enforced upstream by the route's Zod schema, so this helper just chooses between two valid values. Used by the auth-callback loader here, and later by `SignUpForm` and ``'s `onVerified` from the signup callsite (commit 3). Lands here because the auth-callback loader is its first caller. -**Wiring:** -- `web/src/router.tsx` — add `/auth/callback` as a public child of root (sibling to `/sign-up`, `/sign-in`). Accepts `code` and optional `redirect` search params validated via Zod. +**Wiring** (`web/src/router.tsx`): +- New Zod schema `authCallbackSearchSchema` validates `code` (optional string) and `redirect` (optional string, must start with `/` — same shape as the existing `redirectSearchSchema`). +- New `authCallbackRoute` as a public child of root (sibling to `/sign-up`, `/sign-in`). Built as a loader-driven route: + - `loaderDeps` pulls `code` and `redirect` from the validated search params. + - `loader` wraps the exchange in `try/catch`: if `code` is missing or `supabase.auth.exchangeCodeForSession` returns/rejects with an error, capture to Sentry with `tags: { component: 'authCallbackRoute', operation: 'exchangeCodeForSession' }` and rethrow. On success, `throw redirect({ to: getPostSignupDestination(redirect) })`. + - `pendingComponent` renders a "Confirming your email..." spinner during the loader phase. + - `errorComponent` is inlined: renders `` with the user-friendly copy and a "Back to sign in" ``. No dedicated component file — the shared `ErrorFallback` doesn't fit (hardcoded copy, "Try again" button that can't recover a consumed PKCE code), and the bespoke UI is ~15 lines of JSX. + - No `component` field — the loader always throws (`redirect` on success, captured error otherwise), so the route never renders to a normal component. **Tests:** -- `AuthCallback.test.tsx`: - - On mount with `code` param, calls `exchangeCodeForSession` and navigates to `redirect` or default - - On exchange error, renders `` with `/sign-in` link - - With no `code` param, renders error state +- `web/src/tests/integration/auth-callback.integration.test.tsx` — covers loader branches via a real router mounted over an inline route-tree mirror (matches the convention in `account.integration.test.tsx`): + - Default destination: with `?code=abc`, loader exchanges and redirects to `/create-team`; no Sentry capture. + - `redirect` honored: with `?code=abc&redirect=/leagues`, loader redirects to `/leagues`. + - Missing `code`: renders error UI + `Sentry.captureException` called with `component: authCallbackRoute`. + - Supabase returns error: renders error UI + Sentry captured with the supabase error. + - Supabase rejects: renders error UI + Sentry captured with the rejection. - `auth-destination.test.ts`: - - Returns the redirect param when provided - - Falls back to `/create-team` when redirect is missing/empty + - Returns the redirect param when provided. + - Falls back to `/create-team` when redirect is missing/empty. - Existing `AuthContext.test.tsx` keeps passing under PKCE (no behavioral change to the public API). **Verification:** 1. `npm run web:test` green 2. `npm run web:lint` + `npm run web:format:check` green -3. Manual: existing signup/signin flow still works (auto-confirm path, no email sent because `enable_confirmations` is still `false`) +3. `npm run web:build` green (TypeScript compile) +4. Manual: existing signup/signin flow still works (auto-confirm path, no email sent because `enable_confirmations` is still `false`) --- @@ -229,7 +234,7 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa **Modified:** - `web/src/lib/supabase.ts` (commit 1 — add `flowType: 'pkce'`) -- `web/src/router.tsx` (commit 1 — add `/auth/callback` route) +- `web/src/router.tsx` (commit 1 — add `/auth/callback` route with loader, pendingComponent, and inline errorComponent) - `api/supabase/config.toml` (commit 3) - `e2e/supabase/config.toml` (commit 3) - `web/src/contexts/AuthContext.tsx` (commits 3, 5) @@ -238,8 +243,8 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa - `web/src/components/auth/SignInForm/SignInForm.tsx` (commit 6) **New:** -- `web/src/components/auth/AuthCallback/AuthCallback.tsx` + `.test.tsx` (commit 1) - `web/src/lib/auth-destination.ts` + `auth-destination.test.ts` (commit 1) +- `web/src/tests/integration/auth-callback.integration.test.tsx` (commit 1) - `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` + `.test.tsx` (commit 2; expanded in commit 5) - `web/src/components/ui/input-otp.tsx` (commit 2; added via shadcn workflow — exact path matches existing shadcn components in the project) - `web/package.json` — adds `input-otp` dependency (commit 2) @@ -248,7 +253,7 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa - New tests in `e2e/tests/auth.spec.ts` (commit 4) **Reused (existing components, no changes):** -- `web/src/components/InlineError/InlineError.tsx` — error display in ``, ``, existing forms +- `web/src/components/InlineError/InlineError.tsx` — error display in the auth-callback `errorComponent`, ``, existing forms - `web/src/components/LiveRegion/LiveRegion.tsx` — screen-reader announcements - `web/src/components/LoadingButton/LoadingButton.tsx` — Verify and Resend button loading states - `web/src/hooks/useAuth.ts` — auth hook (gains `resendConfirmation` in commit 5) @@ -277,3 +282,4 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa ## Known preexisting concern (out of scope) - `on_auth_user_created` trigger in `api/supabase/migrations/20260108000000_create_user_profile_trigger.sql` fires on every `INSERT` into `auth.users`, regardless of whether the user has confirmed their email. With confirmations enabled, this means `Accounts` and `UserProfiles` rows are created at signup time even if the user never confirms. Result: orphan rows accumulate from abandoned signups. Cleanup (e.g., a periodic job to remove unconfirmed users older than N days, or moving the trigger to fire only after confirmation) is out of 164's scope but should be tracked as follow-up. +- Non-HTTP errors thrown from route loaders and `beforeLoad` guards are silently rendered to the user via `errorComponent` and never reach Sentry — `apiClient` only captures HTTP failures, and React's error-boundary path doesn't see errors caught at the route layer. The auth-callback loader works around this with manual `Sentry.captureException`, but the broader codebase-wide gap (parsing errors, schema mismatches, third-party SDK errors, runtime bugs across every data-loading route) is tracked separately in #180. From 7cc96008523fe1949879aaa7fcee58f404695cdc Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 12 May 2026 12:23:48 -0500 Subject: [PATCH 06/25] docs(plans): swap commit 2 to hand-rolled OtpInput primitive Replaces the shadcn input-otp vendoring with a custom OtpInput at components/OtpInput/, fixing the slot-retargeting bug from shadcn/ui #4046 and dropping the input-otp dependency. Updates props/styling/a11y specs, restricts the test list to behavioral coverage per web/CLAUDE.md, and spells out the consumer-side and dev-artifact cleanups (REGEXP_ONLY_DIGITS, elementFromPoint polyfill, /dev/check-email scratch route). --- docs/plans/164-email-confirmation-signup.md | 80 ++++++++++++++++----- 1 file changed, 63 insertions(+), 17 deletions(-) diff --git a/docs/plans/164-email-confirmation-signup.md b/docs/plans/164-email-confirmation-signup.md index e94216a..7564312 100644 --- a/docs/plans/164-email-confirmation-signup.md +++ b/docs/plans/164-email-confirmation-signup.md @@ -20,7 +20,7 @@ Email confirmations are currently disabled in Supabase (`enable_confirmations = | Decision | Choice | Rationale | |---|---|---| | Confirmation method | **Both magic link AND OTP code** in the email; user can use whichever | Resilient to corporate email scanners (Defender, Barracuda, Mimecast) that pre-fetch links and consume the token before users click. Supabase auth issue [#1214](https://github.com/supabase/auth/issues/1214) is open and unfixed; no clean recovery for affected users without an OTP fallback. Default Supabase template already emits both, so the email-side cost is near zero. | -| OTP input component | shadcn/ui `` (uses `input-otp` library) | Renders single invisible `` behind visual digit boxes — combines accessible single input (WCAG 1.3.5) with visual digit-box UX. Consistent with project's existing shadcn/ui usage. | +| OTP input component | Custom `` at `web/src/components/OtpInput/` | Hand-rolled single-input + overlay slot pattern (same architecture as `input-otp`) that fixes the slot-retargeting bug ([shadcn/ui #4046](https://github.com/shadcn-ui/ui/issues/4046)) where clicking a non-final slot focuses the last cell. Preserves every must-have: single accessible `` for WCAG 1.3.5, iOS/Android SMS autofill via `autocomplete="one-time-code"`, native paste, full keyboard a11y, auto-submit. Lives outside `ui/` because `ui/` is reserved for vendored shadcn; sits next to the other custom primitives (`LoadingButton`, `InlineError`). Removes the `input-otp` dependency. | | Pending-UI surface | Inline state + shared `` component | No URL params (OWASP: PII like email shouldn't appear in URLs — leaks via browser history, server logs, Referer to Sentry). Refresh degrades benignly: link in inbox still works, resend reachable via signin path. | | Email passing | React state within form components | User never retypes — Supabase's confirmation token uniquely identifies the user. Email passed as prop from parent (`SignUpForm`, `SignInForm`) into ``. | | Sign-in unconfirmed handling | In scope, same issue (commit 6) | Completes the verification story: signup → pending → resend → confirm AND signin → unconfirmed → resend. | @@ -79,32 +79,79 @@ Each commit is a gate. Each must independently build, lint, test, and format. Wa --- -### Commit 2 — `` component + `input-otp` vendoring - -**Goal:** Add the pending-state UI as a self-contained, tested component. Not yet wired into any form — that happens in commit 3. - -**New dependency:** -- `input-otp` package via shadcn CLI: `npx shadcn@latest add input-otp` — installs to `web/src/components/ui/input-otp.tsx`, matching the project's existing shadcn vendoring per `web/components.json` aliases. Per `web/CLAUDE.md`, shadcn primitives are vendored — do not modify after install. Adds `input-otp` to `web/package.json`. - -**New components:** -- `web/src/components/auth/CheckEmailNotice/CheckEmailNotice.tsx` — shared pending UI. Props: +### Commit 2 — `` primitive + `` component + +**Goal:** Add the pending-state UI as a self-contained, tested component, backed by a hand-rolled `` primitive (no `input-otp` dependency). End result is visually identical to the prior shadcn-based version but fixes the slot-retargeting bug so a user can click any slot to edit just that digit. Not yet wired into any form — that happens in commit 3. + +**New primitive — ``** (`web/src/components/OtpInput/OtpInput.tsx`): + +- **Architecture:** Single-input + overlay slots, stacked via CSS Grid (not `position: absolute`). One real `` and one slot-row `
` are both children of a `grid` container, both placed at `col-start-1 row-start-1` so they occupy the same cell. The input is rendered with transparent text and caret (`text-transparent caret-transparent`) and sits on top of the slot row, which is the visible UI driven by `value` and `selectionStart`. The slot-row `
` is itself a flex row (`flex justify-center gap-2.5`) carrying the 6 slot children. Grid stacking is preferred over `position: absolute` here because the container intrinsic-sizes to the slot row automatically — no `relative` ancestor or hand-tuned insets needed. Same end behavior as the `input-otp` library's overlay approach; the only meaningful difference is the click-to-edit handler that the library gets wrong. +- **Scope of this primitive:** numeric OTP input only. The non-digit filter and `inputMode="numeric"` default are baked in; this is not a generic OTP primitive (no `pattern` prop). Spell that out in the component's JSDoc so a future caller doesn't try to pass alpha codes. +- **Why this shape, not N individual inputs:** `` SMS autofill on iOS only works with a single input ([web.dev — SMS OTP form](https://web.dev/articles/sms-otp-form), confirmed broken on multi-input in [Chakra #4095](https://github.com/chakra-ui/chakra-ui/issues/4095), [react-verification-input #57](https://github.com/andreaswilli/react-verification-input/issues/57)). Paste, backspace, and arrow nav also come free at the browser level instead of needing hand-coded ref coordination. +- **Props** (minimal, mirrors what `CheckEmailNotice` actually uses): + - `id?: string` — forwarded to the underlying `` so external `