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

Ratelimit doesn't work after token expires #31

Open
himalacharya opened this issue Jan 8, 2021 · 4 comments
Open

Ratelimit doesn't work after token expires #31

himalacharya opened this issue Jan 8, 2021 · 4 comments

Comments

@himalacharya
Copy link

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
Copy link
Owner

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

@himalacharya
Copy link
Author

himalacharya commented Jan 11, 2021

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

@himalacharya himalacharya reopened this Jan 11, 2021
@laurentS
Copy link
Owner

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
Copy link
Owner

@himalacharya Is this still an issue?

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

2 participants