From c2bff491852d3142868f7729d32c1f4c212c853f Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 11 Nov 2022 15:08:23 +0100 Subject: [PATCH 1/4] fix: [beforesave] hook removed on get requests --- src/Controller/Component/CRUDComponent.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 5216086..7699417 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -787,12 +787,6 @@ 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); From f6f94983e4ebae429393c56bc94a4a94e3dd1e0e Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 11 Nov 2022 15:08:56 +0100 Subject: [PATCH 2/4] fix: [users] several fixes - User enrollment in KC moved to the aftersave (we consider cerebrate to be authoritative) - adhere to restriction parameters in deletion --- src/Controller/UsersController.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index 8ac1735..8de4c92 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -96,8 +96,12 @@ class UsersController extends AppController throw new MethodNotAllowedException(__('Invalid individual selected - when KeyCloak is enabled, only one user account may be assigned to an individual.')); } } - $this->Users->enrollUserRouter($data); return $data; + }, + 'afterSave' => function($data) { + if (Configure::read('keycloak.enabled')) { + $this->Users->enrollUserRouter($data); + } } ]); $responsePayload = $this->CRUD->getResponsePayload(); @@ -282,16 +286,21 @@ class UsersController extends AppController 'beforeSave' => function($data) use ($currentUser, $validRoles) { if (!$currentUser['role']['perm_admin']) { if ($data['organisation_id'] !== $currentUser['organisation_id']) { - throw new MethodNotAllowedException(__('You do not have permission to remove the given user.')); + throw new MethodNotAllowedException(__('You do not have permission to delete the given user.')); } if (!in_array($data['role_id'], array_keys($validRoles))) { - throw new MethodNotAllowedException(__('You do not have permission to remove the given user.')); + throw new MethodNotAllowedException(__('You do not have permission to delete the given user.')); + } + } + if (Configure::read('keycloak.enabled')) { + if (!$this->Users->deleteUser($data)) { + throw new MethodNotAllowedException(__('Could not delete the user from KeyCloak. Please try again later, or consider disabling the user instead.')); } } return $data; } ]; - $this->CRUD->delete($id); + $this->CRUD->delete($id, $params); $responsePayload = $this->CRUD->getResponsePayload(); if (!empty($responsePayload)) { return $responsePayload; From 6d41622129fe678cc724208daf37bb8e0185ab73 Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 11 Nov 2022 15:10:04 +0100 Subject: [PATCH 3/4] new: [user deletion] tied into KeyCloak - remove user from KC when possible - proceed for local users --- src/Model/Behavior/AuthKeycloakBehavior.php | 56 +++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/Model/Behavior/AuthKeycloakBehavior.php b/src/Model/Behavior/AuthKeycloakBehavior.php index 36abea2..2104bda 100644 --- a/src/Model/Behavior/AuthKeycloakBehavior.php +++ b/src/Model/Behavior/AuthKeycloakBehavior.php @@ -84,6 +84,62 @@ class AuthKeycloakBehavior extends Behavior ); } + public function getUserIdByUsername(string $username) + { + $response = $this->restApiRequest( + '%s/admin/realms/%s/users/?username=' . urlencode($username), + [], + 'GET' + ); + if (!$response->isOk()) { + $responseBody = json_decode($response->getStringBody(), true); + $this->_table->auditLogs()->insert([ + 'request_action' => 'keycloakGetUser', + 'model' => 'User', + 'model_id' => 0, + 'model_title' => __('Failed to fetch user ({0}) from keycloak', $username), + 'changed' => ['error' => empty($responseBody['errorMessage']) ? 'Unknown error.' : $responseBody['errorMessage']] + ]); + } + $responseBody = json_decode($response->getStringBody(), true); + if (empty($responseBody[0]['id'])) { + return false; + } + return $responseBody[0]['id']; + } + + public function deleteUser($data): bool + { + $userId = $this->getUserIdByUsername($data['username']); + if ($userId === false) { + $this->_table->auditLogs()->insert([ + 'request_action' => 'keycloakUserDeletion', + 'model' => 'User', + 'model_id' => 0, + 'model_title' => __('User {0} not found in keycloak, deleting the user locally.', $data['username']), + 'changed' => [] + ]); + return true; + } + $response = $this->restApiRequest( + '%s/admin/realms/%s/users/' . urlencode($userId), + [], + 'delete' + ); + if (!$response->isOk()) { + $responseBody = json_decode($response->getStringBody(), true); + $this->_table->auditLogs()->insert([ + 'request_action' => 'keycloakUserDeletion', + 'model' => 'User', + 'model_id' => 0, + 'model_title' => __('Failed to delete user {0} ({1}) in keycloak', $data['username'], $userId), + 'changed' => ['error' => empty($responseBody['errorMessage']) ? 'Unknown error.' : $responseBody['errorMessage']] + ]); + return false; + } + return true; + } + public function enrollUser($data): bool { $roleConditions = [ From ab5cee58ad82471f0c3ab387f86105cd143f13ee Mon Sep 17 00:00:00 2001 From: iglocska Date: Fri, 11 Nov 2022 15:30:55 +0100 Subject: [PATCH 4/4] fix: [crud] speculative fix for notice error on metatemplates being accessed that aren't loaded --- src/Controller/Component/CRUDComponent.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 7699417..1808f42 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -674,6 +674,7 @@ class CRUDComponent extends Component if (!empty($pruneEmptyDisabled) && !$metaTemplate->enabled) { unset($metaTemplates[$i]); } + continue; } $newestTemplate = $this->MetaTemplates->getNewestVersion($metaTemplate); if (!empty($newestTemplate) && !empty($metaTemplates[$i])) {