From 110a5be253ef366194c00c0b35ed1d674ff918ce Mon Sep 17 00:00:00 2001
From: David Balderston <dbalders@gmail.com>
Date: Fri, 25 Mar 2016 17:17:06 -0700
Subject: [PATCH] Check Old Password on Password Change

Closes #6620

* Changed it from always returning true, to evaluate if it is the
current logged in user, and if so, check the old password. If not,
ignore
---
 core/server/models/user.js                  |  8 ++++++--
 core/test/integration/api/api_users_spec.js | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/core/server/models/user.js b/core/server/models/user.js
index c7789a50f7..fb1a03e268 100644
--- a/core/server/models/user.js
+++ b/core/server/models/user.js
@@ -594,28 +594,32 @@ User = ghostBookshelf.Model.extend({
         var self = this,
             newPassword = object.newPassword,
             ne2Password = object.ne2Password,
-            userId = object.user_id,
+            userId = parseInt(object.user_id),
             oldPassword = object.oldPassword,
             user;
 
+        // If the two passwords do not match
         if (newPassword !== ne2Password) {
             return Promise.reject(new errors.ValidationError(i18n.t('errors.models.user.newPasswordsDoNotMatch')));
         }
 
+        // If the old password is empty when changing current user's password
         if (userId === options.context.user && _.isEmpty(oldPassword)) {
             return Promise.reject(new errors.ValidationError(i18n.t('errors.models.user.passwordRequiredForOperation')));
         }
 
+        // If password is not complex enough
         if (!validatePasswordLength(newPassword)) {
             return Promise.reject(new errors.ValidationError(i18n.t('errors.models.user.passwordDoesNotComplyLength')));
         }
 
         return self.forge({id: userId}).fetch({require: true}).then(function then(_user) {
             user = _user;
+            // If the user is the current user, check old password
             if (userId === options.context.user) {
                 return bcryptCompare(oldPassword, user.get('password'));
             }
-            // if user is admin, password isn't compared
+            // If user is admin and changing another user's password, old password isn't compared to the old one
             return true;
         }).then(function then(matched) {
             if (!matched) {
diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js
index 026021bb3e..0f471f7945 100644
--- a/core/test/integration/api/api_users_spec.js
+++ b/core/test/integration/api/api_users_spec.js
@@ -1138,6 +1138,21 @@ describe('Users API', function () {
                 }).catch(checkForErrorType('ValidationError', done));
         });
 
+        it('Owner can\'t change password without old password', function (done) {
+            var payload = {
+                password: [{
+                    user_id: userIdFor.owner,
+                    oldPassword: '',
+                    newPassword: 'Sl1m3rson1',
+                    ne2Password: 'Sl1m3rson1'
+                }]
+            };
+            UserAPI.changePassword(payload, _.extend({}, context.owner, {id: userIdFor.owner}))
+                .then(function () {
+                    done(new Error('Password change is not denied.'));
+                }).catch(checkForErrorType('ValidationError', done));
+        });
+
         it('Owner can\'t change password without matching passwords', function (done) {
             var payload = {
                 password: [{