hypothesis / h

Annotate with anyone, anywhere.
https://hypothes.is/
BSD 2-Clause "Simplified" License
2.95k stars 427 forks source link

White list internal requests made from lms-h from nginx rate limiting #5649

Closed hmstepanek closed 5 years ago

hmstepanek commented 5 years ago

Problem

The lms service issues some h api requests directly via an internal path (aka: doesn't go through Cloudflare). There are two problems with this:

  1. It causes a network delay in lms to h api requests.
  2. In a class of ~ 15 students, the students hit rate limiting when trying to register through lms for the first time because they are unauthenticated and all coming from the same ip address (the school's ip).

Solution

Requests that internally happen between our services should not be rate limited. We should instead rate limit at the external layer. So all incoming external requests get rate limited at the first service they reach but internally between our services the requests are allowed to proceed straight through to the service. This simplifies debug and also simplifies the rate limiting because the only thing each service has to concern itself with in regards to rate limiting are external requests.

All external requests have a Cloudflare ip header. Internal requests do not have this header. The order of priority on rate limiting is:

  1. Rate limit based on the authenticated user header.
  2. If the authenticated user header is not present fall back to the Cloudflare ip header.
  3. If there is not a Cloudflare ip header assume it's an internal request and do not rate limit.

The nginx rate limiting configuration is here.

Validation

I find vegeta is really useful for simulating requests rates.

Tests

  1. Configure a vegeta run (we'll call this runX) to hammer a rate limited endpoint with a Cloudflare header set to ip A at a request rate over the allowed amount and another vegeta run (we'll call this runY) to hammer the same rate limited endpoint at a request rate under the allowed amount with an authorization header and a Cloudflare header set to ip A.

    • RunX should be rate limited but RunY should not.
  2. Configure a vegeta run (we'll call this runX) to hammer a rate limited endpoint with a Cloudflare header set to ip A at a request rate over the allowed amount and another vegeta run (we'll call this runY) to hammer the same rate limited endpoint at a request rate above the allowed amount with no headers.

    • RunX should be rate limited but RunY should not.
  3. Configure a vegeta run (we'll call this runX) to hammer a rate limited endpoint with a Cloudflare header set to ip A at a request rate over the allowed amount and another vegeta run (we'll call this runY) to hammer the same rate limited endpoint at a request rate under the allowed amount with a Cloudflare header set to ip A.

    • RunX and RunY should be rate limited.
  4. Configure a vegeta run (we'll call this runX) to hammer a rate limited endpoint with a Cloudflare header set to ip A and an authorization header set to userA at a request rate over the allowed amount and another vegeta run (we'll call this runY) to hammer the same rate limited endpoint at a request rate under the allowed amount with a Cloudflare header set to ip B and an authorization header set to userA.

    • RunX and RunY should be rate limited.
  5. Configure a vegeta run (we'll call this runX) to hammer a rate limited endpoint with a Cloudflare header set to ip A and an authorization header set to userA at a request rate over the allowed amount and another vegeta run (we'll call this runY) to hammer the same rate limited endpoint at a request rate above the allowed amount with no headers.

    • RunX should be rate limited but RunY should not.
dmfine commented 5 years ago

Running, seems to work in QA @hmstepanek

seanh commented 5 years ago

Suggestion from DevOps sync: only rate limit requests that do have a Cloudflare header