From 6c7a98f61e74aeae700b523729abb49ff19d3704 Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Thu, 15 Mar 2018 21:17:45 +0000 Subject: [PATCH] A few bugs fixed related to role revamp. --- WEB-INF/lib/ttRoleHelper.class.php | 27 ------------- WEB-INF/lib/ttTeamHelper.class.php | 4 +- WEB-INF/lib/ttUser.class.php | 5 --- WEB-INF/lib/ttUserHelper.class.php | 15 +++---- WEB-INF/templates/footer.tpl | 2 +- WEB-INF/templates/mobile/user_add.tpl | 27 ++++++++++--- WEB-INF/templates/mobile/user_edit.tpl | 30 ++++++++++---- WEB-INF/templates/mobile/users.tpl | 55 +++++--------------------- WEB-INF/templates/user_add.tpl | 2 - WEB-INF/templates/user_edit.tpl | 5 +-- WEB-INF/templates/users.tpl | 52 ++++++------------------ mobile/user_add.php | 13 +++--- mobile/user_delete.php | 20 +++++----- mobile/user_edit.php | 31 ++++++--------- user_add.php | 12 ++---- user_delete.php | 21 +++++----- user_edit.php | 33 ++++++---------- 17 files changed, 126 insertions(+), 228 deletions(-) diff --git a/WEB-INF/lib/ttRoleHelper.class.php b/WEB-INF/lib/ttRoleHelper.class.php index 329e0a82..4a666cb6 100644 --- a/WEB-INF/lib/ttRoleHelper.class.php +++ b/WEB-INF/lib/ttRoleHelper.class.php @@ -83,33 +83,6 @@ class ttRoleHelper { return false; } - // The getLegacyRole obtains a legacy role value for a role_id. - // This is a temporary function to allow usage of both old and new roles - // while new role code is being written and deployed. - static function getLegacyRole($role_id) { - global $user; - $mdb2 = getConnection(); - - $sql = "select rank from tt_roles where team_id = $user->team_id and id = $role_id"; - $res = $mdb2->query($sql); - - if (!is_a($res, 'PEAR_Error')) { - $val = $res->fetchRow(); - if ($val['rank']) { - $rank = $val['rank']; - if ($rank >= ROLE_MANAGER) - return ROLE_MANAGER; - else if ($rank >= ROLE_COMANAGER) - return ROLE_COMANAGER; - else if ($rank >= ROLE_CLIENT) - return ROLE_CLIENT; - else - return ROLE_USER; - } - } - return false; - } - // isClientRole determines if the role is a "client" role. // This simply means the role has no "track_own_time" right. static function isClientRole($role_id) { diff --git a/WEB-INF/lib/ttTeamHelper.class.php b/WEB-INF/lib/ttTeamHelper.class.php index 684fa63c..a90ee7f6 100644 --- a/WEB-INF/lib/ttTeamHelper.class.php +++ b/WEB-INF/lib/ttTeamHelper.class.php @@ -74,7 +74,7 @@ class ttTeamHelper { $mdb2 = getConnection(); if (isset($options['getAllFields'])) - $sql = "select * from tt_users where team_id = $user->team_id and status = 1 order by upper(name)"; + $sql = "select u.*, r.name as role_name, r.rank from tt_users u left join tt_roles r on (u.role_id = r.id) where u.team_id = $user->team_id and u.status = 1 order by upper(u.name)"; else $sql = "select id, name from tt_users where team_id = $user->team_id and status = 1 order by upper(name)"; $res = $mdb2->query($sql); @@ -121,7 +121,7 @@ class ttTeamHelper { $mdb2 = getConnection(); if ($all_fields) - $sql = "select * from tt_users where team_id = $team_id and status = 0 order by upper(name)"; + $sql = "select u.*, r.name as role_name from tt_users u left join tt_roles r on (u.role_id = r.id) where u.team_id = $team_id and u.status = 0 order by upper(u.name)"; else $sql = "select id, name from tt_users where team_id = $team_id and status = 0 order by upper(name)"; $res = $mdb2->query($sql); diff --git a/WEB-INF/lib/ttUser.class.php b/WEB-INF/lib/ttUser.class.php index 9a4dee48..f7b62344 100644 --- a/WEB-INF/lib/ttUser.class.php +++ b/WEB-INF/lib/ttUser.class.php @@ -31,10 +31,6 @@ class ttUser { var $name = null; // User name. var $id = null; // User id. var $team_id = null; // Team id. - var $legacy_role = null; // Old user role (user, client, comanager, manager, admin). TODO: remove when new roles are done. - // Complete removal requires refactoring migrateLegacyRole, which is used in dbinstall.php. - // Perhaps, after doing an installer? - var $role_id = null; // Role id. var $rank = null; // User role rank. var $client_id = null; // Client id for client user role. @@ -97,7 +93,6 @@ class ttUser { $this->name = $val['name']; $this->id = $val['id']; $this->team_id = $val['team_id']; - $this->legacy_role = $val['role']; $this->role_id = $val['role_id']; $this->rights = explode(',', $val['rights']); $this->is_client = !in_array('track_own_time', $this->rights); diff --git a/WEB-INF/lib/ttUserHelper.class.php b/WEB-INF/lib/ttUserHelper.class.php index e70f085b..8dceed83 100644 --- a/WEB-INF/lib/ttUserHelper.class.php +++ b/WEB-INF/lib/ttUserHelper.class.php @@ -33,10 +33,10 @@ class ttUserHelper { // The getUserDetails function returns user details. static function getUserDetails($user_id) { - $result = array(); + global $user; $mdb2 = getConnection(); - $sql = "select * from tt_users where id = $user_id"; + $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"; $res = $mdb2->query($sql); if (!is_a($res, 'PEAR_Error')) { @@ -116,7 +116,6 @@ class ttUserHelper { $password = 'md5('.$password.')'; $email = isset($fields['email']) ? $fields['email'] : ''; $team_id = (int) $fields['team_id']; - $role = (int) $fields['role']; $rate = str_replace(',', '.', isset($fields['rate']) ? $fields['rate'] : 0); if($rate == '') $rate = 0; @@ -125,9 +124,9 @@ class ttUserHelper { $status_v = ', '.$mdb2->quote($fields['status']); } - $sql = "insert into tt_users (name, login, password, team_id, role, role_id, client_id, rate, email $status_f) values (". + $sql = "insert into tt_users (name, login, password, team_id, role_id, client_id, rate, email $status_f) values (". $mdb2->quote($fields['name']).", ".$mdb2->quote($fields['login']). - ", $password, $team_id, $role, ".$mdb2->quote($fields['role_id']).", ".$mdb2->quote($fields['client_id']).", $rate, ".$mdb2->quote($email)." $status_v)"; + ", $password, $team_id, ".$mdb2->quote($fields['role_id']).", ".$mdb2->quote($fields['client_id']).", $rate, ".$mdb2->quote($email)." $status_v)"; $affected = $mdb2->exec($sql); // Now deal with project assignment. @@ -168,10 +167,6 @@ class ttUserHelper { if (isset($fields['password'])) $pass_part = ', password = md5('.$mdb2->quote($fields['password']).')'; if (in_array('manage_users', $user->rights)) { - if (isset($fields['role'])) { - $role = (int) $fields['role']; - $role_part = ", role = $role"; - } if (isset($fields['role_id'])) { $role_id = (int) $fields['role_id']; $role_id_part = ", role_id = $role_id"; @@ -193,7 +188,7 @@ class ttUserHelper { $sql = "update tt_users set login = ".$mdb2->quote($fields['login']). "$pass_part, name = ".$mdb2->quote($fields['name']). - "$role_part $role_id_part $client_part $rate_part $status_part, email = ".$mdb2->quote($fields['email']). + "$role_id_part $client_part $rate_part $status_part, email = ".$mdb2->quote($fields['email']). " where id = $user_id"; $affected = $mdb2->exec($sql); if (is_a($affected, 'PEAR_Error')) return false; diff --git a/WEB-INF/templates/footer.tpl b/WEB-INF/templates/footer.tpl index 058c4eb6..918caff6 100644 --- a/WEB-INF/templates/footer.tpl +++ b/WEB-INF/templates/footer.tpl @@ -12,7 +12,7 @@
- -{if $user->isManager()} -{/if} diff --git a/WEB-INF/templates/mobile/user_edit.tpl b/WEB-INF/templates/mobile/user_edit.tpl index cbfded78..ecbff599 100644 --- a/WEB-INF/templates/mobile/user_edit.tpl +++ b/WEB-INF/templates/mobile/user_edit.tpl @@ -1,4 +1,13 @@ @@ -78,14 +95,11 @@ function handleClientControl() { -{if $user->isManager() && ($user->id != $user_id)} +{if $user->id != $user_id} -{/if} -{* Prohibit deactivating team manager. Deactivating others is ok. *} -{if $user->canManageTeam() && !($user->isManager() && $user->id == $user_id)} diff --git a/WEB-INF/templates/mobile/users.tpl b/WEB-INF/templates/mobile/users.tpl index f4a8ec54..f3dbe850 100644 --- a/WEB-INF/templates/mobile/users.tpl +++ b/WEB-INF/templates/mobile/users.tpl @@ -5,7 +5,7 @@
 Anuko Time Tracker 1.17.45.4082 | Copyright © Anuko | +  Anuko Time Tracker 1.17.46.4083 | Copyright © Anuko | {$i18n.footer.credits} | {$i18n.footer.license} | {$i18n.footer.improve} diff --git a/WEB-INF/templates/mobile/user_add.tpl b/WEB-INF/templates/mobile/user_add.tpl index 29714044..e1a05350 100644 --- a/WEB-INF/templates/mobile/user_add.tpl +++ b/WEB-INF/templates/mobile/user_add.tpl @@ -1,4 +1,13 @@ @@ -56,12 +73,10 @@ function handleClientControl() { {$i18n.label.email}: {$forms.userForm.email.control}
{$i18n.form.users.role}: {$forms.userForm.role.control} {$forms.userForm.client.control}
{$i18n.form.users.default_rate} (0{$user->decimal_mark}00): {$forms.userForm.rate.control}{$i18n.label.email}: {$forms.userForm.email.control}
{$i18n.form.users.role}: {$forms.userForm.role.control} {$forms.userForm.client.control}
{$i18n.label.status}: {$forms.userForm.status.control}
-{if $user->isManager()} -{/if} diff --git a/WEB-INF/templates/user_edit.tpl b/WEB-INF/templates/user_edit.tpl index 4a45f771..85e2cd44 100644 --- a/WEB-INF/templates/user_edit.tpl +++ b/WEB-INF/templates/user_edit.tpl @@ -95,14 +95,11 @@ function handleClientControl() { -{if $user->isManager() && ($user->id != $user_id)} +{if $user->id != $user_id} -{/if} -{* Prohibit deactivating team manager. Deactivating others is ok. *} -{if $user->canManageTeam() && !($user->isManager() && $user->id == $user_id)} diff --git a/WEB-INF/templates/users.tpl b/WEB-INF/templates/users.tpl index 09a99bd3..85d6efc8 100644 --- a/WEB-INF/templates/users.tpl +++ b/WEB-INF/templates/users.tpl @@ -5,7 +5,7 @@
-{if $user->canManageTeam()} +{if $user->can('manage_users')} {if $inactive_users} @@ -22,26 +22,14 @@ {if $user->uncompleted_indicators} {/if} - {if $user->isManager()} + {if $u.rank <= $user->rank} {$u.name|escape} {else} - {if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)} - {$u.name|escape} - {else} - {$u.name|escape} - {/if} + {$u.name|escape} {/if} - {if $smarty.const.ROLE_MANAGER == $u.role} - - {elseif $smarty.const.ROLE_COMANAGER == $u.role} - - {elseif $smarty.const.ROLE_CLIENT == $u.role} - - {elseif $smarty.const.ROLE_USER == $u.role} - - {/if} + {/foreach} {/if} @@ -56,43 +44,26 @@
{$i18n.form.users.active_users}
{$u.login|escape}{$i18n.form.users.manager}{$i18n.form.users.comanager}{$i18n.label.client}{$i18n.label.user}{$u.role_name|escape}
{if $inactive_users} - +
- {foreach $inactive_users as $u} - {if $smarty.const.ROLE_MANAGER == $u.role} - - {elseif $smarty.const.ROLE_COMANAGER == $u.role} - - {elseif $smarty.const.ROLE_CLIENT == $u.role} - - {elseif $smarty.const.ROLE_USER == $u.role} - - {/if} - {if $user->isManager()} - - - {else} - - - {/if} + {/foreach} -
{$i18n.form.users.inactive_users}
{$i18n.label.person_name} {$i18n.label.login} {$i18n.form.users.role}{$i18n.label.edit}
- {if $user->isManager()} + {if $u.rank <= $user->rank} {$u.name|escape} {else} - {if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)}{$u.name|escape}{/if} + {$u.name|escape} {/if} {$u.login|escape}{$i18n.form.users.manager}{$i18n.form.users.comanager}{$i18n.label.client}{$i18n.label.user}{$i18n.label.edit}{if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)}{$i18n.label.edit}{/if}{$u.role_name|escape}
@@ -114,15 +85,7 @@ - {if $smarty.const.ROLE_MANAGER == $u.role} - - {elseif $smarty.const.ROLE_COMANAGER == $u.role} - - {elseif $smarty.const.ROLE_CLIENT == $u.role} - - {elseif $smarty.const.ROLE_USER == $u.role} - - {/if} + {/foreach}
{$u.name|escape} {$u.login|escape}{$i18n.form.users.manager}{$i18n.form.users.comanager}{$i18n.label.client}{$i18n.label.user}{$u.role_name|escape}
diff --git a/WEB-INF/templates/user_add.tpl b/WEB-INF/templates/user_add.tpl index d9452cc8..e1a05350 100644 --- a/WEB-INF/templates/user_add.tpl +++ b/WEB-INF/templates/user_add.tpl @@ -73,12 +73,10 @@ function handleClientControl() { 		{$i18n.label.email}: {$forms.userForm.email.control}
{$i18n.form.users.role}: {$forms.userForm.role.control} {$forms.userForm.client.control}
{$i18n.form.users.default_rate} (0{$user->decimal_mark}00): {$forms.userForm.rate.control}{$i18n.label.email}: {$forms.userForm.email.control}
{$i18n.form.users.role}: {$forms.userForm.role.control} {$forms.userForm.client.control}
{$i18n.label.status}: {$forms.userForm.status.control}
-{if $user->canManageTeam()} +{if $user->can('manage_users')} {if $inactive_users} @@ -27,23 +27,13 @@ {$u.name|escape} - {if $smarty.const.ROLE_MANAGER == $u.role} - - {elseif $smarty.const.ROLE_COMANAGER == $u.role} - - {elseif $smarty.const.ROLE_CLIENT == $u.role} - - {elseif $smarty.const.ROLE_USER == $u.role} - - {/if} - {if $user->isManager()} - + + {if $u.rank <= $user->rank} - + {if $u.id != $user->id}{else}{/if} {else} - - - + + {/if} {/foreach} @@ -72,23 +62,13 @@ - {if $smarty.const.ROLE_MANAGER == $u.role} - - {elseif $smarty.const.ROLE_COMANAGER == $u.role} - - {elseif $smarty.const.ROLE_CLIENT == $u.role} - - {elseif $smarty.const.ROLE_USER == $u.role} - - {/if} - {if $user->isManager()} - + + {if $u.rank <= $user->rank} - + {if $u.id != $user->id}{else}{/if} {else} - - - + + {/if} {/foreach} @@ -114,15 +94,7 @@ - {if $smarty.const.ROLE_MANAGER == $u.role} - - {elseif $smarty.const.ROLE_COMANAGER == $u.role} - - {elseif $smarty.const.ROLE_CLIENT == $u.role} - - {elseif $smarty.const.ROLE_USER == $u.role} - - {/if} + {/foreach}
{$i18n.form.users.active_users}
{$u.login|escape}{$i18n.form.users.manager}{$i18n.form.users.comanager}{$i18n.label.client}{$i18n.label.user}{$u.role_name|escape}{$i18n.label.edit}{if $smarty.const.ROLE_MANAGER != $u.role || $can_delete_manager}{$i18n.label.delete}{/if}{$i18n.label.delete}{if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)}{$i18n.label.edit}{/if}{if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)}{$i18n.label.delete}{/if}
{$u.name|escape} {$u.login|escape}{$i18n.form.users.manager}{$i18n.form.users.comanager}{$i18n.label.client}{$i18n.label.user}{$u.role_name|escape}{$i18n.label.edit}{if $smarty.const.ROLE_MANAGER != $u.role || $can_delete_manager}{$i18n.label.delete}{/if}{$i18n.label.delete}{if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)}{$i18n.label.edit}{/if}{if ($user->id == $u.id) || ($smarty.const.ROLE_CLIENT == $u.role) || ($smarty.const.ROLE_USER == $u.role)}{$i18n.label.delete}{/if}
{$u.name|escape} {$u.login|escape}{$i18n.form.users.manager}{$i18n.form.users.comanager}{$i18n.label.client}{$i18n.label.user}{$u.role_name|escape}
diff --git a/mobile/user_add.php b/mobile/user_add.php index 7737ed9a..7fff9a06 100644 --- a/mobile/user_add.php +++ b/mobile/user_add.php @@ -55,8 +55,7 @@ if ($request->isPost()) { $cl_password2 = $request->getParameter('pas2'); } $cl_email = trim($request->getParameter('email')); - $cl_role = $request->getParameter('role'); - if (!$cl_role) $cl_role = ROLE_USER; + $cl_role_id = $request->getParameter('role'); $cl_client_id = $request->getParameter('client'); $cl_rate = $request->getParameter('rate'); $cl_projects = $request->getParameter('projects'); @@ -82,11 +81,8 @@ if (!$auth->isPasswordExternal()) { } $form->addInput(array('type'=>'text','maxlength'=>'100','name'=>'email','value'=>$cl_email)); -$roles[ROLE_USER] = $i18n->getKey('label.user'); -$roles[ROLE_COMANAGER] = $i18n->getKey('form.users.comanager'); -if ($user->isPluginEnabled('cl')) - $roles[ROLE_CLIENT] = $i18n->getKey('label.client'); -$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role,'data'=>$roles)); +$active_roles = ttTeamHelper::getActiveRolesForUser(); +$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role_id,'data'=>$active_roles,'datakeys'=>array('id', 'name'))); if ($user->isPluginEnabled('cl')) $form->addInput(array('type'=>'combobox','name'=>'client','value'=>$cl_client_id,'data'=>$clients,'datakeys'=>array('id', 'name'),'empty'=>array(''=>$i18n->getKey('dropdown.select')))); @@ -151,7 +147,7 @@ if ($request->isPost()) { 'password' => $cl_password1, 'rate' => $cl_rate, 'team_id' => $user->team_id, - 'role' => $cl_role, + 'role_id' => $cl_role_id, 'client_id' => $cl_client_id, 'projects' => $assigned_projects, 'email' => $cl_email); @@ -166,6 +162,7 @@ if ($request->isPost()) { } // isPost $smarty->assign('auth_external', $auth->isPasswordExternal()); +$smarty->assign('active_roles', $active_roles); $smarty->assign('forms', array($form->getName()=>$form->toArray())); $smarty->assign('onload', 'onLoad="document.userForm.name.focus();handleClientControl();"'); $smarty->assign('title', $i18n->getKey('title.add_user')); diff --git a/mobile/user_delete.php b/mobile/user_delete.php index 8a4236b0..cb0e1a2c 100644 --- a/mobile/user_delete.php +++ b/mobile/user_delete.php @@ -44,18 +44,16 @@ $user_id = (int) $request->getParameter('id'); $user_details = ttUserHelper::getUserDetails($user_id); // Security checks. -$ok_to_go = $user->canManageTeam(); // Are we authorized for user deletes? -if ($ok_to_go) $ok_to_go = $ok_to_go && $user_details; // Are we deleting a real user? -if ($ok_to_go) $ok_to_go = $ok_to_go && ($user->team_id == $user_details['team_id']); // User belongs to our team? -if ($ok_to_go && $user->isCoManager() && (ROLE_COMANAGER == $user_details['role'])) - $ok_to_go = ($user->id == $user_details['id']); // Comanager is not allowed to delete other comanagers. -if ($ok_to_go && $user->isCoManager() && (ROLE_MANAGER == $user_details['role'])) - $ok_to_go = false; // Comanager is not allowed to delete a manager. +if (!$user_details || // No details. + $user_details['team_id'] <> $user->team_id || // User not in team. + $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. + ) { + header('Location: access_denied.php'); + exit(); +} -if (!$ok_to_go) - die ($i18n->getKey('error.sys')); -else - $smarty->assign('user_to_delete', $user_details['name']." (".$user_details['login'].")"); +$smarty->assign('user_to_delete', $user_details['name']." (".$user_details['login'].")"); // Create confirmation form. $form = new Form('userDeleteForm'); diff --git a/mobile/user_edit.php b/mobile/user_edit.php index b353047a..89a473cb 100644 --- a/mobile/user_edit.php +++ b/mobile/user_edit.php @@ -42,20 +42,17 @@ if (!ttAccessAllowed('manage_users')) { // 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. -$ok_to_go = $user->canManageTeam(); // Are we authorized for user management? -if ($ok_to_go) $ok_to_go = $ok_to_go && $user_details; // Are we editing a real user? -if ($ok_to_go) $ok_to_go = $ok_to_go && ($user->team_id == $user_details['team_id']); // User belongs to our team? -if ($ok_to_go && $user->isCoManager() && (ROLE_COMANAGER == $user_details['role'])) - $ok_to_go = ($user->id == $user_details['id']); // Comanager is not allowed to edit other comanagers. -if ($ok_to_go && $user->isCoManager() && (ROLE_MANAGER == $user_details['role'])) - $ok_to_go = false; // Comanager is not allowed to edit a manager. -if (!$ok_to_go) { - die ($i18n->getKey('error.sys')); +if (!$user_details || // No details. + $user_details['team_id'] <> $user->team_id || // User not in team. + $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. + ) { + header('Location: access_denied.php'); + exit(); } if ($user->isPluginEnabled('cl')) @@ -72,7 +69,7 @@ if ($request->isPost()) { $cl_password2 = $request->getParameter('pas2'); } $cl_email = trim($request->getParameter('email')); - $cl_role = $request->getParameter('role'); + $cl_role_id = $request->getParameter('role'); $cl_client_id = $request->getParameter('client'); $cl_status = $request->getParameter('status'); $cl_rate = $request->getParameter('rate'); @@ -93,7 +90,7 @@ if ($request->isPost()) { $cl_login = $user_details['login']; $cl_email = $user_details['email']; $cl_rate = str_replace('.', $user->decimal_mark, $user_details['rate']); - $cl_role = $user_details['role']; + $cl_role_id = $user_details['role_id']; $cl_client_id = $user_details['client_id']; $cl_status = $user_details['status']; $cl_projects = array(); @@ -112,11 +109,8 @@ if (!$auth->isPasswordExternal()) { } $form->addInput(array('type'=>'text','maxlength'=>'100','name'=>'email','value'=>$cl_email)); -$roles[ROLE_USER] = $i18n->getKey('label.user'); -$roles[ROLE_COMANAGER] = $i18n->getKey('form.users.comanager'); -if ($user->isPluginEnabled('cl')) - $roles[ROLE_CLIENT] = $i18n->getKey('label.client'); -$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role,'data'=>$roles)); +$active_roles = ttTeamHelper::getActiveRolesForUser(); +$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role_id,'data'=>$active_roles,'datakeys'=>array('id', 'name'))); if ($user->isPluginEnabled('cl')) $form->addInput(array('type'=>'combobox','name'=>'client','value'=>$cl_client_id,'data'=>$clients,'datakeys'=>array('id', 'name'),'empty'=>array(''=>$i18n->getKey('dropdown.select')))); @@ -189,7 +183,7 @@ if ($request->isPost()) { 'rate' => $cl_rate, 'projects' => $assigned_projects); if (in_array('manage_users', $user->rights)) { - $fields['role'] = $cl_role; + $fields['role_id'] = $cl_role_id; $fields['client_id'] = $cl_client_id; } @@ -236,6 +230,7 @@ $rates = ttProjectHelper::getRates($user_id); $smarty->assign('rates', $rates); $smarty->assign('auth_external', $auth->isPasswordExternal()); +$smarty->assign('active_roles', $active_roles); $smarty->assign('forms', array($form->getName()=>$form->toArray())); $smarty->assign('onload', 'onLoad="document.userForm.name.focus();handleClientControl();"'); $smarty->assign('user_id', $user_id); diff --git a/user_add.php b/user_add.php index 69ee3b11..a31669c7 100644 --- a/user_add.php +++ b/user_add.php @@ -56,8 +56,7 @@ if ($request->isPost()) { $cl_password2 = $request->getParameter('pas2'); } $cl_email = trim($request->getParameter('email')); - $cl_role = $request->getParameter('role'); - if (!$cl_role) $cl_role = ROLE_USER; + $cl_role_id = $request->getParameter('role'); $cl_client_id = $request->getParameter('client'); $cl_rate = $request->getParameter('rate'); $cl_projects = $request->getParameter('projects'); @@ -84,7 +83,7 @@ if (!$auth->isPasswordExternal()) { $form->addInput(array('type'=>'text','maxlength'=>'100','name'=>'email','value'=>$cl_email)); $active_roles = ttTeamHelper::getActiveRolesForUser(); -$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role,'data'=>$active_roles,'datakeys'=>array('id', 'name'))); +$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role_id,'data'=>$active_roles,'datakeys'=>array('id', 'name'))); if ($user->isPluginEnabled('cl')) $form->addInput(array('type'=>'combobox','name'=>'client','value'=>$cl_client_id,'data'=>$clients,'datakeys'=>array('id', 'name'),'empty'=>array(''=>$i18n->getKey('dropdown.select')))); @@ -140,21 +139,18 @@ if ($request->isPost()) { } if (!ttValidEmail($cl_email, true)) $err->add($i18n->getKey('error.field'), $i18n->getKey('label.email')); // Require selection of a client for a client role. - if ($user->isPluginEnabled('cl') && ttRoleHelper::isClientRole($cl_role) && !$cl_client_id) $err->add($i18n->getKey('error.client')); + if ($user->isPluginEnabled('cl') && ttRoleHelper::isClientRole($cl_role_id) && !$cl_client_id) $err->add($i18n->getKey('error.client')); if (!ttValidFloat($cl_rate, true)) $err->add($i18n->getKey('error.field'), $i18n->getKey('form.users.default_rate')); if ($err->no()) { if (!ttUserHelper::getUserByLogin($cl_login)) { - // Get legacy role value. - $legacy_role = ttRoleHelper::getLegacyRole($cl_role); // TODO: remove after roles revamp. $fields = array( 'name' => $cl_name, 'login' => $cl_login, 'password' => $cl_password1, 'rate' => $cl_rate, 'team_id' => $user->team_id, - 'role' => $legacy_role, - 'role_id' => $cl_role, + 'role_id' => $cl_role_id, 'client_id' => $cl_client_id, 'projects' => $assigned_projects, 'email' => $cl_email); diff --git a/user_delete.php b/user_delete.php index f30ec8a8..97b25572 100644 --- a/user_delete.php +++ b/user_delete.php @@ -39,23 +39,20 @@ if (!ttAccessAllowed('manage_users')) { // 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. -$ok_to_go = $user->canManageTeam(); // Are we authorized for user deletes? -if ($ok_to_go) $ok_to_go = $ok_to_go && $user_details; // Are we deleting a real user? -if ($ok_to_go) $ok_to_go = $ok_to_go && ($user->team_id == $user_details['team_id']); // User belongs to our team? -if ($ok_to_go && $user->isCoManager() && (ROLE_COMANAGER == $user_details['role'])) - $ok_to_go = ($user->id == $user_details['id']); // Comanager is not allowed to delete other comanagers. -if ($ok_to_go && $user->isCoManager() && (ROLE_MANAGER == $user_details['role'])) - $ok_to_go = false; // Comanager is not allowed to delete a manager. +if (!$user_details || // No details. + $user_details['team_id'] <> $user->team_id || // User not in team. + $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. + ) { + header('Location: access_denied.php'); + exit(); +} -if (!$ok_to_go) - die ($i18n->getKey('error.sys')); -else - $smarty->assign('user_to_delete', $user_details['name']." (".$user_details['login'].")"); +$smarty->assign('user_to_delete', $user_details['name']." (".$user_details['login'].")"); // Create confirmation form. $form = new Form('userDeleteForm'); diff --git a/user_edit.php b/user_edit.php index 8e68583c..ee572a7f 100644 --- a/user_edit.php +++ b/user_edit.php @@ -43,20 +43,17 @@ if (!ttAccessAllowed('manage_users')) { // 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. -$ok_to_go = $user->canManageTeam(); // Are we authorized for user management? -if ($ok_to_go) $ok_to_go = $ok_to_go && $user_details; // Are we editing a real user? -if ($ok_to_go) $ok_to_go = $ok_to_go && ($user->team_id == $user_details['team_id']); // User belongs to our team? -if ($ok_to_go && $user->isCoManager() && (ROLE_COMANAGER == $user_details['role'])) - $ok_to_go = ($user->id == $user_details['id']); // Comanager is not allowed to edit other comanagers. -if ($ok_to_go && $user->isCoManager() && (ROLE_MANAGER == $user_details['role'])) - $ok_to_go = false; // Comanager is not allowed to edit a manager. -if (!$ok_to_go) { - die ($i18n->getKey('error.sys')); +if (!$user_details || // No details. + $user_details['team_id'] <> $user->team_id || // User not in team. + $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. + ) { + header('Location: access_denied.php'); + exit(); } if ($user->isPluginEnabled('cl')) @@ -73,7 +70,7 @@ if ($request->isPost()) { $cl_password2 = $request->getParameter('pas2'); } $cl_email = trim($request->getParameter('email')); - $cl_role = $request->getParameter('role'); + $cl_role_id = $request->getParameter('role'); $cl_client_id = $request->getParameter('client'); $cl_status = $request->getParameter('status'); $cl_rate = $request->getParameter('rate'); @@ -94,7 +91,7 @@ if ($request->isPost()) { $cl_login = $user_details['login']; $cl_email = $user_details['email']; $cl_rate = str_replace('.', $user->decimal_mark, $user_details['rate']); - $cl_role = $user_details['role_id']; + $cl_role_id = $user_details['role_id']; $cl_client_id = $user_details['client_id']; $cl_status = $user_details['status']; $cl_projects = array(); @@ -114,7 +111,7 @@ if (!$auth->isPasswordExternal()) { $form->addInput(array('type'=>'text','maxlength'=>'100','name'=>'email','style'=>'width: 300px;','value'=>$cl_email)); $active_roles = ttTeamHelper::getActiveRolesForUser(); -$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role,'data'=>$active_roles, 'datakeys'=>array('id', 'name'))); +$form->addInput(array('type'=>'combobox','onchange'=>'handleClientControl()','name'=>'role','value'=>$cl_role_id,'data'=>$active_roles, 'datakeys'=>array('id', 'name'))); if ($user->isPluginEnabled('cl')) $form->addInput(array('type'=>'combobox','name'=>'client','value'=>$cl_client_id,'data'=>$clients,'datakeys'=>array('id', 'name'),'empty'=>array(''=>$i18n->getKey('dropdown.select')))); @@ -172,7 +169,7 @@ if ($request->isPost()) { } if (!ttValidEmail($cl_email, true)) $err->add($i18n->getKey('error.field'), $i18n->getKey('label.email')); // Require selection of a client for a client role. - if ($user->isPluginEnabled('cl') && ttRoleHelper::isClientRole($cl_role) && !$cl_client_id) $err->add($i18n->getKey('error.client')); + if ($user->isPluginEnabled('cl') && ttRoleHelper::isClientRole($cl_role_id) && !$cl_client_id) $err->add($i18n->getKey('error.client')); if (!ttValidFloat($cl_rate, true)) $err->add($i18n->getKey('error.field'), $i18n->getKey('form.users.default_rate')); if ($err->no()) { @@ -187,12 +184,8 @@ if ($request->isPost()) { 'status' => $cl_status, 'rate' => $cl_rate, 'projects' => $assigned_projects); - if (in_array('manage_users', $user->rights) && $cl_role) { - // Get legacy role value. - $legacy_role = ttRoleHelper::getLegacyRole($cl_role); // TODO: remove after roles revamp. - $fields['role'] = $legacy_role; - - $fields['role_id'] = $cl_role; + if (in_array('manage_users', $user->rights) && $cl_role_id) { + $fields['role_id'] = $cl_role_id; $fields['client_id'] = $cl_client_id; } -- 2.20.1