Skip to content

Commit

Permalink
[FIX] Password reset/change accepting current password as new password (
Browse files Browse the repository at this point in the history
#16331)

Co-authored-by: Diego Sampaio <chinello@gmail.com>
  • Loading branch information
ashwaniYDV and sampaiodiego committed May 21, 2020
1 parent d46c864 commit d6a6332
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 23 deletions.
2 changes: 2 additions & 0 deletions app/ui-login/client/reset-password/resetPassword.js
Expand Up @@ -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'));
Expand Down
1 change: 1 addition & 0 deletions packages/rocketchat-i18n/i18n/en.i18n.json
Expand Up @@ -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": "<strong>__field__</strong> 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",
Expand Down
29 changes: 29 additions & 0 deletions 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;
}
34 changes: 13 additions & 21 deletions server/methods/saveUserProfile.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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',
});
Expand All @@ -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, {
Expand Down
11 changes: 9 additions & 2 deletions server/methods/setUserPassword.js
Expand Up @@ -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) {
Expand All @@ -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, {
Expand Down

0 comments on commit d6a6332

Please sign in to comment.