|
@@ -1,324 +1,232 @@
|
|
|
-# 权限管理系统 - 深度代码审计报告
|
|
|
|
|
|
|
+# 权限系统深度审计报告
|
|
|
|
|
|
|
|
-> 审计范围:`internal/` 下所有非测试源代码,包括 logic、model、middleware、handler、config、loader、server 层
|
|
|
|
|
> 审计时间:2026-04-17
|
|
> 审计时间:2026-04-17
|
|
|
-> 排除范围:`*_test.go`、`*_mock_test.go`、`testutil/`、`cli/`、`pb/`(生成代码)
|
|
|
|
|
|
|
+> 审计范围:`perms-system-server` 全部业务源码(测试代码除外)
|
|
|
|
|
+> 审计维度:逻辑一致性、并发与竞态、资源管理、数据完整性、安全漏洞、边界崩溃
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-### H1. AdminLogin 未校验用户是否为超级管理员 —— 越权访问风险
|
|
|
|
|
|
|
+### 1. UserList 接口数据越权泄漏
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/pub/adminLoginLogic.go:33-91`
|
|
|
|
|
-- **描述**:`AdminLogin` 接口仅校验了 `ManagementKey` 和用户名密码,但**没有校验用户的 `isSuperAdmin` 字段**。任何普通用户只要知道 `ManagementKey`,就能通过管理后台登录接口获取一个 `productCode=""` 的 Token。
|
|
|
|
|
-- **影响**:
|
|
|
|
|
- - 拿到此 Token 后,用户通过 JWT 中间件校验,可以调用所有 `JwtAuth` 保护的接口。
|
|
|
|
|
- - 虽然 `RequireSuperAdmin()` 会在创建产品、管理部门等操作中拦截,但 `UserDetail`、`UserList`、`RoleList`、`ProductList`、`DeptTree` 等查询接口**没有额外权限校验**,非超管用户可以通过此途径浏览所有系统数据。
|
|
|
|
|
- - `ManagementKey` 一旦泄露(如被抓包、配置文件泄露),整个系统对该用户门户大开。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
|
|
+- **文件**:`internal/logic/user/userListLogic.go` 第 45 行
|
|
|
|
|
+- **描述**:非超管用户(产品管理员)调用 UserList 时,虽然校验了 `caller.ProductCode == req.ProductCode`,但底层查询 `SysUserModel.FindListByPage` 执行的是 **全表分页查询**(无任何 WHERE 条件),返回了系统中**所有用户**,而非仅当前产品的成员用户。memberMap 只是在返回结果上附加了 memberType 信息,并不过滤非成员用户。
|
|
|
|
|
+- **影响**:产品管理员可以看到其他产品甚至全系统的用户信息(用户名、昵称、邮箱、手机号、部门等),构成**水平越权数据泄漏**。
|
|
|
|
|
+- **修复方案**:非超管用户查询时,应先查 `sys_product_member` 获取当前产品的成员 userId 列表,再用这些 userId 进行分页查询。示例:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// adminLoginLogic.go — 在密码验证通过后增加超管校验
|
|
|
|
|
-if u.IsSuperAdmin != consts.IsSuperAdminYes {
|
|
|
|
|
- return nil, response.ErrForbidden("仅超级管理员可通过管理后台登录")
|
|
|
|
|
|
|
+// userListLogic.go - 非超管场景
|
|
|
|
|
+if req.ProductCode != "" && !caller.IsSuperAdmin {
|
|
|
|
|
+ list, total, err := l.svcCtx.SysUserModel.FindListByProductMembers(
|
|
|
|
|
+ l.ctx, req.ProductCode, page, pageSize,
|
|
|
|
|
+ )
|
|
|
|
|
+ // ...
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
|
|
+需要在 Model 层新增 `FindListByProductMembers` 方法,JOIN `sys_product_member` 表过滤。
|
|
|
|
|
+
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### H2. 限流中间件 IP 提取逻辑有两个严重缺陷
|
|
|
|
|
|
|
+### 2. BindRoles 缺少角色级别校验,存在提权漏洞
|
|
|
|
|
|
|
|
-- **文件**:`internal/middleware/ratelimitMiddleware.go:24-28`
|
|
|
|
|
-- **描述**:
|
|
|
|
|
- 1. **`r.RemoteAddr` 包含端口号**:Go 的 `http.Request.RemoteAddr` 格式为 `IP:Port`(如 `192.168.1.1:54321`)。由于每个 TCP 连接的临时端口不同,限流 Key 变成了**每个连接独立计数**,同一个客户端 IP 几乎不可能触发限流。
|
|
|
|
|
- 2. **未处理反向代理场景**:生产环境通常有 Nginx/Envoy 做反向代理,此时 `RemoteAddr` 是代理服务器的 IP,所有客户端共享同一个限流桶,导致少量正常请求就会触发全局限流。
|
|
|
|
|
-- **影响**:登录接口(`/auth/login`、`/auth/adminLogin`)的暴力破解防护**形同虚设**。攻击者可以不受限制地进行密码爆破。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
|
|
+- **文件**:`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 校验:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-func (m *RateLimitMiddleware) Handle(next http.HandlerFunc) http.HandlerFunc {
|
|
|
|
|
- return func(w http.ResponseWriter, r *http.Request) {
|
|
|
|
|
- ip := extractClientIP(r)
|
|
|
|
|
- key := fmt.Sprintf("ip:%s", ip)
|
|
|
|
|
- code, _ := m.limiter.Take(key)
|
|
|
|
|
- if code == limit.OverQuota {
|
|
|
|
|
- httpx.ErrorCtx(r.Context(), w, response.ErrTooManyRequests("请求过于频繁,请稍后再试"))
|
|
|
|
|
- return
|
|
|
|
|
- }
|
|
|
|
|
- next(w, r)
|
|
|
|
|
- }
|
|
|
|
|
-}
|
|
|
|
|
-
|
|
|
|
|
-func extractClientIP(r *http.Request) string {
|
|
|
|
|
- // 优先从反代标准头提取
|
|
|
|
|
- if ip := r.Header.Get("X-Real-IP"); ip != "" {
|
|
|
|
|
- return ip
|
|
|
|
|
|
|
+// bindRolesLogic.go - 在遍历 roles 时增加
|
|
|
|
|
+caller := middleware.GetUserDetails(l.ctx)
|
|
|
|
|
+for _, r := range roles {
|
|
|
|
|
+ if r.ProductCode != productCode {
|
|
|
|
|
+ return response.ErrBadRequest("不能绑定其他产品的角色")
|
|
|
}
|
|
}
|
|
|
- if forwarded := r.Header.Get("X-Forwarded-For"); forwarded != "" {
|
|
|
|
|
- // 取第一个 IP(最靠近客户端的)
|
|
|
|
|
- if idx := strings.Index(forwarded, ","); idx != -1 {
|
|
|
|
|
- return strings.TrimSpace(forwarded[:idx])
|
|
|
|
|
- }
|
|
|
|
|
- return strings.TrimSpace(forwarded)
|
|
|
|
|
|
|
+ if r.Status != consts.StatusEnabled {
|
|
|
|
|
+ return response.ErrBadRequest("不能绑定已禁用的角色")
|
|
|
}
|
|
}
|
|
|
- // 兜底:去掉端口
|
|
|
|
|
- host, _, err := net.SplitHostPort(r.RemoteAddr)
|
|
|
|
|
- if err != nil {
|
|
|
|
|
- return r.RemoteAddr
|
|
|
|
|
|
|
+ // 非超管不能分配超出自身权限级别的角色
|
|
|
|
|
+ if !caller.IsSuperAdmin && r.PermsLevel < caller.MinPermsLevel {
|
|
|
|
|
+ return response.ErrForbidden("不能分配权限级别高于自身的角色")
|
|
|
}
|
|
}
|
|
|
- return host
|
|
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### H3. 多个查询接口存在水平越权 —— 无跨产品/无权限校验
|
|
|
|
|
|
|
+### 3. appSecret 数据库明文存储
|
|
|
|
|
|
|
|
-- **文件**:
|
|
|
|
|
- - `internal/logic/user/userDetailLogic.go` — 任意用户可查看任意用户详情
|
|
|
|
|
- - `internal/logic/user/userListLogic.go` — 任意用户可列出全系统所有用户
|
|
|
|
|
- - `internal/logic/role/roleListLogic.go` — 可传入任意 `productCode` 查看其他产品角色
|
|
|
|
|
- - `internal/logic/role/roleDetailLogic.go` — 可查看任意角色详情(含所绑定的权限 ID 列表)
|
|
|
|
|
- - `internal/logic/perm/permListLogic.go` — 可传入任意 `productCode` 查看其他产品权限
|
|
|
|
|
- - `internal/logic/member/memberListLogic.go` — 可传入任意 `productCode` 查看其他产品成员
|
|
|
|
|
-- **描述**:上述接口只经过 `JwtAuth` 中间件校验了登录态,但**没有校验调用者是否属于目标产品、是否有权访问该数据**。一个产品 A 的普通成员(MEMBER),可以通过构造请求查看产品 B 的角色、权限、成员信息。
|
|
|
|
|
-- **影响**:**信息泄露**。权限系统本身的数据(角色名称、权限列表、用户列表、成员关系)被无差别暴露给所有已登录用户,违反了多产品之间的数据隔离原则。
|
|
|
|
|
-- **修复方案**:对含 `productCode` 参数的查询接口,增加产品归属校验:
|
|
|
|
|
|
|
+- **文件**:`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`:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// 在 roleListLogic.go 等接口中增加
|
|
|
|
|
-caller := middleware.GetUserDetails(l.ctx)
|
|
|
|
|
-if caller == nil {
|
|
|
|
|
- return nil, response.ErrUnauthorized("未登录")
|
|
|
|
|
-}
|
|
|
|
|
-if !caller.IsSuperAdmin {
|
|
|
|
|
- if caller.ProductCode != req.ProductCode {
|
|
|
|
|
- return nil, response.ErrForbidden("无权访问该产品的数据")
|
|
|
|
|
- }
|
|
|
|
|
|
|
+// syncPermsLogic.go / permserver.go
|
|
|
|
|
+if err := bcrypt.CompareHashAndPassword(
|
|
|
|
|
+ []byte(product.AppSecretHash), []byte(req.AppSecret),
|
|
|
|
|
+); err != nil {
|
|
|
|
|
+ return nil, response.ErrUnauthorized("appSecret验证失败")
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
-对 `UserDetail` 和 `UserList`,应限制非超管用户只能查看自己所在产品的成员。
|
|
|
|
|
-
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### H4. UpdateUser 权限校验与同类接口不一致 —— 超管可被越权修改
|
|
|
|
|
|
|
+### 4. UpdateUser 权限模型与其他接口不一致,产品管理员无法修改自己创建的用户
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/user/updateUserLogic.go:31-45`
|
|
|
|
|
-- **描述**:`UpdateUser` 的权限逻辑为「只能改自己,或者超管改别人」。但对比 `UpdateUserStatus`(使用了 `CheckManageAccess` 检查部门层级和权限等级),`UpdateUser` 缺少以下校验:
|
|
|
|
|
- 1. **超管 A 可以修改超管 B 的信息**(包括部门、状态),没有类似 `UpdateUserStatus` 中 "不能修改超级管理员的状态" 的保护。
|
|
|
|
|
- 2. **没有 `CheckManageAccess`**:不校验部门层级关系和 `permsLevel`,超管可以直接修改任何用户的部门归属(`DeptId`),这可能绕过部门层级隔离的安全模型。
|
|
|
|
|
-- **影响**:如果系统中有多个超级管理员,超管 A 可以将超管 B 的状态改为 `StatusDisabled`(冻结),或将其部门改为下级部门从而降低其管理范围。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
|
|
+- **文件**:`internal/logic/user/updateUserLogic.go` 第 37-45 行
|
|
|
|
|
+- **描述**:`UpdateUser` 的权限判断逻辑为"只有超管或用户自身可修改",而系统中 `UpdateUserStatus`、`BindRoles`、`SetUserPerms` 等接口均使用 `CheckManageAccess`(支持产品管理员和部门层级管理)。这导致产品管理员可以创建用户(`CreateUser`)、冻结用户(`UpdateUserStatus`)、绑定角色(`BindRoles`),却**无法修改**用户的昵称、邮箱、部门等基本信息。
|
|
|
|
|
+- **影响**:产品管理员的管理权限出现逻辑断裂——能创建和冻结用户,但不能编辑用户信息,不得不依赖超管操作,严重影响管理效率。
|
|
|
|
|
+- **修复方案**:将 UpdateUser 的权限模型统一为 `CheckManageAccess`,并保留对自身修改的限制(不能改自己的部门和状态):
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-// 对非自身操作增加更严格的校验
|
|
|
|
|
-if caller.UserId != req.Id {
|
|
|
|
|
- // 仅超管可操作
|
|
|
|
|
- if !caller.IsSuperAdmin {
|
|
|
|
|
- return response.ErrForbidden("仅超管可修改其他用户信息")
|
|
|
|
|
|
|
+func (l *UpdateUserLogic) UpdateUser(req *types.UpdateUserReq) error {
|
|
|
|
|
+ caller := middleware.GetUserDetails(l.ctx)
|
|
|
|
|
+ if caller == nil {
|
|
|
|
|
+ return response.ErrUnauthorized("未登录")
|
|
|
}
|
|
}
|
|
|
- // 不允许通过此接口修改其他超管
|
|
|
|
|
- if user.IsSuperAdmin == consts.IsSuperAdminYes {
|
|
|
|
|
- if req.Status != 0 || req.DeptId != nil {
|
|
|
|
|
- return response.ErrForbidden("不能修改其他超级管理员的状态和部门")
|
|
|
|
|
- }
|
|
|
|
|
- }
|
|
|
|
|
-}
|
|
|
|
|
-```
|
|
|
|
|
-
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-### H5. BindRolePerms 未对 PermIds 去重 —— 重复 ID 导致数据库约束错误
|
|
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/role/bindRolePermsLogic.go:41-54`
|
|
|
|
|
-- **描述**:`BindRoles` 接口对 `RoleIds` 做了去重处理(第 47-57 行),但 `BindRolePerms` 没有对 `PermIds` 做同样的去重。当客户端传入重复的 `PermIds`(如 `[1, 1, 2]`)时:
|
|
|
|
|
- - `FindByIds` 返回去重后的 2 条记录
|
|
|
|
|
- - `len(perms) != len(req.PermIds)` → `2 != 3` → 返回「包含无效的权限ID」
|
|
|
|
|
- - 错误信息具有误导性,实际上权限 ID 都是有效的,只是有重复
|
|
|
|
|
- - 如果绕过该检查(例如未来修改了校验逻辑),`BatchInsertWithTx` 会因 `UNIQUE KEY uk_role_perm (roleId, permId)` 约束而报错
|
|
|
|
|
-- **影响**:前端传入重复数据时,用户收到令人困惑的错误提示,体验差且难以排查。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
-
|
|
|
|
|
-```go
|
|
|
|
|
-// 在 BindRolePerms 方法开头增加去重逻辑(同 BindRoles 的处理方式)
|
|
|
|
|
-if len(req.PermIds) > 0 {
|
|
|
|
|
- seen := make(map[int64]bool, len(req.PermIds))
|
|
|
|
|
- uniqueIds := make([]int64, 0, len(req.PermIds))
|
|
|
|
|
- for _, id := range req.PermIds {
|
|
|
|
|
- if !seen[id] {
|
|
|
|
|
- seen[id] = true
|
|
|
|
|
- uniqueIds = append(uniqueIds, id)
|
|
|
|
|
|
|
+ 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
|
|
|
}
|
|
}
|
|
|
}
|
|
}
|
|
|
- req.PermIds = uniqueIds
|
|
|
|
|
|
|
+ // ... 后续逻辑不变
|
|
|
}
|
|
}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### H6. SyncPerms 接口缺乏鉴权强度 —— 仅靠 LoginRateLimit 保护
|
|
|
|
|
-
|
|
|
|
|
-- **文件**:`internal/handler/routes.go:176-188` + `internal/logic/pub/syncPermsLogic.go`
|
|
|
|
|
-- **描述**:`/api/perm/sync` 接口被分配到 `LoginRateLimit` 中间件组(而非 `JwtAuth`),且由于 H2 中的限流失效问题,该接口实际上**几乎没有任何访问频率限制**。虽然接口内部使用 `appKey + appSecret` 做认证,但:
|
|
|
|
|
- - `appKey` 和 `appSecret` 是长期有效的静态凭证
|
|
|
|
|
- - 没有 IP 白名单、签名时间戳、Nonce 等额外防重放机制
|
|
|
|
|
- - 攻击者获取凭证后可无限次调用,覆盖或禁用产品的所有权限
|
|
|
|
|
-- **影响**:一旦 `appKey/appSecret` 泄露,攻击者可以:
|
|
|
|
|
- - 传入空的 `Perms` 列表,将目标产品**所有权限禁用**
|
|
|
|
|
- - 注入恶意权限 Code,污染权限数据
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
- 1. 为 SyncPerms 接口增加独立的限流策略(区别于登录限流)
|
|
|
|
|
- 2. 考虑增加请求签名(`timestamp + nonce + HMAC(appSecret, body)`)防重放
|
|
|
|
|
- 3. 在运维层面增加 IP 白名单
|
|
|
|
|
-
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-## ⚠️ 健壮性与性能建议 (Medium)
|
|
|
|
|
|
|
+## ⚠️ 健壮性与性能建议 (Medium/Low)
|
|
|
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-### M1. 配置文件明文存储敏感信息
|
|
|
|
|
-
|
|
|
|
|
-- **文件**:`etc/perm-api-dev.yaml`(及其他环境配置)
|
|
|
|
|
-- **描述**:MySQL 密码、Redis 密码、JWT Secret、ManagementKey 均以明文存储在 YAML 文件中。如果这些文件被提交到 Git 仓库,所有有仓库访问权限的人都能获取生产环境密钥。
|
|
|
|
|
-- **建议**:
|
|
|
|
|
- - 生产环境使用环境变量注入或密钥管理服务(如 Vault、AWS Secrets Manager)
|
|
|
|
|
- - 开发环境配置加入 `.gitignore`,仅保留 `perm-api-example.yaml` 模板
|
|
|
|
|
-
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-### M2. CreateUser 不会自动关联产品成员 —— 业务流程断裂
|
|
|
|
|
|
|
+### 5. [Medium] CreateUser 未将用户加入产品成员,工作流存在断裂
|
|
|
|
|
|
|
|
- **文件**:`internal/logic/user/createUserLogic.go`
|
|
- **文件**:`internal/logic/user/createUserLogic.go`
|
|
|
-- **描述**:`CreateUser` 接口要求 `RequireProductAdminFor(productCode)` 校验调用者是产品管理员,但创建用户后**并不自动将新用户加入该产品**。调用方需要额外调用 `AddMember` 接口,形成两步操作。
|
|
|
|
|
-- **影响**:
|
|
|
|
|
- - 如果 `CreateUser` 成功但 `AddMember` 失败(网络中断、前端 Bug),系统中会出现"孤儿用户"——用户存在但不属于任何产品,无法登录任何产品
|
|
|
|
|
- - 增加了前端集成的复杂度和出错概率
|
|
|
|
|
-- **建议**:在 `CreateUser` 事务中同时插入 `sys_product_member` 记录,或者至少返回一个明确的提示告知前端需要调用 AddMember。
|
|
|
|
|
|
|
+- **描述**:`CreateUser` 要求 `RequireProductAdminFor(productCode)` 校验产品管理员身份,但创建用户后并未将其加入当前产品的成员列表。新建用户需要管理员再单独调用 `AddMember` 才能登录和使用产品。
|
|
|
|
|
+- **影响**:管理员可能误以为创建用户即完成了入组操作,导致新用户无法登录。增加了操作步骤和出错概率。
|
|
|
|
|
+- **建议**:考虑在 CreateUser 中增加可选参数 `memberType`,当传入时在同一事务中自动创建产品成员记录;或在文档/前端层面明确引导管理员完成两步操作。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### M3. Model 初始化修改包级变量 —— 非线程安全
|
|
|
|
|
|
|
+### 6. [Medium] 缓存前缀变量是包级可变全局状态
|
|
|
|
|
|
|
|
-- **文件**:`internal/model/user/sysUserModel_gen.go:75-80`(所有 model 的 `_gen.go` 均有此问题)
|
|
|
|
|
-- **描述**:`newSysUserModel()` 函数在初始化时会修改包级变量 `cacheSysUserIdPrefix` 和 `cacheSysUserUsernamePrefix`。这些变量在包加载时已有初始值,被函数调用覆写。
|
|
|
|
|
- - 虽然当前代码只在 `NewModels()` 中调用一次,不会出现并发问题
|
|
|
|
|
- - 但作为生成代码模板,如果未来存在多实例或单测并行场景,会产生数据竞争
|
|
|
|
|
-- **建议**:将 cache prefix 存储在 struct 实例中,而非修改包级变量。由于这是 goctl 生成代码,建议修改 `cli/goctl/model/model-new.tpl` 模板。
|
|
|
|
|
|
|
+- **文件**:各 `*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. gRPC Login 的限流器可能为 nil
|
|
|
|
|
|
|
+### 7. [Medium] AddMember 并发重复请求错误信息不友好
|
|
|
|
|
|
|
|
-- **文件**:`internal/server/permserver.go:116-125`
|
|
|
|
|
-- **描述**:`Login` 方法中有 `if s.svcCtx.GrpcLoginLimiter != nil` 的判断,说明设计上允许 limiter 为 nil。但在 `servicecontext.go:30` 中 limiter 总是被创建。如果未来配置变更导致 Redis 不可用,limiter 创建会 panic(`redis.MustNewRedis`),而非优雅降级。
|
|
|
|
|
-- **建议**:与当前实现保持一致,确保 `GrpcLoginLimiter` 始终非 nil,或在 `NewServiceContext` 中做容错处理。
|
|
|
|
|
-
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-### M5. UserDetailsLoader.loadPerms 中研发部门判定可能不符合预期
|
|
|
|
|
-
|
|
|
|
|
-- **文件**:`internal/loaders/userDetailsLoader.go:312-316`
|
|
|
|
|
-- **描述**:
|
|
|
|
|
|
|
+- **文件**:`internal/logic/member/addMemberLogic.go` 第 52-55 行
|
|
|
|
|
+- **描述**:使用 check-then-insert 模式检查成员是否已存在。若两个请求并发执行,都通过了检查,其中一个在 Insert 时会触发数据库唯一约束错误(`uk_product_user`),但此错误未被捕获转换为友好提示,而是返回原始的 500 错误。
|
|
|
|
|
+- **影响**:并发场景下用户收到 "服务器内部错误" 而非 "该用户已是该产品成员"。
|
|
|
|
|
+- **建议**:在 Insert 错误处理中捕获唯一约束冲突:
|
|
|
|
|
|
|
|
```go
|
|
```go
|
|
|
-if ud.IsSuperAdmin ||
|
|
|
|
|
- ud.MemberType == consts.MemberTypeAdmin ||
|
|
|
|
|
- ud.MemberType == consts.MemberTypeDeveloper ||
|
|
|
|
|
- (ud.DeptType == consts.DeptTypeDev && ud.DeptStatus == consts.StatusEnabled) {
|
|
|
|
|
|
|
+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("该用户已是该产品成员")
|
|
|
|
|
+ }
|
|
|
|
|
+ return nil, err
|
|
|
|
|
+}
|
|
|
```
|
|
```
|
|
|
|
|
|
|
|
- 研发部门(`DeptType == "DEV"`)的判定**不与 productCode 关联**——只要用户所在部门类型是 `DEV` 且部门启用,该用户在**所有产品**下都自动拥有全量权限。这意味着一个被拉进产品 A 的 MEMBER 类型成员,如果碰巧在研发部门,他在产品 A 下拥有的权限和 ADMIN 一样。
|
|
|
|
|
-- **影响**:研发部门成员的权限范围可能超出业务预期,与成员类型(MEMBER)赋予的权限不匹配。
|
|
|
|
|
-- **建议**:确认此行为是否为设计意图。如果研发部门全量权限仅应作用于特定产品,需增加产品关联判断。
|
|
|
|
|
-
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### M6. RefreshToken 不会续签 refreshToken 本身
|
|
|
|
|
|
|
+### 8. [Medium] JWT 中嵌入了完整权限列表,Token 体积随权限数增长
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/pub/refreshTokenLogic.go:67-68`
|
|
|
|
|
-- **描述**:`RefreshToken` 接口返回新的 `accessToken`,但**原样返回旧的 `refreshToken`**。随着时间推移,refreshToken 会过期(7 天),用户被迫重新登录。
|
|
|
|
|
-- **影响**:对于需要长期保持登录状态的场景(如桌面客户端、后台管理系统),用户体验不佳——每 7 天必须重新输入密码。
|
|
|
|
|
-- **建议**:根据业务需求决定是否在每次刷新时签发新的 refreshToken(滑动过期策略)。如果不续签,应在 API 文档中明确说明 refreshToken 有效期为固定 7 天。
|
|
|
|
|
|
|
+- **文件**:`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 中移除。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### M7. UserDetailsLoader 缓存清理使用 SCAN —— 性能与兼容性风险
|
|
|
|
|
|
|
+### 9. [Medium] RefreshToken 未限制刷新次数,旧 RefreshToken 可无限复用
|
|
|
|
|
|
|
|
-- **文件**:`internal/loaders/userDetailsLoader.go:162-180`
|
|
|
|
|
-- **描述**:`cleanByPattern` 使用 `SCAN` 命令按 pattern 匹配并删除缓存 key。代码注释中已标注此方法不兼容 Redis Cluster。此外:
|
|
|
|
|
- - `CleanByProduct` 使用 pattern `*:ud:*:{productCode}`,在产品成员较多时可能扫描大量 key
|
|
|
|
|
- - `UpdateDept` 中对每个子部门的每个用户逐个调用 `Clean`,如果部门内有几十个用户,会产生多次 SCAN 操作
|
|
|
|
|
-- **建议**:
|
|
|
|
|
- - 如果确定使用单节点 Redis,当前实现可接受
|
|
|
|
|
- - 若考虑未来迁移到 Redis Cluster,建议使用 Hash Tag(如 `{productCode}:ud:userId`)或维护一个 Set 记录某个产品下的所有缓存 key,以支持批量删除
|
|
|
|
|
|
|
+- **文件**:`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 的黑名单。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### M8. 部分接口缺少输入长度校验
|
|
|
|
|
|
|
+### 10. [Low] 用户不存在时 JwtAuth 中间件返回误导性错误信息
|
|
|
|
|
|
|
|
-- **文件**:各 `createXxxLogic.go`、`updateXxxLogic.go`
|
|
|
|
|
-- **描述**:以下字段没有长度校验,但数据库有 `varchar` 长度限制:
|
|
|
|
|
- - `username`(最大 64)、`nickname`(最大 64)、`email`(最大 64)
|
|
|
|
|
- - `productCode`(最大 64)、`productName`(最大 64)
|
|
|
|
|
- - `roleName`(最大 64)、`remark`(最大 255)
|
|
|
|
|
- - `dept.name`(最大 64)、`dept.path`(最大 512)
|
|
|
|
|
-
|
|
|
|
|
- 当前未做前端/后端长度校验,超长输入会直接触发 MySQL 的 `Data too long` 错误(1406),返回不友好的 500 错误。
|
|
|
|
|
-- **建议**:在 Logic 层统一增加关键字段的长度校验,返回可读的 400 错误信息。
|
|
|
|
|
|
|
+- **文件**:`internal/middleware/jwtauthMiddleware.go` 第 74-78 行;`internal/loaders/userDetailsLoader.go` 第 129-133 行
|
|
|
|
|
+- **描述**:当 JWT 有效但用户已被删除时,`UserDetailsLoader.Load` 返回一个默认的 `UserDetails`(Status=0)。中间件判断 `ud.Status != StatusEnabled` 后返回"账号已被冻结"。但实际上用户是不存在/已被删除。
|
|
|
|
|
+- **影响**:用户收到错误的提示信息,可能导致困惑或误导排查方向。
|
|
|
|
|
+- **建议**:在中间件中增加用户是否存在的判断:
|
|
|
|
|
|
|
|
----
|
|
|
|
|
-
|
|
|
|
|
-## 💡 低风险优化建议 (Low)
|
|
|
|
|
|
|
+```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
|
|
|
|
|
+}
|
|
|
|
|
+```
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-### L1. JWT Claims 中存储完整权限列表可能导致 Token 膨胀
|
|
|
|
|
-
|
|
|
|
|
-- **文件**:`internal/logic/auth/jwt.go:22-38`
|
|
|
|
|
-- **描述**:`Claims.Perms` 是 `[]string`,权限 Code 字符串数组被完整编码进 JWT。如果某个产品配置了数百个权限,Token 可能达到数 KB 甚至超过 HTTP Header 限制(通常 8KB)。
|
|
|
|
|
-- **建议**:考虑只在 JWT 中存储必要标识(userId、productCode、memberType),权限列表由服务端通过 `UserDetailsLoader` 实时获取(当前中间件已经在这样做)。
|
|
|
|
|
|
|
+### 11. [Low] 缺少操作审计日志
|
|
|
|
|
|
|
|
-### L2. DeptTree 返回所有部门包含已禁用的
|
|
|
|
|
|
|
+- **描述**:当前系统在所有写操作(创建用户、绑定角色、修改权限、冻结用户、删除角色等)中未记录操作审计日志。仅有 go-zero 框架默认的请求日志。
|
|
|
|
|
+- **影响**:当发生安全事件(如权限被篡改、用户被异常冻结)时,无法追溯是谁在什么时间执行了什么操作。对于权限管理系统,审计追溯能力是合规性的基本要求。
|
|
|
|
|
+- **建议**:设计一个 `sys_audit_log` 表,记录 `operator_id`、`action`、`target_type`、`target_id`、`detail`、`ip`、`timestamp` 等字段。在关键业务逻辑中通过异步方式写入审计记录,避免影响主流程性能。
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/dept/deptTreeLogic.go:27`
|
|
|
|
|
-- **描述**:`FindAll` 查询所有部门不区分状态,禁用的部门也会出现在树中。
|
|
|
|
|
-- **建议**:根据业务需求,可增加参数控制是否过滤禁用部门,或在返回中标注状态供前端处理。
|
|
|
|
|
-
|
|
|
|
|
-### L3. CreateProduct 返回明文 AdminPassword
|
|
|
|
|
|
|
+---
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/product/createProductLogic.go:116-124`
|
|
|
|
|
-- **描述**:创建产品时自动生成的管理员密码在 HTTP 响应中明文返回。若响应被日志系统记录(如 access log、网关日志),密码可能泄露。
|
|
|
|
|
-- **建议**:确保 API 网关/日志系统不记录响应体,或改为邮件/消息通知的方式下发初始密码。
|
|
|
|
|
|
|
+### 12. [Low] DeptTree 和 ProductList 对非超管暴露全量数据
|
|
|
|
|
|
|
|
-### L4. 错误处理中 `errors.As` 与 `==` 混用
|
|
|
|
|
|
|
+- **文件**:`internal/logic/dept/deptTreeLogic.go`;`internal/logic/product/productListLogic.go`
|
|
|
|
|
+- **描述**:`DeptTree` 接口无任何权限过滤,所有已登录用户可看到整个组织架构。`ProductList` 也对所有用户返回全部产品列表(AppKey 已对非超管隐藏)。
|
|
|
|
|
+- **影响**:部门结构和产品列表属于组织内部信息,虽然不包含高敏感数据,但在安全要求较高的场景下可能不符合最小暴露原则。
|
|
|
|
|
+- **建议**:根据实际业务需求评估是否需要按产品/部门范围过滤。如果当前业务场景下所有用户确实需要查看组织架构(如选择部门),可保持现状。
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/pub/loginService.go:33` 使用 `== user.ErrNotFound`,而 `response.go:47` 使用 `errors.As`
|
|
|
|
|
-- **描述**:`ErrNotFound` 比较使用 `==`(值比较),如果未来 ErrNotFound 被 `fmt.Errorf("%w", ...)` 包装,`==` 会失效。
|
|
|
|
|
-- **建议**:统一使用 `errors.Is(err, user.ErrNotFound)` 进行哨兵错误判断。
|
|
|
|
|
|
|
+---
|
|
|
|
|
|
|
|
-### L5. ChangePassword 成功后不会使旧 Token 失效
|
|
|
|
|
|
|
+### 13. [Low] UpdateDept 修改 deptType 时缺少影响评估
|
|
|
|
|
|
|
|
-- **文件**:`internal/logic/auth/changePasswordLogic.go`
|
|
|
|
|
-- **描述**:修改密码后清理了 UserDetails 缓存,但已签发的 Access Token 和 Refresh Token 仍然有效(最长可达 7 天)。如果用户因密码泄露而修改密码,攻击者持有的旧 Token 仍可正常使用。
|
|
|
|
|
-- **建议**:引入 Token 版本号(存储在用户记录中),修改密码时递增版本号,中间件校验时比对版本号。
|
|
|
|
|
|
|
+- **文件**:`internal/logic/dept/updateDeptLogic.go`
|
|
|
|
|
+- **描述**:将部门类型从 `DEV`(研发)改为 `NORMAL`,该部门下所有用户会立即失去"全权限"特权(因 `loadPerms` 中 DEV 部门自动获取全量权限的逻辑不再生效)。代码正确清除了缓存,但未在响应中提示此操作的影响范围。
|
|
|
|
|
+- **影响**:超管执行部门类型变更后,该部门及子部门下的用户可能突然失去大量权限,如果未提前通知,可能导致业务中断。
|
|
|
|
|
+- **建议**:在响应中返回受影响的用户数量,或在执行前增加确认机制。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## 📊 审计总结
|
|
|
|
|
|
|
+## 总结
|
|
|
|
|
|
|
|
-| 级别 | 数量 | 关键词 |
|
|
|
|
|
-|------|------|--------|
|
|
|
|
|
-| 🚩 High | 6 | 越权登录、限流失效、水平越权、权限校验不一致、数据校验缺失、接口防护不足 |
|
|
|
|
|
-| ⚠️ Medium | 8 | 明文密钥、流程断裂、线程安全、缓存一致性、权限范围、输入校验 |
|
|
|
|
|
-| 💡 Low | 5 | Token 膨胀、状态过滤、明文密码返回、错误处理、Token 吊销 |
|
|
|
|
|
-
|
|
|
|
|
-**优先修复建议**:H1(管理后台越权)→ H2(限流失效)→ H3(水平越权)→ H4(权限不一致)→ M1(密钥管理)→ H5/H6
|
|
|
|
|
-
|
|
|
|
|
----
|
|
|
|
|
|
|
+| 级别 | 编号 | 问题 | 类型 |
|
|
|
|
|
+|------|------|------|------|
|
|
|
|
|
+| 🚩 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 建议在下一个迭代中修复。
|