[Workspace]Restrict saved objects finding when workspace enabled (#7125)

* Limit data source saved objects finding and access when workspace enabled Signed-off-by: Lin Wang <[email protected]> * Remove options.workspaces drop behavior in conflict client wrapper Signed-off-by: Lin Wang <[email protected]> * Changeset file for PR #7125 created/updated * Update src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: Lin Wang <[email protected]> * Update src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Co-authored-by: Yulong Ruan <[email protected]> Signed-off-by: Lin Wang <[email protected]> * Remove formatFindParams Signed-off-by: Lin Wang <[email protected]> * Fix code format Signed-off-by: Lin Wang <[email protected]> * Fix ACLSearchParams and empty workspaces Signed-off-by: Lin Wang <[email protected]> * Refactor workspace client wrapper find Signed-off-by: Lin Wang <[email protected]> * Remove global saved objects validation logic Signed-off-by: Lin Wang <[email protected]> * Update uts and its Signed-off-by: Lin Wang <[email protected]> * Changeset file for PR #7125 created/updated * Remove no need changes and refactor Signed-off-by: Lin Wang <[email protected]> * Fix global saved objects not found Signed-off-by: Lin Wang <[email protected]> * Change saved object get to assert Signed-off-by: Lin Wang <[email protected]> --------- Signed-off-by: Lin Wang <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Yulong Ruan <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]> Co-authored-by: Yulong Ruan <[email protected]>
opensearch-project · Jul 5, 2024 · 1cb5956 · 1cb5956
1 parent e0945af
commit 1cb5956
Show file tree

Hide file tree

Showing 7 changed files with 165 additions and 291 deletions.
diff --git a/changelogs/fragments/7125.yml b/changelogs/fragments/7125.yml
@@ -0,0 +1,2 @@
+fix:
+- [Workspace]Restrict saved objects finding when workspace enabled ([#7125](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7125))
diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts
@@ -656,7 +656,7 @@ describe('#getQueryParams', () => {
         expect(result.query.bool.filter[1]).toEqual(undefined);
       });
 
-      it('workspacesSearchOperator prvided as "OR"', () => {
+      it('workspacesSearchOperator provided as "OR"', () => {
         const result: Result = getQueryParams({
           registry,
           workspaces: ['foo'],
@@ -665,22 +665,6 @@ describe('#getQueryParams', () => {
         expect(result.query.bool.filter[1]).toEqual({
           bool: {
             should: [
-              {
-                bool: {
-                  must_not: [
-                    {
-                      exists: {
-                        field: 'workspaces',
-                      },
-                    },
-                    {
-                      exists: {
-                        field: 'permissions',
-                      },
-                    },
-                  ],
-                },
-              },
               {
                 bool: {
                   minimum_should_match: 1,
@@ -718,22 +702,6 @@ describe('#getQueryParams', () => {
         expect(result.query.bool.filter[1]).toEqual({
           bool: {
             should: [
-              {
-                bool: {
-                  must_not: [
-                    {
-                      exists: {
-                        field: 'workspaces',
-                      },
-                    },
-                    {
-                      exists: {
-                        field: 'permissions',
-                      },
-                    },
-                  ],
-                },
-              },
               {
                 bool: {
                   filter: [
@@ -771,6 +739,36 @@ describe('#getQueryParams', () => {
         });
       });
     });
+
+    describe('when using empty workspace search', () => {
+      it('workspacesSearchOperator provided as "OR"', () => {
+        const result: Result = getQueryParams({
+          registry,
+          workspaces: [],
+          workspacesSearchOperator: 'OR',
+        });
+        expect(result.query.bool.filter[1]).toEqual({
+          bool: {
+            should: [
+              {
+                match_none: {},
+              },
+            ],
+          },
+        });
+      });
+
+      it('workspacesSearchOperator provided as "AND"', () => {
+        const result: Result = getQueryParams({
+          registry,
+          workspaces: [],
+          workspacesSearchOperator: 'AND',
+        });
+        expect(result.query.bool.filter[1]).toEqual({
+          match_none: {},
+        });
+      });
+    });
   });
 
   describe('namespaces property', () => {

diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts
@@ -286,53 +286,32 @@ export function getQueryParams({
     }
   }
 
-  if (workspaces?.length) {
-    if (workspacesSearchOperator === 'OR') {
-      ACLSearchParamsShouldClause.push({
-        bool: {
-          should: workspaces.map((workspace) => {
-            return getClauseForWorkspace(workspace);
-          }),
-          minimum_should_match: 1,
-        },
-      });
-    } else {
-      bool.filter.push({
-        bool: {
-          should: workspaces.map((workspace) => {
-            return getClauseForWorkspace(workspace);
-          }),
-          minimum_should_match: 1,
-        },
-      });
+  if (workspaces) {
+    const conditions =
+      workspacesSearchOperator === 'OR' ? ACLSearchParamsShouldClause : bool.filter;
+
+    switch (workspaces.length) {
+      case 0:
+        conditions.push({
+          match_none: {},
+        });
+        break;
+      default:
+        conditions.push({
+          bool: {
+            should: workspaces.map((workspace) => {
+              return getClauseForWorkspace(workspace);
+            }),
+            minimum_should_match: 1,
+          },
+        });
     }
   }
 
   if (ACLSearchParamsShouldClause.length) {
     bool.filter.push({
       bool: {
-        should: [
-          /**
-           * Return those objects without workspaces field and permissions field to keep find API backward compatible
-           */
-          {
-            bool: {
-              must_not: [
-                {
-                  exists: {
-                    field: 'workspaces',
-                  },
-                },
-                {
-                  exists: {
-                    field: 'permissions',
-                  },
-                },
-              ],
-            },
-          },
-          ...ACLSearchParamsShouldClause,
-        ],
+        should: ACLSearchParamsShouldClause,
       },
     });
   }

diff --git a/...aved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/...aved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts
@@ -381,10 +381,10 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test',
         ],
       });
 
-      const findAdvancedSettings = await osdTestServer.request
-        .get(root, `/api/saved_objects/_find?type=${advancedSettings.type}`)
+      const getAdvancedSettingsResult = await osdTestServer.request
+        .get(root, `/api/saved_objects/${advancedSettings.type}/${packageInfo.version}`)
         .expect(200);
-      expect(findAdvancedSettings.body.total).toEqual(1);
+      expect(getAdvancedSettingsResult.body.id).toBe(packageInfo.version);
     });
 
     it('checkConflicts when importing ndjson', async () => {

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
@@ -241,20 +241,20 @@ describe('WorkspaceSavedObjectsClientWrapper', () => {
   });
 
   describe('find', () => {
-    it('should throw not authorized error when user not permitted', async () => {
-      let error;
-      try {
-        await notPermittedSavedObjectedClient.find({
-          type: 'dashboard',
-          workspaces: ['workspace-1'],
-          perPage: 999,
-          page: 1,
-        });
-      } catch (e) {
-        error = e;
-      }
+    it('should return empty result if user not permitted', async () => {
+      const result = await notPermittedSavedObjectedClient.find({
+        type: 'dashboard',
+        workspaces: ['workspace-1'],
+        perPage: 999,
+        page: 1,
+      });
 
-      expect(SavedObjectsErrorHelpers.isNotAuthorizedError(error)).toBe(true);
+      expect(result).toEqual({
+        saved_objects: [],
+        total: 0,
+        page: 1,
+        per_page: 999,
+      });
     });
 
     it('should return consistent inner workspace data when user permitted', async () => {
@@ -269,6 +269,36 @@ describe('WorkspaceSavedObjectsClientWrapper', () => {
         true
       );
     });
+
+    it('should return consistent result when workspaces and ACLSearchParams not provided', async () => {
+      const result = await permittedSavedObjectedClient.find({
+        type: 'dashboard',
+        perPage: 999,
+        page: 1,
+      });
+
+      expect(result.saved_objects).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({ id: 'inner-workspace-dashboard-1' }),
+          expect.objectContaining({ id: 'acl-controlled-dashboard-2' }),
+        ])
+      );
+    });
+
+    it('should return acl controled dashboards when only ACLSearchParams provided', async () => {
+      const result = await permittedSavedObjectedClient.find({
+        type: 'dashboard',
+        perPage: 999,
+        page: 1,
+        ACLSearchParams: {
+          permissionModes: ['read', 'write'],
+        },
+      });
+
+      expect(result.saved_objects).toEqual(
+        expect.arrayContaining([expect.objectContaining({ id: 'acl-controlled-dashboard-2' })])
+      );
+    });
   });
 
   describe('create', () => {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- [Workspace]Restrict saved objects finding when workspace enabled ([#7125](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7125))