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

RateLimiter: perPeriod/periodSeconds should be configurable per middleware/request #23

Open
jrdnull opened this issue Apr 17, 2018 · 3 comments

Comments

@jrdnull
Copy link

jrdnull commented Apr 17, 2018

Brought up in the unified api doc:

Price Plans per product may sell increased Rate Limits as a feature which needs to be taken in to account

Currently the perPeriod/periodSeconds would be shared between multiple instances of the middleware, this should be configurable so you can apply different rate limiters on different endpoints.

To meet the requirements of the quote above you would need to check the session to see if they had a higher rate limit, so perhaps it should change to use a function to return the perPeriod/perSeconds so it can be different depending on the session similar to the GetKey callback.

@jrdnull jrdnull changed the title RateLimiter: perPeriod/periodSeconds should be configurable per middleware RateLimiter: perPeriod/periodSeconds should be configurable per middleware/request Apr 17, 2018
@ready4god2513
Copy link
Contributor

Anyone have a good idea on how to do this without destroying the database? If the account level needs to be checked then it means that we are authing before running the rate limit check which would mean that a malicious actor could send large numbers of requests causing spikes on our infrastructure.

I suppose installations should be cached is the simplest answer.

@arp242
Copy link
Contributor

arp242 commented May 30, 2018

The way I had planned to do it in deskedge (but haven't done yet) is to cache it in Redis; so that way you only need to get it from the DB once.

Caching in a map would be even better/faster, but cache invalidation for that in case someone changes their plan will be a bit more involved/ugly.

@ready4god2513
Copy link
Contributor

Yeah, that is probably the best way to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants