eeg_portal/docs/compose/specs/code-review-critical-fixes.md

Spec: Code-Review-Fixes — Kritische und hohe Befunde

Zusammenfassung

Behebung von 3 kritischen und 5 hohen Code-Review-Findings im EEG Portal Backend. Die kritischen Finds betreffen Sicherheitslücken (fehlende Authorization, IDOR) und inkonsistente Fehlerbehandlung. Die hohen Finds betreffen Code-Qualität, Konsistenz und ein Produktionsrisiko.

Ausgangslage

Ein Code-Review hat 8 Befunde identifiziert:

#	Schwere	Befund
1	KRITISCH	EnergyCommunityAdminController fehlt @PreAuthorize — Admin-Endpunkte ohne Rollenprüfung
2	KRITISCH	NotificationService.markAsRead hat IDOR — jede User kann beliebige Notifications lesen
3	KRITISCH	GlobalExceptionHandler fehlt MethodArgumentNotValidException-Handler — @Valid-Fehler → 500 statt 400
4	HOCH	Inkonsistente Exception-Strategy — ResponseStatusException bypassed GlobalExceptionHandler
5	HOCH	TariffService Code-Duplizierung — ~30 Zeilen validierende Logik doppelt
6	HOCH	AtNumberAlreadyExistsException-Handler verwirft Exception-Message
7	HOCH	MeteringPoint.memberships — CascadeType.ALL ohne orphanRemoval (latentes Risiko)
8	HOCH	ddl-auto: update als Prod-Default — kann Schema still ändern

Befund-Detail

1. Fehlende @PreAuthorize auf EnergyCommunityAdminController (KRITISCH)

Datei: eeg_backend/.../community/api/EnergyCommunityAdminController.java:18-21

Ist-Zustand: Kein @PreAuthorize auf Klassenebene. Endpunkte unter /api/admin/energy-communities sind nur durch URL-Muster gesichert (SecurityFilterChain), nicht durch annotation-basierte Authorization. Im Vergleich: AdminIamController hat @PreAuthorize("hasRole('ADMIN')") auf Klassenebene.

Soll-Zustand: @PreAuthorize("hasRole('ADMIN')") auf Klassenebene, konsistent mit AdminIamController.

Risiko: Jeder authentifizierte MEMBER kann Admin-Endpoints aufrufen (Community erstellen, bearbeiten, löschen), sofern der SecurityFilterChain keine explizite URL-Prüfung hat.

2. IDOR in NotificationService.markAsRead (KRITISCH)

Dateien:

• eeg_backend/.../common/api/NotificationController.java:33-38
• eeg_backend/.../common/service/NotificationService.java:39-45

Ist-Zustand: markAsRead(UUID id) nimmt nur die Notification-ID, kein @CurrentUserId. Der Service macht keinen Ownership-Check. Andere Methoden im selben Controller (getNotifications, getUnreadCount, markAllAsRead) verwenden korrekt @CurrentUserId.

Soll-Zustand: Controller fügt @CurrentUserId String userId Parameter hinzu. Service-Signatur wird markAsRead(UUID id, UUID userId). Ownership-Check: notification.getUserId().equals(userId), sonst AccessDeniedException.

3. Fehlender MethodArgumentNotValidException-Handler (KRITISCH)

Datei: eeg_backend/.../common/exception/GlobalExceptionHandler.java

Ist-Zustand: Kein @ExceptionHandler(MethodArgumentNotValidException.class). Spring's Default-Handler liefert ein eigenes JSON-Format (field errors, global errors, object name) statt des projektspezifischen ErrorResponse(message, status). @Valid-Fehler produzieren inkonsistente Responses.

Soll-Zustand: Handler extrahiert BindingResult-Fehler, formatiert sie als komma-separierte Nachricht, gibt 400 BAD_REQUEST mit ErrorResponse zurück.

4. Inkonsistente Exception-Strategy (HOCH)

Dateien:

• eeg_backend/.../iam/service/AdminIamService.java:53-66 (rejectUser)
• eeg_backend/.../community/service/EnergyCommunityService.java:53-73 (update, delete)

Ist-Zustand:

• AdminIamService.approveUser: wirft IllegalArgumentException/IllegalStateException → GlobalExceptionHandler → 400/409
• AdminIamService.rejectUser: wirft ResponseStatusException → bypassed GlobalExceptionHandler → Spring-Default
• EnergyCommunityService.getCommunityById: IllegalArgumentException → 400
• EnergyCommunityService.update: ResponseStatusException(NOT_FOUND) → bypassed
• EnergyCommunityService.delete: IllegalArgumentException + ResponseStatusException(CONFLICT) → gemischt

Soll-Zustand: Einheitlich IllegalArgumentException (400) und IllegalStateException (409) in Services. ResponseStatusException nur in Controllern (wie AdminIamController.approveUser already macht).

Mapping: IAE → 400 (GlobalExceptionHandler:39-45), ISE → 409 (GlobalExceptionHandler:47-53).

5. TariffService Code-Duplizierung (HOCH)

Datei: eeg_backend/.../tariff/service/TariffService.java:74-197

Ist-Zustand: createUserTariff (Zeilen 76-126) und updateUserTariff (Zeilen 160-197) teilen ~30 Zeilen identische Validierung:

• source != target MeteringPoint
• Beide Points vorhanden (findByById)
• Beide Points ACTIVE (MakoState)
• Community-Tarif vorhanden
• Preis <= Maximalpreis
• Beide User aktive Members der Community

Soll-Zustand: Private Methode validateUserTariffRequest(UUID communityId, UserTariffRequest request) extrahieren. createUserTariff ruft sie plus Duplicate-Tarif-Check und Invite-Check auf. updateUserTariff ruft sie plus Ownership-Check auf.

6. AtNumberAlreadyExistsException-Handler verwirft Message (HOCH)

Datei: eeg_backend/.../common/exception/GlobalExceptionHandler.java:55-60

Ist-Zustand: Handler gibt hardcoded "Zählerpunkt existiert bereits" zurück, obwohl AtNumberAlreadyExistsException eine Message mit der AT-Nummer enthält.

Soll-Zustand: ex.getMessage() verwenden, wie bei allen anderen Handlern.

7. MeteringPoint.memberships ohne orphanRemoval (HOCH)

Datei: eeg_backend/.../community/domain/MeteringPoint.java:51-52

Ist-Zustand: @OneToMany(mappedBy = "meteringPoint", cascade = CascadeType.ALL) — kein orphanRemoval. Wenn ein Membership aus der Liste entfernt wird, wird es nicht automatisch aus der DB gelöscht.

Soll-Zustand: orphanRemoval = true hinzufügen. Defensive Absicherung, auch wenn aktuell kein Delete-Endpoint existiert.

8. ddl-auto: update als Prod-Default (HOCH)

Datei: eeg_backend/src/main/resources/application-prod.yml:9

Ist-Zustand: ddl-auto: ${JPA_DDL_AUTO:update} — Default update kann in Produktion Spalten still hinzufügen/ändern, aber nie löschen.

Soll-Zustand: Default auf validate ändern. Schema-Änderungen nur über Migrationen (Flyway/Liquibase) oder manuell.

Technische Abhängigkeiten

Finding 3 (Handler-Ergänzung) ─┐
Finding 6 (AtNumber-Handler)  ─┤── Finding 4 (Exception-Unification)
Finding 1 (PreAuthorize)      ─┤
Finding 2 (IDOR-Fix)          ─┤
Finding 5 (Refactoring)       ─┤
Finding 7 (orphanRemoval)     ─┤
Finding 8 (ddl-auto)          ─┘

Finding 3 und 6 müssen VOR Finding 4 kommen, da die Exception-Unification auf den bestehenden Handlern aufbaut. Alle anderen Findings sind unabhängig voneinander.

Scope

Im Scope

• Nur Backend-Änderungen (Java, YAML)
• Bestehende Tests aktualisieren (nicht neue Test-Suiten erstellen, wo Tests existieren)
• Conventional Commits

Nicht im Scope

• Frontend-Änderungen
• Flyway/Liquibase-Migration (nur ddl-auto Default ändern)
• MeteringPoint-Lösch-Endpoint erstellen (nur orphanRemoval als Absicherung)
• Audit-Logging
• Rollenbasierte SecurityFilterChain-Änderungen