fix: v6 code review P0 auth/IDOR fixes + frontend regression patches

Backend fixes: - auth_handler: P0 认证逻辑修复 - ratelimit: 限速中间件增强 + 新增单元测试 - auth_service: 认证服务逻辑完善 + 新增测试 - server: server 配置增强 + 新增测试 - handler_test: 新增 handler 层集成测试 - auth_bootstrap_test: bootstrap 路径测试 Frontend patches: - LoginPage/RegisterPage: CSRF + 表单交互修复 - BootstrapAdminPage: 引导流程修复 - DevicesPage: 设备管理页修复 - auth/social-accounts/users/webhooks services: 类型修正 - csrf.ts: CSRF token 处理修正 - E2E 脚本: CDP smoke + auth e2e 增强 Docs: - FULL_CODE_REVIEW_REPORT_2026-04-20 - report-v6 执行计划 - REAL_PROJECT_STATUS 更新 - .gitignore: 新增 .gocache-*/config.yaml 排除验证: go build/vet 0错误, go test 42/42 PASS, 0 FAIL
2026-04-23 07:14:12 +08:00
parent 82109ec216
commit 3f3bb82f1d
41 changed files with 2681 additions and 283 deletions
--- a/docs/code-review/FULL_CODE_REVIEW_REPORT_2026-04-20.md
+++ b/docs/code-review/FULL_CODE_REVIEW_REPORT_2026-04-20.md
@@ -0,0 +1,340 @@
+# UMS 项目全面代码复核报告 v6.0
+
+**报告日期**: 2026-04-20  
+**审查范围**: 当前 `main` 工作区全部实现代码、旧报告未闭环问题、自动化门禁、系统化静态审查结果  
+**基线说明**: 本报告按日期拆分，作为 [FULL_CODE_REVIEW_REPORT_2026-04-17.md](./FULL_CODE_REVIEW_REPORT_2026-04-17.md) 的后续复核报告。凡与旧报告或旧附录冲突之处，以本报告基于 2026-04-20 新鲜命令证据和当前代码实现得到的结论为准。  
+
+---
+
+## 一句话结论
+
+项目在 2026-04-17 报告中的多数首轮 P0 缺陷已经被修复，但当前代码仍存在新的认证与授权断层，且旧报告中的一部分未修复问题仍未真正闭环。当前状态不适合宣称“全部问题已修完”或“可直接上线”。
+
+---
+
+## 2026-04-20 新鲜验证证据
+
+| 项目 | 命令 | 结果 | 说明 |
+|---|---|---|---|
+| 后端构建 | `go build ./cmd/server` | PASS | 2026-04-20 23:07:51 +08:00 实跑通过 |
+| 后端静态检查 | `go vet ./...` | PASS | 实跑通过 |
+| 后端测试 | `go test ./... -count=1` | PASS | 全量通过，`internal/service` 仍是主要耗时段 |
+| 前端 Lint | `cd frontend/admin && npm.cmd run lint` | PASS | 与 2026-04-18 红灯状态相比已恢复 |
+| 前端构建 | `cd frontend/admin && npm.cmd run build` | PASS | 实跑通过 |
+| 系统化静态检查 | `staticcheck ./...` | FAIL | 发现测试代码 `nil context`、潜在空指针、死代码等问题 |
+| 安全静态检查 | `gosec ./internal/... ./cmd/...` | FAIL | 有真实问题，也有大量误报/高噪音结果，需要人工过滤 |
+
+---
+
+## 当前阻塞级问题
+
+### P0-01: `TOTP` 二次验证链路缺少首因子绑定，形成独立登录入口
+
+**位置**
+- `internal/api/handler/auth_handler.go:151`
+- `internal/service/auth.go:125`
+- `internal/service/auth.go:811`
+
+**问题**
+- `/api/v1/auth/login/totp-verify` 只要求 `user_id + code + device_id`
+- 服务端 `VerifyTOTPAfterPasswordLogin()` 只校验用户状态与 `TOTP` 码，然后直接签发完整 token
+- 代码里虽然保留了 `TempToken` 字段，但当前登录闭环并未使用任何临时登录态或 challenge 票据
+
+**影响**
+- “密码登录后第二步验证”被降级成“知道用户 ID 且拿到有效 TOTP 即可直接登录”
+- 这不是旧 P0-07 的原样复现，但本质上仍然属于 MFA 闭环未正确实现
+
+**结论**
+- 旧报告 P0-07 不能标记为“已完全修复”，应迁移为“修复方向已变化，但认证闭环仍未完成”
+
+### P0-02: 设备接口存在成组 `IDOR`
+
+**位置**
+- `internal/api/handler/device_handler.go:114`
+- `internal/api/handler/device_handler.go:147`
+- `internal/api/handler/device_handler.go:183`
+- `internal/api/handler/device_handler.go:214`
+- `internal/api/handler/device_handler.go:392`
+- `internal/api/handler/device_handler.go:474`
+- `internal/service/device.go:121`
+- `internal/service/device.go:158`
+- `internal/service/device.go:163`
+- `internal/service/device.go:181`
+- `internal/service/device.go:204`
+- `internal/service/device.go:236`
+
+**问题**
+- `GET/PUT/DELETE /devices/:id`
+- `PUT /devices/:id/status`
+- `POST/DELETE /devices/:id/trust`
+
+这些接口的 handler 没有 owner/admin 校验，service 层也没有按 `user_id` 兜底约束，只按设备主键直接读写删除。
+
+**影响**
+- 任意已登录用户只要知道设备 ID，就可以读取、修改、删除、信任或取消信任他人设备
+
+**结论**
+- 这是本轮新增发现，严重程度等同发布阻塞
+
+### P0-03: 修改密码接口缺少“本人或管理员”授权校验
+
+**位置**
+- `internal/api/handler/user_handler.go:275`
+- `internal/service/user_service.go:84`
+
+**问题**
+- `PUT /api/v1/users/:id/password` 直接使用路径里的 `id`
+- handler 没有 self-or-admin 校验
+- service 只验证目标用户旧密码是否正确
+
+**影响**
+- 普通用户在知道目标用户旧密码时可直接修改目标用户密码
+- 管理员也没有单独的安全重置路径，权限模型与接口语义混杂
+
+**结论**
+- 这是一条真实的授权缺口，应纳入 P0
+
+### P0-04: 上下文协议漂移导致多处管理员路径失效
+
+**位置**
+- `internal/api/middleware/auth.go:90`
+- `internal/api/middleware/auth.go:91`
+- `internal/api/handler/user_handler.go:191`
+- `internal/api/handler/user_handler.go:374`
+- `internal/api/handler/avatar_handler.go:74`
+
+**问题**
+- 认证中间件当前只写入 `role_codes` / `permission_codes`
+- 多个 handler 仍读取旧的 `user_roles`
+
+**影响**
+- 管理员跨用户更新资料
+- 管理员查看他人角色
+- 管理员代传头像
+
+这些路径都会被错误判定为无权限。
+
+**结论**
+- 旧 P0-06 已做过一轮修复，但当前实现没有真正闭环，应以“部分修复后回归失效”迁移进新报告
+
+### P0-05: OAuth handler 仍返回“200 假成功”占位响应
+
+**位置**
+- `internal/api/handler/auth_handler.go:316`
+- `internal/api/handler/auth_handler.go:329`
+- `internal/api/handler/auth_handler.go:342`
+- `internal/api/handler/auth_handler.go:353`
+- `internal/service/auth.go:939`
+- `internal/service/auth.go:946`
+- `internal/service/auth.go:1492`
+
+**问题**
+- handler 仍直接返回 `OAuth not configured` 或空 provider 列表
+- service 层实际上已经存在 `OAuthLogin` / `OAuthCallback` / `GetEnabledOAuthProviders` 逻辑
+
+**影响**
+- API 层向前端暴露假成功语义
+- 与仓库“禁止 fake success / fail closed”的运行时规则冲突
+
+**结论**
+- 这不是旧报告中的原编号问题，但属于当前实现真实性问题，应纳入高优先级修复
+
+### P0-06: 游标分页与动态排序的契约仍未真正闭环
+
+**位置**
+- `internal/repository/user.go:353`
+
+**问题**
+- 当前实现只在 `sortBy == created_at` 时应用游标条件
+- 其他排序字段下并不会报错，只是静默忽略游标条件
+
+**影响**
+- 前端如果带着非 `created_at` 排序继续请求下一页，得到的不是严格意义上的“下一页”
+- 旧报告的“数据错乱”主因已经被收敛，但 API 契约仍然是不闭合的，容易出现重复页或错误分页预期
+
+**结论**
+- 旧 P0-08 不应从报告中移除，应以下降风险后的“残留契约缺口”形式迁移
+
+---
+
+## 从旧报告迁移的未闭环问题
+
+下表只迁移“当前仍未真正闭环”的旧问题；已经明确修复完成的问题不再重复记为未完成。
+
+| 旧编号 | 当前状态 | 新报告结论 |
+|---|---|---|
+| P0-06 UpdateUser IDOR | 部分修复后再次失效 | 迁移为 P0-04，上下文协议漂移导致管理员授权逻辑失效 |
+| P0-07 Login 绕过 TOTP | 修复方向变化，但未闭环 | 迁移为 P0-01，`totp-verify` 未绑定首因子 |
+| P0-08 ListCursor / sort | 风险下降但契约未闭合 | 迁移为 P0-06，非 `created_at` 排序下游标被静默忽略 |
+| P1-12 ~ P1-14 响应格式不一致 | 仍未修复 | 保留为 P1，`auth_handler`、`password_reset_handler` 等多处仍返回非统一响应格式 |
+| P2-12 `/uploads` 直接暴露 | 仍未修复 | 保留为 P2，`router.Setup()` 仍静态暴露上传目录 |
+
+---
+
+## 已确认修复完成的旧问题
+
+以下问题在当前代码中已具备明确修复证据，不再迁移为“未修复项”：
+
+| 旧编号 | 当前状态 | 证据 |
+|---|---|---|
+| P0-01 LIKE 通配/模式注入 | 已修复 | `internal/repository/operation_log.go`、`internal/repository/device.go`、`internal/repository/user.go` 已统一使用 `escapeLikePattern()` |
+| P0-02 登录失败计数竞态 | 主路径已修复 | `internal/service/auth.go:492` 已改用 `cache.Increment()`；但降级 fallback 仍保留非原子路径，见“残留风险” |
+| P0-03 refresh 黑名单 fail-open | 已修复 | `internal/service/auth.go` 中黑名单写入失败已向上返回错误 |
+| P0-04 手机重置 replay | 基本修复 | `internal/service/password_reset.go` 在验证码校验通过后先删除 key 再继续流程 |
+| P0-05 CORS 默认危险组合 | 已修复 | `internal/api/middleware/cors.go` 默认值已改为空 origins + `AllowCredentials=false` |
+| P1-01 错误处理中间件泄露内部错误 | 已修复 | `internal/api/middleware/error.go` 对未知错误返回通用消息 |
+| P1-03 导出接口泄露内部错误 | 已修复 | `internal/api/handler/export_handler.go` 已改为通用错误文案 |
+| P1-04 CountByResultSince 静默忽略错误 | 已修复 | `internal/repository/login_log.go` 已返回 `(int64, error)` |
+| P1-07 Theme SetDefault 非原子 | 已修复 | `internal/repository/theme.go` 已改用事务 |
+| P1-08 数据库连接池硬编码 | 已修复 | `internal/database/db.go` 已使用配置参数 |
+| P1-15 分页参数无上限 | 大体修复 | `user_handler.go`、`device_handler.go`、`log_handler.go` 均已限制 `page_size <= 100` |
+
+---
+
+## 仍需保留的中高优先级问题
+
+### P1-01: API 响应格式仍然不统一
+
+**位置**
+- `internal/api/handler/auth_handler.go`
+- `internal/api/handler/password_reset_handler.go`
+- `internal/api/handler/user_handler.go`
+
+**问题**
+- 同一套 API 中同时存在 `{error: ...}`、`{message: ...}`、`{code,message,data}` 等多种响应结构
+- `Logout`、`CSRF`、认证错误分支、参数绑定错误分支的格式仍不一致
+
+**影响**
+- 前端错误处理成本高
+- 自动化契约测试难写
+- 文档与真实行为容易继续漂移
+
+### P1-02: 登录失败计数器仍保留非原子降级路径
+
+**位置**
+- `internal/service/auth.go:492`
+
+**问题**
+- 主路径已使用 `cache.Increment()`
+- 但 `Increment` 出错时仍回退到 `Get + current++ + Set`
+
+**影响**
+- 在缓存不支持原子递增或运行时出错场景下，旧竞态仍可能重现
+
+**结论**
+- 不再按 P0 处理，但仍是必须收尾的 P1
+
+### P1-03: CLI/初始化路径存在权限与类型转换告警
+
+**系统化工具证据**
+- `cmd/ums/cmd/init.go:306` `gosec G115`
+- `cmd/ums/cmd/init.go:341` `gosec G301`
+- `cmd/ums/cmd/init.go:446` `gosec G306`
+
+**人工判断**
+- `int(os.Stdin.Fd())` 在 Windows 常见运行路径下不一定形成真实高危，但应改成更明确的受控转换
+- 初始化命令写目录/文件权限偏宽，适合作为 P1/P2 收敛项
+
+### P2-01: 上传目录仍被直接公开暴露
+
+**位置**
+- `internal/api/router/router.go`
+
+**问题**
+- `r.engine.Static("/uploads", "./uploads")` 仍直接公开暴露上传目录
+
+**影响**
+- 上传内容默认可被匿名访问
+- 一旦上传内容策略控制不足，容易扩大文件暴露面
+
+---
+
+## 系统化工具补充审查
+
+### `staticcheck ./...` 结果摘要
+
+人工过滤后，当前值得保留的信号主要有三类：
+
+1. **测试代码错误用法**
+- `internal/api/handler/captcha_handler_test.go`
+- `internal/service/auth_capabilities_test.go`
+
+存在 `SA1012`，测试里向需要 `context.Context` 的调用传了 `nil`。
+
+2. **测试代码潜在空指针**
+- `internal/service/sms_provider_test.go`
+- `internal/service/user_roles_test.go`
+- `internal/service/webhook_service_test.go`
+
+存在 `SA5011`，说明部分测试断言路径缺少空值保护。
+
+3. **仓库内死代码/遗留辅助代码**
+- `internal/api/middleware/auth.go`
+- `internal/monitoring/slo.go`
+- `internal/repository/sql_scan.go`
+- `internal/repository/pagination.go`
+
+存在 `U1000`，说明最近几轮修复后有未清理的遗留函数或字段。
+
+### `gosec ./internal/... ./cmd/...` 结果摘要
+
+`gosec` 本轮输出噪音较大，尤其把 OAuth URL、常量名、header 名、token URL 大量误判为“硬编码凭证”。人工过滤后，建议保留的结果如下：
+
+1. **真实可收敛问题**
+- `internal/api/handler/avatar_handler.go:147` `G301`
+- `internal/api/handler/avatar_handler.go:159` `G306`
+- `internal/service/password_reset.go:237`
+- `internal/service/password_reset.go:252`
+
+前者是目录/文件权限偏宽，后者是关键删除操作忽略返回错误。
+
+2. **低风险但建议修整**
+- `internal/service/captcha.go:164` `G404`
+
+这里使用 `math/rand` 仅用于验证码图片背景色随机化，不直接影响验证码秘密值，但可以考虑改为更明确的非安全随机用途注释，或避免被安全扫描反复报警。
+
+3. **高噪音误报，不建议直接据此立项**
+- OAuth token URL / auth URL
+- Header 名称
+- 非凭证字符串常量
+
+这些不应直接写进缺陷列表，否则会污染修复优先级。
+
+---
+
+## 当前建议修复顺序
+
+### 第一批：立即处理
+
+1. 修复 `totp-verify` 登录闭环，要求必须携带首因子验证后的临时态
+2. 为设备接口补全 owner/admin 校验，并在 service 层增加按 `user_id` 的兜底约束
+3. 为 `/users/:id/password` 增加 self-or-admin 授权，并区分“本人修改密码”和“管理员重置密码”语义
+4. 统一 handler 上下文字段，彻底移除 `user_roles` 旧协议
+5. 去掉 OAuth handler 的假成功返回，改成真实能力分发或显式 fail closed
+
+### 第二批：本周内收口
+
+1. 统一 API 响应结构
+2. 清理登录失败计数器 fallback 竞态
+3. 清理 `staticcheck` 暴露的测试错误与死代码
+4. 收敛 `gosec` 中目录/文件权限与关键错误忽略问题
+
+---
+
+## 对旧报告的处理建议
+
+1. 保留旧报告作为历史记录，不删除
+2. 明确以本报告作为后续复核基线
+3. 旧报告中“2026-04-18 修复完成附录”的“全部问题已修复完成”说法不再可信，后续对外引用时应停止使用该表述
+
+---
+
+## 最终判断
+
+| 维度 | 结论 |
+|---|---|
+| 当前是否全部修复完成 | 否 |
+| 当前是否适合直接上线 | 否 |
+| 是否比 2026-04-17 更接近可上线 | 是，门禁更绿，旧 P0 多数已修，但出现新的授权/认证断层 |
+| 当前最真实的状态 | “旧高危问题大部分已修，当前仍有新的 P0 授权与认证问题待收口，系统化静态审查还暴露出测试与遗留代码清理不足” |
+
--- a/docs/plans/2026-04-21-report-v6-execution-plan.md
+++ b/docs/plans/2026-04-21-report-v6-execution-plan.md
@@ -0,0 +1,89 @@
+# Report v6 Blocking Fixes Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** 修复 `FULL_CODE_REVIEW_REPORT_2026-04-20.md` 中当前阻塞上线的认证、授权和假成功问题，并为每项修复补齐回归验证。
+
+**Architecture:** 以后端授权和认证闭环为主，优先通过测试锁定期望行为，再做最小实现修改。每个批次修复后运行受影响测试集，最后跑完整后端/前端门禁。
+
+**Tech Stack:** Go, Gin, GORM, React, Vitest, PowerShell, Git
+
+---
+
+### Task 1: 锁定 TOTP 二阶段登录闭环
+
+**Files:**
+- Modify: `internal/service/auth.go`
+- Modify: `internal/api/handler/auth_handler.go`
+- Modify: `frontend/admin/src/services/auth.ts`
+- Modify: `frontend/admin/src/types/auth.ts`
+- Test: `internal/service/auth_social_test.go`
+- Test: `internal/api/handler/auth_handler_test.go`
+
+- [ ] **Step 1: 写服务层失败测试**
+- [ ] **Step 2: 运行服务层测试确认当前允许无首因子直接换 token**
+- [ ] **Step 3: 实现临时登录态或 challenge 约束**
+- [ ] **Step 4: 写 handler/前端契约测试**
+- [ ] **Step 5: 运行受影响测试并确认通过**
+
+### Task 2: 修复设备接口 IDOR
+
+**Files:**
+- Modify: `internal/api/handler/device_handler.go`
+- Modify: `internal/service/device.go`
+- Test: `internal/api/handler/device_handler_test.go`
+- Test: `internal/service/device_service_test.go`
+
+- [ ] **Step 1: 写失败测试覆盖跨用户读取/修改/删除/信任设备**
+- [ ] **Step 2: 运行测试确认当前越权成立**
+- [ ] **Step 3: 在 handler 和 service 层补 owner/admin 双层校验**
+- [ ] **Step 4: 运行受影响测试并确认通过**
+
+### Task 3: 修复修改密码接口授权模型
+
+**Files:**
+- Modify: `internal/api/handler/user_handler.go`
+- Modify: `internal/service/user_service.go`
+- Test: `internal/api/handler/user_handler_test.go`
+
+- [ ] **Step 1: 写失败测试覆盖非本人访问 `/users/:id/password`**
+- [ ] **Step 2: 运行测试确认当前缺口存在**
+- [ ] **Step 3: 增加 self-or-admin 校验并明确管理员重置策略**
+- [ ] **Step 4: 运行受影响测试并确认通过**
+
+### Task 4: 清理 `user_roles` 到 `role_codes` 协议漂移
+
+**Files:**
+- Modify: `internal/api/handler/user_handler.go`
+- Modify: `internal/api/handler/avatar_handler.go`
+- Test: `internal/api/handler/user_handler_test.go`
+- Test: `internal/api/handler/avatar_handler_test.go`
+
+- [ ] **Step 1: 写失败测试覆盖管理员跨用户操作被误拒绝**
+- [ ] **Step 2: 运行测试确认当前回归存在**
+- [ ] **Step 3: 统一读取 `role_codes` 或复用 RBAC helper**
+- [ ] **Step 4: 运行受影响测试并确认通过**
+
+### Task 5: 去掉 OAuth 假成功响应
+
+**Files:**
+- Modify: `internal/api/handler/auth_handler.go`
+- Test: `internal/api/handler/auth_handler_test.go`
+
+- [ ] **Step 1: 写失败测试覆盖 OAuth provider 列表与入口行为**
+- [ ] **Step 2: 运行测试确认 handler 当前没有调用 service**
+- [ ] **Step 3: 改成真实 service 分发或显式错误返回**
+- [ ] **Step 4: 运行受影响测试并确认通过**
+
+### Task 6: 全量回归与提交流程
+
+**Files:**
+- Modify: `docs/code-review/FULL_CODE_REVIEW_REPORT_2026-04-20.md`
+- Modify: `docs/status/REAL_PROJECT_STATUS.md`
+
+- [ ] **Step 1: 更新报告中已修复项和剩余风险**
+- [ ] **Step 2: 运行完整后端/前端门禁**
+- [ ] **Step 3: 检查 git diff 与工作区状态**
+- [ ] **Step 4: 按逻辑批次提交**
+- [ ] **Step 5: 推送远程分支**
+
--- a/docs/status/REAL_PROJECT_STATUS.md
+++ b/docs/status/REAL_PROJECT_STATUS.md
@@ -1,5 +1,49 @@
 # REAL PROJECT STATUS

+## 2026-04-23 E2E Recovery Update
+
+### Latest Verification Snapshot
+
+| Command | Result | Note |
+|------|------|------|
+| `cd frontend/admin && npm.cmd run test:run -- src/pages/admin/DevicesPage/DevicesPage.test.tsx` | `PASS` | cursor pagination no longer auto-advances and flood-loads `/admin/devices` |
+| `cd frontend/admin && npm.cmd run test:run -- src/services/webhooks.test.ts` | `PASS` | webhook list and deliveries decoding now matches backend envelopes |
+| `cd frontend/admin && npm.cmd run test:run -- src/pages/admin/WebhooksPage/WebhooksPage.test.tsx` | `PASS` | webhook management page still works after service fix |
+| `cd frontend/admin && npm.cmd run test:run -- src/services/social-accounts.test.ts` | `PASS` | social accounts decoding now matches backend `accounts` payload |
+| `cd frontend/admin && npm.cmd run lint` | `PASS` | frontend lint is green after the recovery changes |
+| `cd frontend/admin && npm.cmd run build` | `PASS` | frontend production build is green after the recovery changes |
+| `cd frontend/admin && npm.cmd run e2e:full:win` | `PASS` | supported browser-level Playwright CDP E2E path re-ran green in the current workspace |
+
+### Current Honest Status
+
+- The supported browser-level real E2E command `cd frontend/admin && npm.cmd run e2e:full:win` is green again in the current workspace.
+- The re-verified scenarios now include:
+  - `admin-bootstrap`
+  - `public-registration`
+  - `email-activation`
+  - `login-surface`
+  - `auth-workflow`
+  - `responsive-login`
+  - `desktop-mobile-navigation`
+  - `user-management-crud`
+  - `role-management-crud`
+  - `device-management`
+  - `login-logs`
+  - `operation-logs`
+  - `webhook-management`
+  - `profile-and-security`
+  - `dashboard-stats`
+- The concrete defects fixed in this round were:
+  - `DevicesPage` cursor state was auto-chaining next-page fetches and could drive `/api/v1/admin/devices` into `429`.
+  - webhook frontend services were decoding `/webhooks` and `/webhooks/:id/deliveries` with the wrong response shape.
+  - social account frontend service was decoding `/users/me/social-accounts` with the wrong response shape.
+  - the Playwright CDP suite had multiple over-broad locators and stale route/title assumptions in the later admin scenarios.
+
+### Boundary
+
+- This update re-proves the supported browser-level E2E path only.
+- It does **not** by itself re-prove full backend `go test ./... -count=1`, real third-party OAuth live verification, or complete OS-level automation closure.
+
 ## 2026-04-10 复核更新（TDD修复后）

 本节记录 2026-04-10 TDD修复后的最新状态。