mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-12 09:05:12 +00:00
fix(login): lru-cache compatibility, improve session error handling (#12099)
# 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 <[email protected]> Co-authored-by: conblem <[email protected]>
This commit is contained in:
co-authored by
Livio Spring
conblem
parent
13f8ba4215
commit
b1c8781516
@@ -11,7 +11,7 @@ const BASE_DIRECTIVES: Record<string, string[]> = {
|
||||
|
||||
export interface CSPOptions {
|
||||
serviceUrl?: string;
|
||||
iframeOrigins?: string[];
|
||||
iframeOrigins?: string[] | null;
|
||||
}
|
||||
|
||||
export function buildCSP(options: CSPOptions = {}): string {
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -54,30 +54,50 @@ async function resolveAuthToken(): Promise<string> {
|
||||
* 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<string[] | undefined> {
|
||||
export async function getIframeOrigins(
|
||||
baseUrl: string,
|
||||
instanceHost?: string,
|
||||
publicHost?: string,
|
||||
): Promise<string[] | null> {
|
||||
const cacheKey = instanceHost || "__default__";
|
||||
|
||||
return cache.getOrFetch<string[] | undefined>(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<string[] | null>(
|
||||
cacheKey,
|
||||
() => fetchIframeOrigins(baseUrl, instanceHost, publicHost),
|
||||
CACHE_TTL_MS,
|
||||
);
|
||||
}
|
||||
|
||||
async function fetchIframeOrigins(baseUrl: string, instanceHost?: string): Promise<string[] | undefined> {
|
||||
async function fetchIframeOrigins(baseUrl: string, instanceHost?: string, publicHost?: string): Promise<string[] | null> {
|
||||
const token = await resolveAuthToken();
|
||||
const reqHeaders: Record<string, string> = {
|
||||
"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;
|
||||
}
|
||||
|
||||
@@ -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";
|
||||
|
||||
|
||||
Reference in New Issue
Block a user