feat: require CAPTCHA before sending email verification code on registration - #5857
Open
Gravirei wants to merge 2 commits into
Open
feat: require CAPTCHA before sending email verification code on registration#5857Gravirei wants to merge 2 commits into
Gravirei wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new registration CAPTCHA setting, wires it through backend option storage and status APIs, enforces enablement rules and middleware behavior, and surfaces the toggle in both classic and default web settings with updated translations. ChangesRegister CAPTCHA setting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AdminUI
participant UpdateOption
participant TurnstileCheck
participant RegisterAPI
AdminUI->>UpdateOption: enable RegisterPageWithCaptchaEnabled
UpdateOption->>UpdateOption: require TurnstileSiteKey
RegisterAPI->>TurnstileCheck: request to /api/user/register
TurnstileCheck->>TurnstileCheck: enable verification when flag is set
sequenceDiagram
participant SignUpForm
participant useTurnstile
participant StatusAPI
SignUpForm->>StatusAPI: read register_page_with_captcha, turnstile_check, turnstile_site_key
SignUpForm->>SignUpForm: compute signupTurnstileRequired
SignUpForm->>useTurnstile: useTurnstile({ forceEnable: signupTurnstileRequired })
useTurnstile-->>SignUpForm: isTurnstileEnabled
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
controller/option.go (1)
201-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate validation logic between
TurnstileCheckEnabledandRegisterPageWithCaptchaEnabled.Both branches perform the identical check (
option.Value == "true" && common.TurnstileSiteKey == "") and only differ in the error message. Consider extracting a small helper to avoid drift if the check logic changes later.♻️ Proposed refactor
- case "TurnstileCheckEnabled": - if option.Value == "true" && common.TurnstileSiteKey == "" { - c.JSON(http.StatusOK, gin.H{ - "success": false, - "message": "无法启用 Turnstile 校验，请先填入 Turnstile 校验相关配置信息！", - }) - return - } - case "RegisterPageWithCaptchaEnabled": - if option.Value == "true" && common.TurnstileSiteKey == "" { - c.JSON(http.StatusOK, gin.H{ - "success": false, - "message": "无法启用注册人机验证，请先填入 Turnstile 校验相关配置信息！", - }) - return - } + case "TurnstileCheckEnabled", "RegisterPageWithCaptchaEnabled": + if option.Value == "true" && common.TurnstileSiteKey == "" { + msg := "无法启用 Turnstile 校验，请先填入 Turnstile 校验相关配置信息！" + if option.Key == "RegisterPageWithCaptchaEnabled" { + msg = "无法启用注册人机验证，请先填入 Turnstile 校验相关配置信息！" + } + c.JSON(http.StatusOK, gin.H{ + "success": false, + "message": msg, + }) + return + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/option.go` around lines 201 - 218, The `TurnstileCheckEnabled` and `RegisterPageWithCaptchaEnabled` branches in `option.go` duplicate the same `option.Value == "true" && common.TurnstileSiteKey == ""` validation. Extract that shared check into a small helper or shared condition near the existing switch logic, and keep the two branches only for their specific error messages so the validation stays consistent if it changes later.web/classic/src/components/settings/SystemSetting.jsx (1)
1075-1083: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider warning admins that a Turnstile Site Key is required.
Backend
UpdateOptionrejects enabling this flag without a configured Turnstile Site Key (per PR objectives). Other cross-dependent checkboxes in this file (e.g. the SSRF checkboxes) useextraTextto surface such dependencies proactively; this new checkbox has none, so admins will only discover the requirement via a generic error toast after clicking, and the checkbox will remain visually checked despite the rejected request (same UI-rollback gap shared by other checkboxes in this component, not new to this PR).💡 Suggested extraText addition
<Form.Checkbox field='RegisterPageWithCaptchaEnabled' noLabel + extraText={t('启用前请先配置 Turnstile Site Key')} onChange={(e) => handleCheckboxChange('RegisterPageWithCaptchaEnabled', e) } > {t('注册及发送邮箱验证码时需要 Turnstile 校验')} </Form.Checkbox>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/classic/src/components/settings/SystemSetting.jsx` around lines 1075 - 1083, The RegisterPageWithCaptchaEnabled checkbox in SystemSetting.jsx should proactively warn admins that a Turnstile Site Key is required. Add an extraText message to this Form.Checkbox, matching the pattern used by the SSRF-related checkboxes in this component, so the dependency is visible before submission. Keep the existing handleCheckboxChange flow intact and place the guidance near the RegisterPageWithCaptchaEnabled label so it’s easy to find if the UI changes.web/default/src/features/auth/hooks/use-turnstile.ts (1)
28-35: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
forceEnabledoesn't guard against missing site key.
isTurnstileEnabledcan becometrueviaforceEnableeven whenturnstileSiteKeyis empty (if a future caller doesn't pre-checkturnstile_site_keylikesign-up-form.tsxdoes). Consider derivingisTurnstileEnabledso it also requires a non-empty site key when relying onforceEnable, to make the hook self-contained rather than depending on caller discipline.♻️ Suggested defensive fix
- const isTurnstileEnabled = - options?.forceEnable || - !!(status?.turnstile_check && status?.turnstile_site_key) const turnstileSiteKey = status?.turnstile_site_key || '' + const isTurnstileEnabled = + !!turnstileSiteKey && + (options?.forceEnable || !!status?.turnstile_check)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/auth/hooks/use-turnstile.ts` around lines 28 - 35, The `useTurnstile` hook’s `isTurnstileEnabled` flag can become true from `forceEnable` even when `turnstileSiteKey` is empty, so make the hook self-contained by requiring a non-empty site key in the `useTurnstile` logic. Update the `isTurnstileEnabled` derivation in `useTurnstile` so `forceEnable` only enables Turnstile when `status.turnstile_site_key` is present, instead of relying on callers like `sign-up-form.tsx` to precheck it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@middleware/turnstile-check.go`:
- Around line 20-24: The Turnstile check is too broadly enabled for
"/api/verification", which affects non-registration flows like email binding.
Update the logic in turnstile-check.go within the turnstile middleware to scope
CAPTCHA enforcement to registration only, either by splitting the verification
route or by checking the request purpose before setting enabled in the
route-matching block that currently uses common.RegisterPageWithCaptchaEnabled.
---
Nitpick comments:
In `@controller/option.go`:
- Around line 201-218: The `TurnstileCheckEnabled` and
`RegisterPageWithCaptchaEnabled` branches in `option.go` duplicate the same
`option.Value == "true" && common.TurnstileSiteKey == ""` validation. Extract
that shared check into a small helper or shared condition near the existing
switch logic, and keep the two branches only for their specific error messages
so the validation stays consistent if it changes later.
In `@web/classic/src/components/settings/SystemSetting.jsx`:
- Around line 1075-1083: The RegisterPageWithCaptchaEnabled checkbox in
SystemSetting.jsx should proactively warn admins that a Turnstile Site Key is
required. Add an extraText message to this Form.Checkbox, matching the pattern
used by the SSRF-related checkboxes in this component, so the dependency is
visible before submission. Keep the existing handleCheckboxChange flow intact
and place the guidance near the RegisterPageWithCaptchaEnabled label so it’s
easy to find if the UI changes.
In `@web/default/src/features/auth/hooks/use-turnstile.ts`:
- Around line 28-35: The `useTurnstile` hook’s `isTurnstileEnabled` flag can
become true from `forceEnable` even when `turnstileSiteKey` is empty, so make
the hook self-contained by requiring a non-empty site key in the `useTurnstile`
logic. Update the `isTurnstileEnabled` derivation in `useTurnstile` so
`forceEnable` only enables Turnstile when `status.turnstile_site_key` is
present, instead of relying on callers like `sign-up-form.tsx` to precheck it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34a26cb1-97b7-41f3-b248-0f7718a02a7c
📒 Files selected for processing (26)
common/constants.gocontroller/misc.gocontroller/option.gomiddleware/turnstile-check.gomodel/option.goweb/classic/src/components/settings/SystemSetting.jsxweb/classic/src/i18n/locales/en.jsonweb/classic/src/i18n/locales/fr.jsonweb/classic/src/i18n/locales/ja.jsonweb/classic/src/i18n/locales/ru.jsonweb/classic/src/i18n/locales/vi.jsonweb/classic/src/i18n/locales/zh-CN.jsonweb/classic/src/i18n/locales/zh-TW.jsonweb/classic/src/i18n/locales/zh.jsonweb/default/src/features/auth/hooks/use-turnstile.tsweb/default/src/features/auth/sign-up/components/sign-up-form.tsxweb/default/src/features/system-settings/auth/bot-protection-section.tsxweb/default/src/features/system-settings/auth/index.tsxweb/default/src/features/system-settings/auth/section-registry.tsxweb/default/src/features/system-settings/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
Calcium-Ion
force-pushed
the
main
branch
2 times, most recently
from
August 30, 2026 15:03
51fdfc5 to
2b6f1df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 变更描述 / Description
新增注册及发送邮箱验证码时的人机验证支持，以防止机器人恶意刷注册接口和消耗 SMTP 额度。
允许管理员在后台的"Bot Protection"设置中，单独开启此选项（RegisterPageWithCaptchaEnabled）。
主要改动：
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
请在合并前在本地验证：开启该选项后，不完成 Turnstile 时注册接口返回校验失败；完成 Turnstile 后可正常发送验证码和注册。
Summary by CodeRabbit
New Features
Bug Fixes
Documentation