diff --git a/apps/login/src/app/(login)/passkey/page.tsx b/apps/login/src/app/(login)/passkey/page.tsx index c236d626f..eda5093b3 100644 --- a/apps/login/src/app/(login)/passkey/page.tsx +++ b/apps/login/src/app/(login)/passkey/page.tsx @@ -40,9 +40,6 @@ export default async function Page(props: { searchParams: Promise { - // ignore error - return undefined; }); } diff --git a/apps/login/src/app/(login)/password/page.tsx b/apps/login/src/app/(login)/password/page.tsx index 3962285e4..0f6db975f 100644 --- a/apps/login/src/app/(login)/password/page.tsx +++ b/apps/login/src/app/(login)/password/page.tsx @@ -33,19 +33,13 @@ export default async function Page(props: { searchParams: Promise }) { loginName, organization, }, - }).catch(async (error) => { - loginSettings = await getLoginSettings({ serviceConfig, organization }); - if (!loginSettings?.ignoreUnknownUsernames) { - console.error("loadMostRecentSession failed", error); - } - // ignore error, as we might not have a session yet - return undefined; }); } else if ("userId" in searchParams && userId) { const userResponse = await getUserByID({ serviceConfig, userId }); diff --git a/apps/login/src/app/(login)/verify/success/page.tsx b/apps/login/src/app/(login)/verify/success/page.tsx index 17f46653e..58db5165b 100644 --- a/apps/login/src/app/(login)/verify/success/page.tsx +++ b/apps/login/src/app/(login)/verify/success/page.tsx @@ -18,11 +18,7 @@ export default async function Page(props: { searchParams: Promise }) { const branding = await getBrandingSettings({ serviceConfig, organization }); - const sessionFactors = await loadMostRecentSession({ serviceConfig, sessionParams: { loginName, organization } }).catch( - (error) => { - console.warn("Error loading session:", error); - }, - ); + const sessionFactors = await loadMostRecentSession({ serviceConfig, sessionParams: { loginName, organization } }); const id = userId ?? sessionFactors?.factors?.user?.id; diff --git a/apps/login/src/lib/session.test.ts b/apps/login/src/lib/session.test.ts index a8b18985c..e64f4f59e 100644 --- a/apps/login/src/lib/session.test.ts +++ b/apps/login/src/lib/session.test.ts @@ -13,10 +13,12 @@ * - Edge cases like sessions without expiration date */ +import { Code, ConnectError } from "@connectrpc/connect"; import { timestampDate } from "@zitadel/client"; import { AuthenticationMethodType } from "@zitadel/proto/zitadel/user/v2/user_service_pb"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; -import { isSessionValid } from "./session"; +import * as cookiesModule from "./cookies"; +import { isSessionValid, loadMostRecentSession } from "./session"; import * as verifyHelperModule from "./verify-helper"; import * as zitadelModule from "./zitadel"; @@ -28,6 +30,11 @@ vi.mock("./zitadel", () => ({ listAuthenticationMethodTypes: vi.fn(), getLoginSettings: vi.fn(), getUserByID: vi.fn(), + getSession: vi.fn(), +})); + +vi.mock("./cookies", () => ({ + getMostRecentCookieWithLoginname: vi.fn(), })); vi.mock("./verify-helper", () => ({ @@ -1447,3 +1454,77 @@ describe("isSessionValid", () => { }); }); }); + +describe("loadMostRecentSession", () => { + const serviceConfig = { baseUrl: "https://zitadel-abc123.zitadel.cloud" }; + const sessionParams = { loginName: "test@example.com", organization: "test-org-id" }; + const cookie = { id: "session-id", token: "session-token", loginName: "test@example.com", organization: "test-org-id" }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("returns undefined without calling getSession when no cookie is found", async () => { + vi.mocked(cookiesModule.getMostRecentCookieWithLoginname).mockResolvedValue(undefined as any); + + const result = await loadMostRecentSession({ serviceConfig, sessionParams }); + + expect(result).toBeUndefined(); + expect(zitadelModule.getSession).not.toHaveBeenCalled(); + }); + + test("returns the session when getSession succeeds", async () => { + const session = { id: "session-id", factors: { user: { id: "user-id" } } }; + vi.mocked(cookiesModule.getMostRecentCookieWithLoginname).mockResolvedValue(cookie as any); + vi.mocked(zitadelModule.getSession).mockResolvedValue({ session } as any); + + const result = await loadMostRecentSession({ serviceConfig, sessionParams }); + + expect(result).toBe(session); + expect(zitadelModule.getSession).toHaveBeenCalledWith({ + serviceConfig, + sessionId: cookie.id, + sessionToken: cookie.token, + }); + }); + + test("returns undefined instead of throwing when getSession rejects with NotFound (stale cookie)", async () => { + // The `sessions` cookie can outlive the server-side session, so getSession may reject with + // a NotFound ConnectError. loadMostRecentSession must treat that the same as no session. + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const notFound = new ConnectError("Session does not exist (QUERY-SFeaa)", Code.NotFound); + vi.mocked(cookiesModule.getMostRecentCookieWithLoginname).mockResolvedValue(cookie as any); + vi.mocked(zitadelModule.getSession).mockRejectedValue(notFound); + + const result = await loadMostRecentSession({ serviceConfig, sessionParams }); + + expect(result).toBeUndefined(); + expect(consoleSpy).toHaveBeenCalledWith("[Session] Could not load most recent session", notFound); + consoleSpy.mockRestore(); + }); + + test("re-throws non-NotFound ConnectErrors (real failures must propagate)", async () => { + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const unavailable = new ConnectError("backend unavailable", Code.Unavailable); + vi.mocked(cookiesModule.getMostRecentCookieWithLoginname).mockResolvedValue(cookie as any); + vi.mocked(zitadelModule.getSession).mockRejectedValue(unavailable); + + await expect(loadMostRecentSession({ serviceConfig, sessionParams })).rejects.toBe(unavailable); + expect(consoleSpy).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + + test("re-throws non-ConnectError rejections (only coded NotFound is treated as no session)", async () => { + // The guard is `instanceof ConnectError && code === NotFound`; anything that is not a + // ConnectError (e.g. an unexpected runtime error) must not be mistaken for a stale + // cookie and must propagate. + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const unexpected = new Error("boom"); + vi.mocked(cookiesModule.getMostRecentCookieWithLoginname).mockResolvedValue(cookie as any); + vi.mocked(zitadelModule.getSession).mockRejectedValue(unexpected); + + await expect(loadMostRecentSession({ serviceConfig, sessionParams })).rejects.toBe(unexpected); + expect(consoleSpy).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); +}); diff --git a/apps/login/src/lib/session.ts b/apps/login/src/lib/session.ts index 540fe78dc..b43f1446d 100644 --- a/apps/login/src/lib/session.ts +++ b/apps/login/src/lib/session.ts @@ -29,9 +29,26 @@ export async function loadMostRecentSession({ return undefined; } - return getSession({ serviceConfig, sessionId: recent.id, sessionToken: recent.token }).then( - (resp: GetSessionResponse) => resp.session, - ); + return getSession({ serviceConfig, sessionId: recent.id, sessionToken: recent.token }) + .then((resp: GetSessionResponse) => resp.session) + .catch(async (error) => { + const { Code, ConnectError } = await import("@connectrpc/connect"); + const isNotFound = error instanceof ConnectError && error.code === Code.NotFound; + + // The `sessions` cookie has no maxAge, so it can outlive the server-side session + // and reference one the session projection no longer holds — e.g. the user logged out, an + // admin/API terminated the session, or the user/org/instance was removed. In that + // case getSession throws `not_found`. Treat it like the no-cookie case above and + // return undefined instead of letting the error crash the caller's render. Every + // call site already handles an undefined session — either with a graceful fallback + // or by throwing its own explicit "no session" error. + if (isNotFound) { + console.warn("[Session] Could not load most recent session", error); + return undefined; + } + + throw error; + }); } /**