diff --git a/README.md b/README.md index 9537f86221..ce05d0865b 100644 --- a/README.md +++ b/README.md @@ -637,6 +637,13 @@ The bundled Docker, local and Helm nginx configs proxy Live Browser WebSocket up Browser login uses `HttpOnly` session cookies. The login page offers a "keep me signed in" option that extends the browser session when the request is HTTPS (including trusted `X-Forwarded-Proto: https`) or localhost HTTP. The localhost exception uses the direct request `Host` and ignores forwarded host headers. Public HTTP deployments, including many temporary sandbox URLs, fall back to session cookies by default. DeerFlow never stores the password in browser storage; the UI may remember only the email address. +If an authentication refresh fails during a temporary network outage, a visible +tab rechecks the unknown session when connectivity returns or on a later tab +switch. Reconnects received in a background tab are remembered until it becomes +visible, without waiting for the tab-switch throttle. Identity is restored only +after a successful auth response; a confirmed expired session or explicit logout +still requires signing in again. + Password forms accept non-ASCII email addresses and trim surrounding email whitespace. Sign-in requires at least six password characters; creating an account or choosing a new password requires eight. These checks run before sending a request. diff --git a/backend/tests/test_agent_guidance_check.py b/backend/tests/test_agent_guidance_check.py index 464461b1fd..60f9649a93 100644 --- a/backend/tests/test_agent_guidance_check.py +++ b/backend/tests/test_agent_guidance_check.py @@ -55,6 +55,7 @@ EXPECTED_GUIDANCE_PATHS = { "backend/packages/harness/deerflow/utils/AGENTS.md", "frontend/src/AGENTS.md", "frontend/src/components/workspace/browser-view/AGENTS.md", + "frontend/src/core/auth/AGENTS.md", "frontend/src/core/memory/AGENTS.md", "frontend/tests/e2e/AGENTS.md", "scripts/AGENTS.md", diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index bc440ec4a2..51b80b0805 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -106,14 +106,8 @@ Users create **threads** (conversations), send messages, set thread-scoped `/goa ### Source Layout (`src/`) -`AuthProvider` publishes the query owner (user ID, confirmed logout, or unknown -auth). `QueryClientProvider` owns a mount-local client and remounts its consumers -when that owner changes, clearing the old cache and cancelling old queries. -Confirmed logout unmounts query consumers immediately; public and unknown-auth -routes still render with an independent client. Keep auth outside the query -provider. Superseded auth refreshes cannot restore an earlier owner. - -Auth: trim emails; validate raw passwords. +Read [auth ownership and recovery](src/core/auth/AGENTS.md) before changing +`AuthProvider` or `QueryClientProvider`. Auth forms trim emails, not passwords. See [project structure](README.md#project-structure) for the directory map and [src/AGENTS.md](src/AGENTS.md) for component and domain ownership. diff --git a/frontend/src/core/auth/AGENTS.md b/frontend/src/core/auth/AGENTS.md new file mode 100644 index 0000000000..6a66f98be4 --- /dev/null +++ b/frontend/src/core/auth/AGENTS.md @@ -0,0 +1,22 @@ +# Authentication state and recovery + +`AuthProvider` publishes the query owner: user ID, confirmed logout (`null`), or +unknown auth (`undefined`). Keep auth outside `QueryClientProvider`. Its mount-local +client remounts consumers, clears the old cache and cancels queries on owner changes. +Confirmed logout unmounts consumers immediately; public/unknown routes retain an +independent client. Superseded refreshes cannot restore an earlier owner. + +Transport failure leaves unknown auth eligible for the 60-second visible-tab +check. A visible `online` event revalidates unknown auth immediately, but never +authenticates on connectivity alone. Confirmed anonymous/logout and static demos +do not probe. Keep wakeups single-flight and remove listeners on unmount. +Only the current request may release its in-flight guard; confirmed user/logout +supersedes that ownership. Coalesce reconnects during a pending refresh into one +follow-up only if auth remains unknown. Success, 401/logout, account replacement +and unmount discard the deferred wake. Hidden tabs retain reconnect wakeups until +visible, bypassing the tab-switch throttle once. Keep `isLoading` in the effect +dependencies so failed requests that leave auth unchanged still drain pending wakes. + +Regressions: `tests/unit/core/auth/AuthProvider-recovery.dom.test.tsx` and +`tests/e2e/auth-network-recovery.spec.ts` under `frontend/`, alongside query-owner +and artifact-draft isolation suites. Forms trim emails, never raw passwords. diff --git a/frontend/src/core/auth/AuthProvider.tsx b/frontend/src/core/auth/AuthProvider.tsx index 7d35e5be84..31553879d5 100644 --- a/frontend/src/core/auth/AuthProvider.tsx +++ b/frontend/src/core/auth/AuthProvider.tsx @@ -76,6 +76,8 @@ export function AuthProvider({ const [isLoading, setIsLoading] = useState(false); // Logout and externally confirmed users supersede pending auth refreshes. const authRequestRef = React.useRef(0); + const authRefreshInFlightRef = React.useRef(false); + const reconnectPendingRef = React.useRef(false); const router = useRouter(); const pathname = usePathname(); const staticMode = isStaticWebsiteOnly(); @@ -89,6 +91,8 @@ export function AuthProvider({ */ const applyUser = useCallback((next: User | null) => { authRequestRef.current++; + authRefreshInFlightRef.current = false; + reconnectPendingRef.current = false; setUser(next); setAuthKnown(true); setIsLoading(false); @@ -101,6 +105,7 @@ export function AuthProvider({ const refreshUser = useCallback(async () => { if (staticMode) return; const requestId = ++authRequestRef.current; + authRefreshInFlightRef.current = true; try { setIsLoading(true); @@ -139,7 +144,10 @@ export function AuthProvider({ setUser(null); setAuthKnown(false); } finally { - if (requestId === authRequestRef.current) setIsLoading(false); + if (requestId === authRequestRef.current) { + authRefreshInFlightRef.current = false; + setIsLoading(false); + } } }, [staticMode, pathname, router]); @@ -157,6 +165,8 @@ export function AuthProvider({ const logout = useCallback(async () => { // Immediately clear local state to prevent UI flicker authRequestRef.current++; + authRefreshInFlightRef.current = false; + reconnectPendingRef.current = false; setUser(null); setAuthKnown(true); setIsLoading(false); @@ -190,27 +200,54 @@ export function AuthProvider({ }, [staticMode, router]); /** - * Handle visibility change - refresh user when tab becomes visible again. - * Throttled to at most once per 60 s to avoid spamming the backend on rapid tab switches. + * Revalidate visible tabs, including unknown auth after a transport failure. + * Tab switches are throttled to 60 s; reconnect wakes unknown auth immediately. */ const lastCheckRef = React.useRef(0); + useEffect( + () => () => { + reconnectPendingRef.current = false; + }, + [], + ); + useEffect(() => { if (staticMode) return; - const handleVisibilityChange = () => { - if (document.visibilityState !== "visible" || user === null) return; + const checkUser = (reconnecting = false) => { + if (user === null && authKnown) return; + if (reconnecting) reconnectPendingRef.current = true; + if ( + document.visibilityState !== "visible" || + authRefreshInFlightRef.current + ) + return; const now = Date.now(); - if (now - lastCheckRef.current < 60_000) return; + if (!reconnectPendingRef.current && now - lastCheckRef.current < 60_000) + return; + reconnectPendingRef.current = false; lastCheckRef.current = now; void refreshUser(); }; + const handleVisibilityChange = () => checkUser(); + const handleOnline = () => { + if (!authKnown || authRefreshInFlightRef.current) checkUser(true); + }; + + // isLoading re-runs this effect when a failed request leaves auth unchanged. + if (reconnectPendingRef.current && !authRefreshInFlightRef.current) { + if (authKnown) reconnectPendingRef.current = false; + else checkUser(); + } document.addEventListener("visibilitychange", handleVisibilityChange); + window.addEventListener("online", handleOnline); return () => { document.removeEventListener("visibilitychange", handleVisibilityChange); + window.removeEventListener("online", handleOnline); }; - }, [staticMode, user, refreshUser]); + }, [staticMode, user, authKnown, isLoading, refreshUser]); const value: AuthContextType = { user, diff --git a/frontend/tests/e2e/auth-network-recovery.spec.ts b/frontend/tests/e2e/auth-network-recovery.spec.ts new file mode 100644 index 0000000000..2376adbadf --- /dev/null +++ b/frontend/tests/e2e/auth-network-recovery.spec.ts @@ -0,0 +1,76 @@ +import { expect, test } from "@playwright/test"; + +import { mockLangGraphAPI } from "./utils/mock-api"; + +for (const recoveryEvent of [ + "online", + "visibilitychange", + "hidden reconnect", +] as const) { + test(`authentication recovers after a network failure on ${recoveryEvent}`, async ({ + page, + }) => { + mockLangGraphAPI(page); + await page.clock.install(); + let requests = 0; + await page.route("**/api/v1/auth/me", async (route) => { + requests++; + if (requests === 1) { + await route.abort("internetdisconnected"); + return; + } + await route.fulfill({ + json: { + id: "default", + email: "recovered@test.local", + system_role: "admin", + needs_setup: false, + oauth_provider: null, + }, + }); + }); + + await page.goto("/workspace/chats/new?settings=account"); + const dialog = page.getByRole("dialog", { name: "Settings", exact: true }); + await expect(dialog.getByText("default@test.local")).toBeVisible(); + + await page.evaluate(() => + document.dispatchEvent(new Event("visibilitychange")), + ); + await expect.poll(() => requests).toBe(1); + await expect(dialog.getByText("default@test.local")).toBeHidden(); + await expect(page).not.toHaveURL(/\/login/); + + if (recoveryEvent === "hidden reconnect") { + await page.evaluate(() => { + Object.defineProperty(document, "visibilityState", { + configurable: true, + value: "hidden", + }); + window.dispatchEvent(new Event("online")); + }); + expect(requests).toBe(1); + await expect(dialog.getByText("recovered@test.local")).toBeHidden(); + await page.clock.fastForward(30_000); + await page.evaluate(() => { + Object.defineProperty(document, "visibilityState", { + configurable: true, + value: "visible", + }); + document.dispatchEvent(new Event("visibilitychange")); + }); + } else { + if (recoveryEvent === "visibilitychange") { + await page.clock.fastForward(60_001); + } + await page.evaluate((event) => { + if (event === "online") window.dispatchEvent(new Event(event)); + else document.dispatchEvent(new Event(event)); + }, recoveryEvent); + } + + await expect(dialog.getByText("recovered@test.local")).toBeVisible(); + expect(requests).toBe(2); + await expect(page).not.toHaveURL(/\/login/); + }); +} diff --git a/frontend/tests/unit/core/auth/AuthProvider-recovery.dom.test.tsx b/frontend/tests/unit/core/auth/AuthProvider-recovery.dom.test.tsx new file mode 100644 index 0000000000..fa298a42e4 --- /dev/null +++ b/frontend/tests/unit/core/auth/AuthProvider-recovery.dom.test.tsx @@ -0,0 +1,297 @@ +import { afterEach, beforeEach, expect, rs, test } from "@rstest/core"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import { useContext } from "react"; + +import { AuthProvider, useAuth } from "@/core/auth/AuthProvider"; +import { AuthQueryScopeContext } from "@/core/auth/query-scope"; +import type { User } from "@/core/auth/types"; +import { isStaticWebsiteOnly } from "@/core/static-mode"; + +const push = rs.fn(); +const router = { push }; +rs.mock("next/navigation", () => ({ + useRouter: () => router, + usePathname: () => "/workspace/chats/example", +})); +rs.mock("@/core/static-mode", () => ({ + isStaticWebsiteOnly: rs.fn(() => false), +})); + +const user: User = { + id: "account-a", + email: "a@example.com", + system_role: "user", + needs_setup: false, + oauth_provider: null, +}; +let auth: ReturnType; +let owner: string | null | undefined; + +function Probe() { + auth = useAuth(); + owner = useContext(AuthQueryScopeContext); + return {auth.user?.email ?? "No confirmed user"}; +} + +function mount(initialUser: User | null = user, initialAuthUnknown = false) { + return render( + + + , + ); +} + +function userResponse(next: User = user) { + return new Response(JSON.stringify(next), { + headers: { "Content-Type": "application/json" }, + }); +} + +async function wake(event: "online" | "visibilitychange") { + await act(async () => { + if (event === "online") window.dispatchEvent(new Event(event)); + else document.dispatchEvent(new Event(event)); + }); +} + +beforeEach(() => { + rs.mocked(isStaticWebsiteOnly).mockReturnValue(false); + rs.spyOn(document, "visibilityState", "get").mockReturnValue("visible"); + rs.spyOn(Date, "now").mockReturnValue(100_000); + rs.spyOn(console, "error").mockImplementation(() => undefined); +}); + +afterEach(() => { + cleanup(); + rs.restoreAllMocks(); + push.mockReset(); +}); + +test.each(["online", "visibilitychange"] as const)( + "%s recovers unknown auth after a failed visible-tab refresh", + async (event) => { + const fetch = rs + .spyOn(globalThis, "fetch") + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(userResponse()); + mount(); + + await wake("visibilitychange"); + expect(fetch).toHaveBeenCalledTimes(1); + expect(auth.user).toBeNull(); + expect(owner).toBeUndefined(); + expect(push).not.toHaveBeenCalled(); + + // Reconnect must not wait for the failed visibility check's throttle. + rs.mocked(Date.now).mockReturnValue(event === "online" ? 100_001 : 160_001); + await wake(event); + + expect(screen.getByText("a@example.com")).toBeTruthy(); + expect(owner).toBe("account-a"); + expect(fetch).toHaveBeenCalledTimes(2); + expect(push).not.toHaveBeenCalled(); + }, +); + +test("reconnect events do not overlap an in-flight unknown-auth check", async () => { + let resolve!: (response: Response) => void; + const fetch = rs + .spyOn(globalThis, "fetch") + .mockImplementation( + () => new Promise((done) => (resolve = done)), + ); + mount(null, true); + + await act(async () => { + window.dispatchEvent(new Event("online")); + window.dispatchEvent(new Event("online")); + document.dispatchEvent(new Event("visibilitychange")); + }); + expect(fetch).toHaveBeenCalledTimes(1); + expect(auth.user).toBeNull(); + expect(owner).toBeUndefined(); + + await act(async () => resolve(userResponse())); + expect(screen.getByText("a@example.com")).toBeTruthy(); + expect(owner).toBe("account-a"); + expect(fetch).toHaveBeenCalledTimes(1); +}); + +test.each([false, true])( + "coalesces a reconnect received before a failed refresh settles (initially unknown: %s)", + async (initiallyUnknown) => { + let reject!: (reason: Error) => void; + const fetch = rs + .spyOn(globalThis, "fetch") + .mockImplementationOnce( + () => new Promise((_resolve, fail) => (reject = fail)), + ) + .mockResolvedValueOnce(userResponse()); + mount(initiallyUnknown ? null : user, initiallyUnknown); + await act(async () => { + document.dispatchEvent(new Event("visibilitychange")); + window.dispatchEvent(new Event("online")); + window.dispatchEvent(new Event("online")); + }); + expect(fetch).toHaveBeenCalledTimes(1); + + await act(async () => reject(new TypeError("Failed to fetch"))); + + expect(screen.getByText("a@example.com")).toBeTruthy(); + expect(owner).toBe("account-a"); + expect(fetch).toHaveBeenCalledTimes(2); + }, +); + +test("unknown-auth recovery retains the visible-tab throttle", async () => { + const fetch = rs + .spyOn(globalThis, "fetch") + .mockRejectedValue(new TypeError("Failed to fetch")); + mount(); + await wake("visibilitychange"); + rs.mocked(Date.now).mockReturnValue(100_001); + await wake("visibilitychange"); + expect(fetch).toHaveBeenCalledTimes(1); +}); + +test.each(["401", "logout", "account", "unmount"] as const)( + "%s cancels a queued reconnect wakeup", + async (action) => { + let resolve!: (response: Response) => void; + const pending = new Promise((done) => (resolve = done)); + const fetch = rs + .spyOn(globalThis, "fetch") + .mockImplementation((input) => + input === "/api/v1/auth/me" + ? pending + : Promise.resolve(new Response(null, { status: 204 })), + ); + const view = mount(null, true); + await wake("visibilitychange"); + await wake("online"); + + await act(async () => { + if (action === "logout") await auth.logout(); + if (action === "account") auth.applyUser({ ...user, id: "account-b" }); + if (action === "unmount") view.unmount(); + resolve( + new Response(JSON.stringify({ detail: "expired" }), { status: 401 }), + ); + }); + + expect( + fetch.mock.calls.filter(([url]) => url === "/api/v1/auth/me"), + ).toHaveLength(1); + if (action !== "unmount") { + expect(owner).toBe(action === "account" ? "account-b" : null); + } + }, +); + +test.each([false, true])( + "confirmed anonymous sessions do not restart checks (explicit logout: %s)", + async (logout) => { + const fetch = rs + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response(null, { status: 204 })); + mount(logout ? user : null); + if (logout) await act(async () => auth.logout()); + fetch.mockClear(); + await wake("online"); + await wake("visibilitychange"); + expect(fetch).not.toHaveBeenCalled(); + expect(owner).toBeNull(); + }, +); + +test("a recovered 401 remains a confirmed logout", async () => { + const fetch = rs + .spyOn(globalThis, "fetch") + .mockResolvedValue( + new Response(JSON.stringify({ detail: "expired" }), { status: 401 }), + ); + mount(null, true); + await wake("online"); + expect(owner).toBeNull(); + expect(push).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenCalledWith(expect.stringContaining("/login?next=")); + await wake("online"); + expect(fetch).toHaveBeenCalledTimes(1); +}); + +test("a hidden-tab reconnect bypasses the failed visible-tab throttle", async () => { + const fetch = rs + .spyOn(globalThis, "fetch") + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(userResponse()); + const visibility = rs.spyOn(document, "visibilityState", "get"); + mount(); + await wake("visibilitychange"); + expect(auth.user).toBeNull(); + expect(owner).toBeUndefined(); + + visibility.mockReturnValue("hidden"); + rs.mocked(Date.now).mockReturnValue(100_001); + await wake("online"); + await wake("online"); + await wake("visibilitychange"); + expect(fetch).toHaveBeenCalledTimes(1); + expect(auth.user).toBeNull(); + + visibility.mockReturnValue("visible"); + rs.mocked(Date.now).mockReturnValue(130_000); + await wake("visibilitychange"); + expect(screen.getByText("a@example.com")).toBeTruthy(); + expect(owner).toBe("account-a"); + await wake("visibilitychange"); + expect(fetch).toHaveBeenCalledTimes(2); + expect(push).not.toHaveBeenCalled(); +}); + +test("a reconnect survives a failed refresh settling while hidden", async () => { + let reject!: (reason: Error) => void; + const fetch = rs + .spyOn(globalThis, "fetch") + .mockImplementationOnce( + () => new Promise((_resolve, fail) => (reject = fail)), + ) + .mockResolvedValueOnce(userResponse()); + const visibility = rs.spyOn(document, "visibilityState", "get"); + mount(null, true); + await wake("visibilitychange"); + await wake("online"); + visibility.mockReturnValue("hidden"); + await act(async () => reject(new TypeError("Failed to fetch"))); + expect(fetch).toHaveBeenCalledTimes(1); + expect(auth.user).toBeNull(); + expect(owner).toBeUndefined(); + + visibility.mockReturnValue("visible"); + rs.mocked(Date.now).mockReturnValue(130_000); + await wake("visibilitychange"); + expect(screen.getByText("a@example.com")).toBeTruthy(); + expect(owner).toBe("account-a"); + expect(fetch).toHaveBeenCalledTimes(2); + expect(push).not.toHaveBeenCalled(); +}); + +test("static demos do not probe even when auth starts unknown", async () => { + rs.mocked(isStaticWebsiteOnly).mockReturnValue(true); + const fetch = rs.spyOn(globalThis, "fetch"); + mount(null, true); + await wake("online"); + await wake("visibilitychange"); + expect(fetch).not.toHaveBeenCalled(); +}); + +test("unmount removes automatic auth wakeups", async () => { + const fetch = rs.spyOn(globalThis, "fetch"); + const view = mount(null, true); + view.unmount(); + await wake("online"); + await wake("visibilitychange"); + expect(fetch).not.toHaveBeenCalled(); +});