Fix redirect bug on SaaS

frontend/editor/src/core/services/httpErrorHandler.ts

@@ -13,6 +13,7 @@ import {
   extractAxiosErrorMessage,
 } from "@app/services/httpErrorUtils";
 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

frontend/editor/src/core/services/postLoginRedirect.test.ts

@@ -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);
+  });
+});

frontend/editor/src/core/services/postLoginRedirect.ts

@@ -0,0 +1,11 @@
+/**
+ * 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/");
+}

frontend/editor/src/proprietary/auth/spring/springAuthClient.ts

@@ -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,

frontend/editor/src/proprietary/services/postLoginRedirect.test.ts

@@ -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);
+  });
+});

frontend/editor/src/proprietary/services/postLoginRedirect.ts

@@ -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");
+}

frontend/editor/src/saas/routes/AuthCallback.tsx

@@ -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);

frontend/editor/src/saas/routes/Login.tsx

@@ -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;
     }

frontend/editor/src/saas/services/apiClient.test.ts

@@ -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("");
     }
   });
 });

frontend/editor/src/saas/services/apiClient.ts

@@ -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) {