long-agent/user-system
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
main
user-system/docs/code-review/FULL_REVIEW_2026-05-30.md
Your Name a332917142 fix: harden auth flows and align api contracts
2026-05-30 21:29:24 +08:00

15 KiB
Raw Permalink Blame History

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 通过项

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 失败项

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 中：

"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" 时：

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 中存在：

client_secret: "GOCSPX-abcdef123456"

影响：

  • 容易被误判为真实 secret
  • 不符合敏感信息示例占位规范

结论：文档安全卫生问题。

P2：建议优化的问题

P2-1：strconvAtoi 非法输入返回 (0, nil)，会吞掉参数错误

证据：

internal/api/handler/export_handler.go 中：

if c < '0' || c > '9' {
    return 0, nil
}

这会把非法 status=abc 静默转换成 0。

影响：

  • 参数错误被吞掉
  • 查询语义可能被扭曲

结论：中优先级正确性问题。

P2-2：头像上传一次性读入整个文件，不必要

证据：

internal/api/handler/avatar_handler.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 返回：

"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，再收紧测试与门禁，最后同步更新状态文档与对外表述。

Reference in New Issue View Git Blame Copy Permalink