OPT and Improved Installation Wizzard
CI / ci (push) Canceled after 0s

1. Summary of changes
Password auth → email OTP, everywhere it existed for the tenant/business app: sign-in, customer self-registration (/register), staff-invitation acceptance, and the installer's administrator step. Flow is always: email → 6-digit code emailed → code entered → session created, via one shared service (otp.service.ts) reused by every entry point.
NextChair Manager's own auth was not touched — this only affects tenant-side (business admin/staff/customer) authentication, per the spec's explicit instruction.
Installer improvements: Step 2 (Administrator) now asks only Name + Email + optional Phone, verified inline via the same OTP widget (no redirect). Step 3 (Business Details) pre-fills Business Name from the Manager-provisioned BUSINESS_NAME, shows the booking URL (NEXT_PUBLIC_APP_URL) as a read-only info box instead of asking for it, and has a distinctly-labeled "Business Website (optional)" field with copy clarifying it's not the booking URL.
Installation cannot complete with an unverified admin email — enforced server-side (finalizeOtp runs inside the same DB transaction as user/business creation), not just trusted from client state. A mid-wizard refresh still can't create duplicate admins or partial records — that property was preserved by design, not incidentally.
2. Prisma/database changes
New EmailOtp model (email as PK — one row per address, so a new request naturally invalidates the previous code): codeHash, purpose, expiresAt, attempts, verifiedAt, consumedAt, timestamps. Only a bcrypt hash of the code is ever stored.
User.passwordHash — unchanged schema (already nullable), just annotated as deprecated. No destructive migration; historical rows are untouched.
Migration: prisma/migrations/20260905190022_email_otp_authentication — adds EmailOtp only. Generated and applied against throwaway containers only; the real next-chair-mysql production database was never touched (confirmed not even running on this host during this session).
3. New environment variables
None required for production. One new variable, E2E_TEST_MODE, exists purely for the Playwright suite (set only in playwright.config.ts's webServer.env) — it unlocks a /api/test/otp-code debug endpoint and bypasses OTP rate limiting so the same seeded users can sign in dozens of times across one test run. It must never be set on a real deployment; nothing in the app sets it anywhere else.

4. Email delivery configuration
Nothing new to configure. OTP emails reuse the existing EmailProvider abstraction and EMAIL_PROVIDER env var exactly as configured today — console in dev (prints to server logs), brevo in production via the same SMTP credentials already documented. The OTP email is a new template (otp-email.ts) sent directly (bypassing the async notification/cron pipeline, since a 10-minute-expiry code can't wait for a delivery retry loop).

5. Migration considerations for existing users/installations
Existing users lose password sign-in — the only way to authenticate now is email OTP. There is no in-app fallback; this is an intentional, sharp cutover. If existing customers/staff/admins need advance notice, that's a business/communications decision outside this change.
No account data is deleted or migrated — passwordHash rows are simply never read for auth anymore.
prisma/seed.ts / scripts/ensure-super-admin.ts still set a passwordHash for seeded dev users — harmless and vestigial now, not cleaned up (flagging it rather than touching seed data silently).
Installations already completed are unaffected; only new installer runs go through the OTP admin-verification step.
6. Files changed
New:

src/lib/auth/otp.service.ts — the core requestOtp/verifyOtp/finalizeOtp service
src/lib/auth/__tests__/otp.service.integration.test.ts
src/services/notifications/templates/otp-email.ts
src/features/auth/components/otp-code-step.tsx — shared code-entry UI
src/features/auth/actions/request-sign-in-otp.ts
src/app/api/test/otp-code/route.ts — e2e-only, gated on E2E_TEST_MODE
prisma/migrations/20260905190022_email_otp_authentication/
Modified — auth core: prisma/schema.prisma, src/auth.ts, src/features/auth/schemas.ts

Modified — sign-in/register/invitations: sign-in-form.tsx, register.ts, register-form.tsx, accept-invitation.ts, accept-invitation-form.tsx, staff-invitation.service.ts

Modified — installer: installer.service.ts, schema.ts, install.ts, install-wizard.tsx, install/page.tsx, installer.service.integration.test.ts

Modified — email / e2e infra: console-email-provider.ts, playwright.config.ts, e2e/helpers.ts, and the four e2e/*.spec.ts files (dropped the now-unused password argument from every signIn() call).

7. Verification performed
npx tsc --noEmit, npm run lint, npm test (276 unit) — all clean.
npm run test:integration (207 tests, throwaway DB) — includes 11 new OTP-service tests and a rewritten installer suite; a genuine race condition in requestOtp (concurrent requests for a brand-new email double-hitting the upsert's create branch) was caught and fixed here.
Full npm run test:e2e against a fresh throwaway database: 36/37 passed. The one failure (owner-journeys "opening hours conflict" test) is a pre-existing, auth-unrelated ordering flake — appointment-count ambiguity from earlier tests' bookings — not something this change touches or introduced.
Live manual verification in-browser: existing seeded customer sign-in via OTP; new customer registration via OTP (session created, landed on Home); installer wizard end-to-end through Step 3, confirming email-verification gating, Business Name pre-population from BUSINESS_NAME, and the read-only Booking URL / distinct Business Website fields.
All throwaway Docker containers/DBs used for this work have been removed; the real production database was never touched.
This commit is contained in:
Stefan Lazic committed 2026-09-05 21:03:36 +01:00
1 parent ba46334179
commit 94aa1b2f26
29 files changed
+1434 -357

No files matched your search

@@ -0,0 +1,154 @@
import { describe, expect, it } from "vitest";
import { prisma } from "@/services/scheduling/__tests__/fixtures";
import { MockEmailProvider } from "@/services/notifications/providers/mock-email-provider";
import { finalizeOtp, requestOtp, verifyOtp } from "../otp.service";
/**
* Real MySQL, real upsert/race semantics — deliberately not a hand-rolled
* fake Prisma client, since the behaviour this file exists to prove
* (a second request invalidating the first, one-time consumption) is
* exactly the kind of thing a fake could get subtly wrong. See
* installer.service.integration.test.ts for the same reasoning applied
* to installApplication.
*/
function uniqueEmail(prefix: string): string {
return `otp-test-${prefix}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}@example.com`;
}
async function requestAndExtractCode(email: string, purpose: "login" | "register" | "install" | "staff-invite" = "login"): Promise<string> {
const mockProvider = new MockEmailProvider();
const result = await requestOtp(prisma, { email, purpose, emailProvider: mockProvider });
expect(result.ok).toBe(true);
const sent = mockProvider.sentEmails[0];
const code = /verification code is (\d{6})/.exec(sent?.text ?? "")?.[1];
if (!code) throw new Error("Test setup failed: couldn't extract the OTP code from the captured email.");
return code;
}
describe("otp.service (integration)", () => {
it("verifies a correctly-entered code", async () => {
const email = uniqueEmail("verify-ok");
const code = await requestAndExtractCode(email);
const result = await verifyOtp(prisma, email, code);
expect(result.ok).toBe(true);
});
it("rejects an incorrect code and records the attempt", async () => {
const email = uniqueEmail("wrong-code");
await requestAndExtractCode(email);
const result = await verifyOtp(prisma, email, "000000");
expect(result.ok).toBe(false);
expect(result.reason).toBe("invalid_code");
const row = await prisma.emailOtp.findUnique({ where: { email } });
expect(row?.attempts).toBe(1);
});
it("locks out further attempts once the attempt cap is exhausted", async () => {
const email = uniqueEmail("lockout");
await requestAndExtractCode(email);
for (let i = 0; i < 5; i += 1) {
await verifyOtp(prisma, email, "000000");
}
// Even the (unknown to this test) correct code can no longer succeed —
// the row's attempts counter, not the code's correctness, is what's
// gating this now.
const result = await verifyOtp(prisma, email, "000000");
expect(result.ok).toBe(false);
expect(result.reason).toBe("too_many_attempts");
});
it("rejects an expired code", async () => {
const email = uniqueEmail("expired");
const code = await requestAndExtractCode(email);
await prisma.emailOtp.update({ where: { email }, data: { expiresAt: new Date(Date.now() - 1000) } });
const result = await verifyOtp(prisma, email, code);
expect(result.ok).toBe(false);
expect(result.reason).toBe("expired");
});
it("rejects a code that's already been consumed (reuse)", async () => {
const email = uniqueEmail("reuse");
const code = await requestAndExtractCode(email);
const first = await verifyOtp(prisma, email, code);
expect(first.ok).toBe(true);
expect(await finalizeOtp(prisma, email)).toBe(true);
// Same code, after consumption — must fail, not succeed a second time.
const second = await verifyOtp(prisma, email, code);
expect(second.ok).toBe(false);
expect(second.reason).toBe("not_found");
});
it("a new request invalidates the previous code entirely", async () => {
const email = uniqueEmail("invalidate");
const firstCode = await requestAndExtractCode(email);
const secondCode = await requestAndExtractCode(email);
expect(secondCode).not.toBe(firstCode);
const oldCodeResult = await verifyOtp(prisma, email, firstCode);
expect(oldCodeResult.ok).toBe(false);
const newCodeResult = await verifyOtp(prisma, email, secondCode);
expect(newCodeResult.ok).toBe(true);
});
it("finalizeOtp enforces one-time use even when verifiedAt is fresh", async () => {
const email = uniqueEmail("finalize-once");
const code = await requestAndExtractCode(email);
expect((await verifyOtp(prisma, email, code)).ok).toBe(true);
expect(await finalizeOtp(prisma, email)).toBe(true);
// A second finalize on the same (now-consumed) row must fail — this is
// exactly what stands between the installer and a duplicate admin if
// its final submit were somehow retried after already succeeding once.
expect(await finalizeOtp(prisma, email)).toBe(false);
});
it("finalizeOtp refuses a row that was never verified", async () => {
const email = uniqueEmail("never-verified");
await requestAndExtractCode(email);
expect(await finalizeOtp(prisma, email)).toBe(false);
});
it("rate-limits repeated requests for the same email", async () => {
const email = uniqueEmail("rate-limit-request");
const mockProvider = new MockEmailProvider();
const results = await Promise.all(
Array.from({ length: 7 }, () => requestOtp(prisma, { email, purpose: "login", emailProvider: mockProvider })),
);
expect(results.some((r) => !r.ok)).toBe(true);
});
it("rate-limits repeated verify attempts for the same email", async () => {
const email = uniqueEmail("rate-limit-verify");
await requestAndExtractCode(email);
const results: boolean[] = [];
for (let i = 0; i < 12; i += 1) {
const result = await verifyOtp(prisma, email, "111111");
results.push(result.reason === "rate_limited");
}
expect(results.some(Boolean)).toBe(true);
});
it("onlyIfUserExists sends nothing for an email with no account, but still reports ok", async () => {
const email = uniqueEmail("no-such-user");
const mockProvider = new MockEmailProvider();
const result = await requestOtp(prisma, { email, purpose: "login", onlyIfUserExists: true, emailProvider: mockProvider });
expect(result.ok).toBe(true);
expect(mockProvider.sentEmails).toHaveLength(0);
const row = await prisma.emailOtp.findUnique({ where: { email } });
expect(row).toBeNull();
});
});
+237
View File
@@ -0,0 +1,237 @@
import { randomInt } from "node:crypto";
import bcrypt from "bcryptjs";
import type { Prisma, PrismaClient } from "@/generated/prisma/client";
import { checkRateLimit } from "@/lib/security/rate-limit";
// Type-only — the barrel this comes from has a `server-only` guard that
// throws under Vitest's plain Node runtime (see reminder-worker.ts's own
// established pattern: never call getEmailProvider() from inside a
// service function Vitest loads directly, only from the outermost
// Server Action/route caller, which passes it down explicitly).
import type { EmailProvider } from "@/services/notifications/providers";
import { renderOtpEmail } from "@/services/notifications/templates/otp-email";
/**
* The one place email OTPs (one-time passcodes) are generated, sent,
* verified, and consumed — reused identically by normal sign-in,
* registration, the installer's administrator step, and staff-invitation
* acceptance (see src/auth.ts's Credentials.authorize, the actual caller
* of verifyOtp/finalizeOtp; the four request call sites each just pick a
* `purpose` and whether an email should only be sent to an already-
* existing account). Bcrypt cost 12 throughout, matching this app's
* existing password-hashing convention (see scripts/ensure-super-admin.ts,
* src/auth.ts's old DUMMY_HASH) even though a 6-digit code's real defense
* is the rate limits and attempt cap below, not hash cost alone.
*
* Every function here takes its Prisma client as an explicit first
* parameter — the same convention as installer.service.ts/notification.
* service.ts's notifySafely — rather than importing the app's singleton
* internally, specifically so installer.service.ts can call finalizeOtp
* with its own `tx` and have the OTP consumption roll back together with
* the rest of installation if a later step fails.
*
* See prisma/schema.prisma's EmailOtp model doc comment for the
* verifiedAt/consumedAt two-phase design this is built around.
*/
type Db = PrismaClient | Prisma.TransactionClient;
const OTP_TTL_MS = 10 * 60 * 1000;
const VERIFIED_GRACE_MS = 30 * 60 * 1000;
const MAX_VERIFY_ATTEMPTS = 5;
const BCRYPT_COST = 12;
// The e2e suite signs in as the same handful of seeded users dozens of
// times across a single dev-server run — a real user never does that in
// 15 minutes, but the rate limiter can't tell the difference. E2E_TEST_MODE
// only exists on playwright.config.ts's own webServer (see that config and
// /api/test/otp-code's identical gate) and must never be set anywhere a
// real user could reach, so bypassing rate limiting under it doesn't weaken
// anything in production.
const rateLimitBypassed = process.env.E2E_TEST_MODE === "true";
// Compared against when no row exists, already consumed, or attempts are
// exhausted — so verifyOtp's response timing doesn't itself reveal which
// of those is true, mirroring src/auth.ts's own DUMMY_HASH pattern for
// the exact same reason (account/state enumeration via timing).
const DUMMY_HASH = bcrypt.hashSync("nextchair-otp-timing-safety-placeholder", BCRYPT_COST);
export type OtpPurpose = "login" | "register" | "install" | "staff-invite";
function normalizeEmail(email: string): string {
return email.trim().toLowerCase();
}
function generateCode(): string {
// randomInt's upper bound is exclusive — 0..999999 inclusive, zero-padded
// to always be exactly 6 digits (e.g. "004213").
return randomInt(0, 1_000_000).toString().padStart(6, "0");
}
export interface RequestOtpOptions {
email: string;
purpose: OtpPurpose;
/** From getRequestIp() — rate-limited alongside the per-email key, so a flood spread across many target emails from one source is also bounded. */
ip?: string;
/** Shown as the email's visible sender — the resolved business name where one exists, a neutral "NextChair" pre-install/pre-account. */
senderName?: string;
/**
* Login-specific: only actually generates/sends a code if a User already
* exists for this email. The caller still gets the same {ok:true} result
* either way — this is what keeps "do not reveal whether an email
* address exists" true for the sign-in page without silently creating a
* dead OTP row (or emailing a stranger) for a typo'd/unregistered address.
*/
onlyIfUserExists?: boolean;
/** Required, not defaulted internally — see this file's own import comment for why. Real callers pass getEmailProvider(); tests pass a MockEmailProvider. */
emailProvider: EmailProvider;
}
export interface RequestOtpResult {
ok: boolean;
retryAfterSeconds?: number;
}
export async function requestOtp(db: Db, options: RequestOtpOptions): Promise<RequestOtpResult> {
const email = normalizeEmail(options.email);
if (!rateLimitBypassed) {
const emailLimit = checkRateLimit(`otp-request:${email}`, { maxAttempts: 5, windowMs: 15 * 60 * 1000 });
if (!emailLimit.allowed) return { ok: false, retryAfterSeconds: emailLimit.retryAfterSeconds };
if (options.ip) {
const ipLimit = checkRateLimit(`otp-request-ip:${options.ip}`, { maxAttempts: 20, windowMs: 15 * 60 * 1000 });
if (!ipLimit.allowed) return { ok: false, retryAfterSeconds: ipLimit.retryAfterSeconds };
}
}
if (options.onlyIfUserExists) {
const user = await db.user.findUnique({ where: { email }, select: { id: true } });
// Same generic {ok:true} either way — see this option's own doc comment.
if (!user) return { ok: true };
}
// ~1% of requests — see cleanupExpiredOtps's own comment.
if (Math.random() < 0.01) void cleanupExpiredOtps(db);
const code = generateCode();
const codeHash = await bcrypt.hash(code, BCRYPT_COST);
const expiresAt = new Date(Date.now() + OTP_TTL_MS);
// Upsert, not insert — this IS the "a new OTP invalidates the previous
// one" requirement: there is only ever one row per email, so requesting
// again always replaces whatever code/attempts/verified state existed.
//
// Two concurrent requests for the same brand-new email (a double-
// clicked "send code" button, a client retry) can both reach this line
// finding no existing row and both attempt the `create` branch — only
// one wins, the other's upsert throws a unique-constraint violation
// (P2002) rather than transparently falling back to `update` the way a
// single atomic "INSERT ... ON DUPLICATE KEY UPDATE" would. Retrying
// once as a plain update resolves it: by the time this catch runs, the
// row the other request just created definitely exists.
try {
await db.emailOtp.upsert({
where: { email },
create: { email, codeHash, purpose: options.purpose, expiresAt },
update: { codeHash, purpose: options.purpose, expiresAt, attempts: 0, verifiedAt: null, consumedAt: null },
});
} catch (error) {
const isDuplicateKey =
typeof error === "object" && error !== null && "code" in error && (error as { code?: string }).code === "P2002";
if (!isDuplicateKey) throw error;
await db.emailOtp.update({
where: { email },
data: { codeHash, purpose: options.purpose, expiresAt, attempts: 0, verifiedAt: null, consumedAt: null },
});
}
const rendered = renderOtpEmail({ senderName: options.senderName ?? "NextChair", code, expiresInMinutes: 10 });
// Never logged — the code must not reach application logs, only this
// one email (see the brief's "do not include the OTP in logs").
await options.emailProvider.sendEmail({
to: { email },
sender: { name: options.senderName ?? "NextChair" },
subject: rendered.subject,
html: rendered.html,
text: rendered.text,
});
return { ok: true };
}
export type VerifyOtpFailureReason = "not_found" | "expired" | "too_many_attempts" | "invalid_code" | "rate_limited";
export interface VerifyOtpResult {
ok: boolean;
reason?: VerifyOtpFailureReason;
retryAfterSeconds?: number;
}
/** Checks the code is correct and not expired/exhausted, and marks the row `verifiedAt` — does NOT consume it. See finalizeOtp for the second phase, and this module's own header for why. */
export async function verifyOtp(db: Db, email: string, code: string, ip?: string): Promise<VerifyOtpResult> {
const normalizedEmail = normalizeEmail(email);
if (!rateLimitBypassed) {
const emailLimit = checkRateLimit(`otp-verify:${normalizedEmail}`, { maxAttempts: 10, windowMs: 15 * 60 * 1000 });
if (!emailLimit.allowed) return { ok: false, reason: "rate_limited", retryAfterSeconds: emailLimit.retryAfterSeconds };
if (ip) {
const ipLimit = checkRateLimit(`otp-verify-ip:${ip}`, { maxAttempts: 30, windowMs: 15 * 60 * 1000 });
if (!ipLimit.allowed) return { ok: false, reason: "rate_limited", retryAfterSeconds: ipLimit.retryAfterSeconds };
}
}
const row = await db.emailOtp.findUnique({ where: { email: normalizedEmail } });
// Always compare against *something* — see DUMMY_HASH above — so this
// call's timing doesn't reveal whether a row/valid-state exists before
// the early-return checks below run.
const codeMatches = await bcrypt.compare(code, row?.codeHash ?? DUMMY_HASH);
if (!row || row.consumedAt) return { ok: false, reason: "not_found" };
if (row.expiresAt < new Date()) return { ok: false, reason: "expired" };
if (row.attempts >= MAX_VERIFY_ATTEMPTS) return { ok: false, reason: "too_many_attempts" };
if (!codeMatches) {
await db.emailOtp.update({ where: { email: normalizedEmail }, data: { attempts: { increment: 1 } } });
return { ok: false, reason: "invalid_code" };
}
await db.emailOtp.update({ where: { email: normalizedEmail }, data: { verifiedAt: new Date() } });
return { ok: true };
}
/**
* Second phase: consumes a previously-`verifyOtp`'d row, enforcing
* one-time use. For a plain sign-in, callers run this immediately after
* verifyOtp in the same request — functionally "verify and consume in one
* shot". For the installer, this runs *inside* the same transaction that
* creates the administrator (see installer.service.ts), using the
* `verifiedAt` proof from an earlier request instead of re-entering a
* code that's no longer usable — and rolls back with the rest of that
* transaction if a later step fails, so a retry isn't forced to re-verify.
* Staff-invitation-acceptance uses the invite token as its trust anchor
* instead (see auth.ts's `finalize` credential path) and never calls this
* at all.
*/
export async function finalizeOtp(db: Db, email: string): Promise<boolean> {
const normalizedEmail = normalizeEmail(email);
const row = await db.emailOtp.findUnique({ where: { email: normalizedEmail } });
if (!row || !row.verifiedAt || row.consumedAt) return false;
if (Date.now() - row.verifiedAt.getTime() > VERIFIED_GRACE_MS) return false;
await db.emailOtp.update({ where: { email: normalizedEmail }, data: { consumedAt: new Date() } });
return true;
}
/**
* Opportunistic cleanup, same spirit as rate-limit.ts's sweepExpired — no
* cron needed since a stale row is bounded by the number of distinct real
* emails that ever requested a code, and any later request for that email
* overwrites it anyway. Called with low probability from requestOtp so it
* doesn't add latency to every single request.
*/
export async function cleanupExpiredOtps(db: Db): Promise<void> {
const cutoff = new Date(Date.now() - 24 * 60 * 60 * 1000);
await db.emailOtp.deleteMany({ where: { expiresAt: { lt: cutoff } } });
}