From 909ada030c4898d21d7ae8022912e9f11e3fa746 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 4 Jun 2024 02:28:16 +0000 Subject: [PATCH] [Workspace] Only OSD admin can create workspace (#6831) * Only OSD admin can create workspace Signed-off-by: yubonluo * Changeset file for PR #6831 created/updated * Changeset file for PR #6831 created/updated * optimize the code Signed-off-by: yubonluo * delete useless code Signed-off-by: yubonluo * add integration tests Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * delete useless code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * add overwrite check Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * Add the logic of whether to update the operation Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * Optimize the code Signed-off-by: yubonluo * Optimize the code Signed-off-by: yubonluo --------- Signed-off-by: yubonluo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 32fbe1876102dbeb613b122b6d08ce020c87542b) Signed-off-by: github-actions[bot] --- changelogs/fragments/6831.yml | 2 + ...space_saved_objects_client_wrapper.test.ts | 122 +++++++++++++++++- ...space_saved_objects_client_wrapper.test.ts | 48 +++++++ .../workspace_saved_objects_client_wrapper.ts | 23 ++++ 4 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/6831.yml diff --git a/changelogs/fragments/6831.yml b/changelogs/fragments/6831.yml new file mode 100644 index 000000000000..a7dc8210023b --- /dev/null +++ b/changelogs/fragments/6831.yml @@ -0,0 +1,2 @@ +feat: +- [Workspace] Only OSD admin can create workspace ([#6831](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6831)) \ 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 432760aa692a..b5399de9fb5d 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 @@ -97,6 +97,20 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { id: 'workspace-1', permissions: { library_read: { users: ['foo'] }, + write: { users: ['foo'] }, + library_write: { users: ['foo'] }, + }, + } + ); + + await repositoryKit.create( + internalSavedObjectsRepository, + 'workspace', + {}, + { + id: 'workspace-2', + permissions: { + write: { users: ['foo'] }, library_write: { users: ['foo'] }, }, } @@ -132,7 +146,9 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { return { users: ['foo'] }; }); - permittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient(permittedRequest); + permittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient(permittedRequest, { + includedHiddenTypes: ['workspace'], + }); notPermittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( notPermittedRequest ); @@ -316,6 +332,62 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.error).toBeUndefined(); }); + + it('should able to create workspace with override', async () => { + const createResult = await permittedSavedObjectedClient.create( + 'workspace', + {}, + { + id: 'workspace-2', + overwrite: true, + permissions: { + library_write: { users: ['foo'] }, + write: { users: ['foo'] }, + }, + } + ); + + expect(createResult.error).toBeUndefined(); + }); + + it('should throw forbidden error when user create a workspce and is not OSD admin', async () => { + let error; + try { + await permittedSavedObjectedClient.create('workspace', {}, {}); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + try { + await permittedSavedObjectedClient.create('workspace', {}, { id: 'workspace-1' }); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + try { + await permittedSavedObjectedClient.create('workspace', {}, { overwrite: true }); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + try { + await permittedSavedObjectedClient.create( + 'workspace', + {}, + { id: 'no-exist', overwrite: true } + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); + }); }); describe('bulkCreate', () => { @@ -384,6 +456,54 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(createResult.saved_objects).toHaveLength(1); }); + + it('should able to bulk create workspace with override', async () => { + const createResult = await permittedSavedObjectedClient.bulkCreate( + [ + { + id: 'workspace-2', + type: 'workspace', + attributes: {}, + }, + ], + { + overwrite: true, + } + ); + + expect(createResult.saved_objects).toHaveLength(1); + }); + + it('should throw forbidden error when user bulk create workspace and is not OSD admin', async () => { + let error; + try { + await permittedSavedObjectedClient.bulkCreate([{ type: 'workspace', attributes: {} }]); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + try { + await permittedSavedObjectedClient.bulkCreate([{ type: 'workspace', attributes: {} }], { + overwrite: true, + }); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + try { + await permittedSavedObjectedClient.bulkCreate([ + { type: 'workspace', id: 'test', attributes: {} }, + ]); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); }); describe('update', () => { 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 f488d83e7946..79e1b25cd823 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 @@ -352,6 +352,30 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { workspaces: ['workspace-1'], }); }); + it('should throw permission error when user bulkCreate workspace and is not dashboard admin', async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.bulkCreate([{ type: 'workspace', attributes: {} }]); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid permission, please contact OSD admin'); + + try { + await wrapper.bulkCreate([{ type: 'workspace', id: 'test', attributes: {} }]); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid permission, please contact OSD admin'); + + try { + await wrapper.bulkCreate([{ type: 'workspace', attributes: {} }], { overwrite: true }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid permission, please contact OSD admin'); + }); }); describe('create', () => { @@ -426,6 +450,30 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { } ); }); + it('should throw permission error when user create a workspace and is not dashboard admin', async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.create('workspace', { name: 'test' }, { overwrite: true }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid permission, please contact OSD admin'); + + try { + await wrapper.create('workspace', { name: 'test' }, { id: 'test' }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid permission, please contact OSD admin'); + + try { + await wrapper.create('workspace', { name: 'test' }, {}); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid permission, please contact OSD admin'); + }); }); describe('get', () => { it('should return saved object if no need to validate permission', async () => { 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 f97d423b3058..06701fba338a 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 @@ -53,6 +53,15 @@ const generateSavedObjectsPermissionError = () => ) ); +const generateOSDAdminPermissionError = () => + SavedObjectsErrorHelpers.decorateForbiddenError( + new Error( + i18n.translate('dashboard.admin.permission.invalidate', { + defaultMessage: 'Invalid permission, please contact OSD admin', + }) + ) + ); + const intersection = (...args: T[][]) => { const occursCountMap: { [key: string]: number } = {}; for (let i = 0; i < args.length; i++) { @@ -272,6 +281,13 @@ export class WorkspaceSavedObjectsClientWrapper { objects: Array>, options: SavedObjectsCreateOptions = {} ): Promise> => { + // Objects with id in overwrite mode will be regarded as update + const objectsToCreate = options.overwrite ? objects.filter((obj) => !obj.id) : objects; + // Only OSD admin can bulkCreate workspace. + if (objectsToCreate.some((obj) => obj.type === WORKSPACE_TYPE)) { + throw generateOSDAdminPermissionError(); + } + const hasTargetWorkspaces = options?.workspaces && options.workspaces.length > 0; if ( @@ -330,6 +346,13 @@ export class WorkspaceSavedObjectsClientWrapper { attributes: T, options?: SavedObjectsCreateOptions ) => { + // If options contains id and overwrite, it is an update action. + const isUpdateMode = options?.id && options?.overwrite; + // Only OSD admin can create workspace. + if (type === WORKSPACE_TYPE && !isUpdateMode) { + throw generateOSDAdminPermissionError(); + } + const hasTargetWorkspaces = options?.workspaces && options.workspaces.length > 0; if (