Add rate limiting with user and endpoint-specific limits #98
Conversation
❌ Deploy Preview for splitwizer failed. Why did it fail? →
|
WalkthroughRate limiting is introduced to backend user API endpoints using the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI App
participant Limiter (slowapi)
participant User API Endpoint
Client->>FastAPI App: Sends API request (e.g., get/update/delete profile)
FastAPI App->>Limiter (slowapi): Check rate limit for client IP
alt Not rate-limited
Limiter (slowapi)-->>FastAPI App: Allow request
FastAPI App->>User API Endpoint: Process request
User API Endpoint-->>FastAPI App: Return response
FastAPI App-->>Client: Send response
else Rate limit exceeded
Limiter (slowapi)-->>FastAPI App: Block request
FastAPI App-->>Client: Return rate limit error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
ui-poc/Home.py (1)
2-2: Remove unused import.The
datetimeimport is not used anywhere in the file and should be removed.-from datetime import datetimebackend/main.py (1)
13-14: Remove unused imports.These imports are not used in the current implementation and should be removed to keep the code clean.
-from slowapi import _rate_limit_exceeded_handler -from slowapi.errors import RateLimitExceededbackend/utils/limiter_helper.py (3)
1-1: Remove unused import.The
get_remote_addressimport is not used in this file.-from slowapi.util import get_remote_address
8-8: Remove debug print statement.Debug print statements should not be included in production code.
- print(route)
5-11: Consider adding type hints and documentation.The function would benefit from type hints and documentation to clarify its purpose and usage.
+from fastapi import APIRouter + -def limit_all_routes(router, rate: str): +def limit_all_routes(router: APIRouter, rate: str) -> None: + """ + Apply rate limiting to all routes in the given router. + + Args: + router: The FastAPI router to apply limits to + rate: Rate limit string (e.g., "5/minute") + """ for route in router.routes: - - print(route) if hasattr(route, "endpoint"): - route.endpoint = limiter.limit(rate)(route.endpoint)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/app/user/routes.py(3 hunks)backend/main.py(2 hunks)backend/requirements.txt(1 hunks)backend/utils/limiter.py(1 hunks)backend/utils/limiter_helper.py(1 hunks)ui-poc/Home.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
backend/**/*.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Backend code must be implemented using FastAPI with Python 3.9+ and organized under the /backend/ directory.
Files:
backend/utils/limiter_helper.pybackend/app/user/routes.pybackend/main.pybackend/utils/limiter.py
ui-poc/Home.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
ui-poc/Home.py: Frontend POC must be implemented using Streamlit, with the entry point in ui-poc/Home.py.
Streamlit session state must be used to manage user sessions in the frontend, as shown in Home.py.
Files:
ui-poc/Home.py
backend/app/{auth,user,groups,expenses}/**/*.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
backend/app/{auth,user,groups,expenses}/**/*.py: Backend services should be modular, with authentication and user registration in app/auth/, user profile management in app/user/, group management in app/groups/, and expense tracking in app/expenses/.
When adding a new API endpoint, add the route to the appropriate service router file in the backend.
Files:
backend/app/user/routes.py
backend/main.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
The main FastAPI application entry point must be in backend/main.py.
Files:
backend/main.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/{auth,user,groups,expenses}/**/*.py : When adding a new API endpoint, add the route to the appropriate service router file in the backend.
📚 Learning: applies to backend/**/*.py : backend code must be implemented using fastapi with python 3.9+ and org...
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/**/*.py : Backend code must be implemented using FastAPI with Python 3.9+ and organized under the /backend/ directory.
Applied to files:
backend/requirements.txt
📚 Learning: applies to backend/app/{auth,user,groups,expenses}/**/*.py : when adding a new api endpoint, add the...
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/{auth,user,groups,expenses}/**/*.py : When adding a new API endpoint, add the route to the appropriate service router file in the backend.
Applied to files:
backend/utils/limiter_helper.pybackend/app/user/routes.py
📚 Learning: applies to ui-poc/home.py : streamlit session state must be used to manage user sessions in the fron...
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to ui-poc/Home.py : Streamlit session state must be used to manage user sessions in the frontend, as shown in Home.py.
Applied to files:
ui-poc/Home.py
📚 Learning: applies to backend/main.py : the main fastapi application entry point must be in backend/main.py....
Learnt from: CR
PR: Devasy23/splitwiser#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/main.py : The main FastAPI application entry point must be in backend/main.py.
Applied to files:
backend/main.py
🪛 Ruff (0.12.2)
backend/utils/limiter_helper.py
1-1: slowapi.util.get_remote_address imported but unused
Remove unused import: slowapi.util.get_remote_address
(F401)
ui-poc/Home.py
2-2: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
backend/app/user/routes.py
10-10: fastapi.status imported but unused
Remove unused import: fastapi.status
(F401)
57-57: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
backend/main.py
13-13: slowapi._rate_limit_exceeded_handler imported but unused
Remove unused import: slowapi._rate_limit_exceeded_handler
(F401)
14-14: slowapi.errors.RateLimitExceeded imported but unused
Remove unused import: slowapi.errors.RateLimitExceeded
(F401)
18-18: Redefinition of unused limiter from line 16
(F811)
🔇 Additional comments (8)
backend/requirements.txt (1)
21-21: LGTM! slowapi added for rate limiting.The addition of
slowapialigns with the PR objectives to implement rate limiting functionality.backend/utils/limiter.py (1)
1-4: LGTM! Clean rate limiter implementation.The limiter configuration using IP-based rate limiting is appropriate and follows slowapi best practices. The implementation is clean and reusable across the application.
backend/main.py (2)
25-25: LGTM! Proper limiter integration.The limiter is correctly assigned to the app state during startup, making it available for middleware and route decorators.
88-88: LGTM! SlowAPIMiddleware correctly added.The middleware is properly integrated to enable global rate limiting functionality.
backend/app/user/routes.py (4)
17-27: Rate limit appears too permissive for profile retrieval.The current rate limit of 20 requests per minute for retrieving user profile seems high compared to the PR objectives which mentioned 5 requests per minute for GET /users/me. This discrepancy could lead to insufficient protection against abuse.
Consider aligning with the original PR objectives or document the rationale for the higher limit.
-@limiter.limit("20/minute") +@limiter.limit("5/minute")
31-51: Rate limiting implementation looks correct.The 5 requests per minute limit for profile updates aligns with the PR objectives and provides appropriate protection for this sensitive operation.
55-66: Rate limiting implementation is appropriate for account deletion.The 2 requests per minute limit for account deletion is correctly configured and matches the PR objectives. This conservative limit is appropriate for such a critical operation.
57-57: Function signature follows FastAPI dependency injection pattern correctly.The static analysis tool flagged the
Depends()call in the function signature, but this is the standard and recommended pattern for FastAPI dependency injection. The current implementation is correct and should not be changed.
| from fastapi import APIRouter, Depends, HTTPException, Request, status | ||
| from utils.limiter import limiter |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove unused import and verify limiter import path.
The status import from FastAPI is unused and should be removed as indicated by static analysis. Also, verify that the limiter import path is correct relative to the backend directory structure.
-from fastapi import APIRouter, Depends, HTTPException, Request, status
+from fastapi import APIRouter, Depends, HTTPException, Request🏁 Script executed:
#!/bin/bash
# Verify the limiter module exists and the import path is correct
fd -t f "limiter.py" backend/Length of output: 54
Remove unused FastAPI import
The status symbol isn’t referenced in this file, so it can be dropped. The limiter module path is correct (backend/utils/limiter.py), no change needed there.
-from fastapi import APIRouter, Depends, HTTPException, Request, status
+from fastapi import APIRouter, Depends, HTTPException, Request— limiter import verified at backend/utils/limiter.py
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from fastapi import APIRouter, Depends, HTTPException, Request, status | |
| from utils.limiter import limiter | |
| -from fastapi import APIRouter, Depends, HTTPException, Request, status | |
| +from fastapi import APIRouter, Depends, HTTPException, Request | |
| from utils.limiter import limiter |
🧰 Tools
🪛 Ruff (0.12.2)
10-10: fastapi.status imported but unused
Remove unused import: fastapi.status
(F401)
🤖 Prompt for AI Agents
In backend/app/user/routes.py at lines 10 to 11, the import of 'status' from
FastAPI is unused and should be removed to clean up the imports. Keep the import
of 'limiter' as is since it is correctly referenced. Simply delete ', status'
from the import statement on line 10.
| from utils.limiter import Limiter, get_remote_address, limiter | ||
|
|
||
| limiter = Limiter(key_func=get_remote_address) | ||
| # limiter = Limiter(key_func=get_remote_address) |
There was a problem hiding this comment.
Fix duplicate limiter definition.
The limiter is imported from utils.limiter and then redefined locally, creating confusion and redundancy.
-from utils.limiter import Limiter, get_remote_address, limiter
+from utils.limiter import limiter
-limiter = Limiter(key_func=get_remote_address)
-# limiter = Limiter(key_func=get_remote_address)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from utils.limiter import Limiter, get_remote_address, limiter | |
| limiter = Limiter(key_func=get_remote_address) | |
| # limiter = Limiter(key_func=get_remote_address) | |
| from utils.limiter import limiter |
🧰 Tools
🪛 Ruff (0.12.2)
18-18: Redefinition of unused limiter from line 16
(F811)
🤖 Prompt for AI Agents
In backend/main.py around lines 16 to 19, there is a duplicate definition of the
limiter variable: it is both imported from utils.limiter and redefined locally.
Remove the local redefinition of limiter and use the imported limiter directly
to avoid redundancy and confusion.
| fastapi | ||
| uvicorn[standard] | ||
| python-jose[cryptography] | ||
| passlib[bcrypt] | ||
| python-multipart | ||
| pydantic | ||
| pydantic-settings | ||
| pymongo | ||
| motor | ||
| firebase-admin | ||
| python-dotenv | ||
| bcrypt | ||
| email-validator |
There was a problem hiding this comment.
Maintain version pins for production stability.
Removing version pins from all dependencies can lead to unpredictable builds and potential breaking changes when new versions are released. This affects reproducibility and production stability.
Consider maintaining version pins for critical dependencies:
-fastapi
-uvicorn[standard]
-python-jose[cryptography]
-passlib[bcrypt]
-python-multipart
-pydantic
-pydantic-settings
-pymongo
-motor
-firebase-admin
-python-dotenv
-bcrypt
-email-validator
+fastapi>=0.104.0,<1.0.0
+uvicorn[standard]>=0.24.0,<1.0.0
+python-jose[cryptography]>=3.3.0,<4.0.0
+passlib[bcrypt]>=1.7.4,<2.0.0
+python-multipart>=0.0.6,<1.0.0
+pydantic>=2.0.0,<3.0.0
+pydantic-settings>=2.0.0,<3.0.0
+pymongo>=4.5.0,<5.0.0
+motor>=3.3.0,<4.0.0
+firebase-admin>=6.2.0,<7.0.0
+python-dotenv>=1.0.0,<2.0.0
+bcrypt>=4.0.0,<5.0.0
+email-validator>=2.0.0,<3.0.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fastapi | |
| uvicorn[standard] | |
| python-jose[cryptography] | |
| passlib[bcrypt] | |
| python-multipart | |
| pydantic | |
| pydantic-settings | |
| pymongo | |
| motor | |
| firebase-admin | |
| python-dotenv | |
| bcrypt | |
| email-validator | |
| fastapi>=0.104.0,<1.0.0 | |
| uvicorn[standard]>=0.24.0,<1.0.0 | |
| python-jose[cryptography]>=3.3.0,<4.0.0 | |
| passlib[bcrypt]>=1.7.4,<2.0.0 | |
| python-multipart>=0.0.6,<1.0.0 | |
| pydantic>=2.0.0,<3.0.0 | |
| pydantic-settings>=2.0.0,<3.0.0 | |
| pymongo>=4.5.0,<5.0.0 | |
| motor>=3.3.0,<4.0.0 | |
| firebase-admin>=6.2.0,<7.0.0 | |
| python-dotenv>=1.0.0,<2.0.0 | |
| bcrypt>=4.0.0,<5.0.0 | |
| email-validator>=2.0.0,<3.0.0 |
🤖 Prompt for AI Agents
In backend/requirements.txt lines 1 to 13, the dependencies lack version pins,
which can cause unpredictable builds and instability. To fix this, specify exact
or minimum versions for each critical dependency by adding version constraints
(e.g., == or >=) to ensure reproducible and stable production environments.
1 participant
Summary:
Integrated slowapi to add rate limiting across user-related endpoints.
Key Changes:
Added limiter to FastAPI lifespan.
Applied user-specific rate limits:
GET /users/me: 5 requests/min
PATCH /users/me: 3 requests/min
DELETE /users/me: 2 requests/min
Dependencies:
Added slowapi==0.1.8 to requirements.
Summary by CodeRabbit