From d666038c8f6767f6790cb78f29e0027ff73d83ee Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Tue, 22 Sep 2020 12:40:38 -0400 Subject: [PATCH 1/3] Change saved objects client `find` to allow partial authorization (#77699) --- ...gin-core-public.savedobjectsfindoptions.md | 1 + ...dobjectsfindoptions.typetonamespacesmap.md | 13 ++ ...gin-core-server.savedobjectsfindoptions.md | 1 + ...dobjectsfindoptions.typetonamespacesmap.md | 13 ++ ...core-server.savedobjectsrepository.find.md | 4 +- ...ugin-core-server.savedobjectsrepository.md | 2 +- ...vedobjectsutils.createemptyfindresponse.md | 13 ++ ...na-plugin-core-server.savedobjectsutils.md | 1 + src/core/public/public.api.md | 1 + .../saved_objects/saved_objects_client.ts | 2 +- .../service/lib/repository.test.js | 93 +++++++-- .../saved_objects/service/lib/repository.ts | 65 ++++--- .../lib/search_dsl/query_params.test.ts | 99 ++++++---- .../service/lib/search_dsl/query_params.ts | 25 ++- .../service/lib/search_dsl/search_dsl.test.ts | 4 +- .../service/lib/search_dsl/search_dsl.ts | 3 + .../saved_objects/service/lib/utils.test.ts | 25 ++- .../server/saved_objects/service/lib/utils.ts | 18 ++ src/core/server/saved_objects/types.ts | 8 + src/core/server/server.api.md | 4 +- ...ecure_saved_objects_client_wrapper.test.ts | 75 ++++++- .../secure_saved_objects_client_wrapper.ts | 184 +++++++++++++----- .../server/lib/spaces_client/spaces_client.ts | 2 +- .../spaces_saved_objects_client.test.ts | 30 +++ .../spaces_saved_objects_client.ts | 37 ++-- .../common/lib/saved_object_test_cases.ts | 54 ++++- .../common/lib/saved_object_test_utils.ts | 45 ++--- .../common/lib/types.ts | 1 + .../common/suites/bulk_create.ts | 16 +- .../common/suites/bulk_get.ts | 5 +- .../common/suites/bulk_update.ts | 5 +- .../common/suites/create.ts | 13 +- .../common/suites/delete.ts | 5 +- .../common/suites/export.ts | 61 +++--- .../common/suites/find.ts | 166 ++++++---------- .../common/suites/get.ts | 2 +- .../common/suites/import.ts | 2 +- .../common/suites/resolve_import_errors.ts | 2 +- .../common/suites/update.ts | 5 +- .../security_and_spaces/apis/bulk_create.ts | 36 +++- .../security_and_spaces/apis/create.ts | 32 ++- .../security_and_spaces/apis/export.ts | 32 ++- .../security_and_spaces/apis/find.ts | 119 ++++++----- .../security_only/apis/bulk_create.ts | 28 ++- .../security_only/apis/create.ts | 27 ++- .../security_only/apis/export.ts | 32 ++- .../security_only/apis/find.ts | 44 ++--- .../spaces_only/apis/bulk_create.ts | 68 ++++--- .../spaces_only/apis/create.ts | 50 +++-- .../spaces_only/apis/find.ts | 19 +- 50 files changed, 1067 insertions(+), 525 deletions(-) create mode 100644 docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.typetonamespacesmap.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.typetonamespacesmap.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.createemptyfindresponse.md diff --git a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md index 903462ac3039d..470a41f30afbf 100644 --- a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md +++ b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.md @@ -29,4 +29,5 @@ export interface SavedObjectsFindOptions | [sortField](./kibana-plugin-core-public.savedobjectsfindoptions.sortfield.md) | string | | | [sortOrder](./kibana-plugin-core-public.savedobjectsfindoptions.sortorder.md) | string | | | [type](./kibana-plugin-core-public.savedobjectsfindoptions.type.md) | string | string[] | | +| [typeToNamespacesMap](./kibana-plugin-core-public.savedobjectsfindoptions.typetonamespacesmap.md) | Map<string, string[] | undefined> | This map defines each type to search for, and the namespace(s) to search for the type in; this is only intended to be used by a saved object client wrapper. If this is defined, it supersedes the type and namespaces fields when building the Elasticsearch query. Any types that are not included in this map will be excluded entirely. If a type is included but its value is undefined, the operation will search for that type in the Default namespace. | diff --git a/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.typetonamespacesmap.md b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.typetonamespacesmap.md new file mode 100644 index 0000000000000..4af8c9ddeaff4 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-core-public.savedobjectsfindoptions.typetonamespacesmap.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-public](./kibana-plugin-core-public.md) > [SavedObjectsFindOptions](./kibana-plugin-core-public.savedobjectsfindoptions.md) > [typeToNamespacesMap](./kibana-plugin-core-public.savedobjectsfindoptions.typetonamespacesmap.md) + +## SavedObjectsFindOptions.typeToNamespacesMap property + +This map defines each type to search for, and the namespace(s) to search for the type in; this is only intended to be used by a saved object client wrapper. If this is defined, it supersedes the `type` and `namespaces` fields when building the Elasticsearch query. Any types that are not included in this map will be excluded entirely. If a type is included but its value is undefined, the operation will search for that type in the Default namespace. + +Signature: + +```typescript +typeToNamespacesMap?: Map; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md index 804c83f7c1b48..ce5c20e60ca11 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.md @@ -29,4 +29,5 @@ export interface SavedObjectsFindOptions | [sortField](./kibana-plugin-core-server.savedobjectsfindoptions.sortfield.md) | string | | | [sortOrder](./kibana-plugin-core-server.savedobjectsfindoptions.sortorder.md) | string | | | [type](./kibana-plugin-core-server.savedobjectsfindoptions.type.md) | string | string[] | | +| [typeToNamespacesMap](./kibana-plugin-core-server.savedobjectsfindoptions.typetonamespacesmap.md) | Map<string, string[] | undefined> | This map defines each type to search for, and the namespace(s) to search for the type in; this is only intended to be used by a saved object client wrapper. If this is defined, it supersedes the type and namespaces fields when building the Elasticsearch query. Any types that are not included in this map will be excluded entirely. If a type is included but its value is undefined, the operation will search for that type in the Default namespace. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.typetonamespacesmap.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.typetonamespacesmap.md new file mode 100644 index 0000000000000..8bec759f05580 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsfindoptions.typetonamespacesmap.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsFindOptions](./kibana-plugin-core-server.savedobjectsfindoptions.md) > [typeToNamespacesMap](./kibana-plugin-core-server.savedobjectsfindoptions.typetonamespacesmap.md) + +## SavedObjectsFindOptions.typeToNamespacesMap property + +This map defines each type to search for, and the namespace(s) to search for the type in; this is only intended to be used by a saved object client wrapper. If this is defined, it supersedes the `type` and `namespaces` fields when building the Elasticsearch query. Any types that are not included in this map will be excluded entirely. If a type is included but its value is undefined, the operation will search for that type in the Default namespace. + +Signature: + +```typescript +typeToNamespacesMap?: Map; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.find.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.find.md index 1b562263145da..d3e93e7af2aa0 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.find.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.find.md @@ -7,14 +7,14 @@ Signature: ```typescript -find({ search, defaultSearchOperator, searchFields, rootSearchFields, hasReference, page, perPage, sortField, sortOrder, fields, namespaces, type, filter, preference, }: SavedObjectsFindOptions): Promise>; +find(options: SavedObjectsFindOptions): Promise>; ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| { search, defaultSearchOperator, searchFields, rootSearchFields, hasReference, page, perPage, sortField, sortOrder, fields, namespaces, type, filter, preference, } | SavedObjectsFindOptions | | +| options | SavedObjectsFindOptions | | Returns: diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.md index 14d3741425987..1d11d5262a9c4 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.md @@ -24,7 +24,7 @@ export declare class SavedObjectsRepository | [delete(type, id, options)](./kibana-plugin-core-server.savedobjectsrepository.delete.md) | | Deletes an object | | [deleteByNamespace(namespace, options)](./kibana-plugin-core-server.savedobjectsrepository.deletebynamespace.md) | | Deletes all objects from the provided namespace. | | [deleteFromNamespaces(type, id, namespaces, options)](./kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md) | | Removes one or more namespaces from a given multi-namespace saved object. If no namespaces remain, the saved object is deleted entirely. This method and \[addToNamespaces\][SavedObjectsRepository.addToNamespaces()](./kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md) are the only ways to change which Spaces a multi-namespace saved object is shared to. | -| [find({ search, defaultSearchOperator, searchFields, rootSearchFields, hasReference, page, perPage, sortField, sortOrder, fields, namespaces, type, filter, preference, })](./kibana-plugin-core-server.savedobjectsrepository.find.md) | | | +| [find(options)](./kibana-plugin-core-server.savedobjectsrepository.find.md) | | | | [get(type, id, options)](./kibana-plugin-core-server.savedobjectsrepository.get.md) | | Gets a single object | | [incrementCounter(type, id, counterFieldName, options)](./kibana-plugin-core-server.savedobjectsrepository.incrementcounter.md) | | Increases a counter field by one. Creates the document if one doesn't exist for the given id. | | [update(type, id, attributes, options)](./kibana-plugin-core-server.savedobjectsrepository.update.md) | | Updates an object | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.createemptyfindresponse.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.createemptyfindresponse.md new file mode 100644 index 0000000000000..40e865cb02ce8 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.createemptyfindresponse.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsUtils](./kibana-plugin-core-server.savedobjectsutils.md) > [createEmptyFindResponse](./kibana-plugin-core-server.savedobjectsutils.createemptyfindresponse.md) + +## SavedObjectsUtils.createEmptyFindResponse property + +Creates an empty response for a find operation. This is only intended to be used by saved objects client wrappers. + +Signature: + +```typescript +static createEmptyFindResponse: ({ page, perPage, }: SavedObjectsFindOptions) => SavedObjectsFindResponse; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md index e365dfbcb5142..83831f65bd41a 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsutils.md @@ -15,6 +15,7 @@ export declare class SavedObjectsUtils | Property | Modifiers | Type | Description | | --- | --- | --- | --- | +| [createEmptyFindResponse](./kibana-plugin-core-server.savedobjectsutils.createemptyfindresponse.md) | static | <T>({ page, perPage, }: SavedObjectsFindOptions) => SavedObjectsFindResponse<T> | Creates an empty response for a find operation. This is only intended to be used by saved objects client wrappers. | | [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/src/core/public/public.api.md b/src/core/public/public.api.md index 1c17be50454c5..7179c6cf8b133 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -1079,6 +1079,7 @@ export interface SavedObjectsFindOptions { sortOrder?: string; // (undocumented) type: string | string[]; + typeToNamespacesMap?: Map; } // @public diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index 5a8949ca2f55f..6a10eb44d9ca4 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -34,7 +34,7 @@ import { HttpFetchOptions, HttpSetup } from '../http'; type SavedObjectsFindOptions = Omit< SavedObjectFindOptionsServer, - 'namespace' | 'sortOrder' | 'rootSearchFields' + 'sortOrder' | 'rootSearchFields' | 'typeToNamespacesMap' >; type PromiseType> = T extends Promise ? U : never; 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 352ce4c1c16eb..0e72ad2fec06c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -2477,6 +2477,33 @@ describe('SavedObjectsRepository', () => { expect(client.search).not.toHaveBeenCalled(); }); + it(`throws when namespaces is an empty array`, async () => { + await expect( + savedObjectsRepository.find({ type: 'foo', namespaces: [] }) + ).rejects.toThrowError('options.namespaces cannot be an empty array'); + expect(client.search).not.toHaveBeenCalled(); + }); + + it(`throws when type is not falsy and typeToNamespacesMap is defined`, async () => { + await expect( + savedObjectsRepository.find({ type: 'foo', typeToNamespacesMap: new Map() }) + ).rejects.toThrowError( + 'options.type must be an empty string when options.typeToNamespacesMap is used' + ); + expect(client.search).not.toHaveBeenCalled(); + }); + + it(`throws when type is not an empty array and typeToNamespacesMap is defined`, async () => { + const test = async (args) => { + await expect(savedObjectsRepository.find(args)).rejects.toThrowError( + 'options.namespaces must be an empty array when options.typeToNamespacesMap is used' + ); + expect(client.search).not.toHaveBeenCalled(); + }; + await test({ type: '', typeToNamespacesMap: new Map() }); + await test({ type: '', namespaces: ['some-ns'], typeToNamespacesMap: new Map() }); + }); + it(`throws when searchFields is defined but not an array`, async () => { await expect( savedObjectsRepository.find({ type, searchFields: 'string' }) @@ -2493,7 +2520,7 @@ describe('SavedObjectsRepository', () => { it(`throws when KQL filter syntax is invalid`, async () => { const findOpts = { - namespace, + namespaces: [namespace], search: 'foo*', searchFields: ['foo'], type: ['dashboard'], @@ -2577,38 +2604,70 @@ describe('SavedObjectsRepository', () => { const test = async (types) => { const result = await savedObjectsRepository.find({ type: types }); expect(result).toEqual(expect.objectContaining({ saved_objects: [] })); + expect(client.search).not.toHaveBeenCalled(); }; await test('unknownType'); await test(HIDDEN_TYPE); await test(['unknownType', HIDDEN_TYPE]); }); + + it(`should return empty results when attempting to find only invalid or hidden types using typeToNamespacesMap`, async () => { + const test = async (types) => { + const result = await savedObjectsRepository.find({ + typeToNamespacesMap: new Map(types.map((x) => [x, undefined])), + type: '', + namespaces: [], + }); + expect(result).toEqual(expect.objectContaining({ saved_objects: [] })); + expect(client.search).not.toHaveBeenCalled(); + }; + + await test(['unknownType']); + await test([HIDDEN_TYPE]); + await test(['unknownType', HIDDEN_TYPE]); + }); }); describe('search dsl', () => { - it(`passes mappings, registry, search, defaultSearchOperator, searchFields, type, sortField, sortOrder and hasReference to getSearchDsl`, async () => { + const commonOptions = { + type: [type], // cannot be used when `typeToNamespacesMap` is present + namespaces: [namespace], // cannot be used when `typeToNamespacesMap` is present + search: 'foo*', + searchFields: ['foo'], + sortField: 'name', + sortOrder: 'desc', + defaultSearchOperator: 'AND', + hasReference: { + type: 'foo', + id: '1', + }, + kueryNode: undefined, + }; + + it(`passes mappings, registry, and search options to getSearchDsl`, async () => { + await findSuccess(commonOptions, namespace); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, commonOptions); + }); + + it(`accepts typeToNamespacesMap`, async () => { const relevantOpts = { - namespaces: [namespace], - search: 'foo*', - searchFields: ['foo'], - type: [type], - sortField: 'name', - sortOrder: 'desc', - defaultSearchOperator: 'AND', - hasReference: { - type: 'foo', - id: '1', - }, - kueryNode: undefined, + ...commonOptions, + type: '', + namespaces: [], + typeToNamespacesMap: new Map([[type, [namespace]]]), // can only be used when `type` is falsy and `namespaces` is an empty array }; await findSuccess(relevantOpts, namespace); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, relevantOpts); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + ...relevantOpts, + type: [type], + }); }); it(`accepts KQL expression filter and passes KueryNode to getSearchDsl`, async () => { const findOpts = { - namespace, + namespaces: [namespace], search: 'foo*', searchFields: ['foo'], type: ['dashboard'], @@ -2649,7 +2708,7 @@ describe('SavedObjectsRepository', () => { it(`accepts KQL KueryNode filter and passes KueryNode to getSearchDsl`, async () => { const findOpts = { - namespace, + namespaces: [namespace], search: 'foo*', searchFields: ['foo'], type: ['dashboard'], diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 125f97e7feb11..a83c86e585628 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -67,7 +67,7 @@ import { } from '../../types'; import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { validateConvertFilterToKueryNode } from './filter_utils'; -import { SavedObjectsUtils } from './utils'; +import { FIND_DEFAULT_PAGE, FIND_DEFAULT_PER_PAGE, 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. @@ -693,37 +693,51 @@ export class SavedObjectsRepository { * @property {string} [options.preference] * @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page } */ - async find({ - search, - defaultSearchOperator = 'OR', - searchFields, - rootSearchFields, - hasReference, - page = 1, - perPage = 20, - sortField, - sortOrder, - fields, - namespaces, - type, - filter, - preference, - }: SavedObjectsFindOptions): Promise> { - if (!type) { + async find(options: SavedObjectsFindOptions): Promise> { + const { + search, + defaultSearchOperator = 'OR', + searchFields, + rootSearchFields, + hasReference, + page = FIND_DEFAULT_PAGE, + perPage = FIND_DEFAULT_PER_PAGE, + sortField, + sortOrder, + fields, + namespaces, + type, + typeToNamespacesMap, + filter, + preference, + } = options; + + if (!type && !typeToNamespacesMap) { throw SavedObjectsErrorHelpers.createBadRequestError( 'options.type must be a string or an array of strings' ); + } else if (namespaces?.length === 0 && !typeToNamespacesMap) { + throw SavedObjectsErrorHelpers.createBadRequestError( + 'options.namespaces cannot be an empty array' + ); + } else if (type && typeToNamespacesMap) { + throw SavedObjectsErrorHelpers.createBadRequestError( + 'options.type must be an empty string when options.typeToNamespacesMap is used' + ); + } else if ((!namespaces || namespaces?.length) && typeToNamespacesMap) { + throw SavedObjectsErrorHelpers.createBadRequestError( + 'options.namespaces must be an empty array when options.typeToNamespacesMap is used' + ); } - const types = Array.isArray(type) ? type : [type]; + const types = type + ? Array.isArray(type) + ? type + : [type] + : Array.from(typeToNamespacesMap!.keys()); const allowedTypes = types.filter((t) => this._allowedTypes.includes(t)); if (allowedTypes.length === 0) { - return { - page, - per_page: perPage, - total: 0, - saved_objects: [], - }; + return SavedObjectsUtils.createEmptyFindResponse(options); } if (searchFields && !Array.isArray(searchFields)) { @@ -766,6 +780,7 @@ export class SavedObjectsRepository { sortField, sortOrder, namespaces, + typeToNamespacesMap, hasReference, kueryNode, }), diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index 4adc92df31805..e13c67a720400 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -50,6 +50,40 @@ const ALL_TYPE_SUBSETS = ALL_TYPES.reduce( .filter((x) => x.length) // exclude empty set .map((x) => (x.length === 1 ? x[0] : x)); // if a subset is a single string, destructure it +const createTypeClause = (type: string, namespaces?: string[]) => { + if (registry.isMultiNamespace(type)) { + return { + bool: { + must: expect.arrayContaining([{ terms: { namespaces: namespaces ?? ['default'] } }]), + must_not: [{ exists: { field: 'namespace' } }], + }, + }; + } else if (registry.isSingleNamespace(type)) { + const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; + const should: any = []; + if (nonDefaultNamespaces.length > 0) { + should.push({ terms: { namespace: nonDefaultNamespaces } }); + } + if (namespaces?.includes('default')) { + should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); + } + return { + bool: { + must: [{ term: { type } }], + should: expect.arrayContaining(should), + minimum_should_match: 1, + must_not: [{ exists: { field: 'namespaces' } }], + }, + }; + } + // isNamespaceAgnostic + return { + bool: expect.objectContaining({ + must_not: [{ exists: { field: 'namespace' } }, { exists: { field: 'namespaces' } }], + }), + }; +}; + /** * Note: these tests cases are defined in the order they appear in the source code, for readability's sake */ @@ -198,40 +232,6 @@ describe('#getQueryParams', () => { }); describe('`namespaces` parameter', () => { - const createTypeClause = (type: string, namespaces?: string[]) => { - if (registry.isMultiNamespace(type)) { - return { - bool: { - must: expect.arrayContaining([{ terms: { namespaces: namespaces ?? ['default'] } }]), - must_not: [{ exists: { field: 'namespace' } }], - }, - }; - } else if (registry.isSingleNamespace(type)) { - const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; - const should: any = []; - if (nonDefaultNamespaces.length > 0) { - should.push({ terms: { namespace: nonDefaultNamespaces } }); - } - if (namespaces?.includes('default')) { - should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); - } - return { - bool: { - must: [{ term: { type } }], - should: expect.arrayContaining(should), - minimum_should_match: 1, - must_not: [{ exists: { field: 'namespaces' } }], - }, - }; - } - // isNamespaceAgnostic - return { - bool: expect.objectContaining({ - must_not: [{ exists: { field: 'namespace' } }, { exists: { field: 'namespaces' } }], - }), - }; - }; - const expectResult = (result: Result, ...typeClauses: any) => { expect(result.query.bool.filter).toEqual( expect.arrayContaining([ @@ -281,6 +281,37 @@ describe('#getQueryParams', () => { test(['default']); }); }); + + describe('`typeToNamespacesMap` parameter', () => { + const expectResult = (result: Result, ...typeClauses: any) => { + expect(result.query.bool.filter).toEqual( + expect.arrayContaining([ + { bool: expect.objectContaining({ should: typeClauses, minimum_should_match: 1 }) }, + ]) + ); + }; + + it('supersedes `type` and `namespaces` parameters', () => { + const result = getQueryParams({ + mappings, + registry, + type: ['pending', 'saved', 'shared', 'global'], + namespaces: ['foo', 'bar', 'default'], + typeToNamespacesMap: new Map([ + ['pending', ['foo']], // 'pending' is only authorized in the 'foo' namespace + // 'saved' is not authorized in any namespaces + ['shared', ['bar', 'default']], // 'shared' is only authorized in the 'bar' and 'default' namespaces + ['global', ['foo', 'bar', 'default']], // 'global' is authorized in all namespaces (which are ignored anyway) + ]), + }); + expectResult( + result, + createTypeClause('pending', ['foo']), + createTypeClause('shared', ['bar', 'default']), + createTypeClause('global') + ); + }); + }); }); describe('search clause (query.bool.must.simple_query_string)', () => { 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 642d51c70766e..eaddc05fa921c 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 @@ -129,6 +129,7 @@ interface QueryParams { registry: ISavedObjectTypeRegistry; namespaces?: string[]; type?: string | string[]; + typeToNamespacesMap?: Map; search?: string; searchFields?: string[]; rootSearchFields?: string[]; @@ -145,6 +146,7 @@ export function getQueryParams({ registry, namespaces, type, + typeToNamespacesMap, search, searchFields, rootSearchFields, @@ -152,7 +154,10 @@ export function getQueryParams({ hasReference, kueryNode, }: QueryParams) { - const types = getTypes(mappings, type); + const types = getTypes( + mappings, + typeToNamespacesMap ? Array.from(typeToNamespacesMap.keys()) : type + ); // A de-duplicated set of namespaces makes for a more effecient query. // @@ -163,9 +168,12 @@ export function getQueryParams({ // since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place // 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((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x)))) - : undefined; + const normalizeNamespaces = (namespacesToNormalize?: string[]) => + namespacesToNormalize + ? Array.from( + new Set(namespacesToNormalize.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x))) + ) + : undefined; const bool: any = { filter: [ @@ -197,9 +205,12 @@ export function getQueryParams({ }, ] : undefined, - should: types.map((shouldType) => - getClauseForType(registry, normalizedNamespaces, shouldType) - ), + should: types.map((shouldType) => { + const normalizedNamespaces = normalizeNamespaces( + typeToNamespacesMap ? typeToNamespacesMap.get(shouldType) : namespaces + ); + return getClauseForType(registry, normalizedNamespaces, shouldType); + }), minimum_should_match: 1, }, }, diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts index 62e629ad33cc8..7276e505bce7d 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts @@ -57,10 +57,11 @@ describe('getSearchDsl', () => { }); describe('passes control', () => { - it('passes (mappings, schema, namespaces, type, search, searchFields, rootSearchFields, hasReference) to getQueryParams', () => { + it('passes (mappings, schema, namespaces, type, typeToNamespacesMap, search, searchFields, rootSearchFields, hasReference) to getQueryParams', () => { const opts = { namespaces: ['foo-namespace'], type: 'foo', + typeToNamespacesMap: new Map(), search: 'bar', searchFields: ['baz'], rootSearchFields: ['qux'], @@ -78,6 +79,7 @@ describe('getSearchDsl', () => { registry, namespaces: opts.namespaces, type: opts.type, + typeToNamespacesMap: opts.typeToNamespacesMap, search: opts.search, searchFields: opts.searchFields, rootSearchFields: opts.rootSearchFields, diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index aa79a10b2a9be..858770579fb9e 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -35,6 +35,7 @@ interface GetSearchDslOptions { sortField?: string; sortOrder?: string; namespaces?: string[]; + typeToNamespacesMap?: Map; hasReference?: { type: string; id: string; @@ -56,6 +57,7 @@ export function getSearchDsl( sortField, sortOrder, namespaces, + typeToNamespacesMap, hasReference, kueryNode, } = options; @@ -74,6 +76,7 @@ export function getSearchDsl( registry, namespaces, type, + typeToNamespacesMap, search, searchFields, rootSearchFields, diff --git a/src/core/server/saved_objects/service/lib/utils.test.ts b/src/core/server/saved_objects/service/lib/utils.test.ts index ea4fa68242bea..ac06ca9275783 100644 --- a/src/core/server/saved_objects/service/lib/utils.test.ts +++ b/src/core/server/saved_objects/service/lib/utils.test.ts @@ -17,10 +17,11 @@ * under the License. */ +import { SavedObjectsFindOptions } from '../../types'; import { SavedObjectsUtils } from './utils'; describe('SavedObjectsUtils', () => { - const { namespaceIdToString, namespaceStringToId } = SavedObjectsUtils; + const { namespaceIdToString, namespaceStringToId, createEmptyFindResponse } = SavedObjectsUtils; describe('#namespaceIdToString', () => { it('converts `undefined` to default namespace string', () => { @@ -54,4 +55,26 @@ describe('SavedObjectsUtils', () => { test(''); }); }); + + describe('#createEmptyFindResponse', () => { + it('returns expected result', () => { + const options = {} as SavedObjectsFindOptions; + expect(createEmptyFindResponse(options)).toEqual({ + page: 1, + per_page: 20, + total: 0, + saved_objects: [], + }); + }); + + it('handles `page` field', () => { + const options = { page: 42 } as SavedObjectsFindOptions; + expect(createEmptyFindResponse(options).page).toEqual(42); + }); + + it('handles `perPage` field', () => { + const options = { perPage: 42 } as SavedObjectsFindOptions; + expect(createEmptyFindResponse(options).per_page).toEqual(42); + }); + }); }); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 6101ad57cc401..3efe8614da1d7 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -17,7 +17,12 @@ * under the License. */ +import { SavedObjectsFindOptions } from '../../types'; +import { SavedObjectsFindResponse } from '..'; + export const DEFAULT_NAMESPACE_STRING = 'default'; +export const FIND_DEFAULT_PAGE = 1; +export const FIND_DEFAULT_PER_PAGE = 20; /** * @public @@ -50,4 +55,17 @@ export class SavedObjectsUtils { return namespace !== DEFAULT_NAMESPACE_STRING ? namespace : undefined; }; + + /** + * Creates an empty response for a find operation. This is only intended to be used by saved objects client wrappers. + */ + public static createEmptyFindResponse = ({ + page = FIND_DEFAULT_PAGE, + perPage = FIND_DEFAULT_PER_PAGE, + }: SavedObjectsFindOptions): SavedObjectsFindResponse => ({ + page, + per_page: perPage, + total: 0, + saved_objects: [], + }); } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 1885f5ec50139..01128e4f8cf51 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -89,6 +89,14 @@ export interface SavedObjectsFindOptions { defaultSearchOperator?: 'AND' | 'OR'; filter?: string | KueryNode; namespaces?: string[]; + /** + * This map defines each type to search for, and the namespace(s) to search for the type in; this is only intended to be used by a saved + * object client wrapper. + * If this is defined, it supersedes the `type` and `namespaces` fields when building the Elasticsearch query. + * Any types that are not included in this map will be excluded entirely. + * If a type is included but its value is undefined, the operation will search for that type in the Default namespace. + */ + typeToNamespacesMap?: Map; /** An optional ES preference value to be used for the query **/ preference?: string; } diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index d755ef3e1b676..8a764d9bd2f66 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2177,6 +2177,7 @@ export interface SavedObjectsFindOptions { sortOrder?: string; // (undocumented) type: string | string[]; + typeToNamespacesMap?: Map; } // @public @@ -2388,7 +2389,7 @@ export class SavedObjectsRepository { deleteByNamespace(namespace: string, options?: SavedObjectsDeleteByNamespaceOptions): Promise; deleteFromNamespaces(type: string, id: string, namespaces: string[], options?: SavedObjectsDeleteFromNamespacesOptions): Promise; // (undocumented) - find({ search, defaultSearchOperator, searchFields, rootSearchFields, hasReference, page, perPage, sortField, sortOrder, fields, namespaces, type, filter, preference, }: SavedObjectsFindOptions): Promise>; + find(options: SavedObjectsFindOptions): Promise>; get(type: string, id: string, options?: SavedObjectsBaseOptions): Promise>; incrementCounter(type: string, id: string, counterFieldName: string, options?: SavedObjectsIncrementCounterOptions): Promise; update(type: string, id: string, attributes: Partial, options?: SavedObjectsUpdateOptions): Promise>; @@ -2496,6 +2497,7 @@ export interface SavedObjectsUpdateResponse extends Omit({ page, perPage, }: SavedObjectsFindOptions) => SavedObjectsFindResponse; static namespaceIdToString: (namespace?: string | undefined) => string; static namespaceStringToId: (namespace: string) => string | undefined; } 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 7ada34ff5ccac..86d1b68ba761e 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 @@ -609,22 +609,83 @@ describe('#find', () => { await expectGeneralError(client.find, { type: type1 }); }); - test(`throws decorated ForbiddenError when type's singular and unauthorized`, async () => { + test(`returns empty result when unauthorized`, async () => { + clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockImplementation( + getMockCheckPrivilegesFailure + ); + const options = Object.freeze({ type: type1, namespaces: ['some-ns'] }); - await expectForbiddenError(client.find, { options }); - }); + const result = await client.find(options); - test(`throws decorated ForbiddenError when type's an array and unauthorized`, async () => { - const options = Object.freeze({ type: [type1, type2], namespaces: ['some-ns'] }); - await expectForbiddenError(client.find, { options }); + expect(clientOpts.baseClient.find).not.toHaveBeenCalled(); + expect(clientOpts.auditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledTimes(1); + expect(clientOpts.auditLogger.savedObjectsAuthorizationFailure).toHaveBeenCalledWith( + USERNAME, + 'find', + [type1], + options.namespaces, + [{ spaceId: 'some-ns', privilege: 'mock-saved_object:foo/find' }], + { options } + ); + expect(result).toEqual({ page: 1, per_page: 20, total: 0, saved_objects: [] }); }); - test(`returns result of baseClient.find when authorized`, async () => { + test(`returns result of baseClient.find when fully authorized`, async () => { const apiCallReturnValue = { saved_objects: [], foo: 'bar' }; clientOpts.baseClient.find.mockReturnValue(apiCallReturnValue as any); const options = Object.freeze({ type: type1, namespaces: ['some-ns'] }); const result = await expectSuccess(client.find, { options }); + expect(clientOpts.baseClient.find.mock.calls[0][0]).toEqual({ + ...options, + typeToNamespacesMap: undefined, + }); + expect(result).toEqual(apiCallReturnValue); + }); + + test(`returns result of baseClient.find when partially authorized`, async () => { + clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockResolvedValue({ + hasAllRequested: false, + username: USERNAME, + privileges: { + kibana: [ + { resource: 'some-ns', privilege: 'mock-saved_object:foo/find', authorized: true }, + { resource: 'some-ns', privilege: 'mock-saved_object:bar/find', authorized: true }, + { resource: 'some-ns', privilege: 'mock-saved_object:baz/find', authorized: false }, + { resource: 'some-ns', privilege: 'mock-saved_object:qux/find', authorized: false }, + { resource: 'another-ns', privilege: 'mock-saved_object:foo/find', authorized: true }, + { resource: 'another-ns', privilege: 'mock-saved_object:bar/find', authorized: false }, + { resource: 'another-ns', privilege: 'mock-saved_object:baz/find', authorized: true }, + { resource: 'another-ns', privilege: 'mock-saved_object:qux/find', authorized: false }, + { resource: 'forbidden-ns', privilege: 'mock-saved_object:foo/find', authorized: false }, + { resource: 'forbidden-ns', privilege: 'mock-saved_object:bar/find', authorized: false }, + { resource: 'forbidden-ns', privilege: 'mock-saved_object:baz/find', authorized: false }, + { resource: 'forbidden-ns', privilege: 'mock-saved_object:qux/find', authorized: false }, + ], + }, + }); + + const apiCallReturnValue = { saved_objects: [], foo: 'bar' }; + clientOpts.baseClient.find.mockReturnValue(apiCallReturnValue as any); + + const options = Object.freeze({ + type: ['foo', 'bar', 'baz', 'qux'], + namespaces: ['some-ns', 'another-ns', 'forbidden-ns'], + }); + const result = await client.find(options); + // 'expect(clientOpts.baseClient.find).toHaveBeenCalledWith' resulted in false negatives, resorting to manually comparing mock call args + expect(clientOpts.baseClient.find.mock.calls[0][0]).toEqual({ + ...options, + typeToNamespacesMap: new Map([ + ['foo', ['some-ns', 'another-ns']], + ['bar', ['some-ns']], + ['baz', ['another-ns']], + // qux is not authorized, so there is no entry for it + // forbidden-ns is completely forbidden, so there are no entries with this namespace + ]), + type: '', + namespaces: [], + }); expect(result).toEqual(apiCallReturnValue); }); 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 16e52c69f274f..f5de8f4b226f3 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 @@ -16,6 +16,7 @@ import { SavedObjectsUpdateOptions, SavedObjectsAddToNamespacesOptions, SavedObjectsDeleteFromNamespacesOptions, + SavedObjectsUtils, } from '../../../../../src/core/server'; import { SecurityAuditLogger } from '../audit'; import { Actions, CheckSavedObjectsPrivileges } from '../authorization'; @@ -39,8 +40,19 @@ interface SavedObjectsNamespaces { saved_objects: SavedObjectNamespaces[]; } -function uniq(arr: T[]): T[] { - return Array.from(new Set(arr)); +interface EnsureAuthorizedOptions { + args?: Record; + auditAction?: string; + requireFullAuthorization?: boolean; +} + +interface EnsureAuthorizedResult { + status: 'fully_authorized' | 'partially_authorized' | 'unauthorized'; + typeMap: Map; +} +interface EnsureAuthorizedTypeResult { + authorizedSpaces: string[]; + isGloballyAuthorized?: boolean; } export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContract { @@ -72,7 +84,8 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra attributes: T = {} as T, options: SavedObjectsCreateOptions = {} ) { - await this.ensureAuthorized(type, 'create', options.namespace, { type, attributes, options }); + const args = { type, attributes, options }; + await this.ensureAuthorized(type, 'create', options.namespace, { args }); const savedObject = await this.baseClient.create(type, attributes, options); return await this.redactSavedObjectNamespaces(savedObject); @@ -82,9 +95,12 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra objects: SavedObjectsCheckConflictsObject[] = [], options: SavedObjectsBaseOptions = {} ) { - const types = this.getUniqueObjectTypes(objects); const args = { objects, options }; - await this.ensureAuthorized(types, 'bulk_create', options.namespace, args, 'checkConflicts'); + const types = this.getUniqueObjectTypes(objects); + await this.ensureAuthorized(types, 'bulk_create', options.namespace, { + args, + auditAction: 'checkConflicts', + }); const response = await this.baseClient.checkConflicts(objects, options); return response; @@ -94,11 +110,12 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra objects: Array>, options: SavedObjectsBaseOptions = {} ) { + const args = { objects, options }; await this.ensureAuthorized( this.getUniqueObjectTypes(objects), 'bulk_create', options.namespace, - { objects, options } + { args } ); const response = await this.baseClient.bulkCreate(objects, options); @@ -106,7 +123,8 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra } public async delete(type: string, id: string, options: SavedObjectsBaseOptions = {}) { - await this.ensureAuthorized(type, 'delete', options.namespace, { type, id, options }); + const args = { type, id, options }; + await this.ensureAuthorized(type, 'delete', options.namespace, { args }); return await this.baseClient.delete(type, id, options); } @@ -121,9 +139,29 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra `_find across namespaces is not permitted when the Spaces plugin is disabled.` ); } - await this.ensureAuthorized(options.type, 'find', options.namespaces, { options }); + const args = { options }; + const { status, typeMap } = await this.ensureAuthorized( + options.type, + 'find', + options.namespaces, + { args, requireFullAuthorization: false } + ); + + if (status === 'unauthorized') { + // return empty response + return SavedObjectsUtils.createEmptyFindResponse(options); + } - const response = await this.baseClient.find(options); + const typeToNamespacesMap = Array.from(typeMap).reduce>( + (acc, [type, { authorizedSpaces, isGloballyAuthorized }]) => + isGloballyAuthorized ? acc.set(type, options.namespaces) : acc.set(type, authorizedSpaces), + new Map() + ); + const response = await this.baseClient.find({ + ...options, + typeToNamespacesMap: undefined, // if the user is fully authorized, use `undefined` as the typeToNamespacesMap to prevent privilege escalation + ...(status === 'partially_authorized' && { typeToNamespacesMap, type: '', namespaces: [] }), // the repository requires that `type` and `namespaces` must be empty if `typeToNamespacesMap` is defined + }); return await this.redactSavedObjectsNamespaces(response); } @@ -131,9 +169,9 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra objects: SavedObjectsBulkGetObject[] = [], options: SavedObjectsBaseOptions = {} ) { + const args = { objects, options }; await this.ensureAuthorized(this.getUniqueObjectTypes(objects), 'bulk_get', options.namespace, { - objects, - options, + args, }); const response = await this.baseClient.bulkGet(objects, options); @@ -141,7 +179,8 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra } public async get(type: string, id: string, options: SavedObjectsBaseOptions = {}) { - await this.ensureAuthorized(type, 'get', options.namespace, { type, id, options }); + const args = { type, id, options }; + await this.ensureAuthorized(type, 'get', options.namespace, { args }); const savedObject = await this.baseClient.get(type, id, options); return await this.redactSavedObjectNamespaces(savedObject); @@ -154,7 +193,7 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra options: SavedObjectsUpdateOptions = {} ) { const args = { type, id, attributes, options }; - await this.ensureAuthorized(type, 'update', options.namespace, args); + await this.ensureAuthorized(type, 'update', options.namespace, { args }); const savedObject = await this.baseClient.update(type, id, attributes, options); return await this.redactSavedObjectNamespaces(savedObject); @@ -169,13 +208,19 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra const args = { type, id, namespaces, options }; const { namespace } = options; // To share an object, the user must have the "create" permission in each of the destination namespaces. - await this.ensureAuthorized(type, 'create', namespaces, args, 'addToNamespacesCreate'); + await this.ensureAuthorized(type, 'create', namespaces, { + args, + auditAction: 'addToNamespacesCreate', + }); // To share an object, the user must also have the "update" permission in one or more of the source namespaces. Because the // `addToNamespaces` operation is scoped to the current namespace, we can just check if the user has the "update" permission in the // current namespace. If the user has permission, but the saved object doesn't exist in this namespace, the base client operation will // result in a 404 error. - await this.ensureAuthorized(type, 'update', namespace, args, 'addToNamespacesUpdate'); + await this.ensureAuthorized(type, 'update', namespace, { + args, + auditAction: 'addToNamespacesUpdate', + }); const result = await this.baseClient.addToNamespaces(type, id, namespaces, options); return await this.redactSavedObjectNamespaces(result); @@ -189,7 +234,10 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra ) { const args = { type, id, namespaces, options }; // To un-share an object, the user must have the "delete" permission in each of the target namespaces. - await this.ensureAuthorized(type, 'delete', namespaces, args, 'deleteFromNamespaces'); + await this.ensureAuthorized(type, 'delete', namespaces, { + args, + auditAction: 'deleteFromNamespaces', + }); const result = await this.baseClient.deleteFromNamespaces(type, id, namespaces, options); return await this.redactSavedObjectNamespaces(result); @@ -205,9 +253,9 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra .filter(({ namespace }) => namespace !== undefined) .map(({ namespace }) => namespace!); const namespaces = [options?.namespace, ...objectNamespaces]; + const args = { objects, options }; await this.ensureAuthorized(this.getUniqueObjectTypes(objects), 'bulk_update', namespaces, { - objects, - options, + args, }); const response = await this.baseClient.bulkUpdate(objects, options); @@ -228,11 +276,10 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra private async ensureAuthorized( typeOrTypes: string | string[], action: string, - namespaceOrNamespaces?: string | Array, - args?: Record, - auditAction: string = action, - requiresAll = true - ) { + namespaceOrNamespaces: undefined | string | Array, + options: EnsureAuthorizedOptions = {} + ): Promise { + const { args, auditAction = action, requireFullAuthorization = true } = options; const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; const actionsToTypesMap = new Map( types.map((type) => [this.actions.savedObject.get(type, action), type]) @@ -245,27 +292,60 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra privileges.kibana.map(({ resource }) => resource).filter((x) => x !== undefined) ).sort() as string[]; - const isAuthorized = - (requiresAll && hasAllRequested) || - (!requiresAll && privileges.kibana.some(({ authorized }) => authorized)); - if (isAuthorized) { - this.auditLogger.savedObjectsAuthorizationSuccess( + const missingPrivileges = this.getMissingPrivileges(privileges); + const typeMap = privileges.kibana.reduce>( + (acc, { resource, privilege, authorized }) => { + if (!authorized) { + return acc; + } + const type = actionsToTypesMap.get(privilege)!; // always defined + const value = acc.get(type) ?? { authorizedSpaces: [] }; + if (resource === undefined) { + return acc.set(type, { ...value, isGloballyAuthorized: true }); + } + const authorizedSpaces = value.authorizedSpaces.concat(resource); + return acc.set(type, { ...value, authorizedSpaces }); + }, + new Map() + ); + + const logAuthorizationFailure = () => { + this.auditLogger.savedObjectsAuthorizationFailure( username, auditAction, types, spaceIds, + missingPrivileges, args ); - } else { - const missingPrivileges = this.getMissingPrivileges(privileges); - this.auditLogger.savedObjectsAuthorizationFailure( + }; + const logAuthorizationSuccess = (typeArray: string[], spaceIdArray: string[]) => { + this.auditLogger.savedObjectsAuthorizationSuccess( username, auditAction, - types, - spaceIds, - missingPrivileges, + typeArray, + spaceIdArray, args ); + }; + + if (hasAllRequested) { + logAuthorizationSuccess(types, spaceIds); + return { typeMap, status: 'fully_authorized' }; + } else if (!requireFullAuthorization) { + const isPartiallyAuthorized = privileges.kibana.some(({ authorized }) => authorized); + if (isPartiallyAuthorized) { + for (const [type, { isGloballyAuthorized, authorizedSpaces }] of typeMap.entries()) { + // generate an individual audit record for each authorized type + logAuthorizationSuccess([type], isGloballyAuthorized ? spaceIds : authorizedSpaces); + } + return { typeMap, status: 'partially_authorized' }; + } else { + logAuthorizationFailure(); + return { typeMap, status: 'unauthorized' }; + } + } else { + logAuthorizationFailure(); const targetTypes = uniq( missingPrivileges.map(({ privilege }) => actionsToTypesMap.get(privilege)).sort() ).join(','); @@ -303,19 +383,7 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra } private redactAndSortNamespaces(spaceIds: string[], privilegeMap: Record) { - const comparator = (a: string, b: string) => { - const _a = a.toLowerCase(); - const _b = b.toLowerCase(); - if (_a === '?') { - return 1; - } else if (_a < _b) { - return -1; - } else if (_a > _b) { - return 1; - } - return 0; - }; - return spaceIds.map((spaceId) => (privilegeMap[spaceId] ? spaceId : '?')).sort(comparator); + return spaceIds.map((x) => (privilegeMap[x] ? x : '?')).sort(namespaceComparator); } private async redactSavedObjectNamespaces( @@ -362,3 +430,25 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra }; } } + +/** + * Returns all unique elements of an array. + */ +function uniq(arr: T[]): T[] { + return Array.from(new Set(arr)); +} + +/** + * Utility function to sort potentially redacted namespaces. + * Sorts in a case-insensitive manner, and ensures that redacted namespaces ('?') always show up at the end of the array. + */ +function namespaceComparator(a: string, b: string) { + const A = a.toUpperCase(); + const B = b.toUpperCase(); + if (A === '?' && B !== '?') { + return 1; + } else if (A !== '?' && B === '?') { + return -1; + } + return A > B ? 1 : A < B ? -1 : 0; +} diff --git a/x-pack/plugins/spaces/server/lib/spaces_client/spaces_client.ts b/x-pack/plugins/spaces/server/lib/spaces_client/spaces_client.ts index acb00a87bf7d9..5ef0b5375d796 100644 --- a/x-pack/plugins/spaces/server/lib/spaces_client/spaces_client.ts +++ b/x-pack/plugins/spaces/server/lib/spaces_client/spaces_client.ts @@ -104,7 +104,7 @@ export class SpacesClient { `SpacesClient.getAll(), using RBAC. returning 403/Forbidden. Not authorized for any spaces for ${purpose} purpose.` ); this.auditLogger.spacesAuthorizationFailure(username, 'getAll'); - throw Boom.forbidden(); + throw Boom.forbidden(); // Note: there is a catch for this in `SpacesSavedObjectsClient.find`; if we get rid of this error, remove that too } this.auditLogger.spacesAuthorizationSuccess(username, 'getAll', authorized as string[]); 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 c9c17d091cd55..f7621f11a1c05 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 @@ -10,6 +10,8 @@ import { spacesServiceMock } from '../spaces_service/spaces_service.mock'; import { savedObjectsClientMock } from '../../../../../src/core/server/mocks'; import { SavedObjectTypeRegistry } from 'src/core/server'; import { SpacesClient } from '../lib/spaces_client'; +import { spacesClientMock } from '../lib/spaces_client/spaces_client.mock'; +import Boom from 'boom'; const typeRegistry = new SavedObjectTypeRegistry(); typeRegistry.registerType({ @@ -129,6 +131,34 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); describe('#find', () => { + 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 } = await createSpacesSavedObjectsClient(); + const spacesClient = spacesClientMock.create(); + spacesClient.getAll.mockResolvedValue([]); + spacesService.scopedClient.mockResolvedValue(spacesClient); + + const options = Object.freeze({ type: 'foo', namespaces: ['some-ns'] }); + const actualReturnValue = await client.find(options); + + expect(actualReturnValue).toEqual(EMPTY_RESPONSE); + expect(baseClient.find).not.toHaveBeenCalled(); + }); + + test(`returns empty result if user is unauthorized in any space`, async () => { + const { client, baseClient, spacesService } = await createSpacesSavedObjectsClient(); + const spacesClient = spacesClientMock.create(); + spacesClient.getAll.mockRejectedValue(Boom.unauthorized()); + spacesService.scopedClient.mockResolvedValue(spacesClient); + + const options = Object.freeze({ type: 'foo', namespaces: ['some-ns'] }); + const actualReturnValue = await client.find(options); + + expect(actualReturnValue).toEqual(EMPTY_RESPONSE); + expect(baseClient.find).not.toHaveBeenCalled(); + }); + test(`passes options.type to baseClient if valid singular type specified`, async () => { const { client, baseClient } = await createSpacesSavedObjectsClient(); const expectedReturnValue = { 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 4e830d6149537..a65e0431aef92 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 @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import Boom from 'boom'; import { SavedObjectsBaseOptions, SavedObjectsBulkCreateObject, @@ -16,8 +17,9 @@ import { SavedObjectsUpdateOptions, SavedObjectsAddToNamespacesOptions, SavedObjectsDeleteFromNamespacesOptions, + SavedObjectsUtils, ISavedObjectTypeRegistry, -} from 'src/core/server'; +} from '../../../../../src/core/server'; import { SpacesServiceSetup } from '../spaces_service/spaces_service'; import { spaceIdToNamespace } from '../lib/utils/namespace'; import { SpacesClient } from '../lib/spaces_client'; @@ -164,19 +166,26 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { let namespaces = options.namespaces; if (namespaces) { const spacesClient = await this.getSpacesClient; - const availableSpaces = await spacesClient.getAll('findSavedObjects'); - if (namespaces.includes('*')) { - namespaces = availableSpaces.map((space) => space.id); - } else { - namespaces = namespaces.filter((namespace) => - availableSpaces.some((space) => space.id === namespace) - ); - } - // This forbidden error allows this scenario to be consistent - // with the way the SpacesClient behaves when no spaces are authorized - // there. - if (namespaces.length === 0) { - throw this.errors.decorateForbiddenError(new Error()); + + try { + const availableSpaces = await spacesClient.getAll('findSavedObjects'); + if (namespaces.includes('*')) { + namespaces = availableSpaces.map((space) => space.id); + } else { + namespaces = namespaces.filter((namespace) => + availableSpaces.some((space) => space.id === namespace) + ); + } + if (namespaces.length === 0) { + // return empty response, since the user is unauthorized in this space (or these spaces), but we don't return forbidden errors for `find` operations + return SavedObjectsUtils.createEmptyFindResponse(options); + } + } catch (err) { + if (Boom.isBoom(err) && err.output.payload.statusCode === 403) { + // return empty response, since the user is unauthorized in any space, but we don't return forbidden errors for `find` operations + return SavedObjectsUtils.createEmptyFindResponse(options); + } + throw err; } } else { namespaces = [this.spaceId]; diff --git a/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_cases.ts b/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_cases.ts index b32950538f8e5..190b12e038b27 100644 --- a/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_cases.ts +++ b/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_cases.ts @@ -4,30 +4,48 @@ * you may not use this file except in compliance with the Elastic License. */ -export const SAVED_OBJECT_TEST_CASES = Object.freeze({ +import { SPACES } from './spaces'; +import { TestCase } from './types'; + +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, + SPACE_1: { spaceId: SPACE_1_ID }, + SPACE_2: { spaceId: SPACE_2_ID }, +} = SPACES; +const EACH_SPACE = [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID]; + +type CommonTestCase = Omit & { originId?: string }; + +export const SAVED_OBJECT_TEST_CASES: Record = Object.freeze({ SINGLE_NAMESPACE_DEFAULT_SPACE: Object.freeze({ type: 'isolatedtype', id: 'defaultspace-isolatedtype-id', + expectedNamespaces: [DEFAULT_SPACE_ID], }), SINGLE_NAMESPACE_SPACE_1: Object.freeze({ type: 'isolatedtype', id: 'space1-isolatedtype-id', + expectedNamespaces: [SPACE_1_ID], }), SINGLE_NAMESPACE_SPACE_2: Object.freeze({ type: 'isolatedtype', id: 'space2-isolatedtype-id', + expectedNamespaces: [SPACE_2_ID], }), MULTI_NAMESPACE_DEFAULT_AND_SPACE_1: Object.freeze({ type: 'sharedtype', id: 'default_and_space_1', + expectedNamespaces: [DEFAULT_SPACE_ID, SPACE_1_ID], }), MULTI_NAMESPACE_ONLY_SPACE_1: Object.freeze({ type: 'sharedtype', id: 'only_space_1', + expectedNamespaces: [SPACE_1_ID], }), MULTI_NAMESPACE_ONLY_SPACE_2: Object.freeze({ type: 'sharedtype', id: 'only_space_2', + expectedNamespaces: [SPACE_2_ID], }), NAMESPACE_AGNOSTIC: Object.freeze({ type: 'globaltype', @@ -38,3 +56,37 @@ export const SAVED_OBJECT_TEST_CASES = Object.freeze({ id: 'any', }), }); + +/** + * These objects exist in the test data for all saved object test suites, but they are only used to test various conflict scenarios. + */ +export const CONFLICT_TEST_CASES: Record = Object.freeze({ + CONFLICT_1_OBJ: Object.freeze({ + type: 'sharedtype', + id: 'conflict_1', + expectedNamespaces: EACH_SPACE, + }), + CONFLICT_2A_OBJ: Object.freeze({ + type: 'sharedtype', + id: 'conflict_2a', + originId: 'conflict_2', + expectedNamespaces: EACH_SPACE, + }), + CONFLICT_2B_OBJ: Object.freeze({ + type: 'sharedtype', + id: 'conflict_2b', + originId: 'conflict_2', + expectedNamespaces: EACH_SPACE, + }), + CONFLICT_3_OBJ: Object.freeze({ + type: 'sharedtype', + id: 'conflict_3', + expectedNamespaces: EACH_SPACE, + }), + CONFLICT_4A_OBJ: Object.freeze({ + type: 'sharedtype', + id: 'conflict_4a', + originId: 'conflict_4', + expectedNamespaces: EACH_SPACE, + }), +}); diff --git a/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_utils.ts b/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_utils.ts index 595986c08efc1..9d4b5e80e9c3d 100644 --- a/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_utils.ts +++ b/x-pack/test/saved_object_api_integration/common/lib/saved_object_test_utils.ts @@ -6,7 +6,6 @@ import expect from '@kbn/expect'; import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; -import { SAVED_OBJECT_TEST_CASES as CASES } from './saved_object_test_cases'; import { SPACES } from './spaces'; import { AUTHENTICATION } from './authentication'; import { TestCase, TestUser, ExpectResponseBody } from './types'; @@ -73,6 +72,28 @@ export const getTestTitle = ( return `${list.join(' and ')}`; }; +export const isUserAuthorizedAtSpace = (user: TestUser | undefined, namespace: string) => + !user || user.authorizedAtSpaces.includes('*') || user.authorizedAtSpaces.includes(namespace); + +export const getRedactedNamespaces = ( + user: TestUser | undefined, + namespaces: string[] | undefined +) => namespaces?.map((x) => (isUserAuthorizedAtSpace(user, x) ? x : '?')).sort(namespaceComparator); +function namespaceComparator(a: string, b: string) { + // namespaces get sorted so that they're all in alphabetical order, and unknown ones appear at the end + // this is to prevent information disclosure + if (a === '?' && b !== '?') { + return 1; + } else if (b === '?' && a !== '?') { + return -1; + } else if (a > b) { + return 1; + } else if (a < b) { + return -1; + } + return 0; +} + export const testCaseFailures = { // test suites need explicit return types for number primitives fail400: (condition?: boolean): { failure?: 400 } => @@ -150,7 +171,7 @@ export const expectResponses = { } }, /** - * Additional assertions that we use in `bulk_create` and `create` to ensure that + * Additional assertions that we use in `import` and `resolve_import_errors` to ensure that * newly-created (or overwritten) objects don't have unexpected properties */ successCreated: async (es: any, spaceId: string, type: string, id: string) => { @@ -161,26 +182,6 @@ export const expectResponses = { id: `${expectedSpacePrefix}${type}:${id}`, index: '.kibana', }); - const { namespace: actualNamespace, namespaces: actualNamespaces } = savedObject._source; - if (isNamespaceUndefined) { - expect(actualNamespace).to.eql(undefined); - } else { - expect(actualNamespace).to.eql(spaceId); - } - if (isMultiNamespace(type)) { - if (['conflict_1', 'conflict_2a', 'conflict_2b', 'conflict_3', 'conflict_4a'].includes(id)) { - expect(actualNamespaces).to.eql([DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID]); - } else if (id === CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1.id) { - expect(actualNamespaces).to.eql([DEFAULT_SPACE_ID, SPACE_1_ID]); - } else if (id === CASES.MULTI_NAMESPACE_ONLY_SPACE_1.id) { - expect(actualNamespaces).to.eql([SPACE_1_ID]); - } else if (id === CASES.MULTI_NAMESPACE_ONLY_SPACE_2.id) { - expect(actualNamespaces).to.eql([SPACE_2_ID]); - } else { - // newly created in this space - expect(actualNamespaces).to.eql([spaceId]); - } - } return savedObject; }, }; diff --git a/x-pack/test/saved_object_api_integration/common/lib/types.ts b/x-pack/test/saved_object_api_integration/common/lib/types.ts index 56e6a992b6b62..b52a84f352999 100644 --- a/x-pack/test/saved_object_api_integration/common/lib/types.ts +++ b/x-pack/test/saved_object_api_integration/common/lib/types.ts @@ -21,6 +21,7 @@ export interface TestSuite { export interface TestCase { type: string; id: string; + expectedNamespaces?: string[]; failure?: 400 | 403 | 404 | 409; } diff --git a/x-pack/test/saved_object_api_integration/common/suites/bulk_create.ts b/x-pack/test/saved_object_api_integration/common/suites/bulk_create.ts index e3163ef77d427..b1608946b8e62 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/bulk_create.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/bulk_create.ts @@ -14,8 +14,9 @@ import { expectResponses, getUrlPrefix, getTestTitle, + getRedactedNamespaces, } from '../lib/saved_object_test_utils'; -import { ExpectResponseBody, TestCase, TestDefinition, TestSuite } from '../lib/types'; +import { ExpectResponseBody, TestCase, TestDefinition, TestSuite, TestUser } from '../lib/types'; export interface BulkCreateTestDefinition extends TestDefinition { request: Array<{ type: string; id: string }>; @@ -33,7 +34,7 @@ const NEW_ATTRIBUTE_VAL = `New attribute value ${Date.now()}`; const NEW_SINGLE_NAMESPACE_OBJ = Object.freeze({ type: 'dashboard', id: 'new-dashboard-id' }); const NEW_MULTI_NAMESPACE_OBJ = Object.freeze({ type: 'sharedtype', id: 'new-sharedtype-id' }); const NEW_NAMESPACE_AGNOSTIC_OBJ = Object.freeze({ type: 'globaltype', id: 'new-globaltype-id' }); -export const TEST_CASES = Object.freeze({ +export const TEST_CASES: Record = Object.freeze({ ...CASES, NEW_SINGLE_NAMESPACE_OBJ, NEW_MULTI_NAMESPACE_OBJ, @@ -45,7 +46,7 @@ export function bulkCreateTestSuiteFactory(es: any, esArchiver: any, supertest: const expectResponseBody = ( testCases: BulkCreateTestCase | BulkCreateTestCase[], statusCode: 200 | 403, - spaceId = SPACES.DEFAULT.spaceId + user?: TestUser ): ExpectResponseBody => async (response: Record) => { const testCaseArray = Array.isArray(testCases) ? testCases : [testCases]; if (statusCode === 403) { @@ -70,7 +71,8 @@ export function bulkCreateTestSuiteFactory(es: any, esArchiver: any, supertest: await expectResponses.permitted(object, testCase); if (!testCase.failure) { expect(object.attributes[NEW_ATTRIBUTE_KEY]).to.eql(NEW_ATTRIBUTE_VAL); - await expectResponses.successCreated(es, spaceId, object.type, object.id); + const redactedNamespaces = getRedactedNamespaces(user, testCase.expectedNamespaces); + expect(object.namespaces).to.eql(redactedNamespaces); } } } @@ -81,6 +83,7 @@ export function bulkCreateTestSuiteFactory(es: any, esArchiver: any, supertest: overwrite: boolean, options?: { spaceId?: string; + user?: TestUser; singleRequest?: boolean; responseBodyOverride?: ExpectResponseBody; } @@ -95,8 +98,7 @@ export function bulkCreateTestSuiteFactory(es: any, esArchiver: any, supertest: request: [createRequest(x)], responseStatusCode, responseBody: - options?.responseBodyOverride || - expectResponseBody(x, responseStatusCode, options?.spaceId), + options?.responseBodyOverride || expectResponseBody(x, responseStatusCode, options?.user), overwrite, })); } @@ -108,7 +110,7 @@ export function bulkCreateTestSuiteFactory(es: any, esArchiver: any, supertest: responseStatusCode, responseBody: options?.responseBodyOverride || - expectResponseBody(cases, responseStatusCode, options?.spaceId), + expectResponseBody(cases, responseStatusCode, options?.user), overwrite, }, ]; 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 8de54fe499c07..71ece1265347c 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 @@ -25,7 +25,10 @@ export interface BulkGetTestCase extends TestCase { } const DOES_NOT_EXIST = Object.freeze({ type: 'dashboard', id: 'does-not-exist' }); -export const TEST_CASES = Object.freeze({ ...CASES, DOES_NOT_EXIST }); +export const TEST_CASES: Record = Object.freeze({ + ...CASES, + DOES_NOT_EXIST, +}); export function bulkGetTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectForbidden = expectResponses.forbiddenTypes('bulk_get'); 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 2e3c55f029d29..c3020b2da3219 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 @@ -24,7 +24,10 @@ const NEW_ATTRIBUTE_KEY = 'title'; // all type mappings include this attribute, 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 }); +export const TEST_CASES: Record = Object.freeze({ + ...CASES, + DOES_NOT_EXIST, +}); const createRequest = ({ type, id, namespace }: BulkUpdateTestCase) => ({ type, diff --git a/x-pack/test/saved_object_api_integration/common/suites/create.ts b/x-pack/test/saved_object_api_integration/common/suites/create.ts index 2a5ab696c4f53..7e28d5ed9ed94 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/create.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/create.ts @@ -13,8 +13,9 @@ import { expectResponses, getUrlPrefix, getTestTitle, + getRedactedNamespaces, } from '../lib/saved_object_test_utils'; -import { ExpectResponseBody, TestCase, TestDefinition, TestSuite } from '../lib/types'; +import { ExpectResponseBody, TestCase, TestDefinition, TestSuite, TestUser } from '../lib/types'; export interface CreateTestDefinition extends TestDefinition { request: { type: string; id: string }; @@ -33,7 +34,7 @@ const NEW_ATTRIBUTE_VAL = `New attribute value ${Date.now()}`; const NEW_SINGLE_NAMESPACE_OBJ = Object.freeze({ type: 'dashboard', id: '' }); const NEW_MULTI_NAMESPACE_OBJ = Object.freeze({ type: 'sharedtype', id: 'new-sharedtype-id' }); const NEW_NAMESPACE_AGNOSTIC_OBJ = Object.freeze({ type: 'globaltype', id: 'new-globaltype-id' }); -export const TEST_CASES = Object.freeze({ +export const TEST_CASES: Record = Object.freeze({ ...CASES, NEW_SINGLE_NAMESPACE_OBJ, NEW_MULTI_NAMESPACE_OBJ, @@ -44,7 +45,7 @@ export function createTestSuiteFactory(es: any, esArchiver: any, supertest: Supe const expectForbidden = expectResponses.forbiddenTypes('create'); const expectResponseBody = ( testCase: CreateTestCase, - spaceId = SPACES.DEFAULT.spaceId + user?: TestUser ): ExpectResponseBody => async (response: Record) => { if (testCase.failure === 403) { await expectForbidden(testCase.type)(response); @@ -54,7 +55,8 @@ export function createTestSuiteFactory(es: any, esArchiver: any, supertest: Supe await expectResponses.permitted(object, testCase); if (!testCase.failure) { expect(object.attributes[NEW_ATTRIBUTE_KEY]).to.eql(NEW_ATTRIBUTE_VAL); - await expectResponses.successCreated(es, spaceId, object.type, object.id); + const redactedNamespaces = getRedactedNamespaces(user, testCase.expectedNamespaces); + expect(object.namespaces).to.eql(redactedNamespaces); } } }; @@ -64,6 +66,7 @@ export function createTestSuiteFactory(es: any, esArchiver: any, supertest: Supe overwrite: boolean, options?: { spaceId?: string; + user?: TestUser; responseBodyOverride?: ExpectResponseBody; } ): CreateTestDefinition[] => { @@ -76,7 +79,7 @@ export function createTestSuiteFactory(es: any, esArchiver: any, supertest: Supe title: getTestTitle(x), responseStatusCode: x.failure ?? 200, request: createRequest(x), - responseBody: options?.responseBodyOverride || expectResponseBody(x, options?.spaceId), + responseBody: options?.responseBodyOverride || expectResponseBody(x, options?.user), overwrite, })); }; diff --git a/x-pack/test/saved_object_api_integration/common/suites/delete.ts b/x-pack/test/saved_object_api_integration/common/suites/delete.ts index 3179b1b0c9ac5..228e7977f99ac 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/delete.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/delete.ts @@ -25,7 +25,10 @@ export interface DeleteTestCase extends TestCase { } const DOES_NOT_EXIST = Object.freeze({ type: 'dashboard', id: 'does-not-exist' }); -export const TEST_CASES = Object.freeze({ ...CASES, DOES_NOT_EXIST }); +export const TEST_CASES: Record = Object.freeze({ + ...CASES, + DOES_NOT_EXIST, +}); export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectForbidden = expectResponses.forbiddenTypes('delete'); diff --git a/x-pack/test/saved_object_api_integration/common/suites/export.ts b/x-pack/test/saved_object_api_integration/common/suites/export.ts index 4a8eff1fb380c..4eb967a952c60 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/export.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/export.ts @@ -30,7 +30,10 @@ export interface ExportTestCase { type: string; id?: string; successResult?: SuccessResult | SuccessResult[]; - failure?: 400 | 403; + failure?: { + statusCode: 200 | 400 | 403; // if the user searches for only types they are not authorized for, they will get an empty 200 result + reason: 'unauthorized' | 'bad_request'; + }; } // additional sharedtype objects that exist but do not have common test cases defined @@ -90,41 +93,45 @@ export const getTestCases = (spaceId?: string): { [key: string]: ExportTestCase type: 'globaltype', successResult: CASES.NAMESPACE_AGNOSTIC, }, - hiddenObject: { title: 'hidden object', ...CASES.HIDDEN, failure: 400 }, - hiddenType: { title: 'hidden type', type: 'hiddentype', failure: 400 }, + hiddenObject: { + title: 'hidden object', + ...CASES.HIDDEN, + failure: { statusCode: 400, reason: 'bad_request' }, + }, + hiddenType: { + title: 'hidden type', + type: 'hiddentype', + failure: { statusCode: 400, reason: 'bad_request' }, + }, }); export const createRequest = ({ type, id }: ExportTestCase) => id ? { objects: [{ type, id }] } : { type }; -const getTestTitle = ({ failure, title }: ExportTestCase) => { - let description = 'success'; - if (failure === 400) { - description = 'bad request'; - } else if (failure === 403) { - description = 'forbidden'; - } - return `${description} ["${title}"]`; -}; +const getTestTitle = ({ failure, title }: ExportTestCase) => + `${failure?.reason || 'success'} ["${title}"]`; + +const EMPTY_RESULT = { exportedCount: 0, missingRefCount: 0, missingReferences: [] }; export function exportTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectForbiddenBulkGet = expectResponses.forbiddenTypes('bulk_get'); - const expectForbiddenFind = expectResponses.forbiddenTypes('find'); const expectResponseBody = (testCase: ExportTestCase): ExpectResponseBody => async ( response: Record ) => { const { type, id, successResult = { type, id } as SuccessResult, failure } = testCase; - if (failure === 403) { - // In export only, the API uses "bulk_get" or "find" depending on the parameters it receives. - // The best that could be done here is to have an if statement to ensure at least one of the - // two errors has been thrown. - if (id) { + if (failure?.reason === 'unauthorized') { + // In export only, the API uses "bulkGet" or "find" depending on the parameters it receives. + if (failure.statusCode === 403) { + // "bulkGet" was unauthorized, which returns a forbidden error await expectForbiddenBulkGet(type)(response); + } else if (failure.statusCode === 200) { + // "find" was unauthorized, which returns an empty result + expect(response.body).not.to.have.property('error'); + expect(response.text).to.equal(JSON.stringify(EMPTY_RESULT)); } else { - await expectForbiddenFind(type)(response); + throw new Error(`Unexpected failure status code: ${failure.statusCode}`); } - } else if (failure === 400) { - // 400 + } else if (failure?.reason === 'bad_request') { expect(response.body.error).to.eql('Bad Request'); - expect(response.body.statusCode).to.eql(failure); + expect(response.body.statusCode).to.eql(failure.statusCode); if (id) { expect(response.body.message).to.eql( `Trying to export object(s) with non-exportable types: ${type}:${id}` @@ -132,6 +139,8 @@ export function exportTestSuiteFactory(esArchiver: any, supertest: SuperTest { let cases = Array.isArray(testCases) ? testCases : [testCases]; - if (forbidden) { + if (failure) { // override the expected result in each test case - cases = cases.map((x) => ({ ...x, failure: 403 })); + cases = cases.map((x) => ({ ...x, failure })); } return cases.map((x) => ({ title: getTestTitle(x), - responseStatusCode: x.failure ?? 200, + responseStatusCode: x.failure?.statusCode ?? 200, request: createRequest(x), responseBody: options?.responseBodyOverride || expectResponseBody(x), })); diff --git a/x-pack/test/saved_object_api_integration/common/suites/find.ts b/x-pack/test/saved_object_api_integration/common/suites/find.ts index bab4a4d88534a..381306f810122 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/find.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/find.ts @@ -7,10 +7,13 @@ import expect from '@kbn/expect'; import { SuperTest } from 'supertest'; import querystring from 'querystring'; -import { Assign } from '@kbn/utility-types'; -import { SAVED_OBJECT_TEST_CASES as CASES } from '../lib/saved_object_test_cases'; +import { SAVED_OBJECT_TEST_CASES, CONFLICT_TEST_CASES } from '../lib/saved_object_test_cases'; import { SPACES } from '../lib/spaces'; -import { expectResponses, getUrlPrefix } from '../lib/saved_object_test_utils'; +import { + getUrlPrefix, + isUserAuthorizedAtSpace, + getRedactedNamespaces, +} from '../lib/saved_object_test_utils'; import { ExpectResponseBody, TestCase, TestDefinition, TestSuite, TestUser } from '../lib/types'; const { @@ -22,80 +25,34 @@ export interface FindTestDefinition extends TestDefinition { } export type FindTestSuite = TestSuite; -type FindSavedObjectCase = Assign; - export interface FindTestCase { title: string; query: string; successResult?: { - savedObjects?: FindSavedObjectCase | FindSavedObjectCase[]; + savedObjects?: TestCase | TestCase[]; page?: number; perPage?: number; total?: number; }; failure?: { - statusCode: 400 | 403; - reason: - | 'forbidden_types' - | 'forbidden_namespaces' - | 'cross_namespace_not_permitted' - | 'bad_request'; + statusCode: 200 | 400; // if the user searches for types and/or namespaces they are not authorized for, they will get a 200 result with those types/namespaces omitted + reason: 'unauthorized' | 'cross_namespace_not_permitted' | 'bad_request'; }; } -// additional sharedtype objects that exist but do not have common test cases defined -const CONFLICT_1_OBJ = Object.freeze({ - type: 'sharedtype', - id: 'conflict_1', - namespaces: ['default', 'space_1', 'space_2'], -}); -const CONFLICT_2A_OBJ = Object.freeze({ - type: 'sharedtype', - id: 'conflict_2a', - originId: 'conflict_2', - namespaces: ['default', 'space_1', 'space_2'], -}); -const CONFLICT_2B_OBJ = Object.freeze({ - type: 'sharedtype', - id: 'conflict_2b', - originId: 'conflict_2', - namespaces: ['default', 'space_1', 'space_2'], -}); -const CONFLICT_3_OBJ = Object.freeze({ - type: 'sharedtype', - id: 'conflict_3', - namespaces: ['default', 'space_1', 'space_2'], -}); -const CONFLICT_4A_OBJ = Object.freeze({ - type: 'sharedtype', - id: 'conflict_4a', - originId: 'conflict_4', - namespaces: ['default', 'space_1', 'space_2'], -}); - const TEST_CASES = [ - { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, namespaces: ['default'] }, - { ...CASES.SINGLE_NAMESPACE_SPACE_1, namespaces: ['space_1'] }, - { ...CASES.SINGLE_NAMESPACE_SPACE_2, namespaces: ['space_2'] }, - { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, namespaces: ['default', 'space_1'] }, - { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, namespaces: ['space_1'] }, - { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, namespaces: ['space_2'] }, - { ...CASES.NAMESPACE_AGNOSTIC, namespaces: undefined }, - { ...CASES.HIDDEN, namespaces: undefined }, + ...Object.values(SAVED_OBJECT_TEST_CASES), + ...Object.values(CONFLICT_TEST_CASES), ]; -expect(TEST_CASES.length).to.eql( - Object.values(CASES).length, - 'Unhandled test cases in `find` suite' -); - export const getTestCases = ( { currentSpace, crossSpaceSearch }: { currentSpace?: string; crossSpaceSearch?: string[] } = { currentSpace: undefined, crossSpaceSearch: undefined, } ) => { - const crossSpaceIds = crossSpaceSearch?.filter((s) => s !== (currentSpace ?? 'default')) ?? []; + const crossSpaceIds = + crossSpaceSearch?.filter((s) => s !== (currentSpace ?? DEFAULT_SPACE_ID)) ?? []; // intentionally exclude the current space const isCrossSpaceSearch = crossSpaceIds.length > 0; const isWildcardSearch = crossSpaceIds.includes('*'); @@ -104,7 +61,7 @@ export const getTestCases = ( : ''; const buildTitle = (title: string) => - crossSpaceSearch ? `${title} (cross-space ${isWildcardSearch ? 'with wildcard' : ''})` : title; + crossSpaceSearch ? `${title} (cross-space${isWildcardSearch ? ' with wildcard' : ''})` : title; type CasePredicate = (testCase: TestCase) => boolean; const getExpectedSavedObjects = (predicate: CasePredicate) => { @@ -117,13 +74,16 @@ export const getTestCases = ( return TEST_CASES.filter((t) => { const hasOtherNamespaces = - Array.isArray(t.namespaces) && - t.namespaces!.some((ns) => ns !== (currentSpace ?? 'default')); + !t.expectedNamespaces || // namespace-agnostic types do not have an expectedNamespaces field + t.expectedNamespaces.some((ns) => ns !== (currentSpace ?? DEFAULT_SPACE_ID)); return hasOtherNamespaces && predicate(t); }); } return TEST_CASES.filter( - (t) => (!t.namespaces || t.namespaces.includes(currentSpace ?? 'default')) && predicate(t) + (t) => + (!t.expectedNamespaces || + t.expectedNamespaces.includes(currentSpace ?? DEFAULT_SPACE_ID)) && + predicate(t) ); }; @@ -140,19 +100,13 @@ export const getTestCases = ( query: `type=sharedtype&fields=title${namespacesQueryParam}`, successResult: { // expected depends on which spaces the user is authorized against... - savedObjects: getExpectedSavedObjects((t) => t.type === 'sharedtype').concat( - CONFLICT_1_OBJ, - CONFLICT_2A_OBJ, - CONFLICT_2B_OBJ, - CONFLICT_3_OBJ, - CONFLICT_4A_OBJ - ), + savedObjects: getExpectedSavedObjects((t) => t.type === 'sharedtype'), }, } as FindTestCase, namespaceAgnosticType: { title: buildTitle('find namespace-agnostic type'), query: `type=globaltype&fields=title${namespacesQueryParam}`, - successResult: { savedObjects: CASES.NAMESPACE_AGNOSTIC }, + successResult: { savedObjects: SAVED_OBJECT_TEST_CASES.NAMESPACE_AGNOSTIC }, } as FindTestCase, hiddenType: { title: buildTitle('find hidden type'), @@ -162,6 +116,15 @@ export const getTestCases = ( title: buildTitle('find unknown type'), query: `type=wigwags${namespacesQueryParam}`, } as FindTestCase, + eachType: { + title: buildTitle('find each type'), + query: `type=isolatedtype&type=sharedtype&type=globaltype&type=hiddentype&type=wigwags${namespacesQueryParam}`, + successResult: { + savedObjects: getExpectedSavedObjects((t) => + ['isolatedtype', 'sharedtype', 'globaltype'].includes(t.type) + ), + }, + } as FindTestCase, pageBeyondTotal: { title: buildTitle('find page beyond total'), query: `type=isolatedtype&page=100&per_page=100${namespacesQueryParam}`, @@ -179,7 +142,7 @@ export const getTestCases = ( filterWithNamespaceAgnosticType: { title: buildTitle('filter with namespace-agnostic type'), query: `type=globaltype&filter=globaltype.attributes.title:*global*${namespacesQueryParam}`, - successResult: { savedObjects: CASES.NAMESPACE_AGNOSTIC }, + successResult: { savedObjects: SAVED_OBJECT_TEST_CASES.NAMESPACE_AGNOSTIC }, } as FindTestCase, filterWithHiddenType: { title: buildTitle('filter with hidden type'), @@ -200,49 +163,48 @@ export const getTestCases = ( }; }; +function objectComparator(a: { id: string }, b: { id: string }) { + return a.id > b.id ? 1 : a.id < b.id ? -1 : 0; +} + export const createRequest = ({ query }: FindTestCase) => ({ query }); -const getTestTitle = ({ failure, title }: FindTestCase) => { - let description = 'success'; - if (failure?.statusCode === 400) { - description = 'bad request'; - } else if (failure?.statusCode === 403) { - description = 'forbidden'; - } - return `${description} ["${title}"]`; -}; +const getTestTitle = ({ failure, title }: FindTestCase) => + `${failure?.reason || 'success'} ["${title}"]`; export function findTestSuiteFactory(esArchiver: any, supertest: SuperTest) { - const expectForbiddenTypes = expectResponses.forbiddenTypes('find'); - const expectForbiddeNamespaces = expectResponses.forbiddenSpaces; const expectResponseBody = ( testCase: FindTestCase, user?: TestUser ): ExpectResponseBody => async (response: Record) => { const { failure, successResult = {}, query } = testCase; const parsedQuery = querystring.parse(query); - if (failure?.statusCode === 403) { - if (failure?.reason === 'forbidden_types') { - const type = parsedQuery.type; - await expectForbiddenTypes(type)(response); - } else if (failure?.reason === 'forbidden_namespaces') { - await expectForbiddeNamespaces(response); + if (failure?.statusCode === 200) { + if (failure?.reason === 'unauthorized') { + // if the user is completely unauthorized, they will receive an empty response body + const expected = { + page: parsedQuery.page || 1, + per_page: parsedQuery.per_page || 20, + total: 0, + saved_objects: [], + }; + expect(response.body).to.eql(expected); } else { - throw new Error(`Unexpected failure reason: ${failure?.reason}`); + throw new Error(`Unexpected failure reason: ${failure.reason}`); } } else if (failure?.statusCode === 400) { - if (failure?.reason === 'bad_request') { + if (failure.reason === 'bad_request') { const type = (parsedQuery.filter as string).split('.')[0]; expect(response.body.error).to.eql('Bad Request'); - expect(response.body.statusCode).to.eql(failure?.statusCode); + expect(response.body.statusCode).to.eql(failure.statusCode); expect(response.body.message).to.eql(`This type ${type} is not allowed: Bad Request`); - } else if (failure?.reason === 'cross_namespace_not_permitted') { + } else if (failure.reason === 'cross_namespace_not_permitted') { expect(response.body.error).to.eql('Bad Request'); - expect(response.body.statusCode).to.eql(failure?.statusCode); + expect(response.body.statusCode).to.eql(failure.statusCode); expect(response.body.message).to.eql( `_find across namespaces is not permitted when the Spaces plugin is disabled.: Bad Request` ); } else { - throw new Error(`Unexpected failure reason: ${failure?.reason}`); + throw new Error(`Unexpected failure reason: ${failure.reason}`); } } else { // 2xx @@ -251,11 +213,8 @@ export function findTestSuiteFactory(esArchiver: any, supertest: SuperTest) const savedObjectsArray = Array.isArray(savedObjects) ? savedObjects : [savedObjects]; const authorizedSavedObjects = savedObjectsArray.filter( (so) => - !user || - !so.namespaces || - so.namespaces.some( - (ns) => user.authorizedAtSpaces.includes(ns) || user.authorizedAtSpaces.includes('*') - ) + !so.expectedNamespaces || + so.expectedNamespaces.some((x) => isUserAuthorizedAtSpace(user, x)) ); expect(response.body.page).to.eql(page); expect(response.body.per_page).to.eql(perPage); @@ -265,16 +224,17 @@ export function findTestSuiteFactory(esArchiver: any, supertest: SuperTest) expect(response.body.total).to.eql(total || authorizedSavedObjects.length); } - authorizedSavedObjects.sort((s1, s2) => (s1.id < s2.id ? -1 : 1)); - response.body.saved_objects.sort((s1: any, s2: any) => (s1.id < s2.id ? -1 : 1)); + authorizedSavedObjects.sort(objectComparator); + response.body.saved_objects.sort(objectComparator); for (let i = 0; i < authorizedSavedObjects.length; i++) { const object = response.body.saved_objects[i]; - const { type: expectedType, id: expectedId } = authorizedSavedObjects[i]; - expect(object.type).to.eql(expectedType); - expect(object.id).to.eql(expectedId); + const expected = authorizedSavedObjects[i]; + const expectedNamespaces = getRedactedNamespaces(user, expected.expectedNamespaces); + expect(object.type).to.eql(expected.type); + expect(object.id).to.eql(expected.id); expect(object.updated_at).to.match(/^[\d-]{10}T[\d:\.]{12}Z$/); - expect(object.namespaces).to.eql(object.namespaces); + expect(object.namespaces).to.eql(expectedNamespaces); // don't test attributes, version, or references } } diff --git a/x-pack/test/saved_object_api_integration/common/suites/get.ts b/x-pack/test/saved_object_api_integration/common/suites/get.ts index fb03cd548d41a..8d8938b5ee79f 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/get.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/get.ts @@ -21,7 +21,7 @@ export type GetTestSuite = TestSuite; export type GetTestCase = TestCase; const DOES_NOT_EXIST = Object.freeze({ type: 'dashboard', id: 'does-not-exist' }); -export const TEST_CASES = Object.freeze({ ...CASES, DOES_NOT_EXIST }); +export const TEST_CASES: Record = Object.freeze({ ...CASES, DOES_NOT_EXIST }); export function getTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectForbidden = expectResponses.forbiddenTypes('get'); diff --git a/x-pack/test/saved_object_api_integration/common/suites/import.ts b/x-pack/test/saved_object_api_integration/common/suites/import.ts index 5036d7b200881..b0d0b4f8a815a 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/import.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/import.ts @@ -36,7 +36,7 @@ const NEW_ATTRIBUTE_VAL = `New attribute value ${Date.now()}`; // * id: conflict_4a, originId: conflict_4 // using the seven conflict test case objects below, we can exercise various permutations of exact/inexact/ambiguous conflict scenarios const CID = 'conflict_'; -export const TEST_CASES = Object.freeze({ +export const TEST_CASES: Record = Object.freeze({ ...CASES, CONFLICT_1_OBJ: Object.freeze({ type: 'sharedtype', id: `${CID}1` }), CONFLICT_1A_OBJ: Object.freeze({ type: 'sharedtype', id: `${CID}1a`, originId: `${CID}1` }), diff --git a/x-pack/test/saved_object_api_integration/common/suites/resolve_import_errors.ts b/x-pack/test/saved_object_api_integration/common/suites/resolve_import_errors.ts index 6d294aed9b4de..02fa614ac2b55 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/resolve_import_errors.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/resolve_import_errors.ts @@ -37,7 +37,7 @@ const NEW_ATTRIBUTE_VAL = `New attribute value ${Date.now()}`; // * id: conflict_3 // * id: conflict_4a, originId: conflict_4 // using the five conflict test case objects below, we can exercise various permutations of exact/inexact/ambiguous conflict scenarios -export const TEST_CASES = Object.freeze({ +export const TEST_CASES: Record = Object.freeze({ ...CASES, CONFLICT_1A_OBJ: Object.freeze({ type: 'sharedtype', diff --git a/x-pack/test/saved_object_api_integration/common/suites/update.ts b/x-pack/test/saved_object_api_integration/common/suites/update.ts index 82f4699babf46..19921a82b2eb4 100644 --- a/x-pack/test/saved_object_api_integration/common/suites/update.ts +++ b/x-pack/test/saved_object_api_integration/common/suites/update.ts @@ -28,7 +28,10 @@ const NEW_ATTRIBUTE_KEY = 'title'; // all type mappings include this attribute, 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 }); +export const TEST_CASES: Record = Object.freeze({ + ...CASES, + DOES_NOT_EXIST, +}); export function updateTestSuiteFactory(esArchiver: any, supertest: SuperTest) { const expectForbidden = expectResponses.forbiddenTypes('update'); diff --git a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_create.ts b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_create.ts index 0cc5969e2b7ab..93ae439d01166 100644 --- a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_create.ts +++ b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/bulk_create.ts @@ -26,13 +26,23 @@ const unresolvableConflict = (condition?: boolean) => const createTestCases = (overwrite: boolean, spaceId: string) => { // for each permitted (non-403) outcome, if failure !== undefined then we expect // to receive an error; otherwise, we expect to receive a success result + const expectedNamespaces = [spaceId]; // newly created objects should have this `namespaces` array in their return value const normalTypes = [ { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, ...fail409(!overwrite && spaceId === DEFAULT_SPACE_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_1, + ...fail409(!overwrite && spaceId === SPACE_1_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + ...fail409(!overwrite && spaceId === SPACE_2_ID), + expectedNamespaces, }, - { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail409(!overwrite && spaceId === SPACE_1_ID) }, - { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail409(!overwrite && spaceId === SPACE_2_ID) }, { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, ...fail409(!overwrite || (spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID)), @@ -49,8 +59,8 @@ const createTestCases = (overwrite: boolean, spaceId: string) => { ...unresolvableConflict(spaceId !== SPACE_2_ID), }, { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, - CASES.NEW_SINGLE_NAMESPACE_OBJ, - CASES.NEW_MULTI_NAMESPACE_OBJ, + { ...CASES.NEW_SINGLE_NAMESPACE_OBJ, expectedNamespaces }, + { ...CASES.NEW_MULTI_NAMESPACE_OBJ, expectedNamespaces }, CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; @@ -68,22 +78,28 @@ export default function ({ getService }: FtrProviderContext) { esArchiver, supertest ); - const createTests = (overwrite: boolean, spaceId: string) => { + const createTests = (overwrite: boolean, spaceId: string, user: TestUser) => { const { normalTypes, hiddenType, allTypes } = createTestCases(overwrite, spaceId); // use singleRequest to reduce execution time and/or test combined cases return { - unauthorized: createTestDefinitions(allTypes, true, overwrite, { spaceId }), + unauthorized: createTestDefinitions(allTypes, true, overwrite, { spaceId, user }), authorized: [ - createTestDefinitions(normalTypes, false, overwrite, { spaceId, singleRequest: true }), - createTestDefinitions(hiddenType, true, overwrite, { spaceId }), + createTestDefinitions(normalTypes, false, overwrite, { + spaceId, + user, + singleRequest: true, + }), + createTestDefinitions(hiddenType, true, overwrite, { spaceId, user }), createTestDefinitions(allTypes, true, overwrite, { spaceId, + user, singleRequest: true, responseBodyOverride: expectForbidden(['hiddentype']), }), ].flat(), superuser: createTestDefinitions(allTypes, false, overwrite, { spaceId, + user, singleRequest: true, }), }; @@ -93,7 +109,6 @@ export default function ({ getService }: FtrProviderContext) { getTestScenarios([false, true]).securityAndSpaces.forEach( ({ spaceId, users, modifier: overwrite }) => { const suffix = ` within the ${spaceId} space${overwrite ? ' with overwrite enabled' : ''}`; - const { unauthorized, authorized, superuser } = createTests(overwrite!, spaceId); const _addTests = (user: TestUser, tests: BulkCreateTestDefinition[]) => { addTests(`${user.description}${suffix}`, { user, spaceId, tests }); }; @@ -106,11 +121,14 @@ export default function ({ getService }: FtrProviderContext) { users.readAtSpace, users.allAtOtherSpace, ].forEach((user) => { + const { unauthorized } = createTests(overwrite!, spaceId, user); _addTests(user, unauthorized); }); [users.dualAll, users.allGlobally, users.allAtSpace].forEach((user) => { + const { authorized } = createTests(overwrite!, spaceId, user); _addTests(user, authorized); }); + const { superuser } = createTests(overwrite!, spaceId, users.superuser); _addTests(users.superuser, superuser); } ); diff --git a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/create.ts b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/create.ts index f81488603dc83..7353dafb5e1b5 100644 --- a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/create.ts +++ b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/create.ts @@ -24,13 +24,23 @@ const { fail400, fail409 } = testCaseFailures; const createTestCases = (overwrite: boolean, spaceId: string) => { // for each permitted (non-403) outcome, if failure !== undefined then we expect // to receive an error; otherwise, we expect to receive a success result + const expectedNamespaces = [spaceId]; // newly created objects should have this `namespaces` array in their return value const normalTypes = [ { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, ...fail409(!overwrite && spaceId === DEFAULT_SPACE_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_1, + ...fail409(!overwrite && spaceId === SPACE_1_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + ...fail409(!overwrite && spaceId === SPACE_2_ID), + expectedNamespaces, }, - { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail409(!overwrite && spaceId === SPACE_1_ID) }, - { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail409(!overwrite && spaceId === SPACE_2_ID) }, { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, ...fail409(!overwrite || (spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID)), @@ -38,8 +48,8 @@ const createTestCases = (overwrite: boolean, spaceId: string) => { { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail409(!overwrite || spaceId !== SPACE_1_ID) }, { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail409(!overwrite || spaceId !== SPACE_2_ID) }, { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, - CASES.NEW_SINGLE_NAMESPACE_OBJ, - CASES.NEW_MULTI_NAMESPACE_OBJ, + { ...CASES.NEW_SINGLE_NAMESPACE_OBJ, expectedNamespaces }, + { ...CASES.NEW_MULTI_NAMESPACE_OBJ, expectedNamespaces }, CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; @@ -53,15 +63,15 @@ export default function ({ getService }: FtrProviderContext) { const es = getService('legacyEs'); const { addTests, createTestDefinitions } = createTestSuiteFactory(es, esArchiver, supertest); - const createTests = (overwrite: boolean, spaceId: string) => { + const createTests = (overwrite: boolean, spaceId: string, user: TestUser) => { const { normalTypes, hiddenType, allTypes } = createTestCases(overwrite, spaceId); return { - unauthorized: createTestDefinitions(allTypes, true, overwrite, { spaceId }), + unauthorized: createTestDefinitions(allTypes, true, overwrite, { spaceId, user }), authorized: [ - createTestDefinitions(normalTypes, false, overwrite, { spaceId }), - createTestDefinitions(hiddenType, true, overwrite, { spaceId }), + createTestDefinitions(normalTypes, false, overwrite, { spaceId, user }), + createTestDefinitions(hiddenType, true, overwrite, { spaceId, user }), ].flat(), - superuser: createTestDefinitions(allTypes, false, overwrite, { spaceId }), + superuser: createTestDefinitions(allTypes, false, overwrite, { spaceId, user }), }; }; @@ -69,7 +79,6 @@ export default function ({ getService }: FtrProviderContext) { getTestScenarios([false, true]).securityAndSpaces.forEach( ({ spaceId, users, modifier: overwrite }) => { const suffix = ` within the ${spaceId} space${overwrite ? ' with overwrite enabled' : ''}`; - const { unauthorized, authorized, superuser } = createTests(overwrite!, spaceId); const _addTests = (user: TestUser, tests: CreateTestDefinition[]) => { addTests(`${user.description}${suffix}`, { user, spaceId, tests }); }; @@ -82,11 +91,14 @@ export default function ({ getService }: FtrProviderContext) { users.readAtSpace, users.allAtOtherSpace, ].forEach((user) => { + const { unauthorized } = createTests(overwrite!, spaceId, user); _addTests(user, unauthorized); }); [users.dualAll, users.allGlobally, users.allAtSpace].forEach((user) => { + const { authorized } = createTests(overwrite!, spaceId, user); _addTests(user, authorized); }); + const { superuser } = createTests(overwrite!, spaceId, users.superuser); _addTests(users.superuser, superuser); } ); diff --git a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/export.ts b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/export.ts index c581a1757565e..be3906209032f 100644 --- a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/export.ts +++ b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/export.ts @@ -15,17 +15,23 @@ import { const createTestCases = (spaceId: string) => { const cases = getTestCases(spaceId); - const exportableTypes = [ + const exportableObjects = [ cases.singleNamespaceObject, - cases.singleNamespaceType, cases.multiNamespaceObject, - cases.multiNamespaceType, cases.namespaceAgnosticObject, + ]; + const exportableTypes = [ + cases.singleNamespaceType, + cases.multiNamespaceType, cases.namespaceAgnosticType, ]; - const nonExportableTypes = [cases.hiddenObject, cases.hiddenType]; - const allTypes = exportableTypes.concat(nonExportableTypes); - return { exportableTypes, nonExportableTypes, allTypes }; + const nonExportableObjectsAndTypes = [cases.hiddenObject, cases.hiddenType]; + const allObjectsAndTypes = [ + exportableObjects, + exportableTypes, + nonExportableObjectsAndTypes, + ].flat(); + return { exportableObjects, exportableTypes, nonExportableObjectsAndTypes, allObjectsAndTypes }; }; export default function ({ getService }: FtrProviderContext) { @@ -34,13 +40,19 @@ export default function ({ getService }: FtrProviderContext) { const { addTests, createTestDefinitions } = exportTestSuiteFactory(esArchiver, supertest); const createTests = (spaceId: string) => { - const { exportableTypes, nonExportableTypes, allTypes } = createTestCases(spaceId); + const { + exportableObjects, + exportableTypes, + nonExportableObjectsAndTypes, + allObjectsAndTypes, + } = createTestCases(spaceId); return { unauthorized: [ - createTestDefinitions(exportableTypes, true), - createTestDefinitions(nonExportableTypes, false), + createTestDefinitions(exportableObjects, { statusCode: 403, reason: 'unauthorized' }), + createTestDefinitions(exportableTypes, { statusCode: 200, reason: 'unauthorized' }), // failure with empty result + createTestDefinitions(nonExportableObjectsAndTypes, false), ].flat(), - authorized: createTestDefinitions(allTypes, false), + authorized: createTestDefinitions(allObjectsAndTypes, false), }; }; diff --git a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/find.ts b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/find.ts index 6ac77507df473..afd4783fab792 100644 --- a/x-pack/test/saved_object_api_integration/security_and_spaces/apis/find.ts +++ b/x-pack/test/saved_object_api_integration/security_and_spaces/apis/find.ts @@ -4,18 +4,30 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getTestScenarios } from '../../common/lib/saved_object_test_utils'; +import { SPACES } from '../../common/lib/spaces'; +import { AUTHENTICATION } from '../../common/lib/authentication'; +import { + getTestScenarios, + isUserAuthorizedAtSpace, +} from '../../common/lib/saved_object_test_utils'; import { TestUser } from '../../common/lib/types'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { findTestSuiteFactory, getTestCases } from '../../common/suites/find'; -const createTestCases = (currentSpace: string, crossSpaceSearch: string[]) => { +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, + SPACE_1: { spaceId: SPACE_1_ID }, + SPACE_2: { spaceId: SPACE_2_ID }, +} = SPACES; + +const createTestCases = (currentSpace: string, crossSpaceSearch?: string[]) => { const cases = getTestCases({ currentSpace, crossSpaceSearch }); const normalTypes = [ cases.singleNamespaceType, cases.multiNamespaceType, cases.namespaceAgnosticType, + cases.eachType, cases.pageBeyondTotal, cases.unknownSearchField, cases.filterWithNamespaceAgnosticType, @@ -37,89 +49,72 @@ export default function ({ getService }: FtrProviderContext) { const { addTests, createTestDefinitions } = findTestSuiteFactory(esArchiver, supertest); const createTests = (spaceId: string, user: TestUser) => { - const currentSpaceCases = createTestCases(spaceId, []); + const currentSpaceCases = createTestCases(spaceId); - const explicitCrossSpace = createTestCases(spaceId, ['default', 'space_1', 'space_2']); + const EACH_SPACE = [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID]; + const explicitCrossSpace = createTestCases(spaceId, EACH_SPACE); const wildcardCrossSpace = createTestCases(spaceId, ['*']); - if (user.username === 'elastic') { + if (user.username === AUTHENTICATION.SUPERUSER.username) { return { currentSpace: createTestDefinitions(currentSpaceCases.allTypes, false, { user }), - crossSpace: createTestDefinitions(explicitCrossSpace.allTypes, false, { user }), + crossSpace: [ + createTestDefinitions(explicitCrossSpace.allTypes, false, { user }), + createTestDefinitions(wildcardCrossSpace.allTypes, false, { user }), + ].flat(), }; } - const authorizedAtCurrentSpace = - user.authorizedAtSpaces.includes(spaceId) || user.authorizedAtSpaces.includes('*'); - - const authorizedExplicitCrossSpaces = ['default', 'space_1', 'space_2'].filter( - (s) => - user.authorizedAtSpaces.includes('*') || - (s !== spaceId && user.authorizedAtSpaces.includes(s)) + const isAuthorizedExplicitCrossSpaces = EACH_SPACE.some( + (s) => s !== spaceId && isUserAuthorizedAtSpace(user, s) ); - - const authorizedWildcardCrossSpaces = ['default', 'space_1', 'space_2'].filter( - (s) => user.authorizedAtSpaces.includes('*') || user.authorizedAtSpaces.includes(s) + const isAuthorizedWildcardCrossSpaces = EACH_SPACE.some((s) => + isUserAuthorizedAtSpace(user, s) ); - const explicitCrossSpaceDefinitions = - authorizedExplicitCrossSpaces.length > 0 - ? [ - createTestDefinitions(explicitCrossSpace.normalTypes, false, { user }), - createTestDefinitions( - explicitCrossSpace.hiddenAndUnknownTypes, - { - statusCode: 403, - reason: 'forbidden_types', - }, - { user } - ), - ].flat() - : createTestDefinitions( - explicitCrossSpace.allTypes, - { - statusCode: 403, - reason: 'forbidden_namespaces', - }, + const explicitCrossSpaceDefinitions = isAuthorizedExplicitCrossSpaces + ? [ + createTestDefinitions(explicitCrossSpace.normalTypes, false, { user }), + createTestDefinitions( + explicitCrossSpace.hiddenAndUnknownTypes, + { statusCode: 200, reason: 'unauthorized' }, { user } - ); - - const wildcardCrossSpaceDefinitions = - authorizedWildcardCrossSpaces.length > 0 - ? [ - createTestDefinitions(wildcardCrossSpace.normalTypes, false, { user }), - createTestDefinitions( - wildcardCrossSpace.hiddenAndUnknownTypes, - { - statusCode: 403, - reason: 'forbidden_types', - }, - { user } - ), - ].flat() - : createTestDefinitions( - wildcardCrossSpace.allTypes, - { - statusCode: 403, - reason: 'forbidden_namespaces', - }, + ), + ].flat() + : createTestDefinitions( + explicitCrossSpace.allTypes, + { statusCode: 200, reason: 'unauthorized' }, + { user } + ); + const wildcardCrossSpaceDefinitions = isAuthorizedWildcardCrossSpaces + ? [ + createTestDefinitions(wildcardCrossSpace.normalTypes, false, { user }), + createTestDefinitions( + wildcardCrossSpace.hiddenAndUnknownTypes, + { statusCode: 200, reason: 'unauthorized' }, { user } - ); + ), + ].flat() + : createTestDefinitions( + wildcardCrossSpace.allTypes, + { statusCode: 200, reason: 'unauthorized' }, + { user } + ); return { - currentSpace: authorizedAtCurrentSpace + currentSpace: isUserAuthorizedAtSpace(user, spaceId) ? [ createTestDefinitions(currentSpaceCases.normalTypes, false, { user, }), createTestDefinitions(currentSpaceCases.hiddenAndUnknownTypes, { - statusCode: 403, - reason: 'forbidden_types', + statusCode: 200, + reason: 'unauthorized', }), ].flat() : createTestDefinitions(currentSpaceCases.allTypes, { - statusCode: 403, - reason: 'forbidden_types', + statusCode: 200, + reason: 'unauthorized', }), crossSpace: [...explicitCrossSpaceDefinitions, ...wildcardCrossSpaceDefinitions], }; diff --git a/x-pack/test/saved_object_api_integration/security_only/apis/bulk_create.ts b/x-pack/test/saved_object_api_integration/security_only/apis/bulk_create.ts index 725120687c231..cc2c5e2e7fc00 100644 --- a/x-pack/test/saved_object_api_integration/security_only/apis/bulk_create.ts +++ b/x-pack/test/saved_object_api_integration/security_only/apis/bulk_create.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,22 +14,26 @@ import { BulkCreateTestDefinition, } from '../../common/suites/bulk_create'; +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, +} = SPACES; const { fail400, fail409 } = testCaseFailures; const unresolvableConflict = () => ({ fail409Param: 'unresolvableConflict' }); const createTestCases = (overwrite: boolean) => { // for each permitted (non-403) outcome, if failure !== undefined then we expect // to receive an error; otherwise, we expect to receive a success result + const expectedNamespaces = [DEFAULT_SPACE_ID]; // newly created objects should have this `namespaces` array in their return value const normalTypes = [ { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, ...fail409(!overwrite) }, - CASES.SINGLE_NAMESPACE_SPACE_1, - CASES.SINGLE_NAMESPACE_SPACE_2, + { ...CASES.SINGLE_NAMESPACE_SPACE_1, expectedNamespaces }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, expectedNamespaces }, { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, ...fail409(!overwrite) }, { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail409(), ...unresolvableConflict() }, { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail409(), ...unresolvableConflict() }, { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, - CASES.NEW_SINGLE_NAMESPACE_OBJ, - CASES.NEW_MULTI_NAMESPACE_OBJ, + { ...CASES.NEW_SINGLE_NAMESPACE_OBJ, expectedNamespaces }, + { ...CASES.NEW_MULTI_NAMESPACE_OBJ, expectedNamespaces }, CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; @@ -46,27 +51,27 @@ export default function ({ getService }: FtrProviderContext) { esArchiver, supertest ); - const createTests = (overwrite: boolean) => { + const createTests = (overwrite: boolean, user: TestUser) => { const { normalTypes, hiddenType, allTypes } = createTestCases(overwrite); // use singleRequest to reduce execution time and/or test combined cases return { - unauthorized: createTestDefinitions(allTypes, true, overwrite), + unauthorized: createTestDefinitions(allTypes, true, overwrite, { user }), authorized: [ - createTestDefinitions(normalTypes, false, overwrite, { singleRequest: true }), - createTestDefinitions(hiddenType, true, overwrite), + createTestDefinitions(normalTypes, false, overwrite, { user, singleRequest: true }), + createTestDefinitions(hiddenType, true, overwrite, { user }), createTestDefinitions(allTypes, true, overwrite, { + user, singleRequest: true, responseBodyOverride: expectForbidden(['hiddentype']), }), ].flat(), - superuser: createTestDefinitions(allTypes, false, overwrite, { singleRequest: true }), + superuser: createTestDefinitions(allTypes, false, overwrite, { user, singleRequest: true }), }; }; describe('_bulk_create', () => { getTestScenarios([false, true]).security.forEach(({ users, modifier: overwrite }) => { const suffix = overwrite ? ' with overwrite enabled' : ''; - const { unauthorized, authorized, superuser } = createTests(overwrite!); const _addTests = (user: TestUser, tests: BulkCreateTestDefinition[]) => { addTests(`${user.description}${suffix}`, { user, tests }); }; @@ -81,11 +86,14 @@ export default function ({ getService }: FtrProviderContext) { users.allAtSpace1, users.readAtSpace1, ].forEach((user) => { + const { unauthorized } = createTests(overwrite!, user); _addTests(user, unauthorized); }); [users.dualAll, users.allGlobally].forEach((user) => { + const { authorized } = createTests(overwrite!, user); _addTests(user, authorized); }); + const { superuser } = createTests(overwrite!, users.superuser); _addTests(users.superuser, superuser); }); }); diff --git a/x-pack/test/saved_object_api_integration/security_only/apis/create.ts b/x-pack/test/saved_object_api_integration/security_only/apis/create.ts index 88d096f05d846..b7c6ecef979bd 100644 --- a/x-pack/test/saved_object_api_integration/security_only/apis/create.ts +++ b/x-pack/test/saved_object_api_integration/security_only/apis/create.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,21 +14,25 @@ import { CreateTestDefinition, } from '../../common/suites/create'; +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, +} = SPACES; const { fail400, fail409 } = testCaseFailures; const createTestCases = (overwrite: boolean) => { // for each permitted (non-403) outcome, if failure !== undefined then we expect // to receive an error; otherwise, we expect to receive a success result + const expectedNamespaces = [DEFAULT_SPACE_ID]; // newly created objects should have this `namespaces` array in their return value const normalTypes = [ { ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, ...fail409(!overwrite) }, - CASES.SINGLE_NAMESPACE_SPACE_1, - CASES.SINGLE_NAMESPACE_SPACE_2, + { ...CASES.SINGLE_NAMESPACE_SPACE_1, expectedNamespaces }, + { ...CASES.SINGLE_NAMESPACE_SPACE_2, expectedNamespaces }, { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, ...fail409(!overwrite) }, { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail409() }, { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail409() }, { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, - CASES.NEW_SINGLE_NAMESPACE_OBJ, - CASES.NEW_MULTI_NAMESPACE_OBJ, + { ...CASES.NEW_SINGLE_NAMESPACE_OBJ, expectedNamespaces }, + { ...CASES.NEW_MULTI_NAMESPACE_OBJ, expectedNamespaces }, CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, ]; const hiddenType = [{ ...CASES.HIDDEN, ...fail400() }]; @@ -41,22 +46,21 @@ export default function ({ getService }: FtrProviderContext) { const es = getService('legacyEs'); const { addTests, createTestDefinitions } = createTestSuiteFactory(es, esArchiver, supertest); - const createTests = (overwrite: boolean) => { + const createTests = (overwrite: boolean, user: TestUser) => { const { normalTypes, hiddenType, allTypes } = createTestCases(overwrite); return { - unauthorized: createTestDefinitions(allTypes, true, overwrite), + unauthorized: createTestDefinitions(allTypes, true, overwrite, { user }), authorized: [ - createTestDefinitions(normalTypes, false, overwrite), - createTestDefinitions(hiddenType, true, overwrite), + createTestDefinitions(normalTypes, false, overwrite, { user }), + createTestDefinitions(hiddenType, true, overwrite, { user }), ].flat(), - superuser: createTestDefinitions(allTypes, false, overwrite), + superuser: createTestDefinitions(allTypes, false, overwrite, { user }), }; }; describe('_create', () => { getTestScenarios([false, true]).security.forEach(({ users, modifier: overwrite }) => { const suffix = overwrite ? ' with overwrite enabled' : ''; - const { unauthorized, authorized, superuser } = createTests(overwrite!); const _addTests = (user: TestUser, tests: CreateTestDefinition[]) => { addTests(`${user.description}${suffix}`, { user, tests }); }; @@ -71,11 +75,14 @@ export default function ({ getService }: FtrProviderContext) { users.allAtSpace1, users.readAtSpace1, ].forEach((user) => { + const { unauthorized } = createTests(overwrite!, user); _addTests(user, unauthorized); }); [users.dualAll, users.allGlobally].forEach((user) => { + const { authorized } = createTests(overwrite!, user); _addTests(user, authorized); }); + const { superuser } = createTests(overwrite!, users.superuser); _addTests(users.superuser, superuser); }); }); diff --git a/x-pack/test/saved_object_api_integration/security_only/apis/export.ts b/x-pack/test/saved_object_api_integration/security_only/apis/export.ts index 99babf683ccfa..ea1ed56921d22 100644 --- a/x-pack/test/saved_object_api_integration/security_only/apis/export.ts +++ b/x-pack/test/saved_object_api_integration/security_only/apis/export.ts @@ -15,17 +15,23 @@ import { const createTestCases = () => { const cases = getTestCases(); - const exportableTypes = [ + const exportableObjects = [ cases.singleNamespaceObject, - cases.singleNamespaceType, cases.multiNamespaceObject, - cases.multiNamespaceType, cases.namespaceAgnosticObject, + ]; + const exportableTypes = [ + cases.singleNamespaceType, + cases.multiNamespaceType, cases.namespaceAgnosticType, ]; - const nonExportableTypes = [cases.hiddenObject, cases.hiddenType]; - const allTypes = exportableTypes.concat(nonExportableTypes); - return { exportableTypes, nonExportableTypes, allTypes }; + const nonExportableObjectsAndTypes = [cases.hiddenObject, cases.hiddenType]; + const allObjectsAndTypes = [ + exportableObjects, + exportableTypes, + nonExportableObjectsAndTypes, + ].flat(); + return { exportableObjects, exportableTypes, nonExportableObjectsAndTypes, allObjectsAndTypes }; }; export default function ({ getService }: FtrProviderContext) { @@ -34,13 +40,19 @@ export default function ({ getService }: FtrProviderContext) { const { addTests, createTestDefinitions } = exportTestSuiteFactory(esArchiver, supertest); const createTests = () => { - const { exportableTypes, nonExportableTypes, allTypes } = createTestCases(); + const { + exportableObjects, + exportableTypes, + nonExportableObjectsAndTypes, + allObjectsAndTypes, + } = createTestCases(); return { unauthorized: [ - createTestDefinitions(exportableTypes, true), - createTestDefinitions(nonExportableTypes, false), + createTestDefinitions(exportableObjects, { statusCode: 403, reason: 'unauthorized' }), + createTestDefinitions(exportableTypes, { statusCode: 200, reason: 'unauthorized' }), // failure with empty result + createTestDefinitions(nonExportableObjectsAndTypes, false), ].flat(), - authorized: createTestDefinitions(allTypes, false), + authorized: createTestDefinitions(allObjectsAndTypes, false), }; }; diff --git a/x-pack/test/saved_object_api_integration/security_only/apis/find.ts b/x-pack/test/saved_object_api_integration/security_only/apis/find.ts index 3a435119436ca..aa18f32600949 100644 --- a/x-pack/test/saved_object_api_integration/security_only/apis/find.ts +++ b/x-pack/test/saved_object_api_integration/security_only/apis/find.ts @@ -4,18 +4,27 @@ * you may not use this file except in compliance with the Elastic License. */ +import { SPACES } from '../../common/lib/spaces'; +import { AUTHENTICATION } from '../../common/lib/authentication'; import { getTestScenarios } from '../../common/lib/saved_object_test_utils'; import { TestUser } from '../../common/lib/types'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { findTestSuiteFactory, getTestCases } from '../../common/suites/find'; -const createTestCases = (crossSpaceSearch: string[]) => { +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, + SPACE_1: { spaceId: SPACE_1_ID }, + SPACE_2: { spaceId: SPACE_2_ID }, +} = SPACES; + +const createTestCases = (crossSpaceSearch?: string[]) => { const cases = getTestCases({ crossSpaceSearch }); const normalTypes = [ cases.singleNamespaceType, cases.multiNamespaceType, cases.namespaceAgnosticType, + cases.eachType, cases.pageBeyondTotal, cases.unknownSearchField, cases.filterWithNamespaceAgnosticType, @@ -37,46 +46,35 @@ export default function ({ getService }: FtrProviderContext) { const { addTests, createTestDefinitions } = findTestSuiteFactory(esArchiver, supertest); const createTests = (user: TestUser) => { - const defaultCases = createTestCases([]); - const crossSpaceCases = createTestCases(['default', 'space_1', 'space_2']); + const defaultCases = createTestCases(); + const crossSpaceCases = createTestCases([DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID]); - if (user.username === 'elastic') { + if (user.username === AUTHENTICATION.SUPERUSER.username) { return { defaultCases: createTestDefinitions(defaultCases.allTypes, false, { user }), crossSpace: createTestDefinitions( crossSpaceCases.allTypes, - { - statusCode: 400, - reason: 'cross_namespace_not_permitted', - }, + { statusCode: 400, reason: 'cross_namespace_not_permitted' }, { user } ), }; } - const authorizedGlobally = user.authorizedAtSpaces.includes('*'); + const isAuthorizedGlobally = user.authorizedAtSpaces.includes('*'); return { - defaultCases: authorizedGlobally + defaultCases: isAuthorizedGlobally ? [ - createTestDefinitions(defaultCases.normalTypes, false, { - user, - }), + createTestDefinitions(defaultCases.normalTypes, false, { user }), createTestDefinitions(defaultCases.hiddenAndUnknownTypes, { - statusCode: 403, - reason: 'forbidden_types', + statusCode: 200, + reason: 'unauthorized', }), ].flat() - : createTestDefinitions(defaultCases.allTypes, { - statusCode: 403, - reason: 'forbidden_types', - }), + : createTestDefinitions(defaultCases.allTypes, { statusCode: 200, reason: 'unauthorized' }), crossSpace: createTestDefinitions( crossSpaceCases.allTypes, - { - statusCode: 400, - reason: 'cross_namespace_not_permitted', - }, + { statusCode: 400, reason: 'cross_namespace_not_permitted' }, { user } ), }; diff --git a/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_create.ts b/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_create.ts index 74fade39bf7a5..ef47b09eddbc8 100644 --- a/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_create.ts +++ b/x-pack/test/saved_object_api_integration/spaces_only/apis/bulk_create.ts @@ -19,36 +19,48 @@ const { fail400, fail409 } = testCaseFailures; const unresolvableConflict = (condition?: boolean) => condition !== false ? { fail409Param: 'unresolvableConflict' } : {}; -const createTestCases = (overwrite: boolean, spaceId: string) => [ +const createTestCases = (overwrite: boolean, 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, - ...fail409(!overwrite && spaceId === DEFAULT_SPACE_ID), - }, - { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail409(!overwrite && spaceId === SPACE_1_ID) }, - { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail409(!overwrite && spaceId === SPACE_2_ID) }, - { - ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, - ...fail409(!overwrite || (spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID)), - ...unresolvableConflict(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID), - }, - { - ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, - ...fail409(!overwrite || spaceId !== SPACE_1_ID), - ...unresolvableConflict(spaceId !== SPACE_1_ID), - }, - { - ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, - ...fail409(!overwrite || spaceId !== SPACE_2_ID), - ...unresolvableConflict(spaceId !== SPACE_2_ID), - }, - { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, - { ...CASES.HIDDEN, ...fail400() }, - CASES.NEW_SINGLE_NAMESPACE_OBJ, - CASES.NEW_MULTI_NAMESPACE_OBJ, - CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, -]; + const expectedNamespaces = [spaceId]; // newly created objects should have this `namespaces` array in their return value + return [ + { + ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, + ...fail409(!overwrite && spaceId === DEFAULT_SPACE_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_1, + ...fail409(!overwrite && spaceId === SPACE_1_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + ...fail409(!overwrite && spaceId === SPACE_2_ID), + expectedNamespaces, + }, + { + ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, + ...fail409(!overwrite || (spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID)), + ...unresolvableConflict(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID), + }, + { + ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, + ...fail409(!overwrite || spaceId !== SPACE_1_ID), + ...unresolvableConflict(spaceId !== SPACE_1_ID), + }, + { + ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, + ...fail409(!overwrite || spaceId !== SPACE_2_ID), + ...unresolvableConflict(spaceId !== SPACE_2_ID), + }, + { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, + { ...CASES.HIDDEN, ...fail400() }, + { ...CASES.NEW_SINGLE_NAMESPACE_OBJ, expectedNamespaces }, + { ...CASES.NEW_MULTI_NAMESPACE_OBJ, expectedNamespaces }, + CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, + ]; +}; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); diff --git a/x-pack/test/saved_object_api_integration/spaces_only/apis/create.ts b/x-pack/test/saved_object_api_integration/spaces_only/apis/create.ts index 1040f7fd81dde..10e57b4db82dc 100644 --- a/x-pack/test/saved_object_api_integration/spaces_only/apis/create.ts +++ b/x-pack/test/saved_object_api_integration/spaces_only/apis/create.ts @@ -16,27 +16,39 @@ const { } = SPACES; const { fail400, fail409 } = testCaseFailures; -const createTestCases = (overwrite: boolean, spaceId: string) => [ +const createTestCases = (overwrite: boolean, 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, - ...fail409(!overwrite && spaceId === DEFAULT_SPACE_ID), - }, - { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail409(!overwrite && spaceId === SPACE_1_ID) }, - { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail409(!overwrite && spaceId === SPACE_2_ID) }, - { - ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, - ...fail409(!overwrite || (spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID)), - }, - { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail409(!overwrite || spaceId !== SPACE_1_ID) }, - { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail409(!overwrite || spaceId !== SPACE_2_ID) }, - { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, - { ...CASES.HIDDEN, ...fail400() }, - CASES.NEW_SINGLE_NAMESPACE_OBJ, - CASES.NEW_MULTI_NAMESPACE_OBJ, - CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, -]; + const expectedNamespaces = [spaceId]; // newly created objects should have this `namespaces` array in their return value + return [ + { + ...CASES.SINGLE_NAMESPACE_DEFAULT_SPACE, + ...fail409(!overwrite && spaceId === DEFAULT_SPACE_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_1, + ...fail409(!overwrite && spaceId === SPACE_1_ID), + expectedNamespaces, + }, + { + ...CASES.SINGLE_NAMESPACE_SPACE_2, + ...fail409(!overwrite && spaceId === SPACE_2_ID), + expectedNamespaces, + }, + { + ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, + ...fail409(!overwrite || (spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID)), + }, + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail409(!overwrite || spaceId !== SPACE_1_ID) }, + { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail409(!overwrite || spaceId !== SPACE_2_ID) }, + { ...CASES.NAMESPACE_AGNOSTIC, ...fail409(!overwrite) }, + { ...CASES.HIDDEN, ...fail400() }, + { ...CASES.NEW_SINGLE_NAMESPACE_OBJ, expectedNamespaces }, + { ...CASES.NEW_MULTI_NAMESPACE_OBJ, expectedNamespaces }, + CASES.NEW_NAMESPACE_AGNOSTIC_OBJ, + ]; +}; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertestWithoutAuth'); diff --git a/x-pack/test/saved_object_api_integration/spaces_only/apis/find.ts b/x-pack/test/saved_object_api_integration/spaces_only/apis/find.ts index 1d46985916cd5..c6779402d3291 100644 --- a/x-pack/test/saved_object_api_integration/spaces_only/apis/find.ts +++ b/x-pack/test/saved_object_api_integration/spaces_only/apis/find.ts @@ -4,11 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ +import { SPACES } from '../../common/lib/spaces'; import { getTestScenarios } from '../../common/lib/saved_object_test_utils'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { findTestSuiteFactory, getTestCases } from '../../common/suites/find'; -const createTestCases = (spaceId: string, crossSpaceSearch: string[]) => { +const { + DEFAULT: { spaceId: DEFAULT_SPACE_ID }, + SPACE_1: { spaceId: SPACE_1_ID }, + SPACE_2: { spaceId: SPACE_2_ID }, +} = SPACES; + +const createTestCases = (spaceId: string, crossSpaceSearch?: string[]) => { const cases = getTestCases({ currentSpace: spaceId, crossSpaceSearch }); return Object.values(cases); }; @@ -18,15 +25,19 @@ export default function ({ getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const { addTests, createTestDefinitions } = findTestSuiteFactory(esArchiver, supertest); - const createTests = (spaceId: string, crossSpaceSearch: string[]) => { + const createTests = (spaceId: string, crossSpaceSearch?: string[]) => { const testCases = createTestCases(spaceId, crossSpaceSearch); return createTestDefinitions(testCases, false); }; describe('_find', () => { getTestScenarios().spaces.forEach(({ spaceId }) => { - const currentSpaceTests = createTests(spaceId, []); - const explicitCrossSpaceTests = createTests(spaceId, ['default', 'space_1', 'space_2']); + const currentSpaceTests = createTests(spaceId); + const explicitCrossSpaceTests = createTests(spaceId, [ + DEFAULT_SPACE_ID, + SPACE_1_ID, + SPACE_2_ID, + ]); const wildcardCrossSpaceTests = createTests(spaceId, ['*']); addTests(`within the ${spaceId} space`, { spaceId, From 0238206ace13c57cd5de58d0d0b14df38d36043b Mon Sep 17 00:00:00 2001 From: Luca Belluccini Date: Tue, 22 Sep 2020 18:44:30 +0200 Subject: [PATCH 2/3] [DOC] Clarify supported realms when accessing remote monitoring clusters (#77938) Co-authored-by: lcawl --- docs/user/monitoring/viewing-metrics.asciidoc | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/user/monitoring/viewing-metrics.asciidoc b/docs/user/monitoring/viewing-metrics.asciidoc index f35caea025cdd..0c48e3b7d011d 100644 --- a/docs/user/monitoring/viewing-metrics.asciidoc +++ b/docs/user/monitoring/viewing-metrics.asciidoc @@ -13,13 +13,19 @@ At a minimum, you must have monitoring data for the {es} production cluster. Once that data exists, {kib} can display monitoring data for other products in the cluster. +TIP: If you use a separate monitoring cluster to store the monitoring data, it +is strongly recommended that you use a separate {kib} instance to view it. If +you log in to {kib} using SAML, Kerberos, PKI, OpenID Connect, or token +authentication providers, a dedicated {kib} instance is *required*. The security +tokens that are used in these contexts are cluster-specific, therefore you +cannot use a single {kib} instance to connect to both production and monitoring +clusters. For more information about the recommended configuration, see +{ref}/monitoring-overview.html[Monitoring overview]. + . Identify where to retrieve monitoring data from. + -- -The cluster that contains the monitoring data is referred to -as the _monitoring cluster_. - -TIP: If the monitoring data is stored on a *dedicated* monitoring cluster, it is +If the monitoring data is stored on a dedicated monitoring cluster, it is accessible even when the cluster you're monitoring is not. If you have at least a gold license, you can send data from multiple clusters to the same monitoring cluster and view them all through the same instance of {kib}. From 311805a57d8109832648db6e3b7fe4143e18e030 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com> Date: Tue, 22 Sep 2020 12:50:44 -0400 Subject: [PATCH 3/3] [Ingest Manager] Adding bulk packages upgrade api (#77827) * Adding bulk upgrade api * Addressing comments * Removing todo * Changing body field * Adding helper for getting the bulk install route * Adding request spec * Pulling in Johns changes * Removing test for same package upgraded multiple times * Pulling in John's error handling changes * Fixing type error --- .../ingest_manager/common/constants/routes.ts | 2 + .../ingest_manager/common/services/routes.ts | 4 + .../common/types/rest_spec/epm.ts | 24 +++ .../ingest_manager/server/errors/handlers.ts | 29 +-- .../ingest_manager/server/errors/index.ts | 2 +- .../server/routes/epm/handlers.ts | 62 +++--- .../ingest_manager/server/routes/epm/index.ts | 11 + .../server/services/epm/packages/install.ts | 188 +++++++++++++++++- .../server/types/rest_spec/epm.ts | 6 + .../apis/epm/bulk_upgrade.ts | 113 +++++++++++ .../apis/epm/index.js | 1 + 11 files changed, 394 insertions(+), 48 deletions(-) create mode 100644 x-pack/test/ingest_manager_api_integration/apis/epm/bulk_upgrade.ts diff --git a/x-pack/plugins/ingest_manager/common/constants/routes.ts b/x-pack/plugins/ingest_manager/common/constants/routes.ts index 3e065142ea101..378a6c6c12159 100644 --- a/x-pack/plugins/ingest_manager/common/constants/routes.ts +++ b/x-pack/plugins/ingest_manager/common/constants/routes.ts @@ -15,9 +15,11 @@ export const LIMITED_CONCURRENCY_ROUTE_TAG = 'ingest:limited-concurrency'; // EPM API routes const EPM_PACKAGES_MANY = `${EPM_API_ROOT}/packages`; +const EPM_PACKAGES_BULK = `${EPM_PACKAGES_MANY}/_bulk`; const EPM_PACKAGES_ONE = `${EPM_PACKAGES_MANY}/{pkgkey}`; const EPM_PACKAGES_FILE = `${EPM_PACKAGES_MANY}/{pkgName}/{pkgVersion}`; export const EPM_API_ROUTES = { + BULK_INSTALL_PATTERN: EPM_PACKAGES_BULK, LIST_PATTERN: EPM_PACKAGES_MANY, LIMITED_LIST_PATTERN: `${EPM_PACKAGES_MANY}/limited`, INFO_PATTERN: EPM_PACKAGES_ONE, diff --git a/x-pack/plugins/ingest_manager/common/services/routes.ts b/x-pack/plugins/ingest_manager/common/services/routes.ts index b7521f95b4f83..ec7c0ee850834 100644 --- a/x-pack/plugins/ingest_manager/common/services/routes.ts +++ b/x-pack/plugins/ingest_manager/common/services/routes.ts @@ -46,6 +46,10 @@ export const epmRouteService = { ); // trim trailing slash }, + getBulkInstallPath: () => { + return EPM_API_ROUTES.BULK_INSTALL_PATTERN; + }, + getRemovePath: (pkgkey: string) => { return EPM_API_ROUTES.DELETE_PATTERN.replace('{pkgkey}', pkgkey).replace(/\/$/, ''); // trim trailing slash }, diff --git a/x-pack/plugins/ingest_manager/common/types/rest_spec/epm.ts b/x-pack/plugins/ingest_manager/common/types/rest_spec/epm.ts index 54e767fee4b22..7ed2fed91aa93 100644 --- a/x-pack/plugins/ingest_manager/common/types/rest_spec/epm.ts +++ b/x-pack/plugins/ingest_manager/common/types/rest_spec/epm.ts @@ -71,6 +71,30 @@ export interface InstallPackageResponse { response: AssetReference[]; } +export interface IBulkInstallPackageError { + name: string; + statusCode: number; + error: string | Error; +} + +export interface BulkInstallPackageInfo { + name: string; + newVersion: string; + // this will be null if no package was present before the upgrade (aka it was an install) + oldVersion: string | null; + assets: AssetReference[]; +} + +export interface BulkInstallPackagesResponse { + response: Array; +} + +export interface BulkInstallPackagesRequest { + body: { + packages: string[]; + }; +} + export interface MessageResponse { response: string; } diff --git a/x-pack/plugins/ingest_manager/server/errors/handlers.ts b/x-pack/plugins/ingest_manager/server/errors/handlers.ts index 9f776565cf262..b621f2dd29331 100644 --- a/x-pack/plugins/ingest_manager/server/errors/handlers.ts +++ b/x-pack/plugins/ingest_manager/server/errors/handlers.ts @@ -56,10 +56,7 @@ const getHTTPResponseCode = (error: IngestManagerError): number => { return 400; // Bad Request }; -export const defaultIngestErrorHandler: IngestErrorHandler = async ({ - error, - response, -}: IngestErrorHandlerParams): Promise => { +export function ingestErrorToResponseOptions(error: IngestErrorHandlerParams['error']) { const logger = appContextService.getLogger(); if (isLegacyESClientError(error)) { // there was a problem communicating with ES (e.g. via `callCluster`) @@ -72,36 +69,44 @@ export const defaultIngestErrorHandler: IngestErrorHandler = async ({ logger.error(message); - return response.customError({ + return { statusCode: error?.statusCode || error.status, body: { message }, - }); + }; } // our "expected" errors if (error instanceof IngestManagerError) { // only log the message logger.error(error.message); - return response.customError({ + return { statusCode: getHTTPResponseCode(error), body: { message: error.message }, - }); + }; } // handle any older Boom-based errors or the few places our app uses them if (isBoom(error)) { // only log the message logger.error(error.output.payload.message); - return response.customError({ + return { statusCode: error.output.statusCode, body: { message: error.output.payload.message }, - }); + }; } // not sure what type of error this is. log as much as possible logger.error(error); - return response.customError({ + return { statusCode: 500, body: { message: error.message }, - }); + }; +} + +export const defaultIngestErrorHandler: IngestErrorHandler = async ({ + error, + response, +}: IngestErrorHandlerParams): Promise => { + const options = ingestErrorToResponseOptions(error); + return response.customError(options); }; diff --git a/x-pack/plugins/ingest_manager/server/errors/index.ts b/x-pack/plugins/ingest_manager/server/errors/index.ts index 5e36a2ec9a884..f495bf551dcff 100644 --- a/x-pack/plugins/ingest_manager/server/errors/index.ts +++ b/x-pack/plugins/ingest_manager/server/errors/index.ts @@ -5,7 +5,7 @@ */ /* eslint-disable max-classes-per-file */ -export { defaultIngestErrorHandler } from './handlers'; +export { defaultIngestErrorHandler, ingestErrorToResponseOptions } from './handlers'; export class IngestManagerError extends Error { constructor(message?: string) { diff --git a/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts b/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts index c40e0e4ac5c0b..7ae896c1f30a6 100644 --- a/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts +++ b/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts @@ -5,7 +5,6 @@ */ import { TypeOf } from '@kbn/config-schema'; import { RequestHandler, CustomHttpResponseOptions } from 'src/core/server'; -import { appContextService } from '../../services'; import { GetInfoResponse, InstallPackageResponse, @@ -14,6 +13,7 @@ import { GetCategoriesResponse, GetPackagesResponse, GetLimitedPackagesResponse, + BulkInstallPackagesResponse, } from '../../../common'; import { GetCategoriesRequestSchema, @@ -23,6 +23,7 @@ import { InstallPackageFromRegistryRequestSchema, InstallPackageByUploadRequestSchema, DeletePackageRequestSchema, + BulkUpgradePackagesFromRegistryRequestSchema, } from '../../types'; import { getCategories, @@ -34,9 +35,12 @@ import { getLimitedPackages, getInstallationObject, } from '../../services/epm/packages'; -import { IngestManagerError, defaultIngestErrorHandler } from '../../errors'; +import { defaultIngestErrorHandler } from '../../errors'; import { splitPkgKey } from '../../services/epm/registry'; -import { getInstallType } from '../../services/epm/packages/install'; +import { + handleInstallPackageFailure, + bulkInstallPackages, +} from '../../services/epm/packages/install'; export const getCategoriesHandler: RequestHandler< undefined, @@ -136,13 +140,11 @@ export const installPackageFromRegistryHandler: RequestHandler< undefined, TypeOf > = async (context, request, response) => { - const logger = appContextService.getLogger(); const savedObjectsClient = context.core.savedObjects.client; const callCluster = context.core.elasticsearch.legacy.client.callAsCurrentUser; const { pkgkey } = request.params; const { pkgName, pkgVersion } = splitPkgKey(pkgkey); const installedPkg = await getInstallationObject({ savedObjectsClient, pkgName }); - const installType = getInstallType({ pkgVersion, installedPkg }); try { const res = await installPackage({ savedObjectsClient, @@ -155,36 +157,38 @@ export const installPackageFromRegistryHandler: RequestHandler< }; return response.ok({ body }); } catch (e) { - // could have also done `return defaultIngestErrorHandler({ error: e, response })` at each of the returns, - // but doing it this way will log the outer/install errors before any inner/rollback errors const defaultResult = await defaultIngestErrorHandler({ error: e, response }); - if (e instanceof IngestManagerError) { - return defaultResult; - } + await handleInstallPackageFailure({ + savedObjectsClient, + error: e, + pkgName, + pkgVersion, + installedPkg, + callCluster, + }); - // if there is an unknown server error, uninstall any package assets or reinstall the previous version if update - try { - if (installType === 'install' || installType === 'reinstall') { - logger.error(`uninstalling ${pkgkey} after error installing`); - await removeInstallation({ savedObjectsClient, pkgkey, callCluster }); - } - if (installType === 'update') { - // @ts-ignore getInstallType ensures we have installedPkg - const prevVersion = `${pkgName}-${installedPkg.attributes.version}`; - logger.error(`rolling back to ${prevVersion} after error installing ${pkgkey}`); - await installPackage({ - savedObjectsClient, - pkgkey: prevVersion, - callCluster, - }); - } - } catch (error) { - logger.error(`failed to uninstall or rollback package after installation error ${error}`); - } return defaultResult; } }; +export const bulkInstallPackagesFromRegistryHandler: RequestHandler< + undefined, + undefined, + TypeOf +> = async (context, request, response) => { + const savedObjectsClient = context.core.savedObjects.client; + const callCluster = context.core.elasticsearch.legacy.client.callAsCurrentUser; + const res = await bulkInstallPackages({ + savedObjectsClient, + callCluster, + packagesToUpgrade: request.body.packages, + }); + const body: BulkInstallPackagesResponse = { + response: res, + }; + return response.ok({ body }); +}; + export const installPackageByUploadHandler: RequestHandler< undefined, undefined, diff --git a/x-pack/plugins/ingest_manager/server/routes/epm/index.ts b/x-pack/plugins/ingest_manager/server/routes/epm/index.ts index 9048652f0e8a9..eaf61335b5e06 100644 --- a/x-pack/plugins/ingest_manager/server/routes/epm/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/epm/index.ts @@ -14,6 +14,7 @@ import { installPackageFromRegistryHandler, installPackageByUploadHandler, deletePackageHandler, + bulkInstallPackagesFromRegistryHandler, } from './handlers'; import { GetCategoriesRequestSchema, @@ -23,6 +24,7 @@ import { InstallPackageFromRegistryRequestSchema, InstallPackageByUploadRequestSchema, DeletePackageRequestSchema, + BulkUpgradePackagesFromRegistryRequestSchema, } from '../../types'; const MAX_FILE_SIZE_BYTES = 104857600; // 100MB @@ -82,6 +84,15 @@ export const registerRoutes = (router: IRouter) => { installPackageFromRegistryHandler ); + router.post( + { + path: EPM_API_ROUTES.BULK_INSTALL_PATTERN, + validate: BulkUpgradePackagesFromRegistryRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + bulkInstallPackagesFromRegistryHandler + ); + router.post( { path: EPM_API_ROUTES.INSTALL_BY_UPLOAD_PATTERN, diff --git a/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts b/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts index 54b9c4d3fbb17..800151a41a429 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts @@ -6,6 +6,9 @@ import { SavedObject, SavedObjectsClientContract } from 'src/core/server'; import semver from 'semver'; +import Boom from 'boom'; +import { UnwrapPromise } from '@kbn/utility-types'; +import { BulkInstallPackageInfo, IBulkInstallPackageError } from '../../../../common'; import { PACKAGES_SAVED_OBJECT_TYPE, MAX_TIME_COMPLETE_INSTALL } from '../../../constants'; import { AssetReference, @@ -32,10 +35,15 @@ import { ArchiveAsset, } from '../kibana/assets/install'; import { updateCurrentWriteIndices } from '../elasticsearch/template/template'; -import { deleteKibanaSavedObjectsAssets } from './remove'; -import { PackageOutdatedError } from '../../../errors'; +import { deleteKibanaSavedObjectsAssets, removeInstallation } from './remove'; +import { + IngestManagerError, + PackageOutdatedError, + ingestErrorToResponseOptions, +} from '../../../errors'; import { getPackageSavedObjects } from './get'; import { installTransformForDataset } from '../elasticsearch/transform/install'; +import { appContextService } from '../../app_context'; export async function installLatestPackage(options: { savedObjectsClient: SavedObjectsClientContract; @@ -94,17 +102,185 @@ export async function ensureInstalledPackage(options: { return installation; } -export async function installPackage({ +export async function handleInstallPackageFailure({ savedObjectsClient, - pkgkey, + error, + pkgName, + pkgVersion, + installedPkg, callCluster, - force = false, }: { + savedObjectsClient: SavedObjectsClientContract; + error: IngestManagerError | Boom | Error; + pkgName: string; + pkgVersion: string; + installedPkg: SavedObject | undefined; + callCluster: CallESAsCurrentUser; +}) { + if (error instanceof IngestManagerError) { + return; + } + const logger = appContextService.getLogger(); + const pkgkey = Registry.pkgToPkgKey({ + name: pkgName, + version: pkgVersion, + }); + + // if there is an unknown server error, uninstall any package assets or reinstall the previous version if update + try { + const installType = getInstallType({ pkgVersion, installedPkg }); + if (installType === 'install' || installType === 'reinstall') { + logger.error(`uninstalling ${pkgkey} after error installing`); + await removeInstallation({ savedObjectsClient, pkgkey, callCluster }); + } + + if (installType === 'update') { + if (!installedPkg) { + logger.error( + `failed to rollback package after installation error ${error} because saved object was undefined` + ); + return; + } + const prevVersion = `${pkgName}-${installedPkg.attributes.version}`; + logger.error(`rolling back to ${prevVersion} after error installing ${pkgkey}`); + await installPackage({ + savedObjectsClient, + pkgkey: prevVersion, + callCluster, + }); + } + } catch (e) { + logger.error(`failed to uninstall or rollback package after installation error ${e}`); + } +} + +type BulkInstallResponse = BulkInstallPackageInfo | IBulkInstallPackageError; +function bulkInstallErrorToOptions({ + pkgToUpgrade, + error, +}: { + pkgToUpgrade: string; + error: Error; +}): IBulkInstallPackageError { + const { statusCode, body } = ingestErrorToResponseOptions(error); + return { + name: pkgToUpgrade, + statusCode, + error: body.message, + }; +} + +interface UpgradePackageParams { + savedObjectsClient: SavedObjectsClientContract; + callCluster: CallESAsCurrentUser; + installedPkg: UnwrapPromise>; + latestPkg: UnwrapPromise>; + pkgToUpgrade: string; +} +async function upgradePackage({ + savedObjectsClient, + callCluster, + installedPkg, + latestPkg, + pkgToUpgrade, +}: UpgradePackageParams): Promise { + if (!installedPkg || semver.gt(latestPkg.version, installedPkg.attributes.version)) { + const pkgkey = Registry.pkgToPkgKey({ + name: latestPkg.name, + version: latestPkg.version, + }); + + try { + const assets = await installPackage({ savedObjectsClient, pkgkey, callCluster }); + return { + name: pkgToUpgrade, + newVersion: latestPkg.version, + oldVersion: installedPkg?.attributes.version ?? null, + assets, + }; + } catch (installFailed) { + await handleInstallPackageFailure({ + savedObjectsClient, + error: installFailed, + pkgName: latestPkg.name, + pkgVersion: latestPkg.version, + installedPkg, + callCluster, + }); + return bulkInstallErrorToOptions({ pkgToUpgrade, error: installFailed }); + } + } else { + // package was already at the latest version + return { + name: pkgToUpgrade, + newVersion: latestPkg.version, + oldVersion: latestPkg.version, + assets: [ + ...installedPkg.attributes.installed_es, + ...installedPkg.attributes.installed_kibana, + ], + }; + } +} + +interface BulkInstallPackagesParams { + savedObjectsClient: SavedObjectsClientContract; + packagesToUpgrade: string[]; + callCluster: CallESAsCurrentUser; +} +export async function bulkInstallPackages({ + savedObjectsClient, + packagesToUpgrade, + callCluster, +}: BulkInstallPackagesParams): Promise { + const installedAndLatestPromises = packagesToUpgrade.map((pkgToUpgrade) => + Promise.all([ + getInstallationObject({ savedObjectsClient, pkgName: pkgToUpgrade }), + Registry.fetchFindLatestPackage(pkgToUpgrade), + ]) + ); + const installedAndLatestResults = await Promise.allSettled(installedAndLatestPromises); + const installResponsePromises = installedAndLatestResults.map(async (result, index) => { + const pkgToUpgrade = packagesToUpgrade[index]; + if (result.status === 'fulfilled') { + const [installedPkg, latestPkg] = result.value; + return upgradePackage({ + savedObjectsClient, + callCluster, + installedPkg, + latestPkg, + pkgToUpgrade, + }); + } else { + return bulkInstallErrorToOptions({ pkgToUpgrade, error: result.reason }); + } + }); + const installResults = await Promise.allSettled(installResponsePromises); + const installResponses = installResults.map((result, index) => { + const pkgToUpgrade = packagesToUpgrade[index]; + if (result.status === 'fulfilled') { + return result.value; + } else { + return bulkInstallErrorToOptions({ pkgToUpgrade, error: result.reason }); + } + }); + + return installResponses; +} + +interface InstallPackageParams { savedObjectsClient: SavedObjectsClientContract; pkgkey: string; callCluster: CallESAsCurrentUser; force?: boolean; -}): Promise { +} + +export async function installPackage({ + savedObjectsClient, + pkgkey, + callCluster, + force = false, +}: InstallPackageParams): Promise { // TODO: change epm API to /packageName/version so we don't need to do this const { pkgName, pkgVersion } = Registry.splitPkgKey(pkgkey); // TODO: calls to getInstallationObject, Registry.fetchInfo, and Registry.fetchFindLatestPackge diff --git a/x-pack/plugins/ingest_manager/server/types/rest_spec/epm.ts b/x-pack/plugins/ingest_manager/server/types/rest_spec/epm.ts index d7a801feec34f..5d2a078374854 100644 --- a/x-pack/plugins/ingest_manager/server/types/rest_spec/epm.ts +++ b/x-pack/plugins/ingest_manager/server/types/rest_spec/epm.ts @@ -43,6 +43,12 @@ export const InstallPackageFromRegistryRequestSchema = { ), }; +export const BulkUpgradePackagesFromRegistryRequestSchema = { + body: schema.object({ + packages: schema.arrayOf(schema.string(), { minSize: 1 }), + }), +}; + export const InstallPackageByUploadRequestSchema = { body: schema.buffer(), }; diff --git a/x-pack/test/ingest_manager_api_integration/apis/epm/bulk_upgrade.ts b/x-pack/test/ingest_manager_api_integration/apis/epm/bulk_upgrade.ts new file mode 100644 index 0000000000000..e377ea5a762f9 --- /dev/null +++ b/x-pack/test/ingest_manager_api_integration/apis/epm/bulk_upgrade.ts @@ -0,0 +1,113 @@ +/* + * 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. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../../api_integration/ftr_provider_context'; +import { skipIfNoDockerRegistry } from '../../helpers'; +import { + BulkInstallPackageInfo, + BulkInstallPackagesResponse, + IBulkInstallPackageError, +} from '../../../../plugins/ingest_manager/common'; + +export default function (providerContext: FtrProviderContext) { + const { getService } = providerContext; + const supertest = getService('supertest'); + + const deletePackage = async (pkgkey: string) => { + await supertest.delete(`/api/ingest_manager/epm/packages/${pkgkey}`).set('kbn-xsrf', 'xxxx'); + }; + + describe('bulk package upgrade api', async () => { + skipIfNoDockerRegistry(providerContext); + + describe('bulk package upgrade with a package already installed', async () => { + beforeEach(async () => { + await supertest + .post(`/api/ingest_manager/epm/packages/multiple_versions-0.1.0`) + .set('kbn-xsrf', 'xxxx') + .send({ force: true }) + .expect(200); + }); + afterEach(async () => { + await deletePackage('multiple_versions-0.1.0'); + await deletePackage('multiple_versions-0.3.0'); + await deletePackage('overrides-0.1.0'); + }); + + it('should return 400 if no packages are requested for upgrade', async function () { + await supertest + .post(`/api/ingest_manager/epm/packages/_bulk`) + .set('kbn-xsrf', 'xxxx') + .expect(400); + }); + it('should return 200 and an array for upgrading a package', async function () { + const { body }: { body: BulkInstallPackagesResponse } = await supertest + .post(`/api/ingest_manager/epm/packages/_bulk`) + .set('kbn-xsrf', 'xxxx') + .send({ packages: ['multiple_versions'] }) + .expect(200); + expect(body.response.length).equal(1); + expect(body.response[0].name).equal('multiple_versions'); + const entry = body.response[0] as BulkInstallPackageInfo; + expect(entry.oldVersion).equal('0.1.0'); + expect(entry.newVersion).equal('0.3.0'); + }); + it('should return an error for packages that do not exist', async function () { + const { body }: { body: BulkInstallPackagesResponse } = await supertest + .post(`/api/ingest_manager/epm/packages/_bulk`) + .set('kbn-xsrf', 'xxxx') + .send({ packages: ['multiple_versions', 'blahblah'] }) + .expect(200); + expect(body.response.length).equal(2); + expect(body.response[0].name).equal('multiple_versions'); + const entry = body.response[0] as BulkInstallPackageInfo; + expect(entry.oldVersion).equal('0.1.0'); + expect(entry.newVersion).equal('0.3.0'); + + const err = body.response[1] as IBulkInstallPackageError; + expect(err.statusCode).equal(404); + expect(body.response[1].name).equal('blahblah'); + }); + it('should upgrade multiple packages', async function () { + const { body }: { body: BulkInstallPackagesResponse } = await supertest + .post(`/api/ingest_manager/epm/packages/_bulk`) + .set('kbn-xsrf', 'xxxx') + .send({ packages: ['multiple_versions', 'overrides'] }) + .expect(200); + expect(body.response.length).equal(2); + expect(body.response[0].name).equal('multiple_versions'); + let entry = body.response[0] as BulkInstallPackageInfo; + expect(entry.oldVersion).equal('0.1.0'); + expect(entry.newVersion).equal('0.3.0'); + + entry = body.response[1] as BulkInstallPackageInfo; + expect(entry.oldVersion).equal(null); + expect(entry.newVersion).equal('0.1.0'); + expect(entry.name).equal('overrides'); + }); + }); + + describe('bulk upgrade without package already installed', async () => { + afterEach(async () => { + await deletePackage('multiple_versions-0.3.0'); + }); + + it('should return 200 and an array for upgrading a package', async function () { + const { body }: { body: BulkInstallPackagesResponse } = await supertest + .post(`/api/ingest_manager/epm/packages/_bulk`) + .set('kbn-xsrf', 'xxxx') + .send({ packages: ['multiple_versions'] }) + .expect(200); + expect(body.response.length).equal(1); + expect(body.response[0].name).equal('multiple_versions'); + const entry = body.response[0] as BulkInstallPackageInfo; + expect(entry.oldVersion).equal(null); + expect(entry.newVersion).equal('0.3.0'); + }); + }); + }); +} diff --git a/x-pack/test/ingest_manager_api_integration/apis/epm/index.js b/x-pack/test/ingest_manager_api_integration/apis/epm/index.js index 28743ee5f43c2..e509babc9828b 100644 --- a/x-pack/test/ingest_manager_api_integration/apis/epm/index.js +++ b/x-pack/test/ingest_manager_api_integration/apis/epm/index.js @@ -16,6 +16,7 @@ export default function loadTests({ loadTestFile }) { loadTestFile(require.resolve('./install_prerelease')); loadTestFile(require.resolve('./install_remove_assets')); loadTestFile(require.resolve('./install_update')); + loadTestFile(require.resolve('./bulk_upgrade')); loadTestFile(require.resolve('./update_assets')); loadTestFile(require.resolve('./data_stream')); loadTestFile(require.resolve('./package_install_complete'));