[Workspace] Only OSD admin can create workspace (#6831) (#6900)

* Only OSD admin can create workspace * Changeset file for PR #6831 created/updated * Changeset file for PR #6831 created/updated * optimize the code * delete useless code * add integration tests * optimize the code * delete useless code * optimize the code * optimize the code * optimize the code * add overwrite check * optimize the code * optimize the code * Add the logic of whether to update the operation * optimize the code * Optimize the code * Optimize the code --------- (cherry picked from commit 32fbe18) Signed-off-by: yubonluo <yubonluo@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> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Jun 4, 2024 · ead2800 · ead2800
1 parent 5082f42
commit ead2800
Show file tree

Hide file tree

Showing 4 changed files with 194 additions and 1 deletion.
diff --git a/changelogs/fragments/6831.yml 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))
diff --git a/...ace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/...ace/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
@@ -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
@@ -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 = <T extends string>(...args: T[][]) => {
   const occursCountMap: { [key: string]: number } = {};
   for (let i = 0; i < args.length; i++) {
@@ -272,6 +281,13 @@ export class WorkspaceSavedObjectsClientWrapper {
       objects: Array<SavedObjectsBulkCreateObject<T>>,
       options: SavedObjectsCreateOptions = {}
     ): Promise<SavedObjectsBulkResponse<T>> => {
+      // 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 (
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		feat:
		- [Workspace] Only OSD admin can create workspace ([#6831](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6831))