Skip to content

Commit 85b9a8e

Browse files
committed
fix: add request size
1 parent 3b72794 commit 85b9a8e

22 files changed

Lines changed: 445 additions & 28 deletions

File tree

.env.example

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ REDIS_URL=redis://:password@127.0.0.1:6379/0
77

88
SECRET_KEY=your-super-secret-production-key-here
99

10+
MAX_REQUEST_SIZE_MB=5242880 #5mb
11+
1012
ALGORITHM=HS256
1113
ACCESS_TOKEN_EXPIRE_MINUTES=30
1214
REFRESH_TOKEN_EXPIRE_MINUTES=10080

README.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ The API is currently versioned under `/api/v1`.
2828
- [Docker Notes](#docker-notes)
2929
- [Development Guide](#development-guide)
3030
- [Troubleshooting](#troubleshooting)
31+
- [Security TODO](#security-todo)
3132
- [Known Notes](#known-notes)
3233

3334
## Features
@@ -476,6 +477,51 @@ Authorization: Bearer <token>
476477

477478
The token must contain a `sub` claim with a valid user id.
478479

480+
## Security TODO
481+
482+
Legend: `Implemented` means code exists in the repository. `Partial` means code exists but still needs a fix, test, or production hardening.
483+
484+
| Category | Recommended | Current Status | Notes |
485+
| --- | --- | --- | --- |
486+
| JWT Authentication | Required | Implemented | `AuthenticationMiddleware` validates bearer tokens for non-public routes. |
487+
| Refresh Token Rotation | Required | Implemented | Refresh flow revokes the old refresh token and persists a new token. |
488+
| RBAC + Permissions | Required | Implemented | Casbin-backed role and permission checks are wired through route dependencies. |
489+
| Rate Limiting (Redis-backed) | Required | Partial | Redis-backed limiter exists, but `apply_global_rate_limit` reads `GLOBAL_RATE_LIMIT` while settings expose `RATE_LIMIT`. |
490+
| Security Headers Middleware | Required | Not Implemented | Add headers such as `X-Content-Type-Options`, `X-Frame-Options` or CSP `frame-ancestors`, `Referrer-Policy`, and production CSP. |
491+
| CORS Configuration | Required | Partial | CORS middleware exists, but production origins, methods, and headers should be environment-driven. |
492+
| Request ID Middleware | Required | Not Implemented | Add request/correlation ID generation and response header propagation. |
493+
| Audit Logging | Required | Not Implemented | Add audit events for sensitive auth, user, role, permission, and todo mutations. |
494+
| Structured Logging | Required | Not Implemented | Add structured application logs with request ID, method, path, status, latency, and user context when available. |
495+
| Global Exception Handling | Required | Partial | Exception handlers exist, but `Exception` is registered twice; verify domain and fallback handling behavior. |
496+
| Input Validation | Required | Implemented | Pydantic schemas and application validation functions are used across user and todo flows. |
497+
| Password Hashing (Argon2 or bcrypt) | Required | Implemented | User auth service uses bcrypt hashing. |
498+
| Account Lockout | Required | Not Implemented | Add failed-login tracking and temporary lockout or throttling by account. |
499+
| Token Revocation | Required | Implemented | Refresh tokens are revoked on rotation/logout, and access tokens are denylisted in Redis until expiry. |
500+
| OpenAPI Authentication | Required | Partial | Swagger OAuth2 auth is configured, but `/docs`, `/redoc`, and `/openapi.json` are public; disable them in production or protect them with authentication. |
501+
| Health Check Endpoint | Required | Implemented | `/health` endpoint returns service health. |
502+
| Readiness/Liveness Endpoints | Required | Not Implemented | Add separate readiness and liveness endpoints for deployment orchestration. |
503+
| Request Size Limiting | Required | Implemented | `LimitRequestSizeMiddleware` rejects oversized write requests. |
504+
| Idempotency Support (for applicable POST endpoints) | Optional but valuable | Not Implemented | Consider idempotency keys for retry-safe create/payment-like workflows. |
505+
| Database Migrations | Required | Implemented | Alembic is configured with migration commands in the README and Makefile. |
506+
| Dependency Injection | Required | Implemented | FastAPI dependencies wire repositories, handlers, auth, authorization, and database sessions. |
507+
| Configuration via Environment Variables | Required | Partial | Pydantic settings read `.env`, but production validation should reject unsafe defaults. |
508+
509+
### Next Implementation Checklist
510+
511+
- [ ] Fix and verify rate limit configuration wiring.
512+
- [ ] Add security headers middleware.
513+
- [ ] Add request ID middleware.
514+
- [ ] Add structured request logging.
515+
- [ ] Add audit logging for sensitive actions.
516+
- [ ] Add account lockout or equivalent failed-login protection.
517+
- [ ] Disable or authenticate `/docs`, `/redoc`, and `/openapi.json` in production.
518+
- [ ] Add readiness and liveness endpoints.
519+
- [ ] Add production config validation for secrets and unsafe defaults.
520+
- [ ] Harden CORS through environment-driven allowed origins, methods, and headers.
521+
- [ ] Review exception responses to avoid leaking token parsing details or internal exception messages.
522+
- [ ] Add automated tests for request size limits, rate limiting, auth failures, authorization failures, CORS, security headers, and request IDs.
523+
- [ ] Add dependency vulnerability scanning to local or CI checks, for example `pip-audit` or an equivalent Poetry-compatible scanner.
524+
479525
## Known Notes
480526

481527
- `alembic/env.py` currently prints metadata debug output during migrations.

docker-compose.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ services:
88
depends_on:
99
- db
1010
volumes:
11-
- ./src:/app/src # For hot-reloading during dev
11+
- ./src:/app/src
1212

1313
db:
1414
image: postgres:15-alpine

src/core/bootstrap/middleware.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
from fastapi import FastAPI
22
from fastapi.middleware.cors import CORSMiddleware
33

4+
from src.core.config.setting import get_settings
5+
from src.core.middleware.request_size import LimitRequestSizeMiddleware
46
from src.core.middleware.auth import AuthenticationMiddleware
57

8+
settings = get_settings()
9+
610

711
def register_middleware(app: FastAPI):
12+
app.add_middleware(
13+
LimitRequestSizeMiddleware,
14+
max_upload_size=settings.MAX_REQUEST_SIZE_MB,
15+
)
816
app.add_middleware(
917
CORSMiddleware,
10-
allow_origins=["http://localhost:3000"], # Your frontend URL
18+
allow_origins=["http://localhost:3000"],
1119
allow_credentials=True,
1220
allow_methods=["*"],
1321
allow_headers=["*"],

src/core/config/setting.py

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,34 @@
11
from functools import lru_cache
22

3+
from pydantic import Field
34
from pydantic_settings import BaseSettings, SettingsConfigDict
45

56

67
class Settings(BaseSettings):
7-
APP_NAME: str = "Todo Modulith API"
8-
APP_ENV: str = "development"
9-
DATABASE_URL: str = "postgresql+asyncpg://user:password@localhost:5432/todo_db"
10-
REDIS_URL: str = "redis://:eYVX7EwVmmxKPCDmwMtyKVge8oLd2t81@127.0.0.1:6379/0"
11-
SECRET_KEY: str = "super-secret-key-change-in-production"
12-
ALGORITHM: str = "HS256"
13-
ACCESS_TOKEN_EXPIRE_MINUTES: int = 30
14-
REFRESH_TOKEN_EXPIRE_MINUTES: int = 10080
15-
RATE_LIMIT: str = "100/minute"
8+
APP_NAME: str = Field(alias="APP_NAME", default="Todo Modulith API")
9+
APP_ENV: str = Field(alias="APP_ENV", default="development")
10+
DATABASE_URL: str = Field(
11+
alias="DATABASE_URL",
12+
default="postgresql+asyncpg://user:password@localhost:5432/todo_db",
13+
)
14+
REDIS_URL: str = Field(
15+
alias="REDIS_URL",
16+
default="redis://:eYVX7EwVmmxKPCDmwMtyKVge8oLd2t81@127.0.0.1:6379/0",
17+
)
18+
SECRET_KEY: str = Field(
19+
alias="SECRET_KEY", default="super-secret-key-change-in-production"
20+
)
21+
ALGORITHM: str = Field(alias="ALGORITHM", default="HS256")
22+
ACCESS_TOKEN_EXPIRE_MINUTES: int = Field(
23+
alias="ACCESS_TOKEN_EXPIRE_MINUTES", default=30
24+
)
25+
REFRESH_TOKEN_EXPIRE_MINUTES: int = Field(
26+
alias="REFRESH_TOKEN_EXPIRE_MINUTES", default=10080
27+
)
28+
RATE_LIMIT: str = Field(alias="RATE_LIMIT", default="100/minute")
29+
MAX_REQUEST_SIZE_MB: int = Field(
30+
alias="MAX_REQUEST_SIZE_MB", default=5 * 1024 * 1024
31+
)
1632

1733
model_config = SettingsConfigDict(
1834
env_file=".env",

src/core/middleware/auth.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from starlette.responses import JSONResponse, Response
66

77
from src.core.security.jwt import JWTService
8+
from src.core.security.token_revocation import TokenRevocationService
89

910
PUBLIC_PATHS = frozenset(
1011
{
@@ -43,6 +44,12 @@ async def dispatch(
4344

4445
try:
4546
payload = JWTService.decode_token(token)
47+
if await TokenRevocationService.is_access_token_revoked(token):
48+
return JSONResponse(
49+
status_code=status.HTTP_401_UNAUTHORIZED,
50+
content={"detail": "Token has been revoked"},
51+
)
52+
4653
user_id = payload.get("sub")
4754
if not user_id:
4855
raise ValueError("Token missing 'sub' claim")
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
from fastapi import Request, status
2+
from fastapi.responses import JSONResponse
3+
from starlette.middleware.base import BaseHTTPMiddleware
4+
5+
6+
class LimitRequestSizeMiddleware(BaseHTTPMiddleware):
7+
def __init__(self, app, max_upload_size: int):
8+
super().__init__(app)
9+
self.max_upload_size = max_upload_size
10+
11+
async def dispatch(self, request: Request, call_next):
12+
if request.method in ("POST", "PUT", "PATCH"):
13+
content_length = request.headers.get("content-length")
14+
15+
if content_length:
16+
if int(content_length) > self.max_upload_size:
17+
return JSONResponse(
18+
status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE,
19+
content={
20+
"detail": "Request payload exceeds maximum allowed size."
21+
},
22+
)
23+
else:
24+
return JSONResponse(
25+
status_code=status.HTTP_411_LENGTH_REQUIRED,
26+
content={"detail": "Content-Length header is required."},
27+
)
28+
29+
return await call_next(request)

src/core/security/jwt.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from datetime import datetime, timedelta, timezone
2+
from uuid import uuid4
23

34
from jose import JWTError, jwt
45

@@ -15,7 +16,7 @@ def create_access_token(data: dict) -> str:
1516
expire = datetime.now(timezone.utc) + timedelta(
1617
minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES
1718
)
18-
to_encode.update({"exp": expire})
19+
to_encode.update({"exp": expire, "jti": str(uuid4())})
1920
return jwt.encode(to_encode, settings.SECRET_KEY, algorithm=settings.ALGORITHM)
2021

2122
@staticmethod
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
from datetime import datetime, timezone
2+
3+
from src.core.database.redis.client import get_redis_client
4+
from src.core.security.jwt import JWTService
5+
6+
7+
class TokenRevocationService:
8+
KEY_PREFIX = "revoked_access_token"
9+
10+
@staticmethod
11+
def _key(jti: str) -> str:
12+
return f"{TokenRevocationService.KEY_PREFIX}:{jti}"
13+
14+
@staticmethod
15+
async def revoke_access_token(token: str, redis=None) -> None:
16+
try:
17+
payload = JWTService.decode_token(token)
18+
except Exception:
19+
return
20+
21+
jti = payload.get("jti")
22+
exp = payload.get("exp")
23+
if not jti or not exp:
24+
return
25+
26+
ttl = int(exp - datetime.now(timezone.utc).timestamp())
27+
if ttl <= 0:
28+
return
29+
30+
redis_client = redis or await get_redis_client()
31+
await redis_client.setex(TokenRevocationService._key(jti), ttl, "1")
32+
33+
@staticmethod
34+
async def is_access_token_revoked(token: str, redis=None) -> bool:
35+
try:
36+
payload = JWTService.decode_token(token)
37+
except Exception:
38+
return False
39+
40+
jti = payload.get("jti")
41+
if not jti:
42+
return False
43+
44+
redis_client = redis or await get_redis_client()
45+
return bool(await redis_client.exists(TokenRevocationService._key(jti)))

src/modules/authorization/presenter/routers/permission_router.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from fastapi import APIRouter, Depends, HTTPException, status
44

5-
from core.authorization.dependencies import require_permission
5+
from src.core.authorization.dependencies import require_permission
66
from src.core.authorization.infrastructure.services.casbin_authorization_service import (
77
CasbinAuthorizationService,
88
)

0 commit comments

Comments
 (0)