diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index ca51b1cdfea1b..5f6260eb2451c 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -353,6 +353,36 @@ describe('create()', () => { ); }); + test('validates connector: config and secrets', async () => { + const connectorValidator = ({}, secrets: { param1: '1' }) => { + if (secrets.param1 == null) { + return '[param1] is required'; + } + return null; + }; + actionTypeRegistry.register({ + id: 'my-action-type', + name: 'My action type', + minimumLicenseRequired: 'basic', + validate: { + connector: connectorValidator, + }, + executor, + }); + await expect( + actionsClient.create({ + action: { + name: 'my name', + actionTypeId: 'my-action-type', + config: {}, + secrets: {}, + }, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: [param1] is required"` + ); + }); + test(`throws an error when an action type doesn't exist`, async () => { await expect( actionsClient.create({ @@ -1539,6 +1569,40 @@ describe('update()', () => { ); }); + test('validates connector: config and secrets', async () => { + actionTypeRegistry.register({ + id: 'my-action-type', + name: 'My action type', + minimumLicenseRequired: 'basic', + validate: { + connector: () => { + return '[param1] is required'; + }, + }, + executor, + }); + unsecuredSavedObjectsClient.get.mockResolvedValueOnce({ + id: 'my-action', + type: 'action', + attributes: { + actionTypeId: 'my-action-type', + }, + references: [], + }); + await expect( + actionsClient.update({ + id: 'my-action', + action: { + name: 'my name', + config: {}, + secrets: {}, + }, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: [param1] is required"` + ); + }); + test('encrypts action type options unless specified not to', async () => { actionTypeRegistry.register({ id: 'my-action-type', diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index b391e50283ad1..25730bfd4c949 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -22,7 +22,7 @@ import { import { AuditLogger } from '../../security/server'; import { ActionType } from '../common'; import { ActionTypeRegistry } from './action_type_registry'; -import { validateConfig, validateSecrets, ActionExecutorContract } from './lib'; +import { validateConfig, validateSecrets, ActionExecutorContract, validateConnector } from './lib'; import { ActionResult, FindActionResult, @@ -150,7 +150,9 @@ export class ActionsClient { const actionType = this.actionTypeRegistry.get(actionTypeId); const validatedActionTypeConfig = validateConfig(actionType, config); const validatedActionTypeSecrets = validateSecrets(actionType, secrets); - + if (actionType.validate?.connector) { + validateConnector(actionType, { config, secrets }); + } this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId); this.auditLogger?.log( @@ -221,6 +223,9 @@ export class ActionsClient { const actionType = this.actionTypeRegistry.get(actionTypeId); const validatedActionTypeConfig = validateConfig(actionType, config); const validatedActionTypeSecrets = validateSecrets(actionType, secrets); + if (actionType.validate?.connector) { + validateConnector(actionType, { config, secrets }); + } this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId); diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/plugins/actions/server/builtin_action_types/email.test.ts index 710f0c84f0cef..48110e29ff911 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.test.ts @@ -12,7 +12,7 @@ jest.mock('./lib/send_email', () => ({ import { Logger } from '../../../../../src/core/server'; import { actionsConfigMock } from '../actions_config.mock'; -import { validateConfig, validateSecrets, validateParams } from '../lib'; +import { validateConfig, validateConnector, validateParams, validateSecrets } from '../lib'; import { createActionTypeRegistry } from './index.test'; import { sendEmail } from './lib/send_email'; import { actionsMock } from '../mocks'; @@ -303,6 +303,75 @@ describe('secrets validation', () => { }); }); +describe('connector validation: secrets with config', () => { + test('connector validation succeeds when username/password was populated for hasAuth true', () => { + const secrets: Record = { + user: 'bob', + password: 'supersecret', + }; + const config: Record = { + hasAuth: true, + }; + expect(validateConnector(actionType, { config, secrets })).toBeNull(); + }); + + test('connector validation succeeds when username/password not filled for hasAuth false', () => { + const secrets: Record = { + user: null, + password: null, + clientSecret: null, + }; + const config: Record = { + hasAuth: false, + }; + expect(validateConnector(actionType, { config, secrets })).toBeNull(); + expect(validateConnector(actionType, { config, secrets: {} })).toBeNull(); + expect(validateConnector(actionType, { config, secrets: { user: null } })).toBeNull(); + expect(validateConnector(actionType, { config, secrets: { password: null } })).toBeNull(); + }); + + test('connector validation fails when username/password was populated for hasAuth true', () => { + const secrets: Record = { + password: null, + user: null, + }; + const config: Record = { + hasAuth: true, + }; + // invalid user + expect(() => { + validateConnector(actionType, { config, secrets }); + }).toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: [user] is required"` + ); + }); + + test('connector validation succeeds when service is exchange_server and clientSecret is populated', () => { + const secrets: Record = { + clientSecret: '12345678', + }; + const config: Record = { + service: 'exchange_server', + }; + expect(validateConnector(actionType, { config, secrets })).toBeNull(); + }); + + test('connector validation fails when service is exchange_server and clientSecret is not populated', () => { + const secrets: Record = { + clientSecret: null, + }; + const config: Record = { + service: 'exchange_server', + }; + // invalid user + expect(() => { + validateConnector(actionType, { config, secrets }); + }).toThrowErrorMatchingInlineSnapshot( + `"error validating action type connector: [clientSecret] is required"` + ); + }); +}); + describe('params validation', () => { test('params validation succeeds when params is valid', () => { const params: Record = { diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.ts b/x-pack/plugins/actions/server/builtin_action_types/email.ts index fcd003286d5bb..624fb2b418f48 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.ts @@ -116,11 +116,13 @@ function validateConfig( export type ActionTypeSecretsType = TypeOf; -const SecretsSchema = schema.object({ +const SecretsSchemaProps = { user: schema.nullable(schema.string()), password: schema.nullable(schema.string()), clientSecret: schema.nullable(schema.string()), -}); +}; + +const SecretsSchema = schema.object(SecretsSchemaProps); // params definition @@ -167,6 +169,25 @@ interface GetActionTypeParams { configurationUtilities: ActionsConfigurationUtilities; } +function validateConnector( + config: ActionTypeConfigType, + secrets: ActionTypeSecretsType +): string | null { + if (config.service === AdditionalEmailServices.EXCHANGE) { + if (secrets.clientSecret == null) { + return '[clientSecret] is required'; + } + } else if (config.hasAuth && (secrets.password == null || secrets.user == null)) { + if (secrets.user == null) { + return '[user] is required'; + } + if (secrets.password == null) { + return '[password] is required'; + } + } + return null; +} + // action type definition export const ActionTypeId = '.email'; export function getActionType(params: GetActionTypeParams): EmailActionType { @@ -183,6 +204,7 @@ export function getActionType(params: GetActionTypeParams): EmailActionType { }), secrets: SecretsSchema, params: ParamsSchema, + connector: validateConnector, }, renderParameterTemplates, executor: curry(executor)({ logger, publicBaseUrl, configurationUtilities }), 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 ba7f750859d40..4175649454f71 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -273,6 +273,45 @@ test('throws an error when config is invalid', async () => { }); }); +test('throws an error when connector is invalid', async () => { + const actionType: jest.Mocked = { + id: 'test', + name: 'Test', + minimumLicenseRequired: 'basic', + validate: { + connector: () => { + return 'error'; + }, + }, + executor: jest.fn(), + }; + const actionSavedObject = { + id: '1', + type: 'action', + attributes: { + actionTypeId: 'test', + }, + references: [], + }; + const actionResult = { + id: actionSavedObject.id, + name: actionSavedObject.id, + actionTypeId: actionSavedObject.attributes.actionTypeId, + isPreconfigured: false, + }; + actionsClient.get.mockResolvedValueOnce(actionResult); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(actionSavedObject); + actionTypeRegistry.get.mockReturnValueOnce(actionType); + + const result = await actionExecutor.execute(executeParams); + expect(result).toEqual({ + actionId: '1', + status: 'error', + retry: false, + message: `error validating action type connector: error`, + }); +}); + test('throws an error when params is invalid', async () => { const actionType: jest.Mocked = { id: 'test', diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index d265bca237c3b..518d4582de2bc 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -9,7 +9,12 @@ import type { PublicMethodsOf } from '@kbn/utility-types'; import { Logger, KibanaRequest } from 'src/core/server'; import { cloneDeep } from 'lodash'; import { withSpan } from '@kbn/apm-utils'; -import { validateParams, validateConfig, validateSecrets } from './validate_with_schema'; +import { + validateParams, + validateConfig, + validateSecrets, + validateConnector, +} from './validate_with_schema'; import { ActionTypeExecutorResult, ActionTypeRegistryContract, @@ -142,11 +147,16 @@ export class ActionExecutor { let validatedParams: Record; let validatedConfig: Record; let validatedSecrets: Record; - try { validatedParams = validateParams(actionType, params); validatedConfig = validateConfig(actionType, config); validatedSecrets = validateSecrets(actionType, secrets); + if (actionType.validate?.connector) { + validateConnector(actionType, { + config, + secrets, + }); + } } catch (err) { span?.setOutcome('failure'); return { status: 'error', actionId, message: err.message, retry: false }; diff --git a/x-pack/plugins/actions/server/lib/index.ts b/x-pack/plugins/actions/server/lib/index.ts index c47325c19fad9..c52a8b14ee6d8 100644 --- a/x-pack/plugins/actions/server/lib/index.ts +++ b/x-pack/plugins/actions/server/lib/index.ts @@ -6,7 +6,12 @@ */ export { ExecutorError } from './executor_error'; -export { validateParams, validateConfig, validateSecrets } from './validate_with_schema'; +export { + validateParams, + validateConfig, + validateSecrets, + validateConnector, +} from './validate_with_schema'; export { TaskRunnerFactory } from './task_runner_factory'; export { ActionExecutor, ActionExecutorContract } from './action_executor'; export { ILicenseState, LicenseState } from './license_state'; diff --git a/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts b/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts index 480a3e31fcb59..4f0a11252eb48 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.test.ts @@ -7,7 +7,12 @@ import { schema } from '@kbn/config-schema'; -import { validateParams, validateConfig, validateSecrets } from './validate_with_schema'; +import { + validateParams, + validateConfig, + validateSecrets, + validateConnector, +} from './validate_with_schema'; import { ActionType, ExecutorType } from '../types'; const executor: ExecutorType<{}, {}, {}, void> = async (options) => { @@ -47,6 +52,9 @@ test('should validate when there are no individual validators', () => { result = validateSecrets(actionType, testValue); expect(result).toEqual(testValue); + + result = validateConnector(actionType, { config: testValue }); + expect(result).toBeNull(); }); test('should validate when validators return incoming value', () => { @@ -74,6 +82,9 @@ test('should validate when validators return incoming value', () => { result = validateSecrets(actionType, testValue); expect(result).toEqual(testValue); + + result = validateConnector(actionType, { config: testValue }); + expect(result).toBeNull(); }); test('should validate when validators return different values', () => { @@ -102,6 +113,9 @@ test('should validate when validators return different values', () => { result = validateSecrets(actionType, testValue); expect(result).toEqual(returnedValue); + + result = validateConnector(actionType, { config: testValue, secrets: { user: 'test' } }); + expect(result).toBeNull(); }); test('should throw with expected error when validators fail', () => { @@ -119,6 +133,9 @@ test('should throw with expected error when validators fail', () => { params: erroringValidator, config: erroringValidator, secrets: erroringValidator, + connector: () => { + return 'test error'; + }, }, }; @@ -135,6 +152,10 @@ test('should throw with expected error when validators fail', () => { expect(() => validateSecrets(actionType, testValue)).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: test error"` ); + + expect(() => + validateConnector(actionType, { config: testValue, secrets: { user: 'test' } }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action type connector: test error"`); }); test('should work with @kbn/config-schema', () => { @@ -148,6 +169,7 @@ test('should work with @kbn/config-schema', () => { params: testSchema, config: testSchema, secrets: testSchema, + connector: () => null, }, }; diff --git a/x-pack/plugins/actions/server/lib/validate_with_schema.ts b/x-pack/plugins/actions/server/lib/validate_with_schema.ts index 335fe4eee3da1..8ff0a3666c4b7 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.ts @@ -35,6 +35,22 @@ export function validateSecrets< return validateWithSchema(actionType, 'secrets', value); } +export function validateConnector< + Config extends ActionTypeConfig = ActionTypeConfig, + Secrets extends ActionTypeSecrets = ActionTypeSecrets, + Params extends ActionTypeParams = ActionTypeParams, + ExecutorResultData = void +>(actionType: ActionType, value: unknown) { + if (actionType.validate && actionType.validate.connector) { + const connectorValue = value as { config: Config; secrets: Secrets }; + const result = actionType.validate.connector(connectorValue.config, connectorValue.secrets); + if (result !== null) { + throw Boom.badRequest(`error validating action type connector: ${result}`); + } + } + return null; +} + type ValidKeys = 'params' | 'config' | 'secrets'; function validateWithSchema< @@ -45,7 +61,7 @@ function validateWithSchema< >( actionType: ActionType, key: ValidKeys, - value: unknown + value: unknown | { config: unknown; secrets: unknown } ): Record { if (actionType.validate) { let name; diff --git a/x-pack/plugins/actions/server/types.ts b/x-pack/plugins/actions/server/types.ts index 64250ca77fba4..627cd7028e5b1 100644 --- a/x-pack/plugins/actions/server/types.ts +++ b/x-pack/plugins/actions/server/types.ts @@ -111,6 +111,7 @@ export interface ActionType< params?: ValidatorType; config?: ValidatorType; secrets?: ValidatorType; + connector?: (config: Config, secrets: Secrets) => string | null; }; renderParameterTemplates?( params: Params, diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts index c353ae7b9ebae..d7716c5f30f66 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts @@ -284,6 +284,7 @@ export default function emailTest({ getService }: FtrProviderContext) { config: { service: '__json', from: 'jim@example.com', + hasAuth: false, }, }) .expect(200);