From 80c151d0846512c52eaaf78dffa4ad03d77b7cd4 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 26 Oct 2026 07:52:06 +0900 Subject: [PATCH] fix: fence recovered passkey results against cross-tab changes --- .../workspace/auth/browser-session.svelte.ts | 27 +++- .../auth/passkey-results.browser.spec.ts | 150 ++++++++++++++++++ web/workspace/src/routes/account/+page.svelte | 16 +- .../routes/account/account.browser.spec.ts | 137 ++++++++++++++++ 4 files changed, 318 insertions(+), 12 deletions(-) diff --git a/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts b/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts index 713d8d162..e11a9a980 100644 --- a/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts +++ b/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts @@ -252,12 +252,30 @@ export class BrowserAuth { this.busy = true; this.error = ""; this.status = "Checking the saved addition result…"; + // 保存の履歴は失敗に戻さないが、過去の照合で再追加を許可しない。 + this.additionSaved = false; const generation = browserSession.authenticationGeneration; + const stamp = browserSession.authenticationStamp; const controller = new AbortController(); this.controller = controller; this.operationGeneration = generation; + const assertCurrent = () => { + controller.signal.throwIfAborted(); + if ( + this.disposed || + generation !== browserSession.authenticationGeneration || + stamp !== browserSession.authenticationStamp + ) { + throw new AuthOperationError( + "confirmation", + "session-changed", + "unknown", + ); + } + }; try { const before = await loadWhoami(controller.signal); + assertCurrent(); if ( !before.actor || before.actor.user_id !== attempt.expected_user_id || before.actor.account_id !== attempt.expected_account_id @@ -269,13 +287,10 @@ export class BrowserAuth { ); } const result = await readAdditionResult(attempt, controller.signal); + assertCurrent(); const after = await loadWhoami(controller.signal); - controller.signal.throwIfAborted(); - if ( - this.disposed || - generation !== browserSession.authenticationGeneration || - !sameSession(before, after) - ) { + assertCurrent(); + if (!sameSession(before, after)) { throw new AuthOperationError( "confirmation", "session-changed", diff --git a/web/workspace/src/lib/workspace/auth/passkey-results.browser.spec.ts b/web/workspace/src/lib/workspace/auth/passkey-results.browser.spec.ts index 6ed262f97..bf4f7f469 100644 --- a/web/workspace/src/lib/workspace/auth/passkey-results.browser.spec.ts +++ b/web/workspace/src/lib/workspace/auth/passkey-results.browser.spec.ts @@ -70,6 +70,156 @@ function harness() { owners.push(auth); return { state, create, get, fetcher, auth }; } +function savedAddition() { + return Response.json({ + status: "saved", + user: authUser, + key: { + key_reference: "safe-opaque-reference", + created_at: "2026-01-01T00:00:00Z", + last_used_at: null, + normal_login_verified_at: null, + }, + }); +} +async function recoveryHarness() { + const context = harness(); + await context.auth.refresh(); + context.auth.additionAttempt = { + challenge_id: "synthetic-addition", + expected_user_id: authUser.user_id, + expected_account_id: authUser.account_id, + expected_authentication_reference: "original-operation-reference", + }; + return context; +} +for (const phase of ["initial-whoami", "receipt", "final-whoami"] as const) { + for (const reason of ["logout", "authentication"] as const) { + test(`recovery ${phase} rejects a shared ${reason} stamp before event delivery`, async () => { + const { auth, fetcher } = await recoveryHarness(); + const started = gate(), response = gate(); + let whoami = 0, receiptReads = 0; + fetcher.mockImplementation(async (input, init) => { + const path = String(input); + if (path === "/api/auth/whoami") { + ++whoami; + if (phase === (whoami === 1 ? "initial-whoami" : "final-whoami")) { + started.resolve(); + return response.promise; + } + return identity(authUser); + } + if (path.endsWith("/addition/result")) { + ++receiptReads; + expect( + JSON.parse(String(init?.body)).expected_authentication_reference, + ) + .toBe("original-operation-reference"); + if (phase === "receipt") { + started.resolve(); + return response.promise; + } + return savedAddition(); + } + throw new Error("Unexpected recovery endpoint"); + }); + const generation = browserSession.authenticationGeneration; + const stamp = browserSession.authenticationStamp; + const checking = auth.checkAddition(); + await started.promise; + await auth.checkAddition(); // busy recovery cannot issue a second read + const changed = JSON.stringify({ + id: `recovery-${phase}-${reason}`, + reason, + }); + localStorage.setItem("yoi.browser-auth-generation", changed); + expect(browserSession.authenticationStamp).not.toBe(stamp); + expect(browserSession.authenticationGeneration).toBe(generation); + response.resolve( + phase === "receipt" ? savedAddition() : identity(authUser), + ); + await checking; + expect(auth.additionSaved).toBe(false); + expect(auth.additionAttempt?.expected_authentication_reference) + .toBe("original-operation-reference"); + expect(auth.status).not.toContain("current session was checked"); + expect(auth.error).toContain("does not prove failure"); + expect(receiptReads).toBe(phase === "initial-whoami" ? 0 : 1); + expect(whoami).toBe(phase === "final-whoami" ? 2 : 1); + expect(localStorage.getItem("yoi.browser-auth-generation")).toBe(changed); + expect( + fetcher.mock.calls.some(([path]) => String(path).endsWith("/complete")), + ) + .toBe(false); + }); + } +} +for (const phase of ["receipt", "final-whoami"] as const) { + for (const invalidation of ["cancel", "dispose", "generation"] as const) { + test(`recovery ${phase} ignores a delayed saved response after ${invalidation}`, async () => { + const { auth, fetcher } = await recoveryHarness(); + const started = gate(), response = gate(); + let whoami = 0; + fetcher.mockImplementation(async (input) => { + const path = String(input); + if (path === "/api/auth/whoami") { + if (++whoami === 2 && phase === "final-whoami") { + started.resolve(); + return response.promise; + } + return identity(authUser); + } + if (path.endsWith("/addition/result")) { + if (phase === "receipt") { + started.resolve(); + return response.promise; + } + return savedAddition(); + } + throw new Error("Unexpected recovery endpoint"); + }); + const checking = auth.checkAddition(); + await started.promise; + if (invalidation === "cancel") auth.cancel(); + else if (invalidation === "dispose") auth.dispose(); + else sessionMutation(); + response.resolve( + phase === "receipt" ? savedAddition() : identity(authUser), + ); + await checking; + expect(auth.additionSaved).toBe(false); + expect(auth.additionAttempt).not.toBeNull(); + expect(auth.status).not.toContain("current session was checked"); + expect(whoami).toBe(phase === "receipt" ? 1 : 2); + expect( + fetcher.mock.calls.some(([path]) => String(path).endsWith("/complete")), + ) + .toBe(false); + }); + } +} +test("failed recheck cannot retain repeat-add permission from an older saved confirmation", async () => { + const { auth, fetcher } = await recoveryHarness(); + auth.additionSaved = true; + fetcher.mockResolvedValue(identity(null)); + await auth.checkAddition(); + expect(auth.additionSaved).toBe(false); + expect(auth.additionAttempt).not.toBeNull(); + expect(auth.error).toContain("does not prove failure"); +}); +test("unknown receipt retains the unresolved attempt and never permits blind repetition", async () => { + const { auth, fetcher } = await recoveryHarness(); + fetcher.mockImplementation(async (input) => + String(input).endsWith("/addition/result") + ? Response.json({ status: "unknown" }) + : identity(authUser) + ); + await auth.checkAddition(); + expect(auth.additionSaved).toBe(false); + expect(auth.additionAttempt).not.toBeNull(); + expect(auth.status).toContain("still unknown"); + expect(auth.error).toBe(""); +}); for (const outcome of ["anonymous", "other-actor", "unavailable"] as const) { test(`addition response is not Added when confirming whoami is ${outcome}`, async () => { const { auth, state, fetcher } = harness(); diff --git a/web/workspace/src/routes/account/+page.svelte b/web/workspace/src/routes/account/+page.svelte index 99f582066..2a54e0948 100644 --- a/web/workspace/src/routes/account/+page.svelte +++ b/web/workspace/src/routes/account/+page.svelte @@ -4,6 +4,7 @@ import type { PasskeyListResponse } from '#lib/generated/auth-api.ts'; import { addPasskey, logout, loadPasskeys, loadWhoami } from '#lib/workspace/auth/api.ts'; import { BrowserAuth } from '#lib/workspace/auth/browser-session.svelte.ts'; + import { browserSession } from '#lib/workspace/auth/session-state.ts'; import { loginUrl } from '#lib/workspace/auth/routes.ts'; import '#lib/workspace/auth/auth-page.css'; @@ -22,18 +23,21 @@ const controller = new AbortController(); keyController = controller; const read = ++keyRead; + const generation = browserSession.authenticationGeneration; + const stamp = browserSession.authenticationStamp; + const assertCurrent = () => { + controller.signal.throwIfAborted(); + if (!active || read !== keyRead || generation !== browserSession.authenticationGeneration || stamp !== browserSession.authenticationStamp) throw new Error('Session changed'); + }; keysLoading = true; keysError = ''; try { const before = await loadWhoami(controller.signal); - controller.signal.throwIfAborted(); - if (!active || read !== keyRead) return; + assertCurrent(); if (before.actor?.user_id !== actor.user_id || before.actor.account_id !== actor.account_id) throw new Error('Account changed'); const result = await loadPasskeys(controller.signal); - controller.signal.throwIfAborted(); - if (!active || read !== keyRead) return; + assertCurrent(); const after = await loadWhoami(controller.signal); - controller.signal.throwIfAborted(); - if (read !== keyRead) return; + assertCurrent(); if (result.user.user_id !== actor.user_id || result.user.account_id !== actor.account_id || after.actor?.user_id !== actor.user_id || after.actor.account_id !== actor.account_id || before.authentication_reference !== after.authentication_reference) throw new Error('Session changed'); diff --git a/web/workspace/src/routes/account/account.browser.spec.ts b/web/workspace/src/routes/account/account.browser.spec.ts index c79e378e0..6ac7d506d 100644 --- a/web/workspace/src/routes/account/account.browser.spec.ts +++ b/web/workspace/src/routes/account/account.browser.spec.ts @@ -158,6 +158,143 @@ async function submit(name: string) { await fireEvent.submit(screen.getByRole("button", { name }).closest("form")!); } +for (const phase of ["initial-whoami", "list", "final-whoami"] as const) { + for (const reason of ["logout", "authentication"] as const) { + test(`registered-key ${phase} confirmation rejects shared ${reason} before event delivery`, async () => { + const { fetcher } = boundary({ authenticated: true }); + render(Account); + await screen.findByText( + "No registered keys were returned for this account.", + ); + const request = fetcher.getMockImplementation()!; + const started = gate(), response = gate(); + let whoami = 0, listReads = 0; + const metadata = () => + Response.json({ + user: authUser, + keys: [{ + key_reference: "delayed-key-metadata", + created_at: "2026-01-01T00:00:00Z", + last_used_at: null, + normal_login_verified_at: null, + }], + }); + fetcher.mockImplementation(async (input, init) => { + const path = String(input); + if (path === "/api/auth/whoami") { + ++whoami; + if (phase === (whoami === 1 ? "initial-whoami" : "final-whoami")) { + started.resolve(); + return response.promise; + } + } + if (path === "/api/auth/passkeys") { + ++listReads; + if (phase === "list") { + started.resolve(); + return response.promise; + } + return metadata(); + } + return request(input, init); + }); + const generation = browserSession.authenticationGeneration; + const stamp = browserSession.authenticationStamp; + await fireEvent.click( + screen.getByRole("button", { name: "Refresh registered keys" }), + ); + await started.promise; + const changed = JSON.stringify({ id: `keys-${phase}-${reason}`, reason }); + localStorage.setItem("yoi.browser-auth-generation", changed); + expect(browserSession.authenticationStamp).not.toBe(stamp); + expect(browserSession.authenticationGeneration).toBe(generation); + response.resolve( + phase === "list" ? metadata() : await request("/api/auth/whoami"), + ); + await screen.findByText(/Registered keys could not be confirmed/); + expect(screen.queryByText("delayed-key-metadata")).toBeNull(); + expect(listReads).toBe(phase === "initial-whoami" ? 0 : 1); + expect(whoami).toBe(phase === "final-whoami" ? 2 : 1); + expect(localStorage.getItem("yoi.browser-auth-generation")).toBe(changed); + expect(posted(fetcher)).toEqual([]); + }); + } +} + +for (const reason of ["logout", "authentication"] as const) { + test(`stale receipt confirmation after shared ${reason} leaves visible repeat-add disabled`, async () => { + const { fetcher } = boundary({ authenticated: true }); + render(Account); + await screen.findByText( + "No registered keys were returned for this account.", + ); + const request = fetcher.getMockImplementation()!; + const started = gate(), response = gate(); + let receiptRead = false; + fetcher.mockImplementation(async (input, init) => { + const path = String(input); + if (path.endsWith("/addition/complete")) { + throw new TypeError("lost response"); + } + if (path.endsWith("/addition/result")) { + receiptRead = true; + expect(JSON.parse(String(init?.body)).expected_authentication_reference) + .toBe("fixture-authentication"); + return Response.json({ + status: "saved", + user: authUser, + key: { + key_reference: "saved-but-session-unconfirmed", + created_at: "2026-01-01T00:00:00Z", + last_used_at: null, + normal_login_verified_at: null, + }, + }); + } + if (path === "/api/auth/whoami" && receiptRead) { + receiptRead = false; + started.resolve(); + return response.promise; + } + return request(input, init); + }); + const add = screen.getByRole("button", { + name: /Add passkey to/, + }) as HTMLButtonElement; + await fireEvent.click(add); + await screen.findByText(/result is unknown/); + await waitFor(() => + expect(screen.queryByText("Checking registered keys…")).toBeNull() + ); + expect(add.disabled).toBe(true); + const checking = screen.getByRole("button", { + name: "Check addition result", + }); + await waitFor(() => + expect((checking as HTMLButtonElement).disabled).toBe(false) + ); + await fireEvent.click(checking); + await started.promise; + const generation = browserSession.authenticationGeneration; + localStorage.setItem( + "yoi.browser-auth-generation", + JSON.stringify({ id: `visible-${reason}`, reason }), + ); + expect(browserSession.authenticationGeneration).toBe(generation); + response.resolve(await request("/api/auth/whoami")); + await screen.findByText(/The addition result could not be read/); + await waitFor(() => + expect((checking as HTMLButtonElement).disabled).toBe(false) + ); + expect(screen.queryByText(/Your current session was checked/)).toBeNull(); + expect(add.disabled).toBe(true); + await fireEvent.click(add); + expect( + posted(fetcher).filter(({ path }) => path.endsWith("/addition/complete")), + ).toHaveLength(1); + }); +} + test("normal login needs no account ID and links separate registration and code activation", async () => { const { fetcher } = boundary({ policy: true }); page.url = new URL("https://example.test/login?returnTo=%2Faccount");