From 191cfb1d572356b89c0ed75a6786744a4d6129c6 Mon Sep 17 00:00:00 2001 From: Colby Gutierrez-Kraybill Date: Thu, 22 Sep 2022 00:23:59 -0400 Subject: [PATCH] Add check on password complexity by using a backend call without divulging the list being checked against. --- src/components/join-flow/username-step.jsx | 39 ++++++++++++++++---- src/lib/validate.js | 41 ++++++++++++++++++++-- test/unit/lib/validate.test.js | 22 ++++++------ 3 files changed, 83 insertions(+), 19 deletions(-) diff --git a/src/components/join-flow/username-step.jsx b/src/components/join-flow/username-step.jsx index ee8b72008..64d763a11 100644 --- a/src/components/join-flow/username-step.jsx +++ b/src/components/join-flow/username-step.jsx @@ -24,6 +24,7 @@ class UsernameStep extends React.Component { 'handleSetUsernameRef', 'handleValidSubmit', 'validatePasswordIfPresent', + 'validatePasswordRemotelyWithCache', 'validatePasswordConfirmIfPresent', 'validateUsernameIfPresent', 'validateUsernameRemotelyWithCache', @@ -32,9 +33,10 @@ class UsernameStep extends React.Component { this.state = { focused: null }; - // simple object to memoize remote requests for usernames. + // memoize remote requests for username check and password weakness // keeps us from submitting multiple requests for same data. this.usernameRemoteCache = Object.create(null); + this.passwordRemoteCache = Object.create(null); } componentDidMount () { // Send info to analytics when we aren't on the standalone page. @@ -92,12 +94,37 @@ class UsernameStep extends React.Component { } ); } + // memoize remote requests for weak password check + validatePasswordRemotelyWithCache (password) { + if (typeof this.passwordRemoteCache[password] === 'object') { + return Promise.resolve(this.passwordRemoteCache[password]); + } + // password is not in our cache + return validate.validatePasswordRemotely(password).then( + remoteResult => { + // cache result, if it successfully heard back from server + if (remoteResult.requestSucceeded) { + this.passwordRemoteCache[password] = remoteResult; + } + return remoteResult; + } + ); + } validatePasswordIfPresent (password, username) { if (!password) return null; // skip validation if password is blank; null indicates valid - const localResult = validate.validatePassword(password, username); - if (localResult.valid) return null; - return this.props.intl.formatMessage({id: localResult.errMsgId}); - } + // if password is not blank, run both local and remote validations + const localResult = validate.validatePasswordLocally(password, username); + return this.validatePasswordRemotelyWithCache(password).then( + remoteResult => { + if (localResult.valid === false) { // defer to local check first + return this.props.intl.formatMessage({id: localResult.errMsgId}); + } else if (remoteResult.valid === false) { + return this.props.intl.formatMessage({id: remoteResult.errMsgId}); + } + return null; + } // remoteResult + ); // validatePasswordRemotelyWithCache + } // validatePasswordIfPresent validatePasswordConfirmIfPresent (password, passwordConfirm) { if (!passwordConfirm) return null; // allow blank password if not submitting yet const localResult = validate.validatePasswordConfirm(password, passwordConfirm); @@ -114,7 +141,7 @@ class UsernameStep extends React.Component { if (!usernameResult.valid) { errors.username = this.props.intl.formatMessage({id: usernameResult.errMsgId}); } - const passwordResult = validate.validatePassword(values.password, values.username); + const passwordResult = validate.validatePasswordLocally(values.password, values.username); if (!passwordResult.valid) { errors.password = this.props.intl.formatMessage({id: passwordResult.errMsgId}); } diff --git a/src/lib/validate.js b/src/lib/validate.js index d97b46a64..ae4408650 100644 --- a/src/lib/validate.js +++ b/src/lib/validate.js @@ -54,7 +54,7 @@ module.exports.validateUsernameRemotely = username => ( * @param {string} username username value to compare * @return {object} {valid: boolean, errMsgId: string} */ -module.exports.validatePassword = (password, username) => { +module.exports.validatePasswordLocally = (password, username) => { if (!password) { return {valid: false, errMsgId: 'general.required'}; // Using Array.from(string).length, instead of string.length, improves unicode @@ -67,7 +67,7 @@ module.exports.validatePassword = (password, username) => { // https://stackoverflow.com/a/54370584/2308190 } else if (Array.from(password).length < 6) { return {valid: false, errMsgId: 'registration.validationPasswordLength'}; - } else if (password === 'password') { + } else if (password.toLowerCase() === 'password') { return {valid: false, errMsgId: 'registration.validationPasswordNotEquals'}; } else if (username && password === username) { return {valid: false, errMsgId: 'registration.validationPasswordNotUsername'}; @@ -75,6 +75,43 @@ module.exports.validatePassword = (password, username) => { return {valid: true}; }; +module.exports.validatePasswordRemotely = password => ( + new Promise(resolve => { + api({ + method: 'POST', + uri: `/accounts/checkpassword`, + json: { + password: `${password}` + } + }, (err, body, res) => { + if (err || res.statusCode !== 200) { + return resolve({ + requestSucceeded: false, + valid: false, + errMsgId: 'general.error' + }); + } + let msg = ''; + if (body && body.msg) msg = body.msg; + else if (body && body[0]) msg = body[0].msg; + switch (msg) { + case 'valid password': + return resolve({ + requestSucceeded: true, + valid: true + }); + case 'valid password': + default: + return resolve({ + requestSucceeded: true, + valid: false, + errMsgId: 'registration.validationPasswordNotEquals' + }); + }; // switch + }); // api + }) // promise +); + module.exports.validatePasswordConfirm = (password, passwordConfirm) => { if (!passwordConfirm) { return {valid: false, errMsgId: 'general.required'}; diff --git a/test/unit/lib/validate.test.js b/test/unit/lib/validate.test.js index ab3f5fc97..fd2644baa 100644 --- a/test/unit/lib/validate.test.js +++ b/test/unit/lib/validate.test.js @@ -55,38 +55,38 @@ describe('unit test lib/validate.js', () => { test('validate password existence', () => { let response; expect(typeof validate.validatePassword).toBe('function'); - response = validate.validatePassword('abcdef'); + response = validate.validatePasswordLocally('abcdef'); expect(response).toEqual({valid: true}); - response = validate.validatePassword(''); + response = validate.validatePasswordLocally(''); expect(response).toEqual({valid: false, errMsgId: 'general.required'}); }); test('validate password length', () => { let response; - response = validate.validatePassword('abcdefghijklmnopqrst'); + response = validate.validatePasswordLocally('abcdefghijklmnopqrst'); expect(response).toEqual({valid: true}); - response = validate.validatePassword('abcde'); + response = validate.validatePasswordLocally('abcde'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordLength'}); - response = validate.validatePassword('😺'); + response = validate.validatePasswordLocally('😺'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordLength'}); - response = validate.validatePassword('😺🦆🐝'); + response = validate.validatePasswordLocally('😺🦆🐝'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordLength'}); - response = validate.validatePassword('😺🦆🐝🐮🐠'); + response = validate.validatePasswordLocally('😺🦆🐝🐮🐠'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordLength'}); - response = validate.validatePassword('😺🦆🐝🐮🐠🐻'); + response = validate.validatePasswordLocally('😺🦆🐝🐮🐠🐻'); expect(response).toEqual({valid: true}); }); test('validate password cannot be "password"', () => { - const response = validate.validatePassword('password'); + const response = validate.validatePasswordLocally('password'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordNotEquals'}); }); test('validate password cannot be same as username', () => { let response; - response = validate.validatePassword('abcdefg', 'abcdefg'); + response = validate.validatePasswordLocally('abcdefg', 'abcdefg'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordNotUsername'}); - response = validate.validatePassword('abcdefg', 'abcdefG'); + response = validate.validatePasswordLocally('abcdefg', 'abcdefG'); expect(response).toEqual({valid: true}); });