From 15af802b8ffe215cd6f80c98a6b96d5dccb57147 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 18 Aug 2021 10:58:02 +0200 Subject: [PATCH 1/9] initial modifications --- .../service/lib/point_in_time_finder.ts | 4 +- .../saved_objects/service/lib/repository.ts | 31 +++++++++- .../service/saved_objects_client.ts | 14 ++++- .../secure_saved_objects_client_wrapper.ts | 46 +++++++++----- .../spaces_saved_objects_client.ts | 61 +++++++++++-------- 5 files changed, 113 insertions(+), 43 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/point_in_time_finder.ts b/src/core/server/saved_objects/service/lib/point_in_time_finder.ts index f0ed943c585e5..d11be250ad0a9 100644 --- a/src/core/server/saved_objects/service/lib/point_in_time_finder.ts +++ b/src/core/server/saved_objects/service/lib/point_in_time_finder.ts @@ -139,7 +139,9 @@ export class PointInTimeFinder private async open() { try { - const { id } = await this.#client.openPointInTimeForType(this.#findOptions.type); + const { id } = await this.#client.openPointInTimeForType(this.#findOptions.type, { + namespaces: this.#findOptions.namespaces, + }); this.#pitId = id; this.#open = true; } catch (e) { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 17b0f10ef67c8..2ca1edf0139f2 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -1863,9 +1863,36 @@ export class SavedObjectsRepository { */ async openPointInTimeForType( type: string | string[], - { keepAlive = '5m', preference }: SavedObjectsOpenPointInTimeOptions = {} + { + keepAlive = '5m', + preference, + namespaces, + typeToNamespacesMap, + }: SavedObjectsOpenPointInTimeOptions = {} ): Promise { - const types = Array.isArray(type) ? type : [type]; + 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 = type + ? Array.isArray(type) + ? type + : [type] + : Array.from(typeToNamespacesMap!.keys()); const allowedTypes = types.filter((t) => this._allowedTypes.includes(t)); if (allowedTypes.length === 0) { throw SavedObjectsErrorHelpers.createGenericNotFoundError(); diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index 00d47d8d1fb03..a9b31e884f4ff 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -334,7 +334,7 @@ export interface SavedObjectsResolveResponse { /** * @public */ -export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOptions { +export interface SavedObjectsOpenPointInTimeOptions { /** * Optionally specify how long ES should keep the PIT alive until the next request. Defaults to `5m`. */ @@ -343,6 +343,18 @@ export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOpti * An optional ES preference value to be used for the query. */ preference?: string; + /** + * An optional list of namespaces to be used by the PIT. + */ + 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; } /** 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 a3bd215211983..74fa51065cff5 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 @@ -29,7 +29,7 @@ import type { SavedObjectsUpdateOptions, } from 'src/core/server'; -import { SavedObjectsUtils } from '../../../../../src/core/server'; +import { SavedObjectsErrorHelpers, SavedObjectsUtils } from '../../../../../src/core/server'; import { ALL_SPACES_ID, UNKNOWN_SPACE } from '../../common/constants'; import type { AuditLogger, SecurityAuditLogger } from '../audit'; import { SavedObjectAction, savedObjectEvent } from '../audit'; @@ -54,6 +54,7 @@ interface SecureSavedObjectsClientWrapperOptions { baseClient: SavedObjectsClientContract; errors: SavedObjectsClientContract['errors']; checkSavedObjectsPrivilegesAsCurrentUser: CheckSavedObjectsPrivileges; + getSpacesService(): SpacesService | undefined; } @@ -75,10 +76,12 @@ interface LegacyEnsureAuthorizedResult { status: 'fully_authorized' | 'partially_authorized' | 'unauthorized'; typeMap: Map; } + interface LegacyEnsureAuthorizedTypeResult { authorizedSpaces: string[]; isGloballyAuthorized?: boolean; } + export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContract { private readonly actions: Actions; private readonly legacyAuditLogger: PublicMethodsOf; @@ -236,11 +239,6 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra `_find across namespaces is not permitted when the Spaces plugin is disabled.` ); } - if (options.pit && Array.isArray(options.namespaces) && options.namespaces.length > 1) { - throw this.errors.createBadRequestError( - '_find across namespaces is not permitted when using the `pit` option.' - ); - } const args = { options }; const { status, typeMap } = await this.legacyEnsureAuthorized( @@ -508,24 +506,37 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra type: string | string[], options: SavedObjectsOpenPointInTimeOptions ) { - try { - const args = { type, options }; - await this.legacyEnsureAuthorized(type, 'open_point_in_time', options?.namespace, { + const args = { type, options }; + const { status, typeMap } = await this.legacyEnsureAuthorized( + type, + 'open_point_in_time', + options?.namespaces, + { args, // Partial authorization is acceptable in this case because this method is only designed // to be used with `find`, which already allows for partial authorization. requireFullAuthorization: false, - }); - } catch (error) { + } + ); + + if (status === 'unauthorized') { this.auditLogger.log( savedObjectEvent({ action: SavedObjectAction.OPEN_POINT_IN_TIME, - error, + error: new Error(status), }) ); - throw error; + throw SavedObjectsErrorHelpers.decorateForbiddenError(new Error(status)); } + const typeToNamespacesMap = Array.from(typeMap).reduce>( + (acc, [currentType, { authorizedSpaces, isGloballyAuthorized }]) => + isGloballyAuthorized + ? acc.set(currentType, options.namespaces) + : acc.set(currentType, authorizedSpaces), + new Map() + ); + this.auditLogger.log( savedObjectEvent({ action: SavedObjectAction.OPEN_POINT_IN_TIME, @@ -533,7 +544,14 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra }) ); - return await this.baseClient.openPointInTimeForType(type, options); + return await this.baseClient.openPointInTimeForType( + status === 'partially_authorized' ? '' : type, // if the user is partially authorized, type must be an empty string to use typeToNamespacesMap instead + { + ...options, + typeToNamespacesMap: undefined, // if the user is fully authorized, use `undefined` as the typeToNamespacesMap to prevent privilege escalation + ...(status === 'partially_authorized' && { typeToNamespacesMap, namespaces: [] }), // the repository requires that `type` and `namespaces` must be empty if `typeToNamespacesMap` is defined + } + ); } public async closePointInTime(id: string, options?: SavedObjectsClosePointInTimeOptions) { 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 e344aa8cecf07..c9f5ea5cad11b 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 @@ -30,7 +30,7 @@ import type { SavedObjectsUpdateOptions, } from 'src/core/server'; -import { SavedObjectsUtils } from '../../../../../src/core/server'; +import { SavedObjectsErrorHelpers, SavedObjectsUtils } from '../../../../../src/core/server'; import { ALL_SPACES_ID } from '../../common/constants'; import { spaceIdToNamespace } from '../lib/utils/namespace'; import type { ISpacesClient } from '../spaces_client'; @@ -177,30 +177,19 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { public async find(options: SavedObjectsFindOptions) { throwErrorIfNamespaceSpecified(options); - let namespaces = options.namespaces; - if (namespaces) { - try { - const availableSpaces = await this.spacesClient.getAll({ purpose: 'findSavedObjects' }); - if (namespaces.includes(ALL_SPACES_ID)) { - 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; + let namespaces: string[]; + try { + namespaces = await this.getSearchableSpaces(options.namespaces); + } 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); } - } else { - namespaces = [this.spaceId]; + throw err; + } + 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); } return await this.client.find({ @@ -212,6 +201,21 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { }); } + private async getSearchableSpaces(namespaces?: string[]): Promise { + if (namespaces) { + const availableSpaces = await this.spacesClient.getAll({ purpose: 'findSavedObjects' }); + if (namespaces.includes(ALL_SPACES_ID)) { + return availableSpaces.map((space) => space.id); + } else { + return namespaces.filter((namespace) => + availableSpaces.some((space) => space.id === namespace) + ); + } + } else { + return [this.spaceId]; + } + } + /** * Returns an array of objects by id * @@ -397,9 +401,16 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { options: SavedObjectsOpenPointInTimeOptions = {} ) { throwErrorIfNamespaceSpecified(options); + + const namespaces = await this.getSearchableSpaces(options.namespaces); + if (namespaces.length === 0) { + // throw bad request if no valid spaces were found. + throw SavedObjectsErrorHelpers.createBadRequestError(); + } + return await this.client.openPointInTimeForType(type, { ...options, - namespace: spaceIdToNamespace(this.spaceId), + namespaces, }); } From 3fbfcad060baf3cd084bda4d9318afa9e5a3a334 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 20 Aug 2021 09:59:36 +0200 Subject: [PATCH 2/9] change approach for openPointInTime and add tests for spaces wrapper changes --- .../saved_objects/service/lib/repository.ts | 31 +------ .../service/saved_objects_client.ts | 8 -- .../secure_saved_objects_client_wrapper.ts | 18 +--- .../spaces_saved_objects_client.test.ts | 85 +++++++++++++++++-- .../spaces_saved_objects_client.ts | 32 ++++--- 5 files changed, 95 insertions(+), 79 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 1138c4e4ff946..365fc6a3734e4 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -1936,36 +1936,9 @@ export class SavedObjectsRepository { */ async openPointInTimeForType( type: string | string[], - { - keepAlive = '5m', - preference, - namespaces, - typeToNamespacesMap, - }: SavedObjectsOpenPointInTimeOptions = {} + { keepAlive = '5m', preference }: SavedObjectsOpenPointInTimeOptions = {} ): Promise { - 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 = type - ? Array.isArray(type) - ? type - : [type] - : Array.from(typeToNamespacesMap!.keys()); + const types = Array.isArray(type) ? type : [type]; const allowedTypes = types.filter((t) => this._allowedTypes.includes(t)); if (allowedTypes.length === 0) { throw SavedObjectsErrorHelpers.createGenericNotFoundError(); diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index a9b31e884f4ff..34413e62d4e00 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -347,14 +347,6 @@ export interface SavedObjectsOpenPointInTimeOptions { * An optional list of namespaces to be used by the PIT. */ 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; } /** 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 74fa51065cff5..add939fbfe6f2 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 @@ -529,14 +529,6 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra throw SavedObjectsErrorHelpers.decorateForbiddenError(new Error(status)); } - const typeToNamespacesMap = Array.from(typeMap).reduce>( - (acc, [currentType, { authorizedSpaces, isGloballyAuthorized }]) => - isGloballyAuthorized - ? acc.set(currentType, options.namespaces) - : acc.set(currentType, authorizedSpaces), - new Map() - ); - this.auditLogger.log( savedObjectEvent({ action: SavedObjectAction.OPEN_POINT_IN_TIME, @@ -544,14 +536,8 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra }) ); - return await this.baseClient.openPointInTimeForType( - status === 'partially_authorized' ? '' : type, // if the user is partially authorized, type must be an empty string to use typeToNamespacesMap instead - { - ...options, - typeToNamespacesMap: undefined, // if the user is fully authorized, use `undefined` as the typeToNamespacesMap to prevent privilege escalation - ...(status === 'partially_authorized' && { typeToNamespacesMap, namespaces: [] }), // the repository requires that `type` and `namespaces` must be empty if `typeToNamespacesMap` is defined - } - ); + const allowedTypes = [...typeMap.keys()]; // only allow the user to open a PIT against indices for type(s) they are authorized to access + return await this.baseClient.openPointInTimeForType(allowedTypes, options); } public async closePointInTime(id: string, options?: SavedObjectsClosePointInTimeOptions) { 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 56bfe71b581ed..d56b81089d6c8 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 @@ -533,27 +533,94 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; }); describe('#openPointInTimeForType', () => { - test(`throws error if options.namespace is specified`, async () => { - const { client } = createSpacesSavedObjectsClient(); + test(`throws error if if user is unauthorized in this space`, async () => { + const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const spacesClient = spacesClientMock.create(); + spacesClient.getAll.mockResolvedValue([]); + spacesService.createSpacesClient.mockReturnValue(spacesClient); - await expect(client.openPointInTimeForType('foo', { namespace: 'bar' })).rejects.toThrow( - ERROR_NAMESPACE_SPECIFIED + await expect( + client.openPointInTimeForType('foo', { namespaces: ['bar'] }) + ).rejects.toThrowError('Bad Request'); + + expect(baseClient.openPointInTimeForType).not.toHaveBeenCalled(); + }); + + test(`throws error if if user is unauthorized in any space`, async () => { + const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const spacesClient = spacesClientMock.create(); + spacesClient.getAll.mockRejectedValue(Boom.unauthorized()); + spacesService.createSpacesClient.mockReturnValue(spacesClient); + + await expect( + client.openPointInTimeForType('foo', { namespaces: ['bar'] }) + ).rejects.toThrowError('Bad Request'); + + expect(baseClient.openPointInTimeForType).not.toHaveBeenCalled(); + }); + + test(`filters options.namespaces based on authorization`, async () => { + const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const expectedReturnValue = { id: 'abc123' }; + baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); + + const spacesClient = spacesService.createSpacesClient( + null as any + ) as jest.Mocked; + spacesClient.getAll.mockImplementation(() => + Promise.resolve([ + { id: 'ns-1', name: '', disabledFeatures: [] }, + { id: 'ns-2', name: '', disabledFeatures: [] }, + ]) ); + + const options = Object.freeze({ namespaces: ['ns-1', 'ns-3'] }); + const actualReturnValue = await client.openPointInTimeForType(['foo', 'bar'], options); + + expect(actualReturnValue).toBe(expectedReturnValue); + expect(baseClient.openPointInTimeForType).toHaveBeenCalledWith(['foo', 'bar'], { + namespaces: ['ns-1'], + }); + expect(spacesClient.getAll).toHaveBeenCalledWith({ purpose: 'findSavedObjects' }); }); - test(`supplements options with the current namespace`, async () => { + test(`translates options.namespace: ['*']`, async () => { + const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); + const expectedReturnValue = { id: 'abc123' }; + baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); + + const spacesClient = spacesService.createSpacesClient( + null as any + ) as jest.Mocked; + spacesClient.getAll.mockImplementation(() => + Promise.resolve([ + { id: 'ns-1', name: '', disabledFeatures: [] }, + { id: 'ns-2', name: '', disabledFeatures: [] }, + ]) + ); + + const options = Object.freeze({ namespaces: ['*'] }); + const actualReturnValue = await client.openPointInTimeForType(['foo', 'bar'], options); + + expect(actualReturnValue).toBe(expectedReturnValue); + expect(baseClient.openPointInTimeForType).toHaveBeenCalledWith(['foo', 'bar'], { + namespaces: ['ns-1', 'ns-2'], + }); + expect(spacesClient.getAll).toHaveBeenCalledWith({ purpose: 'findSavedObjects' }); + }); + + test(`supplements options with the current namespace if unspecified`, async () => { const { client, baseClient } = createSpacesSavedObjectsClient(); const expectedReturnValue = { id: 'abc123' }; baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); - const options = Object.freeze({ foo: 'bar' }); - // @ts-expect-error + const options = Object.freeze({ keepAlive: '2m' }); const actualReturnValue = await client.openPointInTimeForType('foo', options); expect(actualReturnValue).toBe(expectedReturnValue); expect(baseClient.openPointInTimeForType).toHaveBeenCalledWith('foo', { - foo: 'bar', - namespace: currentSpace.expectedNamespace, + keepAlive: '2m', + namespaces: [currentSpace.expectedNamespace ?? 'default'], }); }); }); 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 c9f5ea5cad11b..ad9a4f8f2c390 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 @@ -175,8 +175,6 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { * @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page } */ public async find(options: SavedObjectsFindOptions) { - throwErrorIfNamespaceSpecified(options); - let namespaces: string[]; try { namespaces = await this.getSearchableSpaces(options.namespaces); @@ -201,21 +199,6 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { }); } - private async getSearchableSpaces(namespaces?: string[]): Promise { - if (namespaces) { - const availableSpaces = await this.spacesClient.getAll({ purpose: 'findSavedObjects' }); - if (namespaces.includes(ALL_SPACES_ID)) { - return availableSpaces.map((space) => space.id); - } else { - return namespaces.filter((namespace) => - availableSpaces.some((space) => space.id === namespace) - ); - } - } else { - return [this.spaceId]; - } - } - /** * Returns an array of objects by id * @@ -457,4 +440,19 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { ...dependencies, }); } + + private async getSearchableSpaces(namespaces?: string[]): Promise { + if (namespaces) { + const availableSpaces = await this.spacesClient.getAll({ purpose: 'findSavedObjects' }); + if (namespaces.includes(ALL_SPACES_ID)) { + return availableSpaces.map((space) => space.id); + } else { + return namespaces.filter((namespace) => + availableSpaces.some((space) => space.id === namespace) + ); + } + } else { + return [this.spaceId]; + } + } } From 3c0e37c1daf84e54eda7f9d17625c5accaefc547 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 20 Aug 2021 10:50:35 +0200 Subject: [PATCH 3/9] fix and add security wrapper tests --- ...ecure_saved_objects_client_wrapper.test.ts | 116 +++++++++++++++--- 1 file changed, 100 insertions(+), 16 deletions(-) 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 e5a2340aba3f0..2f622d9e8a0e1 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 @@ -779,17 +779,6 @@ describe('#find', () => { ); }); - test(`throws BadRequestError when searching across namespaces when pit is provided`, async () => { - const options = { - type: [type1, type2], - pit: { id: 'abc123' }, - namespaces: ['some-ns', 'another-ns'], - }; - await expect(client.find(options)).rejects.toThrowErrorMatchingInlineSnapshot( - `"_find across namespaces is not permitted when using the \`pit\` option."` - ); - }); - test(`checks privileges for user, actions, and namespaces`, async () => { const options = { type: [type1, type2], namespaces }; await expectPrivilegeCheck(client.find, { options }, namespaces); @@ -884,7 +873,7 @@ describe('#openPointInTimeForType', () => { const apiCallReturnValue = Symbol(); clientOpts.baseClient.openPointInTimeForType.mockReturnValue(apiCallReturnValue as any); - const options = { namespace }; + const options = { namespaces: [namespace] }; const result = await expectSuccess(client.openPointInTimeForType, { type, options }); expect(result).toBe(apiCallReturnValue); }); @@ -892,18 +881,113 @@ describe('#openPointInTimeForType', () => { test(`adds audit event when successful`, async () => { const apiCallReturnValue = Symbol(); clientOpts.baseClient.openPointInTimeForType.mockReturnValue(apiCallReturnValue as any); - const options = { namespace }; + const options = { namespaces: [namespace] }; await expectSuccess(client.openPointInTimeForType, { type, options }); expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1); expectAuditEvent('saved_object_open_point_in_time', 'unknown'); }); - test(`adds audit event when not successful`, async () => { - clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockRejectedValue(new Error()); - await expect(() => client.openPointInTimeForType(type, { namespace })).rejects.toThrow(); + test(`throws an error when unauthorized`, async () => { + clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockImplementation( + getMockCheckPrivilegesFailure + ); + const options = { namespaces: [namespace] }; + await expect(() => client.openPointInTimeForType(type, options)).rejects.toThrowError( + 'unauthorized' + ); + }); + + test(`adds audit event when unauthorized`, async () => { + clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockImplementation( + getMockCheckPrivilegesFailure + ); + const options = { namespaces: [namespace] }; + await expect(() => client.openPointInTimeForType(type, options)).rejects.toThrow(); expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1); expectAuditEvent('saved_object_open_point_in_time', 'failure'); }); + + test(`filters types based on authorization`, async () => { + clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockResolvedValue({ + hasAllRequested: false, + username: USERNAME, + privileges: { + kibana: [ + { + resource: 'some-ns', + privilege: 'mock-saved_object:foo/open_point_in_time', + authorized: true, + }, + { + resource: 'some-ns', + privilege: 'mock-saved_object:bar/open_point_in_time', + authorized: true, + }, + { + resource: 'some-ns', + privilege: 'mock-saved_object:baz/open_point_in_time', + authorized: false, + }, + { + resource: 'some-ns', + privilege: 'mock-saved_object:qux/open_point_in_time', + authorized: false, + }, + { + resource: 'another-ns', + privilege: 'mock-saved_object:foo/open_point_in_time', + authorized: true, + }, + { + resource: 'another-ns', + privilege: 'mock-saved_object:bar/open_point_in_time', + authorized: false, + }, + { + resource: 'another-ns', + privilege: 'mock-saved_object:baz/open_point_in_time', + authorized: true, + }, + { + resource: 'another-ns', + privilege: 'mock-saved_object:qux/open_point_in_time', + authorized: false, + }, + { + resource: 'forbidden-ns', + privilege: 'mock-saved_object:foo/open_point_in_time', + authorized: false, + }, + { + resource: 'forbidden-ns', + privilege: 'mock-saved_object:bar/open_point_in_time', + authorized: false, + }, + { + resource: 'forbidden-ns', + privilege: 'mock-saved_object:baz/open_point_in_time', + authorized: false, + }, + { + resource: 'forbidden-ns', + privilege: 'mock-saved_object:qux/open_point_in_time', + authorized: false, + }, + ], + }, + }); + + await client.openPointInTimeForType(['foo', 'bar', 'baz', 'qux'], { + namespaces: ['some-ns', 'another-ns', 'forbidden-ns'], + }); + + expect(clientOpts.baseClient.openPointInTimeForType).toHaveBeenCalledWith( + ['foo', 'bar', 'baz'], + { + namespaces: ['some-ns', 'another-ns', 'forbidden-ns'], + } + ); + }); }); describe('#closePointInTime', () => { From edf1a19e1da10087fbcb3e40c313dba571b6f652 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 20 Aug 2021 12:29:48 +0200 Subject: [PATCH 4/9] fix export security FTR tests --- .../common/suites/export.ts | 12 ++++++++++-- .../security_and_spaces/apis/export.ts | 2 +- .../security_only/apis/export.ts | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) 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 ea2f321458c22..77eb93a4107a5 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 @@ -155,8 +155,16 @@ export function exportTestSuiteFactory(esArchiver: any, supertest: SuperTest Date: Fri, 20 Aug 2021 12:33:51 +0200 Subject: [PATCH 5/9] update generated doc --- ...ore-server.savedobjectsopenpointintimeoptions.md | 3 ++- ...savedobjectsopenpointintimeoptions.namespaces.md | 13 +++++++++++++ src/core/server/server.api.md | 3 ++- 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md index 46516be2329e9..4b31d30a9e740 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md @@ -8,7 +8,7 @@ Signature: ```typescript -export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOptions +export interface SavedObjectsOpenPointInTimeOptions ``` ## Properties @@ -16,5 +16,6 @@ export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOpti | Property | Type | Description | | --- | --- | --- | | [keepAlive](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.keepalive.md) | string | Optionally specify how long ES should keep the PIT alive until the next request. Defaults to 5m. | +| [namespaces](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md) | string[] | An optional list of namespaces to be used by the PIT. | | [preference](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.preference.md) | string | An optional ES preference value to be used for the query. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md new file mode 100644 index 0000000000000..9fe5f8d09fb0a --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsOpenPointInTimeOptions](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md) > [namespaces](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md) + +## SavedObjectsOpenPointInTimeOptions.namespaces property + +An optional list of namespaces to be used by the PIT. + +Signature: + +```typescript +namespaces?: string[]; +``` diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 47455e0c14316..1bd59dfc7fdb1 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2460,8 +2460,9 @@ export interface SavedObjectsMigrationVersion { export type SavedObjectsNamespaceType = 'single' | 'multiple' | 'multiple-isolated' | 'agnostic'; // @public (undocumented) -export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOptions { +export interface SavedObjectsOpenPointInTimeOptions { keepAlive?: string; + namespaces?: string[]; preference?: string; } From 3024a47a9a0a86cfe1d63a5c2565d04fc41b5983 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 20 Aug 2021 14:04:44 +0200 Subject: [PATCH 6/9] add tests for PIT finder --- .../service/lib/point_in_time_finder.test.ts | 222 +++++++++--------- 1 file changed, 112 insertions(+), 110 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/point_in_time_finder.test.ts b/src/core/server/saved_objects/service/lib/point_in_time_finder.test.ts index 044bb45269538..160852f9160b7 100644 --- a/src/core/server/saved_objects/service/lib/point_in_time_finder.test.ts +++ b/src/core/server/saved_objects/service/lib/point_in_time_finder.test.ts @@ -7,7 +7,6 @@ */ import { loggerMock, MockedLogger } from '../../../logging/logger.mock'; -import type { SavedObjectsClientContract } from '../../types'; import type { SavedObjectsFindResult } from '../'; import { savedObjectsRepositoryMock } from './repository.mock'; @@ -43,37 +42,67 @@ const mockHits = [ describe('createPointInTimeFinder()', () => { let logger: MockedLogger; - let find: jest.Mocked['find']; - let openPointInTimeForType: jest.Mocked['openPointInTimeForType']; - let closePointInTime: jest.Mocked['closePointInTime']; + let repository: ReturnType; beforeEach(() => { logger = loggerMock.create(); - const mockRepository = savedObjectsRepositoryMock.create(); - find = mockRepository.find; - openPointInTimeForType = mockRepository.openPointInTimeForType; - closePointInTime = mockRepository.closePointInTime; + repository = savedObjectsRepositoryMock.create(); }); describe('#find', () => { - test('throws if a PIT is already open', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + test('opens a PIT with the correct parameters', async () => { + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'abc123', }); - find.mockResolvedValueOnce({ + repository.find.mockResolvedValue({ total: 2, saved_objects: mockHits, pit_id: 'abc123', per_page: 1, page: 0, }); - find.mockResolvedValueOnce({ - total: 2, - saved_objects: mockHits, - pit_id: 'abc123', - per_page: 1, - page: 1, + + const findOptions: SavedObjectsCreatePointInTimeFinderOptions = { + type: ['visualization'], + search: 'foo*', + perPage: 1, + namespaces: ['ns1', 'ns2'], + }; + + const finder = new PointInTimeFinder(findOptions, { + logger, + client: repository, + }); + + expect(repository.openPointInTimeForType).not.toHaveBeenCalled(); + + await finder.find().next(); + + expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(1); + expect(repository.openPointInTimeForType).toHaveBeenCalledWith(findOptions.type, { + namespaces: findOptions.namespaces, }); + }); + + test('throws if a PIT is already open', async () => { + repository.openPointInTimeForType.mockResolvedValueOnce({ + id: 'abc123', + }); + repository.find + .mockResolvedValueOnce({ + total: 2, + saved_objects: mockHits, + pit_id: 'abc123', + per_page: 1, + page: 0, + }) + .mockResolvedValueOnce({ + total: 2, + saved_objects: mockHits, + pit_id: 'abc123', + per_page: 1, + page: 1, + }); const findOptions: SavedObjectsCreatePointInTimeFinderOptions = { type: ['visualization'], @@ -83,30 +112,25 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); await finder.find().next(); - expect(find).toHaveBeenCalledTimes(1); - find.mockClear(); + expect(repository.find).toHaveBeenCalledTimes(1); expect(async () => { await finder.find().next(); }).rejects.toThrowErrorMatchingInlineSnapshot( `"Point In Time has already been opened for this finder instance. Please call \`close()\` before calling \`find()\` again."` ); - expect(find).toHaveBeenCalledTimes(0); + expect(repository.find).toHaveBeenCalledTimes(1); }); test('works with a single page of results', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'abc123', }); - find.mockResolvedValueOnce({ + repository.find.mockResolvedValueOnce({ total: 2, saved_objects: mockHits, pit_id: 'abc123', @@ -121,11 +145,7 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); const hits: SavedObjectsFindResult[] = []; for await (const result of finder.find()) { @@ -133,10 +153,10 @@ describe('createPointInTimeFinder()', () => { } expect(hits.length).toBe(2); - expect(openPointInTimeForType).toHaveBeenCalledTimes(1); - expect(closePointInTime).toHaveBeenCalledTimes(1); - expect(find).toHaveBeenCalledTimes(1); - expect(find).toHaveBeenCalledWith( + expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(1); + expect(repository.closePointInTime).toHaveBeenCalledTimes(1); + expect(repository.find).toHaveBeenCalledTimes(1); + expect(repository.find).toHaveBeenCalledWith( expect.objectContaining({ pit: expect.objectContaining({ id: 'abc123', keepAlive: '2m' }), sortField: 'updated_at', @@ -147,24 +167,25 @@ describe('createPointInTimeFinder()', () => { }); test('works with multiple pages of results', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'abc123', }); - find.mockResolvedValueOnce({ - total: 2, - saved_objects: [mockHits[0]], - pit_id: 'abc123', - per_page: 1, - page: 0, - }); - find.mockResolvedValueOnce({ - total: 2, - saved_objects: [mockHits[1]], - pit_id: 'abc123', - per_page: 1, - page: 0, - }); - find.mockResolvedValueOnce({ + repository.find + .mockResolvedValueOnce({ + total: 2, + saved_objects: [mockHits[0]], + pit_id: 'abc123', + per_page: 1, + page: 0, + }) + .mockResolvedValueOnce({ + total: 2, + saved_objects: [mockHits[1]], + pit_id: 'abc123', + per_page: 1, + page: 0, + }); + repository.find.mockResolvedValueOnce({ total: 2, saved_objects: [], per_page: 1, @@ -180,11 +201,7 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); const hits: SavedObjectsFindResult[] = []; for await (const result of finder.find()) { @@ -192,12 +209,12 @@ describe('createPointInTimeFinder()', () => { } expect(hits.length).toBe(2); - expect(openPointInTimeForType).toHaveBeenCalledTimes(1); - expect(closePointInTime).toHaveBeenCalledTimes(1); + expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(1); + expect(repository.closePointInTime).toHaveBeenCalledTimes(1); // called 3 times since we need a 3rd request to check if we // are done paginating through results. - expect(find).toHaveBeenCalledTimes(3); - expect(find).toHaveBeenCalledWith( + expect(repository.find).toHaveBeenCalledTimes(3); + expect(repository.find).toHaveBeenCalledWith( expect.objectContaining({ pit: expect.objectContaining({ id: 'abc123', keepAlive: '2m' }), sortField: 'updated_at', @@ -210,10 +227,10 @@ describe('createPointInTimeFinder()', () => { describe('#close', () => { test('calls closePointInTime with correct ID', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'test', }); - find.mockResolvedValueOnce({ + repository.find.mockResolvedValueOnce({ total: 1, saved_objects: [mockHits[0]], pit_id: 'test', @@ -229,11 +246,7 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); const hits: SavedObjectsFindResult[] = []; for await (const result of finder.find()) { @@ -241,28 +254,28 @@ describe('createPointInTimeFinder()', () => { await finder.close(); } - expect(closePointInTime).toHaveBeenCalledWith('test'); + expect(repository.closePointInTime).toHaveBeenCalledWith('test'); }); test('causes generator to stop', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'test', }); - find.mockResolvedValueOnce({ + repository.find.mockResolvedValueOnce({ total: 2, saved_objects: [mockHits[0]], pit_id: 'test', per_page: 1, page: 0, }); - find.mockResolvedValueOnce({ + repository.find.mockResolvedValueOnce({ total: 2, saved_objects: [mockHits[1]], pit_id: 'test', per_page: 1, page: 0, }); - find.mockResolvedValueOnce({ + repository.find.mockResolvedValueOnce({ total: 2, saved_objects: [], per_page: 1, @@ -278,11 +291,7 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); const hits: SavedObjectsFindResult[] = []; for await (const result of finder.find()) { @@ -290,15 +299,15 @@ describe('createPointInTimeFinder()', () => { await finder.close(); } - expect(closePointInTime).toHaveBeenCalledTimes(1); + expect(repository.closePointInTime).toHaveBeenCalledTimes(1); expect(hits.length).toBe(1); }); test('is called if `find` throws an error', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'test', }); - find.mockRejectedValueOnce(new Error('oops')); + repository.find.mockRejectedValueOnce(new Error('oops')); const findOptions: SavedObjectsCreatePointInTimeFinderOptions = { type: ['visualization'], @@ -308,11 +317,7 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); const hits: SavedObjectsFindResult[] = []; try { @@ -323,27 +328,28 @@ describe('createPointInTimeFinder()', () => { // intentionally empty } - expect(closePointInTime).toHaveBeenCalledWith('test'); + expect(repository.closePointInTime).toHaveBeenCalledWith('test'); }); test('finder can be reused after closing', async () => { - openPointInTimeForType.mockResolvedValueOnce({ + repository.openPointInTimeForType.mockResolvedValueOnce({ id: 'abc123', }); - find.mockResolvedValueOnce({ - total: 2, - saved_objects: mockHits, - pit_id: 'abc123', - per_page: 1, - page: 0, - }); - find.mockResolvedValueOnce({ - total: 2, - saved_objects: mockHits, - pit_id: 'abc123', - per_page: 1, - page: 1, - }); + repository.find + .mockResolvedValueOnce({ + total: 2, + saved_objects: mockHits, + pit_id: 'abc123', + per_page: 1, + page: 0, + }) + .mockResolvedValueOnce({ + total: 2, + saved_objects: mockHits, + pit_id: 'abc123', + per_page: 1, + page: 1, + }); const findOptions: SavedObjectsCreatePointInTimeFinderOptions = { type: ['visualization'], @@ -353,11 +359,7 @@ describe('createPointInTimeFinder()', () => { const finder = new PointInTimeFinder(findOptions, { logger, - client: { - find, - openPointInTimeForType, - closePointInTime, - }, + client: repository, }); const findA = finder.find(); @@ -370,9 +372,9 @@ describe('createPointInTimeFinder()', () => { expect((await findA.next()).done).toBe(true); expect((await findB.next()).done).toBe(true); - expect(openPointInTimeForType).toHaveBeenCalledTimes(2); - expect(find).toHaveBeenCalledTimes(2); - expect(closePointInTime).toHaveBeenCalledTimes(2); + expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(2); + expect(repository.find).toHaveBeenCalledTimes(2); + expect(repository.closePointInTime).toHaveBeenCalledTimes(2); }); }); }); From b9a1b388c30da49a71a071a97a4faefc026ade76 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 20 Aug 2021 14:46:49 +0200 Subject: [PATCH 7/9] NIT --- .../server/saved_objects/secure_saved_objects_client_wrapper.ts | 1 - 1 file changed, 1 deletion(-) 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 add939fbfe6f2..6f2b8d28a5601 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 @@ -54,7 +54,6 @@ interface SecureSavedObjectsClientWrapperOptions { baseClient: SavedObjectsClientContract; errors: SavedObjectsClientContract['errors']; checkSavedObjectsPrivilegesAsCurrentUser: CheckSavedObjectsPrivileges; - getSpacesService(): SpacesService | undefined; } From 16c0b0a9aec269052cfabccf4f5145524d787a20 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 20 Aug 2021 18:15:50 +0200 Subject: [PATCH 8/9] improve doc --- ...lugin-core-server.savedobjectsopenpointintimeoptions.md | 2 +- ...server.savedobjectsopenpointintimeoptions.namespaces.md | 4 +++- .../server/saved_objects/service/saved_objects_client.ts | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md index 4b31d30a9e740..fc825e3bf2937 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md @@ -16,6 +16,6 @@ export interface SavedObjectsOpenPointInTimeOptions | Property | Type | Description | | --- | --- | --- | | [keepAlive](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.keepalive.md) | string | Optionally specify how long ES should keep the PIT alive until the next request. Defaults to 5m. | -| [namespaces](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md) | string[] | An optional list of namespaces to be used by the PIT. | +| [namespaces](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md) | string[] | An optional list of namespaces to be used when opening the PIT.When the spaces plugin is enabled: - this will default to the user's current space (as determined by the URL) - if specified, the user's current space will be ignored - ['*'] will search across all available spaces | | [preference](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.preference.md) | string | An optional ES preference value to be used for the query. | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md index 9fe5f8d09fb0a..06fb7519d52c2 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md @@ -4,7 +4,9 @@ ## SavedObjectsOpenPointInTimeOptions.namespaces property -An optional list of namespaces to be used by the PIT. +An optional list of namespaces to be used when opening the PIT. + +When the spaces plugin is enabled: - this will default to the user's current space (as determined by the URL) - if specified, the user's current space will be ignored - `['*']` will search across all available spaces Signature: diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index 34413e62d4e00..1564df2969ecc 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -344,7 +344,12 @@ export interface SavedObjectsOpenPointInTimeOptions { */ preference?: string; /** - * An optional list of namespaces to be used by the PIT. + * An optional list of namespaces to be used when opening the PIT. + * + * When the spaces plugin is enabled: + * - this will default to the user's current space (as determined by the URL) + * - if specified, the user's current space will be ignored + * - `['*']` will search across all available spaces */ namespaces?: string[]; } From 9d5fb6f1bc8164d7d74ac3e32eaa890c1e59ad15 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 23 Aug 2021 09:24:15 +0200 Subject: [PATCH 9/9] nits --- .../server/saved_objects/spaces_saved_objects_client.test.ts | 4 ++-- .../server/saved_objects/spaces_saved_objects_client.ts | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) 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 d56b81089d6c8..b94113436d7ad 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 @@ -584,7 +584,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; expect(spacesClient.getAll).toHaveBeenCalledWith({ purpose: 'findSavedObjects' }); }); - test(`translates options.namespace: ['*']`, async () => { + test(`translates options.namespaces: ['*']`, async () => { const { client, baseClient, spacesService } = createSpacesSavedObjectsClient(); const expectedReturnValue = { id: 'abc123' }; baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); @@ -620,7 +620,7 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces'; expect(actualReturnValue).toBe(expectedReturnValue); expect(baseClient.openPointInTimeForType).toHaveBeenCalledWith('foo', { keepAlive: '2m', - namespaces: [currentSpace.expectedNamespace ?? 'default'], + namespaces: [currentSpace.expectedNamespace ?? DEFAULT_SPACE_ID], }); }); }); 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 ad9a4f8f2c390..9c51f22e280d8 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 @@ -383,8 +383,6 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract { type: string | string[], options: SavedObjectsOpenPointInTimeOptions = {} ) { - throwErrorIfNamespaceSpecified(options); - const namespaces = await this.getSearchableSpaces(options.namespaces); if (namespaces.length === 0) { // throw bad request if no valid spaces were found.