From 10a66b786b8125f1438c214608e10ce97c20bf79 Mon Sep 17 00:00:00 2001 From: Cameron Waldron <1000178+CamW@users.noreply.github.com> Date: Wed, 29 Jul 2026 14:06:36 +0200 Subject: [PATCH] fix(login): honour login_hint and skip the auto-submit loop in OIDC flow (#12431) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # 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 Co-authored-by: Max Peintner --- .../src/lib/server/flow-initiation.test.ts | 179 +++++++++++++++++- apps/login/src/lib/server/flow-initiation.ts | 154 ++++++++++----- 2 files changed, 280 insertions(+), 53 deletions(-) diff --git a/apps/login/src/lib/server/flow-initiation.test.ts b/apps/login/src/lib/server/flow-initiation.test.ts index e7f9fd246..aa32da1f2 100644 --- a/apps/login/src/lib/server/flow-initiation.test.ts +++ b/apps/login/src/lib/server/flow-initiation.test.ts @@ -32,6 +32,7 @@ vi.mock("@/lib/zitadel", () => ({ createResponse: vi.fn(), getActiveIdentityProviders: vi.fn(), getAuthRequest: vi.fn(), + getLoginSettings: vi.fn(), getOrgsByDomain: vi.fn(), getSAMLRequest: vi.fn(), getSecuritySettings: vi.fn(), @@ -184,6 +185,8 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { let mockGetAuthRequest: ReturnType; let mockConstructUrl: ReturnType; let mockFindValidSession: ReturnType; + let mockSendLoginname: ReturnType; + let mockGetLoginSettings: ReturnType; const orgSession = { id: "session-org", @@ -202,10 +205,14 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { const serviceUrl = await import("@/lib/service-url"); const session = await import("@/lib/session"); const authUtils = await import("@/lib/auth-utils"); + const loginname = await import("@/lib/server/loginname"); mockGetAuthRequest = vi.mocked(zitadel.getAuthRequest); mockConstructUrl = vi.mocked(serviceUrl.constructUrl); mockFindValidSession = vi.mocked(session.findValidSession); + mockSendLoginname = vi.mocked(loginname.sendLoginname); + mockGetLoginSettings = vi.mocked(zitadel.getLoginSettings); + mockGetLoginSettings.mockResolvedValue(undefined); vi.mocked(authUtils.getValidLocaleFromUILocales).mockReturnValue(null); mockConstructUrl.mockImplementation((_req: any, path: string) => { @@ -213,6 +220,9 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { }); mockFindValidSession.mockResolvedValue(null); + // Default: hint cannot be resolved to a redirect, exercising the prefilled + // /loginname fallback. Individual tests override this to assert the happy path. + mockSendLoginname.mockResolvedValue(undefined); }); afterEach(() => { @@ -242,7 +252,7 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { expect(location).not.toContain("/accounts"); }); - test("should include submit=true when loginHint is set and org scope filters out all sessions", async () => { + test("should resolve loginHint server-side (straight to next step) when org scope filters out all sessions", async () => { mockGetAuthRequest.mockResolvedValue({ authRequest: { id: "abc123", @@ -253,6 +263,36 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { }, }); + mockSendLoginname.mockResolvedValue({ + redirect: "/password?loginName=user%40example.com", + }); + + const res = await handleOIDCFlowInitiation( + makeBaseParams({ + sessions: [otherOrgSession] as any, + sessionCookies: [{ id: "session-other", token: "tok" }], + }), + ); + + const location = res.headers.get("location") ?? ""; + expect(location).toContain("/password"); + expect(location).not.toContain("/loginname"); + expect(location).not.toContain("/accounts"); + }); + + test("should prefill /loginname WITHOUT submit=true when loginHint cannot be resolved and org scope filters out all sessions", async () => { + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: ["urn:zitadel:iam:org:id:111111"], + prompt: [], + loginHint: "user@example.com", + }, + }); + + // mockSendLoginname resolves to undefined (default) → fallback path. + const res = await handleOIDCFlowInitiation( makeBaseParams({ sessions: [otherOrgSession] as any, @@ -263,7 +303,7 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { const location = res.headers.get("location") ?? ""; expect(location).toContain("/loginname"); expect(location).toContain("loginName=user%40example.com"); - expect(location).toContain("submit=true"); + expect(location).not.toContain("submit=true"); expect(location).not.toContain("/accounts"); }); @@ -289,6 +329,66 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { expect(location).toContain("/accounts"); }); + test("should resolve loginHint server-side when it matches no session but an unrelated session exists (default prompt)", async () => { + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: [], + prompt: [], + loginHint: "user@example.com", + }, + }); + + // An unrelated session is present (so eligibleSessions is non-empty), but + // findValidSession filtered by the hint returns nothing. + mockFindValidSession.mockResolvedValue(null); + + mockSendLoginname.mockResolvedValue({ + redirect: "/password?loginName=user%40example.com", + }); + + const res = await handleOIDCFlowInitiation( + makeBaseParams({ + sessions: [otherOrgSession] as any, + sessionCookies: [{ id: "session-other", token: "tok" }], + }), + ); + + const location = res.headers.get("location") ?? ""; + expect(location).toContain("/password"); + expect(location).not.toContain("/loginname"); + expect(location).not.toContain("/accounts"); + }); + + test("should fall back to prefilled /loginname (no submit) when loginHint cannot be resolved and an unrelated session exists (default prompt)", async () => { + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: [], + prompt: [], + loginHint: "user@example.com", + }, + }); + + mockFindValidSession.mockResolvedValue(null); + // mockSendLoginname resolves to undefined (default) → fallback path. + + const res = await handleOIDCFlowInitiation( + makeBaseParams({ + sessions: [otherOrgSession] as any, + sessionCookies: [{ id: "session-other", token: "tok" }], + }), + ); + + const location = res.headers.get("location") ?? ""; + expect(location).toContain("/loginname"); + expect(location).toContain("loginName=user%40example.com"); + expect(location).not.toContain("submit=true"); + expect(location).not.toContain("/accounts"); + }); + test("should redirect to /loginname when org scope filters out all sessions (SELECT_ACCOUNT prompt)", async () => { const { Prompt } = await import("@zitadel/proto/zitadel/oidc/v2/authorization_pb"); @@ -354,6 +454,81 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => { const location = res.headers.get("location") ?? ""; expect(location).toContain("/loginname"); }); + + test("should resolve loginHint server-side when there are no sessions (default prompt)", async () => { + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: [], + prompt: [], + loginHint: "user@example.com", + }, + }); + + mockSendLoginname.mockResolvedValue({ + redirect: "/password?loginName=user%40example.com", + }); + + const res = await handleOIDCFlowInitiation(makeBaseParams({ sessions: [] })); + + const location = res.headers.get("location") ?? ""; + expect(location).toContain("/password"); + expect(location).not.toContain("/loginname"); + }); + + test("should prefill /loginname WITHOUT submit=true when there are no sessions and loginHint cannot be resolved", async () => { + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: [], + prompt: [], + loginHint: "user@example.com", + }, + }); + + // mockSendLoginname resolves to undefined (default) → fallback path. + + const res = await handleOIDCFlowInitiation(makeBaseParams({ sessions: [] })); + + const location = res.headers.get("location") ?? ""; + expect(location).toContain("/loginname"); + expect(location).toContain("loginName=user%40example.com"); + expect(location).not.toContain("submit=true"); + }); + + test("should forward ignoreUnknownUsernames from login settings when resolving loginHint", async () => { + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: [], + prompt: [], + loginHint: "unknown@example.com", + }, + }); + + // Org has enumeration protection enabled: sendLoginname must receive the + // flag so an unknown hint still redirects to the (fake) /password step. + mockGetLoginSettings.mockResolvedValue({ ignoreUnknownUsernames: true }); + mockSendLoginname.mockResolvedValue({ + redirect: "/password?loginName=unknown%40example.com", + }); + + const res = await handleOIDCFlowInitiation(makeBaseParams({ sessions: [] })); + + expect(mockSendLoginname).toHaveBeenCalledWith( + expect.objectContaining({ + loginName: "unknown@example.com", + ignoreUnknownUsernames: true, + }), + ); + + const location = res.headers.get("location") ?? ""; + expect(location).toContain("/password"); + expect(location).not.toContain("/loginname"); + }); }); describe("handleOIDCFlowInitiation — Prompt.LOGIN + loginHint requestId prefix", () => { diff --git a/apps/login/src/lib/server/flow-initiation.ts b/apps/login/src/lib/server/flow-initiation.ts index b55f1881c..034b2f035 100644 --- a/apps/login/src/lib/server/flow-initiation.ts +++ b/apps/login/src/lib/server/flow-initiation.ts @@ -5,7 +5,7 @@ import { getLanguageCookie, setLanguageCookie } from "@/lib/cookies"; import { shouldUILocalesOverrideCookie } from "@/lib/i18n"; import { idpTypeToSlug } from "@/lib/idp"; import { createLogger } from "@/lib/logger"; -import { sendLoginname, SendLoginnameCommand } from "@/lib/server/loginname"; +import { sendLoginname } from "@/lib/server/loginname"; import { constructUrl } from "@/lib/service-url"; import { findValidSession } from "@/lib/session"; import { @@ -13,6 +13,7 @@ import { createResponse, getActiveIdentityProviders, getAuthRequest, + getLoginSettings, getOrgsByDomain, getSAMLRequest, getSecuritySettings, @@ -95,9 +96,9 @@ const gotoLoginname = ({ const loginNameUrl = constructUrl(request, "/loginname"); loginNameUrl.searchParams.set("requestId", requestId); + // Prefill only, no auto-submit: see resolveLoginHint. if (loginHint) { loginNameUrl.searchParams.set("loginName", loginHint); - loginNameUrl.searchParams.set("submit", "true"); } if (organization) { loginNameUrl.searchParams.set("organization", organization); @@ -109,6 +110,57 @@ const gotoLoginname = ({ return NextResponse.redirect(loginNameUrl); }; +/** + * Resolve a login_hint server-side to a redirect straight to the next step + * (e.g. /password), skipping the /loginname screen entirely. Returns null when + * there is no hint or it can't be resolved (unknown/ambiguous user, transient + * error), in which case callers fall back to the prefilled /loginname screen. + */ +const resolveLoginHint = async ({ + serviceConfig, + request, + requestId, + loginHint, + organization, +}: { + serviceConfig: ServiceConfig; + request: NextRequest; + requestId: string; + loginHint?: string; + organization?: string; +}): Promise | null> => { + if (!loginHint) { + return null; + } + + try { + // Mirror the client-side loginname form: forward ignoreUnknownUsernames so + // an unknown hint redirects to the (fake) /password step like a known one, + // instead of falling back to /loginname and disclosing that the user does + // not exist. getLoginSettings is TTL-cached, so this adds no extra RPC. + const loginSettings = await getLoginSettings({ serviceConfig, organization: organization || undefined }); + + const res = await sendLoginname({ + loginName: loginHint, + requestId, + organization: organization || undefined, + ignoreUnknownUsernames: loginSettings?.ignoreUnknownUsernames, + }); + + if (res && "redirect" in res && res.redirect) { + return NextResponse.redirect(constructUrl(request, res.redirect)); + } + + if (res && "error" in res && res.error) { + logger.debug("login_hint could not be resolved, falling back to /loginname", { error: res.error }); + } + } catch (error) { + logger.error("Failed to resolve login_hint via sendLoginname:", { error }); + } + + return null; +}; + /** * Filter sessions by organization, matching the same logic used in * the accounts page and findValidSession. @@ -290,41 +342,25 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr orgDomain, }); } else if (authRequest.prompt.includes(Prompt.LOGIN)) { - if (authRequest.loginHint) { - try { - let command: SendLoginnameCommand = { - loginName: authRequest.loginHint, - requestId: requestId, - }; + const hintResponse = await resolveLoginHint({ + serviceConfig, + request, + requestId, + loginHint: authRequest.loginHint, + organization, + }); - if (organization) { - command = { ...command, organization }; - } - - const res = await sendLoginname(command); - - if (res && "redirect" in res && res?.redirect) { - const absoluteUrl = constructUrl(request, res.redirect); - return NextResponse.redirect(absoluteUrl.toString()); - } - } catch (error) { - logger.error("Failed to execute sendLoginname:", { error }); - } + if (hintResponse) { + return hintResponse; } - const loginNameUrl = constructUrl(request, "/loginname"); - loginNameUrl.searchParams.set("requestId", requestId); - - if (authRequest.loginHint) { - loginNameUrl.searchParams.set("loginName", authRequest.loginHint); - } - if (organization) { - loginNameUrl.searchParams.set("organization", organization); - } - if (orgDomain) { - loginNameUrl.searchParams.set("orgDomain", orgDomain); - } - return NextResponse.redirect(loginNameUrl); + return gotoLoginname({ + request, + requestId, + loginHint: authRequest.loginHint, + organization, + orgDomain, + }); } else if (authRequest.prompt.includes(Prompt.NONE)) { let securitySettings: SecuritySettings | undefined; try { @@ -376,9 +412,22 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr let selectedSession = await findValidSession({ serviceConfig, sessions, authRequest, organization }); if (!selectedSession || !selectedSession.id) { - // If no sessions are eligible for this organization, go directly to - // loginname instead of showing an empty accounts page. - if (eligibleSessions.length === 0) { + // login_hint matches no session: resolve it straight to the next step. + const hintResponse = await resolveLoginHint({ + serviceConfig, + 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: the picker would only list non-matching accounts. + if (authRequest.loginHint || eligibleSessions.length === 0) { return gotoLoginname({ request, requestId, @@ -448,23 +497,26 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr } } } else { - const loginNameUrl = constructUrl(request, "/loginname"); - loginNameUrl.searchParams.set("requestId", requestId); + // No session: resolve a login_hint straight to the next step if we can. + const hintResponse = await resolveLoginHint({ + serviceConfig, + request, + requestId, + loginHint: authRequest?.loginHint, + organization, + }); - if (authRequest?.loginHint) { - loginNameUrl.searchParams.set("loginName", authRequest.loginHint); - loginNameUrl.searchParams.set("submit", "true"); + if (hintResponse) { + return hintResponse; } - if (organization) { - loginNameUrl.searchParams.set("organization", organization); - } - - if (orgDomain) { - loginNameUrl.searchParams.set("orgDomain", orgDomain); - } - - return NextResponse.redirect(loginNameUrl); + return gotoLoginname({ + request, + requestId, + loginHint: authRequest?.loginHint, + organization, + orgDomain, + }); } }