Add AI review results - 2026-05-18 04:53:48
This commit is contained in:
@@ -0,0 +1,350 @@
|
||||
# 1. Zusammenfassung
|
||||
|
||||
Claude benennt mehrere valide Architektur-Risiken. Übernommen werden vor allem die Punkte, die direkt sicherheits-, compliance- oder konsistenzrelevant sind:
|
||||
|
||||
- Tenant-Isolation muss technisch erzwungen werden.
|
||||
- Secrets dürfen nicht direkt in Fachmodellen gespeichert werden.
|
||||
- Superadmin-Zugriffe brauchen Schutzmechanismen.
|
||||
- Kritische Statuswechsel brauchen State Machines und Historisierung.
|
||||
- Retry/DLQ-Verhalten muss begrenzt und beobachtbar sein.
|
||||
- Domänensprache und Modulgrenzen müssen vereinheitlicht werden.
|
||||
|
||||
Nicht übernommen wird die Empfehlung „zurück ans Reißbrett“. Stattdessen werden konkrete Projektdateien ergänzt/geändert.
|
||||
|
||||
---
|
||||
|
||||
# 2. Übernommene Punkte
|
||||
|
||||
## 2.1 Tenant-Isolation
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- PostgreSQL Row Level Security für tenant-bezogene Tabellen.
|
||||
- Tenant-Kontext zentral auf Request-/Job-Ebene.
|
||||
- Automatischer Model-Scope zusätzlich zu RLS.
|
||||
- Tests gegen Cross-Tenant-Zugriffe.
|
||||
|
||||
## 2.2 Secrets-Management
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Keine API-Keys, Passwörter oder Auth-Codes direkt in Business-Tabellen.
|
||||
- Speicherung nur als Secret-Referenz.
|
||||
- Verschlüsselung at-rest über Secret-Backend.
|
||||
- Rotation und Zugriffsaudit dokumentieren.
|
||||
|
||||
## 2.3 Audit-Log
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Hash-Chain allein reicht nicht als Revisionssicherheit.
|
||||
- Audit-Log als append-only behandeln.
|
||||
- Export/Archivierung für Compliance ergänzen.
|
||||
- Keine Aussage „GoBD-konform“ ohne Verfahrensdokumentation.
|
||||
|
||||
## 2.4 Domain-Management
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Domainmodell um produktionsrelevante Attribute erweitern.
|
||||
- Auth-Codes nur als Secret-Referenz.
|
||||
- Status für Transfer-Lock, DNSSEC, Whois-Privacy und Expiry ergänzen.
|
||||
|
||||
## 2.5 Kritische Flows
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Vertragsstatus, Zahlungsstatus, Kündigungen und Domain-Transfers nicht als freies CRUD.
|
||||
- State Machines einführen.
|
||||
- Statusübergänge historisieren.
|
||||
|
||||
## 2.6 Adapter-Resilienz
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Circuit Breaker ergänzen.
|
||||
- Health Checks je Adapter.
|
||||
- Retry-Limits, Backoff-Caps und DLQ-Verhalten definieren.
|
||||
|
||||
## 2.7 Domänensprache
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Einheitliche Begriffe festlegen.
|
||||
- `tenant_id`, `organisation_id`, `customer_id`, `kunde` nicht vermischen.
|
||||
- Fachsprache in Datenmodell, Code und API vereinheitlichen.
|
||||
|
||||
## 2.8 Versionierung von Produkten/Verträgen
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Preise, Produktbedingungen und Vertragsstände versionieren.
|
||||
- Rechnungen müssen reproduzierbar bleiben.
|
||||
|
||||
## 2.9 Superadmin
|
||||
|
||||
Übernehmen:
|
||||
|
||||
- Superadmin-Zugriffe begrenzen.
|
||||
- Break-Glass-Prozess dokumentieren.
|
||||
- Kritische Aktionen auditieren.
|
||||
- Optional 4-Augen-Freigabe für besonders kritische Aktionen.
|
||||
|
||||
---
|
||||
|
||||
# 3. Abgelehnte oder verschobene Punkte
|
||||
|
||||
## Abgelehnt
|
||||
|
||||
- **„Zurück ans Reißbrett“**
|
||||
Nicht notwendig. Die Kritikpunkte werden als konkrete Architektur- und Implementierungsänderungen eingearbeitet.
|
||||
|
||||
- **Vollständiges Event Sourcing für alle kritischen Flows sofort**
|
||||
Zu groß für den nächsten Schritt. Stattdessen: State Machines, Statushistorien, Audit-Events.
|
||||
|
||||
- **Vollständige Microservice-Aufteilung nach Bounded Contexts**
|
||||
Wird nicht sofort umgesetzt. Ziel bleibt zunächst ein modularer Monolith mit klaren Modulgrenzen.
|
||||
|
||||
## Verschoben
|
||||
|
||||
- **Externer Timestamp-Service für Audit-Logs**
|
||||
Später prüfen. Kurzfristig wichtiger: append-only Audit, Export, Archivierung, Berechtigungsschutz.
|
||||
|
||||
- **Vollständiges DNS-Record-Management**
|
||||
Nur umsetzen, wenn das Backoffice tatsächlich DNS-Zonen verwalten soll. Kurzfristig werden Domain-Statusfelder ergänzt.
|
||||
|
||||
- **Formale GoBD-Zertifizierung**
|
||||
Verschoben. Zuerst technische Grundlagen und Verfahrensdokumentation vorbereiten.
|
||||
|
||||
---
|
||||
|
||||
# 4. Neue/geänderte Dateien
|
||||
|
||||
## Architektur / ADRs
|
||||
|
||||
Neu:
|
||||
|
||||
- `docs/adr/0020-tenant-isolation-rls.md`
|
||||
- `docs/adr/0021-secrets-management.md`
|
||||
- `docs/adr/0022-audit-log-and-archiving.md`
|
||||
- `docs/adr/0023-state-machines-for-critical-flows.md`
|
||||
- `docs/adr/0024-adapter-resilience-circuit-breaker.md`
|
||||
- `docs/adr/0025-domain-language.md`
|
||||
- `docs/adr/0026-superadmin-break-glass.md`
|
||||
- `docs/adr/0027-product-contract-versioning.md`
|
||||
|
||||
Ändern:
|
||||
|
||||
- `docs/adr/0017-adapter-resilience.md`
|
||||
|
||||
---
|
||||
|
||||
## Security / Tenancy
|
||||
|
||||
Neu:
|
||||
|
||||
- `app/Support/Tenancy/TenantContext.php`
|
||||
- `app/Http/Middleware/ResolveTenantContext.php`
|
||||
- `app/Models/Concerns/BelongsToTenant.php`
|
||||
- `app/Jobs/Middleware/ResolveTenantForJob.php`
|
||||
- `tests/Feature/Security/TenantIsolationTest.php`
|
||||
- `tests/Feature/Security/TenantContextRequiredTest.php`
|
||||
|
||||
Neu/ändern:
|
||||
|
||||
- `database/migrations/XXXX_enable_rls_for_tenant_tables.sql`
|
||||
- `database/migrations/XXXX_add_tenant_id_indexes.sql`
|
||||
|
||||
---
|
||||
|
||||
## Secrets
|
||||
|
||||
Neu:
|
||||
|
||||
- `config/secrets.php`
|
||||
- `app/Services/Secrets/SecretStore.php`
|
||||
- `app/Services/Secrets/VaultSecretStore.php`
|
||||
- `app/Services/Secrets/DatabaseEncryptedSecretStore.php`
|
||||
- `app/Services/Secrets/SecretReference.php`
|
||||
- `tests/Feature/Security/SecretStorageTest.php`
|
||||
- `docs/security/secrets-management.md`
|
||||
|
||||
Ändern:
|
||||
|
||||
- `app/Models/RegistrarAccount.php`
|
||||
- `app/Models/Domain.php`
|
||||
|
||||
Erwartete Änderung:
|
||||
|
||||
- Keine Spalten wie `api_key`, `password`, `auth_code`.
|
||||
- Stattdessen Spalten wie `secret_reference`, `auth_code_secret_reference`.
|
||||
|
||||
---
|
||||
|
||||
## Audit / Compliance
|
||||
|
||||
Neu:
|
||||
|
||||
- `app/Models/AuditLog.php`
|
||||
- `app/Services/Audit/AuditLogger.php`
|
||||
- `app/Services/Audit/AuditArchiveExporter.php`
|
||||
- `database/migrations/XXXX_create_audit_logs_table.php`
|
||||
- `database/migrations/XXXX_restrict_audit_log_mutations.sql`
|
||||
- `tests/Feature/Audit/AuditLogAppendOnlyTest.php`
|
||||
- `docs/compliance/audit-log.md`
|
||||
- `docs/compliance/gobd-verfahrensdokumentation-template.md`
|
||||
|
||||
---
|
||||
|
||||
## Domain-Management
|
||||
|
||||
Ändern:
|
||||
|
||||
- `app/Models/Domain.php`
|
||||
- `database/migrations/XXXX_extend_domains_table.php`
|
||||
|
||||
Ergänzen:
|
||||
|
||||
- `dnssec_status`
|
||||
- `transfer_lock_enabled`
|
||||
- `whois_privacy_enabled`
|
||||
- `expiry_date`
|
||||
- `expiry_notification_status`
|
||||
- `auth_code_secret_reference`
|
||||
- `registrar_status`
|
||||
- `last_registry_sync_at`
|
||||
|
||||
Neu:
|
||||
|
||||
- `app/Enums/Domain/DnssecStatus.php`
|
||||
- `app/Enums/Domain/RegistrarStatus.php`
|
||||
- `app/Enums/Domain/ExpiryNotificationStatus.php`
|
||||
|
||||
---
|
||||
|
||||
## State Machines / Statushistorie
|
||||
|
||||
Neu:
|
||||
|
||||
- `app/StateMachines/ContractStateMachine.php`
|
||||
- `app/StateMachines/PaymentStateMachine.php`
|
||||
- `app/StateMachines/CancellationStateMachine.php`
|
||||
- `app/StateMachines/DomainTransferStateMachine.php`
|
||||
|
||||
Neu:
|
||||
|
||||
- `database/migrations/XXXX_create_contract_status_history_table.php`
|
||||
- `database/migrations/XXXX_create_payment_status_history_table.php`
|
||||
- `database/migrations/XXXX_create_domain_transfer_status_history_table.php`
|
||||
|
||||
Neu:
|
||||
|
||||
- `tests/Feature/StateMachines/ContractStateMachineTest.php`
|
||||
- `tests/Feature/StateMachines/PaymentStateMachineTest.php`
|
||||
- `tests/Feature/StateMachines/DomainTransferStateMachineTest.php`
|
||||
|
||||
---
|
||||
|
||||
## Adapter / Retry / DLQ
|
||||
|
||||
Neu:
|
||||
|
||||
- `config/adapters.php`
|
||||
- `app/Support/Adapters/CircuitBreaker.php`
|
||||
- `app/Support/Adapters/AdapterHealthCheck.php`
|
||||
- `app/Support/Adapters/RetryPolicy.php`
|
||||
- `app/Support/Adapters/DeadLetterHandler.php`
|
||||
- `app/Console/Commands/CheckAdapterHealthCommand.php`
|
||||
- `tests/Feature/Adapters/CircuitBreakerTest.php`
|
||||
- `tests/Feature/Queue/DeadLetterHandlingTest.php`
|
||||
|
||||
Ändern:
|
||||
|
||||
- `docs/adr/0017-adapter-resilience.md`
|
||||
|
||||
Ergänzen:
|
||||
|
||||
- Max Retry Count
|
||||
- Exponential Backoff Cap
|
||||
- DLQ nach endgültigem Fehlschlag
|
||||
- Alerting-Schwelle
|
||||
- Poison-Message-Handling
|
||||
|
||||
---
|
||||
|
||||
## Domänensprache / Modulgrenzen
|
||||
|
||||
Neu:
|
||||
|
||||
- `docs/domain/glossary.md`
|
||||
- `docs/domain/module-boundaries.md`
|
||||
|
||||
Ändern:
|
||||
|
||||
- `docs/api/openapi.yaml`
|
||||
- betroffene Models, DTOs, Requests und Resources
|
||||
|
||||
Festlegung:
|
||||
|
||||
- Einheitlich verwenden:
|
||||
- `tenant` für Mandant/Systemgrenze
|
||||
- `customer` für Kunde
|
||||
- `contract` für Vertrag
|
||||
- `product` für Produkt
|
||||
- `invoice` für Rechnung
|
||||
|
||||
---
|
||||
|
||||
## Produkt- und Vertragsversionierung
|
||||
|
||||
Neu:
|
||||
|
||||
- `app/Models/ProductVersion.php`
|
||||
- `app/Models/ContractVersion.php`
|
||||
- `database/migrations/XXXX_create_product_versions_table.php`
|
||||
- `database/migrations/XXXX_create_contract_versions_table.php`
|
||||
- `tests/Feature/Billing/ProductVersioningTest.php`
|
||||
- `tests/Feature/Billing/InvoiceReproducibilityTest.php`
|
||||
|
||||
Ändern:
|
||||
|
||||
- `app/Models/Product.php`
|
||||
- `app/Models/Contract.php`
|
||||
- `app/Models/Invoice.php`
|
||||
|
||||
---
|
||||
|
||||
## Superadmin / Break-Glass
|
||||
|
||||
Neu:
|
||||
|
||||
- `app/Policies/SuperadminPolicy.php`
|
||||
- `app/Services/Security/BreakGlassAccessService.php`
|
||||
- `app/Models/PrivilegedAccessGrant.php`
|
||||
- `database/migrations/XXXX_create_privileged_access_grants_table.php`
|
||||
- `tests/Feature/Security/BreakGlassAccessTest.php`
|
||||
- `docs/security/break-glass.md`
|
||||
|
||||
Ergänzen:
|
||||
|
||||
- zeitlich begrenzte Rechte
|
||||
- Begründungspflicht
|
||||
- Audit-Log für jede privilegierte Aktion
|
||||
- optional 4-Augen-Freigabe für definierte Aktionen
|
||||
|
||||
---
|
||||
|
||||
# 5. Konkrete nächste Schritte
|
||||
|
||||
1. `docs/adr/0020-tenant-isolation-rls.md` erstellen und RLS als verbindliche Entscheidung festhalten.
|
||||
2. Tenant-Kontext mit `TenantContext`, Middleware und Model-Concern implementieren.
|
||||
3. RLS-Migration für alle tenant-bezogenen Tabellen ergänzen.
|
||||
4. Cross-Tenant-Sicherheitstests schreiben und in CI verpflichtend machen.
|
||||
5. `docs/adr/0021-secrets-management.md` erstellen.
|
||||
6. Registrar- und Domain-Secrets aus Fachmodellen entfernen und durch Secret-Referenzen ersetzen.
|
||||
7. Adapter-ADR `0017` um Circuit Breaker, Health Checks, Retry-Limits und DLQ-Regeln erweitern.
|
||||
8. State Machines für Vertrag, Zahlung, Kündigung und Domain-Transfer definieren.
|
||||
9. Produkt- und Vertragsversionierung als Tabellen und Tests ergänzen.
|
||||
10. `docs/domain/glossary.md` erstellen und Begriffe im Code vereinheitlichen.
|
||||
11. Break-Glass-Zugriffe für Superadmin modellieren und auditieren.
|
||||
12. Audit-Log append-only absichern und Archivierungsdokumentation ergänzen.
|
||||
@@ -0,0 +1,126 @@
|
||||
## Kritische Architekturprobleme
|
||||
|
||||
### 1. **Fataler Tenant-Isolation-Fehler**
|
||||
|
||||
Das Multi-Tenancy-Konzept ist oberflächlich und gefährlich:
|
||||
|
||||
```sql
|
||||
-- So wird es NICHT funktionieren:
|
||||
SELECT * FROM customers WHERE tenant_id = ?
|
||||
-- Vergessener tenant_id Filter = Datenleak!
|
||||
```
|
||||
|
||||
**Problem:** Keine technische Enforcement-Ebene definiert. Laravel Policies allein reichen nicht - ein vergessener Scope und alle Kundendaten sind exponiert.
|
||||
|
||||
**Lösung erforderlich:**
|
||||
- PostgreSQL Row Level Security (RLS) als Pflicht
|
||||
- Automatische Query-Scopes auf Model-Ebene
|
||||
- Tenant-Context in Session, nicht nur als Parameter
|
||||
|
||||
### 2. **Secrets-Management ist ein Luftschloss**
|
||||
|
||||
"Secrets werden nur über einen Secret-Service gelesen/geschrieben" - aber:
|
||||
- Kein Service definiert
|
||||
- Keine Rotation-Strategie
|
||||
- Keine Verschlüsselung at-rest spezifiziert
|
||||
- Registrar-Account speichert "Zugangsdaten/API-Referenzen" direkt?
|
||||
|
||||
Das ist ein Compliance-Alptraum und PCI-DSS-Verstoß.
|
||||
|
||||
### 3. **Audit-Log Hash-Chain ist Theatralik**
|
||||
|
||||
```python
|
||||
# "Hash-Chain" ohne:
|
||||
- Timestamp-Server
|
||||
- Externe Verifikation
|
||||
- Key-Management für Signierung
|
||||
```
|
||||
|
||||
Eine selbstgebaute Hash-Chain beweist nichts. Jeder mit DB-Zugriff kann die gesamte Chain neu berechnen.
|
||||
|
||||
### 4. **Domain-Model ist naiv**
|
||||
|
||||
Domain-Objekt ohne:
|
||||
- DNS-Record-Management
|
||||
- DNSSEC-Status
|
||||
- Transfer-Locks
|
||||
- Auth-Codes verschlüsselt?
|
||||
- Whois-Privacy-Settings
|
||||
- Expiry-Notifications
|
||||
|
||||
Das ist kein produktionsfähiges Domain-Management.
|
||||
|
||||
### 5. **Fehlendes Event-Sourcing für kritische Flows**
|
||||
|
||||
Vertragsänderungen, Zahlungen und Kündigungen nur als CRUD?
|
||||
|
||||
```yaml
|
||||
# Fehlende State-Machines für:
|
||||
- Vertragsstatus-Übergänge
|
||||
- Zahlungsflows
|
||||
- Domain-Transfers
|
||||
```
|
||||
|
||||
Inkonsistente Zustände sind vorprogrammiert.
|
||||
|
||||
### 6. **Adapter-Resilienz ohne Circuit Breaker**
|
||||
|
||||
ADR 0017 listet Retry/Backoff, aber:
|
||||
- Kein Circuit Breaker Pattern
|
||||
- Keine Health-Check Definition
|
||||
- Keine Fallback-Strategien
|
||||
- "Reconciliation" ohne Konfliktauflösung
|
||||
|
||||
### 7. **Customer vs Kunde - Inkonsistente Domänensprache**
|
||||
|
||||
Mal "Kunde", mal "Customer", mal "tenant_id", mal "organisation_id". Das führt zu:
|
||||
- Verwirrung im Team
|
||||
- Fehlerhaften Mappings
|
||||
- Doppelten Konzepten
|
||||
|
||||
### 8. **Fehlende Bounded Contexts**
|
||||
|
||||
Alles in einem großen Datenmodell:
|
||||
- Billing gehört nicht zu Hosting
|
||||
- Tickets sind ein eigenes System
|
||||
- Domains vs Server-Management vermischt
|
||||
|
||||
Monolithische Verkopplung statt modularer Architektur.
|
||||
|
||||
### 9. **Keine Versionierungsstrategie für Verträge/Produkte**
|
||||
|
||||
Was passiert wenn:
|
||||
- Produktpreise sich ändern?
|
||||
- Vertragsbedingungen angepasst werden?
|
||||
- Alte Rechnungen reproduziert werden müssen?
|
||||
|
||||
### 10. **Dead Letter Queue ohne Retry-Limits**
|
||||
|
||||
Infinite Retry-Loops möglich. Keine Definition von:
|
||||
- Max-Retry-Count
|
||||
- Exponential Backoff Caps
|
||||
- Alert-Thresholds
|
||||
- Poison Message Handling
|
||||
|
||||
### 11. **GoBD-Compliance ist Wunschdenken**
|
||||
|
||||
- Keine Unveränderbarkeits-Garantie
|
||||
- Keine Verfahrensdokumentation
|
||||
- Hash-Chain != Revisionssicherheit
|
||||
- Fehlende Archivierungsstrategie
|
||||
|
||||
### 12. **Superadmin als Backdoor**
|
||||
|
||||
"Systemweiter Zugriff" für Superadmin ohne:
|
||||
- 4-Augen-Prinzip
|
||||
- Break-Glass-Procedures
|
||||
- Time-boxed Privileges
|
||||
- Segregation of Duties
|
||||
|
||||
Das ist ein Insider-Threat-Paradies.
|
||||
|
||||
## Fazit
|
||||
|
||||
Diese Architektur wird in Produktion scheitern. Zu viele Kernkonzepte sind halbgar durchdacht. Besonders die Tenant-Isolation und das Secrets-Management sind brandgefährlich.
|
||||
|
||||
Empfehlung: Zurück ans Reißbrett. Mit einem erfahrenen Security-Architekten die Grundlagen neu aufsetzen.
|
||||
Reference in New Issue
Block a user