Merge pull request #19 from christianlouis/fix-imap-credentials-logging-vulnerability-5693644702815765698

🔒 [security] Move IMAP credentials to request body

backend/app/api/api_v1/endpoints/imap.py

@@ -1,39 +1,43 @@
 import logging
 from datetime import datetime
-from typing import Any, Dict
+from typing import Any, Dict, Optional
 
 from app.core.security import require_admin_auth
 from app.services.imap_client import IMAPClient
 from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException
+from pydantic import BaseModel
 
 router = APIRouter()
 logger = logging.getLogger(__name__)
 
 
+class IMAPTestRequest(BaseModel):
+    """IMAP connection test request body"""
+
+    server: Optional[str] = None
+    port: int = 993
+    username: Optional[str] = None
+    password: Optional[str] = None
+    ssl: bool = True
+
+
 @router.post("/test-connection")
 async def test_imap_connection(
-    auth: dict = Depends(require_admin_auth),
+    request: IMAPTestRequest,
-    server: str = None,
+    _auth: dict = Depends(require_admin_auth),
-    port: int = 993,
-    username: str = None,
-    password: str = None,
-    ssl: bool = True,
 ) -> Dict[str, Any]:
     """
     Test connection to an IMAP server and gather mailbox statistics
 
     Security: Requires authentication (X-API-Key or Bearer token)
-    Note: Credentials should be passed in request body, not query params
+    Credentials should be passed in request body
     """
-    # Security: Don't accept credentials in query parameters (they get logged)
+    imap_client = IMAPClient(
-    if any([server, username, password]):
+        server=request.server,
-        logger.warning("IMAP credentials passed as query parameters - this is insecure")
+        port=request.port,
-        raise HTTPException(
+        username=request.username,
-            status_code=400,
+        password=request.password,
-            detail="Credentials should be passed in request body, not query parameters",
+    )
-        )
-
-    imap_client = IMAPClient(server=server, port=port, username=username, password=password)
 
     success, message, stats = imap_client.test_connection()
 
@@ -51,7 +55,7 @@ async def test_imap_connection(
 @router.post("/fetch-reports")
 async def fetch_imap_reports(
     background_tasks: BackgroundTasks,
-    auth: dict = Depends(require_admin_auth),
+    _auth: dict = Depends(require_admin_auth),
     days: int = 7,
     delete_emails: bool = False,
 ) -> Dict[str, Any]:
@@ -88,14 +92,14 @@ async def fetch_imap_reports(
             "timestamp": datetime.now().isoformat(),
         }
     except Exception as e:
-        logger.error(f"Error fetching IMAP reports: {str(e)}")
+        logger.error("Error fetching IMAP reports: %s", str(e))
         raise HTTPException(
             status_code=500, detail="Failed to fetch reports. Check server logs for details."
-        )
+        ) from e
 
 
 @router.get("/status")
-async def get_imap_status(auth: dict = Depends(require_admin_auth)) -> Dict[str, Any]:
+async def get_imap_status(_auth: dict = Depends(require_admin_auth)) -> Dict[str, Any]:
     """
     Get the current status of IMAP polling background processes
 

backend/app/api/api_v1/endpoints/setup.py

@@ -1,4 +1,3 @@
-
 from fastapi import APIRouter, HTTPException, status
 from pydantic import BaseModel, EmailStr
 
@@ -38,7 +37,8 @@ class SystemConfigRequest(BaseModel):
 async def get_setup_status():
     """Get the current setup status"""
     return SetupStatusResponse(
-        is_setup_complete=setup_status["is_setup_complete"], app_name=setup_status["app_name"]
+        is_setup_complete=setup_status["is_setup_complete"],
+        app_name=setup_status["app_name"],
     )
 
 

backend/app/models/user.py

@@ -1,8 +1,8 @@
-
-from app.core.database import Base
 from sqlalchemy import Boolean, Column, Integer, String
 from sqlalchemy.orm import relationship
 
+from app.core.database import Base
+
 
 class User(Base):
     """User model"""