Change public objects logic & Search with ACL control (opensearch-pro…

…ject#71) * feat: update public workspace Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * refractor: change public objects logic Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: create public workspace when service start Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: some modify Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feature: some optimize and create workspace when ui settings is open Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * temp: submit Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update query dsl Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: use same constants Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: make it run Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove dashboard admin Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: modify query DSL Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: modify query DSL Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: modify query DSL Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: list principals route Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: optimize query DSL Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: change public logic Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove init Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add judgement when workspaceList is empty Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
SuZhou-Joe · Aug 31, 2023 · 8e1ead5 · 8e1ead5
1 parent 9d0ae81
commit 8e1ead5
Show file tree

Hide file tree

Showing 18 changed files with 206 additions and 74 deletions.
diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts
@@ -42,6 +42,7 @@ import {
 
 import { SimpleSavedObject } from './simple_saved_object';
 import { HttpFetchOptions, HttpSetup } from '../http';
+import { PUBLIC_WORKSPACE } from '../../utils';
 
 type SavedObjectsFindOptions = Omit<
   SavedObjectFindOptionsServer,
@@ -184,7 +185,11 @@ const getObjectsToFetch = (queue: BatchQueueEntry[]): ObjectTypeAndId[] => {
 export class SavedObjectsClient {
   private http: HttpSetup;
   private batchQueue: BatchQueueEntry[];
-  private currentWorkspaceId?: string;
+  /**
+   * if currentWorkspaceId is undefined, it means
+   * we should not carry out workspace info when doing any operation.
+   */
+  private currentWorkspaceId: string | undefined;
 
   /**
    * Throttled processing of get requests into bulk requests at 100ms interval
@@ -229,11 +234,11 @@ export class SavedObjectsClient {
     this.batchQueue = [];
   }
 
-  private async _getCurrentWorkspace(): Promise<string | null> {
-    return this.currentWorkspaceId || null;
+  private _getCurrentWorkspace(): string | undefined {
+    return this.currentWorkspaceId;
   }
 
-  public async setCurrentWorkspace(workspaceId: string): Promise<boolean> {
+  public setCurrentWorkspace(workspaceId: string): boolean {
     this.currentWorkspaceId = workspaceId;
     return true;
   }
@@ -259,7 +264,13 @@ export class SavedObjectsClient {
     const query = {
       overwrite: options.overwrite,
     };
-    const currentWorkspaceId = await this._getCurrentWorkspace();
+    const currentWorkspaceId = this._getCurrentWorkspace();
+    let finalWorkspaces;
+    if (options.hasOwnProperty('workspaces')) {
+      finalWorkspaces = options.workspaces;
+    } else if (typeof currentWorkspaceId === 'string') {
+      finalWorkspaces = [currentWorkspaceId];
+    }
 
     const createRequest: Promise<SavedObject<T>> = this.savedObjectsFetch(path, {
       method: 'POST',
@@ -268,9 +279,9 @@ export class SavedObjectsClient {
         attributes,
         migrationVersion: options.migrationVersion,
         references: options.references,
-        ...(options.workspaces || currentWorkspaceId
+        ...(finalWorkspaces
           ? {
-              workspaces: options.workspaces || [currentWorkspaceId],
+              workspaces: finalWorkspaces,
             }
           : {}),
       }),
@@ -366,14 +377,21 @@ export class SavedObjectsClient {
       queryDSL: 'queryDSL',
     };
 
-    const workspaces = [
-      ...(options.workspaces || [await this._getCurrentWorkspace()]),
-      'public',
-    ].filter((item) => item);
+    const currentWorkspaceId = this._getCurrentWorkspace();
+    let finalWorkspaces;
+    if (options.hasOwnProperty('workspaces')) {
+      finalWorkspaces = options.workspaces;
+    } else if (typeof currentWorkspaceId === 'string') {
+      finalWorkspaces = Array.from(new Set([PUBLIC_WORKSPACE, currentWorkspaceId]));
+    }
 
     const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, {
       ...options,
-      workspaces,
+      ...(finalWorkspaces
+        ? {
+            workspaces: finalWorkspaces,
+          }
+        : {}),
     });
     const query = pick.apply(null, [renamedQuery, ...Object.values<string>(renameMap)]) as Partial<
       Record<string, any>

diff --git a/src/core/public/utils/index.ts b/src/core/public/utils/index.ts
@@ -32,4 +32,8 @@ export { shareWeakReplay } from './share_weak_replay';
 export { Sha256 } from './crypto';
 export { MountWrapper, mountReactNode } from './mount';
 export { getWorkspaceIdFromUrl, WORKSPACE_TYPE } from './workspace';
-export { WORKSPACE_PATH_PREFIX } from '../../utils';
+export {
+  WORKSPACE_PATH_PREFIX,
+  PUBLIC_WORKSPACE,
+  WORKSPACE_FEATURE_FLAG_KEY_IN_UI_SETTINGS,
+} from '../../utils';
diff --git a/src/core/public/workspace/workspaces_client.ts b/src/core/public/workspace/workspaces_client.ts
@@ -48,25 +48,27 @@ export class WorkspacesClient {
 
     combineLatest([this.workspaceList$, this.currentWorkspaceId$]).subscribe(
       ([workspaceList, currentWorkspaceId]) => {
-        const currentWorkspace = this.findWorkspace([workspaceList, currentWorkspaceId]);
+        if (workspaceList.length) {
+          const currentWorkspace = this.findWorkspace([workspaceList, currentWorkspaceId]);
 
-        /**
-         * Do a simple idempotent verification here
-         */
-        if (!isEqual(currentWorkspace, this.currentWorkspace$.getValue())) {
-          this.currentWorkspace$.next(currentWorkspace);
-        }
-
-        if (currentWorkspaceId && !currentWorkspace?.id) {
           /**
-           * Current workspace is staled
+           * Do a simple idempotent verification here
            */
-          this.currentWorkspaceId$.error({
-            reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED,
-          });
-          this.currentWorkspace$.error({
-            reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED,
-          });
+          if (!isEqual(currentWorkspace, this.currentWorkspace$.getValue())) {
+            this.currentWorkspace$.next(currentWorkspace);
+          }
+
+          if (currentWorkspaceId && !currentWorkspace?.id) {
+            /**
+             * Current workspace is staled
+             */
+            this.currentWorkspaceId$.error({
+              reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED,
+            });
+            this.currentWorkspace$.error({
+              reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED,
+            });
+          }
         }
       }
     );

diff --git a/src/core/server/saved_objects/permission_control/routes/index.ts b/src/core/server/saved_objects/permission_control/routes/index.ts
@@ -5,6 +5,7 @@
 
 import { InternalHttpServiceSetup } from '../../../http';
 import { SavedObjectsPermissionControlContract } from '../client';
+import { registerListRoute } from './principals';
 import { registerValidateRoute } from './validate';
 
 export function registerPermissionCheckRoutes({
@@ -17,4 +18,5 @@ export function registerPermissionCheckRoutes({
   const router = http.createRouter('/api/saved_objects_permission_control/');
 
   registerValidateRoute(router, permissionControl);
+  registerListRoute(router, permissionControl);
 }
diff --git a/...s/permission_control/routes/principles.ts → ...s/permission_control/routes/principals.ts b/...s/permission_control/routes/principles.ts → ...s/permission_control/routes/principals.ts
diff --git a/src/core/server/saved_objects/routes/share.ts b/src/core/server/saved_objects/routes/share.ts
@@ -9,7 +9,7 @@ import { exportSavedObjectsToStream } from '../export';
 import { validateObjects } from './utils';
 import { collectSavedObjects } from '../import/collect_saved_objects';
 import { WORKSPACE_TYPE } from '../../workspaces';
-import { GLOBAL_WORKSPACE_ID } from '../../workspaces/constants';
+import { PUBLIC_WORKSPACE } from '../../../utils/constants';
 
 const SHARE_LIMIT = 10000;
 
@@ -73,7 +73,7 @@ export const registerShareRoute = (router: IRouter) => {
           (obj) =>
             obj.workspaces &&
             obj.workspaces.length > 0 &&
-            !obj.workspaces.includes(GLOBAL_WORKSPACE_ID)
+            !obj.workspaces.includes(PUBLIC_WORKSPACE)
         )
         .map((obj) => ({ id: obj.id, type: obj.type, workspaces: obj.workspaces }));
 

diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -87,7 +87,7 @@ import {
   FIND_DEFAULT_PER_PAGE,
   SavedObjectsUtils,
 } from './utils';
-import { GLOBAL_WORKSPACE_ID } from '../../../workspaces/constants';
+import { PUBLIC_WORKSPACE } 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.
@@ -1299,7 +1299,7 @@ export class SavedObjectsRepository {
         if (
           obj.workspaces &&
           obj.workspaces.length > 0 &&
-          !obj.workspaces.includes(GLOBAL_WORKSPACE_ID)
+          !obj.workspaces.includes(PUBLIC_WORKSPACE)
         ) {
           return intersection(obj.workspaces, options.workspaces).length === 0;
         }
@@ -1352,7 +1352,7 @@ export class SavedObjectsRepository {
             params: {
               time,
               workspaces,
-              globalWorkspaceId: GLOBAL_WORKSPACE_ID,
+              globalWorkspaceId: PUBLIC_WORKSPACE,
             },
           },
         },

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
@@ -142,14 +142,6 @@ function getClauseForWorkspace(workspace: string) {
     };
   }
 
-  if (workspace === 'public') {
-    return {
-      bool: {
-        must_not: [{ exists: { field: 'workspaces' } }],
-      },
-    };
-  }
-
   return {
     bool: {
       must: [{ term: { workspaces: workspace } }],

diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts
@@ -29,7 +29,6 @@
  */
 
 import { Permissions } from '../permission_control/acl';
-
 import { ISavedObjectsRepository } from './lib';
 import {
   SavedObject,

diff --git a/src/core/server/server.ts b/src/core/server/server.ts
@@ -262,7 +262,9 @@ export class Server {
       opensearch: opensearchStart,
       savedObjects: savedObjectsStart,
     });
-    await this.workspaces.start();
+    await this.workspaces.start({
+      savedObjects: savedObjectsStart,
+    });
 
     this.coreStart = {
       capabilities: capabilitiesStart,

diff --git a/src/core/server/workspaces/constants.ts b/src/core/server/workspaces/constants.ts
@@ -4,4 +4,3 @@
  */
 
 export const WORKSPACE_TYPE = 'workspace';
-export const GLOBAL_WORKSPACE_ID = 'public';
diff --git a/src/core/server/workspaces/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/core/server/workspaces/saved_objects/workspace_saved_objects_client_wrapper.ts
@@ -214,26 +214,49 @@ export class WorkspaceSavedObjectsClientWrapper {
         );
         if (options.workspaces) {
           const isEveryWorkspaceIsPermitted = options.workspaces.every((item) =>
-            // TODO modify this line to use permittedWorkspaceIds if public workspace is also a workspace
-            ['public', ...(permittedWorkspaceIds || [])]?.includes(item)
+            (permittedWorkspaceIds || []).includes(item)
           );
           if (!isEveryWorkspaceIsPermitted) {
             throw generateWorkspacePermissionError();
           }
         } else {
           const queryDSL = ACL.genereateGetPermittedSavedObjectsQueryDSL(
-            [
-              PermissionMode.LibraryRead,
-              PermissionMode.LibraryWrite,
-              PermissionMode.Management,
-              PermissionMode.Read,
-              PermissionMode.Write,
-            ],
+            [PermissionMode.Read, PermissionMode.Write],
             principals,
             options.type
           );
-          options.workspaces = permittedWorkspaceIds;
-          options.queryDSL = queryDSL;
+          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,
+                          },
+                        },
+                      ],
+                    },
+                  },
+                ],
+              },
+            },
+          };
         }
       }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,4 +4,3 @@
		*/

		export const WORKSPACE_TYPE = 'workspace';
		export const GLOBAL_WORKSPACE_ID = 'public';