🪃 fix: Prevent Recursive Login Redirect Loop (#11964)

* fix: Prevent recursive login redirect loop

    buildLoginRedirectUrl() would blindly encode the current URL into a
    redirect_to param even when already on /login, causing infinite nesting
    (/login?redirect_to=%2Flogin%3Fredirect_to%3D...). Guard at source so
    it returns plain /login when pathname starts with /login.

    Also validates redirect_to in the login error handler with isSafeRedirect
    to close an open-redirect vector, and removes a redundant /login guard
    from useAuthRedirect now handled by the centralized check.

* 🔀 fix: Handle basename-prefixed login paths and remove double URL decoding

    buildLoginRedirectUrl now uses isLoginPath() which matches /login,
    /librechat/login, and /login/2fa — covering subdirectory deployments
    where window.location.pathname includes the basename prefix.

    Remove redundant decodeURIComponent calls on URLSearchParams.get()
    results (which already returns decoded values) in getPostLoginRedirect,
    Login.tsx, and AuthContext login error handler. The extra decode could
    throw URIError on inputs containing literal percent signs.

* 🔀 fix: Tighten login path matching and add onError redirect tests

    Replace overbroad `endsWith('/login')` with a single regex
    `/(^|\/)login(\/|$)/` that matches `/login` only as a full path
    segment. Unifies `isSafeRedirect` and `buildLoginRedirectUrl` to use
    the same `LOGIN_PATH_RE` constant — no more divergent definitions.

    Add tests for the AuthContext onError redirect_to preservation logic
    (valid path preserved, open-redirect blocked, /login loop blocked),
    and a false-positive guard proving `/c/loginhistory` is not matched.

    Update JSDoc on `buildLoginRedirectUrl` to document the plain `/login`
    early-return, and add explanatory comment in AuthContext `onError`
    for why `buildLoginRedirectUrl()` cannot be used there.

* test: Add unit tests for AuthContextProvider login error handling

    Introduced a new test suite for AuthContextProvider to validate the handling of login errors and the preservation of redirect parameters. The tests cover various scenarios including valid redirect preservation, open-redirect prevention, and recursive redirect prevention. This enhances the robustness of the authentication flow and ensures proper navigation behavior during login failures.

client/src/components/Auth/Login.tsx

@@ -29,7 +29,7 @@ function Login() {
   useEffect(() => {
     const redirectTo = searchParams.get('redirect_to');
     if (redirectTo) {
-      persistRedirectToSession(decodeURIComponent(redirectTo));
+      persistRedirectToSession(redirectTo);
     } else {
       const state = location.state as LoginLocationState | null;
       if (state?.redirect_to) {

client/src/hooks/AuthContext.tsx

@@ -93,8 +93,14 @@ const AuthContextProvider = ({
     onError: (error: TResError | unknown) => {
       const resError = error as TResError;
       doSetError(resError.message);
+      // Preserve a valid redirect_to across login failures so the deep link survives retries.
+      // Cannot use buildLoginRedirectUrl() here — it reads the current pathname (already /login)
+      // and would return plain /login, dropping the redirect_to destination.
       const redirectTo = new URLSearchParams(window.location.search).get('redirect_to');
-      const loginPath = redirectTo ? `/login?redirect_to=${redirectTo}` : '/login';
+      const loginPath =
+        redirectTo && isSafeRedirect(redirectTo)
+          ? `/login?redirect_to=${encodeURIComponent(redirectTo)}`
+          : '/login';
       navigate(loginPath, { replace: true });
     },
   });

client/src/hooks/__tests__/AuthContext.spec.tsx

@@ -0,0 +1,174 @@
+import React from 'react';
+import { render, act } from '@testing-library/react';
+import { RecoilRoot } from 'recoil';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { MemoryRouter } from 'react-router-dom';
+
+import type { TAuthConfig } from '~/common';
+
+import { AuthContextProvider, useAuthContext } from '../AuthContext';
+
+const mockNavigate = jest.fn();
+jest.mock('react-router-dom', () => ({
+  ...jest.requireActual('react-router-dom'),
+  useNavigate: () => mockNavigate,
+}));
+
+jest.mock('librechat-data-provider', () => ({
+  ...jest.requireActual('librechat-data-provider'),
+  setTokenHeader: jest.fn(),
+}));
+
+let mockCapturedLoginOptions: {
+  onSuccess: (...args: unknown[]) => void;
+  onError: (...args: unknown[]) => void;
+};
+
+jest.mock('~/data-provider', () => ({
+  useLoginUserMutation: jest.fn(
+    (options: {
+      onSuccess: (...args: unknown[]) => void;
+      onError: (...args: unknown[]) => void;
+    }) => {
+      mockCapturedLoginOptions = options;
+      return { mutate: jest.fn() };
+    },
+  ),
+  useLogoutUserMutation: jest.fn(() => ({ mutate: jest.fn() })),
+  useRefreshTokenMutation: jest.fn(() => ({ mutate: jest.fn() })),
+  useGetUserQuery: jest.fn(() => ({
+    data: undefined,
+    isError: false,
+    error: null,
+  })),
+  useGetRole: jest.fn(() => ({ data: null })),
+}));
+
+const authConfig: TAuthConfig = { loginRedirect: '/login', test: true };
+
+function TestConsumer() {
+  const ctx = useAuthContext();
+  return <div data-testid="consumer" data-authenticated={ctx.isAuthenticated} />;
+}
+
+function renderProvider() {
+  const queryClient = new QueryClient({
+    defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
+  });
+
+  return render(
+    <QueryClientProvider client={queryClient}>
+      <RecoilRoot>
+        <MemoryRouter>
+          <AuthContextProvider authConfig={authConfig}>
+            <TestConsumer />
+          </AuthContextProvider>
+        </MemoryRouter>
+      </RecoilRoot>
+    </QueryClientProvider>,
+  );
+}
+
+describe('AuthContextProvider — login onError redirect handling', () => {
+  const originalLocation = window.location;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    Object.defineProperty(window, 'location', {
+      value: { ...originalLocation, pathname: '/login', search: '', hash: '' },
+      writable: true,
+      configurable: true,
+    });
+  });
+
+  afterEach(() => {
+    Object.defineProperty(window, 'location', {
+      value: originalLocation,
+      writable: true,
+      configurable: true,
+    });
+  });
+
+  it('preserves a valid redirect_to param across login failure', () => {
+    Object.defineProperty(window, 'location', {
+      value: { pathname: '/login', search: '?redirect_to=%2Fc%2Fabc123', hash: '' },
+      writable: true,
+      configurable: true,
+    });
+
+    renderProvider();
+
+    act(() => {
+      mockCapturedLoginOptions.onError({ message: 'Invalid credentials' });
+    });
+
+    expect(mockNavigate).toHaveBeenCalledWith('/login?redirect_to=%2Fc%2Fabc123', {
+      replace: true,
+    });
+  });
+
+  it('drops redirect_to when it contains an absolute URL (open-redirect prevention)', () => {
+    Object.defineProperty(window, 'location', {
+      value: { pathname: '/login', search: '?redirect_to=https%3A%2F%2Fevil.com', hash: '' },
+      writable: true,
+      configurable: true,
+    });
+
+    renderProvider();
+
+    act(() => {
+      mockCapturedLoginOptions.onError({ message: 'Invalid credentials' });
+    });
+
+    expect(mockNavigate).toHaveBeenCalledWith('/login', { replace: true });
+  });
+
+  it('drops redirect_to when it points to /login (recursive redirect prevention)', () => {
+    Object.defineProperty(window, 'location', {
+      value: { pathname: '/login', search: '?redirect_to=%2Flogin', hash: '' },
+      writable: true,
+      configurable: true,
+    });
+
+    renderProvider();
+
+    act(() => {
+      mockCapturedLoginOptions.onError({ message: 'Invalid credentials' });
+    });
+
+    expect(mockNavigate).toHaveBeenCalledWith('/login', { replace: true });
+  });
+
+  it('navigates to plain /login when no redirect_to param exists', () => {
+    renderProvider();
+
+    act(() => {
+      mockCapturedLoginOptions.onError({ message: 'Server error' });
+    });
+
+    expect(mockNavigate).toHaveBeenCalledWith('/login', { replace: true });
+  });
+
+  it('preserves redirect_to with query params and hash', () => {
+    const target = '/c/abc123?model=gpt-4#section';
+    Object.defineProperty(window, 'location', {
+      value: {
+        pathname: '/login',
+        search: `?redirect_to=${encodeURIComponent(target)}`,
+        hash: '',
+      },
+      writable: true,
+      configurable: true,
+    });
+
+    renderProvider();
+
+    act(() => {
+      mockCapturedLoginOptions.onError({ message: 'Invalid credentials' });
+    });
+
+    const navigatedUrl = mockNavigate.mock.calls[0][0] as string;
+    const params = new URLSearchParams(navigatedUrl.split('?')[1]);
+    expect(decodeURIComponent(params.get('redirect_to')!)).toBe(target);
+  });
+});

client/src/routes/useAuthRedirect.ts

@@ -14,11 +14,6 @@ export default function useAuthRedirect() {
         return;
       }
 
-      if (location.pathname.startsWith('/login')) {
-        navigate('/login', { replace: true });
-        return;
-      }
-
       navigate(buildLoginRedirectUrl(location.pathname, location.search, location.hash), {
         replace: true,
       });

client/src/utils/__tests__/redirect.test.ts

@@ -100,6 +100,45 @@ describe('buildLoginRedirectUrl', () => {
     const result = buildLoginRedirectUrl();
     expect(result).toBe('/login?redirect_to=%2F');
   });
+
+  it('returns plain /login when pathname is /login (prevents recursive redirect)', () => {
+    const result = buildLoginRedirectUrl('/login', '?redirect_to=%2Fc%2Fnew', '');
+    expect(result).toBe('/login');
+  });
+
+  it('returns plain /login when window.location is already /login', () => {
+    Object.defineProperty(window, 'location', {
+      value: { pathname: '/login', search: '?redirect_to=%2Fc%2Fabc', hash: '' },
+      writable: true,
+    });
+    const result = buildLoginRedirectUrl();
+    expect(result).toBe('/login');
+  });
+
+  it('returns plain /login for /login sub-paths', () => {
+    const result = buildLoginRedirectUrl('/login/2fa', '', '');
+    expect(result).toBe('/login');
+  });
+
+  it('returns plain /login for basename-prefixed /login (e.g. /librechat/login)', () => {
+    Object.defineProperty(window, 'location', {
+      value: { pathname: '/librechat/login', search: '?redirect_to=%2Fc%2Fabc', hash: '' },
+      writable: true,
+    });
+    const result = buildLoginRedirectUrl();
+    expect(result).toBe('/login');
+  });
+
+  it('returns plain /login for basename-prefixed /login sub-paths', () => {
+    const result = buildLoginRedirectUrl('/librechat/login/2fa', '', '');
+    expect(result).toBe('/login');
+  });
+
+  it('does NOT match paths where "login" is a substring of a segment', () => {
+    const result = buildLoginRedirectUrl('/c/loginhistory', '', '');
+    expect(result).toContain('redirect_to=');
+    expect(decodeURIComponent(result.split('redirect_to=')[1])).toBe('/c/loginhistory');
+  });
 });
 
 describe('getPostLoginRedirect', () => {
@@ -170,6 +209,36 @@ describe('getPostLoginRedirect', () => {
   });
 });
 
+describe('login error redirect_to preservation (AuthContext onError pattern)', () => {
+  /** Mirrors the logic in AuthContext.tsx loginUser.onError */
+  function buildLoginErrorPath(search: string): string {
+    const redirectTo = new URLSearchParams(search).get('redirect_to');
+    return redirectTo && isSafeRedirect(redirectTo)
+      ? `/login?redirect_to=${encodeURIComponent(redirectTo)}`
+      : '/login';
+  }
+
+  it('preserves a valid redirect_to across login failure', () => {
+    const result = buildLoginErrorPath('?redirect_to=%2Fc%2Fnew');
+    expect(result).toBe('/login?redirect_to=%2Fc%2Fnew');
+  });
+
+  it('drops an open-redirect attempt (absolute URL)', () => {
+    const result = buildLoginErrorPath('?redirect_to=https%3A%2F%2Fevil.com');
+    expect(result).toBe('/login');
+  });
+
+  it('drops a /login redirect_to to prevent loops', () => {
+    const result = buildLoginErrorPath('?redirect_to=%2Flogin');
+    expect(result).toBe('/login');
+  });
+
+  it('returns plain /login when no redirect_to param exists', () => {
+    const result = buildLoginErrorPath('');
+    expect(result).toBe('/login');
+  });
+});
+
 describe('persistRedirectToSession', () => {
   beforeEach(() => {
     sessionStorage.clear();

client/src/utils/redirect.ts

@@ -1,18 +1,27 @@
 const REDIRECT_PARAM = 'redirect_to';
 const SESSION_KEY = 'post_login_redirect_to';
 
+/** Matches `/login` as a full path segment, with optional basename prefix (e.g. `/librechat/login/2fa`) */
+const LOGIN_PATH_RE = /(?:^|\/)login(?:\/|$)/;
+
 /** Validates that a redirect target is a safe relative path (not an absolute or protocol-relative URL) */
 function isSafeRedirect(url: string): boolean {
   if (!url.startsWith('/') || url.startsWith('//')) {
     return false;
   }
   const path = url.split('?')[0].split('#')[0];
-  return !path.startsWith('/login');
+  return !LOGIN_PATH_RE.test(path);
 }
 
-/** Builds a `/login?redirect_to=...` URL, reading from window.location when no args are provided */
+/**
+ * Builds a `/login?redirect_to=...` URL from the given or current location.
+ * Returns plain `/login` (no param) when already on a login route to prevent recursive nesting.
+ */
 function buildLoginRedirectUrl(pathname?: string, search?: string, hash?: string): string {
   const p = pathname ?? window.location.pathname;
+  if (LOGIN_PATH_RE.test(p)) {
+    return '/login';
+  }
   const s = search ?? window.location.search;
   const h = hash ?? window.location.hash;
   const currentPath = `${p}${s}${h}`;
@@ -25,8 +34,7 @@ function buildLoginRedirectUrl(pathname?: string, search?: string, hash?: string
  * cleans up both sources, and returns the validated target (or null).
  */
 function getPostLoginRedirect(searchParams: URLSearchParams): string | null {
-  const encoded = searchParams.get(REDIRECT_PARAM);
-  const urlRedirect = encoded ? decodeURIComponent(encoded) : null;
+  const urlRedirect = searchParams.get(REDIRECT_PARAM);
   const storedRedirect = sessionStorage.getItem(SESSION_KEY);
 
   const target = urlRedirect ?? storedRedirect;