From 29ccc203b591141aaf4486e361ec31acf1b93be4 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 25 Sep 2023 17:59:00 +0800 Subject: [PATCH] Feat remove management permission mode in acl permission (#195) * feat: remove management permission mode and clearify library permission mode usage Signed-off-by: Lin Wang * address PR comments and add annotations Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang --- src/core/utils/constants.ts | 1 - .../workspace_permission_setting_panel.tsx | 161 +++++++++++------- .../workspace/public/workspace_client.ts | 3 +- src/plugins/workspace/server/routes/index.ts | 10 +- .../workspace_saved_objects_client_wrapper.ts | 93 +++++----- src/plugins/workspace/server/types.ts | 3 +- 6 files changed, 147 insertions(+), 124 deletions(-) diff --git a/src/core/utils/constants.ts b/src/core/utils/constants.ts index 718996a0718f..1eecb44ae96f 100644 --- a/src/core/utils/constants.ts +++ b/src/core/utils/constants.ts @@ -10,7 +10,6 @@ export const WORKSPACE_PATH_PREFIX = '/w'; export enum WorkspacePermissionMode { Read = 'read', Write = 'write', - Management = 'management', LibraryRead = 'library_read', LibraryWrite = 'library_write', } diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx index 58e7fab0906b..3e685d968f86 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_permission_setting_panel.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useMemo, useRef } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { EuiDescribedFormGroup, EuiFlexGroup, @@ -25,45 +25,53 @@ export type WorkspacePermissionSetting = ( type: 'user' | 'group'; userId?: string; group?: string; - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Management - >; + modes: WorkspacePermissionMode[]; }; +enum PermissionModeId { + Read = 'read', + ReadAndWrite = 'read+write', + Admin = 'admin', +} + const permissionModeOptions = [ { - id: WorkspacePermissionMode.LibraryRead, - label: 'View', + id: PermissionModeId.Read, + label: 'Read', iconType: 'eye', }, { - id: WorkspacePermissionMode.LibraryWrite, - label: 'Edit', + id: PermissionModeId.ReadAndWrite, + label: 'Read + Write', iconType: 'pencil', }, { - id: WorkspacePermissionMode.Management, + id: PermissionModeId.Admin, label: 'Management', iconType: 'visTimelion', }, ]; +const optionIdToWorkspacePermissionModesMap: { + [key: string]: WorkspacePermissionMode[]; +} = { + [PermissionModeId.Read]: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + [PermissionModeId.ReadAndWrite]: [ + WorkspacePermissionMode.LibraryWrite, + WorkspacePermissionMode.Read, + ], + [PermissionModeId.Admin]: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], +}; + const permissionTypeOptions = [ { value: 'user' as const, inputDisplay: 'User' }, { value: 'group' as const, inputDisplay: 'Group' }, ]; -const isWorkspacePermissionMode = ( - test: string -): test is - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Management => - test === WorkspacePermissionMode.LibraryRead || - test === WorkspacePermissionMode.LibraryWrite || - test === WorkspacePermissionMode.Management; +const generateWorkspacePermissionItemKey = ( + item: Partial, + index?: number +) => [item.type, item.userId, item.group, ...(item.modes ?? []), index].filter(Boolean).join('-'); interface WorkspacePermissionSettingInputProps { index: number; @@ -71,11 +79,7 @@ interface WorkspacePermissionSettingInputProps { type?: 'user' | 'group'; userId?: string; group?: string; - modes?: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Management - >; + modes?: WorkspacePermissionMode[]; onTypeChange: (type: 'user' | 'group', index: number) => void; onGroupOrUserIdChange: ( groupOrUserId: @@ -85,11 +89,7 @@ interface WorkspacePermissionSettingInputProps { index: number ) => void; onPermissionModesChange: ( - WorkspacePermissionMode: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Management - >, + WorkspacePermissionMode: WorkspacePermissionMode[], index: number ) => void; onDelete: (index: number) => void; @@ -111,16 +111,16 @@ const WorkspacePermissionSettingInput = ({ () => (group || userId ? [{ label: (group || userId) as string }] : []), [group, userId] ); - const permissionModesIdToSelectMap = useMemo( - () => ({ - [WorkspacePermissionMode.LibraryRead]: !!modes?.includes(WorkspacePermissionMode.LibraryRead), - [WorkspacePermissionMode.LibraryWrite]: !!modes?.includes( - WorkspacePermissionMode.LibraryWrite - ), - [WorkspacePermissionMode.Management]: !!modes?.includes(WorkspacePermissionMode.Management), - }), - [modes] - ); + const permissionModesSelectedId = useMemo(() => { + if (!modes) { + return undefined; + } + for (const key in optionIdToWorkspacePermissionModesMap) { + if (optionIdToWorkspacePermissionModesMap[key].every((mode) => modes?.includes(mode))) { + return key; + } + } + }, [modes]); const handleTypeChange = useCallback( (newType: 'user' | 'group') => { @@ -154,16 +154,13 @@ const WorkspacePermissionSettingInput = ({ [index, type, onGroupOrUserIdChange] ); - const handlePermissionStateChange = useCallback( + const handlePermissionModeOptionChange = useCallback( (id: string) => { - if (isWorkspacePermissionMode(id)) { - onPermissionModesChange( - modes?.includes(id) ? modes.filter((value) => value !== id) : [...(modes ?? []), id], - index - ); + if (optionIdToWorkspacePermissionModesMap[id]) { + onPermissionModesChange([...optionIdToWorkspacePermissionModesMap[id]], index); } }, - [index, modes, onPermissionModesChange] + [index, onPermissionModesChange] ); const handleDelete = useCallback(() => { @@ -195,11 +192,11 @@ const WorkspacePermissionSettingInput = ({ @@ -227,18 +224,54 @@ export const WorkspacePermissionSettingPanel = ({ onChange, firstRowDeletable, }: WorkspacePermissionSettingPanelProps) => { - const valueRef = useRef(value); - valueRef.current = value; + const transformedValue = useMemo(() => { + if (!value) { + return []; + } + const result: Array> = []; + /** + * One workspace permission setting may includes multi setting options, + * for loop the workspace permission setting array to separate it to multi rows. + **/ + for (let i = 0; i < value.length; i++) { + const valueItem = value[i]; + // Incomplete workspace permission setting don't need to separate to multi rows + if ( + !valueItem.modes || + !valueItem.type || + (valueItem.type === 'user' && !valueItem.userId) || + (valueItem.type === 'group' && !valueItem.group) + ) { + result.push(valueItem); + continue; + } + /** + * For loop the option id to workspace permission modes map, + * if one settings includes all permission modes in a specific option, + * add these permission modes to the result array. + */ + for (const key in optionIdToWorkspacePermissionModesMap) { + if (!Object.prototype.hasOwnProperty.call(optionIdToWorkspacePermissionModesMap, key)) { + continue; + } + const modesForCertainPermissionId = optionIdToWorkspacePermissionModesMap[key]; + if (modesForCertainPermissionId.every((mode) => valueItem.modes?.includes(mode))) { + result.push({ ...valueItem, modes: modesForCertainPermissionId }); + } + } + } + return result; + }, [value]); const handleAddNewOne = useCallback(() => { - onChange?.([...(valueRef.current ?? []), {}]); - }, [onChange]); + onChange?.([...(transformedValue ?? []), {}]); + }, [onChange, transformedValue]); const handleDelete = useCallback( (index: number) => { - onChange?.((valueRef.current ?? []).filter((_item, itemIndex) => itemIndex !== index)); + onChange?.((transformedValue ?? []).filter((_item, itemIndex) => itemIndex !== index)); }, - [onChange] + [onChange, transformedValue] ); const handlePermissionModesChange = useCallback< @@ -246,23 +279,23 @@ export const WorkspacePermissionSettingPanel = ({ >( (modes, index) => { onChange?.( - (valueRef.current ?? []).map((item, itemIndex) => + (transformedValue ?? []).map((item, itemIndex) => index === itemIndex ? { ...item, modes } : item ) ); }, - [onChange] + [onChange, transformedValue] ); const handleTypeChange = useCallback( (type, index) => { onChange?.( - (valueRef.current ?? []).map((item, itemIndex) => + (transformedValue ?? []).map((item, itemIndex) => index === itemIndex ? { ...item, type } : item ) ); }, - [onChange] + [onChange, transformedValue] ); const handleGroupOrUserIdChange = useCallback< @@ -270,20 +303,20 @@ export const WorkspacePermissionSettingPanel = ({ >( (userOrGroupIdWithType, index) => { onChange?.( - (valueRef.current ?? []).map((item, itemIndex) => + (transformedValue ?? []).map((item, itemIndex) => index === itemIndex ? { ...userOrGroupIdWithType, ...(item.modes ? { modes: item.modes } : {}) } : item ) ); }, - [onChange] + [onChange, transformedValue] ); return ( Users, User Groups & Groups}> - {value?.map((item, index) => ( - + {transformedValue?.map((item, index) => ( + ; } & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index f017c385e7d0..21475ede368b 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -21,7 +21,8 @@ const workspacePermissionMode = schema.oneOf([ schema.literal(WorkspacePermissionMode.Write), schema.literal(WorkspacePermissionMode.LibraryRead), schema.literal(WorkspacePermissionMode.LibraryWrite), - schema.literal(WorkspacePermissionMode.Management), + schema.literal(WorkspacePermissionMode.Read), + schema.literal(WorkspacePermissionMode.Write), ]); const workspacePermission = schema.oneOf([ @@ -187,7 +188,12 @@ export function registerRoutes({ permissions.push({ type: 'user', userId: authInfo.user_name, - modes: [WorkspacePermissionMode.Management], + modes: [WorkspacePermissionMode.LibraryWrite], + }); + permissions.push({ + type: 'user', + userId: authInfo.user_name, + modes: [WorkspacePermissionMode.Write], }); } diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index e38e44d0fa15..f277bd177663 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -32,12 +32,6 @@ import { import { SavedObjectsPermissionControlContract } from '../permission_control/client'; import { getPrincipalsFromRequest } from '../utils'; -const ALL_WORKSPACE_INNER_DATA_PERMISSION_MODES: string[] = [ - WorkspacePermissionMode.LibraryRead, - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Management, -]; - // Can't throw unauthorized for now, the page will be refreshed if unauthorized const generateWorkspacePermissionError = () => SavedObjectsErrorHelpers.decorateForbiddenError( @@ -193,8 +187,8 @@ export class WorkspaceSavedObjectsClientWrapper { !(await this.validateWorkspacesAndSavedObjectsPermissions( objectToDeleted, wrapperOptions.request, - [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management], - [WorkspacePermissionMode.Management, WorkspacePermissionMode.Write] + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write] )) ) { throw generateSavedObjectsPermissionError(); @@ -209,8 +203,8 @@ export class WorkspaceSavedObjectsClientWrapper { return await this.validateWorkspacesAndSavedObjectsPermissions( objectToUpdate, wrapperOptions.request, - [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management], - [WorkspacePermissionMode.Management, WorkspacePermissionMode.Write], + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write], false ); }; @@ -253,7 +247,7 @@ export class WorkspaceSavedObjectsClientWrapper { const permitted = await this.validateMultiWorkspacesPermissions( options.workspaces, wrapperOptions.request, - [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management] + [WorkspacePermissionMode.LibraryWrite] ); if (!permitted) { throw generateSavedObjectsPermissionError(); @@ -273,7 +267,7 @@ export class WorkspaceSavedObjectsClientWrapper { !(await this.validateMultiWorkspacesPermissions( options.workspaces, wrapperOptions.request, - [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management] + [WorkspacePermissionMode.LibraryWrite] )) ) { throw generateWorkspacePermissionError(); @@ -285,8 +279,8 @@ export class WorkspaceSavedObjectsClientWrapper { !(await this.validateWorkspacesAndSavedObjectsPermissions( await wrapperOptions.client.get(type, options.id), wrapperOptions.request, - [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management], - [WorkspacePermissionMode.Write, WorkspacePermissionMode.Management], + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write], false )) ) { @@ -307,17 +301,8 @@ export class WorkspaceSavedObjectsClientWrapper { !(await this.validateWorkspacesAndSavedObjectsPermissions( objectToGet, wrapperOptions.request, - [ - WorkspacePermissionMode.LibraryRead, - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Management, - ], - [ - WorkspacePermissionMode.LibraryRead, - WorkspacePermissionMode.Management, - WorkspacePermissionMode.Read, - WorkspacePermissionMode.Write, - ], + [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write], false )) ) { @@ -337,17 +322,8 @@ export class WorkspaceSavedObjectsClientWrapper { !(await this.validateWorkspacesAndSavedObjectsPermissions( object, wrapperOptions.request, - [ - WorkspacePermissionMode.LibraryRead, - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Management, - ], - [ - WorkspacePermissionMode.LibraryRead, - WorkspacePermissionMode.Management, - WorkspacePermissionMode.Write, - WorkspacePermissionMode.Read, - ], + [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write, WorkspacePermissionMode.Read], false )) ) { @@ -365,15 +341,13 @@ export class WorkspaceSavedObjectsClientWrapper { if (!options.ACLSearchParams) { options.ACLSearchParams = {}; } - const workspaceInnerPermissionModes = options.ACLSearchParams.permissionModes - ? intersection( - options.ACLSearchParams.permissionModes, - ALL_WORKSPACE_INNER_DATA_PERMISSION_MODES - ) - : ALL_WORKSPACE_INNER_DATA_PERMISSION_MODES; if (this.isRelatedToWorkspace(options.type)) { - options.ACLSearchParams.permissionModes = workspaceInnerPermissionModes; + // Find all "read" saved objects by object's ACL for default + options.ACLSearchParams.permissionModes = options.ACLSearchParams.permissionModes ?? [ + WorkspacePermissionMode.Read, + WorkspacePermissionMode.Write, + ]; options.ACLSearchParams.principals = principals; } else { const permittedWorkspaceIds = ( @@ -382,7 +356,19 @@ export class WorkspaceSavedObjectsClientWrapper { perPage: 999, ACLSearchParams: { principals, - permissionModes: workspaceInnerPermissionModes, + /** + * The permitted workspace ids will be passed to the options.workspaces + * or options.ACLSearchParams.workspaces. These two were indicated the saved + * objects data inner specific workspaces. We use Library related permission here. + * For outside passed permission modes, it may contains other permissions. Add a intersection + * here to make sure only Library related permission modes will be used. + */ + permissionModes: options.ACLSearchParams.permissionModes + ? intersection(options.ACLSearchParams.permissionModes, [ + WorkspacePermissionMode.LibraryRead, + WorkspacePermissionMode.LibraryWrite, + ]) + : [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], }, }) ).saved_objects.map((item) => item.id); @@ -412,18 +398,16 @@ export class WorkspaceSavedObjectsClientWrapper { } else { /** * Select all the docs that - * 1. ACL matches read or write permission OR - * 2. workspaces matches library_read or library_write or management OR + * 1. ACL matches read / write / user passed permission OR + * 2. workspaces matches library_read or library_write OR * 3. Advanced settings */ options.workspaces = undefined; options.ACLSearchParams.workspaces = permittedWorkspaceIds; - options.ACLSearchParams.permissionModes = options.ACLSearchParams.permissionModes - ? intersection(options.ACLSearchParams.permissionModes, [ - WorkspacePermissionMode.Read, - WorkspacePermissionMode.Write, - ]) - : [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write]; + options.ACLSearchParams.permissionModes = options.ACLSearchParams.permissionModes ?? [ + WorkspacePermissionMode.Read, + WorkspacePermissionMode.Write, + ]; options.ACLSearchParams.principals = principals; } } @@ -440,7 +424,7 @@ export class WorkspaceSavedObjectsClientWrapper { const workspacePermitted = await this.validateMultiWorkspacesPermissions( targetWorkspaces, wrapperOptions.request, - [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Management] + [WorkspacePermissionMode.LibraryWrite] ); if (!workspacePermitted) { throw generateWorkspacePermissionError(); @@ -467,8 +451,7 @@ export class WorkspaceSavedObjectsClientWrapper { options: SavedObjectsDeleteByWorkspaceOptions = {} ) => { await this.validateMultiWorkspacesPermissions([workspace], wrapperOptions.request, [ - WorkspacePermissionMode.LibraryWrite, - WorkspacePermissionMode.Management, + WorkspacePermissionMode.Write, ]); return await wrapperOptions.client.deleteByWorkspace(workspace, options); diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index 5762c7d08ca5..a0e6c7c2025e 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -78,7 +78,8 @@ export type WorkspaceRoutePermissionItem = { modes: Array< | WorkspacePermissionMode.LibraryRead | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Management + | WorkspacePermissionMode.Read + | WorkspacePermissionMode.Write >; } & ({ type: 'user'; userId: string } | { type: 'group'; group: string });