|
@@ -1,232 +1,301 @@
|
|
|
-# 权限系统深度审计报告
|
|
|
|
|
|
|
+# 权限系统深度代码审计报告
|
|
|
|
|
|
|
|
-> 审计时间:2026-04-17
|
|
|
|
|
-> 审计范围:`perms-system-server` 全部业务源码(测试代码除外)
|
|
|
|
|
-> 审计维度:逻辑一致性、并发与竞态、资源管理、数据完整性、安全漏洞、边界崩溃
|
|
|
|
|
|
|
+> 审计范围:`internal/logic`、`internal/model`、`internal/middleware`、`internal/loaders`、`internal/server` 全部非测试业务代码
|
|
|
|
|
+> 审计时间:2026-04-17
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
|
|
-### 1. UserList 接口数据越权泄漏
|
|
|
|
|
|
|
+### H1. UpdateUser 修改状态时未递增 tokenVersion,导致冻结用户可能仍持有有效令牌
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/user/userListLogic.go` 第 45 行
|
|
|
|
|
-- **描述**:非超管用户(产品管理员)调用 UserList 时,虽然校验了 `caller.ProductCode == req.ProductCode`,但底层查询 `SysUserModel.FindListByPage` 执行的是 **全表分页查询**(无任何 WHERE 条件),返回了系统中**所有用户**,而非仅当前产品的成员用户。memberMap 只是在返回结果上附加了 memberType 信息,并不过滤非成员用户。
|
|
|
|
|
-- **影响**:产品管理员可以看到其他产品甚至全系统的用户信息(用户名、昵称、邮箱、手机号、部门等),构成**水平越权数据泄漏**。
|
|
|
|
|
-- **修复方案**:非超管用户查询时,应先查 `sys_product_member` 获取当前产品的成员 userId 列表,再用这些 userId 进行分页查询。示例:
|
|
|
|
|
|
|
+- **位置**:`internal/logic/user/updateUserLogic.go` 第 82-93 行
|
|
|
|
|
+- **描述**:`UpdateUser` 接口允许通过 `req.Status` 将用户冻结(`status=2`),但内部调用的是通用 `SysUserModel.Update()`,该方法**不会递增 `tokenVersion`**。而专用接口 `UpdateUserStatus` 正确调用了 `SysUserModel.UpdateStatus()`,其中执行了 `tokenVersion = tokenVersion + 1`。两个接口实现同一个业务操作(冻结用户),安全保障强度却不同。
|
|
|
|
|
+- **影响**:
|
|
|
|
|
+ 1. 通过 `UpdateUser` 冻结用户后,`tokenVersion` 不变,被冻结用户的 RefreshToken 在 Redis 缓存未命中的情况下(如 Redis 故障、缓存过期后重新加载前的竞态窗口)仍可用于换取新 AccessToken。
|
|
|
|
|
+ 2. 虽然 `UserDetailsLoader.Clean()` 提供了即时保护(清缓存后下次请求会从 DB 加载到 `status=2`),但若 Redis 操作失败(网络抖动等),旧缓存最长可存活 5 分钟(`defaultCacheTTL=300`),期间用户不受冻结影响。
|
|
|
|
|
+ 3. 与 `UpdateUserStatus` 的行为不一致,容易让维护者产生误解。
|
|
|
|
|
+- **修复方案**:在 `UpdateUser` 中,当 `status` 发生变更时,改用 `UpdateStatus` 或手动递增 `tokenVersion`:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// userListLogic.go - 非超管场景
|
|
|
|
|
-if req.ProductCode != "" && !caller.IsSuperAdmin {
|
|
|
|
|
- list, total, err := l.svcCtx.SysUserModel.FindListByProductMembers(
|
|
|
|
|
- l.ctx, req.ProductCode, page, pageSize,
|
|
|
|
|
- )
|
|
|
|
|
- // ...
|
|
|
|
|
|
|
+// updateUserLogic.go — 在 status 变更分支中
|
|
|
|
|
+if req.Status == consts.StatusEnabled || req.Status == consts.StatusDisabled {
|
|
|
|
|
+ if user.Status != req.Status {
|
|
|
|
|
+ // status 发生了实际变更,走 UpdateStatus 以递增 tokenVersion
|
|
|
|
|
+ if err := l.svcCtx.SysUserModel.UpdateStatus(l.ctx, req.Id, req.Status); err != nil {
|
|
|
|
|
+ return err
|
|
|
|
|
+ }
|
|
|
|
|
+ user.Status = req.Status // 同步内存值,后续 Update 不会覆盖回旧值
|
|
|
|
|
+ }
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-需要在 Model 层新增 `FindListByProductMembers` 方法,JOIN `sys_product_member` 表过滤。
|
|
|
|
|
|
|
+或者更彻底的方案:移除 `UpdateUser` 中的 `status` 字段支持,强制状态变更只能通过 `updateUserStatus` 接口。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 2. BindRoles 缺少角色级别校验,存在提权漏洞
|
|
|
|
|
|
|
+### H2. UserDetail 接口返回的 roleIds 未按产品隔离,存在跨产品信息泄露
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/user/bindRolesLogic.go` 第 33-101 行
|
|
|
|
|
-- **描述**:`BindRoles` 仅通过 `CheckManageAccess` 验证操作者对**目标用户当前状态**的管理权限,但未校验所绑定角色的 `permsLevel` 是否在操作者自身权限范围内。攻击路径:
|
|
|
|
|
- 1. 操作者 A(permsLevel=50)管理目标用户 B(当前 permsLevel=100)
|
|
|
|
|
- 2. `CheckManageAccess` 通过(50 < 100,A 的权限高于 B)
|
|
|
|
|
- 3. A 给 B 绑定一个 permsLevel=1 的角色
|
|
|
|
|
- 4. B 的 permsLevel 变为 1,权限反超操作者 A
|
|
|
|
|
-- **影响**:任何拥有用户管理权限的操作者可以通过分配高等级角色来实现**权限提升**,使目标用户获得超过自身的权限级别。
|
|
|
|
|
-- **修复方案**:在校验角色有效性的循环中,增加 permsLevel 校验:
|
|
|
|
|
|
|
+- **位置**:`internal/logic/user/userDetailLogic.go` 第 44 行
|
|
|
|
|
+- **描述**:`FindRoleIdsByUserId` 查询 `sys_user_role` 时没有关联产品过滤,返回了用户在**所有产品**下的全部角色 ID。非超管的产品 A 成员查看某用户详情时,能看到该用户在产品 B 下的角色 ID 列表。
|
|
|
|
|
+- **影响**:虽然角色 ID 本身只是数字,但结合角色列表接口可反推出目标用户在其他产品中的角色配置,构成越权信息泄露。
|
|
|
|
|
+- **修复方案**:在查询时加入产品维度的过滤:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// bindRolesLogic.go - 在遍历 roles 时增加
|
|
|
|
|
-caller := middleware.GetUserDetails(l.ctx)
|
|
|
|
|
-for _, r := range roles {
|
|
|
|
|
- if r.ProductCode != productCode {
|
|
|
|
|
- return response.ErrBadRequest("不能绑定其他产品的角色")
|
|
|
|
|
- }
|
|
|
|
|
- if r.Status != consts.StatusEnabled {
|
|
|
|
|
- return response.ErrBadRequest("不能绑定已禁用的角色")
|
|
|
|
|
- }
|
|
|
|
|
- // 非超管不能分配超出自身权限级别的角色
|
|
|
|
|
- if !caller.IsSuperAdmin && r.PermsLevel < caller.MinPermsLevel {
|
|
|
|
|
- return response.ErrForbidden("不能分配权限级别高于自身的角色")
|
|
|
|
|
- }
|
|
|
|
|
|
|
+productCode := middleware.GetProductCode(l.ctx)
|
|
|
|
|
+var roleIds []int64
|
|
|
|
|
+if productCode != "" {
|
|
|
|
|
+ roleIds, err = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserIdForProduct(l.ctx, user.Id, productCode)
|
|
|
|
|
+} else {
|
|
|
|
|
+ roleIds, err = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserId(l.ctx, user.Id)
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
|
|
+需要在 `SysUserRoleModel` 中新增 `FindRoleIdsByUserIdForProduct` 方法,关联 `sys_role` 表按 `productCode` 过滤。
|
|
|
|
|
+
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 3. appSecret 数据库明文存储
|
|
|
|
|
|
|
+### H3. CreateUser 未校验 DeptId 是否存在,可写入不存在的部门关联
|
|
|
|
|
|
|
|
-- **文件**:`perm.sql` 第 15 行;`internal/logic/pub/syncPermsLogic.go` 第 37 行;`internal/server/permserver.go` 第 40 行
|
|
|
|
|
-- **描述**:`sys_product.appSecret` 以明文存储在数据库中。当前仅用 `subtle.ConstantTimeCompare` 进行比对(防时序攻击)。若数据库被拖库或备份泄漏,所有产品的 appSecret 直接暴露。
|
|
|
|
|
-- **影响**:数据库泄漏场景下,攻击者可以使用任何产品的 appSecret 调用 `SyncPerms` 接口,篡改该产品的全部权限定义,影响所有用户的权限体系。
|
|
|
|
|
-- **修复方案**:将 appSecret 使用 bcrypt 或 HMAC-SHA256 哈希后存储。`CreateProduct` 时仅一次性返回原文,之后只存哈希值。验证时改用 `bcrypt.CompareHashAndPassword`:
|
|
|
|
|
|
|
+- **位置**:`internal/logic/user/createUserLogic.go` 第 69-80 行
|
|
|
|
|
+- **描述**:`CreateUser` 直接将 `req.DeptId` 写入数据库,未校验该部门是否存在。而 `UpdateUser` 在修改 `DeptId` 时正确地进行了存在性校验。
|
|
|
|
|
+- **影响**:创建用户时传入不存在的 `deptId`,用户记录会关联到一个"幽灵部门"。后续 `UserDetailsLoader.loadDept` 会查询失败并静默跳过(`DeptPath` 为空),导致该用户在部门层级校验 `checkDeptHierarchy` 中行为异常——具体表现为 `caller.DeptPath == ""`,触发 `"您的部门信息异常"` 错误,无法管理任何其他用户。
|
|
|
|
|
+- **修复方案**:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// syncPermsLogic.go / permserver.go
|
|
|
|
|
-if err := bcrypt.CompareHashAndPassword(
|
|
|
|
|
- []byte(product.AppSecretHash), []byte(req.AppSecret),
|
|
|
|
|
-); err != nil {
|
|
|
|
|
- return nil, response.ErrUnauthorized("appSecret验证失败")
|
|
|
|
|
|
|
+if req.DeptId > 0 {
|
|
|
|
|
+ if _, err := l.svcCtx.SysDeptModel.FindOne(l.ctx, req.DeptId); err != nil {
|
|
|
|
|
+ return nil, response.ErrBadRequest("部门不存在")
|
|
|
|
|
+ }
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 4. UpdateUser 权限模型与其他接口不一致,产品管理员无法修改自己创建的用户
|
|
|
|
|
|
|
+### H4. 成员类型管理层级与 SQL 设计文档不一致
|
|
|
|
|
+
|
|
|
|
|
+- **位置**:`internal/logic/auth/access.go` `memberTypePriority` 函数 vs `perm.sql` 第 151-155 行注释
|
|
|
|
|
+- **描述**:`perm.sql` 中明确标注管理层级顺序为 `超级管理员 > DEVELOPER > ADMIN > MEMBER`,但代码中 `memberTypePriority` 的实现为:
|
|
|
|
|
+
|
|
|
|
|
+| 类型 | 代码优先级 | SQL 文档预期 |
|
|
|
|
|
+|------|-----------|-------------|
|
|
|
|
|
+| SUPER_ADMIN | 0 (最高) | 最高 |
|
|
|
|
|
+| ADMIN | 1 | 2 |
|
|
|
|
|
+| DEVELOPER | 2 | 1 |
|
|
|
|
|
+| MEMBER | 3 (最低) | 最低 |
|
|
|
|
|
+
|
|
|
|
|
+ 代码中 ADMIN(1) > DEVELOPER(2),而文档描述 DEVELOPER > ADMIN。
|
|
|
|
|
+- **影响**:如果文档是正确的设计意图,那么当前实现存在以下问题:
|
|
|
|
|
+ - DEVELOPER 无法管理同产品下的 ADMIN 用户(被 `checkPermLevel` 拦截)
|
|
|
|
|
+ - ADMIN 可以管理 DEVELOPER 用户(不应被允许)
|
|
|
|
|
+ - `CheckMemberTypeAssignment` 中 DEVELOPER 无法分配 ADMIN 类型(若按文档应该可以)
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/user/updateUserLogic.go` 第 37-45 行
|
|
|
|
|
-- **描述**:`UpdateUser` 的权限判断逻辑为"只有超管或用户自身可修改",而系统中 `UpdateUserStatus`、`BindRoles`、`SetUserPerms` 等接口均使用 `CheckManageAccess`(支持产品管理员和部门层级管理)。这导致产品管理员可以创建用户(`CreateUser`)、冻结用户(`UpdateUserStatus`)、绑定角色(`BindRoles`),却**无法修改**用户的昵称、邮箱、部门等基本信息。
|
|
|
|
|
-- **影响**:产品管理员的管理权限出现逻辑断裂——能创建和冻结用户,但不能编辑用户信息,不得不依赖超管操作,严重影响管理效率。
|
|
|
|
|
-- **修复方案**:将 UpdateUser 的权限模型统一为 `CheckManageAccess`,并保留对自身修改的限制(不能改自己的部门和状态):
|
|
|
|
|
|
|
+ 如果代码是正确的、文档是过时的,则应更新 SQL 注释以避免后续维护者误解。
|
|
|
|
|
+- **修复方案**:确认真实的业务层级意图,统一代码与文档。若 DEVELOPER 应高于 ADMIN:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-func (l *UpdateUserLogic) UpdateUser(req *types.UpdateUserReq) error {
|
|
|
|
|
- caller := middleware.GetUserDetails(l.ctx)
|
|
|
|
|
- if caller == nil {
|
|
|
|
|
- return response.ErrUnauthorized("未登录")
|
|
|
|
|
|
|
+func memberTypePriority(memberType string) int {
|
|
|
|
|
+ switch memberType {
|
|
|
|
|
+ case consts.MemberTypeSuperAdmin:
|
|
|
|
|
+ return 0
|
|
|
|
|
+ case consts.MemberTypeDeveloper:
|
|
|
|
|
+ return 1
|
|
|
|
|
+ case consts.MemberTypeAdmin:
|
|
|
|
|
+ return 2
|
|
|
|
|
+ case consts.MemberTypeMember:
|
|
|
|
|
+ return 3
|
|
|
|
|
+ default:
|
|
|
|
|
+ return math.MaxInt32
|
|
|
}
|
|
}
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
|
|
|
- if caller.UserId == req.Id {
|
|
|
|
|
- if req.DeptId != nil || req.Status != 0 {
|
|
|
|
|
- return response.ErrForbidden("不允许修改自己的部门和状态")
|
|
|
|
|
- }
|
|
|
|
|
- } else {
|
|
|
|
|
- productCode := middleware.GetProductCode(l.ctx)
|
|
|
|
|
- if err := authHelper.CheckManageAccess(l.ctx, l.svcCtx, req.Id, productCode); err != nil {
|
|
|
|
|
- return err
|
|
|
|
|
- }
|
|
|
|
|
- }
|
|
|
|
|
- // ... 后续逻辑不变
|
|
|
|
|
|
|
+若代码实现是正确的(ADMIN > DEVELOPER),则更新 `perm.sql` 注释为:`超级管理员 > ADMIN > DEVELOPER > MEMBER`。
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### H5. SyncPerms 传入空权限列表会禁用产品全部权限,缺乏防护
|
|
|
|
|
+
|
|
|
|
|
+- **位置**:`internal/logic/pub/syncPermsLogic.go` 及 `internal/model/perm/sysPermModel.go` `DisableNotInCodesWithTx`
|
|
|
|
|
+- **描述**:当 `SyncPerms` 请求中 `perms` 数组为空时,`codes` 也为空,`DisableNotInCodesWithTx` 会执行:
|
|
|
|
|
+ ```sql
|
|
|
|
|
+ UPDATE sys_perm SET status=2 WHERE productCode=? AND status=1
|
|
|
|
|
+ ```
|
|
|
|
|
+ 一次性禁用该产品下**所有启用的权限**。
|
|
|
|
|
+- **影响**:客户端代码 bug(如序列化异常导致 perms 为空数组)、网络问题(请求被截断)都可能触发全量权限禁用,影响该产品下所有用户的访问。这是一个潜在的可用性灾难。
|
|
|
|
|
+- **修复方案**:在 `SyncPerms` 入口处增加空数组保护:
|
|
|
|
|
+
|
|
|
|
|
+```go
|
|
|
|
|
+if len(req.Perms) == 0 {
|
|
|
|
|
+ return nil, response.ErrBadRequest("权限列表不能为空,如需禁用所有权限请使用专用接口")
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
|
|
+gRPC 端的 `SyncPermissions` 也需要同步添加此校验。
|
|
|
|
|
+
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## ⚠️ 健壮性与性能建议 (Medium/Low)
|
|
## ⚠️ 健壮性与性能建议 (Medium/Low)
|
|
|
|
|
|
|
|
-### 5. [Medium] CreateUser 未将用户加入产品成员,工作流存在断裂
|
|
|
|
|
|
|
+### M1. RefreshToken 未实现轮转,被盗令牌在有效期内可无限复用
|
|
|
|
|
+
|
|
|
|
|
+- **位置**:`internal/logic/pub/refreshTokenLogic.go` 第 75 行
|
|
|
|
|
+- **级别**:Medium
|
|
|
|
|
+- **描述**:`RefreshToken` 接口刷新后直接原样返回旧的 `refreshToken`(`RefreshToken: tokenStr`)。这意味着 refresh token 在整个有效期内是静态不变的,一旦泄露,攻击者可以持续用它换取新的 access token,直到 refresh token 过期或用户主动修改密码(触发 tokenVersion 递增)。
|
|
|
|
|
+- **建议**:实现 Refresh Token Rotation:每次刷新时签发新的 refresh token 并使旧的失效(可通过在 Redis 中维护一个 token 黑名单或版本号实现)。
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### M2. HTTP 与 gRPC 的 SyncPermissions 逻辑重复,存在不同步风险
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/user/createUserLogic.go`
|
|
|
|
|
-- **描述**:`CreateUser` 要求 `RequireProductAdminFor(productCode)` 校验产品管理员身份,但创建用户后并未将其加入当前产品的成员列表。新建用户需要管理员再单独调用 `AddMember` 才能登录和使用产品。
|
|
|
|
|
-- **影响**:管理员可能误以为创建用户即完成了入组操作,导致新用户无法登录。增加了操作步骤和出错概率。
|
|
|
|
|
-- **建议**:考虑在 CreateUser 中增加可选参数 `memberType`,当传入时在同一事务中自动创建产品成员记录;或在文档/前端层面明确引导管理员完成两步操作。
|
|
|
|
|
|
|
+- **位置**:`internal/logic/pub/syncPermsLogic.go` vs `internal/server/permserver.go` `SyncPermissions` 方法
|
|
|
|
|
+- **级别**:Medium
|
|
|
|
|
+- **描述**:HTTP 端走 `SyncPermsLogic`,gRPC 端在 `permserver.go` 中内联实现了几乎相同的逻辑。两处代码维护相同的业务语义(认证、去重、事务批量更新、缓存清理),但互相独立。
|
|
|
|
|
+- **建议**:将核心逻辑抽取为共享的 service 函数(类似 `ValidateProductLogin` 的做法),让 HTTP Logic 和 gRPC Server 都调用同一份代码。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 6. [Medium] 缓存前缀变量是包级可变全局状态
|
|
|
|
|
|
|
+### M3. RequireProductAdmin 函数未绑定具体产品,存在跨产品越权隐患
|
|
|
|
|
+
|
|
|
|
|
+- **位置**:`internal/logic/auth/access.go` `RequireProductAdmin` 函数
|
|
|
|
|
+- **级别**:Medium
|
|
|
|
|
+- **描述**:`RequireProductAdmin` 只检查 `caller.MemberType == ADMIN`,不验证操作者是否是**目标产品**的管理员。如果产品 A 的 ADMIN 调用了使用此函数鉴权的接口来操作产品 B 的数据,会被错误放行。
|
|
|
|
|
+- **现状**:当前所有业务代码使用的是带产品校验的 `RequireProductAdminFor`,`RequireProductAdmin` 实际未被引用。
|
|
|
|
|
+- **建议**:删除 `RequireProductAdmin` 函数或标记为 deprecated,避免后续开发者误用。
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
|
|
|
-- **文件**:各 `*Model_gen.go`(如 `sysUserModel_gen.go` 第 26-27 行,`sysPermModel_gen.go` 第 26-27 行)
|
|
|
|
|
-- **描述**:`cacheSysUserIdPrefix`、`cacheSysPermIdPrefix` 等缓存前缀在 `newSys*Model` 函数中被直接修改(`cacheSysUserIdPrefix = cachePrefix + ":cache:sysUser:id:"`)。这些是 `var` 级别的全局变量。
|
|
|
|
|
-- **影响**:若代码中有多处以不同 `cachePrefix` 创建同类 Model 实例(当前不存在此情况),会产生竞态条件。虽然当前启动时仅初始化一次,但这种模式在代码演进中存在隐患。
|
|
|
|
|
-- **建议**:由于此代码由 goctl 生成,短期内可接受。长期建议修改 goctl 模板,将前缀存入 struct 字段而非修改包级变量。
|
|
|
|
|
|
|
+### M4. CreateProduct 事务中管理员用户名可能冲突导致整体回滚
|
|
|
|
|
+
|
|
|
|
|
+- **位置**:`internal/logic/product/createProductLogic.go` 第 87 行
|
|
|
|
|
+- **级别**:Medium
|
|
|
|
|
+- **描述**:创建产品时自动生成管理员用户名为 `admin_{productCode}`。如果系统中已存在同名用户(如手动创建或之前产品删除后遗留),`InsertWithTx` 会因唯一索引冲突报错,导致整个事务回滚——产品也不会被创建,且错误信息为底层数据库错误,对调用方不友好。
|
|
|
|
|
+- **建议**:在事务开始前先检查用户名是否已存在,给出明确的业务错误提示:
|
|
|
|
|
+
|
|
|
|
|
+```go
|
|
|
|
|
+if _, err := l.svcCtx.SysUserModel.FindOneByUsername(l.ctx, adminUsername); err == nil {
|
|
|
|
|
+ return nil, response.ErrConflict(fmt.Sprintf("用户名 %s 已存在,无法自动创建管理员账号", adminUsername))
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 7. [Medium] AddMember 并发重复请求错误信息不友好
|
|
|
|
|
|
|
+### M5. BindRolePerms / SetUserPerms 未校验权限的启用状态
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/member/addMemberLogic.go` 第 52-55 行
|
|
|
|
|
-- **描述**:使用 check-then-insert 模式检查成员是否已存在。若两个请求并发执行,都通过了检查,其中一个在 Insert 时会触发数据库唯一约束错误(`uk_product_user`),但此错误未被捕获转换为友好提示,而是返回原始的 500 错误。
|
|
|
|
|
-- **影响**:并发场景下用户收到 "服务器内部错误" 而非 "该用户已是该产品成员"。
|
|
|
|
|
-- **建议**:在 Insert 错误处理中捕获唯一约束冲突:
|
|
|
|
|
|
|
+- **位置**:`internal/logic/role/bindRolePermsLogic.go` 第 58-64 行、`internal/logic/user/setUserPermsLogic.go` 第 65-74 行
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:绑定权限时只校验了权限 ID 存在且属于同一产品,但未检查权限是否处于启用状态(`status=1`)。已被 `SyncPerms` 禁用的权限仍可被绑定到角色或用户。
|
|
|
|
|
+- **实际影响有限**:`loadPerms` 在计算最终权限时会过滤掉 `status != 1` 的权限,所以被禁用的权限不会生效。但绑定关系的存在可能造成管理界面上的困惑。
|
|
|
|
|
+- **建议**:在校验循环中增加状态检查:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-result, err := l.svcCtx.SysProductMemberModel.Insert(l.ctx, &productmember.SysProductMember{...})
|
|
|
|
|
-if err != nil {
|
|
|
|
|
- if strings.Contains(err.Error(), "1062") || strings.Contains(err.Error(), "Duplicate entry") {
|
|
|
|
|
- return nil, response.ErrConflict("该用户已是该产品成员")
|
|
|
|
|
|
|
+for _, p := range perms {
|
|
|
|
|
+ if p.ProductCode != role.ProductCode {
|
|
|
|
|
+ return response.ErrBadRequest("不能绑定其他产品的权限")
|
|
|
|
|
+ }
|
|
|
|
|
+ if p.Status != consts.StatusEnabled {
|
|
|
|
|
+ return response.ErrBadRequest(fmt.Sprintf("权限 %s 已被禁用,无法绑定", p.Code))
|
|
|
}
|
|
}
|
|
|
- return nil, err
|
|
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 8. [Medium] JWT 中嵌入了完整权限列表,Token 体积随权限数增长
|
|
|
|
|
|
|
+### M6. ProductDetail / ProductList 缺少产品维度的访问控制
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/auth/jwt.go` 第 23-40 行;`internal/middleware/jwtauthMiddleware.go` Claims 结构
|
|
|
|
|
-- **描述**:`Claims.Perms` 字段将用户的所有权限 code 列表嵌入 access token。对于拥有数百个权限的用户,Token 体积会显著增长(可能超过 4KB),导致每次 HTTP 请求的 Header 过大。
|
|
|
|
|
-- **影响**:增加网络传输开销;某些反向代理(如 Nginx 默认 4KB/8KB header buffer)可能拒绝过大的请求头。
|
|
|
|
|
-- **建议**:Token 中仅保留 userId、productCode 等核心标识,权限列表通过 `UserDetailsLoader`(已有 Redis 缓存)在需要时加载。当前中间件已经通过 `loader.Load` 重新加载了完整用户信息,Token 中的 Perms 字段实际上未被中间件使用,仅用于前端展示。可以考虑在登录响应中单独返回 perms,从 Token 中移除。
|
|
|
|
|
|
|
+- **位置**:`internal/logic/product/productDetailLogic.go`、`productListLogic.go`
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:任何已登录用户(包括非超管的普通产品成员)都可以查看系统中**所有产品**的列表和详情(除 AppKey 外)。虽然敏感字段 `AppKey` 仅对超管可见、`AppSecret` 不返回,但产品编码、名称等信息对非本产品成员可见。
|
|
|
|
|
+- **建议**:评估业务需求。如果产品信息确实应该对所有登录用户可见(如产品选择页面),则当前实现合理。否则应增加 `caller.ProductCode` 校验或只返回用户所属产品的列表。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 9. [Medium] RefreshToken 未限制刷新次数,旧 RefreshToken 可无限复用
|
|
|
|
|
|
|
+### M7. DeptTree 接口无任何权限过滤
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/pub/refreshTokenLogic.go`;`internal/server/permserver.go` 第 162-200 行
|
|
|
|
|
-- **描述**:`RefreshToken` 接口在签发新的 AccessToken 后,直接将原 RefreshToken 原样返回给客户端。RefreshToken 在有效期内可以被无限次调用,每次都会生成新的 AccessToken。
|
|
|
|
|
-- **影响**:若 RefreshToken 泄漏,攻击者可在整个有效期内持续获取新的 AccessToken。通常的最佳实践是 Refresh Token Rotation——每次刷新时同时签发新的 RefreshToken 并使旧的失效。
|
|
|
|
|
-- **建议**:当前通过 `tokenVersion` 机制可以在修改密码/冻结用户时使所有 token 失效,这提供了基本保障。若需更严格的安全性,可实现 Refresh Token Rotation(每次刷新签发新 RefreshToken),或在 Redis 中维护一个已使用 RefreshToken 的黑名单。
|
|
|
|
|
|
|
+- **位置**:`internal/logic/dept/deptTreeLogic.go`
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:`DeptTree` 加载并返回系统中**全部部门**的树形结构,无论调用者的身份和所属产品。任何已登录用户都能看到完整的组织架构。
|
|
|
|
|
+- **建议**:如果部门树只应由超管可见,增加 `RequireSuperAdmin` 校验。如果需要按层级裁剪(只看本部门及子部门),应根据 `caller.DeptPath` 过滤。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 10. [Low] 用户不存在时 JwtAuth 中间件返回误导性错误信息
|
|
|
|
|
|
|
+### M8. loadRoles 全量加载后内存过滤,存在轻微效率损失
|
|
|
|
|
|
|
|
-- **文件**:`internal/middleware/jwtauthMiddleware.go` 第 74-78 行;`internal/loaders/userDetailsLoader.go` 第 129-133 行
|
|
|
|
|
-- **描述**:当 JWT 有效但用户已被删除时,`UserDetailsLoader.Load` 返回一个默认的 `UserDetails`(Status=0)。中间件判断 `ud.Status != StatusEnabled` 后返回"账号已被冻结"。但实际上用户是不存在/已被删除。
|
|
|
|
|
-- **影响**:用户收到错误的提示信息,可能导致困惑或误导排查方向。
|
|
|
|
|
-- **建议**:在中间件中增加用户是否存在的判断:
|
|
|
|
|
|
|
+- **位置**:`internal/loaders/userDetailsLoader.go` `loadRoles` 方法
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:`FindRoleIdsByUserId` 返回用户在所有产品下的全部角色 ID,再通过 `FindByIds` 批量查询角色详情,最后在内存中按 `ProductCode` 和 `Status` 过滤。对于只加入了 1-2 个产品的普通用户不会有问题,但逻辑上可以在 SQL 层面就做好过滤。
|
|
|
|
|
+- **建议**:新增按产品过滤的查询方法:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-ud := m.loader.Load(r.Context(), claims.UserId, claims.ProductCode)
|
|
|
|
|
-if ud.Username == "" {
|
|
|
|
|
- httpx.ErrorCtx(r.Context(), w, response.NewCodeError(401, "用户不存在或已被删除"))
|
|
|
|
|
- return
|
|
|
|
|
-}
|
|
|
|
|
-if ud.Status != consts.StatusEnabled {
|
|
|
|
|
- httpx.ErrorCtx(r.Context(), w, response.NewCodeError(403, "账号已被冻结"))
|
|
|
|
|
- return
|
|
|
|
|
-}
|
|
|
|
|
|
|
+func FindRoleIdsByUserIdForProduct(ctx context.Context, userId int64, productCode string) ([]int64, error)
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
|
|
+通过 JOIN `sys_role` 表在查询时过滤 `productCode`,减少不必要的数据传输和内存开销。
|
|
|
|
|
+
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 11. [Low] 缺少操作审计日志
|
|
|
|
|
|
|
+### M9. DeleteRole 缓存失效存在极小时间窗口遗漏
|
|
|
|
|
|
|
|
-- **描述**:当前系统在所有写操作(创建用户、绑定角色、修改权限、冻结用户、删除角色等)中未记录操作审计日志。仅有 go-zero 框架默认的请求日志。
|
|
|
|
|
-- **影响**:当发生安全事件(如权限被篡改、用户被异常冻结)时,无法追溯是谁在什么时间执行了什么操作。对于权限管理系统,审计追溯能力是合规性的基本要求。
|
|
|
|
|
-- **建议**:设计一个 `sys_audit_log` 表,记录 `operator_id`、`action`、`target_type`、`target_id`、`detail`、`ip`、`timestamp` 等字段。在关键业务逻辑中通过异步方式写入审计记录,避免影响主流程性能。
|
|
|
|
|
|
|
+- **位置**:`internal/logic/role/deleteRoleLogic.go` 第 40-42 行
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:`affectedUserIds` 在事务执行**之前**查询。如果在查询之后、事务执行之前有新的用户被绑定到该角色,这些用户的缓存不会被主动清理(但会在 5 分钟后自然过期)。
|
|
|
|
|
+- **实际影响极低**:这个时间窗口极短(微秒级),且角色删除事务内已清除所有 user-role 绑定关系,新绑定的用户在缓存过期后会自动生效。
|
|
|
|
|
+- **建议**:可接受当前实现。如需极致一致性,可在事务内查询 affectedUserIds。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 12. [Low] DeptTree 和 ProductList 对非超管暴露全量数据
|
|
|
|
|
|
|
+### M10. CreateUser 缺少用户名格式校验
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/dept/deptTreeLogic.go`;`internal/logic/product/productListLogic.go`
|
|
|
|
|
-- **描述**:`DeptTree` 接口无任何权限过滤,所有已登录用户可看到整个组织架构。`ProductList` 也对所有用户返回全部产品列表(AppKey 已对非超管隐藏)。
|
|
|
|
|
-- **影响**:部门结构和产品列表属于组织内部信息,虽然不包含高敏感数据,但在安全要求较高的场景下可能不符合最小暴露原则。
|
|
|
|
|
-- **建议**:根据实际业务需求评估是否需要按产品/部门范围过滤。如果当前业务场景下所有用户确实需要查看组织架构(如选择部门),可保持现状。
|
|
|
|
|
|
|
+- **位置**:`internal/logic/user/createUserLogic.go`
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:`CreateUser` 仅校验了用户名长度(最大 64 字符),未校验格式。用户名可以包含空格、特殊字符、中文等,可能导致:
|
|
|
|
|
+ - 与自动生成的 `admin_{code}` 格式冲突
|
|
|
|
|
+ - 登录时的编码问题
|
|
|
|
|
+ - 日志可读性降低
|
|
|
|
|
+- **建议**:增加用户名格式校验(如只允许字母数字下划线):
|
|
|
|
|
+
|
|
|
|
|
+```go
|
|
|
|
|
+if !regexp.MustCompile(`^[a-zA-Z0-9_]{2,64}$`).MatchString(req.Username) {
|
|
|
|
|
+ return nil, response.ErrBadRequest("用户名只能包含字母、数字和下划线,长度2-64个字符")
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### 13. [Low] UpdateDept 修改 deptType 时缺少影响评估
|
|
|
|
|
|
|
+### M11. gRPC GetUserPerms 无鉴权保护
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/dept/updateDeptLogic.go`
|
|
|
|
|
-- **描述**:将部门类型从 `DEV`(研发)改为 `NORMAL`,该部门下所有用户会立即失去"全权限"特权(因 `loadPerms` 中 DEV 部门自动获取全量权限的逻辑不再生效)。代码正确清除了缓存,但未在响应中提示此操作的影响范围。
|
|
|
|
|
-- **影响**:超管执行部门类型变更后,该部门及子部门下的用户可能突然失去大量权限,如果未提前通知,可能导致业务中断。
|
|
|
|
|
-- **建议**:在响应中返回受影响的用户数量,或在执行前增加确认机制。
|
|
|
|
|
|
|
+- **位置**:`internal/server/permserver.go` `GetUserPerms` 方法
|
|
|
|
|
+- **级别**:Low(取决于部署方式)
|
|
|
|
|
+- **描述**:gRPC 端的 `GetUserPerms` 接口没有任何认证或授权校验,任何能访问 gRPC 端口的客户端都可以查询任意用户的权限列表。
|
|
|
|
|
+- **现状评估**:如果 gRPC 仅在内网(如 K8s 集群内部)暴露给可信的下游服务,这是合理的设计。但如果 gRPC 端口意外暴露到公网,则构成严重的信息泄露。
|
|
|
|
|
+- **建议**:确保 gRPC 端口不暴露到外部网络。如有需要,可增加 gRPC 拦截器进行 mTLS 或 token 校验。
|
|
|
|
|
+
|
|
|
|
|
+---
|
|
|
|
|
+
|
|
|
|
|
+### M12. UpdateMember / UpdateRole / UpdateDept 对无效 status 值静默忽略
|
|
|
|
|
+
|
|
|
|
|
+- **位置**:`updateMemberLogic.go`、`updateRoleLogic.go`、`updateDeptLogic.go` 多处
|
|
|
|
|
+- **级别**:Low
|
|
|
|
|
+- **描述**:这些接口在处理 `status` 字段时采用"白名单匹配"模式——只有 1 和 2 才会赋值,其他值(如 3、-1)被静默忽略。调用方传入非法值时不会收到任何错误提示,可能导致前端 bug 难以排查。
|
|
|
|
|
+- **建议**:对非 0 的非法 status 值返回明确错误:
|
|
|
|
|
+
|
|
|
|
|
+```go
|
|
|
|
|
+if req.Status != 0 {
|
|
|
|
|
+ if req.Status != consts.StatusEnabled && req.Status != consts.StatusDisabled {
|
|
|
|
|
+ return response.ErrBadRequest("状态值无效,仅支持 1(启用) 和 2(冻结)")
|
|
|
|
|
+ }
|
|
|
|
|
+ role.Status = req.Status
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## 总结
|
|
## 总结
|
|
|
|
|
|
|
|
-| 级别 | 编号 | 问题 | 类型 |
|
|
|
|
|
-|------|------|------|------|
|
|
|
|
|
-| 🚩 High | #1 | UserList 数据越权泄漏 | 安全漏洞 |
|
|
|
|
|
-| 🚩 High | #2 | BindRoles 提权漏洞 | 安全漏洞 |
|
|
|
|
|
-| 🚩 High | #3 | appSecret 明文存储 | 安全漏洞 |
|
|
|
|
|
-| 🚩 High | #4 | UpdateUser 权限模型不一致 | 逻辑一致性 |
|
|
|
|
|
-| ⚠️ Medium | #5 | CreateUser 未加入产品成员 | 数据完整性 |
|
|
|
|
|
-| ⚠️ Medium | #6 | 缓存前缀全局可变状态 | 并发安全 |
|
|
|
|
|
-| ⚠️ Medium | #7 | AddMember 并发错误信息不友好 | 边界处理 |
|
|
|
|
|
-| ⚠️ Medium | #8 | JWT Token 体积过大风险 | 性能 |
|
|
|
|
|
-| ⚠️ Medium | #9 | RefreshToken 可无限复用 | 安全加固 |
|
|
|
|
|
-| ⚠️ Low | #10 | 用户不存在时错误信息误导 | 边界崩溃 |
|
|
|
|
|
-| ⚠️ Low | #11 | 缺少操作审计日志 | 安全合规 |
|
|
|
|
|
-| ⚠️ Low | #12 | DeptTree/ProductList 过度暴露 | 安全加固 |
|
|
|
|
|
-| ⚠️ Low | #13 | UpdateDept 修改类型缺少影响提示 | 健壮性 |
|
|
|
|
|
-
|
|
|
|
|
-**优先修复建议**:#1 和 #2 为最高优先级,直接影响数据安全和权限体系的可信度;#3 和 #4 建议在下一个迭代中修复。
|
|
|
|
|
|
|
+| 级别 | 数量 | 关键发现 |
|
|
|
|
|
+|------|------|---------|
|
|
|
|
|
+| 🚩 High | 5 | token 失效不一致、跨产品信息泄露、层级定义矛盾、数据完整性、无保护的全量禁用 |
|
|
|
|
|
+| ⚠️ Medium | 4 | RefreshToken 无轮转、gRPC/HTTP 逻辑重复、函数越权隐患、用户名冲突 |
|
|
|
|
|
+| 💡 Low | 8 | 无效权限绑定、产品访问控制、部门树全量暴露、效率优化、格式校验等 |
|
|
|
|
|
+
|
|
|
|
|
+**整体评价**:项目的架构设计合理,go-zero 框架的使用规范,核心安全机制(JWT tokenVersion、bcrypt、参数化 SQL、限流)实现到位。主要风险集中在**同一业务操作的多个入口未保持一致性**(如 UpdateUser vs UpdateUserStatus)以及**数据隔离在产品边界上的不完整**。建议优先修复 H1(token 失效不一致)和 H5(空列表全量禁用),这两个问题在生产环境中最有可能造成实际影响。
|