Skip to content

Discord 灰度(#53 / frontend#389)xhigh review 的 15 条待修项 #54

Description

@longsizhuo

对已合并上线的 Discord 灰度改动(backend #53 a073050 + frontend involutionhell#389 e7ab467)做 xhigh 多角度 review,43 条候选经独立验证后收敛为 15 条。按严重度排列。

这两个 PR 是自审自合上线的,绕过了"commit 前先 CR"的流程——下面至少 3 条本应在合并前被拦下。


🔴 P0 — 线上活跃缺陷

1. ?error=__proto__ 可让整个登录页崩溃 [frontend]

LoginErrorNotice.tsx:18messages[error]无保护的原型链查找:

参数 messages[error] 结果
discord_canary string ✅
__proto__ object
constructor / toString / valueOf function

messages["__proto__"] 返回 Object.prototype(非 undefined),所以 ?? messages.generic 兜底不触发,React 抛 Objects are not valid as a React child/loginerror.tsx、全站无 app/error.tsx → 落到 Next 全局兜底 → 整页变成 "Application error",GitHub 登录也一并不可用。发条链接即可让人登不了录。

:显式 code→key 映射 + Object.hasOwn


🔴 P1 — 闸的双向故障

2. fail-open 且完全无声 [backend]

@Value("${auth.discord.allowlist:}") 用空默认值关掉一道安全闸。env 丢失/拼错 → 值为空 → discordAllowed 对所有人返回 true → Discord 对全网开放。启动时和放行时都没有任何日志说明处于哪种模式,唯一信号是"没有拒绝日志",而没人在 tail 它。届时每个访客都会经 loginByProvider 建号——正是 OTP wiring 未完成的那条路径。:启动时按模式打日志。

3. 非空但零条目 / 带引号 → 反向锁死所有人 [backend]

  • Java 里 ",".split(",") 返回零长数组:.env 里手滑留个逗号 = 非空 + 零条目 = 连白名单本人都进不去,日志还写"不在白名单"。
  • docker-compose env_file 不剥引号,AUTH_DISCORD_ALLOWLIST="123" 会带引号参与比较,trim() 不去引号 → 同样锁死。

十几行之上的 requireConfigured() 就是 fail-fast 的正确示范,新代码没照做。:启动时解析成 Set、剥引号、丢空项,非空却零条目要告警。

4. AUTH_DISCORD_ALLOWLIST 未进 .env.example [backend]

照文档配出来的环境 = 白名单为空 = 全开放。


🟠 P2 — 设计与语义

5. 闸装在回调处:被拒用户已付出完整授权代价 [frontend+backend]

按钮对所有访客可见 → 走完 Discord 授权页、批准 identify emailtoken 已交换完成,我们已持有其邮箱与 access token → 才被弹回。且没有调用 Discord 的 token revoke,授权记录永久留在其「已授权应用」列表里。灰度期只有 1 个 id 放行,意味着其他每一次点击都是这个结局

6. 闸没有"已存在账号"豁免,会把老用户锁在自己账号外 [backend]

闸在 loginByProvider 之前。按钮下线期间 /oauth/render/discord 一直是活的,任何直接敲过该 URL 的人都已有真实账号;闸生效后他们再登录只会看到"敬请期待",不提示账号存在、也无恢复路径。

实测当前影响面为 0:user_identities 里 discord 仅 1 条(站长,已关联到 GitHub 账号),discord_* 独立账号 0 个。但闸的语义位置仍应修正——要保护的是"建新号",不是"回访登录"

7. 白名单并未恢复原注释保护的不变量 [frontend]

618f71f 下线按钮的理由是"OTP wiring 未完成,以免已有 GitHub 用户被分叉出第二个账号"。白名单只限制了能触发分叉,不消除分叉本身:被放行者若 Discord 邮箱与 GitHub 主邮箱不同,就会新建一个 Set.of("user") 账号,且无合并入口。

对当前唯一放行者不成立(identity 行 361 证明其邮箱已成功自动关联)。


🟠 P3 — OTP dev 兜底(三条同源)

8. dev 日志同时打印收件邮箱与明文验证码 [backend]

新增行 log.warn("[DEV-OTP] ... email={} code={}"),而同文件下方八行的既有日志专门写着 // 不落收件邮箱(PII),ResendEmailService javadoc 亦然。挡住它进生产的只有一句注释,无任何代码保护(无 @Profile、无条件)。

9. 触发条件与生产事故同形 [backend]

兜底判据是 resend.api-key 为空——这也正是生产掉 key 的样子。接线后 prod 掉 key 会变成"提示已发送、实则未发、用户永远卡在输码页";改动前该情形返 SEND_FAILED(可重试)。:改由显式 dev 开关驱动,而非"缺少生产凭据"。

10. RegistrationService 当前零生产调用方 [backend]

无 send-otp/verify-otp 控制器,该便利今天兑现不了,反倒只有会进生产的那行日志真上了线。宜与接线 PR 同批。


🟡 P4 — 测试与文档缺口

11. 新闸零测试 + 无 SECURITY.md 条目 [backend]

闸就长在 INV-007 保护的方法体内,而该文档自己的规矩是"改这里必须同步更新测试"。今后重构挪走/下移这道闸,289 个测试全绿照过。应补 INV-008 + 回调测试(断言重定向为 discord_canaryloginByProvider 从未被调用)。

12. 新属性未在 test profile 中 pin [backend]

OAuthControllerIntegrationTests 已因"本机 .env 会渗进测试 JVM"而显式 pin 空 justauth.type.discord.*,新属性漏了。结果:本机(有真实 id)闸是开的、CI 是关的,同一个测试两台机器结论不同

13. dev 兜底测试断言过弱 [backend]

只断言 SENT + never() sendHtml,二者都与"该分支必须位于 synchronized 块之后"无关。今后把 isConfigured() 检查上提(很自然的 fail-fast 重构)会导致会话态不写入 → 拿到码也 verifyAndConsume 失败 → 本地流程彻底坏掉,而测试照过

14. 只映射了 2/4 个 error code [frontend]

后端实发四种:discord_canary / oauth_failed / oauth_state / oauth_provider。后两者都塌缩成"请重试"——而 oauth_state(cookie 被拦或超 300s)重试会无限复现;oauth_provider(服务端缺配置)再怎么重试都不可能好

15. 两处 cleanup [frontend]

  • messages prop 重复造轮子:NextIntlClientProvider 已在 layout.tsx:65 包裹全站,直接 useTranslations 即可,省掉"加 error code 要改三个文件"的同步负担。
  • 按钮旁的注释叙述当前灰度阶段,违反 frontend/CLAUDE.md §3 的注释规约,且 GA 是改另一个仓库的 env,没人会回来删它。

修复 PR 随后提交(后端 + 前端各一,因分属两个仓库)。

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions