mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-19 07:33:18 +02:00
fix(auth): redirect to /login when JWT expires mid-session (#1698)
When the JWT expired on an open tab, the next API request hit a 401 with
"Token has expired"; client.ts cleared the token from storage but
AuthContext.user stayed populated from the original mount. ProtectedRoute
only redirects when user === null, so the protected tree kept rendering
and every subsequent request silently failed with no Authorization
header — the UI looked like every list was empty until a manual refresh
remounted AuthProvider.
The 3 other setAuthToken(null) sites live inside AuthContext itself and
already pair with setUser(null), so only the client.ts cross-module
site needed a React-tree signal.
- client.ts: after setAuthToken(null) on a token-invalidating 401,
window.dispatchEvent(new CustomEvent('auth:expired')). Guarded on
`typeof window !== 'undefined'` for SSR / test safety. Generic
"Authentication required" 401s still don't clear the token or fire
the event — treated as transient timing issues per the pre-existing
comment at client.ts:155.
- AuthContext.tsx: mount useEffect adds a window listener that calls
setUser(null) under the mountedRef guard; cleanup removes the
listener so unmount → remount doesn't double-bind.
Mirrors the patch the reporter shipped on their fork (deec96d1).
This commit is contained in:
@@ -196,6 +196,55 @@ describe('API Client Auth Header', () => {
|
||||
// Token should NOT be cleared for generic auth errors (might be timing issue)
|
||||
expect(getAuthToken()).toBe('valid-token');
|
||||
});
|
||||
|
||||
it("dispatches 'auth:expired' event on 401 with invalid token message (#1698)", async () => {
|
||||
server.use(
|
||||
http.get('/api/v1/settings/spoolman', () => {
|
||||
return HttpResponse.json(
|
||||
{ detail: 'Token has expired' },
|
||||
{ status: 401 }
|
||||
);
|
||||
})
|
||||
);
|
||||
|
||||
setAuthToken('expired-token');
|
||||
const listener = vi.fn();
|
||||
window.addEventListener('auth:expired', listener);
|
||||
|
||||
try {
|
||||
await api.getSpoolmanSettings();
|
||||
} catch {
|
||||
// Expected to throw
|
||||
}
|
||||
|
||||
expect(listener).toHaveBeenCalledTimes(1);
|
||||
window.removeEventListener('auth:expired', listener);
|
||||
});
|
||||
|
||||
it("does not dispatch 'auth:expired' on 401 with generic auth error (#1698)", async () => {
|
||||
server.use(
|
||||
http.get('/api/v1/settings/spoolman', () => {
|
||||
return HttpResponse.json(
|
||||
{ detail: 'Authentication required' },
|
||||
{ status: 401 }
|
||||
);
|
||||
})
|
||||
);
|
||||
|
||||
setAuthToken('valid-token');
|
||||
const listener = vi.fn();
|
||||
window.addEventListener('auth:expired', listener);
|
||||
|
||||
try {
|
||||
await api.getSpoolmanSettings();
|
||||
} catch {
|
||||
// Expected to throw
|
||||
}
|
||||
|
||||
// Generic 401s might be timing issues, not real expiries — must NOT redirect.
|
||||
expect(listener).not.toHaveBeenCalled();
|
||||
window.removeEventListener('auth:expired', listener);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Slicer download URLs', () => {
|
||||
|
||||
@@ -2,8 +2,8 @@
|
||||
* Tests for the AuthContext permission helpers.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { renderHook, waitFor } from '@testing-library/react';
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { act, renderHook, waitFor } from '@testing-library/react';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import { BrowserRouter } from 'react-router-dom';
|
||||
import { http, HttpResponse } from 'msw';
|
||||
@@ -11,7 +11,7 @@ import { server } from '../mocks/server';
|
||||
import { AuthProvider, useAuth } from '../../contexts/AuthContext';
|
||||
import { ThemeProvider } from '../../contexts/ThemeContext';
|
||||
import { ToastProvider } from '../../contexts/ToastContext';
|
||||
import type { Permission } from '../../api/client';
|
||||
import { setAuthToken, type Permission } from '../../api/client';
|
||||
|
||||
function createWrapper() {
|
||||
const queryClient = new QueryClient({
|
||||
@@ -264,4 +264,65 @@ describe('AuthContext', () => {
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('auth:expired event (#1698)', () => {
|
||||
beforeEach(() => {
|
||||
// authToken is a module-level variable initialised once at import time;
|
||||
// writing to sessionStorage after import doesn't propagate. Use the
|
||||
// canonical setter so checkAuthStatus() finds the token and loads /me.
|
||||
setAuthToken('valid-token');
|
||||
server.use(
|
||||
http.get('/api/v1/auth/status', () => {
|
||||
return HttpResponse.json({
|
||||
auth_enabled: true,
|
||||
requires_setup: false,
|
||||
});
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
setAuthToken(null);
|
||||
});
|
||||
|
||||
it('clears user when an auth:expired event is dispatched', async () => {
|
||||
const { result } = renderHook(() => useAuth(), {
|
||||
wrapper: createWrapper(),
|
||||
});
|
||||
|
||||
// Wait for the initial user load from /auth/me.
|
||||
await waitFor(() => {
|
||||
expect(result.current.user).not.toBeNull();
|
||||
});
|
||||
|
||||
// Simulate client.ts dispatching the event after a 401 + token clear.
|
||||
act(() => {
|
||||
window.dispatchEvent(new CustomEvent('auth:expired'));
|
||||
});
|
||||
|
||||
// User is cleared synchronously — ProtectedRoute (App.tsx:101) sees
|
||||
// user === null and redirects to /login on the next render.
|
||||
await waitFor(() => {
|
||||
expect(result.current.user).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('does not crash when the event fires after unmount', async () => {
|
||||
const { result, unmount } = renderHook(() => useAuth(), {
|
||||
wrapper: createWrapper(),
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.user).not.toBeNull();
|
||||
});
|
||||
|
||||
unmount();
|
||||
|
||||
// mountedRef guards setUser; dispatching after unmount must be a no-op,
|
||||
// not a React state-update-after-unmount warning.
|
||||
expect(() => {
|
||||
window.dispatchEvent(new CustomEvent('auth:expired'));
|
||||
}).not.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -163,6 +163,13 @@ async function request<T>(
|
||||
];
|
||||
if (invalidTokenMessages.some(m => message.includes(m))) {
|
||||
setAuthToken(null);
|
||||
// Notify AuthContext so the protected route guard re-evaluates and
|
||||
// redirects to /login on the same tab — without this, AuthContext.user
|
||||
// stays cached and the tab silently fails every request until a manual
|
||||
// refresh remounts AuthProvider (#1698, reported by @TCL987).
|
||||
if (typeof window !== 'undefined') {
|
||||
window.dispatchEvent(new CustomEvent('auth:expired'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -94,8 +94,20 @@ export function AuthProvider({ children }: { children: React.ReactNode }) {
|
||||
mountedRef.current = true;
|
||||
// Check auth status on mount
|
||||
checkAuthStatus();
|
||||
|
||||
// Listen for token-expiry events from the API client. setAuthToken(null)
|
||||
// in client.ts only clears storage; without this listener, `user` stays
|
||||
// populated and ProtectedRoute keeps rendering the protected tree until a
|
||||
// manual refresh — every request silently fails in the meantime (#1698).
|
||||
const handleAuthExpired = () => {
|
||||
if (!mountedRef.current) return;
|
||||
setUser(null);
|
||||
};
|
||||
window.addEventListener('auth:expired', handleAuthExpired);
|
||||
|
||||
return () => {
|
||||
mountedRef.current = false;
|
||||
window.removeEventListener('auth:expired', handleAuthExpired);
|
||||
};
|
||||
}, []);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user