-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix(web): prevent unauthorized API calls with null token (#634) #669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When users refresh the page or directly access trader pages, SWR immediately
fires API requests before AuthContext finishes loading the token from
localStorage, causing requests with `Authorization: Bearer null`.
**Error Example:**
```
GET /api/account?trader_id=xxxxxx
Authorization: Bearer null
→ {"error": "无效的token: token is malformed: token contains an invalid number of segments"}
```
SWR key conditions only checked `currentPage` and `selectedTraderId`:
```typescript
// ❌ Before: Missing auth check
const { data: account } = useSWR(
currentPage === 'trader' && selectedTraderId ? 'account-...' : null,
() => api.getAccount(...)
)
```
This created a race condition:
1. App renders → SWR evaluates key → fires request
2. AuthContext still loading → `token = null`
3. getAuthHeaders() gets `null` → `Bearer null`
Add `user && token &&` to all authenticated SWR keys:
```typescript
// ✅ After: Wait for authentication
const { data: account } = useSWR(
user && token && currentPage === 'trader' && selectedTraderId
? 'account-...'
: null,
() => api.getAccount(...)
)
```
Fixed 5 SWR calls:
- ✅ `status` (line 92): Added `user && token &&`
- ✅ `account` (line 104): Added `user && token &&`
- ✅ `positions` (line 116): Added `user && token &&`
- ✅ `decisions` (line 128): Added `user && token &&`
- ✅ `stats` (line 140): Added `user && token &&`
- ✅ Import `useAuth` hook
- ✅ Fix `history` SWR (line 37): Added `user && token &&`
- ✅ Fix `account` SWR (line 47): Added `user && token &&`
- ✅ Eliminates "Bearer null" errors on page refresh
- ✅ Prevents malformed token errors when editing traders
- ✅ Consistent with existing pattern (see line 105: traders list already uses this)
- ✅ TypeScript compilation: `npx tsc --noEmit`
- ✅ Pattern verified against working `traders` SWR call
Fixes NoFxAiOS#634
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
🤖 Advisory Check ResultsThese are advisory checks to help improve code quality. They won't block your PR from being merged. 📋 PR InformationTitle Format: ✅ Good - Follows Conventional Commits 🔧 Backend ChecksGo Formatting: Files needing formattingGo Vet: ✅ Good Fix locally: go fmt ./... # Format code
go vet ./... # Check for issues
go test ./... # Run tests⚛️ Frontend ChecksBuild & Type Check: ✅ Success Fix locally: cd web
npm run build # Test build (includes type checking)📖 ResourcesQuestions? Feel free to ask in the comments! 🙏 These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml. |
代码审查报告 - PR #669审查结果:✅ 通过(安全增强)业务层面审查✅ 需求验证
✅ 功能完整性
技术层面审查✅ 问题分析安全漏洞: // Before - 未检查认证状态
const { data: status } = useSWR<SystemStatus>(
currentPage === 'trader' && selectedTraderId
? `status-${selectedTraderId}`
: null,
() => api.getStatus(selectedTraderId),
{ refreshInterval: 10000 }
)
// 问题:
// 1. 即使 token = null,仍然会调用 API
// 2. 后端收到 null token,返回 401
// 3. 前端无限重试(refreshInterval: 10000)
// 4. 产生大量无效请求 ❌影响:
✅ 解决方案添加认证检查: // After - 添加 user && token 检查
const { data: status } = useSWR<SystemStatus>(
user && token && currentPage === 'trader' && selectedTraderId
? `status-${selectedTraderId}`
: null,
() => api.getStatus(selectedTraderId),
{ refreshInterval: 10000 }
)工作原理:
✅ 修改覆盖App.tsx (5处修改):
EquityChart.tsx (2处修改):
总计:7个需要认证的API调用全部保护 ✅ E2E 验证报告✅ 测试场景1:未登录用户✅ 测试场景2:登录用户✅ 测试场景3:登出场景✅ 测试场景4:Token过期代码质量审查✅ 一致性所有检查都使用相同模式: user && token && otherConditions ? key : null顺序正确:
✅ SWR最佳实践条件请求: // ✅ 正确:使用 null key 阻止请求
useSWR(
shouldFetch ? key : null,
fetcher
)
// ❌ 错误:在 fetcher 中检查
useSWR(
key,
() => shouldFetch ? api.get() : null // 仍会调用 fetcher
)本PR采用:✅ 正确模式 ✅ 性能优化减少无效请求:
网络流量节省:
安全审查✅ 认证保护防御层级:
纵深防御 ✅ ✅ 防止信息泄露Before:
After:
✅ 符合最小权限原则
🟡 改进建议(非 BLOCKING)建议1:提取认证检查Hook// src/hooks/useAuthenticatedSWR.ts
export function useAuthenticatedSWR<T>(
key: string | null,
fetcher: () => Promise<T>,
options?: SWRConfiguration
) {
const { user, token } = useAuth()
return useSWR<T>(
user && token && key ? key : null,
fetcher,
options
)
}
// 使用
const { data: status } = useAuthenticatedSWR(
currentPage === 'trader' && selectedTraderId
? `status-${selectedTraderId}`
: null,
() => api.getStatus(selectedTraderId),
{ refreshInterval: 10000 }
)优点:
建议2:添加loading状态const { data, error, isLoading } = useAuthenticatedSWR(...)
if (!user || !token) {
return <LoginPrompt />
}
if (isLoading) {
return <Skeleton />
}
if (error) {
return <ErrorMessage />
}
return <DataDisplay data={data} />建议3:添加TypeScript类型守卫function isAuthenticated(user: User | null, token: string | null): user is User {
return user !== null && token !== null
}
// 使用
const { user, token } = useAuth()
if (!isAuthenticated(user, token)) {
return <LoginPrompt />
}
// TypeScript知道这里 user 一定不是 null
const { data } = useSWR(`status-${user.id}`, ...)建议4:添加单元测试describe('API authentication checks', () => {
it('does not call API when user is not logged in', () => {
const mockFetcher = jest.fn()
renderHook(() =>
useSWR(
user && token && condition ? key : null,
mockFetcher
),
{ wrapper: createWrapper({ user: null, token: null }) }
)
expect(mockFetcher).not.toHaveBeenCalled()
})
it('calls API when user is logged in', () => {
const mockFetcher = jest.fn()
renderHook(() =>
useSWR(
user && token && condition ? key : null,
mockFetcher
),
{ wrapper: createWrapper({ user: {...}, token: 'xxx' }) }
)
expect(mockFetcher).toHaveBeenCalled()
})
})与其他PR的配合与PR #685配合PR #685:HttpClient层拦截401 协同效果:
总结这是一个重要的安全增强 PR: 优点
改进空间(非 BLOCKING)
审查结论:✅ 通过,建议合并 重要性:
影响范围:
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
已修復 go fmt 格式問題 ✅ 可以合併了嗎?謝謝!🙏 |
|
Please resolve the conflict, thanks @zhouyongyou |
…en prevention) - Test SWR key generation with null/undefined user/token - Test multiple API endpoints (status/account/positions/decisions/statistics) - Test EquityChart API guard conditions - Test edge cases (empty strings, undefined, numeric traderId, zero) - Test API call prevention flow All 25 test cases passed, covering: 1. SWR key generation (6 cases): null user/token, wrong page, no traderId, valid case 2. Multiple API endpoints (6 cases): all guarded endpoints + authenticated state 3. EquityChart guard (3 cases): unauthenticated, no traderId, valid 4. Edge cases (6 cases): empty string, undefined, numeric/zero traderId 5. API call prevention (4 cases): null key behavior, SWR simulation Related to PR NoFxAiOS#669 - ensures no unauthorized API calls with null token. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Closing in favor of #880 - a clean rebuild from the latest dev branch without merge conflicts. |
Add `user && token &&` guard to all authenticated SWR calls to prevent requests with `Authorization: Bearer null` when users refresh the page before AuthContext finishes loading. ## Problem When users refresh the page, SWR fires requests before AuthContext loads the token from localStorage, causing "Bearer null" errors. ## Solution Add auth check to SWR key conditions: - ✅ App.tsx (5 SWR calls): status, account, positions, decisions, stats (traders already has guard in upstream/dev) - ✅ EquityChart.tsx (2 SWR calls): history, account - ✅ apiGuard.test.ts: 370 lines of comprehensive unit tests ## Testing - ✅ TypeScript compilation passes for modified files - ✅ 370 lines of unit tests covering all edge cases Fixes NoFxAiOS#634 Supersedes NoFxAiOS#669 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
統一所有繁體中文/簡繁混合注釋為英文,與上游保持一致。 ## 🔴 High Priority (User-Facing) ### 1. web/src/components/TwoStageKeyModal.tsx (4 changes) **User-visible Toast messages (3):** - Line 108: '已复制混淆字符串到剪贴板' → 'Obfuscation string copied to clipboard' - Line 115: '复制失败,请手动复制混淆字符串' → 'Copy failed, please copy the obfuscation string manually' - Line 123: '当前浏览器不支持自动复制,请手动复制' → 'Browser does not support auto-copy, please copy manually' **Comment (1):** - Line 148: '拼接時移除可能的 0x 前綴' → 'Remove possible 0x prefix when concatenating' ### 2. web/src/lib/cache.ts (3 changes) - Line 66: '更新列表(移除已刪除的交易員)' → 'Update list (remove deleted trader)' - Line 68: '清除該交易員的所有緩存' → 'Clear all caches for this trader' - Line 70: '清除包含該交易員的對比圖表緩存' → 'Clear comparison chart caches containing this trader' ## 🟡 Medium Priority (Test Files) ### 3. web/src/components/CompetitionPage.test.tsx (3 changes) - Lines 4-7: Test description (Issue/Fix) - Lines 11-13: Test logic description - Lines 238-240: Leading/trailing message logic ### 4. web/src/lib/apiGuard.test.tsx (3 changes) - Lines 3-7: PR NoFxAiOS#669 test description - Lines 11-13: SWR key generation logic - Lines 351, 355: SWR behavior simulation ### 5. web/src/components/RegisterPage.test.tsx (3 changes) - Lines 11-16: Fix description and test focus - Lines 196-198: Special character consistency - Lines 313-315: Refactoring consistency ### 6. crypto/encryption_test.go (3 changes) - Line 47: '驗證加密後不等於明文' → 'Verify encrypted text is not equal to plaintext' - Line 58: '驗證解密後等於明文' → 'Verify decrypted text equals plaintext' - Line 73: '模擬前端加密私鑰' → 'Simulate frontend encrypted private key' ### 7. trader/partial_close_test.go (7 changes) - Line 117: '驗證計算...' → 'Verify calculation (allow small floating point errors)' - Line 124: '驗證最小倉位檢查邏輯' → 'Verify minimum position check logic' - Line 177: '模擬止盈止損恢復邏輯' → 'Simulate TP/SL restoration logic' - Line 239: '模擬百分比驗證邏輯' → 'Simulate percentage verification logic' - Line 332: '驗證最小倉位檢查' → 'Verify minimum position check' - Line 341: '模擬執行邏輯' → 'Simulate execution logic' - Line 366: '驗證調用' → 'Verify invocation' ## 🟢 Low Priority (Internal Logic) ### 8. logger/decision_logger.go (2 changes) - Line 390: '移除已平仓记录' → 'Remove closed position records' - Line 514: '只在完全平倉時計數' → 'Only count when fully closed' ## Summary - **Total**: 31 changes across 8 files - **User-facing**: 3 toast messages + 4 comments - **Test files**: 19 comments - **Backend**: 5 comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Fixes race condition where SWR fires API requests before AuthContext completes loading authentication token from localStorage, resulting in
Authorization: Bearer nullerrors.Problem
When users refresh the page or directly access trader pages:
tokenstill loading from localStorageAuthorization: Bearer nullError Example:
Root Cause
SWR key conditions only checked page state, not authentication state:
Solution
Add
user && token &&checks to all authenticated SWR keys to ensure requests only fire after authentication completes:Changes
web/src/App.tsxuseAuthhook from AuthContextuser && token &&to 5 authenticated SWR calls:status(line 92)account(line 104)positions(line 116)decisions(line 128)stats(line 140)web/src/components/EquityChart.tsxuseAuthhook from AuthContextuser && token &&to 2 authenticated SWR calls:history(line 37)account(line 47)Impact
Testing
npx tsc --noEmittradersSWR call (App.tsx line 105)Fixes #634
Co-Authored-By: Claude [email protected]
🎯 Type of Change | 變更類型
🔗 Related Issues | 相關 Issue
🧪 Testing | 測試
Test Environment | 測試環境
Manual Testing | 手動測試
🌐 Internationalization | 國際化
✅ Checklist | 檢查清單
Code Quality | 代碼質量
npm run build)npm run lint| 已運行npm run lintDocumentation | 文檔
Git
devbranch | 已 rebase 到最新dev分支By submitting this PR, I confirm | 提交此 PR,我確認:
🌟 Thank you for your contribution! | 感謝你的貢獻!