Merge pull request 'Fix #206: Admin-Prüfung bei GET /settings ergänzt' (#207) from fds2610-bot/Steuerkram:main into main
Reviewed-on: https://codeberg.org/fds2610/Steuerkram/pulls/207
This commit is contained in:
commit
597ce4f420
4 changed files with 14 additions and 2 deletions
|
|
@ -2,6 +2,11 @@
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 2026-09-20 – Fehlende Admin-Prüfung bei GET /settings (#206)
|
||||||
|
|
||||||
|
- **`src/Controllers/SettingsController.php`:** `show()` prüfte keine Rolle, sodass jeder eingeloggte Nicht-Admin IBAN/BIC, Steuernummer, Nextcloud- und SMTP-Daten einsehen konnte. `Auth::requireAdmin()` ergänzt.
|
||||||
|
- **`tests/e2e/specs/settings-admin-authorization.spec.js`:** Fall für `GET /settings` (403) ergänzt; die CSRF-Token-Beschaffung in diesem und dem SMTP-Spec läuft nun über `/` statt `/settings`.
|
||||||
|
|
||||||
## 2026-08-26 – Fehlende Admin-Prüfung im BankinstituteController (#203)
|
## 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.
|
- **`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.
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,10 @@ class SettingsController extends BaseController
|
||||||
{
|
{
|
||||||
public function show(array $params): void
|
public function show(array $params): void
|
||||||
{
|
{
|
||||||
|
// Die Seite zeigt IBAN/BIC, Steuernummer, Nextcloud- und SMTP-Daten
|
||||||
|
// (Issue #206) – wie alle übrigen Settings-Endpoints nur für Admins.
|
||||||
|
Auth::requireAdmin();
|
||||||
|
|
||||||
$settings = Setting::get();
|
$settings = Setting::get();
|
||||||
$versionFile = dirname(__DIR__) . '/version.txt';
|
$versionFile = dirname(__DIR__) . '/version.txt';
|
||||||
$version = file_exists($versionFile) ? trim(file_get_contents($versionFile)) : '';
|
$version = file_exists($versionFile) ? trim(file_get_contents($versionFile)) : '';
|
||||||
|
|
|
||||||
|
|
@ -36,12 +36,15 @@ test.describe('Autorisierung: Settings-Verwaltungsendpoints nur für Admins (Fix
|
||||||
await userPage.click('form[action="/login"] button[type="submit"]')
|
await userPage.click('form[action="/login"] button[type="submit"]')
|
||||||
await userPage.waitForURL(url => !url.pathname.startsWith('/login'))
|
await userPage.waitForURL(url => !url.pathname.startsWith('/login'))
|
||||||
|
|
||||||
await userPage.goto('/settings')
|
await userPage.goto('/')
|
||||||
const csrfToken = await userPage.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.content || '')
|
const csrfToken = await userPage.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.content || '')
|
||||||
expect(csrfToken).not.toBe('')
|
expect(csrfToken).not.toBe('')
|
||||||
|
|
||||||
const headers = { 'X-CSRF-Token': csrfToken }
|
const headers = { 'X-CSRF-Token': csrfToken }
|
||||||
|
|
||||||
|
const settingsGet = await userPage.request.get('/settings')
|
||||||
|
expect(settingsGet.status()).toBe(403)
|
||||||
|
|
||||||
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)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -35,7 +35,7 @@ test.describe('Autorisierung: SMTP-Einstellungen nur für Admins (Fix #193)', ()
|
||||||
// GET /settings bleibt für alle eingeloggten Benutzer erreichbar
|
// GET /settings bleibt für alle eingeloggten Benutzer erreichbar
|
||||||
// (Firmendaten sind kein Admin-Feature) – nur die SMTP-mutierenden
|
// (Firmendaten sind kein Admin-Feature) – nur die SMTP-mutierenden
|
||||||
// Endpunkte werden geschützt.
|
// Endpunkte werden geschützt.
|
||||||
await userPage.goto('/settings')
|
await userPage.goto('/')
|
||||||
const csrfToken = await userPage.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.content || '')
|
const csrfToken = await userPage.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.content || '')
|
||||||
expect(csrfToken).not.toBe('')
|
expect(csrfToken).not.toBe('')
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue