From 0bd9ff562a12db0e2be3efaf774e17fdbb26586a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 15:51:11 +0900 Subject: [PATCH 01/26] fix(auth): add safe browser sign-in recovery Keep identity-provider errors out of customer UI and offer a retry action that preserves the requested local return path. Exercise Authorization Code with PKCE, protected API access, callback rejection, and desktop/mobile rendering. Signed-off-by: Seongho Bae --- docs/storybook-inventory.md | 1 + frontend/e2e/smoke.spec.ts | 28 ++++++++++++++++-- frontend/e2e/support/auth.ts | 28 ++++++++++++++++-- frontend/playwright.config.ts | 5 ++++ frontend/src/App.test.tsx | 19 ++++++++++++ frontend/src/App.tsx | 14 ++++++++- .../src/components/SignInRecovery.stories.tsx | 24 +++++++++++++++ frontend/src/components/SignInRecovery.tsx | 29 +++++++++++++++++++ frontend/src/i18n.ts | 8 +++++ 9 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 frontend/src/components/SignInRecovery.stories.tsx create mode 100644 frontend/src/components/SignInRecovery.tsx diff --git a/docs/storybook-inventory.md b/docs/storybook-inventory.md index f426285a6..bcc9e97f9 100644 --- a/docs/storybook-inventory.md +++ b/docs/storybook-inventory.md @@ -21,6 +21,7 @@ operator-facing control you can click before changing product CSS. | `Lineage/LineageDag` | Open a reconstructed connection to read its inferred channel scores and Allen interval relation, or open the current branch node; compare empty, single-branch, grouped/forked, mobile-scroll, ungrouped, and long-title states before changing graph CSS. On narrow viewports, swipe the named viewport or focus it and use arrow keys to inspect the full lineage. | `--color-accent-background`, `--radius-control`, `--surface`, `--border`, `--color-focus-border`, `--size-control-min`, `LineageDag` | | `Chrome/StatusNotice` | Read success, unavailable, or retry copy, then take the named next action. Success and unavailable are a named region (not live `role=status`); Retry is `role=alert` and only on the retry kind. Calendar's missing Naruon projection uses unavailable. | `--badge-status-success-*`, `--badge-status-pending-*`, `--badge-status-danger-*`, `StatusNotice` | | `Chrome/PopupCloseButton` | Close the evidence panel or post popup. | `--space-close-inset`, `--font-size-close`, `PopupCloseButton` | +| `Chrome/Sign-in recovery` | Restart browser sign-in after an invalid or expired callback without exposing identity-provider details. `Desktop` and `NarrowViewport` cover both supported viewport classes. | `login-card`, `error`, `btn-primary`, `SignInRecovery` | | `Workspace/WorkspaceCalendar` | Read observed Naruon events, or open a commitment to land on that post. Fail-closed copy stays `이 범위의 일정을 아직 받을 수 없습니다`. | `--color-chip-border`, `WorkspaceCalendar`, `EvidenceStatusMark` | | `Ask Agent/Public claim verification` | Compare supported, refuted, and not-enough-information states; open only the external evidence link, then review the separate internal citation before changing governed graph state. | `--space-panel-block`, `--space-control-gap`, `--color-border`, `--size-control-min`, `PublicClaimVerification` | | `Ask Agent/Knowledge cutoff` | Exercise partial historical grounding, retained-revision provenance, later-live-change disclosure, and the narrow viewport before relying on a historical answer. | Native `datetime-local`, `--space-panel-block`, `--space-control-gap`, `--color-border`, `--size-control-min` | diff --git a/frontend/e2e/smoke.spec.ts b/frontend/e2e/smoke.spec.ts index af625adbb..295654265 100644 --- a/frontend/e2e/smoke.spec.ts +++ b/frontend/e2e/smoke.spec.ts @@ -1,7 +1,29 @@ import { expect, test } from "@playwright/test"; import { loginAsDemoAnalyst } from "./support/auth.ts"; -test("logs in and reaches an authenticated destination", async ({ page }) => { - await loginAsDemoAnalyst(page); - await expect(page.getByRole("button", { name: "Ask Agent" })).toBeVisible(); +test("logs in with PKCE, restores the requested URL, and reaches a protected destination", async ({ page }, testInfo) => { + const protectedResponse = page.waitForResponse((response) => + response.url().endsWith("/api/me") && response.request().method() === "GET" + ); + await loginAsDemoAnalyst(page, "/?auth_return=evidence#workspace"); + await expect(page).toHaveURL(/\/?auth_return=evidence#workspace$/); + expect((await protectedResponse).ok()).toBe(true); + await expect(page.getByRole("button", { name: "Log out" })).toBeVisible(); + await page.screenshot({ + path: testInfo.outputPath(`authenticated-${testInfo.project.name}.png`), + mask: [page.locator("main")], + }); +}); + +test("rejects a callback with unrecognized state without exposing provider details", async ({ page }, testInfo) => { + await page.goto("/?code=synthetic-invalid-code&state=synthetic-unrecognized-state#workspace"); + await expect(page.getByRole("alert")).toHaveText( + "Sign-in could not be completed. Start again to return to your work.", + ); + await expect(page.getByRole("button", { name: "Start sign-in again" })).toBeVisible(); + await expect(page.locator("body")).not.toContainText(/invalid_grant|correlation|state mismatch/i); + await page.screenshot({ + path: testInfo.outputPath(`rejected-callback-${testInfo.project.name}.png`), + fullPage: true, + }); }); diff --git a/frontend/e2e/support/auth.ts b/frontend/e2e/support/auth.ts index 5f3740ade..bd2e91ec8 100644 --- a/frontend/e2e/support/auth.ts +++ b/frontend/e2e/support/auth.ts @@ -16,12 +16,36 @@ const DEMO_PASSWORD = "lineageweave-demo-only"; * Next action: call this once per test before interacting with any * authenticated destination. */ -export async function loginAsDemoAnalyst(page: Page): Promise { - await page.goto("/"); +export async function loginAsDemoAnalyst( + page: Page, + returnPath = "/", +): Promise { + const tokenRequest = page.waitForRequest((request) => + request.url().includes("/protocol/openid-connect/token") && request.method() === "POST" + ); + await page.goto(returnPath); await page.getByRole("button", { name: "Log in" }).click(); await page.waitForURL(/\/realms\/lineageweave-demo\/protocol\/openid-connect\/auth/); + const authorizationUrl = new URL(page.url()); + if (authorizationUrl.searchParams.get("response_type") !== "code") { + throw new Error("browser sign-in did not request an authorization code"); + } + if (authorizationUrl.searchParams.get("code_challenge_method") !== "S256") { + throw new Error("browser sign-in did not request PKCE S256"); + } + if (!authorizationUrl.searchParams.get("code_challenge")) { + throw new Error("browser sign-in omitted the PKCE challenge"); + } await page.getByLabel("Username or email").fill(DEMO_USERNAME); await page.getByLabel("Password", { exact: true }).fill(DEMO_PASSWORD); await page.getByRole("button", { name: "Sign In" }).click(); await page.waitForURL((url) => !url.pathname.includes("/realms/")); + const exchange = await tokenRequest; + const form = new URLSearchParams(exchange.postData() ?? ""); + if (form.get("grant_type") !== "authorization_code" || !form.get("code_verifier")) { + throw new Error("browser sign-in did not exchange an authorization code with PKCE"); + } + if (form.has("username") || form.has("password")) { + throw new Error("browser sign-in attempted a password-token exchange"); + } } diff --git a/frontend/playwright.config.ts b/frontend/playwright.config.ts index a3fb286f5..acda27036 100644 --- a/frontend/playwright.config.ts +++ b/frontend/playwright.config.ts @@ -23,5 +23,10 @@ export default defineConfig({ name: "chromium", use: { ...devices["Desktop Chrome"] }, }, + { + name: "mobile-chromium", + testMatch: /smoke\.spec\.ts/, + use: { ...devices["Pixel 7"] }, + }, ], }); diff --git a/frontend/src/App.test.tsx b/frontend/src/App.test.tsx index 790c4da69..c2003ae24 100644 --- a/frontend/src/App.test.tsx +++ b/frontend/src/App.test.tsx @@ -97,6 +97,25 @@ describe("App, unauthenticated", () => { render(); expect(screen.getByRole("status")).toHaveTextContent("Loading authentication state..."); }); + + it("keeps provider errors private and offers a safe sign-in restart", async () => { + window.history.replaceState({}, "", "/?code=private-code&state=invalid#evidence"); + mockAuth = { + ...mockAuth, + error: new Error("invalid_grant: provider correlation details"), + }; + render(); + + expect(screen.getByRole("alert")).toHaveTextContent( + "Sign-in could not be completed. Start again to return to your work.", + ); + expect(screen.queryByText(/invalid_grant|provider correlation/i)).toBeNull(); + + await userEvent.click(screen.getByRole("button", { name: "Start sign-in again" })); + expect(signinRedirect).toHaveBeenCalledWith({ + state: { returnUrl: "/#evidence" }, + }); + }); }); function jsonResponse(body: unknown): Response { diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index e3fb6c796..ffdbea57a 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,5 +1,6 @@ import { focusedGraphMustReset } from "./focusedGraphSelection"; import { canAuthorVoice, postPrimaryVoiceLabel } from "./voicePerspective"; +import { SignInRecovery } from "./components/SignInRecovery"; import { Component, lazy, Suspense, useCallback, useEffect, useEffectEvent, useRef, useState, type ReactNode } from "react"; import { useAuth } from "react-oidc-context"; @@ -5302,7 +5303,18 @@ export default function App({ showLabPanels = false }: { showLabPanels?: boolean } if (auth.error) { - return

{t(auth.error.message)}

; + return ( + { + const returnUrl = returnUrlFromLocation(); + rememberOidcReturnUrl(returnUrl); + void auth.signinRedirect({ state: { returnUrl } }); + }} + /> + ); } if (!auth.isAuthenticated) { diff --git a/frontend/src/components/SignInRecovery.stories.tsx b/frontend/src/components/SignInRecovery.stories.tsx new file mode 100644 index 000000000..c8fe0cebf --- /dev/null +++ b/frontend/src/components/SignInRecovery.stories.tsx @@ -0,0 +1,24 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { fn } from "storybook/test"; +import { SignInRecovery } from "./SignInRecovery"; + +const meta = { + title: "Chrome/Sign-in recovery", + component: SignInRecovery, + args: { + brandName: "LineageWeave", + message: "Sign-in could not be completed. Start again to return to your work.", + actionLabel: "Start sign-in again", + onRetry: fn(), + }, + parameters: { layout: "fullscreen" }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Desktop: Story = {}; + +export const NarrowViewport: Story = { + parameters: { viewport: { defaultViewport: "mobile1" } }, +}; diff --git a/frontend/src/components/SignInRecovery.tsx b/frontend/src/components/SignInRecovery.tsx new file mode 100644 index 000000000..5c2be591a --- /dev/null +++ b/frontend/src/components/SignInRecovery.tsx @@ -0,0 +1,29 @@ +/** Offer a safe next action after the browser sign-in callback fails. */ +export function SignInRecovery({ + brandName, + message, + actionLabel, + onRetry, +}: { + brandName: string; + message: string; + actionLabel: string; + onRetry: () => void; +}) { + return ( +
+
+
+
+

{brandName}

+

Marketing & Operational Lineage Intelligence

+
+
+

{message}

+ +
+
+
+
+ ); +} diff --git a/frontend/src/i18n.ts b/frontend/src/i18n.ts index bfe170ef8..5c91e5885 100644 --- a/frontend/src/i18n.ts +++ b/frontend/src/i18n.ts @@ -50,6 +50,8 @@ const TRANSLATIONS: Partial>> = { "Loading authentication state...": "인증 상태를 불러오는 중...", "Authenticated, but no access token was returned.": "인증되었지만 액세스 토큰이 반환되지 않았습니다.", "Log in": "로그인", + "Sign-in could not be completed. Start again to return to your work.": "로그인을 완료하지 못했습니다. 다시 시작하면 작업 화면으로 돌아갑니다.", + "Start sign-in again": "로그인 다시 시작", "Log out": "로그아웃", Calendar: "캘린더", Rankings: "순위", @@ -639,6 +641,8 @@ const TRANSLATIONS: Partial>> = { "Loading authentication state...": "正在加载身份验证状态...", "Authenticated, but no access token was returned.": "已完成身份验证,但未返回访问令牌。", "Log in": "登录", + "Sign-in could not be completed. Start again to return to your work.": "无法完成登录。请重新开始以返回工作页面。", + "Start sign-in again": "重新登录", "Log out": "退出登录", Calendar: "日历", Rankings: "排名", @@ -1243,6 +1247,8 @@ const TRANSLATIONS: Partial>> = { "Loading authentication state...": "認証状態を読み込んでいます...", "Authenticated, but no access token was returned.": "認証済みですが、アクセストークンが返されませんでした。", "Log in": "ログイン", + "Sign-in could not be completed. Start again to return to your work.": "サインインを完了できませんでした。もう一度開始すると作業画面に戻ります。", + "Start sign-in again": "サインインをやり直す", "Log out": "ログアウト", Calendar: "カレンダー", Rankings: "ランキング", @@ -1827,6 +1833,8 @@ const TRANSLATIONS: Partial>> = { "Loading authentication state...": "Đang tải trạng thái xác thực...", "Authenticated, but no access token was returned.": "Đã xác thực nhưng không nhận được mã thông báo truy cập.", "Log in": "Đăng nhập", + "Sign-in could not be completed. Start again to return to your work.": "Không thể hoàn tất đăng nhập. Hãy bắt đầu lại để trở về công việc của bạn.", + "Start sign-in again": "Đăng nhập lại", "Log out": "Đăng xuất", Calendar: "Lịch", Rankings: "Xếp hạng", From d19da523bcf952d8fdb27cceff5ab38f458fdfd4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 16:46:24 +0900 Subject: [PATCH 02/26] test(auth): reject failed OIDC callback fields in return URLs --- frontend/src/oidcReturnUrl.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index 2ecddb1c1..6c6002bff 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -30,6 +30,17 @@ describe("OIDC return URL handling", () => { expect(cleaned).toBe("/?post=abc"); }); + it("strips OAuth/OIDC error response fields before retrying a failed sign-in", () => { + const cleaned = returnUrlFromLocation({ + pathname: "/", + search: + "?post=abc&error=access_denied&error_description=provider%20correlation%20details&error_uri=https%3A%2F%2Fidp.example%2Ferrors%2F42&state=s", + hash: "#evidence", + }); + + expect(cleaned).toBe("/?post=abc#evidence"); + }); + it("restores an object or serialized OIDC state before storage fallback", () => { rememberOidcReturnUrl("/?post=stored-before-direct"); expect(restoreOidcReturnUrl("/?post=from-direct-state")).toBe( From 81f885d322d09253786eb48622deaf941743aa2d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 16:46:39 +0900 Subject: [PATCH 03/26] fix(auth): strip OAuth error fields from retry return URLs --- frontend/src/oidcReturnUrl.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index 5d478668c..45c0653ad 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -1,11 +1,20 @@ export const OIDC_RETURN_URL_STORAGE_KEY = "lineageweave.oidc.returnUrl"; const MAX_OIDC_RETURN_URL_LENGTH = 4096; -/** Authorization-code response params Keycloak appends to the redirect URI - * (RFC 6749 sec. 4.1.2; `session_state` per OIDC Session Management). Any - * link built from `window.location` must strip these -- they're a one-time - * auth exchange, never part of a shareable URL. */ -const OIDC_CALLBACK_PARAMS = ["code", "state", "session_state", "iss"] as const; +/** Authorization-endpoint response params appended to the redirect URI. + * Success responses use `code`/`state`; OAuth error responses may add + * `error`, `error_description`, and `error_uri` (RFC 6749 sec. 4.1.2/4.1.2.1). + * `session_state` and `iss` are OIDC/session-response metadata. None belongs + * in a shareable or retried product return URL. */ +const OIDC_CALLBACK_PARAMS = [ + "code", + "state", + "session_state", + "iss", + "error", + "error_description", + "error_uri", +] as const; /** Removes OIDC callback artifacts from `url` in place -- call before turning * `window.location` into a link a user can copy or share. */ From 05ea958ccaa2f1662a12665841f2fe448645e8c3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 16:49:38 +0900 Subject: [PATCH 04/26] docs(auth): align OIDC deep-link ADR with callback scrubbing --- docs/adr/0109-oidc-deep-link-state-recovery.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index 808f12aff..225753bc3 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -12,6 +12,12 @@ tab's `sessionStorage` is not available. Falling back to `/` loses the post deep link and presents the unauthenticated language/login surface again, even when the member's OIDC session is otherwise valid. +The authorization endpoint can return either a successful code response or an +OAuth error response. Those response fields are one-time protocol artifacts, +not application navigation state. If a failed callback is turned back into a +remembered return URL, provider error fields can be replayed on the next +successful sign-in and can expose provider detail in a product-controlled URL. + ## Decision - Keep the OIDC `state.returnUrl` as the first recovery source. @@ -21,6 +27,10 @@ when the member's OIDC session is otherwise valid. - Persist the same validated same-origin path in both `sessionStorage` and `localStorage` before redirecting to OIDC. `localStorage` is only a bounded recovery fallback, not an authentication or authorization store. +- Before deriving, storing, sharing, or retrying a return URL from the browser + location, remove authorization-response artifacts: `code`, `state`, + `session_state`, `iss`, `error`, `error_description`, and `error_uri`. + Preserve unrelated same-origin product query parameters and the fragment. - On callback, remove the key from both stores and use session storage before local storage. Reject external and protocol-relative URLs. - Keep member language preference account-scoped in @@ -30,6 +40,7 @@ when the member's OIDC session is otherwise valid. ## Consequences Opening a shared post link survives a missing OIDC state payload or a changed -storage context without losing the post. A stale internal return path is -removed at callback, and authorization still comes only from the authenticated -OIDC token and backend ABAC checks. +storage context without losing the post. Successful and failed authorization +response fields are not minted into a later product return path. A stale +internal return path is removed at callback, and authorization still comes only +from the authenticated OIDC token and backend ABAC checks. From 3bf1c3700f818e59f5bf0e8d5c8f106ce1523740 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 17:46:26 +0900 Subject: [PATCH 05/26] test(auth): reject callback artifacts in persisted return URLs --- frontend/src/oidcReturnUrl.test.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index 6c6002bff..4b259b287 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -41,6 +41,31 @@ describe("OIDC return URL handling", () => { expect(cleaned).toBe("/?post=abc#evidence"); }); + it("never persists or restores authorization response artifacts", () => { + rememberOidcReturnUrl( + "/?post=stored&code=private-code&state=private-state&error=access_denied&error_description=provider-detail#evidence", + ); + expect(window.sessionStorage.getItem("lineageweave.oidc.returnUrl")).toBe( + "/?post=stored#evidence", + ); + expect(window.localStorage.getItem("lineageweave.oidc.returnUrl")).toBe( + "/?post=stored#evidence", + ); + + expect( + restoreOidcReturnUrl({ + returnUrl: + "/?post=state&code=stale-code&state=stale-state&error_uri=https%3A%2F%2Fidp.example%2Ferror#workspace", + }), + ).toBe("/?post=state#workspace"); + + window.sessionStorage.setItem( + "lineageweave.oidc.returnUrl", + "/?post=legacy&session_state=legacy-session&iss=https%3A%2F%2Fidp.example#error", + ); + expect(restoreOidcReturnUrl(undefined)).toBe("/?post=legacy#error"); + }); + it("restores an object or serialized OIDC state before storage fallback", () => { rememberOidcReturnUrl("/?post=stored-before-direct"); expect(restoreOidcReturnUrl("/?post=from-direct-state")).toBe( From 0a59cb3c5e20996b84f6363c7c560588a037eff5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 17:51:54 +0900 Subject: [PATCH 06/26] fix(auth): sanitize persisted and state return URLs --- frontend/src/oidcReturnUrl.ts | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index 45c0653ad..9ad4d4199 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -32,27 +32,33 @@ function isSafeReturnUrl(value: string): boolean { ); } +function sanitizeReturnUrl(value: string): string { + if (!isSafeReturnUrl(value)) return ""; + const url = new URL(value, "https://lineageweave.invalid"); + stripOidcCallbackParams(url); + const cleaned = `${url.pathname}${url.search}${url.hash}`; + return isSafeReturnUrl(cleaned) ? cleaned : ""; +} + export function returnUrlFromLocation(location: UrlLike = window.location): string { // A restored return URL can itself be a post-redirect URL still carrying // Keycloak callback artifacts (devin review thread on PR #576): strip // them here too, so no consumer of this module re-mints a URL with a // one-time authorization code in it. - const params = new URLSearchParams(location.search); - OIDC_CALLBACK_PARAMS.forEach((param) => params.delete(param)); - const cleanedSearch = params.toString(); - const value = `${location.pathname}${cleanedSearch ? `?${cleanedSearch}` : ""}${location.hash}`; - return isSafeReturnUrl(value) ? value : "/"; + const value = `${location.pathname}${location.search}${location.hash}`; + return sanitizeReturnUrl(value) || "/"; } export function rememberOidcReturnUrl(value: string): void { - if (!isSafeReturnUrl(value)) return; + const cleaned = sanitizeReturnUrl(value); + if (!cleaned) return; try { - window.sessionStorage.setItem(OIDC_RETURN_URL_STORAGE_KEY, value); + window.sessionStorage.setItem(OIDC_RETURN_URL_STORAGE_KEY, cleaned); } catch { // OIDC state remains the fallback when session storage is unavailable. } try { - window.localStorage.setItem(OIDC_RETURN_URL_STORAGE_KEY, value); + window.localStorage.setItem(OIDC_RETURN_URL_STORAGE_KEY, cleaned); } catch { // The OIDC state and session storage remain the fallbacks. } @@ -61,7 +67,8 @@ export function rememberOidcReturnUrl(value: string): void { function stateReturnUrl(state: unknown): string { let candidate = state; if (typeof candidate === "string") { - if (isSafeReturnUrl(candidate)) return candidate; + const cleaned = sanitizeReturnUrl(candidate); + if (cleaned) return cleaned; if (candidate.length > MAX_OIDC_RETURN_URL_LENGTH) return ""; try { candidate = JSON.parse(candidate); @@ -73,7 +80,7 @@ function stateReturnUrl(state: unknown): string { return ""; } const value = (candidate as { returnUrl?: unknown }).returnUrl; - return typeof value === "string" && isSafeReturnUrl(value) ? value : ""; + return typeof value === "string" ? sanitizeReturnUrl(value) : ""; } export function restoreOidcReturnUrl(state: unknown): string { @@ -93,8 +100,10 @@ export function restoreOidcReturnUrl(state: unknown): string { // Fall through to the current path. } if (fromState) return fromState; - if (isSafeReturnUrl(sessionStored)) return sessionStored; - if (isSafeReturnUrl(localStored)) return localStored; + const fromSession = sanitizeReturnUrl(sessionStored); + if (fromSession) return fromSession; + const fromLocal = sanitizeReturnUrl(localStored); + if (fromLocal) return fromLocal; return new URLSearchParams(window.location.search).has("post") ? returnUrlFromLocation() : window.location.pathname; From b536b11d456266e806df3ef70a15b789beff695a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 17:52:13 +0900 Subject: [PATCH 07/26] docs(adr): sanitize every OIDC return-path admission --- docs/adr/0109-oidc-deep-link-state-recovery.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index 225753bc3..4e4376301 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -27,10 +27,13 @@ successful sign-in and can expose provider detail in a product-controlled URL. - Persist the same validated same-origin path in both `sessionStorage` and `localStorage` before redirecting to OIDC. `localStorage` is only a bounded recovery fallback, not an authentication or authorization store. -- Before deriving, storing, sharing, or retrying a return URL from the browser - location, remove authorization-response artifacts: `code`, `state`, - `session_state`, `iss`, `error`, `error_description`, and `error_uri`. - Preserve unrelated same-origin product query parameters and the fragment. +- On every return-path admission boundary — current browser location, + `state.returnUrl`, `sessionStorage`, and `localStorage` — and before writing a + return path back to storage, remove authorization-response artifacts: `code`, + `state`, `session_state`, `iss`, `error`, `error_description`, and + `error_uri`. Preserve unrelated same-origin product query parameters and the + fragment. This also cleans values persisted by an older client before this + boundary existed. - On callback, remove the key from both stores and use session storage before local storage. Reject external and protocol-relative URLs. - Keep member language preference account-scoped in @@ -41,6 +44,7 @@ successful sign-in and can expose provider detail in a product-controlled URL. Opening a shared post link survives a missing OIDC state payload or a changed storage context without losing the post. Successful and failed authorization -response fields are not minted into a later product return path. A stale -internal return path is removed at callback, and authorization still comes only -from the authenticated OIDC token and backend ABAC checks. +response fields are not minted into a later product return path, including when +an older stored/state value is recovered. A stale internal return path is +removed at callback, and authorization still comes only from the authenticated +OIDC token and backend ABAC checks. From ccdde2ae235ef4e291ce5dc75fa0b9dd5e54350c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:16:43 +0900 Subject: [PATCH 08/26] test(auth): preserve remembered deep link on retry --- frontend/src/oidcReturnUrl.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index 4b259b287..df982d9aa 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -41,6 +41,18 @@ describe("OIDC return URL handling", () => { expect(cleaned).toBe("/?post=abc#evidence"); }); + it("retries an OIDC error callback with the remembered pre-redirect deep link", () => { + rememberOidcReturnUrl("/?post=requested#evidence"); + + const retryTarget = returnUrlFromLocation({ + pathname: "/", + search: "?error=access_denied&error_description=provider-detail&state=callback-state", + hash: "", + }); + + expect(retryTarget).toBe("/?post=requested#evidence"); + }); + it("never persists or restores authorization response artifacts", () => { rememberOidcReturnUrl( "/?post=stored&code=private-code&state=private-state&error=access_denied&error_description=provider-detail#evidence", From 192c78da6eb9e015f74d2c2aa07b00382fbf592e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:17:03 +0900 Subject: [PATCH 09/26] fix(auth): keep requested deep link across retry --- frontend/src/oidcReturnUrl.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index 9ad4d4199..fc5250871 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -40,7 +40,37 @@ function sanitizeReturnUrl(value: string): string { return isSafeReturnUrl(cleaned) ? cleaned : ""; } +function isOidcCallbackLocation(location: UrlLike): boolean { + const params = new URLSearchParams(location.search); + return OIDC_CALLBACK_PARAMS.some((param) => params.has(param)); +} + +function peekRememberedReturnUrl(): string { + try { + const sessionStored = window.sessionStorage.getItem(OIDC_RETURN_URL_STORAGE_KEY) ?? ""; + const fromSession = sanitizeReturnUrl(sessionStored); + if (fromSession) return fromSession; + } catch { + // Fall through to local storage or the current callback location. + } + try { + const localStored = window.localStorage.getItem(OIDC_RETURN_URL_STORAGE_KEY) ?? ""; + return sanitizeReturnUrl(localStored); + } catch { + return ""; + } +} + export function returnUrlFromLocation(location: UrlLike = window.location): string { + // A provider callback is rooted at redirect_uri, so rebuilding a retry URL + // from that callback alone can erase the deep link remembered before the + // redirect. Prefer the validated remembered path while callback artifacts + // are still present; ordinary product navigation continues to use location. + if (isOidcCallbackLocation(location)) { + const remembered = peekRememberedReturnUrl(); + if (remembered) return remembered; + } + // A restored return URL can itself be a post-redirect URL still carrying // Keycloak callback artifacts (devin review thread on PR #576): strip // them here too, so no consumer of this module re-mints a URL with a From eddfed0d505b60a36050b501472ce0c28f1f94e9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:17:19 +0900 Subject: [PATCH 10/26] docs(adr): preserve pre-redirect path on OIDC retry --- .../adr/0109-oidc-deep-link-state-recovery.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index 4e4376301..5f8d928ff 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -17,6 +17,9 @@ OAuth error response. Those response fields are one-time protocol artifacts, not application navigation state. If a failed callback is turned back into a remembered return URL, provider error fields can be replayed on the next successful sign-in and can expose provider detail in a product-controlled URL. +A provider callback is also rooted at the configured redirect URI rather than +the original buyer deep link, so rebuilding retry state from the failed +callback alone can overwrite the path that was remembered before redirect. ## Decision @@ -34,6 +37,10 @@ successful sign-in and can expose provider detail in a product-controlled URL. `error_uri`. Preserve unrelated same-origin product query parameters and the fragment. This also cleans values persisted by an older client before this boundary existed. +- When retrying while the browser is still on an OIDC success/error callback, + prefer the validated path remembered before redirect over the callback's + sanitized redirect-URI path. Ordinary product navigation without callback + artifacts continues to derive its return path from the current location. - On callback, remove the key from both stores and use session storage before local storage. Reject external and protocol-relative URLs. - Keep member language preference account-scoped in @@ -43,8 +50,10 @@ successful sign-in and can expose provider detail in a product-controlled URL. ## Consequences Opening a shared post link survives a missing OIDC state payload or a changed -storage context without losing the post. Successful and failed authorization -response fields are not minted into a later product return path, including when -an older stored/state value is recovered. A stale internal return path is -removed at callback, and authorization still comes only from the authenticated -OIDC token and backend ABAC checks. +storage context without losing the post. A failed provider callback no longer +replaces the pre-redirect deep link with `/` merely because the callback was +rooted at the redirect URI. Successful and failed authorization response fields +are not minted into a later product return path, including when an older +stored/state value is recovered. A stale internal return path is removed at +callback, and authorization still comes only from the authenticated OIDC token +and backend ABAC checks. From 5c648e2e48e16928484b72a7400262108e98fb00 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:19:34 +0900 Subject: [PATCH 11/26] test(auth): distinguish callback signal from lone state --- frontend/src/oidcReturnUrl.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index df982d9aa..e4192976f 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -53,6 +53,18 @@ describe("OIDC return URL handling", () => { expect(retryTarget).toBe("/?post=requested#evidence"); }); + it("does not let a lone state parameter make stale storage override current product navigation", () => { + rememberOidcReturnUrl("/?post=stale"); + + const currentTarget = returnUrlFromLocation({ + pathname: "/", + search: "?post=current&state=product-state", + hash: "#workspace", + }); + + expect(currentTarget).toBe("/?post=current#workspace"); + }); + it("never persists or restores authorization response artifacts", () => { rememberOidcReturnUrl( "/?post=stored&code=private-code&state=private-state&error=access_denied&error_description=provider-detail#evidence", From caa77b62d05d34b715b7f20b48206b85d953be50 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:19:58 +0900 Subject: [PATCH 12/26] fix(auth): require real callback signal before storage precedence --- frontend/src/oidcReturnUrl.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index fc5250871..80c8dc297 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -16,6 +16,18 @@ const OIDC_CALLBACK_PARAMS = [ "error_uri", ] as const; +/** A callback must carry something stronger than a lone `state` parameter. + * `state` is scrubbed when present, but by itself it must not let stale auth + * storage override an otherwise current product URL. */ +const OIDC_CALLBACK_SIGNAL_PARAMS = [ + "code", + "session_state", + "iss", + "error", + "error_description", + "error_uri", +] as const; + /** Removes OIDC callback artifacts from `url` in place -- call before turning * `window.location` into a link a user can copy or share. */ export function stripOidcCallbackParams(url: URL): void { @@ -42,7 +54,7 @@ function sanitizeReturnUrl(value: string): string { function isOidcCallbackLocation(location: UrlLike): boolean { const params = new URLSearchParams(location.search); - return OIDC_CALLBACK_PARAMS.some((param) => params.has(param)); + return OIDC_CALLBACK_SIGNAL_PARAMS.some((param) => params.has(param)); } function peekRememberedReturnUrl(): string { From fa819f29a6019d9f90d33ee15cadda4238eaec54 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:21:34 +0900 Subject: [PATCH 13/26] docs(adr): distinguish callback evidence from lone state --- .../adr/0109-oidc-deep-link-state-recovery.md | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index 5f8d928ff..e7574622f 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -39,8 +39,12 @@ callback alone can overwrite the path that was remembered before redirect. boundary existed. - When retrying while the browser is still on an OIDC success/error callback, prefer the validated path remembered before redirect over the callback's - sanitized redirect-URI path. Ordinary product navigation without callback - artifacts continues to derive its return path from the current location. + sanitized redirect-URI path. Treat `code`, an OAuth `error*` field, or OIDC + response metadata such as `session_state`/`iss` as callback evidence; + `state` alone is scrubbed as reserved protocol data but does not authorize + stale browser storage to override an otherwise current product URL. +- Ordinary product navigation without callback evidence continues to derive + its return path from the current location. - On callback, remove the key from both stores and use session storage before local storage. Reject external and protocol-relative URLs. - Keep member language preference account-scoped in @@ -52,8 +56,9 @@ callback alone can overwrite the path that was remembered before redirect. Opening a shared post link survives a missing OIDC state payload or a changed storage context without losing the post. A failed provider callback no longer replaces the pre-redirect deep link with `/` merely because the callback was -rooted at the redirect URI. Successful and failed authorization response fields -are not minted into a later product return path, including when an older -stored/state value is recovered. A stale internal return path is removed at -callback, and authorization still comes only from the authenticated OIDC token -and backend ABAC checks. +rooted at the redirect URI, while an unrelated lone `state` query cannot make +stale return-path storage win over current product navigation. Successful and +failed authorization response fields are not minted into a later product +return path, including when an older stored/state value is recovered. A stale +internal return path is removed at callback, and authorization still comes only +from the authenticated OIDC token and backend ABAC checks. From ba951dde63eacbe51553f67d0a3292fd948e3fb5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:46:55 +0900 Subject: [PATCH 14/26] test(auth): reject backslash authority return paths --- frontend/src/oidcReturnUrl.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index e4192976f..7f0491c16 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -17,6 +17,14 @@ describe("OIDC return URL handling", () => { "/?post=abc#evidence", ); expect(returnUrlFromLocation({ pathname: "//evil.example", search: "", hash: "" })).toBe("/"); + expect( + returnUrlFromLocation({ + pathname: "/\\evil.example/forged", + search: "?post=attacker", + hash: "#workspace", + }), + ).toBe("/"); + expect(restoreOidcReturnUrl({ returnUrl: "/\\evil.example/forged?post=attacker" })).toBe("/"); }); it("strips OIDC callback params from a restored post-redirect location", () => { From d310514cd2e62e1e97c6578644aec9bdb43fc0f7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:47:20 +0900 Subject: [PATCH 15/26] fix(auth): enforce same-origin return URL parsing --- frontend/src/oidcReturnUrl.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index 80c8dc297..bdec445e2 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -1,5 +1,6 @@ export const OIDC_RETURN_URL_STORAGE_KEY = "lineageweave.oidc.returnUrl"; const MAX_OIDC_RETURN_URL_LENGTH = 4096; +const OIDC_RETURN_URL_BASE = "https://lineageweave.invalid"; /** Authorization-endpoint response params appended to the redirect URI. * Success responses use `code`/`state`; OAuth error responses may add @@ -46,7 +47,11 @@ function isSafeReturnUrl(value: string): boolean { function sanitizeReturnUrl(value: string): string { if (!isSafeReturnUrl(value)) return ""; - const url = new URL(value, "https://lineageweave.invalid"); + const url = new URL(value, OIDC_RETURN_URL_BASE); + // WHATWG URL parsing treats backslashes as authority separators for special + // schemes. Reject any value that parses away from the product origin rather + // than silently re-minting its path as a local deep link. + if (url.origin !== OIDC_RETURN_URL_BASE) return ""; stripOidcCallbackParams(url); const cleaned = `${url.pathname}${url.search}${url.hash}`; return isSafeReturnUrl(cleaned) ? cleaned : ""; From 2e7bd6ca9a2bff66d06abf2b4bf0c8938c8ea15a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 18:47:39 +0900 Subject: [PATCH 16/26] docs(adr): pin parsed same-origin return URL boundary --- docs/adr/0109-oidc-deep-link-state-recovery.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index e7574622f..333f0d45f 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -21,12 +21,22 @@ A provider callback is also rooted at the configured redirect URI rather than the original buyer deep link, so rebuilding retry state from the failed callback alone can overwrite the path that was remembered before redirect. +A lexical leading-slash check is not sufficient to prove that a candidate is a +same-origin path. WHATWG URL parsing treats backslashes as authority separators +for special schemes, so a value such as `/\\example.invalid/path` can begin +with a single slash yet parse to a different origin. Reconstructing only the +parsed pathname would then silently turn an external-shaped value into a new +local deep link rather than rejecting the invalid admission. + ## Decision - Keep the OIDC `state.returnUrl` as the first recovery source. - Accept only a direct same-origin path, one bounded serialized object, or one object value. Never recursively parse JSON-encoded strings; reject serialized state and return paths longer than 4,096 characters before further handling. +- Validate return paths after WHATWG parsing against the product origin, not + only by string prefix. Reject any candidate whose parsed origin differs; + never strip an unexpected authority and re-mint only its pathname as local. - Persist the same validated same-origin path in both `sessionStorage` and `localStorage` before redirecting to OIDC. `localStorage` is only a bounded recovery fallback, not an authentication or authorization store. @@ -59,6 +69,8 @@ replaces the pre-redirect deep link with `/` merely because the callback was rooted at the redirect URI, while an unrelated lone `state` query cannot make stale return-path storage win over current product navigation. Successful and failed authorization response fields are not minted into a later product -return path, including when an older stored/state value is recovered. A stale +return path, including when an older stored/state value is recovered. Inputs +that only look path-relative before parsing but resolve to another origin are +rejected instead of being host-stripped into a different local path. A stale internal return path is removed at callback, and authorization still comes only from the authenticated OIDC token and backend ABAC checks. From 80816667c2023a6a0c3f204d2563ec7d77573cbe Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 19:47:05 +0900 Subject: [PATCH 17/26] test(auth): require correlated callback evidence before storage precedence --- frontend/src/oidcReturnUrl.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index 7f0491c16..5145696ce 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -73,6 +73,25 @@ describe("OIDC return URL handling", () => { expect(currentTarget).toBe("/?post=current#workspace"); }); + it("does not let uncorrelated response-looking params make stale storage override current navigation", () => { + rememberOidcReturnUrl("/?post=stale"); + + expect( + returnUrlFromLocation({ + pathname: "/", + search: "?post=current&error=validation_failed", + hash: "#workspace", + }), + ).toBe("/?post=current#workspace"); + expect( + returnUrlFromLocation({ + pathname: "/", + search: "?post=current&code=customer-code", + hash: "#workspace", + }), + ).toBe("/?post=current#workspace"); + }); + it("never persists or restores authorization response artifacts", () => { rememberOidcReturnUrl( "/?post=stored&code=private-code&state=private-state&error=access_denied&error_description=provider-detail#evidence", From 3873ef4d15900d14b9b716fe968876a747e570a7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 19:47:35 +0900 Subject: [PATCH 18/26] fix(auth): require correlated state before callback storage precedence --- frontend/src/oidcReturnUrl.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index bdec445e2..68dbcbc83 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -17,9 +17,10 @@ const OIDC_CALLBACK_PARAMS = [ "error_uri", ] as const; -/** A callback must carry something stronger than a lone `state` parameter. - * `state` is scrubbed when present, but by itself it must not let stale auth - * storage override an otherwise current product URL. */ +/** A correlated callback carries the client-supplied `state` plus a response + * signal. `state` alone is not sufficient, and response-looking query names + * without `state` must not let stale auth storage override current product + * navigation. */ const OIDC_CALLBACK_SIGNAL_PARAMS = [ "code", "session_state", @@ -59,7 +60,10 @@ function sanitizeReturnUrl(value: string): string { function isOidcCallbackLocation(location: UrlLike): boolean { const params = new URLSearchParams(location.search); - return OIDC_CALLBACK_SIGNAL_PARAMS.some((param) => params.has(param)); + return ( + params.has("state") && + OIDC_CALLBACK_SIGNAL_PARAMS.some((param) => params.has(param)) + ); } function peekRememberedReturnUrl(): string { @@ -81,8 +85,8 @@ function peekRememberedReturnUrl(): string { export function returnUrlFromLocation(location: UrlLike = window.location): string { // A provider callback is rooted at redirect_uri, so rebuilding a retry URL // from that callback alone can erase the deep link remembered before the - // redirect. Prefer the validated remembered path while callback artifacts - // are still present; ordinary product navigation continues to use location. + // redirect. Prefer the validated remembered path only for a correlated + // callback; ordinary product navigation continues to use location. if (isOidcCallbackLocation(location)) { const remembered = peekRememberedReturnUrl(); if (remembered) return remembered; From 395742312682011624cd9a03d2fd8a5486249f89 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 19:48:09 +0900 Subject: [PATCH 19/26] docs(adr): require correlated state for callback precedence --- .../adr/0109-oidc-deep-link-state-recovery.md | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index 333f0d45f..f1166ad4b 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -21,6 +21,15 @@ A provider callback is also rooted at the configured redirect URI rather than the original buyer deep link, so rebuilding retry state from the failed callback alone can overwrite the path that was remembered before redirect. +The browser also accepts arbitrary product query parameters on the SPA root. +A response-shaped query name such as `code` or `error` therefore cannot, by +itself, prove that the current URL is the response to the OIDC transaction that +created remembered return-path storage. LineageWeave sends `state` on the +Authorization Code request; OAuth 2.0 and OpenID Connect require that value to +be returned on both success and error responses when it was present in the +request. Remembered-path precedence therefore needs both the returned `state` +and a response signal. `state` alone is likewise insufficient. + A lexical leading-slash check is not sufficient to prove that a candidate is a same-origin path. WHATWG URL parsing treats backslashes as authority separators for special schemes, so a value such as `/\\example.invalid/path` can begin @@ -49,12 +58,16 @@ local deep link rather than rejecting the invalid admission. boundary existed. - When retrying while the browser is still on an OIDC success/error callback, prefer the validated path remembered before redirect over the callback's - sanitized redirect-URI path. Treat `code`, an OAuth `error*` field, or OIDC - response metadata such as `session_state`/`iss` as callback evidence; - `state` alone is scrubbed as reserved protocol data but does not authorize - stale browser storage to override an otherwise current product URL. -- Ordinary product navigation without callback evidence continues to derive - its return path from the current location. + sanitized redirect-URI path only when `state` is present together with a + response signal (`code`, an OAuth `error*` field, `session_state`, or `iss`). + `state` alone and response-looking query names without `state` are still + scrubbed as reserved protocol data, but neither condition authorizes stale + browser storage to override otherwise current product navigation. This is + consistent with RFC 6749 §§4.1.2 and 4.1.2.1 and OpenID Connect Core 1.0, + which require the authorization response to return `state` when the request + supplied it. +- Ordinary product navigation without correlated callback evidence continues + to derive its return path from the current location. - On callback, remove the key from both stores and use session storage before local storage. Reject external and protocol-relative URLs. - Keep member language preference account-scoped in @@ -66,11 +79,19 @@ local deep link rather than rejecting the invalid admission. Opening a shared post link survives a missing OIDC state payload or a changed storage context without losing the post. A failed provider callback no longer replaces the pre-redirect deep link with `/` merely because the callback was -rooted at the redirect URI, while an unrelated lone `state` query cannot make -stale return-path storage win over current product navigation. Successful and -failed authorization response fields are not minted into a later product -return path, including when an older stored/state value is recovered. Inputs -that only look path-relative before parsing but resolve to another origin are -rejected instead of being host-stripped into a different local path. A stale -internal return path is removed at callback, and authorization still comes only -from the authenticated OIDC token and backend ABAC checks. +rooted at the redirect URI, while an unrelated lone `state`, `code`, `error`, +or other response-shaped query cannot make stale return-path storage win over +current product navigation without the correlated `state` + response-signal +shape. Successful and failed authorization response fields are not minted into +a later product return path, including when an older stored/state value is +recovered. Inputs that only look path-relative before parsing but resolve to +another origin are rejected instead of being host-stripped into a different +local path. A stale internal return path is removed at callback, and +authorization still comes only from the authenticated OIDC token and backend +ABAC checks. + +## References + +Hardt, D. (2012). *The OAuth 2.0 authorization framework* (RFC 6749). Internet Engineering Task Force. + +OpenID Foundation. (2014). *OpenID Connect Core 1.0 incorporating errata set 2*. From f82b38498bca83254ce6bcab0ac4174fa52a0d18 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 20:44:51 +0900 Subject: [PATCH 20/26] test(auth): require primary OIDC response signal for retry precedence --- frontend/src/oidcReturnUrl.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index 5145696ce..765bc07fc 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -92,6 +92,25 @@ describe("OIDC return URL handling", () => { ).toBe("/?post=current#workspace"); }); + it("requires a primary code or error response before ancillary OIDC metadata can activate remembered-path precedence", () => { + rememberOidcReturnUrl("/?post=stale"); + + for (const search of [ + "?post=current&state=product-state&session_state=provider-session", + "?post=current&state=product-state&iss=https%3A%2F%2Fidp.example", + "?post=current&state=product-state&error_description=provider-detail", + "?post=current&state=product-state&error_uri=https%3A%2F%2Fidp.example%2Ferrors%2F42", + ]) { + expect( + returnUrlFromLocation({ + pathname: "/", + search, + hash: "#workspace", + }), + ).toBe("/?post=current#workspace"); + } + }); + it("never persists or restores authorization response artifacts", () => { rememberOidcReturnUrl( "/?post=stored&code=private-code&state=private-state&error=access_denied&error_description=provider-detail#evidence", From d0d2026c71ce39c595dab2267f4ecfede19d25d3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 20:45:09 +0900 Subject: [PATCH 21/26] fix(auth): require primary OIDC response signal for retry precedence --- frontend/src/oidcReturnUrl.ts | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index 68dbcbc83..8291829ac 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -17,18 +17,12 @@ const OIDC_CALLBACK_PARAMS = [ "error_uri", ] as const; -/** A correlated callback carries the client-supplied `state` plus a response - * signal. `state` alone is not sufficient, and response-looking query names - * without `state` must not let stale auth storage override current product - * navigation. */ -const OIDC_CALLBACK_SIGNAL_PARAMS = [ - "code", - "session_state", - "iss", - "error", - "error_description", - "error_uri", -] as const; +/** A correlated Authorization Code response carries client `state` plus a + * primary response member: `code` on success or `error` on failure. Ancillary + * metadata such as `iss`, `session_state`, `error_description`, or `error_uri` + * never proves a response by itself and must not let stale auth storage + * override current product navigation. */ +const OIDC_CALLBACK_PRIMARY_PARAMS = ["code", "error"] as const; /** Removes OIDC callback artifacts from `url` in place -- call before turning * `window.location` into a link a user can copy or share. */ @@ -62,7 +56,7 @@ function isOidcCallbackLocation(location: UrlLike): boolean { const params = new URLSearchParams(location.search); return ( params.has("state") && - OIDC_CALLBACK_SIGNAL_PARAMS.some((param) => params.has(param)) + OIDC_CALLBACK_PRIMARY_PARAMS.some((param) => params.has(param)) ); } From fca1b35669f1deac5a780058ca478a7866bafbe6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 20:45:32 +0900 Subject: [PATCH 22/26] docs(adr): require primary OAuth outcome for retry precedence --- .../adr/0109-oidc-deep-link-state-recovery.md | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index f1166ad4b..52b695ea1 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -27,8 +27,11 @@ itself, prove that the current URL is the response to the OIDC transaction that created remembered return-path storage. LineageWeave sends `state` on the Authorization Code request; OAuth 2.0 and OpenID Connect require that value to be returned on both success and error responses when it was present in the -request. Remembered-path precedence therefore needs both the returned `state` -and a response signal. `state` alone is likewise insufficient. +request. A valid Authorization Code response also has a primary outcome member: +`code` for success or `error` for failure. `session_state`, `iss`, +`error_description`, and `error_uri` are ancillary metadata and cannot establish +a success or error response by themselves. Remembered-path precedence therefore +needs returned `state` plus `code` or `error`; either side alone is insufficient. A lexical leading-slash check is not sufficient to prove that a candidate is a same-origin path. WHATWG URL parsing treats backslashes as authority separators @@ -59,13 +62,15 @@ local deep link rather than rejecting the invalid admission. - When retrying while the browser is still on an OIDC success/error callback, prefer the validated path remembered before redirect over the callback's sanitized redirect-URI path only when `state` is present together with a - response signal (`code`, an OAuth `error*` field, `session_state`, or `iss`). - `state` alone and response-looking query names without `state` are still - scrubbed as reserved protocol data, but neither condition authorizes stale - browser storage to override otherwise current product navigation. This is - consistent with RFC 6749 §§4.1.2 and 4.1.2.1 and OpenID Connect Core 1.0, - which require the authorization response to return `state` when the request - supplied it. + primary Authorization Code response member: `code` for success or `error` + for failure. `state` alone, `code`/`error` without `state`, and ancillary + metadata (`session_state`, `iss`, `error_description`, `error_uri`) without a + primary outcome are still scrubbed as reserved protocol data, but none of + those incomplete shapes authorizes stale browser storage to override current + product navigation. This follows RFC 6749 §§4.1.2 and 4.1.2.1: success + responses carry `code`, error responses carry `error`, and either response + returns `state` when the request supplied it. OpenID Connect metadata does + not replace that primary OAuth response member. - Ordinary product navigation without correlated callback evidence continues to derive its return path from the current location. - On callback, remove the key from both stores and use session storage before @@ -79,15 +84,15 @@ local deep link rather than rejecting the invalid admission. Opening a shared post link survives a missing OIDC state payload or a changed storage context without losing the post. A failed provider callback no longer replaces the pre-redirect deep link with `/` merely because the callback was -rooted at the redirect URI, while an unrelated lone `state`, `code`, `error`, -or other response-shaped query cannot make stale return-path storage win over -current product navigation without the correlated `state` + response-signal -shape. Successful and failed authorization response fields are not minted into -a later product return path, including when an older stored/state value is -recovered. Inputs that only look path-relative before parsing but resolve to -another origin are rejected instead of being host-stripped into a different -local path. A stale internal return path is removed at callback, and -authorization still comes only from the authenticated OIDC token and backend +rooted at the redirect URI, while incomplete response-shaped query combinations +cannot make stale return-path storage win over current product navigation. A +remembered path is preferred only for the correlated `state + code` or +`state + error` shapes. Successful and failed authorization response fields are +not minted into a later product return path, including when an older +stored/state value is recovered. Inputs that only look path-relative before +parsing but resolve to another origin are rejected instead of being host-stripped +into a different local path. A stale internal return path is removed at callback, +and authorization still comes only from the authenticated OIDC token and backend ABAC checks. ## References From bc782925149274f81c4c4bdda46b75d0fa3ef86d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 21:48:43 +0900 Subject: [PATCH 23/26] test(auth): reject unsafe current-path fallback --- frontend/src/oidcReturnUrl.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frontend/src/oidcReturnUrl.test.ts b/frontend/src/oidcReturnUrl.test.ts index 765bc07fc..f2b35b222 100644 --- a/frontend/src/oidcReturnUrl.test.ts +++ b/frontend/src/oidcReturnUrl.test.ts @@ -27,6 +27,18 @@ describe("OIDC return URL handling", () => { expect(restoreOidcReturnUrl({ returnUrl: "/\\evil.example/forged?post=attacker" })).toBe("/"); }); + it("sanitizes the current-path fallback before returning it to the History API", () => { + const originalUrl = window.location.href; + try { + window.history.replaceState({}, "", `${window.location.origin}//evil.example/callback`); + expect(window.location.pathname).toBe("//evil.example/callback"); + + expect(restoreOidcReturnUrl(undefined)).toBe("/"); + } finally { + window.history.replaceState({}, "", originalUrl); + } + }); + it("strips OIDC callback params from a restored post-redirect location", () => { // After Keycloak redirects back, window.location still carries the // one-time code/state; a return URL built from it must not. From 42614d44b69a558716741b79de06918a08f4d790 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 21:49:02 +0900 Subject: [PATCH 24/26] fix(auth): sanitize final OIDC return fallback --- frontend/src/oidcReturnUrl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/oidcReturnUrl.ts b/frontend/src/oidcReturnUrl.ts index 8291829ac..4ef6e4b52 100644 --- a/frontend/src/oidcReturnUrl.ts +++ b/frontend/src/oidcReturnUrl.ts @@ -151,5 +151,5 @@ export function restoreOidcReturnUrl(state: unknown): string { if (fromLocal) return fromLocal; return new URLSearchParams(window.location.search).has("post") ? returnUrlFromLocation() - : window.location.pathname; + : sanitizeReturnUrl(window.location.pathname) || "/"; } From a5281e85fb5765c9b0d1ccf3831233eb2627a9cb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 21:49:26 +0900 Subject: [PATCH 25/26] docs(adr): cover final OIDC history fallback --- .../adr/0109-oidc-deep-link-state-recovery.md | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/docs/adr/0109-oidc-deep-link-state-recovery.md b/docs/adr/0109-oidc-deep-link-state-recovery.md index 52b695ea1..7c1dec328 100644 --- a/docs/adr/0109-oidc-deep-link-state-recovery.md +++ b/docs/adr/0109-oidc-deep-link-state-recovery.md @@ -40,6 +40,13 @@ with a single slash yet parse to a different origin. Reconstructing only the parsed pathname would then silently turn an external-shaped value into a new local deep link rather than rejecting the invalid admission. +The same rule must also cover the final callback fallback. A same-origin document +can itself have a pathname beginning with `//`; returning that raw pathname to +`history.replaceState` makes it a protocol-relative URL candidate even though +it came from the current document. Bypassing the shared sanitizer at that last +fallback can therefore turn a harmless current path into a `SecurityError` +during callback cleanup and leave sign-in completion broken. + ## Decision - Keep the OIDC `state.returnUrl` as the first recovery source. @@ -72,7 +79,11 @@ local deep link rather than rejecting the invalid admission. returns `state` when the request supplied it. OpenID Connect metadata does not replace that primary OAuth response member. - Ordinary product navigation without correlated callback evidence continues - to derive its return path from the current location. + to derive its return path from the current location. Every value returned to + the History API, including the no-state/no-storage current-path fallback, + must pass the same bounded same-origin sanitizer; a protocol-relative or + otherwise inadmissible current pathname falls back to `/` rather than being + returned raw. - On callback, remove the key from both stores and use session storage before local storage. Reject external and protocol-relative URLs. - Keep member language preference account-scoped in @@ -91,9 +102,11 @@ remembered path is preferred only for the correlated `state + code` or not minted into a later product return path, including when an older stored/state value is recovered. Inputs that only look path-relative before parsing but resolve to another origin are rejected instead of being host-stripped -into a different local path. A stale internal return path is removed at callback, -and authorization still comes only from the authenticated OIDC token and backend -ABAC checks. +into a different local path. The final current-path fallback is subject to the +same admission rule, so a `//...` pathname cannot escape as a protocol-relative +History API target and break callback cleanup. A stale internal return path is +removed at callback, and authorization still comes only from the authenticated +OIDC token and backend ABAC checks. ## References From 5e734e3040a6f1a61566074cc3e5122a2acfef3e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 06:13:30 +0900 Subject: [PATCH 26/26] fix(auth): contain sign-in recovery at narrow widths Signed-off-by: Seongho Bae --- frontend/e2e/smoke.spec.ts | 5 +++++ frontend/src/App.css | 1 + 2 files changed, 6 insertions(+) diff --git a/frontend/e2e/smoke.spec.ts b/frontend/e2e/smoke.spec.ts index 295654265..269ac2a1b 100644 --- a/frontend/e2e/smoke.spec.ts +++ b/frontend/e2e/smoke.spec.ts @@ -22,6 +22,11 @@ test("rejects a callback with unrecognized state without exposing provider detai ); await expect(page.getByRole("button", { name: "Start sign-in again" })).toBeVisible(); await expect(page.locator("body")).not.toContainText(/invalid_grant|correlation|state mismatch/i); + expect( + await page.evaluate( + "document.documentElement.scrollWidth <= document.documentElement.clientWidth", + ), + ).toBe(true); await page.screenshot({ path: testInfo.outputPath(`rejected-callback-${testInfo.project.name}.png`), fullPage: true, diff --git a/frontend/src/App.css b/frontend/src/App.css index 3e4c13599..cb15f6a6b 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -22,6 +22,7 @@ } .login-card { + box-sizing: border-box; width: 100%; max-width: 420px; padding: 2.5rem 2rem;