Merge pull request #351 from christianlouis/copilot/configure-cors-for-api-endpoints

feat(api): Add configurable CORS middleware, disabled by default
2026-02-22 20:32:36 +01:00
parent 3522d9a5c0 969b2d3d2f
commit 389beaa1ed
5 changed files with 326 additions and 2 deletions
@@ -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
@@ -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
@@ -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:
@@ -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="*")

@@ -0,0 +1,230 @@
+#!/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
+"""
+
+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