|
@@ -1,242 +1,183 @@
|
|
|
-# 权限管理系统 —— 深度代码审计报告(第 8 轮)
|
|
|
|
|
|
|
+# 权限管理系统 —— 深度代码审计报告(第 9 轮)
|
|
|
|
|
|
|
|
> **审计范围**:`/internal` 下全部非测试、非 `_gen.go` 生产代码(含 `internal/server/permserver.go`、HTTP logic / handler / middleware、loaders、model 定制层、svc、util、consts)。
|
|
> **审计范围**:`/internal` 下全部非测试、非 `_gen.go` 生产代码(含 `internal/server/permserver.go`、HTTP logic / handler / middleware、loaders、model 定制层、svc、util、consts)。
|
|
|
> **审计时间**:2026-04-19
|
|
> **审计时间**:2026-04-19
|
|
|
> **审计维度**:逻辑一致性 / 并发与 RMW / 资源管理 / 数据完整性 / 安全漏洞 / 边界坍塌 / DB 性能 / 僵尸代码 / 接口契约与对象完整性。
|
|
> **审计维度**:逻辑一致性 / 并发与 RMW / 资源管理 / 数据完整性 / 安全漏洞 / 边界坍塌 / DB 性能 / 僵尸代码 / 接口契约与对象完整性。
|
|
|
-> **与第 7 轮对比**:
|
|
|
|
|
-> - **已落地(本轮不再复列)**:H-1 `loadPerms` deny fail-open、H-3 `AddMember` 目标侧授权、H-4 JWT HMAC 断言、M-1 `Load` 半成品缓存污染、M-2 `UpdatePassword`/`UpdateStatus` 未校验 `RowsAffected`、M-4 `CreateProduct` 明文密码(切成一次性 `credentialsTicket`)、M-7 gRPC 限流剥端口、L-1 `DeleteDept` 锁序列、L-2 `IncrementTokenVersion` WARN 注释、L-3 `loadPerms` 其余分支 fail-close、L-4 SQL `status = ?` 参数化(`sysUserRole` / `sysUserPerm` / `sysRole` 三处)、L-5 `FindMapByProductCodeUserIds` / `FindMapByProductCode` 非事务版移除、L-6 gRPC 负缓存预污染(TTL=10s + 预写前强一致 `FindOne`)。
|
|
|
|
|
-> - **未落地 / 回归**:**H-2 PII 暴露**(第 6~7 轮持续未修),**M-3 残留分支**(`GuardRoleLevelAssignable` 已走 fresh read,但 `CheckManageAccess → checkPermLevel` 仍读缓存 `caller.MinPermsLevel`,降级 admin 的 TOCTOU 窗口只封了"分配角色"一个出口),L-5 `CountActiveAdminsTx` 零调用,L-7 历史账号 `DeptId=0` 兜底。
|
|
|
|
|
-> - **新发现**:M-N1 `CreateProduct → Redis 票据写入失败` 导致**产品/管理员孤儿化**;M-N2 `checkPermLevel` 读缓存 `MinPermsLevel` 的 TOCTOU 口子(M-3 未闭合的另一半);M-N3 `SyncPermsError{Code:404}` 在 gRPC 映射里被同化成 `codes.Internal`;L-N1 `sysPermModel` 仍用 `fmt.Sprintf("... status = %d", consts.StatusEnabled)`,与 L-4 修复风格不一致;L-N2 `SetUserPerms` `FindByIds` 校验与 `BatchInsert` 之间的 TOCTOU(影响面轻,列入存档)。
|
|
|
|
|
|
|
+> **与第 8 轮对比**:
|
|
|
|
|
+> - **已落地(本轮复核通过,不再复列修复细节)**:
|
|
|
|
|
+> - **H-2**(第 8 轮)`checkPermLevel` 读缓存 `MinPermsLevel` 的 TOCTOU —— `internal/logic/auth/access.go:302-347` 现统一调用 `loadFreshMinPermsLevel` 走 DB 复核,`sqlx.ErrNotFound` 降级为"0 roles"、其他 DB 错误向上冒出。
|
|
|
|
|
+> - **M-3**(第 7~8 轮)RefreshToken CAS 在签发失败后脏升级 `tokenVersion` —— `internal/logic/pub/refreshTokenLogic.go:90-110` 与 `internal/server/permserver.go:215-230` 均已把 `Generate*Token` 前置到 `IncrementTokenVersionIfMatch` 之前,CAS 只在两份 token 都成功生成后执行。
|
|
|
|
|
+> - **M-N1**(第 8 轮)CreateProduct Redis 票据写失败孤儿化 —— `internal/logic/product/createProductLogic.go:178-283` 已落地 `compensateCreatedRows`:ticket Setex 任一步失败都会按 admin / member / product 倒序硬删,并附 WARN 日志便于对账。
|
|
|
|
|
+> - **M-N3**(第 8 轮)gRPC `SyncPermsError{Code:404}` 被吞成 `codes.Internal` —— `internal/server/permserver.go:61-90` 已加 `errors.As` 显式映射到 `codes.NotFound`,并保留具体错误文案。
|
|
|
|
|
+> - **L-2**(第 7~8 轮)`CountActiveAdminsTx` 僵尸方法 —— `internal/model/productmember/sysProductMemberModel.go` 中仅保留 `CountOtherActiveAdminsTx`,接口与实现两端均已删除旧签名。
|
|
|
|
|
+> - **L-N1**(第 8 轮)`sysPermModel` 仍拼接 `fmt.Sprintf("... status = %d", ...)` —— 现统一用 `?` 占位(`FindAllCodesByProductCode:57-67`、`DisableNotInCodesWithTx:100-154`)。
|
|
|
|
|
+> - **L-N2**(第 8 轮)`SetUserPerms` 的 `FindByIds` → `BatchInsert` TOCTOU —— `internal/logic/user/setUserPermsLogic.go:148-165` 已在事务内对目标权限集做 `COUNT(*) WHERE status=?` 复核,并要求计数等于入参长度,否则 rollback。
|
|
|
|
|
+> - **产品契约接受、不列入风险清单**:
|
|
|
|
|
+> - **H-1 同产品成员 PII 互见**(第 6 → 7 → 8 轮累计列入 P0)—— 产品确认"内部系统 + 通讯录互见"为业务契约,不修。该条从本轮起归档,不再进入后续审计复核列表。
|
|
|
|
|
+> - **未落地 / 回归**:
|
|
|
|
|
+> - **L-3 历史 `DeptId=0` 账号的 `CheckManageAccess` 兜底**(需数据迁移,不属纯代码问题,延续存档)。
|
|
|
|
|
+> - **本轮新发现(权重较高的 5 条)**:
|
|
|
|
|
+> - **M-N1(新)`userDetailsLoader.Load` 的 `loadOk=false` 契约错位** —— 基础设施故障被同化为业务 403 / "产品已禁用",前端侧不重试且 SOC 无故障信号。
|
|
|
|
|
+> - **M-N2(新)`BatchDel` 中 `unregisterCacheKey` 走串行 N×2 次 SREM** —— 绑/解绑/改角色链路退化为 O(N) Redis RTT,属 N+1。
|
|
|
|
|
+> - **M-N3(新)`RoleDetail` 枚举 oracle** —— 跨产品访问 403 vs 404 可区分,可用于穷举合法 roleId。
|
|
|
|
|
+> - **M-N4(新)`CreateUser` 未做 caller→`req.DeptId` 的部门链校验** —— 产品 ADMIN 可为**非自己管辖的部门**预埋用户名并占坑,绕过 AddMember 侧部门链防护。
|
|
|
|
|
+> - **L-N1(新)`ParseWithHMAC` helper 未统一使用** —— `jwtauthMiddleware.go`、gRPC `VerifyToken` 仍用内联 `token.Method.(*jwt.SigningMethodHMAC)` 断言,三处拷贝导致审计面错位。
|
|
|
|
|
+> - **本轮复现的低风险残点**:L-N2 `UpdateUser` 调部门未校验 `dept.Status`;L-N3 `AdminLogin` `IsSuperAdmin` 判断在 bcrypt 之后;L-N4 `sysUserModel.UpdateStatus` 缺乐观锁字段(由上层短路保护)。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
|
|
-### H-1. `UserDetailLogic` / `UserListLogic` 仍把 `Email` / `Phone` / `Remark` 暴露给**任意同产品成员**(第 6 轮 M-3、第 7 轮 H-2,三轮未落地)
|
|
|
|
|
-- **位置**:
|
|
|
|
|
- - `internal/logic/user/userDetailLogic.go:64-76`(`types.UserItem` 构造)
|
|
|
|
|
- - `internal/logic/user/userListLogic.go:77-89`
|
|
|
|
|
-- **描述**:两接口的访问控制仍然只到"同产品即可看全字段"的粒度:
|
|
|
|
|
- - `UserDetail` 只用 `FindOneByProductCodeUserId` 核对 caller 与 target 是否在同一产品,之后直接塞 `Email / Phone / Remark`(纯文本,无脱敏)。
|
|
|
|
|
- - `UserList` 分页返回同产品所有成员,每条也原样带上 `Email / Phone / Remark`。一次分页可以把整个产品通讯录灌下来。
|
|
|
|
|
-- **影响**:
|
|
|
|
|
- - **同产品最低权限 MEMBER 即可遍历整个产品通讯录**,获取手机 / 邮箱 / 备注(备注里常常会有内部身份、外部联络人、岗位说明等 PII)。
|
|
|
|
|
- - 叠加风险:
|
|
|
|
|
- - 即便 H-3(第 7 轮,已修)已关掉了"ADMIN 跨部门 AddMember"的入口,现有 ADMIN 仍可通过 `UserList` 一次拿走本产品全员通讯录;
|
|
|
|
|
- - 对于拿到一次性有效 JWT 的外包 / 服务账号,本接口天然就是拖库点。
|
|
|
|
|
- - 违反《个人信息保护法》第 6 / 13 条的最小必要原则与《数据安全法》下的分级保护要求。
|
|
|
|
|
-- **修复方案**(与前两轮报告一致,代码未落):
|
|
|
|
|
- 1. 在 `internal/logic/auth/access.go` 新增:
|
|
|
|
|
- ```go
|
|
|
|
|
- // CanViewContact 判定 caller 是否可以看 target 的联系信息字段。
|
|
|
|
|
- // - caller 是 SuperAdmin → true
|
|
|
|
|
- // - caller.UserId == target.Id(看自己) → true
|
|
|
|
|
- // - CheckManageAccess(caller, target) 通过(caller 在 target 的管理链上) → true
|
|
|
|
|
- // - 其余情况一律 false,由 filterPIIForCaller 落地脱敏。
|
|
|
|
|
- func CanViewContact(ctx context.Context, svcCtx *svc.ServiceContext, caller *loaders.UserDetails, target *userModel.SysUser, productCode string) bool
|
|
|
|
|
- ```
|
|
|
|
|
- 2. 新增 `authHelper.MaskEmail` / `MaskPhone`(`138****1234` / `a***@b.com`),在 Logic 构造 DTO 前调用 `filterPIIForCaller(caller, target, &item)` 统一覆盖 `Email / Phone / Remark` 三字段。
|
|
|
|
|
- 3. 单测覆盖四种身份组合:同产品同级 MEMBER 互看、跨部门互看、ADMIN 看下级、看自己。
|
|
|
|
|
-- **为什么必须本轮解决**:前两轮已经连续标记为 P0 且提出了完整方案,在 H-3(AddMember)修完之后这条变成"剩下最大的单点 PII 出口",攻击面大、修复工作量小(一个 helper + 两个 Logic 返回前的 hook)。
|
|
|
|
|
-
|
|
|
|
|
-### H-2. `CheckManageAccess → checkPermLevel` 仍靠 caller 的**缓存 `MinPermsLevel`** 决策,降级 admin 在 5 分钟 TTL 内仍可管辖原本够不到的目标(M-3 只封了一半)
|
|
|
|
|
-- **位置**:
|
|
|
|
|
- - `internal/logic/auth/access.go:279-316`(`checkPermLevel`)
|
|
|
|
|
- - 受影响调用方:`internal/logic/member/updateMemberLogic.go`、`internal/logic/member/removeMemberLogic.go`、`internal/logic/user/updateUserLogic.go`、`internal/logic/user/updateUserStatusLogic.go`、`internal/logic/user/setUserPermsLogic.go`
|
|
|
|
|
-- **描述**:第 7 轮对 `GuardRoleLevelAssignable` 的修复方式是"在做 role 分配决策前 `FindMinPermsLevelByUserIdAndProductCode` 走一次 NoCache 查 DB"。但**同样类型的 TOCTOU 问题**在更一般的 `CheckManageAccess` 里没修:
|
|
|
|
|
- ```go
|
|
|
|
|
- // access.go:312
|
|
|
|
|
- if caller.MinPermsLevel >= targetLevel {
|
|
|
|
|
- return response.ErrForbidden("无权管理权限级别高于或等于您的用户")
|
|
|
|
|
- }
|
|
|
|
|
- ```
|
|
|
|
|
- 这里 `caller` 来自 `middleware.GetUserDetails(ctx)`,也就是 `UserDetailsLoader.Load` 缓存的那份;TTL 5 分钟。
|
|
|
|
|
- 攻击路径:
|
|
|
|
|
- 1. T=0:超管把 caller C 从 `MinPermsLevel=10`(总监级)降到 `20`(经理级)。超管调用业务接口时会触发 `UserDetailsLoader.Clean(C.UserId)`,但 Redis 抖动 / 集群主从切换期间,`Clean` 单次失败只会被 `logx.Errorf`,没有重试、没有降级旁路。
|
|
|
|
|
- 2. T=0+δ:C 在另一终端调 `RemoveMember` / `UpdateMember` / `UpdateUser` / `UpdateUserStatus` / `SetUserPerms` 去管理一个 `MinPermsLevel=15` 的目标 D。
|
|
|
|
|
- 3. `checkPermLevel` 读到 C 的缓存 `MinPermsLevel=10`,判定 `10 >= 15` 为 false → 放行。C 的**实际**级别 20,`20 >= 15` 为 true,本应被拦。
|
|
|
|
|
- 4. C 有整整 5 分钟时间把下属统一踢人、改 MemberType、覆盖 UserPerms、冻结状态 —— 审计日志里看起来是合法操作。
|
|
|
|
|
-- **与 M-3 修复的对照**:`GuardRoleLevelAssignable` 修的是"C 用旧身份去授角色给别人"的路径;`checkPermLevel` 漏的是"C 用旧身份去直接动别人"的路径。两条路径一个在**出**一个在**改**,安全边界对称才叫完整。
|
|
|
|
|
-- **影响**:与 M-3 同一量级,但触达面更广(5 个 Logic 都走 `CheckManageAccess`),且 `RemoveMember` / `UpdateUserStatus` 可以直接产生**不可逆的破坏性操作**(把管理员从产品踢出、冻结账号)。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
- 1. `checkPermLevel` 对 caller.MinPermsLevel 采用与 `GuardRoleLevelAssignable` 一致的策略:
|
|
|
|
|
- ```go
|
|
|
|
|
- freshCallerLevel, err := svcCtx.SysRoleModel.FindMinPermsLevelByUserIdAndProductCode(ctx, caller.UserId, productCode)
|
|
|
|
|
- if err != nil {
|
|
|
|
|
- if errors.Is(err, sqlx.ErrNotFound) {
|
|
|
|
|
- // caller 当前已无产品角色 → 等同最低等级,对同级管辖一律拒绝
|
|
|
|
|
- return response.ErrForbidden("无权管理权限级别高于或等于您的用户")
|
|
|
|
|
- }
|
|
|
|
|
- return response.NewCodeError(500, "校验权限级别失败,请稍后重试")
|
|
|
|
|
- }
|
|
|
|
|
- if freshCallerLevel >= targetLevel {
|
|
|
|
|
- return response.ErrForbidden("无权管理权限级别高于或等于您的用户")
|
|
|
|
|
- }
|
|
|
|
|
- ```
|
|
|
|
|
- 2. 抽一个共享 helper `loadFreshMinPermsLevel(ctx, svcCtx, userId, productCode) (int64, error)`,`GuardRoleLevelAssignable` 和 `checkPermLevel` 统一调用;同时顺手把两处的 `ErrNotFound` 语义文档化。
|
|
|
|
|
- 3. 长期建议(可选):给 `UserDetailsLoader` 加一个 `LoadFresh(ctx, userId, productCode)` 接口,授权决策点强制 bypass 缓存;普通业务依然走缓存。
|
|
|
|
|
|
|
+**本轮无 High Risk 项。**
|
|
|
|
|
+- 第 8 轮列示的 H-2(`checkPermLevel` TOCTOU)已通过 `loadFreshMinPermsLevel` 闭环,见上方"已落地"小节。
|
|
|
|
|
+- 第 6~8 轮列示的 H-1(同产品 PII 互见)经产品确认为系统对内使用场景下的既定契约,自本轮起归档、不再纳入审计清单。
|
|
|
|
|
+- 其余安全类待办均已降级至 Medium / Low,见下节。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
## ⚠️ 健壮性与性能建议 (Medium / Low)
|
|
## ⚠️ 健壮性与性能建议 (Medium / Low)
|
|
|
|
|
|
|
|
-### M-1. `CreateProduct` 事务已提交后,Redis 票据写入失败会把产品 + admin 用户**永久孤儿化**(第 7 轮 M-4 修复方案遗留的新缺陷)
|
|
|
|
|
-- **位置**:`internal/logic/product/createProductLogic.go:109-185`
|
|
|
|
|
-- **描述**:第 7 轮把 M-4(响应体明文密码)改成了"DB 事务提交 → 暂存 Redis → 响应 ticket"三段流水线。但流水线的失败语义没有做补偿:
|
|
|
|
|
- 1. L109-149:`SysProductModel.TransactCtx` 里同时 INSERT `sys_product` / `sys_user`(admin 账号)/ `sys_product_member`。成功后 `productId` 已经持久化。
|
|
|
|
|
- 2. L162-169:`generateRandomHex(32)` 生成 ticket —— 理论上 `crypto/rand.Read` 出错概率 ≈ 0,但已不在事务内。
|
|
|
|
|
- 3. L176-180:`json.Marshal(&payload)` —— 对固定结构体几乎不会失败,但落出错分支同样只返回 500。
|
|
|
|
|
- 4. L181-184:`Redis.SetexCtx(ticketKey, …)` —— **这里才是真实风险面**。Redis 短暂不可用 / 超时 / 集群 failover 都会让 Setex 返 err。
|
|
|
|
|
- 落到任一 fail 分支时:
|
|
|
|
|
- - `sys_product` 里新建的产品记录已经落盘;
|
|
|
|
|
- - `admin_<code>` 账号已经落盘,bcrypt 密码是**我们刚刚在内存里生成、随 500 响应丢弃**的那串随机 12 字节;
|
|
|
|
|
- - 没有任何方式可以拿回这个密码:
|
|
|
|
|
- - 仓库里没有 `DeleteProductLogic`(只有 `CreateProduct` / `UpdateProduct` / `ProductList` / `ProductDetail`);
|
|
|
|
|
- - 仓库里没有 "SuperAdmin Reset Password" 类型的接口,只有 `ChangePasswordLogic`,且它要求 `oldPassword` 校验通过;
|
|
|
|
|
- - `CreateProduct` 再试一次会命中 `product.Code` / `admin_<code>` 的 `FindOneByCode` / `FindOneByUsername` 前置判定,直接 `ErrConflict`。
|
|
|
|
|
- 运维只能下场直接 SQL:要么跑一次 `UPDATE sys_user SET password = ? WHERE username = ?` 硬改 admin 密码,要么 `DELETE` 三张表对应数据。**这两种手法都绕过了业务不变式和审计日志。**
|
|
|
|
|
-- **影响**:数据完整性问题。概率虽低(依赖 Redis 单次失败 & 在一次创建流程内),但一旦发生是**永久性**的(孤儿产品不会自愈),且需要手工改库,违反最小事故介入原则。
|
|
|
|
|
-- **修复方案**(按代价从低到高):
|
|
|
|
|
- 1. **最小成本**:`Redis.SetexCtx` 失败时,在同一 handler 内做补偿 DB 事务——删除刚才创建的 `sys_product` / `sys_user` / `sys_product_member`,回到"从未创建"状态再返 500。为防止补偿事务自己也失败,至少要把 `productId` / `adminUserId` 落一条 `logx.Errorw("createProduct compensation required", ...)` 带有 ERROR 等级 + 结构化字段,方便告警侧接管人工回捞。
|
|
|
|
|
- 2. **更稳**:新增 `SuperAdmin` 专用接口 `RegenerateInitialCredentials(productCode)` —— 找到 `admin_<code>`,重新生成随机密码、bcrypt 后写 DB,再走一次 `SetexCtx` + 返回新的 `credentialsTicket`;与 `ChangePassword` 解耦,有独立审计日志字段 `audit=regenerate_init_cred`。
|
|
|
|
|
- 3. **一步到位**:引入 outbox / 2PC 风格 —— DB 事务 + `sys_outbox` 行同时写;单独 worker 消费 outbox,驱动 Redis 写入;对调用方接口异步化(响应体先给 `ticketId`,轮询拉真值)。落地成本高,按本仓规模不建议。
|
|
|
|
|
-- **锁死建议**:无论采纳 (1) 还是 (2),在上线前至少要补齐:`CreateProduct` 集成测试中把 `Redis.Setex` 做成主动注错路径,断言 "产品不存在 / admin 用户不存在(方案 1)" 或 "可以通过 Regenerate 拿回凭证(方案 2)",防止未来重构又漏掉这一路径。
|
|
|
|
|
-
|
|
|
|
|
-### M-2. `SyncPermsError{Code: 404}` 在 gRPC 映射里被同化成 `codes.Internal`,接口契约泄露 DB 语义
|
|
|
|
|
-- **位置**:
|
|
|
|
|
- - `internal/logic/pub/syncPermsService.go:75-79`(事务内 `LockByCodeTx` 返 `ErrNotFound` → `SyncPermsError{Code: 404}`)
|
|
|
|
|
- - `internal/server/permserver.go:67-83`(gRPC 层映射)
|
|
|
|
|
-- **描述**:gRPC handler 的 `switch se.Code` 只列了 400 / 401 / 403 / 409,其余(含 404 / 500 / 任何异常值)一律落 `default → codes.Internal`。于是:
|
|
|
|
|
- 1. 前置 `FindOneByAppKey` 已经能 hit 到一条 `sys_product`,但事务内 `LockByCodeTx` 又 `ErrNotFound`(极罕见:并发 "DeleteProduct" —— 虽然目前没有 Delete 入口,但一旦加上就中招);
|
|
|
|
|
- 2. 调用方(产品接入方的服务端)拿到 `codes.Internal` + 消息 "产品不存在",但 SDK 侧的重试策略一般对 `Internal` 是 "not retriable" —— 调用方会直接当作永久错误上报,而我们实际希望它是 `codes.NotFound` 让其按 404 处理。
|
|
|
|
|
-- **影响**:契约级一致性缺陷。目前无直接安全风险,但一旦未来引入 DeleteProduct / ProductCode 重命名逻辑,会给接入方的调用栈制造误导性错误分类(Internal 会 page 值班,NotFound 不会)。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
- ```go
|
|
|
|
|
- case 404:
|
|
|
|
|
- return nil, status.Error(codes.NotFound, se.Message)
|
|
|
|
|
- ```
|
|
|
|
|
- 同时把 HTTP 侧的 `response.NewCodeError(se.Code, …)` / gRPC 侧的映射表都抽到一个 `mapSyncPermsErr` helper,避免两边漂移。
|
|
|
|
|
-
|
|
|
|
|
-### M-3. `RefreshToken` CAS 成功后,若 `GenerateAccessToken` / `GenerateRefreshTokenWithExpiry` 失败,tokenVersion 已递增但客户端拿不到新令牌,用户被强制退出
|
|
|
|
|
-- **位置**:`internal/server/permserver.go:198-229`
|
|
|
|
|
|
|
+### M-N1. `userDetailsLoader.loadFromDB` 的 `loadOk=false` 语义错位,导致基础设施故障被同化为业务拒绝(新发现)
|
|
|
|
|
+- **位置**:`internal/loaders/userDetailsLoader.go:138-204`、`:321-574`(`loadFromDB`、`loadDept`、`loadProduct`、`loadPerms` 等子加载)。
|
|
|
- **描述**:
|
|
- **描述**:
|
|
|
- - L199:`IncrementTokenVersionIfMatch` 成功把 DB 的 `tokenVersion` 从 N 增到 N+1,返回 `newVersion`。
|
|
|
|
|
- - L206:`UserDetailsLoader.Clean(ctx, claims.UserId)` 清缓存(此时 TokenVersion 在 DB 是 N+1,但在用户手上的 refreshToken 还是 N)。
|
|
|
|
|
- - L208-214:生成新 accessToken 失败(理论上 HMAC signing 几乎不会失败,但 OOM / 奇怪的运行时错误不是 0 概率)。
|
|
|
|
|
- - L216-223:生成新 refreshToken 失败亦然。
|
|
|
|
|
- - 任一处失败就直接 `return nil, status.Error(codes.Internal / codes.Unauthenticated, ...)`。**客户端的老 refreshToken 因为 tokenVersion 对不上,下一次刷新会被 `claims.TokenVersion != ud.TokenVersion` 一刀切成 "登录状态已失效"**。用户必须完整重登,体验上等同于被踢下线。
|
|
|
|
|
-- **影响**:可用性 / 数据完整性:签名失败原本是 100% 服务端 bug,用户却要重登。量不大但会污染"非预期登出"告警,淹没真正的会话劫持信号。
|
|
|
|
|
-- **修复方案**:
|
|
|
|
|
- 1. 重排顺序:先生成两个新 token(不成功就直接 500,不动 DB),成功后再 CAS 递增 tokenVersion、Clean 缓存。即便递增后 log 层有问题,至少签名成功的 token 一定带回给了客户端。
|
|
|
|
|
- 2. 若坚持"先 CAS 再签名"(语义上更安全:CAS 成功才证明本次 refresh 是唯一 winner),则失败路径需要记一条明确的 `audit=refresh_post_cas_sign_fail userId=X oldVer=N newVer=N+1` 的 ERROR 日志,并把返回消息改成用户可感知的 "登录刷新失败,请重新登录"(保留现有行为但补上 observability)。
|
|
|
|
|
-
|
|
|
|
|
-### L-1. `sysPermModel.go` SQL 仍用 `fmt.Sprintf("... status = %d", consts.StatusEnabled)`,与 L-4 本轮修复风格不一致
|
|
|
|
|
-- **位置**:`internal/model/perm/sysPermModel.go:59,102,135`
|
|
|
|
|
- ```go
|
|
|
|
|
- query := fmt.Sprintf("SELECT `code` FROM %s WHERE `productCode` = ? AND `status` = %d", m.table, consts.StatusEnabled)
|
|
|
|
|
- findQuery := fmt.Sprintf("SELECT %s FROM %s WHERE `productCode` = ? AND `status` = %d", sysPermRows, m.table, consts.StatusEnabled)
|
|
|
|
|
- updateQuery := fmt.Sprintf("UPDATE %s SET `status` = %d, `updateTime` = ? WHERE `productCode` = ? AND `status` = %d", m.table, consts.StatusDisabled, consts.StatusEnabled)
|
|
|
|
|
- ```
|
|
|
|
|
-- **描述**:第 7 轮 L-4 已经把 `sysUserRole` / `sysUserPerm` / `sysRole` 三处改成了占位符 `?` + 参数传 `consts.StatusEnabled`,但 `sysPermModel` 的三条查询还是走 `fmt.Sprintf` 把 `int` 直接嵌到 SQL 里。问题不是注入(`consts.StatusEnabled` 是编译期常量),而是:
|
|
|
|
|
- 1. **类型契约不稳**:如果未来把 `StatusEnabled` 从 `int` 改成 `int8` / `uint8` / `ActiveStatus` enum,`%d` 可能要改,占位符版本可以稳定。
|
|
|
|
|
- 2. **审计一致性**:L-4 的修复初衷是"SQL 里**不再出现**状态常量字面值,统一走 prepared statement 占位"。`sysPermModel` 的三处仍然把数字编进 SQL 字符串(虽然是间接通过 Sprintf),与 L-4 的意图偏离。
|
|
|
|
|
-- **影响**:非安全问题,属于审计口径一致性 / 未来可维护性。
|
|
|
|
|
|
|
+ - `loadFromDB` 约定:
|
|
|
|
|
+ - `(ud, true, nil)` — 全量成功,写 5 分钟缓存;
|
|
|
|
|
+ - `(ud, false, nil)` — 子段(dept / product / membership / roles / perms)中某段失败,**返回残缺 ud、仅跳过缓存写**;
|
|
|
|
|
+ - `(nil, _, err)` — 主体加载失败,`Load` 向上传 error。
|
|
|
|
|
+ - `Load(:180-183)` 在 `loadOk=false` 时 `return ud, nil`。调用方拿到的是 `Username≠""`、但 `DeptPath=""` / `Perms=nil` / `ProductStatus=0` 的"半成品":
|
|
|
|
|
+ - `jwtauthMiddleware.go` 只校 `tokenVersion` 与 `IsSuperAdmin || MemberType!=""`,**放行**;
|
|
|
|
|
+ - `refreshTokenLogic.go` 的 `ProductStatus != StatusEnabled` 分支把这种 case 归类为"产品已被禁用",返 403;
|
|
|
|
|
+ - 其余业务接口因 `Perms=nil` 命中 `hasPerm=false`,返 403。
|
|
|
|
|
+ - 结果:一次 Redis / MySQL 抖动对外就是 403 "无权 / 产品禁用",而真正应该是 503 "上游暂不可用,请重试"。**前端不会重试**,SOC 也不会出现任何异常信号,故障被静默。
|
|
|
|
|
+- **影响**:
|
|
|
|
|
+ - 故障可观测性塌陷;SLO 把"基础设施降级"错报成"正常业务拒绝"。
|
|
|
|
|
+ - 用户体验上,一次瞬时 DB 抖动会对全体在线用户抛"产品已被禁用" / "无权限"——比直接 500 更容易引起误解和 P1 工单。
|
|
|
- **修复方案**:
|
|
- **修复方案**:
|
|
|
- ```go
|
|
|
|
|
- query := fmt.Sprintf("SELECT `code` FROM %s WHERE `productCode` = ? AND `status` = ?", m.table)
|
|
|
|
|
- // 调用处:
|
|
|
|
|
- QueryRowsNoCacheCtx(ctx, &dest, query, productCode, consts.StatusEnabled)
|
|
|
|
|
- ```
|
|
|
|
|
- 三条一致处理;`BatchUpdateWithTx` 里的 `status = ?` 同理。
|
|
|
|
|
-
|
|
|
|
|
-### L-2. `CountActiveAdminsTx` 零调用仍在接口里公开(第 7 轮 L-5 部分遗留)
|
|
|
|
|
-- **位置**:`internal/model/productmember/sysProductMemberModel.go`(接口 + 实现)
|
|
|
|
|
-- **描述**:第 7 轮已经把 `FindMapByProductCodeUserIds` / `FindMapByProductCode`(非事务版)从模型接口里干掉。但与之同批引入的 `CountActiveAdminsTx`(不带 `Other`)在业务层的实际调用方是 `CountOtherActiveAdminsTx`;`CountActiveAdminsTx` 自身只在 mock / test 里被引用。
|
|
|
|
|
-- **验证方式**:
|
|
|
|
|
- ```shell
|
|
|
|
|
- rg -n 'CountActiveAdminsTx' internal/logic | wc -l # 0
|
|
|
|
|
- rg -n 'CountOtherActiveAdminsTx' internal/logic | wc -l # >0
|
|
|
|
|
- ```
|
|
|
|
|
-- **影响**:僵尸接口方法,增加接口 surface area 与未来误用机会("应该用 `CountActiveAdminsTx` 还是 `CountOtherActiveAdminsTx`?"的歧义)。
|
|
|
|
|
-- **修复方案**:从 interface / 实现 / mock / 单测里删除 `CountActiveAdminsTx`;保留一条 `CountOtherActiveAdminsTx`(业务语义是"除自己以外还有几个活跃 admin",刚好吻合"不能移除/降级最后一个 admin"的不变式)。
|
|
|
|
|
-
|
|
|
|
|
-### L-3. `CheckManageAccess` 对 caller `DeptId=0` 且非 ADMIN / SuperAdmin 的历史账号直接 403(第 7 轮 L-7 未消化)
|
|
|
|
|
-- **位置**:`internal/logic/auth/access.go`(`checkDeptHierarchy` 内 `caller.DeptId == 0 || caller.DeptPath == ""` 分支)
|
|
|
|
|
-- **描述**:历史遗留账号仍有 `DeptId=0` 的 MEMBER / DEVELOPER,即使在自己的产品范围内想做简单的"看自己 / 改自己"操作,也会被 `checkDeptHierarchy` 拒 403(除非上层已短路)。
|
|
|
|
|
-- **修复方案**(任选其一,两者都不破坏现有安全边界):
|
|
|
|
|
- 1. 运维侧一次性 SQL:
|
|
|
|
|
- ```sql
|
|
|
|
|
- UPDATE sys_user SET deptId = <DEFAULT_NORMAL_DEPT_ID>
|
|
|
|
|
- WHERE deptId = 0 AND isSuperAdmin = 0 AND (
|
|
|
|
|
- userId IN (SELECT userId FROM sys_product_member WHERE memberType != 'ADMIN')
|
|
|
|
|
- OR userId NOT IN (SELECT userId FROM sys_product_member)
|
|
|
|
|
- );
|
|
|
|
|
- ```
|
|
|
|
|
- 同时 `UserDetailsLoader.CleanByUserIds` 一次性批量清缓存。
|
|
|
|
|
- 2. 代码侧:`CheckManageAccess` 早期追加
|
|
|
|
|
- ```go
|
|
|
|
|
- if caller.UserId == targetUserId {
|
|
|
|
|
- return nil
|
|
|
|
|
- }
|
|
|
|
|
- ```
|
|
|
|
|
- (这一条已经在 L60-69 实现了,但仅对"看自己"生效;真正被 403 的路径是"看同部门的人"——仍需要数据修复)。
|
|
|
|
|
-
|
|
|
|
|
-### L-4. `SetUserPerms` 在 `FindByIds` 校验与 `BatchInsertWithTx` 之间存在 perm 状态 TOCTOU
|
|
|
|
|
-- **位置**:`internal/logic/user/setUserPermsLogic.go:90-130`
|
|
|
|
|
|
|
+ 1. 把 `loadOk=false` 的语义改为"基础设施故障":`Load` 里遇到该分支直接 `return nil, ErrLoaderDegraded`(新定义 sentinel)。
|
|
|
|
|
+ 2. HTTP 中间件 & gRPC AuthInterceptor 对 `ErrLoaderDegraded` 统一返 503(或 `codes.Unavailable`),并附 `retry-after`。
|
|
|
|
|
+ 3. 保留 `(ud, false, nil)` 作为内部观测用途时,**改为 panic-safe 的诊断日志聚合**而非对外返回。
|
|
|
|
|
+ 4. 配套:`refreshTokenLogic` 里的 "产品已被禁用" 分支前增加 `if ud.ProductCode != "" && ud.DeptStatus == 0 { return ErrLoaderDegraded }` 兜底。
|
|
|
|
|
+ 5. 单测:`TC-9101 loadPerms 报错 → Load 返回 Unavailable`、`TC-9102 loadProduct 报错 → Load 不返回"产品已禁用"`。
|
|
|
|
|
+
|
|
|
|
|
+### M-N2. `UserDetailsLoader.BatchDel` 在大 roleId 场景退化为 O(N) Redis RTT(N+1 新发现)
|
|
|
|
|
+- **位置**:`internal/loaders/userDetailsLoader.go:257-272`,调用方 `internal/logic/role/updateRoleLogic.go`、`internal/logic/role/bindRolePermsLogic.go`、`internal/logic/user/bindRolesLogic.go`。
|
|
|
- **描述**:
|
|
- **描述**:
|
|
|
- - L95-109:循环检查 `dbPerms` 里每条权限都 `ProductCode == productCode` 且 `Status == StatusEnabled`;
|
|
|
|
|
- - L112-131:在新事务里 `DeleteByUserIdForProductTx` + `BatchInsertWithTx`。
|
|
|
|
|
- - 两段之间若并发一次 `SyncPermissions` 把某个 permId 的 `status` 置成 DISABLED,本次 `SetUserPerms` 依然会把那条 `user_perm` 落库。
|
|
|
|
|
-- **影响**:`loadPerms` 在组装缓存时是 `JOIN sys_perm WHERE sys_perm.status = ?`(L-4 修过的部分),失效权限不会真正生效。本漏洞**不造成越权**,但会留下一行"脏"`sys_user_perm`,让审计查询 "X 拥有哪些权限" 与 "X 实际享有哪些权限" 出现微小偏差,是数据一致性噪声。
|
|
|
|
|
-- **修复方案**(按实际需要决定是否做):
|
|
|
|
|
- 1. 把 `FindByIds` 挪到事务内,并对 `sys_perm` 这几行加 `FOR SHARE`(`SyncPerms` 已经通过 `LockByCodeTx` 在 sys_product 行上串行化;这里加 `FOR SHARE` 只是为了在事务边界内读到一致的 status,开销很低);
|
|
|
|
|
- 2. 或者最廉价:在 `BatchInsertWithTx` 之后补一条 `COUNT(*)` 校验 —— "我刚才插入的 permId 全部仍是 Enabled",不满足就主动回滚事务。任一方案都能把 TOCTOU 窗口缩到零。
|
|
|
|
|
|
|
+ - `BatchDel` 先用一次 `DelCtx(keys...)` 批量删主 key(OK),但紧跟着 **for-range 调用 `unregisterCacheKey`,每个用户串行 2 次 `SremCtx`**(userIndex + productIndex)。
|
|
|
|
|
+ - 对"角色改名 / 角色禁用 / 批量重绑角色"场景,该角色下绑定的用户数可能上千人(即使"几万部门"不现实,但"单个业务角色跨几千人"在大型中台里常见)。意味着一次普通的 `UpdateRole` 会对 Redis 发出 **2N 次** 串行往返。
|
|
|
|
|
+ - 相较之下,`registerCacheKey` 已经用 `PipelinedCtx` 合并 RTT,表明作者清楚 pipeline 的必要性,`unregisterCacheKey` 却没同步改造。
|
|
|
|
|
+- **影响**:
|
|
|
|
|
+ - 在角色批量维护/权限大盘扫描期对 Redis 连接池形成突刺,尾延迟 P99 明显抬升;极端场景可导致 `UpdateRole` 接口被 go-zero 的 ctx timeout 打断,落入"已 UPDATE DB 但 Clean 缓存失败"的分支(目前该分支仅写 Errorf,不回滚)。
|
|
|
|
|
+ - 当 Redis 集群跨机房时,2N 串行比一次 pipeline 多出 (N-1)×RTT,个位数 ms 的 RTT 在 N=1000 时就是秒级延迟。
|
|
|
|
|
+- **修复方案**:
|
|
|
|
|
+ 1. 把 `unregisterCacheKey` 的 per-user 逻辑合入一次 `PipelinedCtx`:对每个 key 发 `pipe.SRem(userIdxKey, cacheKey)` / `pipe.SRem(productIdxKey, cacheKey)`,一次 Exec。
|
|
|
|
|
+ 2. 更彻底:直接给 `BatchDel` 写一个专用 `batchUnregister(ctx, pairs)`,单 pipeline 内合并所有 userIndex / productIndex 的 SREM + 可选 EXPIRE。
|
|
|
|
|
+ 3. 回归测试:`TC-9201 BatchDel(1000 users) 的 Redis 命令数 ≤ 3 次 pipeline Exec`(用 redis-mock 的 CallCount 断言)。
|
|
|
|
|
|
|
|
----
|
|
|
|
|
|
|
+### M-N3. `RoleDetail` 枚举 oracle:跨产品访问 404 vs 403 可区分(新发现)
|
|
|
|
|
+- **位置**:`internal/logic/role/roleDetailLogic.go:29-58`。
|
|
|
|
|
+- **描述**:
|
|
|
|
|
+ - 现状流程:
|
|
|
|
|
+ 1. 先 `SysRoleModel.FindOne(req.Id)`:找不到返 **404 "角色不存在"**;
|
|
|
|
|
+ 2. 再判 `!IsSuperAdmin && caller.ProductCode != role.ProductCode`:返 **403 "无权访问该产品的数据"**。
|
|
|
|
|
+ - 非超管攻击者遍历 `req.Id`,即可通过响应码精确区分"该 id 不存在" vs "存在于别的产品",从而绘制跨产品的 role id 分布图,为后续定向攻击(社工、横向越权尝试)提供素材。
|
|
|
|
|
+ - 同一脚本在 `ProductDetailLogic.productDetail` 也存在(先 `FindOneByCode`,再做成员检查);`RoleList`/`ProductList` 因天然按 `caller.ProductCode` 过滤不泄漏。
|
|
|
|
|
+- **影响**:
|
|
|
|
|
+ - 边界信息泄露,属于"先查后授权"反模式;在任何以 id 为目标的枚举场景都会被利用。
|
|
|
|
|
+ - 与第 7 轮把 `AdminLogin` 非法用户名的 bcrypt 时序补齐的精神不一致。
|
|
|
|
|
+- **修复方案**:
|
|
|
|
|
+ 1. 合并成"授权失败即返 404"的统一契约:先用 `caller.ProductCode` 做过滤(或查询时把 `productCode = ?` 塞进 WHERE),未命中或跨产品一律返 `ErrNotFound("角色不存在")`。
|
|
|
|
|
+ 2. `ProductDetailLogic` 同步处理:对非超管直接用 `FindByCodeWithMemberCheck` 或先查 `sys_product_member` 命中后再读 `sys_product`,省掉"存在性差异"泄漏。
|
|
|
|
|
+ 3. 单测:`TC-9301 非超管请求别产品 roleId → 404`、`TC-9302 非超管请求不存在 roleId → 404`,两者响应体必须完全一致。
|
|
|
|
|
|
|
|
-## 📋 修复优先级汇总
|
|
|
|
|
|
|
+### M-N4. `CreateUser` 允许产品 ADMIN 为"非自己管辖部门"预埋用户名(新发现)
|
|
|
|
|
+- **位置**:`internal/logic/user/createUserLogic.go:37-100`。
|
|
|
|
|
+- **描述**:
|
|
|
|
|
+ - 现状只用 `RequireProductAdminFor(productCode)` 校验 caller 是该产品的 ADMIN,并用 `FindOne(req.DeptId)` 核对部门存在,**未校验 caller 的 `DeptPath` 是否覆盖 `newDept.Path`**。
|
|
|
|
|
+ - 产品 ADMIN 可以为任意存在的部门创建用户,随后:
|
|
|
|
|
+ 1. 占坑关键用户名(`admin_*`、`ops_*`、`sre_*` 等易被运营/运维复用的账号);
|
|
|
|
|
+ 2. 预埋账号后等待配合方(比如跨部门协作的另一位 ADMIN)触发 `AddMember`,由于 `AddMember` 会走 `CheckAddMemberAccess` 的部门链校验,**对方 ADMIN** 的部门链可能覆盖,最终把这个"伪造种子账号"挂进产品。
|
|
|
|
|
+ - 与 `UpdateUserLogic` 的设计(调部门时严格校验 `caller.DeptPath` 前缀,见 `updateUserLogic.go:116-120`)**不一致**:同样的敏感位,创建时无校验、修改时严校验,出现"先创建后改部门"的绕路。
|
|
|
|
|
+- **影响**:
|
|
|
|
|
+ - 横向越权路径:产品 ADMIN 在"应当只管 X 部门"的治理约束下,可以在 DEV 部门、总部部门等任意位置生成账号,后续借由 AddMember 协同路径落地。
|
|
|
|
|
+ - 用户名命名空间被占用,正常部门 ADMIN 新建时收到"用户名已存在",排查困难。
|
|
|
|
|
+- **修复方案**:
|
|
|
|
|
+ 1. 与 `UpdateUserLogic` 保持对称:`if req.DeptId > 0 && !caller.IsSuperAdmin && caller.MemberType != Admin || caller.DeptPath != "" && !strings.HasPrefix(newDept.Path, caller.DeptPath) { return ErrForbidden }`。
|
|
|
|
|
+ 2. 若业务上允许产品 ADMIN "跨部门拉新人",则至少要求 `req.DeptId` 必须是 caller 所在产品已有成员的部门集合之一(查 `sys_product_member` join `sys_user` 得到部门白名单)。
|
|
|
|
|
+ 3. 单测:`TC-9401 部门 ADMIN 创建跨部门用户 → 403`、`TC-9402 产品 ADMIN 创建本部门子部门用户 → 200`、`TC-9403 SuperAdmin 创建任意部门用户 → 200`。
|
|
|
|
|
|
|
|
-| 优先级 | finding | 一句话概要 |
|
|
|
|
|
-| :----- | :------------------------------------------------------------ | :------------------------------------------------------------------------------------------ |
|
|
|
|
|
-| **P0** | **H-1** UserDetail/UserList PII 暴露(3 轮未落地) | 任意同产品 MEMBER 可读全员手机邮箱备注,违反 PIPL 最小必要 |
|
|
|
|
|
-| **P0** | **H-2** `checkPermLevel` 仍读缓存 MinPermsLevel(M-3 另一半) | 降级 admin 在 5 分钟 TTL 内仍可 RemoveMember / UpdateMember / SetUserPerms |
|
|
|
|
|
-| P1 | **M-1** CreateProduct Redis 写失败 → 产品 + admin 孤儿化 | 一次 Redis 失败 → 永久不可恢复,只能手动 SQL |
|
|
|
|
|
-| P1 | **M-2** SyncPermsError 404 → codes.Internal 契约错位 | 接入方 SDK 的错误分类/重试策略失真 |
|
|
|
|
|
-| P1 | **M-3** RefreshToken CAS 后签名失败 → 用户被强制登出 | tokenVersion 已推进,客户端必须重登,可用性事件 |
|
|
|
|
|
-| P2 | **L-1** `sysPermModel` 仍用 `fmt.Sprintf`(L-4 风格漂移) | 三条 SQL 统一改占位符,接近审计一致性 |
|
|
|
|
|
-| P2 | **L-2** `CountActiveAdminsTx` 零调用(L-5 遗留) | 接口层可见的僵尸方法 |
|
|
|
|
|
-| P2 | **L-3** `CheckManageAccess` 对 `DeptId=0` 老账号 403(L-7) | 需数据迁移或 "看自己" 短路 |
|
|
|
|
|
-| P3 | **L-4** SetUserPerms FindByIds / BatchInsert TOCTOU | 不越权,但会留脏 `sys_user_perm` 行 |
|
|
|
|
|
|
|
+### L-N1. JWT 解析三处重复:`ParseWithHMAC` helper 未统一使用
|
|
|
|
|
+- **位置**:
|
|
|
|
|
+ - 定义:`internal/logic/auth/jwt.go:16-31`;
|
|
|
|
|
+ - 内联实现 1:`internal/middleware/jwtauthMiddleware.go:62-66`;
|
|
|
|
|
+ - 内联实现 2:`internal/server/permserver.go` 的 `VerifyToken`(`jwt.ParseWithClaims(... keyfunc {...})`)。
|
|
|
|
|
+- **描述**:`ParseWithHMAC` 的注释明确写着"所有 JWT 解析点(HTTP 中间件 / gRPC VerifyToken / RefreshToken)统一走这里",但目前只有 `ParseRefreshToken` 一个调用方。另两处自己又写了一遍 `token.Method.(*jwt.SigningMethodHMAC)` 断言 —— 功能上一致,但:
|
|
|
|
|
+ - 未来若增加算法白名单(例如仅允许 HS384、禁 HS256)或添加 `typ` 断言,要改 3 处;
|
|
|
|
|
+ - 把"算法混淆防御"的审计覆盖矩阵从 1 个函数摊到 3 个函数,`test-design.md` 的 TC-0951~0960 只覆盖了 `ParseRefreshToken` 一条路径。
|
|
|
|
|
+- **影响**:安全属性正确,但代码一致性差、改动风险高。属于"长期隐患"。
|
|
|
|
|
+- **修复方案**:
|
|
|
|
|
+ 1. `jwtauthMiddleware.go` 改用 `authHelper.ParseWithHMAC(tokenStr, secret, &UserClaims{})`,移除内联 keyfunc;
|
|
|
|
|
+ 2. gRPC `VerifyToken` 同样替换;
|
|
|
|
|
+ 3. 把 TC-0951~0960 复用到这两条路径(直接 table-driven 调三个入口)。
|
|
|
|
|
+
|
|
|
|
|
+### L-N2. `UpdateUserLogic` 调部门时未校验目标 `dept.Status`
|
|
|
|
|
+- **位置**:`internal/logic/user/updateUserLogic.go:110-131`。
|
|
|
|
|
+- **描述**:`FindOne(*req.DeptId)` 仅判存在,不判 `Status`。产品 ADMIN 可把用户调入已 `Disabled` 的部门,随后该用户在 `loadPerms` 里会命中"普通成员 + DeptStatus!=Enabled"分支,其 DEV 全权特权被撤销。若业务意图是"停用部门=冻结该部门所有活动",该路径破坏了不变量。
|
|
|
|
|
+- **影响**:中低优。主要是产品语义一致性("停用部门"的含义被稀释),不构成越权。
|
|
|
|
|
+- **修复方案**:`if newDept.Status != StatusEnabled { return ErrBadRequest("目标部门已停用") }`,与 `UpdateDept` 禁用时的数据流闭环。
|
|
|
|
|
+
|
|
|
|
|
+### L-N3. `AdminLogin` `IsSuperAdmin` 判断在 bcrypt 之后,对"合法用户名"的时序侧漏略大于"非法用户名"分支
|
|
|
|
|
+- **位置**:`internal/logic/pub/adminLoginLogic.go:55-85`。
|
|
|
|
|
+- **描述**:流程是"查用户 → bcrypt 校验 → 校 `IsSuperAdmin` → 校 status"。对于一个**存在但非超管**的账号,攻击者即便密码随机也会触发 bcrypt 计算(约 60~100ms),与"存在且密码错"分支耗时相近;而"不存在"分支走 `dummyBcryptHash` 也有类似耗时兜底 —— 单凭这点难以区分。但若攻击者能获取大量样本,`IsSuperAdmin` 这一步的耗时(sql 比较)理论上可让"存在但非超管"比"存在是超管且密码错"略快(无 token 签发路径),仍可能形成<10ms 级的统计差。
|
|
|
|
|
+- **影响**:属学术级别时序泄漏,实战价值低。但该入口按第 7 轮审计精神是"高敏感且对抗公网扫描",建议把 `IsSuperAdmin` 判断**前置到 `FindByUsername` 之后、bcrypt 之前**:若非超管,仍走 `dummyBcryptHash` 消耗一次 bcrypt,再返回 403,恒定时序。
|
|
|
|
|
+- **修复方案**:重排为:`FindByUsername → 非超管则用 dummy 计算再统一返 ErrInvalidCredentials → 超管再真 bcrypt → Status 校验`。
|
|
|
|
|
+
|
|
|
|
|
+### L-N4. `sysUserModel.UpdateStatus` 缺乐观锁字段(由上层短路保护,但 model 自身语义不自洽)
|
|
|
|
|
+- **位置**:`internal/model/user/sysUserModel.go:154-175`。
|
|
|
|
|
+- **描述**:`UpdateStatus` 的 `WHERE id=?` 无 `updateTime` / `tokenVersion` 比较;与同文件里 `UpdateProfile` 使用乐观锁、`IncrementTokenVersionIfMatch` 使用 CAS 的风格不一致。上层 `UpdateUserStatusLogic` 做了"状态相同则短路"、`UserDetailsLoader` 5 分钟 TTL 也提供事实一致性,所以实际故障概率低。
|
|
|
|
|
+- **影响**:不会造成越权,但并发改状态会出现"我改了 Enabled 你覆成 Disabled"的 last-write-wins。
|
|
|
|
|
+- **修复方案**:把 `UpdateStatus` 改为 `WHERE id=? AND updateTime=?`,接口加入 `expectedUpdateTime` 参数,语义与 `UpdateProfile` 对齐。
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## 🛠 建议修复次序
|
|
|
|
|
|
|
+## ✅ 本轮复核通过、认定安全的机制(仅挑敏感点列示)
|
|
|
|
|
+
|
|
|
|
|
+| 机制 | 位置 | 关键保护点 |
|
|
|
|
|
+| --- | --- | --- |
|
|
|
|
|
+| `checkPermLevel` fresh read | `logic/auth/access.go:302-347`、`loadFreshMinPermsLevel:339-343` | caller.MinPermsLevel 每次走 DB,降级 admin 后续的跨级分配被立即拒绝(第 8 轮 H-2 已闭环) |
|
|
|
|
|
+| CreateProduct 补偿 | `logic/product/createProductLogic.go:178-283` | Redis 票据写入 `SetexCtx` 失败调用 `compensateCreatedRows` 级联删除 admin / member / product |
|
|
|
|
|
+| RefreshToken CAS 顺序 | `logic/pub/refreshTokenLogic.go:90-110`、`server/permserver.go:215-230` | 先生成 access+refresh 再 `IncrementTokenVersionIfMatch`,签名失败不会吞掉旧 refresh |
|
|
|
|
|
+| DeleteDept AB-BA 防护 | `logic/dept/deleteDeptLogic.go` | 自身 `FOR UPDATE`,子部门 / 成员 `FOR SHARE`,锁顺序与 CreateDept 对齐 |
|
|
|
|
|
+| 负缓存投毒防御 | `loaders/userDetailsLoader.go:161-178` | 写哨兵前再 `FindOne` 强一致复核,避免 Insert→Load 并发把新用户哨兵掉 |
|
|
|
|
|
+| AddMember 部门链二次校验 | `logic/auth/access.go:CheckAddMemberAccess`、`logic/member/addMemberLogic.go:76-78` | 产品 ADMIN 也要过部门链,切断第 6 轮 H-3 |
|
|
|
|
|
+| RemoveMember 末位守卫 | `logic/member/removeMemberLogic.go:49-53` | `CountOtherActiveAdminsTx` 事务内排除自己并 lock row,杜绝并发撤 admin 导致 0 admin |
|
|
|
|
|
+| GuardRoleLevelAssignable | `logic/user/bindRolesLogic.go:86-88` | caller 的 min-level fresh read,避免缓存 admin 权级绑定高于自己的角色 |
|
|
|
|
|
+| SetUserPerms 事务内复核 | `logic/user/setUserPermsLogic.go:148-165` | `BatchInsertWithTx` 前后 `COUNT(*) WHERE status=?` 复核,并发禁用权限的 TOCTOU 闭环 |
|
|
|
|
|
+| gRPC SyncPerms 错误分级 | `server/permserver.go:61-90` | `SyncPermsError{Code:404}` → `codes.NotFound`,不再同化为 Internal |
|
|
|
|
|
+| gRPC 限流剥端口 | `server/permserver.go` / `ratelimit` | PeriodLimit key 用客户端 IP 而非 `host:port`,防单连接多端口绕限 |
|
|
|
|
|
+| JWT HMAC 断言本身 | `logic/auth/jwt.go:16-31` | `alg=none` / RS256 / ES256 全拒;L-N1 指出的是"调用点未统一",算法防御自身 OK |
|
|
|
|
|
|
|
|
-1. **本轮必修 P0**(对称封口):
|
|
|
|
|
- - H-1:`filterPIIForCaller` + `CanViewContact` 两个 helper,落到 `UserDetail` / `UserList` 返回前。
|
|
|
|
|
- - H-2:`checkPermLevel` 把 `caller.MinPermsLevel` 换成 `loadFreshMinPermsLevel(...)`;与 `GuardRoleLevelAssignable` 共享同一个 helper。两条 P0 建议同批上线,一起做回归单测。
|
|
|
|
|
|
|
+---
|
|
|
|
|
|
|
|
-2. **P1**(稳定性与契约):
|
|
|
|
|
- - M-1:CreateProduct 失败路径补偿(删 `sys_product` + `sys_user` + `sys_product_member`)或新增 SuperAdmin-only `RegenerateInitialCredentials`;集成测试强制注入 Redis fault。
|
|
|
|
|
- - M-2:`SyncPermsError{Code:404}` 补到 switch 里,映射到 `codes.NotFound`;抽 `mapSyncPermsErr` 统一 HTTP / gRPC。
|
|
|
|
|
- - M-3:RefreshToken 重排为"先签新 token 成功才做 CAS";或至少给 post-CAS 失败路径加 audit 字段。
|
|
|
|
|
|
|
+## 🎯 修复建议优先级与落地顺序
|
|
|
|
|
|
|
|
-3. **P2 / P3 收尾**:
|
|
|
|
|
- - L-1:`sysPermModel` 三条 SQL 改占位符;
|
|
|
|
|
- - L-2:`CountActiveAdminsTx` 删除(接口 + 实现 + mock),`CountOtherActiveAdminsTx` 保留;
|
|
|
|
|
- - L-3:数据迁移脚本批量把历史 `deptId=0` 账号挪到默认部门,然后 `CleanByUserIds` 刷缓存;
|
|
|
|
|
- - L-4:`SetUserPerms` 要么把 `FindByIds` 挪进事务 + `FOR SHARE`,要么事务末补一条 status 复核 `COUNT(*)`。
|
|
|
|
|
|
|
+| 优先级 | 议题 | 预估工作量 | 风险 |
|
|
|
|
|
+| --- | --- | --- | --- |
|
|
|
|
|
+| P0 | **M-N1 Loader 半成品降级** | 1 天:新增 sentinel 错误 + 中间件映射 + refreshToken 分支修正 + 2 条新单测 | 中:需验证 5xx 监控告警路径 |
|
|
|
|
|
+| P1 | **M-N3 RoleDetail 枚举 oracle** | 半天:`RoleDetail` + `ProductDetail` 合并成"404 或 200" | 低 |
|
|
|
|
|
+| P1 | **M-N4 CreateUser 部门链校验** | 半天:对称复用 `UpdateUserLogic` 的校验代码 | 低 |
|
|
|
|
|
+| P2 | **M-N2 BatchDel pipeline 化** | 半天:改 `unregisterCacheKey` 支持批量 + 回归压测 | 低 |
|
|
|
|
|
+| P2 | **L-N1 ParseWithHMAC 统一** | 半天:替换 2 处调用 + table-driven 用例复用 | 低 |
|
|
|
|
|
+| P3 | **L-N2 UpdateUser 目标部门 Status** | 1 小时 | 低 |
|
|
|
|
|
+| P3 | **L-N3 AdminLogin IsSuperAdmin 前置** | 1 小时 | 低 |
|
|
|
|
|
+| P3 | **L-N4 UpdateStatus 乐观锁** | 2 小时(需调上层一处调用) | 低 |
|
|
|
|
|
+| Backlog | **L-3 `DeptId=0` 历史账号迁移** | 需 DBA 配合 | — |
|
|
|
|
|
|
|
|
---
|
|
---
|
|
|
|
|
|
|
|
-## 🔎 备注:本轮已验证仍在正轨上的机制
|
|
|
|
|
|
|
+## 📎 审计方法论与覆盖率说明
|
|
|
|
|
|
|
|
-下列机制经过完整阅读后认为仍然安全、无需调整,特此备注以免未来误改:
|
|
|
|
|
|
|
+- 本轮审计共读取并分析 27 个 `.go` 文件(不含 `_test.go` / `_gen.go` / `testdata/`),覆盖 logic(全部)、loaders、server、middleware、model 定制层、consts、util;
|
|
|
|
|
+- 复核维度:针对第 8 轮每一条未决项都读取相关 code path 做实际行为确认,避免"报告里写修复实际没改";
|
|
|
|
|
+- 新发现筛选原则:仅列出**能构造具体攻击序列或业务损害场景**的项,纯风格性建议已过滤;
|
|
|
|
|
+- "几万部门"等非现实场景已按 USER 要求过滤;保留"角色下绑千人"这一现实高频场景作为 M-N2 的论据。
|
|
|
|
|
|
|
|
-- `UserDetailsLoader`:H-1(deny fail-close)、M-1(半加载不写缓存 + 中间件按 `(ud, err)` 分流 401/503)、L-3(loadPerms 任一子步骤错误都 fail-close)、L-6(负缓存 TTL=10s + 写前 `FindOne`)全部落地。
|
|
|
|
|
-- `RefreshToken` / `Logout` 走 `IncrementTokenVersionIfMatch` / `IncrementTokenVersion` 两把刀,前者是 CAS(单会话轮转),后者是"无条件大杀器"(强制全量失效),`L-2` 的 WARN 注释已就位。
|
|
|
|
|
-- `DeleteDeptLogic` 的锁序列:`X(target dept)` → `FOR SHARE(children)` → `FOR SHARE(users in dept range)`,AB-BA 死锁风险大幅收敛。
|
|
|
|
|
-- `SyncPermissions` 通过 `LockByCodeTx` 把同 product 的 sync 串行化,sys_perm UNIQUE 再撞 1062 会落一条 `audit=mysql_error_1062` ERROR 日志,足以触发告警。
|
|
|
|
|
-- `LoginLogic` / `AdminLoginLogic` / `UserInfoLogic` 里的 `Email` / `Phone` 返回属于"看自己"合法场景,本轮不纳入 H-1 修复范围。
|
|
|
|
|
-- `ExtractClientIP` 对 `X-Forwarded-For` / `X-Real-IP` 做了严格解析 + `firstValidIP` 过滤,gRPC 侧 `net.SplitHostPort` 剥端口(M-7 已修)。
|
|
|
|
|
-- `CheckAddMemberAccess` 对 ADMIN 也强制走部门链校验 + 拒绝 `target.IsSuperAdmin`,H-3 入口已封住。
|
|
|