Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .github/workflows/security-audit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: Tenant Isolation Security Audit

on:
push:
branches: [ main ]
pull_request:
branches: [ main ]

jobs:
security-audit:
name: Run Tenant Isolation Security Tests
runs-on: ubuntu-latest

steps:
- name: Checkout Code
uses: actions/checkout@v3

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'
cache: 'pip'

- name: Install Dependencies
run: |
python -m pip install --upgrade pip
# Install backend dependencies (skip torch/transformers if possible for speed, but they might be imported)
# To avoid heavy PyTorch download in CI, we enable degraded mode or load mocks.
# But since the tests import backend.main which imports services that load models,
# we set ALLOW_DEGRADED_STARTUP=1 so it starts successfully without downloading large weights.
pip install -r backend/requirements.txt
pip install pytest httpx pytest-cov

- name: Run Tenant Isolation Security Tests
env:
ALLOW_DEGRADED_STARTUP: "1"
REQUIRE_SUPABASE: "false"
run: |
python -m pytest backend/tests/test_tenant_isolation.py -v

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}
Comment on lines +11 to +39
16 changes: 14 additions & 2 deletions Frontend/src/services/api.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import axios from 'axios';
import { MOCK_TICKETS } from './mockData';
import { API_CONFIG } from '../config';
import { supabase } from '../lib/supabaseClient';

const USE_MOCK = true;
const API_BASE_URL = API_CONFIG.BACKEND_URL;
Expand Down Expand Up @@ -69,11 +70,18 @@ export const api = {

predictTicket: async (issueText, imageBase64 = "") => {
try {
const currentUser = JSON.parse(sessionStorage.getItem("currentUser") || "{}");
const { data: { session } } = await supabase.auth.getSession();
const token = session?.access_token;

// ALWAYS call the real backend for prediction if possible
const response = await axios.post(`${API_BASE_URL}/ai/analyze_ticket`, {
text: issueText,
image_base64: imageBase64,
image_text: ""
image_text: "",
company_id: currentUser.company_id || currentUser.companyId || null
}, {
headers: token ? { Authorization: `Bearer ${token}` } : {}
});

const result = response.data;
Expand Down Expand Up @@ -120,7 +128,11 @@ export const api = {

logCorrection: async (correctionPayload) => {
try {
await axios.post(`${API_BASE_URL}/ai/log_correction`, correctionPayload);
const { data: { session } } = await supabase.auth.getSession();
const token = session?.access_token;
await axios.post(`${API_BASE_URL}/ai/log_correction`, correctionPayload, {
headers: token ? { Authorization: `Bearer ${token}` } : {}
});
} catch (error) {
// Non-fatal: log but don't break the UI flow
console.warn("[Correction Log] Failed to save correction:", error);
Expand Down
8 changes: 7 additions & 1 deletion Frontend/src/user/pages/TicketTracking.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Card, CardContent } from "../../components/ui/card";
import TicketTimeline from "../components/TicketTimeline";
import axios from 'axios';
import { API_CONFIG } from '../../config';
import { supabase } from '../../lib/supabaseClient';

const TicketTracking = () => {
const navigate = useNavigate();
Expand Down Expand Up @@ -68,7 +69,12 @@ const TicketTracking = () => {
routing_confidence: aiTicket.confidence
};

const res = await axios.post(`${API_CONFIG.BACKEND_URL}/tickets/save`, savePayload);
const { data: { session } } = await supabase.auth.getSession();
const token = session?.access_token;

const res = await axios.post(`${API_CONFIG.BACKEND_URL}/tickets/save`, savePayload, {
headers: token ? { Authorization: `Bearer ${token}` } : {}
});

if (res.data?.ticket_id) {
const newTicket = { ...aiTicket, id: res.data.ticket_id, ticket_id: res.data.ticket_id, status };
Expand Down
233 changes: 233 additions & 0 deletions backend/auth/tenant_middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
import os
import time
import logging
from typing import Dict, Optional
from fastapi import Request, HTTPException, Depends, status
from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials
from postgrest.exceptions import APIError

logger = logging.getLogger(__name__)

# Reusable security scheme to extract token
security_scheme = HTTPBearer(auto_error=False)

# Cache user profiles in memory (user_id -> {company_id, role, cache_time})
_profile_cache: Dict[str, dict] = {}
CACHE_TTL_SECONDS = 300 # 5 minutes cache

class TenantSecurityManager:
def __init__(self, supabase_client=None):
self._supabase = supabase_client

@property
def supabase(self):
# Lazy load or use the global client
if self._supabase is None:
from backend.main import supabase as global_supabase
self._supabase = global_supabase
return self._supabase

def resolve_user_profile(self, user_id: str) -> dict:
"""Retrieves and caches the user's company_id and role from the profiles table."""
now = time.time()

# Check cache
if user_id in _profile_cache:
cache_entry = _profile_cache[user_id]
if now - cache_entry["cached_at"] < CACHE_TTL_SECONDS:
return cache_entry["profile"]

if not self.supabase:
# Degraded/Mock fallback
return {"company_id": None, "role": "user", "id": user_id}

try:
res = (
self.supabase.table("profiles")
.select("id, company_id, role")
.eq("id", user_id)
.single()
.execute()
)
profile_data = res.data or {}

# Cache the result
_profile_cache[user_id] = {
"profile": profile_data,
"cached_at": now
}
return profile_data
except Exception as e:
logger.error(f"Error fetching user profile for {user_id}: {e}")
# Fallback to no company_id (safe default)
return {"company_id": None, "role": "user", "id": user_id}

async def get_current_user_profile(self, request: Request, credentials: Optional[HTTPAuthorizationCredentials] = Depends(security_scheme)) -> dict:
"""
Extracts token, validates auth with Supabase, and returns the resolved profile.
Supports mock tokens for testing/offline audits.
"""
if not credentials:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Authentication credentials missing."
)

token = credentials.credentials

# --- MOCK TOKENS FOR TESTING / OFFLINE MODE ---
if token.startswith("mock-token-"):
parts = token.split("-")
# Format: mock-token-[company_id]-[role]-[user_id]
# e.g., mock-token-companyA-admin-user123
company_id = parts[2] if len(parts) > 2 else "company-mock-default"
role = parts[3] if len(parts) > 3 else "user"
user_id = parts[4] if len(parts) > 4 else f"user-{company_id}-{role}"

if company_id == "master":
return {"id": "master-admin-id", "company_id": None, "role": "master_admin"}

return {"id": user_id, "company_id": company_id, "role": role}
Comment on lines +78 to +90
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: unconditional mock-token path is an authentication bypass in production.

Any client can send Authorization: Bearer mock-token-master-... to be authenticated as master_admin, or mock-token-<company>-<role>-<id> to impersonate any tenant/role — Supabase validation is skipped entirely. There is no environment guard, so this is reachable in production and defeats the tenant isolation this PR introduces. The same family of bypass exists for mock- resource IDs in verify_resource_ownership (Line 171) and mock-user- IDs in main.py::get_user_by_id.

Gate all mock paths behind an explicit, default-off env flag.

🔒 Proposed fix (apply the same guard to the other mock paths)
         token = credentials.credentials

         # --- MOCK TOKENS FOR TESTING / OFFLINE MODE ---
-        if token.startswith("mock-token-"):
+        if os.getenv("ENABLE_MOCK_AUTH") == "1" and token.startswith("mock-token-"):
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/auth/tenant_middleware.py` around lines 78 - 90, The mock-token
branch in tenant_middleware.py must be gated behind an explicit default-off
environment flag (e.g., ENABLE_MOCK_AUTH=false); change the
token.startswith("mock-token-") check to first read and validate that flag (only
allow mock handling when the env var is truthy), otherwise proceed with normal
Supabase validation and reject/unauthenticate the request; apply the same guard
logic to the other mock paths mentioned (verify_resource_ownership's mock-
resource IDs and main.py's get_user_by_id mock-user- handling) so all mock
shortcuts are disabled unless the flag is enabled, and emit a clear log message
when mock mode is rejected.


if not self.supabase:
raise HTTPException(
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
detail="Database service not initialized."
)

try:
# Validate token against Supabase Auth
user_res = self.supabase.auth.get_user(token)
if not user_res or not user_res.user:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Invalid or expired token."
)

user = user_res.user
profile = self.resolve_user_profile(user.id)
if not profile:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="User profile not registered."
)
return profile

except Exception as e:
logger.warning(f"Auth verification failed: {e}")
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Authentication failed."
)
Comment on lines +98 to +121
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Inner HTTPExceptions are swallowed and rewritten to a generic 401.

The broad except Exception catches the deliberate HTTPException(401, "Invalid or expired token.") and HTTPException(403, "User profile not registered.") and re-raises everything as 401 "Authentication failed.", discarding the intended status/detail (the 403 case can never surface). Add an except HTTPException: raise before the catch-all, and chain with from e (addresses Ruff B904).

🛠️ Proposed fix
             return profile

+        except HTTPException:
+            raise
         except Exception as e:
             logger.warning(f"Auth verification failed: {e}")
             raise HTTPException(
                 status_code=status.HTTP_401_UNAUTHORIZED,
                 detail="Authentication failed."
-            )
+            ) from e
🧰 Tools
🪛 Ruff (0.15.14)

[warning] 116-116: Do not catch blind exception: Exception

(BLE001)


[warning] 118-121: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/auth/tenant_middleware.py` around lines 98 - 121, The current broad
except block in the tenant middleware swallows deliberate HTTPExceptions from
the Supabase token checks and profile resolution (e.g., the HTTPException raised
for invalid token and for unregistered profile), so update the exception
handling in the method that calls self.supabase.auth.get_user and
resolve_user_profile by adding an explicit "except HTTPException: raise" before
the generic except, and in the generic except Exception as e re-raise a new
HTTPException while chaining from e (using "from e") so original trace is
preserved (also ensure you reference logger for the warning as currently used).


def verify_tenant_access(self, target_company_id: Optional[str], current_user: dict) -> None:
"""
Verifies that the authenticated user belongs to the target company.
Master Admins can access any company.
"""
if current_user.get("role") == "master_admin":
return # Master admin bypass

user_company_id = current_user.get("company_id")
if not user_company_id:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="User is not assigned to any tenant organization."
)

if target_company_id and str(target_company_id) != str(user_company_id):
logger.warning(
f"Tenant Access Spoofing Blocked: user {current_user.get('id')} "
f"tried accessing company {target_company_id} (assigned: {user_company_id})"
)
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied: You do not have permissions for this tenant."
)

def verify_resource_ownership(self, table_name: str, resource_id: str, current_user: dict) -> dict:
"""
Verifies that a database resource (e.g. ticket) belongs to the authenticated user's company.
Prevents Insecure Direct Object References (IDOR).
"""
if current_user.get("role") == "master_admin":
# Master Admin bypass, fetch directly
if not self.supabase:
return {}
try:
res = self.supabase.table(table_name).select("*").eq("id", resource_id).single().execute()
return res.data or {}
except Exception:
raise HTTPException(status_code=404, detail=f"{table_name.capitalize()} not found.")

user_company_id = current_user.get("company_id")
if not user_company_id:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied: User has no tenant assignments."
)

# MOCK FALLBACK for testing
if resource_id.startswith("mock-"):
parts = resource_id.split("-")
# Resource ID format: mock-[type]-[company_id]-[id]
resource_company = parts[2] if len(parts) > 2 else "company-mock-default"
if resource_company != user_company_id:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied: Resource belongs to another organization."
)
return {"id": resource_id, "company_id": resource_company}

if not self.supabase:
raise HTTPException(
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
detail="Database service not initialized."
)

try:
# Query the table to check resource ownership.
# We enforce company_id check in the SQL query itself for secure-by-design lookup
res = (
self.supabase.table(table_name)
.select("*")
.eq("id", resource_id)
.eq("company_id", user_company_id)
.execute()
)
if not res.data:
# To prevent resource enumeration/enumeration attacks, we can either return 404 or 403.
# Returning 404 makes it seem like the ticket doesn't exist, which is safer,
# but if the ticket *does* exist in another company, returning 404 avoids leakage.
# However, returning 403 or 404 depending on requirements. Let's return 404 to block scanning,
# or check if it exists in another company to return 403. Returning 403 Forbidden is requested.
# Let's do a quick check if it exists at all to differentiate 404 vs 403.
exist_check = self.supabase.table(table_name).select("id").eq("id", resource_id).execute()
if exist_check.data:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Access denied: Resource belongs to another organization."
)
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=f"{table_name.capitalize()} not found."
)

return res.data[0]
except APIError as e:
logger.error(f"Supabase APIError in verify_resource_ownership: {e}")
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="Database query error."
)
except HTTPException:
raise
except Exception as e:
logger.error(f"Error checking resource ownership: {e}")
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=f"{table_name.capitalize()} not found."
)

# Create singleton security manager
security_manager = TenantSecurityManager()
Loading
Loading