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
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:
@@ -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 />
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
Reference in New Issue
Block a user