feat: skip permission validate when no workspaces and permissions att…

…ributes (#163) (#177)

* feat: skip permission validate when saved object without workspaces and permissions attributes



* feat: add annontation to skip permission check



* refactor: remove bind and simplify validate logic



* feat: remove library write for object based ACL



---------


(cherry picked from commit cfa9c4b)

Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

@@ -23,12 +23,12 @@ import {
   SavedObjectsBulkUpdateObject,
   SavedObjectsBulkUpdateResponse,
   SavedObjectsBulkUpdateOptions,
-  SavedObjectsPermissionControlContract,
   WORKSPACE_TYPE,
   WorkspacePermissionMode,
   SavedObjectsDeleteByWorkspaceOptions,
   SavedObjectsErrorHelpers,
 } from '../../../../core/server';
+import { SavedObjectsPermissionControlContract } from '../permission_control/client';
 import { WorkspaceFindOptions } from '../types';
 
 // Can't throw unauthorized for now, the page will be refreshed if unauthorized
@@ -86,24 +86,24 @@ export class WorkspaceSavedObjectsClientWrapper {
   }
 
   // validate if the `request` has the specified permission(`permissionMode`) to the given `workspaceIds`
-  private async validateMultiWorkspacesPermissions(
+  private validateMultiWorkspacesPermissions = async (
     workspacesIds: string[],
     request: OpenSearchDashboardsRequest,
     permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[]
-  ) {
+  ) => {
     // for attributes and options passed in this function, the num of workspaces may be 0.This case should not be passed permission check.
     if (workspacesIds.length === 0) {
       return false;
     }
     const workspaces = workspacesIds.map((id) => ({ id, type: WORKSPACE_TYPE }));
     return await this.validateObjectsPermissions(workspaces, request, permissionMode);
-  }
+  };
 
-  private async validateAtLeastOnePermittedWorkspaces(
+  private validateAtLeastOnePermittedWorkspaces = async (
     workspaces: string[] | undefined,
     request: OpenSearchDashboardsRequest,
     permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[]
-  ) {
+  ) => {
     // for attributes and options passed in this function, the num of workspaces attribute may be 0.This case should not be passed permission check.
     if (!workspaces || workspaces.length === 0) {
       return false;
@@ -122,7 +122,7 @@ export class WorkspaceSavedObjectsClientWrapper {
       }
     }
     return false;
-  }
+  };
 
   /**
    * check if the type include workspace
@@ -134,32 +134,64 @@ export class WorkspaceSavedObjectsClientWrapper {
     return type === WORKSPACE_TYPE || (Array.isArray(type) && type.includes(WORKSPACE_TYPE));
   }
 
+  private async validateWorkspacesAndSavedObjectsPermissions(
+    savedObject: Pick<SavedObject, 'id' | 'type' | 'workspaces' | 'permissions'>,
+    request: OpenSearchDashboardsRequest,
+    workspacePermissionModes: WorkspacePermissionMode[],
+    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;
+    }
+
+    let hasPermission = false;
+    // Check permission based on object's workspaces
+    if (savedObject.workspaces) {
+      const workspacePermissionValidator = validateAllWorkspaces
+        ? this.validateMultiWorkspacesPermissions
+        : this.validateAtLeastOnePermittedWorkspaces;
+      hasPermission = await workspacePermissionValidator(
+        savedObject.workspaces,
+        request,
+        workspacePermissionModes
+      );
+    }
+    // If already has permissions based on workspaces, we don't need to check object's ACL(defined by permissions attribute)
+    // So return true immediately
+    if (hasPermission) {
+      return true;
+    }
+    // Check permission based on object's ACL(defined by permissions attribute)
+    if (savedObject.permissions) {
+      hasPermission = await this.validateObjectsPermissions(
+        [{ type, id }],
+        request,
+        objectPermissionModes
+      );
+    }
+    return hasPermission;
+  }
+
   public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => {
     const deleteWithWorkspacePermissionControl = async (
       type: string,
       id: string,
       options: SavedObjectsDeleteOptions = {}
     ) => {
       const objectToDeleted = await wrapperOptions.client.get(type, id, options);
-      const workspacePermitted = await this.validateMultiWorkspacesPermissions(
-        objectToDeleted.workspaces ?? [],
-        wrapperOptions.request,
-        [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management]
-      );
-
-      if (!workspacePermitted) {
-        const objectsPermitted = await this.validateObjectsPermissions(
-          [{ type, id }],
+      if (
+        !(await this.validateWorkspacesAndSavedObjectsPermissions(
+          objectToDeleted,
           wrapperOptions.request,
-          [
-            WorkspacePermissionMode.Management,
-            WorkspacePermissionMode.LibraryWrite,
-            WorkspacePermissionMode.Write,
-          ]
-        );
-        if (!objectsPermitted) {
-          throw generateSavedObjectsPermissionError();
-        }
+          [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management],
+          [WorkspacePermissionMode.Management, WorkspacePermissionMode.Write]
+        ))
+      ) {
+        throw generateSavedObjectsPermissionError();
       }
       return await wrapperOptions.client.delete(type, id, options);
     };
@@ -168,31 +200,13 @@ export class WorkspaceSavedObjectsClientWrapper {
     const validateUpdateWithWorkspacePermission = async <T = unknown>(
       objectToUpdate: SavedObject<T>
     ): Promise<boolean> => {
-      let workspacePermitted = false;
-      if (objectToUpdate.workspaces && objectToUpdate.workspaces.length > 0) {
-        workspacePermitted =
-          (await this.validateAtLeastOnePermittedWorkspaces(
-            objectToUpdate.workspaces,
-            wrapperOptions.request,
-            [WorkspacePermissionMode.Management, WorkspacePermissionMode.LibraryWrite]
-          )) ?? false;
-      }
-
-      if (workspacePermitted) {
-        return true;
-      } else {
-        const { id, type } = objectToUpdate;
-        const objectsPermitted = await this.validateObjectsPermissions(
-          [{ id, type }],
-          wrapperOptions.request,
-          [
-            WorkspacePermissionMode.Management,
-            WorkspacePermissionMode.LibraryWrite,
-            WorkspacePermissionMode.Write,
-          ]
-        );
-        return objectsPermitted ?? false;
-      }
+      return await this.validateWorkspacesAndSavedObjectsPermissions(
+        objectToUpdate,
+        wrapperOptions.request,
+        [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management],
+        [WorkspacePermissionMode.Management, WorkspacePermissionMode.Write],
+        false
+      );
     };
 
     const updateWithWorkspacePermissionControl = async <T = unknown>(
@@ -247,18 +261,29 @@ export class WorkspaceSavedObjectsClientWrapper {
       attributes: T,
       options?: SavedObjectsCreateOptions
     ) => {
-      let workspacePermitted;
-      if (options?.workspaces && options.workspaces.length > 0) {
-        workspacePermitted = await this.validateMultiWorkspacesPermissions(
+      if (
+        options?.workspaces &&
+        options.workspaces.length > 0 &&
+        !(await this.validateMultiWorkspacesPermissions(
           options.workspaces,
           wrapperOptions.request,
           [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management]
-        );
-      } else {
-        workspacePermitted = true;
+        ))
+      ) {
+        throw generateWorkspacePermissionError();
       }
 
-      if (!workspacePermitted) {
+      if (
+        options?.overwrite &&
+        options.id &&
+        !(await this.validateWorkspacesAndSavedObjectsPermissions(
+          await wrapperOptions.client.get(type, options.id),
+          wrapperOptions.request,
+          [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management],
+          [WorkspacePermissionMode.Write, WorkspacePermissionMode.Management],
+          false
+        ))
+      ) {
         throw generateWorkspacePermissionError();
       }
 
@@ -271,31 +296,26 @@ export class WorkspaceSavedObjectsClientWrapper {
       options: SavedObjectsBaseOptions = {}
     ): Promise<SavedObject<T>> => {
       const objectToGet = await wrapperOptions.client.get<T>(type, id, options);
-      const workspacePermitted = await this.validateAtLeastOnePermittedWorkspaces(
-        objectToGet.workspaces,
-        wrapperOptions.request,
-        [
-          WorkspacePermissionMode.LibraryRead,
-          WorkspacePermissionMode.LibraryWrite,
-          WorkspacePermissionMode.Management,
-        ]
-      );
 
-      if (!workspacePermitted) {
-        const objectsPermitted = await this.validateObjectsPermissions(
-          [{ id, type }],
+      if (
+        !(await this.validateWorkspacesAndSavedObjectsPermissions(
+          objectToGet,
           wrapperOptions.request,
           [
             WorkspacePermissionMode.LibraryRead,
             WorkspacePermissionMode.LibraryWrite,
             WorkspacePermissionMode.Management,
+          ],
+          [
+            WorkspacePermissionMode.LibraryRead,
+            WorkspacePermissionMode.Management,
             WorkspacePermissionMode.Read,
             WorkspacePermissionMode.Write,
-          ]
-        );
-        if (!objectsPermitted) {
-          throw generateSavedObjectsPermissionError();
-        }
+          ],
+          false
+        ))
+      ) {
+        throw generateSavedObjectsPermissionError();
       }
       return objectToGet;
     };
@@ -304,37 +324,27 @@ export class WorkspaceSavedObjectsClientWrapper {
       objects: SavedObjectsBulkGetObject[] = [],
       options: SavedObjectsBaseOptions = {}
     ): Promise<SavedObjectsBulkResponse<T>> => {
-      const nonWorkspacePermittedObjects = [];
       const objectToBulkGet = await wrapperOptions.client.bulkGet<T>(objects, options);
 
       for (const object of objectToBulkGet.saved_objects) {
-        const workspacePermitted = await this.validateAtLeastOnePermittedWorkspaces(
-          object.workspaces,
-          wrapperOptions.request,
-          [
-            WorkspacePermissionMode.LibraryRead,
-            WorkspacePermissionMode.LibraryWrite,
-            WorkspacePermissionMode.Management,
-          ]
-        );
-        if (!workspacePermitted) {
-          nonWorkspacePermittedObjects.push(object);
-        }
-      }
-
-      if (nonWorkspacePermittedObjects.length > 0) {
-        const objectsPermitted = this.permissionControl.batchValidate(
-          wrapperOptions.request,
-          nonWorkspacePermittedObjects,
-          [
-            WorkspacePermissionMode.LibraryRead,
-            WorkspacePermissionMode.LibraryWrite,
-            WorkspacePermissionMode.Management,
-            WorkspacePermissionMode.Write,
-            WorkspacePermissionMode.Read,
-          ]
-        );
-        if (!objectsPermitted) {
+        if (
+          !(await this.validateWorkspacesAndSavedObjectsPermissions(
+            object,
+            wrapperOptions.request,
+            [
+              WorkspacePermissionMode.LibraryRead,
+              WorkspacePermissionMode.LibraryWrite,
+              WorkspacePermissionMode.Management,
+            ],
+            [
+              WorkspacePermissionMode.LibraryRead,
+              WorkspacePermissionMode.Management,
+              WorkspacePermissionMode.Write,
+              WorkspacePermissionMode.Read,
+            ],
+            false
+          ))
+        ) {
           throw generateSavedObjectsPermissionError();
         }
       }