From 637b370c17e8af0ff17d9a032ae1f0c426474a60 Mon Sep 17 00:00:00 2001 From: Max Peintner Date: Mon, 22 Sep 2025 18:09:20 +0200 Subject: [PATCH] chore(login): Extract auth flow utilities and eliminate RSC request interference (#10644) The /login route was experiencing issues with React Server Component (RSC) requests interfering with one-time authentication callbacks. When users navigated to /login via client-side routing (router.push()), Next.js automatically triggered _rsc requests that could consume single-use createCallback tokens, breaking OIDC and SAML authentication flows. # Which Problems Are Solved When users attempt to log in, Next.js automatically makes requests with the `_rsc=1` query parameter for React Server Components. The current implementation treats these as server errors: ```typescript // Before if (_rsc) { return NextResponse.json({ error: "No _rsc supported" }, { status: 500 }); } ``` This results in: - Spurious 500 error logs polluting monitoring systems - False alerts for server failures - Difficulty distinguishing real issues from benign RSC requests # How the Problems Are Solved This PR implements a comprehensive refactoring that: - Eliminates RSC interference by providing server actions for internal auth flow completion - Separates concerns between external flow initiation and internal flow completion - Extracts shared utilities to improve code maintainability and reusability - Maintains full backward compatibility for external applications # Additional Context ## New Architecture - auth-flow.ts: Shared utilities for auth flow completion with RSC protection - flow-initiation.ts: Extracted OIDC/SAML flow initiation logic (~400 lines) - auth.ts: Server actions for internal components ## Route Handler Simplification - route.ts: Reduced from ~350 lines to ~75 lines - External-only focus: Now handles only flow initiation for external applications - Removed completion logic: External apps use their own callback URLs - Enhanced validation: Early RSC blocking and parameter validation ## Flow Logic Improvements - Early return patterns: Guard clauses eliminate deep nesting - Better error handling: Specific error messages for different failure modes - Fixed SAML flow: Addressed incomplete logic - Consistent session handling: Unified approach across OIDC and SAML --- .../src/app/(login)/password/change/page.tsx | 2 +- apps/login/src/app/login/route.ts | 571 ++---------------- .../choose-second-factor-to-setup.tsx | 50 +- apps/login/src/components/login-otp.tsx | 56 +- apps/login/src/components/password-form.tsx | 24 +- .../register-form-idp-incomplete.tsx | 12 +- apps/login/src/components/register-u2f.tsx | 100 ++- apps/login/src/components/session-item.tsx | 6 +- apps/login/src/components/totp-register.tsx | 72 +-- apps/login/src/lib/auth-utils.ts | 23 + apps/login/src/lib/client.ts | 73 ++- apps/login/src/lib/oidc.ts | 62 +- apps/login/src/lib/saml.ts | 70 +-- apps/login/src/lib/server/auth-flow.ts | 83 +++ apps/login/src/lib/server/flow-initiation.ts | 450 ++++++++++++++ apps/login/src/lib/server/idp.ts | 73 ++- apps/login/src/lib/server/passkeys.ts | 75 +-- apps/login/src/lib/server/password.ts | 109 ++-- apps/login/src/lib/server/register.ts | 40 +- apps/login/src/lib/server/session.ts | 82 ++- apps/login/src/lib/server/verify.ts | 43 +- apps/login/src/lib/session.test.ts | 306 +++++++++- apps/login/src/lib/session.ts | 98 +-- 23 files changed, 1334 insertions(+), 1146 deletions(-) create mode 100644 apps/login/src/lib/auth-utils.ts create mode 100644 apps/login/src/lib/server/auth-flow.ts create mode 100644 apps/login/src/lib/server/flow-initiation.ts diff --git a/apps/login/src/app/(login)/password/change/page.tsx b/apps/login/src/app/(login)/password/change/page.tsx index fe920cda8a1..690c1dcb3f8 100644 --- a/apps/login/src/app/(login)/password/change/page.tsx +++ b/apps/login/src/app/(login)/password/change/page.tsx @@ -62,7 +62,7 @@ export default async function Page(props: { )}

- +

{/* show error only if usernames should be shown to be unknown */} diff --git a/apps/login/src/app/login/route.ts b/apps/login/src/app/login/route.ts index 7b57e1a5e92..2c1d947460c 100644 --- a/apps/login/src/app/login/route.ts +++ b/apps/login/src/app/login/route.ts @@ -1,66 +1,24 @@ import { getAllSessions } from "@/lib/cookies"; -import { idpTypeToSlug } from "@/lib/idp"; -import { loginWithOIDCAndSession } from "@/lib/oidc"; -import { loginWithSAMLAndSession } from "@/lib/saml"; -import { sendLoginname, SendLoginnameCommand } from "@/lib/server/loginname"; -import { constructUrl, getServiceUrlFromHeaders } from "@/lib/service-url"; -import { findValidSession } from "@/lib/session"; -import { - createCallback, - createResponse, - getActiveIdentityProviders, - getAuthRequest, - getOrgsByDomain, - getSAMLRequest, - getSecuritySettings, - listSessions, - startIdentityProviderFlow, -} from "@/lib/zitadel"; -import { create } from "@zitadel/client"; -import { Prompt } from "@zitadel/proto/zitadel/oidc/v2/authorization_pb"; -import { - CreateCallbackRequestSchema, - SessionSchema, -} from "@zitadel/proto/zitadel/oidc/v2/oidc_service_pb"; -import { CreateResponseRequestSchema } from "@zitadel/proto/zitadel/saml/v2/saml_service_pb"; +import { getServiceUrlFromHeaders } from "@/lib/service-url"; +import { + validateAuthRequest, + isRSCRequest +} from "@/lib/auth-utils"; +import { + handleOIDCFlowInitiation, + handleSAMLFlowInitiation, + FlowInitiationParams +} from "@/lib/server/flow-initiation"; +import { listSessions } from "@/lib/zitadel"; import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; -import { IdentityProviderType } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; import { headers } from "next/headers"; import { NextRequest, NextResponse } from "next/server"; -import { DEFAULT_CSP } from "../../../constants/csp"; export const dynamic = "force-dynamic"; export const revalidate = false; export const fetchCache = "default-no-store"; -const gotoAccounts = ({ - request, - requestId, - organization, -}: { - request: NextRequest; - requestId: string; - organization?: string; -}): NextResponse => { - const accountsUrl = constructUrl(request, "/accounts"); - - if (requestId) { - accountsUrl.searchParams.set("requestId", requestId); - } - if (organization) { - accountsUrl.searchParams.set("organization", organization); - } - - return NextResponse.redirect(accountsUrl); -}; - -async function loadSessions({ - serviceUrl, - ids, -}: { - serviceUrl: string; - ids: string[]; -}): Promise { +async function loadSessions({ serviceUrl, ids }: { serviceUrl: string; ids: string[] }): Promise { const response = await listSessions({ serviceUrl, ids: ids.filter((id: string | undefined) => !!id), @@ -69,34 +27,24 @@ async function loadSessions({ return response?.sessions ?? []; } -const ORG_SCOPE_REGEX = /urn:zitadel:iam:org:id:([0-9]+)/; -const ORG_DOMAIN_SCOPE_REGEX = /urn:zitadel:iam:org:domain:primary:(.+)/; // TODO: check regex for all domain character options -const IDP_SCOPE_REGEX = /urn:zitadel:iam:org:idp:id:(.+)/; - export async function GET(request: NextRequest) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); const searchParams = request.nextUrl.searchParams; - const oidcRequestId = searchParams.get("authRequest"); // oidc initiated request - const samlRequestId = searchParams.get("samlRequest"); // saml initiated request + // Defensive check: block RSC requests early + if (isRSCRequest(searchParams)) { + return NextResponse.json({ error: "RSC requests not supported" }, { status: 400 }); + } - // internal request id which combines authRequest and samlRequest with the prefix oidc_ or saml_ - let requestId = - searchParams.get("requestId") ?? - (oidcRequestId - ? `oidc_${oidcRequestId}` - : samlRequestId - ? `saml_${samlRequestId}` - : undefined); - - const sessionId = searchParams.get("sessionId"); - - // TODO: find a better way to handle _rsc (react server components) requests and block them to avoid conflicts when creating oidc callback - const _rsc = searchParams.get("_rsc"); - if (_rsc) { - return NextResponse.json({ error: "No _rsc supported" }, { status: 500 }); + // Early validation: if no valid request parameters, return error immediately + const requestId = validateAuthRequest(searchParams); + if (!requestId) { + return NextResponse.json( + { error: "No valid authentication request found" }, + { status: 400 }, + ); } const sessionCookies = await getAllSessions(); @@ -106,460 +54,29 @@ export async function GET(request: NextRequest) { sessions = await loadSessions({ serviceUrl, ids }); } - // complete flow if session and request id are provided - if (requestId && sessionId) { - if (requestId.startsWith("oidc_")) { - // this finishes the login process for OIDC - return loginWithOIDCAndSession({ - serviceUrl, - authRequest: requestId.replace("oidc_", ""), - sessionId, - sessions, - sessionCookies, - request, - }); - } else if (requestId.startsWith("saml_")) { - // this finishes the login process for SAML - return loginWithSAMLAndSession({ - serviceUrl, - samlRequest: requestId.replace("saml_", ""), - sessionId, - sessions, - sessionCookies, - request, - }); - } - } + // Flow initiation - delegate to appropriate handler + const flowParams: FlowInitiationParams = { + serviceUrl, + requestId, + sessions, + sessionCookies, + request, + }; - // continue with OIDC - if (requestId && requestId.startsWith("oidc_")) { - const { authRequest } = await getAuthRequest({ - serviceUrl, - authRequestId: requestId.replace("oidc_", ""), - }); - - let organization = ""; - let suffix = ""; - let idpId = ""; - - if (authRequest?.scope) { - const orgScope = authRequest.scope.find((s: string) => - ORG_SCOPE_REGEX.test(s), - ); - - const idpScope = authRequest.scope.find((s: string) => - IDP_SCOPE_REGEX.test(s), - ); - - if (orgScope) { - const matched = ORG_SCOPE_REGEX.exec(orgScope); - organization = matched?.[1] ?? ""; - } else { - const orgDomainScope = authRequest.scope.find((s: string) => - ORG_DOMAIN_SCOPE_REGEX.test(s), - ); - - if (orgDomainScope) { - const matched = ORG_DOMAIN_SCOPE_REGEX.exec(orgDomainScope); - const orgDomain = matched?.[1] ?? ""; - if (orgDomain) { - const orgs = await getOrgsByDomain({ - serviceUrl, - domain: orgDomain, - }); - if (orgs.result && orgs.result.length === 1) { - organization = orgs.result[0].id ?? ""; - suffix = orgDomain; - } - } - } - } - - if (idpScope) { - const matched = IDP_SCOPE_REGEX.exec(idpScope); - idpId = matched?.[1] ?? ""; - - const identityProviders = await getActiveIdentityProviders({ - serviceUrl, - orgId: organization ? organization : undefined, - }).then((resp) => { - return resp.identityProviders; - }); - - const idp = identityProviders.find((idp) => idp.id === idpId); - - if (idp) { - const origin = request.nextUrl.origin; - - const identityProviderType = identityProviders[0].type; - - if (identityProviderType === IdentityProviderType.LDAP) { - const ldapUrl = constructUrl(request, "/ldap"); - if (authRequest.id) { - ldapUrl.searchParams.set("requestId", `oidc_${authRequest.id}`); - } - if (organization) { - ldapUrl.searchParams.set("organization", organization); - } - - return NextResponse.redirect(ldapUrl); - } - - let provider = idpTypeToSlug(identityProviderType); - - const params = new URLSearchParams(); - - if (requestId) { - params.set("requestId", requestId); - } - - if (organization) { - params.set("organization", organization); - } - - let url: string | null = await startIdentityProviderFlow({ - serviceUrl, - idpId, - urls: { - successUrl: - `${origin}/idp/${provider}/success?` + - new URLSearchParams(params), - failureUrl: - `${origin}/idp/${provider}/failure?` + - new URLSearchParams(params), - }, - }); - - if (!url) { - return NextResponse.json( - { error: "Could not start IDP flow" }, - { status: 500 }, - ); - } - - if (url.startsWith("/")) { - // if the url is a relative path, construct the absolute url - url = constructUrl(request, url).toString(); - } - - return NextResponse.redirect(url); - } - } - } - - if (authRequest && authRequest.prompt.includes(Prompt.CREATE)) { - const registerUrl = constructUrl(request, "/register"); - if (authRequest.id) { - registerUrl.searchParams.set("requestId", `oidc_${authRequest.id}`); - } - if (organization) { - registerUrl.searchParams.set("organization", organization); - } - - return NextResponse.redirect(registerUrl); - } - - // use existing session and hydrate it for oidc - if (authRequest && sessions.length) { - // if some accounts are available for selection and select_account is set - if (authRequest.prompt.includes(Prompt.SELECT_ACCOUNT)) { - return gotoAccounts({ - request, - requestId: `oidc_${authRequest.id}`, - organization, - }); - } else if (authRequest.prompt.includes(Prompt.LOGIN)) { - /** - * The login prompt instructs the authentication server to prompt the user for re-authentication, regardless of whether the user is already authenticated - */ - - // if a hint is provided, skip loginname page and jump to the next page - if (authRequest.loginHint) { - try { - let command: SendLoginnameCommand = { - loginName: authRequest.loginHint, - requestId: authRequest.id, - }; - - if (organization) { - command = { ...command, organization }; - } - - const res = await sendLoginname(command); - - if (res && "redirect" in res && res?.redirect) { - const absoluteUrl = constructUrl(request, res.redirect); - return NextResponse.redirect(absoluteUrl.toString()); - } - } catch (error) { - console.error("Failed to execute sendLoginname:", error); - } - } - - const loginNameUrl = constructUrl(request, "/loginname"); - if (authRequest.id) { - loginNameUrl.searchParams.set("requestId", `oidc_${authRequest.id}`); - } - if (authRequest.loginHint) { - loginNameUrl.searchParams.set("loginName", authRequest.loginHint); - } - if (organization) { - loginNameUrl.searchParams.set("organization", organization); - } - if (suffix) { - loginNameUrl.searchParams.set("suffix", suffix); - } - return NextResponse.redirect(loginNameUrl); - } else if (authRequest.prompt.includes(Prompt.NONE)) { - /** - * With an OIDC none prompt, the authentication server must not display any authentication or consent user interface pages. - * This means that the user should not be prompted to enter their password again. - * Instead, the server attempts to silently authenticate the user using an existing session or other authentication mechanisms that do not require user interaction - **/ - const securitySettings = await getSecuritySettings({ - serviceUrl, - }); - - const selectedSession = await findValidSession({ - serviceUrl, - sessions, - authRequest, - }); - - const noSessionResponse = NextResponse.json( - { error: "No active session found" }, - { status: 400 }, - ); - - if (securitySettings?.embeddedIframe?.enabled) { - securitySettings.embeddedIframe.allowedOrigins; - noSessionResponse.headers.set( - "Content-Security-Policy", - `${DEFAULT_CSP} frame-ancestors ${securitySettings.embeddedIframe.allowedOrigins.join(" ")};`, - ); - noSessionResponse.headers.delete("X-Frame-Options"); - } - - if (!selectedSession || !selectedSession.id) { - return noSessionResponse; - } - - const cookie = sessionCookies.find( - (cookie) => cookie.id === selectedSession.id, - ); - - if (!cookie || !cookie.id || !cookie.token) { - return noSessionResponse; - } - - const session = { - sessionId: cookie.id, - sessionToken: cookie.token, - }; - - const { callbackUrl } = await createCallback({ - serviceUrl, - req: create(CreateCallbackRequestSchema, { - authRequestId: requestId.replace("oidc_", ""), - callbackKind: { - case: "session", - value: create(SessionSchema, session), - }, - }), - }); - - const callbackResponse = NextResponse.redirect(callbackUrl); - - if (securitySettings?.embeddedIframe?.enabled) { - securitySettings.embeddedIframe.allowedOrigins; - callbackResponse.headers.set( - "Content-Security-Policy", - `${DEFAULT_CSP} frame-ancestors ${securitySettings.embeddedIframe.allowedOrigins.join(" ")};`, - ); - callbackResponse.headers.delete("X-Frame-Options"); - } - - return callbackResponse; - } else { - // check for loginHint, userId hint and valid sessions - let selectedSession = await findValidSession({ - serviceUrl, - sessions, - authRequest, - }); - - if (!selectedSession || !selectedSession.id) { - return gotoAccounts({ - request, - requestId: `oidc_${authRequest.id}`, - organization, - }); - } - - const cookie = sessionCookies.find( - (cookie) => cookie.id === selectedSession.id, - ); - - if (!cookie || !cookie.id || !cookie.token) { - return gotoAccounts({ - request, - requestId: `oidc_${authRequest.id}`, - organization, - }); - } - - const session = { - sessionId: cookie.id, - sessionToken: cookie.token, - }; - - try { - const { callbackUrl } = await createCallback({ - serviceUrl, - req: create(CreateCallbackRequestSchema, { - authRequestId: requestId.replace("oidc_", ""), - callbackKind: { - case: "session", - value: create(SessionSchema, session), - }, - }), - }); - if (callbackUrl) { - return NextResponse.redirect(callbackUrl); - } else { - console.log( - "could not create callback, redirect user to choose other account", - ); - return gotoAccounts({ - request, - organization, - requestId: `oidc_${authRequest.id}`, - }); - } - } catch (error) { - console.error(error); - return gotoAccounts({ - request, - requestId: `oidc_${authRequest.id}`, - organization, - }); - } - } - } else { - const loginNameUrl = constructUrl(request, "/loginname"); - - loginNameUrl.searchParams.set("requestId", requestId); - if (authRequest?.loginHint) { - loginNameUrl.searchParams.set("loginName", authRequest.loginHint); - loginNameUrl.searchParams.set("submit", "true"); // autosubmit - } - - if (organization) { - loginNameUrl.searchParams.append("organization", organization); - // loginNameUrl.searchParams.set("organization", organization); - } - - return NextResponse.redirect(loginNameUrl); - } - } - // continue with SAML - else if (requestId && requestId.startsWith("saml_")) { - const { samlRequest } = await getSAMLRequest({ - serviceUrl, - samlRequestId: requestId.replace("saml_", ""), - }); - - if (!samlRequest) { - return NextResponse.json( - { error: "No samlRequest found" }, - { status: 400 }, - ); - } - - let selectedSession = await findValidSession({ - serviceUrl, - sessions, - samlRequest, - }); - - if (!selectedSession || !selectedSession.id) { - return gotoAccounts({ - request, - requestId: `saml_${samlRequest.id}`, - }); - } - - const cookie = sessionCookies.find( - (cookie) => cookie.id === selectedSession.id, - ); - - if (!cookie || !cookie.id || !cookie.token) { - return gotoAccounts({ - request, - requestId: `saml_${samlRequest.id}`, - // organization, - }); - } - - const session = { - sessionId: cookie.id, - sessionToken: cookie.token, - }; - - try { - const { url, binding } = await createResponse({ - serviceUrl, - req: create(CreateResponseRequestSchema, { - samlRequestId: requestId.replace("saml_", ""), - responseKind: { - case: "session", - value: session, - }, - }), - }); - if (url && binding.case === "redirect") { - return NextResponse.redirect(url); - } else if (url && binding.case === "post") { - // Create HTML form that auto-submits via POST and escape the SAML cookie - const html = ` - - -
- - - -
- - - `; - - return new NextResponse(html, { - headers: { "Content-Type": "text/html" }, - }); - } else { - console.log( - "could not create response, redirect user to choose other account", - ); - return gotoAccounts({ - request, - requestId: `saml_${samlRequest.id}`, - }); - } - } catch (error) { - console.error(error); - return gotoAccounts({ - request, - requestId: `saml_${samlRequest.id}`, - }); - } - } - // Device Authorization does not need to start here as it is handled on the /device endpoint - else { + if (requestId.startsWith("oidc_")) { + return handleOIDCFlowInitiation(flowParams); + } else if (requestId.startsWith("saml_")) { + return handleSAMLFlowInitiation(flowParams); + } else if (requestId.startsWith("device_")) { + // Device Authorization does not need to start here as it is handled on the /device endpoint return NextResponse.json( - { error: "No authRequest nor samlRequest provided" }, - { status: 500 }, + { error: "Device authorization should use /device endpoint" }, + { status: 400 } + ); + } else { + return NextResponse.json( + { error: "Invalid request ID format" }, + { status: 400 } ); } } diff --git a/apps/login/src/components/choose-second-factor-to-setup.tsx b/apps/login/src/components/choose-second-factor-to-setup.tsx index 38599053ffb..d636cb722fc 100644 --- a/apps/login/src/components/choose-second-factor-to-setup.tsx +++ b/apps/login/src/components/choose-second-factor-to-setup.tsx @@ -1,14 +1,13 @@ "use client"; import { skipMFAAndContinueWithNextUrl } from "@/lib/server/session"; -import { - LoginSettings, - SecondFactorType, -} from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; +import { LoginSettings, SecondFactorType } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; import { AuthenticationMethodType } from "@zitadel/proto/zitadel/user/v2/user_service_pb"; import { useRouter } from "next/navigation"; import { EMAIL, SMS, TOTP, U2F } from "./auth-methods"; import { Translated } from "./translated"; +import { useState } from "react"; +import { Alert } from "./alert"; type Props = { userId: string; @@ -40,6 +39,8 @@ export function ChooseSecondFactorToSetup({ const router = useRouter(); const params = new URLSearchParams({}); + const [error, setError] = useState(""); + if (loginName) { params.append("loginName", loginName); } @@ -62,31 +63,15 @@ export function ChooseSecondFactorToSetup({ {loginSettings.secondFactors.map((factor) => { switch (factor) { case SecondFactorType.OTP: - return TOTP( - userMethods.includes(AuthenticationMethodType.TOTP), - "/otp/time-based/set?" + params, - ); + return TOTP(userMethods.includes(AuthenticationMethodType.TOTP), "/otp/time-based/set?" + params); case SecondFactorType.U2F: - return U2F( - userMethods.includes(AuthenticationMethodType.U2F), - "/u2f/set?" + params, - ); + return U2F(userMethods.includes(AuthenticationMethodType.U2F), "/u2f/set?" + params); case SecondFactorType.OTP_EMAIL: return ( - emailVerified && - EMAIL( - userMethods.includes(AuthenticationMethodType.OTP_EMAIL), - "/otp/email/set?" + params, - ) + emailVerified && EMAIL(userMethods.includes(AuthenticationMethodType.OTP_EMAIL), "/otp/email/set?" + params) ); case SecondFactorType.OTP_SMS: - return ( - phoneVerified && - SMS( - userMethods.includes(AuthenticationMethodType.OTP_SMS), - "/otp/sms/set?" + params, - ) - ); + return phoneVerified && SMS(userMethods.includes(AuthenticationMethodType.OTP_SMS), "/otp/sms/set?" + params); default: return null; } @@ -96,7 +81,7 @@ export function ChooseSecondFactorToSetup({ )} + {error && ( +
+ {error} +
+ )} ); } diff --git a/apps/login/src/components/login-otp.tsx b/apps/login/src/components/login-otp.tsx index abd4829792b..d4fa0fccbb2 100644 --- a/apps/login/src/components/login-otp.tsx +++ b/apps/login/src/components/login-otp.tsx @@ -1,6 +1,6 @@ "use client"; -import { getNextUrl } from "@/lib/client"; +import { completeFlowOrGetUrl } from "@/lib/client"; import { updateSession } from "@/lib/server/session"; import { create } from "@zitadel/client"; import { RequestChallengesSchema } from "@zitadel/proto/zitadel/session/v2/challenge_pb"; @@ -33,16 +33,7 @@ type Inputs = { code: string; }; -export function LoginOTP({ - host, - loginName, - sessionId, - requestId, - organization, - method, - code, - loginSettings, -}: Props) { +export function LoginOTP({ host, loginName, sessionId, requestId, organization, method, code, loginSettings }: Props) { const t = useTranslations("otp"); const [error, setError] = useState(""); @@ -190,29 +181,33 @@ export function LoginOTP({ // Wait for 2 seconds to avoid eventual consistency issues with an OTP code being verified in the /login endpoint await new Promise((resolve) => setTimeout(resolve, 2000)); - const url = - requestId && response.sessionId - ? await getNextUrl( - { + // Use unified approach that handles both OIDC/SAML and regular flows + if (response.factors?.user) { + const callbackResponse = await completeFlowOrGetUrl( + requestId && response.sessionId + ? { sessionId: response.sessionId, requestId: requestId, organization: response.factors?.user?.organizationId, + } + : { + loginName: response.factors.user.loginName, + organization: response.factors?.user?.organizationId, }, - loginSettings?.defaultRedirectUri, - ) - : response.factors?.user - ? await getNextUrl( - { - loginName: response.factors.user.loginName, - organization: response.factors?.user?.organizationId, - }, - loginSettings?.defaultRedirectUri, - ) - : null; + loginSettings?.defaultRedirectUri, + ); + setLoading(false); - setLoading(false); - if (url) { - router.push(url); + if ("error" in callbackResponse) { + setError(callbackResponse.error); + return; + } + + if ("redirect" in callbackResponse) { + return router.push(callbackResponse.redirect); + } + } else { + setLoading(false); } } }); @@ -278,8 +273,7 @@ export function LoginOTP({ })} data-testid="submit-button" > - {loading && }{" "} - + {loading && } diff --git a/apps/login/src/components/password-form.tsx b/apps/login/src/components/password-form.tsx index 1bf63e9969e..1b4c73b950f 100644 --- a/apps/login/src/components/password-form.tsx +++ b/apps/login/src/components/password-form.tsx @@ -26,16 +26,11 @@ type Props = { requestId?: string; }; -export function PasswordForm({ - loginSettings, - loginName, - organization, - requestId, -}: Props) { +export function PasswordForm({ loginSettings, loginName, organization, requestId }: Props) { const { register, handleSubmit, formState } = useForm({ mode: "onBlur", }); - + const t = useTranslations("password"); const [info, setInfo] = useState(""); @@ -57,8 +52,9 @@ export function PasswordForm({ }), requestId, }) - .catch(() => { + .catch((error) => { setError("Could not verify password"); + console.error("Error verifying password:", error); return; }) .finally(() => { @@ -137,14 +133,7 @@ export function PasswordForm({ )} - {loginName && ( - - )} + {loginName && } {info && ( @@ -170,8 +159,7 @@ export function PasswordForm({ onClick={handleSubmit(submitPassword)} data-testid="submit-button" > - {loading && }{" "} - + {loading && } diff --git a/apps/login/src/components/register-form-idp-incomplete.tsx b/apps/login/src/components/register-form-idp-incomplete.tsx index 9cefe8d3abb..d6bd128bb76 100644 --- a/apps/login/src/components/register-form-idp-incomplete.tsx +++ b/apps/login/src/components/register-form-idp-incomplete.tsx @@ -1,7 +1,6 @@ "use client"; import { registerUserAndLinkToIDP } from "@/lib/server/register"; -import { useRouter } from "next/navigation"; import { useState } from "react"; import { useTranslations } from "next-intl"; import { FieldValues, useForm } from "react-hook-form"; @@ -60,8 +59,6 @@ export function RegisterFormIDPIncomplete({ const [loading, setLoading] = useState(false); const [error, setError] = useState(""); - const router = useRouter(); - async function submitAndRegister(values: Inputs) { setLoading(true); const response = await registerUserAndLinkToIDP({ @@ -88,11 +85,7 @@ export function RegisterFormIDPIncomplete({ return; } - if (response && "redirect" in response && response.redirect) { - return router.push(response.redirect); - } - - return response; + // If no error, the function has already handled the redirect } const { errors } = formState; @@ -150,8 +143,7 @@ export function RegisterFormIDPIncomplete({ onClick={handleSubmit(submitAndRegister)} data-testid="submit-button" > - {loading && }{" "} - + {loading && } diff --git a/apps/login/src/components/register-u2f.tsx b/apps/login/src/components/register-u2f.tsx index ebaabb83905..44276304b10 100644 --- a/apps/login/src/components/register-u2f.tsx +++ b/apps/login/src/components/register-u2f.tsx @@ -1,7 +1,7 @@ "use client"; import { coerceToArrayBuffer, coerceToBase64Url } from "@/helpers/base64"; -import { getNextUrl } from "@/lib/client"; +import { completeFlowOrGetUrl } from "@/lib/client"; import { addU2F, verifyU2F } from "@/lib/server/u2f"; import { LoginSettings } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; import { RegisterU2FResponse } from "@zitadel/proto/zitadel/user/v2/user_service_pb"; @@ -22,26 +22,14 @@ type Props = { loginSettings?: LoginSettings; }; -export function RegisterU2f({ - loginName, - sessionId, - organization, - requestId, - checkAfter, - loginSettings, -}: Props) { +export function RegisterU2f({ loginName, sessionId, organization, requestId, checkAfter, loginSettings }: Props) { const [error, setError] = useState(""); const [loading, setLoading] = useState(false); const router = useRouter(); - async function submitVerify( - u2fId: string, - passkeyName: string, - publicKeyCredential: any, - sessionId: string, - ) { + async function submitVerify(u2fId: string, passkeyName: string, publicKeyCredential: any, sessionId: string) { setError(""); setLoading(true); const response = await verifyU2F({ @@ -94,24 +82,14 @@ export function RegisterU2f({ const u2fId = u2fResponse.u2fId; const options: CredentialCreationOptions = - (u2fResponse?.publicKeyCredentialCreationOptions as CredentialCreationOptions) ?? - {}; + (u2fResponse?.publicKeyCredentialCreationOptions as CredentialCreationOptions) ?? {}; if (options.publicKey) { - options.publicKey.challenge = coerceToArrayBuffer( - options.publicKey.challenge, - "challenge", - ); - options.publicKey.user.id = coerceToArrayBuffer( - options.publicKey.user.id, - "userid", - ); + options.publicKey.challenge = coerceToArrayBuffer(options.publicKey.challenge, "challenge"); + options.publicKey.user.id = coerceToArrayBuffer(options.publicKey.user.id, "userid"); if (options.publicKey.excludeCredentials) { options.publicKey.excludeCredentials.map((cred: any) => { - cred.id = coerceToArrayBuffer( - cred.id as string, - "excludeCredentials.id", - ); + cred.id = coerceToArrayBuffer(cred.id as string, "excludeCredentials.id"); return cred; }); } @@ -137,10 +115,7 @@ export function RegisterU2f({ rawId: coerceToBase64Url(rawId, "rawId"), type: resp.type, response: { - attestationObject: coerceToBase64Url( - attestationObject, - "attestationObject", - ), + attestationObject: coerceToBase64Url(attestationObject, "attestationObject"), clientDataJSON: coerceToBase64Url(clientDataJSON, "clientDataJSON"), }, }; @@ -170,27 +145,41 @@ export function RegisterU2f({ return router.push(`/u2f?` + paramsToContinue); } else { - const url = - requestId && sessionId - ? await getNextUrl( - { - sessionId: sessionId, - requestId: requestId, - organization: organization, - }, - loginSettings?.defaultRedirectUri, - ) - : loginName - ? await getNextUrl( - { - loginName: loginName, - organization: organization, - }, - loginSettings?.defaultRedirectUri, - ) - : null; - if (url) { - return router.push(url); + if (requestId && sessionId) { + const callbackResponse = await completeFlowOrGetUrl( + { + sessionId: sessionId, + requestId: requestId, + organization: organization, + }, + loginSettings?.defaultRedirectUri, + ); + + if ("error" in callbackResponse) { + setError(callbackResponse.error); + return; + } + + if ("redirect" in callbackResponse) { + return router.push(callbackResponse.redirect); + } + } else if (loginName) { + const callbackResponse = await completeFlowOrGetUrl( + { + loginName: loginName, + organization: organization, + }, + loginSettings?.defaultRedirectUri, + ); + + if ("error" in callbackResponse) { + setError(callbackResponse.error); + return; + } + + if ("redirect" in callbackResponse) { + return router.push(callbackResponse.redirect); + } } } } @@ -216,8 +205,7 @@ export function RegisterU2f({ onClick={submitRegisterAndContinue} data-testid="submit-button" > - {loading && }{" "} - + {loading && } diff --git a/apps/login/src/components/session-item.tsx b/apps/login/src/components/session-item.tsx index 41948282a14..3ae425d0bfa 100644 --- a/apps/login/src/components/session-item.tsx +++ b/apps/login/src/components/session-item.tsx @@ -73,14 +73,10 @@ export function SessionItem({ + + + + + `; + + return new NextResponse(html, { + headers: { "Content-Type": "text/html" }, + }); + } + } catch (error) { + console.error("SAML createResponse failed:", error); + } + + // Final fallback: SAML response creation failed - show account selection + return gotoAccounts({ + request, + requestId, + }); +} \ No newline at end of file diff --git a/apps/login/src/lib/server/idp.ts b/apps/login/src/lib/server/idp.ts index 87f88a7c329..50630dc50bf 100644 --- a/apps/login/src/lib/server/idp.ts +++ b/apps/login/src/lib/server/idp.ts @@ -3,22 +3,20 @@ import { getLoginSettings, getUserByID, + listAuthenticationMethodTypes, startIdentityProviderFlow, startLDAPIdentityProviderFlow, } from "@/lib/zitadel"; import { headers } from "next/headers"; import { redirect } from "next/navigation"; -import { getNextUrl } from "../client"; +import { completeFlowOrGetUrl } from "../client"; import { getServiceUrlFromHeaders } from "../service-url"; -import { checkEmailVerification } from "../verify-helper"; +import { checkEmailVerification, checkMFAFactors } from "../verify-helper"; import { createSessionForIdpAndUpdateCookie } from "./cookie"; export type RedirectToIdpState = { error?: string | null } | undefined; -export async function redirectToIdp( - prevState: RedirectToIdpState, - formData: FormData, -): Promise { +export async function redirectToIdp(prevState: RedirectToIdpState, formData: FormData): Promise { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); const host = _headers.get("host"); @@ -102,9 +100,7 @@ type CreateNewSessionCommand = { requestId?: string; }; -export async function createNewSessionFromIdpIntent( - command: CreateNewSessionCommand, -) { +export async function createNewSessionFromIdpIntent(command: CreateNewSessionCommand) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -143,30 +139,43 @@ export async function createNewSessionFromIdpIntent( return { error: "Could not create session" }; } - const humanUser = - userResponse.user.type.case === "human" - ? userResponse.user.type.value - : undefined; + const humanUser = userResponse.user.type.case === "human" ? userResponse.user.type.value : undefined; // check to see if user was verified - const emailVerificationCheck = checkEmailVerification( - session, - humanUser, - command.organization, - command.requestId, - ); + const emailVerificationCheck = checkEmailVerification(session, humanUser, command.organization, command.requestId); if (emailVerificationCheck?.redirect) { return emailVerificationCheck; } - // TODO: check if user has MFA methods - // const mfaFactorCheck = checkMFAFactors(session, loginSettings, authMethods, organization, requestId); - // if (mfaFactorCheck?.redirect) { - // return mfaFactorCheck; - // } + // check if user has MFA methods + let authMethods; + if (session.factors?.user?.id) { + const response = await listAuthenticationMethodTypes({ + serviceUrl, + userId: session.factors.user.id, + }); + if (response.authMethodTypes && response.authMethodTypes.length) { + authMethods = response.authMethodTypes; + } + } - const url = await getNextUrl( + if (authMethods) { + const mfaFactorCheck = await checkMFAFactors( + serviceUrl, + session, + loginSettings, + authMethods, + command.organization, + command.requestId, + ); + + if (mfaFactorCheck?.redirect) { + return mfaFactorCheck; + } + } + + return completeFlowOrGetUrl( command.requestId && session.id ? { sessionId: session.id, @@ -179,10 +188,6 @@ export async function createNewSessionFromIdpIntent( }, loginSettings?.defaultRedirectUri, ); - - if (url) { - return { redirect: url }; - } } type createNewSessionForLDAPCommand = { @@ -192,9 +197,7 @@ type createNewSessionForLDAPCommand = { link: boolean; }; -export async function createNewSessionForLDAP( - command: createNewSessionForLDAPCommand, -) { +export async function createNewSessionForLDAP(command: createNewSessionForLDAPCommand) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -215,11 +218,7 @@ export async function createNewSessionForLDAP( password: command.password, }); - if ( - !response || - response.nextStep.case !== "idpIntent" || - !response.nextStep.value - ) { + if (!response || response.nextStep.case !== "idpIntent" || !response.nextStep.value) { return { error: "Could not start LDAP identity provider flow" }; } diff --git a/apps/login/src/lib/server/passkeys.ts b/apps/login/src/lib/server/passkeys.ts index ca603471ab2..57ab7136185 100644 --- a/apps/login/src/lib/server/passkeys.ts +++ b/apps/login/src/lib/server/passkeys.ts @@ -18,17 +18,10 @@ import { } from "@zitadel/proto/zitadel/user/v2/user_service_pb"; import { headers } from "next/headers"; import { userAgent } from "next/server"; -import { getNextUrl } from "../client"; -import { - getMostRecentSessionCookie, - getSessionCookieById, - getSessionCookieByLoginName, -} from "../cookies"; +import { completeFlowOrGetUrl } from "../client"; +import { getMostRecentSessionCookie, getSessionCookieById, getSessionCookieByLoginName } from "../cookies"; import { getServiceUrlFromHeaders } from "../service-url"; -import { - checkEmailVerification, - checkUserVerification, -} from "../verify-helper"; +import { checkEmailVerification, checkUserVerification } from "../verify-helper"; import { setSessionAndUpdateCookie } from "./cookie"; type VerifyPasskeyCommand = { @@ -48,9 +41,7 @@ function isSessionValid(session: Partial): { } { const validPassword = session?.factors?.password?.verifiedAt; const validPasskey = session?.factors?.webAuthN?.verifiedAt; - const stillValid = session.expirationDate - ? timestampDate(session.expirationDate) > new Date() - : true; + const stillValid = session.expirationDate ? timestampDate(session.expirationDate) > new Date() : true; const verifiedAt = validPassword || validPasskey; const valid = !!((validPassword || validPasskey) && stillValid); @@ -93,15 +84,12 @@ export async function registerPasskeyLink( // 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", + error: "You have to authenticate or have a valid User Verification Check", }; } // check if a verification was done earlier - const hasValidUserVerificationCheck = await checkUserVerification( - session.session.factors.user.id, - ); + const hasValidUserVerificationCheck = await checkUserVerification(session.session.factors.user.id); if (!hasValidUserVerificationCheck) { return { error: "User Verification Check has to be done" }; @@ -246,41 +234,30 @@ export async function sendPasskey(command: SendPasskeyCommand) { return { error: "User not found in the system" }; } - const humanUser = - userResponse.user.type.case === "human" - ? userResponse.user.type.value - : undefined; + const humanUser = userResponse.user.type.case === "human" ? userResponse.user.type.value : undefined; - const emailVerificationCheck = checkEmailVerification( - session, - humanUser, - organization, - requestId, - ); + const emailVerificationCheck = checkEmailVerification(session, humanUser, organization, requestId); if (emailVerificationCheck?.redirect) { return emailVerificationCheck; } - const url = - requestId && session.id - ? await getNextUrl( - { - sessionId: session.id, - requestId: requestId, - organization: organization, - }, - loginSettings?.defaultRedirectUri, - ) - : session?.factors?.user?.loginName - ? await getNextUrl( - { - loginName: session.factors.user.loginName, - organization: organization, - }, - loginSettings?.defaultRedirectUri, - ) - : null; - - return { redirect: url }; + if (requestId && session.id) { + return completeFlowOrGetUrl( + { + sessionId: session.id, + requestId: requestId, + organization: organization, + }, + loginSettings?.defaultRedirectUri, + ); + } else if (session?.factors?.user?.loginName) { + return completeFlowOrGetUrl( + { + loginName: session.factors.user.loginName, + organization: organization, + }, + loginSettings?.defaultRedirectUri, + ); + } } diff --git a/apps/login/src/lib/server/password.ts b/apps/login/src/lib/server/password.ts index 013255d06f7..be60d0f00ca 100644 --- a/apps/login/src/lib/server/password.ts +++ b/apps/login/src/lib/server/password.ts @@ -1,9 +1,6 @@ "use server"; -import { - createSessionAndUpdateCookie, - setSessionAndUpdateCookie, -} from "@/lib/server/cookie"; +import { createSessionAndUpdateCookie, setSessionAndUpdateCookie } from "@/lib/server/cookie"; import { getLockoutSettings, getLoginSettings, @@ -18,18 +15,12 @@ import { } from "@/lib/zitadel"; import { ConnectError, create, Duration } from "@zitadel/client"; import { createUserServiceClient } from "@zitadel/client/v2"; -import { - Checks, - ChecksSchema, -} from "@zitadel/proto/zitadel/session/v2/session_service_pb"; +import { Checks, ChecksSchema } from "@zitadel/proto/zitadel/session/v2/session_service_pb"; import { LoginSettings } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; import { User, UserState } from "@zitadel/proto/zitadel/user/v2/user_pb"; -import { - AuthenticationMethodType, - SetPasswordRequestSchema, -} from "@zitadel/proto/zitadel/user/v2/user_service_pb"; +import { AuthenticationMethodType, SetPasswordRequestSchema } from "@zitadel/proto/zitadel/user/v2/user_service_pb"; import { headers } from "next/headers"; -import { getNextUrl } from "../client"; +import { completeFlowOrGetUrl } from "../client"; import { getSessionCookieById, getSessionCookieByLoginName } from "../cookies"; import { getServiceUrlFromHeaders } from "../service-url"; import { @@ -61,11 +52,7 @@ export async function resetPassword(command: ResetPasswordCommand) { organizationId: command.organization, }); - if ( - !users.details || - users.details.totalResult !== BigInt(1) || - !users.result[0].userId - ) { + if (!users.details || users.details.totalResult !== BigInt(1) || !users.result[0].userId) { return { error: "Could not send Password Reset Link" }; } const userId = users.result[0].userId; @@ -88,7 +75,7 @@ export type UpdateSessionCommand = { requestId?: string; }; -export async function sendPassword(command: UpdateSessionCommand) { +export async function sendPassword(command: UpdateSessionCommand): Promise<{ error: string } | { redirect: string }> { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -139,18 +126,17 @@ export async function sendPassword(command: UpdateSessionCommand) { return { error: `Failed to authenticate. You had ${error.failedAttempts} of ${lockoutSettings?.maxPasswordAttempts} password attempts.` + - (lockoutSettings?.maxPasswordAttempts && - error.failedAttempts >= lockoutSettings?.maxPasswordAttempts + (lockoutSettings?.maxPasswordAttempts && error.failedAttempts >= lockoutSettings?.maxPasswordAttempts ? "Contact your administrator to unlock your account" : ""), }; } return { error: "Could not create session for user" }; } + } else { + // this is a fake error message to hide that the user does not even exist + return { error: "Could not verify password" }; } - - // this is a fake error message to hide that the user does not even exist - return { error: "Could not verify password" }; } else { loginSettings = await getLoginSettings({ serviceUrl, @@ -188,8 +174,7 @@ export async function sendPassword(command: UpdateSessionCommand) { return { error: `Failed to authenticate. You had ${error.failedAttempts} of ${lockoutSettings?.maxPasswordAttempts} password attempts.` + - (lockoutSettings?.maxPasswordAttempts && - error.failedAttempts >= lockoutSettings?.maxPasswordAttempts + (lockoutSettings?.maxPasswordAttempts && error.failedAttempts >= lockoutSettings?.maxPasswordAttempts ? " Contact your administrator to unlock your account" : ""), }; @@ -216,12 +201,11 @@ export async function sendPassword(command: UpdateSessionCommand) { if (!loginSettings) { loginSettings = await getLoginSettings({ serviceUrl, - organization: - command.organization ?? session.factors?.user?.organizationId, + organization: command.organization ?? session.factors?.user?.organizationId, }); } - if (!session?.factors?.user?.id || !sessionCookie) { + if (!session?.factors?.user?.id) { return { error: "Could not create session for user" }; } @@ -251,12 +235,7 @@ export async function sendPassword(command: UpdateSessionCommand) { } // check to see if user was verified - const emailVerificationCheck = checkEmailVerification( - session, - humanUser, - command.organization, - command.requestId, - ); + const emailVerificationCheck = checkEmailVerification(session, humanUser, command.organization, command.requestId); if (emailVerificationCheck?.redirect) { return emailVerificationCheck; @@ -292,36 +271,49 @@ export async function sendPassword(command: UpdateSessionCommand) { } if (command.requestId && session.id) { - const nextUrl = await getNextUrl( + // OIDC/SAML flow - use completeFlowOrGetUrl for proper handling + console.log("Password auth: OIDC/SAML flow with requestId:", command.requestId, "sessionId:", session.id); + const result = await completeFlowOrGetUrl( { sessionId: session.id, requestId: command.requestId, - organization: - command.organization ?? session.factors?.user?.organizationId, + organization: command.organization ?? session.factors?.user?.organizationId, }, loginSettings?.defaultRedirectUri, ); + console.log("Password auth: OIDC/SAML flow result:", result); - return { redirect: nextUrl }; + // Safety net - ensure we always return a valid object + if (!result || typeof result !== "object" || (!("redirect" in result) && !("error" in result))) { + console.error("Password auth: Invalid result from completeFlowOrGetUrl (OIDC/SAML):", result); + return { error: "Authentication completed but navigation failed" }; + } + + return result; } - const url = await getNextUrl( + // Regular flow (no requestId) - return URL for client-side navigation + console.log("Password auth: Regular flow with loginName:", session.factors.user.loginName); + const result = await completeFlowOrGetUrl( { loginName: session.factors.user.loginName, organization: session.factors?.user?.organizationId, }, loginSettings?.defaultRedirectUri, ); + console.log("Password auth: Regular flow result:", result); - return { redirect: url }; + // Safety net - ensure we always return a valid object + if (!result || typeof result !== "object" || (!("redirect" in result) && !("error" in result))) { + console.error("Password auth: Invalid result from completeFlowOrGetUrl:", result); + return { error: "Authentication completed but navigation failed" }; + } + + return result; } // this function lets users with code set a password or users with valid User Verification Check -export async function changePassword(command: { - code?: string; - userId: string; - password: string; -}) { +export async function changePassword(command: { code?: string; userId: string; password: string }) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -350,15 +342,12 @@ export async function changePassword(command: { // 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 provide a code or have a valid User Verification Check", + error: "You have to provide a code or have a valid User Verification Check", }; } // check if a verification was done earlier - const hasValidUserVerificationCheck = await checkUserVerification( - user.userId, - ); + const hasValidUserVerificationCheck = await checkUserVerification(user.userId); if (!hasValidUserVerificationCheck) { return { error: "User Verification Check has to be done" }; @@ -378,10 +367,7 @@ type CheckSessionAndSetPasswordCommand = { password: string; }; -export async function checkSessionAndSetPassword({ - sessionId, - password, -}: CheckSessionAndSetPasswordCommand) { +export async function checkSessionAndSetPassword({ sessionId, password }: CheckSessionAndSetPasswordCommand) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -421,18 +407,14 @@ export async function checkSessionAndSetPassword({ AuthenticationMethodType.U2F, ]; - const hasNoMFAMethods = requiredAuthMethodsForForceMFA.every( - (method) => !authmethods.authMethodTypes.includes(method), - ); + const hasNoMFAMethods = requiredAuthMethodsForForceMFA.every((method) => !authmethods.authMethodTypes.includes(method)); const loginSettings = await getLoginSettings({ serviceUrl, organization: session.factors.user.organizationId, }); - const forceMfa = !!( - loginSettings?.forceMfa || loginSettings?.forceMfaLocalOnly - ); + const forceMfa = !!(loginSettings?.forceMfa || loginSettings?.forceMfaLocalOnly); // if the user has no MFA but MFA is enforced, we can set a password otherwise we use the token of the user if (forceMfa && hasNoMFAMethods) { @@ -454,10 +436,7 @@ export async function checkSessionAndSetPassword({ return createUserServiceClient(transportPromise); }; - const selfService = await myUserService( - serviceUrl, - `${sessionCookie.token}`, - ); + const selfService = await myUserService(serviceUrl, `${sessionCookie.token}`); return selfService .setPassword( diff --git a/apps/login/src/lib/server/register.ts b/apps/login/src/lib/server/register.ts index f84b4c8d512..a41d6a46873 100644 --- a/apps/login/src/lib/server/register.ts +++ b/apps/login/src/lib/server/register.ts @@ -1,23 +1,12 @@ "use server"; -import { - createSessionAndUpdateCookie, - createSessionForIdpAndUpdateCookie, -} from "@/lib/server/cookie"; -import { - addHumanUser, - addIDPLink, - getLoginSettings, - getUserByID, -} from "@/lib/zitadel"; +import { createSessionAndUpdateCookie, createSessionForIdpAndUpdateCookie } from "@/lib/server/cookie"; +import { addHumanUser, addIDPLink, getLoginSettings, getUserByID } from "@/lib/zitadel"; import { create } from "@zitadel/client"; import { Factors } from "@zitadel/proto/zitadel/session/v2/session_pb"; -import { - ChecksJson, - ChecksSchema, -} from "@zitadel/proto/zitadel/session/v2/session_service_pb"; +import { ChecksJson, ChecksSchema } from "@zitadel/proto/zitadel/session/v2/session_service_pb"; import { headers } from "next/headers"; -import { getNextUrl } from "../client"; +import { completeFlowOrGetUrl } from "../client"; import { getServiceUrlFromHeaders } from "../service-url"; import { checkEmailVerification } from "../verify-helper"; @@ -78,9 +67,7 @@ export async function registerUser(command: RegisterUserCommand) { const session = await createSessionAndUpdateCookie({ checks, requestId: command.requestId, - lifetime: command.password - ? loginSettings?.passwordCheckLifetime - : undefined, + lifetime: command.password ? loginSettings?.passwordCheckLifetime : undefined, }); if (!session || !session.factors?.user) { @@ -108,10 +95,7 @@ export async function registerUser(command: RegisterUserCommand) { return { error: "User not found in the system" }; } - const humanUser = - userResponse.user.type.case === "human" - ? userResponse.user.type.value - : undefined; + const humanUser = userResponse.user.type.case === "human" ? userResponse.user.type.value : undefined; const emailVerificationCheck = checkEmailVerification( session, @@ -124,7 +108,7 @@ export async function registerUser(command: RegisterUserCommand) { return emailVerificationCheck; } - const url = await getNextUrl( + return completeFlowOrGetUrl( command.requestId && session.id ? { sessionId: session.id, @@ -137,8 +121,6 @@ export async function registerUser(command: RegisterUserCommand) { }, loginSettings?.defaultRedirectUri, ); - - return { redirect: url }; } } @@ -162,9 +144,7 @@ export type registerUserAndLinkToIDPResponse = { sessionId: string; factors: Factors | undefined; }; -export async function registerUserAndLinkToIDP( - command: RegisterUserAndLinkToIDPommand, -) { +export async function registerUserAndLinkToIDP(command: RegisterUserAndLinkToIDPommand) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); const host = _headers.get("host"); @@ -215,7 +195,7 @@ export async function registerUserAndLinkToIDP( return { error: "Could not create session" }; } - const url = await getNextUrl( + return completeFlowOrGetUrl( command.requestId && session.id ? { sessionId: session.id, @@ -228,6 +208,4 @@ export async function registerUserAndLinkToIDP( }, loginSettings?.defaultRedirectUri, ); - - return { redirect: url }; } diff --git a/apps/login/src/lib/server/session.ts b/apps/login/src/lib/server/session.ts index 7f8e9e46e9f..de045534c57 100644 --- a/apps/login/src/lib/server/session.ts +++ b/apps/login/src/lib/server/session.ts @@ -13,7 +13,7 @@ import { RequestChallenges } from "@zitadel/proto/zitadel/session/v2/challenge_p import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; import { Checks } from "@zitadel/proto/zitadel/session/v2/session_service_pb"; import { headers } from "next/headers"; -import { getNextUrl } from "../client"; +import { completeFlowOrGetUrl } from "../client"; import { getMostRecentSessionCookie, getSessionCookieById, @@ -34,7 +34,7 @@ export async function skipMFAAndContinueWithNextUrl({ sessionId?: string; requestId?: string; organization?: string; -}) { +}): Promise<{ redirect: string } | { error: string }> { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -45,28 +45,26 @@ export async function skipMFAAndContinueWithNextUrl({ await humanMFAInitSkipped({ serviceUrl, userId }); - const url = - requestId && sessionId - ? await getNextUrl( - { - sessionId: sessionId, - requestId: requestId, - organization: organization, - }, - loginSettings?.defaultRedirectUri, - ) - : loginName - ? await getNextUrl( - { - loginName: loginName, - organization: organization, - }, - loginSettings?.defaultRedirectUri, - ) - : null; - if (url) { - return { redirect: url }; + if (requestId && sessionId) { + return completeFlowOrGetUrl( + { + sessionId: sessionId, + requestId: requestId, + organization: organization, + }, + loginSettings?.defaultRedirectUri, + ); + } else if (loginName) { + return completeFlowOrGetUrl( + { + loginName: loginName, + organization: organization, + }, + loginSettings?.defaultRedirectUri, + ); } + + return { error: "Could not skip MFA and continue" }; } export async function continueWithSession({ requestId, ...session }: Session & { requestId?: string }) { @@ -78,27 +76,23 @@ export async function continueWithSession({ requestId, ...session }: Session & { organization: session.factors?.user?.organizationId, }); - const url = - requestId && session.id && session.factors?.user - ? await getNextUrl( - { - sessionId: session.id, - requestId: requestId, - organization: session.factors.user.organizationId, - }, - loginSettings?.defaultRedirectUri, - ) - : session.factors?.user - ? await getNextUrl( - { - loginName: session.factors.user.loginName, - organization: session.factors.user.organizationId, - }, - loginSettings?.defaultRedirectUri, - ) - : null; - if (url) { - return { redirect: url }; + if (requestId && session.id && session.factors?.user) { + return completeFlowOrGetUrl( + { + sessionId: session.id, + requestId: requestId, + organization: session.factors.user.organizationId, + }, + loginSettings?.defaultRedirectUri, + ); + } else if (session.factors?.user) { + return completeFlowOrGetUrl( + { + loginName: session.factors.user.loginName, + organization: session.factors.user.organizationId, + }, + loginSettings?.defaultRedirectUri, + ); } } diff --git a/apps/login/src/lib/server/verify.ts b/apps/login/src/lib/server/verify.ts index cf60f739b3c..2adc08c1eec 100644 --- a/apps/login/src/lib/server/verify.ts +++ b/apps/login/src/lib/server/verify.ts @@ -17,7 +17,7 @@ import { create } from "@zitadel/client"; import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; import { ChecksSchema } from "@zitadel/proto/zitadel/session/v2/session_service_pb"; import { cookies, headers } from "next/headers"; -import { getNextUrl } from "../client"; +import { completeFlowOrGetUrl } from "../client"; import { getSessionCookieByLoginName } from "../cookies"; import { getOrSetFingerprintId } from "../fingerprint"; import { getServiceUrlFromHeaders } from "../service-url"; @@ -25,11 +25,7 @@ import { loadMostRecentSession } from "../session"; import { checkMFAFactors } from "../verify-helper"; import { createSessionAndUpdateCookie } from "./cookie"; -export async function verifyTOTP( - code: string, - loginName?: string, - organization?: string, -) { +export async function verifyTOTP(code: string, loginName?: string, organization?: string) { const _headers = await headers(); const { serviceUrl } = getServiceUrlFromHeaders(_headers); @@ -104,8 +100,7 @@ export async function sendVerification(command: VerifyUserByEmailCommand) { const user = userResponse.user; const sessionCookie = await getSessionCookieByLoginName({ - loginName: - "loginName" in command ? command.loginName : user.preferredLoginName, + loginName: "loginName" in command ? command.loginName : user.preferredLoginName, organization: command.organization, }).catch((error) => { console.warn("Ignored error:", error); // checked later @@ -134,11 +129,7 @@ export async function sendVerification(command: VerifyUserByEmailCommand) { } // if no authmethods are found on the user, redirect to set one up - if ( - authMethodResponse && - authMethodResponse.authMethodTypes && - authMethodResponse.authMethodTypes.length == 0 - ) { + if (authMethodResponse && authMethodResponse.authMethodTypes && authMethodResponse.authMethodTypes.length == 0) { if (!sessionCookie) { const checks = create(ChecksSchema, { user: { @@ -171,10 +162,7 @@ export async function sendVerification(command: VerifyUserByEmailCommand) { const cookiesList = await cookies(); const userAgentId = await getOrSetFingerprintId(); - const verificationCheck = crypto - .createHash("sha256") - .update(`${user.userId}:${userAgentId}`) - .digest("hex"); + const verificationCheck = crypto.createHash("sha256").update(`${user.userId}:${userAgentId}`).digest("hex"); await cookiesList.set({ name: "verificationCheck", @@ -196,15 +184,10 @@ export async function sendVerification(command: VerifyUserByEmailCommand) { verifySuccessParams.set("userId", command.userId); } - if ( - ("loginName" in command && command.loginName) || - user.preferredLoginName - ) { + if (("loginName" in command && command.loginName) || user.preferredLoginName) { verifySuccessParams.set( "loginName", - "loginName" in command && command.loginName - ? command.loginName - : user.preferredLoginName, + "loginName" in command && command.loginName ? command.loginName : user.preferredLoginName, ); } if (command.requestId) { @@ -238,28 +221,24 @@ export async function sendVerification(command: VerifyUserByEmailCommand) { // login user if no additional steps are required if (command.requestId && session.id) { - const nextUrl = await getNextUrl( + return completeFlowOrGetUrl( { sessionId: session.id, requestId: command.requestId, - organization: - command.organization ?? session.factors?.user?.organizationId, + organization: command.organization ?? session.factors?.user?.organizationId, }, loginSettings?.defaultRedirectUri, ); - - return { redirect: nextUrl }; } - const url = await getNextUrl( + // Regular flow - return URL for client-side navigation + return completeFlowOrGetUrl( { loginName: session.factors.user.loginName, organization: session.factors?.user?.organizationId, }, loginSettings?.defaultRedirectUri, ); - - return { redirect: url }; } type resendVerifyEmailCommand = { diff --git a/apps/login/src/lib/session.test.ts b/apps/login/src/lib/session.test.ts index 80ca32854f7..2be64a65860 100644 --- a/apps/login/src/lib/session.test.ts +++ b/apps/login/src/lib/session.test.ts @@ -1,6 +1,6 @@ /** * Unit tests for the isSessionValid function. - * + * * This test suite covers the comprehensive session validation logic including: * - Session expiration checks * - User presence validation @@ -113,10 +113,7 @@ describe("isSessionValid", () => { const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); expect(result).toBe(false); - expect(consoleSpy).toHaveBeenCalledWith( - "Session is expired", - expect.any(String) - ); + expect(consoleSpy).toHaveBeenCalledWith("Session is expired", expect.any(String)); consoleSpy.mockRestore(); }); }); @@ -147,7 +144,7 @@ describe("isSessionValid", () => { }); describe("MFA validation with configured authentication methods", () => { - test("should return true when TOTP is configured and verified", async () => { + test("should return true when TOTP is configured and verified with MFA required", async () => { const verifiedTimestamp = createMockTimestamp(); const session = createMockSession({ factors: { @@ -167,8 +164,9 @@ describe("isSessionValid", () => { }, }); - vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({ - authMethodTypes: [AuthenticationMethodType.TOTP], + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: true, + forceMfaLocalOnly: false, } as any); const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); @@ -176,7 +174,35 @@ describe("isSessionValid", () => { expect(result).toBe(true); }); - test("should return false when TOTP is configured but not verified", async () => { + test("should return true when TOTP is configured but not verified and MFA is not required", async () => { + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + password: { + verifiedAt: verifiedTimestamp, + }, + // No TOTP verification + }, + }); + + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: false, + forceMfaLocalOnly: false, + } as any); + + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + expect(result).toBe(true); + }); + + test("should return false when TOTP is configured but not verified and MFA is required", async () => { const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const verifiedTimestamp = createMockTimestamp(); const session = createMockSession({ @@ -195,28 +221,19 @@ describe("isSessionValid", () => { }, }); - vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({ - authMethodTypes: [AuthenticationMethodType.TOTP], + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: true, + forceMfaLocalOnly: false, } as any); const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); expect(result).toBe(false); - expect(consoleSpy).toHaveBeenCalledWith( - "Session has no valid MFA factor. Configured methods:", - [AuthenticationMethodType.TOTP], - "Session factors:", - expect.objectContaining({ - totp: undefined, - otpEmail: undefined, - otpSms: undefined, - webAuthN: undefined, - }) - ); + expect(consoleSpy).toHaveBeenCalledWith("Session has no valid multifactor", expect.any(Object)); consoleSpy.mockRestore(); }); - test("should return true when OTP Email is configured and verified", async () => { + test("should return true when OTP Email is configured and verified with MFA required", async () => { const verifiedTimestamp = createMockTimestamp(); const session = createMockSession({ factors: { @@ -274,7 +291,7 @@ describe("isSessionValid", () => { expect(result).toBe(true); }); - test("should return true when multiple auth methods are configured and one is verified", async () => { + test("should return true when multiple auth methods are configured and one is verified with MFA required", async () => { const verifiedTimestamp = createMockTimestamp(); const session = createMockSession({ factors: { @@ -295,6 +312,11 @@ describe("isSessionValid", () => { }, }); + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: true, + forceMfaLocalOnly: false, + } as any); + vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({ authMethodTypes: [AuthenticationMethodType.TOTP, AuthenticationMethodType.OTP_EMAIL], } as any); @@ -303,6 +325,195 @@ describe("isSessionValid", () => { expect(result).toBe(true); }); + + test("should return true when session has only password and MFA is not required by policy", async () => { + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + password: { + verifiedAt: verifiedTimestamp, + }, + // No MFA factors verified + }, + }); + + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: false, + forceMfaLocalOnly: false, + } as any); + + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + expect(result).toBe(true); + }); + + test("should return true when user has PASSWORD and TOTP configured but only password verified and MFA not required", async () => { + // This test specifically covers the original bug scenario: + // - User has PASSWORD and TOTP configured (would show up in listAuthenticationMethodTypes) + // - User has only verified password, not TOTP + // - MFA is not required by policy + // - Session should be valid (this was the bug - it was returning false) + + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + password: { + verifiedAt: verifiedTimestamp, + }, + // TOTP is configured but NOT verified - this is the key part + // totp: undefined (no verifiedAt) + }, + }); + + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: false, + forceMfaLocalOnly: false, + } as any); + + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + expect(result).toBe(true); + }); + + test("should return false when user has PASSWORD and TOTP configured but only password verified and MFA IS required", async () => { + // This is the counterpart test to ensure MFA is still enforced when required + + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + password: { + verifiedAt: verifiedTimestamp, + }, + // TOTP is configured but NOT verified + // totp: undefined (no verifiedAt) + }, + }); + + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: true, + forceMfaLocalOnly: false, + } as any); + + vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({ + authMethodTypes: [AuthenticationMethodType.TOTP], + } as any); + + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + expect(result).toBe(false); + expect(consoleSpy).toHaveBeenCalledWith( + "Session has no valid MFA factor. Configured methods:", + [AuthenticationMethodType.TOTP], + "Session factors:", + expect.objectContaining({ + totp: undefined, + otpEmail: undefined, + otpSms: undefined, + webAuthN: undefined, + }), + ); + consoleSpy.mockRestore(); + }); + + test("REGRESSION TEST: user with only PASSWORD factor should be valid when MFA not required", async () => { + // This test specifically verifies the original bug is fixed + // Original bug: A user with only PASSWORD authentication would be invalid + // because the code checked if authMethods.length > 0 (which included PASSWORD) + // and then required MFA verification even when MFA was not required by policy + + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + password: { + verifiedAt: verifiedTimestamp, + }, + // Explicitly no MFA factors at all + totp: undefined, + otpEmail: undefined, + otpSms: undefined, + webAuthN: undefined, + intent: undefined, + }, + }); + + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: false, + forceMfaLocalOnly: false, + } as any); + + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + // This should be true - if it's false, the original bug still exists + expect(result).toBe(true); + }); + + test("DEMONSTRATION: how the original bug would manifest with old logic", async () => { + // This test demonstrates the original problematic scenario: + // 1. listAuthenticationMethodTypes returns [PASSWORD, TOTP] + // 2. Old logic would check authMethods.length > 0 (true because PASSWORD is included) + // 3. Old logic would then require MFA verification regardless of policy + // 4. User has only password verified, no TOTP + // 5. Session would be marked invalid even though MFA is not required + + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + password: { + verifiedAt: verifiedTimestamp, + }, + // User has TOTP configured but not verified + totp: undefined, + }, + }); + + // MFA is NOT required by policy + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: false, + forceMfaLocalOnly: false, + } as any); + + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + // With our fix, this should be true (session is valid) + // With the old logic, this would have been false (bug) + expect(result).toBe(true); + }); }); describe("MFA validation with login settings (no configured auth methods)", () => { @@ -368,10 +579,7 @@ describe("isSessionValid", () => { const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); expect(result).toBe(false); - expect(consoleSpy).toHaveBeenCalledWith( - "Session has no valid multifactor", - expect.any(Object) - ); + expect(consoleSpy).toHaveBeenCalledWith("Session has no valid multifactor", expect.any(Object)); consoleSpy.mockRestore(); }); @@ -493,7 +701,7 @@ describe("isSessionValid", () => { expect(result).toBe(false); expect(consoleSpy).toHaveBeenCalledWith( "Session invalid: Email not verified and EMAIL_VERIFICATION is enabled", - mockUserId + mockUserId, ); consoleSpy.mockRestore(); }); @@ -647,6 +855,44 @@ describe("isSessionValid", () => { expect(result).toBe(true); }); + + test("should return true when authenticated with IDP intent even with forced MFA", async () => { + const verifiedTimestamp = createMockTimestamp(); + const session = createMockSession({ + factors: { + user: { + id: mockUserId, + organizationId: mockOrganizationId, + loginName: "test@example.com", + displayName: "Test User", + verifiedAt: verifiedTimestamp, + }, + intent: { + verifiedAt: verifiedTimestamp, + }, + // No password factor, no MFA factors + }, + }); + + // Organization enforces MFA + vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({ + forceMfa: true, + forceMfaLocalOnly: false, + } as any); + + // User has MFA methods configured but none verified + vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({ + authMethodTypes: [AuthenticationMethodType.TOTP, AuthenticationMethodType.OTP_EMAIL], + } as any); + + // Should still return true because IDP bypasses MFA requirements + const result = await isSessionValid({ serviceUrl: mockServiceUrl, session }); + + expect(result).toBe(true); + // Verify that getLoginSettings was not called since IDP should bypass MFA check entirely + expect(zitadelModule.getLoginSettings).not.toHaveBeenCalled(); + expect(zitadelModule.listAuthenticationMethodTypes).not.toHaveBeenCalled(); + }); }); describe("edge cases", () => { @@ -682,4 +928,4 @@ describe("isSessionValid", () => { expect(result).toBe(true); }); }); -}); \ No newline at end of file +}); diff --git a/apps/login/src/lib/session.ts b/apps/login/src/lib/session.ts index 45c3274c514..77bf59616fd 100644 --- a/apps/login/src/lib/session.ts +++ b/apps/login/src/lib/session.ts @@ -44,55 +44,76 @@ export async function isSessionValid({ serviceUrl, session }: { serviceUrl: stri let mfaValid = true; - const authMethodTypes = await listAuthenticationMethodTypes({ - serviceUrl, - userId: session.factors.user.id, - }); - - const authMethods = authMethodTypes.authMethodTypes; - if (authMethods && authMethods.length > 0) { - // Check if any of the configured authentication methods have been verified - const totpValid = authMethods.includes(AuthenticationMethodType.TOTP) && !!session.factors.totp?.verifiedAt; - const otpEmailValid = authMethods.includes(AuthenticationMethodType.OTP_EMAIL) && !!session.factors.otpEmail?.verifiedAt; - const otpSmsValid = authMethods.includes(AuthenticationMethodType.OTP_SMS) && !!session.factors.otpSms?.verifiedAt; - const u2fValid = authMethods.includes(AuthenticationMethodType.U2F) && !!session.factors.webAuthN?.verifiedAt; - - mfaValid = totpValid || otpEmailValid || otpSmsValid || u2fValid; - - if (!mfaValid) { - console.warn("Session has no valid MFA factor. Configured methods:", authMethods, "Session factors:", { - totp: session.factors.totp?.verifiedAt, - otpEmail: session.factors.otpEmail?.verifiedAt, - otpSms: session.factors.otpSms?.verifiedAt, - webAuthN: session.factors.webAuthN?.verifiedAt - }); - } + // Check if user authenticated via IDP - if so, skip MFA validation entirely + const validIDP = session?.factors?.intent?.verifiedAt; + if (validIDP) { + // IDP authentication bypasses MFA requirements + mfaValid = true; } else { - // only check settings if no auth methods are available, as this would require a setup + // Get login settings to determine if MFA is actually required by policy const loginSettings = await getLoginSettings({ serviceUrl, organization: session.factors?.user?.organizationId, }); - if (loginSettings?.forceMfa || loginSettings?.forceMfaLocalOnly) { - const otpEmail = session.factors.otpEmail?.verifiedAt; - const otpSms = session.factors.otpSms?.verifiedAt; - const totp = session.factors.totp?.verifiedAt; - const webAuthN = session.factors.webAuthN?.verifiedAt; - const idp = session.factors.intent?.verifiedAt; // TODO: forceMFA should not consider this as valid factor - // must have one single check - mfaValid = !!(otpEmail || otpSms || totp || webAuthN || idp); - if (!mfaValid) { - console.warn("Session has no valid multifactor", session.factors); + const isMfaRequired = loginSettings?.forceMfa || loginSettings?.forceMfaLocalOnly; + + // Only enforce MFA validation if MFA is required by policy + if (isMfaRequired) { + const authMethodTypes = await listAuthenticationMethodTypes({ + serviceUrl, + userId: session.factors.user.id, + }); + + const authMethods = authMethodTypes.authMethodTypes; + // Filter to only MFA methods (exclude PASSWORD and PASSKEY) + const mfaMethods = authMethods?.filter( + (method) => + method === AuthenticationMethodType.TOTP || + method === AuthenticationMethodType.OTP_EMAIL || + method === AuthenticationMethodType.OTP_SMS || + method === AuthenticationMethodType.U2F, + ); + + if (mfaMethods && mfaMethods.length > 0) { + // Check if any of the configured MFA methods have been verified + const totpValid = mfaMethods.includes(AuthenticationMethodType.TOTP) && !!session.factors.totp?.verifiedAt; + const otpEmailValid = + mfaMethods.includes(AuthenticationMethodType.OTP_EMAIL) && !!session.factors.otpEmail?.verifiedAt; + const otpSmsValid = mfaMethods.includes(AuthenticationMethodType.OTP_SMS) && !!session.factors.otpSms?.verifiedAt; + const u2fValid = mfaMethods.includes(AuthenticationMethodType.U2F) && !!session.factors.webAuthN?.verifiedAt; + + mfaValid = totpValid || otpEmailValid || otpSmsValid || u2fValid; + + if (!mfaValid) { + console.warn("Session has no valid MFA factor. Configured methods:", mfaMethods, "Session factors:", { + totp: session.factors.totp?.verifiedAt, + otpEmail: session.factors.otpEmail?.verifiedAt, + otpSms: session.factors.otpSms?.verifiedAt, + webAuthN: session.factors.webAuthN?.verifiedAt, + }); + } + } else { + // No specific MFA methods configured, but MFA is forced - check for any verified MFA factors + // (excluding IDP which should be handled separately) + const otpEmail = session.factors.otpEmail?.verifiedAt; + const otpSms = session.factors.otpSms?.verifiedAt; + const totp = session.factors.totp?.verifiedAt; + const webAuthN = session.factors.webAuthN?.verifiedAt; + // Note: Removed IDP (session.factors.intent?.verifiedAt) as requested + + mfaValid = !!(otpEmail || otpSms || totp || webAuthN); + if (!mfaValid) { + console.warn("Session has no valid multifactor", session.factors); + } } - } else { - mfaValid = true; } } + // If MFA is not required by policy, mfaValid remains true const validPassword = session?.factors?.password?.verifiedAt; const validPasskey = session?.factors?.webAuthN?.verifiedAt; - const validIDP = session?.factors?.intent?.verifiedAt; + // validIDP already declared above for IDP bypass logic const stillValid = session.expirationDate ? timestampDate(session.expirationDate).getTime() > new Date().getTime() : true; @@ -151,7 +172,8 @@ export async function findValidSession({ return s.factors?.user?.loginName === authRequest.loginHint; } if (samlRequest) { - // TODO: do whatever + // SAML requests don't contain user hints like OIDC (hintUserId/loginHint) + // so we return all sessions for further processing return true; } return true;