effigenix/backend/docs/tickets/001-code-review-usermanagement.md

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:

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