fix: token refresh and seed tree visibility
Fix broken JWT token refresh that caused "Failed to load trees" after idle timeout. The refresh endpoint expected token as query param but frontend sent it as Authorization header. Added proper dependency (get_refresh_token_payload) and refresh queue to handle concurrent 401s. Also fix seed trees not being visible to non-admin users by updating the seed script to set is_public/is_default on existing trees. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -49,6 +49,20 @@ async def get_current_user(
|
||||
return user
|
||||
|
||||
|
||||
async def get_refresh_token_payload(
|
||||
token: Annotated[str, Depends(oauth2_scheme)]
|
||||
) -> dict:
|
||||
"""Extract and validate a refresh token from the Authorization header."""
|
||||
payload = decode_token(token)
|
||||
if payload is None or payload.get("type") != "refresh":
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid refresh token",
|
||||
headers={"WWW-Authenticate": "Bearer"},
|
||||
)
|
||||
return payload
|
||||
|
||||
|
||||
async def get_current_active_user(
|
||||
current_user: Annotated[User, Depends(get_current_user)]
|
||||
) -> User:
|
||||
|
||||
@@ -12,13 +12,12 @@ from app.core.security import (
|
||||
get_password_hash,
|
||||
create_access_token,
|
||||
create_refresh_token,
|
||||
decode_token
|
||||
)
|
||||
from app.models.user import User
|
||||
from app.models.invite_code import InviteCode
|
||||
from app.schemas.user import UserCreate, UserResponse, UserLogin
|
||||
from app.schemas.token import Token
|
||||
from app.api.deps import get_current_user
|
||||
from app.api.deps import get_current_user, get_refresh_token_payload
|
||||
|
||||
router = APIRouter(prefix="/auth", tags=["authentication"])
|
||||
|
||||
@@ -154,17 +153,10 @@ async def login_json(
|
||||
|
||||
@router.post("/refresh", response_model=Token)
|
||||
async def refresh_token(
|
||||
refresh_token: str,
|
||||
payload: Annotated[dict, Depends(get_refresh_token_payload)],
|
||||
db: Annotated[AsyncSession, Depends(get_db)]
|
||||
):
|
||||
"""Refresh access token using refresh token."""
|
||||
payload = decode_token(refresh_token)
|
||||
if payload is None or payload.get("type") != "refresh":
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid refresh token"
|
||||
)
|
||||
|
||||
user_id = payload.get("sub")
|
||||
result = await db.execute(select(User).where(User.id == user_id))
|
||||
user = result.scalar_one_or_none()
|
||||
|
||||
@@ -3327,19 +3327,29 @@ async def create_tree(client: httpx.AsyncClient, token: str, tree_data: dict) ->
|
||||
"""Create a tree via the API. Returns None if tree already exists."""
|
||||
headers = {"Authorization": f"Bearer {token}"}
|
||||
|
||||
# Mark as default/system tree (public and visible to all)
|
||||
tree_data["is_default"] = True
|
||||
tree_data["is_public"] = True
|
||||
|
||||
# Check if tree with same name exists
|
||||
list_response = await client.get(f"{API_BASE_URL}/trees", headers=headers)
|
||||
if list_response.status_code == 200:
|
||||
existing_trees = list_response.json()
|
||||
for tree in existing_trees:
|
||||
if tree["name"] == tree_data["name"]:
|
||||
# Ensure existing trees have correct visibility flags
|
||||
if not tree.get("is_public") or not tree.get("is_default"):
|
||||
patch_response = await client.put(
|
||||
f"{API_BASE_URL}/trees/{tree['id']}",
|
||||
json={"is_public": True, "is_default": True},
|
||||
headers=headers
|
||||
)
|
||||
if patch_response.status_code == 200:
|
||||
print(f" [UPDATE] Tree '{tree_data['name']}' visibility updated (ID: {tree['id']})")
|
||||
return None
|
||||
print(f" [SKIP] Tree '{tree_data['name']}' already exists (ID: {tree['id']})")
|
||||
return None
|
||||
|
||||
# Mark as default/system tree (public and visible to all)
|
||||
tree_data["is_default"] = True
|
||||
tree_data["is_public"] = True
|
||||
|
||||
# Create the tree
|
||||
response = await client.post(
|
||||
f"{API_BASE_URL}/trees",
|
||||
|
||||
81
docs/plans/2026-02-04-token-refresh-fix-design.md
Normal file
81
docs/plans/2026-02-04-token-refresh-fix-design.md
Normal file
@@ -0,0 +1,81 @@
|
||||
# Token Refresh Fix Design
|
||||
|
||||
> **Date:** 2026-02-04
|
||||
> **Status:** Approved
|
||||
> **Issue:** After idle period, app shows "Failed to load trees" with 401 cascade
|
||||
|
||||
---
|
||||
|
||||
## Problem
|
||||
|
||||
When the access token expires (15 minutes), the Axios response interceptor attempts to refresh it by calling `POST /api/v1/auth/refresh`. This call fails because of a request/response mismatch:
|
||||
|
||||
- **Frontend** sends the refresh token in the `Authorization: Bearer <token>` header
|
||||
- **Backend** expects `refresh_token` as a bare `str` query/body parameter
|
||||
|
||||
FastAPI cannot extract a bare string parameter from the Authorization header, so the refresh call returns 422 (missing required parameter). The interceptor catches this as a generic failure, but the error propagates to the calling component rather than redirecting to login — leaving the user stuck on a broken page.
|
||||
|
||||
Additionally, when multiple API calls (trees, folders, categories) all return 401 simultaneously, each independently triggers its own refresh attempt, creating a race condition.
|
||||
|
||||
## Root Cause
|
||||
|
||||
`backend/app/api/endpoints/auth.py:156-158`:
|
||||
```python
|
||||
async def refresh_token(
|
||||
refresh_token: str, # FastAPI expects query/body param
|
||||
db: ...
|
||||
)
|
||||
```
|
||||
|
||||
`frontend/src/api/client.ts:37-41`:
|
||||
```typescript
|
||||
await axios.post(url, null, {
|
||||
headers: { Authorization: `Bearer ${refreshToken}` } // Sent as header
|
||||
})
|
||||
```
|
||||
|
||||
## Solution
|
||||
|
||||
### 1. Backend: Add `get_refresh_token_payload` dependency
|
||||
|
||||
Create a proper FastAPI dependency in `deps.py` that extracts and validates a refresh token from the Authorization header, mirroring the existing `get_current_user` pattern:
|
||||
|
||||
```python
|
||||
async def get_refresh_token_payload(
|
||||
token: Annotated[str, Depends(oauth2_scheme)]
|
||||
) -> dict:
|
||||
payload = decode_token(token)
|
||||
if payload is None or payload.get("type") != "refresh":
|
||||
raise HTTPException(status_code=401, detail="Invalid refresh token")
|
||||
return payload
|
||||
```
|
||||
|
||||
### 2. Backend: Refactor refresh endpoint
|
||||
|
||||
Change `refresh_token()` in `auth.py` to use the new dependency instead of a bare string parameter.
|
||||
|
||||
### 3. Frontend: Add refresh queue (single-flight pattern)
|
||||
|
||||
When multiple requests hit 401 simultaneously, only the first triggers the actual refresh call. Others queue up and retry with the new token once the refresh completes.
|
||||
|
||||
### 4. Frontend: Sync auth store after refresh
|
||||
|
||||
Add a `setTokens` action to `authStore.ts`. Call it from the interceptor after a successful refresh so the Zustand store stays consistent with localStorage.
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `backend/app/api/deps.py` | Add `get_refresh_token_payload` dependency |
|
||||
| `backend/app/api/endpoints/auth.py` | Use new dependency in refresh endpoint |
|
||||
| `frontend/src/api/client.ts` | Refresh queue + auth store sync |
|
||||
| `frontend/src/store/authStore.ts` | Add `setTokens` action |
|
||||
|
||||
## Testing
|
||||
|
||||
- Set `ACCESS_TOKEN_EXPIRE_MINUTES=1` temporarily to reproduce quickly
|
||||
- Verify silent refresh: login, wait >1 min, interact — no errors visible
|
||||
- Verify concurrent requests: page load after expiry triggers one refresh, all requests succeed
|
||||
- Verify refresh token expiry: after 7 days, clean redirect to login
|
||||
- Run existing backend tests (`pytest`) to confirm no regressions
|
||||
- Run frontend build (`npm run build`) to confirm no compile errors
|
||||
@@ -1,4 +1,5 @@
|
||||
import axios, { type AxiosError, type InternalAxiosRequestConfig } from 'axios'
|
||||
import { useAuthStore } from '@/store/authStore'
|
||||
|
||||
const API_BASE_URL = import.meta.env.VITE_API_URL || 'http://localhost:8000'
|
||||
|
||||
@@ -21,45 +22,97 @@ apiClient.interceptors.request.use(
|
||||
(error) => Promise.reject(error)
|
||||
)
|
||||
|
||||
// Refresh queue: when multiple requests hit 401 simultaneously,
|
||||
// only the first triggers the actual refresh. Others wait for the result.
|
||||
let isRefreshing = false
|
||||
let refreshSubscribers: ((token: string) => void)[] = []
|
||||
let refreshFailSubscribers: ((error: unknown) => void)[] = []
|
||||
|
||||
function onRefreshed(token: string) {
|
||||
refreshSubscribers.forEach(cb => cb(token))
|
||||
refreshSubscribers = []
|
||||
refreshFailSubscribers = []
|
||||
}
|
||||
|
||||
function onRefreshFailed(error: unknown) {
|
||||
refreshFailSubscribers.forEach(cb => cb(error))
|
||||
refreshSubscribers = []
|
||||
refreshFailSubscribers = []
|
||||
}
|
||||
|
||||
// Response interceptor - handle token refresh
|
||||
apiClient.interceptors.response.use(
|
||||
(response) => response,
|
||||
async (error: AxiosError) => {
|
||||
const originalRequest = error.config as InternalAxiosRequestConfig & { _retry?: boolean }
|
||||
|
||||
// If 401 and not already retrying, attempt token refresh
|
||||
if (error.response?.status === 401 && !originalRequest._retry) {
|
||||
originalRequest._retry = true
|
||||
|
||||
const refreshToken = localStorage.getItem('refresh_token')
|
||||
if (refreshToken) {
|
||||
try {
|
||||
const response = await axios.post(`${API_BASE_URL}/api/v1/auth/refresh`, null, {
|
||||
headers: {
|
||||
Authorization: `Bearer ${refreshToken}`,
|
||||
},
|
||||
})
|
||||
|
||||
const { access_token, refresh_token } = response.data
|
||||
localStorage.setItem('access_token', access_token)
|
||||
localStorage.setItem('refresh_token', refresh_token)
|
||||
|
||||
// Retry original request with new token
|
||||
if (originalRequest.headers) {
|
||||
originalRequest.headers.Authorization = `Bearer ${access_token}`
|
||||
}
|
||||
return apiClient(originalRequest)
|
||||
} catch (refreshError) {
|
||||
// Refresh failed - clear tokens and redirect to login
|
||||
localStorage.removeItem('access_token')
|
||||
localStorage.removeItem('refresh_token')
|
||||
window.location.href = '/login'
|
||||
return Promise.reject(refreshError)
|
||||
}
|
||||
}
|
||||
// Only handle 401s that haven't already been retried
|
||||
if (error.response?.status !== 401 || originalRequest._retry) {
|
||||
return Promise.reject(error)
|
||||
}
|
||||
|
||||
return Promise.reject(error)
|
||||
originalRequest._retry = true
|
||||
|
||||
const refreshToken = localStorage.getItem('refresh_token')
|
||||
if (!refreshToken) {
|
||||
return Promise.reject(error)
|
||||
}
|
||||
|
||||
// If a refresh is already in progress, queue this request
|
||||
if (isRefreshing) {
|
||||
return new Promise((resolve, reject) => {
|
||||
refreshSubscribers.push((newToken: string) => {
|
||||
if (originalRequest.headers) {
|
||||
originalRequest.headers.Authorization = `Bearer ${newToken}`
|
||||
}
|
||||
resolve(apiClient(originalRequest))
|
||||
})
|
||||
refreshFailSubscribers.push((refreshError: unknown) => {
|
||||
reject(refreshError)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// This is the first 401 — perform the refresh
|
||||
isRefreshing = true
|
||||
|
||||
try {
|
||||
const response = await axios.post(`${API_BASE_URL}/api/v1/auth/refresh`, null, {
|
||||
headers: {
|
||||
Authorization: `Bearer ${refreshToken}`,
|
||||
},
|
||||
})
|
||||
|
||||
const { access_token, refresh_token } = response.data
|
||||
localStorage.setItem('access_token', access_token)
|
||||
localStorage.setItem('refresh_token', refresh_token)
|
||||
|
||||
// Sync Zustand auth store
|
||||
useAuthStore.getState().setTokens({
|
||||
access_token,
|
||||
refresh_token,
|
||||
token_type: 'bearer',
|
||||
})
|
||||
|
||||
isRefreshing = false
|
||||
onRefreshed(access_token)
|
||||
|
||||
// Retry original request with new token
|
||||
if (originalRequest.headers) {
|
||||
originalRequest.headers.Authorization = `Bearer ${access_token}`
|
||||
}
|
||||
return apiClient(originalRequest)
|
||||
} catch (refreshError) {
|
||||
isRefreshing = false
|
||||
onRefreshFailed(refreshError)
|
||||
|
||||
// Refresh failed — clear tokens and redirect to login
|
||||
localStorage.removeItem('access_token')
|
||||
localStorage.removeItem('refresh_token')
|
||||
useAuthStore.getState().logout()
|
||||
window.location.href = '/login'
|
||||
return Promise.reject(refreshError)
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ interface AuthState {
|
||||
register: (data: UserCreate) => Promise<void>
|
||||
logout: () => Promise<void>
|
||||
fetchUser: () => Promise<void>
|
||||
setTokens: (token: Token) => void
|
||||
clearError: () => void
|
||||
setLoading: (loading: boolean) => void
|
||||
}
|
||||
@@ -85,6 +86,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
}
|
||||
},
|
||||
|
||||
setTokens: (token: Token) => set({ token }),
|
||||
clearError: () => set({ error: null }),
|
||||
setLoading: (loading: boolean) => set({ isLoading: loading }),
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user