diff --git a/Docs/Documentation/Configuration.md b/Docs/Documentation/Configuration.md index dea40fd4d..c25cc26cc 100644 --- a/Docs/Documentation/Configuration.md +++ b/Docs/Documentation/Configuration.md @@ -116,7 +116,10 @@ NOTE: SOME keys were hidden in this doc page, please refer to `vendor/cakedc/use // configure Remember Me component 'active' => true, ], - 'Superuser' => ['allowedToChangePasswords' => false], // able to reset any users password + 'Superuser' => [ + 'allowedToChangePasswords' => false,// able to reset any users password + 'allowedToChangeRoles' => false // able to change user roles + ], ], //Default authentication/authorization setup 'Auth' => [ diff --git a/config/users.php b/config/users.php index e927ea4d4..79048fbbc 100644 --- a/config/users.php +++ b/config/users.php @@ -97,7 +97,14 @@ ] ] ], - 'Superuser' => ['allowedToChangePasswords' => false], // able to reset any users password + 'Superuser' => [ + 'allowedToChangePasswords' => false,// able to reset any users password + 'allowedToChangeRoles' => false // able to change user roles + ], + 'AvailableRoles'=>[ + 'admin', + 'user' + ], ], 'OneTimePasswordAuthenticator' => [ 'checker' => \CakeDC\Auth\Authentication\DefaultOneTimePasswordAuthenticationChecker::class, diff --git a/src/Controller/Traits/RoleManagementTrait.php b/src/Controller/Traits/RoleManagementTrait.php new file mode 100644 index 000000000..88cba29bd --- /dev/null +++ b/src/Controller/Traits/RoleManagementTrait.php @@ -0,0 +1,142 @@ +getRequest()->getAttribute('identity'); + $identity = $identity ?? []; + $userId = $identity['id'] ?? null; + + if ($userId) { + if ($this->canUserEditRole($id, $identity)) { + // superuser update user roles + $user = $this->getUsersTable()->get($id); + $configRoles = $this->getConfigRoles(); + if (!$configRoles) { + //set the defaults + $configRoles = [UsersTable::ROLE_ADMIN, UsersTable::ROLE_USER]; + } + $availableRoles = []; + foreach ($configRoles as $role) { + $availableRoles[$role] = $role; + } + $redirect = ['action' => 'index']; + } else { + $this->Flash->error( + __d('cake_d_c/users', 'Changing role is not allowed') + ); + + return $this->redirect(Configure::read('Users.Profile.route')); + } + } else { + $this->Flash->error( + __d('cake_d_c/users', 'Login to perform this action') + ); + return $this->redirect(Configure::read('Users.Profile.route')); + } + + if ($this->getRequest()->is(['post', 'put'])) { + try { + if (!in_array($this->getRequest()->getData()['role'], $availableRoles)) { + throw new Exception('Invalid role supplied'); + } + $user = $this->getUsersTable()->patchEntity( + $user, + $this->getRequest()->getData(), + [ + 'accessibleFields' => [ + 'role' => true, + 'is_superuser' => true, + ], + ] + ); + + + if ($user->getErrors()) { + $this->Flash->error(__d('cake_d_c/users', 'Role could not be changed')); + } else { + $result = $this->getUsersTable()->save($user); + if ($result) { + $event = $this->dispatchEvent(Plugin::EVENT_AFTER_CHANGE_ROLE, ['user' => $result]); + if (!empty($event) && is_array($event->getResult())) { + return $this->redirect($event->getResult()); + } + $this->Flash->success(__d('cake_d_c/users', 'Role has been changed successfully')); + + return $this->redirect($redirect); + } else { + $this->Flash->error(__d('cake_d_c/users', 'Role could not be changed')); + } + } + } catch (UserNotFoundException $exception) { + $this->Flash->error(__d('cake_d_c/users', 'User was not found')); + } catch (Exception $exception) { + $this->Flash->error(__d('cake_d_c/users', 'Role could not be changed')); + $this->log($exception->getMessage()); + } + } + $this->set(compact('user', 'availableRoles')); + $this->set('_serialize', ['user']); + } + + /** + * Checks and returns boolean value if the user can edit the role + * @param int|string|null $id user_id, null for logged in user id + * @param mixed|string $identity + * @return bool + */ + protected function canUserEditRole($id, $identity) + { + return $id && $identity['is_superuser'] && Configure::read('Users.Superuser.allowedToChangeRoles'); + } + + + /** + * @return array|false[]|mixed + */ + protected function getConfigRoles() + { + $configRoles = Configure::read('Users.AvailableRoles'); + if (is_array($configRoles) && count($configRoles) == 0) { + //if the config key is present and roles array is empty + throw new ConfigNotSetException('No Available role found in the users config. please set Users.AvailableRoles'); + } + return $configRoles; + } +} diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index 6cffb4590..1ae621b9d 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -19,6 +19,7 @@ use CakeDC\Users\Controller\Traits\ProfileTrait; use CakeDC\Users\Controller\Traits\ReCaptchaTrait; use CakeDC\Users\Controller\Traits\RegisterTrait; +use CakeDC\Users\Controller\Traits\RoleManagementTrait; use CakeDC\Users\Controller\Traits\SimpleCrudTrait; use CakeDC\Users\Controller\Traits\SocialTrait; use CakeDC\Users\Controller\Traits\U2fTrait; @@ -37,6 +38,7 @@ class UsersController extends AppController use ProfileTrait; use ReCaptchaTrait; use RegisterTrait; + use RoleManagementTrait; use SimpleCrudTrait; use SocialTrait; use U2fTrait; diff --git a/src/Exception/ConfigNotSetException.php b/src/Exception/ConfigNotSetException.php new file mode 100644 index 000000000..93dd10083 --- /dev/null +++ b/src/Exception/ConfigNotSetException.php @@ -0,0 +1,19 @@ + + Flash->render('auth') ?> + Form->create($user) ?> +
+ + + Form->select('role', $availableRoles, ['empty' => false]); ?> + Form->control('is_superuser', ['type' => 'checkbox','label'=>'Superuser']);?> + +
+ Form->button(__d('cake_d_c/users', 'Submit')); ?> + Form->end() ?> + diff --git a/templates/Users/index.php b/templates/Users/index.php index 273c5211c..4cdf73c53 100644 --- a/templates/Users/index.php +++ b/templates/Users/index.php @@ -36,6 +36,7 @@ Html->link(__d('cake_d_c/users', 'View'), ['action' => 'view', $user->id]) ?> Html->link(__d('cake_d_c/users', 'Change password'), ['action' => 'changePassword', $user->id]) ?> + Html->link(__d('cake_d_c/users', 'Change role'), ['action' => 'changeRole', $user->id]) ?> Html->link(__d('cake_d_c/users', 'Edit'), ['action' => 'edit', $user->id]) ?> Form->postLink(__d('cake_d_c/users', 'Delete'), ['action' => 'delete', $user->id], ['confirm' => __d('cake_d_c/users', 'Are you sure you want to delete # {0}?', $user->id)]) ?> diff --git a/tests/TestCase/Controller/Traits/RoleManagementTraitTest.php b/tests/TestCase/Controller/Traits/RoleManagementTraitTest.php new file mode 100644 index 000000000..d00f4f9a2 --- /dev/null +++ b/tests/TestCase/Controller/Traits/RoleManagementTraitTest.php @@ -0,0 +1,256 @@ +traitClassName = 'CakeDC\Users\Controller\UsersController'; + $this->traitMockMethods = ['set', 'getUsersTable', 'redirect', 'validate']; + parent::setUp(); + } + + /** + * tearDown + * + * @return void + */ + public function tearDown(): void + { + parent::tearDown(); + } + + /** + * test calling the action without authentication + * + * @return void + */ + public function testCallingActionWithoutAuthentication() + { + $userId = '00000000-0000-0000-0000-000000000002'; + + $this->_mockRequestPost(); + $this->Trait->getRequest() + ->method('getData') + ->will($this->returnValue([ + 'role' => 'superuser', + ])); + $this->_mockFlash(); + $this->Trait->Flash->expects($this->once()) + ->method('error') + ->with('Login to perform this action'); + $this->Trait->changeRole($userId); + } + + + /** + * test superuser cannot change roles with config set to false + * + * @return void + */ + public function testSuperUserCannotChangeRoleWithDefaultConfig() + { + $superUser = $this->table->get('00000000-0000-0000-0000-000000000001'); + $userId = '00000000-0000-0000-0000-000000000002'; + $this->_mockRequestPost(); + $this->Trait->getRequest() + ->method('getData') + ->will($this->returnValue([ + 'role' => 'superuser', + ])); + $this->_mockAuthLoggedIn($superUser->toArray()); + $this->_mockFlash(); + + $this->Trait->Flash->expects($this->once()) + ->method('error') + ->with('Changing role is not allowed'); + $this->Trait->changeRole($userId); + } + + /** + * test superuser can change role with updated config + * + * @return void + */ + public function testSuperUserCanChangeRoleWithUpdatedConfig() + { + $superUser = $this->table->get('00000000-0000-0000-0000-000000000001'); + $userId = '00000000-0000-0000-0000-000000000002'; + + //set the request + $request = $this->getMockBuilder('Cake\Http\ServerRequest') + ->setMethods(['is', 'getData']) + ->getMock(); + $this->Trait->setRequest($request); + $this->Trait->getRequest() + ->method('is') + ->with(['post', 'put']) + ->will($this->returnValue(true)); + + $this->Trait->getRequest() + ->method('getData') + ->will($this->returnValue([ + 'role' => 'user' + ])); + + $this->_mockAuthLoggedIn($superUser->toArray()); + $this->_mockFlash(); + + $this->Trait->Flash->expects($this->once()) + ->method('success') + ->with('Role has been changed successfully'); + + //update configuration + Configure::write('Users.Superuser.allowedToChangeRoles', true); + $this->Trait->changeRole($userId); + + $user = $this->table->get($userId); + $this->assertEquals('user', $user->role); + } + + + /** + * test superuser can change role with updated config + * + * @return void + */ + public function testSuperUserCannotChangeRoleWithUpdatedConfigWhenInvalidRolePassed() + { + $superUser = $this->table->get('00000000-0000-0000-0000-000000000001'); + $userId = '00000000-0000-0000-0000-000000000002'; + + //set the request + $request = $this->getMockBuilder('Cake\Http\ServerRequest') + ->setMethods(['is', 'getData']) + ->getMock(); + $this->Trait->setRequest($request); + $this->Trait->getRequest() + ->method('is') + ->with(['post', 'put']) + ->will($this->returnValue(true)); + + $this->Trait->getRequest() + ->method('getData') + ->will($this->returnValue([ + 'role' => 'random_role' + ])); + + $this->_mockAuthLoggedIn($superUser->toArray()); + $this->_mockFlash(); + + $this->Trait->Flash->expects($this->once()) + ->method('error') + ->with('Role could not be changed'); + //update configuration + Configure::write('Users.Superuser.allowedToChangeRoles', true); + $this->Trait->changeRole($userId); + } + + /** + * test + * + * @return void + */ + public function testErrorWhenNoConfigIsPresentForAvailableRoles() + { + $superUser = $this->table->get('00000000-0000-0000-0000-000000000001'); + $userId = '00000000-0000-0000-0000-000000000002'; + + //set the request + $request = $this->getMockBuilder('Cake\Http\ServerRequest') + ->setMethods(['is', 'getData']) + ->getMock(); + $this->Trait->setRequest($request); + $this->Trait->getRequest() + ->method('is') + ->with(['post', 'put']) + ->will($this->returnValue(true)); + + $this->Trait->getRequest() + ->method('getData') + ->will($this->returnValue([ + 'role' => 'random_role' + ])); + + $this->_mockAuthLoggedIn($superUser->toArray()); + $this->_mockFlash(); + + $this->expectException(ConfigNotSetException::class); + //update configuration + Configure::write('Users.Superuser.allowedToChangeRoles', true); + Configure::write('Users.AvailableRoles', []); + $this->Trait->changeRole($userId); + } + + /** + * test + * + * @return void + */ + public function testNoErrorWhenAvailableRolesConfigArrayIsEmpty() + { + $superUser = $this->table->get('00000000-0000-0000-0000-000000000001'); + $userId = '00000000-0000-0000-0000-000000000002'; + + //set the request + $request = $this->getMockBuilder('Cake\Http\ServerRequest') + ->setMethods(['is', 'getData']) + ->getMock(); + $this->Trait->setRequest($request); + $this->Trait->getRequest() + ->method('is') + ->with(['post', 'put']) + ->will($this->returnValue(true)); + + $this->Trait->getRequest() + ->method('getData') + ->will($this->returnValue([ + 'role' => 'admin' + ])); + + $this->_mockAuthLoggedIn($superUser->toArray()); + $this->_mockFlash(); + + $this->Trait->Flash->expects($this->once()) + ->method('success') + ->with('Role has been changed successfully'); + //update configuration + Configure::write('Users.Superuser.allowedToChangeRoles', true); + Configure::delete('Users.AvailableRoles'); + $this->Trait->changeRole($userId); + } +} diff --git a/tests/test_app/config/users.php b/tests/test_app/config/users.php index 35e35adcd..9bdb2b0f1 100644 --- a/tests/test_app/config/users.php +++ b/tests/test_app/config/users.php @@ -2,6 +2,7 @@ return [ 'Users.Social.login' => true, + 'Users.Superuser.allowedToChangeRoles' => false, 'Auth.AuthenticationComponent.loginRedirect' => '/pages/home', 'OAuth.providers.facebook.options.clientId' => '1010101010101010', 'OAuth.providers.facebook.options.clientSecret' => 'ABABABABABABABABA',