Fix Turnstile signup bypass and forward remote IP#140
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR updates the Cloudflare Turnstile verification flow in CustomSignUpForm by changing the configuration gating from SECRET_KEY to CLOUDFLARE_TURNSTILE_SITEKEY, adding optional remote IP address extraction and passing it to the verification API, and updating the method signature and payload construction accordingly. New test cases cover success, failure, and configuration scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Submission
participant Form as CustomSignUpForm
participant Request as Request Context
participant API as Cloudflare API
User->>Form: submit form with cf-turnstile-response
Form->>Form: check CLOUDFLARE_TURNSTILE_SITEKEY
Form->>Request: extract remote IP (if available)
Form->>Form: construct verification payload<br/>(secret + response + remoteip)
Form->>API: POST verification request
API-->>Form: return success/failure
Form-->>User: validation result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ 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 |
Greptile SummaryThis PR closes a critical security vulnerability where Turnstile verification could be bypassed when the site key was configured but the secret key was missing. The fix enforces server-side validation whenever the site key is enabled and fails closed when the secret key is absent. Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User submits signup form] --> B{Site key configured?}
B -->|No| C[Skip Turnstile validation]
B -->|Yes| D{Token present?}
D -->|No| E[Raise ValidationError]
D -->|Yes| F{Secret key configured?}
F -->|No| G[Return False - Fail closed]
F -->|Yes| H[Build verification payload]
H --> I{Remote IP available?}
I -->|Yes| J[Add remoteip to payload]
I -->|No| K[Skip remoteip]
J --> L[POST to Cloudflare API]
K --> L
L --> M{API call successful?}
M -->|No| N[Log error, return False]
M -->|Yes| O{success: true?}
O -->|No| P[Log error codes, return False]
O -->|Yes| Q[Return True]
G --> R[Raise ValidationError]
N --> R
P --> R
Q --> S[Continue signup]
C --> S
E --> T[Show error to user]
R --> T
Last reviewed commit: 8ccf010 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @override_settings( | ||
| CLOUDFLARE_TURNSTILE_SITEKEY="site-key", | ||
| CLOUDFLARE_TURNSTILE_SECRET_KEY="", | ||
| ) | ||
| @patch("allauth.account.forms.SignupForm.clean", return_value={}) | ||
| def test_clean_requires_turnstile_token_when_site_key_is_configured(self, mock_signup_clean): | ||
| form = CustomSignUpForm(data={}) | ||
| form.request = RequestFactory().post("/accounts/signup/") | ||
|
|
||
| with pytest.raises(forms.ValidationError, match="Please complete the verification challenge"): | ||
| form.clean() |
There was a problem hiding this comment.
Missing test for _verify_turnstile_token when secret key is missing. The test validates form-level enforcement but doesn't verify the fail-closed behavior at the method level.
Add test:
@override_settings(CLOUDFLARE_TURNSTILE_SECRET_KEY="")
def test_verify_turnstile_token_returns_false_when_secret_key_missing(self):
form = CustomSignUpForm()
is_valid = form._verify_turnstile_token("test-token", "1.2.3.4")
assert is_valid is FalseNote: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
siteverifyasremoteipWhy
Signup verification could be bypassed when the site key was configured but the secret key was missing, because server-side validation only ran when the secret key existed.
Testing
ENVIRONMENT=dev SECRET_KEY=test DEBUG=True SITE_URL=http://localhost POSTGRES_DB=test POSTGRES_USER=test POSTGRES_PASSWORD=test POSTGRES_HOST=localhost JINA_READER_API_KEY=test GEMINI_API_KEY=test PERPLEXITY_API_KEY=test KEYWORDS_EVERYWHERE_API_KEY=test uv run python -m pytest core/tests/test_forms.py -k turnstileSummary by CodeRabbit
Release Notes
Bug Fixes
Tests