From 008ca3543bdfe6afefd4d37331c1b193718ce5ab Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 18 Apr 2019 23:29:05 +0100 Subject: [PATCH] Migrate passwords on registration to new validation In addition to migrating password fields, this also removes the remaining support for old-style validation in registration now that all checks have been converted. --- .../structures/auth/Registration.js | 35 ---- src/components/views/auth/RegistrationForm.js | 191 +++++++----------- src/components/views/elements/Validation.js | 7 +- src/i18n/strings/en_EN.json | 8 +- 4 files changed, 85 insertions(+), 156 deletions(-) diff --git a/src/components/structures/auth/Registration.js b/src/components/structures/auth/Registration.js index 90e7d99bfe..87fea3ec4b 100644 --- a/src/components/structures/auth/Registration.js +++ b/src/components/structures/auth/Registration.js @@ -308,40 +308,6 @@ module.exports = React.createClass({ }); }, - onFormValidationChange: function(fieldErrors) { - // `fieldErrors` is an object mapping field IDs to error codes when there is an - // error or `null` for no error, so the values array will be something like: - // `[ null, "RegistrationForm.ERR_PASSWORD_MISSING", null]` - // Find the first non-null error code and show that. - const errCode = Object.values(fieldErrors).find(value => !!value); - if (!errCode) { - this.setState({ - errorText: null, - }); - return; - } - - let errMsg; - switch (errCode) { - case "RegistrationForm.ERR_PASSWORD_MISSING": - errMsg = _t('Missing password.'); - break; - case "RegistrationForm.ERR_PASSWORD_MISMATCH": - errMsg = _t('Passwords don\'t match.'); - break; - case "RegistrationForm.ERR_PASSWORD_LENGTH": - errMsg = _t('Password too short (min %(MIN_PASSWORD_LENGTH)s).', {MIN_PASSWORD_LENGTH}); - break; - default: - console.error("Unknown error code: %s", errCode); - errMsg = _t('An unknown error occurred.'); - break; - } - this.setState({ - errorText: errMsg, - }); - }, - onLoginClick: function(ev) { ev.preventDefault(); ev.stopPropagation(); @@ -517,7 +483,6 @@ module.exports = React.createClass({ defaultPhoneNumber={this.state.formVals.phoneNumber} defaultPassword={this.state.formVals.password} minPasswordLength={MIN_PASSWORD_LENGTH} - onValidationChange={this.onFormValidationChange} onRegisterClick={this.onFormSubmit} onEditServerDetailsClick={onEditServerDetailsClick} flows={this.state.flows} diff --git a/src/components/views/auth/RegistrationForm.js b/src/components/views/auth/RegistrationForm.js index a3816829e7..a7d7efee0c 100644 --- a/src/components/views/auth/RegistrationForm.js +++ b/src/components/views/auth/RegistrationForm.js @@ -47,7 +47,6 @@ module.exports = React.createClass({ defaultUsername: PropTypes.string, defaultPassword: PropTypes.string, minPasswordLength: PropTypes.number, - onValidationChange: PropTypes.func, onRegisterClick: PropTypes.func.isRequired, // onRegisterClick(Object) => ?Promise onEditServerDetailsClick: PropTypes.func, flows: PropTypes.arrayOf(PropTypes.object).isRequired, @@ -68,8 +67,6 @@ module.exports = React.createClass({ getInitialState: function() { return { // Field error codes by field ID - // TODO: Remove `fieldErrors` once converted to new-style validation - fieldErrors: {}, fieldValid: {}, // The ISO2 country code selected in the phone number entry phoneCountry: this.props.defaultPhoneCountry, @@ -84,15 +81,6 @@ module.exports = React.createClass({ onSubmit: function(ev) { ev.preventDefault(); - // validate everything, in reverse order so - // the error that ends up being displayed - // is the one from the first invalid field. - // It's not super ideal that this just calls - // onValidationChange once for each invalid field. - // TODO: Remove these calls once converted to new-style validation. - this.validateField(FIELD_PASSWORD_CONFIRM, ev.type); - this.validateField(FIELD_PASSWORD, ev.type); - const allFieldsValid = this.verifyFieldsBeforeSubmit(); if (!allFieldsValid) { return; @@ -178,14 +166,7 @@ module.exports = React.createClass({ * @returns {boolean} true if all fields were valid last time they were validated. */ allFieldsValid: function() { - // TODO: Remove `fieldErrors` here when all fields converted - let keys = Object.keys(this.state.fieldErrors); - for (let i = 0; i < keys.length; ++i) { - if (this.state.fieldErrors[keys[i]]) { - return false; - } - } - keys = Object.keys(this.state.fieldValid); + const keys = Object.keys(this.state.fieldValid); for (let i = 0; i < keys.length; ++i) { if (!this.state.fieldValid[keys[i]]) { return false; @@ -203,62 +184,6 @@ module.exports = React.createClass({ return null; }, - validateField: function(fieldID, eventType) { - const pwd1 = this.state.password.trim(); - const pwd2 = this.state.passwordConfirm.trim(); - const allowEmpty = eventType === "blur"; - - // TODO: Remove rules here as they are converted to new-style validation - - switch (fieldID) { - case FIELD_PASSWORD: - if (allowEmpty && pwd1 === "") { - this.markFieldError(fieldID, true); - } else if (pwd1 == '') { - this.markFieldError( - fieldID, - false, - "RegistrationForm.ERR_PASSWORD_MISSING", - ); - } else if (pwd1.length < this.props.minPasswordLength) { - this.markFieldError( - fieldID, - false, - "RegistrationForm.ERR_PASSWORD_LENGTH", - ); - } else { - this.markFieldError(fieldID, true); - } - break; - case FIELD_PASSWORD_CONFIRM: - if (allowEmpty && pwd2 === "") { - this.markFieldError(fieldID, true); - } else { - this.markFieldError( - fieldID, pwd1 == pwd2, - "RegistrationForm.ERR_PASSWORD_MISMATCH", - ); - } - break; - } - }, - - markFieldError: function(fieldID, valid, errorCode) { - // TODO: Remove this function once all fields converted to new-style validation. - const { fieldErrors } = this.state; - if (valid) { - fieldErrors[fieldID] = null; - } else { - fieldErrors[fieldID] = errorCode; - } - this.setState({ - fieldErrors, - }); - // TODO: Remove outer validation handling once all fields converted to new-style - // validation in the form. - this.props.onValidationChange(fieldErrors); - }, - markFieldValid: function(fieldID, valid) { const { fieldValid } = this.state; fieldValid[fieldID] = valid; @@ -267,16 +192,6 @@ module.exports = React.createClass({ }); }, - _classForField: function(fieldID, ...baseClasses) { - let cls = baseClasses.join(' '); - // TODO: Remove this from fields as they are converted to new-style validation. - if (this.state.fieldErrors[fieldID]) { - if (cls) cls += ' '; - cls += 'error'; - } - return cls; - }, - onEmailChange(ev) { this.setState({ email: ev.target.value, @@ -307,26 +222,66 @@ module.exports = React.createClass({ ], }), - onPasswordBlur(ev) { - this.validateField(FIELD_PASSWORD, ev.type); - }, - onPasswordChange(ev) { this.setState({ password: ev.target.value, }); }, - onPasswordConfirmBlur(ev) { - this.validateField(FIELD_PASSWORD_CONFIRM, ev.type); + onPasswordValidate(fieldState) { + const result = this.validatePasswordRules(fieldState); + this.markFieldValid(FIELD_PASSWORD, result.valid); + return result; }, + validatePasswordRules: withValidation({ + rules: [ + { + key: "required", + test: ({ value, allowEmpty }) => allowEmpty || !!value, + invalid: () => _t("Enter password"), + }, + { + key: "minLength", + test: function({ value }) { + return !value || value.length >= this.props.minPasswordLength; + }, + invalid: function() { + return _t("Too short (min %(length)s)", { length: this.props.minPasswordLength }); + }, + }, + ], + }), + onPasswordConfirmChange(ev) { this.setState({ passwordConfirm: ev.target.value, }); }, + onPasswordConfirmValidate(fieldState) { + const result = this.validatePasswordConfirmRules(fieldState); + this.markFieldValid(FIELD_PASSWORD_CONFIRM, result.valid); + return result; + }, + + validatePasswordConfirmRules: withValidation({ + rules: [ + { + key: "required", + test: ({ value, allowEmpty }) => allowEmpty || !!value, + invalid: () => _t("Confirm password"), + }, + { + key: "match", + test: function({ value }) { + return !value || value === this.state.password; + }, + invalid: () => _t("Passwords don't match"), + }, + ], + }), + onPhoneCountryChange(newVal) { this.setState({ phoneCountry: newVal.iso2, @@ -436,6 +391,34 @@ module.exports = React.createClass({ />; }, + renderPassword() { + const Field = sdk.getComponent('elements.Field'); + return this[FIELD_PASSWORD] = field} + type="password" + label={_t("Password")} + defaultValue={this.props.defaultPassword} + value={this.state.password} + onChange={this.onPasswordChange} + onValidate={this.onPasswordValidate} + />; + }, + + renderPasswordConfirm() { + const Field = sdk.getComponent('elements.Field'); + return this[FIELD_PASSWORD_CONFIRM] = field} + type="password" + label={_t("Confirm")} + defaultValue={this.props.defaultPassword} + value={this.state.passwordConfirm} + onChange={this.onPasswordConfirmChange} + onValidate={this.onPasswordConfirmValidate} + />; + }, + renderPhoneNumber() { const threePidLogin = !SdkConfig.get().disable_3pid_login; if (!threePidLogin || !this._authStepIsUsed('m.login.msisdn')) { @@ -481,8 +464,6 @@ module.exports = React.createClass({ }, render: function() { - const Field = sdk.getComponent('elements.Field'); - let yourMatrixAccountText = _t('Create your Matrix account'); if (this.props.hsName) { yourMatrixAccountText = _t('Create your Matrix account on %(serverName)s', { @@ -523,26 +504,8 @@ module.exports = React.createClass({ {this.renderUsername()}
- - + {this.renderPassword()} + {this.renderPasswordConfirm()}
{this.renderEmail()} diff --git a/src/components/views/elements/Validation.js b/src/components/views/elements/Validation.js index 458d000e0e..b567eb4058 100644 --- a/src/components/views/elements/Validation.js +++ b/src/components/views/elements/Validation.js @@ -52,7 +52,7 @@ export default function withValidation({ description, rules }) { } // We're setting `this` to whichever component hold the validation // function. That allows rules to access the state of the component. - // eslint-disable-next-line babel/no-invalid-this + /* eslint-disable babel/no-invalid-this */ const ruleValid = rule.test.call(this, { value, allowEmpty }); valid = valid && ruleValid; if (ruleValid && rule.valid) { @@ -61,7 +61,7 @@ export default function withValidation({ description, rules }) { results.push({ key: rule.key, valid: true, - text: rule.valid(), + text: rule.valid.call(this), }); } else if (!ruleValid && rule.invalid) { // If the rule's result is invalid and has text to show for @@ -69,9 +69,10 @@ export default function withValidation({ description, rules }) { results.push({ key: rule.key, valid: false, - text: rule.invalid(), + text: rule.invalid.call(this), }); } + /* eslint-enable babel/no-invalid-this */ } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 910fc8f74f..5fd97a4305 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1325,6 +1325,9 @@ "Use an email address to recover your account": "Use an email address to recover your account", "Enter email address (required on this homeserver)": "Enter email address (required on this homeserver)", "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)", + "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)", "Doesn't look like a valid phone number": "Doesn't look like a valid phone number", @@ -1332,10 +1335,10 @@ "Enter username": "Enter username", "Some characters not allowed": "Some characters not allowed", "Email (optional)": "Email (optional)", + "Confirm": "Confirm", "Phone (optional)": "Phone (optional)", "Create your Matrix account": "Create your Matrix account", "Create your Matrix account on %(serverName)s": "Create your Matrix account on %(serverName)s", - "Confirm": "Confirm", "Use an email address to recover your account.": "Use an email address to recover your account.", "Other users can invite you to rooms using your contact details.": "Other users can invite you to rooms using your contact details.", "Other servers": "Other servers", @@ -1525,9 +1528,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.", - "Missing password.": "Missing password.", - "Passwords don't match.": "Passwords don't match.", - "Password too short (min %(MIN_PASSWORD_LENGTH)s).": "Password too short (min %(MIN_PASSWORD_LENGTH)s).", "An unknown error occurred.": "An unknown error occurred.", "Create your account": "Create your account", "Commands": "Commands",