mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-05 07:40:19 +01:00
📍 feat: Preserve Deep Link Destinations Through the Auth Redirect Flow (#10275)
* added support for url query param persistance * refactor: authentication redirect handling - Introduced utility functions for managing login redirects, including `persistRedirectToSession`, `buildLoginRedirectUrl`, and `getPostLoginRedirect`. - Updated `Login` and `AuthContextProvider` components to utilize these utilities for improved redirect logic. - Refactored `useAuthRedirect` to streamline navigation to the login page while preserving intended destinations. - Cleaned up the `StartupLayout` to remove unnecessary redirect handling, ensuring a more straightforward navigation flow. - Added a new `redirect.ts` file to encapsulate redirect-related logic, enhancing code organization and maintainability. * fix: enhance safe redirect validation logic - Updated the `isSafeRedirect` function to improve validation of redirect URLs. - Ensured that only safe relative paths are accepted, specifically excluding paths that lead to the login page. - Refactored the logic to streamline the checks for valid redirect targets. * test: add unit tests for redirect utility functions - Introduced comprehensive tests for `isSafeRedirect`, `buildLoginRedirectUrl`, `getPostLoginRedirect`, and `persistRedirectToSession` functions. - Validated various scenarios including safe and unsafe redirects, URL encoding, and session storage behavior. - Enhanced test coverage to ensure robust handling of redirect logic and prevent potential security issues. * chore: streamline authentication and redirect handling - Removed unused `useLocation` import from `AuthContextProvider` and replaced its usage with `window.location` for better clarity. - Updated `StartupLayout` to check for pending redirects before navigating to the new chat page, ensuring users are directed appropriately based on their session state. - Enhanced unit tests for `useAuthRedirect` to verify correct handling of redirect parameters, including encoding of the current path and query parameters. * test: add unit tests for StartupLayout redirect behavior - Introduced a new test suite for the StartupLayout component to validate redirect logic based on authentication status and session storage. - Implemented tests to ensure correct navigation to the new conversation page when authenticated without pending redirects, and to prevent navigation when a redirect URL parameter or session storage redirect is present. - Enhanced coverage for scenarios where users are not authenticated, ensuring robust handling of redirect conditions. --------- Co-authored-by: Vamsi Konakanchi <vamsi.k@trackmind.com> Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
a0f9782e60
commit
e978a934fc
9 changed files with 529 additions and 44 deletions
|
|
@ -1,9 +1,10 @@
|
|||
import { useEffect, useState } from 'react';
|
||||
import { Outlet, useNavigate, useLocation } from 'react-router-dom';
|
||||
import type { TStartupConfig } from 'librechat-data-provider';
|
||||
import { TranslationKeys, useLocalize } from '~/hooks';
|
||||
import { useGetStartupConfig } from '~/data-provider';
|
||||
import AuthLayout from '~/components/Auth/AuthLayout';
|
||||
import { TranslationKeys, useLocalize } from '~/hooks';
|
||||
import { REDIRECT_PARAM, SESSION_KEY } from '~/utils';
|
||||
|
||||
const headerMap: Record<string, TranslationKeys> = {
|
||||
'/login': 'com_auth_welcome_back',
|
||||
|
|
@ -30,7 +31,12 @@ export default function StartupLayout({ isAuthenticated }: { isAuthenticated?: b
|
|||
|
||||
useEffect(() => {
|
||||
if (isAuthenticated) {
|
||||
navigate('/c/new', { replace: true });
|
||||
const hasPendingRedirect =
|
||||
new URLSearchParams(window.location.search).has(REDIRECT_PARAM) ||
|
||||
sessionStorage.getItem(SESSION_KEY) != null;
|
||||
if (!hasPendingRedirect) {
|
||||
navigate('/c/new', { replace: true });
|
||||
}
|
||||
}
|
||||
if (data) {
|
||||
setStartupConfig(data);
|
||||
|
|
|
|||
128
client/src/routes/__tests__/StartupLayout.spec.tsx
Normal file
128
client/src/routes/__tests__/StartupLayout.spec.tsx
Normal file
|
|
@ -0,0 +1,128 @@
|
|||
/* eslint-disable i18next/no-literal-string */
|
||||
import React from 'react';
|
||||
import { render, waitFor } from '@testing-library/react';
|
||||
import { createMemoryRouter, RouterProvider } from 'react-router-dom';
|
||||
import { SESSION_KEY } from '~/utils';
|
||||
import StartupLayout from '../Layouts/Startup';
|
||||
|
||||
if (typeof Request === 'undefined') {
|
||||
global.Request = class Request {
|
||||
constructor(
|
||||
public url: string,
|
||||
public init?: RequestInit,
|
||||
) {}
|
||||
} as any;
|
||||
}
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useGetStartupConfig: jest.fn(() => ({
|
||||
data: null,
|
||||
isFetching: false,
|
||||
error: null,
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useLocalize: jest.fn(() => (key: string) => key),
|
||||
TranslationKeys: {},
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Auth/AuthLayout', () => {
|
||||
return function MockAuthLayout({ children }: { children: React.ReactNode }) {
|
||||
return <div data-testid="auth-layout">{children}</div>;
|
||||
};
|
||||
});
|
||||
|
||||
function ChildRoute() {
|
||||
return <div data-testid="child-route">Child</div>;
|
||||
}
|
||||
|
||||
function NewConversation() {
|
||||
return <div data-testid="new-conversation">New Conversation</div>;
|
||||
}
|
||||
|
||||
const createTestRouter = (initialEntry: string, isAuthenticated: boolean) =>
|
||||
createMemoryRouter(
|
||||
[
|
||||
{
|
||||
path: '/login',
|
||||
element: <StartupLayout isAuthenticated={isAuthenticated} />,
|
||||
children: [{ index: true, element: <ChildRoute /> }],
|
||||
},
|
||||
{
|
||||
path: '/c/new',
|
||||
element: <NewConversation />,
|
||||
},
|
||||
],
|
||||
{ initialEntries: [initialEntry] },
|
||||
);
|
||||
|
||||
describe('StartupLayout — redirect race condition', () => {
|
||||
const originalLocation = window.location;
|
||||
|
||||
beforeEach(() => {
|
||||
sessionStorage.clear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(window, 'location', { value: originalLocation, writable: true });
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('navigates to /c/new when authenticated with no pending redirect', async () => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: { ...originalLocation, search: '' },
|
||||
writable: true,
|
||||
});
|
||||
|
||||
const router = createTestRouter('/login', true);
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(router.state.location.pathname).toBe('/c/new');
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT navigate to /c/new when redirect_to URL param is present', async () => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: { ...originalLocation, search: '?redirect_to=%2Fc%2Fabc123' },
|
||||
writable: true,
|
||||
});
|
||||
|
||||
const router = createTestRouter('/login?redirect_to=%2Fc%2Fabc123', true);
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
expect(router.state.location.pathname).toBe('/login');
|
||||
});
|
||||
|
||||
it('does NOT navigate to /c/new when sessionStorage redirect is present', async () => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: { ...originalLocation, search: '' },
|
||||
writable: true,
|
||||
});
|
||||
sessionStorage.setItem(SESSION_KEY, '/c/abc123');
|
||||
|
||||
const router = createTestRouter('/login', true);
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
expect(router.state.location.pathname).toBe('/login');
|
||||
});
|
||||
|
||||
it('does NOT navigate when not authenticated', async () => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: { ...originalLocation, search: '' },
|
||||
writable: true,
|
||||
});
|
||||
|
||||
const router = createTestRouter('/login', false);
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
expect(router.state.location.pathname).toBe('/login');
|
||||
});
|
||||
});
|
||||
|
|
@ -33,9 +33,8 @@ function TestComponent() {
|
|||
* Creates a test router with optional basename to verify navigation works correctly
|
||||
* with subdirectory deployments (e.g., /librechat)
|
||||
*/
|
||||
const createTestRouter = (basename = '/') => {
|
||||
// When using basename, initialEntries must include the basename
|
||||
const initialEntry = basename === '/' ? '/' : `${basename}/`;
|
||||
const createTestRouter = (basename = '/', initialEntry?: string) => {
|
||||
const defaultEntry = basename === '/' ? '/' : `${basename}/`;
|
||||
|
||||
return createMemoryRouter(
|
||||
[
|
||||
|
|
@ -47,10 +46,14 @@ const createTestRouter = (basename = '/') => {
|
|||
path: '/login',
|
||||
element: <div data-testid="login-page">Login Page</div>,
|
||||
},
|
||||
{
|
||||
path: '/c/:id',
|
||||
element: <TestComponent />,
|
||||
},
|
||||
],
|
||||
{
|
||||
basename,
|
||||
initialEntries: [initialEntry],
|
||||
initialEntries: [initialEntry ?? defaultEntry],
|
||||
},
|
||||
);
|
||||
};
|
||||
|
|
@ -199,4 +202,73 @@ describe('useAuthRedirect', () => {
|
|||
expect(testResult.isAuthenticated).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('should include redirect_to param with encoded current path when redirecting', async () => {
|
||||
(useAuthContext as jest.Mock).mockReturnValue({
|
||||
user: null,
|
||||
isAuthenticated: false,
|
||||
});
|
||||
|
||||
const router = createTestRouter('/', '/c/abc123');
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(router.state.location.pathname).toBe('/login');
|
||||
const search = router.state.location.search;
|
||||
const params = new URLSearchParams(search);
|
||||
const redirectTo = params.get('redirect_to');
|
||||
expect(redirectTo).not.toBeNull();
|
||||
expect(decodeURIComponent(redirectTo!)).toBe('/c/abc123');
|
||||
},
|
||||
{ timeout: 1000 },
|
||||
);
|
||||
});
|
||||
|
||||
it('should encode query params and hash from the source URL', async () => {
|
||||
(useAuthContext as jest.Mock).mockReturnValue({
|
||||
user: null,
|
||||
isAuthenticated: false,
|
||||
});
|
||||
|
||||
const router = createTestRouter('/', '/c/abc123?q=hello&submit=true#section');
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(router.state.location.pathname).toBe('/login');
|
||||
const params = new URLSearchParams(router.state.location.search);
|
||||
const decoded = decodeURIComponent(params.get('redirect_to')!);
|
||||
expect(decoded).toBe('/c/abc123?q=hello&submit=true#section');
|
||||
},
|
||||
{ timeout: 1000 },
|
||||
);
|
||||
});
|
||||
|
||||
it('should not append redirect_to when already on /login', async () => {
|
||||
(useAuthContext as jest.Mock).mockReturnValue({
|
||||
user: null,
|
||||
isAuthenticated: false,
|
||||
});
|
||||
|
||||
const router = createMemoryRouter(
|
||||
[
|
||||
{
|
||||
path: '/login',
|
||||
element: <TestComponent />,
|
||||
},
|
||||
],
|
||||
{ initialEntries: ['/login'] },
|
||||
);
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(router.state.location.pathname).toBe('/login');
|
||||
},
|
||||
{ timeout: 1000 },
|
||||
);
|
||||
|
||||
expect(router.state.location.search).toBe('');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,22 +1,33 @@
|
|||
import { useEffect } from 'react';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { useLocation, useNavigate } from 'react-router-dom';
|
||||
import { buildLoginRedirectUrl } from '~/utils';
|
||||
import { useAuthContext } from '~/hooks';
|
||||
|
||||
export default function useAuthRedirect() {
|
||||
const { user, roles, isAuthenticated } = useAuthContext();
|
||||
const navigate = useNavigate();
|
||||
const location = useLocation();
|
||||
|
||||
useEffect(() => {
|
||||
const timeout = setTimeout(() => {
|
||||
if (!isAuthenticated) {
|
||||
navigate('/login', { replace: true });
|
||||
if (isAuthenticated) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (location.pathname.startsWith('/login')) {
|
||||
navigate('/login', { replace: true });
|
||||
return;
|
||||
}
|
||||
|
||||
navigate(buildLoginRedirectUrl(location.pathname, location.search, location.hash), {
|
||||
replace: true,
|
||||
});
|
||||
}, 300);
|
||||
|
||||
return () => {
|
||||
clearTimeout(timeout);
|
||||
};
|
||||
}, [isAuthenticated, navigate]);
|
||||
}, [isAuthenticated, navigate, location]);
|
||||
|
||||
return {
|
||||
user,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue