Security hardening: CSRF, rate limiting, session/password policy, audit log
Fixes critical pre-existing issues found during review: bulk_action.php had no auth check at all (unauthenticated download/delete of any qrcode) and built a table name from unwhitelisted user input (SQL injection); the QR generator classes wrote files from unvalidated filename/format, allowing path traversal and arbitrary file writes. Also pins chillerlan/php-qrcode to 5.0.5 since master now requires PHP 8.4, breaking the PHP 8.3 build. - CSRF tokens on all POST forms and the bulk_action.php JSON endpoint - Login rate limiting (5 attempts / 15 min) via new login_attempts table - Hardened sessions: httponly/samesite cookies, 30 min idle timeout, session regeneration on login - Forced password change for the default superadmin/superadmin account - Server-side validation in Users/DynamicQrcode/Qrcode classes - Audit log table for auth, user, and qrcode actions - Checked-in db schema (db/init.sql, migrations/) instead of relying on an opaque prebuilt db image - Production docker-compose with Nginx + php-fpm instead of the PHP dev server
This commit is contained in:
+59
-11
@@ -3,6 +3,8 @@ require_once 'config/config.php';
|
||||
|
||||
class Users
|
||||
{
|
||||
const ALLOWED_TYPES = ['super', 'admin'];
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
@@ -10,6 +12,25 @@ class Users
|
||||
{
|
||||
}
|
||||
|
||||
/**
|
||||
* Server-side validatie van username/type. Geeft een foutmelding terug (string) of null als geldig.
|
||||
*/
|
||||
private function validateUsernameAndType($username, $type) {
|
||||
if (!is_string($username) || strlen($username) < 3 || strlen($username) > 50) {
|
||||
return 'Username must be between 3 and 50 characters.';
|
||||
}
|
||||
|
||||
if (!preg_match('/^[a-zA-Z0-9._-]+$/', $username)) {
|
||||
return 'Username may only contain letters, numbers, dots, underscores and hyphens.';
|
||||
}
|
||||
|
||||
if (!in_array($type, self::ALLOWED_TYPES, true)) {
|
||||
return 'Invalid user type.';
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
@@ -54,6 +75,15 @@ class Users
|
||||
public function addUser($input_data) {
|
||||
$db = getDbInstance();
|
||||
|
||||
$validation_error = $this->validateUsernameAndType($input_data['username'] ?? '', $input_data['type'] ?? '');
|
||||
if ($validation_error !== null) {
|
||||
$this->failure($validation_error, 'Location: user.php');
|
||||
}
|
||||
|
||||
if (!isset($input_data['password']) || strlen($input_data['password']) < 10) {
|
||||
$this->failure('Password must be at least 10 characters long.', 'Location: user.php');
|
||||
}
|
||||
|
||||
$data_to_db["username"] = $input_data["username"];
|
||||
$data_to_db['password'] = password_hash($input_data['password'], PASSWORD_DEFAULT);
|
||||
$data_to_db["type"] = $input_data["type"];
|
||||
@@ -66,8 +96,10 @@ class Users
|
||||
|
||||
$last_id = $db->insert('users', $data_to_db);
|
||||
|
||||
if ($last_id)
|
||||
if ($last_id) {
|
||||
audit_log('user_created', 'user', $last_id);
|
||||
$this->success('User added successfully');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -77,28 +109,43 @@ class Users
|
||||
public function editUser($input_data) {
|
||||
$db = getDbInstance();
|
||||
|
||||
$query_string = http_build_query(array(
|
||||
'id' => $input_data["id"],
|
||||
'edit' => "true",
|
||||
));
|
||||
|
||||
$validation_error = $this->validateUsernameAndType($input_data['username'] ?? '', $input_data['type'] ?? '');
|
||||
if ($validation_error !== null) {
|
||||
$this->failure($validation_error, 'Location: user.php?'.$query_string);
|
||||
}
|
||||
|
||||
if (isset($input_data['password']) && strlen($input_data['password']) > 0 && strlen($input_data['password']) < 10) {
|
||||
$this->failure('Password must be at least 10 characters long.', 'Location: user.php?'.$query_string);
|
||||
}
|
||||
|
||||
$db->where('username', $input_data['username']);
|
||||
$db->where('id', $input_data["id"], '!=');
|
||||
$row = $db->getOne('users');
|
||||
|
||||
if (!empty($row['username'])) {
|
||||
$query_string = http_build_query(array(
|
||||
'id' => $input_data["id"],
|
||||
'edit' => "true",
|
||||
));
|
||||
$this->failure('Username already exists', 'Location: user.php?'.$query_string);
|
||||
}
|
||||
|
||||
$data_to_db["username"] = $input_data["username"];
|
||||
$data_to_db['password'] = password_hash($input_data['password'], PASSWORD_DEFAULT);
|
||||
$data_to_db["type"] = $input_data["type"];
|
||||
|
||||
// Alleen wachtwoord overschrijven als er een nieuwe waarde is opgegeven.
|
||||
if (!empty($input_data['password'])) {
|
||||
$data_to_db['password'] = password_hash($input_data['password'], PASSWORD_DEFAULT);
|
||||
}
|
||||
|
||||
$db->where('id', $input_data["id"]);
|
||||
$stat = $db->update('users', $data_to_db);
|
||||
|
||||
if ($stat)
|
||||
|
||||
if ($stat) {
|
||||
audit_log('user_updated', 'user', $input_data['id']);
|
||||
$this->success('User updated successfully!');
|
||||
else
|
||||
} else
|
||||
$this->failure('Failed to update User: ' . $db->getLastError());
|
||||
}
|
||||
|
||||
@@ -116,9 +163,10 @@ class Users
|
||||
$db->where('id', $id);
|
||||
$stat = $db->delete('users');
|
||||
|
||||
if ($stat)
|
||||
if ($stat) {
|
||||
audit_log('user_deleted', 'user', $id);
|
||||
$this->info('User deleted successfully!');
|
||||
else
|
||||
} else
|
||||
$this->failure('Unable to delete user');
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user