diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f89f0eb..ae875d87 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -240,6 +240,17 @@ jobs: - name: Typecheck run: cd ui && pnpm run typecheck + # Neither of these ran here before. `pnpm lint` had been failing in + # ui-components for long enough to accumulate 13 findings, and nothing + # reported it, because this job only built and typechecked. The tests are + # added for the same reason: a harness CI never runs is only marginally + # more visible than a lint that CI never runs. + - name: Lint + run: cd ui && pnpm run lint + + - name: Test + run: cd ui && pnpm run test + - name: Build SDK TypeScript run: | cd sdk/typescript diff --git a/ui/packages/components/package.json b/ui/packages/components/package.json index 6e4cddb3..2d9e95c2 100644 --- a/ui/packages/components/package.json +++ b/ui/packages/components/package.json @@ -29,7 +29,8 @@ "dev": "tsup --watch", "typecheck": "tsc --noEmit", "clean": "rm -rf dist", - "lint": "eslint ." + "lint": "eslint .", + "test": "vitest run" }, "dependencies": { "@authsome/ui-core": "workspace:*", @@ -56,12 +57,15 @@ "react-dom": "^18.0.0 || ^19.0.0" }, "devDependencies": { + "@testing-library/react": "^16.3.0", "@types/react": "^19.2.18", "@types/react-dom": "^19.2.5", + "jsdom": "^28.0.1", "react": "^19.2.8", "react-dom": "^19.2.8", "tailwindcss": "^4.3.3", "tsup": "^8.5.1", - "typescript": "^5.9.3" + "typescript": "^5.9.3", + "vitest": "^4.1.11" } } diff --git a/ui/packages/components/src/components/device-authorization-form.test.tsx b/ui/packages/components/src/components/device-authorization-form.test.tsx new file mode 100644 index 00000000..45947f5d --- /dev/null +++ b/ui/packages/components/src/components/device-authorization-form.test.tsx @@ -0,0 +1,124 @@ +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { DeviceAuthorizationForm } from "./device-authorization-form"; +import { routedFetch, withProvider } from "../test-support"; + +function at(search: string): void { + window.history.pushState({}, "", `/device${search}`); +} + +function navigate(search: string): void { + act(() => { + window.history.pushState({}, "", `/device${search}`); + window.dispatchEvent(new PopStateEvent("popstate")); + }); +} + +function codeInput(container: HTMLElement): HTMLInputElement { + const input = container.querySelector("input"); + if (!input) throw new Error("no code input rendered"); + return input as HTMLInputElement; +} + +describe("DeviceAuthorizationForm", () => { + it("takes the code from ?user_code, stripping the dashes", () => { + at("?user_code=ABCD-EFGH"); + const { fetchFn } = routedFetch({}); + const { container } = render( + withProvider(, { + fetch: fetchFn, + }), + ); + expect(codeInput(container).value).toBe("ABCDEFGH"); + }); + + it("also accepts ?code", () => { + at("?code=WXYZ1234"); + const { fetchFn } = routedFetch({}); + const { container } = render( + withProvider(, { + fetch: fetchFn, + }), + ); + expect(codeInput(container).value).toBe("WXYZ1234"); + }); + + it("follows popstate to a new user_code", () => { + at("?user_code=AAAA1111"); + const { fetchFn } = routedFetch({}); + const { container } = render( + withProvider(, { + fetch: fetchFn, + }), + ); + expect(codeInput(container).value).toBe("AAAA1111"); + + navigate("?user_code=BBBB2222"); + expect(codeInput(container).value).toBe("BBBB2222"); + }); + + it("adopts a new initialCode prop", () => { + at(""); + const { fetchFn } = routedFetch({}); + const { container, rerender } = render( + withProvider( + , + { fetch: fetchFn }, + ), + ); + expect(codeInput(container).value).toBe("AAAA1111"); + + rerender( + withProvider( + , + { fetch: fetchFn }, + ), + ); + expect(codeInput(container).value).toBe("CCCC3333"); + }); + + it("keeps what the user typed when the props have not changed", () => { + // Family C regression guard. Deriving the code from the prop on every + // render, rather than only when the prop changes, silently throws away + // typing the moment anything else re-renders the form. + at("?user_code=AAAA1111"); + const { fetchFn } = routedFetch({}); + const { container, rerender } = render( + withProvider(, { + fetch: fetchFn, + }), + ); + expect(codeInput(container).value).toBe("AAAA1111"); + + fireEvent.change(codeInput(container), { target: { value: "ZZZZ9999" } }); + expect(codeInput(container).value).toBe("ZZZZ9999"); + + rerender( + withProvider(, { + fetch: fetchFn, + }), + ); + expect(codeInput(container).value).toBe("ZZZZ9999"); + }); + + it("auto-submits a complete code from the URL exactly once", async () => { + at("?user_code=ABCD-EFGH"); + const bodies: string[] = []; + const { fetchFn } = routedFetch({ + "POST /v1/oauth/device/complete": () => { + bodies.push("called"); + return { status: "approved" }; + }, + }); + + render( + withProvider(, { fetch: fetchFn }), + ); + + await waitFor(() => + expect(screen.getByText("Device authorized successfully")).toBeTruthy(), + ); + expect(bodies).toHaveLength(1); + }); +}); diff --git a/ui/packages/components/src/components/device-authorization-form.tsx b/ui/packages/components/src/components/device-authorization-form.tsx index 40d926e5..31c1d23f 100644 --- a/ui/packages/components/src/components/device-authorization-form.tsx +++ b/ui/packages/components/src/components/device-authorization-form.tsx @@ -5,6 +5,7 @@ import { useState, useCallback, useEffect, useRef } from "react"; import { useAuth } from "@authsome/ui-react"; import { CheckCircle2 } from "lucide-react"; import { cn } from "../lib/utils"; +import { useCodeFromURL } from "../lib/use-code-from-url"; import { Button } from "../primitives/button"; import { InputOTP, @@ -79,6 +80,7 @@ export function DeviceAuthorizationForm({ const initialCode = initialCodeProp ?? autoCode; const [code, setCode] = useState(initialCode?.toUpperCase() ?? ""); + const [appliedCode, setAppliedCode] = useState(initialCode?.toUpperCase()); const [error, setError] = useState(null); const [isSubmitting, setIsSubmitting] = useState(false); const [isSuccess, setIsSuccess] = useState(false); @@ -102,7 +104,7 @@ export function DeviceAuthorizationForm({ // Use the AuthClient method — routes through the client's baseURL. // Sends Bearer token when available; the method also sets // credentials: "include" so cookies are sent for same-origin setups. - await (client as any).completeDeviceAuthorization( + await client.completeDeviceAuthorization( clean, "approve", token ?? undefined, @@ -126,13 +128,23 @@ export function DeviceAuthorizationForm({ [client, codeLength, isSubmitting, onError, onSubmitProp, onSuccess, token], ); - // Update code if initialCode changes. - useEffect(() => { - const newCode = (initialCodeProp ?? autoCode)?.toUpperCase(); - if (newCode && newCode !== code && !isSubmitting) { - setCode(newCode); + // Adopt a new code from the props or the URL. This is React's documented + // "adjusting state when a prop changes" pattern rather than an effect: it + // compares against the last code it applied, so it fires once when that + // source changes and never on an unrelated re-render. + // + // The effect it replaces listed only [initialCodeProp, autoCode], which is + // what react-hooks/exhaustive-deps reported. Completing that dep list the + // way the rule asks is worse than leaving it: with `code` in the deps, every + // keystroke re-runs the effect, sees the typed value differ from the URL + // code, and overwrites what the user just typed. The test file pins that. + const desiredCode = (initialCodeProp ?? autoCode)?.toUpperCase(); + if (desiredCode !== appliedCode) { + setAppliedCode(desiredCode); + if (desiredCode && !isSubmitting) { + setCode(desiredCode); } - }, [initialCodeProp, autoCode]); + } // Auto-submit when code is pre-filled from URL and is complete. // Wait for auth to finish loading so the token is available. @@ -259,34 +271,3 @@ export function DeviceAuthorizationForm({ ); } - -/** - * Reads user_code or code from the current URL query params. - * Supports both raw codes (`ABCDEFGH`) and dash-formatted (`ABCD-EFGH`). - */ -function useCodeFromURL(): string | undefined { - const [code, setCode] = useState(() => { - if (typeof window === "undefined") return undefined; - return parseCodeFromSearch(window.location.search); - }); - - useEffect(() => { - setCode(parseCodeFromSearch(window.location.search)); - - const handlePopState = () => { - setCode(parseCodeFromSearch(window.location.search)); - }; - window.addEventListener("popstate", handlePopState); - return () => window.removeEventListener("popstate", handlePopState); - }, []); - - return code; -} - -function parseCodeFromSearch(search: string): string | undefined { - const params = new URLSearchParams(search); - const raw = params.get("user_code") ?? params.get("code"); - if (!raw) return undefined; - const cleaned = raw.replace(/[^A-Z0-9]/gi, "").toUpperCase(); - return cleaned || undefined; -} diff --git a/ui/packages/components/src/components/device-list.test.tsx b/ui/packages/components/src/components/device-list.test.tsx new file mode 100644 index 00000000..6d197592 --- /dev/null +++ b/ui/packages/components/src/components/device-list.test.tsx @@ -0,0 +1,168 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { AuthClient } from "@authsome/ui-core"; +import { describe, expect, it } from "vitest"; + +import { DeviceList } from "./device-list"; +import { + BASE, + json, + makeSession, + routedFetch, + stubAuth, + withAuth, + withProvider, +} from "../test-support"; + +/** Skeleton is the only thing in this tree with animate-pulse. */ +function showsLoading(container: HTMLElement): boolean { + return container.querySelector(".animate-pulse") !== null; +} + +const laptop = { + id: "dev_1", + name: "Ada's laptop", + browser: "Firefox", + os: "Linux", + last_seen_at: new Date().toISOString(), + trusted: true, + type: "desktop", + created_at: new Date(0).toISOString(), + user_id: "usr_1", + app_id: "app_1", + updated_at: new Date(0).toISOString(), +}; + +describe("DeviceList", () => { + it("shows the loading skeletons until the devices arrive", async () => { + let release!: () => void; + const gate = new Promise((r) => (release = r)); + + const { fetchFn } = routedFetch({ + "GET /v1/devices": async () => { + await gate; + return { devices: [laptop] }; + }, + }); + + const { container } = render( + withProvider(, { fetch: fetchFn }), + ); + + // In flight: skeletons, no device row. + await waitFor(() => expect(showsLoading(container)).toBe(true)); + expect(screen.queryByText("Ada's laptop")).toBeNull(); + + release(); + + await waitFor(() => expect(screen.getByText("Ada's laptop")).toBeTruthy()); + expect(showsLoading(container)).toBe(false); + }); + + it("reports loading again when the session token changes", async () => { + // The regression guard. A rewrite that only sets isLoading after the await + // still passes the mount case, because isLoading starts true. It fails + // here: the second load has to put the component back into loading, or the + // user stares at the previous account's devices with no spinner. + const seen: string[] = []; + let release!: () => void; + let gate = Promise.resolve(); + + const { fetchFn } = routedFetch({ + "GET /v1/devices": async ({ token }) => { + seen.push(token ?? "none"); + await gate; + return { devices: [{ ...laptop, name: `laptop for ${token}` }] }; + }, + }); + + const client = new AuthClient({ baseURL: BASE, fetch: fetchFn }); + const first = stubAuth({ fetch: fetchFn, session: makeSession("tok-1"), client }); + + const { container, rerender } = render( + withAuth(, first), + ); + + await waitFor(() => + expect(screen.getByText("laptop for tok-1")).toBeTruthy(), + ); + expect(showsLoading(container)).toBe(false); + + // Second load, held open so the in-flight state is observable. + gate = new Promise((r) => (release = r)); + const second = stubAuth({ + fetch: fetchFn, + session: makeSession("tok-2"), + client, + }); + rerender(withAuth(, second)); + + await waitFor(() => expect(showsLoading(container)).toBe(true)); + expect(screen.queryByText("laptop for tok-1")).toBeNull(); + + release(); + await waitFor(() => + expect(screen.getByText("laptop for tok-2")).toBeTruthy(), + ); + expect(seen).toEqual(["tok-1", "tok-2"]); + }); + + it("surfaces the server's message when the load fails", async () => { + const { fetchFn } = routedFetch({ + "GET /v1/devices": () => json({ error: "devices unavailable" }, 500), + }); + + const { container } = render( + withProvider(, { fetch: fetchFn }), + ); + + await waitFor(() => + expect(screen.getByRole("alert").textContent).toContain( + "devices unavailable", + ), + ); + // The error clears the loading state rather than leaving both on screen. + expect(showsLoading(container)).toBe(false); + }); + + it("reloads through the loading state after trusting a device, then reports", async () => { + // Pins two things the fix must not disturb: the refetch a handler triggers + // goes through the same loading state as any other load, and onTrust fires + // after that refetch rather than before it. + const order: string[] = []; + let trusted = false; + let release!: () => void; + let gate = Promise.resolve(); + + const { fetchFn } = routedFetch({ + "GET /v1/devices": async () => { + await gate; + order.push("list"); + return { devices: [{ ...laptop, trusted }] }; + }, + "PATCH /v1/devices/dev_1/trust": () => { + order.push("trust"); + trusted = true; + return { ...laptop, trusted: true }; + }, + }); + + const seen: string[] = []; + const { container } = render( + withProvider( seen.push(id)} />, { + fetch: fetchFn, + }), + ); + + await waitFor(() => expect(screen.getByText("Untrusted")).toBeTruthy()); + + gate = new Promise((r) => (release = r)); + fireEvent.click(screen.getByTitle("Trust device")); + + await waitFor(() => expect(showsLoading(container)).toBe(true)); + expect(seen).toEqual([]); + + release(); + await waitFor(() => expect(seen).toEqual(["dev_1"])); + expect(order).toEqual(["list", "trust", "list"]); + }); +}); diff --git a/ui/packages/components/src/components/device-list.tsx b/ui/packages/components/src/components/device-list.tsx index 05b5bc69..62477d55 100644 --- a/ui/packages/components/src/components/device-list.tsx +++ b/ui/packages/components/src/components/device-list.tsx @@ -94,17 +94,67 @@ export function DeviceList({ const { client, session } = useAuth(); const [devices, setDevices] = useState([]); - const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); + const [loadedToken, setLoadedToken] = useState(null); + const [isReloading, setIsReloading] = useState(false); const [actionLoading, setActionLoading] = useState(null); const [deleteTarget, setDeleteTarget] = useState(null); const token = session?.session_token; + // Loading is derived, not stored. The list is loading whenever what is on + // screen does not belong to the token it is now being asked about: true on + // the first render, and true again the instant the token changes. + // + // Deriving it is the whole point. It lets the effect below set no state + // synchronously, which is what react-hooks/set-state-in-effect asks for, + // without losing the spinner on a reload. Moving setIsLoading(true) past the + // await instead satisfies the rule and silently drops the reload spinner, + // because the initial `true` covers the mount and nothing covers the rest. + // That is the regression #94 shipped, and device-list.test.tsx pins it. + const isLoading = + isReloading || + loadedToken === null || + (token !== undefined && loadedToken !== token); + + // A message from the previous attempt does not belong to this one, which is + // what clearing the error at the start of a fetch used to express. + const visibleError = isLoading ? null : error; + + // The automatic load. Runs on mount and whenever the token changes, and + // touches state only after the await. + useEffect(() => { + if (!token || loadedToken === token) return; + + let cancelled = false; + void (async () => { + try { + const response = await client.listDevices(token); + if (cancelled) return; + setDevices((response.devices ?? []) as unknown as Device[]); + setError(null); + } catch (err) { + if (cancelled) return; + setError( + err instanceof Error ? err.message : "Failed to load devices", + ); + } finally { + if (!cancelled) setLoadedToken(token); + } + })(); + + return () => { + cancelled = true; + }; + }, [client, token, loadedToken]); + + // The refetch the action handlers trigger. This one is called from an event + // handler, where setting state synchronously is exactly what React expects, + // so it can raise the loading flag up front the way the old code did. const fetchDevices = useCallback(async () => { if (!token) return; - setIsLoading(true); + setIsReloading(true); setError(null); try { @@ -115,14 +165,10 @@ export function DeviceList({ err instanceof Error ? err.message : "Failed to load devices", ); } finally { - setIsLoading(false); + setIsReloading(false); } }, [client, token]); - useEffect(() => { - void fetchDevices(); - }, [fetchDevices]); - const handleTrust = async (device: Device) => { if (!token) return; @@ -169,7 +215,7 @@ export function DeviceList({ - + {isLoading ? (
diff --git a/ui/packages/components/src/components/passkey-list.test.tsx b/ui/packages/components/src/components/passkey-list.test.tsx new file mode 100644 index 00000000..8583cc8a --- /dev/null +++ b/ui/packages/components/src/components/passkey-list.test.tsx @@ -0,0 +1,123 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { AuthClient } from "@authsome/ui-core"; +import { describe, expect, it } from "vitest"; + +import { PasskeyList } from "./passkey-list"; +import { + BASE, + json, + makeSession, + routedFetch, + stubAuth, + withAuth, + withProvider, +} from "../test-support"; + +function showsLoading(container: HTMLElement): boolean { + return container.querySelector(".animate-pulse") !== null; +} + +function makeCredential(name: string) { + return { + id: `cred_${name}`, + display_name: name, + transport: ["internal"], + created_at: new Date(0).toISOString(), + }; +} + +describe("PasskeyList", () => { + it("shows the loading skeletons until the passkeys arrive", async () => { + let release!: () => void; + const gate = new Promise((r) => (release = r)); + + const { fetchFn } = routedFetch({ + "GET /v1/passkeys": async () => { + await gate; + return { credentials: [makeCredential("Yubikey")] }; + }, + }); + + const { container } = render( + withProvider(, { fetch: fetchFn }), + ); + + await waitFor(() => expect(showsLoading(container)).toBe(true)); + release(); + + await waitFor(() => expect(screen.getByText("Yubikey")).toBeTruthy()); + expect(showsLoading(container)).toBe(false); + }); + + it("settles on the empty state rather than a spinner when signed out", async () => { + // The branch that makes this component different from the other two + // lists: with no token it stops loading instead of waiting forever. A fix + // that derives loading purely from "have I loaded for this token" would + // spin here for good. + const { fetchFn, calls } = routedFetch({}); + + const { container } = render( + withProvider(, { fetch: fetchFn, session: null }), + ); + + await waitFor(() => + expect(screen.getByText("No passkeys registered")).toBeTruthy(), + ); + expect(showsLoading(container)).toBe(false); + expect(calls).not.toContain("GET /v1/passkeys"); + }); + + it("reports loading again when the session token changes", async () => { + let release!: () => void; + let gate = Promise.resolve(); + + const { fetchFn } = routedFetch({ + "GET /v1/passkeys": async ({ token }) => { + await gate; + return { credentials: [makeCredential(`key-${token}`)] }; + }, + }); + + const client = new AuthClient({ baseURL: BASE, fetch: fetchFn }); + const { container, rerender } = render( + withAuth( + , + stubAuth({ fetch: fetchFn, session: makeSession("tok-1"), client }), + ), + ); + + await waitFor(() => expect(screen.getByText("key-tok-1")).toBeTruthy()); + expect(showsLoading(container)).toBe(false); + + gate = new Promise((r) => (release = r)); + rerender( + withAuth( + , + stubAuth({ fetch: fetchFn, session: makeSession("tok-2"), client }), + ), + ); + + await waitFor(() => expect(showsLoading(container)).toBe(true)); + expect(screen.queryByText("key-tok-1")).toBeNull(); + + release(); + await waitFor(() => expect(screen.getByText("key-tok-2")).toBeTruthy()); + }); + + it("surfaces the server's message when the load fails", async () => { + const { fetchFn } = routedFetch({ + "GET /v1/passkeys": () => json({ error: "passkeys unavailable" }, 500), + }); + + const { container } = render( + withProvider(, { fetch: fetchFn }), + ); + + await waitFor(() => + expect(screen.getByRole("alert").textContent).toContain( + "passkeys unavailable", + ), + ); + expect(showsLoading(container)).toBe(false); + }); +}); diff --git a/ui/packages/components/src/components/passkey-list.tsx b/ui/packages/components/src/components/passkey-list.tsx index 60d0e151..05610120 100644 --- a/ui/packages/components/src/components/passkey-list.tsx +++ b/ui/packages/components/src/components/passkey-list.tsx @@ -1,7 +1,7 @@ "use client"; import * as React from "react"; -import { useEffect, useState, useCallback } from "react"; +import { useEffect, useState } from "react"; import { useAuth } from "@authsome/ui-react"; import { Key, @@ -210,40 +210,70 @@ export function PasskeyList({ const { client, session } = useAuth(); const [credentials, setCredentials] = useState([]); - const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); const [deleteTarget, setDeleteTarget] = useState(null); const [isDeleting, setIsDeleting] = useState(false); - const fetchPasskeys = useCallback(async () => { - if (!session?.session_token) { - setIsLoading(false); - return; - } + const token = session?.session_token; - setError(null); - setIsLoading(true); + // Which token the list on screen belongs to. `undefined` means nothing has + // settled yet, `null` means the signed-out case has settled, a string means + // that token's passkeys are what is rendered. + const [loadedFor, setLoadedFor] = useState( + undefined, + ); - try { - // The codegen collapses multiple Go "ListResponse" structs into one - // TS type and the consents-list shape wins. The real passkey - // endpoint returns {credentials}; cast to the actual shape here. - const { credentials } = (await client.listPasskeys( - session.session_token, - )) as unknown as { credentials: CredentialInfo[] }; - setCredentials(credentials); - } catch (err) { - setError( - err instanceof Error ? err.message : "Failed to load passkeys", - ); - } finally { - setIsLoading(false); - } - }, [client, session?.session_token]); + // Loading is derived rather than stored, so the effect below can avoid + // setting state synchronously (react-hooks/set-state-in-effect) without + // losing the spinner. Deriving it also keeps the signed-out case honest: + // once that settles to null it stops loading instead of spinning forever. + // + // The tempting alternative, moving setIsLoading past the await, is worse + // here than it was in #94: the pre-hydration pass has no token, settles + // loading to false, and the spinner never appears at all. passkey-list.test + // .tsx pins both that and the reload case. + const isLoading = loadedFor !== (token ?? null); + + // A message from the previous attempt does not belong to this one, which is + // what clearing the error at the start of a fetch used to express. + const visibleError = isLoading ? null : error; useEffect(() => { - void fetchPasskeys(); - }, [fetchPasskeys]); + const current = token ?? null; + if (loadedFor === current) return; + + let cancelled = false; + void (async () => { + if (current === null) { + // Nothing to fetch. Settle so the empty state can replace the skeleton. + if (!cancelled) setLoadedFor(null); + return; + } + + try { + // The codegen collapses multiple Go "ListResponse" structs into one + // TS type and the consents-list shape wins. The real passkey + // endpoint returns {credentials}; cast to the actual shape here. + const { credentials } = (await client.listPasskeys( + current, + )) as unknown as { credentials: CredentialInfo[] }; + if (cancelled) return; + setCredentials(credentials); + setError(null); + } catch (err) { + if (cancelled) return; + setError( + err instanceof Error ? err.message : "Failed to load passkeys", + ); + } finally { + if (!cancelled) setLoadedFor(current); + } + })(); + + return () => { + cancelled = true; + }; + }, [client, token, loadedFor]); async function handleDelete(): Promise { if (!deleteTarget || !session?.session_token) { @@ -285,11 +315,11 @@ export function PasskeyList({ )} - + {isLoading && } - {!isLoading && credentials.length === 0 && !error && ( + {!isLoading && credentials.length === 0 && !visibleError && ( )} diff --git a/ui/packages/components/src/components/session-list.test.tsx b/ui/packages/components/src/components/session-list.test.tsx new file mode 100644 index 00000000..58bea3fb --- /dev/null +++ b/ui/packages/components/src/components/session-list.test.tsx @@ -0,0 +1,112 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { AuthClient } from "@authsome/ui-core"; +import { describe, expect, it } from "vitest"; + +import { SessionList } from "./session-list"; +import { + BASE, + json, + makeSession, + routedFetch, + stubAuth, + withAuth, + withProvider, +} from "../test-support"; + +function showsLoading(container: HTMLElement): boolean { + return container.querySelector(".animate-pulse") !== null; +} + +function makeRow(device: string) { + return { + id: `sess_${device}`, + device, + browser: "Firefox", + ip_address: "10.0.0.1", + last_active: new Date().toISOString(), + created_at: new Date(0).toISOString(), + session_token: "other", + }; +} + +describe("SessionList", () => { + it("shows the loading skeletons until the sessions arrive", async () => { + let release!: () => void; + const gate = new Promise((r) => (release = r)); + + const { fetchFn } = routedFetch({ + "GET /v1/sessions": async () => { + await gate; + return { sessions: [makeRow("Thinkpad")] }; + }, + }); + + const { container } = render( + withProvider(, { fetch: fetchFn }), + ); + + await waitFor(() => expect(showsLoading(container)).toBe(true)); + expect(screen.queryByText(/Thinkpad/)).toBeNull(); + + release(); + + await waitFor(() => expect(screen.getByText(/Thinkpad/)).toBeTruthy()); + expect(showsLoading(container)).toBe(false); + }); + + it("reports loading again when the session token changes", async () => { + // Family A regression guard: see device-list.test.tsx. A fix that only + // sets loading after the await still passes the mount case and fails here. + let release!: () => void; + let gate = Promise.resolve(); + + const { fetchFn } = routedFetch({ + "GET /v1/sessions": async ({ token }) => { + await gate; + return { sessions: [makeRow(`box-${token}`)] }; + }, + }); + + const client = new AuthClient({ baseURL: BASE, fetch: fetchFn }); + const first = stubAuth({ + fetch: fetchFn, + session: makeSession("tok-1"), + client, + }); + + const { container, rerender } = render(withAuth(, first)); + await waitFor(() => expect(screen.getByText(/box-tok-1/)).toBeTruthy()); + expect(showsLoading(container)).toBe(false); + + gate = new Promise((r) => (release = r)); + rerender( + withAuth( + , + stubAuth({ fetch: fetchFn, session: makeSession("tok-2"), client }), + ), + ); + + await waitFor(() => expect(showsLoading(container)).toBe(true)); + expect(screen.queryByText(/box-tok-1/)).toBeNull(); + + release(); + await waitFor(() => expect(screen.getByText(/box-tok-2/)).toBeTruthy()); + }); + + it("surfaces the server's message when the load fails", async () => { + const { fetchFn } = routedFetch({ + "GET /v1/sessions": () => json({ error: "sessions unavailable" }, 500), + }); + + const { container } = render( + withProvider(, { fetch: fetchFn }), + ); + + await waitFor(() => + expect(screen.getByRole("alert").textContent).toContain( + "sessions unavailable", + ), + ); + expect(showsLoading(container)).toBe(false); + }); +}); diff --git a/ui/packages/components/src/components/session-list.tsx b/ui/packages/components/src/components/session-list.tsx index b5e4a6b9..cc289f5a 100644 --- a/ui/packages/components/src/components/session-list.tsx +++ b/ui/packages/components/src/components/session-list.tsx @@ -68,7 +68,8 @@ export function SessionList({ const { client, session } = useAuth(); const [sessions, setSessions] = useState([]); - const [isLoading, setIsLoading] = useState(true); + const [loadedToken, setLoadedToken] = useState(null); + const [isReloading, setIsReloading] = useState(false); const [error, setError] = useState(null); const [actionLoading, setActionLoading] = useState(null); const [revokeTarget, setRevokeTarget] = useState(null); @@ -77,10 +78,49 @@ export function SessionList({ const token = session?.session_token; const activeToken = currentSessionToken ?? token; + // Loading is derived, not stored: the list is loading whenever what is on + // screen does not belong to the token it is now being asked about. See the + // longer note in device-list.tsx — moving setIsLoading(true) past the await + // instead is lint-clean and silently drops the reload spinner. + const isLoading = + isReloading || + loadedToken === null || + (token !== undefined && loadedToken !== token); + + const visibleError = isLoading ? null : error; + + // The automatic load. Touches state only after the await. + useEffect(() => { + if (!token || loadedToken === token) return; + + let cancelled = false; + void (async () => { + try { + const response = await client.listSessions(token); + if (cancelled) return; + setSessions((response.sessions ?? []) as unknown as Session[]); + setError(null); + } catch (err) { + if (cancelled) return; + setError( + err instanceof Error ? err.message : "Failed to load sessions", + ); + } finally { + if (!cancelled) setLoadedToken(token); + } + })(); + + return () => { + cancelled = true; + }; + }, [client, token, loadedToken]); + + // The refetch the revoke handlers trigger. Called from an event handler, so + // it can raise the flag up front the way the old code did. const fetchSessions = useCallback(async () => { if (!token) return; - setIsLoading(true); + setIsReloading(true); setError(null); try { @@ -91,14 +131,10 @@ export function SessionList({ err instanceof Error ? err.message : "Failed to load sessions", ); } finally { - setIsLoading(false); + setIsReloading(false); } }, [client, token]); - useEffect(() => { - void fetchSessions(); - }, [fetchSessions]); - const isCurrentSession = (s: Session): boolean => { return Boolean(activeToken && s.session_token === activeToken); }; @@ -162,7 +198,7 @@ export function SessionList({ - + {isLoading ? (
diff --git a/ui/packages/components/src/components/sign-in.test.tsx b/ui/packages/components/src/components/sign-in.test.tsx new file mode 100644 index 00000000..5ba004b8 --- /dev/null +++ b/ui/packages/components/src/components/sign-in.test.tsx @@ -0,0 +1,50 @@ +import { act, render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { SignIn } from "./sign-in"; +import { routedFetch, withProvider } from "../test-support"; + +function at(pathname: string): void { + window.history.pushState({}, "", pathname); +} + +function navigate(pathname: string): void { + act(() => { + window.history.pushState({}, "", pathname); + window.dispatchEvent(new PopStateEvent("popstate")); + }); +} + +/** + * SignIn picks its screen off the sub-path. These pin that mapping through the + * component, so the shared useSubPath hook cannot be swapped underneath it + * without the routing being re-checked. + */ +describe("SignIn routing", () => { + const mount = () => { + const { fetchFn } = routedFetch({}); + return render(withProvider(, { fetch: fetchFn, session: null })); + }; + + it("shows the forgot-password screen at /sign-in/forgot-password", () => { + at("/sign-in/forgot-password"); + mount(); + expect(screen.getByText("Forgot password")).toBeTruthy(); + }); + + it("shows the reset-password screen at /sign-in/reset-password", () => { + at("/sign-in/reset-password?token=abc"); + mount(); + // The title and the submit button share this label. + expect(screen.getAllByText("Reset password").length).toBeGreaterThan(0); + }); + + it("leaves the sub-screens when the user navigates back", () => { + at("/sign-in/forgot-password"); + mount(); + expect(screen.getByText("Forgot password")).toBeTruthy(); + + navigate("/sign-in"); + expect(screen.queryByText("Forgot password")).toBeNull(); + }); +}); diff --git a/ui/packages/components/src/components/sign-in.tsx b/ui/packages/components/src/components/sign-in.tsx index 75b3ffe5..5c91cd18 100644 --- a/ui/packages/components/src/components/sign-in.tsx +++ b/ui/packages/components/src/components/sign-in.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import { useAuth } from "@authsome/ui-react"; import { safeRedirectTarget } from "@authsome/ui-core"; +import { useSubPath } from "../lib/use-sub-path"; import { SignInForm } from "./sign-in-form"; import { ForgotPasswordForm } from "./forgot-password-form"; import { ResetPasswordForm } from "./reset-password-form"; @@ -198,32 +199,3 @@ function VerifyEmailRoute({ ); } -/** - * Extracts the sub-path segment after the base path from the current URL. - * E.g. for base="/sign-in" and URL="/sign-in/forgot-password", returns "forgot-password". - */ -function useSubPath(basePath: string): string | undefined { - const [subPath, setSubPath] = React.useState(() => { - if (typeof window === "undefined") return undefined; - return extractSubPath(window.location.pathname, basePath); - }); - - React.useEffect(() => { - setSubPath(extractSubPath(window.location.pathname, basePath)); - - const handler = () => { - setSubPath(extractSubPath(window.location.pathname, basePath)); - }; - window.addEventListener("popstate", handler); - return () => window.removeEventListener("popstate", handler); - }, [basePath]); - - return subPath; -} - -function extractSubPath(pathname: string, basePath: string): string | undefined { - const normalized = basePath.replace(/\/+$/, ""); - if (!pathname.startsWith(normalized)) return undefined; - const rest = pathname.slice(normalized.length).replace(/^\/+/, ""); - return rest || undefined; -} diff --git a/ui/packages/components/src/components/sign-up-form.test.tsx b/ui/packages/components/src/components/sign-up-form.test.tsx new file mode 100644 index 00000000..835dcc57 --- /dev/null +++ b/ui/packages/components/src/components/sign-up-form.test.tsx @@ -0,0 +1,76 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import type { ClientConfig } from "@authsome/ui-core"; +import { describe, expect, it } from "vitest"; + +import { SignUpForm } from "./sign-up-form"; +import { routedFetch, withProvider } from "../test-support"; + +const withDefaults = { + signup_fields: [ + { + key: "company", + label: "Company", + type: "text", + order: 1, + default: "Acme Inc", + }, + { key: "role", label: "Role", type: "text", order: 2 }, + ], +} as unknown as ClientConfig; + +function field(key: string): HTMLInputElement { + return document.getElementById(`signup-field-${key}`) as HTMLInputElement; +} + +/** The dynamic fields live on the second step, behind the email form. */ +function continuePastEmail(): void { + fireEvent.change(document.getElementById("signup-email")!, { + target: { value: "ada@test" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Continue" })); +} + +describe("SignUpForm dynamic field defaults", () => { + const mount = (clientConfig: ClientConfig) => { + const { fetchFn } = routedFetch({}); + return render( + withProvider(, { + fetch: fetchFn, + session: null, + clientConfig, + }), + ); + }; + + it("pre-fills a configured field default", async () => { + mount(withDefaults); + continuePastEmail(); + + await waitFor(() => expect(field("company")).toBeTruthy()); + expect(field("company").value).toBe("Acme Inc"); + expect(field("role").value).toBe(""); + }); + + it("keeps what the user typed over a default across re-renders", async () => { + // Family C regression guard. Seeding defaults on every render, rather than + // only when the field config changes, throws away the user's edit as soon + // as anything else re-renders the form. + const { rerender } = mount(withDefaults); + continuePastEmail(); + + await waitFor(() => expect(field("company")).toBeTruthy()); + fireEvent.change(field("company"), { target: { value: "Other Co" } }); + expect(field("company").value).toBe("Other Co"); + + const { fetchFn } = routedFetch({}); + rerender( + withProvider(, { + fetch: fetchFn, + session: null, + clientConfig: withDefaults, + }), + ); + + expect(field("company").value).toBe("Other Co"); + }); +}); diff --git a/ui/packages/components/src/components/sign-up-form.tsx b/ui/packages/components/src/components/sign-up-form.tsx index b3fa019b..00fd2804 100644 --- a/ui/packages/components/src/components/sign-up-form.tsx +++ b/ui/packages/components/src/components/sign-up-form.tsx @@ -56,6 +56,17 @@ export interface SignUpFormComponentProps { /** * Renders a single dynamic signup field based on its type. */ +/** The configured default value for each field that declares one. */ +function defaultsFor( + fields: SignupFieldConfig[] | null, +): Record { + const defaults: Record = {}; + for (const f of fields ?? []) { + if (f.default) defaults[f.key] = f.default; + } + return defaults; +} + function DynamicField({ field, value, @@ -242,23 +253,32 @@ export function SignUpForm({ captchaCfg.provider === "turnstile" && !!captchaCfg.site_key; - // Dynamic field values — keyed by field key. - const [fieldValues, setFieldValues] = useState>({}); - - // Initialize defaults when signup fields change. - React.useEffect(() => { - if (signupFields) { - const defaults: Record = {}; - for (const f of signupFields) { - if (f.default && !fieldValues[f.key]) { - defaults[f.key] = f.default; - } - } - if (Object.keys(defaults).length > 0) { - setFieldValues((prev) => ({ ...defaults, ...prev })); - } + // Dynamic field values — keyed by field key. Seeded from the configured + // defaults on the first render rather than from an effect afterwards, so + // the first paint already shows them. + const [fieldValues, setFieldValues] = useState>(() => + defaultsFor(signupFields), + ); + + // Re-seed when the configured fields change. This is React's documented + // "adjusting state when a prop changes" pattern rather than an effect, so + // nothing is set synchronously inside one. + // + // Seeding has to happen on the first render as well as on a change, which is + // what the initializer above covers. Keying only on the identity comparison + // here would compare equal on mount and never apply the defaults at all — + // the failure sign-up-form.test.tsx pins. + // + // `prev` deliberately wins the spread: a default may only fill a field the + // user has not set. + const [appliedFields, setAppliedFields] = useState(signupFields); + if (signupFields !== appliedFields) { + setAppliedFields(signupFields); + const defaults = defaultsFor(signupFields); + if (Object.keys(defaults).length > 0) { + setFieldValues((prev) => ({ ...defaults, ...prev })); } - }, [signupFields]); // eslint-disable-line react-hooks/exhaustive-deps + } const setFieldValue = (key: string, value: string) => { setFieldValues((prev) => ({ ...prev, [key]: value })); diff --git a/ui/packages/components/src/components/sign-up.tsx b/ui/packages/components/src/components/sign-up.tsx index b90fef8e..ea672349 100644 --- a/ui/packages/components/src/components/sign-up.tsx +++ b/ui/packages/components/src/components/sign-up.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import { useClientConfig } from "@authsome/ui-react"; import { safeRedirectTarget } from "@authsome/ui-core"; +import { useSubPath } from "../lib/use-sub-path"; import { SignUpForm } from "./sign-up-form"; import { EmailVerificationForm } from "./email-verification-form"; import type { AuthCardAlign, AuthCardVariant } from "./auth-card"; @@ -118,28 +119,3 @@ export function SignUp({ ); } -function useSubPath(basePath: string): string | undefined { - const [subPath, setSubPath] = React.useState(() => { - if (typeof window === "undefined") return undefined; - return extractSubPath(window.location.pathname, basePath); - }); - - React.useEffect(() => { - setSubPath(extractSubPath(window.location.pathname, basePath)); - - const handler = () => { - setSubPath(extractSubPath(window.location.pathname, basePath)); - }; - window.addEventListener("popstate", handler); - return () => window.removeEventListener("popstate", handler); - }, [basePath]); - - return subPath; -} - -function extractSubPath(pathname: string, basePath: string): string | undefined { - const normalized = basePath.replace(/\/+$/, ""); - if (!pathname.startsWith(normalized)) return undefined; - const rest = pathname.slice(normalized.length).replace(/^\/+/, ""); - return rest || undefined; -} diff --git a/ui/packages/components/src/components/waitlist-form.tsx b/ui/packages/components/src/components/waitlist-form.tsx index 9772136c..4769acb1 100644 --- a/ui/packages/components/src/components/waitlist-form.tsx +++ b/ui/packages/components/src/components/waitlist-form.tsx @@ -58,8 +58,11 @@ export function WaitlistForm({ setIsSubmitting(true); try { - // Access baseURL from the client instance. - const baseURL = (client as any).baseURL ?? ""; + // There is no waitlist method on the generated client and no public + // accessor for its base URL, so this reads the field directly — the same + // reach-in that ui-core's own client.ts uses for /v1/client-config. Named + // shape rather than `any`, so what is being assumed is written down. + const baseURL = (client as unknown as { baseURL?: string }).baseURL ?? ""; const res = await fetch(baseURL + "/v1/waitlist/join", { method: "POST", headers: { "Content-Type": "application/json" }, diff --git a/ui/packages/components/src/lib/pop-state.ts b/ui/packages/components/src/lib/pop-state.ts new file mode 100644 index 00000000..6a98f5bf --- /dev/null +++ b/ui/packages/components/src/lib/pop-state.ts @@ -0,0 +1,12 @@ +/** + * Subscribes to history navigation. + * + * Shared by the hooks that read the current location. They read it through + * useSyncExternalStore rather than mirroring it into state, because the + * location is an external store and treating it as one is what keeps them + * from having to resync it from an effect. + */ +export function subscribeToPopState(onChange: () => void): () => void { + window.addEventListener("popstate", onChange); + return () => window.removeEventListener("popstate", onChange); +} diff --git a/ui/packages/components/src/lib/social-login.test.ts b/ui/packages/components/src/lib/social-login.test.ts new file mode 100644 index 00000000..d2169050 --- /dev/null +++ b/ui/packages/components/src/lib/social-login.test.ts @@ -0,0 +1,77 @@ +import { AuthClient } from "@authsome/ui-core"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { handleSocialLogin } from "./social-login"; +import { BASE, routedFetch } from "../test-support"; + +const realOpen = window.open; + +afterEach(() => { + window.open = realOpen; +}); + +/** A popup that never closes, so the completion poll stays parked. */ +function stubPopup(): void { + window.open = vi.fn(() => ({ closed: false }) as unknown as Window); +} + +describe("handleSocialLogin", () => { + it("sends the return target as redirect_url", async () => { + // The backend reads redirect_url (query or body) and validates + // frontend_url against the origin allowlist — see + // plugins/social/plugin.go. Passing the target in the wrong slot means + // the user is not returned where they started after signing in. + stubPopup(); + window.history.pushState({}, "", "/dashboard?next=1"); + + const { fetchFn, urls } = routedFetch({ + "POST /v1/social/github": () => ({ + auth_url: "https://github.test/login/oauth", + }), + }); + const client = new AuthClient({ baseURL: BASE, fetch: fetchFn }); + + await handleSocialLogin(client, "github", () => {}); + + const call = urls.find((u) => u.includes("/v1/social/github")); + expect(call).toBeTruthy(); + const params = new URLSearchParams(call!.split("?")[1] ?? ""); + expect(params.get("redirect_url")).toBe(window.location.href); + }); + + it("never sends a stringified object as frontend_url", async () => { + stubPopup(); + + const { fetchFn, urls } = routedFetch({ + "POST /v1/social/github": () => ({ + auth_url: "https://github.test/login/oauth", + }), + }); + const client = new AuthClient({ baseURL: BASE, fetch: fetchFn }); + + await handleSocialLogin(client, "github", () => {}); + + const call = urls.find((u) => u.includes("/v1/social/github")) ?? ""; + const params = new URLSearchParams(call.split("?")[1] ?? ""); + // The generated client runs String() over whatever it is handed, so an + // object passed into a string slot arrives as "[object Object]" — and + // frontend_url is checked against the origin allowlist. + expect(params.get("frontend_url")).not.toBe("[object Object]"); + }); + + it("reports a failed start through onError", async () => { + stubPopup(); + const { fetchFn } = routedFetch({}); + const client = new AuthClient({ baseURL: BASE, fetch: fetchFn }); + + let seen: unknown = null; + await handleSocialLogin( + client, + "github", + () => {}, + (err) => (seen = err), + ); + + expect(seen).toBeTruthy(); + }); +}); diff --git a/ui/packages/components/src/lib/social-login.ts b/ui/packages/components/src/lib/social-login.ts index 12320724..1c3e72d6 100644 --- a/ui/packages/components/src/lib/social-login.ts +++ b/ui/packages/components/src/lib/social-login.ts @@ -29,17 +29,19 @@ export function openOAuthPopup( * so the caller typically just needs to redirect/reload after completion. */ export async function handleSocialLogin( - // The codegen mis-types this endpoint on both sides: the social-OAuth - // Start{Request,Response} Go structs collide with the phone-auth pair, - // and the wrong shapes win (request gains a required `phone`, response - // loses `auth_url`). The real backend takes {app_id?, redirect_url?} - // and returns {auth_url}. The body is typed as `any` so this function - // accepts the real AuthClient; the response is cast at the assertion - // site below. + // The codegen mis-types this endpoint's response: the social-OAuth + // Start{Request,Response} Go structs collide with the phone-auth pair and + // the wrong shape wins, so the declared response loses `auth_url`. It is + // cast at the assertion site below. + // + // The request is not a body. startOAuth takes frontend_url and redirect_url + // positionally and puts them in the query string, which is what + // plugins/social/plugin.go reads (both are `query:` tagged there). client: { startOAuth: ( provider: string, - body: any, + frontendUrl?: string, + redirectUrl?: string, ) => Promise; }, providerId: string, @@ -47,11 +49,14 @@ export async function handleSocialLogin( onError?: (err: unknown) => void, ): Promise { try { - // The provider is a path parameter (already passed positionally); the - // body only carries the post-auth redirect target. - const res = (await client.startOAuth(providerId, { - redirect_url: window.location.href, - })) as { auth_url: string }; + // redirect_url is the post-auth return target. frontend_url is left unset + // so the backend falls back to the Origin/Referer it already trusts, + // rather than a value this function would be asserting. + const res = (await client.startOAuth( + providerId, + undefined, + window.location.href, + )) as { auth_url: string }; const { auth_url } = res; const popup = openOAuthPopup(auth_url); diff --git a/ui/packages/components/src/lib/use-code-from-url.ts b/ui/packages/components/src/lib/use-code-from-url.ts new file mode 100644 index 00000000..736d2909 --- /dev/null +++ b/ui/packages/components/src/lib/use-code-from-url.ts @@ -0,0 +1,29 @@ +import { useSyncExternalStore } from "react"; + +import { subscribeToPopState } from "./pop-state"; + +/** + * Reads user_code or code from the current URL query params. + * Supports both raw codes (`ABCDEFGH`) and dash-formatted (`ABCD-EFGH`). + */ +export function parseCodeFromSearch(search: string): string | undefined { + const params = new URLSearchParams(search); + const raw = params.get("user_code") ?? params.get("code"); + if (!raw) return undefined; + const cleaned = raw.replace(/[^A-Z0-9]/gi, "").toUpperCase(); + return cleaned || undefined; +} + +function getSnapshot(): string | undefined { + return parseCodeFromSearch(window.location.search); +} + +/** + * The device code carried in the current URL, kept in step with history + * navigation. Reads the location as the external store it is, so there is no + * mirrored state for an effect to resync. Reports undefined during SSR, which + * is what the previous useState initializer's window guard did. + */ +export function useCodeFromURL(): string | undefined { + return useSyncExternalStore(subscribeToPopState, getSnapshot, () => undefined); +} diff --git a/ui/packages/components/src/lib/use-sub-path.test.tsx b/ui/packages/components/src/lib/use-sub-path.test.tsx new file mode 100644 index 00000000..a08ebd35 --- /dev/null +++ b/ui/packages/components/src/lib/use-sub-path.test.tsx @@ -0,0 +1,109 @@ +import { act, render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { extractSubPath, useSubPath } from "./use-sub-path"; + +function go(pathname: string): void { + window.history.pushState({}, "", pathname); +} + +function pop(pathname: string): void { + act(() => { + window.history.pushState({}, "", pathname); + window.dispatchEvent(new PopStateEvent("popstate")); + }); +} + +function Probe({ basePath }: { basePath: string }) { + const sub = useSubPath(basePath); + return {sub ?? "(none)"}; +} + +function current(): string { + return screen.getByTestId("sub").textContent ?? ""; +} + +describe("extractSubPath", () => { + it("returns the segment following the base path", () => { + expect(extractSubPath("/sign-in/forgot-password", "/sign-in")).toBe( + "forgot-password", + ); + }); + + it("returns undefined when the path is exactly the base path", () => { + expect(extractSubPath("/sign-in", "/sign-in")).toBeUndefined(); + }); + + it("ignores trailing slashes on the base path", () => { + expect(extractSubPath("/sign-in/verify-email", "/sign-in//")).toBe( + "verify-email", + ); + }); + + it("returns undefined when the path is outside the base path", () => { + expect(extractSubPath("/settings/profile", "/sign-in")).toBeUndefined(); + }); + + it("normalizes a long run of trailing slashes", () => { + const base = "/sign-in" + "/".repeat(50_000); + expect(extractSubPath("/sign-in/verify-email", base)).toBe("verify-email"); + }); + + it("stays linear on a long run of slashes followed by a non-slash", () => { + // basePath is the `path` prop, so it is library input rather than + // something this module controls. Trimming the trailing slashes with + // /\/+$/ is polynomial on this shape: the engine matches \/+ up to the + // "x" from every start position in the run, fails $, and backtracks the + // whole way. Measured on the regex version: 46ms at 10k, 4.2s at 100k, + // 16.8s at 200k. CodeQL reports it as js/polynomial-redos. + const base = "/".repeat(100_000) + "x"; + + const started = performance.now(); + expect(extractSubPath("/unrelated", base)).toBeUndefined(); + const elapsed = performance.now() - started; + + expect(elapsed).toBeLessThan(250); + }); +}); + +describe("useSubPath", () => { + it("reports the sub-path of the current location on first render", () => { + go("/sign-in/forgot-password"); + render(); + expect(current()).toBe("forgot-password"); + }); + + it("follows popstate navigation", () => { + go("/sign-in"); + render(); + expect(current()).toBe("(none)"); + + pop("/sign-in/reset-password"); + expect(current()).toBe("reset-password"); + + pop("/sign-in"); + expect(current()).toBe("(none)"); + }); + + it("resyncs when the base path changes", () => { + // The family B regression guard. The old effect re-derived the value on + // every run, which looks redundant next to the useState initializer and + // is the obvious thing to drop. Dropping it leaves mount behaviour intact + // and breaks exactly this: the same URL read against a new base path. + go("/sign-up/verify-email"); + const { rerender } = render(); + expect(current()).toBe("(none)"); + + rerender(); + expect(current()).toBe("verify-email"); + }); + + it("stops listening once unmounted", () => { + go("/sign-in"); + const { unmount } = render(); + unmount(); + // Would warn about setting state on an unmounted component if the + // subscription outlived the render. + pop("/sign-in/forgot-password"); + }); +}); diff --git a/ui/packages/components/src/lib/use-sub-path.ts b/ui/packages/components/src/lib/use-sub-path.ts new file mode 100644 index 00000000..f94d1411 --- /dev/null +++ b/ui/packages/components/src/lib/use-sub-path.ts @@ -0,0 +1,57 @@ +import { useCallback, useSyncExternalStore } from "react"; + +import { subscribeToPopState } from "./pop-state"; + +/** + * Extracts the sub-path segment after the base path. + * + * E.g. for base="/sign-in" and pathname="/sign-in/forgot-password", returns + * "forgot-password". + */ +export function extractSubPath( + pathname: string, + basePath: string, +): string | undefined { + // The slashes are trimmed by scanning rather than with a regex. /\/+$/ is + // polynomial on a run of slashes that does not end the string: the engine + // consumes the run from every start position, fails the $, and backtracks + // through the whole run before trying the next one. It cost 46ms at 10k + // slashes and 4.2s at 100k. basePath is the caller's `path` prop, so it is + // input this module does not control, which is why CodeQL reports it as + // js/polynomial-redos. Scanning does the same job in linear time. + let end = basePath.length; + while (end > 0 && basePath[end - 1] === "/") end--; + const normalized = basePath.slice(0, end); + + if (!pathname.startsWith(normalized)) return undefined; + + let start = normalized.length; + while (start < pathname.length && pathname[start] === "/") start++; + + return pathname.slice(start) || undefined; +} + +/** + * The sub-path of the current location relative to `basePath`, kept in step + * with history navigation. + * + * The location is an external store, so this reads it as one rather than + * mirroring it into state and resyncing from an effect. That version had to + * re-derive the value at the top of every effect run, which reads as dead code + * next to the useState initializer and is the obvious line to delete — but + * deleting it silently stops the hook resyncing when `basePath` changes. + * useSyncExternalStore has no such line to lose: a new basePath is a new + * snapshot function, and React re-reads it. + * + * The third argument is the server snapshot. There is no location to read + * during SSR, so it reports undefined, which is what the useState initializer's + * `typeof window === "undefined"` guard did. + */ +export function useSubPath(basePath: string): string | undefined { + const getSnapshot = useCallback( + () => extractSubPath(window.location.pathname, basePath), + [basePath], + ); + + return useSyncExternalStore(subscribeToPopState, getSnapshot, () => undefined); +} diff --git a/ui/packages/components/src/test-support.tsx b/ui/packages/components/src/test-support.tsx new file mode 100644 index 00000000..ab9a8aae --- /dev/null +++ b/ui/packages/components/src/test-support.tsx @@ -0,0 +1,217 @@ +/** + * Shared scaffolding for this package's component tests. + * + * Two levers, deliberately: `withProvider` drives the real AuthProvider -> + * AuthManager -> AuthClient stack over an injected fetch and storage, which is + * how a test proves a component works against the shipping code path. + * `stubAuth` hands back an AuthContextValue the test owns outright, for the + * cases that need the session to change while the component stays mounted — + * AuthContext is exported from ui-react for exactly that. + */ + +import { AuthClient, AuthManager } from "@authsome/ui-core"; +import type { + AuthState, + ClientConfig, + Session, + TokenStorage, + User, +} from "@authsome/ui-core"; +import { + AuthContext, + AuthProvider, + type AuthContextValue, +} from "@authsome/ui-react"; +import type { ReactElement, ReactNode } from "react"; + +export const BASE = "https://api.example.test"; + +/** A session far enough from expiry that the manager will not try to refresh. */ +export function makeSession(token = "tok"): Session { + return { + session_token: token, + refresh_token: `${token}-refresh`, + expires_at: new Date(Date.now() + 3_600_000).toISOString(), + }; +} + +export function makeUser(overrides: Partial = {}): User { + return { + id: "usr_1", + app_id: "app_1", + env_id: "env_1", + email: "ada@test", + email_verified: true, + first_name: "Ada", + last_name: "Lovelace", + banned: false, + phone_verified: false, + created_at: new Date(0).toISOString(), + updated_at: new Date(0).toISOString(), + ...overrides, + } as User; +} + +export function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +/** Handler for one "METHOD /path" route. Return a Response or a plain body. */ +export type Route = (req: { + path: string; + method: string; + token: string | null; +}) => unknown | Promise; + +export interface RoutedFetch { + fetchFn: typeof globalThis.fetch; + /** Every "METHOD /path" the component actually requested, in order. */ + calls: string[]; + /** The same requests with their query strings, for asserting on params. */ + urls: string[]; +} + +/** + * A fetch that dispatches on "METHOD /path". Anything unrouted 404s loudly + * rather than hanging, so a missing route shows up as a failed assertion + * instead of a timeout. + */ +export function routedFetch(routes: Record): RoutedFetch { + const calls: string[] = []; + const urls: string[] = []; + + // AuthManager.initialize() fetches the profile before it will report a + // session, so every component test needs this route. Defaulting it here + // keeps it out of the tests, which care about their own endpoint. A test + // that wants a different profile, or a failing one, just overrides it. + const table: Record = { "GET /v1/me": () => makeUser(), ...routes }; + + const fetchFn = (async ( + input: string | URL | Request, + init?: RequestInit, + ): Promise => { + const url = new URL( + String(input instanceof Request ? input.url : input), + BASE, + ); + const method = (init?.method ?? "GET").toUpperCase(); + const key = `${method} ${url.pathname}`; + calls.push(key); + urls.push(`${method} ${url.pathname}${url.search}`); + + const handler = table[key]; + if (!handler) return json({ error: `no route for ${key}` }, 404); + + const auth = new Headers(init?.headers).get("Authorization"); + // Await it: a handler that holds a request open (to make the in-flight + // state observable) returns a promise, and stringifying that yields "{}". + const result = await handler({ + path: url.pathname, + method, + token: auth?.replace(/^Bearer /, "") ?? null, + }); + return result instanceof Response ? result : json(result); + }) as typeof globalThis.fetch; + + return { fetchFn, calls, urls }; +} + +export function memoryStorage(seed?: Session): TokenStorage { + const store = new Map(); + if (seed) store.set("authsome:session", JSON.stringify(seed)); + return { + getItem: (k) => store.get(k) ?? null, + setItem: (k, v) => void store.set(k, v), + removeItem: (k) => void store.delete(k), + }; +} + +/** Wrap children in a real AuthProvider over an injected fetch and storage. */ +export function withProvider( + children: ReactNode, + opts: { + fetch: typeof globalThis.fetch; + session?: Session | null; + /** Seeds the manager's client config, so useClientConfig has something + * to report without a discovery round trip. */ + clientConfig?: ClientConfig; + }, +): ReactElement { + const session = opts.session === undefined ? makeSession() : opts.session; + return ( + + {children} + + ); +} + +function unsupported(name: string): () => never { + return () => { + throw new Error(`${name} is not wired in this test`); + }; +} + +/** + * A complete AuthContextValue the test controls. Only `client` and `session` + * matter to the components under test here; the rest are present because the + * type demands them and throw if something reaches for them unexpectedly. + */ +export function stubAuth(opts: { + fetch: typeof globalThis.fetch; + session: Session | null; + user?: User | null; + /** + * Reuse a client across values. A real token change comes from the same + * AuthManager and therefore the same client, so a test that swaps the + * session must hold the client identity steady or it is also changing a + * dependency the component keys its fetch on. + */ + client?: AuthClient; +}): AuthContextValue { + const client = + opts.client ?? new AuthClient({ baseURL: BASE, fetch: opts.fetch }); + const user = opts.user === undefined ? makeUser() : opts.user; + const state: AuthState = + opts.session && user + ? { status: "authenticated", user, session: opts.session } + : { status: "unauthenticated" }; + + return { + state, + manager: new AuthManager({ baseURL: BASE, fetch: opts.fetch }), + client, + user: opts.session ? user : null, + session: opts.session, + isAuthenticated: Boolean(opts.session), + isLoading: false, + clientConfig: null, + isConfigLoaded: true, + signIn: unsupported("signIn"), + signUp: unsupported("signUp"), + signOut: unsupported("signOut"), + resendVerification: unsupported("resendVerification"), + submitMFAChallenge: unsupported("submitMFAChallenge"), + submitMFACode: unsupported("submitMFACode"), + submitRecoveryCode: unsupported("submitRecoveryCode"), + sendSMSCode: unsupported("sendSMSCode"), + submitSMSCode: unsupported("submitSMSCode"), + }; +} + +/** Wrap children in a caller-owned context value. */ +export function withAuth( + children: ReactNode, + value: AuthContextValue, +): ReactElement { + return ( + {children} + ); +} diff --git a/ui/packages/components/vitest.config.ts b/ui/packages/components/vitest.config.ts new file mode 100644 index 00000000..790fa0e9 --- /dev/null +++ b/ui/packages/components/vitest.config.ts @@ -0,0 +1,16 @@ +import { defineConfig } from "vitest/config"; + +// Same shape as ui-react's config. These components render, so they need a +// DOM, and several of them read window.location or listen for popstate. +// +// globals is on so @testing-library/react can register its own afterEach +// cleanup. Without it every render stays in the document and the next test +// finds several copies of the same element. Tests still import describe/it/ +// expect explicitly, the same way the other packages do. +export default defineConfig({ + test: { + environment: "jsdom", + globals: true, + include: ["src/**/*.test.{ts,tsx}"], + }, +}); diff --git a/ui/pnpm-lock.yaml b/ui/pnpm-lock.yaml index 158df92d..abd14536 100644 --- a/ui/pnpm-lock.yaml +++ b/ui/pnpm-lock.yaml @@ -158,12 +158,18 @@ importers: specifier: ^3.6.0 version: 3.6.0 devDependencies: + '@testing-library/react': + specifier: ^16.3.0 + version: 16.3.2(@testing-library/dom@10.4.0)(@types/react-dom@19.2.5(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) '@types/react': specifier: ^19.2.18 version: 19.2.18 '@types/react-dom': specifier: ^19.2.5 version: 19.2.5(@types/react@19.2.18) + jsdom: + specifier: ^28.0.1 + version: 28.1.0 react: specifier: ^19.2.8 version: 19.2.8 @@ -179,6 +185,9 @@ importers: typescript: specifier: ^5.9.3 version: 5.9.3 + vitest: + specifier: ^4.1.11 + version: 4.1.11(@types/node@26.3.0)(jsdom@28.1.0)(vite@8.2.2(@types/node@26.3.0)(esbuild@0.28.2)(jiti@2.7.0)) packages/core: devDependencies: