mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
051506b5bb
<!-- Please inform yourself about the contribution guidelines on submitting a PR here: https://github.com/zitadel/zitadel/blob/main/CONTRIBUTING.md#submit-a-pull-request-pr. Take note of how PR/commit titles should be written and replace the template texts in the sections below. Don't remove any of the sections. It is important that the commit history clearly shows what is changed and why. Important: By submitting a contribution you agree to the terms from our Licensing Policy as described here: https://github.com/zitadel/zitadel/blob/main/LICENSING.md#community-contributions. --> # Which Problems Are Solved The `sessions` cookie lifetime is independent of Zitadel's server-side session validity, so a cookie can reference an invalid session. In that case `getSession` throws `not_found` (QUERY-SFeaa). - `loadMostRecentSession` lets that error propagate instead of treating it as "no session", crashing the server render of every page that calls it without its own guard. - Most visibly, the "forgot password" flow landing on `/password/set` throws `[not_found] Session does not exist (QUERY-SFeaa)` (gRPC code 5, HTTP 404) and fails to render. - 12 of the 16 call sites are unguarded and share this crash. # How the Problems Are Solved Catch the error in `loadMostRecentSession` and return `undefined`, unifying "cookie references a dropped session" with the existing "no cookie" case. Every call site already handles an undefined session, either with a graceful fallback or by throwing its own explicit error. ```ts return getSession({ serviceConfig, sessionId: recent.id, sessionToken: recent.token }) .then((resp: GetSessionResponse) => resp.session) .catch((error) => { console.warn("[Session] Could not load most recent session", error); return undefined; }); ``` All `loadMostRecentSession` call sites — **Previous** = behaviour when the cookie referenced an invalid session; **Change** = what this PR does to that call site: | # | Call site | Change in this PR | Previous behaviour (invalid session) | New behaviour | |---|-----------|-------------------|--------------------------------------|---------------| | — | `lib/session.ts` — `loadMostRecentSession` | **Added `.catch` → `undefined`** (core fix) | Threw `not_found` to caller | Resolves to `undefined` | | 1 | `password/set/page.tsx` | none | **Crash** (the reported bug) | Falls back to login-name flow | | 2 | `password/change/page.tsx` | none | **Crash** | `undefined` → `failedLoading` alert if no user | | 3 | `passkey/set/page.tsx` | none | **Crash** | `undefined` → `userId` fallback / `unknownContext` | | 4 | `otp/[method]/page.tsx` | none | **Crash** | `undefined` → `unknownContext` alert | | 5 | `otp/[method]/set/page.tsx` | none | **Crash** (raw `ConnectError`) | `undefined` → own `throw "No session found"` | | 6 | `mfa/page.tsx` | none | **Crash** | `undefined` → `verify.noResults` alert | | 7 | `mfa/set/page.tsx` | none | **Crash** (raw `ConnectError`) | `undefined` → own `throw "Could not get user id from session"` | | 8 | `authenticator/set/page.tsx` | none | **Crash** (raw `ConnectError`) | `undefined` → own throw / `unknownContext` | | 9 | `signedin/page.tsx` | none | **Crash** | `undefined` → renders with `loginName` fallback | | 10 | `u2f/set/page.tsx` | none | **Crash** | `undefined` → `unknownContext` alert | | 11 | `u2f/page.tsx` | none | **Crash** | `undefined` → renders on `loginName` | | 12 | `lib/server/verify.ts` — `verifyTOTP` | none | **Crash** (raw `ConnectError`) | `undefined` → own `throw "No user id found in session."` | | 13 | `password/page.tsx` | **Removed redundant `try/catch`** | Caught locally → `undefined` | `undefined` — unchanged | | 14 | `verify/page.tsx` | **Removed redundant `.catch`** ¹ | Caught locally → `undefined` | `undefined` — unchanged | | 15 | `verify/success/page.tsx` | **Removed redundant `.catch`** | Caught locally → `undefined` | `undefined` — unchanged | | 16 | `passkey/page.tsx` | **Removed redundant `.catch`** (login-name path) | Caught locally → `undefined` | `undefined` — unchanged | Rows 1–12 were unguarded and crashed; the central change fixes them. Rows 13–16 already handled the error locally. # Additional Changes - This makes the four callers that wrapped the call in their own try/catch or `.catch` redundant, so remove that now-dead handling (password, verify, verify/success, passkey). - Add unit tests covering `loadMostRecentSession`: no cookie, success, and the stale-cookie rejection path. ¹ `verify/page.tsx`'s `.catch` also eagerly fetched `loginSettings`; that is already covered by the page's own lazy fetch, so behaviour is unchanged (one cosmetic edge case under `ignoreUnknownUsernames` with both `userId` and `loginName` present — no functional impact). # Additional Context - Session *expiry* alone does not trigger this: expired rows are not removed from the session projection, so `getSession` still returns them. The `not_found` specifically means the session was **removed** server-side (logout, admin/API termination, or a user/org/instance cascade) while the browser still held the cookie. - Mirrors the pattern upstream already applies inline in `verify/page.tsx` (*"ignore error, as we might not have a session yet"*), lifted to the shared function. - **Related issue (not in this PR):** the sibling `loadSessionById` helpers on the `?sessionId=` path (`passkey`, `otp`, `mfa`, `mfa/set`, `signedin`, `authenticator/set`, `u2f`) call `getSession` directly and share the same defect. Lower practical exposure (short-lived in-flow URLs; a clean logout clears the cookie), so a follow-up PR should lift those into a shared `loadSessionById` in `lib/session.ts`. - Prior art: upstream added an equivalent guard to verify/page.tsx in #11130, but the other call sites were never given the same treatment — this PR resolves that inconsistency at the source. Error trace: ``` {"level":"error","message":"⨯ Error [ConnectError]: [not_found] Session does not exist (QUERY-SFeaa)\n at R (.next/server/chunks/ssr/_0uqqxk7._.js:3:25490)\n at <unknown> (.next/server/chunks/ssr/_0uqqxk7._.js:3:26477)\n at next (.next/server/chunks/ssr/_0uqqxk7._.js:3:68112)\n at async (.next/server/chunks/ssr/_0uqqxk7._.js:10:31326)\n at async (.next/server/chunks/ssr/_0uqqxk7._.js:10:33734)\n at async Object.unary (.next/server/chunks/ssr/_0uqqxk7._.js:3:66838)\n at async Object.getSession (.next/server/chunks/ssr/_0uqqxk7._.js:3:72371)\n at async o (.next/server/chunks/ssr/[root-of-the-server]__0msfcq_._.js:1:7440) {\n rawMessage: 'Session does not exist (QUERY-SFeaa)',\n code: 5,\n metadata: Headers {\n 'accept-encoding': 'gzip',\n 'alt-svc': 'h3=\":443\"; ma=2592000',\n 'content-length': '240',\n 'content-type': 'application/json',\n date: 'Wed, 08 Jul 2026 09:06:09 GMT',\n 'strict-transport-security': 'max-age=63072000; includeSubDomains; preload',\n vary: 'Origin',\n },\n details: [\n {\n type: 'zitadel.v1.ErrorDetail',\n value: [Uint8Array],\n debug: [Object]\n }\n ],\n cause: undefined,\n httpStatus: 404,\n isUserError: true,\n digest: '634020356'\n}","timestamp":"2026-07-08T09:06:09.969Z"} ``` --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Marco A. <[email protected]> Co-authored-by: Liam Neville <[email protected]>