mirror of
https://github.com/s-frick/effigenix.git
synced 2026-03-28 19:40:18 +01:00
192 lines
8.3 KiB
Markdown
192 lines
8.3 KiB
Markdown
# Ticket 002 - Komplettes Code- und Security-Review: Effigenix Backend
|
|
|
|
**Datum:** 2026-02-19
|
|
**Scope:** Gesamtes Backend (Security, Auth/AuthZ, API-Schutz, Secrets, Betriebssicherheit)
|
|
**Status:** Offen
|
|
|
|
---
|
|
|
|
## Kritisch
|
|
|
|
### K1 - Fehlende Autorisierung auf sensiblen User-Endpunkten (IDOR / Privilege Escalation)
|
|
|
|
**Betroffene Dateien:**
|
|
- `src/main/java/de/effigenix/infrastructure/usermanagement/web/controller/UserController.java:200`
|
|
- `src/main/java/de/effigenix/infrastructure/usermanagement/web/controller/UserController.java:250`
|
|
- `src/main/java/de/effigenix/infrastructure/usermanagement/web/controller/UserController.java:313`
|
|
- `src/main/java/de/effigenix/infrastructure/usermanagement/web/controller/UserController.java:613`
|
|
- `src/main/java/de/effigenix/application/usermanagement/ListUsers.java:32`
|
|
- `src/main/java/de/effigenix/application/usermanagement/GetUser.java:25`
|
|
- `src/main/java/de/effigenix/application/usermanagement/UpdateUser.java:29`
|
|
- `src/main/java/de/effigenix/application/usermanagement/ChangePassword.java:36`
|
|
|
|
**Problem:**
|
|
Mehrere User-Endpunkte haben kein `@PreAuthorize`, und die zugehörigen Use Cases prüfen weder Berechtigungen noch Ownership (`actorId` vs. Ziel-User) robust. Jeder authentifizierte Benutzer kann dadurch potenziell Benutzerdaten lesen und ändern.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Auf allen betroffenen Endpunkten explizite Policy erzwingen (`USER_READ`, `USER_WRITE` oder Self-Service-Policy).
|
|
- Zusätzlich serverseitige Ownership-/Scope-Prüfung im Application Layer einbauen (Defense in Depth).
|
|
- Negative Security-Tests ergänzen (fremden User lesen/ändern, Passwortänderung für fremden User).
|
|
|
|
---
|
|
|
|
### K2 - Refresh-Token wird als Access-Token akzeptiert
|
|
|
|
**Betroffene Dateien:**
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtTokenProvider.java:113`
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtTokenProvider.java:127`
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtSessionManager.java:89`
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtAuthenticationFilter.java:73`
|
|
|
|
**Problem:**
|
|
Refresh-Tokens enthalten zwar `type=refresh`, bei Validierung und Request-Authentifizierung wird der Typ aber nicht geprüft. Dadurch können Refresh-Tokens als Bearer für API-Zugriffe missbraucht werden.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Token-Typ verpflichtend validieren (`access` nur im Request-Auth-Filter, `refresh` nur im Refresh-Flow).
|
|
- Separate Validierungsmethoden einführen (`validateAccessToken`, `validateRefreshToken`).
|
|
- Security-Tests ergänzen: Refresh-Token gegen geschützte Endpunkte muss 401 liefern.
|
|
|
|
---
|
|
|
|
## Hoch
|
|
|
|
### H1 - Unsichere Default-Credentials und Secrets im Standardprofil
|
|
|
|
**Betroffene Dateien:**
|
|
- `src/main/resources/application.yml:6`
|
|
- `src/main/resources/application.yml:8`
|
|
- `src/main/resources/application.yml:26`
|
|
- `src/main/resources/application.yml:27`
|
|
- `src/main/resources/application.yml:31`
|
|
- `src/main/resources/db/changelog/changes/004-seed-admin-user.sql:3`
|
|
- `src/main/resources/db/changelog/changes/004-seed-admin-user.sql:12`
|
|
- `src/main/resources/db/changelog/changes/004-seed-admin-user.sql:26`
|
|
|
|
**Problem:**
|
|
Statische Defaults für DB-Credentials, Security-User, JWT-Secret und Seed-Admin-Passwort sind im produktionsnahen Pfad hinterlegt. Bei fehlender Überschreibung entsteht ein direktes Übernahmerisiko.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Keine sicherheitsrelevanten Defaults im Standardprofil; App soll fail-fast starten, wenn Secrets fehlen.
|
|
- Seed-Admin ausschließlich über dediziertes Dev-/Bootstrap-Profil.
|
|
- Secret-Management über Vault/Kubernetes Secrets/CI Secret Store erzwingen.
|
|
|
|
---
|
|
|
|
### H2 - Masterdata-Read-Endpunkte ohne Permission-Check
|
|
|
|
**Betroffene Dateien:**
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/ArticleController.java:95`
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/ArticleController.java:121`
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/CustomerController.java:99`
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/CustomerController.java:125`
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/SupplierController.java:90`
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/SupplierController.java:113`
|
|
- `src/main/java/de/effigenix/infrastructure/masterdata/web/controller/ProductCategoryController.java:72`
|
|
- `src/main/resources/db/changelog/changes/008-add-masterdata-permissions.sql:1`
|
|
|
|
**Problem:**
|
|
`MASTERDATA_READ` ist im Rollenmodell vorhanden, wird auf mehreren GET-Endpunkten aber nicht erzwungen.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Alle Read-Endpunkte mit `@PreAuthorize("hasAuthority('MASTERDATA_READ')")` absichern.
|
|
- Optional feiner aufteilen (`ARTICLE_READ`, `CUSTOMER_READ` etc.) für Least Privilege.
|
|
- Integrationstests für positive und negative Berechtigungsfälle ergänzen.
|
|
|
|
---
|
|
|
|
## Mittel
|
|
|
|
### M1 - Refresh-Flow funktional unvollständig (Session Lifecycle)
|
|
|
|
**Betroffene Datei:**
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtSessionManager.java:141`
|
|
|
|
**Problem:**
|
|
`refreshSession()` wirft `UnsupportedOperationException`. Der Endpoint ist öffentlich verfügbar, aber die Kernlogik fehlt.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Vollständigen Refresh-Flow implementieren: Refresh-Token validieren, User laden, Status prüfen, neues Token-Paar ausstellen.
|
|
- Rotation/Family-Tracking oder wenigstens Single-Use-Refresh-Tokens einführen.
|
|
|
|
---
|
|
|
|
### M2 - Kein Brute-Force-Schutz auf Login
|
|
|
|
**Betroffene Datei:**
|
|
- `src/main/java/de/effigenix/application/usermanagement/AuthenticateUser.java:42`
|
|
|
|
**Problem:**
|
|
Keine Rate-Limits, kein Backoff, kein attempt-based Locking. Ermöglicht Credential-Stuffing und Passwort-Raten.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- IP- und Username-basiertes Rate-Limiting (z. B. Bucket4j).
|
|
- Temporärer Lockout oder exponentielles Backoff nach Fehlversuchen.
|
|
- Security-Monitoring/Alerting auf auffällige Login-Muster.
|
|
|
|
---
|
|
|
|
### M3 - Logout-Invalidierung nur in-memory, ohne Cleanup und ohne Cluster-Fähigkeit
|
|
|
|
**Betroffene Dateien:**
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtSessionManager.java:35`
|
|
- `src/main/java/de/effigenix/infrastructure/security/JwtSessionManager.java:162`
|
|
|
|
**Problem:**
|
|
Blacklist liegt nur im Prozessspeicher, ist nach Neustart weg und wächst ohne TTL/Cleanup.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Redis-basierte Blacklist mit TTL bis Token-Ablauf einsetzen.
|
|
- Alternativ zentraler Session Store + revocable token ids (`jti`).
|
|
- Cleanup-Strategie und Betriebsmetriken einführen.
|
|
|
|
---
|
|
|
|
## Niedrig
|
|
|
|
### N1 - Falsches Audit-Event bei fehlgeschlagenem Passwortwechsel
|
|
|
|
**Betroffene Datei:**
|
|
- `src/main/java/de/effigenix/application/usermanagement/ChangePassword.java:53`
|
|
|
|
**Problem:**
|
|
Bei falschem `currentPassword` wird `PASSWORD_CHANGED` statt eines Failure-Events geloggt.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Eigenes Event `PASSWORD_CHANGE_FAILED` verwenden.
|
|
- Audit-Event-Mapping für Erfolgs-/Fehlerpfade systematisch prüfen.
|
|
|
|
---
|
|
|
|
### N2 - Verbose Fehler-/Debug-Ausgabe im Standardprofil
|
|
|
|
**Betroffene Dateien:**
|
|
- `src/main/resources/application.yml:39`
|
|
- `src/main/resources/application.yml:46`
|
|
- `src/main/resources/application.yml:47`
|
|
- `src/main/resources/application.yml:48`
|
|
|
|
**Problem:**
|
|
`include-message: always` plus DEBUG für Security und SQL im Standardprofil erhöht Risiko für Informationsabfluss.
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Sichere Logging-Baseline für Produktivbetrieb (`INFO/WARN`, keine sensiblen Details).
|
|
- Fehlermeldungen für Clients generisch halten, Details nur serverseitig protokollieren.
|
|
- Profilabhängige Logging-Policies (`dev`, `test`, `prod`) trennen.
|
|
|
|
---
|
|
|
|
## Test-/Betriebsnotiz
|
|
|
|
### T1 - Security-Integrationstests in aktueller Umgebung nicht ausführbar
|
|
|
|
**Nachweis:**
|
|
- `target/surefire-reports/de.effigenix.infrastructure.usermanagement.web.UserControllerIntegrationTest.txt`
|
|
- `target/surefire-reports/de.effigenix.infrastructure.usermanagement.web.SecurityIntegrationTest.txt`
|
|
- `target/surefire-reports/de.effigenix.infrastructure.usermanagement.web.AuthControllerIntegrationTest.txt`
|
|
|
|
**Problem:**
|
|
Tests schlagen hier fehl, weil der Sandbox-Runner keinen lokalen Socket/Port für den Embedded Tomcat öffnen darf (`java.net.SocketException: Operation not permitted`).
|
|
|
|
**Empfohlene Maßnahme:**
|
|
- Security-ITs in CI/Umgebung mit erlaubtem Port-Binding ausführen.
|
|
- Alternativ `@WebMvcTest`-basierte Tests ohne echten Server ergänzen, um AuthZ-Regeln isoliert zu prüfen.
|