Commit d24ca71bc67213c038fa6088168903aa039f443f
1 parent
26523d4f
docs(review:REQ-USR-004:r1): approve
Showing
1 changed file
with
63 additions
and
0 deletions
docs/superpowers/reviews/2026-06-01-REQ-USR-004.md
0 → 100644
| 1 | +# REQ-USR-004 登录用户 — AI 代码自审（review, round 1） | |
| 2 | + | |
| 3 | +> 关联 spec：`docs/superpowers/specs/2026-06-01-REQ-USR-004.md` | |
| 4 | +> 阶段：后端（backend）。审查维度 = 通用代码审查（正确性 / 边界 / 错误处理 / 一致性 / 分层 / 安全）。 | |
| 5 | +> 裁决：**approve** | |
| 6 | +> 生成时间：2026-06-01。 | |
| 7 | + | |
| 8 | +## 1. 审查范围 | |
| 9 | + | |
| 10 | +本轮针对 `cb7705e..26523d4`（REQ-USR-004 引入的全部 commit）做 diff 级自审，覆盖： | |
| 11 | + | |
| 12 | +- 生产代码：`UsrAuthController` / `UsrAuthService` / `UsrAuthServiceImpl` / `LoginDTO` / `LoginVO` / `CompanyOptionVO` / `UsrCompany` / `UsrCompanyMapper` / `ResultCode`（新增 `42901`）/ `SecurityConfig`（放行 `/api/usr/companies`）/ `application.yml`（jwt + auth.login 配置）。 | |
| 13 | +- 测试代码：`UsrLoginIT`（12 端到端）、`UsrAuthServiceImplTest`（9）、`UsrAuthControllerTest`（6）、`LoginDTOValidationTest`、`LoginVOJsonTest`、`CompanyOptionVOJsonTest`、`UsrCompanyMapperTest`、`ResultCodeLoginTest`、`AuthLoginConfigIT`、`SecurityConfigTest`。 | |
| 14 | + | |
| 15 | +## 2. 裁决依据（逐维度） | |
| 16 | + | |
| 17 | +### 2.1 计划一致性（plan-alignment）— 通过 | |
| 18 | + | |
| 19 | +- 认证判定顺序与 spec § 3 规则 1 / § 8 D3 完全一致：①限流窗（命中 → `42901`）→ ②按 `sUserName(trim)` 查用户，null → `40101` → ③BCrypt 比对，不匹配 → `40101` → ④`iIsVoid=1` → `40302`（先验密码再判禁用）→ ⑤`companyId` 存在性 → `40001` → ⑥签发 JWT + 更新 `tLastLoginDate` + 清零计数。 | |
| 20 | +- 防账号枚举：用户不存在与密码错误均抛 `ResultCode.UNAUTHORIZED`（同码 `40101` 同 message），调用方无法区分（AC4/AC5）。 | |
| 21 | +- 禁用账号不计入失败计数（`recordFailure` 仅在 ②③ 调用，④不调用），避免锁死禁用账号——与 D3 意图一致。 | |
| 22 | +- `tLastLoginDate` 采用「仅 id + 时间」的部分更新（`loginTimeUpdate` 只 set 主键与时间），不触碰其他列（规则 7）。 | |
| 23 | +- 写操作置于 `@Transactional(rollbackFor = Exception.class)`；`listCompanies` 标 `@Transactional(readOnly = true)`（D6 / 规则 10）。 | |
| 24 | + | |
| 25 | +### 2.2 正确性 / 边界 / 错误处理 — 通过 | |
| 26 | + | |
| 27 | +- `getSUserName` 上 `@JsonProperty("sUserName")` 正确锁定契约 JSON 键，规避 Jackson 对匈牙利前缀 getter 的大驼峰推断（与 `CreateUserDTO`/`UserVO` 同做法）。 | |
| 28 | +- `LoginDTO` 三字段 `@NotBlank`/`@NotNull` + `@Size` 校验齐备，失败经 `GlobalExceptionHandler` 统一转 `40001`（AC7）。 | |
| 29 | +- `iIsVoid` 判定带 null 防护（`!= null && == 1`）。 | |
| 30 | +- 限流窗过期自动重置（`isLocked` 内 `lockUntilEpochSec <= now` 时 `attempts.remove`），成功登录 `attempts.remove` 清零（AC9）。 | |
| 31 | +- `companyId` 非法在密码校验通过后才判，归 `40001`（AC8），不泄露认证维度信息。 | |
| 32 | + | |
| 33 | +### 2.3 安全 — 通过 | |
| 34 | + | |
| 35 | +- 密码经 `PasswordEncoder.matches` 比对，无明文存取；`LoginVO` 不含 `sPassword`，无 `SELECT *` 透传实体。 | |
| 36 | +- 密码明文不进日志 / 异常 message（`BusinessException` 仅携带 `ResultCode` 文案）。 | |
| 37 | +- JWT 密钥取自配置（`jwt.secret`，env 注入），令牌含 `exp`（`JwtUtil.generateToken` 设 expiration），claim 含 subject + `sUserType`（AC12 / 规则 4）。 | |
| 38 | +- `SecurityConfig.PERMIT_ALL_PATHS` 追加 `/api/usr/companies`，登录前可访问，且抽常量供单测断言防漂移（AC10）。 | |
| 39 | + | |
| 40 | +### 2.4 分层 / 架构 / 一致性 — 通过 | |
| 41 | + | |
| 42 | +- Controller 仅 `@Valid` + 委派，不碰 Mapper / 业务逻辑；Service 持有逻辑；Mapper 走 MyBatis-Plus `BaseMapper`，参数化查询防注入。 | |
| 43 | +- 错误码集中于 `ResultCode`（新增 `42901 LOGIN_RATE_LIMITED`）。 | |
| 44 | +- 端点路径 / 请求体 / 响应体字段名（`token`、`user{ id, sUserName, sUserType, sLanguage }`）与 `docs/05 § REQ-USR-004` 一致。 | |
| 45 | +- 包路径符合 spec § 4（`modules/usr/{controller,service,service.impl,mapper,entity,dto,vo}`），未跨业务模块。 | |
| 46 | + | |
| 47 | +### 2.5 测试 — 通过 | |
| 48 | + | |
| 49 | +verify 证据（`2026-06-01-REQ-USR-004-verify.md`）：JDK 21 下 `mvn -B test` = 125/125 通过、0 失败 / 0 错误 / 0 跳过、BUILD SUCCESS。AC1–AC12 由 `UsrLoginIT` 端到端逐条覆盖，Service / Controller / DTO / VO / Mapper / 安全配置 / 限流配置均有单元或集成断言。 | |
| 50 | + | |
| 51 | +## 3. 非阻塞建议（verbal，不计入 must-fix） | |
| 52 | + | |
| 53 | +1. **限流计数与事务非原子**：`recordFailure` / `attempts.remove` 为进程内内存副作用，不随 `@Transactional` 回滚。但失败计数仅在抛异常路径（②③）发生，此时事务内无其他已提交副作用；成功路径的 `attempts.remove` 在更新提交前执行，若 `tLastLoginDate` 更新回滚则计数已被清零——属可接受的极小窗口，且 D7 已声明为 MVP 限制（后续可平滑替换 Redis）。建议后续将限流移出事务方法或改为 Redis 原子计数。 | |
| 54 | +2. **契约反向同步**：`42901` 限流码与 `GET /api/usr/companies` 端点尚未补入 `docs/05`，spec § 8 D1/D7 已将其列为非强制反向同步建议，建议择机补齐保持 SSoT 完整。 | |
| 55 | +3. **限流维度**：当前按 `sUserName` 计数，分布式部署下进程内计数不共享；与 D7 一致，留待 Redis 化时一并处理。 | |
| 56 | + | |
| 57 | +以上均为改进性建议，无客观可定位缺陷，不构成 must-fix。 | |
| 58 | + | |
| 59 | +## 4. 结论 | |
| 60 | + | |
| 61 | +- 计划一致性 / 正确性 / 边界 / 错误处理 / 安全 / 分层 / 一致性 / 测试 全部通过。 | |
| 62 | +- 未发现客观、可验证的阻断性缺陷。 | |
| 63 | +- 裁决：**approve**，`issues = []`。 | ... | ... |