From cfa9c4bc4a9541419faf1d7f9fadf8320cc87dca Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 18 Sep 2023 17:27:45 +0800 Subject: [PATCH] feat: skip permission validate when no workspaces and permissions attributes (#163) * feat: skip permission validate when saved object without workspaces and permissions attributes Signed-off-by: Lin Wang * feat: add annontation to skip permission check Signed-off-by: Lin Wang * refactor: remove bind and simplify validate logic Signed-off-by: Lin Wang * feat: remove library write for object based ACL Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang --- .../workspace_saved_objects_client_wrapper.ts | 214 +++++++++--------- 1 file changed, 112 insertions(+), 102 deletions(-) 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 index eb255eb72771..31ef3789bb1f 100644 --- 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 @@ -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,6 +134,48 @@ export class WorkspaceSavedObjectsClientWrapper { return type === WORKSPACE_TYPE || (Array.isArray(type) && type.includes(WORKSPACE_TYPE)); } + private async validateWorkspacesAndSavedObjectsPermissions( + savedObject: Pick, + 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, @@ -141,25 +183,15 @@ export class WorkspaceSavedObjectsClientWrapper { 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 ( objectToUpdate: SavedObject ): Promise => { - 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 ( @@ -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> => { const objectToGet = await wrapperOptions.client.get(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> => { - const nonWorkspacePermittedObjects = []; const objectToBulkGet = await wrapperOptions.client.bulkGet(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(); } }