Skip to content

feat: require security verification before changing password in UC - #10228

Open
JohnNiang wants to merge 23 commits into
halo-dev:mainfrom
JohnNiang:feat/security-verification
Open

JohnNiang wants to merge 23 commits into
halo-dev:mainfrom
JohnNiang:feat/security-verification

Conversation

@JohnNiang

Copy link
Copy Markdown
Member

What type of PR is this?

  • Feature

What this PR does / why we need it:

当前 UC(个人中心)修改密码仅验证原密码,不验证邮箱验证码或 TOTP,攻击者一旦拿到密码即可改密并将用户锁在门外。本 PR 引入 GitHub 式「安全验证」(sudo 模式):

  • 修改密码前需通过邮箱验证码或 TOTP 验证(两者都可用时用户任选其一),原密码验证保留
  • 新增服务端渲染的安全验证页 /security-verification(Thymeleaf 模板,与登录/二步验证页同机制,主题可覆盖)
  • 验证通过后会话获得 30 分钟有效期的已验证标记(通用 sudo 窗口,后续敏感操作可复用)
  • 仅已有密码的用户需要验证;无已验证邮箱且无 TOTP 的用户回退到仅原密码,不会卡死
  • passwordless 用户首次设置密码不要求验证
  • 验证码通过新增 security-verification 通知原因类型发送到已验证邮箱(10 分钟过期、5 次尝试、黑名单防爆破)
  • TOTP 校验从 TwoFactorAuthEndpoint 抽取为共享 TotpVerificationService;验证接口按会话限流(5 次/5 分钟)
  • redirect 参数仅接受站内相对路径(拒绝 //\、畸形 URI),防 open redirect

Which issue(s) this PR fixes:

无关联 issue。

Special notes for your reviewer:

  • 本 PR 由 AI(Claude Code,subagent 驱动开发)辅助实现,代码经过逐任务评审(8 个任务各评审 1 次)+ 最终整分支评审,修复了评审发现的:订阅原因类型不匹配导致邮件无法送达、safeRedirect 反斜杠绕过、邮箱黑名单错误映射、验证分发绕过(纯邮箱用户提交任意 TOTP 码即可通过)等问题
  • 设计文档:docs/superpowers/specs/2026-08-11-security-verification-design.md
  • 实现计划:docs/superpowers/plans/2026-08-11-security-verification.md
  • 后端全量测试通过;前端 typecheck/lint/单元测试通过

Does this PR introduce a user-facing change?

修改密码前需先完成安全验证(邮箱验证码或 TOTP 验证码,任选其一)。已验证邮箱与 TOTP 均未配置的用户不受影响。

修改密码前增加安全验证(邮箱验证码或 TOTP 验证码),提升账号安全性

@pkg-pr-new

pkg-pr-new Bot commented Aug 11, 2026

Copy link
Copy Markdown

Open in StackBlitz

@halo-dev/api-client

npm i https://pkg.pr.new/@halo-dev/api-client@10228

@halo-dev/components

npm i https://pkg.pr.new/@halo-dev/components@10228

@halo-dev/richtext-editor

npm i https://pkg.pr.new/@halo-dev/richtext-editor@10228

@halo-dev/ui-shared

npm i https://pkg.pr.new/@halo-dev/ui-shared@10228

@halo-dev/ui-plugin-bundler-kit

npm i https://pkg.pr.new/@halo-dev/ui-plugin-bundler-kit@10228

commit: e409295

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.86408% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.59%. Comparing base (42c374d) to head (e409295).
⚠️ Report is 744 commits behind head on main.

Files with missing lines Patch % Lines
...ity/verification/SecurityVerificationEndpoint.java 86.66% 4 Missing and 4 partials ⚠️
...ser/service/impl/EmailVerificationServiceImpl.java 73.07% 5 Missing and 2 partials ⚠️
...uthentication/twofactor/TwoFactorAuthEndpoint.java 16.66% 5 Missing ⚠️
.../verification/SecurityVerificationFlowService.java 88.37% 1 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #10228      +/-   ##
============================================
+ Coverage     59.55%   65.59%   +6.03%     
- Complexity     3812     5130    +1318     
============================================
  Files           677      761      +84     
  Lines         23248    26741    +3493     
  Branches       1500     1884     +384     
============================================
+ Hits          13846    17541    +3695     
+ Misses         8764     8294     -470     
- Partials        638      906     +268     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JohnNiang and others added 4 commits August 11, 2026 23:55
- Keep the password-change=1 redirect when a verification-required
  error falls back to the verification page, so the change-password
  modal reopens after re-verification
- Add integration tests covering session-marked verification followed
  by password change, and rate-limit exhaustion mapped to
  rate-limit-exceeded
- Document the session-keyed verification rate limiter trade-off and
  drop its unreachable fallback
- Update the implementation plan to match the hardened dispatch logic
Split the combined verification form into independent per-method
endpoints and templates, following the login page structure:

- POST /security-verification/email and /security-verification/totp
  share the session rate limit, mark-verified and redirect/error mapping
  through a new public SecurityVerificationFlowService, so method
  endpoints (community or plugin-provided) reuse it instead of
  duplicating the plumbing
- each method gets its own template file (security-verification_email /
  security-verification_totp), selected by fragmentTemplateName like the
  login page; methods switch by URL (?method=) with tabs rendered from
  availableMethods via th:each, no JS visibility toggling
- the attempted method is echoed through the error redirect so a failed
  TOTP attempt stays on the TOTP form for a retry
- drop the redundant explicit CSRF field; Thymeleaf injects one per form
- center the cancel link; dedupe email/TOTP verification logic; rename
  SecurityVerificationService.isAvailable to hasVerificationMethod
Update the plan to match the implemented design: per-method endpoints
and independent template files, availableMethods-driven tabs via a plain
bean, the shared SecurityVerificationFlowService, and the
isAvailable -> hasVerificationMethod rename.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants