From 9bc49f5356364acb78672726a39f2f8a48625874 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 24 Oct 2023 17:15:14 +0800 Subject: [PATCH] feat: throw error when create or update with invalid permission modes Signed-off-by: Lin Wang --- .../workspace_creator/workspace_creator.tsx | 3 +- .../workspace_updater/workspace_updater.tsx | 3 +- .../workspace/public/workspace_client.ts | 14 +++--- src/plugins/workspace/server/routes/index.ts | 37 ++++++-------- .../workspace/server/workspace_client.ts | 50 +++++++++++++++++++ 5 files changed, 76 insertions(+), 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 index 6d475f43a659..22663aad1dc0 100644 --- 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 index 6a8d85d450dd..59268fa7d917 100644 --- 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 index 9f1a106a0a50..edc6d0846270 100644 --- a/src/plugins/workspace/public/workspace_client.ts +++ b/src/plugins/workspace/public/workspace_client.ts @@ -176,9 +176,8 @@ export class WorkspaceClient { * @returns */ public async create( - attributes: Omit & { - permissions: WorkspaceRoutePermissionItem[]; - } + attributes: Omit, + permissions: WorkspaceRoutePermissionItem[] ): Promise> { const path = this.getPath(); @@ -186,6 +185,7 @@ export class WorkspaceClient { 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, + permissions: WorkspaceRoutePermissionItem[] ): Promise> { 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 index 2701c2a767ab..e95717f81c08 100644 --- 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 index ff9bd9a06cb1..6402032704a5 100644 --- 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>( 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>(WORKSPACE_TYPE, attributes, { id, permissions,