From 5c9aa865cb19113325d8100bb7a7718a30334a8f Mon Sep 17 00:00:00 2001 From: Patrick Teichmann Date: Wed, 15 May 2019 14:26:10 +0200 Subject: [PATCH] Fixed issue #14514: Purpose of permission "update" - CPDB Fixed issue: Administrators now have access to the CPDB if they have shared participants or have global Permissions 'read, create, update, delete', "global Permissions" > "shared Permissions" --- .../controllers/admin/participantsaction.php | 81 ++++++++++++++----- application/models/Participant.php | 22 ++--- application/models/ParticipantShare.php | 41 +++++++++- .../participants/displayParticipants_view.php | 14 ++-- .../participants/participantsPanel_view.php | 3 +- .../admin/participants/sharePanel_view.php | 2 +- .../views/admin/super/_configuration_menu.php | 30 ++++--- application/views/admin/super/layout_main.php | 4 +- assets/scripts/admin/participantpanel.js | 4 +- 9 files changed, 146 insertions(+), 55 deletions(-) diff --git a/application/controllers/admin/participantsaction.php b/application/controllers/admin/participantsaction.php index 34be78ce399..0d102161c21 100644 --- a/application/controllers/admin/participantsaction.php +++ b/application/controllers/admin/participantsaction.php @@ -48,8 +48,12 @@ class participantsaction extends Survey_Common_Action public function runWithParams($params) { -// $this->checkPermission(['read','create']); - if (!(Permission::model()->hasGlobalPermission('participantpanel', 'read') || Permission::model()->hasGlobalPermission('participantpanel', 'create'))) { + if (!(Permission::model()->hasGlobalPermission('participantpanel', 'read') + || Permission::model()->hasGlobalPermission('participantpanel', 'create') + || Permission::model()->hasGlobalPermission('participantpanel', 'update') + || Permission::model()->hasGlobalPermission('participantpanel', 'delete') + || ParticipantShare::model()->exists('share_uid = ' . App()->user->id ? App()->user->id : '')) + ) { App()->setFlashMessage(gT('No permission'), 'error'); App()->getController()->redirect(App()->request->urlReferrer); } @@ -303,10 +307,10 @@ public function displayParticipants() } // if superadmin all the records in the cpdb will be displayed + $iUserId = App()->user->getId(); if (Permission::model()->hasGlobalPermission('superadmin', 'read')) { $iTotalRecords = Participant::model()->count(); } else {// if not only the participants on which he has right on (shared and owned) - $iUserId = Yii::app()->user->getId(); $iTotalRecords = Participant::model()->getParticipantsOwnerCount($iUserId); } $model = new Participant(); @@ -345,7 +349,7 @@ public function displayParticipants() Yii::app()->clientScript->registerPackage('bootstrap-switch'); // check global and custom permissions and pass them to $aData - $permissions = permissionsAsArray( + $aData['permissions'] = permissionsAsArray( [ 'superadmin' => ['read'], 'templates' => ['read'], @@ -358,15 +362,16 @@ public function displayParticipants() [ 'participantpanel' => [ 'editSharedParticipants' => empty(ParticipantShare::model()->findAllByAttributes( - ['share_uid' => App()->user->id ? App()->user->id : ''], + ['share_uid' => $iUserId], ['condition' => 'can_edit = \'0\' OR can_edit = \'\'',] )), - 'sharedParticipantExists' => ParticipantShare::model()->exists('share_uid = ' . App()->user->id ? App()->user->id : '') + 'sharedParticipantExists' => ParticipantShare::model()->exists('share_uid = ' . $iUserId), + 'isOwner' => isset($participantParam['owner_uid']) && ($participantParam['owner_uid'] === $iUserId) ? true : false ], ] ); - $aData['massiveAction'] = App()->getController()->renderPartial('/admin/participants/massive_actions/_selector', array('permissions' => $permissions), true, false); + $aData['massiveAction'] = App()->getController()->renderPartial('/admin/participants/massive_actions/_selector', array('permissions' => $aData['permissions']), true, false); // Set page size if ($request->getPost('pageSizeParticipantView')) { @@ -585,8 +590,15 @@ public function editParticipant() } } - public function batchEdit() { - if (!Permission::model()->hasGlobalPermission('participantpanel', 'update')) { + public function batchEdit() + { + $hasUpdatePermission = Permission::model()->hasGlobalPermission('participantpanel', 'update'); + if (!$hasUpdatePermission + && empty(ParticipantShare::model()->findAllByAttributes( + ['share_uid' => App()->user->id ? App()->user->id : ''], + ['condition' => 'can_edit = 1',] + )) + ) { Yii::app()->user->setFlash('error', gT("Access denied")); $this->getController()->redirect(Yii::app()->createUrl('/admin')); return; @@ -622,13 +634,22 @@ public function batchEdit() { $oParticipant->$key = $value; } - $bUpdateSuccess = $oParticipant->save(); + // Check if the User is allowed to edit the participant + if (ParticipantShare::model()->canEditSharedParticipant($sParticipantId) + || $oParticipant->isOwnerOrSuperAdmin() + || $hasUpdatePermission + ) { + $bUpdateSuccess = $oParticipant->save(); + } else { + $bUpdateSuccess = ''; + }; + if ($bUpdateSuccess) { $aResults[$sParticipantId]['status'] = true; $aResults[$sParticipantId]['message'] = gT('Updated'); } else { $aResults[$sParticipantId]['status'] = false; - $aResults[$sParticipantId]['message'] = $oParticipant->error; + $aResults[$sParticipantId]['message'] = $oParticipant->getError('participant_id'); } } } else { @@ -2084,15 +2105,18 @@ public function setSession() /** * Stores the shared participant information in participant_shares + * * @return void + * @throws CException */ public function shareParticipants() { - if (!Permission::model()->hasGlobalPermission('participantpanel', 'update')) { - ls\ajax\AjaxHelper::outputNoPermission(); - return; - } - + $hasUpdatePermission = Permission::model()->hasGlobalPermission('update'); + $isSuperAdmin = Permission::model()->hasGlobalPermission('superadmin', 'read'); + $permissions = [ + 'hasUpdatePermission' => $hasUpdatePermission, + 'isSuperAdmin' => $isSuperAdmin + ]; $participantIds = Yii::app()->request->getPost('participant_id'); $iShareUserId = Yii::app()->request->getPost('shareuser'); $bCanEdit = Yii::app()->request->getPost('can_edit') == 'on'; @@ -2108,7 +2132,7 @@ public function shareParticipants() $i = 0; // $iShareUserId == 0 means any user - if (Permission::model()->hasGlobalPermission('participantpanel', 'update') && $iShareUserId !== '') { + if ($iShareUserId !== '') { foreach ($participantIds as $id) { $time = time(); $aData = array( @@ -2117,7 +2141,7 @@ public function shareParticipants() 'date_added' => date('Y-m-d H:i:s', $time), 'can_edit' => $bCanEdit ); - ParticipantShare::model()->storeParticipantShare($aData); + ParticipantShare::model()->storeParticipantShare($aData, $permissions); $i++; } } @@ -2125,15 +2149,27 @@ public function shareParticipants() } /** - * Stores the shared participant information in participant_shares for ONE participant + * Stores the shared participant information in participant_shares for ONE participant * + * * @return void + * @throws CException + * TODO: Is this function even used anymore? Seems all logic goes through shareParticipants() */ public function shareParticipant() { + $hasUpdatePermission = Permission::model()->hasGlobalPermission('update'); + $isSuperAdmin = Permission::model()->hasGlobalPermission('superadmin', 'read'); + $permissions = [ + 'hasUpdatePermission' => $hasUpdatePermission, + 'isSuperAdmin' => $isSuperAdmin + ]; + $iParticipantId = Yii::app()->request->getPost('participant_id'); $bCanEdit = Yii::app()->request->getPost('can_edit'); - if (Permission::model()->hasGlobalPermission('participantpanel', 'update')) { + if (ParticipantShare::model()->canEditSharedParticipant($iParticipantId) + || $hasUpdatePermission + || $isSuperAdmin) { $time = time(); $aData = array( 'participant_id' => $iParticipantId, @@ -2141,7 +2177,7 @@ public function shareParticipant() 'date_added' => date('Y-m-d H:i:s', $time), 'can_edit' => $bCanEdit ); - ParticipantShare::model()->storeParticipantShare($aData); + ParticipantShare::model()->storeParticipantShare($aData, $permissions); ls\ajax\AjaxHelper::outputSuccess(gT("Participant shared.")); } else { @@ -2239,7 +2275,8 @@ public function changeSharedEditableStatus() { $participant_id = Yii::app()->request->getPost('participant_id'); $can_edit = Yii::app()->request->getPost('can_edit'); - $shareModel = ParticipantShare::model()->findByAttributes(array('participant_id' => $participant_id)); + $share_uid = Yii::app()->request->getPost('share_uid'); + $shareModel = ParticipantShare::model()->findByAttributes(array('participant_id' => $participant_id, 'share_uid' => $share_uid)); if ($shareModel) { $shareModel->can_edit = ($can_edit == 'true' ? 1 : 0); diff --git a/application/models/Participant.php b/application/models/Participant.php index a51edfc71fe..c4c0afb434d 100755 --- a/application/models/Participant.php +++ b/application/models/Participant.php @@ -2101,10 +2101,9 @@ public function userHasPermissionToEdit() { $userId = Yii::app()->user->id; - $shared = ParticipantShare::model()->findByAttributes(array( - 'participant_id' => $this->participant_id - )); - + $shared = ParticipantShare::model()->findByAttributes( + ['participant_id' => $this->participant_id], 'share_uid = ' . $userId. ' AND can_edit = 1' + ); $owner = $this->owner_uid == $userId; if (Permission::model()->hasGlobalPermission('superadmin') || (Permission::model()->hasGlobalPermission('participantpanel', 'update'))) { @@ -2113,7 +2112,7 @@ public function userHasPermissionToEdit() } else if ($shared && $shared->share_uid == -1 && $shared->can_edit) { // -1 = shared with everyone return true; - } else if ($shared && $shared->share_uid == $userId && $shared->can_edit) { + } else if ($shared && $shared->exists('share_uid' == $userId) && $shared->can_edit) { // Shared with this particular user return true; } else if ($owner) { @@ -2188,12 +2187,12 @@ public function permissionCheckedActionsArray($aActions, $permissions) if (isset($aAction['action'])) { switch ($aAction['action']) { case 'delete': - if ($permissions['superadmin']['read'] || $permissions['participantpanel']['delete'] || !$permissions['participantpanel']['sharedParticipantExists']) { + if ($permissions['participantpanel']['isOwner'] || $permissions['superadmin']['read'] || $permissions['participantpanel']['delete'] || !$permissions['participantpanel']['sharedParticipantExists']) { array_push($checkedActions, $aAction); } break; case 'batchEdit': - if ($permissions['superadmin']['read'] + if ($permissions['participantpanel']['isOwner'] || $permissions['superadmin']['read'] || $permissions['participantpanel']['update'] || $permissions['participantpanel']['editSharedParticipants'] || !$permissions['participantpanel']['sharedParticipantExists'] @@ -2202,12 +2201,17 @@ public function permissionCheckedActionsArray($aActions, $permissions) } break; case 'export': - if ($permissions['superadmin']['read'] || $permissions['participantpanel']['export'] || !$permissions['participantpanel']['sharedParticipantExists']) { + if ($permissions['participantpanel']['isOwner'] || $permissions['superadmin']['read'] || $permissions['participantpanel']['export'] || !$permissions['participantpanel']['sharedParticipantExists']) { array_push($checkedActions, $aAction); } break; case 'share': - if ($permissions['superadmin']['read'] || $permissions['participantpanel']['update'] || !$permissions['participantpanel']['sharedParticipantExists']) { + if ($permissions['participantpanel']['isOwner'] + || $permissions['superadmin']['read'] + || $permissions['participantpanel']['update'] + || !$permissions['participantpanel']['sharedParticipantExists'] + || $permissions['participantpanel']['editSharedParticipants'] + ) { array_push($checkedActions, $aAction); } break; diff --git a/application/models/ParticipantShare.php b/application/models/ParticipantShare.php index df428492464..41c1ab127be 100644 --- a/application/models/ParticipantShare.php +++ b/application/models/ParticipantShare.php @@ -11,6 +11,9 @@ * See COPYRIGHT.php for copyright notices and details. * */ + +use ls\ajax\AjaxHelper; + /** * This is the model class for table "{{participant_shares}}". * @@ -229,6 +232,7 @@ public function getColumns() array( "name" => 'share_uid', "value" => '$data->sharedBy', + "type" => 'raw', "header" => gT("Shared by"), "filter" => $this->getSharedByList($this->share_uid) ), @@ -317,14 +321,25 @@ public function search() /** * @param array $data + * @param array $permission + * * @return void + * @throws CException */ - public function storeParticipantShare($data) + public function storeParticipantShare($data, $permission) { - $ownerid = Yii::app()->db->createCommand()->select('*')->from('{{participants}}')->where('participant_id = :participant_id')->bindParam(":participant_id", $data['participant_id'], PDO::PARAM_STR)->queryRow(); + $hasUpdatePermission = isset($permission['hasUpdatePermission'])? $permission['hasUpdatePermission'] : false; + $isSuperAdmin = isset($permission['isSuperAdmin'])? $permission['isSuperAdmin'] : false; + $userId = App()->user->getId(); + $ownerid = App()->db->createCommand()->select('*')->from('{{participants}}')->where('participant_id = :participant_id')->bindParam(":participant_id", $data['participant_id'], PDO::PARAM_STR)->queryRow(); + // CHeck if share already exists - $arShare = $this->findByPk(array('participant_id'=>$data['participant_id'], 'share_uid'=>$data['share_uid'])); - if ($ownerid['owner_uid'] == $data['share_uid']) { + $arShare = $this->findByPk(['participant_id' => $data['participant_id'], 'share_uid' => $data['share_uid']]); + $canEditShared = $this->canEditSharedParticipant($data['participant_id']); + $isOwner = $ownerid['owner_uid'] == $userId; + + if ($ownerid['owner_uid'] == $data['share_uid'] || (!$permission && !$canEditShared && !$isOwner && !$isSuperAdmin && !$hasUpdatePermission)) { + ls\ajax\AjaxHelper::outputNoPermission(); return; } if (is_null($arShare)) { @@ -376,4 +391,22 @@ public function getOwnerName() return $this->participant->owner->full_name; } + /** + * Returns true if the user is allowed to edit the participant + * + * @param $participent_id + * + * @return boolean + */ + public function canEditSharedParticipant($participent_id) + { + $participent = $this->findByAttributes( + ['participant_id' => $participent_id], + ['condition' => 'can_edit = 1 AND share_uid = ' . App()->user->getId()]); + if ($participent) { + return true; + } + return false; + } + } diff --git a/application/views/admin/participants/displayParticipants_view.php b/application/views/admin/participants/displayParticipants_view.php index 8dcc8f5697b..263145cffab 100644 --- a/application/views/admin/participants/displayParticipants_view.php +++ b/application/views/admin/participants/displayParticipants_view.php @@ -12,11 +12,15 @@
- + hasGlobalPermission('superadmin', 'read') + || Permission::model()->hasGlobalPermission('participantpanel', 'create') + ): ?> + +
diff --git a/application/views/admin/participants/participantsPanel_view.php b/application/views/admin/participants/participantsPanel_view.php index 9d999b764d1..58c50bc554b 100644 --- a/application/views/admin/participants/participantsPanel_view.php +++ b/application/views/admin/participants/participantsPanel_view.php @@ -59,7 +59,8 @@ - hasGlobalPermission('participantpanel','create')):?> + hasGlobalPermission('participantpanel','create') + || ParticipantShare::model()->exists('share_uid = ' . App()->user->getId())):?> " role="button"> diff --git a/application/views/admin/participants/sharePanel_view.php b/application/views/admin/participants/sharePanel_view.php index bd36a7d96bf..ea59b4655cb 100644 --- a/application/views/admin/participants/sharePanel_view.php +++ b/application/views/admin/participants/sharePanel_view.php @@ -18,7 +18,7 @@ 'itemsCssClass' => 'table table-striped items', 'htmlOptions' => array('class'=> 'table-responsive'), 'dataProvider' => $model->search(), - 'rowHtmlOptionsExpression' => '["data-participant_id" => $data->participant_id ]', + 'rowHtmlOptionsExpression' => '["data-participant_id" => $data->participant_id, "data-share_uid" => $data->share_uid]', 'columns' => $model->columns, 'filter'=>$model, 'ajaxType' => 'POST', diff --git a/application/views/admin/super/_configuration_menu.php b/application/views/admin/super/_configuration_menu.php index 92bf9674e2e..511bfffa0a4 100644 --- a/application/views/admin/super/_configuration_menu.php +++ b/application/views/admin/super/_configuration_menu.php @@ -8,13 +8,19 @@ ?> -hasGlobalPermission('superadmin','read') - || Permission::model()->hasGlobalPermission('templates','read') - || Permission::model()->hasGlobalPermission('labelsets','read') - || Permission::model()->hasGlobalPermission('users','read') - || Permission::model()->hasGlobalPermission('usergroups','read') - || Permission::model()->hasGlobalPermission('participantpanel','read') - || Permission::model()->hasGlobalPermission('settings','read') ): ?> +hasGlobalPermission('superadmin', 'read') + || Permission::model()->hasGlobalPermission('templates', 'read') + || Permission::model()->hasGlobalPermission('labelsets', 'read') + || Permission::model()->hasGlobalPermission('users', 'read') + || Permission::model()->hasGlobalPermission('usergroups', 'read') + || Permission::model()->hasGlobalPermission('participantpanel', 'read') + || Permission::model()->hasGlobalPermission('participantpanel', 'create') + || Permission::model()->hasGlobalPermission('participantpanel', 'update') + || Permission::model()->hasGlobalPermission('participantpanel', 'delete') + || ParticipantShare::model()->exists('share_uid = ' . App()->user->id ? App()->user->id : '') + || Permission::model()->hasGlobalPermission('settings', 'read') +): ?> +
  • @@ -211,8 +217,12 @@ - hasGlobalPermission('participantpanel','read')): ?> - + hasGlobalPermission('participantpanel', 'read') + || Permission::model()->hasGlobalPermission('participantpanel', 'create') + || Permission::model()->hasGlobalPermission('participantpanel', 'update') + || Permission::model()->hasGlobalPermission('participantpanel', 'delete') + || ParticipantShare::model()->exists('share_uid = ' . App()->user->id ? App()->user->id : '') + ): ?>
  • "> @@ -271,7 +281,7 @@
    • - + diff --git a/application/views/admin/super/layout_main.php b/application/views/admin/super/layout_main.php index bb0ff504019..a25f30dc9cb 100644 --- a/application/views/admin/super/layout_main.php +++ b/application/views/admin/super/layout_main.php @@ -1,7 +1,9 @@ _showHeaders($aData); //The adminmenu bar will be generated from /admin/super/adminmenu.php diff --git a/assets/scripts/admin/participantpanel.js b/assets/scripts/admin/participantpanel.js index 97e73d432bd..9ca90bdf195 100644 --- a/assets/scripts/admin/participantpanel.js +++ b/assets/scripts/admin/participantpanel.js @@ -363,12 +363,12 @@ LS.CPDB = (function() { $.ajax({ url: editValueParticipantPanel, method: "POST", - data: {actionTarget: 'changeSharedEditableStatus', 'participant_id': $(self).closest('tr').data('participant_id'), 'can_edit': state}, + data: {actionTarget: 'changeSharedEditableStatus', 'participant_id': $(self).closest('tr').data('participant_id'), 'can_edit': state, 'share_uid': $(self).closest('tr').data('share_uid')}, dataType: 'json', success: function(resolve){ $(self).prop("checked", resolve.newValue); } - }) + }); }); $('#pageSizeShareParticipantView').on("change", function(){