From 2a93a372ec8571e9c650bab6751af979e3d2274a Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Thu, 13 Oct 2022 09:54:15 -0400 Subject: [PATCH 01/38] Adding unsecured actions client --- .../create_unsecured_execute_function.ts | 160 ++++++++++++++++++ x-pack/plugins/actions/server/plugin.ts | 36 ++++ .../unsecured_actions_client.ts | 44 +++++ ...nsecured_actions_client_access_registry.ts | 24 +++ x-pack/plugins/alerting/server/plugin.ts | 17 ++ 5 files changed, 281 insertions(+) create mode 100644 x-pack/plugins/actions/server/create_unsecured_execute_function.ts create mode 100644 x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts create mode 100644 x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts diff --git a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts new file mode 100644 index 00000000000000..b21f1a04ccb60b --- /dev/null +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -0,0 +1,160 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { compact } from 'lodash'; +import { ISavedObjectsRepository, SavedObjectsBulkResponse } from '@kbn/core/server'; +import { TaskManagerStartContract } from '@kbn/task-manager-plugin/server'; +import { + ActionTypeRegistryContract as ConnectorTypeRegistryContract, + PreConfiguredAction as PreconfiguredConnector, +} 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 { RelatedSavedObjects } from './lib/related_saved_objects'; + +interface CreateBulkUnsecuredExecuteFunctionOptions { + taskManager: TaskManagerStartContract; + isESOCanEncrypt: boolean; + connectorTypeRegistry: ConnectorTypeRegistryContract; + preconfiguredConnectors: PreconfiguredConnector[]; +} + +export interface ExecuteOptions extends Pick { + id: string; + spaceId: string; + apiKey: string | null; + executionId: string; + consumer?: string; + relatedSavedObjects?: RelatedSavedObjects; +} + +export interface ActionTaskParams extends Pick { + actionId: string; + apiKey: string | null; + executionId: string; + consumer?: string; + relatedSavedObjects?: RelatedSavedObjects; +} + +export type BulkUnsecuredExecutionEnqueuer = ( + internalSavedObjectsRepository: ISavedObjectsRepository, + actionsToExectute: ExecuteOptions[] +) => Promise; + +export function createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager, + connectorTypeRegistry, + isESOCanEncrypt, + preconfiguredConnectors, +}: CreateBulkUnsecuredExecuteFunctionOptions): BulkUnsecuredExecutionEnqueuer { + return async function execute( + internalSavedObjectsRepository: ISavedObjectsRepository, + actionsToExecute: ExecuteOptions[] + ) { + if (!isESOCanEncrypt) { + throw new Error( + `Unable to execute actions 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.` + ); + } + + const connectorTypeIds: Record = {}; + const spaceIds: Record = {}; + const connectorIds = [...new Set(actionsToExecute.map((action) => action.id))]; + + const notPreconfiguredConnectors = connectorIds.filter( + (connectorId) => + preconfiguredConnectors.find((connector) => connector.id === connectorId) == null + ); + + if (notPreconfiguredConnectors.length > 0) { + // log warning or throw error? + } + + const connectors: PreconfiguredConnector[] = compact( + connectorIds.map((connectorId) => + preconfiguredConnectors.find((pConnector) => pConnector.id === connectorId) + ) + ); + + connectors.forEach((connector) => { + const { id, actionTypeId } = connector; + if (!connectorTypeRegistry.isActionExecutable(id, actionTypeId, { notifyUsage: true })) { + connectorTypeRegistry.ensureActionTypeEnabled(actionTypeId); + } + + connectorTypeIds[id] = actionTypeId; + }); + + const actions = await Promise.all( + actionsToExecute.map(async (actionToExecute) => { + // Get saved object references from action ID and relatedSavedObjects + const { references, relatedSavedObjectWithRefs } = extractSavedObjectReferences( + actionToExecute.id, + true, + actionToExecute.relatedSavedObjects + ); + const executionSourceReference = executionSourceAsSavedObjectReferences( + actionToExecute.source + ); + + const taskReferences = []; + if (executionSourceReference.references) { + taskReferences.push(...executionSourceReference.references); + } + if (references) { + taskReferences.push(...references); + } + + spaceIds[actionToExecute.id] = actionToExecute.spaceId; + + return { + type: ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, + attributes: { + actionId: actionToExecute.id, + params: actionToExecute.params, + apiKey: actionToExecute.apiKey, + executionId: actionToExecute.executionId, + consumer: actionToExecute.consumer, + relatedSavedObjects: relatedSavedObjectWithRefs, + }, + references: taskReferences, + }; + }) + ); + + const actionTaskParamsRecords: SavedObjectsBulkResponse = + await internalSavedObjectsRepository.bulkCreate(actions); + + const taskInstances = actionTaskParamsRecords.saved_objects.map((so) => { + const actionId = so.attributes.actionId; + return { + taskType: `actions:${connectorTypeIds[actionId]}`, + params: { + spaceId: spaceIds[actionId], + actionTaskParamsId: so.id, + }, + state: {}, + scope: ['actions'], + }; + }); + await taskManager.bulkSchedule(taskInstances); + }; +} + +function executionSourceAsSavedObjectReferences(executionSource: ActionExecutorOptions['source']) { + return isSavedObjectExecutionSource(executionSource) + ? { + references: [ + { + name: 'source', + ...executionSource.source, + }, + ], + } + : {}; +} diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index fa70e0dc713545..319bd9c0ffce60 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/x-pack/plugins/actions/server/plugin.ts @@ -101,6 +101,9 @@ import { createSubActionConnectorFramework } from './sub_action_framework'; import { IServiceAbstract, SubActionConnectorType } from './sub_action_framework/types'; import { SubActionConnector } from './sub_action_framework/sub_action_connector'; import { CaseConnector } from './sub_action_framework/case'; +import { UnsecuredActionsClientAccessRegistry } from './unsecured_actions_client/unsecured_actions_client_access_registry'; +import { UnsecuredActionsClient } from './unsecured_actions_client/unsecured_actions_client'; +import { createBulkUnsecuredExecutionEnqueuerFunction } from './create_unsecured_execute_function'; export interface PluginSetupContract { registerType< @@ -117,6 +120,7 @@ export interface PluginSetupContract { >( connector: SubActionConnectorType ): void; + registerUnsecuredActionsClientAccess(featureId: string): void; isPreconfiguredConnector(connectorId: string): boolean; getSubActionConnectorClass: () => IServiceAbstract; getCaseConnectorClass: () => IServiceAbstract; @@ -138,6 +142,8 @@ export interface PluginStartContract { preconfiguredActions: PreConfiguredAction[]; + getUnsecuredActionsClient(): Promise>; + renderActionParameterTemplates( actionTypeId: string, actionId: string, @@ -188,6 +194,7 @@ export class ActionsPlugin implements Plugin { subActionFramework.registerConnector(connector); }, + registerUnsecuredActionsClientAccess: (featureId: string) => { + this.unsecuredActionsClientAccessRegistry?.register(featureId); + }, isPreconfiguredConnector: (connectorId: string): boolean => { return !!this.preconfiguredActions.find( (preconfigured) => preconfigured.id === connectorId @@ -452,6 +464,29 @@ export class ActionsPlugin implements Plugin { + if (isESOCanEncrypt !== true) { + throw new Error( + `Unable to create unsecured actions client 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.` + ); + } + + const internalSavedObjectsRepository = core.savedObjects.createInternalRepository([ + ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, + ]); + + return new UnsecuredActionsClient({ + internalSavedObjectsRepository, + unsecuredActionsClientAccessRegistry: this.unsecuredActionsClientAccessRegistry!, + executionEnqueuer: createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: plugins.taskManager, + connectorTypeRegistry: actionTypeRegistry!, + isESOCanEncrypt: isESOCanEncrypt!, + preconfiguredConnectors: preconfiguredActions, + }), + }); + }; + // Ensure the public API cannot be used to circumvent authorization // using our legacy exemption mechanism by passing in a legacy SO // as authorizationContext which would then set a Legacy AuthorizationMode @@ -532,6 +567,7 @@ export class ActionsPlugin implements Plugin renderActionParameterTemplates(actionTypeRegistry, ...args), 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 new file mode 100644 index 00000000000000..7ff0c3d72c8e71 --- /dev/null +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { ISavedObjectsRepository } from '@kbn/core/server'; +import { UnsecuredActionsClientAccessRegistry } from './unsecured_actions_client_access_registry'; +import { + BulkUnsecuredExecutionEnqueuer, + ExecuteOptions, +} from '../create_unsecured_execute_function'; + +export interface UnsecuredActionsClientOpts { + unsecuredActionsClientAccessRegistry: UnsecuredActionsClientAccessRegistry; + internalSavedObjectsRepository: ISavedObjectsRepository; + executionEnqueuer: BulkUnsecuredExecutionEnqueuer; +} + +export class UnsecuredActionsClient { + private readonly unsecuredActionsClientAccessRegistry: UnsecuredActionsClientAccessRegistry; + private readonly internalSavedObjectsRepository: ISavedObjectsRepository; + private readonly executionEnqueuer: BulkUnsecuredExecutionEnqueuer; + + constructor(params: UnsecuredActionsClientOpts) { + this.unsecuredActionsClientAccessRegistry = params.unsecuredActionsClientAccessRegistry; + this.executionEnqueuer = params.executionEnqueuer; + this.internalSavedObjectsRepository = params.internalSavedObjectsRepository; + } + + public async bulkEnqueueExecution( + requesterId: string, + actionsToExecute: ExecuteOptions[] + ): Promise { + // Check that requesterId is allowed + if (!this.unsecuredActionsClientAccessRegistry.has(requesterId)) { + throw new Error( + `${requesterId} feature is not registered for UnsecuredActionsClient access.` + ); + } + return this.executionEnqueuer(this.internalSavedObjectsRepository, actionsToExecute); + } +} diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts new file mode 100644 index 00000000000000..39975a9b7fb252 --- /dev/null +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts @@ -0,0 +1,24 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export class UnsecuredActionsClientAccessRegistry { + private readonly allowedFeatureIds: Map = new Map(); + + /** + * Returns if the access registry has the given feature id registered + */ + public has(id: string) { + return this.allowedFeatureIds.has(id); + } + + /** + * Registers feature id to the access registry + */ + public register(id: string) { + this.allowedFeatureIds.set(id, true); + } +} diff --git a/x-pack/plugins/alerting/server/plugin.ts b/x-pack/plugins/alerting/server/plugin.ts index 7450dcb1a45d02..e437e9b7d0e659 100644 --- a/x-pack/plugins/alerting/server/plugin.ts +++ b/x-pack/plugins/alerting/server/plugin.ts @@ -219,6 +219,8 @@ export class AlertingPlugin { this.eventLogService = plugins.eventLog; plugins.eventLog.registerProviderActions(EVENT_LOG_PROVIDER, Object.values(EVENT_LOG_ACTIONS)); + plugins.actions.registerUnsecuredActionsClientAccess('alerting'); + const ruleTypeRegistry = new RuleTypeRegistry({ logger: this.logger, taskManager: plugins.taskManager, @@ -462,6 +464,21 @@ export class AlertingPlugin { scheduleAlertingHealthCheck(this.logger, this.config, plugins.taskManager); scheduleApiKeyInvalidatorTask(this.telemetryLogger, this.config, plugins.taskManager); + plugins.actions.getUnsecuredActionsClient().then((unsecuredActionsClient) => { + unsecuredActionsClient.bulkEnqueueExecution('alerting', [ + { + id: 'gmail', + params: { + to: ['xxxxxx'], + subject: 'hello from Kibana!', + message: 'does this work??', + }, + spaceId: 'default', + apiKey: null, + executionId: 'abc', + }, + ]); + }); return { listTypes: ruleTypeRegistry!.list.bind(this.ruleTypeRegistry!), getAlertingAuthorizationWithRequest, From 2fe5ce579c72935b0c2a57228292407e3e5c9050 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Thu, 13 Oct 2022 11:46:08 -0400 Subject: [PATCH 02/38] Removing isESOCanEncrypt check --- .../create_unsecured_execute_function.ts | 30 +++++++++---------- .../actions/server/lib/action_executor.ts | 15 ++++++---- x-pack/plugins/actions/server/plugin.ts | 7 ----- x-pack/plugins/alerting/server/plugin.ts | 2 -- 4 files changed, 23 insertions(+), 31 deletions(-) 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 b21f1a04ccb60b..acc4d01e7c1a2b 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -17,17 +17,15 @@ import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; import { RelatedSavedObjects } from './lib/related_saved_objects'; +const ALLOWED_CONNECTOR_TYPE_IDS = ['.email', '.slack']; interface CreateBulkUnsecuredExecuteFunctionOptions { taskManager: TaskManagerStartContract; - isESOCanEncrypt: boolean; connectorTypeRegistry: ConnectorTypeRegistryContract; preconfiguredConnectors: PreconfiguredConnector[]; } export interface ExecuteOptions extends Pick { id: string; - spaceId: string; - apiKey: string | null; executionId: string; consumer?: string; relatedSavedObjects?: RelatedSavedObjects; @@ -49,21 +47,13 @@ export type BulkUnsecuredExecutionEnqueuer = ( export function createBulkUnsecuredExecutionEnqueuerFunction({ taskManager, connectorTypeRegistry, - isESOCanEncrypt, preconfiguredConnectors, }: CreateBulkUnsecuredExecuteFunctionOptions): BulkUnsecuredExecutionEnqueuer { return async function execute( internalSavedObjectsRepository: ISavedObjectsRepository, actionsToExecute: ExecuteOptions[] ) { - if (!isESOCanEncrypt) { - throw new Error( - `Unable to execute actions 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.` - ); - } - const connectorTypeIds: Record = {}; - const spaceIds: Record = {}; const connectorIds = [...new Set(actionsToExecute.map((action) => action.id))]; const notPreconfiguredConnectors = connectorIds.filter( @@ -72,7 +62,11 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ ); if (notPreconfiguredConnectors.length > 0) { - // log warning or throw error? + throw new Error( + `${notPreconfiguredConnectors.join( + ',' + )} are not preconfigured connectors and can't be scheduled for unsecured actions execution` + ); } const connectors: PreconfiguredConnector[] = compact( @@ -87,6 +81,12 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ connectorTypeRegistry.ensureActionTypeEnabled(actionTypeId); } + if (!ALLOWED_CONNECTOR_TYPE_IDS.includes(actionTypeId)) { + throw new Error( + `${actionTypeId} actions cannot be scheduled for unsecured actions execution` + ); + } + connectorTypeIds[id] = actionTypeId; }); @@ -110,14 +110,12 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ taskReferences.push(...references); } - spaceIds[actionToExecute.id] = actionToExecute.spaceId; - return { type: ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, attributes: { actionId: actionToExecute.id, params: actionToExecute.params, - apiKey: actionToExecute.apiKey, + apiKey: null, executionId: actionToExecute.executionId, consumer: actionToExecute.consumer, relatedSavedObjects: relatedSavedObjectWithRefs, @@ -135,7 +133,7 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ return { taskType: `actions:${connectorTypeIds[actionId]}`, params: { - spaceId: spaceIds[actionId], + spaceId: 'default', actionTaskParamsId: so.id, }, state: {}, diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index 00a3980833ef2d..f778b8aabfd415 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -105,12 +105,6 @@ export class ActionExecutor { throw new Error('ActionExecutor not initialized'); } - if (!this.isESOCanEncrypt) { - throw new Error( - `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.` - ); - } - return withSpan( { name: `execute_action`, @@ -137,6 +131,7 @@ export class ActionExecutor { const actionInfo = await getActionInfoInternal( await getActionsClientWithRequest(request, source), + this.isESOCanEncrypt, encryptedSavedObjectsClient, preconfiguredActions, actionId, @@ -319,6 +314,7 @@ export class ActionExecutor { if (!this.actionInfo || this.actionInfo.actionId !== actionId) { this.actionInfo = await getActionInfoInternal( await getActionsClientWithRequest(request, source), + this.isESOCanEncrypt, encryptedSavedObjectsClient, preconfiguredActions, actionId, @@ -370,6 +366,7 @@ interface ActionInfo { async function getActionInfoInternal( actionsClient: PublicMethodsOf, + isESOCanEncrypt: boolean, encryptedSavedObjectsClient: EncryptedSavedObjectsClient, preconfiguredActions: PreConfiguredAction[], actionId: string, @@ -389,6 +386,12 @@ async function getActionInfoInternal( }; } + if (!isESOCanEncrypt) { + throw new Error( + `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.` + ); + } + // 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 }); diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index 319bd9c0ffce60..e5eb5f8dcaa8d0 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/x-pack/plugins/actions/server/plugin.ts @@ -465,12 +465,6 @@ export class ActionsPlugin implements Plugin { - if (isESOCanEncrypt !== true) { - throw new Error( - `Unable to create unsecured actions client 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.` - ); - } - const internalSavedObjectsRepository = core.savedObjects.createInternalRepository([ ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, ]); @@ -481,7 +475,6 @@ export class ActionsPlugin implements Plugin Date: Thu, 13 Oct 2022 12:11:29 -0400 Subject: [PATCH 03/38] Only getting actions client when needed in executor --- .../actions/server/lib/action_executor.ts | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index f778b8aabfd415..f230920cfeea0f 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -130,12 +130,14 @@ export class ActionExecutor { const namespace = spaceId && spaceId !== 'default' ? { namespace: spaceId } : {}; const actionInfo = await getActionInfoInternal( - await getActionsClientWithRequest(request, source), + getActionsClientWithRequest, + request, this.isESOCanEncrypt, encryptedSavedObjectsClient, preconfiguredActions, actionId, - namespace.namespace + namespace.namespace, + source ); const { actionTypeId, name, config, secrets } = actionInfo; @@ -313,12 +315,14 @@ export class ActionExecutor { const namespace = spaceId && spaceId !== 'default' ? { namespace: spaceId } : {}; if (!this.actionInfo || this.actionInfo.actionId !== actionId) { this.actionInfo = await getActionInfoInternal( - await getActionsClientWithRequest(request, source), + getActionsClientWithRequest, + request, this.isESOCanEncrypt, encryptedSavedObjectsClient, preconfiguredActions, actionId, - namespace.namespace + namespace.namespace, + source ); } const task = taskInfo @@ -364,13 +368,18 @@ interface ActionInfo { actionId: string; } -async function getActionInfoInternal( - actionsClient: PublicMethodsOf, +async function getActionInfoInternal( + getActionsClientWithRequest: ( + request: KibanaRequest, + authorizationContext?: ActionExecutionSource + ) => Promise>, + request: KibanaRequest, isESOCanEncrypt: boolean, encryptedSavedObjectsClient: EncryptedSavedObjectsClient, preconfiguredActions: PreConfiguredAction[], actionId: string, - namespace: string | undefined + namespace: string | undefined, + source?: ActionExecutionSource ): Promise { // check to see if it's a pre-configured action first const pcAction = preconfiguredActions.find( @@ -392,6 +401,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 }); From c3ce0b19cf8bb31ce8fc81cf4df54814c035172b Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 13 Oct 2022 18:18:51 +0200 Subject: [PATCH 04/38] First draft --- tsconfig.base.json | 2 + x-pack/plugins/notifications/README.md | 73 +++++++++++++++ x-pack/plugins/notifications/common/index.ts | 8 ++ .../plugins/notifications/common/lib/index.ts | 9 ++ .../notifications/common/lib/promise_utils.ts | 25 ++++++ x-pack/plugins/notifications/kibana.json | 12 +++ .../notifications/server/config/config.ts | 27 ++++++ .../notifications/server/config/index.ts | 8 ++ x-pack/plugins/notifications/server/index.ts | 18 ++++ x-pack/plugins/notifications/server/plugin.ts | 89 +++++++++++++++++++ .../notifications/server/services/email.ts | 31 +++++++ .../server/services/email_service.ts | 16 ++++ .../notifications/server/services/index.ts | 9 ++ x-pack/plugins/notifications/server/types.ts | 28 ++++++ 14 files changed, 355 insertions(+) create mode 100755 x-pack/plugins/notifications/README.md create mode 100644 x-pack/plugins/notifications/common/index.ts create mode 100755 x-pack/plugins/notifications/common/lib/index.ts create mode 100755 x-pack/plugins/notifications/common/lib/promise_utils.ts create mode 100755 x-pack/plugins/notifications/kibana.json create mode 100644 x-pack/plugins/notifications/server/config/config.ts create mode 100644 x-pack/plugins/notifications/server/config/index.ts create mode 100755 x-pack/plugins/notifications/server/index.ts create mode 100755 x-pack/plugins/notifications/server/plugin.ts create mode 100755 x-pack/plugins/notifications/server/services/email.ts create mode 100755 x-pack/plugins/notifications/server/services/email_service.ts create mode 100644 x-pack/plugins/notifications/server/services/index.ts create mode 100755 x-pack/plugins/notifications/server/types.ts diff --git a/tsconfig.base.json b/tsconfig.base.json index 3054a36f2bb86c..379e019d958d8a 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -391,6 +391,8 @@ "@kbn/monitoring-collection-plugin/*": ["x-pack/plugins/monitoring_collection/*"], "@kbn/monitoring-plugin": ["x-pack/plugins/monitoring"], "@kbn/monitoring-plugin/*": ["x-pack/plugins/monitoring/*"], + "@kbn/notifications-plugin": ["x-pack/plugins/notifications"], + "@kbn/notifications-plugin/*": ["x-pack/plugins/notifications/*"], "@kbn/observability-plugin": ["x-pack/plugins/observability"], "@kbn/observability-plugin/*": ["x-pack/plugins/observability/*"], "@kbn/osquery-plugin": ["x-pack/plugins/osquery"], diff --git a/x-pack/plugins/notifications/README.md b/x-pack/plugins/notifications/README.md new file mode 100755 index 00000000000000..0a6a28b2e35016 --- /dev/null +++ b/x-pack/plugins/notifications/README.md @@ -0,0 +1,73 @@ +# Kibana Notifications Plugin + +The Notifications plugin provides a set of services to help Solutions and plugins send notifications to users. + +## Notifications Plugin public API + +### Setup + +The `setup` function exposes the following interface: + +- `email: Promise`: + A service that must be obtained asynchronously, and can be used to send plain text emails. + +### Start + +The `start` function exposes the following interface: + +- `email: Promise`: + A service that must be obtained asynchronously, and can be used to send plain text emails. + + +### Usage + +To use the exposed plugin start and setup contracts: + +1. Make sure `notifications` is in your `optionalPlugins` in the `kibana.json` file: + +```json5 +// /kibana.json +{ +"id": "...", +"optionalPlugins": ["notifications"] +} +``` + +2. Use the exposed contracts: + +```ts +// /server/plugin.ts +import { NotificationsPluginStart } from '../notifications/server`; + +interface MyPluginStartDeps { + notifications?: NotificationsPluginStart; +} + +class MyPlugin { + public async start( + core: CoreStart, + { notifications }: MyPluginStartDeps + ) { + const emailService = await notifications?.email; + emailService.sendPlainTextEmail({ + to: 'foo@bar.com', + subject: 'Some subject', + message: 'Hello world!', + }); + ... + } +} +``` + +### Requirements + +- This plugin currently depends on the `'actions'` plugin, as it uses `Connectors` under the hood. Please make sure the `'actions'` plugin is included in your deployment. +- Note also that for each notification channel the corresponding connector must be preconfigured. E.g. to enable email notifications, an `Email` connector must exist in the system. +- Once the appropriate connectors are preconfigured in `kibana.yaml`, you can configure the `'notifications'` plugin by adding: + + ```yaml + notifications: + connectors: + default: + email: elastic-cloud-email # The identifier of the configured connector + ``` diff --git a/x-pack/plugins/notifications/common/index.ts b/x-pack/plugins/notifications/common/index.ts new file mode 100644 index 00000000000000..bc315c3c9e028f --- /dev/null +++ b/x-pack/plugins/notifications/common/index.ts @@ -0,0 +1,8 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export const PLUGIN_ID = 'notifications'; diff --git a/x-pack/plugins/notifications/common/lib/index.ts b/x-pack/plugins/notifications/common/lib/index.ts new file mode 100755 index 00000000000000..5689f2629dfe32 --- /dev/null +++ b/x-pack/plugins/notifications/common/lib/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export type { ResolvablePromise } from './promise_utils'; +export { getResolvablePromise } from './promise_utils'; diff --git a/x-pack/plugins/notifications/common/lib/promise_utils.ts b/x-pack/plugins/notifications/common/lib/promise_utils.ts new file mode 100755 index 00000000000000..035bba091a05bf --- /dev/null +++ b/x-pack/plugins/notifications/common/lib/promise_utils.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export type ResolvablePromise = Promise & { + doResolve: (value: unknown) => void; + doReject: (reason?: any) => void; +}; + +export function getResolvablePromise(): ResolvablePromise { + const resolvablePromise: Partial> = {}; + + Object.assign( + resolvablePromise, + new Promise((resolve, reject) => { + resolvablePromise.doResolve = resolve; + resolvablePromise.doReject = reject; + }) + ); + + return resolvablePromise as ResolvablePromise; +} diff --git a/x-pack/plugins/notifications/kibana.json b/x-pack/plugins/notifications/kibana.json new file mode 100755 index 00000000000000..04720bf3f481d1 --- /dev/null +++ b/x-pack/plugins/notifications/kibana.json @@ -0,0 +1,12 @@ +{ + "id": "notifications", + "owner": { + "name": "App Services", + "githubTeam": "kibana-app-services" + }, + "version": "kibana", + "server": true, + "ui": false, + "requiredPlugins": ["actions"], + "optionalPlugins": [] +} diff --git a/x-pack/plugins/notifications/server/config/config.ts b/x-pack/plugins/notifications/server/config/config.ts new file mode 100644 index 00000000000000..2abeb8bc1fa88d --- /dev/null +++ b/x-pack/plugins/notifications/server/config/config.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { schema, TypeOf } from '@kbn/config-schema'; +import { PluginConfigDescriptor } from '@kbn/core/server'; + +const configSchema = schema.object({ + connectors: schema.maybe( + schema.object({ + default: schema.maybe( + schema.object({ + email: schema.maybe(schema.string()), + }) + ), + }) + ), +}); + +export type NotificationsConfigType = TypeOf; + +export const config: PluginConfigDescriptor = { + schema: configSchema, +}; diff --git a/x-pack/plugins/notifications/server/config/index.ts b/x-pack/plugins/notifications/server/config/index.ts new file mode 100644 index 00000000000000..39da9b5b8842ec --- /dev/null +++ b/x-pack/plugins/notifications/server/config/index.ts @@ -0,0 +1,8 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { config, type NotificationsConfigType } from './config'; diff --git a/x-pack/plugins/notifications/server/index.ts b/x-pack/plugins/notifications/server/index.ts new file mode 100755 index 00000000000000..a130383a7b9bdb --- /dev/null +++ b/x-pack/plugins/notifications/server/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { PluginInitializerContext } from '@kbn/core/server'; +import { NotificationsPlugin } from './plugin'; + +// This exports static code and TypeScript types, +// as well as, Kibana Platform `plugin()` initializer. + +export function plugin(initializerContext: PluginInitializerContext) { + return new NotificationsPlugin(initializerContext); +} + +export { NotificationsPluginSetup, NotificationsPluginStart } from './types'; diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts new file mode 100755 index 00000000000000..03abe9280e9d33 --- /dev/null +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -0,0 +1,89 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { + PluginInitializerContext, + CoreSetup, + CoreStart, + Plugin, + Logger, +} from '@kbn/core/server'; +import type { + NotificationsPluginSetupDeps, + NotificationsPluginStartDeps, + NotificationsPluginSetup, + NotificationsPluginStart, +} from './types'; +import { EmailService } from './services'; +import { NotificationsConfigType } from './config'; +import { getResolvablePromise, type ResolvablePromise } from '../common/lib'; +import { PLUGIN_ID } from '../common'; + +export class NotificationsPlugin + implements Plugin +{ + private readonly logger: Logger; + private readonly initialConfig: NotificationsConfigType; + private email: ResolvablePromise; + private emailConnector: string; + + constructor(initializerContext: PluginInitializerContext) { + this.logger = initializerContext.logger.get(); + this.email = getResolvablePromise(); + this.initialConfig = initializerContext.config.get(); + } + + public setup(core: CoreSetup, plugins: NotificationsPluginSetupDeps) { + // TODO this must be defaulted to 'elastic-cloud-email' for cloud + if (!plugins.actions) { + this.email.doReject(`Error starting notification services: 'actions' plugin not available.`); + return { email: this.email }; + } + + const emailConnector = this.initialConfig.connectors?.default?.email; + if (!emailConnector) { + this.email.doReject('Error starting notification services: Email connector not specified'); + return { email: this.email }; + } + + if (!plugins.actions.isPreconfiguredConnector(emailConnector)) { + this.email.doReject( + `Error starting notification services: Unexisting email connector '${emailConnector}' specified` + ); + return { email: this.email }; + } + + plugins.actions.registerUnsecuredActionsClientAccess(PLUGIN_ID); + + this.emailConnector = emailConnector; + + return { + email: this.email, + }; + } + + public start(core: CoreStart, plugins: NotificationsPluginStartDeps) { + if (this.emailConnector) { + plugins.actions.getUnsecuredActionsClient().then( + (actionsClient) => { + const email = new EmailService(PLUGIN_ID, this.emailConnector, actionsClient); + this.email.doResolve(email); + }, + (error) => { + this.logger.warn(`Error starting notification services: ${error}`); + this.email.doReject(error); + } + ); + } + + return { + email: this.email, + }; + } + + public stop() {} +} diff --git a/x-pack/plugins/notifications/server/services/email.ts b/x-pack/plugins/notifications/server/services/email.ts new file mode 100755 index 00000000000000..b6cb05b4d368db --- /dev/null +++ b/x-pack/plugins/notifications/server/services/email.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { UnsecuredActionsClient } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client'; +import type { IEmailService, PlainTextEmail } from './email_service'; + +export class EmailService implements IEmailService { + constructor( + private requesterId: string, + private connectorId: string, + private actionsClient: UnsecuredActionsClient + ) {} + + async sendPlainTextEmail(params: PlainTextEmail): Promise { + const actions = params.to.map((to) => ({ + id: this.connectorId, + spaceId: 'default', // TODO should be space agnostic? + apiKey: null, // not needed for Email connector + executionId: '???', + params: { + ...params, + to, + }, + })); + return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); + } +} diff --git a/x-pack/plugins/notifications/server/services/email_service.ts b/x-pack/plugins/notifications/server/services/email_service.ts new file mode 100755 index 00000000000000..9b0dcf81db2899 --- /dev/null +++ b/x-pack/plugins/notifications/server/services/email_service.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export interface IEmailService { + sendPlainTextEmail(payload: PlainTextEmail): Promise; +} + +export interface PlainTextEmail extends Record { + to: string[]; + subject: string; + message: string; +} diff --git a/x-pack/plugins/notifications/server/services/index.ts b/x-pack/plugins/notifications/server/services/index.ts new file mode 100644 index 00000000000000..c6e225e6661039 --- /dev/null +++ b/x-pack/plugins/notifications/server/services/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export type { IEmailService, PlainTextEmail } from './email_service'; +export { EmailService } from './email'; diff --git a/x-pack/plugins/notifications/server/types.ts b/x-pack/plugins/notifications/server/types.ts new file mode 100755 index 00000000000000..c33ffe57119e6b --- /dev/null +++ b/x-pack/plugins/notifications/server/types.ts @@ -0,0 +1,28 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { + PluginSetupContract as ActionsPluginSetupContract, + PluginStartContract as ActionsPluginStartContract, +} from '@kbn/actions-plugin/server'; +import type { IEmailService } from './services/email_service'; + +export interface NotificationsPluginSetupDeps { + actions: ActionsPluginSetupContract; +} + +export interface NotificationsPluginStartDeps { + actions: ActionsPluginStartContract; +} + +export interface NotificationsPluginSetup { + email: Promise; +} + +export interface NotificationsPluginStart { + email: Promise; +} From 00c611d9c15ef68a905b6a47ec0b6d0193df1263 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Fri, 14 Oct 2022 09:57:41 -0400 Subject: [PATCH 05/38] Changing to feature id allowlist. Adding unit tests --- .../create_unsecured_execute_function.test.ts | 499 ++++++++++++++++++ .../create_unsecured_execute_function.ts | 3 - .../server/lib/action_executor.test.ts | 241 ++++++++- x-pack/plugins/actions/server/mocks.ts | 2 + x-pack/plugins/actions/server/plugin.ts | 9 - .../unsecured_actions_client.mock.ts | 25 + .../unsecured_actions_client.test.ts | 64 +++ .../unsecured_actions_client.ts | 12 +- ...nsecured_actions_client_access_registry.ts | 24 - 9 files changed, 836 insertions(+), 43 deletions(-) create mode 100644 x-pack/plugins/actions/server/create_unsecured_execute_function.test.ts create mode 100644 x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.mock.ts create mode 100644 x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts delete mode 100644 x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts 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 new file mode 100644 index 00000000000000..d4d52b3d1236b6 --- /dev/null +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.test.ts @@ -0,0 +1,499 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import uuid from 'uuid'; +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'; + +const mockTaskManager = taskManagerMock.createStart(); +const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); + +beforeEach(() => jest.resetAllMocks()); + +describe('bulkExecute()', () => { + test('schedules the actions with all given parameters with a preconfigured connector', async () => { + const executeFn = createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: mockTaskManager, + connectorTypeRegistry: actionTypeRegistryMock.create(), + preconfiguredConnectors: [ + { + id: '123', + actionTypeId: '.email', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + ], + }); + + internalSavedObjectsRepository.bulkCreate.mockResolvedValueOnce({ + saved_objects: [ + { + id: '234', + type: 'action_task_params', + attributes: { + actionId: '123', + }, + references: [], + }, + { + id: '345', + type: 'action_task_params', + attributes: { + actionId: '123', + }, + references: [], + }, + ], + }); + await executeFn(internalSavedObjectsRepository, [ + { + id: '123', + params: { baz: false }, + executionId: '123abc', + }, + { + id: '123', + params: { baz: true }, + executionId: '234xyz', + }, + ]); + expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); + expect(mockTaskManager.bulkSchedule.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "params": Object { + "actionTaskParamsId": "234", + "spaceId": "default", + }, + "scope": Array [ + "actions", + ], + "state": Object {}, + "taskType": "actions:.email", + }, + Object { + "params": Object { + "actionTaskParamsId": "345", + "spaceId": "default", + }, + "scope": Array [ + "actions", + ], + "state": Object {}, + "taskType": "actions:.email", + }, + ], + ] + `); + + expect(internalSavedObjectsRepository.bulkCreate).toHaveBeenCalledWith([ + { + type: 'action_task_params', + attributes: { + actionId: '123', + params: { baz: false }, + executionId: '123abc', + apiKey: null, + }, + references: [], + }, + { + type: 'action_task_params', + attributes: { + actionId: '123', + params: { baz: true }, + executionId: '234xyz', + apiKey: null, + }, + references: [], + }, + ]); + }); + + test('schedules the actions with all given parameters with a preconfigured connector and source specified', async () => { + const sourceUuid = uuid.v4(); + const source = { type: 'alert', id: sourceUuid }; + const executeFn = createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: mockTaskManager, + connectorTypeRegistry: actionTypeRegistryMock.create(), + preconfiguredConnectors: [ + { + id: '123', + actionTypeId: '.email', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + ], + }); + + internalSavedObjectsRepository.bulkCreate.mockResolvedValueOnce({ + saved_objects: [ + { + id: '234', + type: 'action_task_params', + attributes: { + actionId: '123', + }, + references: [ + { + id: sourceUuid, + name: 'source', + type: 'alert', + }, + ], + }, + { + id: '345', + type: 'action_task_params', + attributes: { + actionId: '123', + }, + references: [], + }, + ], + }); + await executeFn(internalSavedObjectsRepository, [ + { + id: '123', + params: { baz: false }, + executionId: '123abc', + source: asSavedObjectExecutionSource(source), + }, + { + id: '123', + params: { baz: true }, + executionId: '234xyz', + }, + ]); + expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); + expect(mockTaskManager.bulkSchedule.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "params": Object { + "actionTaskParamsId": "234", + "spaceId": "default", + }, + "scope": Array [ + "actions", + ], + "state": Object {}, + "taskType": "actions:.email", + }, + Object { + "params": Object { + "actionTaskParamsId": "345", + "spaceId": "default", + }, + "scope": Array [ + "actions", + ], + "state": Object {}, + "taskType": "actions:.email", + }, + ], + ] + `); + + expect(internalSavedObjectsRepository.bulkCreate).toHaveBeenCalledWith([ + { + type: 'action_task_params', + attributes: { + actionId: '123', + params: { baz: false }, + executionId: '123abc', + apiKey: null, + }, + references: [ + { + id: sourceUuid, + name: 'source', + type: 'alert', + }, + ], + }, + { + type: 'action_task_params', + attributes: { + actionId: '123', + params: { baz: true }, + executionId: '234xyz', + apiKey: null, + }, + references: [], + }, + ]); + }); + + test('schedules the actions with all given parameters with a preconfigured connector and relatedSavedObjects specified', async () => { + const sourceUuid = uuid.v4(); + const source = { type: 'alert', id: sourceUuid }; + const executeFn = createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: mockTaskManager, + connectorTypeRegistry: actionTypeRegistryMock.create(), + preconfiguredConnectors: [ + { + id: '123', + actionTypeId: '.email', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + ], + }); + + internalSavedObjectsRepository.bulkCreate.mockResolvedValueOnce({ + saved_objects: [ + { + id: '234', + type: 'action_task_params', + attributes: { + actionId: '123', + }, + references: [ + { + id: sourceUuid, + name: 'source', + type: 'alert', + }, + ], + }, + { + id: '345', + type: 'action_task_params', + attributes: { + actionId: '123', + }, + references: [ + { + id: 'some-id', + name: 'related_some-type_0', + type: 'some-type', + }, + ], + }, + ], + }); + await executeFn(internalSavedObjectsRepository, [ + { + id: '123', + params: { baz: false }, + executionId: '123abc', + source: asSavedObjectExecutionSource(source), + }, + { + id: '123', + params: { baz: true }, + executionId: '234xyz', + relatedSavedObjects: [ + { + id: 'some-id', + namespace: 'some-namespace', + type: 'some-type', + }, + ], + }, + ]); + expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); + expect(mockTaskManager.bulkSchedule.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "params": Object { + "actionTaskParamsId": "234", + "spaceId": "default", + }, + "scope": Array [ + "actions", + ], + "state": Object {}, + "taskType": "actions:.email", + }, + Object { + "params": Object { + "actionTaskParamsId": "345", + "spaceId": "default", + }, + "scope": Array [ + "actions", + ], + "state": Object {}, + "taskType": "actions:.email", + }, + ], + ] + `); + + expect(internalSavedObjectsRepository.bulkCreate).toHaveBeenCalledWith([ + { + type: 'action_task_params', + attributes: { + actionId: '123', + params: { baz: false }, + executionId: '123abc', + apiKey: null, + }, + references: [ + { + id: sourceUuid, + name: 'source', + type: 'alert', + }, + ], + }, + { + type: 'action_task_params', + attributes: { + actionId: '123', + params: { baz: true }, + executionId: '234xyz', + apiKey: null, + relatedSavedObjects: [ + { + id: 'related_some-type_0', + namespace: 'some-namespace', + type: 'some-type', + }, + ], + }, + references: [ + { + id: 'some-id', + name: 'related_some-type_0', + type: 'some-type', + }, + ], + }, + ]); + }); + + test('throws when scheduling action using non preconfigured connector', async () => { + const executeFn = createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: mockTaskManager, + connectorTypeRegistry: actionTypeRegistryMock.create(), + preconfiguredConnectors: [ + { + id: '123', + actionTypeId: '.email', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + ], + }); + await expect( + executeFn(internalSavedObjectsRepository, [ + { + id: '123', + params: { baz: false }, + executionId: '123abc', + }, + { + id: 'not-preconfigured', + params: { baz: true }, + executionId: '234xyz', + }, + ]) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"not-preconfigured are not preconfigured connectors and can't be scheduled for unsecured actions execution"` + ); + }); + + test('throws when connector type is not enabled', async () => { + const mockedConnectorTypeRegistry = actionTypeRegistryMock.create(); + const executeFn = createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: mockTaskManager, + connectorTypeRegistry: mockedConnectorTypeRegistry, + preconfiguredConnectors: [ + { + id: '123', + actionTypeId: '.email', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + ], + }); + mockedConnectorTypeRegistry.ensureActionTypeEnabled.mockImplementation(() => { + throw new Error('Fail'); + }); + + await expect( + executeFn(internalSavedObjectsRepository, [ + { + id: '123', + params: { baz: false }, + executionId: '123abc', + }, + { + id: '123', + params: { baz: true }, + executionId: '234xyz', + }, + ]) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); + }); + + test('throws when scheduling action using non allow-listed preconfigured connector', async () => { + const executeFn = createBulkUnsecuredExecutionEnqueuerFunction({ + taskManager: mockTaskManager, + connectorTypeRegistry: actionTypeRegistryMock.create(), + preconfiguredConnectors: [ + { + id: '123', + actionTypeId: '.email', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + { + id: '456', + actionTypeId: 'not-in-allowlist', + config: {}, + isPreconfigured: true, + isDeprecated: false, + name: 'x', + secrets: {}, + }, + ], + }); + await expect( + executeFn(internalSavedObjectsRepository, [ + { + id: '123', + params: { baz: false }, + executionId: '123abc', + }, + { + id: '456', + params: { baz: true }, + executionId: '234xyz', + }, + ]) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"not-in-allowlist actions cannot be scheduled for unsecured actions execution"` + ); + }); +}); 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 acc4d01e7c1a2b..45c0eb5c602884 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -27,7 +27,6 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { export interface ExecuteOptions extends Pick { id: string; executionId: string; - consumer?: string; relatedSavedObjects?: RelatedSavedObjects; } @@ -35,7 +34,6 @@ export interface ActionTaskParams extends Pick actionId: string; apiKey: string | null; executionId: string; - consumer?: string; relatedSavedObjects?: RelatedSavedObjects; } @@ -117,7 +115,6 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ params: actionToExecute.params, apiKey: null, executionId: actionToExecute.executionId, - consumer: actionToExecute.consumer, 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 54405b03e7c02a..e0ef54601fc825 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -45,7 +45,21 @@ actionExecutor.initialize({ actionTypeRegistry, encryptedSavedObjectsClient, eventLogger, - preconfiguredActions: [], + preconfiguredActions: [ + { + id: 'preconfigured', + name: 'Preconfigured', + actionTypeId: 'test', + config: { + bar: 'preconfigured', + }, + secrets: { + apiKey: 'abc', + }, + isPreconfigured: true, + isDeprecated: false, + }, + ], }); beforeEach(() => { @@ -179,6 +193,106 @@ test('successfully executes', async () => { `); }); +test('successfully executes with preconfigured connector', async () => { + const actionType: jest.Mocked = { + id: 'test', + name: 'Test', + minimumLicenseRequired: 'basic', + supportedFeatureIds: ['alerting'], + executor: jest.fn(), + }; + + actionTypeRegistry.get.mockReturnValueOnce(actionType); + await actionExecutor.execute({ ...executeParams, actionId: 'preconfigured' }); + + expect(actionsClient.get).not.toHaveBeenCalled(); + expect(encryptedSavedObjectsClient.getDecryptedAsInternalUser).not.toHaveBeenCalled(); + + expect(actionTypeRegistry.get).toHaveBeenCalledWith('test'); + expect(actionTypeRegistry.isActionExecutable).toHaveBeenCalledWith('preconfigured', 'test', { + notifyUsage: true, + }); + + expect(actionType.executor).toHaveBeenCalledWith({ + actionId: 'preconfigured', + services: expect.anything(), + config: { + bar: 'preconfigured', + }, + secrets: { + apiKey: 'abc', + }, + params: { foo: true }, + }); + + expect(loggerMock.debug).toBeCalledWith('executing action test:preconfigured: Preconfigured'); + expect(eventLogger.logEvent.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "event": Object { + "action": "execute-start", + "kind": "action", + }, + "kibana": Object { + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "preconfigured", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action started: test:preconfigured: Preconfigured", + }, + ], + Array [ + Object { + "event": Object { + "action": "execute", + "kind": "action", + "outcome": "success", + }, + "kibana": Object { + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "preconfigured", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action executed: test:preconfigured: Preconfigured", + }, + ], + ] + `); +}); + test('successfully executes as a task', async () => { const actionType: jest.Mocked = { id: 'test', @@ -504,6 +618,131 @@ test('throws an error when passing isESOCanEncrypt with value of false', async ( ); }); +test('should not throw error if action is preconfigured and isESOCanEncrypt is false', async () => { + const customActionExecutor = new ActionExecutor({ isESOCanEncrypt: false }); + customActionExecutor.initialize({ + logger: loggingSystemMock.create().get(), + spaces: spacesMock, + getActionsClientWithRequest, + getServices: () => services, + actionTypeRegistry, + encryptedSavedObjectsClient, + eventLogger: eventLoggerMock.create(), + preconfiguredActions: [ + { + id: 'preconfigured', + name: 'Preconfigured', + actionTypeId: 'test', + config: { + bar: 'preconfigured', + }, + secrets: { + apiKey: 'abc', + }, + isPreconfigured: true, + isDeprecated: false, + }, + ], + }); + const actionType: jest.Mocked = { + id: 'test', + name: 'Test', + minimumLicenseRequired: 'basic', + supportedFeatureIds: ['alerting'], + executor: jest.fn(), + }; + + actionTypeRegistry.get.mockReturnValueOnce(actionType); + await actionExecutor.execute({ ...executeParams, actionId: 'preconfigured' }); + + expect(actionsClient.get).not.toHaveBeenCalled(); + expect(encryptedSavedObjectsClient.getDecryptedAsInternalUser).not.toHaveBeenCalled(); + + expect(actionTypeRegistry.get).toHaveBeenCalledWith('test'); + expect(actionTypeRegistry.isActionExecutable).toHaveBeenCalledWith('preconfigured', 'test', { + notifyUsage: true, + }); + + expect(actionType.executor).toHaveBeenCalledWith({ + actionId: 'preconfigured', + services: expect.anything(), + config: { + bar: 'preconfigured', + }, + secrets: { + apiKey: 'abc', + }, + params: { foo: true }, + }); + + expect(loggerMock.debug).toBeCalledWith('executing action test:preconfigured: Preconfigured'); + expect(eventLogger.logEvent.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + Object { + "event": Object { + "action": "execute-start", + "kind": "action", + }, + "kibana": Object { + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "preconfigured", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action started: test:preconfigured: Preconfigured", + }, + ], + Array [ + Object { + "event": Object { + "action": "execute", + "kind": "action", + "outcome": "success", + }, + "kibana": Object { + "alert": Object { + "rule": Object { + "execution": Object { + "uuid": "123abc", + }, + }, + }, + "saved_objects": Array [ + Object { + "id": "preconfigured", + "namespace": "some-namespace", + "rel": "primary", + "type": "action", + "type_id": "test", + }, + ], + "space_ids": Array [ + "some-namespace", + ], + }, + "message": "action executed: test:preconfigured: Preconfigured", + }, + ], + ] + `); +}); + test('does not log warning when alert executor succeeds', async () => { const executorMock = setupActionExecutorMock(); executorMock.mockResolvedValue({ diff --git a/x-pack/plugins/actions/server/mocks.ts b/x-pack/plugins/actions/server/mocks.ts index 3b8155818452f8..4d5846de9528fa 100644 --- a/x-pack/plugins/actions/server/mocks.ts +++ b/x-pack/plugins/actions/server/mocks.ts @@ -17,6 +17,7 @@ import { PluginSetupContract, PluginStartContract, renderActionParameterTemplate import { Services } from './types'; import { actionsAuthorizationMock } from './authorization/actions_authorization.mock'; import { ConnectorTokenClient } from './lib/connector_token_client'; +import { unsecuredActionsClientMock } from './unsecured_actions_client/unsecured_actions_client.mock'; export { actionsAuthorizationMock }; export { actionsClientMock }; const logger = loggingSystemMock.create().get() as jest.Mocked; @@ -38,6 +39,7 @@ const createStartMock = () => { isActionTypeEnabled: jest.fn(), isActionExecutable: jest.fn(), getActionsClientWithRequest: jest.fn().mockResolvedValue(actionsClientMock.create()), + getUnsecuredActionsClient: jest.fn().mockResolvedValue(unsecuredActionsClientMock.create()), getActionsAuthorizationWithRequest: jest .fn() .mockReturnValue(actionsAuthorizationMock.create()), diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index e5eb5f8dcaa8d0..a5183148b14a74 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/x-pack/plugins/actions/server/plugin.ts @@ -101,7 +101,6 @@ import { createSubActionConnectorFramework } from './sub_action_framework'; import { IServiceAbstract, SubActionConnectorType } from './sub_action_framework/types'; import { SubActionConnector } from './sub_action_framework/sub_action_connector'; import { CaseConnector } from './sub_action_framework/case'; -import { UnsecuredActionsClientAccessRegistry } from './unsecured_actions_client/unsecured_actions_client_access_registry'; import { UnsecuredActionsClient } from './unsecured_actions_client/unsecured_actions_client'; import { createBulkUnsecuredExecutionEnqueuerFunction } from './create_unsecured_execute_function'; @@ -120,7 +119,6 @@ export interface PluginSetupContract { >( connector: SubActionConnectorType ): void; - registerUnsecuredActionsClientAccess(featureId: string): void; isPreconfiguredConnector(connectorId: string): boolean; getSubActionConnectorClass: () => IServiceAbstract; getCaseConnectorClass: () => IServiceAbstract; @@ -194,7 +192,6 @@ export class ActionsPlugin implements Plugin { subActionFramework.registerConnector(connector); }, - registerUnsecuredActionsClientAccess: (featureId: string) => { - this.unsecuredActionsClientAccessRegistry?.register(featureId); - }, isPreconfiguredConnector: (connectorId: string): boolean => { return !!this.preconfiguredActions.find( (preconfigured) => preconfigured.id === connectorId @@ -471,7 +463,6 @@ export class ActionsPlugin implements Plugin; +export type UnsecuredActionsClientMock = jest.Mocked; + +const createUnsecuredActionsClientMock = () => { + const mocked: UnsecuredActionsClientMock = { + bulkEnqueueExecution: jest.fn(), + }; + return mocked; +}; + +export const unsecuredActionsClientMock: { + create: () => UnsecuredActionsClientMock; +} = { + create: createUnsecuredActionsClientMock, +}; 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 new file mode 100644 index 00000000000000..c863e943b8dc09 --- /dev/null +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { UnsecuredActionsClient } from './unsecured_actions_client'; +import { savedObjectsRepositoryMock } from '@kbn/core/server/mocks'; + +const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); +const executionEnqueuer = jest.fn(); + +let unsecuredActionsClient: UnsecuredActionsClient; + +beforeEach(() => { + jest.resetAllMocks(); + unsecuredActionsClient = new UnsecuredActionsClient({ + internalSavedObjectsRepository, + executionEnqueuer, + }); +}); + +describe('bulkEnqueueExecution()', () => { + test('throws error when enqueuing execution with not allowed requester id', async () => { + const opts = [ + { + id: 'preconfigured1', + params: {}, + executionId: '123abc', + }, + { + id: 'preconfigured2', + params: {}, + executionId: '456def', + }, + ]; + await expect( + unsecuredActionsClient.bulkEnqueueExecution('badId', opts) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"\\"badId\\" feature is not allow-listed for UnsecuredActionsClient access."` + ); + }); + + test('calls the executionEnqueuer with the appropriate parameters', async () => { + const opts = [ + { + id: 'preconfigured1', + params: {}, + executionId: '123abc', + }, + { + id: 'preconfigured2', + params: {}, + executionId: '456def', + }, + ]; + await expect( + unsecuredActionsClient.bulkEnqueueExecution('notifications', opts) + ).resolves.toMatchInlineSnapshot(`undefined`); + + expect(executionEnqueuer).toHaveBeenCalledWith(internalSavedObjectsRepository, opts); + }); +}); 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 7ff0c3d72c8e71..3ec5d159109529 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 @@ -6,25 +6,25 @@ */ import { ISavedObjectsRepository } from '@kbn/core/server'; -import { UnsecuredActionsClientAccessRegistry } from './unsecured_actions_client_access_registry'; import { BulkUnsecuredExecutionEnqueuer, ExecuteOptions, } from '../create_unsecured_execute_function'; +// 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', 'alerting']; + export interface UnsecuredActionsClientOpts { - unsecuredActionsClientAccessRegistry: UnsecuredActionsClientAccessRegistry; internalSavedObjectsRepository: ISavedObjectsRepository; executionEnqueuer: BulkUnsecuredExecutionEnqueuer; } export class UnsecuredActionsClient { - private readonly unsecuredActionsClientAccessRegistry: UnsecuredActionsClientAccessRegistry; private readonly internalSavedObjectsRepository: ISavedObjectsRepository; private readonly executionEnqueuer: BulkUnsecuredExecutionEnqueuer; constructor(params: UnsecuredActionsClientOpts) { - this.unsecuredActionsClientAccessRegistry = params.unsecuredActionsClientAccessRegistry; this.executionEnqueuer = params.executionEnqueuer; this.internalSavedObjectsRepository = params.internalSavedObjectsRepository; } @@ -34,9 +34,9 @@ export class UnsecuredActionsClient { actionsToExecute: ExecuteOptions[] ): Promise { // Check that requesterId is allowed - if (!this.unsecuredActionsClientAccessRegistry.has(requesterId)) { + if (!ALLOWED_REQUESTER_IDS.includes(requesterId)) { throw new Error( - `${requesterId} feature is not registered for UnsecuredActionsClient access.` + `"${requesterId}" feature is not allow-listed for UnsecuredActionsClient access.` ); } return this.executionEnqueuer(this.internalSavedObjectsRepository, actionsToExecute); diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts deleted file mode 100644 index 39975a9b7fb252..00000000000000 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client_access_registry.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -export class UnsecuredActionsClientAccessRegistry { - private readonly allowedFeatureIds: Map = new Map(); - - /** - * Returns if the access registry has the given feature id registered - */ - public has(id: string) { - return this.allowedFeatureIds.has(id); - } - - /** - * Registers feature id to the access registry - */ - public register(id: string) { - this.allowedFeatureIds.set(id, true); - } -} From baf051ab58db193e51174eed215ce5e7371feb3a Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Fri, 14 Oct 2022 10:21:16 -0400 Subject: [PATCH 06/38] Removing execution id --- .../actions/server/create_execute_function.ts | 2 +- .../create_unsecured_execute_function.test.ts | 12 --------- .../create_unsecured_execute_function.ts | 5 +--- .../create_action_event_log_record_object.ts | 26 +++++++++---------- x-pack/plugins/alerting/server/plugin.ts | 3 --- 5 files changed, 15 insertions(+), 33 deletions(-) diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index 8f4c4eee61e846..b37f618e644618 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 extends Pick { +interface ActionTaskParams extends Pick { actionId: string; apiKey: string | null; executionId: string; 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 d4d52b3d1236b6..591755b7df40b2 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 @@ -59,12 +59,10 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, - executionId: '123abc', }, { id: '123', params: { baz: true }, - executionId: '234xyz', }, ]); expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); @@ -170,13 +168,11 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, - executionId: '123abc', source: asSavedObjectExecutionSource(source), }, { id: '123', params: { baz: true }, - executionId: '234xyz', }, ]); expect(mockTaskManager.bulkSchedule).toHaveBeenCalledTimes(1); @@ -294,13 +290,11 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, - executionId: '123abc', source: asSavedObjectExecutionSource(source), }, { id: '123', params: { baz: true }, - executionId: '234xyz', relatedSavedObjects: [ { id: 'some-id', @@ -404,12 +398,10 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, - executionId: '123abc', }, { id: 'not-preconfigured', params: { baz: true }, - executionId: '234xyz', }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -443,12 +435,10 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, - executionId: '123abc', }, { id: '123', params: { baz: true }, - executionId: '234xyz', }, ]) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); @@ -484,12 +474,10 @@ describe('bulkExecute()', () => { { id: '123', params: { baz: false }, - executionId: '123abc', }, { id: '456', params: { baz: true }, - executionId: '234xyz', }, ]) ).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 45c0eb5c602884..0f044cd78e06ec 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -26,14 +26,12 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { export interface ExecuteOptions extends Pick { id: string; - executionId: string; relatedSavedObjects?: RelatedSavedObjects; } -export interface ActionTaskParams extends Pick { +interface ActionTaskParams extends Pick { actionId: string; apiKey: string | null; - executionId: string; relatedSavedObjects?: RelatedSavedObjects; } @@ -114,7 +112,6 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ actionId: actionToExecute.id, params: actionToExecute.params, apiKey: null, - executionId: actionToExecute.executionId, relatedSavedObjects: relatedSavedObjectWithRefs, }, references: taskReferences, 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 5d556398dc6683..2632ead26a4770 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 @@ -5,7 +5,7 @@ * 2.0. */ -import { set } from 'lodash'; +import { isEmpty, set } from 'lodash'; import { IEvent, SAVED_OBJECT_REL_PRIMARY } from '@kbn/event-log-plugin/server'; import { RelatedSavedObjects } from './related_saved_objects'; @@ -38,6 +38,17 @@ export function createActionEventLogRecordObject(params: CreateActionEventLogRec const { action, message, task, namespace, executionId, spaceId, consumer, relatedSavedObjects } = params; + const kibanaAlertRule = { + ...(consumer ? { consumer } : {}), + ...(executionId + ? { + execution: { + uuid: executionId, + }, + } + : {}), + }; + const event: Event = { ...(params.timestamp ? { '@timestamp': params.timestamp } : {}), event: { @@ -45,18 +56,7 @@ export function createActionEventLogRecordObject(params: CreateActionEventLogRec kind: 'action', }, kibana: { - alert: { - rule: { - ...(consumer ? { consumer } : {}), - ...(executionId - ? { - execution: { - uuid: executionId, - }, - } - : {}), - }, - }, + ...(!isEmpty(kibanaAlertRule) ? { alert: { rule: kibanaAlertRule } } : {}), saved_objects: params.savedObjects.map((so) => ({ ...(so.relation ? { rel: so.relation } : {}), type: so.type, diff --git a/x-pack/plugins/alerting/server/plugin.ts b/x-pack/plugins/alerting/server/plugin.ts index a303814dd261c6..d2c58dddc879f9 100644 --- a/x-pack/plugins/alerting/server/plugin.ts +++ b/x-pack/plugins/alerting/server/plugin.ts @@ -219,8 +219,6 @@ export class AlertingPlugin { this.eventLogService = plugins.eventLog; plugins.eventLog.registerProviderActions(EVENT_LOG_PROVIDER, Object.values(EVENT_LOG_ACTIONS)); - plugins.actions.registerUnsecuredActionsClientAccess('alerting'); - const ruleTypeRegistry = new RuleTypeRegistry({ logger: this.logger, taskManager: plugins.taskManager, @@ -473,7 +471,6 @@ export class AlertingPlugin { subject: 'hello from Kibana!', message: 'does this work??', }, - executionId: 'abc', }, ]); }); From 96b696f44746865796706b57e4b6bc791efd5316 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Fri, 14 Oct 2022 11:05:11 -0400 Subject: [PATCH 07/38] Cleanup --- ...ate_action_event_log_record_object.test.ts | 37 +++++++++++++++++++ .../unsecured_actions_client.ts | 2 +- x-pack/plugins/alerting/server/plugin.ts | 12 ------ 3 files changed, 38 insertions(+), 13 deletions(-) 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 72cbda1312b9a5..69eca915cc721d 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 @@ -109,6 +109,43 @@ describe('createActionEventLogRecordObject', () => { }); }); + test('created action event "execute" with no kibana.alert.rule fields', async () => { + expect( + createActionEventLogRecordObject({ + actionId: '1', + name: 'test name', + action: 'execute', + message: 'action execution start', + namespace: 'default', + savedObjects: [ + { + id: '2', + type: 'action', + typeId: '.email', + relation: 'primary', + }, + ], + }) + ).toStrictEqual({ + event: { + action: 'execute', + kind: 'action', + }, + kibana: { + saved_objects: [ + { + id: '2', + namespace: 'default', + rel: 'primary', + type: 'action', + type_id: '.email', + }, + ], + }, + message: 'action execution start', + }); + }); + test('created action event "execute-timeout"', async () => { expect( createActionEventLogRecordObject({ 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 3ec5d159109529..45980a64123903 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 @@ -13,7 +13,7 @@ import { // 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', 'alerting']; +const ALLOWED_REQUESTER_IDS = ['notifications']; export interface UnsecuredActionsClientOpts { internalSavedObjectsRepository: ISavedObjectsRepository; diff --git a/x-pack/plugins/alerting/server/plugin.ts b/x-pack/plugins/alerting/server/plugin.ts index d2c58dddc879f9..7450dcb1a45d02 100644 --- a/x-pack/plugins/alerting/server/plugin.ts +++ b/x-pack/plugins/alerting/server/plugin.ts @@ -462,18 +462,6 @@ export class AlertingPlugin { scheduleAlertingHealthCheck(this.logger, this.config, plugins.taskManager); scheduleApiKeyInvalidatorTask(this.telemetryLogger, this.config, plugins.taskManager); - plugins.actions.getUnsecuredActionsClient().then((unsecuredActionsClient) => { - unsecuredActionsClient.bulkEnqueueExecution('alerting', [ - { - id: 'gmail', - params: { - to: ['xxxxxx'], - subject: 'hello from Kibana!', - message: 'does this work??', - }, - }, - ]); - }); return { listTypes: ruleTypeRegistry!.list.bind(this.ruleTypeRegistry!), getAlertingAuthorizationWithRequest, From 6cf7a3aa19b50e0865fdb810b63e6866f8b3a6c9 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Fri, 14 Oct 2022 11:59:22 -0400 Subject: [PATCH 08/38] Fixing unit tests --- .../server/create_unsecured_execute_function.test.ts | 6 ------ 1 file changed, 6 deletions(-) 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 591755b7df40b2..ea8407b9562765 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 @@ -101,7 +101,6 @@ describe('bulkExecute()', () => { attributes: { actionId: '123', params: { baz: false }, - executionId: '123abc', apiKey: null, }, references: [], @@ -111,7 +110,6 @@ describe('bulkExecute()', () => { attributes: { actionId: '123', params: { baz: true }, - executionId: '234xyz', apiKey: null, }, references: [], @@ -211,7 +209,6 @@ describe('bulkExecute()', () => { attributes: { actionId: '123', params: { baz: false }, - executionId: '123abc', apiKey: null, }, references: [ @@ -227,7 +224,6 @@ describe('bulkExecute()', () => { attributes: { actionId: '123', params: { baz: true }, - executionId: '234xyz', apiKey: null, }, references: [], @@ -340,7 +336,6 @@ describe('bulkExecute()', () => { attributes: { actionId: '123', params: { baz: false }, - executionId: '123abc', apiKey: null, }, references: [ @@ -356,7 +351,6 @@ describe('bulkExecute()', () => { attributes: { actionId: '123', params: { baz: true }, - executionId: '234xyz', apiKey: null, relatedSavedObjects: [ { From 8865a8090f6edf3d35fe46487451c113ac321fe5 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Mon, 17 Oct 2022 09:55:45 -0400 Subject: [PATCH 09/38] Removing slack from allowlist --- .../actions/server/create_unsecured_execute_function.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 0f044cd78e06ec..2bd291899ceeff 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { compact } from 'lodash'; import { ISavedObjectsRepository, SavedObjectsBulkResponse } from '@kbn/core/server'; import { TaskManagerStartContract } from '@kbn/task-manager-plugin/server'; import { @@ -17,7 +16,7 @@ import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; import { RelatedSavedObjects } from './lib/related_saved_objects'; -const ALLOWED_CONNECTOR_TYPE_IDS = ['.email', '.slack']; +const ALLOWED_CONNECTOR_TYPE_IDS = ['.email']; interface CreateBulkUnsecuredExecuteFunctionOptions { taskManager: TaskManagerStartContract; connectorTypeRegistry: ConnectorTypeRegistryContract; @@ -65,11 +64,11 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ ); } - const connectors: PreconfiguredConnector[] = compact( - connectorIds.map((connectorId) => + const connectors: PreconfiguredConnector[] = connectorIds + .map((connectorId) => preconfiguredConnectors.find((pConnector) => pConnector.id === connectorId) ) - ); + .filter(Boolean) as PreconfiguredConnector[]; connectors.forEach((connector) => { const { id, actionTypeId } = connector; From 7c884aea7ce047dd97e1ea066c1fe27a95d7ef88 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Mon, 17 Oct 2022 10:58:04 -0400 Subject: [PATCH 10/38] Make getUnsecuredActionsClient synchronous --- x-pack/plugins/actions/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index a5183148b14a74..8248dee8623948 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/x-pack/plugins/actions/server/plugin.ts @@ -140,7 +140,7 @@ export interface PluginStartContract { preconfiguredActions: PreConfiguredAction[]; - getUnsecuredActionsClient(): Promise>; + getUnsecuredActionsClient(): PublicMethodsOf; renderActionParameterTemplates( actionTypeId: string, @@ -456,7 +456,7 @@ export class ActionsPlugin implements Plugin { + const getUnsecuredActionsClient = () => { const internalSavedObjectsRepository = core.savedObjects.createInternalRepository([ ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, ]); From f4c18514c141bc54ce1f4bd3ad40b5ab16864687 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Mon, 17 Oct 2022 11:24:26 -0400 Subject: [PATCH 11/38] Add comment --- .../plugins/actions/server/create_unsecured_execute_function.ts | 2 ++ 1 file changed, 2 insertions(+) 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 2bd291899ceeff..ea98ee6e9f2c01 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -16,6 +16,8 @@ import { ExecuteOptions as ActionExecutorOptions } from './lib/action_executor'; import { extractSavedObjectReferences, isSavedObjectExecutionSource } from './lib'; import { RelatedSavedObjects } from './lib/related_saved_objects'; +// This allowlist should only contain connector types that don't require API keys for +// execution. const ALLOWED_CONNECTOR_TYPE_IDS = ['.email']; interface CreateBulkUnsecuredExecuteFunctionOptions { taskManager: TaskManagerStartContract; From 4b7f4829b0a940f0bfcd79210773df4ed315a895 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Mon, 17 Oct 2022 17:56:28 +0200 Subject: [PATCH 12/38] Misc enhancements following PR comments --- .github/CODEOWNERS | 1 + src/plugins/telemetry/README.md | 4 +-- x-pack/plugins/notifications/README.md | 6 ++-- .../notifications/common/lib/promise_utils.ts | 25 --------------- x-pack/plugins/notifications/server/plugin.ts | 31 ++++++++++--------- .../notifications/server/services/email.ts | 31 ------------------- .../server/services/email_service.ts | 29 ++++++++++++----- .../notifications/server/services/index.ts | 4 +-- .../lib/index.ts => server/services/types.ts} | 11 +++++-- 9 files changed, 56 insertions(+), 86 deletions(-) delete mode 100755 x-pack/plugins/notifications/common/lib/promise_utils.ts delete mode 100755 x-pack/plugins/notifications/server/services/email.ts rename x-pack/plugins/notifications/{common/lib/index.ts => server/services/types.ts} (57%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index e19165cf4faa16..6676ae9401f96c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -90,6 +90,7 @@ x-pack/examples/files_example @elastic/kibana-app-services /x-pack/test/search_sessions_integration/ @elastic/kibana-app-services /test/plugin_functional/test_suites/panel_actions @elastic/kibana-app-services /test/plugin_functional/test_suites/data_plugin @elastic/kibana-app-services +/x-pack/plugins/notifications/ @elastic/kibana-app-services ### Observability Plugins diff --git a/src/plugins/telemetry/README.md b/src/plugins/telemetry/README.md index df0070effe7546..6b57eeda9dc80d 100644 --- a/src/plugins/telemetry/README.md +++ b/src/plugins/telemetry/README.md @@ -51,14 +51,14 @@ To use the exposed plugin start and setup contracts: import { TelemetryPluginsStart } from '../telemetry/server`; -interface MyPlyginStartDeps { +interface MyPluginStartDeps { telemetry?: TelemetryPluginsStart; } class MyPlugin { public async start( core: CoreStart, - { telemetry }: MyPlyginStartDeps + { telemetry }: MyPluginStartDeps ) { const isOptedIn = await telemetry?.getIsOptedIn(); ... diff --git a/x-pack/plugins/notifications/README.md b/x-pack/plugins/notifications/README.md index 0a6a28b2e35016..66f079a395516e 100755 --- a/x-pack/plugins/notifications/README.md +++ b/x-pack/plugins/notifications/README.md @@ -29,7 +29,7 @@ To use the exposed plugin start and setup contracts: // /kibana.json { "id": "...", -"optionalPlugins": ["notifications"] +"requiredPlugins": ["notifications"] } ``` @@ -48,7 +48,7 @@ class MyPlugin { core: CoreStart, { notifications }: MyPluginStartDeps ) { - const emailService = await notifications?.email; + const emailService = await notifications.email; emailService.sendPlainTextEmail({ to: 'foo@bar.com', subject: 'Some subject', @@ -61,7 +61,7 @@ class MyPlugin { ### Requirements -- This plugin currently depends on the `'actions'` plugin, as it uses `Connectors` under the hood. Please make sure the `'actions'` plugin is included in your deployment. +- This plugin currently depends on the `'actions'` plugin, as it uses `Connectors` under the hood. - Note also that for each notification channel the corresponding connector must be preconfigured. E.g. to enable email notifications, an `Email` connector must exist in the system. - Once the appropriate connectors are preconfigured in `kibana.yaml`, you can configure the `'notifications'` plugin by adding: diff --git a/x-pack/plugins/notifications/common/lib/promise_utils.ts b/x-pack/plugins/notifications/common/lib/promise_utils.ts deleted file mode 100755 index 035bba091a05bf..00000000000000 --- a/x-pack/plugins/notifications/common/lib/promise_utils.ts +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -export type ResolvablePromise = Promise & { - doResolve: (value: unknown) => void; - doReject: (reason?: any) => void; -}; - -export function getResolvablePromise(): ResolvablePromise { - const resolvablePromise: Partial> = {}; - - Object.assign( - resolvablePromise, - new Promise((resolve, reject) => { - resolvablePromise.doResolve = resolve; - resolvablePromise.doReject = reject; - }) - ); - - return resolvablePromise as ResolvablePromise; -} diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts index 03abe9280e9d33..1acff96f6c988e 100755 --- a/x-pack/plugins/notifications/server/plugin.ts +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -12,6 +12,7 @@ import type { Plugin, Logger, } from '@kbn/core/server'; +import { Defer, defer } from '@kbn/kibana-utils-plugin/common'; import type { NotificationsPluginSetupDeps, NotificationsPluginStartDeps, @@ -20,7 +21,6 @@ import type { } from './types'; import { EmailService } from './services'; import { NotificationsConfigType } from './config'; -import { getResolvablePromise, type ResolvablePromise } from '../common/lib'; import { PLUGIN_ID } from '../common'; export class NotificationsPlugin @@ -28,33 +28,36 @@ export class NotificationsPlugin { private readonly logger: Logger; private readonly initialConfig: NotificationsConfigType; - private email: ResolvablePromise; + private emailService: Defer; private emailConnector: string; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); - this.email = getResolvablePromise(); + this.emailService = defer(); this.initialConfig = initializerContext.config.get(); } public setup(core: CoreSetup, plugins: NotificationsPluginSetupDeps) { - // TODO this must be defaulted to 'elastic-cloud-email' for cloud if (!plugins.actions) { - this.email.doReject(`Error starting notification services: 'actions' plugin not available.`); - return { email: this.email }; + this.emailService.reject( + `Error starting notification services: 'actions' plugin not available.` + ); + return { email: this.emailService.promise }; } const emailConnector = this.initialConfig.connectors?.default?.email; if (!emailConnector) { - this.email.doReject('Error starting notification services: Email connector not specified'); - return { email: this.email }; + this.emailService.reject( + 'Error starting notification services: Email connector not specified' + ); + return { email: this.emailService.promise }; } if (!plugins.actions.isPreconfiguredConnector(emailConnector)) { - this.email.doReject( + this.emailService.reject( `Error starting notification services: Unexisting email connector '${emailConnector}' specified` ); - return { email: this.email }; + return { email: this.emailService.promise }; } plugins.actions.registerUnsecuredActionsClientAccess(PLUGIN_ID); @@ -62,7 +65,7 @@ export class NotificationsPlugin this.emailConnector = emailConnector; return { - email: this.email, + email: this.emailService.promise, }; } @@ -71,17 +74,17 @@ export class NotificationsPlugin plugins.actions.getUnsecuredActionsClient().then( (actionsClient) => { const email = new EmailService(PLUGIN_ID, this.emailConnector, actionsClient); - this.email.doResolve(email); + this.emailService.resolve(email); }, (error) => { this.logger.warn(`Error starting notification services: ${error}`); - this.email.doReject(error); + this.emailService.reject(error); } ); } return { - email: this.email, + email: this.emailService.promise, }; } diff --git a/x-pack/plugins/notifications/server/services/email.ts b/x-pack/plugins/notifications/server/services/email.ts deleted file mode 100755 index b6cb05b4d368db..00000000000000 --- a/x-pack/plugins/notifications/server/services/email.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import type { UnsecuredActionsClient } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client'; -import type { IEmailService, PlainTextEmail } from './email_service'; - -export class EmailService implements IEmailService { - constructor( - private requesterId: string, - private connectorId: string, - private actionsClient: UnsecuredActionsClient - ) {} - - async sendPlainTextEmail(params: PlainTextEmail): Promise { - const actions = params.to.map((to) => ({ - id: this.connectorId, - spaceId: 'default', // TODO should be space agnostic? - apiKey: null, // not needed for Email connector - executionId: '???', - params: { - ...params, - to, - }, - })); - return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); - } -} diff --git a/x-pack/plugins/notifications/server/services/email_service.ts b/x-pack/plugins/notifications/server/services/email_service.ts index 9b0dcf81db2899..95d29d219998ef 100755 --- a/x-pack/plugins/notifications/server/services/email_service.ts +++ b/x-pack/plugins/notifications/server/services/email_service.ts @@ -5,12 +5,27 @@ * 2.0. */ -export interface IEmailService { - sendPlainTextEmail(payload: PlainTextEmail): Promise; -} +import type { UnsecuredActionsClient } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client'; +import type { IEmailService, PlainTextEmail } from './types'; + +export class EmailService implements IEmailService { + constructor( + private requesterId: string, + private connectorId: string, + private actionsClient: UnsecuredActionsClient + ) {} -export interface PlainTextEmail extends Record { - to: string[]; - subject: string; - message: string; + async sendPlainTextEmail(params: PlainTextEmail): Promise { + const actions = params.to.map((to) => ({ + id: this.connectorId, + spaceId: 'default', // TODO should be space agnostic? + apiKey: null, // not needed for Email connector + executionId: '???', + params: { + ...params, + to, + }, + })); + return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); + } } diff --git a/x-pack/plugins/notifications/server/services/index.ts b/x-pack/plugins/notifications/server/services/index.ts index c6e225e6661039..a2e1904028db44 100644 --- a/x-pack/plugins/notifications/server/services/index.ts +++ b/x-pack/plugins/notifications/server/services/index.ts @@ -5,5 +5,5 @@ * 2.0. */ -export type { IEmailService, PlainTextEmail } from './email_service'; -export { EmailService } from './email'; +export type { IEmailService, PlainTextEmail } from './types'; +export { EmailService } from './email_service'; diff --git a/x-pack/plugins/notifications/common/lib/index.ts b/x-pack/plugins/notifications/server/services/types.ts similarity index 57% rename from x-pack/plugins/notifications/common/lib/index.ts rename to x-pack/plugins/notifications/server/services/types.ts index 5689f2629dfe32..ee8d3063623f98 100755 --- a/x-pack/plugins/notifications/common/lib/index.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -5,5 +5,12 @@ * 2.0. */ -export type { ResolvablePromise } from './promise_utils'; -export { getResolvablePromise } from './promise_utils'; +export interface IEmailService { + sendPlainTextEmail(payload: PlainTextEmail): Promise; +} + +export interface PlainTextEmail { + to: string[]; + subject: string; + message: string; +} From 47f1ca99a61e7cc7a4338431094c514ec6b56d15 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Tue, 18 Oct 2022 10:55:34 -0400 Subject: [PATCH 13/38] Adding functional tests --- .../unsecured_actions_client.ts | 6 +- .../alerting_api_integration/common/config.ts | 12 ++ .../actions_simulators/server/plugin.ts | 32 ++- .../server/unsecured_actions_simulation.ts | 48 +++++ .../spaces_only/tests/actions/index.ts | 1 + .../actions/schedule_unsecured_action.ts | 191 ++++++++++++++++++ 6 files changed, 279 insertions(+), 11 deletions(-) create mode 100644 x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/unsecured_actions_simulation.ts create mode 100644 x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts 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 45980a64123903..beb5c9d648cdc1 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 @@ -13,7 +13,11 @@ import { // 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']; +const ALLOWED_REQUESTER_IDS = [ + 'notifications', + // For functional testing + 'functional_tester', +]; export interface UnsecuredActionsClientOpts { internalSavedObjectsRepository: ISavedObjectsRepository; diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index 090e48f7a8a2da..d2831b61799f53 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -197,6 +197,18 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) ])}`, `--xpack.actions.preconfiguredAlertHistoryEsIndex=${preconfiguredAlertHistoryEsIndex}`, `--xpack.actions.preconfigured=${JSON.stringify({ + 'my-test-email': { + actionTypeId: '.email', + name: 'TestEmail#xyz', + config: { + from: 'me@test.com', + service: '__json', + }, + secrets: { + user: 'user', + password: 'password', + }, + }, 'my-slack1': { actionTypeId: '.slack', name: 'Slack#xyz', diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts index ae874d942e75b9..ec5bad5ab3ae73 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts @@ -7,10 +7,13 @@ import http from 'http'; import https from 'https'; -import { Plugin, CoreSetup, IRouter } from '@kbn/core/server'; +import { Plugin, CoreSetup, CoreStart, IRouter } from '@kbn/core/server'; import { EncryptedSavedObjectsPluginStart } from '@kbn/encrypted-saved-objects-plugin/server'; import { PluginSetupContract as FeaturesPluginSetup } from '@kbn/features-plugin/server'; -import { PluginSetupContract as ActionsPluginSetupContract } from '@kbn/actions-plugin/server/plugin'; +import { + PluginSetupContract as ActionsPluginSetupContract, + PluginStartContract as ActionsPluginStartContract, +} from '@kbn/actions-plugin/server/plugin'; import { ActionType } from '@kbn/actions-plugin/server'; import { initPlugin as initPagerduty } from './pagerduty_simulation'; import { initPlugin as initSwimlane } from './swimlane_simulation'; @@ -22,6 +25,7 @@ import { initPlugin as initSlack } from './slack_simulation'; import { initPlugin as initWebhook } from './webhook_simulation'; import { initPlugin as initMSExchange } from './ms_exchage_server_simulation'; import { initPlugin as initXmatters } from './xmatters_simulation'; +import { initPlugin as initUnsecuredAction } from './unsecured_actions_simulation'; export const NAME = 'actions-FTS-external-service-simulators'; @@ -84,9 +88,12 @@ interface FixtureSetupDeps { interface FixtureStartDeps { encryptedSavedObjects: EncryptedSavedObjectsPluginStart; + actions: ActionsPluginStartContract; } export class FixturePlugin implements Plugin { + private router: IRouter; + public setup(core: CoreSetup, { features, actions }: FixtureSetupDeps) { // this action is specifically NOT enabled in ../../config.ts const notEnabledActionType: ActionType = { @@ -126,19 +133,24 @@ export class FixturePlugin implements Plugin, + res: KibanaResponseFactory + ): Promise> { + const { body } = req; + + try { + const unsecuredActionsClient = actionsStart.getUnsecuredActionsClient(); + const { requesterId, id, params } = body; + await unsecuredActionsClient.bulkEnqueueExecution(requesterId, [{ id, params }]); + + return res.ok({ body: { status: 'success' } }); + } catch (err) { + return res.ok({ body: { status: 'error', error: `${err}` } }); + } + } + ); +} diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts index 866f13ed5294c8..b4dbb42e8f993a 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts @@ -28,6 +28,7 @@ export default function actionsTests({ loadTestFile, getService }: FtrProviderCo loadTestFile(require.resolve('./connector_types/stack/webhook')); loadTestFile(require.resolve('./connector_types/stack/preconfigured_alert_history_connector')); loadTestFile(require.resolve('./type_not_enabled')); + loadTestFile(require.resolve('./schedule_unsecured_action')); // note that this test will destroy existing spaces loadTestFile(require.resolve('./migrations')); 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 new file mode 100644 index 00000000000000..9a5719b7fa7000 --- /dev/null +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/schedule_unsecured_action.ts @@ -0,0 +1,191 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { Spaces } from '../../scenarios'; +import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; + +// eslint-disable-next-line import/no-default-export +export default function createUnsecuredActionTests({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const kibanaServer = getService('kibanaServer'); + const es = getService('es'); + const retry = getService('retry'); + + describe('schedule unsecured action', () => { + const objectRemover = new ObjectRemover(supertest); + + // need to wait for kibanaServer to settle ... + before(() => { + kibanaServer.resolveUrl(`/api/sample_unsecured_action`); + }); + + after(() => objectRemover.removeAll()); + + it('should successfully schedule email action', async () => { + const testStart = new Date().toISOString(); + const { body: result } = await supertest + .post(`/api/sample_unsecured_action`) + .set('kbn-xsrf', 'xxx') + .send({ + requesterId: 'functional_tester', + id: 'my-test-email', + params: { + to: ['you@test.com'], + subject: 'hello from Kibana!', + message: 'does this work??', + }, + }) + .expect(200); + expect(result.status).to.eql('success'); + + await retry.try(async () => { + const searchResult = await es.search({ + index: '.kibana-event-log*', + body: { + query: { + bool: { + filter: [ + { + term: { + 'event.provider': { + value: 'actions', + }, + }, + }, + { + term: { + 'event.action': 'execute', + }, + }, + { + range: { + '@timestamp': { + gte: testStart, + }, + }, + }, + { + nested: { + path: 'kibana.saved_objects', + query: { + bool: { + filter: [ + { + term: { + 'kibana.saved_objects.id': { + value: 'my-test-email', + }, + }, + }, + { + term: { + 'kibana.saved_objects.type': 'action', + }, + }, + ], + }, + }, + }, + }, + ], + }, + }, + }, + }); + expect((searchResult.hits.total as SearchTotalHits).value).to.eql(1); + + const hit = searchResult.hits.hits[0]; + // @ts-expect-error _source: unknown + expect(hit?._source?.event?.outcome).to.eql('success'); + // @ts-expect-error _source: unknown + expect(hit?._source?.message).to.eql( + `action executed: .email:my-test-email: TestEmail#xyz` + ); + }); + }); + + it('should not allow scheduling email action from unallowed requester', async () => { + const { body: result } = await supertest + .post(`/api/sample_unsecured_action`) + .set('kbn-xsrf', 'xxx') + .send({ + requesterId: 'not_allowed', + id: 'my-test-email', + params: { + to: ['you@test.com'], + subject: 'hello from Kibana!', + message: 'does this work??', + }, + }) + .expect(200); + expect(result.status).to.eql('error'); + expect(result.error).to.eql( + `Error: "not_allowed" feature is not allow-listed for UnsecuredActionsClient access.` + ); + }); + + it('should not allow scheduling action from unallowed connector types', async () => { + const { body: result } = await supertest + .post(`/api/sample_unsecured_action`) + .set('kbn-xsrf', 'xxx') + .send({ + requesterId: 'functional_tester', + id: 'my-slack1', + params: { + message: 'does this work??', + }, + }) + .expect(200); + expect(result.status).to.eql('error'); + expect(result.error).to.eql( + `Error: .slack actions cannot be scheduled for unsecured actions execution` + ); + }); + + it('should not allow scheduling action from non preconfigured connectors', async () => { + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'My email action', + connector_type_id: '.email', + config: { + from: 'me@test.com', + service: '__json', + }, + secrets: { + user: 'user', + password: 'password', + }, + }); + expect(response.status).to.eql(200); + + const connectorId = response.body.id; + objectRemover.add(Spaces.space1.id, connectorId, 'action', 'actions'); + const { body: result } = await supertest + .post(`/api/sample_unsecured_action`) + .set('kbn-xsrf', 'xxx') + .send({ + requesterId: 'functional_tester', + id: connectorId, + params: { + to: ['you@test.com'], + subject: 'hello from Kibana!', + message: 'does this work??', + }, + }) + .expect(200); + expect(result.status).to.eql('error'); + expect(result.error).to.eql( + `Error: ${connectorId} are not preconfigured connectors and can't be scheduled for unsecured actions execution` + ); + }); + }); +} From c87baee6375a3d900ae4dc24010c213d3a1ba007 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Tue, 18 Oct 2022 11:28:38 -0400 Subject: [PATCH 14/38] Fixing types --- .../fixtures/plugins/actions_simulators/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts index ec5bad5ab3ae73..40090208207ace 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts @@ -92,7 +92,7 @@ interface FixtureStartDeps { } export class FixturePlugin implements Plugin { - private router: IRouter; + private router?: IRouter; public setup(core: CoreSetup, { features, actions }: FixtureSetupDeps) { // this action is specifically NOT enabled in ../../config.ts @@ -150,7 +150,7 @@ export class FixturePlugin implements Plugin Date: Tue, 18 Oct 2022 13:23:10 -0400 Subject: [PATCH 15/38] Fixing tests --- .../server/lib/action_executor.test.ts | 2 ++ .../group2/tests/actions/get_all.ts | 24 +++++++++++++++++++ .../alerting_and_actions_telemetry.ts | 2 +- .../spaces_only/tests/actions/get_all.ts | 24 +++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) 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 73a5b66c40c547..0bcb9cf527548f 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -227,6 +227,7 @@ test('successfully executes with preconfigured connector', async () => { apiKey: 'abc', }, params: { foo: true }, + logger: loggerMock, }); expect(loggerMock.debug).toBeCalledWith('executing action test:preconfigured: Preconfigured'); @@ -678,6 +679,7 @@ test('should not throw error if action is preconfigured and isESOCanEncrypt is f apiKey: 'abc', }, params: { foo: true }, + logger: loggerMock, }); expect(loggerMock.debug).toBeCalledWith('executing action test:preconfigured: Preconfigured'); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/get_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/get_all.ts index 69f618c804eb1f..d4bfe3cbdd7049 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/get_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/get_all.ts @@ -127,6 +127,14 @@ export default function getAllActionTests({ getService }: FtrProviderContext) { name: 'Test:_Preconfigured_Index_Record', referenced_by_count: 0, }, + { + id: 'my-test-email', + is_preconfigured: true, + is_deprecated: false, + connector_type_id: '.email', + name: 'TestEmail#xyz', + referenced_by_count: 0, + }, ]); break; default: @@ -262,6 +270,14 @@ export default function getAllActionTests({ getService }: FtrProviderContext) { name: 'Test:_Preconfigured_Index_Record', referenced_by_count: 0, }, + { + id: 'my-test-email', + is_preconfigured: true, + is_deprecated: false, + connector_type_id: '.email', + name: 'TestEmail#xyz', + referenced_by_count: 0, + }, ]); break; default: @@ -361,6 +377,14 @@ export default function getAllActionTests({ getService }: FtrProviderContext) { name: 'Test:_Preconfigured_Index_Record', referenced_by_count: 0, }, + { + id: 'my-test-email', + is_preconfigured: true, + is_deprecated: false, + connector_type_id: '.email', + name: 'TestEmail#xyz', + referenced_by_count: 0, + }, ]); break; default: diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/telemetry/alerting_and_actions_telemetry.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/telemetry/alerting_and_actions_telemetry.ts index b4cb36ab59d85a..707f15534e6639 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/telemetry/alerting_and_actions_telemetry.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/telemetry/alerting_and_actions_telemetry.ts @@ -571,7 +571,7 @@ export default function createAlertingAndActionsTelemetryTests({ getService }: F expect(taskState).not.to.be(undefined); actionsTelemetry = JSON.parse(taskState!); expect(actionsTelemetry.runs).to.equal(2); - expect(actionsTelemetry.count_total).to.equal(19); + expect(actionsTelemetry.count_total).to.equal(20); }); // request alerting telemetry task to run diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all.ts index 0632f48ed6e8d5..7846c9512867e9 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all.ts @@ -115,6 +115,14 @@ export default function getAllActionTests({ getService }: FtrProviderContext) { name: 'Test:_Preconfigured_Index_Record', referenced_by_count: 0, }, + { + id: 'my-test-email', + is_preconfigured: true, + is_deprecated: false, + connector_type_id: '.email', + name: 'TestEmail#xyz', + referenced_by_count: 0, + }, ]); }); @@ -202,6 +210,14 @@ export default function getAllActionTests({ getService }: FtrProviderContext) { name: 'Test:_Preconfigured_Index_Record', referenced_by_count: 0, }, + { + id: 'my-test-email', + is_preconfigured: true, + is_deprecated: false, + connector_type_id: '.email', + name: 'TestEmail#xyz', + referenced_by_count: 0, + }, ]); }); @@ -302,6 +318,14 @@ export default function getAllActionTests({ getService }: FtrProviderContext) { name: 'Test:_Preconfigured_Index_Record', referencedByCount: 0, }, + { + id: 'my-test-email', + isPreconfigured: true, + isDeprecated: false, + actionTypeId: '.email', + name: 'TestEmail#xyz', + referencedByCount: 0, + }, ]); }); }); From 026646a630e613d17e00da254fba6875ad2335de Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 19 Oct 2022 10:39:05 -0400 Subject: [PATCH 16/38] Removing unnecessary Promise.all --- .../actions/server/create_execute_function.ts | 69 +++++++++---------- .../create_unsecured_execute_function.ts | 59 ++++++++-------- 2 files changed, 61 insertions(+), 67 deletions(-) diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index b37f618e644618..30923134235fea 100644 --- a/x-pack/plugins/actions/server/create_execute_function.ts +++ b/x-pack/plugins/actions/server/create_execute_function.ts @@ -176,43 +176,40 @@ export function createBulkExecutionEnqueuerFunction({ connectorIsPreconfigured[id] = isPreconfigured; }); - const actions = await Promise.all( - actionsToExecute.map(async (actionToExecute) => { - // Get saved object references from action ID and relatedSavedObjects - const { references, relatedSavedObjectWithRefs } = extractSavedObjectReferences( - actionToExecute.id, - connectorIsPreconfigured[actionToExecute.id], - actionToExecute.relatedSavedObjects - ); - const executionSourceReference = executionSourceAsSavedObjectReferences( - actionToExecute.source - ); - - const taskReferences = []; - if (executionSourceReference.references) { - taskReferences.push(...executionSourceReference.references); - } - if (references) { - taskReferences.push(...references); - } - - spaceIds[actionToExecute.id] = actionToExecute.spaceId; - - return { - type: ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, - attributes: { - actionId: actionToExecute.id, - params: actionToExecute.params, - apiKey: actionToExecute.apiKey, - executionId: actionToExecute.executionId, - consumer: actionToExecute.consumer, - relatedSavedObjects: relatedSavedObjectWithRefs, - }, - references: taskReferences, - }; - }) - ); + const actions = actionsToExecute.map((actionToExecute) => { + // Get saved object references from action ID and relatedSavedObjects + const { references, relatedSavedObjectWithRefs } = extractSavedObjectReferences( + actionToExecute.id, + connectorIsPreconfigured[actionToExecute.id], + actionToExecute.relatedSavedObjects + ); + const executionSourceReference = executionSourceAsSavedObjectReferences( + actionToExecute.source + ); + + const taskReferences = []; + if (executionSourceReference.references) { + taskReferences.push(...executionSourceReference.references); + } + if (references) { + taskReferences.push(...references); + } + spaceIds[actionToExecute.id] = actionToExecute.spaceId; + + return { + type: ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, + attributes: { + actionId: actionToExecute.id, + params: actionToExecute.params, + apiKey: actionToExecute.apiKey, + executionId: actionToExecute.executionId, + consumer: actionToExecute.consumer, + relatedSavedObjects: relatedSavedObjectWithRefs, + }, + references: taskReferences, + }; + }); const actionTaskParamsRecords: SavedObjectsBulkResponse = await unsecuredSavedObjectsClient.bulkCreate(actions); const taskInstances = actionTaskParamsRecords.saved_objects.map((so) => { 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 ea98ee6e9f2c01..231a298de3db2f 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -87,39 +87,36 @@ export function createBulkUnsecuredExecutionEnqueuerFunction({ connectorTypeIds[id] = actionTypeId; }); - const actions = await Promise.all( - actionsToExecute.map(async (actionToExecute) => { - // Get saved object references from action ID and relatedSavedObjects - const { references, relatedSavedObjectWithRefs } = extractSavedObjectReferences( - actionToExecute.id, - true, - actionToExecute.relatedSavedObjects - ); - const executionSourceReference = executionSourceAsSavedObjectReferences( - actionToExecute.source - ); + const actions = actionsToExecute.map((actionToExecute) => { + // Get saved object references from action ID and relatedSavedObjects + const { references, relatedSavedObjectWithRefs } = extractSavedObjectReferences( + actionToExecute.id, + true, + actionToExecute.relatedSavedObjects + ); + const executionSourceReference = executionSourceAsSavedObjectReferences( + actionToExecute.source + ); - const taskReferences = []; - if (executionSourceReference.references) { - taskReferences.push(...executionSourceReference.references); - } - if (references) { - taskReferences.push(...references); - } - - return { - type: ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, - attributes: { - actionId: actionToExecute.id, - params: actionToExecute.params, - apiKey: null, - relatedSavedObjects: relatedSavedObjectWithRefs, - }, - references: taskReferences, - }; - }) - ); + const taskReferences = []; + if (executionSourceReference.references) { + taskReferences.push(...executionSourceReference.references); + } + if (references) { + taskReferences.push(...references); + } + return { + type: ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, + attributes: { + actionId: actionToExecute.id, + params: actionToExecute.params, + apiKey: null, + relatedSavedObjects: relatedSavedObjectWithRefs, + }, + references: taskReferences, + }; + }); const actionTaskParamsRecords: SavedObjectsBulkResponse = await internalSavedObjectsRepository.bulkCreate(actions); From 802dfa01fba0d4cbd2ec14a0114432a8b45021a3 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 19 Oct 2022 11:44:42 -0400 Subject: [PATCH 17/38] Cleanup --- .../actions/server/create_execute_function.ts | 12 ++++-------- .../server/create_unsecured_execute_function.ts | 10 ++++------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/actions/server/create_execute_function.ts b/x-pack/plugins/actions/server/create_execute_function.ts index 30923134235fea..19447bf8e79e5e 100644 --- a/x-pack/plugins/actions/server/create_execute_function.ts +++ b/x-pack/plugins/actions/server/create_execute_function.ts @@ -16,7 +16,6 @@ import { 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 { RelatedSavedObjects } from './lib/related_saved_objects'; interface CreateExecuteFunctionOptions { taskManager: TaskManagerStartContract; @@ -25,21 +24,18 @@ interface CreateExecuteFunctionOptions { preconfiguredActions: PreConfiguredAction[]; } -export interface ExecuteOptions extends Pick { +export interface ExecuteOptions + extends Pick { id: string; spaceId: string; apiKey: string | null; executionId: string; - consumer?: string; - relatedSavedObjects?: RelatedSavedObjects; } -interface ActionTaskParams extends Pick { - actionId: string; +interface ActionTaskParams + extends Pick { apiKey: string | null; executionId: string; - consumer?: string; - relatedSavedObjects?: RelatedSavedObjects; } export interface GetConnectorsResult { 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 231a298de3db2f..4670601ecff83b 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -14,7 +14,6 @@ import { 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 { RelatedSavedObjects } from './lib/related_saved_objects'; // This allowlist should only contain connector types that don't require API keys for // execution. @@ -25,15 +24,14 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { preconfiguredConnectors: PreconfiguredConnector[]; } -export interface ExecuteOptions extends Pick { +export interface ExecuteOptions + extends Pick { id: string; - relatedSavedObjects?: RelatedSavedObjects; } -interface ActionTaskParams extends Pick { - actionId: string; +interface ActionTaskParams + extends Pick { apiKey: string | null; - relatedSavedObjects?: RelatedSavedObjects; } export type BulkUnsecuredExecutionEnqueuer = ( From c04822531efded1eb6d324710104d45b2b266cc0 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Wed, 19 Oct 2022 17:57:14 +0200 Subject: [PATCH 18/38] Misc fixes and simplifications --- .../create_unsecured_execute_function.ts | 4 +- x-pack/plugins/notifications/jest.config.js | 15 ++++ .../notifications/server/config/config.ts | 18 +---- .../server/config/connectors_email_config.ts | 30 +++++++ .../notifications/server/config/index.ts | 2 +- x-pack/plugins/notifications/server/index.ts | 4 +- x-pack/plugins/notifications/server/plugin.ts | 80 +++---------------- ...service.ts => connectors_email_service.ts} | 8 +- .../connectors_email_service_factory.ts | 46 +++++++++++ .../notifications/server/services/index.ts | 4 +- .../notifications/server/services/types.ts | 11 ++- x-pack/plugins/notifications/server/types.ts | 20 +---- 12 files changed, 129 insertions(+), 113 deletions(-) create mode 100644 x-pack/plugins/notifications/jest.config.js create mode 100644 x-pack/plugins/notifications/server/config/connectors_email_config.ts rename x-pack/plugins/notifications/server/services/{email_service.ts => connectors_email_service.ts} (76%) create mode 100644 x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts 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 231a298de3db2f..21fb0508038c4c 100644 --- a/x-pack/plugins/actions/server/create_unsecured_execute_function.ts +++ b/x-pack/plugins/actions/server/create_unsecured_execute_function.ts @@ -25,9 +25,9 @@ interface CreateBulkUnsecuredExecuteFunctionOptions { preconfiguredConnectors: PreconfiguredConnector[]; } -export interface ExecuteOptions extends Pick { +export interface ExecuteOptions + extends Pick { id: string; - relatedSavedObjects?: RelatedSavedObjects; } interface ActionTaskParams extends Pick { diff --git a/x-pack/plugins/notifications/jest.config.js b/x-pack/plugins/notifications/jest.config.js new file mode 100644 index 00000000000000..b19a8f2efe3349 --- /dev/null +++ b/x-pack/plugins/notifications/jest.config.js @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +module.exports = { + preset: '@kbn/test/jest_node', + rootDir: '../../..', + roots: ['/x-pack/plugins/notifications'], + coverageDirectory: '/target/kibana-coverage/jest/x-pack/plugins/notifications', + coverageReporters: ['text', 'html'], + collectCoverageFrom: ['/x-pack/plugins/notifications/{common,server}/**/*.{js,ts,tsx}'], +}; diff --git a/x-pack/plugins/notifications/server/config/config.ts b/x-pack/plugins/notifications/server/config/config.ts index 2abeb8bc1fa88d..c70f235eab422d 100644 --- a/x-pack/plugins/notifications/server/config/config.ts +++ b/x-pack/plugins/notifications/server/config/config.ts @@ -5,23 +5,11 @@ * 2.0. */ -import { schema, TypeOf } from '@kbn/config-schema'; import { PluginConfigDescriptor } from '@kbn/core/server'; +import { configSchema, type ConnectorsEmailConfigType } from './connectors_email_config'; -const configSchema = schema.object({ - connectors: schema.maybe( - schema.object({ - default: schema.maybe( - schema.object({ - email: schema.maybe(schema.string()), - }) - ), - }) - ), -}); +export type NotificationsConfigType = ConnectorsEmailConfigType; -export type NotificationsConfigType = TypeOf; - -export const config: PluginConfigDescriptor = { +export const config: PluginConfigDescriptor = { schema: configSchema, }; diff --git a/x-pack/plugins/notifications/server/config/connectors_email_config.ts b/x-pack/plugins/notifications/server/config/connectors_email_config.ts new file mode 100644 index 00000000000000..6337900d8d32d2 --- /dev/null +++ b/x-pack/plugins/notifications/server/config/connectors_email_config.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { schema, TypeOf } from '@kbn/config-schema'; +import { PluginConfigDescriptor } from '@kbn/core/server'; + +export const configSchema = schema.object( + { + connectors: schema.maybe( + schema.object({ + default: schema.maybe( + schema.object({ + email: schema.maybe(schema.string()), + }) + ), + }) + ), + }, + { defaultValue: {} } +); + +export type ConnectorsEmailConfigType = TypeOf; + +export const config: PluginConfigDescriptor = { + schema: configSchema, +}; diff --git a/x-pack/plugins/notifications/server/config/index.ts b/x-pack/plugins/notifications/server/config/index.ts index 39da9b5b8842ec..662050c13d2ec7 100644 --- a/x-pack/plugins/notifications/server/config/index.ts +++ b/x-pack/plugins/notifications/server/config/index.ts @@ -5,4 +5,4 @@ * 2.0. */ -export { config, type NotificationsConfigType } from './config'; +export { type NotificationsConfigType, config } from './config'; diff --git a/x-pack/plugins/notifications/server/index.ts b/x-pack/plugins/notifications/server/index.ts index a130383a7b9bdb..9e8785d680de52 100755 --- a/x-pack/plugins/notifications/server/index.ts +++ b/x-pack/plugins/notifications/server/index.ts @@ -7,12 +7,12 @@ import type { PluginInitializerContext } from '@kbn/core/server'; import { NotificationsPlugin } from './plugin'; +export { config } from './config'; // This exports static code and TypeScript types, // as well as, Kibana Platform `plugin()` initializer. +export type { NotificationsPluginStart } from './types'; export function plugin(initializerContext: PluginInitializerContext) { return new NotificationsPlugin(initializerContext); } - -export { NotificationsPluginSetup, NotificationsPluginStart } from './types'; diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts index 1acff96f6c988e..90b5f7f67421a9 100755 --- a/x-pack/plugins/notifications/server/plugin.ts +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -5,87 +5,31 @@ * 2.0. */ -import type { - PluginInitializerContext, - CoreSetup, - CoreStart, - Plugin, - Logger, -} from '@kbn/core/server'; -import { Defer, defer } from '@kbn/kibana-utils-plugin/common'; -import type { - NotificationsPluginSetupDeps, - NotificationsPluginStartDeps, - NotificationsPluginSetup, - NotificationsPluginStart, -} from './types'; -import { EmailService } from './services'; -import { NotificationsConfigType } from './config'; -import { PLUGIN_ID } from '../common'; +import type { PluginInitializerContext, CoreStart, Plugin, Logger } from '@kbn/core/server'; +import type { NotificationsPluginStartDeps, NotificationsPluginStart } from './types'; +import { type EmailService, getEmailService } from './services'; +import type { NotificationsConfigType } from './config'; -export class NotificationsPlugin - implements Plugin -{ +export class NotificationsPlugin implements Plugin { private readonly logger: Logger; private readonly initialConfig: NotificationsConfigType; - private emailService: Defer; - private emailConnector: string; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); - this.emailService = defer(); this.initialConfig = initializerContext.config.get(); } - public setup(core: CoreSetup, plugins: NotificationsPluginSetupDeps) { - if (!plugins.actions) { - this.emailService.reject( - `Error starting notification services: 'actions' plugin not available.` - ); - return { email: this.emailService.promise }; - } - - const emailConnector = this.initialConfig.connectors?.default?.email; - if (!emailConnector) { - this.emailService.reject( - 'Error starting notification services: Email connector not specified' - ); - return { email: this.emailService.promise }; - } - - if (!plugins.actions.isPreconfiguredConnector(emailConnector)) { - this.emailService.reject( - `Error starting notification services: Unexisting email connector '${emailConnector}' specified` - ); - return { email: this.emailService.promise }; - } - - plugins.actions.registerUnsecuredActionsClientAccess(PLUGIN_ID); - - this.emailConnector = emailConnector; - - return { - email: this.emailService.promise, - }; - } + public setup() {} public start(core: CoreStart, plugins: NotificationsPluginStartDeps) { - if (this.emailConnector) { - plugins.actions.getUnsecuredActionsClient().then( - (actionsClient) => { - const email = new EmailService(PLUGIN_ID, this.emailConnector, actionsClient); - this.emailService.resolve(email); - }, - (error) => { - this.logger.warn(`Error starting notification services: ${error}`); - this.emailService.reject(error); - } - ); + let email: EmailService | undefined; + try { + email = getEmailService({ config: this.initialConfig, plugins }); + } catch (err) { + this.logger.warn(`Error creating email service: ${err}`); } - return { - email: this.emailService.promise, - }; + return { email }; } public stop() {} diff --git a/x-pack/plugins/notifications/server/services/email_service.ts b/x-pack/plugins/notifications/server/services/connectors_email_service.ts similarity index 76% rename from x-pack/plugins/notifications/server/services/email_service.ts rename to x-pack/plugins/notifications/server/services/connectors_email_service.ts index 95d29d219998ef..3dbe93197a366e 100755 --- a/x-pack/plugins/notifications/server/services/email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -6,9 +6,9 @@ */ import type { UnsecuredActionsClient } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client'; -import type { IEmailService, PlainTextEmail } from './types'; +import type { EmailService, PlainTextEmail } from './types'; -export class EmailService implements IEmailService { +export class ConnectorsEmailService implements EmailService { constructor( private requesterId: string, private connectorId: string, @@ -18,13 +18,11 @@ export class EmailService implements IEmailService { async sendPlainTextEmail(params: PlainTextEmail): Promise { const actions = params.to.map((to) => ({ id: this.connectorId, - spaceId: 'default', // TODO should be space agnostic? - apiKey: null, // not needed for Email connector - executionId: '???', params: { ...params, to, }, + relatedSavedObjects: params.context?.length ? params.context : undefined, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); } diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts new file mode 100644 index 00000000000000..ee17b74cbbe0d7 --- /dev/null +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts @@ -0,0 +1,46 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { PluginStartContract as ActionsPluginStartContract } from '@kbn/actions-plugin/server'; +import { ConnectorsEmailService } from './connectors_email_service'; +import type { EmailService } from './types'; +import { PLUGIN_ID } from '../../common'; +import type { ConnectorsEmailConfigType } from '../config/connectors_email_config'; + +export interface EmailServiceStartDeps { + actions?: ActionsPluginStartContract; +} + +export interface EmailServiceFactoryParams { + config: ConnectorsEmailConfigType; + plugins: EmailServiceStartDeps; +} + +export function getEmailService(params: EmailServiceFactoryParams): EmailService { + const { + config, + plugins: { actions }, + } = params; + + if (!actions) { + throw new Error(`'actions' plugin not available.`); + } + + const emailConnector = config.connectors?.default?.email; + if (!emailConnector) { + throw new Error('Email connector not specified'); + } + + if (!actions.isPreconfiguredConnector(emailConnector)) { + throw new Error(`Unexisting email connector '${emailConnector}' specified`); + } + + actions.registerUnsecuredActionsClientAccess(PLUGIN_ID); + + const unsecuredActionsClient = actions.getUnsecuredActionsClient(); + return new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient); +} diff --git a/x-pack/plugins/notifications/server/services/index.ts b/x-pack/plugins/notifications/server/services/index.ts index a2e1904028db44..3b06efd0b79374 100644 --- a/x-pack/plugins/notifications/server/services/index.ts +++ b/x-pack/plugins/notifications/server/services/index.ts @@ -5,5 +5,5 @@ * 2.0. */ -export type { IEmailService, PlainTextEmail } from './types'; -export { EmailService } from './email_service'; +export type { EmailService, PlainTextEmail } from './types'; +export { getEmailService, type EmailServiceStartDeps } from './connectors_email_service_factory'; diff --git a/x-pack/plugins/notifications/server/services/types.ts b/x-pack/plugins/notifications/server/services/types.ts index ee8d3063623f98..bee62f76df966b 100755 --- a/x-pack/plugins/notifications/server/services/types.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -5,12 +5,21 @@ * 2.0. */ -export interface IEmailService { +export interface EmailService { sendPlainTextEmail(payload: PlainTextEmail): Promise; } +interface RelatedSavedObject { + id: string; + type: string; + namespace: string; +} + +export type SenderContext = RelatedSavedObject[]; + export interface PlainTextEmail { to: string[]; subject: string; message: string; + context?: SenderContext; } diff --git a/x-pack/plugins/notifications/server/types.ts b/x-pack/plugins/notifications/server/types.ts index c33ffe57119e6b..852331d1b4801d 100755 --- a/x-pack/plugins/notifications/server/types.ts +++ b/x-pack/plugins/notifications/server/types.ts @@ -5,24 +5,10 @@ * 2.0. */ -import type { - PluginSetupContract as ActionsPluginSetupContract, - PluginStartContract as ActionsPluginStartContract, -} from '@kbn/actions-plugin/server'; -import type { IEmailService } from './services/email_service'; +import type { EmailService, EmailServiceStartDeps } from './services'; -export interface NotificationsPluginSetupDeps { - actions: ActionsPluginSetupContract; -} - -export interface NotificationsPluginStartDeps { - actions: ActionsPluginStartContract; -} - -export interface NotificationsPluginSetup { - email: Promise; -} +export type NotificationsPluginStartDeps = EmailServiceStartDeps; export interface NotificationsPluginStart { - email: Promise; + email?: EmailService; } From cd500db0e3ecf016c1c2bb58c852852414fe5a46 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 20 Oct 2022 11:19:49 +0200 Subject: [PATCH 19/38] Add missing tsconfig.json --- x-pack/plugins/notifications/tsconfig.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 x-pack/plugins/notifications/tsconfig.json diff --git a/x-pack/plugins/notifications/tsconfig.json b/x-pack/plugins/notifications/tsconfig.json new file mode 100644 index 00000000000000..54f119e198f2ad --- /dev/null +++ b/x-pack/plugins/notifications/tsconfig.json @@ -0,0 +1,16 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "outDir": "./target/types", + "emitDeclarationOnly": true, + "declaration": true, + "declarationMap": true + }, + "include": [ + "server/**/*", + // have to declare *.json explicitly due to https://github.com/microsoft/TypeScript/issues/25636 + "server/**/*.json", + "public/**/*", + "common/**/*" + ] +} From a2930859e97bbb14840d3ecbbd145934557f9390 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 20 Oct 2022 09:59:34 +0000 Subject: [PATCH 20/38] [CI] Auto-commit changed files from 'node scripts/build_plugin_list_docs' --- docs/developer/plugin-list.asciidoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/developer/plugin-list.asciidoc b/docs/developer/plugin-list.asciidoc index feb87de90c2e6b..a19095ef566562 100644 --- a/docs/developer/plugin-list.asciidoc +++ b/docs/developer/plugin-list.asciidoc @@ -589,6 +589,10 @@ Elastic. |This plugin allows for other plugins to add data to Kibana stack monitoring documents. +|{kib-repo}blob/{branch}/x-pack/plugins/notifications/README.md[notifications] +|The Notifications plugin provides a set of services to help Solutions and plugins send notifications to users. + + |{kib-repo}blob/{branch}/x-pack/plugins/observability/README.md[observability] |This plugin provides shared components and services for use across observability solutions, as well as the observability landing page UI. From e241d0d5d0c1a0d39ed8586d508a6ccc90d4fe3b Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 20 Oct 2022 16:19:17 +0200 Subject: [PATCH 21/38] Add dependency to Actions plugin in tsconfig.json --- x-pack/plugins/notifications/tsconfig.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/plugins/notifications/tsconfig.json b/x-pack/plugins/notifications/tsconfig.json index 54f119e198f2ad..e87642e7454388 100644 --- a/x-pack/plugins/notifications/tsconfig.json +++ b/x-pack/plugins/notifications/tsconfig.json @@ -12,5 +12,8 @@ "server/**/*.json", "public/**/*", "common/**/*" + ], + "references": [ + { "path": "../actions/tsconfig.json" } ] } From 7f358bfd0dea00ed9dd8c14a56febbb5db5737f4 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 20 Oct 2022 18:38:20 +0200 Subject: [PATCH 22/38] Separate setup logic from start logic --- x-pack/plugins/notifications/server/plugin.ts | 26 ++++++++++++---- .../services/connectors_email_service.ts | 3 +- .../connectors_email_service_factory.ts | 30 +++++++++++++++---- .../notifications/server/services/index.ts | 7 ++++- x-pack/plugins/notifications/server/types.ts | 3 +- 5 files changed, 55 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts index 90b5f7f67421a9..bf021af9d7d2be 100755 --- a/x-pack/plugins/notifications/server/plugin.ts +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -5,9 +5,19 @@ * 2.0. */ -import type { PluginInitializerContext, CoreStart, Plugin, Logger } from '@kbn/core/server'; -import type { NotificationsPluginStartDeps, NotificationsPluginStart } from './types'; -import { type EmailService, getEmailService } from './services'; +import type { + PluginInitializerContext, + CoreSetup, + CoreStart, + Plugin, + Logger, +} from '@kbn/core/server'; +import type { + NotificationsPluginSetupDeps, + NotificationsPluginStartDeps, + NotificationsPluginStart, +} from './types'; +import { type EmailService, checkEmailServiceConfiguration, getEmailService } from './services'; import type { NotificationsConfigType } from './config'; export class NotificationsPlugin implements Plugin { @@ -19,14 +29,20 @@ export class NotificationsPlugin implements Plugin ) {} async sendPlainTextEmail(params: PlainTextEmail): Promise { diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts index ee17b74cbbe0d7..69855db3b50d4f 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts @@ -5,14 +5,23 @@ * 2.0. */ -import type { PluginStartContract as ActionsPluginStartContract } from '@kbn/actions-plugin/server'; +import type { PluginSetupContract, PluginStartContract } from '@kbn/actions-plugin/server'; import { ConnectorsEmailService } from './connectors_email_service'; import type { EmailService } from './types'; import { PLUGIN_ID } from '../../common'; import type { ConnectorsEmailConfigType } from '../config/connectors_email_config'; +export interface EmailServiceSetupDeps { + actions?: PluginSetupContract; +} + export interface EmailServiceStartDeps { - actions?: ActionsPluginStartContract; + actions?: PluginStartContract; +} + +export interface CheckEmailServiceParams { + config: ConnectorsEmailConfigType; + plugins: EmailServiceSetupDeps; } export interface EmailServiceFactoryParams { @@ -20,7 +29,7 @@ export interface EmailServiceFactoryParams { plugins: EmailServiceStartDeps; } -export function getEmailService(params: EmailServiceFactoryParams): EmailService { +export function checkEmailServiceConfiguration(params: CheckEmailServiceParams) { const { config, plugins: { actions }, @@ -38,9 +47,18 @@ export function getEmailService(params: EmailServiceFactoryParams): EmailService if (!actions.isPreconfiguredConnector(emailConnector)) { throw new Error(`Unexisting email connector '${emailConnector}' specified`); } +} - actions.registerUnsecuredActionsClientAccess(PLUGIN_ID); +export function getEmailService(params: EmailServiceFactoryParams): EmailService | undefined { + const { + config, + plugins: { actions }, + } = params; - const unsecuredActionsClient = actions.getUnsecuredActionsClient(); - return new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient); + const emailConnector = config.connectors?.default?.email; + const unsecuredActionsClient = actions?.getUnsecuredActionsClient(); + + if (emailConnector && unsecuredActionsClient) { + return new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient); + } } diff --git a/x-pack/plugins/notifications/server/services/index.ts b/x-pack/plugins/notifications/server/services/index.ts index 3b06efd0b79374..5c9194e43995e3 100644 --- a/x-pack/plugins/notifications/server/services/index.ts +++ b/x-pack/plugins/notifications/server/services/index.ts @@ -6,4 +6,9 @@ */ export type { EmailService, PlainTextEmail } from './types'; -export { getEmailService, type EmailServiceStartDeps } from './connectors_email_service_factory'; +export { + type EmailServiceSetupDeps, + type EmailServiceStartDeps, + checkEmailServiceConfiguration, + getEmailService, +} from './connectors_email_service_factory'; diff --git a/x-pack/plugins/notifications/server/types.ts b/x-pack/plugins/notifications/server/types.ts index 852331d1b4801d..96704052b1c237 100755 --- a/x-pack/plugins/notifications/server/types.ts +++ b/x-pack/plugins/notifications/server/types.ts @@ -5,8 +5,9 @@ * 2.0. */ -import type { EmailService, EmailServiceStartDeps } from './services'; +import type { EmailService, EmailServiceSetupDeps, EmailServiceStartDeps } from './services'; +export type NotificationsPluginSetupDeps = EmailServiceSetupDeps; export type NotificationsPluginStartDeps = EmailServiceStartDeps; export interface NotificationsPluginStart { From 89299961f1ac25703ec8250aeddef69b4b916adf Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Mon, 24 Oct 2022 15:45:42 +0200 Subject: [PATCH 23/38] Fix bulkEnqueueExecution params structure --- .../server/services/connectors_email_service.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 3862d86d1add24..1ec7d485e85273 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -20,8 +20,9 @@ export class ConnectorsEmailService implements EmailService { const actions = params.to.map((to) => ({ id: this.connectorId, params: { - ...params, - to, + to: [to], + subject: params.subject, + message: params.message, }, relatedSavedObjects: params.context?.length ? params.context : undefined, })); From fd4b2efe4d9da46c266ef3d6ccc5d01453f45172 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Mon, 24 Oct 2022 15:45:54 +0200 Subject: [PATCH 24/38] Update README --- x-pack/plugins/notifications/README.md | 31 +++++++++++--------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/notifications/README.md b/x-pack/plugins/notifications/README.md index 66f079a395516e..0c615dbda244f6 100755 --- a/x-pack/plugins/notifications/README.md +++ b/x-pack/plugins/notifications/README.md @@ -4,24 +4,17 @@ The Notifications plugin provides a set of services to help Solutions and plugin ## Notifications Plugin public API -### Setup - -The `setup` function exposes the following interface: - -- `email: Promise`: - A service that must be obtained asynchronously, and can be used to send plain text emails. - ### Start The `start` function exposes the following interface: -- `email: Promise`: - A service that must be obtained asynchronously, and can be used to send plain text emails. +- `email: EmailService`: + A simple service that can be used to send plain text emails. ### Usage -To use the exposed plugin start and setup contracts: +To use the exposed plugin start contract: 1. Make sure `notifications` is in your `optionalPlugins` in the `kibana.json` file: @@ -33,7 +26,7 @@ To use the exposed plugin start and setup contracts: } ``` -2. Use the exposed contracts: +2. Use the exposed contract: ```ts // /server/plugin.ts @@ -44,16 +37,18 @@ interface MyPluginStartDeps { } class MyPlugin { - public async start( + public start( core: CoreStart, { notifications }: MyPluginStartDeps ) { - const emailService = await notifications.email; - emailService.sendPlainTextEmail({ - to: 'foo@bar.com', - subject: 'Some subject', - message: 'Hello world!', - }); + const emailService = notifications.email; + if (emailService) { + emailService.sendPlainTextEmail({ + to: 'foo@bar.com', + subject: 'Some subject', + message: 'Hello world!', + }); + } ... } } From c344a61659d900e5729c9470c97312e7ff0f77a8 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Mon, 24 Oct 2022 15:46:13 +0200 Subject: [PATCH 25/38] Add UTs --- .../notifications/server/plugin.test.ts | 137 ++++++++++++++++++ x-pack/plugins/notifications/server/plugin.ts | 14 +- .../services/connectors_email_service.test.ts | 109 ++++++++++++++ .../connectors_email_service_factory.test.ts | 128 ++++++++++++++++ .../connectors_email_service_factory.ts | 4 +- 5 files changed, 387 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugins/notifications/server/plugin.test.ts create mode 100644 x-pack/plugins/notifications/server/services/connectors_email_service.test.ts create mode 100644 x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts diff --git a/x-pack/plugins/notifications/server/plugin.test.ts b/x-pack/plugins/notifications/server/plugin.test.ts new file mode 100644 index 00000000000000..11313b33fc74e2 --- /dev/null +++ b/x-pack/plugins/notifications/server/plugin.test.ts @@ -0,0 +1,137 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { coreMock } from '@kbn/core/server/mocks'; +import { actionsMock } from '@kbn/actions-plugin/server/mocks'; +import { NotificationsConfigType } from './config'; +import { NotificationsPlugin } from './plugin'; + +const missingConnectorConfig = { + connectors: { + default: {}, + }, +}; + +const invalidConnectorConfig = { + connectors: { + default: { + email: 'someUnexistingConnectorId', + }, + }, +}; + +const validConnectorConfig = { + connectors: { + default: { + email: 'validConnectorId', + }, + }, +}; + +const createNotificationsPlugin = (config: NotificationsConfigType) => { + const context = coreMock.createPluginInitializerContext(config); + const plugin = new NotificationsPlugin(context); + const coreSetup = coreMock.createSetup(); + const coreStart = coreMock.createStart(); + + const actionsSetup = actionsMock.createSetup(); + actionsSetup.isPreconfiguredConnector.mockImplementationOnce( + (connectorId) => connectorId === 'validConnectorId' + ); + const pluginSetup = { + actions: actionsSetup, + }; + + const actionsStart = actionsMock.createStart(); + const pluginStart = { + actions: actionsStart, + }; + + return { + context, + logger: context.logger, + plugin, + coreSetup, + coreStart, + actionsSetup, + pluginSetup, + actionsStart, + pluginStart, + }; +}; + +describe('Notifications Plugin', () => { + describe('setup()', () => { + it('should log warning if Actions plugin is not available', () => { + const { plugin, logger, coreSetup } = createNotificationsPlugin({}); + plugin.setup(coreSetup, {}); + expect(logger.get().warn).toHaveBeenCalledWith( + `Email Service Setup Error: 'actions' plugin not available.` + ); + }); + + it('should log warning if no default email connector has been defined', () => { + const { plugin, logger, coreSetup } = createNotificationsPlugin(missingConnectorConfig); + plugin.setup(coreSetup, { + actions: actionsMock.createSetup(), + }); + + expect(logger.get().warn).toHaveBeenCalledWith( + `Email Service Setup Error: Email connector not specified.` + ); + }); + + it('should log warning if the specified email connector is not a preconfigured connector', () => { + const { plugin, logger, coreSetup, pluginSetup } = + createNotificationsPlugin(invalidConnectorConfig); + plugin.setup(coreSetup, pluginSetup); + expect(logger.get().warn).toHaveBeenCalledWith( + `Email Service Setup Error: Unexisting email connector 'someUnexistingConnectorId' specified.` + ); + }); + + it('should not log warning if actions plugin is defined and the specified email connector is valid', () => { + const { plugin, logger, coreSetup, pluginSetup } = + createNotificationsPlugin(validConnectorConfig); + plugin.setup(coreSetup, pluginSetup); + expect(logger.get().warn).not.toHaveBeenCalled(); + }); + }); + + describe('start()', () => { + it('should not return an Email service if Actions plugin is not available', () => { + const { plugin, coreSetup, coreStart } = createNotificationsPlugin(validConnectorConfig); + plugin.setup(coreSetup, {}); + const { email } = plugin.start(coreStart, {}); + expect(email).toBe(undefined); + }); + + it('should not return an Email service if no default email connector has been defined', () => { + const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = + createNotificationsPlugin(missingConnectorConfig); + plugin.setup(coreSetup, pluginSetup); + const { email } = plugin.start(coreStart, pluginStart); + expect(email).toBe(undefined); + }); + + it('should not return an Email service if the specified email connector is not a preconfigured connector', () => { + const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = + createNotificationsPlugin(invalidConnectorConfig); + plugin.setup(coreSetup, pluginSetup); + const { email } = plugin.start(coreStart, pluginStart); + expect(email).toBe(undefined); + }); + + it('should return an Email service if actions plugin is defined and the specified email connector is valid', () => { + const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = + createNotificationsPlugin(validConnectorConfig); + plugin.setup(coreSetup, pluginSetup); + const { email } = plugin.start(coreStart, pluginStart); + expect(email?.sendPlainTextEmail).toBeInstanceOf(Function); + }); + }); +}); diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts index bf021af9d7d2be..5e4dc4ca0f86f7 100755 --- a/x-pack/plugins/notifications/server/plugin.ts +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -23,24 +23,32 @@ import type { NotificationsConfigType } from './config'; export class NotificationsPlugin implements Plugin { private readonly logger: Logger; private readonly initialConfig: NotificationsConfigType; + private emailServiceSetupSuccessful: boolean; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); this.initialConfig = initializerContext.config.get(); + this.emailServiceSetupSuccessful = false; } public setup(core: CoreSetup, plugins: NotificationsPluginSetupDeps) { try { - checkEmailServiceConfiguration({ config: this.initialConfig, plugins }); + checkEmailServiceConfiguration({ + config: this.initialConfig, + plugins, + }); + this.emailServiceSetupSuccessful = true; } catch (err) { - this.logger.warn(`Email service setup: ${err}`); + this.logger.warn(`Email Service Setup ${err}`); } } public start(core: CoreStart, plugins: NotificationsPluginStartDeps) { let email: EmailService | undefined; try { - email = getEmailService({ config: this.initialConfig, plugins }); + if (this.emailServiceSetupSuccessful) { + email = getEmailService({ config: this.initialConfig, plugins }); + } } catch (err) { this.logger.warn(`Error starting email service: ${err}`); } 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 new file mode 100644 index 00000000000000..2aa9bdbbd0666c --- /dev/null +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.test.ts @@ -0,0 +1,109 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { unsecuredActionsClientMock } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client.mock'; +import { ConnectorsEmailService } from './connectors_email_service'; +import { PlainTextEmail } from './types'; + +const REQUESTER_ID = 'requesterId'; +const CONNECTOR_ID = 'connectorId'; + +describe('sendPlainTextEmail()', () => { + describe('calls the provided ActionsClient#bulkEnqueueExecution() with the appropriate params', () => { + it(`omits the 'relatedSavedObjects' field if no context is provided`, () => { + const actionsClient = unsecuredActionsClientMock.create(); + const email = new ConnectorsEmailService(REQUESTER_ID, CONNECTOR_ID, actionsClient); + const payload: PlainTextEmail = { + to: ['user1@email.com'], + subject: 'This is a notification email', + message: 'With some contents inside.', + }; + + email.sendPlainTextEmail(payload); + + expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1); + expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledWith(REQUESTER_ID, [ + { + id: CONNECTOR_ID, + params: { + to: ['user1@email.com'], + subject: 'This is a notification email', + message: 'With some contents inside.', + }, + }, + ]); + }); + + it(`populates the 'relatedSavedObjects' field if context is provided`, () => { + const actionsClient = unsecuredActionsClientMock.create(); + const email = new ConnectorsEmailService(REQUESTER_ID, CONNECTOR_ID, actionsClient); + const payload: PlainTextEmail = { + to: ['user1@email.com', 'user2@email.com', 'user3@email.com'], + subject: 'This is a notification email', + message: 'With some contents inside.', + context: [ + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'some-space', + }, + ], + }; + + email.sendPlainTextEmail(payload); + + expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1); + expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledWith(REQUESTER_ID, [ + { + id: CONNECTOR_ID, + params: { + to: ['user1@email.com'], + subject: 'This is a notification email', + message: 'With some contents inside.', + }, + relatedSavedObjects: [ + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'some-space', + }, + ], + }, + { + id: CONNECTOR_ID, + params: { + to: ['user2@email.com'], + subject: 'This is a notification email', + message: 'With some contents inside.', + }, + relatedSavedObjects: [ + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'some-space', + }, + ], + }, + { + id: CONNECTOR_ID, + params: { + to: ['user3@email.com'], + subject: 'This is a notification email', + message: 'With some contents inside.', + }, + relatedSavedObjects: [ + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'some-space', + }, + ], + }, + ]); + }); + }); +}); diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts new file mode 100644 index 00000000000000..064979d7d0f31b --- /dev/null +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { actionsMock } from '@kbn/actions-plugin/server/mocks'; +import { ConnectorsEmailService } from './connectors_email_service'; +import { + checkEmailServiceConfiguration, + CheckEmailServiceParams, + EmailServiceFactoryParams, + getEmailService, +} from './connectors_email_service_factory'; + +const missingConnectorConfig = { + connectors: { + default: {}, + }, +}; + +const invalidConnectorConfig = { + connectors: { + default: { + email: 'someUnexistingConnectorId', + }, + }, +}; + +const validConnectorConfig = { + connectors: { + default: { + email: 'validConnectorId', + }, + }, +}; + +describe('checkEmailServiceConfiguration()', () => { + it('should throw an Error if Actions plugin is not available', () => { + expect(() => { + const params: CheckEmailServiceParams = { + config: validConnectorConfig, + plugins: {}, + }; + checkEmailServiceConfiguration(params); + }).toThrowErrorMatchingInlineSnapshot(`"'actions' plugin not available."`); + }); + + it('should throw an Error if no default email connector has been defined', () => { + expect(() => { + const params: CheckEmailServiceParams = { + config: missingConnectorConfig, + plugins: { + actions: actionsMock.createSetup(), + }, + }; + checkEmailServiceConfiguration(params); + }).toThrowErrorMatchingInlineSnapshot(`"Email connector not specified."`); + }); + + it('should throw an Error if the specified email connector is not a preconfigured connector', () => { + expect(() => { + const actions = actionsMock.createSetup(); + actions.isPreconfiguredConnector.mockImplementationOnce( + (connectorId) => connectorId === 'validConnectorId' + ); + const params: CheckEmailServiceParams = { + config: invalidConnectorConfig, + plugins: { + actions, + }, + }; + checkEmailServiceConfiguration(params); + }).toThrowErrorMatchingInlineSnapshot( + `"Unexisting email connector 'someUnexistingConnectorId' specified."` + ); + }); + + it('should not throw an Error if actions plugin is defined and the specified email connector is valid', () => { + expect(() => { + const actions = actionsMock.createSetup(); + actions.isPreconfiguredConnector.mockImplementationOnce( + (connectorId) => connectorId === 'validConnectorId' + ); + const params: CheckEmailServiceParams = { + config: validConnectorConfig, + plugins: { + actions, + }, + }; + checkEmailServiceConfiguration(params); + }).not.toThrowError(); + }); +}); + +describe('getEmailService()', () => { + it('returns undefined if Actions plugin start contract is not available', () => { + const params: EmailServiceFactoryParams = { + config: validConnectorConfig, + plugins: {}, + }; + const email = getEmailService(params); + expect(email).toBeUndefined(); + }); + + it('returns undefined if default connector has not been specified', () => { + const params: EmailServiceFactoryParams = { + config: missingConnectorConfig, + plugins: { + actions: actionsMock.createStart(), + }, + }; + const email = getEmailService(params); + expect(email).toBeUndefined(); + }); + + it('returns an instance of ConnectorsEmailService if all params have been specified', () => { + const params: EmailServiceFactoryParams = { + config: invalidConnectorConfig, // note that the factory does not check for connector validity + plugins: { + actions: actionsMock.createStart(), + }, + }; + const email = getEmailService(params); + expect(email).toBeInstanceOf(ConnectorsEmailService); + }); +}); diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts index 69855db3b50d4f..cf69ae16d50d78 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts @@ -41,11 +41,11 @@ export function checkEmailServiceConfiguration(params: CheckEmailServiceParams) const emailConnector = config.connectors?.default?.email; if (!emailConnector) { - throw new Error('Email connector not specified'); + throw new Error('Email connector not specified.'); } if (!actions.isPreconfiguredConnector(emailConnector)) { - throw new Error(`Unexisting email connector '${emailConnector}' specified`); + throw new Error(`Unexisting email connector '${emailConnector}' specified.`); } } From 53575d9dca600bea8e6fb103255bb7b869f77096 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Tue, 25 Oct 2022 15:05:07 +0200 Subject: [PATCH 26/38] Check license type >platinum for email notifications --- x-pack/plugins/notifications/kibana.json | 2 +- .../notifications/server/plugin.test.ts | 88 ++++++------ x-pack/plugins/notifications/server/plugin.ts | 10 +- .../connectors_email_service_factory.test.ts | 34 +++-- .../connectors_email_service_factory.ts | 26 +++- .../services/licensed_email_service.test.ts | 128 ++++++++++++++++++ .../server/services/licensed_email_service.ts | 48 +++++++ 7 files changed, 273 insertions(+), 63 deletions(-) create mode 100644 x-pack/plugins/notifications/server/services/licensed_email_service.test.ts create mode 100644 x-pack/plugins/notifications/server/services/licensed_email_service.ts diff --git a/x-pack/plugins/notifications/kibana.json b/x-pack/plugins/notifications/kibana.json index 04720bf3f481d1..45cf4c4cd47b0c 100755 --- a/x-pack/plugins/notifications/kibana.json +++ b/x-pack/plugins/notifications/kibana.json @@ -7,6 +7,6 @@ "version": "kibana", "server": true, "ui": false, - "requiredPlugins": ["actions"], + "requiredPlugins": ["actions", "licensing"], "optionalPlugins": [] } diff --git a/x-pack/plugins/notifications/server/plugin.test.ts b/x-pack/plugins/notifications/server/plugin.test.ts index 11313b33fc74e2..4ade4dd3a2e742 100644 --- a/x-pack/plugins/notifications/server/plugin.test.ts +++ b/x-pack/plugins/notifications/server/plugin.test.ts @@ -9,12 +9,15 @@ import { coreMock } from '@kbn/core/server/mocks'; import { actionsMock } from '@kbn/actions-plugin/server/mocks'; import { NotificationsConfigType } from './config'; import { NotificationsPlugin } from './plugin'; +import * as emailServiceFactory from './services/connectors_email_service_factory'; +import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; -const missingConnectorConfig = { - connectors: { - default: {}, - }, -}; +const checkEmailServiceConfigurationSpy = jest.spyOn( + emailServiceFactory, + 'checkEmailServiceConfiguration' +); + +const getEmailServiceSpy = jest.spyOn(emailServiceFactory, 'getEmailService'); const invalidConnectorConfig = { connectors: { @@ -44,11 +47,13 @@ const createNotificationsPlugin = (config: NotificationsConfigType) => { ); const pluginSetup = { actions: actionsSetup, + licensing: licensingMock.createSetup(), }; const actionsStart = actionsMock.createStart(); const pluginStart = { actions: actionsStart, + licensing: licensingMock.createStart(), }; return { @@ -66,71 +71,74 @@ const createNotificationsPlugin = (config: NotificationsConfigType) => { describe('Notifications Plugin', () => { describe('setup()', () => { - it('should log warning if Actions plugin is not available', () => { - const { plugin, logger, coreSetup } = createNotificationsPlugin({}); - plugin.setup(coreSetup, {}); - expect(logger.get().warn).toHaveBeenCalledWith( - `Email Service Setup Error: 'actions' plugin not available.` - ); - }); + beforeEach(() => checkEmailServiceConfigurationSpy.mockClear()); - it('should log warning if no default email connector has been defined', () => { - const { plugin, logger, coreSetup } = createNotificationsPlugin(missingConnectorConfig); - plugin.setup(coreSetup, { + it('should call checkEmailServiceConfiguration(), catch Exceptions and log a warn', () => { + const { plugin, logger, coreSetup } = createNotificationsPlugin(validConnectorConfig); + const pluginSetup = { actions: actionsMock.createSetup(), + }; + plugin.setup(coreSetup, pluginSetup); + expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledTimes(1); + expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledWith({ + config: validConnectorConfig, + plugins: pluginSetup, }); - expect(logger.get().warn).toHaveBeenCalledWith( - `Email Service Setup Error: Email connector not specified.` + `Email Service Setup Error: 'actions' and 'licensing' plugins are required.` ); - }); - it('should log warning if the specified email connector is not a preconfigured connector', () => { - const { plugin, logger, coreSetup, pluginSetup } = - createNotificationsPlugin(invalidConnectorConfig); - plugin.setup(coreSetup, pluginSetup); - expect(logger.get().warn).toHaveBeenCalledWith( - `Email Service Setup Error: Unexisting email connector 'someUnexistingConnectorId' specified.` - ); + // eslint-disable-next-line dot-notation + expect(plugin['emailServiceSetupSuccessful']).toEqual(false); }); - it('should not log warning if actions plugin is defined and the specified email connector is valid', () => { + it('should pass the setup checks successfully and not log any warning if the configuration is correct', () => { const { plugin, logger, coreSetup, pluginSetup } = createNotificationsPlugin(validConnectorConfig); plugin.setup(coreSetup, pluginSetup); + expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledTimes(1); + expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledWith({ + config: validConnectorConfig, + plugins: pluginSetup, + }); expect(logger.get().warn).not.toHaveBeenCalled(); + + // eslint-disable-next-line dot-notation + expect(plugin['emailServiceSetupSuccessful']).toEqual(true); }); }); describe('start()', () => { - it('should not return an Email service if Actions plugin is not available', () => { - const { plugin, coreSetup, coreStart } = createNotificationsPlugin(validConnectorConfig); - plugin.setup(coreSetup, {}); - const { email } = plugin.start(coreStart, {}); - expect(email).toBe(undefined); - }); + beforeEach(() => getEmailServiceSpy.mockClear()); - it('should not return an Email service if no default email connector has been defined', () => { - const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = - createNotificationsPlugin(missingConnectorConfig); - plugin.setup(coreSetup, pluginSetup); + it('should not call getEmailService() if the setup() has not been run', () => { + const { plugin, coreStart, pluginStart } = createNotificationsPlugin(validConnectorConfig); const { email } = plugin.start(coreStart, pluginStart); - expect(email).toBe(undefined); + expect(getEmailServiceSpy).not.toHaveBeenCalled(); + expect(email).toBeUndefined(); }); - it('should not return an Email service if the specified email connector is not a preconfigured connector', () => { + it('should not call getEmailService() if setup() has failed', () => { const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = createNotificationsPlugin(invalidConnectorConfig); plugin.setup(coreSetup, pluginSetup); const { email } = plugin.start(coreStart, pluginStart); - expect(email).toBe(undefined); + expect(getEmailServiceSpy).not.toHaveBeenCalled(); + expect(email).toBeUndefined(); }); - it('should return an Email service if actions plugin is defined and the specified email connector is valid', () => { + it('should call getEmailService() if setup() was successful', () => { const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = createNotificationsPlugin(validConnectorConfig); plugin.setup(coreSetup, pluginSetup); const { email } = plugin.start(coreStart, pluginStart); + expect(getEmailServiceSpy).toHaveBeenCalledTimes(1); + expect(getEmailServiceSpy).toHaveBeenCalledWith({ + config: validConnectorConfig, + plugins: pluginStart, + logger: expect.any(Object), + }); + expect(email).toEqual(getEmailServiceSpy.mock.results[0].value); expect(email?.sendPlainTextEmail).toBeInstanceOf(Function); }); }); diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts index 5e4dc4ca0f86f7..f8a01f73cbd957 100755 --- a/x-pack/plugins/notifications/server/plugin.ts +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -31,7 +31,7 @@ export class NotificationsPlugin implements Plugin { - it('should throw an Error if Actions plugin is not available', () => { + it('should throw an Error if Actions or Licensing plugins are not available', () => { expect(() => { const params: CheckEmailServiceParams = { config: validConnectorConfig, - plugins: {}, + plugins: { + actions: actionsMock.createSetup(), + }, }; checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot(`"'actions' plugin not available."`); + }).toThrowErrorMatchingInlineSnapshot(`"'actions' and 'licensing' plugins are required."`); }); it('should throw an Error if no default email connector has been defined', () => { @@ -56,7 +60,7 @@ describe('checkEmailServiceConfiguration()', () => { }, }; checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot(`"Email connector not specified."`); + }).toThrowErrorMatchingInlineSnapshot(`"'actions' and 'licensing' plugins are required."`); }); it('should throw an Error if the specified email connector is not a preconfigured connector', () => { @@ -72,12 +76,10 @@ describe('checkEmailServiceConfiguration()', () => { }, }; checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot( - `"Unexisting email connector 'someUnexistingConnectorId' specified."` - ); + }).toThrowErrorMatchingInlineSnapshot(`"'actions' and 'licensing' plugins are required."`); }); - it('should not throw an Error if actions plugin is defined and the specified email connector is valid', () => { + it('should not throw an Error if required plugins are present and the specified email connector is valid', () => { expect(() => { const actions = actionsMock.createSetup(); actions.isPreconfiguredConnector.mockImplementationOnce( @@ -87,6 +89,7 @@ describe('checkEmailServiceConfiguration()', () => { config: validConnectorConfig, plugins: { actions, + licensing: licensingMock.createSetup(), }, }; checkEmailServiceConfiguration(params); @@ -95,10 +98,13 @@ describe('checkEmailServiceConfiguration()', () => { }); describe('getEmailService()', () => { - it('returns undefined if Actions plugin start contract is not available', () => { + it('returns undefined if Actions or Licensing plugins are not available', () => { const params: EmailServiceFactoryParams = { config: validConnectorConfig, - plugins: {}, + plugins: { + licensing: licensingMock.createStart(), + }, + logger: loggerMock.create(), }; const email = getEmailService(params); expect(email).toBeUndefined(); @@ -109,7 +115,9 @@ describe('getEmailService()', () => { config: missingConnectorConfig, plugins: { actions: actionsMock.createStart(), + licensing: licensingMock.createStart(), }, + logger: loggerMock.create(), }; const email = getEmailService(params); expect(email).toBeUndefined(); @@ -120,9 +128,11 @@ describe('getEmailService()', () => { config: invalidConnectorConfig, // note that the factory does not check for connector validity plugins: { actions: actionsMock.createStart(), + licensing: licensingMock.createStart(), }, + logger: loggerMock.create(), }; const email = getEmailService(params); - expect(email).toBeInstanceOf(ConnectorsEmailService); + expect(email).toBeInstanceOf(LicensedEmailService); }); }); diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts index cf69ae16d50d78..42d299dfe227bc 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts @@ -5,18 +5,23 @@ * 2.0. */ +import { Logger } from '@kbn/logging'; import type { PluginSetupContract, PluginStartContract } from '@kbn/actions-plugin/server'; +import { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing-plugin/server'; import { ConnectorsEmailService } from './connectors_email_service'; import type { EmailService } from './types'; import { PLUGIN_ID } from '../../common'; import type { ConnectorsEmailConfigType } from '../config/connectors_email_config'; +import { LicensedEmailService } from './licensed_email_service'; export interface EmailServiceSetupDeps { actions?: PluginSetupContract; + licensing?: LicensingPluginSetup; } export interface EmailServiceStartDeps { actions?: PluginStartContract; + licensing?: LicensingPluginStart; } export interface CheckEmailServiceParams { @@ -27,16 +32,17 @@ export interface CheckEmailServiceParams { export interface EmailServiceFactoryParams { config: ConnectorsEmailConfigType; plugins: EmailServiceStartDeps; + logger: Logger; } export function checkEmailServiceConfiguration(params: CheckEmailServiceParams) { const { config, - plugins: { actions }, + plugins: { actions, licensing }, } = params; - if (!actions) { - throw new Error(`'actions' plugin not available.`); + if (!actions || !licensing) { + throw new Error(`'actions' and 'licensing' plugins are required.`); } const emailConnector = config.connectors?.default?.email; @@ -52,13 +58,19 @@ export function checkEmailServiceConfiguration(params: CheckEmailServiceParams) export function getEmailService(params: EmailServiceFactoryParams): EmailService | undefined { const { config, - plugins: { actions }, + plugins: { actions, licensing }, + logger, } = params; - const emailConnector = config.connectors?.default?.email; const unsecuredActionsClient = actions?.getUnsecuredActionsClient(); + const emailConnector = config.connectors?.default?.email; - if (emailConnector && unsecuredActionsClient) { - return new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient); + if (licensing && unsecuredActionsClient && emailConnector) { + return new LicensedEmailService( + new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient), + licensing.license$, + 'platinum', + logger + ); } } diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts new file mode 100644 index 00000000000000..f88705e29563ee --- /dev/null +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Subject } from 'rxjs'; +import { licenseMock } from '@kbn/licensing-plugin/common/licensing.mock'; +import { loggerMock } from '@kbn/logging-mocks'; +import { LicensedEmailService } from './licensed_email_service'; +import { ILicense } from '@kbn/licensing-plugin/server'; +import { EmailService, PlainTextEmail } from './types'; + +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +const emailServiceMock: EmailService = { + sendPlainTextEmail: jest.fn(), +}; + +const validLicense = licenseMock.createLicenseMock(); +const invalidLicense = licenseMock.createLicenseMock(); +invalidLicense.type = 'basic'; +invalidLicense.check = jest.fn(() => ({ + state: 'invalid', + message: 'This is an invalid testing license', +})) as unknown as any; + +const someEmail: PlainTextEmail = { + to: ['user1@email.com'], + subject: 'Some subject', + message: 'Some message', +}; + +describe('LicensedEmailService', () => { + it('observes license$ changes and logs info or warning messages accordingly', () => { + const license$ = new Subject(); + const logger = loggerMock.create(); + + new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); + + license$.next(invalidLicense); + + expect(logger.info).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith('This is an invalid testing license'); + + license$.next(validLicense); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.info).toHaveBeenCalledTimes(1); + expect(logger.info).toHaveBeenCalledWith( + 'Your current license allows sending email notifications' + ); + }); + + describe('sendPlainTextEmail()', () => { + it('does not call the underlying email service until the license is determined and valid', async () => { + const license$ = new Subject(); + const logger = loggerMock.create(); + const email = new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); + + email.sendPlainTextEmail(someEmail); + expect(emailServiceMock.sendPlainTextEmail).not.toHaveBeenCalled(); + license$.next(validLicense); + + await delay(1); + + expect(emailServiceMock.sendPlainTextEmail).toHaveBeenCalledTimes(1); + expect(emailServiceMock.sendPlainTextEmail).toHaveBeenCalledWith(someEmail); + }); + + it('does not call the underlying email service if the license is invalid', async () => { + const license$ = new Subject(); + const logger = loggerMock.create(); + const email = new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); + license$.next(invalidLicense); + + try { + await email.sendPlainTextEmail(someEmail); + } catch (err) { + expect(err.message).toEqual( + 'The current license does not allow sending email notifications' + ); + return; + } + + expect('it should have thrown').toEqual('but it did not'); + }); + + it('does not log a warning for every email attempt, but rather for every license change', async () => { + const license$ = new Subject(); + const logger = loggerMock.create(); + const email = new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); + license$.next(invalidLicense); + license$.next(validLicense); + license$.next(invalidLicense); + + expect(logger.info).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledTimes(2); + + let emailsOk = 0; + let emailsKo = 0; + const silentSend = async () => { + try { + await email.sendPlainTextEmail(someEmail); + emailsOk++; + } catch (err) { + emailsKo++; + } + }; + + await silentSend(); + await silentSend(); + await silentSend(); + await silentSend(); + license$.next(validLicense); + await silentSend(); + await silentSend(); + await silentSend(); + await silentSend(); + + expect(logger.info).toHaveBeenCalledTimes(2); + expect(logger.warn).toHaveBeenCalledTimes(2); + expect(emailsKo).toEqual(4); + expect(emailsOk).toEqual(4); + }); + }); +}); diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.ts new file mode 100644 index 00000000000000..d72f8ebf5d9cd9 --- /dev/null +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Logger } from '@kbn/logging'; +import { ILicense, LicenseType } from '@kbn/licensing-plugin/server'; +import { firstValueFrom, map, Observable, ReplaySubject, Subject } from 'rxjs'; +import { EmailService, PlainTextEmail } from './types'; +import { PLUGIN_ID } from '../../common'; + +export class LicensedEmailService implements EmailService { + private validLicense$: Subject = new ReplaySubject(1); + + constructor( + private emailService: EmailService, + license$: Observable, + private minimumLicense: LicenseType, + private logger: Logger + ) { + // no need to explicitly unsubscribe as the license$ observable already completes on stop() + license$.pipe(map((license) => this.checkValidLicense(license))).subscribe(this.validLicense$); + } + + async sendPlainTextEmail(payload: PlainTextEmail): Promise { + if (await firstValueFrom(this.validLicense$, { defaultValue: false })) { + await this.emailService.sendPlainTextEmail(payload); + } else { + throw new Error('The current license does not allow sending email notifications'); + } + } + + private checkValidLicense(license: ILicense): boolean { + const licenseCheck = license.check(PLUGIN_ID, this.minimumLicense); + + if (licenseCheck.state === 'valid') { + this.logger.info('Your current license allows sending email notifications'); + return true; + } + + this.logger.warn( + licenseCheck.message || 'The current license does not allow sending email notifications' + ); + return false; + } +} From f2143247a504ab9ac7f33f428d88c5aed6fc572c Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Wed, 26 Oct 2022 11:12:01 +0200 Subject: [PATCH 27/38] Fix incorrect UTs --- .../services/connectors_email_service_factory.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts index 5b2a2de67857bc..cd0ca3f9535386 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts @@ -57,10 +57,11 @@ describe('checkEmailServiceConfiguration()', () => { config: missingConnectorConfig, plugins: { actions: actionsMock.createSetup(), + licensing: licensingMock.createSetup(), }, }; checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot(`"'actions' and 'licensing' plugins are required."`); + }).toThrowErrorMatchingInlineSnapshot(`"Email connector not specified."`); }); it('should throw an Error if the specified email connector is not a preconfigured connector', () => { @@ -73,10 +74,13 @@ describe('checkEmailServiceConfiguration()', () => { config: invalidConnectorConfig, plugins: { actions, + licensing: licensingMock.createSetup(), }, }; checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot(`"'actions' and 'licensing' plugins are required."`); + }).toThrowErrorMatchingInlineSnapshot( + `"Unexisting email connector 'someUnexistingConnectorId' specified."` + ); }); it('should not throw an Error if required plugins are present and the specified email connector is valid', () => { From e0dd0f7dd4fa8c10ad880e23fba26cd99802e39d Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Wed, 26 Oct 2022 11:17:25 +0200 Subject: [PATCH 28/38] Import types when possible --- x-pack/plugins/notifications/server/config/config.ts | 2 +- .../server/config/connectors_email_config.ts | 4 ++-- x-pack/plugins/notifications/server/plugin.test.ts | 2 +- .../server/services/connectors_email_service.test.ts | 2 +- .../server/services/connectors_email_service.ts | 2 +- .../services/connectors_email_service_factory.test.ts | 4 ++-- .../server/services/connectors_email_service_factory.ts | 4 ++-- .../server/services/licensed_email_service.test.ts | 4 ++-- .../server/services/licensed_email_service.ts | 8 ++++---- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/notifications/server/config/config.ts b/x-pack/plugins/notifications/server/config/config.ts index c70f235eab422d..f2967b6f59fd83 100644 --- a/x-pack/plugins/notifications/server/config/config.ts +++ b/x-pack/plugins/notifications/server/config/config.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { PluginConfigDescriptor } from '@kbn/core/server'; +import type { PluginConfigDescriptor } from '@kbn/core/server'; import { configSchema, type ConnectorsEmailConfigType } from './connectors_email_config'; export type NotificationsConfigType = ConnectorsEmailConfigType; diff --git a/x-pack/plugins/notifications/server/config/connectors_email_config.ts b/x-pack/plugins/notifications/server/config/connectors_email_config.ts index 6337900d8d32d2..3bd6ead9451b06 100644 --- a/x-pack/plugins/notifications/server/config/connectors_email_config.ts +++ b/x-pack/plugins/notifications/server/config/connectors_email_config.ts @@ -5,8 +5,8 @@ * 2.0. */ -import { schema, TypeOf } from '@kbn/config-schema'; -import { PluginConfigDescriptor } from '@kbn/core/server'; +import { schema, type TypeOf } from '@kbn/config-schema'; +import type { PluginConfigDescriptor } from '@kbn/core/server'; export const configSchema = schema.object( { diff --git a/x-pack/plugins/notifications/server/plugin.test.ts b/x-pack/plugins/notifications/server/plugin.test.ts index 4ade4dd3a2e742..51990d79d8a28b 100644 --- a/x-pack/plugins/notifications/server/plugin.test.ts +++ b/x-pack/plugins/notifications/server/plugin.test.ts @@ -7,7 +7,7 @@ import { coreMock } from '@kbn/core/server/mocks'; import { actionsMock } from '@kbn/actions-plugin/server/mocks'; -import { NotificationsConfigType } from './config'; +import type { NotificationsConfigType } from './config'; import { NotificationsPlugin } from './plugin'; import * as emailServiceFactory from './services/connectors_email_service_factory'; import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; 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 2aa9bdbbd0666c..659fae2b26891b 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 @@ -7,7 +7,7 @@ import { unsecuredActionsClientMock } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client.mock'; import { ConnectorsEmailService } from './connectors_email_service'; -import { PlainTextEmail } from './types'; +import type { PlainTextEmail } from './types'; const REQUESTER_ID = 'requesterId'; const CONNECTOR_ID = 'connectorId'; 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 1ec7d485e85273..300d594e3c70ed 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -6,7 +6,7 @@ */ import type { UnsecuredActionsClient } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client'; -import { PublicMethodsOf } from '@kbn/utility-types'; +import type { PublicMethodsOf } from '@kbn/utility-types'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts index cd0ca3f9535386..3a9bef3241569e 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts @@ -10,8 +10,8 @@ import { actionsMock } from '@kbn/actions-plugin/server/mocks'; import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; import { checkEmailServiceConfiguration, - CheckEmailServiceParams, - EmailServiceFactoryParams, + type CheckEmailServiceParams, + type EmailServiceFactoryParams, getEmailService, } from './connectors_email_service_factory'; import { LicensedEmailService } from './licensed_email_service'; diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts index 42d299dfe227bc..d6f0a177748c73 100644 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts @@ -5,9 +5,9 @@ * 2.0. */ -import { Logger } from '@kbn/logging'; +import type { Logger } from '@kbn/logging'; import type { PluginSetupContract, PluginStartContract } from '@kbn/actions-plugin/server'; -import { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing-plugin/server'; +import type { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing-plugin/server'; import { ConnectorsEmailService } from './connectors_email_service'; import type { EmailService } from './types'; import { PLUGIN_ID } from '../../common'; diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts index f88705e29563ee..5be91e791cc4a7 100644 --- a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts @@ -9,8 +9,8 @@ import { Subject } from 'rxjs'; import { licenseMock } from '@kbn/licensing-plugin/common/licensing.mock'; import { loggerMock } from '@kbn/logging-mocks'; import { LicensedEmailService } from './licensed_email_service'; -import { ILicense } from '@kbn/licensing-plugin/server'; -import { EmailService, PlainTextEmail } from './types'; +import type { ILicense } from '@kbn/licensing-plugin/server'; +import type { EmailService, PlainTextEmail } from './types'; const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.ts index d72f8ebf5d9cd9..3f847c97226ac0 100644 --- a/x-pack/plugins/notifications/server/services/licensed_email_service.ts +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.ts @@ -5,10 +5,10 @@ * 2.0. */ -import { Logger } from '@kbn/logging'; -import { ILicense, LicenseType } from '@kbn/licensing-plugin/server'; -import { firstValueFrom, map, Observable, ReplaySubject, Subject } from 'rxjs'; -import { EmailService, PlainTextEmail } from './types'; +import type { Logger } from '@kbn/logging'; +import type { ILicense, LicenseType } from '@kbn/licensing-plugin/server'; +import { firstValueFrom, map, type Observable, ReplaySubject, type Subject } from 'rxjs'; +import type { EmailService, PlainTextEmail } from './types'; import { PLUGIN_ID } from '../../common'; export class LicensedEmailService implements EmailService { From 8218cc4e44c444f05ba7b441a4e95f5f73e2d872 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Wed, 26 Oct 2022 12:57:10 +0200 Subject: [PATCH 29/38] Misc enhancements and code cleanup --- .../services/connectors_email_service.test.ts | 16 +++++++++------- .../services/connectors_email_service.ts | 4 +++- .../services/licensed_email_service.test.ts | 19 ++++++++----------- .../server/services/licensed_email_service.ts | 2 +- .../notifications/server/services/types.ts | 6 +++--- 5 files changed, 24 insertions(+), 23 deletions(-) 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 659fae2b26891b..ece46977d62e3f 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 @@ -45,13 +45,15 @@ describe('sendPlainTextEmail()', () => { to: ['user1@email.com', 'user2@email.com', 'user3@email.com'], subject: 'This is a notification email', message: 'With some contents inside.', - context: [ - { - id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', - type: 'cases', - namespace: 'some-space', - }, - ], + context: { + relatedObjects: [ + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'some-space', + }, + ], + }, }; email.sendPlainTextEmail(payload); 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 300d594e3c70ed..756f37ca587861 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -24,7 +24,9 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - relatedSavedObjects: params.context?.length ? params.context : undefined, + relatedSavedObjects: params.context?.relatedObjects?.length + ? params.context.relatedObjects + : undefined, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); } diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts index 5be91e791cc4a7..811ab3eb71f055 100644 --- a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts @@ -33,22 +33,22 @@ const someEmail: PlainTextEmail = { }; describe('LicensedEmailService', () => { + const logger = loggerMock.create(); + + beforeEach(() => loggerMock.clear(logger)); it('observes license$ changes and logs info or warning messages accordingly', () => { const license$ = new Subject(); - const logger = loggerMock.create(); - new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); - license$.next(invalidLicense); - expect(logger.info).not.toHaveBeenCalled(); + expect(logger.debug).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith('This is an invalid testing license'); license$.next(validLicense); expect(logger.warn).toHaveBeenCalledTimes(1); - expect(logger.info).toHaveBeenCalledTimes(1); - expect(logger.info).toHaveBeenCalledWith( + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledWith( 'Your current license allows sending email notifications' ); }); @@ -56,7 +56,6 @@ describe('LicensedEmailService', () => { describe('sendPlainTextEmail()', () => { it('does not call the underlying email service until the license is determined and valid', async () => { const license$ = new Subject(); - const logger = loggerMock.create(); const email = new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); email.sendPlainTextEmail(someEmail); @@ -71,7 +70,6 @@ describe('LicensedEmailService', () => { it('does not call the underlying email service if the license is invalid', async () => { const license$ = new Subject(); - const logger = loggerMock.create(); const email = new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); license$.next(invalidLicense); @@ -89,13 +87,12 @@ describe('LicensedEmailService', () => { it('does not log a warning for every email attempt, but rather for every license change', async () => { const license$ = new Subject(); - const logger = loggerMock.create(); const email = new LicensedEmailService(emailServiceMock, license$, 'platinum', logger); license$.next(invalidLicense); license$.next(validLicense); license$.next(invalidLicense); - expect(logger.info).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledTimes(2); let emailsOk = 0; @@ -119,7 +116,7 @@ describe('LicensedEmailService', () => { await silentSend(); await silentSend(); - expect(logger.info).toHaveBeenCalledTimes(2); + expect(logger.debug).toHaveBeenCalledTimes(2); expect(logger.warn).toHaveBeenCalledTimes(2); expect(emailsKo).toEqual(4); expect(emailsOk).toEqual(4); diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.ts index 3f847c97226ac0..63fc6f8d13df36 100644 --- a/x-pack/plugins/notifications/server/services/licensed_email_service.ts +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.ts @@ -36,7 +36,7 @@ export class LicensedEmailService implements EmailService { const licenseCheck = license.check(PLUGIN_ID, this.minimumLicense); if (licenseCheck.state === 'valid') { - this.logger.info('Your current license allows sending email notifications'); + this.logger.debug('Your current license allows sending email notifications'); return true; } diff --git a/x-pack/plugins/notifications/server/services/types.ts b/x-pack/plugins/notifications/server/services/types.ts index bee62f76df966b..671bbe4eafdf22 100755 --- a/x-pack/plugins/notifications/server/services/types.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -15,11 +15,11 @@ interface RelatedSavedObject { namespace: string; } -export type SenderContext = RelatedSavedObject[]; - export interface PlainTextEmail { to: string[]; subject: string; message: string; - context?: SenderContext; + context?: { + relatedObjects?: RelatedSavedObject[]; + }; } From 0ccf326b6ba21ab0e13cd7ba867027c9654267ba Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 27 Oct 2022 13:12:03 +0200 Subject: [PATCH 30/38] Transform factory => provider, update start contract --- x-pack/plugins/actions/server/mocks.ts | 2 +- .../notifications/server/plugin.test.ts | 110 +++----- x-pack/plugins/notifications/server/plugin.ts | 48 +--- .../connectors_email_service_factory.test.ts | 142 ---------- .../connectors_email_service_factory.ts | 76 ------ .../connectors_email_service_provider.test.ts | 243 ++++++++++++++++++ .../connectors_email_service_provider.ts | 93 +++++++ .../notifications/server/services/index.ts | 12 +- .../notifications/server/services/types.ts | 10 + x-pack/plugins/notifications/server/types.ts | 9 +- 10 files changed, 404 insertions(+), 341 deletions(-) delete mode 100644 x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts delete mode 100644 x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts create mode 100644 x-pack/plugins/notifications/server/services/connectors_email_service_provider.test.ts create mode 100755 x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts diff --git a/x-pack/plugins/actions/server/mocks.ts b/x-pack/plugins/actions/server/mocks.ts index 4d5846de9528fa..34e02b9c43e588 100644 --- a/x-pack/plugins/actions/server/mocks.ts +++ b/x-pack/plugins/actions/server/mocks.ts @@ -39,7 +39,7 @@ const createStartMock = () => { isActionTypeEnabled: jest.fn(), isActionExecutable: jest.fn(), getActionsClientWithRequest: jest.fn().mockResolvedValue(actionsClientMock.create()), - getUnsecuredActionsClient: jest.fn().mockResolvedValue(unsecuredActionsClientMock.create()), + getUnsecuredActionsClient: jest.fn().mockReturnValue(unsecuredActionsClientMock.create()), getActionsAuthorizationWithRequest: jest .fn() .mockReturnValue(actionsAuthorizationMock.create()), diff --git a/x-pack/plugins/notifications/server/plugin.test.ts b/x-pack/plugins/notifications/server/plugin.test.ts index 51990d79d8a28b..687414becb051b 100644 --- a/x-pack/plugins/notifications/server/plugin.test.ts +++ b/x-pack/plugins/notifications/server/plugin.test.ts @@ -9,23 +9,15 @@ import { coreMock } from '@kbn/core/server/mocks'; import { actionsMock } from '@kbn/actions-plugin/server/mocks'; import type { NotificationsConfigType } from './config'; import { NotificationsPlugin } from './plugin'; -import * as emailServiceFactory from './services/connectors_email_service_factory'; import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; +import { EmailServiceProvider } from './services/connectors_email_service_provider'; +import { EmailServiceStart } from './services'; -const checkEmailServiceConfigurationSpy = jest.spyOn( - emailServiceFactory, - 'checkEmailServiceConfiguration' -); +jest.mock('./services/connectors_email_service_provider'); -const getEmailServiceSpy = jest.spyOn(emailServiceFactory, 'getEmailService'); - -const invalidConnectorConfig = { - connectors: { - default: { - email: 'someUnexistingConnectorId', - }, - }, -}; +const emailServiceProviderMock = EmailServiceProvider as jest.MockedClass< + typeof EmailServiceProvider +>; const validConnectorConfig = { connectors: { @@ -58,7 +50,7 @@ const createNotificationsPlugin = (config: NotificationsConfigType) => { return { context, - logger: context.logger, + logger: context.logger.get(), plugin, coreSetup, coreStart, @@ -70,76 +62,46 @@ const createNotificationsPlugin = (config: NotificationsConfigType) => { }; describe('Notifications Plugin', () => { - describe('setup()', () => { - beforeEach(() => checkEmailServiceConfigurationSpy.mockClear()); + beforeEach(() => emailServiceProviderMock.mockClear()); - it('should call checkEmailServiceConfiguration(), catch Exceptions and log a warn', () => { - const { plugin, logger, coreSetup } = createNotificationsPlugin(validConnectorConfig); - const pluginSetup = { - actions: actionsMock.createSetup(), - }; - plugin.setup(coreSetup, pluginSetup); - expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledTimes(1); - expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledWith({ - config: validConnectorConfig, - plugins: pluginSetup, - }); - expect(logger.get().warn).toHaveBeenCalledWith( - `Email Service Setup Error: 'actions' and 'licensing' plugins are required.` - ); - - // eslint-disable-next-line dot-notation - expect(plugin['emailServiceSetupSuccessful']).toEqual(false); - }); + it('should create an EmailServiceProvider passing in the configuration and logger from the initializer context', () => { + const { logger } = createNotificationsPlugin(validConnectorConfig); + expect(emailServiceProviderMock).toHaveBeenCalledTimes(1); + expect(emailServiceProviderMock).toHaveBeenCalledWith(validConnectorConfig, logger); + }); - it('should pass the setup checks successfully and not log any warning if the configuration is correct', () => { - const { plugin, logger, coreSetup, pluginSetup } = - createNotificationsPlugin(validConnectorConfig); + describe('setup()', () => { + it('should call setup() on the created EmailServiceProvider, passing in the setup plugin dependencies', () => { + const { plugin, coreSetup, pluginSetup } = createNotificationsPlugin(validConnectorConfig); plugin.setup(coreSetup, pluginSetup); - expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledTimes(1); - expect(checkEmailServiceConfigurationSpy).toHaveBeenCalledWith({ - config: validConnectorConfig, - plugins: pluginSetup, - }); - expect(logger.get().warn).not.toHaveBeenCalled(); - - // eslint-disable-next-line dot-notation - expect(plugin['emailServiceSetupSuccessful']).toEqual(true); + expect(emailServiceProviderMock.mock.instances[0].setup).toHaveBeenCalledTimes(1); + expect(emailServiceProviderMock.mock.instances[0].setup).toBeCalledWith(pluginSetup); }); }); describe('start()', () => { - beforeEach(() => getEmailServiceSpy.mockClear()); - - it('should not call getEmailService() if the setup() has not been run', () => { + it('should call start() on the created EmailServiceProvider, passing in the setup plugin dependencies', () => { const { plugin, coreStart, pluginStart } = createNotificationsPlugin(validConnectorConfig); - const { email } = plugin.start(coreStart, pluginStart); - expect(getEmailServiceSpy).not.toHaveBeenCalled(); - expect(email).toBeUndefined(); + plugin.start(coreStart, pluginStart); + expect(emailServiceProviderMock.mock.instances[0].start).toHaveBeenCalledTimes(1); + expect(emailServiceProviderMock.mock.instances[0].start).toBeCalledWith(pluginStart); }); - it('should not call getEmailService() if setup() has failed', () => { - const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = - createNotificationsPlugin(invalidConnectorConfig); - plugin.setup(coreSetup, pluginSetup); - const { email } = plugin.start(coreStart, pluginStart); - expect(getEmailServiceSpy).not.toHaveBeenCalled(); - expect(email).toBeUndefined(); - }); + it('should return EmailServiceProvider.start() contract as part of its contract', () => { + const { plugin, coreStart, pluginStart } = createNotificationsPlugin(validConnectorConfig); - it('should call getEmailService() if setup() was successful', () => { - const { plugin, coreSetup, pluginSetup, coreStart, pluginStart } = - createNotificationsPlugin(validConnectorConfig); - plugin.setup(coreSetup, pluginSetup); - const { email } = plugin.start(coreStart, pluginStart); - expect(getEmailServiceSpy).toHaveBeenCalledTimes(1); - expect(getEmailServiceSpy).toHaveBeenCalledWith({ - config: validConnectorConfig, - plugins: pluginStart, - logger: expect.any(Object), - }); - expect(email).toEqual(getEmailServiceSpy.mock.results[0].value); - expect(email?.sendPlainTextEmail).toBeInstanceOf(Function); + const emailStart: EmailServiceStart = { + getEmailService: jest.fn(), + isEmailServiceAvailable: jest.fn(), + }; + + const providerMock = emailServiceProviderMock.mock + .instances[0] as jest.Mocked; + providerMock.start.mockReturnValue(emailStart); + const start = plugin.start(coreStart, pluginStart); + expect(emailServiceProviderMock.mock.instances[0].start).toHaveBeenCalledTimes(1); + expect(emailServiceProviderMock.mock.instances[0].start).toBeCalledWith(pluginStart); + expect(start).toEqual(expect.objectContaining(emailStart)); }); }); }); diff --git a/x-pack/plugins/notifications/server/plugin.ts b/x-pack/plugins/notifications/server/plugin.ts index f8a01f73cbd957..562db1977a73cc 100755 --- a/x-pack/plugins/notifications/server/plugin.ts +++ b/x-pack/plugins/notifications/server/plugin.ts @@ -5,59 +5,35 @@ * 2.0. */ -import type { - PluginInitializerContext, - CoreSetup, - CoreStart, - Plugin, - Logger, -} from '@kbn/core/server'; +import type { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from '@kbn/core/server'; import type { NotificationsPluginSetupDeps, NotificationsPluginStartDeps, NotificationsPluginStart, } from './types'; -import { type EmailService, checkEmailServiceConfiguration, getEmailService } from './services'; import type { NotificationsConfigType } from './config'; +import { EmailServiceProvider } from './services/connectors_email_service_provider'; export class NotificationsPlugin implements Plugin { - private readonly logger: Logger; - private readonly initialConfig: NotificationsConfigType; - private emailServiceSetupSuccessful: boolean; + private emailServiceProvider: EmailServiceProvider; constructor(initializerContext: PluginInitializerContext) { - this.logger = initializerContext.logger.get(); - this.initialConfig = initializerContext.config.get(); - this.emailServiceSetupSuccessful = false; + this.emailServiceProvider = new EmailServiceProvider( + initializerContext.config.get(), + initializerContext.logger.get() + ); } public setup(_core: CoreSetup, plugins: NotificationsPluginSetupDeps) { - try { - checkEmailServiceConfiguration({ - config: this.initialConfig, - plugins, - }); - this.emailServiceSetupSuccessful = true; - } catch (err) { - this.logger.warn(`Email Service Setup ${err}`); - } + this.emailServiceProvider.setup(plugins); } public start(_core: CoreStart, plugins: NotificationsPluginStartDeps) { - let email: EmailService | undefined; - try { - if (this.emailServiceSetupSuccessful) { - email = getEmailService({ - config: this.initialConfig, - plugins, - logger: this.logger, - }); - } - } catch (err) { - this.logger.warn(`Error starting email service: ${err}`); - } + const emailStartContract = this.emailServiceProvider.start(plugins); - return { email }; + return { + ...emailStartContract, + }; } public stop() {} diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts deleted file mode 100644 index 3a9bef3241569e..00000000000000 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.test.ts +++ /dev/null @@ -1,142 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { loggerMock } from '@kbn/logging-mocks'; -import { actionsMock } from '@kbn/actions-plugin/server/mocks'; -import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; -import { - checkEmailServiceConfiguration, - type CheckEmailServiceParams, - type EmailServiceFactoryParams, - getEmailService, -} from './connectors_email_service_factory'; -import { LicensedEmailService } from './licensed_email_service'; - -const missingConnectorConfig = { - connectors: { - default: {}, - }, -}; - -const invalidConnectorConfig = { - connectors: { - default: { - email: 'someUnexistingConnectorId', - }, - }, -}; - -const validConnectorConfig = { - connectors: { - default: { - email: 'validConnectorId', - }, - }, -}; - -describe('checkEmailServiceConfiguration()', () => { - it('should throw an Error if Actions or Licensing plugins are not available', () => { - expect(() => { - const params: CheckEmailServiceParams = { - config: validConnectorConfig, - plugins: { - actions: actionsMock.createSetup(), - }, - }; - checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot(`"'actions' and 'licensing' plugins are required."`); - }); - - it('should throw an Error if no default email connector has been defined', () => { - expect(() => { - const params: CheckEmailServiceParams = { - config: missingConnectorConfig, - plugins: { - actions: actionsMock.createSetup(), - licensing: licensingMock.createSetup(), - }, - }; - checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot(`"Email connector not specified."`); - }); - - it('should throw an Error if the specified email connector is not a preconfigured connector', () => { - expect(() => { - const actions = actionsMock.createSetup(); - actions.isPreconfiguredConnector.mockImplementationOnce( - (connectorId) => connectorId === 'validConnectorId' - ); - const params: CheckEmailServiceParams = { - config: invalidConnectorConfig, - plugins: { - actions, - licensing: licensingMock.createSetup(), - }, - }; - checkEmailServiceConfiguration(params); - }).toThrowErrorMatchingInlineSnapshot( - `"Unexisting email connector 'someUnexistingConnectorId' specified."` - ); - }); - - it('should not throw an Error if required plugins are present and the specified email connector is valid', () => { - expect(() => { - const actions = actionsMock.createSetup(); - actions.isPreconfiguredConnector.mockImplementationOnce( - (connectorId) => connectorId === 'validConnectorId' - ); - const params: CheckEmailServiceParams = { - config: validConnectorConfig, - plugins: { - actions, - licensing: licensingMock.createSetup(), - }, - }; - checkEmailServiceConfiguration(params); - }).not.toThrowError(); - }); -}); - -describe('getEmailService()', () => { - it('returns undefined if Actions or Licensing plugins are not available', () => { - const params: EmailServiceFactoryParams = { - config: validConnectorConfig, - plugins: { - licensing: licensingMock.createStart(), - }, - logger: loggerMock.create(), - }; - const email = getEmailService(params); - expect(email).toBeUndefined(); - }); - - it('returns undefined if default connector has not been specified', () => { - const params: EmailServiceFactoryParams = { - config: missingConnectorConfig, - plugins: { - actions: actionsMock.createStart(), - licensing: licensingMock.createStart(), - }, - logger: loggerMock.create(), - }; - const email = getEmailService(params); - expect(email).toBeUndefined(); - }); - - it('returns an instance of ConnectorsEmailService if all params have been specified', () => { - const params: EmailServiceFactoryParams = { - config: invalidConnectorConfig, // note that the factory does not check for connector validity - plugins: { - actions: actionsMock.createStart(), - licensing: licensingMock.createStart(), - }, - logger: loggerMock.create(), - }; - const email = getEmailService(params); - expect(email).toBeInstanceOf(LicensedEmailService); - }); -}); diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts deleted file mode 100644 index d6f0a177748c73..00000000000000 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_factory.ts +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import type { Logger } from '@kbn/logging'; -import type { PluginSetupContract, PluginStartContract } from '@kbn/actions-plugin/server'; -import type { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing-plugin/server'; -import { ConnectorsEmailService } from './connectors_email_service'; -import type { EmailService } from './types'; -import { PLUGIN_ID } from '../../common'; -import type { ConnectorsEmailConfigType } from '../config/connectors_email_config'; -import { LicensedEmailService } from './licensed_email_service'; - -export interface EmailServiceSetupDeps { - actions?: PluginSetupContract; - licensing?: LicensingPluginSetup; -} - -export interface EmailServiceStartDeps { - actions?: PluginStartContract; - licensing?: LicensingPluginStart; -} - -export interface CheckEmailServiceParams { - config: ConnectorsEmailConfigType; - plugins: EmailServiceSetupDeps; -} - -export interface EmailServiceFactoryParams { - config: ConnectorsEmailConfigType; - plugins: EmailServiceStartDeps; - logger: Logger; -} - -export function checkEmailServiceConfiguration(params: CheckEmailServiceParams) { - const { - config, - plugins: { actions, licensing }, - } = params; - - if (!actions || !licensing) { - throw new Error(`'actions' and 'licensing' plugins are required.`); - } - - const emailConnector = config.connectors?.default?.email; - if (!emailConnector) { - throw new Error('Email connector not specified.'); - } - - if (!actions.isPreconfiguredConnector(emailConnector)) { - throw new Error(`Unexisting email connector '${emailConnector}' specified.`); - } -} - -export function getEmailService(params: EmailServiceFactoryParams): EmailService | undefined { - const { - config, - plugins: { actions, licensing }, - logger, - } = params; - - const unsecuredActionsClient = actions?.getUnsecuredActionsClient(); - const emailConnector = config.connectors?.default?.email; - - if (licensing && unsecuredActionsClient && emailConnector) { - return new LicensedEmailService( - new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient), - licensing.license$, - 'platinum', - logger - ); - } -} diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_provider.test.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.test.ts new file mode 100644 index 00000000000000..6c36f94db1a7cc --- /dev/null +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.test.ts @@ -0,0 +1,243 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { loggerMock } from '@kbn/logging-mocks'; +import { actionsMock } from '@kbn/actions-plugin/server/mocks'; +import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; +import { LicensedEmailService } from './licensed_email_service'; +import { EmailServiceProvider } from './connectors_email_service_provider'; +import { ConnectorsEmailService } from './connectors_email_service'; +import { PLUGIN_ID } from '../../common'; + +jest.mock('./licensed_email_service'); +jest.mock('./connectors_email_service'); + +const licensedEmailServiceMock = LicensedEmailService as jest.MockedClass< + typeof LicensedEmailService +>; +const connectorsEmailServiceMock = ConnectorsEmailService as jest.MockedClass< + typeof ConnectorsEmailService +>; + +const missingConnectorConfig = { + connectors: { + default: {}, + }, +}; + +const invalidConnectorConfig = { + connectors: { + default: { + email: 'someUnexistingConnectorId', + }, + }, +}; + +const validConnectorConfig = { + connectors: { + default: { + email: 'validConnectorId', + }, + }, +}; + +describe('ConnectorsEmailServiceProvider', () => { + const logger = loggerMock.create(); + const actionsSetup = actionsMock.createSetup(); + actionsSetup.isPreconfiguredConnector.mockImplementation( + (connectorId) => connectorId === 'validConnectorId' + ); + + beforeEach(() => { + loggerMock.clear(logger); + licensedEmailServiceMock.mockClear(); + connectorsEmailServiceMock.mockClear(); + }); + + it('implements the IEmailServiceProvider interface', () => { + const serviceProvider = new EmailServiceProvider(validConnectorConfig, loggerMock.create()); + expect(serviceProvider.setup).toBeInstanceOf(Function); + expect(serviceProvider.start).toBeInstanceOf(Function); + }); + + describe('setup()', () => { + it('should log a warning if Actions or Licensing plugins are not available', () => { + const serviceProvider = new EmailServiceProvider(validConnectorConfig, logger); + serviceProvider.setup({ + actions: actionsSetup, + }); + + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + `Email Service Error: 'actions' and 'licensing' plugins are required.` + ); + // eslint-disable-next-line dot-notation + expect(serviceProvider['setupSuccessful']).toEqual(false); + }); + + it('should log a warning if no default email connector has been defined', () => { + const serviceProvider = new EmailServiceProvider(missingConnectorConfig, logger); + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + `Email Service Error: Email connector not specified.` + ); + // eslint-disable-next-line dot-notation + expect(serviceProvider['setupSuccessful']).toEqual(false); + }); + + it('should log a warning if the specified email connector is not a preconfigured connector', () => { + const serviceProvider = new EmailServiceProvider(invalidConnectorConfig, logger); + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + `Email Service Error: Unexisting email connector 'someUnexistingConnectorId' specified.` + ); + // eslint-disable-next-line dot-notation + expect(serviceProvider['setupSuccessful']).toEqual(false); + }); + + it('should not log a warning if required plugins are present and the specified email connector is valid', () => { + const serviceProvider = new EmailServiceProvider(validConnectorConfig, logger); + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + + expect(logger.warn).not.toHaveBeenCalled(); + // eslint-disable-next-line dot-notation + expect(serviceProvider['setupSuccessful']).toEqual(true); + }); + }); + + describe('start()', () => { + it('returns an object that implements the EmailServiceStart contract', () => { + const serviceProvider = new EmailServiceProvider(missingConnectorConfig, logger); + const start = serviceProvider.start({}); + expect(start.getEmailService).toBeInstanceOf(Function); + expect(start.isEmailServiceAvailable).toBeInstanceOf(Function); + }); + + describe('if setup has not been run', () => { + it('the start contract methods fail accordingly', () => { + const serviceProvider = new EmailServiceProvider(missingConnectorConfig, logger); + const start = serviceProvider.start({}); + expect(start.isEmailServiceAvailable()).toEqual(false); + expect(() => { + start.getEmailService(); + }).toThrowErrorMatchingInlineSnapshot(`"Email Service Error: setup() has not been run"`); + }); + }); + + describe('if setup() did not complete successfully', () => { + it('the start contract methods fail accordingly', () => { + const serviceProvider = new EmailServiceProvider(invalidConnectorConfig, logger); + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + const start = serviceProvider.start({ + actions: actionsMock.createStart(), + licensing: licensingMock.createStart(), + }); + expect(start.isEmailServiceAvailable()).toEqual(false); + expect(() => { + start.getEmailService(); + }).toThrowErrorMatchingInlineSnapshot( + `"Email Service Error: Unexisting email connector 'someUnexistingConnectorId' specified."` + ); + }); + }); + + describe('if setup() did complete successfully and Action and Licensing plugin start contracts are available', () => { + it('attempts to build an UnsecuredActionsClient', () => { + const serviceProvider = new EmailServiceProvider(validConnectorConfig, logger); + const actionsStart = actionsMock.createStart(); + + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + serviceProvider.start({ + actions: actionsStart, + licensing: licensingMock.createStart(), + }); + expect(actionsStart.getUnsecuredActionsClient).toHaveBeenCalledTimes(1); + }); + + describe('if getUnsecuredActionsClient() throws an Exception', () => { + it('catches the exception, and the start contract methods fail accordingly', () => { + const serviceProvider = new EmailServiceProvider(validConnectorConfig, logger); + const actionsStart = actionsMock.createStart(); + actionsStart.getUnsecuredActionsClient.mockImplementation(() => { + throw new Error('Something went terribly wrong.'); + }); + + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + const start = serviceProvider.start({ + actions: actionsStart, + licensing: licensingMock.createStart(), + }); + + expect(start.isEmailServiceAvailable()).toEqual(false); + expect(() => { + start.getEmailService(); + }).toThrowErrorMatchingInlineSnapshot( + `"Email Service Error: Something went terribly wrong."` + ); + }); + }); + + describe('if getUnsecuredActionsClient() returns an UnsecuredActionsClient', () => { + it('returns a start contract that provides valid EmailService', () => { + const serviceProvider = new EmailServiceProvider(validConnectorConfig, logger); + const licensingStart = licensingMock.createStart(); + const actionsStart = actionsMock.createStart(); + + serviceProvider.setup({ + actions: actionsSetup, + licensing: licensingMock.createSetup(), + }); + const start = serviceProvider.start({ + actions: actionsStart, + licensing: licensingStart, + }); + + expect(start.isEmailServiceAvailable()).toEqual(true); + const email = start.getEmailService(); + expect(email).toBeInstanceOf(LicensedEmailService); + expect(licensedEmailServiceMock).toHaveBeenCalledTimes(1); + + expect(licensedEmailServiceMock).toHaveBeenCalledWith( + connectorsEmailServiceMock.mock.instances[0], + licensingStart.license$, + 'platinum', + expect.objectContaining({ debug: expect.any(Function), warn: expect.any(Function) }) + ); + + expect(connectorsEmailServiceMock).toHaveBeenCalledTimes(1); + expect(connectorsEmailServiceMock).toHaveBeenCalledWith( + PLUGIN_ID, + validConnectorConfig.connectors.default.email, + actionsStart.getUnsecuredActionsClient() + ); + }); + }); + }); + }); +}); diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts new file mode 100755 index 00000000000000..0bcb86ff53d88d --- /dev/null +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { Logger } from '@kbn/core/server'; +import { PluginSetupContract, PluginStartContract } from '@kbn/actions-plugin/server'; +import { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing-plugin/server'; +import type { EmailService, EmailServiceStart, IEmailServiceProvider } from './types'; +import type { NotificationsConfigType } from '../config'; +import { LicensedEmailService } from './licensed_email_service'; +import { ConnectorsEmailService } from './connectors_email_service'; +import { PLUGIN_ID } from '../../common'; + +export interface EmailServiceSetupDeps { + actions?: PluginSetupContract; + licensing?: LicensingPluginSetup; +} + +export interface EmailServiceStartDeps { + actions?: PluginStartContract; + licensing?: LicensingPluginStart; +} + +export class EmailServiceProvider + implements IEmailServiceProvider +{ + private setupSuccessful: boolean; + private setupError: string; + + constructor(private config: NotificationsConfigType, private logger: Logger) { + this.setupSuccessful = false; + this.setupError = 'Email Service Error: setup() has not been run'; + } + + public setup(plugins: EmailServiceSetupDeps) { + const { actions, licensing } = plugins; + + if (!actions || !licensing) { + return this._registerServiceError(`Error: 'actions' and 'licensing' plugins are required.`); + } + + const emailConnector = this.config.connectors?.default?.email; + if (!emailConnector) { + return this._registerServiceError('Error: Email connector not specified.'); + } + + if (!actions.isPreconfiguredConnector(emailConnector)) { + return this._registerServiceError( + `Error: Unexisting email connector '${emailConnector}' specified.` + ); + } + + this.setupSuccessful = true; + } + + public start(plugins: EmailServiceStartDeps): EmailServiceStart { + const { actions, licensing } = plugins; + + let email: EmailService; + if (this.setupSuccessful && actions && licensing) { + const emailConnector = this.config.connectors!.default!.email; + + try { + const unsecuredActionsClient = actions.getUnsecuredActionsClient(); + email = new LicensedEmailService( + new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient), + licensing.license$, + 'platinum', + this.logger + ); + } catch (err) { + this._registerServiceError(err); + } + } + + return { + isEmailServiceAvailable: () => !!email, + getEmailService: () => { + if (email) return email; + throw new Error(this.setupError); + }, + }; + } + + private _registerServiceError(error: string) { + const message = `Email Service ${error}`; + this.setupError = message; + this.logger.warn(message); + } +} diff --git a/x-pack/plugins/notifications/server/services/index.ts b/x-pack/plugins/notifications/server/services/index.ts index 5c9194e43995e3..f0ad8abb4885d9 100644 --- a/x-pack/plugins/notifications/server/services/index.ts +++ b/x-pack/plugins/notifications/server/services/index.ts @@ -5,10 +5,8 @@ * 2.0. */ -export type { EmailService, PlainTextEmail } from './types'; -export { - type EmailServiceSetupDeps, - type EmailServiceStartDeps, - checkEmailServiceConfiguration, - getEmailService, -} from './connectors_email_service_factory'; +export type { EmailService, EmailServiceStart, PlainTextEmail } from './types'; +export type { + EmailServiceSetupDeps, + EmailServiceStartDeps, +} from './connectors_email_service_provider'; diff --git a/x-pack/plugins/notifications/server/services/types.ts b/x-pack/plugins/notifications/server/services/types.ts index 671bbe4eafdf22..5e565f26f8ce39 100755 --- a/x-pack/plugins/notifications/server/services/types.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -9,6 +9,16 @@ export interface EmailService { sendPlainTextEmail(payload: PlainTextEmail): Promise; } +export interface EmailServiceStart { + isEmailServiceAvailable(): boolean; + getEmailService(): EmailService; +} + +export interface IEmailServiceProvider { + setup(setupDeps: T): void; + start(startDeps: U): EmailServiceStart; +} + interface RelatedSavedObject { id: string; type: string; diff --git a/x-pack/plugins/notifications/server/types.ts b/x-pack/plugins/notifications/server/types.ts index 96704052b1c237..e7132a33cbb198 100755 --- a/x-pack/plugins/notifications/server/types.ts +++ b/x-pack/plugins/notifications/server/types.ts @@ -5,11 +5,10 @@ * 2.0. */ -import type { EmailService, EmailServiceSetupDeps, EmailServiceStartDeps } from './services'; +import type { EmailServiceStart, EmailServiceSetupDeps, EmailServiceStartDeps } from './services'; +// The 'notifications' plugin is currently only exposing an email service. +// If we want to expose other services in the future, we should update these types accordingly export type NotificationsPluginSetupDeps = EmailServiceSetupDeps; export type NotificationsPluginStartDeps = EmailServiceStartDeps; - -export interface NotificationsPluginStart { - email?: EmailService; -} +export type NotificationsPluginStart = EmailServiceStart; From 2d267406e4ef3fd6d3bb6f4f95a1359a5704ab45 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 27 Oct 2022 13:22:41 +0200 Subject: [PATCH 31/38] Fix merge-related issues --- .../fixtures/plugins/actions_simulators/server/plugin.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts index df624eb57fec28..316d1916b4af24 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/actions_simulators/server/plugin.ts @@ -92,8 +92,6 @@ export interface FixtureStartDeps { } export class FixturePlugin implements Plugin { - private router?: IRouter; - public setup(core: CoreSetup, { features, actions }: FixtureSetupDeps) { // this action is specifically NOT enabled in ../../config.ts const notEnabledActionType: ActionType = { @@ -139,10 +137,7 @@ export class FixturePlugin implements Plugin Date: Thu, 27 Oct 2022 13:36:32 +0200 Subject: [PATCH 32/38] Code cleanup, update README --- x-pack/plugins/notifications/README.md | 10 ++++++---- .../server/services/connectors_email_service.ts | 5 ++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/notifications/README.md b/x-pack/plugins/notifications/README.md index 0c615dbda244f6..75ea13570ec2b3 100755 --- a/x-pack/plugins/notifications/README.md +++ b/x-pack/plugins/notifications/README.md @@ -8,8 +8,10 @@ The Notifications plugin provides a set of services to help Solutions and plugin The `start` function exposes the following interface: -- `email: EmailService`: - A simple service that can be used to send plain text emails. +- `isEmailServiceAvailable(): boolean`: + A function to check whether the deployment is properly configured and the EmailService can be correctly retrieved. +- `getEmailService(): EmailService`: +- A function to get the basic EmailService, which can be used to send plain text emails. If the EmailService is not available, trying to retrieve it will result in an Exception. ### Usage @@ -41,8 +43,8 @@ class MyPlugin { core: CoreStart, { notifications }: MyPluginStartDeps ) { - const emailService = notifications.email; - if (emailService) { + if (notifications.isEmailServiceAvailable()) { + const emailService = notifications.getEmailService(); emailService.sendPlainTextEmail({ to: 'foo@bar.com', subject: 'Some subject', 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 756f37ca587861..ef3a785d8924c7 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -5,15 +5,14 @@ * 2.0. */ -import type { UnsecuredActionsClient } from '@kbn/actions-plugin/server/unsecured_actions_client/unsecured_actions_client'; -import type { PublicMethodsOf } from '@kbn/utility-types'; +import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { constructor( private requesterId: string, private connectorId: string, - private actionsClient: PublicMethodsOf + private actionsClient: IUnsecuredActionsClient ) {} async sendPlainTextEmail(params: PlainTextEmail): Promise { From 9dbe5772b28952c8941de157b2f7a6b8ba3f1000 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 27 Oct 2022 14:37:53 +0200 Subject: [PATCH 33/38] Fix TS error --- .../server/services/connectors_email_service_provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts index 0bcb86ff53d88d..d0666138164744 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts @@ -61,7 +61,7 @@ export class EmailServiceProvider let email: EmailService; if (this.setupSuccessful && actions && licensing) { - const emailConnector = this.config.connectors!.default!.email; + const emailConnector = this.config.connectors!.default!.email!; try { const unsecuredActionsClient = actions.getUnsecuredActionsClient(); From 023887796ba5e06ccf36672597706a3028fe52ad Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Fri, 28 Oct 2022 11:48:27 +0200 Subject: [PATCH 34/38] Support list of spaces for each related SO --- .../services/connectors_email_service.test.ts | 23 ++++++++++++--- .../services/connectors_email_service.ts | 28 ++++++++++++++++--- .../notifications/server/services/types.ts | 4 +-- 3 files changed, 45 insertions(+), 10 deletions(-) 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 ece46977d62e3f..66262581791133 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 @@ -50,7 +50,7 @@ describe('sendPlainTextEmail()', () => { { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', type: 'cases', - namespace: 'some-space', + spaceIds: ['space1', 'space2'], }, ], }, @@ -71,7 +71,12 @@ describe('sendPlainTextEmail()', () => { { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', type: 'cases', - namespace: 'some-space', + namespace: 'space1', + }, + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'space2', }, ], }, @@ -86,7 +91,12 @@ describe('sendPlainTextEmail()', () => { { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', type: 'cases', - namespace: 'some-space', + namespace: 'space1', + }, + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'space2', }, ], }, @@ -101,7 +111,12 @@ describe('sendPlainTextEmail()', () => { { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', type: 'cases', - namespace: 'some-space', + namespace: 'space1', + }, + { + id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', + type: 'cases', + namespace: 'space2', }, ], }, 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 ef3a785d8924c7..5ff461870c3d96 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -6,7 +6,8 @@ */ import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; -import type { EmailService, PlainTextEmail } from './types'; +import type { RelatedSavedObjects } from '@kbn/actions-plugin/server/lib/related_saved_objects'; +import type { EmailService, PlainTextEmail, RelatedSavedObject } from './types'; export class ConnectorsEmailService implements EmailService { constructor( @@ -23,10 +24,29 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - relatedSavedObjects: params.context?.relatedObjects?.length - ? params.context.relatedObjects - : undefined, + ...(params.context?.relatedObjects?.length && { + relatedSavedObjects: this._getRelatedSavedObjects(params.context!.relatedObjects!), + }), })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); } + + private _getRelatedSavedObjects(relatedObjects: RelatedSavedObject[]): RelatedSavedObjects { + const relatedSavedObjects: RelatedSavedObjects = []; + + relatedObjects.forEach((relatedObject) => { + // FIXME we temporarily map each related SO to multiple ones (one per space) + // we can remove this workaround after the following PR is merged: + // https://github.com/elastic/kibana/pull/144111/ + relatedObject.spaceIds.forEach((spaceId) => { + relatedSavedObjects.push({ + id: relatedObject.id, + type: relatedObject.type, + namespace: spaceId, + }); + }); + }); + + return relatedSavedObjects; + } } diff --git a/x-pack/plugins/notifications/server/services/types.ts b/x-pack/plugins/notifications/server/services/types.ts index 5e565f26f8ce39..7ac87af37475a0 100755 --- a/x-pack/plugins/notifications/server/services/types.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -19,10 +19,10 @@ export interface IEmailServiceProvider { start(startDeps: U): EmailServiceStart; } -interface RelatedSavedObject { +export interface RelatedSavedObject { id: string; type: string; - namespace: string; + spaceIds: string[]; } export interface PlainTextEmail { From dc4c9a670039da457335c73340db6fd797e245d3 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Wed, 2 Nov 2022 11:24:00 +0100 Subject: [PATCH 35/38] Accept single spaceId for related SOs. Add convenience mocks --- x-pack/plugins/notifications/server/mocks.ts | 52 +++++++++++++++++++ .../services/connectors_email_service.test.ts | 17 +----- .../services/connectors_email_service.ts | 17 +----- .../notifications/server/services/types.ts | 2 +- 4 files changed, 55 insertions(+), 33 deletions(-) create mode 100644 x-pack/plugins/notifications/server/mocks.ts diff --git a/x-pack/plugins/notifications/server/mocks.ts b/x-pack/plugins/notifications/server/mocks.ts new file mode 100644 index 00000000000000..6360e0ece597b8 --- /dev/null +++ b/x-pack/plugins/notifications/server/mocks.ts @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { PublicMethodsOf } from '@kbn/utility-types'; +import type { EmailService } from './services'; +import type { NotificationsPluginStart } from './types'; +import type { NotificationsPlugin } from './plugin'; + +const emailServiceMock: jest.Mocked = { + sendPlainTextEmail: jest.fn(), +}; + +const createEmailServiceMock = () => { + return emailServiceMock; +}; + +const startMock: jest.Mocked = { + isEmailServiceAvailable: jest.fn(), + getEmailService: jest.fn(createEmailServiceMock), +}; + +const createStartMock = () => { + return startMock; +}; + +const notificationsPluginMock: jest.Mocked> = { + setup: jest.fn(), + start: jest.fn(createStartMock) as jest.Mock, + stop: jest.fn(), +}; + +const createNotificationsPluginMock = () => { + return notificationsPluginMock; +}; + +export const notificationsMock = { + createNotificationsPlugin: createNotificationsPluginMock, + createEmailService: createEmailServiceMock, + createStart: createStartMock, + clear: () => { + emailServiceMock.sendPlainTextEmail.mockClear(); + startMock.getEmailService.mockClear(); + startMock.isEmailServiceAvailable.mockClear(); + notificationsPluginMock.setup.mockClear(); + notificationsPluginMock.start.mockClear(); + notificationsPluginMock.stop.mockClear(); + }, +}; 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 66262581791133..eb68554a01f129 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 @@ -50,7 +50,7 @@ describe('sendPlainTextEmail()', () => { { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', type: 'cases', - spaceIds: ['space1', 'space2'], + spaceId: 'space1', }, ], }, @@ -73,11 +73,6 @@ describe('sendPlainTextEmail()', () => { type: 'cases', namespace: 'space1', }, - { - id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', - type: 'cases', - namespace: 'space2', - }, ], }, { @@ -93,11 +88,6 @@ describe('sendPlainTextEmail()', () => { type: 'cases', namespace: 'space1', }, - { - id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', - type: 'cases', - namespace: 'space2', - }, ], }, { @@ -113,11 +103,6 @@ describe('sendPlainTextEmail()', () => { type: 'cases', namespace: 'space1', }, - { - id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', - type: 'cases', - namespace: 'space2', - }, ], }, ]); 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 5ff461870c3d96..d013b300d05c77 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -32,21 +32,6 @@ export class ConnectorsEmailService implements EmailService { } private _getRelatedSavedObjects(relatedObjects: RelatedSavedObject[]): RelatedSavedObjects { - const relatedSavedObjects: RelatedSavedObjects = []; - - relatedObjects.forEach((relatedObject) => { - // FIXME we temporarily map each related SO to multiple ones (one per space) - // we can remove this workaround after the following PR is merged: - // https://github.com/elastic/kibana/pull/144111/ - relatedObject.spaceIds.forEach((spaceId) => { - relatedSavedObjects.push({ - id: relatedObject.id, - type: relatedObject.type, - namespace: spaceId, - }); - }); - }); - - return relatedSavedObjects; + return relatedObjects.map(({ id, type, spaceId: namespace }) => ({ id, type, namespace })); } } diff --git a/x-pack/plugins/notifications/server/services/types.ts b/x-pack/plugins/notifications/server/services/types.ts index 7ac87af37475a0..d597ec1a29cf5e 100755 --- a/x-pack/plugins/notifications/server/services/types.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -22,7 +22,7 @@ export interface IEmailServiceProvider { export interface RelatedSavedObject { id: string; type: string; - spaceIds: string[]; + spaceId: string; } export interface PlainTextEmail { From d6e46f5db5becbf02d73411f29e48acab4a469a4 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Wed, 2 Nov 2022 17:31:48 +0100 Subject: [PATCH 36/38] Fix CI types error --- .../server/services/connectors_email_service.ts | 11 ++++------- .../server/services/licensed_email_service.test.ts | 6 +++--- x-pack/plugins/notifications/tsconfig.json | 6 ++++-- 3 files changed, 11 insertions(+), 12 deletions(-) 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 d013b300d05c77..267e12df5c6f59 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -6,8 +6,7 @@ */ import type { IUnsecuredActionsClient } from '@kbn/actions-plugin/server'; -import type { RelatedSavedObjects } from '@kbn/actions-plugin/server/lib/related_saved_objects'; -import type { EmailService, PlainTextEmail, RelatedSavedObject } from './types'; +import type { EmailService, PlainTextEmail } from './types'; export class ConnectorsEmailService implements EmailService { constructor( @@ -25,13 +24,11 @@ export class ConnectorsEmailService implements EmailService { message: params.message, }, ...(params.context?.relatedObjects?.length && { - relatedSavedObjects: this._getRelatedSavedObjects(params.context!.relatedObjects!), + relatedSavedObjects: params.context!.relatedObjects!.map( + ({ id, type, spaceId: namespace }) => ({ id, type, namespace }) + ), }), })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); } - - private _getRelatedSavedObjects(relatedObjects: RelatedSavedObject[]): RelatedSavedObjects { - return relatedObjects.map(({ id, type, spaceId: namespace }) => ({ id, type, namespace })); - } } diff --git a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts index 811ab3eb71f055..ac196ebb7321b0 100644 --- a/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts +++ b/x-pack/plugins/notifications/server/services/licensed_email_service.test.ts @@ -6,7 +6,7 @@ */ import { Subject } from 'rxjs'; -import { licenseMock } from '@kbn/licensing-plugin/common/licensing.mock'; +import { licensingMock } from '@kbn/licensing-plugin/server/mocks'; import { loggerMock } from '@kbn/logging-mocks'; import { LicensedEmailService } from './licensed_email_service'; import type { ILicense } from '@kbn/licensing-plugin/server'; @@ -18,8 +18,8 @@ const emailServiceMock: EmailService = { sendPlainTextEmail: jest.fn(), }; -const validLicense = licenseMock.createLicenseMock(); -const invalidLicense = licenseMock.createLicenseMock(); +const validLicense = licensingMock.createLicenseMock(); +const invalidLicense = licensingMock.createLicenseMock(); invalidLicense.type = 'basic'; invalidLicense.check = jest.fn(() => ({ state: 'invalid', diff --git a/x-pack/plugins/notifications/tsconfig.json b/x-pack/plugins/notifications/tsconfig.json index e87642e7454388..6f2c186803b0cf 100644 --- a/x-pack/plugins/notifications/tsconfig.json +++ b/x-pack/plugins/notifications/tsconfig.json @@ -13,7 +13,9 @@ "public/**/*", "common/**/*" ], - "references": [ - { "path": "../actions/tsconfig.json" } + "kbn_references": [ + { "path": "../../../src/core/tsconfig.json" }, + { "path": "../actions/tsconfig.json" }, + { "path": "../licensing/tsconfig.json" } ] } From 8d026c5fa628d739fb78017d767aa2b60940f268 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 3 Nov 2022 11:06:22 +0100 Subject: [PATCH 37/38] Address PR remarks --- .../notifications/server/config/config.ts | 20 +++++++++++-- .../server/config/connectors_email_config.ts | 30 ------------------- .../services/connectors_email_service.ts | 16 ++++++---- .../connectors_email_service_provider.ts | 11 +++++-- 4 files changed, 36 insertions(+), 41 deletions(-) delete mode 100644 x-pack/plugins/notifications/server/config/connectors_email_config.ts diff --git a/x-pack/plugins/notifications/server/config/config.ts b/x-pack/plugins/notifications/server/config/config.ts index f2967b6f59fd83..f2dc570adabe98 100644 --- a/x-pack/plugins/notifications/server/config/config.ts +++ b/x-pack/plugins/notifications/server/config/config.ts @@ -5,11 +5,25 @@ * 2.0. */ +import { schema, type TypeOf } from '@kbn/config-schema'; import type { PluginConfigDescriptor } from '@kbn/core/server'; -import { configSchema, type ConnectorsEmailConfigType } from './connectors_email_config'; -export type NotificationsConfigType = ConnectorsEmailConfigType; +export const configSchema = schema.object( + { + connectors: schema.maybe( + schema.object({ + default: schema.maybe( + schema.object({ + email: schema.maybe(schema.string()), + }) + ), + }) + ), + }, + { defaultValue: {} } +); +export type NotificationsConfigType = TypeOf; -export const config: PluginConfigDescriptor = { +export const config: PluginConfigDescriptor = { schema: configSchema, }; diff --git a/x-pack/plugins/notifications/server/config/connectors_email_config.ts b/x-pack/plugins/notifications/server/config/connectors_email_config.ts deleted file mode 100644 index 3bd6ead9451b06..00000000000000 --- a/x-pack/plugins/notifications/server/config/connectors_email_config.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { schema, type TypeOf } from '@kbn/config-schema'; -import type { PluginConfigDescriptor } from '@kbn/core/server'; - -export const configSchema = schema.object( - { - connectors: schema.maybe( - schema.object({ - default: schema.maybe( - schema.object({ - email: schema.maybe(schema.string()), - }) - ), - }) - ), - }, - { defaultValue: {} } -); - -export type ConnectorsEmailConfigType = TypeOf; - -export const config: PluginConfigDescriptor = { - schema: configSchema, -}; 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 267e12df5c6f59..4647a16ccbec11 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -16,6 +16,16 @@ export class ConnectorsEmailService implements EmailService { ) {} async sendPlainTextEmail(params: PlainTextEmail): Promise { + const relatedSavedObjects = params.context?.relatedObjects?.length + ? params.context!.relatedObjects!.map(({ id, type, spaceId }) => { + return { + id, + type, + namespace: spaceId, + }; + }) + : undefined; + const actions = params.to.map((to) => ({ id: this.connectorId, params: { @@ -23,11 +33,7 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - ...(params.context?.relatedObjects?.length && { - relatedSavedObjects: params.context!.relatedObjects!.map( - ({ id, type, spaceId: namespace }) => ({ id, type, namespace }) - ), - }), + relatedSavedObjects, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); } diff --git a/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts index d0666138164744..f034116eb701ce 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service_provider.ts @@ -14,6 +14,8 @@ import { LicensedEmailService } from './licensed_email_service'; import { ConnectorsEmailService } from './connectors_email_service'; import { PLUGIN_ID } from '../../common'; +const MINIMUM_LICENSE = 'platinum'; + export interface EmailServiceSetupDeps { actions?: PluginSetupContract; licensing?: LicensingPluginSetup; @@ -54,6 +56,7 @@ export class EmailServiceProvider } this.setupSuccessful = true; + this.setupError = ''; } public start(plugins: EmailServiceStartDeps): EmailServiceStart { @@ -68,7 +71,7 @@ export class EmailServiceProvider email = new LicensedEmailService( new ConnectorsEmailService(PLUGIN_ID, emailConnector, unsecuredActionsClient), licensing.license$, - 'platinum', + MINIMUM_LICENSE, this.logger ); } catch (err) { @@ -79,8 +82,10 @@ export class EmailServiceProvider return { isEmailServiceAvailable: () => !!email, getEmailService: () => { - if (email) return email; - throw new Error(this.setupError); + if (!email) { + throw new Error(this.setupError); + } + return email; }, }; } From 3aef92a93fbded27f815a7a66908fe782622c794 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 3 Nov 2022 13:45:49 +0100 Subject: [PATCH 38/38] Address PR remarks #2 --- .../server/services/connectors_email_service.test.ts | 2 +- .../server/services/connectors_email_service.ts | 12 +----------- .../plugins/notifications/server/services/types.ts | 2 +- 3 files changed, 3 insertions(+), 13 deletions(-) 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 eb68554a01f129..f07b3a3ab34ae6 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 @@ -50,7 +50,7 @@ describe('sendPlainTextEmail()', () => { { id: '9c9456a4-c160-46f5-96f7-e9ac734d0d9b', type: 'cases', - spaceId: 'space1', + namespace: 'space1', }, ], }, 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 4647a16ccbec11..55586dd05b0783 100755 --- a/x-pack/plugins/notifications/server/services/connectors_email_service.ts +++ b/x-pack/plugins/notifications/server/services/connectors_email_service.ts @@ -16,16 +16,6 @@ export class ConnectorsEmailService implements EmailService { ) {} async sendPlainTextEmail(params: PlainTextEmail): Promise { - const relatedSavedObjects = params.context?.relatedObjects?.length - ? params.context!.relatedObjects!.map(({ id, type, spaceId }) => { - return { - id, - type, - namespace: spaceId, - }; - }) - : undefined; - const actions = params.to.map((to) => ({ id: this.connectorId, params: { @@ -33,7 +23,7 @@ export class ConnectorsEmailService implements EmailService { subject: params.subject, message: params.message, }, - relatedSavedObjects, + relatedSavedObjects: params.context?.relatedObjects, })); return await this.actionsClient.bulkEnqueueExecution(this.requesterId, actions); } diff --git a/x-pack/plugins/notifications/server/services/types.ts b/x-pack/plugins/notifications/server/services/types.ts index d597ec1a29cf5e..798b4d7e246996 100755 --- a/x-pack/plugins/notifications/server/services/types.ts +++ b/x-pack/plugins/notifications/server/services/types.ts @@ -22,7 +22,7 @@ export interface IEmailServiceProvider { export interface RelatedSavedObject { id: string; type: string; - spaceId: string; + namespace?: string; // namespace is undefined for the spaceId 'default' } export interface PlainTextEmail {