Refractor: remove processFindOptions and getPermissionQuery metho…

…ds declaration in repositoryClient (opensearch-project#142) * refractor: remove additional method declaration Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: make bootstrap run Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: change to single permission modes Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: rename variable Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
SuZhou-Joe · Sep 13, 2023 · 05fab00 · 05fab00
1 parent bb2d886
commit 05fab00
Show file tree

Hide file tree

Showing 11 changed files with 175 additions and 240 deletions.
diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap
diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts
@@ -373,7 +373,6 @@ export class SavedObjectsClient {
       namespaces: 'namespaces',
       preference: 'preference',
       workspaces: 'workspaces',
-      queryDSL: 'queryDSL',
     };
 
     const currentWorkspaceId = this._getCurrentWorkspace();
@@ -384,14 +383,17 @@ export class SavedObjectsClient {
       finalWorkspaces = Array.from(new Set([currentWorkspaceId]));
     }
 
-    const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, {
-      ...options,
-      ...(finalWorkspaces
-        ? {
-            workspaces: finalWorkspaces,
-          }
-        : {}),
-    });
+    const renamedQuery = renameKeys<Omit<SavedObjectsFindOptions, 'ACLSearchParams'>, any>(
+      renameMap,
+      {
+        ...options,
+        ...(finalWorkspaces
+          ? {
+              workspaces: finalWorkspaces,
+            }
+          : {}),
+      }
+    );
     const query = pick.apply(null, [renamedQuery, ...Object.values<string>(renameMap)]) as Partial<
       Record<string, any>
     >;

diff --git a/src/core/server/saved_objects/permission_control/client.ts b/src/core/server/saved_objects/permission_control/client.ts
@@ -6,9 +6,10 @@ import { i18n } from '@osd/i18n';
 import { OpenSearchDashboardsRequest } from '../../http';
 import { ensureRawRequest } from '../../http/router';
 import { SavedObjectsServiceStart } from '../saved_objects_service';
-import { SavedObjectsBulkGetObject, SavedObjectsRepository, SavedObjectsUtils } from '../service';
+import { SavedObjectsBulkGetObject } from '../service';
 import { ACL, Principals, TransformedPermission, PrincipalType } from './acl';
 import { Logger } from '../../logging';
+import { WORKSPACE_TYPE } from '../../../utils';
 
 export type SavedObjectsPermissionControlContract = Pick<
   SavedObjectsPermissionControl,
@@ -163,11 +164,19 @@ export class SavedObjectsPermissionControl {
     permissionModes: SavedObjectsPermissionModes
   ) {
     const principals = this.getPrincipalsFromRequest(request);
-    const repository = this.getInternalRepository() as SavedObjectsRepository;
-    return await SavedObjectsUtils.getPermittedWorkspaceIds({
-      principals,
-      repository,
-      permissionModes,
-    });
+    const repository = this.getInternalRepository();
+    try {
+      const result = await repository?.find({
+        type: [WORKSPACE_TYPE],
+        ACLSearchParams: {
+          permissionModes,
+          principals,
+        },
+        perPage: 999,
+      });
+      return result?.saved_objects.map((item) => item.id);
+    } catch (e) {
+      return [];
+    }
   }
 }
diff --git a/src/core/server/saved_objects/service/lib/repository.mock.ts b/src/core/server/saved_objects/service/lib/repository.mock.ts
@@ -45,8 +45,6 @@ const create = (): jest.Mocked<ISavedObjectsRepository> => ({
   deleteByNamespace: jest.fn(),
   incrementCounter: jest.fn(),
   addToWorkspaces: jest.fn(),
-  getPermissionQuery: jest.fn(),
-  processFindOptions: jest.fn(),
   deleteByWorkspace: jest.fn(),
   deleteFromWorkspaces: jest.fn(),
 });

diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -31,7 +31,6 @@
 import { omit, intersection } from 'lodash';
 import type { opensearchtypes } from '@opensearch-project/opensearch';
 import uuid from 'uuid';
-import { i18n } from '@osd/i18n';
 import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry';
 import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry';
 import { DeleteDocumentResponse, OpenSearchClient } from '../../../opensearch/';
@@ -91,9 +90,7 @@ import {
   FIND_DEFAULT_PER_PAGE,
   SavedObjectsUtils,
 } from './utils';
-import { PUBLIC_WORKSPACE_ID, WorkspacePermissionMode } from '../../../../utils/constants';
-import { ACL, Principals } from '../../permission_control/acl';
-import { WORKSPACE_TYPE } from '../../../../utils';
+import { PUBLIC_WORKSPACE_ID } from '../../../../utils/constants';
 
 // BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
 // so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
@@ -902,7 +899,7 @@ export class SavedObjectsRepository {
       filter,
       preference,
       workspaces,
-      queryDSL,
+      ACLSearchParams,
     } = options;
 
     if (!type && !typeToNamespacesMap) {
@@ -977,7 +974,7 @@ export class SavedObjectsRepository {
           hasReference,
           kueryNode,
           workspaces,
-          queryDSL,
+          ACLSearchParams,
         }),
       },
     };
@@ -1934,111 +1931,6 @@ export class SavedObjectsRepository {
     };
   }
 
-  async getPermissionQuery(props: {
-    permissionTypes: string[];
-    principals: Principals;
-    savedObjectType?: string[];
-  }) {
-    return ACL.generateGetPermittedSavedObjectsQueryDSL(
-      props.permissionTypes,
-      props.principals,
-      props.savedObjectType
-    );
-  }
-
-  async processFindOptions(props: {
-    options: SavedObjectsFindOptions & { permissionModes?: string[] };
-    principals: Principals;
-  }): Promise<SavedObjectsFindOptions> {
-    const { principals } = props;
-    const options = { ...props.options };
-    if (this.isRelatedToWorkspace(options.type)) {
-      options.queryDSL = await this.getPermissionQuery({
-        permissionTypes: options.permissionModes ?? [
-          WorkspacePermissionMode.LibraryRead,
-          WorkspacePermissionMode.LibraryWrite,
-          WorkspacePermissionMode.Management,
-        ],
-        principals,
-        savedObjectType: [WORKSPACE_TYPE],
-      });
-    } else {
-      const permittedWorkspaceIds = await SavedObjectsUtils.getPermittedWorkspaceIds({
-        permissionModes: [
-          WorkspacePermissionMode.LibraryRead,
-          WorkspacePermissionMode.LibraryWrite,
-          WorkspacePermissionMode.Management,
-        ],
-        principals,
-        repository: this,
-      });
-
-      if (options.workspaces) {
-        const permittedWorkspaces = options.workspaces.filter((item) =>
-          (permittedWorkspaceIds || []).includes(item)
-        );
-        if (!permittedWorkspaces.length) {
-          /**
-           * If user does not have any one workspace access
-           * deny the request
-           */
-          throw SavedObjectsErrorHelpers.decorateNotAuthorizedError(
-            new Error(
-              i18n.translate('workspace.permission.invalidate', {
-                defaultMessage: 'Invalid workspace permission',
-              })
-            )
-          );
-        }
-
-        /**
-         * Overwrite the options.workspaces when user has access on partial workspaces.
-         */
-        options.workspaces = permittedWorkspaces;
-      } else {
-        const queryDSL = await this.getPermissionQuery({
-          permissionTypes: [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write],
-          principals,
-          savedObjectType: Array.isArray(options.type) ? options.type : [options.type],
-        });
-        options.workspaces = undefined;
-        /**
-         * Select all the docs that
-         * 1. ACL matches read or write permission OR
-         * 2. workspaces matches library_read or library_write or management OR
-         * 3. Advanced settings
-         */
-        options.queryDSL = {
-          query: {
-            bool: {
-              filter: [
-                {
-                  bool: {
-                    should: [
-                      {
-                        term: {
-                          type: 'config',
-                        },
-                      },
-                      queryDSL.query,
-                      {
-                        terms: {
-                          workspaces: permittedWorkspaceIds,
-                        },
-                      },
-                    ],
-                  },
-                },
-              ],
-            },
-          },
-        };
-      }
-    }
-
-    return options;
-  }
-
   /**
    * Returns index specified by the given type or the default index
    *
@@ -2167,16 +2059,6 @@ export class SavedObjectsRepository {
     }
     return body;
   }
-
-  /**
-   * check if the type include workspace
-   * Workspace permission check is totally different from object permission check.
-   * @param type
-   * @returns
-   */
-  private isRelatedToWorkspace(type: string | string[]): boolean {
-    return type === WORKSPACE_TYPE || (Array.isArray(type) && type.includes(WORKSPACE_TYPE));
-  }
 }
 
 function getBulkOperationError(error: { type: string; reason?: string }, type: string, id: string) {

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
@@ -27,13 +27,14 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { mergeWith, isArray } from 'lodash';
 // @ts-expect-error no ts
 import { opensearchKuery } from '../../../opensearch_query';
 type KueryNode = any;
 
 import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
 import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
+import { SavedObjectsFindOptions } from '../../../types';
+import { ACL } from '../../../permission_control/acl';
 
 /**
  * Gets the types based on the type. Uses mappings to support
@@ -166,7 +167,7 @@ interface QueryParams {
   hasReference?: HasReferenceQueryParams;
   kueryNode?: KueryNode;
   workspaces?: string[];
-  queryDSL?: Record<string, any>;
+  ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams'];
 }
 
 export function getClauseForReference(reference: HasReferenceQueryParams) {
@@ -224,7 +225,7 @@ export function getQueryParams({
   hasReference,
   kueryNode,
   workspaces,
-  queryDSL,
+  ACLSearchParams,
 }: QueryParams) {
   const types = getTypes(
     registry,
@@ -283,12 +284,43 @@ export function getQueryParams({
 
   const result = { query: { bool } };
 
-  if (queryDSL) {
-    return mergeWith({}, result, queryDSL, (objValue, srcValue) => {
-      if (isArray(objValue)) {
-        return objValue.concat(srcValue);
-      }
-    });
+  if (ACLSearchParams) {
+    const shouldClause: any = [];
+    if (ACLSearchParams.permissionModes && ACLSearchParams.principals) {
+      const permissionDSL = ACL.generateGetPermittedSavedObjectsQueryDSL(
+        ACLSearchParams.permissionModes,
+        ACLSearchParams.principals
+      );
+      shouldClause.push(permissionDSL.query);
+    }
+
+    if (ACLSearchParams.workspaces) {
+      shouldClause.push({
+        terms: {
+          workspaces: ACLSearchParams.workspaces,
+        },
+      });
+    }
+
+    if (shouldClause.length) {
+      bool.filter.push({
+        bool: {
+          should: [
+            /**
+             * TODO remove this clause once advanced settings has attached with permission
+             */
+            {
+              term: {
+                type: 'config',
+              },
+            },
+            ...shouldClause,
+          ],
+        },
+      });
+    }
+
+    return result;
   }
   return result;
 }