diff --git a/CHANGELOG.md b/CHANGELOG.md index d48fef6e3efa..da498dc4706a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Replace OuiSelect component with OuiSuperSelect in data-source plugin ([#5315](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5315)) - [Workspace] Add core workspace service module to enable the implementation of workspace features within OSD plugins ([#5092](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5092)) - [Workspace] Setup workspace skeleton and implement basic CRUD API ([#5075](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5075)) +- [Workspace] Add ACL related functions ([#5084](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5084/)) - [Decouple] Add new cross compatibility check core service which export functionality for plugins to verify if their OpenSearch plugin counterpart is installed on the cluster or has incompatible version to configure the plugin behavior([#4710](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4710)) - [Discover] Add long numerals support [#5592](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5592) - [Discover] Display inner properties in the left navigation bar [#5429](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5429) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 523a483d1cec..6e8ded0d1e62 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -276,8 +276,10 @@ # Set the value of this setting to true to enable plugin augmentation on Dashboard # vis_augmenter.pluginAugmentationEnabled: true +# Set the value to true to enable permission control for saved objects +# Permission control depends on OpenSearch Dashboards has authentication enabled, set it to false when the security plugin is not installed, +# if the security plugin is not installed and this config is true, permission control takes no effect. +# savedObjects.permission.enabled: true + # Set the value to true to enable workspace feature # workspace.enabled: false -# Set the value to false to disable permission check on workspace -# Permission check depends on OpenSearch Dashboards has authentication enabled, set it to false if no authentication is configured -# workspace.permission.enabled: true diff --git a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap index 6f67893104e7..f8ef47cae894 100644 --- a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap +++ b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap @@ -10,7 +10,6 @@ Object { "namespace": "2f4316de49999235636386fe51dc06c1", "namespaces": "2f4316de49999235636386fe51dc06c1", "originId": "2f4316de49999235636386fe51dc06c1", - "permissions": "07c04cdd060494956fdddaa7ef86e8ac", "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", @@ -37,60 +36,6 @@ Object { "originId": Object { "type": "keyword", }, - "permissions": Object { - "properties": Object { - "library_read": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "library_write": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "management": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "read": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "write": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - }, - }, "references": Object { "properties": Object { "id": Object { @@ -124,7 +69,6 @@ Object { "namespace": "2f4316de49999235636386fe51dc06c1", "namespaces": "2f4316de49999235636386fe51dc06c1", "originId": "2f4316de49999235636386fe51dc06c1", - "permissions": "07c04cdd060494956fdddaa7ef86e8ac", "references": "7997cf5a56cc02bdc9c93361bde732b0", "secondType": "72d57924f415fbadb3ee293b67d233ab", "thirdType": "510f1f0adb69830cf8a1c5ce2923ed82", @@ -155,60 +99,6 @@ Object { "originId": Object { "type": "keyword", }, - "permissions": Object { - "properties": Object { - "library_read": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "library_write": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "management": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "read": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "write": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - }, - }, "references": Object { "properties": Object { "id": Object { diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts index 4acc161c4bab..5fb3bb3b4c8a 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts @@ -93,6 +93,12 @@ describe('buildActiveMappings', () => { expect(hashes.aaa).not.toEqual(hashes.ccc); }); + test('permissions field is added when permission control flag is enabled', () => { + const rawConfig = configMock.create(); + rawConfig.get.mockReturnValue(true); + expect(buildActiveMappings({}, rawConfig)).toHaveProperty('properties.permissions'); + }); + test('workspaces field is added when workspace feature flag is enabled', () => { const rawConfig = configMock.create(); rawConfig.get.mockReturnValue(true); diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts index d2978dc5abf6..55b73daabc3e 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts @@ -56,6 +56,29 @@ export function buildActiveMappings( let mergedProperties = validateAndMerge(mapping.properties, typeDefinitions); // if permission control for saved objects is enabled, the permissions field should be added to the mapping + if (opensearchDashboardsRawConfig?.get('savedObjects.permission.enabled')) { + const principals: SavedObjectsFieldMapping = { + properties: { + users: { + type: 'keyword', + }, + groups: { + type: 'keyword', + }, + }, + }; + mergedProperties = validateAndMerge(mapping.properties, { + permissions: { + properties: { + read: principals, + write: principals, + library_read: principals, + library_write: principals, + }, + }, + }); + } + if (opensearchDashboardsRawConfig?.get('workspace.enabled')) { mergedProperties = validateAndMerge(mapping.properties, { workspaces: { @@ -148,16 +171,6 @@ function findChangedProp(actual: any, expected: any) { * @returns {IndexMapping} */ function defaultMapping(): IndexMapping { - const principals: SavedObjectsFieldMapping = { - properties: { - users: { - type: 'keyword', - }, - groups: { - type: 'keyword', - }, - }, - }; return { dynamic: 'strict', properties: { @@ -196,15 +209,6 @@ function defaultMapping(): IndexMapping { }, }, }, - permissions: { - properties: { - read: principals, - write: principals, - management: principals, - library_read: principals, - library_write: principals, - }, - }, }, }; } diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 728e508e3c23..8b1f5df9640a 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -60,12 +60,18 @@ describe('IndexMigrator', () => { }; }); - test('creates the index when workspaces feature flag is enabled', async () => { + test('creates the index when permission control for saved objects is enabled', async () => { const { client } = testOpts; testOpts.mappingProperties = { foo: { type: 'long' } as any }; const rawConfig = configMock.create(); - rawConfig.get.mockReturnValue(true); + rawConfig.get.mockImplementation((path) => { + if (path === 'savedObjects.permission.enabled') { + return true; + } else { + return false; + } + }); testOpts.opensearchDashboardsRawConfig = rawConfig; withIndex(client, { index: { statusCode: 404 }, alias: { statusCode: 404 } }); @@ -83,11 +89,10 @@ describe('IndexMigrator', () => { namespace: '2f4316de49999235636386fe51dc06c1', namespaces: '2f4316de49999235636386fe51dc06c1', originId: '2f4316de49999235636386fe51dc06c1', + permissions: 'f3ad308fa2a0c34007eb9ad461d6294a', references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - workspaces: '2f4316de49999235636386fe51dc06c1', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, properties: { @@ -98,69 +103,42 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - references: { - type: 'nested', - properties: { - name: { type: 'keyword' }, - type: { type: 'keyword' }, - id: { type: 'keyword' }, - }, - }, - workspaces: { type: 'keyword' }, permissions: { properties: { library_read: { properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, + users: { type: 'keyword' }, + groups: { type: 'keyword' }, }, }, library_write: { properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - management: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, + users: { type: 'keyword' }, + groups: { type: 'keyword' }, }, }, read: { properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, + users: { type: 'keyword' }, + groups: { type: 'keyword' }, }, }, write: { properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, + users: { type: 'keyword' }, + groups: { type: 'keyword' }, }, }, }, }, + references: { + type: 'nested', + properties: { + name: { type: 'keyword' }, + type: { type: 'keyword' }, + id: { type: 'keyword' }, + }, + }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -169,10 +147,19 @@ describe('IndexMigrator', () => { }); }); - test('creates the index if it does not exist', async () => { + test('creates the index when workspaces feature flag is enabled', async () => { const { client } = testOpts; testOpts.mappingProperties = { foo: { type: 'long' } as any }; + const rawConfig = configMock.create(); + rawConfig.get.mockImplementation((path) => { + if (path === 'workspace.enabled') { + return true; + } else { + return false; + } + }); + testOpts.opensearchDashboardsRawConfig = rawConfig; withIndex(client, { index: { statusCode: 404 }, alias: { statusCode: 404 } }); @@ -192,7 +179,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -203,60 +190,56 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - permissions: { + references: { + type: 'nested', properties: { - library_read: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - library_write: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - management: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - read: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - write: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, + name: { type: 'keyword' }, + type: { type: 'keyword' }, + id: { type: 'keyword' }, }, }, + workspaces: { type: 'keyword' }, + }, + }, + settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, + }, + index: '.kibana_1', + }); + }); + + test('creates the index if it does not exist', async () => { + const { client } = testOpts; + + testOpts.mappingProperties = { foo: { type: 'long' } as any }; + + withIndex(client, { index: { statusCode: 404 }, alias: { statusCode: 404 } }); + + await new IndexMigrator(testOpts).migrate(); + + expect(client.indices.create).toHaveBeenCalledWith({ + body: { + mappings: { + dynamic: 'strict', + _meta: { + migrationMappingPropertyHashes: { + foo: '18c78c995965207ed3f6e7fc5c6e55fe', + migrationVersion: '4a1746014a75ade3a714e1db5763276f', + namespace: '2f4316de49999235636386fe51dc06c1', + namespaces: '2f4316de49999235636386fe51dc06c1', + originId: '2f4316de49999235636386fe51dc06c1', + references: '7997cf5a56cc02bdc9c93361bde732b0', + type: '2f4316de49999235636386fe51dc06c1', + updated_at: '00da57df13e94e9d98437d13ace4bfe0', + }, + }, + properties: { + foo: { type: 'long' }, + migrationVersion: { dynamic: 'true', type: 'object' }, + namespace: { type: 'keyword' }, + namespaces: { type: 'keyword' }, + originId: { type: 'keyword' }, + type: { type: 'keyword' }, + updated_at: { type: 'date' }, references: { type: 'nested', properties: { @@ -364,7 +347,6 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, properties: { @@ -376,60 +358,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - permissions: { - properties: { - library_read: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - library_write: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - management: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - read: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - write: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - }, - }, references: { type: 'nested', properties: { @@ -480,7 +408,6 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, properties: { @@ -492,60 +419,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - permissions: { - properties: { - library_read: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - library_write: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - management: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - read: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - write: { - properties: { - groups: { - type: 'keyword', - }, - users: { - type: 'keyword', - }, - }, - }, - }, - }, references: { type: 'nested', properties: { diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap index 5e39af788d79..baebb7848798 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap @@ -10,7 +10,6 @@ Object { "namespace": "2f4316de49999235636386fe51dc06c1", "namespaces": "2f4316de49999235636386fe51dc06c1", "originId": "2f4316de49999235636386fe51dc06c1", - "permissions": "07c04cdd060494956fdddaa7ef86e8ac", "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", @@ -45,60 +44,6 @@ Object { "originId": Object { "type": "keyword", }, - "permissions": Object { - "properties": Object { - "library_read": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "library_write": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "management": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "read": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - "write": Object { - "properties": Object { - "groups": Object { - "type": "keyword", - }, - "users": Object { - "type": "keyword", - }, - }, - }, - }, - }, "references": Object { "properties": Object { "id": Object { diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts b/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts index b0350a00b211..e65effdd8eaa 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts @@ -78,8 +78,14 @@ describe('OpenSearchDashboardsMigrator', () => { expect(mappings).toMatchSnapshot(); }); + it('permissions field exists in the mappings when the feature is enabled', () => { + const options = mockOptions(false, true); + const mappings = new OpenSearchDashboardsMigrator(options).getActiveMappings(); + expect(mappings).toHaveProperty('properties.permissions'); + }); + it('workspaces field exists in the mappings when the feature is enabled', () => { - const options = mockOptions(true); + const options = mockOptions(true, false); const mappings = new OpenSearchDashboardsMigrator(options).getActiveMappings(); expect(mappings).toHaveProperty('properties.workspaces'); }); @@ -153,12 +159,29 @@ type MockedOptions = OpenSearchDashboardsMigratorOptions & { client: ReturnType; }; -const mockOptions = (isWorkspaceEnabled?: boolean) => { +const mockOptions = (isWorkspaceEnabled?: boolean, isPermissionControlEnabled?: boolean) => { const rawConfig = configMock.create(); rawConfig.get.mockReturnValue(false); - if (isWorkspaceEnabled) { + if (isWorkspaceEnabled || isPermissionControlEnabled) { rawConfig.get.mockReturnValue(true); } + rawConfig.get.mockImplementation((path) => { + if (path === 'savedObjects.permission.enabled') { + if (isPermissionControlEnabled) { + return true; + } else { + return false; + } + } else if (path === 'workspace.enabled') { + if (isWorkspaceEnabled) { + return true; + } else { + return false; + } + } else { + return false; + } + }); const options: MockedOptions = { logger: loggingSystemMock.create().get(), opensearchDashboardsVersion: '8.2.3', diff --git a/src/core/server/saved_objects/permission_control/acl.test.ts b/src/core/server/saved_objects/permission_control/acl.test.ts index 3c71ac82bda1..184c10a36aaa 100644 --- a/src/core/server/saved_objects/permission_control/acl.test.ts +++ b/src/core/server/saved_objects/permission_control/acl.test.ts @@ -5,9 +5,7 @@ import { Principals, Permissions, ACL } from './acl'; -describe('SavedObjectTypeRegistry', () => { - let acl: ACL; - +describe('acl', () => { it('test has permission', () => { const principals: Principals = { users: ['user1'], @@ -16,97 +14,150 @@ describe('SavedObjectTypeRegistry', () => { const permissions: Permissions = { read: principals, }; - acl = new ACL(permissions); + const acl = new ACL(permissions); expect( acl.hasPermission(['read'], { users: ['user1'], groups: [], }) ).toEqual(true); + expect( acl.hasPermission(['read'], { users: ['user2'], groups: [], }) ).toEqual(false); + + expect( + acl.hasPermission([], { + users: ['user2'], + groups: [], + }) + ).toEqual(false); + + const nullValue: unknown = undefined; + expect(acl.hasPermission(['read'], nullValue as Principals)).toEqual(false); + expect(acl.hasPermission(['read'], {})).toEqual(false); + + acl.resetPermissions(); + expect(acl.hasPermission(['read'], nullValue as Principals)).toEqual(false); + expect(acl.hasPermission(['read'], {})).toEqual(false); + expect(acl.hasPermission(['read'], principals)).toEqual(false); }); it('test add permission', () => { - acl = new ACL(); - const result1 = acl + const acl = new ACL(); + let result = acl .addPermission(['read'], { users: ['user1'], groups: [], }) .getPermissions(); - expect(result1?.read?.users).toEqual(['user1']); + expect(result?.read?.users).toEqual(['user1']); acl.resetPermissions(); - const result2 = acl - .addPermission(['write', 'management'], { + result = acl + .addPermission(['write', 'library_write'], { users: ['user2'], groups: ['group1', 'group2'], }) .getPermissions(); - expect(result2?.write?.users).toEqual(['user2']); - expect(result2?.management?.groups).toEqual(['group1', 'group2']); + expect(result?.write?.users).toEqual(['user2']); + expect(result?.library_write?.groups).toEqual(['group1', 'group2']); + + acl.resetPermissions(); + result = acl + .addPermission(['write', 'library_write'], { + users: ['user2'], + }) + .addPermission(['write', 'library_write'], { + groups: ['group1'], + }) + .getPermissions(); + expect(result?.write?.users).toEqual(['user2']); + expect(result?.write?.groups).toEqual(['group1']); + expect(result?.library_write?.users).toEqual(['user2']); + expect(result?.library_write?.groups).toEqual(['group1']); + + acl.resetPermissions(); + const nullValue: unknown = undefined; + result = acl.addPermission([], nullValue as Principals).getPermissions(); + expect(result).toEqual({}); + + acl.resetPermissions(); + result = acl.addPermission(nullValue as string[], {} as Principals).getPermissions(); + expect(result).toEqual({}); }); it('test remove permission', () => { - const principals1: Principals = { + let principals: Principals = { users: ['user1'], groups: ['group1', 'group2'], }; - const permissions1 = { - read: principals1, - write: principals1, + let permissions = { + read: principals, + write: principals, }; - acl = new ACL(permissions1); - const result1 = acl + let acl = new ACL(permissions); + let result = acl .removePermission(['read'], { users: ['user1'], - groups: [], }) .removePermission(['write'], { - users: [], groups: ['group2'], }) + .removePermission(['write'], { + users: ['user3'], + groups: ['group3'], + }) + .removePermission(['library_write'], { + users: ['user1'], + groups: ['group1'], + }) .getPermissions(); - expect(result1?.read?.users).toEqual([]); - expect(result1?.write?.groups).toEqual(['group1']); + expect(result?.read?.users).toEqual([]); + expect(result?.write?.groups).toEqual(['group1']); - const principals2: Principals = { + principals = { users: ['*'], groups: ['*'], }; - - const permissions2 = { - read: principals2, - write: principals2, + permissions = { + read: principals, + write: principals, }; - - acl = new ACL(permissions2); - const result2 = acl + acl = new ACL(permissions); + result = acl .removePermission(['read', 'write'], { users: ['user1'], groups: ['group1'], }) .getPermissions(); - expect(result2?.read?.users).toEqual(['*']); - expect(result2?.write?.groups).toEqual(['*']); + expect(result?.read?.users).toEqual(['*']); + expect(result?.write?.groups).toEqual(['*']); + + acl.resetPermissions(); + const nullValue: unknown = undefined; + result = acl.removePermission([], nullValue as Principals).getPermissions(); + expect(result).toEqual({}); + + acl.resetPermissions(); + result = acl.removePermission(nullValue as string[], principals).getPermissions(); + expect(result).toEqual({}); }); - it('test transform permission', () => { - const principals: Principals = { + it('test toFlatList', () => { + let principals: Principals = { users: ['user1'], groups: ['group1', 'group2'], }; - const permissions = { + let permissions = { read: principals, write: principals, }; - acl = new ACL(permissions); - const result = acl.toFlatList(); + let acl = new ACL(permissions); + let result = acl.toFlatList(); expect(result).toHaveLength(3); expect(result).toEqual( expect.arrayContaining([{ type: 'users', name: 'user1', permissions: ['read', 'write'] }]) @@ -117,14 +168,63 @@ describe('SavedObjectTypeRegistry', () => { expect(result).toEqual( expect.arrayContaining([{ type: 'groups', name: 'group2', permissions: ['read', 'write'] }]) ); + + acl.resetPermissions(); + principals = { + users: ['user1'], + }; + permissions = { + read: principals, + write: principals, + }; + acl = new ACL(permissions); + result = acl.toFlatList(); + expect(result).toHaveLength(1); + expect(result).toEqual( + expect.arrayContaining([{ type: 'users', name: 'user1', permissions: ['read', 'write'] }]) + ); + + acl.resetPermissions(); + principals = { + groups: ['group1', 'group2'], + }; + permissions = { + read: principals, + write: principals, + }; + acl = new ACL(permissions); + result = acl.toFlatList(); + expect(result).toHaveLength(2); + expect(result).toEqual( + expect.arrayContaining([{ type: 'groups', name: 'group1', permissions: ['read', 'write'] }]) + ); + expect(result).toEqual( + expect.arrayContaining([{ type: 'groups', name: 'group2', permissions: ['read', 'write'] }]) + ); }); it('test generate query DSL', () => { + const nullValue: unknown = undefined; + let result = ACL.generateGetPermittedSavedObjectsQueryDSL(['read'], nullValue as Principals); + expect(result).toEqual({ + query: { + match_none: {}, + }, + }); + const principals = { users: ['user1'], groups: ['group1'], }; - const result = ACL.generateGetPermittedSavedObjectsQueryDSL(['read'], principals, 'workspace'); + + result = ACL.generateGetPermittedSavedObjectsQueryDSL(nullValue as string[], principals); + expect(result).toEqual({ + query: { + match_none: {}, + }, + }); + + result = ACL.generateGetPermittedSavedObjectsQueryDSL(['read'], principals, 'workspace'); expect(result).toEqual({ query: { bool: { @@ -164,5 +264,85 @@ describe('SavedObjectTypeRegistry', () => { }, }, }); + + result = ACL.generateGetPermittedSavedObjectsQueryDSL(['read'], principals, [ + 'workspace', + 'index-pattern', + ]); + expect(result).toEqual({ + query: { + bool: { + filter: [ + { + bool: { + should: [ + { + terms: { + 'permissions.read.users': ['user1'], + }, + }, + { + term: { + 'permissions.read.users': '*', + }, + }, + { + terms: { + 'permissions.read.groups': ['group1'], + }, + }, + { + term: { + 'permissions.read.groups': '*', + }, + }, + ], + }, + }, + { + terms: { + type: ['workspace', 'index-pattern'], + }, + }, + ], + }, + }, + }); + + result = ACL.generateGetPermittedSavedObjectsQueryDSL(['read'], principals); + expect(result).toEqual({ + query: { + bool: { + filter: [ + { + bool: { + should: [ + { + terms: { + 'permissions.read.users': ['user1'], + }, + }, + { + term: { + 'permissions.read.users': '*', + }, + }, + { + terms: { + 'permissions.read.groups': ['group1'], + }, + }, + { + term: { + 'permissions.read.groups': '*', + }, + }, + ], + }, + }, + ], + }, + }, + }); }); }); diff --git a/src/core/server/saved_objects/permission_control/acl.ts b/src/core/server/saved_objects/permission_control/acl.ts index 1631b0cbef46..769304fe8736 100644 --- a/src/core/server/saved_objects/permission_control/acl.ts +++ b/src/core/server/saved_objects/permission_control/acl.ts @@ -21,17 +21,22 @@ export interface TransformedPermission { permissions: string[]; } -const addToPrincipals = (principals?: Principals, users?: string[], groups?: string[]) => { - if (!principals) { - principals = {}; - } - if (!!users) { +const addToPrincipals = ({ + principals = {}, + users, + groups, +}: { + principals: Principals; + users?: string[]; + groups?: string[]; +}) => { + if (users) { if (!principals.users) { principals.users = []; } principals.users = Array.from(new Set([...principals.users, ...users])); } - if (!!groups) { + if (groups) { if (!principals.groups) { principals.groups = []; } @@ -40,39 +45,50 @@ const addToPrincipals = (principals?: Principals, users?: string[], groups?: str return principals; }; -const deleteFromPrincipals = (principals?: Principals, users?: string[], groups?: string[]) => { +const deleteFromPrincipals = ({ + principals, + users, + groups, +}: { + principals?: Principals; + users?: string[]; + groups?: string[]; +}) => { if (!principals) { return principals; } - if (!!users && !!principals.users) { + if (users && principals.users) { principals.users = principals.users.filter((item) => !users.includes(item)); } - if (!!groups && !!principals.groups) { + if (groups && principals.groups) { principals.groups = principals.groups.filter((item) => !groups.includes(item)); } return principals; }; -const checkPermission = (currentPrincipals: Principals | undefined, principals: Principals) => { +const checkPermission = ( + allowedPrincipals: Principals | undefined, + requestedPrincipals: Principals +) => { return ( - (currentPrincipals?.users && - principals?.users && - checkPermissionForSinglePrincipalType(currentPrincipals.users, principals.users)) || - (currentPrincipals?.groups && - principals.groups && - checkPermissionForSinglePrincipalType(currentPrincipals.groups, principals.groups)) + (allowedPrincipals?.users && + requestedPrincipals?.users && + checkPermissionForSinglePrincipalType(allowedPrincipals.users, requestedPrincipals.users)) || + (allowedPrincipals?.groups && + requestedPrincipals?.groups && + checkPermissionForSinglePrincipalType(allowedPrincipals.groups, requestedPrincipals.groups)) ); }; const checkPermissionForSinglePrincipalType = ( - currentPrincipalArray: string[], - principalArray: string[] + allowedPrincipalArray: string[], + requestedPrincipalArray: string[] ) => { return ( - currentPrincipalArray && - principalArray && - (currentPrincipalArray.includes('*') || - principalArray.some((item) => currentPrincipalArray.includes(item))) + allowedPrincipalArray && + requestedPrincipalArray && + (allowedPrincipalArray.includes('*') || + requestedPrincipalArray.some((item) => allowedPrincipalArray.includes(item))) ); }; @@ -82,7 +98,15 @@ export class ACL { this.permissions = initialPermissions || {}; } - // parse the permissions object to check whether the specific principal has the specific permission types or not + /** + * A function that parses the permissions object to check whether the specific principal has the specific permission types or not + * + * @param {Array} permissionTypes permission types + * @param {Object} principals the users or groups + * @returns true if the principal has the specified permission types, false if the principal has no permission + * + * @public + */ public hasPermission(permissionTypes: string[], principals: Principals) { if (!permissionTypes || permissionTypes.length === 0 || !this.permissions || !principals) { return false; @@ -94,7 +118,27 @@ export class ACL { ); } - // permissions object build function, add principal with specific permission to the object + /** + * A permissions object build function that adds principal with specific permission to the object + * + * This function is used to contruct a new permissions object or add principals with specified permissions to + * the existing permissions object. The usage is: + * + * const permissionObject = new ACL() + * .addPermission(['write', 'library_write'], { + * users: ['user2'], + * }) + * .addPermission(['write', 'library_write'], { + * groups: ['group1'], + * }) + * .getPermissions(); + * + * @param {Array} permissionTypes the permission types + * @param {Object} principals the users or groups + * @returns the permissions object + * + * @public + */ public addPermission(permissionTypes: string[], principals: Principals) { if (!permissionTypes || !principals) { return this; @@ -104,17 +148,37 @@ export class ACL { } for (const permissionType of permissionTypes) { - this.permissions[permissionType] = addToPrincipals( - this.permissions[permissionType], - principals.users, - principals.groups - ); + this.permissions[permissionType] = addToPrincipals({ + principals: this.permissions[permissionType], + users: principals.users, + groups: principals.groups, + }); } return this; } - // permissions object build function, remove specific permission of specific principal from the object + /** + * A permissions object build function that removes specific permission of specific principal from the object + * + * This function is used to remove principals with specified permissions to + * the existing permissions object. The usage is: + * + * const newPermissionObject = new ACL() + * .removePermission(['write', 'library_write'], { + * users: ['user2'], + * }) + * .removePermission(['write', 'library_write'], { + * groups: ['group1'], + * }) + * .getPermissions(); + * + * @param {Array} permissionTypes the permission types + * @param {Object} principals the users or groups + * @returns the permissions object + * + * @public + */ public removePermission(permissionTypes: string[], principals: Principals) { if (!permissionTypes || !principals) { return this; @@ -124,11 +188,11 @@ export class ACL { } for (const permissionType of permissionTypes) { - const result = deleteFromPrincipals( - this.permissions![permissionType], - principals.users, - principals.groups - ); + const result = deleteFromPrincipals({ + principals: this.permissions![permissionType], + users: principals.users, + groups: principals.groups, + }); if (result) { this.permissions[permissionType] = result; } @@ -138,8 +202,11 @@ export class ACL { } /** - * transform permissions format - * original permissions: { + * A function that transforms permissions format, change the format from permissionType->principals to principal->permissionTypes, + * which is used to clearyly dispaly user/group list and their granted permissions in the UI + * + * for example: + * the original permissions object is: { * read: { * users:['user1'] * }, @@ -148,10 +215,14 @@ export class ACL { * } * } * - * transformed permissions: [ - * {type:'users',name:'user1',permissions:['read']}, - * {type:'groups',name:'group1',permissions:['write']}, + * the transformed permissions object will be: [ + * {type:'users', name:'user1', permissions:['read']}, + * {type:'groups', name:'group1', permissions:['write']}, * ] + * + * @returns the flat list of the permissions object + * + * @public */ public toFlatList(): TransformedPermission[] { const result: TransformedPermission[] = []; @@ -184,18 +255,35 @@ export class ACL { return result; } + /** + * A permissions object build function that resets the permissions object + * + * @public + */ public resetPermissions() { // reset permissions this.permissions = {}; } - // return the permissions object + /** + * A function that gets the premissions object + * + * @public + */ public getPermissions() { return this.permissions; } /** - * generate query DSL by the specific conditions, used for fetching saved objects from the saved objects index + * A function that generates query DSL by the specific conditions, used for fetching saved objects from the saved objects index + * + * @param {Array} permissionTypes the permission types + * @param {Object} principals the users or groups + * @param {String | Array} savedObjectType saved object type, such as workspace, index-pattern etc. + * @returns the generated query DSL + * + * @public + * @static */ public static generateGetPermittedSavedObjectsQueryDSL( permissionTypes: string[], @@ -236,7 +324,7 @@ export class ACL { bool: subBool, }); - if (!!savedObjectType) { + if (savedObjectType) { bool.filter.push({ terms: { type: Array.isArray(savedObjectType) ? savedObjectType : [savedObjectType], diff --git a/src/core/server/saved_objects/permission_control/index.ts b/src/core/server/saved_objects/permission_control/index.ts new file mode 100644 index 000000000000..f0e41a125b1c --- /dev/null +++ b/src/core/server/saved_objects/permission_control/index.ts @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export { ACL, Permissions, Principals, PrincipalType, TransformedPermission } from './acl'; diff --git a/src/core/server/saved_objects/saved_objects_config.ts b/src/core/server/saved_objects/saved_objects_config.ts index 291350bf93a6..e6ffaefb8a59 100644 --- a/src/core/server/saved_objects/saved_objects_config.ts +++ b/src/core/server/saved_objects/saved_objects_config.ts @@ -49,6 +49,9 @@ export const savedObjectsConfig = { schema: schema.object({ maxImportPayloadBytes: schema.byteSize({ defaultValue: 26214400 }), maxImportExportSize: schema.byteSize({ defaultValue: 10000 }), + permission: schema.object({ + enabled: schema.boolean({ defaultValue: false }), + }), }), }; diff --git a/src/core/server/saved_objects/saved_objects_service.test.ts b/src/core/server/saved_objects/saved_objects_service.test.ts index 75b0d756f0cf..02eaff20331c 100644 --- a/src/core/server/saved_objects/saved_objects_service.test.ts +++ b/src/core/server/saved_objects/saved_objects_service.test.ts @@ -41,7 +41,7 @@ import { errors as opensearchErrors } from '@opensearch-project/opensearch'; import { SavedObjectsService } from './saved_objects_service'; import { mockCoreContext } from '../core_context.mock'; -import { Env } from '../config'; +import { Config, Env, ObjectToConfigAdapter } from '../config'; import { configServiceMock, savedObjectsRepositoryMock } from '../mocks'; import { opensearchServiceMock } from '../opensearch/opensearch_service.mock'; import { opensearchClientMock } from '../opensearch/client/mocks'; @@ -70,6 +70,13 @@ describe('SavedObjectsService', () => { maxImportExportSize: new ByteSizeValue(0), }); }); + const config$ = new BehaviorSubject( + new ObjectToConfigAdapter({ + savedObjects: { permission: { enabled: true } }, + }) + ); + + configService.getConfig$.mockReturnValue(config$); return mockCoreContext.create({ configService, env }); }; diff --git a/src/core/server/saved_objects/serialization/serializer.ts b/src/core/server/saved_objects/serialization/serializer.ts index 492379068cdb..b6c240022deb 100644 --- a/src/core/server/saved_objects/serialization/serializer.ts +++ b/src/core/server/saved_objects/serialization/serializer.ts @@ -86,6 +86,7 @@ export class SavedObjectsSerializer { ...(namespace && this.registry.isSingleNamespace(type) && { namespace }), ...(namespaces && this.registry.isMultiNamespace(type) && { namespaces }), ...(originId && { originId }), + ...(permissions && { permissions }), attributes: _source[type], references: _source.references || [], ...(_source.migrationVersion && { migrationVersion: _source.migrationVersion }), diff --git a/src/core/server/saved_objects/serialization/types.ts b/src/core/server/saved_objects/serialization/types.ts index fee9f503dceb..989dac7c6606 100644 --- a/src/core/server/saved_objects/serialization/types.ts +++ b/src/core/server/saved_objects/serialization/types.ts @@ -28,7 +28,7 @@ * under the License. */ -import { Permissions } from '../permission_control/acl'; +import { Permissions } from '../permission_control'; import { SavedObjectsMigrationVersion, SavedObjectReference } from '../types'; /** 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 c548fb144a95..5fc08f0a8c37 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, workspaces }, + { type, id, references, namespace: objectNamespace, originId, workspaces, permissions }, namespace ) => { const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; @@ -184,6 +184,7 @@ describe('SavedObjectsRepository', () => { ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), workspaces, ...(originId && { originId }), + ...(permissions && { permissions }), type, [type]: { title: 'Testing' }, references, @@ -629,25 +630,36 @@ describe('SavedObjectsRepository', () => { references: [{ name: 'ref_0', type: 'test', id: '2' }], }; const namespace = 'foo-namespace'; + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; const workspace = 'foo-workspace'; const getMockBulkCreateResponse = (objects, namespace) => { return { - items: objects.map(({ type, id, originId, attributes, references, migrationVersion }) => ({ - create: { - _id: `${namespace ? `${namespace}:` : ''}${type}:${id}`, - _source: { - [type]: attributes, - type, - namespace, - ...(originId && { originId }), - references, - ...mockTimestampFields, - migrationVersion: migrationVersion || { [type]: '1.1.1' }, + items: objects.map( + ({ type, id, originId, attributes, references, migrationVersion, permissions }) => ({ + create: { + _id: `${namespace ? `${namespace}:` : ''}${type}:${id}`, + _source: { + [type]: attributes, + type, + namespace, + ...(originId && { originId }), + ...(permissions && { permissions }), + references, + ...mockTimestampFields, + migrationVersion: migrationVersion || { [type]: '1.1.1' }, + }, + ...mockVersionProps, }, - ...mockVersionProps, - }, - })), + }) + ), }; }; @@ -918,6 +930,18 @@ describe('SavedObjectsRepository', () => { expectClientCallArgsAction(objects, { method: 'create', getId }); }); + it(`accepts permissions property when providing permissions info`, async () => { + const objects = [obj1, obj2].map((obj) => ({ ...obj, permissions: permissions })); + await bulkCreateSuccess(objects); + const expected = expect.objectContaining({ permissions }); + const body = [expect.any(Object), expected, expect.any(Object), expected]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + client.bulk.mockClear(); + }); + it(`adds workspaces to request body for any types`, async () => { await bulkCreateSuccess([obj1, obj2], { workspaces: [workspace] }); const expected = expect.objectContaining({ workspaces: [workspace] }); @@ -1199,6 +1223,17 @@ describe('SavedObjectsRepository', () => { ); expect(result.saved_objects[1].id).toEqual(obj2.id); }); + + it(`includes permissions property if present`, async () => { + const objects = [obj1, obj2].map((obj) => ({ ...obj, permissions: permissions })); + const result = await bulkCreateSuccess(objects); + expect(result).toEqual({ + saved_objects: [ + expect.objectContaining({ permissions }), + expect.objectContaining({ permissions }), + ], + }); + }); }); }); @@ -1418,6 +1453,22 @@ describe('SavedObjectsRepository', () => { ], }); }); + + it(`includes permissions property if present`, async () => { + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; + const obj = { id: 'three', type: MULTI_NAMESPACE_TYPE, permissions: permissions }; + const result = await bulkGetSuccess([obj]); + expect(result).toEqual({ + saved_objects: [expect.objectContaining({ permissions: permissions })], + }); + }); }); }); @@ -1435,6 +1486,14 @@ describe('SavedObjectsRepository', () => { const references = [{ name: 'ref_0', type: 'test', id: '1' }]; const originId = 'some-origin-id'; const namespace = 'foo-namespace'; + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; const getMockBulkUpdateResponse = (objects, options, includeOriginId) => ({ items: objects.map(({ type, id }) => ({ @@ -1695,6 +1754,20 @@ describe('SavedObjectsRepository', () => { await bulkUpdateSuccess([{ ..._obj2, namespace }]); expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }, 2); }); + + it(`accepts permissions property when providing permissions info`, async () => { + const objects = [obj1, obj2].map((obj) => ({ ...obj, permissions: permissions })); + await bulkUpdateSuccess(objects); + const doc = { + doc: expect.objectContaining({ permissions }), + }; + const body = [expect.any(Object), doc, expect.any(Object), doc]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + client.bulk.mockClear(); + }); }); describe('errors', () => { @@ -1887,6 +1960,14 @@ describe('SavedObjectsRepository', () => { ], }); }); + + it(`includes permissions property if present`, async () => { + const obj = { type: MULTI_NAMESPACE_TYPE, id: 'three', permissions: permissions }; + const result = await bulkUpdateSuccess([obj1, obj], {}, true); + expect(result).toEqual({ + saved_objects: [expect.anything(), expect.objectContaining({ permissions })], + }); + }); }); }); @@ -2042,6 +2123,14 @@ describe('SavedObjectsRepository', () => { id: '123', }, ]; + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); @@ -2239,6 +2328,16 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); + + it(`accepts permissions property`, async () => { + await createSuccess(type, attributes, { id, permissions }); + expect(client.create).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ permissions }), + }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -2319,6 +2418,11 @@ describe('SavedObjectsRepository', () => { expect(serializer.savedObjectToRaw).toHaveBeenLastCalledWith(migratedDoc); }); + it(`adds permissions to body when providing permissions info`, async () => { + await createSuccess(type, attributes, { id, permissions }); + expectMigrationArgs({ permissions }); + }); + it(`adds namespace to body when providing namespace for single-namespace type`, async () => { await createSuccess(type, attributes, { id, namespace }); expectMigrationArgs({ namespace }); @@ -2365,11 +2469,13 @@ describe('SavedObjectsRepository', () => { namespace, references, originId, + permissions, }); expect(result).toEqual({ type, id, originId, + permissions, ...mockTimestampFields, version: mockVersion, attributes, @@ -3576,7 +3682,7 @@ describe('SavedObjectsRepository', () => { const namespace = 'foo-namespace'; const originId = 'some-origin-id'; - const getSuccess = async (type, id, options, includeOriginId) => { + const getSuccess = async (type, id, options, includeOriginId, permissions) => { const response = getMockGetResponse( { type, @@ -3584,6 +3690,7 @@ describe('SavedObjectsRepository', () => { // "includeOriginId" is not an option for the operation; however, if the existing saved object contains an originId attribute, the // 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 }), }, options?.namespace ); @@ -3734,6 +3841,21 @@ describe('SavedObjectsRepository', () => { const result = await getSuccess(type, id, {}, true); expect(result).toMatchObject({ originId }); }); + + it(`includes permissions property if present`, async () => { + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; + const result = await getSuccess(type, id, { namespace }, undefined, permissions); + expect(result).toMatchObject({ + permissions: permissions, + }); + }); }); }); @@ -4335,6 +4457,14 @@ describe('SavedObjectsRepository', () => { }, ]; const originId = 'some-origin-id'; + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; const updateSuccess = async (type, id, attributes, options, includeOriginId) => { if (registry.isMultiNamespace(type)) { @@ -4511,6 +4641,18 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); + + it(`accepts permissions when providing permissions info`, async () => { + await updateSuccess(type, id, attributes, { permissions }); + const expected = expect.objectContaining({ permissions }); + const body = { + doc: expected, + }; + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -4605,6 +4747,11 @@ describe('SavedObjectsRepository', () => { const result = await updateSuccess(type, id, attributes, {}, true); expect(result).toMatchObject({ originId }); }); + + it(`includes permissions property if present`, async () => { + const result = await updateSuccess(type, id, attributes, { permissions }); + expect(result).toMatchObject({ permissions }); + }); }); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 314f96b2016c..b6c093e97bb6 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -479,6 +479,7 @@ export class SavedObjectsRepository { references: object.references || [], originId: object.originId, ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }), + ...(object.permissions && { permissions: object.permissions }), }) as SavedObjectSanitizedDoc ), }; @@ -1156,6 +1157,7 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(workspaces && { workspaces }), + ...(permissions && { permissions }), references, attributes, }; @@ -1565,7 +1567,7 @@ export class SavedObjectsRepository { }; } - const { attributes, references, version, namespace: objectNamespace } = object; + const { attributes, references, version, namespace: objectNamespace, permissions } = object; if (objectNamespace === ALL_NAMESPACES_STRING) { return { @@ -1586,6 +1588,7 @@ export class SavedObjectsRepository { [type]: attributes, updated_at: time, ...(Array.isArray(references) && { references }), + ...(permissions && { permissions }), }; const requiresNamespacesCheck = this._registry.isMultiNamespace(object.type); @@ -1738,7 +1741,7 @@ export class SavedObjectsRepository { )[0] as any; // eslint-disable-next-line @typescript-eslint/naming-convention - const { [type]: attributes, references, updated_at } = documentToSave; + const { [type]: attributes, references, updated_at, permissions } = documentToSave; if (error) { return { id, @@ -1758,6 +1761,7 @@ export class SavedObjectsRepository { version: encodeVersion(seqNo, primaryTerm), attributes, references, + ...(permissions && { permissions }), }; }), }; @@ -2070,7 +2074,7 @@ function getSavedObjectFromSource( attributes: doc._source[type], references: doc._source.references || [], migrationVersion: doc._source.migrationVersion, - permissions, + ...(permissions && { permissions }), }; } diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index c9990977bb48..eec49b13ccad 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -28,7 +28,6 @@ * under the License. */ -import { Permissions } from '../permission_control/acl'; import { ISavedObjectsRepository } from './lib'; import { SavedObject, @@ -40,6 +39,7 @@ import { SavedObjectsFindOptions, } from '../types'; import { SavedObjectsErrorHelpers } from './lib/errors'; +import { Permissions } from '../permission_control'; /** * @@ -69,12 +69,12 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; - /** permission control describe by ACL object */ - permissions?: Permissions; /** * workspaces the new created objects belong to */ workspaces?: string[]; + /** permission control describe by ACL object */ + permissions?: Permissions; } /** @@ -109,7 +109,7 @@ export interface SavedObjectsBulkCreateObject { * @public */ export interface SavedObjectsBulkUpdateObject - extends Pick { + extends Pick { /** The ID of this Saved Object, guaranteed to be unique for all objects of the same `type` */ id: string; /** The type of this Saved Object. Each plugin can define it's own custom Saved Object types. */ diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index c1c8ede27593..06d03f5f24c4 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -9,7 +9,6 @@ * GitHub history for details. */ -import { Permissions } from '../server/saved_objects/permission_control/acl'; /* * Licensed to Elasticsearch B.V. under one or more contributor * license agreements. See the NOTICE file distributed with @@ -28,6 +27,7 @@ import { Permissions } from '../server/saved_objects/permission_control/acl'; * specific language governing permissions and limitations * under the License. */ +import { Permissions } from '../server/saved_objects/permission_control/acl'; /** * Don't use this type, it's simply a helper type for {@link SavedObjectAttribute} @@ -116,6 +116,7 @@ export interface SavedObject { originId?: string; /** Workspace(s) that this saved object exists in. */ workspaces?: string[]; + /** Permissions that this saved objects exists in. */ permissions?: Permissions; }