From b20b6cc5caf116c980100adf25b7e00a2fcaf47f Mon Sep 17 00:00:00 2001 From: Bryan Ashby Date: Sat, 4 Feb 2023 13:44:55 -0700 Subject: [PATCH] Updates on form validation * Better errors, using enig Errors and ErrorReasons * If subject isn't required, don't enforce it * Allow validator listeners to override error (ie: ignore) --- core/bbs_list.js | 8 +-- core/enig_error.js | 9 +++ core/file_area_filter_edit.js | 5 +- core/fse.js | 31 ++++++--- core/nua.js | 6 +- core/system_view_validate.js | 122 ++++++++++++++++++++++++++-------- core/upload.js | 4 +- core/user_config.js | 5 +- core/view_controller.js | 13 ++-- 9 files changed, 147 insertions(+), 56 deletions(-) diff --git a/core/bbs_list.js b/core/bbs_list.js index f87349fd..b4e45994 100644 --- a/core/bbs_list.js +++ b/core/bbs_list.js @@ -80,13 +80,13 @@ exports.getModule = class BBSListModule extends MenuModule { const errMsgView = self.viewControllers.add.getView(MciViewIds.add.Error); if (errMsgView) { if (err) { - errMsgView.setText(err.message); + errMsgView.setText(err.friendlyText); } else { errMsgView.clearText(); } } - return cb(null); + return cb(err, null); }, // @@ -119,7 +119,7 @@ exports.getModule = class BBSListModule extends MenuModule { } self.database.run( - `DELETE FROM bbs_list + `DELETE FROM bbs_list WHERE id=?;`, [entry.id], err => { @@ -162,7 +162,7 @@ exports.getModule = class BBSListModule extends MenuModule { } self.database.run( - `INSERT INTO bbs_list (bbs_name, sysop, telnet, www, location, software, submitter_user_id, notes) + `INSERT INTO bbs_list (bbs_name, sysop, telnet, www, location, software, submitter_user_id, notes) VALUES(?, ?, ?, ?, ?, ?, ?, ?);`, [ formData.value.name, diff --git a/core/enig_error.js b/core/enig_error.js index c5e9f065..a84c6a30 100644 --- a/core/enig_error.js +++ b/core/enig_error.js @@ -61,6 +61,8 @@ exports.Errors = { new EnigError('Bad or missing form data', -32016, reason, reasonCode), Duplicate: (reason, reasonCode) => new EnigError('Duplicate', -32017, reason, reasonCode), + ValidationFailed: (reason, reasonCode) => + new EnigError('Validation failed', -32018, reason, reasonCode), }; exports.ErrorReasons = { @@ -76,4 +78,11 @@ exports.ErrorReasons = { Locked: 'LOCKED', NotAllowed: 'NOTALLOWED', Invalid2FA: 'INVALID2FA', + + ValueTooShort: 'VALUE_TOO_SHORT', + ValueTooLong: 'VALUE_TOO_LONG', + ValueInvalid: 'VALUE_INVALID', + + NotAvailable: 'NOT_AVAILABLE', + DoesNotExist: 'EEXIST', }; diff --git a/core/file_area_filter_edit.js b/core/file_area_filter_edit.js index 28a8b38d..9fc1fb01 100644 --- a/core/file_area_filter_edit.js +++ b/core/file_area_filter_edit.js @@ -146,18 +146,17 @@ exports.getModule = class FileAreaFilterEdit extends MenuModule { const errorView = this.viewControllers.editor.getView( MciViewIds.editor.error ); - let newFocusId; if (errorView) { if (err) { - errorView.setText(err.message); + errorView.setText(err.friendlyText); err.view.clearText(); // clear out the invalid data } else { errorView.clearText(); } } - return cb(newFocusId); + return cb(err, null); }, }; } diff --git a/core/fse.js b/core/fse.js index f7d39d07..9a9c1f18 100644 --- a/core/fse.js +++ b/core/fse.js @@ -38,6 +38,7 @@ const fse = require('fs-extra'); const fs = require('graceful-fs'); const paths = require('path'); const sanatizeFilename = require('sanitize-filename'); +const { ErrorReasons } = require('./enig_error.js'); exports.moduleInfo = { name: 'Full Screen Editor (FSE)', @@ -165,23 +166,35 @@ exports.FullScreenEditorModule = // // Validation stuff // - viewValidationListener: function (err, cb) { - var errMsgView = self.viewControllers.header.getView( + viewValidationListener: (err, cb) => { + if ( + err && + err.view.getId() === MciViewIds.header.subject && + err.reasonCode === ErrorReasons.ValueTooShort + ) { + // Ignore validation errors if this is the subject field + // and it's optional + const toView = this.getView('header', MciViewIds.header.to); + const msgInfo = messageInfoFromAddressedToInfo( + getAddressedToInfo(toView.getData()) + ); + if (true === msgInfo.subjectOptional) { + return cb(null, null); + } + } + + const errMsgView = this.viewControllers.header.getView( MciViewIds.header.errorMsg ); - var newFocusViewId; if (errMsgView) { if (err) { - errMsgView.setText(err.message); - - if (MciViewIds.header.subject === err.view.getId()) { - // :TODO: for "area" mode, should probably just bail if this is emtpy (e.g. cancel) - } + errMsgView.setText(err.friendlyText); } else { errMsgView.clearText(); } } - cb(newFocusViewId); + + return cb(err, null); }, headerSubmit: function (formData, extraArgs, cb) { self.switchToBody(); diff --git a/core/nua.js b/core/nua.js index 4f6f355d..198264f8 100644 --- a/core/nua.js +++ b/core/nua.js @@ -49,10 +49,10 @@ exports.getModule = class NewUserAppModule extends MenuModule { viewValidationListener: function (err, cb) { const errMsgView = self.viewControllers.menu.getView(MciViewIds.errMsg); - let newFocusId; + let newFocusId; if (err) { - errMsgView.setText(err.message); + errMsgView.setText(err.friendlyText); err.view.clearText(); if (err.view.getId() === MciViewIds.confirm) { @@ -65,7 +65,7 @@ exports.getModule = class NewUserAppModule extends MenuModule { errMsgView.clearText(); } - return cb(newFocusId); + return cb(err, newFocusId); }, // diff --git a/core/system_view_validate.js b/core/system_view_validate.js index 763e0c47..13d1568f 100644 --- a/core/system_view_validate.js +++ b/core/system_view_validate.js @@ -2,11 +2,12 @@ 'use strict'; // ENiGMA½ -const User = require('./user.js'); -const Config = require('./config.js').get; -const Log = require('./logger.js').log; -const { getAddressedToInfo } = require('./mail_util.js'); -const Message = require('./message.js'); +const User = require('./user'); +const Config = require('./config').get; +const Log = require('./logger').log; +const { getAddressedToInfo } = require('./mail_util'); +const Message = require('./message'); +const { Errors, ErrorReasons } = require('./enig_error'); // note: Only use ValidationFailed in this module! // deps const fs = require('graceful-fs'); @@ -22,36 +23,66 @@ exports.validateBirthdate = validateBirthdate; exports.validatePasswordSpec = validatePasswordSpec; function validateNonEmpty(data, cb) { - return cb(data && data.length > 0 ? null : new Error('Field cannot be empty')); + return cb( + data && data.length > 0 + ? null + : Errors.ValidationFailed('Field cannot be empty', ErrorReasons.ValueTooShort) + ); } function validateMessageSubject(data, cb) { - return cb(data && data.length > 1 ? null : new Error('Subject too short')); + return cb( + data && data.length > 1 + ? null + : Errors.ValidationFailed('Subject too short', ErrorReasons.ValueTooShort) + ); } function validateUserNameAvail(data, cb) { const config = Config(); if (!data || data.length < config.users.usernameMin) { - cb(new Error('Username too short')); + cb(Errors.ValidationFailed('Username too short', ErrorReasons.ValueTooShort)); } else if (data.length > config.users.usernameMax) { // generally should be unreached due to view restraints - return cb(new Error('Username too long')); + return cb( + Errors.ValidationFailed('Username too long', ErrorReasons.ValueTooLong) + ); } else { const usernameRegExp = new RegExp(config.users.usernamePattern); const invalidNames = config.users.newUserNames + config.users.badUserNames; if (!usernameRegExp.test(data)) { - return cb(new Error('Username contains invalid characters')); + return cb( + Errors.ValidationFailed( + 'Username contains invalid characters', + ErrorReasons.ValueInvalid + ) + ); } else if (invalidNames.indexOf(data.toLowerCase()) > -1) { - return cb(new Error('Username is blacklisted')); + return cb( + Errors.ValidationFailed( + 'Username is blacklisted', + ErrorReasons.NotAllowed + ) + ); } else if (/^[0-9]+$/.test(data)) { - return cb(new Error('Username cannot be a number')); + return cb( + Errors.ValidationFailed( + 'Username cannot be a number', + ErrorReasons.ValueInvalid + ) + ); } else { // a new user name cannot be an existing user name or an existing real name User.getUserIdAndNameByLookup(data, function userIdAndName(err) { if (!err) { // err is null if we succeeded -- meaning this user exists already - return cb(new Error('Username unavailable')); + return cb( + Errors.ValidationFailed( + 'Username unavailable', + ErrorReasons.NotAvailable + ) + ); } return cb(null); @@ -60,25 +91,41 @@ function validateUserNameAvail(data, cb) { } } -const invalidUserNameError = () => new Error('Invalid username'); - function validateUserNameExists(data, cb) { if (0 === data.length) { - return cb(invalidUserNameError()); + return cb( + Errors.ValidationFailed('Invalid username', ErrorReasons.ValueTooShort) + ); } User.getUserIdAndName(data, err => { - return cb(err ? invalidUserNameError() : null); + return cb( + err + ? Errors.ValidationFailed( + 'Failed to find username', + err.reasonCode || ErrorReasons.DoesNotExist + ) + : null + ); }); } function validateUserNameOrRealNameExists(data, cb) { if (0 === data.length) { - return cb(invalidUserNameError()); + return cb( + Errors.ValidationFailed('Invalid username', ErrorReasons.ValueTooShort) + ); } User.getUserIdAndNameByLookup(data, err => { - return cb(err ? invalidUserNameError() : null); + return cb( + err + ? Errors.ValidationFailed( + 'Failed to find user', + err.reasonCode || ErrorReasons.DoesNotExist + ) + : null + ); }); } @@ -112,7 +159,9 @@ function validateEmailAvail(data, cb) { // const emailRegExp = /[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(.[a-z0-9-]+)*/; if (!emailRegExp.test(data)) { - return cb(new Error('Invalid email address')); + return cb( + Errors.ValidationFailed('Invalid email address', ErrorReasons.ValueInvalid) + ); } User.getUserIdsWithProperty( @@ -120,9 +169,19 @@ function validateEmailAvail(data, cb) { data, function userIdsWithEmail(err, uids) { if (err) { - return cb(new Error('Internal system error')); + return cb( + Errors.ValidationFailed( + err.message, + err.reasonCode || ErrorReasons.DoesNotExist + ) + ); } else if (uids.length > 0) { - return cb(new Error('Email address not unique')); + return cb( + Errors.ValidationFailed( + 'Email address not unique', + ErrorReasons.NotAvailable + ) + ); } return cb(null); @@ -132,25 +191,36 @@ function validateEmailAvail(data, cb) { function validateBirthdate(data, cb) { // :TODO: check for dates in the future, or > reasonable values - return cb(isNaN(Date.parse(data)) ? new Error('Invalid birthdate') : null); + return cb( + isNaN(Date.parse(data)) + ? Errors.ValidationFailed('Invalid birthdate', ErrorReasons.ValueInvalid) + : null + ); } function validatePasswordSpec(data, cb) { const config = Config(); if (!data || data.length < config.users.passwordMin) { - return cb(new Error('Password too short')); + return cb( + Errors.ValidationFailed('Password too short', ErrorReasons.ValueTooShort) + ); } // check badpass, if avail fs.readFile(config.users.badPassFile, 'utf8', (err, passwords) => { if (err) { - Log.warn({ error: err.message }, 'Cannot read bad pass file'); + Log.warn( + { error: err.message, path: config.users.badPassFile }, + 'Cannot read bad pass file' + ); return cb(null); } passwords = passwords.toString().split(/\r\n|\n/g); if (passwords.includes(data)) { - return cb(new Error('Password is too common')); + return cb( + Errors.ValidationFailed('Password is too common', ErrorReasons.NotAllowed) + ); } return cb(null); diff --git a/core/upload.js b/core/upload.js index ec73a233..d27e9d2e 100644 --- a/core/upload.js +++ b/core/upload.js @@ -120,13 +120,13 @@ exports.getModule = class UploadModule extends MenuModule { ); if (errView) { if (err) { - errView.setText(err.message); + errView.setText(err.friendlyText); } else { errView.clearText(); } } - return cb(null); + return cb(err, null); }, }; } diff --git a/core/user_config.js b/core/user_config.js index 859c63e8..e5bbff6f 100644 --- a/core/user_config.js +++ b/core/user_config.js @@ -90,7 +90,7 @@ exports.getModule = class UserConfigModule extends MenuModule { var newFocusId; if (errMsgView) { if (err) { - errMsgView.setText(err.message); + errMsgView.setText(err.friendlyText); if (err.view.getId() === MciCodeIds.PassConfirm) { newFocusId = MciCodeIds.Password; @@ -102,7 +102,8 @@ exports.getModule = class UserConfigModule extends MenuModule { errMsgView.clearText(); } } - cb(newFocusId); + + return cb(err, newFocusId); }, // diff --git a/core/view_controller.js b/core/view_controller.js index 6a0597e7..420d4b2b 100644 --- a/core/view_controller.js +++ b/core/view_controller.js @@ -385,19 +385,18 @@ function ViewController(options) { this.validateView = function (view, cb) { if (view && _.isFunction(view.validate)) { view.validate(view.getData(), function validateResult(err) { - var viewValidationListener = + const viewValidationListener = self.client.currentMenuModule.menuMethods.viewValidationListener; if (_.isFunction(viewValidationListener)) { if (err) { err.view = view; // pass along the view that failed + err.friendlyText = err.reason || err.message; } - viewValidationListener( - err, - function validationComplete(newViewFocusId) { - cb(err, newViewFocusId); - } - ); + viewValidationListener(err, (err, newFocusedViewId) => { + // validator may have updated |err| + return cb(err, newFocusedViewId); + }); } else { cb(err); }