diff --git a/app/2fa/server/code/ICodeCheck.ts b/app/2fa/server/code/ICodeCheck.ts index 3c8f6a896fc3a..3cdd9fb6f4e7e 100644 --- a/app/2fa/server/code/ICodeCheck.ts +++ b/app/2fa/server/code/ICodeCheck.ts @@ -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; } diff --git a/app/2fa/server/code/PasswordCheckFallback.ts b/app/2fa/server/code/PasswordCheckFallback.ts index ad11f02711677..ed6a3898d9a88 100644 --- a/app/2fa/server/code/PasswordCheckFallback.ts +++ b/app/2fa/server/code/PasswordCheckFallback.ts @@ -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')) { @@ -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; } diff --git a/app/2fa/server/code/index.ts b/app/2fa/server/code/index.ts index 6bf56df20d66f..c708400c80d6c 100644 --- a/app/2fa/server/code/index.ts +++ b/app/2fa/server/code/index.ts @@ -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(); @@ -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; } @@ -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); } @@ -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) { @@ -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 }); @@ -173,5 +198,3 @@ function _checkCodeForUser({ user, code, method, options = {}, connection }: ICh return true; } - -export const checkCodeForUser = process.env.TEST_MODE ? (): boolean => true : _checkCodeForUser; diff --git a/app/api/server/v1/users.js b/app/api/server/v1/users.js index 2e17b8ff9df59..a7d99d933c728 100644 --- a/app/api/server/v1/users.js +++ b/app/api/server/v1/users.js @@ -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 }) }); }, diff --git a/server/methods/saveUserProfile.js b/server/methods/saveUserProfile.js index 11115c8923c0f..9888331914ee9 100644 --- a/server/methods/saveUserProfile.js +++ b/server/methods/saveUserProfile.js @@ -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) { + // 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); + }, });