fix: enhance security setting

Author: fiqrikm18

.env.example

@@ -15,4 +15,7 @@ JWT_AUDIENCE=todo-modulith-client
 ACCESS_TOKEN_EXPIRE_MINUTES=30
 REFRESH_TOKEN_EXPIRE_MINUTES=10080
 
-RATE_LIMIT="100/minute" 
+RATE_LIMIT="100/minute"
+CORS_ALLOW_ORIGINS=http://localhost:3000
+CORS_ALLOW_METHODS=*
+CORS_ALLOW_HEADERS=*

README.md

@@ -489,38 +489,38 @@ Legend: `Implemented` means code exists in the repository. `Partial` means code
 | JWT Authentication | Required | Implemented | `AuthenticationMiddleware` validates bearer tokens for non-public routes. |
 | Refresh Token Rotation | Required | Implemented | Refresh flow revokes the old refresh token and persists a new token. |
 | RBAC + Permissions | Required | Implemented | Casbin-backed role and permission checks are wired through route dependencies. |
-| Rate Limiting (Redis-backed) | Required | Partial | Redis-backed limiter exists, but `apply_global_rate_limit` reads `GLOBAL_RATE_LIMIT` while settings expose `RATE_LIMIT`. |
+| Rate Limiting (Redis-backed) | Required | Implemented | Redis-backed limiter reads the configured `RATE_LIMIT` value. |
 | 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. |
-| CORS Configuration | Required | Partial | CORS middleware exists, but production origins, methods, and headers should be environment-driven. |
+| CORS Configuration | Required | Implemented | CORS origins, methods, and headers are environment-driven through settings. |
 | Request ID Middleware | Required | Not Implemented | Add request/correlation ID generation and response header propagation. |
 | Audit Logging | Required | Not Implemented | Add audit events for sensitive auth, user, role, permission, and todo mutations. |
 | Structured Logging | Required | Not Implemented | Add structured application logs with request ID, method, path, status, latency, and user context when available. |
-| Global Exception Handling | Required | Partial | Exception handlers exist, but `Exception` is registered twice; verify domain and fallback handling behavior. |
+| Global Exception Handling | Required | Implemented | Domain exceptions are registered explicitly and `Exception` is used only as the fallback handler. |
 | Input Validation | Required | Implemented | Pydantic schemas and application validation functions are used across user and todo flows. |
 | Password Hashing (Argon2 or bcrypt) | Required | Implemented | User auth service uses bcrypt hashing. |
 | Account Lockout | Required | Not Implemented | Add failed-login tracking and temporary lockout or throttling by account. |
 | Token Revocation | Required | Implemented | Refresh tokens are revoked on rotation/logout, and access tokens are denylisted in Redis until expiry. |
-| 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. |
+| OpenAPI Authentication | Required | Implemented | Swagger OAuth2 auth is configured, and docs/OpenAPI endpoints are disabled when `APP_ENV=production`. |
 | Health Check Endpoint | Required | Implemented | `/health` endpoint returns service health. |
 | Readiness/Liveness Endpoints | Required | Not Implemented | Add separate readiness and liveness endpoints for deployment orchestration. |
 | Request Size Limiting | Required | Implemented | `LimitRequestSizeMiddleware` rejects oversized write requests. |
 | Idempotency Support (for applicable POST endpoints) | Optional but valuable | Not Implemented | Consider idempotency keys for retry-safe create/payment-like workflows. |
 | Database Migrations | Required | Implemented | Alembic is configured with migration commands in the README and Makefile. |
 | Dependency Injection | Required | Implemented | FastAPI dependencies wire repositories, handlers, auth, authorization, and database sessions. |
-| Configuration via Environment Variables | Required | Partial | Pydantic settings read `.env`, but production validation should reject unsafe defaults. |
+| Configuration via Environment Variables | Required | Implemented | Pydantic settings read `.env` and reject the default secret key in production. |
 
 ### Next Implementation Checklist
 
-- [ ] Fix and verify rate limit configuration wiring.
+- [x] Fix and verify rate limit configuration wiring.
 - [ ] Add security headers middleware.
 - [ ] Add request ID middleware.
 - [ ] Add structured request logging.
 - [ ] Add audit logging for sensitive actions.
 - [ ] Add account lockout or equivalent failed-login protection.
-- [ ] Disable or authenticate `/docs`, `/redoc`, and `/openapi.json` in production.
+- [x] Disable or authenticate `/docs`, `/redoc`, and `/openapi.json` in production.
 - [ ] Add readiness and liveness endpoints.
-- [ ] Add production config validation for secrets and unsafe defaults.
-- [ ] Harden CORS through environment-driven allowed origins, methods, and headers.
+- [x] Add production config validation for secrets and unsafe defaults.
+- [x] Harden CORS through environment-driven allowed origins, methods, and headers.
 - [ ] Review exception responses to avoid leaking token parsing details or internal exception messages.
 - [ ] Add automated tests for request size limits, rate limiting, auth failures, authorization failures, CORS, security headers, and request IDs.
 - [ ] Add dependency vulnerability scanning to local or CI checks, for example `pip-audit` or an equivalent Poetry-compatible scanner.

src/core/bootstrap/exception.py

@@ -3,6 +3,7 @@
 from starlette.exceptions import HTTPException as StarletteHTTPException
 
 from src.core.exceptions.handler import (
+    DOMAIN_EXCEPTION_MAP,
     domain_exception_handler,
     global_exception_handler,
     http_exception_handler,
@@ -14,8 +15,7 @@ def register_exception(app: FastAPI):
     app.add_exception_handler(RequestValidationError, validation_exception_handler)
     app.add_exception_handler(StarletteHTTPException, http_exception_handler)
 
-    # Catches custom domain exceptions
-    app.add_exception_handler(Exception, domain_exception_handler)
+    for exception_type in DOMAIN_EXCEPTION_MAP:
+        app.add_exception_handler(exception_type, domain_exception_handler)
 
-    # Fallback (Note: order matters, put specific ones first)
     app.add_exception_handler(Exception, global_exception_handler)

src/core/bootstrap/middleware.py

@@ -15,9 +15,9 @@ def register_middleware(app: FastAPI):
     )
     app.add_middleware(
         CORSMiddleware,
-        allow_origins=["http://localhost:3000"],
+        allow_origins=settings.cors_allow_origins,
         allow_credentials=True,
-        allow_methods=["*"],
-        allow_headers=["*"],
+        allow_methods=settings.cors_allow_methods,
+        allow_headers=settings.cors_allow_headers,
     )
     app.add_middleware(AuthenticationMiddleware)

src/core/config/setting.py

@@ -1,10 +1,13 @@
 from functools import lru_cache
+from typing import ClassVar
 
-from pydantic import Field
+from pydantic import Field, model_validator
 from pydantic_settings import BaseSettings, SettingsConfigDict
 
 
 class Settings(BaseSettings):
+    DEFAULT_SECRET_KEY: ClassVar[str] = "super-secret-key-change-in-production"
+
     APP_NAME: str = Field(alias="APP_NAME", default="Todo Modulith API")
     APP_ENV: str = Field(alias="APP_ENV", default="development")
     DATABASE_URL: str = Field(
@@ -15,9 +18,7 @@ class Settings(BaseSettings):
         alias="REDIS_URL",
         default="redis://:eYVX7EwVmmxKPCDmwMtyKVge8oLd2t81@127.0.0.1:6379/0",
     )
-    SECRET_KEY: str = Field(
-        alias="SECRET_KEY", default="super-secret-key-change-in-production"
-    )
+    SECRET_KEY: str = Field(alias="SECRET_KEY", default=DEFAULT_SECRET_KEY)
     ALGORITHM: str = Field(alias="ALGORITHM", default="HS256")
     JWT_ISSUER: str = Field(alias="JWT_ISSUER", default="todo-modulith-api")
     JWT_AUDIENCE: str = Field(alias="JWT_AUDIENCE", default="todo-modulith-client")
@@ -28,10 +29,42 @@ class Settings(BaseSettings):
         alias="REFRESH_TOKEN_EXPIRE_MINUTES", default=10080
     )
     RATE_LIMIT: str = Field(alias="RATE_LIMIT", default="100/minute")
+    CORS_ALLOW_ORIGINS: str = Field(
+        alias="CORS_ALLOW_ORIGINS",
+        default="http://localhost:3000",
+    )
+    CORS_ALLOW_METHODS: str = Field(alias="CORS_ALLOW_METHODS", default="*")
+    CORS_ALLOW_HEADERS: str = Field(alias="CORS_ALLOW_HEADERS", default="*")
     MAX_REQUEST_SIZE_MB: int = Field(
         alias="MAX_REQUEST_SIZE_MB", default=5 * 1024 * 1024
     )
 
+    @property
+    def is_production(self) -> bool:
+        return self.APP_ENV.lower() == "production"
+
+    @property
+    def cors_allow_origins(self) -> list[str]:
+        return self._split_csv(self.CORS_ALLOW_ORIGINS)
+
+    @property
+    def cors_allow_methods(self) -> list[str]:
+        return self._split_csv(self.CORS_ALLOW_METHODS)
+
+    @property
+    def cors_allow_headers(self) -> list[str]:
+        return self._split_csv(self.CORS_ALLOW_HEADERS)
+
+    @staticmethod
+    def _split_csv(value: str) -> list[str]:
+        return [item.strip() for item in value.split(",") if item.strip()]
+
+    @model_validator(mode="after")
+    def validate_production_security(self):
+        if self.is_production and self.SECRET_KEY == self.DEFAULT_SECRET_KEY:
+            raise ValueError("SECRET_KEY must be changed in production")
+        return self
+
     model_config = SettingsConfigDict(
         env_file=".env",
         env_file_encoding="utf-8",

src/core/dependency/rate_limit.py

@@ -50,7 +50,7 @@ async def apply_global_rate_limit(request: Request):
     if request.url.path in EXEMPT_PATHS:
         return
 
-    limit_str = settings.GLOBAL_RATE_LIMIT
+    limit_str = settings.RATE_LIMIT
     times_str, period = limit_str.split("/")
     times = int(times_str)
     seconds = 60 if "minute" in period else 1

src/main.py

@@ -10,26 +10,35 @@
 
 settings = get_settings()
 
-app = FastAPI(
-    title=settings.APP_NAME,
-    version="1.0.0",
-    lifespan=lifespan.lifespan,
-    swagger_ui_parameters={
-        "persistAuthorization": True,
-        "displayRequestDuration": True,
-        "filter": True,
-        "deepLinking": True,
-        "tryItOutEnabled": True,
-    },
-    dependencies=[Depends(apply_global_rate_limit)],
-)
-
-register_exception(app=app)
-register_middleware(app=app)
-v1_router.register_router(app=app)
-admin_router.register_router(app=app)
-
-
-@app.get("/health", tags=["Health Check"])
-def health_check():
-    return {"status": "healthy"}
+
+def create_app(app_settings=settings) -> FastAPI:
+    app = FastAPI(
+        title=app_settings.APP_NAME,
+        version="1.0.0",
+        lifespan=lifespan.lifespan,
+        docs_url=None if app_settings.is_production else "/docs",
+        redoc_url=None if app_settings.is_production else "/redoc",
+        openapi_url=None if app_settings.is_production else "/openapi.json",
+        swagger_ui_parameters={
+            "persistAuthorization": True,
+            "displayRequestDuration": True,
+            "filter": True,
+            "deepLinking": True,
+            "tryItOutEnabled": True,
+        },
+        dependencies=[Depends(apply_global_rate_limit)],
+    )
+
+    register_exception(app=app)
+    register_middleware(app=app)
+    v1_router.register_router(app=app)
+    admin_router.register_router(app=app)
+
+    @app.get("/health", tags=["Health Check"])
+    def health_check():
+        return {"status": "healthy"}
+
+    return app
+
+
+app = create_app(settings)

tests/core/test_security_todo.py

@@ -0,0 +1,103 @@
+import pytest
+from fastapi import FastAPI, Request
+
+from src.core.bootstrap.exception import register_exception
+from src.core.bootstrap.middleware import register_middleware
+from src.core.config.setting import Settings
+from src.core.dependency import rate_limit as rate_limit_module
+from src.core.dependency.rate_limit import apply_global_rate_limit
+from src.core.exceptions.handler import (
+    DOMAIN_EXCEPTION_MAP,
+    domain_exception_handler,
+    global_exception_handler,
+)
+from src.main import create_app
+
+
+def test_rate_limit_uses_configured_rate_limit_setting(monkeypatch):
+    created_limiters = []
+
+    class FakeLimiter:
+        def __init__(self, times: int, seconds: int):
+            self.times = times
+            self.seconds = seconds
+            created_limiters.append(self)
+
+        async def __call__(self, request):
+            return None
+
+    request = Request(
+        {
+            "type": "http",
+            "method": "GET",
+            "path": "/api/v1/todos/",
+            "headers": [],
+            "query_string": b"",
+            "server": ("testserver", 80),
+            "scheme": "http",
+            "client": ("testclient", 50000),
+        }
+    )
+
+    monkeypatch.setattr(rate_limit_module.settings, "RATE_LIMIT", "42/minute")
+    monkeypatch.setattr(rate_limit_module, "RateLimiter", FakeLimiter)
+
+    import asyncio
+
+    asyncio.run(apply_global_rate_limit(request))
+
+    assert created_limiters[0].times == 42
+    assert created_limiters[0].seconds == 60
+
+
+def test_cors_middleware_uses_environment_driven_settings(monkeypatch):
+    monkeypatch.setattr(
+        "src.core.bootstrap.middleware.settings",
+        Settings(
+            CORS_ALLOW_ORIGINS="https://app.example.com,https://admin.example.com",
+            CORS_ALLOW_METHODS="GET,POST",
+            CORS_ALLOW_HEADERS="Authorization,Content-Type",
+        ),
+    )
+    app = FastAPI()
+
+    register_middleware(app)
+
+    cors = next(
+        middleware
+        for middleware in app.user_middleware
+        if middleware.cls.__name__ == "CORSMiddleware"
+    )
+    assert cors.kwargs["allow_origins"] == [
+        "https://app.example.com",
+        "https://admin.example.com",
+    ]
+    assert cors.kwargs["allow_methods"] == ["GET", "POST"]
+    assert cors.kwargs["allow_headers"] == ["Authorization", "Content-Type"]
+
+
+def test_register_exception_uses_specific_domain_handlers_and_single_fallback():
+    app = FastAPI()
+
+    register_exception(app)
+
+    for exception_type in DOMAIN_EXCEPTION_MAP:
+        assert app.exception_handlers[exception_type] is domain_exception_handler
+    assert app.exception_handlers[Exception] is global_exception_handler
+
+
+def test_create_app_disables_openapi_entrypoints_in_production():
+    app = create_app(Settings(APP_ENV="production", SECRET_KEY="production-secret"))
+
+    assert app.docs_url is None
+    assert app.redoc_url is None
+    assert app.openapi_url is None
+
+
+def test_production_settings_reject_default_secret_key():
+    with pytest.raises(ValueError, match="SECRET_KEY must be changed"):
+        Settings(
+            APP_ENV="production",
+            SECRET_KEY=Settings.DEFAULT_SECRET_KEY,
+            _env_file=None,
+        )