feat: add in memory validate remove unnecessary saved object get call

Signed-off-by: Lin Wang <[email protected]>
wanglam · Oct 9, 2023 · d9b2a22 · d9b2a22
1 parent 354d1af
commit d9b2a22
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 32 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,59 @@ export class SavedObjectsPermissionControl {
   public async setup(getScopedClient: SavedObjectsServiceStart['getScopedClient']) {
     this._getScopedClient = getScopedClient;
   }
+
+  private convertToSavedObjectsBasicInfo(
+    savedObject: Pick<SavedObject<unknown>, 'id' | 'type' | 'workspaces' | 'permissions'>
+  ) {
+    return {
+      id: savedObject.id,
+      type: savedObject.type,
+      workspaces: savedObject.workspaces,
+      permissions: savedObject.permissions,
+    };
+  }
+
+  private logNotPermitted(
+    savedObjectsOrSavedObject:
+      | Array<Pick<SavedObject<unknown>, 'id' | 'type' | 'workspaces' | 'permissions'>>
+      | 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(
+        Array.isArray(savedObjectsOrSavedObject)
+          ? savedObjectsOrSavedObject.map(this.convertToSavedObjectsBasicInfo)
+          : this.convertToSavedObjectsBasicInfo(savedObjectsOrSavedObject)
+      )}`
+    );
+  }
+
+  public inMemoryValidate({
+    savedObject,
+    principals,
+    permissionModes,
+    shouldLogNotPermitted = true,
+  }: {
+    savedObject: Pick<SavedObject<unknown>, 'id' | 'type' | 'workspaces' | 'permissions'>;
+    principals: Principals;
+    permissionModes: SavedObjectsPermissionModes;
+    shouldLogNotPermitted?: boolean;
+  }) {
+    // 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 && shouldLogNotPermitted) {
+      this.logNotPermitted(savedObject, principals, permissionModes);
+    }
+    return hasPermission;
+  }
+
   public async validate(
     request: OpenSearchDashboardsRequest,
     savedObject: SavedObjectsBulkGetObject,
@@ -81,35 +134,21 @@ 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);
+    const notPermittedSavedObjects: Array<SavedObject<unknown>> = [];
+    const hasAllPermission = savedObjectsGet.every((savedObject) => {
+      const hasPermission = this.inMemoryValidate({
+        savedObject,
+        permissionModes,
+        principals,
+        shouldLogNotPermitted: false,
+      });
       if (!hasPermission) {
-        savedObjectsBasicInfo = [
-          ...savedObjectsBasicInfo,
-          {
-            id: item.id,
-            type: item.type,
-            workspaces: item.workspaces,
-            permissions: item.permissions,
-          },
-        ];
+        notPermittedSavedObjects.push(savedObject);
       }
       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
-        )}`
-      );
+      this.logNotPermitted(notPermittedSavedObjects, principals, permissionModes);
     }
     return {
       success: true,

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;
@@ -167,11 +165,11 @@ export class WorkspaceSavedObjectsClientWrapper {
     }
     // Check permission based on object's ACL(defined by permissions attribute)
     if (savedObject.permissions) {
-      hasPermission = await this.validateObjectsPermissions(
-        [{ type, id }],
-        request,
-        objectPermissionModes
-      );
+      hasPermission = await this.permissionControl.inMemoryValidate({
+        savedObject,
+        principals: getPrincipalsFromRequest(request),
+        permissionModes: objectPermissionModes,
+      });
     }
     return hasPermission;
   }