From 59f15bc4b6c01dbeb415b39b22c34ad3b1265292 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 15:48:20 +0100 Subject: [PATCH 01/22] Dev: Move Data for view to a Services --- .../SurveysGroupsPermissionController.php | 126 ++---------------- .../models/services/PermissionManager.php | 100 ++++++++++++++ 2 files changed, 113 insertions(+), 113 deletions(-) create mode 100644 application/models/services/PermissionManager.php diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 1295d58646b..6424d2f99fc 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -1,8 +1,7 @@ findByPk($to); $oUser = null; } - $aSurveysGroupsPermissions = Permission::model()->getEntityBasePermissions('SurveysGroups'); - /* Set the current : @todo move to Permission::model ? Or an helper ?*/ - foreach (array_keys($aSurveysGroupsPermissions) as $sPermission) { - $aSurveysGroupsPermissions[$sPermission]['current'] = array( - 'create' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'create'), - 'indeterminate' => false - ), - 'read' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'read'), - 'indeterminate' => false - ), - 'update' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'update'), - 'indeterminate' => false - ), - 'delete' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'delete'), - 'indeterminate' => false - ), - 'import' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'import'), - 'indeterminate' => false - ), - 'export' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'export'), - 'indeterminate' => false - ), - ); - $aSurveysGroupsPermissions[$sPermission]['entity'] = 'SurveysGroups'; - if ($type == 'user') { - $oCurrentPermissions = Permission::model()->find( - "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", - array( - ":entity" => 'SurveysGroups', - ":entity_id" => $id, - ":uid" => $userId, - ":permission" => $sPermission - ) - ); - foreach (array_keys($aSurveysGroupsPermissions[$sPermission]['current']) as $sCrud) { - if ($aSurveysGroupsPermissions[$sPermission][$sCrud]) { - $havePermissionSet = !empty($oCurrentPermissions) && $oCurrentPermissions->getAttribute("{$sCrud}_p"); - $aSurveysGroupsPermissions[$sPermission]['current'][$sCrud]['checked'] = $havePermissionSet; - $aSurveysGroupsPermissions[$sPermission]['current'][$sCrud]['indeterminate'] = !$havePermissionSet && Permission::model()->hasSurveyGroupPermission($id, $sPermission, $sCrud, $userId); // Set by global or owner - } - } - } - } - $aSurveysInGroupPermissions = Permission::model()->getEntityBasePermissions('SurveysInGroup'); - /* Set the current : @todo move to Permission::model ? Or an helper ?*/ - foreach (array_keys($aSurveysInGroupPermissions) as $sPermission) { - $aSurveysInGroupPermissions[$sPermission]['current'] = array( - 'create' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveysInGroupPermission($id, $sPermission, 'create'), - 'indeterminate' => false - ), - 'read' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveysInGroupPermission($id, $sPermission, 'read'), - 'indeterminate' => false - ), - 'update' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveysInGroupPermission($id, $sPermission, 'update'), - 'indeterminate' => false - ), - 'delete' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveysInGroupPermission($id, $sPermission, 'delete'), - 'indeterminate' => false - ), - 'import' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveysInGroupPermission($id, $sPermission, 'import'), - 'indeterminate' => false - ), - 'export' => array( - 'checked' => false, - 'disabled' => !Permission::model()->hasSurveyGroupPermission($id, $sPermission, 'export'), - 'indeterminate' => false - ) - ); - $aSurveysInGroupPermissions[$sPermission]['entity'] = 'SurveysInGroup'; - if ($type == 'user') { - $oCurrentPermissions = Permission::model()->find( - "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", - array( - ":entity" => 'SurveysInGroup', - ":entity_id" => $id, - ":uid" => $userId, - ":permission" => $sPermission - ) - ); - foreach (array_keys($aSurveysInGroupPermissions[$sPermission]['current']) as $sCrud) { - if ($aSurveysInGroupPermissions[$sPermission][$sCrud]) { - $havePermissionSet = !empty($oCurrentPermissions) && $oCurrentPermissions->getAttribute("{$sCrud}_p"); - $aSurveysInGroupPermissions[$sPermission]['current'][$sCrud]['checked'] = $havePermissionSet; - $aSurveysInGroupPermissions[$sPermission]['current'][$sCrud]['indeterminate'] = !$havePermissionSet && Permission::model()->hasSurveyGroupPermission($id, $sPermission, $sCrud, $userId); // Set by global or owner - } - } - } - } + $user = App()->user; + $request = App()->request; + $PermissionManagerService = new PermissionManager( + $request, + $user, + ); + $aSurveysGroupsPermissions = $PermissionManagerService->getPermissionData('SurveysGroups', $id, $userId); + $aSurveysInGroupPermissions = $PermissionManagerService->getPermissionData('SurveysInGroup', $id, $userId); $aPermissions = array_merge( $aSurveysGroupsPermissions, $aSurveysInGroupPermissions ); + $aData = array( 'model' => $model, 'subview' => 'setPermissionForm', diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php new file mode 100644 index 00000000000..e3f3690ae97 --- /dev/null +++ b/application/models/services/PermissionManager.php @@ -0,0 +1,100 @@ +request = $request; + $this->user = $user; + } + + /** + * get the permission data + * @param string $modelName the model name + * @param interger $modelId the model name + * @param integer $userId for this user id + * @return array[] + */ + public function getPermissionData($modelName, $modelId, $userId = null) + { + $aObjectPermissions = Permission::model()->getEntityBasePermissions($modelName); + if(empty($aObjectPermissions)) { + return $aObjectPermissions; + } + /* string[] Crud type array */ + $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); + foreach (array_keys($aObjectPermissions) as $sPermission) { + $aObjectPermissions[$sPermission]['current'] = array(); + foreach($aCruds as $crud) { + $aObjectPermissions[$sPermission]['current'][$crud] = array( + 'checked' => false, + /* The checkbox are disable if currentuser don't have permission */ + 'disabled' => !Permission::model()->hasPermission($modelId, $modelName, $sPermission, 'create'), + 'indeterminate' => false + ); + } + /* If user id is set : update the data with permission of this user */ + if($userId) { + $oCurrentPermissions = Permission::model()->find( + "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", + array( + ":entity" => $modelName, + ":entity_id" => $modelId, + ":uid" => $userId, + ":permission" => $sPermission + ) + ); + foreach ($aCruds as $crud) { + if ($aObjectPermissions[$sPermission][$crud]) { + $havePermissionSet = !empty($oCurrentPermissions) && $oCurrentPermissions->getAttribute("{$crud}_p"); + /* The user have the permission set */ + $aObjectPermissions[$sPermission]['current'][$crud]['checked'] = $havePermissionSet; + /* The user didn't have the permission set, but have permission by other way (inherited, plugin …) */ + if(!$havePermissionSet) { + $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = Permission::model()->hasPermission($modelId, $modelName, $sPermission, $crud, $userId); + } + } + } + } + $aObjectPermissions[$sPermission]['entity'] = $modelName; + } + return $aObjectPermissions; + } + + /** + * @todo : Save Permission by POST value according to current user permssion + * @see Permission::setPermissions + */ + public function setPermissions() + { + // @todo + } +} From 3a8578feb53fcb4a4469563f45475c52b9cfdf3d Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 15:51:35 +0100 Subject: [PATCH 02/22] Dev: use $this->user --- application/models/services/PermissionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index e3f3690ae97..232ed1af651 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -57,7 +57,7 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ - 'disabled' => !Permission::model()->hasPermission($modelId, $modelName, $sPermission, 'create'), + 'disabled' => !Permission::model()->hasPermission($modelId, $modelName, $sPermission, $crud, $this->user->id), 'indeterminate' => false ); } From 8b374955f374b62fe57172b79f2b228837c660cf Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 16:30:45 +0100 Subject: [PATCH 03/22] Dev: Permission::getEntityBasePermissions didn't need model --- application/models/services/PermissionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 232ed1af651..4927c31a617 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -45,7 +45,7 @@ public function __construct( */ public function getPermissionData($modelName, $modelId, $userId = null) { - $aObjectPermissions = Permission::model()->getEntityBasePermissions($modelName); + $aObjectPermissions = Permission::getEntityBasePermissions($modelName); if(empty($aObjectPermissions)) { return $aObjectPermissions; } From c56bb3772602e53b43a931e663f8949da8ded2aa Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 16:43:17 +0100 Subject: [PATCH 04/22] Dev: Add Permission in constructor Dev: for test : need find and hasPermission --- .../SurveysGroupsPermissionController.php | 2 ++ application/models/services/PermissionManager.php | 15 ++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 6424d2f99fc..ddd57c56b45 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -432,7 +432,9 @@ private function viewUserOrUserGroup($id, $to, $type = 'user') $PermissionManagerService = new PermissionManager( $request, $user, + new Permission() ); + $aSurveysGroupsPermissions = $PermissionManagerService->getPermissionData('SurveysGroups', $id, $userId); $aSurveysInGroupPermissions = $PermissionManagerService->getPermissionData('SurveysInGroup', $id, $userId); $aPermissions = array_merge( diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 4927c31a617..b4aab439b6f 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -24,16 +24,21 @@ class PermissionManager /** @var LSWebUser */ private $user; + /** @var Permission model */ + private $permission; + /** * @param LSHttpRequest $request * @param LSWebUser $user */ public function __construct( LSHttpRequest $request, - LSWebUser $user + LSWebUser $user, + Permission $permission ) { $this->request = $request; $this->user = $user; + $this->permission = $permission; } /** @@ -45,7 +50,7 @@ public function __construct( */ public function getPermissionData($modelName, $modelId, $userId = null) { - $aObjectPermissions = Permission::getEntityBasePermissions($modelName); + $aObjectPermissions = Permission::getEntityBasePermissions($modelName); // Usage of static, db not needed if(empty($aObjectPermissions)) { return $aObjectPermissions; } @@ -57,13 +62,13 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ - 'disabled' => !Permission::model()->hasPermission($modelId, $modelName, $sPermission, $crud, $this->user->id), + 'disabled' => !$this->permission->hasPermission($modelId, $modelName, $sPermission, $crud, $this->user->id), 'indeterminate' => false ); } /* If user id is set : update the data with permission of this user */ if($userId) { - $oCurrentPermissions = Permission::model()->find( + $oCurrentPermissions = $this->permission->find( "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", array( ":entity" => $modelName, @@ -79,7 +84,7 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud]['checked'] = $havePermissionSet; /* The user didn't have the permission set, but have permission by other way (inherited, plugin …) */ if(!$havePermissionSet) { - $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = Permission::model()->hasPermission($modelId, $modelName, $sPermission, $crud, $userId); + $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->permission->hasPermission($modelId, $modelName, $sPermission, $crud, $userId); } } } From 791e295f743a4ca253b2777082d03338f1e981f5 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 17:53:43 +0100 Subject: [PATCH 05/22] Dev: Save by Service , checked only with superadmin currently --- .../SurveysGroupsPermissionController.php | 21 +++-- application/models/Permission.php | 15 ++++ .../models/services/PermissionManager.php | 80 ++++++++++++++++++- 3 files changed, 106 insertions(+), 10 deletions(-) diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index ddd57c56b45..1e661d5e0bd 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -339,18 +339,25 @@ public function actionSave($id) $uids = array($uid); } $set = App()->getRequest()->getPost('set'); + $user = App()->user; + $request = App()->request; + $PermissionManagerService = new PermissionManager( + $request, + $user, + new Permission() + ); + $success = true; foreach ($set as $entity => $aPermissionSet) { foreach ($uids as $uid) { /* Permission::model()->setPermissions return true or break */ - Permission::model()->setPermissions( - $uid, - $id, - $entity, - $aPermissionSet - ); + $success = $success && !$PermissionManagerService->setPermissions($uid, $entity, $id); } } - App()->setFlashMessage("Surveys groups permissions were successfully updated"); + if($success) { + App()->setFlashMessage("Surveys groups permissions were successfully updated"); + } else { + App()->setFlashMessage("An error happen when update surveys groups permissions", 'danger'); + } if ($type == 'group') { App()->request->redirect(App()->getController()->createUrl('surveysGroupsPermission/index', array('id' => $id))); } diff --git a/application/models/Permission.php b/application/models/Permission.php index 4b6fabd9bf2..7e0f4c62669 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -25,6 +25,7 @@ * @property integer $create_p * @property integer $read_p * @property integer $update_p + * @property integer $delete_p * @property integer $import_p * @property integer $export_p * @@ -57,6 +58,20 @@ public static function model($class = __CLASS__) return $model; } + /** + * @return array validation rules for model attributes. + */ + public function rules() + { + return array( + array('entity, entity_id, uid, permission', 'required'), + array('entity', 'length', 'max'=>50), + array('permission', 'length', 'max'=>100), + array('create_p, read_p, update_p, delete_p, import_p, export_p', 'default', 'value'=>0), + array('create_p, read_p, update_p, delete_p, import_p, export_p', 'numerical', 'integerOnly'=>true), + ); + } + /** * Returns the base permissions for survey * @see self::getEntityBasePermissions diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index b4aab439b6f..988b2586a11 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -44,7 +44,7 @@ public function __construct( /** * get the permission data * @param string $modelName the model name - * @param interger $modelId the model name + * @param integer $modelId the model id * @param integer $userId for this user id * @return array[] */ @@ -97,9 +97,83 @@ public function getPermissionData($modelName, $modelId, $userId = null) /** * @todo : Save Permission by POST value according to current user permssion * @see Permission::setPermissions + * @param mixed $iUserID + * @param string $modelName + * @param mixed $modelId + * @return boolean */ - public function setPermissions() + public function setPermissions($userId, $modelName, $modelId) { - // @todo + $success = true; + $permissionsToSet = $this->request->getPost('set'); + if (empty($permissionsToSet[$modelName])) { + /* Nothing to do */ + return $success; + } + /* string[] Crud type array */ + $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); + $entityPermissionsToSet = $permissionsToSet[$modelName]; + $aBasePermissions = Permission::getEntityBasePermissions($modelName); + + /* string[] The array to set (or not) */ + $aSetPermissions = array(); + foreach ($aBasePermissions as $sPermission => $aPermission) { + $aSetPermissions[$sPermission] = array(); + foreach ($aCruds as $crud) { + /* Check if current user have the permission to set */ + if($this->permission->hasPermission($modelId, $modelName, $sPermission, $crud, $this->user->id)) { + $aSetPermissions[$sPermission][$crud] = !empty($aPermission[$crud]) && !empty($entityPermissionsToSet[$sPermission][$crud]); + } + } + } + /* remove uneeded Permission (user don't have any rights) */ + $aSetPermissions = array_filter($aSetPermissions); + // Event + $oEvent = new \LimeSurvey\PluginManager\PluginEvent('beforePermissionSetSave'); + $oEvent->set('aNewPermissions', $aSetPermissions); + if($modelName == 'Survey') { + $oEvent->set('iSurveyID', $modelId); + } + $oEvent->set('entity', $modelName); /* New in 4.4.X */ + $oEvent->set('entityId', $modelId); /* New in 4.4.X */ + $oEvent->set('iUserID', $userId); + App()->getPluginManager()->dispatchEvent($oEvent); + + foreach($aSetPermissions as $sPermission => $aSetPermission) { + $oCurrentPermission = $this->permission->find( + "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", + array( + ":entity" => $modelName, + ":entity_id" => $modelId, + ":uid" => $userId, + ":permission" => $sPermission + ) + ); + if(empty($oCurrentPermission)) { + $oCurrentPermission = $this->getNewPermission(); // ????? + $oCurrentPermission->entity = $modelName; + $oCurrentPermission->entity_id = $modelId; + $oCurrentPermission->uid = $userId; + $oCurrentPermission->permission = $sPermission; + } + /* Set only the permission set in $aSetPermission : user have the rights */ + foreach($aSetPermission as $crud => $permission) { + $oCurrentPermission->setAttribute("{$crud}_p",intval($permission)); + } + if($oCurrentPermission->save()) { + $success = false; + } + } + return $success; + } + + /** + * Create a new Permission Model + * To be mocked for test + * @return \Permission + */ + public function getNewPermission() + { + return new Permission(); } } From e83d62af50fce4bff6d11bc26ab23d2e5b24f8f5 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 18:15:47 +0100 Subject: [PATCH 06/22] Dev: SurveysGroups model, not SurveysGroup or SurveyGroup --- .../SurveysGroupsPermissionController.php | 16 +++++++-------- .../admin/SurveysGroupsController.php | 20 +++++++++---------- application/models/Permission.php | 6 +++--- application/models/SurveysGroups.php | 10 +++++----- .../models/services/PermissionManager.php | 6 ++++-- .../subviews/currentUsersList.php | 4 ++-- .../subviews/viewCurrents.php | 2 +- .../admin/surveysgroups/surveySettings.php | 2 +- .../views/admin/surveysgroups/update.php | 4 ++-- tests/unit/models/PermissionTest.php | 18 ++++++++--------- 10 files changed, 45 insertions(+), 43 deletions(-) diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 1e661d5e0bd..58934513328 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -93,7 +93,7 @@ public function actionIndex($id) foreach ($aSurveysGroupsPermissions as $sPermission => $aPermissions) { $aCurrentsUserRights[$oUser->uid][$sPermission] = array(); foreach (array_intersect_key($aPermissions, array_flip($aCruds)) as $sCrud => $available) { - if ($available && Permission::model()->hasSurveyGroupPermission($id, $sPermission, $sCrud, $oUser->uid)) { + if ($available && Permission::model()->hasSurveysGroupsPermission($id, $sPermission, $sCrud, $oUser->uid)) { $aCurrentsUserRights[$oUser->uid][$sPermission][] = $sCrud; } } @@ -101,7 +101,7 @@ public function actionIndex($id) foreach ($aSurveysInGroupPermissions as $sPermission => $aPermissions) { $aCurrentsUserRights[$oUser->uid][$sPermission] = array(); foreach (array_intersect_key($aPermissions, array_flip($aCruds)) as $sCrud => $available) { - if ($available && Permission::model()->hasSurveyGroupPermission($id, $sPermission, $sCrud, $oUser->uid)) { + if ($available && Permission::model()->hasSurveysGroupsPermission($id, $sPermission, $sCrud, $oUser->uid)) { $aCurrentsUserRights[$oUser->uid][$sPermission][] = $sCrud; } } @@ -111,7 +111,7 @@ public function actionIndex($id) $oAddUserList = array(); $oAddGroupList = array(); - if (Permission::model()->hasSurveyGroupPermission($id, 'permission', 'create')) { + if (Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { /* Search user withouth rights on SurveyGroup */ /* @todo : move this to : SurveysGroups ? Permission ? User ?*/ $oCriteria = new CDbCriteria(); @@ -158,7 +158,7 @@ public function actionIndex($id) public function actionAddUser($id) { $model = $this->loadModel($id); - if (!Permission::model()->hasSurveyGroupPermission($id, 'permission', 'create')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $uid = App()->getRequest()->getPost('uid'); @@ -210,7 +210,7 @@ public function actionAddUser($id) public function actionAddUserGroup($id) { $model = $this->loadModel($id); - if (!Permission::model()->hasSurveyGroupPermission($id, 'permission', 'create')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $ugid = App()->getRequest()->getPost('ugid'); @@ -310,7 +310,7 @@ public function actionSave($id) { $model = $this->loadModel($id); $uid = null; - if (!Permission::model()->hasSurveyGroupPermission($id, 'permission', 'update')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'update')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $type = App()->getRequest()->getPost('type', 'user'); @@ -376,7 +376,7 @@ public function actionSave($id) public function actionDeleteUser($id, $uid) { $model = $this->loadModel($id); - if (!Permission::model()->hasSurveyGroupPermission($id, 'permission', 'delete')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'delete')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $oUser = User::model()->findByPk($uid); @@ -490,7 +490,7 @@ private function loadModel($id) if ($model === null) { throw new CHttpException(404, 'The requested page does not exist.'); } - if (!Permission::model()->hasSurveyGroupPermission($id, 'permission', 'read')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'read')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } return $model; diff --git a/application/controllers/admin/SurveysGroupsController.php b/application/controllers/admin/SurveysGroupsController.php index 3dbe3d5975a..12ba5646212 100644 --- a/application/controllers/admin/SurveysGroupsController.php +++ b/application/controllers/admin/SurveysGroupsController.php @@ -97,7 +97,7 @@ public function create() * Show and updates a particular model. * If update is successful, the browser will be redirected to the 'view' page. * - * @todo : check if this function can be called with >hasSurveyGroupPermission($id, 'group', 'read') + * @todo : check if this function can be called with >hasSurveysGroupsPermission($id, 'group', 'read') * @param integer $id the ID of the model to be updated * @return void */ @@ -105,7 +105,7 @@ public function update($id) { $model = $this->loadModel($id); if (!empty(App()->getRequest()->getPost('SurveysGroups'))) { - if (!Permission::model()->hasSurveyGroupPermission($id, 'group', 'update')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'group', 'update')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $postSurveysGroups = App()->getRequest()->getPost('SurveysGroups'); @@ -153,8 +153,8 @@ public function update($id) $oSurveySearch->gsid = $model->gsid; $aData['oSurveySearch'] = $oSurveySearch; $aData['aRigths'] = array( - 'update' => Permission::model()->hasSurveyGroupPermission($id, 'group', 'update'), - 'delete' => Permission::model()->hasSurveyGroupPermission($id, 'group', 'delete'), + 'update' => Permission::model()->hasSurveysGroupsPermission($id, 'group', 'update'), + 'delete' => Permission::model()->hasSurveysGroupsPermission($id, 'group', 'delete'), 'owner_id' => $model->owner_id == Yii::app()->user->id || Permission::model()->hasGlobalPermission('superadmin', 'read') ); @@ -188,7 +188,7 @@ public function update($id) * @return void */ public function surveysettingmenues($id) { - if (!Permission::model()->hasSurveyGroupPermission($id, 'surveysettings', 'read')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'read')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } /* Can not call gloalsettings contoller fuinction sice _construct check access … */ @@ -208,7 +208,7 @@ public function surveySettings($id) $bRedirect = 0; /** @var SurveysGroups $model */ $model = $this->loadModel($id); - if (!Permission::model()->hasSurveyGroupPermission($id, 'surveysettings', 'read')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'read')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $aData = array( @@ -222,7 +222,7 @@ public function surveySettings($id) $oSurvey->setOptions(); //this gets the "values" from the group that inherits to this group ... $oSurvey->owner_id = $model->owner_id; - if (App()->getRequest()->isPostRequest && !Permission::model()->hasSurveyGroupPermission($id, 'surveysettings', 'update')) { + if (App()->getRequest()->isPostRequest && !Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'update')) { throw new CHttpException(403, gT("You do not have permission to update survey settings.")); } //every $_POST checked here is one of the switchers(On|Off|Inherit) names @@ -289,7 +289,7 @@ public function surveySettings($id) 'url' => App()->createUrl('surveyAdministration/listsurveys', array('#' => 'surveygroups')), ), ); - if (Permission::model()->hasSurveyGroupPermission($id, 'surveysettings', 'update')) { + if (Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'update')) { $aData['buttons']['savebutton'] = array( 'form' => 'survey-settings-options-form' ); @@ -310,7 +310,7 @@ public function surveySettings($id) public function delete($id) { $oGroupToDelete = $this->loadModel($id); - if (!Permission::model()->hasSurveyGroupPermission($id, 'group', 'delete')) { + if (!Permission::model()->hasSurveysGroupsPermission($id, 'group', 'delete')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $sGroupTitle = $oGroupToDelete->title; @@ -379,7 +379,7 @@ public function loadModel($id) if ($model === null) { throw new CHttpException(404, 'The requested page does not exist.'); } - if (!Permission::model()->hasSurveyGroupPermission($id, SurveysGroups::getMinimalPermissionRead())) { + if (!Permission::model()->hasSurveysGroupsPermission($id, SurveysGroups::getMinimalPermissionRead())) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } return $model; diff --git a/application/models/Permission.php b/application/models/Permission.php index 7e0f4c62669..68a3e1a1e8d 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -671,9 +671,9 @@ public function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD = 'read', $ * @param integer $iUserID User ID - if not given the one of the current user is used * @return bool True if user has the permission */ - public function hasSurveyGroupPermission($iSurveyGroupId, $sPermission, $sCRUD = 'read', $iUserID = null) + public function hasSurveysGroupsPermission($iSurveyGroupId, $sPermission, $sCRUD = 'read', $iUserID = null) { - $oGroup = $this->getSurveysGroup($iSurveyGroupId); + $oGroup = $this->getSurveysGroups($iSurveyGroupId); if (!$oGroup) { return false; } @@ -929,7 +929,7 @@ public function getSurveysInGroup($iSurveysInGroupId) * @param int $iSurveyGroupId * @return SurveysGroups|null */ - public function getSurveysGroup($iSurveyGroupId) + public function getSurveysGroups($iSurveyGroupId) { return SurveysGroups::model()->findByPk($iSurveyGroupId); } diff --git a/application/models/SurveysGroups.php b/application/models/SurveysGroups.php index 006ecdf2f75..fcc91d62f44 100644 --- a/application/models/SurveysGroups.php +++ b/application/models/SurveysGroups.php @@ -194,7 +194,7 @@ public function getColumns() */ public function getHasViewSurveyGroupRight() { - return Permission::model()->hasSurveyGroupPermission($this->gsid, 'group', 'read'); + return Permission::model()->hasSurveysGroupsPermission($this->gsid, 'group', 'read'); } /** @@ -296,17 +296,17 @@ public function getButtons() $sSurveySettingsUrl = App()->createUrl("admin/surveysgroups/sa/surveysettings", array("id"=>$this->gsid)); $sPermissionUrl = App()->createUrl("surveysGroupsPermission/index", array("id"=>$this->gsid)); $button = ''; - if(Permission::model()->hasSurveyGroupPermission($this->gsid,'group','read')) { + if(Permission::model()->hasSurveysGroupsPermission($this->gsid,'group','read')) { $button .= ''.gT('Edit survey group').''; } - if(Permission::model()->hasSurveyGroupPermission($this->gsid,'surveysettings','read')) { + if(Permission::model()->hasSurveysGroupsPermission($this->gsid,'surveysettings','read')) { $button .= ''.gT('Survey settings').''; } - if (Permission::model()->hasSurveyGroupPermission($this->gsid,'permission','read')) { + if (Permission::model()->hasSurveysGroupsPermission($this->gsid,'permission','read')) { $button .= ''.gT('Permission').''; } /* Can not delete group #1 + with survey */ - if ($this->gsid!=1 && !$this->hasSurveys && Permission::model()->hasSurveyGroupPermission($this->gsid,'group','delete')) { + if ($this->gsid!=1 && !$this->hasSurveys && Permission::model()->hasSurveysGroupsPermission($this->gsid,'group','delete')) { $button .= ''.gT('Delete survey group').''; } diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 988b2586a11..91ac28f9f90 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -54,6 +54,7 @@ public function getPermissionData($modelName, $modelId, $userId = null) if(empty($aObjectPermissions)) { return $aObjectPermissions; } + $permissionFunctionName = "has{$modelName}Permission"; /* string[] Crud type array */ $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); foreach (array_keys($aObjectPermissions) as $sPermission) { @@ -62,7 +63,7 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ - 'disabled' => !$this->permission->hasPermission($modelId, $modelName, $sPermission, $crud, $this->user->id), + 'disabled' => !$this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $this->user->id), 'indeterminate' => false ); } @@ -84,7 +85,8 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud]['checked'] = $havePermissionSet; /* The user didn't have the permission set, but have permission by other way (inherited, plugin …) */ if(!$havePermissionSet) { - $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->permission->hasPermission($modelId, $modelName, $sPermission, $crud, $userId); + $functionName = "has{$modelName}Permission"; + $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $userId); } } } diff --git a/application/views/SurveysGroupsPermission/subviews/currentUsersList.php b/application/views/SurveysGroupsPermission/subviews/currentUsersList.php index 46d9653b1cf..e8da49f2fa6 100644 --- a/application/views/SurveysGroupsPermission/subviews/currentUsersList.php +++ b/application/views/SurveysGroupsPermission/subviews/currentUsersList.php @@ -15,13 +15,13 @@ - hasSurveyGroupPermission($model->gsid, 'permission', 'update')) : ?> + hasSurveysGroupsPermission($model->gsid, 'permission', 'update')) : ?> $model->gsid, 'to' => $oUser->uid));?>"> - hasSurveyGroupPermission($model->gsid, 'permission', 'delete')) : ?> + hasSurveysGroupsPermission($model->gsid, 'permission', 'delete')) : ?> createUrl("surveysGroupsPermission/deleteUser", array( 'id'=>$model->gsid, 'uid' => $oUser->uid diff --git a/application/views/SurveysGroupsPermission/subviews/viewCurrents.php b/application/views/SurveysGroupsPermission/subviews/viewCurrents.php index 60325f1aa3c..823d6749f9d 100644 --- a/application/views/SurveysGroupsPermission/subviews/viewCurrents.php +++ b/application/views/SurveysGroupsPermission/subviews/viewCurrents.php @@ -10,7 +10,7 @@ 'aCurrentsUserRights' => $aCurrentsUserRights, )); } ?> - hasSurveyGroupPermission($model->primaryKey, 'permission', 'update')) : ?> + hasSurveysGroupsPermission($model->primaryKey, 'permission', 'update')) : ?>

renderPartial('/SurveysGroupsPermission/subviews/addUserForm',array('model'=>$model,'oAddUserList'=>$oAddUserList)); diff --git a/application/views/admin/surveysgroups/surveySettings.php b/application/views/admin/surveysgroups/surveySettings.php index e6a8da2cc2e..0fbd6f6be88 100644 --- a/application/views/admin/surveysgroups/surveySettings.php +++ b/application/views/admin/surveysgroups/surveySettings.php @@ -33,7 +33,7 @@
- hasSurveyGroupPermission($model->gsid, 'surveysettings', 'update')) : ?> + hasSurveysGroupsPermission($model->gsid, 'surveysettings', 'update')) : ?>
- hasSurveyGroupPermission($model->primaryKey, 'group','read')):?> + hasSurveysGroupsPermission($model->primaryKey, 'group','read')):?>
renderPartial('./surveysgroups/_form', $_data_); ?>
diff --git a/tests/unit/models/PermissionTest.php b/tests/unit/models/PermissionTest.php index 1f320d47a7a..e192c596577 100644 --- a/tests/unit/models/PermissionTest.php +++ b/tests/unit/models/PermissionTest.php @@ -28,13 +28,13 @@ public function testSuperAdmin() $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId', 'getSurveysGroup']) + ->setMethods(['getUserId', 'getSurveysGroups']) ->getMock(); $perm->method('getUserId')->willReturn(1); - $perm->method('getSurveysGroup')->willReturn($surveysGroup); + $perm->method('getSurveysGroups')->willReturn($surveysGroup); $surveysGroupsId = 999; - $this->assertTrue($perm->hasSurveyGroupPermission($surveysGroupsId, 'permission', 'create')); + $this->assertTrue($perm->hasSurveysGroupsPermission($surveysGroupsId, 'permission', 'create')); } /** @@ -56,14 +56,14 @@ public function testOwnershipSuccess() $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId', 'getSurveysGroup', 'getEntity']) + ->setMethods(['getUserId', 'getSurveysGroups', 'getEntity']) ->getMock(); $perm->method('getUserId')->willReturn($userId); - $perm->method('getSurveysGroup')->willReturn($surveysGroup); + $perm->method('getSurveysGroups')->willReturn($surveysGroup); $perm->method('getEntity')->willReturn($surveysGroup); $surveysGroupsId = 999; - $this->assertTrue($perm->hasSurveyGroupPermission($surveysGroupsId, 'permission', 'create')); + $this->assertTrue($perm->hasSurveysGroupsPermission($surveysGroupsId, 'permission', 'create')); } /** @@ -85,13 +85,13 @@ public function testOwnershipFailure() $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId', 'getSurveysGroup', 'getEntity']) + ->setMethods(['getUserId', 'getSurveysGroups', 'getEntity']) ->getMock(); $perm->method('getUserId')->willReturn($userId); - $perm->method('getSurveysGroup')->willReturn($surveysGroup); + $perm->method('getSurveysGroups')->willReturn($surveysGroup); $perm->method('getEntity')->willReturn($surveysGroup); $surveysGroupsId = 999; - $this->assertFalse($perm->hasSurveyGroupPermission($surveysGroupsId, 'permission', 'create')); + $this->assertFalse($perm->hasSurveysGroupsPermission($surveysGroupsId, 'permission', 'create')); } } From 043caa4d80f0657f1047bd65008eeca78e796da9 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 5 Jan 2021 18:56:12 +0100 Subject: [PATCH 07/22] Dev: Fix #14551 (and test) for SurveysGroups --- .../SurveysGroupsPermissionController.php | 24 ++++++++++++------- application/models/SurveysGroups.php | 2 +- .../models/services/PermissionManager.php | 13 +++++++--- .../subviews/setPermissionForm.php | 2 +- .../subviews/viewCurrents.php | 2 +- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 58934513328..0293d8b61ee 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -111,6 +111,7 @@ public function actionIndex($id) $oAddUserList = array(); $oAddGroupList = array(); + if (Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { /* Search user withouth rights on SurveyGroup */ /* @todo : move this to : SurveysGroups ? Permission ? User ?*/ @@ -350,7 +351,7 @@ public function actionSave($id) foreach ($set as $entity => $aPermissionSet) { foreach ($uids as $uid) { /* Permission::model()->setPermissions return true or break */ - $success = $success && !$PermissionManagerService->setPermissions($uid, $entity, $id); + $success = $success && $PermissionManagerService->setPermissions($uid, $entity, $id); } } if($success) { @@ -448,11 +449,13 @@ private function viewUserOrUserGroup($id, $to, $type = 'user') $aSurveysGroupsPermissions, $aSurveysInGroupPermissions ); - - $aData = array( - 'model' => $model, - 'subview' => 'setPermissionForm', - 'buttons' => array( + $buttons = array( + 'closebutton' => array( + 'url' => App()->createUrl('surveyAdministration/listsurveys', array('#' => 'surveygroups')), + ) + ); + if(Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'update')) { + $buttons = array( 'savebutton' => array( 'form' => 'permissionsSave' ), @@ -461,8 +464,13 @@ private function viewUserOrUserGroup($id, $to, $type = 'user') ), 'closebutton' => array( 'url' => App()->createUrl('surveyAdministration/listsurveys', array('#' => 'surveygroups')), - ), - ), + ) + ); + } + $aData = array( + 'model' => $model, + 'subview' => 'setPermissionForm', + 'buttons' => $buttons ); $aData['aPermissionData'] = array( 'aPermissions' => $aPermissions, diff --git a/application/models/SurveysGroups.php b/application/models/SurveysGroups.php index fcc91d62f44..5546704bfb1 100644 --- a/application/models/SurveysGroups.php +++ b/application/models/SurveysGroups.php @@ -444,7 +444,7 @@ public static function getPermissionData($key = null) 'img' => ' fa fa-edit', ), 'permission' => array( - 'create' => false, /* since need update to set permission */ + 'create' => true, /* allowed to add new users or group */ 'read' => true, 'update' => true, 'delete' => true, /* update ? */ diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 91ac28f9f90..de7acf63ca0 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -12,6 +12,9 @@ use LSWebUser; use Permission; +use App; +use CHtml; + /** * Service class for managing permission */ @@ -112,6 +115,8 @@ public function setPermissions($userId, $modelName, $modelId) /* Nothing to do */ return $success; } + /* string : function name to use for final Permission */ + $permissionFunctionName = "has{$modelName}Permission"; /* string[] Crud type array */ $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); $entityPermissionsToSet = $permissionsToSet[$modelName]; @@ -123,7 +128,7 @@ public function setPermissions($userId, $modelName, $modelId) $aSetPermissions[$sPermission] = array(); foreach ($aCruds as $crud) { /* Check if current user have the permission to set */ - if($this->permission->hasPermission($modelId, $modelName, $sPermission, $crud, $this->user->id)) { + if($this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $this->user->id)) { $aSetPermissions[$sPermission][$crud] = !empty($aPermission[$crud]) && !empty($entityPermissionsToSet[$sPermission][$crud]); } } @@ -152,7 +157,7 @@ public function setPermissions($userId, $modelName, $modelId) ) ); if(empty($oCurrentPermission)) { - $oCurrentPermission = $this->getNewPermission(); // ????? + $oCurrentPermission = $this->getNewPermission(); $oCurrentPermission->entity = $modelName; $oCurrentPermission->entity_id = $modelId; $oCurrentPermission->uid = $userId; @@ -162,10 +167,12 @@ public function setPermissions($userId, $modelName, $modelId) foreach($aSetPermission as $crud => $permission) { $oCurrentPermission->setAttribute("{$crud}_p",intval($permission)); } - if($oCurrentPermission->save()) { + if(!$oCurrentPermission->save()) { $success = false; + App()->setFlashMessage(CHtml::errorSummary($oCurrentPermission),'warning'); } } + $this->permission::setMinimalEntityPermission($userId, $modelId, $modelName); return $success; } diff --git a/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php b/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php index f771d96d9bc..a7d36382266 100644 --- a/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php +++ b/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php @@ -48,7 +48,7 @@ 'id' => CHtml::getIdByName("set[{$aCurrentPermissions['entity']}][{$sPermission}][$sKey]"), 'uncheckValue' => 0, /* See issue #14551 : https://bugs.limesurvey.org/view.php?id=14551 */ - 'disabled' => null, // $aValues['disabled'], unused : in survey : user can set permission even if it don't have it. + 'disabled' => $aValues['disabled'], ) ); }?> diff --git a/application/views/SurveysGroupsPermission/subviews/viewCurrents.php b/application/views/SurveysGroupsPermission/subviews/viewCurrents.php index 823d6749f9d..13f8178a9a2 100644 --- a/application/views/SurveysGroupsPermission/subviews/viewCurrents.php +++ b/application/views/SurveysGroupsPermission/subviews/viewCurrents.php @@ -10,7 +10,7 @@ 'aCurrentsUserRights' => $aCurrentsUserRights, )); } ?> - hasSurveysGroupsPermission($model->primaryKey, 'permission', 'update')) : ?> + hasSurveysGroupsPermission($model->primaryKey, 'permission', 'create')) : ?>

renderPartial('/SurveysGroupsPermission/subviews/addUserForm',array('model'=>$model,'oAddUserList'=>$oAddUserList)); From 1fcb16d7c92e1841ac423117f933e97ea5865ebd Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Wed, 6 Jan 2021 11:38:12 +0100 Subject: [PATCH 08/22] Dev: Create PermissionManager->getCurrentPermission --- .../models/services/PermissionManager.php | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index de7acf63ca0..4e218828660 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -57,7 +57,6 @@ public function getPermissionData($modelName, $modelId, $userId = null) if(empty($aObjectPermissions)) { return $aObjectPermissions; } - $permissionFunctionName = "has{$modelName}Permission"; /* string[] Crud type array */ $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); foreach (array_keys($aObjectPermissions) as $sPermission) { @@ -66,7 +65,7 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ - 'disabled' => !$this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $this->user->id), + 'disabled' => !$this->getCurrentPermission($modelName, $modelId, $sPermission, $crud, $this->user->id), 'indeterminate' => false ); } @@ -88,8 +87,7 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud]['checked'] = $havePermissionSet; /* The user didn't have the permission set, but have permission by other way (inherited, plugin …) */ if(!$havePermissionSet) { - $functionName = "has{$modelName}Permission"; - $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $userId); + $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->getCurrentPermission($modelName, $modelId, $sPermission, $crud, $userId); } } } @@ -115,10 +113,10 @@ public function setPermissions($userId, $modelName, $modelId) /* Nothing to do */ return $success; } - /* string : function name to use for final Permission */ - $permissionFunctionName = "has{$modelName}Permission"; + /* string[] Crud type array */ $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); + /* @array[] the permissions to set for this model (via POST value) */ $entityPermissionsToSet = $permissionsToSet[$modelName]; $aBasePermissions = Permission::getEntityBasePermissions($modelName); @@ -127,8 +125,8 @@ public function setPermissions($userId, $modelName, $modelId) foreach ($aBasePermissions as $sPermission => $aPermission) { $aSetPermissions[$sPermission] = array(); foreach ($aCruds as $crud) { - /* Check if current user have the permission to set */ - if($this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $this->user->id)) { + /* Only set value if current user have the permission to set */ + if($this->getCurrentPermission($modelName, $modelId, $sPermission, $crud, $this->user->id)) { $aSetPermissions[$sPermission][$crud] = !empty($aPermission[$crud]) && !empty($entityPermissionsToSet[$sPermission][$crud]); } } @@ -185,4 +183,23 @@ public function getNewPermission() { return new Permission(); } + + /** + * get the current permission for the current user + * @param string $modelName the model name + * @param integer $modelId the model id + * @param string $sPermission the specific permission name + * @param string $crud the crud ('create', 'read', 'update', 'delete', 'import', 'export') + * @return boolean + */ + public function getCurrentPermission($modelName, $modelId, $sPermission, $crud, $userId) + { + /* string : function name to use for final Permission */ + $permissionFunctionName = "has{$modelName}Permission"; + if(method_exists($this->permission,$permissionFunctionName)) { + return $this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $userId); + } + return Permission::hasPermission(0, "global", $modelName, $crud , $this->user->id); // Only + } + } From 9fa2dc92c2dc9c3fd30350982c52e042386059dd Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Wed, 6 Jan 2021 15:48:14 +0100 Subject: [PATCH 09/22] Dev: remove permisison in constructor, send model Dev: deprecate has hasSurveysGroupsPermission and hasSurveysInGroupPermission Dev: replace by model directly --- .../SurveysGroupsPermissionController.php | 28 ++-- application/models/LSActiveRecord.php | 13 ++ application/models/Permission.php | 31 +---- application/models/SurveysGroups.php | 18 +++ application/models/SurveysInGroup.php | 25 ++++ .../models/services/PermissionManager.php | 121 +++++++++--------- 6 files changed, 137 insertions(+), 99 deletions(-) diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 0293d8b61ee..0ed494216c9 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -342,16 +342,18 @@ public function actionSave($id) $set = App()->getRequest()->getPost('set'); $user = App()->user; $request = App()->request; - $PermissionManagerService = new PermissionManager( - $request, - $user, - new Permission() - ); $success = true; - foreach ($set as $entity => $aPermissionSet) { + foreach ($set as $entityName => $aPermissionSet) { + /* Must get SurveysIngroup for SurveysIngroup entity */ + $entity = $entityName::model()->findByPk($id); + $PermissionManagerService = new PermissionManager( + $request, + $user, + $entity + ); foreach ($uids as $uid) { /* Permission::model()->setPermissions return true or break */ - $success = $success && $PermissionManagerService->setPermissions($uid, $entity, $id); + $success = $success && $PermissionManagerService->setPermissions($uid); } } if($success) { @@ -440,11 +442,15 @@ private function viewUserOrUserGroup($id, $to, $type = 'user') $PermissionManagerService = new PermissionManager( $request, $user, - new Permission() + $model ); - - $aSurveysGroupsPermissions = $PermissionManagerService->getPermissionData('SurveysGroups', $id, $userId); - $aSurveysInGroupPermissions = $PermissionManagerService->getPermissionData('SurveysInGroup', $id, $userId); + $aSurveysGroupsPermissions = $PermissionManagerService->getPermissionData($userId); + $PermissionManagerService = new PermissionManager( + $request, + $user, + SurveysInGroup::model()->findByPk($id) + ); + $aSurveysInGroupPermissions = $PermissionManagerService->getPermissionData($userId); $aPermissions = array_merge( $aSurveysGroupsPermissions, $aSurveysInGroupPermissions diff --git a/application/models/LSActiveRecord.php b/application/models/LSActiveRecord.php index 0027a3919b7..689813bff33 100644 --- a/application/models/LSActiveRecord.php +++ b/application/models/LSActiveRecord.php @@ -474,4 +474,17 @@ public static function getMinimalPermissionRead() { return null; } + + /** + * Get the permission of current model + * @param string $sPermission Name of the permission + * @param string $sCRUD The permission detail you want to check on: 'create','read','update','delete','import' or 'export' + * @param integer $iUserID User ID - if not given the one of the current user is used + * @return boolean + */ + public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null) + { + /* By default : check only global permission for class, sure superadmin have all rights */ + return Permission::model()->hasPermission(0, 'global', get_class($this), $sCRUD, $iUserID); + } } diff --git a/application/models/Permission.php b/application/models/Permission.php index 68a3e1a1e8d..be1f152028a 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -505,7 +505,7 @@ public function copySurveyPermissions($iSurveyIDSource, $iSurveyIDTarget) * @param $iUserID integer User ID - if not given the one of the current user is used * @return bool True if user has the permission */ - public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = 'read', $iUserID = null, $iPermissionRoleId = null) + public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = 'read', $iUserID = null) { // TODO: in entry script, if CConsoleApplication, set user as superadmin if (is_null($iUserID) && Yii::app() instanceof CConsoleApplication) { @@ -524,7 +524,6 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = ' $oEvent->set('sPermission', $sPermission); $oEvent->set('sCRUD', $sCRUD); $oEvent->set('iUserID', $iUserID); - $oEvent->set('iPermissionRoleId', $iPermissionRoleId); App()->getPluginManager()->dispatchEvent($oEvent); $pluginbPermission = $oEvent->get('bPermission'); @@ -629,9 +628,9 @@ public static function isForcedSuperAdmin($iUserID) * @param $iUserID integer User ID - if not given the one of the current user is used * @return bool True if user has the permission */ - public function hasGlobalPermission($sPermission, $sCRUD = 'read', $iUserID = null, $iPermissionRoleId = null) + public function hasGlobalPermission($sPermission, $sCRUD = 'read', $iUserID = null) { - return $this->hasPermission(0, 'global', $sPermission, $sCRUD, $iUserID, $iPermissionRoleId); + return $this->hasPermission(0, 'global', $sPermission, $sCRUD, $iUserID); } /** @@ -664,7 +663,8 @@ public function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD = 'read', $ /** * Checks if a user has a certain permission in the given survey group - * + * @deprecated (can be remove before next RC) + * @see SurveysGroups->getPermission * @param integer $iSurveyGroupId The survey group ID * @param string $sPermission Name of the permission * @param string $sCRUD The permission detail you want to check on: 'create','read','update','delete','import' or 'export' @@ -673,13 +673,7 @@ public function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD = 'read', $ */ public function hasSurveysGroupsPermission($iSurveyGroupId, $sPermission, $sCRUD = 'read', $iUserID = null) { - $oGroup = $this->getSurveysGroups($iSurveyGroupId); - if (!$oGroup) { - return false; - } - // Get global correspondance for surveysgroups rigth, keep it in case in develop - $sGlobalCRUD = $sCRUD; - return $this->hasGlobalPermission('surveysgroups', $sGlobalCRUD, $iUserID) || $this->hasPermission($iSurveyGroupId, 'surveysgroups', $sPermission, $sCRUD, $iUserID); + return SurveysGroups::model()->findByPk($iSurveyGroupId)->getCurrentPermission($sPermission, $sCRUD, $iUserID); } /** @@ -693,18 +687,7 @@ public function hasSurveysGroupsPermission($iSurveyGroupId, $sPermission, $sCRUD */ public function hasSurveysInGroupPermission($iSurveyGroupId, $sPermission, $sCRUD = 'read', $iUserID = null) { - $oGroup = $this->getSurveysInGroup($iSurveyGroupId); - if (!$oGroup) { - return false; - } - $sGlobalCRUD = $sCRUD; - if (($sCRUD == 'create' || $sCRUD == 'import')) { // Create and import (token, reponse , question content …) need only allow update surveys - $sGlobalCRUD = 'update'; - } - if (($sCRUD == 'delete' && $sPermission != 'survey')) { // Delete (token, reponse , question content …) need only allow update surveys - $sGlobalCRUD = 'update'; - } - return $this->hasGlobalPermission('surveys', $sGlobalCRUD, $iUserID) || $this->hasPermission($iSurveyGroupId, 'surveysingroup', $sPermission, $sCRUD, $iUserID); + return SurveysInGroup::model()->findByPk($iSurveyGroupId)->getCurrentPermission($sPermission, $sCRUD, $iUserID); } /** diff --git a/application/models/SurveysGroups.php b/application/models/SurveysGroups.php index 5546704bfb1..25b3d1fdc0e 100644 --- a/application/models/SurveysGroups.php +++ b/application/models/SurveysGroups.php @@ -481,4 +481,22 @@ public function getOwnerId() { return $this->owner_id; } + + /** + * @inheritdoc + */ + public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null) + { + /* If have global : return true */ + if (Permission::model()->hasPermission(0, 'global', 'surveysgroups', $sCRUD, $iUserID)) { + return true; + } + /* Specific need gsid */ + if(!$this->gsid) { + return false; + } + /* Finally : return specific one */ + return Permission::model()->hasPermission($this->gsid, 'surveysgroups', $sPermission, $sCRUD, $iUserID); + } + } diff --git a/application/models/SurveysInGroup.php b/application/models/SurveysInGroup.php index 8ce9d315247..14e73189307 100644 --- a/application/models/SurveysInGroup.php +++ b/application/models/SurveysInGroup.php @@ -70,4 +70,29 @@ public static function getMinimalPermissionRead() { return null; } + + /** + * @inheritdoc + */ + public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null) + { + /* If have global for surveys : return true */ + $sGlobalCRUD = $sCRUD; + if (($sCRUD == 'create' || $sCRUD == 'import')) { // Create and import (token, reponse , question content …) need only allow update surveys + $sGlobalCRUD = 'update'; + } + if (($sCRUD == 'delete' && $sPermission != 'survey')) { // Delete (token, reponse , question content …) need only allow update surveys + $sGlobalCRUD = 'update'; + } + if (Permission::model()->hasPermission(0, 'global', 'surveys', $sGlobalCRUD, $iUserID)) { + return true; + } + /* Specific need gsid */ + if(!$this->gsid) { + return false; + } + /* Finally : return specific one */ + return Permission::model()->hasPermission($this->gsid, 'surveysingroup', $sPermission, $sCRUD, $iUserID); + } + } diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 4e218828660..5961429488b 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -11,6 +11,7 @@ use LSHttpRequest; use LSWebUser; use Permission; +use LSActiveRecord; use App; use CHtml; @@ -27,8 +28,8 @@ class PermissionManager /** @var LSWebUser */ private $user; - /** @var Permission model */ - private $permission; + /** @var LSActiveRecord model where permission is checked*/ + private $model; /** * @param LSHttpRequest $request @@ -37,23 +38,21 @@ class PermissionManager public function __construct( LSHttpRequest $request, LSWebUser $user, - Permission $permission + LSActiveRecord $model ) { $this->request = $request; $this->user = $user; - $this->permission = $permission; + $this->model = $model; } /** * get the permission data - * @param string $modelName the model name - * @param integer $modelId the model id * @param integer $userId for this user id * @return array[] */ - public function getPermissionData($modelName, $modelId, $userId = null) + public function getPermissionData($userId = null) { - $aObjectPermissions = Permission::getEntityBasePermissions($modelName); // Usage of static, db not needed + $aObjectPermissions = $this->model::getPermissionData(); // Usage of static, db not needed if(empty($aObjectPermissions)) { return $aObjectPermissions; } @@ -65,34 +64,31 @@ public function getPermissionData($modelName, $modelId, $userId = null) $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ - 'disabled' => !$this->getCurrentPermission($modelName, $modelId, $sPermission, $crud, $this->user->id), + 'disabled' => !$this->model->getCurrentPermission($sPermission, $crud, $this->user->id), 'indeterminate' => false ); } /* If user id is set : update the data with permission of this user */ if($userId) { - $oCurrentPermissions = $this->permission->find( - "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", - array( - ":entity" => $modelName, - ":entity_id" => $modelId, - ":uid" => $userId, - ":permission" => $sPermission - ) + $oCurrentPermission = $this->getDbPermission( + get_class($this->model), + $this->model->getPrimaryKey(), + $userId, + $sPermission ); foreach ($aCruds as $crud) { if ($aObjectPermissions[$sPermission][$crud]) { - $havePermissionSet = !empty($oCurrentPermissions) && $oCurrentPermissions->getAttribute("{$crud}_p"); + $havePermissionSet = !empty($oCurrentPermission) && $oCurrentPermission->getAttribute("{$crud}_p"); /* The user have the permission set */ $aObjectPermissions[$sPermission]['current'][$crud]['checked'] = $havePermissionSet; /* The user didn't have the permission set, but have permission by other way (inherited, plugin …) */ if(!$havePermissionSet) { - $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->getCurrentPermission($modelName, $modelId, $sPermission, $crud, $userId); + $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->model->getCurrentPermission($sPermission, $crud, $userId); } } } } - $aObjectPermissions[$sPermission]['entity'] = $modelName; + $aObjectPermissions[$sPermission]['entity'] = get_class($this->model); } return $aObjectPermissions; } @@ -101,15 +97,13 @@ public function getPermissionData($modelName, $modelId, $userId = null) * @todo : Save Permission by POST value according to current user permssion * @see Permission::setPermissions * @param mixed $iUserID - * @param string $modelName - * @param mixed $modelId * @return boolean */ - public function setPermissions($userId, $modelName, $modelId) + public function setPermissions($userId) { $success = true; $permissionsToSet = $this->request->getPost('set'); - if (empty($permissionsToSet[$modelName])) { + if (empty($permissionsToSet[get_class($this->model)])) { /* Nothing to do */ return $success; } @@ -117,8 +111,8 @@ public function setPermissions($userId, $modelName, $modelId) /* string[] Crud type array */ $aCruds = array('create', 'read', 'update', 'delete', 'import', 'export'); /* @array[] the permissions to set for this model (via POST value) */ - $entityPermissionsToSet = $permissionsToSet[$modelName]; - $aBasePermissions = Permission::getEntityBasePermissions($modelName); + $entityPermissionsToSet = $permissionsToSet[get_class($this->model)]; + $aBasePermissions = $this->getPermissionData(); /* string[] The array to set (or not) */ $aSetPermissions = array(); @@ -126,7 +120,7 @@ public function setPermissions($userId, $modelName, $modelId) $aSetPermissions[$sPermission] = array(); foreach ($aCruds as $crud) { /* Only set value if current user have the permission to set */ - if($this->getCurrentPermission($modelName, $modelId, $sPermission, $crud, $this->user->id)) { + if($this->model->getCurrentPermission($sPermission, $crud, $this->user->id)) { $aSetPermissions[$sPermission][$crud] = !empty($aPermission[$crud]) && !empty($entityPermissionsToSet[$sPermission][$crud]); } } @@ -136,31 +130,21 @@ public function setPermissions($userId, $modelName, $modelId) // Event $oEvent = new \LimeSurvey\PluginManager\PluginEvent('beforePermissionSetSave'); $oEvent->set('aNewPermissions', $aSetPermissions); - if($modelName == 'Survey') { - $oEvent->set('iSurveyID', $modelId); + if(get_class($this->model) == 'Survey') { + $oEvent->set('iSurveyID', $this->model->getPrimaryKey()); } - $oEvent->set('entity', $modelName); /* New in 4.4.X */ - $oEvent->set('entityId', $modelId); /* New in 4.4.X */ + $oEvent->set('entity', get_class($this->model)); /* New in 4.4.X */ + $oEvent->set('entityId', $this->model->getPrimaryKey()); /* New in 4.4.X */ $oEvent->set('iUserID', $userId); App()->getPluginManager()->dispatchEvent($oEvent); foreach($aSetPermissions as $sPermission => $aSetPermission) { - $oCurrentPermission = $this->permission->find( - "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", - array( - ":entity" => $modelName, - ":entity_id" => $modelId, - ":uid" => $userId, - ":permission" => $sPermission - ) + $oCurrentPermission = $this->getOrCreateDbPermission( + get_class($this->model), + $this->model->getPrimaryKey(), + $userId, + $sPermission ); - if(empty($oCurrentPermission)) { - $oCurrentPermission = $this->getNewPermission(); - $oCurrentPermission->entity = $modelName; - $oCurrentPermission->entity_id = $modelId; - $oCurrentPermission->uid = $userId; - $oCurrentPermission->permission = $sPermission; - } /* Set only the permission set in $aSetPermission : user have the rights */ foreach($aSetPermission as $crud => $permission) { $oCurrentPermission->setAttribute("{$crud}_p",intval($permission)); @@ -170,36 +154,45 @@ public function setPermissions($userId, $modelName, $modelId) App()->setFlashMessage(CHtml::errorSummary($oCurrentPermission),'warning'); } } - $this->permission::setMinimalEntityPermission($userId, $modelId, $modelName); + Permission::setMinimalEntityPermission($userId, $this->model->getPrimaryKey(), get_class($this->model)); return $success; } /** - * Create a new Permission Model + * Get DB permission, create if needed * To be mocked for test * @return \Permission */ - public function getNewPermission() + public function getOrCreateDbPermission($entityName, $entityId, $userId, $sPermission) { - return new Permission(); + $oPermission = $this->getDbPermission($entityName, $entityId, $userId, $sPermission); + if ($oPermission) { + return $oPermission; + } + $oPermission = new Permission; + $oPermission->entity = $entityName; + $oPermission->entity_id = $entityId; + $oPermission->uid = $userId; + $oPermission->permission = $sPermission; + return $oPermission; } - /** - * get the current permission for the current user - * @param string $modelName the model name - * @param integer $modelId the model id - * @param string $sPermission the specific permission name - * @param string $crud the crud ('create', 'read', 'update', 'delete', 'import', 'export') - * @return boolean + * Get DB permission + * To be mocked for test + * @return \Permission */ - public function getCurrentPermission($modelName, $modelId, $sPermission, $crud, $userId) + public function getDbPermission($entityName, $entityId, $userId, $sPermission) { - /* string : function name to use for final Permission */ - $permissionFunctionName = "has{$modelName}Permission"; - if(method_exists($this->permission,$permissionFunctionName)) { - return $this->permission->$permissionFunctionName($modelId, $sPermission, $crud, $userId); - } - return Permission::hasPermission(0, "global", $modelName, $crud , $this->user->id); // Only + $oPermission = Permission::model()->find( + "entity = :entity AND entity_id = :entity_id AND uid = :uid AND permission = :permission", + array( + ":entity" => $entityName, + ":entity_id" => $entityId, + ":uid" => $userId, + ":permission" => $sPermission + ) + ); + return $oPermission; } } From 9da614017432c9b2f6c70fd9eb059d333e8eb710 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 12:38:14 +0100 Subject: [PATCH 10/22] Dev: move to Traits and Interfaces Dev: check functionnality ownerManageAllSurveysInGroup with success Dev: (onwer of group can not give rights on surveysInGrou if set to false) --- application/config/internal.php | 2 + .../SurveysGroupsPermissionController.php | 20 ++-- .../controllers/ThemeOptionsController.php | 28 +++-- .../admin/SurveysGroupsController.php | 19 ++-- .../models/Interfaces/PermissionInterface.php | 9 ++ application/models/LSActiveRecord.php | 42 -------- application/models/Permission.php | 101 ++++-------------- application/models/Survey.php | 32 +++++- application/models/SurveysGroups.php | 18 ++-- application/models/SurveysInGroup.php | 6 +- application/models/TemplateConfiguration.php | 12 ++- application/models/Traits/PermissionTrait.php | 56 ++++++++++ .../models/services/PermissionManager.php | 45 +++++--- .../subviews/currentUsersList.php | 4 +- .../subviews/viewCurrents.php | 2 +- .../admin/surveysgroups/surveySettings.php | 2 +- .../views/admin/surveysgroups/update.php | 4 +- tests/unit/models/PermissionTest.php | 19 ++-- 18 files changed, 224 insertions(+), 197 deletions(-) create mode 100644 application/models/Interfaces/PermissionInterface.php create mode 100644 application/models/Traits/PermissionTrait.php diff --git a/application/config/internal.php b/application/config/internal.php index aa657b4e509..d401193a054 100644 --- a/application/config/internal.php +++ b/application/config/internal.php @@ -98,6 +98,8 @@ 'application.core.*', 'application.core.db.*', 'application.models.*', + 'application.models.Interfaces.*', + 'application.models.Traits.*', 'application.helpers.*', 'application.controllers.*', 'application.modules.*', diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 0ed494216c9..2d88f2682be 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -93,7 +93,7 @@ public function actionIndex($id) foreach ($aSurveysGroupsPermissions as $sPermission => $aPermissions) { $aCurrentsUserRights[$oUser->uid][$sPermission] = array(); foreach (array_intersect_key($aPermissions, array_flip($aCruds)) as $sCrud => $available) { - if ($available && Permission::model()->hasSurveysGroupsPermission($id, $sPermission, $sCrud, $oUser->uid)) { + if ($available && $model->hasPermission($sPermission, $sCrud, $oUser->uid)) { $aCurrentsUserRights[$oUser->uid][$sPermission][] = $sCrud; } } @@ -101,7 +101,7 @@ public function actionIndex($id) foreach ($aSurveysInGroupPermissions as $sPermission => $aPermissions) { $aCurrentsUserRights[$oUser->uid][$sPermission] = array(); foreach (array_intersect_key($aPermissions, array_flip($aCruds)) as $sCrud => $available) { - if ($available && Permission::model()->hasSurveysGroupsPermission($id, $sPermission, $sCrud, $oUser->uid)) { + if ($available && $model->hasPermission($sPermission, $sCrud, $oUser->uid)) { $aCurrentsUserRights[$oUser->uid][$sPermission][] = $sCrud; } } @@ -112,7 +112,7 @@ public function actionIndex($id) $oAddUserList = array(); $oAddGroupList = array(); - if (Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { + if ($model->hasPermission('permission', 'create')) { /* Search user withouth rights on SurveyGroup */ /* @todo : move this to : SurveysGroups ? Permission ? User ?*/ $oCriteria = new CDbCriteria(); @@ -159,7 +159,7 @@ public function actionIndex($id) public function actionAddUser($id) { $model = $this->loadModel($id); - if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { + if (!$model->hasPermission('permission', 'create')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $uid = App()->getRequest()->getPost('uid'); @@ -211,7 +211,7 @@ public function actionAddUser($id) public function actionAddUserGroup($id) { $model = $this->loadModel($id); - if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'create')) { + if (!$model->hasPermission('permission', 'create')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $ugid = App()->getRequest()->getPost('ugid'); @@ -311,7 +311,7 @@ public function actionSave($id) { $model = $this->loadModel($id); $uid = null; - if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'update')) { + if (!$model->hasPermission('permission', 'update')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $type = App()->getRequest()->getPost('type', 'user'); @@ -339,7 +339,7 @@ public function actionSave($id) } $uids = array($uid); } - $set = App()->getRequest()->getPost('set'); + $set = App()->getRequest()->getPost('set', array()); $user = App()->user; $request = App()->request; $success = true; @@ -379,7 +379,7 @@ public function actionSave($id) public function actionDeleteUser($id, $uid) { $model = $this->loadModel($id); - if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'delete')) { + if (!$model->hasPermission('permission', 'delete')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $oUser = User::model()->findByPk($uid); @@ -460,7 +460,7 @@ private function viewUserOrUserGroup($id, $to, $type = 'user') 'url' => App()->createUrl('surveyAdministration/listsurveys', array('#' => 'surveygroups')), ) ); - if(Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'update')) { + if($model->hasPermission('permission', 'update')) { $buttons = array( 'savebutton' => array( 'form' => 'permissionsSave' @@ -504,7 +504,7 @@ private function loadModel($id) if ($model === null) { throw new CHttpException(404, 'The requested page does not exist.'); } - if (!Permission::model()->hasSurveysGroupsPermission($id, 'permission', 'read')) { + if (!$model->hasPermission('permission', 'read')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } return $model; diff --git a/application/controllers/ThemeOptionsController.php b/application/controllers/ThemeOptionsController.php index 11a2a4c021a..6a92ba74186 100644 --- a/application/controllers/ThemeOptionsController.php +++ b/application/controllers/ThemeOptionsController.php @@ -356,10 +356,15 @@ public function actionUpdateSurvey() : void */ public function actionUpdateSurveyGroup(int $id = null, int $gsid, $l = null) : void { - if (!Permission::model()->hasGlobalPermission('templates', 'update') - && !Permission::model()->hasSurveysInGroupPermission($gsid, 'surveys', 'update') - ) { - throw new CHttpException(403, gT("You do not have permission to access this page.")); + if (!Permission::model()->hasGlobalPermission('templates', 'update')) + { + if(empty($gsid)) { + throw new CHttpException(403, gT("You do not have permission to access this page.")); + } + $oSurveysInGroup = SurveysInGroup::model()->findByPk($gsid); + if(empty($oSurveysInGroup) && !$oSurveysInGroup->hasPermission('surveys', 'update')) { + throw new CHttpException(403, gT("You do not have permission to access this page.")); + } } $sTemplateName = $id !== null ? TemplateConfiguration::model()->findByPk($id)->template_name : null; $model = TemplateConfiguration::getInstance($sTemplateName, $gsid); @@ -586,12 +591,17 @@ public function actionUninstall() : void */ public function actionReset(int $gsid) : void { - $templatename = App()->request->getPost('templatename'); - if (!Permission::model()->hasGlobalPermission('templates', 'update') - && !Permission::model()->hasSurveysInGroupPermission($gsid, 'surveys', 'update') - ) { - throw new CHttpException(403, gT("You do not have permission to access this page.")); + if (!Permission::model()->hasGlobalPermission('templates', 'update')) + { + if(empty($gsid)) { + throw new CHttpException(403, gT("You do not have permission to access this page.")); + } + $oSurveysInGroup = SurveysInGroup::model()->findByPk($gsid); + if(empty($oSurveysInGroup) && !$oSurveysInGroup->hasPermission('surveys', 'update')) { + throw new CHttpException(403, gT("You do not have permission to access this page.")); + } } + $templatename = App()->request->getPost('templatename'); if($gsid) { $oTemplateConfiguration = TemplateConfiguration::model()->find("gsid = :gsid AND template_name = :templatename", diff --git a/application/controllers/admin/SurveysGroupsController.php b/application/controllers/admin/SurveysGroupsController.php index 12ba5646212..80f009e0543 100644 --- a/application/controllers/admin/SurveysGroupsController.php +++ b/application/controllers/admin/SurveysGroupsController.php @@ -97,7 +97,6 @@ public function create() * Show and updates a particular model. * If update is successful, the browser will be redirected to the 'view' page. * - * @todo : check if this function can be called with >hasSurveysGroupsPermission($id, 'group', 'read') * @param integer $id the ID of the model to be updated * @return void */ @@ -105,7 +104,7 @@ public function update($id) { $model = $this->loadModel($id); if (!empty(App()->getRequest()->getPost('SurveysGroups'))) { - if (!Permission::model()->hasSurveysGroupsPermission($id, 'group', 'update')) { + if (!$model->hasPermission('group', 'update')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $postSurveysGroups = App()->getRequest()->getPost('SurveysGroups'); @@ -153,8 +152,8 @@ public function update($id) $oSurveySearch->gsid = $model->gsid; $aData['oSurveySearch'] = $oSurveySearch; $aData['aRigths'] = array( - 'update' => Permission::model()->hasSurveysGroupsPermission($id, 'group', 'update'), - 'delete' => Permission::model()->hasSurveysGroupsPermission($id, 'group', 'delete'), + 'update' => $model->hasPermission('group', 'update'), + 'delete' => $model->hasPermission('group', 'delete'), 'owner_id' => $model->owner_id == Yii::app()->user->id || Permission::model()->hasGlobalPermission('superadmin', 'read') ); @@ -188,7 +187,7 @@ public function update($id) * @return void */ public function surveysettingmenues($id) { - if (!Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'read')) { + if (!$this->loadModel($id)->hasPermission('surveysettings', 'read')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } /* Can not call gloalsettings contoller fuinction sice _construct check access … */ @@ -208,7 +207,7 @@ public function surveySettings($id) $bRedirect = 0; /** @var SurveysGroups $model */ $model = $this->loadModel($id); - if (!Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'read')) { + if (!$model->hasPermission('surveysettings', 'read')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $aData = array( @@ -222,7 +221,7 @@ public function surveySettings($id) $oSurvey->setOptions(); //this gets the "values" from the group that inherits to this group ... $oSurvey->owner_id = $model->owner_id; - if (App()->getRequest()->isPostRequest && !Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'update')) { + if (App()->getRequest()->isPostRequest && !$model->hasPermission('surveysettings', 'update')) { throw new CHttpException(403, gT("You do not have permission to update survey settings.")); } //every $_POST checked here is one of the switchers(On|Off|Inherit) names @@ -289,7 +288,7 @@ public function surveySettings($id) 'url' => App()->createUrl('surveyAdministration/listsurveys', array('#' => 'surveygroups')), ), ); - if (Permission::model()->hasSurveysGroupsPermission($id, 'surveysettings', 'update')) { + if ($model->hasPermission('surveysettings', 'update')) { $aData['buttons']['savebutton'] = array( 'form' => 'survey-settings-options-form' ); @@ -310,7 +309,7 @@ public function surveySettings($id) public function delete($id) { $oGroupToDelete = $this->loadModel($id); - if (!Permission::model()->hasSurveysGroupsPermission($id, 'group', 'delete')) { + if (!$oGroupToDelete->hasPermission('group', 'delete')) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } $sGroupTitle = $oGroupToDelete->title; @@ -379,7 +378,7 @@ public function loadModel($id) if ($model === null) { throw new CHttpException(404, 'The requested page does not exist.'); } - if (!Permission::model()->hasSurveysGroupsPermission($id, SurveysGroups::getMinimalPermissionRead())) { + if (!$model->hasPermission(SurveysGroups::getMinimalPermissionRead())) { throw new CHttpException(403, gT("You do not have permission to access this page.")); } return $model; diff --git a/application/models/Interfaces/PermissionInterface.php b/application/models/Interfaces/PermissionInterface.php new file mode 100644 index 00000000000..f9d58a3cc14 --- /dev/null +++ b/application/models/Interfaces/PermissionInterface.php @@ -0,0 +1,9 @@ +hasPermission(0, 'global', get_class($this), $sCRUD, $iUserID); - } } diff --git a/application/models/Permission.php b/application/models/Permission.php index be1f152028a..2fd12aca128 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -90,6 +90,7 @@ public static function getSurveyBasePermissions() */ public static function getEntityBasePermissions($sEntityName) { + /* @todo : check $sEntityName implement PermissionInterface */ $defaults = array( 'create' => true, 'read' => true, @@ -114,6 +115,7 @@ public static function getEntityBasePermissions($sEntityName) */ public static function getEntityMinimalPermissionRead($sEntityName) { + /* @todo : check $sEntityName implement PermissionInterface */ return $sEntityName::getMinimalPermissionRead(); } @@ -553,7 +555,7 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = ' /* Always return true if you are the owner : this can be done in core plugin ? */ // TODO: give the rights to owner adding line in permissions table, so it will return true with the normal way - if ($iUserID == $this->getEntityOwnerId($iEntityID, $sEntityName) && $sEntityName != 'role') { + if ($iUserID == $this->getEntityOwnerId($iEntityID, $sEntityName)) { return true; } @@ -581,16 +583,19 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD = ' return true; } - /* Find the roles the user is part of and return thoese permissions */ - /* Ignore roles for surveypermissions */ + /* Find the roles the user is part of and return those permissions */ + /* roles are only for global permission */ // @TODO add surveypermission to roles - $aRoles = self::getUserRole($iUserID); - if (safecount($aRoles) > 0 && $sEntityName != 'survey') { - $allowed = false; - foreach ($aRoles as $role) { - $allowed = $allowed || $this->hasRolePermission($role['ptid'], $sPermission, substr($sCRUD, 0, -2)); + if($sEntityName == 'global') { + $aRoles = self::getUserRole($iUserID); + if (safecount($aRoles) > 0 ) { + $allowed = false; + foreach ($aRoles as $role) { + $allowed = $allowed || $this->hasRolePermission($role['ptid'], $sPermission, substr($sCRUD, 0, -2)); + } + /* Can return false ? Even if user have the specific right … */ + return $allowed; } - return $allowed; } /* Check in permission DB and static it */ @@ -648,46 +653,7 @@ public function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD = 'read', $ if (!$oSurvey) { return false; } - // Get global correspondance for surveys rigth - $sGlobalCRUD = $sCRUD; - if (($sCRUD == 'create' || $sCRUD == 'import')) { // Create and import (token, reponse , question content …) need only allow update surveys - $sGlobalCRUD = 'update'; - } - if (($sCRUD == 'delete' && $sPermission != 'survey')) { // Delete (token, reponse , question content …) need only allow update surveys - $sGlobalCRUD = 'update'; - } - return $this->hasGlobalPermission('surveys', $sGlobalCRUD, $iUserID) - || $this->hasSurveysInGroupPermission($oSurvey->gsid, 'surveys', $sGlobalCRUD, $iUserID) - || $this->hasPermission($iSurveyID, 'survey', $sPermission, $sCRUD, $iUserID); - } - - /** - * Checks if a user has a certain permission in the given survey group - * @deprecated (can be remove before next RC) - * @see SurveysGroups->getPermission - * @param integer $iSurveyGroupId The survey group ID - * @param string $sPermission Name of the permission - * @param string $sCRUD The permission detail you want to check on: 'create','read','update','delete','import' or 'export' - * @param integer $iUserID User ID - if not given the one of the current user is used - * @return bool True if user has the permission - */ - public function hasSurveysGroupsPermission($iSurveyGroupId, $sPermission, $sCRUD = 'read', $iUserID = null) - { - return SurveysGroups::model()->findByPk($iSurveyGroupId)->getCurrentPermission($sPermission, $sCRUD, $iUserID); - } - - /** - * Checks if a user has a certain permission in the given surveys inside survey group - * - * @param integer $iSurveyGroupId The survey group ID - * @param string $sPermission Name of the permission - * @param string $sCRUD The permission detail you want to check on: 'create','read','update','delete','import' or 'export' - * @param integer $iUserID User ID - if not given the one of the current user is used - * @return bool True if user has the permission - */ - public function hasSurveysInGroupPermission($iSurveyGroupId, $sPermission, $sCRUD = 'read', $iUserID = null) - { - return SurveysInGroup::model()->findByPk($iSurveyGroupId)->getCurrentPermission($sPermission, $sCRUD, $iUserID); + return $oSurvey->hasPermission($iSurveyID, $sPermission, $sCRUD , $iUserID); } /** @@ -762,17 +728,18 @@ public static function getUserRole($iUserID) protected function getEntityOwnerId($iEntityID, $sEntityName) { /* know invalid entity */ - if (in_array($sEntityName, array('global','template'))) { + if (in_array($sEntityName, array('global', 'template', 'role'))) { return null; } /* allow to get it dynamically from any model */ $oEntity = $this->getEntity($sEntityName, $iEntityID); - // TODO: Check HasOwnershipInterface instead? - if (method_exists($sEntityName, 'model') && $oEntity) { - // Or check if $sEntityName is a child of LSActiveRecord ? - return $oEntity->getOwnerId(); + if (empty($oEntity)) { + return null; + } + if(!method_exists($oEntity,'getOwnerId')) { + return null; } - return null; + return $oEntity->getOwnerId(); } /** @@ -893,30 +860,6 @@ public static function editThemePermissionsUser($userId, $aTemplatePermissions){ return $results; } - /** - * Get SurveysInGroup with id $iSurveysInGroupId - * NB: This method needs to be public so that it can be mocked. - * - * @param int $iSurveysInGroupId - * @return SurveysInGroup|null - */ - public function getSurveysInGroup($iSurveysInGroupId) - { - return SurveysInGroup::model()->findByPk($iSurveysInGroupId); - } - - /** - * Get SurveysGroup with id $iSurveyGroupId - * NB: This method needs to be public so that it can be mocked. - * - * @param int $iSurveyGroupId - * @return SurveysGroups|null - */ - public function getSurveysGroups($iSurveyGroupId) - { - return SurveysGroups::model()->findByPk($iSurveyGroupId); - } - /** * Get object with $iEntityID of type $sEntityName * NB: This method needs to be public so that it can be mocked. diff --git a/application/models/Survey.php b/application/models/Survey.php index fdf2357ed74..73ad790fd92 100755 --- a/application/models/Survey.php +++ b/application/models/Survey.php @@ -15,7 +15,6 @@ * See COPYRIGHT.php for copyright notices and details. * */ - use \LimeSurvey\PluginManager\PluginEvent; /** @@ -152,8 +151,10 @@ * @property boolean $isDateExpired Whether survey is expired depending on the current time and survey configuration status * @method mixed active() */ -class Survey extends LSActiveRecord +class Survey extends LSActiveRecord implements PermissionInterface { + use PermissionTrait; + /** * This is a static cache, it lasts only during the active request. If you ever need * to clear it, like on activation of a survey when in the same request a row is read, @@ -1637,7 +1638,9 @@ public function search() /** * Get criteria from Permission - * @param $userid for thius user id , if not set : get current one + * @param $userid for thius user id , if not set : get current one + * @todo : move to PermissionInterface + * @todo : create an event * @return CDbCriteria */ protected static function getPermissionCriteria( $userid = null) @@ -2238,6 +2241,29 @@ public static function getPermissionData($key = null) return $key == null ? $aPermission : $aPermission[$key]; } + /** + * @inheritdoc + */ + public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) + { + $sGlobalCRUD = $sCRUD; + if (($sCRUD == 'create' || $sCRUD == 'import')) { // Create and import (token, reponse , question content …) need only allow update surveys + $sGlobalCRUD = 'update'; + } + if (($sCRUD == 'delete' && $sPermission != 'survey')) { // Delete (token, reponse , question content …) need only allow update surveys + $sGlobalCRUD = 'update'; + } + /* Global */ + if (Permission::model()->hasPermission(0, 'global', 'surveys', $sCRUD, $iUserID)) { + return true; + } + /* Inherited by SurveysInGroup */ + if(SurveysInGroup::model()->findByPk($this->gsid)->hasPermission($sPermission, $sCRUD, $iUserID)) { + return true; + } + return Permission::model()->hasPermission($this->getPrimaryKey(), 'survey', $sPermission, $sCRUD, $iUserID); + } + /* * Find all public surveys * @return Survey[] diff --git a/application/models/SurveysGroups.php b/application/models/SurveysGroups.php index 25b3d1fdc0e..97b98e38fc4 100644 --- a/application/models/SurveysGroups.php +++ b/application/models/SurveysGroups.php @@ -1,5 +1,4 @@ hasSurveysGroupsPermission($this->gsid, 'group', 'read'); + return $this->hasPermission('group', 'read'); } /** @@ -296,17 +296,17 @@ public function getButtons() $sSurveySettingsUrl = App()->createUrl("admin/surveysgroups/sa/surveysettings", array("id"=>$this->gsid)); $sPermissionUrl = App()->createUrl("surveysGroupsPermission/index", array("id"=>$this->gsid)); $button = ''; - if(Permission::model()->hasSurveysGroupsPermission($this->gsid,'group','read')) { + if($this->hasPermission('group','read')) { $button .= ''.gT('Edit survey group').''; } - if(Permission::model()->hasSurveysGroupsPermission($this->gsid,'surveysettings','read')) { + if($this->hasPermission('surveysettings','read')) { $button .= ''.gT('Survey settings').''; } - if (Permission::model()->hasSurveysGroupsPermission($this->gsid,'permission','read')) { + if ($this->hasPermission('permission','read')) { $button .= ''.gT('Permission').''; } - /* Can not delete group #1 + with survey */ - if ($this->gsid!=1 && !$this->hasSurveys && Permission::model()->hasSurveysGroupsPermission($this->gsid,'group','delete')) { + /* Can not delete group #1 + with survey (or move it to hasPermission function ?) */ + if ($this->gsid!=1 && !$this->hasSurveys && $this->hasPermission('group','delete')) { $button .= ''.gT('Delete survey group').''; } @@ -485,7 +485,7 @@ public function getOwnerId() /** * @inheritdoc */ - public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null) + public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) { /* If have global : return true */ if (Permission::model()->hasPermission(0, 'global', 'surveysgroups', $sCRUD, $iUserID)) { diff --git a/application/models/SurveysInGroup.php b/application/models/SurveysInGroup.php index 14e73189307..fb64659469d 100644 --- a/application/models/SurveysInGroup.php +++ b/application/models/SurveysInGroup.php @@ -5,8 +5,9 @@ * Used for Permission on survey inside group : * */ -class SurveysInGroup extends SurveysGroups +class SurveysInGroup extends SurveysGroups implements PermissionInterface { + use PermissionTrait; /** * Returns the static model of the specified AR class. * Please note that you should have this exact method in all your CActiveRecord descendants! @@ -74,7 +75,7 @@ public static function getMinimalPermissionRead() /** * @inheritdoc */ - public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null) + public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) { /* If have global for surveys : return true */ $sGlobalCRUD = $sCRUD; @@ -84,6 +85,7 @@ public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = n if (($sCRUD == 'delete' && $sPermission != 'survey')) { // Delete (token, reponse , question content …) need only allow update surveys $sGlobalCRUD = 'update'; } + /* Have surveys permission */ if (Permission::model()->hasPermission(0, 'global', 'surveys', $sGlobalCRUD, $iUserID)) { return true; } diff --git a/application/models/TemplateConfiguration.php b/application/models/TemplateConfiguration.php index 151a9bbffa9..aaf46ec3083 100755 --- a/application/models/TemplateConfiguration.php +++ b/application/models/TemplateConfiguration.php @@ -699,16 +699,20 @@ public function getTypeIcon() */ public function getButtons() { - - $gsid = App()->request->getQuery('id', null); + /* What ? We can get but $this->getAttribute ??? */ + $gsid = App()->request->getQuery('id', null); // $this->gsid; // don't show any buttons if user doesn't have update permission if(!Permission::model()->hasGlobalPermission('templates', 'update')) { /* Global settings */ if (empty($gsid) || App()->getController()->action->id != "surveysgroups") { return ''; } - /* SureysGroups settings */ - if (!Permission::model()->hasSurveysInGroupPermission($gsid, 'surveys', 'update')) { + /* SurveysGroups settings */ + $oSurveysGroups = SurveysGroups::model()->findByPk($gsid); + if(empty($oSurveysGroups)) { + return ''; + } + if (!$oSurveysGroups->hasPermission('surveys', 'update')) { return ''; } } diff --git a/application/models/Traits/PermissionTrait.php b/application/models/Traits/PermissionTrait.php new file mode 100644 index 00000000000..4e5ac6271b9 --- /dev/null +++ b/application/models/Traits/PermissionTrait.php @@ -0,0 +1,56 @@ +hasGlobalPermission('superadmin', $sCRUD, $iUserID)) { + return true; + } + /* No default global : adding it ? */ + return false; + } +} diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 5961429488b..1714a48b100 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -11,7 +11,7 @@ use LSHttpRequest; use LSWebUser; use Permission; -use LSActiveRecord; +use PermissionInterface; use App; use CHtml; @@ -21,14 +21,13 @@ */ class PermissionManager { - /** @var LSHttpRequest */ private $request; /** @var LSWebUser */ private $user; - /** @var LSActiveRecord model where permission is checked*/ + /** @var PermissionInterface model where permission is checked*/ private $model; /** @@ -38,7 +37,7 @@ class PermissionManager public function __construct( LSHttpRequest $request, LSWebUser $user, - LSActiveRecord $model + PermissionInterface $model ) { $this->request = $request; $this->user = $user; @@ -64,7 +63,7 @@ public function getPermissionData($userId = null) $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ - 'disabled' => !$this->model->getCurrentPermission($sPermission, $crud, $this->user->id), + 'disabled' => !$this->getCurrentPermission($sPermission, $crud, $this->user->id), 'indeterminate' => false ); } @@ -83,7 +82,7 @@ public function getPermissionData($userId = null) $aObjectPermissions[$sPermission]['current'][$crud]['checked'] = $havePermissionSet; /* The user didn't have the permission set, but have permission by other way (inherited, plugin …) */ if(!$havePermissionSet) { - $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->model->getCurrentPermission($sPermission, $crud, $userId); + $aObjectPermissions[$sPermission]['current'][$crud]['indeterminate'] = $this->getCurrentPermission($sPermission, $crud, $userId); } } } @@ -120,7 +119,7 @@ public function setPermissions($userId) $aSetPermissions[$sPermission] = array(); foreach ($aCruds as $crud) { /* Only set value if current user have the permission to set */ - if($this->model->getCurrentPermission($sPermission, $crud, $this->user->id)) { + if($this->getCurrentPermission($sPermission, $crud, $this->user->id)) { $aSetPermissions[$sPermission][$crud] = !empty($aPermission[$crud]) && !empty($entityPermissionsToSet[$sPermission][$crud]); } } @@ -139,12 +138,20 @@ public function setPermissions($userId) App()->getPluginManager()->dispatchEvent($oEvent); foreach($aSetPermissions as $sPermission => $aSetPermission) { - $oCurrentPermission = $this->getOrCreateDbPermission( + $oCurrentPermission = $this->getDbPermission( get_class($this->model), $this->model->getPrimaryKey(), $userId, $sPermission ); + if (empty($oCurrentPermission)) { + $oCurrentPermission = $this->setDbPermission( + get_class($this->model), + $this->model->getPrimaryKey(), + $userId, + $sPermission + ); + } /* Set only the permission set in $aSetPermission : user have the rights */ foreach($aSetPermission as $crud => $permission) { $oCurrentPermission->setAttribute("{$crud}_p",intval($permission)); @@ -159,16 +166,25 @@ public function setPermissions($userId) } /** - * Get DB permission, create if needed + * get the current permission * To be mocked for test - * @return \Permission + * @return boolean */ - public function getOrCreateDbPermission($entityName, $entityId, $userId, $sPermission) + public function getCurrentPermission($sPermission, $crud, $userId) { - $oPermission = $this->getDbPermission($entityName, $entityId, $userId, $sPermission); - if ($oPermission) { - return $oPermission; + if(empty($this->model)) { + return false; } + return $this->model->hasPermission($sPermission, $crud, $userId); + } + + /** + * Set a new DB permission + * To be mocked for test + * @return \Permission + */ + public function setDbPermission($entityName, $entityId, $userId, $sPermission) + { $oPermission = new Permission; $oPermission->entity = $entityName; $oPermission->entity_id = $entityId; @@ -176,6 +192,7 @@ public function getOrCreateDbPermission($entityName, $entityId, $userId, $sPermi $oPermission->permission = $sPermission; return $oPermission; } + /** * Get DB permission * To be mocked for test diff --git a/application/views/SurveysGroupsPermission/subviews/currentUsersList.php b/application/views/SurveysGroupsPermission/subviews/currentUsersList.php index e8da49f2fa6..d721b3b78ca 100644 --- a/application/views/SurveysGroupsPermission/subviews/currentUsersList.php +++ b/application/views/SurveysGroupsPermission/subviews/currentUsersList.php @@ -15,13 +15,13 @@ - hasSurveysGroupsPermission($model->gsid, 'permission', 'update')) : ?> + hasPermission('permission', 'update')) : ?> $model->gsid, 'to' => $oUser->uid));?>"> - hasSurveysGroupsPermission($model->gsid, 'permission', 'delete')) : ?> + hasPermission('permission', 'delete')) : ?> createUrl("surveysGroupsPermission/deleteUser", array( 'id'=>$model->gsid, 'uid' => $oUser->uid diff --git a/application/views/SurveysGroupsPermission/subviews/viewCurrents.php b/application/views/SurveysGroupsPermission/subviews/viewCurrents.php index 13f8178a9a2..bc4b7bd81a3 100644 --- a/application/views/SurveysGroupsPermission/subviews/viewCurrents.php +++ b/application/views/SurveysGroupsPermission/subviews/viewCurrents.php @@ -10,7 +10,7 @@ 'aCurrentsUserRights' => $aCurrentsUserRights, )); } ?> - hasSurveysGroupsPermission($model->primaryKey, 'permission', 'create')) : ?> + hasPermission('permission', 'create')) : ?>

renderPartial('/SurveysGroupsPermission/subviews/addUserForm',array('model'=>$model,'oAddUserList'=>$oAddUserList)); diff --git a/application/views/admin/surveysgroups/surveySettings.php b/application/views/admin/surveysgroups/surveySettings.php index 0fbd6f6be88..831b08a816c 100644 --- a/application/views/admin/surveysgroups/surveySettings.php +++ b/application/views/admin/surveysgroups/surveySettings.php @@ -33,7 +33,7 @@
- hasSurveysGroupsPermission($model->gsid, 'surveysettings', 'update')) : ?> + hasPermission('surveysettings', 'update')) : ?>
- hasSurveysGroupsPermission($model->primaryKey, 'group','read')):?> + hasPermission('group','read')):?>
renderPartial('./surveysgroups/_form', $_data_); ?>
diff --git a/tests/unit/models/PermissionTest.php b/tests/unit/models/PermissionTest.php index e192c596577..00547d7243e 100644 --- a/tests/unit/models/PermissionTest.php +++ b/tests/unit/models/PermissionTest.php @@ -1,5 +1,6 @@ getMockBuilder(SurveysGroups::class) ->setMethods(['save', 'attributes']) @@ -28,13 +30,12 @@ public function testSuperAdmin() $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId', 'getSurveysGroups']) + ->setMethods(['getUserId']) ->getMock(); $perm->method('getUserId')->willReturn(1); - $perm->method('getSurveysGroups')->willReturn($surveysGroup); $surveysGroupsId = 999; - $this->assertTrue($perm->hasSurveysGroupsPermission($surveysGroupsId, 'permission', 'create')); + $this->assertTrue($surveysGroup->hasPermission('permission', 'create')); } /** @@ -42,6 +43,7 @@ public function testSuperAdmin() */ public function testOwnershipSuccess() { + return; // NB: Not 1 (superadmin). $userId = 2; $surveysGroup = $this @@ -56,14 +58,13 @@ public function testOwnershipSuccess() $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId', 'getSurveysGroups', 'getEntity']) + ->setMethods(['getUserId', 'getEntity']) ->getMock(); $perm->method('getUserId')->willReturn($userId); - $perm->method('getSurveysGroups')->willReturn($surveysGroup); $perm->method('getEntity')->willReturn($surveysGroup); $surveysGroupsId = 999; - $this->assertTrue($perm->hasSurveysGroupsPermission($surveysGroupsId, 'permission', 'create')); + $this->assertTrue($surveysGroup->hasPermission('permission', 'create')); } /** @@ -71,6 +72,7 @@ public function testOwnershipSuccess() */ public function testOwnershipFailure() { + return; // NB: Not 1 (superadmin). $userId = 2; $surveysGroup = $this @@ -85,13 +87,12 @@ public function testOwnershipFailure() $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId', 'getSurveysGroups', 'getEntity']) + ->setMethods(['getUserId', 'getEntity']) ->getMock(); $perm->method('getUserId')->willReturn($userId); - $perm->method('getSurveysGroups')->willReturn($surveysGroup); $perm->method('getEntity')->willReturn($surveysGroup); $surveysGroupsId = 999; - $this->assertFalse($perm->hasSurveysGroupsPermission($surveysGroupsId, 'permission', 'create')); + $this->assertFalse($surveysGroup->hasPermission('permission', 'create')); } } From 946591f9001eb4b39cc2352021cb623e5ea9c0a3 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 13:15:34 +0100 Subject: [PATCH 11/22] Dev: fix hasSurveyPermission --- application/models/Permission.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/models/Permission.php b/application/models/Permission.php index 2fd12aca128..0219c62e873 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -653,7 +653,7 @@ public function hasSurveyPermission($iSurveyID, $sPermission, $sCRUD = 'read', $ if (!$oSurvey) { return false; } - return $oSurvey->hasPermission($iSurveyID, $sPermission, $sCRUD , $iUserID); + return $oSurvey->hasPermission($sPermission, $sCRUD , $iUserID); } /** From 35c71175055ea966b2f4578a6658a457a7a538f6 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 15:23:17 +0100 Subject: [PATCH 12/22] Dev: remove some unusued code Dev: add some comment for scrut --- .../controllers/SurveysGroupsPermissionController.php | 2 +- application/models/Interfaces/PermissionInterface.php | 4 ++-- application/models/Survey.php | 11 +++++------ application/models/SurveysGroups.php | 11 ++--------- application/models/SurveysInGroup.php | 8 +------- application/models/Traits/PermissionTrait.php | 6 +++--- 6 files changed, 14 insertions(+), 28 deletions(-) diff --git a/application/controllers/SurveysGroupsPermissionController.php b/application/controllers/SurveysGroupsPermissionController.php index 2d88f2682be..36430cba429 100644 --- a/application/controllers/SurveysGroupsPermissionController.php +++ b/application/controllers/SurveysGroupsPermissionController.php @@ -448,7 +448,7 @@ private function viewUserOrUserGroup($id, $to, $type = 'user') $PermissionManagerService = new PermissionManager( $request, $user, - SurveysInGroup::model()->findByPk($id) + /** @scrutinizer ignore-type : we alreadty check SurveysGroup then we have it*/ SurveysInGroup::model()->findByPk($id) ); $aSurveysInGroupPermissions = $PermissionManagerService->getPermissionData($userId); $aPermissions = array_merge( diff --git a/application/models/Interfaces/PermissionInterface.php b/application/models/Interfaces/PermissionInterface.php index f9d58a3cc14..6dae3d2e75f 100644 --- a/application/models/Interfaces/PermissionInterface.php +++ b/application/models/Interfaces/PermissionInterface.php @@ -3,7 +3,7 @@ interface PermissionInterface { public function getOwnerId(); - public static function getPermissionData($key = null); + public static function getPermissionData(); public static function getMinimalPermissionRead(); - public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null); + public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null); } diff --git a/application/models/Survey.php b/application/models/Survey.php index 73ad790fd92..c0f64916240 100755 --- a/application/models/Survey.php +++ b/application/models/Survey.php @@ -2138,11 +2138,10 @@ public static function getMinimalPermissionRead() } /** - * Get Permission data for Permission object - * @param string $key + * Get Permission data for Survey * @return array */ - public static function getPermissionData($key = null) + public static function getPermissionData() { $aPermission = array( 'assessments' => array( @@ -2238,7 +2237,7 @@ public static function getPermissionData($key = null) ), ); - return $key == null ? $aPermission : $aPermission[$key]; + return $aPermission; } /** @@ -2254,11 +2253,11 @@ public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) $sGlobalCRUD = 'update'; } /* Global */ - if (Permission::model()->hasPermission(0, 'global', 'surveys', $sCRUD, $iUserID)) { + if (Permission::model()->hasPermission(0, 'global', 'surveys', $sGlobalCRUD, $iUserID)) { return true; } /* Inherited by SurveysInGroup */ - if(SurveysInGroup::model()->findByPk($this->gsid)->hasPermission($sPermission, $sCRUD, $iUserID)) { + if(SurveysInGroup::model()->findByPk($this->gsid)->hasPermission($sPermission, $sGlobalCRUD, $iUserID)) { return true; } return Permission::model()->hasPermission($this->getPrimaryKey(), 'survey', $sPermission, $sCRUD, $iUserID); diff --git a/application/models/SurveysGroups.php b/application/models/SurveysGroups.php index 97b98e38fc4..96bec74dba2 100644 --- a/application/models/SurveysGroups.php +++ b/application/models/SurveysGroups.php @@ -414,11 +414,10 @@ protected static function getPermissionCriteria() } /** - * Get Permission data for Permission object - * @param string $key + * Get Permission data for SurveysGroup * @return array */ - public static function getPermissionData($key = null) + public static function getPermissionData() { $aPermission = array( 'group' => array( @@ -455,12 +454,6 @@ public static function getPermissionData($key = null) 'img' => ' fa fa-shield', ), ); - if ($key) { - if(isset($aPermission[$key])) { - return $aPermission[$key]; - } - return null; - } return $aPermission; } diff --git a/application/models/SurveysInGroup.php b/application/models/SurveysInGroup.php index fb64659469d..96053dbb1c6 100644 --- a/application/models/SurveysInGroup.php +++ b/application/models/SurveysInGroup.php @@ -26,7 +26,7 @@ public static function model($className = __CLASS__) * @param string $key * @return array */ - public static function getPermissionData($key = null) + public static function getPermissionData() { $aPermission = array( 'surveys' => array( @@ -41,12 +41,6 @@ public static function getPermissionData($key = null) 'img' => ' fa fa-edit', ), ); - if ($key) { - if(isset($aPermission[$key])) { - return $aPermission[$key]; - } - return null; - } return $aPermission; } diff --git a/application/models/Traits/PermissionTrait.php b/application/models/Traits/PermissionTrait.php index 4e5ac6271b9..6fc59f90b14 100644 --- a/application/models/Traits/PermissionTrait.php +++ b/application/models/Traits/PermissionTrait.php @@ -18,7 +18,7 @@ public function getOwnerId() * @param string $key * @return array */ - public static function getPermissionData($key = null) + public static function getPermissionData() { return array(); } @@ -39,13 +39,13 @@ public static function getMinimalPermissionRead() * @param integer $iUserID User ID - if not given the one of the current user is used * @return boolean */ - public function getCurrentPermission($sPermission, $sCRUD = 'read', $iUserID = null) + public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) { if(empty($iUserID)) { if (Yii::app() instanceof CConsoleApplication) { return true; } - $iUserID = Permission::getUserId(); + $iUserID = \Permission::model()->getUserId(); } if(\Permission::model()->hasGlobalPermission('superadmin', $sCRUD, $iUserID)) { return true; From a7576c39d29ed6f32fe206b1d00e376a46ab3459 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 15:29:22 +0100 Subject: [PATCH 13/22] Dev: Add getPrimaryKey in PermissionTrait --- application/models/Interfaces/PermissionInterface.php | 1 + application/models/Permission.php | 2 +- application/models/Traits/PermissionTrait.php | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/application/models/Interfaces/PermissionInterface.php b/application/models/Interfaces/PermissionInterface.php index 6dae3d2e75f..7bb2334b9b1 100644 --- a/application/models/Interfaces/PermissionInterface.php +++ b/application/models/Interfaces/PermissionInterface.php @@ -6,4 +6,5 @@ public function getOwnerId(); public static function getPermissionData(); public static function getMinimalPermissionRead(); public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null); + public function getPrimaryKey(); } diff --git a/application/models/Permission.php b/application/models/Permission.php index 0219c62e873..41b52138293 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -699,7 +699,7 @@ private static function comparePermissionTitle($aApermission, $aBpermission) */ public function getUserId($iUserID = null) { - if (is_null($iUserID) && $iUserID !== 0) { + if (is_null($iUserID)) { if (Yii::app() instanceof CConsoleApplication) { throw new Exception('Permission must not be tested with console application.'); } diff --git a/application/models/Traits/PermissionTrait.php b/application/models/Traits/PermissionTrait.php index 6fc59f90b14..e12b29bf88d 100644 --- a/application/models/Traits/PermissionTrait.php +++ b/application/models/Traits/PermissionTrait.php @@ -53,4 +53,14 @@ public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) /* No default global : adding it ? */ return false; } + + /** + * Implemented in CActiveRecord + * @see CActiveRecord->getPrimaryKey + * @throw Exception + */ + public function getPrimaryKey() + { + throw new Exception('Not implemented'); + } } From 91db9d40f07a2f7b1827b24f107913344bf18917 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 16:44:56 +0100 Subject: [PATCH 14/22] Dev: scrutinizer --- application/models/Traits/PermissionTrait.php | 2 +- application/models/services/PermissionManager.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/application/models/Traits/PermissionTrait.php b/application/models/Traits/PermissionTrait.php index e12b29bf88d..d43a7270952 100644 --- a/application/models/Traits/PermissionTrait.php +++ b/application/models/Traits/PermissionTrait.php @@ -39,7 +39,7 @@ public static function getMinimalPermissionRead() * @param integer $iUserID User ID - if not given the one of the current user is used * @return boolean */ - public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null) + public function hasPermission(/** @scrutinizer ignore-unused */ $sPermission, $sCRUD = 'read', $iUserID = null) { if(empty($iUserID)) { if (Yii::app() instanceof CConsoleApplication) { diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 1714a48b100..176c3324928 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -68,7 +68,7 @@ public function getPermissionData($userId = null) ); } /* If user id is set : update the data with permission of this user */ - if($userId) { + if(!is_null($userId)) { $oCurrentPermission = $this->getDbPermission( get_class($this->model), $this->model->getPrimaryKey(), From 2449ce0158be870f93521a38067aba432d42d84d Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 17:43:09 +0100 Subject: [PATCH 15/22] Dev: reactivate current test --- tests/unit/models/PermissionTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit/models/PermissionTest.php b/tests/unit/models/PermissionTest.php index 00547d7243e..be268c46856 100644 --- a/tests/unit/models/PermissionTest.php +++ b/tests/unit/models/PermissionTest.php @@ -20,7 +20,6 @@ public static function setupBeforeClass() */ public function testSuperAdmin() { - return; $surveysGroup = $this ->getMockBuilder(SurveysGroups::class) ->setMethods(['save', 'attributes']) @@ -43,7 +42,6 @@ public function testSuperAdmin() */ public function testOwnershipSuccess() { - return; // NB: Not 1 (superadmin). $userId = 2; $surveysGroup = $this @@ -72,7 +70,6 @@ public function testOwnershipSuccess() */ public function testOwnershipFailure() { - return; // NB: Not 1 (superadmin). $userId = 2; $surveysGroup = $this From 331eb4c7f31fa9d2422dada8d22afa92e17f7160 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 8 Jan 2021 18:51:45 +0100 Subject: [PATCH 16/22] =?UTF-8?q?Dev=20=E2=80=A6=20silly=20=E2=80=A6=20i?= =?UTF-8?q?=20can't=20add=20getPrimaryKey=20in=20PermissionTrait?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/models/Interfaces/PermissionInterface.php | 1 - application/models/Traits/PermissionTrait.php | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/application/models/Interfaces/PermissionInterface.php b/application/models/Interfaces/PermissionInterface.php index 7bb2334b9b1..6dae3d2e75f 100644 --- a/application/models/Interfaces/PermissionInterface.php +++ b/application/models/Interfaces/PermissionInterface.php @@ -6,5 +6,4 @@ public function getOwnerId(); public static function getPermissionData(); public static function getMinimalPermissionRead(); public function hasPermission($sPermission, $sCRUD = 'read', $iUserID = null); - public function getPrimaryKey(); } diff --git a/application/models/Traits/PermissionTrait.php b/application/models/Traits/PermissionTrait.php index d43a7270952..c49d2adb3be 100644 --- a/application/models/Traits/PermissionTrait.php +++ b/application/models/Traits/PermissionTrait.php @@ -53,14 +53,4 @@ public function hasPermission(/** @scrutinizer ignore-unused */ $sPermission, $s /* No default global : adding it ? */ return false; } - - /** - * Implemented in CActiveRecord - * @see CActiveRecord->getPrimaryKey - * @throw Exception - */ - public function getPrimaryKey() - { - throw new Exception('Not implemented'); - } } From 8986870e3a71c9fc423a0c0d20fb3695dd02fae2 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Mon, 11 Jan 2021 16:45:47 +0100 Subject: [PATCH 17/22] Dev: review PermissionTest --- application/models/Traits/PermissionTrait.php | 3 --- tests/unit/models/PermissionTest.php | 10 ++++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/application/models/Traits/PermissionTrait.php b/application/models/Traits/PermissionTrait.php index c49d2adb3be..612a85213d8 100644 --- a/application/models/Traits/PermissionTrait.php +++ b/application/models/Traits/PermissionTrait.php @@ -42,9 +42,6 @@ public static function getMinimalPermissionRead() public function hasPermission(/** @scrutinizer ignore-unused */ $sPermission, $sCRUD = 'read', $iUserID = null) { if(empty($iUserID)) { - if (Yii::app() instanceof CConsoleApplication) { - return true; - } $iUserID = \Permission::model()->getUserId(); } if(\Permission::model()->hasGlobalPermission('superadmin', $sCRUD, $iUserID)) { diff --git a/tests/unit/models/PermissionTest.php b/tests/unit/models/PermissionTest.php index be268c46856..8c3f4405d10 100644 --- a/tests/unit/models/PermissionTest.php +++ b/tests/unit/models/PermissionTest.php @@ -50,8 +50,10 @@ public function testOwnershipSuccess() ->getMock(); $surveysGroup->method('save')->willReturn(true); $surveysGroup->method('attributes')->willReturn([ + 'gsid', 'owner_id' ]); + $surveysGroup->gsid = 999; $surveysGroup->owner_id = $userId; $perm = $this @@ -61,8 +63,7 @@ public function testOwnershipSuccess() $perm->method('getUserId')->willReturn($userId); $perm->method('getEntity')->willReturn($surveysGroup); - $surveysGroupsId = 999; - $this->assertTrue($surveysGroup->hasPermission('permission', 'create')); + $this->assertTrue($surveysGroup->hasPermission('permission', 'update')); } /** @@ -78,8 +79,10 @@ public function testOwnershipFailure() ->getMock(); $surveysGroup->method('save')->willReturn(true); $surveysGroup->method('attributes')->willReturn([ + 'gsid', 'owner_id' ]); + $surveysGroup->gsid = 999; $surveysGroup->owner_id = $userId + 1; $perm = $this @@ -89,7 +92,6 @@ public function testOwnershipFailure() $perm->method('getUserId')->willReturn($userId); $perm->method('getEntity')->willReturn($surveysGroup); - $surveysGroupsId = 999; - $this->assertFalse($surveysGroup->hasPermission('permission', 'create')); + $this->assertFalse($surveysGroup->hasPermission('permission', 'update')); } } From f2f4991ca32160a7ac3317b6c1914048e42d5b71 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Mon, 11 Jan 2021 18:15:59 +0100 Subject: [PATCH 18/22] Dev: test fix --- tests/unit/models/PermissionTest.php | 30 +++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/unit/models/PermissionTest.php b/tests/unit/models/PermissionTest.php index 8c3f4405d10..5ea3a8dd498 100644 --- a/tests/unit/models/PermissionTest.php +++ b/tests/unit/models/PermissionTest.php @@ -20,21 +20,29 @@ public static function setupBeforeClass() */ public function testSuperAdmin() { + $userId = 1; + $surveysGroupGid = 999; + $surveysGroup = $this ->getMockBuilder(SurveysGroups::class) ->setMethods(['save', 'attributes']) ->getMock(); $surveysGroup->method('save')->willReturn(true); - $surveysGroup->method('attributes')->willReturn([]); + $surveysGroup->method('attributes')->willReturn([ + 'gsid', + 'owner_id' + ]); + $surveysGroup->gsid = $surveysGroupGid; + $surveysGroup->owner_id = $userId + 1; $perm = $this ->getMockBuilder(Permission::class) - ->setMethods(['getUserId']) + ->setMethods(['getUserId', 'getEntity']) ->getMock(); - $perm->method('getUserId')->willReturn(1); + $perm->method('getUserId')->willReturn($userId); + $perm->method('getEntity')->willReturn($surveysGroup); - $surveysGroupsId = 999; - $this->assertTrue($surveysGroup->hasPermission('permission', 'create')); + $this->assertTrue($perm->hasPermission($surveysGroupGid, 'SurveysGroups', 'permission', 'create')); } /** @@ -44,6 +52,8 @@ public function testOwnershipSuccess() { // NB: Not 1 (superadmin). $userId = 2; + $surveysGroupGid = 999; + $surveysGroup = $this ->getMockBuilder(SurveysGroups::class) ->setMethods(['save', 'attributes']) @@ -53,7 +63,7 @@ public function testOwnershipSuccess() 'gsid', 'owner_id' ]); - $surveysGroup->gsid = 999; + $surveysGroup->gsid = $surveysGroupGid; $surveysGroup->owner_id = $userId; $perm = $this @@ -63,7 +73,7 @@ public function testOwnershipSuccess() $perm->method('getUserId')->willReturn($userId); $perm->method('getEntity')->willReturn($surveysGroup); - $this->assertTrue($surveysGroup->hasPermission('permission', 'update')); + $this->assertTrue($perm->hasPermission($surveysGroupGid, 'SurveysGroups', 'permission', 'create')); } /** @@ -73,6 +83,8 @@ public function testOwnershipFailure() { // NB: Not 1 (superadmin). $userId = 2; + $surveysGroupGid = 999; + $surveysGroup = $this ->getMockBuilder(SurveysGroups::class) ->setMethods(['save', 'attributes']) @@ -82,7 +94,7 @@ public function testOwnershipFailure() 'gsid', 'owner_id' ]); - $surveysGroup->gsid = 999; + $surveysGroup->gsid = $surveysGroupGid; $surveysGroup->owner_id = $userId + 1; $perm = $this @@ -92,6 +104,6 @@ public function testOwnershipFailure() $perm->method('getUserId')->willReturn($userId); $perm->method('getEntity')->willReturn($surveysGroup); - $this->assertFalse($surveysGroup->hasPermission('permission', 'update')); + $this->assertFalse($perm->hasPermission($surveysGroupGid, 'SurveysGroups', 'permission', 'create')); } } From d6c7b32f94817de61f946368751426d89f807bb0 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 12 Jan 2021 11:02:32 +0100 Subject: [PATCH 19/22] Dev: UserPermissionsWidget --- .../UserPermissionsWidget.php | 18 +++ .../UserPermissionsWidget/assets/script.js | 146 ++++++++++++++++++ .../UserPermissionsWidget/views/table.php | 48 ++++++ .../subviews/setPermissionForm.php | 52 +------ 4 files changed, 216 insertions(+), 48 deletions(-) create mode 100644 application/extensions/UserPermissionsWidget/UserPermissionsWidget.php create mode 100644 application/extensions/UserPermissionsWidget/assets/script.js create mode 100644 application/extensions/UserPermissionsWidget/views/table.php diff --git a/application/extensions/UserPermissionsWidget/UserPermissionsWidget.php b/application/extensions/UserPermissionsWidget/UserPermissionsWidget.php new file mode 100644 index 00000000000..7924a9cf483 --- /dev/null +++ b/application/extensions/UserPermissionsWidget/UserPermissionsWidget.php @@ -0,0 +1,18 @@ +aPermissions)) { + return; + } + $this->render('table', ['aPermissions' => $this->aPermissions]); + } +} diff --git a/application/extensions/UserPermissionsWidget/assets/script.js b/application/extensions/UserPermissionsWidget/assets/script.js new file mode 100644 index 00000000000..3bc65dc5997 --- /dev/null +++ b/application/extensions/UserPermissionsWidget/assets/script.js @@ -0,0 +1,146 @@ +var LS = LS || { + onDocumentReady: {} +}; + +$(document).on('ready pjax:scriptcomplete', function(){ + $(':checkbox:not(:checked)[data-indeterminate=1]').prop('indeterminate', true) + + $(".surveysecurity").tablesorter({ + widgets: ['zebra'], + sortList: [[2,0]], + headers: { 19: { sorter: false} } + }); + + + $(".table-permissions-set").tablesorter({ + widgets: ['zebra'], + headers: { 0: { sorter: false}, + 2: { sorter: false}, + 3: { sorter: false}, + 4: { sorter: false}, + 5: { sorter: false}, + 6: { sorter: false}, + 7: { sorter: false}, + 8: { sorter: false} + } + }); + + $(".markrow").click( + function(){ + $(this).removeClass('mixed'); + $(this).closest('tr').find('input').prop('checked',$(this).prop('checked')).prop('indeterminate',false); + updateAllCheckboxes(); + } + ) + + // mark all checkboxes + $(".markall").click( + function(){ + $(this).removeClass('mixed'); + var checked = $(this).prop('checked'); + $('.table-permissions-set tbody tr').each(function(){ + var rowSelector = $(this).find('input'); + $(rowSelector).prop('checked',checked).prop('indeterminate',false); + }); + } + ) + + $('.extended input').click( + function(){ + if ($(this).closest('tr').find('.extended input:checked').size()==$(this).closest('tr').find('.extended input').size()) + { + $(this).closest('tr').find('.markrow').prop('checked',true).removeClass('mixed'); + } + else if ($(this).closest('tr').find('.extended input:checked').size()==0) + { + $(this).closest('tr').find('.markrow').prop('checked',false).removeClass('mixed'); + } + else + { + $(this).closest('tr').find('.markrow').prop('checked',true).addClass('mixed'); + } + updateAllCheckboxes(); + } + ) + + if (Cookies.get('surveysecurityas')!='true') + { + $('.table-permissions-set .extended').hide(); + } + /* Show on the all comuln the state of included checkbox */ + $('.table-permissions-set tbody tr').each(function(){ + if ($(this).find('.extended input:checkbox:checked').length == $(this).find('.extended input:checkbox').length) { + /* All is checked */ + $(this).find('.markrow').prop('checked',true).removeClass('mixed'); + } else if (!$(this).find('.extended input:checkbox:checked').length) { + /* None is checked */ + if ($(this).find('.extended input:checkbox[data-indeterminate="1"]').length == $(this).find('.extended input:checkbox').length) { + $(this).find('.markrow').prop('indeterminate',true).removeClass('mixed'); + } else if(!$(this).find('.extended input:checkbox[data-indeterminate="1"]').length) { + $(this).find('.markrow').prop('indeterminate',false).removeClass('mixed'); + } else { + $(this).find('.markrow').prop('indeterminate',true).addClass('mixed'); + } + + } else { + /* Partially checked */ + $(this).find('.markrow').prop('checked',true).addClass('mixed'); + } + }) + + $('#btnToggleAdvanced').click(function(){ + extendoptionsvisible=$('.table-permissions-set .extended').is(':visible'); + if (extendoptionsvisible==false) + { + $('.table-permissions-set .extended').fadeIn('slow'); + } + else + { + $('.table-permissions-set .extended').fadeOut(); + } + updateExtendedButton(!extendoptionsvisible); + Cookies.set('surveysecurityas',!extendoptionsvisible); + }); + updateExtendedButton(true); + + updateAllCheckboxes(); +}); + +function updateExtendedButton(bVisible) +{ + if (bVisible==true) + { + $('#btnToggleAdvanced').val('<<'); + } + else + { + $('#btnToggleAdvanced').val('>>'); + } + +} + +function updateAllCheckboxes(){ + var iFullCheckedRows = 0; + var iHalfCheckedRows = 0; + var iNoCheckedRows = 0; + $('.table-permissions-set tbody tr').each(function(){ + var rowSelector = $(this).find('.markrow'); + if (rowSelector.prop('checked') === true && !rowSelector.hasClass('mixed')){ + iFullCheckedRows += 1; + } else if (rowSelector.prop('checked') === true && rowSelector.hasClass('mixed')){ + iHalfCheckedRows += 1; + } else if (rowSelector.prop('checked') === false){ + iNoCheckedRows += 1; + } + }); + + var markAllSelector = $('.table-permissions-set thead tr').find('.markall'); + + if (iFullCheckedRows > 0 && iHalfCheckedRows == 0 && iNoCheckedRows == 0){ + markAllSelector.prop('checked',true).removeClass('mixed'); + } else if (iFullCheckedRows > 0 || iHalfCheckedRows > 0){ + markAllSelector.prop('checked',true).addClass('mixed'); + } else { + markAllSelector.prop('checked',false).removeClass('mixed'); + } +} diff --git a/application/extensions/UserPermissionsWidget/views/table.php b/application/extensions/UserPermissionsWidget/views/table.php new file mode 100644 index 00000000000..b80ae81d847 --- /dev/null +++ b/application/extensions/UserPermissionsWidget/views/table.php @@ -0,0 +1,48 @@ + + + + + + + + + + + + + + + $aCurrentPermissions): ?> + + + + + $aValues): ?> + + + + +
+ + + + +
'markrow')) ?> 1, + 'data-indeterminate' => $aValues['indeterminate'], + 'id' => CHtml::getIdByName("set[{$aCurrentPermissions['entity']}][{$sPermission}][$sKey]"), + 'uncheckValue' => 0, + /* See issue #14551 : https://bugs.limesurvey.org/view.php?id=14551 */ + 'disabled' => $aValues['disabled'], + ) + ); + }?> +
+getClientScript()->registerPackage('jquery-tablesorter'); + App()->getClientScript()->registerScriptFile(App()->getAssetManager()->publish(Yii::getPathOfAlias('ext.UserPermissionsWidget.assets'). '/script.js')); +?> diff --git a/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php b/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php index a7d36382266..ab01ed3946a 100644 --- a/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php +++ b/application/views/SurveysGroupsPermission/subviews/setPermissionForm.php @@ -1,7 +1,3 @@ -getClientScript()->registerPackage('jquery-tablesorter'); - App()->getClientScript()->registerScriptFile(App()->getConfig('adminscripts').'surveypermissions.js'); -?> $model->gsid), @@ -13,50 +9,10 @@ }else { printf(gT("Set permission for user group : %s"),"".CHtml::encode($oUserGroup->name).""); } ?> - - - - - - - - - - - - - - - $aCurrentPermissions): ?> - - - - - $aValues): ?> - - - - -
- - - - -
'markrow')) ?> 1, - 'data-indeterminate' => $aValues['indeterminate'], - 'id' => CHtml::getIdByName("set[{$aCurrentPermissions['entity']}][{$sPermission}][$sKey]"), - 'uncheckValue' => 0, - /* See issue #14551 : https://bugs.limesurvey.org/view.php?id=14551 */ - 'disabled' => $aValues['disabled'], - ) - ); - }?> -
+ widget( + 'ext.UserPermissionsWidget.UserPermissionsWidget', + ['aPermissions' => $aPermissions] + ); ?> Date: Tue, 12 Jan 2021 12:10:26 +0100 Subject: [PATCH 20/22] =?UTF-8?q?Fixed=20issue=20#14551:=20user=20can=20gr?= =?UTF-8?q?ant=20more=20permissions=20on=20a=20survey=20than=20he=20has=20?= =?UTF-8?q?himself=20Dev:=20after=20create=20services=20+=20widget=20?= =?UTF-8?q?=E2=80=A6=20use=20it=20for=20Survey?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../controllers/admin/surveypermission.php | 267 ++++++++---------- .../models/services/PermissionManager.php | 4 + 2 files changed, 118 insertions(+), 153 deletions(-) diff --git a/application/controllers/admin/surveypermission.php b/application/controllers/admin/surveypermission.php index daa7d4c95e9..c0660ee0248 100644 --- a/application/controllers/admin/surveypermission.php +++ b/application/controllers/admin/surveypermission.php @@ -20,6 +20,8 @@ * @copyright 2011 * @access public */ +use LimeSurvey\Models\Services\PermissionManager; + class surveypermission extends Survey_Common_Action { @@ -419,115 +421,77 @@ function adduser($surveyid) */ function set($surveyid) { - $aData['surveyid'] = $surveyid = sanitize_int($surveyid); $oSurvey = Survey::model()->findByPk($surveyid); + if (!$oSurvey->hasPermission('surveysecurity', 'update')) { + throw new CHttpException(403, gT("You do not have permission to access this page.")); + } + $aData['surveyid'] = $surveyid = sanitize_int($surveyid); $aViewUrls = array(); $action = App()->getRequest()->getParam('action'); - $imageurl = Yii::app()->getConfig('adminimageurl'); - $postuserid = App()->getRequest()->getParam('uid'); + $postuserid = App()->getRequest()->getParam('uid'); /* Allow to get it via GET and POST */ $postusergroupid = App()->getRequest()->getParam('ugid'); if ($action == "setsurveysecurity") { - if ((!Permission::model()->hasGlobalPermission('superadmin', 'read') && Yii::app()->user->getId() == $postuserid) // User can not change own security (except superadmin) - || !in_array($postuserid, getUserList('onlyuidarray')) // User can not set user security if it can not see it - ) { - $this->getController()->error('Access denied'); + if (!in_array($postuserid, getUserList('onlyuidarray'))) { + throw new CHttpException(403, gT("You do not have permission to this user.")); + } + if ($postuserid == App()->user->getId()) { + throw new CHttpException(403, gT("You can not set your own permission.")); } } elseif ($action == "setusergroupsurveysecurity") { - if (!Permission::model()->hasGlobalPermission('superadmin', 'read') && !in_array($postusergroupid, getUserGroupList())) { - // User can not change own security (except for superadmin ?) - $this->getController()->error('Access denied'); + if (!in_array($postusergroupid, getUserGroupList())) { + throw new CHttpException(403, gT("You do not have permission to this user group.")); } + $postuserid = null; } else { - Yii::app()->request->redirect(Yii::app()->getController()->createUrl('admin/surveypermission/sa/view', array('surveyid'=>$surveyid))); - //$this->getController()->error('Unknow action'); + throw new CHttpException(400, gT("Unknow action.")); } - if (Permission::model()->hasSurveyPermission($surveyid, 'surveysecurity', 'update')) { - $usersummary = "
"; - - App()->getClientScript()->registerPackage('jquery-tablesorter'); - App()->getClientScript()->registerScriptFile(App()->getConfig('adminscripts').'surveypermissions.js'); - if ($action == "setsurveysecurity") { - $query = "select users_name from {{users}} where uid=:uid"; - $resrow = Yii::app()->db->createCommand($query)->bindParam(":uid", $postuserid, PDO::PARAM_INT)->queryRow(); - $sUsername = $resrow['users_name']; - $usersummary .= "

".sprintf(gT("Edit survey permissions for user %s"), "".\CHtml::encode($sUsername)."")."

"; - } else { - $resrow = UserGroup::model()->find('ugid = :ugid', array(':ugid' => $postusergroupid)); - $sUsergroupName = $resrow['name']; - $usersummary .= "

".sprintf(gT("Edit survey permissions for group %s"), "".\CHtml::encode($sUsergroupName)."")."

"; - } - $usersummary .= '
'; - $usersummary .= "
" - .CHtml::form(array("admin/surveypermission/sa/surveyright/surveyid/{$surveyid}"), 'post') - . "\n"; - - $usersummary .= "" - . "\n" - . "\n" - . "\n" - . "\n" - . "\n" - . "\n" - . "\n" - . "\n" - . "\n"; - - //content - - $aBasePermissions = Permission::model()->getSurveyBasePermissions(); - - $oddcolumn = false; - foreach ($aBasePermissions as $sPermissionKey=>$aCRUDPermissions) { - $oddcolumn = !$oddcolumn; - $usersummary .= ""; - $usersummary .= ""; - $usersummary .= ""; - foreach ($aCRUDPermissions as $sCRUDKey=>$CRUDValue) { - if (!in_array($sCRUDKey, array('create', 'read', 'update', 'delete', 'import', 'export'))) { - continue; - } - $usersummary .= ""; - } - $usersummary .= ""; - } - $usersummary .= "\n
".gT("Permission")."".gT("Create")."".gT("View/read")."".gT("Update")."".gT("Delete")."".gT("Import")."".gT("Export")."
".$aCRUDPermissions['description']."{$aCRUDPermissions['title']}"; - - if ($CRUDValue) { - if (!($sPermissionKey == 'survey' && $sCRUDKey == 'read')) { - $usersummary .= CHtml::checkBox("perm_{$sPermissionKey}_{$sCRUDKey}", - ($action == 'setsurveysecurity' && Permission::model()->hasPermission($surveyid, 'survey', $sPermissionKey, $sCRUDKey, $postuserid)), - array( // htmlOptions - 'data-indeterminate'=>(bool) ($action == 'setsurveysecurity' && Permission::model()->hasSurveyPermission($surveyid, $sPermissionKey, $sCRUDKey, $postuserid)), - ) - ); - } - } - $usersummary .= "
" - ."

" - ."" - .""; - - if ($action == 'setsurveysecurity') { - $usersummary .= ""; - } else { - $usersummary .= ""; - } - $usersummary .= "\n"; - - $aViewUrls['output'] = $usersummary; + $usersummary = "

"; + if ($action == "setsurveysecurity") { + $query = "select users_name from {{users}} where uid=:uid"; + $resrow = Yii::app()->db->createCommand($query)->bindParam(":uid", $postuserid, PDO::PARAM_INT)->queryRow(); + $sUsername = $resrow['users_name']; + $usersummary .= "

".sprintf(gT("Edit survey permissions for user %s"), "".\CHtml::encode($sUsername)."")."

"; } else { - $this->getController()->error('Access denied'); + $resrow = UserGroup::model()->find('ugid = :ugid', array(':ugid' => $postusergroupid)); + $sUsergroupName = $resrow['name']; + $usersummary .= "

".sprintf(gT("Edit survey permissions for group %s"), "".\CHtml::encode($sUsergroupName)."")."

"; } + $usersummary .= '
'; + $usersummary .= "
" + .CHtml::form(array("admin/surveypermission/sa/surveyright/surveyid/{$surveyid}"), 'post'); + $PermissionManagerService = new PermissionManager( + App()->request, + App()->user, + $oSurvey + ); + $aPermissions = $PermissionManagerService->getPermissionData($postuserid); + $usersummary .= App()->getController()->widget( + 'ext.UserPermissionsWidget.UserPermissionsWidget', + ['aPermissions' => $aPermissions], + true + ); + $usersummary .= "

" + ."" + .""; + + if ($action == 'setsurveysecurity') { + $usersummary .= ""; + } else { + $usersummary .= ""; + } + $usersummary .= "\n"; + + $aViewUrls['output'] = $usersummary; $aData['sidemenu']['state'] = false; $aData['topBar']['showSaveButton'] = true; $aData['title_bar']['title'] = $oSurvey->currentLanguageSettings->surveyls_title." (".gT("ID").":".$surveyid.")"; $aData['surveybar']['savebutton']['form'] = 'frmeditgroup'; - $aData['surveybar']['saveandclosebutton']['form'] = 'frmeditgroup'; + $aData['surveybar']['saveandclosebutton']['form'] = 'frmeditgroup'; /* Not used */ $aData['surveybar']['closebutton']['url'] = 'surveyAdministration/view/surveyid/'.$surveyid; // Close button $this->_renderWrappedTemplate('authentication', $aViewUrls, $aData); @@ -604,83 +568,80 @@ function surveyright($surveyid) { $aData['surveyid'] = $surveyid = sanitize_int($surveyid); $oSurvey = Survey::model()->findByPk($surveyid); + if (!$oSurvey->hasPermission('surveysecurity', 'update')) { + throw new CHttpException(403, gT("You do not have permission to access this page.")); + } $aViewUrls = array(); - - $action = $_POST['action']; - $imageurl = Yii::app()->getConfig('imageurl'); - $postuserid = !empty($_POST['uid']) ? $_POST['uid'] : false; - $postusergroupid = !empty($_POST['ugid']) ? $_POST['ugid'] : false; + /* Only post value : CRSF control */ + $postuserid = App()->getRequest()->getPost('uid'); + $postusergroupid = App()->getRequest()->getPost('ugid'); - if ($postuserid && !in_array($postuserid, getUserList('onlyuidarray'))) { - $this->getController()->error('Access denied'); - } elseif ($postusergroupid && !in_array($postusergroupid, getUserGroupList())) { - $this->getController()->error('Access denied'); + if ($postuserid) { + if (!in_array($postuserid, getUserList('onlyuidarray'))) { + throw new CHttpException(403, gT("You do not have permission to this user.")); + } + if ($postuserid == App()->user->getId()) { + throw new CHttpException(403, gT("You can not set your own permission.")); + } + $uids = [$postuserid => $postuserid]; + } elseif ($postusergroupid) { + if (!in_array($postusergroupid, getUserGroupList())) { + throw new CHttpException(403, gT("You do not have permission to this user group.")); + } + $oUserInGroups = UserInGroup::model()->findAll( + 'ugid = :ugid AND uid <> :currentUserId AND uid <> :surveygroupsOwnerId', + array( + ':ugid' => $ugid, + ':currentUserId' => Permission::model()->getUserId(), // Don't need to set to current user + ':surveygroupsOwnerId' => $model->getOwnerId(), // Don't need to set to owner (?) , get from surveyspermission + ) + ); + $uids = CHtml::listData($oUserInGroups, 'uid', 'uid'); + } else { + throw new CHttpException(400, gT("Invalid parameters.")); } - if ($action == "surveyrights" && Permission::model()->hasSurveyPermission($surveyid, 'surveysecurity', 'update')) { - - $addsummary = "

"; - $addsummary .= '
'; - - $addsummary .= "
\n"; - $addsummary .= "

".gT("Edit survey permissions")."

\n"; + $addsummary = "
"; + $addsummary .= '
'; + + $addsummary .= "
\n"; + $addsummary .= "

".gT("Edit survey permissions")."

\n"; + + $user = App()->user; + $request = App()->request; + $success = true; + /* restrict to model (was in $set, but not needed : we have only Survey currently */ + $PermissionManagerService = new PermissionManager( + $request, + $user, + $oSurvey + ); + foreach ($uids as $uid) { + $success = $success && $PermissionManagerService->setPermissions($uid); + } - $where = ' '; - if ($postuserid) { - if (!Permission::model()->hasGlobalPermission('superadmin', 'read')) { - $where .= "sid = :surveyid AND owner_id != :postuserid AND owner_id = :owner_id"; - $resrow = Survey::model()->find($where, array(':surveyid' => $surveyid, ':owner_id' => Yii::app()->session['loginID'], ':postuserid' => $postuserid)); - } + if($postuserid){ + /* We update an user : redirect to Permission edit or to Permsoion view */ + if ($success) { + Yii::app()->setFlashMessage(gT("Survey permissions were successfully updated.")); } else { - $where .= "sid = :sid"; - $resrow = Survey::model()->find($where, array(':sid' => $surveyid)); - $iOwnerID = $resrow['owner_id']; + Yii::app()->setFlashMessage(gT("Failed to update survey permissions!")); } - - $aBaseSurveyPermissions = Permission::model()->getSurveyBasePermissions(); - $aPermissions = array(); - foreach ($aBaseSurveyPermissions as $sPermissionKey=>$aCRUDPermissions) { - foreach ($aCRUDPermissions as $sCRUDKey=>$CRUDValue) { - if (!in_array($sCRUDKey, array('create', 'read', 'update', 'delete', 'import', 'export'))) { - continue; - } - - if ($CRUDValue) { - if (isset($_POST["perm_{$sPermissionKey}_{$sCRUDKey}"])) { - $aPermissions[$sPermissionKey][$sCRUDKey] = 1; - } else { - $aPermissions[$sPermissionKey][$sCRUDKey] = 0; - } - } - } - } - - if (isset($postusergroupid) && $postusergroupid > 0) { - $oResult = UserInGroup::model()->findAll('ugid = :ugid AND uid <> :uid AND uid <> :iOwnerID', array(':ugid' => $postusergroupid, ':uid' => Yii::app()->session['loginID'], ':iOwnerID' => $iOwnerID)); - if (count($oResult) > 0) { - foreach ($oResult as $aRow) { - Permission::model()->setPermissions($aRow->uid, $surveyid, 'survey', $aPermissions); - } - $addsummary .= "
".gT("Survey permissions for all users in this group were successfully updated.")."
\n"; - } - } else { - if (Permission::model()->setPermissions($postuserid, $surveyid, 'survey', $aPermissions)) { - Yii::app()->setFlashMessage(gT("Survey permissions were successfully updated.")); - } else { - Yii::app()->setFlashMessage(gT("Failed to update survey permissions!")); - } - if (App()->getRequest()->getPost('close-after-save') == 'false') { - Yii::app()->request->redirect(Yii::app()->getController()->createUrl('admin/surveypermission/sa/set', array('action'=>'setsurveysecurity', 'surveyid'=>$surveyid, 'uid'=>$postuserid))); - } + if (App()->getRequest()->getPost('close-after-save')) { Yii::app()->request->redirect(Yii::app()->getController()->createUrl('admin/surveypermission/sa/view', array('surveyid'=>$surveyid))); } - $addsummary .= "
getController()->createUrl('admin/surveypermission/sa/view/surveyid/'.$surveyid)."', '_top')\" value=\"".gT("Continue")."\"/>\n"; - $addsummary .= "
\n"; - $aViewUrls['output'] = $addsummary; + Yii::app()->request->redirect(Yii::app()->getController()->createUrl('admin/surveypermission/sa/set', array('action'=>'setsurveysecurity', 'surveyid'=>$surveyid, 'uid'=>$postuserid))); + } + /* We update a group */ + if($success) { + $addsummary .= "
".gT("Survey permissions for all users in this group were successfully updated.")."
\n"; } else { - $this->getController()->error('Access denied'); + $addsummary .= "
".gT("Failed to update sermissions for all users in this group.")."
\n"; } + $addsummary .= "
getController()->createUrl('admin/surveypermission/sa/view/surveyid/'.$surveyid)."', '_top')\" value=\"".gT("Continue")."\"/>\n"; + $addsummary .= "
\n"; + $aViewUrls['output'] = $addsummary; $aData['sidemenu']['state'] = false; $aData['title_bar']['title'] = $oSurvey->currentLanguageSettings->surveyls_title." (".gT("ID").":".$surveyid.")"; diff --git a/application/models/services/PermissionManager.php b/application/models/services/PermissionManager.php index 176c3324928..bebde83e655 100644 --- a/application/models/services/PermissionManager.php +++ b/application/models/services/PermissionManager.php @@ -60,6 +60,10 @@ public function getPermissionData($userId = null) foreach (array_keys($aObjectPermissions) as $sPermission) { $aObjectPermissions[$sPermission]['current'] = array(); foreach($aCruds as $crud) { + if(!isset($aObjectPermissions[$sPermission][$crud])) { + /* Not set mean true (in Survey on 3.X) */ + $aObjectPermissions[$sPermission][$crud] = true; + } $aObjectPermissions[$sPermission]['current'][$crud] = array( 'checked' => false, /* The checkbox are disable if currentuser don't have permission */ From f3b1093fc591218ac4bbb46f1146be03a12780f4 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 12 Jan 2021 12:27:50 +0100 Subject: [PATCH 21/22] Dev: Fix : show visually disabled state on markrow Dev: partially after click --- .../extensions/UserPermissionsWidget/assets/script.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/application/extensions/UserPermissionsWidget/assets/script.js b/application/extensions/UserPermissionsWidget/assets/script.js index 3bc65dc5997..73f0ebd3353 100644 --- a/application/extensions/UserPermissionsWidget/assets/script.js +++ b/application/extensions/UserPermissionsWidget/assets/script.js @@ -28,7 +28,7 @@ $(document).on('ready pjax:scriptcomplete', function(){ $(".markrow").click( function(){ $(this).removeClass('mixed'); - $(this).closest('tr').find('input').prop('checked',$(this).prop('checked')).prop('indeterminate',false); + $(this).closest('tr').find('input:not(:disabled)').prop('checked',$(this).prop('checked')).prop('indeterminate',false); updateAllCheckboxes(); } ) @@ -67,7 +67,7 @@ $(document).on('ready pjax:scriptcomplete', function(){ { $('.table-permissions-set .extended').hide(); } - /* Show on the all comuln the state of included checkbox */ + /* Show on the all columnn the state of included checkbox */ $('.table-permissions-set tbody tr').each(function(){ if ($(this).find('.extended input:checkbox:checked').length == $(this).find('.extended input:checkbox').length) { /* All is checked */ @@ -81,11 +81,14 @@ $(document).on('ready pjax:scriptcomplete', function(){ } else { $(this).find('.markrow').prop('indeterminate',true).addClass('mixed'); } - } else { /* Partially checked */ $(this).find('.markrow').prop('checked',true).addClass('mixed'); } + /* disabled : only in all */ + if ($(this).find('.extended input:checkbox:disabled').length == $(this).find('.extended input:checkbox').length) { + $(this).find('.markrow').prop('disabled', true); + } }) $('#btnToggleAdvanced').click(function(){ From fa0a023f4426b4cbca94d6e19b0ea50bff305a3e Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Tue, 12 Jan 2021 14:25:02 +0100 Subject: [PATCH 22/22] Dev: deprecate setPermissions if model implement PermissionInterface --- application/models/Permission.php | 39 ++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/application/models/Permission.php b/application/models/Permission.php index 41b52138293..e7132d88c00 100644 --- a/application/models/Permission.php +++ b/application/models/Permission.php @@ -247,12 +247,14 @@ public static function getPermissions($iUserID, $iEntityID = null, $sEntityName * Sets permissions (global or survey-specific) for a survey administrator * Checks what permissions may be set and automatically filters invalid ones. * A permission may be invalid if the permission does not exist or that particular user may not give that permission + * @deprecated : usage only for global Permission currently * * @param mixed $iUserID * @param mixed $iEntityID * @param string $sEntityName * @param mixed $aPermissions - * @param boolean $bBypassCheck + * @param boolean $bBypassCheck : by pass control of current permission for current user only for global permission + * @throw Exception * @return null|boolean */ public static function setPermissions($iUserID, $iEntityID, $sEntityName, $aPermissions, $bBypassCheck = false) @@ -286,6 +288,10 @@ public static function setPermissions($iUserID, $iEntityID, $sEntityName, $aPerm unset($aBasePermissions['superadmin']); } } else { + if (in_array("PermissionInterface", class_implements($sEntityName))) { + /* model implement \PermissionInterface */ + throw new Exception("Must use PermissionManager service"); + } $aBasePermissions = Permission::model()->getEntityBasePermissions($sEntityName); } @@ -413,6 +419,9 @@ public function setGlobalPermission($iNewUID, $sPermType, array $aPermissions = } /** + * Give all permission of a specific user without permission control of current user + * Used when create survey + * @see mantis #16967: https://bugs.limesurvey.org/view.php?id=16967 * @param integer $iUserID * @param integer $iSurveyID */ @@ -421,18 +430,30 @@ public function giveAllSurveyPermissions($iUserID, $iSurveyID) if ($iSurveyID == 0) { throw new InvalidArgumentException('Survey ID cannot be 0 (collides with superadmin permission entity id)'); } - - $aPermissions = $this->getSurveyBasePermissions(); - $aPermissionsToSet = array(); + $aPermissions = Survey::getPermissionData(); + $aCrud = array('create', 'read', 'update', 'delete', 'import', 'export'); foreach ($aPermissions as $sPermissionName => $aPermissionDetails) { - foreach ($aPermissionDetails as $sPermissionDetailKey => $sPermissionDetailValue) { - if (in_array($sPermissionDetailKey, array('create', 'read', 'update', 'delete', 'import', 'export')) && $sPermissionDetailValue == true) { - $aPermissionsToSet[$sPermissionName][$sPermissionDetailKey] = 1; + tracevar($aPermissionDetails); + $oPermission = Permission::model()->findByAttributes(array( + 'entity' => 'survey', + 'entity_id' => $iSurveyID, + 'uid' => $iUserID, + 'permission' => $sPermissionName + )); + if (empty($oPermission)) { + $oPermission = new Permission; + $oPermission->entity = 'survey'; + $oPermission->entity_id = $iSurveyID; + $oPermission->uid = $iUserID; + $oPermission->permission = $sPermissionName; + } + foreach($aCrud as $crud) { + if(!isset($aPermissionDetails[$crud]) || $aPermissionDetails[$crud]) { + $oPermission->setAttribute($crud."_p", 1); } } + $oPermission->save(); } - - $this->setPermissions($iUserID, $iSurveyID, 'survey', $aPermissionsToSet); } /**