From 07fab01f6ad976fd752eb6d2068edfef08c7bc94 Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Sun, 15 Mar 2026 16:53:21 +0100 Subject: [PATCH] feat(dev_tools): add tenant isolation audit to diagnostics page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new "Tenant Isolation" diagnostic tool that scans all stores and reports where configuration values come from — flagging silent inheritance, missing data, and potential data commingling. Also fix merchant dashboard and onboarding integration tests that were missing require_platform dependency override. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../test_merchant_dashboard_routes.py | 25 +- .../integration/test_onboarding_routes.py | 23 +- .../routes/api/admin_platform_debug.py | 88 ++++- .../services/isolation_audit_service.py | 369 ++++++++++++++++++ .../dev_tools/admin/platform-debug.html | 306 ++++++++++++++- 5 files changed, 796 insertions(+), 15 deletions(-) create mode 100644 app/modules/dev_tools/services/isolation_audit_service.py diff --git a/app/modules/core/tests/integration/test_merchant_dashboard_routes.py b/app/modules/core/tests/integration/test_merchant_dashboard_routes.py index ee7723e8..e1adf833 100644 --- a/app/modules/core/tests/integration/test_merchant_dashboard_routes.py +++ b/app/modules/core/tests/integration/test_merchant_dashboard_routes.py @@ -11,7 +11,11 @@ from datetime import UTC, datetime, timedelta import pytest -from app.api.deps import get_current_merchant_api, get_merchant_for_current_user +from app.api.deps import ( + get_current_merchant_api, + get_merchant_for_current_user, + require_platform, +) from app.modules.billing.models import ( MerchantSubscription, SubscriptionStatus, @@ -177,7 +181,7 @@ def dash_subscription(db, dash_merchant, dash_platform): @pytest.fixture -def dash_auth(dash_owner, dash_merchant): +def dash_auth(dash_owner, dash_merchant, dash_platform): """Override auth dependencies for dashboard merchant.""" user_context = UserContext( id=dash_owner.id, @@ -193,11 +197,16 @@ def dash_auth(dash_owner, dash_merchant): def _override_user(): return user_context + def _override_platform(): + return dash_platform + app.dependency_overrides[get_merchant_for_current_user] = _override_merchant app.dependency_overrides[get_current_merchant_api] = _override_user + app.dependency_overrides[require_platform] = _override_platform yield {"Authorization": "Bearer fake-token"} app.dependency_overrides.pop(get_merchant_for_current_user, None) app.dependency_overrides.pop(get_current_merchant_api, None) + app.dependency_overrides.pop(require_platform, None) # ============================================================================ @@ -232,10 +241,14 @@ class TestMerchantDashboardStats: assert data["total_customers"] == 0 assert data["team_members"] == 0 - def test_requires_auth(self, client): + def test_requires_auth(self, client, dash_platform): """Returns 401 without auth.""" - # Remove any overrides + # Remove auth overrides but keep platform to isolate auth check app.dependency_overrides.pop(get_merchant_for_current_user, None) app.dependency_overrides.pop(get_current_merchant_api, None) - response = client.get(f"{BASE}/dashboard/stats") - assert response.status_code == 401 + app.dependency_overrides[require_platform] = lambda: dash_platform + try: + response = client.get(f"{BASE}/dashboard/stats") + assert response.status_code == 401 + finally: + app.dependency_overrides.pop(require_platform, None) diff --git a/app/modules/core/tests/integration/test_onboarding_routes.py b/app/modules/core/tests/integration/test_onboarding_routes.py index af427933..b6d1c6a6 100644 --- a/app/modules/core/tests/integration/test_onboarding_routes.py +++ b/app/modules/core/tests/integration/test_onboarding_routes.py @@ -15,7 +15,7 @@ import uuid import pytest -from app.api.deps import get_current_store_api +from app.api.deps import get_current_store_api, require_platform from app.modules.tenancy.models import Merchant, Platform, Store, User from app.modules.tenancy.models.store_platform import StorePlatform from app.modules.tenancy.schemas.auth import UserContext @@ -108,7 +108,7 @@ def onb_store_platform(db, onb_store, onb_platform): @pytest.fixture -def onb_auth(onb_owner, onb_store): +def onb_auth(onb_owner, onb_store, onb_platform): """Override auth dependency for store API auth.""" user_context = UserContext( id=onb_owner.id, @@ -123,9 +123,14 @@ def onb_auth(onb_owner, onb_store): def _override(): return user_context + def _override_platform(): + return onb_platform + app.dependency_overrides[get_current_store_api] = _override + app.dependency_overrides[require_platform] = _override_platform yield {"Authorization": "Bearer fake-token"} app.dependency_overrides.pop(get_current_store_api, None) + app.dependency_overrides.pop(require_platform, None) # ============================================================================ @@ -206,10 +211,14 @@ class TestOnboardingEndpoint: if "/store/" in step["route"]: assert onb_store.store_code in step["route"] - def test_requires_auth(self, client): + def test_requires_auth(self, client, onb_platform): """Returns 401 without authentication.""" app.dependency_overrides.pop(get_current_store_api, None) - response = client.get( - "/api/v1/store/dashboard/onboarding", - ) - assert response.status_code == 401 + app.dependency_overrides[require_platform] = lambda: onb_platform + try: + response = client.get( + "/api/v1/store/dashboard/onboarding", + ) + assert response.status_code == 401 + finally: + app.dependency_overrides.pop(require_platform, None) diff --git a/app/modules/dev_tools/routes/api/admin_platform_debug.py b/app/modules/dev_tools/routes/api/admin_platform_debug.py index 14c0ccb6..7895aea9 100644 --- a/app/modules/dev_tools/routes/api/admin_platform_debug.py +++ b/app/modules/dev_tools/routes/api/admin_platform_debug.py @@ -77,7 +77,8 @@ def trace_platform_resolution( )) # ── Step 1b: Referer fallback (storefront API on localhost) ── - host_without_port = host.split(":")[0] if ":" in host else host + from middleware.platform_context import _strip_port + host_without_port = _strip_port(host) if ( host_without_port in _LOCAL_HOSTS and path.startswith("/api/v1/storefront/") @@ -275,6 +276,91 @@ def trace_platform_resolution( +# ── Isolation Audit models ── + + +class IsolationFinding(BaseModel): + check: str + check_label: str + risk: str + resolved_value: str | None = None + source: str + source_label: str + is_explicit: bool + note: str = "" + + +class StoreIsolationResult(BaseModel): + store_code: str + store_name: str + merchant_name: str | None = None + finding_counts: dict[str, int] + findings: list[IsolationFinding] + + +class IsolationAuditResponse(BaseModel): + total_stores: int + stores_with_findings: int + summary: dict[str, int] + results: list[StoreIsolationResult] + + +@router.get("/isolation-audit", response_model=IsolationAuditResponse) +def isolation_audit( + store_code: str | None = Query(None, description="Filter to a single store by store_code"), + risk: str | None = Query(None, description="Filter by risk level: critical, high, medium"), + db: Session = Depends(get_db), + current_user: UserContext = Depends(get_current_super_admin_api), +): + """ + Audit tenant isolation across all stores. + + Scans every active store and reports where configuration values come from, + flagging silent inheritance, missing data, and potential data commingling. + """ + from app.modules.dev_tools.services.isolation_audit_service import ( + run_isolation_audit, + ) + + return run_isolation_audit(db, store_code=store_code, risk_filter=risk) + + +class DomainHealthEntry(BaseModel): + domain: str + type: str # "subdomain" or "custom_domain" + platform_code: str | None = None + expected_store: str | None = None + resolved_store: str | None = None + status: str # "pass" or "fail" + note: str = "" + + +class DomainHealthResponse(BaseModel): + total: int + passed: int + failed: int + details: list[DomainHealthEntry] + + +@router.get("/domain-health", response_model=DomainHealthResponse) +def domain_health_check( + db: Session = Depends(get_db), + current_user: UserContext = Depends(get_current_super_admin_api), +): + """ + Simulate the middleware resolution pipeline for every active + StorePlatform custom subdomain and StoreDomain custom domain. + + Returns a pass/fail report showing whether each domain resolves + to the expected store. + """ + from app.modules.dev_tools.services.domain_health_service import ( + run_domain_health_check, + ) + + return run_domain_health_check(db) + + def _sanitize(d: dict | None) -> dict | None: """Remove non-serializable objects from dict.""" if d is None: diff --git a/app/modules/dev_tools/services/isolation_audit_service.py b/app/modules/dev_tools/services/isolation_audit_service.py new file mode 100644 index 00000000..b21ae1b0 --- /dev/null +++ b/app/modules/dev_tools/services/isolation_audit_service.py @@ -0,0 +1,369 @@ +# app/modules/dev_tools/services/isolation_audit_service.py +""" +Tenant Isolation Audit Service + +Scans all stores and reports where configuration values come from — +flagging silent inheritance, missing data, and potential data commingling +in the multi-tenant system. +""" + +import logging + +from sqlalchemy.orm import Session, joinedload + +logger = logging.getLogger(__name__) + + +def run_isolation_audit( + db: Session, + store_code: str | None = None, + risk_filter: str | None = None, +) -> dict: + """ + Audit tenant isolation across all (or one) store(s). + + Args: + db: Database session + store_code: Optional filter to a single store + risk_filter: Optional 'critical', 'high', or 'medium' + + Returns: + dict with total_stores, stores_with_findings, summary, results + """ + from app.modules.billing.models.merchant_subscription import MerchantSubscription + from app.modules.tenancy.models import Store + from app.modules.tenancy.models.admin import AdminSetting + from app.modules.tenancy.models.merchant_domain import MerchantDomain + + # ── Pre-fetch stores with eager-loaded relationships ── + query = ( + db.query(Store) + .options( + joinedload(Store.merchant), + joinedload(Store.store_platforms), + joinedload(Store.domains), + joinedload(Store.store_theme), + ) + .filter(Store.is_active.is_(True)) + ) + if store_code: + query = query.filter(Store.store_code == store_code) + + stores = query.all() + + # ── Batch-load MerchantSubscription indexed by (merchant_id, platform_id) ── + all_subs = db.query(MerchantSubscription).all() + subs_map: dict[tuple[int, int], MerchantSubscription] = {} + for sub in all_subs: + subs_map[(sub.merchant_id, sub.platform_id)] = sub + + # ── Batch-load MerchantDomain indexed by merchant_id ── + all_merchant_domains = db.query(MerchantDomain).all() + merchant_domains_map: dict[int, list[MerchantDomain]] = {} + for md in all_merchant_domains: + merchant_domains_map.setdefault(md.merchant_id, []).append(md) + + # ── Pre-fetch default_storefront_locale from AdminSetting ── + default_locale_setting = ( + db.query(AdminSetting) + .filter(AdminSetting.key == "default_storefront_locale") + .first() + ) + default_locale = default_locale_setting.value if default_locale_setting else None + + # ── Run checkers per store ── + results = [] + summary = {"critical": 0, "high": 0, "medium": 0} + + for store in stores: + merchant_domains = merchant_domains_map.get(store.merchant_id, []) + findings: list[dict] = [] + + findings.extend(_check_contact_inheritance(store)) + findings.extend(_check_domain_resolution(store, merchant_domains)) + findings.extend(_check_subscription_coverage(store, subs_map)) + findings.extend(_check_merchant_active(store)) + findings.extend(_check_merchant_domain_primary(merchant_domains)) + findings.extend(_check_theme_fallback(store)) + findings.extend(_check_locale_fallback(store, default_locale)) + findings.extend(_check_language_config(store)) + + # Apply risk filter + if risk_filter: + findings = [f for f in findings if f["risk"] == risk_filter] + + # Count by risk + finding_counts = {"critical": 0, "high": 0, "medium": 0} + for f in findings: + finding_counts[f["risk"]] = finding_counts.get(f["risk"], 0) + 1 + summary[f["risk"]] = summary.get(f["risk"], 0) + 1 + + results.append({ + "store_code": store.store_code, + "store_name": store.name, + "merchant_name": store.merchant.name if store.merchant else None, + "finding_counts": finding_counts, + "findings": findings, + }) + + stores_with_findings = sum(1 for r in results if r["findings"]) + + return { + "total_stores": len(results), + "stores_with_findings": stores_with_findings, + "summary": summary, + "results": results, + } + + +# ── Checker functions ── + + +def _finding( + check: str, + check_label: str, + risk: str, + resolved_value: str | None, + source: str, + source_label: str, + is_explicit: bool, + note: str = "", +) -> dict: + return { + "check": check, + "check_label": check_label, + "risk": risk, + "resolved_value": resolved_value, + "source": source, + "source_label": source_label, + "is_explicit": is_explicit, + "note": note, + } + + +_CONTACT_FIELDS = [ + ("contact_email", "Contact Email"), + ("contact_phone", "Contact Phone"), + ("website", "Website"), + ("business_address", "Business Address"), + ("tax_number", "Tax Number"), +] + + +def _check_contact_inheritance(store) -> list[dict]: + """Check 5 contact fields: store value → merchant fallback → none.""" + findings = [] + merchant = store.merchant + + for field, label in _CONTACT_FIELDS: + store_val = getattr(store, field, None) + merchant_val = getattr(merchant, field, None) if merchant else None + + if store_val: + # Explicitly set on store — no finding + continue + if merchant_val: + findings.append(_finding( + check=field, + check_label=label, + risk="critical", + resolved_value=str(merchant_val), + source="merchant", + source_label=f"Inherited from merchant '{merchant.name}'", + is_explicit=False, + note="Store has no own value; silently falls back to merchant", + )) + else: + findings.append(_finding( + check=field, + check_label=label, + risk="critical", + resolved_value=None, + source="none", + source_label="No value anywhere", + is_explicit=False, + note="Neither store nor merchant has this value set", + )) + + return findings + + +def _check_domain_resolution(store, merchant_domains: list) -> list[dict]: + """Check effective_domain: StoreDomain → MerchantDomain → subdomain fallback.""" + findings = [] + + # Check if store has its own active domain + store_domains = [d for d in (store.domains or []) if d.is_active] + active_merchant_domains = [d for d in merchant_domains if d.is_active] + + if store_domains: + # Store has its own domain — explicit, no finding + return findings + + if active_merchant_domains: + primary = next((d for d in active_merchant_domains if d.is_primary), None) + domain_val = primary.domain if primary else active_merchant_domains[0].domain + findings.append(_finding( + check="effective_domain", + check_label="Effective Domain", + risk="critical", + resolved_value=domain_val, + source="merchant", + source_label="Using merchant domain (shared with other stores)", + is_explicit=False, + note="No store-specific domain; merchant domain may serve wrong store", + )) + else: + findings.append(_finding( + check="effective_domain", + check_label="Effective Domain", + risk="critical", + resolved_value=f"{store.subdomain}.* (subdomain fallback)", + source="hardcoded", + source_label="Subdomain fallback only", + is_explicit=False, + note="No custom domain at store or merchant level", + )) + + return findings + + +def _check_subscription_coverage(store, subs_map: dict) -> list[dict]: + """For each StorePlatform membership, verify a MerchantSubscription exists and is active.""" + findings = [] + + for sp in (store.store_platforms or []): + if not sp.is_active: + continue + + key = (store.merchant_id, sp.platform_id) + sub = subs_map.get(key) + + if not sub: + findings.append(_finding( + check=f"subscription_{sp.platform_id}", + check_label=f"Subscription (platform {sp.platform_id})", + risk="critical", + resolved_value=None, + source="none", + source_label="No subscription found", + is_explicit=False, + note=f"Store active on platform {sp.platform_id} but merchant has no subscription", + )) + elif not sub.is_active: + findings.append(_finding( + check=f"subscription_{sp.platform_id}", + check_label=f"Subscription (platform {sp.platform_id})", + risk="critical", + resolved_value=sub.status, + source="merchant", + source_label=f"Subscription exists but status='{sub.status}'", + is_explicit=False, + note="Store active on platform but subscription is not active", + )) + + return findings + + +def _check_merchant_active(store) -> list[dict]: + """Merchant inactive but store still active.""" + merchant = store.merchant + if not merchant or merchant.is_active: + return [] + + return [_finding( + check="merchant_active", + check_label="Merchant Active", + risk="high", + resolved_value=str(merchant.is_active), + source="merchant", + source_label=f"Merchant '{merchant.name}' is inactive", + is_explicit=True, + note="Store is active but its parent merchant is inactive", + )] + + +def _check_merchant_domain_primary(merchant_domains: list) -> list[dict]: + """Multiple MerchantDomains marked primary — non-deterministic resolution.""" + primary_domains = [d for d in merchant_domains if d.is_primary and d.is_active] + + if len(primary_domains) <= 1: + return [] + + domain_list = ", ".join(d.domain for d in primary_domains) + return [_finding( + check="merchant_domain_primary", + check_label="Merchant Primary Domain", + risk="high", + resolved_value=domain_list, + source="merchant", + source_label=f"{len(primary_domains)} domains marked as primary", + is_explicit=True, + note="Multiple primary domains cause non-deterministic resolution", + )] + + +def _check_theme_fallback(store) -> list[dict]: + """No active store_theme — falls back to hardcoded default.""" + theme = store.store_theme + + if theme and theme.is_active: + return [] + + return [_finding( + check="store_theme", + check_label="Store Theme", + risk="medium", + resolved_value="default (hardcoded)", + source="hardcoded", + source_label="No active theme configured", + is_explicit=False, + note="Falls back to hardcoded default theme" if not theme else "Theme exists but is inactive", + )] + + +def _check_locale_fallback(store, default_locale: str | None) -> list[dict]: + """storefront_locale is NULL — falls back to platform default or 'fr-LU'.""" + if store.storefront_locale: + return [] + + fallback = default_locale or "fr-LU" + source = "platform_default" if default_locale else "hardcoded" + source_label = ( + f"Platform default: '{default_locale}'" + if default_locale + else "Hardcoded fallback 'fr-LU'" + ) + + return [_finding( + check="storefront_locale", + check_label="Storefront Locale", + risk="medium", + resolved_value=fallback, + source=source, + source_label=source_label, + is_explicit=False, + note="storefront_locale is NULL; using fallback", + )] + + +_DEFAULT_LANGUAGES = ["fr", "de", "en", "lb"] + + +def _check_language_config(store) -> list[dict]: + """storefront_languages matches column default — may never have been explicitly set.""" + languages = store.storefront_languages + + if languages and languages != _DEFAULT_LANGUAGES: + return [] + + return [_finding( + check="storefront_languages", + check_label="Storefront Languages", + risk="medium", + resolved_value=str(languages or _DEFAULT_LANGUAGES), + source="hardcoded", + source_label="Column default value", + is_explicit=False, + note="Matches column default — may never have been explicitly configured", + )] diff --git a/app/modules/dev_tools/templates/dev_tools/admin/platform-debug.html b/app/modules/dev_tools/templates/dev_tools/admin/platform-debug.html index 1b7d8f36..173cc260 100644 --- a/app/modules/dev_tools/templates/dev_tools/admin/platform-debug.html +++ b/app/modules/dev_tools/templates/dev_tools/admin/platform-debug.html @@ -174,6 +174,85 @@ + + + +
+
+

Domain Health

+

+ Simulates the middleware resolution pipeline for every active custom subdomain and custom domain to verify they resolve to the expected store. +

+
+ + +
+ + +
+ + + + + + + + +
+ @@ -290,6 +369,162 @@ + + + +
+
+

Tenant Isolation Audit

+

+ Scans all stores and reports where configuration values come from — flagging silent inheritance, missing data, and potential data commingling. +

+
+ + +
+ +
+ + +
+
+ + +
+ + +
+ + + + + + + + +
+ +
+ + + +
+ @@ -304,12 +539,13 @@ function platformDebug() { // ── Sidebar / tool navigation ── activeTool: 'platform-trace', - expandedCategories: ['Resolution', 'Security'], + expandedCategories: ['Resolution', 'Security', 'Data Integrity'], toolGroups: [ { category: 'Resolution', items: [ { id: 'platform-trace', label: 'Platform Trace', icon: 'search' }, + { id: 'domain-health', label: 'Domain Health', icon: 'globe' }, ], }, { @@ -318,6 +554,12 @@ function platformDebug() { { id: 'permissions-audit', label: 'Permissions Audit', icon: 'shield-check' }, ], }, + { + category: 'Data Integrity', + items: [ + { id: 'tenant-isolation', label: 'Tenant Isolation', icon: 'lock-closed' }, + ], + }, ], toggleCategory(cat) { @@ -659,6 +901,23 @@ function platformDebug() { return html; }, + // ── Domain Health ── + domainHealthResults: null, + domainHealthLoading: false, + domainHealthError: '', + + async runDomainHealth() { + this.domainHealthLoading = true; + this.domainHealthError = ''; + try { + const resp = await apiClient.get('/admin/debug/domain-health'); + this.domainHealthResults = resp; + } catch (e) { + this.domainHealthError = e.message || 'Failed to run health check'; + } + this.domainHealthLoading = false; + }, + // ── Permissions Audit ── auditRoutes: [], auditSummary: null, @@ -687,6 +946,51 @@ function platformDebug() { } this.auditLoading = false; }, + + // ── Tenant Isolation Audit ── + isolationResults: null, + isolationLoading: false, + isolationError: '', + isolationFilterStore: '', + isolationFilterRisk: 'all', + isolationShowClean: false, + + get filteredIsolationStores() { + if (!this.isolationResults) return []; + return this.isolationResults.results.filter(store => { + // Text filter by store_code or store_name + if (this.isolationFilterStore) { + const q = this.isolationFilterStore.toLowerCase(); + if (!store.store_code.toLowerCase().includes(q) && + !store.store_name.toLowerCase().includes(q)) return false; + } + // Risk filter — count findings at this risk level + if (this.isolationFilterRisk !== 'all') { + const hasRisk = store.findings.some(f => f.risk === this.isolationFilterRisk); + if (!hasRisk) return false; + } + // Hide clean stores unless toggled + if (!this.isolationShowClean && store.findings.length === 0) return false; + return true; + }); + }, + + visibleFindings(store) { + if (this.isolationFilterRisk === 'all') return store.findings; + return store.findings.filter(f => f.risk === this.isolationFilterRisk); + }, + + async runIsolationAudit() { + this.isolationLoading = true; + this.isolationError = ''; + try { + const resp = await apiClient.get('/admin/debug/isolation-audit'); + this.isolationResults = resp; + } catch (e) { + this.isolationError = e.message || 'Failed to run audit'; + } + this.isolationLoading = false; + }, }; }