From 8655258ac3acee9fe768ddf87a47cebc1418bab0 Mon Sep 17 00:00:00 2001 From: Yasen Pramatarov Date: Mon, 14 Apr 2025 10:39:58 +0300 Subject: [PATCH] Standartizes $userId as user ID variable in whole app --- app/classes/config.php | 6 +- app/classes/log.php | 12 +-- app/classes/user.php | 92 +++++++++---------- app/pages/agents.php | 2 +- app/pages/config.php | 12 +-- app/pages/credentials.php | 17 ++-- app/pages/login.php | 7 +- app/pages/logs.php | 10 +- app/pages/profile.php | 14 +-- app/pages/register.php | 4 +- app/pages/security.php | 26 +++--- app/pages/settings.php | 4 +- app/templates/config.php | 2 +- app/templates/page-sidebar.php | 12 +-- app/templates/security.php | 12 +-- app/templates/settings.php | 2 +- public_html/index.php | 13 ++- tests/Feature/Security/CsrfProtectionTest.php | 2 +- 18 files changed, 125 insertions(+), 124 deletions(-) diff --git a/app/classes/config.php b/app/classes/config.php index 9ccb278..6acd729 100644 --- a/app/classes/config.php +++ b/app/classes/config.php @@ -16,7 +16,7 @@ class Config { * @return array Returns an array with 'success' and 'updated' keys on success, or 'success' and 'error' keys on failure. */ public function editConfigFile($updatedConfig, $config_file) { - global $logObject, $user_id; + global $logObject, $userId; $allLogs = []; $updated = []; @@ -140,7 +140,7 @@ class Config { } if (!empty($allLogs)) { - $logObject->insertLog($user_id, implode("\n", $allLogs), 'system'); + $logObject->insertLog($userId, implode("\n", $allLogs), 'system'); } return [ @@ -148,7 +148,7 @@ class Config { 'updated' => $updated ]; } catch (Exception $e) { - $logObject->insertLog($user_id, "Config update error: " . $e->getMessage(), 'system'); + $logObject->insertLog($userId, "Config update error: " . $e->getMessage(), 'system'); return [ 'success' => false, 'error' => $e->getMessage() diff --git a/app/classes/log.php b/app/classes/log.php index 8391e0a..03ae1d5 100644 --- a/app/classes/log.php +++ b/app/classes/log.php @@ -24,13 +24,13 @@ class Log { /** * Insert a log event into the database. * - * @param int $user_id The ID of the user associated with the log event. + * @param int $userId The ID of the user associated with the log event. * @param string $message The log message to insert. * @param string $scope The scope of the log event (e.g., 'user', 'system'). Default is 'user'. * * @return bool|string True on success, or an error message on failure. */ - public function insertLog($user_id, $message, $scope='user') { + public function insertLog($userId, $message, $scope='user') { try { $sql = 'INSERT INTO logs (user_id, scope, message) @@ -39,7 +39,7 @@ class Log { $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ':scope' => $scope, ':message' => $message, ]); @@ -54,7 +54,7 @@ class Log { /** * Retrieve log entries from the database. * - * @param int $user_id The ID of the user whose logs are being retrieved. + * @param int $userId The ID of the user whose logs are being retrieved. * @param string $scope The scope of the logs ('user' or 'system'). * @param int $offset The offset for pagination. Default is 0. * @param int $items_per_page The number of log entries to retrieve per page. Default is no limit. @@ -62,7 +62,7 @@ class Log { * * @return array An array of log entries. */ - public function readLog($user_id, $scope, $offset=0, $items_per_page='', $filters=[]) { + public function readLog($userId, $scope, $offset=0, $items_per_page='', $filters=[]) { $params = []; $where_clauses = []; @@ -74,7 +74,7 @@ class Log { // Add scope condition if ($scope === 'user') { $where_clauses[] = 'l.user_id = :user_id'; - $params[':user_id'] = $user_id; + $params[':user_id'] = $userId; } // Add time range filters if specified diff --git a/app/classes/user.php b/app/classes/user.php index 96427d5..71d329a 100644 --- a/app/classes/user.php +++ b/app/classes/user.php @@ -93,9 +93,9 @@ class User { /** * Logs in a user by verifying credentials. * - * @param string $username The username of the user. - * @param string $password The password of the user. - * @param string $twoFactorCode Optional. The 2FA code if 2FA is enabled. + * @param string $username The username of the user. + * @param string $password The password of the user. + * @param string $twoFactorCode Optional. The 2FA code if 2FA is enabled. * * @return array Login result with status and any necessary data */ @@ -180,11 +180,11 @@ class User { /** * Fetches user details by user ID. * - * @param int $user_id The user ID. + * @param int $userId The user ID. * * @return array|null User details or null if not found. */ - public function getUserDetails($user_id) { + public function getUserDetails($userId) { $sql = 'SELECT um.*, u.username @@ -197,7 +197,7 @@ class User { $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ]); return $query->fetchAll(PDO::FETCH_ASSOC); @@ -208,19 +208,19 @@ class User { /** * Grants a user a specific right. * - * @param int $user_id The user ID. - * @param int $right_id The right ID to grant. + * @param int $userId The user ID. + * @param int $right_id The right ID to grant. * * @return void */ - public function addUserRight($user_id, $right_id) { + public function addUserRight($userId, $right_id) { $sql = 'INSERT INTO users_rights (user_id, right_id) VALUES (:user_id, :right_id)'; $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ':right_id' => $right_id, ]); } @@ -229,12 +229,12 @@ class User { /** * Revokes a specific right from a user. * - * @param int $user_id The user ID. - * @param int $right_id The right ID to revoke. + * @param int $userId The user ID. + * @param int $right_id The right ID to revoke. * * @return void */ - public function removeUserRight($user_id, $right_id) { + public function removeUserRight($userId, $right_id) { $sql = 'DELETE FROM users_rights WHERE user_id = :user_id @@ -242,7 +242,7 @@ class User { right_id = :right_id'; $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ':right_id' => $right_id, ]); } @@ -270,11 +270,11 @@ class User { /** * Retrieves the rights assigned to a specific user. * - * @param int $user_id The user ID. + * @param int $userId The user ID. * * @return array List of user rights. */ - public function getUserRights($user_id) { + public function getUserRights($userId) { $sql = 'SELECT u.id AS user_id, r.id AS right_id, @@ -290,7 +290,7 @@ class User { $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ]); $result = $query->fetchAll(PDO::FETCH_ASSOC); @@ -299,7 +299,7 @@ class User { $specialEntries = []; // user 1 is always superuser - if ($user_id == 1) { + if ($userId == 1) { $specialEntries = [ [ 'user_id' => 1, @@ -309,7 +309,7 @@ class User { ]; // user 2 is always demo - } elseif ($user_id == 2) { + } elseif ($userId == 2) { $specialEntries = [ [ 'user_id' => 2, @@ -333,17 +333,17 @@ class User { /** * Check if the user has a specific right. * - * @param int $user_id The user ID. - * @param string $right_name The human-readable name of the user right. + * @param int $userId The user ID. + * @param string $right_name The human-readable name of the user right. * * @return bool True if the user has the right, false otherwise. */ - function hasRight($user_id, $right_name) { - $userRights = $this->getUserRights($user_id); + function hasRight($userId, $right_name) { + $userRights = $this->getUserRights($userId); $userHasRight = false; // superuser always has all the rights - if ($user_id === 1) { + if ($userId === 1) { $userHasRight = true; } @@ -362,8 +362,8 @@ class User { /** * Updates a user's metadata in the database. * - * @param int $user_id The ID of the user to update. - * @param array $updatedUser An associative array containing updated user data: + * @param int $userId The ID of the user to update. + * @param array $updatedUser An associative array containing updated user data: * - 'name' (string): The updated name of the user. * - 'email' (string): The updated email of the user. * - 'timezone' (string): The updated timezone of the user. @@ -371,7 +371,7 @@ class User { * * @return bool|string Returns true if the update is successful, or an error message if an exception occurs. */ - public function editUser($user_id, $updatedUser) { + public function editUser($userId, $updatedUser) { try { $sql = 'UPDATE users_meta SET name = :name, @@ -381,7 +381,7 @@ class User { WHERE user_id = :user_id'; $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ':name' => $updatedUser['name'], ':email' => $updatedUser['email'], ':timezone' => $updatedUser['timezone'], @@ -400,12 +400,12 @@ class User { /** * Removes a user's avatar from the database and deletes the associated file. * - * @param int $user_id The ID of the user whose avatar is being removed. - * @param string $old_avatar Optional. The file path of the current avatar to delete. Default is an empty string. + * @param int $userId The ID of the user whose avatar is being removed. + * @param string $old_avatar Optional. The file path of the current avatar to delete. Default is an empty string. * * @return bool|string Returns true if the avatar is successfully removed, or an error message if an exception occurs. */ - public function removeAvatar($user_id, $old_avatar = '') { + public function removeAvatar($userId, $old_avatar = '') { try { // remove from database $sql = 'UPDATE users_meta SET @@ -413,7 +413,7 @@ class User { WHERE user_id = :user_id'; $query = $this->db->prepare($sql); $query->execute([ - ':user_id' => $user_id, + ':user_id' => $userId, ]); // delete the old avatar file @@ -433,14 +433,14 @@ class User { /** * Updates a user's avatar by uploading a new file and saving its path in the database. * - * @param int $user_id The ID of the user whose avatar is being updated. - * @param array $avatar_file The uploaded avatar file from the $_FILES array. - * Should include 'tmp_name', 'name', 'error', etc. - * @param string $avatars_path The directory path where avatar files should be saved. + * @param int $userId The ID of the user whose avatar is being updated. + * @param array $avatar_file The uploaded avatar file from the $_FILES array. + * Should include 'tmp_name', 'name', 'error', etc. + * @param string $avatars_path The directory path where avatar files should be saved. * * @return bool|string Returns true if the avatar is successfully updated, or an error message if an exception occurs. */ - public function changeAvatar($user_id, $avatar_file, $avatars_path) { + public function changeAvatar($userId, $avatar_file, $avatars_path) { try { // check if the file was uploaded if (isset($avatar_file) && $avatar_file['error'] === UPLOAD_ERR_OK) { @@ -463,7 +463,7 @@ class User { $query = $this->db->prepare($sql); $query->execute([ ':avatar' => $newFileName, - ':user_id' => $user_id + ':user_id' => $userId ]); // all went OK $_SESSION['notice'] .= 'Avatar updated successfully. '; @@ -505,9 +505,9 @@ class User { /** * Enable two-factor authentication for a user * - * @param int $userId User ID - * @param string $secret Secret key to use - * @param string $code Verification code to validate + * @param int $userId User ID + * @param string $secret Secret key to use + * @param string $code Verification code to validate * @return bool True if enabled successfully */ public function enableTwoFactor($userId, $secret = null, $code = null) { @@ -527,8 +527,8 @@ class User { /** * Verify a two-factor authentication code * - * @param int $userId User ID - * @param string $code The verification code + * @param int $userId User ID + * @param string $code The verification code * @return bool True if verified */ public function verifyTwoFactor($userId, $code) { @@ -548,9 +548,9 @@ class User { /** * Change a user's password * - * @param int $userId User ID - * @param string $currentPassword Current password for verification - * @param string $newPassword New password to set + * @param int $userId User ID + * @param string $currentPassword Current password for verification + * @param string $newPassword New password to set * @return bool True if password was changed successfully */ public function changePassword($userId, $currentPassword, $newPassword) { diff --git a/app/pages/agents.php b/app/pages/agents.php index 0f66142..211d17f 100644 --- a/app/pages/agents.php +++ b/app/pages/agents.php @@ -50,7 +50,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { // Apply rate limiting for adding new contacts require '../app/includes/rate_limit_middleware.php'; - checkRateLimit($dbWeb, 'contact', $user_id); + checkRateLimit($dbWeb, 'contact', $userId); // Validate agent ID for POST operations if ($agentId === false || $agentId === null) { diff --git a/app/pages/config.php b/app/pages/config.php index c11a8a0..1cf2835 100644 --- a/app/pages/config.php +++ b/app/pages/config.php @@ -51,8 +51,8 @@ if (!$isWritable) { if ($_SERVER['REQUEST_METHOD'] === 'POST') { // Check if user has permission to edit config - if (!$userObject->hasRight($user_id, 'edit config file')) { - $logObject->insertLog($user_id, "Unauthorized: User \"$currentUser\" tried to edit config file. IP: $user_IP", 'system'); + if (!$userObject->hasRight($userId, 'edit config file')) { + $logObject->insertLog($userId, "Unauthorized: User \"$currentUser\" tried to edit config file. IP: $user_IP", 'system'); if ($isAjax) { ApiResponse::error('Forbidden: You do not have permission to edit the config file', null, 403); exit; @@ -64,7 +64,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { // Apply rate limiting require '../app/includes/rate_limit_middleware.php'; - checkRateLimit($dbWeb, 'config', $user_id); + checkRateLimit($dbWeb, 'config', $userId); // Ensure no output before this point ob_clean(); @@ -74,7 +74,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { // Get raw input $jsonData = file_get_contents('php://input'); if ($jsonData === false) { - $logObject->insertLog($user_id, "Failed to read request data for config update", 'system'); + $logObject->insertLog($userId, "Failed to read request data for config update", 'system'); ApiResponse::error('Failed to read request data'); exit; } @@ -115,10 +115,10 @@ if (!$isAjax) { * Handles GET requests to display templates. */ - if ($userObject->hasRight($user_id, 'view config file')) { + if ($userObject->hasRight($userId, 'view config file')) { include '../app/templates/config.php'; } else { - $logObject->insertLog($user_id, "Unauthorized: User \"$currentUser\" tried to access \"config\" page. IP: $user_IP", 'system'); + $logObject->insertLog($userId, "Unauthorized: User \"$currentUser\" tried to access \"config\" page. IP: $user_IP", 'system'); include '../app/templates/error-unauthorized.php'; } } diff --git a/app/pages/credentials.php b/app/pages/credentials.php index f70042b..cb22f31 100644 --- a/app/pages/credentials.php +++ b/app/pages/credentials.php @@ -14,11 +14,10 @@ * - `password`: Change password */ -$user_id = $_SESSION['user_id']; - // Initialize user object $userObject = new User($dbWeb); +// Get action and item from request $action = $_REQUEST['action'] ?? ''; $item = $_REQUEST['item'] ?? ''; @@ -34,7 +33,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { // Apply rate limiting require_once '../app/includes/rate_limit_middleware.php'; - checkRateLimit($dbWeb, 'credentials', $user_id); + checkRateLimit($dbWeb, 'credentials', $userId); switch ($item) { case '2fa': @@ -44,7 +43,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { $code = $_POST['code'] ?? ''; $secret = $_POST['secret'] ?? ''; - if ($userObject->enableTwoFactor($user_id, $secret, $code)) { + if ($userObject->enableTwoFactor($userId, $secret, $code)) { Feedback::flash('NOTICE', 'DEFAULT', 'Two-factor authentication has been enabled successfully.'); header("Location: $app_root?page=credentials"); exit(); @@ -61,7 +60,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { case 'verify': // This is a user-initiated verification $code = $_POST['code'] ?? ''; - if ($userObject->verifyTwoFactor($user_id, $code)) { + if ($userObject->verifyTwoFactor($userId, $code)) { $_SESSION['2fa_verified'] = true; header("Location: $app_root?page=dashboard"); exit(); @@ -73,7 +72,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { break; case 'disable': - if ($userObject->disableTwoFactor($user_id)) { + if ($userObject->disableTwoFactor($userId)) { Feedback::flash('NOTICE', 'DEFAULT', 'Two-factor authentication has been disabled.'); } else { Feedback::flash('ERROR', 'DEFAULT', 'Failed to disable two-factor authentication.'); @@ -109,7 +108,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { exit(); } - if ($userObject->changePassword($user_id, $_POST['current_password'], $_POST['new_password'])) { + if ($userObject->changePassword($userId, $_POST['current_password'], $_POST['new_password'])) { Feedback::flash('NOTICE', 'DEFAULT', 'Password has been changed successfully.'); } else { Feedback::flash('ERROR', 'DEFAULT', 'Failed to change password. Please verify your current password.'); @@ -130,12 +129,12 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { $security->generateCsrfToken(); // Get 2FA status for the template - $has2fa = $userObject->isTwoFactorEnabled($user_id); + $has2fa = $userObject->isTwoFactorEnabled($userId); switch ($action) { case 'setup': if (!$has2fa) { - $result = $userObject->enableTwoFactor($user_id); + $result = $userObject->enableTwoFactor($userId); if ($result['success']) { $setupData = $result['data']; } else { diff --git a/app/pages/login.php b/app/pages/login.php index 655f696..c06fecc 100644 --- a/app/pages/login.php +++ b/app/pages/login.php @@ -62,6 +62,9 @@ try { // Get any new feedback messages include '../app/helpers/feedback.php'; + // Make userId available to template + $userId = $pending2FA['user_id']; + // Load the 2FA verification template include '../app/templates/credentials-2fa-verify.php'; exit(); @@ -257,8 +260,8 @@ try { // Log the failed attempt Feedback::flash('ERROR', 'DEFAULT', $e->getMessage()); if (isset($username)) { - $user_id = $userObject->getUserId($username)[0]['id'] ?? 0; - $logObject->insertLog($user_id, "Login: Failed login attempt for user \"$username\". IP: $user_IP. Reason: {$e->getMessage()}", 'user'); + $userId = $userObject->getUserId($username)[0]['id'] ?? 0; + $logObject->insertLog($userId, "Login: Failed login attempt for user \"$username\". IP: $user_IP. Reason: {$e->getMessage()}", 'user'); } } } diff --git a/app/pages/logs.php b/app/pages/logs.php index 9b96dda..c0dd065 100644 --- a/app/pages/logs.php +++ b/app/pages/logs.php @@ -12,8 +12,8 @@ include '../app/helpers/feedback.php'; // Check for rights; user or system -$has_system_access = ($userObject->hasRight($user_id, 'superuser') || - $userObject->hasRight($user_id, 'view app logs')); +$has_system_access = ($userObject->hasRight($userId, 'superuser') || + $userObject->hasRight($userId, 'view app logs')); // Get current page for pagination $currentPage = $_REQUEST['page_num'] ?? 1; @@ -69,8 +69,8 @@ if (isset($_REQUEST['tab'])) { } // prepare the result -$search = $logObject->readLog($user_id, $scope, $offset, $items_per_page, $filters); -$search_all = $logObject->readLog($user_id, $scope, 0, 0, $filters); +$search = $logObject->readLog($userId, $scope, $offset, $items_per_page, $filters); +$search_all = $logObject->readLog($userId, $scope, 0, 0, $filters); if (!empty($search)) { // we get total items and number of pages @@ -103,7 +103,7 @@ if (!empty($search)) { } } -$username = $userObject->getUserDetails($user_id)[0]['username']; +$username = $userObject->getUserDetails($userId)[0]['username']; // Load the template include '../app/templates/logs.php'; diff --git a/app/pages/profile.php b/app/pages/profile.php index c7f86a8..d4f5a2d 100644 --- a/app/pages/profile.php +++ b/app/pages/profile.php @@ -30,11 +30,11 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { // Apply rate limiting for profile operations require_once '../app/includes/rate_limit_middleware.php'; - checkRateLimit($dbWeb, 'profile', $user_id); + checkRateLimit($dbWeb, 'profile', $userId); // avatar removal if ($item === 'avatar' && $action === 'remove') { - $validator = new Validator(['user_id' => $user_id]); + $validator = new Validator(['user_id' => $userId]); $rules = [ 'user_id' => [ 'required' => true, @@ -48,7 +48,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { exit(); } - $result = $userObject->removeAvatar($user_id, $config['avatars_path'].$userDetails[0]['avatar']); + $result = $userObject->removeAvatar($userId, $config['avatars_path'].$userDetails[0]['avatar']); if ($result === true) { Feedback::flash('NOTICE', 'DEFAULT', "Avatar for user \"{$userDetails[0]['username']}\" is removed."); } else { @@ -89,7 +89,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { 'timezone' => htmlspecialchars($_POST['timezone'] ?? ''), 'bio' => htmlspecialchars($_POST['bio'] ?? ''), ]; - $result = $userObject->editUser($user_id, $updatedUser); + $result = $userObject->editUser($userId, $updatedUser); if ($result === true) { Feedback::flash('NOTICE', 'DEFAULT', "User details for \"{$updatedUser['name']}\" are edited."); } else { @@ -118,21 +118,21 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { $rightsToAdd = array_diff($newRights, $userRightsIds); if (!empty($rightsToAdd)) { foreach ($rightsToAdd as $rightId) { - $userObject->addUserRight($user_id, $rightId); + $userObject->addUserRight($userId, $rightId); } } // what rights we need to remove $rightsToRemove = array_diff($userRightsIds, $newRights); if (!empty($rightsToRemove)) { foreach ($rightsToRemove as $rightId) { - $userObject->removeUserRight($user_id, $rightId); + $userObject->removeUserRight($userId, $rightId); } } } // update the avatar if (!empty($_FILES['avatar_file']['tmp_name'])) { - $result = $userObject->changeAvatar($user_id, $_FILES['avatar_file'], $config['avatars_path']); + $result = $userObject->changeAvatar($userId, $_FILES['avatar_file'], $config['avatars_path']); } header("Location: $app_root?page=profile"); diff --git a/app/pages/register.php b/app/pages/register.php index af7c0d1..135c56e 100644 --- a/app/pages/register.php +++ b/app/pages/register.php @@ -61,8 +61,8 @@ if ($config['registration_enabled'] == true) { // redirect to login if ($result === true) { // Get the new user's ID for logging - $user_id = $userObject->getUserId($username)[0]['id']; - $logObject->insertLog($user_id, "Registration: New user \"$username\" registered successfully. IP: $user_IP", 'user'); + $userId = $userObject->getUserId($username)[0]['id']; + $logObject->insertLog($userId, "Registration: New user \"$username\" registered successfully. IP: $user_IP", 'user'); Feedback::flash('NOTICE', 'DEFAULT', "Registration successful. You can log in now."); header('Location: ' . htmlspecialchars($app_root)); exit(); diff --git a/app/pages/security.php b/app/pages/security.php index 457d66b..6b980e8 100644 --- a/app/pages/security.php +++ b/app/pages/security.php @@ -1,10 +1,10 @@ hasRight($user_id, 'superuser') || - $userObject->hasRight($user_id, 'edit whitelist') || - $userObject->hasRight($user_id, 'edit blacklist') || - $userObject->hasRight($user_id, 'edit ratelimiting'))) { +if (!($userObject->hasRight($userId, 'superuser') || + $userObject->hasRight($userId, 'edit whitelist') || + $userObject->hasRight($userId, 'edit blacklist') || + $userObject->hasRight($userId, 'edit ratelimiting'))) { include '../app/templates/error-unauthorized.php'; exit; } @@ -22,7 +22,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { // Apply rate limiting for security operations require_once '../app/includes/rate_limit_middleware.php'; - checkRateLimit($dbWeb, 'security', $user_id); + checkRateLimit($dbWeb, 'security', $userId); $action = $_POST['action']; $validator = new Validator($_POST); @@ -30,7 +30,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { try { switch ($action) { case 'add_whitelist': - if (!$userObject->hasRight($user_id, 'superuser') && !$userObject->hasRight($user_id, 'edit whitelist')) { + if (!$userObject->hasRight($userId, 'superuser') && !$userObject->hasRight($userId, 'edit whitelist')) { Feedback::flash('SECURITY', 'PERMISSION_DENIED'); break; } @@ -49,7 +49,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { if ($validator->validate($rules)) { $is_network = isset($_POST['is_network']) && $_POST['is_network'] === 'on'; - if (!$rateLimiter->addToWhitelist($_POST['ip_address'], $is_network, $_POST['description'] ?? '', $currentUser, $user_id)) { + if (!$rateLimiter->addToWhitelist($_POST['ip_address'], $is_network, $_POST['description'] ?? '', $currentUser, $userId)) { Feedback::flash('SECURITY', 'WHITELIST_ADD_FAILED'); } else { Feedback::flash('SECURITY', 'WHITELIST_ADD_SUCCESS'); @@ -60,7 +60,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { break; case 'remove_whitelist': - if (!$userObject->hasRight($user_id, 'superuser') && !$userObject->hasRight($user_id, 'edit whitelist')) { + if (!$userObject->hasRight($userId, 'superuser') && !$userObject->hasRight($userId, 'edit whitelist')) { Feedback::flash('SECURITY', 'PERMISSION_DENIED'); break; } @@ -74,7 +74,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { ]; if ($validator->validate($rules)) { - if (!$rateLimiter->removeFromWhitelist($_POST['ip_address'], $currentUser, $user_id)) { + if (!$rateLimiter->removeFromWhitelist($_POST['ip_address'], $currentUser, $userId)) { Feedback::flash('SECURITY', 'WHITELIST_REMOVE_FAILED'); } else { Feedback::flash('SECURITY', 'WHITELIST_REMOVE_SUCCESS'); @@ -85,7 +85,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { break; case 'add_blacklist': - if (!$userObject->hasRight($user_id, 'superuser') && !$userObject->hasRight($user_id, 'edit blacklist')) { + if (!$userObject->hasRight($userId, 'superuser') && !$userObject->hasRight($userId, 'edit blacklist')) { Feedback::flash('SECURITY', 'PERMISSION_DENIED'); break; } @@ -111,7 +111,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { $is_network = isset($_POST['is_network']) && $_POST['is_network'] === 'on'; $expiry_hours = !empty($_POST['expiry_hours']) ? (int)$_POST['expiry_hours'] : null; - if (!$rateLimiter->addToBlacklist($_POST['ip_address'], $is_network, $_POST['reason'], $currentUser, $user_id, $expiry_hours)) { + if (!$rateLimiter->addToBlacklist($_POST['ip_address'], $is_network, $_POST['reason'], $currentUser, $userId, $expiry_hours)) { Feedback::flash('SECURITY', 'BLACKLIST_ADD_FAILED'); } else { Feedback::flash('SECURITY', 'BLACKLIST_ADD_SUCCESS'); @@ -122,7 +122,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { break; case 'remove_blacklist': - if (!$userObject->hasRight($user_id, 'superuser') && !$userObject->hasRight($user_id, 'edit blacklist')) { + if (!$userObject->hasRight($userId, 'superuser') && !$userObject->hasRight($userId, 'edit blacklist')) { Feedback::flash('SECURITY', 'PERMISSION_DENIED'); break; } @@ -136,7 +136,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['action'])) { ]; if ($validator->validate($rules)) { - if (!$rateLimiter->removeFromBlacklist($_POST['ip_address'], $currentUser, $user_id)) { + if (!$rateLimiter->removeFromBlacklist($_POST['ip_address'], $currentUser, $userId)) { Feedback::flash('SECURITY', 'BLACKLIST_REMOVE_FAILED'); } else { Feedback::flash('SECURITY', 'BLACKLIST_REMOVE_SUCCESS'); diff --git a/app/pages/settings.php b/app/pages/settings.php index 2037d24..dc32e3c 100644 --- a/app/pages/settings.php +++ b/app/pages/settings.php @@ -31,7 +31,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { // Apply rate limiting for profile operations require_once '../app/includes/rate_limit_middleware.php'; - checkRateLimit($dbWeb, 'profile', $user_id); + checkRateLimit($dbWeb, 'profile', $userId); // Get hash from URL if present $hash = parse_url($_SERVER['REQUEST_URI'], PHP_URL_FRAGMENT) ?? ''; @@ -170,7 +170,7 @@ if ($_SERVER['REQUEST_METHOD'] == 'POST') { * Handles GET requests to display templates. */ - if ($userObject->hasRight($user_id, 'view settings')) { + if ($userObject->hasRight($userId, 'view settings')) { $jilo_agent_types = $agentObject->getAgentTypes(); include '../app/templates/settings.php'; } else { diff --git a/app/templates/config.php b/app/templates/config.php index 2d15a36..d2c5151 100644 --- a/app/templates/config.php +++ b/app/templates/config.php @@ -17,7 +17,7 @@ app configuration -hasRight($user_id, 'edit config file')) { ?> +hasRight($userId, 'edit config file')) { ?>
-hasRight($user_id, 'superuser') || $userObject->hasRight($user_id, 'edit whitelist'))) { ?> +hasRight($userId, 'superuser') || $userObject->hasRight($userId, 'edit whitelist'))) { ?>
@@ -93,7 +93,7 @@
-hasRight($user_id, 'superuser') || $userObject->hasRight($user_id, 'edit blacklist'))) { ?> +hasRight($userId, 'superuser') || $userObject->hasRight($userId, 'edit blacklist'))) { ?>
@@ -167,7 +167,7 @@
-hasRight($user_id, 'superuser') || $userObject->hasRight($user_id, 'edit ratelimiting'))) { ?> +hasRight($userId, 'superuser') || $userObject->hasRight($userId, 'edit ratelimiting'))) { ?>
diff --git a/app/templates/settings.php b/app/templates/settings.php index a6dc843..44a8c0d 100644 --- a/app/templates/settings.php +++ b/app/templates/settings.php @@ -57,7 +57,7 @@ - hasRight($user_id, 'delete platform')): ?> + hasRight($userId, 'delete platform')): ?> diff --git a/public_html/index.php b/public_html/index.php index efc3368..9a068b1 100644 --- a/public_html/index.php +++ b/public_html/index.php @@ -28,6 +28,9 @@ require '../app/includes/sanitize.php'; // Check session validity $validSession = Session::isValidSession(); +// Get user ID early if session is valid +$userId = $validSession ? Session::getUserId() : null; + // Initialize feedback message system require_once '../app/classes/feedback.php'; $system_messages = []; @@ -175,9 +178,6 @@ $userObject = new User($dbWeb); // logout is a special case, as we can't use session vars for notices if ($page == 'logout') { - // get user info before destroying session - $user_id = $userObject->getUserId($currentUser)[0]['id']; - // clean up session Session::destroySession(); @@ -187,7 +187,7 @@ if ($page == 'logout') { setcookie('username', "", time() - 100, $config['folder'], $config['domain'], isset($_SERVER['HTTPS']), true); // Log successful logout - $logObject->insertLog($user_id, "Logout: User \"$currentUser\" logged out. IP: $user_IP", 'user'); + $logObject->insertLog($userId, "Logout: User \"$currentUser\" logged out. IP: $user_IP", 'user'); // Set success message Feedback::flash('LOGIN', 'LOGOUT_SUCCESS'); @@ -206,9 +206,8 @@ if ($page == 'logout') { header('Location: ' . htmlspecialchars($app_root)); exit(); } - $user_id = $userObject->getUserId($currentUser)[0]['id']; - $userDetails = $userObject->getUserDetails($user_id); - $userRights = $userObject->getUserRights($user_id); + $userDetails = $userObject->getUserDetails($userId); + $userRights = $userObject->getUserRights($userId); $userTimezone = (!empty($userDetails[0]['timezone'])) ? $userDetails[0]['timezone'] : 'UTC'; // Default to UTC if no timezone is set (or is missing) // check if the Jilo Server is running diff --git a/tests/Feature/Security/CsrfProtectionTest.php b/tests/Feature/Security/CsrfProtectionTest.php index 248297c..59cd5a5 100644 --- a/tests/Feature/Security/CsrfProtectionTest.php +++ b/tests/Feature/Security/CsrfProtectionTest.php @@ -8,7 +8,7 @@ require_once dirname(__DIR__, 3) . '/app/helpers/security.php'; use PHPUnit\Framework\TestCase; class TestLogger { - public static function insertLog($user_id, $message, $scope = 'user') { + public static function insertLog($userId, $message, $scope = 'user') { return true; } }