feat(auth): add Cloudflare Turnstile bot protection and rate limiting#81
feat(auth): add Cloudflare Turnstile bot protection and rate limiting#81pranavshankar1221 wants to merge 7 commits into
Conversation
|
@pranavshankar1221 is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
9507436 to
8e4c91c
Compare
|
Hi @jpdevhub I have done issue #61 Implemented Cloudflare Turnstile integration with backend token verification and auth endpoint rate limiting. Addressed the GitGuardian finding by removing the Turnstile secret from the example environment file. Security checks are now passing. Ready for review. Feedback is welcome. If any of the changes is required feel free to ask mee !! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
just git pull all the changes the merge conflicts will be either decreased or fully vanished and then fix if any error came in lint error or npm run build error then push . |
|
also tag the issue assigned to you |
|
the backend test is failing. |
There was a problem hiding this comment.
Pull request overview
Adds Cloudflare Turnstile-based bot protection to the Google OAuth initiation flow and introduces rate limiting on auth-start endpoints, wiring the frontend to request a Turnstile token and the backend to verify it before returning/redirecting to the OAuth provider.
Changes:
- Frontend: adds a Turnstile hook + UI gating and sends Turnstile tokens when starting Google login.
- Backend: adds Turnstile verification helper and applies
5/minuterate limits to auth login endpoints (GET redirect + new POST returningredirect_url). - Docs/config: documents new env vars and expands
.gitignoreto avoid committing local/secrets artifacts.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/AuthPage.tsx | Runs Turnstile before initiating Google OAuth and disables login until verification is ready. |
| src/lib/useTurnstile.ts | New hook to load/render/execute Cloudflare Turnstile and return tokens to callers. |
| src/lib/api.ts | Changes loginUrl() to async; optionally POSTs Turnstile token to get a redirect URL. |
| backend/main.py | Adds Turnstile enforcement + rate limiting to auth-start endpoints and introduces a POST variant. |
| backend/turnstile.py | New helper to verify Turnstile tokens server-side via Cloudflare siteverify. |
| backend/requirements-base.txt | Adds/adjusts SlowAPI dependency for rate limiting. |
| README.md | Documents Turnstile-related frontend/backend environment variables and auth rate limiting behavior. |
| backend/.env.example | Adds TURNSTILE_SECRET_KEY example configuration. |
| .env.example | Adds VITE_TURNSTILE_SITE_KEY example configuration. |
| .gitignore | Adds additional ignores for local build outputs, caches, and secret files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from fastapi import Body, FastAPI, File, UploadFile, Form, HTTPException, Depends, Query, Request | ||
| from fastapi.middleware.cors import CORSMiddleware | ||
| from fastapi.responses import RedirectResponse | ||
| from slowapi import Limiter, _rate_limit_exceeded_handler | ||
| from slowapi.middleware import SlowAPIMiddleware | ||
| from slowapi.util import get_remote_address | ||
| from fastapi import FastAPI, File, Request, UploadFile, Form, HTTPException, Depends, Query | ||
| from fastapi.middleware.cors import CORSMiddleware | ||
| from fastapi.responses import JSONResponse, RedirectResponse |
| limiter = Limiter(key_func=get_remote_address) | ||
| app.state.limiter = limiter | ||
| app.add_exception_handler(429, _rate_limit_exceeded_handler) | ||
| app.add_middleware(SlowAPIMiddleware) | ||
| app.state.limiter = limiter | ||
| app.add_middleware(SlowAPIMiddleware) |
| async def _verify_turnstile(turnstile_token: str | None, request: Request) -> None: | ||
| if TURNSTILE_SECRET_KEY: | ||
| await verify_turnstile_token(turnstile_token, request.client.host) |
| slowapi>=0.1.4 | ||
| slowapi==0.1.9 # rate limiting for FastAPI; added for per-user scan endpoint throttling |
| except Exception as exc: | ||
| raise HTTPException( | ||
| status_code=502, | ||
| detail=f'Turnstile verification failed: {exc}', | ||
| ) |
| useEffect(() => { | ||
| if (!siteKey) { | ||
| setReady(false); | ||
| return; | ||
| } |
|
Also check the review of the copilot, focus on the high severity one basically as its saying its dublicated somewhere else the module is their use that. |
|
Tag your issue no with closes #.. |
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Configuration, documentation, and dependencies .env.example, backend/.env.example, README.md, .gitignore, backend/requirements-base.txt, .github/workflows/greetings.yml |
Environment templates introduce VITE_TURNSTILE_SITE_KEY (frontend) and TURNSTILE_SECRET_KEY (backend). README documents rate limiting (5 req/min) and token rejection behavior. .gitignore expanded for dev artifacts. slowapi dependency pinned. |
Backend Turnstile verification module backend/turnstile.py |
New module reads TURNSTILE_SECRET_KEY, validates token via async POST to Cloudflare /siteverify, handles network failures as 502, and rejects invalid tokens with 400 including error codes. |
Backend OAuth routes with Turnstile and rate limiting backend/main.py |
/api/v1/auth/login/google endpoint split into GET (query turnstile_token, redirects) and POST (JSON body, returns redirect_url). Both handlers rate-limited via slowapi with explicit 429 exception handler. Turnstile verification called before OAuth URL generation. |
Frontend Turnstile React hook src/lib/useTurnstile.ts |
New hook dynamically loads Cloudflare script once, renders widget into container ref, tracks readiness and errors, and exposes execute() that returns verification token via promise resolved by Turnstile callback. |
Frontend API and AuthPage Turnstile integration src/lib/api.ts, src/pages/AuthPage.tsx |
api.loginUrl() accepts optional turnstileToken and returns Promise<string> via POST when token provided. AuthPage wires hook, renders Turnstile container, conditionally executes verification with readiness/error UI, and navigates to OAuth URL after token acquisition. |
Backend auth tests with Turnstile and rate limiting backend/test_auth.py |
Tests cover POST without Turnstile, invalid Turnstile rejection, GET redirect flow, and rate-limit enforcement with recovery. SKIP_TURNSTILE_VERIFICATION env flag allows CI to bypass external service calls. |
Sequence Diagram
sequenceDiagram
participant User
participant Browser as Browser + useTurnstile
participant AuthPage as AuthPage Component
participant Backend as FastAPI Backend
participant Cloudflare
participant GoogleOAuth as Google OAuth
User->>AuthPage: Click "Login with Google"
AuthPage->>Browser: useTurnstile.execute()
Browser->>Cloudflare: Request CAPTCHA widget
Cloudflare-->>Browser: Widget rendered
User->>Browser: Solve CAPTCHA
Browser->>Cloudflare: Submit response
Cloudflare-->>Browser: Verification token
Browser-->>AuthPage: Token returned
AuthPage->>Backend: POST /api/v1/auth/login/google {turnstile_token}
Backend->>Cloudflare: POST /siteverify (secret + token)
Cloudflare-->>Backend: {success: true}
Backend->>GoogleOAuth: Generate OAuth URL
GoogleOAuth-->>Backend: redirect_url
Backend-->>AuthPage: {redirect_url}
AuthPage->>GoogleOAuth: window.location.href = redirect_url
GoogleOAuth-->>User: OAuth consent flow
🎯 3 (Moderate) | ⏱️ ~25 minutes
🐰 A carrot-guarded gate now stands tall,
Turnstile spins to catch them all,
Bots beware, the CAPTCHA's here,
OAuth flows with rabbit cheer! 🥕✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately and clearly describes the main changes: adding Cloudflare Turnstile bot protection and rate limiting to authentication routes, which are the primary features introduced across the changeset. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 @.github/workflows/greetings.yml:
- Line 1: The workflow root key "name: Greetings" is indented and must start at
column 0; remove any leading spaces on that line (and ensure other top-level
keys such as "on" and "jobs" are also not indented) so the YAML root keys begin
at the leftmost column and the workflow is valid.
In `@backend/.env.example`:
- Around line 33-35: Remove the unresolved merge artifact "=========" from the
env template; edit the .env.example so only valid dotenv lines remain (e.g.,
keep CORS_ALLOW_ALL=true) and delete the dangling "=========" line to prevent
dotenv parsing/linting errors.
In `@backend/main.py`:
- Line 397: Replace the HTTPException detail that interpolates raw exception
text (the raise HTTPException(status_code=500, detail=f"Could not generate OAuth
URL: {exc}") and the similar raise around line 413) with a generic client-facing
message like "Internal server error generating OAuth URL", and instead log the
full exception server-side using the application's logger (e.g., call
logger.exception or logger.error with exc_info=True) inside the except block; do
this for both occurrences so clients never receive raw exception text while full
details are kept in server logs.
- Around line 379-383: The _verify_turnstile function currently skips
verification when TURNSTILE_SECRET_KEY is falsy, silently disabling protection;
change it to fail closed by rejecting requests (raise an HTTP error like
HTTPException 401/403) when TURNSTILE_SECRET_KEY is not set unless an explicit
test-only bypass flag is enabled (e.g., TURNSTILE_BYPASS_FOR_TESTING). Update
_verify_turnstile to check TURNSTILE_SECRET_KEY and if missing: if the bypass
flag is true, allow; otherwise log context (include client_host) and raise the
HTTP error; keep the existing call to verify_turnstile_token when the secret
exists. Ensure references: _verify_turnstile, TURNSTILE_SECRET_KEY,
verify_turnstile_token, and the new TURNSTILE_BYPASS_FOR_TESTING flag are used
so reviewers can locate the change.
- Around line 386-402: The GET and POST handlers currently use independent rate
buckets; replace the decorator on both login_google_get and login_google_post by
using limiter.shared_limit("5/minute", scope="login_google") (i.e., swap
`@limiter.limit`(...) for `@limiter.shared_limit`("5/minute", scope="login_google")
on both functions) so they share the same rate-limit scope; ensure the
import/limiter setup supports shared_limit if needed.
In `@backend/test_auth.py`:
- Around line 297-306: The test currently permits silent false positives by
logging when no 429 was seen; change it to assert that the limiter was hit
(raise an AssertionError with a clear message if rate_limit_hit is False) so the
test fails when no 429 occurred, and update the recovery check: replace
time.sleep(1) with a wait that exceeds the configured window for the 5/minute
policy (e.g., sleep(61) or use the policy window variable) and assert
r_recovery.status_code indicates success (e.g., 200) rather than just logging
success; reference variables/functions rate_limit_hit, attempts, info, url_get,
and r_recovery when making these changes.
In `@backend/turnstile.py`:
- Line 10: The module currently captures TURNSTILE_SECRET_KEY (and other
env-derived values) at import time; change this by removing the module-level
TURNSTILE_SECRET_KEY and any other env.get() calls and instead read
os.environ.get('TURNSTILE_SECRET_KEY', '') inside the verification routine
(e.g., verify_turnstile or whatever function performs the call), or accept a
secret_key argument passed in and fall back to reading the env there; update
references to use the local variable so runtime calls reflect env changes (e.g.,
after load_dotenv).
In `@README.md`:
- Around line 117-120: The blockquote in README.md contains a blank line
breaking the quote (triggering MD028); fix it by making the blockquote
contiguous: ensure each quoted line begins with ">" (e.g. for the two paragraphs
starting "Note: When using production auth flows..." and "Authentication start
requests...") and remove the empty line between them (or combine them into a
single continuous blockquote), so there are no blank lines inside the
blockquote.
In `@src/lib/useTurnstile.ts`:
- Around line 54-57: The hook useTurnstile leaves a stale error state set via
setError, causing execute() to keep throwing after a later successful
initialization; update the useEffect branches in useTurnstile (the siteKey check
and the other effect blocks around lines noted) to clear the error on
retry/success by calling setError(undefined/null) when you setReady(true) or
when re-initializing, and also clear error when calling reset/cleanup paths so
execute() sees a fresh state; locate functions/vars useTurnstile, setReady,
setError, execute and ensure every successful setup path resets the error state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ee5eb6e-3801-4c61-8d05-33c7ad4ca6f7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.env.example.github/workflows/greetings.yml.gitignoreREADME.mdbackend/.env.examplebackend/main.pybackend/requirements-base.txtbackend/test_auth.pybackend/turnstile.pysrc/lib/api.tssrc/lib/useTurnstile.tssrc/pages/AuthPage.tsx
| if not rate_limit_hit and attempts >= 5: | ||
| info("Rate limiting may take time to accumulate; requests might not trigger immediately.") | ||
| info(f" (Made {attempts} requests without hitting 429 limit)") | ||
|
|
||
| # Wait a bit and try again to confirm recovery | ||
| time.sleep(1) | ||
| r_recovery = requests.get(url_get, allow_redirects=False, timeout=10) | ||
| if r_recovery.status_code != 429: | ||
| info("✓ Rate limit counter appears to reset after brief wait") | ||
|
|
There was a problem hiding this comment.
Rate-limit test currently allows false positives.
If no 429 is hit, the test only logs and still passes; and the 1-second “recovery” check is not valid for a 5/minute policy. This should fail when the limiter isn’t enforced.
Proposed fix
- if not rate_limit_hit and attempts >= 5:
- info("Rate limiting may take time to accumulate; requests might not trigger immediately.")
- info(f" (Made {attempts} requests without hitting 429 limit)")
+ if not rate_limit_hit:
+ fail(f"Rate limiting was not enforced after {attempts} requests")
- # Wait a bit and try again to confirm recovery
- time.sleep(1)
- r_recovery = requests.get(url_get, allow_redirects=False, timeout=10)
- if r_recovery.status_code != 429:
- info("✓ Rate limit counter appears to reset after brief wait")
+ # Wait for server-provided retry window before checking recovery
+ retry_after = int(r.headers.get("Retry-After", "60")) if rate_limit_hit else 60
+ time.sleep(retry_after)
+ r_recovery = requests.get(url_get, allow_redirects=False, timeout=10)
+ if r_recovery.status_code == 429:
+ fail("Rate limit did not recover after Retry-After window")
+ ok("Rate limit recovered after Retry-After window ✓")🤖 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 `@backend/test_auth.py` around lines 297 - 306, The test currently permits
silent false positives by logging when no 429 was seen; change it to assert that
the limiter was hit (raise an AssertionError with a clear message if
rate_limit_hit is False) so the test fails when no 429 occurred, and update the
recovery check: replace time.sleep(1) with a wait that exceeds the configured
window for the 5/minute policy (e.g., sleep(61) or use the policy window
variable) and assert r_recovery.status_code indicates success (e.g., 200) rather
than just logging success; reference variables/functions rate_limit_hit,
attempts, info, url_get, and r_recovery when making these changes.
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| TURNSTILE_SECRET_KEY = os.environ.get('TURNSTILE_SECRET_KEY', '') |
There was a problem hiding this comment.
Avoid import-time snapshot of TURNSTILE_SECRET_KEY.
Because this value is captured at import time, later env loading (e.g., via load_dotenv) won’t update it. Read the secret inside verification instead.
Proposed fix
-TURNSTILE_SECRET_KEY = os.environ.get('TURNSTILE_SECRET_KEY', '')
+def _get_turnstile_secret_key() -> str:
+ return os.environ.get("TURNSTILE_SECRET_KEY", "")
async def verify_turnstile_token(
turnstile_token: Optional[str],
remote_ip: Optional[str] = None,
) -> None:
- if not TURNSTILE_SECRET_KEY:
+ secret = _get_turnstile_secret_key()
+ if not secret:
raise HTTPException(
status_code=500,
detail='Turnstile secret key is not configured. Set TURNSTILE_SECRET_KEY.',
)
@@
payload = {
- 'secret': TURNSTILE_SECRET_KEY,
+ 'secret': secret,
'response': turnstile_token,
}Also applies to: 16-27
🤖 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 `@backend/turnstile.py` at line 10, The module currently captures
TURNSTILE_SECRET_KEY (and other env-derived values) at import time; change this
by removing the module-level TURNSTILE_SECRET_KEY and any other env.get() calls
and instead read os.environ.get('TURNSTILE_SECRET_KEY', '') inside the
verification routine (e.g., verify_turnstile or whatever function performs the
call), or accept a secret_key argument passed in and fall back to reading the
env there; update references to use the local variable so runtime calls reflect
env changes (e.g., after load_dotenv).
| > Note: When using production auth flows, set `VITE_TURNSTILE_SITE_KEY` in the frontend and `TURNSTILE_SECRET_KEY` in the backend. This enables Cloudflare Turnstile protection for auth endpoint requests. | ||
|
|
||
| > Authentication start requests are rate limited in the backend to 5 requests per minute. Invalid or missing Turnstile tokens are rejected with a standard 400 response. | ||
|
|
There was a problem hiding this comment.
Fix blockquote formatting to satisfy markdownlint.
There is a blank line inside a blockquote block, which triggers MD028. Keep the blockquote contiguous.
Suggested patch
> Note: When using production auth flows, set `VITE_TURNSTILE_SITE_KEY` in the frontend and `TURNSTILE_SECRET_KEY` in the backend. This enables Cloudflare Turnstile protection for auth endpoint requests.
-
> Authentication start requests are rate limited in the backend to 5 requests per minute. Invalid or missing Turnstile tokens are rejected with a standard 400 response.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > Note: When using production auth flows, set `VITE_TURNSTILE_SITE_KEY` in the frontend and `TURNSTILE_SECRET_KEY` in the backend. This enables Cloudflare Turnstile protection for auth endpoint requests. | |
| > Authentication start requests are rate limited in the backend to 5 requests per minute. Invalid or missing Turnstile tokens are rejected with a standard 400 response. | |
| > Note: When using production auth flows, set `VITE_TURNSTILE_SITE_KEY` in the frontend and `TURNSTILE_SECRET_KEY` in the backend. This enables Cloudflare Turnstile protection for auth endpoint requests. | |
| > Authentication start requests are rate limited in the backend to 5 requests per minute. Invalid or missing Turnstile tokens are rejected with a standard 400 response. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 118-118: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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 `@README.md` around lines 117 - 120, The blockquote in README.md contains a
blank line breaking the quote (triggering MD028); fix it by making the
blockquote contiguous: ensure each quoted line begins with ">" (e.g. for the two
paragraphs starting "Note: When using production auth flows..." and
"Authentication start requests...") and remove the empty line between them (or
combine them into a single continuous blockquote), so there are no blank lines
inside the blockquote.
Source: Linters/SAST tools
| useEffect(() => { | ||
| if (!siteKey) { | ||
| setReady(false); | ||
| return; |
There was a problem hiding this comment.
Clear stale Turnstile error state on retry/success.
Once error is set, it is never reset on a later successful setup, so execute() keeps throwing and auth can stay blocked after transient failures.
Suggested patch
useEffect(() => {
if (!siteKey) {
setReady(false);
+ setError(null);
return;
}
let canceled = false;
async function setup() {
try {
+ setError(null);
await loadTurnstileScript();
if (canceled) return;
if (!window.turnstile || !containerRef.current) {
throw new Error('Turnstile is unavailable in this environment.');
}
@@
}
+ setError(null);
setReady(true);
} catch (err) {
if (!canceled) {
const loadError = err instanceof Error ? err : new Error('Unknown Turnstile load error');
setError(loadError);Also applies to: 96-101, 117-119
🤖 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 `@src/lib/useTurnstile.ts` around lines 54 - 57, The hook useTurnstile leaves a
stale error state set via setError, causing execute() to keep throwing after a
later successful initialization; update the useEffect branches in useTurnstile
(the siteKey check and the other effect blocks around lines noted) to clear the
error on retry/success by calling setError(undefined/null) when you
setReady(true) or when re-initializing, and also clear error when calling
reset/cleanup paths so execute() sees a fresh state; locate functions/vars
useTurnstile, setReady, setError, execute and ensure every successful setup path
resets the error state.
|
Description
Checklist
npm run lintpasses with no errorsnpm run buildcompiles without TypeScript errorspython -m pytestpasses (including new tests I added).envfiles, API keys, secrets, model weights, or__pycache__in this diffmain, not mergedSummary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores