Rate limiting should result in consistent return types and be appropriately documented

[jwoytek]

Prerequisites

• Put an X between the brackets on this line if you have done all of the following:
  • Checked the FAQs on the message board for common solutions: (TBD)
  • Checked that your issue isn't already filed.

Description

Rate limiting incoming requests is supported and currently appears to be in place in production. At least one layer of rate limiting in production is returning 403 Forbidden, where 429 Too Many Requests might be more appropriate.

• Document the rate limiting environment variables accepted by the code to configure request rate limiting.
• Consider returning 429 Too Many Requests for all types of rate limiting to help users understand and deal with limits.

Steps to Reproduce

1. Exceed the rate limit(s) with API requests.

Expected behavior:

Rate limit issues should return a 429 Too Many Requests, which clients can choose to retry after a timeout.

Actual behavior:

Some rate limits in production return a 403 Forbidden, which most clients will not attempt to retry unless configured explicitly to do so.

Reproduces how often:

Consistently.

Versions

Production through current.

Additional Information

[zmanion]

An additional request would be to document the rate limits, or if they change frequently, at least note that they exist. Somewhat documented here https://github.com/CVEProject/cve-services/blob/dev/.env.

[david-rocca]

I am currently working this ticket.

In CVE-Services, our middleware that triggers on a rate limiting event will always return a 429. It appears that one of the limiting levels that we have (outside of services itself) is throwing the 403. I will update the documentation for the ENV vars in the mean time, but I will get with a few members of the infrastructure team to see if we can track down that 403 being thrown.

Thanks.

[david-rocca]

After some review, we have found the rogue 403. I have a request out to our cloud team to rectify that to become a 429. I am going to close this ticket after the merging in of the documentation of the ENV vars as this is no longer something that can be handled in services. Thanks for bringing this up!

[jwoytek]

That's great @david-rocca ! Thank you!