mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-04-18 05:39:05 +02:00
🔒 fix: Request interceptor for Shared Link Page Scenarios (#12036)
* ♻️ refactor: Centralize `buildLoginRedirectUrl` in data-provider Move `buildLoginRedirectUrl` from `client/src/utils/redirect.ts` into `packages/data-provider/src/api-endpoints.ts` so the axios 401 interceptor (and any other data-provider consumer) can use the canonical implementation with the LOGIN_PATH_RE guard and BASE_URL awareness. The client module now re-exports from `librechat-data-provider`, keeping all existing imports working unchanged. * 🔒 fix: Shared link 401 interceptor bypass and redirect loop (#12033) Fixes three issues in the axios 401 response interceptor that prevented private shared links (ALLOW_SHARED_LINKS_PUBLIC=false) from working: 1. `window.location.href.includes('share/')` matched the full URL (including query params and hash), causing false positives. Changed to `window.location.pathname.startsWith('/share/')`. 2. When token refresh returned no token on a share page, the interceptor logged and fell through without redirecting, causing an infinite retry loop via React Query. Now redirects to login using `buildLoginRedirectUrl()` which preserves the share URL for post-login navigation. 3. `processQueue` was never called in the no-token branch, leaving queued requests with dangling promise callbacks. Added `processQueue(error, null)` before the redirect. * ✅ test: Comprehensive 401 interceptor tests for shared link auth flow Rewrite interceptor test suite to cover all shared link auth scenarios: - Unauthenticated user on share page with failed refresh → redirect - Authenticated user on share page with failed refresh → redirect - share/ in query params does NOT bypass the auth header guard - Login path guard: redirect to plain /login (no redirect_to loop) - Refresh success: assert exact call count (toBe(3) vs toBeGreaterThan) Test reliability improvements: - window.location teardown moved to afterEach (no state leak on failure) - expect.assertions(N) on all tests (catch silent false passes) - Shared setWindowLocation helper for consistent location mocking * ♻️ refactor: Import `buildLoginRedirectUrl` directly from data-provider Update `AuthContext.tsx` and `useAuthRedirect.ts` to import `buildLoginRedirectUrl` from `librechat-data-provider` instead of re-exporting through `~/utils/redirect.ts`. Convert `redirect.ts` to ESM-style inline exports and remove the re-export of `buildLoginRedirectUrl`. * ✅ test: Move `buildLoginRedirectUrl` tests to data-provider Tests for `buildLoginRedirectUrl` now live alongside the implementation in `packages/data-provider/specs/api-endpoints.spec.ts`. Removed the duplicate describe block from the client redirect test file since it no longer owns that function.
This commit is contained in:
parent
23237255d8
commit
619d35360d
9 changed files with 339 additions and 143 deletions
86
packages/data-provider/specs/api-endpoints.spec.ts
Normal file
86
packages/data-provider/specs/api-endpoints.spec.ts
Normal file
|
|
@ -0,0 +1,86 @@
|
|||
/**
|
||||
* @jest-environment jsdom
|
||||
*/
|
||||
import { buildLoginRedirectUrl } from '../src/api-endpoints';
|
||||
|
||||
describe('buildLoginRedirectUrl', () => {
|
||||
let savedLocation: Location;
|
||||
|
||||
beforeEach(() => {
|
||||
savedLocation = window.location;
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: { pathname: '/c/abc123', search: '?model=gpt-4', hash: '#msg-5' },
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(window, 'location', { value: savedLocation, 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');
|
||||
});
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
|
@ -13,20 +13,20 @@ import { setTokenHeader } from '../src/headers-helpers';
|
|||
* a refresh POST or is immediately rejected.
|
||||
*/
|
||||
|
||||
/** Mock the axios adapter to simulate responses without HTTP */
|
||||
const mockAdapter = jest.fn();
|
||||
let originalAdapter: typeof axios.defaults.adapter;
|
||||
let savedLocation: Location;
|
||||
|
||||
beforeAll(async () => {
|
||||
originalAdapter = axios.defaults.adapter;
|
||||
axios.defaults.adapter = mockAdapter;
|
||||
|
||||
/** Import triggers interceptor registration */
|
||||
await import('../src/request');
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
mockAdapter.mockReset();
|
||||
savedLocation = window.location;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
|
|
@ -35,14 +35,24 @@ afterAll(() => {
|
|||
|
||||
afterEach(() => {
|
||||
delete axios.defaults.headers.common['Authorization'];
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: savedLocation,
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
|
||||
function setWindowLocation(overrides: Partial<Location>) {
|
||||
Object.defineProperty(window, 'location', {
|
||||
value: { ...window.location, ...overrides },
|
||||
writable: true,
|
||||
});
|
||||
}
|
||||
|
||||
describe('axios 401 interceptor — Authorization header guard', () => {
|
||||
it('skips refresh and rejects when Authorization header is cleared', async () => {
|
||||
/** Simulate a cleared header (as done by setTokenHeader(undefined) during logout) */
|
||||
expect.assertions(1);
|
||||
setTokenHeader(undefined);
|
||||
|
||||
/** Set up adapter: first call returns 401, second would be the refresh */
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/messages', headers: {} },
|
||||
|
|
@ -54,23 +64,213 @@ describe('axios 401 interceptor — Authorization header guard', () => {
|
|||
// expected rejection
|
||||
}
|
||||
|
||||
/**
|
||||
* If the interceptor skipped refresh, only 1 call was made (the original).
|
||||
* If it attempted refresh, there would be 2+ calls (original + refresh POST).
|
||||
*/
|
||||
expect(mockAdapter).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('attempts refresh when Authorization header is present', async () => {
|
||||
setTokenHeader('valid-token');
|
||||
it('attempts refresh on shared link page even without Authorization header', async () => {
|
||||
expect.assertions(2);
|
||||
setTokenHeader(undefined);
|
||||
|
||||
setWindowLocation({
|
||||
href: 'http://localhost/share/abc123',
|
||||
pathname: '/share/abc123',
|
||||
search: '',
|
||||
hash: '',
|
||||
} as Partial<Location>);
|
||||
|
||||
/** First call: 401 on the original request */
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/messages', headers: {}, _retry: false },
|
||||
config: { url: '/api/share/abc123', headers: {} },
|
||||
});
|
||||
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { token: 'new-token' },
|
||||
status: 200,
|
||||
headers: {},
|
||||
config: {},
|
||||
});
|
||||
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { sharedLink: {} },
|
||||
status: 200,
|
||||
headers: {},
|
||||
config: {},
|
||||
});
|
||||
|
||||
try {
|
||||
await axios.get('/api/share/abc123');
|
||||
} catch {
|
||||
// may reject depending on exact flow
|
||||
}
|
||||
|
||||
expect(mockAdapter.mock.calls.length).toBe(3);
|
||||
|
||||
const refreshCall = mockAdapter.mock.calls[1];
|
||||
expect(refreshCall[0].url).toContain('api/auth/refresh');
|
||||
});
|
||||
|
||||
it('does not bypass guard when share/ appears only in query params', async () => {
|
||||
expect.assertions(1);
|
||||
setTokenHeader(undefined);
|
||||
|
||||
setWindowLocation({
|
||||
href: 'http://localhost/c/chat?ref=share/token',
|
||||
pathname: '/c/chat',
|
||||
search: '?ref=share/token',
|
||||
hash: '',
|
||||
} as Partial<Location>);
|
||||
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/messages', headers: {} },
|
||||
});
|
||||
|
||||
try {
|
||||
await axios.get('/api/messages');
|
||||
} catch {
|
||||
// expected rejection
|
||||
}
|
||||
|
||||
expect(mockAdapter).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('redirects to login with redirect_to when unauthenticated on share page and refresh fails', async () => {
|
||||
expect.assertions(1);
|
||||
setTokenHeader(undefined);
|
||||
|
||||
setWindowLocation({
|
||||
href: 'http://localhost/share/abc123',
|
||||
pathname: '/share/abc123',
|
||||
search: '',
|
||||
hash: '',
|
||||
} as Partial<Location>);
|
||||
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/share/abc123', headers: {} },
|
||||
});
|
||||
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { token: '' },
|
||||
status: 200,
|
||||
headers: {},
|
||||
config: {},
|
||||
});
|
||||
|
||||
try {
|
||||
await axios.get('/api/share/abc123');
|
||||
} catch {
|
||||
// expected rejection
|
||||
}
|
||||
|
||||
expect(window.location.href).toBe('/login?redirect_to=%2Fshare%2Fabc123');
|
||||
});
|
||||
|
||||
it('redirects to login with redirect_to when authenticated and refresh returns no token on share page', async () => {
|
||||
expect.assertions(1);
|
||||
setTokenHeader('some-token');
|
||||
|
||||
setWindowLocation({
|
||||
href: 'http://localhost/share/abc123',
|
||||
pathname: '/share/abc123',
|
||||
search: '',
|
||||
hash: '',
|
||||
} as Partial<Location>);
|
||||
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/share/abc123', headers: {} },
|
||||
});
|
||||
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { token: '' },
|
||||
status: 200,
|
||||
headers: {},
|
||||
config: {},
|
||||
});
|
||||
|
||||
try {
|
||||
await axios.get('/api/share/abc123');
|
||||
} catch {
|
||||
// expected rejection
|
||||
}
|
||||
|
||||
expect(window.location.href).toBe('/login?redirect_to=%2Fshare%2Fabc123');
|
||||
});
|
||||
|
||||
it('redirects to login with redirect_to when refresh returns no token on regular page', async () => {
|
||||
expect.assertions(1);
|
||||
setTokenHeader('some-token');
|
||||
|
||||
setWindowLocation({
|
||||
href: 'http://localhost/c/some-conversation',
|
||||
pathname: '/c/some-conversation',
|
||||
search: '',
|
||||
hash: '',
|
||||
} as Partial<Location>);
|
||||
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/messages', headers: {} },
|
||||
});
|
||||
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { token: '' },
|
||||
status: 200,
|
||||
headers: {},
|
||||
config: {},
|
||||
});
|
||||
|
||||
try {
|
||||
await axios.get('/api/messages');
|
||||
} catch {
|
||||
// expected rejection
|
||||
}
|
||||
|
||||
expect(window.location.href).toBe('/login?redirect_to=%2Fc%2Fsome-conversation');
|
||||
});
|
||||
|
||||
it('redirects to plain /login without redirect_to when already on a login path', async () => {
|
||||
expect.assertions(1);
|
||||
setTokenHeader('some-token');
|
||||
|
||||
setWindowLocation({
|
||||
href: 'http://localhost/login/2fa',
|
||||
pathname: '/login/2fa',
|
||||
search: '',
|
||||
hash: '',
|
||||
} as Partial<Location>);
|
||||
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/messages', headers: {} },
|
||||
});
|
||||
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { token: '' },
|
||||
status: 200,
|
||||
headers: {},
|
||||
config: {},
|
||||
});
|
||||
|
||||
try {
|
||||
await axios.get('/api/messages');
|
||||
} catch {
|
||||
// expected rejection
|
||||
}
|
||||
|
||||
expect(window.location.href).toBe('/login');
|
||||
});
|
||||
|
||||
it('attempts refresh when Authorization header is present', async () => {
|
||||
expect.assertions(2);
|
||||
setTokenHeader('valid-token');
|
||||
|
||||
mockAdapter.mockRejectedValueOnce({
|
||||
response: { status: 401 },
|
||||
config: { url: '/api/messages', headers: {}, _retry: false },
|
||||
});
|
||||
|
||||
/** Second call: the refresh endpoint succeeds */
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { token: 'new-token' },
|
||||
status: 200,
|
||||
|
|
@ -78,7 +278,6 @@ describe('axios 401 interceptor — Authorization header guard', () => {
|
|||
config: {},
|
||||
});
|
||||
|
||||
/** Third call: retried original request succeeds */
|
||||
mockAdapter.mockResolvedValueOnce({
|
||||
data: { messages: [] },
|
||||
status: 200,
|
||||
|
|
@ -92,10 +291,8 @@ describe('axios 401 interceptor — Authorization header guard', () => {
|
|||
// may reject depending on exact flow
|
||||
}
|
||||
|
||||
/** More than 1 call means the interceptor attempted refresh */
|
||||
expect(mockAdapter.mock.calls.length).toBeGreaterThan(1);
|
||||
expect(mockAdapter.mock.calls.length).toBe(3);
|
||||
|
||||
/** Verify the second call targeted the refresh endpoint */
|
||||
const refreshCall = mockAdapter.mock.calls[1];
|
||||
expect(refreshCall[0].url).toContain('api/auth/refresh');
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue