From bdbf162130c02c6a5510205c7704ef7d5e8f22ea Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Mon, 26 Mar 2018 14:19:37 +0000 Subject: [PATCH] A bit of refactoring of access checks in user edits and deletes. --- WEB-INF/lib/ttUser.class.php | 19 +++++++++++++++++++ WEB-INF/lib/ttUserHelper.class.php | 15 --------------- WEB-INF/templates/footer.tpl | 2 +- mobile/user_delete.php | 15 +++------------ mobile/user_edit.php | 13 +++---------- user_delete.php | 10 ++-------- user_edit.php | 9 ++------- 7 files changed, 30 insertions(+), 53 deletions(-) diff --git a/WEB-INF/lib/ttUser.class.php b/WEB-INF/lib/ttUser.class.php index 5321e74d..7f9894ef 100644 --- a/WEB-INF/lib/ttUser.class.php +++ b/WEB-INF/lib/ttUser.class.php @@ -298,6 +298,25 @@ class ttUser { return $user_list; } + // getUser function is used to manage users in group and returns user details. + // At the moment, the function is used for user edits and deletes. + function getUser($user_id) { + if (!$this->can('manage_users')) return false; + + $mdb2 = getConnection(); + + $sql = "select u.id, u.name, u.login, u.role_id, u.status, u.rate, u.email, r.rank from tt_users u". + " left join tt_roles r on (u.role_id = r.id)". + " where u.id = $user_id and u.team_id = $this->team_id and u.status is not null". + " and (r.rank < $this->rank or (r.rank = $this->rank and u.id = $this->id))"; // Users with lesser roles or self. + $res = $mdb2->query($sql); + if (!is_a($res, 'PEAR_Error')) { + $val = $res->fetchRow(); + return $val; + } + return false; + } + // checkBehalfId checks whether behalf_id is appropriate. // On behalf user must be active and have lower rank. function checkBehalfId() { diff --git a/WEB-INF/lib/ttUserHelper.class.php b/WEB-INF/lib/ttUserHelper.class.php index 583049bd..31c7bde4 100644 --- a/WEB-INF/lib/ttUserHelper.class.php +++ b/WEB-INF/lib/ttUserHelper.class.php @@ -31,21 +31,6 @@ import('ttTeamHelper'); // Class ttUserHelper contains helper functions for operations with users. class ttUserHelper { - // The getUserDetails function returns user details. - static function getUserDetails($user_id) { - global $user; - $mdb2 = getConnection(); - - $sql = "select u.*, r.rank from tt_users u left join tt_roles r on (u.role_id = r.id) where u.id = $user_id and u.team_id = $user->team_id and u.status is not null"; - $res = $mdb2->query($sql); - - if (!is_a($res, 'PEAR_Error')) { - $val = $res->fetchRow(); - return $val; - } - return false; - } - // The getUserName function returns user name. static function getUserName($user_id) { $mdb2 = getConnection(); diff --git a/WEB-INF/templates/footer.tpl b/WEB-INF/templates/footer.tpl index 77e160e9..851570f3 100644 --- a/WEB-INF/templates/footer.tpl +++ b/WEB-INF/templates/footer.tpl @@ -12,7 +12,7 @@
-
 Anuko Time Tracker 1.17.72.4171 | Copyright © Anuko | +  Anuko Time Tracker 1.17.72.4172 | Copyright © Anuko | {$i18n.footer.credits} | {$i18n.footer.license} | {$i18n.footer.improve} diff --git a/mobile/user_delete.php b/mobile/user_delete.php index 0e045d01..4c9aad87 100644 --- a/mobile/user_delete.php +++ b/mobile/user_delete.php @@ -35,22 +35,13 @@ if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } - -// Get user id we are deleting from the request. -// A cast to int is for safety against manipulation of request parameter (sql injection). $user_id = (int) $request->getParameter('id'); - -// We need user name and login to display. -$user_details = ttUserHelper::getUserDetails($user_id); - -// Security checks. -if (!$user_details || // No details. - $user_details['rank'] > $user->rank || // User has a bigger rank. - ($user_details['rank'] == $user->rank && $user_details['id'] <> $user->id) // Same rank but not us. - ) { +$user_details = $user->getUser($user_id); +if (!$user_details) { header('Location: access_denied.php'); exit(); } +// End of access checks. $smarty->assign('user_to_delete', $user_details['name']." (".$user_details['login'].")"); diff --git a/mobile/user_edit.php b/mobile/user_edit.php index 49781226..3baaf1e9 100644 --- a/mobile/user_edit.php +++ b/mobile/user_edit.php @@ -39,20 +39,13 @@ if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } - -// Get user id we are editing from the request. $user_id = (int) $request->getParameter('id'); -// Get user details. -$user_details = ttUserHelper::getUserDetails($user_id); - -// Security checks. -if (!$user_details || // No details. - $user_details['rank'] > $user->rank || // User has a bigger rank. - ($user_details['rank'] == $user->rank && $user_details['id'] <> $user->id) // Same rank but not us. - ) { +$user_details = $user->getUser($user_id); +if (!$user_details) { header('Location: access_denied.php'); exit(); } +// End of access checks. if ($user->isPluginEnabled('cl')) $clients = ttTeamHelper::getActiveClients($user->team_id); diff --git a/user_delete.php b/user_delete.php index b8ec5e9a..d06463b8 100644 --- a/user_delete.php +++ b/user_delete.php @@ -35,15 +35,9 @@ if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } -// Get user id we are deleting. -// A cast to int is for safety against manipulation of request parameter (sql injection). $user_id = (int) $request->getParameter('id'); -// We need user name and login to display. -$user_details = ttUserHelper::getUserDetails($user_id); -if (!$user_details || // No details. - $user_details['rank'] > $user->rank || // User has a bigger rank. - ($user_details['rank'] == $user->rank && $user_details['id'] <> $user->id) // Same rank but not us. - ) { +$user_details = $user->getUser($user_id); +if (!$user_details) { header('Location: access_denied.php'); exit(); } diff --git a/user_edit.php b/user_edit.php index 5ceac373..8f588db9 100644 --- a/user_edit.php +++ b/user_edit.php @@ -40,14 +40,9 @@ if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } -// Get user id we are editing from the request. $user_id = (int) $request->getParameter('id'); -// Get user details. -$user_details = ttUserHelper::getUserDetails($user_id); -if (!$user_details || // No details. - $user_details['rank'] > $user->rank || // User has a bigger rank. - ($user_details['rank'] == $user->rank && $user_details['id'] <> $user->id) // Same rank but not us. - ) { +$user_details = $user->getUser($user_id); +if (!$user_details) { header('Location: access_denied.php'); exit(); } -- 2.20.1