refactor: simplified ACL transformPermission function (opensearch-pro…

…ject#78) refactor: simplified ACL transformPermission function --------- Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
SuZhou-Joe · Aug 9, 2023 · d4e27e5 · d4e27e5
1 parent 7715023
commit d4e27e5
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 54 deletions.
diff --git a/src/core/server/saved_objects/permission_control/acl.test.ts b/src/core/server/saved_objects/permission_control/acl.test.ts
@@ -107,8 +107,17 @@ describe('SavedObjectTypeRegistry', () => {
       write: principals,
     };
     acl = new ACL(permissions);
-    const result = acl.transformPermissions();
-    expect(result?.length).toEqual(3);
+    const result = acl.toFlatList();
+    expect(result).toHaveLength(3);
+    expect(result).toEqual(
+      expect.arrayContaining([{ type: 'users', name: 'user1', permissions: ['read', 'write'] }])
+    );
+    expect(result).toEqual(
+      expect.arrayContaining([{ type: 'groups', name: 'group1', permissions: ['read', 'write'] }])
+    );
+    expect(result).toEqual(
+      expect.arrayContaining([{ type: 'groups', name: 'group2', permissions: ['read', 'write'] }])
+    );
   });
 
   it('test generate query DSL', () => {

diff --git a/src/core/server/saved_objects/permission_control/acl.ts b/src/core/server/saved_objects/permission_control/acl.ts
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { PrincipalType } from '../../../../core/utils/constants';
+import { PrincipalType } from '../../../utils/constants';
 
 export interface Principals {
   users?: string[];
@@ -111,7 +111,7 @@ export class ACL {
     return this;
   }
 
-  // permissions object build funciton, remove specific permission of specific principal from the object
+  // permissions object build function, remove specific permission of specific principal from the object
   public removePermission(permissionTypes: string[], principals: Principals) {
     if (!permissionTypes || !principals) {
       return this;
@@ -134,62 +134,50 @@ export class ACL {
     return this;
   }
 
-  /* 
-        transfrom permissions format
-        original permissions:   {
-            read: {
-                users:['user1']
-            },
-            write:{
-                groups:['group1']
-            }
-        }
-    
-        transformed permissions: [
-            {type:'users',name:'user1',permissions:['read']},
-            {type:'groups',name:'group1',permissions:['write']},
-        ]
-        */
-  public transformPermissions(): TransformedPermission[] {
+  /**
+   * transform permissions format
+   * original permissions:   {
+   *     read: {
+   *         users:['user1']
+   *     },
+   *     write:{
+   *         groups:['group1']
+   *     }
+   * }
+   *
+   * transformed permissions: [
+   *     {type:'users',name:'user1',permissions:['read']},
+   *     {type:'groups',name:'group1',permissions:['write']},
+   * ]
+   */
+  public toFlatList(): TransformedPermission[] {
     const result: TransformedPermission[] = [];
     if (!this.permissions) {
       return result;
     }
 
-    const permissionMapResult: Record<string, Record<string, string[]>> = {};
-    const principalTypes = [PrincipalType.Users, PrincipalType.Groups];
     for (const permissionType in this.permissions) {
-      if (!!permissionType) {
-        const value = this.permissions[permissionType];
-        principalTypes.forEach((principalType) => {
-          if (value?.[principalType]) {
-            for (const principal of value[principalType]!) {
-              if (!permissionMapResult[principalType]) {
-                permissionMapResult[principalType] = {};
-              }
-              if (!permissionMapResult[principalType][principal]) {
-                permissionMapResult[principalType][principal] = [];
-              }
-              permissionMapResult[principalType][principal] = [
-                ...permissionMapResult[principalType][principal]!,
-                permissionType,
-              ];
-            }
+      if (Object.prototype.hasOwnProperty.call(this.permissions, permissionType)) {
+        const { users = [], groups = [] } = this.permissions[permissionType] ?? {};
+        users.forEach((user) => {
+          const found = result.find((r) => r.type === PrincipalType.Users && r.name === user);
+          if (found) {
+            found.permissions.push(permissionType);
+          } else {
+            result.push({ type: PrincipalType.Users, name: user, permissions: [permissionType] });
+          }
+        });
+        groups.forEach((group) => {
+          const found = result.find((r) => r.type === PrincipalType.Groups && r.name === group);
+          if (found) {
+            found.permissions.push(permissionType);
+          } else {
+            result.push({ type: PrincipalType.Groups, name: group, permissions: [permissionType] });
           }
         });
       }
     }
 
-    Object.entries(permissionMapResult).forEach(([type, permissionMap]) => {
-      Object.entries(permissionMap).forEach(([principal, permissions]) => {
-        result.push({
-          type,
-          name: principal,
-          permissions,
-        });
-      });
-    });
-
     return result;
   }
 
@@ -203,9 +191,9 @@ export class ACL {
     return this.permissions;
   }
 
-  /*
-        generate query DSL by the specific conditions, used for fetching saved objects from the saved objects index
-        */
+  /**
+   * generate query DSL by the specific conditions, used for fetching saved objects from the saved objects index
+   */
   public static genereateGetPermittedSavedObjectsQueryDSL(
     permissionTypes: string[],
     principals: Principals,

diff --git a/src/core/server/saved_objects/permission_control/client.ts b/src/core/server/saved_objects/permission_control/client.ts
@@ -116,7 +116,7 @@ export class SavedObjectsPermissionControl {
     return detailedSavedObjects.reduce((total, current) => {
       return {
         ...total,
-        [current.id]: new ACL(current.permissions).transformPermissions(),
+        [current.id]: new ACL(current.permissions).toFlatList(),
       };
     }, {});
   }

diff --git a/src/core/server/workspaces/routes/index.ts b/src/core/server/workspaces/routes/index.ts
@@ -67,7 +67,7 @@ const convertToACL = (
 const convertFromACL = (permissions: Permissions) => {
   const acl = new ACL(permissions);
 
-  return acl.transformPermissions().map(({ name, permissions: modes, type }) => ({
+  return acl.toFlatList().map(({ name, permissions: modes, type }) => ({
     type: type === 'users' ? 'user' : 'group',
     modes,
     ...{ [type === 'users' ? 'userId' : 'group']: name },