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.

Sign up for GitHub

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

Jump to bottom

Rate Limiting By IP #1283

Open
AlexGilleran opened this issue Jun 23, 2018 · 1 comment · May be fixed by #2931
Open

Rate Limiting By IP #1283

AlexGilleran opened this issue Jun 23, 2018 · 1 comment · May be fixed by #2931
Labels
good first issue Magda Future priority: medium refined Issues that have been refined, reviewed, estimated and are ready to work on

Comments

@AlexGilleran
Copy link
Contributor

AlexGilleran commented Jun 23, 2018 •
edited
Loading

Description

Currently it's possible for anyone to hit any API as fast as they want. This has been mostly fine so far because the public APIs we expose aren't really that expensive (they just retrieve info which can be cached anyway), but it'd be wise to build this in, especially as we start exposing other, more expensive operations via API keys.

Acceptance Criteria

  • If logged out, rate limits should be per IP address. If logged in, rate limit should be per user. (NOTE: If doing it by user id turns out to be an order of magnitude harder than doing just by IP, we can split this ticket up and do by IP then figure out how to do by user id).
  • Rate limits should be configurable without changing the code, by number of requests / time. i.e. you should be able to configure both the number of requests and the time period in that equation
  • If rate limit is exceeded, all requests after that point should return 429 with an indication of when the next request can be made (try to do this in the most standard way possible).
  • Needs to be able to work behind a CDN - i.e. all requests might come in from exactly the same IP, so it needs to be able to look at a configurable header in order to rate-limit unauthenticated requests
  • Needs to work with multiple gateways instances... i.e. keeping state in the gateway memory itself isn't a great idea, because that means that the rate limit is effectively doubled/tripled/etc as you add more instances, and if an instance restarts so does the limit.
    • Probably the best way to engineer around this is to use a new postgres database as the store of limits, we'll have to monitor how this affects performance though
  • Needs to be fully disable-able - if you don't turn this on, it shouldn't rate-limit and it shouldn't set up any new database or other infrastructure until you do

Technical Notes
@aneesha09 aneesha09 added this to the Sprint 15 milestone Jul 25, 2018
@aneesha09 aneesha09 modified the milestones: Sprint 15, Sprint 16 Aug 1, 2018
@aneesha09 aneesha09 removed this from the Sprint 16 milestone Aug 15, 2018
@AlexGilleran AlexGilleran added the refined Issues that have been refined, reviewed, estimated and are ready to work on label Jul 7, 2020
@sajidanower23 sajidanower23 self-assigned this Aug 11, 2020
@sajidanower23
Copy link
Contributor

sajidanower23 commented Aug 11, 2020 •
edited
Loading

Adding my note on rate limiting in #2883

Rate Limiting

Rate limiting is important to prevent the abuse of our API, since API keys are
being provided for free.

Express has a library express-rate-limit.
that can be used to rate-limit endpoints as express middleware.

Code for that would look like this:

import rateLimit from 'express-rate-limit';

export const rateLimiter = rateLimit({
  windowMs: 60 * 1000, // 1 minute, in milliseconds
  max: 1000,
  message: 'You have exceeded the API rate limit',
  headers: true,
});

router.get('/endpoint', rateLimiter, function(req, res) {
    return "Woah";
})

This will result in denial of requests if the limit is exceeded.

To go into more depth, check out these articles:

https://nordicapis.com/everything-you-need-to-know-about-api-rate-limiting

https://blog.logrocket.com/rate-limiting-node-js

@sajidanower23 sajidanower23 linked a pull request Aug 14, 2020 that will close this issue
Draft
3 tasks
@sajidanower23 sajidanower23 removed their assignment Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Magda Future priority: medium refined Issues that have been refined, reviewed, estimated and are ready to work on
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rate limiting magda-io/magda
3 participants
@AlexGilleran @sajidanower23 @aneesha09