From c0e63c33990b616a7927f19cf470de42297c6354 Mon Sep 17 00:00:00 2001 From: JNO Date: Mon, 2 Nov 2026 09:01:29 +0000 Subject: [PATCH 1/3] feat(auth): ask to sign in again when the session expires When the sign-in session ended while a page was open, the page kept showing the old view and every action failed with its own confusing error, because API routes answered 401 AUTH_SESSION_MISSING. With single sign-on the session has a fixed lifetime (OIDC_SESSION_MAX_AGE, 24 h by default, no refresh), so this happened every day to anyone who kept a tab open. - A SessionWatcher in the signed-in app layout schedules a session check at the session's expiresAt and re-checks when the tab becomes visible or the browser comes back online (at most once a minute). Each check re-reads expiresAt; a check that returns an already-past expiresAt (clock skew) is retried after 30, 60 and 120 s. A network error or 5xx is never treated as expiry. There is no periodic polling. - One shared window fetch observer inspects a clone of same-origin /api/ responses with status 401 (not /api/auth/*) asynchronously, bounded to 16 KiB and one second, and raises the prompt on code AUTH_SESSION_MISSING. Responses are returned to callers unchanged and undelayed; REAUTHENTICATION_REQUIRED, 403 and other codes are ignored. - The prompt is a modal: "Your sign-in has expired" with "Sign in again" (a full page load to /login?returnTo=) and "Later". It closes again if a later check finds the session restored. - toastApiError stays silent for a 401 AUTH_SESSION_MISSING response, so the modal replaces the per-action error toasts; every other error still shows. - The login page accepts returnTo, validated as a same-origin relative path (no //, backslash, control characters or scheme), and honours it after password sign-in, for an already signed-in visitor, and through SSO, where the callback is an absolute URL on the app origin because better-auth rejects relative callbacks with a fragment or colon. When the provider cancels before better-auth reads the OAuth state, the destination is recovered from sessionStorage for the retry. Session lifetime, disableSessionRefresh and the /api/v1 contract are unchanged. --- messages/cs-CZ.json | 3 + messages/en.json | 3 + src/app/(app)/layout.tsx | 12 +- src/app/(auth)/login/page.tsx | 18 +- src/components/auth/login-form.tsx | 5 +- src/components/auth/session-watcher.tsx | 139 +++++++++ src/components/auth/sso-login.tsx | 21 +- src/components/ui/alert-dialog.tsx | 4 +- src/lib/api-errors.ts | 3 + src/lib/auth/return-path.ts | 56 ++++ src/lib/auth/session-expiry.ts | 119 ++++++++ src/tests/auth/login-return.test.tsx | 136 +++++++++ src/tests/auth/return-path.test.ts | 32 +++ src/tests/auth/session-expiry.test.ts | 228 +++++++++++++++ src/tests/auth/session-watcher.test.tsx | 280 +++++++++++++++++++ src/tests/auth/sso-callback.test.tsx | 152 ++++++++++ src/tests/dashboard/arrival-toaster.test.tsx | 1 + src/tests/sso/sso-login.test.tsx | 4 +- 18 files changed, 1204 insertions(+), 12 deletions(-) create mode 100644 src/components/auth/session-watcher.tsx create mode 100644 src/lib/auth/return-path.ts create mode 100644 src/lib/auth/session-expiry.ts create mode 100644 src/tests/auth/login-return.test.tsx create mode 100644 src/tests/auth/return-path.test.ts create mode 100644 src/tests/auth/session-expiry.test.ts create mode 100644 src/tests/auth/session-watcher.test.tsx create mode 100644 src/tests/auth/sso-callback.test.tsx diff --git a/messages/cs-CZ.json b/messages/cs-CZ.json index 16ce0d46..26337311 100644 --- a/messages/cs-CZ.json +++ b/messages/cs-CZ.json @@ -586,6 +586,9 @@ "8HimVK": "Potvrďte heslo", "PdMcML": "Resetuji...", "Yy_yDL": "Resetovat heslo", + "nfREwD": "Přihlášení vypršelo", + "BXx4Ts": "Chcete-li pokračovat, přihlaste se znovu. Neuložené úpravy na této stránce se mohou ztratit.", + "scfgdM": "Přihlásit znovu", "w3UX-L": "Účet aplikace {name} s touto e-mailovou adresou již existuje, ale poskytovatel identity adresu neověřil. Požádejte správce o její ověření.", "GZuWet": "S touto identitou se nelze přihlásit do aplikace {name}. Obraťte se na správce.", "d3nOZp": "Tato instance aplikace {name} nepřijímá nové účty. Obraťte se na správce.", diff --git a/messages/en.json b/messages/en.json index d18f435a..8625a78d 100644 --- a/messages/en.json +++ b/messages/en.json @@ -586,6 +586,9 @@ "8HimVK": "Confirm password", "PdMcML": "Resetting...", "Yy_yDL": "Reset password", + "nfREwD": "Your sign-in has expired", + "BXx4Ts": "Sign in again to continue. Unsaved edits on this page may be lost.", + "scfgdM": "Sign in again", "w3UX-L": "A {name} account with this email already exists, but your identity provider has not verified the address. Ask your administrator to verify it.", "GZuWet": "This identity cannot sign in to {name}. Ask your administrator.", "d3nOZp": "This {name} instance does not accept new accounts. Ask your administrator.", diff --git a/src/app/(app)/layout.tsx b/src/app/(app)/layout.tsx index db55a116..d07a56db 100644 --- a/src/app/(app)/layout.tsx +++ b/src/app/(app)/layout.tsx @@ -1,7 +1,9 @@ import { AppChromeProvider } from "@/components/app-chrome-provider"; +import { SessionWatcher } from "@/components/auth/session-watcher"; import { Footer } from "@/components/footer"; import { HelpProvider } from "@/components/help/help-provider"; import { RebrandBanner } from "@/components/rebrand-banner"; +import { getSession } from "@/lib/auth-server"; import { env } from "@/lib/env"; import { appChromeConfig } from "@/lib/settings/app-chrome"; @@ -10,10 +12,18 @@ export default async function AppLayout({ }: { children: React.ReactNode; }) { - const config = await appChromeConfig(); + const [config, session] = await Promise.all([ + appChromeConfig(), + getSession(), + ]); return ( + {session ? ( + + ) : null}
{/* In-app rebrand banner. Hosted-only: existing OpenPlaud users land here directly (auth redirect from `/`) and diff --git a/src/app/(auth)/login/page.tsx b/src/app/(auth)/login/page.tsx index 4bb5b2bc..69b5e12f 100644 --- a/src/app/(auth)/login/page.tsx +++ b/src/app/(auth)/login/page.tsx @@ -1,3 +1,4 @@ +import { redirect } from "next/navigation"; import { getExtracted, getLocale } from "next-intl/server"; import { HostedAuthChrome, @@ -5,14 +6,18 @@ import { } from "@/components/auth/auth-chrome"; import { LoginForm } from "@/components/auth/login-form"; import { SsoLogin } from "@/components/auth/sso-login"; -import { redirectIfAuthenticated } from "@/lib/auth-server"; +import { loginReturnPath } from "@/lib/auth/return-path"; +import { getSession } from "@/lib/auth-server"; import { deploymentProductName } from "@/lib/deployment-brand"; import { env } from "@/lib/env"; import { isSmtpConfigured } from "@/lib/smtp"; import { isSsoEnabled } from "@/lib/sso/config"; interface LoginPageProps { - searchParams: Promise<{ error?: string | string[] }>; + searchParams: Promise<{ + error?: string | string[]; + returnTo?: string | string[]; + }>; } export default async function LoginPage({ searchParams }: LoginPageProps) { @@ -22,10 +27,13 @@ export default async function LoginPage({ searchParams }: LoginPageProps) { const instanceSubtitle = i18n("Sign in to your {name} instance.", { name, }); - await redirectIfAuthenticated(); + const { error, returnTo } = await searchParams; + const destination = loginReturnPath( + Array.isArray(returnTo) ? returnTo[0] : returnTo, + ); + if ((await getSession())?.user) redirect(destination); if (isSsoEnabled()) { - const { error } = await searchParams; return ( ); } const formProps = { + returnTo: destination, registrationEnabled: !env.DISABLE_REGISTRATION, smtpConfigured: isSmtpConfigured(), }; diff --git a/src/components/auth/login-form.tsx b/src/components/auth/login-form.tsx index 48a631f7..33c2b35d 100644 --- a/src/components/auth/login-form.tsx +++ b/src/components/auth/login-form.tsx @@ -9,9 +9,11 @@ import { toast } from "sonner"; import { MetalButton } from "@/components/metal-button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; +import { loginReturnPath } from "@/lib/auth/return-path"; import { signIn } from "@/lib/auth-client"; interface LoginFormProps { + returnTo?: string; /** * Whether the "Don't have an account? Register" footer link should be * shown. Default `true`. Set to `false` when `DISABLE_REGISTRATION=true` @@ -38,6 +40,7 @@ interface LoginFormProps { export function LoginForm({ registrationEnabled = true, smtpConfigured = false, + returnTo, }: LoginFormProps) { const i18n = useExtracted(); const [email, setEmail] = useState(""); @@ -69,7 +72,7 @@ export function LoginForm({ } toast.success(i18n("Logged in successfully")); - push("/dashboard"); + push(loginReturnPath(returnTo)); refresh(); } catch (error) { const message = diff --git a/src/components/auth/session-watcher.tsx b/src/components/auth/session-watcher.tsx new file mode 100644 index 00000000..56d951ec --- /dev/null +++ b/src/components/auth/session-watcher.tsx @@ -0,0 +1,139 @@ +"use client"; + +import { useExtracted } from "next-intl"; +import { useEffect, useState } from "react"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; +import { clearSsoReturnPath, loginReturnPath } from "@/lib/auth/return-path"; +import { observeSessionExpiry } from "@/lib/auth/session-expiry"; +import { authClient } from "@/lib/auth-client"; + +const MAX_TIMER_DELAY = 2_147_483_647; +const CHECK_THROTTLE = 60_000; +const SKEW_RETRY_DELAYS = [30_000, 60_000, 120_000]; + +/** Watch a signed-in page for confirmed session expiry and offer a return to sign-in. */ +export function SessionWatcher({ expiresAt }: { expiresAt: string }) { + const i18n = useExtracted(); + const [open, setOpen] = useState(false); + const [loginUrl, setLoginUrl] = useState("/login"); + + useEffect(() => { + let active = true; + let checking = false; + let lastCheck = -Infinity; + let timer: ReturnType | undefined; + let expirySignal = 0; + let skewRetries = 0; + + clearSsoReturnPath(); + setOpen(false); + const showExpiry = () => { + expirySignal++; + const returnPath = loginReturnPath( + `${window.location.pathname}${window.location.search}${window.location.hash}`, + ); + setLoginUrl(`/login?returnTo=${encodeURIComponent(returnPath)}`); + setOpen(true); + }; + + const schedule = (deadline: number) => { + clearTimeout(timer); + if (!Number.isFinite(deadline)) return; + const delay = Math.max(0, deadline - Date.now()); + timer = setTimeout( + () => { + if (deadline > Date.now()) schedule(deadline); + else void check(); + }, + Math.min(delay, MAX_TIMER_DELAY), + ); + }; + + const check = async () => { + if (checking) return; + checking = true; + lastCheck = Date.now(); + const signal = expirySignal; + try { + const result = await authClient.getSession({ + query: { disableCookieCache: true }, + }); + if (!active || result.error || signal !== expirySignal) return; + if (result.data === null) { + clearTimeout(timer); + showExpiry(); + } else if (result.data?.session) { + setOpen(false); + const deadline = new Date( + result.data.session.expiresAt, + ).getTime(); + clearTimeout(timer); + if (deadline > Date.now()) { + skewRetries = 0; + schedule(deadline); + } else if (Number.isFinite(deadline)) { + const delay = SKEW_RETRY_DELAYS[skewRetries++]; + if (delay !== undefined) schedule(Date.now() + delay); + } + } + } catch { + } finally { + checking = false; + } + }; + + const recheck = () => { + if (Date.now() - lastCheck >= CHECK_THROTTLE) void check(); + }; + const onVisible = () => { + if (document.visibilityState === "visible") recheck(); + }; + + const stopObserving = observeSessionExpiry(showExpiry); + schedule(Date.parse(expiresAt)); + document.addEventListener("visibilitychange", onVisible); + window.addEventListener("online", recheck); + return () => { + active = false; + clearTimeout(timer); + stopObserving(); + document.removeEventListener("visibilitychange", onVisible); + window.removeEventListener("online", recheck); + }; + }, [expiresAt]); + + return ( + + + + + {i18n("Your sign-in has expired")} + + + {i18n( + "Sign in again to continue. Unsaved edits on this page may be lost.", + )} + + + + {i18n("Later")} + + {i18n("Sign in again")} + + + + + ); +} diff --git a/src/components/auth/sso-login.tsx b/src/components/auth/sso-login.tsx index f28a4f96..44b5d560 100644 --- a/src/components/auth/sso-login.tsx +++ b/src/components/auth/sso-login.tsx @@ -5,6 +5,11 @@ import { useState } from "react"; import { toast } from "sonner"; import { MetalButton } from "@/components/metal-button"; import { useProductName } from "@/components/product-name-provider"; +import { + loginReturnPath, + recalledSsoReturnPath, + rememberSsoReturnPath, +} from "@/lib/auth/return-path"; import { signIn } from "@/lib/auth-client"; import { SSO_ACCESS_DENIED_ERROR, @@ -17,6 +22,7 @@ import { } from "@/lib/sso/constants"; interface SsoLoginProps { + returnTo?: string; /** The identity provider's name, as the operator configured it. */ providerName: string; /** The `error` code the sign-in callback redirected back with, if any. */ @@ -27,7 +33,7 @@ interface SsoLoginProps { * Sign-in through the instance's OpenID Connect provider: the only way in * when single sign-on is configured. */ -export function SsoLogin({ providerName, error }: SsoLoginProps) { +export function SsoLogin({ providerName, error, returnTo }: SsoLoginProps) { const i18n = useExtracted(); const name = useProductName(); const [isLoading, setIsLoading] = useState(false); @@ -82,11 +88,20 @@ export function SsoLogin({ providerName, error }: SsoLoginProps) { const handleClick = async () => { setIsLoading(true); + const destination = loginReturnPath( + returnTo ?? (error ? recalledSsoReturnPath() : undefined), + ); + const callbackURL = new URL(destination, window.location.origin).href; + const errorCallbackURL = new URL("/login", window.location.origin); + if (destination !== "/dashboard") { + errorCallbackURL.searchParams.set("returnTo", destination); + } + rememberSsoReturnPath(destination); try { const result = await signIn.oauth2({ providerId: SSO_PROVIDER_ID, - callbackURL: "/dashboard", - errorCallbackURL: "/login", + callbackURL, + errorCallbackURL: errorCallbackURL.href, }); if (result.error) { toast.error(i18n("Sign-in failed. Please try again.")); diff --git a/src/components/ui/alert-dialog.tsx b/src/components/ui/alert-dialog.tsx index d90a3e33..cc9599f3 100644 --- a/src/components/ui/alert-dialog.tsx +++ b/src/components/ui/alert-dialog.tsx @@ -51,14 +51,16 @@ function AlertDialogOverlay({ function AlertDialogContent({ className, + overlayClassName, size = "default", ...props }: React.ComponentProps & { size?: "default" | "sm"; + overlayClassName?: string; }) { return ( - + { const body = await parseApiError(response); + if (response.status === 401 && body.code === "AUTH_SESSION_MISSING") { + return body; + } const message = body.error || opts.fallback || "Request failed"; const errorId = typeof body.details?.errorId === "string" diff --git a/src/lib/auth/return-path.ts b/src/lib/auth/return-path.ts new file mode 100644 index 00000000..c5606645 --- /dev/null +++ b/src/lib/auth/return-path.ts @@ -0,0 +1,56 @@ +/** Validate a login destination as a local path, falling back to the dashboard. */ +export function loginReturnPath(value: unknown): string { + if (typeof value !== "string" || !value.startsWith("/")) { + return "/dashboard"; + } + try { + const decoded = decodeURIComponent(value); + if ( + decoded.startsWith("//") || + decoded.includes("\\") || + Array.from(decoded).some((char) => char.charCodeAt(0) < 32) + ) { + return "/dashboard"; + } + const url = new URL(value, "https://example.com"); + if ( + url.origin !== "https://example.com" || + url.pathname.startsWith("//") + ) { + return "/dashboard"; + } + return `${url.pathname}${url.search}${url.hash}`; + } catch { + return "/dashboard"; + } +} + +const SSO_RETURN_PATH_KEY = "auth.ssoReturnPath"; + +/** Retain the destination across provider redirects that omit the OAuth state. */ +export function rememberSsoReturnPath(value: string): void { + try { + window.sessionStorage.setItem( + SSO_RETURN_PATH_KEY, + loginReturnPath(value), + ); + } catch {} +} + +/** Recover and validate the destination for a failed provider sign-in. */ +export function recalledSsoReturnPath(): string { + try { + return loginReturnPath( + window.sessionStorage.getItem(SSO_RETURN_PATH_KEY), + ); + } catch { + return "/dashboard"; + } +} + +/** Discard the retry destination once an authenticated page starts. */ +export function clearSsoReturnPath(): void { + try { + window.sessionStorage.removeItem(SSO_RETURN_PATH_KEY); + } catch {} +} diff --git a/src/lib/auth/session-expiry.ts b/src/lib/auth/session-expiry.ts new file mode 100644 index 00000000..c9c18b65 --- /dev/null +++ b/src/lib/auth/session-expiry.ts @@ -0,0 +1,119 @@ +const MAX_INSPECTION_BYTES = 16_384; +const INSPECTION_TIMEOUT = 1_000; + +type Listener = { onExpired: () => void }; +let observation: { listeners: Set; restore: () => void } | undefined; + +function isAppApiUrl(value: string, base: string): boolean { + const url = new URL(value, base); + return ( + url.origin === new URL(base).origin && + url.pathname.startsWith("/api/") && + url.pathname !== "/api/auth" && + !url.pathname.startsWith("/api/auth/") + ); +} + +async function inspectResponse( + response: Response, + onExpired: () => void, + cancellations: Set<() => void>, +): Promise { + const reader = response.clone().body?.getReader(); + if (!reader) return; + let cancelled = false; + const cancel = () => { + cancelled = true; + void reader.cancel().catch(() => {}); + }; + cancellations.add(cancel); + const timer = setTimeout(cancel, INSPECTION_TIMEOUT); + try { + const decoder = new TextDecoder(); + let text = ""; + let bytes = 0; + while (!cancelled) { + const { done, value } = await reader.read(); + if (cancelled) return; + if (done) break; + bytes += value.byteLength; + if (bytes > MAX_INSPECTION_BYTES) { + cancel(); + return; + } + text += decoder.decode(value, { stream: true }); + } + const body: unknown = JSON.parse(text + decoder.decode()); + if ( + body !== null && + typeof body === "object" && + "code" in body && + body.code === "AUTH_SESSION_MISSING" + ) { + onExpired(); + } + } finally { + clearTimeout(timer); + cancellations.delete(cancel); + reader.releaseLock(); + } +} + +/** Observe browser API expiry responses without consuming or delaying them. */ +export function observeSessionExpiry(onExpired: () => void): () => void { + if (!observation) { + const originalFetch = window.fetch; + const listeners = new Set(); + const cancellations = new Set<() => void>(); + const observedFetch: typeof fetch = async (input, init) => { + const recipients = [...listeners]; + const response = await originalFetch.call(window, input, init); + if (listeners.size === 0 || response.status !== 401) + return response; + try { + const requestedUrl = + typeof input === "string" + ? input + : input instanceof URL + ? input.href + : input.url; + if ( + isAppApiUrl(requestedUrl, window.location.href) && + (!response.url || + isAppApiUrl(response.url, window.location.href)) + ) { + void inspectResponse( + response, + () => { + for (const listener of recipients) { + if (listeners.has(listener)) + listener.onExpired(); + } + }, + cancellations, + ).catch(() => {}); + } + } catch {} + return response; + }; + window.fetch = observedFetch; + observation = { + listeners, + restore: () => { + for (const cancel of cancellations) cancel(); + if (window.fetch === observedFetch) + window.fetch = originalFetch; + }, + }; + } + const current = observation; + const listener = { onExpired }; + current.listeners.add(listener); + return () => { + current.listeners.delete(listener); + if (current.listeners.size === 0 && observation === current) { + current.restore(); + observation = undefined; + } + }; +} diff --git a/src/tests/auth/login-return.test.tsx b/src/tests/auth/login-return.test.tsx new file mode 100644 index 00000000..e561eaee --- /dev/null +++ b/src/tests/auth/login-return.test.tsx @@ -0,0 +1,136 @@ +// @vitest-environment jsdom + +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { LoginForm } from "@/components/auth/login-form"; +import { SsoLogin } from "@/components/auth/sso-login"; +import { + clearSsoReturnPath, + rememberSsoReturnPath, +} from "@/lib/auth/return-path"; + +const { email, oauth2, push, refresh } = vi.hoisted(() => ({ + email: vi.fn().mockResolvedValue({ data: {}, error: null }), + oauth2: vi.fn().mockResolvedValue({ data: {}, error: null }), + push: vi.fn(), + refresh: vi.fn(), +})); +vi.mock("@/lib/auth-client", () => ({ signIn: { email, oauth2 } })); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push, refresh }) })); +vi.mock("posthog-js", () => ({ default: { __loaded: false } })); +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); + clearSsoReturnPath(); +}); + +describe("login return destination", () => { + it.each([ + ["/dashboard?x=1", "/dashboard?x=1"], + ["//evil.example", "/dashboard"], + ])("returns password login to the validated path %s", async (returnTo, destination) => { + render(); + fireEvent.change(screen.getByLabelText("Email"), { + target: { value: "jitka@example.com" }, + }); + fireEvent.change(screen.getByLabelText("Password"), { + target: { value: "test-password" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Sign In" })); + await waitFor(() => expect(push).toHaveBeenCalledWith(destination)); + expect(refresh).toHaveBeenCalledTimes(1); + }); + + it("passes the return path to the SSO callback and keeps it for retries", () => { + render(); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + expect(oauth2).toHaveBeenCalledWith({ + providerId: "oidc", + callbackURL: `${window.location.origin}/tasks?x=1#notes`, + errorCallbackURL: `${window.location.origin}/login?returnTo=%2Ftasks%3Fx%3D1%23notes`, + }); + }); + + it("refuses an external SSO callback", () => { + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + expect(oauth2).toHaveBeenCalledWith( + expect.objectContaining({ + callbackURL: `${window.location.origin}/dashboard`, + }), + ); + }); + it("keeps the destination when provider cancellation omits returnTo", async () => { + const first = render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + await waitFor(() => expect(oauth2).toHaveBeenCalledOnce()); + first.unmount(); + render(); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + expect(oauth2).toHaveBeenLastCalledWith( + expect.objectContaining({ + callbackURL: `${window.location.origin}/tasks?x=1#notes`, + }), + ); + }); + + it("validates a tampered stored destination before retrying", () => { + rememberSsoReturnPath("/tasks"); + const key = window.sessionStorage.key(0); + if (!key) throw new Error("Missing retry storage key"); + window.sessionStorage.setItem(key, "//evil.example"); + render(); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + expect(oauth2).toHaveBeenLastCalledWith( + expect.objectContaining({ + callbackURL: `${window.location.origin}/dashboard`, + }), + ); + }); + + it("starts sign-in even when tab storage is unavailable", () => { + const get = vi + .spyOn(Storage.prototype, "getItem") + .mockImplementation(() => { + throw new Error("Unavailable"); + }); + const set = vi + .spyOn(Storage.prototype, "setItem") + .mockImplementation(() => { + throw new Error("Unavailable"); + }); + render(); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + expect(oauth2).toHaveBeenCalledWith( + expect.objectContaining({ + callbackURL: `${window.location.origin}/dashboard`, + }), + ); + get.mockRestore(); + set.mockRestore(); + }); +}); diff --git a/src/tests/auth/return-path.test.ts b/src/tests/auth/return-path.test.ts new file mode 100644 index 00000000..4c6fe64c --- /dev/null +++ b/src/tests/auth/return-path.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { loginReturnPath } from "@/lib/auth/return-path"; + +describe("loginReturnPath", () => { + it.each([ + "/dashboard?x=1", + "/tasks#notes", + "/", + "/recordings/example", + ])("accepts %s", (path) => { + expect(loginReturnPath(path)).toBe(path); + }); + it.each([ + "//evil.example", + "https://evil.example", + "/\\evil", + "javascript:", + "/%5cevil", + "/%2fevil.example", + "/..//evil.example", + "/\nevil", + "/%0aevil", + "dashboard", + "", + null, + undefined, + ["/tasks"], + "/%ZZ", + ])("refuses %s", (path) => { + expect(loginReturnPath(path)).toBe("/dashboard"); + }); +}); diff --git a/src/tests/auth/session-expiry.test.ts b/src/tests/auth/session-expiry.test.ts new file mode 100644 index 00000000..797e8b06 --- /dev/null +++ b/src/tests/auth/session-expiry.test.ts @@ -0,0 +1,228 @@ +// @vitest-environment jsdom + +import { toast } from "sonner"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { toastApiError } from "@/lib/api-errors"; +import { observeSessionExpiry } from "@/lib/auth/session-expiry"; + +vi.mock("sonner", () => ({ toast: { error: vi.fn() } })); +const originalFetch = window.fetch; +let stop = () => {}; + +afterEach(() => { + stop(); + window.fetch = originalFetch; + vi.clearAllMocks(); + vi.useRealTimers(); +}); + +function observe(response: Response) { + const fetch = vi.fn().mockResolvedValue(response); + window.fetch = fetch; + const expired = vi.fn(); + stop = observeSessionExpiry(expired); + return { expired, fetch }; +} + +describe("session expiry fetch observer", () => { + it.each([ + "string", + "URL", + "Request", + ])("detects an app 401 with %s input and leaves its body readable", async (inputType) => { + const body = { code: "AUTH_SESSION_MISSING", error: "Unauthorized" }; + const response = Response.json(body, { status: 401 }); + const { expired, fetch } = observe(response); + const url = new URL("/api/x", window.location.href); + const input = + inputType === "string" + ? "/api/x" + : inputType === "URL" + ? url + : new Request(url); + const init = { method: "POST" }; + const result = await window.fetch(input, init); + await vi.waitFor(() => expect(expired).toHaveBeenCalledTimes(1)); + expect(result).toBe(response); + expect(await result.json()).toEqual(body); + expect(fetch).toHaveBeenCalledWith(input, init); + }); + + it.each([ + [401, "REAUTHENTICATION_REQUIRED", "/api/x"], + [401, "ACCESS_NOT_GRANTED", "/api/x"], + [401, "ACCOUNT_SUSPENDED", "/api/x"], + [403, "AUTH_SESSION_MISSING", "/api/x"], + [500, "AUTH_SESSION_MISSING", "/api/x"], + [401, "AUTH_SESSION_MISSING", "/api/auth/get-session"], + [401, "AUTH_SESSION_MISSING", "/api/auth"], + [401, "AUTH_SESSION_MISSING", "https://evil.example/api/x"], + [401, "AUTH_SESSION_MISSING", "/other"], + ])("ignores %s %s at %s", async (status, code, url) => { + const { expired } = observe(Response.json({ code }, { status })); + await window.fetch(url); + expect(expired).not.toHaveBeenCalled(); + }); + + it("ignores cross-origin redirected responses", async () => { + const response = Response.json( + { code: "AUTH_SESSION_MISSING" }, + { status: 401 }, + ); + Object.defineProperty(response, "url", { + value: "https://evil.example/api/x", + }); + const { expired } = observe(response); + await window.fetch("/api/x"); + expect(expired).not.toHaveBeenCalled(); + }); + + it("preserves malformed responses and network failures", async () => { + const { expired, fetch } = observe( + new Response("not JSON", { status: 401 }), + ); + expect(await (await window.fetch("/api/x")).text()).toBe("not JSON"); + fetch.mockRejectedValueOnce(new TypeError("Network unavailable")); + await expect(window.fetch("/api/x")).rejects.toThrow( + "Network unavailable", + ); + expect(expired).not.toHaveBeenCalled(); + }); + + it("does not signal for requests completing after cleanup", async () => { + const { expired } = observe( + Response.json({ code: "AUTH_SESSION_MISSING" }, { status: 401 }), + ); + const pending = window.fetch("/api/x"); + stop(); + await pending; + expect(expired).not.toHaveBeenCalled(); + }); + + it("suppresses only this response's missing-session error", async () => { + await toastApiError( + Response.json( + { code: "AUTH_SESSION_MISSING", error: "Unauthorized" }, + { status: 401 }, + ), + { productName: "Acme" }, + ); + expect(toast.error).not.toHaveBeenCalled(); + for (const [status, code] of [ + [400, "AUTH_SESSION_MISSING"], + [401, "PLAUD_TOKEN_INVALID"], + [500, "INTERNAL_ERROR"], + ] as const) { + await toastApiError( + Response.json({ code, error: "Failed" }, { status }), + { productName: "Acme" }, + ); + } + expect(toast.error).toHaveBeenCalledTimes(3); + }); + + it("keeps genuine errors visible after an observed expiry", async () => { + const { expired } = observe( + Response.json({ code: "AUTH_SESSION_MISSING" }, { status: 401 }), + ); + await window.fetch("/api/x"); + await vi.waitFor(() => expect(expired).toHaveBeenCalledOnce()); + await toastApiError( + Response.json( + { code: "PLAUD_TOKEN_INVALID", error: "Reconnect" }, + { status: 401 }, + ), + { productName: "Acme" }, + ); + expect(toast.error).toHaveBeenCalledWith("Reconnect"); + }); + + it.each([ + "time", + "size", + ])("returns an unfinished response immediately and cancels inspection at the %s bound", async (bound) => { + vi.useFakeTimers(); + const response = new Response( + new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode( + '{"code":"AUTH_SESSION_MISSING"}', + ), + ); + }, + }), + { status: 401 }, + ); + const clone = response.clone(); + const reader = clone.body?.getReader(); + if (!reader || !clone.body) throw new Error("Missing response stream"); + const cancel = vi.spyOn(reader, "cancel"); + vi.spyOn(clone.body, "getReader").mockReturnValue(reader); + vi.spyOn(response, "clone").mockReturnValue(clone); + if (bound === "size") { + vi.spyOn(reader, "read").mockResolvedValue({ + done: false, + value: new Uint8Array(16_385), + }); + } + const { expired } = observe(response); + let returned: Response | undefined; + void window.fetch("/api/x").then((value) => { + returned = value; + }); + await vi.advanceTimersByTimeAsync(0); + expect(returned).toBe(response); + expect(response.bodyUsed).toBe(false); + if (bound === "time") { + expect(cancel).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1_000); + } + expect(cancel).toHaveBeenCalledOnce(); + expect(expired).not.toHaveBeenCalled(); + void response.body?.cancel(); + }); + + it.each([ + "first", + "last", + ])("shares one wrapper and restores fetch when the %s observer unmounts first", async (order) => { + const fetch = vi + .fn() + .mockImplementation(() => + Promise.resolve( + Response.json( + { code: "AUTH_SESSION_MISSING" }, + { status: 401 }, + ), + ), + ); + window.fetch = fetch; + const first = vi.fn(); + const second = vi.fn(); + const stopFirst = observeSessionExpiry(first); + const wrapper = window.fetch; + const stopSecond = observeSessionExpiry(second); + stop = () => { + stopFirst(); + stopSecond(); + }; + expect(window.fetch).toBe(wrapper); + await window.fetch("/api/x"); + await vi.waitFor(() => { + expect(first).toHaveBeenCalledOnce(); + expect(second).toHaveBeenCalledOnce(); + }); + const dispose = order === "first" ? stopFirst : stopSecond; + const remaining = order === "first" ? second : first; + const removed = order === "first" ? first : second; + dispose(); + dispose(); + expect(window.fetch).toBe(wrapper); + await window.fetch("/api/x"); + await vi.waitFor(() => expect(remaining).toHaveBeenCalledTimes(2)); + expect(removed).toHaveBeenCalledOnce(); + stop(); + expect(window.fetch).toBe(fetch); + }); +}); diff --git a/src/tests/auth/session-watcher.test.tsx b/src/tests/auth/session-watcher.test.tsx new file mode 100644 index 00000000..8b82a349 --- /dev/null +++ b/src/tests/auth/session-watcher.test.tsx @@ -0,0 +1,280 @@ +// @vitest-environment jsdom + +import { + act, + cleanup, + fireEvent, + render, + screen, +} from "@testing-library/react"; +import Link from "next/link"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { SessionWatcher } from "@/components/auth/session-watcher"; +import { + clearSsoReturnPath, + recalledSsoReturnPath, + rememberSsoReturnPath, +} from "@/lib/auth/return-path"; + +const { getSession } = vi.hoisted(() => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth-client", () => ({ authClient: { getSession } })); +vi.mock("next/link", () => ({ default: vi.fn() })); + +const originalFetch = window.fetch; +const now = new Date("2026-01-01T00:00:00Z").getTime(); +const expiry = (delay: number) => new Date(now + delay).toISOString(); + +async function advance(ms: number) { + await act(async () => { + await vi.advanceTimersByTimeAsync(ms); + }); +} + +async function visible() { + await act(async () => { + document.dispatchEvent(new Event("visibilitychange")); + }); +} + +beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(now); + getSession.mockReset(); + clearSsoReturnPath(); + vi.mocked(Link).mockClear(); + getSession.mockResolvedValue({ data: null, error: null }); + Object.defineProperty(document, "visibilityState", { + configurable: true, + value: "visible", + }); + window.fetch = vi.fn().mockImplementation(() => { + const response = Response.json( + { code: "AUTH_SESSION_MISSING" }, + { status: 401 }, + ); + vi.spyOn(response, "clone").mockImplementation(() => + Response.json({ code: "AUTH_SESSION_MISSING" }, { status: 401 }), + ); + return Promise.resolve(response); + }); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); + window.fetch = originalFetch; + window.history.replaceState(null, "", "/"); + vi.restoreAllMocks(); +}); + +describe("SessionWatcher", () => { + it("checks exactly at expiresAt and opens one prompt for a confirmed missing session", async () => { + render(); + await advance(4_999); + expect(getSession).not.toHaveBeenCalled(); + await advance(1); + expect(getSession).toHaveBeenCalledWith({ + query: { disableCookieCache: true }, + }); + expect(screen.getAllByRole("alertdialog")).toHaveLength(1); + expect(screen.getByText("Your sign-in has expired")).toBeTruthy(); + await act(async () => { + await window.fetch("/api/tasks"); + }); + expect(screen.getAllByRole("alertdialog")).toHaveLength(1); + }); + + it.each([ + "network", + "server", + ])("does not expire the session on a %s failure", async (failure) => { + if (failure === "network") + getSession.mockRejectedValue(new TypeError("Network unavailable")); + else + getSession.mockResolvedValue({ + data: null, + error: { status: 503 }, + }); + render(); + await advance(1_000); + expect(screen.queryByRole("alertdialog")).toBeNull(); + await advance(120_000); + expect(getSession).toHaveBeenCalledTimes(1); + }); + + it("throttles visible and online checks together and ignores hidden tabs", async () => { + getSession.mockResolvedValue({ + data: { session: { expiresAt: expiry(600_000) } }, + error: null, + }); + render(); + Object.defineProperty(document, "visibilityState", { value: "hidden" }); + await visible(); + expect(getSession).not.toHaveBeenCalled(); + Object.defineProperty(document, "visibilityState", { + value: "visible", + }); + await visible(); + await visible(); + await act(async () => { + window.dispatchEvent(new Event("online")); + }); + expect(getSession).toHaveBeenCalledTimes(1); + await advance(59_999); + await visible(); + expect(getSession).toHaveBeenCalledTimes(1); + await advance(1); + await act(async () => { + window.dispatchEvent(new Event("online")); + }); + expect(getSession).toHaveBeenCalledTimes(2); + }); + + it("replaces the original deadline with a refreshed expiresAt", async () => { + getSession.mockResolvedValueOnce({ + data: { session: { expiresAt: expiry(10_000) } }, + error: null, + }); + render(); + await advance(1_000); + expect(screen.queryByRole("alertdialog")).toBeNull(); + await advance(8_999); + expect(getSession).toHaveBeenCalledTimes(1); + await advance(1); + expect(getSession).toHaveBeenCalledTimes(2); + expect(screen.getByRole("alertdialog")).toBeTruthy(); + }); + + it("closes the prompt when a successful check confirms the session again", async () => { + render(); + await advance(1_000); + expect(screen.getByRole("alertdialog")).toBeTruthy(); + getSession.mockResolvedValue({ + data: { session: { expiresAt: expiry(600_000) } }, + error: null, + }); + await advance(60_000); + await visible(); + expect(screen.queryByRole("alertdialog")).toBeNull(); + }); + + it("retries clock-skewed expiry with bounded backoff", async () => { + getSession.mockResolvedValue({ + data: { session: { expiresAt: expiry(-1_000) } }, + error: null, + }); + render(); + await advance(1_000); + expect(getSession).toHaveBeenCalledTimes(1); + await advance(29_999); + expect(getSession).toHaveBeenCalledTimes(1); + await advance(1); + expect(getSession).toHaveBeenCalledTimes(2); + await advance(60_000); + expect(getSession).toHaveBeenCalledTimes(3); + await advance(120_000); + expect(getSession).toHaveBeenCalledTimes(4); + await advance(600_000); + expect(getSession).toHaveBeenCalledTimes(4); + expect(screen.queryByRole("alertdialog")).toBeNull(); + }); + + it("resumes normal checks when a skew retry returns a future expiry", async () => { + getSession + .mockResolvedValueOnce({ + data: { session: { expiresAt: expiry(-1_000) } }, + error: null, + }) + .mockResolvedValueOnce({ + data: { session: { expiresAt: expiry(40_000) } }, + error: null, + }); + render(); + await advance(31_000); + expect(getSession).toHaveBeenCalledTimes(2); + await advance(9_000); + expect(getSession).toHaveBeenCalledTimes(3); + expect(screen.getByRole("alertdialog")).toBeTruthy(); + }); + + it("clears the SSO retry destination when an authenticated page starts", () => { + rememberSsoReturnPath("/tasks?x=1"); + render(); + expect(recalledSsoReturnPath()).toBe("/dashboard"); + }); + + it("caps long timers without checking early", async () => { + const maxDelay = 2_147_483_647; + render(); + await advance(maxDelay); + expect(getSession).not.toHaveBeenCalled(); + await advance(5_000); + expect(getSession).toHaveBeenCalledTimes(1); + }); + + it("lets Later preserve the page and reopens on the next definite signal", async () => { + render(); + await act(async () => { + await window.fetch("/api/tasks"); + }); + fireEvent.click(screen.getByRole("button", { name: "Later" })); + expect(screen.queryByRole("alertdialog")).toBeNull(); + await act(async () => { + await window.fetch("/api/tasks"); + }); + expect(screen.getByRole("alertdialog")).toBeTruthy(); + }); + + it("offers sign-in with the current path, query and fragment", async () => { + window.history.replaceState(null, "", "/dashboard?x=1#notes"); + render(); + await advance(1_000); + const link = screen.getByRole("link", { name: "Sign in again" }); + expect(Link).not.toHaveBeenCalled(); + expect(link.getAttribute("href")).toBe( + "/login?returnTo=%2Fdashboard%3Fx%3D1%23notes", + ); + expect( + screen.getByText(/Unsaved edits on this page may be lost/), + ).toBeTruthy(); + document.addEventListener("click", (event) => event.preventDefault(), { + once: true, + }); + fireEvent.click(link); + expect(screen.queryByRole("alertdialog")).toBeNull(); + }); + + it("restores fetch and removes timers and listeners on unmount", async () => { + const fetchBeforeMount = window.fetch; + const view = render(); + view.unmount(); + expect(window.fetch).toBe(fetchBeforeMount); + await advance(120_000); + await visible(); + expect(getSession).not.toHaveBeenCalled(); + }); + + it("ignores a session check that completes after unmount", async () => { + let resolve: (result: { + data: { session: { expiresAt: string } }; + error: null; + }) => void = () => {}; + getSession.mockImplementation( + () => + new Promise((done) => { + resolve = done; + }), + ); + const view = render(); + await advance(1_000); + view.unmount(); + await act(async () => { + resolve({ + data: { session: { expiresAt: expiry(600_000) } }, + error: null, + }); + }); + expect(vi.getTimerCount()).toBe(0); + expect(screen.queryByRole("alertdialog")).toBeNull(); + }); +}); diff --git a/src/tests/auth/sso-callback.test.tsx b/src/tests/auth/sso-callback.test.tsx new file mode 100644 index 00000000..6014d3e0 --- /dev/null +++ b/src/tests/auth/sso-callback.test.tsx @@ -0,0 +1,152 @@ +// @vitest-environment jsdom + +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import { betterAuth } from "better-auth"; +import { genericOAuth } from "better-auth/plugins/generic-oauth"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { SsoLogin } from "@/components/auth/sso-login"; +import { clearSsoReturnPath } from "@/lib/auth/return-path"; + +const { handlerFetch } = vi.hoisted(() => { + const handlerFetch = vi.fn(); + vi.stubGlobal("fetch", handlerFetch); + return { handlerFetch }; +}); + +const origin = window.location.origin; +const auth = betterAuth({ + baseURL: origin, + secret: "test-secret-for-callback-validation-only", + logger: { disabled: true }, + advanced: { disableOriginCheck: false }, + account: { storeStateStrategy: "cookie" }, + onAPIError: { errorURL: `${origin}/login` }, + plugins: [ + genericOAuth({ + config: [ + { + providerId: "oidc", + clientId: "test-client", + clientSecret: "test-secret", + authorizationUrl: "https://idp.example.com/authorize", + tokenUrl: "https://idp.example.com/token", + }, + ], + }), + ], +}); + +afterEach(() => { + cleanup(); + clearSsoReturnPath(); + vi.unstubAllGlobals(); +}); + +function connectHandler() { + const responses: Response[] = []; + handlerFetch + .mockReset() + .mockImplementation( + async (input: RequestInfo | URL, init?: RequestInit) => { + const request = new Request( + input instanceof Request + ? input + : new URL(String(input), origin), + init, + ); + const response = await auth.handler(request); + responses.push(response.clone()); + if (!response.ok) return response; + const body: unknown = await response.json(); + return Response.json({ + ...(body as Record), + redirect: false, + }); + }, + ); + vi.stubGlobal("fetch", handlerFetch); + return responses; +} + +describe("SSO callbacks through better-auth", () => { + it.each([ + "/tasks?x=1#notes", + "/dashboard?filter=a:b", + ])("starts sign-in for %s through the real callback validator", async (returnTo) => { + const rejected = await auth.handler( + new Request(`${origin}/api/auth/sign-in/oauth2`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + providerId: "oidc", + callbackURL: returnTo, + }), + }), + ); + expect(rejected.status).toBe(403); + expect(await rejected.json()).toEqual( + expect.objectContaining({ code: "INVALID_CALLBACK_URL" }), + ); + const responses = connectHandler(); + render(); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + await waitFor(() => expect(responses).toHaveLength(1)); + expect(responses[0].status).toBe(200); + expect(await responses[0].json()).toEqual( + expect.objectContaining({ + url: expect.stringContaining( + "https://idp.example.com/authorize?", + ), + }), + ); + }); + + it("retains the destination after the real provider-cancellation redirect", async () => { + const responses = connectHandler(); + const first = render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + await waitFor(() => expect(responses).toHaveLength(1)); + const cancellation = await auth.handler( + new Request( + `${origin}/api/auth/oauth2/callback/oidc?error=access_denied&state=example`, + ), + ); + expect(cancellation.status).toBe(302); + const retry = new URL( + cancellation.headers.get("location") ?? "", + origin, + ); + expect(retry.pathname).toBe("/login"); + expect(retry.searchParams.has("returnTo")).toBe(false); + first.unmount(); + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + await waitFor(() => expect(responses).toHaveLength(2)); + expect(responses[1].status).toBe(200); + const request = handlerFetch.mock.calls[1]; + expect(JSON.parse(String(request[1]?.body))).toEqual( + expect.objectContaining({ + callbackURL: `${origin}/tasks?x=1#notes`, + }), + ); + }); +}); diff --git a/src/tests/dashboard/arrival-toaster.test.tsx b/src/tests/dashboard/arrival-toaster.test.tsx index 6362f662..9ccc14a3 100644 --- a/src/tests/dashboard/arrival-toaster.test.tsx +++ b/src/tests/dashboard/arrival-toaster.test.tsx @@ -16,6 +16,7 @@ vi.mock("@/components/help/help-provider", () => ({ vi.mock("@/components/footer", () => ({ Footer: () => null })); vi.mock("@/components/rebrand-banner", () => ({ RebrandBanner: () => null })); vi.mock("@/lib/env", () => ({ env: { IS_HOSTED: false } })); +vi.mock("@/lib/auth-server", () => ({ getSession: async () => null })); vi.mock("@/lib/settings/app-chrome", () => ({ appChromeConfig: async () => ({}), })); diff --git a/src/tests/sso/sso-login.test.tsx b/src/tests/sso/sso-login.test.tsx index aae48c7a..0f1b03db 100644 --- a/src/tests/sso/sso-login.test.tsx +++ b/src/tests/sso/sso-login.test.tsx @@ -23,8 +23,8 @@ describe("SsoLogin", () => { ); expect(oauth2).toHaveBeenCalledWith({ providerId: "oidc", - callbackURL: "/dashboard", - errorCallbackURL: "/login", + callbackURL: `${window.location.origin}/dashboard`, + errorCallbackURL: `${window.location.origin}/login`, }); expect(screen.queryByRole("alert")).toBeNull(); }); From 7f7b56a35746d5ab427b9e28e34cf67eb2d5eef3 Mon Sep 17 00:00:00 2001 From: JNO Date: Mon, 2 Nov 2026 09:43:22 +0000 Subject: [PATCH 2/3] fix(auth): recover the SSO return path after a provider cancellation The login page always passed a validated returnTo (falling back to /dashboard) to SsoLogin, so the client's sessionStorage fallback never ran: after a cancellation better-auth redirects to /login?error=... without returnTo and the retry landed on the dashboard. The page now forwards returnTo only when the query has one; the redirect of a signed-in visitor still uses the validated destination. --- src/app/(auth)/login/page.tsx | 5 +- .../321-sso-cancellation-return-path.test.tsx | 131 ++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 src/tests/regressions/321-sso-cancellation-return-path.test.tsx diff --git a/src/app/(auth)/login/page.tsx b/src/app/(auth)/login/page.tsx index 69b5e12f..5432755f 100644 --- a/src/app/(auth)/login/page.tsx +++ b/src/app/(auth)/login/page.tsx @@ -32,6 +32,7 @@ export default async function LoginPage({ searchParams }: LoginPageProps) { Array.isArray(returnTo) ? returnTo[0] : returnTo, ); if ((await getSession())?.user) redirect(destination); + const clientReturnTo = returnTo === undefined ? undefined : destination; if (isSsoEnabled()) { return ( @@ -42,14 +43,14 @@ export default async function LoginPage({ searchParams }: LoginPageProps) { ); } const formProps = { - returnTo: destination, + returnTo: clientReturnTo, registrationEnabled: !env.DISABLE_REGISTRATION, smtpConfigured: isSmtpConfigured(), }; diff --git a/src/tests/regressions/321-sso-cancellation-return-path.test.tsx b/src/tests/regressions/321-sso-cancellation-return-path.test.tsx new file mode 100644 index 00000000..62df3f20 --- /dev/null +++ b/src/tests/regressions/321-sso-cancellation-return-path.test.tsx @@ -0,0 +1,131 @@ +// @vitest-environment jsdom + +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import type { PropsWithChildren } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import LoginPage from "@/app/(auth)/login/page"; +import { + clearSsoReturnPath, + rememberSsoReturnPath, +} from "@/lib/auth/return-path"; + +const { oauth2, email, push, refresh, redirect, getSession, ssoEnabled } = + vi.hoisted(() => ({ + oauth2: vi.fn().mockResolvedValue({ data: {}, error: null }), + email: vi.fn().mockResolvedValue({ data: {}, error: null }), + push: vi.fn(), + refresh: vi.fn(), + redirect: vi.fn(() => { + throw new Error("NEXT_REDIRECT"); + }), + getSession: vi + .fn<() => Promise<{ user: { id: string } } | null>>() + .mockResolvedValue(null), + ssoEnabled: vi.fn().mockReturnValue(true), + })); + +vi.mock("@/lib/auth-client", () => ({ signIn: { oauth2, email } })); +vi.mock("@/lib/auth-server", () => ({ getSession })); +vi.mock("@/lib/sso/config", () => ({ isSsoEnabled: ssoEnabled })); +vi.mock("@/lib/smtp", () => ({ isSmtpConfigured: () => false })); +vi.mock("@/lib/env", () => ({ + env: { + IS_HOSTED: false, + DISABLE_REGISTRATION: false, + OIDC_PROVIDER_NAME: "Acme", + }, +})); +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push, refresh }), + redirect, +})); +vi.mock("next-intl/server", () => ({ + getExtracted: async () => (message: string) => message, + getLocale: async () => "en", +})); +vi.mock("@/components/auth/auth-chrome", () => ({ + SelfHostAuthChrome: ({ children }: PropsWithChildren) => children, + HostedAuthChrome: ({ children }: PropsWithChildren) => children, +})); +vi.mock("posthog-js", () => ({ default: { __loaded: false } })); +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +afterEach(() => { + cleanup(); + clearSsoReturnPath(); + vi.clearAllMocks(); + getSession.mockResolvedValue(null); + ssoEnabled.mockReturnValue(true); +}); + +describe("login page return destination wiring", () => { + it.each([ + [undefined, "/tasks?x=1#notes"], + [ + "/recordings?view=recent#selected", + "/recordings?view=recent#selected", + ], + ["/dashboard", "/dashboard"], + ["//evil.example", "/dashboard"], + ])("retries provider cancellation with returnTo=%s", async (returnTo, destination) => { + rememberSsoReturnPath("/tasks?x=1#notes"); + render( + await LoginPage({ + searchParams: Promise.resolve({ + error: "access_denied", + ...(returnTo === undefined ? {} : { returnTo }), + }), + }), + ); + expect(screen.getByRole("alert").textContent).toContain( + "Sign-in was cancelled at the identity provider.", + ); + fireEvent.click( + screen.getByRole("button", { name: "Sign in with Acme" }), + ); + expect(oauth2).toHaveBeenCalledWith( + expect.objectContaining({ + callbackURL: `${window.location.origin}${destination}`, + }), + ); + }); + + it.each([ + [undefined, "/dashboard"], + ["/tasks?x=1#notes", "/tasks?x=1#notes"], + ["//evil.example", "/dashboard"], + ])("redirects a signed-in visitor with returnTo=%s", async (returnTo, destination) => { + getSession.mockResolvedValue({ user: { id: "example-user" } }); + await expect( + LoginPage({ searchParams: Promise.resolve({ returnTo }) }), + ).rejects.toThrow("NEXT_REDIRECT"); + expect(redirect).toHaveBeenCalledWith(destination); + }); + + it.each([ + [undefined, "/dashboard"], + ["/tasks?x=1#notes", "/tasks?x=1#notes"], + ])("returns password login with returnTo=%s", async (returnTo, destination) => { + ssoEnabled.mockReturnValue(false); + render( + await LoginPage({ + searchParams: Promise.resolve({ returnTo }), + }), + ); + fireEvent.change(screen.getByLabelText("Email"), { + target: { value: "jitka@example.com" }, + }); + fireEvent.change(screen.getByLabelText("Password"), { + target: { value: "test-password" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Sign In" })); + await waitFor(() => expect(push).toHaveBeenCalledWith(destination)); + expect(refresh).toHaveBeenCalledOnce(); + }); +}); From 464875f87545d2e435452802ebe66aff18c6b1de Mon Sep 17 00:00:00 2001 From: JNO Date: Mon, 2 Nov 2026 10:00:28 +0000 Subject: [PATCH 3/3] fix(auth): notify watchers that subscribe while a request is in flight The fetch observer snapshotted its listeners before awaiting the request and notified only that snapshot, so a watcher mounted during the request missed an AUTH_SESSION_MISSING response. It now notifies the listeners registered when the response is inspected. --- src/lib/auth/session-expiry.ts | 6 ++---- src/tests/auth/session-expiry.test.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/lib/auth/session-expiry.ts b/src/lib/auth/session-expiry.ts index c9c18b65..5e24a7d3 100644 --- a/src/lib/auth/session-expiry.ts +++ b/src/lib/auth/session-expiry.ts @@ -66,7 +66,6 @@ export function observeSessionExpiry(onExpired: () => void): () => void { const listeners = new Set(); const cancellations = new Set<() => void>(); const observedFetch: typeof fetch = async (input, init) => { - const recipients = [...listeners]; const response = await originalFetch.call(window, input, init); if (listeners.size === 0 || response.status !== 401) return response; @@ -85,9 +84,8 @@ export function observeSessionExpiry(onExpired: () => void): () => void { void inspectResponse( response, () => { - for (const listener of recipients) { - if (listeners.has(listener)) - listener.onExpired(); + for (const listener of [...listeners]) { + listener.onExpired(); } }, cancellations, diff --git a/src/tests/auth/session-expiry.test.ts b/src/tests/auth/session-expiry.test.ts index 797e8b06..6533abd5 100644 --- a/src/tests/auth/session-expiry.test.ts +++ b/src/tests/auth/session-expiry.test.ts @@ -99,6 +99,32 @@ describe("session expiry fetch observer", () => { expect(expired).not.toHaveBeenCalled(); }); + it("signals a watcher that subscribed while the request was in flight", async () => { + let resolve: (response: Response) => void = () => {}; + window.fetch = vi.fn().mockReturnValue( + new Promise((done) => { + resolve = done; + }), + ); + const first = vi.fn(); + const late = vi.fn(); + const stopFirst = observeSessionExpiry(first); + const pending = window.fetch("/api/x"); + const stopLate = observeSessionExpiry(late); + stop = () => { + stopFirst(); + stopLate(); + }; + resolve( + Response.json({ code: "AUTH_SESSION_MISSING" }, { status: 401 }), + ); + await pending; + await vi.waitFor(() => { + expect(first).toHaveBeenCalledOnce(); + expect(late).toHaveBeenCalledOnce(); + }); + }); + it("suppresses only this response's missing-session error", async () => { await toastApiError( Response.json(