abersheeran / asgi-ratelimit

A ASGI Middleware to rate limit
Apache License 2.0
292 stars 11 forks source link

Any way to get apply limit on entire app or a group of endpoints #32

Closed adityaguru149 closed 3 years ago

adityaguru149 commented 3 years ago

Please consider this a Q or a feature request

I have 3 endpoints

from typing import Tuple

from fastapi import FastAPI
from ratelimit import RateLimitMiddleware, Rule
from ratelimit.backends.redis import RedisBackend
from ratelimit.types import Scope

app = FastAPI()

async def auth_func(scope: Scope) -> Tuple[str, str]:
    print('user1', 'customer1')
    return ('user1', 'customer1')

app.add_middleware(
    RateLimitMiddleware,
    authenticate = auth_func,
    backend=RedisBackend(host='localhost', port=6379),
    config={
        r"^/message": [Rule(minute=2, group='customer1')],
        r"^/": [Rule(minute=1, group='customer1')],
    },
)

@app.get("/")
async def root():
    return {"message": "Hello World"}

@app.get("/hi")
async def root():
    return {"message": "Hi World"}

@app.get("/message")
async def root():
    return {"message": "message"}

I would like to handle /message separately and the others separately as a group.
Ex- if there is 1 request to either of / or /hi then it should not allow another for that period(1 min) to either of them.

As far as I can understand the code base, we need another backend? or should it be another config parser?
Thoughts?

euri10 commented 3 years ago

I think your config is good, just need a little star in the 2nd rule, I've not tested it but this might do the job

    config={
        r"^/message": [Rule(minute=2, group='customer1')],
        r"^/*": [Rule(minute=1, group='customer1')],
    },

the 1st rule will match /message, I just added a star on the 2nd one to catch either / or /hi

adityaguru149 commented 3 years ago

Thanks @euri10 I am not sure, if I was clear enough in the Q. My phrasing probably was not up to the mark.

Another attempt - I basically want to limit a set of APIs as a whole. Ex - If I have Rule(minute=3, group='customer1') for a group of endpoints / and /hi, and user1 belonging to customer1 makes 1 call to / and then 2 calls to /hi then the quota of 3 calls per minute to the group should have been exhausted and user1 should get restricted for further calls to both / and /hi.

abersheeran commented 3 years ago

Ex - If I have Rule(minute=3, group='customer1') for a group of endpoints / and /hi, and user1 belonging to customer1 makes 1 call to / and then 2 calls to /hi then the quota of 3 calls per minute to the group should have been exhausted and user1 should get restricted for further calls to both / and /hi.

The current limit is based on the path, and the current limit of each path is calculated independently. There is currently no way to share traffic limits between multiple paths.

adityaguru149 commented 3 years ago

@abersheeran Thanks So, to accommodate such a request we would need a separate Backend? or another config parser? Any pointers would be helpful.

adityaguru149 commented 3 years ago

@euri10 You can check nginx, it has a zone parameter which groups a set of APIs together.

euri10 commented 3 years ago

sorry I misunderstood you :ear: that zone concept is interesting, I'm not familiar with how it works in nginx but i suppose you have to specify for each and every endpoint the zone it belongs to ?

adityaguru149 commented 3 years ago

@euri10 AFAIK a zone is a setting and to whichever endpoint it is applied, it gets applied for all sub-paths and as a group. You can check in the above blog they give an example below.

@abersheeran I went through the code, I guess for now I can try to inherit Rule to be able to apply it across the entire app but for accommodating the grouping, API changes might be required. Thoughts?

abersheeran commented 3 years ago

Use #33 to solving it

    config={
        r"^/message": [Rule(minute=2, group='customer1')],
        r"^/*": [Rule(minute=1, group='customer1', zone="common")],
    },