From 57daaf21b71656cd748adb3df15c28e5d2b871fd Mon Sep 17 00:00:00 2001 From: Qxisylolo Date: Wed, 8 Jan 2025 14:54:43 +0800 Subject: [PATCH] [workspace] refactor: refactor the bulk_get handler in permission wrapper when item has permission error (#8906) * return response with error Signed-off-by: Qxisylolo * set Id Signed-off-by: Qxisylolo * resolve tests Signed-off-by: Qxisylolo * Changeset file for PR #8906 created/updated * typo Signed-off-by: Qxisylolo * fix integration tests Signed-off-by: Qxisylolo * add try catch Signed-off-by: Qxisylolo * resolve conflicts Signed-off-by: Qxisylolo * delete data source permission Signed-off-by: Qxisylolo * add try catch Signed-off-by: Qxisylolo --------- Signed-off-by: Qxisylolo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/8906.yml | 2 + ...space_saved_objects_client_wrapper.test.ts | 37 ++++++------ ...space_saved_objects_client_wrapper.test.ts | 28 ++++++--- .../workspace_saved_objects_client_wrapper.ts | 60 +++++++++++++------ 4 files changed, 81 insertions(+), 46 deletions(-) create mode 100644 changelogs/fragments/8906.yml diff --git a/changelogs/fragments/8906.yml b/changelogs/fragments/8906.yml new file mode 100644 index 000000000000..35d14192ce3d --- /dev/null +++ b/changelogs/fragments/8906.yml @@ -0,0 +1,2 @@ +feat: +- Refactor the bulk_get handler in permission wrapper when item has permission error ([#8906](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8906)) \ No newline at end of file diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index e3eddb443990..83339b76c6f4 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -200,27 +200,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { describe('bulkGet', () => { it('should throw forbidden error when user not permitted', async () => { - let error; - try { - await notPermittedSavedObjectedClient.bulkGet([ - { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, - ]); - } catch (e) { - error = e; - } - expect(error).not.toBeUndefined(); - expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + const result = await notPermittedSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, + ]); - error = undefined; - try { - await notPermittedSavedObjectedClient.bulkGet([ - { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, - ]); - } catch (e) { - error = e; - } - expect(error).not.toBeUndefined(); - expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + expect(result.saved_objects).toEqual([ + { + ...result.saved_objects[0], + id: 'acl-controlled-dashboard-2', + type: 'dashboard', + attributes: {}, + error: { + error: 'Forbidden', + statusCode: 403, + message: 'Invalid saved objects permission', + }, + workspaces: [], + }, + ]); }); it('should return consistent dashboard when user permitted', async () => { diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 55098d6e2b27..abb15204f5de 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -660,12 +660,11 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { permissionControlMock, requestMock, } = generateWorkspaceSavedObjectsClientWrapper(); - let errorCatched; - try { - await wrapper.bulkGet([{ type: 'dashboard', id: 'not-permitted-dashboard' }]); - } catch (e) { - errorCatched = e; - } + + const result = await wrapper.bulkGet([ + { type: 'dashboard', id: 'not-permitted-dashboard' }, + ]); + expect(permissionControlMock.validate).toHaveBeenCalledWith( requestMock, { @@ -674,7 +673,22 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }, ['library_read', 'library_write'] ); - expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + + expect(result.saved_objects).toEqual([ + { + id: 'not-permitted-dashboard', + type: 'dashboard', + attributes: {}, + error: { + error: 'Forbidden', + message: 'Invalid saved objects permission', + statusCode: 403, + }, + workspaces: [], + permissions: {}, + references: [], + }, + ]); }); it('should call permission validateSavedObjectsACL with object', async () => { const { wrapper, permissionControlMock } = generateWorkspaceSavedObjectsClientWrapper(); 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 7fdf4706b9ad..b49262d67f98 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 @@ -450,26 +450,48 @@ export class WorkspaceSavedObjectsClientWrapper { wrapperOptions.request, getWorkspacesFromSavedObjects(objectToBulkGet.saved_objects) ); - - for (const object of objectToBulkGet.saved_objects) { - if ( - !(await this.validateWorkspacesAndSavedObjectsPermissions( - object, - wrapperOptions.request, - [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], - [WorkspacePermissionMode.Write, WorkspacePermissionMode.Read], - false - )) - ) { - ACLAuditor?.increment(ACLAuditorStateKey.VALIDATE_FAILURE, 1); - throw generateSavedObjectsPermissionError(); - } - } - ACLAuditor?.increment( - ACLAuditorStateKey.VALIDATE_SUCCESS, - objectToBulkGet.saved_objects.length + const processedObjects = await Promise.all( + objectToBulkGet.saved_objects.map(async (object) => { + try { + const hasPermission = await this.validateWorkspacesAndSavedObjectsPermissions( + object, + wrapperOptions.request, + [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write, WorkspacePermissionMode.Read], + false + ); + if (hasPermission) { + ACLAuditor?.increment(ACLAuditorStateKey.VALIDATE_SUCCESS, 1); + return object; + } else { + ACLAuditor?.increment(ACLAuditorStateKey.VALIDATE_FAILURE, 1); + return { + ...object, + workspaces: [], + attributes: {} as T, + error: { + ...generateSavedObjectsPermissionError().output.payload, + statusCode: 403, + }, + }; + } + } catch (error) { + ACLAuditor?.increment(ACLAuditorStateKey.VALIDATE_FAILURE, 1); + return { + ...object, + workspaces: [], + attributes: {} as T, + error: { + ...generateWorkspacePermissionError().output.payload, + statusCode: error.statusCode, + message: error.message, + }, + }; + } + }) ); - return objectToBulkGet; + + return { saved_objects: processedObjects }; }; const findWithWorkspacePermissionControl = async (