Fix issue with updating group roles

auth0 · Oct 8, 2024 · 913e8f4 · 913e8f4
1 parent 559087e
commit 913e8f4
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 5 deletions.
diff --git a/server/api/groups-roles/routes/patch_roles.js b/server/api/groups-roles/routes/patch_roles.js
@@ -1,4 +1,5 @@
 import Joi from 'joi';
+import _ from 'lodash';
 
 export default () => ({
   method: 'PATCH',
@@ -29,11 +30,9 @@ export default () => ({
       group.roles = [];
     }
 
-    const rolesToAdd = group.roles
-      ? roles.filter(roleId => !group.roles.includes(roleId))
-      : roles;
+    const newGroupRoles = _.uniq([ ...group.roles, ...roles ]);
 
-    await req.storage.updateGroup(req.params.id, { ...group, roles: rolesToAdd });
+    await req.storage.updateGroup(req.params.id, { ...group, roles: newGroupRoles });
 
     return h.response().code(204);
   }

diff --git a/tests/integration/group.tests.js b/tests/integration/group.tests.js
@@ -21,7 +21,7 @@ const parallelGroups = [ ...new Array(20) ].map(() => ({
   description: faker.lorem.sentence()
 }));
 
-describe('groups', () => {
+describe.only('groups', () => {
   before(async () => {
     accessToken = await getAccessToken();
 
@@ -418,6 +418,90 @@ describe('groups', () => {
       expect(res2.body.find((role) => role._id === role1._id)).toBeDefined();
     });
 
+    it('should add a single role to a group and not replace existing roles', async () => {
+      const group1 = await createGroup();
+      const role1 = await createRole();
+      const role2 = await createRole();
+      const role3 = await createRole();
+
+      const patchResult1 = await request
+        .patch(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .send([ role1._id, role2._id ])
+        .accept('json');
+
+      expect(patchResult1.statusCode).toEqual(204);
+
+      const getResult1 = await request
+        .get(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .accept('json');
+
+      expect(getResult1.body.length).toBe(2);
+      expect(getResult1.body.find((role) => role._id === role1._id)).toBeDefined();
+      expect(getResult1.body.find((role) => role._id === role2._id)).toBeDefined();
+
+      const patchResult2 = await request
+        .patch(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .send([ role3._id ])
+        .accept('json');
+
+      expect(patchResult2.statusCode).toEqual(204);
+
+      const getResult2 = await request
+        .get(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .accept('json');
+
+      expect(getResult2.body.length).toBe(3);
+      expect(getResult2.body.find((role) => role._id === role1._id)).toBeDefined();
+      expect(getResult2.body.find((role) => role._id === role2._id)).toBeDefined();
+      expect(getResult2.body.find((role) => role._id === role3._id)).toBeDefined();
+    });
+
+    it('should add multiple roles to a group and not duplicate existing roles', async () => {
+      const group1 = await createGroup();
+      const role1 = await createRole();
+      const role2 = await createRole();
+      const role3 = await createRole();
+
+      const patchResult1 = await request
+        .patch(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .send([ role1._id, role2._id ])
+        .accept('json');
+
+      expect(patchResult1.statusCode).toEqual(204);
+
+      const getResult1 = await request
+        .get(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .accept('json');
+
+      expect(getResult1.body.length).toBe(2);
+      expect(getResult1.body.find((role) => role._id === role1._id)).toBeDefined();
+      expect(getResult1.body.find((role) => role._id === role2._id)).toBeDefined();
+
+      const patchResult2 = await request
+        .patch(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .send([ role1._id, role2._id, role3._id ])
+        .accept('json');
+
+      expect(patchResult2.statusCode).toEqual(204);
+
+      const getResult2 = await request
+        .get(authzApi(`/groups/${group1._id}/roles`))
+        .auth(accessToken, { type: 'bearer' })
+        .accept('json');
+
+      expect(getResult2.body.length).toBe(3);
+      expect(getResult2.body.find((role) => role._id === role1._id)).toBeDefined();
+      expect(getResult2.body.find((role) => role._id === role2._id)).toBeDefined();
+      expect(getResult2.body.find((role) => role._id === role3._id)).toBeDefined();
+    });
+
     it('should get the roles of a group', async () => {
       const res = await request
       .get(authzApi(`/groups/${newGroup._id}/roles`))