From b1c8781516410f9f79cb475f8b2bb07eb7e16da3 Mon Sep 17 00:00:00 2001 From: Max Peintner Date: Thu, 30 Apr 2026 15:27:44 +0200 Subject: [PATCH] fix(login): lru-cache compatibility, improve session error handling (#12099) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Which Problems Are Solved - Logo and heading overlap on login pages due to a -mb-4 negative margin on the logo container - The /login route returns a 500 when `listSessions` fails (e.g. stale session cookies), because loadSessions has no error handling. - The proxy logs `"fetch() returned undefined"` because `fetchIframeOrigins` returns undefined, which lru-cache's forceFetch treats as a fetch failure. # How the Problems Are Solved - Remove `-mb-4` from the logo container in DynamicTheme. - Wrap `loadSessions` in a try/catch — failures fall through as empty sessions instead of crashing. - Return `null` instead of `undefined` from `fetchIframeOrigins` so lru-cache caches it normally. --------- Co-authored-by: Livio Spring <9405495+livio-a@users.noreply.github.com> Co-authored-by: conblem --- apps/login/next-env-vars.d.ts | 7 ++++ apps/login/next.config.mjs | 1 - apps/login/src/app/login/route.ts | 41 +++++++++++++------ apps/login/src/components/dynamic-theme.tsx | 2 +- apps/login/src/components/register-form.tsx | 2 +- apps/login/src/lib/csp.ts | 2 +- apps/login/src/lib/server/flow-initiation.ts | 11 +++-- apps/login/src/lib/server/host.test.ts | 17 -------- apps/login/src/lib/server/host.ts | 3 +- .../login/src/lib/server/security-settings.ts | 39 ++++++++++++++---- apps/login/src/lib/service-url.test.ts | 18 -------- apps/login/src/proxy.ts | 28 ++++++++----- 12 files changed, 96 insertions(+), 75 deletions(-) diff --git a/apps/login/next-env-vars.d.ts b/apps/login/next-env-vars.d.ts index f56784cbc..83fafb3e7 100644 --- a/apps/login/next-env-vars.d.ts +++ b/apps/login/next-env-vars.d.ts @@ -96,6 +96,13 @@ declare namespace NodeJS { */ API_CACHE_CONFIG?: string; + /** + * Optional: Disable the proxy's runtime CSP fetch from the ZITADEL API. + * Defaults to true (enabled). Set to "false" to skip fetching iframe origins + * and use a restrictive default CSP instead. + */ + CSP_FETCH_ENABLED?: string; + /** * Optional: Disable OpenTelemetry instrumentation. * Set to "true" to bypass OTEL initialization. diff --git a/apps/login/next.config.mjs b/apps/login/next.config.mjs index 5bdeea755..ce25fab68 100755 --- a/apps/login/next.config.mjs +++ b/apps/login/next.config.mjs @@ -30,7 +30,6 @@ const nextConfig = { experimental: { // Add React 19 compatibility optimizations optimizePackageImports: ["@radix-ui/react-tooltip", "@heroicons/react"], - useCache: true, serverActions: { ...(process.env.SERVER_ACTION_ALLOWED_ORIGINS ? { allowedOrigins: process.env.SERVER_ACTION_ALLOWED_ORIGINS.split(",").map((o) => o.trim()) } diff --git a/apps/login/src/app/login/route.ts b/apps/login/src/app/login/route.ts index a68fe86d6..24ef2179c 100644 --- a/apps/login/src/app/login/route.ts +++ b/apps/login/src/app/login/route.ts @@ -1,16 +1,19 @@ import { isRSCRequest, validateAuthRequest } from "@/lib/auth-utils"; import { getAllSessions } from "@/lib/cookies"; +import { createLogger } from "@/lib/logger"; import { FlowInitiationParams, handleOIDCFlowInitiation, handleSAMLFlowInitiation } from "@/lib/server/flow-initiation"; import { getServiceConfig } from "@/lib/service-url"; import { listSessions, ServiceConfig } from "@/lib/zitadel"; import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb"; -import { headers } from "next/headers"; + import { NextRequest, NextResponse } from "next/server"; export const dynamic = "force-dynamic"; export const revalidate = false; export const fetchCache = "default-no-store"; +const logger = createLogger("login-route"); + async function loadSessions({ serviceConfig, ids }: { serviceConfig: ServiceConfig; ids: string[] }): Promise { const response = await listSessions({ serviceConfig, ids: ids.filter((id: string | undefined) => !!id) }); @@ -18,8 +21,7 @@ async function loadSessions({ serviceConfig, ids }: { serviceConfig: ServiceConf } export async function GET(request: NextRequest) { - const _headers = await headers(); - const { serviceConfig } = getServiceConfig(_headers); + const { serviceConfig } = getServiceConfig(request.headers); const searchParams = request.nextUrl.searchParams; @@ -38,20 +40,33 @@ export async function GET(request: NextRequest) { const ids = sessionCookies.map((s) => s.id); let sessions: Session[] = []; if (ids && ids.length) { - sessions = await loadSessions({ serviceConfig, ids }); + try { + sessions = await loadSessions({ serviceConfig, ids }); + } catch (error) { + logger.warn("Failed to load sessions", { error }); + // listSessions can fail for various reasons (stale/expired session IDs + // still in cookies, API errors, etc.). Treat any failure as "no valid + // sessions" so the user is redirected to loginname instead of a 500. + sessions = []; + } } // Flow initiation - delegate to appropriate handler const flowParams: FlowInitiationParams = { serviceConfig, requestId, sessions, sessionCookies, request }; - 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: "Device authorization should use /device endpoint" }, { status: 400 }); - } else { - return NextResponse.json({ error: "Invalid request ID format" }, { status: 400 }); + try { + if (requestId.startsWith("oidc_")) { + return await handleOIDCFlowInitiation(flowParams); + } else if (requestId.startsWith("saml_")) { + return await 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: "Device authorization should use /device endpoint" }, { status: 400 }); + } else { + return NextResponse.json({ error: "Invalid request ID format" }, { status: 400 }); + } + } catch (error: unknown) { + logger.error("Flow initiation failed", { requestId, error }); + return NextResponse.json({ error: "Internal server error" }, { status: 500 }); } } diff --git a/apps/login/src/components/dynamic-theme.tsx b/apps/login/src/components/dynamic-theme.tsx index fef15c6dc..701d9c20d 100644 --- a/apps/login/src/components/dynamic-theme.tsx +++ b/apps/login/src/components/dynamic-theme.tsx @@ -99,7 +99,7 @@ export function DynamicTheme({
-
+
{branding && ( = { export interface CSPOptions { serviceUrl?: string; - iframeOrigins?: string[]; + iframeOrigins?: string[] | null; } export function buildCSP(options: CSPOptions = {}): string { diff --git a/apps/login/src/lib/server/flow-initiation.ts b/apps/login/src/lib/server/flow-initiation.ts index b17462b54..cb268e926 100644 --- a/apps/login/src/lib/server/flow-initiation.ts +++ b/apps/login/src/lib/server/flow-initiation.ts @@ -267,8 +267,14 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr } return NextResponse.redirect(loginNameUrl); } else if (authRequest.prompt.includes(Prompt.NONE)) { - const securitySettings = await getSecuritySettings({ serviceConfig }); - + let securitySettings: SecuritySettings | undefined; + try { + securitySettings = await getSecuritySettings({ serviceConfig }); + } catch (err) { + logger.error("Failed to load security settings for CSP in prompt=none flow", { + error: err, + }); + } const selectedSession = await findValidSession({ serviceConfig, sessions, authRequest, organization }); const noSessionResponse = NextResponse.json({ error: "No active session found" }, { status: 400 }); @@ -302,7 +308,6 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr const callbackResponse = NextResponse.redirect(callbackUrl); setCSPHeaders(callbackResponse, serviceConfig, securitySettings); - return callbackResponse; } else { let selectedSession = await findValidSession({ serviceConfig, sessions, authRequest, organization }); diff --git a/apps/login/src/lib/server/host.test.ts b/apps/login/src/lib/server/host.test.ts index b2051d6d7..4e29f0b46 100644 --- a/apps/login/src/lib/server/host.test.ts +++ b/apps/login/src/lib/server/host.test.ts @@ -39,23 +39,6 @@ describe("Host utility functions", () => { expect(mockHeaders.get).toHaveBeenCalledWith("x-zitadel-instance-host"); expect(mockHeaders.get).toHaveBeenCalledWith("x-zitadel-forward-host"); }); - - test("should return null when neither x-zitadel-instance-host nor x-zitadel-forward-host are available", () => { - const mockHeaders = { - get: vi.fn((key: string) => { - if (key === "x-zitadel-instance-host") return null; - if (key === "x-zitadel-forward-host") return null; - if (key === "x-forwarded-host") return "accounts.mycompany.com"; - if (key === "host") return "internal.server"; - return null; - }), - } as any; - - const result = getInstanceHost(mockHeaders); - expect(result).toBeNull(); - expect(mockHeaders.get).toHaveBeenCalledWith("x-zitadel-instance-host"); - expect(mockHeaders.get).toHaveBeenCalledWith("x-zitadel-forward-host"); - }); }); describe("getPublicHostWithProtocol", () => { diff --git a/apps/login/src/lib/server/host.ts b/apps/login/src/lib/server/host.ts index 6c44f8fee..9ec272f86 100644 --- a/apps/login/src/lib/server/host.ts +++ b/apps/login/src/lib/server/host.ts @@ -9,7 +9,8 @@ import { ReadonlyHeaders } from "next/dist/server/web/spec-extension/adapters/he */ export function getInstanceHost(headers: ReadonlyHeaders): string | null { // use standard proxy headers (x-forwarded-host → host) for both multi-tenant and self-hosted, do not use x-zitadel-instance-host - const instanceHost = headers.get("x-zitadel-instance-host") || headers.get("x-zitadel-forward-host"); + const instanceHost = + headers.get("x-zitadel-instance-host") || headers.get("x-zitadel-forward-host") || headers.get("host"); return instanceHost; } diff --git a/apps/login/src/lib/server/security-settings.ts b/apps/login/src/lib/server/security-settings.ts index 566969d99..1bf6d4bcc 100644 --- a/apps/login/src/lib/server/security-settings.ts +++ b/apps/login/src/lib/server/security-settings.ts @@ -54,30 +54,50 @@ async function resolveAuthToken(): Promise { * via the Connect protocol (POST + JSON). This uses raw fetch (no connectRPC * node transport) so it stays compatible with the Next.js proxy runtime. * + * Uses the same header pattern as createServerTransport in zitadel.ts: + * - baseUrl (ZITADEL_API_URL) as the fetch URL + * - x-zitadel-instance-host and x-zitadel-public-host as headers + * - CUSTOM_REQUEST_HEADERS applied + * * Results are cached in-memory for 1 hour per instance host using a bounded * LRU cache. Concurrent requests for the same key share a single in-flight * promise to prevent thundering-herd stampedes on the backend. * * @param baseUrl - The ZITADEL API base URL (ZITADEL_API_URL) - * @param instanceHost - Optional instance host for multi-tenant deployments - * @returns An array of allowed iframe origins, or undefined if not configured + * @param instanceHost - Optional instance host for multi-tenant routing + * @param publicHost - Optional public host for multi-tenant routing + * @returns An array of allowed iframe origins, or null if not configured */ -export async function getIframeOrigins(baseUrl: string, instanceHost?: string): Promise { +export async function getIframeOrigins( + baseUrl: string, + instanceHost?: string, + publicHost?: string, +): Promise { const cacheKey = instanceHost || "__default__"; - return cache.getOrFetch(cacheKey, () => fetchIframeOrigins(baseUrl, instanceHost), CACHE_TTL_MS); + // The fetcher returns null (not undefined) because lru-cache treats + // undefined as a fetch failure. + return cache.getOrFetch( + cacheKey, + () => fetchIframeOrigins(baseUrl, instanceHost, publicHost), + CACHE_TTL_MS, + ); } -async function fetchIframeOrigins(baseUrl: string, instanceHost?: string): Promise { +async function fetchIframeOrigins(baseUrl: string, instanceHost?: string, publicHost?: string): Promise { const token = await resolveAuthToken(); const reqHeaders: Record = { "Content-Type": "application/json", Authorization: `Bearer ${token}`, }; + // Apply instance/public host headers — same pattern as createServerTransport if (instanceHost) { reqHeaders["x-zitadel-instance-host"] = instanceHost; } + if (publicHost) { + reqHeaders["x-zitadel-public-host"] = publicHost; + } // Apply custom headers from environment applyCustomHeaders({ @@ -93,6 +113,7 @@ async function fetchIframeOrigins(baseUrl: string, instanceHost?: string): Promi method: "POST", headers: reqHeaders, body: "{}", + cache: "no-store", }); if (!response.ok) { @@ -100,13 +121,15 @@ async function fetchIframeOrigins(baseUrl: string, instanceHost?: string): Promi status: response.status, statusText: response.statusText, }); - return undefined; + // Return null instead of undefined — lru-cache treats undefined as a + // fetch failure and throws "fetch() returned undefined". + return null; } const data: GetSecuritySettingsResponseJson = await response.json(); const settings = data.settings; - const origins = settings?.embeddedIframe?.enabled ? settings.embeddedIframe.allowedOrigins : undefined; + const origins = settings?.embeddedIframe?.enabled ? settings.embeddedIframe.allowedOrigins : null; - return origins && origins.length > 0 ? origins : undefined; + return origins && origins.length > 0 ? origins : null; } diff --git a/apps/login/src/lib/service-url.test.ts b/apps/login/src/lib/service-url.test.ts index 9cffecaed..26b04dddf 100644 --- a/apps/login/src/lib/service-url.test.ts +++ b/apps/login/src/lib/service-url.test.ts @@ -24,24 +24,6 @@ describe("Service URL utilities", () => { expect(() => getServiceConfig(mockHeaders)).toThrow("ZITADEL_API_URL is not set"); }); - test("should return only baseUrl when x-zitadel-forward-host is not present (self-hosted)", () => { - process.env.ZITADEL_API_URL = "https://zitadel.mycompany.com"; - - const mockHeaders = { - get: vi.fn((key: string) => { - if (key === "x-zitadel-forward-host") return null; - if (key === "host") return "mycompany.com"; - return null; - }), - } as any; - - const result = getServiceConfig(mockHeaders); - - expect(result.serviceConfig.baseUrl).toBe("https://zitadel.mycompany.com"); - expect(result.serviceConfig.instanceHost).toBeUndefined(); - expect(result.serviceConfig.publicHost).toBe("mycompany.com"); - }); - test("should use x-zitadel-forward-host when present (multi-tenant)", () => { process.env.ZITADEL_API_URL = "https://api.zitadel.cloud"; diff --git a/apps/login/src/proxy.ts b/apps/login/src/proxy.ts index 9ce0dbb19..198bb8d55 100644 --- a/apps/login/src/proxy.ts +++ b/apps/login/src/proxy.ts @@ -1,4 +1,3 @@ -import { headers } from "next/headers"; import { NextRequest, NextResponse } from "next/server"; import { buildCSP } from "./lib/csp"; import { createLogger } from "./lib/logger"; @@ -6,6 +5,7 @@ import { getIframeOrigins } from "./lib/server/security-settings"; import { getServiceConfig } from "./lib/service-url"; const logger = createLogger("middleware"); + export const config = { matcher: ["/.well-known/:path*", "/oauth/:path*", "/oidc/:path*", "/idps/callback/:path*", "/saml/:path*", "/:path*"], }; @@ -28,26 +28,32 @@ export async function proxy(request: NextRequest) { return NextResponse.next({ request: { headers: requestHeaders } }); } - const _headers = await headers(); - const { serviceConfig } = getServiceConfig(_headers); + const { serviceConfig } = getServiceConfig(request.headers); const { baseUrl, publicHost, instanceHost } = serviceConfig; // Build CSP headers using security settings fetched directly from the // ZITADEL API (no self-loopback through the load balancer). const responseHeaders = new Headers(); - try { - const iframeOrigins = await getIframeOrigins(baseUrl, instanceHost); + const cspFetchEnabled = process.env.CSP_FETCH_ENABLED !== "false"; - responseHeaders.set("Content-Security-Policy", buildCSP({ serviceUrl: baseUrl, iframeOrigins })); + if (cspFetchEnabled) { + try { + const iframeOrigins = await getIframeOrigins(baseUrl, instanceHost, publicHost); - if (!iframeOrigins) { + responseHeaders.set("Content-Security-Policy", buildCSP({ serviceUrl: baseUrl, iframeOrigins })); + + if (!iframeOrigins) { + responseHeaders.set("X-Frame-Options", "deny"); + } + } catch (err) { + logger.error("Failed to load security settings for CSP, using default CSP", { + error: err instanceof Error ? err.message : String(err), + }); + responseHeaders.set("Content-Security-Policy", buildCSP({ serviceUrl: baseUrl })); responseHeaders.set("X-Frame-Options", "deny"); } - } catch (err) { - logger.error("Failed to load security settings for CSP, using default CSP", { - error: err instanceof Error ? err.message : String(err), - }); + } else { responseHeaders.set("Content-Security-Policy", buildCSP({ serviceUrl: baseUrl })); responseHeaders.set("X-Frame-Options", "deny"); }