From 672847b2140cd08b5a3ce62af6164e2d84cfc0b3 Mon Sep 17 00:00:00 2001 From: Sami Mokaddem Date: Wed, 13 Sep 2023 09:15:16 +0200 Subject: [PATCH] chg: [users:acl] Improved waterfall model for CRUD operation and updated UI to reflect them --- src/Controller/Component/ACLComponent.php | 45 ++++++++++--------- src/Controller/Component/Navigation/Users.php | 18 ++++++++ src/Controller/UsersController.php | 15 ++++--- templates/Users/index.php | 27 ++--------- 4 files changed, 57 insertions(+), 48 deletions(-) diff --git a/src/Controller/Component/ACLComponent.php b/src/Controller/Component/ACLComponent.php index a82964e..729f81e 100644 --- a/src/Controller/Component/ACLComponent.php +++ b/src/Controller/Component/ACLComponent.php @@ -352,31 +352,36 @@ class ACLComponent extends Component if (empty($user) || empty($currentUser)) { return false; } + if ($currentUser['role']['perm_admin']) { + return true; + } if ($user['id'] === $currentUser['id']) { return true; } - if (!$currentUser['role']['perm_admin']) { - if ($user['role']['perm_admin']) { - return false; // org_admins cannot edit admins - } - if ($currentUser['role']['perm_group_admin']) { - $this->OrgGroups = TableRegistry::get('OrgGroups'); - if ($this->OrgGroups->checkIfUserBelongsToGroupAdminsGroup($currentUser, $user)) { - return true; - } - } - if (!$currentUser['role']['perm_org_admin']) { - return false; - } else { - if ($currentUser['id'] == $user['id']) { - return true; - } - if ($currentUser['organisation_id'] !== $user['organisation_id']) { - return false; - } + + if ($user['role']['perm_admin']) { + return false; // org_admins cannot edit admins + } + if ($currentUser['role']['perm_org_admin'] && $user['role']['perm_group_admin']) { + return false; // org_admins cannot edit group_admin + } + if ($currentUser['role']['perm_group_admin']) { + $this->OrgGroups = TableRegistry::get('OrgGroups'); + if ($this->OrgGroups->checkIfUserBelongsToGroupAdminsGroup($currentUser, $user)) { + return true; } } - return true; + 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; + } + } + return false; } /* diff --git a/src/Controller/Component/Navigation/Users.php b/src/Controller/Component/Navigation/Users.php index c14ec70..06536bd 100644 --- a/src/Controller/Component/Navigation/Users.php +++ b/src/Controller/Component/Navigation/Users.php @@ -103,5 +103,23 @@ class UsersNavigation extends BaseNavigation $this->bcf->addSelfLink('Users', 'settings', [ 'label' => __('Account settings') ]); + + $controller = 'Users'; + if (empty($this->viewVars['canEdit'])) { + $this->bcf->removeLink($controller, 'view', $controller, 'edit'); + $this->bcf->removeLink($controller, 'edit', $controller, 'edit'); + } + } + + public function addActions() + { + $controller = 'Users'; + if ( + empty($this->viewVars['canEdit']) || + (!empty($this->viewVars['entity']) && $this->viewVars['loggedUser']['id'] == $this->viewVars['entity']['id']) + ) { + $this->bcf->removeAction($controller, 'view', $controller, 'delete'); + $this->bcf->removeAction($controller, 'edit', $controller, 'delete'); + } } } diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index dd2bf9b..c2c8b57 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -19,11 +19,13 @@ class UsersController extends AppController { $currentUser = $this->ACL->getUser(); $conditions = []; + $validOrgIDsFOrEdition = []; if (empty($currentUser['role']['perm_admin'])) { $conditions['organisation_id IN'] = [$currentUser['organisation_id']]; if (!empty($currentUser['role']['perm_group_admin'])) { $this->loadModel('OrgGroups'); - $conditions['organisation_id IN'] = array_merge($conditions['organisation_id IN'], $this->OrgGroups->getGroupOrgIdsForUser($currentUser)); + $validOrgIDsFOrEdition = array_merge($conditions['organisation_id IN'], $this->OrgGroups->getGroupOrgIdsForUser($currentUser)); + $conditions['organisation_id IN'] = $validOrgIDsFOrEdition; } } $keycloakUsersParsed = null; @@ -40,7 +42,8 @@ class UsersController extends AppController 'filters' => $this->filterFields, 'quickFilters' => $this->quickFilterFields, 'conditions' => $conditions, - 'afterFind' => function($data) use ($keycloakUsersParsed) { + 'afterFind' => function($data) use ($keycloakUsersParsed, $currentUser) { + $data->_canBeEdited = $this->ACL->canEditUser($currentUser, $data); // TODO: We might want to uncomment this at some point Still need to evaluate the impact // if (!empty(Configure::read('keycloak.enabled'))) { // $keycloakUser = $keycloakUsersParsed[$data->username]; @@ -57,7 +60,7 @@ class UsersController extends AppController 'validRoles', $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_admin' => 0, 'perm_org_admin' => 0])->all()->toArray() ); - $this->set('metaGroup', $this->isAdmin ? 'Administration' : 'Cerebrate'); + $this->set('validOrgIDsFOrEdition', $validOrgIDsFOrEdition); } public function add() @@ -207,8 +210,9 @@ class UsersController extends AppController if (!empty($responsePayload)) { return $responsePayload; } + $userToEdit = $this->Users->find()->where(['Users.id' => $id])->contain('Roles')->first(); + $this->set('canEdit', $this->ACL->canEditUser($this->ACL->getUser(), $userToEdit)); $this->set('keycloakConfig', Configure::read('keycloak', ['enabled' => false])); - $this->set('metaGroup', $this->isAdmin ? 'Administration' : 'Cerebrate'); } public function edit($id = false) @@ -308,7 +312,8 @@ class UsersController extends AppController ])->toArray() ]; $this->set(compact('dropdownData')); - $this->set('metaGroup', $this->isAdmin ? 'Administration' : 'Cerebrate'); + $userToEdit = $this->Users->find()->where(['Users.id' => $id])->contain('Roles')->first(); + $this->set('canEdit', $this->ACL->canEditUser($this->ACL->getUser(), $userToEdit)); $this->render('add'); } diff --git a/templates/Users/index.php b/templates/Users/index.php index 3720240..4071a5a 100644 --- a/templates/Users/index.php +++ b/templates/Users/index.php @@ -133,19 +133,8 @@ echo $this->element('genericElements/IndexTable/index_table', [ 'role_id' => 'role_id' ] ], - 'function' => function ($row, $options) use ($loggedUser, $validRoles) { - if (empty($loggedUser['role']['perm_admin'])) { - if ($row['id'] == $loggedUser['id']) { - return true; - } - if (empty($loggedUser['role']['perm_org_admin'])) { - return false; - } - if (!isset($validRoles[$options['datapath']['role_id']])) { - return false; - } - } - return true; + 'function' => function ($row, $options) use ($loggedUser, $validRoles, $validOrgIDsFOrEdition) { + return $row['_canBeEdited']; } ] ], @@ -159,22 +148,14 @@ echo $this->element('genericElements/IndexTable/index_table', [ 'role_id' => 'role_id' ] ], - 'function' => function ($row, $options) use ($loggedUser, $validRoles) { + 'function' => function ($row, $options) use ($loggedUser, $validRoles, $validOrgIDsFOrEdition) { if (empty(Configure::read('user.allow-user-deletion'))) { return false; } if ($row['id'] == $loggedUser['id']) { return false; } - if (empty($loggedUser['role']['perm_admin'])) { - if (empty($loggedUser['role']['perm_org_admin'])) { - return false; - } - if (!isset($validRoles[$options['datapath']['role_id']])) { - return false; - } - } - return true; + return $row['_canBeEdited']; } ] ],