diff --git a/backend/app/api/api_v1/endpoints/domains.py b/backend/app/api/api_v1/endpoints/domains.py index ae9a064..c91d8a3 100644 --- a/backend/app/api/api_v1/endpoints/domains.py +++ b/backend/app/api/api_v1/endpoints/domains.py @@ -20,6 +20,7 @@ from app.services.report_store import ReportStore logger = logging.getLogger(__name__) router = APIRouter() +DOMAIN_SELECTOR_LOOKUP_CHUNK_SIZE = 500 class DomainBase(BaseModel): @@ -145,17 +146,26 @@ 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]]: +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() + unique_names = list(dict.fromkeys(domain_names)) 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() - ] + for index in range(0, len(unique_names), DOMAIN_SELECTOR_LOOKUP_CHUNK_SIZE): + chunk = unique_names[index : index + DOMAIN_SELECTOR_LOOKUP_CHUNK_SIZE] + rows = ( + db.query(Domain.name, Domain.dkim_selectors) + .filter(Domain.name.in_(chunk)) + .all() + ) + for name, selectors in rows: + selectors_by_domain[name] = [ + selector.strip() for selector in (selectors or "").split(",") if selector.strip() + ] return selectors_by_domain diff --git a/backend/app/api/api_v1/endpoints/setup.py b/backend/app/api/api_v1/endpoints/setup.py index 9ebc88d..11b3bf1 100644 --- a/backend/app/api/api_v1/endpoints/setup.py +++ b/backend/app/api/api_v1/endpoints/setup.py @@ -3,8 +3,11 @@ from typing import Optional from fastapi import APIRouter, Depends, HTTPException, Request, Security, status from fastapi.security import HTTPAuthorizationCredentials from pydantic import BaseModel, EmailStr +from sqlalchemy.orm import Session +from app.core.database import get_db from app.core.security import api_key_header, require_admin_auth, security_bearer +from app.models.setting import Setting router = APIRouter() @@ -15,6 +18,71 @@ setup_status = { "app_name": "DMARQ", } +SETUP_COMPLETE_KEY = "setup.is_complete" +SETUP_ADMIN_EMAIL_KEY = "setup.admin_email" +GENERAL_APP_NAME_KEY = "general.app_name" +GENERAL_BASE_URL_KEY = "general.base_url" + + +def _setting_value(db: Session, key: str) -> Optional[str]: + row = db.query(Setting).filter(Setting.key == key).first() + return row.value if row else None + + +def _is_true(value: Optional[str]) -> bool: + return str(value).strip().lower() in {"1", "true", "yes", "on"} + + +def _upsert_setting( + db: Session, + key: str, + value: Optional[str], + *, + description: str, + value_type: str = "string", + category: str = "setup", +) -> None: + row = db.query(Setting).filter(Setting.key == key).first() + if row is None: + db.add( + Setting( + key=key, + value=value, + description=description, + value_type=value_type, + category=category, + ) + ) + return + + row.value = value + row.description = row.description or description + row.value_type = row.value_type or value_type + row.category = row.category or category + + +def _refresh_setup_status_from_db(db: Session) -> dict: + """Merge persisted setup state into the legacy in-memory setup status.""" + persisted_complete = _is_true(_setting_value(db, SETUP_COMPLETE_KEY)) + if persisted_complete: + setup_status["is_setup_complete"] = True + + persisted_admin_email = _setting_value(db, SETUP_ADMIN_EMAIL_KEY) + if persisted_admin_email: + setup_status["admin_email"] = persisted_admin_email + + persisted_app_name = _setting_value(db, GENERAL_APP_NAME_KEY) + if persisted_app_name: + setup_status["app_name"] = persisted_app_name + + return setup_status + + +def _setup_is_complete(db: Session) -> bool: + return bool(setup_status["is_setup_complete"]) or _is_true( + _setting_value(db, SETUP_COMPLETE_KEY) + ) + class SetupStatusResponse(BaseModel): """Setup status response""" @@ -40,40 +108,50 @@ class SystemConfigRequest(BaseModel): async def require_setup_write_auth( request: Request, + db: Session = Depends(get_db), 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"]: + if not _setup_is_complete(db): 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(): +async def get_setup_status(db: Session = Depends(get_db)): """Get the current setup status""" + current_status = _refresh_setup_status_from_db(db) return SetupStatusResponse( - is_setup_complete=setup_status["is_setup_complete"], - app_name=setup_status["app_name"], + is_setup_complete=current_status["is_setup_complete"], + app_name=current_status["app_name"], ) @router.post("/admin", status_code=201) async def setup_admin( request: AdminSetupRequest, + db: Session = Depends(get_db), _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. """ - if setup_status["is_setup_complete"]: + if _setup_is_complete(db): raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Setup already completed" ) # Store admin email setup_status["admin_email"] = request.email + _upsert_setting( + db, + SETUP_ADMIN_EMAIL_KEY, + request.email, + description="Email address configured during initial setup", + ) + db.commit() return {"message": "Admin user setup completed"} @@ -81,6 +159,7 @@ async def setup_admin( @router.post("/system", status_code=200) async def setup_system( request: SystemConfigRequest, + db: Session = Depends(get_db), _auth: dict = Depends(require_setup_write_auth), ): """ @@ -90,5 +169,27 @@ async def setup_system( # Store app name setup_status["app_name"] = request.app_name setup_status["is_setup_complete"] = True + _upsert_setting( + db, + GENERAL_APP_NAME_KEY, + request.app_name, + description="Application display name shown in the UI", + category="general", + ) + _upsert_setting( + db, + GENERAL_BASE_URL_KEY, + request.base_url, + description="Public base URL for this DMARQ instance", + category="general", + ) + _upsert_setting( + db, + SETUP_COMPLETE_KEY, + "true", + description="Whether initial setup has been completed", + value_type="boolean", + ) + db.commit() return {"message": "System settings saved successfully"} diff --git a/backend/app/tests/test_config.py b/backend/app/tests/test_config.py index cd962b4..41fbf56 100644 --- a/backend/app/tests/test_config.py +++ b/backend/app/tests/test_config.py @@ -173,7 +173,8 @@ class TestAdminApiKeySetting: class TestLogtoSettings: - def test_ssl_verification_is_enabled_by_default(self): + def test_ssl_verification_is_enabled_by_default(self, monkeypatch): + monkeypatch.delenv("LOGTO_SKIP_SSL_VERIFY", raising=False) settings = Settings() assert settings.LOGTO_SKIP_SSL_VERIFY is False diff --git a/backend/app/tests/test_dns_endpoints.py b/backend/app/tests/test_dns_endpoints.py index e2b5872..aa887d2 100644 --- a/backend/app/tests/test_dns_endpoints.py +++ b/backend/app/tests/test_dns_endpoints.py @@ -13,6 +13,8 @@ from unittest.mock import AsyncMock, patch import pytest from fastapi.testclient import TestClient +from app.api.api_v1.endpoints import domains as domains_endpoint +from app.models.domain import Domain from app.services.dns_resolver import DomainDNSResult from app.services.report_store import ReportStore @@ -290,6 +292,50 @@ def test_summary_dns_failure_defaults_false(client: TestClient): assert domain["dkim_status"] is False +def test_summary_endpoint_uses_manual_selectors(client: TestClient): + """Manually configured selectors are forwarded by the summary endpoint.""" + client.post(f"/api/v1/domains/{DOMAIN}/selectors", json={"selector": "manualsel"}) + captured_selectors = [] + + async def _fake_check_domain(domain, selectors=None): + captured_selectors.extend(selectors or []) + return MOCK_DNS_RESULT + + with patch( + "app.api.api_v1.endpoints.domains.get_default_provider", + return_value=AsyncMock(check_domain=_fake_check_domain), + ): + response = client.get("/api/v1/domains/summary") + + assert response.status_code == 200 + assert "manualsel" in captured_selectors + assert "google" in captured_selectors + + +def test_selector_map_lookup_chunks_domain_names(db_session, monkeypatch): + """Large summary batches are split to avoid database parameter limits.""" + monkeypatch.setattr(domains_endpoint, "DOMAIN_SELECTOR_LOOKUP_CHUNK_SIZE", 2) + db_session.add_all( + [ + Domain(name="one.example", dkim_selectors="a,b"), + Domain(name="two.example", dkim_selectors="c"), + Domain(name="three.example", dkim_selectors="d"), + ] + ) + db_session.commit() + + selectors = domains_endpoint._get_domain_selectors_map_from_db( + db_session, + ["one.example", "two.example", "three.example", "one.example"], + ) + + assert selectors == { + "one.example": ["a", "b"], + "two.example": ["c"], + "three.example": ["d"], + } + + # --------------------------------------------------------------------------- # GET /api/v1/domains/{domain_id}/sources (PTR + fix hints) # --------------------------------------------------------------------------- diff --git a/backend/app/tests/test_main_polling.py b/backend/app/tests/test_main_polling.py index 04d1707..9ed7a04 100644 --- a/backend/app/tests/test_main_polling.py +++ b/backend/app/tests/test_main_polling.py @@ -21,9 +21,9 @@ class TestNextSleepSeconds: def test_respects_min_sleep(self): from app.main import _next_sleep_seconds - source = SimpleNamespace(polling_interval=0) + source = SimpleNamespace(polling_interval=1) - assert _next_sleep_seconds(min_sleep=120, enabled_sources=[source]) == 3600 + assert _next_sleep_seconds(min_sleep=120, enabled_sources=[source]) == 120 def test_queries_database_when_sources_not_supplied(self): from app.main import _next_sleep_seconds diff --git a/backend/app/tests/test_setup_endpoints.py b/backend/app/tests/test_setup_endpoints.py index 93c43a6..ae9740e 100644 --- a/backend/app/tests/test_setup_endpoints.py +++ b/backend/app/tests/test_setup_endpoints.py @@ -9,6 +9,7 @@ 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 +from app.models.setting import Setting @pytest.fixture(autouse=True) @@ -42,6 +43,38 @@ class TestSetupStatus: assert data["is_setup_complete"] is True assert data["app_name"] == "MyApp" + def test_status_reads_persisted_setup_state_after_memory_reset( + self, client: TestClient, db_session + ): + db_session.add( + Setting( + key="setup.is_complete", + value="true", + description="Whether initial setup has been completed", + value_type="boolean", + category="setup", + ) + ) + db_session.add( + Setting( + key="general.app_name", + value="Persisted DMARQ", + description="Application display name shown in the UI", + value_type="string", + category="general", + ) + ) + db_session.commit() + setup_status["is_setup_complete"] = False + setup_status["app_name"] = "DMARQ" + + response = client.get("/api/v1/setup/status") + + assert response.status_code == 200 + data = response.json() + assert data["is_setup_complete"] is True + assert data["app_name"] == "Persisted DMARQ" + class TestSetupAdmin: """Tests for POST /api/v1/setup/admin""" @@ -137,6 +170,23 @@ class TestSetupSystem: assert response.status_code == 401 + def test_system_setup_requires_auth_if_persisted_complete_after_restart( + self, client: TestClient + ): + first_response = client.post( + "/api/v1/setup/system", + json={"app_name": "DMARQ", "base_url": "https://example.com"}, + ) + assert first_response.status_code == 200 + setup_status["is_setup_complete"] = False + + 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 diff --git a/docs/AUDIT_SUMMARY.md b/docs/AUDIT_SUMMARY.md index 543acea..38a5bf8 100644 --- a/docs/AUDIT_SUMMARY.md +++ b/docs/AUDIT_SUMMARY.md @@ -135,7 +135,7 @@ DMARQ has **excellent Python code quality** and **strong security infrastructure - Some API tests failing 2. **CSP TODOs** - - 3 documented TODOs to remove unsafe-inline/unsafe-eval + - Documented TODOs to remove remaining unsafe-inline allowances - Currently weakens security 3. **Accessibility Gaps** diff --git a/docs/CODE_QUALITY_AUDIT_2026-02.md b/docs/CODE_QUALITY_AUDIT_2026-02.md index 47fa8cb..41b00d0 100644 --- a/docs/CODE_QUALITY_AUDIT_2026-02.md +++ b/docs/CODE_QUALITY_AUDIT_2026-02.md @@ -94,7 +94,7 @@ This comprehensive audit evaluated the DMARQ codebase across multiple dimensions #### 📝 Recommendations 1. **Priority: Low** - Consider extracting helper functions from complex methods if readability suffers -2. **Priority: Medium** - Address CSP TODOs (remove unsafe-inline/unsafe-eval) +2. **Priority: Medium** - Address CSP TODOs (remove remaining unsafe-inline allowances) 3. **Priority: Low** - Migrate from Pydantic v1 validators to v2 field_validator --- @@ -272,14 +272,15 @@ document.getElementById('openModalBtn').addEventListener('click', () => { - Documented with clear warnings in code 2. **CSP Unsafe Directives** - - Uses 'unsafe-inline' and 'unsafe-eval' + - Uses 'unsafe-inline' + - 'unsafe-eval' has been removed from the current policy - Tracked with TODOs in code - Documented in SECURITY.md #### 📝 Recommendations **Priority: MEDIUM** -1. Remove CSP unsafe-inline/unsafe-eval directives +1. Remove remaining CSP unsafe-inline directives 2. Implement nonce-based CSP for scripts/styles 3. Move API keys to database/Redis for production @@ -425,7 +426,7 @@ document.getElementById('openModalBtn').addEventListener('click', () => { ## Summary of TODOs Found in Codebase 1. **middleware/security.py (3 instances):** - - Line 55: Remove 'unsafe-inline' and 'unsafe-eval' and use nonces/hashes instead + - Line 55: Remove remaining 'unsafe-inline' allowances and use nonces/hashes instead - Line 61: Use nonces for script-src - Line 62: Use nonces for style-src diff --git a/docs/FOLLOW_UP_SUMMARY.md b/docs/FOLLOW_UP_SUMMARY.md index d3b8a07..190526e 100644 --- a/docs/FOLLOW_UP_SUMMARY.md +++ b/docs/FOLLOW_UP_SUMMARY.md @@ -45,11 +45,11 @@ Content Security Policy hardening has been documented with detailed plans: - Added CDN sources to CSP whitelist 2. **Analysis** - ✅ COMPLETE - - Verified no eval() usage (unsafe-eval can be removed) + - Verified no eval() usage and removed `unsafe-eval` from `script-src` - Identified all inline script locations - Documented inline style usage -3. **Implementation** - ⚠️ FUTURE WORK +3. **Remaining Implementation** - ⚠️ FUTURE WORK - Requires moving inline scripts to external files - Or implementing CSP nonces (more complex) - Priority: HIGH @@ -58,7 +58,8 @@ Content Security Policy hardening has been documented with detailed plans: **Current CSP Status**: - ✅ Documented comprehensive plan - ✅ Added TODO comments with specific steps -- ⚠️ Still includes unsafe-inline/unsafe-eval +- ✅ Removed `unsafe-eval` from `script-src` +- ⚠️ Still includes `unsafe-inline` - ⚠️ Requires template refactoring to fix ### 📊 MEDIUM - Test Suite Remediation (ANALYZED, PARTIAL) @@ -143,7 +144,7 @@ Comprehensive audit process has been documented: 2. ✅ Audit process established for ongoing monitoring ### Remaining Risks -1. ⚠️ CSP still allows unsafe-inline/unsafe-eval (documented, planned) +1. ⚠️ CSP still allows `unsafe-inline` (documented, planned) 2. ⚠️ Some test failures indicate potential integration issues (non-security) ## Metrics @@ -177,7 +178,8 @@ Comprehensive audit process has been documented: ### Short-term (Next Sprint) - [ ] Move inline scripts to external files -- [ ] Remove 'unsafe-eval' from CSP +- [x] Remove 'unsafe-eval' from CSP +- [ ] Remove 'unsafe-inline' from CSP - [ ] Test with stricter CSP - [ ] Fix test suite database schema issues - [ ] Resolve failing API tests diff --git a/docs/XSS_FIXES.md b/docs/XSS_FIXES.md index b63b819..64c26df 100644 --- a/docs/XSS_FIXES.md +++ b/docs/XSS_FIXES.md @@ -239,7 +239,7 @@ After fixing the XSS issues, update your CSP header in `backend/app/middleware/s ### Current (Insecure) ```python -"script-src 'self' 'unsafe-inline' 'unsafe-eval'", +"script-src 'self' 'unsafe-inline' https://cdn.tailwindcss.com https://cdn.jsdelivr.net", "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com", ``` @@ -255,6 +255,7 @@ After fixing the XSS issues, update your CSP header in `backend/app/middleware/s - [ ] All user input uses `textContent` not `innerHTML` - [ ] No credentials stored in localStorage - [ ] Inline styles replaced with CSS classes +- [x] CSP headers updated to remove 'unsafe-eval' - [ ] CSP headers updated to remove 'unsafe-inline' - [ ] Manual XSS testing completed - [ ] Automated tests added diff --git a/docs/XSS_FIXES_VERIFICATION.md b/docs/XSS_FIXES_VERIFICATION.md index 2c67284..be60f86 100644 --- a/docs/XSS_FIXES_VERIFICATION.md +++ b/docs/XSS_FIXES_VERIFICATION.md @@ -195,7 +195,8 @@ No review comments found. ## Remaining Work ### CSP Hardening (Future Work) -The Content Security Policy still includes `unsafe-inline` and `unsafe-eval` directives. To remove these: +The Content Security Policy no longer includes `unsafe-eval`, but it still +includes `unsafe-inline`. To remove the remaining unsafe inline allowances: 1. **For script-src 'unsafe-inline'**: - Move inline `