-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Proposal: Custom rate limiting rules #1232
Comments
While I agree that this would be a nice to have feature, for now it remains "unplanned" because there are way too many other issues with higher priority that represents a blocker for some use cases (multi-match and I'm not even sure that I'll accept a PR if someone decide to work on this. I want to keep PocketBase as simple as possible. Maybe the better approach in the long run will be to prioritize the Admin UI extendability (#898) to allow users to create plugins that eventually may get integrated/embedded in the core. I'll try to reorganize and update the roadmap after the upcoming few releases. |
Sure, I was just posting this so that we can track possible future development here.
At this point, I would even say that inbuilt rate-limiting would keep PocketBase more simple. Running production apps will always require to rate limit the creation of users and stuff like posts. Which means, that every PocketBase instance has to run behind a reverse proxy with a lot of custom rules to different API endpoints, to be production ready. This makes a PocketBase deployment much harder, than just deploying the single binary on a rather cheap VPS / Droplet.
I hope the funding will increase! Really love the project and your commitment. You totally deserve it!
I am very excited for this! But I think rate-limiting might be too major, to be completely written by community plugins.
I am not sure how much maintenance this would take. If I am not mistaken, it can be split up into:
As the rate-limiting middleware comes with echo directly, I think the maintenance on the rate-limiting itself would be small? The biggest part would probably be adding the middleware dynamically and saving it? |
A far-fetched idea would be to make it possible to add middlewares in EventHooks (if that's possible to implement). A rate limiting route could then be implemented like this, when PocketBase is used as a framework: userCreateLimiter := middleware.RateLimiter(middleware.NewRateLimiterMemoryStore(100))
app.OnRecordBeforeCreateRequest().Add(func(e *core.RecordCreateEvent) error {
if e.Record.Collection().Name == "users" {
return e.Use(userCreateLimiter)
}
// ...
return nil
}) |
If you want to use the default echo rate-limiter middleware you can register it to the app.OnBeforeServe().Add(func(e *core.ServeEvent) error {
e.Router.Use(middleware.RateLimiterWithConfig(yourConfig))
return nil
}) You can specify to which routes to apply by using the We can't register middlewares inside other event hooks because they are triggered more than once. Additionally the echo In any case, as mentioned previously for me this personally is a very low priority. |
I made my own custom middleware, and I think that should be enough for now. If anyone is interested, here is the implementation: Middleware// RateLimitCollectionOperations is a middleware that rate limits HTTP requests on collection operations.
// Possible operations:
// - list
// - view
// - create
// - update
// - delete
func RateLimitCollectionOperations(collection string, operations []string, limiter echo.MiddlewareFunc) echo.MiddlewareFunc {
return func(next echo.HandlerFunc) echo.HandlerFunc {
return func(c echo.Context) error {
contains := func(s string, list []string) bool {
for _, v := range list {
if v == s {
return true
}
}
return false
}
collectionEndpoint := fmt.Sprintf("/api/collections/%s/records", collection)
if contains("list", operations) && c.Request().URL.Path == collectionEndpoint && c.Request().Method == http.MethodGet {
return limiter(next)(c)
} else if contains("view", operations) && strings.HasPrefix(c.Request().URL.Path, collectionEndpoint) && c.Request().Method == http.MethodGet {
return limiter(next)(c)
} else if contains("create", operations) && strings.HasPrefix(c.Request().URL.Path, collectionEndpoint) && c.Request().Method == http.MethodPost {
return limiter(next)(c)
} else if contains("update", operations) && strings.HasPrefix(c.Request().URL.Path, collectionEndpoint) && c.Request().Method == http.MethodPatch {
return limiter(next)(c)
} else if contains("delete", operations) && strings.HasPrefix(c.Request().URL.Path, collectionEndpoint) && c.Request().Method == http.MethodDelete {
return limiter(next)(c)
}
return next(c)
}
}
} Usagefunc main() {
app := pocketbase.New()
limiter := middleware.RateLimiter(middleware.NewRateLimiterMemoryStore(2))
app.OnBeforeServe().Add(func(e *core.ServeEvent) error {
// Rate limit list and view operations on the "posts" collection
e.Router.Use(RateLimitCollectionOperations("posts", []string{"list", "view"}, limiter))
return nil
})
app.Start()
}
|
We need a login abuse limiter as well |
I think the most common approach for login and registration abuse prevention is implementing a captcha. There are ones that can be configured to only prompt the user, if suspicious activities are detected. Even with basic rate limiting, logins could still be brute forced with proxy lists. For basic rate limiting registrations, you can use the middleware I posted earlier (with a |
This comment was marked as spam.
This comment was marked as spam.
This would be a great addition to PocketBase! Max requests per ip/user per action would be amazing! |
While this feature is needed in production, I'm not sure it belongs to Pocketbase itself. https://www.nginx.com/blog/rate-limiting-nginx/ |
@adam-ah Many highly scalable applications run behind a reverse proxy, of course. But PocketBase is an all-in-one solution. Therefore I think it should not be necessary to put a reverse-proxy in between. Also, some routes need a different rate-limit. When setting up a reverse-proxy you would have to know each API route and limit it individually, which is quite a time-consuming task. Especially when apps get new features, you would have to evaluate each time if the reverse-proxy config needs to be adjusted. |
I think the right question to ask is "Is a rate limiter needed?" If the answer is "yes," then in the spirit of PocketBase's tagline, "Open Source backend for your next SaaS and Mobile app in 1 file," then rate limiting should be included in PocketBase. Personally, I think rate limiting is an important feature, as all services exposed to the internet of any interest at all will suffer some attack. It is certainly as important as some of the nice-to-have features already slated to be implemented. |
To add to alternatives, you can also apply a rate limit on a dns level. Cloudflare makes it easy to configure a WAF |
You cannot apply "rate limit on a dns level", whatever that means, @TheRedSpy15 |
DNS level referring to using your domain provider. But no. You are correct, not technically DNS level. You can still run a reverse proxy such as nginx directly in front of the application. I suggested it only as an additional counter measure |
It needs some tests but a builtin rate limiter is implemented as part of the ongoing refactoring and will be available with the next v0.23.0 release (there are no hard ETAs but I expect to finalize at least an RC of the new Go APIs within a month or two at worst). The planned rate limiter implementation is part of a new "Security" PocketBase Settings section. The implementation differs a little from the proposal and it is not tightly coupled to the collections in order to allow managing rate limits for custom user defined routes in a unified manner. This is currently done using "tags". The general idea is that users will have an option to define several "tagged" rate limit Note that we don't have to pass a concrete PocketBase The builtin endpoints have the For example, the create action for the "users" collection will have something like All For now the Based on my local tests it performs fine and I wasn't able to cause lock contention once replaced the regular For simplicity the unique client identifier initially will be only the client IP. For the cases where PocketBase is deployed behind a reverse proxy there is an option in the settings for users to explicit mark On every rate limiter setting change we reset all collected rates data. In case the rate limiter is disabled from the settings the |
Preliminary: I know that rate limiting has been proposed before (in #387), but due to the lack of a description, I would like to submit a justified proposal herewith.
Description
I propose the option to implement basic rate-limiting to specific API endpoints. Most, if not all apps, would benefit from rate-limiting user and record creation. For this proposal, I want to create an imaginary app that is like Instagram.
Attack vector
Servers / apps will, sooner or later, be attacked.
Current solution
The current solution requires the use of a reverse-proxy. This would be feasible, but the programmer would have to create individual rules for each route (which are currently not marked as stable, so the rules might need to be updated frequently). With larger apps this is hardly possible to keep synchronous and would require the usage of lots of wildcards (posts creation might have a different rate-limit than post-view for example). Also, some people do not have a reverse-proxy or are not capable to set one up.
Proposed solution
I propose that an option is added to each collection, how rate-limiting should be handled. This option could be embedded in the existing API rules. A simple implementation could look like this:
As a quick mockup, this could look like:
![image](https://user-images.githubusercontent.com/31022056/206884363-d278cbbd-fed2-4162-b182-500925294e94.png)
![image](https://user-images.githubusercontent.com/31022056/206884774-7579789e-4a8c-4f0f-be59-7a926f5a2cd3.png)
Maybe this could be integrated in the API Rule syntax somehow, but I'll leave that up to you :)
This could also be extended, so that unauthorized users, or users which fit a requirement (like a role), have different rate-limits.
Please let me know what you think, in my opinion this would be a very useful security feature!
The text was updated successfully, but these errors were encountered: