diff --git a/Changes.md b/Changes.md index bf16a38..b1a2369 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,19 @@ --- +## 2026-08-09 – Fix: Fehlende Autorisierung in der Benutzerverwaltung (#147) + +- `users` hatte kein Rollen-Feld – `Auth::require()` prüfte für alle `/useradmin`-Routen (`GET/POST /useradmin`, `.../{id}`, `.../{id}/delete`, `.../{id}/toggle`) nur eine gültige Session, keine besondere Berechtigung. Jeder eingeloggte Benutzer (auch einer, der nur für die Belegerfassung angelegt wurde) konnte damit beliebige Konten anlegen, das Passwort jedes anderen Benutzers – inklusive des ursprünglichen Admin-Accounts – setzen sowie andere Benutzer löschen oder deaktivieren (Privilege Escalation, CWE-862/CWE-269). +- **`migrations/093_users_role.sql`** (neu): `users.role ENUM('admin','user') NOT NULL DEFAULT 'user'`. Auf Bestandsinstallationen wird der älteste Benutzer (`MIN(id)`) automatisch zu `admin`, damit die Benutzerverwaltung nach dem Update nicht für alle gesperrt ist. +- **`src/Models/User.php`:** `create()` akzeptiert jetzt eine optionale Rolle (Default `user`), neue Methode `setRole()`; `all()`/`find()`/`findByUsername()` liefern das Rollen-Feld mit. +- **`src/Auth.php`:** `login()` legt die Rolle in der Session ab (`$_SESSION['role']`). Neue Methoden `isAdmin()` und `requireAdmin()` (analog zu `require()`, antwortet bei fehlender Admin-Rolle mit HTTP 403). +- **`src/Controllers/LoginController.php`:** Der bei der Ersteinrichtung angelegte erste Benutzer erhält automatisch die Rolle `admin`. +- **`public/index.php`:** Zentraler Guard vor dem Routing – jede Anfrage unter `/useradmin` durchläuft zusätzlich `Auth::requireAdmin()`. +- **`src/Views/layout/nav.php`:** Der Menüpunkt „Benutzer“ wird nur noch für Admins gerendert (rein kosmetisch, ersetzt nicht die serverseitige Prüfung). +- **`scripts/e2e-user.php`:** Der E2E-Testnutzer wird als Admin angelegt bzw. beim erneuten Ausführen auf die Rolle `admin` zurückgesetzt, damit die E2E-Suite weiterhin auf `/useradmin` zugreifen kann. +- **`tests/83_UserRoleTest.php`** (neu): Rollen-Default `user`, explizite Vergabe von `admin`, `setRole()`, `Auth::isAdmin()` anhand der Session. +- **`tests/e2e/specs/useradmin-authorization.spec.js`** (neu): Ein frisch angelegter Benutzer ohne Admin-Rolle sieht den Nav-Link nicht und bekommt beim direkten Aufruf von `/useradmin` HTTP 403. + ## 2026-08-03 – Fix: AfA-Buchungen umgehen das Booking-Modell (#155) - `AssetController::bookAfa()` und `reverseAfaBooking()` setzten ihre `INSERT INTO bookings`-Statements bislang direkt ab statt über `Booking::create()`/`reverse()`. Vier Folgen: Die Sperre eines abgeschlossenen Geschäftsjahres griff nicht, ein zweiter, unabhängiger Nummernkreis (`AFA-NNNNN`) lief neben `BU-YYYY-NNNNNN` auf derselben Spalte, die Nummernvergabe stand außerhalb der Transaktion, und das Storno einer AfA-Buchung wurde immer dem *aktuellen* Kalenderjahr statt dem Jahr der Originalbuchung zugeordnet. diff --git a/docs/funktionen.md b/docs/funktionen.md index af54efb..2141212 100644 --- a/docs/funktionen.md +++ b/docs/funktionen.md @@ -145,4 +145,5 @@ Zuordnungsregeln: [berechnungslogik.md](berechnungslogik.md) - Benutzer anlegen, bearbeiten, deaktivieren und löschen - Session-basierte Authentifizierung mit 24 h Timeout -- Erste Benutzeranlage über Ersteinrichtungs-Dialog beim ersten Aufruf +- Erste Benutzeranlage über Ersteinrichtungs-Dialog beim ersten Aufruf, dieser Benutzer erhält automatisch die Rolle `admin` +- Rollen `admin`/`user`: `/useradmin` (Benutzer anlegen, Passwörter setzen, löschen, deaktivieren) ist ausschließlich Admins vorbehalten (`Auth::requireAdmin()`); der Menüpunkt ist für andere Benutzer ausgeblendet diff --git a/migrations/093_users_role.sql b/migrations/093_users_role.sql new file mode 100644 index 0000000..b75a087 --- /dev/null +++ b/migrations/093_users_role.sql @@ -0,0 +1,13 @@ +-- Migration 093: Rollen-Feld für users (Fix #147 - fehlende Autorisierung +-- in der Benutzerverwaltung, jeder eingeloggte Benutzer konnte bislang +-- beliebige Konten anlegen/löschen/übernehmen). +ALTER TABLE users + ADD COLUMN IF NOT EXISTS role ENUM('admin','user') NOT NULL DEFAULT 'user' AFTER password_hash; + +-- Bestandsinstallationen: den ältesten Benutzer automatisch zum Admin +-- machen, damit die Benutzerverwaltung nach dem Update nicht für alle +-- Bestandsnutzer gesperrt ist. +UPDATE users u +JOIN (SELECT MIN(id) AS min_id FROM users) m ON u.id = m.min_id +SET u.role = 'admin' +WHERE u.role != 'admin'; diff --git a/public/index.php b/public/index.php index 4c19048..0c17376 100644 --- a/public/index.php +++ b/public/index.php @@ -109,6 +109,9 @@ $requestPath = parse_url($_SERVER['REQUEST_URI'] ?? '/', PHP_URL_PATH); if ($requestPath !== '/login') { Auth::require(); } +if (str_starts_with($requestPath ?? '', '/useradmin')) { + Auth::requireAdmin(); +} $router = new Router(); diff --git a/scripts/e2e-user.php b/scripts/e2e-user.php index a7520ec..a750ed7 100644 --- a/scripts/e2e-user.php +++ b/scripts/e2e-user.php @@ -59,10 +59,13 @@ if ($existing) { $id = (int)$existing['id']; User::update($id, $username, $password); User::setActive($id, true); + User::setRole($id, 'admin'); User::resetFailedLogins($id); - echo "Testnutzer \"{$username}\" aktualisiert (Passwort neu gesetzt, aktiv, entsperrt).\n"; + echo "Testnutzer \"{$username}\" aktualisiert (Passwort neu gesetzt, aktiv, entsperrt, Rolle admin).\n"; } else { - User::create($username, $password); + // Rolle admin, damit die E2E-Suite auch die Benutzerverwaltung + // (/useradmin) erreichen kann (Fix #147). + User::create($username, $password, 'admin'); $created = User::findByUsername($username); if (!$created) { fwrite(STDERR, "Fehler: Testnutzer \"{$username}\" konnte nicht angelegt werden.\n"); diff --git a/src/Auth.php b/src/Auth.php index 5dc30fb..3d73702 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -65,12 +65,33 @@ class Auth } } - public static function login(int $userId): void + public static function login(int $userId, string $role = 'user'): void { self::startSession(); session_regenerate_id(true); $_SESSION['user_id'] = $userId; $_SESSION['login_time'] = time(); + $_SESSION['role'] = $role; + } + + /** + * Wie require(), verlangt zusätzlich die Rolle 'admin'. Schützt die + * Benutzerverwaltung vor Privilege Escalation durch normale Benutzer + * (Issue #147) – die Rolle steckt in der Session und wird erst beim + * nächsten Login neu geladen. + */ + public static function requireAdmin(): void + { + self::require(); + if (!self::isAdmin()) { + http_response_code(403); + exit('Kein Zugriff.'); + } + } + + public static function isAdmin(): bool + { + return ($_SESSION['role'] ?? '') === 'admin'; } public static function logout(): void diff --git a/src/Controllers/LoginController.php b/src/Controllers/LoginController.php index cc63130..31123fa 100644 --- a/src/Controllers/LoginController.php +++ b/src/Controllers/LoginController.php @@ -51,9 +51,9 @@ class LoginController return; } - User::create($username, $password); + User::create($username, $password, 'admin'); $user = User::findByUsername($username); - Auth::login((int)$user['id']); + Auth::login((int)$user['id'], (string)$user['role']); View::redirect('/'); return; } @@ -102,7 +102,7 @@ class LoginController } User::resetFailedLogins((int)$user['id']); - Auth::login((int)$user['id']); + Auth::login((int)$user['id'], (string)$user['role']); return null; } } diff --git a/src/Models/User.php b/src/Models/User.php index 764eccb..43ca336 100644 --- a/src/Models/User.php +++ b/src/Models/User.php @@ -14,14 +14,14 @@ class User public static function all(): array { return Database::fetchAll( - 'SELECT id, username, active, created_at FROM users ORDER BY username' + 'SELECT id, username, role, active, created_at FROM users ORDER BY username' ); } public static function find(int $id): array|false { return Database::fetchOne( - 'SELECT id, username, active, created_at FROM users WHERE id = ?', + 'SELECT id, username, role, active, created_at FROM users WHERE id = ?', [$id] ); } @@ -29,17 +29,17 @@ class User public static function findByUsername(string $username): array|false { return Database::fetchOne( - 'SELECT id, username, password_hash, active, failed_attempts, locked_until FROM users WHERE username = ?', + 'SELECT id, username, password_hash, role, active, failed_attempts, locked_until FROM users WHERE username = ?', [$username] ); } - public static function create(string $username, string $password): void + public static function create(string $username, string $password, string $role = 'user'): void { $hash = password_hash($password, PASSWORD_BCRYPT); Database::execute( - 'INSERT INTO users (username, password_hash) VALUES (?, ?)', - [$username, $hash] + 'INSERT INTO users (username, password_hash, role) VALUES (?, ?, ?)', + [$username, $hash, $role] ); } @@ -72,6 +72,14 @@ class User ); } + public static function setRole(int $id, string $role): void + { + Database::execute( + 'UPDATE users SET role = ? WHERE id = ?', + [$role, $id] + ); + } + /** * Zählt einen Fehlversuch. Ab LOCK_THRESHOLD Fehlversuchen wird das Konto * für ein mit jedem weiteren Fehlversuch steigendes Zeitfenster gesperrt. diff --git a/src/Views/layout/nav.php b/src/Views/layout/nav.php index 0372c3a..efbec6e 100644 --- a/src/Views/layout/nav.php +++ b/src/Views/layout/nav.php @@ -80,10 +80,12 @@ function navActive(string $prefix, string $current): string {
Verwaltung