From e978a934fc5ee9b84c46545aef1b9b18159d9f7b Mon Sep 17 00:00:00 2001 From: Vamsi Konakanchi <39833739+vmskonakanchi@users.noreply.github.com> Date: Thu, 26 Feb 2026 08:51:19 +0530 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=8D=20feat:=20Preserve=20Deep=20Link?= =?UTF-8?q?=20Destinations=20Through=20the=20Auth=20Redirect=20Flow=20(#10?= =?UTF-8?q?275)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 Co-authored-by: Danny Avila --- client/src/components/Auth/Login.tsx | 26 ++- client/src/hooks/AuthContext.tsx | 54 ++--- client/src/routes/Layouts/Startup.tsx | 10 +- .../routes/__tests__/StartupLayout.spec.tsx | 128 ++++++++++++ .../routes/__tests__/useAuthRedirect.spec.tsx | 80 ++++++- client/src/routes/useAuthRedirect.ts | 19 +- client/src/utils/__tests__/redirect.test.ts | 197 ++++++++++++++++++ client/src/utils/index.ts | 1 + client/src/utils/redirect.ts | 58 ++++++ 9 files changed, 529 insertions(+), 44 deletions(-) create mode 100644 client/src/routes/__tests__/StartupLayout.spec.tsx create mode 100644 client/src/utils/__tests__/redirect.test.ts create mode 100644 client/src/utils/redirect.ts diff --git a/client/src/components/Auth/Login.tsx b/client/src/components/Auth/Login.tsx index 48a506879f..e0bf89bacd 100644 --- a/client/src/components/Auth/Login.tsx +++ b/client/src/components/Auth/Login.tsx @@ -1,15 +1,19 @@ import { useEffect, useState } from 'react'; import { ErrorTypes, registerPage } from 'librechat-data-provider'; import { OpenIDIcon, useToastContext } from '@librechat/client'; -import { useOutletContext, useSearchParams } from 'react-router-dom'; +import { useOutletContext, useSearchParams, useLocation } from 'react-router-dom'; import type { TLoginLayoutContext } from '~/common'; +import { getLoginError, persistRedirectToSession } from '~/utils'; import { ErrorMessage } from '~/components/Auth/ErrorMessage'; import SocialButton from '~/components/Auth/SocialButton'; import { useAuthContext } from '~/hooks/AuthContext'; -import { getLoginError } from '~/utils'; import { useLocalize } from '~/hooks'; import LoginForm from './LoginForm'; +interface LoginLocationState { + redirect_to?: string; +} + function Login() { const localize = useLocalize(); const { showToast } = useToastContext(); @@ -17,13 +21,22 @@ function Login() { const { startupConfig } = useOutletContext(); const [searchParams, setSearchParams] = useSearchParams(); - // Determine if auto-redirect should be disabled based on the URL parameter + const location = useLocation(); const disableAutoRedirect = searchParams.get('redirect') === 'false'; - // Persist the disable flag locally so that once detected, auto-redirect stays disabled. const [isAutoRedirectDisabled, setIsAutoRedirectDisabled] = useState(disableAutoRedirect); useEffect(() => { + const redirectTo = searchParams.get('redirect_to'); + if (redirectTo) { + persistRedirectToSession(decodeURIComponent(redirectTo)); + } else { + const state = location.state as LoginLocationState | null; + if (state?.redirect_to) { + persistRedirectToSession(state.redirect_to); + } + } + const oauthError = searchParams?.get('error'); if (oauthError && oauthError === ErrorTypes.AUTH_FAILED) { showToast({ @@ -34,9 +47,8 @@ function Login() { newParams.delete('error'); setSearchParams(newParams, { replace: true }); } - }, [searchParams, setSearchParams, showToast, localize]); + }, [searchParams, setSearchParams, showToast, localize, location.state]); - // Once the disable flag is detected, update local state and remove the parameter from the URL. useEffect(() => { if (disableAutoRedirect) { setIsAutoRedirectDisabled(true); @@ -46,7 +58,6 @@ function Login() { } }, [disableAutoRedirect, searchParams, setSearchParams]); - // Determine whether we should auto-redirect to OpenID. const shouldAutoRedirect = startupConfig?.openidLoginEnabled && startupConfig?.openidAutoRedirect && @@ -60,7 +71,6 @@ function Login() { } }, [shouldAutoRedirect, startupConfig]); - // Render fallback UI if auto-redirect is active. if (shouldAutoRedirect) { return (
diff --git a/client/src/hooks/AuthContext.tsx b/client/src/hooks/AuthContext.tsx index d9d583783a..04bc3445c9 100644 --- a/client/src/hooks/AuthContext.tsx +++ b/client/src/hooks/AuthContext.tsx @@ -3,7 +3,6 @@ import { useMemo, useState, useEffect, - ReactNode, useContext, useCallback, createContext, @@ -12,6 +11,7 @@ import { debounce } from 'lodash'; import { useRecoilState } from 'recoil'; import { useNavigate } from 'react-router-dom'; import { setTokenHeader, SystemRoles } from 'librechat-data-provider'; +import type { ReactNode } from 'react'; import type * as t from 'librechat-data-provider'; import { useGetRole, @@ -20,6 +20,7 @@ import { useLogoutUserMutation, useRefreshTokenMutation, } from '~/data-provider'; +import { isSafeRedirect, buildLoginRedirectUrl, getPostLoginRedirect } from '~/utils'; import { TAuthConfig, TUserContext, TAuthContext, TResError } from '~/common'; import useTimeout from './useTimeout'; import store from '~/store'; @@ -58,20 +59,22 @@ const AuthContextProvider = ({ setTokenHeader(token); setIsAuthenticated(isAuthenticated); - // Use a custom redirect if set - const finalRedirect = logoutRedirectRef.current || redirect; - // Clear the stored redirect + const searchParams = new URLSearchParams(window.location.search); + const postLoginRedirect = getPostLoginRedirect(searchParams); + + const logoutRedirect = logoutRedirectRef.current; logoutRedirectRef.current = undefined; + const finalRedirect = + logoutRedirect ?? + postLoginRedirect ?? + (redirect && isSafeRedirect(redirect) ? redirect : null); + if (finalRedirect == null) { return; } - if (finalRedirect.startsWith('http://') || finalRedirect.startsWith('https://')) { - window.location.href = finalRedirect; - } else { - navigate(finalRedirect, { replace: true }); - } + navigate(finalRedirect, { replace: true }); }, 50), [navigate, setUser], ); @@ -81,7 +84,6 @@ const AuthContextProvider = ({ onSuccess: (data: t.TLoginResponse) => { const { user, token, twoFAPending, tempToken } = data; if (twoFAPending) { - // Redirect to the two-factor authentication route. navigate(`/login/2fa?tempToken=${tempToken}`, { replace: true }); return; } @@ -91,7 +93,9 @@ const AuthContextProvider = ({ onError: (error: TResError | unknown) => { const resError = error as TResError; doSetError(resError.message); - navigate('/login', { replace: true }); + const redirectTo = new URLSearchParams(window.location.search).get('redirect_to'); + const loginPath = redirectTo ? `/login?redirect_to=${redirectTo}` : '/login'; + navigate(loginPath, { replace: true }); }, }); const logoutUser = useLogoutUserMutation({ @@ -141,30 +145,30 @@ const AuthContextProvider = ({ const { user, token = '' } = data ?? {}; if (token) { setUserContext({ token, isAuthenticated: true, user }); - } else { - console.log('Token is not present. User is not authenticated.'); - if (authConfig?.test === true) { - return; - } - navigate('/login'); + return; } + console.log('Token is not present. User is not authenticated.'); + if (authConfig?.test === true) { + return; + } + navigate(buildLoginRedirectUrl()); }, onError: (error) => { console.log('refreshToken mutation error:', error); if (authConfig?.test === true) { return; } - navigate('/login'); + navigate(buildLoginRedirectUrl()); }, }); - }, []); + }, [authConfig?.test, refreshToken, setUserContext, navigate]); useEffect(() => { if (userQuery.data) { setUser(userQuery.data); } else if (userQuery.isError) { doSetError((userQuery.error as Error).message); - navigate('/login', { replace: true }); + navigate(buildLoginRedirectUrl(), { replace: true }); } if (error != null && error && isAuthenticated) { doSetError(undefined); @@ -186,24 +190,22 @@ const AuthContextProvider = ({ ]); useEffect(() => { - const handleTokenUpdate = (event) => { + const handleTokenUpdate = (event: CustomEvent) => { console.log('tokenUpdated event received event'); - const newToken = event.detail; setUserContext({ - token: newToken, + token: event.detail, isAuthenticated: true, user: user, }); }; - window.addEventListener('tokenUpdated', handleTokenUpdate); + window.addEventListener('tokenUpdated', handleTokenUpdate as EventListener); return () => { - window.removeEventListener('tokenUpdated', handleTokenUpdate); + window.removeEventListener('tokenUpdated', handleTokenUpdate as EventListener); }; }, [setUserContext, user]); - // Make the provider update only when it should const memoedValue = useMemo( () => ({ user, diff --git a/client/src/routes/Layouts/Startup.tsx b/client/src/routes/Layouts/Startup.tsx index 9c9e0952dd..bb0e5ef254 100644 --- a/client/src/routes/Layouts/Startup.tsx +++ b/client/src/routes/Layouts/Startup.tsx @@ -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 = { '/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); diff --git a/client/src/routes/__tests__/StartupLayout.spec.tsx b/client/src/routes/__tests__/StartupLayout.spec.tsx new file mode 100644 index 0000000000..8d2c183137 --- /dev/null +++ b/client/src/routes/__tests__/StartupLayout.spec.tsx @@ -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
{children}
; + }; +}); + +function ChildRoute() { + return
Child
; +} + +function NewConversation() { + return
New Conversation
; +} + +const createTestRouter = (initialEntry: string, isAuthenticated: boolean) => + createMemoryRouter( + [ + { + path: '/login', + element: , + children: [{ index: true, element: }], + }, + { + path: '/c/new', + element: , + }, + ], + { 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(); + + 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(); + + 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(); + + 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(); + + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(router.state.location.pathname).toBe('/login'); + }); +}); diff --git a/client/src/routes/__tests__/useAuthRedirect.spec.tsx b/client/src/routes/__tests__/useAuthRedirect.spec.tsx index 19226aa29f..2f3a47c022 100644 --- a/client/src/routes/__tests__/useAuthRedirect.spec.tsx +++ b/client/src/routes/__tests__/useAuthRedirect.spec.tsx @@ -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:
Login Page
, }, + { + path: '/c/:id', + element: , + }, ], { 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(); + + 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(); + + 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: , + }, + ], + { initialEntries: ['/login'] }, + ); + render(); + + await waitFor( + () => { + expect(router.state.location.pathname).toBe('/login'); + }, + { timeout: 1000 }, + ); + + expect(router.state.location.search).toBe(''); + }); }); diff --git a/client/src/routes/useAuthRedirect.ts b/client/src/routes/useAuthRedirect.ts index 86d8103384..7303952155 100644 --- a/client/src/routes/useAuthRedirect.ts +++ b/client/src/routes/useAuthRedirect.ts @@ -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, diff --git a/client/src/utils/__tests__/redirect.test.ts b/client/src/utils/__tests__/redirect.test.ts new file mode 100644 index 0000000000..36336b0d94 --- /dev/null +++ b/client/src/utils/__tests__/redirect.test.ts @@ -0,0 +1,197 @@ +import { + isSafeRedirect, + buildLoginRedirectUrl, + getPostLoginRedirect, + persistRedirectToSession, + SESSION_KEY, +} from '../redirect'; + +describe('isSafeRedirect', () => { + it('accepts a simple relative path', () => { + expect(isSafeRedirect('/c/new')).toBe(true); + }); + + it('accepts a path with query params and hash', () => { + expect(isSafeRedirect('/c/new?q=hello&submit=true#section')).toBe(true); + }); + + it('accepts a nested path', () => { + expect(isSafeRedirect('/dashboard/settings/profile')).toBe(true); + }); + + it('rejects an absolute http URL', () => { + expect(isSafeRedirect('https://evil.com')).toBe(false); + }); + + it('rejects an absolute http URL with path', () => { + expect(isSafeRedirect('https://evil.com/phishing')).toBe(false); + }); + + it('rejects a protocol-relative URL', () => { + expect(isSafeRedirect('//evil.com')).toBe(false); + }); + + it('rejects a bare domain', () => { + expect(isSafeRedirect('evil.com')).toBe(false); + }); + + it('rejects an empty string', () => { + expect(isSafeRedirect('')).toBe(false); + }); + + it('rejects /login to prevent redirect loops', () => { + expect(isSafeRedirect('/login')).toBe(false); + }); + + it('rejects /login with query params', () => { + expect(isSafeRedirect('/login?redirect_to=/c/new')).toBe(false); + }); + + it('rejects /login sub-paths', () => { + expect(isSafeRedirect('/login/2fa')).toBe(false); + }); + + it('rejects /login with hash', () => { + expect(isSafeRedirect('/login#foo')).toBe(false); + }); + + it('accepts the root path', () => { + expect(isSafeRedirect('/')).toBe(true); + }); +}); + +describe('buildLoginRedirectUrl', () => { + const originalLocation = window.location; + + beforeEach(() => { + Object.defineProperty(window, 'location', { + value: { pathname: '/c/abc123', search: '?model=gpt-4', hash: '#msg-5' }, + writable: true, + }); + }); + + afterEach(() => { + Object.defineProperty(window, 'location', { value: originalLocation, writable: true }); + }); + + it('builds a login URL from explicit args', () => { + const result = buildLoginRedirectUrl('/c/new', '?q=hello', ''); + expect(result).toBe('/login?redirect_to=%2Fc%2Fnew%3Fq%3Dhello'); + }); + + it('encodes complex paths with query and hash', () => { + const result = buildLoginRedirectUrl('/c/new', '?q=hello&submit=true', '#section'); + expect(result).toContain('redirect_to='); + const encoded = result.split('redirect_to=')[1]; + expect(decodeURIComponent(encoded)).toBe('/c/new?q=hello&submit=true#section'); + }); + + it('falls back to window.location when no args provided', () => { + const result = buildLoginRedirectUrl(); + const encoded = result.split('redirect_to=')[1]; + expect(decodeURIComponent(encoded)).toBe('/c/abc123?model=gpt-4#msg-5'); + }); + + it('falls back to "/" when all location parts are empty', () => { + Object.defineProperty(window, 'location', { + value: { pathname: '', search: '', hash: '' }, + writable: true, + }); + const result = buildLoginRedirectUrl(); + expect(result).toBe('/login?redirect_to=%2F'); + }); +}); + +describe('getPostLoginRedirect', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + it('returns the redirect_to param when valid', () => { + const params = new URLSearchParams('redirect_to=%2Fc%2Fnew'); + expect(getPostLoginRedirect(params)).toBe('/c/new'); + }); + + it('falls back to sessionStorage when no URL param', () => { + sessionStorage.setItem(SESSION_KEY, '/c/abc123'); + const params = new URLSearchParams(); + expect(getPostLoginRedirect(params)).toBe('/c/abc123'); + }); + + it('prefers URL param over sessionStorage', () => { + sessionStorage.setItem(SESSION_KEY, '/c/old'); + const params = new URLSearchParams('redirect_to=%2Fc%2Fnew'); + expect(getPostLoginRedirect(params)).toBe('/c/new'); + }); + + it('clears sessionStorage after reading', () => { + sessionStorage.setItem(SESSION_KEY, '/c/abc123'); + const params = new URLSearchParams(); + getPostLoginRedirect(params); + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); + + it('returns null when no redirect source exists', () => { + const params = new URLSearchParams(); + expect(getPostLoginRedirect(params)).toBeNull(); + }); + + it('rejects an absolute URL from params', () => { + const params = new URLSearchParams('redirect_to=https%3A%2F%2Fevil.com'); + expect(getPostLoginRedirect(params)).toBeNull(); + }); + + it('rejects a protocol-relative URL from params', () => { + const params = new URLSearchParams('redirect_to=%2F%2Fevil.com'); + expect(getPostLoginRedirect(params)).toBeNull(); + }); + + it('rejects an absolute URL from sessionStorage', () => { + sessionStorage.setItem(SESSION_KEY, 'https://evil.com'); + const params = new URLSearchParams(); + expect(getPostLoginRedirect(params)).toBeNull(); + }); + + it('rejects /login redirect to prevent loops', () => { + const params = new URLSearchParams('redirect_to=%2Flogin'); + expect(getPostLoginRedirect(params)).toBeNull(); + }); + + it('rejects /login sub-path redirect', () => { + const params = new URLSearchParams('redirect_to=%2Flogin%2F2fa'); + expect(getPostLoginRedirect(params)).toBeNull(); + }); + + it('still clears sessionStorage even when target is unsafe', () => { + sessionStorage.setItem(SESSION_KEY, 'https://evil.com'); + const params = new URLSearchParams(); + getPostLoginRedirect(params); + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); +}); + +describe('persistRedirectToSession', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + it('stores a valid relative path', () => { + persistRedirectToSession('/c/new?q=hello'); + expect(sessionStorage.getItem(SESSION_KEY)).toBe('/c/new?q=hello'); + }); + + it('rejects an absolute URL', () => { + persistRedirectToSession('https://evil.com'); + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); + + it('rejects a protocol-relative URL', () => { + persistRedirectToSession('//evil.com'); + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); + + it('rejects /login paths', () => { + persistRedirectToSession('/login?redirect_to=/c/new'); + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); +}); diff --git a/client/src/utils/index.ts b/client/src/utils/index.ts index b8117b2677..6f081c7300 100644 --- a/client/src/utils/index.ts +++ b/client/src/utils/index.ts @@ -13,6 +13,7 @@ export * from './agents'; export * from './drafts'; export * from './convos'; export * from './routes'; +export * from './redirect'; export * from './presets'; export * from './prompts'; export * from './textarea'; diff --git a/client/src/utils/redirect.ts b/client/src/utils/redirect.ts new file mode 100644 index 0000000000..d2b7588151 --- /dev/null +++ b/client/src/utils/redirect.ts @@ -0,0 +1,58 @@ +const REDIRECT_PARAM = 'redirect_to'; +const SESSION_KEY = 'post_login_redirect_to'; + +/** 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'); +} + +/** Builds a `/login?redirect_to=...` URL, reading from window.location when no args are provided */ +function buildLoginRedirectUrl(pathname?: string, search?: string, hash?: string): string { + const p = pathname ?? window.location.pathname; + const s = search ?? window.location.search; + const h = hash ?? window.location.hash; + const currentPath = `${p}${s}${h}`; + const encoded = encodeURIComponent(currentPath || '/'); + return `/login?${REDIRECT_PARAM}=${encoded}`; +} + +/** + * Resolves the post-login redirect from URL params and sessionStorage, + * 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 storedRedirect = sessionStorage.getItem(SESSION_KEY); + + const target = urlRedirect ?? storedRedirect; + + if (storedRedirect) { + sessionStorage.removeItem(SESSION_KEY); + } + + if (target == null || !isSafeRedirect(target)) { + return null; + } + + return target; +} + +function persistRedirectToSession(value: string): void { + if (isSafeRedirect(value)) { + sessionStorage.setItem(SESSION_KEY, value); + } +} + +export { + SESSION_KEY, + REDIRECT_PARAM, + isSafeRedirect, + persistRedirectToSession, + buildLoginRedirectUrl, + getPostLoginRedirect, +};