diff --git a/back/methods/account/privileges.js b/back/methods/account/privileges.js index df421125e..6510779f2 100644 --- a/back/methods/account/privileges.js +++ b/back/methods/account/privileges.js @@ -29,6 +29,8 @@ module.exports = Self => { }); Self.privileges = async function(ctx, id, roleFk, hasGrant, options) { + if (!(hasGrant != null || roleFk)) return; + const models = Self.app.models; const userId = ctx.req.accessToken.userId; @@ -37,22 +39,26 @@ module.exports = Self => { if (typeof options == 'object') Object.assign(myOptions, options); - const user = await models.Account.findById(userId, null, myOptions); + const user = await models.Account.findById(userId, {fields: ['hasGrant']}, myOptions); if (!user.hasGrant) - throw new UserError(`You don't have enough privileges`); + throw new UserError(`You don't have grant privilege`); - const userToUpdate = await models.Account.findById(id); + const userToUpdate = await models.Account.findById(id, ['name', 'hasGrant', 'roleFk'], myOptions); if (hasGrant != null) - return await userToUpdate.updateAttribute('hasGrant', hasGrant, myOptions); - if (!roleFk) return; + userToUpdate.hasGrant = hasGrant; - const role = await models.Role.findById(roleFk, null, myOptions); - const hasRole = await models.Account.hasRole(userId, role.name, myOptions); + if (roleFk) { + const role = await models.Role.findById(roleFk, {fields: ['name']}, myOptions); + const hasRole = await models.Account.hasRole(userId, role.name, myOptions); - if (!hasRole) - throw new UserError(`You don't have enough privileges`); + if (!hasRole) + throw new UserError(`You don't own the role and you can't assign it to another user`); - await userToUpdate.updateAttribute('roleFk', roleFk, myOptions); + userToUpdate.roleFk = roleFk; + } + + await userToUpdate.save(userToUpdate); + await models.UserAccount.sync(userToUpdate.name); }; }; diff --git a/back/methods/account/specs/privileges.spec.js b/back/methods/account/specs/privileges.spec.js index 137c08671..959130e8b 100644 --- a/back/methods/account/specs/privileges.spec.js +++ b/back/methods/account/specs/privileges.spec.js @@ -4,7 +4,7 @@ describe('account privileges()', () => { const employeeId = 1; const developerId = 9; const sysadminId = 66; - const bruceWayneId = 1101; + const clarkKent = 1103; it('should throw an error when user not has privileges', async() => { const ctx = {req: {accessToken: {userId: developerId}}}; @@ -22,7 +22,7 @@ describe('account privileges()', () => { await tx.rollback(); } - expect(error.message).toContain(`You don't have enough privileges`); + expect(error.message).toContain(`You don't have grant privilege`); }); it('should throw an error when user has privileges but not has the role', async() => { @@ -46,7 +46,7 @@ describe('account privileges()', () => { await tx.rollback(); } - expect(error.message).toContain(`You don't have enough privileges`); + expect(error.message).toContain(`You don't own the role and you can't assign it to another user`); }); it('should change role', async() => { @@ -63,8 +63,8 @@ describe('account privileges()', () => { let error; let result; try { - await models.Account.privileges(ctx, bruceWayneId, agency.id, null, options); - result = await models.Account.findById(bruceWayneId, null, options); + await models.Account.privileges(ctx, clarkKent, agency.id, null, options); + result = await models.Account.findById(clarkKent, null, options); await tx.rollback(); } catch (e) { @@ -84,8 +84,8 @@ describe('account privileges()', () => { let result; try { const options = {transaction: tx}; - await models.Account.privileges(ctx, bruceWayneId, null, true, options); - result = await models.Account.findById(bruceWayneId, null, options); + await models.Account.privileges(ctx, clarkKent, null, true, options); + result = await models.Account.findById(clarkKent, null, options); await tx.rollback(); } catch (e) { diff --git a/db/changes/10490-august/00-user_hasGrant.sql b/db/changes/10490-august/00-user_hasGrant.sql index 60d1273d8..05a09f87b 100644 --- a/db/changes/10490-august/00-user_hasGrant.sql +++ b/db/changes/10490-august/00-user_hasGrant.sql @@ -1 +1,4 @@ ALTER TABLE `account`.`user` ADD hasGrant TINYINT(1) NOT NULL; + +INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId) + VALUES('Account', 'privileges', '*', 'ALLOW', 'ROLE', '$authenticated'); diff --git a/db/dump/fixtures.sql b/db/dump/fixtures.sql index 7e59c1a54..c4079adc0 100644 --- a/db/dump/fixtures.sql +++ b/db/dump/fixtures.sql @@ -14,10 +14,10 @@ INSERT INTO `salix`.`AccessToken` (`id`, `ttl`, `created`, `userId`) ('DEFAULT_TOKEN', '1209600', util.VN_CURDATE(), 66); INSERT INTO `salix`.`printConfig` (`id`, `itRecipient`, `incidencesEmail`) - VALUES + VALUES (1, 'it@gotamcity.com', 'incidences@gotamcity.com'); -INSERT INTO `vn`.`ticketConfig` (`id`, `scopeDays`) +INSERT INTO `vn`.`ticketConfig` (`id`, `scopeDays`) VALUES ('1', '6'); @@ -45,8 +45,8 @@ INSERT INTO `account`.`roleConfig`(`id`, `mysqlPassword`, `rolePrefix`, `userPre CALL `account`.`role_sync`; -INSERT INTO `account`.`user`(`id`,`name`, `nickname`, `password`,`role`,`active`,`email`, `lang`, `image`) - SELECT id, name, CONCAT(name, 'Nick'),MD5('nightmare'), id, 1, CONCAT(name, '@mydomain.com'), 'en', '4fa3ada0-3ac4-11eb-9ab8-27f6fc3b85fd' +INSERT INTO `account`.`user`(`id`,`name`, `nickname`, `password`,`role`,`active`,`email`, `lang`, `image`, `bcryptPassword`) + SELECT id, name, CONCAT(name, 'Nick'),MD5('nightmare'), id, 1, CONCAT(name, '@mydomain.com'), 'en', '4fa3ada0-3ac4-11eb-9ab8-27f6fc3b85fd', '$2b$10$UzQHth.9UUQ1T5aiQJ21lOU0oVlbxoqH4PFM9V8T90KNSAcg0eEL2' FROM `account`.`role` WHERE id <> 20 ORDER BY id; diff --git a/e2e/paths/14-account/07_ldap.spec.js b/e2e/paths/14-account/07_ldap.spec.js index a3b8137d3..eb22f695c 100644 --- a/e2e/paths/14-account/07_ldap.spec.js +++ b/e2e/paths/14-account/07_ldap.spec.js @@ -29,4 +29,13 @@ describe('Account LDAP path', () => { expect(message.text).toContain('Data saved!'); }); + + it('should reset data', async() => { + await page.waitToClick(selectors.accountLdap.checkEnable); + await page.waitToClick(selectors.accountLdap.save); + + const message = await page.waitForSnackbar(); + + expect(message.text).toContain('Data saved!'); + }); }); diff --git a/e2e/paths/14-account/08_samba.spec.js b/e2e/paths/14-account/08_samba.spec.js index c3db026dc..6e7ef9bbf 100644 --- a/e2e/paths/14-account/08_samba.spec.js +++ b/e2e/paths/14-account/08_samba.spec.js @@ -29,4 +29,13 @@ describe('Account Samba path', () => { expect(message.text).toContain('Data saved!'); }); + + it('should reset data', async() => { + await page.waitToClick(selectors.accountSamba.checkEnable); + await page.waitToClick(selectors.accountSamba.save); + + const message = await page.waitForSnackbar(); + + expect(message.text).toContain('Data saved!'); + }); }); diff --git a/e2e/paths/14-account/09_privileges.spec.js b/e2e/paths/14-account/09_privileges.spec.js index 71e9345a8..e4b8fb24c 100644 --- a/e2e/paths/14-account/09_privileges.spec.js +++ b/e2e/paths/14-account/09_privileges.spec.js @@ -24,7 +24,7 @@ describe('Account privileges path', () => { const message = await page.waitForSnackbar(); - expect(message.text).toContain(`You don't have enough privileges`); + expect(message.text).toContain(`You don't have grant privilege`); }); it('should throw error when change role', async() => { @@ -33,7 +33,7 @@ describe('Account privileges path', () => { const message = await page.waitForSnackbar(); - expect(message.text).toContain(`You don't have enough privileges`); + expect(message.text).toContain(`You don't have grant privilege`); }); }); @@ -56,7 +56,16 @@ describe('Account privileges path', () => { expect(result).toBe('checked'); }); - it('should change role', async() => { + it('should throw error when change role and not own role', async() => { + await page.autocompleteSearch(selectors.accountPrivileges.role, 'itBoss'); + await page.waitToClick(selectors.accountPrivileges.save); + + const message = await page.waitForSnackbar(); + + expect(message.text).toContain(`You don't own the role and you can't assign it to another user`); + }); + + it('should change role to employee', async() => { await page.autocompleteSearch(selectors.accountPrivileges.role, 'employee'); await page.waitToClick(selectors.accountPrivileges.save); const message = await page.waitForSnackbar(); @@ -67,6 +76,18 @@ describe('Account privileges path', () => { expect(message.text).toContain(`Data saved!`); expect(result).toContain('employee'); }); + + it('should return role to developer', async() => { + await page.autocompleteSearch(selectors.accountPrivileges.role, 'developer'); + await page.waitToClick(selectors.accountPrivileges.save); + const message = await page.waitForSnackbar(); + + await page.reloadSection('account.card.privileges'); + const result = await page.waitToGetProperty(selectors.accountPrivileges.role, 'value'); + + expect(message.text).toContain(`Data saved!`); + expect(result).toContain('developer'); + }); }); describe('as developer again', () => { @@ -76,7 +97,12 @@ describe('Account privileges path', () => { await page.waitToClick(selectors.accountPrivileges.checkHasGrant); await page.waitToClick(selectors.accountPrivileges.save); + const message = await page.waitForSnackbar(); + expect(message.text).toContain(`Data saved!`); + }); + + it('should logIn in developer', async() => { await page.reloadSection('account.card.privileges'); const result = await page.checkboxState(selectors.accountPrivileges.checkHasGrant); diff --git a/loopback/locale/en.json b/loopback/locale/en.json index e5a0fae32..1e151294f 100644 --- a/loopback/locale/en.json +++ b/loopback/locale/en.json @@ -133,5 +133,7 @@ "Descanso semanal 36h. / 72h.": "Weekly rest 36h. / 72h.", "Password does not meet requirements": "Password does not meet requirements", "You don't have privileges to change the zone": "You don't have privileges to change the zone or for these parameters there are more than one shipping options, talk to agencies", - "Not enough privileges to edit a client": "Not enough privileges to edit a client" -} \ No newline at end of file + "Not enough privileges to edit a client": "Not enough privileges to edit a client", + "You don't have grant privilege": "You don't have grant privilege", + "You don't own the role and you can't assign it to another user": "You don't own the role and you can't assign it to another user" +} diff --git a/loopback/locale/es.json b/loopback/locale/es.json index 67370b343..a41315dd1 100644 --- a/loopback/locale/es.json +++ b/loopback/locale/es.json @@ -235,5 +235,7 @@ "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 + "Not enough privileges to edit a client": "No tienes suficientes privilegios para editar un cliente", + "You don't have grant privilege": "No tienes privilegios para dar privilegios", + "You don't own the role and you can't assign it to another user": "No eres el propietario del rol y no puedes asignarlo a otro usuario" +} diff --git a/modules/account/front/privileges/locale/es.yml b/modules/account/front/privileges/locale/es.yml index f7330e1be..17f1ef29e 100644 --- a/modules/account/front/privileges/locale/es.yml +++ b/modules/account/front/privileges/locale/es.yml @@ -1,2 +1,2 @@ Privileges: Privilegios -Has grant: Tiene privilegios +Has grant: Puede dar privilegios diff --git a/modules/client/back/models/client.js b/modules/client/back/models/client.js index 3bd89eff1..e66cdb83f 100644 --- a/modules/client/back/models/client.js +++ b/modules/client/back/models/client.js @@ -425,14 +425,19 @@ module.exports = Self => { account.observe('before save', async ctx => { if (ctx.isNewInstance) return; - ctx.hookState.oldInstance = JSON.parse(JSON.stringify(ctx.currentInstance)); + if (ctx.currentInstance) + ctx.hookState.oldInstance = JSON.parse(JSON.stringify(ctx.currentInstance)); }); account.observe('after save', async ctx => { const changes = ctx.data || ctx.instance; if (!ctx.isNewInstance && changes) { const oldData = ctx.hookState.oldInstance; - const hasChanges = oldData.name != changes.name || oldData.active != changes.active; + let hasChanges; + + if (oldData) + hasChanges = oldData.name != changes.name || oldData.active != changes.active; + if (!hasChanges) return; const isClient = await Self.app.models.Client.count({id: oldData.id});