From bcd4793f780d889de79ef269fa4e83cd563524a7 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 10 Aug 2023 11:14:40 +0800 Subject: [PATCH 1/4] feat: add permission check when updating workspace Signed-off-by: tygao --- .../workspace_saved_objects_client_wrapper.ts | 79 +++++++++++++++++-- 1 file changed, 72 insertions(+), 7 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 bd1d68bd0f7b..10a70e3d3739 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 @@ -19,11 +19,16 @@ import { SavedObjectsDeleteOptions, SavedObjectsFindOptions, SavedObjectsShareObjects, - SavedObjectsPermissionControlContract, - WORKSPACE_TYPE, - ACL, - WorkspacePermissionMode, -} from '../../../../core/server'; + SavedObjectsUpdateOptions, + SavedObjectsUpdateResponse, + SavedObjectsBulkUpdateObject, + SavedObjectsBulkUpdateResponse, + SavedObjectsBulkUpdateOptions, +} from 'opensearch-dashboards/server'; +import { SavedObjectsPermissionControlContract } from '../../saved_objects/permission_control/client'; +import { WORKSPACE_TYPE } from '../constants'; +import { PermissionMode } from '../../../utils'; +import { ACL } from '../../saved_objects/permission_control/acl'; // Can't throw unauthorized for now, the page will be refreshed if unauthorized const generateWorkspacePermissionError = () => @@ -60,6 +65,28 @@ export class WorkspaceSavedObjectsClientWrapper { return [permission]; } + private async validateSingleWorkspacePermissions( + workspaceId: string | undefined, + request: OpenSearchDashboardsRequest, + permissionMode: PermissionMode | PermissionMode[] + ) { + if (!workspaceId) { + return; + } + if ( + !(await this.permissionControl.validate( + request, + { + type: WORKSPACE_TYPE, + id: workspaceId, + }, + this.formatPermissionModeToStringArray(permissionMode) + )) + ) { + throw generateWorkspacePermissionError(); + } + } + private async validateMultiWorkspacesPermissions( workspaces: string[] | undefined, request: OpenSearchDashboardsRequest, @@ -129,6 +156,13 @@ export class WorkspaceSavedObjectsClientWrapper { id: string, options: SavedObjectsDeleteOptions = {} ) => { + if (this.isRelatedToWorkspace(type)) { + await this.validateSingleWorkspacePermissions(id, wrapperOptions.request, [ + PermissionMode.LibraryWrite, + PermissionMode.Management, + ]); + } + const objectToDeleted = await wrapperOptions.client.get(type, id, options); await this.validateMultiWorkspacesPermissions( objectToDeleted.workspaces, @@ -138,6 +172,37 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.delete(type, id, options); }; + const updateWithWorkspacePermissionControl = async ( + type: string, + id: string, + attributes: Partial, + options: SavedObjectsUpdateOptions = {} + ): Promise> => { + if (this.isRelatedToWorkspace(type)) { + await this.validateSingleWorkspacePermissions(id, wrapperOptions.request, [ + PermissionMode.LibraryWrite, + PermissionMode.Management, + ]); + } + return await wrapperOptions.client.update(type, id, attributes, options); + }; + + const bulkUpdateWithWorkspacePermissionControl = async ( + objects: Array>, + options?: SavedObjectsBulkUpdateOptions + ): Promise> => { + for (const object of objects) { + if (this.isRelatedToWorkspace(object.type)) { + await this.validateSingleWorkspacePermissions(object.id, wrapperOptions.request, [ + PermissionMode.LibraryWrite, + PermissionMode.Management, + ]); + } + } + + return await wrapperOptions.client.bulkUpdate(objects, options); + }; + const bulkCreateWithWorkspacePermissionControl = async ( objects: Array>, options: SavedObjectsCreateOptions = {} @@ -310,8 +375,8 @@ export class WorkspaceSavedObjectsClientWrapper { create: createWithWorkspacePermissionControl, bulkCreate: bulkCreateWithWorkspacePermissionControl, delete: deleteWithWorkspacePermissionControl, - update: wrapperOptions.client.update, - bulkUpdate: wrapperOptions.client.bulkUpdate, + update: updateWithWorkspacePermissionControl, + bulkUpdate: bulkUpdateWithWorkspacePermissionControl, addToWorkspaces: addToWorkspacesWithPermissionControl, }; }; From 205a4c370be61db5e6de49090d85bd508482035b Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 10 Aug 2023 16:34:22 +0800 Subject: [PATCH 2/4] fix: only use management access and update bulkUpdate logic Signed-off-by: tygao --- .../workspace_saved_objects_client_wrapper.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 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 10a70e3d3739..cfd57b0e36b8 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 @@ -158,7 +158,6 @@ export class WorkspaceSavedObjectsClientWrapper { ) => { if (this.isRelatedToWorkspace(type)) { await this.validateSingleWorkspacePermissions(id, wrapperOptions.request, [ - PermissionMode.LibraryWrite, PermissionMode.Management, ]); } @@ -180,7 +179,6 @@ export class WorkspaceSavedObjectsClientWrapper { ): Promise> => { if (this.isRelatedToWorkspace(type)) { await this.validateSingleWorkspacePermissions(id, wrapperOptions.request, [ - PermissionMode.LibraryWrite, PermissionMode.Management, ]); } @@ -191,13 +189,19 @@ export class WorkspaceSavedObjectsClientWrapper { objects: Array>, options?: SavedObjectsBulkUpdateOptions ): Promise> => { - for (const object of objects) { - if (this.isRelatedToWorkspace(object.type)) { - await this.validateSingleWorkspacePermissions(object.id, wrapperOptions.request, [ - PermissionMode.LibraryWrite, - PermissionMode.Management, - ]); + const workspaceIds = objects.reduce((acc, cur) => { + if (this.isRelatedToWorkspace(cur.type)) { + acc.push(cur.id); } + return acc; + }, []); + const permittedWorkspaceIds = + (await this.permissionControl.getPermittedWorkspaceIds(wrapperOptions.request, [ + PermissionMode.Management, + ])) ?? []; + const workspacePermitted = workspaceIds.every((id) => permittedWorkspaceIds?.includes(id)); + if (!workspacePermitted) { + throw generateWorkspacePermissionError(); } return await wrapperOptions.client.bulkUpdate(objects, options); From 0d178a6674f5348ba3bd9f26a46f27ca724a6ad8 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 10 Aug 2023 16:35:44 +0800 Subject: [PATCH 3/4] chore: update code Signed-off-by: tygao --- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cfd57b0e36b8..2b51a9e151fd 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 @@ -199,7 +199,7 @@ export class WorkspaceSavedObjectsClientWrapper { (await this.permissionControl.getPermittedWorkspaceIds(wrapperOptions.request, [ PermissionMode.Management, ])) ?? []; - const workspacePermitted = workspaceIds.every((id) => permittedWorkspaceIds?.includes(id)); + const workspacePermitted = workspaceIds.every((id) => permittedWorkspaceIds.includes(id)); if (!workspacePermitted) { throw generateWorkspacePermissionError(); } From 0a5053cb23a11d55abe6ad32936c2883b17689ae Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 11 Aug 2023 16:32:00 +0800 Subject: [PATCH 4/4] chore: update code after rebase Signed-off-by: tygao --- .../workspace_saved_objects_client_wrapper.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 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 2b51a9e151fd..170498fb40bb 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 @@ -24,11 +24,11 @@ import { SavedObjectsBulkUpdateObject, SavedObjectsBulkUpdateResponse, SavedObjectsBulkUpdateOptions, -} from 'opensearch-dashboards/server'; -import { SavedObjectsPermissionControlContract } from '../../saved_objects/permission_control/client'; -import { WORKSPACE_TYPE } from '../constants'; -import { PermissionMode } from '../../../utils'; -import { ACL } from '../../saved_objects/permission_control/acl'; + SavedObjectsPermissionControlContract, + WORKSPACE_TYPE, + ACL, + WorkspacePermissionMode, +} from '../../../../core/server'; // Can't throw unauthorized for now, the page will be refreshed if unauthorized const generateWorkspacePermissionError = () => @@ -68,7 +68,7 @@ export class WorkspaceSavedObjectsClientWrapper { private async validateSingleWorkspacePermissions( workspaceId: string | undefined, request: OpenSearchDashboardsRequest, - permissionMode: PermissionMode | PermissionMode[] + permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] ) { if (!workspaceId) { return; @@ -80,7 +80,7 @@ export class WorkspaceSavedObjectsClientWrapper { type: WORKSPACE_TYPE, id: workspaceId, }, - this.formatPermissionModeToStringArray(permissionMode) + this.formatWorkspacePermissionModeToStringArray(permissionMode) )) ) { throw generateWorkspacePermissionError(); @@ -158,7 +158,7 @@ export class WorkspaceSavedObjectsClientWrapper { ) => { if (this.isRelatedToWorkspace(type)) { await this.validateSingleWorkspacePermissions(id, wrapperOptions.request, [ - PermissionMode.Management, + WorkspacePermissionMode.Management, ]); } @@ -179,7 +179,7 @@ export class WorkspaceSavedObjectsClientWrapper { ): Promise> => { if (this.isRelatedToWorkspace(type)) { await this.validateSingleWorkspacePermissions(id, wrapperOptions.request, [ - PermissionMode.Management, + WorkspacePermissionMode.Management, ]); } return await wrapperOptions.client.update(type, id, attributes, options); @@ -197,7 +197,7 @@ export class WorkspaceSavedObjectsClientWrapper { }, []); const permittedWorkspaceIds = (await this.permissionControl.getPermittedWorkspaceIds(wrapperOptions.request, [ - PermissionMode.Management, + WorkspacePermissionMode.Management, ])) ?? []; const workspacePermitted = workspaceIds.every((id) => permittedWorkspaceIds.includes(id)); if (!workspacePermitted) {