From 43ec14e74181e52f906e5b2092b477d294e72e7c Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 22 Jan 2021 16:29:02 +1100 Subject: [PATCH] Fix the password length limit when not setting a new password Relates #1446 --- _locales/en/messages.json | 2 +- _locales/fr/messages.json | 2 +- _locales/ru/messages.json | 2 +- password_preload.js | 1 - preload.js | 1 - ts/components/Lightbox.tsx | 5 +- ts/components/session/RegistrationTabs.tsx | 5 +- .../session/SessionPasswordModal.tsx | 241 +++++++++--------- .../session/SessionPasswordPrompt.tsx | 21 -- ts/components/session/SessionSeedModal.tsx | 7 +- .../session/settings/SessionSettings.tsx | 7 +- .../test/session/unit/utils/Password.ts | 37 ++- ts/util/passwordUtils.ts | 9 +- ts/window.d.ts | 1 - 14 files changed, 156 insertions(+), 185 deletions(-) rename test/app/password_util_test.js => ts/test/session/unit/utils/Password.ts (67%) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index f82c6c8eb..d008c8873 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -1915,7 +1915,7 @@ "message": "Failed to set password" }, "passwordLengthError": { - "message": "Password must be between 6 and 50 characters long", + "message": "Password must be between 6 and 64 characters long", "description": "Error string shown to the user when password doesn't meet length criteria" }, "passwordTypeError": { diff --git a/_locales/fr/messages.json b/_locales/fr/messages.json index e507f7352..1b68565be 100644 --- a/_locales/fr/messages.json +++ b/_locales/fr/messages.json @@ -1698,7 +1698,7 @@ "message": "Échec de la définition du mot de passe" }, "passwordLengthError": { - "message": "Le mot de passe doit avoir une longueur comprise entre 6 et 50 caractères", + "message": "Le mot de passe doit avoir une longueur comprise entre 6 et 64 caractères", "description": "Error string shown to the user when password doesn't meet length criteria" }, "passwordTypeError": { diff --git a/_locales/ru/messages.json b/_locales/ru/messages.json index 9870f1a0d..06233c395 100644 --- a/_locales/ru/messages.json +++ b/_locales/ru/messages.json @@ -1698,7 +1698,7 @@ "message": "Failed to set password" }, "passwordLengthError": { - "message": "Password must be between 6 and 50 characters long", + "message": "Password must be between 6 and 64 characters long", "description": "Error string shown to the user when password doesn't meet length criteria" }, "passwordTypeError": { diff --git a/password_preload.js b/password_preload.js index d5c29af89..2e4f0693e 100644 --- a/password_preload.js +++ b/password_preload.js @@ -40,7 +40,6 @@ window.CONSTANTS = { MAX_USERNAME_LENGTH: 20, }; -window.passwordUtil = require('./ts/util/passwordUtils'); window.Signal.Logs = require('./js/modules/logs'); window.resetDatabase = () => { diff --git a/preload.js b/preload.js index baa2f30f2..1319bfc36 100644 --- a/preload.js +++ b/preload.js @@ -173,7 +173,6 @@ window.setPassword = (passPhrase, oldPhrase) => ipc.send('set-password', passPhrase, oldPhrase); }); -window.passwordUtil = require('./ts/util/passwordUtils'); window.libsession = require('./ts/session'); window.getMessageController = diff --git a/ts/components/Lightbox.tsx b/ts/components/Lightbox.tsx index 2a7b32d21..790ba2295 100644 --- a/ts/components/Lightbox.tsx +++ b/ts/components/Lightbox.tsx @@ -237,8 +237,7 @@ export class Lightbox extends React.Component { } if (current.paused) { - // tslint:disable-next-line no-floating-promises - current.play(); + void current.play(); } else { current.pause(); } @@ -272,7 +271,7 @@ export class Lightbox extends React.Component {
- + { error={this.state.passwordErrorString} type="password" placeholder={window.i18n('enterOptionalPassword')} - maxLength={window.CONSTANTS.MAX_PASSWORD_LENGTH} onValueChanged={(val: string) => { this.onPasswordChanged(val); }} @@ -470,7 +470,6 @@ export class RegistrationTabs extends React.Component { error={passwordsDoNotMatch} type="password" placeholder={window.i18n('confirmPassword')} - maxLength={window.CONSTANTS.MAX_PASSWORD_LENGTH} onValueChanged={(val: string) => { this.onPasswordVerifyChanged(val); }} @@ -592,7 +591,7 @@ export class RegistrationTabs extends React.Component { return; } - const error = window.passwordUtil.validatePassword(input, window.i18n); + const error = PasswordUtil.validatePassword(input, window.i18n); if (error) { this.setState({ passwordErrorString: error, diff --git a/ts/components/session/SessionPasswordModal.tsx b/ts/components/session/SessionPasswordModal.tsx index b31919512..a6f5a4ace 100644 --- a/ts/components/session/SessionPasswordModal.tsx +++ b/ts/components/session/SessionPasswordModal.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { SessionModal } from './SessionModal'; import { SessionButton, SessionButtonColor } from './SessionButton'; -import { PasswordUtil } from '../../util/'; +import { missingCaseError, PasswordUtil } from '../../util/'; import { ToastUtils } from '../../session/utils'; import { toast } from 'react-toastify'; import { SessionToast, SessionToastType } from './SessionToast'; @@ -46,8 +46,6 @@ class SessionPasswordModalInner extends React.Component { this.onPasswordInput = this.onPasswordInput.bind(this); this.onPasswordConfirmInput = this.onPasswordConfirmInput.bind(this); - - this.onPaste = this.onPaste.bind(this); } public componentDidMount() { @@ -86,8 +84,6 @@ class SessionPasswordModalInner extends React.Component { }} placeholder={placeholders[0]} onKeyUp={this.onPasswordInput} - maxLength={window.CONSTANTS.MAX_PASSWORD_LENGTH} - onPaste={this.onPaste} /> {action !== PasswordAction.Remove && ( { id="password-modal-input-confirm" placeholder={placeholders[1]} onKeyUp={this.onPasswordConfirmInput} - maxLength={window.CONSTANTS.MAX_PASSWORD_LENGTH} - onPaste={this.onPaste} /> )}
@@ -108,7 +102,7 @@ class SessionPasswordModalInner extends React.Component { this.setPassword(onOk)} + onClick={this.setPassword} /> { ); } + /** + * Returns false and set the state error field in the input is not a valid password + * or returns true + */ + private validatePassword(firstPassword: string) { + // if user did not fill the first password field, we can't do anything + const errorFirstInput = PasswordUtil.validatePassword( + firstPassword, + window.i18n + ); + if (errorFirstInput !== null) { + this.setState({ + error: errorFirstInput, + }); + return false; + } + return true; + } + + private async handleActionSet( + enteredPassword: string, + enteredPasswordConfirm: string + ) { + // be sure both password are valid + if (!this.validatePassword(enteredPassword)) { + return; + } + // no need to validate second password. we just need to check that enteredPassword is valid, and that both password matches + + if (enteredPassword !== enteredPasswordConfirm) { + this.setState({ + error: window.i18n('setPasswordInvalid'), + }); + return; + } + await window.setPassword(enteredPassword, null); + ToastUtils.pushToastSuccess( + 'setPasswordSuccessToast', + window.i18n('setPasswordTitle'), + window.i18n('setPasswordToastDescription'), + SessionIconType.Lock + ); + + this.props.onOk(this.props.action); + this.closeDialog(); + } + + private async handleActionChange(oldPassword: string, newPassword: string) { + // We don't validate oldPassword on change: this is validate on the validatePasswordHash below + // we only validate the newPassword here + if (!this.validatePassword(newPassword)) { + return; + } + const isValidWithStoredInDB = Boolean( + await this.validatePasswordHash(oldPassword) + ); + if (!isValidWithStoredInDB) { + this.setState({ + error: window.i18n('changePasswordInvalid'), + }); + return; + } + await window.setPassword(newPassword, oldPassword); + + ToastUtils.pushToastSuccess( + 'setPasswordSuccessToast', + window.i18n('changePasswordTitle'), + window.i18n('changePasswordToastDescription'), + SessionIconType.Lock + ); + + this.props.onOk(this.props.action); + this.closeDialog(); + } + + private async handleActionRemove(oldPassword: string) { + // We don't validate oldPassword on change: this is validate on the validatePasswordHash below + const isValidWithStoredInDB = Boolean( + await this.validatePasswordHash(oldPassword) + ); + if (!isValidWithStoredInDB) { + this.setState({ + error: window.i18n('removePasswordInvalid'), + }); + return; + } + await window.setPassword(null, oldPassword); + + ToastUtils.pushToastWarning( + 'setPasswordSuccessToast', + window.i18n('removePasswordTitle'), + window.i18n('removePasswordToastDescription') + ); + + this.props.onOk(this.props.action); + this.closeDialog(); + } + // tslint:disable-next-line: cyclomatic-complexity - private async setPassword(onSuccess?: any) { + private async setPassword() { const { action } = this.props; const { currentPasswordEntered, @@ -155,106 +247,28 @@ class SessionPasswordModalInner extends React.Component { const { Set, Remove, Change } = PasswordAction; // Trim leading / trailing whitespace for UX - const enteredPassword = (currentPasswordEntered || '').trim(); - const enteredPasswordConfirm = (currentPasswordConfirmEntered || '').trim(); + const firstPasswordEntered = (currentPasswordEntered || '').trim(); + const secondPasswordEntered = (currentPasswordConfirmEntered || '').trim(); - // if user did not fill the first password field, we can't do anything - const errorFirstInput = PasswordUtil.validatePassword( - enteredPassword, - window.i18n - ); - if (errorFirstInput !== null) { - this.setState({ - error: errorFirstInput, - }); - return; - } - - // if action is Set or Change, we need a valid ConfirmPassword - if (action === Set || action === Change) { - const errorSecondInput = PasswordUtil.validatePassword( - enteredPasswordConfirm, - window.i18n - ); - if (errorSecondInput !== null) { - this.setState({ - error: errorSecondInput, - }); + switch (action) { + case Set: { + await this.handleActionSet(firstPasswordEntered, secondPasswordEntered); return; } - } - - // Passwords match or remove password successful - const newPassword = action === Remove ? null : enteredPasswordConfirm; - const oldPassword = action === Set ? null : enteredPassword; - - // Check if password match, when setting, changing or removing - let valid; - if (action === Set) { - valid = enteredPassword === enteredPasswordConfirm; - } else { - valid = Boolean(await this.validatePasswordHash(oldPassword)); - } - - if (!valid) { - let str; - switch (action) { - case Set: - str = window.i18n('setPasswordInvalid'); - break; - case Change: - str = window.i18n('changePasswordInvalid'); - break; - case Remove: - str = window.i18n('removePasswordInvalid'); - break; - default: - throw new Error(`Invalid action ${action}`); + case Change: { + await this.handleActionChange( + firstPasswordEntered, + secondPasswordEntered + ); + return; + } + case Remove: { + await this.handleActionRemove(firstPasswordEntered); + return; } - this.setState({ - error: str, - }); - - return; - } - - await window.setPassword(newPassword, oldPassword); - let title; - let description; - switch (action) { - case Set: - title = window.i18n('setPasswordTitle'); - description = window.i18n('setPasswordToastDescription'); - break; - case Change: - title = window.i18n('changePasswordTitle'); - description = window.i18n('changePasswordToastDescription'); - break; - case Remove: - title = window.i18n('removePasswordTitle'); - description = window.i18n('removePasswordToastDescription'); - break; default: - throw new Error(`Invalid action ${action}`); + throw missingCaseError(action); } - - if (action !== Remove) { - ToastUtils.pushToastSuccess( - 'setPasswordSuccessToast', - title, - description, - SessionIconType.Lock - ); - } else { - ToastUtils.pushToastWarning( - 'setPasswordSuccessToast', - title, - description - ); - } - - onSuccess(this.props.action); - this.closeDialog(); } private closeDialog() { @@ -263,26 +277,9 @@ class SessionPasswordModalInner extends React.Component { } } - private onPaste(event: any) { - const clipboard = event.clipboardData.getData('text'); - - if (clipboard.length > window.CONSTANTS.MAX_PASSWORD_LENGTH) { - const title = String( - window.i18n( - 'pasteLongPasswordToastTitle', - window.CONSTANTS.MAX_PASSWORD_LENGTH - ) - ); - ToastUtils.pushToastWarning('passwordModal', title); - } - - // Prevent pating into input - return false; - } - private async onPasswordInput(event: any) { if (event.key === 'Enter') { - return this.setPassword(this.props.onOk); + return this.setPassword(); } const currentPasswordEntered = event.target.value; @@ -291,7 +288,7 @@ class SessionPasswordModalInner extends React.Component { private async onPasswordConfirmInput(event: any) { if (event.key === 'Enter') { - return this.setPassword(this.props.onOk); + return this.setPassword(); } const currentPasswordConfirmEntered = event.target.value; diff --git a/ts/components/session/SessionPasswordPrompt.tsx b/ts/components/session/SessionPasswordPrompt.tsx index fa6f4d061..07b30ff00 100644 --- a/ts/components/session/SessionPasswordPrompt.tsx +++ b/ts/components/session/SessionPasswordPrompt.tsx @@ -32,7 +32,6 @@ class SessionPasswordPromptInner extends React.PureComponent< }; this.onKeyUp = this.onKeyUp.bind(this); - this.onPaste = this.onPaste.bind(this); this.initLogin = this.initLogin.bind(this); this.initClearDataView = this.initClearDataView.bind(this); @@ -72,8 +71,6 @@ class SessionPasswordPromptInner extends React.PureComponent< defaultValue="" placeholder={' '} onKeyUp={this.onKeyUp} - maxLength={window.CONSTANTS.MAX_PASSWORD_LENGTH} - onPaste={this.onPaste} ref={this.inputRef} /> ); @@ -135,24 +132,6 @@ class SessionPasswordPromptInner extends React.PureComponent< event.preventDefault(); } - public onPaste(event: any) { - const clipboard = event.clipboardData.getData('text'); - - if (clipboard.length > window.CONSTANTS.MAX_PASSWORD_LENGTH) { - this.setState({ - error: String( - window.i18n( - 'pasteLongPasswordToastTitle', - window.CONSTANTS.MAX_PASSWORD_LENGTH - ) - ), - }); - } - - // Prevent pasting into input - return false; - } - public async onLogin(passPhrase: string) { const passPhraseTrimmed = passPhrase.trim(); diff --git a/ts/components/session/SessionSeedModal.tsx b/ts/components/session/SessionSeedModal.tsx index 207dac966..3b9f30b04 100644 --- a/ts/components/session/SessionSeedModal.tsx +++ b/ts/components/session/SessionSeedModal.tsx @@ -4,6 +4,7 @@ import { SessionModal } from './SessionModal'; import { SessionButton } from './SessionButton'; import { ToastUtils } from '../../session/utils'; import { DefaultTheme, withTheme } from 'styled-components'; +import { PasswordUtil } from '../../util'; interface Props { onClose: any; @@ -77,7 +78,6 @@ class SessionSeedModalInner extends React.Component { } private renderPasswordView() { - const maxPasswordLen = 64; const error = this.state.error; const i18n = window.i18n; const { onClose } = this.props; @@ -90,7 +90,6 @@ class SessionSeedModalInner extends React.Component { id="seed-input-password" placeholder={i18n('password')} onKeyUp={this.onEnter} - maxLength={maxPasswordLen} /> {error && ( @@ -143,8 +142,8 @@ class SessionSeedModalInner extends React.Component { private confirmPassword() { const passwordHash = this.state.passwordHash; const passwordValue = jQuery('#seed-input-password').val(); - const isPasswordValid = window.passwordUtil.matchesHash( - passwordValue, + const isPasswordValid = PasswordUtil.matchesHash( + passwordValue as string, passwordHash ); diff --git a/ts/components/session/settings/SessionSettings.tsx b/ts/components/session/settings/SessionSettings.tsx index 0ea3e995c..63c5b6a28 100644 --- a/ts/components/session/settings/SessionSettings.tsx +++ b/ts/components/session/settings/SessionSettings.tsx @@ -7,7 +7,7 @@ import { SessionButtonColor, SessionButtonType, } from '../SessionButton'; -import { BlockedNumberController } from '../../../util'; +import { BlockedNumberController, PasswordUtil } from '../../../util'; import { ToastUtils } from '../../../session/utils'; import { ConversationLookupType } from '../../../state/ducks/conversations'; import { StateType } from '../../../state/reducer'; @@ -172,8 +172,7 @@ class SettingsViewInner extends React.Component { type="password" id="password-lock-input" defaultValue="" - placeholder={' '} - maxLength={window.CONSTANTS.MAX_PASSWORD_LENGTH} + placeholder="Password" />
@@ -211,7 +210,7 @@ class SettingsViewInner extends React.Component { // Check if the password matches the hash we have stored const hash = await window.Signal.Data.getPasswordHash(); - if (hash && !window.passwordUtil.matchesHash(enteredPassword, hash)) { + if (hash && !PasswordUtil.matchesHash(enteredPassword, hash)) { this.setState({ pwdLockError: window.i18n('invalidPassword'), }); diff --git a/test/app/password_util_test.js b/ts/test/session/unit/utils/Password.ts similarity index 67% rename from test/app/password_util_test.js rename to ts/test/session/unit/utils/Password.ts index 7037ec32e..f52d78e2f 100644 --- a/test/app/password_util_test.js +++ b/ts/test/session/unit/utils/Password.ts @@ -1,17 +1,16 @@ -const { assert } = require('chai'); - -const passwordUtil = require('../../ts/util/passwordUtils'); +import { assert } from 'chai'; +import { PasswordUtil } from '../../../../util'; describe('Password Util', () => { describe('hash generation', () => { it('generates the same hash for the same phrase', () => { - const first = passwordUtil.generateHash('phrase'); - const second = passwordUtil.generateHash('phrase'); + const first = PasswordUtil.generateHash('phrase'); + const second = PasswordUtil.generateHash('phrase'); assert.strictEqual(first, second); }); it('generates different hashes for different phrases', () => { - const first = passwordUtil.generateHash('0'); - const second = passwordUtil.generateHash('1'); + const first = PasswordUtil.generateHash('0'); + const second = PasswordUtil.generateHash('1'); assert.notStrictEqual(first, second); }); }); @@ -19,12 +18,12 @@ describe('Password Util', () => { describe('hash matching', () => { it('returns true for the same hash', () => { const phrase = 'phrase'; - const hash = passwordUtil.generateHash(phrase); - assert.isTrue(passwordUtil.matchesHash(phrase, hash)); + const hash = PasswordUtil.generateHash(phrase); + assert.isTrue(PasswordUtil.matchesHash(phrase, hash)); }); it('returns false for different hashes', () => { - const hash = passwordUtil.generateHash('phrase'); - assert.isFalse(passwordUtil.matchesHash('phrase2', hash)); + const hash = PasswordUtil.generateHash('phrase'); + assert.isFalse(PasswordUtil.matchesHash('phrase2', hash)); }); }); @@ -44,26 +43,26 @@ describe('Password Util', () => { '#'.repeat(50), ]; valid.forEach(pass => { - assert.isNull(passwordUtil.validatePassword(pass)); + assert.isNull(PasswordUtil.validatePassword(pass)); }); }); it('should return an error if password is not a string', () => { - const invalid = [0, 123456, [], {}, null, undefined]; - invalid.forEach(pass => { + const invalid = [0, 123456, [], {}, null, undefined] as any; + invalid.forEach((pass: any) => { assert.strictEqual( - passwordUtil.validatePassword(pass), + PasswordUtil.validatePassword(pass), 'Password must be a string' ); }); }); - it('should return an error if password is not between 6 and 50 characters', () => { + it('should return an error if password is not between 6 and 64 characters', () => { const invalid = ['a', 'abcde', '#'.repeat(51), '#'.repeat(100)]; invalid.forEach(pass => { assert.strictEqual( - passwordUtil.validatePassword(pass), - 'Password must be between 6 and 50 characters long' + PasswordUtil.validatePassword(pass), + 'Password must be between 6 and 64 characters long' ); }); }); @@ -82,7 +81,7 @@ describe('Password Util', () => { ]; invalid.forEach(pass => { assert.strictEqual( - passwordUtil.validatePassword(pass), + PasswordUtil.validatePassword(pass), 'Password must only contain letters, numbers and symbols' ); }); diff --git a/ts/util/passwordUtils.ts b/ts/util/passwordUtils.ts index ea2ca0d24..48ba154a3 100644 --- a/ts/util/passwordUtils.ts +++ b/ts/util/passwordUtils.ts @@ -3,7 +3,7 @@ import { LocalizerType } from '../types/Util'; const ERRORS = { TYPE: 'Password must be a string', - LENGTH: 'Password must be between 6 and 50 characters long', + LENGTH: 'Password must be between 6 and 64 characters long', CHARACTER: 'Password must only contain letters, numbers and symbols', }; @@ -17,7 +17,7 @@ export const generateHash = (phrase: string) => phrase && sha512(phrase.trim()); export const matchesHash = (phrase: string | null, hash: string) => phrase && sha512(phrase.trim()) === hash.trim(); -export const validatePassword = (phrase: string, i18n: LocalizerType) => { +export const validatePassword = (phrase: string, i18n?: LocalizerType) => { if (typeof phrase !== 'string') { return i18n ? i18n('passwordTypeError') : ERRORS.TYPE; } @@ -27,7 +27,10 @@ export const validatePassword = (phrase: string, i18n: LocalizerType) => { return i18n ? i18n('noGivenPassword') : ERRORS.LENGTH; } - if (trimmed.length < 6 || trimmed.length > 50) { + if ( + trimmed.length < 6 || + trimmed.length > window.CONSTANTS.MAX_PASSWORD_LENGTH + ) { return i18n ? i18n('passwordLengthError') : ERRORS.LENGTH; } diff --git a/ts/window.d.ts b/ts/window.d.ts index c79d74d01..1c8190c9c 100644 --- a/ts/window.d.ts +++ b/ts/window.d.ts @@ -72,7 +72,6 @@ declare global { lokiSnodeAPI: LokiSnodeAPI; mnemonic: RecoveryPhraseUtil; onLogin: any; - passwordUtil: any; resetDatabase: any; restart: any; seedNodeList: any;