From e6ab7d61f19ea3b11376a8565926f1474b2b223f Mon Sep 17 00:00:00 2001 From: Bartek Fabiszewski Date: Tue, 18 Feb 2020 17:42:40 +0100 Subject: [PATCH] Allow to set user admin status in dialog --- .tests/tests/UserTest.php | 14 ++++++ helpers/user.php | 21 +++++++++ helpers/utils.php | 4 ++ js/src/user.js | 29 ++++++++++-- js/src/userdialogmodel.js | 72 +++++++++++++++++++++--------- js/test/userdialogmodel.test.js | 78 ++++++++++++++++++++++++++++++--- lang/en.php | 1 + utils/getusers.php | 4 +- utils/handleuser.php | 13 ++++-- 9 files changed, 202 insertions(+), 34 deletions(-) diff --git a/.tests/tests/UserTest.php b/.tests/tests/UserTest.php index 872f441..2df6435 100644 --- a/.tests/tests/UserTest.php +++ b/.tests/tests/UserTest.php @@ -50,6 +50,20 @@ class UserTest extends UloggerDatabaseTestCase { $this->assertFalse($userInvalid->setPass($newPass), "Setting pass for nonexistant user should fail"); } + public function testSetAdmin() { + $this->addTestUser($this->testUser); + $this->assertEquals(1, $this->getConnection()->getRowCount('users'), "Wrong row count"); + $user = new uUser($this->testUser); + $this->assertFalse((bool) $this->pdoGetColumn("SELECT admin FROM users"), "User should not be admin"); + $this->assertFalse($user->isAdmin, "User should not be admin"); + $user->setAdmin(true); + $this->assertTrue((bool) $this->pdoGetColumn("SELECT admin FROM users"), "User should be admin"); + $this->assertTrue($user->isAdmin, "User should be admin"); + $user->setAdmin(false); + $this->assertFalse((bool) $this->pdoGetColumn("SELECT admin FROM users"), "User should not be admin"); + $this->assertFalse($user->isAdmin, "User should not be admin"); + } + public function testGetAll() { $this->addTestUser($this->testUser); $this->addTestUser($this->testUser2); diff --git a/helpers/user.php b/helpers/user.php index 5aae21c..0fa6f08 100644 --- a/helpers/user.php +++ b/helpers/user.php @@ -126,6 +126,27 @@ return $ret; } + /** + * Set user admin status + * + * @param bool $isAdmin True if is admin + * @return bool True on success, false otherwise + */ + public function setAdmin($isAdmin) { + $ret = false; + try { + $query = "UPDATE " . self::db()->table('users') . " SET admin = ? WHERE login = ?"; + $stmt = self::db()->prepare($query); + $stmt->execute([ $isAdmin, $this->login ]); + $ret = true; + $this->isAdmin = $isAdmin; + } catch (PDOException $e) { + // TODO: handle exception + syslog(LOG_ERR, $e->getMessage()); + } + return $ret; + } + /** * Set user password * diff --git a/helpers/utils.php b/helpers/utils.php index 2524e4c..a4a8829 100644 --- a/helpers/utils.php +++ b/helpers/utils.php @@ -149,6 +149,10 @@ return self::requestString($name, $default, INPUT_GET); } + public static function postBool($name, $default = NULL) { + return self::requestValue($name, $default, INPUT_POST, FILTER_VALIDATE_BOOLEAN); + } + public static function getBool($name, $default = NULL) { return self::requestValue($name, $default, INPUT_GET, FILTER_VALIDATE_BOOLEAN); } diff --git a/js/src/user.js b/js/src/user.js index 18a6c9b..68ded28 100644 --- a/js/src/user.js +++ b/js/src/user.js @@ -31,14 +31,16 @@ export default class uUser extends uListItem { /** * @param {number} id * @param {string} login + * @param {boolean=} isAdmin */ - constructor(id, login) { + constructor(id, login, isAdmin = null) { super(); if (!Number.isSafeInteger(id) || id <= 0) { throw new Error('Invalid argument for user constructor'); } this.id = id; this.login = login; + this.isAdmin = isAdmin; this.listItem(id, login); } @@ -65,7 +67,7 @@ export default class uUser extends uListItem { return uAjax.get('utils/getusers.php').then((_users) => { const users = []; for (const user of _users) { - users.push(new uUser(user.id, user.login)); + users.push(new uUser(user.id, user.login, user.isAdmin)); } return users; }); @@ -101,8 +103,8 @@ export default class uUser extends uListItem { } /** - * @param {string} password - * @param {string=} oldPassword Needed when changing own password + * @param {string} password New password + * @param {string} oldPassword Current password * @return {Promise} */ setPassword(password, oldPassword) { @@ -113,4 +115,23 @@ export default class uUser extends uListItem { oldpass: oldPassword }); } + + /** + * @param {boolean} isAdmin + * @param {string|null} password + * @return {Promise} + */ + modify(isAdmin, password = null) { + const data = { + action: 'update', + login: this.login, + admin: isAdmin + }; + if (password) { + data.pass = password; + } + return uUser.update(data) + .then(() => { this.isAdmin = isAdmin; }); + } + } diff --git a/js/src/userdialogmodel.js b/js/src/userdialogmodel.js index ce0b617..4869661 100644 --- a/js/src/userdialogmodel.js +++ b/js/src/userdialogmodel.js @@ -33,18 +33,22 @@ export default class UserDialogModel extends ViewModel { super({ onUserDelete: null, onUserUpdate: null, + onPassChange: null, onUserAdd: null, onCancel: null, + passVisibility: false, login: null, password: null, password2: null, - oldPassword: null + oldPassword: null, + admin: false }); this.user = viewModel.state.currentUser; this.type = type; this.userVM = viewModel; this.model.onUserDelete = () => this.onUserDelete(); this.model.onUserUpdate = () => this.onUserUpdate(); + this.model.onPassChange = () => this.onPassChange(); this.model.onUserAdd = () => this.onUserAdd(); this.model.onCancel = () => this.onCancel(); } @@ -54,6 +58,14 @@ export default class UserDialogModel extends ViewModel { this.dialog = new uDialog(html); this.dialog.show(); this.bindAll(this.dialog.element); + const passInput = this.getBoundElement('passInput'); + this.onChanged('passVisibility', () => { + if (passInput.style.display === 'none') { + passInput.style.display = 'block'; + } else { + passInput.style.display = 'none'; + } + }); } onUserDelete() { @@ -67,8 +79,16 @@ export default class UserDialogModel extends ViewModel { onUserUpdate() { if (this.validate()) { - const user = this.type === 'pass' ? auth.user : this.user; - user.setPassword(this.model.password, this.model.oldPassword) + const password = this.model.passVisibility ? this.model.password : null; + this.user.modify(this.model.admin, password) + .then(() => this.dialog.destroy()) + .catch((e) => { uUtils.error(e, `${$._('actionfailure')}\n${e.message}`); }); + } + } + + onPassChange() { + if (this.validate()) { + auth.user.setPassword(this.model.password, this.model.oldPassword) .then(() => this.dialog.destroy()) .catch((e) => { uUtils.error(e, `${$._('actionfailure')}\n${e.message}`); }); } @@ -76,7 +96,7 @@ export default class UserDialogModel extends ViewModel { onUserAdd() { if (this.validate()) { - uUser.add(this.model.login, this.model.password).then((user) => { + uUser.add(this.model.login, this.model.password, this.model.admin).then((user) => { this.userVM.onUserAdded(user); this.dialog.destroy(); }).catch((e) => { uUtils.error(e, `${$._('actionfailure')}\n${e.message}`); }); @@ -103,17 +123,19 @@ export default class UserDialogModel extends ViewModel { return false; } } - if (!this.model.password || !this.model.password2) { - alert($._('allrequired')); - return false; - } - if (this.model.password !== this.model.password2) { - alert($._('passnotmatch')); - return false; - } - if (!config.passRegex.test(this.model.password)) { - alert($._('passlenmin') + '\n' + $._('passrules')); - return false; + if (this.type === 'pass' || this.model.passVisibility) { + if (!this.model.password || !this.model.password2) { + alert($._('allrequired')); + return false; + } + if (this.model.password !== this.model.password2) { + alert($._('passnotmatch')); + return false; + } + if (!config.passRegex.test(this.model.password)) { + alert($._('passlenmin') + '\n' + $._('passrules')); + return false; + } } return true; } @@ -134,20 +156,28 @@ export default class UserDialogModel extends ViewModel { fields = ` - `; + + + `; break; case 'edit': observer = 'onUserUpdate'; deleteButton = `
${$._('deluser')}
${$._('editinguser', `${uUtils.htmlEncode(this.user.login)}`)}
`; - fields = ` - - - `; + fields = ` +
+
+ + + + +
+ + `; break; case 'pass': - observer = 'onUserUpdate'; + observer = 'onPassChange'; fields = ` diff --git a/js/test/userdialogmodel.test.js b/js/test/userdialogmodel.test.js index ff4cd0d..e51d072 100644 --- a/js/test/userdialogmodel.test.js +++ b/js/test/userdialogmodel.test.js @@ -46,6 +46,8 @@ describe('UserDialogModel tests', () => { dm.user = new uUser(1, 'testUser'); spyOn(dm.user, 'delete').and.returnValue(Promise.resolve()); spyOn(dm.user, 'setPassword').and.returnValue(Promise.resolve()); + spyOn(dm.user, 'modify').and.callThrough(); + spyOn(uUser, 'update').and.returnValue(Promise.resolve()); spyOn(auth.user, 'setPassword').and.returnValue(Promise.resolve()); spyOn(uUser, 'add').and.returnValue(Promise.resolve(newUser)); spyOn(config.passRegex, 'test').and.returnValue(true); @@ -92,7 +94,7 @@ describe('UserDialogModel tests', () => { dm.init(); // then expect(document.querySelector('#modal')).toBeInstanceOf(HTMLDivElement); - expect(dm.dialog.element.querySelector("[data-bind='onUserUpdate']")).toBeInstanceOf(HTMLButtonElement); + expect(dm.dialog.element.querySelector("[data-bind='onPassChange']")).toBeInstanceOf(HTMLButtonElement); expect(dm.dialog.element.querySelector("[data-bind='onUserDelete']")).toBe(null); }); @@ -135,16 +137,67 @@ describe('UserDialogModel tests', () => { dm.type = 'edit'; dm.init(); const button = dm.dialog.element.querySelector("[data-bind='onUserUpdate']"); + const passVisibility = dm.dialog.element.querySelector("[data-bind='passVisibility']"); const passEl = dm.dialog.element.querySelector("[data-bind='password']"); const newPassword = 'newpass'; // when + passVisibility.checked = true; + passVisibility.dispatchEvent(new Event('change')); passEl.value = newPassword; passEl.dispatchEvent(new Event('change')); button.click(); // then setTimeout(() => { - expect(dm.user.setPassword).toHaveBeenCalledTimes(1); - expect(dm.user.setPassword).toHaveBeenCalledWith(newPassword, null); + expect(dm.user.modify).toHaveBeenCalledTimes(1); + expect(dm.user.modify).toHaveBeenCalledWith(dm.model.admin, newPassword); + expect(document.querySelector('#modal')).toBe(null); + done(); + }, 100); + }); + + it('should toggle password input fields visibility on user edit form', (done) => { + // given + dm.type = 'edit'; + dm.init(); + const passInput = dm.getBoundElement('passInput'); + const passVisibility = dm.dialog.element.querySelector("[data-bind='passVisibility']"); + + expect(passInput.style.display).toBe('none'); + // when + passVisibility.checked = true; + passVisibility.dispatchEvent(new Event('change')); + // then + setTimeout(() => { + expect(passInput.style.display).toBe('block'); + // when + passVisibility.checked = false; + passVisibility.dispatchEvent(new Event('change')); + // then + setTimeout(() => { + expect(passInput.style.display).toBe('none'); + done(); + }, 100); + done(); + }, 100); + }); + + it('should update user admin status and hide edit dialog on positive button clicked', (done) => { + // given + spyOn(dm, 'validate').and.returnValue(true); + dm.type = 'edit'; + dm.init(); + const button = dm.dialog.element.querySelector("[data-bind='onUserUpdate']"); + const adminEl = dm.dialog.element.querySelector("[data-bind='admin']"); + const isAdmin = true; + // when + adminEl.checked = isAdmin; + adminEl.dispatchEvent(new Event('change')); + button.click(); + // then + setTimeout(() => { + expect(dm.user.modify).toHaveBeenCalledTimes(1); + expect(dm.user.modify).toHaveBeenCalledWith(isAdmin, null); + expect(dm.user.isAdmin).toBeTrue(); expect(document.querySelector('#modal')).toBe(null); done(); }, 100); @@ -155,7 +208,7 @@ describe('UserDialogModel tests', () => { spyOn(dm, 'validate').and.returnValue(true); dm.type = 'pass'; dm.init(); - const button = dm.dialog.element.querySelector("[data-bind='onUserUpdate']"); + const button = dm.dialog.element.querySelector("[data-bind='onPassChange']"); const passEl = dm.dialog.element.querySelector("[data-bind='password']"); const passOldEl = dm.dialog.element.querySelector("[data-bind='oldPassword']"); const newPassword = 'newpass'; @@ -194,7 +247,7 @@ describe('UserDialogModel tests', () => { // then setTimeout(() => { expect(uUser.add).toHaveBeenCalledTimes(1); - expect(uUser.add).toHaveBeenCalledWith(newUser.login, newPassword); + expect(uUser.add).toHaveBeenCalledWith(newUser.login, newPassword, false); expect(mockVM.onUserAdded).toHaveBeenCalledWith(newUser); expect(document.querySelector('#modal')).toBe(null); done(); @@ -257,6 +310,7 @@ describe('UserDialogModel tests', () => { it('should return false on add user dialog passwords not match', () => { // given dm.model.login = 'test'; + dm.model.passVisibility = true; dm.model.password = 'password'; dm.model.password2 = 'password2'; // when @@ -266,10 +320,24 @@ describe('UserDialogModel tests', () => { expect(window.alert).toHaveBeenCalledTimes(1); }); + it('should return true and ignore passwords on add user dialog passwords hidden', () => { + // given + dm.model.login = 'test'; + dm.model.passVisibility = false; + dm.model.password = 'password'; + dm.model.password2 = 'password2'; + // when + const result = dm.validate(); + // then + expect(result).toBe(true); + expect(window.alert).toHaveBeenCalledTimes(0); + }); + it('should test password regex on dialog validate', () => { // given const password = 'password'; dm.model.login = 'test'; + dm.model.passVisibility = true; dm.model.password = password; dm.model.password2 = password; // when diff --git a/lang/en.php b/lang/en.php index 06649a1..19a10fa 100644 --- a/lang/en.php +++ b/lang/en.php @@ -82,6 +82,7 @@ $lang["units"] = "Units"; $lang["metric"] = "Metric"; $lang["imperial"] = "Imperial/US"; $lang["nautical"] = "Nautical"; +$lang["admin"] = "Administrator"; $lang["adminmenu"] = "Administration"; $lang["passwordrepeat"] = "Repeat password"; $lang["passwordenter"] = "Enter password"; diff --git a/utils/getusers.php b/utils/getusers.php index 42fb1fd..30ab17c 100644 --- a/utils/getusers.php +++ b/utils/getusers.php @@ -35,7 +35,9 @@ if ($usersArr === false) { $result = [ "error" => true ]; } else if (!empty($usersArr)) { foreach ($usersArr as $user) { - $result[] = [ "id" => $user->id, "login" => $user->login ]; + // only load admin status on admin user request + $isAdmin = $auth->isAdmin() ? $user->isAdmin : null; + $result[] = [ "id" => $user->id, "login" => $user->login, "isAdmin" => $isAdmin ]; } } header("Content-type: application/json"); diff --git a/utils/handleuser.php b/utils/handleuser.php index ab135ef..4cd8720 100644 --- a/utils/handleuser.php +++ b/utils/handleuser.php @@ -27,6 +27,7 @@ $action = uUtils::postString('action'); $login = uUtils::postString('login'); $pass = uUtils::postPass('pass'); + $admin = uUtils::postBool('admin', false); $lang = (new uLang(uConfig::$lang))->getStrings(); @@ -34,6 +35,10 @@ uUtils::exitWithError($lang["servererror"]); } + if ($admin && !$auth->isAdmin()) { + uUtils::exitWithError($lang["notauthorized"]); + } + $aUser = new uUser($login); $data = NULL; @@ -42,7 +47,7 @@ if ($aUser->isValid) { uUtils::exitWithError($lang["userexists"]); } - if (empty($pass) || ($userId = uUser::add($login, $pass)) === false) { + if (empty($pass) || ($userId = uUser::add($login, $pass, $admin)) === false) { uUtils::exitWithError($lang["servererror"]); } else { $data = [ 'id' => $userId ]; @@ -50,8 +55,10 @@ break; case 'update': - // update password - if (empty($pass) || $aUser->setPass($pass) === false) { + if ($aUser->setAdmin($admin) === false) { + uUtils::exitWithError($lang["servererror"]); + } + if (!empty($pass) && $aUser->setPass($pass) === false) { uUtils::exitWithError($lang["servererror"]); } break;