From 825db93ddf441d9c1791ae45016d85e98360e856 Mon Sep 17 00:00:00 2001 From: Navid Shad Date: Mon, 4 May 2026 14:58:23 +0300 Subject: [PATCH] fix: persist anonymous tokens and stop logout-cascade on anon sessions Two-part fix for the user-reported "translate fails after language change" bug. The chrome.storage.sync token store was empty for anonymous users (loginAsAnonymous never persisted), and loginWithLastSession treated a valid anon session as "login failed" because updateIsLogin returns false unless user.type === "user". Every fresh popup open therefore broadcast StoreUserTokenMessage(null) and 412'd the next translate from any tab. Also guard onDocClick against non-Element event targets, which surfaced as "target.closest is not a function" in the same Wikipedia console run. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/nibble/composables/useTextSelection.ts | 12 +- src/plugins/modular-rest.ts | 41 ++- tests/auth-anon-flow.test.ts | 242 ++++++++++++++++++ .../e2e/auth-survives-language-switch.spec.ts | 186 ++++++++++++++ 4 files changed, 470 insertions(+), 11 deletions(-) create mode 100644 tests/auth-anon-flow.test.ts create mode 100644 tests/e2e/auth-survives-language-switch.spec.ts diff --git a/src/nibble/composables/useTextSelection.ts b/src/nibble/composables/useTextSelection.ts index a46fad5..31d57c7 100644 --- a/src/nibble/composables/useTextSelection.ts +++ b/src/nibble/composables/useTextSelection.ts @@ -152,9 +152,15 @@ export function useTextSelection() { } function onDocClick(e: MouseEvent) { - const target = e.target as Element | null; - if (!target) return; - if (target.closest(`#${NIBBLE_ROOT_ID}`)) return; + const target = e.target; + const el = + target instanceof Element + ? target + : target instanceof Node + ? target.parentElement + : null; + if (!el) return; + if (el.closest(`#${NIBBLE_ROOT_ID}`)) return; if (window.getSelection()?.isCollapsed !== false) return; } diff --git a/src/plugins/modular-rest.ts b/src/plugins/modular-rest.ts index 9157eb4..299acf7 100644 --- a/src/plugins/modular-rest.ts +++ b/src/plugins/modular-rest.ts @@ -76,27 +76,52 @@ export async function loginWithLastSession() { return user; }) .then((_user) => updateIsLogin()) - .then(async (isSuccess) => { - - // if the login failed, it means token is invalid or expired. - // so the token should be removed from the storage. - if (!isSuccess) { + .then(async (_isRegisteredUser) => { + // updateIsLogin's truthy result means "registered user with a real + // account". Anonymous users return false here even though they hold a + // perfectly valid session — so don't conflate "not a registered user" + // with "login failed". Only broadcast logout when the underlying token + // truly couldn't be validated (authentication.isLogin === false). + // Without this guard, every fresh popup open would `logout()` the anon + // session, clearing chrome.storage.sync and broadcasting null to every + // tab — and the next translate from any content script then 412s + // because its Authorization header is empty. + if (!authentication.isLogin) { await logout(); return false; } - - return isSuccess; + return true; }) .finally(() => { if (!authentication.isLogin) { authentication .loginAsAnonymous() - .then((user) => { + .then(async () => { console.log( "Subturtle Anonymous login succeded", authentication.isLogin ); + // Persist the anonymous token so subsequent mounts (other bundles + // on the same page, the popup, page reloads) reuse it instead of + // each calling /user/loginAnonymous and stranding the previous + // anonymous user — which the server then 412s on the next call. + // Writes to chrome.storage.sync (cross-context) and to this + // page's localStorage (modular-rest's own per-origin cache). + const token = authentication.getToken; + if (token) { + try { + await sendMessage(new StoreUserTokenMessage(token)); + } catch (err) { + console.warn( + "Subturtle: persisting anonymous token to background failed", + err + ); + } + if (typeof localStorage !== "undefined") { + localStorage.setItem("token", token); + } + } updateIsLogin(); }) .catch((err) => { diff --git a/tests/auth-anon-flow.test.ts b/tests/auth-anon-flow.test.ts new file mode 100644 index 0000000..92e4815 --- /dev/null +++ b/tests/auth-anon-flow.test.ts @@ -0,0 +1,242 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { setActivePinia, createPinia } from "pinia"; +import { + MESSAGE_TYPE, + StoreUserTokenMessage, + type LoginStatusResponse, +} from "../src/common/types/messaging"; + +// Controllable @modular-rest/client mock. The plugin module under test reads +// `authentication.{isLogin,user,getToken}` and calls +// `loginWithToken|loginAsAnonymous|logout`. We expose hooks so each test can +// shape the auth state before exercising loginWithLastSession. +const auth = { + isLogin: false, + user: null as null | { id: string; type: string; email?: string }, + getToken: null as string | null, + loginWithToken: vi.fn(), + loginAsAnonymous: vi.fn(), + logout: vi.fn(() => { + auth.isLogin = false; + auth.user = null; + auth.getToken = null; + }), +}; + +vi.mock("@modular-rest/client", () => ({ + GlobalOptions: { set: vi.fn() }, + authentication: auth, + dataProvider: {}, + fileProvider: {}, + functionProvider: { run: vi.fn() }, +})); + +// useProfileStore is only invoked inside updateIsLogin's registered-user +// branch and inside logout(); the anon flow doesn't hit those, but logout() is +// still called when the token truly fails to validate. Keep it as a no-op so +// it doesn't pull in the sibling dashboard-app type imports at module load. +vi.mock("../src/stores/profile", () => ({ + useProfileStore: () => ({ + logout: vi.fn(), + bootstrap: vi.fn().mockResolvedValue(undefined), + }), +})); + +// Mixpanel is wired everywhere via the analytic singleton; in tests we don't +// want network or to require dotenv-injected env vars. +vi.mock("../src/plugins/mixpanel", () => ({ + analytic: { + identify: vi.fn(), + track: vi.fn(), + register: vi.fn(), + reset: vi.fn(), + people: { set: vi.fn() }, + }, +})); + +// Capture chrome.runtime.sendMessage so we can assert what crosses to the +// background. The setup.ts shim makes it a vi.fn() that resolves with {}. +function getSendMessageMock() { + return (globalThis as any).chrome.runtime.sendMessage as ReturnType< + typeof vi.fn + >; +} + +// Make chrome.runtime.sendMessage shape its response based on which message +// type was passed. GetLoginStatusMessage callers expect {status, token}, +// everyone else can get the default {} from the setup shim. +function stubBackgroundLoginStatus(token: string | null) { + const sendMessage = getSendMessageMock(); + sendMessage.mockImplementation( + (message: any, callback?: (response: any) => void) => { + if (message?.type === MESSAGE_TYPE.GET_LOGIN_STATUS) { + const response: LoginStatusResponse = { + status: !!token, + ...(token ? { token } : {}), + }; + callback?.(response); + return Promise.resolve(response); + } + callback?.({}); + return Promise.resolve({}); + } + ); +} + +describe("loginWithLastSession (anonymous flow)", () => { + let loginWithLastSession: typeof import("../src/plugins/modular-rest").loginWithLastSession; + + beforeEach(async () => { + setActivePinia(createPinia()); + + // Reset auth state. + auth.isLogin = false; + auth.user = null; + auth.getToken = null; + auth.loginWithToken.mockReset(); + auth.loginAsAnonymous.mockReset(); + auth.logout.mockReset(); + auth.logout.mockImplementation(() => { + auth.isLogin = false; + auth.user = null; + auth.getToken = null; + }); + + // Reset the chrome shim default. + getSendMessageMock().mockReset(); + stubBackgroundLoginStatus(null); + + // Reset localStorage between tests (happy-dom gives us a real one). + localStorage.clear(); + + // Re-import the plugin fresh each test so the chrome.runtime.onMessage + // listener doesn't accumulate. + vi.resetModules(); + const mod = await import("../src/plugins/modular-rest"); + loginWithLastSession = mod.loginWithLastSession; + + // Suppress noisy console output from the plugin's anon-login console.log + // and bootstrap error path. + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + it("falls through to anonymous login when no token is stored", async () => { + auth.loginAsAnonymous.mockImplementation(async () => { + auth.isLogin = true; + auth.user = { id: "anon-1", type: "anonymous" }; + auth.getToken = "anon-token-abc"; + return { token: "anon-token-abc" }; + }); + + await loginWithLastSession(); + // .finally fires the anon login asynchronously; let microtasks settle. + await new Promise((r) => setTimeout(r, 0)); + + expect(auth.loginAsAnonymous).toHaveBeenCalledTimes(1); + }); + + it("persists the new anonymous token to chrome.storage.sync and localStorage", async () => { + auth.loginAsAnonymous.mockImplementation(async () => { + auth.isLogin = true; + auth.user = { id: "anon-1", type: "anonymous" }; + auth.getToken = "anon-token-abc"; + return { token: "anon-token-abc" }; + }); + + await loginWithLastSession(); + await new Promise((r) => setTimeout(r, 0)); + + // The "no token" path actually emits two StoreUserTokenMessages: the + // wrapper logout() that runs because authentication.isLogin was still + // false sends StoreUserTokenMessage(null) first, then the anon-fallback + // .then in the finally writes the fresh anon token. The end state is + // what matters — the LAST write must be the new anon token, so the + // background's chrome.storage.sync ends up populated. + const sendMessage = getSendMessageMock(); + const storeCalls = sendMessage.mock.calls.filter( + ([m]) => + m && (m as any).type === MESSAGE_TYPE.STORE_USER_TOKEN + ); + expect(storeCalls.length).toBeGreaterThanOrEqual(1); + const lastStore = storeCalls[storeCalls.length - 1][0] as StoreUserTokenMessage; + expect(lastStore.token).toBe("anon-token-abc"); + + // localStorage cache for the page itself, mirroring what + // @modular-rest/client's authentication.saveSession() would do. + expect(localStorage.getItem("token")).toBe("anon-token-abc"); + }); + + it("does NOT broadcast logout when the token validates as an anonymous user", async () => { + // Background returns a stored anon token (the success path the user hits + // every fresh popup open). + stubBackgroundLoginStatus("anon-token-abc"); + auth.loginWithToken.mockImplementation(async (token: string) => { + auth.isLogin = true; + auth.user = { id: "anon-1", type: "anonymous" }; + auth.getToken = token; + return auth.user; + }); + + await loginWithLastSession(); + await new Promise((r) => setTimeout(r, 0)); + + // The wrapper logout() would broadcast StoreUserTokenMessage(null) and + // call authentication.logout(). Neither must happen for an anon session + // — that's the cascade that wiped chrome.storage.sync and 412'd every + // subsequent translate before the fix. + expect(auth.logout).not.toHaveBeenCalled(); + const sendMessage = getSendMessageMock(); + const nullStoreCalls = sendMessage.mock.calls.filter( + ([m]) => + m && + (m as any).type === MESSAGE_TYPE.STORE_USER_TOKEN && + (m as any).token === null + ); + expect(nullStoreCalls).toHaveLength(0); + + // And we should NOT have re-rolled an anon login when validation worked. + expect(auth.loginAsAnonymous).not.toHaveBeenCalled(); + }); + + it("falls through to a fresh anon login when a stored token is rejected by the server", async () => { + stubBackgroundLoginStatus("stale-token"); + auth.loginWithToken.mockImplementation(async () => { + // modular-rest's internal loginWithToken catch path calls + // authentication.logout() before rethrowing. Mirror that. + auth.logout(); + throw new Error("token rejected"); + }); + auth.loginAsAnonymous.mockImplementation(async () => { + auth.isLogin = true; + auth.user = { id: "anon-2", type: "anonymous" }; + auth.getToken = "fresh-anon"; + return { token: "fresh-anon" }; + }); + + // The plugin's promise chain doesn't catch loginWithToken rejections, so + // the rejection propagates out of loginWithLastSession. The .finally + // anon-fallback still runs first. Swallow here so the test asserts on + // observable side-effects rather than the throw itself. + await loginWithLastSession().catch(() => undefined); + await new Promise((r) => setTimeout(r, 0)); + + // modular-rest's internal logout fired (mocked above before throwing). + expect(auth.logout).toHaveBeenCalled(); + + // And we fell through to a fresh anon login that overwrites the stale + // token in chrome.storage.sync with the new one — recovery without the + // user having to do anything. + expect(auth.loginAsAnonymous).toHaveBeenCalledTimes(1); + + const sendMessage = getSendMessageMock(); + const storeCalls = sendMessage.mock.calls.filter( + ([m]) => + m && (m as any).type === MESSAGE_TYPE.STORE_USER_TOKEN + ); + const lastStore = storeCalls[storeCalls.length - 1]?.[0] as + | StoreUserTokenMessage + | undefined; + expect(lastStore?.token).toBe("fresh-anon"); + }); +}); diff --git a/tests/e2e/auth-survives-language-switch.spec.ts b/tests/e2e/auth-survives-language-switch.spec.ts new file mode 100644 index 0000000..92b266b --- /dev/null +++ b/tests/e2e/auth-survives-language-switch.spec.ts @@ -0,0 +1,186 @@ +import { test, expect } from "./extension-fixture"; +import type { Route, Page } from "@playwright/test"; + +// Regression for the "translate fails after language change" bug. Before the +// fix, an anonymous Subturtle session lost its token whenever the popup +// re-ran loginWithLastSession (which it does on every fresh popup open): +// 1. authentication.loginAsAnonymous() never persisted its token, so +// chrome.storage.sync was empty even though the user *had* a working +// anon session in memory. +// 2. updateIsLogin() returns false for `user.type !== "user"`, so +// loginWithLastSession's `.then` treated a perfectly good anon login +// as "login failed" and called the wrapper logout(), which broadcast +// StoreUserTokenMessage(null) to every tab — instantly invalidating +// the session that was in flight. +// 3. The next translate had no Authorization header, the modular-rest +// auth middleware returned 412 Precondition Failed, and the user +// saw "Translation failed." +// +// This test pins both halves of the fix: +// * After an anonymous login, chrome.storage.sync is populated and the +// in-page localStorage cache holds the same token. +// * Mutating chrome.storage.local.settings to simulate a popup language +// change does NOT clear chrome.storage.sync.token, and a fresh +// /function/run still goes out with a non-empty Authorization header. + +const ANON_TOKEN = "test-anon-token-abc123"; + +async function stubBackend(page: Page, observed: { authHeaders: string[] }) { + await page.route("**/user/loginAnonymous", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ token: ANON_TOKEN }), + }) + ); + + // /verify/token is hit by authentication.loginWithToken on every mount. + // Always return success so the persisted anon token is treated as valid. + await page.route("**/verify/token", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + user: { + id: "anon-user-1", + type: "anonymous", + email: null, + phone: null, + permissionGroup: "anonymous", + }, + }), + }) + ); + + // Capture the Authorization header on every translate so we can assert + // it remains non-empty across language switches. + await page.route("**/function/run", async (route: Route) => { + const headers = route.request().headers(); + observed.authHeaders.push(headers["authorization"] ?? ""); + const body = route.request().postDataJSON(); + if (body?.name === "translateWithContext") { + const phrase: string = body.args?.phrase ?? ""; + return route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: `[stub:${body.args?.targetLanguage}] ${phrase}` }), + }); + } + // Other modular-rest function calls (subscription, etc.) — succeed empty. + return route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: {} }), + }); + }); + + // Anything else under /user/** that the bundles might hit on mount. + await page.route("**/user/**", (route) => { + if (route.request().url().includes("/user/loginAnonymous")) { + return route.fallback(); + } + return route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: {} }), + }); + }); +} + +test.describe("auth survives language switch (anonymous user)", () => { + test("anon login persists token and language change preserves it", async ({ + context, + serviceWorker, + }) => { + const page = await context.newPage(); + const observed = { authHeaders: [] as string[] }; + await stubBackend(page, observed); + + await page.goto("/index.html"); + await expect(page.locator("#subturtle-nibble-root")).toBeAttached({ + timeout: 10_000, + }); + + // Anon login is fire-and-forget after page load; wait for the wrapper + // to persist the token to chrome.storage.sync (the .finally chain + // sends StoreUserTokenMessage to background). + await expect + .poll( + async () => { + const { token } = (await serviceWorker.evaluate(async () => { + return chrome.storage.sync.get("token"); + })) as { token?: string }; + return token ?? null; + }, + { timeout: 5_000 } + ) + .toBe(ANON_TOKEN); + + // Same token in the page's localStorage (modular-rest's per-origin cache). + expect( + await page.evaluate(() => localStorage.getItem("token")) + ).toBe(ANON_TOKEN); + + // First translate — works, Authorization header carries the anon token. + await page.locator("#test-word").click({ clickCount: 2 }); + await expect(page.locator(".nibble-icon-btn")).toBeVisible({ + timeout: 5_000, + }); + await page.locator(".nibble-icon-btn").click(); + await expect(page.locator(".nibble-translation")).toBeVisible({ + timeout: 5_000, + }); + expect(observed.authHeaders.at(-1)).toBe(ANON_TOKEN); + const beforeSwitchCount = observed.authHeaders.length; + + // Simulate the popup's "user changed target language" broadcast by + // writing settings to chrome.storage.local. Both content scripts' + // settings stores listen on chrome.storage.onChanged and update their + // `language` ref reactively — same path the popup uses end-to-end. + await serviceWorker.evaluate(async () => { + const existing = await chrome.storage.local.get("settings"); + const settings = (existing.settings as any) || {}; + await chrome.storage.local.set({ + settings: { + theme: settings.theme ?? "dark", + language: "es", + nibbleDisabledDomains: settings.nibbleDisabledDomains ?? [], + }, + }); + }); + await page.waitForTimeout(300); + + // chrome.storage.sync.token must NOT be cleared. Before the fix, the + // popup's loginWithLastSession path called the wrapper logout() because + // it confused "anon user" with "login failed", and that broadcast + // StoreUserTokenMessage(null) -> background cleared the token. We + // assert it's still there. + const tokenAfterSwitch = (await serviceWorker.evaluate(async () => { + const { token } = await chrome.storage.sync.get("token"); + return token ?? null; + })) as string | null; + expect(tokenAfterSwitch).toBe(ANON_TOKEN); + + // Second translate after the language change — must succeed, must + // carry the same Authorization header. Before the fix, this was the + // 412 case. + await page.locator("body").click(); // dismiss any leftover icon + await page.locator("#test-word").click({ clickCount: 2 }); + await expect(page.locator(".nibble-icon-btn")).toBeVisible({ + timeout: 5_000, + }); + await page.locator(".nibble-icon-btn").click(); + await expect(page.locator(".nibble-translation")).toBeVisible({ + timeout: 5_000, + }); + expect(observed.authHeaders.length).toBeGreaterThan(beforeSwitchCount); + expect(observed.authHeaders.at(-1)).toBe(ANON_TOKEN); + + // Sanity: the new request was made with the new targetLanguage. The + // stub echoes it into the response body so the rendered translation + // string carries the language tag. + await expect(page.locator(".nibble-translation")).toContainText("[stub:Spanish]"); + + await page.close(); + }); +});