From d6a6332cede71fbeb8a9f282f1fc170dc32579a1 Mon Sep 17 00:00:00 2001 From: Ashwani Yadav Date: Thu, 21 May 2020 10:57:58 +0530 Subject: [PATCH] [FIX] Password reset/change accepting current password as new password (#16331) Co-authored-by: Diego Sampaio --- .../client/reset-password/resetPassword.js | 2 ++ packages/rocketchat-i18n/i18n/en.i18n.json | 1 + server/lib/compareUserPassword.js | 29 ++++++++++++++++ server/methods/saveUserProfile.js | 34 +++++++------------ server/methods/setUserPassword.js | 11 ++++-- 5 files changed, 54 insertions(+), 23 deletions(-) create mode 100644 server/lib/compareUserPassword.js diff --git a/app/ui-login/client/reset-password/resetPassword.js b/app/ui-login/client/reset-password/resetPassword.js index e22c9b10c84..e5eaff9559f 100644 --- a/app/ui-login/client/reset-password/resetPassword.js +++ b/app/ui-login/client/reset-password/resetPassword.js @@ -59,6 +59,8 @@ async function setUserPassword(password) { requirePasswordChange: false, }, }); + toastr.remove(); + toastr.success(t('Password_changed_successfully')); } catch (e) { console.error(e); toastr.error(t('Error')); diff --git a/packages/rocketchat-i18n/i18n/en.i18n.json b/packages/rocketchat-i18n/i18n/en.i18n.json index 6ecfeef40d1..35433b6bdb9 100644 --- a/packages/rocketchat-i18n/i18n/en.i18n.json +++ b/packages/rocketchat-i18n/i18n/en.i18n.json @@ -1362,6 +1362,7 @@ "error-edit-permissions-not-allowed": "Editing permissions is not allowed", "error-email-domain-blacklisted": "The email domain is blacklisted", "error-email-send-failed": "Error trying to send email: __message__", + "error-password-same-as-current": "Entered password same as current password", "error-field-unavailable": "__field__ is already in use :(", "error-file-too-large": "File is too large", "error-forwarding-chat-same-department": "The selected department and the current room department are the same", diff --git a/server/lib/compareUserPassword.js b/server/lib/compareUserPassword.js new file mode 100644 index 00000000000..34c2fb4c699 --- /dev/null +++ b/server/lib/compareUserPassword.js @@ -0,0 +1,29 @@ +import { Accounts } from 'meteor/accounts-base'; + +/** + * Check if a given password is the one user by given user or if the user doesn't have a password + * @param {object} user User object + * @param {object} pass Object with { plain: 'plain-test-password' } or { sha256: 'sha256password' } + */ +export function compareUserPassword(user, pass) { + if (!user?.services?.password?.bcrypt?.trim()) { + return true; + } + + if (!pass || (!pass.plain && !pass.sha256)) { + return false; + } + + const password = pass.plain || { + digest: pass.sha256.toLowerCase(), + algorithm: 'sha-256', + }; + + const passCheck = Accounts._checkPassword(user, password); + + if (passCheck.error) { + return false; + } + + return true; +} diff --git a/server/methods/saveUserProfile.js b/server/methods/saveUserProfile.js index 53cd761d6a1..13159231efa 100644 --- a/server/methods/saveUserProfile.js +++ b/server/methods/saveUserProfile.js @@ -2,27 +2,12 @@ import { Meteor } from 'meteor/meteor'; import { Match, check } from 'meteor/check'; import { Accounts } from 'meteor/accounts-base'; -import { saveCustomFields, passwordPolicy } from '../../app/lib'; -import { Users } from '../../app/models'; -import { settings as rcSettings } from '../../app/settings'; +import { saveCustomFields, passwordPolicy } from '../../app/lib/server'; +import { Users } from '../../app/models/server'; +import { settings as rcSettings } from '../../app/settings/server'; import { twoFactorRequired } from '../../app/2fa/server/twoFactorRequired'; import { saveUserIdentity } from '../../app/lib/server/functions/saveUserIdentity'; - -function checkPassword(user = {}, typedPassword) { - if (!(user.services && user.services.password && user.services.password.bcrypt && user.services.password.bcrypt.trim())) { - return true; - } - - const passCheck = Accounts._checkPassword(user, { - digest: typedPassword.toLowerCase(), - algorithm: 'sha-256', - }); - - if (passCheck.error) { - return false; - } - return true; -} +import { compareUserPassword } from '../lib/compareUserPassword'; Meteor.methods({ saveUserProfile: twoFactorRequired(function(settings, customFields) { @@ -67,7 +52,7 @@ Meteor.methods({ } if (settings.email) { - if (!checkPassword(user, settings.typedPassword)) { + if (!compareUserPassword(user, { sha256: settings.typedPassword })) { throw new Meteor.Error('error-invalid-password', 'Invalid password', { method: 'saveUserProfile', }); @@ -78,12 +63,19 @@ Meteor.methods({ // Should be the last check to prevent error when trying to check password for users without password if (settings.newPassword && rcSettings.get('Accounts_AllowPasswordChange') === true) { - if (!checkPassword(user, settings.typedPassword)) { + if (!compareUserPassword(user, { sha256: settings.typedPassword })) { throw new Meteor.Error('error-invalid-password', 'Invalid password', { method: 'saveUserProfile', }); } + // don't let user change to same password + if (compareUserPassword(user, { plain: settings.newPassword })) { + throw new Meteor.Error('error-password-same-as-current', 'Entered password same as current password', { + method: 'saveUserProfile', + }); + } + passwordPolicy.validate(settings.newPassword); Accounts.setPassword(this.userId, settings.newPassword, { diff --git a/server/methods/setUserPassword.js b/server/methods/setUserPassword.js index 5ebcc135a2a..1d53c68f605 100644 --- a/server/methods/setUserPassword.js +++ b/server/methods/setUserPassword.js @@ -2,8 +2,9 @@ import { Meteor } from 'meteor/meteor'; import { check } from 'meteor/check'; import { Accounts } from 'meteor/accounts-base'; -import { Users } from '../../app/models'; -import { passwordPolicy } from '../../app/lib'; +import { Users } from '../../app/models/server'; +import { passwordPolicy } from '../../app/lib/server'; +import { compareUserPassword } from '../lib/compareUserPassword'; Meteor.methods({ setUserPassword(password) { @@ -25,6 +26,12 @@ Meteor.methods({ }); } + if (compareUserPassword(user, { plain: password })) { + throw new Meteor.Error('error-password-same-as-current', 'Entered password same as current password', { + method: 'setUserPassword', + }); + } + passwordPolicy.validate(password); Accounts.setPassword(userId, password, {