Fix #215: Session deaktivierter/gelöschter Benutzer sofort entziehen
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
870f0f2118
commit
a95c4efa21
5 changed files with 161 additions and 6 deletions
|
|
@ -2,6 +2,13 @@
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 2026-09-29 – Sessions deaktivierter/gelöschter Benutzer sofort ungültig (#215)
|
||||||
|
|
||||||
|
- **`src/Auth.php`:** `Auth::require()` lud den Benutzer bislang nie neu aus der DB – nur `$_SESSION['user_id']` und das 24h-Alter der Session zählten. Deaktivierte, gelöschte oder im Passwort geänderte Konten behielten dadurch bis zu 24h vollen Zugriff, auch auf Admin-Funktionen. `require()` lädt jetzt bei jedem Request `User::findAuthData()` nach und verwirft die Session (Logout + Redirect `/login`), sobald der Benutzer fehlt, `active = 0` ist oder ein in der Session hinterlegter Passwort-Fingerprint nicht mehr zum aktuellen `password_hash` passt. Die Rolle wird dabei jedes Mal frisch aus der DB in die Session übernommen, damit auch eine Rollenänderung sofort greift statt erst beim nächsten Login. Der Fingerprint (`Auth::passwordFingerprint()`, SHA-256 über den bcrypt-Hash) wird beim Login in `$_SESSION['pwd_fp']` abgelegt – Bestandssessions ohne diesen Wert werden beim nächsten Request einmalig abgemeldet.
|
||||||
|
- **`src/Models/User.php`:** neue Methode `User::findAuthData()` liefert den schlanken Datensatz (`id, role, active, password_hash`) für die Prüfung in `Auth::require()`.
|
||||||
|
- **`src/Controllers/LoginController.php`:** `Auth::login()` erhält jetzt zusätzlich den `password_hash`, um den Fingerprint zu setzen.
|
||||||
|
- **`tests/100_SessionRevocationTest.php`** (neu): prüft `Auth::passwordFingerprint()`/`Auth::isSessionRevoked()` gegen Deaktivierung, Löschung, Reaktivierung und Passwortwechsel.
|
||||||
|
|
||||||
## 2026-09-25 – Stored XSS im KI-Vorschlag-Badge behoben (#213)
|
## 2026-09-25 – Stored XSS im KI-Vorschlag-Badge behoben (#213)
|
||||||
|
|
||||||
- **`public/static/js/beleg-ki.js`:** `_showKiBadgeDone()` baut das Badge jetzt per `createElement`/`textContent` statt `innerHTML`. Kontobezeichnungen (aus `option.text`, also bereits dekodiert), Buchungstyp, USt-Satz und Lieferant landen nur noch als Text im DOM – eine Kontobezeichnung wie `Bank<img src=x onerror=…>` wird nicht mehr ausgeführt. Der „Neu generieren"-Button wird per `addEventListener` verdrahtet, die Optik liegt in neuen `.ki-badge-*`-Klassen in `app.css`. KI-Werte werden außerdem nicht mehr in CSS-Selektoren eingesetzt (Option/Radio werden per Wertvergleich gesucht).
|
- **`public/static/js/beleg-ki.js`:** `_showKiBadgeDone()` baut das Badge jetzt per `createElement`/`textContent` statt `innerHTML`. Kontobezeichnungen (aus `option.text`, also bereits dekodiert), Buchungstyp, USt-Satz und Lieferant landen nur noch als Text im DOM – eine Kontobezeichnung wie `Bank<img src=x onerror=…>` wird nicht mehr ausgeführt. Der „Neu generieren"-Button wird per `addEventListener` verdrahtet, die Optik liegt in neuen `.ki-badge-*`-Klassen in `app.css`. KI-Werte werden außerdem nicht mehr in CSS-Selektoren eingesetzt (Option/Radio werden per Wertvergleich gesucht).
|
||||||
|
|
|
||||||
59
src/Auth.php
59
src/Auth.php
|
|
@ -4,6 +4,8 @@ declare(strict_types=1);
|
||||||
|
|
||||||
namespace Steuerkram;
|
namespace Steuerkram;
|
||||||
|
|
||||||
|
use Steuerkram\Models\User;
|
||||||
|
|
||||||
class Auth
|
class Auth
|
||||||
{
|
{
|
||||||
private const SESSION_LIFETIME = 86400; // 24 h absolutes Timeout
|
private const SESSION_LIFETIME = 86400; // 24 h absolutes Timeout
|
||||||
|
|
@ -51,6 +53,15 @@ class Auth
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Prüft bei jedem Request per DB-Abfrage, ob Benutzer-ID und Session noch
|
||||||
|
* gültig sind. Nicht mehr nur die 24h-Laufzeit zählt: ein zwischenzeitlich
|
||||||
|
* deaktiviertes oder gelöschtes Konto sowie ein per Admin geänderter
|
||||||
|
* Passwort-Hash entziehen eine bereits laufende Session sofort statt erst
|
||||||
|
* beim nächsten Login (Fix #215). Die Rolle wird dabei jedes Mal frisch
|
||||||
|
* aus der DB in die Session übernommen, damit auch eine Rollenänderung
|
||||||
|
* sofort greift.
|
||||||
|
*/
|
||||||
public static function require(): void
|
public static function require(): void
|
||||||
{
|
{
|
||||||
self::startSession();
|
self::startSession();
|
||||||
|
|
@ -59,19 +70,59 @@ class Auth
|
||||||
$loginTime = $_SESSION['login_time'] ?? 0;
|
$loginTime = $_SESSION['login_time'] ?? 0;
|
||||||
|
|
||||||
if (!$userId || (time() - (int)$loginTime) > self::SESSION_LIFETIME) {
|
if (!$userId || (time() - (int)$loginTime) > self::SESSION_LIFETIME) {
|
||||||
$_SESSION = [];
|
self::destroyAndRedirectToLogin();
|
||||||
session_destroy();
|
|
||||||
View::redirect('/login');
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$user = User::findAuthData((int)$userId);
|
||||||
|
if (self::isSessionRevoked($user, (string)($_SESSION['pwd_fp'] ?? ''))) {
|
||||||
|
self::destroyAndRedirectToLogin();
|
||||||
|
}
|
||||||
|
|
||||||
|
$_SESSION['role'] = $user['role'];
|
||||||
}
|
}
|
||||||
|
|
||||||
public static function login(int $userId, string $role = 'user'): void
|
private static function destroyAndRedirectToLogin(): void
|
||||||
|
{
|
||||||
|
$_SESSION = [];
|
||||||
|
session_destroy();
|
||||||
|
View::redirect('/login');
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reine Entscheidungslogik ohne Session-/DB-Zugriff, damit sie sich
|
||||||
|
* isoliert testen lässt (siehe cookieParams()). $user ist der per
|
||||||
|
* User::findAuthData() geladene Datensatz, $sessionFingerprint der beim
|
||||||
|
* Login in der Session hinterlegte Passwort-Fingerprint.
|
||||||
|
*/
|
||||||
|
public static function isSessionRevoked(array|false $user, string $sessionFingerprint): bool
|
||||||
|
{
|
||||||
|
if ($user === false || empty($user['active'])) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return !hash_equals(self::passwordFingerprint((string)$user['password_hash']), $sessionFingerprint);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Kurzer, nicht umkehrbarer Fingerprint des Passwort-Hashes: wird in der
|
||||||
|
* Session hinterlegt (nicht der Hash selbst) und bei jedem Request gegen
|
||||||
|
* den aktuellen DB-Wert geprüft. Ändert ein Admin das Passwort eines
|
||||||
|
* Benutzers, weicht der Fingerprint ab und die alte Session wird ungültig
|
||||||
|
* (Fix #215), statt bis zu 24h weiter gültig zu bleiben.
|
||||||
|
*/
|
||||||
|
public static function passwordFingerprint(string $passwordHash): string
|
||||||
|
{
|
||||||
|
return hash('sha256', $passwordHash);
|
||||||
|
}
|
||||||
|
|
||||||
|
public static function login(int $userId, string $role, string $passwordHash): void
|
||||||
{
|
{
|
||||||
self::startSession();
|
self::startSession();
|
||||||
session_regenerate_id(true);
|
session_regenerate_id(true);
|
||||||
$_SESSION['user_id'] = $userId;
|
$_SESSION['user_id'] = $userId;
|
||||||
$_SESSION['login_time'] = time();
|
$_SESSION['login_time'] = time();
|
||||||
$_SESSION['role'] = $role;
|
$_SESSION['role'] = $role;
|
||||||
|
$_SESSION['pwd_fp'] = self::passwordFingerprint($passwordHash);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
||||||
|
|
@ -53,7 +53,7 @@ class LoginController
|
||||||
|
|
||||||
User::create($username, $password, 'admin');
|
User::create($username, $password, 'admin');
|
||||||
$user = User::findByUsername($username);
|
$user = User::findByUsername($username);
|
||||||
Auth::login((int)$user['id'], (string)$user['role']);
|
Auth::login((int)$user['id'], (string)$user['role'], (string)$user['password_hash']);
|
||||||
View::redirect('/');
|
View::redirect('/');
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -102,7 +102,7 @@ class LoginController
|
||||||
}
|
}
|
||||||
|
|
||||||
User::resetFailedLogins((int)$user['id']);
|
User::resetFailedLogins((int)$user['id']);
|
||||||
Auth::login((int)$user['id'], (string)$user['role']);
|
Auth::login((int)$user['id'], (string)$user['role'], (string)$user['password_hash']);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -34,6 +34,20 @@ class User
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Schlanker Datensatz für Auth::require(): wird bei jedem Request geladen,
|
||||||
|
* um Sperrung/Löschung/Rollenänderung/Passwortwechsel sofort wirksam
|
||||||
|
* werden zu lassen (Fix #215), statt nur der bis zu 24h alten Session zu
|
||||||
|
* vertrauen.
|
||||||
|
*/
|
||||||
|
public static function findAuthData(int $id): array|false
|
||||||
|
{
|
||||||
|
return Database::fetchOne(
|
||||||
|
'SELECT id, role, active, password_hash FROM users WHERE id = ?',
|
||||||
|
[$id]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
public static function create(string $username, string $password, string $role = 'user'): void
|
public static function create(string $username, string $password, string $role = 'user'): void
|
||||||
{
|
{
|
||||||
$hash = password_hash($password, PASSWORD_BCRYPT);
|
$hash = password_hash($password, PASSWORD_BCRYPT);
|
||||||
|
|
|
||||||
83
tests/100_SessionRevocationTest.php
Normal file
83
tests/100_SessionRevocationTest.php
Normal file
|
|
@ -0,0 +1,83 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
use Steuerkram\Auth;
|
||||||
|
use Steuerkram\Models\User;
|
||||||
|
|
||||||
|
class SessionRevocationTest extends TestCase
|
||||||
|
{
|
||||||
|
protected string $testName = 'Test 100: Sitzungsentzug bei Deaktivierung/Löschung/Passwortwechsel (Fix #215)';
|
||||||
|
|
||||||
|
public function run(): void
|
||||||
|
{
|
||||||
|
User::create('sessionrevoke-user', 'password123');
|
||||||
|
$user = User::findByUsername('sessionrevoke-user');
|
||||||
|
$id = (int)$user['id'];
|
||||||
|
|
||||||
|
$fingerprint = Auth::passwordFingerprint((string)$user['password_hash']);
|
||||||
|
$this->assertEquals(
|
||||||
|
$fingerprint,
|
||||||
|
Auth::passwordFingerprint((string)$user['password_hash']),
|
||||||
|
'passwordFingerprint() ist deterministisch für denselben Hash'
|
||||||
|
);
|
||||||
|
$this->assert(
|
||||||
|
$fingerprint !== Auth::passwordFingerprint('$2y$anderer-hash'),
|
||||||
|
'passwordFingerprint() unterscheidet unterschiedliche Hashes'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Frisch angemeldete Session: aktiver Benutzer, passender Fingerprint -> gültig
|
||||||
|
$authData = User::findAuthData($id);
|
||||||
|
$this->assert(
|
||||||
|
!Auth::isSessionRevoked($authData, $fingerprint),
|
||||||
|
'Aktiver Benutzer mit passendem Passwort-Fingerprint gilt als gültige Session'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Fehlender/falscher Fingerprint (z. B. Session von vor dem Fix, oder nach Passwortänderung)
|
||||||
|
$this->assert(
|
||||||
|
Auth::isSessionRevoked($authData, ''),
|
||||||
|
'Fehlender Passwort-Fingerprint in der Session entzieht den Zugriff'
|
||||||
|
);
|
||||||
|
$this->assert(
|
||||||
|
Auth::isSessionRevoked($authData, 'falscher-fingerprint'),
|
||||||
|
'Abweichender Passwort-Fingerprint entzieht den Zugriff'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Admin ändert das Passwort -> alter Fingerprint wird ungültig
|
||||||
|
User::update($id, 'sessionrevoke-user', 'einneuespasswort123');
|
||||||
|
$afterPasswordChange = User::findAuthData($id);
|
||||||
|
$this->assert(
|
||||||
|
Auth::isSessionRevoked($afterPasswordChange, $fingerprint),
|
||||||
|
'Passwortänderung entzieht bereits bestehende Sessions (alter Fingerprint)'
|
||||||
|
);
|
||||||
|
$newFingerprint = Auth::passwordFingerprint((string)$afterPasswordChange['password_hash']);
|
||||||
|
$this->assert(
|
||||||
|
!Auth::isSessionRevoked($afterPasswordChange, $newFingerprint),
|
||||||
|
'Nach der Passwortänderung ist eine Session mit dem neuen Fingerprint wieder gültig'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Admin deaktiviert den Benutzer -> Session wird ungültig, obwohl Fingerprint passt
|
||||||
|
User::setActive($id, false);
|
||||||
|
$afterDeactivation = User::findAuthData($id);
|
||||||
|
$this->assert(
|
||||||
|
Auth::isSessionRevoked($afterDeactivation, $newFingerprint),
|
||||||
|
'Deaktivierung entzieht den Zugriff, obwohl der Passwort-Fingerprint noch passt'
|
||||||
|
);
|
||||||
|
|
||||||
|
User::setActive($id, true);
|
||||||
|
$afterReactivation = User::findAuthData($id);
|
||||||
|
$this->assert(
|
||||||
|
!Auth::isSessionRevoked($afterReactivation, $newFingerprint),
|
||||||
|
'Reaktivierung macht die Session mit passendem Fingerprint wieder gültig'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Admin löscht den Benutzer -> Session wird ungültig
|
||||||
|
User::delete($id);
|
||||||
|
$afterDelete = User::findAuthData($id);
|
||||||
|
$this->assert($afterDelete === false, 'Gelöschter Benutzer wird von findAuthData() nicht mehr gefunden');
|
||||||
|
$this->assert(
|
||||||
|
Auth::isSessionRevoked($afterDelete, $newFingerprint),
|
||||||
|
'Löschung entzieht den Zugriff'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue