From d4c0f8d8f30a0e6268ce6fdadb7ca01089316635 Mon Sep 17 00:00:00 2001 From: lobo Date: Thu, 24 Sep 2026 13:29:09 +0200 Subject: [PATCH] feat(auth): tell the user when the session expired Every 401 from Auth or Core now goes through useSessionExpired: the login page says the session expired and returns to the last page after login. App no longer removes a token that only the browser clock calls expired, so a backend 401 is the one signal. A 403 on the profile now shows an error instead. --- public/i18n/de.json | 3 +- public/i18n/en.json | 3 +- src/App.jsx | 5 +- src/App.test.jsx | 55 +++++++++++++++++++ src/components/BacklogCard.jsx | 5 +- src/components/FeedbackAlert.jsx | 9 ++- src/hooks/useForceSync.js | 6 ++ src/hooks/useLibrarySync.js | 9 ++- src/hooks/useSessionExpired.js | 20 +++++++ src/hooks/useSessionExpired.test.jsx | 39 +++++++++++++ src/pages/Dashboard.jsx | 34 +++++++++--- src/pages/Dashboard.test.jsx | 49 +++++++++++++++++ src/pages/GameDetail.jsx | 7 ++- src/pages/GameLibrary.jsx | 8 ++- src/pages/LoginPage.jsx | 16 ++++-- src/pages/LoginPage.test.jsx | 82 ++++++++++++++++++++++++++++ src/pages/SettingsPage.jsx | 10 ++-- src/utils/auth.js | 13 +++++ src/utils/auth.test.js | 13 ++++- 19 files changed, 351 insertions(+), 35 deletions(-) create mode 100644 src/App.test.jsx create mode 100644 src/hooks/useSessionExpired.js create mode 100644 src/hooks/useSessionExpired.test.jsx create mode 100644 src/pages/Dashboard.test.jsx create mode 100644 src/pages/LoginPage.test.jsx diff --git a/public/i18n/de.json b/public/i18n/de.json index f01c24e..ab75cbb 100644 --- a/public/i18n/de.json +++ b/public/i18n/de.json @@ -14,7 +14,8 @@ "ERROR_PASSWORD": "Das Passwort darf nicht leer sein.", "ERROR_INVALID_CREDENTIALS": "E-Mail oder Passwort ist falsch.", "ERROR_TOO_MANY_ATTEMPTS": "Zu viele fehlgeschlagene Versuche. Bitte in {{minutes}} Minuten erneut versuchen.", - "ERROR_GENERIC": "Anmeldung fehlgeschlagen. Bitte später erneut versuchen." + "ERROR_GENERIC": "Anmeldung fehlgeschlagen. Bitte später erneut versuchen.", + "SESSION_EXPIRED": "Deine Sitzung ist abgelaufen. Bitte melde dich erneut an." }, "REGISTRATION": { "TITLE": "Registrieren", diff --git a/public/i18n/en.json b/public/i18n/en.json index fb8cefe..dffe91c 100644 --- a/public/i18n/en.json +++ b/public/i18n/en.json @@ -14,7 +14,8 @@ "ERROR_PASSWORD": "The password must have at least one character.", "ERROR_INVALID_CREDENTIALS": "Email or password is wrong.", "ERROR_TOO_MANY_ATTEMPTS": "Too many failed attempts. Please try again in {{minutes}} minutes.", - "ERROR_GENERIC": "Login failed. Please try again later." + "ERROR_GENERIC": "Login failed. Please try again later.", + "SESSION_EXPIRED": "Your session has expired. Please log in again." }, "REGISTRATION": { "TITLE": "Register", diff --git a/src/App.jsx b/src/App.jsx index 3a66e2d..86675b5 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -13,7 +13,6 @@ import GameDetail from './pages/GameDetail.jsx'; import AppHeader from './components/AppHeader.jsx'; import {Toaster} from '@/components/ui/sonner'; import {TooltipProvider} from '@/components/ui/tooltip'; -import {getValidToken} from './utils/auth.js'; function App() { const {t} = useTranslation(); @@ -24,7 +23,9 @@ function App() { const location = useLocation(); useEffect(() => { - setIsAuthenticated(!!getValidToken()); + // No local expiry check here: a 401 from the backend ends the session (useSessionExpired), and a browser + // clock that runs ahead would otherwise end it silently. + setIsAuthenticated(!!localStorage.getItem('tbp_jwt')); }, [location]); const handleLogout = () => { diff --git a/src/App.test.jsx b/src/App.test.jsx new file mode 100644 index 0000000..691eebe --- /dev/null +++ b/src/App.test.jsx @@ -0,0 +1,55 @@ +// @vitest-environment jsdom +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {act, cleanup, fireEvent, render, screen, waitFor} from '@testing-library/react'; +import {MemoryRouter, useLocation} from 'react-router-dom'; +import App from './App.jsx'; +import {PreferencesProvider} from './context/PreferencesContext.jsx'; + +vi.mock('react-i18next', () => import('./test/i18n-mock.js')); +vi.mock('./hooks/useLibrarySync.js', () => ({default: () => ({status: 'OK', lastSyncedAt: null, isRunning: false})})); +vi.mock('./hooks/useForceSync.js', () => ({default: () => ({trigger: vi.fn(), busy: false})})); + +const DEBOUNCE_WAIT_MS = 700; +const b64 = (o) => btoa(JSON.stringify(o)).replace(/=+$/, ''); +// Expired by the browser clock. A clock that runs ahead of the server makes the backend still accept it. +const EXPIRED_BY_LOCAL_CLOCK = `${b64({alg: 'EdDSA'})}.${b64({exp: Math.floor(Date.now() / 1000) - 60})}.sig`; +const page = {content: [{appid: 220, name: 'Half-Life 2', playtimeMinutes: 0, status: 'BACKLOG'}], page: 0, size: 20, totalElements: 1, totalPages: 1}; + +const wait = (ms) => act(() => new Promise((resolve) => setTimeout(resolve, ms))); + +const current = {location: null}; + +function Spy() { + current.location = useLocation(); + return null; +} + +beforeEach(() => { + localStorage.setItem('tbp_jwt', EXPIRED_BY_LOCAL_CLOCK); + globalThis.fetch = vi.fn(async () => ({ok: true, status: 200, json: async () => page})); +}); + +afterEach(() => { + cleanup(); + localStorage.clear(); +}); + +describe('App', () => { + it('keeps a token that only the browser clock calls expired; a 401 from the backend ends the session', async () => { + render( + + + , + ); + await waitFor(() => expect(screen.getByText('Half-Life 2')).toBeTruthy()); + expect(localStorage.getItem('tbp_jwt')).toBe(EXPIRED_BY_LOCAL_CLOCK); + + fetch.mockImplementation(async () => ({ok: false, status: 401, json: async () => ({})})); + fireEvent.change(screen.getByRole('searchbox'), {target: {value: 'half'}}); + await wait(DEBOUNCE_WAIT_MS); + + await waitFor(() => expect(current.location.pathname).toBe('/login')); + expect(current.location.state).toEqual({sessionExpired: true, from: '/library?q=half&page=0'}); + expect(localStorage.getItem('tbp_jwt')).toBeNull(); + }); +}); diff --git a/src/components/BacklogCard.jsx b/src/components/BacklogCard.jsx index 896896a..81cf210 100644 --- a/src/components/BacklogCard.jsx +++ b/src/components/BacklogCard.jsx @@ -12,6 +12,7 @@ import {Textarea} from '@/components/ui/textarea'; import {cn} from '@/lib/utils'; import {STATUS_META} from './GameStatusBadge.jsx'; import {GAME_PRIORITIES, GAME_STATUSES, NO_PRIORITY, toBacklogRequest} from '../utils/backlog.js'; +import useSessionExpired from '../hooks/useSessionExpired.js'; const NOTES_MAX = 2000; @@ -25,6 +26,7 @@ const toForm = (entry) => ({ export default function BacklogCard({game, onSaved}) { const {t} = useTranslation(); const navigate = useNavigate(); + const expireSession = useSessionExpired(); const [saved, setSaved] = useState(() => toForm(game)); const [form, setForm] = useState(saved); const [busy, setBusy] = useState(false); @@ -46,8 +48,7 @@ export default function BacklogCard({game, onSaved}) { body: JSON.stringify(toBacklogRequest(form)), }); if (response.status === 401) { - localStorage.removeItem('tbp_jwt'); - navigate('/login'); + expireSession(); return; } if (!response.ok) { diff --git a/src/components/FeedbackAlert.jsx b/src/components/FeedbackAlert.jsx index 82d2dc6..eeeb13e 100644 --- a/src/components/FeedbackAlert.jsx +++ b/src/components/FeedbackAlert.jsx @@ -2,12 +2,15 @@ import {CircleAlert, CircleCheck} from 'lucide-react'; import {Alert, AlertDescription} from '@/components/ui/alert'; -export default function FeedbackAlert({message, isError}) { +/** + * @param icon replaces the default icon, e.g. for a note that is neither an error nor a success + */ +export default function FeedbackAlert({message, isError, icon}) { if (!message) return null; - const Icon = isError ? CircleAlert : CircleCheck; + const Icon = icon ?? (isError ? CircleAlert : CircleCheck); return ( - + {message} ); diff --git a/src/hooks/useForceSync.js b/src/hooks/useForceSync.js index 6add492..41d9997 100644 --- a/src/hooks/useForceSync.js +++ b/src/hooks/useForceSync.js @@ -2,6 +2,7 @@ import {useState} from 'react'; import {useNavigate} from 'react-router-dom'; import {useTranslation} from 'react-i18next'; import {toast} from 'sonner'; +import useSessionExpired from './useSessionExpired.js'; const RELOAD_DELAY_MS = 4000; @@ -12,6 +13,7 @@ const RELOAD_DELAY_MS = 4000; export default function useForceSync(onStarted) { const {t} = useTranslation(); const navigate = useNavigate(); + const expireSession = useSessionExpired(); const [busy, setBusy] = useState(false); const trigger = async () => { @@ -26,6 +28,10 @@ export default function useForceSync(onStarted) { method: 'POST', headers: {'Authorization': `Bearer ${token}`}, }); + if (response.status === 401) { + expireSession(); + return; + } if (response.status === 429) { const json = await response.json().catch(() => ({})); toast.warning(t('LIBRARY.SYNC_COOLDOWN', {seconds: json.retryAfterSeconds ?? 60})); diff --git a/src/hooks/useLibrarySync.js b/src/hooks/useLibrarySync.js index 92ff310..bffa834 100644 --- a/src/hooks/useLibrarySync.js +++ b/src/hooks/useLibrarySync.js @@ -13,13 +13,16 @@ const INITIAL_SYNC_GRACE_MS = 15_000; * @param {boolean} opts.expectInitialSync - if true and no row yet, keep polling (covers the * gap between async sync dispatch and first DB write) * @param {() => void} opts.onComplete - called once per transition to OK/FAILED + * @param {() => void} [opts.onUnauthorized] - called on a 401; polling stops * @returns {{status: string|null, lastSyncedAt: string|null, isRunning: boolean, error: string|null}} */ -export default function useLibrarySync({enabled, expectInitialSync = false, onComplete}) { +export default function useLibrarySync({enabled, expectInitialSync = false, onComplete, onUnauthorized}) { const [state, setState] = useState({status: null, lastSyncedAt: null, error: null}); const previousRunningRef = useRef(null); const onCompleteRef = useRef(onComplete); onCompleteRef.current = onComplete; + const onUnauthorizedRef = useRef(onUnauthorized); + onUnauthorizedRef.current = onUnauthorized; useEffect(() => { if (!enabled) return undefined; @@ -39,6 +42,10 @@ export default function useLibrarySync({enabled, expectInitialSync = false, onCo ); if (cancelled) return; + if (response.status === 401) { + onUnauthorizedRef.current?.(); + return; + } if (!response.ok) { setState((s) => ({...s, error: 'sync-status fetch failed'})); return; diff --git a/src/hooks/useSessionExpired.js b/src/hooks/useSessionExpired.js new file mode 100644 index 0000000..88a6ccd --- /dev/null +++ b/src/hooks/useSessionExpired.js @@ -0,0 +1,20 @@ +import {useCallback, useRef} from 'react'; +import {useLocation, useNavigate} from 'react-router-dom'; + +/** + * Returns a function for a 401 answer: it removes the token and opens the login page, which then says that + * the session expired and comes back to the current page after login. The function is as stable as + * `navigate`, so an effect that lists `navigate` can list it too. + */ +export default function useSessionExpired() { + const navigate = useNavigate(); + const location = useLocation(); + const locationRef = useRef(location); + locationRef.current = location; + + return useCallback(() => { + const {pathname, search} = locationRef.current; + localStorage.removeItem('tbp_jwt'); + navigate('/login', {replace: true, state: {sessionExpired: true, from: pathname + search}}); + }, [navigate]); +} diff --git a/src/hooks/useSessionExpired.test.jsx b/src/hooks/useSessionExpired.test.jsx new file mode 100644 index 0000000..3d6c683 --- /dev/null +++ b/src/hooks/useSessionExpired.test.jsx @@ -0,0 +1,39 @@ +// @vitest-environment jsdom +import {afterEach, describe, expect, it} from 'vitest'; +import {cleanup, fireEvent, render, screen} from '@testing-library/react'; +import {MemoryRouter, Route, Routes, useLocation} from 'react-router-dom'; +import useSessionExpired from './useSessionExpired.js'; + +function ExpireButton() { + const expireSession = useSessionExpired(); + return ; +} + +function LoginState() { + return
{JSON.stringify(useLocation().state)}
; +} + +describe('useSessionExpired', () => { + afterEach(() => { + cleanup(); + localStorage.clear(); + }); + + it('removes the token and opens the login page with the current page as return path', () => { + localStorage.setItem('tbp_jwt', 'expired'); + render( + + + }/> + }/> + + , + ); + + fireEvent.click(screen.getByRole('button', {name: 'expire'})); + + expect(JSON.parse(screen.getByTestId('state').textContent)) + .toEqual({sessionExpired: true, from: '/library?status=PLAYING'}); + expect(localStorage.getItem('tbp_jwt')).toBeNull(); + }); +}); diff --git a/src/pages/Dashboard.jsx b/src/pages/Dashboard.jsx index 58ecfc7..be5a1ff 100644 --- a/src/pages/Dashboard.jsx +++ b/src/pages/Dashboard.jsx @@ -16,6 +16,7 @@ import SyncStatusBadge from '../components/SyncStatusBadge.jsx'; import useLibrarySync from '../hooks/useLibrarySync.js'; import useForceSync from '../hooks/useForceSync.js'; import {formatPlaytime} from '../utils/games.js'; +import useSessionExpired from '../hooks/useSessionExpired.js'; const TOP_GAMES = 5; // The games endpoint accepts only the page sizes 20, 40 and 80. @@ -31,6 +32,7 @@ const authHeaders = () => ({ const Dashboard = () => { const {t} = useTranslation(); const navigate = useNavigate(); + const expireSession = useSessionExpired(); const [userData, setUserData] = useState(null); const [steamData, setSteamData] = useState({count: 0, hasSteamId: false}); const [topGames, setTopGames] = useState([]); @@ -44,17 +46,25 @@ const Dashboard = () => { const fetchSteamCount = useCallback(async () => { try { const r = await fetch(`${import.meta.env.VITE_CORE_API_URL}/api/library/games-count`, {headers: authHeaders()}); + if (r.status === 401) { + expireSession(); + return; + } if (r.ok) { setSteamData(await r.json()); } } catch (err) { console.error('Error fetching steam data:', err); } - }, []); + }, [expireSession]); const fetchTopGames = useCallback(async () => { try { const r = await fetch(`${import.meta.env.VITE_CORE_API_URL}${TOP_GAMES_URL}`, {headers: authHeaders()}); + if (r.status === 401) { + expireSession(); + return; + } if (r.ok) { const page = await r.json(); setTopGames((page.content ?? []).slice(0, TOP_GAMES)); @@ -62,11 +72,15 @@ const Dashboard = () => { } catch (err) { console.error('Error fetching top games:', err); } - }, []); + }, [expireSession]); const fetchPlaying = useCallback(async () => { try { const r = await fetch(`${import.meta.env.VITE_CORE_API_URL}${PLAYING_URL}`, {headers: authHeaders()}); + if (r.status === 401) { + expireSession(); + return; + } if (r.ok) { const page = await r.json(); setPlaying((page.content ?? []).slice(0, PLAYING_GAMES)); @@ -74,7 +88,7 @@ const Dashboard = () => { } catch (err) { console.error('Error fetching playing games:', err); } - }, []); + }, [expireSession]); /** * @param excludeAppid the current suggestion, so the new pick differs from it @@ -84,6 +98,10 @@ const Dashboard = () => { const query = excludeAppid ? `?exclude=${excludeAppid}` : ''; try { const r = await fetch(`${import.meta.env.VITE_CORE_API_URL}/api/library/summary${query}`, {headers: authHeaders()}); + if (r.status === 401) { + expireSession(); + return true; + } if (r.ok) { setSummary(await r.json()); setSummaryFailed(false); @@ -94,7 +112,7 @@ const Dashboard = () => { } setSummaryFailed(true); return false; - }, []); + }, [expireSession]); const shuffleSuggestion = async () => { setShuffling(true); @@ -131,9 +149,8 @@ const Dashboard = () => { if (data.steamId) { await Promise.all([fetchSteamCount(), fetchTopGames(), fetchPlaying(), fetchSummary()]); } - } else if (profileResponse.status === 401 || profileResponse.status === 403) { - localStorage.removeItem('tbp_jwt'); - navigate('/login'); + } else if (profileResponse.status === 401) { + expireSession(); } else { setError(t('INFO.ERROR.LOAD_PROFILE')); } @@ -146,13 +163,14 @@ const Dashboard = () => { }; fetchUserData(); - }, [navigate, t, fetchSteamCount, fetchTopGames, fetchPlaying, fetchSummary]); + }, [navigate, expireSession, t, fetchSteamCount, fetchTopGames, fetchPlaying, fetchSummary]); const hasSteamId = !!userData?.steamId; const sync = useLibrarySync({ enabled: hasSteamId, expectInitialSync: hasSteamId && steamData.count === 0, onComplete: reloadLibrary, + onUnauthorized: expireSession, }); const forceSync = useForceSync(reloadLibrary); diff --git a/src/pages/Dashboard.test.jsx b/src/pages/Dashboard.test.jsx new file mode 100644 index 0000000..98eb1d4 --- /dev/null +++ b/src/pages/Dashboard.test.jsx @@ -0,0 +1,49 @@ +// @vitest-environment jsdom +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {cleanup, render, screen, waitFor} from '@testing-library/react'; +import {MemoryRouter, Route, Routes, useLocation} from 'react-router-dom'; +import Dashboard from './Dashboard.jsx'; + +vi.mock('react-i18next', () => import('../test/i18n-mock.js')); +vi.mock('../hooks/useLibrarySync.js', () => ({default: () => ({status: 'OK', lastSyncedAt: null, isRunning: false})})); +vi.mock('../hooks/useForceSync.js', () => ({default: () => ({trigger: vi.fn(), busy: false})})); + +const current = {location: null}; + +function Spy() { + current.location = useLocation(); + return null; +} + +const ok = (body) => ({ok: true, status: 200, json: async () => body}); +const unauthorized = () => ({ok: false, status: 401, json: async () => ({})}); + +beforeEach(() => { + localStorage.setItem('tbp_jwt', 'token'); +}); + +afterEach(() => { + cleanup(); + localStorage.clear(); +}); + +describe('Dashboard', () => { + it('opens the login page when the core API answers 401', async () => { + globalThis.fetch = vi.fn(async (url) => + url.includes('/api/users/me') ? ok({email: 'a@b.de', steamId: '76561198000000000'}) : unauthorized()); + render( + + + }/> + }/> + + , + ); + + await waitFor(() => expect(fetch.mock.calls.length).toBeGreaterThanOrEqual(5)); + + await waitFor(() => expect(current.location.pathname).toBe('/login')); + expect(current.location.state).toEqual({sessionExpired: true, from: '/dashboard'}); + expect(screen.queryByText('LIBRARY.EMPTY')).toBeNull(); + }); +}); diff --git a/src/pages/GameDetail.jsx b/src/pages/GameDetail.jsx index ef21361..9463282 100644 --- a/src/pages/GameDetail.jsx +++ b/src/pages/GameDetail.jsx @@ -12,6 +12,7 @@ import BacklogCard from '../components/BacklogCard.jsx'; import GameCover from '../components/GameCover.jsx'; import GameStatusBadge from '../components/GameStatusBadge.jsx'; import {formatPlaytime} from '../utils/games.js'; +import useSessionExpired from '../hooks/useSessionExpired.js'; const formatUnlockedAt = (instant, locale) => { if (!instant) return ''; @@ -26,6 +27,7 @@ export default function GameDetail() { const {t, i18n} = useTranslation(); const {appid} = useParams(); const navigate = useNavigate(); + const expireSession = useSessionExpired(); const [data, setData] = useState(null); const [isLoading, setIsLoading] = useState(true); @@ -51,8 +53,7 @@ export default function GameDetail() { }); if (cancelled) return; if (response.status === 401) { - localStorage.removeItem('tbp_jwt'); - navigate('/login'); + expireSession(); return; } if (response.status === 404) { @@ -77,7 +78,7 @@ export default function GameDetail() { return () => { cancelled = true; }; - }, [appid, navigate, t]); + }, [appid, navigate, expireSession, t]); const sortedAchievements = useMemo(() => { if (!data?.achievements) return []; diff --git a/src/pages/GameLibrary.jsx b/src/pages/GameLibrary.jsx index 778d404..57bfdbd 100644 --- a/src/pages/GameLibrary.jsx +++ b/src/pages/GameLibrary.jsx @@ -4,6 +4,7 @@ import {useTranslation} from 'react-i18next'; import useLibrarySync from '../hooks/useLibrarySync.js'; import SyncStatusBadge from '../components/SyncStatusBadge.jsx'; import useForceSync from '../hooks/useForceSync.js'; +import useSessionExpired from '../hooks/useSessionExpired.js'; import {formatPlaytime} from '../utils/games.js'; import {GAME_STATUSES, libraryQuery, parseStatus} from '../utils/backlog.js'; import {ArrowUpDown, ChevronLeft, ChevronRight, Clock, Library, RefreshCw, Search, SearchX} from 'lucide-react'; @@ -33,6 +34,7 @@ const SORT_OPTIONS = [ export default function GameLibrary() { const {t} = useTranslation(); const navigate = useNavigate(); + const expireSession = useSessionExpired(); const [searchParams, setSearchParams] = useSearchParams(); const page = Math.max(0, parseInt(searchParams.get('page') ?? '0', 10) || 0); @@ -68,8 +70,7 @@ export default function GameLibrary() { }); if (response.status === 401) { - localStorage.removeItem('tbp_jwt'); - navigate('/login'); + expireSession(); return; } @@ -86,7 +87,7 @@ export default function GameLibrary() { } finally { setIsLoading(false); } - }, [navigate, page, size, sort, status, q, t]); + }, [navigate, expireSession, page, size, sort, status, q, t]); useEffect(() => { load(); @@ -97,6 +98,7 @@ export default function GameLibrary() { enabled: true, expectInitialSync, onComplete: load, + onUnauthorized: expireSession, }); const updateParams = (next) => { diff --git a/src/pages/LoginPage.jsx b/src/pages/LoginPage.jsx index 14b03e4..69e6512 100644 --- a/src/pages/LoginPage.jsx +++ b/src/pages/LoginPage.jsx @@ -1,14 +1,14 @@ import {useTranslation} from 'react-i18next'; import {useState} from 'react'; -import {Link, Navigate, useNavigate} from 'react-router-dom'; -import {Loader2} from 'lucide-react'; +import {Link, Navigate, useLocation, useNavigate} from 'react-router-dom'; +import {Clock, Loader2} from 'lucide-react'; import {Button} from '@/components/ui/button'; import {Input} from '@/components/ui/input'; import {Label} from '@/components/ui/label'; import AuthLayout from '../components/AuthLayout.jsx'; import FeedbackAlert from '../components/FeedbackAlert.jsx'; -import {getValidToken} from '../utils/auth.js'; +import {getValidToken, safeReturnPath} from '../utils/auth.js'; const LOGIN_ERROR_KEY = { INVALID_CREDENTIALS: 'LOGIN.ERROR_INVALID_CREDENTIALS', @@ -18,6 +18,8 @@ const LOGIN_ERROR_KEY = { export default function LoginPage() { const {t} = useTranslation(); const navigate = useNavigate(); + const {state} = useLocation(); + const target = safeReturnPath(state?.from) ?? '/dashboard'; const [email, setEmail] = useState(''); const [password, setPassword] = useState(''); @@ -50,7 +52,7 @@ export default function LoginPage() { if (response.ok) { localStorage.setItem('tbp_jwt', data.token); - navigate('/dashboard', {replace: true}); + navigate(target, {replace: true}); } else { const minutes = Math.ceil(Number(response.headers.get('Retry-After') ?? 900) / 60); setFeedback({ @@ -71,7 +73,7 @@ export default function LoginPage() { }; if (getValidToken()) { - return ; + return ; } return ( @@ -84,6 +86,10 @@ export default function LoginPage() { } > + {state?.sessionExpired && !feedback.message && ( + + )} +
diff --git a/src/pages/LoginPage.test.jsx b/src/pages/LoginPage.test.jsx new file mode 100644 index 0000000..144b87e --- /dev/null +++ b/src/pages/LoginPage.test.jsx @@ -0,0 +1,82 @@ +// @vitest-environment jsdom +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {cleanup, fireEvent, render, screen, waitFor} from '@testing-library/react'; +import {MemoryRouter, Route, Routes, useLocation} from 'react-router-dom'; +import LoginPage from './LoginPage.jsx'; + +vi.mock('react-i18next', () => import('../test/i18n-mock.js')); + +const b64 = (o) => btoa(JSON.stringify(o)).replace(/=+$/, ''); +const TOKEN = `${b64({alg: 'EdDSA'})}.${b64({exp: Math.floor(Date.now() / 1000) + 3600})}.sig`; + +function CurrentPath() { + const location = useLocation(); + return
{location.pathname + location.search}
; +} + +function renderLogin(state) { + render( + + + }/> + }/> + + , + ); +} + +function submit() { + fireEvent.change(screen.getByLabelText('LOGIN.EMAIL'), {target: {value: 'a@b.de'}}); + fireEvent.change(screen.getByLabelText('LOGIN.PASSWORD'), {target: {value: 'secret'}}); + fireEvent.click(screen.getByRole('button', {name: 'LOGIN.SUBMIT'})); +} + +describe('LoginPage', () => { + beforeEach(() => { + globalThis.fetch = vi.fn(async () => ({ok: true, status: 200, json: async () => ({token: TOKEN})})); + }); + + afterEach(() => { + cleanup(); + localStorage.clear(); + }); + + it('says that the session expired', () => { + renderLogin({sessionExpired: true, from: '/library'}); + + expect(screen.getByText('LOGIN.SESSION_EXPIRED')).toBeTruthy(); + }); + + it('shows no message on a normal visit', () => { + renderLogin(undefined); + + expect(screen.queryByText('LOGIN.SESSION_EXPIRED')).toBeNull(); + }); + + it('goes back to the page of the expired session after login', async () => { + renderLogin({sessionExpired: true, from: '/library?status=PLAYING'}); + + submit(); + + await waitFor(() => expect(screen.getByTestId('path').textContent).toBe('/library?status=PLAYING')); + expect(localStorage.getItem('tbp_jwt')).toBe(TOKEN); + }); + + it('ignores a return path outside the app', async () => { + renderLogin({sessionExpired: true, from: '//evil.example'}); + + submit(); + + await waitFor(() => expect(screen.getByTestId('path').textContent).toBe('/dashboard')); + }); + + it('hides the expired message once a login attempt fails', async () => { + fetch.mockResolvedValueOnce({ok: false, status: 401, headers: new Headers(), json: async () => ({code: 'INVALID_CREDENTIALS'})}); + renderLogin({sessionExpired: true, from: '/library'}); + + submit(); + + await waitFor(() => expect(screen.getByText('LOGIN.ERROR_INVALID_CREDENTIALS')).toBeTruthy()); + expect(screen.queryByText('LOGIN.SESSION_EXPIRED')).toBeNull(); + }); +}); diff --git a/src/pages/SettingsPage.jsx b/src/pages/SettingsPage.jsx index d4d7730..45bee6c 100644 --- a/src/pages/SettingsPage.jsx +++ b/src/pages/SettingsPage.jsx @@ -17,6 +17,7 @@ import {Switch} from '@/components/ui/switch'; import FormField from '../components/FormField.jsx'; import {settingsSchema} from '../utils/validation.js'; import {usePreferences} from '../context/PreferencesContext.jsx'; +import useSessionExpired from '../hooks/useSessionExpired.js'; const BACKEND_ERROR_KEY = { STEAM_ID_ALREADY_IN_USE: 'REGISTRATION.ERROR_STEAM_ID_IN_USE', @@ -37,6 +38,7 @@ const formatDate = (value, locale) => { export default function SettingsPage() { const {t, i18n} = useTranslation(); const navigate = useNavigate(); + const expireSession = useSessionExpired(); const {isDarkMode, setDarkMode, setLanguage} = usePreferences(); const [profile, setProfile] = useState(null); @@ -71,8 +73,7 @@ export default function SettingsPage() { }); if (response.status === 401) { - localStorage.removeItem('tbp_jwt'); - navigate('/login'); + expireSession(); return; } @@ -98,7 +99,7 @@ export default function SettingsPage() { }; load(); - }, [navigate, reset, t]); + }, [navigate, expireSession, reset, t]); const persistSettings = async (payload) => { const token = localStorage.getItem('tbp_jwt'); @@ -118,8 +119,7 @@ export default function SettingsPage() { }); if (response.status === 401) { - localStorage.removeItem('tbp_jwt'); - navigate('/login'); + expireSession(); return {ok: false}; } diff --git a/src/utils/auth.js b/src/utils/auth.js index 48a42ad..80b3fc0 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -24,3 +24,16 @@ export function getValidToken() { } return token; } + +/** + * Returns the path when it is a path inside this app, else null. The login page uses it to go back after a + * session expired; "//host", "/\host" and tabs or line breaks (browsers drop them, so "/\t/host" is "//host") + * would leave the app, "/login" would loop. + */ +export function safeReturnPath(path) { + if (typeof path !== 'string' || !path.startsWith('/') || path.startsWith('//') || path.startsWith('/\\') + || /[\t\n\r]/.test(path)) { + return null; + } + return path === '/login' || path.startsWith('/login?') ? null : path; +} diff --git a/src/utils/auth.test.js b/src/utils/auth.test.js index 15631a9..97b6f26 100644 --- a/src/utils/auth.test.js +++ b/src/utils/auth.test.js @@ -1,5 +1,5 @@ import {describe, expect, it} from 'vitest'; -import {isTokenValid} from './auth.js'; +import {isTokenValid, safeReturnPath} from './auth.js'; const tokenWithPayload = (payload) => { const encode = (obj) => btoa(JSON.stringify(obj)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); @@ -25,3 +25,14 @@ describe('isTokenValid', () => { expect(isTokenValid(token, now)).toBe(false); }); }); + +describe('safeReturnPath', () => { + it.each(['/library?status=PLAYING', '/library/220', '/settings'])('keeps the internal path %s', (path) => { + expect(safeReturnPath(path)).toBe(path); + }); + + it.each([undefined, null, '', 'library', '//evil.example', '/\\evil.example', '/\t/evil.example', '/\n/evil.example', 'https://evil.example', '/login', '/login?x=1'])( + 'rejects %j', (path) => { + expect(safeReturnPath(path)).toBeNull(); + }); +});