From 4a303a61d18efaf44e1049595887fe364b02365b Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Fri, 13 Sep 2024 11:13:38 +0200 Subject: [PATCH] More robust test on missing permission for CTI --- htdocs/core/ajax/vatrates.php | 7 +++++++ htdocs/hrm/skill_tab.php | 14 ++++++-------- htdocs/install/step1.php | 7 +++++++ htdocs/public/onlinesign/newonlinesign.php | 2 +- htdocs/public/ticket/ajax/ajax.php | 13 +++++++++++-- htdocs/supplier_proposal/info.php | 10 ++++++++++ test/phpunit/CodingPhpTest.php | 3 ++- 7 files changed, 44 insertions(+), 12 deletions(-) diff --git a/htdocs/core/ajax/vatrates.php b/htdocs/core/ajax/vatrates.php index 4c15aea792d..2cab4ca9a8e 100644 --- a/htdocs/core/ajax/vatrates.php +++ b/htdocs/core/ajax/vatrates.php @@ -43,6 +43,13 @@ $productid = (GETPOSTINT('productid') ? GETPOSTINT('productid') : 0); $result = restrictedArea($user, 'societe', $id, '&societe', '', 'fk_soc', 'rowid', 0); +/* + * Actions + */ + +// None + + /* * View */ diff --git a/htdocs/hrm/skill_tab.php b/htdocs/hrm/skill_tab.php index 443eea7b19f..66354128fc8 100644 --- a/htdocs/hrm/skill_tab.php +++ b/htdocs/hrm/skill_tab.php @@ -127,7 +127,7 @@ if (empty($reshook)) { } // update national_registration_number - if ($action == 'setnational_registration_number') { + if ($action == 'setnational_registration_number' && $permissiontoadd) { $object->national_registration_number = (string) GETPOST('national_registration_number', 'alphanohtml'); $result = $object->update($user); if ($result < 0) { @@ -135,7 +135,7 @@ if (empty($reshook)) { } } - if ($action == 'addSkill') { + if ($action == 'addSkill' && $permissiontoadd) { $error = 0; if (empty($TSkillsToAdd)) { @@ -158,7 +158,7 @@ if (empty($reshook)) { setEventMessages($langs->trans("SaveAddSkill"), null); } } - } elseif ($action == 'saveSkill') { + } elseif ($action == 'saveSkill' && $permissiontoadd) { if (!empty($TNote)) { foreach ($TNote as $skillId => $rank) { $TSkills = $skill->fetchAll('ASC', 't.rowid', 0, 0, '(fk_object:=:'.((int) $id).") AND (objecttype:=:'".$db->escape($objecttype)."') AND (fk_skill:=:".((int) $skillId).')'); @@ -173,7 +173,7 @@ if (empty($reshook)) { header("Location: " . DOL_URL_ROOT.'/hrm/skill_tab.php?id=' . $id. '&objecttype=job'); exit; } - } elseif ($action == 'confirm_deleteskill' && $confirm == 'yes') { + } elseif ($action == 'confirm_deleteskill' && $confirm == 'yes' && $permissiontoadd) { $skillToDelete = new SkillRank($db); $ret = $skillToDelete->fetch($lineid); setEventMessages($langs->trans("DeleteSkill"), null); @@ -183,6 +183,7 @@ if (empty($reshook)) { } } + /* * View */ @@ -216,15 +217,12 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $formconfirm = ''; // Confirmation to delete - /*if ($action == 'delete') { - $formconfirm = $form->formconfirm($_SERVER["PHP_SELF"].'?id='.$object->id, $langs->trans('DeleteSkill'), $langs->trans('ConfirmDeleteObject'), 'confirm_delete', '', 0, 1); - }*/ // Confirmation to delete line if ($action == 'ask_deleteskill') { $formconfirm = $form->formconfirm($_SERVER["PHP_SELF"] . '?id=' . $object->id . '&objecttype=' . $objecttype . '&lineid=' . $lineid, $langs->trans('DeleteLine'), $langs->trans('ConfirmDeleteLine'), 'confirm_deleteskill', '', 0, 1); } // Clone confirmation - /*if ($action == 'clone') { + /*if ($action == 'clone' && $permissiontoadd) { // Create an array for form $formquestion = array(); $formconfirm = $form->formconfirm($_SERVER["PHP_SELF"].'?id='.$object->id, $langs->trans('ToClone'), $langs->trans('ConfirmCloneAsk', $object->ref), 'confirm_clone', $formquestion, 'yes', 1); diff --git a/htdocs/install/step1.php b/htdocs/install/step1.php index adf2d2f4839..d884465f093 100644 --- a/htdocs/install/step1.php +++ b/htdocs/install/step1.php @@ -159,6 +159,13 @@ if (@file_exists($forcedfile)) { $error = 0; +/* + * Actions + */ + +// None + + /* * View */ diff --git a/htdocs/public/onlinesign/newonlinesign.php b/htdocs/public/onlinesign/newonlinesign.php index 05e715b3be2..b2eb79195e4 100644 --- a/htdocs/public/onlinesign/newonlinesign.php +++ b/htdocs/public/onlinesign/newonlinesign.php @@ -178,7 +178,7 @@ $error = 0; * Actions */ -if ($action == 'confirm_refusepropal' && $confirm == 'yes') { +if ($action == 'confirm_refusepropal' && $confirm == 'yes') { // Test on pemrission not required here. Public form. Security checked on the securekey and on mitigation $db->begin(); $sql = "UPDATE ".MAIN_DB_PREFIX."propal"; diff --git a/htdocs/public/ticket/ajax/ajax.php b/htdocs/public/ticket/ajax/ajax.php index 33a2d210d72..14295437113 100644 --- a/htdocs/public/ticket/ajax/ajax.php +++ b/htdocs/public/ticket/ajax/ajax.php @@ -1,6 +1,6 @@ + * Copyright (C) 2020-2024 Laurent Destailleur * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -61,18 +61,27 @@ if (!isModEnabled('ticket')) { httponly_accessforbidden('Module Ticket not enabled'); } +// Option TICKET_CREATE_THIRD_PARTY_WITH_CONTACT_IF_NOT_EXIST must be set. +// Warning: this option is not secured so has been disabled from setup. if (!getDolGlobalString('TICKET_CREATE_THIRD_PARTY_WITH_CONTACT_IF_NOT_EXIST')) { httponly_accessforbidden('Option TICKET_CREATE_THIRD_PARTY_WITH_CONTACT_IF_NOT_EXIST of module ticket is not enabled'); } +/* + * Actions + */ + +// None + + /* * View */ top_httphead(); -if ($action == 'getContacts') { +if ($action == 'getContacts') { // Test on permission not required here. Access is allowed only if TICKET_CREATE_THIRD_PARTY_WITH_CONTACT_IF_NOT_EXIST is on and option has been disabled because not secured. $return = array( 'contacts' => array(), 'error' => '', diff --git a/htdocs/supplier_proposal/info.php b/htdocs/supplier_proposal/info.php index a15c1e045b7..92e1c380a67 100644 --- a/htdocs/supplier_proposal/info.php +++ b/htdocs/supplier_proposal/info.php @@ -46,10 +46,20 @@ if (!empty($user->socid)) { } $result = restrictedArea($user, 'supplier_proposal', $id); +$permissiontoadd = $user->hasRight('supplier_proposal', 'creer'); + + +/* + * Actions + */ + +// None + /* * View */ + $form = new Form($db); $object = new SupplierProposal($db); $object->fetch($id); diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index c98b966ec97..4a2a4067e99 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -662,7 +662,8 @@ class CodingPhpTest extends CommonClassTest $filecontentaction = $filecontent; } - preg_match_all('/if.*\$action\s*==\s*[\'"][a-z\-]+[\'"].*$/', $filecontentaction, $matches, PREG_SET_ORDER); + preg_match_all('/if.*\$action\s*==\s*[\'"][a-z\-]+[\'"].*$/si', $filecontentaction, $matches, PREG_SET_ORDER); + foreach ($matches as $key => $val) { if (!preg_match('/\$user->hasR/', $val[0]) && !preg_match('/\$permission/', $val[0])