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

feat: harden rate limits #909

Merged
merged 6 commits into from
Dec 21, 2020
Merged

feat: harden rate limits #909

merged 6 commits into from
Dec 21, 2020

Conversation

mantariksh
Copy link
Contributor

Problem

We currently do not limit the number of submissions per IP per minute. Instead, we only log a message when a threshold is exceeded. This means that in case of a DDoS attempt, there is a risk of downstream systems (particularly those linked via webhook) being affected indirectly.

Solution

Harden the rate-limiting by return HTTP 429.

Some UI issues surfaced as a result of this change. In particular, the /auth/sendotp and submissions endpoints return different error message shapes. As a result, it was not clear what shape the limitRate message should return, since it is shared between those two endpoints.

I decided to change the /auth/sendotp endpoint to return an object with a message key instead of a plain string since this gives us the flexibility to return more error metadata in future. This was technically a backwards-incompatible change, but the impact is minimal since old clients will simply see the default error message if their OTP emails fail to be sent, which is a rare situation anyway.

Screenshots

image

Screenshot 2020-12-21 at 9 30 06 AM

Tests

  • Change the SUBMISSIONS_RATE_LIMIT and AUTH_OTP_RATE_LIMIT env vars to 3 each (choosing 3 instead of 0/1/2 so that hopefully it won't affect the other tests running concurrently)
  • Attempt to make 4 email mode submissions across at least 2 different forms within 1 minute. The 4th submission should return 429 with a "please try again in one minute" error message. Cloudwatch should contain a limitRate log message with your IP address.
  • Make 3 email mode submissions within 1 minute, then another one from a different IP address . The one from the different IP address should go through.
  • Attempt to make 4 encrypt mode submissions across at least 2 different forms within 1 minute. The 4th submission should return 429 with a "please try again in one minute" error message. Cloudwatch should contain a limitRate log message with your IP address.
  • Make 3 email encrypt submissions within 1 minute, then another one from a different IP address . The one from the different IP address should go through.
  • Attempt to resend auth OTP 4 times within 1 minute. The 4th submission should return 429 with a "please try again in one minute" error message. Cloudwatch should contain a limitRate log message with your IP address.

@mantariksh mantariksh merged commit 06f463e into develop Dec 21, 2020
@tshuli tshuli mentioned this pull request Dec 22, 2020
@liangyuanruo liangyuanruo deleted the feat/harden-rate-limit-2 branch December 22, 2020 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet

2 participants