From 41e36cca7e2c1565280779e77d7eb0e094ec89f7 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 20 Aug 2013 19:52:44 +0100 Subject: [PATCH 1/2] 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 --- core/client/views/base.js | 7 ++-- core/client/views/blog.js | 4 +-- core/client/views/editor.js | 58 ++++++++++++++------------------ core/client/views/login.js | 14 ++++---- core/client/views/settings.js | 27 +++++++++------ core/server/api.js | 3 +- core/server/controllers/admin.js | 4 +-- core/server/models/base.js | 7 ++-- core/server/models/post.js | 11 ++++-- core/server/models/user.js | 47 ++++++++++++++++++++------ index.js | 35 +------------------ 11 files changed, 108 insertions(+), 109 deletions(-) diff --git a/core/client/views/base.js b/core/client/views/base.js index 6b447d1bd6..542900100b 100644 --- a/core/client/views/base.js +++ b/core/client/views/base.js @@ -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 diff --git a/core/client/views/blog.js b/core/client/views/blog.js index 276a3473f9..f8204cc3b2 100644 --- a/core/client/views/blog.js +++ b/core/client/views/blog.js @@ -72,9 +72,9 @@ }, removeItem: function () { - var view = this; + var self = this; $.when(this.$el.slideUp()).then(function () { - view.remove(); + self.remove(); }); }, diff --git a/core/client/views/editor.js b/core/client/views/editor.js index cf9c4c4dfe..e98d6b2766 100644 --- a/core/client/views/editor.js +++ b/core/client/views/editor.js @@ -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(); }); }, diff --git a/core/client/views/login.js b/core/client/views/login.js index 71d0eef903..e6b1a584e7 100644 --- a/core/client/views/login.js +++ b/core/client/views/login.js @@ -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' }); } diff --git a/core/client/views/settings.js b/core/client/views/settings.js index fdc426613c..ce8ef09469 100644 --- a/core/client/views/settings.js +++ b/core/client/views/settings.js @@ -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' }); } diff --git a/core/server/api.js b/core/server/api.js index 1ec46fcccb..6c9e38a658 100644 --- a/core/server/api.js +++ b/core/server/api.js @@ -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); }); }; }; diff --git a/core/server/controllers/admin.js b/core/server/controllers/admin.js index 39d1a75ae8..a83e3d52a7 100644 --- a/core/server/controllers/admin.js +++ b/core/server/controllers/admin.js @@ -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}); }); }, diff --git a/core/server/models/base.js b/core/server/models/base.js index cc7ba2b343..02a8af6d76 100644 --- a/core/server/models/base.js +++ b/core/server/models/base.js @@ -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) { diff --git a/core/server/models/post.js b/core/server/models/post.js index ad06688aa0..3485fe04fe 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -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); diff --git a/core/server/models/user.js b/core/server/models/user.js index 0e900c7f61..92f5a06359 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -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) { diff --git a/index.js b/index.js index 78d1ed67c6..1e64ef9b50 100644 --- a/index.js +++ b/index.js @@ -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); From c70dfde7e372d7bb4368b7d72b5fbe7408b2141c Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sun, 25 Aug 2013 11:49:31 +0100 Subject: [PATCH 2/2] Agressive stripping of the model attributes - fixes #517 - prevents this from occuring again in future with other relations - validation function & stripping done for all models - casper test for flow, plus validation & logged out tests --- core/server/models/permission.js | 19 ++++ core/server/models/post.js | 13 ++- core/server/models/role.js | 19 ++++ core/server/models/settings.js | 23 +++++ core/server/models/user.js | 13 +++ core/test/functional/admin/01_login_test.js | 3 +- core/test/functional/admin/03_editor_test.js | 14 +-- .../test/functional/admin/05_settings_test.js | 40 ++++++++ core/test/functional/admin/06_flow_test.js | 60 ++++++++++++ core/test/functional/admin/07_logout_test.js | 98 +++++++++++++++++++ core/test/unit/api_users_spec.js | 4 +- 11 files changed, 296 insertions(+), 10 deletions(-) create mode 100644 core/test/functional/admin/06_flow_test.js create mode 100644 core/test/functional/admin/07_logout_test.js diff --git a/core/server/models/permission.js b/core/server/models/permission.js index b38d9f5860..befe29125f 100644 --- a/core/server/models/permission.js +++ b/core/server/models/permission.js @@ -7,6 +7,25 @@ var GhostBookshelf = require('./base'), Permission = GhostBookshelf.Model.extend({ tableName: 'permissions', + permittedAttributes: ['id', 'name', 'object_type', 'action_type', 'object_id'], + + initialize: function () { + this.on('saving', this.saving, this); + this.on('saving', this.validate, this); + }, + + validate: function () { + // TODO: validate object_type, action_type and object_id + GhostBookshelf.validator.check(this.get('name'), "Permission name cannot be blank").notEmpty(); + }, + + saving: function () { + // Deal with the related data here + + // Remove any properties which don't belong on the post model + this.attributes = this.pick(this.permittedAttributes); + }, + roles: function () { return this.belongsToMany(Role); }, diff --git a/core/server/models/post.js b/core/server/models/post.js index 3485fe04fe..21307162a2 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -13,6 +13,12 @@ Post = GhostBookshelf.Model.extend({ tableName: 'posts', + permittedAttributes: [ + 'id', 'uuid', 'title', 'slug', 'content_raw', 'content', 'meta_title', 'meta_description', 'meta_keywords', + 'featured', 'image', 'status', 'language', 'author_id', 'created_at', 'created_by', 'updated_at', 'updated_by', + 'published_at', 'published_by' + ], + hasTimestamps: true, defaults: function () { @@ -24,9 +30,9 @@ Post = GhostBookshelf.Model.extend({ }, initialize: function () { - this.on('saving', this.validate, this); this.on('creating', this.creating, this); this.on('saving', this.saving, this); + this.on('saving', this.validate, this); }, validate: function () { @@ -36,6 +42,11 @@ Post = GhostBookshelf.Model.extend({ }, saving: function () { + // Deal with the related data here + + // Remove any properties which don't belong on the post model + this.attributes = this.pick(this.permittedAttributes); + this.set('content', converter.makeHtml(this.get('content_raw'))); if (this.hasChanged('status') && this.get('status') === 'published') { diff --git a/core/server/models/role.js b/core/server/models/role.js index 719f250985..76fa86199c 100644 --- a/core/server/models/role.js +++ b/core/server/models/role.js @@ -7,6 +7,25 @@ var User = require('./user').User, Role = GhostBookshelf.Model.extend({ tableName: 'roles', + permittedAttributes: ['id', 'name', 'description'], + + initialize: function () { + this.on('saving', this.saving, this); + this.on('saving', this.validate, this); + }, + + validate: function () { + GhostBookshelf.validator.check(this.get('name'), "Role name cannot be blank").notEmpty(); + GhostBookshelf.validator.check(this.get('description'), "Role description cannot be blank").notEmpty(); + }, + + saving: function () { + // Deal with the related data here + + // Remove any properties which don't belong on the post model + this.attributes = this.pick(this.permittedAttributes); + }, + users: function () { return this.belongsToMany(User); }, diff --git a/core/server/models/settings.js b/core/server/models/settings.js index e0c9892ee8..56aa896085 100644 --- a/core/server/models/settings.js +++ b/core/server/models/settings.js @@ -8,13 +8,36 @@ var Settings, // Each setting is saved as a separate row in the database, // but the overlying API treats them as a single key:value mapping Settings = GhostBookshelf.Model.extend({ + tableName: 'settings', + hasTimestamps: true, + + permittedAttributes: ['id', 'uuid', 'key', 'value', 'type', 'created_at', 'created_by', 'updated_at', 'update_by'], + defaults: function () { return { uuid: uuid.v4(), type: 'general' }; + }, + + initialize: function () { + this.on('saving', this.saving, this); + this.on('saving', this.validate, this); + }, + + validate: function () { + // TODO: validate value, check type is one of the allowed values etc + GhostBookshelf.validator.check(this.get('key'), "Setting key cannot be blank").notEmpty(); + GhostBookshelf.validator.check(this.get('type'), "Setting type cannot be blank").notEmpty(); + }, + + saving: function () { + // Deal with the related data here + + // Remove any properties which don't belong on the post model + this.attributes = this.pick(this.permittedAttributes); } }, { read: function (_key) { diff --git a/core/server/models/user.js b/core/server/models/user.js index 92f5a06359..9ff72ab6f9 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -34,6 +34,11 @@ User = GhostBookshelf.Model.extend({ hasTimestamps: true, + permittedAttributes: [ + 'id', 'uuid', 'full_name', 'password', 'email_address', 'profile_picture', 'cover_picture', 'bio', 'url', 'location', + 'created_at', 'created_by', 'updated_at', 'updated_by' + ], + defaults: function () { return { uuid: uuid.v4() @@ -41,6 +46,7 @@ User = GhostBookshelf.Model.extend({ }, initialize: function () { + this.on('saving', this.saving, this); this.on('saving', this.validate, this); }, @@ -53,6 +59,13 @@ User = GhostBookshelf.Model.extend({ return true; }, + saving: function () { + // Deal with the related data here + + // Remove any properties which don't belong on the post model + this.attributes = this.pick(this.permittedAttributes); + }, + posts: function () { return this.hasMany(Posts, 'created_by'); }, diff --git a/core/test/functional/admin/01_login_test.js b/core/test/functional/admin/01_login_test.js index bafff15848..6e0f4486fe 100644 --- a/core/test/functional/admin/01_login_test.js +++ b/core/test/functional/admin/01_login_test.js @@ -37,6 +37,7 @@ casper.test.begin("Can login to Ghost", 3, function suite(test) { casper.waitFor(function checkOpaque() { return this.evaluate(function () { var loginBox = document.querySelector('.login-box'); + return window.getComputedStyle(loginBox).getPropertyValue('display') === "block" && window.getComputedStyle(loginBox).getPropertyValue('opacity') === "1"; }); @@ -62,7 +63,7 @@ casper.test.begin("Can't spam it", 2, function suite(test) { casper.test.filename = "login_test.png"; - casper.start(url + "ghost/signin/", function testTitle() { + casper.start(url + "ghost/login/", function testTitle() { test.assertTitle("", "Ghost admin has no title"); }).viewport(1280, 1024); diff --git a/core/test/functional/admin/03_editor_test.js b/core/test/functional/admin/03_editor_test.js index d5c9cd8192..8689b1574a 100644 --- a/core/test/functional/admin/03_editor_test.js +++ b/core/test/functional/admin/03_editor_test.js @@ -1,6 +1,6 @@ /*globals casper, __utils__, url, testPost */ -casper.test.begin("Ghost editor is correct", 8, function suite(test) { +casper.test.begin("Ghost editor is correct", 10, function suite(test) { casper.test.filename = "editor_test.png"; @@ -18,6 +18,12 @@ casper.test.begin("Ghost editor is correct", 8, function suite(test) { } } + // test saving with no data + casper.thenClick('.button-save').wait(500, function doneWait() { + test.assertExists('.notification-error', 'got error notification'); + test.assertSelectorDoesntHaveText('.notification-error', '[object Object]'); + }); + casper.then(function createTestPost() { casper.sendKeys('#entry-title', testPost.title); casper.writeContentToCodeMirror(testPost.content); @@ -30,11 +36,7 @@ casper.test.begin("Ghost editor is correct", 8, function suite(test) { casper.on('resource.received', handleResource); }); - casper.thenClick('.button-save').wait(1000, function doneWait() { - this.echo("I've waited for another 1 seconds."); - }); - - casper.then(function checkPostWasCreated() { + casper.thenClick('.button-save').waitForResource(/posts/, function checkPostWasCreated() { var urlRegExp = new RegExp("^" + url + "ghost\/editor\/[0-9]*"); test.assertUrlMatch(urlRegExp, 'got an id on our URL'); test.assertExists('.notification-success', 'got success notification'); diff --git a/core/test/functional/admin/05_settings_test.js b/core/test/functional/admin/05_settings_test.js index 4432e5a4b6..de35a8bda1 100644 --- a/core/test/functional/admin/05_settings_test.js +++ b/core/test/functional/admin/05_settings_test.js @@ -113,4 +113,44 @@ casper.test.begin("Settings screen is correct", 19, function suite(test) { casper.removeListener('resource.requested', handleSettingsRequest); test.done(); }); +}); + +casper.test.begin("User settings screen validates email", 6, function suite(test) { + var email, brokenEmail; + + casper.test.filename = "user_settings_test.png"; + + casper.start(url + "ghost/settings/user", function testTitleAndUrl() { + test.assertTitle("", "Ghost admin has no title"); + test.assertEquals(this.getCurrentUrl(), url + "ghost/settings/user", "Ghost doesn't require login this time"); + }).viewport(1280, 1024); + + casper.then(function setEmailToInvalid() { + email = casper.getElementInfo('#user-email').attributes.value; + brokenEmail = email.replace('.', '-'); + + casper.fillSelectors('.user-details-container', { + '#user-email': brokenEmail + }, false); + }); + + casper.thenClick('#user .button-save').waitForResource('/users/', function () { + test.assertExists('.notification-error', 'got error notification'); + test.assertSelectorDoesntHaveText('.notification-error', '[object Object]'); + }); + + casper.then(function resetEmailToValid() { + casper.fillSelectors('.user-details-container', { + '#user-email': email + }, false); + }); + + casper.thenClick('#user .button-save').waitForResource('/users/', function () { + test.assertExists('.notification-success', 'got success notification'); + test.assertSelectorDoesntHaveText('.notification-success', '[object Object]'); + }); + + casper.run(function () { + test.done(); + }); }); \ No newline at end of file diff --git a/core/test/functional/admin/06_flow_test.js b/core/test/functional/admin/06_flow_test.js new file mode 100644 index 0000000000..4c0a092072 --- /dev/null +++ b/core/test/functional/admin/06_flow_test.js @@ -0,0 +1,60 @@ +/** + * Tests the flow of creating, editing and publishing tests + */ + +/*globals casper, __utils__, url, testPost */ +casper.test.begin("Ghost edit draft flow works correctly", 7, function suite(test) { + + casper.test.filename = "flow_test.png"; + + casper.start(url + "ghost/editor", function then() { + test.assertEquals(casper.getCurrentUrl(), url + "ghost/editor", "Ghost doesn't require login this time"); + }).viewport(1280, 1024); + + // First, create a new draft post + casper.then(function createTestPost() { + casper.sendKeys('#entry-title', 'Test Draft Post'); + casper.writeContentToCodeMirror('I am a draft'); + }); + + // We must wait after sending keys to CodeMirror + casper.wait(1000, function doneWait() { + this.echo("I've waited for 1 seconds."); + }); + + casper.thenClick('.button-save').waitForResource(/posts/, function then() { + test.assertExists('.notification-success', 'got success notification'); + }); + + casper.thenOpen(url + 'ghost/content/', function then() { + test.assertEquals(casper.getCurrentUrl(), url + "ghost/content/", "Ghost doesn't require login this time"); + }); + + casper.then(function then() { + test.assertEvalEquals(function () { + return document.querySelector('.content-list-content li').className; + }, "active", "first item is active"); + + test.assertSelectorHasText(".content-list-content li:first-child h3", 'Test Draft Post', "first item is the post we created"); + }); + + casper.thenClick('.post-edit').waitForResource(/editor/, function then() { + test.assertUrlMatch(/editor/, "Ghost doesn't require login this time"); + }); + + casper.thenClick('.button-save').waitForResource(/posts/, function then() { + test.assertExists('.notification-success', 'got success notification'); + }); + + casper.run(function () { + test.done(); + }); +}); + +// TODO: test publishing, editing, republishing, unpublishing etc +//casper.test.begin("Ghost edit published flow works correctly", 6, function suite(test) { +// +// casper.test.filename = "flow_test.png"; +// +// +//}); \ No newline at end of file diff --git a/core/test/functional/admin/07_logout_test.js b/core/test/functional/admin/07_logout_test.js new file mode 100644 index 0000000000..bc1920649d --- /dev/null +++ b/core/test/functional/admin/07_logout_test.js @@ -0,0 +1,98 @@ +/** + * Tests logging out and attempting to sign up + */ + +/*globals casper, __utils__, url, testPost, falseUser, email */ +casper.test.begin("Ghost logout works correctly", 2, function suite(test) { + + casper.test.filename = "logout_test.png"; + + casper.start(url + "ghost/", function then() { + test.assertEquals(casper.getCurrentUrl(), url + "ghost/", "Ghost doesn't require login this time"); + }).viewport(1280, 1024); + + casper.thenClick('#usermenu a').waitFor(function checkOpaque() { + return this.evaluate(function () { + var loginBox = document.querySelector('#usermenu .overlay.open'); + return window.getComputedStyle(loginBox).getPropertyValue('display') === "block" + && window.getComputedStyle(loginBox).getPropertyValue('opacity') === "1"; + }); + }); + + casper.thenClick('.usermenu-signout a').waitForResource(/login/, function then() { + test.assertExists('.notification-success', 'got success notification'); + }); + + casper.run(function () { + test.done(); + }); +}); + +// has to be done after signing out +casper.test.begin("Can't spam signin", 3, function suite(test) { + + casper.test.filename = "spam_test.png"; + + casper.start(url + "ghost/signin/", function testTitle() { + test.assertTitle("", "Ghost admin has no title"); + }).viewport(1280, 1024); + + casper.waitFor(function checkOpaque() { + return this.evaluate(function () { + var loginBox = document.querySelector('.login-box'); + return window.getComputedStyle(loginBox).getPropertyValue('display') === "block" + && window.getComputedStyle(loginBox).getPropertyValue('opacity') === "1"; + }); + }, function then() { + this.fill("#login", falseUser, true); + casper.wait(200, function doneWait() { + this.fill("#login", falseUser, true); + }); + + }); + casper.wait(200, function doneWait() { + this.echo("I've waited for 1 seconds."); + }); + + casper.then(function testForErrorMessage() { + test.assertExists('.notification-error', 'got error notification'); + test.assertSelectorDoesntHaveText('.notification-error', '[object Object]'); + }); + + casper.run(function () { + test.done(); + }); +}); + +casper.test.begin("Ghost signup fails properly", 5, function suite(test) { + + casper.test.filename = "signup_test.png"; + + casper.start(url + "ghost/signup/", function then() { + test.assertEquals(casper.getCurrentUrl(), url + "ghost/signup/", "Reached signup page"); + }).viewport(1280, 1024); + + casper.then(function signupWithShortPassword() { + this.fill("#register", {email: email, password: 'test'}, true); + }); + + // should now throw a short password error + casper.waitForResource(/signup/, function () { + test.assertExists('.notification-error', 'got error notification'); + test.assertSelectorDoesntHaveText('.notification-error', '[object Object]'); + }); + + casper.then(function signupWithLongPassword() { + this.fill("#register", {email: email, password: 'testing1234'}, true); + }); + + // should now throw a 1 user only error + casper.waitForResource(/signup/, function () { + test.assertExists('.notification-error', 'got error notification'); + test.assertSelectorDoesntHaveText('.notification-error', '[object Object]'); + }); + + casper.run(function () { + test.done(); + }); +}); \ No newline at end of file diff --git a/core/test/unit/api_users_spec.js b/core/test/unit/api_users_spec.js index 53d89ad3eb..9cff2c79eb 100644 --- a/core/test/unit/api_users_spec.js +++ b/core/test/unit/api_users_spec.js @@ -3,7 +3,7 @@ var _ = require('underscore'), should = require('should'), helpers = require('./helpers'), errors = require('../../server/errorHandling'), - Models = require('../../server/models'); + Models = require('../../server/models'), when = require('when'); describe('User Model', function run() { @@ -39,7 +39,7 @@ describe('User Model', function run() { should.exist(createdUser); createdUser.has('uuid').should.equal(true); createdUser.attributes.password.should.not.equal(userData.password, "password was hashed"); - createdUser.attributes.email_address.should.eql(userData.email_address, "email address corred"); + createdUser.attributes.email_address.should.eql(userData.email_address, "email address correct"); done(); }).then(null, done);