fix: harden auth flows and align api contracts
This commit is contained in:
Your Name
@@ -33,14 +33,16 @@ cp configs/oauth_config.example.yaml configs/oauth_config.yaml
|
||||
# 示例：微信配置
|
||||
wechat:
|
||||
enabled: true
|
||||
app_id: "wx1234567890abcdef"
|
||||
app_secret: "1234567890abcdef1234567890abcdef"
|
||||
app_id: "<wechat-app-id>"
|
||||
app_secret: "<wechat-app-secret>"
|
||||
|
||||
# 示例：Google配置
|
||||
google:
|
||||
enabled: true
|
||||
client_id: "123456789-abcdef.apps.googleusercontent.com"
|
||||
client_secret: "GOCSPX-abcdef123456"
|
||||
client_id: "<google-client-id>"
|
||||
client_secret: "<google-client-secret>"
|
||||
|
||||
|
||||
```
|
||||
|
||||
### 3. 数据库迁移
|
||||
@@ -290,13 +292,13 @@ Authorization: Bearer <access_token>
|
||||
```bash
|
||||
# 微信
|
||||
WECHAT_OAUTH_ENABLED=true
|
||||
WECHAT_APP_ID=wx1234567890abcdef
|
||||
WECHAT_APP_SECRET=1234567890abcdef1234567890abcdef
|
||||
WECHAT_APP_ID=<wechat-app-id>
|
||||
WECHAT_APP_SECRET=<wechat-app-secret>
|
||||
|
||||
# Google
|
||||
GOOGLE_OAUTH_ENABLED=true
|
||||
GOOGLE_CLIENT_ID=123456789-abcdef.apps.googleusercontent.com
|
||||
GOOGLE_CLIENT_SECRET=GOCSPX-abcdef123456
|
||||
GOOGLE_CLIENT_ID=<google-client-id>
|
||||
GOOGLE_CLIENT_SECRET=<google-client-secret>
|
||||
|
||||
# Facebook
|
||||
FACEBOOK_OAUTH_ENABLED=true
|
||||
|
||||
561
docs/code-review/FULL_REVIEW_2026-05-30.md
Normal file
561
docs/code-review/FULL_REVIEW_2026-05-30.md
Normal file
@@ -0,0 +1,561 @@
|
||||
# user-system 全面 Review 报告
|
||||
|
||||
**审查日期**：2026-05-30
|
||||
**审查范围**：`/home/long/project/user-system`
|
||||
**审查模式**：严格、系统、全面
|
||||
**审查方式**：源码审阅 + 实际构建/测试/静态检查验证 + 第二轮契约一致性对账
|
||||
**结论等级**：**B- / 有条件可运行，不可宣称“已全面收口”**
|
||||
|
||||
---
|
||||
|
||||
## 一、执行摘要
|
||||
|
||||
该项目不是不可用项目。后端、前端、测试主链路均可运行，说明系统已经具备较高完成度；但它距离“高可靠、可审计、严格闭环”的标准仍有明显差距，主要集中在以下五类问题：
|
||||
|
||||
1. **SSO/OAuth 协议正确性存在关键缺口**
|
||||
2. **Swagger / 路由 / 文档之间存在系统性契约漂移**
|
||||
3. **测试数量很多，但契约强度不足，且掩盖了真实路由/鉴权问题**
|
||||
4. **质量门禁对外表述与实际状态不一致**
|
||||
5. **缓存失效、参数校验、上传实现等边界质量仍不够严谨**
|
||||
|
||||
一句话结论：
|
||||
|
||||
> 当前项目可以诚实表述为“主体功能可运行、可测试，但仍存在高价值安全与契约治理缺口”；不能诚实表述为“严格闭环、全面审计通过”。
|
||||
|
||||
---
|
||||
|
||||
## 二、审查范围与方法
|
||||
|
||||
### 2.1 重点审查模块
|
||||
|
||||
- 启动与配置链路
|
||||
- `cmd/server/main.go`
|
||||
- `internal/server/server.go`
|
||||
- `internal/config/config.go`
|
||||
- 认证 / 授权 / 会话
|
||||
- `internal/api/middleware/auth.go`
|
||||
- `internal/service/auth.go`
|
||||
- `internal/service/user_service.go`
|
||||
- `internal/auth/sso.go`
|
||||
- `internal/api/handler/sso_handler.go`
|
||||
- 核心 Handler 与 API 暴露
|
||||
- `internal/api/handler/user_handler.go`
|
||||
- `internal/api/handler/export_handler.go`
|
||||
- `internal/api/handler/avatar_handler.go`
|
||||
- `internal/api/router/router.go`
|
||||
- 仓储层
|
||||
- `internal/repository/user.go`
|
||||
- `internal/repository/operation_log.go`
|
||||
- 前端契约与测试
|
||||
- `frontend/admin/src/services/*`
|
||||
- `frontend/admin/src/pages/admin/ImportExportPage/*`
|
||||
- `internal/api/handler/*_test.go`
|
||||
- `internal/e2e/*`
|
||||
- 文档与 Swagger
|
||||
- `docs/swagger.go`
|
||||
- `docs/docs.go`
|
||||
- `docs/API.md`
|
||||
- `docs/archive/OAUTH_INTEGRATION.md`
|
||||
|
||||
### 2.2 第二轮差异化审查方法
|
||||
|
||||
除第一轮常规源码审阅外，第二轮增加了以下“不同方式”的 review：
|
||||
|
||||
1. **路由注册 vs Swagger 注释逐项对账**
|
||||
- 以 `internal/api/router/router.go` 为真实路由基准
|
||||
- 对照 `internal/api/handler/*.go` 中所有 `@Router` 注释
|
||||
2. **协议路径 vs 鉴权模型对账**
|
||||
- 重点检查 SSO `/authorize`、`/token`、`/introspect`、`/revoke`、`/userinfo`
|
||||
- 核对它们是否被挂在了正确的 middleware / route group 下
|
||||
3. **测试行为 vs 真实路由语义对账**
|
||||
- 检查测试是否在错误的前提下仍“允许通过”
|
||||
4. **文档路径 vs 前端调用路径对账**
|
||||
- 对照 Swagger 注释、路由、前端 service、API 文档的四方一致性
|
||||
|
||||
第二轮发现了**新的系统性问题**，已补充到本报告和修复计划中。
|
||||
|
||||
---
|
||||
|
||||
## 三、实际执行的验证
|
||||
|
||||
以下命令已实际执行。
|
||||
|
||||
### 3.1 通过项
|
||||
|
||||
```bash
|
||||
go test ./... -count=1
|
||||
go build ./cmd/server
|
||||
cd frontend/admin && env -u NODE_ENV npm run test:run
|
||||
cd frontend/admin && env -u NODE_ENV npm run build
|
||||
```
|
||||
|
||||
结果：
|
||||
|
||||
- `go test ./... -count=1`：**通过**
|
||||
- `go build ./cmd/server`：**通过**
|
||||
- 前端 `npm run test:run`：**通过**
|
||||
- `82 files / 525 tests`
|
||||
- 前端 `npm run build`：**通过**
|
||||
|
||||
### 3.2 失败项
|
||||
|
||||
```bash
|
||||
go vet ./...
|
||||
```
|
||||
|
||||
结果：**失败**
|
||||
|
||||
失败位置：
|
||||
|
||||
- `internal/api/handler/avatar_handler_test.go:204`
|
||||
- `internal/api/handler/export_handler_test.go:174`
|
||||
- `internal/api/handler/export_handler_test.go:202`
|
||||
- `internal/api/handler/export_handler_test.go:229`
|
||||
|
||||
失败信息：
|
||||
|
||||
- `using resp before checking for errors`
|
||||
|
||||
这说明当前仓库不能继续对外宣称 `go vet` 已通过。
|
||||
|
||||
---
|
||||
|
||||
## 四、主要发现
|
||||
|
||||
---
|
||||
|
||||
## P0：必须优先修复的问题
|
||||
|
||||
### P0-1：Swagger 文档实际为空壳，当前不能算有效 API 文档
|
||||
|
||||
**证据**：
|
||||
|
||||
`docs/swagger.go` 中：
|
||||
|
||||
```json
|
||||
"paths": {}
|
||||
```
|
||||
|
||||
同时 `internal/api/router/router.go` 公开暴露了：
|
||||
|
||||
- `/swagger/*any`
|
||||
|
||||
**影响**：
|
||||
|
||||
- Swagger UI 可能可访问
|
||||
- 但 API spec 本身没有有效路径
|
||||
- “Swagger 已完成”是错误表述
|
||||
|
||||
**结论**：高优先级治理缺陷。
|
||||
|
||||
---
|
||||
|
||||
### P0-2：Swagger 注释与真实路由存在系统性漂移，不是单点问题
|
||||
|
||||
第一轮只确认了导入导出接口漂移；第二轮确认：**这不是局部问题，而是全局契约漂移**。
|
||||
|
||||
**明确证据示例**：
|
||||
|
||||
1. **导入导出接口**
|
||||
- 注释：`/api/v1/exports/users`、`/api/v1/exports/template`
|
||||
- 实际：`/api/v1/admin/users/export`、`/api/v1/admin/users/import`、`/api/v1/admin/users/import/template`
|
||||
|
||||
2. **刷新令牌接口**
|
||||
- 注释：`/api/v1/auth/refresh-token`
|
||||
- 实际：`/api/v1/auth/refresh`
|
||||
|
||||
3. **邮箱验证码登录接口**
|
||||
- 注释：`/api/v1/auth/login-by-email-code`
|
||||
- 实际：`/api/v1/auth/login/email-code`
|
||||
|
||||
4. **重发激活邮件接口**
|
||||
- 注释：`/api/v1/auth/resend-activation-email`
|
||||
- 实际：`/api/v1/auth/resend-activation`
|
||||
|
||||
5. **TOTP / 2FA 接口**
|
||||
- 注释：`/api/v1/auth/totp/*`
|
||||
- 实际：`/api/v1/auth/2fa/*`
|
||||
- 且 `SetupTOTP` 注释是 `POST`，实际路由是 `GET`
|
||||
|
||||
6. **Captcha 接口**
|
||||
- 注释：`/api/v1/captcha/*`
|
||||
- 实际：`/api/v1/auth/captcha*`
|
||||
|
||||
7. **密码重置接口**
|
||||
- 注释：`/api/v1/auth/password/forgot`、`/reset` 等
|
||||
- 实际：`/api/v1/auth/forgot-password`、`/reset-password`、`/forgot-password/phone`
|
||||
|
||||
8. **自定义字段接口**
|
||||
- 注释：`/api/v1/fields/*`
|
||||
- 实际：`/api/v1/custom-fields/*`
|
||||
|
||||
9. **日志接口**
|
||||
- 注释：`/api/v1/users/me/login-logs`、`/operation-logs`
|
||||
- 实际：`/api/v1/logs/login/me`、`/api/v1/logs/operation/me`
|
||||
|
||||
10. **管理员接口**
|
||||
- 注释：`/api/v1/users/admins`
|
||||
- 实际：`/api/v1/admin/admins`
|
||||
|
||||
11. **方法不一致**
|
||||
- `AssignRoles` 注释为 `POST /api/v1/users/{id}/roles`，实际是 `PUT`
|
||||
- `AssignPermissions` 注释为 `POST /api/v1/roles/{id}/permissions`，实际是 `PUT`
|
||||
|
||||
**影响**：
|
||||
|
||||
- 当前 Swagger 注释整体**不可信**
|
||||
- 不能基于其生成正确 SDK 或自动化客户端
|
||||
- 文档、前端、后端、测试之间存在多套契约
|
||||
- 即使把 Swagger 重新生成，也仍会生成错误契约，除非先修注释
|
||||
|
||||
**结论**：严重契约一致性问题。
|
||||
|
||||
---
|
||||
|
||||
### P0-3：SSO 授权码没有绑定 redirect_uri，token 兑换阶段未校验 redirect_uri / code / client 三元绑定
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/auth/sso.go` 中 `SSOSession` 结构体不包含 `RedirectURI` 字段。
|
||||
|
||||
`GenerateAuthorizationCode(clientID, redirectURI, scope, ...)` 虽接收 `redirectURI`，但没有保存到 session。
|
||||
|
||||
`internal/api/handler/sso_handler.go` 的 `Token` 流程中：
|
||||
|
||||
- 校验了 `grant_type`
|
||||
- 校验了 `client_secret`
|
||||
- 校验了 `code` 是否存在
|
||||
- **未校验** `req.RedirectURI == session.RedirectURI`
|
||||
- **未做严格的 code-client-redirect 三元绑定**
|
||||
|
||||
**影响**：
|
||||
|
||||
- 授权码模式协议实现不完整
|
||||
- 授权码被截获或混用时，服务端缺少关键约束
|
||||
- 不满足高可靠安全要求
|
||||
|
||||
**结论**：严重安全问题。
|
||||
|
||||
---
|
||||
|
||||
### P0-4：SSO implicit flow 仍被支持，并通过 URL fragment 返回 access token
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/handler/sso_handler.go` 中，当 `response_type == "token"` 时：
|
||||
|
||||
```go
|
||||
redirectURL := req.RedirectURI + "#access_token=" + token + "&expires_in=7200"
|
||||
```
|
||||
|
||||
**影响**：
|
||||
|
||||
- access token 暴露给前端地址片段
|
||||
- 不适合高安全系统
|
||||
- 与现代 OAuth 推荐实践不一致
|
||||
|
||||
**结论**：严重安全设计问题。
|
||||
|
||||
---
|
||||
|
||||
### P0-5：SSO `/token`、`/introspect`、`/revoke`、`/userinfo` 被挂在错误的鉴权模型下，协议语义与访问控制同时出错
|
||||
|
||||
这是第二轮新增的关键发现。
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/router/router.go` 中：
|
||||
|
||||
- SSO 整组被挂在：
|
||||
- `protected := v1.Group("")`
|
||||
- `protected.Use(r.authMiddleware.Required())`
|
||||
- 然后：
|
||||
- `sso := protected.Group("/sso")`
|
||||
- `sso.POST("/token", r.ssoHandler.Token)`
|
||||
- `sso.POST("/introspect", r.ssoHandler.Introspect)`
|
||||
- `sso.POST("/revoke", r.ssoHandler.Revoke)`
|
||||
- `sso.GET("/userinfo", r.ssoHandler.UserInfo)`
|
||||
|
||||
而对应 handler 语义是：
|
||||
|
||||
- `Token`：使用 `grant_type + code + client_id + client_secret` 兑换 token，不依赖当前登录用户
|
||||
- `Introspect`：只收 `token` / `client_id`
|
||||
- `Revoke`：只收 `token`
|
||||
- `UserInfo`：当前实现反而直接读 app auth middleware 注入的 `user_id` / `username`
|
||||
|
||||
**影响**：
|
||||
|
||||
1. **OAuth 客户端无法按协议直接兑换授权码**
|
||||
- 因为 `/token` 被错误地要求先通过平台 BearerAuth
|
||||
2. **`/introspect` 与 `/revoke` 不是 client-auth 模型，而是 app-user-auth 模型**
|
||||
- 任意已登录平台用户如果拿到 token 字符串，就可能执行 introspect / revoke
|
||||
3. **`/userinfo` 返回的是平台 JWT 上下文中的用户，而不是 SSO access token 的 subject**
|
||||
- 协议语义错误
|
||||
4. **现有测试已经在掩盖这个问题**
|
||||
- 测试里直接不带认证访问 `/api/v1/sso/token`、`/introspect`、`/revoke`
|
||||
- 但断言允许 200/400/401 多种状态混过
|
||||
|
||||
**结论**：严重的协议与访问控制双重错误，必须优先修复。
|
||||
|
||||
---
|
||||
|
||||
## P1：应尽快修复的问题
|
||||
|
||||
### P1-1：测试大量使用“宽松状态码断言”，无法守住真实接口契约
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/handler/export_handler_test.go`、`internal/api/handler/sso_handler_test.go` 中大量断言允许：
|
||||
|
||||
- 200
|
||||
- 302
|
||||
- 400
|
||||
- 401
|
||||
- 403
|
||||
- 500
|
||||
|
||||
中的多个同时通过。
|
||||
|
||||
**第二轮补充证据**：
|
||||
|
||||
- `sso_handler_test.go` 中多处直接对 `/api/v1/sso/token`、`/introspect`、`/revoke` 发起**无认证请求**
|
||||
- 但测试依旧允许 `401`、`400`、`200` 等多个互斥结果
|
||||
- 这恰好掩盖了 `router.go` 中 SSO route group 被错误挂到 `protected` 下的问题
|
||||
|
||||
**影响**：
|
||||
|
||||
- 测试数量多但行为约束弱
|
||||
- 路由语义漂移、鉴权模型错误时测试仍可能全绿
|
||||
- 会制造“测试全绿”的假象
|
||||
|
||||
**结论**：高优先级测试质量问题。
|
||||
|
||||
---
|
||||
|
||||
### P1-2：`go vet ./...` 实际不通过，项目对外表述与真实状态不一致
|
||||
|
||||
**证据**：
|
||||
|
||||
本次实际执行 `go vet ./...` 失败，失败点见第三节。
|
||||
|
||||
**影响**：
|
||||
|
||||
- README 与状态文档中若继续宣称 `go vet PASS`，属于事实不符
|
||||
- 静态分析未真正成为质量门禁
|
||||
|
||||
**结论**：高优先级工程质量问题。
|
||||
|
||||
---
|
||||
|
||||
### P1-3：JWT secret 治理与项目自我标准不完全一致
|
||||
|
||||
**证据**：
|
||||
|
||||
`cmd/server/main.go` 使用 `config.Load()`，不是 `LoadForBootstrap()`，这点是好的；但 `internal/config/config.go` 中对弱 JWT secret 仅见 `warn` 级处理证据，而未见 release 模式弱值硬失败证据。
|
||||
|
||||
仓库多份 review / 标准文档则明确要求：
|
||||
|
||||
- 生产环境通过环境变量注入 `JWT_SECRET`
|
||||
- 缺失 / 弱值应 fatal
|
||||
|
||||
**影响**：
|
||||
|
||||
- 代码行为与治理标准之间存在差距
|
||||
- 高可靠环境下，弱密钥仅告警不足够
|
||||
|
||||
**结论**：重要安全治理问题。
|
||||
|
||||
---
|
||||
|
||||
### P1-4：用户状态 / 权限缓存失效接口存在，但未见业务路径接入证据
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/middleware/auth.go` 暴露了：
|
||||
|
||||
- `InvalidateUserStateCache(userID)`
|
||||
- `InvalidateUserPermCache(userID)`
|
||||
|
||||
但在 service / handler / server 调用链中未找到这些失效方法的业务接入证据。
|
||||
|
||||
同时缓存 TTL 为：
|
||||
|
||||
- 用户状态：5s
|
||||
- 权限缓存：5min
|
||||
|
||||
**影响**：
|
||||
|
||||
- 密码修改、状态修改、角色修改、权限调整后可能短时继续沿用旧授权结果
|
||||
- 在高敏感场景中不够严格
|
||||
|
||||
**结论**：重要一致性问题。
|
||||
|
||||
---
|
||||
|
||||
### P1-5：归档文档中存在拟真 OAuth secret 示例，文档边界不干净
|
||||
|
||||
**证据**：
|
||||
|
||||
`docs/archive/OAUTH_INTEGRATION.md` 中存在：
|
||||
|
||||
```yaml
|
||||
client_secret: "GOCSPX-abcdef123456"
|
||||
```
|
||||
|
||||
**影响**：
|
||||
|
||||
- 容易被误判为真实 secret
|
||||
- 不符合敏感信息示例占位规范
|
||||
|
||||
**结论**：文档安全卫生问题。
|
||||
|
||||
---
|
||||
|
||||
## P2：建议优化的问题
|
||||
|
||||
### P2-1：`strconvAtoi` 非法输入返回 `(0, nil)`，会吞掉参数错误
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/handler/export_handler.go` 中：
|
||||
|
||||
```go
|
||||
if c < '0' || c > '9' {
|
||||
return 0, nil
|
||||
}
|
||||
```
|
||||
|
||||
这会把非法 `status=abc` 静默转换成 `0`。
|
||||
|
||||
**影响**：
|
||||
|
||||
- 参数错误被吞掉
|
||||
- 查询语义可能被扭曲
|
||||
|
||||
**结论**：中优先级正确性问题。
|
||||
|
||||
---
|
||||
|
||||
### P2-2：头像上传一次性读入整个文件，不必要
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/handler/avatar_handler.go`：
|
||||
|
||||
```go
|
||||
data := make([]byte, file.Size)
|
||||
src.Read(data)
|
||||
os.WriteFile(dstPath, data, 0o644)
|
||||
```
|
||||
|
||||
**影响**：
|
||||
|
||||
- 不必要的整块内存分配
|
||||
- 虽当前 5MB 限制可控，但实现不够稳健
|
||||
|
||||
**结论**：中优先级实现质量问题。
|
||||
|
||||
---
|
||||
|
||||
### P2-3：头像上传成功响应使用匿名 `gin.H`，接口 schema 易漂移
|
||||
|
||||
**证据**：
|
||||
|
||||
`internal/api/handler/avatar_handler.go` 返回：
|
||||
|
||||
```go
|
||||
"data": gin.H{
|
||||
"avatar_url": avatarURL,
|
||||
"thumbnail": avatarURL,
|
||||
}
|
||||
```
|
||||
|
||||
但注释中宣称的是 `AvatarResponse`。
|
||||
|
||||
**影响**：
|
||||
|
||||
- 文档与实现松耦合
|
||||
- 前端类型契约不稳
|
||||
|
||||
**结论**：中优先级可维护性问题。
|
||||
|
||||
---
|
||||
|
||||
## 五、值得保留的正面设计
|
||||
|
||||
### 5.1 头像上传做了扩展名 + Magic Bytes 双校验
|
||||
位置：`internal/api/handler/avatar_handler.go`
|
||||
|
||||
这是正确的防伪装上传设计。
|
||||
|
||||
### 5.2 LIKE 搜索做了特殊字符转义
|
||||
位置：
|
||||
- `internal/repository/user.go`
|
||||
- `internal/repository/operation_log.go`
|
||||
|
||||
说明对模式匹配误用和干扰有明确防御意识。
|
||||
|
||||
### 5.3 权限查询做了合并查询 + 缓存
|
||||
位置：`internal/api/middleware/auth.go`
|
||||
|
||||
方向正确，说明系统已考虑权限查询成本。
|
||||
|
||||
### 5.4 密码修改事务中避免重复 Argon2id 计算
|
||||
位置：`internal/service/user_service.go`
|
||||
|
||||
这体现了不错的成本意识与事务处理意识。
|
||||
|
||||
### 5.5 前端对原生弹窗做了 guard
|
||||
位置：`frontend/admin/src/app/bootstrap/installWindowGuards.ts`
|
||||
|
||||
与仓库“禁止原生 alert/confirm/prompt/open”的规则一致。
|
||||
|
||||
---
|
||||
|
||||
## 六、测试体系评估
|
||||
|
||||
### 6.1 测试“很多”，但不等于“严格”
|
||||
|
||||
当前问题不是缺测试，而是：
|
||||
|
||||
- 测试覆盖面不算窄
|
||||
- 但很多 handler 测试不对行为做强约束
|
||||
- 真实接口契约未被有效锁定
|
||||
|
||||
### 6.2 E2E 有价值，但仍偏“可访问性验证”
|
||||
|
||||
`internal/e2e/e2e_advanced_test.go` 已对真实 admin 导出路由做访问限制验证，这是正面项；但协议严谨性、返回结构一致性、错误语义边界仍缺少强验证。
|
||||
|
||||
### 6.3 第二轮确认：测试还在掩盖路由/鉴权模型错误
|
||||
|
||||
SSO 相关测试已经直接暴露出一个事实：
|
||||
|
||||
- 被测接口在路由层要求平台 BearerAuth
|
||||
- 测试却在无认证前提下继续跑
|
||||
- 断言又接受 200/400/401 多种结果
|
||||
|
||||
这类测试不是“有弹性”，而是**无法担任回归保护**。
|
||||
|
||||
### 6.4 `go vet` 尚未纳入真实闭环
|
||||
|
||||
当前最直接证据就是：`go vet ./...` 失败，而项目文档却可能继续声称通过。
|
||||
|
||||
---
|
||||
|
||||
## 七、最终结论
|
||||
|
||||
该项目：
|
||||
|
||||
- **可以运行**
|
||||
- **可以构建**
|
||||
- **大部分测试可以通过**
|
||||
- **但仍不能宣称“严格闭环、全面收口、可全面审计通过”**
|
||||
|
||||
最关键的阻塞点不是“功能没做完”，而是：
|
||||
|
||||
1. **SSO/OAuth 协议与路由鉴权模型不够严谨**
|
||||
2. **Swagger / 路由 / 文档契约漂移是系统性的，不是局部的**
|
||||
3. **测试绿但不够硬，且会掩盖真实问题**
|
||||
4. **静态检查门禁未真正闭环**
|
||||
|
||||
建议下一步按修复计划先处理 P0，再收紧测试与门禁，最后同步更新状态文档与对外表述。
|
||||
436
docs/code-review/REMEDIATION_PLAN_2026-05-30.md
Normal file
436
docs/code-review/REMEDIATION_PLAN_2026-05-30.md
Normal file
@@ -0,0 +1,436 @@
|
||||
# user-system 修复执行计划（按 P0 / P1 / P2 排序）
|
||||
|
||||
**计划日期**：2026-05-30
|
||||
**输入依据**：`docs/code-review/FULL_REVIEW_2026-05-30.md`
|
||||
**目标**：修复本轮 review 暴露出的安全、正确性、测试与文档一致性问题，并形成新的可审计验证证据。
|
||||
|
||||
---
|
||||
|
||||
## 一、执行原则
|
||||
|
||||
1. **先修协议与契约，再修测试与文档**
|
||||
- 先修 SSO / Swagger / 路由契约错误
|
||||
- 再收敛测试与静态检查
|
||||
2. **每一类问题修完都必须立即验证**
|
||||
3. **文档只能反映已验证事实，不能提前宣称完成**
|
||||
4. **对外可见契约必须单点真实**
|
||||
- 路由
|
||||
- Swagger
|
||||
- 前端调用
|
||||
- 测试断言
|
||||
- 状态文档
|
||||
5. **修复计划必须覆盖 review 报告中的全部问题**
|
||||
- 不能只修“代表性问题”
|
||||
- 必须处理系统性问题源头
|
||||
|
||||
---
|
||||
|
||||
## 二、P0 修复计划（必须最优先）
|
||||
|
||||
### P0-1：把空壳 Swagger 修成真实有效文档
|
||||
|
||||
#### 目标
|
||||
让 `/swagger/*any` 对应的不是空 `paths`，而是真实可用 OpenAPI 文档。
|
||||
|
||||
#### 具体动作
|
||||
1. 梳理 Swagger 生成入口与当前生成流程
|
||||
2. 确认 `swag init` 或项目既定生成方式
|
||||
3. 生成有效 `docs/swagger.go` / `docs/docs.go`
|
||||
4. 校验 `paths` 非空
|
||||
5. 校验至少以下路径存在：
|
||||
- `/api/v1/auth/login`
|
||||
- `/api/v1/auth/register`
|
||||
- `/api/v1/admin/users/export`
|
||||
- `/api/v1/users/{id}`
|
||||
|
||||
#### 验证
|
||||
- 生成 Swagger
|
||||
- 检查 `docs/swagger.go` 中 `paths` 非空
|
||||
- 如可本地启动，验证 `/swagger/index.html` 与 `/swagger/doc.json` 可用
|
||||
|
||||
---
|
||||
|
||||
### P0-2：系统性修正 Swagger 注释与真实路由的漂移
|
||||
|
||||
> 这是对报告中“系统性契约漂移”的完整修复，不再只处理导入导出接口。
|
||||
|
||||
#### 目标
|
||||
统一以下来源的 API 契约：
|
||||
|
||||
- `internal/api/router/router.go`
|
||||
- `internal/api/handler/*.go` 中全部 `@Router`
|
||||
- `docs/API.md`
|
||||
- 前端调用与测试
|
||||
- 生成后的 Swagger 文档
|
||||
|
||||
#### 具体动作
|
||||
1. 全量审计并修复以下类别的 `@Router` 漂移：
|
||||
- export/import：admin 路径
|
||||
- refresh：`/refresh-token` → `/refresh`
|
||||
- email-code login：`/login-by-email-code` → `/login/email-code`
|
||||
- resend activation：`/resend-activation-email` → `/resend-activation`
|
||||
- TOTP：`/auth/totp/*` → `/auth/2fa/*`
|
||||
- captcha：`/captcha/*` → `/auth/captcha*`
|
||||
- password reset：`/auth/password/*` → `/forgot-password` / `/reset-password` / phone 变体
|
||||
- custom fields：`/fields/*` → `/custom-fields/*`
|
||||
- logs：`/users/me/*logs` → `/logs/*/me`
|
||||
- admins：`/users/admins` → `/admin/admins`
|
||||
- users/me 绑定类接口：bind-email / bind-phone / social accounts
|
||||
2. 修复 HTTP method 漂移：
|
||||
- `AssignRoles`：`POST` → `PUT`
|
||||
- `AssignPermissions`：`POST` → `PUT`
|
||||
- `SetupTOTP`：注释 method 与真实 method 对齐
|
||||
3. 对照 `router.go` 做一次全量注释-路由对账，直到关键差异清零
|
||||
4. 更新 `docs/API.md` 中对应路径
|
||||
5. 重新生成 Swagger 文档
|
||||
|
||||
#### 验证
|
||||
- `go test ./internal/api/handler ./internal/api/router -count=1`
|
||||
- 生成 Swagger 后检查关键路径与 method 全部正确
|
||||
- 使用脚本或审查清单确认：关键业务路由不再存在注释/注册漂移
|
||||
|
||||
---
|
||||
|
||||
### P0-3：修复 SSO 授权码模式未绑定 `redirect_uri` 的问题
|
||||
|
||||
#### 目标
|
||||
让 authorization code 与 client / redirect URI 形成强绑定。
|
||||
|
||||
#### 具体动作
|
||||
1. 在 `internal/auth/sso.go` 的 `SSOSession` 中加入 `RedirectURI`
|
||||
2. `GenerateAuthorizationCode(...)` 保存该字段
|
||||
3. `Token(...)` 兑换令牌时校验：
|
||||
- `session.ClientID == req.ClientID`
|
||||
- `session.RedirectURI == req.RedirectURI`
|
||||
4. 对不匹配场景返回明确错误
|
||||
5. 为此补回归测试
|
||||
|
||||
#### 验证
|
||||
- `go test ./internal/auth ./internal/api/handler -count=1`
|
||||
- 增加测试覆盖：
|
||||
- 正确 client + redirect_uri 成功
|
||||
- 错误 redirect_uri 失败
|
||||
- 错误 client_id 失败
|
||||
|
||||
---
|
||||
|
||||
### P0-4：禁用 implicit flow
|
||||
|
||||
#### 目标
|
||||
系统只支持更安全的授权码模式，不再通过 fragment 返回 access token。
|
||||
|
||||
#### 具体动作
|
||||
1. 修改 `internal/api/handler/sso_handler.go`
|
||||
2. 对 `response_type=token`：
|
||||
- 返回 `400 unsupported response_type`
|
||||
- 或仅允许 `code`
|
||||
3. 清理相应的宽松测试
|
||||
4. 同步文档说明只支持 code flow
|
||||
|
||||
#### 验证
|
||||
- `response_type=token` 应明确失败
|
||||
- `response_type=code` 正常工作
|
||||
|
||||
---
|
||||
|
||||
### P0-5：重构 SSO 路由分组与鉴权模型，使 `/token`、`/introspect`、`/revoke`、`/userinfo` 语义正确
|
||||
|
||||
> 这是第二轮新增问题；若不修，P0-3/P0-4 仍不完整。
|
||||
|
||||
#### 目标
|
||||
让 SSO/OAuth 相关端点符合正确的访问控制模型，而不是错误复用平台用户 BearerAuth。
|
||||
|
||||
#### 具体动作
|
||||
1. 将 SSO 路由按语义拆分，不再整体挂在 `protected` 下
|
||||
2. 至少区分：
|
||||
- `/authorize`：需要当前平台登录用户完成授权
|
||||
- `/token`：客户端凭证 + 授权码模型，不依赖当前平台 BearerAuth
|
||||
- `/introspect`：客户端认证模型
|
||||
- `/revoke`：客户端认证模型或 token-owner 受控模型，必须明确
|
||||
- `/userinfo`：基于 SSO access token，而不是平台 JWT 上下文
|
||||
3. 为 `/token`、`/introspect`、`/revoke` 设计明确的 client auth 机制
|
||||
4. 修正 `UserInfo` 的 token 解析来源，不能继续直接读平台 auth middleware 的 `user_id`
|
||||
5. 同步更新测试与文档
|
||||
|
||||
#### 验证
|
||||
- `/token` 在无平台 BearerAuth、仅有正确 client/code 条件下可成功
|
||||
- `/introspect` / `/revoke` 不接受任意平台登录用户代操作
|
||||
- `/userinfo` 返回的是 SSO token subject，而不是平台当前 session user
|
||||
|
||||
---
|
||||
|
||||
## 三、P1 修复计划（紧随 P0）
|
||||
|
||||
### P1-1：修复 `go vet ./...` 失败并收口静态分析门禁
|
||||
|
||||
#### 目标
|
||||
让项目重新具备诚实宣称 `go vet` 通过的资格。
|
||||
|
||||
#### 具体动作
|
||||
1. 修复：
|
||||
- `internal/api/handler/avatar_handler_test.go`
|
||||
- `internal/api/handler/export_handler_test.go`
|
||||
2. 所有 `resp` 使用前先检查 `err`
|
||||
3. 扫描同类 helper/测试模式，避免只修报错行
|
||||
|
||||
#### 验证
|
||||
- `go vet ./...`
|
||||
- `go test ./... -count=1`
|
||||
|
||||
---
|
||||
|
||||
### P1-2：把宽松状态码测试改成严格契约测试
|
||||
|
||||
#### 目标
|
||||
让测试真正约束行为，而不是“什么都算通过”。
|
||||
|
||||
#### 具体动作
|
||||
1. 优先重写以下测试文件：
|
||||
- `internal/api/handler/export_handler_test.go`
|
||||
- `internal/api/handler/sso_handler_test.go`
|
||||
2. 逐场景收紧断言：
|
||||
- 未认证 → 401
|
||||
- 未授权 → 403
|
||||
- 参数错误 → 400
|
||||
- 成功 → 200 / 302
|
||||
3. 删除允许 `500` 的正常断言路径
|
||||
4. 对有环境差异的场景，先修被测逻辑，再收紧测试
|
||||
5. 针对 SSO 补充协议级回归测试：
|
||||
- `/token` 不再被平台 BearerAuth 门禁误拦
|
||||
- `/introspect` / `/revoke` 权限模型正确
|
||||
- `/userinfo` 基于 SSO token，而不是平台 session
|
||||
6. 对关键契约类 handler 增加“路由/方法/状态码固定断言”
|
||||
|
||||
#### 验证
|
||||
- 受影响包 `go test -count=1`
|
||||
- 必须确保断言收紧后仍稳定通过
|
||||
|
||||
---
|
||||
|
||||
### P1-3：强化 JWT secret 治理为启动硬门禁
|
||||
|
||||
#### 目标
|
||||
让 release 模式下的 JWT 配置符合项目自身文档标准。
|
||||
|
||||
#### 具体动作
|
||||
1. 明确 `config.Load()` 下的正常启动规则
|
||||
2. 在 release/standard 服务路径中强制：
|
||||
- secret 缺失 → fail fast
|
||||
- weak secret → fail fast
|
||||
3. 保留 `LoadForBootstrap()` 仅用于初始化场景
|
||||
4. 增加配置单元测试
|
||||
|
||||
#### 验证
|
||||
- `go test ./internal/config -count=1`
|
||||
- 缺失/弱 secret 场景必须失败
|
||||
|
||||
---
|
||||
|
||||
### P1-4：接通用户状态 / 权限变更后的缓存失效链路
|
||||
|
||||
#### 目标
|
||||
避免密码、状态、角色、权限变更后继续使用陈旧缓存。
|
||||
|
||||
#### 具体动作
|
||||
1. 梳理以下写路径：
|
||||
- `ChangePassword`
|
||||
- `UpdateStatus`
|
||||
- `BatchUpdateStatus`
|
||||
- `AssignRoles`
|
||||
- `DeleteAdmin`
|
||||
- `AssignPermissions`
|
||||
2. 设计缓存失效注入方式
|
||||
- 推荐通过依赖注入引入失效能力
|
||||
- 不要让 service 直接依赖具体 middleware 实现细节
|
||||
3. 在写路径完成后主动失效：
|
||||
- user_state
|
||||
- user_perms
|
||||
- 受影响角色下的用户权限缓存
|
||||
|
||||
#### 验证
|
||||
- 增加回归测试：
|
||||
- 改密码后旧 token / 旧状态缓存失效
|
||||
- 改角色/权限后权限即时生效
|
||||
|
||||
---
|
||||
|
||||
### P1-5：清理拟真 secret 示例
|
||||
|
||||
#### 目标
|
||||
恢复文档敏感边界清洁度。
|
||||
|
||||
#### 具体动作
|
||||
1. 清理 `docs/archive/OAUTH_INTEGRATION.md` 中拟真值
|
||||
2. 全仓搜索其它类似格式示例
|
||||
3. 统一替换为显式占位符
|
||||
|
||||
#### 验证
|
||||
- 搜索确认无拟真 secret 示例残留
|
||||
|
||||
---
|
||||
|
||||
## 四、P2 修复计划（在 P0/P1 收口后处理）
|
||||
|
||||
### P2-1：修复 `strconvAtoi` 吞错问题
|
||||
|
||||
#### 目标
|
||||
非法 status 参数返回显式错误，而不是静默当作 0。
|
||||
|
||||
#### 动作
|
||||
1. 修改 `internal/api/handler/export_handler.go` 中 `strconvAtoi`
|
||||
2. 非数字输入返回 error
|
||||
3. `ExportUsers` 中对非法 `status` 返回 400
|
||||
4. 增加回归测试
|
||||
|
||||
#### 验证
|
||||
- `status=abc` → 400
|
||||
|
||||
---
|
||||
|
||||
### P2-2：头像上传改为流式写盘
|
||||
|
||||
#### 目标
|
||||
消除不必要的整块内存分配。
|
||||
|
||||
#### 动作
|
||||
1. 用 `os.Create` + `io.Copy` 代替 `Read + WriteFile`
|
||||
2. 保持现有 magic bytes 校验逻辑
|
||||
3. 确保失败时清理半成品文件
|
||||
|
||||
#### 验证
|
||||
- 头像上传相关测试通过
|
||||
- 文件写入失败场景仍能回滚
|
||||
|
||||
---
|
||||
|
||||
### P2-3：头像上传响应改为明确 struct
|
||||
|
||||
#### 目标
|
||||
让返回 schema 与注释一致。
|
||||
|
||||
#### 动作
|
||||
1. 引入明确响应 struct
|
||||
2. 更新 Swagger 注释 / handler 返回值
|
||||
3. 同步前端类型
|
||||
|
||||
#### 验证
|
||||
- 相关 handler test
|
||||
- 前端编译通过
|
||||
|
||||
---
|
||||
|
||||
### P2-4：前端构建大 chunk 警告优化
|
||||
|
||||
#### 目标
|
||||
降低主包体积，改善生产可维护性。
|
||||
|
||||
#### 动作
|
||||
1. 识别大 chunk 页面
|
||||
2. 做路由级动态拆分
|
||||
3. 必要时拆分 antd 重型页面模块
|
||||
|
||||
#### 验证
|
||||
- `npm run build`
|
||||
- 观察 chunk 体积变化
|
||||
|
||||
---
|
||||
|
||||
## 五、修复计划完整性审核
|
||||
|
||||
本节用于确认：**计划是否覆盖 review 报告中的全部问题**。
|
||||
|
||||
| Review 问题 | 计划覆盖项 | 覆盖状态 |
|
||||
|---|---|---|
|
||||
| Swagger 空壳 | P0-1 | 已覆盖 |
|
||||
| Swagger 注释与真实路由系统性漂移 | P0-2 | 已覆盖 |
|
||||
| SSO code 未绑定 redirect_uri | P0-3 | 已覆盖 |
|
||||
| SSO implicit flow | P0-4 | 已覆盖 |
|
||||
| SSO `/token` `/introspect` `/revoke` `/userinfo` 鉴权模型错误 | P0-5 | 已覆盖 |
|
||||
| 宽松状态码测试掩盖问题 | P1-2 | 已覆盖 |
|
||||
| `go vet` 不通过 | P1-1 | 已覆盖 |
|
||||
| JWT secret 硬门禁不足 | P1-3 | 已覆盖 |
|
||||
| 状态 / 权限缓存失效未接入 | P1-4 | 已覆盖 |
|
||||
| 拟真 secret 示例 | P1-5 | 已覆盖 |
|
||||
| `strconvAtoi` 吞错 | P2-1 | 已覆盖 |
|
||||
| 头像整块读入内存 | P2-2 | 已覆盖 |
|
||||
| 头像响应 schema 漂移 | P2-3 | 已覆盖 |
|
||||
|
||||
### 审核结论
|
||||
|
||||
当前修复计划已经覆盖 review 报告中的**全部问题项**。
|
||||
其中最关键的改进是：
|
||||
|
||||
- 不再把“Swagger 路由错误”视为单点问题，而是按**系统性契约漂移**处理
|
||||
- 新增 P0-5，明确修复 SSO route group / auth model 的结构性错误
|
||||
|
||||
这两点补齐后，计划才具备“能够完整修复 review 报告问题”的条件。
|
||||
|
||||
---
|
||||
|
||||
## 六、推荐执行顺序
|
||||
|
||||
### 阶段 1：协议与契约止血
|
||||
1. P0-5 修 SSO route group / auth model
|
||||
2. P0-3 修 SSO code / redirect_uri 绑定
|
||||
3. P0-4 禁 implicit flow
|
||||
4. P0-2 系统性修正 Swagger 注释与真实路由漂移
|
||||
5. P0-1 生成有效 Swagger
|
||||
|
||||
### 阶段 2：质量门禁与测试收口
|
||||
6. P1-1 修复 `go vet`
|
||||
7. P1-2 收紧 export / sso / 契约类 handler 测试
|
||||
8. P1-3 强化 JWT secret 启动门禁
|
||||
|
||||
### 阶段 3：一致性与边界治理
|
||||
9. P1-4 接通缓存失效链路
|
||||
10. P1-5 清理拟真 secret 示例
|
||||
|
||||
### 阶段 4：实现质量优化
|
||||
11. P2-1 修 status 参数吞错
|
||||
12. P2-2 头像流式写盘
|
||||
13. P2-3 头像响应 struct 化
|
||||
14. P2-4 前端 chunk 优化
|
||||
|
||||
---
|
||||
|
||||
## 七、每阶段完成后的最小验证矩阵
|
||||
|
||||
### P0 阶段后
|
||||
```bash
|
||||
go test ./internal/auth ./internal/api/handler ./internal/api/router -count=1
|
||||
go build ./cmd/server
|
||||
```
|
||||
并检查 Swagger 生成结果。
|
||||
|
||||
### P1 阶段后
|
||||
```bash
|
||||
go vet ./...
|
||||
go test ./... -count=1
|
||||
go build ./cmd/server
|
||||
cd frontend/admin && env -u NODE_ENV npm run test:run
|
||||
cd frontend/admin && env -u NODE_ENV npm run build
|
||||
```
|
||||
|
||||
### P2 阶段后
|
||||
按受影响范围重跑：
|
||||
|
||||
```bash
|
||||
go test ./internal/api/handler ./internal/service ./internal/repository -count=1
|
||||
cd frontend/admin && env -u NODE_ENV npm run build
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 八、完成标准
|
||||
|
||||
只有同时满足以下条件，才能把本轮问题标记为“已收口”：
|
||||
|
||||
1. SSO code flow 绑定完整，implicit flow 已禁用
|
||||
2. SSO `/token`、`/introspect`、`/revoke`、`/userinfo` 的访问控制模型正确
|
||||
3. Swagger 文档非空且关键路径正确
|
||||
4. 注释 / 路由 / 文档 / 前端 / 测试中的 API 契约一致
|
||||
5. `go vet ./...` 通过
|
||||
6. handler 关键测试不再接受互斥状态码混过
|
||||
7. JWT secret 治理与项目文档标准一致
|
||||
8. 缓存失效链路有真实接入与回归测试
|
||||
9. 状态文档与 README 只保留已验证事实
|
||||
8081
docs/docs.go
8081
docs/docs.go
File diff suppressed because it is too large
Load Diff
@@ -1,50 +1 @@
|
||||
// Package docs GENERATED BY SWAG; DO NOT EDIT
|
||||
package docs
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
)
|
||||
|
||||
// SwaggerInfo holds the Swagger information
|
||||
var SwaggerInfo = &swaggerInfo{
|
||||
Version: "1.0",
|
||||
Host: "localhost:8080",
|
||||
BasePath: "/",
|
||||
Schemes: []string{"http", "https"},
|
||||
Title: "User Management System API",
|
||||
Description: "API for user management, authentication, and authorization",
|
||||
}
|
||||
|
||||
type swaggerInfo struct {
|
||||
Version string `json:"version"`
|
||||
Host string `json:"host"`
|
||||
BasePath string `json:"basePath"`
|
||||
Schemes []string `json:"schemes"`
|
||||
Title string `json:"title"`
|
||||
Description string `json:"description"`
|
||||
}
|
||||
|
||||
// SwaggerJSON returns the swagger spec as JSON
|
||||
var SwaggerJSON = `{
|
||||
"swagger": "2.0",
|
||||
"info": {
|
||||
"title": "User Management System API",
|
||||
"description": "API for user management, authentication, and authorization",
|
||||
"version": "1.0"
|
||||
},
|
||||
"host": "localhost:8080",
|
||||
"basePath": "/",
|
||||
"schemes": ["http", "https"],
|
||||
"paths": {}
|
||||
}`
|
||||
|
||||
// GetSwagger returns the swagger specification
|
||||
func GetSwagger() []byte {
|
||||
return []byte(SwaggerJSON)
|
||||
}
|
||||
|
||||
func init() {
|
||||
// Initialize swagger
|
||||
s := GetSwagger()
|
||||
var _ = json.Unmarshal(s, &swaggerInfo{})
|
||||
}
|
||||
|
||||
8065
docs/swagger.json
Normal file
8065
docs/swagger.json
Normal file
File diff suppressed because it is too large
Load Diff
5012
docs/swagger.yaml
Normal file
5012
docs/swagger.yaml
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user