mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-02 21:03:34 +03:00
Fix redirect bugs in SaaS (#7721)
# Description of Changes Fixes various bugs that affected SaaS (and some self-hosted): - Refreshing on Editor caused the user to be redirected to Processor - User was unable to access Processor in SaaS - Deep link hijacking fixes - Fix double prefix `/app/app` issue - Fix going from tool -> editor -> processor -> editor putting you back into tool --------- Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com>
This commit is contained in:
co-authored by
Anthony Stirling
parent
849d616451
commit
0a3f0c1814
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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) {
|
||||
// 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,16 +137,14 @@ 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("//")
|
||||
const destination = isSafePostLoginRedirect(explicitNext)
|
||||
? explicitNext
|
||||
: pendingConnect
|
||||
? `/link?request=${encodeURIComponent(pendingConnect)}`
|
||||
: next.startsWith("/") && !next.startsWith("//")
|
||||
: isSafePostLoginRedirect(next)
|
||||
? next
|
||||
: await resolveLandingPath();
|
||||
console.log("[Auth Callback Debug] Redirecting to:", destination);
|
||||
|
||||
@@ -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("");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user