user-system/docs/review-fix-closure-2026-05-28.md

# user-system review 修复收口（2026-05-28）

## 结论
本轮已完成 review 报告相关最高优先级前端/E2E blocker 修复，并完成后端、前端、E2E 三层验证。

当前状态：
- 最高优先级 blocker：已修复
- Go 全量测试：通过
- 前端全量测试：通过（82 files, 522 tests）
- Playwright CDP 全链路 E2E：通过

## 本轮修复项

### 1. 会话恢复 / refresh 竞态
- 问题：`AuthProvider` 初始恢复会话与 HTTP client 401 重试路径会并发触发 `/auth/refresh`，在 refresh token 轮换模型下导致 `401`。
- 修复：前端改为共享 single-flight refresh。
- 涉及文件：
  - `frontend/admin/src/lib/http/client.ts`
  - `frontend/admin/src/services/auth.ts`
  - `frontend/admin/src/services/auth.test.ts`

### 2. 用户列表响应结构漂移
- 问题：后端 `/users` 返回 `{ users, total, limit, offset }`，前端只按 `items` 读取，导致页面空表。
- 修复：增加 users 列表 normalize，兼容 `items/users` 和 `page_size/limit/offset`。
- 涉及文件：
  - `frontend/admin/src/services/users.ts`
  - `frontend/admin/src/services/users.test.ts`

### 3. Webhooks 列表响应结构漂移
- 问题：Webhooks 页加载时报 `Cannot read properties of undefined (reading 'map')`。
- 修复：兼容 `data/items/webhooks` 多种列表包裹形状。
- 涉及文件：
  - `frontend/admin/src/services/webhooks.ts`
  - `frontend/admin/src/services/webhooks.test.ts`

### 4. Social accounts 响应结构漂移
- 问题：ProfileSecurityPage 报 `socialAccounts.map is not a function`。
- 修复：兼容 `array/items/accounts/social_accounts` 形状。
- 涉及文件：
  - `frontend/admin/src/services/social-accounts.ts`
  - `frontend/admin/src/services/social-accounts.test.ts`

### 5. Playwright CDP E2E harness 漂移
- 修复点包括：
  - refresh token 断言从可读 cookie 改为 HttpOnly cookie / session presence 真相
  - `创建用员` 文案 typo
  - responsive 场景后 viewport 未恢复
  - drawer 选择器 strict mode 冲突
  - delete confirm 由 modal 漂移为 popconfirm
  - 菜单分组/路由漂移：设备、审计日志、Webhooks、profile/security
  - 多处页面断言从宽文本改为更稳定选择器
- 涉及文件：
  - `frontend/admin/scripts/run-playwright-cdp-e2e.mjs`
  - `frontend/admin/scripts/run-playwright-auth-e2e.sh`

### 6. E2E 限流误伤
- 问题：测试流量触发 API rate limit，导致后续场景误报。
- 修复：为 E2E backend 增加 `DISABLE_RATE_LIMIT=1` 开关，仅用于测试启动脚本。
- 涉及文件：
  - `internal/api/middleware/ratelimit.go`
  - `frontend/admin/scripts/run-playwright-auth-e2e.sh`

### 7. 内存限流器全局误伤与条目泄漏风险
- 问题：`internal/api/middleware/ratelimit.go` 之前按 endpoint 只创建单一 limiter，导致同一接口上的所有用户共享一个桶；同时缺少空闲条目清理策略，无法对历史 client key 做收敛。
- 修复：改为按 `endpoint + user_id/IP` 分桶，并在访问路径上按 TTL 清理长期空闲的 limiter 条目。
- 回归测试：
  - 不同 IP 的登录限流相互独立
  - 共享 IP 下不同 `user_id` 的 API 限流相互独立
  - 空闲 limiter 会被清理，不再无限累积
- 涉及文件：
  - `internal/api/middleware/ratelimit.go`
  - `internal/api/middleware/ratelimit_test.go`

### 8. handler context 类型断言补强
- 问题：`SSOHandler` 与 `WebhookHandler` 仍存在 `user_id.(int64)` / `username.(string)` 直接断言，若 middleware 注入异常类型会触发 panic。
- 修复：统一复用 `getUserIDFromContext` / `getUsernameFromContext`，类型不匹配时返回 `401 unauthorized`，避免 handler panic。
- 回归测试：
  - `SSOHandler.Authorize` 非法 context 类型返回 `401`
  - `SSOHandler.UserInfo` 非法 context 类型返回 `401`
  - `WebhookHandler.CreateWebhook/ListWebhooks` 非法 context 类型返回 `401`
- 涉及文件：
  - `internal/api/handler/auth_handler.go`
  - `internal/api/handler/sso_handler.go`
  - `internal/api/handler/webhook_handler.go`
  - `internal/api/handler/context_guard_test.go`

### 9. 密码强度 + 静默错误补强
- 问题：review 报告中指出两类尾部问题：
  - 默认密码校验对刚好达到最小长度的短密码过于宽松
  - TOTP / 操作日志链路存在 `_ = err`、`_ = json.Unmarshal(...)`、`_ = repo.Create(...)` 这类静默吞错
- 修复：
  - `validatePasswordStrength` 改为对“刚好达到最小长度”的密码要求至少 3 种字符类型；较长密码仍保留 2 种类型可过的兼容行为
  - `TOTPService` 对恢复码摘要、JSON 编解码、`UpdateTOTP` 持久化失败全部显式返回错误，不再静默忽略
  - `OperationLogMiddleware` 对 nil repo fail-safe 返回；异步落库失败改为写日志，不再无声吞错
- 回归测试：
  - 8 位两类字符密码被拒绝，8 位三类字符密码通过，较长两类字符密码仍通过
  - 损坏的恢复码 JSON 会返回解析错误
  - 恢复码消费后持久化失败会显式返回更新错误
  - operation log 在 nil repo 情况下不会 panic，参数脱敏/非 JSON fallback 继续受测
- 涉及文件：
  - `internal/service/auth.go`
  - `internal/service/auth_service_test.go`
  - `internal/service/auth_password_internal_test.go`
  - `internal/service/totp.go`
  - `internal/service/totp_internal_test.go`
  - `internal/api/middleware/operation_log.go`
  - `internal/api/middleware/operation_log_test.go`

### 10. review 报告真相校准 + avatar 路径硬化
- 真相校准：`PROJECT_REVIEW_REPORT.md` 中一批条目已不再代表当前仓库真相，至少包括：
  - `uploadAvatar` 字段名错误：前后端当前都使用 `avatar`，该条为陈旧误报
  - `StateManager` 无法停止、`L1Cache` 无容量限制、密码强度过宽松、操作日志未转义、Webhooks 客户端全量分页、`ContactBindingsSection` 未复用：均已在后续提交中关闭
- 本轮额外修复：
  - 将头像上传目录从运行时相对路径解析改为绝对路径归一化，避免 cwd 漂移导致文件落盘位置不稳定
  - 扩展名校验统一转小写，避免 `.JPG/.PNG` 这类常见文件名被误拒
- 回归测试：
  - `resolveAvatarUploadDir("")` 返回绝对路径且收敛到 `/uploads/avatars`
  - 自定义根目录会被保留并归一化到 `<root>/avatars`
- 涉及文件：
  - `internal/api/handler/avatar_handler.go`
  - `internal/api/handler/avatar_handler_path_test.go`

### 11. ApiResponse 空值建模校准
- 问题：`frontend/admin/src/types/http.ts` 之前把 `ApiResponse.data` 固定定义为 `T`，但真实后端在成功/失败分支都可能返回 `data: null`，导致类型真相偏乐观。
- 修复：
  - 将 `ApiResponse<T>.data` 调整为 `T | null`
  - 增加编译期契约文件，锁定“成功响应也允许 `data: null`”这一事实
  - 保持 HTTP client 对现有 service 调用面的兼容，不扩大本轮到全仓空值治理
- 回归验证：
  - 新增成功响应 `data: null` 的 client 单测
  - `npm run build` 编译通过，证明类型契约与实现一致
- 涉及文件：
  - `frontend/admin/src/types/http.ts`
  - `frontend/admin/src/types/http.typecheck.ts`
  - `frontend/admin/src/lib/http/client.ts`
  - `frontend/admin/src/lib/http/client.test.ts`

### 12. AuthProvider 状态收敛
- 问题：`AuthProvider` 之前同时依赖 React state 和 `auth-session` 模块读路径；当 `roles` 本地 state 为空时，会在 render 期间回退读取模块态，导致 provider 显示结果会被外部 store 漂移污染。
- 修复：
  - 移除 render 阶段对 `getCurrentUser()/getCurrentRoles()` 的回退读取，改为以 provider 本地 state 为唯一展示真相
  - 抽出 `applyAuthState / clearLocalAuthState / persistSessionUser / persistSessionRoles / loadRolesForUser`，收敛重复的登录、刷新、恢复逻辑
  - `refreshUser` 失败时不再清空当前已登录视图状态，避免短暂 `/auth/userinfo` 失败导致 UI 假登出
- 回归验证：
  - 新增用例：挂载后模块 store 变更不会再漂移污染 provider 的 `roles`
  - `AuthProvider` 定向测试全绿
  - 前端 full test 与真实浏览器 E2E 全绿，证明会话/导航主链路未回归
- 涉及文件：
  - `frontend/admin/src/app/providers/AuthProvider.tsx`
  - `frontend/admin/src/app/providers/AuthProvider.test.tsx`

### 13. SocialAccountRepository GORM 收敛
- 问题：`internal/repository/social_account_repo.go` 曾长期绕过仓库层通用 GORM 模式，直接持有 `*sql.DB` 并手写 CRUD SQL，导致仓库风格与其余实现不一致。
- 修复：
  - `SocialAccountRepositoryImpl` 改为统一持有 `*gorm.DB`
  - Create / Update / Delete / 查询 / 分页全部改为 GORM 链式调用
  - 保留 `*sql.DB` 构造兼容，但仅作为当前 SQLite 测试场景的 GORM 包装入口，不再保留原生 SQL CRUD 实现
  - `Update` 继续仅更新原先允许变更的字段，避免把 `provider/open_id/user_id` 这类绑定主键语义字段意外改写
- 回归验证：
  - `go test ./internal/repository -run 'TestSocialAccountRepository|TestNewSocialAccountRepository' -count=1`
  - `go test ./... -count=1`
  - `go vet ./...`
  - `go build ./cmd/server`
- 涉及文件：
  - `internal/repository/social_account_repo.go`

## 验证结果

### 后端
- 命令：`go test ./...`
- 结果：通过

### 前端
- 命令：`npm test -- --runInBand`
- 结果：通过
- 统计：`82 passed`, `522 passed`

### E2E
- 命令：`npm run e2e:full`
- 结果：通过
- 结论：`Playwright CDP E2E completed successfully`

## 闭环判断

### 实现闭环
已完成。本轮识别出的真实 blocker 均已修复。

### 证据闭环
已完成。Go 全量测试、前端全量测试、CDP E2E 全部通过。

### 文档真相闭环
已完成。本文件记录了问题、修复、验证与当前结论。

### 防复发闭环
已部分完成：
- 已为 users/webhooks/social-accounts 响应结构漂移补 service-level normalize + tests
- 已把 refresh 单飞与 E2E harness 漂移修复固化
- 后续建议：把 E2E 页面导航/断言进一步抽象为页面对象或稳定 helper，减少文案/菜单变动带来的连锁断言漂移