Validation consistency

- introduced validation method in the post and user model
- moved signup validation onto model
- consistent use of validation & error messaging in the admin UI
- helper methods in base view moved to a utils object
This commit is contained in:
Hannah Wolfe 2013-08-20 19:52:44 +01:00
parent 052aa57360
commit 41e36cca7e
11 changed files with 108 additions and 109 deletions

View File

@ -73,7 +73,10 @@
this.removeSubviews();
}
return Backbone.View.prototype.remove.apply(this, arguments);
},
}
});
Ghost.Views.Utils = {
// Used in API request fail handlers to parse a standard api error
// response json for the message to display
@ -115,7 +118,7 @@
}
return vars;
}
});
};
/**
* This is the view to generate the markup for the individual

View File

@ -72,9 +72,9 @@
},
removeItem: function () {
var view = this;
var self = this;
$.when(this.$el.slideUp()).then(function () {
view.remove();
self.remove();
});
},

View File

@ -90,7 +90,7 @@
},
toggleStatus: function () {
var view = this,
var self = this,
keys = Object.keys(this.statusMap),
model = this.model,
prevStatus = this.model.get('status'),
@ -112,10 +112,10 @@
message: 'Your post: ' + model.get('title') + ' has been ' + keys[newIndex],
status: 'passive'
});
}, function (response) {
}, function (xhr) {
var status = keys[newIndex];
// Show a notification about the error
view.reportSaveError(response, model, status);
self.reportSaveError(xhr, model, status);
// Set the button text back to previous
model.set({ status: prevStatus });
});
@ -136,10 +136,9 @@
handleStatus: function (e) {
if (e) { e.preventDefault(); }
var view = this,
status = $(e.currentTarget).attr('data-set-status');
var status = $(e.currentTarget).attr('data-set-status');
view.setActiveStatus(status, this.statusMap[status]);
this.setActiveStatus(status, this.statusMap[status]);
// Dismiss the popup menu
$('body').find('.overlay:visible').fadeOut();
@ -147,8 +146,8 @@
updatePost: function (e) {
if (e) { e.preventDefault(); }
var view = this,
model = view.model,
var self = this,
model = this.model,
$currentTarget = $(e.currentTarget),
status = $currentTarget.attr('data-status'),
prevStatus = model.get('status');
@ -168,7 +167,7 @@
});
}
view.savePost({
this.savePost({
status: status
}).then(function () {
Ghost.notifications.addItem({
@ -176,15 +175,10 @@
message: ['Your post "', model.get('title'), '" has been ', status, '.'].join(''),
status: 'passive'
});
}, function (request) {
var message = view.getRequestErrorMessage(request) || model.validationError;
Ghost.notifications.addItem({
type: 'error',
message: message,
status: 'passive'
});
}, function (xhr) {
// Show a notification about the error
self.reportSaveError(xhr, model, status);
// Set the button text back to previous
model.set({ status: prevStatus });
});
},
@ -214,7 +208,7 @@
if (response) {
// Get message from response
message = this.getErrorMessageFromResponse(response);
message = Ghost.Views.Utils.getRequestErrorMessage(response);
} else if (model.validationError) {
// Grab a validation error
message += "; " + model.validationError;
@ -228,9 +222,7 @@
},
render: function () {
var status = this.model.get('status');
this.setActiveStatus(status, this.statusMap[status]);
this.$('.js-post-button').text(this.statusMap[this.model.get('status')]);
}
});
@ -337,19 +329,21 @@
// Currently gets called on every key press.
// Also trigger word count update
renderPreview: function () {
var view = this,
var self = this,
preview = document.getElementsByClassName('rendered-markdown')[0];
preview.innerHTML = this.converter.makeHtml(this.editor.getValue());
view.$('.js-drop-zone').upload({editor: true});
this.$('.js-drop-zone').upload({editor: true});
Countable.once(preview, function (counter) {
view.$('.entry-word-count').text($.pluralize(counter.words, 'word'));
view.$('.entry-character-count').text($.pluralize(counter.characters, 'character'));
view.$('.entry-paragraph-count').text($.pluralize(counter.paragraphs, 'paragraph'));
self.$('.entry-word-count').text($.pluralize(counter.words, 'word'));
self.$('.entry-character-count').text($.pluralize(counter.characters, 'character'));
self.$('.entry-paragraph-count').text($.pluralize(counter.paragraphs, 'paragraph'));
});
},
// Markdown converter & markdown shortcut initialization.
initMarkdown: function () {
var self = this;
this.converter = new Showdown.converter({extensions: ['ghostdown']});
this.editor = CodeMirror.fromTextArea(document.getElementById('entry-markdown'), {
mode: 'markdown',
@ -359,24 +353,22 @@
dragDrop: false
});
var view = this;
// Inject modal for HTML to be viewed in
shortcut.add("Ctrl+Alt+C", function () {
view.showHTML();
self.showHTML();
});
shortcut.add("Ctrl+Alt+C", function () {
view.showHTML();
self.showHTML();
});
_.each(MarkdownShortcuts, function (combo) {
shortcut.add(combo.key, function () {
return view.editor.addMarkdown({style: combo.style});
return self.editor.addMarkdown({style: combo.style});
});
});
this.editor.on('change', function () {
view.renderPreview();
self.renderPreview();
});
},

View File

@ -52,8 +52,7 @@
event.preventDefault();
var email = this.$el.find('.email').val(),
password = this.$el.find('.password').val(),
redirect = this.getUrlVariables().r,
self = this;
redirect = Ghost.Views.Utils.getUrlVariables().r;
$.ajax({
url: '/ghost/signin/',
@ -66,10 +65,10 @@
success: function (msg) {
window.location.href = msg.redirect;
},
error: function (obj, string, status) {
error: function (xhr) {
Ghost.notifications.addItem({
type: 'error',
message: self.getRequestErrorMessage(obj),
message: Ghost.Views.Utils.getRequestErrorMessage(xhr),
status: 'passive'
});
}
@ -88,8 +87,7 @@
submitHandler: function (event) {
event.preventDefault();
var email = this.$el.find('.email').val(),
password = this.$el.find('.password').val(),
self = this;
password = this.$el.find('.password').val();
$.ajax({
url: '/ghost/signup/',
@ -101,10 +99,10 @@
success: function (msg) {
window.location.href = msg.redirect;
},
error: function (obj, string, status) {
error: function (xhr) {
Ghost.notifications.addItem({
type: 'error',
message: self.getRequestErrorMessage(obj),
message: Ghost.Views.Utils.getRequestErrorMessage(xhr),
status: 'passive'
});
}

View File

@ -95,15 +95,22 @@
checkboxClass: 'icheckbox_ghost'
});
},
saveSuccess: function () {
saveSuccess: function (model, response, options) {
// TODO: better messaging here?
Ghost.notifications.addItem({
type: 'success',
message: 'Saved',
status: 'passive'
});
},
saveError: function (message) {
message = message || 'Something went wrong, not saved :(';
saveError: function (model, xhr) {
Ghost.notifications.addItem({
type: 'error',
message: Ghost.Views.Utils.getRequestErrorMessage(xhr),
status: 'passive'
});
},
validationError: function (message) {
Ghost.notifications.addItem({
type: 'error',
message: message,
@ -210,12 +217,12 @@
ne2Password = this.$('#user-new-password-verification').val();
if (newPassword !== ne2Password) {
this.saveError('The passwords do not match');
this.validationError('Your new passwords do not match');
return;
}
if (newPassword.length < 7) {
this.saveError('The password is not long enough. Have at least 7 characters');
if (newPassword.length < 8) {
this.validationError('Your password is not long enough. It must be at least 8 chars long.');
return;
}
@ -234,14 +241,12 @@
status: 'passive',
id: 'success-98'
});
self.$('#user-password-old').val('');
self.$('#user-password-new').val('');
self.$('#user-new-password-verification').val('');
self.$('#user-password-old, #user-password-new, #user-new-password-verification').val('');
},
error: function (obj, string, status) {
error: function (xhr) {
Ghost.notifications.addItem({
type: 'error',
message: 'Invalid username or password',
message: Ghost.Views.Utils.getRequestErrorMessage(xhr),
status: 'passive'
});
}

View File

@ -262,7 +262,8 @@ requestHandler = function (apiMethod) {
return apiMethod.call(apiContext, options).then(function (result) {
res.json(result || {});
}, function (error) {
res.json(400, {error: error});
error = {error: _.isString(error) ? error : (_.isObject(error) ? error.message : 'Unknown API Error')};
res.json(400, error);
});
};
};

View File

@ -124,10 +124,10 @@ adminControllers = {
oldpw: req.body.password,
newpw: req.body.newpassword,
ne2pw: req.body.ne2password
}).then(function (user) {
}).then(function () {
res.json(200, {msg: 'Password changed successfully'});
}, function (error) {
res.send(401);
res.send(401, {error: error.message});
});
},

View File

@ -1,12 +1,15 @@
var GhostBookshelf,
Bookshelf = require('bookshelf'),
_ = require('underscore'),
config = require('../../../config');
config = require('../../../config'),
Validator = require('validator').Validator;
// Initializes Bookshelf as its own instance, so we can modify the Models and not mess up
// others' if they're using the library outside of ghost.
GhostBookshelf = Bookshelf.Initialize('ghost', config.env[process.env.NODE_ENV || 'development'].database);
GhostBookshelf.validator = new Validator();
// The Base Model which other Ghost objects will inherit from,
// including some convenience functions as static properties on the model.
GhostBookshelf.Model = GhostBookshelf.Model.extend({
@ -89,7 +92,7 @@ GhostBookshelf.Model = GhostBookshelf.Model.extend({
/**
* Naive create
* @param editedObj
* @param newObj
* @param options (optional)
*/
add: function (newObj, options) {

View File

@ -24,14 +24,18 @@ Post = GhostBookshelf.Model.extend({
},
initialize: function () {
this.on('saving', this.validate, this);
this.on('creating', this.creating, this);
this.on('saving', this.saving, this);
},
validate: function () {
GhostBookshelf.validator.check(this.get('title'), "Post title cannot be blank").notEmpty();
return true;
},
saving: function () {
if (!this.get('title')) {
throw new Error('Post title cannot be blank');
}
this.set('content', converter.makeHtml(this.get('content_raw')));
if (this.hasChanged('status') && this.get('status') === 'published') {
@ -45,6 +49,7 @@ Post = GhostBookshelf.Model.extend({
},
creating: function () {
// set any dynamic default properties
var self = this;
if (!this.get('created_by')) {
this.set('created_by', 1);

View File

@ -13,13 +13,21 @@ var User,
Role = require('./role').Role,
Permission = require('./permission').Permission;
UserRole = GhostBookshelf.Model.extend({
tableName: 'roles_users'
});
function validatePasswordLength(password) {
try {
GhostBookshelf.validator.check(password, "Your password is not long enough. It must be at least 8 chars long.").len(8);
} catch (error) {
return when.reject(error);
}
return when.resolve();
}
User = GhostBookshelf.Model.extend({
tableName: 'users',
@ -32,6 +40,19 @@ User = GhostBookshelf.Model.extend({
};
},
initialize: function () {
this.on('saving', this.validate, this);
},
validate: function () {
GhostBookshelf.validator.check(this.get('email_address'), "Please check your email address. It does not seem to be valid.").isEmail();
GhostBookshelf.validator.check(this.get('bio'), "Your bio is too long. Please keep it to 200 chars.").len(0, 200);
if (this.get('url') && this.get('url').length > 0) {
GhostBookshelf.validator.check(this.get('url'), "Your website is not a valid URL.").isUrl();
}
return true;
},
posts: function () {
return this.hasMany(Posts, 'created_by');
},
@ -63,12 +84,14 @@ User = GhostBookshelf.Model.extend({
* @param {object} user
* @author javorszky
*/
return this.forge().fetch().then(function (user) {
return validatePasswordLength(userData.password).then(function () {
return self.forge().fetch();
}).then(function (user) {
// Check if user exists
if (user) {
return when.reject(new Error('A user is already registered. Only one user for now!'));
}
}).then(function () {
// Hash the provided password with bcrypt
return nodefn.call(bcrypt.hash, _user.password, null, null);
}).then(function (hash) {
@ -113,7 +136,7 @@ User = GhostBookshelf.Model.extend({
}).fetch({require: true}).then(function (user) {
return nodefn.call(bcrypt.compare, _userdata.pw, user.get('password')).then(function (matched) {
if (!matched) {
return when.reject(new Error('Passwords do not match'));
return when.reject(new Error('Your password is incorrect'));
}
return user;
}, errors.logAndThrowError);
@ -128,22 +151,23 @@ User = GhostBookshelf.Model.extend({
*
*/
changePassword: function (_userdata) {
var userid = _userdata.currentUser,
var self = this,
userid = _userdata.currentUser,
oldPassword = _userdata.oldpw,
newPassword = _userdata.newpw,
ne2Password = _userdata.ne2pw;
if (newPassword !== ne2Password) {
return when.reject(new Error('Passwords aren\'t the same'));
return when.reject(new Error('Your new passwords do not match'));
}
return this.forge({
id: userid
}).fetch({require: true}).then(function (user) {
return validatePasswordLength(newPassword).then(function () {
return self.forge({id: userid}).fetch({require: true});
}).then(function (user) {
return nodefn.call(bcrypt.compare, oldPassword, user.get('password'))
.then(function (matched) {
if (!matched) {
return when.reject(new Error('Passwords do not match'));
return when.reject(new Error('Your password is incorrect'));
}
return nodefn.call(bcrypt.hash, newPassword, null, null).then(function (hash) {
user.save({password: hash});
@ -151,6 +175,7 @@ User = GhostBookshelf.Model.extend({
});
});
});
},
effectivePermissions: function (id) {

View File

@ -19,17 +19,11 @@ var express = require('express'),
filters = require('./core/server/filters'),
helpers = require('./core/server/helpers'),
packageInfo = require('./package.json'),
Validator = require('validator').Validator,
v = new Validator(),
// Variables
loading = when.defer(),
ghost = new Ghost();
v.error = function () {
return false;
};
// ##Custom Middleware
// ### Auth Middleware
@ -82,33 +76,6 @@ function cleanNotifications(req, res, next) {
next();
}
/**
* Validation middleware
* Checks on signup whether email is actually a valid email address
* and if password is at least 8 characters long
*
* To change validation rules, see https://github.com/chriso/node-validator
*
* @author javorszky
* @issue https://github.com/TryGhost/Ghost/issues/374
*/
function signupValidate(req, res, next) {
var email = req.body.email,
password = req.body.password;
if (!v.check(email).isEmail()) {
res.json(401, {error: "Please check your email address. It does not seem to be valid."});
return;
}
if (!v.check(password).len(7)) {
res.json(401, {error: 'Your password is not long enough. It must be at least 7 chars long.'});
return;
}
next();
}
// ## AuthApi Middleware
// Authenticate a request to the API by responding with a 401 and json error details
function authAPI(req, res, next) {
@ -254,7 +221,7 @@ when.all([ghost.init(), filters.loadCoreFilters(ghost), helpers.loadCoreHelpers(
ghost.app().get('/ghost/signin/', redirectToDashboard, admin.login);
ghost.app().get('/ghost/signup/', redirectToDashboard, admin.signup);
ghost.app().post('/ghost/signin/', admin.auth);
ghost.app().post('/ghost/signup/', signupValidate, admin.doRegister);
ghost.app().post('/ghost/signup/', admin.doRegister);
ghost.app().post('/ghost/changepw/', auth, admin.changepw);
ghost.app().get('/ghost/editor/:id', auth, admin.editor);
ghost.app().get('/ghost/editor', auth, admin.editor);