Wavyzz/dolibarr
2
0
Fork 1
mirror of https://github.com/Dolibarr/dolibarr.git synced 2026-01-05 16:42:53 +01:00
Code Issues Packages Projects Releases Wiki Activity

FIX empty with hasRight and add phpunit to avoid this in future

Browse Source
This commit is contained in:
Laurent Destailleur
2022-08-14 16:38:20 +02:00
parent 0bd33ae338
commit 72c010622b
8 changed files with 36 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
htdocs/accountancy/index.php
View File

@@ -49,7 +49,7 @@ if (empty($user->rights->accounting->mouvements->lire)) {
if (empty($conf->comptabilite->enabled) && empty($conf->accounting->enabled) && empty($conf->asset->enabled) && empty($conf->intracommreport->enabled)) {
accessforbidden();
}
if (empty($user->hasRight('compta', 'resultat', 'lire')) && empty($user->hasRight('accounting', 'comptarapport', 'lire')) && empty($user->hasRight('accounting', 'mouvements', 'lire')) && empty($user->hasRight('asset', 'read')) && empty($user->hasRight('intracommreport', 'read'))) {
if (!$user->hasRight('compta', 'resultat', 'lire') && !$user->hasRight('accounting', 'comptarapport', 'lire') && !$user->hasRight('accounting', 'mouvements', 'lire') && !$user->hasRight('asset', 'read') && !$user->hasRight('intracommreport', 'read')) {
accessforbidden();
}

6
htdocs/commande/card.php
View File

@@ -113,9 +113,9 @@ $usercancreate = $user->hasRight("commande", "creer");
$usercandelete = $user->hasRight("commande", "supprimer");
// Advanced permissions
$usercanclose = ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($usercancreate)) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->hasRight('commande', 'order_advance', 'close'))));
$usercanvalidate = ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $usercancreate) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->hasRight('commande', 'order_advance', 'validate'))));
$usercancancel = ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $usercancreate) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->hasRight('commande', 'order_advance', 'annuler'))));
$usercanclose = ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($usercancreate)) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $user->hasRight('commande', 'order_advance', 'close')));
$usercanvalidate = ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $usercancreate) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $user->hasRight('commande', 'order_advance', 'validate')));
$usercancancel = ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $usercancreate) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && $user->hasRight('commande', 'order_advance', 'annuler')));
$usercansend = (empty($conf->global->MAIN_USE_ADVANCED_PERMS) || $user->hasRight('commande', 'order_advance', 'send'));
$usercangeneretedoc = (empty($conf->global->MAIN_USE_ADVANCED_PERMS) || $user->hasRight('commande', 'order_advance', 'generetedoc'));

2
htdocs/document.php
View File

@@ -120,7 +120,7 @@ if ($user->socid > 0) {
// For some module part, dir may be privates
if (in_array($modulepart, array('facture_paiement', 'unpaid'))) {
if (empty($user->hasRight('societe', 'client', 'voir')) || $socid) {
if (!$user->hasRight('societe', 'client', 'voir') || $socid) {
$original_file = 'private/'.$user->id.'/'.$original_file; // If user has no permission to see all, output dir is specific to user
}
}

20
htdocs/user/card.php
View File

@@ -1165,7 +1165,7 @@ if ($action == 'create' || $action == 'adduserldap') {
}
// Categories
if (!empty($conf->categorie->enabled) && !empty($user->hasRight("categorie", "read"))) {
if (!empty($conf->categorie->enabled) && $user->hasRight("categorie", "read")) {
print '<tr><td>'.$form->editfieldkey('Categories', 'usercats', '', $object, 0).'</td><td>';
$cate_arbo = $form->select_all_categories('user', null, 'parent', null, null, 1);
print img_picto('', 'category', 'class="pictofixedwidth"').$form->multiselectarray('usercats', $cate_arbo, GETPOST('usercats', 'array'), 0, 0, 'maxwdith300 widthcentpercentminusx', 0, '90%');
@@ -1234,9 +1234,9 @@ if ($action == 'create' || $action == 'adduserldap') {
print '<input class="maxwidth200 maxwidth150onsmartphone" type="text" name="job" value="'.dol_escape_htmltag(GETPOST('job', 'alphanohtml')).'">';
print '</td></tr>';
if ((!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "read")) && in_array($id, $childids))
|| (!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "readall")))
|| (!empty($conf->hrm->enabled) && !empty($user->hasRight("hrm", "employee", "read")))) {
if ((!empty($conf->salaries->enabled) && $user->hasRight("salaries", "read") && in_array($id, $childids))
|| (!empty($conf->salaries->enabled) && $user->hasRight("salaries", "readall"))
|| (!empty($conf->hrm->enabled) && $user->hasRight("hrm", "employee", "read"))) {
$langs->load("salaries");
// THM
@@ -1541,8 +1541,8 @@ if ($action == 'create' || $action == 'adduserldap') {
// Sensitive salary/value information
if ((empty($user->socid) && in_array($id, $childids)) // A user can always see salary/value information for its subordinates
|| (!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "readall")))
|| (!empty($conf->hrm->enabled) && !empty($user->hasRight("hrm", "employee", "read")))) {
|| (!empty($conf->salaries->enabled) && $user->hasRight("salaries", "readall"))
|| (!empty($conf->hrm->enabled) && $user->hasRight("hrm", "employee", "read"))) {
$langs->load("salaries");
// Salary
@@ -1625,7 +1625,7 @@ if ($action == 'create' || $action == 'adduserldap') {
}
// Categories
if (!empty($conf->categorie->enabled) && !empty($user->hasRight("categorie", "read"))) {
if (!empty($conf->categorie->enabled) && $user->hasRight("categorie", "read")) {
print '<tr><td class="titlefield">'.$langs->trans("Categories").'</td>';
print '<td colspan="3">';
print $form->showCategories($object->id, Categorie::TYPE_USER, 1);
@@ -2567,7 +2567,7 @@ if ($action == 'create' || $action == 'adduserldap') {
print '</tr>';
// Categories
if (!empty($conf->categorie->enabled) && !empty($user->hasRight("categorie", "read"))) {
if (!empty($conf->categorie->enabled) && $user->hasRight("categorie", "read")) {
print '<tr><td>'.$form->editfieldkey('Categories', 'usercats', '', $object, 0).'</td>';
print '<td>';
print img_picto('', 'category', 'class="pictofixedwidth"');
@@ -2712,8 +2712,8 @@ if ($action == 'create' || $action == 'adduserldap') {
// Sensitive salary/value information
if ((empty($user->socid) && in_array($id, $childids)) // A user can always see salary/value information for its subordinates
|| (!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "readall")))
|| (!empty($conf->hrm->enabled) && !empty($user->hasRight("hrm", "employee", "read")))) {
|| (!empty($conf->salaries->enabled) && $user->hasRight("salaries", "readall"))
|| (!empty($conf->hrm->enabled) && $user->hasRight("hrm", "employee", "read"))) {
$langs->load("salaries");
// Salary

14
htdocs/user/list.php
View File

@@ -131,7 +131,7 @@ $arrayfields = array(
'u.email'=>array('label'=>"EMail", 'checked'=>1, 'position'=>35),
'u.api_key'=>array('label'=>"ApiKey", 'checked'=>0, 'position'=>40, "enabled"=>(!empty($conf->api->enabled) && $user->admin)),
'u.fk_soc'=>array('label'=>"Company", 'checked'=>($contextpage == 'employeelist' ? 0 : 1), 'position'=>45),
'u.salary'=>array('label'=>"Salary", 'checked'=>1, 'position'=>80, 'enabled'=>(!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "readall")))),
'u.salary'=>array('label'=>"Salary", 'checked'=>1, 'position'=>80, 'enabled'=>(!empty($conf->salaries->enabled) && $user->hasRight("salaries", "readall"))),
'u.datelastlogin'=>array('label'=>"LastConnexion", 'checked'=>1, 'position'=>100),
'u.datepreviouslogin'=>array('label'=>"PreviousConnexion", 'checked'=>0, 'position'=>110),
'u.datec'=>array('label'=>"DateCreation", 'checked'=>0, 'position'=>500),
@@ -189,11 +189,11 @@ $error = 0;
// Permission to list
if ($mode == 'employee') {
if (empty($user->hasRight("salaries", "read"))) {
if (!$user->hasRight("salaries", "read")) {
accessforbidden();
}
} else {
if (empty($user->hasRight("user", "user", "read")) && empty($user->admin)) {
if (!$user->hasRight("user", "user", "read") && empty($user->admin)) {
accessforbidden();
}
}
@@ -441,7 +441,7 @@ if ($search_categ == -2) {
if ($search_warehouse > 0) {
$sql .= " AND u.fk_warehouse = ".((int) $search_warehouse);
}
if ($mode == 'employee' && empty($user->hasRight("salaries", "readall"))) {
if ($mode == 'employee' && !$user->hasRight("salaries", "readall")) {
$sql .= " AND u.rowid IN (".$db->sanitize(join(',', $childids)).")";
}
// Add where from extra fields
@@ -939,9 +939,9 @@ while ($i < $imaxinloop) {
$li = $object->getNomUrl(-1, '', 0, 0, 24, 1, 'login', '', 1);
$canreadhrmdata = 0;
if ((!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "read")) && in_array($obj->rowid, $childids))
|| (!empty($conf->salaries->enabled) && !empty($user->hasRight("salaries", "readall")))
|| (!empty($conf->hrm->enabled) && !empty($user->hasRight("hrm", "employee", "read")))) {
if ((!empty($conf->salaries->enabled) && $user->hasRight("salaries", "read") && in_array($obj->rowid, $childids))
|| (!empty($conf->salaries->enabled) && $user->hasRight("salaries", "readall"))
|| (!empty($conf->hrm->enabled) && $user->hasRight("hrm", "employee", "read"))) {
$canreadhrmdata = 1;
}
$canreadsecretapi = 0;

2
htdocs/user/note.php
View File

@@ -138,7 +138,7 @@ if ($id) {
}
print '</tr>';
$editenabled = (($action == 'edit') && !empty($user->hasRight("user", "user", "write")));
$editenabled = (($action == 'edit') && $user->hasRight("user", "user", "write"));
// Note
print '<tr><td class="tdtop">'.$langs->trans("Note").'</td>';

2
htdocs/user/perms.php
View File

@@ -67,7 +67,7 @@ if (isset($user->socid) && $user->socid > 0) {
}
$feature2 = (($socid && $user->hasRight("user", "self", "write")) ? '' : 'user');
// A user can always read its own card if not advanced perms enabled, or if he has advanced perms, except for admin
if ($user->id == $id && (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && empty($user->hasRight("user", "self_advance", "readperms")) && empty($user->admin))) {
if ($user->id == $id && (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !$user->hasRight("user", "self_advance", "readperms") && empty($user->admin))) {
accessforbidden();
}

13
test/phpunit/CodingPhpTest.php
View File

@@ -513,7 +513,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
$this->assertTrue($ok, 'Found a forbidden string sequence into '.$file['relativename'].' : name="token" value="\'.$_SESSION[..., you must use a newToken() instead of $_SESSION[\'newtoken\'].');
// Test we don't have @var array(
// Test we don't have preg_grep with a param without preg_quote
$ok=true;
$matches=array();
preg_match_all('/preg_grep\(.*\$/', $filecontent, $matches, PREG_SET_ORDER);
@@ -526,6 +526,17 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
$this->assertTrue($ok, 'Found a preg_grep with a param that is a $var but without preg_quote in file '.$file['relativename'].'.');
// Test we don't have empty($user->hasRight
$ok=true;
$matches=array();
preg_match_all('/empty\(\$user->hasRight/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
$ok=false;
break;
}
$this->assertTrue($ok, 'Found code empty($user->hasRight in file '.$file['relativename'].'. empty() must not be used with hasRight.');
// Test we don't have @var array(
$ok=true;
$matches=array();