FDS2610/Steuerkram
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix #202: Admin-Prüfung bei /settings/logo (Logo-Upload) ergänzt

Browse source
This commit is contained in:
fds2610-bot 2026-08-26 07:26:28 +02:00
parent e30c465c16
commit 6fef9eff24
3 changed files with 20 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
Changes.md
View file

@ -2,6 +2,11 @@
--- ---
## 2026-08-26 – Fehlende Admin-Prüfung bei /settings/logo (Logo-Upload) (#202)
- **`src/Controllers/SettingsController.php`:** `uploadLogo()` wurde bei Fix #200 übersehen und rief `Auth::requireAdmin()` nicht auf, obwohl `deleteLogo()` bereits abgesichert ist. Da `/settings` nicht im globalen Admin-Gate in `public/index.php` enthalten ist, konnte jeder eingeloggte Nicht-Admin-Benutzer per `POST /settings/logo` das Firmenlogo austauschen. `Auth::requireAdmin()` als erste Zeile ergänzt, analog zu `deleteLogo()`.
- **`tests/e2e/specs/settings-admin-authorization.spec.js`:** Fall für `POST /settings/logo` (Multipart-Upload) ergänzt, der wie die übrigen Endpoints 403 für Nicht-Admins erwartet.
## 2026-08-24 – Fehlende Admin-Prüfung bei mehreren SettingsController-Endpoints (#200) ## 2026-08-24 – Fehlende Admin-Prüfung bei mehreren SettingsController-Endpoints (#200)
- **`src/Controllers/SettingsController.php`:** `deleteLogo()`, `llmTest()`, `bankImport()`, `createBankImportFormat()`, `updateBankImportField()`, `deleteBankImportFormat()`, `storeSuggestion()` und `dismissSuggestion()` prüften bislang nur den generellen Login (`Auth::require()` im Router-Gate), nicht die Rolle `admin` – anders als `update()`/`mailTest()`, deren Code-Kommentar diesen Bereich bewusst als admin-pflichtig begründet. Ein eingeloggter Benutzer mit Rolle `user` konnte damit das Firmenlogo löschen, CSV-Bankimport-Formate anlegen/ändern/löschen (Integritätsrisiko für importierte Buchungen) und den LLM-Verbindungstest auslösen. Alle acht Methoden rufen jetzt `Auth::requireAdmin()` konsistent zu Beginn auf. - **`src/Controllers/SettingsController.php`:** `deleteLogo()`, `llmTest()`, `bankImport()`, `createBankImportFormat()`, `updateBankImportField()`, `deleteBankImportFormat()`, `storeSuggestion()` und `dismissSuggestion()` prüften bislang nur den generellen Login (`Auth::require()` im Router-Gate), nicht die Rolle `admin` – anders als `update()`/`mailTest()`, deren Code-Kommentar diesen Bereich bewusst als admin-pflichtig begründet. Ein eingeloggter Benutzer mit Rolle `user` konnte damit das Firmenlogo löschen, CSV-Bankimport-Formate anlegen/ändern/löschen (Integritätsrisiko für importierte Buchungen) und den LLM-Verbindungstest auslösen. Alle acht Methoden rufen jetzt `Auth::requireAdmin()` konsistent zu Beginn auf.

2
src/Controllers/SettingsController.php
View file

@ -94,6 +94,8 @@ class SettingsController extends BaseController
public function uploadLogo(array $params): void public function uploadLogo(array $params): void
{ {
Auth::requireAdmin();
if (empty($_FILES['logo']) || $_FILES['logo']['error'] !== UPLOAD_ERR_OK) { if (empty($_FILES['logo']) || $_FILES['logo']['error'] !== UPLOAD_ERR_OK) {
View::redirect('/settings', 'Kein gültiges Bild übermittelt.', 'error'); View::redirect('/settings', 'Kein gültiges Bild übermittelt.', 'error');
return; return;

19
tests/e2e/specs/settings-admin-authorization.spec.js
View file

@ -1,9 +1,10 @@
// @ts-check // @ts-check
// Fix #200: Mehrere SettingsController-Endpoints (Logo, LLM-Test, // Fix #200 / #202: Mehrere SettingsController-Endpoints (Logo-Upload,
// CSV-Bankimport-Formate) prüften Auth::requireAdmin() nicht, obwohl der // Logo-Löschen, LLM-Test, CSV-Bankimport-Formate) prüften
// Bereich laut Code-Kommentar bei update() bewusst admin-pflichtig sein // Auth::requireAdmin() nicht, obwohl der Bereich laut Code-Kommentar bei
// soll. Ein normaler "user"-Account konnte damit Firmenlogo löschen, // update() bewusst admin-pflichtig sein soll. Ein normaler "user"-Account
// Bankimport-Formate anlegen/ändern/löschen und den LLM-Test auslösen. // konnte damit das Firmenlogo hochladen/löschen, Bankimport-Formate
// anlegen/ändern/löschen und den LLM-Test auslösen.
// Dieser Test prüft die Durchsetzung auf HTTP-Ebene, die die PHP-Suite // Dieser Test prüft die Durchsetzung auf HTTP-Ebene, die die PHP-Suite
// nicht erreicht (Router-Guard, Auth::requireAdmin()). // nicht erreicht (Router-Guard, Auth::requireAdmin()).
@ -42,6 +43,12 @@ test.describe('Autorisierung: Settings-Verwaltungsendpoints nur für Admins (Fix
const bankImportGet = await userPage.request.get('/settings/bankimport') const bankImportGet = await userPage.request.get('/settings/bankimport')
expect(bankImportGet.status()).toBe(403) expect(bankImportGet.status()).toBe(403)
const uploadLogo = await userPage.request.post('/settings/logo', {
headers,
multipart: { logo: { name: 'logo.png', mimeType: 'image/png', buffer: Buffer.from([]) } },
})
expect(uploadLogo.status()).toBe(403)
const deleteLogo = await userPage.request.post('/settings/logo/delete', { headers, form: {} }) const deleteLogo = await userPage.request.post('/settings/logo/delete', { headers, form: {} })
expect(deleteLogo.status()).toBe(403) expect(deleteLogo.status()).toBe(403)
@ -72,7 +79,7 @@ test.describe('Autorisierung: Settings-Verwaltungsendpoints nur für Admins (Fix
const deleteFormat = await userPage.request.post('/settings/bankimport/1/delete', { headers, form: {} }) const deleteFormat = await userPage.request.post('/settings/bankimport/1/delete', { headers, form: {} })
expect(deleteFormat.status()).toBe(403) expect(deleteFormat.status()).toBe(403)
for (const response of [bankImportGet, deleteLogo, llmTest, createFormat, suggest, dismiss, updateField, deleteFormat]) { for (const response of [bankImportGet, uploadLogo, deleteLogo, llmTest, createFormat, suggest, dismiss, updateField, deleteFormat]) {
await expect(response.text()).resolves.toContain('Kein Zugriff') await expect(response.text()).resolves.toContain('Kein Zugriff')
} }