Security-Hardening Runde 13: Live-Vollmacht-Konsistenz + embedded DTOs
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) <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,7 @@ import { canAccessContract, getPortalAllowedCustomerIds } from '../utils/accessC
|
||||
export async function getAllTasks(req: AuthRequest, res: Response): Promise<void> {
|
||||
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<void
|
||||
let customerPortalEmails: string[] | undefined;
|
||||
|
||||
if (allowedIds) {
|
||||
// Wenn der Portal-User explizit nach einer customerId filtert, die er
|
||||
// nicht (mehr) vertreten darf → 403 statt 200 mit leerem Array
|
||||
// (Pentest Runde 10 – LOW: konsistentes Response-Verhalten nach
|
||||
// Vollmacht-Widerruf).
|
||||
if (customerIdNum !== undefined && !allowedIds.includes(customerIdNum)) {
|
||||
res.status(403).json({ success: false, error: 'Kein Zugriff auf diese Kundendaten' } as ApiResponse);
|
||||
return;
|
||||
}
|
||||
customerPortalCustomerIds = allowedIds;
|
||||
const customers = await customerService.getCustomersByIds(customerPortalCustomerIds);
|
||||
customerPortalEmails = customers
|
||||
@@ -28,7 +37,7 @@ export async function getAllTasks(req: AuthRequest, res: Response): Promise<void
|
||||
|
||||
const tasks = await contractTaskService.getAllTasks({
|
||||
status: status as 'OPEN' | 'COMPLETED' | undefined,
|
||||
customerId: customerId ? parseInt(customerId as string) : undefined,
|
||||
customerId: customerIdNum,
|
||||
customerPortalCustomerIds,
|
||||
customerPortalEmails,
|
||||
});
|
||||
|
||||
@@ -6,6 +6,7 @@ import { JwtPayload } from '../types/index.js';
|
||||
import { encrypt, decrypt } from '../utils/encryption.js';
|
||||
import { sendEmail, SmtpCredentials } from './smtpService.js';
|
||||
import { getSystemEmailCredentials } from './emailProvider/emailProviderService.js';
|
||||
import { getAuthorizedCustomerIds } from './authorization.service.js';
|
||||
|
||||
// Token-Lifetimes
|
||||
// - Access-Token: kurzlebig, nur im Browser-Memory → XSS klaut max. 15 min
|
||||
@@ -216,10 +217,17 @@ export async function customerLogin(email: string, password: string) {
|
||||
});
|
||||
}
|
||||
|
||||
// IDs der Kunden sammeln, die dieser Kunde vertreten kann
|
||||
const representedCustomerIds = customer.representingFor.map(
|
||||
(rep) => 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,
|
||||
|
||||
@@ -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<T extends Record<string, unknown>>(customer: T | null): T | null {
|
||||
if (!customer) return customer;
|
||||
const copy = { ...customer };
|
||||
const copy: Record<string, unknown> = { ...customer };
|
||||
for (const field of SENSITIVE_CUSTOMER_FIELDS) {
|
||||
delete copy[field];
|
||||
}
|
||||
if (Array.isArray(copy.contracts)) {
|
||||
copy.contracts = (copy.contracts as Record<string, unknown>[]).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<T extends Record<string, unknown>>(customer: T | null): T | null {
|
||||
if (!customer) return customer;
|
||||
@@ -86,6 +94,9 @@ export function sanitizeCustomerStrict<T extends Record<string, unknown>>(custom
|
||||
for (const field of PORTAL_HIDDEN_CUSTOMER_FIELDS) {
|
||||
delete copy[field];
|
||||
}
|
||||
if (Array.isArray(copy.contracts)) {
|
||||
copy.contracts = (copy.contracts as Record<string, unknown>[]).map((c) => sanitizeContractStrict(c));
|
||||
}
|
||||
return copy as T;
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user