From ef238b014583f3b86f78eafcb84cfa92e9d343af Mon Sep 17 00:00:00 2001 From: duffyduck Date: Sun, 17 May 2026 21:47:20 +0200 Subject: [PATCH] Security-Hardening Runde 13: Live-Vollmacht-Konsistenz + embedded DTOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pentest Runde 10: MEDIUM – Stale Token nach Vollmacht-Widerruf: Selbst ein frischer Portal-Login lieferte JWT mit representedCustomer- Ids/representedCustomers, obwohl die Vollmacht widerrufen war. Live- Check beim Datenzugriff fing das ab (403), aber die UI zeigte weiter „kann vertreten". customerLogin und getCustomerPortalUser (= /me + Refresh) filtern representingFor jetzt zusätzlich über getAuthorizedCustomerIds() – nur Beziehungen mit isGranted=true landen im Token. MEDIUM – DTO-Leak in embedded Objekten: GET /customers/:id lieferte contracts[] mit commission/notes/ portalPasswordEncrypted/nextReviewDate; embedded customer in /contracts/:id zeigte notes. sanitizeCustomer(Strict) ruft jetzt sanitizeContract(Strict) auf jedes Element von contracts[] auf; `notes` ist als PORTAL_HIDDEN_CUSTOMER_FIELDS aufgenommen. LOW – /tasks?customerId=X gibt 200 mit leerem Array statt 403: Konsistenz-Fix: wenn Portal-User explizit nach customerId filtert, die er nicht vertreten darf → 403. Live-verifiziert: - Customer 1 vertritt 2+3 (Vollmachten widerrufen) → JWT representedCustomerIds=[], /me dito - Portal /customers/1.contracts[0]: keine Leaks; Admin sieht weiter commission/notes; portalPasswordEncrypted generell weg - Portal /tasks?customerId=2 → 403; /tasks?customerId=1 → 200 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../controllers/contractTask.controller.ts | 11 +++++- backend/src/services/auth.service.ts | 25 +++++++++++--- backend/src/utils/sanitize.ts | 19 ++++++++--- docs/todo.md | 34 +++++++++++++++++++ 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/backend/src/controllers/contractTask.controller.ts b/backend/src/controllers/contractTask.controller.ts index 1718e63a..4a442cd2 100644 --- a/backend/src/controllers/contractTask.controller.ts +++ b/backend/src/controllers/contractTask.controller.ts @@ -12,6 +12,7 @@ import { canAccessContract, getPortalAllowedCustomerIds } from '../utils/accessC export async function getAllTasks(req: AuthRequest, res: Response): Promise { try { const { status, customerId } = req.query; + const customerIdNum = customerId ? parseInt(customerId as string) : undefined; // Für Kundenportal: Filter auf erlaubte Kunden (mit Live-Vollmacht-Check) const allowedIds = await getPortalAllowedCustomerIds(req); @@ -19,6 +20,14 @@ export async function getAllTasks(req: AuthRequest, res: Response): Promise rep.customer.id + // IDs der Kunden sammeln, die dieser Kunde vertreten kann – + // GEFILTERT auf aktive Vollmacht (isGranted: true). Ohne diesen Filter + // hätte das frische JWT nach Vollmacht-Widerruf weiterhin die alte + // representedCustomerIds-Liste; die UI würde dem Vertreter noch + // anzeigen, dass er vertreten kann, obwohl der Live-Check beim + // Datenzugriff dann 403 wirft. Pentest Runde 10 (2026-05-17), MEDIUM. + const grantedCustomerIds = new Set(await getAuthorizedCustomerIds(customer.id)); + const grantedRepresentingFor = customer.representingFor.filter((rep) => + grantedCustomerIds.has(rep.customer.id), ); + const representedCustomerIds = grantedRepresentingFor.map((rep) => rep.customer.id); // Kundenportal-Berechtigungen (eingeschränkt) const customerPermissions = [ @@ -251,7 +259,7 @@ export async function customerLogin(email: string, password: string) { customerId: customer.id, isCustomerPortal: true, mustChangePassword, - representedCustomers: customer.representingFor.map((rep) => ({ + representedCustomers: grantedRepresentingFor.map((rep) => ({ id: rep.customer.id, customerNumber: rep.customer.customerNumber, firstName: rep.customer.firstName, @@ -538,6 +546,13 @@ export async function getCustomerPortalUser(customerId: number) { 'customers:read', ]; + // Selbe Live-Vollmacht-Filterung wie in customerLogin (Pentest Runde 10): + // ohne sie zeigt /me dem Vertreter weiterhin widerrufene Beziehungen. + const grantedCustomerIds = new Set(await getAuthorizedCustomerIds(customer.id)); + const grantedRepresentingFor = customer.representingFor.filter((rep) => + grantedCustomerIds.has(rep.customer.id), + ); + return { id: customer.id, email: customer.portalEmail, @@ -547,7 +562,7 @@ export async function getCustomerPortalUser(customerId: number) { customerId: customer.id, permissions: customerPermissions, isCustomerPortal: true, - representedCustomers: customer.representingFor.map((rep) => ({ + representedCustomers: grantedRepresentingFor.map((rep) => ({ id: rep.customer.id, customerNumber: rep.customer.customerNumber, firstName: rep.customer.firstName, diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 9c5941d9..377c6893 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -31,6 +31,9 @@ const PORTAL_HIDDEN_CUSTOMER_FIELDS = [ 'privacyPolicyPath', 'businessRegistrationPath', 'commercialRegisterPath', + // Pentest Runde 10 (2026-05-17): notes sind interne CRM-Vermerke + // ("Kunde ist schwierig" etc.) und gehören nicht in die Portal-Sicht. + 'notes', ] as const; // Felder die im Contract NIE rausgehen dürfen (auch nicht an Mitarbeiter). @@ -59,24 +62,29 @@ const SENSITIVE_USER_FIELDS = [ * Entfernt Passwort-Hash, Reset-Token etc. aus einem Customer-Objekt. * `portalPasswordEncrypted` bleibt nur drin, wenn der Caller Admin-Rechte hat * (wird in einem zweiten Schritt vom Controller gemacht). Dieser Helper entfernt - * es standardmäßig. + * es standardmäßig. Embedded `contracts[]` werden ebenfalls sanitisiert + * (Pentest Runde 10 – DTO-Leak in eingebetteten Objekten). */ export function sanitizeCustomer>(customer: T | null): T | null { if (!customer) return customer; - const copy = { ...customer }; + const copy: Record = { ...customer }; for (const field of SENSITIVE_CUSTOMER_FIELDS) { delete copy[field]; } + if (Array.isArray(copy.contracts)) { + copy.contracts = (copy.contracts as Record[]).map((c) => sanitizeContract(c)); + } // portalPasswordEncrypted bleibt hier zunächst drin, damit Mitarbeiter das // Portal-Passwort ggf. in der UI anzeigen können. Wird per requirePermission // auf 'customers:update' implizit gesichert. - return copy; + return copy as T; } /** * Entfernt portalPasswordEncrypted + portal-interne Workflow-Felder zusätzlich * zu den allgemein sensiblen Feldern. Für Kontexte in denen der Caller KEIN - * Admin ist (z.B. Portal-Kunde). + * Admin ist (z.B. Portal-Kunde). Embedded `contracts[]` werden mit der + * Strict-Variante sanitisiert. */ export function sanitizeCustomerStrict>(customer: T | null): T | null { if (!customer) return customer; @@ -86,6 +94,9 @@ export function sanitizeCustomerStrict>(custom for (const field of PORTAL_HIDDEN_CUSTOMER_FIELDS) { delete copy[field]; } + if (Array.isArray(copy.contracts)) { + copy.contracts = (copy.contracts as Record[]).map((c) => sanitizeContractStrict(c)); + } return copy as T; } diff --git a/docs/todo.md b/docs/todo.md index 3bf3140f..b45522ef 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,40 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🚨 Pentest Runde 10 – Live-Vollmacht-Konsistenz + DTO-Leaks in embedded Objekten** + - **MEDIUM – Stale Token nach Vollmacht-Widerruf**: + Selbst ein FRISCHER Portal-Login lieferte JWT mit + `representedCustomerIds: [7]` und `representedCustomers: [{Nina,…}]`, + obwohl die Vollmacht widerrufen war. Live-Check beim Datenzugriff + funktionierte (403), aber die UI zeigte dem Vertreter weiter, dass + er Nina vertreten könne. + * **Fix**: `customerLogin` und `getCustomerPortalUser` (= /me + + Refresh-Pfad) filtern `representingFor` jetzt zusätzlich über + `getAuthorizedCustomerIds()` – nur Beziehungen mit + `isGranted: true` landen im Token und in /me. + * Verifiziert: Customer 1 (vertritt 2,3 aber alle Vollmachten + widerrufen) → JWT.representedCustomerIds = `[]`, /me ebenfalls. + - **MEDIUM – DTO-Leak in embedded Objekten**: + `GET /customers/:id` lieferte zwar Customer-Top-Level sanitisiert, + aber `contracts[]` darin enthielt weiterhin `commission`, `notes`, + `portalPasswordEncrypted`, `nextReviewDate`. Analog `notes` auf + embedded customer in `/contracts/:id`. + * **Fix**: `sanitizeCustomer(Strict)` ruft jetzt + `sanitizeContract(Strict)` für jedes Element in `contracts[]` + auf. `notes` zu `PORTAL_HIDDEN_CUSTOMER_FIELDS` ergänzt + (interne CRM-Vermerke). + * Verifiziert: Portal-User sieht in `customers/1.contracts[*]` + keine commission/notes/PW-Encrypted/nextReviewDate mehr; + Admin sieht sie weiterhin (Workflow-Bedarf); + `portalPasswordEncrypted` ist generell entfernt (Klartext nur + via `/contracts/:id/password` mit Audit-Log). + - **LOW – `/tasks?customerId=X` 200 statt 403 für fremde IDs**: + Konsistenz-Issue: nach Vollmacht-Widerruf gab der Endpoint + leeres Array statt einen klaren 403-Fehler. Jetzt: wenn der + Portal-User explizit nach einer customerId filtert, die er nicht + (mehr) vertreten darf → 403 mit "Kein Zugriff auf diese + Kundendaten". Verifiziert. + - [x] **🚨 Pentest Runde 7 (Anschlussrunde) – Information-Disclosure + Input-Validation** - **MEDIUM – Interne Felder in Portal-Responses**: * `sanitizeCustomerStrict` strippt jetzt zusätzlich