From 53ed1fa2698bf18d2df07e2c73e50dde9321fcf3 Mon Sep 17 00:00:00 2001 From: Ying Date: Thu, 23 Feb 2023 14:06:16 -0500 Subject: [PATCH 01/14] Requiring source in action executor execute function. Persisting source type in action task params. Writing source to event log --- .../actions/server/actions_client.test.ts | 25 +++++++- .../plugins/actions/server/actions_client.ts | 8 +-- .../actions/server/constants/event_log.ts | 1 - .../actions/server/create_execute_function.ts | 11 +++- x-pack/plugins/actions/server/index.ts | 6 +- .../server/lib/action_execution_source.ts | 22 +++++++ .../server/lib/action_executor.test.ts | 3 + .../actions/server/lib/action_executor.ts | 54 +++++------------ ...ate_action_event_log_record_object.test.ts | 58 +++++++++++++++++++ .../create_action_event_log_record_object.ts | 4 ++ x-pack/plugins/actions/server/lib/index.ts | 2 + .../actions/server/lib/task_runner_factory.ts | 34 +++++------ .../server/saved_objects/mappings.json | 3 + .../plugins/event_log/generated/mappings.json | 4 ++ x-pack/plugins/event_log/generated/schemas.ts | 1 + x-pack/plugins/event_log/scripts/mappings.js | 4 ++ .../services/connectors_email_service.ts | 6 +- .../server/unsecured_actions_simulation.ts | 12 +++- 18 files changed, 187 insertions(+), 71 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index a164058d3b87d..a53fcc04f548c 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -12,7 +12,12 @@ import { ByteSizeValue } from '@kbn/config-schema'; import { ActionTypeRegistry, ActionTypeRegistryOpts } from './action_type_registry'; import { ActionsClient } from './actions_client'; import { ExecutorType, ActionType } from './types'; -import { ActionExecutor, TaskRunnerFactory, ILicenseState } from './lib'; +import { + ActionExecutor, + TaskRunnerFactory, + ILicenseState, + asHttpRequestExecutionSource, +} from './lib'; import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks'; import { actionsConfigMock } from './actions_config.mock'; import { getActionsConfigurationUtilities } from './actions_config'; @@ -2171,6 +2176,7 @@ describe('execute()', () => { params: { name: 'my name', }, + source: asHttpRequestExecutionSource(request), }); expect(authorization.ensureAuthorized).toHaveBeenCalledWith('execute'); }); @@ -2189,6 +2195,7 @@ describe('execute()', () => { params: { name: 'my name', }, + source: asHttpRequestExecutionSource(request), }) ).rejects.toMatchInlineSnapshot(`[Error: Unauthorized to execute all actions]`); @@ -2205,6 +2212,7 @@ describe('execute()', () => { params: { name: 'my name', }, + source: asHttpRequestExecutionSource(request), }); expect(trackLegacyRBACExemption as jest.Mock).toBeCalledWith('execute', mockUsageCounter); @@ -2221,6 +2229,7 @@ describe('execute()', () => { params: { name: 'my name', }, + source: asHttpRequestExecutionSource(request), }) ).resolves.toMatchObject({ status: 'ok', actionId }); @@ -2239,6 +2248,7 @@ describe('execute()', () => { params: { name: 'my name', }, + source: asHttpRequestExecutionSource(request), relatedSavedObjects: [ { id: 'some-id', @@ -2271,6 +2281,7 @@ describe('execute()', () => { params: { name: 'my name', }, + source: asHttpRequestExecutionSource(request), relatedSavedObjects: [ { id: 'some-id', @@ -2313,6 +2324,7 @@ describe('enqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }); expect(authorization.ensureAuthorized).toHaveBeenCalledWith('execute'); }); @@ -2332,6 +2344,7 @@ describe('enqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }) ).rejects.toMatchInlineSnapshot(`[Error: Unauthorized to execute all actions]`); @@ -2349,6 +2362,7 @@ describe('enqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }); expect(trackLegacyRBACExemption as jest.Mock).toBeCalledWith( @@ -2365,6 +2379,7 @@ describe('enqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), + source: asHttpRequestExecutionSource(request), }; await expect(actionsClient.enqueueExecution(opts)).resolves.toMatchInlineSnapshot(`undefined`); @@ -2385,6 +2400,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, { id: uuidv4(), @@ -2392,6 +2408,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '456def', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]); expect(authorization.ensureAuthorized).toHaveBeenCalledWith('execute'); @@ -2413,6 +2430,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, { id: uuidv4(), @@ -2420,6 +2438,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '456def', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]) ).rejects.toMatchInlineSnapshot(`[Error: Unauthorized to execute all actions]`); @@ -2439,6 +2458,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, { id: uuidv4(), @@ -2446,6 +2466,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '456def', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]); @@ -2468,6 +2489,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, { id: uuidv4(), @@ -2475,6 +2497,7 @@ describe('bulkEnqueueExecution()', () => { spaceId: 'default', executionId: '456def', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]; await expect(actionsClient.bulkEnqueueExecution(opts)).resolves.toMatchInlineSnapshot( diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index 867fc4f9c7e47..a073152895bc8 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -667,9 +667,9 @@ export class ActionsClient { params, source, relatedSavedObjects, - }: Omit): Promise< - ActionTypeExecutorResult - > { + }: Omit & { + source: ActionExecutionSource; + }): Promise> { if ( (await getAuthorizationModeBySource(this.unsecuredSavedObjectsClient, source)) === AuthorizationMode.RBAC @@ -682,7 +682,7 @@ export class ActionsClient { return this.actionExecutor.execute({ actionId, params, - source, + sourceType: source.type, request: this.request, relatedSavedObjects, actionExecutionId: uuidv4(), diff --git a/x-pack/plugins/actions/server/constants/event_log.ts b/x-pack/plugins/actions/server/constants/event_log.ts index 9dba72462f317..e3ae4e8c5b855 100644 --- a/x-pack/plugins/actions/server/constants/event_log.ts +++ b/x-pack/plugins/actions/server/constants/event_log.ts @@ -9,6 +9,5 @@ export const EVENT_LOG_PROVIDER = 'actions'; export const EVENT_LOG_ACTIONS = { execute: 'execute', executeStart: 'execute-start', - executeViaHttp: 'execute-via-http', executeTimeout: 'execute-timeout', }; diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index c050cf34c9fca..b512ec7a58a33 100644 --- a/x-pack/plugins/actions/server/create_execute_function.ts +++ b/x-pack/plugins/actions/server/create_execute_function.ts @@ -15,7 +15,11 @@ import { } from './types'; import { ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from './constants/saved_objects'; import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; -import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; +import { + ActionExecutionSource, + extractSavedObjectReferences, + isSavedObjectExecutionSource, +} from './lib'; interface CreateExecuteFunctionOptions { taskManager: TaskManagerStartContract; @@ -25,11 +29,12 @@ interface CreateExecuteFunctionOptions { } export interface ExecuteOptions - extends Pick { + extends Pick { id: string; spaceId: string; apiKey: string | null; executionId: string; + source: ActionExecutionSource; } interface ActionTaskParams @@ -273,7 +278,7 @@ function validateCanActionBeUsed(action: PreConfiguredAction | RawAction) { } } -function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorOptions['source']) { +function executionSourceAsSavedObjectReferences(executionSource: ActionExecutionSource) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/index.ts b/x-pack/plugins/actions/server/index.ts index 2713ee17463e4..15f3c2fa7e20a 100644 --- a/x-pack/plugins/actions/server/index.ts +++ b/x-pack/plugins/actions/server/index.ts @@ -29,7 +29,11 @@ export type { export type { PluginSetupContract, PluginStartContract } from './plugin'; -export { asSavedObjectExecutionSource, asHttpRequestExecutionSource } from './lib'; +export { + asSavedObjectExecutionSource, + asHttpRequestExecutionSource, + asNotificationExecutionSource, +} from './lib'; export { ACTION_SAVED_OBJECT_TYPE } from './constants/saved_objects'; export const plugin = (initContext: PluginInitializerContext) => new ActionsPlugin(initContext); diff --git a/x-pack/plugins/actions/server/lib/action_execution_source.ts b/x-pack/plugins/actions/server/lib/action_execution_source.ts index f5e1570dfa4c5..1589b889bb84b 100644 --- a/x-pack/plugins/actions/server/lib/action_execution_source.ts +++ b/x-pack/plugins/actions/server/lib/action_execution_source.ts @@ -10,6 +10,7 @@ import { KibanaRequest, SavedObjectReference } from '@kbn/core/server'; export enum ActionExecutionSourceType { SAVED_OBJECT = 'SAVED_OBJECT', HTTP_REQUEST = 'HTTP_REQUEST', + NOTIFICATION = 'NOTIFICATION', } export interface ActionExecutionSource { @@ -19,6 +20,12 @@ export interface ActionExecutionSource { export type HttpRequestExecutionSource = ActionExecutionSource; export type SavedObjectExecutionSource = ActionExecutionSource>; +export interface NotificationSource { + requesterId: string; + connectorId: string; +} +export type NotificationExecutionSource = ActionExecutionSource; + export function asHttpRequestExecutionSource(source: KibanaRequest): HttpRequestExecutionSource { return { type: ActionExecutionSourceType.HTTP_REQUEST, @@ -35,6 +42,15 @@ export function asSavedObjectExecutionSource( }; } +export function asNotificationExecutionSource( + source: NotificationSource +): NotificationExecutionSource { + return { + type: ActionExecutionSourceType.NOTIFICATION, + source, + }; +} + export function isHttpRequestExecutionSource( executionSource?: ActionExecutionSource ): executionSource is HttpRequestExecutionSource { @@ -46,3 +62,9 @@ export function isSavedObjectExecutionSource( ): executionSource is SavedObjectExecutionSource { return executionSource?.type === ActionExecutionSourceType.SAVED_OBJECT; } + +export function isNotificationExecutionSource( + executionSource?: ActionExecutionSource +): executionSource is NotificationExecutionSource { + return executionSource?.type === ActionExecutionSourceType.NOTIFICATION; +} diff --git a/x-pack/plugins/actions/server/lib/action_executor.test.ts b/x-pack/plugins/actions/server/lib/action_executor.test.ts index ecb99eaf9d852..128b726849c9e 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -16,6 +16,7 @@ import { spacesServiceMock } from '@kbn/spaces-plugin/server/spaces_service/spac import { ActionType } from '../types'; import { actionsMock, actionsClientMock } from '../mocks'; import { pick } from 'lodash'; +import { ActionExecutionSourceType } from './action_execution_source'; const actionExecutor = new ActionExecutor({ isESOCanEncrypt: true }); const services = actionsMock.createServices(); @@ -33,6 +34,7 @@ const executeParams = { executionId: '123abc', request: {} as KibanaRequest, actionExecutionId: '2', + sourceType: ActionExecutionSourceType.HTTP_REQUEST, }; const spacesMock = spacesServiceMock.createStartContract(); @@ -863,6 +865,7 @@ test('writes to event log for execute timeout', async () => { relatedSavedObjects: [], request: {} as KibanaRequest, actionExecutionId: '2', + sourceType: ActionExecutionSourceType.HTTP_REQUEST, }); expect(eventLogger.logEvent).toHaveBeenCalledTimes(1); expect(eventLogger.logEvent).toHaveBeenNthCalledWith(1, { diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index 36212bc9ae042..42892db002776 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -30,7 +30,7 @@ import { } from '../types'; import { EVENT_LOG_ACTIONS } from '../constants/event_log'; import { ActionsClient } from '../actions_client'; -import { ActionExecutionSource } from './action_execution_source'; +import { ActionExecutionSource, ActionExecutionSourceType } from './action_execution_source'; import { RelatedSavedObjects } from './related_saved_objects'; import { createActionEventLogRecordObject } from './create_action_event_log_record_object'; import { ActionExecutionError, ActionExecutionErrorReason } from './errors/action_execution_error'; @@ -57,13 +57,13 @@ export interface TaskInfo { attempts: number; } -export interface ExecuteOptions { +export interface ExecuteOptions { actionId: string; actionExecutionId: string; isEphemeral?: boolean; request: KibanaRequest; params: Record; - source?: ActionExecutionSource; + sourceType: ActionExecutionSourceType; taskInfo?: TaskInfo; executionId?: string; consumer?: string; @@ -95,7 +95,7 @@ export class ActionExecutor { actionId, params, request, - source, + sourceType, isEphemeral, taskInfo, executionId, @@ -123,7 +123,6 @@ export class ActionExecutor { actionTypeRegistry, eventLogger, preconfiguredActions, - getActionsClientWithRequest, } = this.actionExecutorContext!; const services = getServices(request); @@ -131,14 +130,11 @@ export class ActionExecutor { const namespace = spaceId && spaceId !== 'default' ? { namespace: spaceId } : {}; const actionInfo = await getActionInfoInternal( - getActionsClientWithRequest, - request, this.isESOCanEncrypt, encryptedSavedObjectsClient, preconfiguredActions, actionId, - namespace.namespace, - source + namespace.namespace ); const { actionTypeId, name, config, secrets } = actionInfo; @@ -194,6 +190,7 @@ export class ActionExecutor { name, actionExecutionId, isPreconfigured: this.actionInfo.isPreconfigured, + source: sourceType, }); eventLogger.startTiming(event); @@ -294,11 +291,11 @@ export class ActionExecutor { ); } - public async logCancellation({ + public async logCancellation({ actionId, request, relatedSavedObjects, - source, + sourceType, executionId, taskInfo, consumer, @@ -310,29 +307,21 @@ export class ActionExecutor { taskInfo?: TaskInfo; executionId?: string; relatedSavedObjects: RelatedSavedObjects; - source?: ActionExecutionSource; + sourceType: ActionExecutionSourceType; consumer?: string; }) { - const { - spaces, - encryptedSavedObjectsClient, - preconfiguredActions, - eventLogger, - getActionsClientWithRequest, - } = this.actionExecutorContext!; + const { spaces, encryptedSavedObjectsClient, preconfiguredActions, eventLogger } = + this.actionExecutorContext!; const spaceId = spaces && spaces.getSpaceId(request); const namespace = spaceId && spaceId !== 'default' ? { namespace: spaceId } : {}; if (!this.actionInfo || this.actionInfo.actionId !== actionId) { this.actionInfo = await getActionInfoInternal( - getActionsClientWithRequest, - request, this.isESOCanEncrypt, encryptedSavedObjectsClient, preconfiguredActions, actionId, - namespace.namespace, - source + namespace.namespace ); } const task = taskInfo @@ -366,6 +355,7 @@ export class ActionExecutor { relatedSavedObjects, actionExecutionId, isPreconfigured: this.actionInfo.isPreconfigured, + source: sourceType, }); eventLogger.logEvent(event); @@ -381,18 +371,12 @@ interface ActionInfo { isPreconfigured?: boolean; } -async function getActionInfoInternal( - getActionsClientWithRequest: ( - request: KibanaRequest, - authorizationContext?: ActionExecutionSource - ) => Promise>, - request: KibanaRequest, +async function getActionInfoInternal( isESOCanEncrypt: boolean, encryptedSavedObjectsClient: EncryptedSavedObjectsClient, preconfiguredActions: PreConfiguredAction[], actionId: string, - namespace: string | undefined, - source?: ActionExecutionSource + namespace: string | undefined ): Promise { // check to see if it's a pre-configured action first const pcAction = preconfiguredActions.find( @@ -415,14 +399,8 @@ async function getActionInfoInternal( ); } - const actionsClient = await getActionsClientWithRequest(request, source); - - // if not pre-configured action, should be a saved object - // ensure user can read the action before processing - const { actionTypeId, config, name } = await actionsClient.get({ id: actionId }); - const { - attributes: { secrets }, + attributes: { secrets, actionTypeId, config, name }, } = await encryptedSavedObjectsClient.getDecryptedAsInternalUser('action', actionId, { namespace: namespace === 'default' ? undefined : namespace, }); diff --git a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts index 58a21c911cf34..c713b60654775 100644 --- a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts +++ b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { ActionExecutionSourceType } from './action_execution_source'; import { createActionEventLogRecordObject } from './create_action_event_log_record_object'; describe('createActionEventLogRecordObject', () => { @@ -297,6 +298,63 @@ describe('createActionEventLogRecordObject', () => { }); }); + test('created action event "execute" with source', async () => { + expect( + createActionEventLogRecordObject({ + actionId: '1', + name: 'test name', + action: 'execute', + message: 'action execution start', + namespace: 'default', + executionId: '123abc', + consumer: 'test-consumer', + savedObjects: [ + { + id: '2', + type: 'action', + typeId: '.email', + relation: 'primary', + }, + ], + actionExecutionId: '123abc', + source: ActionExecutionSourceType.HTTP_REQUEST, + }) + ).toStrictEqual({ + event: { + action: 'execute', + kind: 'action', + }, + kibana: { + alert: { + rule: { + consumer: 'test-consumer', + execution: { + uuid: '123abc', + }, + }, + }, + saved_objects: [ + { + id: '2', + namespace: 'default', + rel: 'primary', + type: 'action', + type_id: '.email', + }, + ], + action: { + name: 'test name', + id: '1', + execution: { + source: 'HTTP_REQUEST', + uuid: '123abc', + }, + }, + }, + message: 'action execution start', + }); + }); + test('created action event "execute" for preconfigured connector with space_agnostic true', async () => { expect( createActionEventLogRecordObject({ diff --git a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts index 95a6101d684f5..61ec6f1518d05 100644 --- a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts +++ b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts @@ -9,6 +9,7 @@ import { set } from '@kbn/safer-lodash-set'; import { isEmpty } from 'lodash'; import { IEvent, SAVED_OBJECT_REL_PRIMARY } from '@kbn/event-log-plugin/server'; import { RelatedSavedObjects } from './related_saved_objects'; +import { ActionExecutionSourceType } from './action_execution_source'; export type Event = Exclude; @@ -35,6 +36,7 @@ interface CreateActionEventLogRecordParams { }>; relatedSavedObjects?: RelatedSavedObjects; isPreconfigured?: boolean; + source?: ActionExecutionSourceType; } export function createActionEventLogRecordObject(params: CreateActionEventLogRecordParams): Event { @@ -51,6 +53,7 @@ export function createActionEventLogRecordObject(params: CreateActionEventLogRec actionExecutionId, isPreconfigured, actionId, + source, } = params; const kibanaAlertRule = { @@ -88,6 +91,7 @@ export function createActionEventLogRecordObject(params: CreateActionEventLogRec id: actionId, execution: { uuid: actionExecutionId, + ...(source ? { source } : {}), }, }, }, diff --git a/x-pack/plugins/actions/server/lib/index.ts b/x-pack/plugins/actions/server/lib/index.ts index 9ac123babecef..bbf86c6644e48 100644 --- a/x-pack/plugins/actions/server/lib/index.ts +++ b/x-pack/plugins/actions/server/lib/index.ts @@ -32,6 +32,8 @@ export { isSavedObjectExecutionSource, asHttpRequestExecutionSource, isHttpRequestExecutionSource, + asNotificationExecutionSource, + isNotificationExecutionSource, } from './action_execution_source'; export { validateEmptyStrings } from './validate_empty_strings'; export { parseDate } from './parse_date'; diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.ts index d5b82e8fc6f6e..51d491b959e11 100644 --- a/x-pack/plugins/actions/server/lib/task_runner_factory.ts +++ b/x-pack/plugins/actions/server/lib/task_runner_factory.ts @@ -6,16 +6,12 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { pick } from 'lodash'; -import { pipe } from 'fp-ts/lib/pipeable'; -import { map, fromNullable, getOrElse } from 'fp-ts/lib/Option'; import { addSpaceIdToPath } from '@kbn/spaces-plugin/server'; import { Logger, SavedObjectsClientContract, KibanaRequest, CoreKibanaRequest, - SavedObjectReference, IBasePath, SavedObject, Headers, @@ -34,7 +30,7 @@ import { isPersistedActionTask, } from '../types'; import { ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from '../constants/saved_objects'; -import { asSavedObjectExecutionSource } from './action_execution_source'; +import { ActionExecutionSourceType } from './action_execution_source'; import { RelatedSavedObjects, validatedRelatedSavedObjects } from './related_saved_objects'; import { injectSavedObjectReferences } from './action_task_params_utils'; import { InMemoryMetrics, IN_MEMORY_METRICS } from '../monitoring'; @@ -93,8 +89,15 @@ export class TaskRunnerFactory { const { spaceId } = actionTaskExecutorParams; const { - attributes: { actionId, params, apiKey, executionId, consumer, relatedSavedObjects }, - references, + attributes: { + actionId, + params, + apiKey, + executionId, + consumer, + source, + relatedSavedObjects, + }, } = await getActionTaskParams( actionTaskExecutorParams, encryptedSavedObjectsClient, @@ -119,7 +122,7 @@ export class TaskRunnerFactory { actionId: actionId as string, isEphemeral: !isPersistedActionTask(actionTaskExecutorParams), request, - ...getSourceFromReferences(references), + sourceType: source as ActionExecutionSourceType, taskInfo, executionId, consumer, @@ -192,8 +195,7 @@ export class TaskRunnerFactory { const { spaceId } = actionTaskExecutorParams; const { - attributes: { actionId, apiKey, executionId, consumer, relatedSavedObjects }, - references, + attributes: { actionId, apiKey, executionId, consumer, source, relatedSavedObjects }, } = await getActionTaskParams( actionTaskExecutorParams, encryptedSavedObjectsClient, @@ -210,7 +212,7 @@ export class TaskRunnerFactory { consumer, executionId, relatedSavedObjects: (relatedSavedObjects || []) as RelatedSavedObjects, - ...getSourceFromReferences(references), + sourceType: source as ActionExecutionSourceType, actionExecutionId, }); @@ -278,13 +280,3 @@ async function getActionTaskParams( return { attributes: executorParams.taskParams, references: executorParams.references ?? [] }; } } - -function getSourceFromReferences(references: SavedObjectReference[]) { - return pipe( - fromNullable(references.find((ref) => ref.name === 'source')), - map((source) => ({ - source: asSavedObjectExecutionSource(pick(source, 'id', 'type')), - })), - getOrElse(() => ({})) - ); -} diff --git a/x-pack/plugins/actions/server/saved_objects/mappings.json b/x-pack/plugins/actions/server/saved_objects/mappings.json index 80646579f86db..2e6a0a7dce6e2 100644 --- a/x-pack/plugins/actions/server/saved_objects/mappings.json +++ b/x-pack/plugins/actions/server/saved_objects/mappings.json @@ -45,6 +45,9 @@ "relatedSavedObjects": { "enabled": false, "type": "object" + }, + "source": { + "type": "keyword" } } }, diff --git a/x-pack/plugins/event_log/generated/mappings.json b/x-pack/plugins/event_log/generated/mappings.json index a5212917393f8..8c61d6385d80b 100644 --- a/x-pack/plugins/event_log/generated/mappings.json +++ b/x-pack/plugins/event_log/generated/mappings.json @@ -445,6 +445,10 @@ }, "execution": { "properties": { + "source": { + "ignore_above": 1024, + "type": "keyword" + }, "uuid": { "ignore_above": 1024, "type": "keyword" diff --git a/x-pack/plugins/event_log/generated/schemas.ts b/x-pack/plugins/event_log/generated/schemas.ts index 93ee9f2977a36..27c3813dae658 100644 --- a/x-pack/plugins/event_log/generated/schemas.ts +++ b/x-pack/plugins/event_log/generated/schemas.ts @@ -201,6 +201,7 @@ export const EventSchema = schema.maybe( id: ecsString(), execution: schema.maybe( schema.object({ + source: ecsString(), uuid: ecsString(), }) ), diff --git a/x-pack/plugins/event_log/scripts/mappings.js b/x-pack/plugins/event_log/scripts/mappings.js index 36838a4208a87..ecc6a5fc30792 100644 --- a/x-pack/plugins/event_log/scripts/mappings.js +++ b/x-pack/plugins/event_log/scripts/mappings.js @@ -227,6 +227,10 @@ exports.EcsCustomPropertyMappings = { }, execution: { properties: { + source: { + ignore_above: 1024, + type: 'keyword', + }, uuid: { ignore_above: 1024, type: 'keyword', diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.ts index 55586dd05b078..d907872e3a16f 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -5,7 +5,7 @@ * 2.0. */ -import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; +import { asNotificationExecutionSource, IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { @@ -23,6 +23,10 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, + source: asNotificationExecutionSource({ + requesterId: this.requesterId, + connectorId: this.connectorId, + }), relatedSavedObjects: params.context?.relatedObjects, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); diff --git a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts index 5900f698045a0..dd42a49905b47 100644 --- a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts +++ b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { asNotificationExecutionSource } from '@kbn/actions-plugin/server'; import { schema } from '@kbn/config-schema'; import { CoreSetup, @@ -39,7 +40,16 @@ export function initPlugin(router: IRouter, coreSetup: CoreSetup Date: Thu, 23 Feb 2023 14:57:08 -0500 Subject: [PATCH 02/14] Fixing unit tests --- .../actions/server/actions_client.test.ts | 3 ++ .../server/create_execute_function.test.ts | 18 ++++++++++++ .../actions/server/create_execute_function.ts | 2 ++ .../server/lib/action_executor.test.ts | 9 ++++++ .../server/lib/task_runner_factory.test.ts | 28 +++++++++++++++++++ x-pack/plugins/actions/server/types.ts | 1 + .../services/connectors_email_service.test.ts | 28 +++++++++++++++++++ .../services/connectors_email_service.ts | 5 +++- 8 files changed, 93 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index a53fcc04f548c..3bf49ab5281d5 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -2240,6 +2240,7 @@ describe('execute()', () => { name: 'my name', }, actionExecutionId, + sourceType: 'HTTP_REQUEST', }); await expect( @@ -2273,6 +2274,7 @@ describe('execute()', () => { }, ], actionExecutionId, + sourceType: 'HTTP_REQUEST', }); await expect( @@ -2299,6 +2301,7 @@ describe('execute()', () => { params: { name: 'my name', }, + sourceType: 'HTTP_REQUEST', relatedSavedObjects: [ { id: 'some-id', diff --git a/x-pack/plugins/actions/server/create_execute_function.test.ts b/x-pack/plugins/actions/server/create_execute_function.test.ts index 8147e14d2a943..e4787101b1cde 100644 --- a/x-pack/plugins/actions/server/create_execute_function.test.ts +++ b/x-pack/plugins/actions/server/create_execute_function.test.ts @@ -79,6 +79,7 @@ describe('execute()', () => { actionId: '123', params: { baz: false }, executionId: '123abc', + source: 'HTTP_REQUEST', apiKey: Buffer.from('123:abc').toString('base64'), }, { @@ -151,6 +152,7 @@ describe('execute()', () => { params: { baz: false }, executionId: '123abc', consumer: 'test-consumer', + source: 'HTTP_REQUEST', apiKey: Buffer.from('123:abc').toString('base64'), }, { @@ -213,6 +215,7 @@ describe('execute()', () => { params: { baz: false }, apiKey: Buffer.from('123:abc').toString('base64'), executionId: '123abc', + source: 'HTTP_REQUEST', relatedSavedObjects: [ { id: 'related_some-type_0', @@ -303,6 +306,7 @@ describe('execute()', () => { actionId: '123', params: { baz: false }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, { @@ -390,6 +394,7 @@ describe('execute()', () => { params: { baz: false }, apiKey: Buffer.from('123:abc').toString('base64'), executionId: '123abc', + source: 'SAVED_OBJECT', relatedSavedObjects: [ { id: 'related_some-type_0', @@ -430,6 +435,7 @@ describe('execute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }) ).rejects.toThrowErrorMatchingInlineSnapshot( `"Unable to execute action because the Encrypted Saved Objects plugin is missing encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command."` @@ -460,6 +466,7 @@ describe('execute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }) ).rejects.toThrowErrorMatchingInlineSnapshot( `"Unable to execute action because no secrets are defined for the \\"mock-action\\" connector."` @@ -493,6 +500,7 @@ describe('execute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); }); @@ -537,6 +545,7 @@ describe('execute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }); expect(mockedActionTypeRegistry.ensureActionTypeEnabled).not.toHaveBeenCalled(); @@ -613,6 +622,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: false }, executionId: '123abc', + source: 'HTTP_REQUEST', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -703,6 +713,7 @@ describe('bulkExecute()', () => { params: { baz: false }, executionId: '123abc', consumer: 'test-consumer', + source: 'HTTP_REQUEST', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -778,6 +789,7 @@ describe('bulkExecute()', () => { params: { baz: false }, apiKey: Buffer.from('123:abc').toString('base64'), executionId: '123abc', + source: 'HTTP_REQUEST', relatedSavedObjects: [ { id: 'related_some-type_0', @@ -885,6 +897,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: false }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -989,6 +1002,7 @@ describe('bulkExecute()', () => { params: { baz: false }, apiKey: Buffer.from('123:abc').toString('base64'), executionId: '123abc', + source: 'SAVED_OBJECT', relatedSavedObjects: [ { id: 'related_some-type_0', @@ -1031,6 +1045,7 @@ describe('bulkExecute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -1067,6 +1082,7 @@ describe('bulkExecute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -1106,6 +1122,7 @@ describe('bulkExecute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); @@ -1162,6 +1179,7 @@ describe('bulkExecute()', () => { spaceId: 'default', executionId: '123abc', apiKey: null, + source: asHttpRequestExecutionSource(request), }, ]); diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index b512ec7a58a33..fe4f229560682 100644 --- a/x-pack/plugins/actions/server/create_execute_function.ts +++ b/x-pack/plugins/actions/server/create_execute_function.ts @@ -120,6 +120,7 @@ export function createExecutionEnqueuerFunction({ apiKey, executionId, consumer, + source: source.type, relatedSavedObjects: relatedSavedObjectWithRefs, }, { @@ -206,6 +207,7 @@ export function createBulkExecutionEnqueuerFunction({ apiKey: actionToExecute.apiKey, executionId: actionToExecute.executionId, consumer: actionToExecute.consumer, + source: actionToExecute.source.type, relatedSavedObjects: relatedSavedObjectWithRefs, }, references: taskReferences, diff --git a/x-pack/plugins/actions/server/lib/action_executor.test.ts b/x-pack/plugins/actions/server/lib/action_executor.test.ts index d9a76997533ed..0afef753244ea 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -132,6 +132,7 @@ test('successfully executes', async () => { "kibana": Object { "action": Object { "execution": Object { + "source": "HTTP_REQUEST", "uuid": "2", }, "id": "1", @@ -170,6 +171,7 @@ test('successfully executes', async () => { "kibana": Object { "action": Object { "execution": Object { + "source": "HTTP_REQUEST", "uuid": "2", }, "id": "1", @@ -246,6 +248,7 @@ test('successfully executes with preconfigured connector', async () => { "kibana": Object { "action": Object { "execution": Object { + "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -285,6 +288,7 @@ test('successfully executes with preconfigured connector', async () => { "kibana": Object { "action": Object { "execution": Object { + "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -664,6 +668,7 @@ test('should not throw error if action is preconfigured and isESOCanEncrypt is f "kibana": Object { "action": Object { "execution": Object { + "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -703,6 +708,7 @@ test('should not throw error if action is preconfigured and isESOCanEncrypt is f "kibana": Object { "action": Object { "execution": Object { + "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -813,6 +819,7 @@ test('writes to event log for execute timeout', async () => { kibana: { action: { execution: { + source: 'HTTP_REQUEST', uuid: '2', }, name: undefined, @@ -858,6 +865,7 @@ test('writes to event log for execute and execute start', async () => { kibana: { action: { execution: { + source: 'HTTP_REQUEST', uuid: '2', }, name: 'action-1', @@ -911,6 +919,7 @@ test('writes to event log for execute and execute start when consumer and relate kibana: { action: { execution: { + source: 'HTTP_REQUEST', uuid: '2', }, name: 'action-1', diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts index 49aadcbefe933..683cb4981eebd 100644 --- a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts +++ b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts @@ -29,6 +29,7 @@ const executeParamsFields = [ 'executionId', 'request.headers', 'taskInfo', + 'sourceType', ]; const spaceIdToNamespace = jest.fn(); const actionTypeRegistry = actionTypeRegistryMock.create(); @@ -131,6 +132,7 @@ test('executes the task by calling the executor with proper parameters, using gi actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -159,6 +161,7 @@ test('executes the task by calling the executor with proper parameters, using gi authorization: 'ApiKey MTIzOmFiYw==', }, }, + sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -185,6 +188,7 @@ test('executes the task by calling the executor with proper parameters, using st actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -213,6 +217,7 @@ test('executes the task by calling the executor with proper parameters, using st params: { baz: true }, executionId: '123abc', relatedSavedObjects: [], + sourceType: 'SAVED_OBJECT', request: { headers: { // base64 encoded "123:abc" @@ -246,6 +251,7 @@ test('executes the task by calling the executor with proper parameters when cons consumer: 'test-consumer', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -269,6 +275,7 @@ test('executes the task by calling the executor with proper parameters when cons params: { baz: true }, relatedSavedObjects: [], executionId: '123abc', + sourceType: 'SAVED_OBJECT', request: { headers: { // base64 encoded "123:abc" @@ -301,6 +308,7 @@ test('cleans up action_task_params object', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -327,6 +335,7 @@ test('task runner should implement CancellableTask cancel method with logging wa actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -365,6 +374,7 @@ test('runs successfully when cleanup fails and logs the error', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -399,6 +409,7 @@ test('throws an error with suggested retry logic when return status is error', a actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -441,6 +452,7 @@ test('uses API key when provided', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -461,6 +473,7 @@ test('uses API key when provided', async () => { params: { baz: true }, executionId: '123abc', relatedSavedObjects: [], + sourceType: 'SAVED_OBJECT', request: { headers: { // base64 encoded "123:abc" @@ -494,6 +507,7 @@ test('uses relatedSavedObjects merged with references when provided', async () = params: { baz: true }, executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), + source: 'SAVED_OBJECT', relatedSavedObjects: [{ id: 'related_some-type_0', type: 'some-type' }], }, references: [ @@ -518,6 +532,7 @@ test('uses relatedSavedObjects merged with references when provided', async () = isEphemeral: false, params: { baz: true }, executionId: '123abc', + sourceType: 'SAVED_OBJECT', relatedSavedObjects: [ { id: 'some-id', @@ -552,6 +567,7 @@ test('uses relatedSavedObjects as is when references are empty', async () => { params: { baz: true }, executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), + source: 'SAVED_OBJECT', relatedSavedObjects: [{ id: 'abc', type: 'some-type', namespace: 'yo' }], }, references: [ @@ -571,6 +587,7 @@ test('uses relatedSavedObjects as is when references are empty', async () => { isEphemeral: false, params: { baz: true }, executionId: '123abc', + sourceType: 'SAVED_OBJECT', relatedSavedObjects: [ { id: 'abc', @@ -606,6 +623,7 @@ test('sanitizes invalid relatedSavedObjects when provided', async () => { params: { baz: true }, executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), + source: 'SAVED_OBJECT', relatedSavedObjects: [{ Xid: 'related_some-type_0', type: 'some-type' }], }, references: [ @@ -637,6 +655,7 @@ test('sanitizes invalid relatedSavedObjects when provided', async () => { }, executionId: '123abc', relatedSavedObjects: [], + sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -657,6 +676,7 @@ test(`doesn't use API key when not provided`, async () => { attributes: { actionId: '2', params: { baz: true }, + source: 'SAVED_OBJECT', executionId: '123abc', }, references: [ @@ -680,6 +700,7 @@ test(`doesn't use API key when not provided`, async () => { request: { headers: {}, }, + sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -710,6 +731,7 @@ test(`throws an error when license doesn't support the action type`, async () => actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -749,6 +771,7 @@ test(`treats errors as errors if the task is retryable`, async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -797,6 +820,7 @@ test(`treats errors as successes if the task is not retryable`, async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -842,6 +866,7 @@ test('treats errors as errors if the error is thrown instead of returned', async actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -883,6 +908,7 @@ test('increments monitoring metrics after execution', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -915,6 +941,7 @@ test('increments monitoring metrics after a failed execution', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -947,6 +974,7 @@ test('increments monitoring metrics after a timeout', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', + source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], diff --git a/x-pack/plugins/actions/server/types.ts b/x-pack/plugins/actions/server/types.ts index 147a71da0c960..72e7eeddce1c1 100644 --- a/x-pack/plugins/actions/server/types.ts +++ b/x-pack/plugins/actions/server/types.ts @@ -155,6 +155,7 @@ export interface ActionTaskParams extends SavedObjectAttributes { apiKey?: string; executionId?: string; consumer?: string; + source?: string; } interface PersistedActionTaskExecutorParams { diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts index f07b3a3ab34ae..c9fd5d8004411 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts @@ -34,6 +34,13 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, + source: { + source: { + connectorId: CONNECTOR_ID, + requesterId: REQUESTER_ID, + }, + type: 'NOTIFICATION', + }, }, ]); }); @@ -67,6 +74,13 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, + source: { + source: { + connectorId: CONNECTOR_ID, + requesterId: REQUESTER_ID, + }, + type: 'NOTIFICATION', + }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -82,6 +96,13 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, + source: { + source: { + connectorId: CONNECTOR_ID, + requesterId: REQUESTER_ID, + }, + type: 'NOTIFICATION', + }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -97,6 +118,13 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, + source: { + source: { + connectorId: CONNECTOR_ID, + requesterId: REQUESTER_ID, + }, + type: 'NOTIFICATION', + }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.ts index d907872e3a16f..630ad8d22fa9c 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -5,7 +5,10 @@ * 2.0. */ -import { asNotificationExecutionSource, IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; +import { + asNotificationExecutionSource, + type IUnsecuredActionsClient, +} from '@kbn/actions-plugin/server'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { From 10f09a03d633069a1017acda3e22cd6603211954 Mon Sep 17 00:00:00 2001 From: Ying Date: Thu, 23 Feb 2023 16:03:32 -0500 Subject: [PATCH 03/14] Fixing types and tests --- .../group2/check_registered_types.test.ts | 2 +- .../create_unsecured_execute_function.test.ts | 21 ++++++++++++++++++- .../create_unsecured_execute_function.ts | 12 ++++++++--- .../unsecured_actions_client.test.ts | 5 +++++ .../plugins/cases/server/client/cases/push.ts | 4 +++- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 7d9e4bae782e2..c15920c6f0cf6 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -56,7 +56,7 @@ describe('checking migration metadata changes on all registered SO types', () => expect(hashMap).toMatchInlineSnapshot(` Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", - "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", + "action_task_params": "c2cb5c9060322e3e51c9e65debd8df48c0e54b8f", "alert": "2568bf6d8ba0876441c61c9e58e08016c1dc1617", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.test.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.test.ts index a61d6193a9ab5..cd50be7558fea 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.test.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.test.ts @@ -10,7 +10,10 @@ import { savedObjectsRepositoryMock } from '@kbn/core/server/mocks'; import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks'; import { createBulkUnsecuredExecutionEnqueuerFunction } from './create_unsecured_execute_function'; import { actionTypeRegistryMock } from './action_type_registry.mock'; -import { asSavedObjectExecutionSource } from './lib/action_execution_source'; +import { + asNotificationExecutionSource, + asSavedObjectExecutionSource, +} from './lib/action_execution_source'; const mockTaskManager = taskManagerMock.createStart(); const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); @@ -59,10 +62,12 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: '123', params: { baz: true }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]); expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); @@ -102,6 +107,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: false }, apiKey: null, + source: 'NOTIFICATION', }, references: [], }, @@ -111,6 +117,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: true }, apiKey: null, + source: 'NOTIFICATION', }, references: [], }, @@ -171,6 +178,7 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: true }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]); expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); @@ -210,6 +218,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: false }, apiKey: null, + source: 'SAVED_OBJECT', }, references: [ { @@ -225,6 +234,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: true }, apiKey: null, + source: 'NOTIFICATION', }, references: [], }, @@ -291,6 +301,7 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: true }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), relatedSavedObjects: [ { id: 'some-id', @@ -337,6 +348,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: false }, apiKey: null, + source: 'SAVED_OBJECT', }, references: [ { @@ -352,6 +364,7 @@ describe('bulkExecute()', () => { actionId: '123', params: { baz: true }, apiKey: null, + source: 'NOTIFICATION', relatedSavedObjects: [ { id: 'related_some-type_0', @@ -392,10 +405,12 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'not-preconfigured', params: { baz: true }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -429,10 +444,12 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: '123', params: { baz: true }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); @@ -468,10 +485,12 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: '456', params: { baz: true }, + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot( diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts index 4670601ecff83..c5870420178ff 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -13,7 +13,11 @@ import { } from './types'; import { ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from './constants/saved_objects'; import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; -import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; +import { + extractSavedObjectReferences, + isSavedObjectExecutionSource, + ActionExecutionSource, +} from './lib'; // This allowlist should only contain connector types that don't require API keys for // execution. @@ -25,8 +29,9 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { } export interface ExecuteOptions - extends Pick { + extends Pick { id: string; + source: ActionExecutionSource; } interface ActionTaskParams @@ -110,6 +115,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ actionId: actionToExecute.id, params: actionToExecute.params, apiKey: null, + source: actionToExecute.source.type, relatedSavedObjects: relatedSavedObjectWithRefs, }, references: taskReferences, @@ -134,7 +140,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ }; } -function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorOptions['source']) { +function executionSourceAsSavedObjectReferences(executionSource: ActionExecutionSource) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts index c863e943b8dc0..b07c80f8da3a0 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts @@ -7,6 +7,7 @@ import { UnsecuredActionsClient } from './unsecured_actions_client'; import { savedObjectsRepositoryMock } from '@kbn/core/server/mocks'; +import { asNotificationExecutionSource } from '../lib'; const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); const executionEnqueuer = jest.fn(); @@ -28,11 +29,13 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( @@ -48,11 +51,13 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', + source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( diff --git a/x-pack/plugins/cases/server/client/cases/push.ts b/x-pack/plugins/cases/server/client/cases/push.ts index 177f6f2ec6ce6..4f3ab74557278 100644 --- a/x-pack/plugins/cases/server/client/cases/push.ts +++ b/x-pack/plugins/cases/server/client/cases/push.ts @@ -11,6 +11,7 @@ import type { SavedObjectsFindResponse } from '@kbn/core/server'; import type { UserProfile } from '@kbn/security-plugin/common'; import type { SecurityPluginStart } from '@kbn/security-plugin/server'; +import { asSavedObjectExecutionSource } from '@kbn/actions-plugin/server'; import type { ActionConnector, CaseResponse, @@ -26,7 +27,7 @@ import { OWNER_FIELD, CommentType, } from '../../../common/api'; -import { CASE_COMMENT_SAVED_OBJECT } from '../../../common/constants'; +import { CASE_COMMENT_SAVED_OBJECT, CASE_SAVED_OBJECT } from '../../../common/constants'; import { createIncident, getDurationInSeconds, getUserProfiles } from './utils'; import { createCaseError } from '../../common/error'; @@ -164,6 +165,7 @@ export const push = async ( subAction: 'pushToService', subActionParams: externalServiceIncident, }, + source: asSavedObjectExecutionSource({ id: caseId, type: CASE_SAVED_OBJECT }), }); if (pushRes.status === 'error') { From 9c43d47c45ad319e9896547d982b0893e848bbde Mon Sep 17 00:00:00 2001 From: Ying Date: Thu, 23 Feb 2023 16:30:16 -0500 Subject: [PATCH 04/14] Making sourceType optional --- .../server/lib/action_executor.test.ts | 147 ++++++++++++++++-- .../actions/server/lib/action_executor.ts | 8 +- .../server/lib/task_runner_factory.test.ts | 83 ++++++---- .../actions/server/lib/task_runner_factory.ts | 4 +- 4 files changed, 200 insertions(+), 42 deletions(-) diff --git a/x-pack/plugins/actions/server/lib/action_executor.test.ts b/x-pack/plugins/actions/server/lib/action_executor.test.ts index 0afef753244ea..35483d75d036a 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -14,8 +14,8 @@ import { loggingSystemMock } from '@kbn/core/server/mocks'; import { eventLoggerMock } from '@kbn/event-log-plugin/server/mocks'; import { spacesServiceMock } from '@kbn/spaces-plugin/server/spaces_service/spaces_service.mock'; import { ActionType } from '../types'; -import { ActionExecutionSourceType } from './action_execution_source'; import { actionsMock } from '../mocks'; +import { ActionExecutionSourceType } from './action_execution_source'; const actionExecutor = new ActionExecutor({ isESOCanEncrypt: true }); const services = actionsMock.createServices(); @@ -32,7 +32,6 @@ const executeParams = { executionId: '123abc', request: {} as KibanaRequest, actionExecutionId: '2', - sourceType: ActionExecutionSourceType.HTTP_REQUEST, }; const spacesMock = spacesServiceMock.createStartContract(); @@ -120,6 +119,142 @@ test('successfully executes', async () => { logger: loggerMock, }); + expect(loggerMock.debug).toBeCalledWith('executing action test:1: 1'); + expect(eventLogger.logEvent.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "event": Object { + "action": "execute-start", + "kind": "action", + }, + "kibana": Object { + "action": Object { + "execution": Object { + "uuid": "2", + }, + "id": "1", + "name": "1", + }, + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "1", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action started: test:1: 1", + }, + ], + Array [ + Object { + "event": Object { + "action": "execute", + "kind": "action", + "outcome": "success", + }, + "kibana": Object { + "action": Object { + "execution": Object { + "uuid": "2", + }, + "id": "1", + "name": "1", + }, + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "1", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action executed: test:1: 1", + }, + ], + ] + `); +}); + +test('successfully executes when sourceType is specified', async () => { + const actionType: jest.Mocked = { + id: 'test', + name: 'Test', + minimumLicenseRequired: 'basic', + supportedFeatureIds: ['alerting'], + executor: jest.fn(), + }; + const actionSavedObject = { + id: '1', + type: 'action', + attributes: { + name: '1', + actionTypeId: 'test', + config: { + bar: true, + }, + secrets: { + baz: true, + }, + }, + references: [], + }; + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(actionSavedObject); + actionTypeRegistry.get.mockReturnValueOnce(actionType); + await actionExecutor.execute({ + ...executeParams, + sourceType: ActionExecutionSourceType.HTTP_REQUEST, + }); + + expect(encryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledWith( + 'action', + '1', + { namespace: 'some-namespace' } + ); + + expect(actionTypeRegistry.get).toHaveBeenCalledWith('test'); + expect(actionTypeRegistry.isActionExecutable).toHaveBeenCalledWith('1', 'test', { + notifyUsage: true, + }); + + expect(actionType.executor).toHaveBeenCalledWith({ + actionId: '1', + services: expect.anything(), + config: { + bar: true, + }, + secrets: { + baz: true, + }, + params: { foo: true }, + logger: loggerMock, + }); + expect(loggerMock.debug).toBeCalledWith('executing action test:1: 1'); expect(eventLogger.logEvent.mock.calls).toMatchInlineSnapshot(` Array [ @@ -248,7 +383,6 @@ test('successfully executes with preconfigured connector', async () => { "kibana": Object { "action": Object { "execution": Object { - "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -288,7 +422,6 @@ test('successfully executes with preconfigured connector', async () => { "kibana": Object { "action": Object { "execution": Object { - "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -668,7 +801,6 @@ test('should not throw error if action is preconfigured and isESOCanEncrypt is f "kibana": Object { "action": Object { "execution": Object { - "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -708,7 +840,6 @@ test('should not throw error if action is preconfigured and isESOCanEncrypt is f "kibana": Object { "action": Object { "execution": Object { - "source": "HTTP_REQUEST", "uuid": "2", }, "id": "preconfigured", @@ -808,7 +939,6 @@ test('writes to event log for execute timeout', async () => { relatedSavedObjects: [], request: {} as KibanaRequest, actionExecutionId: '2', - sourceType: ActionExecutionSourceType.HTTP_REQUEST, }); expect(eventLogger.logEvent).toHaveBeenCalledTimes(1); expect(eventLogger.logEvent).toHaveBeenNthCalledWith(1, { @@ -819,7 +949,6 @@ test('writes to event log for execute timeout', async () => { kibana: { action: { execution: { - source: 'HTTP_REQUEST', uuid: '2', }, name: undefined, @@ -865,7 +994,6 @@ test('writes to event log for execute and execute start', async () => { kibana: { action: { execution: { - source: 'HTTP_REQUEST', uuid: '2', }, name: 'action-1', @@ -919,7 +1047,6 @@ test('writes to event log for execute and execute start when consumer and relate kibana: { action: { execution: { - source: 'HTTP_REQUEST', uuid: '2', }, name: 'action-1', diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index c053528e64770..5379bfd4be432 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -58,7 +58,7 @@ export interface ExecuteOptions { isEphemeral?: boolean; request: KibanaRequest; params: Record; - sourceType: ActionExecutionSourceType; + sourceType?: ActionExecutionSourceType; taskInfo?: TaskInfo; executionId?: string; consumer?: string; @@ -185,7 +185,7 @@ export class ActionExecutor { name, actionExecutionId, isPreconfigured: this.actionInfo.isPreconfigured, - source: sourceType, + ...(sourceType ? { source: sourceType } : {}), }); eventLogger.startTiming(event); @@ -302,7 +302,7 @@ export class ActionExecutor { taskInfo?: TaskInfo; executionId?: string; relatedSavedObjects: RelatedSavedObjects; - sourceType: ActionExecutionSourceType; + sourceType?: ActionExecutionSourceType; consumer?: string; }) { const { spaces, encryptedSavedObjectsClient, preconfiguredActions, eventLogger } = @@ -350,7 +350,7 @@ export class ActionExecutor { relatedSavedObjects, actionExecutionId, isPreconfigured: this.actionInfo.isPreconfigured, - source: sourceType, + ...(sourceType ? { source: sourceType } : {}), }); eventLogger.logEvent(event); diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts index 683cb4981eebd..43813173f17c2 100644 --- a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts +++ b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts @@ -132,7 +132,6 @@ test('executes the task by calling the executor with proper parameters, using gi actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -161,7 +160,6 @@ test('executes the task by calling the executor with proper parameters, using gi authorization: 'ApiKey MTIzOmFiYw==', }, }, - sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -188,7 +186,6 @@ test('executes the task by calling the executor with proper parameters, using st actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -217,7 +214,6 @@ test('executes the task by calling the executor with proper parameters, using st params: { baz: true }, executionId: '123abc', relatedSavedObjects: [], - sourceType: 'SAVED_OBJECT', request: { headers: { // base64 encoded "123:abc" @@ -241,6 +237,62 @@ test('executes the task by calling the executor with proper parameters when cons taskInstance: mockedTaskInstance, }); + mockedActionExecutor.execute.mockResolvedValueOnce({ status: 'ok', actionId: '2' }); + spaceIdToNamespace.mockReturnValueOnce('namespace-test'); + mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ + id: '3', + type: 'action_task_params', + attributes: { + actionId: '2', + consumer: 'test-consumer', + params: { baz: true }, + executionId: '123abc', + apiKey: Buffer.from('123:abc').toString('base64'), + }, + references: [], + }); + + const runnerResult = await taskRunner.run(); + + expect(runnerResult).toBeUndefined(); + expect(spaceIdToNamespace).toHaveBeenCalledWith('test'); + expect(mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledWith( + 'action_task_params', + '3', + { namespace: 'namespace-test' } + ); + + const [executeParams] = mockedActionExecutor.execute.mock.calls[0]; + expect(pick(executeParams, [...executeParamsFields, 'consumer'])).toEqual({ + actionId: '2', + consumer: 'test-consumer', + isEphemeral: false, + params: { baz: true }, + relatedSavedObjects: [], + executionId: '123abc', + request: { + headers: { + // base64 encoded "123:abc" + authorization: 'ApiKey MTIzOmFiYw==', + }, + }, + taskInfo: { + scheduled: new Date(), + attempts: 0, + }, + }); + + expect(taskRunnerFactoryInitializerParams.basePathService.set).toHaveBeenCalledWith( + executeParams.request, + '/s/test' + ); +}); + +test('executes the task by calling the executor with proper parameters when source is provided', async () => { + const taskRunner = taskRunnerFactory.create({ + taskInstance: mockedTaskInstance, + }); + mockedActionExecutor.execute.mockResolvedValueOnce({ status: 'ok', actionId: '2' }); spaceIdToNamespace.mockReturnValueOnce('namespace-test'); mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ @@ -275,13 +327,13 @@ test('executes the task by calling the executor with proper parameters when cons params: { baz: true }, relatedSavedObjects: [], executionId: '123abc', - sourceType: 'SAVED_OBJECT', request: { headers: { // base64 encoded "123:abc" authorization: 'ApiKey MTIzOmFiYw==', }, }, + sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -308,7 +360,6 @@ test('cleans up action_task_params object', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -335,7 +386,6 @@ test('task runner should implement CancellableTask cancel method with logging wa actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -374,7 +424,6 @@ test('runs successfully when cleanup fails and logs the error', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -409,7 +458,6 @@ test('throws an error with suggested retry logic when return status is error', a actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -452,7 +500,6 @@ test('uses API key when provided', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -473,7 +520,6 @@ test('uses API key when provided', async () => { params: { baz: true }, executionId: '123abc', relatedSavedObjects: [], - sourceType: 'SAVED_OBJECT', request: { headers: { // base64 encoded "123:abc" @@ -507,7 +553,6 @@ test('uses relatedSavedObjects merged with references when provided', async () = params: { baz: true }, executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), - source: 'SAVED_OBJECT', relatedSavedObjects: [{ id: 'related_some-type_0', type: 'some-type' }], }, references: [ @@ -532,7 +577,6 @@ test('uses relatedSavedObjects merged with references when provided', async () = isEphemeral: false, params: { baz: true }, executionId: '123abc', - sourceType: 'SAVED_OBJECT', relatedSavedObjects: [ { id: 'some-id', @@ -567,7 +611,6 @@ test('uses relatedSavedObjects as is when references are empty', async () => { params: { baz: true }, executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), - source: 'SAVED_OBJECT', relatedSavedObjects: [{ id: 'abc', type: 'some-type', namespace: 'yo' }], }, references: [ @@ -587,7 +630,6 @@ test('uses relatedSavedObjects as is when references are empty', async () => { isEphemeral: false, params: { baz: true }, executionId: '123abc', - sourceType: 'SAVED_OBJECT', relatedSavedObjects: [ { id: 'abc', @@ -623,7 +665,6 @@ test('sanitizes invalid relatedSavedObjects when provided', async () => { params: { baz: true }, executionId: '123abc', apiKey: Buffer.from('123:abc').toString('base64'), - source: 'SAVED_OBJECT', relatedSavedObjects: [{ Xid: 'related_some-type_0', type: 'some-type' }], }, references: [ @@ -655,7 +696,6 @@ test('sanitizes invalid relatedSavedObjects when provided', async () => { }, executionId: '123abc', relatedSavedObjects: [], - sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -676,7 +716,6 @@ test(`doesn't use API key when not provided`, async () => { attributes: { actionId: '2', params: { baz: true }, - source: 'SAVED_OBJECT', executionId: '123abc', }, references: [ @@ -700,7 +739,6 @@ test(`doesn't use API key when not provided`, async () => { request: { headers: {}, }, - sourceType: 'SAVED_OBJECT', taskInfo: { scheduled: new Date(), attempts: 0, @@ -731,7 +769,6 @@ test(`throws an error when license doesn't support the action type`, async () => actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -771,7 +808,6 @@ test(`treats errors as errors if the task is retryable`, async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -820,7 +856,6 @@ test(`treats errors as successes if the task is not retryable`, async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -866,7 +901,6 @@ test('treats errors as errors if the error is thrown instead of returned', async actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [ @@ -908,7 +942,6 @@ test('increments monitoring metrics after execution', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -941,7 +974,6 @@ test('increments monitoring metrics after a failed execution', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], @@ -974,7 +1006,6 @@ test('increments monitoring metrics after a timeout', async () => { actionId: '2', params: { baz: true }, executionId: '123abc', - source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.ts index 51d491b959e11..17e4b851366d5 100644 --- a/x-pack/plugins/actions/server/lib/task_runner_factory.ts +++ b/x-pack/plugins/actions/server/lib/task_runner_factory.ts @@ -122,12 +122,12 @@ export class TaskRunnerFactory { actionId: actionId as string, isEphemeral: !isPersistedActionTask(actionTaskExecutorParams), request, - sourceType: source as ActionExecutionSourceType, taskInfo, executionId, consumer, relatedSavedObjects: validatedRelatedSavedObjects(logger, relatedSavedObjects), actionExecutionId, + ...(source ? { sourceType: source as ActionExecutionSourceType } : {}), }); } catch (e) { logger.error( @@ -212,8 +212,8 @@ export class TaskRunnerFactory { consumer, executionId, relatedSavedObjects: (relatedSavedObjects || []) as RelatedSavedObjects, - sourceType: source as ActionExecutionSourceType, actionExecutionId, + ...(source ? { sourceType: source as ActionExecutionSourceType } : {}), }); inMemoryMetrics.increment(IN_MEMORY_METRICS.ACTION_TIMEOUTS); From 111447788403be339f9800353fc945954c46913f Mon Sep 17 00:00:00 2001 From: Ying Date: Fri, 24 Feb 2023 15:35:34 -0500 Subject: [PATCH 05/14] Showing source on exec log --- .../actions/common/execution_log_types.ts | 1 + .../lib/get_execution_log_aggregation.test.ts | 24 ++++++++++++++++--- .../lib/get_execution_log_aggregation.ts | 4 ++++ ...ctions_connectors_event_log_list_table.tsx | 15 ++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/actions/common/execution_log_types.ts b/x-pack/plugins/actions/common/execution_log_types.ts index 7fed846f081ca..d28ee5cf50481 100644 --- a/x-pack/plugins/actions/common/execution_log_types.ts +++ b/x-pack/plugins/actions/common/execution_log_types.ts @@ -19,6 +19,7 @@ export interface IExecutionLog { connector_name: string; connector_id: string; timed_out: boolean; + source: string; } export interface IExecutionLogResult { diff --git a/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.test.ts b/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.test.ts index c92f4be981795..f85d51b5ae2c3 100644 --- a/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.test.ts +++ b/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.test.ts @@ -96,6 +96,7 @@ describe('getExecutionLogAggregation', () => { 'kibana.space_ids', 'kibana.action.name', 'kibana.action.id', + 'kibana.action.execution.source', ], }, }, @@ -225,6 +226,7 @@ describe('getExecutionLogAggregation', () => { 'kibana.space_ids', 'kibana.action.name', 'kibana.action.id', + 'kibana.action.execution.source', ], }, }, @@ -370,6 +372,7 @@ describe('getExecutionLogAggregation', () => { 'kibana.space_ids', 'kibana.action.name', 'kibana.action.id', + 'kibana.action.execution.source', ], }, }, @@ -526,7 +529,11 @@ describe('formatExecutionLogResult', () => { kibana: { space_ids: ['default'], version: '8.7.0', - action: { name: 'test connector', id: '1' }, + action: { + name: 'test connector', + id: '1', + execution: { source: 'SAVED_OBJECT' }, + }, }, message: 'action executed: .server-log:6709f660-8d11-11ed-bae5-bd32cbc9eaaa: test connector', @@ -563,6 +570,7 @@ describe('formatExecutionLogResult', () => { timestamp: '2023-01-05T15:55:50.495Z', version: '8.7.0', timed_out: false, + source: 'SAVED_OBJECT', }, ], total: 1, @@ -598,7 +606,11 @@ describe('formatExecutionLogResult', () => { kibana: { space_ids: ['default'], version: '8.7.0', - action: { name: 'test', id: '1' }, + action: { + name: 'test', + id: '1', + execution: { source: 'SAVED_OBJECT' }, + }, }, message: 'action execution failure: .email:e020c620-8d14-11ed-bae5-bd32cbc9eaaa: test', @@ -638,7 +650,11 @@ describe('formatExecutionLogResult', () => { kibana: { space_ids: ['default'], version: '8.7.0', - action: { name: 'test connector', id: '1' }, + action: { + name: 'test connector', + id: '1', + execution: { source: 'SAVED_OBJECT' }, + }, }, message: 'action executed: .server-log:6709f660-8d11-11ed-bae5-bd32cbc9eaaa: test connector', @@ -675,6 +691,7 @@ describe('formatExecutionLogResult', () => { timestamp: '2023-01-05T16:23:53.813Z', version: '8.7.0', timed_out: false, + source: 'SAVED_OBJECT', }, { connector_name: 'test connector', @@ -689,6 +706,7 @@ describe('formatExecutionLogResult', () => { timestamp: '2023-01-05T15:55:50.495Z', version: '8.7.0', timed_out: false, + source: 'SAVED_OBJECT', }, ], total: 2, diff --git a/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.ts b/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.ts index e2f3940716317..ddbb864d4c06a 100644 --- a/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.ts +++ b/x-pack/plugins/actions/server/lib/get_execution_log_aggregation.ts @@ -28,6 +28,7 @@ const VERSION_FIELD = 'kibana.version'; const ERROR_MESSAGE_FIELD = 'error.message'; const SCHEDULE_DELAY_FIELD = 'kibana.task.schedule_delay'; const EXECUTION_UUID_FIELD = 'kibana.action.execution.uuid'; +const EXECUTION_SOURCE_FIELD = 'kibana.action.execution.source'; const Millis2Nanos = 1000 * 1000; @@ -257,6 +258,7 @@ export function getExecutionLogAggregation({ SPACE_ID_FIELD, ACTION_NAME_FIELD, ACTION_ID_FIELD, + EXECUTION_SOURCE_FIELD, ], }, }, @@ -312,6 +314,7 @@ function formatExecutionLogAggBucket(bucket: IExecutionUuidAggBucket): IExecutio const message = status === 'failure' ? `${outcomeMessage} - ${outcomeErrorMessage}` : outcomeMessage; const version = outcomeAndMessage.kibana?.version ?? ''; + const source = outcomeAndMessage.kibana?.action?.execution?.source ?? ''; const spaceIds = outcomeAndMessage?.kibana?.space_ids ?? []; const connectorName = outcomeAndMessage?.kibana?.action?.name ?? ''; @@ -324,6 +327,7 @@ function formatExecutionLogAggBucket(bucket: IExecutionUuidAggBucket): IExecutio status, message, version, + source, schedule_delay_ms: scheduleDelayUs / Millis2Nanos, space_ids: spaceIds, connector_name: connectorName, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_event_log_list_table.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_event_log_list_table.tsx index a2a674ecd73ee..ec8d0d305ba69 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_event_log_list_table.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_event_log_list_table.tsx @@ -377,6 +377,21 @@ export const ConnectorEventLogListTable = Date: Fri, 24 Feb 2023 16:05:21 -0500 Subject: [PATCH 06/14] Fixing tests --- .../actions/server/routes/get_global_execution_logs.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/actions/server/routes/get_global_execution_logs.test.ts b/x-pack/plugins/actions/server/routes/get_global_execution_logs.test.ts index e91ab9268bac5..1c309e14dae09 100644 --- a/x-pack/plugins/actions/server/routes/get_global_execution_logs.test.ts +++ b/x-pack/plugins/actions/server/routes/get_global_execution_logs.test.ts @@ -39,6 +39,7 @@ describe('getRuleExecutionLogRoute', () => { timestamp: '2023-01-05T15:55:50.495Z', version: '8.7.0', timed_out: false, + source: 'SAVED_OBJECT', }, ], total: 1, From 885c1de80e0133affd22a0e94576a1a524bf5509 Mon Sep 17 00:00:00 2001 From: Ying Date: Mon, 27 Feb 2023 09:44:37 -0500 Subject: [PATCH 07/14] Adding functional tests --- .../group2/tests/actions/execute.ts | 9 ++++++++- .../group2/tests/alerting/alerts.ts | 9 ++++++++- .../spaces_only/tests/actions/execute.ts | 20 +++++++++++++++++-- .../actions/schedule_unsecured_action.ts | 5 +++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/execute.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/execute.ts index 6ee116d3052e2..212331905422a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/execute.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/execute.ts @@ -8,6 +8,7 @@ import expect from '@kbn/expect'; import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { ESTestIndexTool, ES_TEST_INDEX_NAME } from '@kbn/alerting-api-integration-helpers'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { UserAtSpaceScenarios } from '../../../scenarios'; import { getUrlPrefix, ObjectRemover, getEventLog } from '../../../../common/lib'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; @@ -115,6 +116,7 @@ export default function ({ getService }: FtrProviderContext) { outcome: 'success', actionTypeId: 'test.index-record', message: `action executed: test.index-record:${createdAction.id}: My action`, + source: ActionExecutionSourceType.HTTP_REQUEST, }); break; default: @@ -502,10 +504,11 @@ export default function ({ getService }: FtrProviderContext) { outcome: string; message: string; errorMessage?: string; + source?: string; } async function validateEventLog(params: ValidateEventLogParams): Promise { - const { spaceId, connectorId, actionTypeId, outcome, message, errorMessage } = params; + const { spaceId, connectorId, actionTypeId, outcome, message, errorMessage, source } = params; const events: IValidatedEvent[] = await retry.try(async () => { return await getEventLog({ @@ -559,6 +562,10 @@ export default function ({ getService }: FtrProviderContext) { expect(executeEvent?.message).to.eql(message); expect(startExecuteEvent?.message).to.eql(message.replace('executed', 'started')); + if (source) { + expect(executeEvent?.kibana?.action?.execution?.source).to.eql(source); + } + if (errorMessage) { expect(executeEvent?.error?.message).to.eql(errorMessage); } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts index e97c0df7bf1df..03f7fbd387f1e 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts @@ -12,6 +12,7 @@ import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { TaskRunning, TaskRunningStage } from '@kbn/task-manager-plugin/server/task_running'; import { ConcreteTaskInstance } from '@kbn/task-manager-plugin/server'; import { ESTestIndexTool, ES_TEST_INDEX_NAME } from '@kbn/alerting-api-integration-helpers'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { UserAtSpaceScenarios, Superuser } from '../../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -230,6 +231,7 @@ instanceStateValue: true outcome: 'success', message: `rule executed: test.always-firing:${alertId}: 'abc'`, ruleObject: alertSearchResultWithoutDates, + source: ActionExecutionSourceType.SAVED_OBJECT, }); break; default: @@ -1365,10 +1367,11 @@ instanceStateValue: true message: string; errorMessage?: string; ruleObject: any; + source?: string; } async function validateEventLog(params: ValidateEventLogParams): Promise { - const { spaceId, alertId, outcome, message, errorMessage, ruleObject } = params; + const { spaceId, alertId, outcome, message, errorMessage, ruleObject, source } = params; const events: IValidatedEvent[] = await retry.try(async () => { return await getEventLog({ @@ -1455,6 +1458,10 @@ instanceStateValue: true expect(event?.message).to.eql(message); + if (source) { + expect(event?.kibana?.action?.execution?.source).to.eql(source); + } + if (errorMessage) { expect(event?.error?.message).to.eql(errorMessage); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts index f66570fc2655f..50f5bc7ac3536 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/execute.ts @@ -8,6 +8,7 @@ import type { Client } from '@elastic/elasticsearch'; import expect from '@kbn/expect'; import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { ESTestIndexTool, ES_TEST_INDEX_NAME } from '@kbn/alerting-api-integration-helpers'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../scenarios'; import { getUrlPrefix, ObjectRemover, getEventLog } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; @@ -95,6 +96,7 @@ export default function ({ getService }: FtrProviderContext) { outcome: 'success', message: `action executed: test.index-record:${createdAction.id}: My action`, startMessage: `action started: test.index-record:${createdAction.id}: My action`, + source: ActionExecutionSourceType.HTTP_REQUEST, }); }); @@ -138,6 +140,7 @@ export default function ({ getService }: FtrProviderContext) { outcome: 'failure', message: `action execution failure: test.failing:${createdAction.id}: failing action`, errorMessage: `an error occurred while running the action: expected failure for .kibana-alerting-test-data actions-failure-1:space1; retry: true`, + source: ActionExecutionSourceType.HTTP_REQUEST, }); }); @@ -336,11 +339,20 @@ export default function ({ getService }: FtrProviderContext) { message: string; errorMessage?: string; startMessage?: string; + source?: string; } async function validateEventLog(params: ValidateEventLogParams): Promise { - const { spaceId, actionId, actionTypeId, outcome, message, startMessage, errorMessage } = - params; + const { + spaceId, + actionId, + actionTypeId, + outcome, + message, + startMessage, + errorMessage, + source, + } = params; const events: IValidatedEvent[] = await retry.try(async () => { return await getEventLog({ @@ -397,6 +409,10 @@ export default function ({ getService }: FtrProviderContext) { expect(executeEvent?.kibana?.task).to.eql(undefined); + if (source) { + expect(executeEvent?.kibana?.action?.execution?.source).to.eql(source); + } + if (errorMessage) { expect(executeEvent?.error?.message).to.eql(errorMessage); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts index 9a5719b7fa700..ed84a6f589f1d 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts @@ -7,6 +7,7 @@ import expect from '@kbn/expect'; import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; @@ -108,6 +109,10 @@ export default function createUnsecuredActionTests({ getService }: FtrProviderCo expect(hit?._source?.message).to.eql( `action executed: .email:my-test-email: TestEmail#xyz` ); + // @ts-expect-error _source: unknown + expect(hit?._source?.kibana?.actions?.execution?.source).to.eql( + ActionExecutionSourceType.NOTIFICATION + ); }); }); From 8e270b40e28204317577266872294f9ff8719ad5 Mon Sep 17 00:00:00 2001 From: Ying Date: Mon, 27 Feb 2023 09:48:25 -0500 Subject: [PATCH 08/14] dont need to index new field --- x-pack/plugins/actions/server/saved_objects/mappings.json | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/actions/server/saved_objects/mappings.json b/x-pack/plugins/actions/server/saved_objects/mappings.json index 2e6a0a7dce6e2..184c867a738e8 100644 --- a/x-pack/plugins/actions/server/saved_objects/mappings.json +++ b/x-pack/plugins/actions/server/saved_objects/mappings.json @@ -47,6 +47,7 @@ "type": "object" }, "source": { + "index": false, "type": "keyword" } } From 6a523ccc808c3070a1a3400594cd834633c7ee1f Mon Sep 17 00:00:00 2001 From: Ying Date: Mon, 27 Feb 2023 11:12:09 -0500 Subject: [PATCH 09/14] cleanup --- .../group2/check_registered_types.test.ts | 2 +- .../create_unsecured_execute_function.ts | 6 ++-- .../unsecured_actions_client.test.ts | 30 +++++++++++++++---- .../unsecured_actions_client.ts | 26 ++++++++++++++-- .../services/connectors_email_service.test.ts | 28 ----------------- .../services/connectors_email_service.ts | 9 +----- .../group2/tests/alerting/alerts.ts | 9 +----- .../actions/schedule_unsecured_action.ts | 5 ---- .../tests/alerting/group1/event_log.ts | 8 +++++ 9 files changed, 63 insertions(+), 60 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 57792759e1fab..d8303fe258b9b 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -56,7 +56,7 @@ describe('checking migration metadata changes on all registered SO types', () => expect(hashMap).toMatchInlineSnapshot(` Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", - "action_task_params": "c2cb5c9060322e3e51c9e65debd8df48c0e54b8f", + "action_task_params": "7cfbd949cbb24f7ede1eeb7a62fb711bdeec501d", "alert": "2568bf6d8ba0876441c61c9e58e08016c1dc1617", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts index c5870420178ff..c16b5e4d0b1a7 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -31,7 +31,7 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { export interface ExecuteOptions extends Pick { id: string; - source: ActionExecutionSource; + source?: ActionExecutionSource; } interface ActionTaskParams @@ -115,8 +115,8 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ actionId: actionToExecute.id, params: actionToExecute.params, apiKey: null, - source: actionToExecute.source.type, relatedSavedObjects: relatedSavedObjectWithRefs, + ...(actionToExecute.source ? { source: actionToExecute.source.type } : {}), }, references: taskReferences, }; @@ -140,7 +140,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ }; } -function executionSourceAsSavedObjectReferences(executionSource: ActionExecutionSource) { +function executionSourceAsSavedObjectReferences(executionSource?: ActionExecutionSource) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts index b07c80f8da3a0..7d0d4d687a71a 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts @@ -29,13 +29,11 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( @@ -51,19 +49,41 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( - unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + unsecuredActionsClient.bulkEnqueueExecution('functional_tester', opts) ).resolves.toMatchInlineSnapshot(`undefined`); expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, opts); }); + + test('injects source and calls the executionEnqueuer with the appropriate parameters when requester is "notifications"', async () => { + const opts = [ + { + id: 'preconfigured1', + params: {}, + executionId: '123abc', + }, + { + id: 'preconfigured2', + params: {}, + executionId: '456def', + }, + ]; + await expect( + unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + ).resolves.toMatchInlineSnapshot(`undefined`); + + const optsWithSource = opts.map((opt) => ({ + ...opt, + source: asNotificationExecutionSource({ connectorId: opt.id, requesterId: 'notifications' }), + })); + expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, optsWithSource); + }); }); diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts index 333490389013a..a2d87c8a5db4a 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts @@ -10,11 +10,14 @@ import { BulkUnsecuredExecutionEnqueuer, ExecuteOptions, } from '../create_unsecured_execute_function'; +import { asNotificationExecutionSource } from '../lib'; + +const NOTIFICATION_REQUESTER_ID = 'notifications'; // allowlist for features wanting access to the unsecured actions client // which allows actions to be enqueued for execution without a user request const ALLOWED_REQUESTER_IDS = [ - 'notifications', + NOTIFICATION_REQUESTER_ID, // For functional testing 'functional_tester', ]; @@ -47,6 +50,25 @@ export class UnsecuredActionsClient { `"${requesterId}" feature is not allow-listed for UnsecuredActionsClient access.` ); } - return this.executionEnqueuer(this.internalSavedObjectsRepository, actionsToExecute); + // Inject source based on requesterId + return this.executionEnqueuer( + this.internalSavedObjectsRepository, + this.injectSource(requesterId, actionsToExecute) + ); + } + + private injectSource(requesterId: string, actionsToExecute: ExecuteOptions[]): ExecuteOptions[] { + switch (requesterId) { + case NOTIFICATION_REQUESTER_ID: + return actionsToExecute.map((actionToExecute) => ({ + ...actionToExecute, + source: asNotificationExecutionSource({ + requesterId, + connectorId: actionToExecute.id, + }), + })); + default: + return actionsToExecute; + } } } diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts index c9fd5d8004411..f07b3a3ab34ae 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts @@ -34,13 +34,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, }, ]); }); @@ -74,13 +67,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -96,13 +82,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -118,13 +97,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.ts index 630ad8d22fa9c..55586dd05b078 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -5,10 +5,7 @@ * 2.0. */ -import { - asNotificationExecutionSource, - type IUnsecuredActionsClient, -} from '@kbn/actions-plugin/server'; +import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { @@ -26,10 +23,6 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - source: asNotificationExecutionSource({ - requesterId: this.requesterId, - connectorId: this.connectorId, - }), relatedSavedObjects: params.context?.relatedObjects, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts index 03f7fbd387f1e..e97c0df7bf1df 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/alerts.ts @@ -12,7 +12,6 @@ import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { TaskRunning, TaskRunningStage } from '@kbn/task-manager-plugin/server/task_running'; import { ConcreteTaskInstance } from '@kbn/task-manager-plugin/server'; import { ESTestIndexTool, ES_TEST_INDEX_NAME } from '@kbn/alerting-api-integration-helpers'; -import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { UserAtSpaceScenarios, Superuser } from '../../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { @@ -231,7 +230,6 @@ instanceStateValue: true outcome: 'success', message: `rule executed: test.always-firing:${alertId}: 'abc'`, ruleObject: alertSearchResultWithoutDates, - source: ActionExecutionSourceType.SAVED_OBJECT, }); break; default: @@ -1367,11 +1365,10 @@ instanceStateValue: true message: string; errorMessage?: string; ruleObject: any; - source?: string; } async function validateEventLog(params: ValidateEventLogParams): Promise { - const { spaceId, alertId, outcome, message, errorMessage, ruleObject, source } = params; + const { spaceId, alertId, outcome, message, errorMessage, ruleObject } = params; const events: IValidatedEvent[] = await retry.try(async () => { return await getEventLog({ @@ -1458,10 +1455,6 @@ instanceStateValue: true expect(event?.message).to.eql(message); - if (source) { - expect(event?.kibana?.action?.execution?.source).to.eql(source); - } - if (errorMessage) { expect(event?.error?.message).to.eql(errorMessage); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts index ed84a6f589f1d..9a5719b7fa700 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts @@ -7,7 +7,6 @@ import expect from '@kbn/expect'; import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; -import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; @@ -109,10 +108,6 @@ export default function createUnsecuredActionTests({ getService }: FtrProviderCo expect(hit?._source?.message).to.eql( `action executed: .email:my-test-email: TestEmail#xyz` ); - // @ts-expect-error _source: unknown - expect(hit?._source?.kibana?.actions?.execution?.source).to.eql( - ActionExecutionSourceType.NOTIFICATION - ); }); }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts index 2ee65b9edc3e7..39f9ca708e10f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts @@ -9,6 +9,7 @@ import expect from '@kbn/expect'; import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { RuleNotifyWhen } from '@kbn/alerting-plugin/common'; import { ES_TEST_INDEX_NAME, ESTestIndexTool } from '@kbn/alerting-api-integration-helpers'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../../scenarios'; import { getUrlPrefix, @@ -319,6 +320,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { ruleTypeId: response.body.rule_type_id, rule: undefined, consumer: 'alertsFixture', + source: ActionExecutionSourceType.SAVED_OBJECT, }); break; } @@ -997,6 +999,7 @@ interface ValidateEventLogParams { namespace?: string; }; flapping?: boolean; + source?: string; } export function validateEvent(event: IValidatedEvent, params: ValidateEventLogParams): void { @@ -1016,6 +1019,7 @@ export function validateEvent(event: IValidatedEvent, params: ValidateEventLogPa consumer, ruleTypeId, flapping, + source, } = params; const { status, actionGroupId, instanceId, reason, shouldHaveEventEnd } = params; @@ -1069,6 +1073,10 @@ export function validateEvent(event: IValidatedEvent, params: ValidateEventLogPa expect(event?.kibana?.alert?.flapping).to.be(flapping); } + if (source) { + expect(event?.kibana?.action?.execution?.source).to.be(source); + } + expect(event?.kibana?.alert?.rule?.rule_type_id).to.be(ruleTypeId); expect(event?.kibana?.space_ids?.[0]).to.equal(spaceId); From 79febfcd0434d93db25465765965a1c95cc31d16 Mon Sep 17 00:00:00 2001 From: Ying Date: Mon, 27 Feb 2023 11:23:47 -0500 Subject: [PATCH 10/14] cleanup --- .../group2/check_registered_types.test.ts | 2 +- .../plugins/actions/server/actions_client.ts | 13 +++++--- .../actions/server/create_execute_function.ts | 8 ++--- .../create_unsecured_execute_function.ts | 6 ++-- .../unsecured_actions_client.test.ts | 30 +++++++++++++++---- .../unsecured_actions_client.ts | 26 ++++++++++++++-- .../services/connectors_email_service.test.ts | 28 ----------------- .../services/connectors_email_service.ts | 9 +----- .../server/unsecured_actions_simulation.ts | 5 ---- .../group2/tests/alerting/alerts.ts | 9 +----- .../actions/schedule_unsecured_action.ts | 5 ---- .../tests/alerting/group1/event_log.ts | 8 +++++ 12 files changed, 76 insertions(+), 73 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 57792759e1fab..d8303fe258b9b 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -56,7 +56,7 @@ describe('checking migration metadata changes on all registered SO types', () => expect(hashMap).toMatchInlineSnapshot(` Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", - "action_task_params": "c2cb5c9060322e3e51c9e65debd8df48c0e54b8f", + "action_task_params": "7cfbd949cbb24f7ede1eeb7a62fb711bdeec501d", "alert": "2568bf6d8ba0876441c61c9e58e08016c1dc1617", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index a073152895bc8..6f29ce3895b48 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -50,7 +50,7 @@ import { ConnectorTokenClientContract, } from './types'; import { PreconfiguredActionDisabledModificationError } from './lib/errors/preconfigured_action_disabled_modification'; -import { ExecuteOptions } from './lib/action_executor'; +import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; import { ExecutionEnqueuer, ExecuteOptions as EnqueueExecutionOptions, @@ -127,6 +127,13 @@ interface ConstructorOptions { getEventLogClient: () => Promise; } +type ExecuteOptions = Omit< + ActionExecutorOptions, + 'request' | 'actionExecutionId' | 'sourceType' +> & { + source: ActionExecutionSource; +}; + export interface UpdateOptions { id: string; action: ActionUpdate; @@ -667,9 +674,7 @@ export class ActionsClient { params, source, relatedSavedObjects, - }: Omit & { - source: ActionExecutionSource; - }): Promise> { + }: ExecuteOptions): Promise> { if ( (await getAuthorizationModeBySource(this.unsecuredSavedObjectsClient, source)) === AuthorizationMode.RBAC diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index fe4f229560682..518126725bef3 100644 --- a/x-pack/plugins/actions/server/create_execute_function.ts +++ b/x-pack/plugins/actions/server/create_execute_function.ts @@ -34,7 +34,7 @@ export interface ExecuteOptions spaceId: string; apiKey: string | null; executionId: string; - source: ActionExecutionSource; + source?: ActionExecutionSource; } interface ActionTaskParams @@ -120,8 +120,8 @@ export function createExecutionEnqueuerFunction({ apiKey, executionId, consumer, - source: source.type, relatedSavedObjects: relatedSavedObjectWithRefs, + ...(source ? { source: source.type } : {}), }, { references: taskReferences, @@ -207,8 +207,8 @@ export function createBulkExecutionEnqueuerFunction({ apiKey: actionToExecute.apiKey, executionId: actionToExecute.executionId, consumer: actionToExecute.consumer, - source: actionToExecute.source.type, relatedSavedObjects: relatedSavedObjectWithRefs, + ...(actionToExecute.source ? { source: actionToExecute.source.type } : {}), }, references: taskReferences, }; @@ -280,7 +280,7 @@ function validateCanActionBeUsed(action: PreConfiguredAction | RawAction) { } } -function executionSourceAsSavedObjectReferences(executionSource: ActionExecutionSource) { +function executionSourceAsSavedObjectReferences(executionSource?: ActionExecutionSource) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts index c5870420178ff..c16b5e4d0b1a7 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -31,7 +31,7 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { export interface ExecuteOptions extends Pick { id: string; - source: ActionExecutionSource; + source?: ActionExecutionSource; } interface ActionTaskParams @@ -115,8 +115,8 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ actionId: actionToExecute.id, params: actionToExecute.params, apiKey: null, - source: actionToExecute.source.type, relatedSavedObjects: relatedSavedObjectWithRefs, + ...(actionToExecute.source ? { source: actionToExecute.source.type } : {}), }, references: taskReferences, }; @@ -140,7 +140,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ }; } -function executionSourceAsSavedObjectReferences(executionSource: ActionExecutionSource) { +function executionSourceAsSavedObjectReferences(executionSource?: ActionExecutionSource) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts index b07c80f8da3a0..7d0d4d687a71a 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts @@ -29,13 +29,11 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( @@ -51,19 +49,41 @@ describe('bulkEnqueueExecution()', () => { id: 'preconfigured1', params: {}, executionId: '123abc', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, { id: 'preconfigured2', params: {}, executionId: '456def', - source: asNotificationExecutionSource({ connectorId: 'abc', requesterId: 'foo' }), }, ]; await expect( - unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + unsecuredActionsClient.bulkEnqueueExecution('functional_tester', opts) ).resolves.toMatchInlineSnapshot(`undefined`); expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, opts); }); + + test('injects source and calls the executionEnqueuer with the appropriate parameters when requester is "notifications"', async () => { + const opts = [ + { + id: 'preconfigured1', + params: {}, + executionId: '123abc', + }, + { + id: 'preconfigured2', + params: {}, + executionId: '456def', + }, + ]; + await expect( + unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + ).resolves.toMatchInlineSnapshot(`undefined`); + + const optsWithSource = opts.map((opt) => ({ + ...opt, + source: asNotificationExecutionSource({ connectorId: opt.id, requesterId: 'notifications' }), + })); + expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, optsWithSource); + }); }); diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts index 333490389013a..a2d87c8a5db4a 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts @@ -10,11 +10,14 @@ import { BulkUnsecuredExecutionEnqueuer, ExecuteOptions, } from '../create_unsecured_execute_function'; +import { asNotificationExecutionSource } from '../lib'; + +const NOTIFICATION_REQUESTER_ID = 'notifications'; // allowlist for features wanting access to the unsecured actions client // which allows actions to be enqueued for execution without a user request const ALLOWED_REQUESTER_IDS = [ - 'notifications', + NOTIFICATION_REQUESTER_ID, // For functional testing 'functional_tester', ]; @@ -47,6 +50,25 @@ export class UnsecuredActionsClient { `"${requesterId}" feature is not allow-listed for UnsecuredActionsClient access.` ); } - return this.executionEnqueuer(this.internalSavedObjectsRepository, actionsToExecute); + // Inject source based on requesterId + return this.executionEnqueuer( + this.internalSavedObjectsRepository, + this.injectSource(requesterId, actionsToExecute) + ); + } + + private injectSource(requesterId: string, actionsToExecute: ExecuteOptions[]): ExecuteOptions[] { + switch (requesterId) { + case NOTIFICATION_REQUESTER_ID: + return actionsToExecute.map((actionToExecute) => ({ + ...actionToExecute, + source: asNotificationExecutionSource({ + requesterId, + connectorId: actionToExecute.id, + }), + })); + default: + return actionsToExecute; + } } } diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts index c9fd5d8004411..f07b3a3ab34ae 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts @@ -34,13 +34,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, }, ]); }); @@ -74,13 +67,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -96,13 +82,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', @@ -118,13 +97,6 @@ describe('sendPlainTextEmail()', () => { subject: 'This is a notification email', message: 'With some contents inside.', }, - source: { - source: { - connectorId: CONNECTOR_ID, - requesterId: REQUESTER_ID, - }, - type: 'NOTIFICATION', - }, relatedSavedObjects: [ { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.ts index 630ad8d22fa9c..55586dd05b078 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -5,10 +5,7 @@ * 2.0. */ -import { - asNotificationExecutionSource, - type IUnsecuredActionsClient, -} from '@kbn/actions-plugin/server'; +import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { @@ -26,10 +23,6 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - source: asNotificationExecutionSource({ - requesterId: this.requesterId, - connectorId: this.connectorId, - }), relatedSavedObjects: params.context?.relatedObjects, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); diff --git a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts index dd42a49905b47..4f82e435387f9 100644 --- a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts +++ b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { asNotificationExecutionSource } from '@kbn/actions-plugin/server'; import { schema } from '@kbn/config-schema'; import { CoreSetup, @@ -44,10 +43,6 @@ export function initPlugin(router: IRouter, coreSetup: CoreSetup { - const { spaceId, alertId, outcome, message, errorMessage, ruleObject, source } = params; + const { spaceId, alertId, outcome, message, errorMessage, ruleObject } = params; const events: IValidatedEvent[] = await retry.try(async () => { return await getEventLog({ @@ -1458,10 +1455,6 @@ instanceStateValue: true expect(event?.message).to.eql(message); - if (source) { - expect(event?.kibana?.action?.execution?.source).to.eql(source); - } - if (errorMessage) { expect(event?.error?.message).to.eql(errorMessage); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts index ed84a6f589f1d..9a5719b7fa700 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts @@ -7,7 +7,6 @@ import expect from '@kbn/expect'; import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; -import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; @@ -109,10 +108,6 @@ export default function createUnsecuredActionTests({ getService }: FtrProviderCo expect(hit?._source?.message).to.eql( `action executed: .email:my-test-email: TestEmail#xyz` ); - // @ts-expect-error _source: unknown - expect(hit?._source?.kibana?.actions?.execution?.source).to.eql( - ActionExecutionSourceType.NOTIFICATION - ); }); }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts index 2ee65b9edc3e7..39f9ca708e10f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts @@ -9,6 +9,7 @@ import expect from '@kbn/expect'; import { IValidatedEvent, nanosToMillis } from '@kbn/event-log-plugin/server'; import { RuleNotifyWhen } from '@kbn/alerting-plugin/common'; import { ES_TEST_INDEX_NAME, ESTestIndexTool } from '@kbn/alerting-api-integration-helpers'; +import { ActionExecutionSourceType } from '@kbn/actions-plugin/server/lib/action_execution_source'; import { Spaces } from '../../../scenarios'; import { getUrlPrefix, @@ -319,6 +320,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { ruleTypeId: response.body.rule_type_id, rule: undefined, consumer: 'alertsFixture', + source: ActionExecutionSourceType.SAVED_OBJECT, }); break; } @@ -997,6 +999,7 @@ interface ValidateEventLogParams { namespace?: string; }; flapping?: boolean; + source?: string; } export function validateEvent(event: IValidatedEvent, params: ValidateEventLogParams): void { @@ -1016,6 +1019,7 @@ export function validateEvent(event: IValidatedEvent, params: ValidateEventLogPa consumer, ruleTypeId, flapping, + source, } = params; const { status, actionGroupId, instanceId, reason, shouldHaveEventEnd } = params; @@ -1069,6 +1073,10 @@ export function validateEvent(event: IValidatedEvent, params: ValidateEventLogPa expect(event?.kibana?.alert?.flapping).to.be(flapping); } + if (source) { + expect(event?.kibana?.action?.execution?.source).to.be(source); + } + expect(event?.kibana?.alert?.rule?.rule_type_id).to.be(ruleTypeId); expect(event?.kibana?.space_ids?.[0]).to.equal(spaceId); From 91209de9cd96b4de562d2e3855f7d0b760334951 Mon Sep 17 00:00:00 2001 From: Ying Date: Wed, 1 Mar 2023 16:20:30 -0500 Subject: [PATCH 11/14] Differentiating between saved object sources and lowercasing the string --- .../actions/server/actions_client.test.ts | 6 +- .../plugins/actions/server/actions_client.ts | 15 +- .../actions/server/create_execute_function.ts | 11 +- .../create_unsecured_execute_function.ts | 11 +- .../server/lib/action_execution_source.ts | 7 + .../server/lib/action_executor.test.ts | 156 +++++++++++++++++- .../actions/server/lib/action_executor.ts | 18 +- ...ate_action_event_log_record_object.test.ts | 66 +++++++- .../create_action_event_log_record_object.ts | 13 +- .../server/lib/task_runner_factory.test.ts | 70 +++++++- .../actions/server/lib/task_runner_factory.ts | 23 ++- .../server/unsecured_actions_simulation.ts | 7 +- .../group2/tests/actions/execute.ts | 2 +- .../spaces_only/tests/actions/execute.ts | 2 +- .../tests/alerting/group1/event_log.ts | 3 +- 15 files changed, 343 insertions(+), 67 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index 3bf49ab5281d5..3c2d91939e688 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -2240,7 +2240,7 @@ describe('execute()', () => { name: 'my name', }, actionExecutionId, - sourceType: 'HTTP_REQUEST', + source: asHttpRequestExecutionSource(request), }); await expect( @@ -2274,7 +2274,7 @@ describe('execute()', () => { }, ], actionExecutionId, - sourceType: 'HTTP_REQUEST', + source: asHttpRequestExecutionSource(request), }); await expect( @@ -2301,7 +2301,7 @@ describe('execute()', () => { params: { name: 'my name', }, - sourceType: 'HTTP_REQUEST', + source: asHttpRequestExecutionSource(request), relatedSavedObjects: [ { id: 'some-id', diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index 6f29ce3895b48..867fc4f9c7e47 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -50,7 +50,7 @@ import { ConnectorTokenClientContract, } from './types'; import { PreconfiguredActionDisabledModificationError } from './lib/errors/preconfigured_action_disabled_modification'; -import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; +import { ExecuteOptions } from './lib/action_executor'; import { ExecutionEnqueuer, ExecuteOptions as EnqueueExecutionOptions, @@ -127,13 +127,6 @@ interface ConstructorOptions { getEventLogClient: () => Promise; } -type ExecuteOptions = Omit< - ActionExecutorOptions, - 'request' | 'actionExecutionId' | 'sourceType' -> & { - source: ActionExecutionSource; -}; - export interface UpdateOptions { id: string; action: ActionUpdate; @@ -674,7 +667,9 @@ export class ActionsClient { params, source, relatedSavedObjects, - }: ExecuteOptions): Promise> { + }: Omit): Promise< + ActionTypeExecutorResult + > { if ( (await getAuthorizationModeBySource(this.unsecuredSavedObjectsClient, source)) === AuthorizationMode.RBAC @@ -687,7 +682,7 @@ export class ActionsClient { return this.actionExecutor.execute({ actionId, params, - sourceType: source.type, + source, request: this.request, relatedSavedObjects, actionExecutionId: uuidv4(), diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index 518126725bef3..c33bcc6923d8a 100644 --- a/x-pack/plugins/actions/server/create_execute_function.ts +++ b/x-pack/plugins/actions/server/create_execute_function.ts @@ -15,11 +15,7 @@ import { } from './types'; import { ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from './constants/saved_objects'; import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; -import { - ActionExecutionSource, - extractSavedObjectReferences, - isSavedObjectExecutionSource, -} from './lib'; +import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; interface CreateExecuteFunctionOptions { taskManager: TaskManagerStartContract; @@ -29,12 +25,11 @@ interface CreateExecuteFunctionOptions { } export interface ExecuteOptions - extends Pick { + extends Pick { id: string; spaceId: string; apiKey: string | null; executionId: string; - source?: ActionExecutionSource; } interface ActionTaskParams @@ -280,7 +275,7 @@ function validateCanActionBeUsed(action: PreConfiguredAction | RawAction) { } } -function executionSourceAsSavedObjectReferences(executionSource?: ActionExecutionSource) { +function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorOptions['source']) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts index c16b5e4d0b1a7..b1ad90d9093eb 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -13,11 +13,7 @@ import { } from './types'; import { ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from './constants/saved_objects'; import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; -import { - extractSavedObjectReferences, - isSavedObjectExecutionSource, - ActionExecutionSource, -} from './lib'; +import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; // This allowlist should only contain connector types that don't require API keys for // execution. @@ -29,9 +25,8 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { } export interface ExecuteOptions - extends Pick { + extends Pick { id: string; - source?: ActionExecutionSource; } interface ActionTaskParams @@ -140,7 +135,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ }; } -function executionSourceAsSavedObjectReferences(executionSource?: ActionExecutionSource) { +function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorOptions['source']) { return isSavedObjectExecutionSource(executionSource) ? { references: [ diff --git a/x-pack/plugins/actions/server/lib/action_execution_source.ts b/x-pack/plugins/actions/server/lib/action_execution_source.ts index 1589b889bb84b..9701b759f981e 100644 --- a/x-pack/plugins/actions/server/lib/action_execution_source.ts +++ b/x-pack/plugins/actions/server/lib/action_execution_source.ts @@ -33,6 +33,13 @@ export function asHttpRequestExecutionSource(source: KibanaRequest): HttpRequest }; } +export function asEmptySource(type: ActionExecutionSourceType): ActionExecutionSource<{}> { + return { + type, + source: {}, + }; +} + export function asSavedObjectExecutionSource( source: Omit ): SavedObjectExecutionSource { diff --git a/x-pack/plugins/actions/server/lib/action_executor.test.ts b/x-pack/plugins/actions/server/lib/action_executor.test.ts index 35483d75d036a..a6acfc25a923a 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -10,12 +10,15 @@ import { schema } from '@kbn/config-schema'; import { ActionExecutor } from './action_executor'; import { actionTypeRegistryMock } from '../action_type_registry.mock'; import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks'; -import { loggingSystemMock } from '@kbn/core/server/mocks'; +import { httpServerMock, loggingSystemMock } from '@kbn/core/server/mocks'; import { eventLoggerMock } from '@kbn/event-log-plugin/server/mocks'; import { spacesServiceMock } from '@kbn/spaces-plugin/server/spaces_service/spaces_service.mock'; import { ActionType } from '../types'; import { actionsMock } from '../mocks'; -import { ActionExecutionSourceType } from './action_execution_source'; +import { + asHttpRequestExecutionSource, + asSavedObjectExecutionSource, +} from './action_execution_source'; const actionExecutor = new ActionExecutor({ isESOCanEncrypt: true }); const services = actionsMock.createServices(); @@ -201,7 +204,7 @@ test('successfully executes', async () => { `); }); -test('successfully executes when sourceType is specified', async () => { +test('successfully executes when http_request source is specified', async () => { const actionType: jest.Mocked = { id: 'test', name: 'Test', @@ -228,7 +231,7 @@ test('successfully executes when sourceType is specified', async () => { actionTypeRegistry.get.mockReturnValueOnce(actionType); await actionExecutor.execute({ ...executeParams, - sourceType: ActionExecutionSourceType.HTTP_REQUEST, + source: asHttpRequestExecutionSource(httpServerMock.createKibanaRequest()), }); expect(encryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledWith( @@ -267,7 +270,7 @@ test('successfully executes when sourceType is specified', async () => { "kibana": Object { "action": Object { "execution": Object { - "source": "HTTP_REQUEST", + "source": "http_request", "uuid": "2", }, "id": "1", @@ -306,7 +309,148 @@ test('successfully executes when sourceType is specified', async () => { "kibana": Object { "action": Object { "execution": Object { - "source": "HTTP_REQUEST", + "source": "http_request", + "uuid": "2", + }, + "id": "1", + "name": "1", + }, + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "1", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action executed: test:1: 1", + }, + ], + ] + `); +}); + +test('successfully executes when saved_object source is specified', async () => { + const actionType: jest.Mocked = { + id: 'test', + name: 'Test', + minimumLicenseRequired: 'basic', + supportedFeatureIds: ['alerting'], + executor: jest.fn(), + }; + const actionSavedObject = { + id: '1', + type: 'action', + attributes: { + name: '1', + actionTypeId: 'test', + config: { + bar: true, + }, + secrets: { + baz: true, + }, + }, + references: [], + }; + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(actionSavedObject); + actionTypeRegistry.get.mockReturnValueOnce(actionType); + await actionExecutor.execute({ + ...executeParams, + source: asSavedObjectExecutionSource({ + id: '573891ae-8c48-49cb-a197-0cd5ec34a88b', + type: 'alert', + }), + }); + + expect(encryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledWith( + 'action', + '1', + { namespace: 'some-namespace' } + ); + + expect(actionTypeRegistry.get).toHaveBeenCalledWith('test'); + expect(actionTypeRegistry.isActionExecutable).toHaveBeenCalledWith('1', 'test', { + notifyUsage: true, + }); + + expect(actionType.executor).toHaveBeenCalledWith({ + actionId: '1', + services: expect.anything(), + config: { + bar: true, + }, + secrets: { + baz: true, + }, + params: { foo: true }, + logger: loggerMock, + }); + + expect(loggerMock.debug).toBeCalledWith('executing action test:1: 1'); + expect(eventLogger.logEvent.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "event": Object { + "action": "execute-start", + "kind": "action", + }, + "kibana": Object { + "action": Object { + "execution": Object { + "source": "alert", + "uuid": "2", + }, + "id": "1", + "name": "1", + }, + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "1", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action started: test:1: 1", + }, + ], + Array [ + Object { + "event": Object { + "action": "execute", + "kind": "action", + "outcome": "success", + }, + "kibana": Object { + "action": Object { + "execution": Object { + "source": "alert", "uuid": "2", }, "id": "1", diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index 5379bfd4be432..d9b2f7abc73cb 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -29,7 +29,7 @@ import { ValidatorServices, } from '../types'; import { EVENT_LOG_ACTIONS } from '../constants/event_log'; -import { ActionExecutionSourceType } from './action_execution_source'; +import { ActionExecutionSource } from './action_execution_source'; import { RelatedSavedObjects } from './related_saved_objects'; import { createActionEventLogRecordObject } from './create_action_event_log_record_object'; import { ActionExecutionError, ActionExecutionErrorReason } from './errors/action_execution_error'; @@ -52,13 +52,13 @@ export interface TaskInfo { attempts: number; } -export interface ExecuteOptions { +export interface ExecuteOptions { actionId: string; actionExecutionId: string; isEphemeral?: boolean; request: KibanaRequest; params: Record; - sourceType?: ActionExecutionSourceType; + source?: ActionExecutionSource; taskInfo?: TaskInfo; executionId?: string; consumer?: string; @@ -90,7 +90,7 @@ export class ActionExecutor { actionId, params, request, - sourceType, + source, isEphemeral, taskInfo, executionId, @@ -185,7 +185,7 @@ export class ActionExecutor { name, actionExecutionId, isPreconfigured: this.actionInfo.isPreconfigured, - ...(sourceType ? { source: sourceType } : {}), + ...(source ? { source } : {}), }); eventLogger.startTiming(event); @@ -286,11 +286,11 @@ export class ActionExecutor { ); } - public async logCancellation({ + public async logCancellation({ actionId, request, relatedSavedObjects, - sourceType, + source, executionId, taskInfo, consumer, @@ -302,7 +302,7 @@ export class ActionExecutor { taskInfo?: TaskInfo; executionId?: string; relatedSavedObjects: RelatedSavedObjects; - sourceType?: ActionExecutionSourceType; + source?: ActionExecutionSource; consumer?: string; }) { const { spaces, encryptedSavedObjectsClient, preconfiguredActions, eventLogger } = @@ -350,7 +350,7 @@ export class ActionExecutor { relatedSavedObjects, actionExecutionId, isPreconfigured: this.actionInfo.isPreconfigured, - ...(sourceType ? { source: sourceType } : {}), + ...(source ? { source } : {}), }); eventLogger.logEvent(event); diff --git a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts index c713b60654775..28c3a96e14507 100644 --- a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts +++ b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts @@ -5,7 +5,8 @@ * 2.0. */ -import { ActionExecutionSourceType } from './action_execution_source'; +import { httpServerMock } from '@kbn/core-http-server-mocks'; +import { asHttpRequestExecutionSource } from './action_execution_source'; import { createActionEventLogRecordObject } from './create_action_event_log_record_object'; describe('createActionEventLogRecordObject', () => { @@ -298,7 +299,7 @@ describe('createActionEventLogRecordObject', () => { }); }); - test('created action event "execute" with source', async () => { + test('created action event "execute" with http_request source', async () => { expect( createActionEventLogRecordObject({ actionId: '1', @@ -317,7 +318,7 @@ describe('createActionEventLogRecordObject', () => { }, ], actionExecutionId: '123abc', - source: ActionExecutionSourceType.HTTP_REQUEST, + source: asHttpRequestExecutionSource(httpServerMock.createKibanaRequest()), }) ).toStrictEqual({ event: { @@ -346,7 +347,64 @@ describe('createActionEventLogRecordObject', () => { name: 'test name', id: '1', execution: { - source: 'HTTP_REQUEST', + source: 'http_request', + uuid: '123abc', + }, + }, + }, + message: 'action execution start', + }); + }); + + test('created action event "execute" with saved_object source', async () => { + expect( + createActionEventLogRecordObject({ + actionId: '1', + name: 'test name', + action: 'execute', + message: 'action execution start', + namespace: 'default', + executionId: '123abc', + consumer: 'test-consumer', + savedObjects: [ + { + id: '2', + type: 'action', + typeId: '.email', + relation: 'primary', + }, + ], + actionExecutionId: '123abc', + source: asHttpRequestExecutionSource(httpServerMock.createKibanaRequest()), + }) + ).toStrictEqual({ + event: { + action: 'execute', + kind: 'action', + }, + kibana: { + alert: { + rule: { + consumer: 'test-consumer', + execution: { + uuid: '123abc', + }, + }, + }, + saved_objects: [ + { + id: '2', + namespace: 'default', + rel: 'primary', + type: 'action', + type_id: '.email', + }, + ], + action: { + name: 'test name', + id: '1', + execution: { + source: 'http_request', uuid: '123abc', }, }, diff --git a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts index 61ec6f1518d05..46dcddd9b55c5 100644 --- a/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts +++ b/x-pack/plugins/actions/server/lib/create_action_event_log_record_object.ts @@ -9,7 +9,7 @@ import { set } from '@kbn/safer-lodash-set'; import { isEmpty } from 'lodash'; import { IEvent, SAVED_OBJECT_REL_PRIMARY } from '@kbn/event-log-plugin/server'; import { RelatedSavedObjects } from './related_saved_objects'; -import { ActionExecutionSourceType } from './action_execution_source'; +import { ActionExecutionSource, isSavedObjectExecutionSource } from './action_execution_source'; export type Event = Exclude; @@ -36,7 +36,7 @@ interface CreateActionEventLogRecordParams { }>; relatedSavedObjects?: RelatedSavedObjects; isPreconfigured?: boolean; - source?: ActionExecutionSourceType; + source?: ActionExecutionSource; } export function createActionEventLogRecordObject(params: CreateActionEventLogRecordParams): Event { @@ -91,13 +91,20 @@ export function createActionEventLogRecordObject(params: CreateActionEventLogRec id: actionId, execution: { uuid: actionExecutionId, - ...(source ? { source } : {}), }, }, }, ...(message ? { message } : {}), }; + if (source) { + if (isSavedObjectExecutionSource(source)) { + set(event, 'kibana.action.execution.source', source.source.type); + } else { + set(event, 'kibana.action.execution.source', source.type?.toLowerCase()); + } + } + for (const relatedSavedObject of relatedSavedObjects || []) { const ruleTypeId = relatedSavedObject.type === 'alert' ? relatedSavedObject.typeId : null; if (ruleTypeId) { diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts index 43813173f17c2..7b454a2150322 100644 --- a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts +++ b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts @@ -29,7 +29,7 @@ const executeParamsFields = [ 'executionId', 'request.headers', 'taskInfo', - 'sourceType', + 'source', ]; const spaceIdToNamespace = jest.fn(); const actionTypeRegistry = actionTypeRegistryMock.create(); @@ -288,7 +288,7 @@ test('executes the task by calling the executor with proper parameters when cons ); }); -test('executes the task by calling the executor with proper parameters when source is provided', async () => { +test('executes the task by calling the executor with proper parameters when saved_object source is provided', async () => { const taskRunner = taskRunnerFactory.create({ taskInstance: mockedTaskInstance, }); @@ -306,6 +306,67 @@ test('executes the task by calling the executor with proper parameters when sour source: 'SAVED_OBJECT', apiKey: Buffer.from('123:abc').toString('base64'), }, + references: [{ name: 'source', id: 'abc', type: 'alert' }], + }); + + const runnerResult = await taskRunner.run(); + + expect(runnerResult).toBeUndefined(); + expect(spaceIdToNamespace).toHaveBeenCalledWith('test'); + expect(mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledWith( + 'action_task_params', + '3', + { namespace: 'namespace-test' } + ); + + const [executeParams] = mockedActionExecutor.execute.mock.calls[0]; + expect(pick(executeParams, [...executeParamsFields, 'consumer'])).toEqual({ + actionId: '2', + consumer: 'test-consumer', + isEphemeral: false, + params: { baz: true }, + relatedSavedObjects: [], + executionId: '123abc', + request: { + headers: { + // base64 encoded "123:abc" + authorization: 'ApiKey MTIzOmFiYw==', + }, + }, + source: { + type: 'SAVED_OBJECT', + source: { id: 'abc', type: 'alert' }, + }, + taskInfo: { + scheduled: new Date(), + attempts: 0, + }, + }); + + expect(taskRunnerFactoryInitializerParams.basePathService.set).toHaveBeenCalledWith( + executeParams.request, + '/s/test' + ); +}); + +test('executes the task by calling the executor with proper parameters when notification source is provided', async () => { + const taskRunner = taskRunnerFactory.create({ + taskInstance: mockedTaskInstance, + }); + + mockedActionExecutor.execute.mockResolvedValueOnce({ status: 'ok', actionId: '2' }); + spaceIdToNamespace.mockReturnValueOnce('namespace-test'); + mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ + id: '3', + type: 'action_task_params', + attributes: { + actionId: '2', + consumer: 'test-consumer', + params: { baz: true }, + executionId: '123abc', + source: 'NOTIFICATION', + apiKey: Buffer.from('123:abc').toString('base64'), + }, references: [], }); @@ -333,7 +394,10 @@ test('executes the task by calling the executor with proper parameters when sour authorization: 'ApiKey MTIzOmFiYw==', }, }, - sourceType: 'SAVED_OBJECT', + source: { + type: 'NOTIFICATION', + source: {}, + }, taskInfo: { scheduled: new Date(), attempts: 0, diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.ts index 17e4b851366d5..293c0900e3169 100644 --- a/x-pack/plugins/actions/server/lib/task_runner_factory.ts +++ b/x-pack/plugins/actions/server/lib/task_runner_factory.ts @@ -6,6 +6,7 @@ */ import { v4 as uuidv4 } from 'uuid'; +import { pick } from 'lodash'; import { addSpaceIdToPath } from '@kbn/spaces-plugin/server'; import { Logger, @@ -16,6 +17,7 @@ import { SavedObject, Headers, FakeRawRequest, + SavedObjectReference, } from '@kbn/core/server'; import { RunContext } from '@kbn/task-manager-plugin/server'; import { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin/server'; @@ -30,7 +32,11 @@ import { isPersistedActionTask, } from '../types'; import { ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from '../constants/saved_objects'; -import { ActionExecutionSourceType } from './action_execution_source'; +import { + ActionExecutionSourceType, + asEmptySource, + asSavedObjectExecutionSource, +} from './action_execution_source'; import { RelatedSavedObjects, validatedRelatedSavedObjects } from './related_saved_objects'; import { injectSavedObjectReferences } from './action_task_params_utils'; import { InMemoryMetrics, IN_MEMORY_METRICS } from '../monitoring'; @@ -98,6 +104,7 @@ export class TaskRunnerFactory { source, relatedSavedObjects, }, + references, } = await getActionTaskParams( actionTaskExecutorParams, encryptedSavedObjectsClient, @@ -127,7 +134,7 @@ export class TaskRunnerFactory { consumer, relatedSavedObjects: validatedRelatedSavedObjects(logger, relatedSavedObjects), actionExecutionId, - ...(source ? { sourceType: source as ActionExecutionSourceType } : {}), + ...getSource(references, source), }); } catch (e) { logger.error( @@ -196,6 +203,7 @@ export class TaskRunnerFactory { const { attributes: { actionId, apiKey, executionId, consumer, source, relatedSavedObjects }, + references, } = await getActionTaskParams( actionTaskExecutorParams, encryptedSavedObjectsClient, @@ -213,7 +221,7 @@ export class TaskRunnerFactory { executionId, relatedSavedObjects: (relatedSavedObjects || []) as RelatedSavedObjects, actionExecutionId, - ...(source ? { sourceType: source as ActionExecutionSourceType } : {}), + ...getSource(references, source), }); inMemoryMetrics.increment(IN_MEMORY_METRICS.ACTION_TIMEOUTS); @@ -280,3 +288,12 @@ async function getActionTaskParams( return { attributes: executorParams.taskParams, references: executorParams.references ?? [] }; } } + +function getSource(references: SavedObjectReference[], sourceType?: string) { + const sourceInReferences = references.find((ref) => ref.name === 'source'); + if (sourceInReferences) { + return { source: asSavedObjectExecutionSource(pick(sourceInReferences, 'id', 'type')) }; + } + + return sourceType ? { source: asEmptySource(sourceType as ActionExecutionSourceType) } : {}; +} diff --git a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts index 4f82e435387f9..5900f698045a0 100644 --- a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts +++ b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts @@ -39,12 +39,7 @@ export function initPlugin(router: IRouter, coreSetup: CoreSetup Date: Wed, 1 Mar 2023 21:26:44 +0000 Subject: [PATCH 12/14] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/actions/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/actions/tsconfig.json b/x-pack/plugins/actions/tsconfig.json index 72ba9c7c92e09..77ef11e88bfe3 100644 --- a/x-pack/plugins/actions/tsconfig.json +++ b/x-pack/plugins/actions/tsconfig.json @@ -32,7 +32,8 @@ "@kbn/logging", "@kbn/logging-mocks", "@kbn/core-elasticsearch-client-server-mocks", - "@kbn/safer-lodash-set" + "@kbn/safer-lodash-set", + "@kbn/core-http-server-mocks" ], "exclude": [ "target/**/*", From af17277c088963bdbd51201a49eaf71889a906d3 Mon Sep 17 00:00:00 2001 From: Ying Date: Wed, 8 Mar 2023 10:52:52 -0500 Subject: [PATCH 13/14] Adding dynamic false to action task param mappings --- x-pack/plugins/actions/server/saved_objects/mappings.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x-pack/plugins/actions/server/saved_objects/mappings.json b/x-pack/plugins/actions/server/saved_objects/mappings.json index 184c867a738e8..be0a691f852b2 100644 --- a/x-pack/plugins/actions/server/saved_objects/mappings.json +++ b/x-pack/plugins/actions/server/saved_objects/mappings.json @@ -25,6 +25,7 @@ } }, "action_task_params": { + "dynamic": false, "properties": { "actionId": { "type": "keyword" @@ -45,10 +46,6 @@ "relatedSavedObjects": { "enabled": false, "type": "object" - }, - "source": { - "index": false, - "type": "keyword" } } }, From 6b3609b632d1bd7f90005f3a0b46cc657e8b35b2 Mon Sep 17 00:00:00 2001 From: Ying Date: Wed, 8 Mar 2023 13:23:12 -0500 Subject: [PATCH 14/14] Fixing test --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index d3f1bb72142dc..0285dc8e98ba8 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -56,7 +56,7 @@ describe('checking migration metadata changes on all registered SO types', () => expect(hashMap).toMatchInlineSnapshot(` Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", - "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", + "action_task_params": "5f419caba96dd8c77d0f94013e71d43890e3d5d6", "alert": "785240e3137f5eb1a0f8986e5b8eff99780fc04f", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2",