laurentS / slowapi

A rate limiter for Starlette and FastAPI
https://pypi.org/project/slowapi/
MIT License
1.14k stars 73 forks source link

Ratelimit doesn't work after token expires #31

Open himalacharya opened 3 years ago

himalacharya commented 3 years ago

I am rate limiting on two approaches: i)based on IP address (for endpoint having no access token) Works fine ii) based on user id ( obtained from JWT token) I have used https://pypi.org/project/fastapi-jwt-auth/ for JWTAuth. On the basis of #25 , in limiter.py

from slowapi import Limiter, _rate_limit_exceeded_handler
from slowapi.util import get_remote_address
from slowapi.errors import RateLimitExceeded
from app.utility.config import DEFAULT_RATE_LIMIT
from starlette.requests import Request
from starlette.responses import JSONResponse, Response
from fastapi_jwt_auth import AuthJWT
from app.core.security.security_utils import decrypt_data

class LimiterClass:
    def __init__(self):

        #redis used locally 
        self.limiter = Limiter(key_func=get_user_id_or_ip, strategy="moving-window", default_limits=[DEFAULT_RATE_LIMIT])

"""
Method : Get user_id for JWT access token , IP address for not having token
@Param : 
    1. request : type-> Request
Return: User id or IP address 
"""
def get_user_id_or_ip(request : Request):
    authorize = AuthJWT(request)  # initial instance fastapi-jwt-auth
    authorize.jwt_optional()  # for validation jwt token
    return decrypt_data(authorize.get_jwt_subject()) or get_remote_address

This works fine on unexpired JWT access token and ratelimits user. But when JWT access token expires, then it throws an error.

ERROR:uvicorn.error:Exception in ASGI application
Traceback (most recent call last):
  File "C:\****\anaconda3\lib\site-packages\uvicorn\protocols\http\h11_impl.py", line 394, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "C:\****\anaconda3\lib\site-packages\uvicorn\middleware\proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "C:\****\anaconda3\lib\site-packages\fastapi\applications.py", line 199, in __call__
    await super().__call__(scope, receive, send)
  File "C:\****\anaconda3\lib\site-packages\starlette\applications.py", line 111, in __call__
    await self.middleware_stack(scope, receive, send)
  File "C:\****\anaconda3\lib\site-packages\starlette\middleware\errors.py", line 181, in __call__
    raise exc from None
  File "C:\****\anaconda3\lib\site-packages\starlette\middleware\errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "C:\****\anaconda3\lib\site-packages\starlette\middleware\base.py", line 25, in __call__
    response = await self.dispatch_func(request, self.call_next)
  File "C:\****\anaconda3\lib\site-packages\slowapi\middleware.py", line 51, in dispatch
    return exception_handler(request, e)
  File "C:\****l\anaconda3\lib\site-packages\slowapi\extension.py", line 88, in _rate_limit_exceeded_handler
    {"error": f"Rate limit exceeded: {exc.detail}"}, status_code=429
AttributeError: 'JWTDecodeError' object has no attribute 'detail'

When accesstoken expires, I need to return HTTP 403 Forbidden access message.

laurentS commented 3 years ago

@himalacharya did you find a solution to your problem? can you share here, if it was related to slowapi?

himalacharya commented 3 years ago

The problem is that user information cann't be decoded from expired token. However, above problem is solved by try except method.

class LimiterClass:
    def __init__(self):
        #redis used locally 
        self.limiter = Limiter(key_func=get_user_id_or_ip, strategy="moving-window", default_limits=[DEFAULT_RATE_LIMIT])

def get_user_id_or_ip(request : Request):
    authorize = AuthJWT(request)  # initial instance fastapi-jwt-auth
    try:
        #URL_Path is to find refresh endpoint
        url_path= request.url.path 
        if re.search(r'/refresh$',url_path):               #refresh - Endpoint for refresh token
            #Find refresh token from Authorization headers
            authorize.jwt_refresh_token_required()

        else:
            #If JWT Token is present then get_jwt_object otherwise retrun client IP address
            authorize.jwt_optional()  # for validation jwt token
        return decrypt_data(authorize.get_jwt_subject()) or request.client.host   #decrypt-data: Decrypts user
    except AuthJWTException:
        return request.client.host

For unexpired token, it ratelimits user and for expired tokens , it ratelimits client IP address. For no token, ratelimits client IP address

laurentS commented 3 years ago

My concern with your fix is that you seem to be mixing access control into rate-limiting, which could have security implications. In my mind, your access control layer should block the request if the access token is expired, and slowapi should never even see the request.

Looking at your problem again, have you tried using 2 limiter instances? one for each type of endpoint? I've not tried this before, so I have no idea if it works, but you should be able to share the data store between them, and it might solve your problem.

laurentS commented 1 year ago

@himalacharya Is this still an issue?