Backport permission control to pr integr (opensearch-project#314)

* [Workspace]Add permission control logic for workspace (opensearch-project#6052) * Add permission control for workspace Signed-off-by: Lin Wang <wonglam@amazon.com> * Add changelog for permission control in workspace Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix integration tests and remove no need type Signed-off-by: Lin Wang <wonglam@amazon.com> * Update permission enabled for workspace CRUD integration tests Signed-off-by: Lin Wang <wonglam@amazon.com> * Change back to config schema Signed-off-by: Lin Wang <wonglam@amazon.com> * feat: do not append workspaces field when no workspaces present (#6) * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: authInfo destructure (#7) * fix: authInfo destructure Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: unit test error Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * Fix permissions assign in attributes Signed-off-by: Lin Wang <wonglam@amazon.com> * Remove deleteByWorkspace since not exists Signed-off-by: Lin Wang <wonglam@amazon.com> * refactor: remove formatWorkspacePermissionModeToStringArray Signed-off-by: Lin Wang <wonglam@amazon.com> * Remove current not used code Signed-off-by: Lin Wang <wonglam@amazon.com> * Add missing unit tests for permission control Signed-off-by: Lin Wang <wonglam@amazon.com> * Update workspaces API test describe Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix workspace CRUD API integration tests failed Signed-off-by: Lin Wang <wonglam@amazon.com> * Address PR comments Signed-off-by: Lin Wang <wonglam@amazon.com> * Store permissions when savedObjects.permissions.enabled Signed-off-by: Lin Wang <wonglam@amazon.com> * Add permission control for deleteByWorkspace Signed-off-by: Lin Wang <wonglam@amazon.com> * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * Refactor permissions field in workspace create and update API Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix workspace CRUD API integration tests Signed-off-by: Lin Wang <wonglam@amazon.com> --------- Signed-off-by: Lin Wang <wonglam@amazon.com> Signed-off-by: SuZhou-Joe <suzhou@amazon.com> Co-authored-by: SuZhou-Joe <suzhou@amazon.com> Signed-off-by: Lin Wang <wonglam@amazon.com> * Convert permission settings in client side Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix workspace list always render Signed-off-by: Lin Wang <wonglam@amazon.com> --------- Signed-off-by: Lin Wang <wonglam@amazon.com> Signed-off-by: SuZhou-Joe <suzhou@amazon.com> Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
wanglam · Apr 7, 2024 · b2ab2bd · b2ab2bd
1 parent 43c4e96
commit b2ab2bd
Show file tree

Hide file tree

Showing 31 changed files with 627 additions and 284 deletions.
diff --git a/src/core/server/index.ts b/src/core/server/index.ts
@@ -321,12 +321,11 @@ export {
   exportSavedObjectsToStream,
   importSavedObjectsFromStream,
   resolveSavedObjectsImportErrors,
-  SavedObjectsDeleteByWorkspaceOptions,
   ACL,
   Principals,
-  TransformedPermission,
   PrincipalType,
   Permissions,
+  SavedObjectsDeleteByWorkspaceOptions,
 } from './saved_objects';
 
 export {

diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts
@@ -89,6 +89,9 @@ export function pluginInitializerContextConfigMock<T>(config: T) {
     path: { data: '/tmp' },
     savedObjects: {
       maxImportPayloadBytes: new ByteSizeValue(26214400),
+      permission: {
+        enabled: true,
+      },
     },
   };
 

diff --git a/src/core/server/plugins/plugin_context.test.ts b/src/core/server/plugins/plugin_context.test.ts
@@ -108,7 +108,12 @@ describe('createPluginInitializerContext', () => {
         pingTimeout: duration(30, 's'),
       },
       path: { data: fromRoot('data') },
-      savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400) },
+      savedObjects: {
+        maxImportPayloadBytes: new ByteSizeValue(26214400),
+        permission: {
+          enabled: false,
+        },
+      },
     });
   });
 

diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts
@@ -295,7 +295,7 @@ export const SharedGlobalConfigKeys = {
   ] as const,
   opensearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const,
   path: ['data'] as const,
-  savedObjects: ['maxImportPayloadBytes'] as const,
+  savedObjects: ['maxImportPayloadBytes', 'permission'] as const,
 };
 
 /**

diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts
@@ -85,10 +85,4 @@ export {
 export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config';
 export { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry';
 
-export {
-  Permissions,
-  ACL,
-  Principals,
-  TransformedPermission,
-  PrincipalType,
-} from './permission_control/acl';
+export { Permissions, ACL, Principals, PrincipalType } from './permission_control/acl';
diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js
@@ -167,7 +167,7 @@ describe('SavedObjectsRepository', () => {
   });
 
   const getMockGetResponse = (
-    { type, id, references, namespace: objectNamespace, originId, workspaces, permissions },
+    { type, id, references, namespace: objectNamespace, originId, permissions, workspaces },
     namespace
   ) => {
     const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace;
@@ -181,9 +181,9 @@ describe('SavedObjectsRepository', () => {
       _source: {
         ...(registry.isSingleNamespace(type) && { namespace: namespaceId }),
         ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }),
-        workspaces,
         ...(originId && { originId }),
         ...(permissions && { permissions }),
+        ...(workspaces && { workspaces }),
         type,
         [type]: { title: 'Testing' },
         references,
@@ -3169,7 +3169,7 @@ describe('SavedObjectsRepository', () => {
     const namespace = 'foo-namespace';
     const originId = 'some-origin-id';
 
-    const getSuccess = async (type, id, options, includeOriginId, permissions) => {
+    const getSuccess = async (type, id, options, includeOriginId, permissions, workspaces) => {
       const response = getMockGetResponse(
         {
           type,
@@ -3178,6 +3178,7 @@ describe('SavedObjectsRepository', () => {
           // operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response.
           ...(includeOriginId && { originId }),
           ...(permissions && { permissions }),
+          ...(workspaces && { workspaces }),
         },
         options?.namespace
       );
@@ -3343,6 +3344,14 @@ describe('SavedObjectsRepository', () => {
           permissions: permissions,
         });
       });
+
+      it(`includes workspaces property if present`, async () => {
+        const workspaces = ['workspace-1'];
+        const result = await getSuccess(type, id, { namespace }, undefined, undefined, workspaces);
+        expect(result).toMatchObject({
+          workspaces: workspaces,
+        });
+      });
     });
   });
 

diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -1044,7 +1044,7 @@ export class SavedObjectsRepository {
       throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
     }
 
-    const { originId, updated_at: updatedAt, workspaces, permissions } = body._source;
+    const { originId, updated_at: updatedAt, permissions, workspaces } = body._source;
 
     let namespaces: string[] = [];
     if (!this._registry.isNamespaceAgnostic(type)) {
@@ -1059,8 +1059,8 @@ export class SavedObjectsRepository {
       namespaces,
       ...(originId && { originId }),
       ...(updatedAt && { updated_at: updatedAt }),
-      ...(workspaces && { workspaces }),
       ...(permissions && { permissions }),
+      ...(workspaces && { workspaces }),
       version: encodeHitVersion(body),
       attributes: body._source[type],
       references: body._source.references || [],

diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts
@@ -126,3 +126,5 @@ export interface SavedObjectError {
   statusCode: number;
   metadata?: Record<string, unknown>;
 }
+
+export type SavedObjectPermissions = Permissions;
diff --git a/src/core/types/workspace.ts b/src/core/types/workspace.ts
@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { Permissions } from '../server/saved_objects';
+
 export interface WorkspaceAttribute {
   id: string;
   name: string;
@@ -14,6 +16,10 @@ export interface WorkspaceAttribute {
   defaultVISTheme?: string;
 }
 
-export interface WorkspaceObject extends WorkspaceAttribute {
+export interface WorkspaceAttributeWithPermission extends WorkspaceAttribute {
+  permissions?: Permissions;
+}
+
+export interface WorkspaceObject extends WorkspaceAttributeWithPermission {
   readonly?: boolean;
 }
diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx
@@ -152,7 +152,7 @@ describe('WorkspaceCreator', () => {
         color: '#000000',
         description: 'test workspace description',
       }),
-      expect.any(Array)
+      undefined
     );
     await waitFor(() => {
       expect(notificationToastsAddSuccess).toHaveBeenCalled();
@@ -174,7 +174,7 @@ describe('WorkspaceCreator', () => {
         name: 'test workspace name',
         features: expect.arrayContaining(['app1', 'app2', 'app3']),
       }),
-      expect.any(Array)
+      undefined
     );
     await waitFor(() => {
       expect(notificationToastsAddSuccess).toHaveBeenCalled();
@@ -201,7 +201,14 @@ describe('WorkspaceCreator', () => {
       expect.objectContaining({
         name: 'test workspace name',
       }),
-      expect.arrayContaining([expect.objectContaining({ type: 'user', userId: 'test user id' })])
+      {
+        read: {
+          users: ['test user id'],
+        },
+        library_read: {
+          users: ['test user id'],
+        },
+      }
     );
     await waitFor(() => {
       expect(notificationToastsAddSuccess).toHaveBeenCalled();

diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx
@@ -11,6 +11,7 @@ import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '
 import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants';
 import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
 import { WorkspaceClient } from '../../workspace_client';
+import { convertPermissionSettingsToPermissions } from '../workspace_form/utils';
 
 export const WorkspaceCreator = () => {
   const {
@@ -22,8 +23,11 @@ export const WorkspaceCreator = () => {
     async (data: WorkspaceFormSubmitData) => {
       let result;
       try {
-        const { permissions, ...attributes } = data;
-        result = await workspaceClient.create(attributes, permissions);
+        const { permissionSettings, ...attributes } = data;
+        result = await workspaceClient.create(
+          attributes,
+          convertPermissionSettingsToPermissions(permissionSettings)
+        );
       } catch (error) {
         notifications?.toasts.addDanger({
           title: i18n.translate('workspace.create.failed', {

diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts
@@ -5,7 +5,7 @@
 
 import type { WorkspacePermissionItemType, WorkspaceOperationType } from './constants';
 import type { WorkspacePermissionMode } from '../../../common/constants';
-import type { App, ApplicationStart } from '../../../../../core/public';
+import type { ApplicationStart } from '../../../../../core/public';
 
 export type WorkspacePermissionSetting =
   | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] }
@@ -18,7 +18,7 @@ export interface WorkspaceFormSubmitData {
   color?: string;
   icon?: string;
   defaultVISTheme?: string;
-  permissions: WorkspacePermissionSetting[];
+  permissionSettings?: WorkspacePermissionSetting[];
 }
 
 export interface WorkspaceFormData extends WorkspaceFormSubmitData {

diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts
@@ -46,8 +46,8 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
   const [permissionSettings, setPermissionSettings] = useState<
     Array<Partial<WorkspacePermissionSetting>>
   >(
-    defaultValues?.permissions && defaultValues.permissions.length > 0
-      ? defaultValues.permissions
+    defaultValues?.permissionSettings && defaultValues.permissionSettings.length > 0
+      ? defaultValues.permissionSettings
       : []
   );
 
@@ -58,7 +58,7 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
     description,
     features: selectedFeatureIds,
     color,
-    permissions: permissionSettings,
+    permissionSettings,
   });
   const getFormDataRef = useRef(getFormData);
   getFormDataRef.current = getFormData;
@@ -96,12 +96,15 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
           }),
         };
       }
-      const permissionErrors: string[] = new Array(formData.permissions.length);
-      for (let i = 0; i < formData.permissions.length; i++) {
-        const permission = formData.permissions[i];
+      const permissionErrors: string[] = new Array(formData.permissionSettings.length);
+      for (let i = 0; i < formData.permissionSettings.length; i++) {
+        const permission = formData.permissionSettings[i];
         if (isValidWorkspacePermissionSetting(permission)) {
           if (
-            isUserOrGroupPermissionSettingDuplicated(formData.permissions.slice(0, i), permission)
+            isUserOrGroupPermissionSettingDuplicated(
+              formData.permissionSettings.slice(0, i),
+              permission
+            )
           ) {
             permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', {
               defaultMessage: 'Duplicate permission setting',
@@ -162,8 +165,11 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works
         formData.features = defaultValues?.features ?? [];
       }
 
-      const permissions = formData.permissions.filter(isValidWorkspacePermissionSetting);
-      onSubmit?.({ ...formData, name: formData.name!, permissions });
+      onSubmit?.({
+        ...formData,
+        name: formData.name!,
+        permissionSettings: formData.permissionSettings.filter(isValidWorkspacePermissionSetting),
+      });
     },
     [defaultFeatures, onSubmit, defaultValues?.features]
   );

diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts
@@ -4,6 +4,7 @@
  */
 
 import { WorkspacePermissionMode, DEFAULT_CHECKED_FEATURES_IDS } from '../../../common/constants';
+import type { SavedObjectPermissions } from '../../../../../core/types';
 
 import {
   WorkspaceFeature,
@@ -95,3 +96,78 @@ export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => {
   }
   return PermissionModeId.Read;
 };
+
+export const convertPermissionSettingsToPermissions = (
+  permissionItems: WorkspacePermissionSetting[] | undefined
+) => {
+  if (!permissionItems || permissionItems.length === 0) {
+    return undefined;
+  }
+  return permissionItems.reduce<SavedObjectPermissions>((previous, current) => {
+    current.modes.forEach((mode) => {
+      if (!previous[mode]) {
+        previous[mode] = {};
+      }
+      switch (current.type) {
+        case 'user':
+          previous[mode].users = [...(previous[mode].users || []), current.userId];
+          break;
+        case 'group':
+          previous[mode].groups = [...(previous[mode].groups || []), current.group];
+          break;
+      }
+    });
+    return previous;
+  }, {});
+};
+
+const isWorkspacePermissionMode = (test: string): test is WorkspacePermissionMode =>
+  test === WorkspacePermissionMode.LibraryRead ||
+  test === WorkspacePermissionMode.LibraryWrite ||
+  test === WorkspacePermissionMode.Read ||
+  test === WorkspacePermissionMode.Write;
+
+export const convertPermissionsToPermissionSettings = (permissions: SavedObjectPermissions) => {
+  const userPermissionSettings: WorkspacePermissionSetting[] = [];
+  const groupPermissionSettings: WorkspacePermissionSetting[] = [];
+  const settingType2Modes: { [key: string]: WorkspacePermissionMode[] } = {};
+
+  Object.keys(permissions).forEach((mode) => {
+    if (!isWorkspacePermissionMode(mode)) {
+      return;
+    }
+    if (permissions[mode].users) {
+      permissions[mode].users?.forEach((userId) => {
+        const settingTypeKey = `userId-${userId}`;
+        const modes = settingType2Modes[settingTypeKey] ? settingType2Modes[settingTypeKey] : [];
+
+        modes.push(mode);
+        if (modes.length === 1) {
+          userPermissionSettings.push({
+            type: WorkspacePermissionItemType.User,
+            userId,
+            modes,
+          });
+          settingType2Modes[settingTypeKey] = modes;
+        }
+      });
+      permissions[mode].groups?.forEach((group) => {
+        const settingTypeKey = `group-${group}`;
+        const modes = settingType2Modes[settingTypeKey] ? settingType2Modes[settingTypeKey] : [];
+
+        modes.push(mode);
+        if (modes.length === 1) {
+          userPermissionSettings.push({
+            type: WorkspacePermissionItemType.Group,
+            group,
+            modes,
+          });
+        }
+      });
+    }
+  });
+
+  return [...userPermissionSettings, ...groupPermissionSettings].filter(
+    isValidWorkspacePermissionSetting
+  );
+};
diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx
@@ -170,7 +170,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
           <WorkspacePermissionSettingPanel
             errors={formErrors.permissions}
             onChange={setPermissionSettings}
-            permissionSettings={formData.permissions}
+            permissionSettings={formData.permissionSettings}
             lastAdminItemDeletable={!!permissionLastAdminItemDeletable}
             data-test-subj={`workspaceForm-permissionSettingPanel`}
           />