From cbafa3d5401a0241ddba48ff0b979abb0affa98e Mon Sep 17 00:00:00 2001 From: lobo Date: Wed, 30 Sep 2026 07:36:08 +0200 Subject: [PATCH] refactor(session): one session store for login, logout and 401 --- README.md | 15 ++-- src/App.jsx | 20 ++--- src/components/AppHeader.jsx | 9 +-- src/components/DeleteAccountCard.jsx | 12 +-- src/components/DeleteAccountCard.test.jsx | 16 ++-- src/components/PageContainer.test.jsx | 2 + src/components/RequireSession.jsx | 34 ++------ src/components/RequireSession.test.jsx | 95 +++++++++++------------ src/context/CurrentUserContext.jsx | 57 +++++--------- src/context/CurrentUserContext.test.jsx | 19 ++++- src/context/PreferencesContext.jsx | 16 ++-- src/hooks/useSaveAppearance.test.jsx | 5 +- src/hooks/useSaveResponse.js | 14 +--- src/hooks/useSession.js | 17 ++++ src/hooks/useSessionExpired.js | 26 ------- src/hooks/useSessionExpired.test.jsx | 39 ---------- src/pages/GameLibrary.test.jsx | 4 +- src/pages/LandingPage.jsx | 5 +- src/pages/LoginPage.jsx | 3 - src/pages/PublicProfilePage.jsx | 6 +- src/utils/api.js | 14 ++-- src/utils/api.test.js | 54 +++++++++---- src/utils/auth.js | 44 ++++++++++- 23 files changed, 246 insertions(+), 280 deletions(-) create mode 100644 src/hooks/useSession.js delete mode 100644 src/hooks/useSessionExpired.js delete mode 100644 src/hooks/useSessionExpired.test.jsx diff --git a/README.md b/README.md index c5ad221..65cfac1 100644 --- a/README.md +++ b/README.md @@ -128,14 +128,19 @@ flowchart LR - Calls for the logged-in user go through `src/utils/api.js`. `clientFetch` sends the HttpOnly cookies and the `X-TBP-Client` header. `apiFetch` answers a `401` with one `POST /api/auth/refresh` and sends the request again. Parallel requests and other tabs share this refresh (`navigator.locks`). When the second answer is still a - `401`, `apiFetch` fires `expired`; the pages only stop. `request()` wraps `apiFetch` as `{ok, status, data}` - and never throws. Public pages (landing page, public profile) and the public `GET /api/version` in the settings - use plain `fetch`. + `401`, `apiFetch` ends the session (`expireSession`); the pages only stop. `request()` wraps `apiFetch` as + `{ok, status, data}` and never throws. Public pages (landing page, public profile) and the public + `GET /api/version` in the settings use plain `fetch`. +- `src/utils/auth.js` is the session store: `markSession`, `clearSession` (logout) and `expireSession` (a `401`) + tell every `useSession()`, also on a login or logout in another tab. The user (`CurrentUserProvider`) and the + look (`PreferencesProvider`) follow it. - The pages that need a login sit under the `RequireSession` layout route in `App.jsx`. Without a session it opens - the login page, which comes back to the same page; on `expired` it ends the session (`useSessionExpired`). A - `401` on a public page ends nothing. + the login page: after a `401` with the "session expired" message and back to the same page, after a logout + without a way back. A public page stays open when the session ends there. - `CurrentUserProvider` holds `GET /api/users/me` for the whole app (`useCurrentUserContext`, `useCurrentUser`). Dashboard and settings load it again when they open. +- Tests render through `src/test/renderWithProviders.jsx` (a data router like `main.jsx`) and answer requests + with `src/test/fetchRoutes.js`, which fails a test on any request without a route. - `src/utils/brokenImages.js` reports pictures that fail to load (`GameCover`, achievement icons on the game detail page) to Core. - `useLibrarySync` polls Core's `/api/library/sync-status` while a Steam sync runs, then loads the data again. diff --git a/src/App.jsx b/src/App.jsx index 03f25ad..bb13544 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -1,6 +1,6 @@ import {Link, Route, Routes, useLocation, useNavigate} from 'react-router-dom'; import {useTranslation} from 'react-i18next'; -import {useEffect, useState} from 'react'; +import {useEffect} from 'react'; import LandingPage from './pages/LandingPage'; import LoginPage from './pages/LoginPage'; @@ -23,26 +23,19 @@ import {Toaster} from '@/components/ui/sonner'; import {TooltipProvider} from '@/components/ui/tooltip'; import PageBackground from './components/PageBackground.jsx'; import {usePreferences} from './context/PreferencesContext.jsx'; -import {useCurrentUserActions} from './context/CurrentUserContext.jsx'; -import {clearSession, hasSession} from './utils/auth.js'; +import {clearSession} from './utils/auth.js'; +import useSession from './hooks/useSession.js'; import {logout} from './utils/api.js'; function App() { const {t} = useTranslation(); - const [isAuthenticated, setIsAuthenticated] = useState(false); - const {appearance, syncPreferences, resetAppearance} = usePreferences(); - const {setUser} = useCurrentUserActions(); + const {active: isAuthenticated} = useSession(); + const {appearance, syncPreferences} = usePreferences(); // Hooks for routing const navigate = useNavigate(); const location = useLocation(); - useEffect(() => { - // 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(hasSession()); - }, [location]); - // On login and on page load; the cached look shows until Auth answers. useEffect(() => { if (isAuthenticated) syncPreferences(); @@ -52,9 +45,6 @@ function App() { const handleLogout = async () => { await logout(); clearSession(); - resetAppearance(); - setUser(null); - setIsAuthenticated(false); navigate('/login'); }; diff --git a/src/components/AppHeader.jsx b/src/components/AppHeader.jsx index 2622e54..83120c7 100644 --- a/src/components/AppHeader.jsx +++ b/src/components/AppHeader.jsx @@ -16,16 +16,14 @@ import { DropdownMenuTrigger, } from '@/components/ui/dropdown-menu'; import {usePreferences} from '@/context/PreferencesContext.jsx'; -import {useCurrentUserActions} from '@/context/CurrentUserContext.jsx'; import {cn} from '@/lib/utils'; import Logo from './Logo.jsx'; import PageContainer from './PageContainer.jsx'; import SteamAvatar from './SteamAvatar.jsx'; import useCurrentUser from '../hooks/useCurrentUser.js'; -import useSessionExpired from '../hooks/useSessionExpired.js'; import useSteamProfile from '../hooks/useSteamProfile.js'; import {ROLES, viewAs} from '../utils/admin.js'; -import {markSession} from '../utils/auth.js'; +import {expireSession, markSession} from '../utils/auth.js'; const NAV_ITEMS = [ {to: '/dashboard', labelKey: 'NAV.DASHBOARD', Icon: LayoutDashboard}, @@ -48,9 +46,7 @@ export default function AppHeader({isAuthenticated, onLogout}) { const {pathname} = useLocation(); const steamProfile = useSteamProfile(isAuthenticated); const currentUser = useCurrentUser(isAuthenticated); - const {reload: reloadUser} = useCurrentUserActions(); const navigate = useNavigate(); - const expireSession = useSessionExpired(); const isAdmin = currentUser?.role === 'PLATFORM_ADMIN'; const viewingAs = currentUser && currentUser.activeRole !== currentUser.role ? currentUser.activeRole : null; @@ -58,9 +54,8 @@ export default function AppHeader({isAuthenticated, onLogout}) { try { const response = await viewAs(role === currentUser.role ? null : role); if (response.ok) { - // New cookies with the other role; the new stamp makes the Steam data load again. + // New cookies with the other role; the new stamp loads the user and the Steam data again. markSession(); - reloadUser(); navigate('/dashboard'); return; } diff --git a/src/components/DeleteAccountCard.jsx b/src/components/DeleteAccountCard.jsx index 70a4dc6..c51ac65 100644 --- a/src/components/DeleteAccountCard.jsx +++ b/src/components/DeleteAccountCard.jsx @@ -18,8 +18,6 @@ import { } from '@/components/ui/dialog'; import {Input} from '@/components/ui/input'; import {Label} from '@/components/ui/label'; -import {useResetAppearance} from '../context/PreferencesContext.jsx'; -import {useCurrentUserActions} from '../context/CurrentUserContext.jsx'; import {deleteAccount} from '../utils/account.js'; import {clearSession} from '../utils/auth.js'; @@ -32,8 +30,6 @@ const sameEmail = (typed, email) => !!email && typed.trim().toLowerCase() === em export default function DeleteAccountCard({email}) { const {t} = useTranslation(); const navigate = useNavigate(); - const resetAppearance = useResetAppearance(); - const {setUser} = useCurrentUserActions(); const [typedEmail, setTypedEmail] = useState(''); const [password, setPassword] = useState(''); const [busy, setBusy] = useState(false); @@ -43,11 +39,11 @@ export default function DeleteAccountCard({email}) { try { const response = await deleteAccount(password); if (response.status === 204) { - clearSession(); - resetAppearance(); - setUser(null); toast.success(t('SETTINGS.DELETE_ACCOUNT.DONE')); - navigate('/', {replace: true}); + // First away from the settings, which would open the login page without a session. flushSync: the + // router shows the new page before the session change renders, not later in a transition. + await navigate('/', {replace: true, flushSync: true}); + clearSession(); return; } if (response.status === 403) { diff --git a/src/components/DeleteAccountCard.test.jsx b/src/components/DeleteAccountCard.test.jsx index 09672e1..154c521 100644 --- a/src/components/DeleteAccountCard.test.jsx +++ b/src/components/DeleteAccountCard.test.jsx @@ -1,7 +1,7 @@ // @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} from 'react-router-dom'; +import {createMemoryRouter, RouterProvider} from 'react-router-dom'; import RequireSession from './RequireSession.jsx'; import {toast} from 'sonner'; import DeleteAccountCard from './DeleteAccountCard.jsx'; @@ -14,14 +14,12 @@ const EMAIL = 'player@example.com'; const deleteAnswers = (answer, routes = {}) => fetchRoutes({'DELETE /api/users/me': answer, ...routes}); -const renderCard = () => render( - - - }>}/> - landing

}/> - login

}/> - - ); +// A data router like main.jsx, without the providers: their /me load would change the request order. +const renderCard = () => render(, children: [{path: '/settings', element: }]}, + {path: '/', element:

landing

}, + {path: '/login', element:

login

}, +], {initialEntries: ['/settings']})}/>); const openAndFill = (email, password) => { fireEvent.click(screen.getByRole('button', {name: 'SETTINGS.DELETE_ACCOUNT.BUTTON'})); diff --git a/src/components/PageContainer.test.jsx b/src/components/PageContainer.test.jsx index bd6e3e4..d965cd9 100644 --- a/src/components/PageContainer.test.jsx +++ b/src/components/PageContainer.test.jsx @@ -21,6 +21,8 @@ describe('PageContainer', () => { }); it('is wider with the wide layout', () => { + // The cached look of a logged-in user; without a session it is reset. + localStorage.setItem('tbp_session', '1'); localStorage.setItem('tbp_appearance', JSON.stringify({wideLayout: true})); const container = renderContainer(); diff --git a/src/components/RequireSession.jsx b/src/components/RequireSession.jsx index 5d39b2f..9ed57fb 100644 --- a/src/components/RequireSession.jsx +++ b/src/components/RequireSession.jsx @@ -1,34 +1,16 @@ -import {useEffect, useRef} from 'react'; import {Navigate, Outlet, useLocation} from 'react-router-dom'; -import useSessionExpired from '../hooks/useSessionExpired.js'; -import {sessionEvents} from '../utils/api.js'; -import {hasSession} from '../utils/auth.js'; +import useSession from '../hooks/useSession.js'; /** - * Layout route of the pages that need a login. Without a session it opens the login page, which comes back here; - * a 401 on one of these pages ends the session (see apiFetch). + * Layout route of the pages that need a login. Without a session it opens the login page, which comes back here. + * After a logout it only opens the login page, so the next login does not land on the page of the last user. */ export default function RequireSession() { const {pathname, search} = useLocation(); - const expireSession = useSessionExpired(); - // In a ref, so the listener is added once and not again on every navigation. - const expireRef = useRef(expireSession); - expireRef.current = expireSession; - // A render before the login page shows must not redirect again without the "session expired" state. - const expiring = useRef(false); + const {active, endReason} = useSession(); - useEffect(() => { - // Parallel requests each fire; only the first ends the session. Another tab may have removed the flag already. - const onExpired = () => { - if (expiring.current) return; - expiring.current = true; - expireRef.current(); - }; - sessionEvents.addEventListener('expired', onExpired); - return () => sessionEvents.removeEventListener('expired', onExpired); - }, []); - - return hasSession() || expiring.current - ? - : ; + if (active) return ; + const from = pathname + search; + if (endReason === 'logout') return ; + return ; } diff --git a/src/components/RequireSession.test.jsx b/src/components/RequireSession.test.jsx index d559727..c442df4 100644 --- a/src/components/RequireSession.test.jsx +++ b/src/components/RequireSession.test.jsx @@ -1,88 +1,81 @@ // @vitest-environment jsdom import {afterEach, describe, expect, it} from 'vitest'; import {act, cleanup, render, screen} from '@testing-library/react'; -import {MemoryRouter, Outlet, Route, Routes, useLocation} from 'react-router-dom'; +import {createMemoryRouter, RouterProvider} from 'react-router-dom'; import RequireSession from './RequireSession.jsx'; -import {sessionEvents} from '../utils/api.js'; - -const visitedLogin = []; - -function LoginState() { - const location = useLocation(); - visitedLogin.push(location.key); - return
{JSON.stringify(location.state)}
; -} +import {clearSession, expireSession, markSession} from '../utils/auth.js'; function renderAt(entry) { - render( - - - }> - }> - library

}/> - - profile

}/> - }/> - - - , - ); + const router = createMemoryRouter([ + {element: , children: [{path: '/library', element:

library

}]}, + {path: '/u/:name', element:

profile

}, + {path: '/login', element:

login

}, + ], {initialEntries: [entry]}); + render(); + return router; } -const loginState = () => JSON.parse(screen.getByTestId('state').textContent); -const expire = () => act(() => { - sessionEvents.dispatchEvent(new Event('expired')); -}); +const at = (router) => ({path: router.state.location.pathname, state: router.state.location.state}); afterEach(() => { cleanup(); localStorage.clear(); - visitedLogin.length = 0; }); describe('RequireSession', () => { - it('sends a deep link without a session to the login page, which comes back to it', () => { - renderAt('/library?status=PLAYING'); + it('sends a deep link without a session to the login page, which comes back to it', async () => { + const router = renderAt('/library?status=PLAYING'); - expect(loginState()).toEqual({from: '/library?status=PLAYING'}); + expect(await screen.findByText('login')).toBeTruthy(); + expect(at(router)).toEqual({path: '/login', state: {from: '/library?status=PLAYING'}}); }); it('shows the page with a session', () => { - localStorage.setItem('tbp_session', '1'); + markSession(); renderAt('/library'); expect(screen.getByText('library')).toBeTruthy(); }); - it('ends the session once when requests fail with a 401', () => { - localStorage.setItem('tbp_session', '1'); - renderAt('/library?status=PLAYING'); + it('says the session expired and comes back after a 401', async () => { + markSession(); + const router = renderAt('/library?status=PLAYING'); - expire(); - expire(); + act(() => { + expireSession(); + expireSession(); + }); - expect(loginState()).toEqual({sessionExpired: true, from: '/library?status=PLAYING'}); - expect(new Set(visitedLogin).size).toBe(1); - expect(localStorage.getItem('tbp_session')).toBeNull(); + expect(await screen.findByText('login')).toBeTruthy(); + expect(at(router)).toEqual({path: '/login', state: {sessionExpired: true, from: '/library?status=PLAYING'}}); }); - it('leaves a public page alone on a 401', () => { - localStorage.setItem('tbp_session', '1'); - renderAt('/u/someone'); + it('comes back to no page after a logout, so the next user starts on their own', async () => { + markSession(); + const router = renderAt('/library'); - expire(); + act(() => clearSession()); - expect(screen.getByText('profile')).toBeTruthy(); - expect(localStorage.getItem('tbp_session')).toBe('1'); + expect(await screen.findByText('login')).toBeTruthy(); + expect(router.state.location.state).toBeNull(); }); - it('ends the session when another tab already removed the session flag', () => { - localStorage.setItem('tbp_session', '1'); + + it('opens the login page when another tab logs out', async () => { + markSession(); renderAt('/library'); localStorage.removeItem('tbp_session'); - expire(); + act(() => window.dispatchEvent(new StorageEvent('storage', {key: 'tbp_session'}))); - expect(screen.queryByText('library')).toBeNull(); - expect(loginState()).toEqual({sessionExpired: true, from: '/library'}); + expect(await screen.findByText('login')).toBeTruthy(); + }); + + it('leaves a public page open when the session ends there', () => { + markSession(); + renderAt('/u/someone'); + + act(() => expireSession()); + + expect(screen.getByText('profile')).toBeTruthy(); }); }); diff --git a/src/context/CurrentUserContext.jsx b/src/context/CurrentUserContext.jsx index b4502cf..4e6e405 100644 --- a/src/context/CurrentUserContext.jsx +++ b/src/context/CurrentUserContext.jsx @@ -1,17 +1,16 @@ import {createContext, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; import {request} from '../utils/api.js'; -import {SESSION_KEY, hasSession} from '../utils/auth.js'; +import {hasSession} from '../utils/auth.js'; +import useSession from '../hooks/useSession.js'; import {ME_URL, REQUEST_TIMEOUT_MS} from '../utils/settings.js'; -const UserContext = createContext(null); -// Apart from the user, so a component that only reloads or clears it does not render again on each change. -const ActionsContext = createContext(null); +const CurrentUserContext = createContext(null); /** * GET /api/users/me for the whole app. `user` is undefined until the first answer and null without a session. * `reload()` loads it again and resolves to the `request()` result (not ok for a 200 without a body). - * `updateUser(fn)` changes it locally after a save, and skips it when the user is gone by then; `setUser(null)` - * drops it. A save that replaces settings still loads /me itself (see saveSettings), as it must be fresh. + * `updateUser(fn)` changes it locally after a save, and skips it when the user is gone by then. It follows the + * session. A save that replaces settings still loads /me itself (see saveSettings), as it must be fresh. */ export function CurrentUserProvider({children}) { const [user, setUser] = useState(() => (hasSession() ? undefined : null)); @@ -32,43 +31,29 @@ export function CurrentUserProvider({children}) { return result; }, []); + // A new stamp is a login, a "view as" or a settings save, also in another tab. + const {stamp} = useSession(); useEffect(() => { - if (hasSession()) reload(); - // A login, logout or "view as" in another tab. - const onStorage = (event) => { - if (event.key !== null && event.key !== SESSION_KEY) return; - if (hasSession()) reload(); - else setUser(null); - }; - window.addEventListener('storage', onStorage); - return () => window.removeEventListener('storage', onStorage); - }, [reload]); + if (stamp) { + reload(); + } else { + // A /me answer still on the way must not bring the user back. + latest.current += 1; + setUser(null); + } + }, [stamp, reload]); const updateUser = useCallback((change) => setUser((current) => current && change(current)), []); - const actions = useMemo(() => ({reload, setUser, updateUser}), [reload, updateUser]); + const value = useMemo(() => ({user, reload, updateUser}), [user, reload, updateUser]); - return ( - - {children} - - ); -} - -const NO_ACTIONS = {reload: async () => ({ok: false, status: 0, data: null}), setUser: () => {}, updateUser: () => {}}; - -/** - * `{reload, setUser, updateUser}`, without the user. Outside a provider (e.g. a test of one component) they do - * nothing. - */ -export function useCurrentUserActions() { - return useContext(ActionsContext) ?? NO_ACTIONS; + return {children}; } /** - * `{user, reload, setUser, updateUser}`; outside a provider there is no user. + * `{user, reload, updateUser}`. */ export function useCurrentUserContext() { - const user = useContext(UserContext); - const actions = useContext(ActionsContext); - return {user: actions ? user : null, ...(actions ?? NO_ACTIONS)}; + const value = useContext(CurrentUserContext); + if (!value) throw new Error('useCurrentUserContext must be used inside '); + return value; } diff --git a/src/context/CurrentUserContext.test.jsx b/src/context/CurrentUserContext.test.jsx index 7393159..6d3e3de 100644 --- a/src/context/CurrentUserContext.test.jsx +++ b/src/context/CurrentUserContext.test.jsx @@ -3,6 +3,7 @@ import {afterEach, describe, expect, it, vi} from 'vitest'; import {act, cleanup, render, screen, waitFor} from '@testing-library/react'; import {CurrentUserProvider, useCurrentUserContext} from './CurrentUserContext.jsx'; import useCurrentUser from '../hooks/useCurrentUser.js'; +import {clearSession, markSession} from '../utils/auth.js'; import {fetchRoutes, json} from '../test/fetchRoutes.js'; const context = {current: null}; @@ -70,10 +71,7 @@ describe('CurrentUserProvider', () => { renderTwo(); expect(context.current.user).toBeNull(); - localStorage.setItem('tbp_session', '1'); - act(() => { - context.current.reload(); - }); + act(() => markSession()); expect(context.current.user).toBeUndefined(); await act(async () => answers[0](json(200, {role: 'USER'}))); @@ -122,6 +120,19 @@ describe('CurrentUserProvider', () => { expect(context.current.user).toBeNull(); }); + it('does not bring the user back when /me answers after a logout', async () => { + markSession(); + const answers = []; + fetchRoutes({'GET /api/users/me': () => new Promise((resolve) => answers.push(resolve))}); + renderTwo(); + await waitFor(() => expect(answers).toHaveLength(1)); + + act(() => clearSession()); + await act(async () => answers[0](json(200, {role: 'PLATFORM_ADMIN'}))); + + expect(screen.getByText('header: none')).toBeTruthy(); + }); + it('takes only the answer of the last reload', async () => { localStorage.setItem('tbp_session', '1'); const answers = []; diff --git a/src/context/PreferencesContext.jsx b/src/context/PreferencesContext.jsx index c52131d..39f7147 100644 --- a/src/context/PreferencesContext.jsx +++ b/src/context/PreferencesContext.jsx @@ -10,6 +10,7 @@ import { saveAppearance, } from '../utils/appearance.js'; import {hasSession} from '../utils/auth.js'; +import useSession from '../hooks/useSession.js'; import {LANGUAGES, saveSettings} from '../utils/settings.js'; const DARK_MODE_STORAGE_KEY = 'tbp_dark_mode'; @@ -124,6 +125,12 @@ export function PreferencesProvider({children}) { setAppearanceState(DEFAULT_APPEARANCE); }, []); + // Without a session the next user must not get this look. + const {active} = useSession(); + useEffect(() => { + if (!active) resetAppearance(); + }, [active, resetAppearance]); + /** * Applies the language at once and saves it to Auth while logged in. * @@ -161,12 +168,3 @@ export function usePreferences() { } return ctx; } - -/** - * resetAppearance, or a no-op outside a PreferencesProvider, so hooks used in bare component tests keep working. - */ -export function useResetAppearance() { - return useContext(PreferencesContext)?.resetAppearance ?? noop; -} - -const noop = () => {}; diff --git a/src/hooks/useSaveAppearance.test.jsx b/src/hooks/useSaveAppearance.test.jsx index 744145b..3bda156 100644 --- a/src/hooks/useSaveAppearance.test.jsx +++ b/src/hooks/useSaveAppearance.test.jsx @@ -15,7 +15,10 @@ vi.mock('sonner', () => ({toast: {success: vi.fn(), error: vi.fn()}})); // RequireSession ends the session on a 401, as on the pages that use the hook. const wrapper = ({children}) => ( - }> + + }> + + ); diff --git a/src/hooks/useSaveResponse.js b/src/hooks/useSaveResponse.js index 1c98590..9688f61 100644 --- a/src/hooks/useSaveResponse.js +++ b/src/hooks/useSaveResponse.js @@ -1,24 +1,18 @@ import {useCallback} from 'react'; import {useTranslation} from 'react-i18next'; import {toast} from 'sonner'; -import useSessionExpired from './useSessionExpired.js'; /** - * Returns `run(send)` for a settings save: it awaits `send()` (a fetch) and shows the error message, or ends the - * session on a 401. Resolves to true when saved. + * Returns `run(send)` for a settings save: it awaits `send()` (a fetch) and shows the error message. Resolves to + * true when saved. */ export default function useSaveResponse() { const {t} = useTranslation(); - const expireSession = useSessionExpired(); return useCallback(async (send) => { try { const response = await send(); - if (response.status === 401) { - // Also on the public profile, which has no RequireSession. - expireSession(); - return false; - } + if (response.status === 401) return false; if (!response.ok) { toast.error(t('SETTINGS.ERROR_SAVE')); return false; @@ -29,5 +23,5 @@ export default function useSaveResponse() { toast.error(t('INFO.ERROR.NETWORK_FAILURE')); return false; } - }, [expireSession, t]); + }, [t]); } diff --git a/src/hooks/useSession.js b/src/hooks/useSession.js new file mode 100644 index 0000000..da035dd --- /dev/null +++ b/src/hooks/useSession.js @@ -0,0 +1,17 @@ +import {useMemo, useSyncExternalStore} from 'react'; +import {sessionEndReason, sessionStamp, subscribeSession} from '../utils/auth.js'; + +// A string, so the same session gives the same snapshot. +const snapshot = () => JSON.stringify([sessionStamp(), sessionEndReason()]); + +/** + * The session of this tab: `active`, its `stamp` (new on every login and settings save) and `endReason` + * (null, 'logout' or 'expired'). Renders again when it changes, also through another tab. + */ +export default function useSession() { + const current = useSyncExternalStore(subscribeSession, snapshot); + return useMemo(() => { + const [stamp, endReason] = JSON.parse(current); + return {active: stamp !== null, stamp, endReason}; + }, [current]); +} diff --git a/src/hooks/useSessionExpired.js b/src/hooks/useSessionExpired.js deleted file mode 100644 index 9c72f11..0000000 --- a/src/hooks/useSessionExpired.js +++ /dev/null @@ -1,26 +0,0 @@ -import {useCallback, useRef} from 'react'; -import {useLocation, useNavigate} from 'react-router-dom'; -import {useResetAppearance} from '../context/PreferencesContext.jsx'; -import {useCurrentUserActions} from '../context/CurrentUserContext.jsx'; -import {clearSession} from '../utils/auth.js'; - -/** - * Returns a function for a 401 answer: it removes the token and the user's look and opens the login page, which - * then says that the session expired and comes back to the current page after login. - */ -export default function useSessionExpired() { - const navigate = useNavigate(); - const resetAppearance = useResetAppearance(); - const {setUser} = useCurrentUserActions(); - const location = useLocation(); - const locationRef = useRef(location); - locationRef.current = location; - - return useCallback(() => { - const {pathname, search} = locationRef.current; - clearSession(); - resetAppearance(); - setUser(null); - navigate('/login', {replace: true, state: {sessionExpired: true, from: pathname + search}}); - }, [navigate, resetAppearance, setUser]); -} diff --git a/src/hooks/useSessionExpired.test.jsx b/src/hooks/useSessionExpired.test.jsx deleted file mode 100644 index 3d6c683..0000000 --- a/src/hooks/useSessionExpired.test.jsx +++ /dev/null @@ -1,39 +0,0 @@ -// @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/GameLibrary.test.jsx b/src/pages/GameLibrary.test.jsx index 4d6bac2..4091b91 100644 --- a/src/pages/GameLibrary.test.jsx +++ b/src/pages/GameLibrary.test.jsx @@ -4,6 +4,7 @@ import {act, cleanup, fireEvent, render, screen, waitFor} from '@testing-library import {createMemoryRouter, RouterProvider, useLocation, useNavigate} from 'react-router-dom'; import GameLibrary from './GameLibrary.jsx'; import {PreferencesProvider} from '../context/PreferencesContext.jsx'; +import {CurrentUserProvider} from '../context/CurrentUserContext.jsx'; import {fetchRoutes, json} from '../test/fetchRoutes.js'; vi.mock('react-i18next', () => import('../test/i18n-mock.js')); @@ -14,6 +15,7 @@ const DEBOUNCE_WAIT_MS = 700; const page = {content: [{appid: 220, name: 'Half-Life 2', playtimeMinutes: 0, status: 'BACKLOG'}], page: 0, size: 20, totalElements: 1, totalPages: 1}; const answer = (games = page.content, hasSteamId = true) => fetchRoutes({ + 'GET /api/users/me': json(200, {settings: {}}), 'GET /api/library/games': json(200, {...page, content: games, totalElements: games.length, totalPages: games.length ? 1 : 0}), 'GET /api/library/games-count': json(200, {count: games.length, hasSteamId}), }); @@ -34,7 +36,7 @@ function renderLibrary(initialEntry = '/library') { } const router = createMemoryRouter([{ path: '/library', - element: , + element: , }], {initialEntries: [initialEntry]}); render(); return { diff --git a/src/pages/LandingPage.jsx b/src/pages/LandingPage.jsx index b12982d..78ae349 100644 --- a/src/pages/LandingPage.jsx +++ b/src/pages/LandingPage.jsx @@ -11,7 +11,7 @@ import {Skeleton} from '@/components/ui/skeleton'; import GameCover from '../components/GameCover.jsx'; import Logo from '../components/Logo.jsx'; import StatCard from '../components/StatCard.jsx'; -import {hasSession} from '../utils/auth.js'; +import useSession from '../hooks/useSession.js'; import usePublicStats from '../hooks/usePublicStats.js'; @@ -39,6 +39,7 @@ function RankedGames({title, games, formatValue}) { export default function LandingPage() { const {t, i18n} = useTranslation(); const {data, isLoading} = usePublicStats(); + const {active} = useSession(); const auth = data.auth; const core = data.core; @@ -65,7 +66,7 @@ export default function LandingPage() {

{t('MAIN.WELCOME')}

{t('MAIN.DESCRIPTION')}

)} - setEditing(false)}/> diff --git a/src/utils/api.js b/src/utils/api.js index 955f930..c64182f 100644 --- a/src/utils/api.js +++ b/src/utils/api.js @@ -1,3 +1,5 @@ +import {expireSession, sessionStamp} from './auth.js'; + // Auth and Core refuse a request that changes data without this header (CSRF guard, see ClientHeaderFilter). export const CLIENT_HEADER = 'X-TBP-Client'; @@ -54,21 +56,17 @@ export async function rotatingFetch(url, init = {}) { return globalThis.navigator?.locks ? navigator.locks.request('tbp-refresh', send) : send(); } -/** - * Fires `expired` when the session is over. RequireSession ends it, so the user sees why and comes back to the same - * page; a public page does not listen. - */ -export const sessionEvents = new EventTarget(); - /** * fetch for the logged-in user. A 401 renews the access cookie once and sends the request again. A 401 after that - * goes back to the caller and fires `expired`; the caller only stops. + * ends the session (RequireSession then opens the login page, a public page stays) and goes back to the caller, + * which only stops. */ export async function apiFetch(url, init = {}) { + const stamp = sessionStamp(); let response = await clientFetch(url, init); if (response.status === 401) { if (await refreshSession()) response = await clientFetch(url, init); - if (response.status === 401) sessionEvents.dispatchEvent(new Event('expired')); + if (response.status === 401) expireSession(stamp); } return response; } diff --git a/src/utils/api.test.js b/src/utils/api.test.js index 6bac5e3..4351d29 100644 --- a/src/utils/api.test.js +++ b/src/utils/api.test.js @@ -1,6 +1,7 @@ // @vitest-environment jsdom import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; -import {apiFetch, clientFetch, logout, request, sessionEvents} from './api.js'; +import {apiFetch, clientFetch, logout, request} from './api.js'; +import {clearSession, markSession, sessionEndReason, subscribeSession} from './auth.js'; const response = (status) => ({ok: status >= 200 && status < 300, status, json: async () => ({})}); const isRefresh = (url) => url.endsWith('/api/auth/refresh'); @@ -59,7 +60,7 @@ describe('apiFetch', () => { expect(refreshCalls()).toHaveLength(1); }); - it('returns the 401 when the refresh fails, without a loop', async () => { + it('returns the 401 when the refresh fails, without a loop, and ends the session', async () => { globalThis.fetch = vi.fn(async () => response(401)); const result = await apiFetch('/api/x'); @@ -67,7 +68,7 @@ describe('apiFetch', () => { expect(result.status).toBe(401); expect(refreshCalls()).toHaveLength(1); expect(dataCalls()).toHaveLength(1); - expect(localStorage.getItem('tbp_session')).toBe('1'); + expect(localStorage.getItem('tbp_session')).toBeNull(); }); it('keeps the session and rejects when the refresh has no network', async () => { @@ -126,36 +127,56 @@ describe('apiFetch', () => { }); }); -describe('the expired event', () => { - let expired; - const count = () => expired.mock.calls.length; +describe('the session end on a 401', () => { + let changes; + let unsubscribe; beforeEach(() => { - expired = vi.fn(); - sessionEvents.addEventListener('expired', expired); + markSession(); + changes = vi.fn(); + unsubscribe = subscribeSession(changes); }); afterEach(() => { - sessionEvents.removeEventListener('expired', expired); + unsubscribe(); }); - it('fires once when the refresh says the session is over', async () => { + it('ends the session once when the refresh says it is over', async () => { globalThis.fetch = vi.fn(async () => response(401)); - await apiFetch('/api/x'); + await Promise.all([apiFetch('/api/x'), apiFetch('/api/y')]); - expect(count()).toBe(1); + expect(localStorage.getItem('tbp_session')).toBeNull(); + expect(sessionEndReason()).toBe('expired'); + expect(changes).toHaveBeenCalledTimes(1); }); - it('fires when the request after a good refresh still gets a 401', async () => { + it('keeps a new login when the refresh of the session before it fails late', async () => { + let failRefresh; + globalThis.fetch = vi.fn((url) => isRefresh(url) + ? new Promise((resolve) => { failRefresh = () => resolve(response(401)); }) + : Promise.resolve(response(401))); + + const pending = apiFetch('/api/x'); + await vi.waitFor(() => expect(failRefresh).toBeDefined()); + clearSession(); + markSession(); + failRefresh(); + await pending; + + expect(localStorage.getItem('tbp_session')).not.toBeNull(); + expect(sessionEndReason()).toBeNull(); + }); + + it('ends it when the request after a good refresh still gets a 401', async () => { globalThis.fetch = vi.fn(async (url) => response(isRefresh(url) ? 204 : 401)); await apiFetch('/api/x'); - expect(count()).toBe(1); + expect(sessionEndReason()).toBe('expired'); }); - it('does not fire when the refresh has no network, or for clientFetch', async () => { + it('keeps it when the refresh has no network, and for clientFetch', async () => { globalThis.fetch = vi.fn(async (url) => { if (isRefresh(url)) throw new TypeError('Failed to fetch'); return response(401); @@ -164,7 +185,8 @@ describe('the expired event', () => { await expect(apiFetch('/api/x')).rejects.toThrow(); await clientFetch('/api/x'); - expect(count()).toBe(0); + expect(localStorage.getItem('tbp_session')).not.toBeNull(); + expect(changes).not.toHaveBeenCalled(); }); }); diff --git a/src/utils/auth.js b/src/utils/auth.js index 3ea5f58..19174b5 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -11,16 +11,58 @@ export const hasSession = () => localStorage.getItem(SESSION_KEY) !== null; */ export const sessionStamp = () => localStorage.getItem(SESSION_KEY); +const listeners = new Set(); +// Why the session of this tab ended: null (never had one, or another tab ended it), 'logout' or 'expired'. +let endReason = null; + +const change = (reason) => { + endReason = reason; + listeners.forEach((listener) => listener()); +}; + +export const sessionEndReason = () => endReason; + +const onStorage = (event) => { + if (event.key === SESSION_KEY || event.key === null) change(null); +}; + export const markSession = () => { - localStorage.setItem(SESSION_KEY, String(Date.now())); + // New for every login, also two in the same millisecond: apiFetch tells sessions apart by it. + localStorage.setItem(SESSION_KEY, crypto.randomUUID()); localStorage.removeItem(LEGACY_TOKEN_KEY); + change(null); }; export const clearSession = () => { localStorage.removeItem(SESSION_KEY); localStorage.removeItem(LEGACY_TOKEN_KEY); + change('logout'); }; +/** + * Ends the session after a 401 that the refresh could not fix; the login page then says why. Parallel requests may + * each call it. Only the session with `stamp` can expire: a late answer for the session before a new login must not + * end the new one. + */ +export const expireSession = (stamp = sessionStamp()) => { + if (stamp === null || sessionStamp() !== stamp) return; + localStorage.removeItem(SESSION_KEY); + localStorage.removeItem(LEGACY_TOKEN_KEY); + change('expired'); +}; + +/** + * Calls `listener` on every change of the session, also on a login or logout in another tab. + */ +export function subscribeSession(listener) { + if (listeners.size === 0) window.addEventListener('storage', onStorage); + listeners.add(listener); + return () => { + listeners.delete(listener); + if (listeners.size === 0) window.removeEventListener('storage', onStorage); + }; +} + /** * 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")