From 1f1d8291d6f27b0ff0e1bfe9f4fe2671fa3cdbd8 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 26 Aug 2021 11:26:34 -0400 Subject: [PATCH] bulkGet saved objects across spaces (#109967) --- docs/api/saved-objects/bulk_get.asciidoc | 10 + ...n-core-server.savedobjectsbulkgetobject.md | 1 + ...er.savedobjectsbulkgetobject.namespaces.md | 15 ++ .../server/saved_objects/routes/bulk_get.ts | 1 + .../service/lib/internal_utils.test.ts | 85 +++++++++ .../service/lib/internal_utils.ts | 38 ++++ .../service/lib/repository.test.js | 93 +++++++--- .../saved_objects/service/lib/repository.ts | 63 ++++++- .../service/saved_objects_client.ts | 11 ++ src/core/server/server.api.md | 1 + ...ecure_saved_objects_client_wrapper.test.ts | 13 +- .../secure_saved_objects_client_wrapper.ts | 6 +- .../spaces_saved_objects_client.test.ts | 171 +++++++++++------- .../spaces_saved_objects_client.ts | 86 ++++++++- .../common/suites/bulk_get.ts | 15 +- .../security_and_spaces/apis/bulk_create.ts | 19 +- .../security_and_spaces/apis/bulk_get.ts | 79 ++++++-- .../security_only/apis/bulk_get.ts | 20 ++ .../spaces_only/apis/bulk_get.ts | 17 +- 19 files changed, 606 insertions(+), 138 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md diff --git a/docs/api/saved-objects/bulk_get.asciidoc b/docs/api/saved-objects/bulk_get.asciidoc index 421de31d2a5b5..f37bbeede9356 100644 --- a/docs/api/saved-objects/bulk_get.asciidoc +++ b/docs/api/saved-objects/bulk_get.asciidoc @@ -31,6 +31,16 @@ experimental[] Retrieve multiple {kib} saved objects by ID. `fields`:: (Optional, array) The fields to return in the `attributes` key of the object response. +`namespaces`:: + (Optional, string array) Identifiers for the <> in which to search for this object. If this is provided, the object + is searched for only in the explicitly defined spaces. If this is not provided, the object is searched for in the current space (default + behavior). +* For shareable object types (registered with `namespaceType: 'multiple'`): this option can be used to specify one or more spaces, including +the "All spaces" identifier (`'*'`). +* For isolated object types (registered with `namespaceType: 'single'` or `namespaceType: 'multiple-isolated'`): this option can only be +used to specify a single space, and the "All spaces" identifier (`'*'`) is not allowed. +* For global object types (registered with `namespaceType: 'agnostic'`): this option cannot be used. + [[saved-objects-api-bulk-get-response-body]] ==== Response body diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.md index ab2c0c1110255..0ad5f1d66ee52 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.md @@ -17,5 +17,6 @@ export interface SavedObjectsBulkGetObject | --- | --- | --- | | [fields](./kibana-plugin-core-server.savedobjectsbulkgetobject.fields.md) | string[] | SavedObject fields to include in the response | | [id](./kibana-plugin-core-server.savedobjectsbulkgetobject.id.md) | string | | +| [namespaces](./kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md) | string[] | Optional namespace(s) for the object to be retrieved in. If this is defined, it will supersede the namespace ID that is in the top-level options.\* For shareable object types (registered with namespaceType: 'multiple'): this option can be used to specify one or more spaces, including the "All spaces" identifier ('*'). \* For isolated object types (registered with namespaceType: 'single' or namespaceType: 'multiple-isolated'): this option can only be used to specify a single space, and the "All spaces" identifier ('*') is not allowed. \* For global object types (registered with namespaceType: 'agnostic'): this option cannot be used. | | [type](./kibana-plugin-core-server.savedobjectsbulkgetobject.type.md) | string | | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md new file mode 100644 index 0000000000000..5add0ad1bdf95 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md @@ -0,0 +1,15 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsBulkGetObject](./kibana-plugin-core-server.savedobjectsbulkgetobject.md) > [namespaces](./kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md) + +## SavedObjectsBulkGetObject.namespaces property + +Optional namespace(s) for the object to be retrieved in. If this is defined, it will supersede the namespace ID that is in the top-level options. + +\* For shareable object types (registered with `namespaceType: 'multiple'`): this option can be used to specify one or more spaces, including the "All spaces" identifier (`'*'`). \* For isolated object types (registered with `namespaceType: 'single'` or `namespaceType: 'multiple-isolated'`): this option can only be used to specify a single space, and the "All spaces" identifier (`'*'`) is not allowed. \* For global object types (registered with `namespaceType: 'agnostic'`): this option cannot be used. + +Signature: + +```typescript +namespaces?: string[]; +``` diff --git a/src/core/server/saved_objects/routes/bulk_get.ts b/src/core/server/saved_objects/routes/bulk_get.ts index 3838e4d3b3c8e..cf051d6cd25cc 100644 --- a/src/core/server/saved_objects/routes/bulk_get.ts +++ b/src/core/server/saved_objects/routes/bulk_get.ts @@ -25,6 +25,7 @@ export const registerBulkGetRoute = (router: IRouter, { coreUsageData }: RouteDe type: schema.string(), id: schema.string(), fields: schema.maybe(schema.arrayOf(schema.string())), + namespaces: schema.maybe(schema.arrayOf(schema.string())), }) ), }, diff --git a/src/core/server/saved_objects/service/lib/internal_utils.test.ts b/src/core/server/saved_objects/service/lib/internal_utils.test.ts index d1fd067990f07..a0b8581e582f6 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.test.ts @@ -13,6 +13,7 @@ import { getBulkOperationError, getSavedObjectFromSource, rawDocExistsInNamespace, + rawDocExistsInNamespaces, } from './internal_utils'; import { ALL_NAMESPACES_STRING } from './utils'; @@ -241,3 +242,87 @@ describe('#rawDocExistsInNamespace', () => { }); }); }); + +describe('#rawDocExistsInNamespaces', () => { + const SINGLE_NAMESPACE_TYPE = 'single-type'; + const MULTI_NAMESPACE_TYPE = 'multi-type'; + const NAMESPACE_AGNOSTIC_TYPE = 'agnostic-type'; + + const registry = typeRegistryMock.create(); + registry.isSingleNamespace.mockImplementation((type) => type === SINGLE_NAMESPACE_TYPE); + registry.isMultiNamespace.mockImplementation((type) => type === MULTI_NAMESPACE_TYPE); + registry.isNamespaceAgnostic.mockImplementation((type) => type === NAMESPACE_AGNOSTIC_TYPE); + + function createRawDoc( + type: string, + namespaceAttrs: { namespace?: string; namespaces?: string[] } + ) { + return { + // other fields exist on the raw document, but they are not relevant to these test cases + _source: { + type, + ...namespaceAttrs, + }, + } as SavedObjectsRawDoc; + } + + describe('single-namespace type', () => { + it('returns true regardless of namespace or namespaces fields', () => { + // Technically, a single-namespace type does not exist in a space unless it has a namespace prefix in its raw ID and a matching + // 'namespace' field. However, historically we have not enforced the latter, we have just relied on searching for and deserializing + // documents with the correct namespace prefix. We may revisit this in the future. + const doc1 = createRawDoc(SINGLE_NAMESPACE_TYPE, { namespace: 'some-space' }); // the namespace field is ignored + const doc2 = createRawDoc(SINGLE_NAMESPACE_TYPE, { namespaces: ['some-space'] }); // the namespaces field is ignored + expect(rawDocExistsInNamespaces(registry, doc1, [])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc1, ['some-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc1, ['other-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc2, [])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc2, ['some-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc2, ['other-space'])).toBe(true); + }); + }); + + describe('multi-namespace type', () => { + const docInDefaultSpace = createRawDoc(MULTI_NAMESPACE_TYPE, { namespaces: ['default'] }); + const docInSomeSpace = createRawDoc(MULTI_NAMESPACE_TYPE, { namespaces: ['some-space'] }); + const docInAllSpaces = createRawDoc(MULTI_NAMESPACE_TYPE, { + namespaces: [ALL_NAMESPACES_STRING], + }); + const docInNoSpace = createRawDoc(MULTI_NAMESPACE_TYPE, { namespaces: [] }); + + it('returns true when the document namespaces matches', () => { + expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, ['default'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, docInAllSpaces, ['default'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, docInSomeSpace, ['some-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, docInAllSpaces, ['some-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, ['*'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, docInSomeSpace, ['*'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, docInAllSpaces, ['*'])).toBe(true); + }); + + it('returns false when the document namespace does not match', () => { + expect(rawDocExistsInNamespaces(registry, docInSomeSpace, ['default'])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInNoSpace, ['default'])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, ['some-space'])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInNoSpace, ['some-space'])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInNoSpace, ['*'])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, [])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInSomeSpace, [])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInAllSpaces, [])).toBe(false); + expect(rawDocExistsInNamespaces(registry, docInNoSpace, [])).toBe(false); + }); + }); + + describe('namespace-agnostic type', () => { + it('returns true regardless of namespace or namespaces fields', () => { + const doc1 = createRawDoc(NAMESPACE_AGNOSTIC_TYPE, { namespace: 'some-space' }); // the namespace field is ignored + const doc2 = createRawDoc(NAMESPACE_AGNOSTIC_TYPE, { namespaces: ['some-space'] }); // the namespaces field is ignored + expect(rawDocExistsInNamespaces(registry, doc1, [])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc1, ['some-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc1, ['other-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc2, [])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc2, ['some-space'])).toBe(true); + expect(rawDocExistsInNamespaces(registry, doc2, ['other-space'])).toBe(true); + }); + }); +}); diff --git a/src/core/server/saved_objects/service/lib/internal_utils.ts b/src/core/server/saved_objects/service/lib/internal_utils.ts index feaaea15649c7..ed6ede0fe6d49 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.ts @@ -141,3 +141,41 @@ export function rawDocExistsInNamespace( namespaces?.includes(ALL_NAMESPACES_STRING); return existsInNamespace ?? false; } + +/** + * Check to ensure that a raw document exists in at least one of the given namespaces. If the document is not a multi-namespace type, then + * this returns `true` as we rely on the guarantees of the document ID format. If the document is a multi-namespace type, this checks to + * ensure that the document's `namespaces` value includes the string representation of at least one of the given namespaces. + * + * WARNING: This should only be used for documents that were retrieved from Elasticsearch. Otherwise, the guarantees of the document ID + * format mentioned above do not apply. + * + * @param registry + * @param raw + * @param namespaces + */ +export function rawDocExistsInNamespaces( + registry: ISavedObjectTypeRegistry, + raw: SavedObjectsRawDoc, + namespaces: string[] +) { + const rawDocType = raw._source.type; + + // if the type is namespace isolated, or namespace agnostic, we can continue to rely on the guarantees + // of the document ID format and don't need to check this + if (!registry.isMultiNamespace(rawDocType)) { + return true; + } + + const namespacesToCheck = new Set(namespaces); + const existingNamespaces = raw._source.namespaces ?? []; + + if (namespacesToCheck.size === 0 || existingNamespaces.length === 0) { + return false; + } + if (namespacesToCheck.has(ALL_NAMESPACES_STRING)) { + return true; + } + + return existingNamespaces.some((x) => x === ALL_NAMESPACES_STRING || namespacesToCheck.has(x)); +} 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 efae5bd737020..d4d794c165fe2 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -953,7 +953,7 @@ describe('SavedObjectsRepository', () => { const bulkGet = async (objects, options) => savedObjectsRepository.bulkGet( - objects.map(({ type, id }) => ({ type, id })), // bulkGet only uses type and id + objects.map(({ type, id, namespaces }) => ({ type, id, namespaces })), // bulkGet only uses type, id, and optionally namespaces options ); const bulkGetSuccess = async (objects, options) => { @@ -992,6 +992,13 @@ describe('SavedObjectsRepository', () => { _expectClientCallArgs([obj1, obj2], { getId }); }); + it(`prepends namespace to the id when providing namespaces for single-namespace type`, async () => { + const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const objects = [obj1, obj2].map((obj) => ({ ...obj, namespaces: [namespace] })); + await bulkGetSuccess(objects, { namespace: 'some-other-ns' }); + _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}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkGetSuccess([obj1, obj2]); @@ -1011,33 +1018,35 @@ describe('SavedObjectsRepository', () => { _expectClientCallArgs(objects, { getId }); client.mget.mockClear(); - objects = [obj1, obj2].map((obj) => ({ ...obj, type: MULTI_NAMESPACE_ISOLATED_TYPE })); + objects = [obj1, { ...obj2, namespaces: ['some-other-ns'] }].map((obj) => ({ + ...obj, + type: MULTI_NAMESPACE_ISOLATED_TYPE, + })); await bulkGetSuccess(objects, { namespace }); _expectClientCallArgs(objects, { getId }); }); }); describe('errors', () => { - const bulkGetErrorInvalidType = async ([obj1, obj, obj2]) => { - const response = getMockMgetResponse([obj1, obj2]); + const bulkGetError = async (obj, isBulkError, expectedErrorResult) => { + let response; + if (isBulkError) { + // mock the bulk error for only the second object + mockGetBulkOperationError.mockReturnValueOnce(undefined); + mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error); + response = getMockMgetResponse([obj1, obj, obj2]); + } else { + response = getMockMgetResponse([obj1, obj2]); + } client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); - const result = await bulkGet([obj1, obj, obj2]); - expect(client.mget).toHaveBeenCalled(); - expect(result).toEqual({ - saved_objects: [expectSuccess(obj1), expectErrorInvalidType(obj), expectSuccess(obj2)], - }); - }; - const bulkGetErrorNotFound = async ([obj1, obj, obj2], options, response) => { - client.mget.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise(response) - ); - const result = await bulkGet([obj1, obj, obj2], options); + const objects = [obj1, obj, obj2]; + const result = await bulkGet(objects); expect(client.mget).toHaveBeenCalled(); expect(result).toEqual({ - saved_objects: [expectSuccess(obj1), expectErrorNotFound(obj), expectSuccess(obj2)], + saved_objects: [expectSuccess(obj1), expectedErrorResult, expectSuccess(obj2)], }); }; @@ -1063,33 +1072,65 @@ describe('SavedObjectsRepository', () => { ).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"')); }); + it(`returns error when namespaces is used with a space-agnostic object`, async () => { + const obj = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'three', namespaces: [] }; + await bulkGetError( + obj, + undefined, + expectErrorResult( + obj, + createBadRequestError('"namespaces" cannot be used on space-agnostic types') + ) + ); + }); + + it(`returns error when namespaces is used with a space-isolated object and does not specify a single space`, async () => { + const doTest = async (objType, namespaces) => { + const obj = { type: objType, id: 'three', namespaces }; + await bulkGetError( + obj, + undefined, + expectErrorResult( + obj, + createBadRequestError( + '"namespaces" can only specify a single space when used with space-isolated types' + ) + ) + ); + }; + await doTest('dashboard', ['spacex', 'spacey']); + await doTest('dashboard', ['*']); + await doTest(MULTI_NAMESPACE_ISOLATED_TYPE, ['spacex', 'spacey']); + await doTest(MULTI_NAMESPACE_ISOLATED_TYPE, ['*']); + }); + it(`returns error when type is invalid`, async () => { const obj = { type: 'unknownType', id: 'three' }; - await bulkGetErrorInvalidType([obj1, obj, obj2]); + await bulkGetError(obj, undefined, expectErrorInvalidType(obj)); }); it(`returns error when type is hidden`, async () => { const obj = { type: HIDDEN_TYPE, id: 'three' }; - await bulkGetErrorInvalidType([obj1, obj, obj2]); + await bulkGetError(obj, undefined, expectErrorInvalidType(obj)); }); it(`returns error when document is not found`, async () => { const obj = { type: 'dashboard', id: 'three', found: false }; - const response = getMockMgetResponse([obj1, obj, obj2]); - await bulkGetErrorNotFound([obj1, obj, obj2], undefined, response); + await bulkGetError(obj, true, expectErrorNotFound(obj)); }); it(`handles missing ids gracefully`, async () => { const obj = { type: 'dashboard', id: undefined, found: false }; - const response = getMockMgetResponse([obj1, obj, obj2]); - await bulkGetErrorNotFound([obj1, obj, obj2], undefined, response); + await bulkGetError(obj, true, expectErrorNotFound(obj)); }); it(`returns error when type is multi-namespace and the document exists, but not in this namespace`, async () => { - const obj = { type: MULTI_NAMESPACE_ISOLATED_TYPE, id: 'three' }; - const response = getMockMgetResponse([obj1, obj, obj2]); - response.docs[1].namespaces = ['bar-namespace']; - await bulkGetErrorNotFound([obj1, obj, obj2], { namespace }, response); + const obj = { + type: MULTI_NAMESPACE_ISOLATED_TYPE, + id: 'three', + namespace: 'bar-namespace', + }; + await bulkGetError(obj, true, expectErrorNotFound(obj)); }); it(`throws when ES mget action responds with a 404 and a missing Elasticsearch product header`, async () => { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 365fc6a3734e4..c2ae6ebab00e9 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -74,6 +74,7 @@ import { getExpectedVersionProperties, getSavedObjectFromSource, rawDocExistsInNamespace, + rawDocExistsInNamespaces, } from './internal_utils'; import { ALL_NAMESPACES_STRING, @@ -966,16 +967,23 @@ export class SavedObjectsRepository { let bulkGetRequestIndexCounter = 0; const expectedBulkGetResults: Either[] = objects.map((object) => { - const { type, id, fields } = object; + const { type, id, fields, namespaces } = object; + let error: DecoratedError | undefined; if (!this._allowedTypes.includes(type)) { + error = SavedObjectsErrorHelpers.createUnsupportedTypeError(type); + } else { + try { + this.validateObjectNamespaces(type, id, namespaces); + } catch (e) { + error = e; + } + } + + if (error) { return { tag: 'Left' as 'Left', - error: { - id, - type, - error: errorContent(SavedObjectsErrorHelpers.createUnsupportedTypeError(type)), - }, + error: { id, type, error: errorContent(error) }, }; } @@ -985,15 +993,18 @@ export class SavedObjectsRepository { type, id, fields, + namespaces, esRequestIndex: bulkGetRequestIndexCounter++, }, }; }); + const getNamespaceId = (namespaces?: string[]) => + namespaces !== undefined ? SavedObjectsUtils.namespaceStringToId(namespaces[0]) : namespace; const bulkGetDocs = expectedBulkGetResults .filter(isRight) - .map(({ value: { type, id, fields } }) => ({ - _id: this._serializer.generateRawId(namespace, type, id), + .map(({ value: { type, id, fields, namespaces } }) => ({ + _id: this._serializer.generateRawId(getNamespaceId(namespaces), type, id), // the namespace prefix is only used for single-namespace object types _index: this.getIndexForType(type), _source: { includes: includedFields(type, fields) }, })); @@ -1023,11 +1034,16 @@ export class SavedObjectsRepository { return expectedResult.error as any; } - const { type, id, esRequestIndex } = expectedResult.value; + const { + type, + id, + namespaces = [SavedObjectsUtils.namespaceIdToString(namespace)], + esRequestIndex, + } = expectedResult.value; const doc = bulkGetResponse?.body.docs[esRequestIndex]; // @ts-expect-error MultiGetHit._source is optional - if (!doc?.found || !this.rawDocExistsInNamespace(doc, namespace)) { + if (!doc?.found || !this.rawDocExistsInNamespaces(doc, namespaces)) { return ({ id, type, @@ -2116,6 +2132,10 @@ export class SavedObjectsRepository { return omit(savedObject, ['namespace']) as SavedObject; } + private rawDocExistsInNamespaces(raw: SavedObjectsRawDoc, namespaces: string[]) { + return rawDocExistsInNamespaces(this._registry, raw, namespaces); + } + private rawDocExistsInNamespace(raw: SavedObjectsRawDoc, namespace: string | undefined) { return rawDocExistsInNamespace(this._registry, raw, namespace); } @@ -2228,6 +2248,7 @@ export class SavedObjectsRepository { ).catch(() => {}); // if the call fails for some reason, intentionally swallow the error } + /** The `initialNamespaces` field (create, bulkCreate) is used to create an object in an initial set of spaces. */ private validateInitialNamespaces(type: string, initialNamespaces: string[] | undefined) { if (!initialNamespaces) { return; @@ -2250,6 +2271,28 @@ export class SavedObjectsRepository { ); } } + + /** The object-specific `namespaces` field (bulkGet) is used to check if an object exists in any of a given number of spaces. */ + private validateObjectNamespaces(type: string, id: string, namespaces: string[] | undefined) { + if (!namespaces) { + return; + } + + if (this._registry.isNamespaceAgnostic(type)) { + throw SavedObjectsErrorHelpers.createBadRequestError( + '"namespaces" cannot be used on space-agnostic types' + ); + } else if (!namespaces.length) { + throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); + } else if ( + !this._registry.isShareable(type) && + (namespaces.length > 1 || namespaces.includes(ALL_NAMESPACES_STRING)) + ) { + throw SavedObjectsErrorHelpers.createBadRequestError( + '"namespaces" can only specify a single space when used with space-isolated types' + ); + } + } } /** 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 1564df2969ecc..dd9a8393e0624 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -278,6 +278,17 @@ export interface SavedObjectsBulkGetObject { type: string; /** SavedObject fields to include in the response */ fields?: string[]; + /** + * Optional namespace(s) for the object to be retrieved in. If this is defined, it will supersede the namespace ID that is in the + * top-level options. + * + * * For shareable object types (registered with `namespaceType: 'multiple'`): this option can be used to specify one or more spaces, + * including the "All spaces" identifier (`'*'`). + * * For isolated object types (registered with `namespaceType: 'single'` or `namespaceType: 'multiple-isolated'`): this option can only + * be used to specify a single space, and the "All spaces" identifier (`'*'`) is not allowed. + * * For global object types (registered with `namespaceType: 'agnostic'`): this option cannot be used. + */ + namespaces?: string[]; } /** diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 67b08f4c0d9b7..1278439f1649f 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -1857,6 +1857,7 @@ export interface SavedObjectsBulkGetObject { fields?: string[]; // (undocumented) id: string; + namespaces?: string[]; // (undocumented) type: string; } 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 2f622d9e8a0e1..96f8c5ff02d24 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 @@ -426,10 +426,17 @@ describe('#bulkGet', () => { expect(result).toEqual(apiCallReturnValue); }); - test(`checks privileges for user, actions, and namespace`, async () => { - const objects = [obj1, obj2]; + test(`checks privileges for user, actions, namespace, and (object) namespaces`, async () => { + const objects = [ + { ...obj1, namespaces: ['another-ns'] }, + { ...obj2, namespaces: ['yet-another-ns'] }, + ]; const options = { namespace }; - await expectPrivilegeCheck(client.bulkGet, { objects, options }, namespace); + await expectPrivilegeCheck(client.bulkGet, { objects, options }, [ + namespace, + 'another-ns', + 'yet-another-ns', + ]); }); 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 6f2b8d28a5601..11eca287cd4f5 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 @@ -287,11 +287,15 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra options: SavedObjectsBaseOptions = {} ) { try { + const namespaces = objects.reduce( + (acc, { namespaces: objNamespaces = [] }) => acc.concat(objNamespaces), + [options.namespace] + ); const args = { objects, options }; await this.legacyEnsureAuthorized( this.getUniqueObjectTypes(objects), 'bulk_get', - options.namespace, + namespaces, { args, } diff --git a/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.test.ts b/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.test.ts index b94113436d7ad..85c6ce74763b2 100644 --- a/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.test.ts +++ b/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.test.ts @@ -7,37 +7,15 @@ import Boom from '@hapi/boom'; -import { SavedObjectTypeRegistry } from 'src/core/server'; -import { savedObjectsClientMock } from 'src/core/server/mocks'; +import type { SavedObject, SavedObjectsType } from 'src/core/server'; +import { SavedObjectsErrorHelpers } from 'src/core/server'; +import { savedObjectsClientMock, savedObjectsTypeRegistryMock } from 'src/core/server/mocks'; import { DEFAULT_SPACE_ID } from '../../common/constants'; -import type { SpacesClient } from '../spaces_client'; import { spacesClientMock } from '../spaces_client/spaces_client.mock'; import { spacesServiceMock } from '../spaces_service/spaces_service.mock'; import { SpacesSavedObjectsClient } from './spaces_saved_objects_client'; -const typeRegistry = new SavedObjectTypeRegistry(); -typeRegistry.registerType({ - name: 'foo', - namespaceType: 'single', - hidden: false, - mappings: { properties: {} }, -}); - -typeRegistry.registerType({ - name: 'bar', - namespaceType: 'single', - hidden: false, - mappings: { properties: {} }, -}); - -typeRegistry.registerType({ - name: 'space', - namespaceType: 'agnostic', - hidden: true, - mappings: { properties: {} }, -}); - const createMockRequest = () => ({}); const createMockClient = () => savedObjectsClientMock.create(); @@ -68,6 +46,15 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; const request = createMockRequest(); const baseClient = createMockClient(); const spacesService = createSpacesService(currentSpace.id); + const spacesClient = spacesClientMock.create(); + spacesService.createSpacesClient.mockReturnValue(spacesClient); + const typeRegistry = savedObjectsTypeRegistryMock.create(); + typeRegistry.getAllTypes.mockReturnValue(([ + // for test purposes we only need the names of the object types + { name: 'foo' }, + { name: 'bar' }, + { name: 'space' }, + ] as unknown) as SavedObjectsType[]); const client = new SpacesSavedObjectsClient({ request, @@ -75,7 +62,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; getSpacesService: () => spacesService, typeRegistry, }); - return { client, baseClient, spacesService }; + return { client, baseClient, spacesClient, typeRegistry }; }; describe('#get', () => { @@ -147,7 +134,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`supplements options with the current namespace`, async () => { - const { client, baseClient } = createSpacesSavedObjectsClient(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { saved_objects: [createMockResponse()] }; baseClient.bulkGet.mockReturnValue(Promise.resolve(expectedReturnValue)); @@ -156,11 +143,94 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; // @ts-expect-error const actualReturnValue = await client.bulkGet(objects, options); - expect(actualReturnValue).toBe(expectedReturnValue); + expect(actualReturnValue).toEqual(expectedReturnValue); expect(baseClient.bulkGet).toHaveBeenCalledWith(objects, { foo: 'bar', namespace: currentSpace.expectedNamespace, }); + expect(spacesClient.getAll).not.toHaveBeenCalled(); + }); + + test(`replaces object namespaces '*' with available spaces`, async () => { + const { client, baseClient, spacesClient, typeRegistry } = createSpacesSavedObjectsClient(); + spacesClient.getAll.mockResolvedValue([ + { id: 'available-space-a', name: 'a', disabledFeatures: [] }, + { id: 'available-space-b', name: 'b', disabledFeatures: [] }, + ]); + typeRegistry.isNamespaceAgnostic.mockImplementation((type) => type === 'foo'); + typeRegistry.isShareable.mockImplementation((type) => type === 'bar'); + // 'baz' is neither agnostic nor shareable, so it is isolated (namespaceType: 'single' or namespaceType: 'multiple-isolated') + baseClient.bulkGet.mockResolvedValue({ + saved_objects: ([ + { type: 'foo', id: '1', key: 'val' }, + { type: 'bar', id: '2', key: 'val' }, + { type: 'baz', id: '3', key: 'val' }, // this should be replaced with a 400 error + { type: 'foo', id: '4', key: 'val' }, + { type: 'bar', id: '5', key: 'val' }, + { type: 'baz', id: '6', key: 'val' }, // this should not be replaced with a 400 error because the user did not search for it in '*' all spaces + ] as unknown) as SavedObject[], + }); + + const objects = [ + { type: 'foo', id: '1', namespaces: ['*', 'this-is-ignored'] }, + { type: 'bar', id: '2', namespaces: ['*', 'this-is-ignored'] }, + { type: 'baz', id: '3', namespaces: ['*', 'this-is-ignored'] }, + { type: 'foo', id: '4', namespaces: ['another-space'] }, + { type: 'bar', id: '5', namespaces: ['another-space'] }, + { type: 'baz', id: '6', namespaces: ['another-space'] }, + ]; + const result = await client.bulkGet(objects); + + expect(result.saved_objects).toEqual([ + { type: 'foo', id: '1', key: 'val' }, + { type: 'bar', id: '2', key: 'val' }, + { + type: 'baz', + id: '3', + error: SavedObjectsErrorHelpers.createBadRequestError( + '"namespaces" can only specify a single space when used with space-isolated types' + ).output.payload, + }, + { type: 'foo', id: '4', key: 'val' }, + { type: 'bar', id: '5', key: 'val' }, + { type: 'baz', id: '6', key: 'val' }, + ]); + expect(baseClient.bulkGet).toHaveBeenCalledWith( + [ + { type: 'foo', id: '1', namespaces: ['available-space-a', 'available-space-b'] }, + { type: 'bar', id: '2', namespaces: ['available-space-a', 'available-space-b'] }, + { type: 'baz', id: '3', namespaces: ['available-space-a', 'available-space-b'] }, + // even if another space doesn't exist, it can be specified explicitly + { type: 'foo', id: '4', namespaces: ['another-space'] }, + { type: 'bar', id: '5', namespaces: ['another-space'] }, + { type: 'baz', id: '6', namespaces: ['another-space'] }, + ], + { namespace: currentSpace.expectedNamespace } + ); + expect(spacesClient.getAll).toHaveBeenCalledTimes(1); + }); + + test(`replaces object namespaces '*' with an empty array when the user doesn't have access to any spaces`, async () => { + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); + spacesClient.getAll.mockRejectedValue(Boom.forbidden()); + baseClient.bulkGet.mockResolvedValue({ saved_objects: [] }); // doesn't matter for this test + + const objects = [ + { type: 'foo', id: '1', namespaces: ['*'] }, + { type: 'bar', id: '2', namespaces: ['*', 'this-is-ignored'] }, + { type: 'baz', id: '3', namespaces: ['another-space'] }, + ]; + await client.bulkGet(objects); + + expect(baseClient.bulkGet).toHaveBeenCalledWith( + [ + { type: 'foo', id: '1', namespaces: [] }, + { type: 'bar', id: '2', namespaces: [] }, + { type: 'baz', id: '3', namespaces: ['another-space'] }, // even if another space doesn't exist, it can be specified explicitly + ], + { namespace: currentSpace.expectedNamespace } + ); + expect(spacesClient.getAll).toHaveBeenCalledTimes(1); }); }); @@ -168,10 +238,8 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; const EMPTY_RESPONSE = { saved_objects: [], total: 0, per_page: 20, page: 1 }; test(`returns empty result if user is unauthorized in this space`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); - const spacesClient = spacesClientMock.create(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); spacesClient.getAll.mockResolvedValue([]); - spacesService.createSpacesClient.mockReturnValue(spacesClient); const options = Object.freeze({ type: 'foo', namespaces: ['some-ns'] }); const actualReturnValue = await client.find(options); @@ -181,10 +249,8 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`returns empty result if user is unauthorized in any space`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); - const spacesClient = spacesClientMock.create(); - spacesClient.getAll.mockRejectedValue(Boom.unauthorized()); - spacesService.createSpacesClient.mockReturnValue(spacesClient); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); + spacesClient.getAll.mockRejectedValue(Boom.forbidden()); const options = Object.freeze({ type: 'foo', namespaces: ['some-ns'] }); const actualReturnValue = await client.find(options); @@ -234,7 +300,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`passes options.namespaces along`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { saved_objects: [createMockResponse()], total: 1, @@ -243,9 +309,6 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }; baseClient.find.mockReturnValue(Promise.resolve(expectedReturnValue)); - const spacesClient = spacesService.createSpacesClient( - null as any - ) as jest.Mocked; spacesClient.getAll.mockImplementation(() => Promise.resolve([ { id: 'ns-1', name: '', disabledFeatures: [] }, @@ -265,7 +328,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`filters options.namespaces based on authorization`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { saved_objects: [createMockResponse()], total: 1, @@ -274,9 +337,6 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }; baseClient.find.mockReturnValue(Promise.resolve(expectedReturnValue)); - const spacesClient = spacesService.createSpacesClient( - null as any - ) as jest.Mocked; spacesClient.getAll.mockImplementation(() => Promise.resolve([ { id: 'ns-1', name: '', disabledFeatures: [] }, @@ -296,7 +356,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`translates options.namespace: ['*']`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { saved_objects: [createMockResponse()], total: 1, @@ -305,9 +365,6 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }; baseClient.find.mockReturnValue(Promise.resolve(expectedReturnValue)); - const spacesClient = spacesService.createSpacesClient( - null as any - ) as jest.Mocked; spacesClient.getAll.mockImplementation(() => Promise.resolve([ { id: 'ns-1', name: '', disabledFeatures: [] }, @@ -534,10 +591,8 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; describe('#openPointInTimeForType', () => { test(`throws error if if user is unauthorized in this space`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); - const spacesClient = spacesClientMock.create(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); spacesClient.getAll.mockResolvedValue([]); - spacesService.createSpacesClient.mockReturnValue(spacesClient); await expect( client.openPointInTimeForType('foo', { namespaces: ['bar'] }) @@ -547,10 +602,8 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`throws error if if user is unauthorized in any space`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); - const spacesClient = spacesClientMock.create(); - spacesClient.getAll.mockRejectedValue(Boom.unauthorized()); - spacesService.createSpacesClient.mockReturnValue(spacesClient); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); + spacesClient.getAll.mockRejectedValue(Boom.forbidden()); await expect( client.openPointInTimeForType('foo', { namespaces: ['bar'] }) @@ -560,13 +613,10 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`filters options.namespaces based on authorization`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { id: 'abc123' }; baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); - const spacesClient = spacesService.createSpacesClient( - null as any - ) as jest.Mocked; spacesClient.getAll.mockImplementation(() => Promise.resolve([ { id: 'ns-1', name: '', disabledFeatures: [] }, @@ -585,13 +635,10 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); test(`translates options.namespaces: ['*']`, async () => { - const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const { client, baseClient, spacesClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { id: 'abc123' }; baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); - const spacesClient = spacesService.createSpacesClient( - null as any - ) as jest.Mocked; spacesClient.getAll.mockImplementation(() => Promise.resolve([ { id: 'ns-1', name: '', disabledFeatures: [] }, diff --git a/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts b/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts index 9c51f22e280d8..6cfd784042317 100644 --- a/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts +++ b/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts @@ -9,6 +9,7 @@ import Boom from '@hapi/boom'; import type { ISavedObjectTypeRegistry, + SavedObject, SavedObjectsBaseOptions, SavedObjectsBulkCreateObject, SavedObjectsBulkGetObject, @@ -36,6 +37,19 @@ import { spaceIdToNamespace } from '../lib/utils/namespace'; import type { ISpacesClient } from '../spaces_client'; import type { SpacesServiceStart } from '../spaces_service/spaces_service'; +interface Left { + tag: 'Left'; + value: L; +} + +interface Right { + tag: 'Right'; + value: R; +} + +type Either = Left | Right; +const isLeft = (either: Either): either is Left => either.tag === 'Left'; + interface SpacesSavedObjectsClientOptions { baseClient: SavedObjectsClientContract; request: any; @@ -59,6 +73,7 @@ const throwErrorIfNamespaceSpecified = (options: any) => { export class SpacesSavedObjectsClient implements SavedObjectsClientContract { private readonly client: SavedObjectsClientContract; + private readonly typeRegistry: ISavedObjectTypeRegistry; private readonly spaceId: string; private readonly types: string[]; private readonly spacesClient: ISpacesClient; @@ -70,6 +85,7 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { const spacesService = getSpacesService(); this.client = baseClient; + this.typeRegistry = typeRegistry; this.spacesClient = spacesService.createSpacesClient(request); this.spaceId = spacesService.getSpaceId(request); this.types = typeRegistry.getAllTypes().map((t) => t.name); @@ -219,10 +235,61 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { ) { throwErrorIfNamespaceSpecified(options); - return await this.client.bulkGet(objects, { - ...options, - namespace: spaceIdToNamespace(this.spaceId), - }); + let availableSpacesPromise: Promise | undefined; + const getAvailableSpaces = async () => { + if (!availableSpacesPromise) { + availableSpacesPromise = this.getSearchableSpaces([ALL_SPACES_ID]).catch((err) => { + if (Boom.isBoom(err) && err.output.payload.statusCode === 403) { + return []; // the user doesn't have access to any spaces + } else { + throw err; + } + }); + } + return availableSpacesPromise; + }; + + const expectedResults = await Promise.all( + objects.map>>(async (object) => { + const { namespaces, type } = object; + if (namespaces?.includes(ALL_SPACES_ID)) { + // If searching for an isolated object in all spaces, we may need to return a 400 error for consistency with the validation at the + // repository level. This is needed if there is only one space available *and* the user is authorized to access the object in that + // space; in that case, we don't want to unintentionally bypass the repository's validation by deconstructing the '*' identifier + // into all available spaces. + const tag = + !this.typeRegistry.isNamespaceAgnostic(type) && !this.typeRegistry.isShareable(type) + ? 'Left' + : 'Right'; + return { tag, value: { ...object, namespaces: await getAvailableSpaces() } }; + } + return { tag: 'Right', value: object }; + }) + ); + + const objectsToGet = expectedResults.map(({ value }) => value); + const { saved_objects: responseObjects } = objectsToGet.length + ? await this.client.bulkGet(objectsToGet, { + ...options, + namespace: spaceIdToNamespace(this.spaceId), + }) + : { saved_objects: [] }; + return { + saved_objects: expectedResults.map((expectedResult, i) => { + const actualResult = responseObjects[i]; + if (isLeft(expectedResult)) { + const { type, id } = expectedResult.value; + return ({ + type, + id, + error: SavedObjectsErrorHelpers.createBadRequestError( + '"namespaces" can only specify a single space when used with space-isolated types' + ).output.payload, + } as unknown) as SavedObject; + } + return actualResult; + }), + }; } /** @@ -383,7 +450,16 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { type: string | string[], options: SavedObjectsOpenPointInTimeOptions = {} ) { - const namespaces = await this.getSearchableSpaces(options.namespaces); + let namespaces: string[]; + try { + namespaces = await this.getSearchableSpaces(options.namespaces); + } catch (err) { + if (Boom.isBoom(err) && err.output.payload.statusCode === 403) { + // throw bad request since the user is unauthorized in any space + throw SavedObjectsErrorHelpers.createBadRequestError(); + } + throw err; + } if (namespaces.length === 0) { // throw bad request if no valid spaces were found. throw SavedObjectsErrorHelpers.createBadRequestError(); diff --git a/x-pack/test/saved_object_api_integration/common/suites/bulk_get.ts b/x-pack/test/saved_object_api_integration/common/suites/bulk_get.ts index 26a693349496d..abfb1f12a2771 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/bulk_get.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/bulk_get.ts @@ -9,12 +9,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 BulkGetTestDefinition extends TestDefinition { @@ -22,6 +17,7 @@ export interface BulkGetTestDefinition extends TestDefinition { } export type BulkGetTestSuite = TestSuite; export interface BulkGetTestCase extends TestCase { + namespaces?: string[]; // used to define individual "object namespaces" string arrays, e.g., bulkGet across multiple namespaces failure?: 400 | 404; // only used for permitted response case } @@ -31,6 +27,12 @@ export const TEST_CASES: Record = Object.freeze({ DOES_NOT_EXIST, }); +const createRequest = ({ type, id, namespaces }: BulkGetTestCase) => ({ + type, + id, + ...(namespaces && { namespaces }), // individual "object namespaces" string array +}); + export function bulkGetTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectSavedObjectForbidden = expectResponses.forbiddenTypes('bulk_get'); const expectResponseBody = ( @@ -49,6 +51,7 @@ export function bulkGetTestSuiteFactory(esArchiver: any, supertest: SuperTest { CASES.INITIAL_NS_MULTI_NAMESPACE_OBJ_ALL_SPACES, ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; - const allTypes = normalTypes.concat(hiddenType); + const allTypes = [...normalTypes, ...crossNamespace, ...hiddenType]; return { normalTypes, crossNamespace, hiddenType, allTypes }; }; @@ -118,18 +118,17 @@ export default function ({ getService }: FtrProviderContext) { singleRequest: true, }), createTestDefinitions(hiddenType, true, overwrite, { spaceId, user }), - createTestDefinitions(allTypes, true, overwrite, { - spaceId, - user, - singleRequest: true, - responseBodyOverride: expectSavedObjectForbidden(['hiddentype']), - }), ].flat(); return { unauthorized: createTestDefinitions(allTypes, true, overwrite, { spaceId, user }), authorizedAtSpace: [ authorizedCommon, createTestDefinitions(crossNamespace, true, overwrite, { spaceId, user }), + createTestDefinitions(allTypes, true, overwrite, { + spaceId, + user, + singleRequest: true, + }), ].flat(), authorizedEverywhere: [ authorizedCommon, @@ -138,6 +137,12 @@ export default function ({ getService }: FtrProviderContext) { user, singleRequest: true, }), + createTestDefinitions(allTypes, true, overwrite, { + spaceId, + user, + singleRequest: true, + responseBodyOverride: expectSavedObjectForbidden(['hiddentype']), + }), ].flat(), superuser: createTestDefinitions(allTypes, false, overwrite, { spaceId, diff --git a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_get.ts b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_get.ts index d547b95d34f7e..bfea8e558010b 100644 --- a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_get.ts +++ b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_get.ts @@ -5,13 +5,14 @@ * 2.0. */ -import { SPACES } from '../../common/lib/spaces'; +import { SPACES, ALL_SPACES_ID } 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'; import { bulkGetTestSuiteFactory, TEST_CASES as CASES, + BulkGetTestCase, BulkGetTestDefinition, } from '../../common/suites/bulk_get'; @@ -44,9 +45,26 @@ const createTestCases = (spaceId: string) => { CASES.NAMESPACE_AGNOSTIC, { ...CASES.DOES_NOT_EXIST, ...fail404() }, ]; + const crossNamespace = [ + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + namespaces: ['x', 'y'], + ...fail400(), // cannot be searched for in multiple spaces + }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespaces: [SPACE_2_ID] }, // second try searches for it in a single other space, which is valid + { + ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, + namespaces: [ALL_SPACES_ID], + ...fail400(), // cannot be searched for in multiple spaces + }, + { ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, namespaces: [SPACE_1_ID] }, // second try searches for it in a single other space, which is valid + { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespaces: [SPACE_2_ID], ...fail404() }, + { ...CASES.MULTI_NAMESPACE_ALL_SPACES, namespaces: [SPACE_2_ID, 'x'] }, // unknown space is allowed / ignored + { ...CASES.MULTI_NAMESPACE_ALL_SPACES, namespaces: [ALL_SPACES_ID] }, // this is different than the same test case in the spaces_only and security_only suites, since MULTI_NAMESPACE_ONLY_SPACE_1 *may* return a 404 error to a partially authorized user + ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; - const allTypes = normalTypes.concat(hiddenType); - return { normalTypes, hiddenType, allTypes }; + const allTypes = [...normalTypes, ...crossNamespace, ...hiddenType]; + return { normalTypes, crossNamespace, hiddenType, allTypes }; }; export default function ({ getService }: FtrProviderContext) { @@ -58,13 +76,39 @@ export default function ({ getService }: FtrProviderContext) { supertest ); const createTests = (spaceId: string) => { - const { normalTypes, hiddenType, allTypes } = createTestCases(spaceId); + const { normalTypes, crossNamespace, hiddenType, allTypes } = createTestCases(spaceId); // use singleRequest to reduce execution time and/or test combined cases + const authorizedCommon = [ + createTestDefinitions(normalTypes, false, { singleRequest: true }), + createTestDefinitions(hiddenType, true), + ].flat(); + const crossNamespaceAuthorizedAtSpace = crossNamespace.reduce<{ + authorized: BulkGetTestCase[]; + unauthorized: BulkGetTestCase[]; + }>( + ({ authorized, unauthorized }, cur) => { + // A user who is only authorized in a single space will be authorized to execute some of the cross-namespace test cases, but not all + if (cur.namespaces.some((x) => ![ALL_SPACES_ID, spaceId].includes(x))) { + return { authorized, unauthorized: [...unauthorized, cur] }; + } + return { authorized: [...authorized, cur], unauthorized }; + }, + { authorized: [], unauthorized: [] } + ); + return { unauthorized: createTestDefinitions(allTypes, true), - authorized: [ - createTestDefinitions(normalTypes, false, { singleRequest: true }), - createTestDefinitions(hiddenType, true), + authorizedAtSpace: [ + authorizedCommon, + createTestDefinitions(crossNamespaceAuthorizedAtSpace.authorized, false, { + singleRequest: true, + }), + createTestDefinitions(crossNamespaceAuthorizedAtSpace.unauthorized, true), + createTestDefinitions(allTypes, true, { singleRequest: true }), + ].flat(), + authorizedEverywhere: [ + authorizedCommon, + createTestDefinitions(crossNamespace, false, { singleRequest: true }), createTestDefinitions(allTypes, true, { singleRequest: true, responseBodyOverride: expectSavedObjectForbidden(['hiddentype']), @@ -77,7 +121,9 @@ export default function ({ getService }: FtrProviderContext) { describe('_bulk_get', () => { getTestScenarios().securityAndSpaces.forEach(({ spaceId, users }) => { const suffix = ` within the ${spaceId} space`; - const { unauthorized, authorized, superuser } = createTests(spaceId); + const { unauthorized, authorizedAtSpace, authorizedEverywhere, superuser } = createTests( + spaceId + ); const _addTests = (user: TestUser, tests: BulkGetTestDefinition[]) => { addTests(`${user.description}${suffix}`, { user, spaceId, tests }); }; @@ -85,16 +131,15 @@ export default function ({ getService }: FtrProviderContext) { [users.noAccess, users.legacyAll, users.allAtOtherSpace].forEach((user) => { _addTests(user, unauthorized); }); - [ - users.dualAll, - users.dualRead, - users.allGlobally, - users.readGlobally, - users.allAtSpace, - users.readAtSpace, - ].forEach((user) => { - _addTests(user, authorized); + + [users.allAtSpace, users.readAtSpace].forEach((user) => { + _addTests(user, authorizedAtSpace); + }); + + [users.dualAll, users.dualRead, users.allGlobally, users.readGlobally].forEach((user) => { + _addTests(user, authorizedEverywhere); }); + _addTests(users.superuser, superuser); }); }); diff --git a/x-pack/test/saved_object_api_integration/security_only/apis/bulk_get.ts b/x-pack/test/saved_object_api_integration/security_only/apis/bulk_get.ts index 18edb7502c65a..4aa722bfc6b07 100644 --- a/x-pack/test/saved_object_api_integration/security_only/apis/bulk_get.ts +++ b/x-pack/test/saved_object_api_integration/security_only/apis/bulk_get.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { SPACES, ALL_SPACES_ID } 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'; @@ -14,6 +15,10 @@ import { BulkGetTestDefinition, } from '../../common/suites/bulk_get'; +const { + SPACE_1: { spaceId: SPACE_1_ID }, + SPACE_2: { spaceId: SPACE_2_ID }, +} = SPACES; const { fail400, fail404 } = testCaseFailures; const createTestCases = () => { @@ -31,6 +36,21 @@ const createTestCases = () => { { ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, ...fail404() }, CASES.NAMESPACE_AGNOSTIC, { ...CASES.DOES_NOT_EXIST, ...fail404() }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + namespaces: ['x', 'y'], + ...fail400(), // cannot be searched for in multiple spaces + }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespaces: [SPACE_2_ID] }, // second try searches for it in a single other space, which is valid + { + ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, + namespaces: [ALL_SPACES_ID], + ...fail400(), // cannot be searched for in multiple spaces + }, + { ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, namespaces: [SPACE_1_ID] }, // second try searches for it in a single other space, which is valid + { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespaces: [SPACE_2_ID], ...fail404() }, + { ...CASES.MULTI_NAMESPACE_ALL_SPACES, namespaces: [SPACE_2_ID, 'x'] }, // unknown space is allowed / ignored + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, namespaces: [ALL_SPACES_ID] }, ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; const allTypes = normalTypes.concat(hiddenType); diff --git a/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_get.ts b/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_get.ts index e1d0243377b8e..41fa4749cc48e 100644 --- a/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_get.ts +++ b/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_get.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { SPACES } from '../../common/lib/spaces'; +import { SPACES, ALL_SPACES_ID } from '../../common/lib/spaces'; import { testCaseFailures, getTestScenarios } from '../../common/lib/saved_object_test_utils'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { bulkGetTestSuiteFactory, TEST_CASES as CASES } from '../../common/suites/bulk_get'; @@ -38,6 +38,21 @@ const createTestCases = (spaceId: string) => [ CASES.NAMESPACE_AGNOSTIC, { ...CASES.HIDDEN, ...fail400() }, { ...CASES.DOES_NOT_EXIST, ...fail404() }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + namespaces: ['x', 'y'], + ...fail400(), // cannot be searched for in multiple spaces + }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespaces: [SPACE_2_ID] }, // second try searches for it in a single other space, which is valid + { + ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, + namespaces: [ALL_SPACES_ID], + ...fail400(), // cannot be searched for in multiple spaces + }, + { ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, namespaces: [SPACE_1_ID] }, // second try searches for it in a single other space, which is valid + { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespaces: [SPACE_2_ID], ...fail404() }, + { ...CASES.MULTI_NAMESPACE_ALL_SPACES, namespaces: [SPACE_2_ID, 'x'] }, // unknown space is allowed / ignored + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, namespaces: [ALL_SPACES_ID] }, ]; export default function ({ getService }: FtrProviderContext) {