FIX Pb with permission on holiday and expense report

htdocs/expensereport/card.php

@@ -120,6 +120,14 @@ if ($object->id > 0) {
 	}
 }
 
+$candelete = 0;
+if (!empty($user->rights->expensereport->supprimer)) {
+	$candelete = 1;
+}
+if ($object->statut == ExpenseReport::STATUS_DRAFT && $user->rights->expensereport->write && in_array($object->fk_user_author, $childids)) {
+	$candelete = 1;
+}
+
 // Security check
 if ($user->socid) {
 	$socid = $user->socid;
@@ -195,7 +203,7 @@ if (empty($reshook)) {
 		}
 	}
 
-	if ($action == 'confirm_delete' && GETPOST("confirm", 'alpha') == "yes" && $id > 0 && $user->rights->expensereport->supprimer) {
+	if ($action == 'confirm_delete' && GETPOST("confirm", 'alpha') == "yes" && $id > 0 && $candelete) {
 		$object = new ExpenseReport($db);
 		$result = $object->fetch($id);
 		$result = $object->delete($user);
@@ -2657,7 +2665,7 @@ if ($action != 'create' && $action != 'edit' && $action != 'editline') {
 	if ($user->rights->expensereport->creer && $user->id == $object->fk_user_author && $object->status < ExpenseReport::STATUS_APPROVED) {
 		// Delete
 		print '<div class="inline-block divButAction"><a class="butActionDelete" href="'.$_SERVER["PHP_SELF"].'?action=delete&token='.newToken().'&id='.$object->id.'">'.$langs->trans('Delete').'</a></div>';
-	} elseif ($user->rights->expensereport->supprimer && $object->status != ExpenseReport::STATUS_CLOSED) {
+	} elseif ($candelete && $object->status != ExpenseReport::STATUS_CLOSED) {
 		// Delete
 		print '<div class="inline-block divButAction"><a class="butActionDelete" href="'.$_SERVER["PHP_SELF"].'?action=delete&token='.newToken().'&id='.$object->id.'">'.$langs->trans('Delete').'</a></div>';
 	}

htdocs/holiday/card.php

@@ -90,12 +90,15 @@ if (($id > 0) || $ref) {
 $hookmanager->initHooks(array('holidaycard', 'globalcard'));
 
 $cancreate = 0;
-
-if (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance)) {
+$cancreateall = 0;
+if (!empty($user->rights->holiday->write) && in_array($fuserid, $childids)) {
 	$cancreate = 1;
 }
-if (!empty($user->rights->holiday->write) && in_array($fuserid, $childids)) {
+// TODO Remove the writeall_advance and replace it with writeall
+if ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall))
+	|| (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance))) {
 	$cancreate = 1;
+	$cancreateall = 1;
 }
 
 $candelete = 0;
@@ -166,23 +169,25 @@ if (empty($reshook)) {
 			$description = trim(GETPOST('description', 'restricthtml'));
 
 			// Check that leave is for a user inside the hierarchy or advanced permission for all is set
-			if (empty($conf->global->MAIN_USE_ADVANCED_PERMS)) {
-				if (empty($user->rights->holiday->write)) {
-					$error++;
-					setEventMessages($langs->trans("NotEnoughPermissions"), null, 'errors');
-				} elseif (!in_array($fuserid, $childids)) {
-					$error++;
-					setEventMessages($langs->trans("UserNotInHierachy"), null, 'errors');
-					$action = 'create';
-				}
-			} else {
-				if (empty($user->rights->holiday->write) && empty($user->rights->holiday->writeall_advance)) {
-					$error++;
-					setEventMessages($langs->trans("NotEnoughPermissions"), null, 'errors');
-				} elseif (empty($user->rights->holiday->writeall_advance) && !in_array($fuserid, $childids)) {
-					$error++;
-					setEventMessages($langs->trans("UserNotInHierachy"), null, 'errors');
-					$action = 'create';
+			if (!$cancreateall) {
+				if (empty($conf->global->MAIN_USE_ADVANCED_PERMS)) {
+					if (empty($user->rights->holiday->write)) {
+						$error++;
+						setEventMessages($langs->trans("NotEnoughPermissions"), null, 'errors');
+					} elseif (!in_array($fuserid, $childids)) {
+						$error++;
+						setEventMessages($langs->trans("UserNotInHierachy"), null, 'errors');
+						$action = 'create';
+					}
+				} else {
+					if (empty($user->rights->holiday->write) && empty($user->rights->holiday->writeall_advance)) {
+						$error++;
+						setEventMessages($langs->trans("NotEnoughPermissions"), null, 'errors');
+					} elseif (empty($user->rights->holiday->writeall_advance) && !in_array($fuserid, $childids)) {
+						$error++;
+						setEventMessages($langs->trans("UserNotInHierachy"), null, 'errors');
+						$action = 'create';
+					}
 				}
 			}
 
@@ -264,7 +269,7 @@ if (empty($reshook)) {
 		}
 	}
 
-	// If update and we are an approver, we can update with another approver
+	// If this is an update and we are an approver, we can update to change the approver
 	if ($action == 'update' && GETPOSTISSET('savevalidator') && !empty($user->rights->holiday->approve)) {
 		$object->fetch($id);
 
@@ -304,10 +309,18 @@ if (empty($reshook)) {
 		}
 
 		// If no right to modify a request
-		if (!$user->rights->holiday->write) {
-			setEventMessages($langs->trans("CantUpdate"), null, 'errors');
-			header('Location: '.$_SERVER["PHP_SELF"].'?action=create');
-			exit;
+		if (!$cancreateall) {
+			if ($cancreate) {
+				if (!in_array($fuserid, $childids)) {
+					setEventMessages($langs->trans("UserNotInHierachy"), null, 'errors');
+					header('Location: '.$_SERVER["PHP_SELF"].'?action=create');
+					exit;
+				}
+			} else {
+				setEventMessages($langs->trans("NotEnoughPermissions"), null, 'errors');
+				header('Location: '.$_SERVER["PHP_SELF"].'?action=create');
+				exit;
+			}
 		}
 
 		$object->fetch($id);
@@ -322,13 +335,13 @@ if (empty($reshook)) {
 				$description = trim(GETPOST('description', 'restricthtml'));
 
 				// If no start date
-				if (empty($_POST['date_debut_'])) {
+				if (!GETPOST('date_debut_')) {
 					header('Location: '.$_SERVER["PHP_SELF"].'?id='.$object->id.'&action=edit&error=nodatedebut');
 					exit;
 				}
 
 				// If no end date
-				if (empty($_POST['date_fin_'])) {
+				if (!GETPOST('date_fin_')) {
 					header('Location: '.$_SERVER["PHP_SELF"].'?id='.$object->id.'&action=edit&error=nodatefin');
 					exit;
 				}
@@ -379,7 +392,7 @@ if (empty($reshook)) {
 	}
 
 	// If delete of request
-	if ($action == 'confirm_delete' && GETPOST('confirm') == 'yes' && $user->rights->holiday->delete) {
+	if ($action == 'confirm_delete' && GETPOST('confirm') == 'yes' && $candelete) {
 		$error = 0;
 
 		$db->begin();
@@ -388,14 +401,11 @@ if (empty($reshook)) {
 
 		// If this is a rough draft, approved, canceled or refused
 		if ($object->statut == Holiday::STATUS_DRAFT || $object->statut == Holiday::STATUS_CANCELED || $object->statut == Holiday::STATUS_REFUSED) {
-			// Si l'utilisateur à le droit de lire cette demande, il peut la supprimer
-			if ($candelete) {
-				$result = $object->delete($user);
-			} else {
-				$error++;
-				setEventMessages($langs->trans('ErrorCantDeleteCP'), null, 'errors');
-				$action = '';
-			}
+			$result = $object->delete($user);
+		} else {
+			$error++;
+			setEventMessages($langs->trans('BadStatusOfObject'), null, 'errors');
+			$action = '';
 		}
 
 		if (!$error) {
@@ -748,7 +758,7 @@ if (empty($reshook)) {
 
 		// If status pending validation and validator = validator or user, or rights to do for others
 		if (($object->statut == Holiday::STATUS_VALIDATED || $object->statut == Holiday::STATUS_APPROVED) &&
-			(!empty($user->admin) || $user->id == $object->fk_validator || in_array($object->fk_user, $childids) || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance)))) {
+			(!empty($user->admin) || $user->id == $object->fk_validator || in_array($object->fk_user, $childids) || $cancreateall)) {
 			$db->begin();
 
 			$oldstatus = $object->statut;
@@ -981,12 +991,11 @@ if ((empty($id) && empty($ref)) || $action == 'create' || $action == 'add') {
 		print '<tr>';
 		print '<td class="titlefield fieldrequired">'.$langs->trans("User").'</td>';
 		print '<td>';
-
-		if (empty($conf->global->MAIN_USE_ADVANCED_PERMS) || empty($user->rights->holiday->writeall_advance)) {
+		if ($cancreate && !$cancreateall) {
 			print img_picto('', 'user').$form->select_dolusers(($fuserid ? $fuserid : $user->id), 'fuserid', 0, '', 0, 'hierarchyme', '', '0,'.$conf->entity, 0, 0, $morefilter, 0, '', 'minwidth200 maxwidth500');
 			//print '<input type="hidden" name="fuserid" value="'.($fuserid?$fuserid:$user->id).'">';
 		} else {
-			print img_picto('', 'user').$form->select_dolusers(GETPOST('fuserid', 'int') ? GETPOST('fuserid', 'int') : $user->id, 'fuserid', 0, '', 0, '', '', '0,'.$conf->entity, 0, 0, $morefilter, 0, '', 'minwidth200 maxwidth500');
+			print img_picto('', 'user').$form->select_dolusers($fuserid ? $fuserid : $user->id, 'fuserid', 0, '', 0, '', '', '0,'.$conf->entity, 0, 0, $morefilter, 0, '', 'minwidth200 maxwidth500');
 		}
 		print '</td>';
 		print '</tr>';
@@ -1467,9 +1476,7 @@ if ((empty($id) && empty($ref)) || $action == 'create' || $action == 'add') {
 						}
 					}
 					if ($object->statut == Holiday::STATUS_APPROVED) { // If validated or approved
-						if ($user->id == $object->fk_validator
-							|| in_array($object->fk_user, $childids)
-							|| (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance))) {
+						if ($user->id == $object->fk_validator || in_array($object->fk_user, $childids) || $cancreateall) {
 							if (($object->date_debut > dol_now()) || !empty($user->admin)) {
 								print '<a href="'.$_SERVER["PHP_SELF"].'?id='.$object->id.'&action=cancel&token='.newToken().'" class="butAction">'.$langs->trans("ActionCancelCP").'</a>';
 							} else {

htdocs/holiday/document.php

@@ -93,23 +93,6 @@ if (($id > 0) || $ref) {
 	}
 }
 
-/*$cancreate = 0;
-
-if (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance)) {
-	$cancreate = 1;
-}
-if (!empty($user->rights->holiday->write) && in_array($fuserid, $childids)) {
-	$cancreate = 1;
-}
-
-$candelete = 0;
-if (!empty($user->rights->holiday->delete)) {
-	$candelete = 1;
-}
-if ($object->statut == Holiday::STATUS_DRAFT && $user->rights->holiday->write && in_array($object->fk_user, $childids)) {
-	$candelete = 1;
-}
-*/
 
 $upload_dir = $conf->holiday->dir_output.'/'.get_exdir(0, 0, 0, 1, $object, '');
 $modulepart = 'holiday';

htdocs/holiday/list.php

@@ -484,9 +484,16 @@ if ($resql) {
 
 		print '<div class="tabsAction">';
 
-		$canedit = (($user->id == $user_id && $user->rights->holiday->write) || ($user->id != $user_id && (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance))));
+		$cancreate = 0;
+		if ((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall))
+			|| (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->holiday->writeall_advance))) {
+				$cancreate = 1;
+		}
+		if (!empty($user->rights->holiday->write) && in_array($user_id, $childids)) {
+			$cancreate = 1;
+		}
 
-		if ($canedit) {
+		if ($cancreate) {
 			print '<a href="'.DOL_URL_ROOT.'/holiday/card.php?action=create&fuserid='.$user_id.'" class="butAction">'.$langs->trans("AddCP").'</a>';
 		}
 

htdocs/langs/en_US/admin.lang

@@ -882,12 +882,11 @@ Permission701=Read donations
 Permission702=Create/modify donations
 Permission703=Delete donations
 Permission771=Read expense reports (yours and your subordinates)
-Permission772=Create/modify expense reports
+Permission772=Create/modify expense reports (for you and your subordinates) 
 Permission773=Delete expense reports
-Permission774=Read all expense reports (even for user not subordinates)
 Permission775=Approve expense reports
 Permission776=Pay expense reports
-Permission777=Read expense reports of everybody
+Permission777=Read all expense reports (even those of user not subordinates)
 Permission778=Create/modify expense reports of everybody
 Permission779=Export expense reports
 Permission1001=Read stocks
@@ -962,9 +961,9 @@ Permission10005=Delete website content
 Permission20001=Read leave requests (your leave and those of your subordinates)
 Permission20002=Create/modify your leave requests (your leave and those of your subordinates)
 Permission20003=Delete leave requests
-Permission20004=Read all leave requests (even of user not subordinates)
-Permission20005=Create/modify leave requests for everybody (even of user not subordinates)
-Permission20006=Admin leave requests (setup and update balance)
+Permission20004=Read all leave requests (even those of user not subordinates)
+Permission20005=Create/modify leave requests for everybody (even those of user not subordinates)
+Permission20006=Administer leave requests (setup and update balance)
 Permission20007=Approve leave requests
 Permission23001=Read Scheduled job
 Permission23002=Create/update Scheduled job

htdocs/user/class/user.class.php

@@ -970,7 +970,9 @@ class User extends CommonObject
 		if (!empty($conf->global->MULTICOMPANY_BACKWARD_COMPATIBILITY)) {
 			$sql .= " AND r.entity IN (0,".(!empty($conf->multicompany->enabled) && !empty($conf->global->MULTICOMPANY_TRANSVERSE_MODE) ? "1," : "").$conf->entity.")";
 		} else {
-			$sql .= " AND ur.entity = ".((int) $conf->entity);
+			// On table r=rights_def, the unique key is (id, entity) because id is hard coded into module descriptor and insert during module activation.
+			// So we must include the filter on r.entity on both r and ur.
+			$sql .= " AND r.entity = ".((int) $conf->entity)." AND ur.entity = ".((int) $conf->entity);
 		}
 		$sql .= " AND ur.fk_user= ".((int) $this->id);
 		$sql .= " AND r.perms IS NOT NULL";