# Ticket 001 – Code Review: User Management Bounded Context

**Datum:** 2026-02-17
**Commit:** `ec9114a` – feat: add Spring Boot ERP application with user management domain
**Status:** Offen

---

## Kritisch

### K1 – Aggregates sind nicht immutable

**Betroffene Dateien:**
- `domain/usermanagement/User.java` (Zeilen 22–29)
- `domain/usermanagement/Role.java` (Zeilen 18–21)

**Problem:**
Felder in `User` und `Role` sind nicht `final`. Business-Methoden wie `lock()`, `unlock()`, `updateEmail()`, `changePassword()` mutieren den internen State direkt statt neue Instanzen oder `Result` zurückzugeben. Die interne `roles`-Collection in `User` (Zeile 25) ist mutable und wird per `assignRole()`/`removeRole()` in-place modifiziert.

**Lösung:**
- Alle Felder `final` machen
- Business-Methoden geben `Result<UserError, User>` zurück (Copy-on-Write)
- Interne Collections nur über defensive Kopien exponieren

---

### K2 – Fehlende Command-Validierung im Application Layer

**Betroffene Dateien:**
- `application/usermanagement/command/CreateUserCommand.java`
- `application/usermanagement/command/UpdateUserCommand.java`
- `application/usermanagement/command/AuthenticateCommand.java`
- `application/usermanagement/command/ChangePasswordCommand.java`
- `application/usermanagement/command/AssignRoleCommand.java`

**Problem:**
Alle Commands sind reine Records ohne jegliche Validierung. Wird der Application Layer außerhalb des HTTP-Kontexts aufgerufen (z.B. Scheduled Jobs, Message Queues), gibt es keine Input-Checks. Die Validierung liegt ausschließlich in den Web-DTOs (`@Valid`-Annotationen).

**Lösung:**
- Self-validating Commands mit Factory-Methoden, die `Result` zurückgeben
- Oder Validierung als ersten Schritt im Use Case

---

### K3 – Authorization fehlt im Application Layer

**Betroffene Dateien:**
- `application/usermanagement/CreateUser.java`
- `application/usermanagement/LockUser.java`
- `application/usermanagement/AssignRole.java`
- (alle Use Cases betroffen)

**Problem:**
`AuthorizationPort` wird im Application Layer nicht verwendet. Autorisierung liegt ausschließlich auf `@PreAuthorize` im Controller. Bei Non-HTTP-Aufrufen wird die Authorization komplett umgangen.

**Lösung:**
- `AuthorizationPort.can(actorId, action)` als ersten Check in jedem Use Case aufrufen
- Bei fehlender Berechtigung `Result.failure(UserError.Unauthorized(...))` zurückgeben

---

### K4 – Token-Blacklist Memory Leak

**Betroffene Datei:**
- `infrastructure/security/JwtSessionManager.java` (Zeilen 35–36, 164–168)

**Problem:**
Die Token-Blacklist nutzt `ConcurrentHashMap.newKeySet()` ohne TTL oder Cleanup. Bei laufendem Server wächst die Blacklist unbegrenzt. Nach Server-Restart ist die Blacklist leer – ausgeloggte Tokens werden wieder gültig. Funktioniert nicht in Cluster-Deployments.

**Lösung:**
- Redis-basierte Blacklist mit TTL = Token-Expiration
- Oder In-Memory mit Scheduled Cleanup abgelaufener Tokens

---

### K5 – Audit-Bug in ChangePassword

**Betroffene Datei:**
- `application/usermanagement/ChangePassword.java` (Zeile 52)

**Problem:**
Im Fehlerfall wird `AuditEvent.PASSWORD_CHANGED` geloggt statt `PASSWORD_CHANGE_FAILED`. Falsche Audit-Events verfälschen das Security-Monitoring.

**Lösung:**
- Korrektes Event `PASSWORD_CHANGE_FAILED` im Fehlerfall loggen
- Audit-Events auf Success/Failure-Pfade prüfen

---

## Mittel

### M1 – Fehlende Status-Transitionen-Validierung

**Betroffene Datei:**
- `domain/usermanagement/User.java` (Zeilen 118–132)

**Problem:**
`lock()`, `unlock()`, `deactivate()`, `activate()` validieren den aktuellen Status nicht. Invalide Übergänge sind möglich (z.B. einen inaktiven User sperren). Keine Idempotenz-Checks.

**Lösung:**
- State-Machine oder explizite Guards für erlaubte Übergänge
- `Result<UserError, User>` als Rückgabetyp

---

### M2 – Inkonsistente Result-Nutzung in Domain-Methoden

**Betroffene Datei:**
- `domain/usermanagement/User.java` (Zeilen 106–159)

**Problem:**
Manche Business-Methoden geben `Result` zurück (`assignRole()`, `changePassword()`), andere `void` (`removeRole()`, `lock()`, `unlock()`, `updateBranch()`, `updateLastLogin()`). Inkonsistentes API-Design.

**Lösung:**
- Alle Business-Methoden einheitlich auf `Result<UserError, User>` umstellen (siehe K1)

---

### M3 – RemoveRole bricht Command-Pattern

**Betroffene Datei:**
- `application/usermanagement/RemoveRole.java` (Zeile 44)

**Problem:**
Einziger Use Case, der Raw-Parameter statt eines Command-Objekts akzeptiert:
```java
public Result<UserError, UserDTO> execute(String userId, RoleName roleName, ActorId performedBy)
```
Alle anderen Use Cases nutzen Command-Records.

**Lösung:**
- `RemoveRoleCommand` Record erstellen und in `execute()` verwenden

---

### M4 – Refresh Token nicht implementiert

**Betroffene Datei:**
- `infrastructure/security/JwtSessionManager.java` (Zeilen 141–144)

**Problem:**
`refreshSession()` wirft `UnsupportedOperationException`. User-Status (gesperrt/gelöscht) wird beim Token-Refresh nicht geprüft.

**Lösung:**
- Implementierung: User laden, Status verifizieren, neues Token-Paar ausstellen
- Altes Refresh-Token auf Blacklist setzen

---

### M5 – Kein Rate Limiting auf Login-Endpoint

**Betroffene Datei:**
- `infrastructure/usermanagement/web/controller/AuthController.java` (Zeile 90)

**Problem:**
`/api/auth/login` hat keinen Brute-Force-Schutz. Angreifer können Credentials unbegrenzt durchprobieren.

**Lösung:**
- Rate Limiting per IP/Username (z.B. Bucket4j, Spring Cloud Gateway)
- Account-Lockout nach N fehlgeschlagenen Versuchen
- Exponentielles Backoff

---

### M6 – CORS deaktiviert statt konfiguriert

**Betroffene Datei:**
- `infrastructure/security/SecurityConfig.java` (Zeile 69)

**Problem:**
CORS ist komplett deaktiviert (`AbstractHttpConfigurer::disable`). TODO-Kommentar im Code, aber keine Konfiguration. Browser-basierte Clients können keine Cross-Origin-Requests senden.

**Lösung:**
- CORS mit expliziten Allowed Origins, Methods und Headers konfigurieren
- Environment-abhängig (Dev: localhost, Prod: spezifische Domain)

---

### M7 – Error Response Information Disclosure

**Betroffene Datei:**
- `infrastructure/security/SecurityConfig.java` (Zeilen 100–113)

**Problem:**
Custom Exception Handler gibt `authException.getMessage()` direkt im JSON-Response zurück. Kann interne Implementierungsdetails exponieren.

**Lösung:**
- Generische Fehlermeldungen für Authentication/Authorization Failures
- Details nur ins Server-Log schreiben

---

## Niedrig

### N1 – Swagger UI öffentlich zugänglich

**Betroffene Datei:**
- `infrastructure/security/SecurityConfig.java` (Zeile 83)
- `application.yml` (Zeile 56)

**Problem:**
Swagger UI ist standardmäßig ohne Authentifizierung erreichbar. Exponiert alle API-Endpoints mit Parametern.

**Lösung:**
- In Production deaktivieren oder hinter Authentication legen
- Spring-Profile nutzen: `springdoc.swagger-ui.enabled=${SWAGGER_ENABLED:false}`

---

### N2 – Default-Admin Credentials in Seed-Daten

**Betroffene Datei:**
- `db/changelog/changes/004-seed-admin-user.sql` (Zeilen 2–3, 26)

**Problem:**
Admin-Credentials `admin/admin123` stehen als Kommentar in der SQL-Datei und als Table-Comment in der Datenbank. Bei versehentlicher DB-Exposure sind Credentials sofort sichtbar.

**Lösung:**
- Seed-Daten nur in Dev-Profil ausführen
- Table-Comment mit Credentials entfernen
- Production: Admin über Environment-Variablen oder Init-Script bootstrappen

---

### N3 – CreateUser erlaubt User ohne Rollen

**Betroffene Datei:**
- `application/usermanagement/CreateUser.java` (Zeile 76)

**Problem:**
Wenn `cmd.roleNames()` leer ist, wird ein User ohne Rollen erstellt. Kein Check, ob mindestens eine Rolle zugewiesen wird.

**Lösung:**
- Validierung in Command oder Use Case: mindestens eine Rolle erforderlich

---

### N4 – Falscher Error-Typ in Role.addPermission()

**Betroffene Datei:**
- `domain/usermanagement/Role.java` (Zeile 70)

**Problem:**
Bei `null`-Permission wird `UserError.NullRole()` verwendet – semantisch falsch.

**Lösung:**
- Eigenen Error-Typ verwenden oder `IllegalArgumentException` (da interner Programmierfehler)

---

### N5 – Fehlende Uniqueness-Validierung im Domain Layer

**Betroffene Datei:**
- `domain/usermanagement/User.java` (Zeilen 64–69)

**Problem:**
Username- und Email-Uniqueness wird nur im Application Layer geprüft (Repository-Abfrage). Der Domain Layer dokumentiert diese Invariante nicht explizit.

**Lösung:**
- Domain Service für Uniqueness-Checks oder explizite Dokumentation als Application-Layer-Invariante

---

### N6 – Repetitiver Switch-Pattern in Use Cases

**Betroffene Dateien:**
- Alle Use Cases im Application Layer

**Problem:**
Das Pattern `switch (repository.method()) { case Failure f -> ...; case Success s -> ...; }` wiederholt sich ~30+ Mal. Hoher Boilerplate-Anteil.

**Lösung:**
- `Result.flatMap()` und `Result.mapError()` konsequent nutzen
- Helper-Methode für Repository-Error → UserError Mapping

---

## Checkliste Production-Readiness

- [ ] K1: Aggregates immutable machen
- [ ] K2: Command-Validierung im Application Layer
- [ ] K3: AuthorizationPort in Use Cases integrieren
- [ ] K4: Token-Blacklist mit TTL/Redis ersetzen
- [ ] K5: Audit-Bug in ChangePassword fixen
- [ ] M4: Refresh Token implementieren
- [ ] M5: Rate Limiting einrichten
- [ ] M6: CORS konfigurieren
- [ ] N1: Swagger in Production absichern
- [ ] N2: Seed-Daten Production-safe machen
- [ ] JWT_SECRET, DB_PASSWORD als Environment-Variablen setzen
- [ ] HTTPS/TLS konfigurieren
- [ ] Log-Aggregation und Security-Monitoring aufsetzen