From fc24be26279667dd34c6d520a608129db1920fb0 Mon Sep 17 00:00:00 2001
From: Hiep Le <69354317+hieptl@users.noreply.github.com>
Date: Tue, 10 Mar 2026 22:52:40 +0700
Subject: [PATCH] fix(frontend): preserve login_method param to enable session
 re-authentication (#13310)

---
 frontend/__tests__/routes/login.test.tsx      |  84 ++++++++++--
 .../__tests__/routes/root-layout.test.tsx     | 124 ++++++++++++++++++
 frontend/src/routes/login.tsx                 |  11 +-
 frontend/src/routes/root-layout.tsx           |  20 +--
 4 files changed, 216 insertions(+), 23 deletions(-)

diff --git a/frontend/__tests__/routes/login.test.tsx b/frontend/__tests__/routes/login.test.tsx
index 35751a591e..8158413dfc 100644
--- a/frontend/__tests__/routes/login.test.tsx
+++ b/frontend/__tests__/routes/login.test.tsx
@@ -2,7 +2,7 @@ import { render, screen, waitFor } from "@testing-library/react";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import userEvent from "@testing-library/user-event";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
-import { createRoutesStub } from "react-router";
+import { createRoutesStub, useSearchParams } from "react-router";
 import LoginPage from "#/routes/login";
 import OptionService from "#/api/option-service/option-service.api";
 import AuthService from "#/api/auth-service/auth-service.api";
@@ -80,6 +80,29 @@ const RouterStub = createRoutesStub([
   },
 ]);
 
+function DestinationStub() {
+  const [params] = useSearchParams();
+  const loginMethod = params.get("login_method");
+  return (
+    <div data-testid="destination-page">
+      {loginMethod && (
+        <span data-testid="login-method-param">{loginMethod}</span>
+      )}
+    </div>
+  );
+}
+
+const RouterStubWithDestination = createRoutesStub([
+  {
+    Component: LoginPage,
+    path: "/login",
+  },
+  {
+    Component: DestinationStub,
+    path: "/settings",
+  },
+]);
+
 const createWrapper = () => {
   const queryClient = new QueryClient({
     defaultOptions: {
@@ -282,7 +305,9 @@ describe("LoginPage", () => {
       await user.click(gitlabButton);
 
       // URL includes state parameter added by handleAuthRedirect
-      expect(window.location.href).toContain("https://gitlab.com/oauth/authorize");
+      expect(window.location.href).toContain(
+        "https://gitlab.com/oauth/authorize",
+      );
     });
 
     it("should redirect to Bitbucket auth URL when Bitbucket button is clicked", async () => {
@@ -347,6 +372,30 @@ describe("LoginPage", () => {
       );
     });
 
+    it("should preserve login_method param when redirecting authenticated users", async () => {
+      // Arrange
+      vi.spyOn(AuthService, "authenticate").mockResolvedValue(true);
+
+      // Act
+      render(
+        <RouterStubWithDestination
+          initialEntries={["/login?returnTo=/settings&login_method=github"]}
+        />,
+        { wrapper: createWrapper() },
+      );
+
+      // Assert
+      await waitFor(
+        () => {
+          expect(screen.getByTestId("destination-page")).toBeInTheDocument();
+          expect(screen.getByTestId("login-method-param")).toHaveTextContent(
+            "github",
+          );
+        },
+        { timeout: 2000 },
+      );
+    });
+
     it("should redirect OSS mode users to home", async () => {
       // @ts-expect-error - partial mock for testing
       vi.spyOn(OptionService, "getConfig").mockResolvedValue({
@@ -552,10 +601,12 @@ describe("LoginPage", () => {
 
     it("should pass buildOAuthStateData to LoginContent for OAuth state encoding", async () => {
       const user = userEvent.setup();
-      const mockBuildOAuthStateData = vi.fn((baseState: Record<string, string>) => ({
-        ...baseState,
-        invitation_token: "inv-test-token-12345",
-      }));
+      const mockBuildOAuthStateData = vi.fn(
+        (baseState: Record<string, string>) => ({
+          ...baseState,
+          invitation_token: "inv-test-token-12345",
+        }),
+      );
 
       useInvitationMock.mockReturnValue({
         invitationToken: "inv-test-token-12345",
@@ -585,10 +636,12 @@ describe("LoginPage", () => {
 
     it("should include invitation token in OAuth state when invitation is present", async () => {
       const user = userEvent.setup();
-      const mockBuildOAuthStateData = vi.fn((baseState: Record<string, string>) => ({
-        ...baseState,
-        invitation_token: "inv-test-token-12345",
-      }));
+      const mockBuildOAuthStateData = vi.fn(
+        (baseState: Record<string, string>) => ({
+          ...baseState,
+          invitation_token: "inv-test-token-12345",
+        }),
+      );
 
       useInvitationMock.mockReturnValue({
         invitationToken: "inv-test-token-12345",
@@ -634,9 +687,14 @@ describe("LoginPage", () => {
         clearInvitation: vi.fn(),
       });
 
-      render(<RouterStub initialEntries={["/login?invitation_token=inv-url-token-67890"]} />, {
-        wrapper: createWrapper(),
-      });
+      render(
+        <RouterStub
+          initialEntries={["/login?invitation_token=inv-url-token-67890"]}
+        />,
+        {
+          wrapper: createWrapper(),
+        },
+      );
 
       await waitFor(() => {
         expect(screen.getByText("AUTH$INVITATION_PENDING")).toBeInTheDocument();
diff --git a/frontend/__tests__/routes/root-layout.test.tsx b/frontend/__tests__/routes/root-layout.test.tsx
index 0183553534..ba7327d0c4 100644
--- a/frontend/__tests__/routes/root-layout.test.tsx
+++ b/frontend/__tests__/routes/root-layout.test.tsx
@@ -366,6 +366,130 @@ describe("MainApp", () => {
     });
   });
 
+  describe("Re-authentication with stored login method", () => {
+    it("should show ReauthModal instead of redirecting to /login when login method exists", async () => {
+      // Arrange - user is unauthenticated but has a stored login method
+      vi.spyOn(AuthService, "authenticate").mockRejectedValue({
+        response: { status: 401 },
+        isAxiosError: true,
+      });
+
+      vi.stubGlobal("localStorage", {
+        getItem: vi.fn((key: string) => {
+          if (key === "openhands_login_method") {
+            return "github";
+          }
+          return null;
+        }),
+        setItem: vi.fn(),
+        removeItem: vi.fn(),
+        clear: vi.fn(),
+      });
+
+      // Act
+      renderWithLoginStub(RouterStubWithLogin, ["/"]);
+
+      // Assert - should show ReauthModal (with "Logging back in" text), not redirect to /login
+      await waitFor(
+        () => {
+          expect(screen.getByText("AUTH$LOGGING_BACK_IN")).toBeInTheDocument();
+        },
+        { timeout: 2000 },
+      );
+
+      // Login page should NOT be shown when login method exists
+      expect(screen.queryByTestId("login-page")).not.toBeInTheDocument();
+    });
+
+    it("should redirect to /login when no login method is stored", async () => {
+      // Arrange - user is unauthenticated and has no stored login method
+      vi.spyOn(AuthService, "authenticate").mockRejectedValue({
+        response: { status: 401 },
+        isAxiosError: true,
+      });
+
+      vi.stubGlobal("localStorage", {
+        getItem: vi.fn(() => null),
+        setItem: vi.fn(),
+        removeItem: vi.fn(),
+        clear: vi.fn(),
+      });
+
+      // Act
+      renderWithLoginStub(RouterStubWithLogin, ["/"]);
+
+      // Assert - should redirect to /login
+      await waitFor(
+        () => {
+          expect(screen.getByTestId("login-page")).toBeInTheDocument();
+        },
+        { timeout: 2000 },
+      );
+    });
+  });
+
+  describe("Loading states", () => {
+    it("should show loading spinner while config is loading without redirecting", async () => {
+      // Arrange - config never resolves (loading state)
+      vi.spyOn(OptionService, "getConfig").mockImplementation(
+        () => new Promise(() => {}),
+      );
+
+      vi.stubGlobal("localStorage", {
+        getItem: vi.fn((key: string) => {
+          if (key === "openhands_login_method") {
+            return "github";
+          }
+          return null;
+        }),
+        setItem: vi.fn(),
+        removeItem: vi.fn(),
+        clear: vi.fn(),
+      });
+
+      // Act
+      renderWithLoginStub(RouterStubWithLogin, ["/"]);
+
+      // Assert - should show loading spinner
+      await waitFor(() => {
+        expect(screen.getByTestId("loading-spinner")).toBeInTheDocument();
+      });
+
+      // Should NOT redirect to login while loading
+      expect(screen.queryByTestId("login-page")).not.toBeInTheDocument();
+    });
+
+    it("should show loading spinner while auth is loading without redirecting", async () => {
+      // Arrange - auth never resolves (loading state)
+      vi.spyOn(AuthService, "authenticate").mockImplementation(
+        () => new Promise(() => {}),
+      );
+
+      vi.stubGlobal("localStorage", {
+        getItem: vi.fn((key: string) => {
+          if (key === "openhands_login_method") {
+            return "github";
+          }
+          return null;
+        }),
+        setItem: vi.fn(),
+        removeItem: vi.fn(),
+        clear: vi.fn(),
+      });
+
+      // Act
+      renderWithLoginStub(RouterStubWithLogin, ["/"]);
+
+      // Assert - should show loading spinner
+      await waitFor(() => {
+        expect(screen.getByTestId("loading-spinner")).toBeInTheDocument();
+      });
+
+      // Should NOT redirect to login while loading
+      expect(screen.queryByTestId("login-page")).not.toBeInTheDocument();
+    });
+  });
+
   describe("Invitation URL Parameters", () => {
     beforeEach(() => {
       vi.spyOn(AuthService, "authenticate").mockRejectedValue({
diff --git a/frontend/src/routes/login.tsx b/frontend/src/routes/login.tsx
index 0b357909b6..e8766d9500 100644
--- a/frontend/src/routes/login.tsx
+++ b/frontend/src/routes/login.tsx
@@ -40,11 +40,18 @@ export default function LoginPage() {
   }, [config.isLoading, config.data?.app_mode, navigate]);
 
   // Redirect authenticated users away from login page
+  // Preserve login_method param so useAuthCallback can store it for auto-login
   React.useEffect(() => {
     if (!isAuthLoading && isAuthed) {
-      navigate(returnTo, { replace: true });
+      const loginMethod = searchParams.get("login_method");
+      let destination = returnTo;
+      if (loginMethod) {
+        const separator = returnTo.includes("?") ? "&" : "?";
+        destination = `${returnTo}${separator}login_method=${encodeURIComponent(loginMethod)}`;
+      }
+      navigate(destination, { replace: true });
     }
-  }, [isAuthed, isAuthLoading, navigate, returnTo]);
+  }, [isAuthed, isAuthLoading, navigate, returnTo, searchParams]);
 
   if (isAuthLoading || config.isLoading) {
     return (
diff --git a/frontend/src/routes/root-layout.tsx b/frontend/src/routes/root-layout.tsx
index 967d26eeea..28cf7eee21 100644
--- a/frontend/src/routes/root-layout.tsx
+++ b/frontend/src/routes/root-layout.tsx
@@ -173,14 +173,17 @@ export default function MainApp() {
     setLoginMethodExists(checkLoginMethodExists());
   }, [isAuthed, checkLoginMethodExists]);
 
+  // Show loading spinner while config or auth is loading
+  const isLoading = config.isLoading || isAuthLoading;
+
+  // Only decide to redirect AFTER loading completes
   const shouldRedirectToLogin =
-    config.isLoading ||
-    isAuthLoading ||
-    (!isAuthed &&
-      !isAuthError &&
-      !isOnIntermediatePage &&
-      config.data?.app_mode === "saas" &&
-      !loginMethodExists);
+    !isLoading &&
+    !isAuthed &&
+    !isAuthError &&
+    !isOnIntermediatePage &&
+    config.data?.app_mode === "saas" &&
+    !loginMethodExists;
 
   React.useEffect(() => {
     if (shouldRedirectToLogin) {
@@ -197,7 +200,8 @@ export default function MainApp() {
     }
   }, [shouldRedirectToLogin, pathname, searchParams, navigate]);
 
-  if (shouldRedirectToLogin) {
+  // Show loading spinner while loading OR when about to redirect
+  if (isLoading || shouldRedirectToLogin) {
     return (
       <div className="min-h-screen flex items-center justify-center bg-base">
         <LoadingSpinner size="large" />