diff --git a/core/server/models/user.js b/core/server/models/user.js index 19c957a182..f71d3671bb 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -131,7 +131,7 @@ User = ghostBookshelf.Model.extend({ * normal behaviour if not (set random password, lock user, and hash password) * - no validations should run, when importing */ - if (self.isNew() || self.hasChanged('password')) { + if (self.hasChanged('password')) { this.set('password', String(this.get('password'))); // CASE: import with `importPersistUser` should always be an bcrypt password already, diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index 63df6d9fdf..d6cc2cb685 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -117,20 +117,6 @@ describe('User Model', function run() { }).catch(done); }); - it('can set password of only numbers', function () { - var userData = testUtils.DataGenerator.forModel.users[0]; - - // avoid side-effects! - userData = _.cloneDeep(userData); - userData.password = 109674836589; - - // mocha supports promises - return UserModel.add(userData, context).then(function (createdUser) { - should.exist(createdUser); - // cannot validate password - }); - }); - it('can find by email and is case insensitive', function (done) { var userData = testUtils.DataGenerator.forModel.users[2], email = testUtils.DataGenerator.forModel.users[2].email; diff --git a/core/test/unit/models/user_spec.js b/core/test/unit/models/user_spec.js index ee1e24465f..5a9a3370d7 100644 --- a/core/test/unit/models/user_spec.js +++ b/core/test/unit/models/user_spec.js @@ -1,16 +1,73 @@ -var should = require('should'), +'use strict'; + +const should = require('should'), sinon = require('sinon'), models = require('../../../server/models'), common = require('../../../server/lib/common'), - utils = require('../../utils'), - + security = require('../../../server/lib/security'), + testUtils = require('../../utils'), sandbox = sinon.sandbox.create(); describe('Models: User', function () { + let knexMock; + before(function () { models.init(); }); + afterEach(function () { + sandbox.restore(); + }); + + describe('validation', function () { + before(function () { + knexMock = new testUtils.mocks.knex(); + knexMock.mock(); + }); + + beforeEach(function () { + sandbox.stub(security.password, 'hash').resolves('$2a$10$we16f8rpbrFZ34xWj0/ZC.LTPUux8ler7bcdTs5qIleN6srRHhilG'); + }); + + after(function () { + knexMock.unmock(); + }); + + describe('password', function () { + it('no password', function () { + return models.User.add({email: 'test1@ghost.org', name: 'Ghosty'}) + .then(function (user) { + user.get('name').should.eql('Ghosty'); + should.exist(user.get('password')); + }); + }); + + it('only numbers', function () { + return models.User.add({email: 'test2@ghost.org', name: 'Wursti', password: 109674836589}) + .then(function (user) { + user.get('name').should.eql('Wursti'); + should.exist(user.get('password')); + }); + }); + + it('can change password', function () { + let oldPassword; + + return models.User.findOne({slug: 'joe-bloggs'}) + .then(function (user) { + user.get('slug').should.eql('joe-bloggs'); + oldPassword = user.get('password'); + user.set('password', '12734!!332'); + return user.save(); + }) + .then(function (user) { + user.get('slug').should.eql('joe-bloggs'); + user.get('password').should.not.eql(oldPassword); + }); + }); + }); + }); + describe('Permissible', function () { function getUserModel(id, role) { var hasRole = sandbox.stub(); @@ -28,7 +85,7 @@ describe('Models: User', function () { var mockUser = getUserModel(1, 'Owner'), context = {user: 1}; - models.User.permissible(mockUser, 'destroy', context, {}, utils.permissions.owner, true, true).then(() => { + models.User.permissible(mockUser, 'destroy', context, {}, testUtils.permissions.owner, true, true).then(() => { done(new Error('Permissible function should have errored')); }).catch((error) => { error.should.be.an.instanceof(common.errors.NoPermissionError); @@ -41,7 +98,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Contributor'), context = {user: 3}; - return models.User.permissible(mockUser, 'edit', context, {}, utils.permissions.contributor, false, true).then(() => { + return models.User.permissible(mockUser, 'edit', context, {}, testUtils.permissions.contributor, false, true).then(() => { should(mockUser.get.calledOnce).be.true(); }); }); @@ -51,7 +108,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Editor'), context = {user: 2}; - models.User.permissible(mockUser, 'edit', context, {}, utils.permissions.editor, true, true).then(() => { + models.User.permissible(mockUser, 'edit', context, {}, testUtils.permissions.editor, true, true).then(() => { done(new Error('Permissible function should have errored')); }).catch((error) => { error.should.be.an.instanceof(common.errors.NoPermissionError); @@ -65,7 +122,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Administrator'), context = {user: 2}; - models.User.permissible(mockUser, 'edit', context, {}, utils.permissions.editor, true, true).then(() => { + models.User.permissible(mockUser, 'edit', context, {}, testUtils.permissions.editor, true, true).then(() => { done(new Error('Permissible function should have errored')); }).catch((error) => { error.should.be.an.instanceof(common.errors.NoPermissionError); @@ -79,7 +136,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Author'), context = {user: 2}; - return models.User.permissible(mockUser, 'edit', context, {}, utils.permissions.editor, true, true).then(() => { + return models.User.permissible(mockUser, 'edit', context, {}, testUtils.permissions.editor, true, true).then(() => { should(mockUser.hasRole.called).be.true(); should(mockUser.get.calledOnce).be.true(); }); @@ -89,7 +146,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Contributor'), context = {user: 2}; - return models.User.permissible(mockUser, 'edit', context, {}, utils.permissions.editor, true, true).then(() => { + return models.User.permissible(mockUser, 'edit', context, {}, testUtils.permissions.editor, true, true).then(() => { should(mockUser.hasRole.called).be.true(); should(mockUser.get.calledOnce).be.true(); }); @@ -99,7 +156,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Editor'), context = {user: 3}; - return models.User.permissible(mockUser, 'destroy', context, {}, utils.permissions.editor, true, true).then(() => { + return models.User.permissible(mockUser, 'destroy', context, {}, testUtils.permissions.editor, true, true).then(() => { should(mockUser.hasRole.called).be.true(); should(mockUser.get.calledOnce).be.true(); }); @@ -109,7 +166,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Editor'), context = {user: 2}; - models.User.permissible(mockUser, 'destroy', context, {}, utils.permissions.editor, true, true).then(() => { + models.User.permissible(mockUser, 'destroy', context, {}, testUtils.permissions.editor, true, true).then(() => { done(new Error('Permissible function should have errored')); }).catch((error) => { error.should.be.an.instanceof(common.errors.NoPermissionError); @@ -123,7 +180,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Administrator'), context = {user: 2}; - models.User.permissible(mockUser, 'destroy', context, {}, utils.permissions.editor, true, true).then(() => { + models.User.permissible(mockUser, 'destroy', context, {}, testUtils.permissions.editor, true, true).then(() => { done(new Error('Permissible function should have errored')); }).catch((error) => { error.should.be.an.instanceof(common.errors.NoPermissionError); @@ -137,7 +194,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Author'), context = {user: 2}; - return models.User.permissible(mockUser, 'destroy', context, {}, utils.permissions.editor, true, true).then(() => { + return models.User.permissible(mockUser, 'destroy', context, {}, testUtils.permissions.editor, true, true).then(() => { should(mockUser.hasRole.called).be.true(); should(mockUser.get.calledOnce).be.true(); }); @@ -147,7 +204,7 @@ describe('Models: User', function () { var mockUser = getUserModel(3, 'Contributor'), context = {user: 2}; - return models.User.permissible(mockUser, 'destroy', context, {}, utils.permissions.editor, true, true).then(() => { + return models.User.permissible(mockUser, 'destroy', context, {}, testUtils.permissions.editor, true, true).then(() => { should(mockUser.hasRole.called).be.true(); should(mockUser.get.calledOnce).be.true(); });