feat: remove unnecessary saved object get call (#214)

* feat: add in memory validate remove unnecessary saved object get call Signed-off-by: Lin Wang <wonglam@amazon.com> * refactor: renaming validateSavedObjectsACL and remove log flag Signed-off-by: Lin Wang <wonglam@amazon.com> --------- Signed-off-by: Lin Wang <wonglam@amazon.com>
ruanyl · Oct 11, 2023 · 2771ff9 · 2771ff9
1 parent b935e36
commit 2771ff9
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 37 deletions.
diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 import { i18n } from '@osd/i18n';
-import { OpenSearchDashboardsRequest } from '../../../../core/server';
+import { OpenSearchDashboardsRequest, Principals, SavedObject } from '../../../../core/server';
 import {
   ACL,
   TransformedPermission,
@@ -43,6 +43,54 @@ export class SavedObjectsPermissionControl {
   public async setup(getScopedClient: SavedObjectsServiceStart['getScopedClient']) {
     this._getScopedClient = getScopedClient;
   }
+
+  private logNotPermitted(
+    savedObjects: Array<Pick<SavedObject<unknown>, 'id' | 'type' | 'workspaces' | 'permissions'>>,
+    principals: Principals,
+    permissionModes: SavedObjectsPermissionModes
+  ) {
+    this.logger.debug(
+      `Authorization failed, principals: ${JSON.stringify(
+        principals
+      )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify(
+        savedObjects.map((savedObject) => ({
+          id: savedObject.id,
+          type: savedObject.type,
+          workspaces: savedObject.workspaces,
+          permissions: savedObject.permissions,
+        }))
+      )}`
+    );
+  }
+
+  public validateSavedObjectsACL(
+    savedObjects: Array<Pick<SavedObject<unknown>, 'id' | 'type' | 'workspaces' | 'permissions'>>,
+    principals: Principals,
+    permissionModes: SavedObjectsPermissionModes
+  ) {
+    const notPermittedSavedObjects: Array<Pick<
+      SavedObject<unknown>,
+      'id' | 'type' | 'workspaces' | 'permissions'
+    >> = [];
+    const hasAllPermission = savedObjects.every((savedObject) => {
+      // for object that doesn't contain ACL like config, return true
+      if (!savedObject.permissions) {
+        return true;
+      }
+
+      const aclInstance = new ACL(savedObject.permissions);
+      const hasPermission = aclInstance.hasPermission(permissionModes, principals);
+      if (!hasPermission) {
+        notPermittedSavedObjects.push(savedObject);
+      }
+      return hasPermission;
+    });
+    if (!hasAllPermission) {
+      this.logNotPermitted(notPermittedSavedObjects, principals, permissionModes);
+    }
+    return hasAllPermission;
+  }
+
   public async validate(
     request: OpenSearchDashboardsRequest,
     savedObject: SavedObjectsBulkGetObject,
@@ -81,39 +129,9 @@ export class SavedObjectsPermissionControl {
     }
 
     const principals = getPrincipalsFromRequest(request);
-    let savedObjectsBasicInfo: any[] = [];
-    const hasAllPermission = savedObjectsGet.every((item) => {
-      // for object that doesn't contain ACL like config, return true
-      if (!item.permissions) {
-        return true;
-      }
-      const aclInstance = new ACL(item.permissions);
-      const hasPermission = aclInstance.hasPermission(permissionModes, principals);
-      if (!hasPermission) {
-        savedObjectsBasicInfo = [
-          ...savedObjectsBasicInfo,
-          {
-            id: item.id,
-            type: item.type,
-            workspaces: item.workspaces,
-            permissions: item.permissions,
-          },
-        ];
-      }
-      return hasPermission;
-    });
-    if (!hasAllPermission) {
-      this.logger.debug(
-        `Authorization failed, principals: ${JSON.stringify(
-          principals
-        )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify(
-          savedObjectsBasicInfo
-        )}`
-      );
-    }
     return {
       success: true,
-      result: hasAllPermission,
+      result: this.validateSavedObjectsACL(savedObjectsGet, principals, permissionModes),
     };
   }
 

diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts
@@ -141,8 +141,6 @@ export class WorkspaceSavedObjectsClientWrapper {
     objectPermissionModes: WorkspacePermissionMode[],
     validateAllWorkspaces = true
   ) {
-    const { id, type } = savedObject;
-
     // Advanced settings have no permissions and workspaces, so we need to skip it.
     if (!savedObject.workspaces && !savedObject.permissions) {
       return true;
@@ -168,9 +166,9 @@ export class WorkspaceSavedObjectsClientWrapper {
     }
     // Check permission based on object's ACL(defined by permissions attribute)
     if (savedObject.permissions) {
-      hasPermission = await this.validateObjectsPermissions(
-        [{ type, id }],
-        request,
+      hasPermission = await this.permissionControl.validateSavedObjectsACL(
+        [savedObject],
+        getPrincipalsFromRequest(request),
         objectPermissionModes
       );
     }