Merge pull request 'MASTER_4073-user_hasGrant' (!1107) from 4073-user-hasGrant-master into master

Reviewed-on: #1107
Reviewed-by: Juan Ferrer <juan@verdnatura.es>

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,40 @@ 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);
+
+        const userToUpdate = await models.Account.findById(id, {
+            fields: ['id', 'name', 'hasGrant', 'roleFk', 'password'],
+            include: {
+                relation: 'role',
+                scope: {
+                    fields: ['name']
+                }
+            }
+        }, myOptions);
 
         if (!user.hasGrant)
-            throw new UserError(`You don't have enough privileges`);
+            throw new UserError(`You don't have grant privilege`);
+
+        const hasRoleFromUser = await models.Account.hasRole(userId, userToUpdate.role().name, myOptions);
+
+        if (!hasRoleFromUser)
+            throw new UserError(`You don't own the role and you can't assign it to another user`);
 
-        const userToUpdate = await models.Account.findById(id);
         if (hasGrant != null)
-            return await userToUpdate.updateAttribute('hasGrant', hasGrant, myOptions);
+            userToUpdate.hasGrant = hasGrant;
-        if (!roleFk) return;
 
-        const role = await models.Role.findById(roleFk, null, 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`);
+                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);
     };
 };

back/methods/account/specs/privileges.spec.js

@@ -4,7 +4,9 @@ describe('account privileges()', () => {
     const employeeId = 1;
     const developerId = 9;
     const sysadminId = 66;
-    const bruceWayneId = 1101;
+    const itBossId = 104;
+    const rootId = 100;
+    const clarkKent = 1103;
 
     it('should throw an error when user not has privileges', async() => {
         const ctx = {req: {accessToken: {userId: developerId}}};
@@ -22,7 +24,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() => {
@@ -33,12 +35,7 @@ describe('account privileges()', () => {
         try {
             const options = {transaction: tx};
 
-            const root = await models.Role.findOne({
+            await models.Account.privileges(ctx, employeeId, rootId, null, options);
-                where: {
-                    name: 'root'
-                }
-            }, options);
-            await models.Account.privileges(ctx, employeeId, root.id, null, options);
 
             await tx.rollback();
         } catch (e) {
@@ -46,7 +43,26 @@ 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 throw an error when user has privileges but not has the role from user', async() => {
+        const ctx = {req: {accessToken: {userId: sysadminId}}};
+        const tx = await models.Account.beginTransaction({});
+
+        let error;
+        try {
+            const options = {transaction: tx};
+
+            await models.Account.privileges(ctx, itBossId, developerId, null, options);
+
+            await tx.rollback();
+        } catch (e) {
+            error = e;
+            await tx.rollback();
+        }
+
+        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 +79,8 @@ describe('account privileges()', () => {
         let error;
         let result;
         try {
-            await models.Account.privileges(ctx, bruceWayneId, agency.id, null, options);
+            await models.Account.privileges(ctx, clarkKent, agency.id, null, options);
-            result = await models.Account.findById(bruceWayneId, null, options);
+            result = await models.Account.findById(clarkKent, null, options);
 
             await tx.rollback();
         } catch (e) {
@@ -84,8 +100,8 @@ describe('account privileges()', () => {
         let result;
         try {
             const options = {transaction: tx};
-            await models.Account.privileges(ctx, bruceWayneId, null, true, options);
+            await models.Account.privileges(ctx, clarkKent, null, true, options);
-            result = await models.Account.findById(bruceWayneId, null, options);
+            result = await models.Account.findById(clarkKent, null, options);
 
             await tx.rollback();
         } catch (e) {

back/models/account.json

@@ -102,6 +102,13 @@
 			"principalType": "ROLE",
 			"principalId": "$authenticated",
 			"permission": "ALLOW"
+		},
+		{
+			"property": "privileges",
+			"accessType": "*",
+			"principalType": "ROLE",
+			"principalId": "$authenticated",
+			"permission": "ALLOW"
 		}
 	]
 }

db/dump/fixtures.sql

@@ -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`)
+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'
+    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;
 

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!');
+    });
 });

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!');
+    });
 });

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);
 

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"
+	"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"
 }

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"
+	"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"
 }

modules/account/front/privileges/locale/es.yml

@@ -1,2 +1,2 @@
 Privileges: Privilegios
-Has grant: Tiene privilegios
+Has grant: Puede delegar privilegios

modules/client/back/models/client.js

@@ -425,6 +425,7 @@ module.exports = Self => {
 
         account.observe('before save', async ctx => {
             if (ctx.isNewInstance) return;
+            if (ctx.currentInstance)
                 ctx.hookState.oldInstance = JSON.parse(JSON.stringify(ctx.currentInstance));
         });
 
@@ -432,7 +433,11 @@ module.exports = Self => {
             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});

package.json

@@ -37,7 +37,7 @@
         "node-ssh": "^11.0.0",
         "object-diff": "0.0.4",
         "object.pick": "^1.3.0",
-        "puppeteer": "^18.0.5",
+        "puppeteer": "^19.0.0",
         "read-chunk": "^3.2.0",
         "require-yaml": "0.0.1",
         "sharp": "^0.27.1",