From 40c78493a38c6ea82137026851fe7af3f79ab945 Mon Sep 17 00:00:00 2001 From: Sebastian Gierlinger Date: Thu, 11 Dec 2014 21:23:07 +0100 Subject: [PATCH] Password change MU closes #4624 - added user_id to password reset request - hide old password field - updated changePassword method to check permissions - updated changePassword method to work without oldPassword - fixed bug for errors shown as [Object object] --- core/client/models/user.js | 1 + core/client/templates/settings/users/user.hbs | 3 +- core/server/api/users.js | 20 ++- core/server/models/user.js | 22 +++- core/test/integration/api/api_users_spec.js | 120 ++++++++++++++++++ 5 files changed, 151 insertions(+), 15 deletions(-) diff --git a/core/client/models/user.js b/core/client/models/user.js index ff03b847c1..e6e7e11169 100644 --- a/core/client/models/user.js +++ b/core/client/models/user.js @@ -52,6 +52,7 @@ var User = DS.Model.extend(NProgressSaveMixin, SelectiveSaveMixin, ValidationEng type: 'PUT', data: { password: [{ + user_id: this.get('id'), oldPassword: this.get('password'), newPassword: this.get('newPassword'), ne2Password: this.get('ne2Password') diff --git a/core/client/templates/settings/users/user.hbs b/core/client/templates/settings/users/user.hbs index 6e6f75c06b..cab93006c5 100644 --- a/core/client/templates/settings/users/user.hbs +++ b/core/client/templates/settings/users/user.hbs @@ -96,11 +96,12 @@
- + {{#unless view.isNotOwnProfile}}
{{input value=user.password type="password" id="user-password-old"}}
+ {{/unless}}
diff --git a/core/server/api/users.js b/core/server/api/users.js index aa0412ca18..ff12469522 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -323,17 +323,23 @@ users = { changePassword: function changePassword(object, options) { var oldPassword, newPassword, - ne2Password; + ne2Password, + userId; + return utils.checkObject(object, 'password').then(function (checkedPasswordReset) { oldPassword = checkedPasswordReset.password[0].oldPassword; newPassword = checkedPasswordReset.password[0].newPassword; ne2Password = checkedPasswordReset.password[0].ne2Password; - - return dataProvider.User.changePassword(oldPassword, newPassword, ne2Password, options).then(function () { - return Promise.resolve({password: [{message: 'Password changed successfully.'}]}); - }).catch(function (error) { - return Promise.reject(new errors.ValidationError(error.message)); - }); + userId = parseInt(checkedPasswordReset.password[0].user_id); + }).then(function () { + return canThis(options.context).edit.user(userId); + }).then(function () { + return dataProvider.User.changePassword(oldPassword, newPassword, ne2Password, userId, options); + }).then(function () { + return Promise.resolve({password: [{message: 'Password changed successfully.'}]}); + }).catch(function (error) { + // return Promise.reject(new errors.ValidationError(error.message)); + return errors.handleAPIError(error, 'You do not have permission to change the password for this user'); }); }, diff --git a/core/server/models/user.js b/core/server/models/user.js index c0014a53f1..224dc64d32 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -24,7 +24,7 @@ var _ = require('lodash'), function validatePasswordLength(password) { try { if (!validator.isLength(password, 8)) { - throw new Error('Your password must be at least 8 characters long.'); + throw new errors.ValidationError('Your password must be at least 8 characters long.'); } } catch (error) { return Promise.reject(error); @@ -687,25 +687,33 @@ User = ghostBookshelf.Model.extend({ * @param {String} oldPassword * @param {String} newPassword * @param {String} ne2Password + * @param {Integer} userId * @param {Object} options */ - changePassword: function (oldPassword, newPassword, ne2Password, options) { + changePassword: function (oldPassword, newPassword, ne2Password, userId, options) { var self = this, - userid = options.context.user, user = null; if (newPassword !== ne2Password) { - return Promise.reject(new Error('Your new passwords do not match')); + return Promise.reject(new errors.ValidationError('Your new passwords do not match')); + } + + if (userId === options.context.user && _.isEmpty(oldPassword)) { + return Promise.reject(new errors.ValidationError('Password is required for this operation')); } return validatePasswordLength(newPassword).then(function () { - return self.forge({id: userid}).fetch({require: true}); + return self.forge({id: userId}).fetch({require: true}); }).then(function (_user) { user = _user; - return bcryptCompare(oldPassword, user.get('password')); + if (userId === options.context.user) { + return bcryptCompare(oldPassword, user.get('password')); + } + // if user is admin, password isn't compared + return true; }).then(function (matched) { if (!matched) { - return Promise.reject(new Error('Your password is incorrect')); + return Promise.reject(new errors.ValidationError('Your password is incorrect')); } return bcryptGenSalt(); }).then(function (salt) { diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index 65425b87ee..177ce24d21 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -1019,4 +1019,124 @@ describe('Users API', function () { }); }); }); + + describe('Change Password', function () { + it('Owner can change own password', function (done) { + var payload = { + password: [{ + user_id: userIdFor.owner, + oldPassword: 'Sl1m3rson', + newPassword: 'newSl1m3rson', + ne2Password: 'newSl1m3rson' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) + .then(function (response) { + response.password[0].message.should.eql('Password changed successfully.'); + done(); + }).catch(done); + }); + + it('Owner can\'t change password with wrong oldPassword', function (done) { + var payload = { + password: [{ + user_id: userIdFor.owner, + oldPassword: 'wrong', + newPassword: 'Sl1m3rson', + ne2Password: 'Sl1m3rson' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) + .then(function () { + done(new Error('Password change is not denied.')); + }).catch(function (error) { + error.type.should.eql('ValidationError'); + done(); + }); + }); + + it('Owner can\'t change password without matching passwords', function (done) { + var payload = { + password: [{ + user_id: userIdFor.owner, + oldPassword: 'Sl1m3rson', + newPassword: 'Sl1m3rson1', + ne2Password: 'Sl1m3rson2' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) + .then(function () { + done(new Error('Password change is not denied.')); + }).catch(function (error) { + error.type.should.eql('ValidationError'); + done(); + }); + }); + + it('Owner can\'t change editor password without matching passwords', function (done) { + var payload = { + password: [{ + user_id: userIdFor.editor, + newPassword: 'Sl1m3rson1', + ne2Password: 'Sl1m3rson2' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) + .then(function () { + done(new Error('Password change is not denied.')); + }).catch(function (error) { + error.type.should.eql('ValidationError'); + done(); + }); + }); + + it('Owner can\'t change editor password without short passwords', function (done) { + var payload = { + password: [{ + user_id: userIdFor.editor, + newPassword: 'Sl', + ne2Password: 'Sl' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) + .then(function () { + done(new Error('Password change is not denied.')); + }).catch(function (error) { + error.type.should.eql('ValidationError'); + done(); + }); + }); + + it('Owner can change password for editor', function (done) { + var payload = { + password: [{ + user_id: userIdFor.editor, + newPassword: 'newSl1m3rson', + ne2Password: 'newSl1m3rson' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner})) + .then(function (response) { + response.password[0].message.should.eql('Password changed successfully.'); + done(); + }).catch(done); + }); + + it('Editor can\'t change password for admin', function (done) { + var payload = { + password: [{ + user_id: userIdFor.admin, + newPassword: 'newSl1m3rson', + ne2Password: 'newSl1m3rson' + }] + }; + UserAPI.changePassword(payload, _.extend({}, context.editor, {id: userIdFor.editor})) + .then(function () { + done(new Error('Password change is not denied.')); + }).catch(function (error) { + error.type.should.eql('NoPermissionError'); + done(); + }); + }); + }); });