fix(api): give failures real HTTP semantics; fix 401/403 confusion and CORS default

Phase 23B ended with an E2E assertion changed to expect "HTTP 200 + body.code
== BAD_REQUEST", documented at the time as "this project's convention". That
description was accurate about the code and wrong about the intent: it described
a defect, and I made the test accommodate it instead of fixing it.

Auditing it found the convention was already broken, not merely risky:

- ~100 sites (105, across 30 controllers) do `return ApiResponse.error(...)` /
  `forbidden()` / `notFound()` with no exception and no `@ResponseStatus`, so
  the HTTP status is always 200. Gateways and APM cannot separate 4xx from 5xx;
  callers cannot tell "fix your request" (400) from "don't retry" (403) from
  "check state first" (409) from "retry later" (429).
- miniapp/utils/api.js already had 401/403/429 branches that could never be
  reached, since the backend never returned those codes. The concrete damage:
  an expired token returned 403, so `onSessionExpired` never fired and users
  were stuck with no way back to the login flow.
- SecurityConfig had no `authenticationEntryPoint`, so Spring's default
  `Http403ForbiddenEntryPoint` returned 403 for unauthenticated calls, with a
  framework body that has no `code` field at all.
- CorsConfig defaulted `allowed-origins` to `*` together with
  `allowCredentials(true)`, while its comment claimed "localhost only by
  default". Comment contradicting code is worse than no comment: it reads as
  safe.
- Only 4 files / 15 assertions in the whole backend suite touched HTTP status,
  which is how this survived.

Approach: one definition, two applications, zero controller changes.
- ApiResponseCode gains an `httpStatus` field — the single source of truth. The
  constructor requires it, so a new code cannot be added without one.
- GlobalExceptionHandler reads status from the enum instead of hardcoding
  `@ResponseStatus` per handler.
- New ApiResponseStatusAdvice (ResponseBodyAdvice) fills in the status just
  before the body is written, covering all ~100 direct-return sites without
  touching them. Two constraints, both noted in code: only downgrade when the
  current status is still 2xx (the advice also sees exception-handler results,
  and must not override an already-correct 401/403/409), and never alter the
  body.
- SecurityConfig: entry point returns 401 with a uniform ApiResponse body;
  access denied handler returns 403 likewise.
- CorsConfig: default changed to empty (deny). Both clients are same-origin
  anyway — nginx proxies /api/ and Vite proxies /api — so nothing depends on it.
- miniapp api.js: 401/403/429/409 branches now prefer the server's
  `body.message`, otherwise "账号已被封禁" would be flattened to "无权限访问".

Implementation trap worth remembering: `ServerHttpResponse` has `setStatusCode`
but no getter, so reading the current status requires casting to
`ServletServerHttpResponse`.

Verification: 37 E2E passed (33 + 4 new contract cases), including a live-stack
check that a controller's direct `notFound()` now yields 404 — that path throws
no exception and never reaches the exception handler, so if the advice were
inactive it would silently return 200 while the enum unit tests stayed green.
Backend regression 662 / 0 failures / 0 errors / 3 skipped (+6 new guards).

The first regression run had 3 failures, all tests asserting the old wrong
behavior (anonymous access expected 403; outsiders reading a conversation
expected 200). Fixed the assertions, not the implementation. Where 404 is used
instead of 403 for "invisible", that is deliberate — it prevents probing whether
another user's conversation exists — and the substantive assertions (e.g. that an
unauthorized response must not leak message bodies) were kept.

Not done, needs a product decision: third-party credential issuance, whether
rate limiting is actually active, publishing OpenAPI externally, per-caller
audit and quotas, pagination/idempotency coverage, desensitization boundaries.
This commit is contained in:
WorkBuddy
2026-09-18 14:34:58 +08:00
parent 730dddde30
commit 7682abd85c
13 changed files with 767 additions and 99 deletions
@@ -48,7 +48,16 @@ class AuthControllerTest {
}
@Test
void testProtectedEndpointRejectsAnonymousRequest() throws Exception {
/*
* 未认证 = 401,不是 403。
*
* 此前断言 403 是迁就了实现:SecurityConfig 未配置
* authenticationEntryPoint,Spring Security 默认走
* Http403ForbiddenEntryPoint,未认证也回 403。
* 401(需要认证)与 403(认证了但权限不足)必须分开,
* 否则客户端无从判断该去登录还是就此放弃。
*/
mockMvc.perform(get("/api/v1/users/me").contextPath("/api/v1"))
.andExpect(status().isForbidden());
.andExpect(status().isUnauthorized());
}
}
@@ -368,9 +368,15 @@ class ChatPhase09IntegrationTest {
String body = mockMvc.perform(get("/api/v1/conversations/" + conv.getId() + "/messages")
.contextPath("/api/v1")
.header("Authorization", "Bearer " + outsiderToken))
// 契约:控制器层以 body 级错误码表达"不可见"(success=false + code=NOT_FOUND),
// 与 MatchController.authorizedMatch 的既有约定一致;HTTP 层仍是 200。
.andExpect(status().isOk())
/**
* 契约:控制器层以 NOT_FOUND 表达"不可见",与
* MatchController.authorizedMatch 的既有约定一致 ——
* 刻意用 404 而非 403,避免通过错误码探测他人会话是否存在。
*
* HTTP 层此前恒为 200(Controller 直接 return 不带状态),
* 现已随业务码带上真实语义:404 + body.code=NOT_FOUND。
*/
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.success").value(false))
.andExpect(jsonPath("$.code").value("NOT_FOUND"))
.andReturn().getResponse().getContentAsString();
@@ -400,14 +406,14 @@ class ChatPhase09IntegrationTest {
mockMvc.perform(get("/api/v1/conversations/" + conv.getId())
.contextPath("/api/v1")
.header("Authorization", "Bearer " + outsiderToken))
.andExpect(status().isOk())
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.success").value(false))
.andExpect(jsonPath("$.code").value("NOT_FOUND"));
mockMvc.perform(get("/api/v1/conversations/" + conv.getId() + "/unread")
.contextPath("/api/v1")
.header("Authorization", "Bearer " + outsiderToken))
.andExpect(status().isOk())
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.success").value(false))
.andExpect(jsonPath("$.code").value("NOT_FOUND"));
@@ -0,0 +1,126 @@
package com.danyang.match.platform.shared.enums;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* 业务码 → HTTP 状态码映射的契约守卫。
*
* <p>为什么需要它:此前 Controller 里约 100 处 {@code return ApiResponse.error(...)}
* 返回 HTTP 200,客户端只能靠 body 的 {@code success} 判断成败。
* 现在状态由 {@link ApiResponseCode#getHttpStatus()} 统一提供,
* 但"映射写错"或"后来者给新码配了个 400"都不会有编译错误。
* 本测试把语义钉死,防止这套契约悄悄退化。
*/
class ApiResponseCodeHttpStatusTest {
/**
* 核心不变量:任何表示失败的业务码,HTTP 状态必须是 4xx/5xx,绝不能是 2xx。
*
* <p>若这条挂了,说明某个失败码会被当成成功响应发出去 —— 网关、监控、
* 客户端的重试逻辑会全部误判。
*/
@Test
@DisplayName("所有失败业务码的 HTTP 状态都不得是 2xx")
void testFailureCodesNeverMapTo2xx() {
List<ApiResponseCode> offenders = Arrays.stream(ApiResponseCode.values())
.filter(c -> c != ApiResponseCode.OK)
.filter(c -> c.getHttpStatus().is2xxSuccessful())
.collect(Collectors.toList());
assertTrue(offenders.isEmpty(),
"失败业务码不得映射为 2xx(会让客户端/网关误判为成功):" + offenders);
}
@Test
@DisplayName("身份或状态不允许类的失败必须是 403")
void testDenialCodesMapToForbidden() {
Set<ApiResponseCode> forbidden = Set.of(
ApiResponseCode.FORBIDDEN,
ApiResponseCode.PROFILE_NOT_APPROVED,
ApiResponseCode.USER_UNDER_18,
ApiResponseCode.USER_BANNED,
ApiResponseCode.USER_SUSPENDED,
ApiResponseCode.ACCOUNT_CLOSED,
ApiResponseCode.REQUIRE_MUTUAL_MATCH,
ApiResponseCode.USER_BLOCKED,
ApiResponseCode.SENSITIVE_ACCESS_DENIED,
ApiResponseCode.CROSS_ORGANIZATION_DENIED,
ApiResponseCode.OPERATION_NOT_ALLOWED,
ApiResponseCode.MATCHMAKER_CANNOT_LIKE);
assertAllMapTo(forbidden, HttpStatus.FORBIDDEN);
}
@Test
@DisplayName("与当前状态冲突(重复操作/名额已满)必须是 409")
void testConflictCodesMapToConflict() {
Set<ApiResponseCode> conflicts = Set.of(
ApiResponseCode.CONFLICT,
ApiResponseCode.MATCH_UNMATCHED,
ApiResponseCode.ALREADY_LIKED,
ApiResponseCode.ALREADY_PASSED,
ApiResponseCode.ALREADY_BLOCKED,
ApiResponseCode.ACTIVITY_FULL,
ApiResponseCode.ACTIVITY_ALREADY_REGISTERED,
ApiResponseCode.ALREADY_CHECKED_IN,
ApiResponseCode.DUPLICATE_SUBMISSION,
ApiResponseCode.REDUNDANT_OPERATION,
ApiResponseCode.MATCH_SUCCESS_NEED_BOTH_CONFIRM);
assertAllMapTo(conflicts, HttpStatus.CONFLICT);
}
@Test
@DisplayName("限流与冷却必须是 429,好让调用方知道该稍后重试")
void testThrottlingCodesMapToTooManyRequests() {
assertAllMapTo(Set.of(ApiResponseCode.RATE_LIMITED, ApiResponseCode.PASS_COOLDOWN),
HttpStatus.TOO_MANY_REQUESTS);
}
@Test
@DisplayName("认证与参数类基础码映射正确")
void testBasicCodes() {
assertEquals(HttpStatus.OK, ApiResponseCode.OK.getHttpStatus());
assertEquals(HttpStatus.UNAUTHORIZED, ApiResponseCode.UNAUTHORIZED.getHttpStatus());
assertEquals(HttpStatus.NOT_FOUND, ApiResponseCode.NOT_FOUND.getHttpStatus());
assertEquals(HttpStatus.BAD_REQUEST, ApiResponseCode.BAD_REQUEST.getHttpStatus());
assertEquals(HttpStatus.BAD_REQUEST, ApiResponseCode.VALIDATION_ERROR.getHttpStatus());
assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, ApiResponseCode.SERVER_ERROR.getHttpStatus());
}
/**
* 409 与 403 的区分必须保持:前者"改状态后可重试",后者"重试也没用"。
* 若两者混同,调用方就无法决定后续动作。
*/
@Test
@DisplayName("403 与 409 不得混同")
void testForbiddenAndConflictAreDistinct() {
long forbidden = Arrays.stream(ApiResponseCode.values())
.filter(c -> c.getHttpStatus() == HttpStatus.FORBIDDEN).count();
long conflict = Arrays.stream(ApiResponseCode.values())
.filter(c -> c.getHttpStatus() == HttpStatus.CONFLICT).count();
assertTrue(forbidden > 0 && conflict > 0,
"403 与 409 都应有业务码使用;混同会让调用方无法判断能否重试");
}
private static void assertAllMapTo(Set<ApiResponseCode> codes, HttpStatus expected) {
List<String> wrong = codes.stream()
.filter(c -> c.getHttpStatus() != expected)
.map(c -> c.getCode() + "->" + c.getHttpStatus())
.collect(Collectors.toList());
assertTrue(wrong.isEmpty(),
"以下业务码应映射为 " + expected + ",实际不符:" + wrong);
}
}