Skip to content

[FIX] Save user password and email from My Account #20737

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.

Sign up for GitHub

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

Merged
sampaiodiego merged 3 commits into from
Feb 18, 2021
Merged

[FIX] Save user password and email from My Account #20737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a476d2
Fix save user password and email
sampaiodiego Feb 12, 2021
93d9e8e
Ignore TEST_MODE for required second factor endpoints
sampaiodiego Feb 12, 2021
85de3b5
Use helper function to select second factor method
sampaiodiego Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/2fa/server/code/ICodeCheck.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ export interface IProcessInvalidCodeResult {
export interface ICodeCheck {
readonly name: string;

isEnabled(user: IUser): boolean;
isEnabled(user: IUser, force?: boolean): boolean;

verify(user: IUser, code: string): boolean;
verify(user: IUser, code: string, force?: boolean): boolean;

processInvalidCode(user: IUser): IProcessInvalidCodeResult;
}
9 changes: 6 additions & 3 deletions app/2fa/server/code/PasswordCheckFallback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { IUser } from '../../../../definition/IUser';
export class PasswordCheckFallback implements ICodeCheck {
public readonly name = 'password';

public isEnabled(user: IUser): boolean {
public isEnabled(user: IUser, force: boolean): boolean {
if (force) {
return true;
}
// TODO: Remove this setting for version 4.0 forcing the
// password fallback for who has password set.
if (settings.get('Accounts_TwoFactorAuthentication_Enforce_Password_Fallback')) {
Expand All @@ -16,8 +19,8 @@ export class PasswordCheckFallback implements ICodeCheck {
return false;
}

public verify(user: IUser, code: string): boolean {
if (!this.isEnabled(user)) {
public verify(user: IUser, code: string, force: boolean): boolean {
if (!this.isEnabled(user, force)) {
return false;
}

Expand Down
43 changes: 33 additions & 10 deletions app/2fa/server/code/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { IMethodConnection } from '../../../../definition/IMethodThisType';
export interface ITwoFactorOptions {
disablePasswordFallback?: boolean;
disableRememberMe?: boolean;
requireSecondFactor?: boolean; // whether any two factor should be required
}

export const totpCheck = new TOTPCheck();
Expand Down Expand Up @@ -83,6 +84,11 @@ export function isAuthorizedForToken(connection: IMethodConnection, user: IUser,
return false;
}

// if any two factor is required, early abort
if (options.requireSecondFactor) {
return false;
}

if (tokenObject.bypassTwoFactor === true) {
return true;
}
Expand Down Expand Up @@ -131,7 +137,29 @@ interface ICheckCodeForUser {
connection?: IMethodConnection;
}

function _checkCodeForUser({ user, code, method, options = {}, connection }: ICheckCodeForUser): boolean {
const getSecondFactorMethod = (user: IUser, method: string | undefined, options: ITwoFactorOptions): ICodeCheck | undefined => {
// try first getting one of the available methods or the one that was already provided
const selectedMethod = getMethodByNameOrFirstActiveForUser(user, method);
if (selectedMethod) {
return selectedMethod;
}

// if none found but a second factor is required, chose the password check
if (options.requireSecondFactor) {
return passwordCheckFallback;
}

// check if password fallback is enabled
if (!options.disablePasswordFallback && passwordCheckFallback.isEnabled(user, !!options.requireSecondFactor)) {
return passwordCheckFallback;
}
};

export function checkCodeForUser({ user, code, method, options = {}, connection }: ICheckCodeForUser): boolean {
if (process.env.TEST_MODE && !options.requireSecondFactor) {
return true;
}

if (typeof user === 'string') {
user = getUserForCheck(user);
}
Expand All @@ -145,13 +173,10 @@ function _checkCodeForUser({ user, code, method, options = {}, connection }: ICh
return true;
}

let selectedMethod = getMethodByNameOrFirstActiveForUser(user, method);

// select a second factor method or return if none is found/available
const selectedMethod = getSecondFactorMethod(user, method, options);
if (!selectedMethod) {
if (options.disablePasswordFallback || !passwordCheckFallback.isEnabled(user)) {
return true;
}
selectedMethod = passwordCheckFallback;
return true;
}

if (!code) {
Expand All @@ -161,7 +186,7 @@ function _checkCodeForUser({ user, code, method, options = {}, connection }: ICh
throw new Meteor.Error('totp-required', 'TOTP Required', { method: selectedMethod.name, ...data, availableMethods });
}

const valid = selectedMethod.verify(user, code);
const valid = selectedMethod.verify(user, code, options.requireSecondFactor);

if (!valid) {
throw new Meteor.Error('totp-invalid', 'TOTP Invalid', { method: selectedMethod.name });
Expand All @@ -173,5 +198,3 @@ function _checkCodeForUser({ user, code, method, options = {}, connection }: ICh

return true;
}

export const checkCodeForUser = process.env.TEST_MODE ? (): boolean => true : _checkCodeForUser;
10 changes: 9 additions & 1 deletion app/api/server/v1/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,15 @@ API.v1.addRoute('users.updateOwnBasicInfo', { authRequired: true }, {
typedPassword: this.bodyParams.data.currentPassword,
};

Meteor.runAsUser(this.userId, () => Meteor.call('saveUserProfile', userData, this.bodyParams.customFields));
// saveUserProfile now uses the default two factor authentication procedures, so we need to provide that
const twoFactorOptions = !userData.typedPassword
? null
: {
twoFactorCode: userData.typedPassword,
twoFactorMethod: 'password',
};

Meteor.runAsUser(this.userId, () => Meteor.call('saveUserProfile', userData, this.bodyParams.customFields, twoFactorOptions));

return API.v1.success({ user: Users.findOneById(this.userId, { fields: API.v1.defaultFieldsToExclude }) });
},
Expand Down
164 changes: 82 additions & 82 deletions server/methods/saveUserProfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,108 +9,108 @@ import { twoFactorRequired } from '../../app/2fa/server/twoFactorRequired';
import { saveUserIdentity } from '../../app/lib/server/functions/saveUserIdentity';
import { compareUserPassword } from '../lib/compareUserPassword';

Meteor.methods({
saveUserProfile: twoFactorRequired(function(settings, customFields) {
check(settings, Object);
check(customFields, Match.Maybe(Object));
function saveUserProfile(settings, customFields) {
if (!rcSettings.get('Accounts_AllowUserProfileChange')) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', {
method: 'saveUserProfile',
});
}

if (!this.userId) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
method: 'saveUserProfile',
});
}

const user = Users.findOneById(this.userId);

if (settings.realname || settings.username) {
if (!saveUserIdentity(this.userId, {
_id: this.userId,
name: settings.realname,
username: settings.username,
})) {
throw new Meteor.Error('error-could-not-save-identity', 'Could not save user identity', { method: 'saveUserProfile' });
}
}

if (!rcSettings.get('Accounts_AllowUserProfileChange')) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', {
if (settings.statusText || settings.statusText === '') {
Meteor.call('setUserStatus', null, settings.statusText);
}

if (settings.statusType) {
Meteor.call('setUserStatus', settings.statusType, null);
}

if (settings.bio != null) {
if (typeof settings.bio !== 'string' || settings.bio.length > 260) {
throw new Meteor.Error('error-invalid-field', 'bio', {
method: 'saveUserProfile',
});
}
Users.setBio(user._id, settings.bio.trim());
}

if (!this.userId) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
if (settings.nickname != null) {
if (typeof settings.nickname !== 'string' || settings.nickname.length > 120) {
throw new Meteor.Error('error-invalid-field', 'nickname', {
method: 'saveUserProfile',
});
}

const user = Users.findOneById(this.userId);

if (settings.realname || settings.username) {
if (!saveUserIdentity(this.userId, {
_id: this.userId,
name: settings.realname,
username: settings.username,
})) {
throw new Meteor.Error('error-could-not-save-identity', 'Could not save user identity', { method: 'saveUserProfile' });
}
}

if (settings.statusText || settings.statusText === '') {
Meteor.call('setUserStatus', null, settings.statusText);
}

if (settings.statusType) {
Meteor.call('setUserStatus', settings.statusType, null);
}

if (settings.bio != null) {
if (typeof settings.bio !== 'string' || settings.bio.length > 260) {
throw new Meteor.Error('error-invalid-field', 'bio', {
Users.setNickname(user._id, settings.nickname.trim());
}

if (settings.email) {
Meteor.call('setEmail', settings.email);
}

const canChangePasswordForOAuth = rcSettings.get('Accounts_AllowPasswordChangeForOAuthUsers');
if (canChangePasswordForOAuth || user.services?.password) {
// 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) {
Copy link
Member

@KevLehman KevLehman Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the === true part of this

Copy link
Member Author

@sampaiodiego sampaiodiego Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, this is an old code that was just re-indented, but I agree 👍

// 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',
});
}
Users.setBio(user._id, settings.bio.trim());
}

if (settings.nickname != null) {
if (typeof settings.nickname !== 'string' || settings.nickname.length > 120) {
throw new Meteor.Error('error-invalid-field', 'nickname', {
method: 'saveUserProfile',
});
passwordPolicy.validate(settings.newPassword);

Accounts.setPassword(this.userId, settings.newPassword, {
logout: false,
});

try {
Meteor.call('removeOtherTokens');
} catch (e) {
Accounts._clearAllLoginTokens(this.userId);
}
Users.setNickname(user._id, settings.nickname.trim());
}
}

if (settings.email) {
if (!compareUserPassword(user, { sha256: settings.typedPassword })) {
throw new Meteor.Error('error-invalid-password', 'Invalid password', {
method: 'saveUserProfile',
});
}
Users.setProfile(this.userId, {});

Meteor.call('setEmail', settings.email);
}
if (customFields && Object.keys(customFields).length) {
saveCustomFields(this.userId, customFields);
}

const canChangePasswordForOAuth = rcSettings.get('Accounts_AllowPasswordChangeForOAuthUsers');
if (canChangePasswordForOAuth || user.services?.password) {
// 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 (!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, {
logout: false,
});
return true;
}

try {
Meteor.call('removeOtherTokens');
} catch (e) {
Accounts._clearAllLoginTokens(this.userId);
}
}
}
const saveUserProfileWithTwoFactor = twoFactorRequired(saveUserProfile, {
requireSecondFactor: true,
});

Users.setProfile(this.userId, {});
Meteor.methods({
saveUserProfile(settings, customFields, ...args) {
check(settings, Object);
check(customFields, Match.Maybe(Object));

if (customFields && Object.keys(customFields).length) {
saveCustomFields(this.userId, customFields);
if (settings.email || settings.newPassword) {
return saveUserProfileWithTwoFactor.call(this, settings, customFields, ...args);
}

return true;
}),
return saveUserProfile.call(this, settings, customFields);
},
});