Merge pull request 'Fix #202 / #203: Admin-Prüfung bei Logo-Upload und BankinstituteController ergänzt' (#204) from fds2610-bot/Steuerkram:main into main

Reviewed-on: https://codeberg.org/fds2610/Steuerkram/pulls/204

Changes.md

@@ -2,6 +2,16 @@
 
 ---
 
+## 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()`.
+- **`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)
 
 - **`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/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();

src/Controllers/SettingsController.php

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

tests/e2e/specs/settings-admin-authorization.spec.js

@@ -1,9 +1,12 @@
 // @ts-check
-// Fix #200: Mehrere SettingsController-Endpoints (Logo, LLM-Test,
-// CSV-Bankimport-Formate) prüften Auth::requireAdmin() nicht, obwohl der
-// Bereich laut Code-Kommentar bei update() bewusst admin-pflichtig sein
-// soll. Ein normaler "user"-Account konnte damit Firmenlogo löschen,
-// Bankimport-Formate anlegen/ändern/löschen und den LLM-Test auslösen.
+// 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, 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()).
 
@@ -42,6 +45,12 @@ test.describe('Autorisierung: Settings-Verwaltungsendpoints nur für Admins (Fix
 			const bankImportGet = await userPage.request.get('/settings/bankimport')
 			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: {} })
 			expect(deleteLogo.status()).toBe(403)
 
@@ -72,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, 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"]')