From 4f41161a474f35f244e27af24f16314608683035 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 23 Apr 2019 14:55:57 +0100 Subject: [PATCH] Check password complexity during registration This adds a password complexity rule during registration to require strong passwords. This is based on the `zxcvbn` module that we already use for key backup passphrases. In addition, this also tweaks validation more generally to allow rules to be async functions. --- src/components/views/auth/RegistrationForm.js | 64 +++++++++++++++---- src/components/views/elements/Field.js | 4 +- src/components/views/elements/Validation.js | 27 +++++--- src/i18n/strings/en_EN.json | 4 +- src/utils/PasswordScorer.js | 4 +- 5 files changed, 77 insertions(+), 26 deletions(-) diff --git a/src/components/views/auth/RegistrationForm.js b/src/components/views/auth/RegistrationForm.js index a7d7efee0c..2ba01dc0c6 100644 --- a/src/components/views/auth/RegistrationForm.js +++ b/src/components/views/auth/RegistrationForm.js @@ -33,6 +33,8 @@ const FIELD_USERNAME = 'field_username'; const FIELD_PASSWORD = 'field_password'; const FIELD_PASSWORD_CONFIRM = 'field_password_confirm'; +const PASSWORD_MIN_SCORE = 4; // So secure, many characters, much complex, wow, etc, etc. + /** * A pure UI component which displays a registration form. */ @@ -75,13 +77,14 @@ module.exports = React.createClass({ phoneNumber: "", password: "", passwordConfirm: "", + passwordComplexity: null, }; }, - onSubmit: function(ev) { + onSubmit: async function(ev) { ev.preventDefault(); - const allFieldsValid = this.verifyFieldsBeforeSubmit(); + const allFieldsValid = await this.verifyFieldsBeforeSubmit(); if (!allFieldsValid) { return; } @@ -126,7 +129,7 @@ module.exports = React.createClass({ } }, - verifyFieldsBeforeSubmit() { + async verifyFieldsBeforeSubmit() { const fieldIDsInDisplayOrder = [ FIELD_USERNAME, FIELD_PASSWORD, @@ -145,6 +148,10 @@ module.exports = React.createClass({ field.validate({ allowEmpty: false }); } + // Validation and state updates are async, so we need to wait for them to complete + // first. Queue a `setState` callback and wait for it to resolve. + await new Promise(resolve => this.setState({}, resolve)); + if (this.allFieldsValid()) { return true; } @@ -198,8 +205,8 @@ module.exports = React.createClass({ }); }, - onEmailValidate(fieldState) { - const result = this.validateEmailRules(fieldState); + async onEmailValidate(fieldState) { + const result = await this.validateEmailRules(fieldState); this.markFieldValid(FIELD_EMAIL, result.valid); return result; }, @@ -228,13 +235,21 @@ module.exports = React.createClass({ }); }, - onPasswordValidate(fieldState) { - const result = this.validatePasswordRules(fieldState); + async onPasswordValidate(fieldState) { + const result = await this.validatePasswordRules(fieldState); this.markFieldValid(FIELD_PASSWORD, result.valid); return result; }, validatePasswordRules: withValidation({ + description: function() { + const complexity = this.state.passwordComplexity; + const score = complexity ? complexity.score : 0; + return ; + }, rules: [ { key: "required", @@ -250,6 +265,29 @@ module.exports = React.createClass({ return _t("Too short (min %(length)s)", { length: this.props.minPasswordLength }); }, }, + { + key: "complexity", + test: async function({ value }) { + if (!value) { + return false; + } + const { scorePassword } = await import('../../../utils/PasswordScorer'); + const complexity = scorePassword(value); + this.setState({ + passwordComplexity: complexity, + }); + return complexity.score >= PASSWORD_MIN_SCORE; + }, + valid: () => _t("Nice, strong password!"), + invalid: function() { + const complexity = this.state.passwordComplexity; + if (!complexity) { + return null; + } + const { feedback } = complexity; + return feedback.warning || feedback.suggestions[0] || _t("Keep going..."); + }, + }, ], }), @@ -259,8 +297,8 @@ module.exports = React.createClass({ }); }, - onPasswordConfirmValidate(fieldState) { - const result = this.validatePasswordConfirmRules(fieldState); + async onPasswordConfirmValidate(fieldState) { + const result = await this.validatePasswordConfirmRules(fieldState); this.markFieldValid(FIELD_PASSWORD_CONFIRM, result.valid); return result; }, @@ -295,8 +333,8 @@ module.exports = React.createClass({ }); }, - onPhoneNumberValidate(fieldState) { - const result = this.validatePhoneNumberRules(fieldState); + async onPhoneNumberValidate(fieldState) { + const result = await this.validatePhoneNumberRules(fieldState); this.markFieldValid(FIELD_PHONE_NUMBER, result.valid); return result; }, @@ -325,8 +363,8 @@ module.exports = React.createClass({ }); }, - onUsernameValidate(fieldState) { - const result = this.validateUsernameRules(fieldState); + async onUsernameValidate(fieldState) { + const result = await this.validateUsernameRules(fieldState); this.markFieldValid(FIELD_USERNAME, result.valid); return result; }, diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 6eba832523..8f0746dff2 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -87,12 +87,12 @@ export default class Field extends React.PureComponent { this.input.focus(); } - validate({ focused, allowEmpty = true }) { + async validate({ focused, allowEmpty = true }) { if (!this.props.onValidate) { return; } const value = this.input ? this.input.value : null; - const { valid, feedback } = this.props.onValidate({ + const { valid, feedback } = await this.props.onValidate({ value, focused, allowEmpty, diff --git a/src/components/views/elements/Validation.js b/src/components/views/elements/Validation.js index b567eb4058..c37970e534 100644 --- a/src/components/views/elements/Validation.js +++ b/src/components/views/elements/Validation.js @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +/* eslint-disable babel/no-invalid-this */ + import classNames from 'classnames'; /** @@ -34,7 +36,7 @@ import classNames from 'classnames'; * the overall validity and a feedback UI that can be rendered for more detail. */ export default function withValidation({ description, rules }) { - return function onValidate({ value, focused, allowEmpty = true }) { + return async function onValidate({ value, focused, allowEmpty = true }) { // TODO: Re-run only after ~200ms of inactivity if (!value && allowEmpty) { return { @@ -50,29 +52,35 @@ export default function withValidation({ description, rules }) { if (!rule.key || !rule.test) { continue; } - // We're setting `this` to whichever component hold the validation + // We're setting `this` to whichever component holds the validation // function. That allows rules to access the state of the component. - /* eslint-disable babel/no-invalid-this */ - const ruleValid = rule.test.call(this, { value, allowEmpty }); + const ruleValid = await rule.test.call(this, { value, allowEmpty }); valid = valid && ruleValid; if (ruleValid && rule.valid) { // If the rule's result is valid and has text to show for // the valid state, show it. + const text = rule.valid.call(this); + if (!text) { + continue; + } results.push({ key: rule.key, valid: true, - text: rule.valid.call(this), + text, }); } else if (!ruleValid && rule.invalid) { // If the rule's result is invalid and has text to show for // the invalid state, show it. + const text = rule.invalid.call(this); + if (!text) { + continue; + } results.push({ key: rule.key, valid: false, - text: rule.invalid.call(this), + text, }); } - /* eslint-enable babel/no-invalid-this */ } } @@ -102,7 +110,10 @@ export default function withValidation({ description, rules }) { let summary; if (description) { - summary =
{description()}
; + // We're setting `this` to whichever component holds the validation + // function. That allows rules to access the state of the component. + const content = description.call(this); + summary =
{content}
; } let feedback; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 5fd97a4305..262e143ae6 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1327,6 +1327,8 @@ "Doesn't look like a valid email address": "Doesn't look like a valid email address", "Enter password": "Enter password", "Too short (min %(length)s)": "Too short (min %(length)s)", + "Nice, strong password!": "Nice, strong password!", + "Keep going...": "Keep going...", "Passwords don't match": "Passwords don't match", "Other users can invite you to rooms using your contact details": "Other users can invite you to rooms using your contact details", "Enter phone number (required on this homeserver)": "Enter phone number (required on this homeserver)", @@ -1528,7 +1530,6 @@ "Registration has been disabled on this homeserver.": "Registration has been disabled on this homeserver.", "Unable to query for supported registration methods.": "Unable to query for supported registration methods.", "This server does not support authentication with a phone number.": "This server does not support authentication with a phone number.", - "An unknown error occurred.": "An unknown error occurred.", "Create your account": "Create your account", "Commands": "Commands", "Results from DuckDuckGo": "Results from DuckDuckGo", @@ -1567,7 +1568,6 @@ "File to import": "File to import", "Import": "Import", "Great! This passphrase looks strong enough.": "Great! This passphrase looks strong enough.", - "Keep going...": "Keep going...", "We'll store an encrypted copy of your keys on our server. Protect your backup with a passphrase to keep it secure.": "We'll store an encrypted copy of your keys on our server. Protect your backup with a passphrase to keep it secure.", "For maximum security, this should be different from your account password.": "For maximum security, this should be different from your account password.", "Enter a passphrase...": "Enter a passphrase...", diff --git a/src/utils/PasswordScorer.js b/src/utils/PasswordScorer.js index 647436c131..3c366a73f8 100644 --- a/src/utils/PasswordScorer.js +++ b/src/utils/PasswordScorer.js @@ -67,7 +67,9 @@ export function scorePassword(password) { if (password.length === 0) return null; const userInputs = ZXCVBN_USER_INPUTS.slice(); - userInputs.push(MatrixClientPeg.get().getUserIdLocalpart()); + if (MatrixClientPeg.get()) { + userInputs.push(MatrixClientPeg.get().getUserIdLocalpart()); + } let zxcvbnResult = zxcvbn(password, userInputs); // Work around https://github.com/dropbox/zxcvbn/issues/216