Merge commit from fork

* fix(login): strip returnCode from browser supplied session challenges

The updateOrCreateSession server action forwarded the browser's challenges
object to the session API verbatim and returned the resulting challenges in its
response. A party who knew only a victim's login name could create an
identify-only session, request OTP-Email and OTP-SMS challenges with the
returnCode delivery type, and read both plaintext codes straight out of the
server action response.

sanitizeChallenges forces returnCode off before anything is forwarded. It is
applied in createSessionAndUpdateCookie and setSessionAndUpdateCookie rather
than in the action itself, so every caller including sendPasskey is covered by
one gate. An OTP-Email returnCode request is rewritten to sendCode rather than
dropped, so the legitimate flow still completes and the code is delivered to the
user; WebAuthN challenges carry no code and pass through untouched.

The action response is also narrowed to the WebAuthN challenge. That is
redundant while the sanitizer holds, but it keeps "no OTP code crosses this
boundary" a local, readable property.

Co-Authored-By: Claude Opus 5 <[email protected]>

* test(login): assert session challenges are sanitized before reaching the API

challenges.test.ts covers what sanitizeChallenges does, but nothing asserted
that cookie.ts actually calls it. A refactor could drop either call site and the
whole suite would still pass, silently reopening the hole.

Covers both entry points to the session API by asserting on the challenges the
mocked client receives, so the test fails if the sanitizer is ever bypassed.

Co-Authored-By: Claude Opus 5 <[email protected]>

---------

Co-authored-by: Claude Opus 5 <[email protected]>

apps/login/src/lib/server/challenges.test.ts

@@ -0,0 +1,82 @@
+import { create } from "@zitadel/client";
+import { RequestChallengesSchema } from "@zitadel/proto/zitadel/session/v2/challenge_pb";
+import { assert, describe, expect, it } from "vitest";
+import { sanitizeChallenges } from "./challenges";
+
+describe("sanitizeChallenges", () => {
+  it("returns undefined when no challenges are requested", () => {
+    expect(sanitizeChallenges(undefined)).toBeUndefined();
+  });
+
+  it("forces returnCode off for an OTP SMS challenge", () => {
+    const challenges = create(RequestChallengesSchema, {
+      otpSms: { returnCode: true },
+    });
+
+    expect(sanitizeChallenges(challenges)?.otpSms?.returnCode).toBe(false);
+  });
+
+  it("leaves an OTP SMS challenge that already sends the code alone", () => {
+    const challenges = create(RequestChallengesSchema, {
+      otpSms: {},
+    });
+
+    expect(sanitizeChallenges(challenges)?.otpSms?.returnCode).toBe(false);
+  });
+
+  it("rewrites an OTP email returnCode challenge to sendCode", () => {
+    const challenges = create(RequestChallengesSchema, {
+      otpEmail: { deliveryType: { case: "returnCode", value: {} } },
+    });
+
+    const deliveryType = sanitizeChallenges(challenges)?.otpEmail?.deliveryType;
+
+    assert(deliveryType?.case === "sendCode");
+    // no url template: Zitadel falls back to its default verification url
+    expect(deliveryType.value.urlTemplate).toBeUndefined();
+  });
+
+  it("preserves the url template of an OTP email sendCode challenge", () => {
+    const urlTemplate = "https://example.com/otp/verify?code={{.Code}}";
+    const challenges = create(RequestChallengesSchema, {
+      otpEmail: { deliveryType: { case: "sendCode", value: { urlTemplate } } },
+    });
+
+    const deliveryType = sanitizeChallenges(challenges)?.otpEmail?.deliveryType;
+
+    assert(deliveryType?.case === "sendCode");
+    expect(deliveryType.value.urlTemplate).toBe(urlTemplate);
+  });
+
+  it("passes a WebAuthN challenge through untouched", () => {
+    const challenges = create(RequestChallengesSchema, {
+      webAuthN: { domain: "login.example.com", userVerificationRequirement: 1 },
+    });
+
+    expect(sanitizeChallenges(challenges)?.webAuthN).toEqual(challenges.webAuthN);
+  });
+
+  it("strips returnCode from both OTP methods requested together", () => {
+    const challenges = create(RequestChallengesSchema, {
+      otpSms: { returnCode: true },
+      otpEmail: { deliveryType: { case: "returnCode", value: {} } },
+    });
+
+    const sanitized = sanitizeChallenges(challenges);
+
+    expect(sanitized?.otpSms?.returnCode).toBe(false);
+    expect(sanitized?.otpEmail?.deliveryType?.case).toBe("sendCode");
+  });
+
+  it("does not mutate the caller's challenges", () => {
+    const challenges = create(RequestChallengesSchema, {
+      otpSms: { returnCode: true },
+      otpEmail: { deliveryType: { case: "returnCode", value: {} } },
+    });
+
+    sanitizeChallenges(challenges);
+
+    expect(challenges.otpSms?.returnCode).toBe(true);
+    expect(challenges.otpEmail?.deliveryType?.case).toBe("returnCode");
+  });
+});

apps/login/src/lib/server/challenges.ts

@@ -0,0 +1,44 @@
+import { create } from "@zitadel/client";
+import {
+  RequestChallenges,
+  RequestChallenges_OTPEmail_SendCodeSchema,
+} from "@zitadel/proto/zitadel/session/v2/challenge_pb";
+
+/**
+ * Strips the `returnCode` delivery type from a client-supplied challenge request.
+ *
+ * `returnCode` makes Zitadel return the freshly generated OTP in plaintext in the response
+ * body instead of sending it to the user's mailbox or phone. It is a legitimate API feature
+ * for callers that deliver the code themselves, but this app proxies requests on behalf of
+ * an untrusted browser: anything it forwards is attacker-controllable, and anything it
+ * returns is attacker-readable.
+ *
+ * Without this, a party who knows only a victim's login name could request both OTP-Email
+ * and OTP-SMS challenges on an identify-only session, read both codes straight out of the
+ * server-action response and submit them back as two verified factors — GHSA-3gwm-5wx8-4gm6.
+ *
+ * OTP challenges are rewritten to their out-of-band equivalent rather than dropped, so the
+ * legitimate 2FA flow still completes and the code reaches the actual user. WebAuthN
+ * challenges carry no code and are passed through untouched.
+ */
+export function sanitizeChallenges(challenges?: RequestChallenges): RequestChallenges | undefined {
+  if (!challenges) {
+    return challenges;
+  }
+
+  const sanitized = { ...challenges };
+
+  if (sanitized.otpSms) {
+    sanitized.otpSms = { ...sanitized.otpSms, returnCode: false };
+  }
+
+  if (sanitized.otpEmail?.deliveryType?.case === "returnCode") {
+    sanitized.otpEmail = {
+      ...sanitized.otpEmail,
+      // fall back to the default Zitadel verification url
+      deliveryType: { case: "sendCode", value: create(RequestChallenges_OTPEmail_SendCodeSchema, {}) },
+    };
+  }
+
+  return sanitized;
+}

apps/login/src/lib/server/cookie.test.ts

@@ -0,0 +1,101 @@
+import { create } from "@zitadel/client";
+import { RequestChallengesSchema } from "@zitadel/proto/zitadel/session/v2/challenge_pb";
+import { beforeEach, describe, expect, test, vi } from "vitest";
+
+vi.mock("next/headers", () => ({
+  headers: vi.fn(),
+}));
+
+vi.mock("../service-url", () => ({
+  getServiceConfig: vi.fn(),
+}));
+
+vi.mock("@/lib/zitadel", () => ({
+  createSessionForUserIdAndIdpIntent: vi.fn(),
+  createSessionFromChecksAndChallenges: vi.fn(),
+  getSecuritySettings: vi.fn(),
+  getSession: vi.fn(),
+  setSession: vi.fn(),
+}));
+
+vi.mock("@/lib/cookies", () => ({
+  addSessionToCookie: vi.fn(),
+  updateSessionCookie: vi.fn(),
+}));
+
+import { addSessionToCookie, updateSessionCookie } from "@/lib/cookies";
+import { createSessionFromChecksAndChallenges, getSecuritySettings, getSession, setSession } from "@/lib/zitadel";
+import { headers } from "next/headers";
+import { getServiceConfig } from "../service-url";
+import { createSessionAndUpdateCookie, setSessionAndUpdateCookie } from "./cookie";
+
+// A challenge request as an attacker-controlled browser could construct it: both OTP methods
+// asking for the code to come back in the response instead of being sent to the user.
+function returnCodeChallenges() {
+  return create(RequestChallengesSchema, {
+    otpSms: { returnCode: true },
+    otpEmail: { deliveryType: { case: "returnCode", value: {} } },
+  });
+}
+
+function expectSanitized(challenges: any) {
+  expect(challenges.otpSms.returnCode).toBe(false);
+  expect(challenges.otpEmail.deliveryType.case).toBe("sendCode");
+}
+
+describe("session challenge sanitization", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    vi.mocked(headers).mockResolvedValue({} as any);
+    vi.mocked(getServiceConfig).mockReturnValue({ serviceConfig: {} } as any);
+    vi.mocked(getSecuritySettings).mockResolvedValue({ embeddedIframe: { enabled: false } } as any);
+    vi.mocked(getSession).mockResolvedValue({
+      session: {
+        id: "sessionId",
+        factors: { user: { loginName: "[email protected]", organizationId: "orgId" } },
+      },
+    } as any);
+    vi.mocked(addSessionToCookie).mockResolvedValue(undefined as any);
+    vi.mocked(updateSessionCookie).mockResolvedValue(undefined as any);
+  });
+
+  test("createSessionAndUpdateCookie strips returnCode before calling the API", async () => {
+    vi.mocked(createSessionFromChecksAndChallenges).mockResolvedValue({
+      sessionId: "sessionId",
+      sessionToken: "sessionToken",
+    } as any);
+
+    await createSessionAndUpdateCookie({
+      checks: {} as any,
+      requestId: undefined,
+      challenges: returnCodeChallenges(),
+    });
+
+    expect(createSessionFromChecksAndChallenges).toHaveBeenCalledTimes(1);
+    expectSanitized(vi.mocked(createSessionFromChecksAndChallenges).mock.calls[0][0].challenges);
+  });
+
+  test("setSessionAndUpdateCookie strips returnCode before calling the API", async () => {
+    vi.mocked(setSession).mockResolvedValue({
+      sessionToken: "sessionToken",
+      details: {},
+    } as any);
+
+    await setSessionAndUpdateCookie({
+      recentCookie: {
+        id: "sessionId",
+        token: "sessionToken",
+        loginName: "[email protected]",
+        creationTs: "",
+        expirationTs: "",
+        changeTs: "",
+      },
+      challenges: returnCodeChallenges(),
+      lifetime: {} as any,
+    });
+
+    expect(setSession).toHaveBeenCalledTimes(1);
+    expectSanitized(vi.mocked(setSession).mock.calls[0][0].challenges);
+  });
+});

apps/login/src/lib/server/cookie.ts

@@ -2,6 +2,7 @@
 
 import { addSessionToCookie, updateSessionCookie } from "@/lib/cookies";
 import { createLogger } from "@/lib/logger";
+import { sanitizeChallenges } from "@/lib/server/challenges";
 import {
   createSessionForUserIdAndIdpIntent,
   createSessionFromChecksAndChallenges,
@@ -67,7 +68,7 @@ export async function createSessionAndUpdateCookie(command: {
     serviceConfig,
     checks: command.checks,
     lifetime: sessionLifetime,
-    challenges: command.challenges,
+    challenges: sanitizeChallenges(command.challenges),
   });
 
   if (createdSession) {
@@ -211,7 +212,7 @@ export async function setSessionAndUpdateCookie(command: {
     serviceConfig,
     sessionId: command.recentCookie.id,
     sessionToken: command.recentCookie.token,
-    challenges: command.challenges,
+    challenges: sanitizeChallenges(command.challenges),
     checks: command.checks,
     lifetime: command.lifetime,
   })

apps/login/src/lib/server/session.ts

@@ -12,7 +12,7 @@ import {
   listUsers,
 } from "@/lib/zitadel";
 import { create, Duration } from "@zitadel/client";
-import { RequestChallenges } from "@zitadel/proto/zitadel/session/v2/challenge_pb";
+import { Challenges, RequestChallenges } from "@zitadel/proto/zitadel/session/v2/challenge_pb";
 import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb";
 import { Checks, ChecksSchema } from "@zitadel/proto/zitadel/session/v2/session_service_pb";
 import { getTranslations } from "next-intl/server";
@@ -275,11 +275,17 @@ export async function updateOrCreateSession(options: UpdateSessionCommand) {
     }
   }
 
+  // @ts-ignore
+  const challengeResponse: Challenges | undefined = session.challenges;
+
   return {
     sessionId: session.id,
     factors: session.factors,
-    // @ts-ignore
-    challenges: session.challenges,
+    // Only the WebAuthN challenge may reach the browser. `sanitizeChallenges` already stops
+    // `returnCode` from being requested, so `otpSms`/`otpEmail` should always be unset here —
+    // keeping the projection explicit means no OTP code can leak through this boundary even
+    // if that ever regresses (GHSA-3gwm-5wx8-4gm6).
+    challenges: challengeResponse?.webAuthN ? { webAuthN: challengeResponse.webAuthN } : undefined,
     authMethods,
   };
 }