From 8aec569f01293813e03d62286c3b2128255ab0df Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 00:04:34 +0800 Subject: [PATCH 1/2] Patch/acl (#231) * consume permissions in repository Signed-off-by: SuZhou-Joe * feat: consume permissions in serializer Signed-off-by: SuZhou-Joe * Add unit tests for consuming permissions in repository Signed-off-by: gaobinlong * feat: update Signed-off-by: SuZhou-Joe * fix: unit test Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe Signed-off-by: gaobinlong Co-authored-by: gaobinlong (cherry picked from commit 120d3b7cede38d4578b4ef259b031ddc4fcfc5e8) --- .../saved_objects/serialization/serializer.ts | 1 + .../service/lib/repository.test.js | 213 ++++++++++++++++-- .../saved_objects/service/lib/repository.ts | 10 +- .../service/saved_objects_client.ts | 3 +- src/core/types/saved_objects.ts | 3 +- 5 files changed, 209 insertions(+), 21 deletions(-) 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/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 816d1013e5be..e8183fca8643 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,24 +630,36 @@ describe('SavedObjectsRepository', () => { references: [{ name: 'ref_0', type: 'test', id: '2' }], }; const namespace = 'foo-namespace'; + const workspace = 'foo-workspace'; + const permissions = { + read: { + users: ['user1'], + }, + write: { + groups: ['groups1'], + }, + }; 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, - }, - })), + }) + ), }; }; @@ -922,6 +935,28 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects, { namespace }); expectClientCallArgsAction(objects, { method: 'create', getId }); }); + + it(`adds workspaces to request body for any types`, async () => { + await bulkCreateSuccess([obj1, obj2], { workspaces: [workspace] }); + const expected = expect.objectContaining({ workspaces: [workspace] }); + const body = [expect.any(Object), expected, expect.any(Object), expected]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + }); + + 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(); + }); }); describe('errors', () => { @@ -1204,6 +1239,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 }), + ], + }); + }); }); }); @@ -1423,6 +1469,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 })], + }); + }); }); }); @@ -1440,6 +1502,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 }) => ({ @@ -1700,6 +1770,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', () => { @@ -1892,6 +1976,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 })], + }); + }); }); }); @@ -2047,6 +2139,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); @@ -2252,6 +2352,39 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); + + it(`doesn't modify workspaces when overwrite without target workspaces`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + await savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: [], + }); + + expect(client.index).toHaveBeenCalledWith( + expect.objectContaining({ + id: `dashboard:${id}`, + body: expect.objectContaining({ + workspaces: ['foo'], + }), + }), + 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', () => { @@ -2332,6 +2465,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 }); @@ -2378,11 +2516,13 @@ describe('SavedObjectsRepository', () => { namespace, references, originId, + permissions, }); expect(result).toEqual({ type, id, originId, + permissions, ...mockTimestampFields, version: mockVersion, attributes, @@ -3589,7 +3729,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, @@ -3597,6 +3737,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 ); @@ -3747,6 +3888,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, + }); + }); }); }); @@ -4348,6 +4504,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)) { @@ -4524,6 +4688,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', () => { @@ -4618,6 +4794,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 a8fd54ef6cfe..8f98f6abc23f 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -542,6 +542,7 @@ export class SavedObjectsRepository { references: object.references || [], originId: object.originId, workspaces: savedObjectWorkspaces, + ...(object.permissions && { permissions: object.permissions }), }) as SavedObjectSanitizedDoc ), }; @@ -1228,6 +1229,7 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(workspaces && { workspaces }), + ...(permissions && { permissions }), references, attributes, }; @@ -1637,7 +1639,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 { @@ -1658,6 +1660,7 @@ export class SavedObjectsRepository { [type]: attributes, updated_at: time, ...(Array.isArray(references) && { references }), + ...(permissions && { permissions }), }; const requiresNamespacesCheck = this._registry.isMultiNamespace(object.type); @@ -1810,7 +1813,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, @@ -1830,6 +1833,7 @@ export class SavedObjectsRepository { version: encodeVersion(seqNo, primaryTerm), attributes, references, + ...(permissions && { permissions }), }; }), }; @@ -2142,7 +2146,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 4f430d7f7134..1008834d52ea 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -40,6 +40,7 @@ import { SavedObjectsFindOptions, } from '../types'; import { SavedObjectsErrorHelpers } from './lib/errors'; +import { Permissions } from '../permission_control/acl'; /** * @@ -101,7 +102,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 fccce5f72947..57834a66150d 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} @@ -115,6 +115,7 @@ export interface SavedObject { */ originId?: string; workspaces?: string[]; + /** ACL description of this saved object */ permissions?: Permissions; } From 75da7483076d3377191dbd96be7d27b554ab159d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 17:11:44 +0800 Subject: [PATCH 2/2] fix: bootstrap error Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/saved_objects_client.ts | 1 - 1 file changed, 1 deletion(-) 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 1008834d52ea..0be48bd6156f 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,