diff --git a/htdocs/accountancy/index.php b/htdocs/accountancy/index.php
index 926097c1509..f6f86461d9b 100644
--- a/htdocs/accountancy/index.php
+++ b/htdocs/accountancy/index.php
@@ -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();
}
diff --git a/htdocs/commande/card.php b/htdocs/commande/card.php
index 16cb1d24b27..12e29ac9b2b 100644
--- a/htdocs/commande/card.php
+++ b/htdocs/commande/card.php
@@ -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'));
diff --git a/htdocs/document.php b/htdocs/document.php
index e432d6a92f1..65e5cce7e5f 100644
--- a/htdocs/document.php
+++ b/htdocs/document.php
@@ -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
}
}
diff --git a/htdocs/user/card.php b/htdocs/user/card.php
index c1dc297bfcb..139834c6a64 100644
--- a/htdocs/user/card.php
+++ b/htdocs/user/card.php
@@ -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 '
'.$form->editfieldkey('Categories', 'usercats', '', $object, 0).' ';
$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 ' ';
print ' ';
- 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 ''.$langs->trans("Categories").' ';
print '';
print $form->showCategories($object->id, Categorie::TYPE_USER, 1);
@@ -2567,7 +2567,7 @@ if ($action == 'create' || $action == 'adduserldap') {
print ' ';
// Categories
- if (!empty($conf->categorie->enabled) && !empty($user->hasRight("categorie", "read"))) {
+ if (!empty($conf->categorie->enabled) && $user->hasRight("categorie", "read")) {
print ''.$form->editfieldkey('Categories', 'usercats', '', $object, 0).' ';
print '';
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
diff --git a/htdocs/user/list.php b/htdocs/user/list.php
index f0196789e25..443893fa096 100644
--- a/htdocs/user/list.php
+++ b/htdocs/user/list.php
@@ -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;
diff --git a/htdocs/user/note.php b/htdocs/user/note.php
index 2b1e02b1376..8269dfa93aa 100644
--- a/htdocs/user/note.php
+++ b/htdocs/user/note.php
@@ -138,7 +138,7 @@ if ($id) {
}
print ' ';
- $editenabled = (($action == 'edit') && !empty($user->hasRight("user", "user", "write")));
+ $editenabled = (($action == 'edit') && $user->hasRight("user", "user", "write"));
// Note
print ''.$langs->trans("Note").' ';
diff --git a/htdocs/user/perms.php b/htdocs/user/perms.php
index bfc234abce1..ef00c90d6cb 100644
--- a/htdocs/user/perms.php
+++ b/htdocs/user/perms.php
@@ -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();
}
diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php
index bff78d47b94..6388d3985e7 100644
--- a/test/phpunit/CodingPhpTest.php
+++ b/test/phpunit/CodingPhpTest.php
@@ -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();