From 96c6d1499e3ae28fe8014ced5b55b280ab514b34 Mon Sep 17 00:00:00 2001
From: Gerard Kavanagh <gerard161@gmail.com>
Date: Tue, 25 Aug 2026 16:28:31 +0100
Subject: [PATCH 1/4] fix(security): refuse consultation signups for accounts
 the caller doesn't own
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

POST /api/consultation/submit is the PUBLIC signup route (IP rate-limited
only), so the submitted email is unproven — anyone can type anyone's.
Creating records for a brand-new address is fine; operating on an address
that already has an account was not.

The route swallowed Clerk's form_identifier_exists ('proceed to DB/DrGreen')
and carried on against the existing account, ending in:

    prisma.users.update({ where: { id: userId },
                          data: { drGreenClientId, tenantId } })

i.e. an anonymous caller could submit a victim's email and re-point that
victim's account at a Dr Green client the caller controls. Approving the
caller's own genuine ID then made the VICTIM's account read VERIFIED —
inherited by the purchase gate, the tenant-admin badge, and (once client
status webhooks are enabled) propagated in near real time.

Fix — ownership must be proven before the handler touches a pre-existing
account. Provable two ways only: Clerk minted a NEW account for that address
in this request (nobody held it), or the caller holds a session for it.

- lib/security/email-ownership.ts: the rule as a pure, tested module
  (emailsMatch + canClaimAccount).
- Route: session email read Clerk-direct (never getCurrentUser — this path
  must keep working for anonymous visitors and must not throw on
  unprovisioned/multi-tenant accounts), preferring the primary address since
  the value is an ownership claim.
- Two choke points now answer 409 'sign in first, then complete your
  consultation': the Clerk-exists path and the local-existing-row path
  (legacy/webhook-provisioned users with no live Clerk account).
- The linking write re-asserts ownership, so a future path that sets userId
  another way cannot silently re-open this.

Unchanged for every legitimate flow: new signups (Clerk mints the account),
and signed-in customers completing their own consultation. The only newly
refused case is the attack — an anonymous caller submitting an address that
already belongs to someone.

Tests: rule table incl. both refusal cases, plus a static regression guard
(same idiom as the Article 9 persistence check) asserting the route keeps the
linking write gated and never restores the swallow-and-proceed path.
---
 .../app/api/consultation/submit/route.ts      | 104 ++++++++++++++++--
 nextjs_space/lib/security/email-ownership.ts  |  54 +++++++++
 .../tests/unit/email-ownership.test.ts        | 101 +++++++++++++++++
 3 files changed, 252 insertions(+), 7 deletions(-)
 create mode 100644 nextjs_space/lib/security/email-ownership.ts
 create mode 100644 nextjs_space/tests/unit/email-ownership.test.ts

diff --git a/nextjs_space/app/api/consultation/submit/route.ts b/nextjs_space/app/api/consultation/submit/route.ts
index e764c882..bc648dc6 100644
--- a/nextjs_space/app/api/consultation/submit/route.ts
+++ b/nextjs_space/app/api/consultation/submit/route.ts
@@ -1,5 +1,6 @@
 import { NextRequest, NextResponse } from "next/server";
-import { clerkClient } from "@clerk/nextjs/server";
+import { clerkClient, currentUser } from "@clerk/nextjs/server";
+import { canClaimAccount } from "@/lib/security/email-ownership";
 
 import { createAuditLog, AUDIT_ACTIONS, getClientInfo } from "@/lib/audit-log";
 import { triggerWebhook, WEBHOOK_EVENTS } from "@/lib/integrations/webhook";
@@ -25,6 +26,43 @@ import { logger } from '@/lib/logger';
 import { apiError, apiValidationError } from '@/lib/api-error';
 import { checkPolicyGate } from '@/lib/legal/policy-gate';
 
+/**
+ * The signed-in caller's email, or null when the request is anonymous.
+ *
+ * Deliberately Clerk-direct rather than `getCurrentUser()`: this is a PUBLIC
+ * signup route that must keep working for anonymous visitors, and
+ * getCurrentUser additionally resolves tenants and can throw for
+ * not-yet-provisioned or multi-tenant accounts. All we need here is "which
+ * address, if any, has this caller already authenticated as".
+ */
+async function getSessionEmail(): Promise<string | null> {
+  try {
+    const sessionUser = await currentUser();
+    if (!sessionUser) return null;
+    // Prefer the PRIMARY address: this value is an ownership claim, and on a
+    // multi-address account the positionally-first entry is not necessarily
+    // the one the session is anchored to.
+    const primary =
+      sessionUser.emailAddresses?.find(
+        (address) => address.id === sessionUser.primaryEmailAddressId,
+      ) ?? sessionUser.emailAddresses?.[0];
+    return primary?.emailAddress ?? null;
+  } catch {
+    // Expired/invalid token — treat as anonymous, never as an error.
+    return null;
+  }
+}
+
+/** 409 for "that address already belongs to an account you have not proven you own". */
+function accountExistsResponse() {
+  return apiError(new Error("Account already exists for this email"), {
+    route: "POST /api/consultation/submit",
+    status: 409,
+    safeMessage:
+      "An account already exists for this email address. Please sign in first, then complete your consultation.",
+  });
+}
+
 // SECURITY (C1, C13): Strict whitelist schema — no `.passthrough()`. Every
 // field that lands in the database or is forwarded to Dr. Green must be
 // declared here and length-capped. The tenant is resolved server-side from
@@ -173,6 +211,19 @@ export async function POST(request: NextRequest) {
       });
     }
 
+    // SECURITY (account takeover): this route is PUBLIC — the caller has not
+    // proven they own `body.email`, anyone can type anyone's address. Creating
+    // records for a BRAND-NEW address is fine; touching an address that
+    // already has an account is not. Without the ownership gate below, an
+    // anonymous caller could submit a victim's email, have Clerk's
+    // "already exists" swallowed, and reach the linking step further down that
+    // re-points the VICTIM's users row at a Dr Green client the ATTACKER
+    // controls — so once the attacker's own (genuine-looking) ID is approved,
+    // the victim's account inherits VERIFIED. Ownership is provable only by
+    // holding a session for that address, or by Clerk accepting a new account
+    // for it below (i.e. nobody held it).
+    const sessionEmail = await getSessionEmail();
+
     // 1. Create Clerk User (Auth)
     let clerkUser;
     try {
@@ -190,10 +241,22 @@ export async function POST(request: NextRequest) {
           },
         });
       } catch (clerkError: any) {
-        // Ignore if user already exists in Clerk, proceed to DB/DrGreen
+        // The address already has an account. Continuing "to DB/DrGreen" here
+        // is what let an anonymous caller operate on someone else's row —
+        // refuse unless they are signed in as that address.
         if (clerkError.errors?.[0]?.code === "form_identifier_exists") {
-          logger.info("[Consultation] user already exists in Clerk", { tenantId });
-          // Optionally fetch the user to get their ID if needed, but for now we proceed
+          if (!canClaimAccount({
+            accountJustCreated: false,
+            sessionEmail,
+            submittedEmail: body.email,
+          })) {
+            logger.warn(
+              "[Consultation] refused submission for an existing address by a caller not signed in as it",
+              { tenantId },
+            );
+            return accountExistsResponse();
+          }
+          logger.info("[Consultation] existing Clerk account, caller is signed in as it", { tenantId });
         } else {
           throw clerkError; // Re-throw other errors (e.g., weak password)
         }
@@ -217,6 +280,26 @@ export async function POST(request: NextRequest) {
       where: { email: body.email.toLowerCase() },
     });
 
+    // Resolved once: Clerk minted this account in this request (nobody held
+    // the address), or the caller is signed in as it. Guards both the adopt
+    // path below and the linking write near the end of the handler.
+    const callerOwnsAccount = canClaimAccount({
+      accountJustCreated: Boolean(clerkUser),
+      sessionEmail,
+      submittedEmail: body.email,
+    });
+
+    // Same rule for a local row with no live Clerk account behind it (legacy
+    // or webhook-provisioned users): Clerk did not vouch for this caller, so
+    // an unauthenticated request must not adopt — or mutate — that account.
+    if (existingUser && !callerOwnsAccount) {
+      logger.warn(
+        "[Consultation] refused submission for an existing local account by a caller not signed in as it",
+        { tenantId },
+      );
+      return accountExistsResponse();
+    }
+
     let userId: string | undefined;
 
     if (existingUser) {
@@ -588,9 +671,16 @@ export async function POST(request: NextRequest) {
         },
       });
 
-      // CRITICAL FIX: Also update the User record with the Dr. Green Client ID
-      // This is required for the kyc-check to work, as it looks at the User table.
-      if (userId) {
+      // Also update the User record with the Dr. Green Client ID — required
+      // for the kyc-check, which reads the User table.
+      //
+      // SECURITY: this is the write an account-takeover would target (point a
+      // victim's row at an attacker-controlled Dr Green client, then inherit
+      // its approval). The handler already refuses unowned addresses above;
+      // re-asserting ownership here keeps that guarantee attached to the
+      // dangerous line itself, so a future path that sets `userId` some other
+      // way cannot silently re-open it.
+      if (userId && callerOwnsAccount) {
         await prisma.users.update({
           where: { id: userId },
           data: {
diff --git a/nextjs_space/lib/security/email-ownership.ts b/nextjs_space/lib/security/email-ownership.ts
new file mode 100644
index 00000000..2f6079a8
--- /dev/null
+++ b/nextjs_space/lib/security/email-ownership.ts
@@ -0,0 +1,54 @@
+/**
+ * Ownership rule for PUBLIC endpoints that accept an email address.
+ *
+ * A signup-style endpoint cannot assume the caller owns the address they
+ * typed — anyone can type anyone's. That is fine while the request only
+ * CREATES things for a brand-new address, and dangerous the moment it
+ * MUTATES an account that already exists: re-pointing an existing user's
+ * tenant binding or their external client id lets an attacker attach a
+ * victim's account to a record the attacker controls, and the victim then
+ * inherits whatever status that record earns (approval, verification…).
+ *
+ * Ownership is provable in exactly two ways here:
+ *   1. the identity provider accepted a BRAND-NEW account for the address in
+ *      this same request (nobody else held it), or
+ *   2. the caller is signed in AS that address.
+ *
+ * Anything else must be refused and told to sign in — never silently
+ * "reuse the existing account".
+ */
+
+/** Case/whitespace-insensitive address comparison. Null/empty never matches. */
+export function emailsMatch(
+  a: string | null | undefined,
+  b: string | null | undefined,
+): boolean {
+  const left = a?.trim().toLowerCase();
+  const right = b?.trim().toLowerCase();
+  if (!left || !right) return false;
+  return left === right;
+}
+
+export interface AccountClaimInput {
+  /** The identity provider created a NEW account for this email in this request. */
+  accountJustCreated: boolean;
+  /** The authenticated caller's email, or null for an anonymous request. */
+  sessionEmail: string | null | undefined;
+  /** The email supplied in the request body. */
+  submittedEmail: string;
+}
+
+/**
+ * May this caller act on (and mutate) the account behind `submittedEmail`?
+ *
+ * Returns false for the anonymous-caller-hits-an-existing-address case, which
+ * is the one the caller must refuse.
+ */
+export function canClaimAccount({
+  accountJustCreated,
+  sessionEmail,
+  submittedEmail,
+}: AccountClaimInput): boolean {
+  if (accountJustCreated) return true;
+  return emailsMatch(sessionEmail, submittedEmail);
+}
diff --git a/nextjs_space/tests/unit/email-ownership.test.ts b/nextjs_space/tests/unit/email-ownership.test.ts
new file mode 100644
index 00000000..41dbe9cc
--- /dev/null
+++ b/nextjs_space/tests/unit/email-ownership.test.ts
@@ -0,0 +1,101 @@
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+import { describe, expect, it } from "vitest";
+
+import { canClaimAccount, emailsMatch } from "@/lib/security/email-ownership";
+
+/**
+ * Account-takeover guard for the public consultation signup.
+ *
+ * The hole this closes: `POST /api/consultation/submit` is unauthenticated
+ * (it IS the signup), and it used to swallow Clerk's "email already exists"
+ * and carry on against the existing account — ending in a write that pointed
+ * that user's `drGreenClientId`/`tenantId` at the caller's freshly created
+ * Dr Green client. An attacker submitting a victim's address, then getting
+ * their own genuine ID approved, would have the victim's account inherit
+ * VERIFIED — and with status webhooks live, in near real time.
+ */
+
+describe("emailsMatch", () => {
+  it("compares case- and whitespace-insensitively", () => {
+    expect(emailsMatch("Ann@Example.com", " ann@example.com ")).toBe(true);
+  });
+
+  it("never matches on a missing side", () => {
+    expect(emailsMatch(null, "ann@example.com")).toBe(false);
+    expect(emailsMatch(undefined, "ann@example.com")).toBe(false);
+    expect(emailsMatch("", "ann@example.com")).toBe(false);
+    expect(emailsMatch("   ", "ann@example.com")).toBe(false);
+    expect(emailsMatch("ann@example.com", null)).toBe(false);
+  });
+
+  it("does not treat different addresses as equal", () => {
+    expect(emailsMatch("ann@example.com", "ann@example.co")).toBe(false);
+    expect(emailsMatch("ann+tag@example.com", "ann@example.com")).toBe(false);
+  });
+});
+
+describe("canClaimAccount", () => {
+  it("REFUSES the takeover case: anonymous caller, address already taken", () => {
+    expect(
+      canClaimAccount({
+        accountJustCreated: false,
+        sessionEmail: null,
+        submittedEmail: "victim@example.com",
+      }),
+    ).toBe(false);
+  });
+
+  it("REFUSES a signed-in caller submitting someone else's address", () => {
+    expect(
+      canClaimAccount({
+        accountJustCreated: false,
+        sessionEmail: "attacker@example.com",
+        submittedEmail: "victim@example.com",
+      }),
+    ).toBe(false);
+  });
+
+  it("allows a brand-new account (the identity provider vouched nobody held it)", () => {
+    expect(
+      canClaimAccount({
+        accountJustCreated: true,
+        sessionEmail: null,
+        submittedEmail: "new@example.com",
+      }),
+    ).toBe(true);
+  });
+
+  it("allows a signed-in caller completing their own consultation", () => {
+    expect(
+      canClaimAccount({
+        accountJustCreated: false,
+        sessionEmail: "Ann@Example.com",
+        submittedEmail: "ann@example.com",
+      }),
+    ).toBe(true);
+  });
+});
+
+/**
+ * Static regression guard, in the same spirit as the Article 9 persistence
+ * check: the rule above only protects anything if the route actually applies
+ * it to the write that links a user row to a Dr Green client.
+ */
+describe("consultation submit route wiring", () => {
+  const source = readFileSync(
+    join(process.cwd(), "app/api/consultation/submit/route.ts"),
+    "utf8",
+  );
+
+  it("gates the Dr Green client-id linking write on proven ownership", () => {
+    expect(source).toMatch(/if\s*\(\s*userId\s*&&\s*callerOwnsAccount\s*\)/);
+  });
+
+  it("refuses, rather than silently reuses, an existing Clerk account", () => {
+    expect(source).toContain("form_identifier_exists");
+    expect(source).toMatch(/canClaimAccount\s*\(/);
+    // The pre-fix comment that marked the vulnerable "carry on anyway" path.
+    expect(source).not.toContain("for now we proceed");
+  });
+});

From 60a8104253d9bdf402468221d583df3c69f98e1c Mon Sep 17 00:00:00 2001
From: Gerard Kavanagh <gerard161@gmail.com>
Date: Tue, 25 Aug 2026 16:50:49 +0100
Subject: [PATCH 2/4] fix(security): make the ownership gate actually fire
 (review finding)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The first cut of this fix was defective, and the security review caught it:
`callerOwnsAccount` was computed as

    canClaimAccount({ accountJustCreated: Boolean(clerkUser), ... })

AFTER both paths that reach it had already forced it true — the Clerk-exists
path returns 409 unless the session matches, and the success path sets
clerkUser. So `if (existingUser && !callerOwnsAccount)` and the guard on the
linking write were dead code: present, correctly named, never able to fire.

Worse, the abstraction itself was wrong. 'The identity provider just minted
an account' proves nobody held the CLERK identity; it proves nothing about a
local users row that predates the request (legacy import, or a dropped Clerk
delete-webhook leaving an orphan). That left the mirror-image attack open:
target an address with a local row but no Clerk account, Clerk's createUser
succeeds for the attacker, and the handler adopts and re-points the
pre-existing row — the same outcome via the opposite precondition.

Corrected:
- Ownership of anything pre-existing is now provable ONE way: an
  authenticated session for that address. canClaimAccount is deleted rather
  than fixed — a helper that offers 'freshly minted' as a route to ownership
  is a footgun; lib/security/email-ownership.ts keeps only emailsMatch, and
  documents why the removed rule was unsound.
- ONE choke point (`existingUser && !sessionOwnsEmail`) sits ahead of every
  write in the handler — questionnaire, Dr Green client and linking update
  all follow it. The linking write documents that invariant instead of
  re-testing it, since a second check there would again be unfirable.
- getSessionEmail now requires the primary address to be VERIFIED, so the
  guarantee does not rest on Clerk's verified-primary invariant holding.
- The P2002 race branch documents why the row it adopts is necessarily the
  webhook mirror of the account this request just minted.

Tests: the static regex guard is gone — it passed against the vulnerable
code, which is the whole lesson. Replaced with
consultation-submit-ownership.test.ts, which drives the real handler and
asserts NO write (users, questionnaire, Dr Green) happens for each refusal
case, including the mirror-image one the first fix missed and an unverified
primary address; plus positive controls for new signups and signed-in
customers.

A no-Clerk-account legacy user who lands on the 409 can sign in with the
account Clerk just created and re-submit — refused, not locked out.
---
 .../app/api/consultation/submit/route.ts      |  92 ++++---
 nextjs_space/lib/security/email-ownership.ts  |  52 ++--
 .../consultation-submit-ownership.test.ts     | 248 ++++++++++++++++++
 .../tests/unit/email-ownership.test.ts        |  86 +-----
 4 files changed, 326 insertions(+), 152 deletions(-)
 create mode 100644 nextjs_space/tests/unit/consultation-submit-ownership.test.ts

diff --git a/nextjs_space/app/api/consultation/submit/route.ts b/nextjs_space/app/api/consultation/submit/route.ts
index bc648dc6..59fcb59c 100644
--- a/nextjs_space/app/api/consultation/submit/route.ts
+++ b/nextjs_space/app/api/consultation/submit/route.ts
@@ -1,6 +1,6 @@
 import { NextRequest, NextResponse } from "next/server";
 import { clerkClient, currentUser } from "@clerk/nextjs/server";
-import { canClaimAccount } from "@/lib/security/email-ownership";
+import { emailsMatch } from "@/lib/security/email-ownership";
 
 import { createAuditLog, AUDIT_ACTIONS, getClientInfo } from "@/lib/audit-log";
 import { triggerWebhook, WEBHOOK_EVENTS } from "@/lib/integrations/webhook";
@@ -39,13 +39,17 @@ async function getSessionEmail(): Promise<string | null> {
   try {
     const sessionUser = await currentUser();
     if (!sessionUser) return null;
-    // Prefer the PRIMARY address: this value is an ownership claim, and on a
-    // multi-address account the positionally-first entry is not necessarily
-    // the one the session is anchored to.
-    const primary =
-      sessionUser.emailAddresses?.find(
-        (address) => address.id === sessionUser.primaryEmailAddressId,
-      ) ?? sessionUser.emailAddresses?.[0];
+    // This value is an ownership claim, so it must be the PRIMARY address AND
+    // verified: the positionally-first entry is not necessarily the one the
+    // session is anchored to, and an unverified address proves nothing about
+    // who controls the mailbox. Clerk is expected to allow only verified
+    // addresses as primary — asserting it here means the guarantee does not
+    // depend on that remaining true.
+    const primary = sessionUser.emailAddresses?.find(
+      (address) =>
+        address.id === sessionUser.primaryEmailAddressId &&
+        address.verification?.status === "verified",
+    );
     return primary?.emailAddress ?? null;
   } catch {
     // Expired/invalid token — treat as anonymous, never as an error.
@@ -219,10 +223,15 @@ export async function POST(request: NextRequest) {
     // "already exists" swallowed, and reach the linking step further down that
     // re-points the VICTIM's users row at a Dr Green client the ATTACKER
     // controls — so once the attacker's own (genuine-looking) ID is approved,
-    // the victim's account inherits VERIFIED. Ownership is provable only by
-    // holding a session for that address, or by Clerk accepting a new account
-    // for it below (i.e. nobody held it).
+    // the victim's account inherits VERIFIED.
+    //
+    // Ownership over anything that already exists is provable ONE way: an
+    // authenticated session for that address. Notably NOT by Clerk accepting a
+    // new account below — that only proves nobody held the *Clerk* identity,
+    // which says nothing about a local users row that predates this request
+    // (legacy import, or a dropped Clerk delete-webhook).
     const sessionEmail = await getSessionEmail();
+    const sessionOwnsEmail = emailsMatch(sessionEmail, body.email);
 
     // 1. Create Clerk User (Auth)
     let clerkUser;
@@ -245,11 +254,7 @@ export async function POST(request: NextRequest) {
         // is what let an anonymous caller operate on someone else's row —
         // refuse unless they are signed in as that address.
         if (clerkError.errors?.[0]?.code === "form_identifier_exists") {
-          if (!canClaimAccount({
-            accountJustCreated: false,
-            sessionEmail,
-            submittedEmail: body.email,
-          })) {
+          if (!sessionOwnsEmail) {
             logger.warn(
               "[Consultation] refused submission for an existing address by a caller not signed in as it",
               { tenantId },
@@ -280,22 +285,21 @@ export async function POST(request: NextRequest) {
       where: { email: body.email.toLowerCase() },
     });
 
-    // Resolved once: Clerk minted this account in this request (nobody held
-    // the address), or the caller is signed in as it. Guards both the adopt
-    // path below and the linking write near the end of the handler.
-    const callerOwnsAccount = canClaimAccount({
-      accountJustCreated: Boolean(clerkUser),
-      sessionEmail,
-      submittedEmail: body.email,
-    });
-
-    // Same rule for a local row with no live Clerk account behind it (legacy
-    // or webhook-provisioned users): Clerk did not vouch for this caller, so
-    // an unauthenticated request must not adopt — or mutate — that account.
-    if (existingUser && !callerOwnsAccount) {
+    // THE ownership gate — the single choke point, ahead of every write in
+    // this handler (the questionnaire, the Dr Green client, and the linking
+    // update all come after it). A pre-existing row may only be adopted by a
+    // caller signed in as its address.
+    //
+    // Deliberately NOT "…or Clerk just minted the account": for an address
+    // that has a local row but no Clerk account (legacy import, dropped
+    // delete-webhook), Clerk's createUser SUCCEEDS for anyone, so accepting
+    // that as proof would re-open this hole from the opposite direction. Such
+    // a caller now gets the 409 and can sign in with the account Clerk just
+    // created for them, then re-submit.
+    if (existingUser && !sessionOwnsEmail) {
       logger.warn(
         "[Consultation] refused submission for an existing local account by a caller not signed in as it",
-        { tenantId },
+        { tenantId, clerkAccountJustCreated: Boolean(clerkUser) },
       );
       return accountExistsResponse();
     }
@@ -352,7 +356,11 @@ export async function POST(request: NextRequest) {
         userId = newUser.id;
         logger.info("[Consultation] created local user mirror", { userId, tenantId });
       } catch (prismaError: any) {
-        // Race condition: Clerk webhook may have created the user between our check and create
+        // Race condition: Clerk webhook may have created the user between our
+        // check and create. Reaching here means the ownership gate above saw
+        // NO pre-existing row, so this row appeared during this request — it
+        // is the webhook's mirror of the Clerk account this request just
+        // minted for this same address, not a stranger's record.
         if (prismaError.code === "P2002") {
           const raceUser = await prisma.users.findUnique({
             where: { email: body.email.toLowerCase() },
@@ -674,13 +682,21 @@ export async function POST(request: NextRequest) {
       // Also update the User record with the Dr. Green Client ID — required
       // for the kyc-check, which reads the User table.
       //
-      // SECURITY: this is the write an account-takeover would target (point a
-      // victim's row at an attacker-controlled Dr Green client, then inherit
-      // its approval). The handler already refuses unowned addresses above;
-      // re-asserting ownership here keeps that guarantee attached to the
-      // dangerous line itself, so a future path that sets `userId` some other
-      // way cannot silently re-open it.
-      if (userId && callerOwnsAccount) {
+      // SECURITY: this is the write an account-takeover targets — point a
+      // stranger's row at an attacker-controlled Dr Green client, then have
+      // that row inherit the client's approval.
+      //
+      // Safe by the ownership gate above, which returns 409 before any write
+      // unless `userId` is either a row THIS request created or a
+      // pre-existing row whose address the caller is signed in as. That
+      // invariant is enforced there, not re-tested here: a second check on
+      // this line would be unfirable by construction, which is exactly the
+      // mistake the first version of this fix made — a guard that reads like
+      // protection but can never trigger. The real regression net is the
+      // behavioural test in tests/unit/consultation-submit-ownership.test.ts,
+      // which drives this route with a pre-existing row and asserts nothing
+      // is written.
+      if (userId) {
         await prisma.users.update({
           where: { id: userId },
           data: {
diff --git a/nextjs_space/lib/security/email-ownership.ts b/nextjs_space/lib/security/email-ownership.ts
index 2f6079a8..a1bf68db 100644
--- a/nextjs_space/lib/security/email-ownership.ts
+++ b/nextjs_space/lib/security/email-ownership.ts
@@ -1,21 +1,25 @@
 /**
- * Ownership rule for PUBLIC endpoints that accept an email address.
+ * Ownership of an email address on PUBLIC endpoints.
  *
  * A signup-style endpoint cannot assume the caller owns the address they
  * typed — anyone can type anyone's. That is fine while the request only
- * CREATES things for a brand-new address, and dangerous the moment it
- * MUTATES an account that already exists: re-pointing an existing user's
- * tenant binding or their external client id lets an attacker attach a
- * victim's account to a record the attacker controls, and the victim then
- * inherits whatever status that record earns (approval, verification…).
+ * CREATES records for that address, and dangerous the moment it MUTATES a
+ * record that already existed: re-pointing an existing user's tenant binding
+ * or their external client id lets an attacker attach a stranger's account to
+ * a record the attacker controls, and that account then inherits whatever
+ * status the record earns (approval, verification…).
  *
- * Ownership is provable in exactly two ways here:
- *   1. the identity provider accepted a BRAND-NEW account for the address in
- *      this same request (nobody else held it), or
- *   2. the caller is signed in AS that address.
+ * The ONLY proof of ownership over a pre-existing record is an authenticated
+ * session for that address — hence this module exposes just the comparison.
  *
- * Anything else must be refused and told to sign in — never silently
- * "reuse the existing account".
+ * Explicitly NOT proof: "the identity provider accepted a brand-new account
+ * for this address in this request". An earlier version of this module
+ * offered that as a second route to ownership, which is wrong and was
+ * exploitable: the provider only vouches that nobody held the *provider's*
+ * identity, which says nothing about a local row that predates the request
+ * (a legacy import, or a dropped delete-webhook leaving an orphaned row).
+ * A caller who mints a fresh provider account for a stranger's address must
+ * still not be able to mutate that stranger's existing row.
  */
 
 /** Case/whitespace-insensitive address comparison. Null/empty never matches. */
@@ -28,27 +32,3 @@ export function emailsMatch(
   if (!left || !right) return false;
   return left === right;
 }
-
-export interface AccountClaimInput {
-  /** The identity provider created a NEW account for this email in this request. */
-  accountJustCreated: boolean;
-  /** The authenticated caller's email, or null for an anonymous request. */
-  sessionEmail: string | null | undefined;
-  /** The email supplied in the request body. */
-  submittedEmail: string;
-}
-
-/**
- * May this caller act on (and mutate) the account behind `submittedEmail`?
- *
- * Returns false for the anonymous-caller-hits-an-existing-address case, which
- * is the one the caller must refuse.
- */
-export function canClaimAccount({
-  accountJustCreated,
-  sessionEmail,
-  submittedEmail,
-}: AccountClaimInput): boolean {
-  if (accountJustCreated) return true;
-  return emailsMatch(sessionEmail, submittedEmail);
-}
diff --git a/nextjs_space/tests/unit/consultation-submit-ownership.test.ts b/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
new file mode 100644
index 00000000..e045adb7
--- /dev/null
+++ b/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
@@ -0,0 +1,248 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest } from "next/server";
+
+/**
+ * Account-takeover guard on the PUBLIC consultation signup.
+ *
+ * The hole: this route is unauthenticated (it IS the signup), so the submitted
+ * address is unproven. It used to swallow Clerk's "email already exists" and
+ * carry on against the existing account, ending in
+ * `users.update({ drGreenClientId, tenantId })` — re-pointing a stranger's
+ * account at a Dr Green client the caller controls. Approving the caller's own
+ * genuine ID then made the stranger's account read VERIFIED, which the
+ * purchase gate, the tenant-admin badge and the status webhooks all trust.
+ *
+ * The first fix was itself defective: it accepted "Clerk just minted a new
+ * account" as proof of ownership, which is true of the CLERK identity and says
+ * nothing about a local row that predates the request — so the mirror-image
+ * case (local row exists, no Clerk account for it) walked straight through a
+ * guard that could never fire. These tests drive the real handler, because a
+ * unit test of the helper and a regex over the source both passed against that
+ * defective version.
+ */
+
+const clerkMock = vi.hoisted(() => ({
+  currentUser: vi.fn(),
+  createUser: vi.fn(),
+  clerkClient: vi.fn(),
+}));
+const prismaMock = vi.hoisted(() => ({
+  users: { findUnique: vi.fn(), create: vi.fn(), update: vi.fn() },
+  consultation_questionnaires: { create: vi.fn(), update: vi.fn() },
+}));
+const libMock = vi.hoisted(() => ({
+  checkRateLimit: vi.fn(),
+  getTenantFromRequest: vi.fn(),
+  resolveTenant: vi.fn(),
+  checkPolicyGate: vi.fn(),
+  getTenantVerificationMode: vi.fn(),
+  isSaIdUploadEnabled: vi.fn(),
+  getTenantDrGreenConfig: vi.fn(),
+  callDrGreenAPI: vi.fn(),
+  createSaIdClient: vi.fn(),
+  uploadIdentityDocument: vi.fn(),
+  recordIdDocumentOutcome: vi.fn(),
+  createAuditLog: vi.fn(),
+  triggerWebhook: vi.fn(),
+  mapMedicalConditionsForDrGreen: vi.fn(),
+}));
+
+vi.mock("@clerk/nextjs/server", () => ({
+  currentUser: clerkMock.currentUser,
+  clerkClient: clerkMock.clerkClient,
+}));
+vi.mock("@/lib/db", () => ({ prisma: prismaMock }));
+vi.mock("@/lib/security/rate-limit", () => ({ checkRateLimit: libMock.checkRateLimit }));
+vi.mock("@/lib/tenant/tenant", () => ({ getTenantFromRequest: libMock.getTenantFromRequest }));
+vi.mock("@/lib/tenant/tenant-resolver", () => ({ resolveTenant: libMock.resolveTenant }));
+vi.mock("@/lib/legal/policy-gate", () => ({ checkPolicyGate: libMock.checkPolicyGate }));
+vi.mock("@/lib/verification-mode", () => ({
+  getTenantVerificationMode: libMock.getTenantVerificationMode,
+  isSaIdUploadEnabled: libMock.isSaIdUploadEnabled,
+}));
+vi.mock("@/lib/tenant/tenant-config", () => ({
+  getTenantDrGreenConfig: libMock.getTenantDrGreenConfig,
+}));
+vi.mock("@/lib/drgreen/drgreen-api-client", () => ({ callDrGreenAPI: libMock.callDrGreenAPI }));
+vi.mock("@/lib/drgreen-identity", () => ({
+  createSaIdClient: libMock.createSaIdClient,
+  uploadIdentityDocument: libMock.uploadIdentityDocument,
+}));
+vi.mock("@/lib/verification/id-document-status", () => ({
+  recordIdDocumentOutcome: libMock.recordIdDocumentOutcome,
+}));
+vi.mock("@/lib/drgreen/dr-green-mapping", () => ({
+  mapMedicalConditionsForDrGreen: libMock.mapMedicalConditionsForDrGreen,
+}));
+vi.mock("@/lib/audit-log", () => ({
+  createAuditLog: libMock.createAuditLog,
+  AUDIT_ACTIONS: { CONSULTATION_SUBMITTED: "consultation.submitted" },
+  getClientInfo: () => ({}),
+}));
+vi.mock("@/lib/integrations/webhook", () => ({
+  triggerWebhook: libMock.triggerWebhook,
+  WEBHOOK_EVENTS: { CONSULTATION_SUBMITTED: "consultation.submitted" },
+}));
+
+import { POST } from "@/app/api/consultation/submit/route";
+
+const TENANT = { id: "tenant-1", countryCode: "ZA", settings: {} };
+const VICTIM_EMAIL = "victim@example.com";
+
+/** A users row that already exists — the thing an attacker wants to re-point. */
+const existingVictimRow = {
+  id: "user-victim",
+  email: VICTIM_EMAIL,
+  tenantId: "tenant-victim",
+  drGreenClientId: "drg_victim_original",
+};
+
+function submission(over: Record<string, unknown> = {}) {
+  return {
+    firstName: "Attacker",
+    lastName: "Person",
+    email: VICTIM_EMAIL,
+    password: "sup3rsecret!",
+    phoneCode: "+27",
+    phoneNumber: "821234567",
+    dateOfBirth: "1990-01-01",
+    gender: "Other",
+    ...over,
+  };
+}
+
+function request(body: unknown) {
+  return new NextRequest("http://store.localhost/api/consultation/submit", {
+    method: "POST",
+    headers: { "content-type": "application/json", "x-forwarded-for": "203.0.113.9" },
+    body: JSON.stringify(body),
+  });
+}
+
+/** Signed-in session for `email`, shaped like Clerk's verified primary. */
+function session(email: string) {
+  return {
+    primaryEmailAddressId: "idn_1",
+    emailAddresses: [
+      { id: "idn_1", emailAddress: email, verification: { status: "verified" } },
+    ],
+  };
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  libMock.checkRateLimit.mockResolvedValue({ success: true });
+  libMock.getTenantFromRequest.mockResolvedValue(TENANT);
+  libMock.checkPolicyGate.mockResolvedValue({ allowed: true });
+  libMock.getTenantVerificationMode.mockReturnValue("KYC");
+  libMock.isSaIdUploadEnabled.mockReturnValue(false);
+  libMock.getTenantDrGreenConfig.mockResolvedValue({ apiKey: "k", secretKey: "s" });
+  libMock.mapMedicalConditionsForDrGreen.mockReturnValue([]);
+  libMock.callDrGreenAPI.mockResolvedValue({
+    data: { data: { id: "drg_attacker_client", kycLink: "https://kyc.example/x" } },
+  });
+  libMock.createAuditLog.mockResolvedValue(undefined);
+  libMock.triggerWebhook.mockResolvedValue(undefined);
+  clerkMock.clerkClient.mockResolvedValue({ users: { createUser: clerkMock.createUser } });
+  prismaMock.consultation_questionnaires.create.mockResolvedValue({ id: "q-1" });
+  prismaMock.consultation_questionnaires.update.mockResolvedValue({});
+  prismaMock.users.create.mockResolvedValue({ id: "user-new" });
+  prismaMock.users.update.mockResolvedValue({});
+});
+
+/** Nothing may be written for a submission the caller has not proven it owns. */
+function expectNoSideEffects() {
+  expect(prismaMock.users.update).not.toHaveBeenCalled();
+  expect(prismaMock.users.create).not.toHaveBeenCalled();
+  expect(prismaMock.consultation_questionnaires.create).not.toHaveBeenCalled();
+  expect(libMock.callDrGreenAPI).not.toHaveBeenCalled();
+  expect(libMock.createSaIdClient).not.toHaveBeenCalled();
+}
+
+describe("consultation submit — ownership of an existing account", () => {
+  it("refuses an anonymous caller when Clerk already holds the address", async () => {
+    clerkMock.currentUser.mockResolvedValue(null);
+    clerkMock.createUser.mockRejectedValue({
+      errors: [{ code: "form_identifier_exists", message: "That email address is taken." }],
+    });
+    prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
+
+    const response = await POST(request(submission()));
+
+    expect(response.status).toBe(409);
+    expectNoSideEffects();
+  });
+
+  it("refuses an anonymous caller when a local row exists but Clerk does NOT hold the address", async () => {
+    // The mirror-image case the first fix missed: Clerk mints an account for
+    // the attacker (the address was free THERE), while a users row for it —
+    // legacy import, or a dropped Clerk delete-webhook — already exists.
+    clerkMock.currentUser.mockResolvedValue(null);
+    clerkMock.createUser.mockResolvedValue({ id: "clerk_attacker" });
+    prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
+
+    const response = await POST(request(submission()));
+
+    expect(response.status).toBe(409);
+    expectNoSideEffects();
+    // The specific write the takeover needs: the pre-existing row must keep
+    // pointing at its own Dr Green client, in its own tenant.
+    expect(prismaMock.users.update).not.toHaveBeenCalledWith(
+      expect.objectContaining({ where: { id: existingVictimRow.id } }),
+    );
+  });
+
+  it("refuses a caller signed in as somebody else", async () => {
+    clerkMock.currentUser.mockResolvedValue(session("attacker@example.com"));
+    clerkMock.createUser.mockResolvedValue({ id: "clerk_attacker" });
+    prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
+
+    const response = await POST(request(submission()));
+
+    expect(response.status).toBe(409);
+    expectNoSideEffects();
+  });
+
+  it("refuses when the session's primary address is unverified", async () => {
+    clerkMock.currentUser.mockResolvedValue({
+      primaryEmailAddressId: "idn_1",
+      emailAddresses: [
+        { id: "idn_1", emailAddress: VICTIM_EMAIL, verification: { status: "unverified" } },
+      ],
+    });
+    clerkMock.createUser.mockResolvedValue({ id: "clerk_attacker" });
+    prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
+
+    const response = await POST(request(submission()));
+
+    expect(response.status).toBe(409);
+    expectNoSideEffects();
+  });
+});
+
+describe("consultation submit — legitimate flows still work", () => {
+  it("lets a brand-new address through the gate", async () => {
+    clerkMock.currentUser.mockResolvedValue(null);
+    clerkMock.createUser.mockResolvedValue({ id: "clerk_new_user" });
+    prismaMock.users.findUnique.mockResolvedValue(null);
+
+    const response = await POST(request(submission({ email: "brand-new@example.com" })));
+
+    expect(response.status).not.toBe(409);
+    expect(prismaMock.users.create).toHaveBeenCalled();
+    expect(prismaMock.consultation_questionnaires.create).toHaveBeenCalled();
+  });
+
+  it("lets a signed-in customer complete their own consultation", async () => {
+    clerkMock.currentUser.mockResolvedValue(session(VICTIM_EMAIL));
+    clerkMock.createUser.mockRejectedValue({
+      errors: [{ code: "form_identifier_exists", message: "That email address is taken." }],
+    });
+    prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
+
+    const response = await POST(request(submission()));
+
+    expect(response.status).not.toBe(409);
+    expect(prismaMock.consultation_questionnaires.create).toHaveBeenCalled();
+  });
+});
diff --git a/nextjs_space/tests/unit/email-ownership.test.ts b/nextjs_space/tests/unit/email-ownership.test.ts
index 41dbe9cc..01844366 100644
--- a/nextjs_space/tests/unit/email-ownership.test.ts
+++ b/nextjs_space/tests/unit/email-ownership.test.ts
@@ -1,21 +1,14 @@
-import { readFileSync } from "node:fs";
-import { join } from "node:path";
 import { describe, expect, it } from "vitest";
 
-import { canClaimAccount, emailsMatch } from "@/lib/security/email-ownership";
+import { emailsMatch } from "@/lib/security/email-ownership";
 
 /**
- * Account-takeover guard for the public consultation signup.
- *
- * The hole this closes: `POST /api/consultation/submit` is unauthenticated
- * (it IS the signup), and it used to swallow Clerk's "email already exists"
- * and carry on against the existing account — ending in a write that pointed
- * that user's `drGreenClientId`/`tenantId` at the caller's freshly created
- * Dr Green client. An attacker submitting a victim's address, then getting
- * their own genuine ID approved, would have the victim's account inherit
- * VERIFIED — and with status webhooks live, in near real time.
+ * The comparison behind every ownership decision on a public endpoint.
+ * The decision itself (who may touch a pre-existing account) is exercised
+ * against the real handler in consultation-submit-ownership.test.ts — an
+ * earlier version of this suite tested a helper in isolation and a regex over
+ * the route's source, and both passed while the route was still exploitable.
  */
-
 describe("emailsMatch", () => {
   it("compares case- and whitespace-insensitively", () => {
     expect(emailsMatch("Ann@Example.com", " ann@example.com ")).toBe(true);
@@ -27,75 +20,12 @@ describe("emailsMatch", () => {
     expect(emailsMatch("", "ann@example.com")).toBe(false);
     expect(emailsMatch("   ", "ann@example.com")).toBe(false);
     expect(emailsMatch("ann@example.com", null)).toBe(false);
+    expect(emailsMatch(null, null)).toBe(false);
   });
 
   it("does not treat different addresses as equal", () => {
     expect(emailsMatch("ann@example.com", "ann@example.co")).toBe(false);
     expect(emailsMatch("ann+tag@example.com", "ann@example.com")).toBe(false);
-  });
-});
-
-describe("canClaimAccount", () => {
-  it("REFUSES the takeover case: anonymous caller, address already taken", () => {
-    expect(
-      canClaimAccount({
-        accountJustCreated: false,
-        sessionEmail: null,
-        submittedEmail: "victim@example.com",
-      }),
-    ).toBe(false);
-  });
-
-  it("REFUSES a signed-in caller submitting someone else's address", () => {
-    expect(
-      canClaimAccount({
-        accountJustCreated: false,
-        sessionEmail: "attacker@example.com",
-        submittedEmail: "victim@example.com",
-      }),
-    ).toBe(false);
-  });
-
-  it("allows a brand-new account (the identity provider vouched nobody held it)", () => {
-    expect(
-      canClaimAccount({
-        accountJustCreated: true,
-        sessionEmail: null,
-        submittedEmail: "new@example.com",
-      }),
-    ).toBe(true);
-  });
-
-  it("allows a signed-in caller completing their own consultation", () => {
-    expect(
-      canClaimAccount({
-        accountJustCreated: false,
-        sessionEmail: "Ann@Example.com",
-        submittedEmail: "ann@example.com",
-      }),
-    ).toBe(true);
-  });
-});
-
-/**
- * Static regression guard, in the same spirit as the Article 9 persistence
- * check: the rule above only protects anything if the route actually applies
- * it to the write that links a user row to a Dr Green client.
- */
-describe("consultation submit route wiring", () => {
-  const source = readFileSync(
-    join(process.cwd(), "app/api/consultation/submit/route.ts"),
-    "utf8",
-  );
-
-  it("gates the Dr Green client-id linking write on proven ownership", () => {
-    expect(source).toMatch(/if\s*\(\s*userId\s*&&\s*callerOwnsAccount\s*\)/);
-  });
-
-  it("refuses, rather than silently reuses, an existing Clerk account", () => {
-    expect(source).toContain("form_identifier_exists");
-    expect(source).toMatch(/canClaimAccount\s*\(/);
-    // The pre-fix comment that marked the vulnerable "carry on anyway" path.
-    expect(source).not.toContain("for now we proceed");
+    expect(emailsMatch("ann@example.com", "anne@example.com")).toBe(false);
   });
 });

From 8530e2791eedb8d8dc39293010ea15364e8b8ace Mon Sep 17 00:00:00 2001
From: Gerard Kavanagh <gerard161@gmail.com>
Date: Tue, 25 Aug 2026 16:56:03 +0100
Subject: [PATCH 3/4] fix(consultation): extract session-email helper, complete
 test payload
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CI caught two of my own defects on the previous commit:

- Lint max-lines: my comments pushed route.ts to 815 lines over the repo's
  800 cap. getVerifiedSessionEmail moves to lib/security/session-email.ts
  (its own concern anyway — email-ownership.ts stays pure and Clerk-free)
  and the invariant comment at the linking write is trimmed. 776 lines now.
- The new behavioural tests all returned 400, not 409: the submission
  fixture omitted countryCode, which consultationSchema requires. The 400 at
  least proved the tests drive the real handler and the real schema.
---
 .../app/api/consultation/submit/route.ts      | 57 +++----------------
 nextjs_space/lib/security/session-email.ts    | 36 ++++++++++++
 .../consultation-submit-ownership.test.ts     |  2 +
 3 files changed, 47 insertions(+), 48 deletions(-)
 create mode 100644 nextjs_space/lib/security/session-email.ts

diff --git a/nextjs_space/app/api/consultation/submit/route.ts b/nextjs_space/app/api/consultation/submit/route.ts
index 59fcb59c..65442b4b 100644
--- a/nextjs_space/app/api/consultation/submit/route.ts
+++ b/nextjs_space/app/api/consultation/submit/route.ts
@@ -1,6 +1,7 @@
 import { NextRequest, NextResponse } from "next/server";
-import { clerkClient, currentUser } from "@clerk/nextjs/server";
+import { clerkClient } from "@clerk/nextjs/server";
 import { emailsMatch } from "@/lib/security/email-ownership";
+import { getVerifiedSessionEmail } from "@/lib/security/session-email";
 
 import { createAuditLog, AUDIT_ACTIONS, getClientInfo } from "@/lib/audit-log";
 import { triggerWebhook, WEBHOOK_EVENTS } from "@/lib/integrations/webhook";
@@ -26,37 +27,6 @@ import { logger } from '@/lib/logger';
 import { apiError, apiValidationError } from '@/lib/api-error';
 import { checkPolicyGate } from '@/lib/legal/policy-gate';
 
-/**
- * The signed-in caller's email, or null when the request is anonymous.
- *
- * Deliberately Clerk-direct rather than `getCurrentUser()`: this is a PUBLIC
- * signup route that must keep working for anonymous visitors, and
- * getCurrentUser additionally resolves tenants and can throw for
- * not-yet-provisioned or multi-tenant accounts. All we need here is "which
- * address, if any, has this caller already authenticated as".
- */
-async function getSessionEmail(): Promise<string | null> {
-  try {
-    const sessionUser = await currentUser();
-    if (!sessionUser) return null;
-    // This value is an ownership claim, so it must be the PRIMARY address AND
-    // verified: the positionally-first entry is not necessarily the one the
-    // session is anchored to, and an unverified address proves nothing about
-    // who controls the mailbox. Clerk is expected to allow only verified
-    // addresses as primary — asserting it here means the guarantee does not
-    // depend on that remaining true.
-    const primary = sessionUser.emailAddresses?.find(
-      (address) =>
-        address.id === sessionUser.primaryEmailAddressId &&
-        address.verification?.status === "verified",
-    );
-    return primary?.emailAddress ?? null;
-  } catch {
-    // Expired/invalid token — treat as anonymous, never as an error.
-    return null;
-  }
-}
-
 /** 409 for "that address already belongs to an account you have not proven you own". */
 function accountExistsResponse() {
   return apiError(new Error("Account already exists for this email"), {
@@ -230,8 +200,7 @@ export async function POST(request: NextRequest) {
     // new account below — that only proves nobody held the *Clerk* identity,
     // which says nothing about a local users row that predates this request
     // (legacy import, or a dropped Clerk delete-webhook).
-    const sessionEmail = await getSessionEmail();
-    const sessionOwnsEmail = emailsMatch(sessionEmail, body.email);
+    const sessionOwnsEmail = emailsMatch(await getVerifiedSessionEmail(), body.email);
 
     // 1. Create Clerk User (Auth)
     let clerkUser;
@@ -682,20 +651,12 @@ export async function POST(request: NextRequest) {
       // Also update the User record with the Dr. Green Client ID — required
       // for the kyc-check, which reads the User table.
       //
-      // SECURITY: this is the write an account-takeover targets — point a
-      // stranger's row at an attacker-controlled Dr Green client, then have
-      // that row inherit the client's approval.
-      //
-      // Safe by the ownership gate above, which returns 409 before any write
-      // unless `userId` is either a row THIS request created or a
-      // pre-existing row whose address the caller is signed in as. That
-      // invariant is enforced there, not re-tested here: a second check on
-      // this line would be unfirable by construction, which is exactly the
-      // mistake the first version of this fix made — a guard that reads like
-      // protection but can never trigger. The real regression net is the
-      // behavioural test in tests/unit/consultation-submit-ownership.test.ts,
-      // which drives this route with a pre-existing row and asserts nothing
-      // is written.
+      // SECURITY: the write an account-takeover targets. Safe by the ownership
+      // gate above — `userId` is either a row THIS request created, or a
+      // pre-existing row whose address the caller is signed in as. Enforced
+      // there and regression-tested in
+      // tests/unit/consultation-submit-ownership.test.ts; a re-check here
+      // would be unfirable by construction (the mistake the first cut made).
       if (userId) {
         await prisma.users.update({
           where: { id: userId },
diff --git a/nextjs_space/lib/security/session-email.ts b/nextjs_space/lib/security/session-email.ts
new file mode 100644
index 00000000..e312136c
--- /dev/null
+++ b/nextjs_space/lib/security/session-email.ts
@@ -0,0 +1,36 @@
+import { currentUser } from "@clerk/nextjs/server";
+
+/**
+ * The signed-in caller's VERIFIED PRIMARY email address, or null.
+ *
+ * Kept apart from `./email-ownership` so that module stays pure and
+ * dependency-free; this one owns the Clerk read.
+ *
+ * Two deliberate choices, both because the result is used as an ownership
+ * claim over existing records:
+ *  - Clerk-direct rather than `getCurrentUser()`: callers include PUBLIC
+ *    routes that must keep working for anonymous visitors, and getCurrentUser
+ *    additionally resolves tenants and can throw for not-yet-provisioned or
+ *    multi-tenant accounts. All that is needed here is which address, if any,
+ *    this caller has already authenticated as.
+ *  - Primary AND verified: the positionally-first address is not necessarily
+ *    the one the session is anchored to, and an unverified address proves
+ *    nothing about who controls the mailbox. Clerk is expected to allow only
+ *    verified addresses as primary — asserting it here means the guarantee
+ *    does not depend on that remaining true.
+ */
+export async function getVerifiedSessionEmail(): Promise<string | null> {
+  try {
+    const sessionUser = await currentUser();
+    if (!sessionUser) return null;
+    const primary = sessionUser.emailAddresses?.find(
+      (address) =>
+        address.id === sessionUser.primaryEmailAddressId &&
+        address.verification?.status === "verified",
+    );
+    return primary?.emailAddress ?? null;
+  } catch {
+    // Expired/invalid token — treat as anonymous, never as an error.
+    return null;
+  }
+}
diff --git a/nextjs_space/tests/unit/consultation-submit-ownership.test.ts b/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
index e045adb7..375b222b 100644
--- a/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
+++ b/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
@@ -107,6 +107,8 @@ function submission(over: Record<string, unknown> = {}) {
     phoneNumber: "821234567",
     dateOfBirth: "1990-01-01",
     gender: "Other",
+    // Required by consultationSchema (min 2) — everything else defaults.
+    countryCode: "ZA",
     ...over,
   };
 }

From 05110a8a1fa98f84ddf97385076118f8a5ebe2a2 Mon Sep 17 00:00:00 2001
From: Gerard Kavanagh <gerard161@gmail.com>
Date: Tue, 25 Aug 2026 17:01:56 +0100
Subject: [PATCH 4/4] fix(security): gate ownership BEFORE Clerk, close the
 two-request takeover
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second review pass found the fix closed the single-request exploit but not a
two-request version of it, using the very recovery path the previous commit
advertised as benign:

  1. anonymous POST with the target address -> gate refuses 409, no local or
     Dr Green write. BUT Clerk has already minted an account for that
     address, because createUser ran before the gate. Clerk performs no
     mailbox-control check here; the password is the submitter's own.
  2. caller signs in with that account.
  3. same POST again -> the session now satisfies sessionOwnsEmail, the gate
     passes, and the pre-existing row is re-pointed at the caller's Dr Green
     client. Original outcome, no human interaction, fully scriptable.

Fixes:
- The users lookup + ownership gate now run BEFORE the Clerk call. An address
  that already has a local row is refused without minting anything, so there
  is no account to sign into on a second attempt — and no squatting on a
  stranger's address as a side effect either.
- The P2002 race branch no longer asserts an invariant in a comment; it
  enforces it. A raced row is adopted only when it IS the mirror of the Clerk
  account this request minted (id match) or the caller is signed in as the
  address. Another writer landing a row for the same address in that window
  is refused rather than adopted.
- 409 copy no longer implies 'sign up again' (which is indistinguishable from
  the attack); it points at sign-in or support.

Accepted trade-off, stated plainly: the real owner of a local row with no
Clerk account can no longer self-serve here. Restoring that needs a flow
proving mailbox control (Clerk verification / password reset), not a form
that re-links an account to whoever fills it in.

Tests: refusal cases now also assert createUser was never called — without
that, a 409 is just step one of the chain above. New case covers the raced
row this request did not create.

Still unverified and worth confirming empirically (flagged by the reviewer):
whether Clerk's Backend-API createUser marks the address verified. The
reorder makes the fix hold either way, which is why it was chosen over
depending on that answer.
---
 .../app/api/consultation/submit/route.ts      | 75 ++++++++++++-------
 .../consultation-submit-ownership.test.ts     | 46 ++++++++++--
 2 files changed, 84 insertions(+), 37 deletions(-)

diff --git a/nextjs_space/app/api/consultation/submit/route.ts b/nextjs_space/app/api/consultation/submit/route.ts
index 65442b4b..9e5b5301 100644
--- a/nextjs_space/app/api/consultation/submit/route.ts
+++ b/nextjs_space/app/api/consultation/submit/route.ts
@@ -33,7 +33,7 @@ function accountExistsResponse() {
     route: "POST /api/consultation/submit",
     status: 409,
     safeMessage:
-      "An account already exists for this email address. Please sign in first, then complete your consultation.",
+      "An account already exists for this email address. Please sign in and then complete your consultation. If you cannot access that account, contact support — for your protection we cannot link it from an unauthenticated form.",
   });
 }
 
@@ -202,6 +202,34 @@ export async function POST(request: NextRequest) {
     // (legacy import, or a dropped Clerk delete-webhook).
     const sessionOwnsEmail = emailsMatch(await getVerifiedSessionEmail(), body.email);
 
+    // Check if user already exists locally (email is globally unique, don't filter by tenantId)
+    const existingUser = await prisma.users.findUnique({
+      where: { email: body.email.toLowerCase() },
+    });
+
+    // THE ownership gate. It runs BEFORE Clerk is touched, which matters: an
+    // address with a local row but no Clerk account (legacy import, dropped
+    // delete-webhook) is one Clerk will happily mint for ANYONE. Gating after
+    // that call left a two-request takeover — mint an account for the target
+    // address (Clerk performs no mailbox-control check; the password is the
+    // submitter's own), sign in with it, resubmit, and the session now
+    // "proves" ownership of a row the caller never owned. Refusing before the
+    // mint means there is no account to sign into, and no squatting on the
+    // address either.
+    //
+    // Consequence, deliberately accepted: the real owner of a local row with
+    // no Clerk account cannot self-serve through this route. Restoring that
+    // needs a flow that actually proves mailbox control (Clerk verification /
+    // password reset), never "sign up again" — which is indistinguishable
+    // from the attack.
+    if (existingUser && !sessionOwnsEmail) {
+      logger.warn(
+        "[Consultation] refused submission for an existing account by a caller not signed in as it",
+        { tenantId },
+      );
+      return accountExistsResponse();
+    }
+
     // 1. Create Clerk User (Auth)
     let clerkUser;
     try {
@@ -249,30 +277,6 @@ export async function POST(request: NextRequest) {
       );
     }
 
-    // Check if user already exists locally (email is globally unique, don't filter by tenantId)
-    const existingUser = await prisma.users.findUnique({
-      where: { email: body.email.toLowerCase() },
-    });
-
-    // THE ownership gate — the single choke point, ahead of every write in
-    // this handler (the questionnaire, the Dr Green client, and the linking
-    // update all come after it). A pre-existing row may only be adopted by a
-    // caller signed in as its address.
-    //
-    // Deliberately NOT "…or Clerk just minted the account": for an address
-    // that has a local row but no Clerk account (legacy import, dropped
-    // delete-webhook), Clerk's createUser SUCCEEDS for anyone, so accepting
-    // that as proof would re-open this hole from the opposite direction. Such
-    // a caller now gets the 409 and can sign in with the account Clerk just
-    // created for them, then re-submit.
-    if (existingUser && !sessionOwnsEmail) {
-      logger.warn(
-        "[Consultation] refused submission for an existing local account by a caller not signed in as it",
-        { tenantId, clerkAccountJustCreated: Boolean(clerkUser) },
-      );
-      return accountExistsResponse();
-    }
-
     let userId: string | undefined;
 
     if (existingUser) {
@@ -326,15 +330,28 @@ export async function POST(request: NextRequest) {
         logger.info("[Consultation] created local user mirror", { userId, tenantId });
       } catch (prismaError: any) {
         // Race condition: Clerk webhook may have created the user between our
-        // check and create. Reaching here means the ownership gate above saw
-        // NO pre-existing row, so this row appeared during this request — it
-        // is the webhook's mirror of the Clerk account this request just
-        // minted for this same address, not a stranger's record.
+        // check and create. The ownership gate saw no row for this address, so
+        // one appeared mid-request — but "it must therefore be ours" is an
+        // assumption, not a guarantee (another writer could land a row for the
+        // same address in that window), so the adopt below re-checks rather
+        // than trusting it.
         if (prismaError.code === "P2002") {
           const raceUser = await prisma.users.findUnique({
             where: { email: body.email.toLowerCase() },
           });
           if (raceUser) {
+            // Adopt only what this request is entitled to: the row is the
+            // mirror of the Clerk account we just minted (ids match, since
+            // the local id IS clerkUser.id), or the caller is signed in as
+            // the address. Anything else is a stranger's row that landed in
+            // the race window — refuse rather than adopt it.
+            if (!(clerkUser && raceUser.id === clerkUser.id) && !sessionOwnsEmail) {
+              logger.warn(
+                "[Consultation] refused a raced row that this request did not create",
+                { tenantId },
+              );
+              return accountExistsResponse();
+            }
             userId = raceUser.id;
             if (!raceUser.tenantId) {
               await prisma.users.update({
diff --git a/nextjs_space/tests/unit/consultation-submit-ownership.test.ts b/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
index 375b222b..55f9b6bf 100644
--- a/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
+++ b/nextjs_space/tests/unit/consultation-submit-ownership.test.ts
@@ -19,6 +19,13 @@ import { NextRequest } from "next/server";
  * guard that could never fire. These tests drive the real handler, because a
  * unit test of the helper and a regex over the source both passed against that
  * defective version.
+ *
+ * The second fix closed that in one request but not in two: Clerk was still
+ * called first, so a caller could mint an account for the target address
+ * (Clerk performs no mailbox-control check — the password is the submitter's
+ * own), sign in with it, and resubmit with a session that now "proved"
+ * ownership. Hence the gate runs BEFORE Clerk, and the refusal cases below
+ * assert `createUser` was never reached.
  */
 
 const clerkMock = vi.hoisted(() => ({
@@ -162,12 +169,12 @@ function expectNoSideEffects() {
 }
 
 describe("consultation submit — ownership of an existing account", () => {
-  it("refuses an anonymous caller when Clerk already holds the address", async () => {
+  it("refuses an anonymous caller when Clerk holds the address (no local row)", async () => {
     clerkMock.currentUser.mockResolvedValue(null);
     clerkMock.createUser.mockRejectedValue({
       errors: [{ code: "form_identifier_exists", message: "That email address is taken." }],
     });
-    prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
+    prismaMock.users.findUnique.mockResolvedValue(null);
 
     const response = await POST(request(submission()));
 
@@ -175,10 +182,11 @@ describe("consultation submit — ownership of an existing account", () => {
     expectNoSideEffects();
   });
 
-  it("refuses an anonymous caller when a local row exists but Clerk does NOT hold the address", async () => {
-    // The mirror-image case the first fix missed: Clerk mints an account for
-    // the attacker (the address was free THERE), while a users row for it —
-    // legacy import, or a dropped Clerk delete-webhook — already exists.
+  it("refuses an anonymous caller when a local row exists — without letting Clerk mint", async () => {
+    // The mirror-image case the first fix missed: a users row exists (legacy
+    // import, or a dropped Clerk delete-webhook) while the address is free in
+    // Clerk, so createUser would succeed for anyone. Refusing BEFORE that call
+    // is what also closes the mint → sign-in → resubmit chain.
     clerkMock.currentUser.mockResolvedValue(null);
     clerkMock.createUser.mockResolvedValue({ id: "clerk_attacker" });
     prismaMock.users.findUnique.mockResolvedValue(existingVictimRow);
@@ -187,8 +195,10 @@ describe("consultation submit — ownership of an existing account", () => {
 
     expect(response.status).toBe(409);
     expectNoSideEffects();
-    // The specific write the takeover needs: the pre-existing row must keep
-    // pointing at its own Dr Green client, in its own tenant.
+    // No account is minted for an address the caller has not proven it owns —
+    // otherwise the refusal just becomes step one of a two-request takeover.
+    expect(clerkMock.createUser).not.toHaveBeenCalled();
+    // And the pre-existing row keeps pointing at its own Dr Green client.
     expect(prismaMock.users.update).not.toHaveBeenCalledWith(
       expect.objectContaining({ where: { id: existingVictimRow.id } }),
     );
@@ -203,6 +213,7 @@ describe("consultation submit — ownership of an existing account", () => {
 
     expect(response.status).toBe(409);
     expectNoSideEffects();
+    expect(clerkMock.createUser).not.toHaveBeenCalled();
   });
 
   it("refuses when the session's primary address is unverified", async () => {
@@ -219,6 +230,25 @@ describe("consultation submit — ownership of an existing account", () => {
 
     expect(response.status).toBe(409);
     expectNoSideEffects();
+    expect(clerkMock.createUser).not.toHaveBeenCalled();
+  });
+
+  it("refuses to adopt a raced row this request did not create", async () => {
+    // No row at gate time, so the handler creates one and hits P2002 — but the
+    // row that appeared belongs to someone else, not to the Clerk account we
+    // just minted. Adopting it would re-open the same takeover.
+    clerkMock.currentUser.mockResolvedValue(null);
+    clerkMock.createUser.mockResolvedValue({ id: "clerk_new_user" });
+    prismaMock.users.findUnique
+      .mockResolvedValueOnce(null) // ownership gate: nothing there yet
+      .mockResolvedValueOnce({ ...existingVictimRow, id: "user-someone-else" }); // the race
+    prismaMock.users.create.mockRejectedValue({ code: "P2002" });
+
+    const response = await POST(request(submission()));
+
+    expect(response.status).toBe(409);
+    expect(prismaMock.users.update).not.toHaveBeenCalled();
+    expect(prismaMock.consultation_questionnaires.create).not.toHaveBeenCalled();
   });
 });