diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index 9c99d687d66..41c02cbae51 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -321,8 +321,14 @@ authentication = { updatedUser.set('status', 'active'); return updatedUser.save(options); }) + .catch(errors.ValidationError, function (err) { + return Promise.reject(err); + }) .catch(function (err) { - throw new errors.UnauthorizedError({err: err}); + if (errors.utils.isIgnitionError(err)) { + return Promise.reject(err); + } + return Promise.reject(new errors.UnauthorizedError({err: err})); }); } diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index 322e48eb15a..fcc93d8c4e3 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -6,7 +6,10 @@ var schema = require('../schema').tables, Promise = require('bluebird'), errors = require('../../errors'), i18n = require('../../i18n'), + settingsCache = require('../../settings/cache'), + utils = require('../../utils/url'), + validatePassword, validateSchema, validateSettings, validate; @@ -15,6 +18,41 @@ function assertString(input) { assert(typeof input === 'string', 'Validator js validates strings only'); } +/** +* Counts repeated characters in a string. When 50% or more characters are the same, +* we return false and therefore invalidate the string. +* @param {String} stringToTest The password string to check. +* @return {Boolean} +*/ +function characterOccurance(stringToTest) { + var chars = {}, + allowedOccurancy, + valid = true; + + stringToTest = _.toString(stringToTest); + allowedOccurancy = stringToTest.length / 2; + + // Loop through string and accumulate character counts + _.each(stringToTest, function (char) { + if (!chars[char]) { + chars[char] = 1; + } else { + chars[char] += 1; + } + }); + + // check if any of the accumulated chars exceed the allowed occurancy + // of 50% of the words' length. + _.forIn(chars, function (charCount) { + if (charCount >= allowedOccurancy) { + valid = false; + return; + } + }); + + return valid; +} + // extends has been removed in validator >= 5.0.0, need to monkey-patch it back in validator.extend = function (name, fn) { validator[name] = function () { @@ -45,6 +83,84 @@ validator.extend('isSlug', function isSlug(str) { return validator.matches(str, /^[a-z0-9\-_]+$/); }); +/** +* Validation against simple password rules +* Returns false when validation fails and true for a valid password +* @param {String} password The password string to check. +* @param {String} email The users email address to validate agains password. +* @param {String} blogTitle Optional blogTitle value, when blog title is not set yet, e. g. in setup process. +* @return {Object} example for returned validation Object: +* invalid password: `validationResult: {isValid: false, message: 'Sorry, you cannot use an insecure password.'}` +* valid password: `validationResult: {isValid: true}` +*/ +validatePassword = function validatePassword(password, email, blogTitle) { + var validationResult = {isValid: true}, + disallowedPasswords = ['password', 'ghost', 'passw0rd'], + blogUrl = utils.urlFor('home', true), + badPasswords = [ + '1234567890', + 'qwertyuiop', + 'qwertzuiop', + 'asdfghjkl;', + 'abcdefghij', + '0987654321', + '1q2w3e4r5t', + '12345asdfg' + ]; + + blogTitle = blogTitle ? blogTitle : settingsCache.get('title'); + blogUrl = blogUrl.replace(/^http(s?):\/\//, ''); + + // password must be longer than 10 characters + if (!validator.isLength(password, 10)) { + validationResult.isValid = false; + validationResult.message = i18n.t('errors.models.user.passwordDoesNotComplyLength', {minLength: 10}); + + return validationResult; + } + + // dissallow password from badPasswords list (e. g. '1234567890') + _.each(badPasswords, function (badPassword) { + if (badPassword === password) { + validationResult.isValid = false; + } + }); + + // password must not match with users' email + if (email && email.toLowerCase() === password.toLowerCase()) { + validationResult.isValid = false; + } + + // password must not contain the words 'ghost', 'password', or 'passw0rd' + _.each(disallowedPasswords, function (disallowedPassword) { + if (password.toLowerCase().indexOf(disallowedPassword) >= 0) { + validationResult.isValid = false; + } + }); + + // password must not match with blog title + if (blogTitle && blogTitle.toLowerCase() === password.toLowerCase()) { + validationResult.isValid = false; + } + + // password must not match with blog URL (without protocol, with or without trailing slash) + if (blogUrl && (blogUrl.toLowerCase() === password.toLowerCase() || blogUrl.toLowerCase().replace(/\/$/, '') === password.toLowerCase())) { + validationResult.isValid = false; + } + + // dissallow passwords where 50% or more of characters are the same + if (!characterOccurance(password)) { + validationResult.isValid = false; + } + + // Generic error message for the rules where no dedicated error massage is set + if (!validationResult.isValid && !validationResult.message) { + validationResult.message = i18n.t('errors.models.user.passwordDoesNotComplySecurity'); + } + + return validationResult; +}; + // Validation against schema attributes // values are checked against the validation objects from schema.js validateSchema = function validateSchema(tableName, model) { @@ -174,6 +290,7 @@ validate = function validate(value, key, validations) { module.exports = { validate: validate, validator: validator, + validatePassword: validatePassword, validateSchema: validateSchema, validateSettings: validateSettings }; diff --git a/core/server/models/user.js b/core/server/models/user.js index 87cc58cc626..14b58eb8bd2 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -27,10 +27,6 @@ var _ = require('lodash'), User, Users; -function validatePasswordLength(password) { - return validator.isLength(password, 10); -} - /** * generate a random salt and then hash the password with that salt */ @@ -106,7 +102,8 @@ User = ghostBookshelf.Model.extend({ */ onSaving: function onSaving(newPage, attr, options) { var self = this, - tasks = []; + tasks = [], + passwordValidation = {}; ghostBookshelf.Model.prototype.onSaving.apply(this, arguments); @@ -169,11 +166,13 @@ User = ghostBookshelf.Model.extend({ if (this.get('status') !== 'inactive') { this.set('status', 'locked'); } - } + } else { + // CASE: we're not importing data, run the validations + passwordValidation = validation.validatePassword(this.get('password'), this.get('email')); - // don't ever validate passwords when importing - if (!options.importing && !validatePasswordLength(this.get('password'))) { - return Promise.reject(new errors.ValidationError({message: i18n.t('errors.models.user.passwordDoesNotComplyLength', {minLength: 10})})); + if (!passwordValidation.isValid) { + return Promise.reject(new errors.ValidationError({message: passwordValidation.message})); + } } tasks.hashPassword = (function hashPassword() { @@ -563,10 +562,13 @@ User = ghostBookshelf.Model.extend({ */ setup: function setup(data, options) { var self = this, - userData = this.filterData(data); + userData = this.filterData(data), + passwordValidation = {}; + + passwordValidation = validation.validatePassword(userData.password, userData.email, data.blogTitle); - if (!validatePasswordLength(userData.password)) { - return Promise.reject(new errors.ValidationError({message: i18n.t('errors.models.user.passwordDoesNotComplyLength', {minLength: 10})})); + if (!passwordValidation.isValid) { + return Promise.reject(new errors.ValidationError({message: passwordValidation.message})); } options = this.filterOptions(options, 'setup'); diff --git a/core/server/translations/en.json b/core/server/translations/en.json index 9951fbf8a40..e83cce6e7eb 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -229,6 +229,7 @@ "onlyOneRolePerUserSupported": "Only one role per user is supported at the moment.", "methodDoesNotSupportOwnerRole": "This method does not support assigning the owner role", "passwordDoesNotComplyLength": "Your password must be at least {minLength} characters long.", + "passwordDoesNotComplySecurity": "Sorry, you cannot use an insecure password.", "notEnoughPermission": "You do not have permission to perform this action", "noUserWithEnteredEmailAddr": "There is no user with that email address.", "userIsInactive": "The user with that email address is inactive.", diff --git a/core/test/functional/routes/api/authentication_spec.js b/core/test/functional/routes/api/authentication_spec.js index 38bd650f4ce..a8b0587d687 100644 --- a/core/test/functional/routes/api/authentication_spec.js +++ b/core/test/functional/routes/api/authentication_spec.js @@ -231,8 +231,8 @@ describe('Authentication API', function () { .send({ passwordreset: [{ token: token, - newPassword: 'abcdefghij', - ne2Password: 'abcdefghij' + newPassword: 'thisissupersafe', + ne2Password: 'thisissupersafe' }] }) .expect('Content-Type', /json/) diff --git a/core/test/integration/api/api_authentication_spec.js b/core/test/integration/api/api_authentication_spec.js index fe9c37145de..f5f8d5ce175 100644 --- a/core/test/integration/api/api_authentication_spec.js +++ b/core/test/integration/api/api_authentication_spec.js @@ -66,7 +66,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword', + password: 'thisissupersafe', blogTitle: 'a test blog' }; @@ -103,7 +103,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword', + password: 'thisissupersafe', blogTitle: 'a test blog' }; @@ -128,7 +128,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword' + password: 'thisissupersafe' }; AuthAPI.setup({setup: [setupData]}).then(function (result) { @@ -223,7 +223,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword', + password: 'thisissupersafe', blogTitle: 'a test blog' }; @@ -273,7 +273,7 @@ describe('Authentication API', function () { token: invite.get('token'), email: invite.get('email'), name: invite.get('email'), - password: 'eightcharacterslong' + password: 'tencharacterslong' } ] }); @@ -313,7 +313,7 @@ describe('Authentication API', function () { token: invite.get('token'), email: invite.get('email'), name: invite.get('email'), - password: 'eightcharacterslong' + password: 'tencharacterslong' } ] }); @@ -409,7 +409,7 @@ describe('Authentication API', function () { var user = { name: 'uninvited user', email: 'notinvited@example.com', - password: '1234567890', + password: 'thisissupersafe', status: 'active' }, options = { @@ -507,7 +507,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword', + password: 'thisissupersafe', blogTitle: 'a test blog' }; @@ -540,7 +540,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword', + password: 'thisissupersafe', blogTitle: 'a test blog' }; @@ -573,7 +573,7 @@ describe('Authentication API', function () { var setupData = { name: 'test user', email: 'test@example.com', - password: 'areallygoodpassword', + password: 'thisissupersafe', blogTitle: 'a test blog' }; diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index a20cc7d77ae..e990e301245 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -501,13 +501,13 @@ describe('Users API', function () { { users: [{ name: 'newname', - password: 'newpassword' + password: 'thisissupersafe' }] }, _.extend({}, context.author, {id: userIdFor.author}) ).then(function () { return models.User.findOne({id: userIdFor.author}).then(function (response) { response.get('name').should.eql('newname'); - response.get('password').should.not.eql('newpassword'); + response.get('password').should.not.eql('thisissupersafe'); done(); }); }).catch(done); diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index a8f7562f266..e33ecf9ae61 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -123,7 +123,7 @@ describe('User Model', function run() { // avoid side-effects! userData = _.cloneDeep(userData); - userData.password = 1234567890; + userData.password = 109674836589; // mocha supports promises return UserModel.add(userData, context).then(function (createdUser) { @@ -560,17 +560,115 @@ describe('User Model', function run() { done(); }); }); + + it('too short password', function (done) { + UserModel.changePassword({ + newPassword: '12345678', + ne2Password: '12345678', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); + + it('very bad password', function (done) { + UserModel.changePassword({ + newPassword: '1234567890', + ne2Password: '1234567890', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); + + it('password matches users email adress', function (done) { + UserModel.changePassword({ + newPassword: 'jbloggs@example.com', + ne2Password: 'jbloggs@example.com', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); + + it('password contains words "ghost" or "password"', function (done) { + UserModel.changePassword({ + newPassword: 'onepassword', + ne2Password: 'onepassword', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); + + it('password matches blog URL', function (done) { + UserModel.changePassword({ + newPassword: '127.0.0.1:2369', + ne2Password: '127.0.0.1:2369', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); + + it('password contains repeating chars', function (done) { + UserModel.changePassword({ + newPassword: 'cdcdcdcdcd', + ne2Password: 'cdcdcdcdcd', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); + + it('password contains repeating numbers', function (done) { + UserModel.changePassword({ + newPassword: '1231111111', + ne2Password: '1231111111', + oldPassword: 'Sl1m3rson99', + user_id: testUtils.DataGenerator.Content.users[0].id + }, testUtils.context.owner).then(function () { + done(new Error('expected error!')); + }).catch(function (err) { + (err instanceof errors.ValidationError).should.eql(true); + done(); + }); + }); }); describe('success', function () { it('can change password', function (done) { UserModel.changePassword({ - newPassword: '1234567890', - ne2Password: '1234567890', + newPassword: 'thisissupersafe', + ne2Password: 'thisissupersafe', oldPassword: 'Sl1m3rson99', user_id: testUtils.DataGenerator.Content.users[0].id }, testUtils.context.owner).then(function (user) { - user.get('password').should.not.eql('1234567890'); + user.get('password').should.not.eql('thisissupersafe'); done(); }).catch(done); }); @@ -584,7 +682,7 @@ describe('User Model', function run() { var userData = { name: 'Max Mustermann', email: 'test@ghost.org', - password: '1234567890' + password: 'thisissupersafe' }; UserModel.setup(userData, {id: 1}) diff --git a/core/test/unit/validation_spec.js b/core/test/unit/validation_spec.js index 4f3e2dc4346..fa1bbf69241 100644 --- a/core/test/unit/validation_spec.js +++ b/core/test/unit/validation_spec.js @@ -12,6 +12,7 @@ describe('Validation', function () { ); validation.validate.should.be.a.Function(); + validation.validatePassword.should.be.a.Function(); validation.validateSchema.should.be.a.Function(); validation.validateSettings.should.be.a.Function();