diff --git a/_locales/en/messages.json b/_locales/en/messages.json index e168d78c8..be7b4c1e3 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -1733,13 +1733,17 @@ "message": "Password changed" }, "passwordLengthError": { - "message": "Password must be atleast 6 characters long", + "message": "Password must be between 6 and 50 characters long", "description": "Error string shown to the user when password doesn't meet length criteria" }, "passwordTypeError": { "message": "Password must be a string", "description": "Error string shown to the user when password is not a string" }, + "passwordCharacterError": { + "message": "Password must only contain letters, numbers and symbols", + "description": "Error string shown to the user when password contains an invalid character" + }, "change": { "message": "Change" }, diff --git a/app/password_util.js b/app/password_util.js index 736a1c530..d3bbaa44d 100644 --- a/app/password_util.js +++ b/app/password_util.js @@ -1,18 +1,30 @@ const { sha512 } = require('js-sha512'); +const ERRORS = { + TYPE: 'Password must be a string', + LENGTH: 'Password must be between 6 and 50 characters long', + CHARACTER: 'Password must only contain letters, numbers and symbols', +}; + const generateHash = (phrase) => phrase && sha512(phrase.trim()); const matchesHash = (phrase, hash) => phrase && sha512(phrase.trim()) === hash.trim(); const validatePassword = (phrase, i18n) => { - if (typeof phrase !== 'string') { - return i18n ? i18n('passwordTypeError') : 'Password must be a string' + if (!phrase || typeof phrase !== 'string') { + return i18n ? i18n('passwordTypeError') : ERRORS.TYPE; + } + + const trimmed = phrase.trim(); + if (trimmed.length < 6 || trimmed.length > 50) { + return i18n ? i18n('passwordLengthError') : ERRORS.LENGTH; } - if (phrase && phrase.trim().length < 6) { - return i18n ? i18n('passwordLengthError') : 'Password must be atleast 6 characters long'; + // Restrict characters to letters, numbers and symbols + const characterRegex = /^[a-zA-Z0-9-!()._`~@#$%^&*+=[\]{}|<>,;:]+$/ + if (!characterRegex.test(trimmed)) { + return i18n ? i18n('passwordCharacterError') : ERRORS.CHARACTER; } - // An empty password is still valid :P return null; } @@ -20,4 +32,4 @@ module.exports = { generateHash, matchesHash, validatePassword, -}; \ No newline at end of file +}; diff --git a/app/sql.js b/app/sql.js index ea2509e17..be40b0df5 100644 --- a/app/sql.js +++ b/app/sql.js @@ -601,8 +601,9 @@ async function removeIndexedDBFiles() { } // Password hash +const PASS_HASH_ID = 'passHash'; async function getPasswordHash() { - const item = await getItemById('passHash'); + const item = await getItemById(PASS_HASH_ID); return item && item.value; } async function savePasswordHash(hash) { @@ -610,11 +611,11 @@ async function savePasswordHash(hash) { return removePasswordHash(); } - const data = { id: 'passHash', value: hash }; + const data = { id: PASS_HASH_ID, value: hash }; return createOrUpdateItem(data); } async function removePasswordHash() { - return removeItemById('passHash'); + return removeItemById(PASS_HASH_ID); } // Groups diff --git a/js/views/standalone_registration_view.js b/js/views/standalone_registration_view.js index 1c02a1407..be8753f94 100644 --- a/js/views/standalone_registration_view.js +++ b/js/views/standalone_registration_view.js @@ -193,6 +193,11 @@ const input = this.trim(this.$passwordInput.val()); const confirmationInput = this.trim(this.$passwordConfirmationInput.val()); + // If user hasn't set a value then skip + if (!input && !confirmationInput) { + return null; + } + const error = passwordUtil.validatePassword(input, i18n); if (error) { return error; diff --git a/test/app/password_util_test.js b/test/app/password_util_test.js index 8256fe6b5..06293261e 100644 --- a/test/app/password_util_test.js +++ b/test/app/password_util_test.js @@ -7,12 +7,12 @@ describe('Password Util', () => { it('generates the same hash for the same phrase', () => { const first = passwordUtil.generateHash('phrase'); const second = passwordUtil.generateHash('phrase'); - assert.equal(first, second); + assert.strictEqual(first, second); }); it('generates different hashes for different phrases', () => { const first = passwordUtil.generateHash('0'); const second = passwordUtil.generateHash('1'); - assert.notEqual(first, second); + assert.notStrictEqual(first, second); }); }); @@ -33,25 +33,61 @@ describe('Password Util', () => { const valid = [ '123456', '1a5b3C6g', - 'ABC#DE#F$IJ', - 'AabcDegf', + ')CZcy@ccHa', + 'C$D--M;Xv+', + 'X8-;!47IW|', + 'Oi74ZpoSx,p', + '>]K1*g^swHW0]F6}{', + 'TiJf@lk^jsO^z8MUn%)[Sd~UPQ)ci9CGS@jb<^', + '$u&%{r]apg#G@3dQdCkB_p8)gxhNFr=K&yfM_M8O&2Z.vQyvx', + 'bf^OMnYku*iX;{Piw_0zvz', + '#'.repeat(50), ]; valid.forEach(pass => { assert.isNull(passwordUtil.validatePassword(pass)); }); }); - it('should return an error string if password is invalid', () => { + it('should return an error if password is not a string', () => { const invalid = [ 0, 123456, [], {}, - '123', - '1234$', + null, + undefined, ]; invalid.forEach(pass => { - assert.isNotNull(passwordUtil.validatePassword(pass)); + assert.strictEqual(passwordUtil.validatePassword(pass), 'Password must be a string'); + }); + }); + + it('should return an error if password is not between 6 and 50 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'); + }); + }); + + it('should return an error if password has invalid characters', () => { + const invalid = [ + 'ʍʪց3Wͪ݌bΉf', + ')É{b)͎ÔȩҜ٣', + 'ߓܑ˿G֖=3¤)P', + 'ݴ`ԚfĬ8ӝrH(', + 'e̹ωͻܺȬۺ#dӄ', + '谀뤼筎笟ꅅ栗塕카ꭴ', + '俈꛷࿩迭䰡钑럭䛩銛뤙', + '봟㉟ⓓ༭꽫㊡䶷쒨⻯颰', + '<@ȦƘΉوۉaҋ<', + ]; + invalid.forEach(pass => { + assert.strictEqual(passwordUtil.validatePassword(pass), 'Password must only contain letters, numbers and symbols'); }); }); });