From b2ab2bd22f6a6b39d833b6ac893c1e35ed95e78c Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Sun, 7 Apr 2024 13:02:44 +0800 Subject: [PATCH] Backport permission control to pr integr (#314) * [Workspace]Add permission control logic for workspace (#6052) * Add permission control for workspace Signed-off-by: Lin Wang * Add changelog for permission control in workspace Signed-off-by: Lin Wang * Fix integration tests and remove no need type Signed-off-by: Lin Wang * Update permission enabled for workspace CRUD integration tests Signed-off-by: Lin Wang * Change back to config schema Signed-off-by: Lin Wang * 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 * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe * fix: authInfo destructure (#7) * fix: authInfo destructure Signed-off-by: SuZhou-Joe * fix: unit test error Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe * Fix permissions assign in attributes Signed-off-by: Lin Wang * Remove deleteByWorkspace since not exists Signed-off-by: Lin Wang * refactor: remove formatWorkspacePermissionModeToStringArray Signed-off-by: Lin Wang * Remove current not used code Signed-off-by: Lin Wang * Add missing unit tests for permission control Signed-off-by: Lin Wang * Update workspaces API test describe Signed-off-by: Lin Wang * Fix workspace CRUD API integration tests failed Signed-off-by: Lin Wang * Address PR comments Signed-off-by: Lin Wang * Store permissions when savedObjects.permissions.enabled Signed-off-by: Lin Wang * Add permission control for deleteByWorkspace Signed-off-by: Lin Wang * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe * Refactor permissions field in workspace create and update API Signed-off-by: Lin Wang * Fix workspace CRUD API integration tests Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe Signed-off-by: Lin Wang * Convert permission settings in client side Signed-off-by: Lin Wang * Fix workspace list always render Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe --- src/core/server/index.ts | 3 +- src/core/server/mocks.ts | 3 + .../server/plugins/plugin_context.test.ts | 7 +- src/core/server/plugins/types.ts | 2 +- src/core/server/saved_objects/index.ts | 8 +- .../service/lib/repository.test.js | 15 +- .../saved_objects/service/lib/repository.ts | 4 +- src/core/types/saved_objects.ts | 2 + src/core/types/workspace.ts | 8 +- .../workspace_creator.test.tsx | 13 +- .../workspace_creator/workspace_creator.tsx | 8 +- .../public/components/workspace_form/types.ts | 4 +- .../workspace_form/use_workspace_form.ts | 24 ++- .../public/components/workspace_form/utils.ts | 76 ++++++++ .../workspace_form/workspace_form.tsx | 2 +- .../components/workspace_list/index.tsx | 9 +- .../workspace_updater.test.tsx | 9 +- .../workspace_updater/workspace_updater.tsx | 37 ++-- src/plugins/workspace/public/hooks.ts | 4 +- .../workspace/public/workspace_client.ts | 23 +-- .../server/integration_tests/routes.test.ts | 177 +++++++++++------- .../server/permission_control/client.test.ts | 77 ++++++++ .../server/permission_control/client.ts | 62 +++--- src/plugins/workspace/server/plugin.test.ts | 3 - src/plugins/workspace/server/plugin.ts | 18 +- src/plugins/workspace/server/routes/index.ts | 78 ++++---- ...space_saved_objects_client_wrapper.test.ts | 66 +++++++ ...space_saved_objects_client_wrapper.test.ts | 82 +++++++- .../workspace_saved_objects_client_wrapper.ts | 32 +--- src/plugins/workspace/server/types.ts | 30 +-- .../workspace/server/workspace_client.ts | 25 ++- 31 files changed, 627 insertions(+), 284 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 84ee65dcb19..f497bed2275 100644 --- 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 index 687d408e40a..dce39d03da7 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -89,6 +89,9 @@ export function pluginInitializerContextConfigMock(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 index 7a8ba042825..57aa372514d 100644 --- 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 index 59b9881279c..c225a24aa38 100644 --- 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 index 11809c5b88c..dccf63d4dcf 100644 --- 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 index d2658988227..087ff2e458d 100644 --- 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 index c3de94bf84b..34ff9b1e0d8 100644 --- 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 index b37309338c9..74890bb624a 100644 --- 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; } + +export type SavedObjectPermissions = Permissions; diff --git a/src/core/types/workspace.ts b/src/core/types/workspace.ts index ffad76fb48a..7cdc3f92382 100644 --- 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 index f10fd39cfe9..b67870b5529 100644 --- 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 index 2b3511f18b8..4b3e6e57c48 100644 --- 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 index 15af8596594..5f81ba3e6da 100644 --- 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 index 7158693aedf..00bee7b3ab3 100644 --- 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> >( - 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 index 133a3bc563d..8f06581b7ab 100644 --- 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((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 index ec4f2bfed3e..b340a71588c 100644 --- 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) => { diff --git a/src/plugins/workspace/public/components/workspace_list/index.tsx b/src/plugins/workspace/public/components/workspace_list/index.tsx index aac721c236a..f529524d797 100644 --- a/src/plugins/workspace/public/components/workspace_list/index.tsx +++ b/src/plugins/workspace/public/components/workspace_list/index.tsx @@ -17,7 +17,7 @@ import { import useObservable from 'react-use/lib/useObservable'; import { of } from 'rxjs'; import { i18n } from '@osd/i18n'; -import { debounce } from '../../../../../core/public'; +import { debounce, WorkspaceObject } from '../../../../../core/public'; import { WorkspaceAttribute } from '../../../../../core/public'; import { useOpenSearchDashboards } from '../../../../../plugins/opensearch_dashboards_react/public'; import { switchWorkspace, updateWorkspace } from '../utils/workspace'; @@ -34,6 +34,8 @@ const WORKSPACE_LIST_PAGE_DESCRIPTIOIN = i18n.translate('workspace.list.descript 'Workspace allow you to save and organize library items, such as index patterns, visualizations, dashboards, saved searches, and share them with other OpenSearch Dashboards users. You can control which features are visible in each workspace, and which users and groups have read and write access to the library items in the workspace.', }); +const emptyWorkspaceList: WorkspaceObject[] = []; + export const WorkspaceList = () => { const { services: { workspaces, application, http }, @@ -41,7 +43,10 @@ export const WorkspaceList = () => { const initialSortField = 'name'; const initialSortDirection = 'asc'; - const workspaceList = useObservable(workspaces?.workspaceList$ ?? of([]), []); + const workspaceList = useObservable( + workspaces?.workspaceList$ ?? of(emptyWorkspaceList), + emptyWorkspaceList + ); const [queryInput, setQueryInput] = useState(''); const [pagination, setPagination] = useState({ pageIndex: 0, diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx index 3c113a71e94..ff07076d99d 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx @@ -169,7 +169,14 @@ describe('WorkspaceUpdater', () => { description: 'test workspace description', features: expect.arrayContaining(['app1', 'app2', 'app3']), }), - 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_updater/workspace_updater.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx index dcc750f18be..1f67f2063d9 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx @@ -6,27 +6,30 @@ import React, { useCallback, useEffect, useState } from 'react'; import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { WorkspaceAttribute } from 'opensearch-dashboards/public'; import { useObservable } from 'react-use'; import { of } from 'rxjs'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form'; import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; +import { WorkspaceAttributeWithPermission } from '../../../../../core/types'; import { WorkspaceClient } from '../../workspace_client'; -import { WorkspaceFormData, WorkspacePermissionSetting } from '../workspace_form/types'; - -interface WorkspaceWithPermission extends WorkspaceAttribute { - permissions?: WorkspacePermissionSetting[]; -} +import { + convertPermissionSettingsToPermissions, + convertPermissionsToPermissionSettings, +} from '../workspace_form/utils'; function getFormDataFromWorkspace( - currentWorkspace: WorkspaceAttribute | null | undefined -): WorkspaceFormData { - const currentWorkspaceWithPermission = (currentWorkspace || {}) as WorkspaceWithPermission; + currentWorkspace: WorkspaceAttributeWithPermission | null | undefined +) { + if (!currentWorkspace) { + return null; + } return { - ...currentWorkspaceWithPermission, - permissions: currentWorkspaceWithPermission.permissions || [], + ...currentWorkspace, + permissionSettings: currentWorkspace.permissions + ? convertPermissionsToPermissionSettings(currentWorkspace.permissions) + : currentWorkspace.permissions, }; } @@ -38,7 +41,7 @@ export const WorkspaceUpdater = () => { const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null)); - const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState( + const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState( getFormDataFromWorkspace(currentWorkspace) ); @@ -59,8 +62,12 @@ export const WorkspaceUpdater = () => { } try { - const { permissions, ...attributes } = data; - result = await workspaceClient.update(currentWorkspace.id, attributes, permissions); + const { permissionSettings, ...attributes } = data; + result = await workspaceClient.update( + currentWorkspace.id, + attributes, + convertPermissionSettingsToPermissions(permissionSettings) + ); } catch (error) { notifications?.toasts.addDanger({ title: i18n.translate('workspace.update.failed', { @@ -100,7 +107,7 @@ export const WorkspaceUpdater = () => { [notifications?.toasts, currentWorkspace, http, application, workspaceClient] ); - if (!currentWorkspaceFormData.name) { + if (!currentWorkspaceFormData) { return null; } diff --git a/src/plugins/workspace/public/hooks.ts b/src/plugins/workspace/public/hooks.ts index a63dc8f83d3..4309dab2e5b 100644 --- a/src/plugins/workspace/public/hooks.ts +++ b/src/plugins/workspace/public/hooks.ts @@ -8,8 +8,10 @@ import { useMemo } from 'react'; import { of } from 'rxjs'; import { ApplicationStart, PublicAppInfo } from '../../../core/public'; +const emptyMap = new Map(); + export function useApplications(application?: ApplicationStart) { - const applications = useObservable(application?.applications$ ?? of(new Map()), new Map()); + const applications = useObservable(application?.applications$ ?? of(emptyMap), emptyMap); return useMemo(() => { const apps: PublicAppInfo[] = []; applications.forEach((app) => { diff --git a/src/plugins/workspace/public/workspace_client.ts b/src/plugins/workspace/public/workspace_client.ts index 31a08b6ae9c..76bbb618b50 100644 --- a/src/plugins/workspace/public/workspace_client.ts +++ b/src/plugins/workspace/public/workspace_client.ts @@ -11,7 +11,7 @@ import { WorkspaceAttribute, WorkspacesSetup, } from '../../../core/public'; -import { WorkspacePermissionMode } from '../common/constants'; +import { SavedObjectPermissions, WorkspaceAttributeWithPermission } from '../../../core/types'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -31,15 +31,6 @@ type IResponse = error?: string; }; -type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); - interface WorkspaceFindOptions { page?: number; perPage?: number; @@ -195,7 +186,7 @@ export class WorkspaceClient { */ public async create( attributes: Omit, - permissions?: WorkspacePermissionItem[] + permissions?: SavedObjectPermissions ): Promise>> { const path = this.getPath(); @@ -246,7 +237,7 @@ export class WorkspaceClient { options?: WorkspaceFindOptions ): Promise< IResponse<{ - workspaces: WorkspaceAttribute[]; + workspaces: WorkspaceAttributeWithPermission[]; total: number; per_page: number; page: number; @@ -263,9 +254,9 @@ export class WorkspaceClient { * Fetches a single workspace by a workspace id * * @param {string} id - * @returns {Promise>} The metadata of the workspace for the given id. + * @returns {Promise>} The metadata of the workspace for the given id. */ - public get(id: string): Promise> { + public get(id: string): Promise> { const path = this.getPath(id); return this.safeFetch(path, { method: 'GET', @@ -277,13 +268,13 @@ export class WorkspaceClient { * * @param {string} id * @param {object} attributes - * @param {WorkspacePermissionItem[]} permissions + * @param {WorkspacePermissionItem[]} permissionItems * @returns {Promise>} result for this operation */ public async update( id: string, attributes: Partial, - permissions?: WorkspacePermissionItem[] + permissions?: SavedObjectPermissions ): Promise> { const path = this.getPath(id); const body = { diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 0c6e55101b7..832c43c6639 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -5,8 +5,7 @@ import { WorkspaceAttribute } from 'src/core/types'; import * as osdTestServer from '../../../../core/test_helpers/osd_server'; -import { WORKSPACE_TYPE } from '../../../../core/server'; -import { WorkspacePermissionItem } from '../types'; +import { WORKSPACE_TYPE, Permissions } from '../../../../core/server'; const omitId = (object: T): Omit => { const { id, ...others } = object; @@ -19,7 +18,7 @@ const testWorkspace: WorkspaceAttribute = { description: 'test_workspace_description', }; -describe('workspace service', () => { +describe('workspace service api integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; let osd: osdTestServer.TestOpenSearchDashboardsUtils; @@ -36,7 +35,7 @@ describe('workspace service', () => { }, savedObjects: { permission: { - enabled: true, + enabled: false, }, }, migrations: { skip: false }, @@ -89,39 +88,6 @@ describe('workspace service', () => { expect(result.body.success).toEqual(true); expect(typeof result.body.result.id).toBe('string'); }); - it('create with permissions', async () => { - await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'invalid-type', userId: 'foo', modes: ['read'] }], - }) - .expect(400); - - const result: any = await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], - }) - .expect(200); - - expect(result.body.success).toEqual(true); - expect(typeof result.body.result.id).toBe('string'); - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).attributes.permissions - ).toEqual([ - { - modes: ['read'], - type: 'user', - userId: 'foo', - }, - ]); - }); it('get', async () => { const result = await osdTestServer.request .post(root, `/api/workspaces`) @@ -162,39 +128,6 @@ describe('workspace service', () => { expect(getResult.body.success).toEqual(true); expect(getResult.body.result.name).toEqual('updated'); }); - it('update with permissions', async () => { - const result: any = await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], - }) - .expect(200); - - await osdTestServer.request - .put(root, `/api/workspaces/${result.body.result.id}`) - .send({ - attributes: { - ...omitId(testWorkspace), - }, - permissions: [{ type: 'user', userId: 'foo', modes: ['write'] }], - }) - .expect(200); - - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).attributes.permissions - ).toEqual([ - { - modes: ['write'], - type: 'user', - userId: 'foo', - }, - ]); - }); it('delete', async () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) @@ -339,3 +272,107 @@ describe('workspace service', () => { }); }); }); + +describe('workspace service api integration test when savedObjects.permission.enabled equal true', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + let osd: osdTestServer.TestOpenSearchDashboardsUtils; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + savedObjects: { + permission: { + enabled: true, + }, + }, + migrations: { skip: false }, + }, + }, + }); + opensearchServer = await startOpenSearch(); + osd = await startOpenSearchDashboards(); + root = osd.root; + }); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + describe('Workspace CRUD APIs', () => { + afterEach(async () => { + const listResult = await osdTestServer.request + .post(root, `/api/workspaces/_list`) + .send({ + page: 1, + }) + .expect(200); + const savedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([ + WORKSPACE_TYPE, + ]); + await Promise.all( + listResult.body.result.workspaces.map((item: WorkspaceAttribute) => + // this will delete reserved workspace + savedObjectsRepository.delete(WORKSPACE_TYPE, item.id) + ) + ); + }); + it('create', async () => { + await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + permissions: { invalid_type: { users: ['foo'] } }, + }) + .expect(400); + + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + permissions: { read: { users: ['foo'] } }, + }) + .expect(200); + + expect(result.body.success).toEqual(true); + expect(typeof result.body.result.id).toBe('string'); + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ read: { users: ['foo'] } }); + }); + it('update', async () => { + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + }) + .expect(200); + + const updateResult = await osdTestServer.request + .put(root, `/api/workspaces/${result.body.result.id}`) + .send({ + attributes: { + ...omitId(testWorkspace), + }, + permissions: { write: { users: ['foo'] } }, + }) + .expect(200); + expect(updateResult.body.result).toBe(true); + + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ write: { users: ['foo'] } }); + }); + }); +}); diff --git a/src/plugins/workspace/server/permission_control/client.test.ts b/src/plugins/workspace/server/permission_control/client.test.ts index e05e299c153..4d041cc7df5 100644 --- a/src/plugins/workspace/server/permission_control/client.test.ts +++ b/src/plugins/workspace/server/permission_control/client.test.ts @@ -109,6 +109,47 @@ describe('PermissionControl', () => { expect(batchValidateResult.result).toEqual(true); }); + it('should return false and log not permitted saved objects', async () => { + const logger = loggerMock.create(); + const permissionControlClient = new SavedObjectsPermissionControl(logger); + const getScopedClient = jest.fn(); + const clientMock = savedObjectsClientMock.create(); + getScopedClient.mockImplementation((request) => { + return clientMock; + }); + permissionControlClient.setup(getScopedClient, mockAuth); + + clientMock.bulkGet.mockResolvedValue({ + saved_objects: [ + { + id: 'foo', + type: 'dashboard', + references: [], + attributes: {}, + }, + { + id: 'bar', + type: 'dashboard', + references: [], + attributes: {}, + permissions: { + read: { + users: ['foo'], + }, + }, + }, + ], + }); + const batchValidateResult = await permissionControlClient.batchValidate( + httpServerMock.createOpenSearchDashboardsRequest(), + [], + ['read'] + ); + expect(batchValidateResult.success).toEqual(true); + expect(batchValidateResult.result).toEqual(false); + expect(logger.debug).toHaveBeenCalledTimes(1); + }); + describe('getPrincipalsFromRequest', () => { const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); const getScopedClient = jest.fn(); @@ -120,4 +161,40 @@ describe('PermissionControl', () => { expect(result.users).toEqual(['bar']); }); }); + + describe('validateSavedObjectsACL', () => { + it("should return true if saved objects don't have permissions property", () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + expect( + permissionControlClient.validateSavedObjectsACL([{ type: 'workspace', id: 'foo' }], {}, []) + ).toBe(true); + }); + it('should return true if principals permitted to saved objects', () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + expect( + permissionControlClient.validateSavedObjectsACL( + [{ type: 'workspace', id: 'foo', permissions: { write: { users: ['bar'] } } }], + { users: ['bar'] }, + ['write'] + ) + ).toBe(true); + }); + it('should return false and log saved objects if not permitted', () => { + const logger = loggerMock.create(); + const permissionControlClient = new SavedObjectsPermissionControl(logger); + expect( + permissionControlClient.validateSavedObjectsACL( + [{ type: 'workspace', id: 'foo', permissions: { write: { users: ['bar'] } } }], + { users: ['foo'] }, + ['write'] + ) + ).toBe(false); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringMatching( + /Authorization failed, principals:.*has no.*permissions on the requested saved object:.*foo/ + ) + ); + }); + }); }); diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bad46eb156a..ea404e42974 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -6,7 +6,6 @@ import { i18n } from '@osd/i18n'; import { ACL, - TransformedPermission, SavedObjectsBulkGetObject, SavedObjectsServiceStart, Logger, @@ -14,7 +13,6 @@ import { Principals, SavedObject, WORKSPACE_TYPE, - Permissions, HttpAuth, } from '../../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants'; @@ -31,6 +29,13 @@ export class SavedObjectsPermissionControl { private readonly logger: Logger; private _getScopedClient?: SavedObjectsServiceStart['getScopedClient']; private auth?: HttpAuth; + /** + * Returns a saved objects client that is able to: + * 1. Read objects whose type is `workspace` because workspace is a hidden type and the permission control client will need to get the metadata of a specific workspace to do the permission check. + * 2. Bypass saved objects permission control wrapper because the permission control client is a dependency of the wrapper to provide the ACL validation capability. It will run into infinite loop if not bypass. + * @param request + * @returns SavedObjectsContract + */ private getScopedClient(request: OpenSearchDashboardsRequest) { return this._getScopedClient?.(request, { excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], @@ -83,6 +88,15 @@ export class SavedObjectsPermissionControl { return getPrincipalsFromRequest(request, this.auth); } + /** + * Validates the permissions for a collection of saved objects based on their Access Control Lists (ACL). + * This method checks whether the provided principals have the specified permission modes for each saved object. + * If any saved object lacks the required permissions, the function logs details of unauthorized access. + * + * @remarks + * If a saved object doesn't have an ACL (e.g., config objects), it is considered as having the required permissions. + * The function logs detailed information when unauthorized access is detected, including the list of denied saved objects. + */ public validateSavedObjectsACL( savedObjects: Array, 'id' | 'type' | 'workspaces' | 'permissions'>>, principals: Principals, @@ -101,7 +115,12 @@ export class SavedObjectsPermissionControl { const aclInstance = new ACL(savedObject.permissions); const hasPermission = aclInstance.hasPermission(permissionModes, principals); if (!hasPermission) { - notPermittedSavedObjects.push(savedObject); + notPermittedSavedObjects.push({ + id: savedObject.id, + type: savedObject.type, + workspaces: savedObject.workspaces, + permissions: savedObject.permissions, + }); } return hasPermission; }); @@ -145,38 +164,11 @@ export class SavedObjectsPermissionControl { } const principals = this.getPrincipalsFromRequest(request); - const deniedObjects: Array< - Pick & { - workspaces?: string[]; - permissions?: Permissions; - } - > = []; - const hasPermissionToAllObjects = savedObjectsGet.every((item) => { - // for object that doesn't contain ACL like config, return true - if (!item.permissions) { - return true; - } - const aclInstance = new ACL(item.permissions); - const hasPermission = aclInstance.hasPermission(permissionModes, principals); - if (!hasPermission) { - deniedObjects.push({ - id: item.id, - type: item.type, - workspaces: item.workspaces, - permissions: item.permissions, - }); - } - return hasPermission; - }); - if (!hasPermissionToAllObjects) { - this.logger.debug( - `Authorization failed, principals: ${JSON.stringify( - principals - )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify( - deniedObjects - )}` - ); - } + const hasPermissionToAllObjects = this.validateSavedObjectsACL( + savedObjectsGet, + principals, + permissionModes + ); return { success: true, result: hasPermissionToAllObjects, diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index c448fcf209f..684f754ce9d 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -12,9 +12,6 @@ describe('Workspace server plugin', () => { const setupMock = coreMock.createSetup(); const initializerContextConfigMock = coreMock.createPluginInitializerContext({ enabled: true, - permission: { - enabled: true, - }, }); setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn())); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 0fff0082476..8e3da6a8cfe 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,29 +11,29 @@ import { Plugin, Logger, CoreStart, + SharedGlobalConfig, } from '../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, } from '../common/constants'; -import { IWorkspaceClientImpl } from './types'; +import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { IWorkspaceClientImpl, WorkspacePluginSetup, WorkspacePluginStart } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; -import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; -import { WorkspacePluginConfigType } from '../config'; -export class WorkspacePlugin implements Plugin<{}, {}> { +export class WorkspacePlugin implements Plugin { private readonly logger: Logger; private client?: IWorkspaceClientImpl; private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; private permissionControl?: SavedObjectsPermissionControlContract; - private readonly config$: Observable; + private readonly globalConfig$: Observable; private workspaceSavedObjectsClientWrapper?: WorkspaceSavedObjectsClientWrapper; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { @@ -57,14 +57,13 @@ export class WorkspacePlugin implements Plugin<{}, {}> { constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); - this.config$ = initializerContext.config.create(); + this.globalConfig$ = initializerContext.config.legacy.globalConfig$; } public async setup(core: CoreSetup) { this.logger.debug('Setting up Workspaces service'); - const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise(); - const isPermissionControlEnabled = - config.permission.enabled === undefined ? true : config.permission.enabled; + const globalConfig = await this.globalConfig$.pipe(first()).toPromise(); + const isPermissionControlEnabled = globalConfig.savedObjects.permission.enabled === true; this.client = new WorkspaceClient(core, this.logger); @@ -99,6 +98,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { logger: this.logger, client: this.client as IWorkspaceClientImpl, permissionControlClient: this.permissionControl, + isPermissionControlEnabled, }); core.capabilities.registerProvider(() => ({ diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index b02bff76c13..701eb888813 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -4,9 +4,10 @@ */ import { schema } from '@osd/config-schema'; -import { CoreSetup, Logger } from '../../../../core/server'; +import { CoreSetup, Logger, PrincipalType, ACL } from '../../../../core/server'; +import { WorkspaceAttributeWithPermission } from '../../../../core/types'; import { WorkspacePermissionMode } from '../../common/constants'; -import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types'; +import { IWorkspaceClientImpl } from '../types'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -18,19 +19,16 @@ const workspacePermissionMode = schema.oneOf([ schema.literal(WorkspacePermissionMode.LibraryWrite), ]); -const workspacePermission = schema.oneOf([ - schema.object({ - type: schema.literal('user'), - userId: schema.string(), - modes: schema.arrayOf(workspacePermissionMode), - }), - schema.object({ - type: schema.literal('group'), - group: schema.string(), - modes: schema.arrayOf(workspacePermissionMode), - }), +const principalType = schema.oneOf([ + schema.literal(PrincipalType.Users), + schema.literal(PrincipalType.Groups), ]); +const workspacePermissions = schema.recordOf( + workspacePermissionMode, + schema.recordOf(principalType, schema.arrayOf(schema.string()), {}) +); + const workspaceAttributesSchema = schema.object({ description: schema.maybe(schema.string()), name: schema.string(), @@ -46,11 +44,13 @@ export function registerRoutes({ logger, http, permissionControlClient, + isPermissionControlEnabled, }: { client: IWorkspaceClientImpl; logger: Logger; http: CoreSetup['http']; permissionControlClient?: SavedObjectsPermissionControlContract; + isPermissionControlEnabled: boolean; }) { const router = http.createRouter(); router.post( @@ -119,29 +119,30 @@ export function registerRoutes({ validate: { body: schema.object({ attributes: workspaceAttributesSchema, - permissions: schema.maybe( - schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)]) - ), + permissions: schema.maybe(workspacePermissions), }), }, }, router.handleLegacyErrors(async (context, req, res) => { - const { attributes, permissions: permissionsInRequest } = req.body; - const authInfo = permissionControlClient?.getPrincipalsFromRequest(req); - let permissions: WorkspacePermissionItem[] = []; - if (permissionsInRequest) { - permissions = Array.isArray(permissionsInRequest) - ? permissionsInRequest - : [permissionsInRequest]; - } + const { attributes, permissions } = req.body; + const principals = permissionControlClient?.getPrincipalsFromRequest(req); + const createPayload: Omit = attributes; - // Assign workspace owner to current user - if (!!authInfo?.users?.length) { - permissions.push({ - type: 'user', - userId: authInfo.users[0], - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }); + if (isPermissionControlEnabled) { + createPayload.permissions = permissions; + // Assign workspace owner to current user + if (!!principals?.users?.length) { + const acl = new ACL(permissions); + const currentUserId = principals.users[0]; + [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite].forEach( + (permissionMode) => { + if (!acl.hasPermission([permissionMode], { users: [currentUserId] })) { + acl.addPermission([permissionMode], { users: [currentUserId] }); + } + } + ); + createPayload.permissions = acl.getPermissions(); + } } const result = await client.create( @@ -150,10 +151,7 @@ export function registerRoutes({ request: req, logger, }, - { - ...attributes, - ...(permissions.length ? { permissions } : {}), - } + createPayload ); return res.ok({ body: result }); }) @@ -167,19 +165,13 @@ export function registerRoutes({ }), body: schema.object({ attributes: workspaceAttributesSchema, - permissions: schema.maybe( - schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)]) - ), + permissions: schema.maybe(workspacePermissions), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const { id } = req.params; const { attributes, permissions } = req.body; - let finalPermissions: WorkspacePermissionItem[] = []; - if (permissions) { - finalPermissions = Array.isArray(permissions) ? permissions : [permissions]; - } const result = await client.update( { @@ -190,7 +182,7 @@ export function registerRoutes({ id, { ...attributes, - ...(finalPermissions.length ? { permissions: finalPermissions } : {}), + ...(isPermissionControlEnabled ? { permissions } : {}), } ); return res.ok({ body: result }); diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index 2a7fb0e440b..b6ea26456f0 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -527,4 +527,70 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); }); + + describe('deleteByWorkspace', () => { + it('should throw forbidden error when workspace not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.deleteByWorkspace('workspace-1'); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should be able to delete all data in permitted workspace', async () => { + const deleteWorkspaceId = 'workspace-to-delete'; + await repositoryKit.create( + internalSavedObjectsRepository, + 'workspace', + {}, + { + id: deleteWorkspaceId, + permissions: { + library_read: { users: ['foo'] }, + library_write: { users: ['foo'] }, + }, + } + ); + const dashboardIds = [ + 'inner-delete-workspace-dashboard-1', + 'inner-delete-workspace-dashboard-2', + ]; + await Promise.all( + dashboardIds.map((dashboardId) => + repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + id: dashboardId, + workspaces: [deleteWorkspaceId], + } + ) + ) + ); + + expect( + ( + await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: [deleteWorkspaceId], + }) + ).total + ).toBe(2); + + await permittedSavedObjectedClient.deleteByWorkspace(deleteWorkspaceId, { refresh: true }); + + expect( + ( + await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: [deleteWorkspaceId], + }) + ).total + ).toBe(0); + }); + }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 6b40f6e60fa..07d1e6aff40 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -26,6 +26,15 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }, permissions: {}, }, + { + type: 'dashboard', + id: 'dashboard-with-empty-workspace-property', + workspaces: [], + attributes: { + bar: 'baz', + }, + permissions: {}, + }, { type: 'workspace', id: 'workspace-1', attributes: { name: 'Workspace - 1' } }, { type: 'workspace', @@ -40,6 +49,9 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { type: 'config', }; } + if (id === 'unknown-error-dashboard') { + throw new Error('Unknown error'); + } return ( savedObjectsStore.find((item) => item.type === type && item.id === id) || SavedObjectsErrorHelpers.createGenericNotFoundError() @@ -82,14 +94,16 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { getPrincipalsFromRequest: jest.fn().mockImplementation(() => ({ users: ['user-1'] })), }; const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); - wrapper.setScopedClient(() => ({ + const scopedClientMock = { find: jest.fn().mockImplementation(async () => ({ saved_objects: [{ id: 'workspace-1', type: 'workspace' }], })), - })); + }; + wrapper.setScopedClient(() => scopedClientMock); return { wrapper: wrapper.wrapperFactory(wrapperOptions), clientMock, + scopedClientMock, permissionControlMock, requestMock, }; @@ -122,6 +136,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid saved objects permission'); }); + it("should throw permission error if deleting saved object's workspace property is empty", async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.delete('dashboard', 'dashboard-with-empty-workspace-property'); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); it('should call client.delete with arguments if permitted', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const deleteArgs = ['dashboard', 'foo', { force: true }] as const; @@ -157,6 +181,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid saved objects permission'); }); + it("should throw permission error if updating saved object's workspace property is empty", async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.update('dashboard', 'dashboard-with-empty-workspace-property', { + bar: 'foo', + }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); it('should call client.update with arguments if permitted', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const updateArgs = [ @@ -260,6 +296,26 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid workspace permission'); }); + it('should throw error if unable to get object when override', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + permissionControlMock.validate.mockResolvedValueOnce({ success: true, result: false }); + let errorCatched; + try { + await wrapper.bulkCreate( + [{ type: 'dashboard', id: 'unknown-error-dashboard', attributes: { bar: 'baz' } }], + { + overwrite: true, + } + ); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toBe('Unknown error'); + }); it('should call client.bulkCreate with arguments if some objects not found', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const objectsToBulkCreate = [ @@ -268,11 +324,9 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ]; await wrapper.bulkCreate(objectsToBulkCreate, { overwrite: true, - workspaces: ['workspace-1'], }); expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { overwrite: true, - workspaces: ['workspace-1'], }); }); it('should call client.bulkCreate with arguments if permitted', async () => { @@ -549,6 +603,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ACLSearchParams: {}, }); }); + it('should find permitted workspaces with filtered permission modes', async () => { + const { wrapper, scopedClientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'dashboard', + ACLSearchParams: { + permissionModes: ['read', 'library_read'], + }, + }); + expect(scopedClientMock.find).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'workspace', + ACLSearchParams: { + permissionModes: ['library_read'], + principals: { users: ['user-1'] }, + }, + }) + ); + }); it('should call client.find with arguments if not workspace type and no options.workspace', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); await wrapper.find({ @@ -556,8 +628,6 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }); expect(clientMock.find).toHaveBeenCalledWith({ type: 'dashboard', - workspaces: ['workspace-1'], - workspacesSearchOperator: 'OR', ACLSearchParams: { permissionModes: ['read', 'write'], principals: { users: ['user-1'] }, 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 c515f555fa4..30c1c91c422 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 @@ -22,10 +22,10 @@ import { SavedObjectsBulkUpdateResponse, SavedObjectsBulkUpdateOptions, WORKSPACE_TYPE, - SavedObjectsDeleteByWorkspaceOptions, SavedObjectsErrorHelpers, SavedObjectsServiceStart, SavedObjectsClientContract, + SavedObjectsDeleteByWorkspaceOptions, } from '../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; import { @@ -68,22 +68,13 @@ const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[] export class WorkspaceSavedObjectsClientWrapper { private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; - private formatWorkspacePermissionModeToStringArray( - permission: WorkspacePermissionMode | WorkspacePermissionMode[] - ): string[] { - if (Array.isArray(permission)) { - return permission; - } - - return [permission]; - } private async validateObjectsPermissions( objects: Array>, request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) { - // PermissionMode here is an array which is merged by workspace type required permission and other saved object required permission. + // PermissionModes here is an array which is merged by workspace type required permission and other saved object required permission. // So we only need to do one permission check no matter its type. for (const { id, type } of objects) { const validateResult = await this.permissionControl.validate( @@ -92,7 +83,7 @@ export class WorkspaceSavedObjectsClientWrapper { type, id, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (!validateResult?.result) { return false; @@ -105,20 +96,20 @@ export class WorkspaceSavedObjectsClientWrapper { private validateMultiWorkspacesPermissions = async ( workspacesIds: string[], request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) => { // for attributes and options passed in this function, the num of workspaces may be 0.This case should not be passed permission check. if (workspacesIds.length === 0) { return false; } const workspaces = workspacesIds.map((id) => ({ id, type: WORKSPACE_TYPE })); - return await this.validateObjectsPermissions(workspaces, request, permissionMode); + return await this.validateObjectsPermissions(workspaces, request, permissionModes); }; private validateAtLeastOnePermittedWorkspaces = async ( workspaces: string[] | undefined, request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) => { // for attributes and options passed in this function, the num of workspaces attribute may be 0.This case should not be passed permission check. if (!workspaces || workspaces.length === 0) { @@ -131,7 +122,7 @@ export class WorkspaceSavedObjectsClientWrapper { type: WORKSPACE_TYPE, id: workspaceId, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (validateResult?.result) { return true; @@ -495,12 +486,9 @@ export class WorkspaceSavedObjectsClientWrapper { options.workspaces = permittedWorkspaces; } else { /** - * Select all the docs that - * 1. ACL matches read / write / user passed permission OR - * 2. workspaces matches library_read or library_write OR + * If no workspaces present, find all the docs that + * ACL matches read / write / user passed permission */ - options.workspaces = permittedWorkspaceIds; - options.workspacesSearchOperator = 'OR'; options.ACLSearchParams.permissionModes = getDefaultValuesForEmpty( options.ACLSearchParams.permissionModes, [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write] diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index ba1ff8a9cd4..b506bb493a4 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -12,7 +12,7 @@ import { WorkspaceAttribute, SavedObjectsServiceStart, } from '../../../core/server'; -import { WorkspacePermissionMode } from '../common/constants'; +import { WorkspaceAttributeWithPermission } from '../../../core/types'; export interface WorkspaceFindOptions { page?: number; @@ -53,7 +53,7 @@ export interface IWorkspaceClientImpl { */ create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): Promise>; /** * List workspaces @@ -68,7 +68,7 @@ export interface IWorkspaceClientImpl { ): Promise< IResponse< { - workspaces: WorkspaceAttribute[]; + workspaces: WorkspaceAttributeWithPermission[]; } & Pick > >; @@ -76,10 +76,13 @@ export interface IWorkspaceClientImpl { * Get the detail of a given workspace id * @param requestDetail {@link IRequestDetail} * @param id workspace id - * @returns a Promise with the detail of {@link WorkspaceAttribute} + * @returns a Promise with the detail of {@link WorkspaceAttributeWithPermission} * @public */ - get(requestDetail: IRequestDetail, id: string): Promise>; + get( + requestDetail: IRequestDetail, + id: string + ): Promise>; /** * Update the detail of a given workspace * @param requestDetail {@link IRequestDetail} @@ -91,7 +94,7 @@ export interface IWorkspaceClientImpl { update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise>; /** * Delete a given workspace @@ -124,11 +127,10 @@ export interface AuthInfo { user_name?: string; } -export type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); +export interface WorkspacePluginSetup { + client: IWorkspaceClientImpl; +} + +export interface WorkspacePluginStart { + client: IWorkspaceClientImpl; +} diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index e7bdf97b54e..f9da4130921 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -4,7 +4,7 @@ */ import { i18n } from '@osd/i18n'; -import type { +import { SavedObject, SavedObjectsClientContract, CoreSetup, @@ -17,14 +17,11 @@ import { WORKSPACE_TYPE, Logger, } from '../../../core/server'; +import { WorkspaceAttributeWithPermission } from '../../../core/types'; import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { - WORKSPACE_OVERVIEW_APP_ID, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, - WORKSPACE_UPDATE_APP_ID, -} from '../common/constants'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const WORKSPACE_ID_SIZE = 6; @@ -65,10 +62,11 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } private getFlattenedResultWithSavedObject( savedObject: SavedObject - ): WorkspaceAttribute { + ): WorkspaceAttributeWithPermission { return { ...savedObject.attributes, id: savedObject.id, + permissions: savedObject.permissions, }; } private formatError(error: Error | any): string { @@ -114,10 +112,10 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } public async create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): ReturnType { try { - const attributes = payload; + const { permissions, ...attributes } = payload; const id = generateRandomId(WORKSPACE_ID_SIZE); const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const existingWorkspaceRes = await this.getScopedClientWithoutPermission(requestDetail)?.find( @@ -135,6 +133,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { attributes, { id, + permissions, } ); return { @@ -214,7 +213,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { public async get( requestDetail: IRequestDetail, id: string - ): Promise> { + ): ReturnType { try { const result = await this.getSavedObjectClientsFromRequestDetail(requestDetail).get< WorkspaceAttribute @@ -233,9 +232,9 @@ export class WorkspaceClient implements IWorkspaceClientImpl { public async update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise> { - const attributes = payload; + const { permissions, ...attributes } = payload; try { const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const workspaceInDB: SavedObject = await client.get(WORKSPACE_TYPE, id); @@ -255,9 +254,9 @@ export class WorkspaceClient implements IWorkspaceClientImpl { throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); } } - await client.create>(WORKSPACE_TYPE, attributes, { id, + permissions, overwrite: true, version: workspaceInDB.version, });