mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
3c746281f17f6c88571379bf361bbf4d669ab1b5
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
dc1c8620bc |
fix(login): provide hidden username on password set/change forms (#12490)
<!-- 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 set-password, change-password and set-register-password forms render only password fields — there is no username field inside the `<form>`. Password managers look for a username field adjacent to the password field in order to associate and save the credential. Without one, on these screens they either skip offering to save, or save a password entry with no username attached — so the saved record can't later be matched to the account. The existing sign-in form (`password-form.tsx`) already carries a username field for this reason; the password *creation/change* flows were missing the equivalent. # How the Problems Are Solved Add a username field to each of the three forms so the newly set password is saved against the correct account: - `set-password-form.tsx` and `change-password-form.tsx` use the `loginName`. - `set-register-password-form.tsx` uses the `email`. The field is a real, focusable-excluded `type="text"` input rather than `type="hidden"` — password managers ignore `type="hidden"` fields when looking for a username, so a visually-hidden text input is required. It is: - `readOnly` (avoids a controlled-input warning and user edits), - `tabIndex={-1}` and `aria-hidden="true"` (kept out of the tab order and the accessibility tree), - `className="sr-only"` (visually hidden but present in the DOM), - `autoComplete="username"` (tells the password manager what it is). The value comes from props already passed to each form, so no signatures change. # Additional Changes None. # Additional Context - The username values (`loginName` / `email`) are already available in each component, so this is a purely presentational change with no server-side or API impact. Co-authored-by: Max Peintner <[email protected]> |
||
|
10a66b786b |
fix(login): honour login_hint and skip the auto-submit loop in OIDC flow (#12431)
<!-- 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 An OIDC authorize request that carries a `login_hint` (and no `prompt`) is meant to land the user on a **prefilled** login screen and let them proceed straight to authentication. Two problems prevented that: **1. The hint was dropped when an unrelated session existed.** When the browser held a `sessions` cookie for a *different* account: - The user was redirected to the **account picker** (`/accounts`) instead of the prefilled login-name screen. - The picker listed the already-logged-in, unrelated account(s) — never the user identified by `login_hint`. - The `login_hint` was **silently dropped** (the `/accounts` URL carries no hint), so the typed email was lost and the flow dead-ended on the wrong accounts. In a clean session (no `sessions` cookie) the same request prefilled correctly, which made the bug depend on unrelated pre-existing sessions. **2. The prefilled path was jarring and looped on "back".** Even when the hint *was* honoured, the flow redirected to `/loginname?loginName=…&submit=true`. That page renders the username screen and then **client-side auto-submits** it, so the user sees the username flash on screen, watches "Continue" click itself, and only then lands on the password step. Worse, `/loginname?submit=true` stays in browser history, so pressing **Back** from the password screen re-triggers the auto-submit and bounces the user straight forward again — an inescapable loop with no way to edit the username. # How the Problems Are Solved **Honour the hint (problem 1).** In `handleOIDCFlowInitiation`, the default branch (no `prompt`) already calls `findValidSession`, which filters the browser's sessions by `login_hint` and treats the hint as *selecting the user*. When the hint matches none of the sessions it returns `undefined`, and the remaining `eligibleSessions` (filtered by organization only) are therefore never the hinted user. The fallback previously chose between the loginname screen and the account picker based solely on `eligibleSessions.length === 0`, so a present-but-unrelated session sent the user to `gotoAccounts`, discarding the email. The loginname branch is now also gated on `authRequest.loginHint`, mirroring the existing empty-eligible-sessions branch. **Skip the flash and the loop (problem 2).** Instead of bouncing through `/loginname?submit=true` and relying on client-side auto-submit, the hint is now resolved **server-side** via `sendLoginname` (the same approach the `Prompt.LOGIN` branch already used) and the user is redirected **straight to the next step** (e.g. `/password`), skipping the `/loginname` screen entirely. This is factored into a shared `resolveLoginHint` helper and applied to all `login_hint` branches (default-prompt, no-session, and the existing `Prompt.LOGIN` path, now deduplicated). When the hint can't be resolved (unknown/ambiguous user, transient error), the flow falls back to a prefilled `/loginname` **without** `submit=true` — a clean screen where the user clicks once — so there is no auto-submit and no back-button loop in any path. ```ts if (!selectedSession || !selectedSession.id) { // login_hint matches no session: resolve it straight to the next step. if (authRequest.loginHint) { const hintResponse = await resolveLoginHint({ request, requestId, loginHint: authRequest.loginHint, organization }); if (hintResponse) { return hintResponse; } } // Prefill loginname (not the account picker) for an unresolved hint or when no session is eligible. if (authRequest.loginHint || eligibleSessions.length === 0) { return gotoLoginname({ request, requestId, loginHint: authRequest.loginHint, organization, orgDomain }); } return gotoAccounts({ request, requestId, organization, orgDomain }); } ``` Resulting behaviour: - `login_hint` set, a session **matches** it → silent auth (**unchanged**). - `login_hint` set, **no** session matches, hint **resolves** → straight to the next step (e.g. `/password`), no `/loginname` flash (**was**: account picker with the hint dropped / a jarring auto-submit screen). - `login_hint` set, hint **can't** be resolved → prefilled `/loginname`, no auto-submit, Back stays editable. - **No** `login_hint` → account picker / loginname exactly as before (**unchanged**). # Additional Changes - `resolveLoginHint` helper extracted in `flow-initiation.ts`; the `Prompt.LOGIN` branch now reuses it instead of duplicating the `sendLoginname` call. - `submit=true` is no longer emitted for the login-name flow, so the client-side auto-submit in `username-form.tsx` is no longer reached from OIDC initiation. - Tests in `flow-initiation.test.ts`: each `login_hint` branch (org-scope-filtered, unrelated-session, no-session) is covered twice — hint resolves → straight to `/password`; hint unresolved → prefilled `/loginname` with **no** `submit=true`. # Additional Context - The org-only `eligibleSessions` check was introduced in #12346, which correctly handled the *empty* case but did not account for a present-but-unmatched `login_hint`; this PR extends that gate. - Per OIDC, `login_hint` is advisory prefill; a client that wants the chooser would send `prompt=select_account`. Resolving the hint server-side also matches how the `Prompt.LOGIN` branch already behaved. --------- Co-authored-by: Max Peintner <[email protected]> Co-authored-by: Max Peintner <[email protected]> |
||
  
|
051506b5bb |
fix(login): prevent crash on a stale session cookie (#12423)
<!-- 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]> |