From f00c8f71c147961903375ddad971a27c117b1dfb Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 6 Mar 2024 14:21:44 +0800 Subject: [PATCH 01/21] Add permission control for workspace Signed-off-by: Lin Wang --- .../saved_objects/saved_objects_client.ts | 6 +- src/core/server/index.ts | 5 + 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 + .../saved_objects/service/lib/repository.ts | 4 + .../lib/search_dsl/query_params.test.ts | 125 ++++ .../service/lib/search_dsl/query_params.ts | 80 ++- .../service/lib/search_dsl/search_dsl.ts | 7 + src/core/server/saved_objects/types.ts | 10 + src/plugins/workspace/common/constants.ts | 7 + src/plugins/workspace/config.ts | 2 +- .../server/integration_tests/routes.test.ts | 76 ++- .../server/permission_control/client.mock.ts | 12 + .../server/permission_control/client.test.ts | 123 ++++ .../server/permission_control/client.ts | 185 ++++++ src/plugins/workspace/server/plugin.test.ts | 28 + src/plugins/workspace/server/plugin.ts | 47 +- src/plugins/workspace/server/routes/index.ts | 67 +- .../workspace/server/saved_objects/index.ts | 1 + ...space_saved_objects_client_wrapper.test.ts | 528 ++++++++++++++++ ...space_saved_objects_client_wrapper.test.ts | 597 ++++++++++++++++++ .../workspace_saved_objects_client_wrapper.ts | 549 ++++++++++++++++ src/plugins/workspace/server/types.ts | 15 + src/plugins/workspace/server/utils.test.ts | 53 +- src/plugins/workspace/server/utils.ts | 39 ++ 27 files changed, 2562 insertions(+), 24 deletions(-) create mode 100644 src/plugins/workspace/server/permission_control/client.mock.ts create mode 100644 src/plugins/workspace/server/permission_control/client.test.ts create mode 100644 src/plugins/workspace/server/permission_control/client.ts create mode 100644 src/plugins/workspace/server/plugin.test.ts create mode 100644 src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts create mode 100644 src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts create mode 100644 src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index d6b6b6b6d89c..12c1170d08ce 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -45,7 +45,11 @@ import { HttpFetchOptions, HttpSetup } from '../http'; type SavedObjectsFindOptions = Omit< SavedObjectFindOptionsServer, - 'sortOrder' | 'rootSearchFields' | 'typeToNamespacesMap' + | 'sortOrder' + | 'rootSearchFields' + | 'typeToNamespacesMap' + | 'ACLSearchParams' + | 'workspacesSearchOperator' >; type PromiseType> = T extends Promise ? U : never; diff --git a/src/core/server/index.ts b/src/core/server/index.ts index f8526a96a7c2..0ff5f74a4d78 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -321,6 +321,11 @@ export { exportSavedObjectsToStream, importSavedObjectsFromStream, resolveSavedObjectsImportErrors, + ACL, + Principals, + TransformedPermission, + PrincipalType, + Permissions, } from './saved_objects'; export { diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 687d408e40a6..dce39d03da7f 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 7a8ba042825b..57aa372514de 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 59b9881279c3..c225a24aa386 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 06b2b65fd184..11809c5b88c9 100644 --- a/src/core/server/saved_objects/index.ts +++ b/src/core/server/saved_objects/index.ts @@ -84,3 +84,11 @@ 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'; diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 5340008f06a6..81a1b578257a 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -749,6 +749,8 @@ export class SavedObjectsRepository { filter, preference, workspaces, + workspacesSearchOperator, + ACLSearchParams, } = options; if (!type && !typeToNamespacesMap) { @@ -823,6 +825,8 @@ export class SavedObjectsRepository { hasReference, kueryNode, workspaces, + workspacesSearchOperator, + ACLSearchParams, }), }, }; diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index a47bc27fcd92..5af816a1d8f5 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -646,6 +646,131 @@ describe('#getQueryParams', () => { }); }); }); + + describe('when using ACLSearchParams search', () => { + it('no ACLSearchParams provided', () => { + const result: Result = getQueryParams({ + registry, + ACLSearchParams: {}, + }); + expect(result.query.bool.filter[1]).toEqual(undefined); + }); + + it('workspacesSearchOperator prvided as "OR"', () => { + const result: Result = getQueryParams({ + registry, + workspaces: ['foo'], + workspacesSearchOperator: 'OR', + }); + expect(result.query.bool.filter[1]).toEqual({ + bool: { + should: [ + { + bool: { + must_not: [ + { + exists: { + field: 'workspaces', + }, + }, + { + exists: { + field: 'permissions', + }, + }, + ], + }, + }, + { + bool: { + minimum_should_match: 1, + should: [ + { + bool: { + must: [ + { + term: { + workspaces: 'foo', + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }); + }); + + it('principals and permissionModes provided in ACLSearchParams', () => { + const result: Result = getQueryParams({ + registry, + ACLSearchParams: { + principals: { + users: ['user-foo'], + groups: ['group-foo'], + }, + permissionModes: ['read'], + }, + }); + expect(result.query.bool.filter[1]).toEqual({ + bool: { + should: [ + { + bool: { + must_not: [ + { + exists: { + field: 'workspaces', + }, + }, + { + exists: { + field: 'permissions', + }, + }, + ], + }, + }, + { + bool: { + filter: [ + { + bool: { + should: [ + { + terms: { + 'permissions.read.users': ['user-foo'], + }, + }, + { + term: { + 'permissions.read.users': '*', + }, + }, + { + terms: { + 'permissions.read.groups': ['group-foo'], + }, + }, + { + term: { + 'permissions.read.groups': '*', + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }); + }); + }); }); describe('namespaces property', () => { diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index b78c5a032992..abbef0850dba 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -34,6 +34,8 @@ type KueryNode = any; import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; +import { SavedObjectsFindOptions } from '../../../types'; +import { ACL } from '../../../permission_control/acl'; /** * Gets the types based on the type. Uses mappings to support @@ -166,6 +168,8 @@ interface QueryParams { hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; workspaces?: string[]; + workspacesSearchOperator?: 'AND' | 'OR'; + ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } export function getClauseForReference(reference: HasReferenceQueryParams) { @@ -223,6 +227,8 @@ export function getQueryParams({ hasReference, kueryNode, workspaces, + workspacesSearchOperator = 'AND', + ACLSearchParams, }: QueryParams) { const types = getTypes( registry, @@ -247,17 +253,6 @@ export function getQueryParams({ ], }; - if (workspaces) { - bool.filter.push({ - bool: { - should: workspaces.map((workspace) => { - return getClauseForWorkspace(workspace); - }), - minimum_should_match: 1, - }, - }); - } - if (search) { const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search); const simpleQueryStringClause = getSimpleQueryStringClause({ @@ -279,6 +274,69 @@ export function getQueryParams({ } } + const ACLSearchParamsShouldClause: any = []; + + if (ACLSearchParams) { + if (ACLSearchParams.permissionModes?.length && ACLSearchParams.principals) { + const permissionDSL = ACL.generateGetPermittedSavedObjectsQueryDSL( + ACLSearchParams.permissionModes, + ACLSearchParams.principals + ); + ACLSearchParamsShouldClause.push(permissionDSL.query); + } + } + + if (workspaces?.length) { + if (workspacesSearchOperator === 'OR') { + ACLSearchParamsShouldClause.push({ + bool: { + should: workspaces.map((workspace) => { + return getClauseForWorkspace(workspace); + }), + minimum_should_match: 1, + }, + }); + } else { + bool.filter.push({ + bool: { + should: workspaces.map((workspace) => { + return getClauseForWorkspace(workspace); + }), + minimum_should_match: 1, + }, + }); + } + } + + if (ACLSearchParamsShouldClause.length) { + bool.filter.push({ + bool: { + should: [ + /** + * Return those objects without workspaces field and permissions field to keep find API backward compatible + */ + { + bool: { + must_not: [ + { + exists: { + field: 'workspaces', + }, + }, + { + exists: { + field: 'permissions', + }, + }, + ], + }, + }, + ...ACLSearchParamsShouldClause, + ], + }, + }); + } + return { query: { bool } }; } diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index df6109eb9d0a..fa4311576638 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -34,6 +34,7 @@ import { IndexMapping } from '../../../mappings'; import { getQueryParams } from './query_params'; import { getSortingParams } from './sorting_params'; import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; +import { SavedObjectsFindOptions } from '../../../types'; type KueryNode = any; @@ -53,6 +54,8 @@ interface GetSearchDslOptions { }; kueryNode?: KueryNode; workspaces?: string[]; + workspacesSearchOperator?: 'AND' | 'OR'; + ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } export function getSearchDsl( @@ -73,6 +76,8 @@ export function getSearchDsl( hasReference, kueryNode, workspaces, + workspacesSearchOperator, + ACLSearchParams, } = options; if (!type) { @@ -96,6 +101,8 @@ export function getSearchDsl( hasReference, kueryNode, workspaces, + workspacesSearchOperator, + ACLSearchParams, }), ...getSortingParams(mappings, type, sortField, sortOrder), }; diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 4ab6978a3dc1..d21421dbe253 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -45,6 +45,7 @@ export { } from './import/types'; import { SavedObject } from '../../types'; +import { Principals } from './permission_control/acl'; type KueryNode = any; @@ -112,6 +113,15 @@ export interface SavedObjectsFindOptions { preference?: string; /** If specified, will only retrieve objects that are in the workspaces */ workspaces?: string[]; + /** By default the operator will be 'AND' */ + workspacesSearchOperator?: 'AND' | 'OR'; + /** + * The params here will be combined with bool clause and is used for filtering with ACL structure. + */ + ACLSearchParams?: { + principals?: Principals; + permissionModes?: string[]; + }; } /** diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index e60bb6aea0eb..15c40c2848c9 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -6,3 +6,10 @@ export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace'; export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace_conflict_control'; + +export enum WorkspacePermissionMode { + Read = 'read', + Write = 'write', + LibraryRead = 'library_read', + LibraryWrite = 'library_write', +} diff --git a/src/plugins/workspace/config.ts b/src/plugins/workspace/config.ts index 79412f5c02ee..0ee3d9fbf60d 100644 --- a/src/plugins/workspace/config.ts +++ b/src/plugins/workspace/config.ts @@ -9,4 +9,4 @@ export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: false }), }); -export type ConfigSchema = TypeOf; +export type WorkspacePluginConfigType = TypeOf; diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index e14aa3de16a3..c1895986ec09 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -5,6 +5,8 @@ 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'; const omitId = (object: T): Omit => { const { id, ...others } = object; @@ -20,6 +22,7 @@ const testWorkspace: WorkspaceAttribute = { describe('workspace service', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; + let osd: osdTestServer.TestOpenSearchDashboardsUtils; beforeAll(async () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), @@ -27,13 +30,16 @@ describe('workspace service', () => { osd: { workspace: { enabled: true, + permission: { + enabled: false, + }, }, }, }, }); opensearchServer = await startOpenSearch(); - const startOSDResp = await startOpenSearchDashboards(); - root = startOSDResp.root; + osd = await startOpenSearchDashboards(); + root = osd.root; }); afterAll(async () => { await root.shutdown(); @@ -71,6 +77,39 @@ 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`) @@ -111,6 +150,39 @@ 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`) diff --git a/src/plugins/workspace/server/permission_control/client.mock.ts b/src/plugins/workspace/server/permission_control/client.mock.ts new file mode 100644 index 000000000000..687e93de1d71 --- /dev/null +++ b/src/plugins/workspace/server/permission_control/client.mock.ts @@ -0,0 +1,12 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { SavedObjectsPermissionControlContract } from './client'; + +export const savedObjectsPermissionControlMock: SavedObjectsPermissionControlContract = { + validate: jest.fn(), + batchValidate: jest.fn(), + getPrincipalsOfObjects: jest.fn(), + setup: jest.fn(), +}; diff --git a/src/plugins/workspace/server/permission_control/client.test.ts b/src/plugins/workspace/server/permission_control/client.test.ts new file mode 100644 index 000000000000..e05e299c153b --- /dev/null +++ b/src/plugins/workspace/server/permission_control/client.test.ts @@ -0,0 +1,123 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { loggerMock } from '@osd/logging/target/mocks'; +import { SavedObjectsPermissionControl } from './client'; +import { + httpServerMock, + httpServiceMock, + savedObjectsClientMock, +} from '../../../../core/server/mocks'; +import * as utilsExports from '../utils'; + +describe('PermissionControl', () => { + jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation(() => ({ + users: ['bar'], + })); + const mockAuth = httpServiceMock.createAuth(); + + it('validate should return error when no saved objects can be found', async () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + const getScopedClient = jest.fn(); + const clientMock = savedObjectsClientMock.create(); + getScopedClient.mockImplementation((request) => { + return clientMock; + }); + permissionControlClient.setup(getScopedClient, mockAuth); + clientMock.bulkGet.mockResolvedValue({ + saved_objects: [], + }); + const result = await permissionControlClient.validate( + httpServerMock.createOpenSearchDashboardsRequest(), + { id: 'foo', type: 'dashboard' }, + ['read'] + ); + expect(result.success).toEqual(false); + }); + + it('validate should return error when bulkGet return error', async () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + 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: {}, + error: { + error: 'error_bar', + message: 'error_bar', + statusCode: 500, + }, + }, + ], + }); + const errorResult = await permissionControlClient.validate( + httpServerMock.createOpenSearchDashboardsRequest(), + { id: 'foo', type: 'dashboard' }, + ['read'] + ); + expect(errorResult.success).toEqual(false); + expect(errorResult.error).toEqual('error_bar'); + }); + + it('validate should return success normally', async () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + 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: ['bar'], + }, + }, + }, + ], + }); + const batchValidateResult = await permissionControlClient.batchValidate( + httpServerMock.createOpenSearchDashboardsRequest(), + [], + ['read'] + ); + expect(batchValidateResult.success).toEqual(true); + expect(batchValidateResult.result).toEqual(true); + }); + + describe('getPrincipalsFromRequest', () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + const getScopedClient = jest.fn(); + permissionControlClient.setup(getScopedClient, mockAuth); + + it('should return normally when calling getPrincipalsFromRequest', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + const result = permissionControlClient.getPrincipalsFromRequest(mockRequest); + expect(result.users).toEqual(['bar']); + }); + }); +}); diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts new file mode 100644 index 000000000000..bad46eb156a6 --- /dev/null +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -0,0 +1,185 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { i18n } from '@osd/i18n'; +import { + ACL, + TransformedPermission, + SavedObjectsBulkGetObject, + SavedObjectsServiceStart, + Logger, + OpenSearchDashboardsRequest, + Principals, + SavedObject, + WORKSPACE_TYPE, + Permissions, + HttpAuth, +} from '../../../../core/server'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants'; +import { getPrincipalsFromRequest } from '../utils'; + +export type SavedObjectsPermissionControlContract = Pick< + SavedObjectsPermissionControl, + keyof SavedObjectsPermissionControl +>; + +export type SavedObjectsPermissionModes = string[]; + +export class SavedObjectsPermissionControl { + private readonly logger: Logger; + private _getScopedClient?: SavedObjectsServiceStart['getScopedClient']; + private auth?: HttpAuth; + private getScopedClient(request: OpenSearchDashboardsRequest) { + return this._getScopedClient?.(request, { + excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + includedHiddenTypes: [WORKSPACE_TYPE], + }); + } + + constructor(logger: Logger) { + this.logger = logger; + } + + private async bulkGetSavedObjects( + request: OpenSearchDashboardsRequest, + savedObjects: SavedObjectsBulkGetObject[] + ) { + return (await this.getScopedClient?.(request)?.bulkGet(savedObjects))?.saved_objects || []; + } + public async setup(getScopedClient: SavedObjectsServiceStart['getScopedClient'], auth: HttpAuth) { + this._getScopedClient = getScopedClient; + this.auth = auth; + } + public async validate( + request: OpenSearchDashboardsRequest, + savedObject: SavedObjectsBulkGetObject, + permissionModes: SavedObjectsPermissionModes + ) { + return await this.batchValidate(request, [savedObject], permissionModes); + } + + private logNotPermitted( + savedObjects: Array, 'id' | 'type' | 'workspaces' | 'permissions'>>, + principals: Principals, + permissionModes: SavedObjectsPermissionModes + ) { + this.logger.debug( + `Authorization failed, principals: ${JSON.stringify( + principals + )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify( + savedObjects.map((savedObject) => ({ + id: savedObject.id, + type: savedObject.type, + workspaces: savedObject.workspaces, + permissions: savedObject.permissions, + })) + )}` + ); + } + + public getPrincipalsFromRequest(request: OpenSearchDashboardsRequest) { + return getPrincipalsFromRequest(request, this.auth); + } + + public validateSavedObjectsACL( + savedObjects: Array, 'id' | 'type' | 'workspaces' | 'permissions'>>, + principals: Principals, + permissionModes: SavedObjectsPermissionModes + ) { + const notPermittedSavedObjects: Array, + 'id' | 'type' | 'workspaces' | 'permissions' + >> = []; + const hasPermissionToAllObjects = savedObjects.every((savedObject) => { + // for object that doesn't contain ACL like config, return true + if (!savedObject.permissions) { + return true; + } + + const aclInstance = new ACL(savedObject.permissions); + const hasPermission = aclInstance.hasPermission(permissionModes, principals); + if (!hasPermission) { + notPermittedSavedObjects.push(savedObject); + } + return hasPermission; + }); + if (!hasPermissionToAllObjects) { + this.logNotPermitted(notPermittedSavedObjects, principals, permissionModes); + } + return hasPermissionToAllObjects; + } + + /** + * Performs batch validation to check if the current request has access to specified saved objects + * with the given permission modes. + * @param request + * @param savedObjects + * @param permissionModes + * @returns + */ + public async batchValidate( + request: OpenSearchDashboardsRequest, + savedObjects: SavedObjectsBulkGetObject[], + permissionModes: SavedObjectsPermissionModes + ) { + const savedObjectsGet = await this.bulkGetSavedObjects(request, savedObjects); + if (!savedObjectsGet.length) { + return { + success: false, + error: i18n.translate('savedObjects.permission.notFound', { + defaultMessage: 'Can not find target saved objects.', + }), + }; + } + + if (savedObjectsGet.some((item) => item.error)) { + return { + success: false, + error: savedObjectsGet + .filter((item) => item.error) + .map((item) => item.error?.error) + .join('\n'), + }; + } + + 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 + )}` + ); + } + return { + success: true, + result: hasPermissionToAllObjects, + }; + } +} diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts new file mode 100644 index 000000000000..684f754ce9dd --- /dev/null +++ b/src/plugins/workspace/server/plugin.test.ts @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { coreMock } from '../../../core/server/mocks'; +import { WorkspacePlugin } from './plugin'; + +describe('Workspace server plugin', () => { + it('#setup', async () => { + let value; + const setupMock = coreMock.createSetup(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + }); + setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn())); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + expect(value).toMatchInlineSnapshot(` + Object { + "workspaces": Object { + "enabled": true, + "permissionEnabled": true, + }, + } + `); + }); +}); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index e4ed75bad615..e263e2141c94 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -3,30 +3,50 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { Observable } from 'rxjs'; +import { first } from 'rxjs/operators'; import { PluginInitializerContext, CoreSetup, 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 { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; -import { WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; +import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; +import { + SavedObjectsPermissionControl, + SavedObjectsPermissionControlContract, +} from './permission_control/client'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; private client?: IWorkspaceClientImpl; private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; + private permissionControl?: SavedObjectsPermissionControlContract; + private readonly globalConfig$: Observable; + private workspaceSavedObjectsClientWrapper?: WorkspaceSavedObjectsClientWrapper; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); + this.globalConfig$ = initializerContext.config.legacy.globalConfig$; } public async setup(core: CoreSetup) { this.logger.debug('Setting up Workspaces service'); + const globalConfig = await this.globalConfig$.pipe(first()).toPromise(); + const isPermissionControlEnabled = + globalConfig.savedObjects.permission.enabled === undefined + ? true + : globalConfig.savedObjects.permission.enabled; this.client = new WorkspaceClient(core); @@ -40,12 +60,35 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.workspaceConflictControl.wrapperFactory ); + this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled); + if (isPermissionControlEnabled) { + this.permissionControl = new SavedObjectsPermissionControl(this.logger); + + this.workspaceSavedObjectsClientWrapper = new WorkspaceSavedObjectsClientWrapper( + this.permissionControl + ); + + core.savedObjects.addClientWrapper( + 0, + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + this.workspaceSavedObjectsClientWrapper.wrapperFactory + ); + } + registerRoutes({ http: core.http, logger: this.logger, client: this.client as IWorkspaceClientImpl, + permissionControlClient: this.permissionControl, }); + core.capabilities.registerProvider(() => ({ + workspaces: { + enabled: true, + permissionEnabled: isPermissionControlEnabled, + }, + })); + return { client: this.client, }; @@ -53,8 +96,10 @@ export class WorkspacePlugin implements Plugin<{}, {}> { public start(core: CoreStart) { this.logger.debug('Starting Workspace service'); + this.permissionControl?.setup(core.savedObjects.getScopedClient, core.http.auth); this.client?.setSavedObjects(core.savedObjects); this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer()); + this.workspaceSavedObjectsClientWrapper?.setScopedClient(core.savedObjects.getScopedClient); return { client: this.client as IWorkspaceClientImpl, diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 5789aa0481fa..bccee7b0d9b9 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -5,10 +5,32 @@ import { schema } from '@osd/config-schema'; import { CoreSetup, Logger } from '../../../../core/server'; -import { IWorkspaceClientImpl } from '../types'; +import { WorkspacePermissionMode } from '../../common/constants'; +import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types'; +import { SavedObjectsPermissionControlContract } from '../permission_control/client'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; +const workspacePermissionMode = schema.oneOf([ + schema.literal(WorkspacePermissionMode.Read), + schema.literal(WorkspacePermissionMode.Write), + schema.literal(WorkspacePermissionMode.LibraryRead), + 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 workspaceAttributesSchema = schema.object({ description: schema.maybe(schema.string()), name: schema.string(), @@ -22,10 +44,12 @@ export function registerRoutes({ client, logger, http, + permissionControlClient, }: { client: IWorkspaceClientImpl; logger: Logger; http: CoreSetup['http']; + permissionControlClient?: SavedObjectsPermissionControlContract; }) { const router = http.createRouter(); router.post( @@ -39,6 +63,7 @@ export function registerRoutes({ page: schema.number({ min: 0, defaultValue: 1 }), sortField: schema.maybe(schema.string()), searchFields: schema.maybe(schema.arrayOf(schema.string())), + permissionModes: schema.maybe(schema.arrayOf(workspacePermissionMode)), }), }, }, @@ -90,11 +115,30 @@ 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 authInfo = permissionControlClient?.getPrincipalsFromRequest(req); + let permissions: WorkspacePermissionItem[] = []; + if (permissionsInRequest) { + permissions = Array.isArray(permissionsInRequest) + ? permissionsInRequest + : [permissionsInRequest]; + } + + // Assign workspace owner to current user + if (!!authInfo?.users?.length) { + permissions.push({ + type: 'user', + userId: authInfo.users[0], + modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], + }); + } const result = await client.create( { @@ -102,7 +146,10 @@ export function registerRoutes({ request: req, logger, }, - attributes + { + ...attributes, + ...(permissions.length ? { permissions } : {}), + } ); return res.ok({ body: result }); }) @@ -116,12 +163,19 @@ 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 { attributes, permissions } = req.body; + let finalPermissions: WorkspacePermissionItem[] = []; + if (permissions) { + finalPermissions = Array.isArray(permissions) ? permissions : [permissions]; + } const result = await client.update( { @@ -130,7 +184,10 @@ export function registerRoutes({ logger, }, id, - attributes + { + ...attributes, + ...(finalPermissions.length ? { permissions: finalPermissions } : {}), + } ); return res.ok({ body: result }); }) diff --git a/src/plugins/workspace/server/saved_objects/index.ts b/src/plugins/workspace/server/saved_objects/index.ts index 51653c50681e..e47be61b0cd2 100644 --- a/src/plugins/workspace/server/saved_objects/index.ts +++ b/src/plugins/workspace/server/saved_objects/index.ts @@ -4,3 +4,4 @@ */ export { workspace } from './workspace'; +export { WorkspaceSavedObjectsClientWrapper } from './workspace_saved_objects_client_wrapper'; 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 new file mode 100644 index 000000000000..9f4f46b4dc99 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -0,0 +1,528 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + createTestServers, + TestOpenSearchUtils, + TestOpenSearchDashboardsUtils, + TestUtils, +} from '../../../../../core/test_helpers/osd_server'; +import { + SavedObjectsErrorHelpers, + WORKSPACE_TYPE, + ISavedObjectsRepository, + SavedObjectsClientContract, +} from '../../../../../core/server'; +import { httpServerMock } from '../../../../../../src/core/server/mocks'; +import * as utilsExports from '../../utils'; + +const repositoryKit = (() => { + const savedObjects: Array<{ type: string; id: string }> = []; + return { + create: async ( + repository: ISavedObjectsRepository, + ...params: Parameters + ) => { + let result; + try { + result = params[2]?.id ? await repository.get(params[0], params[2].id) : undefined; + } catch (_e) { + // ignore error when get failed + } + if (!result) { + result = await repository.create(...params); + } + savedObjects.push(result); + return result; + }, + clearAll: async (repository: ISavedObjectsRepository) => { + for (let i = 0; i < savedObjects.length; i++) { + try { + await repository.delete(savedObjects[i].type, savedObjects[i].id); + } catch (_e) { + // Ignore delete error + } + } + }, + }; +})(); + +const permittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); +const notPermittedRequest = httpServerMock.createOpenSearchDashboardsRequest(); + +describe('WorkspaceSavedObjectsClientWrapper', () => { + let internalSavedObjectsRepository: ISavedObjectsRepository; + let servers: TestUtils; + let opensearchServer: TestOpenSearchUtils; + let osd: TestOpenSearchDashboardsUtils; + let permittedSavedObjectedClient: SavedObjectsClientContract; + let notPermittedSavedObjectedClient: SavedObjectsClientContract; + + beforeAll(async function () { + servers = createTestServers({ + adjustTimeout: (t) => { + jest.setTimeout(t); + }, + settings: { + osd: { + workspace: { + enabled: true, + permission: { + enabled: true, + }, + }, + migrations: { skip: false }, + }, + }, + }); + opensearchServer = await servers.startOpenSearch(); + osd = await servers.startOpenSearchDashboards(); + + internalSavedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([ + WORKSPACE_TYPE, + ]); + + await repositoryKit.create( + internalSavedObjectsRepository, + 'workspace', + {}, + { + id: 'workspace-1', + permissions: { + library_read: { users: ['foo'] }, + library_write: { users: ['foo'] }, + }, + } + ); + + await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + id: 'inner-workspace-dashboard-1', + workspaces: ['workspace-1'], + } + ); + + await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + id: 'acl-controlled-dashboard-2', + permissions: { + read: { users: ['foo'], groups: [] }, + write: { users: ['foo'], groups: [] }, + }, + } + ); + + jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockImplementation((request) => { + if (request === notPermittedRequest) { + return { users: ['bar'] }; + } + return { users: ['foo'] }; + }); + + permittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient(permittedRequest); + notPermittedSavedObjectedClient = osd.coreStart.savedObjects.getScopedClient( + notPermittedRequest + ); + }); + + afterAll(async () => { + await repositoryKit.clearAll(internalSavedObjectsRepository); + await opensearchServer.stop(); + await osd.stop(); + + jest.spyOn(utilsExports, 'getPrincipalsFromRequest').mockRestore(); + }); + + describe('get', () => { + it('should throw forbidden error when user not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.get('dashboard', 'inner-workspace-dashboard-1'); + } catch (e) { + error = e; + } + expect(error).not.toBeUndefined(); + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + error = undefined; + try { + await notPermittedSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2'); + } catch (e) { + error = e; + } + expect(error).not.toBeUndefined(); + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should return consistent dashboard when user permitted', async () => { + expect( + (await permittedSavedObjectedClient.get('dashboard', 'inner-workspace-dashboard-1')).error + ).toBeUndefined(); + expect( + (await permittedSavedObjectedClient.get('dashboard', 'acl-controlled-dashboard-2')).error + ).toBeUndefined(); + }); + }); + + describe('bulkGet', () => { + it('should throw forbidden error when user not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, + ]); + } catch (e) { + error = e; + } + expect(error).not.toBeUndefined(); + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + error = undefined; + try { + await notPermittedSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, + ]); + } catch (e) { + error = e; + } + expect(error).not.toBeUndefined(); + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should return consistent dashboard when user permitted', async () => { + expect( + ( + await permittedSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1' }, + ]) + ).saved_objects.length + ).toEqual(1); + expect( + ( + await permittedSavedObjectedClient.bulkGet([ + { type: 'dashboard', id: 'acl-controlled-dashboard-2' }, + ]) + ).saved_objects.length + ).toEqual(1); + }); + }); + + describe('find', () => { + it('should throw not authorized error when user not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: ['workspace-1'], + perPage: 999, + page: 1, + }); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isNotAuthorizedError(error)).toBe(true); + }); + + it('should return consistent inner workspace data when user permitted', async () => { + const result = await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: ['workspace-1'], + perPage: 999, + page: 1, + }); + + expect(result.saved_objects.some((item) => item.id === 'inner-workspace-dashboard-1')).toBe( + true + ); + }); + }); + + describe('create', () => { + it('should throw forbidden error when workspace not permitted and create called', async () => { + let error; + try { + await notPermittedSavedObjectedClient.create( + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should able to create saved objects into permitted workspaces after create called', async () => { + const createResult = await permittedSavedObjectedClient.create( + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + expect(createResult.error).toBeUndefined(); + await permittedSavedObjectedClient.delete('dashboard', createResult.id); + }); + + it('should throw forbidden error when create with override', async () => { + let error; + try { + await notPermittedSavedObjectedClient.create( + 'dashboard', + {}, + { + id: 'inner-workspace-dashboard-1', + overwrite: true, + } + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should able to create with override', async () => { + const createResult = await permittedSavedObjectedClient.create( + 'dashboard', + {}, + { + id: 'inner-workspace-dashboard-1', + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.error).toBeUndefined(); + }); + }); + + describe('bulkCreate', () => { + it('should throw forbidden error when workspace not permitted and bulkCreate called', async () => { + let error; + try { + await notPermittedSavedObjectedClient.bulkCreate([{ type: 'dashboard', attributes: {} }], { + workspaces: ['workspace-1'], + }); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should able to create saved objects into permitted workspaces after bulkCreate called', async () => { + const objectId = new Date().getTime().toString(16).toUpperCase(); + const result = await permittedSavedObjectedClient.bulkCreate( + [{ type: 'dashboard', attributes: {}, id: objectId }], + { + workspaces: ['workspace-1'], + } + ); + expect(result.saved_objects.length).toEqual(1); + await permittedSavedObjectedClient.delete('dashboard', objectId); + }); + + it('should throw forbidden error when create with override', async () => { + let error; + try { + await notPermittedSavedObjectedClient.bulkCreate( + [ + { + id: 'inner-workspace-dashboard-1', + type: 'dashboard', + attributes: {}, + }, + ], + { + overwrite: true, + workspaces: ['workspace-1'], + } + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should able to bulk create with override', async () => { + const createResult = await permittedSavedObjectedClient.bulkCreate( + [ + { + id: 'inner-workspace-dashboard-1', + type: 'dashboard', + attributes: {}, + }, + ], + { + overwrite: true, + workspaces: ['workspace-1'], + } + ); + + expect(createResult.saved_objects).toHaveLength(1); + }); + }); + + describe('update', () => { + it('should throw forbidden error when data not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.update( + 'dashboard', + 'inner-workspace-dashboard-1', + {} + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + error = undefined; + try { + await notPermittedSavedObjectedClient.update('dashboard', 'acl-controlled-dashboard-2', {}); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should update saved objects for permitted workspaces', async () => { + expect( + (await permittedSavedObjectedClient.update('dashboard', 'inner-workspace-dashboard-1', {})) + .error + ).toBeUndefined(); + expect( + (await permittedSavedObjectedClient.update('dashboard', 'acl-controlled-dashboard-2', {})) + .error + ).toBeUndefined(); + }); + }); + + describe('bulkUpdate', () => { + it('should throw forbidden error when data not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.bulkUpdate( + [{ type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }], + {} + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + error = undefined; + try { + await notPermittedSavedObjectedClient.bulkUpdate( + [{ type: 'dashboard', id: 'acl-controlled-dashboard-2', attributes: {} }], + {} + ); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should bulk update saved objects for permitted workspaces', async () => { + expect( + ( + await permittedSavedObjectedClient.bulkUpdate([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, + ]) + ).saved_objects.length + ).toEqual(1); + expect( + ( + await permittedSavedObjectedClient.bulkUpdate([ + { type: 'dashboard', id: 'inner-workspace-dashboard-1', attributes: {} }, + ]) + ).saved_objects.length + ).toEqual(1); + }); + }); + + describe('delete', () => { + it('should throw forbidden error when data not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.delete('dashboard', 'inner-workspace-dashboard-1'); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + + error = undefined; + try { + await notPermittedSavedObjectedClient.delete('dashboard', 'acl-controlled-dashboard-2'); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should be able to delete permitted data', async () => { + const createResult = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + workspaces: ['workspace-1'], + } + ); + + await permittedSavedObjectedClient.delete('dashboard', createResult.id); + + let error; + try { + error = await permittedSavedObjectedClient.get('dashboard', createResult.id); + } catch (e) { + error = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); + }); + + it('should be able to delete acl controlled permitted data', async () => { + const createResult = await repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + permissions: { + read: { users: ['foo'] }, + write: { users: ['foo'] }, + }, + } + ); + + await permittedSavedObjectedClient.delete('dashboard', createResult.id); + + let error; + try { + error = await permittedSavedObjectedClient.get('dashboard', createResult.id); + } catch (e) { + error = e; + } + expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); + }); + }); +}); 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 new file mode 100644 index 000000000000..6b40f6e60fa0 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -0,0 +1,597 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObjectsErrorHelpers } from '../../../../core/server'; +import { WorkspaceSavedObjectsClientWrapper } from './workspace_saved_objects_client_wrapper'; + +const generateWorkspaceSavedObjectsClientWrapper = () => { + const savedObjectsStore = [ + { + type: 'dashboard', + id: 'foo', + workspaces: ['workspace-1'], + attributes: { + bar: 'baz', + }, + permissions: {}, + }, + { + type: 'dashboard', + id: 'not-permitted-dashboard', + workspaces: ['not-permitted-workspace'], + attributes: { + bar: 'baz', + }, + permissions: {}, + }, + { type: 'workspace', id: 'workspace-1', attributes: { name: 'Workspace - 1' } }, + { + type: 'workspace', + id: 'not-permitted-workspace', + attributes: { name: 'Not permitted workspace' }, + }, + ]; + const clientMock = { + get: jest.fn().mockImplementation(async (type, id) => { + if (type === 'config') { + return { + type: 'config', + }; + } + return ( + savedObjectsStore.find((item) => item.type === type && item.id === id) || + SavedObjectsErrorHelpers.createGenericNotFoundError() + ); + }), + create: jest.fn(), + bulkCreate: jest.fn(), + checkConflicts: jest.fn(), + delete: jest.fn(), + update: jest.fn(), + bulkUpdate: jest.fn(), + bulkGet: jest.fn().mockImplementation((savedObjectsToFind) => { + return { + saved_objects: savedObjectsStore.filter((item) => + savedObjectsToFind.find( + (itemToFind) => itemToFind.type === item.type && itemToFind.id === item.id + ) + ), + }; + }), + find: jest.fn(), + deleteByWorkspace: jest.fn(), + }; + const requestMock = {}; + const wrapperOptions = { + client: clientMock, + request: requestMock, + typeRegistry: {}, + }; + const permissionControlMock = { + setup: jest.fn(), + validate: jest.fn().mockImplementation((_request, { id }) => { + return { + success: true, + result: !id.startsWith('not-permitted'), + }; + }), + validateSavedObjectsACL: jest.fn(), + batchValidate: jest.fn(), + getPrincipalsFromRequest: jest.fn().mockImplementation(() => ({ users: ['user-1'] })), + }; + const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); + wrapper.setScopedClient(() => ({ + find: jest.fn().mockImplementation(async () => ({ + saved_objects: [{ id: 'workspace-1', type: 'workspace' }], + })), + })); + return { + wrapper: wrapper.wrapperFactory(wrapperOptions), + clientMock, + permissionControlMock, + requestMock, + }; +}; + +describe('WorkspaceSavedObjectsClientWrapper', () => { + describe('wrapperFactory', () => { + describe('delete', () => { + it('should throw permission error if not permitted', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.delete('dashboard', 'not-permitted-dashboard'); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [expect.objectContaining({ type: 'dashboard', id: 'not-permitted-dashboard' })], + { users: ['user-1'] }, + ['write'] + ); + 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; + await wrapper.delete(...deleteArgs); + expect(clientMock.delete).toHaveBeenCalledWith(...deleteArgs); + }); + }); + + describe('update', () => { + it('should throw permission error if not permitted', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.update('dashboard', 'not-permitted-dashboard', { + bar: 'foo', + }); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [expect.objectContaining({ type: 'dashboard', id: 'not-permitted-dashboard' })], + { users: ['user-1'] }, + ['write'] + ); + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); + it('should call client.update with arguments if permitted', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + const updateArgs = [ + 'workspace', + 'foo', + { + bar: 'foo', + }, + {}, + ] as const; + await wrapper.update(...updateArgs); + expect(clientMock.update).toHaveBeenCalledWith(...updateArgs); + }); + }); + + describe('bulk update', () => { + it('should throw permission error if not permitted', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.bulkUpdate([ + { type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }, + ]); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [expect.objectContaining({ type: 'dashboard', id: 'not-permitted-dashboard' })], + { users: ['user-1'] }, + ['write'] + ); + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); + it('should call client.bulkUpdate with arguments if permitted', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + const objectsToUpdate = [{ type: 'dashboard', id: 'foo', attributes: { bar: 'baz' } }]; + await wrapper.bulkUpdate(objectsToUpdate, {}); + expect(clientMock.bulkUpdate).toHaveBeenCalledWith(objectsToUpdate, {}); + }); + }); + + describe('bulk create', () => { + it('should throw workspace permission error if passed workspaces but not permitted', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + permissionControlMock.validate.mockResolvedValueOnce({ success: true, result: false }); + let errorCatched; + try { + await wrapper.bulkCreate([{ type: 'dashboard', id: 'new-dashboard', attributes: {} }], { + workspaces: ['not-permitted-workspace'], + }); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + }); + it("should throw permission error if overwrite and not permitted on object's workspace and object", async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + permissionControlMock.validate.mockResolvedValueOnce({ success: true, result: false }); + let errorCatched; + try { + await wrapper.bulkCreate( + [{ type: 'dashboard', id: 'not-permitted-dashboard', attributes: { bar: 'baz' } }], + { + overwrite: true, + } + ); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [expect.objectContaining({ type: 'dashboard', id: 'not-permitted-dashboard' })], + { users: ['user-1'] }, + ['write'] + ); + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + }); + it('should call client.bulkCreate with arguments if some objects not found', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + const objectsToBulkCreate = [ + { type: 'dashboard', id: 'new-dashboard', attributes: { bar: 'baz' } }, + { type: 'dashboard', id: 'not-found', attributes: { bar: 'foo' } }, + ]; + 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 () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + const objectsToBulkCreate = [ + { type: 'dashboard', id: 'new-dashboard', attributes: { bar: 'baz' } }, + ]; + await wrapper.bulkCreate(objectsToBulkCreate, { + overwrite: true, + workspaces: ['workspace-1'], + }); + expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { + overwrite: true, + workspaces: ['workspace-1'], + }); + }); + }); + + describe('create', () => { + it('should throw workspace permission error if passed workspaces but not permitted', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.create('dashboard', 'new-dashboard', { + workspaces: ['not-permitted-workspace'], + }); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + }); + it("should throw permission error if overwrite and not permitted on object's workspace and object", async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.create( + 'dashboard', + { foo: 'bar' }, + { + id: 'not-permitted-dashboard', + overwrite: true, + } + ); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { type: 'workspace', id: 'not-permitted-workspace' }, + ['library_write'] + ); + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [expect.objectContaining({ type: 'dashboard', id: 'not-permitted-dashboard' })], + { users: ['user-1'] }, + ['write'] + ); + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + }); + it('should call client.create with arguments if permitted', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.create( + 'dashboard', + { foo: 'bar' }, + { + id: 'foo', + overwrite: true, + } + ); + expect(clientMock.create).toHaveBeenCalledWith( + 'dashboard', + { foo: 'bar' }, + { + id: 'foo', + overwrite: true, + } + ); + }); + }); + describe('get', () => { + it('should return saved object if no need to validate permission', async () => { + const { wrapper, permissionControlMock } = generateWorkspaceSavedObjectsClientWrapper(); + const result = await wrapper.get('config', 'config-1'); + expect(result).toEqual({ type: 'config' }); + expect(permissionControlMock.validate).not.toHaveBeenCalled(); + }); + it("should call permission validate with object's workspace and throw permission error", async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.get('dashboard', 'not-permitted-dashboard'); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { + type: 'workspace', + id: 'not-permitted-workspace', + }, + ['library_read', 'library_write'] + ); + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); + it('should call permission validateSavedObjectsACL with object', async () => { + const { wrapper, permissionControlMock } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.get('dashboard', 'not-permitted-dashboard'); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [ + expect.objectContaining({ + type: 'dashboard', + id: 'not-permitted-dashboard', + }), + ], + { users: ['user-1'] }, + ['read', 'write'] + ); + }); + it('should call client.get and return result with arguments if permitted', async () => { + const { + wrapper, + clientMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + permissionControlMock.validate.mockResolvedValueOnce({ success: true, result: true }); + const getArgs = ['workspace', 'foo', {}] as const; + const result = await wrapper.get(...getArgs); + expect(clientMock.get).toHaveBeenCalledWith(...getArgs); + expect(result).toMatchInlineSnapshot(`[Error: Not Found]`); + }); + }); + describe('bulk get', () => { + it("should call permission validate with object's workspace and throw permission error", async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.bulkGet([{ type: 'dashboard', id: 'not-permitted-dashboard' }]); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { + type: 'workspace', + id: 'not-permitted-workspace', + }, + ['library_read', 'library_write'] + ); + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); + it('should call permission validateSavedObjectsACL with object', async () => { + const { wrapper, permissionControlMock } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.bulkGet([{ type: 'dashboard', id: 'not-permitted-dashboard' }]); + } catch (e) { + errorCatched = e; + } + expect(permissionControlMock.validateSavedObjectsACL).toHaveBeenCalledWith( + [ + expect.objectContaining({ + type: 'dashboard', + id: 'not-permitted-dashboard', + }), + ], + { users: ['user-1'] }, + ['write', 'read'] + ); + }); + it('should call client.bulkGet and return result with arguments if permitted', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + + await wrapper.bulkGet( + [ + { + type: 'dashboard', + id: 'foo', + }, + ], + {} + ); + expect(clientMock.bulkGet).toHaveBeenCalledWith( + [ + { + type: 'dashboard', + id: 'foo', + }, + ], + {} + ); + }); + }); + describe('find', () => { + it('should call client.find with ACLSearchParams for workspace type', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'workspace', + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'workspace', + ACLSearchParams: { + principals: { + users: ['user-1'], + }, + permissionModes: ['read', 'write'], + }, + }); + }); + it('should call client.find with only read permission if find workspace and permissionModes provided', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'workspace', + ACLSearchParams: { + permissionModes: ['read'], + }, + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'workspace', + ACLSearchParams: { + principals: { + users: ['user-1'], + }, + permissionModes: ['read'], + }, + }); + }); + it('should throw workspace permission error if provided workspaces not permitted', async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + errorCatched = await wrapper.find({ + type: 'dashboard', + workspaces: ['not-permitted-workspace'], + }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + }); + it('should remove not permitted workspace and call client.find with arguments', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'dashboard', + workspaces: ['not-permitted-workspace', 'workspace-1'], + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'dashboard', + workspaces: ['workspace-1'], + ACLSearchParams: {}, + }); + }); + it('should call client.find with arguments if not workspace type and no options.workspace', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'dashboard', + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'dashboard', + workspaces: ['workspace-1'], + workspacesSearchOperator: 'OR', + ACLSearchParams: { + permissionModes: ['read', 'write'], + principals: { users: ['user-1'] }, + }, + }); + }); + }); + describe('deleteByWorkspace', () => { + it('should call permission validate with workspace and throw workspace permission error if not permitted', async () => { + const { + wrapper, + requestMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.deleteByWorkspace('not-permitted-workspace'); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { id: 'not-permitted-workspace', type: 'workspace' }, + ['library_write'] + ); + }); + + it('should call client.deleteByWorkspace if permitted', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + + await wrapper.deleteByWorkspace('workspace-1', {}); + expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-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 new file mode 100644 index 000000000000..c515f555fa4b --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -0,0 +1,549 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { i18n } from '@osd/i18n'; + +import { + OpenSearchDashboardsRequest, + SavedObject, + SavedObjectsBaseOptions, + SavedObjectsBulkCreateObject, + SavedObjectsBulkGetObject, + SavedObjectsBulkResponse, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsDeleteOptions, + SavedObjectsFindOptions, + SavedObjectsUpdateOptions, + SavedObjectsUpdateResponse, + SavedObjectsBulkUpdateObject, + SavedObjectsBulkUpdateResponse, + SavedObjectsBulkUpdateOptions, + WORKSPACE_TYPE, + SavedObjectsDeleteByWorkspaceOptions, + SavedObjectsErrorHelpers, + SavedObjectsServiceStart, + SavedObjectsClientContract, +} from '../../../../core/server'; +import { SavedObjectsPermissionControlContract } from '../permission_control/client'; +import { + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + WorkspacePermissionMode, +} from '../../common/constants'; + +// Can't throw unauthorized for now, the page will be refreshed if unauthorized +const generateWorkspacePermissionError = () => + SavedObjectsErrorHelpers.decorateForbiddenError( + new Error( + i18n.translate('workspace.permission.invalidate', { + defaultMessage: 'Invalid workspace permission', + }) + ) + ); + +const generateSavedObjectsPermissionError = () => + SavedObjectsErrorHelpers.decorateForbiddenError( + new Error( + i18n.translate('saved_objects.permission.invalidate', { + defaultMessage: 'Invalid saved objects permission', + }) + ) + ); + +const intersection = (...args: T[][]) => { + const occursCountMap: { [key: string]: number } = {}; + for (let i = 0; i < args.length; i++) { + new Set(args[i]).forEach((key) => { + occursCountMap[key] = (occursCountMap[key] || 0) + 1; + }); + } + return Object.keys(occursCountMap).filter((key) => occursCountMap[key] === args.length); +}; + +const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[]) => { + return !values || values.length === 0 ? defaultValues : values; +}; + +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[] + ) { + // PermissionMode 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( + request, + { + type, + id, + }, + this.formatWorkspacePermissionModeToStringArray(permissionMode) + ); + if (!validateResult?.result) { + return false; + } + } + return true; + } + + // validate if the `request` has the specified permission(`permissionMode`) to the given `workspaceIds` + private validateMultiWorkspacesPermissions = async ( + workspacesIds: string[], + request: OpenSearchDashboardsRequest, + permissionMode: WorkspacePermissionMode | 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); + }; + + private validateAtLeastOnePermittedWorkspaces = async ( + workspaces: string[] | undefined, + request: OpenSearchDashboardsRequest, + permissionMode: WorkspacePermissionMode | 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) { + return false; + } + for (const workspaceId of workspaces) { + const validateResult = await this.permissionControl.validate( + request, + { + type: WORKSPACE_TYPE, + id: workspaceId, + }, + this.formatWorkspacePermissionModeToStringArray(permissionMode) + ); + if (validateResult?.result) { + return true; + } + } + return false; + }; + + /** + * check if the type include workspace + * Workspace permission check is totally different from object permission check. + * @param type + * @returns + */ + private isRelatedToWorkspace(type: string | string[]): boolean { + return type === WORKSPACE_TYPE || (Array.isArray(type) && type.includes(WORKSPACE_TYPE)); + } + + private async validateWorkspacesAndSavedObjectsPermissions( + savedObject: Pick, + request: OpenSearchDashboardsRequest, + workspacePermissionModes: WorkspacePermissionMode[], + objectPermissionModes: WorkspacePermissionMode[], + validateAllWorkspaces = true + ) { + /** + * + * Checks if the provided saved object lacks both workspaces and permissions. + * If a saved object lacks both attributes, it implies that the object is neither associated + * with any workspaces nor has permissions defined by itself. Such objects are considered "public" + * and will be excluded from permission checks. + * + **/ + if (!savedObject.workspaces && !savedObject.permissions) { + return true; + } + + let hasPermission = false; + // Check permission based on object's workspaces. + // If workspacePermissionModes is passed with an empty array, we need to skip this validation and continue to validate object ACL. + if (savedObject.workspaces && workspacePermissionModes.length > 0) { + const workspacePermissionValidator = validateAllWorkspaces + ? this.validateMultiWorkspacesPermissions + : this.validateAtLeastOnePermittedWorkspaces; + hasPermission = await workspacePermissionValidator( + savedObject.workspaces, + request, + workspacePermissionModes + ); + } + // If already has permissions based on workspaces, we don't need to check object's ACL(defined by permissions attribute) + // So return true immediately + if (hasPermission) { + return true; + } + // Check permission based on object's ACL(defined by permissions attribute) + if (savedObject.permissions) { + hasPermission = await this.permissionControl.validateSavedObjectsACL( + [savedObject], + this.permissionControl.getPrincipalsFromRequest(request), + objectPermissionModes + ); + } + return hasPermission; + } + + private getWorkspaceTypeEnabledClient(request: OpenSearchDashboardsRequest) { + return this.getScopedClient?.(request, { + includedHiddenTypes: [WORKSPACE_TYPE], + excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + }) as SavedObjectsClientContract; + } + + public setScopedClient(getScopedClient: SavedObjectsServiceStart['getScopedClient']) { + this.getScopedClient = getScopedClient; + } + + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + const deleteWithWorkspacePermissionControl = async ( + type: string, + id: string, + options: SavedObjectsDeleteOptions = {} + ) => { + const objectToDeleted = await wrapperOptions.client.get(type, id, options); + if ( + !(await this.validateWorkspacesAndSavedObjectsPermissions( + objectToDeleted, + wrapperOptions.request, + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write] + )) + ) { + throw generateSavedObjectsPermissionError(); + } + return await wrapperOptions.client.delete(type, id, options); + }; + + /** + * validate if can update`objectToUpdate`, means a user should either + * have `Write` permission on the `objectToUpdate` itself or `LibraryWrite` permission + * to any of the workspaces the `objectToUpdate` associated with. + **/ + const validateUpdateWithWorkspacePermission = async ( + objectToUpdate: SavedObject + ): Promise => { + return await this.validateWorkspacesAndSavedObjectsPermissions( + objectToUpdate, + wrapperOptions.request, + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write], + false + ); + }; + + const updateWithWorkspacePermissionControl = async ( + type: string, + id: string, + attributes: Partial, + options: SavedObjectsUpdateOptions = {} + ): Promise> => { + const objectToUpdate = await wrapperOptions.client.get(type, id, options); + const permitted = await validateUpdateWithWorkspacePermission(objectToUpdate); + if (!permitted) { + throw generateSavedObjectsPermissionError(); + } + return await wrapperOptions.client.update(type, id, attributes, options); + }; + + const bulkUpdateWithWorkspacePermissionControl = async ( + objects: Array>, + options?: SavedObjectsBulkUpdateOptions + ): Promise> => { + const objectsToUpdate = await wrapperOptions.client.bulkGet(objects, options); + + for (const object of objectsToUpdate.saved_objects) { + const permitted = await validateUpdateWithWorkspacePermission(object); + if (!permitted) { + throw generateSavedObjectsPermissionError(); + } + } + + return await wrapperOptions.client.bulkUpdate(objects, options); + }; + + const bulkCreateWithWorkspacePermissionControl = async ( + objects: Array>, + options: SavedObjectsCreateOptions = {} + ): Promise> => { + const hasTargetWorkspaces = options?.workspaces && options.workspaces.length > 0; + + if ( + hasTargetWorkspaces && + !(await this.validateMultiWorkspacesPermissions( + options.workspaces ?? [], + wrapperOptions.request, + [WorkspacePermissionMode.LibraryWrite] + )) + ) { + throw generateWorkspacePermissionError(); + } + + /** + * + * If target workspaces parameter doesn't exists and `overwrite` is true, we need to check + * if it has permission to the object itself(defined by the object ACL) or it has permission + * to any of the workspaces that the object associates with. + * + */ + if (!hasTargetWorkspaces && options.overwrite) { + for (const object of objects) { + const { type, id } = object; + if (id) { + let rawObject; + try { + rawObject = await wrapperOptions.client.get(type, id); + } catch (error) { + // If object is not found, we will skip the validation of this object. + if (SavedObjectsErrorHelpers.isNotFoundError(error as Error)) { + continue; + } else { + throw error; + } + } + if ( + !(await this.validateWorkspacesAndSavedObjectsPermissions( + rawObject, + wrapperOptions.request, + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write], + false + )) + ) { + throw generateWorkspacePermissionError(); + } + } + } + } + + return await wrapperOptions.client.bulkCreate(objects, options); + }; + + const createWithWorkspacePermissionControl = async ( + type: string, + attributes: T, + options?: SavedObjectsCreateOptions + ) => { + const hasTargetWorkspaces = options?.workspaces && options.workspaces.length > 0; + + if ( + hasTargetWorkspaces && + !(await this.validateMultiWorkspacesPermissions( + options?.workspaces ?? [], + wrapperOptions.request, + [WorkspacePermissionMode.LibraryWrite] + )) + ) { + throw generateWorkspacePermissionError(); + } + + /** + * + * If target workspaces parameter doesn't exists, `options.id` was exists and `overwrite` is true, + * we need to check if it has permission to the object itself(defined by the object ACL) or + * it has permission to any of the workspaces that the object associates with. + * + */ + if ( + options?.overwrite && + options.id && + !hasTargetWorkspaces && + !(await this.validateWorkspacesAndSavedObjectsPermissions( + await wrapperOptions.client.get(type, options.id), + wrapperOptions.request, + [WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write], + false + )) + ) { + throw generateWorkspacePermissionError(); + } + + return await wrapperOptions.client.create(type, attributes, options); + }; + + const getWithWorkspacePermissionControl = async ( + type: string, + id: string, + options: SavedObjectsBaseOptions = {} + ): Promise> => { + const objectToGet = await wrapperOptions.client.get(type, id, options); + + if ( + !(await this.validateWorkspacesAndSavedObjectsPermissions( + objectToGet, + wrapperOptions.request, + [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write], + false + )) + ) { + throw generateSavedObjectsPermissionError(); + } + return objectToGet; + }; + + const bulkGetWithWorkspacePermissionControl = async ( + objects: SavedObjectsBulkGetObject[] = [], + options: SavedObjectsBaseOptions = {} + ): Promise> => { + const objectToBulkGet = await wrapperOptions.client.bulkGet(objects, options); + + for (const object of objectToBulkGet.saved_objects) { + if ( + !(await this.validateWorkspacesAndSavedObjectsPermissions( + object, + wrapperOptions.request, + [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite], + [WorkspacePermissionMode.Write, WorkspacePermissionMode.Read], + false + )) + ) { + throw generateSavedObjectsPermissionError(); + } + } + + return objectToBulkGet; + }; + + const findWithWorkspacePermissionControl = async ( + options: SavedObjectsFindOptions + ) => { + const principals = this.permissionControl.getPrincipalsFromRequest(wrapperOptions.request); + if (!options.ACLSearchParams) { + options.ACLSearchParams = {}; + } + + if (this.isRelatedToWorkspace(options.type)) { + /** + * + * This case is for finding workspace saved objects, will use passed permissionModes + * and override passed principals from request to get all readable workspaces. + * + */ + options.ACLSearchParams.permissionModes = getDefaultValuesForEmpty( + options.ACLSearchParams.permissionModes, + [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write] + ); + options.ACLSearchParams.principals = principals; + } else { + /** + * Workspace is a hidden type so that we need to + * initialize a new saved objects client with workspace enabled to retrieve all the workspaces with permission. + */ + const permittedWorkspaceIds = ( + await this.getWorkspaceTypeEnabledClient(wrapperOptions.request).find({ + type: WORKSPACE_TYPE, + perPage: 999, + ACLSearchParams: { + principals, + /** + * 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: getDefaultValuesForEmpty( + options.ACLSearchParams.permissionModes + ? intersection(options.ACLSearchParams.permissionModes, [ + WorkspacePermissionMode.LibraryRead, + WorkspacePermissionMode.LibraryWrite, + ]) + : [], + [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] + ), + }, + }) + ).saved_objects.map((item) => item.id); + + if (options.workspaces) { + const permittedWorkspaces = options.workspaces.filter((item) => + permittedWorkspaceIds.includes(item) + ); + if (!permittedWorkspaces.length) { + /** + * If user does not have any one workspace access + * deny the request + */ + throw SavedObjectsErrorHelpers.decorateNotAuthorizedError( + new Error( + i18n.translate('workspace.permission.invalidate', { + defaultMessage: 'Invalid workspace permission', + }) + ) + ); + } + + /** + * Overwrite the options.workspaces when user has access on partial workspaces. + */ + 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 + */ + options.workspaces = permittedWorkspaceIds; + options.workspacesSearchOperator = 'OR'; + options.ACLSearchParams.permissionModes = getDefaultValuesForEmpty( + options.ACLSearchParams.permissionModes, + [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write] + ); + options.ACLSearchParams.principals = principals; + } + } + + return await wrapperOptions.client.find(options); + }; + + const deleteByWorkspaceWithPermissionControl = async ( + workspace: string, + options: SavedObjectsDeleteByWorkspaceOptions = {} + ) => { + if ( + !(await this.validateMultiWorkspacesPermissions([workspace], wrapperOptions.request, [ + WorkspacePermissionMode.LibraryWrite, + ])) + ) { + throw generateWorkspacePermissionError(); + } + + return await wrapperOptions.client.deleteByWorkspace(workspace, options); + }; + + return { + ...wrapperOptions.client, + get: getWithWorkspacePermissionControl, + checkConflicts: wrapperOptions.client.checkConflicts, + find: findWithWorkspacePermissionControl, + bulkGet: bulkGetWithWorkspacePermissionControl, + errors: wrapperOptions.client.errors, + addToNamespaces: wrapperOptions.client.addToNamespaces, + deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + create: createWithWorkspacePermissionControl, + bulkCreate: bulkCreateWithWorkspacePermissionControl, + delete: deleteWithWorkspacePermissionControl, + update: updateWithWorkspacePermissionControl, + bulkUpdate: bulkUpdateWithWorkspacePermissionControl, + deleteByWorkspace: deleteByWorkspaceWithPermissionControl, + }; + }; + + constructor(private readonly permissionControl: SavedObjectsPermissionControlContract) {} +} diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index 0f60597a7a8a..ba1ff8a9cd47 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -12,6 +12,7 @@ import { WorkspaceAttribute, SavedObjectsServiceStart, } from '../../../core/server'; +import { WorkspacePermissionMode } from '../common/constants'; export interface WorkspaceFindOptions { page?: number; @@ -117,3 +118,17 @@ export type IResponse = success: false; error?: string; }; + +export interface AuthInfo { + backend_roles?: string[]; + user_name?: string; +} + +export type WorkspacePermissionItem = { + modes: Array< + | WorkspacePermissionMode.LibraryRead + | WorkspacePermissionMode.LibraryWrite + | WorkspacePermissionMode.Read + | WorkspacePermissionMode.Write + >; +} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 119b8889f715..5af40eea9b06 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -3,9 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { generateRandomId } from './utils'; +import { AuthStatus } from '../../../core/server'; +import { httpServerMock, httpServiceMock } from '../../../core/server/mocks'; +import { generateRandomId, getPrincipalsFromRequest } from './utils'; describe('workspace utils', () => { + const mockAuth = httpServiceMock.createAuth(); it('should generate id with the specified size', () => { expect(generateRandomId(6)).toHaveLength(6); }); @@ -18,4 +21,52 @@ describe('workspace utils', () => { } expect(ids.size).toBe(NUM_OF_ID); }); + + it('should return empty map when request do not have authentication', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + status: AuthStatus.unknown, + state: { + user_name: 'bar', + backend_roles: ['foo'], + }, + }); + const result = getPrincipalsFromRequest(mockRequest, mockAuth); + expect(result).toEqual({}); + }); + + it('should return normally when request has authentication', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + status: AuthStatus.authenticated, + state: { + user_name: 'bar', + backend_roles: ['foo'], + }, + }); + const result = getPrincipalsFromRequest(mockRequest, mockAuth); + expect(result.users).toEqual(['bar']); + expect(result.groups).toEqual(['foo']); + }); + + it('should throw error when request is not authenticated', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + status: AuthStatus.unauthenticated, + state: {}, + }); + expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow('NOT_AUTHORIZED'); + }); + + it('should throw error when authentication status is not expected', () => { + const mockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + mockAuth.get.mockReturnValueOnce({ + // @ts-ignore + status: 'foo', + state: {}, + }); + expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow( + 'UNEXPECTED_AUTHORIZATION_STATUS' + ); + }); }); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index 89bfabd52657..e51637cd49c3 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -4,6 +4,14 @@ */ import crypto from 'crypto'; +import { + AuthStatus, + HttpAuth, + OpenSearchDashboardsRequest, + Principals, + PrincipalType, +} from '../../../core/server'; +import { AuthInfo } from './types'; /** * Generate URL friendly random ID @@ -11,3 +19,34 @@ import crypto from 'crypto'; export const generateRandomId = (size: number) => { return crypto.randomBytes(size).toString('base64url').slice(0, size); }; + +export const getPrincipalsFromRequest = ( + request: OpenSearchDashboardsRequest, + auth?: HttpAuth +): Principals => { + const payload: Principals = {}; + const authInfoResp = auth?.get(request); + if (authInfoResp?.status === AuthStatus.unknown) { + /** + * Login user have access to all the workspaces when no authentication is presented. + */ + return payload; + } + + if (authInfoResp?.status === AuthStatus.authenticated) { + const authInfo = authInfoResp?.state as AuthInfo | null; + if (authInfo?.backend_roles) { + payload[PrincipalType.Groups] = authInfo.backend_roles; + } + if (authInfo?.user_name) { + payload[PrincipalType.Users] = [authInfo.user_name]; + } + return payload; + } + + if (authInfoResp?.status === AuthStatus.unauthenticated) { + throw new Error('NOT_AUTHORIZED'); + } + + throw new Error('UNEXPECTED_AUTHORIZATION_STATUS'); +}; From 9bdfbf7b3c09188d47e40f3e9db15118dd260cca Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 6 Mar 2024 15:48:12 +0800 Subject: [PATCH 02/21] Add changelog for permission control in workspace Signed-off-by: Lin Wang --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b22b5556ac33..ea947e62c23c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 🛡 Security ### 📈 Features/Enhancements +- [Workspace] Add permission control logic ([#6052](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6052)) ### 🐛 Bug Fixes From bf9c5af43e9c2f1c1b70c5ad3cd1ad1572d6b55c Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 6 Mar 2024 17:46:05 +0800 Subject: [PATCH 03/21] Fix integration tests and remove no need type Signed-off-by: Lin Wang --- src/core/server/index.ts | 1 - src/core/server/saved_objects/index.ts | 8 +------- .../saved_objects/service/lib/repository.test.js | 14 ++++++++++++-- .../server/saved_objects/service/lib/repository.ts | 3 ++- .../workspace/server/permission_control/client.ts | 1 - .../workspace_saved_objects_client_wrapper.test.ts | 2 ++ 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 0ff5f74a4d78..5dd41a6314c5 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -323,7 +323,6 @@ export { resolveSavedObjectsImportErrors, ACL, Principals, - TransformedPermission, PrincipalType, Permissions, } from './saved_objects'; diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts index 11809c5b88c9..dccf63d4dcf4 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 b793046d9a94..63b30f8e733a 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -168,7 +168,7 @@ describe('SavedObjectsRepository', () => { }); const getMockGetResponse = ( - { type, id, references, namespace: objectNamespace, originId, permissions }, + { type, id, references, namespace: objectNamespace, originId, permissions, workspaces }, namespace ) => { const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; @@ -184,6 +184,7 @@ describe('SavedObjectsRepository', () => { ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), ...(originId && { originId }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), type, [type]: { title: 'Testing' }, references, @@ -3077,7 +3078,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, @@ -3086,6 +3087,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 ); @@ -3251,6 +3253,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 81a1b578257a..ca945c60cb3a 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -994,7 +994,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId, updated_at: updatedAt, permissions } = body._source; + const { originId, updated_at: updatedAt, permissions, workspaces } = body._source; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { @@ -1010,6 +1010,7 @@ export class SavedObjectsRepository { ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), version: encodeHitVersion(body), attributes: body._source[type], references: body._source.references || [], diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bad46eb156a6..d5a0e80dae1e 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, 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 9f4f46b4dc99..2a7fb0e440b5 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 @@ -69,6 +69,8 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { osd: { workspace: { enabled: true, + }, + savedObjects: { permission: { enabled: true, }, From fbfe74a406b16ea1de00c3733cb1bca55a757742 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 6 Mar 2024 18:30:28 +0800 Subject: [PATCH 04/21] Update permission enabled for workspace CRUD integration tests Signed-off-by: Lin Wang --- src/plugins/workspace/server/integration_tests/routes.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index c1895986ec09..9fe85659f8c1 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -30,6 +30,8 @@ describe('workspace service', () => { osd: { workspace: { enabled: true, + }, + savedObjects: { permission: { enabled: false, }, From e7279f987945d43a2c1eea2267294cd5cd5f283d Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 6 Mar 2024 18:38:21 +0800 Subject: [PATCH 05/21] Change back to config schema Signed-off-by: Lin Wang --- src/plugins/workspace/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/config.ts b/src/plugins/workspace/config.ts index 0ee3d9fbf60d..79412f5c02ee 100644 --- a/src/plugins/workspace/config.ts +++ b/src/plugins/workspace/config.ts @@ -9,4 +9,4 @@ export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: false }), }); -export type WorkspacePluginConfigType = TypeOf; +export type ConfigSchema = TypeOf; From 7ba2d5e6889ac682d235dd43fbb643158b6dfdfc Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 7 Mar 2024 14:39:36 +0800 Subject: [PATCH 06/21] 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 --- .../workspace_saved_objects_client_wrapper.test.ts | 2 -- .../workspace_saved_objects_client_wrapper.ts | 7 ++----- 2 files changed, 2 insertions(+), 7 deletions(-) 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 6b40f6e60fa0..d87fa8b8d3a2 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 @@ -556,8 +556,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 c515f555fa4b..2ca26f3486d4 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 @@ -495,12 +495,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] From b72b6707d904789f9a6d789185d5d03d06aa960f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 7 Mar 2024 16:02:47 +0800 Subject: [PATCH 07/21] 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 --- src/plugins/workspace/server/utils.test.ts | 12 ++++++++---- src/plugins/workspace/server/utils.ts | 10 +++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts index 5af40eea9b06..1f6c3e58f122 100644 --- a/src/plugins/workspace/server/utils.test.ts +++ b/src/plugins/workspace/server/utils.test.ts @@ -27,8 +27,10 @@ describe('workspace utils', () => { mockAuth.get.mockReturnValueOnce({ status: AuthStatus.unknown, state: { - user_name: 'bar', - backend_roles: ['foo'], + authInfo: { + user_name: 'bar', + backend_roles: ['foo'], + }, }, }); const result = getPrincipalsFromRequest(mockRequest, mockAuth); @@ -40,8 +42,10 @@ describe('workspace utils', () => { mockAuth.get.mockReturnValueOnce({ status: AuthStatus.authenticated, state: { - user_name: 'bar', - backend_roles: ['foo'], + authInfo: { + user_name: 'bar', + backend_roles: ['foo'], + }, }, }); const result = getPrincipalsFromRequest(mockRequest, mockAuth); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts index e51637cd49c3..1c8d73953afa 100644 --- a/src/plugins/workspace/server/utils.ts +++ b/src/plugins/workspace/server/utils.ts @@ -34,12 +34,12 @@ export const getPrincipalsFromRequest = ( } if (authInfoResp?.status === AuthStatus.authenticated) { - const authInfo = authInfoResp?.state as AuthInfo | null; - if (authInfo?.backend_roles) { - payload[PrincipalType.Groups] = authInfo.backend_roles; + const authInfo = authInfoResp?.state as { authInfo: AuthInfo } | null; + if (authInfo?.authInfo?.backend_roles) { + payload[PrincipalType.Groups] = authInfo.authInfo.backend_roles; } - if (authInfo?.user_name) { - payload[PrincipalType.Users] = [authInfo.user_name]; + if (authInfo?.authInfo?.user_name) { + payload[PrincipalType.Users] = [authInfo.authInfo.user_name]; } return payload; } From 166e2c196838900216becc321cf0617e20ecb9c2 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 7 Mar 2024 14:58:31 +0800 Subject: [PATCH 08/21] Fix permissions assign in attributes Signed-off-by: Lin Wang --- .../server/integration_tests/routes.test.ts | 20 +--- src/plugins/workspace/server/types.ts | 8 +- .../workspace/server/workspace_client.ts | 91 +++++++++++++++++-- 3 files changed, 93 insertions(+), 26 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 9fe85659f8c1..73d3e0250a0a 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -103,14 +103,8 @@ describe('workspace service', () => { 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', - }, - ]); + ).permissions + ).toEqual({ read: { users: ['foo'] } }); }); it('get', async () => { const result = await osdTestServer.request @@ -176,14 +170,8 @@ describe('workspace service', () => { 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', - }, - ]); + ).permissions + ).toEqual({ write: { users: ['foo'] } }); }); it('delete', async () => { const result: any = await osdTestServer.request diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index ba1ff8a9cd47..525c918376a1 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -14,6 +14,10 @@ import { } from '../../../core/server'; import { WorkspacePermissionMode } from '../common/constants'; +export interface WorkspaceAttributeWithPermission extends WorkspaceAttribute { + permissions?: WorkspacePermissionItem[]; +} + export interface WorkspaceFindOptions { page?: number; perPage?: number; @@ -53,7 +57,7 @@ export interface IWorkspaceClientImpl { */ create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): Promise>; /** * List workspaces @@ -91,7 +95,7 @@ export interface IWorkspaceClientImpl { update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise>; /** * Delete a given workspace diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 890cf9bdd8a0..d647999c22b1 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -4,18 +4,87 @@ */ import { i18n } from '@osd/i18n'; -import type { +import { SavedObject, SavedObjectsClientContract, CoreSetup, WorkspaceAttribute, SavedObjectsServiceStart, + ACL, + Permissions, } from '../../../core/server'; import { WORKSPACE_TYPE } from '../../../core/server'; -import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; +import { + IWorkspaceClientImpl, + WorkspaceFindOptions, + IResponse, + IRequestDetail, + WorkspaceAttributeWithPermission, + WorkspacePermissionItem, +} from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; +import { + WorkspacePermissionMode, + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, +} from '../common/constants'; + +const convertToACL = ( + workspacePermissions: WorkspacePermissionItem | WorkspacePermissionItem[] +) => { + workspacePermissions = Array.isArray(workspacePermissions) + ? workspacePermissions + : [workspacePermissions]; + + const acl = new ACL(); + + workspacePermissions.forEach((permission) => { + switch (permission.type) { + case 'user': + acl.addPermission(permission.modes, { users: [permission.userId] }); + return; + case 'group': + acl.addPermission(permission.modes, { groups: [permission.group] }); + return; + } + }); + + return acl.getPermissions() || {}; +}; + +const isValidWorkspacePermissionMode = (mode: string): mode is WorkspacePermissionMode => + Object.values(WorkspacePermissionMode).some((modeValue) => modeValue === mode); + +const isWorkspacePermissionItem = ( + test: WorkspacePermissionItem | null +): test is WorkspacePermissionItem => test !== null; + +const convertFromACL = (permissions: Permissions) => { + const acl = new ACL(permissions); + + return acl + .toFlatList() + .map(({ name, permissions: modes, type }) => { + const validModes = modes.filter(isValidWorkspacePermissionMode); + switch (type) { + case 'users': + return { + type: 'user', + modes: validModes, + userId: name, + } as const; + case 'groups': + return { + type: 'group', + modes: validModes, + group: name, + } as const; + default: + return null; + } + }) + .filter(isWorkspacePermissionItem); +}; const WORKSPACE_ID_SIZE = 6; @@ -67,10 +136,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( @@ -88,6 +157,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { attributes, { id, + permissions: permissions ? convertToACL(permissions) : undefined, } ); return { @@ -153,9 +223,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); @@ -172,7 +242,12 @@ export class WorkspaceClient implements IWorkspaceClientImpl { throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); } } - await client.update>(WORKSPACE_TYPE, id, attributes, {}); + await client.create>(WORKSPACE_TYPE, attributes, { + id, + permissions: permissions ? convertToACL(permissions) : undefined, + overwrite: true, + version: workspaceInDB.version, + }); return { success: true, result: true, From 284e10da0eac4a9902e64a8c294bffed3452866e Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Thu, 7 Mar 2024 17:20:25 +0800 Subject: [PATCH 09/21] Remove deleteByWorkspace since not exists Signed-off-by: Lin Wang --- ...space_saved_objects_client_wrapper.test.ts | 29 ------------------- .../workspace_saved_objects_client_wrapper.ts | 17 ----------- 2 files changed, 46 deletions(-) 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 d87fa8b8d3a2..e262b4610440 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 @@ -61,7 +61,6 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }; }), find: jest.fn(), - deleteByWorkspace: jest.fn(), }; const requestMock = {}; const wrapperOptions = { @@ -563,33 +562,5 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }); }); }); - describe('deleteByWorkspace', () => { - it('should call permission validate with workspace and throw workspace permission error if not permitted', async () => { - const { - wrapper, - requestMock, - permissionControlMock, - } = generateWorkspaceSavedObjectsClientWrapper(); - let errorCatched; - try { - await wrapper.deleteByWorkspace('not-permitted-workspace'); - } catch (e) { - errorCatched = e; - } - expect(errorCatched?.message).toEqual('Invalid workspace permission'); - expect(permissionControlMock.validate).toHaveBeenCalledWith( - requestMock, - { id: 'not-permitted-workspace', type: 'workspace' }, - ['library_write'] - ); - }); - - it('should call client.deleteByWorkspace if permitted', async () => { - const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); - - await wrapper.deleteByWorkspace('workspace-1', {}); - expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-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 2ca26f3486d4..8349dc5b1c31 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,7 +22,6 @@ import { SavedObjectsBulkUpdateResponse, SavedObjectsBulkUpdateOptions, WORKSPACE_TYPE, - SavedObjectsDeleteByWorkspaceOptions, SavedObjectsErrorHelpers, SavedObjectsServiceStart, SavedObjectsClientContract, @@ -509,21 +508,6 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.find(options); }; - const deleteByWorkspaceWithPermissionControl = async ( - workspace: string, - options: SavedObjectsDeleteByWorkspaceOptions = {} - ) => { - if ( - !(await this.validateMultiWorkspacesPermissions([workspace], wrapperOptions.request, [ - WorkspacePermissionMode.LibraryWrite, - ])) - ) { - throw generateWorkspacePermissionError(); - } - - return await wrapperOptions.client.deleteByWorkspace(workspace, options); - }; - return { ...wrapperOptions.client, get: getWithWorkspacePermissionControl, @@ -538,7 +522,6 @@ export class WorkspaceSavedObjectsClientWrapper { delete: deleteWithWorkspacePermissionControl, update: updateWithWorkspacePermissionControl, bulkUpdate: bulkUpdateWithWorkspacePermissionControl, - deleteByWorkspace: deleteByWorkspaceWithPermissionControl, }; }; From dd120e32ba8b2ad6f41c295bac0350f8a38a965e Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 8 Mar 2024 11:24:37 +0800 Subject: [PATCH 10/21] refactor: remove formatWorkspacePermissionModeToStringArray Signed-off-by: Lin Wang --- .../workspace_saved_objects_client_wrapper.ts | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) 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 8349dc5b1c31..4037b24f048c 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 @@ -67,22 +67,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( @@ -91,7 +82,7 @@ export class WorkspaceSavedObjectsClientWrapper { type, id, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (!validateResult?.result) { return false; @@ -104,20 +95,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) { @@ -130,7 +121,7 @@ export class WorkspaceSavedObjectsClientWrapper { type: WORKSPACE_TYPE, id: workspaceId, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (validateResult?.result) { return true; From b46caf58b16e65e0d9347ee0b0bf39512d40ea8b Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 8 Mar 2024 11:48:40 +0800 Subject: [PATCH 11/21] Remove current not used code Signed-off-by: Lin Wang --- .../workspace/server/workspace_client.ts | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index d647999c22b1..274d81340e34 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -11,7 +11,6 @@ import { WorkspaceAttribute, SavedObjectsServiceStart, ACL, - Permissions, } from '../../../core/server'; import { WORKSPACE_TYPE } from '../../../core/server'; import { @@ -24,10 +23,7 @@ import { } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { - WorkspacePermissionMode, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, -} from '../common/constants'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const convertToACL = ( workspacePermissions: WorkspacePermissionItem | WorkspacePermissionItem[] @@ -52,40 +48,6 @@ const convertToACL = ( return acl.getPermissions() || {}; }; -const isValidWorkspacePermissionMode = (mode: string): mode is WorkspacePermissionMode => - Object.values(WorkspacePermissionMode).some((modeValue) => modeValue === mode); - -const isWorkspacePermissionItem = ( - test: WorkspacePermissionItem | null -): test is WorkspacePermissionItem => test !== null; - -const convertFromACL = (permissions: Permissions) => { - const acl = new ACL(permissions); - - return acl - .toFlatList() - .map(({ name, permissions: modes, type }) => { - const validModes = modes.filter(isValidWorkspacePermissionMode); - switch (type) { - case 'users': - return { - type: 'user', - modes: validModes, - userId: name, - } as const; - case 'groups': - return { - type: 'group', - modes: validModes, - group: name, - } as const; - default: - return null; - } - }) - .filter(isWorkspacePermissionItem); -}; - const WORKSPACE_ID_SIZE = 6; const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name.error', { From e6c83e79e153c450f951b034998cedd895368700 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 8 Mar 2024 11:49:44 +0800 Subject: [PATCH 12/21] Add missing unit tests for permission control Signed-off-by: Lin Wang --- .../server/permission_control/client.test.ts | 77 ++++++++++++++++++ ...space_saved_objects_client_wrapper.test.ts | 80 ++++++++++++++++++- 2 files changed, 153 insertions(+), 4 deletions(-) diff --git a/src/plugins/workspace/server/permission_control/client.test.ts b/src/plugins/workspace/server/permission_control/client.test.ts index e05e299c153b..4d041cc7df56 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/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index e262b4610440..23e6c49f0254 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() @@ -81,14 +93,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, }; @@ -121,6 +135,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; @@ -156,6 +180,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 = [ @@ -259,6 +295,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 = [ @@ -267,11 +323,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 () => { @@ -548,6 +602,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({ From 2f375677698e9c7c86d177d42ba0f6e49db5f4b1 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Sun, 10 Mar 2024 16:46:56 +0800 Subject: [PATCH 13/21] Update workspaces API test describe Signed-off-by: Lin Wang --- src/plugins/workspace/server/integration_tests/routes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 73d3e0250a0a..351e6820bf1b 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -19,7 +19,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; From 131b0e589dde4a675ddad7d19597d8344799de24 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Mon, 11 Mar 2024 19:27:26 +0800 Subject: [PATCH 14/21] Fix workspace CRUD API integration tests failed Signed-off-by: Lin Wang --- .../workspace/server/integration_tests/routes.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 4fa4cf9c4bdf..629db3c902f7 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -33,7 +33,7 @@ describe('workspace service api integration test', () => { }, savedObjects: { permission: { - enabled: false, + enabled: true, }, }, migrations: { skip: false }, @@ -156,11 +156,10 @@ describe('workspace service api integration test', () => { .post(root, `/api/workspaces`) .send({ attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], }) .expect(200); - await osdTestServer.request + const updateResult = await osdTestServer.request .put(root, `/api/workspaces/${result.body.result.id}`) .send({ attributes: { @@ -169,6 +168,7 @@ describe('workspace service api integration test', () => { permissions: [{ type: 'user', userId: 'foo', modes: ['write'] }], }) .expect(200); + expect(updateResult.body.result).toBe(true); expect( ( From 3a1f63f30076f638f5182434d6fa2051e51c8563 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 12 Mar 2024 17:10:09 +0800 Subject: [PATCH 15/21] Address PR comments Signed-off-by: Lin Wang --- .../server/permission_control/client.ts | 60 +++++++++---------- src/plugins/workspace/server/plugin.ts | 5 +- src/plugins/workspace/server/routes/index.ts | 6 +- 3 files changed, 31 insertions(+), 40 deletions(-) diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index d5a0e80dae1e..bdc67f830913 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -30,6 +30,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], @@ -82,6 +89,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, @@ -100,7 +116,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; }); @@ -144,38 +165,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.ts b/src/plugins/workspace/server/plugin.ts index e263e2141c94..e0afd0e6e38b 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -43,10 +43,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { public async setup(core: CoreSetup) { this.logger.debug('Setting up Workspaces service'); const globalConfig = await this.globalConfig$.pipe(first()).toPromise(); - const isPermissionControlEnabled = - globalConfig.savedObjects.permission.enabled === undefined - ? true - : globalConfig.savedObjects.permission.enabled; + const isPermissionControlEnabled = globalConfig.savedObjects.permission.enabled === true; this.client = new WorkspaceClient(core); diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 585389e1d250..3620b73899d8 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -124,7 +124,7 @@ export function registerRoutes({ }, router.handleLegacyErrors(async (context, req, res) => { const { attributes, permissions: permissionsInRequest } = req.body; - const authInfo = permissionControlClient?.getPrincipalsFromRequest(req); + const principals = permissionControlClient?.getPrincipalsFromRequest(req); let permissions: WorkspacePermissionItem[] = []; if (permissionsInRequest) { permissions = Array.isArray(permissionsInRequest) @@ -133,10 +133,10 @@ export function registerRoutes({ } // Assign workspace owner to current user - if (!!authInfo?.users?.length) { + if (!!principals?.users?.length) { permissions.push({ type: 'user', - userId: authInfo.users[0], + userId: principals.users[0], modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], }); } From 6e7a4630c4f8b3948693d3f60341179419a83765 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Tue, 12 Mar 2024 17:16:54 +0800 Subject: [PATCH 16/21] Store permissions when savedObjects.permissions.enabled Signed-off-by: Lin Wang --- .../server/integration_tests/routes.test.ts | 160 ++++++++++++------ src/plugins/workspace/server/plugin.ts | 1 + src/plugins/workspace/server/routes/index.ts | 8 +- 3 files changed, 112 insertions(+), 57 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 629db3c902f7..8739addaca10 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -33,7 +33,7 @@ describe('workspace service api integration test', () => { }, savedObjects: { permission: { - enabled: true, + enabled: false, }, }, migrations: { skip: false }, @@ -84,33 +84,6 @@ describe('workspace service api integration test', () => { 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) - ).permissions - ).toEqual({ read: { users: ['foo'] } }); - }); it('get', async () => { const result = await osdTestServer.request .post(root, `/api/workspaces`) @@ -151,33 +124,6 @@ describe('workspace service api integration test', () => { 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), - }) - .expect(200); - - const updateResult = 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(updateResult.body.result).toBe(true); - - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).permissions - ).toEqual({ write: { users: ['foo'] } }); - }); it('delete', async () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) @@ -320,3 +266,107 @@ describe('workspace service api integration test', () => { }); }); }); + +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: [{ 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) + ).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: [{ type: 'user', userId: 'foo', modes: ['write'] }], + }) + .expect(200); + expect(updateResult.body.result).toBe(true); + + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ write: { users: ['foo'] } }); + }); + }); +}); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index e0afd0e6e38b..fd862a9dae19 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -77,6 +77,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 3620b73899d8..4fa3751a4425 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -46,11 +46,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( @@ -149,7 +151,7 @@ export function registerRoutes({ }, { ...attributes, - ...(permissions.length ? { permissions } : {}), + ...(isPermissionControlEnabled && permissions.length ? { permissions } : {}), } ); return res.ok({ body: result }); @@ -187,7 +189,9 @@ export function registerRoutes({ id, { ...attributes, - ...(finalPermissions.length ? { permissions: finalPermissions } : {}), + ...(isPermissionControlEnabled && finalPermissions.length + ? { permissions: finalPermissions } + : {}), } ); return res.ok({ body: result }); From 5d7d64fa9a18b765f4b215f3dbffd7f7857d6897 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Wed, 13 Mar 2024 14:23:12 +0800 Subject: [PATCH 17/21] Add permission control for deleteByWorkspace Signed-off-by: Lin Wang --- ...space_saved_objects_client_wrapper.test.ts | 66 +++++++++++++++++++ ...space_saved_objects_client_wrapper.test.ts | 30 +++++++++ .../workspace_saved_objects_client_wrapper.ts | 17 +++++ 3 files changed, 113 insertions(+) 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 2a7fb0e440b5..b6ea26456f0e 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 23e6c49f0254..186ecda0d8ba 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 @@ -73,6 +73,7 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }; }), find: jest.fn(), + deleteByWorkspace: jest.fn(), }; const requestMock = {}; const wrapperOptions = { @@ -634,5 +635,34 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }); }); }); + + describe('deleteByWorkspace', () => { + it('should call permission validate with workspace and throw workspace permission error if not permitted', async () => { + const { + wrapper, + requestMock, + permissionControlMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.deleteByWorkspace('not-permitted-workspace'); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid workspace permission'); + expect(permissionControlMock.validate).toHaveBeenCalledWith( + requestMock, + { id: 'not-permitted-workspace', type: 'workspace' }, + ['library_write'] + ); + }); + + it('should call client.deleteByWorkspace if permitted', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); + + await wrapper.deleteByWorkspace('workspace-1', {}); + expect(clientMock.deleteByWorkspace).toHaveBeenCalledWith('workspace-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 4037b24f048c..30c1c91c4223 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 @@ -25,6 +25,7 @@ import { SavedObjectsErrorHelpers, SavedObjectsServiceStart, SavedObjectsClientContract, + SavedObjectsDeleteByWorkspaceOptions, } from '../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; import { @@ -499,6 +500,21 @@ export class WorkspaceSavedObjectsClientWrapper { return await wrapperOptions.client.find(options); }; + const deleteByWorkspaceWithPermissionControl = async ( + workspace: string, + options: SavedObjectsDeleteByWorkspaceOptions = {} + ) => { + if ( + !(await this.validateMultiWorkspacesPermissions([workspace], wrapperOptions.request, [ + WorkspacePermissionMode.LibraryWrite, + ])) + ) { + throw generateWorkspacePermissionError(); + } + + return await wrapperOptions.client.deleteByWorkspace(workspace, options); + }; + return { ...wrapperOptions.client, get: getWithWorkspacePermissionControl, @@ -513,6 +529,7 @@ export class WorkspaceSavedObjectsClientWrapper { delete: deleteWithWorkspacePermissionControl, update: updateWithWorkspacePermissionControl, bulkUpdate: bulkUpdateWithWorkspacePermissionControl, + deleteByWorkspace: deleteByWorkspaceWithPermissionControl, }; }; From ff66b7aa23aace3a982c859bb3dbcfa892da1a4a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 15 Mar 2024 10:34:33 +0800 Subject: [PATCH 18/21] Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/permission_control/client.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bdc67f830913..73d3c2264cf6 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -37,6 +37,13 @@ export class SavedObjectsPermissionControl { * @param request * @returns SavedObjectsContract */ + /** + * 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], From 81d6e28b6dc0044616adbc2bd1d64a5b9610437c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 19 Mar 2024 09:09:22 +0800 Subject: [PATCH 19/21] Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/permission_control/client.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index 73d3c2264cf6..bdc67f830913 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -37,13 +37,6 @@ export class SavedObjectsPermissionControl { * @param request * @returns SavedObjectsContract */ - /** - * 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], From 76523f53a125a513cbcee1f794a9a323be9af1d4 Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 22 Mar 2024 01:52:10 +0800 Subject: [PATCH 20/21] Refactor permissions field in workspace create and update API Signed-off-by: Lin Wang --- .../server/integration_tests/routes.test.ts | 7 +- src/plugins/workspace/server/routes/index.ts | 74 ++++++++----------- src/plugins/workspace/server/types.ts | 13 +--- .../workspace/server/workspace_client.ts | 29 +------- 4 files changed, 37 insertions(+), 86 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 8739addaca10..2c7137631c87 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; @@ -337,7 +336,7 @@ describe('workspace service api integration test when savedObjects.permission.en ( await osd.coreStart.savedObjects .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) ).permissions ).toEqual({ read: { users: ['foo'] } }); }); @@ -364,7 +363,7 @@ describe('workspace service api integration test when savedObjects.permission.en ( await osd.coreStart.savedObjects .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) ).permissions ).toEqual({ write: { users: ['foo'] } }); }); diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 4fa3751a4425..64baccf09184 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -4,9 +4,9 @@ */ import { schema } from '@osd/config-schema'; -import { CoreSetup, Logger } from '../../../../core/server'; +import { CoreSetup, Logger, PrincipalType, ACL } from '../../../../core/server'; import { WorkspacePermissionMode } from '../../common/constants'; -import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types'; +import { IWorkspaceClientImpl, WorkspaceAttributeWithPermission } from '../types'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -18,19 +18,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(), @@ -118,29 +115,29 @@ 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 { attributes, permissions } = req.body; const principals = permissionControlClient?.getPrincipalsFromRequest(req); - let permissions: WorkspacePermissionItem[] = []; - if (permissionsInRequest) { - permissions = Array.isArray(permissionsInRequest) - ? permissionsInRequest - : [permissionsInRequest]; - } + const createPayload: Omit = attributes; - // Assign workspace owner to current user - if (!!principals?.users?.length) { - permissions.push({ - type: 'user', - userId: principals.users[0], - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }); + if (isPermissionControlEnabled) { + const acl = new ACL(permissions); + // Assign workspace owner to current user + if (!!principals?.users?.length) { + 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( @@ -149,10 +146,7 @@ export function registerRoutes({ request: req, logger, }, - { - ...attributes, - ...(isPermissionControlEnabled && permissions.length ? { permissions } : {}), - } + createPayload ); return res.ok({ body: result }); }) @@ -166,19 +160,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( { @@ -189,9 +177,7 @@ export function registerRoutes({ id, { ...attributes, - ...(isPermissionControlEnabled && finalPermissions.length - ? { permissions: finalPermissions } - : {}), + ...(isPermissionControlEnabled ? { permissions } : {}), } ); return res.ok({ body: result }); diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index 9be3e383ed87..2973ea4dbc31 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -11,11 +11,11 @@ import { CoreSetup, WorkspaceAttribute, SavedObjectsServiceStart, + Permissions, } from '../../../core/server'; -import { WorkspacePermissionMode } from '../common/constants'; export interface WorkspaceAttributeWithPermission extends WorkspaceAttribute { - permissions?: WorkspacePermissionItem[]; + permissions?: Permissions; } export interface WorkspaceFindOptions { @@ -128,15 +128,6 @@ 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; } diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 6b7176a8b741..144ad65de9fc 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -10,7 +10,6 @@ import { CoreSetup, WorkspaceAttribute, SavedObjectsServiceStart, - ACL, } from '../../../core/server'; import { WORKSPACE_TYPE } from '../../../core/server'; import { @@ -19,35 +18,11 @@ import { IResponse, IRequestDetail, WorkspaceAttributeWithPermission, - WorkspacePermissionItem, } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; -const convertToACL = ( - workspacePermissions: WorkspacePermissionItem | WorkspacePermissionItem[] -) => { - workspacePermissions = Array.isArray(workspacePermissions) - ? workspacePermissions - : [workspacePermissions]; - - const acl = new ACL(); - - workspacePermissions.forEach((permission) => { - switch (permission.type) { - case 'user': - acl.addPermission(permission.modes, { users: [permission.userId] }); - return; - case 'group': - acl.addPermission(permission.modes, { groups: [permission.group] }); - return; - } - }); - - return acl.getPermissions() || {}; -}; - const WORKSPACE_ID_SIZE = 6; const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name.error', { @@ -119,7 +94,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { attributes, { id, - permissions: permissions ? convertToACL(permissions) : undefined, + permissions, } ); return { @@ -206,7 +181,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } await client.create>(WORKSPACE_TYPE, attributes, { id, - permissions: permissions ? convertToACL(permissions) : undefined, + permissions, overwrite: true, version: workspaceInDB.version, }); From 5854d9357f6db1e574655775222d6f21b2488e3c Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Fri, 22 Mar 2024 02:19:48 +0800 Subject: [PATCH 21/21] Fix workspace CRUD API integration tests Signed-off-by: Lin Wang --- .../workspace/server/integration_tests/routes.test.ts | 6 +++--- src/plugins/workspace/server/routes/index.ts | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 2c7137631c87..4ef7aeb13d5e 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -318,7 +318,7 @@ describe('workspace service api integration test when savedObjects.permission.en .post(root, `/api/workspaces`) .send({ attributes: omitId(testWorkspace), - permissions: [{ type: 'invalid-type', userId: 'foo', modes: ['read'] }], + permissions: { invalid_type: { users: ['foo'] } }, }) .expect(400); @@ -326,7 +326,7 @@ describe('workspace service api integration test when savedObjects.permission.en .post(root, `/api/workspaces`) .send({ attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], + permissions: { read: { users: ['foo'] } }, }) .expect(200); @@ -354,7 +354,7 @@ describe('workspace service api integration test when savedObjects.permission.en attributes: { ...omitId(testWorkspace), }, - permissions: [{ type: 'user', userId: 'foo', modes: ['write'] }], + permissions: { write: { users: ['foo'] } }, }) .expect(200); expect(updateResult.body.result).toBe(true); diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 64baccf09184..3e08fc298ea9 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -125,9 +125,10 @@ export function registerRoutes({ const createPayload: Omit = attributes; if (isPermissionControlEnabled) { - const acl = new ACL(permissions); + 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) => { @@ -136,8 +137,8 @@ export function registerRoutes({ } } ); + createPayload.permissions = acl.getPermissions(); } - createPayload.permissions = acl.getPermissions(); } const result = await client.create(