diff --git a/htdocs/contact/card.php b/htdocs/contact/card.php index eaaebffdab8..f7a52afa489 100644 --- a/htdocs/contact/card.php +++ b/htdocs/contact/card.php @@ -178,7 +178,7 @@ if (empty($reshook)) { // Confirmation deactivation - if ($action == 'disable' && !empty($permissiontoadd)) { + if ($action == 'disable' && $permissiontoadd) { $object->fetch($id); if ($object->setstatus(0) < 0) { setEventMessages($object->error, $object->errors, 'errors'); @@ -189,7 +189,7 @@ if (empty($reshook)) { } // Confirmation activation - if ($action == 'enable' && !empty($permissiontoadd)) { + if ($action == 'enable' && $permissiontoadd) { $object->fetch($id); if ($object->setstatus(1) < 0) { setEventMessages($object->error, $object->errors, 'errors'); @@ -200,7 +200,7 @@ if (empty($reshook)) { } // Add contact - if ($action == 'add' && !empty($permissiontoadd)) { + if ($action == 'add' && $permissiontoadd) { $db->begin(); if ($canvas) { @@ -337,7 +337,7 @@ if (empty($reshook)) { } } - if ($action == 'update' && empty($cancel) && !empty($permissiontoadd)) { + if ($action == 'update' && empty($cancel) && $permissiontoadd) { if (!GETPOST("lastname", 'alpha')) { $error++; $errors = array($langs->trans("ErrorFieldRequired", $langs->transnoentities("Name").' / '.$langs->transnoentities("Label"))); @@ -482,7 +482,7 @@ if (empty($reshook)) { } } - if ($action == 'setprospectcontactlevel' && !empty($permissiontoadd)) { + if ($action == 'setprospectcontactlevel' && $permissiontoadd) { $object->fetch($id); $object->fk_prospectlevel = GETPOST('prospect_contact_level_id', 'alpha'); $result = $object->update($object->id, $user); @@ -492,7 +492,7 @@ if (empty($reshook)) { } // set communication status - if ($action == 'setstcomm' && !empty($permissiontoadd)) { + if ($action == 'setstcomm' && $permissiontoadd) { $object->fetch($id); $object->stcomm_id = dol_getIdFromCode($db, GETPOST('stcomm', 'alpha'), 'c_stcommcontact'); $result = $object->update($object->id, $user); @@ -502,7 +502,7 @@ if (empty($reshook)) { } // Update extrafields - if ($action == "update_extras" && !empty($permissiontoadd)) { + if ($action == "update_extras" && $permissiontoadd) { $object->fetch(GETPOSTINT('id')); $attributekey = GETPOST('attribute', 'alpha'); diff --git a/htdocs/workstation/class/workstation.class.php b/htdocs/workstation/class/workstation.class.php index 57326018dc7..e5e0fdeb0ce 100644 --- a/htdocs/workstation/class/workstation.class.php +++ b/htdocs/workstation/class/workstation.class.php @@ -241,7 +241,7 @@ class Workstation extends CommonObject $id = $this->createCommon($user, $notrigger); // Usergroups - $groups = GETPOST('groups', 'array:int'); + $groups = GETPOST('groups', 'array:int'); // FIXME We should not GETPOST but receive array as parameter if (empty($groups)) { $groups = $this->usergroups; // createFromClone } @@ -256,7 +256,7 @@ class Workstation extends CommonObject } // Resources - $resources = GETPOST('resources', 'array:int'); + $resources = GETPOST('resources', 'array:int'); // FIXME We should not GETPOST but receive array as parameter if (empty($resources)) { $resources = $this->resources; // createFromClone } diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index fe3a4760003..fbf5d296ff7 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -628,6 +628,27 @@ class CodingPhpTest extends CommonClassTest break; } $this->assertTrue($ok, 'Found a CURDATE\(\) in code. Do not use this SQL method in file '.$file['relativename'].'. You must use the PHP function dol_now() instead.'); + + + // Test we don't have if ($action == 'xxx'... without test on permission + // We do not test on file into admin, protection is done on page on user->admin + if (!preg_match('/admin\//', $filecontent) + && !preg_match('/\.tpl\.php/', $filecontent) + && !preg_match('/\.lib\.php/', $filecontent) + && !preg_match('/\.inc\.php/', $filecontent) + && !preg_match('/\.class\.php/', $filecontent)) { + $ok = true; + $matches = array(); + preg_match_all('/if\s*\(\s*\$action\s*==\s*[\'"][a-z]+[\'"].*/', $filecontent, $matches, PREG_SET_ORDER); + foreach ($matches as $key => $val) { + if (!preg_match('/\$user->hasR/', $val[0])) { + $ok = false; + print "Line: ".$val[0]."\n"; + break; + } + } + $this->assertTrue($ok, 'Found a test on action without check on permission and without comment to say this is expected, in file '.$file['relativename'].'.'); + } }