Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sveltekit-rate-limiter & Fix Rate Limiter Issue #280

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

davidgxue
Copy link
Collaborator

@davidgxue davidgxue commented Jan 24, 2024

Description

  • See Ask Astro Sanic Rate Limiter Incorrectly Blocks Requests by Treating All Requests as a Single IP Source #268 for details of why the error occurs. This won't be explained again in this PR.
  • TL;DR: svelte backend (cloudflare) is sitting in front of the GCP API therefore it is not possible to do IP based rate limiting in sanic. Instead, we must implement it in svelte for now.
  • In production environment, I have set cookie rate limit to be 5 requests per minute, and IP based rate limit to be 100 per day using cloudflare's environment varables

Technical Changes

  • Add sveltekit-rate-limiter
  • Revert the old PR that caused the rate limit issue (where all requests are misinterpreted as the same IP)

Tests

Setup:

  • In Cloudflare, I have set the dev environment to temporarily have these environment varaibles
    image
  1. Test cookie rate limiter. This should trigger after 5 requests using the same browser
Screen.Recording.2024-01-24.at.2.32.14.PM.mov
  1. Using incognito, we can continue to send requests without being blocked. But after 5 more requests we hit our IP based rate limit
image
  1. Now, just to be safe, I used a different computer with VPN and reconnected to the same dev website. And confirmed that I can continue to send additional requests

closes #268

Copy link

cloudflare-pages bot commented Jan 24, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: cb04c91
Status: ✅  Deploy successful!
Preview URL: https://af57f4ba.ask-astro.pages.dev
Branch Preview URL: https://fix-rate-limiter-issue.ask-astro.pages.dev

View logs

@davidgxue davidgxue marked this pull request as ready for review January 24, 2024 22:39
@davidgxue davidgxue self-assigned this Jan 25, 2024
Copy link
Contributor

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Just have one question inline regarding the env vars.

ui/src/routes/+page.server.ts Show resolved Hide resolved
Copy link
Collaborator

@sunank200 sunank200 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@davidgxue davidgxue merged commit 836579a into main Jan 25, 2024
8 checks passed
@davidgxue davidgxue deleted the fix_rate_limiter_issue branch January 25, 2024 22:16
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.

Ask Astro Sanic Rate Limiter Incorrectly Blocks Requests by Treating All Requests as a Single IP Source
3 participants