Compare commits

...
23 changed files with 545 additions and 83 deletions
@@ -20,7 +20,7 @@ const SIZE = "1.125rem";
export function QuickNavRailHost() {
const { t } = useTranslation();
const navigate = useNavigate();
const { pathname, search } = useLocation();
const { pathname } = useLocation();
const host = useQuickNavHost();
const appMounted = Boolean(host?.appMounted);
@@ -74,7 +74,7 @@ export function QuickNavRailHost() {
returnHome();
return;
}
saveEditorReturnPath(pathname + search);
saveEditorReturnPath();
go(PORTAL_BASENAME);
},
},
@@ -29,6 +29,8 @@ import {
isBaseWorkbench,
} from "@app/types/workbench";
import { useNavigationUrlSync } from "@app/hooks/useUrlSync";
import { stripBasePath } from "@app/constants/app";
import { EDITOR_BASENAME } from "@app/routes/editorBasename";
import { filterToolRegistryByQuery } from "@app/utils/toolSearch";
import { useToolHistory } from "@app/hooks/tools/useUserToolActivity";
import {
@@ -373,15 +375,28 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) {
// This runs once to navigate to the user's preferred tab (read/automate)
// instead of always starting on the tools tab.
const hasAppliedStartupView = React.useRef(false);
// Set when the startup view picks the tool, so the URL sync knows this
// selection came from a preference and must not be written to the address.
const startupSelectedToolRef = React.useRef<ToolId | null>(null);
useEffect(() => {
if (hasAppliedStartupView.current) return;
// The URL wins: the startup view decides what you see when you arrive at the
// editor's home, never what a deep link to a tool shows. Without this, a
// "Reader" preference rewrote every /<tool> link to /read.
const path = stripBasePath(window.location.pathname);
if (path !== "/" && path !== EDITOR_BASENAME) {
hasAppliedStartupView.current = true;
return;
}
const startupView = preferences.defaultStartupView;
if (startupView === "read") {
hasAppliedStartupView.current = true;
startupSelectedToolRef.current = "read";
setReaderMode(true);
actions.setSelectedTool("read");
} else if (startupView === "automate") {
hasAppliedStartupView.current = true;
startupSelectedToolRef.current = "automate";
actions.setSelectedTool("automate");
setLeftPanelView("toolContent");
}
@@ -573,6 +588,7 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) {
handleBackToTools,
allTools,
true,
startupSelectedToolRef,
);
// Ref-backed wrappers so callback identities stay stable across renders.
@@ -0,0 +1,80 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { renderHook } from "@testing-library/react";
import { useRef } from "react";
import type { ToolId } from "@app/types/toolId";
const h = vi.hoisted(() => ({
updateToolRoute: vi.fn(),
clearToolRoute: vi.fn(),
}));
vi.mock("@app/utils/urlRouting", () => ({
parseToolRoute: () => ({ workbench: "fileEditor", toolId: null }),
updateToolRoute: h.updateToolRoute,
clearToolRoute: h.clearToolRoute,
}));
vi.mock("@app/utils/scarfTracking", () => ({ firePixel: vi.fn() }));
vi.mock("@app/contexts/AppConfigContext", () => ({
useAppConfig: () => ({ config: { premiumEnabled: true } }),
}));
import { useNavigationUrlSync } from "@app/hooks/useUrlSync";
const registry = {
read: { name: "Read", workbench: "viewer" },
compress: { name: "Compress", workbench: "fileEditor" },
} as never;
/** Drives the hook the way ToolWorkflowContext does, with a startup marker. */
function useHarness(selectedTool: ToolId | null, startupTool: ToolId | null) {
const ref = useRef<ToolId | null>(startupTool);
useNavigationUrlSync(selectedTool, vi.fn(), vi.fn(), registry, true, ref);
return ref;
}
describe("useNavigationUrlSync — startup-view selections", () => {
beforeEach(() => h.updateToolRoute.mockClear());
// The default-startup-view preference selects a tool to change the *view*.
// Writing it to the address turned every visit to /editor into /read.
it("never writes the URL for the startup-applied tool", () => {
const { rerender } = renderHook(
({ tool }: { tool: ToolId | null }) => useHarness(tool, "read" as ToolId),
{ initialProps: { tool: null as ToolId | null } },
);
rerender({ tool: "read" as ToolId });
expect(h.updateToolRoute).not.toHaveBeenCalled();
});
// The effect re-runs whenever the registry identity changes, so a marker that
// was consumed on first sight let the second run write /read anyway.
it("survives a re-run for the same tool", () => {
const { rerender } = renderHook(
({ tool }: { tool: ToolId | null }) => useHarness(tool, "read" as ToolId),
{ initialProps: { tool: null as ToolId | null } },
);
rerender({ tool: "read" as ToolId });
rerender({ tool: "read" as ToolId });
rerender({ tool: "read" as ToolId });
expect(h.updateToolRoute).not.toHaveBeenCalled();
});
it("still writes the URL when the user picks a different tool", () => {
const { rerender } = renderHook(
({ tool }: { tool: ToolId | null }) => useHarness(tool, "read" as ToolId),
{ initialProps: { tool: null as ToolId | null } },
);
rerender({ tool: "read" as ToolId });
rerender({ tool: "compress" as ToolId });
expect(h.updateToolRoute).toHaveBeenCalledWith("compress", registry, false);
});
it("writes the URL for a tool chosen without a startup marker", () => {
const { rerender } = renderHook(
({ tool }: { tool: ToolId | null }) => useHarness(tool, null),
{ initialProps: { tool: null as ToolId | null } },
);
rerender({ tool: "read" as ToolId });
expect(h.updateToolRoute).toHaveBeenCalledWith("read", registry, false);
});
});
+27 -3
View File
@@ -2,7 +2,7 @@
* URL synchronization hooks for tool routing with registry support
*/
import { useEffect, useCallback, useRef } from "react";
import { useEffect, useCallback, useRef, type MutableRefObject } from "react";
import { ToolId } from "@app/types/toolId";
import {
parseToolRoute,
@@ -24,6 +24,11 @@ export function useNavigationUrlSync(
clearToolSelection: () => void,
registry: ToolRegistry,
enableSync: boolean = true,
/**
* Tool the default-startup-view preference selected, if any. That selection
* sets the view, not the address, so it must not be written to the URL.
*/
startupSelectedToolRef?: MutableRefObject<ToolId | null>,
) {
const { config } = useAppConfig();
const premiumEnabled = config?.premiumEnabled;
@@ -77,8 +82,16 @@ export function useNavigationUrlSync(
useEffect(() => {
if (!enableSync) return;
const startupTool = startupSelectedToolRef?.current ?? null;
if (selectedTool) {
updateToolRoute(selectedTool, registry, false); // Use pushState for user navigation
// A startup-view selection is a view preference, not a navigation: writing
// it here rewrote /editor to /read on every load. The effect re-runs
// whenever the registry identity changes, so the marker has to survive
// until the selection actually moves off it (cleared below).
if (startupTool !== selectedTool) {
updateToolRoute(selectedTool, registry, false); // Use pushState for user navigation
}
} else if (prevSelectedTool.current !== null) {
// Only clear URL if we had a tool before (user navigated away)
// Don't clear on initial load when both current and previous are null
@@ -88,8 +101,19 @@ export function useNavigationUrlSync(
}
}
// Spent once the user leaves the startup-applied tool, so re-picking it
// later is a real navigation and does update the URL.
if (
startupSelectedToolRef &&
startupTool !== null &&
prevSelectedTool.current === startupTool &&
selectedTool !== startupTool
) {
startupSelectedToolRef.current = null;
}
prevSelectedTool.current = selectedTool;
}, [selectedTool, registry, enableSync]);
}, [selectedTool, registry, enableSync, startupSelectedToolRef]);
// Handle browser back/forward navigation
useEffect(() => {
@@ -0,0 +1,76 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
vi.mock("@app/components/toast", () => ({ alert: vi.fn() }));
vi.mock("@app/services/specialErrorToasts", () => ({
showSpecialErrorToast: vi.fn(() => false),
}));
vi.mock("@app/services/saasErrorInterceptor", () => ({
handleSaaSError: vi.fn(() => false),
}));
vi.mock("@app/services/errorUtils", () => ({
broadcastErroredFiles: vi.fn(),
extractErrorFileIds: vi.fn(() => []),
normalizeAxiosErrorData: vi.fn(async (d: unknown) => d),
}));
const hrefs: string[] = [];
/** Serve the app from `base`, sitting on `pathname`, then load the handler fresh. */
async function loadAt(base: string, pathname: string) {
document.head.innerHTML = `<base href="${base}" />`;
Object.defineProperty(window, "location", {
configurable: true,
value: {
pathname,
search: "",
origin: "http://localhost:3000",
get href() {
// Absolute: jsdom resolves <base href> against this.
return "http://localhost:3000" + pathname;
},
set href(v: string) {
hrefs.push(v);
},
},
});
vi.resetModules();
return (await import("@app/services/httpErrorHandler")).handleHttpError;
}
const unauthorized = {
isAxiosError: true,
message: "unauthorized",
config: {},
response: { status: 401, data: {} },
};
beforeEach(() => {
hrefs.length = 0;
sessionStorage.clear();
localStorage.clear();
});
afterEach(() => vi.resetModules());
describe("401 return path is router-relative", () => {
// Login replays this through navigate(), which re-applies the router
// basename. Carrying /app here produced /app/app/compress.
it("strips the base path on a subpath deploy", async () => {
const handle = await loadAt("/app/", "/app/compress");
await handle(unauthorized);
expect(sessionStorage.getItem("stirling_post_login_path")).toBe(
"/compress",
);
expect(hrefs[0]).toBe("/app/login?from=%2Fcompress");
});
it("is unchanged at the origin root", async () => {
const handle = await loadAt("/", "/compress");
await handle(unauthorized);
expect(sessionStorage.getItem("stirling_post_login_path")).toBe(
"/compress",
);
expect(hrefs[0]).toBe("/login?from=%2Fcompress");
});
});
@@ -12,7 +12,8 @@ import {
clampText,
extractAxiosErrorMessage,
} from "@app/services/httpErrorUtils";
import { withBasePath } from "@app/constants/app";
import { stripBasePath, withBasePath } from "@app/constants/app";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
// Module-scoped state to reduce global variable usage
const recentSpecialByEndpoint: Record<string, number> = {};
@@ -21,26 +22,9 @@ const SPECIAL_SUPPRESS_MS = 1500; // brief window to suppress generic duplicate
// Mirrors the key in proprietary/auth/springAuthClient.ts; AuthCallback consumes it.
const POST_LOGIN_REDIRECT_STORAGE_KEY = "stirling_post_login_path";
function isSafePostLoginPath(path: string): boolean {
if (
!path.startsWith("/") ||
path.startsWith("//") ||
path.startsWith("/\\")
) {
return false;
}
const lowered = path.toLowerCase();
return (
!lowered.startsWith("/login") &&
!lowered.startsWith("/auth/") &&
!lowered.startsWith("/oauth2") &&
!lowered.startsWith("/saml2")
);
}
function stashPostLoginRedirect(path: string): void {
try {
if (typeof window === "undefined" || !isSafePostLoginPath(path)) return;
if (typeof window === "undefined" || !isSafePostLoginRedirect(path)) return;
window.sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, path);
} catch {
// sessionStorage unavailable (private mode) — fail open
@@ -128,7 +112,11 @@ export async function handleHttpError(error: unknown): Promise<boolean> {
console.debug("[httpErrorHandler] 401 detected, redirecting to login");
// Spring 302-strips the ?from= query from /login, so stash the return
// path in sessionStorage (AuthCallback reads it after SSO round-trip).
const currentLocation = window.location.pathname + window.location.search;
// Router-relative, not browser-relative: every consumer replays this
// through navigate(), which re-applies the basename. Keeping the base
// path here yields /app/app/<tool> on a subpath deploy.
const currentLocation =
stripBasePath(window.location.pathname) + window.location.search;
stashPostLoginRedirect(currentLocation);
let hadStoredJwt = false;
try {
@@ -0,0 +1,39 @@
import { describe, expect, it } from "vitest";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
// Core default. Rejects off-origin forms and the auth routes every build has
// (/login, /auth/…); Spring SSO routes are the proprietary override's concern.
describe("isSafePostLoginRedirect (core base)", () => {
it("accepts same-origin router paths", () => {
expect(isSafePostLoginRedirect("/editor")).toBe(true);
expect(isSafePostLoginRedirect("/compress")).toBe(true);
expect(isSafePostLoginRedirect("/editor?foo=bar")).toBe(true);
expect(isSafePostLoginRedirect("/oauth/consent?x=1")).toBe(true);
expect(isSafePostLoginRedirect("/")).toBe(true);
});
it("rejects empty and non-string values", () => {
expect(isSafePostLoginRedirect(null)).toBe(false);
expect(isSafePostLoginRedirect(undefined)).toBe(false);
expect(isSafePostLoginRedirect("")).toBe(false);
expect(isSafePostLoginRedirect(42 as unknown)).toBe(false);
});
it("rejects off-origin and protocol-relative forms", () => {
expect(isSafePostLoginRedirect("//evil.example.com")).toBe(false);
expect(isSafePostLoginRedirect("/\\evil.example.com")).toBe(false);
expect(isSafePostLoginRedirect("https://evil.example.com")).toBe(false);
expect(isSafePostLoginRedirect("editor")).toBe(false);
});
it("rejects the universal auth routes so returning back can never loop", () => {
expect(isSafePostLoginRedirect("/login")).toBe(false);
expect(isSafePostLoginRedirect("/login?next=%2Feditor")).toBe(false);
expect(isSafePostLoginRedirect("/auth/callback")).toBe(false);
});
it("leaves the Spring SSO routes to the proprietary override", () => {
expect(isSafePostLoginRedirect("/oauth2/authorize")).toBe(true);
expect(isSafePostLoginRedirect("/saml2/login")).toBe(true);
});
});
@@ -0,0 +1,15 @@
/**
* Is `path` safe to send a user back to after they log in?
*/
export function isSafePostLoginRedirect(path: unknown): path is string {
if (typeof path !== "string" || path.length === 0) return false;
if (
!path.startsWith("/") ||
path.startsWith("//") ||
path.startsWith("/\\")
) {
return false;
}
const lowered = path.toLowerCase();
return !lowered.startsWith("/login") && !lowered.startsWith("/auth/");
}
@@ -55,10 +55,14 @@ describe("workbench session record", () => {
});
describe("editor return path", () => {
it("is consumed by the first take", () => {
saveEditorReturnPath("/compress?x=1");
it("captures the live address bar and is consumed by the first take", () => {
// The editor writes its tool route via raw history.pushState, so the save
// must read window.location, not a lagging router location.
window.history.pushState({}, "", "/compress?x=1");
saveEditorReturnPath();
expect(takeEditorReturnPath()).toBe("/compress?x=1");
expect(takeEditorReturnPath()).toBeNull();
window.history.pushState({}, "", "/");
});
});
@@ -2,6 +2,7 @@
// does not cost the user their workbench. sessionStorage on purpose: per-tab, tabs never clobber.
import type { StirlingFileStub } from "@app/types/fileContext";
import { stripBasePath } from "@app/constants/app";
const SESSION_KEY = "stirling.workbench.session";
/** Bumped when the record's shape or meaning changes, so an old one is discarded rather than
@@ -153,8 +154,10 @@ export function isSeedableView(
return view !== undefined && SEEDABLE_VIEWS.includes(view);
}
export function saveEditorReturnPath(path: string): void {
export function saveEditorReturnPath(): void {
try {
const path =
stripBasePath(window.location.pathname) + window.location.search;
sessionStorage.setItem(RETURN_PATH_KEY, path);
} catch {
// Best-effort: the switch back just lands on the editor root.
@@ -16,6 +16,7 @@ import { AxiosError, type AxiosRequestConfig } from "axios";
import { getSpringAuthConfig } from "@app/auth/config";
import { type OAuthProvider } from "@app/auth/spring/oauthTypes";
import { resetOAuthState } from "@app/auth/spring/oauthStorage";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
import type {
AuthUser as User,
AuthSession as Session,
@@ -100,23 +101,10 @@ function persistRedirectPath(path: string): void {
}
}
// Same-origin relative path, not pointing at auth plumbing. Rejects protocol-relative
// URLs to guard against open-redirect abuse if the stored value is tampered with.
export function isSafePostLoginRedirect(path: unknown): path is string {
if (typeof path !== "string" || path.length === 0) return false;
if (!path.startsWith("/") || path.startsWith("//")) return false;
if (path.startsWith("/\\")) return false;
const lowered = path.toLowerCase();
if (
lowered.startsWith("/login") ||
lowered.startsWith("/auth/") ||
lowered.startsWith("/oauth2") ||
lowered.startsWith("/saml2")
) {
return false;
}
return true;
}
// The safe-return-path rule lives in the shared @app/services/postLoginRedirect
// extension point (proprietary override adds the Spring SSO routes). Re-exported
// here so existing importers via @app/auth keep resolving it.
export { isSafePostLoginRedirect };
export function setPostLoginRedirectPath(
path: string | null | undefined,
@@ -16,6 +16,7 @@ import type {
import { getSupabaseClient } from "@app/auth/supabase/supabaseClient";
import { AuthContext } from "@app/auth/context";
import { isAdminRole } from "@app/auth/roles";
import { getApiBaseUrl } from "@app/services/apiClientConfig";
import {
defaultTranslate,
type AuthContextValue,
@@ -154,14 +155,23 @@ export function SupabaseAuthProvider({
return;
}
let cancelled = false;
// Same API base the rest of the app uses: SaaS serves the frontend and the
// API from different hosts, so a root-relative path never reaches /me.
const apiBase = (getApiBaseUrl() || "").replace(/\/+$/, "");
const meUrl = `${apiBase}/api/v1/auth/me`;
const loadAccess = () => {
void fetch("/api/v1/auth/me", {
void fetch(meUrl, {
headers: {
Authorization: `Bearer ${token}`,
Accept: "application/json",
},
})
.then((res) => (res.ok ? res.json() : null))
.then((res) => {
// Must throw, not resolve null: swallowing a non-ok leaves
// portalAccess undefined and hangs the portal gate on a spinner.
if (!res.ok) throw new Error(`auth/me responded ${res.status}`);
return res.json();
})
.then(
(
data: {
@@ -0,0 +1,128 @@
import { beforeEach, afterEach, describe, expect, it, vi } from "vitest";
import { render, waitFor } from "@testing-library/react";
import { useContext } from "react";
const h = vi.hoisted(() => ({ apiBase: "/" }));
vi.mock("@app/services/apiClientConfig", () => ({
getApiBaseUrl: () => h.apiBase,
}));
const sbSession = {
access_token: "supabase-token",
user: {
id: "u1",
email: "user@example.com",
is_anonymous: false,
app_metadata: {},
user_metadata: {},
},
};
vi.mock("@app/auth/supabase/supabaseClient", () => ({
getSupabaseClient: () => ({
auth: {
getSession: () => Promise.resolve({ data: { session: sbSession } }),
onAuthStateChange: () => ({
data: { subscription: { unsubscribe: () => {} } },
}),
refreshSession: () => Promise.resolve({ data: {}, error: null }),
signOut: () => Promise.resolve({ error: null }),
},
}),
}));
import { SupabaseAuthProvider } from "@app/auth/supabase/UseSession";
import { AuthContext } from "@app/auth/context";
function Probe() {
const v = useContext(AuthContext);
return (
<>
<span data-testid="access">{String(v?.portalAccess)}</span>
{/* Raw, un-defaulted value: this is what SaasPortalGate reads to decide
"access not known yet" vs "denied". undefined = spinner forever. */}
<span data-testid="raw">{String(v?.user?.portalAccess)}</span>
</>
);
}
const mount = () =>
render(
<SupabaseAuthProvider>
<Probe />
</SupabaseAuthProvider>,
);
let fetchMock: ReturnType<typeof vi.fn>;
beforeEach(() => {
fetchMock = vi.fn();
vi.stubGlobal("fetch", fetchMock);
});
afterEach(() => vi.unstubAllGlobals());
describe("supabase provider portalAccess lookup", () => {
// SaaS serves the frontend and the API from different hosts; a root-relative
// path silently missed /me, so a granted non-admin was denied the Processor.
it("calls /me on the configured API base, not the page origin", async () => {
h.apiBase = "https://api.example.com";
fetchMock.mockResolvedValue({
ok: true,
json: () => Promise.resolve({ user: { portalAccess: true } }),
});
mount();
await waitFor(() =>
expect(fetchMock).toHaveBeenCalledWith(
"https://api.example.com/api/v1/auth/me",
expect.anything(),
),
);
});
it("keeps a same-origin base as a single leading slash", async () => {
h.apiBase = "/";
fetchMock.mockResolvedValue({
ok: true,
json: () => Promise.resolve({ user: { portalAccess: true } }),
});
mount();
await waitFor(() =>
expect(fetchMock).toHaveBeenCalledWith(
"/api/v1/auth/me",
expect.anything(),
),
);
});
it("grants access when /me says so", async () => {
h.apiBase = "https://api.example.com";
fetchMock.mockResolvedValue({
ok: true,
json: () => Promise.resolve({ user: { portalAccess: true } }),
});
const { getByTestId } = mount();
await waitFor(() => expect(getByTestId("access").textContent).toBe("true"));
});
// A non-ok used to resolve to null and return early, leaving the raw
// portalAccess undefined forever - SaasPortalGate reads that as "still
// loading" and hangs on a spinner instead of falling back.
it("resolves the raw portalAccess when /me returns non-ok", async () => {
h.apiBase = "https://api.example.com";
fetchMock.mockResolvedValue({
ok: false,
status: 401,
json: () => Promise.resolve({}),
});
const { getByTestId } = mount();
await waitFor(() => expect(getByTestId("raw").textContent).toBe("false"));
});
it("leaves the raw portalAccess defined when the request rejects", async () => {
h.apiBase = "https://api.example.com";
fetchMock.mockRejectedValue(new Error("network down"));
const { getByTestId } = mount();
await waitFor(() => expect(getByTestId("raw").textContent).toBe("false"));
});
});
@@ -7,9 +7,10 @@ const mocks = vi.hoisted(() => ({
portalAccess: true,
}));
// The hook reads window.location for the return path (not useLocation), because
// the editor's raw history.pushState leaves react-router's location stale.
vi.mock("react-router-dom", () => ({
useNavigate: () => mocks.navigate,
useLocation: () => ({ pathname: "/compress", search: "?mode=fast" }),
}));
vi.mock("@app/auth/context", () => ({
useAuth: () => ({ portalAccess: mocks.portalAccess }),
@@ -27,6 +28,7 @@ beforeEach(() => {
sessionStorage.clear();
vi.clearAllMocks();
mocks.portalAccess = true;
window.history.pushState({}, "", "/");
});
describe("useOtherAppSwitch", () => {
@@ -45,6 +47,7 @@ describe("useOtherAppSwitch", () => {
});
it("records where to return to, then navigates to the processor", () => {
window.history.pushState({}, "", "/compress?mode=fast");
const { result } = renderHook(() => useOtherAppSwitch());
result.current?.onOpen();
mocks.requestNavigation.mock.calls[0][0]();
@@ -1,4 +1,4 @@
import { useLocation, useNavigate } from "react-router-dom";
import { useNavigate } from "react-router-dom";
import { useAuth } from "@app/auth/context";
import { useNavigationActions } from "@app/contexts/NavigationContext";
import { PORTAL_BASENAME } from "@app/routes/portalBasename";
@@ -12,7 +12,6 @@ import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFoote
export function useOtherAppSwitch(): NavFooterAppLink | null {
const { portalAccess } = useAuth();
const navigate = useNavigate();
const location = useLocation();
const { actions } = useNavigationActions();
if (!portalAccess) return null;
return {
@@ -20,7 +19,7 @@ export function useOtherAppSwitch(): NavFooterAppLink | null {
onOpen: () =>
// Through the guard, so unsaved edits get the same warning as any other navigation.
actions.requestNavigation(() => {
saveEditorReturnPath(location.pathname + location.search);
saveEditorReturnPath();
navigate(PORTAL_BASENAME);
}),
};
@@ -1,5 +1,5 @@
import { AxiosInstance, AxiosError, InternalAxiosRequestConfig } from "axios";
import { withBasePath } from "@app/constants/app";
import { stripBasePath, withBasePath } from "@app/constants/app";
import { getBrowserId } from "@app/utils/browserIdentifier";
import { setPostLoginRedirectPath } from "@app/auth/spring/springAuthClient";
@@ -94,8 +94,10 @@ async function refreshAuthToken(client: AxiosInstance): Promise<string> {
// Redirect to login
const loginPath = withBasePath("/login");
if (window.location.pathname !== loginPath) {
// Router-relative: Login replays this through navigate(), which applies
// the basename itself. See the same note in httpErrorHandler.
setPostLoginRedirectPath(
window.location.pathname + window.location.search,
stripBasePath(window.location.pathname) + window.location.search,
);
console.log("[API Client] Redirecting to login page...");
window.location.href = loginPath;
@@ -0,0 +1,25 @@
import { describe, expect, it } from "vitest";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
// Proprietary override: the core base plus the Spring SSO routes (/oauth2, /saml2).
describe("isSafePostLoginRedirect (proprietary override)", () => {
it("still accepts ordinary router paths", () => {
expect(isSafePostLoginRedirect("/editor")).toBe(true);
expect(isSafePostLoginRedirect("/share/abc123?x=1")).toBe(true);
expect(isSafePostLoginRedirect("/")).toBe(true);
});
it("inherits the base rejections", () => {
expect(isSafePostLoginRedirect("")).toBe(false);
expect(isSafePostLoginRedirect(null)).toBe(false);
expect(isSafePostLoginRedirect("//evil.example")).toBe(false);
expect(isSafePostLoginRedirect("/\\evil")).toBe(false);
expect(isSafePostLoginRedirect("/login")).toBe(false);
expect(isSafePostLoginRedirect("/auth/callback")).toBe(false);
});
it("also rejects the Spring SSO routes", () => {
expect(isSafePostLoginRedirect("/oauth2/authorization/google")).toBe(false);
expect(isSafePostLoginRedirect("/saml2/authenticate/x")).toBe(false);
});
});
@@ -0,0 +1,7 @@
import { isSafePostLoginRedirect as isSafeBaseRedirect } from "@core/services/postLoginRedirect";
export function isSafePostLoginRedirect(path: unknown): path is string {
if (!isSafeBaseRedirect(path)) return false;
const lowered = path.toLowerCase();
return !lowered.startsWith("/oauth2") && !lowered.startsWith("/saml2");
}
@@ -1,4 +1,4 @@
import { useLocation, useNavigate } from "react-router-dom";
import { useNavigate } from "react-router-dom";
import { usePortalAccess } from "@app/hooks/usePortalAccess";
import { useNavigationActions } from "@app/contexts/NavigationContext";
import { PORTAL_BASENAME } from "@app/routes/portalBasename";
@@ -13,7 +13,6 @@ import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFoote
export function useOtherAppSwitch(): NavFooterAppLink | null {
const portalAccess = usePortalAccess();
const navigate = useNavigate();
const location = useLocation();
const { actions } = useNavigationActions();
if (!portalAccess) return null;
return {
@@ -21,7 +20,7 @@ export function useOtherAppSwitch(): NavFooterAppLink | null {
onOpen: () =>
// Through the guard, so unsaved edits get the same warning as any other navigation.
actions.requestNavigation(() => {
saveEditorReturnPath(location.pathname + location.search);
saveEditorReturnPath();
navigate(PORTAL_BASENAME);
}),
};
@@ -5,6 +5,7 @@ import { supabase } from "@app/auth/supabase";
import { Button } from "@app/ui/Button";
import { withBasePath } from "@app/constants/app";
import { readPendingConnect } from "@app/routes/pendingConnect";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
import { AuthShell } from "@app/auth/ui/AuthShell";
import ErrorMessage from "@app/auth/ui/ErrorMessage";
import { Spinner } from "@app/ui/Spinner";
@@ -136,18 +137,16 @@ export default function AuthCallback() {
// else on the editor.
// Explicit `next` first, so a sign-in started for another reason is not
// hijacked by a remembered connect request.
const explicitNext = url.searchParams.get("next");
const explicitNext =
url.searchParams.get("next") ?? url.searchParams.get("from");
const pendingConnect = readPendingConnect();
const destination =
explicitNext &&
explicitNext.startsWith("/") &&
!explicitNext.startsWith("//")
? explicitNext
: pendingConnect
? `/link?request=${encodeURIComponent(pendingConnect)}`
: next.startsWith("/") && !next.startsWith("//")
? next
: await resolveLandingPath();
const destination = isSafePostLoginRedirect(explicitNext)
? explicitNext
: pendingConnect
? `/link?request=${encodeURIComponent(pendingConnect)}`
: isSafePostLoginRedirect(next)
? next
: await resolveLandingPath();
console.log("[Auth Callback Debug] Redirecting to:", destination);
setTimeout(() => navigate(destination, { replace: true }), 1500);
+7 -6
View File
@@ -14,6 +14,7 @@ import {
getBaseUrl,
withBasePath,
} from "@app/constants/app";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
import LinkRoundedIcon from "@mui/icons-material/LinkRounded";
// Import login components
@@ -48,14 +49,14 @@ export default function Login() {
}
}, []);
// Same-origin relative path to return to after login (e.g. the OAuth
// consent page). Same sanitization rules as AuthCallback's `next`.
// Same-origin router path to return to after login (e.g. the OAuth consent
// page, or the editor a 401 bounced the user off). `?next=` is what this app
// writes; `?from=` is what the shared core 401 handler writes.
const nextPath = useMemo(() => {
try {
const next = new URL(window.location.href).searchParams.get("next");
return next && next.startsWith("/") && !next.startsWith("//")
? next
: null;
const params = new URL(window.location.href).searchParams;
const candidate = params.get("next") ?? params.get("from");
return isSafePostLoginRedirect(candidate) ? candidate : null;
} catch (_) {
return null;
}
@@ -219,10 +219,11 @@ describe("apiClient", () => {
// Import apiClient after mocking
const { default: apiClient } = await import("@app/services/apiClient");
// Mock window.location for redirect test
// On /editor when the session dies: the return path must ride along so the
// login screen sends the user back here, not to the role-based landing.
Object.defineProperty(window, "location", {
writable: true,
value: { href: "" },
value: { href: "", pathname: "/editor", search: "" },
});
const mockAdapter = vi.fn((config) => {
@@ -245,8 +246,50 @@ describe("apiClient", () => {
} catch (_) {
// Verify refresh was attempted
expect(supabase.auth.refreshSession).toHaveBeenCalled();
// Verify redirect to login
expect(window.location.href).toBe("/login");
// Verify redirect to login carries the return path
expect(window.location.href).toBe("/login?next=%2Feditor");
}
});
it("does not redirect (or loop) when already on the login page", async () => {
expectConsole.error(/\[API Client\] Token refresh failed/);
const oldSession = { access_token: "old", user: { id: "user-123" } };
vi.mocked(supabase.auth.getSession).mockResolvedValue({
data: { session: oldSession as unknown as Session },
error: null,
});
vi.mocked(supabase.auth.refreshSession).mockResolvedValue({
data: { user: null, session: null },
error: {
name: "AuthError",
message: "Refresh failed",
status: 400,
code: "auth_error",
} as unknown as AuthError,
});
const { default: apiClient } = await import("@app/services/apiClient");
Object.defineProperty(window, "location", {
writable: true,
value: { href: "", pathname: "/login", search: "?next=%2Feditor" },
});
apiClient.defaults.adapter = vi.fn((config) =>
Promise.reject(
Object.assign(new Error("Unauthorized"), {
response: { status: 401, data: { error: "Unauthorized" } },
config,
}),
),
);
try {
await apiClient.get("/api/v1/test");
expect(true).toBe(false);
} catch (_) {
// Left untouched: no second redirect off the login page.
expect(window.location.href).toBe("");
}
});
});
+19 -6
View File
@@ -5,8 +5,9 @@ import {
classifyPaygError,
handlePaygError,
} from "@app/services/paygErrorInterceptor";
import { withBasePath } from "@app/constants/app";
import { stripBasePath, withBasePath } from "@app/constants/app";
import { getBrowserId } from "@app/utils/browserIdentifier";
import { isSafePostLoginRedirect } from "@app/services/postLoginRedirect";
// Helper: decode base64url JWT payload safely
function decodeJwtPayload(token: string): Record<string, unknown> | null {
@@ -113,6 +114,21 @@ function refreshSessionOnce(): ReturnType<typeof supabase.auth.refreshSession> {
return inFlightRefresh;
}
// Hard-redirect to /login, carrying where the user was so the login screen can
// return them there instead of falling through to the role-based landing (which
// sends processor users to the processor - the "refresh /editor bounces me to
// the processor" bug). Router-relative, matching what Login reads via `?next=`.
function redirectToLogin(): void {
const loginPath = withBasePath("/login");
// Already on the login page: another redirect would just loop.
if (window.location.pathname === loginPath) return;
const returnPath =
stripBasePath(window.location.pathname) + window.location.search;
window.location.href = isSafePostLoginRedirect(returnPath)
? `${loginPath}?next=${encodeURIComponent(returnPath)}`
: loginPath;
}
// Response interceptor for handling token refresh
apiClient.interceptors.response.use(
(response) => response,
@@ -173,7 +189,7 @@ apiClient.interceptors.response.use(
// The session genuinely can't be recovered. Send protected requests
// to login; public ones just fail quietly (no redirect).
if (!isPublicEndpoint) {
window.location.href = withBasePath("/login");
redirectToLogin();
}
return Promise.reject(error);
@@ -194,10 +210,7 @@ apiClient.interceptors.response.use(
console.debug(
"[API Client] No session to refresh, 401 on protected endpoint",
);
const loginPath = withBasePath("/login");
if (window.location.pathname !== loginPath) {
window.location.href = loginPath;
}
redirectToLogin();
return Promise.reject(error);
}
} catch (refreshError) {