|
|
@@ -1,442 +1,530 @@
|
|
|
# 权限管理系统 - 深度代码审计报告
|
|
|
|
|
|
-> 审计范围：`/internal` 下全部非测试生产代码（logic、model、middleware、handler、loaders、server、svc、config、consts、response、util）及入口文件 `perm.go`、gRPC 客户端 `permclient/`。
|
|
|
+> 审计范围：`/internal` 下全部非测试生产代码（logic、model、middleware、handler、loaders、server、svc、consts、response、util）及入口文件 `perm.go`。
|
|
|
> 审计时间：2026-04-18
|
|
|
-> 审计重点：逻辑一致性、并发竞态、数据完整性、水平越权、缓存一致性、僵尸代码、N+1、接口契约。
|
|
|
+> 审计重点：业务逻辑闭环、跨接口一致性、权限绕过、缓存一致性、并发竞态、资源与性能、僵尸代码、接口契约完整性。
|
|
|
+> 相对上一轮：H-1（BindRoles 误拦截 ADMIN）、H-2（GetUserPerms 未校验状态）、H-3（DEV 部门绕过）、M-2（批量 DELETE）、M-3/M-4（roleIds 语义）、M-5（UpdateDept 级联）、M-6（Claims.Perms）、M-11（DeleteDept TOCTOU）、L-3（UpdateDept 乐观锁）均已修复。本报告聚焦残留问题与本轮新发现。
|
|
|
|
|
|
---
|
|
|
|
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
-### H-1. BindRoles 的 permsLevel 越级校验错误地封死了 ADMIN/DEVELOPER 成员
|
|
|
+### H-1. 禁用产品后，已持有 token 的成员仍可正常使用该产品
|
|
|
|
|
|
-- **位置**：`internal/logic/user/bindRolesLogic.go` 第 60-82 行
|
|
|
+- **位置**：
|
|
|
+ - `internal/logic/product/updateProductLogic.go:30-63`
|
|
|
+ - `internal/middleware/jwtauthMiddleware.go:45-89`
|
|
|
+ - `internal/loaders/userDetailsLoader.go:280-290`（`loadProduct`）、`348-364`（`loadPerms`）
|
|
|
+ - `internal/server/permserver.go:157-222`（`VerifyToken` / `GetUserPerms`）
|
|
|
- **描述**：
|
|
|
- ```go
|
|
|
- caller := middleware.GetUserDetails(l.ctx)
|
|
|
- ...
|
|
|
- for _, r := range roles {
|
|
|
- ...
|
|
|
- if caller != nil && !caller.IsSuperAdmin {
|
|
|
- if caller.MinPermsLevel == 0 || r.PermsLevel < caller.MinPermsLevel {
|
|
|
- return response.ErrForbidden("不能分配权限级别高于自身的角色")
|
|
|
- }
|
|
|
- }
|
|
|
- }
|
|
|
- ```
|
|
|
+ `UpdateProduct` 在将 `sys_product.status` 置为 `Disabled` 之后，只做了 `UserDetailsLoader.CleanByProduct(product.Code)`。但：
|
|
|
+
|
|
|
+ 1. `loadProduct` 从 DB 只取 `ProductName`，**没有把 `product.Status` 写入 `UserDetails`**。
|
|
|
+ 2. `loadPerms` 的"全量权限"短路条件里完全没有引用产品状态，因此哪怕产品被禁用，`IsSuperAdmin / ADMIN / DEVELOPER / DEV 部门` 四类用户重新 `Load` 后仍会拿到完整 `perms`。
|
|
|
+ 3. `jwtauthMiddleware.Handle` 只校验 `ud.Username / ud.Status / claims.TokenVersion`，**没有校验产品状态**。
|
|
|
+ 4. 对外的 `gRPC VerifyToken` 和 `gRPC GetUserPerms` 也没有校验产品状态。
|
|
|
+ 5. 产品登录入口 `ValidateProductLogin` 是唯一校验了 `product.Status != Enabled` 的点；但这仅影响**新登录**，对已经签发的 access / refresh token 无任何影响。
|
|
|
+
|
|
|
+ 也就是说：管理员把一个产品"冻结"之后，该产品的所有在线用户在整个 `AccessExpire`（甚至通过 `RefreshToken` 可以一直续期到 `RefreshExpire`）窗口内都能继续访问产品端的所有接口，并且接入方通过 gRPC `GetUserPerms` / `VerifyToken` 拿到的权限和"有效"状态也依然是放行的。
|
|
|
+
|
|
|
+- **影响**：
|
|
|
+ - "禁用产品"这一核心管控动作**近乎无效**：离线下线、应急止损、合规处置场景下，管理员无法阻断业务。
|
|
|
+ - 攻击面：当某个产品因为安全事件需要临时下线时（例如 appSecret 泄露、业务侧数据异常），除物理删除该产品之外，没有办法收回其成员的访问能力。
|
|
|
+ - 对接入方（产品服务端）的一致性漏洞：管理系统显示"产品禁用"、但对外 RPC 依然告知"这个用户有全部权限"。
|
|
|
+
|
|
|
+- **修复方案**：
|
|
|
+ - `userDetailsLoader` 的 `UserDetails` 增加 `ProductStatus int64` 字段，`loadProduct` 赋值。
|
|
|
+ - `loadPerms` 在所有"自动给全量权限"的分支上叠加 `ud.ProductStatus == StatusEnabled` 前置；或者在 `loadPerms` 入口直接：
|
|
|
+
|
|
|
+ ```go
|
|
|
+ if ud.ProductCode != "" && ud.ProductStatus != consts.StatusEnabled {
|
|
|
+ ud.Perms = nil
|
|
|
+ return
|
|
|
+ }
|
|
|
+ ```
|
|
|
+
|
|
|
+ - `jwtauthMiddleware.Handle` 与 `RefreshToken` / `VerifyToken` / `GetUserPerms` 在 `claims.ProductCode != ""` 时统一校验 `ud.ProductStatus == StatusEnabled`，非启用直接 `403 "该产品已被禁用"`。
|
|
|
+ - `UpdateProduct` 在置 `Disabled` 时，**同步把该产品所有成员的 `tokenVersion+1`**（或引入一个 `product_token_epoch`），从而让所有既有 token 立即作废。推荐后者：新增 `sys_product.tokenEpoch`，access token 里带 `productEpoch`，中间件对比。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### H-2. JWT 中间件未校验"产品成员是否被禁用"，造成已被剔除/禁用成员仍可访问业务接口
|
|
|
+
|
|
|
+- **位置**：`internal/middleware/jwtauthMiddleware.go:73-87`
|
|
|
+- **描述**：`RefreshToken`（`refreshTokenLogic.go:53`、`permserver.go:125`）、`VerifyToken`（`permserver.go:174`）、`GetUserPerms`（`permserver.go:214`）都会在 `productCode != "" && !IsSuperAdmin` 时校验 `ud.MemberType != ""`——这正是 `loadMembership` 在成员不存在或 `status != Enabled` 时的返回值。
|
|
|
+
|
|
|
+ 但是 **HTTP 主流量入口 `JwtAuth` 中间件**却没有这个校验，只看用户自身 `status`。结果：
|
|
|
|
|
|
- 这段 permsLevel 校验对**所有非超管调用者**生效，包括 `ADMIN` / `DEVELOPER` 成员。问题在于：
|
|
|
- 1. 在 `userDetailsLoader.loadRoles` 中，`MinPermsLevel` **默认为 `math.MaxInt64`**（见 `internal/loaders/userDetailsLoader.go` 第 227 行），仅当用户存在启用角色时才会被覆盖。
|
|
|
- 2. 产品自动创建的 `admin_{code}` 管理员、以及大部分 `ADMIN`/`DEVELOPER` 成员通过 `sys_product_member.memberType` 获得权限，**不关联任何 `sys_user_role` 角色**，因此他们的 `MinPermsLevel` 永远是 `math.MaxInt64`。
|
|
|
- 3. 代码中的 sentinel 判断 `caller.MinPermsLevel == 0` 永远不会命中（MinPermsLevel 只会是 `math.MaxInt64` 或 `[1,999]`）。
|
|
|
- 4. 因此 `r.PermsLevel (1-999) < math.MaxInt64` 必然为 `true`，**任何 permsLevel 的角色都会被判为"权限级别高于自身"而拒绝绑定**。
|
|
|
+ - 管理员执行 `UpdateMember.Status=Disabled` 后，`loader.Del` 清理缓存，但旧 access token 未作废（`tokenVersion` 未变）。
|
|
|
+ - 被禁用成员带着这张 token 继续请求 `/api/dept/tree`、`/api/perm/list`、`/api/user/detail?id=self` 等"只 JwtAuth 不做业务校验"的接口，**全部放行**。
|
|
|
+ - 更严重的是 `loadPerms`：`MemberType == ""` 时会跳过"全量权限"分支，但普通成员分支仍会基于 `sys_user_role` / `sys_user_perm` 返回权限集。也就是被踢出的成员在中间件层不被拒，随后其 `ud.Perms` 还被填充（如果前端仍按 `/api/perm/list` 来做菜单，依然能看到"这个产品下自己之前拥有的权限"）。
|
|
|
|
|
|
- 对比 `checkPermLevel`（access.go 第 143-151 行）的设计：只有在 `callerPri == targetPri`（都是 MEMBER）时才会进入 permsLevel 比较，而 ADMIN/DEVELOPER 因 MemberType 优先级更高就已经放行。`bindRolesLogic` 的这段校验缺失了这一前置判定。
|
|
|
+ `loadPerms` 的 DEV 部门短路已经加了 `ud.MemberType != ""` 这层护栏（第 358 行），但角色/用户权限并没有这层保护，导致普通成员分支依然会生效。
|
|
|
|
|
|
- **影响**：
|
|
|
- - 在真实产线，`ADMIN` 成员（包括系统自动创建的 `admin_{code}`）**无法给任何用户绑定任何角色**，管理员最核心的运营能力被封死。
|
|
|
- - `DEVELOPER` 成员同样被封死。
|
|
|
- - 该 bug 在测试中未暴露，是因为 `bindRolesLogic_test.go` 第 314 行人为构造了 `MinPermsLevel: 50` 的 ADMIN 上下文（见 TC-0208），并不反映 loader 真实产出的 `math.MaxInt64`。
|
|
|
+ - "禁用成员"的实际效果仅对**登录时刻**生效；对**在线成员**只有间接约束（token 过期后才生效）。
|
|
|
+ - 配合 H-1（禁用产品），这条路径让"访问控制回收"能力在产品/成员两个维度都失灵。
|
|
|
+ - 一致性问题：`RefreshToken` 拒绝了已禁用成员，但 HTTP 业务接口不拒，用户可能看到"业务继续可用 / 但 refresh 已失败"的分裂状态。
|
|
|
|
|
|
-- **修复方案**：与 `checkPermLevel` 对齐，只对同为 `MEMBER` 类型的调用者做 permsLevel 比较；对 ADMIN/DEVELOPER 直接放行：
|
|
|
+- **修复方案**：在 `jwtauthMiddleware.Handle` 的 403 校验中对齐 `RefreshToken`：
|
|
|
|
|
|
```go
|
|
|
- if caller != nil && !caller.IsSuperAdmin &&
|
|
|
- caller.MemberType != consts.MemberTypeAdmin &&
|
|
|
- caller.MemberType != consts.MemberTypeDeveloper {
|
|
|
- // 只有 MEMBER 类型调用者才需要 permsLevel 越级校验
|
|
|
- if caller.MinPermsLevel < math.MaxInt64 && r.PermsLevel < caller.MinPermsLevel {
|
|
|
- return response.ErrForbidden("不能分配权限级别高于自身的角色")
|
|
|
- }
|
|
|
+ if claims.ProductCode != "" && !ud.IsSuperAdmin && ud.MemberType == "" {
|
|
|
+ httpx.ErrorCtx(r.Context(), w, response.NewCodeError(403, "您已不是该产品的有效成员"))
|
|
|
+ return
|
|
|
}
|
|
|
```
|
|
|
|
|
|
- 同时，`caller.MinPermsLevel == 0` 这段无效判断应改为 `caller.MinPermsLevel == math.MaxInt64`（或按上面的写法从条件里移除）。
|
|
|
+ 同时 `loadPerms` 的普通成员分支也应当在入口加：
|
|
|
+
|
|
|
+ ```go
|
|
|
+ if ud.ProductCode != "" && !ud.IsSuperAdmin && ud.MemberType == "" {
|
|
|
+ return // 非有效成员，权限置空
|
|
|
+ }
|
|
|
+ ```
|
|
|
+
|
|
|
+ （这条在第 353-358 行的"自动全量"里已有，但需要抽出作用到整函数）
|
|
|
|
|
|
---
|
|
|
|
|
|
-### H-2. gRPC `GetUserPerms` 未校验用户状态，冻结用户仍被下发全量权限
|
|
|
+### H-3. AdminLogin：`UsernameLoginLimit` 在 `ManagementKey` 校验之前计数，允许无凭据 DoS 超管账号
|
|
|
|
|
|
-- **位置**：`internal/server/permserver.go` 第 191-216 行
|
|
|
+- **位置**：`internal/logic/pub/adminLoginLogic.go:35-45`
|
|
|
- **描述**：
|
|
|
```go
|
|
|
- func (s *PermServer) GetUserPerms(ctx context.Context, req *pb.GetUserPermsReq) (*pb.GetUserPermsResp, error) {
|
|
|
- // 产品签名校验 ...
|
|
|
- ud := s.svcCtx.UserDetailsLoader.Load(ctx, req.UserId, req.ProductCode)
|
|
|
- if ud.Username == "" {
|
|
|
- return nil, status.Error(codes.NotFound, "用户不存在")
|
|
|
+ if l.svcCtx.UsernameLoginLimit != nil {
|
|
|
+ code, _ := l.svcCtx.UsernameLoginLimit.Take(req.Username)
|
|
|
+ if code == limit.OverQuota {
|
|
|
+ return nil, response.NewCodeError(429, "该账号登录尝试过于频繁，请5分钟后再试")
|
|
|
}
|
|
|
- return &pb.GetUserPermsResp{
|
|
|
- MemberType: ud.MemberType,
|
|
|
- Perms: ud.Perms,
|
|
|
- }, nil
|
|
|
+ }
|
|
|
+ if subtle.ConstantTimeCompare([]byte(req.ManagementKey), []byte(l.svcCtx.Config.Auth.ManagementKey)) != 1 {
|
|
|
+ return nil, response.ErrUnauthorized("managementKey无效")
|
|
|
}
|
|
|
```
|
|
|
|
|
|
- 对比同一文件的 `VerifyToken`（第 157-189 行），VerifyToken 在返回前校验了 `ud.Status == StatusEnabled` 和 `MemberType != ""`。但 `GetUserPerms` 完全没有这两层过滤，仅判断了用户是否存在。
|
|
|
+ `UsernameLoginLimit` 的 key 维度是**纯 username**（`svcCtx.UsernameLoginLimit = limit.NewPeriodLimit(300, 10, rds, ":rl:user")`，没有叠加 IP），5 分钟内全局 10 次。由于 `Take` 发生在 `ManagementKey` 校验之前，攻击者**不需要任何凭据**就能消耗配额。
|
|
|
|
|
|
- 结果：当管理后台调用 `UpdateUserStatus` 将用户冻结后：
|
|
|
- - `sys_user.status = 2 (Disabled)`，`tokenVersion` +1
|
|
|
- - `userDetailsLoader.Clean` 清理缓存
|
|
|
- - 下一次 Load 从 DB 读出 `ud.Status = 2`，但 `ud.Perms` 依然根据 `loadPerms` 逻辑完整计算
|
|
|
- - 产品服务器通过 gRPC `GetUserPerms` 查询，**仍然会拿到完整的权限列表**
|
|
|
+ 攻击场景：
|
|
|
+ 1. 攻击者对 `/auth/adminLogin` 以任意 `managementKey="x"` 但 `username=admin` 打 10 次（单 IP 一分钟内即可完成，AdminLoginRateLimit 是 IP 60s/20）。
|
|
|
+ 2. 真正的超管此后 5 分钟内无法登录管理后台，返回 `429`。
|
|
|
+ 3. 攻击者持续周期性地（每 5 分钟一波）重放，即可**永久锁死**已知超管用户名的管理后台入口。
|
|
|
|
|
|
- **影响**：
|
|
|
- - 被冻结的用户在接入方（产品服务）一侧仍然具备全部权限。虽然浏览器侧因 tokenVersion 变化 access token 已失效，但如果接入方自己缓存了 userId → perms 的映射并据此鉴权，用户可以继续获得访问。
|
|
|
- - 同样地，`sys_product_member.status = Disabled` 的用户，如果其部门类型为 `DEV`，`loadPerms` 依然会返回全量权限（详见 H-3）。
|
|
|
- - 形成"本系统侧已冻结，但对外仍判定有权限"的一致性漏洞。
|
|
|
+ - 任何已知超管用户名（如 `admin`、`admin_{code}`）可被**无凭据**持续 DoS 锁登录入口；应急响应、事件处置期间超管无法登录管理后台。
|
|
|
+ - 攻击成本极低，IP 级限流（20/min）足以完成锁定。
|
|
|
|
|
|
-- **修复方案**：对齐 `VerifyToken` 的校验逻辑：
|
|
|
+- **修复方案**：
|
|
|
+ - 把 `UsernameLoginLimit.Take` 移动到 `ManagementKey` 校验**之后**、用户名/密码校验之前：
|
|
|
|
|
|
- ```go
|
|
|
- ud := s.svcCtx.UserDetailsLoader.Load(ctx, req.UserId, req.ProductCode)
|
|
|
- if ud.Username == "" {
|
|
|
- return nil, status.Error(codes.NotFound, "用户不存在")
|
|
|
- }
|
|
|
- if ud.Status != consts.StatusEnabled {
|
|
|
- return nil, status.Error(codes.PermissionDenied, "用户已被冻结")
|
|
|
- }
|
|
|
- if !ud.IsSuperAdmin && ud.MemberType == "" {
|
|
|
- // 产品成员已被禁用或移除
|
|
|
- return nil, status.Error(codes.PermissionDenied, "用户已不是该产品的成员")
|
|
|
- }
|
|
|
- ```
|
|
|
+ ```go
|
|
|
+ if subtle.ConstantTimeCompare([]byte(req.ManagementKey), []byte(l.svcCtx.Config.Auth.ManagementKey)) != 1 {
|
|
|
+ return nil, response.ErrUnauthorized("managementKey无效")
|
|
|
+ }
|
|
|
+ if l.svcCtx.UsernameLoginLimit != nil {
|
|
|
+ code, _ := l.svcCtx.UsernameLoginLimit.Take(req.Username)
|
|
|
+ ...
|
|
|
+ }
|
|
|
+ ```
|
|
|
+
|
|
|
+ - 同时把限流 key 改为 `ip+username` 混合维度（推荐独立新桶），避免单个攻击者永久封锁真实用户。
|
|
|
+ - `ValidateProductLogin`（`loginService.go:33-38`）存在同样的账号锁定 DoS（无凭据即可锁任意用户名），推荐也改用 `ip+username` 混合 key 或增加可绕过的 CAPTCHA 机制。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### H-3. DEV 部门用户可绕过"产品成员禁用"继续获得全量权限
|
|
|
+### H-4. 移除产品成员 / 降级 MemberType 时未校验"最后一个 ADMIN"，可把产品彻底变成无人管理态
|
|
|
+
|
|
|
+- **位置**：
|
|
|
+ - `internal/logic/member/removeMemberLogic.go:29-53`
|
|
|
+ - `internal/logic/member/updateMemberLogic.go:30-64`
|
|
|
+- **描述**：
|
|
|
+ - `RemoveMember` 的访问控制只有 `CheckManageAccess(caller, member.UserId, member.ProductCode)`；`CheckManageAccess` 里 `if caller.UserId == targetUserId { return nil }` 允许自删除，`if caller.MemberType == ADMIN { return nil }`（`checkDeptHierarchy`）后续权限级别比较又用 `callerPri < targetPri` 放行——也就是 **任意一个 ADMIN 可以把另一个 ADMIN（或者自己）移出产品**；
|
|
|
+ - `UpdateMember` 可以把 ADMIN 降级为 `MEMBER`，同样没有"最后一个 ADMIN"检查。
|
|
|
+
|
|
|
+ 假设产品 P 最初由 `CreateProduct` 自动生成 `admin_P`（ADMIN）加入。这个 admin_P 之后通过 `AddMember` 邀请了 admin_Q（ADMIN）。两人随后：
|
|
|
+ - admin_Q 先 `UpdateMember(admin_P → MEMBER)` 或直接 `RemoveMember(admin_P)`；
|
|
|
+ - 之后 admin_Q 再自己 `RemoveMember(admin_Q)`。
|
|
|
|
|
|
-- **位置**：`internal/loaders/userDetailsLoader.go` 第 347-363 行（`loadPerms`）、`internal/middleware/jwtauthMiddleware.go` 第 79-86 行
|
|
|
-- **描述**：`loadPerms` 在判断"自动获得全量权限"时使用的是 **OR** 逻辑，其中部门类型判定独立于产品成员状态：
|
|
|
+ 产品 P 从此**没有任何 ADMIN**。前端路径无法再新增管理员（`AddMember` 需要 ADMIN 或 SUPER_ADMIN 操作；`CheckMemberTypeAssignment` 对新 ADMIN 又要求操作者是更高级别）。虽然超管可以通过 `AddMember` 介入，但该场景下产品运营已经 **必须依赖平台管理员介入**，违背了"产品自治"的设计意图。
|
|
|
+
|
|
|
+ 类似的，`admin_P` 可以不小心把自己降级为 `MEMBER`（UpdateMember 允许），产品立刻失去管理员。
|
|
|
+
|
|
|
+- **影响**：
|
|
|
+ - 真实业务中，产品管理员误操作（自己把自己移除或降级）会直接让该产品进入"需平台救援"状态。
|
|
|
+ - 有意的内部对抗中，一个 ADMIN 可以把其他 ADMIN 全部踢出，事实上独占管理权（已属于越权滥用，但本身合规上应有"至少保留一个 ADMIN"约束）。
|
|
|
+
|
|
|
+- **修复方案**：在 `RemoveMember` 与 `UpdateMember`（降级时）增加"最后 ADMIN"保护：
|
|
|
|
|
|
```go
|
|
|
- if ud.IsSuperAdmin ||
|
|
|
- ud.MemberType == consts.MemberTypeAdmin ||
|
|
|
- ud.MemberType == consts.MemberTypeDeveloper ||
|
|
|
- (ud.DeptType == consts.DeptTypeDev && ud.DeptStatus == consts.StatusEnabled) {
|
|
|
- codes, err := l.models.SysPermModel.FindAllCodesByProductCode(ctx, ud.ProductCode)
|
|
|
- ud.Perms = codes
|
|
|
- return
|
|
|
+ // 伪代码
|
|
|
+ if member.MemberType == consts.MemberTypeAdmin &&
|
|
|
+ (operation == Remove || (operation == Update && req.MemberType != ADMIN)) {
|
|
|
+ adminCount, _ := svcCtx.SysProductMemberModel.CountActiveAdmins(ctx, member.ProductCode)
|
|
|
+ if adminCount <= 1 {
|
|
|
+ return response.ErrBadRequest("不能移除/降级该产品的最后一个管理员")
|
|
|
+ }
|
|
|
}
|
|
|
```
|
|
|
|
|
|
- 而 `loadMembership` 针对被禁用的成员只做了一件事：跳过 `ud.MemberType` 赋值（即 `MemberType = ""`），**并不回退/阻断后续的部门判定**。
|
|
|
+ 需要在 model 层新增 `CountByProductCodeAndMemberType(productCode, MemberTypeAdmin, StatusEnabled)`。同时禁止管理员对自己的 MemberType 做降级（对比 `UpdateUserStatus` 已有的"不能修改自己的状态"）。
|
|
|
|
|
|
- 同时，`jwtauthMiddleware.Handle` 仅校验了 `ud.Username`、`ud.Status`、`claims.TokenVersion`，没有校验产品成员是否被禁用（不像 `RefreshToken` 会阻断 `ud.MemberType == ""` 的场景）。
|
|
|
+---
|
|
|
|
|
|
- 复现链路：
|
|
|
- 1. 用户 U 属于研发部门（`deptType=DEV`，`deptStatus=Enabled`），在产品 P 作为 `MEMBER` 登录获得 accessToken
|
|
|
- 2. 管理员通过 `UpdateMember` 将 U 在 P 的成员资格 `Status` 改为 `Disabled`
|
|
|
- 3. 缓存被 `Del`，下次请求 loader 从 DB 重算
|
|
|
- 4. `loadMembership` 因 `member.Status != Enabled` 直接 `return`，`MemberType = ""`
|
|
|
- 5. `loadPerms` 匹配第 4 个条件 `DeptType == DEV && DeptStatus == Enabled`，**依然返回全量权限**
|
|
|
- 6. 中间件不校验 MemberType，放行请求 → 用户携带冻结后的 membership 继续访问
|
|
|
+## ⚠️ 健壮性与安全建议 (Medium)
|
|
|
|
|
|
-- **影响**：产品管理员通过"禁用产品成员"无法真正撤销 DEV 部门用户对该产品的访问；必须同时修改用户部门，才能剥夺其权限。与接口语义不一致，是真实的水平越权旁路。
|
|
|
+### M-1. 无注销接口，refreshToken 被盗后无法主动吊销
|
|
|
|
|
|
-- **修复方案**：`loadPerms` 的 DEV 部门短路应**叠加一个"用户是该产品成员且成员状态启用"的前置条件**；或在 loader 中记录 `memberStatus` 字段，并在 DEV 部门判定里做交集：
|
|
|
+- **位置**：`internal/handler/routes.go` 路由清单、`internal/middleware/jwtauthMiddleware.go`
|
|
|
+- **描述**：系统只有登录 (`/auth/login` / `/auth/adminLogin`)、刷新 (`/auth/refreshToken`)、改密 (`/auth/changePassword`) 接口。**没有任何一个接口会主动 `tokenVersion+1`**（除 `UpdatePassword` / `UpdateStatus`），也没有 `/auth/logout` 路由。
|
|
|
|
|
|
- ```go
|
|
|
- // 方案：DEV 部门自动权限需要"产品成员存在且启用"作为前置
|
|
|
- deptFullPerms := ud.DeptType == consts.DeptTypeDev &&
|
|
|
- ud.DeptStatus == consts.StatusEnabled &&
|
|
|
- ud.MemberType != "" // 等价于"成员存在且启用"，因为禁用成员会把 MemberType 清空
|
|
|
-
|
|
|
- if ud.IsSuperAdmin ||
|
|
|
- ud.MemberType == consts.MemberTypeAdmin ||
|
|
|
- ud.MemberType == consts.MemberTypeDeveloper ||
|
|
|
- deptFullPerms {
|
|
|
- ...
|
|
|
- }
|
|
|
- ```
|
|
|
+ 后果：
|
|
|
+ - 用户即使在客户端"退出登录"，也只是本地清了 token；已签发的 access + refresh 在整个 expire 窗口内仍然可被重放。
|
|
|
+ - 用户怀疑 token 被盗时，唯一的自救手段是**修改密码**（`ChangePassword` 会 `tokenVersion+1` 并 `Clean` 缓存）。对使用 SSO、或不自己设密码（例如 OAuth 接入）的场景不可用。
|
|
|
|
|
|
- 同时建议在 `jwtauthMiddleware` 或在业务层对 `ud.MemberType == "" && !IsSuperAdmin && productCode != ""` 场景也返回 401/403（与 `refreshTokenLogic` 的第 53 行保持一致）。
|
|
|
+- **建议**：
|
|
|
+ - 新增 `/auth/logout` 接口：鉴权后将该用户的 `tokenVersion+1`（或者维护一个"签出黑名单"集合，带 TTL 到 `RefreshExpire`）。
|
|
|
+ - 相应 gRPC 端也暴露 `RevokeTokens(userId, productCode)`。
|
|
|
|
|
|
---
|
|
|
|
|
|
-## ⚠️ 健壮性与性能建议 (Medium)
|
|
|
+### M-2. refreshToken 轮转未消耗原 token，存在"双 token 并存"窗口
|
|
|
+
|
|
|
+- **位置**：`internal/logic/pub/refreshTokenLogic.go:70-77`、`internal/server/permserver.go:141-148`
|
|
|
+- **描述**：`GenerateRefreshTokenWithExpiry` 的行为是"基于 claims 里原 token 的 `ExpiresAt` 重新签一张新 token"——此时 `tokenVersion` 并未递增，新旧两张 refreshToken 都用同一个 `tokenVersion` 命中 `tokenVersion == ud.TokenVersion` 的校验。
|
|
|
|
|
|
-### M-1. 自动生成的产品管理员密码熵过低（仅 32 bits）
|
|
|
+ 如果攻击者偷到 refreshToken 的同时、真实用户也在使用：
|
|
|
+ 1. 真实用户用原 rt 换 rt1、rt2、rt3……
|
|
|
+ 2. 攻击者用原 rt 换 rt'；
|
|
|
+ 3. 两边的 `tokenVersion` 相同，两边都能继续无限续期到 `refreshExpire`。
|
|
|
|
|
|
-- **位置**：`internal/logic/product/createProductLogic.go` 第 80、157-163 行
|
|
|
-- **描述**：`adminPassword` 通过 `generateRandomHex(8)` 生成，而该函数的实现是先 `rand.Read(b)` 读取 8 字节（16 hex 字符），再 `[:length]` 截断取前 8 个字符。因此 adminPassword 实际有效熵只有 **4 字节 = 32 bits**（约 42 亿种可能）。同理 `generateRandomHex(32)` 的 appKey 是 16 字节 = 128 bits（OK），`generateRandomHex(64)` 的 appSecret 是 32 字节 = 256 bits（OK）。
|
|
|
+ 标准的 refresh token rotation 语义应当是"已使用的 refresh token 立即一次性失效"（通过 jti 黑名单、或者每次刷新都让 `tokenVersion` 递增）。当前实现不具备这个能力。
|
|
|
|
|
|
- 虽然 `mustChangePassword=Yes` 会强制首次登录改密，但在管理员拿到 `CreateProductResp` 到其首次登录的时间窗口内，该密码仍可被暴力破解（尤其管理后台目前已经按用户名限流 5min/10 次，基本可挡住穷举，但设计上不应依赖外部限流来保护一个 32 bit 的秘密）。
|
|
|
+- **影响**：refreshToken 泄露后几乎无法挽回，直到 refreshExpire 自然过期，或用户主动改密。
|
|
|
|
|
|
- **建议**：
|
|
|
+ - 方案 1（最小侵入）：在 refresh 时让 `sys_user.tokenVersion` 递增，使老 refresh token 立即失效。缺点是多端登录无法共存。
|
|
|
+ - 方案 2：在 Redis 里维护 `refreshJti → userId` 的一次性 map，`RefreshToken` 时 `GETDEL`，不存在即失败。`jti` 存入 `RegisteredClaims`。
|
|
|
+ - 方案 3：缩短 refreshExpire，降低风险窗口。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### M-3. 产品自动管理员密码实际熵仅 32 bits
|
|
|
+
|
|
|
+- **位置**：`internal/logic/product/createProductLogic.go:80`、`157-163`
|
|
|
+- **描述**：
|
|
|
```go
|
|
|
func generateRandomHex(length int) (string, error) {
|
|
|
- byteLen := (length + 1) / 2
|
|
|
- b := make([]byte, byteLen)
|
|
|
+ b := make([]byte, length)
|
|
|
if _, err := rand.Read(b); err != nil {
|
|
|
- return "", err
|
|
|
+ return "", fmt.Errorf(...)
|
|
|
}
|
|
|
return hex.EncodeToString(b)[:length], nil
|
|
|
}
|
|
|
```
|
|
|
- 这样 `generateRandomHex(8)` 就真正提供 8 个 hex 字符 = 4 字节实熵，再考虑到一次性临时密码，可以直接把 adminPassword 生成改为 `generateRandomHex(16)`（16 hex 字符 ≈ 64 bits）。
|
|
|
+ `rand.Read` 填充 `length` 字节，`hex.EncodeToString` 产生 `2*length` 个 hex 字符，随后**截断取前 `length` 个字符**——也就是实际只保留了前 `length/2` 字节的随机性，相当于 `4*length` bits。
|
|
|
|
|
|
----
|
|
|
+ - `generateRandomHex(32)` appKey：实际 128 bits，OK。
|
|
|
+ - `generateRandomHex(64)` appSecret：实际 256 bits，OK。
|
|
|
+ - `generateRandomHex(8)` adminPassword：**只有 32 bits ≈ 4e9 种可能**。虽然 `MustChangePassword=Yes` 会强制首登改密，但这个一次性密码在超管拿到之后到管理员首次登录之前的窗口内暴力破解可达。依赖外层 `UsernameLoginLimit`（5min/10 次）间接保护并不健壮。
|
|
|
|
|
|
-### M-2. BindRoles 与 BindRolePerms 的 DELETE 走循环逐条执行
|
|
|
+- **建议**：修正截断边界或直接提高长度：
|
|
|
|
|
|
-- **位置**：
|
|
|
- - `internal/logic/user/bindRolesLogic.go` 第 117-122 行
|
|
|
- - `internal/logic/role/bindRolePermsLogic.go` 第 105-110 行
|
|
|
-- **描述**：
|
|
|
```go
|
|
|
- for _, roleId := range toRemove {
|
|
|
- query := fmt.Sprintf("DELETE FROM %s WHERE `userId` = ? AND `roleId` = ?", l.svcCtx.SysUserRoleModel.TableName())
|
|
|
- if _, err := session.ExecCtx(ctx, query, req.UserId, roleId); err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- }
|
|
|
- ```
|
|
|
- 当 `toRemove` 有 N 项时，会在同一事务内执行 N 次独立 DELETE，每次都要经过 session 往返。虽然单用户的角色数一般不多（< 20），但这个实现在事务持锁窗口上是 N 倍于批量 DELETE 的。
|
|
|
-- **建议**：合并为一次 `DELETE ... WHERE userId=? AND roleId IN (?,?,?)`：
|
|
|
- ```go
|
|
|
- if len(toRemove) > 0 {
|
|
|
- placeholders := strings.Repeat("?,", len(toRemove))
|
|
|
- placeholders = placeholders[:len(placeholders)-1]
|
|
|
- args := make([]interface{}, 0, len(toRemove)+1)
|
|
|
- args = append(args, req.UserId)
|
|
|
- for _, id := range toRemove {
|
|
|
- args = append(args, id)
|
|
|
- }
|
|
|
- query := fmt.Sprintf("DELETE FROM %s WHERE `userId`=? AND `roleId` IN (%s)",
|
|
|
- l.svcCtx.SysUserRoleModel.TableName(), placeholders)
|
|
|
- if _, err := session.ExecCtx(ctx, query, args...); err != nil {
|
|
|
- return err
|
|
|
+ func generateRandomHex(length int) (string, error) {
|
|
|
+ byteLen := (length + 1) / 2
|
|
|
+ b := make([]byte, byteLen)
|
|
|
+ if _, err := rand.Read(b); err != nil {
|
|
|
+ return "", err
|
|
|
}
|
|
|
+ return hex.EncodeToString(b)[:length], nil
|
|
|
}
|
|
|
```
|
|
|
|
|
|
+ 同时 `adminPassword` 改为 `generateRandomHex(16)`（64 bits）起步。
|
|
|
+
|
|
|
---
|
|
|
|
|
|
-### M-3. UserDetailLogic 在"超管 + 产品上下文"场景下返回跨产品的 roleIds
|
|
|
+### M-4. `DeptTree` 对所有登录用户开放，暴露完整组织架构
|
|
|
|
|
|
-- **位置**：`internal/logic/user/userDetailLogic.go` 第 50-56 行
|
|
|
-- **描述**：
|
|
|
- ```go
|
|
|
- productCode := middleware.GetProductCode(l.ctx)
|
|
|
- var roleIds []int64
|
|
|
- if productCode != "" && !caller.IsSuperAdmin {
|
|
|
- roleIds, _ = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserIdForProduct(...)
|
|
|
- } else {
|
|
|
- roleIds, _ = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserId(...)
|
|
|
- }
|
|
|
- ```
|
|
|
- 条件只看"非超管 + 有 productCode"。当超管自己带着某产品的 productCode 查看某用户时，会走 `else` 分支，返回用户**在所有产品下**的角色 ID 列表。前端如果基于这个列表渲染"当前产品的已绑定角色"，展示会错乱（例如超管在产品 A 下看用户 U 的详情，却看到 U 在产品 B 的角色）。
|
|
|
-- **建议**：超管的判定与产品过滤解耦——**只要有 productCode，就按产品过滤**：
|
|
|
- ```go
|
|
|
- if productCode != "" {
|
|
|
- roleIds, err = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserIdForProduct(l.ctx, user.Id, productCode)
|
|
|
- } else {
|
|
|
- roleIds, err = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserId(l.ctx, user.Id)
|
|
|
- }
|
|
|
- ```
|
|
|
+- **位置**：`internal/logic/dept/deptTreeLogic.go`、`internal/handler/routes.go:42-69`
|
|
|
+- **描述**：`/api/dept/tree` 只挂了 `JwtAuth`。`DeptTreeLogic.DeptTree` 自身完全不做权限过滤，**任意 JWT 通过的用户（包括任意产品的普通 MEMBER）**都能拉到全公司组织架构（包括 `deptType=DEV`、部门名称、层级结构、备注）。
|
|
|
+- **影响**：组织架构常包含内部命名、隐含岗位属性（DEV 部门自动获得全权限），属于内部敏感信息；不应对产品端的普通成员暴露。
|
|
|
+- **建议**：
|
|
|
+ - 严格版：仅超管可拉全量；其余用户只能拿"自身部门 + 其下级子部门"（`strings.HasPrefix(d.Path, caller.DeptPath)`）。
|
|
|
+ - 宽松版：超管 + ADMIN / DEVELOPER 可见；MEMBER 不可调用该接口。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-4. `FindRoleIdsByUserIdForProduct` 未过滤角色状态，返回含已禁用角色
|
|
|
+### M-5. `ProductList` / `ProductDetail` 对所有登录用户返回产品元数据
|
|
|
|
|
|
-- **位置**：`internal/model/userrole/sysUserRoleModel.go` 第 43-50 行
|
|
|
-- **描述**：
|
|
|
- ```sql
|
|
|
- SELECT ur.roleId FROM sys_user_role ur
|
|
|
- INNER JOIN sys_role r ON ur.roleId = r.id
|
|
|
- WHERE ur.userId = ? AND r.productCode = ?
|
|
|
- ```
|
|
|
- 只按 `productCode` 过滤，没有 `AND r.status = 1`。而 `loadRoles` / `FindMinPermsLevelByUserIdAndProductCode` 都需要基于"启用角色"做判定。
|
|
|
+- **位置**：`internal/logic/product/productListLogic.go`、`internal/logic/product/productDetailLogic.go`
|
|
|
+- **描述**：任何 JWT 通过的用户都能遍历系统全部产品（`code`、`name`、`status`、`remark`、`createTime`）。虽然 `AppKey` 只对超管返回，但产品清单本身对所有成员可见。跨产品用户可以探测出系统内其他产品的存在（例如"内部管理后台"、"支付中心"）。
|
|
|
+- **建议**：
|
|
|
+ - 仅超管可列全部；非超管只能看到自己作为成员的产品（join `sys_product_member` 过滤）。
|
|
|
+ - `ProductDetail` 增加同样的归属校验：非超管只能看自己所在产品。
|
|
|
|
|
|
- 后续在 `userDetailsLoader.loadRoles`（第 314-344 行）通过内存过滤 `r.Status == StatusEnabled` 做了二次兜底，因此用于**权限计算**的路径是正确的。但：
|
|
|
- 1. `UserDetail` 接口直接把这批 roleIds 返回给前端（含禁用角色）。
|
|
|
- 2. `bindRolesLogic` 的 "existingRoleIds diff 逻辑"（第 85-110 行）会把已禁用的旧关联当作"存在"，只有当请求里明确包含了该禁用角色时才会保留，否则会被 `toRemove` 删除 —— 表现为"重新绑定时，用户原本禁用的旧角色会被清掉"。这个行为从业务语义上是可接受的（禁用角色本就不应再绑定），但与 SQL 字面不一致，容易误解。
|
|
|
+---
|
|
|
|
|
|
-- **建议**：要么在 SQL 里显式 `AND r.status = 1`，要么在命名上明确包含禁用（如 `FindAllRoleIdsByUserIdForProduct`）。推荐前者：
|
|
|
+### M-6. 产品端登录的用户名锁定 DoS（账号级）
|
|
|
|
|
|
- ```sql
|
|
|
- WHERE ur.userId = ? AND r.productCode = ? AND r.status = 1
|
|
|
- ```
|
|
|
+- **位置**：`internal/logic/pub/loginService.go:33-38`
|
|
|
+- **描述**：`UsernameLoginLimit.Take(username)` 在无任何前置鉴权的情况下被消费，5min/10 次、纯 username 维度。任何外部攻击者只需知道目标用户名，即可以 10 次失败请求锁定该账号 5 分钟；通过定时重放可达到长时间账号级 DoS。
|
|
|
+- **影响**：
|
|
|
+ - 登录入口对外开放、任意 IP 可触达；
|
|
|
+ - 配合 H-3（管理后台也有同样的问题），造成系统级账号锁定攻击面。
|
|
|
+- **建议**：与 H-3 同步处理，把 rate limit key 改为 `ip:username`（或对同一 IP 的失败次数独立设桶）；对成功登录重置该 username 的计数。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-5. UpdateDept 对"deptType 字段无变化"时也会级联清空子部门用户缓存
|
|
|
+### M-7. `X-Real-IP` 信任策略过简，未支持 `X-Forwarded-For`
|
|
|
|
|
|
-- **位置**：`internal/logic/dept/updateDeptLogic.go` 第 77-89 行
|
|
|
-- **描述**：条件是 `if req.DeptType == DeptTypeNormal || req.DeptType == DeptTypeDev`，只要请求带了合法的 deptType，就会执行 `FindByPathPrefix` 然后挨个 `Clean` 子部门用户缓存；但并没有比较 `req.DeptType` 与当前 `dept.DeptType` 是否真的不同。
|
|
|
+- **位置**：`internal/middleware/ratelimitMiddleware.go:41-52`
|
|
|
+- **描述**：
|
|
|
+ 1. 仅识别 `X-Real-IP`，没有兼容多数 ingress / ELB 默认设置的 `X-Forwarded-For`。
|
|
|
+ 2. 只要 `behindProxy=true`，任何 `X-Real-IP` 无条件被信任；如果反向代理没有覆盖客户端原有 header，攻击者可以通过伪造 header 分散限流桶。
|
|
|
+- **建议**：
|
|
|
+ - 支持 XFF：按最右可信节点取值。
|
|
|
+ - 配置可信代理 CIDR 列表，仅当 `RemoteAddr` 在可信网段内时才信任 header。
|
|
|
+ - 保留目前行为作为降级，优先级：`XFF`（可信时）> `X-Real-IP`（可信时）> `RemoteAddr`。
|
|
|
|
|
|
- 另一层问题：如源代码注释所说，`loadPerms` 只看用户**自身**部门的 deptType/status，子部门用户并不受父部门 deptType 变化影响。所以从权限计算的正确性来看，**根本不需要**级联清理子部门用户缓存；只清理当前部门直属用户已经足够。
|
|
|
+---
|
|
|
|
|
|
-- **建议**：去掉级联清理，或收窄到"deptType 真的发生变化时仅清理自身部门用户"。代码可简化为：
|
|
|
+### M-8. 缓存失效与 DB 事务非原子：`Clean/Del/BatchDel` 失败时静默吞错
|
|
|
+
|
|
|
+- **位置**：
|
|
|
+ - 写路径：`UpdateUser`、`UpdateUserStatus`、`ChangePassword`、`BindRoles`、`SetUserPerms`、`UpdateMember`、`RemoveMember`、`DeleteRole`、`BindRolePerms`、`UpdateRole`、`UpdateProduct`、`ExecuteSyncPerms`、`UpdateDept`
|
|
|
+ - Loader：`internal/loaders/userDetailsLoader.go:138-173`
|
|
|
+- **描述**：所有写操作都是"DB tx 提交 → Redis 失效"两步，但 `Del / Clean / BatchDel` 内部的 Redis 错误只打日志。Redis 瞬时抖动期间，DB 已提交但缓存未失效，在 `defaultCacheTTL=300s` 之内其他请求命中旧缓存，包括 `tokenVersion / MemberType / Perms` 等安全关键字段。
|
|
|
+ - 对修改密码、冻结账号、删除角色这类"收紧"操作，最大 5 分钟的旧视图继续有效，是不可忽视的安全风险窗口。
|
|
|
+ - 对"放宽"操作（添加角色、启用成员），旧视图是"没权限"，用户体验问题，相对安全。
|
|
|
+- **建议**：
|
|
|
+ - 对安全关键动作（`UpdateUserStatus` 冻结、`ChangePassword`、`RemoveMember`、`UpdateMember Status=Disabled`），`Clean` 失败必须返回 5xx，或把这类 key 的 TTL 收紧到 60s。
|
|
|
+ - 引入"延迟双删"或消息队列补偿：DB 提交后发送缓存失效事件，由消费端重试到成功。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### M-9. `BindRolePerms` / `UpdateRole` / `DeleteRole` 的"受影响用户查询"发生在事务之外，存在漏清缓存的窗口
|
|
|
+
|
|
|
+- **位置**：
|
|
|
+ - `internal/logic/role/bindRolePermsLogic.go:126-127`
|
|
|
+ - `internal/logic/role/deleteRoleLogic.go:39-53`
|
|
|
+ - `internal/logic/role/updateRoleLogic.go:66-67`
|
|
|
+- **描述**：三个接口的模式都是"事务外 `FindUserIdsByRoleId` → 事务内写 DB → 事务外 `BatchDel`"。在**事务开始后、事务提交前**，若有另一个 goroutine 通过 `BindRoles` 把新用户加进这个角色（`sys_user_role` 插入并已提交），当前 goroutine 计算 `affectedUserIds` 时没有包含这些新用户。
|
|
|
+ - 事务提交之后，新加入的用户缓存不会被当前流程清理。他们会用"角色权限变更前的 perms 快照"继续工作 5 分钟。
|
|
|
+ - 另一条 `BindRoles` 流程会对它自己绑的用户 `Clean`，但不会感知到本流程对角色权限的改动。
|
|
|
+
|
|
|
+ 虽然是低概率双写，但对"删除角色"这种一次性收紧操作，未清掉的那一批用户仍会基于"已删除角色下的权限集"工作（实际上，一旦事务提交，`sys_role_perm` 清空，`loadPerms` 的 role path 自然会走 `FindPermIdsByRoleIds` 得空——但缓存是上次的；只有下次 miss 才会触发）。本质是**缓存永远滞后于 5 分钟**。
|
|
|
+- **建议**：把 `FindUserIdsByRoleId` 放进事务内，并使用 `SELECT ... FOR UPDATE` 锁住这些用户的绑定关系，避免并发新增；或在事务提交后再 `FindUserIdsByRoleId` 一次（更简单）——这样保证看到的是最新的用户集：
|
|
|
|
|
|
```go
|
|
|
- userIds, _ := l.svcCtx.SysUserModel.FindIdsByDeptId(l.ctx, req.Id)
|
|
|
- for _, uid := range userIds {
|
|
|
- l.svcCtx.UserDetailsLoader.Clean(l.ctx, uid)
|
|
|
- }
|
|
|
+ // 事务提交成功之后
|
|
|
+ affectedUserIds, _ := l.svcCtx.SysUserRoleModel.FindUserIdsByRoleId(l.ctx, req.RoleId)
|
|
|
+ l.svcCtx.UserDetailsLoader.BatchDel(l.ctx, affectedUserIds, role.ProductCode)
|
|
|
```
|
|
|
|
|
|
- 子部门级联逻辑可以删除。
|
|
|
+ 当前是在事务之前拿的，移到事务之后即可显著减少竞态。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-6. 僵尸字段：JWT `Claims` 中的 `Perms` 从未被产线代码写入
|
|
|
+### M-10. `FindByPathPrefix` LIKE 转义依赖 MySQL 默认 `\` 转义
|
|
|
|
|
|
-- **位置**：`internal/middleware/jwtauthMiddleware.go` 第 23-32 行
|
|
|
-- **描述**：`middleware.Claims` 结构体保留了 `Perms []string` 字段：
|
|
|
+- **位置**：`internal/model/dept/sysDeptModel.go:56-64`
|
|
|
+- **描述**：`strings.NewReplacer("%", "\\%", "_", "\\_").Replace(pathPrefix)` 产出的是 `/xxx/\%yyy/`，在 SQL `WHERE path LIKE ?` 下默认依赖 MySQL 的 `\` 作为 LIKE 转义符。当 `sql_mode` 含 `NO_BACKSLASH_ESCAPES` 时，`\%` 会被当作两个字符，匹配失败或命中预期外数据。
|
|
|
+
|
|
|
+ 此函数在当前产线代码中**已无调用方**（见 M-11 僵尸代码），但为了避免将来复用踩坑，建议显式 `LIKE ? ESCAPE '!'` 并在应用层用 `!` 作为转义符。
|
|
|
+
|
|
|
+- **建议**：
|
|
|
```go
|
|
|
- type Claims struct {
|
|
|
- ...
|
|
|
- Perms []string `json:"perms,omitempty"`
|
|
|
- jwt.RegisteredClaims
|
|
|
- }
|
|
|
+ escaped := strings.NewReplacer("!", "!!", "%", "!%", "_", "!_").Replace(pathPrefix)
|
|
|
+ query := fmt.Sprintf("SELECT ... WHERE `path` LIKE ? ESCAPE '!' ORDER BY ...", ...)
|
|
|
```
|
|
|
- 但 `GenerateAccessToken`（`internal/logic/auth/jwt.go` 第 23-39 行）**没有给 `Perms` 赋值**，产线签出的 JWT 永远不带该字段。仅测试 (`jwt_test.go`)、README 出现过引用。属于设计未落地或已重构遗留的字段。
|
|
|
-- **建议**：确认是否还需要（例如未来把 perms 塞进 token 做无状态鉴权）；不再需要则移除 `Perms` 字段，避免维护歧义。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-7. `X-Real-IP` 信任策略过于简单，且不支持标准 `X-Forwarded-For`
|
|
|
+### M-11. 僵尸代码：`FindByPathPrefix` / `FindByParentId` / `FindRoleIdsByUserId` 仅测试引用
|
|
|
+
|
|
|
+- **位置**：
|
|
|
+ - `internal/model/dept/sysDeptModel.go:47-64`（`FindByParentId`、`FindByPathPrefix`）
|
|
|
+ - `internal/model/userrole/sysUserRoleModel.go:37-44`（`FindRoleIdsByUserId`，`UserDetailLogic.userDetailLogic.go:55` 理论上会调用，但看下面）
|
|
|
+- **描述**：经全仓搜索：
|
|
|
+ - `FindByPathPrefix` 和 `FindByParentId`：自上一轮 `DeleteDept` 改为行锁 + 子查询后，两者在生产代码中没有任何调用方，仅测试/mock 保留。
|
|
|
+ - `FindRoleIdsByUserId`（全产品汇总）：在 `userDetailLogic.go:55` 的 `else` 分支调用（"没有 productCode 上下文时"）。该分支仅在超管登录管理后台且未带产品上下文时进入；但前端在"用户详情"页面一般会带产品上下文。调用路径存在但极少。
|
|
|
+
|
|
|
+ 建议清理 `FindByPathPrefix` / `FindByParentId`（或保留但加注释，避免重复发明轮子）。`FindRoleIdsByUserId` 仍有路径保留。
|
|
|
+
|
|
|
+- **建议**：
|
|
|
+ - 删除 `FindByPathPrefix` / `FindByParentId`，或至少加 `// Deprecated` 注释。
|
|
|
+ - `FindRoleIdsByUserId` 保留。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### M-12. `UpdateUserStatus` 的 productCode 归属校验与超管路径重复
|
|
|
|
|
|
-- **位置**：`internal/middleware/ratelimitMiddleware.go` 第 41-52 行
|
|
|
+- **位置**：`internal/logic/user/updateUserStatusLogic.go:49-60`
|
|
|
- **描述**：
|
|
|
```go
|
|
|
- if behindProxy {
|
|
|
- if ip := r.Header.Get("X-Real-IP"); ip != "" {
|
|
|
- return ip
|
|
|
+ if productCode != "" {
|
|
|
+ caller := middleware.GetUserDetails(l.ctx)
|
|
|
+ if caller != nil && !caller.IsSuperAdmin {
|
|
|
+ if _, err := svcCtx.SysProductMemberModel.FindOneByProductCodeUserId(...); err != nil {
|
|
|
+ return response.ErrBadRequest("目标用户不是当前产品的成员")
|
|
|
+ }
|
|
|
}
|
|
|
}
|
|
|
+ if err := authHelper.CheckManageAccess(l.ctx, l.svcCtx, req.Id, productCode); err != nil {
|
|
|
+ return err
|
|
|
+ }
|
|
|
```
|
|
|
- 问题：
|
|
|
- 1. 仅支持 `X-Real-IP`，未兼容更通用的 `X-Forwarded-For`（多数 K8s Ingress、ELB 默认设置的是 XFF）。
|
|
|
- 2. 只要 `behindProxy=true`，任何 `X-Real-IP` 都无条件信任；如果反向代理没有正确覆盖客户端传入的头，攻击者可以伪造 IP 规避限流。
|
|
|
- 3. XFF 是多段逗号分隔，需要按最右可信节点反向取值；本实现一旦支持 XFF 必须小心这一点。
|
|
|
-- **建议**：
|
|
|
- - 同时兼容 `X-Forwarded-For`，取其中未被你控制的最右侧一段作为客户端 IP。
|
|
|
- - 把"可信代理 CIDR 列表"配置化：只有来自可信代理网段的请求才信任 header，其他直接使用 `RemoteAddr`。
|
|
|
- - 如果暂时不打算加复杂度，至少把 XFF 支持补上，且确保部署文档强调反向代理必须覆盖这些 header。
|
|
|
+
|
|
|
+ `CheckManageAccess` 内部已经对"非超管且同一产品下非成员"做了 `checkPermLevel` 的目标成员检查（会返回 "目标用户不是当前产品的成员，无法执行管理操作"）。外层这段手动检查在非超管场景下和内部检查**重复**，且多了一次 `FindOneByProductCodeUserId` DB 查询。
|
|
|
+
|
|
|
+ - 功能上没问题，但多一次 DB 查询且逻辑位置分散。
|
|
|
+
|
|
|
+- **建议**：删掉外层的重复校验，全部交给 `CheckManageAccess`。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-8. DeptTree 对所有登录用户开放，暴露完整组织架构
|
|
|
+### M-13. `UpdateUser` 自修改时能"显式传 `Status=0` + 非 nil `DeptId`"通过第一层但被 `FindOne` 后再补判
|
|
|
+
|
|
|
+- **位置**：`internal/logic/user/updateUserLogic.go:39-59`
|
|
|
+- **描述**：逻辑结构：
|
|
|
+ ```go
|
|
|
+ if caller.UserId == req.Id {
|
|
|
+ if req.DeptId != nil || req.Status != 0 {
|
|
|
+ return 403
|
|
|
+ }
|
|
|
+ } else {
|
|
|
+ if err := CheckManageAccess(...); err != nil { return err }
|
|
|
+ }
|
|
|
+ user, _ := FindOne(req.Id) // 再查一次
|
|
|
+ if caller.UserId != req.Id && user.IsSuperAdmin == Yes {
|
|
|
+ if req.Status != 0 || req.DeptId != nil { return 403 }
|
|
|
+ }
|
|
|
+ ```
|
|
|
+
|
|
|
+ - 自修改场景下：如果传 `DeptId=&0`（指针非 nil，值为 0），第一层 `req.DeptId != nil` 直接拦下，正确。
|
|
|
+ - 他人修改超管场景下：第二道防线阻断 `Status != 0 || DeptId != nil`。OK。
|
|
|
|
|
|
-- **位置**：`internal/logic/dept/deptTreeLogic.go`、`internal/handler/routes.go` 第 42-69 行
|
|
|
-- **描述**：路由仅通过 `JwtAuth` 中间件保护，`DeptTreeLogic` 自身完全不做权限过滤，**任何通过 JWT 的用户（包括产品端 MEMBER）**都能拉到全公司组织架构。
|
|
|
-- **影响**：组织架构通常包含内部部门命名、层级关系、部门类型（NORMAL/DEV 可暗示岗位属性），属于内部敏感信息，不应暴露给产品端普通成员。
|
|
|
+ 但是一个隐含风险：**他人修改普通用户**场景下，如果 caller 通过 `UpdateUser` 改对方 `DeptId`，没有校验 caller 对"新目标部门"的权限。例如普通 ADMIN（部门 A）可以把用户 U 从部门 X 挪到部门 Y，哪怕 Y 不在 ADMIN 的管辖范围。`CheckManageAccess` 只校验 caller 能否管理 U（在 caller 自己的部门子树内），**不校验新 DeptId 是否合法**。
|
|
|
|
|
|
-- **建议**：根据业务定位决定：
|
|
|
- - 严格方案：仅超管可拉全量树；普通成员只能拿自身部门及下级子部门（`strings.HasPrefix(d.Path, caller.DeptPath)`）。
|
|
|
- - 宽松方案：超管和任意 ADMIN/DEVELOPER 可见；MEMBER 只见自身部门节点。
|
|
|
+ 对 ADMIN 的判定：ADMIN 在 `checkDeptHierarchy` 里直接放行（第 101-103 行），所以 ADMIN 可以跨部门挪用户，这是设计意图。但 `MemberType=DEVELOPER` 或无角色的 MEMBER 不会走到这里（会被更早拦下）。
|
|
|
+
|
|
|
+ 结论：目前行为是 ADMIN 可跨部门分配，正常；MEMBER/DEVELOPER 不会触发这个路径。但如果未来放宽 `checkDeptHierarchy`，需要补这层目标部门校验。
|
|
|
+
|
|
|
+- **建议**（低优）：把"变更 DeptId 时，校验目标部门在 caller 的可管理范围或 caller 是超管/ADMIN"独立出来，避免后续逻辑回归。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-9. ProductList 对所有登录用户返回全量产品列表
|
|
|
+### M-14. `setUserPermsLogic` 对已被禁用的权限直接拒绝，但缺少对"产品已被禁用"的校验
|
|
|
|
|
|
-- **位置**：`internal/logic/product/productListLogic.go`
|
|
|
-- **描述**：任何 JWT 通过的用户都能列出系统全部产品（产品编码、名称、状态、创建时间），非超管会隐藏 `AppKey`，但产品列表本身仍然对全员可见。其他产品的普通成员也能"看见"存在哪些其他产品。
|
|
|
-- **影响**：产品信息属于租户元数据，跨产品可见会让攻击者进行产品横向探测（例如发现有"内部管理后台"、"支付中心"等高价值目标并针对性登录）。
|
|
|
-- **建议**：
|
|
|
- - 仅超管可列全部；其他成员只能看到自己是成员的产品（通过 `FindListByUserId` 过滤）。
|
|
|
+- **位置**：`internal/logic/user/setUserPermsLogic.go`
|
|
|
+- **描述**：只校验 `perm.ProductCode == productCode && perm.Status == Enabled`，没有校验该产品本身是否被禁用。结合 H-1，管理员在产品被禁用后依然能对该产品的成员设置权限。
|
|
|
+- **影响**：与 H-1 同源，一旦 H-1 修复（loadPerms 感知产品状态），这里的写入仍然合法；建议一并在管理面增加 `product.Status == Enabled` 的前置校验，作为防御纵深。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-10. 缓存失效与 DB 事务不原子：Clean 失败静默吞错
|
|
|
+### M-15. `BindRoles` 对"重复绑定同一角色"请求，toAdd/toRemove 都为空时直接 `return nil`，不同步缓存
|
|
|
|
|
|
-- **位置**：`UpdateUser` (`updateUserLogic.go:132`)、`UpdateUserStatus` (`updateUserStatusLogic.go:66`)、`ChangePassword` (`changePasswordLogic.go:59`)、`BindRoles` (`bindRolesLogic.go:141`)、`SetUserPerms` (`setUserPermsLogic.go:115`)、`UpdateMember` (`updateMemberLogic.go:62`)、`RemoveMember` (`removeMemberLogic.go:51`)、`DeleteRole` (`deleteRoleLogic.go:53`)、`SyncPerms` (`syncPermsService.go:115`)
|
|
|
-- **描述**：所有写操作都按"先 DB 事务提交，再调用 `UserDetailsLoader.Clean/Del/BatchDel`"的顺序执行；而 `Clean/Del/BatchDel` 内部的 Redis 错误只写日志，不返回。这意味着：
|
|
|
- - Redis 瞬时不可用或网络抖动时，DB 已经提交但缓存未失效。
|
|
|
- - 在 `defaultCacheTTL = 300s`（5 分钟）之内，其他请求命中旧缓存，包括 `tokenVersion` / `MemberType` / `Perms` 等关键字段。
|
|
|
- - 对于密码修改、冻结账号这类安全动作，最大 5 分钟的旧 token 继续可用就是一段不可忽视的风险窗口。
|
|
|
-- **建议**：
|
|
|
- - 对"安全关键操作"（密码修改、冻结、tokenVersion 变化）收紧：在 `Clean` 失败时返回 5xx，或降低这类 key 的 TTL（如 30s）。
|
|
|
- - 引入"延迟双删"或消息队列补偿：DB 提交后投递一条缓存失效消息，由消费端重试直至成功。
|
|
|
- - 至少在 loader 层增加"失败时重试一次或返回错误"的能力，让上层可以决定是否对外暴露失败。
|
|
|
+- **位置**：`internal/logic/user/bindRolesLogic.go:114-116`
|
|
|
+- **描述**：当请求的 `req.RoleIds` 与数据库现状完全相等时，直接 return。这是正确的优化，但它意味着：
|
|
|
+ - 如果由于缓存异常，`UserDetails.Roles` 在 Redis 中是"失效但未清"的错值（例如上一次写入失败），调用 `BindRoles` 做一次"无改动 upsert"**不会**触发缓存清理。
|
|
|
+- **影响**：极低，仅在"上次 Clean 失败 + 当前调用无 diff"的联合场景下出现，属于灰度 / 降级运维场景。
|
|
|
+- **建议**：保留优化，但在 `return nil` 之前仍然做一次 `UserDetailsLoader.Clean(l.ctx, req.UserId)`，确保本次调用语义是"写读一致"。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-11. DeleteDept 的前置校验与实际删除之间存在 TOCTOU 竞态
|
|
|
+### M-16. `loadPerms` 普通成员分支对"用户附加 ALLOW / DENY"未过滤权限启用状态
|
|
|
|
|
|
-- **位置**：`internal/logic/dept/deleteDeptLogic.go`
|
|
|
-- **描述**：流程依次执行：
|
|
|
- 1. `FindByParentId(id)` 校验无子部门
|
|
|
- 2. `FindIdsByDeptId(id)` 校验无关联用户
|
|
|
- 3. `Delete(id)`
|
|
|
+- **位置**：`internal/loaders/userDetailsLoader.go:380-405`
|
|
|
+- **描述**：`FindPermIdsByUserIdAndEffectForProduct` 仅按 `sys_user_perm.effect + productCode` 过滤，未过滤 `sys_perm.status = 1`。最终在第 412-422 行通过 `FindByIds` + `p.Status == Enabled` 过滤已禁用的 perm code，再投入 `ud.Perms`。
|
|
|
+ - 功能上正确：禁用的 perm 不会进入最终 codes。
|
|
|
+ - 但**每次 Load 都白白查询禁用的 permId**，多传一趟到 `FindByIds`。
|
|
|
+- **建议**：`FindPermIdsByUserIdAndEffectForProduct` 的 SQL 加 `AND p.status = 1`（对齐 `FindRoleIdsByUserIdForProduct` 已加的 `r.status = 1`）。同时 `FindPermIdsByRoleIds` 也可以加 `INNER JOIN sys_perm p ON rp.permId = p.id AND p.status = 1`。
|
|
|
|
|
|
- 步骤 2 与步骤 3 之间，另一个管理员可能：
|
|
|
- - 创建以该部门为父的子部门（`CreateDept`）
|
|
|
- - 把某用户迁入该部门（`UpdateUser.DeptId`）
|
|
|
+---
|
|
|
|
|
|
- 由于 `sys_dept` 没有对被删除部门的外键约束，删除会成功，留下"孤儿"子部门或"指向已删除部门的用户"。虽然都是超管才能执行、并发概率极低，但依然是逻辑一致性缺口。
|
|
|
-- **建议**：
|
|
|
- - 方案一：把三步放进一个事务，并对 `sys_dept` 行加 `SELECT ... FOR UPDATE`。
|
|
|
- - 方案二：采用"逻辑删除"代替物理删除。
|
|
|
- - 方案三：在 `CreateDept` / `UpdateUser` 的写入端校验目标部门 `status=Enabled` 且未被删除。
|
|
|
+## 📝 低风险 / 遗留问题 (Low)
|
|
|
+
|
|
|
+### L-1. 响应永远 HTTP 200，业务错误通过 body.code 区分
|
|
|
+
|
|
|
+- **位置**：`internal/response/response.go:45-52`
|
|
|
+- **描述**：所有业务错误返回 HTTP 200 + body.code=4xx/5xx。这让部分 WAF、CDN、监控工具（期望基于 HTTP 状态码告警）失效。属于已知的 API 契约选择，保留一致性即可，但建议在对外文档中明确。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### M-12. AddMember / BindRoles 的权限校验依赖 caller 的 ProductCode（JWT），但可操作的是 req.ProductCode
|
|
|
+### L-2. 敏感配置明文提交到仓库
|
|
|
|
|
|
-- **位置**：`internal/logic/member/addMemberLogic.go`、`internal/logic/user/bindRolesLogic.go`
|
|
|
-- **描述**：当前通过 `RequireProductAdminFor(req.ProductCode)` 校验调用者是 "req.ProductCode 这个产品的管理员"，这一步是正确的。但 `CheckMemberTypeAssignment(assignedType)` 内部是拿 `caller.MemberType`（= caller 自己 JWT 所在产品的 MemberType）和 `assignedType` 比较的；当 caller 切换到别的产品操作（通过传 req.ProductCode 与 caller.ProductCode 不同），这个比较实际是"A 产品的我的级别 vs B 产品要分配的级别"，语义错配。
|
|
|
+- **位置**：`etc/perm-api-*.yaml`
|
|
|
+- **描述**：MySQL/Redis 密码、AccessSecret、RefreshSecret、ManagementKey 等明文存在；即便后续轮换，历史 commit 仍可追溯。建议改用环境变量或密钥管理服务注入。
|
|
|
|
|
|
- 目前由于 `RequireProductAdminFor` 要求 `caller.MemberType == ADMIN && caller.ProductCode == req.ProductCode`，非超管的跨产品路径已经被卡住；只有超管能跨产品，而超管在 `CheckMemberTypeAssignment` 里直接 return nil。所以**当前没有实际越权风险**。
|
|
|
+---
|
|
|
|
|
|
-- **建议**：这属于"防御性冗余"——未来如果放宽 `RequireProductAdminFor` 的跨产品规则（例如允许全局 ADMIN），`CheckMemberTypeAssignment` 的语义就会失效。建议把 `CheckMemberTypeAssignment` 显式接收 `productCode`，内部按目标产品重新读 caller 在该产品的 MemberType：
|
|
|
+### L-3. `UpdateRole` 允许产品 ADMIN 任意下调 permsLevel 到 1
|
|
|
|
|
|
- ```go
|
|
|
- func CheckMemberTypeAssignmentFor(ctx, svcCtx, productCode, assignedType) error
|
|
|
- ```
|
|
|
+- **位置**：`internal/logic/role/updateRoleLogic.go:47-48`
|
|
|
+- **描述**：产品 ADMIN 可以把一个原本 `permsLevel=500` 的角色改为 `permsLevel=1`，然后把它绑给普通 MEMBER，使其 `MinPermsLevel=1`，进而绕过 `checkPermLevel` 对"同级 MEMBER"的级别约束。由于 ADMIN 本身已是产品最高级别，这并没有扩展其能力范围；但它让普通 MEMBER 能"管理与自己同 MemberType 的更多用户"。
|
|
|
+- **建议**：要求 `permsLevel` 的修改必须是超管；或要求新 `permsLevel >= 原 permsLevel`（单调不递减）。
|
|
|
|
|
|
---
|
|
|
|
|
|
-## 📝 低风险 / 遗留问题 (Low)
|
|
|
+### L-4. `CreateRole` 未校验 `req.ProductCode` 是否存在 / 启用
|
|
|
|
|
|
-### L-1. 敏感配置明文提交到仓库（遗留）
|
|
|
+- **位置**：`internal/logic/role/createRoleLogic.go:33-50`
|
|
|
+- **描述**：仅通过 `RequireProductAdminFor(req.ProductCode)` 间接校验（超管或该产品 ADMIN）。但如果超管误传 `productCode="not_exist"`，会插入一条挂在无效产品下的角色；该角色因产品不存在不会被任何人使用，但也不会报错。
|
|
|
+- **建议**：插入前 `FindOneByCode(req.ProductCode)` 校验存在且 `Status == Enabled`。
|
|
|
|
|
|
-- **位置**：`etc/perm-api-dev.yaml`、`etc/perm-api-prod.yaml`、`etc/perm-api-test.yaml`、`etc/perm-api-xiaom.yaml`
|
|
|
-- **描述**：MySQL/Redis 密码、AccessSecret、RefreshSecret、ManagementKey 等均以明文形式存在，且已提交至 Git。即使后续轮换，历史提交仍留痕。
|
|
|
-- **建议**：改用环境变量或密钥管理服务注入，`etc/*.yaml` 只保留模板。
|
|
|
+---
|
|
|
+
|
|
|
+### L-5. `AddMember` 未校验产品启用状态
|
|
|
+
|
|
|
+- **位置**：`internal/logic/member/addMemberLogic.go:33-34`
|
|
|
+- **描述**：只校验产品存在，不校验 `Status == Enabled`。管理员可以在产品已被禁用的情况下继续加成员。虽然被禁用的产品理论上也不应再有新成员流入，但当前不阻断。
|
|
|
+- **建议**：增加 `if product.Status != consts.StatusEnabled { return 400 "产品已被禁用" }`。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-2. 登录限流桶的维度混用
|
|
|
+### L-6. `UserDetailLogic` 对"非超管 + productCode==='' + 查他人"的校验语义模糊
|
|
|
|
|
|
-- **位置**：`internal/svc/servicecontext.go` 第 31 行、`internal/handler/routes.go` 第 143-160 行
|
|
|
-- **描述**：`LoginRateLimit`（IP 60s 20 次）**同时**挂给了 `/auth/login` 和 `/auth/adminLogin`。AdminLogin 现在有了 `UsernameLoginLimit`（5 分钟 10 次）作为第二道防线，这是合适的。但两个路由共享同一个 IP 桶，意味着：
|
|
|
- - 办公网 NAT 出口下 20 人在 1 分钟内都在尝试登录时，其他登录被误判。
|
|
|
- - 对产品端登录的合法压力会挤占管理后台登录的配额，反之亦然。
|
|
|
-- **建议**：拆成两个独立桶（`rl:login:product`、`rl:login:admin`），并可适度提高 product 端桶的配额。
|
|
|
+- **位置**：`internal/logic/user/userDetailLogic.go:33-43`
|
|
|
+- **描述**：逻辑上 "非超管 + 无 productCode + 查自己 → OK / 查他人 → 拒绝"。但对 ADMIN/DEVELOPER 而言，他们的 JWT productCode 不会为空；此分支只可能被"非超管 + 无 productCode"触发，当前系统里几乎只有"超管通过 adminLogin 登录"这一路径。
|
|
|
+ - 换言之这段 `if caller.ProductCode == ""` 分支只对"超管自身"有意义，但条件已显式排除超管。形同死代码分支——超管走不到，其它用户也不会 `ProductCode==""`。
|
|
|
+- **建议**：要么完全删除这段分支，要么明确写成 `if !caller.IsSuperAdmin && caller.ProductCode == "" { return 401 "会话缺少产品上下文" }`，表达"产品端一定有 productCode"的不变式。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-3. `UpdateDept` 整行回写，未使用乐观锁
|
|
|
+### L-7. `singleflight` 在 `Load` 失败时返回零值 UserDetails 而非 nil
|
|
|
|
|
|
-- **位置**：`internal/logic/dept/updateDeptLogic.go` 第 42-65 行
|
|
|
-- **描述**：仍是 Read-Modify-Write 模式，没有 `updateTime` 条件乐观锁（不像 `UpdateUser` 已用 `WHERE updateTime=?`）。部门操作是超管串行动作，并发概率极低，风险有限。
|
|
|
-- **建议**：若希望统一范式，与 `UpdateProfile` 一样接上 `WHERE updateTime = ?` 的乐观锁，失败返回 `ErrConflict`。
|
|
|
+- **位置**：`internal/loaders/userDetailsLoader.go:116-134`
|
|
|
+- **描述**：`sf.Do` 回调在 `loadFromDB` 返回 `ok=false` 时返回 `(nil, nil)`，外层做了兜底 `return &UserDetails{UserId, ProductCode}`。调用方靠 `ud.Username == ""` 判断"用户不存在"。
|
|
|
+ - 语义上"查不到用户"和"DB 报错"无法区分；
|
|
|
+ - 所有 caller 都按 `ud.Username == ""` 判定，耦合在这个不变式上。
|
|
|
+- **建议**：保留现有接口，但内部把 "DB error" 与 "不存在" 分开传递，对 5xx 让上层正确返回 500；同时在 `LoadE`（新增）接口里返回 `(*UserDetails, error)`，旧 `Load` 保持兼容。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-4. UpdateRole 允许 ADMIN 任意调整 permsLevel（设计层面）
|
|
|
+### L-8. `gRPC Login` 的 IP 提取依赖 `peer.Addr`，不识别 XFF
|
|
|
|
|
|
-- **位置**：`internal/logic/role/updateRoleLogic.go`
|
|
|
-- **描述**：产品 ADMIN 可以把一个角色的 permsLevel 从 500 改成 1，并绑定给普通 MEMBER，以此让该 MEMBER 的 `MinPermsLevel = 1`，进而绕过 `checkPermLevel` 的等级约束。由于 ADMIN 本身已经是高级别角色并拥有该产品全部权限，实际并未提升其权力范围；但"下放"级别的能力会让普通 MEMBER 能管理更多同级用户。
|
|
|
-- **建议**：如希望 permsLevel 成为"不可越级下放"的强约束，可要求修改 permsLevel 到低于某个阈值时必须是超管。当前风险较小。
|
|
|
+- **位置**：`internal/server/permserver.go:60-72`
|
|
|
+- **描述**：`GrpcLoginLimiter` 的 key 用 `peer.Addr`。如果 gRPC 入口前面有 gateway/proxy，所有请求的 `peer.Addr` 都是 gateway IP，限流变成"全局 20/min"。不过一般 gRPC 不会直接对外暴露，风险低。
|
|
|
+- **建议**：若 gRPC 会走网关，应在 metadata 中带上真实客户端 IP（如 `x-real-ip`），取 metadata 作为 key。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-5. singleflight 在 `Load` 失败路径仍返回带零值的 UserDetails
|
|
|
+### L-9. `BindRoles` 对 `req.RoleIds = []` 的语义是"清空所有绑定"，但缺少显式确认
|
|
|
|
|
|
-- **位置**：`internal/loaders/userDetailsLoader.go` 第 106-134 行
|
|
|
-- **描述**：当 `loadFromDB` 的 `ok == false`（例如用户不存在）时，仍然把 `ud` 返回给 `sf.Do` 调用方；随后 `Load` 的 caller 通过 `ud.Username == ""` 判断。目前中间件/登录路径都有后续的 `Username == ""` 检查，因此不会被滥用。建议在 Load 内部直接 `return nil`（不缓存、不复用），让上游直接看到 nil 语义，减少误用风险。
|
|
|
+- **位置**：`internal/logic/user/bindRolesLogic.go:48-58`
|
|
|
+- **描述**：传空数组时，`toAdd=[]`、`toRemove=existing`，流程会在事务里删光用户在该产品下的所有角色绑定。该语义合理（前端做全量覆盖），但没有任何二次确认或显式参数（`clearAll bool`），容易在前端误传 `[]` 时造成"误删"。
|
|
|
+- **建议**：在请求体中增加 `Intent: "replace" | "append"` 区分，或前端传 null/omitempty 时禁止清空。可选的 API 契约强化。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-6. FindAllCodesByProductCode 仅按 status 过滤，依赖 UNIQUE (productCode, code) 去重
|
|
|
+### L-10. `productmember` 的 `FindOneByProductCodeUserId` 没有按 `status` 过滤
|
|
|
|
|
|
-- **位置**：`internal/model/perm/sysPermModel.go` 第 53-60 行
|
|
|
-- **描述**：`SELECT code FROM sys_perm WHERE productCode=? AND status=1`。依赖 `uk_product_code(productCode, code)` 来保证单产品下 code 唯一，这是 schema 约束层面保证的。若将来有场景允许软删除（status=2）后重新创建同 code，需要注意历史禁用记录的去重。当前无风险，仅作提醒。
|
|
|
+- **位置**：model 层（`sys_product_member` 的 cached 查询）
|
|
|
+- **描述**：`loadMembership` 在拿到 member 后校验 `member.Status == Enabled`；`bindRolesLogic.go:44`、`setUserPermsLogic.go:44`、`updateUserStatusLogic.go:53` 等业务层都只检查"是否存在成员记录"，没有再检查成员状态。
|
|
|
+ - `bindRoles` 给"已被禁用的成员"重新绑角色：数据上写入，但由于 `jwtauthMiddleware` 不校验 MemberType（见 H-2），会和 H-2 联动放大影响。
|
|
|
+ - `setUserPerms` 同理。
|
|
|
+- **建议**：在这些业务校验处把 `FindOneByProductCodeUserId` 的结果加 `member.Status == Enabled` 判断，明确拒绝对已禁用成员的权限操作。
|
|
|
|
|
|
---
|
|
|
|
|
|
@@ -444,27 +532,33 @@
|
|
|
|
|
|
| 维度 | 评估 |
|
|
|
|------|------|
|
|
|
-| **逻辑一致性** | 发现两个严重逻辑 Bug：BindRoles 对 ADMIN/DEVELOPER 的 permsLevel 校验误伤（H-1），以及 DEV 部门绕过产品成员禁用（H-3）。 |
|
|
|
-| **并发与竞态** | 关键写操作已使用乐观锁/事务，`UpdateProfile` 已引入 `WHERE updateTime=?`；只剩 `DeleteDept` 的 TOCTOU（M-11）和 `UpdateDept` 的 RMW（L-3）。 |
|
|
|
-| **资源管理** | DB/Redis 连接统一由 go-zero 池管理，未见泄漏；事务 `TransactCtx` 用法正确。 |
|
|
|
-| **数据完整性** | 核心写入（创建产品、绑定角色权限、删除角色、同步权限、移除成员）都已放入事务。缓存失效与 DB 提交非原子（M-10），Redis 故障时存在 ≤5 min 的陈旧窗口。 |
|
|
|
-| **安全漏洞** | gRPC `GetUserPerms` 未校验用户状态（H-2），存在"本系统已冻结，对外仍有权限"的一致性漏洞；DEV 部门旁路（H-3）是真实的水平越权路径；组织架构/产品列表过度暴露（M-8、M-9）；adminPassword 熵过低（M-1）。 |
|
|
|
-| **边界处理** | 对 `nil`、空串、可选字段（指针）普遍处理得当；`UserDetails` 零值语义合理。 |
|
|
|
-| **数据库性能** | BindRoles / BindRolePerms 的逐条 DELETE（M-2）可批量化；其他列表接口均采用"批量查询 + map 组装"的正确模式，无 N+1。 |
|
|
|
-| **僵尸代码** | `Claims.Perms` 字段未被产线使用（M-6）；上一版审计报告中的几个 helper 与 model 函数已清理。 |
|
|
|
-| **接口契约与对象完整性** | `UserDetailLogic` 返回字段与产品上下文语义不一致（M-3）；`FindRoleIdsByUserIdForProduct` 未过滤角色状态（M-4）；`UpdateDept` 的缓存级联过宽（M-5）。 |
|
|
|
+| **逻辑一致性** | 新发现：产品禁用未联动 token 失效（H-1）、HTTP 中间件不校验禁用成员（H-2）、最后 ADMIN 保护缺失（H-4）。 |
|
|
|
+| **并发与竞态** | `BindRolePerms/DeleteRole/UpdateRole` 的"受影响用户"查询发生在事务外，存在并发缺漏（M-9）。其它关键写入已有乐观锁或 `FOR UPDATE`。 |
|
|
|
+| **资源管理** | go-zero `TransactCtx` 使用规范，`sqlx` 与 Redis 连接由池管理；未见泄漏。 |
|
|
|
+| **数据完整性** | 核心写路径（createProduct、bindRoles、bindRolePerms、removeMember、deleteRole、syncPerms）均在事务内，缓存失效为 fire-and-forget（M-8）。 |
|
|
|
+| **安全漏洞** | 产品禁用失效（H-1）、成员禁用不生效于 HTTP 路径（H-2）、管理后台账号锁定 DoS（H-3）、最后 ADMIN 失守（H-4）、产品端账号 DoS（M-6）、adminPassword 熵不足（M-3）、DeptTree / ProductList 暴露过度（M-4、M-5）。 |
|
|
|
+| **边界处理** | nil / 空串 / 可选字段（指针）处理普遍得当；`UserDetails` 零值语义仍依赖 `Username == ""` 约定（L-7）。 |
|
|
|
+| **DB 性能** | BindRoles / BindRolePerms / role 删除路径已批量化；其它列表接口采用"批量 IN + map 拼装"，无 N+1。`loadPerms` 的 role/user perm 查询可加 `p.status=1` 减少无效数据（M-16）。 |
|
|
|
+| **僵尸代码** | `SysDeptModel.FindByPathPrefix` / `FindByParentId` 仅测试引用（M-11）。`Claims.Perms` 已清理、`FindRoleIdsByUserId` 仍有调用路径。 |
|
|
|
+| **接口契约与对象完整性** | `UserDetails` 缺 `ProductStatus` 字段（H-1 所需）；`UpdateUserStatus` 有重复归属校验（M-12）。 |
|
|
|
|
|
|
### 修复优先级建议
|
|
|
|
|
|
-1. **立即修复（P0）**：
|
|
|
- - H-1 BindRoles 对 ADMIN 的误拦截 —— 直接影响产品管理员最核心的运营能力。
|
|
|
- - H-2 gRPC `GetUserPerms` 未校验冻结用户 —— 跨系统一致性漏洞，可能被接入方当作"真相源"从而放行冻结用户。
|
|
|
- - H-3 DEV 部门绕过产品成员禁用 —— 真实的越权路径。
|
|
|
-
|
|
|
-2. **短期修复（P1）**：
|
|
|
- - M-1 弱密码生成函数
|
|
|
- - M-2 批量 DELETE
|
|
|
- - M-3/M-4 roleIds 返回语义
|
|
|
- - M-10 缓存失效的原子性补偿
|
|
|
-
|
|
|
-3. **中期修复（P2）**：其他 M 级条目与 L 级遗留项。
|
|
|
+1. **立即修复（P0）**
|
|
|
+ - H-1 产品禁用不生效：加 `ProductStatus` 字段，`loadPerms` / `JwtAuth` / `GetUserPerms` / `VerifyToken` 统一校验，必要时在 `UpdateProduct` 递增成员的 `tokenVersion`。
|
|
|
+ - H-2 中间件不校验 MemberType：与 `RefreshToken` 对齐。
|
|
|
+ - H-3 AdminLogin DoS：ManagementKey 校验前置，rate limit key 加 IP 维度。
|
|
|
+ - H-4 最后 ADMIN 保护：`RemoveMember` / `UpdateMember` 增加 adminCount 前置校验。
|
|
|
+
|
|
|
+2. **短期修复（P1）**
|
|
|
+ - M-1 无注销接口 → 补 `/auth/logout`。
|
|
|
+ - M-2 refreshToken 轮转应让旧 token 失效。
|
|
|
+ - M-3 `generateRandomHex` 截断 bug + adminPassword 长度提升。
|
|
|
+ - M-4 DeptTree 权限过滤。
|
|
|
+ - M-5 ProductList/Detail 权限过滤。
|
|
|
+ - M-6 产品登录账号锁定 DoS（与 H-3 同步）。
|
|
|
+ - M-8 缓存失效原子性补偿（最高优先保障"收紧"类安全操作）。
|
|
|
+ - M-9 事务外用户集查询移到事务后。
|
|
|
+
|
|
|
+3. **中期修复（P2）**
|
|
|
+ - 其它 M/L 级条目（XFF 支持、LIKE 转义、僵尸代码清理、校验冗余去重、权限查询 `status=1` 过滤）。
|
BaiLuoYan