From 0b2af62f5bae1cc96cc8cf267801c080d2cdc7df Mon Sep 17 00:00:00 2001 From: cxymds Date: Tue, 10 Nov 2026 10:44:15 +0800 Subject: [PATCH 1/2] fix(auth): prevent redirects from interrupting OIDC logout Keep federated logout in progress until its full-page navigation completes so dashboard guards and stale API or S3 errors cannot cancel the IdP redirect. Add behavioral regression coverage and a recovery link for cancelled navigation. --- components/dashboard-auth-guard.tsx | 41 +- contexts/api-context.tsx | 5 +- contexts/auth-context.tsx | 28 +- contexts/s3-context.tsx | 15 +- tests/hooks/use-oidc-logout.test.ts | 428 ++++++++++++++++++++ tests/hooks/use-permissions-refresh.test.ts | 3 +- tests/lib/oidc-logout-source.test.js | 16 - 7 files changed, 501 insertions(+), 35 deletions(-) create mode 100644 tests/hooks/use-oidc-logout.test.ts delete mode 100644 tests/lib/oidc-logout-source.test.js diff --git a/components/dashboard-auth-guard.tsx b/components/dashboard-auth-guard.tsx index 52d95ce1..519c9874 100644 --- a/components/dashboard-auth-guard.tsx +++ b/components/dashboard-auth-guard.tsx @@ -3,18 +3,20 @@ import { useEffect } from "react" import { useTranslation } from "react-i18next" import { Spinner } from "@/components/ui/spinner" +import { Button } from "@/components/ui/button" import { usePathname, useRouter } from "next/navigation" import { useAuth } from "@/contexts/auth-context" import { useApiReady } from "@/contexts/api-context" import { useS3Ready } from "@/contexts/s3-context" import { usePermissions } from "@/hooks/use-permissions" import { canAccessDashboardRoute } from "@/lib/dashboard-route-meta" +import { getLoginRoute } from "@/lib/routes" export function DashboardAuthGuard({ children }: { children: React.ReactNode }) { const { t } = useTranslation() const router = useRouter() const pathname = usePathname() - const { isAuthenticated } = useAuth() + const { isAuthenticated, getIsLoggingOut } = useAuth() const { isReady: apiReady } = useApiReady() const { isReady: s3Ready } = useS3Ready() const { isAdmin, hasFetchedPolicy, hasResolvedAdmin, canAccessPath } = usePermissions() @@ -22,25 +24,48 @@ export function DashboardAuthGuard({ children }: { children: React.ReactNode }) const isReady = apiReady && s3Ready useEffect(() => { - if (isReady && !isAuthenticated) { + if (isReady && !isAuthenticated && !getIsLoggingOut()) { router.replace("/auth/login/?unauthorized=true") } - }, [isAuthenticated, isReady, router]) + }, [isAuthenticated, isReady, getIsLoggingOut, router]) useEffect(() => { - if (!isReady || !isAuthenticated || !hasResolvedAdmin) return + if (!isReady || !isAuthenticated || !hasResolvedAdmin || getIsLoggingOut()) return if (!isAdmin && !hasFetchedPolicy) return if (!canAccessDashboardRoute(pathname, { isAdmin, canAccessPath })) { router.replace("/403/") } - }, [isReady, isAuthenticated, hasResolvedAdmin, isAdmin, hasFetchedPolicy, canAccessPath, pathname, router]) + }, [ + isReady, + isAuthenticated, + hasResolvedAdmin, + isAdmin, + hasFetchedPolicy, + canAccessPath, + pathname, + getIsLoggingOut, + router, + ]) // Background refreshes must not unmount the page that requested them. if (!isReady || !isAuthenticated || !hasResolvedAdmin || (!isAdmin && !hasFetchedPolicy)) { return ( -
- - {t("Loading…")} +
+
+ + {t("Loading…")} +
+ {!isAuthenticated && ( + + )}
) } diff --git a/contexts/api-context.tsx b/contexts/api-context.tsx index 82d635a3..5a64e3d8 100644 --- a/contexts/api-context.tsx +++ b/contexts/api-context.tsx @@ -17,7 +17,7 @@ interface ApiContextValue { const ApiContext = createContext({ api: null, isReady: false }) export function ApiProvider({ children }: { children: React.ReactNode }) { - const { credentials, isAuthenticated, logout } = useAuth() + const { credentials, isAuthenticated, logout, getIsLoggingOut } = useAuth() const [apiClient, setApiClient] = useState(null) const [isReady, setIsReady] = useState(false) const router = useRouter() @@ -58,10 +58,12 @@ export function ApiProvider({ children }: { children: React.ReactNode }) { const errorHandler = new ApiErrorHandler({ onUnauthorized: async () => { + if (getIsLoggingOut()) return logout() router.replace("/auth/login/") }, onForbidden: async (url) => { + if (getIsLoggingOut()) return // If the forbidden error happens on the accountinfo endpoint, it means the session is invalid // or the user has no permissions at all. In this case, we should log out. if (url?.includes("/is-admin") || url?.includes("/accountinfo") || url?.includes("/version")) { @@ -94,6 +96,7 @@ export function ApiProvider({ children }: { children: React.ReactNode }) { credentials?.SecretAccessKey, credentials?.SessionToken, logout, + getIsLoggingOut, router, ]) diff --git a/contexts/auth-context.tsx b/contexts/auth-context.tsx index 51d56527..3aae7600 100644 --- a/contexts/auth-context.tsx +++ b/contexts/auth-context.tsx @@ -1,6 +1,6 @@ "use client" -import { createContext, useCallback, useContext, useMemo, type ReactNode } from "react" +import { createContext, useCallback, useContext, useEffect, useMemo, useRef, type ReactNode } from "react" import type { AwsCredentialIdentity, AwsCredentialIdentityProvider } from "@aws-sdk/types" import type { SiteConfig } from "@/types/config" import { getLoginRoute } from "@/lib/routes" @@ -41,6 +41,7 @@ interface AuthContextValue { logout: () => void logoutAndRedirect: () => void logoutWithOidcRedirect: () => Promise + getIsLoggingOut: () => boolean setIsAdmin: (value: boolean) => void getIsAdmin: () => boolean credentials: Credentials | undefined @@ -81,6 +82,9 @@ function isValidOidcLogoutSession(session: OidcLogoutSession | undefined): sessi } export function AuthProvider({ children }: { children: ReactNode }) { + // Old API/S3 callbacks must see this flag before React commits the cleared credentials. + const loggingOutRef = useRef(false) + const getIsLoggingOut = useCallback(() => loggingOutRef.current, []) const [store, setStore] = useLocalStorage("auth.credentials", undefined) const [isAdminStore, setIsAdminStore] = useLocalStorage("auth.isAdmin", undefined) const [oidcSessionStore, setOidcSessionStore] = useLocalStorage( @@ -90,6 +94,7 @@ export function AuthProvider({ children }: { children: ReactNode }) { const setCredentials = useCallback( (credentials: Credentials) => { + loggingOutRef.current = false setStore(credentials) }, [setStore], @@ -221,12 +226,24 @@ export function AuthProvider({ children }: { children: ReactNode }) { setOidcSessionStore(undefined) }, [setStore, setIsAdminStore, setOidcSessionStore]) + useEffect(() => { + const handlePageShow = (event: PageTransitionEvent) => { + if (!event.persisted || !loggingOutRef.current) return + // Reload current storage; another document may already have established a new session. + window.location.replace(getLoginRoute()) + } + window.addEventListener("pageshow", handlePageShow) + return () => window.removeEventListener("pageshow", handlePageShow) + }, []) + const logoutAndRedirect = useCallback(() => { + if (loggingOutRef.current) return logout() window.location.href = getLoginRoute() }, [logout]) const logoutWithOidcRedirect = useCallback(async () => { + if (loggingOutRef.current) return true const oidcSession = isValidOidcLogoutSession(oidcSessionStore) ? oidcSessionStore : undefined if (!oidcSession) { @@ -234,16 +251,17 @@ export function AuthProvider({ children }: { children: ReactNode }) { return false } + loggingOutRef.current = true try { - // Build the URL before logout(), or DashboardAuthGuard redirects to login before this navigation runs. - const { configManager } = await import("@/lib/config") const config = await configManager.loadConfig() const logoutUrl = buildOidcLogoutUrl(config.serverHost, oidcSession.logoutToken) logout() window.location.href = logoutUrl return true - } catch { + } catch (error) { + loggingOutRef.current = false + console.error("Failed to start OIDC logout", error) logout() return false } @@ -260,6 +278,7 @@ export function AuthProvider({ children }: { children: ReactNode }) { logout, logoutAndRedirect, logoutWithOidcRedirect, + getIsLoggingOut, setIsAdmin, getIsAdmin, credentials, @@ -273,6 +292,7 @@ export function AuthProvider({ children }: { children: ReactNode }) { logout, logoutAndRedirect, logoutWithOidcRedirect, + getIsLoggingOut, setIsAdmin, getIsAdmin, credentials, diff --git a/contexts/s3-context.tsx b/contexts/s3-context.tsx index 3b733152..ea0fbff0 100644 --- a/contexts/s3-context.tsx +++ b/contexts/s3-context.tsx @@ -60,7 +60,7 @@ const S3Context = createContext({ }) export function S3Provider({ children }: { children: React.ReactNode }) { - const { credentials, isAuthenticated, logout } = useAuth() + const { credentials, isAuthenticated, logout, getIsLoggingOut } = useAuth() const [s3Client, setS3Client] = useState(null) const [isReady, setIsReady] = useState(false) const router = useRouter() @@ -152,8 +152,10 @@ export function S3Provider({ children }: { children: React.ReactNode }) { message?: string } if (err?.$metadata?.httpStatusCode === 401) { - logout() - router.replace("/auth/login/") + if (!getIsLoggingOut()) { + logout() + router.replace("/auth/login/") + } return { response: { statusCode: 401, headers: {} } } } if (err?.$metadata?.httpStatusCode === 403) { @@ -161,8 +163,10 @@ export function S3Provider({ children }: { children: React.ReactNode }) { const isUnauthorizedAccess = codeText === "unauthorizedaccess" const isInvalidAccessKey = codeText === "invalidaccesskeyid" if (isUnauthorizedAccess || isInvalidAccessKey) { - logout() - router.replace("/auth/login/") + if (!getIsLoggingOut()) { + logout() + router.replace("/auth/login/") + } return { response: { statusCode: 401, headers: {} } } } } @@ -191,6 +195,7 @@ export function S3Provider({ children }: { children: React.ReactNode }) { credentials?.SecretAccessKey, credentials?.SessionToken, logout, + getIsLoggingOut, router, ]) diff --git a/tests/hooks/use-oidc-logout.test.ts b/tests/hooks/use-oidc-logout.test.ts new file mode 100644 index 00000000..84fc7d8d --- /dev/null +++ b/tests/hooks/use-oidc-logout.test.ts @@ -0,0 +1,428 @@ +import test, { type TestContext } from "node:test" +import assert from "node:assert/strict" +import { readFileSync } from "node:fs" +import { act, cloneElement, createElement, type ReactElement, type ReactNode } from "react" +import { createRoot } from "react-dom/client" +import { Window } from "happy-dom" +import ts from "typescript" +import type { ApiErrorHandler } from "../../lib/api-error-handler" +import type { useAuth as UseAuth } from "../../contexts/auth-context" + +const mocksKey = Symbol.for("oidc-logout-navigation") +const mockExports: Record = { + "next/navigation": ["useRouter", "usePathname"], + "react-i18next": ["useTranslation"], + "@/components/ui/button": ["Button"], + "@/components/ui/spinner": ["Spinner"], + "@/hooks/use-permissions": ["usePermissions"], + "@/lib/mfa-challenge": ["fetchMfaChallenge"], + "@/lib/sts": ["getStsToken"], + "@/lib/api-prefix-middleware": ["addApiPrefixMiddleware"], + "@/lib/bucket-replication-delete": ["addDeleteReplicationMiddleware"], +} +const constructors: Record = { + "@/lib/api-client": "ApiClient", + "@/lib/aws4fetch": "AwsClient", + "@aws-sdk/client-s3": "S3Client", +} +const overrides: Record = {} +const dataUrl = (source: string) => `data:text/javascript,${encodeURIComponent(source)}` +const mocks = 'globalThis[Symbol.for("oidc-logout-navigation")]' +overrides["@/lib/config"] = dataUrl( + `export const configManager = { loadConfig: (...args) => ${mocks}.configManager.loadConfig(...args), clearCache: () => {} }`, +) + +function moduleUrl(path: string) { + const source = ts.transpileModule(readFileSync(new URL(`../../${path}`, import.meta.url), "utf8"), { + compilerOptions: { module: ts.ModuleKind.ESNext, jsx: ts.JsxEmit.ReactJSX }, + }).outputText + const resolve = (specifier: string) => { + if (overrides[specifier]) return overrides[specifier] + const constructor = constructors[specifier] + if (constructor) { + return dataUrl( + `export class ${constructor} { constructor(...args) { return new ${mocks}.${constructor}(...args) } }`, + ) + } + if (mockExports[specifier]) { + return dataUrl( + mockExports[specifier] + .map((name) => `export const ${name} = (...args) => ${mocks}.${name}(...args)`) + .join("\n"), + ) + } + return specifier.startsWith("@/") ? specifier : import.meta.resolve(specifier) + } + return dataUrl( + source + .replace(/from "([^"]+)"/g, (_, specifier: string) => `from "${resolve(specifier)}"`) + .replace(/import\("([^"]+)"\)/g, (_, specifier: string) => `import("${resolve(specifier)}")`), + ) +} + +overrides["@/hooks/use-local-storage"] = moduleUrl("hooks/use-local-storage.ts") +overrides["@/contexts/auth-context"] = moduleUrl("contexts/auth-context.tsx") +overrides["@/contexts/api-context"] = moduleUrl("contexts/api-context.tsx") +overrides["@/contexts/s3-context"] = moduleUrl("contexts/s3-context.tsx") +const { AuthProvider, useAuth } = (await import( + overrides["@/contexts/auth-context"] +)) as typeof import("../../contexts/auth-context") +const { ApiProvider } = (await import( + overrides["@/contexts/api-context"] +)) as typeof import("../../contexts/api-context") +const { S3Provider } = (await import(overrides["@/contexts/s3-context"])) as typeof import("../../contexts/s3-context") +const { DashboardAuthGuard } = (await import( + moduleUrl("components/dashboard-auth-guard.tsx") +)) as typeof import("../../components/dashboard-auth-guard") + +type Auth = ReturnType +type Middleware = (next: (args: unknown) => Promise) => (args: unknown) => Promise +const config = { + serverHost: "https://s3.example.test", + api: { baseURL: "https://s3.example.test/rustfs/admin/v3" }, + s3: { endpoint: "https://s3.example.test", region: "us-east-1" }, +} +const logoutUrl = `${config.serverHost}/rustfs/admin/v3/oidc/logout?logout_token=test-logout-token` +const credentials = { + AccessKeyId: "test-access", + SecretAccessKey: "test-secret", + SessionToken: "test-session", + Expiration: "2099-01-01T00:00:00.000Z", +} + +async function mountDashboard(t: TestContext, options: { token?: string | null } = {}) { + const browser = new Window({ url: "https://console.example.test/browser/" }) + const globals = { + window: browser, + document: browser.document, + localStorage: browser.localStorage, + IS_REACT_ACT_ENVIRONMENT: true, + } + const previous = Object.keys(globals).map((key) => [key, Object.getOwnPropertyDescriptor(globalThis, key)] as const) + for (const [key, value] of Object.entries(globals)) { + Object.defineProperty(globalThis, key, { configurable: true, writable: true, value }) + } + const navigate = t.mock.fn<(url: string) => void>() + const replaceDocument = t.mock.fn<(url: string) => void>() + Object.defineProperty(browser, "location", { + configurable: true, + value: { + pathname: "/browser/", + origin: "https://console.example.test", + set href(url: string) { + navigate(url) + }, + replace: replaceDocument, + }, + }) + browser.localStorage.setItem("auth.credentials", JSON.stringify(credentials)) + browser.localStorage.setItem("auth.isAdmin", "true") + const token = options.token === undefined ? "test-logout-token" : options.token + if (token !== null) browser.localStorage.setItem("auth.oidcSession", JSON.stringify({ logoutToken: token })) + + const router = { replace: t.mock.fn<(path: string) => void>(), push: t.mock.fn<(path: string) => void>() } + const configManager = { loadConfig: t.mock.fn(async () => config) } + const apiHandlers: ApiErrorHandler[] = [] + const s3Handlers: Middleware[] = [] + let routeAllowed = true + Reflect.set(globalThis, mocksKey, { + configManager, + useRouter: () => router, + usePathname: () => "/browser/", + useTranslation: () => ({ t: (key: string) => key }), + usePermissions: () => ({ + isAdmin: false, + hasFetchedPolicy: true, + hasResolvedAdmin: true, + canAccessPath: () => routeAllowed, + }), + Spinner: () => null, + Button: ({ children, render }: { children: ReactNode; render?: ReactElement<{ children?: ReactNode }> }) => + render ? cloneElement(render, {}, children) : createElement("button", null, children), + fetchMfaChallenge: async () => ({ required: false }), + getStsToken: async () => ({ ...credentials, Expiration: new Date(credentials.Expiration) }), + addApiPrefixMiddleware: () => {}, + addDeleteReplicationMiddleware: () => {}, + AwsClient: class {}, + ApiClient: class { + constructor(_client: unknown, options: { errorHandler: ApiErrorHandler }) { + apiHandlers.push(options.errorHandler) + } + }, + S3Client: class { + config = {} + middlewareStack = { + addRelativeTo() {}, + add(middleware: Middleware) { + s3Handlers.push(middleware) + }, + } + }, + }) + let auth: Auth + function Probe() { + auth = useAuth() + return null + } + const container = browser.document.createElement("div") + browser.document.body.appendChild(container) + const root = createRoot(container as unknown as Element) + const render = async () => + act(async () => + root.render( + createElement( + AuthProvider, + null, + createElement(Probe), + createElement( + ApiProvider, + null, + createElement( + S3Provider, + null, + createElement(DashboardAuthGuard, null, createElement("main", null, "Dashboard")), + ), + ), + ), + ), + ) + t.after(async () => { + await act(async () => root.unmount()) + await browser.happyDOM.close() + Reflect.deleteProperty(globalThis, mocksKey) + for (const [key, descriptor] of previous) { + if (descriptor) Object.defineProperty(globalThis, key, descriptor) + else Reflect.deleteProperty(globalThis, key) + } + }) + await render() + assert.ok(container.querySelector("main"), "starts with the authenticated dashboard mounted") + assert.equal(apiHandlers.length, 1) + assert.equal(s3Handlers.length, 1) + return { + browser, + container, + router, + configManager, + navigate, + replaceDocument, + // Keep the callbacks from clients created BEFORE logout so stale closures are exercised. + apiHandler: apiHandlers[0], + s3Error: (error: unknown) => + s3Handlers[0](async () => { + throw error + })({}), + get auth() { + return auth! + }, + async logout() { + let redirected = false + await act(async () => { + redirected = await auth.logoutWithOidcRedirect() + }) + return redirected + }, + async denyRoute() { + routeAllowed = false + await render() + }, + } +} + +async function pendingLogout(mounted: Awaited>) { + const pending = Promise.withResolvers() + mounted.configManager.loadConfig.mock.mockImplementation(() => pending.promise) + let task: Promise + await act(async () => { + task = mounted.auth.logoutWithOidcRedirect() + }) + return { + async finish() { + await act(async () => { + pending.resolve(config) + await task + }) + }, + } +} + +test("OIDC logout clears local credentials and navigates only to the federated logout endpoint", async (t) => { + const mounted = await mountDashboard(t) + assert.equal(await mounted.logout(), true) + assert.deepEqual( + mounted.navigate.mock.calls.map((call) => call.arguments), + [[logoutUrl]], + ) + assert.equal(mounted.router.replace.mock.callCount(), 0) + assert.equal(mounted.auth.isAuthenticated, false) + for (const key of ["auth.credentials", "auth.isAdmin", "auth.oidcSession"]) + assert.equal(mounted.browser.localStorage.getItem(key), null) + assert.equal(mounted.container.querySelector("main"), null) + assert.equal(mounted.container.querySelector("a")?.getAttribute("href"), "/auth/login") +}) + +test("repeated logout and the local logout shortcut cannot replace an in-flight federated logout", async (t) => { + const mounted = await mountDashboard(t) + const pending = await pendingLogout(mounted) + assert.equal(mounted.auth.getIsLoggingOut(), true) + await act(async () => { + assert.equal(await mounted.auth.logoutWithOidcRedirect(), true) + mounted.auth.logoutAndRedirect() + }) + assert.equal(mounted.configManager.loadConfig.mock.callCount(), 3, "two initial clients and one logout config load") + assert.equal(mounted.navigate.mock.callCount(), 0) + assert.equal(mounted.router.replace.mock.callCount(), 0) + await pending.finish() + assert.deepEqual( + mounted.navigate.mock.calls.map((call) => call.arguments), + [[logoutUrl]], + ) + assert.equal(mounted.router.replace.mock.callCount(), 0) +}) + +for (const token of [null, " "]) { + test(`logout with ${token === null ? "no" : "an empty"} OIDC token keeps the local login fallback`, async (t) => { + const mounted = await mountDashboard(t, { token }) + assert.equal(await mounted.logout(), false) + assert.equal(mounted.auth.isAuthenticated, false) + assert.equal(mounted.navigate.mock.callCount(), 0) + assert.deepEqual( + mounted.router.replace.mock.calls.map((call) => call.arguments), + [["/auth/login/?unauthorized=true"]], + ) + }) +} + +for (const failure of ["config", "navigation"]) { + test(`${failure} failure restores normal logout and reports the error`, async (t) => { + const mounted = await mountDashboard(t) + const error = new Error(`${failure} unavailable`) + const errors = t.mock.method(console, "error", t.mock.fn()) + if (failure === "config") + mounted.configManager.loadConfig.mock.mockImplementation(async () => { + throw error + }) + else + mounted.navigate.mock.mockImplementation(() => { + throw error + }) + assert.equal(await mounted.logout(), false) + assert.equal(mounted.auth.getIsLoggingOut(), false) + assert.equal(mounted.auth.isAuthenticated, false) + assert.deepEqual( + mounted.router.replace.mock.calls.map((call) => call.arguments), + [["/auth/login/?unauthorized=true"]], + ) + assert.equal(errors.mock.callCount(), 1) + assert.ok(errors.mock.calls[0].arguments.includes(error)) + }) +} + +test("permission changes cannot redirect to forbidden while logout config is pending", async (t) => { + const mounted = await mountDashboard(t) + const pending = await pendingLogout(mounted) + await mounted.denyRoute() + assert.equal(mounted.router.replace.mock.callCount(), 0) + await pending.finish() + assert.equal(mounted.router.replace.mock.callCount(), 0) +}) + +const apiFailures = [ + { status: 401, path: "/buckets", target: "/auth/login/" }, + { status: 403, path: "/accountinfo", target: "/auth/login/" }, + { status: 403, path: "/buckets", target: "/403/" }, +] +const s3Failures = [ + { $metadata: { httpStatusCode: 401 } }, + { $metadata: { httpStatusCode: 403 }, Code: "UnauthorizedAccess" }, + { $metadata: { httpStatusCode: 403 }, Code: "InvalidAccessKeyId" }, +] + +for (const phase of ["config pending", "navigation started"]) { + test(`old API and S3 error callbacks cannot navigate during logout: ${phase}`, async (t) => { + const mounted = await mountDashboard(t) + const pending = await pendingLogout(mounted) + if (phase === "navigation started") await pending.finish() + await act(async () => { + for (const { status, path } of apiFailures) await mounted.apiHandler.handleByStatus(status, path) + for (const error of s3Failures) { + assert.deepEqual(await mounted.s3Error(error), { response: { statusCode: 401, headers: {} } }) + } + }) + assert.equal(mounted.router.replace.mock.callCount(), 0) + if (phase === "config pending") { + assert.equal(mounted.auth.isAuthenticated, true) + await pending.finish() + } + assert.deepEqual( + mounted.navigate.mock.calls.map((call) => call.arguments), + [[logoutUrl]], + ) + }) +} + +for (const { status, path, target } of apiFailures) { + test(`API ${status} at ${path} still redirects outside federated logout`, async (t) => { + const mounted = await mountDashboard(t) + await act(async () => mounted.apiHandler.handleByStatus(status, path)) + assert.equal(mounted.router.replace.mock.calls[0].arguments[0], target) + assert.equal(mounted.auth.isAuthenticated, target === "/403/") + }) +} + +for (const error of s3Failures) { + test(`S3 ${error.Code ?? "401"} still logs out outside federated logout`, async (t) => { + const mounted = await mountDashboard(t) + await act(async () => mounted.s3Error(error)) + assert.equal(mounted.router.replace.mock.calls[0].arguments[0], "/auth/login/") + assert.equal(mounted.auth.isAuthenticated, false) + }) +} + +test("unrelated S3 errors remain rejected during federated logout", async (t) => { + const mounted = await mountDashboard(t) + await mounted.logout() + const error = Object.assign(new Error("Denied"), { $metadata: { httpStatusCode: 403 }, Code: "AccessDenied" }) + await assert.rejects(mounted.s3Error(error), (caught) => caught === error) +}) + +for (const method of ["sts", "password"]) { + test(`a new ${method} login resets the logout flag and restores ordinary expiration handling`, async (t) => { + const mounted = await mountDashboard(t) + await mounted.logout() + await act(async () => { + if (method === "sts") await mounted.auth.loginWithStsCredentials(credentials) + else await mounted.auth.login({ accessKeyId: "test-access", secretAccessKey: "test-secret" }) + }) + assert.equal(mounted.auth.getIsLoggingOut(), false) + assert.equal(mounted.auth.isAuthenticated, true) + await act(async () => mounted.auth.logout()) + assert.deepEqual( + mounted.router.replace.mock.calls.map((call) => call.arguments), + [["/auth/login/?unauthorized=true"]], + ) + }) +} + +test("restoring an old logged-out document preserves a newer session and reloads login", async (t) => { + const mounted = await mountDashboard(t) + await mounted.logout() + const newSession = { + "auth.credentials": JSON.stringify({ ...credentials, AccessKeyId: "new-session" }), + "auth.isAdmin": "false", + "auth.oidcSession": JSON.stringify({ logoutToken: "new-logout-token" }), + } + for (const [key, value] of Object.entries(newSession)) mounted.browser.localStorage.setItem(key, value) + const event = new mounted.browser.Event("pageshow") + Object.defineProperty(event, "persisted", { value: true }) + await act(async () => mounted.browser.dispatchEvent(event)) + assert.deepEqual( + Object.fromEntries(Object.keys(newSession).map((key) => [key, mounted.browser.localStorage.getItem(key)])), + newSession, + ) + assert.deepEqual( + mounted.replaceDocument.mock.calls.map((call) => call.arguments), + [["/auth/login"]], + ) + assert.equal(mounted.router.replace.mock.callCount(), 0) +}) diff --git a/tests/hooks/use-permissions-refresh.test.ts b/tests/hooks/use-permissions-refresh.test.ts index da2dcc41..7a03f0f8 100644 --- a/tests/hooks/use-permissions-refresh.test.ts +++ b/tests/hooks/use-permissions-refresh.test.ts @@ -113,6 +113,7 @@ async function mountBrowser(t: TestContext, options: { deniedPolicyStatus?: bool const setIsAdmin = (value: boolean) => { isAdmin = value } + const getIsLoggingOut = () => false const router = { replace: t.mock.fn(), push: t.mock.fn() } const listBuckets = t.mock.fn<(options?: { force?: boolean }) => Promise>(async () => ({ Buckets: [{ Name: "test-bucket" }], @@ -127,7 +128,7 @@ async function mountBrowser(t: TestContext, options: { deniedPolicyStatus?: bool .flat() .map((name) => [name, () => null]), ), - useAuth: () => ({ isAuthenticated: !!credentials, credentials, isAdmin, setIsAdmin }), + useAuth: () => ({ isAuthenticated: !!credentials, credentials, isAdmin, setIsAdmin, getIsLoggingOut }), useApiOptional: () => api, useApiReady: () => ({ isReady: apiReady }), useS3Ready: () => ({ isReady: s3Ready }), diff --git a/tests/lib/oidc-logout-source.test.js b/tests/lib/oidc-logout-source.test.js deleted file mode 100644 index f1859eec..00000000 --- a/tests/lib/oidc-logout-source.test.js +++ /dev/null @@ -1,16 +0,0 @@ -import test from "node:test" -import assert from "node:assert/strict" -import fs from "node:fs" - -const source = fs.readFileSync("contexts/auth-context.tsx", "utf8") - -test("OIDC federated logout builds its destination before clearing auth state", () => { - assert.match( - source, - /const logoutUrl = buildOidcLogoutUrl\(config\.serverHost, oidcSession\.logoutToken\)\s*\n\s*logout\(\)\s*\n\s*window\.location\.href = logoutUrl/, - ) -}) - -test("OIDC federated logout does not clear auth state before awaiting config", () => { - assert.doesNotMatch(source, /logout\(\)\s*\n\s*if \(!oidcSession\) return false/) -}) From 4f9acea673c02cac9df5220cfd7c7bb1d753bf6c Mon Sep 17 00:00:00 2001 From: cxymds Date: Tue, 10 Nov 2026 11:13:47 +0800 Subject: [PATCH 2/2] fix(auth): reset logout protection after cross-tab login Reset logout protection when storage restores an authenticated session before consumer effects handle authentication errors. Keep pending logout protected against repeated credentials events and cover the cross-tab recovery behavior. --- contexts/auth-context.tsx | 16 ++++- tests/hooks/use-oidc-logout.test.ts | 102 +++++++++++++++++++++++++++- 2 files changed, 114 insertions(+), 4 deletions(-) diff --git a/contexts/auth-context.tsx b/contexts/auth-context.tsx index 3aae7600..01cc2d6c 100644 --- a/contexts/auth-context.tsx +++ b/contexts/auth-context.tsx @@ -1,6 +1,15 @@ "use client" -import { createContext, useCallback, useContext, useEffect, useMemo, useRef, type ReactNode } from "react" +import { + createContext, + useCallback, + useContext, + useEffect, + useLayoutEffect, + useMemo, + useRef, + type ReactNode, +} from "react" import type { AwsCredentialIdentity, AwsCredentialIdentityProvider } from "@aws-sdk/types" import type { SiteConfig } from "@/types/config" import { getLoginRoute } from "@/lib/routes" @@ -270,6 +279,11 @@ export function AuthProvider({ children }: { children: ReactNode }) { const credentials = getCredentials() const isAuthenticated = isValidCredentials(store) + useLayoutEffect(() => { + // Cross-tab login bypasses setCredentials; reset before consumer effects run. + if (isAuthenticated) loggingOutRef.current = false + }, [isAuthenticated]) + const value = useMemo( () => ({ login, diff --git a/tests/hooks/use-oidc-logout.test.ts b/tests/hooks/use-oidc-logout.test.ts index 84fc7d8d..2a4f530e 100644 --- a/tests/hooks/use-oidc-logout.test.ts +++ b/tests/hooks/use-oidc-logout.test.ts @@ -1,7 +1,7 @@ import test, { type TestContext } from "node:test" import assert from "node:assert/strict" import { readFileSync } from "node:fs" -import { act, cloneElement, createElement, type ReactElement, type ReactNode } from "react" +import { act, cloneElement, createElement, useEffect, type ReactElement, type ReactNode } from "react" import { createRoot } from "react-dom/client" import { Window } from "happy-dom" import ts from "typescript" @@ -90,7 +90,10 @@ const credentials = { Expiration: "2099-01-01T00:00:00.000Z", } -async function mountDashboard(t: TestContext, options: { token?: string | null } = {}) { +async function mountDashboard( + t: TestContext, + options: { token?: string | null; onCredentials?: (auth: Auth) => void } = {}, +) { const browser = new Window({ url: "https://console.example.test/browser/" }) const globals = { window: browser, @@ -161,7 +164,11 @@ async function mountDashboard(t: TestContext, options: { token?: string | null } }) let auth: Auth function Probe() { - auth = useAuth() + const value = useAuth() + auth = value + useEffect(() => { + if (value.isAuthenticated) options.onCredentials?.(value) + }, [value]) return null } const container = browser.document.createElement("div") @@ -226,6 +233,20 @@ async function mountDashboard(t: TestContext, options: { token?: string | null } routeAllowed = false await render() }, + async syncStorage(key: string, newValue: string | null) { + await act(async () => { + const oldValue = browser.localStorage.getItem(key) + if (newValue === null) browser.localStorage.removeItem(key) + else browser.localStorage.setItem(key, newValue) + browser.dispatchEvent( + new browser.StorageEvent("storage", { + key, + oldValue: oldValue ?? undefined, + newValue: newValue ?? undefined, + }), + ) + }) + }, } } @@ -404,6 +425,81 @@ for (const method of ["sts", "password"]) { }) } +test("a session received from another tab can log out with its own OIDC token", async (t) => { + const mounted = await mountDashboard(t) + await mounted.logout() + await mounted.syncStorage("auth.credentials", JSON.stringify({ ...credentials, AccessKeyId: "other-tab" })) + await mounted.syncStorage("auth.oidcSession", JSON.stringify({ logoutToken: "other-tab-token" })) + assert.equal(mounted.auth.isAuthenticated, true) + assert.equal(await mounted.logout(), true) + assert.deepEqual( + mounted.navigate.mock.calls.map((call) => call.arguments), + [[logoutUrl], [`${config.serverHost}/rustfs/admin/v3/oidc/logout?logout_token=other-tab-token`]], + ) + assert.equal(mounted.auth.isAuthenticated, false) + assert.equal(mounted.browser.localStorage.getItem("auth.credentials"), null) + assert.equal(mounted.browser.localStorage.getItem("auth.oidcSession"), null) + assert.equal(mounted.router.replace.mock.callCount(), 0) +}) + +for (const service of ["API", "S3"]) { + test(`a cross-tab session restores ${service} authentication handling before child effects run`, async (t) => { + let failure: Promise | undefined + const mounted = await mountDashboard(t, { + onCredentials: (auth) => { + if (auth.credentials?.AccessKeyId !== "other-tab") return + failure = + service === "API" + ? mounted.apiHandler.handle401() + : mounted.s3Error({ $metadata: { httpStatusCode: 403 }, Code: "UnauthorizedAccess" }) + }, + }) + await mounted.logout() + await mounted.syncStorage("auth.credentials", JSON.stringify({ ...credentials, AccessKeyId: "other-tab" })) + await act(async () => failure) + assert.ok(failure, "the new authenticated session runs its child effect") + assert.equal(mounted.router.replace.mock.calls[0]?.arguments[0], "/auth/login/") + assert.equal(mounted.auth.isAuthenticated, false) + }) +} + +test("deleted, invalid and unrelated storage values preserve an in-flight logout", async (t) => { + const mounted = await mountDashboard(t) + await mounted.logout() + const updates: [string, string | null][] = [ + ["auth.credentials", null], + ["auth.credentials", JSON.stringify({ AccessKeyId: "incomplete" })], + ["auth.credentials", JSON.stringify({ ...credentials, Expiration: "2000-01-01T00:00:00.000Z" })], + ["auth.oidcSession", JSON.stringify({ logoutToken: "unrelated-token" })], + ["theme", "dark"], + ] + for (const [key, value] of updates) { + await mounted.syncStorage(key, value) + await act(async () => mounted.apiHandler.handle401()) + assert.equal(mounted.auth.getIsLoggingOut(), true, `keeps logout protection for ${key}: ${value}`) + assert.equal(mounted.router.replace.mock.callCount(), 0) + } + assert.deepEqual( + mounted.navigate.mock.calls.map((call) => call.arguments), + [[logoutUrl]], + ) +}) + +test("a repeated current-credentials storage event cannot interrupt logout config loading", async (t) => { + const mounted = await mountDashboard(t) + const pending = await pendingLogout(mounted) + await mounted.syncStorage("auth.credentials", JSON.stringify(credentials)) + await act(async () => mounted.apiHandler.handle401()) + assert.equal(mounted.auth.getIsLoggingOut(), true) + assert.equal(mounted.router.replace.mock.callCount(), 0) + await pending.finish() + assert.deepEqual( + mounted.navigate.mock.calls.map((call) => call.arguments), + [[logoutUrl]], + ) + assert.equal(mounted.router.replace.mock.callCount(), 0) +}) + test("restoring an old logged-out document preserves a newer session and reloads login", async (t) => { const mounted = await mountDashboard(t) await mounted.logout()