diff --git a/htdocs/asset/accountancy_codes.php b/htdocs/asset/accountancy_codes.php index f68e835a1ff..11a3774010f 100644 --- a/htdocs/asset/accountancy_codes.php +++ b/htdocs/asset/accountancy_codes.php @@ -116,7 +116,7 @@ if (empty($reshook)) { $action = ''; } - if ($action == "update") { + if ($action == "update" && $permissiontoadd) { $assetaccountancycodes->setAccountancyCodesFromPost(); $result = $assetaccountancycodes->updateAccountancyCodes($user, $object->id); diff --git a/htdocs/asset/card.php b/htdocs/asset/card.php index 24af02996b7..6f89e4a3442 100644 --- a/htdocs/asset/card.php +++ b/htdocs/asset/card.php @@ -135,7 +135,7 @@ if (empty($reshook)) { setEventMessages($object->error, $object->errors, 'errors'); } $action = ''; - } elseif ($action == "add") { + } elseif ($action == "add" && $permissiontoadd) { $object->supplier_invoice_id = GETPOSTINT('supplier_invoice_id'); } diff --git a/htdocs/asset/depreciation_options.php b/htdocs/asset/depreciation_options.php index 6202e4f2b6f..577439b9fcd 100644 --- a/htdocs/asset/depreciation_options.php +++ b/htdocs/asset/depreciation_options.php @@ -110,7 +110,7 @@ if (empty($reshook)) { $action = ''; } - if ($action == "update") { + if ($action == "update" && $permissiontoadd) { $result = $assetdepreciationoptions->setDeprecationOptionsFromPost(); if ($result > 0) { $result = $assetdepreciationoptions->updateDeprecationOptions($user, $object->id); diff --git a/htdocs/asset/model/accountancy_codes.php b/htdocs/asset/model/accountancy_codes.php index 61db2261bb1..dff9ca59ace 100644 --- a/htdocs/asset/model/accountancy_codes.php +++ b/htdocs/asset/model/accountancy_codes.php @@ -108,7 +108,7 @@ if (empty($reshook)) { $action = ''; } - if ($action == "update") { + if ($action == "update" && $permissiontoadd) { $assetaccountancycodes->setAccountancyCodesFromPost(); $result = $assetaccountancycodes->updateAccountancyCodes($user, 0, $object->id); diff --git a/htdocs/asset/model/depreciation_options.php b/htdocs/asset/model/depreciation_options.php index 5beeb1a7a33..aefd822dd84 100644 --- a/htdocs/asset/model/depreciation_options.php +++ b/htdocs/asset/model/depreciation_options.php @@ -110,7 +110,7 @@ if (empty($reshook)) { $action = ''; } - if ($action == "update") { + if ($action == "update" && $permissiontoadd) { $result = $assetdepreciationoptions->setDeprecationOptionsFromPost(1); if ($result > 0) { $result = $assetdepreciationoptions->updateDeprecationOptions($user, 0, $object->id); diff --git a/htdocs/barcode/codeinit.php b/htdocs/barcode/codeinit.php index de1f4f96e4d..dd260fa9db3 100644 --- a/htdocs/barcode/codeinit.php +++ b/htdocs/barcode/codeinit.php @@ -96,7 +96,7 @@ if (getDolGlobalString('BARCODE_THIRDPARTY_ADDON_NUM')) { } } -if ($action == 'initbarcodethirdparties') { +if ($action == 'initbarcodethirdparties' && $user->hasRight('societe', 'lire')) { if (!is_object($modBarCodeThirdparty)) { $error++; setEventMessages($langs->trans("NoBarcodeNumberingTemplateDefined"), null, 'errors'); @@ -202,7 +202,7 @@ if (getDolGlobalString('BARCODE_PRODUCT_ADDON_NUM')) { } } -if ($action == 'initbarcodeproducts') { +if ($action == 'initbarcodeproducts' && $user->hasRight('produit', 'lire')) { if (!is_object($modBarCodeProduct)) { $error++; setEventMessages($langs->trans("NoBarcodeNumberingTemplateDefined"), null, 'errors'); diff --git a/htdocs/barcode/printsheet.php b/htdocs/barcode/printsheet.php index 92581f000f0..e08cc5b86bc 100644 --- a/htdocs/barcode/printsheet.php +++ b/htdocs/barcode/printsheet.php @@ -122,7 +122,7 @@ if (empty($reshook)) { } } - if ($action == 'builddoc') { + if ($action == 'builddoc' && $user->hasRight('barcode', 'read')) { $result = 0; $error = 0; diff --git a/htdocs/bom/bom_net_needs.php b/htdocs/bom/bom_net_needs.php index c2660ace0cb..dcf420baf4d 100644 --- a/htdocs/bom/bom_net_needs.php +++ b/htdocs/bom/bom_net_needs.php @@ -115,13 +115,6 @@ if (empty($reshook)) { } } } - - $TChildBom = array(); - if ($action == 'treeview') { - $object->getNetNeedsTree($TChildBom, 1); - } else { - $object->getNetNeeds($TChildBom, 1); - } } @@ -134,9 +127,18 @@ $formfile = new FormFile($db); $title = $langs->trans('BOM'); $help_url ='EN:Module_BOM'; + llxHeader('', $title, $help_url, '', 0, 0, '', '', '', 'mod-bom page-net_needs'); +$TChildBom = array(); +if ($action == 'treeview') { + $object->getNetNeedsTree($TChildBom, 1); +} else { + $object->getNetNeeds($TChildBom, 1); +} + + // Part to show record if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = bomPrepareHead($object); diff --git a/htdocs/bookmarks/card.php b/htdocs/bookmarks/card.php index 47eece588dd..85e2fb7daa3 100644 --- a/htdocs/bookmarks/card.php +++ b/htdocs/bookmarks/card.php @@ -64,8 +64,8 @@ $permissiontodelete = $user->hasRight('bookmark', 'supprimer'); * Actions */ -if ($action == 'add' || $action == 'addproduct' || $action == 'update') { - if ($action == 'update') { +if (($action == 'add' || $action == 'addproduct' || $action == 'update') && $permissiontoadd) { + if ($action == 'update') { // Test on permission already done $invertedaction = 'edit'; } else { $invertedaction = 'create'; @@ -81,7 +81,7 @@ if ($action == 'add' || $action == 'addproduct' || $action == 'update') { exit; } - if ($action == 'update') { + if ($action == 'update') { // Test on permission already done $object->fetch(GETPOSTINT("id")); } // Check if null because user not admin can't set an user and send empty value here. @@ -106,7 +106,7 @@ if ($action == 'add' || $action == 'addproduct' || $action == 'update') { if (!$error) { $object->favicon = 'none'; - if ($action == 'update') { + if ($action == 'update') { // Test on permission already done $res = $object->update(); } else { $res = $object->create(); diff --git a/htdocs/categories/card.php b/htdocs/categories/card.php index a92b7c44efb..11552b32c96 100644 --- a/htdocs/categories/card.php +++ b/htdocs/categories/card.php @@ -100,6 +100,7 @@ $error = 0; /* * Actions */ + $parameters = array('socid' => $socid, 'origin' => $origin, 'catorigin' => $catorigin, 'type' => $type, 'urlfrom' => $urlfrom, 'backtopage' => $backtopage, 'label' => $label, 'description' => $description, 'color' => $color, 'position' => $position, 'visible' => $visible, 'parent' => $parent); // Note that $action and $object may be modified by some hooks $reshook = $hookmanager->executeHooks('doActions', $parameters, $object, $action); @@ -174,42 +175,40 @@ if (empty($reshook)) { } } } - // Confirm action - if (($action == 'add' || $action == 'confirmed') && $user->hasRight('categorie', 'creer')) { - // Action confirmation of creation category - if ($action == 'confirmed') { - if ($urlfrom) { - header("Location: ".$urlfrom); - exit; - } elseif ($backtopage) { - header("Location: ".$backtopage); - exit; - } elseif ($idProdOrigin) { - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idProdOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); - exit; - } elseif ($idCompanyOrigin) { - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idCompanyOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); - exit; - } elseif ($idSupplierOrigin) { - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idSupplierOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); - exit; - } elseif ($idMemberOrigin) { - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idMemberOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); - exit; - } elseif ($idContactOrigin) { - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idContactOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); - exit; - } elseif ($idProjectOrigin) { - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idProjectOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); - exit; - } - - header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$result.'&type='.$type); + // Action confirmation of creation category + if ($action == 'confirmed' && $user->hasRight('categorie', 'creer')) { + if ($urlfrom) { + header("Location: ".$urlfrom); + exit; + } elseif ($backtopage) { + header("Location: ".$backtopage); + exit; + } elseif ($idProdOrigin) { + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idProdOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); + exit; + } elseif ($idCompanyOrigin) { + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idCompanyOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); + exit; + } elseif ($idSupplierOrigin) { + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idSupplierOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); + exit; + } elseif ($idMemberOrigin) { + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idMemberOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); + exit; + } elseif ($idContactOrigin) { + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idContactOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); + exit; + } elseif ($idProjectOrigin) { + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$idProjectOrigin.'&type='.$type.'&mesg='.urlencode($langs->trans("CatCreated"))); exit; } + + header("Location: ".DOL_URL_ROOT.'/categories/viewcat.php?id='.$result.'&type='.$type); + exit; } } + /* * View */ diff --git a/htdocs/categories/photos.php b/htdocs/categories/photos.php index 8b47e14e4bc..4d299e86ea8 100644 --- a/htdocs/categories/photos.php +++ b/htdocs/categories/photos.php @@ -48,9 +48,6 @@ if ($id == '' && $label == '') { exit(); } -// Security check -$result = restrictedArea($user, 'categorie', $id, '&category'); - $object = new Categorie($db); $result = $object->fetch($id, $label); if ($result <= 0) { @@ -67,9 +64,16 @@ $upload_dir = $conf->categorie->multidir_output[$object->entity]; $hookmanager->initHooks(array('categorycard')); +// Security check +$result = restrictedArea($user, 'categorie', $id, '&category'); + +$permissiontoadd = $user->hasRight('categorie', 'creer'); + + /* * Actions */ + $parameters = array('id' => $id, 'label' => $label, 'confirm' => $confirm, 'type' => $type, 'uploaddir' => $upload_dir, 'sendfile' => (GETPOST("sendit") ? true : false)); // Note that $action and $object may be modified by some hooks $reshook = $hookmanager->executeHooks('doActions', $parameters, $object, $action); @@ -96,11 +100,11 @@ if (empty($reshook)) { } } - if ($action == 'confirm_delete' && GETPOST("file") && $confirm == 'yes' && $user->hasRight('categorie', 'creer')) { + if ($action == 'confirm_delete' && GETPOST("file") && $confirm == 'yes' && $permissiontoadd) { $object->delete_photo($upload_dir."/".GETPOST("file")); } - if ($action == 'addthumb' && GETPOST("file")) { + if ($action == 'addthumb' && GETPOST("file") && $permissiontoadd) { $object->addThumbs($upload_dir."/".GETPOST("file")); } } diff --git a/htdocs/categories/traduction.php b/htdocs/categories/traduction.php index 4690a5f8048..8ecd22e6c19 100644 --- a/htdocs/categories/traduction.php +++ b/htdocs/categories/traduction.php @@ -46,9 +46,6 @@ if ($id == '' && $label == '') { exit(); } -// Security check -$result = restrictedArea($user, 'categorie', $id, '&category'); - $object = new Categorie($db); $result = $object->fetch($id, $label); if ($result <= 0) { @@ -61,6 +58,11 @@ if (is_numeric($type)) { $type = Categorie::$MAP_ID_TO_CODE[$type]; // For backward compatibility } +// Security check +$result = restrictedArea($user, 'categorie', $id, '&category'); + +$permissiontoadd = $user->hasRight('categorie', 'creer'); + /* * Actions @@ -75,9 +77,7 @@ if ($cancel == $langs->trans("Cancel")) { // validation of addition -if ($action == 'vadd' && -$cancel != $langs->trans("Cancel") && -($user->hasRight('categorie', 'creer'))) { +if ($action == 'vadd' && $cancel != $langs->trans("Cancel") && $permissiontoadd) { $object->fetch($id); $current_lang = $langs->getDefaultLang(); @@ -124,9 +124,7 @@ $cancel != $langs->trans("Cancel") && } // validation of the edition -if ($action == 'vedit' && -$cancel != $langs->trans("Cancel") && -($user->hasRight('categorie', 'creer'))) { +if ($action == 'vedit' && $cancel != $langs->trans("Cancel") && $permissiontoadd) { $object->fetch($id); $current_lang = $langs->getDefaultLang(); diff --git a/htdocs/collab/index.php b/htdocs/collab/index.php index bee5d778dc7..a89d50b66dc 100644 --- a/htdocs/collab/index.php +++ b/htdocs/collab/index.php @@ -76,7 +76,8 @@ if (empty($action)) { $action = 'preview'; } - +$permissiontoadd = $user->hasRight('collab', 'read'); +$permissiontodelete = $user->hasRight('collab', 'delete'); /* @@ -92,7 +93,7 @@ if (GETPOST('refreshpage')) { // Add a collab page -if ($action == 'add') { +if ($action == 'add' && $permissiontoadd) { $db->begin(); $objectpage->title = GETPOST('WEBSITE_TITLE'); @@ -125,7 +126,7 @@ if ($action == 'add') { } // Update page -if ($action == 'delete') { +if ($action == 'delete' && $permissiontodelete) { $db->begin(); $res = $object->fetch(0, $website); diff --git a/htdocs/comm/action/card.php b/htdocs/comm/action/card.php index 5bec17b11b7..755013df254 100644 --- a/htdocs/comm/action/card.php +++ b/htdocs/comm/action/card.php @@ -185,10 +185,10 @@ if (empty($reshook) && (GETPOST('removedassigned') || GETPOST('removedassigned') $_SESSION['assignedtouser'] = json_encode($tmpassigneduserids); $donotclearsession = 1; - if ($action == 'add') { + if ($action == 'add' && $usercancreate) { $action = 'create'; } - if ($action == 'update') { + if ($action == 'update' && $usercancreate) { $action = 'edit'; } @@ -212,10 +212,10 @@ if (empty($reshook) && (GETPOST('removedassignedresource') || GETPOST('removedas $_SESSION['assignedtoresource'] = json_encode($tmpassignedresourceids); $donotclearsessionresource = 1; - if ($action == 'add') { + if ($action == 'add' && $usercancreate) { $action = 'create'; } - if ($action == 'update') { + if ($action == 'update' && $usercancreate) { $action = 'edit'; } @@ -234,10 +234,10 @@ if (empty($reshook) && (GETPOST('addassignedtouser') || GETPOST('updateassignedt $_SESSION['assignedtouser'] = json_encode($assignedtouser); } $donotclearsession = 1; - if ($action == 'add') { + if ($action == 'add' && $usercancreate) { $action = 'create'; } - if ($action == 'update') { + if ($action == 'update' && $usercancreate) { $action = 'edit'; } @@ -256,10 +256,10 @@ if (empty($reshook) && (GETPOST('addassignedtoresource') || GETPOST('updateassig $_SESSION['assignedtoresource'] = json_encode($assignedtoresource); } $donotclearsession = 1; - if ($action == 'add') { + if ($action == 'add' && $usercancreate) { $action = 'create'; } - if ($action == 'update') { + if ($action == 'update' && $usercancreate) { $action = 'edit'; } @@ -274,7 +274,7 @@ if (empty($reshook) && $action == 'classin' && ($user->hasRight('agenda', 'allac } // Action clone object -if (empty($reshook) && $action == 'confirm_clone' && $confirm == 'yes') { +if (empty($reshook) && $action == 'confirm_clone' && $confirm == 'yes' && $usercancreate) { if (1 == 0 && !GETPOST('clone_content') && !GETPOST('clone_receivers')) { setEventMessages($langs->trans("NoCloneOptionsSpecified"), null, 'errors'); } else { @@ -297,7 +297,7 @@ if (empty($reshook) && $action == 'confirm_clone' && $confirm == 'yes') { } // Add event -if (empty($reshook) && $action == 'add') { +if (empty($reshook) && $action == 'add' && $usercancreate) { $error = 0; if (empty($backtopage)) { @@ -735,10 +735,8 @@ if (empty($reshook) && $action == 'add') { } } -/* - * Action update event - */ -if (empty($reshook) && $action == 'update') { +// Action update event +if (empty($reshook) && $action == 'update' && $usercancreate) { if (empty($cancel)) { $fulldayevent = GETPOST('fullday'); $aphour = GETPOSTINT('aphour'); diff --git a/htdocs/product/card.php b/htdocs/product/card.php index 53d67d84693..8422ba4e103 100644 --- a/htdocs/product/card.php +++ b/htdocs/product/card.php @@ -201,7 +201,7 @@ if ($object->id > 0) { $hookmanager->initHooks(array('productcard', 'globalcard')); // Permissions -$usercanread = (($object->type == Product::TYPE_PRODUCT && $user->hasRight('produit', 'read')) || ($object->type == Product::TYPE_SERVICE && $user->hasRight('service', 'lire'))); +$usercanread = (($object->type == Product::TYPE_PRODUCT && $user->hasRight('produit', 'lire')) || ($object->type == Product::TYPE_SERVICE && $user->hasRight('service', 'lire'))); $usercancreate = (($object->type == Product::TYPE_PRODUCT && $user->hasRight('produit', 'creer')) || ($object->type == Product::TYPE_SERVICE && $user->hasRight('service', 'creer'))); $usercandelete = (($object->type == Product::TYPE_PRODUCT && $user->hasRight('produit', 'supprimer')) || ($object->type == Product::TYPE_SERVICE && $user->hasRight('service', 'supprimer'))); diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 8dd4a1dee6b..747745a6b86 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -650,7 +650,11 @@ class CodingPhpTest extends CommonClassTest preg_match_all('/if\s*\(\s*\$action\s*==\s*[\'"][a-z]+[\'"].*/', $filecontentaction, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { - if (!preg_match('/\$user->hasR/', $val[0]) && !preg_match('/\$permission/', $val[0]) && !preg_match('/\$usercan/', $val[0])) { + if (!preg_match('/\$user->hasR/', $val[0]) + && !preg_match('/\$permission/', $val[0]) + && !preg_match('/\$usercan/', $val[0]) + && !preg_match('/\$canedit/', $val[0]) + && !preg_match('/already done/', $val[0])) { $ok = false; print "Line: ".$val[0]."\n"; break;