Files

293 lines
14 KiB
Markdown
Raw Permalink Normal View History

# HTTP 语义检修报告(面向第三方开放)
- 日期:2026-09-18
- 起因:Phase 23B 收尾时留下的问题——「HTTP 200 + body.code 这套约定,将来要开放第三方还守得住吗?」
- 结论:**守不住,且当时就已经是断的**。已检修;过程中暴露 3 处被测试固化的错误期望与 1 处 CORS 安全缺陷。
---
## 一、起因:一条"迁就实现"的断言
Phase 23B 里我把一条 E2E 断言改成了:
```python
# 本项目的约定:业务错误是 HTTP 200 + body.code
assert body.get("code") == "BAD_REQUEST"
```
那段注释准确描述了代码行为,但它描述的对象恰恰是一个缺陷:
`handleIllegalArgument` 是全局异常处理器里**唯一没有 `@ResponseStatus` 的 handler**,
于是它返回 HTTP 200。我让断言迁就了实现,而不是修实现。
顺着这条线查下去,发现问题远不止一处。
---
## 二、审计发现
### 1. 主体问题:约 100 处失败响应顶着 HTTP 200
`ApiResponse.error(...)` / `forbidden()` / `notFound()` 在 Controller 里被直接
`return`,不抛异常、不带 `@ResponseStatus` —— HTTP 状态恒为 200。
统计:**105 处,分布在 30 个 Controller**。
后果分两层:
- **内部**:网关、监控、APM 无法按状态码统计错误率,「4xx 客户端错」与「5xx 服务端故障」全混在 200 里。
- **外部**:第三方集成商不会去比对字符串业务码。他们看的是状态码——
尤其是「能不能重试」这件事:**400 改参数、403 别重试、409 先查状态、429 稍后重试**。
全部返回 200 等于把这四种处置方式全部抹平。
### 2. 前后端契约当时就是断的(不只是"将来的风险")
小程序 `utils/api.js` 早就写好了 401 / 403 / 429 三个分支:
```js
} else if (res.statusCode === 401) { this.clearToken(); ... } // 触发重新登录
else if (res.statusCode === 403) { ... }
else if (res.statusCode === 429) { ... }
```
后端从未返回过这些状态码,这三个分支**永远走不到**,全部退化成 `BUSINESS_ERROR`。
最实际的一处伤害:token 过期时后端回 403,小程序因此**不会触发 `onSessionExpired`**,
用户卡在无法重新登录的状态。
### 3. 未认证返回 403,且 body 不是统一结构
`SecurityConfig` 从未配置 `authenticationEntryPoint`,Spring Security 默认走
`Http403ForbiddenEntryPoint`:没带 token 和 token 失效都返回 **403**,
body 是框架默认响应,**连 `code` 字段都没有**(实测 `body.code` 为 None)。
401(需要认证)与 403(认证了但权限不足)混同,是第三方集成最基础的分流依据缺失。
### 4. CORS 默认放行任意来源,且注释在撒谎
```java
@Value("${cors.allowed-origins:*}") // 默认通配
config.setAllowCredentials(true); // 允许携带凭证
// 注释却写着:"默认只允许localhost(开发环境)"
```
`allowCredentials(true)` + 通配来源 = 任意站点可发起带凭证的跨域请求(CSRF 直通)。
而注释声称的限制并不存在。**注释与代码不符比没有注释更危险**,它会让人误以为已经安全。
### 5. 状态码这层契约几乎无人看守
后端全量测试中,只有 **4 个文件、15 处**断言触及 HTTP 状态码。
这也是这套错配能长期存在的原因。
---
## 三、检修内容
### 设计取向:一处定义、两处应用、业务代码零改动
改造 105 处 Controller 代价高、回归面大。改为:
| 改动 | 作用 |
|---|---|
| `ApiResponseCode` 新增 `httpStatus` 字段 | 业务码 → HTTP 状态的**单一真相源**,构造函数强制携带,新增码不填就编译不过 |
| `GlobalExceptionHandler` 改为从枚举读状态 | 移除各处硬编码的 `@ResponseStatus`,避免与枚举产生分歧 |
| 新增 `ApiResponseStatusAdvice`(`ResponseBodyAdvice`) | 在响应写出前统一补状态,覆盖 105 处 Controller 直返,**不动任何 Controller** |
| `SecurityConfig` 补 401/403 入口点 | 未认证回 401,且 body 统一为 `ApiResponse` |
| `CorsConfig` 默认值改为空 | 默认拒绝跨域;生产由部署环境显式列出来源 |
| 小程序 `api.js` 401/403/429/409 分支 | 优先使用服务端 `body.message`,避免业务原因被硬编码文案冲掉 |
**Advice 的两个关键约束**(都写进了代码注释):
1. **只补 2xx。** 异常处理器已经设好 401/403/409/500,无差别覆盖会把已正确的状态改错。
由于 `ServerHttpResponse` 只有 setter 没有 getter,需向下转型到
`ServletServerHttpResponse` 才能读到当前状态。
2. **只改状态行,body 一字不动。** 依赖 `success/code` 判断的调用方行为不变;
依赖状态码的调用方从此走到正确分支。
### 业务码映射规则
| 语义 | 状态码 | 含义(告诉调用方下一步做什么) |
|---|---|---|
| 参数/校验问题 | 400 | 改请求就能通过 |
| 身份或状态不允许 | 403 | 改请求也没用,别重试 |
| 与当前状态冲突 | 409 | 先查状态,别盲目重试 |
| 速率或冷却限制 | 429 | 等一会儿重试 |
典型:`USER_BANNED / USER_UNDER_18 / REQUIRE_MUTUAL_MATCH / SENSITIVE_ACCESS_DENIED / MATCHMAKER_CANNOT_LIKE` → 403;
`ALREADY_LIKED / ACTIVITY_FULL / DUPLICATE_SUBMISSION` → 409;
`RATE_LIMITED / PASS_COOLDOWN` → 429。
---
## 四、验证证据
### 1. 活栈实测(最关键:证明 Advice 真的生效)
| 场景 | 路径 | 结果 |
|---|---|---|
| Controller 直返 `notFound()` | `POST /matches/{不存在UUID}/success/request` | **404** |
| 非法活动类型(异常处理器) | `GET /activities/types/not-a-real-type` | **400** |
| 非 UUID 路径参数 | `GET /counseling/services/not-a-uuid` | **400** |
| 未认证 | `GET /notifications`(无 token) | **401** |
第一条是判定性的:那条路径不抛异常、不进异常处理器,
若 Advice 失效(包扫描漏了、`supports` 写错),它会**悄悄退回 200,而枚举层的单元测试依然全绿**。
### 2. E2E
`37 passed`(原 33 + 新增 4 条契约用例)。
新增 `tests/e2e/http_semantics_test.py` 把「失败必须携带真实 HTTP 语义」钉在活栈上。
### 3. 后端全量回归
```
Tests run: 664, Failures: 0, Errors: 0, Skipped: 3 BUILD SUCCESS
```
基线 656(Phase 23B 后),净 +8:`ApiResponseCodeHttpStatusTest`(6 条映射语义守卫)
+ `SecurityPathCoverageTest`(2 条路径覆盖守卫,见第七节)。
3 个 skip 均有显式原因,无一为空壳。
首个回归轮次为 **662 / 3 failures**,三处失败均已判定为「测试断言了旧的错误行为」
(详见第五节),修正断言(而非改回实现)后归零。
---
## 五、被测试固化的错误期望(3 处)
回归从 656 → 662 时暴露 3 处失败,全部是**测试在断言旧的错误行为**:
| 测试 | 旧期望 | 新实际 | 判定 |
|---|---|---|---|
| `AuthControllerTest.testProtectedEndpointRejectsAnonymousRequest` | 403 | 401 | 新行为对 |
| `ChatPhase09IntegrationTest.outsiderCannotReadConversationDetailNorUnread` | 200 | 404 | 新行为对 |
| `ChatPhase09IntegrationTest.outsiderCannotReadMessages` | 200 | 404 | 新行为对 |
后两处的原注释直接写着「HTTP 层仍是 200」,正是本轮检修掉的约定。
改断言时**保留了各自的实质性安全断言**(如「越权响应不得泄露聊天正文」)。
「用 404 而非 403 表达不可见」是**刻意的安全设计**:若回 403,
攻击者可枚举 ID 区分「存在但无权」与「根本不存在」,从而探测他人关系是否存在。
该设计在 `MatchController#authorizedMatch` 注释中亦有说明,本轮予以保留。
---
## 六、遗留:真正开放第三方还需要什么(本轮未做)
以下均属范围扩张,**需产品决策**,本轮不擅自实施:
1. **第三方凭据机制**:目前只有微信登录与运营账号登录,第三方没有标准途径获取 token。
需定义 API Key / OAuth Client Credentials,以及与之配套的权限范围与配额。
2. **限流是否真实生效**:`RATE_LIMITED` 业务码与 429 映射已就位,
但限流本身是否启用、阈值多少,本轮未验证。
3. **OpenAPI 文档对外发布**:已有 `OpenApiConfig`,是否开放、开放哪些端点需决策。
4. **审计与可观测**:第三方调用需要独立的调用审计与配额计量。
5. **分页稳定性与幂等**:~~`Idempotency-Key` 已支持~~,但需确认第三方可见端点的覆盖范围。
> **勘误(同日续做)**:此处"已支持"在写下时不成立——后端当时**零处**读取 `Idempotency-Key` 头,
> 前端 26 处发送全部石沉大海。该断言已由 `docs/幂等键检修报告-2026-09-18.md` 修复并推翻:
> 幂等机制现已真正落地(29+1 处端点声明 + 拦截器 + 唯一索引),此处描述**至此才成立**。
> 保留原文以记录这处"结论先于核实"的失误。
6. **敏感字段脱敏边界**:现有 INV-011 等约束面向内部角色,第三方场景需重新界定。
---
## 七、补充:身份类型与路径规则的收敛(同日续做)
修完 401/403 之后,顺着同一条线索继续查,发现一个**与错误响应质量直接相关**的更严重缺陷。
### 1. 根因链条
`PlatformPrincipal` 有两个实现:`OperatorPrincipal`(运营 token)与 `UserPrincipal`(用户 token),
由 `JwtAuthFilter` 按 token 类型选择。
而 `@AuthenticationPrincipal UserPrincipal` 遇到 `OperatorPrincipal` 时属**类型不匹配**,
Spring Security 默认返回 **null**(`errorOnInvalidType=false`)。
后果:全仓 **244 处**使用该注解,**仅 48 处**做了 null 检查。
其余端点在运营身份访问时会直接 NPE → **500**。
这不是理论风险——`SecurityConfig` 当时的兜底规则是 `authenticated()`,
而运营 token 完全满足"已认证",请求可以一路打进用户端 Controller。
### 2. 为什么不补 null 检查
补上百处 `if (principal == null)` 是机械劳动,还会遗漏、也无法约束将来新增的代码。
根因是**身份类型与端点不匹配**,应该在认证层解决,而不是在每个端点重复防御。
### 3. 修法:兜底规则从「已认证」收紧为「用户身份」
```java
.requestMatchers("/**").hasRole("USER") // 原为 .authenticated()
```
运营身份访问用户端 → 403,请求根本进不到 Controller,196 处潜在 NPE 一次性消解。
角色对应关系由 `JwtAuthFilter` 保证:运营 → `ROLE_OPERATOR`,用户 → `ROLE_USER`。
### 4. ⚠️ 踩坑:`/**/operator/**` 是非法模式
运营端点并非都在 `/operator/` 下——`/counseling/services/operator/queue`、
`/content/operator/articles`、`/activities/operator/checkin/scan`、
`/service-appointments/operator/list` 等嵌在业务路径深处。
第一版试图用 `/**/operator/**` 一行通配,结果:
```
PatternParseException: No more pattern data allowed after {*...} or ** pattern element
```
**Spring 的 PathPattern 不允许 `**` 出现在中间**,只允许在末尾。
更糟的是它的表现:整个过滤器链直接 **500**(Tomcat HTML 错误页,不是我们的 JSON),
E2E 从 37 passed 变成 **33 failed**。
改为显式枚举 9 个前缀(全部符合 PathPattern 语法):
```
/operator/** /admin/**
/counseling/services/operator/** /content/operator/** /risk/operator/**
/activities/operator/** /service-appointments/operator/**
/service-programs/operator/** /referrals/operator/**
```
### 5. 机械守卫:`SecurityPathCoverageTest`
显式枚举的代价是"将来新增会漏",而漏掉的表现是
**运营人员被自己的平台拒绝(管理端对应模块不可用)**——只跑用户侧测试根本看不出来。
因此新增守卫,**从 `SecurityConfig` 源码解析出 OPERATOR 规则前缀**,
再扫描全部 `*Controller.java`,把「类级路径 + 方法级路径」拼成完整路径,
校验每个含 `/operator/` 的端点都被覆盖(`/auth/operator/**` 按设计排除)。
两个必须注意的细节:
- **只能解析 `.hasRole("OPERATOR")` 那条链的规则。** 若把 permitAll 或兜底的 `/**` 也收进来,
任何路径都会被判为"已覆盖",守卫就退化成永远通过的摆设。
- **工作目录是仓库根而非模块根**(同项目 `MigrationValidationTest` 也用 `backend/target/...` 形式),
故源码根需同时尝试两种候选路径。
**负向验证**:摘掉 `/content/operator/**` 前缀后,恰好 1 个用例变红,
并精确列出受影响的 6 个 `ContentController` 端点;恢复后全绿。
### 6. 验证
- E2E **40 passed**(原 37 + 3 条身份隔离用例:运营打用户端 403、
用户打运营端 403、嵌套运营端点仍要求 OPERATOR)。
- 后端全量回归 **664 / 0 failures / 0 errors / 3 skipped**(本轮起点 662,净 +2 为新守卫),
`SecurityPathCoverageTest` 已并入常规回归。
- 首个 E2E 轮次因非法路径模式导致 **33 failed**(过滤器链 500);
修正为显式前缀后全部恢复,佐证了"`/**/operator/**` 不可用"这一结论。
---
## 八、结论
本轮把「失败必须携带真实 HTTP 语义」从约定变成了机制:
- 状态码的**唯一来源**是 `ApiResponseCode.httpStatus`,新增业务码不声明就编译不过;
- 105 处 Controller 直返**零改动**即获得正确状态;
- 未认证与无权限在 HTTP 层分离,小程序终于能触发重新登录;
- CORS 默认从「放行任意来源」改为「默认拒绝」;
- 兜底访问规则从「已认证」收紧为「用户身份」,196 处潜在 NPE → 500 在认证层消解,
并有机械守卫防止将来新增运营端点时漏配规则。
顺带修掉了两个当时就会伤害真实用户的缺陷:
**token 过期后用户卡死无法重登录**,以及**运营身份可打进用户端触发 500**。