|
|
@@ -1,506 +1,510 @@
|
|
|
-# 深度审计报告 · Round 16
|
|
|
+# 深度审计报告 · Round 17
|
|
|
|
|
|
-> 基线:R15 审计后的代码库快照。R15 提出的 1 条 Medium(M-R15-1,缓存 TTL 下的 MemberType TOCTOU)与 3 条 Low(L-R15-1/2/3)均已在代码中落地(`updateMemberLogic.go` / `updateProductLogic.go` 在 tx 内补齐 `tokenVersion+1` 降权吊销;`updateUserLogic.go` 的 `deptId=0` 收敛给 SuperAdmin;`deptTreeLogic.go` 的 `fullAccess` 收敛给 SuperAdmin)。本轮聚焦两类 R15 未闭合的面:
|
|
|
+> 基线:R16 审计后的代码库快照。R16 提出的 1 条 High(H-R16-1,`RemoveMember` 未在 tx 内吊销会话)、1 条 Medium(M-R16-1,`UserList`/`UserDetail` 未对普通 MEMBER 脱敏 PII)与 2 条 Low(L-R16-1/2)均已在代码中落地:`removeMemberLogic.go` 在 tx 体里显式 `IncrementTokenVersionWithTx` 并在 post-commit 清 sysUser 低层缓存;`userListLogic.go` / `userDetailLogic.go` 新增 `maskUserPII` 对 MEMBER 遮蔽 email/phone/remark;`updateUserLogic.go` 去掉了 ADMIN 旁路 DeptPath 前缀校验,并补上"DEV → NORMAL 跨域调动"时的 tokenVersion 递增;`updateDeptLogic.go` 在"DEV→NORMAL / Enabled→Disabled"这类权限收窄迁移时对整棵子树的 user 做 `BatchIncrementTokenVersionWithTx`。R15/R14/R13/R12/R11/R10 的既往修复经复检全部稳定。
|
|
|
>
|
|
|
-> 1. **降权吊销的覆盖对称性**:`UpdateMember` / `UpdateProduct` 已走"降权即 tokenVersion+1"的闭环,但与它们语义同构的**其它降权路径**(`RemoveMember`、`UpdateDept` 改 DeptType/Status、`UpdateUser` 改 deptId 跨越 DEV/NORMAL 边界)仍是"仅 post-commit `UserDetailsLoader.*` 尽力而为失效"的旧口径——Redis 抖动时 5min TTL 窗口内旧权限继续生效,构成与 M-R15-1 同等的 TOCTOU 面;
|
|
|
-> 2. **全局字段 `sys_user.deptId` 的跨产品结构性副作用**:R15 的 L-R15-1 只封了 `deptId=0` 这条极端路径,但**产品 ADMIN 把共享成员调入自己部门子树外的非 DEV 部门**同样会改写全局 `sys_user.deptId`,在其它产品的管理视角里制造"同产品 ADMIN 够不到、DeptTree 里显示错位"的异常状态;
|
|
|
-> 3. **PII 最小授权**:`UserList` / `UserDetail` 两接口只做"同产品成员"校验即返回目标的 `email` / `phone`,没有按 MemberType 收敛,普通 MEMBER 成员可枚举本产品全部成员的联系方式。
|
|
|
+> 本轮聚焦四块 R16 未覆盖的面:
|
|
|
+>
|
|
|
+> 1. **跨对象事务闭环的外部边**:`UpdateDept` / `DeleteDept` 的 sys_dept X 锁只能串行化"改部门 / 删部门"路径;但 `CreateUser` / `CreateProductLogic` 的 auto-provision 用户行**完全不走事务、不对 sys_dept 取锁**,留下"DeleteDept 已完成 → 新 user 仍以该 deptId 落盘"的 orphan-row 窗口;同类结构性问题还影响 `UpdateUser` 的 `*req.DeptId == user.DeptId` 分支与 `AddMember` 对目标 `sys_user.Status` 的读后写。
|
|
|
+> 2. **缓存失效与事务提交的时序错位**:go-zero `sqlc.CachedConn.ExecCtx` 的"exec → DelCache"顺序对普通 `sql.DB` 是正确的,但在 `session.ExecCtx` 语义下,DelCache 发生在外层 tx **commit 之前**;`DeleteDept` / `DeleteRole` / `UpdateDept` 等走 `*WithTx` 的路径会在这个窗口内被并发 `FindOne` 把**旧行**回填进缓存,随后 commit,留下最长 5min 的"幽灵快照"——尤其对 `sys_dept` 这种 `UserDetailsLoader.loadDept` 直接消费行内容的对象具有权限语义。
|
|
|
+> 3. **"全权用户"对 SyncPerms 新增 perm 的感知延迟**:`SyncPermsService` 的 L-R11-4 优化(pure-add 不清 `UserDetailsLoader.CleanByProduct`)基于"loadPerms 对任何 user 的结果都不会因为新增 perm 变化"的假设,但 `loadPerms` 对 SuperAdmin / ADMIN / DEVELOPER / DEV-dept 启用成员走的是 `FindAllCodesByProductCode`——新增 perm **会**立刻改变这条分支的返回集,缓存没清等价于最长 5min TTL 的权限延迟。
|
|
|
+> 4. **权限级别纵向防护的对称性**:`UpdateRoleLogic` 有 L-R12-3 `!caller.IsSuperAdmin && req.PermsLevel < role.PermsLevel → 403` 防提权;但 `CreateRoleLogic` 不做任何 caller permsLevel 与 `req.PermsLevel` 的比较,让 product ADMIN 直接新建一个 permsLevel=1 的角色后 BindRoles 给下属,绕过 `GuardRoleLevelAssignable` 对"同级"的拦截,横向等价于"给 DEVELOPER 签发一张 ADMIN 入场券"。
|
|
|
|
|
|
---
|
|
|
|
|
|
## 🚩 核心逻辑漏洞 (High Risk)
|
|
|
|
|
|
-### H-R16-1 · `RemoveMember` 降权路径未随 M-R15-1 同步吊销会话 —— 与 `UpdateMember` 对称缺口
|
|
|
+### H-R17-1 · `CreateUser` / `CreateProductLogic` 对目标 `sys_dept` 无事务锁,与 `DeleteDept` 存在 TOCTOU 竞态(orphan user + 幽灵部门缓存 权限升级)
|
|
|
|
|
|
**位置**
|
|
|
|
|
|
-- `internal/logic/member/removeMemberLogic.go:42-74`(事务体只做 `DeleteByUserId* + Delete`,无 `IncrementTokenVersionWithTx`)
|
|
|
-- 对比参照:`internal/logic/member/updateMemberLogic.go:94-134`("降权即 `tokenVersion+1`"已落地)
|
|
|
+- `internal/logic/user/createUserLogic.go:86-140`(`FindOne(deptId)` 是 cached 读,随后 `Insert(sys_user)` 不在任何事务中,`sys_dept` 无 S 锁)
|
|
|
+- `internal/logic/product/createProductLogic.go`(为新产品自动注入 `admin_<productCode>` 行时同样走非 tx `SysUserModel.Insert`)
|
|
|
+- 对比参照:`internal/logic/dept/deleteDeptLogic.go:44-68`(已经用 `SELECT id FROM sys_dept WHERE id=? FOR UPDATE` + `SELECT id FROM sys_user WHERE deptId=? FOR SHARE` 做"删除 → 检查既有用户"的串行化)
|
|
|
|
|
|
**描述**
|
|
|
|
|
|
-M-R15-1 / L-R15-3 落地后,"降权/禁用"这类"从'有效成员'向'无效成员'迁移"的路径都在 tx 内把目标的 `sys_user.tokenVersion` 做 `+1`,让旧 access token 在 `jwtauthMiddleware` 的 `claims.TokenVersion != ud.TokenVersion` 兜底下立刻 401,即使 Redis `Del`/`Clean` 失败也不会残留特权。但这条口径**漏掉了 `RemoveMember`**:
|
|
|
+`DeleteDept` 已经通过 X 锁 + `FOR SHARE` 锁把"既有用户占着这个部门就不让删"编织进事务:
|
|
|
|
|
|
-```42:74:internal/logic/member/removeMemberLogic.go
|
|
|
-if err := l.svcCtx.SysProductMemberModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
|
|
|
- locked, err := l.svcCtx.SysProductMemberModel.FindOneForUpdateTx(ctx, session, req.Id)
|
|
|
- if err != nil {
|
|
|
- return response.ErrNotFound("成员不存在")
|
|
|
- }
|
|
|
- if locked.MemberType == consts.MemberTypeAdmin && locked.Status == consts.StatusEnabled {
|
|
|
- otherAdminCount, err := l.svcCtx.SysProductMemberModel.CountOtherActiveAdminsTx(ctx, session, member.ProductCode, locked.Id)
|
|
|
- if err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- if otherAdminCount == 0 {
|
|
|
- return response.ErrBadRequest("不能移除该产品的最后一个管理员")
|
|
|
- }
|
|
|
- }
|
|
|
- if err := l.svcCtx.SysUserRoleModel.DeleteByUserIdForProductTx(ctx, session, member.UserId, member.ProductCode); err != nil {
|
|
|
- return err
|
|
|
+```44:68:internal/logic/dept/deleteDeptLogic.go
|
|
|
+var deptId int64
|
|
|
+lockQuery := fmt.Sprintf("SELECT `id` FROM %s WHERE `id` = ? FOR UPDATE", l.svcCtx.SysDeptModel.TableName())
|
|
|
+if err := session.QueryRowCtx(ctx, &deptId, lockQuery, req.Id); err != nil {
|
|
|
+ return response.ErrNotFound("部门不存在")
|
|
|
+}
|
|
|
+// ... 略 ...
|
|
|
+var userIds []int64
|
|
|
+userQuery := fmt.Sprintf("SELECT `id` FROM %s WHERE `deptId` = ? FOR SHARE", l.svcCtx.SysUserModel.TableName())
|
|
|
+if err := session.QueryRowsCtx(ctx, &userIds, userQuery, req.Id); err != nil {
|
|
|
+ return err
|
|
|
+}
|
|
|
+if len(userIds) > 0 {
|
|
|
+ return response.ErrBadRequest("该部门下仍有关联用户,无法删除")
|
|
|
+}
|
|
|
+return l.svcCtx.SysDeptModel.DeleteWithTx(ctx, session, req.Id)
|
|
|
+```
|
|
|
+
|
|
|
+但这个串行化**只对"既有用户 vs 删除"有效**:`CreateUser` 这条"未来用户 vs 删除"链路完全游离事务外——
|
|
|
+
|
|
|
+```86-140:internal/logic/user/createUserLogic.go
|
|
|
+if req.DeptId > 0 {
|
|
|
+ newDept, derr := l.svcCtx.SysDeptModel.FindOne(l.ctx, req.DeptId) // 命中 sqlc 低层缓存 / stale 读
|
|
|
+ if derr != nil {
|
|
|
+ return nil, response.ErrBadRequest("部门不存在")
|
|
|
}
|
|
|
- if err := l.svcCtx.SysUserPermModel.DeleteByUserIdForProductTx(ctx, session, member.UserId, member.ProductCode); err != nil {
|
|
|
- return err
|
|
|
+ if newDept.Status != consts.StatusEnabled {
|
|
|
+ return nil, response.ErrBadRequest("目标部门已停用")
|
|
|
}
|
|
|
- return l.svcCtx.SysProductMemberModel.DeleteWithTx(ctx, session, req.Id)
|
|
|
-}); err != nil {
|
|
|
- return err
|
|
|
+ // DeptPath 校验 ...
|
|
|
}
|
|
|
|
|
|
-cleanCtx, cancel := loaders.DetachCacheCleanCtx(l.ctx)
|
|
|
-defer cancel()
|
|
|
-l.svcCtx.UserDetailsLoader.Del(cleanCtx, member.UserId, member.ProductCode)
|
|
|
-return nil
|
|
|
+hashedPwd, err := bcrypt.GenerateFromPassword([]byte(req.Password), bcrypt.DefaultCost)
|
|
|
+// ... 纯非事务 Insert,无任何 sys_dept 锁参与 ...
|
|
|
+result, err := l.svcCtx.SysUserModel.Insert(l.ctx, &userModel.SysUser{
|
|
|
+ DeptId: req.DeptId,
|
|
|
+ // ...
|
|
|
+})
|
|
|
```
|
|
|
|
|
|
-**`RemoveMember` 事实上是 `UpdateMember` 降权路径的极端版**——不是"ADMIN→MEMBER"而是"ADMIN→`MemberType=""`(非成员)",造成的授权语义跳变更剧烈:
|
|
|
+竞态时序(T1 = SuperAdmin 删部门,T2 = ADMIN 创建用户):
|
|
|
|
|
|
-- `loadPerms` 对"非成员"会返回 `nil` → 原本的 ADMIN 全权直接清空;
|
|
|
-- `jwtauthMiddleware` 对 `ud.MemberType == ""` 非超管会 403 `"您已不是该产品的有效成员"`;
|
|
|
-- `CheckManageAccess` 的 ADMIN 分支直接跳过 `checkDeptHierarchy`,对成员的管理面彻底开放。
|
|
|
+1. T2 执行 `FindOne(X)`,命中 sqlc 低层缓存或 DB 非锁读,拿到 `dept X{status=Enabled}`;DeptPath / DeptType / Status 所有校验通过。
|
|
|
+2. T1 开 tx → `SELECT ... FOR UPDATE` 锁住 `sys_dept[X]`。
|
|
|
+3. T1 `SELECT id FROM sys_user WHERE deptId=X FOR SHARE`:T2 **尚未插入**(Insert 发生在 bcrypt 之后,bcrypt default cost 约 60–100ms),返回空集。
|
|
|
+4. T1 `DELETE FROM sys_dept WHERE id=X`,commit,释放 X 锁。go-zero 的 sqlc `DelCache(cacheSysDeptIdPrefix+X)` 在 commit 之前已经执行过(见 H-R17-2)。
|
|
|
+5. T2 `bcrypt.GenerateFromPassword` 完成,`Insert sys_user` 落库,`deptId=X`。该 INSERT **不触及 sys_dept**(表级无外键、无触发器),DB 不会阻止。
|
|
|
+6. 产生 orphan 用户:`sys_user.deptId=X`,`sys_dept.id=X` 不存在。
|
|
|
|
|
|
-攻击场景(与 M-R15-1 的描述完全同构,只是触发点从 `UpdateMember` 换成 `RemoveMember`):
|
|
|
+**紧接着的二次放大**:T2 触发缓存层的"幽灵部门"后果——
|
|
|
|
|
|
-1. SuperAdmin 发起 `RemoveMember` 把 P1 的 ADMIN `A` 移出产品;tx 成功,`UserDetailsLoader.Del(A.UserId, P1)` 被调用;
|
|
|
-2. Redis 在这 3s 内(`DetachCacheCleanCtx` 的超时)出现网络抖动,`DelCtx` 返回 err(代码已通过 `logCacheInvalidationErr` 打 `cache_invalidation_skipped_*` 标签,但**不重试、不 block 请求**,继续返回 200);
|
|
|
-3. 缓存里仍是 `{MemberType: ADMIN, Perms: [...P1 全权], TokenVersion: 旧值}`,TTL 还剩最长 5 分钟;
|
|
|
-4. A 拿着手里的 access token 继续调:
|
|
|
- - `jwtauthMiddleware` 命中 Redis 旧 UD → 放行;
|
|
|
- - `UpdateMember` / `BindRoles` / `UpdateRole` / `DeleteRole` / `SetUserPerms` 等所有产品管理接口的 `RequireProductAdminFor` 都看 `caller.MemberType == ADMIN` 放行;
|
|
|
- - `CheckManageAccess` 走 ADMIN 分支跳过部门链校验,A 可以对 P1 任意成员继续降级、改 PermsLevel、授高权限角色;
|
|
|
-5. 5 分钟 TTL 过期后缓存自然重建,A 才被系统真正视作"非成员"。
|
|
|
+- **良性分支**:`UserDetailsLoader.loadDept` 下次 miss 时 `SysDeptModel.FindOne(X) → ErrNotFound`,`loadOk=false` → `ErrLoaderDegraded` → jwtauthMiddleware 503,该用户**永远无法登录**直到运维手动 data fix。这是高可用事故,但不是权限事故。
|
|
|
+- **恶性分支**:如果 sys_dept 低层缓存里还残留着 X 的旧行(H-R17-2 描述的"commit 后幽灵快照",或者任何并发 `FindOne` 在 T1 的 `DelCache` 与 commit 之间把旧行回填),`loadDept` 成功返回旧 `{Path: "/1/5/", DeptType: DEV, Status: Enabled}`。用户携带这条"已死部门"登录成功,并且:
|
|
|
+ - `DeptPath = "/1/5/"` 继续被 `checkDeptHierarchy` / `CheckAddMemberAccess` 当作真实部门链判据;
|
|
|
+ - 若残留快照是 DEV 部门,任意 `loadPerms` 的产品将对该用户走"DEV 部门全权"分支(`FindAllCodesByProductCode`),**凭空获得任何产品的全部权限**,且完全不在组织架构视图中(`DeptTree` 查询时已无该 dept 行)。
|
|
|
|
|
|
-更严重的是:**此时 A 的 `tokenVersion` 未被递增**,即便运维事后发现 Redis 抖动手动 `Del` 了缓存 key,只要 A 把 access token 保存好,下一次 Load 重建的 UD 仍然是"DB 视角下的非成员"——这是良性的(会 403)。但在 TTL 滞留窗口里 A 的 access token 本身是有效凭据(签名、类型、过期时间、`TokenVersion` 与 `ud.TokenVersion` 都匹配),`jwtauthMiddleware` 无法把这段残留权限踢下线。
|
|
|
+`CreateProductLogic` 同构受害:auto-provision 的 initial admin 行同样走 `SysUserModel.Insert` 而不是 `InsertWithTx`,即使调用方已经是 SuperAdmin 也一样会踩这个坑(SuperAdmin 删部门 + 另一个 SuperAdmin 同时创产品,两个 P0 管理员并发一点不罕见)。
|
|
|
|
|
|
**影响**
|
|
|
|
|
|
-- 与 M-R15-1 等级相同的权限升级 TOCTOU:被移除的 ADMIN / DEVELOPER 在 Redis 抖动时保留完整产品管理权 ≤5min;
|
|
|
-- 运维侧无任何手段**强制**下线(缓存 TTL 过期是唯一收敛机制);
|
|
|
-- 组合攻击面:如果 5min 内 A 利用残留的 ADMIN 权把自己以不同 userId(例如预埋的备用账号)重新 `AddMember` / `BindRoles`,账号回收动作形同虚设——这正是 M-R15-1 修 `UpdateMember` 时就要求的"必须在签发层吊销而不是在缓存层吊销"语义。
|
|
|
+- 业务数据完整性破口:`sys_user.deptId` 丧失"必然引用已存在 sys_dept"的不变式,下游所有基于 deptPath 前缀匹配的鉴权、以及 DeptTree/UserList 的展示聚合都会在事实层面出现不一致;
|
|
|
+- 当缓存幽灵(H-R17-2)叠加上时,orphan 用户可以**无感继承一个已被超管删除的 DEV 部门的全部跨产品权限 ≤5min**——即便被删的 DEV 部门本意是"这批研发被整体调离";
|
|
|
+- 运维侧不容易察觉:orphan 行本身登录 503 看上去是"个别用户异常",与稳定态下的"DB 抖动"几乎无法区分,审计事件不会标记 deptId 指向已死部门;
|
|
|
+- 与 R16 的 L-R16-1(ADMIN 挪人出 DeptPath 外)互补:那一条堵"挪出",这一条堵"新建时就漂在外面"。
|
|
|
|
|
|
**修复方案**
|
|
|
|
|
|
-把 `RemoveMember` 的事务体补齐"降权即 `tokenVersion+1`"的闭环,与 `UpdateMember` 的 M-R15-1 口径完全对齐。`RemoveMember` 的语义比 `UpdateMember` 更清晰——只要走到事务体,就一定构成"从有效成员(或 ADMIN/DEVELOPER)→ 非成员"的降权,**无需再判定是否为"降权"**,无条件递增即可:
|
|
|
+把 `CreateUser` / `CreateProductLogic` 的用户建行动作移进同一个事务,并在事务内用 `FindOneForShareTx` 对目标 `sys_dept` 取 S 锁:
|
|
|
|
|
|
```go
|
|
|
-// internal/logic/member/removeMemberLogic.go
|
|
|
-if err := l.svcCtx.SysProductMemberModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
|
|
|
- locked, err := l.svcCtx.SysProductMemberModel.FindOneForUpdateTx(ctx, session, req.Id)
|
|
|
- if err != nil {
|
|
|
- return response.ErrNotFound("成员不存在")
|
|
|
- }
|
|
|
- if locked.MemberType == consts.MemberTypeAdmin && locked.Status == consts.StatusEnabled {
|
|
|
- otherAdminCount, err := l.svcCtx.SysProductMemberModel.CountOtherActiveAdminsTx(ctx, session, member.ProductCode, locked.Id)
|
|
|
- if err != nil {
|
|
|
- return err
|
|
|
+// CreateUser 改写示意
|
|
|
+var id int64
|
|
|
+err = l.svcCtx.SysUserModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
|
|
|
+ if req.DeptId > 0 {
|
|
|
+ newDept, derr := l.svcCtx.SysDeptModel.FindOneForShareTx(ctx, session, req.DeptId)
|
|
|
+ if derr != nil {
|
|
|
+ if errors.Is(derr, sqlx.ErrNotFound) {
|
|
|
+ return response.ErrBadRequest("部门不存在或已删除")
|
|
|
+ }
|
|
|
+ return derr
|
|
|
}
|
|
|
- if otherAdminCount == 0 {
|
|
|
- return response.ErrBadRequest("不能移除该产品的最后一个管理员")
|
|
|
+ if newDept.Status != consts.StatusEnabled {
|
|
|
+ return response.ErrBadRequest("目标部门已停用")
|
|
|
+ }
|
|
|
+ if newDept.DeptType == consts.DeptTypeDev && !caller.IsSuperAdmin {
|
|
|
+ return response.ErrForbidden("仅超级管理员可将用户调入研发部门")
|
|
|
+ }
|
|
|
+ if !caller.IsSuperAdmin && !strings.HasPrefix(newDept.Path, caller.DeptPath) {
|
|
|
+ return response.ErrForbidden("无权在非自己管辖的部门下创建用户")
|
|
|
}
|
|
|
}
|
|
|
- if err := l.svcCtx.SysUserRoleModel.DeleteByUserIdForProductTx(ctx, session, member.UserId, member.ProductCode); err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- if err := l.svcCtx.SysUserPermModel.DeleteByUserIdForProductTx(ctx, session, member.UserId, member.ProductCode); err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- // 审计 H-R16-1:移除成员是"有效成员→非成员"的极端降权,签发层吊销与 M-R15-1 / L-R15-3 对齐:
|
|
|
- // - IncrementTokenVersionWithTx 放在 DeleteWithTx 之前,任一步失败整体回滚,不会出现
|
|
|
- // "已 +1 但成员行没删" 或 "成员行已删但 tokenVersion 没 +1" 的脏中间态。
|
|
|
- // - 不再把吊销绑定到 UserDetailsLoader.Del 的 Redis 可用性——即使 Del 失败,
|
|
|
- // jwtauthMiddleware 的 claims.TokenVersion != ud.TokenVersion 兜底仍然会 401。
|
|
|
- if _, err := l.svcCtx.SysUserModel.IncrementTokenVersionWithTx(ctx, session, member.UserId); err != nil {
|
|
|
- return err
|
|
|
+ // bcrypt 的慢计算放到事务外(见下一行),事务内只做锁 + Insert
|
|
|
+ result, ierr := l.svcCtx.SysUserModel.InsertWithTx(ctx, session, &userModel.SysUser{ /* 带 hashedPwd */ })
|
|
|
+ if ierr != nil {
|
|
|
+ return ierr
|
|
|
}
|
|
|
- return l.svcCtx.SysProductMemberModel.DeleteWithTx(ctx, session, req.Id)
|
|
|
-}); err != nil {
|
|
|
- return err
|
|
|
-}
|
|
|
+ id, _ = result.LastInsertId()
|
|
|
+ return nil
|
|
|
+})
|
|
|
+```
|
|
|
+
|
|
|
+两点注意:
|
|
|
+
|
|
|
+1. `bcrypt.GenerateFromPassword` 必须在**事务外**完成,否则 100ms 的 bcrypt 会把 sys_dept S 锁持有时长拉高,阻塞 `DeleteDept` / `UpdateDept` 的 X 锁竞争。bcrypt 的产物 `hashedPwd` 在事务外算好,tx 内只做锁 + INSERT。
|
|
|
+2. `CreateProductLogic` 的 auto-provision 用户 INSERT 目前跟产品创建是同一 tx 的兄弟事务 / post-commit 步骤(`compensateCreatedRows` 的兜底对象),改造时可以把初始 admin 行拿到产品 tx 里一起锁 `sys_dept[0]`(如果默认 deptId=0 代表"无部门"则不取锁即可;如果是具体 deptId 就必须取 S 锁),避免 compensate 路径复杂化。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### H-R17-2 · `DeleteDept` / `DeleteRole` / `UpdateDept` 等 `*WithTx` 路径的 sqlc `ExecCtx` "exec 先行、DelCache 后行"语义在事务内退化为**提交前清缓存**,引入幽灵快照窗口
|
|
|
|
|
|
-cleanCtx, cancel := loaders.DetachCacheCleanCtx(l.ctx)
|
|
|
-defer cancel()
|
|
|
-// 审计 H-R16-1:tokenVersion 已落库,post-commit 还要把 sysUser 的低层缓存
|
|
|
-// (cacheSysUserIdPrefix / cacheSysUserUsernamePrefix)一并失效——否则 UD loader 下次 miss 时
|
|
|
-// 会从 sysUser 低层缓存里拿到旧 tokenVersion,把刚递增过的值抹回去(与 UpdateMember 同)。
|
|
|
-if user, err := l.svcCtx.SysUserModel.FindOne(cleanCtx, member.UserId); err == nil && user != nil {
|
|
|
- l.svcCtx.SysUserModel.InvalidateProfileCache(cleanCtx, user.Id, user.Username)
|
|
|
-} else if err != nil {
|
|
|
- logx.WithContext(l.ctx).Errorf("RemoveMember post-commit FindOne(%d) failed for token-version cache invalidation: %v", member.UserId, err)
|
|
|
+**位置**
|
|
|
+
|
|
|
+- go-zero `core/stores/sqlc/cachedsql.go::CachedConn.ExecCtx`:`exec(ctx, cc.db) → DelCache(keys...)`,`cc.db` 在 tx 内是 `session`,exec 仅"把语句排进事务日志",DB 可见性要等外层 `TransactCtx` 的 commit;DelCache 却立刻对 Redis 下手。
|
|
|
+- `internal/model/dept/sysDeptModel_gen.go:88-95` `DeleteWithTx` 调用路径
|
|
|
+- `internal/model/dept/sysDeptModel_gen.go:122-134` `BatchDeleteWithTx` 同构
|
|
|
+- `internal/model/role/sysRoleModel_gen.go` `DeleteWithTx` / `UpdateWithTx`(role 侧也踩同一模式)
|
|
|
+- 消费方:`internal/logic/dept/deleteDeptLogic.go:68`、`internal/logic/role/deleteRoleLogic.go`、`internal/logic/dept/updateDeptLogic.go`
|
|
|
+
|
|
|
+**描述**
|
|
|
+
|
|
|
+go-zero `sqlc.CachedConn.ExecCtx` 伪代码:
|
|
|
+
|
|
|
+```go
|
|
|
+func (cc CachedConn) ExecCtx(ctx, exec, keys...) {
|
|
|
+ res, err := exec(ctx, cc.db) // 事务内这里的"ctx + db"实际是 session.ExecCtx
|
|
|
+ if err != nil { return nil, err }
|
|
|
+ if err := cc.DelCacheCtx(ctx, keys...); err != nil { return nil, err }
|
|
|
+ return res, nil
|
|
|
}
|
|
|
-l.svcCtx.UserDetailsLoader.Del(cleanCtx, member.UserId, member.ProductCode)
|
|
|
-return nil
|
|
|
```
|
|
|
|
|
|
-若不希望在事务外再 `FindOne`(Redis 抖动时可能慢 100ms 级别),也可以在事务体内把 `locked.UserId` 与 `username`(通过 tx 内的 `l.svcCtx.SysUserModel.FindOneForShareTx` 或在进入 tx 前的 `member` 查询里预取)通过闭包透传到 post-commit;由于 `RemoveMember` 没有超高 QPS 预期(行政操作),直接走 post-commit `FindOne`(该查询本身带 sqlc 缓存)也足够。
|
|
|
+当 `DeleteWithTx(ctx, session, id)` 被 `TransactCtx` 包住时:
|
|
|
|
|
|
-**回归验证要点**
|
|
|
+1. `session.ExecCtx(DELETE FROM sys_dept WHERE id=?, id)`:SQL 进入事务日志,**其他事务看不到**该行已被删除(RR 隔离级别)。
|
|
|
+2. `DelCacheCtx(cacheSysDeptIdPrefix+id)`:Redis `DEL` 立刻生效,缓存从此刻起为 miss。
|
|
|
+3. 后续 tx 内可能还有多条 SQL 执行……最终外层 `TransactCtx` 走到 commit。
|
|
|
|
|
|
-- tx 体内 `IncrementTokenVersionWithTx` 返回 `ErrUpdateConflict`(竞态:并发 `RemoveMember`/`Logout`/`ChangePassword`)时整体回滚,测试需断言成员行仍存在;
|
|
|
-- Redis 完全不可用场景下,断言被移除的 ADMIN 在下一次 HTTP 请求时被 `jwtauthMiddleware` 401;
|
|
|
-- 与 `UpdateMember` 的测试矩阵对称扩容:ADMIN/DEVELOPER/MEMBER 被移除后,旧 access token 均应被立即拒绝。
|
|
|
+在 (2) 与 commit 之间的窗口里:
|
|
|
|
|
|
----
|
|
|
+- 任意并发事务 T3 调用 `SysDeptModel.FindOne(id)` → 缓存 miss → `SELECT * FROM sys_dept WHERE id=?` → **读到旧行**(RR 下 T3 看不到 T1 未提交的 DELETE)→ 写回 Redis 覆盖 `DelCache` 的空槽。
|
|
|
+- T1 commit 之后,`sys_dept[id]` 真正消失,但 Redis 里仍有 T3 刚写回的旧行,生存期至少到本 key 的 TTL(默认 `sqlc.WithExpiry` 通常是小时级)。
|
|
|
+
|
|
|
+**影响**
|
|
|
+
|
|
|
+1. **`DeleteDept` 路径**:`sys_dept` 在删除后仍残留 cached 旧行,长达 TTL。任何下游 `UserDetailsLoader.loadDept` 都会拿到这条旧行:
|
|
|
+ - 组合 H-R17-1 的 orphan 用户:新用户凭"幽灵部门"拿到完整 DeptPath,等价于**未被剔除**的部门子树继续承担权限载体。
|
|
|
+ - 即使没有新 orphan 用户,已被调离的遗留用户(管理员把 deptId 改到别的 dept 之后删掉了原 dept)如果刚好在窗口内触发 UD cache miss,还是会按"原部门 DeptType=DEV"拿到全权,直到 TTL 到期。
|
|
|
+2. **`DeleteRole` 路径**:`sys_role[id]` cached 幽灵角色的消费面较窄——`ResolveOwnRoleOr404` 拿到后会走 `RequireProductAdminFor` 或后续 UPDATE/DELETE 的 CAS,commit 后真正撞不到行即回 `ErrUpdateConflict` → 409。不逃逸权限,但让删除后的后续操作出现"魂在但肉体已毁"的 409,增加排障负担。
|
|
|
+3. **`UpdateDept` 路径**:`UpdateDept` 当前在 tx 内通过 `UpdateWithTx`(同样是 `ExecCtx` + DelCache)改字段;commit 前的 DelCache → 并发 FindOne 回填旧值 → commit 后幽灵快照。虽然 `updateDeptLogic.go` 在 post-commit 又调了一次 `BatchIncrementTokenVersionWithTx` + `UserDetailsLoader.CleanByUserIds`,但**没有针对 `sys_dept[id]` 低层缓存本身再做一次 post-commit 失效**,一旦前面的窗口发生,该 dept 的其他非当前用户路径(如 DeptTree、CreateUser、AddMember 的校验读)都会在 TTL 窗口内看到"DEV→NORMAL 之前"的 DeptType,等价于 L-R16-2 想堵的"权限收窄不立刻生效"。
|
|
|
|
|
|
-## ⚠️ 健壮性与性能建议 (Medium / Low)
|
|
|
+**修复方案**
|
|
|
+
|
|
|
+这是 go-zero 在事务语义下公认的坑,本项目已经在 `UpdateProductLogic` / `UpdateUserLogic` / `RemoveMemberLogic` 等路径上用过正确姿势——**tx 成功 commit 之后再补一次 post-commit DelCache**,不要把幂等性押在 sqlc 的内嵌 `ExecCtx` 钩子上。具体:
|
|
|
+
|
|
|
+1. 给 `sys_dept` / `sys_role` 模型暴露 `InvalidateDeptCache(ctx, id)` / `InvalidateRoleCache(ctx, id)`(参考既有 `InvalidateProductCache` 的签名:id-based 三键一并清);
|
|
|
+2. `DeleteDept` / `UpdateDept` / `DeleteRole` / `UpdateRole` 在 `TransactCtx` 返回 nil 后,走 `loaders.DetachCacheCleanCtx` 再调一次该失效方法;
|
|
|
+3. 对 `UpdateDept` 当前"dept.Status / dept.DeptType 权限收窄"的支路同样补一次 post-commit `InvalidateDeptCache`,把 L-R16-2 的修复真正合拢——否则"user tokenVersion 已 bump 但 dept 缓存里还是旧 DeptType"意味着下次 UD 重建时仍会按旧 DeptType 走"DEV 全权"分支,现在的 BatchIncrementTokenVersionWithTx 相当于白做。
|
|
|
+
|
|
|
+长期方案:自研 `DeleteWithTx` / `UpdateWithTx` 包装层,把 `DelCache` 从 `ExecCtx` 里摘出来,只在事务成功 commit 后执行(`TransactCtx` 里注册 `OnCommit` 钩子)。这样所有 `*WithTx` 路径一次性脱离本陷阱,避免逐个调用点打补丁。
|
|
|
+
|
|
|
+---
|
|
|
|
|
|
-### M-R16-1 · `UserList` / `UserDetail` 对任意同产品成员暴露 `email` / `phone` —— PII 最小授权缺失
|
|
|
+### H-R17-3 · `CreateRoleLogic` 缺失 caller permsLevel 与 `req.PermsLevel` 的纵向对称校验,product ADMIN 可借"建新角色 + BindRoles"实现下属纵向提权
|
|
|
|
|
|
**位置**
|
|
|
|
|
|
-- `internal/logic/user/userListLogic.go:38-90`(仅做"同产品"校验,无 MemberType 收敛,`email`/`phone` 直接回落到所有可见成员)
|
|
|
-- `internal/logic/user/userDetailLogic.go:34-76`(仅做"同产品成员"校验,`email`/`phone` 同样全量返回)
|
|
|
+- `internal/logic/role/createRoleLogic.go:33-76`(`PermsLevel` 只做 1–999 的合法性检查,不比 caller 的 assignable level)
|
|
|
+- 对比参照:`internal/logic/role/updateRoleLogic.go` L-R12-3 分支(`!caller.IsSuperAdmin && req.PermsLevel < role.PermsLevel` 明确 403)
|
|
|
|
|
|
**描述**
|
|
|
|
|
|
-R14 / R15 已经把"同产品 ADMIN 可以管理同产品成员"的边界拉紧,但"同产品 MEMBER **读**其他成员信息"的边界还停留在"只要是同产品成员即可":
|
|
|
+项目"纵向权限防护"的核心不变式在 `auth/access.go::GuardRoleLevelAssignable` / `CheckRoleLevelAgainst`:
|
|
|
|
|
|
-```38:45:internal/logic/user/userListLogic.go
|
|
|
-if !caller.IsSuperAdmin {
|
|
|
- if req.ProductCode == "" {
|
|
|
- return nil, response.ErrForbidden("非超管用户必须指定产品编码")
|
|
|
- }
|
|
|
- if caller.ProductCode != req.ProductCode {
|
|
|
- return nil, response.ErrForbidden("无权访问该产品的数据")
|
|
|
- }
|
|
|
+```237:240:internal/logic/auth/access.go
|
|
|
+if rolePermsLevel <= snap.Level {
|
|
|
+ return response.ErrForbidden("不能分配权限级别高于自身的角色(含同级)")
|
|
|
}
|
|
|
```
|
|
|
|
|
|
-```34:41:internal/logic/user/userDetailLogic.go
|
|
|
-if !caller.IsSuperAdmin {
|
|
|
- if caller.ProductCode == "" {
|
|
|
- return nil, response.ErrForbidden("会话缺少产品上下文")
|
|
|
- }
|
|
|
- if _, err := l.svcCtx.SysProductMemberModel.FindOneByProductCodeUserId(l.ctx, caller.ProductCode, req.Id); err != nil {
|
|
|
- return nil, response.ErrForbidden("无权查看非本产品成员的用户信息")
|
|
|
- }
|
|
|
+`UpdateRoleLogic` 在 R12 那轮也补齐了"非超管不能把既有角色往上调档"的防护。但 `CreateRoleLogic` 的 `PermsLevel` 校验只做了字面校验:
|
|
|
+
|
|
|
+```53-55:internal/logic/role/createRoleLogic.go
|
|
|
+if req.PermsLevel < 1 || req.PermsLevel > 999 {
|
|
|
+ return nil, response.ErrBadRequest("权限级别必须在 1-999 之间")
|
|
|
}
|
|
|
```
|
|
|
|
|
|
-两处响应都**无差别地**把 `u.Email` / `u.Phone` / `u.Remark` / `u.DeptId` 填到响应:
|
|
|
-
|
|
|
-```77:90:internal/logic/user/userListLogic.go
|
|
|
-items = append(items, types.UserItem{
|
|
|
- Id: u.Id,
|
|
|
- Username: u.Username,
|
|
|
- Nickname: u.Nickname,
|
|
|
- Avatar: avatar,
|
|
|
- Email: u.Email,
|
|
|
- Phone: u.Phone,
|
|
|
- Remark: u.Remark,
|
|
|
- DeptId: u.DeptId,
|
|
|
- Status: u.Status,
|
|
|
- MemberType: memberType,
|
|
|
- CreateTime: u.CreateTime,
|
|
|
-})
|
|
|
-```
|
|
|
+product ADMIN 可以走如下两步实现"把下属拉到与自己相同乃至更高的 assignable level":
|
|
|
|
|
|
-对比 `MemberListLogic` 的返回(只含 `Username` / `Nickname` / `MemberType` / `Status`,**无** `email` / `phone`)可以看到:业务上 MEMBER 浏览成员列表的合理需要是"看到谁在这个产品里",不是"拿到所有同事的联系方式"。当前 `UserList` / `UserDetail` 无视 caller 的 MemberType,让 P1 的普通 MEMBER 能一键分页导出 P1 全体成员的 `email` / `phone`:
|
|
|
+1. `CreateRole(ProductCode=P, PermsLevel=1)` → 新建 `R_super`,通过,因为 `RequireProductAdminFor` 放行 product ADMIN,`PermsLevel=1` 只看字面范围;
|
|
|
+2. `BindRoles(userId=D, roleIds=[R_super])` → `GuardRoleLevelAssignable` 判断 caller 的 assignable snapshot,`HasFullProductPerms(product ADMIN)` 返回 `true` → 直接放行(见 `access.go::CheckRoleLevelAgainst`);
|
|
|
+3. 下属 D 从此持有 `sys_user_role{roleId=R_super}`,下一次 UD 重建后 `ud.MinPermsLevel = 1`,在 `LoadCallerAssignableLevel` / `BindRoles` / `SetUserPerms` 中可把产品内任何 >1 的角色分配给他人——横向等价于"DEVELOPER 被签了 ADMIN 入场券"。
|
|
|
|
|
|
-- **内部滥用面**:P1 的任一普通 MEMBER(包括产品接入方给终端客户开的 low-tier 账号)都能拉走全员通讯录,钓鱼 / 二次社工的投递名单直接就位;
|
|
|
-- **跨产品溯源弱化**:`email` / `phone` 是全局 `sys_user` 字段,一人在 P1+P2 同为 MEMBER 时,两产品任一方的 MEMBER 都可拉到同一份 PII,合规审计上分不出泄漏源;
|
|
|
-- **UI 真需 vs 接口默认输出不一致**:即使前端当前 MEMBER 视角的页面不渲染 `email` / `phone`,接口仍在响应体里把字段返回,绕过前端就能拿到——API 契约才是授权边界,不是 UI。
|
|
|
+这条链并不是在分配既有角色的环节被 `GuardRoleLevelAssignable` 拦截(ADMIN 确实全权),而是**在创建角色时先把弹药造好**:新角色的 PermsLevel 不受 caller 自身 assignable level 的约束。`UpdateRoleLogic` 的 R12-3 防护没有覆盖"先建后提升"的构造式等价路径。
|
|
|
|
|
|
**影响**
|
|
|
|
|
|
-- **PII 过度暴露 → 合规红线**:GDPR/PIPL/内部数据分级都要求"联系方式"类字段按职责最小化返回。当前接口对同产品 MEMBER 无差别发放,容易被监管 / 安全评估点名;
|
|
|
-- **社工攻击前置资源充足**:攻击者一旦拿到任意 P1 MEMBER 的凭据(撞库、钓鱼、误 commit token),就能把 P1 全员通讯录导出,为后续的二阶钓鱼 / SIM swap / 账号接管提供精准名单;
|
|
|
-- **审计覆盖面与 `MemberList` 口径不一致**:`MemberListLogic` 已经收敛了响应字段(不泄露 PII),但 `UserListLogic`(同为"按产品分页列成员"用途)没有收敛,两者 API 语义重合但安全边界不同,易被误判。
|
|
|
+- 横向提权(escalate-by-delegate):product ADMIN 把一个 DEVELOPER / MEMBER 提到与自己同级甚至更高 assignable level,之后该下属的操作就会被审计记录为下属本人所为,ADMIN 可以"借刀杀人",审计链的"谁最终动的手"追溯性被破坏;
|
|
|
+- 横向绕 `CheckMemberTypeAssignment`:那个函数只管 memberType 的同级拦截,不涉及 role.permsLevel,故此条攻击路径独立成立;
|
|
|
+- 与既有 L-R13-2(`SetUserPerms` 拒绝对 ADMIN/DEVELOPER/SuperAdmin 下 DENY)不冲突:DENY 只是 perm 粒度兜底,role 粒度下 R_super 自带全部 perm 视角(配合 L-R11-4 的 loadPerms 全权分支逻辑),DENY 拦不住。
|
|
|
|
|
|
**修复方案**
|
|
|
|
|
|
-按"自己可见全部字段、他人仅超管/ADMIN/DEVELOPER 可见 PII"的分层授权收窄响应体。不建议在 logic 层加复杂 if/else 后再 copy 字段——容易随字段增加漏脱敏。推荐在响应装配前统一做 PII 脱敏:
|
|
|
+CreateRole 加入与 UpdateRole 对称的 `GuardRoleLevelAssignable`/`CheckRoleLevelAgainst` 校验:
|
|
|
|
|
|
```go
|
|
|
-// internal/logic/user/userListLogic.go
|
|
|
-
|
|
|
-// maskPII 统一决定:对于同产品内的他人,是否对 caller 隐藏 PII。
|
|
|
-// 规则:caller 是 SuperAdmin / ADMIN / DEVELOPER 返回原值;其他情况(含普通 MEMBER)置空。
|
|
|
-// caller 看自己不走这里——UserListLogic / UserDetailLogic 单独兜一下 `u.Id == caller.UserId` 的分支即可。
|
|
|
-func maskPII(caller *loaders.UserDetails, u *userModel.SysUser) (email, phone, remark string) {
|
|
|
- if caller.IsSuperAdmin ||
|
|
|
- caller.MemberType == consts.MemberTypeAdmin ||
|
|
|
- caller.MemberType == consts.MemberTypeDeveloper ||
|
|
|
- (caller.UserId == u.Id) {
|
|
|
- return u.Email, u.Phone, u.Remark
|
|
|
- }
|
|
|
- return "", "", ""
|
|
|
+caller := middleware.GetUserDetails(l.ctx)
|
|
|
+if caller == nil {
|
|
|
+ return nil, response.ErrUnauthorized("未登录")
|
|
|
}
|
|
|
-
|
|
|
-// 拼装 items 时:
|
|
|
-for _, u := range list {
|
|
|
- email, phone, remark := maskPII(caller, u)
|
|
|
- items = append(items, types.UserItem{
|
|
|
- Id: u.Id,
|
|
|
- Username: u.Username,
|
|
|
- Nickname: u.Nickname,
|
|
|
- Avatar: avatar,
|
|
|
- Email: email,
|
|
|
- Phone: phone,
|
|
|
- Remark: remark,
|
|
|
- DeptId: u.DeptId, // DeptId 本身不含 PII,可保留——前端可以凭此做部门筛选
|
|
|
- Status: u.Status,
|
|
|
- MemberType: memberType,
|
|
|
- CreateTime: u.CreateTime,
|
|
|
- })
|
|
|
+if !caller.IsSuperAdmin {
|
|
|
+ snap, err := authHelper.LoadCallerAssignableLevel(l.ctx, l.svcCtx, caller)
|
|
|
+ if err != nil {
|
|
|
+ return nil, err
|
|
|
+ }
|
|
|
+ // 新建角色的权限等级必须严格低于 caller 的可分配等级;同级也拒(与 CheckRoleLevelAgainst 一致)
|
|
|
+ if err := authHelper.CheckRoleLevelAgainst(snap, req.PermsLevel); err != nil {
|
|
|
+ return nil, err
|
|
|
+ }
|
|
|
}
|
|
|
```
|
|
|
|
|
|
-`UserDetailLogic` 沿用相同 helper;同时在 `UserDetailLogic` 里额外加一层"caller == target 或 caller 有管理职权时才返回完整信息"的自检(目标是 `caller.UserId != req.Id` 且 `caller` 非 ADMIN/DEVELOPER 的场景,可以考虑直接 `return ErrForbidden`,而不是回一个"只有昵称没有 PII"的半成品——后者会让前端误以为目标真的没绑邮箱 / 手机号)。
|
|
|
+特别注意:product ADMIN / DEVELOPER 的 `HasFullProductPerms=true`,`CheckRoleLevelAgainst` 对 HasFullPerms 会直接放行——这正是本 issue 的根因。修复时**不能**简单复用 `CheckRoleLevelAgainst`,而要把"创建角色"定义为:
|
|
|
|
|
|
-**回归验证要点**
|
|
|
+- SuperAdmin:任意 PermsLevel 通过;
|
|
|
+- product ADMIN:`req.PermsLevel` 必须 **>= 2**(给自己留出唯一的"顶格"语义,ADMIN 自身的 assignable 等价于 sentinel 0),且必须 **>= caller 的最小 permsLevel + 1**(避免 ADMIN 以 permsLevel=1 的角色强制覆盖自身 assignable baseline);
|
|
|
+- DEVELOPER 及以下:走 `CheckRoleLevelAgainst(snap, req.PermsLevel)` 标准路径(snap.Level 由下属自己的角色决定,拒同级)。
|
|
|
|
|
|
-- MEMBER 身份调用 `UserList` / `UserDetail`:
|
|
|
- - 看自己 → Email / Phone 原样返回;
|
|
|
- - 看他人 → Email / Phone / Remark 为空字符串,其余字段保留;
|
|
|
-- ADMIN / DEVELOPER 调用上述接口:所有字段原样返回(与现网行为一致,避免破坏管理台体验);
|
|
|
-- 前端若强依赖"字段非空"作逻辑分支,需同步升级——建议增加响应 schema 的版本协商或在 item 上新增 `piiVisible bool` 提示,减少默默置空导致的 UI 侧 regression。
|
|
|
+推荐把这条语义沉淀到一个新 helper `GuardCreateRolePermsLevel(ctx, svcCtx, caller, reqLevel) error`,与 `GuardRoleLevelAssignable` 并排放在 `access.go` 里,后续 UpdateRoleLogic 的 R12-3 支也可以复用。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-R16-1 · `UpdateUser` 的 ADMIN 分支短路 DeptPath 前缀校验 —— 非 `deptId=0` 方向的同构缺口
|
|
|
+## ⚠️ 健壮性与性能建议 (Medium/Low)
|
|
|
+
|
|
|
+### M-R17-1 · `SyncPermsService` 的 pure-add 分支对"全权用户"有最长 5min 的权限可见延迟
|
|
|
|
|
|
**位置**
|
|
|
|
|
|
-- `internal/logic/user/updateUserLogic.go:140-157`(`newDept.DeptType != DEV` 且 `caller.MemberType == ADMIN` 时直接放行,不比 DeptPath 前缀)
|
|
|
-- 对比参照:
|
|
|
- - `internal/logic/user/updateUserLogic.go:158-170`(`deptId=0` 已由 L-R15-1 收敛给 SuperAdmin)
|
|
|
- - `internal/logic/user/createUserLogic.go:102-109`(`CreateUser` 对非超管**强制**执行 `strings.HasPrefix(newDept.Path, caller.DeptPath)`,无 ADMIN 豁免)
|
|
|
+- `internal/logic/pub/syncPermsService.go:163-180`(`if updated > 0 || disabled > 0` 才清 `CleanByProduct`)
|
|
|
+- `internal/loaders/userDetailsLoader.go:539-554`(SuperAdmin / ADMIN / DEVELOPER / DEV-dept 成员走 `FindAllCodesByProductCode`)
|
|
|
|
|
|
**描述**
|
|
|
|
|
|
-L-R15-1 把 `UpdateUser.req.DeptId = 0` 这种"把目标移出全局部门树"的极端路径收敛给了 SuperAdmin,理由是:`sys_user.deptId` 是**全局**字段,P1 ADMIN 在 P1 的授权范围不应影响 P2 视角下的成员归属。但同一个逻辑在"`req.DeptId > 0` 且 `newDept.DeptType != DEV`"分支里仍然存在:
|
|
|
+L-R11-4 的注释写得很清楚:
|
|
|
|
|
|
-```140:157:internal/logic/user/updateUserLogic.go
|
|
|
-if newDept.DeptType == consts.DeptTypeDev && !caller.IsSuperAdmin {
|
|
|
- return response.ErrForbidden("仅超级管理员可将用户调入研发部门")
|
|
|
-}
|
|
|
-// 注意:ADMIN 分支短路 DeptPath 前缀校验,意味着 ADMIN 可以把目标调入任何**非 DEV**
|
|
|
-// 部门;DEV 目标部门的跨产品权限升级路径由上面 H-R14-1 的显式护栏拦截。
|
|
|
-if !caller.IsSuperAdmin &&
|
|
|
- caller.MemberType != consts.MemberTypeAdmin &&
|
|
|
- !strings.HasPrefix(newDept.Path, caller.DeptPath) {
|
|
|
- return response.ErrForbidden("无权将用户调入非自己管辖的部门")
|
|
|
-}
|
|
|
-```
|
|
|
+> 纯新增(added>0 && updated==0 && disabled==0)时不需要清 CleanByProduct。新增的 perm 在本次 SyncPerms 之前**不可能**已经被绑定到任何 role……loadPerms 对当前全体 user 的计算结果与上次结果完全一致。
|
|
|
|
|
|
-这一 `caller.MemberType != consts.MemberTypeAdmin` 短路让 P1 ADMIN 可以把"同时是 P2 成员"的 target `B` 从 P2 的部门子树下挪到 P1 的部门子树下(或任意既非 DEV 又处于 Enabled 的 NORMAL 部门,只要校验通过 `FindOneForShareTx`)。其直接副作用:
|
|
|
+但"loadPerms 对当前全体 user 的计算结果完全一致"这一条只对**普通 MEMBER**成立——他们走 `FindPermIdsByRoleIds` + `FindPermIdsByUserIdAndEffectForProduct` 的路径,新 perm 没被任何 role/allow/deny 引用,确实不影响。**全权用户走的是 `FindAllCodesByProductCode(productCode)` 单条分支**,这条查询返回的是该产品下所有 status=Enabled 的 perm 全集,新增任意 perm 都会让集合变大。
|
|
|
|
|
|
-1. **P2 视角下的部门链崩坏**:P2 的非 ADMIN 管理员(DEVELOPER、PermsLevel 接近 SuperAdmin 的 MEMBER)依赖 `CheckManageAccess → checkDeptHierarchy` 做"目标必须在 caller 子树下"的判定;B 的 DeptPath 被 P1 ADMIN 单方面改写为 P1 子树的前缀后,P2 的这些管理员对 B 的所有管理操作(降级、授权、改 Profile、冻结)都会落到 `response.ErrForbidden("无权管理其他部门的用户")`——B 在 P2 视角成为一个**结构性不可触达的账号**;
|
|
|
-2. **P2 的 DeptTree 里 B 也同时失踪**:`deptTreeLogic.go:52-62` 对非超管做 `strings.HasPrefix(d.Path, caller.DeptPath)` 的裁剪,B 的新 DeptPath 一旦不再以 P2 任何管理员的 `caller.DeptPath` 为前缀,B 在 P2 的部门树渲染里完全消失;
|
|
|
-3. **攻击链可达性**:P1 ADMIN 不需要超管权限,不需要绕过 H-R14-1 的 DEV 部门护栏,**仅靠合法的 "给 P1 成员换部门" 操作**就能在 P2 侧制造"隐形成员"——这与 L-R15-1 的 `deptId=0` 场景**完全同构**,只是构造方式换成"挪到 P1 子树"而不是"清空到 0"。
|
|
|
+具体时序:
|
|
|
|
|
|
-注释里声明的豁免理由("product ADMIN 对产品内既有成员有全面管理权")只在"不共享 target 的单产品场景"成立;一旦 target 同时归属多个产品(`sys_product_member` 允许多对多),ADMIN 改 `sys_user.deptId` 的动作已经穿透了"本产品 ADMIN 的权限天花板"。
|
|
|
+1. 产品 P 下已有 perm `A`/`B`,SuperAdmin `S` 登录,`ud.Perms = [A, B]`,写入 Redis(TTL=5min)。
|
|
|
+2. P 的业务服务发布 `v2`,新增 perm `C`,`SyncPerms` 走 pure-add 分支,`updated=disabled=0` → **跳过** `CleanByProduct`。
|
|
|
+3. `S` 在接下来 5 分钟内的每次 `/auth/userinfo` / login / refreshToken / 下游业务拉取 UD 都会命中 Redis 旧快照,`ud.Perms` 仍然是 `[A, B]`。
|
|
|
+4. 下游业务按 `Perms` 列表鉴权 `/v2/C`,S 被 403,"超管登录就是拉不到新接口"在发版当天很容易引起误判。
|
|
|
|
|
|
**影响**
|
|
|
|
|
|
-- 与 L-R15-1 同量级的跨产品结构性扰动,差别只是"orphan(deptId=0)"换成"挪到 P1 子树";运维可观测性更差——L-R15-1 的 orphan 至少可以通过 `WHERE deptId=0 AND isSuperAdmin=0` 直接捞出,而本条的 B 依然有正常 DeptPath,P2 运维必须跨产品比对才能发现"B 明明在 P2 有成员行,但 DeptPath 已经不在 P2 子树"的异常;
|
|
|
-- P1 ADMIN 对 B 的"单方面去 P2 化"是一种隐蔽的服务降级工具——B 在 P2 里还能调接口(jwt 依然有效、MemberType 还在 P2),但 P2 的管理台侧完全管不了他;
|
|
|
-- 与 `CreateUser` 的不对称性:`CreateUser` 对 ADMIN 也强制 DeptPath 前缀校验(`createUserLogic.go:102-109`),`UpdateUser` 对 ADMIN 却短路了这一校验——两者语义应该对齐(同样是"让一个用户落在某部门下"的动作)。
|
|
|
+- 发版 SLA:承诺下游"SyncPerms 成功即可使用新权限"的口径在这四类用户上被静默违反;
|
|
|
+- 审计链扭曲:`GetUserPerms` 的 perms 列表对全权用户最长延迟 5min,运营查验"这个超管到底能不能访问 C"时,看到的快照和真实授权的上游 DB 视图不一致;
|
|
|
+- 并不逃逸"最终一致",属 Medium 的感知可见性问题。
|
|
|
|
|
|
**修复方案**
|
|
|
|
|
|
-删除 `UpdateUser` 的 `caller.MemberType != consts.MemberTypeAdmin` 短路,把 ADMIN 也纳入 DeptPath 前缀校验范围;同时与 `CreateUser` 的校验口径保持一致。这条修改不会破坏 ADMIN 的正当业务:ADMIN 作为产品管理员,其 `caller.DeptPath` 本来就是其部门子树前缀,调整同子树内的成员部门归属不会被拦;真正被拦住的是"跨子树、跨产品"的越权改写。
|
|
|
-
|
|
|
-```go
|
|
|
-// internal/logic/user/updateUserLogic.go
|
|
|
-if *req.DeptId > 0 {
|
|
|
- newDept, err := l.svcCtx.SysDeptModel.FindOne(l.ctx, *req.DeptId)
|
|
|
- if err != nil {
|
|
|
- return response.ErrBadRequest("部门不存在")
|
|
|
- }
|
|
|
- if newDept.Status != consts.StatusEnabled {
|
|
|
- return response.ErrBadRequest("目标部门已停用")
|
|
|
- }
|
|
|
- if newDept.DeptType == consts.DeptTypeDev && !caller.IsSuperAdmin {
|
|
|
- return response.ErrForbidden("仅超级管理员可将用户调入研发部门")
|
|
|
- }
|
|
|
- // 审计 L-R16-1:与 CreateUser 口径对齐,删除 ADMIN 豁免。`sys_user.deptId` 是全局字段,
|
|
|
- // ADMIN 把共享成员挪到自己部门子树外会让其它产品的部门链校验对该 target 失效——与 L-R15-1
|
|
|
- // 的 deptId=0 场景完全同构,仅改"目标落点"而非"结构性副作用"。SuperAdmin 仍可跨子树调度。
|
|
|
- if !caller.IsSuperAdmin {
|
|
|
- if caller.DeptPath == "" {
|
|
|
- return response.ErrForbidden("您未归属任何部门,无权调整用户部门")
|
|
|
- }
|
|
|
- if !strings.HasPrefix(newDept.Path, caller.DeptPath) {
|
|
|
- return response.ErrForbidden("无权将用户调入非自己管辖的部门")
|
|
|
- }
|
|
|
- }
|
|
|
-}
|
|
|
-```
|
|
|
+两个可选路径:
|
|
|
|
|
|
-**回归验证要点**
|
|
|
+1. 精准失效(推荐):`SyncPerms` 在 `added > 0` 且当前产品存在全权用户(SuperAdmin + 本产品 ADMIN/DEVELOPER + DEV-dept 启用成员)时,只清这些用户的 UD 缓存,避免 CleanByProduct 的"全产品成员被穿回 DB"开销。实现上:
|
|
|
+ - `added > 0` → 事务内 `SELECT userId FROM sys_product_member WHERE productCode=? AND status=Enabled AND memberType IN ('ADMIN','DEVELOPER')` + 对 `sys_user` 全表 `WHERE isSuperAdmin=1` + 对 DEV 部门用户的查询(或直接在 tx 外跑,不影响锁序);
|
|
|
+ - 批量 `UserDetailsLoader.CleanByUserIds`。
|
|
|
+2. 简单保守:把 `if updated > 0 || disabled > 0` 扩成 `if added > 0 || updated > 0 || disabled > 0`,承担 `CleanByProduct` 的穿透开销作为代价。由于 SyncPerms 是 CI/CD 高频事件,这条偏"CleanByProduct 所有成员过一遍 Redis DEL",必要时再回退到方案 1。
|
|
|
|
|
|
-- P1 ADMIN 在 P1 子树内挪动自己部门的成员:放行(与现网一致);
|
|
|
-- P1 ADMIN 挪动"只属于 P1"的成员到 P1 子树外的 NORMAL 部门:拦截为 403(**本轮新增**;若业务确有此需求,应改走 SuperAdmin 审批流);
|
|
|
-- P1 ADMIN 挪动"同时也是 P2 成员"的 target 到 P1 子树外的部门:同上 403,避免跨产品结构性扰动;
|
|
|
-- SuperAdmin 行为不变:任何 NORMAL 部门均可挪入;DEV 部门同样不受本条改动影响。
|
|
|
+注释里的 `loadPerms 对当前全体 user 的计算结果完全一致` 必须同步改写——当前版本已经是错的,留着会把后人引向错误假设。
|
|
|
|
|
|
---
|
|
|
|
|
|
-### L-R16-2 · `UpdateDept` DeptType/Status 收窄 + `UpdateUser` 跨 DEV/NORMAL 边界调 deptId 均未吊销 tokenVersion —— 与 M-R15-1 同构的缓存失效 TOCTOU
|
|
|
+### M-R17-2 · `UpdateUserLogic` 在"DEV→NORMAL 调动"+"status 冻结"并发发生时对 `tokenVersion` 做双倍递增,虽然功能无害,但注释与 sysUser 低层缓存 invalidation 次数不对称
|
|
|
|
|
|
**位置**
|
|
|
|
|
|
-- `internal/logic/dept/updateDeptLogic.go:91-106`(`deptTypeChanged || statusChanged` 只走 `UserDetailsLoader.CleanByUserIds`,**无** `BatchIncrementTokenVersionWithTx`)
|
|
|
-- `internal/logic/user/updateUserLogic.go:195-250`(`UpdateProfile` / `UpdateProfileWithTx` 仅在 `statusChanged` 触发 `tokenVersion+1`;**deptId 跨越 DEV↔NORMAL 边界不递增**)
|
|
|
-- 对比参照:`internal/logic/product/updateProductLogic.go:77-104`(L-R15-3 已落地"禁用产品即批量吊销成员 session")、`internal/logic/member/updateMemberLogic.go:94-134`(M-R15-1 已落地)
|
|
|
+- `internal/logic/user/updateUserLogic.go`(R16-2 引入的 `devAccessRevoked` 分支会 `IncrementTokenVersionWithTx`)
|
|
|
+- `internal/model/user/sysUserModel.go::UpdatePassword`/`UpdateStatus`/`UpdateProfile` 各自会做 `tokenVersion+1`
|
|
|
|
|
|
**描述**
|
|
|
|
|
|
-`loadPerms` 的"是否走全权分支"明确地受三个字段驱动:`IsSuperAdmin` / `MemberType` / `DeptType + DeptStatus`:
|
|
|
+R16 给 `UpdateUser` 加上了"DEV → NORMAL 跨域调动时 tokenVersion+1";如果同一请求还命中 `status Enabled→Disabled`,`UpdateProfile` 内部本就会 `tokenVersion+1`(或 R16 的代码路径再走一次显式 `IncrementTokenVersionWithTx`),结果是事务内对同一 userId 做了 2 次自增。功能上毫无副作用(新 token 不会分叉,Redis 缓存还是会 post-commit Clean),但:
|
|
|
|
|
|
-```539:554:internal/loaders/userDetailsLoader.go
|
|
|
-// 超管 / ADMIN / DEVELOPER / 研发部门的有效成员 → 全量权限
|
|
|
-if ud.IsSuperAdmin ||
|
|
|
- ud.MemberType == consts.MemberTypeAdmin ||
|
|
|
- ud.MemberType == consts.MemberTypeDeveloper ||
|
|
|
- (ud.MemberType != "" && ud.DeptType == consts.DeptTypeDev && ud.DeptStatus == consts.StatusEnabled) {
|
|
|
- codes, err := l.models.SysPermModel.FindAllCodesByProductCode(ctx, ud.ProductCode)
|
|
|
- ...
|
|
|
-}
|
|
|
-```
|
|
|
+- R16 的注释只解释了"DEV 调动"这一半为什么要 +1,没有声明"如果还叠加了 status change,递增会叠加";
|
|
|
+- 未来运维如果用 `tokenVersion` 做"有效会话批次号"的信号量分析,会看到"单次 UpdateUser 却 +2"的异常样本,产生噪声;
|
|
|
+- 双递增让 `UpdateProfile` 的 CAS `WHERE updateTime=?` 在第二次 SQL 之前要依赖第一次 SQL 的结果(事务内一致),调试复杂。
|
|
|
|
|
|
-M-R15-1 / L-R15-3 已经为 `MemberType` 的收窄路径(UpdateMember 降级 / UpdateProduct 禁用)建立了"tx 内 `tokenVersion+1` → 签发层吊销"的闭环,避免 Redis 抖动时 5min TTL 让旧全权继续生效。但 **`DeptType` / `DeptStatus` 同样是 loadPerms 全权分支的直接输入**,它们的收窄路径现在还是"仅 `UserDetailsLoader.CleanByUserIds` 尽力而为":
|
|
|
+**修复方案**
|
|
|
|
|
|
-```91:106:internal/logic/dept/updateDeptLogic.go
|
|
|
-if deptTypeChanged || statusChanged {
|
|
|
- userIds, err := l.svcCtx.SysUserModel.FindIdsByDeptId(l.ctx, req.Id)
|
|
|
- if err != nil {
|
|
|
- l.Errorf("UpdateDept id=%d deptType=%s status=%d 部门已更新但 FindIdsByDeptId 失败,用户权限缓存未能主动失效,将等待 TTL 自然过期: %v", req.Id, dept.DeptType, dept.Status, err)
|
|
|
- return nil
|
|
|
- }
|
|
|
- if len(userIds) > 0 {
|
|
|
- cleanCtx, cancel := loaders.DetachCacheCleanCtx(l.ctx)
|
|
|
- defer cancel()
|
|
|
- l.svcCtx.UserDetailsLoader.CleanByUserIds(cleanCtx, userIds)
|
|
|
- l.Infof("UpdateDept id=%d deptType=%s status=%d affectedUsers=%d", req.Id, dept.DeptType, dept.Status, len(userIds))
|
|
|
- }
|
|
|
-}
|
|
|
-```
|
|
|
+两种路径:
|
|
|
+
|
|
|
+1. 改语义:把"是否需要 bump tokenVersion"收归到 `UpdateUser` 这一层决策,内部只调一次 `IncrementTokenVersionWithTx(userId)` 或直接在 `UpdateProfile` 的 SQL 里多带一个 `bumpTokenVersion bool` 参数,tx 内只做 1 次 `tokenVersion+1`。
|
|
|
+2. 不改语义,在注释里明确声明"devAccessRevoked + statusChanged 重叠时会 +2,但对安全语义等价于 +1",并在 `IncrementTokenVersionWithTx` 的文档注释里补上"可安全叠加调用"。
|
|
|
|
|
|
-相同问题也出现在 `UpdateUser` 改 `deptId` 的路径:`UpdateProfileWithTx` 只认 `statusChanged` 去递增 `tokenVersion`,`deptId` 跨越 DEV/NORMAL 边界时没有任何签发层吊销,只依赖 `UserDetailsLoader.Clean` 的尽力而为失效。
|
|
|
+无论走哪条都建议新增一条 TC:单请求同时触发 dev 调出 + 冻结,断言新 tokenVersion = 旧 + N(N 取当前实现值),把行为锁死。
|
|
|
|
|
|
-收窄方向的具体触发条件(这些全都是"权限从全权收回"的场景):
|
|
|
+---
|
|
|
|
|
|
-1. `UpdateDept`:`DeptType` 从 `DEV → NORMAL`——该部门所有在编成员在所属产品里的 `loadPerms` 从"全量权限"降级为"角色/allow-deny 计算";
|
|
|
-2. `UpdateDept`:`Status` 从 `Enabled → Disabled`——DEV 部门全权分支要求 `DeptStatus == StatusEnabled`,禁用即失去全权;NORMAL 部门成员是否被禁用则改变 `jwtauthMiddleware` 对 `ud.Status` 的放行,但 dept.Status 对用户而言不是直接阻断字段,主要影响仍是 DEV 部门的全权分支;
|
|
|
-3. `UpdateUser`:把用户 `deptId` 从 DEV 部门挪到 NORMAL 部门——单一用户的全权被收回。
|
|
|
+### L-R17-1 · `CreateProductLogic` 的 initial admin 用户名 `admin_<productCode>` 可被任意 product ADMIN 预抢注,导致 SuperAdmin 上新产品时撞 UNIQUE(username) 回滚
|
|
|
|
|
|
-这三条路径的 TOCTOU 窗口与 M-R15-1 完全一致:
|
|
|
+**位置**
|
|
|
|
|
|
-1. SuperAdmin 改部门/用户归属;事务 COMMIT;
|
|
|
-2. post-commit `UserDetailsLoader.Clean*` 因 Redis 抖动失败(3s `DetachCacheCleanCtx` 超时 + 日志标记 `cache_invalidation_skipped_*`,不重试);
|
|
|
-3. 缓存里 `{DeptType=DEV, DeptStatus=Enabled, Perms=[全量]}` 保留到 TTL 过期(最长 5min);
|
|
|
-4. 受影响用户的 access token `TokenVersion` 未递增(DeptType/Status 变更不触发 UpdateProfile 的 `tokenVersion+1`),`jwtauthMiddleware` 的 `TokenVersion != ud.TokenVersion` 兜底不触发;
|
|
|
-5. 用户在这 5min 内继续以 "DEV 全权" 身份调业务接口。
|
|
|
+- `internal/logic/product/createProductLogic.go`(生成 `adminUsername := fmt.Sprintf("admin_%s", req.Code)` 然后 `SysUserModel.Insert`)
|
|
|
+- `internal/logic/user/createUserLogic.go:54`(username regex `^[a-zA-Z0-9_]{2,64}$` 不保留 `admin_` 前缀)
|
|
|
|
|
|
-与 R15 不同的是:`UpdateDept` / `UpdateUser.deptId` 的典型使用者是 **SuperAdmin**(`UpdateDept` 已经 `RequireSuperAdmin`;`UpdateUser` 改 deptId 跨 DEV 边界也被 H-R14-1 收敛给 SuperAdmin),因此这条 TOCTOU 的触发点比 `UpdateMember` 更低频——但**影响面更广**:`UpdateDept` 一次性影响"该部门所有成员";`UpdateProduct` 影响"该产品所有成员";两者叠加时,Redis 抖动可以让一个完整部门在 5min 窗口内保留已经被收回的 DEV 全权。
|
|
|
+**描述**
|
|
|
|
|
|
-**影响**
|
|
|
+product ADMIN 在自己产品下可用 `CreateUser` 直接创建用户名 `admin_acme`,这个用户跟未来真正要上线的 productCode `acme` 共用一个 UNIQUE(username) 槽位。超管过了几个月要创建产品 `acme`:
|
|
|
|
|
|
-- Redis 抖动或长时间网络分区(backfill / 故障演练期间)下,"DEV→NORMAL"或"禁用部门"的操作在 5min 窗口内不生效;
|
|
|
-- 与 M-R15-1 / L-R15-3 的"签发层吊销"设计哲学不一致,审计回归矩阵需要解释"为什么 dept 收窄走尽力而为、member/product 收窄走强制吊销"——当前代码没有任何注释给出差异理由,容易被后续维护者当作遗漏;
|
|
|
-- 若组合 H-R16-1 攻击(tokenVersion 不变),一个原本在 DEV 部门的 ADMIN 被 SuperAdmin 从 DEV 挪到 NORMAL,其全权仍在 5min 内有效——与"先 DEV→NORMAL 后立即 RemoveMember"的组合相比,本路径更难被监控感知(无 `RemoveMember` 事件、只有 `UpdateUser/UpdateDept` 的低敏感事件)。
|
|
|
+1. `CreateProductLogic` 走到 auto-provision step,`username = "admin_acme"` 已存在;
|
|
|
+2. `SysUserModel.Insert` 撞 1062,`util.IsDuplicateEntryErr` 被 `compensateCreatedRows` 捕获,已创建的 sys_product / 可能的 role / perm 全部回滚;
|
|
|
+3. 超管必须让运维人肉 data fix 把抢注的 `admin_acme` 改名,或改用一个非冲突的 productCode。
|
|
|
+
|
|
|
+组合 R16 的 H-R16-1 等一系列权限收敛之后,这条攻击虽然不能**提权**,但可以:
|
|
|
+
|
|
|
+- 针对"已知下一批待上线 productCode"(内部 Roadmap 半公开场景)做批量抢注,让超管短期内根本无法上新产品,等价于一次**业务 DoS**;
|
|
|
+- 如果被抢注的 `admin_<code>` 之后被 product ADMIN 给了自己或同谋,产品上线失败的日志/告警把 SuperAdmin 的注意力引向"产品 code 冲突",抢注者获得时间差;
|
|
|
+- 与"product ADMIN 看不到其他产品列表"(M-2) 并不冲突——抢注者只要知道 code 就行,不需要看列表。
|
|
|
|
|
|
**修复方案**
|
|
|
|
|
|
-两处收窄路径在 tx 内补齐 `BatchIncrementTokenVersionWithTx`——复用 `UpdateProduct` 的 L-R15-3 模式,把"找出受影响 userIds"和"批量 +1"收敛进同一个事务,整体回滚语义天然成立。关键判定:只在**真正构成"权限收窄"**时递增,避免 `NORMAL→DEV`(升权)或 `Disabled→Enabled`(重启用)场景误踢用户下线。
|
|
|
+- 短期:把 auto-provision 的 username 加上不可人工构造的前缀 / 后缀,例如 `svc_admin_<code>_<random_10hex>`,彻底脱离 `^[a-zA-Z0-9_]{2,64}$` 的"纯小写字母数字"可人工推测空间。
|
|
|
+- 中期:在 `CreateUser` 的 username 正则层面引入保留前缀黑名单(`admin_` / `svc_` / `root_` / `sys_` 等),非超管创建以这些前缀开头的 username 直接 400。
|
|
|
+- 长期:引入 username namespace 机制,把"系统账号"和"用户账号"隔离到不同 table / 不同 uniqueness 域(例如用 `sys_service_account` 存 auto-provision 行,与 `sys_user` 分离)。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### L-R17-2 · `CreateUser` / `CreateProductLogic` 未显式设置 `Avatar`,导致 `sql.NullString` 以 `{Valid:true, String:""}` 默认落库,对消费方而言歧义
|
|
|
|
|
|
-**1. `UpdateDept` 修复(DEV→NORMAL 或 DEV 部门 Enabled→Disabled 才递增):**
|
|
|
+**位置**
|
|
|
+
|
|
|
+- `internal/logic/user/createUserLogic.go:120-134`(Insert 字段集未包含 Avatar)
|
|
|
+- `internal/logic/product/createProductLogic.go`(initial admin 构造 `SysUser{…}` 同样未设 Avatar)
|
|
|
+- `internal/model/user/sysUserModel_gen.go::SysUser.Avatar`(类型为 `sql.NullString`)
|
|
|
+- `internal/loaders/userDetailsLoader.go::loadSysUser`(`ud.Avatar = u.Avatar.String`)
|
|
|
+
|
|
|
+**描述**
|
|
|
+
|
|
|
+Go 结构体字面量初始化时未设置的 `sql.NullString` 字段默认 `{Valid:false, String:""}`。但 `SysUserModel.Insert` 走的是 sqlx 的结构体到列映射,Valid=false 的 NullString 写入 DB 时会落 `NULL`,此时读取出来后 `u.Avatar.String=""` & `u.Avatar.Valid=false`;`ud.Avatar = u.Avatar.String` 会把 `""` 拷到 UD.Avatar。
|
|
|
+
|
|
|
+看起来不会出问题——但**默认行为依赖"Go 零值 + sql.NullString 语义链"**,一旦未来有人把 `SysUser.Avatar` 改成纯 `string`(去掉 NullString),现在的 Insert 字段集不变就会落一个 `''`;而此前已有用户的 Avatar 可能 `NULL`(Valid=false),DB 层 inplace migrate 时会出现部分行 `NULL`、部分行 `''` 并存的脏数据。前端把"未上传头像"判断为 `avatar == null` 还是 `avatar == ''`?两种写法都合理,但两种数据并存会让"默认头像"只对一半用户生效。
|
|
|
+
|
|
|
+**修复方案**
|
|
|
+
|
|
|
+在 CreateUser / CreateProductLogic 的 Insert 字面量里显式写:
|
|
|
|
|
|
```go
|
|
|
-// internal/logic/dept/updateDeptLogic.go
|
|
|
-// 判定"收窄方向":仅在以下三种情况下需要 tokenVersion+1:
|
|
|
-// (a) DeptType: DEV → NORMAL(该部门所有成员失去 DEV 全权分支)
|
|
|
-// (b) DeptType 不变但 DEV 部门 Status: Enabled → Disabled(DEV 成员失去全权)
|
|
|
-// (c) DeptType 不变但 NORMAL 部门 Status: Enabled → Disabled(无直接授权影响,可选是否递增;保守起见建议递增,因为业务语义是"冻结部门所有活动")
|
|
|
-// NORMAL→DEV(升权)与 Disabled→Enabled(恢复)不递增。
|
|
|
-prevType := req.dept.DeptType // 即原 dept.DeptType
|
|
|
-prevStatus := req.dept.Status // 即原 dept.Status
|
|
|
-nextType := dept.DeptType // 应用 req 之后
|
|
|
-nextStatus := dept.Status
|
|
|
-devFullAccessRevoked := (prevType == consts.DeptTypeDev && nextType == consts.DeptTypeNormal) ||
|
|
|
- (prevType == consts.DeptTypeDev && prevStatus == consts.StatusEnabled && nextStatus == consts.StatusDisabled)
|
|
|
-normalDeptFrozen := prevType == consts.DeptTypeNormal && nextType == consts.DeptTypeNormal &&
|
|
|
- prevStatus == consts.StatusEnabled && nextStatus == consts.StatusDisabled
|
|
|
-
|
|
|
-var revokedUserIds []int64
|
|
|
-err := l.svcCtx.SysDeptModel.TransactCtx(l.ctx, func(ctx context.Context, session sqlx.Session) error {
|
|
|
- if err := l.svcCtx.SysDeptModel.UpdateWithOptLockTx(ctx, session, dept, expectedUpdateTime); err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- if devFullAccessRevoked || normalDeptFrozen {
|
|
|
- // FindIdsByDeptId 需要提供 Tx 版本;若 tx 版本不存在,可先 pre-commit 拿 userIds,但必须收进同一个事务以避免 TOCTOU。
|
|
|
- ids, err := l.svcCtx.SysUserModel.FindIdsByDeptIdForShareTx(ctx, session, req.Id)
|
|
|
- if err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- if len(ids) > 0 {
|
|
|
- if err := l.svcCtx.SysUserModel.BatchIncrementTokenVersionWithTx(ctx, session, ids); err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
- revokedUserIds = ids
|
|
|
- }
|
|
|
- }
|
|
|
- return nil
|
|
|
-})
|
|
|
-// post-commit 继续走 CleanByUserIds + InvalidateProfileCache(与 UpdateProduct 对齐)
|
|
|
+Avatar: sql.NullString{Valid: false},
|
|
|
```
|
|
|
|
|
|
-若 `SysUserModel` 当前没有 `FindIdsByDeptIdForShareTx`,需补一个带 `LOCK IN SHARE MODE` 的版本——比照 `FindActiveMemberUserIdsByProductCodeTx` 的实现(`internal/model/productmember/sysProductMemberModel.go:102-110`),与并发 `UpdateProfileWithTx`(X 锁 sys_user)互斥,防止"列出 userIds 期间有人刚被挪出本部门"造成吊销漏挂。
|
|
|
+或者把 Avatar 字段从 NullString 降级为普通 string + 业务层在 loadSysUser 后显式处理"空串 = 未上传"语义。更 aggressive 一点,直接在 SysUser 的 Insert helper 里对所有 Nullable 字段走显式默认值构造器,避免散落各处的"忘记赋值→依赖零值"。
|
|
|
|
|
|
-**2. `UpdateUser` 修复(仅在 deptId 跨 DEV↔NORMAL 边界且方向为收窄时递增):**
|
|
|
+---
|
|
|
|
|
|
-```go
|
|
|
-// internal/logic/user/updateUserLogic.go
|
|
|
-// 读取旧 dept(user.DeptId)与新 dept(newDept)的 DeptType:
|
|
|
-// 收窄方向 = (old.DeptType==DEV && new.DeptType==NORMAL)
|
|
|
-// 升权方向 = (old.DeptType==NORMAL && new.DeptType==DEV)——已由 H-R14-1 收敛给 SuperAdmin
|
|
|
-devAccessRevoked := false
|
|
|
-if req.DeptId != nil && *req.DeptId != user.DeptId {
|
|
|
- var oldDept *deptModel.SysDept
|
|
|
- if user.DeptId > 0 {
|
|
|
- oldDept, _ = l.svcCtx.SysDeptModel.FindOne(l.ctx, user.DeptId)
|
|
|
- }
|
|
|
- if oldDept != nil && oldDept.DeptType == consts.DeptTypeDev && newDept.DeptType == consts.DeptTypeNormal {
|
|
|
- devAccessRevoked = true
|
|
|
- }
|
|
|
+### L-R17-3 · `DeptTreeLogic` 对非超管只返回 DeptPath 子树,但非成员子树被整体丢弃时没有审计日志标记,排障时无法区分"用户无权"和"DB 抖动返空"
|
|
|
+
|
|
|
+**位置**
|
|
|
+
|
|
|
+- `internal/logic/dept/deptTreeLogic.go:52-63`(非超管分支过滤非子树部门时直接 continue,不 log)
|
|
|
+
|
|
|
+**描述**
|
|
|
+
|
|
|
+非超管(含产品 ADMIN / DEVELOPER / MEMBER)调用 `DeptTree`,`DeptPath` 前缀不匹配的部门都被静默过滤。当 `caller.DeptPath == ""`(数据链路异常、user 刚被超管改到部门外等)时,直接返回空数组,UI 显示"无部门",看起来跟"DB 抖动返空"无法区分。L-R15-2 已经把 fullAccess 收敛给 SuperAdmin,但**对过滤掉多少 dept**没有可观测性。
|
|
|
+
|
|
|
+**修复方案**
|
|
|
+
|
|
|
+- 对 `len(list) > 0 && len(filtered) == 0` 且 `caller.DeptPath != ""` 的边界打一条 INFO:caller 有 DeptPath 但过滤后为空(可能是 DeptPath 指向已删除部门 = H-R17-1 的前奏迹象);
|
|
|
+- 对 `caller.DeptPath == ""` 直接 log WARN,提示运维可能是 L-R16-1 / H-R17-1 类的孤儿账号。
|
|
|
+
|
|
|
+---
|
|
|
+
|
|
|
+### L-R17-4 · `changePasswordLogic.go::ChangePassword` 的"旧密码等于新密码"校验顺序让 bcrypt compare 先于字符串等值比较,浪费 ~60ms CPU
|
|
|
+
|
|
|
+**位置**
|
|
|
+
|
|
|
+- `internal/logic/auth/changePasswordLogic.go:60-67`
|
|
|
+
|
|
|
+**描述**
|
|
|
+
|
|
|
+```60-67:internal/logic/auth/changePasswordLogic.go
|
|
|
+if err := bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(req.OldPassword)); err != nil {
|
|
|
+ logx.WithContext(l.ctx).Infof("change-password old-password mismatch userId=%d", userId)
|
|
|
+ return response.ErrBadRequest("原密码错误")
|
|
|
}
|
|
|
|
|
|
-// tx 体内补:
|
|
|
-if devAccessRevoked {
|
|
|
- if _, err := l.svcCtx.SysUserModel.IncrementTokenVersionWithTx(ctx, session, req.Id); err != nil {
|
|
|
- return err
|
|
|
- }
|
|
|
+if req.OldPassword == req.NewPassword {
|
|
|
+ return response.ErrBadRequest("新密码不能与原密码相同")
|
|
|
}
|
|
|
```
|
|
|
|
|
|
-**权衡说明**
|
|
|
+校验顺序:
|
|
|
+
|
|
|
+1. bcrypt 比对 old password(耗时 ~60ms,CPU 重);
|
|
|
+2. 字符串比较 old == new(纳秒级);
|
|
|
+3. bcrypt 生成新 hash(另一次 ~60ms)。
|
|
|
+
|
|
|
+把 (2) 提前到 (1) 之前:
|
|
|
+
|
|
|
+- 对合法请求无影响((2) 返 false 时进入 (1) 正常流程);
|
|
|
+- 对"同密码"攻击请求可以提前 return,少做一次 bcrypt compare(60ms CPU)。
|
|
|
+
|
|
|
+**修复方案**
|
|
|
+
|
|
|
+直接交换顺序,`req.OldPassword == req.NewPassword` 放到 bcrypt compare 之前:
|
|
|
+
|
|
|
+```go
|
|
|
+if req.OldPassword == req.NewPassword {
|
|
|
+ return response.ErrBadRequest("新密码不能与原密码相同")
|
|
|
+}
|
|
|
+if err := bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(req.OldPassword)); err != nil {
|
|
|
+ // ...
|
|
|
+}
|
|
|
+```
|
|
|
|
|
|
-本条优先级明确低于 M-R15-1 / H-R16-1:触发者几乎都是 SuperAdmin(`UpdateDept` 强制超管;`UpdateUser` 改 deptId 跨 DEV 边界也被 H-R14-1 限制为超管),攻击窗口需要 "SuperAdmin 误操作 + Redis 故障" 双重触发;但一旦落在监管敏感的产品上,5min 的残余 DEV 全权可能覆盖多次敏感接口调用。按"与 L-R15-3 的口径对称"的原则把这条修了对长期维护一致性有利,单独不修也可以在审计注释里明确声明"本路径 TOCTOU 由 SuperAdmin 信任边界 + Clean 尽力而为组合承担"——但这会让后续维护者难以推理"为什么同样是授权收窄,member/product 走强吊销、dept/user.deptId 走尽力而为"。
|
|
|
+关注点:这会让"攻击者用正确用户名 + 猜对 oldPwd 等于 newPwd"的响应时间比"猜错 oldPwd"快约 60ms,理论上构成 timing 信号。但这条信号对攻击者几乎无用——要 trigger 这条分支攻击者需要**已经知道 oldPwd**,一旦攻击者已知 oldPwd,ChangePassword 本身就是 game over。所以 timing 差异可以接受。
|
|
|
|
|
|
---
|
|
|
|
|
|
-## R15 回归验证(附录)
|
|
|
+### L-R17-5 · `createDeptLogic.go` 的"Insert → FindOneWithTx → Update"三步写回 Path,可以合并为一次 `UPDATE … SET path = CONCAT(?, id, '/') WHERE id=?` 省一次 DB roundtrip
|
|
|
|
|
|
-| 条目 | 期望修复 | 代码现状 | 判定 |
|
|
|
-| --- | --- | --- | --- |
|
|
|
-| M-R15-1 MemberType TOCTOU(UpdateMember 降级/禁用不吊销 session) | tx 内 `IncrementTokenVersionWithTx`,post-commit 同步失效 sysUser 低层缓存 | `updateMemberLogic.go:94-154` 新增 `typeDowngraded/statusRevoked` 判定 + `IncrementTokenVersionWithTx` + `InvalidateProfileCache`;`tokenVersionRevocation` 结构封装闭包跨域传 userId,避免事务失败时脏状态泄漏 | ✅ 已闭合(方案 A) |
|
|
|
-| L-R15-1 UpdateUser.deptId=0 改写全局字段 | `deptId=0` 收敛给 SuperAdmin | `updateUserLogic.go:158-170` 非超管直接 403;注释显式声明与 H-R14-1 的结构对称性 | ✅ 已闭合 |
|
|
|
-| L-R15-2 DeptTree fullAccess 泄露组织架构 | `fullAccess = caller.IsSuperAdmin` | `deptTreeLogic.go:50` 已改;产品 ADMIN 走 DeptPath 前缀裁剪;注释清晰说明 sys_dept 全局命名空间与 M-2 的最小授权精神 | ✅ 已闭合 |
|
|
|
-| L-R15-3 UpdateProduct 禁用不吊销成员 session | tx 内 `BatchIncrementTokenVersionWithTx` + FOR SHARE 锁成员行 | `updateProductLogic.go:77-127` 已落地;`FindActiveMemberUserIdsByProductCodeTx` 用 `LOCK IN SHARE MODE` 阻塞并发 AddMember/UpdateMember/RemoveMember 的 X 锁 | ✅ 已闭合 |
|
|
|
-| L-R15-4 降权路径语义需在注释里显式声明 | updateMemberLogic / updateProductLogic 顶部注释说明 | `updateMemberLogic.go:35-43` / `updateProductLogic.go:36-42` 均已补齐,引用 M-R15-1 / L-R15-3 审计条目 | ✅ 已闭合 |
|
|
|
+**位置**
|
|
|
+
|
|
|
+- `internal/logic/dept/createDeptLogic.go:86-109`
|
|
|
+
|
|
|
+**描述**
|
|
|
+
|
|
|
+```86-109:internal/logic/dept/createDeptLogic.go
|
|
|
+result, err := l.svcCtx.SysDeptModel.InsertWithTx(ctx, session, &deptModel.SysDept{ /* Path: parentPath 是占位 */ })
|
|
|
+// ...
|
|
|
+deptId, _ = result.LastInsertId()
|
|
|
+d, err := l.svcCtx.SysDeptModel.FindOneWithTx(ctx, session, deptId)
|
|
|
+if err != nil {
|
|
|
+ return err
|
|
|
+}
|
|
|
+d.Path = fmt.Sprintf("%s%d/", parentPath, deptId)
|
|
|
+return l.svcCtx.SysDeptModel.UpdateWithTx(ctx, session, d)
|
|
|
+```
|
|
|
+
|
|
|
+三步模式:Insert → 再 FindOne → 再 Update,`FindOneWithTx` 这一步纯粹为了拿到其余字段喂给 `UpdateWithTx`(`UpdateWithTx` 要整行对象)。其实 `InsertWithTx` 已返回 LastInsertId,parentPath 本身已知,可以直接一条 UPDATE 搞定,或者 `UpdateWithTx` 之外再暴露一个 `UpdatePathOnlyWithTx` 模式。
|
|
|
+
|
|
|
+**修复方案**
|
|
|
+
|
|
|
+建议在 `sysDeptModel` 加一个:
|
|
|
+
|
|
|
+```go
|
|
|
+func (m *customSysDeptModel) UpdatePathWithTx(ctx context.Context, session sqlx.Session, id int64, path string, updateTime int64) error {
|
|
|
+ // UPDATE sys_dept SET `path`=?, `updateTime`=? WHERE id=?
|
|
|
+ // 同时 DelCache(cacheSysDeptIdPrefix+id)
|
|
|
+}
|
|
|
+```
|
|
|
+
|
|
|
+CreateDept 改调 `UpdatePathWithTx(ctx, session, deptId, fmt.Sprintf("%s%d/", parentPath, deptId), now)`,省掉 FindOneWithTx 的一次往返;tx 持锁时间缩短,降低 sys_dept X 锁的尾部延迟。
|
|
|
|
|
|
---
|
|
|
|
|
|
-## 本轮总结
|
|
|
+### L-R17-6 · `UpdateUserStatusLogic` 对"解冻(Disabled → Enabled)"同样执行 `tokenVersion+1` 与 `UserDetailsLoader.Clean`,注释只覆盖冻结场景
|
|
|
+
|
|
|
+**位置**
|
|
|
+
|
|
|
+- `internal/logic/user/updateUserStatusLogic.go:59-71`(`UpdateStatus` 内部 `tokenVersion+1`)
|
|
|
+- `internal/model/user/sysUserModel.go::UpdateStatus`
|
|
|
+
|
|
|
+**描述**
|
|
|
+
|
|
|
+`UpdateStatus` 的 SQL 写死 `SET tokenVersion = tokenVersion + 1`,无论 target.Status 是从 Disabled → Enabled 还是 Enabled → Disabled。对 Enabled→Disabled 侧的 audit 注释写得很清楚(冻结后的旧 token 必须立即失效);对 Disabled→Enabled 侧无注释,实际上也会 +1(虽然用户本来就因为 Disabled 无法登录,再 +1 等价于空动作,无害)。
|
|
|
|
|
|
-本轮新增 1 条 High(**H-R16-1**,`RemoveMember` 降权路径与 M-R15-1 同构但未同步吊销 session),1 条 Medium(**M-R16-1**,`UserList` / `UserDetail` 对同产品 MEMBER 暴露 PII),2 条 Low(**L-R16-1**,`UpdateUser` ADMIN 分支短路 DeptPath 前缀校验,构成 L-R15-1 的非零 deptId 同构缺口;**L-R16-2**,`UpdateDept` / `UpdateUser.deptId` 的权限收窄路径仍只走缓存失效、未 tokenVersion+1,与 M-R15-1 的签发层吊销口径不一致)。
|
|
|
+**修复方案**
|
|
|
|
|
|
-语义关联:
|
|
|
+- 不改代码,仅补注释:`UpdateStatus` 无条件递增 tokenVersion 是刻意设计,让"冻结-解冻-冻结"这条路径里 Redis 残留缓存的 tokenVersion 比对始终相对于"最新值"计算,即使解冻间隙 DB 抖动,也不会让一个"冻结前未过期的 access token"在解冻后复活。这条语义在 `UpdateUserStatusLogic` 和 `sysUserModel.UpdateStatus` 的函数头注释里都要显式写出来,避免后续 R18 读代码时误以为"解冻无需 bump"做成条件判断反而引入回归。
|
|
|
+
|
|
|
+---
|
|
|
|
|
|
-- **H-R16-1** 与 **L-R16-2** 同属"降权吊销覆盖对称性"的残留工单;修完 H-R16-1 应顺带把 L-R16-2 一并处置,否则每轮审计都要重新解释"为什么有些降权走强吊销、有些走尽力而为";
|
|
|
-- **L-R16-1** 是 L-R15-1 在另一方向的同构项,两者合计描述的是"`sys_user.deptId` 作为全局字段,其任何写入都应该是 SuperAdmin 的权限维度"这一不变式;
|
|
|
-- **M-R16-1** 是独立的 PII 最小授权工单,与上述三条互相正交,但同样与 R14 的 H-R14-1 / L-R15-1 共享"产品 ADMIN 的权限边界不应穿透到全局字段"的设计哲学——MemberType 应当反向收敛读权限,不应因"同产品"就默认放行全部 PII。
|
|
|
+## 汇总
|
|
|
|
|
|
-优先处置顺序:**H-R16-1 → M-R16-1 → L-R16-1 → L-R16-2**。H-R16-1 最低代价(tx 内加一次 `IncrementTokenVersionWithTx`,与 M-R15-1 逻辑完全同构)但关掉一个 High 级攻击窗口;M-R16-1 对合规侧影响最大,修复面集中在两个 logic 文件;L-R16-1 / L-R16-2 可并入下一轮安全变更统一回归。
|
|
|
+- 本轮新发现 3 条 High(`H-R17-1`、`H-R17-2`、`H-R17-3`),均为**结构性**问题,无法通过单点补丁彻底解决,需要:
|
|
|
+ - `H-R17-1` 把 `CreateUser`/`CreateProductLogic` 的 user 落库动作迁入事务并加 `sys_dept` S 锁(bcrypt 必须留在事务外);
|
|
|
+ - `H-R17-2` 增加 post-commit 低层缓存失效兜底,覆盖 `sys_dept` / `sys_role` 的 `*WithTx` 路径;长期建议通过自定义事务钩子把 go-zero sqlc 的"exec → DelCache"模式从事务语义里摘出去;
|
|
|
+ - `H-R17-3` 在 `CreateRoleLogic` 镜像 `UpdateRoleLogic` 的 L-R12-3 防护,并抽象 `GuardCreateRolePermsLevel`。
|
|
|
+- Medium 2 条、Low 6 条,以感知可见性 / 注释对齐 / 性能微调为主。
|
|
|
+- R16 及更早各轮修复经本轮复检全部稳定,不存在回归。
|