diff --git a/CHANGELOG.md b/CHANGELOG.md index 57abfb478f0d..a2c5c0a7b165 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 8341d96dbc97..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,15 +103,6 @@ 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: { @@ -121,12 +117,6 @@ describe('IndexMigrator', () => { groups: { type: 'keyword' }, }, }, - management: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, read: { properties: { users: { type: 'keyword' }, @@ -141,6 +131,14 @@ describe('IndexMigrator', () => { }, }, }, + references: { + type: 'nested', + properties: { + name: { type: 'keyword' }, + type: { type: 'keyword' }, + id: { type: 'keyword' }, + }, + }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -149,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 } }); @@ -169,10 +176,10 @@ describe('IndexMigrator', () => { namespace: '2f4316de49999235636386fe51dc06c1', namespaces: '2f4316de49999235636386fe51dc06c1', originId: '2f4316de49999235636386fe51dc06c1', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -183,40 +190,56 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - permissions: { + references: { + type: 'nested', properties: { - library_read: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - library_write: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - management: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - read: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - write: { - properties: { - users: { type: 'keyword' }, - groups: { 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: { @@ -321,7 +344,6 @@ describe('IndexMigrator', () => { namespace: '2f4316de49999235636386fe51dc06c1', namespaces: '2f4316de49999235636386fe51dc06c1', originId: '2f4316de49999235636386fe51dc06c1', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', @@ -336,40 +358,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - permissions: { - properties: { - library_read: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - library_write: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - management: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - read: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - write: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - }, - }, references: { type: 'nested', properties: { @@ -417,7 +405,6 @@ describe('IndexMigrator', () => { namespace: '2f4316de49999235636386fe51dc06c1', namespaces: '2f4316de49999235636386fe51dc06c1', originId: '2f4316de49999235636386fe51dc06c1', - permissions: '07c04cdd060494956fdddaa7ef86e8ac', references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', @@ -432,40 +419,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - permissions: { - properties: { - library_read: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - library_write: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - management: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - read: { - properties: { - users: { type: 'keyword' }, - groups: { type: 'keyword' }, - }, - }, - write: { - properties: { - users: { type: 'keyword' }, - groups: { 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 057d294c3637..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,7 +5,7 @@ import { Principals, Permissions, ACL } from './acl'; -describe('SavedObjectTypeRegistry', () => { +describe('acl', () => { it('test has permission', () => { const principals: Principals = { users: ['user1'], @@ -21,90 +21,143 @@ describe('SavedObjectTypeRegistry', () => { 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', () => { const acl = new ACL(); - const result1 = 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, }; - const acl1 = new ACL(permissions1); - const result1 = acl1 + 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, }; - - const acl2 = new ACL(permissions2); - const result2 = acl2 + 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, }; - const 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'] }]) @@ -115,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: { @@ -162,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/types.ts b/src/core/server/saved_objects/serialization/types.ts index f4c6569de4fa..f882596ce529 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.ts b/src/core/server/saved_objects/service/lib/repository.ts index aa96d8615b5e..ad79282fee54 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -292,8 +292,8 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), - ...(permissions && { permissions }), ...(Array.isArray(workspaces) && { workspaces }), + ...(permissions && { permissions }), }); const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); @@ -476,8 +476,8 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, - ...(object.permissions && { permissions: object.permissions }), ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }), + ...(object.permissions && { permissions: object.permissions }), }) as SavedObjectSanitizedDoc ), }; 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 e7ef871e3004..e1c3d16a9258 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -39,7 +39,7 @@ import { SavedObjectsFindOptions, } from '../types'; import { SavedObjectsErrorHelpers } from './lib/errors'; -import { Permissions } from '../permission_control/acl'; +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; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 9c14aeca98bb..b37309338c9e 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -116,7 +116,7 @@ export interface SavedObject { originId?: string; /** Workspaces that this saved object exists in. */ workspaces?: string[]; - /** ACL description of this saved object */ + /** Permissions that this saved objects exists in. */ permissions?: Permissions; } diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 8eeb010b4a21..0c6e55101b7f 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -34,6 +34,11 @@ describe('workspace service', () => { enabled: false, }, }, + savedObjects: { + permission: { + enabled: true, + }, + }, migrations: { skip: false }, }, }, diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 75b19bb225b0..ec5259608c72 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -35,6 +35,11 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', workspace: { enabled: true, }, + savedObjects: { + permission: { + enabled: true, + }, + }, migrations: { skip: false, }, 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, },