fix(auth): require a same-origin POST to delete an account (H1 #3906522)

web/api/delete-account/index.ts

@@ -1,13 +1,32 @@
+import { isSameOriginRequest } from "@/api/helpers/csrf";
 import { errorResponse } from "@/api/helpers/errors";
-import { logger } from "@/lib/logger";
-import { urls } from "@/lib/urls";
 import { auth0 } from "@/lib/auth0";
+import { logger } from "@/lib/logger";
 import { ManagementClient } from "auth0";
-import { NextRequest } from "next/server";
-import { getAppUrlFromRequest } from "../helpers/utils";
+import { NextRequest, NextResponse } from "next/server";
 
+/**
+ * Irreversibly deletes the caller's Auth0 identity.
+ *
+ * Mounted on POST only, and rejects any request the browser reports as cross-site.
+ * The session cookie is `SameSite=Lax`, so a GET mount here would let any attacker
+ * page destroy a logged-in developer's identity with a single top-level navigation
+ * (`<a href>`, `window.open`, meta-refresh) — a valid session cookie alone does not
+ * prove the request came from our own UI. See `api/helpers/csrf.ts`.
+ *
+ * The caller deletes the Hasura user row first, POSTs here, then navigates to
+ * `/api/auth/logout` to clear the (still-valid, stateless) session cookie.
+ */
 export const deleteAccount = async (req: NextRequest) => {
-  const appUrl = await getAppUrlFromRequest(req);
+  if (!(await isSameOriginRequest(req))) {
+    return errorResponse({
+      statusCode: 403,
+      code: "cross_origin_request",
+      detail: "Account deletion must be initiated from the developer portal",
+      req,
+    });
+  }
+
   if (
     !process.env.AUTH0_CLIENT_ID ||
     !process.env.AUTH0_CLIENT_SECRET ||
@@ -58,5 +77,8 @@ export const deleteAccount = async (req: NextRequest) => {
     });
   }
 
-  return Response.redirect(new URL(urls.logout(), appUrl), 307);
+  // No redirect: the caller is a `fetch()`, and returning a 307 here would make it
+  // replay the POST against `/api/auth/logout`. The client navigates to logout
+  // itself, passing the host it is on so the sibling-domain return lands correctly.
+  return new NextResponse(null, { status: 204 });
 };

web/api/helpers/csrf.ts

@@ -0,0 +1,53 @@
+import { getAllowedAppBaseUrls } from "@/lib/app-base-url";
+import { NextRequest } from "next/server";
+import "server-only";
+import { getAppUrlFromRequest } from "./utils";
+
+const toOrigin = (value: string | undefined): string | undefined => {
+  if (!value) return undefined;
+  try {
+    return new URL(value).origin;
+  } catch {
+    return undefined;
+  }
+};
+
+/**
+ * Cross-site request guard for cookie-authenticated, state-changing endpoints.
+ *
+ * The Auth0 session cookie is `SameSite=Lax` (SDK default — `lib/auth0.ts` sets no
+ * `session.cookie` override), so the browser attaches it to any cross-site
+ * *top-level navigation*. "A valid session cookie is present" is therefore not
+ * evidence that the request came from our own UI, and a destructive handler gated
+ * on the session alone is CSRF-able by a plain `<a href>` on an attacker's page.
+ *
+ * Endpoints using this guard must also be mounted on a non-idempotent method:
+ * `Lax` never sends the cookie on a cross-site POST, so the method restriction is
+ * the primary defence and this check is the layer that survives a future change to
+ * the cookie's `SameSite` attribute.
+ *
+ * `Sec-Fetch-Site` is set by the browser and cannot be forged from page JS. Only
+ * `same-origin` is accepted — `same-site` would let a takeover of any sibling
+ * subdomain reach the endpoint, and our own callers always use a relative URL.
+ *
+ * Fail-closed: a request carrying neither `Sec-Fetch-Site` nor `Origin` cannot be
+ * shown to be same-origin. Every browser that can render the portal sends at least
+ * one of them on a `fetch()`, so rejecting costs no real client.
+ */
+export const isSameOriginRequest = async (
+  req: NextRequest,
+): Promise<boolean> => {
+  const secFetchSite = req.headers.get("sec-fetch-site");
+  if (secFetchSite) return secFetchSite === "same-origin";
+
+  const origin = toOrigin(req.headers.get("origin") ?? undefined);
+  if (!origin) return false;
+
+  const configured = getAllowedAppBaseUrls();
+  const allowed = [
+    await getAppUrlFromRequest(req),
+    ...(Array.isArray(configured) ? configured : [configured]),
+  ].flatMap((value) => toOrigin(value) ?? []);
+
+  return allowed.includes(origin);
+};

web/app/api/auth/delete-account/route.ts

@@ -1 +1,4 @@
-export { deleteAccount as GET } from "@/api/delete-account";
+// POST only. Deleting the Auth0 identity is irreversible and authenticated by the
+// `SameSite=Lax` session cookie alone, which the browser would attach to a
+// cross-site top-level GET navigation. Next returns 405 for every other method.
+export { deleteAccount as POST } from "@/api/delete-account";

web/proxy.ts

@@ -367,6 +367,8 @@ export async function proxy(request: NextRequest) {
 
   // Auth SDK routes pass straight through: login/logout/callback/profile under
   // `/api/auth/*`, plus our custom login-callback / delete-account handlers.
+  // These carry their own request-origin checks — `delete-account` is POST-only
+  // and same-origin gated (`api/helpers/csrf.ts`) because nothing here does it.
   if (pathname.startsWith("/api/auth/")) {
     return authRes;
   }

web/scenes/Portal/Profile/DangerZone/DeleteAccountDialog/index.tsx

@@ -17,6 +17,7 @@ import { useForm } from "react-hook-form";
 import { toast } from "react-toastify";
 import * as yup from "yup";
 import { useMutation } from "@apollo/client/react";
+import { deleteAuth0Identity } from "@/scenes/common/Profile/DangerZone/DeleteAccountDialog/delete-auth0-identity";
 import { DeleteAccountDocument } from "@/scenes/common/Profile/DangerZone/DeleteAccountDialog/graphql/client/delete-account.generated";
 
 const DELETE_WORD = "DELETE";
@@ -60,8 +61,9 @@ export const DeleteAccountDialog = (props: DialogProps) => {
           user_id: user.hasura.id,
         },
       });
+      await deleteAuth0Identity();
       toast.success("Account Deleted!");
-      window.location.href = urls.api.authDeleteAccount();
+      window.location.href = urls.logout(window.location.origin);
     } catch (e) {
       console.error("Delete Account Dialog: ", e);
       toast.error("Error deleting account");

web/scenes/PortalV3/Profile/DangerZone/DeleteAccountDialog/index.tsx

@@ -6,6 +6,7 @@ import { FormDialog } from "@/components/FormDialog";
 import { AlertIcon } from "@/components/Icons/AlertIcon";
 import { Auth0SessionUser } from "@/lib/types";
 import { urls } from "@/lib/urls";
+import { deleteAuth0Identity } from "@/scenes/common/Profile/DangerZone/DeleteAccountDialog/delete-auth0-identity";
 import { DeleteAccountDocument } from "@/scenes/common/Profile/DangerZone/DeleteAccountDialog/graphql/client/delete-account.generated";
 import { useUser } from "@auth0/nextjs-auth0/client";
 import { useMutation } from "@apollo/client/react";
@@ -58,8 +59,9 @@ export const DeleteAccountDialog = (props: DialogProps) => {
           user_id: user.hasura.id,
         },
       });
+      await deleteAuth0Identity();
       toast.success("Account Deleted!");
-      window.location.href = urls.api.authDeleteAccount();
+      window.location.href = urls.logout(window.location.origin);
     } catch (e) {
       console.error("Delete Account Dialog: ", e);
       toast.error("Error deleting account");

web/scenes/common/Profile/DangerZone/DeleteAccountDialog/delete-auth0-identity.ts

@@ -0,0 +1,22 @@
+import { urls } from "@/lib/urls";
+
+/**
+ * Deletes the caller's Auth0 identity after the Hasura user row is gone.
+ *
+ * Must be a POST: the endpoint is authenticated by the `SameSite=Lax` session
+ * cookie, which the browser would also attach to a cross-site top-level GET
+ * navigation, so a navigable delete is CSRF-able from any attacker page. The
+ * response is a 204 — logout navigation is the caller's job.
+ */
+export const deleteAuth0Identity = async (): Promise<void> => {
+  const response = await fetch(urls.api.authDeleteAccount(), {
+    method: "POST",
+    credentials: "same-origin",
+  });
+
+  if (!response.ok) {
+    throw new Error(
+      `Failed to delete Auth0 identity: ${response.status} ${response.statusText}`,
+    );
+  }
+};

web/tests/integration/auth/delete-account.test.ts

@@ -1,9 +1,9 @@
 import { deleteAccount } from "@/api/delete-account";
-import { Auth0User } from "@/lib/types";
-import { urls } from "@/lib/urls";
 import { auth0 } from "@/lib/auth0";
+import { Auth0User } from "@/lib/types";
 import { NextRequest } from "next/server";
 
+// #region Mocks
 const validSessionUser = {
   email: "test@world.org",
   email_verified: true,
@@ -23,52 +23,134 @@ jest.mock("@/lib/auth0", () => ({
 }));
 
 const getSession = auth0.getSession as jest.Mock;
-const updateSession = auth0.updateSession as jest.Mock;
 
 jest.mock("../../../lib/logger", () => ({
   logger: {
+    info: jest.fn(),
     error: jest.fn(),
     warn: jest.fn(),
   },
 }));
 
+const usersDelete = jest.fn();
+
 jest.mock("auth0", () => ({
   ManagementClient: jest.fn().mockImplementation(() => ({
-    users: {
-      delete: jest.fn(() => Promise.resolve()),
-    },
+    users: { delete: (...args: unknown[]) => usersDelete(...args) },
   })),
 }));
+// #endregion
 
-describe("test /delete-account", () => {
-  beforeEach(() => {
-    // Reset mocks before each test
-    (getSession as jest.Mock).mockReset();
-    (updateSession as jest.Mock).mockReset();
+// #region Test Data
+const APP_ORIGIN = "http://localhost:3000";
+
+const createMockRequest = (headers: Record<string, string> = {}) =>
+  new NextRequest(`${APP_ORIGIN}/api/auth/delete-account`, {
+    method: "POST",
+    headers,
   });
 
-  it("should return 401 if session user id is not found", async () => {
-    const mockReq = {} as unknown as NextRequest;
-    (getSession as jest.Mock).mockResolvedValue(null);
-    const response = await deleteAccount(mockReq);
-    const body = await response.json();
-    expect(getSession).toHaveReturned();
-    expect(response.status).toEqual(401);
-    expect(body.code).toEqual("unauthorized");
+const sameOriginRequest = () =>
+  createMockRequest({ "sec-fetch-site": "same-origin" });
+// #endregion
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  usersDelete.mockResolvedValue(undefined);
+  getSession.mockResolvedValue({ user: validSessionUser });
+});
+
+// #region Cross-site request rejection
+// The session cookie is SameSite=Lax, so it rides along on any cross-site
+// top-level navigation. These branches are what stops an attacker page from
+// destroying a logged-in developer's Auth0 identity.
+describe("/api/auth/delete-account [cross-site requests]", () => {
+  it("rejects a request the browser reports as cross-site", async () => {
+    const response = await deleteAccount(
+      createMockRequest({ "sec-fetch-site": "cross-site" }),
+    );
+
+    expect(response.status).toEqual(403);
+    expect((await response.json()).code).toEqual("cross_origin_request");
+    expect(usersDelete).not.toHaveBeenCalled();
   });
 
-  it("Should successfully delete account", async () => {
-    const mockReq = {
-      json: () => Promise.resolve(),
-    } as unknown as NextRequest;
-
-    (getSession as jest.Mock).mockResolvedValue({ user: validSessionUser });
-    const response = await deleteAccount(mockReq);
-    expect(getSession).toHaveReturned();
-    expect(response.status).toEqual(307);
-    console.log(response.headers.get("location"));
-    expect(response.headers.get("location")).toEqual(
-      new URL(urls.logout(), process.env.NEXT_PUBLIC_APP_URL).toString(),
+  it("rejects a same-site request from a sibling subdomain", async () => {
+    const response = await deleteAccount(
+      createMockRequest({ "sec-fetch-site": "same-site" }),
     );
+
+    expect(response.status).toEqual(403);
+    expect(usersDelete).not.toHaveBeenCalled();
+  });
+
+  it("rejects a user-initiated top-level navigation", async () => {
+    // `none` means the URL was typed/bookmarked rather than issued by our UI —
+    // the shape a meta-refresh or `window.open` delivery lands as.
+    const response = await deleteAccount(
+      createMockRequest({ "sec-fetch-site": "none" }),
+    );
+
+    expect(response.status).toEqual(403);
+    expect(usersDelete).not.toHaveBeenCalled();
+  });
+
+  it("fails closed when the request carries no origin metadata", async () => {
+    const response = await deleteAccount(createMockRequest());
+
+    expect(response.status).toEqual(403);
+    expect(usersDelete).not.toHaveBeenCalled();
+  });
+
+  it("rejects a foreign Origin when Sec-Fetch-Site is absent", async () => {
+    const response = await deleteAccount(
+      createMockRequest({ origin: "https://evil.example" }),
+    );
+
+    expect(response.status).toEqual(403);
+    expect(usersDelete).not.toHaveBeenCalled();
+  });
+
+  it("accepts a matching Origin when Sec-Fetch-Site is absent", async () => {
+    const response = await deleteAccount(
+      createMockRequest({ origin: APP_ORIGIN }),
+    );
+
+    expect(response.status).toEqual(204);
+    expect(usersDelete).toHaveBeenCalledWith({ id: validSessionUser.sub });
+  });
+});
+// #endregion
+
+// #region Session and deletion outcomes
+describe("/api/auth/delete-account [same-origin requests]", () => {
+  it("returns 401 if session user id is not found", async () => {
+    getSession.mockResolvedValue(null);
+
+    const response = await deleteAccount(sameOriginRequest());
+
+    expect(response.status).toEqual(401);
+    expect((await response.json()).code).toEqual("unauthorized");
+    expect(usersDelete).not.toHaveBeenCalled();
+  });
+
+  it("deletes the Auth0 identity and returns 204", async () => {
+    const response = await deleteAccount(sameOriginRequest());
+
+    expect(usersDelete).toHaveBeenCalledWith({ id: validSessionUser.sub });
+    expect(response.status).toEqual(204);
+    // The client, not the server, drives the logout navigation: a redirect here
+    // would make the caller's fetch replay the POST against /api/auth/logout.
+    expect(response.headers.get("location")).toBeNull();
+  });
+
+  it("surfaces a 500 when Auth0 rejects the deletion", async () => {
+    usersDelete.mockRejectedValue(new Error("auth0 down"));
+
+    const response = await deleteAccount(sameOriginRequest());
+
+    expect(response.status).toEqual(500);
+    expect((await response.json()).code).toEqual("internal_server_error");
   });
 });
+// #endregion