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) }()