From 52d044c74a0db461dfa25edc51b128aa7bcb8e30 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 10 Sep 2020 22:09:24 -0400 Subject: [PATCH] Change saved object bulkUpdate to work across multiple namespaces (#75478) --- .../core/server/kibana-plugin-core-server.md | 1 + ...ore-server.savedobjectsbulkupdateobject.md | 1 + ....savedobjectsbulkupdateobject.namespace.md | 15 ++ ...na-plugin-core-server.savedobjectsutils.md | 20 ++ ...r.savedobjectsutils.namespaceidtostring.md | 13 + ...r.savedobjectsutils.namespacestringtoid.md | 13 + src/core/server/index.ts | 1 + .../saved_objects/routes/bulk_update.ts | 1 + .../server/saved_objects/service/index.ts | 1 + .../server/saved_objects/service/lib/index.ts | 2 + .../service/lib/repository.test.js | 228 +++++++++++++----- .../saved_objects/service/lib/repository.ts | 100 +++++--- .../service/lib/search_dsl/query_params.ts | 11 +- .../saved_objects/service/lib/utils.test.ts | 57 +++++ .../server/saved_objects/service/lib/utils.ts | 53 ++++ .../service/saved_objects_client.ts | 7 + src/core/server/server.api.md | 7 + ...ypted_saved_objects_client_wrapper.test.ts | 69 +++++- .../encrypted_saved_objects_client_wrapper.ts | 4 +- .../saved_objects/get_descriptor_namespace.ts | 11 +- .../check_saved_objects_privileges.test.ts | 31 ++- .../check_saved_objects_privileges.ts | 13 +- ...ecure_saved_objects_client_wrapper.test.ts | 21 +- .../secure_saved_objects_client_wrapper.ts | 20 +- .../lib/copy_to_spaces/copy_to_spaces.test.ts | 1 + .../resolve_copy_conflicts.test.ts | 1 + .../server/lib/utils/__mocks__/index.ts | 16 ++ .../spaces/server/lib/utils/namespace.test.ts | 46 ++-- .../spaces/server/lib/utils/namespace.ts | 36 ++- .../routes/api/external/copy_to_space.test.ts | 2 +- .../common/suites/bulk_update.ts | 14 +- .../security_and_spaces/apis/bulk_update.ts | 58 +++-- .../security_only/apis/bulk_update.ts | 33 ++- .../spaces_only/apis/bulk_update.ts | 50 ++-- 34 files changed, 731 insertions(+), 226 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.namespace.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespaceidtostring.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespacestringtoid.md create mode 100644 src/core/server/saved_objects/service/lib/utils.test.ts create mode 100644 src/core/server/saved_objects/service/lib/utils.ts create mode 100644 x-pack/plugins/spaces/server/lib/utils/__mocks__/index.ts diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index dfffdffb08a08..c16600d1d0492 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -28,6 +28,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md) | | | [SavedObjectsRepository](./kibana-plugin-core-server.savedobjectsrepository.md) | | | [SavedObjectsSerializer](./kibana-plugin-core-server.savedobjectsserializer.md) | A serializer that can be used to manually convert [raw](./kibana-plugin-core-server.savedobjectsrawdoc.md) or [sanitized](./kibana-plugin-core-server.savedobjectsanitizeddoc.md) documents to the other kind. | +| [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) | | | [SavedObjectTypeRegistry](./kibana-plugin-core-server.savedobjecttyperegistry.md) | Registry holding information about all the registered [saved object types](./kibana-plugin-core-server.savedobjectstype.md). | ## Enumerations diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.md index e079e0fa51aac..d71eda6009284 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.md @@ -17,5 +17,6 @@ export interface SavedObjectsBulkUpdateObject extends PickPartial<T> | The data for a Saved Object is stored as an object in the attributes property. | | [id](./kibana-plugin-core-server.savedobjectsbulkupdateobject.id.md) | string | The ID of this Saved Object, guaranteed to be unique for all objects of the same type | +| [namespace](./kibana-plugin-core-server.savedobjectsbulkupdateobject.namespace.md) | string | Optional namespace string to use when searching for this object. If this is defined, it will supersede the namespace ID that is in [SavedObjectsBulkUpdateOptions](./kibana-plugin-core-server.savedobjectsbulkupdateoptions.md).Note: the default namespace's string representation is 'default', and its ID representation is undefined. | | [type](./kibana-plugin-core-server.savedobjectsbulkupdateobject.type.md) | string | The type of this Saved Object. Each plugin can define it's own custom Saved Object types. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.namespace.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.namespace.md new file mode 100644 index 0000000000000..544efcd3be909 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkupdateobject.namespace.md @@ -0,0 +1,15 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsBulkUpdateObject](./kibana-plugin-core-server.savedobjectsbulkupdateobject.md) > [namespace](./kibana-plugin-core-server.savedobjectsbulkupdateobject.namespace.md) + +## SavedObjectsBulkUpdateObject.namespace property + +Optional namespace string to use when searching for this object. If this is defined, it will supersede the namespace ID that is in [SavedObjectsBulkUpdateOptions](./kibana-plugin-core-server.savedobjectsbulkupdateoptions.md). + +Note: the default namespace's string representation is `'default'`, and its ID representation is `undefined`. + +Signature: + +```typescript +namespace?: string; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md new file mode 100644 index 0000000000000..e365dfbcb5142 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md @@ -0,0 +1,20 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) + +## SavedObjectsUtils class + + +Signature: + +```typescript +export declare class SavedObjectsUtils +``` + +## Properties + +| Property | Modifiers | Type | Description | +| --- | --- | --- | --- | +| [namespaceIdToString](./kibana-plugin-core-server.savedobjectsutils.namespaceidtostring.md) | static | (namespace?: string | undefined) => string | Converts a given saved object namespace ID to its string representation. All namespace IDs have an identical string representation, with the exception of the undefined namespace ID (which has a namespace string of 'default'). | +| [namespaceStringToId](./kibana-plugin-core-server.savedobjectsutils.namespacestringtoid.md) | static | (namespace: string) => string | undefined | Converts a given saved object namespace string to its ID representation. All namespace strings have an identical ID representation, with the exception of the 'default' namespace string (which has a namespace ID of undefined). | + diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespaceidtostring.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespaceidtostring.md new file mode 100644 index 0000000000000..591505892e64f --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespaceidtostring.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) > [namespaceIdToString](./kibana-plugin-core-server.savedobjectsutils.namespaceidtostring.md) + +## SavedObjectsUtils.namespaceIdToString property + +Converts a given saved object namespace ID to its string representation. All namespace IDs have an identical string representation, with the exception of the `undefined` namespace ID (which has a namespace string of `'default'`). + +Signature: + +```typescript +static namespaceIdToString: (namespace?: string | undefined) => string; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespacestringtoid.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespacestringtoid.md new file mode 100644 index 0000000000000..e052fe493b5ea --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.namespacestringtoid.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) > [namespaceStringToId](./kibana-plugin-core-server.savedobjectsutils.namespacestringtoid.md) + +## SavedObjectsUtils.namespaceStringToId property + +Converts a given saved object namespace string to its ID representation. All namespace strings have an identical ID representation, with the exception of the `'default'` namespace string (which has a namespace ID of `undefined`). + +Signature: + +```typescript +static namespaceStringToId: (namespace: string) => string | undefined; +``` diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 97aca74bfd48f..d127471348d9f 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -293,6 +293,7 @@ export { SavedObjectsTypeManagementDefinition, SavedObjectMigrationMap, SavedObjectMigrationFn, + SavedObjectsUtils, exportSavedObjectsToStream, importSavedObjectsFromStream, resolveSavedObjectsImportErrors, diff --git a/src/core/server/saved_objects/routes/bulk_update.ts b/src/core/server/saved_objects/routes/bulk_update.ts index c112833b29f3f..882213644146a 100644 --- a/src/core/server/saved_objects/routes/bulk_update.ts +++ b/src/core/server/saved_objects/routes/bulk_update.ts @@ -40,6 +40,7 @@ export const registerBulkUpdateRoute = (router: IRouter) => { }) ) ), + namespace: schema.maybe(schema.string({ minLength: 1 })), }) ), }, diff --git a/src/core/server/saved_objects/service/index.ts b/src/core/server/saved_objects/service/index.ts index 271d4dd67d43e..c33a9f2f3b157 100644 --- a/src/core/server/saved_objects/service/index.ts +++ b/src/core/server/saved_objects/service/index.ts @@ -27,6 +27,7 @@ export { SavedObjectsErrorHelpers, SavedObjectsClientFactory, SavedObjectsClientFactoryProvider, + SavedObjectsUtils, } from './lib'; export * from './saved_objects_client'; diff --git a/src/core/server/saved_objects/service/lib/index.ts b/src/core/server/saved_objects/service/lib/index.ts index e103120388e35..eae8c5ef2e10c 100644 --- a/src/core/server/saved_objects/service/lib/index.ts +++ b/src/core/server/saved_objects/service/lib/index.ts @@ -30,3 +30,5 @@ export { } from './scoped_client_provider'; export { SavedObjectsErrorHelpers } from './errors'; + +export { SavedObjectsUtils } from './utils'; 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 f2e3b3e633cd6..7d30875b90796 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -155,27 +155,33 @@ describe('SavedObjectsRepository', () => { log: {}, }); - const getMockGetResponse = ({ type, id, references, namespace, originId }) => ({ - // NOTE: Elasticsearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these - found: true, - _id: `${registry.isSingleNamespace(type) && namespace ? `${namespace}:` : ''}${type}:${id}`, - ...mockVersionProps, - _source: { - ...(registry.isSingleNamespace(type) && { namespace }), - ...(registry.isMultiNamespace(type) && { namespaces: [namespace ?? 'default'] }), - ...(originId && { originId }), - type, - [type]: { title: 'Testing' }, - references, - specialProperty: 'specialValue', - ...mockTimestampFields, - }, - }); + const getMockGetResponse = ( + { type, id, references, namespace: objectNamespace, originId }, + namespace + ) => { + const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; + return { + // NOTE: Elasticsearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these + found: true, + _id: `${ + registry.isSingleNamespace(type) && namespaceId ? `${namespaceId}:` : '' + }${type}:${id}`, + ...mockVersionProps, + _source: { + ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), + ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), + ...(originId && { originId }), + type, + [type]: { title: 'Testing' }, + references, + specialProperty: 'specialValue', + ...mockTimestampFields, + }, + }; + }; const getMockMgetResponse = (objects, namespace) => ({ - docs: objects.map((obj) => - obj.found === false ? obj : getMockGetResponse({ ...obj, namespace }) - ), + docs: objects.map((obj) => (obj.found === false ? obj : getMockGetResponse(obj, namespace))), }); expect.extend({ @@ -586,6 +592,16 @@ describe('SavedObjectsRepository', () => { ); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + await bulkCreateSuccess([obj1, obj2], { namespace: 'default' }); + const expected = expect.not.objectContaining({ namespace: 'default' }); + const body = [expect.any(Object), expected, expect.any(Object), expected]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + }); + it(`doesn't add namespace to request body for any types that are not single-namespace`, async () => { const objects = [ { ...obj1, type: NAMESPACE_AGNOSTIC_TYPE }, @@ -653,19 +669,19 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; + const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkCreateSuccess([obj1, obj2], { namespace }); expectClientCallArgsAction([obj1, obj2], { method: 'create', getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkCreateSuccess([obj1, obj2]); expectClientCallArgsAction([obj1, obj2], { method: 'create', getId }); }); it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) const objects = [ { ...obj1, type: NAMESPACE_AGNOSTIC_TYPE }, { ...obj2, type: MULTI_NAMESPACE_TYPE }, @@ -972,19 +988,25 @@ describe('SavedObjectsRepository', () => { describe('client calls', () => { it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; + const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkGetSuccess([obj1, obj2], { namespace }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkGetSuccess([obj1, obj2]); _expectClientCallArgs([obj1, obj2], { getId }); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + await bulkGetSuccess([obj1, obj2], { namespace: 'default' }); + _expectClientCallArgs([obj1, obj2], { getId }); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) let objects = [obj1, obj2].map((obj) => ({ ...obj, type: NAMESPACE_AGNOSTIC_TYPE })); await bulkGetSuccess(objects, { namespace }); _expectClientCallArgs(objects, { getId }); @@ -1327,32 +1349,66 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; + const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkUpdateSuccess([obj1, obj2], { namespace }); expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); + + jest.clearAllMocks(); + // test again with object namespace string that supersedes the operation's namespace ID + await bulkUpdateSuccess([ + { ...obj1, namespace }, + { ...obj2, namespace }, + ]); + expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkUpdateSuccess([obj1, obj2]); expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); + + jest.clearAllMocks(); + // test again with object namespace string that supersedes the operation's namespace ID + await bulkUpdateSuccess( + [ + { ...obj1, namespace: 'default' }, + { ...obj2, namespace: 'default' }, + ], + { namespace } + ); + expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); }); - it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { + it(`normalizes options.namespace from 'default' to undefined`, async () => { const getId = (type, id) => `${type}:${id}`; - const objects1 = [{ ...obj1, type: NAMESPACE_AGNOSTIC_TYPE }]; - await bulkUpdateSuccess(objects1, { namespace }); - expectClientCallArgsAction(objects1, { method: 'update', getId }); - client.bulk.mockClear(); + await bulkUpdateSuccess([obj1, obj2], { namespace: 'default' }); + expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); + }); + + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) const overrides = { // bulkUpdate uses a preflight `get` request for multi-namespace saved objects, and specifies that version on `update` // we aren't testing for this here, but we need to include Jest assertions so this test doesn't fail if_primary_term: expect.any(Number), if_seq_no: expect.any(Number), }; - const objects2 = [{ ...obj2, type: MULTI_NAMESPACE_TYPE }]; - await bulkUpdateSuccess(objects2, { namespace }); - expectClientCallArgsAction(objects2, { method: 'update', getId, overrides }, 2); + const _obj1 = { ...obj1, type: NAMESPACE_AGNOSTIC_TYPE }; + const _obj2 = { ...obj2, type: MULTI_NAMESPACE_TYPE }; + + await bulkUpdateSuccess([_obj1], { namespace }); + expectClientCallArgsAction([_obj1], { method: 'update', getId }); + client.bulk.mockClear(); + await bulkUpdateSuccess([_obj2], { namespace }); + expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }, 2); + + jest.clearAllMocks(); + // test again with object namespace string that supersedes the operation's namespace ID + await bulkUpdateSuccess([{ ..._obj1, namespace }]); + expectClientCallArgsAction([_obj1], { method: 'update', getId }); + client.bulk.mockClear(); + await bulkUpdateSuccess([{ ..._obj2, namespace }]); + expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }, 2); }); }); @@ -1581,19 +1637,25 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; + const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await checkConflictsSuccess([obj1, obj2], { namespace }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await checkConflictsSuccess([obj1, obj2]); _expectClientCallArgs([obj1, obj2], { getId }); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + await checkConflictsSuccess([obj1, obj2], { namespace: 'default' }); + _expectClientCallArgs([obj1, obj2], { getId }); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) // obj3 is multi-namespace, and obj6 is namespace-agnostic await checkConflictsSuccess([obj3, obj6], { namespace }); _expectClientCallArgs([obj3, obj6], { getId }); @@ -1816,6 +1878,16 @@ describe('SavedObjectsRepository', () => { ); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + await createSuccess(type, attributes, { id, namespace: 'default' }); + expect(client.create).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + }), + expect.anything() + ); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { await createSuccess(NAMESPACE_AGNOSTIC_TYPE, attributes, { id, namespace }); expect(client.create).toHaveBeenCalledWith( @@ -1852,11 +1924,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when there is a conflict with an existing multi-namespace saved object (get)`, async () => { - const response = getMockGetResponse({ - type: MULTI_NAMESPACE_TYPE, - id, - namespace: 'bar-namespace', - }); + const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id }, 'bar-namespace'); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -1959,7 +2027,7 @@ describe('SavedObjectsRepository', () => { const deleteSuccess = async (type, id, options) => { if (registry.isMultiNamespace(type)) { - const mockGetResponse = getMockGetResponse({ type, id, namespace: options?.namespace }); + const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(mockGetResponse) ); @@ -2035,6 +2103,14 @@ describe('SavedObjectsRepository', () => { ); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + await deleteSuccess(type, id, { namespace: 'default' }); + expect(client.delete).toHaveBeenCalledWith( + expect.objectContaining({ id: `${type}:${id}` }), + expect.anything() + ); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { await deleteSuccess(NAMESPACE_AGNOSTIC_TYPE, id, { namespace }); expect(client.delete).toHaveBeenCalledWith( @@ -2085,7 +2161,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when the type is multi-namespace and the document exists, but not in this namespace`, async () => { - const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace }); + const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id }, namespace); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -2660,14 +2736,16 @@ describe('SavedObjectsRepository', () => { const originId = 'some-origin-id'; const getSuccess = async (type, id, options, includeOriginId) => { - const response = getMockGetResponse({ - type, - id, - namespace: options?.namespace, - // "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 }), - }); + const response = getMockGetResponse( + { + type, + id, + // "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 }), + }, + options?.namespace + ); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -2702,6 +2780,16 @@ describe('SavedObjectsRepository', () => { ); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + await getSuccess(type, id, { namespace: 'default' }); + expect(client.get).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + }), + expect.anything() + ); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { await getSuccess(NAMESPACE_AGNOSTIC_TYPE, id, { namespace }); expect(client.get).toHaveBeenCalledWith( @@ -2756,7 +2844,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when type is multi-namespace and the document exists, but not in this namespace`, async () => { - const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace }); + const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id }, namespace); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -2812,7 +2900,7 @@ describe('SavedObjectsRepository', () => { const incrementCounterSuccess = async (type, id, field, options) => { const isMultiNamespace = registry.isMultiNamespace(type); if (isMultiNamespace) { - const response = getMockGetResponse({ type, id, namespace: options?.namespace }); + const response = getMockGetResponse({ type, id }, options?.namespace); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -2883,6 +2971,16 @@ describe('SavedObjectsRepository', () => { ); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + await incrementCounterSuccess(type, id, field, { namespace: 'default' }); + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: `${type}:${id}`, + }), + expect.anything() + ); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { await incrementCounterSuccess(NAMESPACE_AGNOSTIC_TYPE, id, field, { namespace }); expect(client.update).toHaveBeenCalledWith( @@ -2949,11 +3047,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when there is a conflict with an existing multi-namespace saved object (get)`, async () => { - const response = getMockGetResponse({ - type: MULTI_NAMESPACE_TYPE, - id, - namespace: 'bar-namespace', - }); + const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id }, 'bar-namespace'); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -3246,7 +3340,7 @@ describe('SavedObjectsRepository', () => { expect(client.update).not.toHaveBeenCalled(); }); - it(`throws when type is not namespace-agnostic`, async () => { + it(`throws when type is not multi-namespace`, async () => { const test = async (type) => { const message = `${type} doesn't support multiple namespaces`; await expectBadRequestError(type, id, [namespace1, namespace2], message); @@ -3388,7 +3482,7 @@ describe('SavedObjectsRepository', () => { const updateSuccess = async (type, id, attributes, options, includeOriginId) => { if (registry.isMultiNamespace(type)) { - const mockGetResponse = getMockGetResponse({ type, id, namespace: options?.namespace }); + const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(mockGetResponse) ); @@ -3519,6 +3613,14 @@ describe('SavedObjectsRepository', () => { ); }); + it(`normalizes options.namespace from 'default' to undefined`, async () => { + await updateSuccess(type, id, attributes, { references, namespace: 'default' }); + expect(client.update).toHaveBeenCalledWith( + expect.objectContaining({ id: expect.stringMatching(`${type}:${id}`) }), + expect.anything() + ); + }); + it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { await updateSuccess(NAMESPACE_AGNOSTIC_TYPE, id, attributes, { namespace }); expect(client.update).toHaveBeenCalledWith( @@ -3589,7 +3691,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when type is multi-namespace and the document exists, but not in this namespace`, async () => { - const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace }); + const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id }, namespace); client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index e3fb7d2306469..125f97e7feb11 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -67,6 +67,7 @@ import { } from '../../types'; import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { validateConvertFilterToKueryNode } from './filter_utils'; +import { SavedObjectsUtils } from './utils'; // BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository // so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient. @@ -220,13 +221,13 @@ export class SavedObjectsRepository { const { id, migrationVersion, - namespace, overwrite = false, references = [], refresh = DEFAULT_REFRESH_SETTING, originId, version, } = options; + const namespace = normalizeNamespace(options.namespace); if (!this._allowedTypes.includes(type)) { throw SavedObjectsErrorHelpers.createUnsupportedTypeError(type); @@ -293,7 +294,8 @@ export class SavedObjectsRepository { objects: Array>, options: SavedObjectsCreateOptions = {} ): Promise> { - const { namespace, overwrite = false, refresh = DEFAULT_REFRESH_SETTING } = options; + const { overwrite = false, refresh = DEFAULT_REFRESH_SETTING } = options; + const namespace = normalizeNamespace(options.namespace); const time = this._getCurrentTime(); let bulkGetRequestIndexCounter = 0; @@ -468,7 +470,7 @@ export class SavedObjectsRepository { return { errors: [] }; } - const { namespace } = options; + const namespace = normalizeNamespace(options.namespace); let bulkGetRequestIndexCounter = 0; const expectedBulkGetResults: Either[] = objects.map((object) => { @@ -551,7 +553,8 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { namespace, refresh = DEFAULT_REFRESH_SETTING } = options; + const { refresh = DEFAULT_REFRESH_SETTING } = options; + const namespace = normalizeNamespace(options.namespace); const rawId = this._serializer.generateRawId(namespace, type, id); let preflightResult: SavedObjectsRawDoc | undefined; @@ -560,7 +563,7 @@ export class SavedObjectsRepository { preflightResult = await this.preflightCheckIncludesNamespace(type, id, namespace); const existingNamespaces = getSavedObjectNamespaces(undefined, preflightResult); const remainingNamespaces = existingNamespaces?.filter( - (x) => x !== getNamespaceString(namespace) + (x) => x !== SavedObjectsUtils.namespaceIdToString(namespace) ); if (remainingNamespaces?.length) { @@ -658,7 +661,7 @@ export class SavedObjectsRepository { } `, lang: 'painless', - params: { namespace: getNamespaceString(namespace) }, + params: { namespace }, }, conflicts: 'proceed', ...getSearchDsl(this._mappings, this._registry, { @@ -814,7 +817,7 @@ export class SavedObjectsRepository { objects: SavedObjectsBulkGetObject[] = [], options: SavedObjectsBaseOptions = {} ): Promise> { - const { namespace } = options; + const namespace = normalizeNamespace(options.namespace); if (objects.length === 0) { return { saved_objects: [] }; @@ -884,7 +887,9 @@ export class SavedObjectsRepository { const { originId, updated_at: updatedAt } = doc._source; let namespaces = []; if (!this._registry.isNamespaceAgnostic(type)) { - namespaces = doc._source.namespaces ?? [getNamespaceString(doc._source.namespace)]; + namespaces = doc._source.namespaces ?? [ + SavedObjectsUtils.namespaceIdToString(doc._source.namespace), + ]; } return { @@ -920,7 +925,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { namespace } = options; + const namespace = normalizeNamespace(options.namespace); const { body, statusCode } = await this.client.get>( { @@ -941,7 +946,9 @@ export class SavedObjectsRepository { let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { - namespaces = body._source.namespaces ?? [getNamespaceString(body._source.namespace)]; + namespaces = body._source.namespaces ?? [ + SavedObjectsUtils.namespaceIdToString(body._source.namespace), + ]; } return { @@ -978,7 +985,8 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { version, namespace, references, refresh = DEFAULT_REFRESH_SETTING } = options; + const { version, references, refresh = DEFAULT_REFRESH_SETTING } = options; + const namespace = normalizeNamespace(options.namespace); let preflightResult: SavedObjectsRawDoc | undefined; if (this._registry.isMultiNamespace(type)) { @@ -1016,7 +1024,9 @@ export class SavedObjectsRepository { const { originId } = body.get._source; let namespaces = []; if (!this._registry.isNamespaceAgnostic(type)) { - namespaces = body.get._source.namespaces ?? [getNamespaceString(body.get._source.namespace)]; + namespaces = body.get._source.namespaces ?? [ + SavedObjectsUtils.namespaceIdToString(body.get._source.namespace), + ]; } return { @@ -1060,6 +1070,7 @@ export class SavedObjectsRepository { } const { version, namespace, refresh = DEFAULT_REFRESH_SETTING } = options; + // we do not need to normalize the namespace to its ID format, since it will be converted to a namespace string before being used const rawId = this._serializer.generateRawId(undefined, type, id); const preflightResult = await this.preflightCheckIncludesNamespace(type, id, namespace); @@ -1122,6 +1133,7 @@ export class SavedObjectsRepository { } const { namespace, refresh = DEFAULT_REFRESH_SETTING } = options; + // we do not need to normalize the namespace to its ID format, since it will be converted to a namespace string before being used const rawId = this._serializer.generateRawId(undefined, type, id); const preflightResult = await this.preflightCheckIncludesNamespace(type, id, namespace); @@ -1208,7 +1220,7 @@ export class SavedObjectsRepository { options: SavedObjectsBulkUpdateOptions = {} ): Promise> { const time = this._getCurrentTime(); - const { namespace } = options; + const namespace = normalizeNamespace(options.namespace); let bulkGetRequestIndexCounter = 0; const expectedBulkGetResults: Either[] = objects.map((object) => { @@ -1225,7 +1237,9 @@ export class SavedObjectsRepository { }; } - const { attributes, references, version } = object; + const { attributes, references, version, namespace: objectNamespace } = object; + // `objectNamespace` is a namespace string, while `namespace` is a namespace ID. + // The object namespace string, if defined, will supersede the operation's namespace ID. const documentToSave = { [type]: attributes, @@ -1242,16 +1256,24 @@ export class SavedObjectsRepository { id, version, documentToSave, + objectNamespace, ...(requiresNamespacesCheck && { esRequestIndex: bulkGetRequestIndexCounter++ }), }, }; }); + const getNamespaceId = (objectNamespace?: string) => + objectNamespace !== undefined + ? SavedObjectsUtils.namespaceStringToId(objectNamespace) + : namespace; + const getNamespaceString = (objectNamespace?: string) => + objectNamespace ?? SavedObjectsUtils.namespaceIdToString(namespace); + const bulkGetDocs = expectedBulkGetResults .filter(isRight) .filter(({ value }) => value.esRequestIndex !== undefined) - .map(({ value: { type, id } }) => ({ - _id: this._serializer.generateRawId(namespace, type, id), + .map(({ value: { type, id, objectNamespace } }) => ({ + _id: this._serializer.generateRawId(getNamespaceId(objectNamespace), type, id), _index: this.getIndexForType(type), _source: ['type', 'namespaces'], })); @@ -1276,14 +1298,25 @@ export class SavedObjectsRepository { return expectedBulkGetResult; } - const { esRequestIndex, id, type, version, documentToSave } = expectedBulkGetResult.value; + const { + esRequestIndex, + id, + type, + version, + documentToSave, + objectNamespace, + } = expectedBulkGetResult.value; + let namespaces; let versionProperties; if (esRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound ? bulkGetResponse?.body.docs[esRequestIndex] : undefined; const docFound = indexFound && actualResult.found === true; - if (!docFound || !this.rawDocExistsInNamespace(actualResult, namespace)) { + if ( + !docFound || + !this.rawDocExistsInNamespace(actualResult, getNamespaceId(objectNamespace)) + ) { return { tag: 'Left' as 'Left', error: { @@ -1294,12 +1327,13 @@ export class SavedObjectsRepository { }; } namespaces = actualResult._source.namespaces ?? [ - getNamespaceString(actualResult._source.namespace), + SavedObjectsUtils.namespaceIdToString(actualResult._source.namespace), ]; versionProperties = getExpectedVersionProperties(version, actualResult); } else { if (this._registry.isSingleNamespace(type)) { - namespaces = [getNamespaceString(namespace)]; + // if `objectNamespace` is undefined, fall back to `options.namespace` + namespaces = [getNamespaceString(objectNamespace)]; } versionProperties = getExpectedVersionProperties(version); } @@ -1315,7 +1349,7 @@ export class SavedObjectsRepository { bulkUpdateParams.push( { update: { - _id: this._serializer.generateRawId(namespace, type, id), + _id: this._serializer.generateRawId(getNamespaceId(objectNamespace), type, id), _index: this.getIndexForType(type), ...versionProperties, }, @@ -1401,7 +1435,8 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createUnsupportedTypeError(type); } - const { migrationVersion, namespace, refresh = DEFAULT_REFRESH_SETTING } = options; + const { migrationVersion, refresh = DEFAULT_REFRESH_SETTING } = options; + const namespace = normalizeNamespace(options.namespace); const time = this._getCurrentTime(); let savedObjectNamespace; @@ -1495,7 +1530,7 @@ export class SavedObjectsRepository { const savedObject = this._serializer.rawToSavedObject(raw); const { namespace, type } = savedObject; if (this._registry.isSingleNamespace(type)) { - savedObject.namespaces = [getNamespaceString(namespace)]; + savedObject.namespaces = [SavedObjectsUtils.namespaceIdToString(namespace)]; } return omit(savedObject, 'namespace') as SavedObject; } @@ -1518,7 +1553,7 @@ export class SavedObjectsRepository { } const namespaces = raw._source.namespaces; - return namespaces?.includes(getNamespaceString(namespace)) ?? false; + return namespaces?.includes(SavedObjectsUtils.namespaceIdToString(namespace)) ?? false; } /** @@ -1623,14 +1658,6 @@ function getExpectedVersionProperties(version?: string, document?: SavedObjectsR return {}; } -/** - * Returns the string representation of a namespace. - * The default namespace is undefined, and is represented by the string 'default'. - */ -function getNamespaceString(namespace?: string) { - return namespace ?? 'default'; -} - /** * Returns a string array of namespaces for a given saved object. If the saved object is undefined, the result is an array that contains the * current namespace. Value may be undefined if an existing saved object has no namespaces attribute; this should not happen in normal @@ -1646,9 +1673,16 @@ function getSavedObjectNamespaces( if (document) { return document._source?.namespaces; } - return [getNamespaceString(namespace)]; + return [SavedObjectsUtils.namespaceIdToString(namespace)]; } +/** + * Ensure that a namespace is always in its namespace ID representation. + * This allows `'default'` to be used interchangeably with `undefined`. + */ +const normalizeNamespace = (namespace?: string) => + namespace === undefined ? namespace : SavedObjectsUtils.namespaceStringToId(namespace); + /** * Extracts the contents of a decorated error to return the attributes for bulk operations. */ diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index ad1a08187dc32..3ff72a86c2f89 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -21,6 +21,7 @@ import { esKuery, KueryNode } from '../../../../../../plugins/data/server'; import { getRootPropertiesObjects, IndexMapping } from '../../../mappings'; import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; +import { DEFAULT_NAMESPACE_STRING } from '../utils'; /** * Gets the types based on the type. Uses mappings to support @@ -73,7 +74,7 @@ function getFieldsForTypes( */ function getClauseForType( registry: ISavedObjectTypeRegistry, - namespaces: string[] = ['default'], + namespaces: string[] = [DEFAULT_NAMESPACE_STRING], type: string ) { if (namespaces.length === 0) { @@ -88,11 +89,11 @@ function getClauseForType( }; } else if (registry.isSingleNamespace(type)) { const should: Array> = []; - const eligibleNamespaces = namespaces.filter((namespace) => namespace !== 'default'); + const eligibleNamespaces = namespaces.filter((x) => x !== DEFAULT_NAMESPACE_STRING); if (eligibleNamespaces.length > 0) { should.push({ terms: { namespace: eligibleNamespaces } }); } - if (namespaces.includes('default')) { + if (namespaces.includes(DEFAULT_NAMESPACE_STRING)) { should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); } if (should.length === 0) { @@ -162,9 +163,7 @@ export function getQueryParams({ // would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard. // We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716 const normalizedNamespaces = namespaces - ? Array.from( - new Set(namespaces.map((namespace) => (namespace === '*' ? 'default' : namespace))) - ) + ? Array.from(new Set(namespaces.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x)))) : undefined; const bool: any = { diff --git a/src/core/server/saved_objects/service/lib/utils.test.ts b/src/core/server/saved_objects/service/lib/utils.test.ts new file mode 100644 index 0000000000000..ea4fa68242bea --- /dev/null +++ b/src/core/server/saved_objects/service/lib/utils.test.ts @@ -0,0 +1,57 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { SavedObjectsUtils } from './utils'; + +describe('SavedObjectsUtils', () => { + const { namespaceIdToString, namespaceStringToId } = SavedObjectsUtils; + + describe('#namespaceIdToString', () => { + it('converts `undefined` to default namespace string', () => { + expect(namespaceIdToString(undefined)).toEqual('default'); + }); + + it('leaves other namespace IDs as-is', () => { + expect(namespaceIdToString('foo')).toEqual('foo'); + }); + + it('throws an error when a namespace ID is an empty string', () => { + expect(() => namespaceIdToString('')).toThrowError('namespace cannot be an empty string'); + }); + }); + + describe('#namespaceStringToId', () => { + it('converts default namespace string to `undefined`', () => { + expect(namespaceStringToId('default')).toBeUndefined(); + }); + + it('leaves other namespace strings as-is', () => { + expect(namespaceStringToId('foo')).toEqual('foo'); + }); + + it('throws an error when a namespace string is falsy', () => { + const test = (arg: any) => + expect(() => namespaceStringToId(arg)).toThrowError('namespace must be a non-empty string'); + + test(undefined); + test(null); + test(''); + }); + }); +}); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts new file mode 100644 index 0000000000000..6101ad57cc401 --- /dev/null +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -0,0 +1,53 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export const DEFAULT_NAMESPACE_STRING = 'default'; + +/** + * @public + */ +export class SavedObjectsUtils { + /** + * Converts a given saved object namespace ID to its string representation. All namespace IDs have an identical string representation, with + * the exception of the `undefined` namespace ID (which has a namespace string of `'default'`). + * + * @param namespace The namespace ID, which must be either a non-empty string or `undefined`. + */ + public static namespaceIdToString = (namespace?: string) => { + if (namespace === '') { + throw new TypeError('namespace cannot be an empty string'); + } + + return namespace ?? DEFAULT_NAMESPACE_STRING; + }; + + /** + * Converts a given saved object namespace string to its ID representation. All namespace strings have an identical ID representation, with + * the exception of the `'default'` namespace string (which has a namespace ID of `undefined`). + * + * @param namespace The namespace string, which must be non-empty. + */ + public static namespaceStringToId = (namespace: string) => { + if (!namespace) { + throw new TypeError('namespace must be a non-empty string'); + } + + return namespace !== DEFAULT_NAMESPACE_STRING ? namespace : undefined; + }; +} 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 347c760f841bc..8c96116de49cb 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -80,6 +80,13 @@ export interface SavedObjectsBulkUpdateObject type: string; /** {@inheritdoc SavedObjectAttributes} */ attributes: Partial; + /** + * Optional namespace string to use when searching for this object. If this is defined, it will supersede the namespace ID that is in + * {@link SavedObjectsBulkUpdateOptions}. + * + * Note: the default namespace's string representation is `'default'`, and its ID representation is `undefined`. + **/ + namespace?: string; } /** diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index aef1bda9ccf4e..ec457704e89c7 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2047,6 +2047,7 @@ export interface SavedObjectsBulkResponse { export interface SavedObjectsBulkUpdateObject extends Pick { attributes: Partial; id: string; + namespace?: string; type: string; } @@ -2630,6 +2631,12 @@ export interface SavedObjectsUpdateResponse extends Omit string; + static namespaceStringToId: (namespace: string) => string | undefined; +} + // @public export class SavedObjectTypeRegistry { getAllTypes(): SavedObjectsType[]; diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts index f8d66b8ecac27..18834f55af0a5 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts @@ -555,7 +555,18 @@ describe('#bulkUpdate', () => { }); describe('namespace', () => { - const doTest = async (namespace: string, expectNamespaceInDescriptor: boolean) => { + interface TestParams { + optionsNamespace: string | undefined; + objectNamespace: string | undefined; + expectOptionsNamespaceInDescriptor: boolean; + expectObjectNamespaceInDescriptor: boolean; + } + const doTest = async ({ + optionsNamespace, + objectNamespace, + expectOptionsNamespaceInDescriptor, + expectObjectNamespaceInDescriptor, + }: TestParams) => { const docs = [ { id: 'some-id', @@ -566,12 +577,13 @@ describe('#bulkUpdate', () => { attrThree: 'three', }, version: 'some-version', + namespace: objectNamespace, }, ]; - const options = { namespace }; + const options = { namespace: optionsNamespace }; mockBaseClient.bulkUpdate.mockResolvedValue({ - saved_objects: docs.map((doc) => ({ ...doc, references: undefined })), + saved_objects: docs.map(({ namespace, ...doc }) => ({ ...doc, references: undefined })), }); await expect(wrapper.bulkUpdate(docs, options)).resolves.toEqual({ @@ -594,7 +606,11 @@ describe('#bulkUpdate', () => { { type: 'known-type', id: 'some-id', - namespace: expectNamespaceInDescriptor ? namespace : undefined, + namespace: expectObjectNamespaceInDescriptor + ? objectNamespace + : expectOptionsNamespaceInDescriptor + ? optionsNamespace + : undefined, }, { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' }, { user: mockAuthenticatedUser() } @@ -612,7 +628,7 @@ describe('#bulkUpdate', () => { attrThree: 'three', }, version: 'some-version', - + namespace: objectNamespace, references: undefined, }, ], @@ -620,13 +636,46 @@ describe('#bulkUpdate', () => { ); }; - it('uses `namespace` to encrypt attributes if it is specified when type is single-namespace', async () => { - await doTest('some-namespace', true); + it('does not use options `namespace` or object `namespace` to encrypt attributes if neither are specified', async () => { + await doTest({ + optionsNamespace: undefined, + objectNamespace: undefined, + expectOptionsNamespaceInDescriptor: false, + expectObjectNamespaceInDescriptor: false, + }); }); - it('does not use `namespace` to encrypt attributes if it is specified when type is not single-namespace', async () => { - mockBaseTypeRegistry.isSingleNamespace.mockReturnValue(false); - await doTest('some-namespace', false); + describe('with a single-namespace type', () => { + it('uses options `namespace` to encrypt attributes if it is specified and object `namespace` is not', async () => { + await doTest({ + optionsNamespace: 'some-namespace', + objectNamespace: undefined, + expectOptionsNamespaceInDescriptor: true, + expectObjectNamespaceInDescriptor: false, + }); + }); + + it('uses object `namespace` to encrypt attributes if it is specified', async () => { + // object namespace supersedes options namespace + await doTest({ + optionsNamespace: 'some-namespace', + objectNamespace: 'another-namespace', + expectOptionsNamespaceInDescriptor: false, + expectObjectNamespaceInDescriptor: true, + }); + }); + }); + + describe('with a non-single-namespace type', () => { + it('does not use object `namespace` or options `namespace` to encrypt attributes if it is specified', async () => { + mockBaseTypeRegistry.isSingleNamespace.mockReturnValue(false); + await doTest({ + optionsNamespace: 'some-namespace', + objectNamespace: 'another-namespace', + expectOptionsNamespaceInDescriptor: false, + expectObjectNamespaceInDescriptor: false, + }); + }); }); }); diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts index a2725cbc6a274..0eeb9943b5be9 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts @@ -150,14 +150,14 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon // sequential processing. const encryptedObjects = await Promise.all( objects.map(async (object) => { - const { type, id, attributes } = object; + const { type, id, attributes, namespace: objectNamespace } = object; if (!this.options.service.isRegistered(type)) { return object; } const namespace = getDescriptorNamespace( this.options.baseTypeRegistry, type, - options?.namespace + objectNamespace ?? options?.namespace ); return { ...object, diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/get_descriptor_namespace.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/get_descriptor_namespace.ts index b2842df909a1d..7201f13fb930b 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/get_descriptor_namespace.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/get_descriptor_namespace.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ISavedObjectTypeRegistry } from 'kibana/server'; +import { ISavedObjectTypeRegistry, SavedObjectsUtils } from '../../../../../src/core/server'; export const getDescriptorNamespace = ( typeRegistry: ISavedObjectTypeRegistry, @@ -12,5 +12,12 @@ export const getDescriptorNamespace = ( namespace?: string ) => { const descriptorNamespace = typeRegistry.isSingleNamespace(type) ? namespace : undefined; - return descriptorNamespace === 'default' ? undefined : descriptorNamespace; + return normalizeNamespace(descriptorNamespace); }; + +/** + * Ensure that a namespace is always in its namespace ID representation. + * This allows `'default'` to be used interchangeably with `undefined`. + */ +const normalizeNamespace = (namespace?: string) => + namespace === undefined ? namespace : SavedObjectsUtils.namespaceStringToId(namespace); diff --git a/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.test.ts b/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.test.ts index 5e38045b88c74..b393c4a9e1a04 100644 --- a/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.test.ts +++ b/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.test.ts @@ -31,7 +31,9 @@ beforeEach(() => { mockSpacesService = { getSpaceId: jest.fn(), - namespaceToSpaceId: jest.fn().mockImplementation((namespace: string) => `${namespace}-id`), + namespaceToSpaceId: jest + .fn() + .mockImplementation((namespace: string = 'default') => `${namespace}-id`), }; }); @@ -41,8 +43,6 @@ describe('#checkSavedObjectsPrivileges', () => { const namespace2 = 'qux'; describe('when checking multiple namespaces', () => { - const namespaces = [namespace1, namespace2]; - test(`throws an error when using an empty namespaces array`, async () => { const checkSavedObjectsPrivileges = createFactory(); @@ -58,6 +58,7 @@ describe('#checkSavedObjectsPrivileges', () => { mockCheckPrivileges.atSpaces.mockReturnValue(expectedResult as any); const checkSavedObjectsPrivileges = createFactory(); + const namespaces = [namespace1, namespace2]; const result = await checkSavedObjectsPrivileges(actions, namespaces); expect(result).toBe(expectedResult); @@ -70,6 +71,30 @@ describe('#checkSavedObjectsPrivileges', () => { const spaceIds = mockSpacesService!.namespaceToSpaceId.mock.results.map((x) => x.value); expect(mockCheckPrivileges.atSpaces).toHaveBeenCalledWith(spaceIds, actions); }); + + test(`de-duplicates namespaces`, async () => { + const expectedResult = Symbol(); + mockCheckPrivileges.atSpaces.mockReturnValue(expectedResult as any); + const checkSavedObjectsPrivileges = createFactory(); + + const namespaces = [undefined, 'default', namespace1, namespace1]; + const result = await checkSavedObjectsPrivileges(actions, namespaces); + + expect(result).toBe(expectedResult); + expect(mockSpacesService!.namespaceToSpaceId).toHaveBeenCalledTimes(4); + expect(mockSpacesService!.namespaceToSpaceId).toHaveBeenNthCalledWith(1, undefined); + expect(mockSpacesService!.namespaceToSpaceId).toHaveBeenNthCalledWith(2, 'default'); + expect(mockSpacesService!.namespaceToSpaceId).toHaveBeenNthCalledWith(3, namespace1); + expect(mockSpacesService!.namespaceToSpaceId).toHaveBeenNthCalledWith(4, namespace1); + expect(mockCheckPrivilegesWithRequest).toHaveBeenCalledTimes(1); + expect(mockCheckPrivilegesWithRequest).toHaveBeenCalledWith(request); + expect(mockCheckPrivileges.atSpaces).toHaveBeenCalledTimes(1); + const spaceIds = [ + mockSpacesService!.namespaceToSpaceId(undefined), // deduplicated with 'default' + mockSpacesService!.namespaceToSpaceId(namespace1), // deduplicated with namespace1 + ]; + expect(mockCheckPrivileges.atSpaces).toHaveBeenCalledWith(spaceIds, actions); + }); }); describe('when checking a single namespace', () => { diff --git a/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.ts b/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.ts index 0c2260542bf72..6d2f724dae948 100644 --- a/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.ts +++ b/x-pack/plugins/security/server/authorization/check_saved_objects_privileges.ts @@ -14,9 +14,13 @@ export type CheckSavedObjectsPrivilegesWithRequest = ( export type CheckSavedObjectsPrivileges = ( actions: string | string[], - namespaceOrNamespaces?: string | string[] + namespaceOrNamespaces?: string | Array ) => Promise; +function uniq(arr: T[]): T[] { + return Array.from(new Set(arr)); +} + export const checkSavedObjectsPrivilegesWithRequestFactory = ( checkPrivilegesWithRequest: CheckPrivilegesWithRequest, getSpacesService: () => SpacesService | undefined @@ -26,7 +30,7 @@ export const checkSavedObjectsPrivilegesWithRequestFactory = ( ): CheckSavedObjectsPrivileges { return async function checkSavedObjectsPrivileges( actions: string | string[], - namespaceOrNamespaces?: string | string[] + namespaceOrNamespaces?: string | Array ) { const spacesService = getSpacesService(); if (!spacesService) { @@ -37,7 +41,10 @@ export const checkSavedObjectsPrivilegesWithRequestFactory = ( if (!namespaceOrNamespaces.length) { throw new Error(`Can't check saved object privileges for 0 namespaces`); } - const spaceIds = namespaceOrNamespaces.map((x) => spacesService.namespaceToSpaceId(x)); + const spaceIds = uniq( + namespaceOrNamespaces.map((x) => spacesService.namespaceToSpaceId(x)) + ); + return await checkPrivilegesWithRequest(request).atSpaces(spaceIds, actions); } else { // Spaces enabled, authorizing against a single space diff --git a/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.test.ts b/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.test.ts index 7f7f969e8b480..2cf072453a3c7 100644 --- a/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.test.ts +++ b/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.test.ts @@ -117,7 +117,11 @@ const expectSuccess = async (fn: Function, args: Record, action?: s return result; }; -const expectPrivilegeCheck = async (fn: Function, args: Record) => { +const expectPrivilegeCheck = async ( + fn: Function, + args: Record, + namespacesOverride?: Array +) => { clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockImplementation( getMockCheckPrivilegesFailure ); @@ -131,7 +135,7 @@ const expectPrivilegeCheck = async (fn: Function, args: Record) => expect(clientOpts.checkSavedObjectsPrivilegesAsCurrentUser).toHaveBeenCalledTimes(1); expect(clientOpts.checkSavedObjectsPrivilegesAsCurrentUser).toHaveBeenCalledWith( actions, - args.options?.namespace ?? args.options?.namespaces + namespacesOverride ?? args.options?.namespace ?? args.options?.namespaces ); }; @@ -483,7 +487,18 @@ describe('#bulkUpdate', () => { test(`checks privileges for user, actions, and namespace`, async () => { const objects = [obj1, obj2]; - await expectPrivilegeCheck(client.bulkUpdate, { objects, options }); + const namespacesOverride = [options.namespace]; // the bulkCreate function checks privileges as an array + await expectPrivilegeCheck(client.bulkUpdate, { objects, options }, namespacesOverride); + }); + + test(`checks privileges for object namespaces if present`, async () => { + const objects = [ + { ...obj1, namespace: 'foo-ns' }, + { ...obj2, namespace: 'bar-ns' }, + ]; + const namespacesOverride = [undefined, 'foo-ns', 'bar-ns']; + // use the default namespace for the options + await expectPrivilegeCheck(client.bulkUpdate, { objects, options: {} }, namespacesOverride); }); test(`filters namespaces that the user doesn't have access to`, async () => { diff --git a/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts b/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts index 68fe65d204d6d..bfa08a0116644 100644 --- a/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts +++ b/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts @@ -199,12 +199,16 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra objects: Array> = [], options: SavedObjectsBaseOptions = {} ) { - await this.ensureAuthorized( - this.getUniqueObjectTypes(objects), - 'bulk_update', - options && options.namespace, - { objects, options } - ); + const objectNamespaces = objects + // The repository treats an `undefined` object namespace is treated as the absence of a namespace, falling back to options.namespace; + // in this case, filter it out here so we don't accidentally check for privileges in the Default space when we shouldn't be doing so. + .filter(({ namespace }) => namespace !== undefined) + .map(({ namespace }) => namespace!); + const namespaces = [options?.namespace, ...objectNamespaces]; + await this.ensureAuthorized(this.getUniqueObjectTypes(objects), 'bulk_update', namespaces, { + objects, + options, + }); const response = await this.baseClient.bulkUpdate(objects, options); return await this.redactSavedObjectsNamespaces(response); @@ -212,7 +216,7 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra private async checkPrivileges( actions: string | string[], - namespaceOrNamespaces?: string | string[] + namespaceOrNamespaces?: string | Array ) { try { return await this.checkSavedObjectsPrivilegesAsCurrentUser(actions, namespaceOrNamespaces); @@ -224,7 +228,7 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra private async ensureAuthorized( typeOrTypes: string | string[], action: string, - namespaceOrNamespaces?: string | string[], + namespaceOrNamespaces?: string | Array, args?: Record, auditAction: string = action, requiresAll = true diff --git a/x-pack/plugins/spaces/server/lib/copy_to_spaces/copy_to_spaces.test.ts b/x-pack/plugins/spaces/server/lib/copy_to_spaces/copy_to_spaces.test.ts index d49dfa2015dc6..1cec7b769fa26 100644 --- a/x-pack/plugins/spaces/server/lib/copy_to_spaces/copy_to_spaces.test.ts +++ b/x-pack/plugins/spaces/server/lib/copy_to_spaces/copy_to_spaces.test.ts @@ -20,6 +20,7 @@ import { copySavedObjectsToSpacesFactory } from './copy_to_spaces'; jest.mock('../../../../../../src/core/server', () => { return { + ...(jest.requireActual('../../../../../../src/core/server') as Record), exportSavedObjectsToStream: jest.fn(), importSavedObjectsFromStream: jest.fn(), }; diff --git a/x-pack/plugins/spaces/server/lib/copy_to_spaces/resolve_copy_conflicts.test.ts b/x-pack/plugins/spaces/server/lib/copy_to_spaces/resolve_copy_conflicts.test.ts index 6a77bf7397cb5..37181c9d81649 100644 --- a/x-pack/plugins/spaces/server/lib/copy_to_spaces/resolve_copy_conflicts.test.ts +++ b/x-pack/plugins/spaces/server/lib/copy_to_spaces/resolve_copy_conflicts.test.ts @@ -20,6 +20,7 @@ import { resolveCopySavedObjectsToSpacesConflictsFactory } from './resolve_copy_ jest.mock('../../../../../../src/core/server', () => { return { + ...(jest.requireActual('../../../../../../src/core/server') as Record), exportSavedObjectsToStream: jest.fn(), resolveSavedObjectsImportErrors: jest.fn(), }; diff --git a/x-pack/plugins/spaces/server/lib/utils/__mocks__/index.ts b/x-pack/plugins/spaces/server/lib/utils/__mocks__/index.ts new file mode 100644 index 0000000000000..2b93e6d87a7af --- /dev/null +++ b/x-pack/plugins/spaces/server/lib/utils/__mocks__/index.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +const mockNamespaceIdToString = jest.fn(); +const mockNamespaceStringToId = jest.fn(); +jest.mock('../../../../../../../src/core/server', () => ({ + SavedObjectsUtils: { + namespaceIdToString: mockNamespaceIdToString, + namespaceStringToId: mockNamespaceStringToId, + }, +})); + +export { mockNamespaceIdToString, mockNamespaceStringToId }; diff --git a/x-pack/plugins/spaces/server/lib/utils/namespace.test.ts b/x-pack/plugins/spaces/server/lib/utils/namespace.test.ts index a81a5f3cee187..79d3dda301045 100644 --- a/x-pack/plugins/spaces/server/lib/utils/namespace.test.ts +++ b/x-pack/plugins/spaces/server/lib/utils/namespace.test.ts @@ -4,45 +4,29 @@ * you may not use this file except in compliance with the Elastic License. */ -import { DEFAULT_SPACE_ID } from '../../../common/constants'; +import { mockNamespaceIdToString, mockNamespaceStringToId } from './__mocks__'; import { spaceIdToNamespace, namespaceToSpaceId } from './namespace'; -describe('#spaceIdToNamespace', () => { - it('converts the default space to undefined', () => { - expect(spaceIdToNamespace(DEFAULT_SPACE_ID)).toBeUndefined(); - }); - - it('returns non-default spaces as-is', () => { - expect(spaceIdToNamespace('foo')).toEqual('foo'); - }); - - it('throws an error when a spaceId is not provided', () => { - // @ts-ignore ts knows this isn't right - expect(() => spaceIdToNamespace()).toThrowErrorMatchingInlineSnapshot(`"spaceId is required"`); +beforeEach(() => { + jest.clearAllMocks(); +}); - // @ts-ignore ts knows this isn't right - expect(() => spaceIdToNamespace(null)).toThrowErrorMatchingInlineSnapshot( - `"spaceId is required"` - ); +describe('#spaceIdToNamespace', () => { + it('returns result of namespaceStringToId', () => { + mockNamespaceStringToId.mockReturnValue('bar'); - expect(() => spaceIdToNamespace('')).toThrowErrorMatchingInlineSnapshot( - `"spaceId is required"` - ); + const result = spaceIdToNamespace('foo'); + expect(mockNamespaceStringToId).toHaveBeenCalledWith('foo'); + expect(result).toEqual('bar'); }); }); describe('#namespaceToSpaceId', () => { - it('returns the default space id for undefined namespaces', () => { - expect(namespaceToSpaceId(undefined)).toEqual(DEFAULT_SPACE_ID); - }); - - it('returns all other namespaces as-is', () => { - expect(namespaceToSpaceId('foo')).toEqual('foo'); - }); + it('returns result of namespaceIdToString', () => { + mockNamespaceIdToString.mockReturnValue('bar'); - it('throws an error when an empty string is provided', () => { - expect(() => namespaceToSpaceId('')).toThrowErrorMatchingInlineSnapshot( - `"namespace cannot be an empty string"` - ); + const result = namespaceToSpaceId('foo'); + expect(mockNamespaceIdToString).toHaveBeenCalledWith('foo'); + expect(result).toEqual('bar'); }); }); diff --git a/x-pack/plugins/spaces/server/lib/utils/namespace.ts b/x-pack/plugins/spaces/server/lib/utils/namespace.ts index 8c7ed2ea1797d..344da18846f3b 100644 --- a/x-pack/plugins/spaces/server/lib/utils/namespace.ts +++ b/x-pack/plugins/spaces/server/lib/utils/namespace.ts @@ -4,28 +4,22 @@ * you may not use this file except in compliance with the Elastic License. */ -import { DEFAULT_SPACE_ID } from '../../../common/constants'; +import { SavedObjectsUtils } from '../../../../../../src/core/server'; -export function spaceIdToNamespace(spaceId: string): string | undefined { - if (!spaceId) { - throw new TypeError('spaceId is required'); - } - - if (spaceId === DEFAULT_SPACE_ID) { - return undefined; - } - - return spaceId; +/** + * Converts a Space ID string to its namespace ID representation. Note that a Space ID string is equivalent to a namespace string. + * + * See also: {@link namespaceStringToId}. + */ +export function spaceIdToNamespace(spaceId: string) { + return SavedObjectsUtils.namespaceStringToId(spaceId); } -export function namespaceToSpaceId(namespace: string | undefined): string { - if (namespace === '') { - throw new TypeError('namespace cannot be an empty string'); - } - - if (!namespace) { - return DEFAULT_SPACE_ID; - } - - return namespace; +/** + * Converts a namespace ID to its Space ID string representation. Note that a Space ID string is equivalent to a namespace string. + * + * See also: {@link namespaceIdToString}. + */ +export function namespaceToSpaceId(namespace?: string) { + return SavedObjectsUtils.namespaceIdToString(namespace); } diff --git a/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts b/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts index bec3a5dcb0b71..dce6de908cfcb 100644 --- a/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts +++ b/x-pack/plugins/spaces/server/routes/api/external/copy_to_space.test.ts @@ -30,10 +30,10 @@ import { securityMock } from '../../../../../security/server/mocks'; import { ObjectType } from '@kbn/config-schema'; jest.mock('../../../../../../../src/core/server', () => { return { + ...(jest.requireActual('../../../../../../../src/core/server') as Record), exportSavedObjectsToStream: jest.fn(), importSavedObjectsFromStream: jest.fn(), resolveSavedObjectsImportErrors: jest.fn(), - kibanaResponseFactory: jest.requireActual('src/core/server').kibanaResponseFactory, }; }); import { diff --git a/x-pack/test/saved_object_api_integration/common/suites/bulk_update.ts b/x-pack/test/saved_object_api_integration/common/suites/bulk_update.ts index 0b5656004492a..2e3c55f029d29 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/bulk_update.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/bulk_update.ts @@ -8,12 +8,7 @@ import expect from '@kbn/expect'; import { SuperTest } from 'supertest'; import { SAVED_OBJECT_TEST_CASES as CASES } from '../lib/saved_object_test_cases'; import { SPACES } from '../lib/spaces'; -import { - createRequest, - expectResponses, - getUrlPrefix, - getTestTitle, -} from '../lib/saved_object_test_utils'; +import { expectResponses, getUrlPrefix, getTestTitle } from '../lib/saved_object_test_utils'; import { ExpectResponseBody, TestCase, TestDefinition, TestSuite } from '../lib/types'; export interface BulkUpdateTestDefinition extends TestDefinition { @@ -21,6 +16,7 @@ export interface BulkUpdateTestDefinition extends TestDefinition { } export type BulkUpdateTestSuite = TestSuite; export interface BulkUpdateTestCase extends TestCase { + namespace?: string; // used to define individual "object namespace" strings, e.g., bulkUpdate across multiple namespaces failure?: 404; // only used for permitted response case } @@ -30,6 +26,12 @@ const NEW_ATTRIBUTE_VAL = `Updated attribute value ${Date.now()}`; const DOES_NOT_EXIST = Object.freeze({ type: 'dashboard', id: 'does-not-exist' }); export const TEST_CASES = Object.freeze({ ...CASES, DOES_NOT_EXIST }); +const createRequest = ({ type, id, namespace }: BulkUpdateTestCase) => ({ + type, + id, + ...(namespace && { namespace }), // individual "object namespace" string +}); + export function bulkUpdateTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectForbidden = expectResponses.forbiddenTypes('bulk_update'); const expectResponseBody = ( diff --git a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_update.ts b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_update.ts index 90f72e0b34449..1e11d1fc61110 100644 --- a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_update.ts +++ b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_update.ts @@ -39,7 +39,18 @@ const createTestCases = (spaceId: string) => { ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail404() }]; const allTypes = normalTypes.concat(hiddenType); - return { normalTypes, hiddenType, allTypes }; + // an "object namespace" string can be specified for individual objects (to bulkUpdate across namespaces) + const withObjectNamespaces = [ + { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, namespace: DEFAULT_SPACE_ID }, + { ...CASES.SINGLE_NAMESPACE_SPACE_1, namespace: SPACE_1_ID }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespace: SPACE_1_ID, ...fail404() }, // intentional 404 test case + { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespace: DEFAULT_SPACE_ID }, // SPACE_1_ID would also work + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, namespace: SPACE_2_ID, ...fail404() }, // intentional 404 test case + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, namespace: SPACE_2_ID }, + CASES.NAMESPACE_AGNOSTIC, // any namespace would work and would make no difference + { ...CASES.DOES_NOT_EXIST, ...fail404() }, + ]; + return { normalTypes, hiddenType, allTypes, withObjectNamespaces }; }; export default function ({ getService }: FtrProviderContext) { @@ -51,26 +62,42 @@ export default function ({ getService }: FtrProviderContext) { supertest ); const createTests = (spaceId: string) => { - const { normalTypes, hiddenType, allTypes } = createTestCases(spaceId); + const { normalTypes, hiddenType, allTypes, withObjectNamespaces } = createTestCases(spaceId); // use singleRequest to reduce execution time and/or test combined cases + const authorizedCommon = [ + createTestDefinitions(normalTypes, false, { singleRequest: true }), + createTestDefinitions(hiddenType, true), + createTestDefinitions(allTypes, true, { + singleRequest: true, + responseBodyOverride: expectForbidden(['hiddentype']), + }), + ].flat(); return { - unauthorized: createTestDefinitions(allTypes, true), - authorized: [ - createTestDefinitions(normalTypes, false, { singleRequest: true }), - createTestDefinitions(hiddenType, true), - createTestDefinitions(allTypes, true, { - singleRequest: true, - responseBodyOverride: expectForbidden(['hiddentype']), - }), + unauthorized: [ + createTestDefinitions(allTypes, true), + createTestDefinitions(withObjectNamespaces, true, { singleRequest: true }), + ].flat(), + authorizedAtSpace: [ + authorizedCommon, + createTestDefinitions(withObjectNamespaces, true, { singleRequest: true }), + ].flat(), + authorizedAllSpaces: [ + authorizedCommon, + createTestDefinitions(withObjectNamespaces, false, { singleRequest: true }), + ].flat(), + superuser: [ + createTestDefinitions(allTypes, false, { singleRequest: true }), + createTestDefinitions(withObjectNamespaces, false, { singleRequest: true }), ].flat(), - superuser: createTestDefinitions(allTypes, false, { singleRequest: true }), }; }; describe('_bulk_update', () => { getTestScenarios().securityAndSpaces.forEach(({ spaceId, users }) => { const suffix = ` within the ${spaceId} space`; - const { unauthorized, authorized, superuser } = createTests(spaceId); + const { unauthorized, authorizedAtSpace, authorizedAllSpaces, superuser } = createTests( + spaceId + ); const _addTests = (user: TestUser, tests: BulkUpdateTestDefinition[]) => { addTests(`${user.description}${suffix}`, { user, spaceId, tests }); }; @@ -85,8 +112,11 @@ export default function ({ getService }: FtrProviderContext) { ].forEach((user) => { _addTests(user, unauthorized); }); - [users.dualAll, users.allGlobally, users.allAtSpace].forEach((user) => { - _addTests(user, authorized); + [users.allAtSpace].forEach((user) => { + _addTests(user, authorizedAtSpace); + }); + [users.dualAll, users.allGlobally].forEach((user) => { + _addTests(user, authorizedAllSpaces); }); _addTests(users.superuser, superuser); }); diff --git a/x-pack/test/saved_object_api_integration/security_only/apis/bulk_update.ts b/x-pack/test/saved_object_api_integration/security_only/apis/bulk_update.ts index d42eb25b81cf5..39ceb5a70d1b2 100644 --- a/x-pack/test/saved_object_api_integration/security_only/apis/bulk_update.ts +++ b/x-pack/test/saved_object_api_integration/security_only/apis/bulk_update.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { SPACES } from '../../common/lib/spaces'; import { testCaseFailures, getTestScenarios } from '../../common/lib/saved_object_test_utils'; import { TestUser } from '../../common/lib/types'; import { FtrProviderContext } from '../../common/ftr_provider_context'; @@ -13,6 +14,11 @@ import { BulkUpdateTestDefinition, } from '../../common/suites/bulk_update'; +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, + SPACE_1: { spaceId: SPACE_1_ID }, + SPACE_2: { spaceId: SPACE_2_ID }, +} = SPACES; const { fail404 } = testCaseFailures; const createTestCases = () => { @@ -30,7 +36,19 @@ const createTestCases = () => { ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail404() }]; const allTypes = normalTypes.concat(hiddenType); - return { normalTypes, hiddenType, allTypes }; + // an "object namespace" string can be specified for individual objects (to bulkUpdate across namespaces) + // even if the Spaces plugin is disabled, this should work, as `namespace` is handled by the Core API + const withObjectNamespaces = [ + { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, namespace: DEFAULT_SPACE_ID }, + { ...CASES.SINGLE_NAMESPACE_SPACE_1, namespace: SPACE_1_ID }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespace: SPACE_1_ID, ...fail404() }, // intentional 404 test case + { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespace: DEFAULT_SPACE_ID }, // SPACE_1_ID would also work + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, namespace: SPACE_2_ID, ...fail404() }, // intentional 404 test case + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, namespace: SPACE_2_ID }, + CASES.NAMESPACE_AGNOSTIC, // any namespace would work and would make no difference + { ...CASES.DOES_NOT_EXIST, ...fail404() }, + ]; + return { normalTypes, hiddenType, allTypes, withObjectNamespaces }; }; export default function ({ getService }: FtrProviderContext) { @@ -42,10 +60,13 @@ export default function ({ getService }: FtrProviderContext) { supertest ); const createTests = () => { - const { normalTypes, hiddenType, allTypes } = createTestCases(); + const { normalTypes, hiddenType, allTypes, withObjectNamespaces } = createTestCases(); // use singleRequest to reduce execution time and/or test combined cases return { - unauthorized: createTestDefinitions(allTypes, true), + unauthorized: [ + createTestDefinitions(allTypes, true), + createTestDefinitions(withObjectNamespaces, true, { singleRequest: true }), + ].flat(), authorized: [ createTestDefinitions(normalTypes, false, { singleRequest: true }), createTestDefinitions(hiddenType, true), @@ -53,8 +74,12 @@ export default function ({ getService }: FtrProviderContext) { singleRequest: true, responseBodyOverride: expectForbidden(['hiddentype']), }), + createTestDefinitions(withObjectNamespaces, false, { singleRequest: true }), + ].flat(), + superuser: [ + createTestDefinitions(allTypes, false, { singleRequest: true }), + createTestDefinitions(withObjectNamespaces, false, { singleRequest: true }), ].flat(), - superuser: createTestDefinitions(allTypes, false, { singleRequest: true }), }; }; diff --git a/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_update.ts b/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_update.ts index 93e44e357918a..b51ec303fadf3 100644 --- a/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_update.ts +++ b/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_update.ts @@ -16,22 +16,37 @@ const { } = SPACES; const { fail404 } = testCaseFailures; -const createTestCases = (spaceId: string) => [ +const createTestCases = (spaceId: string) => { // for each outcome, if failure !== undefined then we expect to receive // an error; otherwise, we expect to receive a success result - { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, ...fail404(spaceId !== DEFAULT_SPACE_ID) }, - { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) }, - { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail404(spaceId !== SPACE_2_ID) }, - { - ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, - ...fail404(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID), - }, - { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) }, - { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail404(spaceId !== SPACE_2_ID) }, - CASES.NAMESPACE_AGNOSTIC, - { ...CASES.HIDDEN, ...fail404() }, - { ...CASES.DOES_NOT_EXIST, ...fail404() }, -]; + const normal = [ + { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, ...fail404(spaceId !== DEFAULT_SPACE_ID) }, + { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail404(spaceId !== SPACE_2_ID) }, + { + ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, + ...fail404(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID), + }, + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) }, + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail404(spaceId !== SPACE_2_ID) }, + CASES.NAMESPACE_AGNOSTIC, + { ...CASES.HIDDEN, ...fail404() }, + { ...CASES.DOES_NOT_EXIST, ...fail404() }, + ]; + + // an "object namespace" string can be specified for individual objects (to bulkUpdate across namespaces) + const withObjectNamespaces = [ + { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, namespace: DEFAULT_SPACE_ID }, + { ...CASES.SINGLE_NAMESPACE_SPACE_1, namespace: SPACE_1_ID }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespace: SPACE_1_ID, ...fail404() }, // intentional 404 test case + { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespace: DEFAULT_SPACE_ID }, // SPACE_1_ID would also work + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, namespace: SPACE_2_ID, ...fail404() }, // intentional 404 test case + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, namespace: SPACE_2_ID }, + CASES.NAMESPACE_AGNOSTIC, // any namespace would work and would make no difference + { ...CASES.DOES_NOT_EXIST, ...fail404() }, + ]; + return { normal, withObjectNamespaces }; +}; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); @@ -39,8 +54,11 @@ export default function ({ getService }: FtrProviderContext) { const { addTests, createTestDefinitions } = bulkUpdateTestSuiteFactory(esArchiver, supertest); const createTests = (spaceId: string) => { - const testCases = createTestCases(spaceId); - return createTestDefinitions(testCases, false, { singleRequest: true }); + const { normal, withObjectNamespaces } = createTestCases(spaceId); + return [ + createTestDefinitions(normal, false, { singleRequest: true }), + createTestDefinitions(withObjectNamespaces, false, { singleRequest: true }), + ].flat(); }; describe('_bulk_update', () => {