refactor: Address technical debt in auth refresh implementation

This commit is contained in:
Yunxiao Xu
2026-02-18 14:36:10 -08:00
parent 341bd08176
commit 6131f27142
4 changed files with 49 additions and 25 deletions

View File

@@ -2,7 +2,7 @@ from typing import Optional
from fastapi import APIRouter, Depends, HTTPException, status, Response, Request from fastapi import APIRouter, Depends, HTTPException, status, Response, Request
from fastapi.responses import RedirectResponse from fastapi.responses import RedirectResponse
from fastapi.security import OAuth2PasswordRequestForm from fastapi.security import OAuth2PasswordRequestForm
from ea_chatbot.api.utils import create_access_token, create_refresh_token, settings from ea_chatbot.api.utils import create_access_token, create_refresh_token, decode_access_token, settings
from ea_chatbot.api.dependencies import history_manager, oidc_client, get_current_user from ea_chatbot.api.dependencies import history_manager, oidc_client, get_current_user
from ea_chatbot.history.models import User as UserDB from ea_chatbot.history.models import User as UserDB
from ea_chatbot.api.schemas import Token, UserCreate, UserResponse, ThemeUpdate from ea_chatbot.api.schemas import Token, UserCreate, UserResponse, ThemeUpdate
@@ -101,7 +101,6 @@ async def refresh(request: Request, response: Response):
detail="Refresh token missing" detail="Refresh token missing"
) )
from ea_chatbot.api.utils import decode_access_token # Using decode_access_token for both
payload = decode_access_token(refresh_token) payload = decode_access_token(refresh_token)
if not payload: if not payload:

View File

@@ -18,11 +18,17 @@ def test_refresh_token_success(client):
# 2. Set the cookie manually in the client # 2. Set the cookie manually in the client
client.cookies.set("refresh_token", refresh_token) client.cookies.set("refresh_token", refresh_token)
import time
time.sleep(1.1) # Wait to ensure iat is different
# 3. Call the refresh endpoint with mock # 3. Call the refresh endpoint with mock
with patch("ea_chatbot.api.routers.auth.history_manager") as mock_hm: with patch("ea_chatbot.api.routers.auth.history_manager") as mock_hm, \
patch("ea_chatbot.api.utils.datetime") as mock_datetime:
# Mock datetime to ensure the second token has a different timestamp
from datetime import datetime, timezone, timedelta
base_now = datetime.now(timezone.utc)
# First call (inside refresh) gets base_now + 1 second
mock_datetime.now.return_value = base_now + timedelta(seconds=1)
mock_datetime.side_effect = lambda *args, **kwargs: datetime(*args, **kwargs)
mock_hm.get_user_by_id.return_value = User(id=user_id, username="test@test.com") mock_hm.get_user_by_id.return_value = User(id=user_id, username="test@test.com")
response = client.post("/api/v1/auth/refresh") response = client.post("/api/v1/auth/refresh")

View File

@@ -1,4 +1,4 @@
import { useEffect, useCallback } from 'react' import { useEffect, useCallback, useRef } from 'react'
import { AuthService } from '@/services/auth' import { AuthService } from '@/services/auth'
/** /**
@@ -6,28 +6,42 @@ import { AuthService } from '@/services/auth'
* It proactively refreshes the session to prevent expiration while the user is active. * It proactively refreshes the session to prevent expiration while the user is active.
*/ */
export function useSilentRefresh(isAuthenticated: boolean) { export function useSilentRefresh(isAuthenticated: boolean) {
const timerRef = useRef<NodeJS.Timeout | null>(null)
const refresh = useCallback(async () => { const refresh = useCallback(async () => {
try { try {
console.log('Proactively refreshing session...') console.debug('[Auth] Proactively refreshing session...')
await AuthService.refreshSession() await AuthService.refreshSession()
console.debug('[Auth] Silent refresh successful.')
} catch (error) { } catch (error) {
console.error('Silent refresh failed:', error) console.warn('[Auth] Silent refresh failed:', error)
// If refresh fails, we don't necessarily logout here, // We don't force logout here; the reactive interceptor in api.ts
// as the reactive interceptor will handle 401s if the session is truly dead. // will handle it if a subsequent data request returns a 401.
} }
}, []) }, [])
useEffect(() => { useEffect(() => {
if (!isAuthenticated) return if (!isAuthenticated) {
if (timerRef.current) {
clearInterval(timerRef.current)
timerRef.current = null
}
return
}
// Refresh every 25 minutes (access token defaults to 30 mins) // Refresh every 25 minutes (access token defaults to 30 mins)
const REFRESH_INTERVAL = 25 * 60 * 1000 const REFRESH_INTERVAL = 25 * 60 * 1000
const intervalId = setInterval(refresh, REFRESH_INTERVAL) // Clear existing timer if any
if (timerRef.current) clearInterval(timerRef.current)
timerRef.current = setInterval(refresh, REFRESH_INTERVAL)
// Also refresh immediately on mount/auth if we want to ensure we have a fresh start return () => {
// refresh() if (timerRef.current) {
clearInterval(timerRef.current)
return () => clearInterval(intervalId) timerRef.current = null
}
}
}, [isAuthenticated, refresh]) }, [isAuthenticated, refresh])
} }

View File

@@ -69,22 +69,27 @@ api.interceptors.response.use(
console.error("Reactive refresh failed:", refreshError) console.error("Reactive refresh failed:", refreshError)
// Final failure - session is dead // Final failure - session is dead
if (onUnauthorized) { handleUnauthorized()
onUnauthorized()
}
return Promise.reject(refreshError) return Promise.reject(refreshError)
} }
} }
// If it's the refresh endpoint itself failing with 401, trigger logout // If it's a 401 on an endpoint we don't/can't refresh (like refresh itself or login)
if (error.response?.status === 401 && isRefreshEndpoint) { if (error.response?.status === 401) {
if (onUnauthorized) { handleUnauthorized()
onUnauthorized()
}
} }
return Promise.reject(error) return Promise.reject(error)
} }
) )
/**
* Shared helper to trigger logout/unauthorized cleanup.
*/
function handleUnauthorized() {
if (onUnauthorized) {
onUnauthorized()
}
}
export default api export default api