From be7ed2dfdc7cb2464d2217f382d8a085a5960d1e Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Sun, 18 Aug 2013 22:50:42 +0100 Subject: [PATCH] Added validation for signup and login screens Closes #374 * Included node-validator as a package * Implemented server side validation (the client side js is a mess, need a LOT of work) * Validates email address both on signup and login screens, gives error message on malformed email addresses * Requires at least 8 chars of password * Tells user if password is too short * Tells user if no such user on login * Tells user if wrong password on login * Tells user if server responds with a 404 (goes away, dies, etc) * Added middleware between req and login / signup for validation --- core/client/views/login.js | 11 +++++----- core/client/views/settings.js | 14 +++++++++---- core/server/controllers/admin.js | 30 ++++++++++++-------------- core/server/models/user.js | 4 ++-- index.js | 36 ++++++++++++++++++++++++++++++-- package.json | 3 ++- 6 files changed, 67 insertions(+), 31 deletions(-) diff --git a/core/client/views/login.js b/core/client/views/login.js index 372fa4316e..e46ddc1305 100644 --- a/core/client/views/login.js +++ b/core/client/views/login.js @@ -45,7 +45,8 @@ submitHandler: function (event) { event.preventDefault(); var email = this.$el.find('.email').val(), - password = this.$el.find('.password').val(); + password = this.$el.find('.password').val(), + self = this; $.ajax({ url: '/ghost/login/', @@ -60,7 +61,7 @@ error: function (obj, string, status) { Ghost.notifications.addItem({ type: 'error', - message: obj.responseText, + message: self.getRequestErrorMessage(obj), status: 'passive' }); } @@ -79,7 +80,8 @@ submitHandler: function (event) { event.preventDefault(); var email = this.$el.find('.email').val(), - password = this.$el.find('.password').val(); + password = this.$el.find('.password').val(), + self = this; $.ajax({ url: '/ghost/signup/', @@ -92,10 +94,9 @@ window.location.href = msg.redirect; }, error: function (obj, string, status) { - var msgobj = $.parseJSON(obj.responseText); Ghost.notifications.addItem({ type: 'error', - message: msgobj.message, + message: self.getRequestErrorMessage(obj), status: 'passive' }); } diff --git a/core/client/views/settings.js b/core/client/views/settings.js index ccfe99e7a5..e90c843f31 100644 --- a/core/client/views/settings.js +++ b/core/client/views/settings.js @@ -102,10 +102,11 @@ }); }, - saveError: function () { + saveError: function (message) { + message = message || 'Something went wrong, not saved :('; Ghost.notifications.addItem({ type: 'error', - message: 'Something went wrong, not saved :(', + message: message, status: 'passive' }); } @@ -208,8 +209,13 @@ newPassword = this.$('#user-password-new').val(), ne2Password = this.$('#user-new-password-verification').val(); - if (newPassword !== ne2Password || newPassword.length < 6 || oldPassword.length < 6) { - this.saveError(); + if (newPassword !== ne2Password) { + this.saveError('The passwords do not match'); + return; + } + + if (newPassword.length < 8) { + this.saveError('The password is not long enough. Have at least 8 characters'); return; } diff --git a/core/server/controllers/admin.js b/core/server/controllers/admin.js index 03642d452f..197b668299 100644 --- a/core/server/controllers/admin.js +++ b/core/server/controllers/admin.js @@ -97,7 +97,7 @@ adminControllers = { req.session.user = user.id; res.json(200, {redirect: req.query.r ? '/ghost/' + req.query.r : '/ghost/'}); }, function (error) { - res.send(401, error.message); + res.json(401, {error: error.message}); }); }, changepw: function (req, res) { @@ -124,22 +124,18 @@ adminControllers = { var email = req.body.email, password = req.body.password; - if (email !== '' && password.length > 5) { - api.users.add({ - email_address: email, - password: password - }).then(function (user) { - // Automatically log the new user in, unless created by an existing account - if (req.session.user === undefined) { - req.session.user = user.id; - } - res.json(200, {redirect: '/ghost/'}); - }, function (error) { - res.json(401, {message: error.message}); - }); - } else { - res.json(400, {message: 'The password is too short. Have at least 6 characters in there'}); - } + api.users.add({ + email_address: email, + password: password + }).then(function (user) { + if (req.session.user === undefined) { + req.session.user = user.id; + } + res.json(200, {redirect: '/ghost/'}); + }, function (error) { + res.json(401, {error: error.message}); + }); + }, 'logout': function (req, res) { delete req.session.user; diff --git a/core/server/models/user.js b/core/server/models/user.js index a23aafe78e..0e900c7f61 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -85,7 +85,7 @@ User = GhostBookshelf.Model.extend({ }).then(function (addedUserRole) { // Return the added user as expected return when.resolve(userData); - }, errors.logAndThrowError); + }); /** * Temporarily replacing the function below with another one that checks @@ -118,7 +118,7 @@ User = GhostBookshelf.Model.extend({ return user; }, errors.logAndThrowError); }, function (error) { - return when.reject(new Error('Email address or password is incorrect')); + return when.reject(new Error('There is no user with that email address.')); }); }, diff --git a/index.js b/index.js index 14758d9e95..9eaa3b08c0 100644 --- a/index.js +++ b/index.js @@ -20,11 +20,16 @@ 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 @@ -45,6 +50,33 @@ function auth(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(8)) { + res.json(401, {error: 'Your password is not long enough. It must be at least 8 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) { @@ -185,8 +217,8 @@ when.all([ghost.init(), filters.loadCoreFilters(ghost), helpers.loadCoreHelpers( ghost.app().get(/^\/logout\/?$/, admin.logout); ghost.app().get('/ghost/login/', admin.login); ghost.app().get('/ghost/signup/', admin.signup); - ghost.app().post('/ghost/login/', admin.auth); - ghost.app().post('/ghost/signup/', admin.doRegister); + ghost.app().post('/ghost/login/', signupValidate, admin.auth); + ghost.app().post('/ghost/signup/', signupValidate, 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); diff --git a/package.json b/package.json index 70cce57d82..b68a9d2cac 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,8 @@ "colors": "0.6.1", "semver": "2.1.0", "fs-extra": "0.6.3", - "downsize": "0.0.2" + "downsize": "0.0.2", + "validator": "1.4.0" }, "devDependencies": { "grunt": "~0.4.1",