mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26:47 +00:00
fix(client): a deliberate logout should not book a return ticket
Pressing "log out" clears isAuthenticated while the page you were on is still mounted, so ProtectedRoute re-renders for it and sends you to /login?redirect=<that page>. Log out from the dashboard and the next login goes to the dashboard - beating the startup destination every time after the first, which is exactly how it looked: worked once, never again. A session that ended on its own still gets the return ticket, because coming back to where you were is the whole point of that parameter. A deliberate sign-out is a fresh start and now gets none, so the startup destination decides. The Navbar already hinted at this with its unused noRedirect state; the flag lives in the store instead, since all seven call sites of logout() need it, not just that one.
This commit is contained in:
+26
-1
@@ -1,6 +1,6 @@
|
||||
import React from 'react'
|
||||
import { render, screen, waitFor, act } from '@testing-library/react'
|
||||
import { MemoryRouter } from 'react-router'
|
||||
import { MemoryRouter, useLocation } from 'react-router'
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||
import { http, HttpResponse } from 'msw'
|
||||
import { server } from '../tests/helpers/msw/server'
|
||||
@@ -30,10 +30,19 @@ vi.mock('./hooks/useInAppNotificationListener.ts', () => ({
|
||||
|
||||
// ── Helpers ────────────────────────────────────────────────────────────────────
|
||||
|
||||
/** Reports the router's current location, so a test can assert where App sent it. */
|
||||
function LocationProbe() {
|
||||
const loc = useLocation()
|
||||
return <span data-testid="loc">{loc.pathname + loc.search}</span>
|
||||
}
|
||||
|
||||
const currentPath = () => screen.getByTestId('loc').textContent
|
||||
|
||||
function renderApp(initialPath = '/') {
|
||||
return render(
|
||||
<MemoryRouter initialEntries={[initialPath]}>
|
||||
<App />
|
||||
<LocationProbe />
|
||||
</MemoryRouter>
|
||||
)
|
||||
}
|
||||
@@ -166,6 +175,22 @@ describe('ProtectedRoute — unauthenticated', () => {
|
||||
await waitFor(() => expect(screen.getByText('Login')).toBeInTheDocument())
|
||||
})
|
||||
|
||||
// A session that ran out should return you to the page you were on. Pressing
|
||||
// "log out" should not: clearing isAuthenticated re-renders this route for the
|
||||
// page still on screen, and the ?redirect= it stamped there beat the user's
|
||||
// startup destination on every login after the first.
|
||||
it('FE-COMP-APP-034: keeps the return ticket when the session merely ended', async () => {
|
||||
seedAuth({ isAuthenticated: false, loggingOut: false })
|
||||
renderApp('/trips/1/files')
|
||||
await waitFor(() => expect(currentPath()).toBe('/login?redirect=%2Ftrips%2F1%2Ffiles'))
|
||||
})
|
||||
|
||||
it('FE-COMP-APP-035: drops it on a deliberate sign-out, so the startup destination decides', async () => {
|
||||
seedAuth({ isAuthenticated: false, loggingOut: true })
|
||||
renderApp('/dashboard')
|
||||
await waitFor(() => expect(currentPath()).toBe('/login'))
|
||||
})
|
||||
|
||||
it('FE-COMP-APP-005: /trips/42 redirects to /login when not authenticated', async () => {
|
||||
seedAuth({ isAuthenticated: false })
|
||||
renderApp('/trips/42')
|
||||
|
||||
@@ -82,6 +82,7 @@ function ProtectedRoute({ children, adminRequired = false, addonId }: ProtectedR
|
||||
const user = useAuthStore((s) => s.user)
|
||||
const isLoading = useAuthStore((s) => s.isLoading)
|
||||
const appRequireMfa = useAuthStore((s) => s.appRequireMfa)
|
||||
const loggingOut = useAuthStore((s) => s.loggingOut)
|
||||
const addonStore = useAddonStore()
|
||||
const { t } = useTranslation()
|
||||
const location = useLocation()
|
||||
@@ -99,6 +100,10 @@ function ProtectedRoute({ children, adminRequired = false, addonId }: ProtectedR
|
||||
}
|
||||
|
||||
if (!isAuthenticated) {
|
||||
// A session that ended on its own should come back to where it left off; a
|
||||
// deliberate sign-out is a fresh start and gets no return ticket, so the
|
||||
// startup destination decides where the next login lands.
|
||||
if (loggingOut) return <Navigate to="/login" replace />
|
||||
const redirectParam = encodeURIComponent(location.pathname + location.search + location.hash)
|
||||
return <Navigate to={`/login?redirect=${redirectParam}`} replace />
|
||||
}
|
||||
|
||||
@@ -33,6 +33,10 @@ interface AuthState {
|
||||
* outage doesn't render as a blank, error-free page that looks like lost data.
|
||||
* Transient, never persisted. #1283 */
|
||||
authCheckFailed: boolean
|
||||
/** The user pressed "log out" — as opposed to a session that simply ended.
|
||||
* Read by ProtectedRoute: a deliberate sign-out should not leave a
|
||||
* ?redirect= pointing back at the page they just left. Transient. */
|
||||
loggingOut: boolean
|
||||
error: string | null
|
||||
demoMode: boolean
|
||||
devMode: boolean
|
||||
@@ -95,6 +99,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
isAuthenticated: false,
|
||||
isLoading: true,
|
||||
authCheckFailed: false,
|
||||
loggingOut: false,
|
||||
error: null,
|
||||
demoMode: localStorage.getItem('demo_mode') === 'true',
|
||||
devMode: false,
|
||||
@@ -120,6 +125,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
set({
|
||||
user: data.user,
|
||||
isAuthenticated: true,
|
||||
loggingOut: false,
|
||||
isLoading: false,
|
||||
error: null,
|
||||
})
|
||||
@@ -145,6 +151,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
set({
|
||||
user: data.user,
|
||||
isAuthenticated: true,
|
||||
loggingOut: false,
|
||||
isLoading: false,
|
||||
error: null,
|
||||
})
|
||||
@@ -170,6 +177,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
set({
|
||||
user: data.user,
|
||||
isAuthenticated: true,
|
||||
loggingOut: false,
|
||||
isLoading: false,
|
||||
error: null,
|
||||
})
|
||||
@@ -188,7 +196,11 @@ export const useAuthStore = create<AuthState>()(
|
||||
logout: async () => {
|
||||
// 1. Gate first so any in-flight flush/syncAll bails before we wipe the DB.
|
||||
setAuthed(false)
|
||||
set({ isAuthenticated: false })
|
||||
// Flagged in the same update that drops the session: clearing isAuthenticated
|
||||
// re-renders ProtectedRoute for whatever page is still on screen, and without
|
||||
// this it would stamp a ?redirect= back to it — which then beats the user's
|
||||
// startup destination on the next login.
|
||||
set({ isAuthenticated: false, loggingOut: true })
|
||||
// 2. Stop background sync triggers (30s interval, WS pre-reconnect hook, listeners).
|
||||
unregisterSyncTriggers()
|
||||
// 3. Tear down the live connection.
|
||||
@@ -233,6 +245,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
set({
|
||||
user: data.user,
|
||||
isAuthenticated: true,
|
||||
loggingOut: false,
|
||||
isLoading: false,
|
||||
authCheckFailed: false,
|
||||
})
|
||||
@@ -336,6 +349,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
set({
|
||||
user: data.user,
|
||||
isAuthenticated: true,
|
||||
loggingOut: false,
|
||||
isLoading: false,
|
||||
demoMode: true,
|
||||
error: null,
|
||||
|
||||
Reference in New Issue
Block a user