From 9323abbb44a8f3470c24033aceed974bbe473153 Mon Sep 17 00:00:00 2001
From: Rem Zolotykh <rem@zolotykh.net>
Date: Tue, 7 Jul 2015 15:32:50 +0200
Subject: [PATCH] Refactor changePassword and resetPassword

issue #5500
- make `changePassword` and `resetPassword` methods on `user` model
  consistent: use `object` and `options` arguments instead of multiple
  different arguments
- change User API `changePassword` method to use these new arguments
---
 core/server/api/users.js                      | 29 ++-----------------
 core/server/models/user.js                    | 19 +++++++-----
 .../integration/model/model_users_spec.js     |  2 +-
 3 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/core/server/api/users.js b/core/server/api/users.js
index ed317cf34c..508c671330 100644
--- a/core/server/api/users.js
+++ b/core/server/api/users.js
@@ -434,26 +434,6 @@ users = {
      */
     changePassword: function changePassword(object, options) {
         var tasks;
-        /**
-         * ### Validation
-         * Ensure we have valid options - special validation just for password
-         * @TODO change User.changePassword to take an object not 4 args
-         * @param {Object} object
-         * @param {Object} options
-         * @returns {Object} options
-         */
-        function validate(object, options) {
-            options = options || {};
-            return utils.checkObject(object, 'password').then(function (data) {
-                options.data = {
-                    oldPassword: data.password[0].oldPassword,
-                    newPassword: data.password[0].newPassword,
-                    ne2Password: data.password[0].ne2Password,
-                    userId: parseInt(data.password[0].user_id)
-                };
-                return options;
-            });
-        }
 
         /**
          * ### Handle Permissions
@@ -462,7 +442,7 @@ users = {
          * @returns {Object} options
          */
         function handlePermissions(options) {
-            return canThis(options.context).edit.user(options.data.userId).then(function permissionGranted() {
+            return canThis(options.context).edit.user(options.data.password[0].user_id).then(function permissionGranted() {
                 return options;
             }).catch(function (error) {
                 return errors.handleAPIError(error, 'You do not have permission to change the password for this user');
@@ -477,16 +457,13 @@ users = {
          */
         function doQuery(options) {
             return dataProvider.User.changePassword(
-                options.data.oldPassword,
-                options.data.newPassword,
-                options.data.ne2Password,
-                options.data.userId,
+                options.data.password[0],
                 _.omit(options, ['data'])
             );
         }
 
         // Push all of our tasks into a `tasks` array in the correct order
-        tasks = [validate, handlePermissions, utils.convertOptions(allowedIncludes), doQuery];
+        tasks = [utils.validate('password'), handlePermissions, utils.convertOptions(allowedIncludes), doQuery];
 
         // Pipeline calls each task passing the result of one to be the arguments for the next
         return pipeline(tasks, object, options).then(function formatResponse() {
diff --git a/core/server/models/user.js b/core/server/models/user.js
index 3b9ce08776..c44ef85fb2 100644
--- a/core/server/models/user.js
+++ b/core/server/models/user.js
@@ -586,14 +586,15 @@ User = ghostBookshelf.Model.extend({
 
     /**
      * Naive change password method
-     * @param {String} oldPassword
-     * @param {String} newPassword
-     * @param {String} ne2Password
-     * @param {Integer} userId
+     * @param {Object} object
      * @param {Object} options
      */
-    changePassword: function changePassword(oldPassword, newPassword, ne2Password, userId, options) {
+    changePassword: function changePassword(object, options) {
         var self = this,
+            newPassword = object.newPassword,
+            ne2Password = object.ne2Password,
+            userId = object.user_id,
+            oldPassword = object.oldPassword,
             user;
 
         if (newPassword !== ne2Password) {
@@ -706,8 +707,12 @@ User = ghostBookshelf.Model.extend({
         });
     },
 
-    resetPassword: function resetPassword(token, newPassword, ne2Password, dbHash) {
-        var self = this;
+    resetPassword: function resetPassword(options) {
+        var self = this,
+            token = options.token,
+            newPassword = options.newPassword,
+            ne2Password = options.ne2Password,
+            dbHash = options.dbHash;
 
         if (newPassword !== ne2Password) {
             return Promise.reject(new Error('Your new passwords do not match'));
diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js
index 0a88b9bc7d..d6a45e3db3 100644
--- a/core/test/integration/model/model_users_spec.js
+++ b/core/test/integration/model/model_users_spec.js
@@ -592,7 +592,7 @@ describe('User Model', function run() {
                 return UserModel.generateResetToken(firstUser.attributes.email, expires, dbHash);
             }).then(function (token) {
                 token = utils.encodeBase64URLsafe(token);
-                return UserModel.resetPassword(token, 'newpassword', 'newpassword', dbHash);
+                return UserModel.resetPassword({token: token, newPassword: 'newpassword', ne2Password: 'newpassword', dbHash: dbHash});
             }).then(function (resetUser) {
                 var resetPassword = resetUser.get('password');