feat: loginview should honor nextauth callbackurl from the query string (middleware-friendly) (#955)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: frano-m

src/hooks/authentication/session/useSessionActive.ts

@@ -1,4 +1,4 @@
-import Router from "next/router";
+import Router, { useRouter } from "next/router";
 import { useEffect } from "react";
 import { AUTH_STATUS, AuthState } from "../../../auth/types/auth";
 import {
@@ -16,17 +16,43 @@ export const useSessionActive = (
 ): void => {
   const { status: authStatus } = authState;
   const { status: authenticationStatus } = authenticationState;
+  const { query } = useRouter();
   const { callbackUrl } = useRouteHistory(2);
   const isReady =
     authenticationStatus === AUTHENTICATION_STATUS.SETTLED &&
     authStatus === AUTH_STATUS.SETTLED;
+  const queryCallbackUrl = getQueryCallbackUrl(query.callbackUrl);
 
   useEffect(() => {
     if (!isReady) return;
-    Router.push(callbackUrl(transformRoute));
+    Router.push(queryCallbackUrl ?? callbackUrl(transformRoute));
+    // eslint-disable-next-line react-hooks/exhaustive-deps -- queryCallbackUrl is intentionally omitted: this effect is a one-shot post-settle redirect, capturing the URL at first settle rather than re-firing each time the query changes.
   }, [callbackUrl, isReady]);
 };
 
+/**
+ * Extracts a same-origin `callbackUrl` path from a Next.js router query value.
+ * Next middleware (`withAuth`) appends `?callbackUrl=<original-path>` when it
+ * redirects an unauthenticated request to the sign-in page; honoring this
+ * keeps the consumer aligned with the standard NextAuth + middleware contract.
+ *
+ * Only accepts same-origin paths (must start with `/` and not `//`). Rejects
+ * absolute URLs (`https://...`) and protocol-relative URLs (`//evil.com/x`,
+ * which browsers treat as absolute) so the value can be safely passed to
+ * `Router.push`, which does not perform its own same-origin filtering.
+ *
+ * @param value - Raw `callbackUrl` query value (string, string[], or undefined).
+ * @returns The same-origin path, or `undefined` when missing or unsafe.
+ */
+export function getQueryCallbackUrl(
+  value: string | string[] | undefined,
+): string | undefined {
+  const raw = Array.isArray(value) ? value[0] : value;
+  if (typeof raw !== "string" || raw.length === 0) return undefined;
+  if (!raw.startsWith("/") || raw.startsWith("//")) return undefined;
+  return raw;
+}
+
 /**
  * Finds the most recent route in the history that is not the login route and removes the inactivity timeout query parameter.
  * The inactivity timeout query parameter is appended to a URL to indicate that a session has expired. This function iterates

src/nextauth/hooks/useNextAuthService.ts

@@ -1,7 +1,11 @@
+import { useRouter } from "next/router";
 import { useCallback } from "react";
 import { Service } from "../../auth/types/auth";
 import { ProviderId } from "../../auth/types/common";
-import { transformRoute } from "../../hooks/authentication/session/useSessionActive";
+import {
+  getQueryCallbackUrl,
+  transformRoute,
+} from "../../hooks/authentication/session/useSessionActive";
 import { useRouteHistory } from "../../hooks/useRouteHistory";
 import { service } from "../service";
 
@@ -10,13 +14,17 @@ import { service } from "../service";
  * @returns auth service.
  */
 export const useNextAuthService = (): Service => {
+  const { query } = useRouter();
   const { callbackUrl } = useRouteHistory(2);
+  const queryCallbackUrl = getQueryCallbackUrl(query.callbackUrl);
 
   const onLogin = useCallback(
     (providerId: ProviderId) => {
-      service.login(providerId, { callbackUrl: callbackUrl(transformRoute) });
+      service.login(providerId, {
+        callbackUrl: queryCallbackUrl ?? callbackUrl(transformRoute),
+      });
     },
-    [callbackUrl],
+    [callbackUrl, queryCallbackUrl],
   );
 
   const onLogout = useCallback(

tests/getQueryCallbackUrl.test.ts

@@ -0,0 +1,47 @@
+import { getQueryCallbackUrl } from "../src/hooks/authentication/session/useSessionActive";
+
+describe("getQueryCallbackUrl", () => {
+  it("returns the string when query is a non-empty same-origin path", () => {
+    expect(getQueryCallbackUrl("/atlases")).toBe("/atlases");
+  });
+
+  it("preserves query string and fragment in the returned path", () => {
+    expect(getQueryCallbackUrl("/atlases?tab=metadata#row-1")).toBe(
+      "/atlases?tab=metadata#row-1",
+    );
+  });
+
+  it("returns the first element when query is a string array", () => {
+    expect(getQueryCallbackUrl(["/atlases", "/other"])).toBe("/atlases");
+  });
+
+  it("returns undefined when query is undefined", () => {
+    expect(getQueryCallbackUrl(undefined)).toBeUndefined();
+  });
+
+  it("returns undefined when query is an empty string", () => {
+    expect(getQueryCallbackUrl("")).toBeUndefined();
+  });
+
+  it("returns undefined when query is an empty array", () => {
+    expect(getQueryCallbackUrl([])).toBeUndefined();
+  });
+
+  it("returns undefined when the first array element is empty", () => {
+    expect(getQueryCallbackUrl(["", "/other"])).toBeUndefined();
+  });
+
+  it("rejects absolute http URLs (open-redirect guard)", () => {
+    expect(getQueryCallbackUrl("https://evil.example/phish")).toBeUndefined();
+    expect(getQueryCallbackUrl("http://evil.example/phish")).toBeUndefined();
+  });
+
+  it("rejects protocol-relative URLs (browsers treat as absolute)", () => {
+    expect(getQueryCallbackUrl("//evil.example/phish")).toBeUndefined();
+  });
+
+  it("rejects values that do not start with '/'", () => {
+    expect(getQueryCallbackUrl("atlases")).toBeUndefined();
+    expect(getQueryCallbackUrl("javascript:alert(1)")).toBeUndefined();
+  });
+});

tests/useNextAuthService.test.ts

@@ -0,0 +1,88 @@
+import { jest } from "@jest/globals";
+import { act, renderHook } from "@testing-library/react";
+import { TransformRouteFn } from "../src/hooks/useRouteHistory";
+
+const ROOT_PATH = "/";
+const ROUTES = ["/login", "/route1", "/route2"];
+const PROVIDER_ID = "google";
+
+let mockRouterQuery: Record<string, string | string[] | undefined> = {};
+
+jest.unstable_mockModule("next/router", () => ({
+  ...jest.requireActual<typeof import("next/router")>("next/router"),
+  useRouter: jest.fn(() => ({ query: mockRouterQuery })),
+}));
+jest.unstable_mockModule("../src/hooks/useRouteHistory", () => ({
+  useRouteHistory: jest.fn(),
+}));
+jest.unstable_mockModule("../src/nextauth/service", () => ({
+  service: { login: jest.fn(), logout: jest.fn() },
+}));
+
+const { useRouteHistory } = await import("../src/hooks/useRouteHistory");
+const { service } = await import("../src/nextauth/service");
+const { useNextAuthService } =
+  await import("../src/nextauth/hooks/useNextAuthService");
+
+const MOCK_USE_ROUTE_HISTORY = useRouteHistory as jest.MockedFunction<
+  typeof useRouteHistory
+>;
+const MOCK_LOGIN = service.login as jest.MockedFunction<typeof service.login>;
+
+describe("useNextAuthService", () => {
+  beforeEach(() => {
+    mockRouterQuery = {};
+    MOCK_LOGIN.mockReset();
+    MOCK_USE_ROUTE_HISTORY.mockReset();
+    MOCK_USE_ROUTE_HISTORY.mockReturnValue({
+      callbackUrl: jest.fn(
+        (transformFn?: TransformRouteFn | undefined) =>
+          transformFn?.(ROUTES) ?? ROOT_PATH,
+      ),
+    });
+  });
+
+  test("requestLogin uses route-history fallback when query.callbackUrl is absent", () => {
+    const { result } = renderHook(() => useNextAuthService());
+    act(() => result.current.requestLogin(PROVIDER_ID));
+    expect(MOCK_LOGIN).toHaveBeenCalledWith(PROVIDER_ID, {
+      callbackUrl: ROUTES[1],
+    });
+  });
+
+  test("requestLogin prefers a same-origin query.callbackUrl over route history", () => {
+    mockRouterQuery = { callbackUrl: "/from-query" };
+    const { result } = renderHook(() => useNextAuthService());
+    act(() => result.current.requestLogin(PROVIDER_ID));
+    expect(MOCK_LOGIN).toHaveBeenCalledWith(PROVIDER_ID, {
+      callbackUrl: "/from-query",
+    });
+  });
+
+  test("requestLogin falls back to route history when query.callbackUrl is empty", () => {
+    mockRouterQuery = { callbackUrl: "" };
+    const { result } = renderHook(() => useNextAuthService());
+    act(() => result.current.requestLogin(PROVIDER_ID));
+    expect(MOCK_LOGIN).toHaveBeenCalledWith(PROVIDER_ID, {
+      callbackUrl: ROUTES[1],
+    });
+  });
+
+  test("requestLogin rejects an absolute URL in query.callbackUrl (open-redirect guard)", () => {
+    mockRouterQuery = { callbackUrl: "https://evil.example/phish" };
+    const { result } = renderHook(() => useNextAuthService());
+    act(() => result.current.requestLogin(PROVIDER_ID));
+    expect(MOCK_LOGIN).toHaveBeenCalledWith(PROVIDER_ID, {
+      callbackUrl: ROUTES[1],
+    });
+  });
+
+  test("requestLogin rejects a protocol-relative URL in query.callbackUrl", () => {
+    mockRouterQuery = { callbackUrl: "//evil.example/phish" };
+    const { result } = renderHook(() => useNextAuthService());
+    act(() => result.current.requestLogin(PROVIDER_ID));
+    expect(MOCK_LOGIN).toHaveBeenCalledWith(PROVIDER_ID, {
+      callbackUrl: ROUTES[1],
+    });
+  });
+});

tests/useSessionActive.test.ts

@@ -35,12 +35,15 @@ const AUTHENTICATION_STATE_SETTLED: AuthenticationState = {
 const ROOT_PATH = "/";
 const ROUTES = ["/login", "/route1", "/route2"];
 
+let mockRouterQuery: Record<string, string | string[] | undefined> = {};
+
 jest.unstable_mockModule("next/router", () => {
   return {
     ...jest.requireActual<typeof import("next/router")>("next/router"),
     default: {
       push: jest.fn(),
     },
+    useRouter: jest.fn(() => ({ query: mockRouterQuery })),
   };
 });
 jest.unstable_mockModule("../src/hooks/useRouteHistory", () => ({
@@ -58,6 +61,7 @@ const MOCK_USE_ROUTE_HISTORY = useRouteHistory as jest.MockedFunction<
 
 describe("useSessionActive", () => {
   beforeEach(() => {
+    mockRouterQuery = {};
     MOCK_USE_ROUTE_HISTORY.mockReset();
     MOCK_USE_ROUTE_HISTORY.mockReturnValue({
       callbackUrl: jest.fn(
@@ -100,4 +104,48 @@ describe("useSessionActive", () => {
     );
     expect(Router.push).toHaveBeenCalledWith(ROUTES[1]);
   });
+
+  test("prefers router.query.callbackUrl over route-history fallback", () => {
+    mockRouterQuery = { callbackUrl: "/from-query" };
+    renderHook(() =>
+      useSessionActive(
+        AUTH_STATE_AUTHENTICATED_SETTLED,
+        AUTHENTICATION_STATE_SETTLED,
+      ),
+    );
+    expect(Router.push).toHaveBeenCalledWith("/from-query");
+  });
+
+  test("falls back to route history when router.query.callbackUrl is an empty string", () => {
+    mockRouterQuery = { callbackUrl: "" };
+    renderHook(() =>
+      useSessionActive(
+        AUTH_STATE_AUTHENTICATED_SETTLED,
+        AUTHENTICATION_STATE_SETTLED,
+      ),
+    );
+    expect(Router.push).toHaveBeenCalledWith(ROUTES[1]);
+  });
+
+  test("falls back to route history when router.query.callbackUrl is an absolute URL (open-redirect guard)", () => {
+    mockRouterQuery = { callbackUrl: "https://evil.example/phish" };
+    renderHook(() =>
+      useSessionActive(
+        AUTH_STATE_AUTHENTICATED_SETTLED,
+        AUTHENTICATION_STATE_SETTLED,
+      ),
+    );
+    expect(Router.push).toHaveBeenCalledWith(ROUTES[1]);
+  });
+
+  test("falls back to route history when router.query.callbackUrl is protocol-relative", () => {
+    mockRouterQuery = { callbackUrl: "//evil.example/phish" };
+    renderHook(() =>
+      useSessionActive(
+        AUTH_STATE_AUTHENTICATED_SETTLED,
+        AUTHENTICATION_STATE_SETTLED,
+      ),
+    );
+    expect(Router.push).toHaveBeenCalledWith(ROUTES[1]);
+  });
 });