diff --git a/frontend/src/App.test.tsx b/frontend/src/App.test.tsx index a49322e..3a14683 100644 --- a/frontend/src/App.test.tsx +++ b/frontend/src/App.test.tsx @@ -179,6 +179,49 @@ describe('App', () => { expect(calls.find((c) => c.path === '/api/v1/auth/logout')?.headers['X-CSRF-Token']).toBe('token-1'); }); + describe('a sign-out that does not complete keeps the user signed in and says so', () => { + const failures: [string, () => Response][] = [ + ['the connection fails', () => { + throw new TypeError('Failed to fetch'); + }], + ['the API answers 500', () => problem(500, 'internal-error')], + ['the edge answers 502', () => new Response(null, { status: 502 })], + ['the CSRF token is refused again after reading the session', () => problem(403, 'csrf-invalid')], + ]; + it.each(failures)('when %s', async (_, answer) => { + fakeApi({ + 'GET /api/v1/auth/session': json(200, session()), + 'GET /api/v1/organizations/org-a': organization(['organization.view', 'membership.list']), + 'GET /api/v1/organizations/org-a/members?page=1': members, + 'POST /api/v1/auth/logout': answer, + }); + renderAt('/o/org-a'); + + await userEvent.click(await screen.findByRole('button', { name: 'Sign out' })); + + expect(await screen.findByRole('alert')).toHaveTextContent('Signing out did not complete. You may still be signed in, so try again.'); + expect(screen.getByRole('button', { name: 'Sign out' })).toBeEnabled(); + expect(screen.getByRole('heading', { name: 'Alpha' })).toBeInTheDocument(); + expect(screen.queryByRole('heading', { name: 'Sign in' })).not.toBeInTheDocument(); + }); + }); + + it('treats a 401 to the sign-out as a session that has already ended', async () => { + fakeApi({ + 'GET /api/v1/auth/session': json(200, session()), + 'GET /api/v1/organizations/org-a': organization(['organization.view', 'membership.list']), + 'GET /api/v1/organizations/org-a/members?page=1': members, + 'POST /api/v1/auth/logout': problem(401, 'unauthenticated'), + 'GET /api/v1/system/status': status, + }); + renderAt('/o/org-a'); + + await userEvent.click(await screen.findByRole('button', { name: 'Sign out' })); + + expect(await screen.findByRole('heading', { name: 'Sign in' })).toBeInTheDocument(); + expect(screen.queryByText('Signing out did not complete', { exact: false })).not.toBeInTheDocument(); + }); + it('shows a forbidden answer without data', async () => { fakeApi({ 'GET /api/v1/auth/session': json(200, session()), diff --git a/frontend/src/auth/AuthProvider.tsx b/frontend/src/auth/AuthProvider.tsx index a485f01..c5b280f 100644 --- a/frontend/src/auth/AuthProvider.tsx +++ b/frontend/src/auth/AuthProvider.tsx @@ -1,12 +1,14 @@ import { type ReactNode, useCallback, useEffect, useMemo, useState } from 'react'; import { getSession, login as apiLogin, logout as apiLogout, type Session } from '../api/auth.ts'; -import { onUnauthenticated, setCsrfRefresher, setCsrfToken } from '../api/client.ts'; +import { ApiProblem, onUnauthenticated, setCsrfRefresher, setCsrfToken } from '../api/client.ts'; import { type Auth, AuthContext, type AuthState } from './context.ts'; /** * The authentication state of the SPA (ADR-019): read from * GET /api/v1/auth/session on load, set by login and logout, and reset to * anonymous whenever the API answers 401 (the server ended the session). + * A logout that does not complete leaves the state signed in and rejects: + * the session may still be valid on the server. * The CSRF token is kept in memory only. Nothing is written to browser * storage. */ @@ -62,10 +64,15 @@ export function AuthProvider({ children }: { children: ReactNode }) { const logout = useCallback(async () => { try { await apiLogout(); - } finally { - setCsrfToken(null); - setState({ status: 'anonymous', ended: false }); + } catch (error) { + // A 401: the session had already ended on the server. Anything else + // (no answer, a server error, a refused CSRF token) did not end it. + if (!(error instanceof ApiProblem && error.status === 401)) { + throw error; + } } + setCsrfToken(null); + setState({ status: 'anonymous', ended: false }); }, []); const auth = useMemo(() => ({ state, login, logout, refresh }), [state, login, logout, refresh]); diff --git a/frontend/src/index.css b/frontend/src/index.css index 55a95fd..10c1d71 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -30,6 +30,11 @@ body { padding: 1.5rem; } +.shell-alert { + max-width: 48rem; + margin: 1rem 1.5rem 0; +} + .status dl { display: grid; grid-template-columns: max-content 1fr; diff --git a/frontend/src/pages/AppShell.tsx b/frontend/src/pages/AppShell.tsx index d0de0af..e41648c 100644 --- a/frontend/src/pages/AppShell.tsx +++ b/frontend/src/pages/AppShell.tsx @@ -7,19 +7,25 @@ export function AppShell() { const { state, logout } = useAuth(); const navigate = useNavigate(); const [pending, setPending] = useState(false); + const [failed, setFailed] = useState(false); if (state.status !== 'authenticated') { return null; } const { user, memberships } = state.session; + /** Goes to the login only when the session has ended; otherwise says that it may not have. */ async function signOut() { setPending(true); + setFailed(false); try { await logout(); - } finally { + } catch { setPending(false); - await navigate('/login', { replace: true }); + setFailed(true); + return; } + setPending(false); + await navigate('/login', { replace: true }); } return ( @@ -42,6 +48,11 @@ export function AppShell() { Sign out + {failed && ( +

+ Signing out did not complete. You may still be signed in, so try again. +

+ )}