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

> 审计范围：`/internal` 下全部非测试生产代码（logic、model、middleware、handler、loaders、server、svc、config、consts、response、util）及入口文件 `perm.go`、gRPC 客户端 `permclient/`。
> 审计时间：2026-04-18

---

## 🚩 核心逻辑漏洞 (High Risk)

---

### H-1. UpdateUser 存在 Read-Modify-Write 竞态，可静默撤销 tokenVersion 安全递增

- **位置**：`internal/logic/user/updateUserLogic.go` 第 47-112 行
- **描述**：`UpdateUser` 先通过 `FindOne` 读取完整用户行（包含 `tokenVersion`），在内存中修改字段后调用通用 `Update()` 回写**全部字段**。`Update()` 对应的 SQL 形如 `UPDATE sys_user SET ... tokenVersion=?, ... WHERE id=?`，会将内存中的 tokenVersion 值**覆盖**到数据库。

  然而 `UpdatePassword` 和 `UpdateStatus` 使用的是原子 SQL：
  ```sql
  UPDATE sys_user SET tokenVersion = tokenVersion + 1 WHERE id = ?
  ```
  如果在 `UpdateUser` 的 `FindOne` 和 `Update` 之间，另一个请求执行了 `UpdatePassword` 或 `UpdateStatus`，原子递增的 tokenVersion 会被 `UpdateUser` 的陈旧值**静默覆盖回去**。

- **影响**：
  - 场景复现：用户 A 修改密码（tokenVersion 5→6，所有旧 session 应失效）。几乎同时，管理员修改该用户昵称（读到 tokenVersion=5，写回 tokenVersion=5）。结果数据库 tokenVersion 被还原为 5，用户旧 session（tokenVersion=5）仍然有效。
  - **密码修改后的安全失效机制被绕过**，攻击者持有旧 token 仍可继续访问系统。

- **修复方案**：`UpdateUser` 应改为**部分字段更新**，不触碰 `tokenVersion`、`password` 等安全敏感字段。如果需要修改 status 并递增 tokenVersion，应使用与 `UpdateStatus` 相同的原子 SQL，或在事务中使用 `SELECT ... FOR UPDATE` 锁定行。

  ```go
  // 方案一：拆分为部分更新 SQL
  func (m *customSysUserModel) UpdateProfile(ctx context.Context, id int64, fields map[string]interface{}) error {
      // 动态构建 SET 子句，仅更新传入字段，不覆盖 tokenVersion/password
  }

  // 方案二：如果更新 status 需要递增 tokenVersion，单独调用 UpdateStatus
  if req.Status != 0 && user.Status != req.Status {
      if err := l.svcCtx.SysUserModel.UpdateStatus(l.ctx, req.Id, req.Status); err != nil {
          return err
      }
  }
  ```

---

### H-2. AdminLogin 缺少按用户名维度的频率限制，暴力破解风险高于产品端

- **位置**：`internal/logic/pub/adminLoginLogic.go`、`internal/handler/routes.go` 第 174-188 行
- **描述**：产品端登录 `ValidateProductLogin` 在 `loginService.go` 中使用了 `UsernameLoginLimit`（每用户名 300 秒 10 次），但管理后台登录 `AdminLogin` 只经过 IP 维度的 `LoginRateLimit`（每 IP 60 秒 20 次），没有按用户名限频。
- **影响**：攻击者通过代理池轮换 IP，可对超级管理员账号进行无限次暴力破解，而 IP 维度的限流完全无法防御此场景。考虑到管理后台登录还需要提供 `managementKey`，实际风险有所降低，但密钥和密码的双重暴力破解仍然可行。
- **修复方案**：在 `AdminLogin` 中复用或新增按用户名维度的频率限制器：

  ```go
  func (l *AdminLoginLogic) AdminLogin(req *types.AdminLoginReq) (*types.LoginResp, error) {
      if l.svcCtx.UsernameLoginLimit != nil {
          code, _ := l.svcCtx.UsernameLoginLimit.Take(req.Username)
          if code == limit.OverQuota {
              return nil, response.ErrTooManyRequests("该账号登录尝试过于频繁，请5分钟后再试")
          }
      }
      // ... 原有逻辑
  }
  ```

---

### H-3. RefreshToken 与 Login 共享同一速率限制桶，正常刷新可导致登录被锁

- **位置**：`internal/handler/routes.go` 第 174-201 行
- **描述**：`refreshToken` 路由与 `login`、`adminLogin` 共用 `LoginRateLimit` 中间件（基于 IP，60 秒 20 次）。RefreshToken 是 access token 过期后的常规续签操作，调用频率远高于登录。

- **影响**：
  - 如果前端在 access token 过期时自动调用 refreshToken，一个 IP 下有多个用户时（如办公网络 NAT 出口），刷新请求很快耗尽配额，导致同 IP 下所有用户无法登录。
  - 反过来，攻击者可以通过大量发送 refreshToken 请求来消耗某个 IP 的登录配额，造成该 IP 下的所有用户无法登录（拒绝服务）。

- **修复方案**：为 `refreshToken` 使用独立的速率限制器，或直接移除其 IP 限流（因为 refreshToken 自身已有 JWT 签名验证和 tokenVersion 校验保护）：

  ```go
  // servicecontext.go 中新增
  refreshRlMiddleware := middleware.NewRateLimitMiddleware(
      rds, 60, 60, c.CacheRedis.KeyPrefix+":rl:refresh", c.BehindProxy,
  )
  ```

---

### H-4. UpdateProduct 状态校验静默忽略无效值

- **位置**：`internal/logic/product/updateProductLogic.go` 第 49-51 行
- **描述**：
  ```go
  if req.Status == consts.StatusEnabled || req.Status == consts.StatusDisabled {
      product.Status = req.Status
  }
  ```
  当传入 `status=3` 或 `status=-1` 等无效值时，代码**静默忽略**，不更新也不报错。而 `updateRoleLogic`、`updateUserLogic`、`updateDeptLogic`、`updateMemberLogic` 对相同场景都会返回 400 错误。

- **影响**：接口行为不一致。调用方传入非法状态值后收到成功响应，误以为状态已修改，实际上并未生效。在前端管理界面可能导致状态显示与实际不符。

- **修复方案**：与其他更新接口保持一致，显式校验并报错：

  ```go
  if req.Status != 0 {
      if req.Status != consts.StatusEnabled && req.Status != consts.StatusDisabled {
          return response.ErrBadRequest("状态值无效，仅支持 1(启用) 和 2(禁用)")
      }
      product.Status = req.Status
  }
  ```

---

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

---

### M-1. UpdateUserStatus 对同一用户产生 3 次数据库读取

- **位置**：`internal/logic/user/updateUserStatusLogic.go`
- **描述**：请求处理链路中对同一 userId 执行了 3 次 `FindOne`：
  1. 第 40 行 `SysUserModel.FindOne` —— 检查用户是否存在、是否超管
  2. 第 56 行 `CheckManageAccess` → `checkDeptHierarchy` 内部又调用 `SysUserModel.FindOne`（第 115 行 `access.go`）
  3. 第 62 行 `SysUserModel.UpdateStatus` 内部再次调用 `FindOne`（用于构建缓存 key）

  虽然 go-zero 的 model 层有 cache，第 2-3 次读取会命中缓存，但仍有序列化/反序列化开销和额外的网络往返（如果 Redis 部署在远端）。

- **建议**：将首次查询的结果传递到后续函数中复用，或让 `UpdateStatus` 接收 username 参数以跳过内部 FindOne。

---

### M-2. gRPC 客户端使用已弃用的 `grpc.WithInsecure()`

- **位置**：`permclient/permclient.go` 第 17 行
- **描述**：`grpc.WithInsecure()` 自 gRPC v1.53 起已被弃用，推荐使用 `grpc.WithTransportCredentials(insecure.NewCredentials())`。
- **建议**：

  ```go
  import "google.golang.org/grpc/credentials/insecure"

  conn, err := grpc.NewClient(target, grpc.WithTransportCredentials(insecure.NewCredentials()))
  ```

---

### M-3. 配置文件包含明文敏感信息并提交到仓库

- **位置**：`etc/perm-api-dev.yaml`、`etc/perm-api-prod.yaml`、`etc/perm-api-test.yaml`、`etc/perm-api-xiaom.yaml`
- **描述**：数据库密码、Redis 密码、JWT 签名密钥、ManagementKey 等敏感信息以明文形式存储在 YAML 配置文件中，且已提交到 Git 仓库。
- **影响**：如果仓库泄露或被分享，所有凭据将暴露。即使后续更改密码，Git 历史中仍保留旧值。
- **建议**：使用环境变量注入或密钥管理服务（如 Vault）管理敏感配置。至少在 `.gitignore` 中排除含真实密钥的配置文件，仅保留模板文件。

---

### M-4. SyncPerms 接口未限制权限数组大小

- **位置**：`internal/logic/pub/syncPermsService.go` 第 42 行
- **描述**：`ExecuteSyncPerms` 对传入的 `perms` 数组长度没有上限检查。产品客户端可以一次性发送极大的权限列表，导致：
  - 数据库事务中执行大量 INSERT/UPDATE
  - 内存中构建大量对象
  - 事务持锁时间过长
- **建议**：增加上限检查，如 `if len(perms) > 5000 { return error }`。

---

### M-5. 僵尸代码：`GetUserPerms` 函数及其参数

- **位置**：`internal/logic/auth/perms.go`
- **描述**：`GetUserPerms` 函数接收 `deptId` 和 `isSuperAdmin` 两个参数，但函数体内**完全未使用**这两个参数，仅转发给 `UserDetailsLoader.Load()`。且该函数在所有生产代码中**从未被调用**，仅在测试文件中使用。
- **建议**：如果此函数预留用于未来 gRPC 接口，应更新签名移除未使用参数；否则应删除。

---

### M-6. 僵尸代码：多个 Middleware Context Helper 函数

- **位置**：`internal/middleware/jwtauthMiddleware.go` 第 110-136 行
- **描述**：以下函数在生产代码中**从未被调用**（仅在 testutil 中使用）：
  - `GetUsername(ctx)` —— 生产代码中直接使用 `GetUserDetails(ctx).Username`
  - `GetMemberType(ctx)` —— 同上
  - `IsSuperAdmin(ctx)` —— 生产代码中直接用 `caller.IsSuperAdmin`
- **建议**：如果这些函数不打算作为公开 API 暴露给外部包使用，应考虑移除以减少维护负担。

---

### M-7. 僵尸代码：多个 Model 层函数仅被测试代码调用

- **位置**：各 model 包的自定义 Model 文件
- **描述**：以下接口方法在生产代码中从未被调用，仅存在于测试或 mock 中：

  | 函数 | 所在包 | 说明 |
  |------|--------|------|
  | `DeleteByUserId` | userrole, userperm | 生产代码全部使用带 `ForProduct` 后缀的变体 |
  | `DeleteByUserIdForProduct`（非 Tx 版本） | userrole, userperm | 生产代码全部使用 `Tx` 事务版本 |
  | `DeleteByRoleId`（非 Tx 版本） | roleperm | 生产代码使用 `DeleteByRoleIdTx` |
  | `DisableNotInCodes`（非 Tx 版本） | perm | 生产代码使用 `DisableNotInCodesWithTx` |
  | `FindAllByProductCode` | perm | 生产代码使用 `FindAllCodesByProductCode` |
  | `FindListByDeptIds` | user | 从未在任何 logic 中被调用 |
  | `FindByUserId` | userperm, userrole, productmember | 仅测试中使用 |
  | `FindPermIdsByUserIdAndEffect`（不含 ForProduct 后缀） | userperm | 生产代码使用 `ForProduct` 版本 |
  | `DeleteByUserIdTx`（不含 ForProduct） | userrole, userperm | 生产代码使用 `ForProductTx` 版本 |

- **建议**：这些函数可能是为了方便测试编写的辅助方法。如果确认不会对外暴露，可以标注注释说明用途；如果完全冗余，建议移除以保持接口精简。

---

### M-8. CreateProduct 缺少产品编码格式校验

- **位置**：`internal/logic/product/createProductLogic.go` 第 44-55 行
- **描述**：`CreateUser` 对 `username` 有严格的正则校验（`^[a-zA-Z0-9_]{2,64}$`），但 `CreateProduct` 对 `code` 仅校验了长度上限（64 字符），未校验格式。产品编码被广泛用作数据库 WHERE 条件和 Redis 缓存 key 的一部分。
- **影响**：如果传入包含特殊字符（如空格、中文、`/`、`:`）的产品编码，虽然不会导致 SQL 注入（参数化查询），但可能导致：
  - Redis key 格式混乱
  - 自动生成的管理员用户名 `admin_{code}` 不合法
  - 前端 URL 编码问题
- **建议**：增加正则校验：
  ```go
  var productCodeRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_-]{1,63}$`)
  if !productCodeRegexp.MatchString(req.Code) {
      return nil, response.ErrBadRequest("产品编码只能包含字母、数字、下划线和中划线，须以字母开头")
  }
  ```

---

### M-9. UpdateDept 修改 status 时未清理子部门用户的缓存

- **位置**：`internal/logic/dept/updateDeptLogic.go` 第 73-92 行
- **描述**：当修改部门 `status` 为禁用时，仅清理了该部门直属用户的缓存。子部门用户的缓存清理逻辑被包裹在 `if req.DeptType == "NORMAL" || req.DeptType == "DEV"` 条件中，仅当请求中显式传入了 `DeptType` 字段才会执行。
- **影响**：如果禁用一个 DEV 类型部门时只传入 `status=2` 而不传入 `deptType`，该部门下所有**直属**用户的缓存会被清理，但子部门的用户缓存不会。由于 `loadPerms` 只检查用户**自身**部门的 DeptType 和 Status，子部门用户不受父部门状态影响，所以当前行为在逻辑上是**正确的**。但代码意图不够清晰，建议增加注释说明为什么子部门不需要级联清理。

---

### L-1. UserList 在超管模式下无 productCode 时查询全量用户

- **位置**：`internal/logic/user/userListLogic.go` 第 56-62 行
- **描述**：当超管不传 `productCode` 时，执行 `FindListByPage` 查询**全量用户**。在用户量较大（如数千用户）时，虽然有分页，但 `COUNT(*)` 查询可能较慢（全表扫描，无 WHERE 条件）。
- **建议**：如果用户表数据量增长到万级以上，考虑为 `COUNT(*)` 增加缓存或近似计数。当前业务规模下此为低优先级。

---

### L-2. DeptTree 一次性加载所有部门到内存构建树

- **位置**：`internal/logic/dept/deptTreeLogic.go` 第 28 行
- **描述**：`FindAll` 一次性查询所有部门记录，在内存中构建树结构。对于权限系统的典型规模（几十到几百个部门），这是完全合理的做法。
- **建议**：无需优化。仅在此记录，如果未来部门数量异常增长，可考虑前端懒加载。

---

### L-3. BindRoles 和 BindRolePerms 采用全量删除后重新插入

- **位置**：`internal/logic/user/bindRolesLogic.go`、`internal/logic/role/bindRolePermsLogic.go`
- **描述**：绑定角色/权限时，事务中先 `DELETE` 该用户（或角色）在当前产品下的所有关联记录，再 `BatchInsert` 新记录。这种"先删后插"方式简洁可靠，适合关联数量不大（通常每用户 < 20 个角色，每角色 < 200 个权限）的场景。
- **建议**：当前实现合理。如果未来出现单角色绑定数百个权限的场景，可优化为差异更新（仅插入新增、删除移除）以减少写入量。

---

### L-4. createDeptLogic 在事务中执行两步操作以回填 Path

- **位置**：`internal/logic/dept/createDeptLogic.go` 第 68-93 行
- **描述**：创建部门时需要先 INSERT 获得自增 ID，再用 ID 构建 `path` 字段并 UPDATE。代码使用了事务保证两步操作的原子性，实现正确。
- **建议**：当前实现合理。如果追求极致优化，可考虑使用 UUID 或预分配 ID 来避免两步操作，但收益极小。

---

### L-5. permclient.GetUserPerms 缺少 AppKey/AppSecret 参数传递

- **位置**：`permclient/permclient.go` 第 58-63 行
- **描述**：
  ```go
  func (c *PermClient) GetUserPerms(ctx context.Context, userId int64, productCode string) (*pb.GetUserPermsResp, error) {
      return c.cli.GetUserPerms(ctx, &pb.GetUserPermsReq{
          UserId:      userId,
          ProductCode: productCode,
      })
  }
  ```
  gRPC 服务端 `GetUserPerms` 需要 `AppKey` 和 `AppSecret` 进行产品身份验证，但客户端封装函数未传递这两个参数，**请求必然失败**（返回 `Unauthenticated`）。
- **影响**：任何通过 `permclient.GetUserPerms` 发起的调用都无法通过服务端的认证校验。
- **修复方案**：
  ```go
  func (c *PermClient) GetUserPerms(ctx context.Context, appKey, appSecret string, userId int64, productCode string) (*pb.GetUserPermsResp, error) {
      return c.cli.GetUserPerms(ctx, &pb.GetUserPermsReq{
          AppKey:      appKey,
          AppSecret:   appSecret,
          UserId:      userId,
          ProductCode: productCode,
      })
  }
  ```

---

## 📋 审计总结

| 维度 | 评估 |
|------|------|
| **逻辑一致性** | 整体良好。UpdateProduct 状态校验是唯一的行为不一致点。 |
| **并发与竞态** | 发现 1 个严重的 Read-Modify-Write 竞态条件（H-1），涉及安全关键的 tokenVersion 字段。 |
| **资源管理** | 良好。所有数据库操作通过 go-zero 连接池管理，事务使用正确，无泄漏风险。 |
| **数据完整性** | 关键写操作（BindRoles、BindPerms、RemoveMember、DeleteRole、CreateProduct、SyncPerms）均在事务中执行，原子性有保障。 |
| **安全漏洞** | SQL 注入风险：无（全部参数化查询）。水平越权：已有完善的 CheckManageAccess 层级校验。发现管理后台暴力破解风险（H-2）和限流竞争问题（H-3）。 |
| **边界处理** | 对 nil、空值、可选字段的处理较为完善。`UserDetails` 的零值初始化合理。 |
| **数据库性能** | 存在可优化的冗余读取（M-1），但整体无 N+1 查询问题。列表接口批量查询+map 组装的模式正确。 |
| **僵尸代码** | 发现 1 个僵尸函数（M-5）、3 个未使用的 middleware helper（M-6）、10+ 个仅测试调用的 model 方法（M-7）。 |
| **接口契约** | gRPC 客户端 `GetUserPerms` 缺少必要参数（L-5），会导致调用方无法正常使用。 |