253 lines
8.9 KiB
Markdown
253 lines
8.9 KiB
Markdown
# Projekt-Audit: Kritische Stellen & Verbesserungsvorschlaege
|
|
|
|
Erstellt: 2026-03-12
|
|
|
|
> Hinweis: Einige Stellen (auskommentierter Code, Debug-Statements) sind bewusst so,
|
|
> da der Code an vielen Stellen noch in Entwicklung ist. Diese Datei dokumentiert
|
|
> trotzdem alle Fundstellen, damit sie spaeter gezielt aufgeraeumt werden koennen.
|
|
|
|
---
|
|
|
|
## 1. Sicherheit
|
|
|
|
### 1.1 APP_KEY als Passwort (KRITISCH) — BEHOBEN 2026-03-12
|
|
|
|
**Problem:** Der Laravel APP_KEY wurde als Default-Passwort fuer neue User gesetzt und mit
|
|
loose comparison (`==`) statt `Hash::check()` verglichen.
|
|
|
|
**Fix:**
|
|
- `app/Repositories/UserRepository.php` — `Hash::make(config('app.key'))` statt `env('APP_KEY')`
|
|
- `app/User.php isPasswort()` — `Hash::check(config('app.key'), $this->password)` statt `==`
|
|
- `app/Repositories/CustomerRepository.php:22` — war bereits auskommentiert, kein Handlungsbedarf
|
|
|
|
> ACHTUNG: Bestehende User mit dem alten Klartext-APP_KEY als Passwort muessen
|
|
> einmalig migriert werden (z.B. per Artisan-Command), da `Hash::check()` gegen
|
|
> den alten Klartext-Wert nicht mehr matcht. Alternativ: Beim naechsten Login
|
|
> pruefen und automatisch re-hashen.
|
|
|
|
---
|
|
|
|
### 1.2 Unauthentifizierter Cron-Endpoint
|
|
|
|
**Problem:** `/cron/jobs/action/{action}/{key}` in `routes/web.php` ist ein GET-Endpoint
|
|
ohne Auth-Middleware. Cron-Jobs koennen von aussen getriggert werden, wenn die URL bekannt ist.
|
|
|
|
**Vorschlag:**
|
|
- Signed-URL Middleware (`signed`) oder ein geheimer Token-Vergleich in Middleware
|
|
- Alternativ: Cron-Jobs ausschliesslich ueber `php artisan schedule:run` ausfuehren
|
|
und den HTTP-Endpoint entfernen
|
|
|
|
---
|
|
|
|
### 1.3 env() ausserhalb von Config-Dateien — TEILWEISE BEHOBEN 2026-03-12
|
|
|
|
**Problem:** `env()` funktioniert nicht wenn `config:cache` aktiv ist.
|
|
|
|
**Fix (im Rahmen von 1.1):**
|
|
- `app/User.php` und `app/Repositories/UserRepository.php` — `env('APP_KEY')` durch
|
|
`config('app.key')` ersetzt.
|
|
- `app/Repositories/CustomerRepository.php:22` — war bereits auskommentiert.
|
|
|
|
**Pruefen:** Ob weitere `env()` Aufrufe ausserhalb von `config/` existieren.
|
|
|
|
---
|
|
|
|
### 1.4 Datei-Upload: Client-Extension vertraut
|
|
|
|
**Problem:** `getClientOriginalExtension()` nutzt die vom Browser gelieferte Dateiendung.
|
|
Ein manipulierter Upload koennte eine unerwartete Extension erhalten.
|
|
|
|
**Dateien:**
|
|
- `app/Repositories/FileRepository.php:56`
|
|
- `app/Repositories/ImportRepository.php` (gleicher Ansatz)
|
|
|
|
**Vorschlag:** `$file->extension()` verwenden (leitet Extension aus MIME-Type ab)
|
|
oder zusaetzlich gegen eine Whitelist validieren.
|
|
|
|
---
|
|
|
|
### 1.5 PayPal-Fehler an User exponiert
|
|
|
|
**Problem:** `abort(403, 'PayPal Type: '.$order['type'])` gibt interne PayPal-Fehlertypen
|
|
direkt an den User weiter.
|
|
|
|
**Datei:** `app/Http/Controllers/Pay/PayPalController.php:65`
|
|
|
|
**Vorschlag:** Fehler intern loggen, dem User eine generische Fehlermeldung zeigen.
|
|
|
|
---
|
|
|
|
## 2. Routing-Probleme
|
|
|
|
### 2.1 Doppelte Route-Namen — BEHOBEN 2026-03-12
|
|
|
|
**Fix in `routes/web.php`:**
|
|
|
|
| Alt (doppelt) | Neu (eindeutig) | Route |
|
|
|---------------------|------------------------------|------------------------------------------|
|
|
| `home` (GET `/`) | `landing` | Oeffentliche Startseite |
|
|
| `home` (GET `/home`)| `home` (unveraendert) | Auth-geschuetztes Dashboard |
|
|
| `user_sales` (Z.204)| `user_sales_orders` | User-Bestellungen (orders-Methode) |
|
|
| `user_sales_detail` (Z.205)| `user_sales_order_detail` | User-Bestellungsdetail (orderDetail) |
|
|
| `user_sales` (Z.248)| `user_sales` (unveraendert) | Shop-Verkaeufe (index-Methode) |
|
|
| `user_sales_detail` (Z.249)| `user_sales_detail` (unveraendert) | Shop-Verkaufsdetail (detail) |
|
|
|
|
Die umbenannten Routen (`user_sales_orders`, `user_sales_order_detail`) wurden in keinen
|
|
Views/Controllern referenziert, daher waren keine weiteren Anpassungen noetig.
|
|
|
|
---
|
|
|
|
### 2.2 GET-Routen fuer Loeschaktionen
|
|
|
|
**Problem:** Loeschaktionen wie `admin_product_delete`, `admin_product_image_delete`,
|
|
`admin_lead_delete_file` sind als GET definiert. GET-Requests sollten keine Daten aendern
|
|
(Browser-Prefetch, Crawler, CSRF-Risiko).
|
|
|
|
**Vorschlag:** Auf DELETE-Method umstellen mit `@method('DELETE')` in Blade-Forms.
|
|
|
|
---
|
|
|
|
### 2.3 GET-Route mutiert Daten
|
|
|
|
**Problem:** `SalesController::detail()` setzt `shipped = 1` bei einem GET-Request.
|
|
|
|
**Datei:** `app/Http/Controllers/SalesController.php` (detail-Methode)
|
|
|
|
**Vorschlag:** Versand-Status per separatem POST/PATCH-Endpoint setzen.
|
|
|
|
---
|
|
|
|
## 3. Bugs / Korrektheitsprobleme
|
|
|
|
### 3.1 Stornierungs-Mail sendet Rechnungs-Mail
|
|
|
|
**Problem:** `createCancellation()` ruft `Invoice::sendInvoiceMail()` auf statt eine
|
|
dedizierte Stornierungs-Mail. Kunden erhalten bei Stornierung die Rechnungs-Mail.
|
|
|
|
**Datei:** `app/Repositories/InvoiceRepository.php:279`
|
|
|
|
**Vorschlag:** Eigene `sendCancellationMail()` Methode erstellen und hier aufrufen.
|
|
|
|
---
|
|
|
|
### 3.2 Schedule: payments:reminders ohne Frequenz
|
|
|
|
**Problem:** Der Command `payments:reminders` hat keine Schedule-Frequenz (`->daily()` etc.)
|
|
konfiguriert. Ohne Frequenz laeuft er ggf. jede Minute.
|
|
|
|
**Datei:** `app/Console/Kernel.php:37-43`
|
|
|
|
**Vorschlag:** Gewuenschte Frequenz setzen, z.B. `->dailyAt('08:00')`.
|
|
|
|
---
|
|
|
|
### 3.3 helpers.php: Doppelte Funktion
|
|
|
|
**Problem:** `make_old_url()` ist zweimal definiert (Zeile 3-9 und 11-16).
|
|
Die zweite Definition wird durch `function_exists`-Guard ignoriert.
|
|
|
|
**Datei:** `app/helpers.php`
|
|
|
|
**Vorschlag:** Doppelte Definition entfernen.
|
|
|
|
---
|
|
|
|
## 4. Performance
|
|
|
|
### 4.1 N+1 Query-Probleme
|
|
|
|
Mehrere Controller laden Beziehungen in Schleifen ohne Eager Loading:
|
|
|
|
| Datei | Stelle | Problem |
|
|
|-------|--------|---------|
|
|
| `app/Http/Controllers/SalesController.php` | datatable() | `$order->member->getFullName()` pro DataTable-Zeile |
|
|
| `app/Repositories/InvoiceRepository.php` | prepairForDelivery() | `Attribute::find()` in dreifach verschachtelter Schleife |
|
|
| `app/Http/Controllers/SyS/AdminToolsController.php` | _dbOrderPaymentFor() | `ShoppingUser::all()` dann `->shopping_order` pro Eintrag |
|
|
| `app/Http/Controllers/User/PaymentController.php` | index() | `user_margins`/`user_credits` in DataTable-Callback |
|
|
|
|
**Vorschlag:** `with()` / `load()` fuer Eager Loading nutzen. Bei DataTables die
|
|
Relationships im Query vorladen.
|
|
|
|
---
|
|
|
|
### 4.2 AdminToolsController: Ganze Tabellen laden
|
|
|
|
**Problem:** `ShoppingUser::all()` laedt die komplette Tabelle ohne Pagination.
|
|
|
|
**Datei:** `app/Http/Controllers/SyS/AdminToolsController.php:97`
|
|
|
|
**Vorschlag:** Chunking (`chunk()`) oder Query mit Filtern nutzen.
|
|
|
|
---
|
|
|
|
## 5. Code-Qualitaet (bei Gelegenheit)
|
|
|
|
### 5.1 Keine FormRequest-Klassen
|
|
|
|
Das Projekt hat keinen einzigen FormRequest. Alle 25+ Controller nutzen inline
|
|
`Validator::make()`. Bei neuen Features sollten FormRequests genutzt werden.
|
|
Bestehende nach und nach migrieren.
|
|
|
|
### 5.2 Fehlende Return-Type-Deklarationen
|
|
|
|
Projekt-weit fehlen Return-Types auf Methoden (Controller, Models, Services, Repositories).
|
|
Bei jeder Aenderung an einer Datei sollten Return-Types ergaenzt werden.
|
|
|
|
### 5.3 Model-Relationships ohne Return-Type-Hints
|
|
|
|
Alle Relationship-Methoden in `app/User.php` und `app/Models/*.php` haben keine
|
|
Return-Type-Hints (`BelongsTo`, `HasMany`, etc.).
|
|
|
|
### 5.4 `$dates` deprecated (10+ Models)
|
|
|
|
`protected $dates = ['deleted_at']` ist seit Laravel 10 deprecated.
|
|
Sollte durch `casts()` Methode ersetzt werden:
|
|
|
|
```php
|
|
protected function casts(): array
|
|
{
|
|
return ['deleted_at' => 'datetime'];
|
|
}
|
|
```
|
|
|
|
Betroffene Models: `ShoppingOrder`, `UserAccount`, `ShoppingOrderItem`, `UserShop`,
|
|
`PromotionUser`, `Product`, `ShoppingUser`, `ShoppingOrderMargin`, `PromotionAdmin`,
|
|
`UserMessage`.
|
|
|
|
### 5.5 PayPal: Fehlendes Error-Handling
|
|
|
|
`PayPalController::paymentSuccess()` und `payment()` haben kein try/catch.
|
|
Netzwerkfehler oder unerwartete PayPal-Antworten fuehren zu unbehandelten 500-Fehlern.
|
|
|
|
**Datei:** `app/Http/Controllers/Pay/PayPalController.php`
|
|
|
|
### 5.6 Invoice Service: Nur statische Methoden
|
|
|
|
`app/Services/Invoice.php` ist komplett statisch — nicht testbar und nicht per
|
|
Dependency Injection nutzbar. Bei Ueberarbeitung auf Instanz-Methoden umstellen.
|
|
|
|
### 5.7 Typo: prepairForDelivery
|
|
|
|
`app/Repositories/InvoiceRepository.php:156` — Methode heisst `prepairForDelivery`
|
|
statt `prepareForDelivery`.
|
|
|
|
---
|
|
|
|
## Zusammenfassung nach Prioritaet
|
|
|
|
| Prio | # | Thema | Aufwand |
|
|
|------|---|-------|---------|
|
|
| ~~KRITISCH~~ | ~~1.1~~ | ~~APP_KEY als Passwort~~ | ~~BEHOBEN~~ |
|
|
| KRITISCH | 1.2 | Cron-Endpoint ohne Auth | Klein |
|
|
| ~~HOCH~~ | ~~1.3~~ | ~~env() ausserhalb Config~~ | ~~BEHOBEN~~ |
|
|
| ~~HOCH~~ | ~~2.1~~ | ~~Doppelte Route-Namen~~ | ~~BEHOBEN~~ |
|
|
| HOCH | 3.1 | Falsche Mail bei Stornierung | Klein |
|
|
| HOCH | 3.2 | Schedule ohne Frequenz | Klein |
|
|
| MITTEL | 1.4 | File-Upload Extension | Klein |
|
|
| MITTEL | 1.5 | PayPal-Fehler exponiert | Klein |
|
|
| MITTEL | 2.2 | GET fuer Loeschaktionen | Mittel |
|
|
| MITTEL | 2.3 | GET mutiert Daten | Klein |
|
|
| MITTEL | 3.3 | Doppelte Funktion helpers.php | Klein |
|
|
| MITTEL | 4.1 | N+1 Queries | Mittel |
|
|
| MITTEL | 4.2 | Ganze Tabellen laden | Klein |
|
|
| NIEDRIG | 5.1-5.7 | Code-Qualitaet | Fortlaufend |
|