mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-07 08:40:19 +01:00
🧭 fix: Subdirectory Deployment Auth Redirect Path Doubling (#12077)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
* fix: subdirectory redirects * fix: use path-segment boundary check when stripping BASE_URL prefix A bare `startsWith(BASE_URL)` matches on character prefix, not path segments. With BASE_URL="/chat", a path like "/chatroom/c/abc" would incorrectly strip to "room/c/abc" (no leading slash). Guard with an exact-match-or-slash check: `p === BASE_URL || p.startsWith(BASE_URL + '/')`. Also removes the dead `BASE_URL !== '/'` guard — module init already converts '/' to ''. * test: add path-segment boundary tests and clarify subdirectory coverage - Add /chatroom, /chatbot, /app/chatroom regression tests to verify BASE_URL stripping only matches on segment boundaries - Clarify useAuthRedirect subdirectory test documents React Router basename behavior (BASE_URL stripping tested in api-endpoints-subdir) - Use `delete proc.browser` instead of undefined assignment for cleanup - Add rationale to eslint-disable comment for isolateModules require * fix: use relative path and correct instructions in subdirectory test script - Replace hardcoded /home/danny/LibreChat/.env with repo-root-relative path so the script works from any checkout location - Update instructions to use production build (npm run build && npm run backend) since nginx proxies to :3080 which only serves the SPA after a full build, not during frontend:dev on :3090 * fix: skip pointless redirect_to=/ for root path and fix jsdom 26+ compat buildLoginRedirectUrl now returns plain /login when the resolved path is root — redirect_to=/ adds no value since / immediately redirects to /c/new after login anyway. Also rewrites api-endpoints.spec.ts to use window.history.replaceState instead of Object.defineProperty(window, 'location', ...) which jsdom 26+ no longer allows. * test: fix request-interceptor.spec.ts for jsdom 26+ compatibility Switch from jsdom to happy-dom environment which allows Object.defineProperty on window.location. jsdom 26+ made location non-configurable, breaking all 8 tests in this file. * chore: update browser property handling in api-endpoints-subdir test Changed the handling of the `proc.browser` property from deletion to setting it to false, ensuring compatibility with the current testing environment. * chore: update backend restart instructions in test subdirectory setup script Changed the instruction for restarting the backend from "npm run backend:dev" to "npm run backend" to reflect the correct command for the current setup. * refactor: ensure proper cleanup in loadModuleWithBase function Wrapped the module loading logic in a try-finally block to guarantee that the `proc.browser` property is reset to false and the base element is removed, improving reliability in the testing environment. * refactor: improve browser property handling in loadModuleWithBase function Revised the management of the `proc.browser` property to store the original value before modification, ensuring it is restored correctly after module loading. This enhances the reliability of the testing environment.
This commit is contained in:
parent
afb35103f1
commit
9956a72694
9 changed files with 426 additions and 34 deletions
|
|
@ -10,7 +10,12 @@ import {
|
|||
import { debounce } from 'lodash';
|
||||
import { useRecoilState } from 'recoil';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { setTokenHeader, SystemRoles, buildLoginRedirectUrl } from 'librechat-data-provider';
|
||||
import {
|
||||
apiBaseUrl,
|
||||
SystemRoles,
|
||||
setTokenHeader,
|
||||
buildLoginRedirectUrl,
|
||||
} from 'librechat-data-provider';
|
||||
import type * as t from 'librechat-data-provider';
|
||||
import type { ReactNode } from 'react';
|
||||
import {
|
||||
|
|
@ -168,7 +173,13 @@ const AuthContextProvider = ({
|
|||
if (token) {
|
||||
const storedRedirect = sessionStorage.getItem(SESSION_KEY);
|
||||
sessionStorage.removeItem(SESSION_KEY);
|
||||
const currentUrl = `${window.location.pathname}${window.location.search}`;
|
||||
const baseUrl = apiBaseUrl();
|
||||
const rawPath = window.location.pathname;
|
||||
const strippedPath =
|
||||
baseUrl && (rawPath === baseUrl || rawPath.startsWith(baseUrl + '/'))
|
||||
? rawPath.slice(baseUrl.length) || '/'
|
||||
: rawPath;
|
||||
const currentUrl = `${strippedPath}${window.location.search}`;
|
||||
const fallbackRedirect = isSafeRedirect(currentUrl) ? currentUrl : '/c/new';
|
||||
const redirect =
|
||||
storedRedirect && isSafeRedirect(storedRedirect) ? storedRedirect : fallbackRedirect;
|
||||
|
|
|
|||
|
|
@ -18,9 +18,12 @@ jest.mock('react-router-dom', () => ({
|
|||
useNavigate: () => mockNavigate,
|
||||
}));
|
||||
|
||||
const mockApiBaseUrl = jest.fn(() => '');
|
||||
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
...jest.requireActual('librechat-data-provider'),
|
||||
setTokenHeader: jest.fn(),
|
||||
apiBaseUrl: () => mockApiBaseUrl(),
|
||||
}));
|
||||
|
||||
let mockCapturedLoginOptions: {
|
||||
|
|
@ -352,6 +355,69 @@ describe('AuthContextProvider — silentRefresh post-login redirect', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('AuthContextProvider — silentRefresh subdirectory deployment', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
sessionStorage.clear();
|
||||
mockApiBaseUrl.mockReturnValue('/chat');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
mockApiBaseUrl.mockReturnValue('');
|
||||
sessionStorage.clear();
|
||||
window.history.replaceState({}, '', '/');
|
||||
});
|
||||
|
||||
it('strips base path from window.location.pathname before navigating (prevents /chat/chat doubling)', () => {
|
||||
jest.useFakeTimers();
|
||||
window.history.replaceState({}, '', '/chat/c/abc123?model=gpt-4');
|
||||
|
||||
renderProviderLive();
|
||||
|
||||
expect(mockRefreshMutate).toHaveBeenCalledTimes(1);
|
||||
const [, refreshOptions] = mockRefreshMutate.mock.calls[0] as [
|
||||
unknown,
|
||||
{ onSuccess: (data: unknown) => void },
|
||||
];
|
||||
|
||||
act(() => {
|
||||
refreshOptions.onSuccess({ user: { id: '1', role: 'USER' }, token: 'new-token' });
|
||||
});
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(100);
|
||||
});
|
||||
|
||||
expect(mockNavigate).toHaveBeenCalledWith('/c/abc123?model=gpt-4', { replace: true });
|
||||
expect(mockNavigate).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining('/chat/c/'),
|
||||
expect.anything(),
|
||||
);
|
||||
jest.useRealTimers();
|
||||
});
|
||||
|
||||
it('falls back to root when window.location.pathname equals the base path', () => {
|
||||
jest.useFakeTimers();
|
||||
window.history.replaceState({}, '', '/chat');
|
||||
|
||||
renderProviderLive();
|
||||
|
||||
const [, refreshOptions] = mockRefreshMutate.mock.calls[0] as [
|
||||
unknown,
|
||||
{ onSuccess: (data: unknown) => void },
|
||||
];
|
||||
|
||||
act(() => {
|
||||
refreshOptions.onSuccess({ user: { id: '1', role: 'USER' }, token: 'new-token' });
|
||||
});
|
||||
act(() => {
|
||||
jest.advanceTimersByTime(100);
|
||||
});
|
||||
|
||||
expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true });
|
||||
jest.useRealTimers();
|
||||
});
|
||||
});
|
||||
|
||||
describe('AuthContextProvider — logout error handling', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
|
|
|
|||
|
|
@ -245,6 +245,37 @@ describe('useAuthRedirect', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should not include basename in redirect_to param (prevents path doubling)', async () => {
|
||||
(useAuthContext as jest.Mock).mockReturnValue({
|
||||
user: null,
|
||||
isAuthenticated: false,
|
||||
});
|
||||
|
||||
/**
|
||||
* Validates that React Router's useLocation() strips the basename before
|
||||
* buildLoginRedirectUrl receives it, so redirect_to never contains
|
||||
* the base prefix. The BASE_URL stripping logic inside buildLoginRedirectUrl
|
||||
* (for callers using window.location.pathname) is tested in
|
||||
* api-endpoints-subdir.spec.ts.
|
||||
*/
|
||||
const router = createTestRouter('/librechat', '/librechat/c/abc123');
|
||||
render(<RouterProvider router={router} />);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(router.state.location.pathname).toBe('/librechat/login');
|
||||
const search = router.state.location.search;
|
||||
const params = new URLSearchParams(search);
|
||||
const redirectTo = decodeURIComponent(params.get('redirect_to')!);
|
||||
/** redirect_to should be /c/abc123, NOT /librechat/c/abc123
|
||||
* because navigate() with basename will re-add the prefix */
|
||||
expect(redirectTo).toBe('/c/abc123');
|
||||
expect(redirectTo).not.toContain('/librechat/');
|
||||
},
|
||||
{ timeout: 1000 },
|
||||
);
|
||||
});
|
||||
|
||||
it('should not append redirect_to when already on /login', async () => {
|
||||
(useAuthContext as jest.Mock).mockReturnValue({
|
||||
user: null,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue