diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index 573fb0e1be580..adef12454f2d5 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -893,7 +893,7 @@ describe('update()', () => { }, references: [], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: 'my-action', type: 'action', attributes: { @@ -946,7 +946,7 @@ describe('update()', () => { }, references: [], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: 'my-action', type: 'action', attributes: { @@ -972,17 +972,21 @@ describe('update()', () => { name: 'my name', config: {}, }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); - expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toMatchInlineSnapshot(` + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toMatchInlineSnapshot(` Array [ "action", - "my-action", Object { "actionTypeId": "my-action-type", "config": Object {}, "name": "my name", "secrets": Object {}, }, + Object { + "id": "my-action", + "overwrite": true, + "references": Array [], + }, ] `); expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledTimes(1); @@ -1043,7 +1047,7 @@ describe('update()', () => { }, references: [], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: 'my-action', type: 'action', attributes: { @@ -1081,11 +1085,10 @@ describe('update()', () => { c: true, }, }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); - expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toMatchInlineSnapshot(` + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toMatchInlineSnapshot(` Array [ "action", - "my-action", Object { "actionTypeId": "my-action-type", "config": Object { @@ -1096,6 +1099,11 @@ describe('update()', () => { "name": "my name", "secrets": Object {}, }, + Object { + "id": "my-action", + "overwrite": true, + "references": Array [], + }, ] `); }); @@ -1118,7 +1126,7 @@ describe('update()', () => { }, references: [], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: 'my-action', type: 'action', attributes: { diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index 06c9555f3a18d..01015f9046654 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -13,6 +13,7 @@ import { } from 'src/core/server'; import { i18n } from '@kbn/i18n'; +import { omitBy, isUndefined } from 'lodash'; import { ActionTypeRegistry } from './action_type_registry'; import { validateConfig, validateSecrets, ActionExecutorContract } from './lib'; import { @@ -151,8 +152,10 @@ export class ActionsClient { 'update' ); } - const existingObject = await this.unsecuredSavedObjectsClient.get('action', id); - const { actionTypeId } = existingObject.attributes; + const { attributes, references, version } = await this.unsecuredSavedObjectsClient.get< + RawAction + >('action', id); + const { actionTypeId } = attributes; const { name, config, secrets } = action; const actionType = this.actionTypeRegistry.get(actionTypeId); const validatedActionTypeConfig = validateConfig(actionType, config); @@ -160,12 +163,25 @@ export class ActionsClient { this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId); - const result = await this.unsecuredSavedObjectsClient.update('action', id, { - actionTypeId, - name, - config: validatedActionTypeConfig as SavedObjectAttributes, - secrets: validatedActionTypeSecrets as SavedObjectAttributes, - }); + const result = await this.unsecuredSavedObjectsClient.create( + 'action', + { + ...attributes, + actionTypeId, + name, + config: validatedActionTypeConfig as SavedObjectAttributes, + secrets: validatedActionTypeSecrets as SavedObjectAttributes, + }, + omitBy( + { + id, + overwrite: true, + references, + version, + }, + isUndefined + ) + ); return { id, diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index 4b5af942024c0..a6cffb0284815 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -220,7 +220,7 @@ describe('create()', () => { params: {}, ownerId: null, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -270,27 +270,33 @@ describe('create()', () => { test('creates an alert', async () => { const data = getMockData(); + const createdAttributes = { + ...data, + alertTypeId: '123', + schedule: { interval: '10s' }, + params: { + bar: true, + }, + createdAt: '2019-02-12T21:01:22.479Z', + createdBy: 'elastic', + updatedBy: 'elastic', + muteAll: false, + mutedInstanceIds: [], + actions: [ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + ], + }; unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', - attributes: { - alertTypeId: '123', - schedule: { interval: '10s' }, - params: { - bar: true, - }, - createdAt: '2019-02-12T21:01:22.479Z', - actions: [ - { - group: 'default', - actionRef: 'action_0', - actionTypeId: 'test', - params: { - foo: true, - }, - }, - ], - }, + attributes: createdAttributes, references: [ { name: 'action_0', @@ -312,11 +318,11 @@ describe('create()', () => { params: {}, ownerId: null, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { - actions: [], + ...createdAttributes, scheduledTaskId: 'task-123', }, references: [ @@ -342,8 +348,14 @@ describe('create()', () => { }, ], "alertTypeId": "123", + "consumer": "bar", "createdAt": 2019-02-12T21:01:22.479Z, + "createdBy": "elastic", + "enabled": true, "id": "1", + "muteAll": false, + "mutedInstanceIds": Array [], + "name": "abc", "params": Object { "bar": true, }, @@ -351,7 +363,12 @@ describe('create()', () => { "interval": "10s", }, "scheduledTaskId": "task-123", + "tags": Array [ + "foo", + ], + "throttle": null, "updatedAt": 2019-02-12T21:01:22.479Z, + "updatedBy": "elastic", } `); expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); @@ -531,7 +548,7 @@ describe('create()', () => { params: {}, ownerId: null, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -965,7 +982,7 @@ describe('create()', () => { params: {}, ownerId: null, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -1081,7 +1098,7 @@ describe('create()', () => { params: {}, ownerId: null, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -1175,6 +1192,16 @@ describe('enable()', () => { alertsClientParams.createAPIKey.mockResolvedValue({ apiKeysEnabled: false, }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + ...existingAlert, + attributes: { + ...existingAlert.attributes, + enabled: true, + apiKey: null, + apiKeyOwner: null, + updatedBy: 'elastic', + }, + }); taskManager.schedule.mockResolvedValue({ id: 'task-123', scheduledAt: new Date(), @@ -1233,6 +1260,17 @@ describe('enable()', () => { }); test('enables an alert', async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + ...existingAlert, + attributes: { + ...existingAlert.attributes, + enabled: true, + apiKey: null, + apiKeyOwner: null, + updatedBy: 'elastic', + }, + }); + await alertsClient.enable({ id: '1' }); expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { @@ -1317,7 +1355,7 @@ describe('enable()', () => { await alertsClient.enable({ id: '1' }); expect(alertsClientParams.getUserName).not.toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).not.toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); }); @@ -1384,6 +1422,7 @@ describe('enable()', () => { }); test('throws error when failing to update the first time', async () => { + unsecuredSavedObjectsClient.update.mockReset(); unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail to update')); await expect(alertsClient.enable({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( @@ -1396,6 +1435,7 @@ describe('enable()', () => { }); test('throws error when failing to update the second time', async () => { + unsecuredSavedObjectsClient.update.mockReset(); unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ ...existingAlert, attributes: { @@ -1460,6 +1500,8 @@ describe('disable()', () => { ...existingAlert.attributes, apiKey: Buffer.from('123:abc').toString('base64'), }, + version: '123', + references: [], }; beforeEach(() => { @@ -1501,13 +1543,13 @@ describe('disable()', () => { consumer: 'myApp', schedule: { interval: '10s' }, alertTypeId: 'myType', - apiKey: null, - apiKeyOwner: null, enabled: false, meta: { versionApiKeyLastmodified: kibanaVersion, }, scheduledTaskId: null, + apiKey: null, + apiKeyOwner: null, updatedBy: 'elastic', actions: [ { @@ -1544,13 +1586,13 @@ describe('disable()', () => { consumer: 'myApp', schedule: { interval: '10s' }, alertTypeId: 'myType', - apiKey: null, - apiKeyOwner: null, enabled: false, meta: { versionApiKeyLastmodified: kibanaVersion, }, scheduledTaskId: null, + apiKey: null, + apiKeyOwner: null, updatedBy: 'elastic', actions: [ { @@ -1739,6 +1781,7 @@ describe('unmuteAll()', () => { muteAll: true, }, references: [], + version: '123', }); await alertsClient.unmuteAll({ id: '1' }); @@ -1829,7 +1872,9 @@ describe('muteInstance()', () => { mutedInstanceIds: ['2'], updatedBy: 'elastic', }, - { version: '123' } + { + version: '123', + } ); }); @@ -1850,7 +1895,7 @@ describe('muteInstance()', () => { }); await alertsClient.muteInstance({ alertId: '1', alertInstanceId: '2' }); - expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test('skips muting when alert is muted', async () => { @@ -1871,7 +1916,7 @@ describe('muteInstance()', () => { }); await alertsClient.muteInstance({ alertId: '1', alertInstanceId: '2' }); - expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); describe('authorization', () => { @@ -1983,7 +2028,7 @@ describe('unmuteInstance()', () => { }); await alertsClient.unmuteInstance({ alertId: '1', alertInstanceId: '2' }); - expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test('skips unmuting when alert is muted', async () => { @@ -2004,7 +2049,7 @@ describe('unmuteInstance()', () => { }); await alertsClient.unmuteInstance({ alertId: '1', alertInstanceId: '2' }); - expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); describe('authorization', () => { @@ -3052,7 +3097,7 @@ describe('update()', () => { }); test('updates given parameters', async () => { - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -3189,11 +3234,10 @@ describe('update()', () => { namespace: 'default', }); expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); - expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toHaveLength(4); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][0]).toEqual('alert'); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][1]).toEqual('1'); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot(` + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(3); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` Object { "actions": Array [ Object { @@ -3244,8 +3288,10 @@ describe('update()', () => { "updatedBy": "elastic", } `); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` + expect(unsecuredSavedObjectsClient.create.mock.calls[0][2]).toMatchInlineSnapshot(` Object { + "id": "1", + "overwrite": true, "references": Array [ Object { "id": "1", @@ -3286,7 +3332,7 @@ describe('update()', () => { apiKeysEnabled: true, result: { id: '123', name: '123', api_key: 'abc' }, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -3365,11 +3411,10 @@ describe('update()', () => { "updatedAt": 2019-02-12T21:01:22.479Z, } `); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); - expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toHaveLength(4); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][0]).toEqual('alert'); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][1]).toEqual('1'); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot(` + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(3); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` Object { "actions": Array [ Object { @@ -3404,18 +3449,20 @@ describe('update()', () => { "updatedBy": "elastic", } `); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - "version": "123", - } - `); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][2]).toMatchInlineSnapshot(` + Object { + "id": "1", + "overwrite": true, + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + "version": "123", + } + `); }); it(`doesn't call the createAPIKey function when alert is disabled`, async () => { @@ -3439,7 +3486,7 @@ describe('update()', () => { }, ], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -3519,11 +3566,10 @@ describe('update()', () => { "updatedAt": 2019-02-12T21:01:22.479Z, } `); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); - expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toHaveLength(4); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][0]).toEqual('alert'); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][1]).toEqual('1'); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot(` + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(3); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` Object { "actions": Array [ Object { @@ -3558,18 +3604,20 @@ describe('update()', () => { "updatedBy": "elastic", } `); - expect(unsecuredSavedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - "version": "123", - } - `); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][2]).toMatchInlineSnapshot(` + Object { + "id": "1", + "overwrite": true, + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + "version": "123", + } + `); }); it('should validate params', async () => { @@ -3627,7 +3675,7 @@ describe('update()', () => { }, ], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -3686,7 +3734,7 @@ describe('update()', () => { }, ], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -3765,7 +3813,7 @@ describe('update()', () => { }, ], }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { @@ -3919,7 +3967,7 @@ describe('update()', () => { params: {}, ownerId: null, }); - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: alertId, type: 'alert', attributes: { @@ -4091,7 +4139,7 @@ describe('update()', () => { describe('authorization', () => { beforeEach(() => { - unsecuredSavedObjectsClient.update.mockResolvedValueOnce({ + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'alert', attributes: { diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 0a08ca848c73d..671b1d6411d7f 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -251,7 +251,7 @@ export class AlertsClient { } throw e; } - await this.unsecuredSavedObjectsClient.update('alert', createdAlert.id, { + await this.unsecuredSavedObjectsClient.update('alert', createdAlert.id, { scheduledTaskId: scheduledTask.id, }); createdAlert.attributes.scheduledTaskId = scheduledTask.id; @@ -488,9 +488,8 @@ export class AlertsClient { : null; const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username); - const updatedObject = await this.unsecuredSavedObjectsClient.update( + const updatedObject = await this.unsecuredSavedObjectsClient.create( 'alert', - id, this.updateMeta({ ...attributes, ...data, @@ -500,6 +499,8 @@ export class AlertsClient { updatedBy: username, }), { + id, + overwrite: true, version, references, } @@ -798,6 +799,7 @@ export class AlertsClient { 'alert', alertId ); + await this.authorization.ensureAuthorized( attributes.alertTypeId, attributes.consumer, @@ -809,7 +811,7 @@ export class AlertsClient { const mutedInstanceIds = attributes.mutedInstanceIds || []; if (!attributes.muteAll && mutedInstanceIds.includes(alertInstanceId)) { - await this.unsecuredSavedObjectsClient.update( + await this.unsecuredSavedObjectsClient.update( 'alert', alertId, this.updateMeta({ diff --git a/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts b/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts index f873b0178ece9..aca447b6adedd 100644 --- a/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts +++ b/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts @@ -19,6 +19,7 @@ import { AlertInstanceContext, AlertType, AlertTypeParams, + RawAlert, } from '../types'; interface CreateExecutionHandlerOptions { @@ -28,7 +29,7 @@ interface CreateExecutionHandlerOptions { actionsPlugin: ActionsPluginStartContract; actions: AlertAction[]; spaceId: string; - apiKey: string | null; + apiKey: RawAlert['apiKey']; alertType: AlertType; logger: Logger; eventLogger: IEventLogger; @@ -99,7 +100,7 @@ export function createExecutionHandler({ id: action.id, params: action.params, spaceId, - apiKey, + apiKey: apiKey ?? null, source: asSavedObjectExecutionSource({ id: alertId, type: 'alert', diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index 4c16d23b485b5..5be684eca4651 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -73,7 +73,7 @@ export class TaskRunner { return apiKey; } - private getFakeKibanaRequest(spaceId: string, apiKey: string | null) { + private getFakeKibanaRequest(spaceId: string, apiKey: RawAlert['apiKey']) { const requestHeaders: Record = {}; if (apiKey) { @@ -98,7 +98,7 @@ export class TaskRunner { private getServicesWithSpaceLevelPermissions( spaceId: string, - apiKey: string | null + apiKey: RawAlert['apiKey'] ): [Services, PublicMethodsOf] { const request = this.getFakeKibanaRequest(spaceId, apiKey); return [this.context.getServices(request), this.context.getAlertsClientWithRequest(request)]; @@ -109,7 +109,7 @@ export class TaskRunner { alertName: string, tags: string[] | undefined, spaceId: string, - apiKey: string | null, + apiKey: RawAlert['apiKey'], actions: Alert['actions'], alertParams: RawAlert['params'] ) { @@ -250,7 +250,11 @@ export class TaskRunner { }; } - async validateAndExecuteAlert(services: Services, apiKey: string | null, alert: SanitizedAlert) { + async validateAndExecuteAlert( + services: Services, + apiKey: RawAlert['apiKey'], + alert: SanitizedAlert + ) { const { params: { alertId, spaceId }, } = this.taskInstance; diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts index 18834f55af0a5..e516ab1cb73b3 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts @@ -82,6 +82,58 @@ describe('#create', () => { expect(mockBaseClient.create).not.toHaveBeenCalled(); }); + it('allows a specified ID when overwriting an existing object', async () => { + const attributes = { + attrOne: 'one', + attrSecret: 'secret', + attrNotSoSecret: 'not-so-secret', + attrThree: 'three', + }; + const options = { id: 'predefined-uuid', overwrite: true, version: 'some-version' }; + const mockedResponse = { + id: 'predefined-uuid', + type: 'known-type', + attributes: { + attrOne: 'one', + attrSecret: '*secret*', + attrNotSoSecret: '*not-so-secret*', + attrThree: 'three', + }, + references: [], + }; + + mockBaseClient.create.mockResolvedValue(mockedResponse); + + expect(await wrapper.create('known-type', attributes, options)).toEqual({ + ...mockedResponse, + attributes: { attrOne: 'one', attrNotSoSecret: 'not-so-secret', attrThree: 'three' }, + }); + + expect(encryptedSavedObjectsServiceMockInstance.encryptAttributes).toHaveBeenCalledTimes(1); + expect(encryptedSavedObjectsServiceMockInstance.encryptAttributes).toHaveBeenCalledWith( + { type: 'known-type', id: 'predefined-uuid' }, + { + attrOne: 'one', + attrSecret: 'secret', + attrNotSoSecret: 'not-so-secret', + attrThree: 'three', + }, + { user: mockAuthenticatedUser() } + ); + + expect(mockBaseClient.create).toHaveBeenCalledTimes(1); + expect(mockBaseClient.create).toHaveBeenCalledWith( + 'known-type', + { + attrOne: 'one', + attrSecret: '*secret*', + attrNotSoSecret: '*not-so-secret*', + attrThree: 'three', + }, + { id: 'predefined-uuid', overwrite: true, version: 'some-version' } + ); + }); + it('generates ID, encrypts attributes and strips them from response except for ones with `dangerouslyExposeValue` set to `true`', async () => { const attributes = { attrOne: 'one', @@ -262,6 +314,77 @@ describe('#bulkCreate', () => { expect(mockBaseClient.bulkCreate).not.toHaveBeenCalled(); }); + it('allows a specified ID when overwriting an existing object', async () => { + const attributes = { + attrOne: 'one', + attrSecret: 'secret', + attrNotSoSecret: 'not-so-secret', + attrThree: 'three', + }; + const mockedResponse = { + saved_objects: [ + { + id: 'predefined-uuid', + type: 'known-type', + attributes: { ...attributes, attrSecret: '*secret*', attrNotSoSecret: '*not-so-secret*' }, + references: [], + }, + { + id: 'some-id', + type: 'unknown-type', + attributes, + references: [], + }, + ], + }; + + mockBaseClient.bulkCreate.mockResolvedValue(mockedResponse); + + const bulkCreateParams = [ + { id: 'predefined-uuid', type: 'known-type', attributes, version: 'some-version' }, + { type: 'unknown-type', attributes }, + ]; + + await expect(wrapper.bulkCreate(bulkCreateParams, { overwrite: true })).resolves.toEqual({ + saved_objects: [ + { + ...mockedResponse.saved_objects[0], + attributes: { attrOne: 'one', attrNotSoSecret: 'not-so-secret', attrThree: 'three' }, + }, + mockedResponse.saved_objects[1], + ], + }); + + expect(encryptedSavedObjectsServiceMockInstance.encryptAttributes).toHaveBeenCalledTimes(1); + expect(encryptedSavedObjectsServiceMockInstance.encryptAttributes).toHaveBeenCalledWith( + { type: 'known-type', id: 'predefined-uuid' }, + { + attrOne: 'one', + attrSecret: 'secret', + attrNotSoSecret: 'not-so-secret', + attrThree: 'three', + }, + { user: mockAuthenticatedUser() } + ); + + expect(mockBaseClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(mockBaseClient.bulkCreate).toHaveBeenCalledWith( + [ + { + ...bulkCreateParams[0], + attributes: { + attrOne: 'one', + attrSecret: '*secret*', + attrNotSoSecret: '*not-so-secret*', + attrThree: 'three', + }, + }, + bulkCreateParams[1], + ], + { overwrite: true } + ); + }); + it('generates ID, encrypts attributes and strips them from response except for ones with `dangerouslyExposeValue` set to `true`', async () => { const attributes = { attrOne: 'one', diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts index 0eeb9943b5be9..eef389186d670 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts @@ -68,13 +68,18 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon // Saved objects with encrypted attributes should have IDs that are hard to guess especially // since IDs are part of the AAD used during encryption, that's why we control them within this // wrapper and don't allow consumers to specify their own IDs directly. - if (options.id) { + + // only allow a specified ID if we're overwriting an existing ESO with a Version + // this helps us ensure that the document really was previously created using ESO + // and not being used to get around the specified ID limitation + const canSpecifyID = options.overwrite && options.version; + if (options.id && !canSpecifyID) { throw new Error( 'Predefined IDs are not allowed for saved objects with encrypted attributes.' ); } - const id = generateID(); + const id = options.id ?? generateID(); const namespace = getDescriptorNamespace( this.options.baseTypeRegistry, type, @@ -97,7 +102,7 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon public async bulkCreate( objects: Array>, - options?: SavedObjectsBaseOptions + options?: SavedObjectsBaseOptions & Pick ) { // We encrypt attributes for every object in parallel and that can potentially exhaust libuv or // NodeJS thread pool. If it turns out to be a problem, we can consider switching to the @@ -110,14 +115,15 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon // Saved objects with encrypted attributes should have IDs that are hard to guess especially // since IDs are part of the AAD used during encryption, that's why we control them within this - // wrapper and don't allow consumers to specify their own IDs directly. - if (object.id) { + // wrapper and don't allow consumers to specify their own IDs directly unless overwriting the original document. + const canSpecifyID = options?.overwrite && object.version; + if (object.id && !canSpecifyID) { throw new Error( 'Predefined IDs are not allowed for saved objects with encrypted attributes.' ); } - const id = generateID(); + const id = object.id ?? generateID(); const namespace = getDescriptorNamespace( this.options.baseTypeRegistry, object.type, diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index 02d39bba42659..bd8c72f57d8f2 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -138,7 +138,13 @@ export function defineRoutes( const savedObjectsWithAlerts = await savedObjects.getScopedClient(req, { includedHiddenTypes: ['alert'], }); - const result = await savedObjectsWithAlerts.update(type, id, attributes, options); + const savedAlert = await savedObjectsWithAlerts.get(type, id); + const result = await savedObjectsWithAlerts.update( + type, + id, + { ...savedAlert.attributes, ...attributes }, + options + ); return res.ok({ body: result }); } ); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts index d82d116396cd6..ef14dd9ec2eff 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts @@ -141,6 +141,77 @@ export default function webhookTest({ getService }: FtrProviderContext) { }); }); + it('should remove headers when a webhook is updated', async () => { + const { body: createdAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'test') + .send({ + name: 'A generic Webhook action', + actionTypeId: '.webhook', + secrets: { + user: 'username', + password: 'mypassphrase', + }, + config: { + url: webhookSimulatorURL, + headers: { + someHeader: '123', + }, + }, + }) + .expect(200); + + expect(createdAction).to.eql({ + id: createdAction.id, + isPreconfigured: false, + name: 'A generic Webhook action', + actionTypeId: '.webhook', + config: { + ...defaultValues, + url: webhookSimulatorURL, + headers: { + someHeader: '123', + }, + }, + }); + + await supertest + .put(`/api/actions/action/${createdAction.id}`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'A generic Webhook action', + secrets: { + user: 'username', + password: 'mypassphrase', + }, + config: { + url: webhookSimulatorURL, + headers: { + someOtherHeader: '456', + }, + }, + }) + .expect(200); + + const { body: fetchedAction } = await supertest + .get(`/api/actions/action/${createdAction.id}`) + .expect(200); + + expect(fetchedAction).to.eql({ + id: fetchedAction.id, + isPreconfigured: false, + name: 'A generic Webhook action', + actionTypeId: '.webhook', + config: { + ...defaultValues, + url: webhookSimulatorURL, + headers: { + someOtherHeader: '456', + }, + }, + }); + }); + it('should send authentication to the webhook target', async () => { const webhookActionId = await createWebhookAction(webhookSimulatorURL, {}, kibanaURL); const { body: result } = await supertest