Fix tenant isolation and monthly revenue accuracy

FINDINGS.md

@@ -0,0 +1,51 @@
+# Revenue dashboard debugging findings
+
+## Bugs and fixes
+
+- **Cross-client cache leakage:** the original Redis key included only property ID, but both clients own `prop-001`. Cache keys now include tenant, property, year, and month, use unambiguous JSON encoding, and carry a new version to avoid old results. Tenant identity comes from authentication; simulation headers cannot select another tenant. Unknown identities no longer default to tenant A, and the dashboard rejects missing tenant membership.
+- **Incorrect monthly boundaries:** the original monthly function was a placeholder and the summary aggregated all dates. Existing local edits added March filtering, but fixed the period to March 2024. Requests now accept a validated month/year. PostgreSQL converts each local month boundary using the property's IANA timezone. The interval includes the start and excludes the next month's start, including across daylight-saving changes.
+- **Broken database access hidden by plausible totals:** the pool referenced missing Supabase settings, used a synchronous pool with an async engine, and returned a coroutine where callers expected a session context manager. It now uses DATABASE_URL, SQLAlchemy's async pool, and a reusable shared pool. Failed revenue queries return HTTP 503 rather than fabricated or zero totals. Redis failures fall back to the real database.
+- **Financial precision:** SQL NUMERIC aggregation stays decimal through the API and UI. No conversion to Python float or JavaScript Number and no per-reservation cent rounding. The UI preserves all three source decimal places. Different currencies are reported separately rather than incorrectly added as USD; empty months do not invent a currency.
+- **Misleading dashboard:** the original static selector exposed other clients' property names and assigned the wrong name to Ocean's shared ID. It now loads the authenticated tenant's properties. The invented 12% growth badge is removed. Reporting month is explicit, and obsolete asynchronous responses are ignored when selection changes.
+
+The four files already edited before this debugging session were reviewed and built upon, including the tenant cache key and decimal-string changes.
+
+## Verified seeded March 2024 results
+
+| Client | Property | Exact total | Reservations |
+| --- | --- | --- | --- |
+| Sunset | prop-001 | USD 2250.000 | 4 |
+| Sunset | prop-002 | USD 4975.500 | 4 |
+| Sunset | prop-003 | USD 6100.500 | 2 |
+| Ocean | prop-001 | No reservations | 0 |
+| Ocean | prop-004 | USD 1776.500 | 4 |
+| Ocean | prop-005 | USD 3256.000 | 3 |
+
+Ocean's prop-001 deliberately has no reservations in the supplied seed. Do not add invented bookings to make it show a nonzero result.
+
+The Sunset boundary reservation checks in at 2024-02-29 23:30 UTC, which is March 1 at 00:30 in Paris. Its USD 1250 belongs in March. The three fractional amounts 333.333 + 333.333 + 333.334 sum exactly to 1000.000.
+
+## Validation
+
+Run from the repository root with Docker services running:
+
+```sh
+docker compose exec -T backend python -m unittest discover -s tests -v
+docker compose build frontend
+docker compose up -d --no-deps frontend
+```
+
+The tests use actual PostgreSQL queries with temporary fixture changes inside rolled-back transactions. They cover Paris/New York month edges and DST, December rollover, empty months, fractional amounts, mixed currencies, foreign-property rejection, tenant/month cache isolation, cache outages, database failure responses, missing tenant identity, real login credentials, warm API responses, and invalid month input.
+
+Browser checks completed: Sunset login, March 2250.000/four bookings, February empty, logout, Ocean login, Ocean-only property names, Lakeside Cottage 1776.500/four bookings. Frontend production build passed in Docker. The host npm command could not run because the checked-in node_modules lacks the Vite executable; Docker supplies a working build environment.
+
+## Loom outline (about 6 minutes)
+
+1. **0:00-0:45 — Explain the reports.** Open ASSIGNMENT.md. Describe the three symptoms: incorrect months, cross-client results, and rounding.
+2. **0:45-2:00 — Show isolation.** Open cache.py and the tenant checks in dashboard.py. Explain why property ID alone is unsafe. Log in as Sunset and Ocean in turn; show the different property lists and prop-001 results.
+3. **2:00-3:15 — Explain the calendar bug.** Open reservations.py and the boundary seed row. Explain Paris midnight and the half-open interval. Show March versus February in the dashboard.
+4. **3:15-4:15 — Explain precision.** Show the three fractional seed values and the exact decimal-string response. Explain that currencies remain separate and no rounding policy was silently invented.
+5. **4:15-5:15 — Explain the hidden failure.** Open database_pool.py. Describe missing settings, the incorrect async session API, and why fake totals or zero on errors are dangerous. Show the HTTP 503 handling.
+6. **5:15-6:00 — Demonstrate regression checks.** Run the unittest command above, show the passing result, and briefly describe DST boundaries, isolation, and outage checks.
+
+The Loom recording itself has not been created. Record the walkthrough with your own narration before submitting. This is a focused assignment fix, not a production security audit of the repository's mock authentication system.

backend/app/api/v1/dashboard.py

@@ -1,25 +1,50 @@
-from fastapi import APIRouter, Depends, HTTPException
-from typing import Dict, Any
+import logging
+from typing import Any, Dict
+
+from fastapi import APIRouter, Depends, HTTPException, Query, Response
 from app.services.cache import get_revenue_summary
 from app.core.auth import authenticate_request as get_current_user
+from app.models.auth import AuthenticatedUser
 
 router = APIRouter()
+logger = logging.getLogger(__name__)
+
+
+@router.get("/dashboard/properties")
+async def get_dashboard_properties(current_user: AuthenticatedUser = Depends(get_current_user)):
+    from app.core.database_pool import db_pool
+    from sqlalchemy import text
+    if not current_user.tenant_id:
+        raise HTTPException(status_code=403, detail="Tenant membership required")
+    await db_pool.initialize()
+    async with db_pool.get_session() as session:
+        result = await session.execute(text(
+            "SELECT id, name FROM properties WHERE tenant_id = :tenant_id ORDER BY id"
+        ), {"tenant_id": current_user.tenant_id})
+        return [dict(row._mapping) for row in result.fetchall()]
+
 
 @router.get("/dashboard/summary")
 async def get_dashboard_summary(
+    response: Response,
     property_id: str,
-    current_user: dict = Depends(get_current_user)
+    month: int = Query(3, ge=1, le=12),
+    year: int = Query(2024, ge=1, le=9998),
+    current_user: AuthenticatedUser = Depends(get_current_user),
 ) -> Dict[str, Any]:
-
-    tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant"
-
-    revenue_data = await get_revenue_summary(property_id, tenant_id)
-
-    total_revenue_float = float(revenue_data['total'])
-
+    response.headers["Cache-Control"] = "private, no-store"
+    tenant_id = current_user.tenant_id
+    if not tenant_id:
+        raise HTTPException(status_code=403, detail="Tenant membership required")
+    try:
+        data = await get_revenue_summary(property_id, tenant_id, month, year)
+    except HTTPException:
+        raise
+    except Exception:
+        logger.exception("Revenue calculation failed")
+        raise HTTPException(status_code=503, detail="Revenue data temporarily unavailable")
     return {
-        "property_id": revenue_data['property_id'],
-        "total_revenue": total_revenue_float,
-        "currency": revenue_data['currency'],
-        "reservations_count": revenue_data['count']
+        "property_id": data["property_id"], "month": month, "year": year,
+        "total_revenue": data["total"], "currency": data["currency"],
+        "reservations_count": data["count"], "totals": data["totals"],
     }

backend/app/core/database_pool.py

@@ -1,6 +1,5 @@
-import asyncio
 from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker
-from sqlalchemy.pool import QueuePool
+from sqlalchemy.engine import make_url
 import logging
 from ..config import settings
 
@@ -13,13 +12,14 @@ def __init__(self):
 
     async def initialize(self):
         """Initialize database connection pool"""
+        if self.session_factory is not None:
+            return
         try:
             # Create async engine with connection pooling
-            database_url = f"postgresql+asyncpg://{settings.supabase_db_user}:{settings.supabase_db_password}@{settings.supabase_db_host}:{settings.supabase_db_port}/{settings.supabase_db_name}"
+            database_url = make_url(settings.database_url).set(drivername="postgresql+asyncpg")
 
             self.engine = create_async_engine(
                 database_url,
-                poolclass=QueuePool,
                 pool_size=20,  # Number of connections to maintain
                 max_overflow=30,  # Additional connections when needed
                 pool_pre_ping=True,  # Validate connections
@@ -44,8 +44,10 @@ async def close(self):
         """Close database connections"""
         if self.engine:
             await self.engine.dispose()
+        self.engine = None
+        self.session_factory = None
 
-    async def get_session(self) -> AsyncSession:
+    def get_session(self) -> AsyncSession:
         """Get database session from pool"""
         if not self.session_factory:
             raise Exception("Database pool not initialized")

backend/app/core/tenant_resolver.py

@@ -69,7 +69,7 @@ def resolve_tenant_from_user(user_data: dict) -> Optional[str]:
         return None
 
     @staticmethod
-    async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] = None) -> str:
+    async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] = None) -> Optional[str]:
         """
         Resolve tenant ID for a user.
 
@@ -88,8 +88,8 @@ async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str]
         if user_email == "candidate@propertyflow.com":
             return "tenant-a"
 
-        # Default fallback
-        return "tenant-a"
+        # Unknown identities must never inherit another client's tenant.
+        return None
 
     @staticmethod
     async def update_user_tenant_metadata(user_id: str, tenant_id: str) -> None:

backend/app/services/cache.py

@@ -1,29 +1,31 @@
 import json
-import redis.asyncio as redis
-from typing import Dict, Any
+import logging
 import os
+from typing import Any, Dict
+
+import redis.asyncio as redis
+from redis.exceptions import RedisError
 
-# Initialize Redis client (typically configured centrally).
+logger = logging.getLogger(__name__)
 redis_client = redis.Redis.from_url(os.getenv("REDIS_URL", "redis://localhost:6379/0"))
 
-async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any]:
-    """
-    Fetches revenue summary, utilizing caching to improve performance.
-    """
-    cache_key = f"revenue:{property_id}"
-
-    # Try to get from cache
-    cached = await redis_client.get(cache_key)
-    if cached:
-        return json.loads(cached)
-
-    # Revenue calculation is delegated to the reservation service.
+
+async def get_revenue_summary(
+    property_id: str, tenant_id: str, month: int = 3, year: int = 2024
+) -> Dict[str, Any]:
+    # Versioning excludes stale results produced by previous calculations.
+    cache_key = "revenue:v2:" + json.dumps([tenant_id, property_id, year, month])
+    try:
+        cached = await redis_client.get(cache_key)
+        if cached:
+            return json.loads(cached)
+    except (RedisError, ValueError):
+        logger.warning("Revenue cache unavailable; querying database")
+
     from app.services.reservations import calculate_total_revenue
-
-    # Calculate revenue
-    result = await calculate_total_revenue(property_id, tenant_id)
-
-    # Cache the result for 5 minutes
-    await redis_client.setex(cache_key, 300, json.dumps(result))
-
+    result = await calculate_total_revenue(property_id, tenant_id, month, year)
+    try:
+        await redis_client.setex(cache_key, 300, json.dumps(result))
+    except RedisError:
+        logger.warning("Could not cache revenue summary")
     return result

backend/app/services/reservations.py

@@ -1,109 +1,56 @@
 from datetime import datetime
 from decimal import Decimal
-from typing import Dict, Any, List
+from typing import Any, Dict
 
-async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal:
-    """
-    Calculates revenue for a specific month.
-    """
+from fastapi import HTTPException
+from sqlalchemy import text
 
-    start_date = datetime(year, month, 1)
-    if month < 12:
-        end_date = datetime(year, month + 1, 1)
-    else:
-        end_date = datetime(year + 1, 1, 1)
-
-    print(f"DEBUG: Querying revenue for {property_id} from {start_date} to {end_date}")
+from app.core.database_pool import db_pool
 
-    # SQL Simulation (This would be executed against the actual DB)
-    query = """
-        SELECT SUM(total_amount) as total
-        FROM reservations
-        WHERE property_id = $1
-        AND tenant_id = $2
-        AND check_in_date >= $3
-        AND check_in_date < $4
-    """
-
-    # In production this query executes against a database session.
-    # result = await db.fetch_val(query, property_id, tenant_id, start_date, end_date)
-    # return result or Decimal('0')
-
-    return Decimal('0') # Placeholder for now until DB connection is finalized
 
-async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]:
-    """
-    Aggregates revenue from database.
-    """
-    try:
-        # Import database pool
-        from app.core.database_pool import DatabasePool
-
-        # Initialize pool if needed
-        db_pool = DatabasePool()
-        await db_pool.initialize()
-
-        if db_pool.session_factory:
-            async with db_pool.get_session() as session:
-                # Use SQLAlchemy text for raw SQL
-                from sqlalchemy import text
-
-                query = text("""
-                    SELECT 
-                        property_id,
-                        SUM(total_amount) as total_revenue,
-                        COUNT(*) as reservation_count
-                    FROM reservations 
-                    WHERE property_id = :property_id AND tenant_id = :tenant_id
-                    GROUP BY property_id
-                """)
-
-                result = await session.execute(query, {
-                    "property_id": property_id, 
-                    "tenant_id": tenant_id
-                })
-                row = result.fetchone()
-
-                if row:
-                    total_revenue = Decimal(str(row.total_revenue))
-                    return {
-                        "property_id": property_id,
-                        "tenant_id": tenant_id,
-                        "total": str(total_revenue),
-                        "currency": "USD", 
-                        "count": row.reservation_count
-                    }
-                else:
-                    # No reservations found for this property
-                    return {
-                        "property_id": property_id,
-                        "tenant_id": tenant_id,
-                        "total": "0.00",
-                        "currency": "USD",
-                        "count": 0
-                    }
-        else:
-            raise Exception("Database pool not available")
-
-    except Exception as e:
-        print(f"Database error for {property_id} (tenant: {tenant_id}): {e}")
-
-        # Create property-specific mock data for testing when DB is unavailable
-        # This ensures each property shows different figures
-        mock_data = {
-            'prop-001': {'total': '1000.00', 'count': 3},
-            'prop-002': {'total': '4975.50', 'count': 4}, 
-            'prop-003': {'total': '6100.50', 'count': 2},
-            'prop-004': {'total': '1776.50', 'count': 4},
-            'prop-005': {'total': '3256.00', 'count': 3}
-        }
-
-        mock_property_data = mock_data.get(property_id, {'total': '0.00', 'count': 0})
-
+async def calculate_monthly_revenue(
+    property_id: str, tenant_id: str, month: int = 3, year: int = 2024
+) -> Dict[str, Any]:
+    """Sum exact amounts in the property's local calendar month, per currency."""
+    start = datetime(year, month, 1)
+    end = datetime(year + (month == 12), 1 if month == 12 else month + 1, 1)
+    await db_pool.initialize()
+    async with db_pool.get_session() as session:
+        result = await session.execute(text("""
+            SELECT timezone FROM properties
+            WHERE id = :property_id AND tenant_id = :tenant_id
+        """), {"property_id": property_id, "tenant_id": tenant_id})
+        prop = result.fetchone()
+        if prop is None:
+            raise HTTPException(status_code=404, detail="Property not found")
+
+        result = await session.execute(text("""
+            SELECT currency, SUM(total_amount) AS total_revenue,
+                   COUNT(*) AS reservation_count
+            FROM reservations
+            WHERE property_id = :property_id AND tenant_id = :tenant_id
+              AND check_in_date >= (CAST(:start AS timestamp) AT TIME ZONE :timezone)
+              AND check_in_date < (CAST(:end AS timestamp) AT TIME ZONE :timezone)
+            GROUP BY currency ORDER BY currency
+        """), {
+            "property_id": property_id, "tenant_id": tenant_id,
+            "start": start, "end": end, "timezone": prop.timezone,
+        })
+        totals = [{
+            "currency": row.currency or "Unknown",
+            "total": str(Decimal(str(row.total_revenue))),
+            "count": row.reservation_count,
+        } for row in result.fetchall()]
         return {
-            "property_id": property_id,
-            "tenant_id": tenant_id, 
-            "total": mock_property_data['total'],
-            "currency": "USD",
-            "count": mock_property_data['count']
+            "property_id": property_id, "tenant_id": tenant_id,
+            "month": month, "year": year, "totals": totals,
+            "total": totals[0]["total"] if len(totals) == 1 else ("0.000" if not totals else None),
+            "currency": totals[0]["currency"] if len(totals) == 1 else None,
+            "count": sum(item["count"] for item in totals),
         }
+
+
+async def calculate_total_revenue(
+    property_id: str, tenant_id: str, month: int = 3, year: int = 2024
+) -> Dict[str, Any]:
+    return await calculate_monthly_revenue(property_id, tenant_id, month, year)

backend/pyproject.toml

@@ -5,6 +5,7 @@ description = "Add your description here"
 readme = "README.md"
 requires-python = ">=3.11"
 dependencies = [
+    "asyncpg>=0.27.0",
     "aiohttp>=3.12.15",
     "aioredis>=2.0.0",
     "beautifulsoup4>=4.13.4",