Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Password reset/change accepting current password as new password #16331

Merged
merged 9 commits into from May 21, 2020
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 @@ -1361,6 +1361,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 true;
}

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