From 194262a93e2fdcd86d14fb0f0bed89d3872d539e Mon Sep 17 00:00:00 2001 From: Juan Ferrer Toribio Date: Mon, 29 Jan 2024 13:48:02 +0100 Subject: [PATCH] fix: refs #6085 ACL canEditAlias checked, migrated to remote hooks --- back/models/specs/mailAliasAccount.spec.js | 17 ------ .../240404/00-revokeMailAliasAccount.sql | 5 ++ gulpfile.js | 4 +- loopback/locale/en.json | 6 ++- loopback/locale/es.json | 2 +- .../account/back/models/mail-alias-account.js | 52 +++++++++---------- 6 files changed, 38 insertions(+), 48 deletions(-) create mode 100644 db/changes/240404/00-revokeMailAliasAccount.sql diff --git a/back/models/specs/mailAliasAccount.spec.js b/back/models/specs/mailAliasAccount.spec.js index c13cc7ae8..8f0278a50 100644 --- a/back/models/specs/mailAliasAccount.spec.js +++ b/back/models/specs/mailAliasAccount.spec.js @@ -1,23 +1,6 @@ const models = require('vn-loopback/server/server').models; describe('loopback model MailAliasAccount', () => { - it('should fail to add a mail Alias if the worker doesnt have ACLs', async() => { - const tx = await models.MailAliasAccount.beginTransaction({}); - let error; - - try { - const options = {transaction: tx, accessToken: {userId: 57}}; - await models.MailAliasAccount.create({mailAlias: 2, account: 5}, options); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - error = e; - } - - expect(error.message).toEqual('The alias cant be modified'); - }); - it('should add a mail Alias', async() => { const tx = await models.MailAliasAccount.beginTransaction({}); let error; diff --git a/db/changes/240404/00-revokeMailAliasAccount.sql b/db/changes/240404/00-revokeMailAliasAccount.sql new file mode 100644 index 000000000..a86de1f3c --- /dev/null +++ b/db/changes/240404/00-revokeMailAliasAccount.sql @@ -0,0 +1,5 @@ +DELETE FROM salix.ACL + WHERE model = 'MailAliasAccount' + AND property = 'canEditAlias' + AND principalType = 'ROLE' + AND principalId = 'marketingBoss'; diff --git a/gulpfile.js b/gulpfile.js index d7e7d8e86..8614ed1b4 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -234,13 +234,13 @@ async function dockerStart() { const container = new Docker('salix-db'); await container.start(); } -dockerStart.description = `Starts the salix-db container`; +dockerStart.description = `Starts the DB container`; async function docker() { const container = new Docker('salix-db'); await container.run(); } -docker.description = `Runs the salix-db container`; +docker.description = `Builds and starts the DB container`; module.exports = { default: defaultTask, diff --git a/loopback/locale/en.json b/loopback/locale/en.json index 568916bef..fb287d185 100644 --- a/loopback/locale/en.json +++ b/loopback/locale/en.json @@ -203,5 +203,7 @@ "Cannot past travels with entries": "Cannot past travels with entries", "It was not able to remove the next expeditions:": "It was not able to remove the next expeditions: {{expeditions}}", "Incorrect pin": "Incorrect pin.", - "The notification subscription of this worker cant be modified": "The notification subscription of this worker cant be modified" -} \ No newline at end of file + "The notification subscription of this worker cant be modified": "The notification subscription of this worker cant be modified", + "You are not allowed to modify the alias": "You are not allowed to modify the alias", + "You already have the mailAlias": "You already have the mailAlias" +} diff --git a/loopback/locale/es.json b/loopback/locale/es.json index 5555ef8b0..9163c494f 100644 --- a/loopback/locale/es.json +++ b/loopback/locale/es.json @@ -335,6 +335,6 @@ "This user does not have an assigned tablet": "Este usuario no tiene tablet asignada", "Incorrect pin": "Pin incorrecto.", "You already have the mailAlias": "Ya tienes este alias de correo", - "The alias cant be modified": "Este alias de correo no puede ser modificado", + "You are not allowed to modify the alias": "No estás autorizado a modificar el alias", "No tickets to invoice": "No hay tickets para facturar" } diff --git a/modules/account/back/models/mail-alias-account.js b/modules/account/back/models/mail-alias-account.js index 5eb561408..61ca344e9 100644 --- a/modules/account/back/models/mail-alias-account.js +++ b/modules/account/back/models/mail-alias-account.js @@ -1,5 +1,5 @@ -const UserError = require('vn-loopback/util/user-error'); +const ForbiddenError = require('vn-loopback/util/forbiddenError'); module.exports = Self => { Self.rewriteDbError(function(err) { @@ -8,38 +8,38 @@ module.exports = Self => { return err; }); - Self.observe('before save', async ctx => { - const changes = ctx.currentInstance || ctx.instance; - - await checkModifyPermission(ctx, changes.mailAlias); + Self.beforeRemote('create', async function(ctx) { + const mailAlias = ctx.args.data?.mailAlias; + if (!mailAlias) return; + await checkModifyPermission(ctx, mailAlias); }); - - Self.observe('before delete', async ctx => { - const mailAliasAccount = await Self.findById(ctx.where.id); - - await checkModifyPermission(ctx, mailAliasAccount.mailAlias); + Self.beforeRemote('deleteById', async function(ctx) { + const instance = await Self.findById(ctx.args.id, + {fields: ['mailAlias']} + ); + await checkModifyPermission(ctx, instance.mailAlias); }); async function checkModifyPermission(ctx, mailAliasFk) { - const userId = ctx.options.accessToken.userId; const models = Self.app.models; + const userId = ctx.req.accessToken.userId; - const roles = await models.RoleMapping.find({ - fields: ['roleId'], - where: {principalId: userId} + const canEditAlias = await models.ACL.checkAccessAcl(ctx, + 'MailAliasAccount', 'canEditAlias', 'WRITE'); + if (canEditAlias) return; + + const allowedRoles = await models.MailAliasAcl.find({ + fields: ['roleFk'], + where: {mailAliasFk} }); + const nRoles = allowedRoles.length && + await models.RoleMapping.count({ + principalId: userId, + principalType: 'USER', + roleId: {inq: allowedRoles.map(x => x.roleFk)} + }); - const availableMailAlias = await models.MailAliasAcl.findOne({ - fields: ['mailAliasFk'], - include: {relation: 'mailAlias'}, - where: { - roleFk: { - inq: roles.map(role => role.roleId), - }, - mailAliasFk - } - }); - - if (!availableMailAlias) throw new UserError('The alias cant be modified'); + if (!nRoles) + throw new ForbiddenError('You are not allowed to modify the alias'); } };