diff --git a/.env.example b/.env.example index 89b3a84..81ec48b 100644 --- a/.env.example +++ b/.env.example @@ -38,10 +38,14 @@ VERCEL_ENV= STRIPE_SECRET_KEY= STRIPE_WEBHOOK_SECRET= -# Email (spec 0011). `log` sends nothing and logs email.sent instead (local, tests, CI); -# `resend` sends through Resend and needs RESEND_API_KEY (a sending only key, re_...). -# The server refuses `log` when VERCEL_ENV is production. -EMAIL_TRANSPORT=log +# Email (spec 0011). `log` sends nothing and logs email.sent instead (unit tests); +# `mailpit` (spec 0012) hands every email to the local stack's Mailpit, so you can read it at +# MAILPIT_URL (local dev, e2e, CI); `resend` sends through Resend and needs RESEND_API_KEY +# (a sending only key, re_...). The server refuses anything but `resend` when VERCEL_ENV is +# production. +EMAIL_TRANSPORT=mailpit +# Mailpit's base URL, required with `mailpit`. Locally: http://127.0.0.1:55324 +MAILPIT_URL=http://127.0.0.1:55324 RESEND_API_KEY= # Sender and reply mailbox, `Name
` or a bare address, on a domain verified in Resend. # Replies land here, so it must be read. @@ -49,3 +53,11 @@ EMAIL_FROM=BeStore
diff --git a/app/api/auth/send-email/route.ts b/app/api/auth/send-email/route.ts
new file mode 100644
index 0000000..099238a
--- /dev/null
+++ b/app/api/auth/send-email/route.ts
@@ -0,0 +1,10 @@
+import { connection } from "next/server";
+
+import { handleSendEmailHook } from "@/features/account/send-email-hook";
+
+// Supabase's Send Email Hook (spec 0012, AC-8): every auth email goes out from here.
+export async function POST(request: Request) {
+ // Request time only: never prerendered or cached.
+ await connection();
+ return handleSendEmailHook(request);
+}
diff --git a/docs/reviews/2026-10-15-feat-customer-accounts.md b/docs/reviews/2026-10-15-feat-customer-accounts.md
new file mode 100644
index 0000000..cf2c652
--- /dev/null
+++ b/docs/reviews/2026-10-15-feat-customer-accounts.md
@@ -0,0 +1,48 @@
+# Review, feat/customer-accounts, 2026-10-15
+
+**Reviewed by**: Sonnet 5.5 (author on a different model per the check skill)
+**Scope**: 162 files, branch vs main
+**Verdict**: Changes requested
+
+## Summary
+This branch adds optional customer accounts on the shared Supabase user pool, a signed Send Email Hook route, an address book, cart merge, signed in checkout and account deletion. The code is careful and consistent: queries filter by the session customer id, the hook verifies the signature before anything else, and sign up and reset never reveal whether an account exists. The headline issue is that the "choose your password after confirming" defence has a hole. A password planted by an attacker survives if the real owner confirms the email and then leaves without choosing a password.
+
+## Major
+### 🟠A planted password survives when the owner confirms and does not set one, `src/features/account/actions/confirm.ts:34`
+**Problem**: Supabase sign up is public (`enable_signup = true`), so anyone can call it directly with the victim's email and a password they know. Our hook then mails the victim a genuine "Confirm your email" link. The design says the owner's new password overwrites the planted one, but AC-3 also lets the owner leave the set password page. After `verifyOtp` the user is confirmed and signed in, and the attacker's password is still valid.
+**Why it matters**: The attacker signs in with the planted password. Account setup then links every guest order for that email, so the attacker sees the victim's order history, names, phones and delivery addresses. The throwaway password in `sendSignUp` only protects sign ups made through our own form, not direct API calls.
+**Suggested fix**: When a `signup` token is spent, immediately replace the password with a fresh throwaway one (service role `updateUserById`, or `updateUser` in the new session) before redirecting, so only a password chosen after the confirm can ever exist. Alternatively keep the account unusable (not linked, no orders shown) until a password has been chosen, tracked by a flag in user metadata. Add a db or action test for "confirm, abandon, old password no longer signs in".
+
+## Minor
+### 🟡 Signed out visitors can use a customer's cart through a leftover cookie, `src/features/cart/actions.ts:39`
+**Problem**: `hasLiveCart` only checks that the cookie cart exists and has not expired, not who owns it. If a customer's session ends without our sign out (expiry, sign out from another device, an admin sign out in the same browser), the cart cookie still names their account cart. A guest on that browser then reads and adds to that customer's cart.
+**Why it matters**: Cart contents leak and get changed on shared computers. Money and addresses are not exposed, so this is not a blocker.
+**Suggested fix**: In the cart read and add paths treat a cookie cart that has a `customer_id` as foreign when the visitor has no matching session, and start a guest cart instead. Or clear the cart cookie in the proxy when there is no session on account pages.
+
+### 🟡 Merging a guest cart can orphan an open Stripe session, `src/lib/cart/merge.ts:79`
+**Problem**: The merge deletes the guest cart, which sets `cart_id` null on any pending order. `clearPendingOrder` in `startCheckout` then looks up pending orders by the account cart id, so it never finds the old one and its Stripe session stays open.
+**Why it matters**: A guest who started checkout, went back, signed in and paid again can end up with two payable sessions for the same goods. The spec accepts the null `cart_id`, but not the duplicate payment path.
+**Suggested fix**: Before deleting the guest cart, expire its pending order's session (the same way `clearPendingOrder` does), or refuse the merge while a pending order exists.
+
+### 🟡 Set password accepts any session without the current password, `src/features/account/actions/set-password.ts:31`
+**Problem**: Anyone holding a valid customer session can change the password here with no re-check. The change password form does ask for the current one, so the two are inconsistent.
+**Why it matters**: A stolen or left open session becomes a permanent account takeover. It is the documented design (the page must work right after a confirm link), so it is minor.
+**Suggested fix**: Only allow this page for a session that came from `verifyOtp` in the last few minutes (for example a short lived marker cookie set by `confirmEmail`), and send everyone else to change password.
+
+### 🟡 Account deletion holds a database transaction open across a network call, `src/features/account/actions/settings.ts:113`
+**Problem**: The advisory lock and an open Prisma transaction wait on `auth.admin.deleteUser` for up to 15 seconds.
+**Why it matters**: Slow Auth responses tie up pooled connections. Rare, because it only runs on deletion.
+**Suggested fix**: Acceptable for now. Note it in the runbook, and keep the 15 second cap.
+
+## Nits
+- ⚪ `src/features/admin-auth/actions/mfa.ts:10`, the moved imports (`@/lib/auth/errors`, `@/lib/form-errors`, `@/lib/supabase/session`) sit among the relative imports; group them with the other `@/` imports (same in `password-reset.ts` and `sign-in.ts`).
+- ⚪ `src/features/account/actions/sign-up-from-order.ts:17`, the eligibility lookup runs before `after()`, so its reply differs slightly from a non eligible one. Harmless because it says nothing about accounts.
+
+## Strengths
+- The Send Email Hook checks the Standard Webhooks signature over the raw body first, refuses every action type except signup and recovery, picks the admin template from the database (not from `redirect_to`), and uses a hashed idempotency key.
+- Order, address and profile reads and writes always filter by the session customer id; an order that is missing, foreign or hidden gives the same not found.
+- The `getUser()` choice, the per customer advisory lock, the confirm link that only a POST spends, and safe `next` handling are well reasoned and carry why comments.
+- Logs carry only ids, counts and reason codes, backed by a redaction test.
+
+## Test coverage
+Good. Vitest covers the hook (signature, types, template choice), the actions, paths and `next` safety, password errors, proxy decisions, log redaction and env rules. The db project covers setup, order linking, address rules and limits, settings, deletion and sign up from an order. Playwright covers sign up, sign in, reset, settings, addresses, orders, checkout and axe on each page. The gap is the abandoned confirm scenario above (a planted password still works), and no test covers a signed out visitor with a leftover account cart cookie.
diff --git a/docs/runbooks/admin-accounts.md b/docs/runbooks/admin-accounts.md
index 0ec521d..08078a3 100644
--- a/docs/runbooks/admin-accounts.md
+++ b/docs/runbooks/admin-accounts.md
@@ -10,21 +10,13 @@ The production auth settings live in the Supabase dashboard, not in code, so the
### Supabase project: Authentication settings
-- **Sign ups**: turn off "Allow new users to sign up". Keep the Email provider itself on (turning it off also blocks password sign in).
-- **Passwords**: minimum length 8, required characters "lowercase, uppercase, digits and symbols". `passwordSchema` in `src/features/admin-auth/schemas.ts` mirrors this; keep the two in step.
+- **Sign ups**: on since customer accounts (spec 0012); see [customer-accounts.md](customer-accounts.md). Signing up never makes an admin: only `admin:create` adds an `admin_users` row. Keep the Email provider itself on (turning it off also blocks password sign in).
+- **Passwords**: minimum length 8, required characters "lowercase, uppercase, digits and symbols". `passwordSchema` in `src/lib/auth/password.ts` mirrors this; keep the two in step.
- **MFA**: TOTP enroll and verify both on.
- **Site URL**: the production domain, the same value as `NEXT_PUBLIC_SITE_URL` (for example `https://shop.example.com`). Reset links always point here, so a reset started from a preview deploy lands on production. Test password reset locally, not on previews.
- **Redirect URLs**: `https:// You can save up to {MAX_SAVED_ADDRESSES} addresses.
+ No saved addresses yet. Add one here, or tick "Save this address" at checkout.
+
+ Paid {formatDate(order.paidAt, format)} · {orderStatusLabels[order.status]}
+ {line.productName} {line.variantLabel}
+ {line.quantity} ×{" "}
+
+ {line}
+ Refunded
+
+ We send a link to the email on this order. Confirm it, choose a password, and this order and
+ your next ones show up in your account.
+
+ This deletes your profile, saved addresses and cart, and you can no longer sign in.
+ {accountMessages.checkEmailBody}
- {statusLabels[order.status]}
+ {orderStatusLabels[order.status]}
diff --git a/src/features/checkout/components/checkout-form.tsx b/src/features/checkout/components/checkout-form.tsx
index 4ed11f1..2f7b907 100644
--- a/src/features/checkout/components/checkout-form.tsx
+++ b/src/features/checkout/components/checkout-form.tsx
@@ -5,12 +5,14 @@ import { LockIcon } from "lucide-react";
import { useRouter } from "next/navigation";
import { useEffect, useMemo, useState, useSyncExternalStore, useTransition } from "react";
import {
+ Controller,
type FieldError as FormFieldError,
useForm,
type UseFormRegisterReturn,
} from "react-hook-form";
import { FormNotice } from "@/components/form-notice";
+import { Checkbox } from "@/components/ui/checkbox";
import { useStoreFormat } from "@/components/store-format-provider";
import { Button } from "@/components/ui/button";
import {
@@ -30,6 +32,21 @@ import type { StoreCountry } from "@/lib/shipping/address";
import { startCheckout, type StartCheckoutError } from "../actions/start-checkout";
import { type CheckoutPrefill, checkoutFields, checkoutSchema } from "../schemas";
+// A saved address the signed in customer can swap in (spec 0012, AC-15).
+export type SavedAddressChoice = {
+ readonly id: string;
+ readonly label: string;
+ readonly values: Omit
+ Have an account?{" "}
+
+ Sign in
+
+ ` with alt text; the secret is shown as text in groups of four. It refuses when the session `amr` contains `recovery` or `otp`.
@@ -269,7 +269,7 @@ Tracer Bullet: task 1 threads one real path (config, clients, proxy, script, act
- [ ] `/sync`: record `otpauth` agent skills as declined in root `AGENTS.md` (none credible found; offered on 2026-09-25).
- [ ] `/sync`: once built, add an `src/features/admin-auth/AGENTS.md` (or root `## Rules` line) noting that `requireAdmin()` also enforces `aal2` and the 12 hour cap, so later admin features do not add their own checks.
- [ ] Spec 0001 Follow-up "Feature 5 decides where the admin role lives and the shape of `requireAdmin()`" is settled here; spec 0002 Follow-ups on `requireAdmin()`, the first admin row, and "one auth user may hold both rows" (confirmed: allowed) are settled here.
-- [ ] Feature 12 (customer accounts): turn public sign up back on, widen the proxy matcher to account routes, and give customer password reset its own template path (the recovery template is shared, so the `next` in `redirectTo` must decide where the link lands).
+- [x] Feature 12 (customer accounts), done in spec 0012: turn public sign up back on, widen the proxy matcher to account routes, and give customer password reset its own template path (the recovery template is shared, so the `next` in `redirectTo` must decide where the link lands).
- [ ] Feature 19 (Production deploy): apply the runbook to production (Supabase auth settings, Resend SMTP, recovery template, redirect URLs, JWT keys), turn on the Vercel Firewall rule and check AC-15, then create the first real admin. Added to the scope on 2026-09-25.
- [ ] Feature 11 (order emails) reuses the Resend domain verified here for Supabase SMTP.
- [ ] If an admin management screen is ever wanted, it needs its own spec (invites, roles, audit of admin changes).
diff --git a/docs/specs/0011-order-emails/index.md b/docs/specs/0011-order-emails/index.md
index 682a444..dfaf2a2 100644
--- a/docs/specs/0011-order-emails/index.md
+++ b/docs/specs/0011-order-emails/index.md
@@ -1,6 +1,6 @@
# 0011. Order emails
-**Date**: 2026-10-13 (amended 2026-10-14 after review: AC-7, AC-9, AC-17)
+**Date**: 2026-10-13 (amended 2026-10-14 after review: AC-7, AC-9, AC-17; amended 2026-10-15 by [spec 0012](../0012-customer-accounts/index.md) AC-23: the `mailpit` transport)
**Status**: Accepted
## Summary
@@ -41,7 +41,7 @@ Customers get three kinds of email: a confirmation receipt when an order is paid
- **AC-15**: `sendEmailAgain` calls `requireAdmin()` itself. A visitor without an active admin at `aal2` gets the spec 0004 behaviour and nothing changes. The row must belong to the order named in the input, else `not_found`.
*Environments, logs, accessibility*
-- **AC-16**: `EMAIL_TRANSPORT` picks `log` or `resend`. With `log`, nothing leaves the machine: delivery runs the same claim, render and status steps, marks the row `sent` with `resend_id` null, and logs `email.sent` with `transport: "log"`. The env schema refuses `log` when `VERCEL_ENV` is `production`, and requires `RESEND_API_KEY` when the transport is `resend`. The Vitest setup, the Playwright web server env and the CI workflow all set `EMAIL_TRANSPORT=log`. DB tests replace `scheduleDelivery` (no request scope for `after()`); e2e runs the real one under `next dev`. `pnpm email:dev` previews every template with fixture props.
+- **AC-16**: `EMAIL_TRANSPORT` picks `log` or `resend`. With `log`, nothing leaves the machine: delivery runs the same claim, render and status steps, marks the row `sent` with `resend_id` null, and logs `email.sent` with `transport: "log"`. The env schema refuses `log` when `VERCEL_ENV` is `production`, and requires `RESEND_API_KEY` when the transport is `resend`. The Vitest setup, the Playwright web server env and the CI workflow all set `EMAIL_TRANSPORT=log`. (Amended by spec 0012 AC-23: a third value, `mailpit`, posts every email to the local Mailpit at `MAILPIT_URL`. Playwright, CI and local dev now use it, Vitest stays on `log`, and the env schema accepts only `resend` when `VERCEL_ENV` is `production`.) DB tests replace `scheduleDelivery` (no request scope for `after()`); e2e runs the real one under `next dev`. `pnpm email:dev` previews every template with fixture props.
- **AC-17**: Logs carry the email row id, kind, order id and number, refund id, attempts and the Resend error name. They never carry a recipient address, subject, body, name, delivery address or tracking number. Events: `email.queued`, `email.sent`, `email.retry` (warn), `email.failed` (warn), `email.gave_up` (error), `email.idempotent_conflict` (warn), `email.config_error` (error), `email.resend_requested`, `cron.email_sweep` (counts), `email.delivery_error` (error: a try that threw, such as a database error or a template bug; it carries the error name and SQLSTATE, and only the row id when the throw came before the row was read), and `order.email_missing` (reused).
- **AC-18**: The Emails section and the Send again dialog work by keyboard alone, move focus into the dialog and back out, and pass axe with zero violations (desktop and phone, same tags as spec 0003). Each email uses one column, real headings, layout tables marked `role="presentation"`, the line items as a data table with header cells, and text contrast of at least 4.5:1.
@@ -155,7 +155,7 @@ Dedupe keys keep spec 0002's format (`order_confirmation:
+ {addresses.map((address) => {
+ const label = shortLabel(address);
+ const needsCheck = addressNeedsCheck(address, country);
+ return (
+
+ )}
+
+
+ {title}
+ {description ? (
+ Order #{order.number}
+
+ Items
+
+
+ {order.lines.map((line, index) => (
+
+
+ Delivery address
+
+
+ Tracking
+
+ {tracking.map((line) => (
+
+ Summary
+
+
+ Track this order in an account
+
+
+ {orders.map((order) => (
+
+ )}
+ {page > 1 || hasNext ? (
+
+ ) : null}
+
+ Your details
+
+
+
+
+ Password
+
+
+ Delete account
+
+
+ {accountMessages.checkEmailTitle}
+
+ .
diff --git a/src/features/admin-auth/actions/password-reset.ts b/src/features/admin-auth/actions/password-reset.ts
index f4a0e48..15756fd 100644
--- a/src/features/admin-auth/actions/password-reset.ts
+++ b/src/features/admin-auth/actions/password-reset.ts
@@ -10,12 +10,12 @@ import { logger } from "@/lib/logger";
import type { ActionResult } from "@/lib/result";
import { createSupabaseServerClient, createSupabaseStatelessClient } from "@/lib/supabase/server";
-import { classifyAuthError } from "../auth-errors";
+import { classifyAuthError } from "@/lib/auth/errors";
import { logAuthEvent, requestIp } from "../log";
import { requireAdminSession } from "../require-admin";
import { mfaPath, signInPath } from "../safe-admin-path";
import { forgotPasswordSchema, resetPasswordSchema } from "../schemas";
-import { fieldErrors } from "../session";
+import { fieldErrors } from "@/lib/form-errors";
export type RequestResetError = {
readonly fields?: Partial
Thank you for your order
Hi Ada,
", + Text: "Hi Ada,", + }); + expect(init.signal).toBeInstanceOf(AbortSignal); + }); + + it("answers a failure with Mailpit's status", async () => { + fetchMock.mockResolvedValue(new Response("nope", { status: 500 })); + + expect(await currentTransport().send(email)).toEqual({ + ok: false, + failure: { name: "mailpit_error", statusCode: 500 }, + }); + }); + + it("answers a network error as a failure with no status", async () => { + fetchMock.mockRejectedValue(new TypeError("fetch failed")); + + expect(await currentTransport().send(email)).toEqual({ + ok: false, + failure: { name: "TypeError", statusCode: null }, + }); + }); +}); diff --git a/src/lib/email/transport.ts b/src/lib/email/transport.ts index 786a337..130cd6c 100644 --- a/src/lib/email/transport.ts +++ b/src/lib/email/transport.ts @@ -6,9 +6,10 @@ import { env } from "@/lib/env"; import type { SendFailure } from "./classify"; -// The two ways an email leaves (spec 0011, AC-16): `resend` sends it, `log` sends nothing so -// local runs, tests and CI never reach a customer. Open and click tracking are domain settings in -// Resend and stay off there (AC-7). +// The ways an email leaves (spec 0011, AC-16): `resend` sends it, `log` sends nothing so local +// runs, tests and CI never reach a customer. `mailpit` (spec 0012, AC-23) hands it to the local +// stack's Mailpit, so e2e can read it; the env schema refuses it in production. Open and click +// tracking are domain settings in Resend and stay off there (AC-7). export type OutgoingEmail = { readonly to: string; @@ -23,9 +24,15 @@ export type SendResult = | { readonly ok: true; readonly resendId: string | null } | { readonly ok: false; readonly failure: SendFailure }; +export type TransportName = "log" | "resend" | "mailpit"; + export type Transport = { - readonly name: "log" | "resend"; - readonly send: (email: OutgoingEmail) => Promise- Someone asked to reset the password for this admin account. If it was you, open the link below and - follow the steps. You will need your authenticator app. -
- -- The link works once and expires in one hour. If you did not ask for this, you can ignore this - email; your password stays the same. -
diff --git a/tests/db/account-addresses.db.test.ts b/tests/db/account-addresses.db.test.ts new file mode 100644 index 0000000..4b985c3 --- /dev/null +++ b/tests/db/account-addresses.db.test.ts @@ -0,0 +1,176 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { resetDatabaseBeforeEach, testDb } from "./client"; +import { createCustomer } from "./fixtures"; + +// The address book (spec 0012, AC-14) against a real Postgres, through its server actions. + +const mocks = vi.hoisted(() => ({ + customer: { id: "", email: "" }, + refresh: vi.fn(), + info: vi.fn(), +})); + +vi.mock("server-only", () => ({})); +vi.mock("@/lib/db", async () => ({ db: (await import("./client")).testDb })); +vi.mock("@/lib/env", () => ({ env: { STORE_COUNTRY: "PL" } })); +vi.mock("next/cache", () => ({ refresh: mocks.refresh })); +vi.mock("@/lib/logger", () => ({ logger: { info: mocks.info, warn: vi.fn(), error: vi.fn() } })); +vi.mock("@/lib/customer/session", () => ({ requireCustomer: async () => mocks.customer })); + +const { createAddress, deleteAddress, setDefaultAddress, updateAddress } = + await import("@/features/account/actions/addresses"); + +resetDatabaseBeforeEach(); + +beforeEach(async () => { + vi.clearAllMocks(); + const customer = await createCustomer(); + mocks.customer = { id: customer.id, email: customer.email }; +}); + +function typed(overrides: Record