fix(admin): P1-04 AuthGuard race condition — always validate cookie before render
Some checks failed
CI / Lint & TypeCheck (push) Has been cancelled
CI / Unit Tests (push) Has been cancelled
CI / Build Frontend (push) Has been cancelled
CI / Rust Check (push) Has been cancelled
CI / Security Scan (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled

Root cause: loadFromStorage() set isAuthenticated=true from localStorage
without validating the HttpOnly cookie. On page refresh with expired cookie,
children rendered and made failing API calls before AuthGuard could redirect.

Fix:
- authStore: isAuthenticated starts false, never trusted from localStorage
- AuthGuard: always calls GET /auth/me on mount (unless login flow set it)
- Three-state guard (checking/authenticated/unauthenticated) eliminates race
This commit is contained in:
iven
2026-04-10 21:32:14 +08:00
parent 1e675947d5
commit 80b7ee8868
2 changed files with 40 additions and 30 deletions

View File

@@ -3,10 +3,14 @@
// ============================================================ // ============================================================
// //
// Auth strategy: // Auth strategy:
// 1. If Zustand has isAuthenticated=true (normal flow after login) -> authenticated // 1. On first mount, always validate the HttpOnly cookie via GET /auth/me
// 2. If isAuthenticated=false but account in localStorage -> call GET /auth/me // 2. If cookie valid -> restore session and render children
// to validate HttpOnly cookie and restore session
// 3. If cookie invalid -> clean up and redirect to /login // 3. If cookie invalid -> clean up and redirect to /login
// 4. If already authenticated (from login flow) -> render immediately
//
// This eliminates the race condition where localStorage had account data
// but the HttpOnly cookie was expired, causing children to render and
// make failing API calls.
import { useEffect, useRef, useState } from 'react' import { useEffect, useRef, useState } from 'react'
import { Navigate, useLocation } from 'react-router-dom' import { Navigate, useLocation } from 'react-router-dom'
@@ -14,40 +18,44 @@ import { Spin } from 'antd'
import { useAuthStore } from '@/stores/authStore' import { useAuthStore } from '@/stores/authStore'
import { authService } from '@/services/auth' import { authService } from '@/services/auth'
type GuardState = 'checking' | 'authenticated' | 'unauthenticated'
export function AuthGuard({ children }: { children: React.ReactNode }) { export function AuthGuard({ children }: { children: React.ReactNode }) {
const isAuthenticated = useAuthStore((s) => s.isAuthenticated) const isAuthenticated = useAuthStore((s) => s.isAuthenticated)
const account = useAuthStore((s) => s.account)
const login = useAuthStore((s) => s.login) const login = useAuthStore((s) => s.login)
const logout = useAuthStore((s) => s.logout) const logout = useAuthStore((s) => s.logout)
const location = useLocation() const location = useLocation()
// Track restore attempt to avoid double-calling // Track validation attempt to avoid double-calling (React StrictMode)
const restoreAttempted = useRef(false) const validated = useRef(false)
const [restoring, setRestoring] = useState(false) const [guardState, setGuardState] = useState<GuardState>(
isAuthenticated ? 'authenticated' : 'checking'
)
useEffect(() => { useEffect(() => {
if (restoreAttempted.current) return // Already authenticated from login flow — skip validation
restoreAttempted.current = true if (isAuthenticated) {
setGuardState('authenticated')
// If not authenticated but account exists in localStorage, return
// try to validate the HttpOnly cookie via /auth/me
if (!isAuthenticated && account) {
setRestoring(true)
authService.me()
.then((meAccount) => {
// Cookie is valid — restore session
login(meAccount)
setRestoring(false)
})
.catch(() => {
// Cookie expired or invalid — clean up stale data
logout()
setRestoring(false)
})
} }
// Prevent double-validation in React StrictMode
if (validated.current) return
validated.current = true
// Validate HttpOnly cookie via /auth/me
authService.me()
.then((meAccount) => {
login(meAccount)
setGuardState('authenticated')
})
.catch(() => {
logout()
setGuardState('unauthenticated')
})
}, []) // eslint-disable-line react-hooks/exhaustive-deps }, []) // eslint-disable-line react-hooks/exhaustive-deps
if (restoring) { if (guardState === 'checking') {
return ( return (
<div style={{ display: 'flex', justifyContent: 'center', alignItems: 'center', height: '100vh' }}> <div style={{ display: 'flex', justifyContent: 'center', alignItems: 'center', height: '100vh' }}>
<Spin size="large" /> <Spin size="large" />
@@ -55,7 +63,7 @@ export function AuthGuard({ children }: { children: React.ReactNode }) {
) )
} }
if (!isAuthenticated) { if (guardState === 'unauthenticated') {
return <Navigate to="/login" state={{ from: location }} replace /> return <Navigate to="/login" state={{ from: location }} replace />
} }

View File

@@ -37,9 +37,11 @@ function loadFromStorage(): { account: AccountPublic | null; isAuthenticated: bo
if (raw) { if (raw) {
try { account = JSON.parse(raw) } catch { /* ignore */ } try { account = JSON.parse(raw) } catch { /* ignore */ }
} }
// If account exists in localStorage, mark as authenticated (cookie validation // IMPORTANT: Do NOT set isAuthenticated = true from localStorage alone.
// happens in AuthGuard via GET /auth/me — this is just a UI hint) // The HttpOnly cookie must be validated via GET /auth/me before we trust
return { account, isAuthenticated: account !== null } // the session. This prevents the AuthGuard race condition where children
// render and make API calls with an expired cookie.
return { account, isAuthenticated: false }
} }
interface AuthState { interface AuthState {