From aa51fa1722604e99d760a43d407b50bc5249d8e4 Mon Sep 17 00:00:00 2001 From: vicent Date: Wed, 5 Oct 2022 12:36:36 +0200 Subject: [PATCH 1/2] feat: check the client to edit is in vn.client and not in account.account --- loopback/locale/en.json | 3 +- loopback/locale/es.json | 7 +++-- .../client/back/methods/client/setPassword.js | 21 +++++++++---- .../methods/client/specs/setPassword.spec.js | 30 +++++++++++++++---- .../methods/client/specs/updateUser.spec.js | 24 +++++++++++++-- .../client/back/methods/client/updateUser.js | 23 +++++++++----- 6 files changed, 83 insertions(+), 25 deletions(-) diff --git a/loopback/locale/en.json b/loopback/locale/en.json index fdea4ad4f..4162761da 100644 --- a/loopback/locale/en.json +++ b/loopback/locale/en.json @@ -131,5 +131,6 @@ "Fichadas impares": "Odd signs", "Descanso diario 9h.": "Daily rest 9h.", "Descanso semanal 36h. / 72h.": "Weekly rest 36h. / 72h.", - "Password does not meet requirements": "Password does not meet requirements" + "Password does not meet requirements": "Password does not meet requirements", + "Not enough privileges to edit a client": "Not enough privileges to edit a client" } \ No newline at end of file diff --git a/loopback/locale/es.json b/loopback/locale/es.json index 327568685..349dbc5df 100644 --- a/loopback/locale/es.json +++ b/loopback/locale/es.json @@ -232,5 +232,8 @@ "Fichadas impares": "Fichadas impares", "Descanso diario 12h.": "Descanso diario 12h.", "Descanso semanal 36h. / 72h.": "Descanso semanal 36h. / 72h.", - "Dirección incorrecta": "Dirección incorrecta" -} + "Dirección incorrecta": "Dirección incorrecta", + "Modifiable user details only by an administrator": "Detalles de usuario modificables solo por un administrador", + "Modifiable password only via recovery or by an administrator": "Contraseña modificable solo a través de la recuperación o por un administrador", + "Not enough privileges to edit a client": "No tienes suficientes privilegios para editar un cliente" +} \ No newline at end of file diff --git a/modules/client/back/methods/client/setPassword.js b/modules/client/back/methods/client/setPassword.js index 19675d0e8..2f0ebca5b 100644 --- a/modules/client/back/methods/client/setPassword.js +++ b/modules/client/back/methods/client/setPassword.js @@ -1,5 +1,6 @@ +const UserError = require('vn-loopback/util/user-error'); module.exports = Self => { - Self.remoteMethod('setPassword', { + Self.remoteMethodCtx('setPassword', { description: 'Sets the password of a non-worker client', accepts: [ { @@ -20,13 +21,21 @@ module.exports = Self => { } }); - Self.setPassword = async function(id, newPassword) { + Self.setPassword = async function(ctx, id, newPassword) { const models = Self.app.models; + const userId = ctx.req.accessToken.userId; - const isWorker = await models.Worker.findById(id); - if (isWorker) - throw new Error(`Can't change the password of another worker`); + const isSalesPerson = await models.Account.hasRole(userId, 'salesPerson'); - await models.Account.setPassword(id, newPassword); + if (!isSalesPerson) + throw new UserError(`Not enough privileges to edit a client`); + + const isClient = await models.Client.findById(id, null); + const isUserAccount = await models.UserAccount.findById(id, null); + + if (isClient && !isUserAccount) + await models.Account.setPassword(id, newPassword); + else + throw new UserError(`Modifiable password only via recovery or by an administrator`); }; }; diff --git a/modules/client/back/methods/client/specs/setPassword.spec.js b/modules/client/back/methods/client/specs/setPassword.spec.js index e0de20249..13065ca12 100644 --- a/modules/client/back/methods/client/specs/setPassword.spec.js +++ b/modules/client/back/methods/client/specs/setPassword.spec.js @@ -1,23 +1,43 @@ 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; + const salesPersonId = 19; + const ctx = { + req: {accessToken: {userId: salesPersonId}} + }; + it(`should throw an error if you don't have enough permissions`, async() => { + let error; + const employeeId = 1; + const ctx = { + req: {accessToken: {userId: employeeId}} + }; try { - await models.Client.setPassword(1106, 'newPass?'); + await models.Client.setPassword(ctx, 1, 't0pl3v3l.p455w0rd!'); } catch (e) { error = e; } - expect(error.message).toEqual(`Can't change the password of another worker`); + expect(error.message).toEqual(`Not enough privileges to edit a client`); + }); + + it('should throw an error the setPassword target is not just a client but a worker', async() => { + let error; + + try { + await models.Client.setPassword(ctx, 1, 't0pl3v3l.p455w0rd!'); + } catch (e) { + error = e; + } + + expect(error.message).toEqual(`Modifiable password only via recovery or by an administrator`); }); it('should change the password of the client', async() => { let error; try { - await models.Client.setPassword(1101, 't0pl3v3l.p455w0rd!'); + await models.Client.setPassword(ctx, 1101, 't0pl3v3l.p455w0rd!'); } catch (e) { error = e; } diff --git a/modules/client/back/methods/client/specs/updateUser.spec.js b/modules/client/back/methods/client/specs/updateUser.spec.js index 4dc969906..2d7f7dce0 100644 --- a/modules/client/back/methods/client/specs/updateUser.spec.js +++ b/modules/client/back/methods/client/specs/updateUser.spec.js @@ -10,8 +10,9 @@ describe('Client updateUser', () => { } } }; + const salesPersonId = 19; const ctx = { - req: {accessToken: {userId: employeeId}}, + req: {accessToken: {userId: salesPersonId}}, args: {name: 'test', active: true} }; @@ -21,8 +22,13 @@ describe('Client updateUser', () => { }); }); - it('should throw an error the target user is not just a client but a worker', async() => { + it(`should throw an error if you don't have enough permissions`, async() => { let error; + const employeeId = 1; + const ctx = { + req: {accessToken: {userId: employeeId}}, + args: {name: 'test', active: true} + }; try { const clientID = 1106; await models.Client.updateUser(ctx, clientID); @@ -30,7 +36,19 @@ describe('Client updateUser', () => { error = e; } - expect(error.message).toEqual(`Can't update the user details of another worker`); + expect(error.message).toEqual(`Not enough privileges to edit a client`); + }); + + it('should throw an error the target user is not just a client but a worker', async() => { + let error; + try { + const clientID = 1; + await models.Client.updateUser(ctx, clientID); + } catch (e) { + error = e; + } + + expect(error.message).toEqual(`Modifiable user details only by an administrator`); }); it('should update the user data', async() => { diff --git a/modules/client/back/methods/client/updateUser.js b/modules/client/back/methods/client/updateUser.js index ef23b92fd..1db8cd6b6 100644 --- a/modules/client/back/methods/client/updateUser.js +++ b/modules/client/back/methods/client/updateUser.js @@ -1,3 +1,4 @@ +const UserError = require('vn-loopback/util/user-error'); module.exports = Self => { Self.remoteMethodCtx('updateUser', { description: 'Updates the user information', @@ -5,8 +6,7 @@ module.exports = Self => { { arg: 'id', type: 'number', - description: 'The user id', - http: {source: 'path'} + description: 'The user id' }, { arg: 'name', @@ -15,7 +15,7 @@ module.exports = Self => { }, { arg: 'email', - type: 'string', + type: 'any', description: 'the user email' }, { @@ -32,6 +32,7 @@ module.exports = Self => { Self.updateUser = async function(ctx, id, options) { const models = Self.app.models; + const userId = ctx.req.accessToken.userId; let tx; const myOptions = {}; @@ -44,13 +45,19 @@ module.exports = Self => { } 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 isSalesPerson = await models.Account.hasRole(userId, 'salesPerson', myOptions); - const user = await models.Account.findById(id, null, myOptions); + if (!isSalesPerson) + throw new UserError(`Not enough privileges to edit a client`); - await user.updateAttributes(ctx.args, myOptions); + const isClient = await models.Client.findById(id, null, myOptions); + const isUserAccount = await models.UserAccount.findById(id, null, myOptions); + + if (isClient && !isUserAccount) { + const user = await models.Account.findById(id, null, myOptions); + await user.updateAttributes(ctx.args, myOptions); + } else + throw new UserError(`Modifiable user details only by an administrator`); if (tx) await tx.commit(); } catch (e) { From 3e4fb87f6bc11a02bb063ee17ba180b537b1f4d5 Mon Sep 17 00:00:00 2001 From: vicent Date: Wed, 5 Oct 2022 13:09:06 +0200 Subject: [PATCH 2/2] fix: te2e --- e2e/paths/02-client/07_edit_web_access.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/paths/02-client/07_edit_web_access.spec.js b/e2e/paths/02-client/07_edit_web_access.spec.js index 8d8036373..3d9ccee62 100644 --- a/e2e/paths/02-client/07_edit_web_access.spec.js +++ b/e2e/paths/02-client/07_edit_web_access.spec.js @@ -8,7 +8,7 @@ describe('Client Edit web access path', () => { beforeAll(async() => { browser = await getBrowser(); page = browser.page; - await page.loginAndModule('employee', 'client'); + await page.loginAndModule('salesPerson', 'client'); await page.accessToSearchResult('max'); await page.accessToSection('client.card.webAccess'); });