From 6fa5c0e662756d34b933a4eedc4a1ab8da72e596 Mon Sep 17 00:00:00 2001 From: "Marco A." Date: Wed, 29 Jul 2026 05:45:56 +0200 Subject: [PATCH] Merge commit from fork MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(login): add shared enrollment authorization guard Introduce assertSessionAuthorizedForEnrollment, a single gate that decides whether a session may enroll a new authenticator: it requires a verified primary factor (password, passkey or IDP) on a non-expired session, or, for onboarding, a user with no auth methods plus a prior user-verification check. This mirrors the gate previously inlined only in registerPasskeyLink, which is refactored to reuse the shared helper. It is the foundation for gating the remaining credential-enrollment entry points (GHSA-45f2-5q3r-xgg6). Co-Authored-By: Claude Opus 4.8 * fix(login): require authentication before WebAuthn/U2F enrollment addU2F and verifyU2F registered a credential for any session that resolved to a user id, including a bare identify-only session created by merely submitting a login name. An unauthenticated attacker who knew a victim's login name could therefore attach an attacker-controlled authenticator to the victim's account and then complete a login as the victim. Both actions now run assertSessionAuthorizedForEnrollment before registering or verifying, so an unauthenticated session is rejected and no credential is persisted. GHSA-45f2-5q3r-xgg6 Co-Authored-By: Claude Opus 4.8 * fix(login): require authentication before TOTP/OTP enrollment verifyTOTP and the otp//set page activated a TOTP/OTP factor for any session with a user id, and the u2f/set page rendered the registration form without an authorization check. An identify-only session could thus enroll a second factor on a victim's account. Gate all of them with assertSessionAuthorizedForEnrollment. The setup pages now render the unknown-context/error state instead of enrolling when the session is not authorized. Also guard the method value in the OTP set page, since the added guard branch removes the control-flow narrowing the previous throw provided. GHSA-45f2-5q3r-xgg6 Co-Authored-By: Claude Opus 4.8 * fix(login): require authentication before legacy (V1) passwordless enrollment The V1 passwordless registration handlers began and verified passwordless setup for any auth request that carried a user id, without checking that a first factor had been verified. An unauthenticated attacker could submit a victim's login name and then POST directly to the passwordless prompt/init endpoints to plant an attacker-controlled passkey, then log in as the victim. Add passwordlessSetupAllowed / CheckPasswordlessSetupAllowed, which permit enrollment only for a legitimate onboarding user (PasswordlessInitRequired) or an auth request whose first factor is already verified, and call it before BeginPasswordlessSetup and VerifyPasswordlessSetup. The verify path is also bound to authReq.UserID instead of the attacker-controllable form field, so a credential cannot be redirected onto a different account. The init-code (mail/onboarding) path is unchanged. GHSA-45f2-5q3r-xgg6 Co-Authored-By: Claude Opus 4.8 * fix(login): remove never-active legacy (V1) authReq passwordless setup path Supersedes the runtime gate added in the previous commit. Maintainer review (Livio, the original author) and investigation confirmed the auth-request based passwordless *setup* path in the legacy login was a never-active leftover: - The passwordless prompt page (passwordless_prompt.html) has no submit button and no script; it only tells the user to complete setup via the emailed link (the PasswordlessButtonText/NextButtonText/SkipButtonText i18n keys are unused). - Legitimate onboarding for a PasswordlessInitRequired user goes through the emailed init-code flow (authReq == nil, BeginPasswordlessInitCodeSetup / VerifyPasswordlessInitCodeSetup), which is untouched. - The authReq branch (handlePasswordlessPrompt -> BeginPasswordlessSetup / VerifyPasswordlessSetup) was reachable only by a crafted direct POST — the GHSA-45f2-5q3r-xgg6 attack. Rather than gate the dead path, remove it entirely, eliminating the attack surface: - delete handlePasswordlessPrompt and the POST /login/passwordless/prompt route (keep renderPasswordlessPrompt: the informational "use your emailed link" page for PasswordlessRegistrationPromptStep), - make renderPasswordlessRegistration / checkPasswordlessRegistration use only the init-code path, - drop the now-unused AuthRequestRepo.BeginPasswordlessSetup / VerifyPasswordlessSetup (and the interface decls) and the passwordlessPromptUrl template helper; the underlying HumanAddPasswordlessSetup / HumanHumanPasswordlessSetup commands remain (used by the auth-API self-service and the v2 passkey APIs). A crafted POST to /login/passwordless/init carrying an authRequestID but no valid code now falls through to the init-code path and fails. GHSA-45f2-5q3r-xgg6 Co-Authored-By: Claude Opus 4.8 * check for userverified true as well, cleanup return message --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: Max Peintner --- apps/login/src/app/(login)/mfa/set/page.tsx | 24 +-- .../src/app/(login)/otp/[method]/set/page.tsx | 14 +- apps/login/src/app/(login)/u2f/set/page.tsx | 18 ++- .../src/lib/server/enrollment-guard.test.ts | 136 +++++++++++++++++ apps/login/src/lib/server/enrollment-guard.ts | 53 +++++++ apps/login/src/lib/server/passkeys.ts | 46 ++---- apps/login/src/lib/server/u2f.test.ts | 144 ++++++++++++++++++ apps/login/src/lib/server/u2f.ts | 15 ++ apps/login/src/lib/server/verify.test.ts | 71 ++++++++- apps/login/src/lib/server/verify.ts | 14 +- apps/login/src/lib/session.ts | 35 ++++- .../ui/login/passwordless_prompt_handler.go | 16 +- .../passwordless_registration_handler.go | 23 ++- internal/api/ui/login/renderer.go | 3 - internal/api/ui/login/router.go | 2 - .../static/templates/passwordless_prompt.html | 20 +-- internal/auth/repository/auth_request.go | 2 - .../eventsourcing/eventstore/auth_request.go | 13 -- 18 files changed, 524 insertions(+), 125 deletions(-) create mode 100644 apps/login/src/lib/server/enrollment-guard.test.ts create mode 100644 apps/login/src/lib/server/enrollment-guard.ts create mode 100644 apps/login/src/lib/server/u2f.test.ts diff --git a/apps/login/src/app/(login)/mfa/set/page.tsx b/apps/login/src/app/(login)/mfa/set/page.tsx index f4d48c00d..467004b8d 100644 --- a/apps/login/src/app/(login)/mfa/set/page.tsx +++ b/apps/login/src/app/(login)/mfa/set/page.tsx @@ -6,7 +6,7 @@ import { Translated } from "@/components/translated"; import { UserAvatar } from "@/components/user-avatar"; import { getSessionCookieById } from "@/lib/cookies"; import { getServiceConfig } from "@/lib/service-url"; -import { loadMostRecentSession } from "@/lib/session"; +import { hasVerifiedPrimaryFactor, loadMostRecentSession } from "@/lib/session"; import { getBrandingSettings, getLoginSettings, @@ -14,7 +14,6 @@ import { getUserByID, listAuthenticationMethodTypes, } from "@/lib/zitadel"; -import { Timestamp, timestampDate } from "@zitadel/client"; import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; import { SecondFactorType } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; import { Metadata } from "next"; @@ -27,25 +26,6 @@ export async function generateMetadata(): Promise { return { title: t("set.title") }; } -function isSessionValidForMfaSet(session: Partial): { - valid: boolean; - verifiedAt?: Timestamp; -} { - const validPassword = session?.factors?.password?.verifiedAt; - const validPasskey = - session?.factors?.webAuthN?.verifiedAt && !!session?.factors?.webAuthN?.userVerified - ? session?.factors?.webAuthN?.verifiedAt - : undefined; - const validIDP = session?.factors?.intent?.verifiedAt; - - const stillValid = session.expirationDate ? timestampDate(session.expirationDate) > new Date() : true; - - const verifiedAt = validPassword || validPasskey || validIDP; - const valid = !!((validPassword || validPasskey || validIDP) && stillValid); - - return { valid, verifiedAt }; -} - export default async function Page(props: { searchParams: Promise> }) { const searchParams = await props.searchParams; @@ -111,7 +91,7 @@ export default async function Page(props: { searchParams: Promise { if (resp) { @@ -86,7 +96,7 @@ export default async function Page(props: { urlToContinue = `/otp/${method}?` + paramsToContinue; // immediately check the OTP on the next page if sms or email was set up - if (["email", "sms"].includes(method)) { + if (method && ["email", "sms"].includes(method)) { return redirect(urlToContinue); } } else if (requestId && sessionId) { diff --git a/apps/login/src/app/(login)/u2f/set/page.tsx b/apps/login/src/app/(login)/u2f/set/page.tsx index 9c5b64905..abebc13cb 100644 --- a/apps/login/src/app/(login)/u2f/set/page.tsx +++ b/apps/login/src/app/(login)/u2f/set/page.tsx @@ -3,6 +3,7 @@ import { DynamicTheme } from "@/components/dynamic-theme"; import { RegisterU2f } from "@/components/register-u2f"; import { Translated } from "@/components/translated"; import { UserAvatar } from "@/components/user-avatar"; +import { getEnrollmentAuthorizationError } from "@/lib/server/enrollment-guard"; import { getServiceConfig } from "@/lib/service-url"; import { loadMostRecentSession } from "@/lib/session"; import { getBrandingSettings } from "@/lib/zitadel"; @@ -33,6 +34,19 @@ export default async function Page(props: { searchParams: Promise
@@ -55,7 +69,7 @@ export default async function Page(props: { searchParams: Promise
- {!sessionFactors && ( + {(!sessionFactors || !enrollmentAuthorized) && (
@@ -63,7 +77,7 @@ export default async function Page(props: { searchParams: Promise )} - {sessionFactors?.id && ( + {sessionFactors?.id && enrollmentAuthorized && ( ({ + // Convert a {seconds} Timestamp-like object into a real Date for expiry checks. + timestampDate: vi.fn((ts: { seconds: bigint | number }) => new Date(Number(ts.seconds) * 1000)), +})); + +vi.mock("@/lib/zitadel", () => ({ + listAuthenticationMethodTypes: vi.fn(), +})); + +vi.mock("@/lib/verify-helper", () => ({ + checkUserVerification: vi.fn(), +})); + +const serviceConfig = { baseUrl: "https://example.com" } as any; + +// A timestamp far in the future / past, expressed the way the proto client shapes them. +const future = { seconds: BigInt(Math.floor(Date.now() / 1000) + 3600), nanos: 0 }; +const past = { seconds: BigInt(Math.floor(Date.now() / 1000) - 3600), nanos: 0 }; + +describe("getEnrollmentAuthorizationError", () => { + let mockListAuthenticationMethodTypes: any; + let mockCheckUserVerification: any; + + beforeEach(async () => { + vi.clearAllMocks(); + const { listAuthenticationMethodTypes } = await import("@/lib/zitadel"); + const { checkUserVerification } = await import("@/lib/verify-helper"); + mockListAuthenticationMethodTypes = vi.mocked(listAuthenticationMethodTypes); + mockCheckUserVerification = vi.mocked(checkUserVerification); + }); + + test("allows enrollment when the session has a verified password factor", async () => { + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "user-1", + session: { factors: { user: { id: "user-1" }, password: { verifiedAt: future } } } as any, + }); + + expect(result).toBeNull(); + // an authenticated session must not trigger the fallback checks + expect(mockListAuthenticationMethodTypes).not.toHaveBeenCalled(); + expect(mockCheckUserVerification).not.toHaveBeenCalled(); + }); + + test("allows enrollment when the session has a user-verified passkey factor", async () => { + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "user-1", + session: { factors: { user: { id: "user-1" }, webAuthN: { verifiedAt: future, userVerified: true } } } as any, + }); + + expect(result).toBeNull(); + }); + + test("rejects a presence-only passkey factor (userVerified false, user has auth methods)", async () => { + // A WebAuthn assertion without user-verification is a second-factor (U2F) check, not + // primary authentication — it must not authorize attaching a new authenticator. + mockListAuthenticationMethodTypes.mockResolvedValue({ authMethodTypes: [1] }); + + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "user-1", + session: { factors: { user: { id: "user-1" }, webAuthN: { verifiedAt: future, userVerified: false } } } as any, + }); + + expect(result).toBe("You have to authenticate or have a valid User Verification Check"); + }); + + test("allows enrollment when the session has a verified IDP intent factor", async () => { + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "user-1", + session: { factors: { user: { id: "user-1" }, intent: { verifiedAt: future } } } as any, + }); + + expect(result).toBeNull(); + }); + + test("rejects an expired session even if a primary factor was verified (user has auth methods)", async () => { + mockListAuthenticationMethodTypes.mockResolvedValue({ authMethodTypes: [1] }); + + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "user-1", + session: { + factors: { user: { id: "user-1" }, password: { verifiedAt: past } }, + expirationDate: past, + } as any, + }); + + expect(result).toBe("You have to authenticate or have a valid User Verification Check"); + }); + + test("rejects an identify-only session when the user already has auth methods", async () => { + mockListAuthenticationMethodTypes.mockResolvedValue({ authMethodTypes: [1] }); + + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "victim-1", + session: { factors: { user: { id: "victim-1" } } } as any, + }); + + expect(result).toBe("You have to authenticate or have a valid User Verification Check"); + expect(mockCheckUserVerification).not.toHaveBeenCalled(); + }); + + test("rejects an identify-only session with no auth methods when user verification is missing", async () => { + mockListAuthenticationMethodTypes.mockResolvedValue({ authMethodTypes: [] }); + mockCheckUserVerification.mockResolvedValue(false); + + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "new-user", + session: { factors: { user: { id: "new-user" } } } as any, + }); + + expect(result).toBe("User Verification Check has to be done"); + expect(mockCheckUserVerification).toHaveBeenCalledWith("new-user"); + }); + + test("allows onboarding: no auth methods and a valid prior user-verification check", async () => { + mockListAuthenticationMethodTypes.mockResolvedValue({ authMethodTypes: [] }); + mockCheckUserVerification.mockResolvedValue(true); + + const result = await getEnrollmentAuthorizationError({ + serviceConfig, + userId: "new-user", + session: { factors: { user: { id: "new-user" } } } as any, + }); + + expect(result).toBeNull(); + }); +}); diff --git a/apps/login/src/lib/server/enrollment-guard.ts b/apps/login/src/lib/server/enrollment-guard.ts new file mode 100644 index 000000000..d584a87c9 --- /dev/null +++ b/apps/login/src/lib/server/enrollment-guard.ts @@ -0,0 +1,53 @@ +import { createLogger } from "@/lib/logger"; +import { hasVerifiedPrimaryFactor } from "@/lib/session"; +import { checkUserVerification } from "@/lib/verify-helper"; +import { listAuthenticationMethodTypes, ServiceConfig } from "@/lib/zitadel"; +import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; + +const logger = createLogger("enrollment-guard"); + +/** + * Central authorization gate for credential-enrollment entry points. + * + * Returns `null` when enrollment is allowed, or an error message string explaining why it + * is not. Enrollment is allowed when EITHER: + * - the session already proves authentication (a verified primary factor, not expired), OR + * - the user has no authentication methods yet AND a prior user-verification check passed + * (the legitimate email/init-code onboarding path, mirroring `registerPasskeyLink`). + * + * This mirrors the gate that `apps/login/src/lib/server/passkeys.ts` already applies to + * passkey registration, so that U2F, TOTP and OTP enrollment share the exact same rules. + */ +export async function getEnrollmentAuthorizationError({ + serviceConfig, + session, + userId, +}: { + serviceConfig: ServiceConfig; + session: Partial; + userId: string; +}): Promise { + if (hasVerifiedPrimaryFactor(session).valid) { + return null; + } + + // The session is only "identified", not authenticated. + const authmethods = await listAuthenticationMethodTypes({ serviceConfig, userId }); + + // If the user already has any auth method configured, enrollment requires a real + // authentication (or a valid user-verification check) — reject the identify-only session. + if (authmethods.authMethodTypes.length !== 0) { + return "You have to authenticate or have a valid User Verification Check"; + } + + // No auth methods yet: allow only if a prior user-verification check was completed + // (e.g. the user just verified their email / redeemed an invite in this browser). + const hasValidUserVerificationCheck = await checkUserVerification(userId); + + logger.info("hasValidUserVerificationCheck", { hasValidUserVerificationCheck }); + if (!hasValidUserVerificationCheck) { + return "User Verification Check has to be done"; + } + + return null; +} diff --git a/apps/login/src/lib/server/passkeys.ts b/apps/login/src/lib/server/passkeys.ts index 70216ee03..45a9364ef 100644 --- a/apps/login/src/lib/server/passkeys.ts +++ b/apps/login/src/lib/server/passkeys.ts @@ -6,11 +6,10 @@ import { getLoginSettings, getSession, getUserByID, - listAuthenticationMethodTypes, registerPasskey, verifyPasskeyRegistration as zitadelVerifyPasskeyRegistration, } from "@/lib/zitadel"; -import { create, Duration, Timestamp, timestampDate } from "@zitadel/client"; +import { create, Duration } from "@zitadel/client"; import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; import { Checks, ChecksSchema, GetSessionResponse } from "@zitadel/proto/zitadel/session/v2/session_service_pb"; import { @@ -23,8 +22,9 @@ import { userAgent } from "next/server"; import { completeFlowOrGetUrl } from "../client"; import { getSessionCookieById } from "../cookies"; import { getServiceConfig } from "../service-url"; -import { checkEmailVerification, checkUserVerification } from "../verify-helper"; +import { checkEmailVerification } from "../verify-helper"; import { createSessionAndUpdateCookie } from "./cookie"; +import { getEnrollmentAuthorizationError } from "./enrollment-guard"; import { getPublicHost } from "./host"; import { updateOrCreateSession } from "./session"; @@ -45,21 +45,6 @@ type RegisterPasskeyCommand = { codeId?: string; }; -function isSessionValid(session: Partial): { - valid: boolean; - verifiedAt?: Timestamp; -} { - const validPassword = session?.factors?.password?.verifiedAt; - const validPasskey = session?.factors?.webAuthN?.verifiedAt; - const validIDP = session?.factors?.intent?.verifiedAt; - const stillValid = session.expirationDate ? timestampDate(session.expirationDate) > new Date() : true; - - const verifiedAt = validPassword || validPasskey || validIDP; - const valid = !!((validPassword || validPasskey || validIDP) && stillValid); - - return { valid, verifiedAt }; -} - export async function registerPasskeyLink( command: RegisterPasskeyCommand, ): Promise { @@ -92,25 +77,14 @@ export async function registerPasskeyLink( currentUserId = session.session.factors.user.id; - const sessionValid = isSessionValid(session.session); + const enrollmentError = await getEnrollmentAuthorizationError({ + serviceConfig, + session: session.session, + userId: currentUserId, + }); - if (!sessionValid.valid) { - const authmethods = await listAuthenticationMethodTypes({ serviceConfig, userId: currentUserId }); - - // if the user has no authmethods set, we need to check if the user was verified - if (authmethods.authMethodTypes.length !== 0) { - return { - error: "You have to authenticate or have a valid User Verification Check", - }; - } - - // check if a verification was done earlier - const hasValidUserVerificationCheck = await checkUserVerification(currentUserId); - - logger.info("hasValidUserVerificationCheck", { hasValidUserVerificationCheck }); - if (!hasValidUserVerificationCheck) { - return { error: "User Verification Check has to be done" }; - } + if (enrollmentError) { + return { error: enrollmentError }; } // Generate registration code if not provided diff --git a/apps/login/src/lib/server/u2f.test.ts b/apps/login/src/lib/server/u2f.test.ts new file mode 100644 index 000000000..d085580c4 --- /dev/null +++ b/apps/login/src/lib/server/u2f.test.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { addU2F, verifyU2F } from "./u2f"; + +vi.mock("@zitadel/client", () => ({ + // `create(schema, data)` is only used to build the verify request — return the data. + create: vi.fn((_schema: unknown, data: unknown) => data), +})); + +vi.mock("@/lib/zitadel", () => ({ + getSession: vi.fn(), + registerU2F: vi.fn(), + verifyU2FRegistration: vi.fn(), +})); + +vi.mock("../service-url", () => ({ + getServiceConfig: vi.fn(() => ({ serviceConfig: { baseUrl: "https://example.com" } })), +})); + +vi.mock("./host", () => ({ + getPublicHost: vi.fn(() => "test.com"), +})); + +vi.mock("../cookies", () => ({ + getSessionCookieById: vi.fn(), +})); + +vi.mock("./enrollment-guard", () => ({ + getEnrollmentAuthorizationError: vi.fn(), +})); + +vi.mock("next/headers", () => ({ + headers: vi.fn(() => new Headers()), +})); + +vi.mock("next/server", () => ({ + userAgent: vi.fn(() => ({ browser: {}, device: {}, os: {} })), +})); + +const userOnlySession = { + session: { id: "session-1", factors: { user: { id: "victim-1", loginName: "victim@example.com" } } }, +}; + +describe("addU2F", () => { + let getSessionCookieById: any; + let getSession: any; + let registerU2F: any; + let getEnrollmentAuthorizationError: any; + + beforeEach(async () => { + vi.clearAllMocks(); + getSessionCookieById = vi.mocked((await import("../cookies")).getSessionCookieById); + getSession = vi.mocked((await import("@/lib/zitadel")).getSession); + registerU2F = vi.mocked((await import("@/lib/zitadel")).registerU2F); + getEnrollmentAuthorizationError = vi.mocked((await import("./enrollment-guard")).getEnrollmentAuthorizationError); + + getSessionCookieById.mockResolvedValue({ id: "session-1", token: "token-1" }); + getSession.mockResolvedValue(userOnlySession); + }); + + test("rejects enrollment on an unauthenticated (identify-only) session without registering", async () => { + getEnrollmentAuthorizationError.mockResolvedValue("You have to authenticate or have a valid User Verification Check"); + + const result = await addU2F({ sessionId: "session-1" }); + + expect(result).toEqual({ error: "You have to authenticate or have a valid User Verification Check" }); + expect(getEnrollmentAuthorizationError).toHaveBeenCalledWith({ + serviceConfig: { baseUrl: "https://example.com" }, + session: userOnlySession.session, + userId: "victim-1", + }); + expect(registerU2F).not.toHaveBeenCalled(); + }); + + test("registers the credential when the session is authorized", async () => { + getEnrollmentAuthorizationError.mockResolvedValue(null); + registerU2F.mockResolvedValue({ u2fId: "u2f-1" }); + + const result = await addU2F({ sessionId: "session-1" }); + + expect(registerU2F).toHaveBeenCalledWith({ + serviceConfig: { baseUrl: "https://example.com" }, + userId: "victim-1", + domain: "test.com", + }); + expect(result).toEqual({ u2fId: "u2f-1" }); + }); + + test("returns error and never calls the guard when no session cookie exists", async () => { + getSessionCookieById.mockResolvedValue(undefined); + + const result = await addU2F({ sessionId: "missing" }); + + expect(result).toEqual({ error: "Could not get session" }); + expect(getEnrollmentAuthorizationError).not.toHaveBeenCalled(); + expect(registerU2F).not.toHaveBeenCalled(); + }); +}); + +describe("verifyU2F", () => { + let getSessionCookieById: any; + let getSession: any; + let verifyU2FRegistration: any; + let getEnrollmentAuthorizationError: any; + + beforeEach(async () => { + vi.clearAllMocks(); + getSessionCookieById = vi.mocked((await import("../cookies")).getSessionCookieById); + getSession = vi.mocked((await import("@/lib/zitadel")).getSession); + verifyU2FRegistration = vi.mocked((await import("@/lib/zitadel")).verifyU2FRegistration); + getEnrollmentAuthorizationError = vi.mocked((await import("./enrollment-guard")).getEnrollmentAuthorizationError); + + getSessionCookieById.mockResolvedValue({ id: "session-1", token: "token-1" }); + getSession.mockResolvedValue(userOnlySession); + }); + + test("rejects verification on an unauthenticated session without persisting the credential", async () => { + getEnrollmentAuthorizationError.mockResolvedValue("User Verification Check has to be done"); + + const result = await verifyU2F({ + u2fId: "u2f-1", + passkeyName: "my key", + publicKeyCredential: {}, + sessionId: "session-1", + }); + + expect(result).toEqual({ error: "User Verification Check has to be done" }); + expect(verifyU2FRegistration).not.toHaveBeenCalled(); + }); + + test("verifies the registration when the session is authorized", async () => { + getEnrollmentAuthorizationError.mockResolvedValue(null); + verifyU2FRegistration.mockResolvedValue({ done: true }); + + const result = await verifyU2F({ + u2fId: "u2f-1", + passkeyName: "my key", + publicKeyCredential: {}, + sessionId: "session-1", + }); + + expect(verifyU2FRegistration).toHaveBeenCalledTimes(1); + expect(result).toEqual({ done: true }); + }); +}); diff --git a/apps/login/src/lib/server/u2f.ts b/apps/login/src/lib/server/u2f.ts index e2c5b8cf0..0f48b5cd6 100644 --- a/apps/login/src/lib/server/u2f.ts +++ b/apps/login/src/lib/server/u2f.ts @@ -7,6 +7,7 @@ import { headers } from "next/headers"; import { userAgent } from "next/server"; import { getSessionCookieById } from "../cookies"; import { getServiceConfig } from "../service-url"; +import { getEnrollmentAuthorizationError } from "./enrollment-guard"; import { getPublicHost } from "./host"; type RegisterU2FCommand = { @@ -47,6 +48,13 @@ export async function addU2F(command: RegisterU2FCommand) { return { error: "Could not get session" }; } + // Enrollment must be authorized: a bare identify-only session (only the user factor set) + // must not be able to attach a new authenticator to the account (GHSA-45f2-5q3r-xgg6). + const enrollmentError = await getEnrollmentAuthorizationError({ serviceConfig, session: session.session!, userId }); + if (enrollmentError) { + return { error: enrollmentError }; + } + return registerU2F({ serviceConfig, userId, domain: hostname }); } @@ -79,6 +87,13 @@ export async function verifyU2F(command: VerifyU2FCommand) { return { error: "Could not get session" }; } + // Enrollment must be authorized: only an authenticated session (or a valid onboarding + // verification) may finish registering a new authenticator (GHSA-45f2-5q3r-xgg6). + const enrollmentError = await getEnrollmentAuthorizationError({ serviceConfig, session: session.session!, userId }); + if (enrollmentError) { + return { error: enrollmentError }; + } + const request = create(VerifyU2FRegistrationRequestSchema, { u2fId: command.u2fId, publicKeyCredential: command.publicKeyCredential, diff --git a/apps/login/src/lib/server/verify.test.ts b/apps/login/src/lib/server/verify.test.ts index 1cb7353e7..6a6c766f6 100644 --- a/apps/login/src/lib/server/verify.test.ts +++ b/apps/login/src/lib/server/verify.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; -import { sendVerification, trySendVerification } from "./verify"; +import { sendVerification, trySendVerification, verifyTOTP } from "./verify"; import { createInviteCode, @@ -23,6 +23,15 @@ vi.mock("@/lib/zitadel", () => ({ getLoginSettings: vi.fn(), sendEmailCode: vi.fn(), createInviteCode: vi.fn(), + verifyTOTPRegistration: vi.fn(), +})); + +vi.mock("../session", () => ({ + loadMostRecentSession: vi.fn(), +})); + +vi.mock("./enrollment-guard", () => ({ + getEnrollmentAuthorizationError: vi.fn(), })); vi.mock("next/headers", () => ({ @@ -344,3 +353,63 @@ describe("trySendVerification", () => { expect(result).toBe(false); }); }); + +describe("verifyTOTP", () => { + let mockLoadMostRecentSession: any; + let mockVerifyTOTPRegistration: any; + let mockGetEnrollmentAuthorizationError: any; + + const totpSession = { + id: "session-1", + factors: { user: { id: "victim-1", loginName: "victim@example.com" } }, + }; + + beforeEach(async () => { + vi.clearAllMocks(); + const _headers = await import("next/headers"); + vi.mocked(_headers.headers).mockResolvedValue(new Headers() as any); + + mockLoadMostRecentSession = vi.mocked((await import("../session")).loadMostRecentSession); + mockVerifyTOTPRegistration = vi.mocked((await import("@/lib/zitadel")).verifyTOTPRegistration); + mockGetEnrollmentAuthorizationError = vi.mocked((await import("./enrollment-guard")).getEnrollmentAuthorizationError); + + mockLoadMostRecentSession.mockResolvedValue(totpSession); + }); + + test("rejects TOTP enrollment on an unauthenticated (identify-only) session", async () => { + mockGetEnrollmentAuthorizationError.mockResolvedValue( + "You have to authenticate or have a valid User Verification Check", + ); + + const result = await verifyTOTP("123456", "victim@example.com", "org-1"); + + expect(result).toEqual({ error: "You have to authenticate or have a valid User Verification Check" }); + expect(mockGetEnrollmentAuthorizationError).toHaveBeenCalledWith({ + serviceConfig: {}, + session: totpSession, + userId: "victim-1", + }); + expect(mockVerifyTOTPRegistration).not.toHaveBeenCalled(); + }); + + test("verifies TOTP registration when the session is authorized", async () => { + mockGetEnrollmentAuthorizationError.mockResolvedValue(null); + mockVerifyTOTPRegistration.mockResolvedValue({ done: true }); + + const result = await verifyTOTP("123456", "victim@example.com", "org-1"); + + expect(mockVerifyTOTPRegistration).toHaveBeenCalledWith({ + serviceConfig: {}, + code: "123456", + userId: "victim-1", + }); + expect(result).toEqual({ done: true }); + }); + + test("throws when the session has no user id", async () => { + mockLoadMostRecentSession.mockResolvedValue({ id: "session-1", factors: {} }); + + await expect(verifyTOTP("123456")).rejects.toThrow("No user id found in session."); + expect(mockVerifyTOTPRegistration).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/login/src/lib/server/verify.ts b/apps/login/src/lib/server/verify.ts index ec39235a3..382c82a6e 100644 --- a/apps/login/src/lib/server/verify.ts +++ b/apps/login/src/lib/server/verify.ts @@ -27,6 +27,7 @@ import { checkMFAFactors } from "../mfa-helper"; import { getServiceConfig } from "../service-url"; import { loadMostRecentSession } from "../session"; import { createSessionAndUpdateCookie } from "./cookie"; +import { getEnrollmentAuthorizationError } from "./enrollment-guard"; import { getPublicHostWithProtocol } from "./host"; const logger = createLogger("verify"); @@ -41,8 +42,19 @@ export async function verifyTOTP(code: string, loginName?: string, organization? loginName, organization, }, - }).then((session) => { + }).then(async (session) => { if (session?.factors?.user?.id) { + // Enrollment must be authorized: a bare identify-only session must not be able to + // activate a TOTP factor on the account (GHSA-45f2-5q3r-xgg6). + const enrollmentError = await getEnrollmentAuthorizationError({ + serviceConfig, + session, + userId: session.factors.user.id, + }); + if (enrollmentError) { + return { error: enrollmentError }; + } + return verifyTOTPRegistration({ serviceConfig, code, userId: session.factors.user.id }); } else { throw Error("No user id found in session."); diff --git a/apps/login/src/lib/session.ts b/apps/login/src/lib/session.ts index f20cf0fab..a710c2618 100644 --- a/apps/login/src/lib/session.ts +++ b/apps/login/src/lib/session.ts @@ -1,4 +1,4 @@ -import { timestampDate } from "@zitadel/client"; +import { Timestamp, timestampDate } from "@zitadel/client"; import { AuthRequest } from "@zitadel/proto/zitadel/oidc/v2/authorization_pb"; import { SAMLRequest } from "@zitadel/proto/zitadel/saml/v2/authorization_pb"; import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; @@ -51,6 +51,39 @@ export async function loadMostRecentSession({ }); } +/** + * A session proves genuine (primary) authentication when a primary factor — password, + * passkey/WebAuthn or IDP intent — has been verified and the session has not expired. + * + * A WebAuthn factor only counts as a *primary* factor when it was user-verified + * (`userVerified` true, i.e. passwordless login). A presence-only WebAuthn assertion is a + * second-factor (U2F) check and must NOT be treated as primary authentication — mirroring + * how `mfa-helper` distinguishes "authenticated with passkey" from a 2FA WebAuthn check. + * + * This is the shared gate behind both the credential-enrollment guard + * (`getEnrollmentAuthorizationError`) and the MFA-setup page, so an "identify-only" + * session (only `factors.user` set, produced by submitting a login name) is never enough + * to attach a new authenticator (GHSA-45f2-5q3r-xgg6). + */ +export function hasVerifiedPrimaryFactor(session: Partial): { + valid: boolean; + verifiedAt?: Timestamp; +} { + const validPassword = session?.factors?.password?.verifiedAt; + const validPasskey = + session?.factors?.webAuthN?.verifiedAt && !!session?.factors?.webAuthN?.userVerified + ? session?.factors?.webAuthN?.verifiedAt + : undefined; + const validIDP = session?.factors?.intent?.verifiedAt; + + const stillValid = session.expirationDate ? timestampDate(session.expirationDate) > new Date() : true; + + const verifiedAt = validPassword || validPasskey || validIDP; + const valid = !!(verifiedAt && stillValid); + + return { valid, verifiedAt }; +} + /** * mfa is required, session is not valid anymore (e.g. session expired, user logged out, etc.) * to check for mfa for automatically selected session -> const response = await listAuthenticationMethodTypes(userId); diff --git a/internal/api/ui/login/passwordless_prompt_handler.go b/internal/api/ui/login/passwordless_prompt_handler.go index 36a3ede71..8d775dbbd 100644 --- a/internal/api/ui/login/passwordless_prompt_handler.go +++ b/internal/api/ui/login/passwordless_prompt_handler.go @@ -14,18 +14,10 @@ type passwordlessPromptData struct { userData } -type passwordlessPromptFormData struct{} - -func (l *Login) handlePasswordlessPrompt(w http.ResponseWriter, r *http.Request) { - data := new(passwordlessPromptFormData) - authReq, err := l.getAuthRequestAndParseData(r, data) - if err != nil { - l.renderError(w, r, authReq, err) - return - } - l.renderPasswordlessRegistration(w, r, authReq, "", "", "", "", 0, nil) -} - +// renderPasswordlessPrompt shows the informational passwordless prompt page for the +// PasswordlessRegistrationPromptStep, instructing the user to complete setup via the link +// they were emailed. The interactive auth-request based setup that this page used to POST to +// was a never-active leftover and has been removed (GHSA-45f2-5q3r-xgg6). func (l *Login) renderPasswordlessPrompt(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, err error) { translator := l.getTranslator(r.Context(), authReq) data := &passwordlessPromptData{ diff --git a/internal/api/ui/login/passwordless_registration_handler.go b/internal/api/ui/login/passwordless_registration_handler.go index 782d62f1f..090228a1c 100644 --- a/internal/api/ui/login/passwordless_registration_handler.go +++ b/internal/api/ui/login/passwordless_registration_handler.go @@ -80,17 +80,13 @@ func (l *Login) handlePasswordlessRegistration(w http.ResponseWriter, r *http.Re func (l *Login) renderPasswordlessRegistration(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, userID, orgID, codeID, code string, requestedPlatformType authPlatform, err error) { var credentialData string var disabled bool - if authReq != nil { - userID = authReq.UserID - orgID = authReq.UserOrgID - } + // Passwordless registration in the legacy login is only ever driven by an emailed + // init code (userID/orgID/codeID/code carried in the query/form). The auth-request based + // setup path was a never-active leftover and has been removed (GHSA-45f2-5q3r-xgg6); + // authReq is only used for rendering (translator/user data) below. var webAuthNToken *domain.WebAuthNToken if err == nil { - if authReq != nil { - webAuthNToken, err = l.authRepo.BeginPasswordlessSetup(setUserContext(r.Context(), userID, authReq.UserOrgID), userID, authReq.UserOrgID, domain.AuthenticatorAttachment(requestedPlatformType)) - } else { - webAuthNToken, err = l.authRepo.BeginPasswordlessInitCodeSetup(setUserContext(r.Context(), userID, orgID), userID, orgID, codeID, code, domain.AuthenticatorAttachment(requestedPlatformType)) - } + webAuthNToken, err = l.authRepo.BeginPasswordlessInitCodeSetup(setUserContext(r.Context(), userID, orgID), userID, orgID, codeID, code, domain.AuthenticatorAttachment(requestedPlatformType)) } if err != nil { disabled = true @@ -171,11 +167,10 @@ func (l *Login) checkPasswordlessRegistration(w http.ResponseWriter, r *http.Req return } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) - if authReq != nil { - err = l.authRepo.VerifyPasswordlessSetup(setContext(r.Context(), authReq.UserOrgID), formData.UserID, authReq.UserOrgID, userAgentID, formData.TokenName, credData) - } else { - err = l.authRepo.VerifyPasswordlessInitCodeSetup(setContext(r.Context(), formData.OrgID), formData.UserID, formData.OrgID, userAgentID, formData.TokenName, formData.CodeID, formData.Code, credData) - } + // Passwordless setup is verified exclusively through the emailed init code; the + // auth-request based setup path was removed (GHSA-45f2-5q3r-xgg6). A request that + // carries an authRequestID but no valid code therefore fails here. + err = l.authRepo.VerifyPasswordlessInitCodeSetup(setContext(r.Context(), formData.OrgID), formData.UserID, formData.OrgID, userAgentID, formData.TokenName, formData.CodeID, formData.Code, credData) if err != nil { l.renderPasswordlessRegistration(w, r, authReq, formData.UserID, formData.OrgID, formData.CodeID, formData.Code, formData.RequestPlatformType, err) return diff --git a/internal/api/ui/login/renderer.go b/internal/api/ui/login/renderer.go index edd733a41..147fc045f 100644 --- a/internal/api/ui/login/renderer.go +++ b/internal/api/ui/login/renderer.go @@ -154,9 +154,6 @@ func CreateRenderer(pathPrefix string, staticStorage static.Storage, cookieName "passwordLessRegistrationUrl": func() string { return path.Join(r.pathPrefix, EndpointPasswordlessRegistration) }, - "passwordlessPromptUrl": func() string { - return path.Join(r.pathPrefix, EndpointPasswordlessPrompt) - }, "passwordResetUrl": func(id string) string { return path.Join(r.pathPrefix, fmt.Sprintf("%s?%s=%s", EndpointPasswordReset, QueryAuthRequestID, id)) }, diff --git a/internal/api/ui/login/router.go b/internal/api/ui/login/router.go index e2a9b8f1b..27e99e35b 100644 --- a/internal/api/ui/login/router.go +++ b/internal/api/ui/login/router.go @@ -24,7 +24,6 @@ const ( EndpointLDAPCallback = "/login/ldap/callback" EndpointPasswordlessLogin = "/login/passwordless" EndpointPasswordlessRegistration = "/login/passwordless/init" - EndpointPasswordlessPrompt = "/login/passwordless/prompt" EndpointLoginName = "/loginname" EndpointUserSelection = "/userselection" EndpointChangeUsername = "/username/change" @@ -89,7 +88,6 @@ func CreateRouter(login *Login, interceptors ...mux.MiddlewareFunc) *mux.Router router.HandleFunc(EndpointPasswordlessLogin, login.handlePasswordlessVerification).Methods(http.MethodPost) router.HandleFunc(EndpointPasswordlessRegistration, login.handlePasswordlessRegistration).Methods(http.MethodGet) router.HandleFunc(EndpointPasswordlessRegistration, login.handlePasswordlessRegistrationCheck).Methods(http.MethodPost) - router.HandleFunc(EndpointPasswordlessPrompt, login.handlePasswordlessPrompt).Methods(http.MethodPost) router.HandleFunc(EndpointLoginName, login.handleLoginName).Methods(http.MethodGet) router.HandleFunc(EndpointLoginName, login.handleLoginNameCheck).Methods(http.MethodPost) router.HandleFunc(EndpointUserSelection, login.handleSelectUser).Methods(http.MethodPost) diff --git a/internal/api/ui/login/static/templates/passwordless_prompt.html b/internal/api/ui/login/static/templates/passwordless_prompt.html index 6763846b2..0b98d8d2d 100644 --- a/internal/api/ui/login/static/templates/passwordless_prompt.html +++ b/internal/api/ui/login/static/templates/passwordless_prompt.html @@ -7,19 +7,11 @@

{{t "PasswordlessPrompt.DescriptionInit"}}

-
- - {{ .CSRF }} - - - -
- - - - -
- -
+
+ + + + +
{{template "main-bottom" .}} \ No newline at end of file diff --git a/internal/auth/repository/auth_request.go b/internal/auth/repository/auth_request.go index 072681784..6b1ed96c2 100644 --- a/internal/auth/repository/auth_request.go +++ b/internal/auth/repository/auth_request.go @@ -31,8 +31,6 @@ type AuthRequestRepository interface { VerifyMFARecoveryCode(ctx context.Context, userID, resourceOwner, code, authRequestID, userAgentID string, info *domain.BrowserInfo) error BeginMFAU2FLogin(ctx context.Context, userID, resourceOwner, authRequestID, userAgentID string) (*domain.WebAuthNLogin, error) VerifyMFAU2F(ctx context.Context, userID, resourceOwner, authRequestID, userAgentID string, credentialData []byte, info *domain.BrowserInfo) error - BeginPasswordlessSetup(ctx context.Context, userID, resourceOwner string, preferredPlatformType domain.AuthenticatorAttachment) (login *domain.WebAuthNToken, err error) - VerifyPasswordlessSetup(ctx context.Context, userID, resourceOwner, userAgentID, tokenName string, credentialData []byte) (err error) BeginPasswordlessInitCodeSetup(ctx context.Context, userID, resourceOwner, codeID, verificationCode string, preferredPlatformType domain.AuthenticatorAttachment) (login *domain.WebAuthNToken, err error) VerifyPasswordlessInitCodeSetup(ctx context.Context, userID, resourceOwner, userAgentID, tokenName, codeID, verificationCode string, credentialData []byte) (err error) BeginPasswordlessLogin(ctx context.Context, userID, resourceOwner, authRequestID, userAgentID string) (*domain.WebAuthNLogin, error) diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 6de05e472..5bf27bc8d 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -503,19 +503,6 @@ func (repo *AuthRequestRepo) VerifyMFAU2F(ctx context.Context, userID, resourceO return repo.Command.HumanFinishU2FLogin(ctx, userID, resourceOwner, credentialData, request) } -func (repo *AuthRequestRepo) BeginPasswordlessSetup(ctx context.Context, userID, resourceOwner string, authenticatorPlatform domain.AuthenticatorAttachment) (login *domain.WebAuthNToken, err error) { - ctx, span := tracing.NewSpan(ctx) - defer func() { span.EndWithError(err) }() - return repo.Command.HumanAddPasswordlessSetup(ctx, userID, resourceOwner, authenticatorPlatform) -} - -func (repo *AuthRequestRepo) VerifyPasswordlessSetup(ctx context.Context, userID, resourceOwner, userAgentID, tokenName string, credentialData []byte) (err error) { - ctx, span := tracing.NewSpan(ctx) - defer func() { span.EndWithError(err) }() - _, err = repo.Command.HumanHumanPasswordlessSetup(ctx, userID, resourceOwner, tokenName, userAgentID, credentialData) - return err -} - func (repo *AuthRequestRepo) BeginPasswordlessInitCodeSetup(ctx context.Context, userID, resourceOwner, codeID, verificationCode string, preferredPlatformType domain.AuthenticatorAttachment) (login *domain.WebAuthNToken, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }()