Merge pull request '4185-secure_client_pass_changes' (#1004) from 4185-secure_client_pass_changes into dev

Reviewed-on: #1004
Reviewed-by: Joan Sanchez <joan@verdnatura.es>

back/methods/account/set-password.js

@@ -1,16 +1,15 @@
-
 module.exports = Self => {
     Self.remoteMethod('setPassword', {
         description: 'Sets the user password',
         accepts: [
             {
                 arg: 'id',
-                type: 'Number',
+                type: 'number',
                 description: 'The user id',
                 http: {source: 'path'}
             }, {
                 arg: 'newPassword',
-                type: 'String',
+                type: 'string',
                 description: 'The new password',
                 required: true
             }

db/changes/10481-june/00-ACL.sql

@@ -0,0 +1,4 @@
+INSERT INTO `salix`.`ACL` (model,property,accessType,permission,principalType,principalId)
+	VALUES 
+		('Client','setPassword','WRITE','ALLOW','ROLE','salesPerson'),
+		('Client','updateUser','WRITE','ALLOW','ROLE','salesPerson');

e2e/paths/02-client/07_edit_web_access.spec.js

@@ -2,6 +2,7 @@ import selectors from '../../helpers/selectors';
 import getBrowser from '../../helpers/puppeteer';
 
 describe('Client Edit web access path', () => {
+    pending('#4170 e2e account descriptor');
     let browser;
     let page;
     beforeAll(async() => {

loopback/locale/en.json

@@ -123,5 +123,6 @@
 	"The worker has hours recorded that day": "The worker has hours recorded that day",
 	"isWithoutNegatives": "isWithoutNegatives",
 	"routeFk": "routeFk",
-	"Not enough privileges to edit a client with verified data": "Not enough privileges to edit a client with verified data"
+	"Not enough privileges to edit a client with verified data": "Not enough privileges to edit a client with verified data",
+	"Can't change the password of another worker": "Can't change the password of another worker"
 }

loopback/locale/es.json

@@ -226,5 +226,6 @@
 	"reference duplicated": "Referencia duplicada",
 	"This ticket is already a refund": "Este ticket ya es un abono",
 	"isWithoutNegatives": "isWithoutNegatives",
-	"routeFk": "routeFk"
+	"routeFk": "routeFk",
+	"Can't change the password of another worker": "No se puede cambiar la contraseña de otro trabajador"
 }

modules/client/back/methods/client/setPassword.js

@@ -0,0 +1,32 @@
+module.exports = Self => {
+    Self.remoteMethod('setPassword', {
+        description: 'Sets the password of a non-worker client',
+        accepts: [
+            {
+                arg: 'id',
+                type: 'number',
+                description: 'The user id',
+                http: {source: 'path'}
+            }, {
+                arg: 'newPassword',
+                type: 'string',
+                description: 'The new password',
+                required: true
+            }
+        ],
+        http: {
+            path: `/:id/setPassword`,
+            verb: 'PATCH'
+        }
+    });
+
+    Self.setPassword = async function(id, newPassword) {
+        const models = Self.app.models;
+
+        const isWorker = await models.Worker.findById(id);
+        if (isWorker)
+            throw new Error(`Can't change the password of another worker`);
+
+        await models.Account.setPassword(id, newPassword);
+    };
+};

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

@@ -0,0 +1,27 @@
+const models = require('vn-loopback/server/server').models;
+
+describe('Client setPassword', () => {
+    it('should throw an error the setPassword target is not just a client but a worker', async() => {
+        let error;
+
+        try {
+            await models.Client.setPassword(1106, 'newPass?');
+        } catch (e) {
+            error = e;
+        }
+
+        expect(error.message).toEqual(`Can't change the password of another worker`);
+    });
+
+    it('should change the password of the client', async() => {
+        let error;
+
+        try {
+            await models.Client.setPassword(1101, 't0pl3v3l.p455w0rd!');
+        } catch (e) {
+            error = e;
+        }
+
+        expect(error).toBeUndefined();
+    });
+});

modules/client/back/methods/client/specs/updateUser.spec.js

@@ -0,0 +1,58 @@
+const models = require('vn-loopback/server/server').models;
+const LoopBackContext = require('loopback-context');
+describe('Client updateUser', () => {
+    const employeeId = 1;
+    const activeCtx = {
+        accessToken: {userId: employeeId},
+        http: {
+            req: {
+                headers: {origin: 'http://localhost'}
+            }
+        }
+    };
+    const ctx = {
+        req: {accessToken: {userId: employeeId}},
+        args: {name: 'test', active: true}
+    };
+
+    beforeEach(() => {
+        spyOn(LoopBackContext, 'getCurrentContext').and.returnValue({
+            active: activeCtx
+        });
+    });
+
+    it('should throw an error the target user is not just a client but a worker', async() => {
+        let error;
+        try {
+            const clientID = 1106;
+            await models.Client.updateUser(ctx, clientID);
+        } catch (e) {
+            error = e;
+        }
+
+        expect(error.message).toEqual(`Can't update the user details of another worker`);
+    });
+
+    it('should update the user data', async() => {
+        let error;
+
+        const tx = await models.Client.beginTransaction({});
+
+        try {
+            const options = {transaction: tx};
+
+            const clientID = 1105;
+            await models.Client.updateUser(ctx, clientID, options);
+            const client = await models.Account.findById(clientID, null, options);
+
+            expect(client.name).toEqual('test');
+
+            await tx.rollback();
+        } catch (e) {
+            await tx.rollback();
+            throw e;
+        }
+
+        expect(error).toBeUndefined();
+    });
+});

modules/client/back/methods/client/updateUser.js

@@ -0,0 +1,56 @@
+module.exports = Self => {
+    Self.remoteMethodCtx('updateUser', {
+        description: 'Updates the user information',
+        accepts: [
+            {
+                arg: 'id',
+                type: 'number',
+                description: 'The user id',
+                http: {source: 'path'}
+            },
+            {
+                arg: 'name',
+                type: 'string',
+                description: 'the user name'
+            },
+            {
+                arg: 'active',
+                type: 'boolean',
+                description: 'whether the user is active or not'
+            },
+        ],
+        http: {
+            path: '/:id/updateUser',
+            verb: 'PATCH'
+        }
+    });
+
+    Self.updateUser = async function(ctx, id, options) {
+        const models = Self.app.models;
+        let tx;
+        const myOptions = {};
+
+        if (typeof options == 'object')
+            Object.assign(myOptions, options);
+
+        if (!myOptions.transaction) {
+            tx = await models.Account.beginTransaction({});
+            myOptions.transaction = tx;
+        }
+
+        try {
+            const isWorker = await models.Worker.findById(id, null, myOptions);
+            if (isWorker)
+                throw new Error(`Can't update the user details of another worker`);
+
+            const user = await models.Account.findById(id, null, myOptions);
+
+            await user.updateAttributes(ctx.args, myOptions);
+
+            if (tx) await tx.commit();
+        } catch (e) {
+            if (tx) await tx.rollback();
+            throw e;
+        }
+    };
+};

modules/client/back/models/client.js

@@ -8,30 +8,32 @@ const LoopBackContext = require('loopback-context');
 
 module.exports = Self => {
     // Methods
-    require('../methods/client/getCard')(Self);
-    require('../methods/client/createWithUser')(Self);
-    require('../methods/client/hasCustomerRole')(Self);
-    require('../methods/client/canCreateTicket')(Self);
-    require('../methods/client/isValidClient')(Self);
     require('../methods/client/addressesPropagateRe')(Self);
+    require('../methods/client/canBeInvoiced')(Self);
+    require('../methods/client/canCreateTicket')(Self);
+    require('../methods/client/checkDuplicated')(Self);
+    require('../methods/client/confirmTransaction')(Self);
+    require('../methods/client/consumption')(Self);
+    require('../methods/client/createAddress')(Self);
+    require('../methods/client/createReceipt')(Self);
+    require('../methods/client/createWithUser')(Self);
+    require('../methods/client/extendedListFilter')(Self);
+    require('../methods/client/getAverageInvoiced')(Self);
+    require('../methods/client/getCard')(Self);
     require('../methods/client/getDebt')(Self);
     require('../methods/client/getMana')(Self);
-    require('../methods/client/getAverageInvoiced')(Self);
-    require('../methods/client/summary')(Self);
-    require('../methods/client/updateFiscalData')(Self);
     require('../methods/client/getTransactions')(Self);
-    require('../methods/client/confirmTransaction')(Self);
-    require('../methods/client/canBeInvoiced')(Self);
-    require('../methods/client/uploadFile')(Self);
+    require('../methods/client/hasCustomerRole')(Self);
+    require('../methods/client/isValidClient')(Self);
     require('../methods/client/lastActiveTickets')(Self);
     require('../methods/client/sendSms')(Self);
-    require('../methods/client/createAddress')(Self);
+    require('../methods/client/setPassword')(Self);
+    require('../methods/client/summary')(Self);
     require('../methods/client/updateAddress')(Self);
-    require('../methods/client/consumption')(Self);
-    require('../methods/client/createReceipt')(Self);
+    require('../methods/client/updateFiscalData')(Self);
     require('../methods/client/updatePortfolio')(Self);
-    require('../methods/client/checkDuplicated')(Self);
-    require('../methods/client/extendedListFilter')(Self);
+    require('../methods/client/updateUser')(Self);
+    require('../methods/client/uploadFile')(Self);
 
     // Validations
 
@@ -446,7 +448,7 @@ module.exports = Self => {
 
     const app = require('vn-loopback/server/server');
     app.on('started', function() {
-        let account = app.models.Account;
+        const account = app.models.Account;
 
         account.observe('before save', async ctx => {
             if (ctx.isNewInstance) return;
@@ -456,20 +458,24 @@ module.exports = Self => {
         account.observe('after save', async ctx => {
             let changes = ctx.data || ctx.instance;
             if (!ctx.isNewInstance && changes) {
-                let oldData = ctx.hookState.oldInstance;
-                let hasChanges = oldData.name != changes.name || oldData.active != changes.active;
+                const oldData = ctx.hookState.oldInstance;
+                const hasChanges = oldData.name != changes.name || oldData.active != changes.active;
                 if (!hasChanges) return;
 
-                let userId = ctx.options.accessToken.userId;
-                let logRecord = {
-                    originFk: oldData.id,
-                    userFk: userId,
-                    action: 'update',
-                    changedModel: 'Account',
-                    oldInstance: {name: oldData.name, active: oldData.active},
-                    newInstance: {name: changes.name, active: changes.active}
-                };
-                await Self.app.models.ClientLog.create(logRecord);
+                const isClient = await Self.app.models.Client.count({id: oldData.id});
+                if (isClient) {
+                    const loopBackContext = LoopBackContext.getCurrentContext();
+                    const userId = loopBackContext.active.accessToken.userId;
+                    const logRecord = {
+                        originFk: oldData.id,
+                        userFk: userId,
+                        action: 'update',
+                        changedModel: 'Account',
+                        oldInstance: {name: oldData.name, active: oldData.active},
+                        newInstance: {name: changes.name, active: changes.active}
+                    };
+                    await Self.app.models.ClientLog.create(logRecord);
+                }
             }
         });
     });

modules/client/front/web-access/index.html

@@ -5,7 +5,7 @@
     data="$ctrl.account"
     form="form">
 </vn-watcher>
-<form name="form" ng-submit="watcher.submit()" class="vn-w-md">
+<form name="form" ng-submit="$ctrl.onSubmit()" class="vn-w-md">
     <vn-card class="vn-pa-lg">
         <vn-horizontal>
             <vn-check

modules/client/front/web-access/index.js

@@ -44,11 +44,11 @@ export default class Controller extends Section {
                 throw new Error(`You must enter a new password`);
             if (this.newPassword != this.repeatPassword)
                 throw new Error(`Passwords don't match`);
-            let account = {
-                password: this.newPassword
+            const data = {
+                newPassword: this.newPassword
             };
 
-            this.$http.patch(`Accounts/${this.client.id}`, account).then(res => {
+            this.$http.patch(`Clients/${this.client.id}/setPassword`, data).then(() => {
                 this.vnApp.showSuccess(this.$t('Data saved!'));
             });
         } catch (e) {
@@ -59,6 +59,17 @@ export default class Controller extends Section {
 
         return true;
     }
+
+    onSubmit() {
+        const data = {
+            name: this.account.name,
+            active: this.account.active
+        };
+        this.$http.patch(`Clients/${this.client.id}/updateUser`, data).then(() => {
+            this.$.watcher.notifySaved();
+            this.$.watcher.updateOriginalData();
+        });
+    }
 }
 Controller.$inject = ['$element', '$scope'];
 

modules/client/front/web-access/index.spec.js

@@ -52,7 +52,7 @@ describe('Component VnClientWebAccess', () => {
     });
 
     describe('checkConditions()', () => {
-        it(`should perform a query to check if the client is valid and then store a boolean into the controller`, () => {
+        it('should perform a query to check if the client is valid', () => {
             controller.client = {id: '1234'};
 
             expect(controller.canEnableCheckBox).toBeTruthy();
@@ -82,7 +82,9 @@ describe('Component VnClientWebAccess', () => {
             controller.newPassword = 'm24x8';
             controller.repeatPassword = 'm24x8';
             controller.canChangePassword = true;
-            $httpBackend.expectPATCH('Accounts/1234', {password: 'm24x8'}).respond('done');
+
+            const query = `Clients/${controller.client.id}/setPassword`;
+            $httpBackend.expectPATCH(query, {newPassword: controller.newPassword}).respond('done');
             controller.onPassChange();
             $httpBackend.flush();
         });