feat: throw error when create or update with invalid permission modes

Signed-off-by: Lin Wang <[email protected]>
ruanyl · Oct 24, 2023 · 9bc49f5 · 9bc49f5
1 parent 1ac7ec4
commit 9bc49f5
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 31 deletions.
diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx
@@ -22,7 +22,8 @@ export const WorkspaceCreator = () => {
     async (data: WorkspaceFormSubmitData) => {
       let result;
       try {
-        result = await workspaceClient.create(data);
+        const { permissions, ...attributes } = data;
+        result = await workspaceClient.create(attributes, permissions);
       } catch (error) {
         notifications?.toasts.addDanger({
           title: i18n.translate('workspace.create.failed', {

diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx
@@ -72,7 +72,8 @@ export const WorkspaceUpdater = () => {
         return;
       }
       try {
-        result = await workspaceClient.update(currentWorkspace?.id, data);
+        const { permissions, ...attributes } = data;
+        result = await workspaceClient.update(currentWorkspace?.id, attributes, permissions);
       } catch (error) {
         notifications?.toasts.addDanger({
           title: i18n.translate('workspace.update.failed', {

diff --git a/src/plugins/workspace/public/workspace_client.ts b/src/plugins/workspace/public/workspace_client.ts
@@ -176,16 +176,16 @@ export class WorkspaceClient {
    * @returns
    */
   public async create(
-    attributes: Omit<WorkspaceAttribute, 'id'> & {
-      permissions: WorkspaceRoutePermissionItem[];
-    }
+    attributes: Omit<WorkspaceAttribute, 'id'>,
+    permissions: WorkspaceRoutePermissionItem[]
   ): Promise<IResponse<WorkspaceAttribute>> {
     const path = this.getPath();
 
     const result = await this.safeFetch<WorkspaceAttribute>(path, {
       method: 'POST',
       body: JSON.stringify({
         attributes,
+        permissions,
       }),
     });
 
@@ -264,15 +264,13 @@ export class WorkspaceClient {
    */
   public async update(
     id: string,
-    attributes: Partial<
-      WorkspaceAttribute & {
-        permissions: WorkspaceRoutePermissionItem[];
-      }
-    >
+    attributes: Partial<WorkspaceAttribute>,
+    permissions: WorkspaceRoutePermissionItem[]
   ): Promise<IResponse<boolean>> {
     const path = this.getPath(id);
     const body = {
       attributes,
+      permissions,
     };
 
     const result = await this.safeFetch(path, {

diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts
@@ -22,8 +22,6 @@ const workspacePermissionMode = schema.oneOf([
   schema.literal(WorkspacePermissionMode.Write),
   schema.literal(WorkspacePermissionMode.LibraryRead),
   schema.literal(WorkspacePermissionMode.LibraryWrite),
-  schema.literal(WorkspacePermissionMode.Read),
-  schema.literal(WorkspacePermissionMode.Write),
 ]);
 
 const workspacePermission = schema.oneOf([
@@ -47,9 +45,6 @@ const workspaceAttributesSchema = schema.object({
   icon: schema.maybe(schema.string()),
   reserved: schema.maybe(schema.boolean()),
   defaultVISTheme: schema.maybe(schema.string()),
-  permissions: schema.maybe(
-    schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)])
-  ),
 });
 
 const convertToACL = (
@@ -180,31 +175,29 @@ export function registerRoutes({
       validate: {
         body: schema.object({
           attributes: workspaceAttributesSchema,
+          permissions: schema.maybe(
+            schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)])
+          ),
         }),
       },
     },
     router.handleLegacyErrors(async (context, req, res) => {
-      const { attributes } = req.body;
+      const { attributes, permissions: permissionsInRequest } = req.body;
       const rawRequest = ensureRawRequest(req);
       const authInfo = rawRequest?.auth?.credentials?.authInfo as { user_name?: string } | null;
-      const { permissions: permissionsInAttributes, ...others } = attributes;
       let permissions: WorkspaceRoutePermissionItem[] = [];
-      if (permissionsInAttributes) {
-        permissions = Array.isArray(permissionsInAttributes)
-          ? permissionsInAttributes
-          : [permissionsInAttributes];
+      if (permissionsInRequest) {
+        permissions = Array.isArray(permissionsInRequest)
+          ? permissionsInRequest
+          : [permissionsInRequest];
       }
 
+      // Assign workspace owner to current user
       if (!!authInfo?.user_name) {
         permissions.push({
           type: 'user',
           userId: authInfo.user_name,
-          modes: [WorkspacePermissionMode.LibraryWrite],
-        });
-        permissions.push({
-          type: 'user',
-          userId: authInfo.user_name,
-          modes: [WorkspacePermissionMode.Write],
+          modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write],
         });
       }
 
@@ -215,7 +208,7 @@ export function registerRoutes({
           logger,
         },
         {
-          ...others,
+          ...attributes,
           ...(permissions.length ? { permissions: convertToACL(permissions) } : {}),
         }
       );
@@ -231,13 +224,15 @@ export function registerRoutes({
         }),
         body: schema.object({
           attributes: workspaceAttributesSchema,
+          permissions: schema.maybe(
+            schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)])
+          ),
         }),
       },
     },
     router.handleLegacyErrors(async (context, req, res) => {
       const { id } = req.params;
-      const { attributes } = req.body;
-      const { permissions, ...others } = attributes;
+      const { attributes, permissions } = req.body;
       let finalPermissions: WorkspaceRoutePermissionItem[] = [];
       if (permissions) {
         finalPermissions = Array.isArray(permissions) ? permissions : [permissions];
@@ -251,7 +246,7 @@ export function registerRoutes({
         },
         id,
         {
-          ...others,
+          ...attributes,
           ...(finalPermissions.length ? { permissions: convertToACL(finalPermissions) } : {}),
         }
       );

diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts
@@ -39,6 +39,43 @@ import {
   WORKSPACE_UPDATE_APP_ID,
 } from '../common/constants';
 
+const validatePermissionModesCombinations = [
+  [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], // Read
+  [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Read], // Write
+  [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], // Admin
+];
+
+const isValidatePermissionModesCombination = (permissionModes: string[]) =>
+  validatePermissionModesCombinations.some(
+    (combination) =>
+      combination.length === permissionModes.length &&
+      combination.every((mode) => permissionModes.includes(mode))
+  );
+const isValidatePermissions = (permissions: Permissions) => {
+  const userOrGroupKey2PermissionModes = permissions
+    ? Object.keys(permissions).reduce<{
+        [key: string]: string[];
+      }>((previousValue, permissionMode) => {
+        permissions[permissionMode].users?.forEach((user) => {
+          const key = `user-${user}`;
+          previousValue[key] = [...(previousValue[key] || []), permissionMode];
+        });
+        permissions[permissionMode].groups?.forEach((user) => {
+          const key = `group-${user}`;
+          previousValue[key] = [...(previousValue[key] || []), permissionMode];
+        });
+        return previousValue;
+      }, {})
+    : {};
+
+  for (const key in userOrGroupKey2PermissionModes) {
+    if (!isValidatePermissionModesCombination(userOrGroupKey2PermissionModes[key])) {
+      return false;
+    }
+  }
+  return true;
+};
+
 const WORKSPACE_ID_SIZE = 6;
 
 const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name.error', {
@@ -49,6 +86,10 @@ const RESERVED_WORKSPACE_NAME_ERROR = i18n.translate('workspace.reserved.name.er
   defaultMessage: 'reserved workspace name cannot be changed',
 });
 
+const INVALID_PERMISSION_MODES_COMBINATION = i18n.translate('workspace.invalid.permission.error', {
+  defaultMessage: 'Invalid workspace permission mode combination',
+});
+
 export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
   private setupDep: CoreSetup;
   private logger: Logger;
@@ -207,6 +248,11 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
       if (existingWorkspaceRes && existingWorkspaceRes.total > 0) {
         throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR);
       }
+
+      if (permissions && !isValidatePermissions(permissions)) {
+        throw new Error(INVALID_PERMISSION_MODES_COMBINATION);
+      }
+
       const result = await client.create<Omit<WorkspaceAttribute, 'id'>>(
         WORKSPACE_TYPE,
         attributes,
@@ -359,6 +405,10 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl {
         }
       }
 
+      if (permissions && !isValidatePermissions(permissions)) {
+        throw new Error(INVALID_PERMISSION_MODES_COMBINATION);
+      }
+
       await client.create<Omit<WorkspaceAttribute, 'id'>>(WORKSPACE_TYPE, attributes, {
         id,
         permissions,