WorkBuddy
ca0db57f24
- 同步版本号：VERSION / backend pom / admin-web / miniapp 均升至 1.1.0 - 合并本轮代码审查修复（详见 docs/CODE-REVIEW-2026-09-24.md）： - 后端：微信登录真实实现 RealWechatLoginProvider、实名核验真实实现 RealIdentityVerificationProvider（本地强校验 + 可配置第三方 + fail-closed/fail-fast） - 照片直传落库端点 POST /me/photos/confirm（fileKey 归属校验 + 对象存在性校验后 savePhoto） - 刷新 Token Redis TTL 修正（getExpirationMs()*2 的 4h → refreshExpirationMs 的 14 天） - 管理端 token 改为 HttpOnly Cookie + silent-refresh，移除 localStorage 明文存储与 30+ 处手工注入 - 小程序：红娘接单鉴权头错位修复、serviceType 中文映射、生产域名可配置 - 隔离本地产物（.workbuddy/、*.bundle、*.dump、secrets、application-dev.yml 等已被 .gitignore 覆盖） 未验证项（打标签 ≠ 生产发布）： - 后端编译 / 单测未在本环境执行（Maven 慢、Docker DNS 受限） - 管理端 登录→刷新→登出 端到端、小程序真机走查待人工验收 - 生产部署需注入 WECHAT_APP_ID / WECHAT_APP_SECRET、VERIFICATION_API_URL / VERIFICATION_API_KEY 与小程序生产域名
179 lines
16 KiB
Markdown
179 lines
16 KiB
Markdown
# 善缘丹阳 代码审查报告
|
||
|
||
> 审查日期：2026-09-24
|
||
> 审查范围：后端（Java 21 / Spring Boot 3）、微信小程序（原生 + TS）、管理端（Vue 3 + Vite + Pinia）
|
||
> 审查视角：CodeReviewExpert — 正确性 / 安全 / 可维护 / 性能 / 测试
|
||
> 核验声明：下方 🔴 阻断 1–4 与 🟡 刷新 Token TTL 均由主审查员**直接读源码 + grep 复核**确认；其余条目为子代理带文件路径核查，结论可信但建议落地前再回读一次原文件。
|
||
|
||
---
|
||
|
||
## 一、总览
|
||
|
||
**总体印象**：本项目的工程纪律落地得相当扎实——幂等（`@Idempotent` + 唯一索引）、数据权限守卫（`DataScopeGuard` 真实接入多处）、WXML 结构守卫、请求层"先 resolve 再收尾"的时序守卫，都不是文档承诺而是真实代码。这是少见的"纪律工程化"水平。
|
||
|
||
**但存在 2 个已确认的上线阻塞**（与 UAT 走查结论一致）：微信登录无真实实现、照片直传不落库；此外发现若干安全/正确性缺陷，其中管理端 token 存储、小程序红娘接单鉴权头错位、刷新 Token TTL 误用会直接影响可用性与安全。
|
||
|
||
**优先级**：上线前必须关闭 🔴 1–4；🟡 建议同批修复。
|
||
|
||
---
|
||
|
||
## 二、🔴 阻断（必须修，否则不能上线）
|
||
|
||
### 1. 微信登录生产默认走 Mock —— 任意 code 可无限建号
|
||
`backend/.../auth/provider/MockWechatLoginProvider.java:19`
|
||
```java
|
||
@ConditionalOnProperty(name = "verification.provider", havingValue = "mock", matchIfMissing = true)
|
||
public class MockWechatLoginProvider implements WechatLoginProvider {
|
||
private final Map<String, String> codeToOpenid = new ConcurrentHashMap<>();
|
||
// :35
|
||
String openid = codeToOpenid.computeIfAbsent(code, k -> "mock_openid_" + UUID.randomUUID()...);
|
||
```
|
||
**为什么是问题**：`matchIfMissing = true` 意味着**未显式配置即启用 Mock**。`application-production.yml` 中并无 `verification.provider`（只有 logging/springdoc/security），故生产环境默认走 Mock。Mock 实现对任意 `code` 都生成随机 openid 并自动建号——攻击者可凭任意字符串无限注册假账号，且 `wechat.app-id` 默认 `dev-app-id`。这是实名/风控链路的根基性缺口。
|
||
|
||
**修复**：
|
||
- 新增 `RealWechatLoginProvider`（`@ConditionalOnProperty(... havingValue = "real")`），调用 `https://api.weixin.qq.com/sns/jscode2session` 用真实 `code` 换取 openid/session_key，并校验 appid/secret。
|
||
- 生产 `application-production.yml` 显式置 `verification.provider=real`，并**删除 `matchIfMissing=true`**（或仅留给本地 dev profile）。
|
||
- 接入 `code2session` 失败、空 openid、appid 不匹配的拒绝分支。
|
||
|
||
### 2. 照片直传不落库 —— 直传后无"确认/落库"端点
|
||
`backend/.../me/controller/MeController.java:227` 与 `photo/controller/PhotoController.java:32` 均暴露 `POST /.../photos/upload-token`，调用 `PhotoService.getUploadToken` 返回 MinIO **预签名 PUT URL + fileKey**（`PhotoService.java:75-86`）。但全仓**没有任何端点接收直传的 `fileKey` 并写 `user_photos`**：
|
||
- `savePhoto(...)` 仅被 `PhotoService.java:233-245` 的多段上传 `POST /photos/upload`（`MultipartFile` 后端代传）调用；
|
||
- `getSignedUrl`（`:96-99`）按 `user_photos` 校验归属：`boolean owned = userPhotos.stream().anyMatch(p -> fileKey.equals(p.getFileKey()))`。
|
||
|
||
**为什么是问题**：走直传路径时，对象进了 MinIO，但 `user_photos` 无记录 → `getPhotos` 不展示，**且 `getSignedUrl` 归属校验直接失败，连访问鉴权都过不了**。直传等于"传了白传"。这是已知阻塞的真实根因。
|
||
|
||
**修复**：新增 `POST /me/photos/confirm`（或 `POST /photos/confirm`），入参 `fileKey + photoType`，服务端校验 fileKey 属当前用户 + 对象已存在于 MinIO 后调用 `savePhoto`，并触发人工审核状态机。小程序侧直传成功后调用该确认端点。
|
||
|
||
### 3. 小程序红娘"接单"鉴权头错位 —— 请求实际无鉴权
|
||
`miniapp/utils/api.js:272`
|
||
```js
|
||
post(url, data, headers, idempotencyKey) { ... }
|
||
```
|
||
`miniapp/pages/matchmaker-workbench/matchmaker-workbench.js:85`
|
||
```js
|
||
api.post(`/operator/matchmaker/services/${serviceId}/claim`, {}, {}, { Authorization: `Bearer ${token}` })
|
||
```
|
||
**为什么是问题**：第 4 个参数是 `idempotencyKey`，第 3 个 `headers` 被传成 `{}`。于是 `Authorization` 对象被塞进幂等键位，而请求头里**没有任何鉴权头**。且 operator 流程用的是 `operatorToken`（非用户 `accessToken`），`api.getToken()` 读不到它——所以本请求必然 401，红娘"接单"必失败。同文件 `:50-51` 用 `api.get(url, {}, {Authorization})` 三参形式才正确，两处不一致即证据。
|
||
|
||
**修复**：改为 `api.post(url, {}, { Authorization: \`Bearer ${token}\` })`（去掉第 4 参）。
|
||
|
||
### 4. 管理端 token 存 localStorage + 权限清单可被本地篡改
|
||
`admin-web/src/stores/auth.ts:26,45-46`
|
||
```ts
|
||
const token = ref<string>(... localStorage.getItem('accessToken') || '')
|
||
localStorage.setItem('accessToken', res.data.accessToken)
|
||
localStorage.setItem('operator', JSON.stringify(operator.value)) // 含 permissions
|
||
```
|
||
`admin-web/src/api/client.ts:11` 拦截器统一注入 `Bearer`，但另有 30+ 处 `.vue` 直接 `localStorage.getItem('accessToken')` 手工注入（如 `ReportCenterView.vue:127/137/155/165`、`OperatorManagementView.vue` 多处、`ActivityCheckinView.vue:114`）。
|
||
|
||
**为什么是问题**：
|
||
- 令牌落 localStorage + 项目内大量 `any` 与裸 `console.error(e)`，任意 XSS 即可 `localStorage.getItem('accessToken')` 取走令牌直连后端，**账号接管**。
|
||
- `operator`（含 `permissions`）序列化进 localStorage，前端 `router/index.ts` 的 `hasPermission` 与 `MainLayout.vue` 菜单过滤只比对本地产物——攻击者改 `localStorage.operator.permissions` 即可在前端解锁菜单/按钮（虽服务端仍 403，但**可见性完全由可被篡改的本地数据决定**，违反"前端不是安全边界"）。
|
||
|
||
**修复**：
|
||
- 短期：令牌改内存态 + 短时效，刷新令牌走 httpOnly Cookie；删除各 `.vue` 的手工 `localStorage.getItem('accessToken')`，统一依赖 `client.ts` 拦截器。
|
||
- 权限可见性由服务端下发 capability 信封驱动（见 🟡 #4），前端不再以本地 `permissions` 数组作为显隐的唯一依据。
|
||
|
||
---
|
||
|
||
## 三、🟡 建议（应优先修，真实缺陷）
|
||
|
||
### 5. 刷新 Token 的 Redis TTL 用错 → 用户每 4 小时被强制登出
|
||
`backend/.../auth/service/AuthService.java:94`（登出同 :177-178）
|
||
```java
|
||
redisTemplate.opsForValue().set(refreshTokenKey, refreshToken,
|
||
Duration.ofMillis(jwtTokenService.getExpirationMs() * 2));
|
||
```
|
||
**为什么是问题**：`getExpirationMs()` 返回 **access** 过期（默认 2h），`*2 = 4h`；而 refresh JWT 自身有效期是 `refreshExpirationMs`（默认 14 天，`JwtTokenService.java:39/81`）。`refreshToken()` 第 3 步（:167-169）用 Redis 记录做可撤销校验，4h 后记录消失即抛 `UnauthorizedException` → **用户每 4h 被迫重新登录**。
|
||
|
||
**修复**：改为 `Duration.ofMillis(jwtTokenService.getRefreshExpirationMs())`（或新增对应 getter），使 Redis 记录生命周期与 refresh JWT 一致。
|
||
|
||
### 6. 手机号绑定无真实校验（TODO 残留）
|
||
`AuthService.java:113`：`// TODO: Phase 03 对接真实短信验证码`，`bindPhone` 仅查重后 `user.setPhone(phoneCode)`。结合 #1 Mock 登录，任何人可绑定任意手机号，属实名链路缺口。需补短信验证码校验。
|
||
|
||
### 7. 小程序 `bind-phone` 空桩 + operator 登录双轨清 token
|
||
- `pages/bind-phone/bind-phone.js` 为 `Page({ data:{}, onLoad(){}, onShow(){} })` 空实现，用户进页空白。
|
||
- `pages/matchmaker-login/matchmaker-login.js:39-51` 自建 `operatorToken` 并裸用 `wx.request` 绕过 `utils/api.js`；当 operator 接口回 401 时，`api.onSessionExpired` → `api.clearToken()` 清的是**用户** `accessToken/refreshToken`（api.js:113-120），会**误清用户登录态**。建议 operator 通道也走 `api` 层并隔离失效回调。
|
||
|
||
### 8. 小程序生产域名是占位符
|
||
`miniapp/env.js:27` / `env.ts:24`：`: 'https://api.danyang-match.example.com/api/v1'`。`example.com` 为保留占位域名，生产构建连不上真实后端。需按环境注入真实域名（构建变量 / 配置中心）。
|
||
|
||
### 9. 小程序 `serviceType` 枚举直渲未映射中文
|
||
`pages/matchmaker-workbench/matchmaker-workbench.wxml:59`
|
||
```xml
|
||
<view class="service-tag">{{item.serviceType === 'recommend_partner' ? '推荐对象' : item.serviceType}}</view>
|
||
```
|
||
除 `recommend_partner` 外，其它 `serviceType`（如 `red_line_prevention`）会把英文枚举码直接显示给用户，违反 `constants/profileLabels.js`"界面不得出现英文枚举码"铁律。需补 `serviceType` 中文映射表。
|
||
|
||
### 10. 管理端敏感处置缺二次确认
|
||
`src/modules/risk/ReportCenterView.vue:91` 对封禁/暂停(30天)等直接 `post(.../process)`，无 `ElMessageBox.confirm`；而 `UserDetailView.vue:113` 改账号状态有确认。同类高危操作应统一二次确认 + 审计提示。
|
||
|
||
### 11. 管理端取数/显隐仍由前端权限启发式决定（Capability 应服务端驱动）
|
||
`src/services/dashboard.ts:66-67,93-119` 用 `permissions.includes('analytics.view')` 决定请求接口并打 `available` 控制显隐；`MainLayout.vue:209-222` 用权限拼"角色名"。能力虽源自服务端权限串，但映射仍在前端硬编码。理想做法是由服务端下发 capability 信封驱动可见性与取数。
|
||
|
||
### 12. 管理端 `any` 滥用、跨端字段漂移、举报列表未分页
|
||
- `UserDetailView.vue:90`、`UserManagementView.vue:112`、`ReportCenterView.vue` 多处 `ref<any>`、`catch(e)` 未标注类型；跨端字段如 `phoneMasked/sensitiveMasked`、`totalLimit↔capacity` 缺共享 TS 接口，易产生字段名不一致。
|
||
- `ReportCenterView.vue:129` 列表 `res.data || []` 直接渲染、未分页，大举报量有性能隐患。
|
||
|
||
---
|
||
|
||
## 四、💭 细节（已良好 / 留意）
|
||
|
||
**✅ 已落实良好的部分（值得肯定）**
|
||
- 请求层时序守卫：`utils/api.js:180-198` 已实现"先 `resolve(body.data)` 再做 `releaseIdempotencyKey` 收尾"，正是工程纪律中"私有方法放 resolve 前致所有 Promise 永不 settle"坑的工程化规避；`:140-142` 的 `isAuthEndpoint` 豁免 `/auth/*` 自我递归。
|
||
- 数据权限守卫真实落地：`DataScopeGuard.deny` 已接入 `CheckinService`、`CounselingService`（Counselor 仅看 assigned + 敏感访问审计）、`ActivityService`、`ServiceAppointmentService`，非注释。
|
||
- 幂等机制扎实：`@Idempotent` 覆盖所有写端点，`IdempotencyInterceptor` 用 DB 唯一索引 `uk_idempotency_scope` 占位防并发重复。
|
||
- 配置可配置化良好：`MatchRuleConfigProperties` 权重/阈值/冷却期全部外部化（`match.*`），符合"不得写死业务规则"。
|
||
- 管理端设计系统 token 收敛良好（`theme/index.css` / `tokens.ts` 为唯一色源，输入框已含 `Microsoft YaHei`）。
|
||
|
||
**⚠️ 仍需留意 / 复核**
|
||
- SQL 注入：`.java` 内 `grep \${` 命中均为 `@Value` 占位符，mapper 走 MyBatis-Plus `LambdaQueryWrapper`，未见 `${}` 拼接 SQL。**但**项目含 MyBatis，需再扫 `src/main/resources` 下 `*Mapper.xml` 原生 SQL（`@Select` 等）确认无 `${}`。
|
||
- `SecurityConfig` 的 `requestMatchers` 相对 context-path `/api/v1` 是否被正确剥离（已有 `SecurityPathCoverageTest` 守护，风险低，建议回读确认）。
|
||
- `HomeBannerOperatorController:153` 仅拦截 `http(s)` 外链，需确认是否只允许 `/pages/` 内部跳转而非任意 http 内部域（INV-016 Banner 不能任意外链）。
|
||
- 跨端字段名靠运行时契约（tsc 不保护）：`stores/authStore.js` 注释自曝已三次踩坑（`accessToken` 非 `token`、`isNewUser` 非 `!needPhone`、`regionCode/createdAt` 等 camelCase）。建议对关键响应用 `result.accessToken || result.token` 兜底断言。
|
||
|
||
---
|
||
|
||
## 五、表扬
|
||
|
||
- **纪律工程化**做到位：把"WXML 结构、私有方法调用、组件选择器、请求层时序、幂等"等易踩坑点都做成了守护测试与行为契约，而不是靠人肉 review。这是本项目最值得保留的资产。
|
||
- **文档与代码分离意识强**：AGENTS.md 明确"文档✅不是需求，只有代码/测试/验收才证明完成"，且本审查中也确实发现若干文档声明与代码现实不符（微信登录、照片落库），说明团队有"不信绿"的底线。
|
||
|
||
---
|
||
|
||
## 六、下一步建议（优先级排序）
|
||
|
||
1. **上线前必须关闭**：🔴 #1 微信真实登录（已修复）、🔴 #2 照片直传落库端点（已修复）、🔴 #3 红娘接单鉴权头（已修复）、🔴 #4 管理端 token/权限存储（已修复，见第二节）。
|
||
2. **同批修复**：🟡 #5 刷新 Token TTL（影响所有用户登录体验）、#6 手机号真实校验。
|
||
3. **排期修复**：#7–#12。
|
||
4. **补一轮针对性测试**：直传落库端到端、刷新 Token 生命周期、operator 通道失效不误伤用户登录态、枚举中文映射契约测试。
|
||
|
||
> 说明：本审查以"证据优先"为原则，🔴 与 #5 均经主审查员直接读源码复核；落地修复前建议对每个文件再回读一次原文，确认行号与上下文未漂移。
|
||
|
||
---
|
||
|
||
## 七、第二轮修复（原延后项已落地，2026-09-24 续）
|
||
|
||
> 以下两项为上一轮"刻意未改/建议下一轮"的延后项，本轮已直接动手修复，均经源码级核验（本机 bash 不可用，未做编译/运行验证）。
|
||
|
||
### A. 生产实名核验 Mock → Real（对应 AGENTS.md 硬规则：本地资格/实名认证分离、敏感数据默认拒绝）
|
||
- 新增 `verification/provider/RealIdentityVerificationProvider.java`（`@ConditionalOnProperty(name="identity.verification.provider", havingValue="real")`）：
|
||
- 本地强校验身份证号 **GB 11643-1999 校验位（mod 11-2）**，伪造证件号在外部调用前即拒绝；
|
||
- 第三方核验走可配置 `identity.verification.real.api-url` + `api-key`（环境变量 `VERIFICATION_API_URL` / `VERIFICATION_API_KEY`）；
|
||
- **fail-closed**：任何无法明确判定为"通过"的响应一律视为未通过；**缺第三方配置即 fail-fast**（不静默放行任意证件）。
|
||
- `MockIdentityVerificationProvider` 属性从 `verification.provider` **解耦**为 `identity.verification.provider`（`matchIfMissing=true` 保留本地默认）。
|
||
- `application-production.yml` 显式置 `identity.verification.provider=real` + 注入 api-url/api-key；`application-dev.yml` / `application-docker.yml` 保持 mock 默认。
|
||
- **契约对接提示**：第三方请求/响应字段（如厂商选公安实名或运营商三要素）需在接入时最终确认；当前以 `success/match/verified/passed` 或 `code==0` 判定通过。
|
||
|
||
### B. 管理端 token 改 HttpOnly Cookie（对应原 🔴 #4）
|
||
- 后端 `OperatorCookieWriter`：登录/刷新成功下发 **HttpOnly + SameSite=Lax** 的 `dy_op_rt`（refresh）/ `dy_op_at`（access）Cookie；登出清除。`Secure` 由 `app.auth-cookie-secure` 控制（dev/docker false、production true）。
|
||
- `OperatorAuthController`：新增 `/auth/operator/refresh`（cookie 优先于 body，支持 silent-refresh）、`/auth/operator/logout`（清 Cookie + 黑名单）；登录/刷新均重下发 Cookie。
|
||
- `JwtAuthFilter`：取 token 改为 **Authorization 头优先、缺则回退 `dy_op_at` Cookie**（小程序/用户端仍走头，不受影响）。
|
||
- 前端 `stores/auth.ts`：**移除 localStorage 存 token/权限**，改内存态；新增 `initializeSession()` 凭 HttpOnly refresh Cookie 静默恢复会话（刷新页面免重登）；`client.ts` 拦截器 401 时自动静默刷新一次后重试（带防重入守卫）；9 个 `.vue` 中的 `localStorage.getItem('accessToken')` 全部改为内存态 `getAccessToken()`。
|
||
- **安全收益**：XSS 无法直接读取/外泄令牌，也无法改写本地权限清单伪造菜单（服务端 `@PreAuthorize` 仍是真边界）。
|
||
- **验证缺口（需你确认）**：本机未编译/运行；建议后端补编译 + 管理端登录→刷新→登出端到端走查；dev 跨端口代理与 prod 同源场景均依赖 Cookie（Cookie 不按端口划分作用域，dev 同样可用）。
|
||
|
||
### 仍延后（非本轮范围 / 需外部凭证）
|
||
- **手机号绑定真实校验**：需接短信验证码网关，无凭证无法真实实现，保留 TODO，未伪造。
|
||
- 上一轮"仍需留意"项（MyBatis xml `${}` 原生 SQL、SecurityConfig 路径剥离、Banner 外链范围、跨端字段名契约）维持原结论，未改动。
|