diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index 801c2c8775361..7bf29a6f2d0a6 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -1227,33 +1227,26 @@ describe('enable()', () => { }); expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - schedule: { interval: '10s' }, - alertTypeId: 'myType', - consumer: 'myApp', - enabled: true, - updatedBy: 'elastic', - apiKey: null, - apiKeyOwner: null, - actions: [ - { - group: 'default', - id: '1', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + schedule: { interval: '10s' }, + alertTypeId: 'myType', + consumer: 'myApp', + enabled: true, + updatedBy: 'elastic', + apiKey: null, + apiKeyOwner: null, + actions: [ + { + group: 'default', + id: '1', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, }, - ], - }, - { - version: '123', - } - ); + }, + ], + }); expect(taskManager.schedule).toHaveBeenCalledWith({ taskType: `alerting:myType`, params: { @@ -1312,33 +1305,26 @@ describe('enable()', () => { }); await alertsClient.enable({ id: '1' }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - schedule: { interval: '10s' }, - alertTypeId: 'myType', - consumer: 'myApp', - enabled: true, - apiKey: Buffer.from('123:abc').toString('base64'), - apiKeyOwner: 'elastic', - updatedBy: 'elastic', - actions: [ - { - group: 'default', - id: '1', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + schedule: { interval: '10s' }, + alertTypeId: 'myType', + consumer: 'myApp', + enabled: true, + apiKey: Buffer.from('123:abc').toString('base64'), + apiKeyOwner: 'elastic', + updatedBy: 'elastic', + actions: [ + { + group: 'default', + id: '1', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, }, - ], - }, - { - version: '123', - } - ); + }, + ], + }); }); test('falls back when failing to getDecryptedAsInternalUser', async () => { @@ -1475,34 +1461,27 @@ describe('disable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - consumer: 'myApp', - schedule: { interval: '10s' }, - alertTypeId: 'myType', - apiKey: null, - apiKeyOwner: null, - enabled: false, - scheduledTaskId: null, - updatedBy: 'elastic', - actions: [ - { - group: 'default', - id: '1', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + consumer: 'myApp', + schedule: { interval: '10s' }, + alertTypeId: 'myType', + apiKey: null, + apiKeyOwner: null, + enabled: false, + scheduledTaskId: null, + updatedBy: 'elastic', + actions: [ + { + group: 'default', + id: '1', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, }, - ], - }, - { - version: '123', - } - ); + }, + ], + }); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); }); @@ -1515,34 +1494,27 @@ describe('disable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - consumer: 'myApp', - schedule: { interval: '10s' }, - alertTypeId: 'myType', - apiKey: null, - apiKeyOwner: null, - enabled: false, - scheduledTaskId: null, - updatedBy: 'elastic', - actions: [ - { - group: 'default', - id: '1', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + consumer: 'myApp', + schedule: { interval: '10s' }, + alertTypeId: 'myType', + apiKey: null, + apiKeyOwner: null, + enabled: false, + scheduledTaskId: null, + updatedBy: 'elastic', + actions: [ + { + group: 'default', + id: '1', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, }, - ], - }, - { - version: '123', - } - ); + }, + ], + }); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); @@ -1797,15 +1769,10 @@ describe('muteInstance()', () => { }); await alertsClient.muteInstance({ alertId: '1', alertInstanceId: '2' }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - mutedInstanceIds: ['2'], - updatedBy: 'elastic', - }, - { version: '123' } - ); + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + mutedInstanceIds: ['2'], + updatedBy: 'elastic', + }); }); test('skips muting when alert instance already muted', async () => { @@ -1930,15 +1897,10 @@ describe('unmuteInstance()', () => { }); await alertsClient.unmuteInstance({ alertId: '1', alertInstanceId: '2' }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - mutedInstanceIds: [], - updatedBy: 'elastic', - }, - { version: '123' } - ); + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + mutedInstanceIds: [], + updatedBy: 'elastic', + }); }); test('skips unmuting when alert instance not muted', async () => { @@ -3233,7 +3195,6 @@ describe('update()', () => { "type": "action", }, ], - "version": "123", } `); }); @@ -3372,17 +3333,16 @@ describe('update()', () => { } `); expect(unsecuredSavedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - "version": "123", - } - `); + Object { + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + } + `); }); it(`doesn't call the createAPIKey function when alert is disabled`, async () => { @@ -3523,17 +3483,16 @@ describe('update()', () => { } `); expect(unsecuredSavedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - "version": "123", - } - `); + Object { + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + } + `); }); it('should validate params', async () => { @@ -4170,31 +4129,26 @@ describe('updateApiKey()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - schedule: { interval: '10s' }, - alertTypeId: 'myType', - consumer: 'myApp', - enabled: true, - apiKey: Buffer.from('234:abc').toString('base64'), - apiKeyOwner: 'elastic', - updatedBy: 'elastic', - actions: [ - { - group: 'default', - id: '1', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + schedule: { interval: '10s' }, + alertTypeId: 'myType', + consumer: 'myApp', + enabled: true, + apiKey: Buffer.from('234:abc').toString('base64'), + apiKeyOwner: 'elastic', + updatedBy: 'elastic', + actions: [ + { + group: 'default', + id: '1', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, }, - ], - }, - { version: '123' } - ); + }, + ], + }); expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); }); @@ -4206,31 +4160,26 @@ describe('updateApiKey()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( - 'alert', - '1', - { - schedule: { interval: '10s' }, - alertTypeId: 'myType', - consumer: 'myApp', - enabled: true, - apiKey: Buffer.from('234:abc').toString('base64'), - apiKeyOwner: 'elastic', - updatedBy: 'elastic', - actions: [ - { - group: 'default', - id: '1', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, + expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', { + schedule: { interval: '10s' }, + alertTypeId: 'myType', + consumer: 'myApp', + enabled: true, + apiKey: Buffer.from('234:abc').toString('base64'), + apiKeyOwner: 'elastic', + updatedBy: 'elastic', + actions: [ + { + group: 'default', + id: '1', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, }, - ], - }, - { version: '123' } - ); + }, + ], + }); expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 0703a1e13937c..258b5565b18c9 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -47,6 +47,10 @@ import { parseIsoOrRelativeDate } from './lib/iso_or_relative_date'; import { alertInstanceSummaryFromEventLog } from './lib/alert_instance_summary_from_event_log'; import { IEvent } from '../../event_log/server'; import { parseDuration } from '../common/parse_duration'; +import { + AlertAttributesExcludedFromAAD, + AlertAttributesExcludedFromAADType, +} from './saved_objects'; export interface RegistryAlertTypeWithAuth extends RegistryAlertType { authorizedConsumers: string[]; @@ -468,7 +472,7 @@ export class AlertsClient { private async updateAlert( { id, data }: UpdateOptions, - { attributes, version }: SavedObject + { attributes }: SavedObject ): Promise { const alertType = this.alertTypeRegistry.get(attributes.alertTypeId); @@ -495,7 +499,6 @@ export class AlertsClient { updatedBy: username, }, { - version, references, } ); @@ -526,7 +529,6 @@ export class AlertsClient { public async updateApiKey({ id }: { id: string }) { let apiKeyToInvalidate: string | null = null; let attributes: RawAlert; - let version: string | undefined; try { const decryptedAlert = await this.encryptedSavedObjectsClient.getDecryptedAsInternalUser< @@ -534,16 +536,14 @@ export class AlertsClient { >('alert', id, { namespace: this.namespace }); apiKeyToInvalidate = decryptedAlert.attributes.apiKey; attributes = decryptedAlert.attributes; - version = decryptedAlert.version; } catch (e) { // We'll skip invalidating the API key since we failed to load the decrypted saved object this.logger.error( `updateApiKey(): Failed to load API key to invalidate on alert ${id}: ${e.message}` ); - // Still attempt to load the attributes and version using SOC + // Still attempt to load the attributes using SOC const alert = await this.unsecuredSavedObjectsClient.get('alert', id); attributes = alert.attributes; - version = alert.version; } await this.authorization.ensureAuthorized( attributes.alertTypeId, @@ -556,19 +556,14 @@ export class AlertsClient { } const username = await this.getUserName(); - await this.unsecuredSavedObjectsClient.update( - 'alert', - id, - { - ...attributes, - ...this.apiKeyAsAlertAttributes( - await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)), - username - ), - updatedBy: username, - }, - { version } - ); + await this.unsecuredSavedObjectsClient.update('alert', id, { + ...attributes, + ...this.apiKeyAsAlertAttributes( + await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)), + username + ), + updatedBy: username, + }); if (apiKeyToInvalidate) { await this.invalidateApiKey({ apiKey: apiKeyToInvalidate }); @@ -594,7 +589,6 @@ export class AlertsClient { public async enable({ id }: { id: string }) { let apiKeyToInvalidate: string | null = null; let attributes: RawAlert; - let version: string | undefined; try { const decryptedAlert = await this.encryptedSavedObjectsClient.getDecryptedAsInternalUser< @@ -602,16 +596,14 @@ export class AlertsClient { >('alert', id, { namespace: this.namespace }); apiKeyToInvalidate = decryptedAlert.attributes.apiKey; attributes = decryptedAlert.attributes; - version = decryptedAlert.version; } catch (e) { // We'll skip invalidating the API key since we failed to load the decrypted saved object this.logger.error( `enable(): Failed to load API key to invalidate on alert ${id}: ${e.message}` ); - // Still attempt to load the attributes and version using SOC + // Still attempt to load the attributes using SOC const alert = await this.unsecuredSavedObjectsClient.get('alert', id); attributes = alert.attributes; - version = alert.version; } await this.authorization.ensureAuthorized( @@ -626,22 +618,15 @@ export class AlertsClient { if (attributes.enabled === false) { const username = await this.getUserName(); - await this.unsecuredSavedObjectsClient.update( - 'alert', - id, - { - ...attributes, - enabled: true, - ...this.apiKeyAsAlertAttributes( - await this.createAPIKey( - this.generateAPIKeyName(attributes.alertTypeId, attributes.name) - ), - username - ), - updatedBy: username, - }, - { version } - ); + await this.unsecuredSavedObjectsClient.update('alert', id, { + ...attributes, + enabled: true, + ...this.apiKeyAsAlertAttributes( + await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)), + username + ), + updatedBy: username, + }); const scheduledTask = await this.scheduleAlert(id, attributes.alertTypeId); await this.unsecuredSavedObjectsClient.update('alert', id, { scheduledTaskId: scheduledTask.id, @@ -655,7 +640,6 @@ export class AlertsClient { public async disable({ id }: { id: string }) { let apiKeyToInvalidate: string | null = null; let attributes: RawAlert; - let version: string | undefined; try { const decryptedAlert = await this.encryptedSavedObjectsClient.getDecryptedAsInternalUser< @@ -663,16 +647,14 @@ export class AlertsClient { >('alert', id, { namespace: this.namespace }); apiKeyToInvalidate = decryptedAlert.attributes.apiKey; attributes = decryptedAlert.attributes; - version = decryptedAlert.version; } catch (e) { // We'll skip invalidating the API key since we failed to load the decrypted saved object this.logger.error( `disable(): Failed to load API key to invalidate on alert ${id}: ${e.message}` ); - // Still attempt to load the attributes and version using SOC + // Still attempt to load the attributes using SOC const alert = await this.unsecuredSavedObjectsClient.get('alert', id); attributes = alert.attributes; - version = alert.version; } await this.authorization.ensureAuthorized( @@ -682,19 +664,14 @@ export class AlertsClient { ); if (attributes.enabled === true) { - await this.unsecuredSavedObjectsClient.update( - 'alert', - id, - { - ...attributes, - enabled: false, - scheduledTaskId: null, - apiKey: null, - apiKeyOwner: null, - updatedBy: await this.getUserName(), - }, - { version } - ); + await this.unsecuredSavedObjectsClient.update('alert', id, { + ...attributes, + enabled: false, + scheduledTaskId: null, + apiKey: null, + apiKeyOwner: null, + updatedBy: await this.getUserName(), + }); await Promise.all([ attributes.scheduledTaskId @@ -717,7 +694,7 @@ export class AlertsClient { await this.actionsAuthorization.ensureAuthorized('execute'); } - await this.unsecuredSavedObjectsClient.update('alert', id, { + await partiallyUpdateAlertSavedObject(this.unsecuredSavedObjectsClient, id, { muteAll: true, mutedInstanceIds: [], updatedBy: await this.getUserName(), @@ -736,7 +713,7 @@ export class AlertsClient { await this.actionsAuthorization.ensureAuthorized('execute'); } - await this.unsecuredSavedObjectsClient.update('alert', id, { + await partiallyUpdateAlertSavedObject(this.unsecuredSavedObjectsClient, id, { muteAll: false, mutedInstanceIds: [], updatedBy: await this.getUserName(), @@ -744,10 +721,7 @@ export class AlertsClient { } public async muteInstance({ alertId, alertInstanceId }: MuteOptions) { - const { attributes, version } = await this.unsecuredSavedObjectsClient.get( - 'alert', - alertId - ); + const { attributes } = await this.unsecuredSavedObjectsClient.get('alert', alertId); await this.authorization.ensureAuthorized( attributes.alertTypeId, @@ -762,15 +736,10 @@ export class AlertsClient { const mutedInstanceIds = attributes.mutedInstanceIds || []; if (!attributes.muteAll && !mutedInstanceIds.includes(alertInstanceId)) { mutedInstanceIds.push(alertInstanceId); - await this.unsecuredSavedObjectsClient.update( - 'alert', - alertId, - { - mutedInstanceIds, - updatedBy: await this.getUserName(), - }, - { version } - ); + await partiallyUpdateAlertSavedObject(this.unsecuredSavedObjectsClient, alertId, { + mutedInstanceIds, + updatedBy: await this.getUserName(), + }); } } @@ -781,10 +750,7 @@ export class AlertsClient { alertId: string; alertInstanceId: string; }) { - const { attributes, version } = await this.unsecuredSavedObjectsClient.get( - 'alert', - alertId - ); + const { attributes } = await this.unsecuredSavedObjectsClient.get('alert', alertId); await this.authorization.ensureAuthorized( attributes.alertTypeId, attributes.consumer, @@ -796,16 +762,10 @@ export class AlertsClient { const mutedInstanceIds = attributes.mutedInstanceIds || []; if (!attributes.muteAll && mutedInstanceIds.includes(alertInstanceId)) { - await this.unsecuredSavedObjectsClient.update( - 'alert', - alertId, - { - updatedBy: await this.getUserName(), - - mutedInstanceIds: mutedInstanceIds.filter((id: string) => id !== alertInstanceId), - }, - { version } - ); + await partiallyUpdateAlertSavedObject(this.unsecuredSavedObjectsClient, alertId, { + updatedBy: await this.getUserName(), + mutedInstanceIds: mutedInstanceIds.filter((id: string) => id !== alertInstanceId), + }); } } @@ -947,6 +907,20 @@ export class AlertsClient { } } +type PartiallyUpdateableAlertAttributes = Partial< + Pick +>; + +// Specialized partial update for fields which do not contribute to AAD. +async function partiallyUpdateAlertSavedObject( + savedObjectsClient: SavedObjectsClientContract, + id: string, + attributes: PartiallyUpdateableAlertAttributes +): Promise> { + const attributeUpdates = pick(attributes, AlertAttributesExcludedFromAAD); + return await savedObjectsClient.update('alert', id, attributeUpdates); +} + function parseDate(dateString: string | undefined, propertyName: string, defaultValue: Date): Date { if (dateString === undefined) { return defaultValue; diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 06ce8d673e6b7..8c2b533164fe6 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -9,6 +9,20 @@ import mappings from './mappings.json'; import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; +export const AlertAttributesExcludedFromAAD = [ + 'scheduledTaskId', + 'muteAll', + 'mutedInstanceIds', + 'updatedBy', +]; + +// useful for Pick +export type AlertAttributesExcludedFromAADType = + | 'scheduledTaskId' + | 'muteAll' + | 'mutedInstanceIds' + | 'updatedBy'; + export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, encryptedSavedObjects: EncryptedSavedObjectsPluginSetup @@ -25,11 +39,6 @@ export function setupSavedObjects( encryptedSavedObjects.registerType({ type: 'alert', attributesToEncrypt: new Set(['apiKey']), - attributesToExcludeFromAAD: new Set([ - 'scheduledTaskId', - 'muteAll', - 'mutedInstanceIds', - 'updatedBy', - ]), + attributesToExcludeFromAAD: new Set(AlertAttributesExcludedFromAAD), }); }