Add multiple namespaces support to PIT search and finder (#109062)

* initial modifications

* change approach for openPointInTime and add tests for spaces wrapper changes

* fix and add security wrapper tests

* fix export security FTR tests

* update generated doc

* add tests for PIT finder

* NIT

* improve doc

* nits
This commit is contained in:
Pierre Gayvallet 2021-08-23 12:02:41 +02:00 committed by GitHub
parent 75cdeae490
commit 5c5e191364
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 385 additions and 186 deletions

View file

@ -8,7 +8,7 @@
<b>Signature:</b> <b>Signature:</b>
```typescript ```typescript
export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOptions export interface SavedObjectsOpenPointInTimeOptions
``` ```
## Properties ## Properties
@ -16,5 +16,6 @@ export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOpti
| Property | Type | Description | | Property | Type | Description |
| --- | --- | --- | | --- | --- | --- |
| [keepAlive](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.keepalive.md) | <code>string</code> | Optionally specify how long ES should keep the PIT alive until the next request. Defaults to <code>5m</code>. | | [keepAlive](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.keepalive.md) | <code>string</code> | Optionally specify how long ES should keep the PIT alive until the next request. Defaults to <code>5m</code>. |
| [namespaces](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md) | <code>string[]</code> | 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 - <code>['*']</code> will search across all available spaces |
| [preference](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.preference.md) | <code>string</code> | An optional ES preference value to be used for the query. | | [preference](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.preference.md) | <code>string</code> | An optional ES preference value to be used for the query. |

View file

@ -0,0 +1,15 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectsOpenPointInTimeOptions](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.md) &gt; [namespaces](./kibana-plugin-core-server.savedobjectsopenpointintimeoptions.namespaces.md)
## SavedObjectsOpenPointInTimeOptions.namespaces property
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
<b>Signature:</b>
```typescript
namespaces?: string[];
```

View file

@ -7,7 +7,6 @@
*/ */
import { loggerMock, MockedLogger } from '../../../logging/logger.mock'; import { loggerMock, MockedLogger } from '../../../logging/logger.mock';
import type { SavedObjectsClientContract } from '../../types';
import type { SavedObjectsFindResult } from '../'; import type { SavedObjectsFindResult } from '../';
import { savedObjectsRepositoryMock } from './repository.mock'; import { savedObjectsRepositoryMock } from './repository.mock';
@ -43,38 +42,68 @@ const mockHits = [
describe('createPointInTimeFinder()', () => { describe('createPointInTimeFinder()', () => {
let logger: MockedLogger; let logger: MockedLogger;
let find: jest.Mocked<SavedObjectsClientContract>['find']; let repository: ReturnType<typeof savedObjectsRepositoryMock.create>;
let openPointInTimeForType: jest.Mocked<SavedObjectsClientContract>['openPointInTimeForType'];
let closePointInTime: jest.Mocked<SavedObjectsClientContract>['closePointInTime'];
beforeEach(() => { beforeEach(() => {
logger = loggerMock.create(); logger = loggerMock.create();
const mockRepository = savedObjectsRepositoryMock.create(); repository = savedObjectsRepositoryMock.create();
find = mockRepository.find;
openPointInTimeForType = mockRepository.openPointInTimeForType;
closePointInTime = mockRepository.closePointInTime;
}); });
describe('#find', () => { describe('#find', () => {
test('throws if a PIT is already open', async () => { test('opens a PIT with the correct parameters', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123', id: 'abc123',
}); });
find.mockResolvedValueOnce({ repository.find.mockResolvedValue({
total: 2, total: 2,
saved_objects: mockHits, saved_objects: mockHits,
pit_id: 'abc123', pit_id: 'abc123',
per_page: 1, per_page: 1,
page: 0, page: 0,
}); });
find.mockResolvedValueOnce({
total: 2, const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
saved_objects: mockHits, type: ['visualization'],
pit_id: 'abc123', search: 'foo*',
per_page: 1, perPage: 1,
page: 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 = { const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
type: ['visualization'], type: ['visualization'],
search: 'foo*', search: 'foo*',
@ -83,30 +112,25 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
await finder.find().next(); await finder.find().next();
expect(find).toHaveBeenCalledTimes(1); expect(repository.find).toHaveBeenCalledTimes(1);
find.mockClear();
expect(async () => { expect(async () => {
await finder.find().next(); await finder.find().next();
}).rejects.toThrowErrorMatchingInlineSnapshot( }).rejects.toThrowErrorMatchingInlineSnapshot(
`"Point In Time has already been opened for this finder instance. Please call \`close()\` before calling \`find()\` again."` `"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 () => { test('works with a single page of results', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123', id: 'abc123',
}); });
find.mockResolvedValueOnce({ repository.find.mockResolvedValueOnce({
total: 2, total: 2,
saved_objects: mockHits, saved_objects: mockHits,
pit_id: 'abc123', pit_id: 'abc123',
@ -121,11 +145,7 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
const hits: SavedObjectsFindResult[] = []; const hits: SavedObjectsFindResult[] = [];
for await (const result of finder.find()) { for await (const result of finder.find()) {
@ -133,10 +153,10 @@ describe('createPointInTimeFinder()', () => {
} }
expect(hits.length).toBe(2); expect(hits.length).toBe(2);
expect(openPointInTimeForType).toHaveBeenCalledTimes(1); expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(1);
expect(closePointInTime).toHaveBeenCalledTimes(1); expect(repository.closePointInTime).toHaveBeenCalledTimes(1);
expect(find).toHaveBeenCalledTimes(1); expect(repository.find).toHaveBeenCalledTimes(1);
expect(find).toHaveBeenCalledWith( expect(repository.find).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
pit: expect.objectContaining({ id: 'abc123', keepAlive: '2m' }), pit: expect.objectContaining({ id: 'abc123', keepAlive: '2m' }),
sortField: 'updated_at', sortField: 'updated_at',
@ -147,24 +167,25 @@ describe('createPointInTimeFinder()', () => {
}); });
test('works with multiple pages of results', async () => { test('works with multiple pages of results', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123', id: 'abc123',
}); });
find.mockResolvedValueOnce({ repository.find
total: 2, .mockResolvedValueOnce({
saved_objects: [mockHits[0]], total: 2,
pit_id: 'abc123', saved_objects: [mockHits[0]],
per_page: 1, pit_id: 'abc123',
page: 0, per_page: 1,
}); page: 0,
find.mockResolvedValueOnce({ })
total: 2, .mockResolvedValueOnce({
saved_objects: [mockHits[1]], total: 2,
pit_id: 'abc123', saved_objects: [mockHits[1]],
per_page: 1, pit_id: 'abc123',
page: 0, per_page: 1,
}); page: 0,
find.mockResolvedValueOnce({ });
repository.find.mockResolvedValueOnce({
total: 2, total: 2,
saved_objects: [], saved_objects: [],
per_page: 1, per_page: 1,
@ -180,11 +201,7 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
const hits: SavedObjectsFindResult[] = []; const hits: SavedObjectsFindResult[] = [];
for await (const result of finder.find()) { for await (const result of finder.find()) {
@ -192,12 +209,12 @@ describe('createPointInTimeFinder()', () => {
} }
expect(hits.length).toBe(2); expect(hits.length).toBe(2);
expect(openPointInTimeForType).toHaveBeenCalledTimes(1); expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(1);
expect(closePointInTime).toHaveBeenCalledTimes(1); expect(repository.closePointInTime).toHaveBeenCalledTimes(1);
// called 3 times since we need a 3rd request to check if we // called 3 times since we need a 3rd request to check if we
// are done paginating through results. // are done paginating through results.
expect(find).toHaveBeenCalledTimes(3); expect(repository.find).toHaveBeenCalledTimes(3);
expect(find).toHaveBeenCalledWith( expect(repository.find).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
pit: expect.objectContaining({ id: 'abc123', keepAlive: '2m' }), pit: expect.objectContaining({ id: 'abc123', keepAlive: '2m' }),
sortField: 'updated_at', sortField: 'updated_at',
@ -210,10 +227,10 @@ describe('createPointInTimeFinder()', () => {
describe('#close', () => { describe('#close', () => {
test('calls closePointInTime with correct ID', async () => { test('calls closePointInTime with correct ID', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'test', id: 'test',
}); });
find.mockResolvedValueOnce({ repository.find.mockResolvedValueOnce({
total: 1, total: 1,
saved_objects: [mockHits[0]], saved_objects: [mockHits[0]],
pit_id: 'test', pit_id: 'test',
@ -229,11 +246,7 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
const hits: SavedObjectsFindResult[] = []; const hits: SavedObjectsFindResult[] = [];
for await (const result of finder.find()) { for await (const result of finder.find()) {
@ -241,28 +254,28 @@ describe('createPointInTimeFinder()', () => {
await finder.close(); await finder.close();
} }
expect(closePointInTime).toHaveBeenCalledWith('test'); expect(repository.closePointInTime).toHaveBeenCalledWith('test');
}); });
test('causes generator to stop', async () => { test('causes generator to stop', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'test', id: 'test',
}); });
find.mockResolvedValueOnce({ repository.find.mockResolvedValueOnce({
total: 2, total: 2,
saved_objects: [mockHits[0]], saved_objects: [mockHits[0]],
pit_id: 'test', pit_id: 'test',
per_page: 1, per_page: 1,
page: 0, page: 0,
}); });
find.mockResolvedValueOnce({ repository.find.mockResolvedValueOnce({
total: 2, total: 2,
saved_objects: [mockHits[1]], saved_objects: [mockHits[1]],
pit_id: 'test', pit_id: 'test',
per_page: 1, per_page: 1,
page: 0, page: 0,
}); });
find.mockResolvedValueOnce({ repository.find.mockResolvedValueOnce({
total: 2, total: 2,
saved_objects: [], saved_objects: [],
per_page: 1, per_page: 1,
@ -278,11 +291,7 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
const hits: SavedObjectsFindResult[] = []; const hits: SavedObjectsFindResult[] = [];
for await (const result of finder.find()) { for await (const result of finder.find()) {
@ -290,15 +299,15 @@ describe('createPointInTimeFinder()', () => {
await finder.close(); await finder.close();
} }
expect(closePointInTime).toHaveBeenCalledTimes(1); expect(repository.closePointInTime).toHaveBeenCalledTimes(1);
expect(hits.length).toBe(1); expect(hits.length).toBe(1);
}); });
test('is called if `find` throws an error', async () => { test('is called if `find` throws an error', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'test', id: 'test',
}); });
find.mockRejectedValueOnce(new Error('oops')); repository.find.mockRejectedValueOnce(new Error('oops'));
const findOptions: SavedObjectsCreatePointInTimeFinderOptions = { const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
type: ['visualization'], type: ['visualization'],
@ -308,11 +317,7 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
const hits: SavedObjectsFindResult[] = []; const hits: SavedObjectsFindResult[] = [];
try { try {
@ -323,27 +328,28 @@ describe('createPointInTimeFinder()', () => {
// intentionally empty // intentionally empty
} }
expect(closePointInTime).toHaveBeenCalledWith('test'); expect(repository.closePointInTime).toHaveBeenCalledWith('test');
}); });
test('finder can be reused after closing', async () => { test('finder can be reused after closing', async () => {
openPointInTimeForType.mockResolvedValueOnce({ repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123', id: 'abc123',
}); });
find.mockResolvedValueOnce({ repository.find
total: 2, .mockResolvedValueOnce({
saved_objects: mockHits, total: 2,
pit_id: 'abc123', saved_objects: mockHits,
per_page: 1, pit_id: 'abc123',
page: 0, per_page: 1,
}); page: 0,
find.mockResolvedValueOnce({ })
total: 2, .mockResolvedValueOnce({
saved_objects: mockHits, total: 2,
pit_id: 'abc123', saved_objects: mockHits,
per_page: 1, pit_id: 'abc123',
page: 1, per_page: 1,
}); page: 1,
});
const findOptions: SavedObjectsCreatePointInTimeFinderOptions = { const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
type: ['visualization'], type: ['visualization'],
@ -353,11 +359,7 @@ describe('createPointInTimeFinder()', () => {
const finder = new PointInTimeFinder(findOptions, { const finder = new PointInTimeFinder(findOptions, {
logger, logger,
client: { client: repository,
find,
openPointInTimeForType,
closePointInTime,
},
}); });
const findA = finder.find(); const findA = finder.find();
@ -370,9 +372,9 @@ describe('createPointInTimeFinder()', () => {
expect((await findA.next()).done).toBe(true); expect((await findA.next()).done).toBe(true);
expect((await findB.next()).done).toBe(true); expect((await findB.next()).done).toBe(true);
expect(openPointInTimeForType).toHaveBeenCalledTimes(2); expect(repository.openPointInTimeForType).toHaveBeenCalledTimes(2);
expect(find).toHaveBeenCalledTimes(2); expect(repository.find).toHaveBeenCalledTimes(2);
expect(closePointInTime).toHaveBeenCalledTimes(2); expect(repository.closePointInTime).toHaveBeenCalledTimes(2);
}); });
}); });
}); });

View file

@ -139,7 +139,9 @@ export class PointInTimeFinder<T = unknown, A = unknown>
private async open() { private async open() {
try { 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.#pitId = id;
this.#open = true; this.#open = true;
} catch (e) { } catch (e) {

View file

@ -334,7 +334,7 @@ export interface SavedObjectsResolveResponse<T = unknown> {
/** /**
* @public * @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`. * Optionally specify how long ES should keep the PIT alive until the next request. Defaults to `5m`.
*/ */
@ -343,6 +343,15 @@ export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOpti
* An optional ES preference value to be used for the query. * An optional ES preference value to be used for the query.
*/ */
preference?: string; preference?: 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
*/
namespaces?: string[];
} }
/** /**

View file

@ -2460,8 +2460,9 @@ export interface SavedObjectsMigrationVersion {
export type SavedObjectsNamespaceType = 'single' | 'multiple' | 'multiple-isolated' | 'agnostic'; export type SavedObjectsNamespaceType = 'single' | 'multiple' | 'multiple-isolated' | 'agnostic';
// @public (undocumented) // @public (undocumented)
export interface SavedObjectsOpenPointInTimeOptions extends SavedObjectsBaseOptions { export interface SavedObjectsOpenPointInTimeOptions {
keepAlive?: string; keepAlive?: string;
namespaces?: string[];
preference?: string; preference?: string;
} }

View file

@ -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 () => { test(`checks privileges for user, actions, and namespaces`, async () => {
const options = { type: [type1, type2], namespaces }; const options = { type: [type1, type2], namespaces };
await expectPrivilegeCheck(client.find, { options }, namespaces); await expectPrivilegeCheck(client.find, { options }, namespaces);
@ -884,7 +873,7 @@ describe('#openPointInTimeForType', () => {
const apiCallReturnValue = Symbol(); const apiCallReturnValue = Symbol();
clientOpts.baseClient.openPointInTimeForType.mockReturnValue(apiCallReturnValue as any); clientOpts.baseClient.openPointInTimeForType.mockReturnValue(apiCallReturnValue as any);
const options = { namespace }; const options = { namespaces: [namespace] };
const result = await expectSuccess(client.openPointInTimeForType, { type, options }); const result = await expectSuccess(client.openPointInTimeForType, { type, options });
expect(result).toBe(apiCallReturnValue); expect(result).toBe(apiCallReturnValue);
}); });
@ -892,18 +881,113 @@ describe('#openPointInTimeForType', () => {
test(`adds audit event when successful`, async () => { test(`adds audit event when successful`, async () => {
const apiCallReturnValue = Symbol(); const apiCallReturnValue = Symbol();
clientOpts.baseClient.openPointInTimeForType.mockReturnValue(apiCallReturnValue as any); clientOpts.baseClient.openPointInTimeForType.mockReturnValue(apiCallReturnValue as any);
const options = { namespace }; const options = { namespaces: [namespace] };
await expectSuccess(client.openPointInTimeForType, { type, options }); await expectSuccess(client.openPointInTimeForType, { type, options });
expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1); expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1);
expectAuditEvent('saved_object_open_point_in_time', 'unknown'); expectAuditEvent('saved_object_open_point_in_time', 'unknown');
}); });
test(`adds audit event when not successful`, async () => { test(`throws an error when unauthorized`, async () => {
clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockRejectedValue(new Error()); clientOpts.checkSavedObjectsPrivilegesAsCurrentUser.mockImplementation(
await expect(() => client.openPointInTimeForType(type, { namespace })).rejects.toThrow(); 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); expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1);
expectAuditEvent('saved_object_open_point_in_time', 'failure'); 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', () => { describe('#closePointInTime', () => {

View file

@ -29,7 +29,7 @@ import type {
SavedObjectsUpdateOptions, SavedObjectsUpdateOptions,
} from 'src/core/server'; } 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 { ALL_SPACES_ID, UNKNOWN_SPACE } from '../../common/constants';
import type { AuditLogger, SecurityAuditLogger } from '../audit'; import type { AuditLogger, SecurityAuditLogger } from '../audit';
import { SavedObjectAction, savedObjectEvent } from '../audit'; import { SavedObjectAction, savedObjectEvent } from '../audit';
@ -75,10 +75,12 @@ interface LegacyEnsureAuthorizedResult {
status: 'fully_authorized' | 'partially_authorized' | 'unauthorized'; status: 'fully_authorized' | 'partially_authorized' | 'unauthorized';
typeMap: Map<string, LegacyEnsureAuthorizedTypeResult>; typeMap: Map<string, LegacyEnsureAuthorizedTypeResult>;
} }
interface LegacyEnsureAuthorizedTypeResult { interface LegacyEnsureAuthorizedTypeResult {
authorizedSpaces: string[]; authorizedSpaces: string[];
isGloballyAuthorized?: boolean; isGloballyAuthorized?: boolean;
} }
export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContract { export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContract {
private readonly actions: Actions; private readonly actions: Actions;
private readonly legacyAuditLogger: PublicMethodsOf<SecurityAuditLogger>; private readonly legacyAuditLogger: PublicMethodsOf<SecurityAuditLogger>;
@ -236,11 +238,6 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra
`_find across namespaces is not permitted when the Spaces plugin is disabled.` `_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 args = { options };
const { status, typeMap } = await this.legacyEnsureAuthorized( const { status, typeMap } = await this.legacyEnsureAuthorized(
@ -508,22 +505,27 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra
type: string | string[], type: string | string[],
options: SavedObjectsOpenPointInTimeOptions options: SavedObjectsOpenPointInTimeOptions
) { ) {
try { const args = { type, options };
const args = { type, options }; const { status, typeMap } = await this.legacyEnsureAuthorized(
await this.legacyEnsureAuthorized(type, 'open_point_in_time', options?.namespace, { type,
'open_point_in_time',
options?.namespaces,
{
args, args,
// Partial authorization is acceptable in this case because this method is only designed // Partial authorization is acceptable in this case because this method is only designed
// to be used with `find`, which already allows for partial authorization. // to be used with `find`, which already allows for partial authorization.
requireFullAuthorization: false, requireFullAuthorization: false,
}); }
} catch (error) { );
if (status === 'unauthorized') {
this.auditLogger.log( this.auditLogger.log(
savedObjectEvent({ savedObjectEvent({
action: SavedObjectAction.OPEN_POINT_IN_TIME, action: SavedObjectAction.OPEN_POINT_IN_TIME,
error, error: new Error(status),
}) })
); );
throw error; throw SavedObjectsErrorHelpers.decorateForbiddenError(new Error(status));
} }
this.auditLogger.log( this.auditLogger.log(
@ -533,7 +535,8 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra
}) })
); );
return await this.baseClient.openPointInTimeForType(type, options); 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) { public async closePointInTime(id: string, options?: SavedObjectsClosePointInTimeOptions) {

View file

@ -533,27 +533,94 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces';
}); });
describe('#openPointInTimeForType', () => { describe('#openPointInTimeForType', () => {
test(`throws error if options.namespace is specified`, async () => { test(`throws error if if user is unauthorized in this space`, async () => {
const { client } = createSpacesSavedObjectsClient(); 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( await expect(
ERROR_NAMESPACE_SPECIFIED client.openPointInTimeForType('foo', { namespaces: ['bar'] })
); ).rejects.toThrowError('Bad Request');
expect(baseClient.openPointInTimeForType).not.toHaveBeenCalled();
}); });
test(`supplements options with the current namespace`, async () => { 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>;
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(`translates options.namespaces: ['*']`, 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>;
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 { client, baseClient } = createSpacesSavedObjectsClient();
const expectedReturnValue = { id: 'abc123' }; const expectedReturnValue = { id: 'abc123' };
baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue)); baseClient.openPointInTimeForType.mockReturnValue(Promise.resolve(expectedReturnValue));
const options = Object.freeze({ foo: 'bar' }); const options = Object.freeze({ keepAlive: '2m' });
// @ts-expect-error
const actualReturnValue = await client.openPointInTimeForType('foo', options); const actualReturnValue = await client.openPointInTimeForType('foo', options);
expect(actualReturnValue).toBe(expectedReturnValue); expect(actualReturnValue).toBe(expectedReturnValue);
expect(baseClient.openPointInTimeForType).toHaveBeenCalledWith('foo', { expect(baseClient.openPointInTimeForType).toHaveBeenCalledWith('foo', {
foo: 'bar', keepAlive: '2m',
namespace: currentSpace.expectedNamespace, namespaces: [currentSpace.expectedNamespace ?? DEFAULT_SPACE_ID],
}); });
}); });
}); });

View file

@ -30,7 +30,7 @@ import type {
SavedObjectsUpdateOptions, SavedObjectsUpdateOptions,
} from 'src/core/server'; } 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 { ALL_SPACES_ID } from '../../common/constants';
import { spaceIdToNamespace } from '../lib/utils/namespace'; import { spaceIdToNamespace } from '../lib/utils/namespace';
import type { ISpacesClient } from '../spaces_client'; import type { ISpacesClient } from '../spaces_client';
@ -175,32 +175,19 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract {
* @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page } * @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page }
*/ */
public async find<T = unknown, A = unknown>(options: SavedObjectsFindOptions) { public async find<T = unknown, A = unknown>(options: SavedObjectsFindOptions) {
throwErrorIfNamespaceSpecified(options); let namespaces: string[];
try {
let namespaces = options.namespaces; namespaces = await this.getSearchableSpaces(options.namespaces);
if (namespaces) { } catch (err) {
try { if (Boom.isBoom(err) && err.output.payload.statusCode === 403) {
const availableSpaces = await this.spacesClient.getAll({ purpose: 'findSavedObjects' }); // return empty response, since the user is unauthorized in any space, but we don't return forbidden errors for `find` operations
if (namespaces.includes(ALL_SPACES_ID)) { return SavedObjectsUtils.createEmptyFindResponse<T, A>(options);
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<T, A>(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<T, A>(options);
}
throw err;
} }
} else { throw err;
namespaces = [this.spaceId]; }
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<T, A>(options);
} }
return await this.client.find<T, A>({ return await this.client.find<T, A>({
@ -396,10 +383,15 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract {
type: string | string[], type: string | string[],
options: SavedObjectsOpenPointInTimeOptions = {} 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, { return await this.client.openPointInTimeForType(type, {
...options, ...options,
namespace: spaceIdToNamespace(this.spaceId), namespaces,
}); });
} }
@ -446,4 +438,19 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract {
...dependencies, ...dependencies,
}); });
} }
private async getSearchableSpaces(namespaces?: string[]): Promise<string[]> {
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];
}
}
} }

View file

@ -155,8 +155,16 @@ export function exportTestSuiteFactory(esArchiver: any, supertest: SuperTest<any
if (failure?.reason === 'unauthorized') { if (failure?.reason === 'unauthorized') {
// In export only, the API uses "bulkGet" or "find" depending on the parameters it receives. // In export only, the API uses "bulkGet" or "find" depending on the parameters it receives.
if (failure.statusCode === 403) { if (failure.statusCode === 403) {
// "bulkGet" was unauthorized, which returns a forbidden error if (id) {
await expectSavedObjectForbiddenBulkGet(type)(response); // "bulkGet" was unauthorized, which returns a forbidden error
await expectSavedObjectForbiddenBulkGet(type)(response);
} else {
expect(response.body).to.eql({
statusCode: 403,
error: 'Forbidden',
message: `unauthorized`,
});
}
} else if (failure.statusCode === 200) { } else if (failure.statusCode === 200) {
// "find" was unauthorized, which returns an empty result // "find" was unauthorized, which returns an empty result
expect(response.body).not.to.have.property('error'); expect(response.body).not.to.have.property('error');

View file

@ -52,7 +52,7 @@ export default function ({ getService }: FtrProviderContext) {
return { return {
unauthorized: [ unauthorized: [
createTestDefinitions(exportableObjects, { statusCode: 403, reason: 'unauthorized' }), createTestDefinitions(exportableObjects, { statusCode: 403, reason: 'unauthorized' }),
createTestDefinitions(exportableTypes, { statusCode: 200, reason: 'unauthorized' }), // failure with empty result createTestDefinitions(exportableTypes, { statusCode: 403, reason: 'unauthorized' }),
createTestDefinitions(nonExportableObjectsAndTypes, false), createTestDefinitions(nonExportableObjectsAndTypes, false),
].flat(), ].flat(),
authorized: createTestDefinitions(allObjectsAndTypes, false), authorized: createTestDefinitions(allObjectsAndTypes, false),

View file

@ -52,7 +52,7 @@ export default function ({ getService }: FtrProviderContext) {
return { return {
unauthorized: [ unauthorized: [
createTestDefinitions(exportableObjects, { statusCode: 403, reason: 'unauthorized' }), createTestDefinitions(exportableObjects, { statusCode: 403, reason: 'unauthorized' }),
createTestDefinitions(exportableTypes, { statusCode: 200, reason: 'unauthorized' }), // failure with empty result createTestDefinitions(exportableTypes, { statusCode: 403, reason: 'unauthorized' }), // failure with empty result
createTestDefinitions(nonExportableObjectsAndTypes, false), createTestDefinitions(nonExportableObjectsAndTypes, false),
].flat(), ].flat(),
authorized: createTestDefinitions(allObjectsAndTypes, false), authorized: createTestDefinitions(allObjectsAndTypes, false),