Add Cloudflare Turnstile support - #662
Conversation
Signed-off-by: Mamadou Babaei <info@babaei.net>
75ee3f5 to
2c716ff
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
@@ Coverage Diff @@
## main #662 +/- ##
==========================================
- Coverage 92.95% 89.56% -3.40%
==========================================
Files 21 22 +1
Lines 1193 1265 +72
==========================================
+ Hits 1109 1133 +24
- Misses 84 132 +48
🚀 New features to boost your workflow:
|
| h.opts.CfTurnstileSecret, | ||
| body.TurnstileToken, | ||
| clientIP, | ||
| time.Duration(h.opts.CfTurnstileTimeoutSeconds)*time.Second, |
CybotTM
left a comment
There was a problem hiding this comment.
Thanks — bot protection on a public password-reset flow is a legitimate complement to the per-IP limiter, and the shape here is right: genuinely opt-in (templates, CSP and handler all gate on the keys), fail-closed, and all three RPC methods are covered; the CSP additions match Cloudflare's documented requirements and only apply when enabled. Verdict: request changes — two of them are load-bearing:
- Critical — the default verification timeout exceeds the server's write timeout.
CF_TURNSTILE_TIMEOUT_SECONDSdefaults to 15 (internal/options/app.go:452) but Fiber runs withWriteTimeout: 10s(main.go:36/237). A slow siteverify response means the client gets a dropped connection instead of the intended 403, and each in-flight request pins a handler goroutine for up to 15s. Please default to ≤5s and bound the flag (the repo'scheckUintMaxpattern — which also resolves the gosec G115 finding at handler.go:112 properly, without a nolint). - Critical — verification runs before rate limiting.
turnstile.Verifyfires inHandlebefore method dispatch (handler.go:106–116), while the per-IP limiter lives inside the method handlers — so junk tokens trigger an unmetered outbound HTTPS call to Cloudflare per request, and none of those requests count toward the limit. Check the IP limiter first (or verify inside the already-limited paths). - Required — tests:
internal/turnstilehas no test file and nothing exercises the feature (codecov gates are red). Make the siteverify endpoint injectable, then cover success /success:false/ non-200 / timeout / malformed JSON, plus handler-level 403-when-token-missing for all three methods and disabled-mode passthrough. - Required — observability of fail-closed: the verification error is discarded and Cloudflare's
error-codesare never read — a wrong secret is indistinguishable from an outage; every request 403s and the operator is blind. Log the error server-side (error-codes are not PII), keep the client message generic, and document that a Cloudflare outage means password changes are unavailable until Turnstile is disabled. - Required — accessible failure: submitting with the widget present but unsolved silently re-enables the form (app.ts:232–235 and siblings) — nothing announced to screen readers, which breaks WCAG 3.3.1 in an AAA repo. Surface it via
setSubmitError/role="alert"(or disable submit until the token callback). - Required — CI/housekeeping: fix the five golangci-lint findings (noctx at turnstile.go:34, package comment, two ST1005 error strings, plus G115 via point 1); extract the triplicated Turnstile block in the three TS files into one shared helper (that alone clears the SonarCloud 12.9%-duplication gate);
bunx prettier --write .; reword the commit to Conventional Commits withSigned-off-by(the DCO check is red). - Required — docs in the same PR: README +
.env.local.examplefor the three new env vars,docs/api-reference.mdfor the newturnstileTokenfield, the CSP change, a privacy note (the widget sends visitor data to Cloudflare — relevant for self-hosted EU deployments), and the fail-closed behavior. Nit:CF_TURNSTILE_SITEKEYvscf-turnstile-site-key— one word-splitting convention,CF_TURNSTILE_SITE_KEY.
One point is ours to decide, not yours: internal/web/AGENTS.md currently says "no third-party scripts", and docs/accessibility.md claims WCAG 2.2 AAA — an opt-in Cloudflare iframe needs a documented exception in both. We'll resolve that on our side; the asks above are independent of it.
|
Maintainer decision on the policy question from the review (#673): the third-party exception is granted — Turnstile can land. One addition to the review's asks: activation must be an explicit flag, not implied by the keys. Please add Assisted by claude-code:claude-fable-5 — Session |
|
Thank you for the review and the feedback. I'll try to implement the suggested changes to the best of my ability and force push the changes to the same branch for another review once I think it's ready. |
ab0c03f to
a52ff7f
Compare
|



Description
Adds optional Cloudflare Turnstile protection to the password change and password reset flows.
When configured, Turnstile is displayed on the change password, forgot password, and reset password forms. The generated token is verified server-side with Cloudflare before the corresponding RPC request is processed.
Turnstile remains disabled when no site key and secret are configured. Verification requests use a configurable timeout and fail closed if verification fails or Cloudflare cannot be reached.
Type of Change
Related Issues
N/A
Changes Made
Testing
Test Environment
Test Cases
go test ./...)Test Results
Manually verified:
Example direct RPC request without a Turnstile token: