fix: billing tenant isolation, invoice status validation, SMTP password masking

H2: list_invoices now passes tenant_id (from current_user) to get_invoices;
    get_invoices applies WHERE tenant_id filter for non-global-admin users;
    get_invoice_endpoint returns 404 when tenant mismatch for non-admins.

H3: InvoiceStatusUpdate.status changed to Literal["draft","sent","paid","cancelled"]
    for schema-level validation; guard also added in update_invoice_status service.

H5: _settings_to_out masks smtp_password as "***" when set, "" when empty;
    update_settings skips writing when value is the "***" sentinel.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-07-22 13:40:59 +02:00
co-authored by Claude Sonnet 4.6
parent a11d2fb1e7
commit b69190dd86
5 changed files with 25 additions and 5 deletions
+9
View File
@@ -534,5 +534,14 @@ Der Admin-Settings-Endpunkt (`GET /api/admin/settings`) erfordert `global_admin`
### 2026-07-22 | Workflow-Editor | _legacy_dispatch umging cancelled/rejected Pre-Check ### 2026-07-22 | Workflow-Editor | _legacy_dispatch umging cancelled/rejected Pre-Check
`_legacy_dispatch` in `dispatch_service.py` rief `render_order_line_task.delay()` direkt auf und umging damit den Pre-Check in `dispatch_order_line_render` (der cancelled/rejected Lines überspringt). Alle Legacy-Dispatch-Pfade (auch im Graph-Fallback) liefen so durch, auch für bereits gecancelte Jobs. **Lösung:** `_legacy_dispatch` ruft jetzt `dispatch_order_line_render.delay()` auf statt `render_order_line_task.delay()` direkt. `_legacy_dispatch` in `dispatch_service.py` rief `render_order_line_task.delay()` direkt auf und umging damit den Pre-Check in `dispatch_order_line_render` (der cancelled/rejected Lines überspringt). Alle Legacy-Dispatch-Pfade (auch im Graph-Fallback) liefen so durch, auch für bereits gecancelte Jobs. **Lösung:** `_legacy_dispatch` ruft jetzt `dispatch_order_line_render.delay()` auf statt `render_order_line_task.delay()` direkt.
### 2026-07-22 | Security | Billing-Rechnungen ohne Tenant-Isolation
`list_invoices` in `billing/router.py` übergab `tenant_id` nicht an `get_invoices`. `get_invoices` im Service ignorierte den Parameter — kein WHERE-Filter vorhanden. Jeder `admin_or_pm`-User konnte alle Rechnungen aller Tenants sehen. **Lösung:** Router extrahiert `tenant_id` aus `current_user` (nur wenn nicht `global_admin`), Service filtert mit `.where(Invoice.tenant_id == tenant_id)`. `get_invoice_endpoint` prüft zusätzlich `inv.tenant_id == current_user.tenant_id` für Nicht-Admins.
### 2026-07-22 | Security | Invoice-Status nicht validiert — beliebige Strings schreibbar
`InvoiceStatusUpdate.status` war `str` ohne Einschränkung. `VALID_STATUSES` in `billing/service.py` war definiert aber nie referenziert. **Lösung:** `InvoiceStatusUpdate.status` auf `Literal["draft","sent","paid","cancelled"]` geändert (Pydantic-Validierung auf Schemaebene). Zusätzlich guard in `update_invoice_status` als defence-in-depth.
### 2026-07-22 | Security | SMTP-Passwort im Klartext in GET /api/admin/settings
`SettingsOut.smtp_password` gab den gespeicherten Wert an jeden `global_admin` zurück. **Lösung:** `_settings_to_out` maskiert den Wert — `"***"` wenn gesetzt, `""` wenn leer. `update_settings` überspringt das Schreiben wenn `body.smtp_password == "***"` (Sentinel für "unverändert lassen").
### 2026-07-22 | Refactoring | Turntable-Branch aus render_order_line_task extrahiert ### 2026-07-22 | Refactoring | Turntable-Branch aus render_order_line_task extrahiert
`render_order_line_task` war ein 427-Zeilen-Monolith. Der Turntable-Branch (~55 Zeilen) wurde in `_render_turntable(*, render_invocation, step_path, output_path, template, order_line_id, emit, pl)` ausgelagert — resolved objects als Parameter (Option B), damit Session und PipelineLogger im Main Task verbleiben und kein doppeltes DB-Lookup entsteht. Der 68-Zeilen Exception-Handler (Mark-as-failed bei max_retries) wurde in `_handle_render_task_exhausted(order_line_id, exc, tenant_id)` extrahiert; die Retry-Logik (`self.retry`) bleibt im Main Task, da sie den Celery `self`-Context benötigt. Beide Helper stehen in `render_order_line.py` vor den Task-Definitionen. `render_order_line_task` war ein 427-Zeilen-Monolith. Der Turntable-Branch (~55 Zeilen) wurde in `_render_turntable(*, render_invocation, step_path, output_path, template, order_line_id, emit, pl)` ausgelagert — resolved objects als Parameter (Option B), damit Session und PipelineLogger im Main Task verbleiben und kein doppeltes DB-Lookup entsteht. Der 68-Zeilen Exception-Handler (Mark-as-failed bei max_retries) wurde in `_handle_render_task_exhausted(order_line_id, exc, tenant_id)` extrahiert; die Retry-Logik (`self.retry`) bleibt im Main Task, da sie den Celery `self`-Context benötigt. Beide Helper stehen in `render_order_line.py` vor den Task-Definitionen.
+2 -2
View File
@@ -232,7 +232,7 @@ def _settings_to_out(raw: dict[str, str]) -> SettingsOut:
smtp_host=raw.get("smtp_host", ""), smtp_host=raw.get("smtp_host", ""),
smtp_port=int(raw.get("smtp_port", "587")), smtp_port=int(raw.get("smtp_port", "587")),
smtp_user=raw.get("smtp_user", ""), smtp_user=raw.get("smtp_user", ""),
smtp_password=raw.get("smtp_password", ""), smtp_password="***" if raw.get("smtp_password", "") else "",
smtp_from_address=raw.get("smtp_from_address", ""), smtp_from_address=raw.get("smtp_from_address", ""),
scene_linear_deflection=float(raw.get("scene_linear_deflection", "0.1")), scene_linear_deflection=float(raw.get("scene_linear_deflection", "0.1")),
scene_angular_deflection=float(raw.get("scene_angular_deflection", "0.1")), scene_angular_deflection=float(raw.get("scene_angular_deflection", "0.1")),
@@ -357,7 +357,7 @@ async def update_settings(
updates["smtp_port"] = str(body.smtp_port) updates["smtp_port"] = str(body.smtp_port)
if body.smtp_user is not None: if body.smtp_user is not None:
updates["smtp_user"] = body.smtp_user updates["smtp_user"] = body.smtp_user
if body.smtp_password is not None: if body.smtp_password is not None and body.smtp_password != "***":
updates["smtp_password"] = body.smtp_password updates["smtp_password"] = body.smtp_password
if body.smtp_from_address is not None: if body.smtp_from_address is not None:
updates["smtp_from_address"] = body.smtp_from_address updates["smtp_from_address"] = body.smtp_from_address
+8 -2
View File
@@ -6,7 +6,8 @@ from fastapi.responses import Response
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.database import get_db from app.database import get_db
from app.utils.auth import require_admin_or_pm from app.utils.auth import require_admin_or_pm, _role_value
from app.domains.auth.models import ADMIN_ROLES
from app.domains.billing.schemas import InvoiceCreate, InvoiceOut, InvoiceStatusUpdate from app.domains.billing.schemas import InvoiceCreate, InvoiceOut, InvoiceStatusUpdate
from app.domains.billing.service import ( from app.domains.billing.service import (
create_invoice, get_invoices, get_invoice, create_invoice, get_invoices, get_invoice,
@@ -26,7 +27,8 @@ async def list_invoices(
db: AsyncSession = Depends(get_db), db: AsyncSession = Depends(get_db),
current_user=Depends(require_admin_or_pm), current_user=Depends(require_admin_or_pm),
): ):
return await get_invoices(db, skip=skip, limit=limit) tenant_id = None if _role_value(current_user) in ADMIN_ROLES else getattr(current_user, "tenant_id", None)
return await get_invoices(db, tenant_id=tenant_id, skip=skip, limit=limit)
@invoice_router.post("/invoices", response_model=InvoiceOut, status_code=status.HTTP_201_CREATED) @invoice_router.post("/invoices", response_model=InvoiceOut, status_code=status.HTTP_201_CREATED)
@@ -57,6 +59,10 @@ async def get_invoice_endpoint(
inv = await get_invoice(db, invoice_id) inv = await get_invoice(db, invoice_id)
if not inv: if not inv:
raise HTTPException(status_code=404, detail="Invoice not found") raise HTTPException(status_code=404, detail="Invoice not found")
if _role_value(current_user) not in ADMIN_ROLES:
user_tenant = getattr(current_user, "tenant_id", None)
if inv.tenant_id != user_tenant:
raise HTTPException(status_code=404, detail="Invoice not found")
return inv return inv
+2 -1
View File
@@ -3,6 +3,7 @@ from __future__ import annotations
import uuid import uuid
from datetime import date, datetime from datetime import date, datetime
from decimal import Decimal from decimal import Decimal
from typing import Literal
from pydantic import BaseModel, computed_field from pydantic import BaseModel, computed_field
@@ -35,7 +36,7 @@ class InvoiceCreate(BaseModel):
class InvoiceStatusUpdate(BaseModel): class InvoiceStatusUpdate(BaseModel):
status: str # draft|sent|paid|cancelled status: Literal["draft", "sent", "paid", "cancelled"]
class InvoiceOut(BaseModel): class InvoiceOut(BaseModel):
+4
View File
@@ -283,6 +283,8 @@ async def get_invoices(
.offset(skip) .offset(skip)
.limit(limit) .limit(limit)
) )
if tenant_id is not None:
q = q.where(Invoice.tenant_id == tenant_id)
result = await db.execute(q) result = await db.execute(q)
return list(result.scalars().all()) return list(result.scalars().all())
@@ -297,6 +299,8 @@ async def get_invoice(db: AsyncSession, invoice_id: uuid.UUID) -> Invoice | None
async def update_invoice_status(db: AsyncSession, invoice_id: uuid.UUID, status: str) -> Invoice | None: async def update_invoice_status(db: AsyncSession, invoice_id: uuid.UUID, status: str) -> Invoice | None:
if status not in VALID_STATUSES:
raise ValueError(f"Invalid status '{status}'. Must be one of: {', '.join(sorted(VALID_STATUSES))}")
invoice = await get_invoice(db, invoice_id) invoice = await get_invoice(db, invoice_id)
if not invoice: if not invoice:
return None return None