diff --git a/Changes.md b/Changes.md index 570a9c4..174a841 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,11 @@ --- +## 2026-08-26 – Fehlende Admin-Prüfung im BankinstituteController (#203) + +- **`src/Controllers/BankinstituteController.php`:** `index()`, `check()` und `import()` prüften bislang keine Rolle, obwohl der globale Admin-Gate in `public/index.php` nur `/useradmin*` abdeckt und `SettingsController` erst seit Fix #200 admin-pflichtig ist – dieser Controller wurde dabei übersehen. Insbesondere `import()` ersetzt über `Bankinstitut::replaceAll()` die komplette BLZ/BIC-Referenztabelle (u. a. für die FinTS-Direktverbindung genutzt) anhand einer hochgeladenen Datei; jeder eingeloggte Nicht-Admin-Nutzer konnte diese systemweite Tabelle beliebig überschreiben. Alle drei Methoden rufen jetzt `Auth::requireAdmin()` zu Beginn auf. `search()` bleibt bewusst offen, da sie unter der eigenen Route `/bankinstitute/search` für die BLZ-Suche beim Anlegen einer FinTS-Verbindung (`bank/fints.php`) von allen eingeloggten Nutzern benötigt wird. +- **`tests/e2e/specs/settings-admin-authorization.spec.js`:** Fälle für `GET /settings/bankinstitute`, `POST /settings/bankinstitute/check` und `POST /settings/bankinstitute/import` ergänzt (403 für Nicht-Admins), sowie ein Check, dass `GET /bankinstitute/search` weiterhin für alle eingeloggten Nutzer erreichbar bleibt. + ## 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()`. diff --git a/src/Controllers/BankinstituteController.php b/src/Controllers/BankinstituteController.php index ac93a40..03f31d2 100644 --- a/src/Controllers/BankinstituteController.php +++ b/src/Controllers/BankinstituteController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Auth; use Steuerkram\Models\Bankinstitut; use Steuerkram\Services\FintsInstituteImport; use Steuerkram\View; @@ -14,6 +15,8 @@ class BankinstituteController extends BaseController public function index(array $params): void { + Auth::requireAdmin(); + $query = trim((string)($_GET['q'] ?? '')); $rows = $query !== '' ? Bankinstitut::search($query, 200) : []; @@ -31,6 +34,8 @@ class BankinstituteController extends BaseController public function check(array $params): void { + Auth::requireAdmin(); + header('Content-Type: application/json'); $analysis = $this->analyzeUpload(); @@ -52,6 +57,8 @@ class BankinstituteController extends BaseController public function import(array $params): void { + Auth::requireAdmin(); + header('Content-Type: application/json'); $analysis = $this->analyzeUpload(); diff --git a/tests/e2e/specs/settings-admin-authorization.spec.js b/tests/e2e/specs/settings-admin-authorization.spec.js index e10e386..77ae021 100644 --- a/tests/e2e/specs/settings-admin-authorization.spec.js +++ b/tests/e2e/specs/settings-admin-authorization.spec.js @@ -1,10 +1,12 @@ // @ts-check -// Fix #200 / #202: Mehrere SettingsController-Endpoints (Logo-Upload, -// Logo-Löschen, LLM-Test, CSV-Bankimport-Formate) prüften +// Fix #200 / #202 / #203: Mehrere SettingsController- und +// BankinstituteController-Endpoints (Logo-Upload, Logo-Löschen, LLM-Test, +// CSV-Bankimport-Formate, Bankinstitute-Verwaltung) prüften // Auth::requireAdmin() nicht, obwohl der Bereich laut Code-Kommentar bei // update() bewusst admin-pflichtig sein soll. Ein normaler "user"-Account // konnte damit das Firmenlogo hochladen/löschen, Bankimport-Formate -// anlegen/ändern/löschen und den LLM-Test auslösen. +// anlegen/ändern/löschen, den LLM-Test auslösen und die systemweite +// Bankinstitute-Referenztabelle (BLZ/BIC) überschreiben. // Dieser Test prüft die Durchsetzung auf HTTP-Ebene, die die PHP-Suite // nicht erreicht (Router-Guard, Auth::requireAdmin()). @@ -79,10 +81,28 @@ test.describe('Autorisierung: Settings-Verwaltungsendpoints nur für Admins (Fix const deleteFormat = await userPage.request.post('/settings/bankimport/1/delete', { headers, form: {} }) expect(deleteFormat.status()).toBe(403) - for (const response of [bankImportGet, uploadLogo, deleteLogo, llmTest, createFormat, suggest, dismiss, updateField, deleteFormat]) { + const bankinstituteGet = await userPage.request.get('/settings/bankinstitute') + expect(bankinstituteGet.status()).toBe(403) + + const bankinstituteCheck = await userPage.request.post('/settings/bankinstitute/check', { + headers, + multipart: { institute_file: { name: 'inst.csv', mimeType: 'text/csv', buffer: Buffer.from('') } }, + }) + expect(bankinstituteCheck.status()).toBe(403) + + const bankinstituteImport = await userPage.request.post('/settings/bankinstitute/import', { + headers, + multipart: { institute_file: { name: 'inst.csv', mimeType: 'text/csv', buffer: Buffer.from('') } }, + }) + expect(bankinstituteImport.status()).toBe(403) + + for (const response of [bankImportGet, uploadLogo, deleteLogo, llmTest, createFormat, suggest, dismiss, updateField, deleteFormat, bankinstituteGet, bankinstituteCheck, bankinstituteImport]) { await expect(response.text()).resolves.toContain('Kein Zugriff') } + const bankinstituteSearch = await userPage.request.get('/bankinstitute/search?q=100') + expect(bankinstituteSearch.status()).toBe(200) + await context.close() } finally { const delForm = row.locator('form[action$="/delete"]')