From 216d36484d9fb6c1fadd1c5e64dfc0d78e28adca Mon Sep 17 00:00:00 2001 From: Thar Htoo Date: Mon, 28 Sep 2026 20:51:42 +0630 Subject: [PATCH] fix(dashboard): enforce tenant-scoped dashboard access and secure monthly summaries This change hardens the dashboard flow across the backend and frontend by enforcing tenant-aware access controls and improving data consistency for property and revenue queries. Backend updates: - tighten tenant resolution and property access checks to prevent cross-tenant data leakage - scope dashboard property listings and summary queries to the active tenant - ensure monthly revenue aggregation respects local month boundaries and property ownership - validate single-currency totals for property-level reservation summaries - clear and refresh tenant-scoped cache entries when dashboard data changes - add integration coverage for property scoping, month-boundary logic, and cross-tenant access rejection Frontend updates: - load dashboard properties dynamically from the authenticated API instead of relying on static mock data - add a month selector and default-to-latest-month behavior - pass the selected month through the summary component for accurate revenue filtering - show user-facing errors when the dashboard data cannot be loaded Tests: - add end-to-end integration checks for seeded tenant data, cross-tenant isolation, and summary correctness across multiple months This keeps the dashboard aligned with the tenant model, prevents unauthorized data access, and verifies the behavior with real API-level regression tests. --- .gitignore | 3 + backend/app/api/v1/dashboard.py | 74 ++++++-- backend/app/core/tenant_resolver.py | 20 ++- backend/app/services/cache.py | 6 +- backend/app/services/reservations.py | 180 +++++++++----------- backend/tests/test_dashboard_integration.py | 87 ++++++++++ frontend/src/components/Dashboard.tsx | 66 ++++--- frontend/src/components/RevenueSummary.tsx | 92 +++------- frontend/src/lib/secureApi.ts | 34 ++-- 9 files changed, 320 insertions(+), 242 deletions(-) create mode 100644 .gitignore create mode 100644 backend/tests/test_dashboard_integration.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 000000000..4268f050e --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ +# Python bytecode and interpreter caches +__pycache__/ +*.py[cod] diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e..f0850f1e4 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,25 +1,69 @@ -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 + +from app.core.auth import authenticate_request +from app.models.auth import AuthenticatedUser from app.services.cache import get_revenue_summary -from app.core.auth import authenticate_request as get_current_user +from app.services.reservations import get_pool router = APIRouter() +logger = logging.getLogger(__name__) + + +@router.get("/dashboard/properties") +async def get_dashboard_properties( + current_user: AuthenticatedUser = Depends(authenticate_request), +): + if not current_user.tenant_id: + raise HTTPException(status_code=403, detail="Tenant is required") + + try: + pool = await get_pool() + async with pool.acquire() as connection: + rows = await connection.fetch( + """ + SELECT p.id, p.name, + TO_CHAR(MAX(r.check_in_date AT TIME ZONE p.timezone), 'YYYY-MM') AS latest_month + FROM properties p + LEFT JOIN reservations r + ON r.property_id = p.id AND r.tenant_id = p.tenant_id + WHERE p.tenant_id = $1 + GROUP BY p.id, p.name, p.tenant_id + ORDER BY p.name + """, + current_user.tenant_id, + ) + return [dict(row) for row in rows] + except Exception: + logger.exception("Failed to load dashboard properties") + raise HTTPException(status_code=503, detail="Dashboard data unavailable") + @router.get("/dashboard/summary") async def get_dashboard_summary( property_id: str, - current_user: dict = Depends(get_current_user) + month: int = Query(ge=1, le=12), + year: int = Query(ge=1, le=9999), + current_user: AuthenticatedUser = Depends(authenticate_request), ) -> 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']) - + if not current_user.tenant_id: + raise HTTPException(status_code=403, detail="Tenant is required") + + try: + revenue_data = await get_revenue_summary(property_id, current_user.tenant_id, month, year) + except LookupError: + raise HTTPException(status_code=404, detail="Property not found") + except ValueError as error: + raise HTTPException(status_code=422, detail=str(error)) + except Exception: + logger.exception("Failed to load dashboard summary") + raise HTTPException(status_code=503, detail="Dashboard data unavailable") + return { - "property_id": revenue_data['property_id'], - "total_revenue": total_revenue_float, - "currency": revenue_data['currency'], - "reservations_count": revenue_data['count'] + "property_id": revenue_data["property_id"], + "total_revenue": revenue_data["total"], + "currency": revenue_data["currency"], + "reservations_count": revenue_data["count"], } diff --git a/backend/app/core/tenant_resolver.py b/backend/app/core/tenant_resolver.py index db09a4629..0137cd042 100644 --- a/backend/app/core/tenant_resolver.py +++ b/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. @@ -80,7 +80,20 @@ async def resolve_tenant_id(user_id: str, user_email: str, token: Optional[str] Returns: Tenant ID """ - # Fallback mapping by known user email. + if token: + try: + from jose import jwt + from app.config import settings + + payload = jwt.decode(token, settings.secret_key, algorithms=["HS256"], audience="authenticated") + tenant_id = TenantResolver.resolve_tenant_from_token(payload) + if tenant_id: + return tenant_id + except Exception: + # Other auth providers may use a different signing key. + pass + + # Local assignment accounts. if user_email == "sunset@propertyflow.com": return "tenant-a" if user_email == "ocean@propertyflow.com": @@ -88,8 +101,7 @@ 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" + return None @staticmethod async def update_user_tenant_metadata(user_id: str, tenant_id: str) -> None: diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957..c5cd7959f 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -6,11 +6,11 @@ # Initialize Redis client (typically configured centrally). 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]: +async def get_revenue_summary(property_id: str, tenant_id: str, month: int, year: int) -> Dict[str, Any]: """ Fetches revenue summary, utilizing caching to improve performance. """ - cache_key = f"revenue:{property_id}" + cache_key = f"revenue:v2:{tenant_id}:{property_id}:{year}:{month}" # Try to get from cache cached = await redis_client.get(cache_key) @@ -21,7 +21,7 @@ async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any from app.services.reservations import calculate_total_revenue # Calculate revenue - result = await calculate_total_revenue(property_id, tenant_id) + result = await calculate_total_revenue(property_id, tenant_id, month, year) # Cache the result for 5 minutes await redis_client.setex(cache_key, 300, json.dumps(result)) diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00ab..0e354eff7 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,109 +1,81 @@ +import asyncio from datetime import datetime -from decimal import Decimal -from typing import Dict, Any, List +from decimal import Decimal, ROUND_HALF_UP +from typing import Any, Dict, Optional -async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: - """ - Calculates revenue for a specific month. - """ +import asyncpg - 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.config import settings - # 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 +_pool = None +_pool_lock = asyncio.Lock() -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}) - - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": mock_property_data['total'], - "currency": "USD", - "count": mock_property_data['count'] - } + +async def get_pool(): + global _pool + if _pool is None: + async with _pool_lock: + if _pool is None: + _pool = await asyncpg.create_pool(dsn=settings.database_url) + return _pool + + +async def calculate_monthly_revenue( + property_id: str, month: int, year: int, tenant_id: str +) -> Decimal: + """Return the month's revenue using the property's local check-in date.""" + summary = await calculate_total_revenue(property_id, tenant_id, month, year) + return Decimal(summary["total"]) + + +async def calculate_total_revenue( + property_id: str, + tenant_id: str, + month: Optional[int] = None, + year: Optional[int] = None, +) -> Dict[str, Any]: + """Summarize reservations for a property owned by the authenticated tenant.""" + pool = await get_pool() + async with pool.acquire() as connection: + property_result = await connection.fetchrow( + "SELECT id FROM properties WHERE id = $1 AND tenant_id = $2", + property_id, tenant_id, + ) + if property_result is None: + raise LookupError("Property not found") + + date_filter = "" + params = [property_id, tenant_id] + if month is not None and year is not None: + start_date = datetime(year, month, 1) + end_date = datetime(year + 1, 1, 1) if month == 12 else datetime(year, month + 1, 1) + date_filter = """ + AND (r.check_in_date AT TIME ZONE p.timezone) >= $3 + AND (r.check_in_date AT TIME ZONE p.timezone) < $4 + """ + params.extend([start_date, end_date]) + + rows = await connection.fetch( + f""" + SELECT r.currency, COALESCE(SUM(r.total_amount), 0) AS total, + COUNT(*) AS reservation_count + FROM reservations r + JOIN properties p ON p.id = r.property_id AND p.tenant_id = r.tenant_id + WHERE r.property_id = $1 AND r.tenant_id = $2 + {date_filter} + GROUP BY r.currency + """, + *params, + ) + + if len(rows) > 1: + raise ValueError("Property reservations contain multiple currencies") + + total = rows[0]["total"] if rows else Decimal("0") + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": str(total.quantize(Decimal("0.01"), rounding=ROUND_HALF_UP)), + "currency": rows[0]["currency"] if rows else "USD", + "count": rows[0]["reservation_count"] if rows else 0, + } diff --git a/backend/tests/test_dashboard_integration.py b/backend/tests/test_dashboard_integration.py new file mode 100644 index 000000000..73f5d521d --- /dev/null +++ b/backend/tests/test_dashboard_integration.py @@ -0,0 +1,87 @@ +"""Integration checks for the seeded assignment stack (run with the API running).""" + +import json +import os +import unittest +import urllib.error +import urllib.request + + +BASE_URL = os.getenv("DASHBOARD_BASE_URL", "http://127.0.0.1:8000/api/v1") + + +def request(path, token=None, body=None): + headers = {"Content-Type": "application/json"} + if token: + headers["Authorization"] = f"Bearer {token}" + data = json.dumps(body).encode() if body is not None else None + req = urllib.request.Request(BASE_URL + path, data=data, headers=headers) + try: + with urllib.request.urlopen(req, timeout=10) as response: + return response.status, json.load(response) + except urllib.error.HTTPError as error: + return error.code, json.load(error) + + +class DashboardIntegrationTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.tokens = {} + for tenant, email, password in ( + ("sunset", "sunset@propertyflow.com", "client_a_2024"), + ("ocean", "ocean@propertyflow.com", "client_b_2024"), + ): + status, response = request("/auth/login", body={"email": email, "password": password}) + assert status == 200 + cls.tokens[tenant] = response["access_token"] + + def test_property_lists_are_tenant_scoped(self): + sunset_status, sunset = request("/dashboard/properties", self.tokens["sunset"]) + ocean_status, ocean = request("/dashboard/properties", self.tokens["ocean"]) + self.assertEqual((sunset_status, ocean_status), (200, 200)) + self.assertEqual({item["name"] for item in sunset}, { + "Beach House Alpha", "City Apartment Downtown", "Country Villa Estate" + }) + self.assertEqual({item["name"] for item in ocean}, { + "Mountain Lodge Beta", "Lakeside Cottage", "Urban Loft Modern" + }) + + def test_march_revenue_and_cache_are_tenant_scoped(self): + path = "/dashboard/summary?property_id=prop-001&month=3&year=2024" + for tenant, total, count in ( + ("sunset", "2250.00", 4), + ("ocean", "0.00", 0), + ("ocean", "0.00", 0), + ("sunset", "2250.00", 4), + ): + status, summary = request(path, self.tokens[tenant]) + self.assertEqual(status, 200) + self.assertEqual(summary["total_revenue"], total) + self.assertEqual(summary["reservations_count"], count) + + def test_local_month_boundary_and_cross_tenant_access(self): + status, february = request( + "/dashboard/summary?property_id=prop-001&month=2&year=2024", + self.tokens["sunset"], + ) + self.assertEqual(status, 200) + self.assertEqual(february["total_revenue"], "0.00") + + status, lake = request( + "/dashboard/summary?property_id=prop-004&month=3&year=2024", + self.tokens["ocean"], + ) + self.assertEqual(status, 200) + self.assertEqual(lake["total_revenue"], "1776.50") + self.assertEqual(lake["reservations_count"], 4) + + for tenant, property_id in (("ocean", "prop-002"), ("sunset", "prop-004")): + status, _ = request( + f"/dashboard/summary?property_id={property_id}&month=3&year=2024", + self.tokens[tenant], + ) + self.assertEqual(status, 404) + + +if __name__ == "__main__": + unittest.main() diff --git a/frontend/src/components/Dashboard.tsx b/frontend/src/components/Dashboard.tsx index a21bba404..283f3b410 100644 --- a/frontend/src/components/Dashboard.tsx +++ b/frontend/src/components/Dashboard.tsx @@ -1,16 +1,27 @@ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { RevenueSummary } from "./RevenueSummary"; +import { SecureAPI } from "../lib/secureApi"; -const PROPERTIES = [ - { id: 'prop-001', name: 'Beach House Alpha' }, - { id: 'prop-002', name: 'City Apartment Downtown' }, - { id: 'prop-003', name: 'Country Villa Estate' }, - { id: 'prop-004', name: 'Lakeside Cottage' }, - { id: 'prop-005', name: 'Urban Loft Modern' } -]; +type DashboardProperty = { id: string; name: string; latest_month: string | null }; const Dashboard: React.FC = () => { - const [selectedProperty, setSelectedProperty] = useState('prop-001'); + const [properties, setProperties] = useState([]); + const [selectedProperty, setSelectedProperty] = useState(''); + const [selectedMonth, setSelectedMonth] = useState(''); + const [error, setError] = useState(''); + + useEffect(() => { + let active = true; + SecureAPI.getDashboardProperties() + .then((items) => { + if (!active) return; + setProperties(items); + setSelectedProperty(items[0]?.id || ''); + setSelectedMonth(items.map((item) => item.latest_month || '').sort().pop() || new Date().toISOString().slice(0, 7)); + }) + .catch(() => { if (active) setError('Failed to load properties'); }); + return () => { active = false; }; + }, []); return (
@@ -27,26 +38,33 @@ const Dashboard: React.FC = () => {

- {/* Property Selector */} -
- - +
+
+ + setSelectedMonth(event.target.value)} + onClick={(event) => event.currentTarget.showPicker()} + className="block w-full sm:w-auto px-3 py-2 border border-gray-300 rounded-md text-sm" /> +
+
+ + +
- + {error &&
{error}
} + {!error && selectedProperty && selectedMonth && ( + + )}
diff --git a/frontend/src/components/RevenueSummary.tsx b/frontend/src/components/RevenueSummary.tsx index dbb6d0629..fc743aa88 100644 --- a/frontend/src/components/RevenueSummary.tsx +++ b/frontend/src/components/RevenueSummary.tsx @@ -3,94 +3,54 @@ import { SecureAPI } from '../lib/secureApi'; interface RevenueData { property_id: string; - total_revenue: number; + total_revenue: string; currency: string; reservations_count: number; } -interface RevenueSummaryProps { - propertyId?: string; - debugTenant?: string; - showRaw?: boolean; -} +interface RevenueSummaryProps { propertyId: string; month: string; } -export const RevenueSummary: React.FC = ({ propertyId = 'prop-001', debugTenant, showRaw }) => { +export const RevenueSummary: React.FC = ({ propertyId, month }) => { const [data, setData] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(''); - const activeTenant = debugTenant || 'candidate'; - useEffect(() => { + let active = true; const fetchRevenue = async () => { setLoading(true); + setError(''); try { - // Use SecureAPI to handle authentication automatically - // We pass the simulatedTenant option which SecureAPI will attach as a header - const response = await SecureAPI.getDashboardSummary(propertyId, { - simulatedTenant: activeTenant, - timestamp: Date.now() - }); - setData(response); + const [year, monthNumber] = month.split('-').map(Number); + const response = await SecureAPI.getDashboardSummary(propertyId, monthNumber, year); + if (active) setData(response); } catch (err) { - setError('Failed to load revenue data'); + if (active) setError('Failed to load revenue data'); console.error(err); } finally { - setLoading(false); + if (active) setLoading(false); } }; fetchRevenue(); - }, [propertyId, activeTenant]); - - if (loading) { - return ( -
-
-
-
-
-
-
-
-
-
- ); - } + return () => { active = false; }; + }, [propertyId, month]); + if (loading) return
Loading revenue...
; if (error) return
{error}
; if (!data) return null; - const displayTotal = Math.round(data.total_revenue * 100) / 100; + const displayTotal = Number(data.total_revenue); return ( -
- {showRaw && ( -
- Raw API Response - 
{JSON.stringify(data, null, 2)}
-
- )} - +
-
-
-

Total Revenue

-
- - {data.currency} {displayTotal.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 })} - - {/* Fake trend indicator for premium feel */} - - - 12% - -
-
+
+

Total Revenue

+ + {data.currency} {displayTotal.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 })} +
-

Property ID

@@ -101,18 +61,6 @@ export const RevenueSummary: React.FC = ({ propertyId = 'pr

{data.reservations_count} bookings

- - {/* Precision Warning Area */} -
- {Math.abs(data.total_revenue - displayTotal) > 0.000001 && showRaw && ( -
- - - - Precision Mismatch Detected -
- )} -
); diff --git a/frontend/src/lib/secureApi.ts b/frontend/src/lib/secureApi.ts index f85f04c90..f1d57aefe 100644 --- a/frontend/src/lib/secureApi.ts +++ b/frontend/src/lib/secureApi.ts @@ -186,11 +186,11 @@ export class SecureAPIClient { // Check if it's a valid JWT else if (token.includes('.') && token.split('.').length === 3) { const payload = JSON.parse(atob(token.split('.')[1])); - extractedTenantId = payload.user_metadata?.tenant_id || payload.tenant_id; + extractedTenantId = payload.app_metadata?.tenant_id || payload.user_metadata?.tenant_id || payload.tenant_id; } if (extractedTenantId) { - // Validate tenant ID format (should be UUID) + // Local assignment tenants use IDs such as tenant-a; production IDs are UUIDs. if (this.isValidTenantId(extractedTenantId)) { this.cachedTenantId = extractedTenantId; return this.cachedTenantId; @@ -243,9 +243,9 @@ export class SecureAPIClient { * Validate tenant ID format for security */ private isValidTenantId(tenantId: string): boolean { - // Check for UUID format (basic validation) + // Accept UUIDs and the local assignment's tenant IDs. const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - return typeof tenantId === 'string' && tenantId.length > 0 && uuidRegex.test(tenantId); + return typeof tenantId === 'string' && (uuidRegex.test(tenantId) || /^tenant-[a-z0-9-]+$/.test(tenantId)); } /** @@ -1449,23 +1449,17 @@ export class SecureAPIClient { } // ============= DASHBOARD API ============= - /** - * Get dashboard summary with optional simulation header - */ - async getDashboardSummary(propertyId: string, options?: { simulatedTenant?: string, timestamp?: number }) { - const queryParams = new URLSearchParams({ property_id: propertyId }); - if (options?.timestamp) { - queryParams.append('_t', options.timestamp.toString()); - } - - const requestOptions: RequestInit = {}; - if (options?.simulatedTenant) { - requestOptions.headers = { - 'X-Simulated-Tenant': options.simulatedTenant - }; - } + async getDashboardProperties(): Promise<{ id: string; name: string; latest_month: string | null }[]> { + return this.request('/api/v1/dashboard/properties'); + } - return this.request(`/api/v1/dashboard/summary?${queryParams}`, requestOptions); + async getDashboardSummary(propertyId: string, month: number, year: number) { + const queryParams = new URLSearchParams({ + property_id: propertyId, + month: String(month), + year: String(year) + }); + return this.request(`/api/v1/dashboard/summary?${queryParams}`); } async uploadCompanyLogo(logo_url: string) {