From c35d67ebcae81a17d5c627b57bfba27c2dd87eb8 Mon Sep 17 00:00:00 2001 From: iglocska Date: Tue, 18 Jan 2022 14:59:41 +0100 Subject: [PATCH 1/5] fix: [encryption keys] functionality to filter orgs/individuals fixed - actually execute the query rather than just build it --- src/Controller/EncryptionKeysController.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Controller/EncryptionKeysController.php b/src/Controller/EncryptionKeysController.php index e04ebc0..ff2c16c 100644 --- a/src/Controller/EncryptionKeysController.php +++ b/src/Controller/EncryptionKeysController.php @@ -90,14 +90,8 @@ class EncryptionKeysController extends AppController $this->loadModel('Organisations'); $this->loadModel('Individuals'); $dropdownData = [ - 'organisation' => $this->Organisations->find('list', [ - 'sort' => ['name' => 'asc'], - 'conditions' => $orgConditions - ]), - 'individual' => $this->Individuals->find('list', [ - 'sort' => ['email' => 'asc'], - 'conditions' => $individualConditions - ]) + 'organisation' => $this->Organisations->find('list')->order(['name' => 'asc'])->where($orgConditions)->all()->toArray(), + 'individual' => $this->Individuals->find('list')->order(['email' => 'asc'])->where($individualConditions)->all()->toArray() ]; return $params; } From 8cb24baf5f1a4f52f68c532a8e55068b948ac131 Mon Sep 17 00:00:00 2001 From: iglocska Date: Tue, 18 Jan 2022 15:35:55 +0100 Subject: [PATCH 2/5] fix: [ACL] tightening for delete functions - implemented beforeSave() function in the CRUD::delete() functionality - added correct handling for the organisation level encryption keys in the beforeSave constructor --- src/Controller/Component/CRUDComponent.php | 32 +++++++++--- src/Controller/EncryptionKeysController.php | 58 +++++++++++---------- 2 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 4ebc674..0adfcdf 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -442,6 +442,12 @@ class CRUDComponent extends Component if (empty($data)) { throw new NotFoundException(__('Invalid {0}.', $this->ObjectAlias)); } + if (isset($params['beforeSave'])) { + $data = $params['beforeSave']($data); + if ($data === false) { + throw new NotFoundException(__('Could not save {0} due to the input failing to meet expectations. Your input is bad and you should feel bad.', $this->ObjectAlias)); + } + } $this->Controller->set('id', $data['id']); $this->Controller->set('data', $data); $this->Controller->set('bulkEnabled', false); @@ -453,6 +459,7 @@ class CRUDComponent extends Component $isBulk = count($ids) > 1; $bulkSuccesses = 0; foreach ($ids as $id) { + $skipExecution = false; $data = $this->Table->find()->where([$this->Table->getAlias() . '.id' => $id]); if (!empty($params['conditions'])) { $data->where($params['conditions']); @@ -460,15 +467,24 @@ class CRUDComponent extends Component if (!empty($params['contain'])) { $data->contain($params['contain']); } - $data = $data->first(); - if (!empty($data)) { - $success = $this->Table->delete($data); - $success = true; - } else { - $success = false; + if (isset($params['beforeSave'])) { + $data = $params['beforeSave']($data); + if ($data === false) { + $skipExecution = true; + $success = false; + } } - if ($success) { - $bulkSuccesses++; + if (!$skipExecution) { + $data = $data->first(); + if (!empty($data)) { + $success = $this->Table->delete($data); + $success = true; + } else { + $success = false; + } + if ($success) { + $bulkSuccesses++; + } } } $message = $this->getMessageBasedOnResult( diff --git a/src/Controller/EncryptionKeysController.php b/src/Controller/EncryptionKeysController.php index ff2c16c..324decb 100644 --- a/src/Controller/EncryptionKeysController.php +++ b/src/Controller/EncryptionKeysController.php @@ -57,36 +57,40 @@ class EncryptionKeysController extends AppController private function buildBeforeSave(array $params, $currentUser, array &$orgConditions, array &$individualConditions, array &$dropdownData): array { - $orgConditions = [ - 'id' => $currentUser['organisation_id'] - ]; - if (empty($currentUser['role']['perm_org_admin'])) { - $individualConditions = [ - 'id' => $currentUser['individual_id'] + if (empty($currentUser['role']['perm_admin'])) { + $orgConditions = [ + 'id' => $currentUser['organisation_id'] ]; - } - $params['beforeSave'] = function($entity) use($currentUser) { - if ($entity['owner_model'] === 'organisation') { - $entity['owner_id'] = $currentUser['organisation_id']; - } else { - if ($currentUser['role']['perm_org_admin']) { - $this->loadModel('Alignments'); - $validIndividuals = $this->Alignments->find('list', [ - 'keyField' => 'individual_id', - 'valueField' => 'id', - 'conditions' => ['organisation_id' => $currentUser['organisation_id']] - ])->toArray(); - if (!isset($validIndividuals[$entity['owner_id']])) { - throw new MethodNotAllowedException(__('Selected individual cannot be linked by the current user.')); + if (empty($currentUser['role']['perm_org_admin'])) { + $individualConditions = [ + 'id' => $currentUser['individual_id'] + ]; + } + $params['beforeSave'] = function($entity) use($currentUser) { + if ($entity['owner_model'] === 'organisation') { + if ($entity['owner_id'] !== $currentUser['organisation_id']) { + throw new MethodNotAllowedException(__('Selected organisation cannot be linked by the current user.')); } } else { - if ($entity['owner_id'] !== $currentUser['id']) { - throw new MethodNotAllowedException(__('Selected individual cannot be linked by the current user.')); + if ($currentUser['role']['perm_org_admin']) { + $this->loadModel('Alignments'); + $validIndividuals = $this->Alignments->find('list', [ + 'keyField' => 'individual_id', + 'valueField' => 'id', + 'conditions' => ['organisation_id' => $currentUser['organisation_id']] + ])->toArray(); + if (!isset($validIndividuals[$entity['owner_id']])) { + throw new MethodNotAllowedException(__('Selected individual cannot be linked by the current user.')); + } + } else { + if ($entity['owner_id'] !== $currentUser['id']) { + throw new MethodNotAllowedException(__('Selected individual cannot be linked by the current user.')); + } } } - } - return $entity; - }; + return $entity; + }; + } $this->loadModel('Organisations'); $this->loadModel('Individuals'); $dropdownData = [ @@ -105,9 +109,7 @@ class EncryptionKeysController extends AppController $params = [ 'redirect' => $this->referer() ]; - if (empty($currentUser['role']['perm_admin'])) { - $params = $this->buildBeforeSave($params, $currentUser, $orgConditions, $individualConditions, $dropdownData); - } + $params = $this->buildBeforeSave($params, $currentUser, $orgConditions, $individualConditions, $dropdownData); $this->CRUD->add($params); $responsePayload = $this->CRUD->getResponsePayload(); if (!empty($responsePayload)) { From eae8e62e5ec600f3bec23327c43da046558d1fb3 Mon Sep 17 00:00:00 2001 From: iglocska Date: Tue, 18 Jan 2022 16:24:24 +0100 Subject: [PATCH 3/5] fix: [CRUD] delete post message fix - correct order of execution for the beforesave command --- src/Controller/Component/CRUDComponent.php | 24 ++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 0adfcdf..d1c78fc 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -467,24 +467,18 @@ class CRUDComponent extends Component if (!empty($params['contain'])) { $data->contain($params['contain']); } + $data = $data->first(); if (isset($params['beforeSave'])) { $data = $params['beforeSave']($data); - if ($data === false) { - $skipExecution = true; - $success = false; - } } - if (!$skipExecution) { - $data = $data->first(); - if (!empty($data)) { - $success = $this->Table->delete($data); - $success = true; - } else { - $success = false; - } - if ($success) { - $bulkSuccesses++; - } + if (!empty($data)) { + $success = $this->Table->delete($data); + $success = true; + } else { + $success = false; + } + if ($success) { + $bulkSuccesses++; } } $message = $this->getMessageBasedOnResult( From dbaa2ba7b320ce3f9511155ab7b9b960e26e67fc Mon Sep 17 00:00:00 2001 From: iglocska Date: Tue, 18 Jan 2022 16:56:38 +0100 Subject: [PATCH 4/5] fix: [encryption keys] several fixes - fix the user view to correctly point to the list of related encryption keys - fix the lookup on the index to be based on owner_model + owner_id combo - fix the filtering of the dropdown in the encryption key add form to only valid options --- src/Controller/EncryptionKeysController.php | 9 ++++++++- templates/Users/view.php | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Controller/EncryptionKeysController.php b/src/Controller/EncryptionKeysController.php index 324decb..803f180 100644 --- a/src/Controller/EncryptionKeysController.php +++ b/src/Controller/EncryptionKeysController.php @@ -14,7 +14,7 @@ use Cake\Error\Debugger; class EncryptionKeysController extends AppController { - public $filterFields = ['owner_model', 'organisation_id', 'individual_id', 'encryption_key']; + public $filterFields = ['owner_model', 'owner_id', 'encryption_key']; public $quickFilterFields = ['encryption_key']; public $containFields = ['Individuals', 'Organisations']; @@ -65,6 +65,13 @@ class EncryptionKeysController extends AppController $individualConditions = [ 'id' => $currentUser['individual_id'] ]; + } else { + $this->loadModel('Alignments'); + $individualConditions = ['id IN' => $this->Alignments->find('list', [ + 'keyField' => 'id', + 'valueField' => 'individual_id', + 'conditions' => ['organisation_id' => $currentUser['organisation_id']] + ])->toArray()]; } $params['beforeSave'] = function($entity) use($currentUser) { if ($entity['owner_model'] === 'organisation') { diff --git a/templates/Users/view.php b/templates/Users/view.php index 26c3c25..fbddf52 100644 --- a/templates/Users/view.php +++ b/templates/Users/view.php @@ -56,8 +56,8 @@ echo $this->element( 'title' => __('Authentication keys') ], [ - 'url' => '/EncryptionKeys/index?Users.id={{0}}', - 'url_params' => ['id'], + 'url' => '/EncryptionKeys/index?owner_id={{0}}', + 'url_params' => ['individual_id'], 'title' => __('Encryption keys') ], [ From f75d0829d1a98e2c686beb89d993df5ac824883f Mon Sep 17 00:00:00 2001 From: iglocska Date: Tue, 18 Jan 2022 17:52:59 +0100 Subject: [PATCH 5/5] fix: [user edit] fixed for non admins --- src/Controller/UsersController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index a5065db..9ffb2fe 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -7,6 +7,7 @@ use Cake\Utility\Text; use Cake\ORM\TableRegistry; use \Cake\Database\Expression\QueryExpression; use Cake\Http\Exception\UnauthorizedException; +use Cake\Http\Exception\MethodNotAllowedException; use Cake\Core\Configure; class UsersController extends AppController @@ -100,11 +101,10 @@ class UsersController extends AppController if (empty($id)) { $id = $currentUser['id']; } else { + $id = intval($id); if ((empty($currentUser['role']['perm_org_admin']) && empty($currentUser['role']['perm_admin']))) { if ($id !== $currentUser['id']) { throw new MethodNotAllowedException(__('You are not authorised to edit that user.')); - } else { - $id = $currentUser['id']; } } }