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