audit-report.md 28 KB

权限管理系统 - 深度代码审计报告

审计范围:/internal 下全部非测试生产代码(logic、model、middleware、handler、loaders、server、svc、config、consts、response、util)及入口文件 perm.go、gRPC 客户端 permclient/。 审计时间:2026-04-18 审计重点:逻辑一致性、并发竞态、数据完整性、水平越权、缓存一致性、僵尸代码、N+1、接口契约。


🚩 核心逻辑漏洞 (High Risk)

H-1. BindRoles 的 permsLevel 越级校验错误地封死了 ADMIN/DEVELOPER 成员

  • 位置internal/logic/user/bindRolesLogic.go 第 60-82 行
  • 描述

    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("不能分配权限级别高于自身的角色")
          }
      }
    }
    

    这段 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 的角色都会被判为"权限级别高于自身"而拒绝绑定

    对比 checkPermLevel(access.go 第 143-151 行)的设计:只有在 callerPri == targetPri(都是 MEMBER)时才会进入 permsLevel 比较,而 ADMIN/DEVELOPER 因 MemberType 优先级更高就已经放行。bindRolesLogic 的这段校验缺失了这一前置判定。

    • 影响
    • 在真实产线,ADMIN 成员(包括系统自动创建的 admin_{code}无法给任何用户绑定任何角色,管理员最核心的运营能力被封死。
    • DEVELOPER 成员同样被封死。
    • 该 bug 在测试中未暴露,是因为 bindRolesLogic_test.go 第 314 行人为构造了 MinPermsLevel: 50 的 ADMIN 上下文(见 TC-0208),并不反映 loader 真实产出的 math.MaxInt64

    • 修复方案:与 checkPermLevel 对齐,只对同为 MEMBER 类型的调用者做 permsLevel 比较;对 ADMIN/DEVELOPER 直接放行:

    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("不能分配权限级别高于自身的角色")
      }
    }
    

同时,caller.MinPermsLevel == 0 这段无效判断应改为 caller.MinPermsLevel == math.MaxInt64(或按上面的写法从条件里移除)。


H-2. gRPC GetUserPerms 未校验用户状态,冻结用户仍被下发全量权限

  • 位置internal/server/permserver.go 第 191-216 行
  • 描述

    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, "用户不存在")
      }
      return &pb.GetUserPermsResp{
          MemberType: ud.MemberType,
          Perms:      ud.Perms,
      }, nil
    }
    

    对比同一文件的 VerifyToken(第 157-189 行),VerifyToken 在返回前校验了 ud.Status == StatusEnabledMemberType != ""。但 GetUserPerms 完全没有这两层过滤,仅判断了用户是否存在。

    结果:当管理后台调用 UpdateUserStatus 将用户冻结后:

    • sys_user.status = 2 (Disabled)tokenVersion +1
    • userDetailsLoader.Clean 清理缓存
    • 下一次 Load 从 DB 读出 ud.Status = 2,但 ud.Perms 依然根据 loadPerms 逻辑完整计算
    • 产品服务器通过 gRPC GetUserPerms 查询,仍然会拿到完整的权限列表

    • 影响

    • 被冻结的用户在接入方(产品服务)一侧仍然具备全部权限。虽然浏览器侧因 tokenVersion 变化 access token 已失效,但如果接入方自己缓存了 userId → perms 的映射并据此鉴权,用户可以继续获得访问。

    • 同样地,sys_product_member.status = Disabled 的用户,如果其部门类型为 DEVloadPerms 依然会返回全量权限(详见 H-3)。

    • 形成"本系统侧已冻结,但对外仍判定有权限"的一致性漏洞。

    • 修复方案:对齐 VerifyToken 的校验逻辑:

    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, "用户已不是该产品的成员")
    }
    

H-3. DEV 部门用户可绕过"产品成员禁用"继续获得全量权限

  • 位置internal/loaders/userDetailsLoader.go 第 347-363 行(loadPerms)、internal/middleware/jwtauthMiddleware.go 第 79-86 行
  • 描述loadPerms 在判断"自动获得全量权限"时使用的是 OR 逻辑,其中部门类型判定独立于产品成员状态:
  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
  }

loadMembership 针对被禁用的成员只做了一件事:跳过 ud.MemberType 赋值(即 MemberType = ""),并不回退/阻断后续的部门判定

同时,jwtauthMiddleware.Handle 仅校验了 ud.Usernameud.Statusclaims.TokenVersion,没有校验产品成员是否被禁用(不像 RefreshToken 会阻断 ud.MemberType == "" 的场景)。

复现链路:

  1. 用户 U 属于研发部门(deptType=DEVdeptStatus=Enabled),在产品 P 作为 MEMBER 登录获得 accessToken
  2. 管理员通过 UpdateMember 将 U 在 P 的成员资格 Status 改为 Disabled
  3. 缓存被 Del,下次请求 loader 从 DB 重算
  4. loadMembershipmember.Status != Enabled 直接 returnMemberType = ""
  5. loadPerms 匹配第 4 个条件 DeptType == DEV && DeptStatus == Enabled依然返回全量权限
  6. 中间件不校验 MemberType,放行请求 → 用户携带冻结后的 membership 继续访问
  • 影响:产品管理员通过"禁用产品成员"无法真正撤销 DEV 部门用户对该产品的访问;必须同时修改用户部门,才能剥夺其权限。与接口语义不一致,是真实的水平越权旁路。

  • 修复方案loadPerms 的 DEV 部门短路应叠加一个"用户是该产品成员且成员状态启用"的前置条件;或在 loader 中记录 memberStatus 字段,并在 DEV 部门判定里做交集:

  // 方案: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 {
      ...
  }

同时建议在 jwtauthMiddleware 或在业务层对 ud.MemberType == "" && !IsSuperAdmin && productCode != "" 场景也返回 401/403(与 refreshTokenLogic 的第 53 行保持一致)。


⚠️ 健壮性与性能建议 (Medium)

M-1. 自动生成的产品管理员密码熵过低(仅 32 bits)

  • 位置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)。

虽然 mustChangePassword=Yes 会强制首次登录改密,但在管理员拿到 CreateProductResp 到其首次登录的时间窗口内,该密码仍可被暴力破解(尤其管理后台目前已经按用户名限流 5min/10 次,基本可挡住穷举,但设计上不应依赖外部限流来保护一个 32 bit 的秘密)。

  • 建议

    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
    }
    

    这样 generateRandomHex(8) 就真正提供 8 个 hex 字符 = 4 字节实熵,再考虑到一次性临时密码,可以直接把 adminPassword 生成改为 generateRandomHex(16)(16 hex 字符 ≈ 64 bits)。


    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 (?,?,?)

    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
      }
    }
    

    M-3. UserDetailLogic 在"超管 + 产品上下文"场景下返回跨产品的 roleIds

    • 位置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,就按产品过滤

    if productCode != "" {
      roleIds, err = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserIdForProduct(l.ctx, user.Id, productCode)
    } else {
      roleIds, err = l.svcCtx.SysUserRoleModel.FindRoleIdsByUserId(l.ctx, user.Id)
    }
    

    M-4. FindRoleIdsByUserIdForProduct 未过滤角色状态,返回含已禁用角色

    • 位置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 都需要基于"启用角色"做判定。

后续在 userDetailsLoader.loadRoles(第 314-344 行)通过内存过滤 r.Status == StatusEnabled 做了二次兜底,因此用于权限计算的路径是正确的。但:

  1. UserDetail 接口直接把这批 roleIds 返回给前端(含禁用角色)。
  2. bindRolesLogic 的 "existingRoleIds diff 逻辑"(第 85-110 行)会把已禁用的旧关联当作"存在",只有当请求里明确包含了该禁用角色时才会保留,否则会被 toRemove 删除 —— 表现为"重新绑定时,用户原本禁用的旧角色会被清掉"。这个行为从业务语义上是可接受的(禁用角色本就不应再绑定),但与 SQL 字面不一致,容易误解。
  • 建议:要么在 SQL 里显式 AND r.status = 1,要么在命名上明确包含禁用(如 FindAllRoleIdsByUserIdForProduct)。推荐前者:
  WHERE ur.userId = ? AND r.productCode = ? AND r.status = 1

M-5. UpdateDept 对"deptType 字段无变化"时也会级联清空子部门用户缓存

  • 位置internal/logic/dept/updateDeptLogic.go 第 77-89 行
  • 描述:条件是 if req.DeptType == DeptTypeNormal || req.DeptType == DeptTypeDev,只要请求带了合法的 deptType,就会执行 FindByPathPrefix 然后挨个 Clean 子部门用户缓存;但并没有比较 req.DeptType 与当前 dept.DeptType 是否真的不同。

另一层问题:如源代码注释所说,loadPerms 只看用户自身部门的 deptType/status,子部门用户并不受父部门 deptType 变化影响。所以从权限计算的正确性来看,根本不需要级联清理子部门用户缓存;只清理当前部门直属用户已经足够。

  • 建议:去掉级联清理,或收窄到"deptType 真的发生变化时仅清理自身部门用户"。代码可简化为:
  userIds, _ := l.svcCtx.SysUserModel.FindIdsByDeptId(l.ctx, req.Id)
  for _, uid := range userIds {
      l.svcCtx.UserDetailsLoader.Clean(l.ctx, uid)
  }

子部门级联逻辑可以删除。


M-6. 僵尸字段:JWT Claims 中的 Perms 从未被产线代码写入

  • 位置internal/middleware/jwtauthMiddleware.go 第 23-32 行
  • 描述middleware.Claims 结构体保留了 Perms []string 字段:

    type Claims struct {
      ...
      Perms []string `json:"perms,omitempty"`
      jwt.RegisteredClaims
    }
    

    GenerateAccessTokeninternal/logic/auth/jwt.go 第 23-39 行)没有给 Perms 赋值,产线签出的 JWT 永远不带该字段。仅测试 (jwt_test.go)、README 出现过引用。属于设计未落地或已重构遗留的字段。

    • 建议:确认是否还需要(例如未来把 perms 塞进 token 做无状态鉴权);不再需要则移除 Perms 字段,避免维护歧义。

    M-7. X-Real-IP 信任策略过于简单,且不支持标准 X-Forwarded-For

    • 位置internal/middleware/ratelimitMiddleware.go 第 41-52 行
    • 描述go if behindProxy { if ip := r.Header.Get("X-Real-IP"); ip != "" { return ip } }

问题:

  1. 仅支持 X-Real-IP,未兼容更通用的 X-Forwarded-For(多数 K8s Ingress、ELB 默认设置的是 XFF)。
  2. 只要 behindProxy=true,任何 X-Real-IP 都无条件信任;如果反向代理没有正确覆盖客户端传入的头,攻击者可以伪造 IP 规避限流。
  3. XFF 是多段逗号分隔,需要按最右可信节点反向取值;本实现一旦支持 XFF 必须小心这一点。
  4. 建议
    • 同时兼容 X-Forwarded-For,取其中未被你控制的最右侧一段作为客户端 IP。
    • 把"可信代理 CIDR 列表"配置化:只有来自可信代理网段的请求才信任 header,其他直接使用 RemoteAddr
    • 如果暂时不打算加复杂度,至少把 XFF 支持补上,且确保部署文档强调反向代理必须覆盖这些 header。

M-8. DeptTree 对所有登录用户开放,暴露完整组织架构

  • 位置internal/logic/dept/deptTreeLogic.gointernal/handler/routes.go 第 42-69 行
  • 描述:路由仅通过 JwtAuth 中间件保护,DeptTreeLogic 自身完全不做权限过滤,任何通过 JWT 的用户(包括产品端 MEMBER)都能拉到全公司组织架构。
  • 影响:组织架构通常包含内部部门命名、层级关系、部门类型(NORMAL/DEV 可暗示岗位属性),属于内部敏感信息,不应暴露给产品端普通成员。

  • 建议:根据业务定位决定:

    • 严格方案:仅超管可拉全量树;普通成员只能拿自身部门及下级子部门(strings.HasPrefix(d.Path, caller.DeptPath))。
    • 宽松方案:超管和任意 ADMIN/DEVELOPER 可见;MEMBER 只见自身部门节点。

M-9. ProductList 对所有登录用户返回全量产品列表

  • 位置internal/logic/product/productListLogic.go
  • 描述:任何 JWT 通过的用户都能列出系统全部产品(产品编码、名称、状态、创建时间),非超管会隐藏 AppKey,但产品列表本身仍然对全员可见。其他产品的普通成员也能"看见"存在哪些其他产品。
  • 影响:产品信息属于租户元数据,跨产品可见会让攻击者进行产品横向探测(例如发现有"内部管理后台"、"支付中心"等高价值目标并针对性登录)。
  • 建议
    • 仅超管可列全部;其他成员只能看到自己是成员的产品(通过 FindListByUserId 过滤)。

M-10. 缓存失效与 DB 事务不原子:Clean 失败静默吞错

  • 位置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 层增加"失败时重试一次或返回错误"的能力,让上层可以决定是否对外暴露失败。

M-11. DeleteDept 的前置校验与实际删除之间存在 TOCTOU 竞态

  • 位置internal/logic/dept/deleteDeptLogic.go
  • 描述:流程依次执行:
    1. FindByParentId(id) 校验无子部门
    2. FindIdsByDeptId(id) 校验无关联用户
    3. Delete(id)

步骤 2 与步骤 3 之间,另一个管理员可能:

  • 创建以该部门为父的子部门(CreateDept
  • 把某用户迁入该部门(UpdateUser.DeptId

由于 sys_dept 没有对被删除部门的外键约束,删除会成功,留下"孤儿"子部门或"指向已删除部门的用户"。虽然都是超管才能执行、并发概率极低,但依然是逻辑一致性缺口。

  • 建议
    • 方案一:把三步放进一个事务,并对 sys_dept 行加 SELECT ... FOR UPDATE
    • 方案二:采用"逻辑删除"代替物理删除。
    • 方案三:在 CreateDept / UpdateUser 的写入端校验目标部门 status=Enabled 且未被删除。

M-12. AddMember / BindRoles 的权限校验依赖 caller 的 ProductCode(JWT),但可操作的是 req.ProductCode

  • 位置internal/logic/member/addMemberLogic.gointernal/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 产品要分配的级别",语义错配。

目前由于 RequireProductAdminFor 要求 caller.MemberType == ADMIN && caller.ProductCode == req.ProductCode,非超管的跨产品路径已经被卡住;只有超管能跨产品,而超管在 CheckMemberTypeAssignment 里直接 return nil。所以当前没有实际越权风险

  • 建议:这属于"防御性冗余"——未来如果放宽 RequireProductAdminFor 的跨产品规则(例如允许全局 ADMIN),CheckMemberTypeAssignment 的语义就会失效。建议把 CheckMemberTypeAssignment 显式接收 productCode,内部按目标产品重新读 caller 在该产品的 MemberType:
  func CheckMemberTypeAssignmentFor(ctx, svcCtx, productCode, assignedType) error

📝 低风险 / 遗留问题 (Low)

L-1. 敏感配置明文提交到仓库(遗留)

  • 位置etc/perm-api-dev.yamletc/perm-api-prod.yamletc/perm-api-test.yamletc/perm-api-xiaom.yaml
  • 描述:MySQL/Redis 密码、AccessSecret、RefreshSecret、ManagementKey 等均以明文形式存在,且已提交至 Git。即使后续轮换,历史提交仍留痕。
  • 建议:改用环境变量或密钥管理服务注入,etc/*.yaml 只保留模板。

L-2. 登录限流桶的维度混用

  • 位置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:productrl:login:admin),并可适度提高 product 端桶的配额。

L-3. UpdateDept 整行回写,未使用乐观锁

  • 位置internal/logic/dept/updateDeptLogic.go 第 42-65 行
  • 描述:仍是 Read-Modify-Write 模式,没有 updateTime 条件乐观锁(不像 UpdateUser 已用 WHERE updateTime=?)。部门操作是超管串行动作,并发概率极低,风险有限。
  • 建议:若希望统一范式,与 UpdateProfile 一样接上 WHERE updateTime = ? 的乐观锁,失败返回 ErrConflict

L-4. UpdateRole 允许 ADMIN 任意调整 permsLevel(设计层面)

  • 位置internal/logic/role/updateRoleLogic.go
  • 描述:产品 ADMIN 可以把一个角色的 permsLevel 从 500 改成 1,并绑定给普通 MEMBER,以此让该 MEMBER 的 MinPermsLevel = 1,进而绕过 checkPermLevel 的等级约束。由于 ADMIN 本身已经是高级别角色并拥有该产品全部权限,实际并未提升其权力范围;但"下放"级别的能力会让普通 MEMBER 能管理更多同级用户。
  • 建议:如希望 permsLevel 成为"不可越级下放"的强约束,可要求修改 permsLevel 到低于某个阈值时必须是超管。当前风险较小。

L-5. singleflight 在 Load 失败路径仍返回带零值的 UserDetails

  • 位置internal/loaders/userDetailsLoader.go 第 106-134 行
  • 描述:当 loadFromDBok == false(例如用户不存在)时,仍然把 ud 返回给 sf.Do 调用方;随后 Load 的 caller 通过 ud.Username == "" 判断。目前中间件/登录路径都有后续的 Username == "" 检查,因此不会被滥用。建议在 Load 内部直接 return nil(不缓存、不复用),让上游直接看到 nil 语义,减少误用风险。

L-6. FindAllCodesByProductCode 仅按 status 过滤,依赖 UNIQUE (productCode, code) 去重

  • 位置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,需要注意历史禁用记录的去重。当前无风险,仅作提醒。

📋 审计总结

维度 评估
逻辑一致性 发现两个严重逻辑 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)。

修复优先级建议

  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 级遗留项。