From 6583ac649a71b8ca71cad9cffb2514da5a6bc4a6 Mon Sep 17 00:00:00 2001 From: Jan Evers Date: Mon, 25 Nov 2024 12:41:16 +0000 Subject: [PATCH] tweak(Admin): improve "must change password" handling --- tests/tine20/Tinebase/User/SqlTest.php | 2 +- tine20/Admin/Frontend/Json.php | 8 ++++- tine20/Admin/js/user/EditDialog.js | 17 +++++----- tine20/Admin/js/user/GridPanel.js | 26 ++++++---------- tine20/Admin/js/user/Users.js | 3 +- tine20/Tinebase/Model/FullUser.php | 43 +++++++++++++++++++++++++- 6 files changed, 69 insertions(+), 30 deletions(-) diff --git a/tests/tine20/Tinebase/User/SqlTest.php b/tests/tine20/Tinebase/User/SqlTest.php index e82de6df3ad..967e6e15575 100644 --- a/tests/tine20/Tinebase/User/SqlTest.php +++ b/tests/tine20/Tinebase/User/SqlTest.php @@ -425,7 +425,7 @@ public function testPasswordMustChange() self::assertNull($user->mustChangePassword()); // change days config to 10 - Tinebase_Config::getInstance()->set(Tinebase_Config::PASSWORD_POLICY_CHANGE_AFTER, 10); + Tinebase_Config::getInstance()->set(Tinebase_Config::USER_PASSWORD_POLICY, [Tinebase_Config::PASSWORD_POLICY_CHANGE_AFTER => 10]); // set password change: 11 days ago) $now = Tinebase_DateTime::now(); diff --git a/tine20/Admin/Frontend/Json.php b/tine20/Admin/Frontend/Json.php index 8b23c68beab..9cb71fa9222 100644 --- a/tine20/Admin/Frontend/Json.php +++ b/tine20/Admin/Frontend/Json.php @@ -194,6 +194,8 @@ public function getUser($id) { if (!empty($id)) { $user = Admin_Controller_User::getInstance()->get($id); + $user->must_change_password = $user->getMustChangePassword(); + $userArray = $this->_recordToJson($user); // don't send some infos to the client: unset email uid+gid @@ -223,7 +225,7 @@ public function getUser($id) ); $userRoles = array(); } - + } else { $userArray = array('accountStatus' => 'enabled', 'visibility' => 'displayed'); @@ -271,6 +273,10 @@ public function getUser($id) public function getUsers($filter, $sort, $dir, $start, $limit) { $accounts = Admin_Controller_User::getInstance()->searchFullUsers($filter, $sort, $dir, $start, $limit); + /* @var Tinebase_Model_FullUser $account */ + foreach ($accounts as $account) { + $account->must_change_password = $account->getMustChangePassword(); + } $results = array(); foreach ($this->_multipleRecordsToJson($accounts) as $val) { $val['filesystemSize'] = null; diff --git a/tine20/Admin/js/user/EditDialog.js b/tine20/Admin/js/user/EditDialog.js index 1934357922f..955430941b1 100644 --- a/tine20/Admin/js/user/EditDialog.js +++ b/tine20/Admin/js/user/EditDialog.js @@ -143,22 +143,21 @@ Tine.Admin.UserEditDialog = Ext.extend(Tine.widgets.dialog.EditDialog, { * @private */ mustChangePasswordCheck: function () { - const changeAfter = Tine.Tinebase.configManager.get('userPwPolicy.pwPolicyChangeAfter', 'Tinebase') - if (!changeAfter || changeAfter === 0) return - - const lastChangeDate = this.record.get('accountLastPasswordChangeRaw') - - const maxDate = new Date(lastChangeDate).add('d', changeAfter) - if (maxDate > new Date()) return + const must_change_password = this.record.get('must_change_password') + if (!must_change_password || must_change_password !== 'expired') return const lastChangeField = this.getForm().findField('accountLastPasswordChange') const mustChangeField = this.getForm().findField('password_must_change') lastChangeField.addClass('tinebase-warning') - mustChangeField.disable() + // store real value of the flag, so it is not overwritten later this.record.set('password_must_change_actual', this.record.get('password_must_change')) - this.record.set('password_must_change', 1) + + // password must be changed no matter whether the flag is set + this.record.set('password_must_change', true) + mustChangeField.disable() + this.mustChangeTriggerPlugin.setVisible(true) }, diff --git a/tine20/Admin/js/user/GridPanel.js b/tine20/Admin/js/user/GridPanel.js index c76ed89796d..3bf74472127 100644 --- a/tine20/Admin/js/user/GridPanel.js +++ b/tine20/Admin/js/user/GridPanel.js @@ -202,7 +202,7 @@ Tine.Admin.user.GridPanel = Ext.extend(Tine.widgets.grid.GridPanel, { { header: this.app.i18n._('Last login at'), id: 'accountLastLogin', hidden: this.isLdapBackend, renderer: Tine.Tinebase.common.dateTimeRenderer}, { header: this.app.i18n._('Last login from'), id: 'accountLastLoginfrom', width: 120, hidden: this.isLdapBackend}, { header: this.app.i18n._('MFA Configured'), id: 'mfa_configs', renderer: this.mfaRenderer.createDelegate(this), hidden: false}, - { header: this.app.i18n._('Password Must Change'), id: 'password_must_change', renderer: this.mustChangeRenderer, hidden: false}, + { header: this.app.i18n._('Password Must Change'), id: 'must_change_password', renderer: this.mustChangeRenderer, hidden: false, sortable: false}, { header: this.app.i18n._('Password changed'), id: 'accountLastPasswordChange', renderer: this.dateTimeRenderer, hidden: false}, { header: this.app.i18n._('Expires'), id: 'accountExpires', renderer: Tine.Tinebase.common.dateTimeRenderer, hidden: false}, { header: this.app.i18n._('Full Name'), id: 'accountFullName'} @@ -232,25 +232,17 @@ Tine.Admin.user.GridPanel = Ext.extend(Tine.widgets.grid.GridPanel, { return rendered }, - mustChangeRenderer: function (_value, metadata, _record) { - let changeAfter = Tine.Tinebase.configManager.get('userPwPolicy.pwPolicyChangeAfter', 'Tinebase') - if (!changeAfter || changeAfter === 0) return Tine.Tinebase.common.booleanRenderer(_value) - - let hoverText = i18n._('Password is expired in accordance with the password policy and needs to be changed') - - let lastChangeDate = _record.get('accountLastPasswordChange') - if (!lastChangeDate) { + mustChangeRenderer: function (_value, metadata) { + if (_value === 'expired') { + const hoverText = i18n._('Password is expired in accordance with the password policy and needs to be changed') metadata.cellAttr = 'title="'+hoverText+'"' - return Tine.Tinebase.common.booleanRenderer('yes') + } else { + metadata.cellAttr = '' } - - let maxDate = new Date(lastChangeDate).add('d', changeAfter) - if (maxDate < new Date()) { - metadata.cellAttr = 'title="'+hoverText+'"' - return Tine.Tinebase.common.booleanRenderer('yes') + if (_value !== null) { + return Tine.Tinebase.common.booleanRenderer('1') } - - return Tine.Tinebase.common.booleanRenderer(_value) + return Tine.Tinebase.common.booleanRenderer('0') }, enableDisableButtonHandler: function(status) { diff --git a/tine20/Admin/js/user/Users.js b/tine20/Admin/js/user/Users.js index e6211d2644a..63f541ae1e2 100644 --- a/tine20/Admin/js/user/Users.js +++ b/tine20/Admin/js/user/Users.js @@ -67,7 +67,8 @@ Tine.Admin.Model.UserArray = [ { name: 'effectiveAndLocalQuota'}, { name: 'mfa_configs'}, { name: 'xprops'}, - { name: 'type'} + { name: 'type'}, + { name: 'must_change_password'} // dynamically calculated value, not a persisted property ]; Tine.Admin.Model.User = Tine.Tinebase.data.Record.create(Tine.Tinebase.Model.genericFields.concat(Tine.Admin.Model.UserArray), { diff --git a/tine20/Tinebase/Model/FullUser.php b/tine20/Tinebase/Model/FullUser.php index 4ae9311d4b9..015cbfde1c9 100644 --- a/tine20/Tinebase/Model/FullUser.php +++ b/tine20/Tinebase/Model/FullUser.php @@ -59,6 +59,11 @@ class Tinebase_Model_FullUser extends Tinebase_Model_User public const USER_TYPE_USER = 'user'; public const USER_TYPE_VOLUNTEER = 'volunteer'; + public const PASSWORD_EXPIRED = 'expired'; + public const PASSWORD_FLAGGED = 'flagged'; + public const PASSWORD_NEVER_CHANGED = 'neverChanged'; + + /** * holds the configuration object (must be declared in the concrete class) * @@ -251,6 +256,12 @@ class Tinebase_Model_FullUser extends Tinebase_Model_User self::TYPE => self::TYPE_VIRTUAL, self::VALIDATORS => [Zend_Filter_Input::ALLOW_EMPTY => true], ), + 'must_change_password' => [ + self::TYPE => self::TYPE_STRING, + self::NULLABLE => true, + self::IS_VIRTUAL => true, + self::VALIDATORS => [Zend_Filter_Input::ALLOW_EMPTY => true], + ] ], ]; @@ -421,7 +432,7 @@ protected function _sqlPasswordChangeNeeded() return true; } - $passwordChangeDays = Tinebase_Config::getInstance()->get(Tinebase_Config::PASSWORD_POLICY_CHANGE_AFTER); + $passwordChangeDays = $this->getPasswordChangeDays(); if ($passwordChangeDays > 0) { $now = Tinebase_DateTime::now(); @@ -512,4 +523,34 @@ public function getEmailUserId($type = self::XPROP_EMAIL_USERID_IMAP) { return Tinebase_EmailUser_XpropsFacade::getEmailUserId($this, $type); } + + public function getMustChangePassword(): ?string + { + if (empty($this->accountLastPasswordChange)) { + return self::PASSWORD_NEVER_CHANGED; + } + + $mustChange = null; + if ($this->password_must_change) { + $mustChange = self::PASSWORD_FLAGGED; + } + + $passwordChangeDays = $this->getPasswordChangeDays(); + if ($passwordChangeDays > 0) { + $now = Tinebase_DateTime::now(); + $mustChange = $this->accountLastPasswordChange->isEarlier($now->subDay($passwordChangeDays)) ? self::PASSWORD_EXPIRED : $mustChange; + } + return $mustChange; + } + + public function getPasswordChangeDays(): ?int + { + $passwordPolicy = Tinebase_Config::getInstance()->get(Tinebase_Config::USER_PASSWORD_POLICY); + + $passwordChangeDays = 0; + if ($passwordPolicy) { + $passwordChangeDays = $passwordPolicy->get(Tinebase_Config::PASSWORD_POLICY_CHANGE_AFTER); + } + return $passwordChangeDays; + } }