From 52493b60fb343e290b560ead58b53ef8aa65494a Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Thu, 13 Jan 2022 15:52:41 +0100 Subject: [PATCH] Fix #yogosha8316 --- htdocs/core/lib/files.lib.php | 34 +++++++-- htdocs/core/lib/security.lib.php | 117 ++++++++++++++++++++++--------- htdocs/document.php | 2 +- 3 files changed, 115 insertions(+), 38 deletions(-) diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php index b08e85851d2..c70ea041e0a 100644 --- a/htdocs/core/lib/files.lib.php +++ b/htdocs/core/lib/files.lib.php @@ -2270,13 +2270,13 @@ function dol_most_recent_file($dir, $regexfilter = '', $excludefilter = array('( /** * Security check when accessing to a document (used by document.php, viewimage.php and webservices to get documents). - * TODO Replace code that set $accesallowed by a call to restrictedArea() + * TODO Replace code that set $accessallowed by a call to restrictedArea() * * @param string $modulepart Module of document ('module', 'module_user_temp', 'module_user' or 'module_temp'). Exemple: 'medias', 'invoice', 'logs', 'tax-vat', ... * @param string $original_file Relative path with filename, relative to modulepart. * @param string $entity Restrict onto entity (0=no restriction) * @param User $fuser User object (forced) - * @param string $refname Ref of object to check permission for external users (autodetect if not provided) + * @param string $refname Ref of object to check permission for external users (autodetect if not provided) or for hierarchy * @param string $mode Check permission for 'read' or 'write' * @return mixed Array with access information : 'accessallowed' & 'sqlprotectagainstexternals' & 'original_file' (as a full path name) * @see restrictedArea() @@ -2423,6 +2423,30 @@ function dol_check_secure_access_document($modulepart, $original_file, $entity, $accessallowed = 1; } $original_file = $conf->fournisseur->facture->dir_output.'/'.$original_file; + } elseif (($modulepart == 'holiday') && !empty($conf->holiday->dir_output)) { + if ($fuser->rights->holiday->{$read} || preg_match('/^specimen/i', $original_file)) { + $accessallowed = 1; + // If we known $id of holiday, call checkUserAccessToObject to check permission on properties and hierarchy of leave request + if ($refname && !preg_match('/^specimen/i', $original_file)) { + include_once DOL_DOCUMENT_ROOT.'/holiday/class/holiday.class.php'; + $tmpholiday = new Holiday($db); + $tmpholiday->fetch('', $refname); + $accessallowed = checkUserAccessToObject($user, array('holiday'), $tmpholiday, 'holiday', '', '', 'rowid', ''); + } + } + $original_file = $conf->holiday->dir_output.'/'.$original_file; + } elseif (($modulepart == 'expensereport') && !empty($conf->expensereport->dir_output)) { + if ($fuser->rights->expensereport->{$lire} || preg_match('/^specimen/i', $original_file)) { + $accessallowed = 1; + // If we known $id of expensereport, call checkUserAccessToObject to check permission on properties and hierarchy of expense report + if ($refname && !preg_match('/^specimen/i', $original_file)) { + include_once DOL_DOCUMENT_ROOT.'/expensereport/class/expensereport.class.php'; + $tmpexpensereport = new ExpenseReport($db); + $tmpexpensereport->fetch('', $refname); + $accessallowed = checkUserAccessToObject($user, array('expensereport'), $tmpexpensereport, 'expensereport', '', '', 'rowid', ''); + } + } + $original_file = $conf->expensereport->dir_output.'/'.$original_file; } elseif (($modulepart == 'apercuexpensereport') && !empty($conf->expensereport->dir_output)) { // Wrapping pour les apercu supplier invoice if ($fuser->rights->expensereport->{$lire}) { @@ -2971,9 +2995,9 @@ function dol_check_secure_access_document($modulepart, $original_file, $entity, } $ret = array( - 'accessallowed' => $accessallowed, - 'sqlprotectagainstexternals'=>$sqlprotectagainstexternals, - 'original_file'=>$original_file + 'accessallowed' => ($accessallowed ? 1 : 0), + 'sqlprotectagainstexternals' => $sqlprotectagainstexternals, + 'original_file' => $original_file ); return $ret; diff --git a/htdocs/core/lib/security.lib.php b/htdocs/core/lib/security.lib.php index 0b47ec5aeb0..4a20fbf96f2 100644 --- a/htdocs/core/lib/security.lib.php +++ b/htdocs/core/lib/security.lib.php @@ -626,24 +626,30 @@ function restrictedArea($user, $features, $objectid = 0, $tableandshare = '', $f } /** - * Check access by user to object is ok. - * This function is also called by restrictedArea that check before if module is enabled and if permission of user for $action is ok. + * Check that access by a given user to an object is ok. + * This function is also called by restrictedArea() that check before if module is enabled and if permission of user for $action is ok. * - * @param User $user User to check - * @param array $featuresarray Features/modules to check. Example: ('user','service','member','project','task',...) - * @param int|string $objectid Object ID if we want to check a particular record (optional) is linked to a owned thirdparty (optional). - * @param string $tableandshare 'TableName&SharedElement' with Tablename is table where object is stored. SharedElement is an optional key to define where to check entity for multicompany modume. Param not used if objectid is null (optional). - * @param string $feature2 Feature to check, second level of permission (optional). Can be or check with 'level1|level2'. - * @param string $dbt_keyfield Field name for socid foreign key if not fk_soc. Not used if objectid is null (optional) - * @param string $dbt_select Field name for select if not rowid. Not used if objectid is null (optional) - * @param string $parenttableforentity Parent table for entity. Example 'fk_website@website' - * @return bool True if user has access, False otherwise + * @param User $user User to check + * @param array $featuresarray Features/modules to check. Example: ('user','service','member','project','task',...) + * @param int|string|Object $object Full object or object ID or list of object id. For example if we want to check a particular record (optional) is linked to a owned thirdparty (optional). + * @param string $tableandshare 'TableName&SharedElement' with Tablename is table where object is stored. SharedElement is an optional key to define where to check entity for multicompany modume. Param not used if objectid is null (optional). + * @param string $feature2 Feature to check, second level of permission (optional). Can be or check with 'level1|level2'. + * @param string $dbt_keyfield Field name for socid foreign key if not fk_soc. Not used if objectid is null (optional) + * @param string $dbt_select Field name for select if not rowid. Not used if objectid is null (optional) + * @param string $parenttableforentity Parent table for entity. Example 'fk_website@website' + * @return bool True if user has access, False otherwise * @see restrictedArea() */ -function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $tableandshare = '', $feature2 = '', $dbt_keyfield = '', $dbt_select = 'rowid', $parenttableforentity = '') +function checkUserAccessToObject($user, array $featuresarray, $object = 0, $tableandshare = '', $feature2 = '', $dbt_keyfield = '', $dbt_select = 'rowid', $parenttableforentity = '') { global $db, $conf; + if (is_object($object)) { + $objectid = $object->id; + } else { + $objectid = $object; // $objectid can be X or 'X,Y,Z' + } + //dol_syslog("functions.lib:restrictedArea $feature, $objectid, $dbtablename, $feature2, $dbt_socfield, $dbt_select, $isdraft"); //print "user_id=".$user->id.", features=".join(',', $featuresarray).", feature2=".$feature2.", objectid=".$objectid; //print ", tableandshare=".$tableandshare.", dbt_socfield=".$dbt_keyfield.", dbt_select=".$dbt_select."
"; @@ -669,11 +675,15 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta $feature = 'projet_task'; } + $checkonentitydone = 0; + + // Array to define rules of checks to do $check = array('adherent', 'banque', 'bom', 'don', 'mrp', 'user', 'usergroup', 'payment', 'payment_supplier', 'product', 'produit', 'service', 'produit|service', 'categorie', 'resource', 'expensereport', 'holiday', 'salaries', 'website'); // Test on entity only (Objects with no link to company) $checksoc = array('societe'); // Test for societe object $checkother = array('contact', 'agenda'); // Test on entity + link to third party on field $dbt_keyfield. Allowed if link is empty (Ex: contacts...). $checkproject = array('projet', 'project'); // Test for project object $checktask = array('projet_task'); // Test for task object + $checkhierarchy = array('expensereport', 'holiday'); $nocheck = array('barcode', 'stock'); // No test //$checkdefault = 'all other not already defined'; // Test on entity + link to third party on field $dbt_keyfield. Not allowed if link is empty (Ex: invoice, orders...). @@ -714,10 +724,12 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta $sql .= " AND dbt.entity IN (".getEntity($sharedelement, 1).")"; } } - } elseif (in_array($feature, $checksoc)) { // We check feature = checksoc + $checkonentitydone = 1; + } + if (in_array($feature, $checksoc)) { // We check feature = checksoc // If external user: Check permission for external users if ($user->socid > 0) { - if ($user->socid <> $objectid) { + if ($user->socid != $objectid) { return false; } } elseif (!empty($conf->societe->enabled) && ($user->rights->societe->lire && empty($user->rights->societe->client->voir))) { @@ -736,7 +748,10 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta $sql .= " WHERE s.rowid IN (".$db->sanitize($objectid, 1).")"; $sql .= " AND s.entity IN (".getEntity($sharedelement, 1).")"; } - } elseif (in_array($feature, $checkother)) { // Test on entity + link to thirdparty. Allowed if link is empty (Ex: contacts...). + + $checkonentitydone = 1; + } + if (in_array($feature, $checkother)) { // Test on entity + link to thirdparty. Allowed if link is empty (Ex: contacts...). // If external user: Check permission for external users if ($user->socid > 0) { $sql = "SELECT COUNT(dbt.".$dbt_select.") as nb"; @@ -758,25 +773,19 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta $sql .= " WHERE dbt.".$dbt_select." IN (".$db->sanitize($objectid, 1).")"; $sql .= " AND dbt.entity IN (".getEntity($sharedelement, 1).")"; } - if ($feature == 'agenda') { - // Also check owner or attendee for users without allactions->read - if ($objectid > 0 && empty($user->rights->agenda->allactions->read)) { - require_once DOL_DOCUMENT_ROOT.'/comm/action/class/actioncomm.class.php'; - $action = new ActionComm($db); - $action->fetch($objectid); - if ($action->authorid != $user->id && $action->userownerid != $user->id && !(array_key_exists($user->id, $action->userassigned))) { - return false; - } - } - } - } elseif (in_array($feature, $checkproject)) { + + $checkonentitydone = 1; + } + if (in_array($feature, $checkproject)) { if (!empty($conf->projet->enabled) && empty($user->rights->projet->all->lire)) { + $projectid = $objectid; + include_once DOL_DOCUMENT_ROOT.'/projet/class/project.class.php'; $projectstatic = new Project($db); $tmps = $projectstatic->getProjectsAuthorizedForUser($user, 0, 1, 0); $tmparray = explode(',', $tmps); - if (!in_array($objectid, $tmparray)) { + if (!in_array($projectid, $tmparray)) { return false; } } else { @@ -785,16 +794,21 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta $sql .= " WHERE dbt.".$dbt_select." IN (".$db->sanitize($objectid, 1).")"; $sql .= " AND dbt.entity IN (".getEntity($sharedelement, 1).")"; } - } elseif (in_array($feature, $checktask)) { + + $checkonentitydone = 1; + } + if (in_array($feature, $checktask)) { if (!empty($conf->projet->enabled) && empty($user->rights->projet->all->lire)) { $task = new Task($db); $task->fetch($objectid); + $projectid = $task->fk_project; include_once DOL_DOCUMENT_ROOT.'/projet/class/project.class.php'; $projectstatic = new Project($db); $tmps = $projectstatic->getProjectsAuthorizedForUser($user, 0, 1, 0); + $tmparray = explode(',', $tmps); - if (!in_array($task->fk_project, $tmparray)) { + if (!in_array($projectid, $tmparray)) { return false; } } else { @@ -803,7 +817,10 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta $sql .= " WHERE dbt.".$dbt_select." IN (".$db->sanitize($objectid, 1).")"; $sql .= " AND dbt.entity IN (".getEntity($sharedelement, 1).")"; } - } elseif (!in_array($feature, $nocheck)) { // By default (case of $checkdefault), we check on object entity + link to third party on field $dbt_keyfield + + $checkonentitydone = 1; + } + if (!$checkonentitydone && !in_array($feature, $nocheck)) { // By default (case of $checkdefault), we check on object entity + link to third party on field $dbt_keyfield // If external user: Check permission for external users if ($user->socid > 0) { if (empty($dbt_keyfield)) { @@ -845,11 +862,47 @@ function checkUserAccessToObject($user, array $featuresarray, $objectid = 0, $ta } //print $sql; + // For events, check on users assigned to event + if ($feature === 'agenda') { + // Also check owner or attendee for users without allactions->read + if ($objectid > 0 && empty($user->rights->agenda->allactions->read)) { + require_once DOL_DOCUMENT_ROOT.'/comm/action/class/actioncomm.class.php'; + $action = new ActionComm($db); + $action->fetch($objectid); + if ($action->authorid != $user->id && $action->userownerid != $user->id && !(array_key_exists($user->id, $action->userassigned))) { + return false; + } + } + } + + // For some object, we also have to check it is in the user hierarchy + // Param $object must be the full object and not a simple id to have this test possible. + if (in_array($feature, $checkhierarchy) && is_object($object)) { + $childids = $user->getAllChildIds(1); + $useridtocheck = 0; + if ($feature == 'holiday') { + $useridtocheck = $object->fk_user; + if (!in_array($useridtocheck, $childids)) { + return false; + } + $useridtocheck = $object->fk_validator; + if (!in_array($useridtocheck, $childids)) { + return false; + } + } + if ($feature == 'expensereport') { + $useridtocheck = $object->fk_user_author; + if (!in_array($useridtocheck, $childids)) { + return false; + } + } + } + if ($sql) { $resql = $db->query($sql); if ($resql) { $obj = $db->fetch_object($resql); - if (!$obj || $obj->nb < count(explode(',', $objectid))) { + if (!$obj || $obj->nb < count(explode(',', $objectid))) { // error if we found 0 or less record than nb of id provided return false; } } else { diff --git a/htdocs/document.php b/htdocs/document.php index 2a708ff3334..7498fbb3cf6 100644 --- a/htdocs/document.php +++ b/htdocs/document.php @@ -214,7 +214,7 @@ $check_access = dol_check_secure_access_document($modulepart, $original_file, $e $accessallowed = $check_access['accessallowed']; $sqlprotectagainstexternals = $check_access['sqlprotectagainstexternals']; $fullpath_original_file = $check_access['original_file']; // $fullpath_original_file is now a full path name -//var_dump($fullpath_original_file);exit; +//var_dump($fullpath_original_file.' '.$original_file.' '.$refname.' '.$accessallowed);exit; if (!empty($hashp)) { $accessallowed = 1; // When using hashp, link is public so we force $accessallowed