optimize the code

Signed-off-by: yubonluo <[email protected]>
opensearch-project · Dec 2, 2024 · 10cef9e · 10cef9e
1 parent cece0f3
commit 10cef9e
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 18 deletions.
diff --git a/...ns/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/...ns/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts
@@ -375,7 +375,7 @@ describe('workspace_id_consumer integration test', () => {
 
       await osdTestServer.request
         .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/${dashboard.type}/bar`)
-        .expect(404);
+        .expect(403);
 
       await Promise.all(
         [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) =>
@@ -434,9 +434,9 @@ describe('workspace_id_consumer integration test', () => {
       ]);
       expect(bulkGetResultWithWorkspace.body.saved_objects[1]?.error).toMatchInlineSnapshot(`
         Object {
-          "error": "Not Found",
-          "message": "Saved object [dashboard/bar] not found",
-          "statusCode": 404,
+          "error": "Forbidden",
+          "message": "Saved object does not belong to the workspace",
+          "statusCode": 403,
         }
       `);
 

diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts
@@ -292,7 +292,7 @@ describe('WorkspaceIdConsumerWrapper', () => {
       };
       mockedClient.get.mockResolvedValueOnce(savedObject);
       expect(wrapperClient.get(savedObject.type, savedObject.id)).rejects.toMatchInlineSnapshot(
-        `[Error: Saved object [dashboard/dashboard_id] not found]`
+        `[Error: Saved object does not belong to the workspace]`
       );
       expect(mockedClient.get).toBeCalledWith(savedObject.type, savedObject.id, {});
     });
@@ -349,7 +349,7 @@ describe('WorkspaceIdConsumerWrapper', () => {
       mockedClient.get.mockResolvedValueOnce(savedObject);
       mockedClient.get.mockResolvedValueOnce(savedObject);
       expect(wrapperClient.get(savedObject.type, savedObject.id)).rejects.toMatchInlineSnapshot(
-        `[Error: Saved object [data-source/data-source_id] not found]`
+        `[Error: Saved object does not belong to the workspace]`
       );
       expect(mockedClient.get).toBeCalledWith(savedObject.type, savedObject.id, {});
     });
@@ -358,6 +358,7 @@ describe('WorkspaceIdConsumerWrapper', () => {
   describe('bulkGet', () => {
     const payload = [
       { id: 'dashboard_id', type: 'dashboard' },
+      { id: 'dashboard_error_id', type: 'dashboard' },
       { id: 'visualization_id', type: 'visualization' },
       { id: 'global_data_source_id', type: 'data-source' },
       { id: 'data_source_id', type: 'data-source' },
@@ -370,6 +371,17 @@ describe('WorkspaceIdConsumerWrapper', () => {
         references: [],
         workspaces: ['foo'],
       },
+      {
+        type: 'dashboard',
+        id: 'dashboard_error_id',
+        attributes: {},
+        references: [],
+        error: {
+          statusCode: 404,
+          error: 'Not Found',
+          message: 'Saved object [dashboard/dashboard_error_id] not found',
+        },
+      },
       {
         type: 'visualization',
         id: 'visualization_id',
@@ -428,9 +440,20 @@ describe('WorkspaceIdConsumerWrapper', () => {
               "attributes": Object {},
               "error": Object {
                 "error": "Not Found",
-                "message": "Saved object [visualization/visualization_id] not found",
+                "message": "Saved object [dashboard/dashboard_error_id] not found",
                 "statusCode": 404,
               },
+              "id": "dashboard_error_id",
+              "references": Array [],
+              "type": "dashboard",
+            },
+            Object {
+              "attributes": Object {},
+              "error": Object {
+                "error": "Forbidden",
+                "message": "Saved object does not belong to the workspace",
+                "statusCode": 403,
+              },
               "id": "visualization_id",
               "references": Array [],
               "type": "visualization",
@@ -453,9 +476,9 @@ describe('WorkspaceIdConsumerWrapper', () => {
             Object {
               "attributes": Object {},
               "error": Object {
-                "error": "Not Found",
-                "message": "Saved object [data-source/global_data_source_id] not found",
-                "statusCode": 404,
+                "error": "Forbidden",
+                "message": "Saved object does not belong to the workspace",
+                "statusCode": 403,
               },
               "id": "global_data_source_id",
               "references": Array [],
@@ -495,9 +518,20 @@ describe('WorkspaceIdConsumerWrapper', () => {
               "attributes": Object {},
               "error": Object {
                 "error": "Not Found",
-                "message": "Saved object [visualization/visualization_id] not found",
+                "message": "Saved object [dashboard/dashboard_error_id] not found",
                 "statusCode": 404,
               },
+              "id": "dashboard_error_id",
+              "references": Array [],
+              "type": "dashboard",
+            },
+            Object {
+              "attributes": Object {},
+              "error": Object {
+                "error": "Forbidden",
+                "message": "Saved object does not belong to the workspace",
+                "statusCode": 403,
+              },
               "id": "visualization_id",
               "references": Array [],
               "type": "visualization",
@@ -520,9 +554,9 @@ describe('WorkspaceIdConsumerWrapper', () => {
             Object {
               "attributes": Object {},
               "error": Object {
-                "error": "Not Found",
-                "message": "Saved object [data-source/global_data_source_id] not found",
-                "statusCode": 404,
+                "error": "Forbidden",
+                "message": "Saved object does not belong to the workspace",
+                "statusCode": 403,
               },
               "id": "global_data_source_id",
               "references": Array [],
@@ -601,9 +635,20 @@ describe('WorkspaceIdConsumerWrapper', () => {
               "attributes": Object {},
               "error": Object {
                 "error": "Not Found",
-                "message": "Saved object [visualization/visualization_id] not found",
+                "message": "Saved object [dashboard/dashboard_error_id] not found",
                 "statusCode": 404,
               },
+              "id": "dashboard_error_id",
+              "references": Array [],
+              "type": "dashboard",
+            },
+            Object {
+              "attributes": Object {},
+              "error": Object {
+                "error": "Forbidden",
+                "message": "Saved object does not belong to the workspace",
+                "statusCode": 403,
+              },
               "id": "visualization_id",
               "references": Array [],
               "type": "visualization",

diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts
@@ -25,6 +25,15 @@ const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config';
 
 type WorkspaceOptions = Pick<SavedObjectsBaseOptions, 'workspaces'> | undefined;
 
+const generateSavedObjectsForbiddenError = () =>
+  SavedObjectsErrorHelpers.decorateForbiddenError(
+    new Error(
+      i18n.translate('workspace.id_consumer.saved_objects.forbidden', {
+        defaultMessage: 'Saved object does not belong to the workspace',
+      })
+    )
+  );
+
 export class WorkspaceIdConsumerWrapper {
   private formatWorkspaceIdParams<T extends WorkspaceOptions>(
     request: OpenSearchDashboardsRequest,
@@ -57,6 +66,10 @@ export class WorkspaceIdConsumerWrapper {
     workspace: string,
     request: OpenSearchDashboardsRequest
   ) {
+    // Keep the original object error
+    if (!!object?.error) {
+      return true;
+    }
     // Data source is a workspace level object, validate if the request has access to the data source within the requested workspace.
     if (validateIsWorkspaceDataSourceAndConnectionObjectType(object.type)) {
       if (!!getWorkspaceState(request).isDataSourceAdmin) {
@@ -177,8 +190,7 @@ export class WorkspaceIdConsumerWrapper {
                 : {
                     ...object,
                     error: {
-                      ...SavedObjectsErrorHelpers.createGenericNotFoundError(object.type, object.id)
-                        .output.payload,
+                      ...generateSavedObjectsForbiddenError().output.payload,
                     },
                   };
             }),
@@ -204,7 +216,7 @@ export class WorkspaceIdConsumerWrapper {
           workspaces?.length === 1 &&
           !this.validateObjectInAWorkspace(objectToGet, workspaces[0], wrapperOptions.request)
         ) {
-          throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
+          throw generateSavedObjectsForbiddenError();
         }
 
         // Allow access if no specific workspace is requested.