Merge pull request '#6744 fix worker setPassword' (!2027) from 6744-fixWorkerSetPassword into dev

Reviewed-on: #2027
Reviewed-by: Javi Gallego <jgallego@verdnatura.es>
Reviewed-by: Juan Ferrer <juan@verdnatura.es>

loopback/locale/en.json

@@ -220,5 +220,7 @@
 	"Shelving not valid": "Shelving not valid",
 	"printerNotExists": "The printer does not exist",
 	"There are not picking tickets": "There are not picking tickets",
-	"ticketCommercial": "The ticket {{ ticket }} for the salesperson {{ salesMan }} is in preparation. (automatically generated message)"
+	"ticketCommercial": "The ticket {{ ticket }} for the salesperson {{ salesMan }} is in preparation. (automatically generated message)",
+    "This password can only be changed by the user themselves": "This password can only be changed by the user themselves",
+    "They're not your subordinate": "They're not your subordinate"
 }

loopback/locale/es.json

@@ -346,5 +346,7 @@
     "CountryFK cannot be empty": "El país no puede estar vacío",
     "Cmr file does not exist": "El archivo del cmr no existe",
     "You are not allowed to modify the alias": "No estás autorizado a modificar el alias",
-    "The address of the customer must have information about Incoterms and Customs Agent": "El consignatario del cliente debe tener informado Incoterms y Agente de aduanas"
+    "The address of the customer must have information about Incoterms and Customs Agent": "El consignatario del cliente debe tener informado Incoterms y Agente de aduanas",
+    "This password can only be changed by the user themselves": "Esta contraseña solo puede ser modificada por el propio usuario",
+    "They're not your subordinate": "No es tu subordinado/a."
 }

modules/account/back/models/account.js

@@ -1,4 +1,7 @@
 
+const ForbiddenError = require('vn-loopback/util/forbiddenError');
+const {models} = require('vn-loopback/server/server');
+
 module.exports = Self => {
     require('../methods/account/sync')(Self);
     require('../methods/account/sync-by-id')(Self);
@@ -7,4 +10,11 @@ module.exports = Self => {
     require('../methods/account/logout')(Self);
     require('../methods/account/change-password')(Self);
     require('../methods/account/set-password')(Self);
+
+    Self.setUnverifiedPassword = async(id, pass, options) => {
+        const {emailVerified} = await models.VnUser.findById(id, {fields: ['emailVerified']}, options);
+        if (emailVerified) throw new ForbiddenError('This password can only be changed by the user themselves');
+
+        await models.VnUser.setPassword(id, pass, options);
+    };
 };

modules/worker/back/methods/worker/setPassword.js

@@ -1,31 +1,29 @@
-const UserError = require('vn-loopback/util/user-error');
+const ForbiddenError = require('vn-loopback/util/forbiddenError');
 module.exports = Self => {
     Self.remoteMethodCtx('setPassword', {
         description: 'Set a new password',
-        accepts: [
-            {
-                arg: 'workerFk',
+        accepts: [{
+            arg: 'id',
             type: 'number',
             required: true,
             description: 'The worker id',
-            },
-            {
+            http: {source: 'path'}
+        }, {
             arg: 'newPass',
             type: 'String',
             required: true,
             description: 'The new worker password'
-            }
-        ],
+        }],
         http: {
             path: `/:id/setPassword`,
             verb: 'PATCH'
         }
     });
-    Self.setPassword = async(ctx, options) => {
+    Self.setPassword = async(ctx, id, newPass, options) => {
         const models = Self.app.models;
         const myOptions = {};
-        const {args} = ctx;
         let tx;
+
         if (typeof options == 'object')
             Object.assign(myOptions, options);
         if (!myOptions.transaction) {
@@ -33,11 +31,10 @@ module.exports = Self => {
             myOptions.transaction = tx;
         }
         try {
-            const isSubordinate = await models.Worker.isSubordinate(ctx, args.workerFk, myOptions);
-            if (!isSubordinate) throw new UserError('You don\'t have enough privileges.');
+            const isSubordinate = await Self.isSubordinate(ctx, id, myOptions);
+            if (!isSubordinate) throw new ForbiddenError('They\'re not your subordinate');
 
-            await models.VnUser.setPassword(args.workerFk, args.newPass, myOptions);
-            await models.VnUser.updateAll({id: args.workerFk}, {emailVerified: true}, myOptions);
+            await models.Account.setUnverifiedPassword(id, newPass, myOptions);
 
             if (tx) await tx.commit();
         } catch (e) {

modules/worker/back/methods/worker/specs/setPassword.spec.js

@@ -1,31 +1,30 @@
-const UserError = require('vn-loopback/util/user-error');
-
-const models = require('vn-loopback/server/server').models;
+const {models} = require('vn-loopback/server/server');
 
 describe('worker setPassword()', () => {
     let ctx;
+    const newPass = 'H3rn4d3z#';
+    const employeeId = 1;
+    const managerId = 20;
+    const administrativeId = 5;
+
     beforeAll(() => {
         ctx = {
             req: {
-                accessToken: {},
+                accessToken: {userId: managerId},
                 headers: {origin: 'http://localhost'}
             },
-            args: {workerFk: 9}
         };
     });
 
-    beforeEach(() => {
-        ctx.req.accessToken.userId = 20;
-        ctx.args.newPass = 'H3rn4d3z#';
-    });
-
-    it('should change the password', async() => {
+    it('should change the password if it is a subordinate and the email is not verified', async() => {
         const tx = await models.Worker.beginTransaction({});
 
         try {
             const options = {transaction: tx};
-            await models.Worker.setPassword(ctx, options);
+            await models.Worker.setPassword(ctx, employeeId, newPass, options);
+            const isNewPass = await passHasBeenChanged(employeeId, newPass, options);
 
+            expect(isNewPass).toBeTrue();
             await tx.rollback();
         } catch (e) {
             await tx.rollback();
@@ -33,29 +32,48 @@ describe('worker setPassword()', () => {
         }
     });
 
-    it('should throw an error: Password does not meet requirements', async() => {
-        const tx = await models.Collection.beginTransaction({});
-        ctx.args.newPass = 'Hi';
+    it('should not change the password if it is a subordinate and the email is verified', async() => {
+        const tx = await models.Worker.beginTransaction({});
+
         try {
             const options = {transaction: tx};
-            await models.Worker.setPassword(ctx, options);
+            await models.VnUser.updateAll({id: employeeId}, {emailVerified: true}, options);
+            await models.Worker.setPassword(ctx, employeeId, newPass, options);
+
+            await tx.rollback();
+        } catch (e) {
+            expect(e.message).toEqual(`This password can only be changed by the user themselves`);
+            await tx.rollback();
+        }
+    });
+
+    it('should not change the password if it is not a subordinate', async() => {
+        const tx = await models.Worker.beginTransaction({});
+        try {
+            const options = {transaction: tx};
+            await models.Worker.setPassword(ctx, administrativeId, newPass, options);
+            await tx.rollback();
+        } catch (e) {
+            expect(e.message).toEqual(`They're not your subordinate`);
+            await tx.rollback();
+        }
+    });
+
+    it('should throw an error: Password does not meet requirements', async() => {
+        const tx = await models.Worker.beginTransaction({});
+        const newPass = 'Hi';
+        try {
+            const options = {transaction: tx};
+            await models.Worker.setPassword(ctx, employeeId, newPass, options);
             await tx.rollback();
         } catch (e) {
             expect(e.sqlMessage).toEqual('Password does not meet requirements');
             await tx.rollback();
         }
     });
-
-    it('should throw an error: You don\'t have enough privileges.', async() => {
-        ctx.req.accessToken.userId = 5;
-        const tx = await models.Collection.beginTransaction({});
-        try {
-            const options = {transaction: tx};
-            await models.Worker.setPassword(ctx, options);
-            await tx.rollback();
-        } catch (e) {
-            expect(e).toEqual(new UserError(`You don't have enough privileges.`));
-            await tx.rollback();
-        }
-    });
 });
+
+const passHasBeenChanged = async(userId, pass, options) => {
+    const user = await models.VnUser.findById(userId, null, options);
+    return user.hasPassword(pass);
+};

modules/worker/front/descriptor/index.html

@@ -11,7 +11,7 @@
                 ? 'Click to allow the user to be disabled'
                 : 'Click to exclude the user from getting disabled'}}
           </vn-item>
-          <vn-item ng-if="!$ctrl.worker.user.emailVerified" ng-click="setPassword.show()" translate>
+          <vn-item ng-if="!$ctrl.worker.user.emailVerified && $ctrl.vnConfig.storage.currentUserWorkerId !=$ctrl.worker.id" ng-click="setPassword.show()" translate>
             Change password 
           </vn-item>
     </slot-menu>

modules/worker/front/descriptor/index.js

@@ -69,6 +69,7 @@ class Controller extends Descriptor {
                 }
             ]
         };
+
         return this.getData(`Workers/${this.id}`, {filter})
             .then(res => this.entity = res.data);
     }
@@ -86,15 +87,14 @@ class Controller extends Descriptor {
         if (this.newPassword != this.repeatPassword)
             throw new UserError(`Passwords don't match`);
         this.$http.patch(
-            `Workers/${this.entity.id}/setPassword`,
-            {workerFk: this.entity.id, newPass: this.newPassword}
+            `Workers/${this.entity.id}/setPassword`, {newPass: this.newPassword}
         ) .then(() => {
             this.vnApp.showSuccess(this.$translate.instant('Password changed!'));
-        });
+        }).then(() => this.loadData());
     }
 }
 
-Controller.$inject = ['$element', '$scope', '$rootScope'];
+Controller.$inject = ['$element', '$scope', '$rootScope', 'vnConfig'];
 
 ngModule.vnComponent('vnWorkerDescriptor', {
     template: require('./index.html'),

modules/worker/front/descriptor/index.spec.js

@@ -16,6 +16,7 @@ describe('vnWorkerDescriptor', () => {
             const id = 1;
             const response = 'foo';
 
+            $httpBackend.whenGET('UserConfigs/getUserConfig').respond({});
             $httpBackend.expectRoute('GET', `Workers/${id}`).respond(response);
             controller.id = id;
             $httpBackend.flush();