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 express rate limiting #8170

Closed
3 tasks done
mtrezza opened this issue Sep 14, 2022 · 15 comments · Fixed by #8174
Closed
3 tasks done

Add express rate limiting #8170

mtrezza opened this issue Sep 14, 2022 · 15 comments · Fixed by #8174
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature

Comments

@mtrezza
Copy link
Member

mtrezza commented Sep 14, 2022

New Feature / Enhancement Checklist

Current Limitation

Rate limiting an API is something that is usually done by a separate part in an architecture, before it even reaches Parse Server. The earlier a rate limiting is enforced to prevent DOS attacks, the lower the impact of such an attack. However, not every developer may have the experience or availability of such rate-limiting components.

Feature / Enhancement Description

Parse Server should offer a basic feature for rate limiting:

  • Rate limiting should be on by default, and allow to be turned off in case a custom rate-limiter is used. To achieve this, a new Parse Server option should be introduced.
  • The feature should be phased by defaulting to being deactivated, with a deprecation warning that it will be activated by default.
  • Some (specifically internal) API routes may need to be excluded from limiting.

Example Use Case

The following example shows an Express application that serves static files without rate limiting:

var express = require('express');
var app = express();

app.get('/:path', function(req, res) {
  let path = req.params.path;
  if (isValidPath(path))
    res.sendFile(path);
});

To prevent denial-of-service attacks, the express-rate-limit package can be used:

var express = require('express');
var app = express();

// set up rate limiter: maximum of five requests per minute
var RateLimit = require('express-rate-limit');
var limiter = new RateLimit({
  windowMs: 1*60*1000, // 1 minute
  max: 5
});

// apply rate limiter to all requests
app.use(limiter);

app.get('/:path', function(req, res) {
  let path = req.params.path;
  if (isValidPath(path))
    res.sendFile(path);
});

Alternatives / Workarounds

Require developer to implement a custom rate-limiter.

3rd Party References

@parse-github-assistant
Copy link

parse-github-assistant bot commented Sep 14, 2022

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mtrezza mtrezza added the type:feature New feature or improvement of existing feature label Sep 14, 2022
@mtrezza
Copy link
Member Author

mtrezza commented Sep 19, 2022

How does this work if there are multiple Parse Server instances behind a load balancer? Is the express-rate-limit package even a solution in that case?

@dblythy
Copy link
Member

dblythy commented Sep 19, 2022

express-rate-limit is compatible with reverse proxies

https://github.com/nfriedly/express-rate-limit#troubleshooting-proxy-issues

@mtrezza
Copy link
Member Author

mtrezza commented Sep 19, 2022

Let's say the server instances are behind a load balancer on AWS or GCP, which is a common scenario. Every server would calculate their rate limit by only considering the request itself handled, without considering requests handled by other instances, correct?

@dblythy
Copy link
Member

dblythy commented Sep 19, 2022

That is correct. In that case, wouldn't using AWS WAF/Cloudflare be preferred for rate limiting

@mtrezza
Copy link
Member Author

mtrezza commented Sep 19, 2022

Yes, that's an important limitation we need to explain in the docs; in the future we could overcome this by logging requests in Redis for example, but we are getting into a complex territory which is far from the core functionality of Parse Server and I don't think we should go there. It's also not the best approach to limit on the application layer, a DoS that reached so far already caused some damage.

Also, does express-rate-limit limit the requests per IP, or can a general limit (regardless of IP) be set? Limiting per IP is ineffective in a DDoS attack.

@dblythy
Copy link
Member

dblythy commented Sep 19, 2022

It's also not the best approach to limit on the application layer, a DoS that reached so far already caused some damage.

Yes, I think this is why in the past we have recommended rate limiting at nginx. However I think a per IP rate limit is still better than nothing and has some practical use for specific use cases, such as only allowing one IP to call a cloud function once a day, for example.

We can actually make the rate limit so it links per sessionToken or user for example, so the proxy issues would be resolved.

@mtrezza
Copy link
Member Author

mtrezza commented Sep 19, 2022

I agree that adding a simple rate limiting feature for Parse Server which run as a single instance (w/o load balancer) makes sense.

I think the basis for rate limiting needs to be a basic, common key that all requests have. A session token is unavailable if the request is anonymous. Maybe we should stick with the per-IP address feature and consider it "a simple rate-limiting implementation that is better than nothing and covers a single-instance server scenario in case of a not very sophisticated DoS attack." That means, if someone starts to develop their first project and deploys Parse Server somewhere, they can easily add a simple layer of protection.

We should remember to clarify that all in the docs.

@dblythy
Copy link
Member

dblythy commented Sep 19, 2022

We can use mongoDB as an optional store which would help for multiple instances.

We could do IP address if session is undefined, otherwise request.user.id. I think this would make sense for a cloud function for example, if you define a rate limit you would expect it to be per user.

In the case of limiting the global amount of requests per second (regardless of IP/user), we can add that too. We would just need to add a global rateLimit with the keyGenerator returning the same value.

Perhaps we could add a parameter for limit which can be either:

  • user
  • IP
  • global

This would let the developer define how they want the restriction to function.

Another option could be a slowdown: https://www.npmjs.com/package/express-slow-down

@mtrezza
Copy link
Member Author

mtrezza commented Sep 19, 2022

Let's limit the scope to a simple version of this feature with rate limiting per IP.

All the other ideas are worth to be discussed at some point, but too complex for an initial feature release. I have doubts regarding using MongoDB as a cache and mixing IP address with session tokens for limiting, or even slowing down requests, which can create complex issues with the timeout configuration in a load balancer architecture.

@dblythy
Copy link
Member

dblythy commented Sep 19, 2022

Ok, no worries. Should we still have the option for the limit to be global, regardless of IP (ddos protection)?

@mtrezza
Copy link
Member Author

mtrezza commented Sep 19, 2022

Let’s leave it with IP for now for a basic protection, we can always add more features later on.

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0-alpha.21

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jan 6, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Jan 31, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:feature New feature or improvement of existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants