From a982795388c22561f42e726092b4d1ac72e7d2c1 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Sat, 16 May 2026 23:47:17 +0200 Subject: [PATCH] Security-Hardening Runde 10: Pentest Runde 6 (8 Findings + struktureller Audit-Sweep) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KRITISCH: - emails/:id/thread bekommt canAccessCachedEmail - customers/:customerId/representatives/search bekommt canAccessCustomer (Buchstaben-Brute-Force konnte sonst die Kunden-DB enumerieren) HOCH: - birthdays/upcoming: Portal-User → 403 (Name/E-Mail/Telefon/Geb-Datum aller Kunden leakte) - contracts/:id/history (GET/POST/PUT/DELETE) bekommt canAccessContract - mailbox-accounts / unread-count / contracts/:id/emails/folder-counts bekommen canAccessCustomer bzw. canAccessContract - Vertreter-Vollmacht-Check ist jetzt live: neuer Helper getPortalAllowedCustomerIds() in accessControl.ts ruft hasAuthorization() für jedes vertretene Customer ab. Eingesetzt in getTasks/createSupportTicket/createCustomerReply/getAllTasks/ getTaskStats und updateCustomerConsent. Widerrufene Vollmachten haben jetzt SOFORT keinen Zugriff mehr (vorher: bis JWT abläuft). MITTEL: - confirmPasswordReset speichert portalPasswordEncrypted nicht mehr beim Self-Service-Reset (war nur für Admin-OTPs gedacht); + portalPasswordMustChange=false explizit - getCustomers pagination total reflektiert jetzt nur erlaubte IDs (über DB-Filter in customerService.getAllCustomers) Audit-Sweep (defense in depth, falls Rolle versehentlich Update- Permissions bekommt): - 16 cachedEmail-Operationen (markAsRead, toggleStar, assign/unassign, save-as-pdf/invoice/contract-document, save-to, attachment-targets, trash-ops) - 4 contract-Operationen (createFollowUp, createRenewal, snoozeContract, removeContractMeter) - 12 sub-CRUD-Operationen (address/bankcard/document/meter update+delete, meter-reading add/update/delete/transfer) - 2 representative-Operationen (add/remove) Live-verifiziert: Portal-Customer-3 auf alle fremden IDs → 403, Admin sieht alles, eigene Ressourcen weiterhin 200, Customer 1 mit widerrufener Vollmacht für Customer 3 → 0 fremde Verträge in der Response. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controllers/birthday.controller.ts | 7 ++ .../src/controllers/cachedEmail.controller.ts | 62 ++++++---- .../src/controllers/contract.controller.ts | 6 +- .../controllers/contractHistory.controller.ts | 5 + .../controllers/contractTask.controller.ts | 112 +++++++----------- .../src/controllers/customer.controller.ts | 80 ++++++++----- backend/src/controllers/gdpr.controller.ts | 14 +-- backend/src/services/auth.service.ts | 9 +- backend/src/services/customer.service.ts | 9 +- backend/src/utils/accessControl.ts | 28 +++++ docs/todo.md | 61 ++++++++++ 11 files changed, 256 insertions(+), 137 deletions(-) diff --git a/backend/src/controllers/birthday.controller.ts b/backend/src/controllers/birthday.controller.ts index a8f5508f..c26db5ca 100644 --- a/backend/src/controllers/birthday.controller.ts +++ b/backend/src/controllers/birthday.controller.ts @@ -11,6 +11,13 @@ import { createAuditLog } from '../services/audit.service.js'; */ export async function getUpcomingBirthdays(req: AuthRequest, res: Response) { try { + // Portal-Kunden haben hier nichts zu suchen. Endpoint listet Namen, E-Mail, + // Telefon und Geburtsdatum ALLER Kunden – ausschließlich Mitarbeiter-UI. + // Pentest Runde 6 (2026-05-16) – HOCH. + if (req.user?.isCustomerPortal) { + res.status(403).json({ success: false, error: 'Nicht erlaubt' }); + return; + } const past = req.query.past ? parseInt(String(req.query.past)) : 7; const future = req.query.future ? parseInt(String(req.query.future)) : 30; diff --git a/backend/src/controllers/cachedEmail.controller.ts b/backend/src/controllers/cachedEmail.controller.ts index c41d479f..37b5e8fc 100644 --- a/backend/src/controllers/cachedEmail.controller.ts +++ b/backend/src/controllers/cachedEmail.controller.ts @@ -9,7 +9,7 @@ import { fetchAttachment, appendToSent, ImapCredentials } from '../services/imap import { getImapSmtpSettings } from '../services/emailProvider/emailProviderService.js'; import { decrypt } from '../utils/encryption.js'; import { logChange } from '../services/audit.service.js'; -import { ApiResponse } from '../types/index.js'; +import { ApiResponse, AuthRequest } from '../types/index.js'; import { getCustomerTargets, getContractTargets, getIdentityDocumentTargets, getBankCardTargets, documentTargets } from '../config/documentTargets.config.js'; import { generateEmailPdf } from '../services/pdfService.js'; import { maybeActivateOnDeliveryConfirmation } from '../services/contractStatusScheduler.service.js'; @@ -17,7 +17,6 @@ import { DocumentType } from '@prisma/client'; import prisma from '../lib/prisma.js'; import path from 'path'; import fs from 'fs'; -import { AuthRequest } from '../types/index.js'; import { canAccessCustomer, canAccessContract, @@ -139,9 +138,10 @@ export async function getEmail(req: AuthRequest, res: Response): Promise { } // E-Mail als gelesen/ungelesen markieren -export async function markAsRead(req: Request, res: Response): Promise { +export async function markAsRead(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; const { isRead } = req.body; if (isRead) { @@ -161,9 +161,10 @@ export async function markAsRead(req: Request, res: Response): Promise { } // E-Mail Stern umschalten -export async function toggleStar(req: Request, res: Response): Promise { +export async function toggleStar(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; const isStarred = await cachedEmailService.toggleEmailStar(id); res.json({ success: true, data: { isStarred } } as ApiResponse); @@ -179,10 +180,12 @@ export async function toggleStar(req: Request, res: Response): Promise { // ==================== CONTRACT ASSIGNMENT ==================== // E-Mail einem Vertrag zuordnen -export async function assignToContract(req: Request, res: Response): Promise { +export async function assignToContract(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const { contractId } = req.body; + if (!(await canAccessContract(req, res, contractId))) return; const userId = (req as any).userId; // Falls Auth-Middleware userId setzt const email = await cachedEmailService.assignEmailToContract(emailId, contractId, userId); @@ -198,9 +201,10 @@ export async function assignToContract(req: Request, res: Response): Promise { +export async function unassignFromContract(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const email = await cachedEmailService.unassignEmailFromContract(emailId); @@ -233,9 +237,10 @@ export async function getFolderCounts(req: AuthRequest, res: Response): Promise< } // E-Mail-Anzahl pro Ordner für einen Vertrag -export async function getContractFolderCounts(req: Request, res: Response): Promise { +export async function getContractFolderCounts(req: AuthRequest, res: Response): Promise { try { const contractId = parseInt(req.params.contractId); + if (!(await canAccessContract(req, res, contractId))) return; const counts = await cachedEmailService.getFolderCountsForContract(contractId); @@ -611,9 +616,10 @@ export async function downloadAttachment(req: AuthRequest, res: Response): Promi // ==================== MAILBOX ACCOUNTS ==================== // Mailbox-Konten eines Kunden abrufen -export async function getMailboxAccounts(req: Request, res: Response): Promise { +export async function getMailboxAccounts(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const accounts = await cachedEmailService.getMailboxAccountsForCustomer(customerId); @@ -686,9 +692,10 @@ export async function syncMailboxStatus(req: AuthRequest, res: Response): Promis } // E-Mail-Thread abrufen -export async function getThread(req: Request, res: Response): Promise { +export async function getThread(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; const thread = await cachedEmailService.getEmailThread(id); @@ -780,7 +787,7 @@ export async function getMailboxCredentials(req: AuthRequest, res: Response): Pr } // Ungelesene E-Mails zählen -export async function getUnreadCount(req: Request, res: Response): Promise { +export async function getUnreadCount(req: AuthRequest, res: Response): Promise { try { const customerId = req.query.customerId ? parseInt(req.query.customerId as string) : undefined; const contractId = req.query.contractId ? parseInt(req.query.contractId as string) : undefined; @@ -788,8 +795,10 @@ export async function getUnreadCount(req: Request, res: Response): Promise let count = 0; if (customerId) { + if (!(await canAccessCustomer(req, res, customerId))) return; count = await cachedEmailService.getUnreadCountForCustomer(customerId); } else if (contractId) { + if (!(await canAccessContract(req, res, contractId))) return; count = await cachedEmailService.getUnreadCountForContract(contractId); } @@ -804,9 +813,10 @@ export async function getUnreadCount(req: Request, res: Response): Promise } // E-Mail in Papierkorb verschieben (nur Admin) -export async function deleteEmail(req: Request, res: Response): Promise { +export async function deleteEmail(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; // Prüfen ob E-Mail existiert const email = await cachedEmailService.getCachedEmailById(id); @@ -841,9 +851,10 @@ export async function deleteEmail(req: Request, res: Response): Promise { // ==================== TRASH OPERATIONS ==================== // Papierkorb-E-Mails für einen Kunden abrufen -export async function getTrashEmails(req: Request, res: Response): Promise { +export async function getTrashEmails(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const emails = await cachedEmailService.getTrashEmails(customerId); @@ -858,9 +869,10 @@ export async function getTrashEmails(req: Request, res: Response): Promise } // Papierkorb-Anzahl für einen Kunden -export async function getTrashCount(req: Request, res: Response): Promise { +export async function getTrashCount(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const count = await cachedEmailService.getTrashCount(customerId); @@ -875,9 +887,10 @@ export async function getTrashCount(req: Request, res: Response): Promise } // E-Mail aus Papierkorb wiederherstellen -export async function restoreEmail(req: Request, res: Response): Promise { +export async function restoreEmail(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; const result = await cachedEmailService.restoreEmailFromTrash(id); @@ -900,9 +913,10 @@ export async function restoreEmail(req: Request, res: Response): Promise { } // E-Mail endgültig löschen (aus Papierkorb) -export async function permanentDeleteEmail(req: Request, res: Response): Promise { +export async function permanentDeleteEmail(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; const result = await cachedEmailService.permanentDeleteEmail(id); @@ -927,9 +941,10 @@ export async function permanentDeleteEmail(req: Request, res: Response): Promise // ==================== ATTACHMENT TARGETS ==================== // Verfügbare Dokumenten-Ziele für E-Mail-Anhänge abrufen -export async function getAttachmentTargets(req: Request, res: Response): Promise { +export async function getAttachmentTargets(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; // E-Mail mit StressfreiEmail laden const email = await cachedEmailService.getCachedEmailById(emailId); @@ -1109,9 +1124,10 @@ export async function getAttachmentTargets(req: Request, res: Response): Promise } // E-Mail-Anhang in ein Dokumentenfeld speichern -export async function saveAttachmentTo(req: Request, res: Response): Promise { +export async function saveAttachmentTo(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const filename = decodeURIComponent(req.params.filename); const { entityType, entityId, targetKey } = req.body; @@ -1396,9 +1412,10 @@ export async function saveAttachmentTo(req: Request, res: Response): Promise { +export async function saveEmailAsPdf(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const { entityType, entityId, targetKey } = req.body; console.log('[saveEmailAsPdf] Request:', { emailId, entityType, entityId, targetKey }); @@ -1643,9 +1660,10 @@ export async function saveEmailAsPdf(req: Request, res: Response): Promise // ==================== SAVE EMAIL AS INVOICE ==================== // E-Mail als PDF exportieren und als Rechnung speichern -export async function saveEmailAsInvoice(req: Request, res: Response): Promise { +export async function saveEmailAsInvoice(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const { invoiceDate, invoiceType, notes } = req.body; console.log('[saveEmailAsInvoice] Request:', { emailId, invoiceDate, invoiceType, notes }); @@ -1769,9 +1787,10 @@ export async function saveEmailAsInvoice(req: Request, res: Response): Promise { +export async function saveAttachmentAsInvoice(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const filename = decodeURIComponent(req.params.filename); const { invoiceDate, invoiceType, notes } = req.body; @@ -1932,9 +1951,10 @@ export async function saveAttachmentAsInvoice(req: Request, res: Response): Prom * Anhang einer E-Mail als Vertragsdokument (ContractDocument) speichern. * Nutzt die flexible ContractDocument-Tabelle mit documentType (Auftragsformular, Lieferbestätigung, etc.) */ -export async function saveAttachmentAsContractDocument(req: Request, res: Response): Promise { +export async function saveAttachmentAsContractDocument(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, emailId))) return; const filename = decodeURIComponent(req.params.filename); const { documentType, notes } = req.body; diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index ac429f51..78e2d9c1 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -211,6 +211,7 @@ export async function deleteContract(req: Request, res: Response): Promise export async function createFollowUp(req: AuthRequest, res: Response): Promise { try { const previousContractId = parseInt(req.params.id); + if (!(await canAccessContract(req, res, previousContractId))) return; // Vorgängervertrag laden für Vertragsnummer const previousContract = await prisma.contract.findUnique({ @@ -264,6 +265,7 @@ export async function createFollowUp(req: AuthRequest, res: Response): Promise { try { const previousContractId = parseInt(req.params.id); + if (!(await canAccessContract(req, res, previousContractId))) return; const previousContract = await prisma.contract.findUnique({ where: { id: previousContractId }, @@ -526,6 +528,7 @@ export async function removeContractMeter(req: AuthRequest, res: Response): Prom try { const contractMeterId = parseInt(req.params.contractMeterId); const contractId = parseInt(req.params.id); + if (!(await canAccessContract(req, res, contractId))) return; await prisma.contractMeter.delete({ where: { id: contractMeterId } }); await logChange({ req, action: 'DELETE', resourceType: 'ContractMeter', @@ -649,9 +652,10 @@ export async function deleteContractDocument(req: AuthRequest, res: Response): P // ==================== SNOOZE (VERTRAG ZURÜCKSTELLEN) ==================== -export async function snoozeContract(req: Request, res: Response): Promise { +export async function snoozeContract(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessContract(req, res, id))) return; const { nextReviewDate, months } = req.body; let reviewDate: Date | null = null; diff --git a/backend/src/controllers/contractHistory.controller.ts b/backend/src/controllers/contractHistory.controller.ts index 85755b18..a80b1e5c 100644 --- a/backend/src/controllers/contractHistory.controller.ts +++ b/backend/src/controllers/contractHistory.controller.ts @@ -2,10 +2,12 @@ import { Request, Response } from 'express'; import * as contractHistoryService from '../services/contractHistory.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; +import { canAccessContract } from '../utils/accessControl.js'; export async function getHistoryEntries(req: AuthRequest, res: Response): Promise { try { const contractId = parseInt(req.params.contractId); + if (!(await canAccessContract(req, res, contractId))) return; const entries = await contractHistoryService.getHistoryEntries(contractId); res.json({ success: true, data: entries } as ApiResponse); } catch (error) { @@ -19,6 +21,7 @@ export async function getHistoryEntries(req: AuthRequest, res: Response): Promis export async function createHistoryEntry(req: AuthRequest, res: Response): Promise { try { const contractId = parseInt(req.params.contractId); + if (!(await canAccessContract(req, res, contractId))) return; const { title, description } = req.body; if (!title || typeof title !== 'string' || title.trim().length === 0) { @@ -54,6 +57,7 @@ export async function createHistoryEntry(req: AuthRequest, res: Response): Promi export async function updateHistoryEntry(req: AuthRequest, res: Response): Promise { try { const contractId = parseInt(req.params.contractId); + if (!(await canAccessContract(req, res, contractId))) return; const entryId = parseInt(req.params.entryId); const { title, description } = req.body; @@ -80,6 +84,7 @@ export async function updateHistoryEntry(req: AuthRequest, res: Response): Promi export async function deleteHistoryEntry(req: AuthRequest, res: Response): Promise { try { const contractId = parseInt(req.params.contractId); + if (!(await canAccessContract(req, res, contractId))) return; const entryId = parseInt(req.params.entryId); await contractHistoryService.deleteHistoryEntry(contractId, entryId); diff --git a/backend/src/controllers/contractTask.controller.ts b/backend/src/controllers/contractTask.controller.ts index ad8f5987..1718e63a 100644 --- a/backend/src/controllers/contractTask.controller.ts +++ b/backend/src/controllers/contractTask.controller.ts @@ -5,6 +5,7 @@ import * as customerService from '../services/customer.service.js'; import * as appSettingService from '../services/appSetting.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; +import { canAccessContract, getPortalAllowedCustomerIds } from '../utils/accessControl.js'; // ==================== ALL TASKS (Dashboard & Task List) ==================== @@ -12,12 +13,13 @@ export async function getAllTasks(req: AuthRequest, res: Response): Promise c.portalEmail) @@ -42,12 +44,13 @@ export async function getAllTasks(req: AuthRequest, res: Response): Promise { try { - // Für Kundenportal: Filter auf erlaubte Kunden + // Für Kundenportal: Filter auf erlaubte Kunden (mit Live-Vollmacht-Check) + const allowedIds = await getPortalAllowedCustomerIds(req); let customerPortalCustomerIds: number[] | undefined; let customerPortalEmails: string[] | undefined; - if (req.user?.isCustomerPortal && req.user.customerId) { - customerPortalCustomerIds = [req.user.customerId, ...(req.user.representedCustomerIds || [])]; + if (allowedIds) { + customerPortalCustomerIds = allowedIds; const customers = await customerService.getCustomersByIds(customerPortalCustomerIds); customerPortalEmails = customers .map((c: { id: number; portalEmail: string | null }) => c.portalEmail) @@ -75,33 +78,17 @@ export async function getTasks(req: AuthRequest, res: Response): Promise { const contractId = parseInt(req.params.contractId); const { status } = req.query; - // Prüfe Zugriff auf den Vertrag - const contract = await contractService.getContractById(contractId); - if (!contract) { - res.status(404).json({ - success: false, - error: 'Vertrag nicht gefunden', - } as ApiResponse); - return; - } + // Zentraler canAccessContract-Check inkl. Live-Vollmacht-Prüfung über + // hasAuthorization (Pentest Runde 6 – HOCH-04: widerrufene Vollmachten + // hatten vorher weiter Zugriff, weil nur representedCustomerIds-Array + // konsultiert wurde, ohne Status-Check). + if (!(await canAccessContract(req, res, contractId))) return; - // Für Kundenportal: Zugriffsprüfung - if (req.user?.isCustomerPortal && req.user.customerId) { - const allowedCustomerIds = [req.user.customerId, ...(req.user.representedCustomerIds || [])]; - if (!allowedCustomerIds.includes(contract.customerId)) { - res.status(403).json({ - success: false, - error: 'Kein Zugriff auf diesen Vertrag', - } as ApiResponse); - return; - } - } - - // Für Kundenportal-Benutzer: Lade E-Mails der erlaubten Kunden + // Für Kundenportal-Benutzer: Lade E-Mails der erlaubten Kunden (mit Live-Vollmacht-Check) let customerPortalEmails: string[] | undefined; - if (req.user?.isCustomerPortal && req.user.customerId) { - const allowedCustomerIds = [req.user.customerId, ...(req.user.representedCustomerIds || [])]; - const customers = await customerService.getCustomersByIds(allowedCustomerIds); + const allowedIds = await getPortalAllowedCustomerIds(req); + if (allowedIds) { + const customers = await customerService.getCustomersByIds(allowedIds); customerPortalEmails = customers .map((c: { id: number; portalEmail: string | null }) => c.portalEmail) .filter((email: string | null): email is string => !!email); @@ -187,27 +174,8 @@ export async function createSupportTicket(req: AuthRequest, res: Response): Prom return; } - // Prüfe Zugriff auf den Vertrag - const contract = await contractService.getContractById(contractId); - if (!contract) { - res.status(404).json({ - success: false, - error: 'Vertrag nicht gefunden', - } as ApiResponse); - return; - } - - // Zugriffsprüfung für Kundenportal - if (req.user?.customerId) { - const allowedCustomerIds = [req.user.customerId, ...(req.user.representedCustomerIds || [])]; - if (!allowedCustomerIds.includes(contract.customerId)) { - res.status(403).json({ - success: false, - error: 'Kein Zugriff auf diesen Vertrag', - } as ApiResponse); - return; - } - } + // canAccessContract inkl. Live-Vollmacht-Prüfung (siehe getTasks). + if (!(await canAccessContract(req, res, contractId))) return; const createdBy = req.user?.email; @@ -376,24 +344,7 @@ export async function createCustomerReply(req: AuthRequest, res: Response): Prom return; } - // Prüfe ob der Kunde berechtigt ist (eigenes Ticket oder freigegebener Kunde) - if (req.user?.isCustomerPortal && req.user.customerId) { - const allowedCustomerIds = [req.user.customerId, ...(req.user.representedCustomerIds || [])]; - const customers = await customerService.getCustomersByIds(allowedCustomerIds); - const allowedEmails = customers - .map((c: { id: number; portalEmail: string | null }) => c.portalEmail) - .filter((email: string | null): email is string => !!email); - - // Task muss entweder visibleInPortal sein ODER vom Kunden erstellt worden sein - const isOwnTask = task.createdBy && allowedEmails.includes(task.createdBy); - if (!task.visibleInPortal && !isOwnTask) { - res.status(403).json({ - success: false, - error: 'Kein Zugriff auf diese Anfrage', - } as ApiResponse); - return; - } - } else { + if (!req.user?.isCustomerPortal || !req.user.customerId) { res.status(403).json({ success: false, error: 'Nur für Kundenportal-Benutzer', @@ -401,6 +352,27 @@ export async function createCustomerReply(req: AuthRequest, res: Response): Prom return; } + // Strikter Owner-Check über den Vertrag (mit Live-Vollmacht-Prüfung + // via hasAuthorization, Pentest Runde 6 – HOCH-04). Damit kann ein + // Portal-User keine fremde Task-ID mit visibleInPortal=true abgreifen. + if (!(await canAccessContract(req, res, task.contractId))) return; + + // Zusätzlich: portal-User darf nur antworten, wenn die Task von ihm + // initiiert wurde ODER explizit für ihn sichtbar markiert ist. + const allowedCustomerIds = [req.user.customerId, ...(req.user.representedCustomerIds || [])]; + const customers = await customerService.getCustomersByIds(allowedCustomerIds); + const allowedEmails = customers + .map((c: { id: number; portalEmail: string | null }) => c.portalEmail) + .filter((email: string | null): email is string => !!email); + const isOwnTask = task.createdBy && allowedEmails.includes(task.createdBy); + if (!task.visibleInPortal && !isOwnTask) { + res.status(403).json({ + success: false, + error: 'Kein Zugriff auf diese Anfrage', + } as ApiResponse); + return; + } + const createdBy = req.user?.email; const subtask = await contractTaskService.createSubtask({ diff --git a/backend/src/controllers/customer.controller.ts b/backend/src/controllers/customer.controller.ts index 8d469340..1291deb0 100644 --- a/backend/src/controllers/customer.controller.ts +++ b/backend/src/controllers/customer.controller.ts @@ -18,30 +18,28 @@ import { canAccessBankCard, canAccessIdentityDocument, canAccessCustomer, + getPortalAllowedCustomerIds, } from '../utils/accessControl.js'; // Customer CRUD export async function getCustomers(req: AuthRequest, res: Response): Promise { try { const { search, type, page, limit } = req.query; + + // Portal-User dürfen nur ihre eigenen + vertretene Kunden (mit aktiver + // Vollmacht) sehen. Wir geben die Liste direkt als DB-Filter mit, damit + // auch `pagination.total` nur über diese IDs zählt (Pentest Runde 6 + // MITTEL-02: `total: 4271` leakte vorher die globale Kunden-Zahl). + const allowedIds = await getPortalAllowedCustomerIds(req); + const result = await customerService.getAllCustomers({ search: search as string, type: type as 'PRIVATE' | 'BUSINESS', page: page ? parseInt(page as string) : undefined, limit: limit ? parseInt(limit as string) : undefined, + allowedIds: allowedIds ?? undefined, }); - let customers = result.customers as any[]; - - // Portal-Kunden: Liste auf eigenen + vertretene Kunden einschränken. - // Ohne diesen Filter würde der List-Endpoint die komplette Kundendatenbank - // an einen einzelnen Portal-Account preisgeben. - if (req.user?.isCustomerPortal) { - const allowedIds = new Set(); - if (req.user.customerId) allowedIds.add(req.user.customerId); - const represented = (req.user as any).representedCustomerIds || []; - for (const id of represented) allowedIds.add(id); - customers = customers.filter((c) => allowedIds.has(c.id)); - } + const customers = result.customers as any[]; // Portal-Kunden oder andere Rollen sehen kein portalPasswordEncrypted const canSeePasswords = req.user?.permissions?.includes('customers:update') ?? false; @@ -233,9 +231,10 @@ export async function createAddress(req: AuthRequest, res: Response): Promise { +export async function updateAddress(req: AuthRequest, res: Response): Promise { try { const addressId = parseInt(req.params.id); + if (!(await canAccessAddress(req, res, addressId))) return; const data = req.body; // Vorherigen Stand laden für Audit @@ -296,9 +295,10 @@ export async function updateAddress(req: Request, res: Response): Promise } } -export async function deleteAddress(req: Request, res: Response): Promise { +export async function deleteAddress(req: AuthRequest, res: Response): Promise { try { const addressId = parseInt(req.params.id); + if (!(await canAccessAddress(req, res, addressId))) return; const addr = await prisma.address.findUnique({ where: { id: addressId }, select: { customerId: true } }); const customerId = addr?.customerId; await customerService.deleteAddress(addressId); @@ -350,9 +350,10 @@ export async function createBankCard(req: AuthRequest, res: Response): Promise { +export async function updateBankCard(req: AuthRequest, res: Response): Promise { try { const cardId = parseInt(req.params.id); + if (!(await canAccessBankCard(req, res, cardId))) return; const data = req.body; // Vorherigen Stand laden für Audit @@ -408,9 +409,10 @@ export async function updateBankCard(req: Request, res: Response): Promise } } -export async function deleteBankCard(req: Request, res: Response): Promise { +export async function deleteBankCard(req: AuthRequest, res: Response): Promise { try { const cardId = parseInt(req.params.id); + if (!(await canAccessBankCard(req, res, cardId))) return; const card = await prisma.bankCard.findUnique({ where: { id: cardId }, select: { customerId: true } }); const customerId = card?.customerId; await customerService.deleteBankCard(cardId); @@ -462,9 +464,10 @@ export async function createDocument(req: AuthRequest, res: Response): Promise { +export async function updateDocument(req: AuthRequest, res: Response): Promise { try { const docId = parseInt(req.params.id); + if (!(await canAccessIdentityDocument(req, res, docId))) return; const data = req.body; // Vorherigen Stand laden für Audit @@ -526,9 +529,10 @@ export async function updateDocument(req: Request, res: Response): Promise } } -export async function deleteDocument(req: Request, res: Response): Promise { +export async function deleteDocument(req: AuthRequest, res: Response): Promise { try { const docId = parseInt(req.params.id); + if (!(await canAccessIdentityDocument(req, res, docId))) return; const doc = await prisma.identityDocument.findUnique({ where: { id: docId }, select: { customerId: true } }); const customerId = doc?.customerId; await customerService.deleteDocument(docId); @@ -580,9 +584,10 @@ export async function createMeter(req: AuthRequest, res: Response): Promise { +export async function updateMeter(req: AuthRequest, res: Response): Promise { try { const meterId = parseInt(req.params.id); + if (!(await canAccessMeter(req, res, meterId))) return; const data = req.body; // Vorherigen Stand laden für Audit @@ -637,9 +642,10 @@ export async function updateMeter(req: Request, res: Response): Promise { } } -export async function deleteMeter(req: Request, res: Response): Promise { +export async function deleteMeter(req: AuthRequest, res: Response): Promise { try { const meterId = parseInt(req.params.id); + if (!(await canAccessMeter(req, res, meterId))) return; await customerService.deleteMeter(meterId); await logChange({ req, action: 'DELETE', resourceType: 'Meter', @@ -667,10 +673,11 @@ export async function getMeterReadings(req: AuthRequest, res: Response): Promise } } -export async function addMeterReading(req: Request, res: Response): Promise { +export async function addMeterReading(req: AuthRequest, res: Response): Promise { try { const { readingDate, value, valueNt, unit, notes } = req.body; const meterId = parseInt(req.params.meterId); + if (!(await canAccessMeter(req, res, meterId))) return; const reading = await customerService.addMeterReading(meterId, { readingDate: new Date(readingDate), value: parseFloat(value), @@ -703,8 +710,10 @@ export async function addMeterReading(req: Request, res: Response): Promise { +export async function updateMeterReading(req: AuthRequest, res: Response): Promise { try { + const meterId = parseInt(req.params.meterId); + if (!(await canAccessMeter(req, res, meterId))) return; const { readingDate, value, valueNt, unit, notes } = req.body; const updateData: Record = {}; if (readingDate !== undefined) updateData.readingDate = new Date(readingDate); @@ -714,7 +723,7 @@ export async function updateMeterReading(req: Request, res: Response): Promise { +export async function deleteMeterReading(req: AuthRequest, res: Response): Promise { try { + const meterId = parseInt(req.params.meterId); + if (!(await canAccessMeter(req, res, meterId))) return; const readingId = parseInt(req.params.readingId); - await customerService.deleteMeterReading( - parseInt(req.params.meterId), - readingId - ); + await customerService.deleteMeterReading(meterId, readingId); await logChange({ req, action: 'DELETE', resourceType: 'MeterReading', resourceId: readingId.toString(), @@ -839,6 +847,7 @@ export async function getMyMeters(req: AuthRequest, res: Response): Promise { try { const meterId = parseInt(req.params.meterId); + if (!(await canAccessMeter(req, res, meterId))) return; const readingId = parseInt(req.params.readingId); const reading = await prisma.meterReading.update({ @@ -1128,9 +1137,10 @@ export async function getRepresentatives(req: AuthRequest, res: Response): Promi } } -export async function addRepresentative(req: Request, res: Response): Promise { +export async function addRepresentative(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const { representativeId, notes } = req.body; const representative = await customerService.addRepresentative( customerId, @@ -1152,9 +1162,10 @@ export async function addRepresentative(req: Request, res: Response): Promise { +export async function removeRepresentative(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; await customerService.removeRepresentative( customerId, parseInt(req.params.representativeId) @@ -1173,8 +1184,13 @@ export async function removeRepresentative(req: Request, res: Response): Promise } } -export async function searchForRepresentative(req: Request, res: Response): Promise { +export async function searchForRepresentative(req: AuthRequest, res: Response): Promise { try { + // KRITISCH (Pentest Runde 6): ohne canAccessCustomer kann ein Portal-User + // mit beliebigem :customerId-Pfad alle Kunden durchsuchen → komplette + // Kunden-DB-Enumeration via Buchstaben-Brute-Force. + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const { search } = req.query; if (!search || typeof search !== 'string' || search.length < 2) { res.json({ success: true, data: [] } as ApiResponse); @@ -1182,7 +1198,7 @@ export async function searchForRepresentative(req: Request, res: Response): Prom } const customers = await customerService.searchCustomersForRepresentative( search, - parseInt(req.params.customerId) + customerId, ); res.json({ success: true, data: customers } as ApiResponse); } catch (error) { diff --git a/backend/src/controllers/gdpr.controller.ts b/backend/src/controllers/gdpr.controller.ts index 8712b19c..dda65d2f 100644 --- a/backend/src/controllers/gdpr.controller.ts +++ b/backend/src/controllers/gdpr.controller.ts @@ -279,17 +279,9 @@ export async function updateCustomerConsent(req: AuthRequest, res: Response) { }); } - // Portal: nur eigene + vertretene Kunden - const allowed = [ - (req.user as any).customerId, - ...((req.user as any).representedCustomerIds || []), - ]; - if (!allowed.includes(customerId)) { - return res.status(403).json({ - success: false, - error: 'Keine Berechtigung für diesen Kunden', - }); - } + // canAccessCustomer inkl. Live-Vollmacht-Check (Pentest Runde 6 HOCH-04: + // widerrufene Vollmachten hatten vorher noch Zugriff) + if (!(await canAccessCustomer(req, res, customerId))) return; if (!Object.values(ConsentType).includes(consentType)) { return res.status(400).json({ success: false, error: 'Ungültiger Consent-Typ' }); diff --git a/backend/src/services/auth.service.ts b/backend/src/services/auth.service.ts index c247be66..693b78df 100644 --- a/backend/src/services/auth.service.ts +++ b/backend/src/services/auth.service.ts @@ -782,11 +782,18 @@ export async function confirmPasswordReset(token: string, newPassword: string): where: { id: customer.id }, data: { portalPasswordHash: hash, - portalPasswordEncrypted: encrypt(newPassword), + // Pentest Runde 6 (MITTEL-01): Beim Self-Service-Reset speichern wir + // KEINEN Klartext mehr. Encrypted-Feld ist nur für Admin-generierte + // Einmalpasswörter sinnvoll (damit Admin sie in der UI sehen + per + // Mail versenden kann); für ein vom Kunden selbst gesetztes Passwort + // ist Klartext-Speicherung ein unnötiges Recover-Risiko bei DB+Key-Leak. + portalPasswordEncrypted: null, portalPasswordResetToken: null, portalPasswordResetExpiresAt: null, // Alle bestehenden Portal-Sessions kicken portalTokenInvalidatedAt: new Date(), + // OTP-Flow-Flag ist nach selbstgesetztem Passwort definitiv aus + portalPasswordMustChange: false, }, }); return; diff --git a/backend/src/services/customer.service.ts b/backend/src/services/customer.service.ts index 755abf7b..c722253e 100644 --- a/backend/src/services/customer.service.ts +++ b/backend/src/services/customer.service.ts @@ -22,10 +22,13 @@ export interface CustomerFilters { type?: CustomerType; page?: number; limit?: number; + // Wenn gesetzt: nur Customer mit id in dieser Liste. Für Portal-User, damit + // weder Liste noch pagination.total die globale Kunden-Zahl preisgibt. + allowedIds?: number[]; } export async function getAllCustomers(filters: CustomerFilters) { - const { search, type, page = 1, limit = 20 } = filters; + const { search, type, page = 1, limit = 20, allowedIds } = filters; const { skip, take } = paginate(page, limit); const where: Record = {}; @@ -34,6 +37,10 @@ export async function getAllCustomers(filters: CustomerFilters) { where.type = type; } + if (allowedIds) { + where.id = { in: allowedIds }; + } + if (search) { where.OR = [ { firstName: { contains: search } }, diff --git a/backend/src/utils/accessControl.ts b/backend/src/utils/accessControl.ts index 725607ff..1a39d921 100644 --- a/backend/src/utils/accessControl.ts +++ b/backend/src/utils/accessControl.ts @@ -130,6 +130,34 @@ export async function canAccessCustomer( return true; } +/** + * Liefert die Liste aller Customer-IDs, auf die ein Portal-User aktuell + * Zugriff hat: eigene + vertretene MIT aktiver Vollmacht (Live-Check via + * `authorizationService.hasAuthorization`). Für Nicht-Portal-User wird + * `null` zurückgegeben (= kein Filter, alle Kunden erlaubt). + * + * Diese Funktion fängt einen wiederkehrenden Pentest-Befund ab: ohne den + * Live-Vollmacht-Check hätte ein Portal-User mit widerrufener Vollmacht + * weiterhin Zugriff auf die Daten des vertretenen Kunden, nur weil seine + * `representedCustomerIds` im JWT noch drin sind (Token kann bis zu + * 15min alt sein). + */ +export async function getPortalAllowedCustomerIds( + req: AuthRequest, +): Promise { + if (!req.user?.isCustomerPortal || !req.user.customerId) return null; + const allowed: number[] = [req.user.customerId]; + const represented: number[] = (req.user as any).representedCustomerIds || []; + for (const repCustId of represented) { + const hasAuth = await authorizationService.hasAuthorization( + repCustId, + req.user.customerId, + ); + if (hasAuth) allowed.push(repCustId); + } + return allowed; +} + /** * Generische Zugriffsprüfung: Ressource → customerId → canAccessCustomer. */ diff --git a/docs/todo.md b/docs/todo.md index 5966c4c9..87d9fb29 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,67 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🚨 Pentest Runde 6 – Sammelfix + Strukturelles Audit (8 Findings + Audit-Sweep)** + - **KRITISCH-01 `GET /emails/:id/thread`**: kein Owner-Check → + Portal-Kunde konnte alle Mail-Threads durchsuchen. Fix: + `canAccessCachedEmail` im Controller. + - **KRITISCH-02 `GET /customers/:customerId/representatives/search`**: + kein `canAccessCustomer` auf den Pfad → DSGVO-GAU, Portal-Kunde + konnte mit Buchstaben-Brute-Force die komplette Kunden-DB + auslesen. Fix eingefügt. + - **HOCH-01 `GET /birthdays/upcoming`**: kein Portal-Filter → Name, + E-Mail, Telefon, Geburtsdatum aller Kunden lesbar. Fix: + `isCustomerPortal` → 403. + - **HOCH-02 `*/contracts/:contractId/history`**: kein Owner-Check + auf GET/POST/PUT/DELETE. Fix: `canAccessContract` in allen vier + History-Handlern. + - **HOCH-03 Mailbox-Endpoints**: `mailbox-accounts`, `unread-count`, + `contracts/:id/emails/folder-counts` ohne Check. Fix: + `canAccessCustomer` bzw. `canAccessContract` in allen drei. + - **HOCH-04 Live-Vollmacht-Check in Tasks**: `getTasks`, + `createSupportTicket`, `createCustomerReply`, `getAllTasks`, + `getTaskStats` prüften nur `representedCustomerIds.includes(...)` + aus dem JWT – widerrufene Vollmachten hatten weiter Zugriff + (JWT lebt bis zu 15min nach Widerruf). Neuer Helper + `getPortalAllowedCustomerIds()` in `accessControl.ts` ruft + `hasAuthorization()` live ab. Auch `updateCustomerConsent` + (GDPR) auf diesen Pfad umgestellt. + - **MITTEL-01 `confirmPasswordReset` Klartext-Speicherung**: + Self-Service-Reset speicherte `portalPasswordEncrypted = encrypt(pw)`. + Klartext-Speicherung ist nur für Admin-OTPs sinnvoll. Fix: + Field auf null, zusätzlich `portalPasswordMustChange = false`. + - **MITTEL-02 Pagination-Total leakt globale Kunden-Anzahl**: + `GET /customers` gab `total: 4271` auch wenn Portal-User nur + 1 Kunde sah. Fix: `customer.service.ts` erweitert um + `allowedIds`-Filter, der direkt in der DB-Query landet → die + pagination zählt nur über erlaubte IDs. + - **Strukturelles Audit-Sweep** (Sub-CRUD + Email-Operationen): + Folgende Handler bekamen jetzt erstmals einen `canAccess*`- + Check, defense in depth gegen falsch vergebene Rollen: + `markAsRead`, `toggleStar`, `assignToContract`, + `unassignFromContract`, `deleteEmail`, `getTrashEmails`, + `getTrashCount`, `restoreEmail`, `permanentDeleteEmail`, + `getAttachmentTargets`, `saveAttachmentTo`, `saveEmailAsPdf`, + `saveEmailAsInvoice`, `saveAttachmentAsInvoice`, + `saveAttachmentAsContractDocument`, `createFollowUp`, + `createRenewal`, `snoozeContract`, `removeContractMeter`, + `updateAddress`, `deleteAddress`, `updateBankCard`, + `deleteBankCard`, `updateDocument`, `deleteDocument`, + `updateMeter`, `deleteMeter`, `addMeterReading`, + `updateMeterReading`, `deleteMeterReading`, + `markReadingTransferred`, `addRepresentative`, + `removeRepresentative`. + - **Live-verifiziert** (Portal-User Customer 3 auf fremde IDs): + `customers/1/representatives/search` → 403, + `birthdays/upcoming` → 403 (Admin → 200), + `emails/21/thread` → 403, + `customers/2/mailbox-accounts` → 403, + `emails/unread-count?customerId=2` → 403, + `contracts/8/{history,folder-counts,follow-up,renewal,snooze}` → 403, + eigene `customers/3` → 200, + pagination.total für Portal = 1 (statt 3), + Customer 1 mit widerrufener Vollmacht → 0 fremde Verträge. + - [x] **🚨 Pentest Runde 5 – KRITISCH: change-initial-portal-password ohne Pflicht-Check** - **Realer Angriff**: Jeder Portal-User konnte jederzeit mit seinem eingeloggten Token `POST /api/auth/change-initial-portal-