From b45cc8ae221c798be65660ba5a1fdc128391986d Mon Sep 17 00:00:00 2001 From: Jeroen Pinoy Date: Sat, 16 Nov 2024 15:27:10 +0100 Subject: [PATCH 01/18] fix: Set proxy settings diagonistics severity level to info. fix #176 --- src/Model/Table/SettingProviders/CerebrateSettingsProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php index 256b3a9..30cf7a3 100644 --- a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php +++ b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php @@ -136,12 +136,14 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'Proxy' => [ 'Proxy.host' => [ 'name' => __('Host'), + 'severity' => 'info', 'type' => 'string', 'description' => __('The hostname of an HTTP proxy for outgoing sync requests. Leave empty to not use a proxy.'), 'test' => 'testHostname', ], 'Proxy.port' => [ 'name' => __('Port'), + 'severity' => 'info', 'type' => 'integer', 'description' => __('The TCP port for the HTTP proxy.'), 'test' => 'testForRangeXY', From ac33e90f0c3e807a2e45eee94ec8c6b66835a8de Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 22 Nov 2024 12:40:21 +0100 Subject: [PATCH 02/18] fix: [message handling] of error messages - correctly handle beforeSave / afterSave failures in ajax contexts - until now it was just silently failing giving cryptic messages to the user --- src/Controller/Component/CRUDComponent.php | 203 +++++++++++------- .../Component/RestResponseComponent.php | 3 +- 2 files changed, 132 insertions(+), 74 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 607faf9..e8f6fc0 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -465,51 +465,73 @@ class CRUDComponent extends Component } } $data = $this->Table->patchEntity($data, $input, $patchEntityParams); + $break = false; if (isset($params['beforeSave'])) { - $data = $params['beforeSave']($data); + try { + $data = $params['beforeSave']($data); + } catch (\Exception $e) { + $message = $e->getMessage(); + $break = true; + $this->__raiseErrorToUser(__('Could not save {0}.', $this->ObjectAlias), ['Custom checks' => ['Custom checks' => $message]]); + } 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)); + $break = true; + $this->__raiseErrorToUser(__('Could not save {0} due to the input failing to meet expectations. Your input is bad and you should feel bad.', $this->ObjectAlias)); } } - $savedData = $this->Table->save($data); - if ($savedData !== false) { - if (isset($params['afterSave'])) { - $params['afterSave']($data); - } - $message = __('{0} added.', $this->ObjectAlias); - if ($this->Controller->ParamHandler->isRest()) { - $this->Controller->restResponsePayload = $this->RestResponse->viewData($savedData, 'json'); - } else if ($this->Controller->ParamHandler->isAjax()) { - if (!empty($params['displayOnSuccess'])) { - $displayOnSuccess = $this->renderViewInVariable($params['displayOnSuccess'], ['entity' => $data]); - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxSuccessResponse($this->ObjectAlias, 'add', $savedData, $message, ['displayOnSuccess' => $displayOnSuccess]); - } else { - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxSuccessResponse($this->ObjectAlias, 'add', $savedData, $message); + if (!$break) { + $savedData = $this->Table->save($data); + if ($savedData !== false) { + if (isset($params['afterSave'])) { + try { + $data = $params['afterSave']($data); + } catch (\Exception $e) { + $message = $e->getMessage(); + $break = true; + $this->__raiseErrorToUser(__('Saved {0}, but could not execute post-save actions.', $this->ObjectAlias), ['Custom checks' => ['Custom checks' => $message]]); + } + if ($data === false) { + $break = true; + $this->__raiseErrorToUser(__('Saved {0}, but the post-save actons failed.', $this->ObjectAlias)); + } + } + if (!$break) { + $message = __('{0} added.', $this->ObjectAlias); + if ($this->Controller->ParamHandler->isRest()) { + $this->Controller->restResponsePayload = $this->RestResponse->viewData($savedData, 'json'); + } else if ($this->Controller->ParamHandler->isAjax()) { + if (!empty($params['displayOnSuccess'])) { + $displayOnSuccess = $this->renderViewInVariable($params['displayOnSuccess'], ['entity' => $data]); + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxSuccessResponse($this->ObjectAlias, 'add', $savedData, $message, ['displayOnSuccess' => $displayOnSuccess]); + } else { + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxSuccessResponse($this->ObjectAlias, 'add', $savedData, $message); + } + } else { + $this->Controller->Flash->success($message); + if (empty($params['redirect'])) { + $this->Controller->redirect(['action' => 'view', $data->id]); + } else { + $this->Controller->redirect($params['redirect']); + } + } } } else { - $this->Controller->Flash->success($message); - if (empty($params['redirect'])) { - $this->Controller->redirect(['action' => 'view', $data->id]); + $this->Controller->isFailResponse = true; + $validationErrors = $data->getErrors(); + $validationMessage = $this->prepareValidationMessage($validationErrors); + $message = __( + '{0} could not be added.{1}', + $this->ObjectAlias, + empty($validationMessage) ? '' : PHP_EOL . __('Reason: {0}', $validationMessage) + ); + if ($this->Controller->ParamHandler->isRest()) { + $this->Controller->restResponsePayload = $this->RestResponse->viewData($message, 'json'); + } else if ($this->Controller->ParamHandler->isAjax()) { + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'add', $data, $message, $validationErrors); } else { - $this->Controller->redirect($params['redirect']); + $this->Controller->Flash->error($message); } } - } else { - $this->Controller->isFailResponse = true; - $validationErrors = $data->getErrors(); - $validationMessage = $this->prepareValidationMessage($validationErrors); - $message = __( - '{0} could not be added.{1}', - $this->ObjectAlias, - empty($validationMessage) ? '' : PHP_EOL . __('Reason: {0}', $validationMessage) - ); - if ($this->Controller->ParamHandler->isRest()) { - $this->Controller->restResponsePayload = $this->RestResponse->viewData($message, 'json'); - } else if ($this->Controller->ParamHandler->isAjax()) { - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'add', $data, $message, $validationErrors); - } else { - $this->Controller->Flash->error($message); - } } } if (!empty($params['fields'])) { @@ -723,6 +745,16 @@ class CRUDComponent extends Component return $input; } + private function __raiseErrorToUser($title, $message = null) + { + if ($this->Controller->ParamHandler->isRest()) { + } else if ($this->Controller->ParamHandler->isAjax()) { + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'edit', [], $title, $message); + } else { + throw new NotFoundException($message); + } + } + public function edit(int $id, array $params = []): void { if (empty($id)) { @@ -800,54 +832,79 @@ class CRUDComponent extends Component } } $data = $this->Table->patchEntity($data, $input, $patchEntityParams); + $break = false; if (isset($params['beforeSave'])) { - $data = $params['beforeSave']($data); + try { + $data = $params['beforeSave']($data); + } catch (\Exception $e) { + $message = $e->getMessage(); + $break = true; + $this->__raiseErrorToUser(__('Could not save {0}.', $this->ObjectAlias), ['Custom checks' => ['Custom checks' => $message]]); + } 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)); + $break = true; + $this->__raiseErrorToUser(__('Could not save {0} due to the input failing to meet expectations.', $this->ObjectAlias), __('Your input is bad and you should feel bad.')); } } - $savedData = $this->Table->save($data); - if ($savedData !== false) { - if ($metaFieldsEnabled && !empty($metaFieldsToDelete)) { - foreach ($metaFieldsToDelete as $k => $v) { - if ($v === null) { - unset($metaFieldsToDelete[$k]); + if (!$break) { + $savedData = $this->Table->save($data); + if ($savedData !== false) { + if ($metaFieldsEnabled && !empty($metaFieldsToDelete)) { + foreach ($metaFieldsToDelete as $k => $v) { + if ($v === null) { + unset($metaFieldsToDelete[$k]); + } + } + if (!empty($metaFieldsToDelete)) { + $this->Table->MetaFields->unlink($savedData, $metaFieldsToDelete); } } - if (!empty($metaFieldsToDelete)) { - $this->Table->MetaFields->unlink($savedData, $metaFieldsToDelete); + $break = false; + if (isset($params['afterSave'])) { + if (isset($params['afterSave'])) { + try { + $data = $params['afterSave']($data); + } catch (\Exception $e) { + $message = $e->getMessage(); + $break = true; + $this->__raiseErrorToUser(__('Saved {0} `{1}`, but could not execute post-save actions.', $this->ObjectAlias, $savedData->{$this->Table->getDisplayField()}), ['Custom checks' => ['Custom checks' => $message]]); + } + if ($data === false) { + $break = true; + $this->__raiseErrorToUser(__('Saved {0} `{1}`, but the post-save actons failed.', $this->ObjectAlias, $savedData->{$this->Table->getDisplayField()})); + } + } + } + if (!$break) { + $message = __('{0} `{1}` updated.', $this->ObjectAlias, $savedData->{$this->Table->getDisplayField()}); + if ($this->Controller->ParamHandler->isRest()) { + $this->Controller->restResponsePayload = $this->RestResponse->viewData($savedData, 'json'); + } else if ($this->Controller->ParamHandler->isAjax()) { + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxSuccessResponse($this->ObjectAlias, 'edit', $savedData, $message); + } else { + $this->Controller->Flash->success($message); + if (empty($params['redirect'])) { + $this->Controller->redirect(['action' => 'view', $id]); + } else { + $this->Controller->redirect($params['redirect']); + } + } } - } - if (isset($params['afterSave'])) { - $params['afterSave']($data); - } - $message = __('{0} `{1}` updated.', $this->ObjectAlias, $savedData->{$this->Table->getDisplayField()}); - if ($this->Controller->ParamHandler->isRest()) { - $this->Controller->restResponsePayload = $this->RestResponse->viewData($savedData, 'json'); - } else if ($this->Controller->ParamHandler->isAjax()) { - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxSuccessResponse($this->ObjectAlias, 'edit', $savedData, $message); } else { - $this->Controller->Flash->success($message); - if (empty($params['redirect'])) { - $this->Controller->redirect(['action' => 'view', $id]); + $validationErrors = $data->getErrors(); + $validationMessage = $this->prepareValidationError($data); + $message = __( + '{0} could not be modified.{1}', + $this->ObjectAlias, + empty($validationMessage) ? '' : PHP_EOL . __('Reason: {0}', $validationMessage) + ); + if ($this->Controller->ParamHandler->isRest()) { + } else if ($this->Controller->ParamHandler->isAjax()) { + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'edit', $data, $message, $validationErrors); } else { - $this->Controller->redirect($params['redirect']); + $this->Controller->Flash->error($message); } } - } else { - $validationErrors = $data->getErrors(); - $validationMessage = $this->prepareValidationError($data); - $message = __( - '{0} could not be modified.{1}', - $this->ObjectAlias, - empty($validationMessage) ? '' : PHP_EOL . __('Reason: {0}', $validationMessage) - ); - if ($this->Controller->ParamHandler->isRest()) { - } else if ($this->Controller->ParamHandler->isAjax()) { - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'edit', $data, $message, $validationErrors); - } else { - $this->Controller->Flash->error($message); - } } } if (!empty($params['fields'])) { diff --git a/src/Controller/Component/RestResponseComponent.php b/src/Controller/Component/RestResponseComponent.php index 4bc5edf..899968c 100644 --- a/src/Controller/Component/RestResponseComponent.php +++ b/src/Controller/Component/RestResponseComponent.php @@ -440,7 +440,7 @@ class RestResponseComponent extends Component return $this->viewData($response); } - public function ajaxFailResponse($ObjectAlias, $action, $entity, $message, $errors = []) + public function ajaxFailResponse($ObjectAlias, $action, $entity, $message, $errors = [], $description = '') { $action = $this->__dissectAdminRouting($action); $entity = is_array($entity) ? $entity : $entity->toArray(); @@ -448,6 +448,7 @@ class RestResponseComponent extends Component 'success' => false, 'message' => $message, 'errors' => $errors, + 'description' => $description, 'url' => !empty($entity['id']) ? $this->__generateURL($action, $ObjectAlias, $entity['id']) : '' ]; return $this->viewData($response); From ab331dcfb9fce8e09bd0a8a75e35edfb471f5417 Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 22 Nov 2024 12:47:16 +0100 Subject: [PATCH 03/18] fix: [crud] fixed the broken non ajax messages just introduced in the previous commit - can't have my cake and eat it too --- src/Controller/Component/CRUDComponent.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index e8f6fc0..24b7268 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -472,7 +472,7 @@ class CRUDComponent extends Component } catch (\Exception $e) { $message = $e->getMessage(); $break = true; - $this->__raiseErrorToUser(__('Could not save {0}.', $this->ObjectAlias), ['Custom checks' => ['Custom checks' => $message]]); + $this->__raiseErrorToUser(__('Could not save {0}.', $this->ObjectAlias), $message); } if ($data === false) { $break = true; @@ -488,7 +488,7 @@ class CRUDComponent extends Component } catch (\Exception $e) { $message = $e->getMessage(); $break = true; - $this->__raiseErrorToUser(__('Saved {0}, but could not execute post-save actions.', $this->ObjectAlias), ['Custom checks' => ['Custom checks' => $message]]); + $this->__raiseErrorToUser(__('Saved {0}, but could not execute post-save actions.', $this->ObjectAlias), $message); } if ($data === false) { $break = true; @@ -748,10 +748,12 @@ class CRUDComponent extends Component private function __raiseErrorToUser($title, $message = null) { if ($this->Controller->ParamHandler->isRest()) { + $this->Controller->restResponsePayload = $this->RestResponse->viewData($message, 'json'); } else if ($this->Controller->ParamHandler->isAjax()) { - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'edit', [], $title, $message); + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, 'edit', [], $title, ['Custom checks' => ['Custom checks' => $message]]); } else { - throw new NotFoundException($message); + $this->Controller->isFailResponse = true; + $this->Controller->Flash->error($message); } } @@ -839,7 +841,7 @@ class CRUDComponent extends Component } catch (\Exception $e) { $message = $e->getMessage(); $break = true; - $this->__raiseErrorToUser(__('Could not save {0}.', $this->ObjectAlias), ['Custom checks' => ['Custom checks' => $message]]); + $this->__raiseErrorToUser(__('Could not save {0}.', $this->ObjectAlias), $message); } if ($data === false) { $break = true; @@ -867,7 +869,7 @@ class CRUDComponent extends Component } catch (\Exception $e) { $message = $e->getMessage(); $break = true; - $this->__raiseErrorToUser(__('Saved {0} `{1}`, but could not execute post-save actions.', $this->ObjectAlias, $savedData->{$this->Table->getDisplayField()}), ['Custom checks' => ['Custom checks' => $message]]); + $this->__raiseErrorToUser(__('Saved {0} `{1}`, but could not execute post-save actions.', $this->ObjectAlias, $savedData->{$this->Table->getDisplayField()}), $message); } if ($data === false) { $break = true; From 55cac2e2e69e45108302618f17efb7e7236e9f05 Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 22 Nov 2024 12:48:37 +0100 Subject: [PATCH 04/18] new: [security] added functionality to tighten bookmark creation rules - site admins can now limit the baseurls of the provided bookmark URLs to a list of values via the server settings --- src/Controller/UserSettingsController.php | 8 ++++ .../CerebrateSettingsProvider.php | 11 +++++ src/Model/Table/UserSettingsTable.php | 45 +++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/src/Controller/UserSettingsController.php b/src/Controller/UserSettingsController.php index 877d25f..d97dbe7 100644 --- a/src/Controller/UserSettingsController.php +++ b/src/Controller/UserSettingsController.php @@ -79,6 +79,10 @@ class UserSettingsController extends AppController if (empty($currentUser['role']['perm_community_admin'])) { $data['user_id'] = $currentUser->id; } + $validationResult = $this->UserSettings->validateUserSetting($data, $currentUser); + if (!$validationResult !== true) { + throw new MethodNotAllowedException(__('You cannot create the given user setting. Reason: {0}', $validationResult)); + } return $data; } ]); @@ -131,6 +135,10 @@ class UserSettingsController extends AppController if ($data['user_id'] != $entity->user_id) { throw new MethodNotAllowedException(__('You cannot assign the setting to a different user.')); } + $validationResult = $this->UserSettings->validateUserSetting($data); + if ($validationResult !== true) { + throw new MethodNotAllowedException(__('Setting value: {0}', $validationResult)); + } return $data; } ]); diff --git a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php index 256b3a9..39def17 100644 --- a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php +++ b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php @@ -338,6 +338,17 @@ class CerebrateSettingsProvider extends BaseSettingsProvider ], ] ], + 'Restrictions' => [ + 'Allowed bookmark domains' => [ + 'security.restrictions.allowed_bookmark_domains' => [ + 'name' => __('Allowed bookmark domains'), + 'type' => 'string', + 'severity' => 'info', + 'description' => __('Comma separated list of allowed bookmark domains. Leave empty to allow all domains.'), + 'default' => '', + ], + ], + ], 'Development' => [ 'Debugging' => [ 'debug' => [ diff --git a/src/Model/Table/UserSettingsTable.php b/src/Model/Table/UserSettingsTable.php index 81cec2c..0bbde9a 100644 --- a/src/Model/Table/UserSettingsTable.php +++ b/src/Model/Table/UserSettingsTable.php @@ -5,6 +5,7 @@ namespace App\Model\Table; use App\Model\Table\AppTable; use Cake\ORM\Table; use Cake\Validation\Validator; +use Cake\Core\Configure; require_once(APP . 'Model' . DS . 'Table' . DS . 'SettingProviders' . DS . 'UserSettingsProvider.php'); use App\Settings\SettingsProvider\UserSettingsProvider; @@ -102,6 +103,14 @@ class UserSettingsTable extends AppTable 'name' => $data['bookmark_name'], 'url' => $data['bookmark_url'], ]; + $restricted_domains = Configure::read('Security.restrictions.allowed_bookmark_domains'); + if (!empty($restricted_domains)) { + $restricted_domains = explode(',', $restricted_domains); + $parsed = parse_url($bookmarkData['url']); + if (!in_array($parsed['host'], $restricted_domains)) { + return null; + } + } if (is_null($setting)) { // setting not found, create it $bookmarksData = json_encode([$bookmarkData]); $result = $this->createSetting($user, $this->BOOKMARK_SETTING_NAME, $bookmarksData); @@ -153,4 +162,40 @@ class UserSettingsTable extends AppTable } return $isLocalPath || $isValidURL; } + + public function validateUserSetting($data): bool|string + { + $errors = []; + $json_fields = ['ui.bookmarks']; + if (in_array($data['name'], $json_fields)) { + $decoded = json_decode($data['value'], true); + if (json_last_error() !== JSON_ERROR_NONE) { + return __('Invalid JSON data'); + } + $value = $decoded; + } else { + $value = $data['value']; + } + if ($data['name'] === 'ui.bookmarks') { + if (array_values($value) !== $value) { + $value = [$value]; + } + $restricted_domains = Configure::read('security.restrictions.allowed_bookmark_domains'); + if (!empty($restricted_domains)) { + $restricted_domains = explode(',', $restricted_domains); + foreach ($restricted_domains as &$rd) { + $rd = trim($rd); + } + } + foreach ($value as $bookmark) { + if (!empty($restricted_domains)) { + $parsed = parse_url($bookmark['url']); + if (!in_array($parsed['host'], $restricted_domains)) { + return __('Invalid domain for bookmark. The only domains allowed are: {0}', implode(', ', $restricted_domains)); + } + } + } + } + return true; + } } From 850d559cef2ba59e5cc1719dd78dbd1105c76506 Mon Sep 17 00:00:00 2001 From: Jeroen Pinoy Date: Fri, 22 Nov 2024 22:40:25 +0100 Subject: [PATCH 05/18] fix: typo in read security allowed bookmark domains config --- src/Model/Table/UserSettingsTable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Table/UserSettingsTable.php b/src/Model/Table/UserSettingsTable.php index 0bbde9a..0fc1dcc 100644 --- a/src/Model/Table/UserSettingsTable.php +++ b/src/Model/Table/UserSettingsTable.php @@ -103,7 +103,7 @@ class UserSettingsTable extends AppTable 'name' => $data['bookmark_name'], 'url' => $data['bookmark_url'], ]; - $restricted_domains = Configure::read('Security.restrictions.allowed_bookmark_domains'); + $restricted_domains = Configure::read('security.restrictions.allowed_bookmark_domains'); if (!empty($restricted_domains)) { $restricted_domains = explode(',', $restricted_domains); $parsed = parse_url($bookmarkData['url']); From a1020bc42b2279e5033f1b6d81b0e35c01424e39 Mon Sep 17 00:00:00 2001 From: Jeroen Pinoy Date: Fri, 22 Nov 2024 23:48:52 +0100 Subject: [PATCH 06/18] fix: users settings view throws internal server error when accessed without user id --- src/Controller/UsersController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index 4159d96..238920a 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -487,7 +487,7 @@ class UsersController extends AppController { $editingAnotherUser = false; $currentUser = $this->ACL->getUser(); - if ((empty($currentUser['role']['perm_community_admin']) && empty($currentUser['role']['perm_group_admin'])) || $user_id == $currentUser->id) { + if ((empty($currentUser['role']['perm_community_admin']) && empty($currentUser['role']['perm_group_admin'])) || empty($user_id) || $user_id == $currentUser->id) { $user = $currentUser; } else { $user = $this->Users->get($user_id, [ From ed592c57c71707587c5a2fc77bd097b58a7a80fc Mon Sep 17 00:00:00 2001 From: Jeroen Pinoy Date: Sat, 23 Nov 2024 20:08:41 +0100 Subject: [PATCH 07/18] fix: default admin role doesn't have group admin and meta field editor permissions --- src/Model/Table/UsersTable.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Model/Table/UsersTable.php b/src/Model/Table/UsersTable.php index 14c4750..61f027e 100644 --- a/src/Model/Table/UsersTable.php +++ b/src/Model/Table/UsersTable.php @@ -221,7 +221,9 @@ class UsersTable extends AppTable 'perm_admin' => 1, 'perm_community_admin' => 1, 'perm_org_admin' => 1, - 'perm_sync' => 1 + 'perm_sync' => 1, + 'perm_group_admin' => 1, + 'perm_meta_field_editor' => 1 ]); $this->Roles->save($role); $this->Organisations = TableRegistry::get('Organisations'); From 0131422ab84cdf0719cc47efa6001276fbce88fd Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 09:17:24 +0100 Subject: [PATCH 08/18] fix: [security] Tightening of the role assignment permissions - If a decoupled perm_admin role was configured on the system, this could be assigned by low privilege administrators, leading to privilege escalation - This fix moves the responsibility of the check to the ACL component rather than the controller - As reported by Jeroen Pinoy (@wachizungu) --- src/Controller/Component/ACLComponent.php | 16 ++++++++++++---- src/Controller/UsersController.php | 8 ++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/Controller/Component/ACLComponent.php b/src/Controller/Component/ACLComponent.php index 484e2de..0178a1e 100644 --- a/src/Controller/Component/ACLComponent.php +++ b/src/Controller/Component/ACLComponent.php @@ -363,11 +363,19 @@ class ACLComponent extends Component return true; } - if ($user['role']['perm_community_admin']) { - return false; // org_admins cannot edit admins + $this->Roles = TableRegistry::get('Roles'); + $validRoles = []; + if (!$currentUser['role']['perm_community_admin']) { + if ($currentUser['role']['perm_group_admin']) { + $validRoles = $this->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_admin' => 0])->all()->toArray(); + } else { + $validRoles = $this->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0, 'perm_admin' => 0])->all()->toArray(); + } + } else { + $validRoles = $this->Roles->find('list')->order(['name' => 'asc'])->all()->toArray(); } - if ($currentUser['role']['perm_org_admin'] && $user['role']['perm_group_admin']) { - return false; // org_admins cannot edit group_admin + if (!in_array($user['role_id'], array_keys($validRoles)) && $currentUser['id'] != $user['id']) { + return false; } if ($currentUser['role']['perm_group_admin']) { $this->OrgGroups = TableRegistry::get('OrgGroups'); diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index 4159d96..e7577e4 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -86,10 +86,10 @@ class UsersController extends AppController $individual_ids = []; if (!$currentUser['role']['perm_community_admin']) { if ($currentUser['role']['perm_group_admin']) { - $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0])->all()->toArray(); + $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_admin' => 0])->all()->toArray(); $individual_ids = $this->Users->Individuals->find('aligned', ['organisation_id' => $currentUser['organisation_id']])->all()->extract('id')->toArray(); } else { - $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0])->all()->toArray(); + $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0, 'perm_admin' => 0])->all()->toArray(); } if (empty($individual_ids)) { @@ -247,10 +247,10 @@ class UsersController extends AppController $validOrgIds = []; if (!$currentUser['role']['perm_community_admin']) { if ($currentUser['role']['perm_group_admin']) { - $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0])->all()->toArray(); + $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_admin' => 0])->all()->toArray(); $validOrgIds = $this->Users->Organisations->OrgGroups->getGroupOrgIdsForUser($currentUser); } else { - $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0])->all()->toArray(); + $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0, 'perm_admin' => 0])->all()->toArray(); } } else { $validRoles = $this->Users->Roles->find('list')->order(['name' => 'asc'])->all()->toArray(); From 07f67fe9eaf37481ceb711c8a0fe20c0ed30a418 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 09:37:06 +0100 Subject: [PATCH 09/18] fix: [cleanup] ACL component - duplicate check removed --- src/Controller/Component/ACLComponent.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Controller/Component/ACLComponent.php b/src/Controller/Component/ACLComponent.php index 0178a1e..3f7ba20 100644 --- a/src/Controller/Component/ACLComponent.php +++ b/src/Controller/Component/ACLComponent.php @@ -386,9 +386,6 @@ class ACLComponent extends Component if (!$currentUser['role']['perm_org_admin']) { return false; } else { - if ($currentUser['id'] == $user['id']) { - return true; - } if ($currentUser['organisation_id'] === $user['organisation_id']) { return true; } From da4bd943b7df42455964f7731aed05859609da29 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 16:46:45 +0100 Subject: [PATCH 10/18] fix: [typo] in the authkeyscontroller - lead to users not being able to generate authkeys --- src/Controller/AuthKeysController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Controller/AuthKeysController.php b/src/Controller/AuthKeysController.php index 94e47d2..083bd5c 100644 --- a/src/Controller/AuthKeysController.php +++ b/src/Controller/AuthKeysController.php @@ -71,7 +71,7 @@ class AuthKeysController extends AppController if (empty($currentUser['role']['perm_org_admin'])) { $userConditions['id'] = $currentUser['id']; } else { - $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin', 'perm_org_admin' => 0])->all()->extract('id')->toList(); + $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin' => 0, 'perm_org_admin' => 0])->all()->extract('id')->toList(); $userConditions['organisation_id'] = $currentUser['organisation_id']; $userConditions['OR'] = [ ['role_id IN' => $role_ids], @@ -84,6 +84,7 @@ class AuthKeysController extends AppController $users->where($userConditions); } $users = $users->order(['username' => 'asc'])->all()->toArray(); + $this->CRUD->add([ 'displayOnSuccess' => 'authkey_display', 'beforeSave' => function($data) use ($users) { From 1c8bcc045eb2cc2a295dd5382b56bbbf82299109 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 16:56:51 +0100 Subject: [PATCH 11/18] fix: [security] Group admin ACL - group admin can inject user into organisation not managed by themselves - as reported by Jeroen Pinoy (@wachizungu) --- src/Controller/UsersController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index 16082ee..0e3ba37 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -320,10 +320,13 @@ class UsersController extends AppController } return $data; }; - $params['beforeSave'] = function ($data) use ($currentUser, $validRoles) { + $params['beforeSave'] = function ($data) use ($currentUser, $validRoles, $validOrgIds) { if (!in_array($data['role_id'], array_keys($validRoles)) && $this->ACL->getUser()['id'] != $data['id']) { throw new MethodNotAllowedException(__('You cannot assign the chosen role to a user.')); } + if (!in_array($data['organisation_id'], $validOrgIds)) { + throw new MethodNotAllowedException(__('You cannot assign the chosen organisation to a user.')); + } return $data; }; } From d799214a415bfa64744f7b9a0837e9444e75b7fb Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 17:13:32 +0100 Subject: [PATCH 12/18] fix: [error] when deleting a role that had users attached to it was cryptic, fixes #180 --- src/Controller/Component/CRUDComponent.php | 3 ++- src/Controller/RolesController.php | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 24b7268..e7092a1 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -1165,6 +1165,7 @@ class CRUDComponent extends Component 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)); } } catch (\Exception $e) { + $exceptionMessage = $e->getMessage(); $entity = false; } } @@ -1182,7 +1183,7 @@ class CRUDComponent extends Component $isBulk, __('{0} deleted.', $this->ObjectAlias), __('All selected {0} have been deleted.', Inflector::pluralize($this->ObjectAlias)), - __('Could not delete {0}.', $this->ObjectAlias), + $exceptionMessage ?? __('Could not delete {0}.', $this->ObjectAlias), __( '{0} / {1} {2} have been deleted.', $bulkSuccesses, diff --git a/src/Controller/RolesController.php b/src/Controller/RolesController.php index 9e909c1..e4c8705 100644 --- a/src/Controller/RolesController.php +++ b/src/Controller/RolesController.php @@ -78,7 +78,15 @@ class RolesController extends AppController public function delete($id) { - $this->CRUD->delete($id); + $this->CRUD->delete($id, [ + 'beforeSave' => function ($data) { + $userCount = $this->Roles->Users->find()->where(['role_id' => $data['id']])->count(); + if ($userCount > 0) { + throw new ForbiddenException(__('You cannot delete a role that has users assigned to it.')); + } + return true; + } + ]); $responsePayload = $this->CRUD->getResponsePayload(); if (!empty($responsePayload)) { return $responsePayload; From cce411541811039e857ddacd2cbb37e68865db95 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 17:47:43 +0100 Subject: [PATCH 13/18] fix: [error handling] better error handling for bookmarks, fixes #188 - show why something failed - actually fail if a field is missing for bookmarks --- src/Controller/Component/CRUDComponent.php | 2 +- .../Component/RestResponseComponent.php | 8 ++++++-- src/Controller/UserSettingsController.php | 5 +++-- src/Model/Table/UserSettingsTable.php | 19 +++++++++++++++++-- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index e7092a1..ed374fe 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -1365,7 +1365,7 @@ class CRUDComponent extends Component if (!empty($additionalData['redirect'])) { // If a redirection occurs, we need to make sure the flash message gets displayed $this->Controller->Flash->error($message); } - $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, $action, $data, $message, !is_null($errors) ? $errors : $data->getErrors()); + $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, $action, $data, $message, $errors); } else { $this->Controller->Flash->error($message); $this->Controller->redirect($this->Controller->referer()); diff --git a/src/Controller/Component/RestResponseComponent.php b/src/Controller/Component/RestResponseComponent.php index 899968c..ecaabba 100644 --- a/src/Controller/Component/RestResponseComponent.php +++ b/src/Controller/Component/RestResponseComponent.php @@ -440,10 +440,14 @@ class RestResponseComponent extends Component return $this->viewData($response); } - public function ajaxFailResponse($ObjectAlias, $action, $entity, $message, $errors = [], $description = '') + public function ajaxFailResponse($ObjectAlias, $action, $entity = null, $message, $errors = [], $description = '') { $action = $this->__dissectAdminRouting($action); - $entity = is_array($entity) ? $entity : $entity->toArray(); + if (empty($entity)) { + $entity = []; + } else { + $entity = is_array($entity) ? $entity : $entity->toArray(); + } $response = [ 'success' => false, 'message' => $message, diff --git a/src/Controller/UserSettingsController.php b/src/Controller/UserSettingsController.php index d97dbe7..54e7e62 100644 --- a/src/Controller/UserSettingsController.php +++ b/src/Controller/UserSettingsController.php @@ -243,9 +243,10 @@ class UserSettingsController extends AppController public function saveMyBookmark() { if (!$this->request->is('get')) { - $result = $this->UserSettings->saveBookmark($this->ACL->getUser(), $this->request->getData()); + $errors = null; + $result = $this->UserSettings->saveBookmark($this->ACL->getUser(), $this->request->getData(), $errors); $success = !empty($result); - $message = $success ? __('Bookmark saved') : __('Could not save bookmark'); + $message = $success ? __('Bookmark saved') : ($errors ?? __('Could not save bookmark')); $this->CRUD->setResponseForController('saveBookmark', $success, $message, $result); $responsePayload = $this->CRUD->getResponsePayload(); if (!empty($responsePayload)) { diff --git a/src/Model/Table/UserSettingsTable.php b/src/Model/Table/UserSettingsTable.php index 0fc1dcc..21c5a84 100644 --- a/src/Model/Table/UserSettingsTable.php +++ b/src/Model/Table/UserSettingsTable.php @@ -95,9 +95,24 @@ class UserSettingsTable extends AppTable return $savedData; } - public function saveBookmark($user, $data) + public function saveBookmark($user, $data, &$message = null) { $setting = $this->getSettingByName($user, $this->BOOKMARK_SETTING_NAME); + $fieldsToCheck = [ + 'bookmark_label' => __('Label'), + 'bookmark_name' => __('Name'), + 'bookmark_url' => __('URL') + ]; + foreach ($fieldsToCheck as $field => $fieldName) { + if (empty($data[$field])) { + $message = __('Please fill in all fields, {0} missing.', $fieldName); + return null; + } + } + if (empty($data['bookmark_label']) || empty($data['bookmark_name']) || empty($data['bookmark_url'])) { + + return null; + } $bookmarkData = [ 'label' => $data['bookmark_label'], 'name' => $data['bookmark_name'], @@ -107,7 +122,7 @@ class UserSettingsTable extends AppTable if (!empty($restricted_domains)) { $restricted_domains = explode(',', $restricted_domains); $parsed = parse_url($bookmarkData['url']); - if (!in_array($parsed['host'], $restricted_domains)) { + if (!empty($parsed['host']) && !in_array($parsed['host'], $restricted_domains)) { return null; } } From 04b640c8b6bf7d583bb7e08b2ca3ca362dd840b0 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 18:00:13 +0100 Subject: [PATCH 14/18] fix: [diagnostics] allow for certain settings to be empty, fixes #176 - via the empty => true key --- .../SettingProviders/BaseSettingsProvider.php | 26 +++++++++++-------- .../CerebrateSettingsProvider.php | 4 +++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/Model/Table/SettingProviders/BaseSettingsProvider.php b/src/Model/Table/SettingProviders/BaseSettingsProvider.php index 7bb6442..df92747 100644 --- a/src/Model/Table/SettingProviders/BaseSettingsProvider.php +++ b/src/Model/Table/SettingProviders/BaseSettingsProvider.php @@ -163,18 +163,22 @@ class BaseSettingsProvider $setting['error'] = false; if (!$skipValidation) { $validationResult = true; - if (!isset($setting['value'])) { - $validationResult = $this->settingValidator->testEmptyBecomesDefault(null, $setting); - } else if (isset($setting['test'])) { - $setting['value'] = $setting['value'] ?? ''; - $validationResult = $this->evaluateFunctionForSetting($setting['test'], $setting); - } - if ($validationResult !== true) { - $setting['severity'] = $setting['severity'] ?? 'warning'; - if (!in_array($setting['severity'], $this->severities)) { - $setting['severity'] = 'warning'; + if (empty($setting['value']) && !empty($setting['empty'])) { + $validationResult = true; + } else { + if (!isset($setting['value'])) { + $validationResult = $this->settingValidator->testEmptyBecomesDefault(null, $setting); + } else if (isset($setting['test'])) { + $setting['value'] = $setting['value'] ?? ''; + $validationResult = $this->evaluateFunctionForSetting($setting['test'], $setting); + } + if ($validationResult !== true) { + $setting['severity'] = $setting['severity'] ?? 'warning'; + if (!in_array($setting['severity'], $this->severities)) { + $setting['severity'] = 'warning'; + } + $setting['errorMessage'] = $validationResult; } - $setting['errorMessage'] = $validationResult; } $setting['error'] = $validationResult !== true ? true : false; } diff --git a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php index d073e92..61f6993 100644 --- a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php +++ b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php @@ -140,6 +140,7 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'type' => 'string', 'description' => __('The hostname of an HTTP proxy for outgoing sync requests. Leave empty to not use a proxy.'), 'test' => 'testHostname', + 'empty' => true ], 'Proxy.port' => [ 'name' => __('Port'), @@ -147,6 +148,7 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'type' => 'integer', 'description' => __('The TCP port for the HTTP proxy.'), 'test' => 'testForRangeXY', + 'empty' => true ], 'Proxy.user' => [ 'name' => __('User'), @@ -154,6 +156,7 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'description' => __('The authentication username for the HTTP proxy.'), 'default' => 'admin', 'dependsOn' => 'proxy.host', + 'empty' => true ], 'Proxy.password' => [ 'name' => __('Password'), @@ -161,6 +164,7 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'description' => __('The authentication password for the HTTP proxy.'), 'default' => '', 'dependsOn' => 'proxy.host', + 'empty' => true ], ], ], From cfceaf0fb78aa021ac9aa2c1e7c1e40256636ecc Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 20:42:27 +0100 Subject: [PATCH 15/18] fix: [authkeys] don't barf if no valid roles exist --- src/Controller/AuthKeysController.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Controller/AuthKeysController.php b/src/Controller/AuthKeysController.php index 083bd5c..e57710e 100644 --- a/src/Controller/AuthKeysController.php +++ b/src/Controller/AuthKeysController.php @@ -72,6 +72,9 @@ class AuthKeysController extends AppController $userConditions['id'] = $currentUser['id']; } else { $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin' => 0, 'perm_org_admin' => 0])->all()->extract('id')->toList(); + if (empty($role_ids)) { + throw new MethodNotAllowedException(__('You are not authorised to do that, as there are no roles that you could assign to a user. Contact your administrator to rectify this.')); + } $userConditions['organisation_id'] = $currentUser['organisation_id']; $userConditions['OR'] = [ ['role_id IN' => $role_ids], From 0ed3bef0009f7d654a965c5291ebdd4d16a66871 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 20:44:51 +0100 Subject: [PATCH 16/18] chg: [internal] authkey adding, more elegant solution --- src/Controller/AuthKeysController.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Controller/AuthKeysController.php b/src/Controller/AuthKeysController.php index e57710e..be4434b 100644 --- a/src/Controller/AuthKeysController.php +++ b/src/Controller/AuthKeysController.php @@ -72,14 +72,14 @@ class AuthKeysController extends AppController $userConditions['id'] = $currentUser['id']; } else { $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin' => 0, 'perm_org_admin' => 0])->all()->extract('id')->toList(); - if (empty($role_ids)) { - throw new MethodNotAllowedException(__('You are not authorised to do that, as there are no roles that you could assign to a user. Contact your administrator to rectify this.')); - } $userConditions['organisation_id'] = $currentUser['organisation_id']; - $userConditions['OR'] = [ - ['role_id IN' => $role_ids], - ['id' => $currentUser['id']], + $subConditions = [ + ['id' => $currentUser['id']] ]; + if (!empty($role_ids)) { + $subConditions[] = ['role_id IN' => $role_ids]; + } + $userConditions['OR'] = $subConditions; } } $users = $this->Users->find('list'); From 1572681307049f7604211f920632b8581234c664 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 21:28:24 +0100 Subject: [PATCH 17/18] fix: [authkeys] better permission / listing handling - allow group admins to manage api keys of their group - when adding an authkey from the user view, don't list every user in the dropdown, focus on the selected user --- src/Controller/AuthKeysController.php | 46 +++++++++------------- src/Controller/Component/CRUDComponent.php | 4 ++ src/Model/Table/AuthKeysTable.php | 32 +++++++++++++++ templates/AuthKeys/index.php | 4 +- 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/Controller/AuthKeysController.php b/src/Controller/AuthKeysController.php index be4434b..e65827b 100644 --- a/src/Controller/AuthKeysController.php +++ b/src/Controller/AuthKeysController.php @@ -22,20 +22,29 @@ class AuthKeysController extends AppController { $currentUser = $this->ACL->getUser(); $conditions = []; + $userId = $this->request->getQuery('Users_id'); + if (!empty($userId)) { + $conditions['AND']['Users.id'] = $userId; + } + if (empty($currentUser['role']['perm_community_admin'])) { $conditions['Users.organisation_id'] = $currentUser['organisation_id']; if (empty($currentUser['role']['perm_org_admin'])) { $conditions['Users.id'] = $currentUser['id']; } } - $this->CRUD->index([ + $indexOptions = [ 'filters' => $this->filterFields, 'quickFilters' => $this->quickFilterFields, 'contain' => $this->containFields, 'exclude_fields' => ['authkey'], 'conditions' => $conditions, 'hidden' => [] - ]); + ]; + if (!empty($userId)) { + $indexOptions['action_query_strings'] = ['Users.id' => $userId]; + } + $this->CRUD->index($indexOptions); $responsePayload = $this->CRUD->getResponsePayload(); if (!empty($responsePayload)) { return $responsePayload; @@ -46,13 +55,7 @@ class AuthKeysController extends AppController public function delete($id) { $currentUser = $this->ACL->getUser(); - $conditions = []; - if (empty($currentUser['role']['perm_community_admin'])) { - $conditions['Users.organisation_id'] = $currentUser['organisation_id']; - if (empty($currentUser['role']['perm_org_admin'])) { - $conditions['Users.id'] = $currentUser['id']; - } - } + $conditions = $this->AuthKeys->buildUserConditions($currentUser); $this->CRUD->delete($id, ['conditions' => $conditions, 'contain' => 'Users']); $responsePayload = $this->CRUD->getResponsePayload(); if (!empty($responsePayload)) { @@ -67,27 +70,16 @@ class AuthKeysController extends AppController $validUsers = []; $userConditions = []; $currentUser = $this->ACL->getUser(); - if (empty($currentUser['role']['perm_community_admin'])) { - if (empty($currentUser['role']['perm_org_admin'])) { - $userConditions['id'] = $currentUser['id']; - } else { - $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin' => 0, 'perm_org_admin' => 0])->all()->extract('id')->toList(); - $userConditions['organisation_id'] = $currentUser['organisation_id']; - $subConditions = [ - ['id' => $currentUser['id']] - ]; - if (!empty($role_ids)) { - $subConditions[] = ['role_id IN' => $role_ids]; - } - $userConditions['OR'] = $subConditions; - } - } + $conditions = $this->AuthKeys->buildUserConditions($currentUser); + $userId = $this->request->getQuery('Users_id'); $users = $this->Users->find('list'); - if (!empty($userConditions)) { - $users->where($userConditions); + if (!empty($conditions)) { + $users->where($conditions); + } + if (!empty($userId)) { + $users->where(['Users.id' => $userId]); } $users = $users->order(['username' => 'asc'])->all()->toArray(); - $this->CRUD->add([ 'displayOnSuccess' => 'authkey_display', 'beforeSave' => function($data) use ($users) { diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index ed374fe..558b439 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -272,6 +272,10 @@ class CRUDComponent extends Component $this->Controller->set('model', $this->Table); $this->Controller->set('data', $data); $this->Controller->set('embedInModal', $embedInModal); + if (!empty($options['action_query_strings'])) { + $this->Controller->set('action_query_strings', $options['action_query_strings']); + + } $this->Controller->set('skipTableToolbar', $skipTableToolbar); } } diff --git a/src/Model/Table/AuthKeysTable.php b/src/Model/Table/AuthKeysTable.php index b7968b0..c2cf1c9 100644 --- a/src/Model/Table/AuthKeysTable.php +++ b/src/Model/Table/AuthKeysTable.php @@ -93,4 +93,36 @@ class AuthKeysTable extends AppTable } return []; } + + public function buildUserConditions($currentUser) + { + $conditions = []; + $validOrgs = $this->Users->getValidOrgsForUser($currentUser); + if (empty($currentUser['role']['perm_community_admin'])) { + $conditions['Users.organisation_id IN'] = $validOrgs; + if (empty($currentUser['role']['perm_group_admin'])) { + if (empty($currentUser['role']['perm_org_admin'])) { + $conditions['Users.id'] = $currentUser['id']; + } else { + $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin' => 0, 'perm_org_admin' => 0, 'perm_group_admin' => 0])->all()->extract('id')->toList(); + $conditions['Users.organisation_id'] = $currentUser['organisation_id']; + $subConditions = [ + ['Users.id' => $currentUser['id']] + ]; + if (!empty($role_ids)) { + $subConditions[] = ['Users.role_id IN' => $role_ids]; + } + $conditions['OR'] = $subConditions; + } + } else { + $conditions['Users.group_id'] = $currentUser['group_id']; + $role_ids = $this->Users->Roles->find()->where(['perm_admin' => 0, 'perm_community_admin' => 0, 'perm_group_admin' => 0])->all()->extract('id')->toList(); + $conditions['OR'] = [ + ['Users.id' => $currentUser['id']], + ['Users.role_id IN' => $role_ids] + ]; + } + } + return $conditions; + } } diff --git a/templates/AuthKeys/index.php b/templates/AuthKeys/index.php index 1b5599a..d240def 100644 --- a/templates/AuthKeys/index.php +++ b/templates/AuthKeys/index.php @@ -10,8 +10,8 @@ echo $this->element('genericElements/IndexTable/index_table', [ 'data' => [ 'type' => 'simple', 'text' => __('Add authentication key'), - 'popover_url' => '/authKeys/add', - 'reload_url' => $this->request->getRequestTarget() + 'popover_url' => '/authKeys/add' . ($action_query_strings ? '?' . http_build_query($action_query_strings) : ''), + 'reload_url' => $this->request->getRequestTarget(), ] ] ], From 9c54a4842f44c9adb2b4f878380e3a3144c70a82 Mon Sep 17 00:00:00 2001 From: iglocska Date: Thu, 28 Nov 2024 21:30:14 +0100 Subject: [PATCH 18/18] chg: [version] bump --- src/VERSION.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VERSION.json b/src/VERSION.json index d14f978..a769bcc 100644 --- a/src/VERSION.json +++ b/src/VERSION.json @@ -1,4 +1,4 @@ { - "version": "1.24", + "version": "1.26", "application": "Cerebrate" }