Skip to content

Commit

Permalink
Address feedback on security/spaces clients.
Browse files Browse the repository at this point in the history
  • Loading branch information
lukeelmers committed Feb 9, 2021
1 parent c01638a commit ea6305f
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1809,7 +1809,7 @@ export class SavedObjectsRepository {
* ```
*
* @param {string|Array<string>} type
* @param {object} [options] - {@link SavedObjectsPointInTimeOptions}
* @param {object} [options] - {@link SavedObjectsOpenPointInTimeOptions}
* @property {string} [options.keepAlive]
* @property {string} [options.preference]
* @returns {promise} - { id: string }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ describe('#openPointInTimeForType', () => {
const options = { namespace };
await expectSuccess(client.openPointInTimeForType, { type, options });
expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1);
expectAuditEvent('saved_object_open_point_in_time', EventOutcome.SUCCESS);
expectAuditEvent('saved_object_open_point_in_time', EventOutcome.UNKNOWN);
});

test(`adds audit event when not successful`, async () => {
Expand All @@ -1026,6 +1026,30 @@ describe('#openPointInTimeForType', () => {
});
});

describe('#closePointInTime', () => {
const id = 'abc123';
const namespace = 'some-ns';

test(`returns result of baseClient.closePointInTime`, async () => {
const apiCallReturnValue = Symbol();
clientOpts.baseClient.closePointInTime.mockReturnValue(apiCallReturnValue as any);

const options = { namespace };
const result = await client.closePointInTime(id, options);
expect(result).toBe(apiCallReturnValue);
});

test(`adds audit event`, async () => {
const apiCallReturnValue = Symbol();
clientOpts.baseClient.closePointInTime.mockReturnValue(apiCallReturnValue as any);

const options = { namespace };
await client.closePointInTime(id, options);
expect(clientOpts.auditLogger.log).toHaveBeenCalledTimes(1);
expectAuditEvent('saved_object_close_point_in_time', EventOutcome.UNKNOWN);
});
});

describe('#resolve', () => {
const type = 'foo';
const id = `${type}-id`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -581,27 +581,25 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra
throw error;
}

const pit = await this.baseClient.openPointInTimeForType(type, options);

this.auditLogger.log(
savedObjectEvent({
action: SavedObjectAction.OPEN_POINT_IN_TIME,
outcome: EventOutcome.UNKNOWN,
})
);

return pit;
return await this.baseClient.openPointInTimeForType(type, options);
}

public async closePointInTime(id: string, options?: SavedObjectsClosePointInTimeOptions) {
const response = await this.baseClient.closePointInTime(id, options);

this.auditLogger.log(
savedObjectEvent({
action: SavedObjectAction.CLOSE_POINT_IN_TIME,
outcome: EventOutcome.UNKNOWN,
})
);

return response;
return await this.baseClient.closePointInTime(id, options);
}

private async checkPrivileges(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -615,5 +615,31 @@ const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces';
});
});
});

describe('#closePointInTime', () => {
test(`throws error if options.namespace is specified`, async () => {
const { client } = createSpacesSavedObjectsClient();

await expect(client.closePointInTime('foo', { namespace: 'bar' })).rejects.toThrow(
ERROR_NAMESPACE_SPECIFIED
);
});

test(`supplements options with the current namespace`, async () => {
const { client, baseClient } = createSpacesSavedObjectsClient();
const expectedReturnValue = { succeeded: true, num_freed: 1 };
baseClient.closePointInTime.mockReturnValue(Promise.resolve(expectedReturnValue));

const options = Object.freeze({ foo: 'bar' });
// @ts-expect-error
const actualReturnValue = await client.closePointInTime('foo', options);

expect(actualReturnValue).toBe(expectedReturnValue);
expect(baseClient.closePointInTime).toHaveBeenCalledWith('foo', {
foo: 'bar',
namespace: currentSpace.expectedNamespace,
});
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract {
* The returned `id` can then be passed to `SavedObjects.find` to search against that PIT.
*
* @param {string|Array<string>} type
* @param {object} [options] - {@link SavedObjectsPointInTimeOptions}
* @param {object} [options] - {@link SavedObjectsOpenPointInTimeOptions}
* @property {string} [options.keepAlive]
* @property {string} [options.preference]
* @returns {promise} - { id: string }
Expand All @@ -406,8 +406,16 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract {
* Closes a Point In Time (PIT) by ID. This simply proxies the request to ES
* via the Elasticsearch client, and is included in the Saved Objects Client
* as a convenience for consumers who are using `openPointInTimeForType`.
*
* @param {string} id - ID returned from `openPointInTimeForType`
* @param {object} [options] - {@link SavedObjectsClosePointInTimeOptions}
* @returns {promise} - { succeeded: boolean; num_freed: number }
*/
async closePointInTime(id: string, options?: SavedObjectsClosePointInTimeOptions) {
return await this.client.closePointInTime(id, options);
throwErrorIfNamespaceSpecified(options);
return await this.client.closePointInTime(id, {
...options,
namespace: spaceIdToNamespace(this.spaceId),
});
}
}

0 comments on commit ea6305f

Please sign in to comment.