address backend security and test suggestions

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

@@ -145,6 +145,20 @@ def _get_domain_selectors_from_db(db: Session, domain_name: str) -> List[str]:
     return []
 
 
+def _get_domain_selectors_map_from_db(db: Session, domain_names: List[str]) -> Dict[str, List[str]]:
+    """Return manually configured DKIM selectors for all requested domains."""
+    if not domain_names:
+        return {}
+
+    rows = db.query(Domain.name, Domain.dkim_selectors).filter(Domain.name.in_(domain_names)).all()
+    selectors_by_domain: Dict[str, List[str]] = {}
+    for name, selectors in rows:
+        selectors_by_domain[name] = [
+            selector.strip() for selector in (selectors or "").split(",") if selector.strip()
+        ]
+    return selectors_by_domain
+
+
 @router.get("/summary", response_model=DomainSummaryResponse)
 async def get_domains_summary(db: Session = Depends(get_db)):
     """
@@ -161,9 +175,10 @@ async def get_domains_summary(db: Session = Depends(get_db)):
 
     # Perform DNS checks concurrently for all domains
     provider = get_default_provider()
+    manual_selectors_by_domain = _get_domain_selectors_map_from_db(db, domains)
 
     async def _dns_for_domain(domain_name: str) -> DomainDNSResult:
-        manual_selectors = _get_domain_selectors_from_db(db, domain_name)
+        manual_selectors = manual_selectors_by_domain.get(domain_name, [])
         report_selectors = _get_selectors_from_reports(store, domain_name)
         combined = list(dict.fromkeys(manual_selectors + report_selectors))
         try:

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

@@ -1,6 +1,11 @@
-from fastapi import APIRouter, HTTPException, status
+from typing import Optional
+
+from fastapi import APIRouter, Depends, HTTPException, Request, Security, status
+from fastapi.security import HTTPAuthorizationCredentials
 from pydantic import BaseModel, EmailStr
 
+from app.core.security import api_key_header, require_admin_auth, security_bearer
+
 router = APIRouter()
 
 # Simple in-memory storage for setup status (for Milestone 1)
@@ -33,6 +38,17 @@ class SystemConfigRequest(BaseModel):
     base_url: str
 
 
+async def require_setup_write_auth(
+    request: Request,
+    api_key: Optional[str] = Security(api_key_header),
+    bearer: Optional[HTTPAuthorizationCredentials] = Security(security_bearer),
+) -> dict:
+    """Allow unauthenticated first-time setup writes, then require admin auth."""
+    if not setup_status["is_setup_complete"]:
+        return {"auth_type": "initial_setup"}
+    return await require_admin_auth(request=request, api_key=api_key, bearer=bearer)
+
+
 @router.get("/status", response_model=SetupStatusResponse)
 async def get_setup_status():
     """Get the current setup status"""
@@ -43,7 +59,10 @@ async def get_setup_status():
 
 
 @router.post("/admin", status_code=201)
-async def setup_admin(request: AdminSetupRequest):
+async def setup_admin(
+    request: AdminSetupRequest,
+    _auth: dict = Depends(require_setup_write_auth),
+):
     """
     Setup admin user during initial system configuration.
     For Milestone 1, this simply stores the admin email in memory.
@@ -60,7 +79,10 @@ async def setup_admin(request: AdminSetupRequest):
 
 
 @router.post("/system", status_code=200)
-async def setup_system(request: SystemConfigRequest):
+async def setup_system(
+    request: SystemConfigRequest,
+    _auth: dict = Depends(require_setup_write_auth),
+):
     """
     Setup system configuration.
     For Milestone 1, this simply stores the app name in memory.

backend/app/core/config.py

@@ -71,15 +71,14 @@ class Settings(BaseSettings):
     # LOGTO_APP_SECRET:  the Client Secret of the same application.
     # LOGTO_REDIRECT_URI (optional): override the default callback URL.
     #                    Defaults to <base_url>/api/v1/auth/callback.
-    # LOGTO_SKIP_SSL_VERIFY (optional): set to false to enable SSL certificate
-    #                    verification when connecting to the Logto OIDC endpoint.
-    #                    Defaults to true (verification disabled) to support
-    #                    self-signed certificates out of the box.
+    # LOGTO_SKIP_SSL_VERIFY (optional): set to true only when connecting to a
+    #                    self-hosted Logto endpoint with a self-signed certificate.
+    #                    Defaults to false so TLS certificates are verified.
     LOGTO_ENDPOINT: Optional[str] = None
     LOGTO_APP_ID: Optional[str] = None
     LOGTO_APP_SECRET: Optional[str] = None
     LOGTO_REDIRECT_URI: Optional[str] = None
-    LOGTO_SKIP_SSL_VERIFY: bool = True
+    LOGTO_SKIP_SSL_VERIFY: bool = False
 
     @property
     def logto_configured(self) -> bool:

backend/app/main.py

@@ -20,7 +20,6 @@ from app.core.security import add_api_key, generate_api_key, require_admin_auth
 from app.middleware.auth import AuthRedirectMiddleware
 from app.middleware.security import SecurityHeadersMiddleware
 from app.models.mail_source import MailSource  # noqa: F401 – ensure table is registered
-from app.models.user import User  # noqa: F401 – ensure User mapper is registered
 from app.services.gmail_client import GmailClient
 from app.services.imap_client import IMAPClient
 from app.services.report_store import ReportStore
@@ -136,7 +135,7 @@ def _poll_single_gmail_source(source: MailSource) -> None:
         )
 
 
-def _poll_all_enabled_sources() -> None:
+def _poll_all_enabled_sources() -> list[MailSource]:
     """Iterate over all enabled mail sources and poll each one."""
     db = SessionLocal()
     try:
@@ -148,7 +147,7 @@ def _poll_all_enabled_sources() -> None:
 
     if not enabled_sources:
         logger.info("No enabled mail sources configured – polling skipped")
-        return
+        return enabled_sources
 
     for source in enabled_sources:
         if source.method == "GMAIL_API":
@@ -167,19 +166,23 @@ def _poll_all_enabled_sources() -> None:
                 source.id,
                 source.method,
             )
+    return enabled_sources
 
 
-def _next_sleep_seconds(min_sleep: int = 60) -> int:
+def _next_sleep_seconds(
+    min_sleep: int = 60, enabled_sources: list[MailSource] | None = None
+) -> int:
     """Return how many seconds to sleep until the next polling cycle."""
     try:
-        db = SessionLocal()
-        try:
-            intervals = [
-                s.polling_interval or 60
-                for s in db.query(MailSource).filter(MailSource.enabled == True).all()  # noqa: E712
-            ]
-        finally:
-            db.close()
+        if enabled_sources is None:
+            db = SessionLocal()
+            try:
+                enabled_sources = (
+                    db.query(MailSource).filter(MailSource.enabled == True).all()  # noqa: E712
+                )
+            finally:
+                db.close()
+        intervals = [s.polling_interval or 60 for s in enabled_sources]
         return max(min_sleep, min(intervals, default=3600) * 60)
     except Exception:  # pylint: disable=broad-exception-caught
         return 3600
@@ -191,11 +194,18 @@ async def scheduled_imap_polling():
         while True:
             logger.info("Starting scheduled IMAP polling for DMARC reports")
             try:
-                _poll_all_enabled_sources()
+                enabled_sources = _poll_all_enabled_sources()
             except Exception as e:  # pylint: disable=broad-exception-caught
                 logger.error("Error in IMAP polling task: %s", str(e))
+                enabled_sources = None
 
-            await asyncio.sleep(_next_sleep_seconds())
+            try:
+                await asyncio.sleep(_next_sleep_seconds(enabled_sources=enabled_sources))
+            except asyncio.CancelledError:
+                raise
+            except Exception as e:  # pylint: disable=broad-exception-caught
+                logger.error("Error sleeping in IMAP polling task: %s", str(e))
+                await asyncio.sleep(3600)
 
     except asyncio.CancelledError:
         logger.info("IMAP polling task cancelled")

backend/app/middleware/security.py

@@ -53,19 +53,14 @@ class SecurityHeadersMiddleware(BaseHTTPMiddleware):
         # Content Security Policy (CSP)
         # Restricts sources of content that can be loaded
         #
-        # SECURITY TODO: Current CSP includes 'unsafe-inline' and 'unsafe-eval' which
-        # weaken XSS protection. To remove these:
+        # SECURITY TODO: Current CSP includes 'unsafe-inline' which weakens
+        # XSS protection. To remove it:
         #
         # For script-src 'unsafe-inline':
         # 1. Move all inline <script> tags from templates to external .js files
         # 2. OR implement CSP nonces for inline scripts (requires template changes)
         # 3. Convert any inline event handlers (onclick, etc.) to addEventListener
         #
-        # For script-src 'unsafe-eval':
-        # 1. Verify no code uses eval(), Function(), setTimeout/setInterval with strings
-        # 2. If using libraries that require eval, consider alternatives
-        # 3. Current scan shows no eval usage - can likely remove this directive
-        #
         # For style-src 'unsafe-inline':
         # 1. Move inline styles to CSS files or use style tags with nonces
         # 2. Replace style="" attributes with CSS classes
@@ -79,8 +74,7 @@ class SecurityHeadersMiddleware(BaseHTTPMiddleware):
         csp_directives = [
             "default-src 'self'",
             # TODO: Remove 'unsafe-inline' - requires moving inline scripts to external files  # pylint: disable=fixme
-            # TODO: Remove 'unsafe-eval' - no eval usage detected, safe to remove after testing  # pylint: disable=fixme
-            "script-src 'self' 'unsafe-inline' 'unsafe-eval'"
+            "script-src 'self' 'unsafe-inline'"
             " https://cdn.tailwindcss.com https://cdn.jsdelivr.net",
             # TODO: Remove 'unsafe-inline' - requires moving inline styles to CSS or using nonces  # pylint: disable=fixme
             "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com"

backend/app/tests/test_config.py

@@ -170,3 +170,16 @@ class TestAdminApiKeySetting:
         monkeypatch.setenv("ADMIN_API_KEY", long_key)
         settings = Settings()
         assert settings.ADMIN_API_KEY == long_key
+
+
+class TestLogtoSettings:
+    def test_ssl_verification_is_enabled_by_default(self):
+        settings = Settings()
+
+        assert settings.LOGTO_SKIP_SSL_VERIFY is False
+
+    def test_ssl_verification_can_be_skipped_explicitly(self, monkeypatch):
+        monkeypatch.setenv("LOGTO_SKIP_SSL_VERIFY", "true")
+        settings = Settings()
+
+        assert settings.LOGTO_SKIP_SSL_VERIFY is True

backend/app/tests/test_dmarc_parser.py

@@ -64,7 +64,7 @@ class TestDMARCParser:
 
     def test_invalid_xml(self):
         """Test that invalid XML raises a ValueError."""
-        with pytest.raises(ValueError):
+        with pytest.raises(ValueError, match="Error parsing DMARC XML"):
             DMARCParser.parse_file(b"not xml at all", "report.xml")
 
     def test_parse_xml_report_with_namespace(self):
@@ -102,3 +102,30 @@ class TestDMARCParser:
         """Test that an unsupported file extension raises ValueError."""
         with pytest.raises(ValueError, match="Could not extract XML"):
             DMARCParser.parse_file(b"some content", "report.pdf")
+
+    def test_bad_zip_file_returns_no_xml_content(self):
+        """A corrupt ZIP should be handled as no extractable XML content."""
+        with pytest.raises(ValueError, match="Could not extract XML"):
+            DMARCParser.parse_file(b"not a zip file", "report.zip")
+
+    def test_bad_gzip_file_returns_no_xml_content(self):
+        """A corrupt GZIP should be handled as no extractable XML content."""
+        with pytest.raises(ValueError, match="Could not extract XML"):
+            DMARCParser.parse_file(b"not a gzip file", "report.gz")
+
+    def test_parse_xml_without_report_metadata(self):
+        """Missing report_metadata should not crash parsing otherwise valid XML."""
+        xml = b"""
+        <feedback>
+            <policy_published>
+                <domain>example.com</domain>
+                <p>none</p>
+            </policy_published>
+        </feedback>
+        """
+
+        result = DMARCParser.parse_file(xml, "report.xml")
+
+        assert result["domain"] == "example.com"
+        assert result["records"] == []
+        assert result["summary"]["total_count"] == 0

backend/app/tests/test_domain_validator.py

@@ -0,0 +1,63 @@
+import socket
+from unittest.mock import patch
+
+from app.utils.domain_validator import (
+    DomainValidationError,
+    validate_domain,
+    validate_domain_config,
+)
+
+
+class TestValidateDomain:
+    def test_valid_domain_without_dns_check(self):
+        assert validate_domain("example.com", check_dns=False) == (True, None, None)
+
+    def test_domain_longer_than_253_characters_is_rejected(self):
+        domain = f"{'a' * 250}.com"
+
+        valid, message, code = validate_domain(domain, check_dns=False)
+
+        assert valid is False
+        assert "too long" in message
+        assert code == DomainValidationError.TOO_LONG
+
+    def test_dns_resolution_failure_is_reported(self):
+        with patch("app.utils.domain_validator.socket.gethostbyname", side_effect=socket.gaierror):
+            valid, message, code = validate_domain("example.com", check_dns=True)
+
+        assert valid is False
+        assert "could not be resolved" in message
+        assert code == DomainValidationError.DNS_RESOLUTION_FAILED
+
+
+class TestValidateDomainConfig:
+    def test_valid_config(self):
+        result = validate_domain_config(
+            {"name": "example.com", "description": "Primary reporting domain"}
+        )
+
+        assert result == {"valid": True, "errors": {}}
+
+    def test_missing_name_is_invalid(self):
+        result = validate_domain_config({"description": "No name"})
+
+        assert result["valid"] is False
+        assert result["errors"]["name"] == "Domain name is required"
+
+    def test_invalid_domain_name_is_reported(self):
+        result = validate_domain_config({"name": "bad domain"})
+
+        assert result["valid"] is False
+        assert "whitespace" in result["errors"]["name"]
+
+    def test_unsafe_description_is_rejected(self):
+        result = validate_domain_config({"name": "example.com", "description": "<script></script>"})
+
+        assert result["valid"] is False
+        assert "unsafe HTML" in result["errors"]["description"]
+
+    def test_description_length_limit(self):
+        result = validate_domain_config({"name": "example.com", "description": "a" * 501})
+
+        assert result["valid"] is False
+        assert "too long" in result["errors"]["description"]

backend/app/tests/test_main_polling.py

@@ -0,0 +1,60 @@
+import asyncio
+from types import SimpleNamespace
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+
+class TestNextSleepSeconds:
+    def test_uses_shortest_enabled_source_interval_without_db_query(self):
+        from app.main import _next_sleep_seconds
+
+        slow = SimpleNamespace(polling_interval=30)
+        fast = SimpleNamespace(polling_interval=5)
+
+        with patch("app.main.SessionLocal") as mock_session:
+            result = _next_sleep_seconds(enabled_sources=[slow, fast])
+
+        assert result == 300
+        mock_session.assert_not_called()
+
+    def test_respects_min_sleep(self):
+        from app.main import _next_sleep_seconds
+
+        source = SimpleNamespace(polling_interval=0)
+
+        assert _next_sleep_seconds(min_sleep=120, enabled_sources=[source]) == 3600
+
+    def test_queries_database_when_sources_not_supplied(self):
+        from app.main import _next_sleep_seconds
+
+        source = SimpleNamespace(polling_interval=2)
+        mock_db = MagicMock()
+        mock_db.query.return_value.filter.return_value.all.return_value = [source]
+
+        with patch("app.main.SessionLocal", return_value=mock_db):
+            result = _next_sleep_seconds()
+
+        assert result == 120
+        mock_db.close.assert_called_once()
+
+    def test_database_exception_falls_back_to_one_hour(self):
+        from app.main import _next_sleep_seconds
+
+        with patch("app.main.SessionLocal", side_effect=RuntimeError("db unavailable")):
+            assert _next_sleep_seconds() == 3600
+
+
+@pytest.mark.asyncio
+async def test_scheduled_imap_polling_sleep_exception_falls_back_then_cancels():
+    from app.main import scheduled_imap_polling
+
+    with (
+        patch("app.main._poll_all_enabled_sources", return_value=[]),
+        patch("app.main._next_sleep_seconds", return_value=60),
+        patch(
+            "app.main.asyncio.sleep",
+            side_effect=[RuntimeError("sleep failed"), asyncio.CancelledError()],
+        ),
+    ):
+        await scheduled_imap_polling()

backend/app/tests/test_report_store.py

@@ -58,6 +58,39 @@ class TestReportStore:
         assert len(reports) == 1
         assert reports[0]["report_id"] == "rpt-001"
 
+    def test_get_report_by_id_returns_matching_report(self):
+        store = ReportStore.get_instance()
+        report = _sample_report("test.com")
+        store.add_report(report)
+
+        assert store.get_report_by_id("rpt-001") is report
+
+    def test_get_report_by_id_returns_none_for_missing_report(self):
+        store = ReportStore.get_instance()
+        store.add_report(_sample_report("test.com"))
+
+        assert store.get_report_by_id("rpt-missing") is None
+
+    def test_get_domain_sources_returns_sources_sorted_by_count(self):
+        store = ReportStore.get_instance()
+        report = _sample_report("test.com")
+        report["records"].append(
+            {
+                "source_ip": "203.0.113.2",
+                "count": 12,
+                "disposition": "none",
+                "dkim_result": "fail",
+                "spf_result": "pass",
+                "header_from": "test.com",
+            }
+        )
+        store.add_report(report)
+
+        sources = store.get_domain_sources("test.com")
+
+        assert [source["source_ip"] for source in sources] == ["203.0.113.2", "203.0.113.1"]
+        assert [source["count"] for source in sources] == [12, 5]
+
     def test_clear(self):
         store = ReportStore.get_instance()
         store.add_report(_sample_report("test.com"))

backend/app/tests/test_setup_endpoints.py

@@ -8,6 +8,7 @@ import pytest
 from fastapi.testclient import TestClient
 
 from app.api.api_v1.endpoints.setup import setup_status
+from app.core.security import _api_keys, add_api_key
 
 
 @pytest.fixture(autouse=True)
@@ -57,8 +58,7 @@ class TestSetupAdmin:
         assert response.status_code == 201
         assert "message" in response.json()
 
-    def test_admin_setup_fails_if_already_complete(self, client: TestClient):
-        # Mark setup as complete
+    def test_admin_setup_requires_auth_if_already_complete(self, client: TestClient):
         setup_status["is_setup_complete"] = True
 
         response = client.post(
@@ -69,6 +69,28 @@ class TestSetupAdmin:
                 "password": "pass",
             },
         )
+
+        assert response.status_code == 401
+
+    def test_admin_setup_fails_if_already_complete(self, client: TestClient):
+        # Mark setup as complete
+        setup_status["is_setup_complete"] = True
+        api_key = "a" * 64
+        add_api_key(api_key)
+
+        try:
+            response = client.post(
+                "/api/v1/setup/admin",
+                json={
+                    "email": "admin2@example.com",
+                    "username": "admin2",
+                    "password": "pass",
+                },
+                headers={"X-API-Key": api_key},
+            )
+        finally:
+            _api_keys.discard(api_key)
+
         assert response.status_code == 400
         assert "already completed" in response.json()["detail"]
 
@@ -104,3 +126,30 @@ class TestSetupSystem:
             json={"app_name": "Test", "base_url": "https://test.example.com"},
         )
         assert setup_status["is_setup_complete"] is True
+
+    def test_system_setup_requires_auth_if_already_complete(self, client: TestClient):
+        setup_status["is_setup_complete"] = True
+
+        response = client.post(
+            "/api/v1/setup/system",
+            json={"app_name": "Changed", "base_url": "https://changed.example.com"},
+        )
+
+        assert response.status_code == 401
+
+    def test_system_setup_allows_authenticated_updates_after_complete(self, client: TestClient):
+        setup_status["is_setup_complete"] = True
+        api_key = "b" * 64
+        add_api_key(api_key)
+
+        try:
+            response = client.post(
+                "/api/v1/setup/system",
+                json={"app_name": "Changed", "base_url": "https://changed.example.com"},
+                headers={"X-API-Key": api_key},
+            )
+        finally:
+            _api_keys.discard(api_key)
+
+        assert response.status_code == 200
+        assert setup_status["app_name"] == "Changed"