diff --git a/.gitignore b/.gitignore index 5565c87..fd25936 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ yarn-error.log* # local visual verification screenshots /.verify /.observability-screens +/.playwright-screenshots # ad-hoc screenshots/logs dropped at repo root during manual testing /*.png /*.jpg diff --git a/app/login/verified/page.tsx b/app/login/verified/page.tsx new file mode 100644 index 0000000..ba8d345 --- /dev/null +++ b/app/login/verified/page.tsx @@ -0,0 +1,22 @@ +import { redirect } from "next/navigation"; + +import { getSessionFromHeaders } from "@/lib/auth/queries"; + +import { VerifiedView } from "./verified-view"; + +// Better Auth's email verification lands here via callbackURL=/login/verified +// (rewritten from the default "/" in lib/auth/config.ts). The token is +// consumed server-side by Better Auth and never forwarded in the 302, so +// `searchParams` has no verification signal we can trust — the only reliable +// signal is `session` (autoSignInAfterVerification is on, see +// lib/auth/config.ts). Anyone landing here without a session is a random +// visitor / bookmark, not a verified user, and gets redirected to /login. +export default async function VerifiedPage() { + const session = await getSessionFromHeaders(); + + if (!session) { + redirect("/login"); + } + + return ; +} diff --git a/app/login/verified/verified-view.tsx b/app/login/verified/verified-view.tsx new file mode 100644 index 0000000..f6e7255 --- /dev/null +++ b/app/login/verified/verified-view.tsx @@ -0,0 +1,60 @@ +"use client"; + +import { CircleCheckIcon, MessagesSquareIcon } from "lucide-react"; +import Link from "next/link"; +import { useRouter } from "next/navigation"; +import { useEffect, useState } from "react"; + +import { Button } from "@/components/ui/button"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { FieldDescription } from "@/components/ui/field"; + +const REDIRECT_SECONDS = 5; + +// The server page at app/login/verified/page.tsx redirects to /login +// whenever there's no session, so by the time this component renders the +// user is verified and signed in. No `hasSession` prop needed — the page +// is the gate. +export function VerifiedView() { + const router = useRouter(); + const [remaining, setRemaining] = useState(REDIRECT_SECONDS); + + useEffect(() => { + if (remaining <= 0) { + router.replace("/chat"); + return; + } + const t = setTimeout(() => setRemaining((s) => s - 1), 1000); + return () => clearTimeout(t); + }, [remaining, router]); + + return ( +
+ + + + Email verified + + + +
+ + Your email is confirmed. You're signed in. + + + + {remaining > 0 ? `Redirecting in ${remaining}s…` : "Redirecting…"} + + + +
+ + +
+ ); +} diff --git a/docs/AUTH.md b/docs/AUTH.md index 11148f8..743567f 100644 --- a/docs/AUTH.md +++ b/docs/AUTH.md @@ -4,12 +4,13 @@ The chat app is gated behind a user account. Registration and sign-in go through ## Routes -| Path | What it does | -| ---------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/login` | Auth UI (sign-in by default; sub-paths `/login/sign-up`, `/login/forgot-password` render the corresponding views). Rendered by `app/login/[[...path]]` from `@daveyplate/better-auth-ui`. | -| `/api/auth/verify-email?token=...` | The verification email links here — Better Auth consumes the token at the API level and 302s to the configured `callbackURL` (default `/`). | -| `/chat` | The actual chat UI. Server component checks the session and redirects unauthenticated requests to `/login`. | -| `/api/auth/*` | Better Auth's catch-all — see `docs/APIS.md` for the endpoint list. | +| Path | What it does | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `/login` | Auth UI (sign-in by default; sub-paths `/login/sign-up`, `/login/forgot-password`, `/login/verify-email` render the corresponding views). Rendered by `app/login/[[...path]]`. | +| `/login/verified` | Post-verification success page. Better Auth consumes the token at `/api/auth/verify-email` and 302s here (the default `callbackURL="/"` is rewritten to `/login/verified` in `lib/auth/config.ts`). `autoSignInAfterVerification: true` is on — a verified user lands here with a real session and the CTA goes straight to `/chat`. Direct visits without a session redirect to `/login` immediately. | +| `/api/auth/verify-email?token=...` | The verification email links here — Better Auth consumes the token at the API level and 302s to the configured `callbackURL` (rewritten to `/login/verified`). | +| `/chat` | The actual chat UI. Server component checks the session and redirects unauthenticated requests to `/login`. | +| `/api/auth/*` | Better Auth's catch-all — see `docs/APIS.md` for the endpoint list. | ## Local dev setup diff --git a/lib/auth/config.ts b/lib/auth/config.ts index f580a00..dd30ee0 100644 --- a/lib/auth/config.ts +++ b/lib/auth/config.ts @@ -69,8 +69,20 @@ export const auth = }, }, emailVerification: { + // ponytail: Better Auth 1.6.x defaults this to falsy — verified users + // land on /login/verified without a session cookie AND without a token + // query param (Better Auth's 302 carries just the callbackURL value). + // Our success page then has no signal to render against and the + // !token && !session fallback bounces them back to /login. Flip this + // on so the verified user gets a real session and the success page is + // reachable. Email link is the auth token either way, so this doesn't + // widen the threat model meaningfully. + autoSignInAfterVerification: true, sendVerificationEmail: async ({ user, url }) => { - const result = await sendVerificationEmail({ to: user.email, url }); + const result = await sendVerificationEmail({ + to: user.email, + url: verificationRedirectUrl(url), + }); if (!result.ok) { // Translate our internal codes to the Better Auth error shape so // the route can return the FR-025 EMAIL_QUOTA_EXCEEDED contract. @@ -87,3 +99,26 @@ export const auth = if (process.env.NODE_ENV !== "production") globalThis.__auth = auth; export type Session = typeof auth.$Infer.Session; + +/** + * Re-point Better Auth's verification-link `callbackURL` at our success page. + * + * Better Auth constructs the verification link as + * `${baseURL}/api/auth/verify-email?token=...&callbackURL=...` (the + * `/api/auth` prefix is Better Auth's default `basePath`; the catch-all + * route at `app/api/auth/[...all]/route.ts` just mounts `auth.handler` + * at that path via `toNextJsHandler` and doesn't add the prefix itself). + * After the user clicks the link, Better Auth consumes the token at that + * endpoint and 302s to the `callbackURL`. The default callbackURL is `/`, + * which silently drops the user on the landing page with no feedback that + * verification succeeded. + * + * We only touch the `callbackURL` query param — the verification endpoint + * path MUST stay intact, otherwise the token is never consumed and the + * user is never verified. + */ +export function verificationRedirectUrl(rawUrl: string): string { + const u = new URL(rawUrl); + u.searchParams.set("callbackURL", "/login/verified"); + return u.toString(); +} diff --git a/tests/api/auth/verify-email.test.ts b/tests/api/auth/verify-email.test.ts new file mode 100644 index 0000000..fd1b0c8 --- /dev/null +++ b/tests/api/auth/verify-email.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect } from "vitest"; +import { verificationRedirectUrl } from "@/lib/auth/config"; + +// Better Auth constructs the verification link as +// `${baseURL}/api/auth/verify-email?token=...&callbackURL=...` (the +// `/api/auth` prefix is Better Auth's default `basePath`; the catch-all +// route at `app/api/auth/[...all]/route.ts` mounts the handler there). +// After the user clicks the link, Better Auth consumes the token at that +// endpoint and 302s to the callbackURL. Default callbackURL is `/` — +// silently bounces the user. We only want to swap the callbackURL value; +// the path must stay /api/auth/verify-email or the token never gets +// consumed and verification never happens. +describe("verificationRedirectUrl", () => { + it("rewrites callbackURL=/ to /login/verified", () => { + const out = verificationRedirectUrl( + "http://localhost:3000/api/auth/verify-email?token=abc&callbackURL=%2F", + ); + const u = new URL(out); + expect(u.searchParams.get("callbackURL")).toBe("/login/verified"); + }); + + it("rewrites callbackURL=/login/sign-in to /login/verified", () => { + const out = verificationRedirectUrl( + "http://localhost:3000/api/auth/verify-email?token=abc&callbackURL=%2Flogin%2Fsign-in", + ); + const u = new URL(out); + expect(u.searchParams.get("callbackURL")).toBe("/login/verified"); + }); + + it("adds callbackURL when missing", () => { + const out = verificationRedirectUrl("http://localhost:3000/api/auth/verify-email?token=abc"); + const u = new URL(out); + expect(u.searchParams.get("callbackURL")).toBe("/login/verified"); + }); + + it("preserves the verification endpoint path", () => { + const out = verificationRedirectUrl( + "http://localhost:3000/api/auth/verify-email?token=abc&callbackURL=%2F", + ); + const u = new URL(out); + // CRITICAL: replacing the path would skip verification — the token + // would never be consumed and the user would land on a success page + // for an unverified email. + expect(u.pathname).toBe("/api/auth/verify-email"); + }); + + it("preserves the token query param", () => { + const out = verificationRedirectUrl( + "http://localhost:3000/api/auth/verify-email?token=long-token-string&callbackURL=%2F", + ); + const u = new URL(out); + expect(u.searchParams.get("token")).toBe("long-token-string"); + }); + + it("URL-encodes the new callbackURL value", () => { + const out = verificationRedirectUrl( + "http://localhost:3000/api/auth/verify-email?token=abc&callbackURL=%2F", + ); + // /login/verified → %2Flogin%2Fverified in the raw query string. + expect(out).toContain("callbackURL=%2Flogin%2Fverified"); + }); +}); diff --git a/tests/frontend/login/verified-view.test.tsx b/tests/frontend/login/verified-view.test.tsx new file mode 100644 index 0000000..22ee7eb --- /dev/null +++ b/tests/frontend/login/verified-view.test.tsx @@ -0,0 +1,65 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup, act } from "@testing-library/react"; +import "@testing-library/jest-dom/vitest"; + +const replace = vi.fn(); +vi.mock("next/navigation", () => ({ + useRouter: () => ({ replace }), +})); + +import { VerifiedView } from "@/app/login/verified/verified-view"; + +// Mirrors the better-auth-ui card pattern: same Card + CardHeader + +// CardTitle + CardContent primitives, same outer `bg-muted/30` shell, same +// FieldDescription style for secondary text. The page itself is a server +// component (tested via Playwright); these tests pin the visible surface of +// the client view so a refactor that drops the success message or rewrites +// the manual link target surfaces here. + +describe("VerifiedView", () => { + afterEach(() => { + cleanup(); + replace.mockReset(); + vi.useRealTimers(); + }); + + it("renders the success heading, signed-in copy, and chat-bound CTA", () => { + render(); + + expect(screen.getByText(/email verified/i)).toBeInTheDocument(); + expect(screen.getByText(/you're signed in/i)).toBeInTheDocument(); + expect(screen.getByRole("link", { name: /chat now/i })).toHaveAttribute("href", "/chat"); + expect(screen.getByText(/redirecting in 5s/i)).toBeInTheDocument(); + }); + + it("renders the success icon", () => { + const { container } = render(); + expect(container.querySelector("svg")).toBeInTheDocument(); + }); + + it("redirects via router.replace to /chat at the 5-second mark", async () => { + vi.useFakeTimers(); + render(); + + // Initial render schedules setTimeout — replace must not fire yet. + expect(replace).not.toHaveBeenCalled(); + + // Advance in 1s ticks. Each tick fires the setRemaining callback, then + // we await act() so React re-renders and re-runs the effect with the + // new remaining value (the effect's dep is `[remaining, router]`). + // Without draining React between advances, all 5 timers fire against + // stale `remaining=5` state and the final router.replace never triggers. + // + // 5 ticks exactly — the 5th decrements remaining to 0 and the effect's + // `if (remaining <= 0)` branch fires router.replace. Pinning this + // count catches a regression where the redirect drifts to 6s. + for (let i = 0; i < 5; i++) { + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + }); + } + + expect(replace).toHaveBeenCalledTimes(1); + expect(replace).toHaveBeenCalledWith("/chat"); + }); +});