diff --git a/.env.demo b/.env.demo index 4869e3ff..073a8425 100644 --- a/.env.demo +++ b/.env.demo @@ -53,6 +53,32 @@ MAX_UPLOAD_SIZE=1073741824 # Always set to 'nosniff' when enabled # SECURITY_HEADER_X_CONTENT_TYPE_OPTIONS_ENABLED=true +# **CORS (Cross-Origin Resource Sharing)** (see SECURITY_AUDIT.md – Infrastructure Security) +# Disabled by default: most deployments rely on a reverse proxy (Traefik, Nginx, etc.) to inject +# CORS headers. Set CORS_ENABLED=true only if DocuElevate is exposed directly without a proxy, +# or if your proxy does not handle CORS. When enabled, only list the exact origins that need access. +# +# Rationale for reverse-proxy-first approach: +# Traefik/Nginx already set Access-Control-Allow-Origin (and related headers) for every response, +# so adding the middleware here would duplicate headers. When this flag is False the application +# trusts the proxy layer to enforce CORS policy; set it to True for standalone / direct-access +# deployments only. +# +# CORS_ENABLED=false +# +# Comma-separated list of allowed origins (use * to allow all - not recommended with credentials) +# CORS_ALLOWED_ORIGINS=https://app.example.com,https://admin.example.com +# +# Allow cookies / Authorization headers in cross-origin requests +# Must be False when CORS_ALLOWED_ORIGINS=* (browser security requirement) +# CORS_ALLOW_CREDENTIALS=false +# +# Allowed HTTP methods (comma-separated) +# CORS_ALLOWED_METHODS=GET,POST,PUT,DELETE,OPTIONS,PATCH +# +# Allowed request headers (use * to allow all) +# CORS_ALLOWED_HEADERS=* + # **Rate Limiting** (see SECURITY_AUDIT.md and docs/API.md) # Protects against DoS attacks and API abuse by limiting request rates per IP/user # Enabled by default - highly recommended for production diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md index a7a4f91d..dee5453d 100644 --- a/SECURITY_AUDIT.md +++ b/SECURITY_AUDIT.md @@ -300,7 +300,11 @@ ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when configured - Can be enabled for direct deployment without reverse proxy - Individual header control and customization - Documented in DeploymentGuide.md and ConfigurationGuide.md -- ⏳ **TODO:** Implement proper CORS configuration (currently not configured) ([#175](https://github.com/christianlouis/DocuElevate/issues/175)) +- ✅ **CORS middleware implemented** - Configurable `CORSMiddleware` with allowed origins, methods, headers, and credentials ([#175](https://github.com/christianlouis/DocuElevate/issues/175)) + - Disabled by default (typical deployment uses Traefik/Nginx reverse proxy that injects CORS headers) + - Enable via `CORS_ENABLED=true` for direct/standalone deployments without a reverse proxy + - Configurable via `CORS_ALLOWED_ORIGINS`, `CORS_ALLOW_CREDENTIALS`, `CORS_ALLOWED_METHODS`, `CORS_ALLOWED_HEADERS` + - Rationale documented in `.env.demo` and `DeploymentGuide.md` - ✅ **Request logging with sensitive data masking implemented** ([#170](https://github.com/christianlouis/DocuElevate/issues/170)) - `AuditLogMiddleware` in `app/middleware/audit_log.py` logs every HTTP request - Logs: method, path, status code, response time, client IP (configurable), username @@ -319,7 +323,7 @@ ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when configured ### Medium Priority 1. ~~**Add security headers**~~ ✅ Implemented - Configurable HSTS, CSP, X-Frame-Options, X-Content-Type-Options middleware -2. **Configure CORS properly** - Currently no CORS middleware configured ([#175](https://github.com/christianlouis/DocuElevate/issues/175)) +2. ~~**Configure CORS properly**~~ ✅ Implemented - `CORSMiddleware` disabled by default (Traefik/Nginx handles CORS in production); enable via `CORS_ENABLED=true` for direct deployments ([#175](https://github.com/christianlouis/DocuElevate/issues/175)) 3. ~~**Implement audit logging**~~ ✅ Implemented - Request/audit logging with sensitive data masking ([#170](https://github.com/christianlouis/DocuElevate/issues/170)) 4. ~~**Add file upload size limits**~~ ✅ Implemented - Configurable limits with 1GB default, optional file splitting 5. **Document security architecture** - Security design decisions diff --git a/app/config.py b/app/config.py index cdeeefad..0def557c 100644 --- a/app/config.py +++ b/app/config.py @@ -300,6 +300,42 @@ class Settings(BaseSettings): description="Stricter rate limit for authentication endpoints to prevent brute force attacks.", ) + # CORS Configuration (see SECURITY_AUDIT.md – Infrastructure Security section) + # Disabled by default since most deployments use a reverse proxy (Traefik, Nginx, etc.) + # that already adds CORS headers. Enable only if deploying without a reverse proxy or if + # the proxy does not handle CORS. See docs/DeploymentGuide.md for rationale. + cors_enabled: bool = Field( + default=False, + description=( + "Enable CORS middleware. Set to False if reverse proxy (Traefik, Nginx) handles CORS headers. " + "When True, CORSMiddleware is added to the application with the settings below." + ), + ) + cors_allowed_origins: Union[List[str], str] = Field( + default_factory=lambda: ["*"], + description=( + "List of allowed CORS origins. Use ['*'] to allow all origins (not recommended with " + "cors_allow_credentials=True). Comma-separated string is also accepted via env var, " + "e.g. CORS_ALLOWED_ORIGINS=https://app.example.com,https://admin.example.com" + ), + ) + cors_allow_credentials: bool = Field( + default=False, + description=( + "Allow credentials (cookies, Authorization headers) in CORS requests. " + "Cannot be True when cors_allowed_origins=['*']. " + "When True, set cors_allowed_origins to specific origins." + ), + ) + cors_allowed_methods: Union[List[str], str] = Field( + default_factory=lambda: ["GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"], + description="Allowed HTTP methods for CORS requests.", + ) + cors_allowed_headers: Union[List[str], str] = Field( + default_factory=lambda: ["*"], + description="Allowed request headers for CORS. Use ['*'] to allow all headers.", + ) + @model_validator(mode="before") @classmethod def strip_outer_quotes(cls, data: Any) -> Any: @@ -331,6 +367,18 @@ class Settings(BaseSettings): return [] return v + @field_validator("cors_allowed_origins", "cors_allowed_methods", "cors_allowed_headers", mode="before") + @classmethod + def parse_comma_separated_list(cls, v: str | list[str]) -> list[str]: + """Parse comma-separated string or list for CORS list settings.""" + if isinstance(v, str): + if "," in v: + return [item.strip() for item in v.split(",") if item.strip()] + elif v.strip(): + return [v.strip()] + return [] + return v + @field_validator("session_secret") @classmethod def validate_session_secret(cls, v: str | None, info: object) -> str | None: diff --git a/app/main.py b/app/main.py index 59d61908..fffde0ca 100644 --- a/app/main.py +++ b/app/main.py @@ -5,6 +5,7 @@ import pathlib from contextlib import asynccontextmanager from fastapi import FastAPI, HTTPException, Request, status +from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse from fastapi.staticfiles import StaticFiles from fastapi.templating import Jinja2Templates @@ -137,6 +138,21 @@ app.add_middleware(AuditLogMiddleware, config=settings) # 3) Session Middleware (for request.session to work) app.add_middleware(SessionMiddleware, secret_key=SESSION_SECRET) +# 3a) CORS Middleware - handles cross-origin requests and preflight (OPTIONS) responses. +# Disabled by default: set CORS_ENABLED=True only when NOT using a reverse proxy +# (Traefik, Nginx) that already injects CORS headers. When enabled, this middleware +# runs after the session layer so preflight requests bypass CSRF/auth checks. +# Allowed origins, methods, headers, and credentials are all configurable via env vars. +# See SECURITY_AUDIT.md – Infrastructure Security section and docs/DeploymentGuide.md. +if settings.cors_enabled: + app.add_middleware( + CORSMiddleware, + allow_origins=settings.cors_allowed_origins, + allow_credentials=settings.cors_allow_credentials, + allow_methods=settings.cors_allowed_methods, + allow_headers=settings.cors_allowed_headers, + ) + # 4) Respect the X-Forwarded-* headers from reverse proxy (Traefik, Nginx) app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*") diff --git a/tests/test_cors.py b/tests/test_cors.py new file mode 100644 index 00000000..edcf7f03 --- /dev/null +++ b/tests/test_cors.py @@ -0,0 +1,232 @@ +#!/usr/bin/env python3 + +""" +Tests for CORS middleware configuration. + +These tests validate that: +- CORS middleware is disabled by default (reverse proxy handles CORS in production) +- When enabled, CORS headers are present on responses +- Configuration settings (allowed origins, methods, headers, credentials) are respected +- Preflight (OPTIONS) requests are handled correctly when CORS is enabled +""" + +from unittest.mock import Mock + +import pytest + + +@pytest.mark.integration +def test_cors_configuration_exists(): + """Test that CORS configuration attributes exist in settings.""" + from app.config import settings + + assert hasattr(settings, "cors_enabled") + assert hasattr(settings, "cors_allowed_origins") + assert hasattr(settings, "cors_allow_credentials") + assert hasattr(settings, "cors_allowed_methods") + assert hasattr(settings, "cors_allowed_headers") + + +@pytest.mark.integration +def test_cors_disabled_by_default(): + """Test that CORS is disabled by default (reverse proxy handles it).""" + from app.config import Settings + + # The default value for cors_enabled should be False + field_info = Settings.model_fields.get("cors_enabled") + assert field_info is not None + assert field_info.default is False, "CORS should be disabled by default" + + +@pytest.mark.integration +def test_cors_configuration_types(): + """Test that CORS configuration settings have correct types.""" + from app.config import settings + + assert isinstance(settings.cors_enabled, bool) + assert isinstance(settings.cors_allowed_origins, list) + assert isinstance(settings.cors_allow_credentials, bool) + assert isinstance(settings.cors_allowed_methods, list) + assert isinstance(settings.cors_allowed_headers, list) + + +@pytest.mark.unit +def test_cors_headers_absent_when_disabled(client): + """Test that CORS headers are NOT present when middleware is disabled.""" + from app.config import settings + + if settings.cors_enabled: + pytest.skip("CORS is enabled in this test environment") + + response = client.get("/api/diagnostic/health", headers={"Origin": "https://example.com"}) + # When CORS middleware is disabled, the application should not add CORS headers + assert "access-control-allow-origin" not in response.headers + + +@pytest.mark.unit +def test_no_cors_preflight_when_disabled(client): + """Test that preflight OPTIONS requests return 405 (or similar) when CORS is disabled.""" + from app.config import settings + + if settings.cors_enabled: + pytest.skip("CORS is enabled in this test environment") + + response = client.options( + "/api/diagnostic/health", + headers={ + "Origin": "https://example.com", + "Access-Control-Request-Method": "GET", + }, + ) + # Without CORSMiddleware, OPTIONS preflight is not handled as CORS + assert "access-control-allow-origin" not in response.headers + + +@pytest.mark.unit +class TestCORSMiddlewareEnabled: + """Tests for CORSMiddleware behavior when enabled.""" + + def _make_app_with_cors(self, allow_origins=None, allow_credentials=False, allow_methods=None, allow_headers=None): + """Create a minimal FastAPI app with CORSMiddleware for isolated testing.""" + from fastapi import FastAPI + from fastapi.middleware.cors import CORSMiddleware + from fastapi.testclient import TestClient + + test_app = FastAPI() + test_app.add_middleware( + CORSMiddleware, + allow_origins=allow_origins or ["https://trusted.example.com"], + allow_credentials=allow_credentials, + allow_methods=allow_methods or ["GET", "POST", "OPTIONS"], + allow_headers=allow_headers or ["*"], + ) + + @test_app.get("/test") + def test_endpoint(): + return {"status": "ok"} + + return TestClient(test_app) + + def test_cors_header_present_for_allowed_origin(self): + """Test that CORS header is present for allowed origin.""" + test_client = self._make_app_with_cors(allow_origins=["https://trusted.example.com"]) + response = test_client.get("/test", headers={"Origin": "https://trusted.example.com"}) + assert response.status_code == 200 + assert "access-control-allow-origin" in response.headers + assert response.headers["access-control-allow-origin"] == "https://trusted.example.com" + + def test_cors_wildcard_origin(self): + """Test that wildcard origin allows any origin.""" + test_client = self._make_app_with_cors(allow_origins=["*"]) + response = test_client.get("/test", headers={"Origin": "https://any-origin.example.com"}) + assert response.status_code == 200 + assert "access-control-allow-origin" in response.headers + + def test_cors_preflight_returns_200(self): + """Test that CORS preflight OPTIONS request returns 200.""" + test_client = self._make_app_with_cors(allow_origins=["https://trusted.example.com"]) + response = test_client.options( + "/test", + headers={ + "Origin": "https://trusted.example.com", + "Access-Control-Request-Method": "GET", + "Access-Control-Request-Headers": "Content-Type", + }, + ) + assert response.status_code == 200 + assert "access-control-allow-origin" in response.headers + + def test_cors_credentials_allowed(self): + """Test that credentials header is present when allow_credentials=True.""" + test_client = self._make_app_with_cors( + allow_origins=["https://trusted.example.com"], + allow_credentials=True, + ) + response = test_client.get("/test", headers={"Origin": "https://trusted.example.com"}) + assert response.status_code == 200 + assert response.headers.get("access-control-allow-credentials") == "true" + + def test_cors_methods_in_preflight(self): + """Test that allowed methods appear in preflight response.""" + test_client = self._make_app_with_cors( + allow_origins=["https://trusted.example.com"], + allow_methods=["GET", "POST"], + ) + response = test_client.options( + "/test", + headers={ + "Origin": "https://trusted.example.com", + "Access-Control-Request-Method": "POST", + }, + ) + assert response.status_code == 200 + + +@pytest.mark.unit +def test_cors_parse_comma_separated_origins(): + """Test that comma-separated CORS origins string is parsed into a list.""" + import os + + # Temporarily set env var to test parsing + original = os.environ.get("CORS_ALLOWED_ORIGINS") + os.environ["CORS_ALLOWED_ORIGINS"] = "https://app.example.com,https://admin.example.com" + try: + from importlib import reload + + import app.config as config_module + + reload(config_module) + test_settings = config_module.Settings( + database_url="sqlite:///:memory:", + redis_url="redis://localhost:6379/0", + openai_api_key="test-key", + azure_ai_key="test-key", + azure_region="test", + azure_endpoint="https://test.cognitiveservices.azure.com/", + gotenberg_url="http://localhost:3000", + workdir="/tmp", + auth_enabled=False, + ) + assert isinstance(test_settings.cors_allowed_origins, list) + assert len(test_settings.cors_allowed_origins) == 2 + origins = set(test_settings.cors_allowed_origins) + assert origins == {"https://app.example.com", "https://admin.example.com"} + finally: + if original is None: + os.environ.pop("CORS_ALLOWED_ORIGINS", None) + else: + os.environ["CORS_ALLOWED_ORIGINS"] = original + + +@pytest.mark.unit +def test_cors_single_origin_string_to_list(): + """Test that a single-origin string is parsed into a list with one item.""" + import os + + original = os.environ.get("CORS_ALLOWED_ORIGINS") + os.environ["CORS_ALLOWED_ORIGINS"] = "https://app.example.com" + try: + from importlib import reload + + import app.config as config_module + + reload(config_module) + test_settings = config_module.Settings( + database_url="sqlite:///:memory:", + redis_url="redis://localhost:6379/0", + openai_api_key="test-key", + azure_ai_key="test-key", + azure_region="test", + azure_endpoint="https://test.cognitiveservices.azure.com/", + gotenberg_url="http://localhost:3000", + workdir="/tmp", + auth_enabled=False, + ) + assert isinstance(test_settings.cors_allowed_origins, list) + assert len(test_settings.cors_allowed_origins) == 1 + assert test_settings.cors_allowed_origins[0] == "https://app.example.com" + finally: + if original is None: + os.environ.pop("CORS_ALLOWED_ORIGINS", None) + else: + os.environ["CORS_ALLOWED_ORIGINS"] = original