From 761daad5523280c898b83b6c21b245a5eff8a477 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Wed, 24 Aug 2022 11:53:28 -0400 Subject: [PATCH 1/7] Updating email first --- .../actions/server/action_type_registry.ts | 7 + .../plugins/actions/server/actions_client.ts | 18 +- .../server/builtin_action_types/email.test.ts | 171 +++++++++++------- .../server/builtin_action_types/email.ts | 44 +++-- .../actions/server/lib/action_executor.ts | 30 +-- .../server/lib/validate_with_schema.ts | 62 +++++-- x-pack/plugins/actions/server/types.ts | 10 +- 7 files changed, 234 insertions(+), 108 deletions(-) diff --git a/x-pack/plugins/actions/server/action_type_registry.ts b/x-pack/plugins/actions/server/action_type_registry.ts index c105034c7f687..a1466d2d40408 100644 --- a/x-pack/plugins/actions/server/action_type_registry.ts +++ b/x-pack/plugins/actions/server/action_type_registry.ts @@ -212,4 +212,11 @@ export class ActionTypeRegistry { supportedFeatureIds: actionType.supportedFeatureIds, })); } + + /** + * Returns the actions configuration utilities + */ + public getUtils(): ActionsConfigurationUtilities { + return this.actionsConfigUtils; + } } diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index ea4dd8d6b5e9e..d2777a8e3ae68 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -188,8 +188,13 @@ export class ActionsClient { } const actionType = this.actionTypeRegistry.get(actionTypeId); - const validatedActionTypeConfig = validateConfig(actionType, config); - const validatedActionTypeSecrets = validateSecrets(actionType, secrets); + const configurationUtilities = this.actionTypeRegistry.getUtils(); + const validatedActionTypeConfig = validateConfig(actionType, config, { + configurationUtilities, + }); + const validatedActionTypeSecrets = validateSecrets(actionType, secrets, { + configurationUtilities, + }); if (actionType.validate?.connector) { validateConnector(actionType, { config, secrets }); } @@ -262,8 +267,13 @@ export class ActionsClient { const { actionTypeId } = attributes; const { name, config, secrets } = action; const actionType = this.actionTypeRegistry.get(actionTypeId); - const validatedActionTypeConfig = validateConfig(actionType, config); - const validatedActionTypeSecrets = validateSecrets(actionType, secrets); + const configurationUtilities = this.actionTypeRegistry.getUtils(); + const validatedActionTypeConfig = validateConfig(actionType, config, { + configurationUtilities, + }); + const validatedActionTypeSecrets = validateSecrets(actionType, secrets, { + configurationUtilities, + }); if (actionType.validate?.connector) { validateConnector(actionType, { config, secrets }); } 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 55f0fc0cb5f70..e586526d74e76 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 @@ -25,6 +25,7 @@ import { EmailActionTypeExecutorOptions, } from './email'; import { ValidateEmailAddressesOptions } from '../../common'; +import { ActionsConfigurationUtilities } from '../actions_config'; const sendEmailMock = sendEmail as jest.Mock; @@ -34,10 +35,12 @@ const services = actionsMock.createServices(); let actionType: EmailActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: ActionsConfigurationUtilities; beforeEach(() => { jest.resetAllMocks(); const { actionTypeRegistry } = createActionTypeRegistry(); + configurationUtilities = actionTypeRegistry.getUtils(); actionType = actionTypeRegistry.get< ActionTypeConfigType, ActionTypeSecretsType, @@ -59,7 +62,7 @@ describe('config validation', () => { from: 'bob@example.com', hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, host: null, port: null, @@ -77,7 +80,7 @@ describe('config validation', () => { port: 8080, hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, service: 'other', secure: null, @@ -95,7 +98,7 @@ describe('config validation', () => { port: 8080, hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, secure: null, clientId: null, @@ -112,7 +115,7 @@ describe('config validation', () => { tenantId: '12345778', hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, secure: null, host: null, @@ -127,7 +130,7 @@ describe('config validation', () => { from: 'bob@example.com', hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, host: null, port: null, @@ -145,70 +148,86 @@ describe('config validation', () => { // empty object expect(() => { - validateConfig(actionType, {}); + validateConfig(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [from]: expected value of type [string] but got [undefined]"` ); // no service or host/port expect(() => { - validateConfig(actionType, baseConfig); + validateConfig(actionType, baseConfig, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [host]/[port] is required"` ); // host but no port expect(() => { - validateConfig(actionType, { ...baseConfig, host: 'elastic.co' }); + validateConfig(actionType, { ...baseConfig, host: 'elastic.co' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [port] is required"` ); // port but no host expect(() => { - validateConfig(actionType, { ...baseConfig, port: 8080 }); + validateConfig(actionType, { ...baseConfig, port: 8080 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [host] is required"` ); // invalid service expect(() => { - validateConfig(actionType, { - ...baseConfig, - service: 'bad-nodemailer-service', - }); + validateConfig( + actionType, + { + ...baseConfig, + service: 'bad-nodemailer-service', + }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [service] value 'bad-nodemailer-service' is not valid"` ); // invalid exchange_server no clientId and no tenantId expect(() => { - validateConfig(actionType, { - ...baseConfig, - service: 'exchange_server', - }); + validateConfig( + actionType, + { + ...baseConfig, + service: 'exchange_server', + }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [clientId]/[tenantId] is required"` ); // invalid exchange_server no clientId expect(() => { - validateConfig(actionType, { - ...baseConfig, - service: 'exchange_server', - tenantId: '342342342', - }); + validateConfig( + actionType, + { + ...baseConfig, + service: 'exchange_server', + tenantId: '342342342', + }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [clientId] is required"` ); // invalid exchange_server no tenantId expect(() => { - validateConfig(actionType, { - ...baseConfig, - service: 'exchange_server', - clientId: '12345667', - }); + validateConfig( + actionType, + { + ...baseConfig, + service: 'exchange_server', + clientId: '12345667', + }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [tenantId] is required"` ); @@ -219,12 +238,13 @@ describe('config validation', () => { const NODEMAILER_AOL_SERVICE_HOST = 'smtp.aol.com'; test('config validation handles email host in allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + isHostnameAllowed: (hostname: string) => hostname === NODEMAILER_AOL_SERVICE_HOST, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - isHostnameAllowed: (hostname) => hostname === NODEMAILER_AOL_SERVICE_HOST, - }, + configurationUtilities: configUtils, }); const baseConfig = { from: 'bob@example.com', @@ -249,46 +269,56 @@ describe('config validation', () => { port: 42, }; - const validatedConfig1 = validateConfig(actionType, allowedHosts1); + const validatedConfig1 = validateConfig(actionType, allowedHosts1, { + configurationUtilities: configUtils, + }); expect(validatedConfig1.service).toEqual(allowedHosts1.service); expect(validatedConfig1.from).toEqual(allowedHosts1.from); - const validatedConfig2 = validateConfig(actionType, allowedHosts2); + const validatedConfig2 = validateConfig(actionType, allowedHosts2, { + configurationUtilities: configUtils, + }); expect(validatedConfig2.host).toEqual(allowedHosts2.host); expect(validatedConfig2.port).toEqual(allowedHosts2.port); expect(validatedConfig2.from).toEqual(allowedHosts2.from); expect(() => { - validateConfig(actionType, notAllowedHosts1); + validateConfig(actionType, notAllowedHosts1, { + configurationUtilities: configUtils, + }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [service] value 'gmail' resolves to host 'smtp.gmail.com' which is not in the allowedHosts configuration"` ); expect(() => { - validateConfig(actionType, notAllowedHosts2); + validateConfig(actionType, notAllowedHosts2, { configurationUtilities: configUtils }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [host] value 'smtp.gmail.com' is not in the allowedHosts configuration"` ); }); test('config validation for emails calls validateEmailAddresses', async () => { - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.validateEmailAddresses.mockImplementation(validateEmailAddressesImpl); + const configUtils = actionsConfigMock.create(); + configUtils.validateEmailAddresses.mockImplementation(validateEmailAddressesImpl); const basicActionType = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); expect(() => { - validateConfig(basicActionType, { - from: 'badmail', - service: 'gmail', - }); + validateConfig( + basicActionType, + { + from: 'badmail', + service: 'gmail', + }, + { configurationUtilities: configUtils } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [from]: stub for actual message"` ); - expect(configurationUtilities.validateEmailAddresses).toHaveBeenNthCalledWith(1, ['badmail']); + expect(configUtils.validateEmailAddresses).toHaveBeenNthCalledWith(1, ['badmail']); }); }); @@ -298,7 +328,10 @@ describe('secrets validation', () => { user: 'bob', password: 'supersecret', }; - expect(validateSecrets(actionType, secrets)).toEqual({ ...secrets, clientSecret: null }); + expect(validateSecrets(actionType, secrets, { configurationUtilities })).toEqual({ + ...secrets, + clientSecret: null, + }); }); test('secrets validation succeeds when secrets props are null/undefined', () => { @@ -307,16 +340,20 @@ describe('secrets validation', () => { password: null, clientSecret: null, }; - expect(validateSecrets(actionType, {})).toEqual(secrets); - expect(validateSecrets(actionType, { user: null })).toEqual(secrets); - expect(validateSecrets(actionType, { password: null })).toEqual(secrets); + expect(validateSecrets(actionType, {}, { configurationUtilities })).toEqual(secrets); + expect(validateSecrets(actionType, { user: null }, { configurationUtilities })).toEqual( + secrets + ); + expect(validateSecrets(actionType, { password: null }, { configurationUtilities })).toEqual( + secrets + ); }); test('secrets validation succeeds when secrets is valid for OAuth 2.0 Client Credentials', () => { const secrets: Record = { clientSecret: '12345678', }; - expect(validateSecrets(actionType, secrets)).toEqual({ + expect(validateSecrets(actionType, secrets, { configurationUtilities })).toEqual({ ...secrets, user: null, password: null, @@ -400,7 +437,7 @@ describe('params validation', () => { subject: 'this is a test', message: 'this is the message', }; - expect(validateParams(actionType, params)).toMatchInlineSnapshot(` + expect(validateParams(actionType, params, { configurationUtilities })).toMatchInlineSnapshot(` Object { "bcc": Array [], "cc": Array [], @@ -420,35 +457,39 @@ describe('params validation', () => { test('params validation fails when params is not valid', () => { // empty object expect(() => { - validateParams(actionType, {}); + validateParams(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [subject]: expected value of type [string] but got [undefined]"` ); }); test('params validation for emails calls validateEmailAddresses', async () => { - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.validateEmailAddresses.mockImplementation(validateEmailAddressesImpl); + const configUtils = actionsConfigMock.create(); + configUtils.validateEmailAddresses.mockImplementation(validateEmailAddressesImpl); const basicActionType = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); expect(() => { - validateParams(basicActionType, { - to: ['to@example.com'], - cc: ['cc@example.com'], - bcc: ['bcc@example.com'], - subject: 'this is a test', - message: 'this is the message', - }); + validateParams( + basicActionType, + { + to: ['to@example.com'], + cc: ['cc@example.com'], + bcc: ['bcc@example.com'], + subject: 'this is a test', + message: 'this is the message', + }, + { configurationUtilities: configUtils } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [to/cc/bcc]: stub for actual message"` ); const allEmails = ['to@example.com', 'cc@example.com', 'bcc@example.com']; - expect(configurationUtilities.validateEmailAddresses).toHaveBeenNthCalledWith(1, allEmails, { + expect(configUtils.validateEmailAddresses).toHaveBeenNthCalledWith(1, allEmails, { treatMustacheTemplatesAsValid: true, }); }); @@ -759,12 +800,12 @@ describe('execute()', () => { }); test('ensure execution runs validator with allowMustache false', async () => { - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.validateEmailAddresses.mockImplementation(validateEmailAddressesImpl); + const configUtils = actionsConfigMock.create(); + configUtils.validateEmailAddresses.mockImplementation(validateEmailAddressesImpl); const testActionType = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); const customExecutorOptions: EmailActionTypeExecutorOptions = { @@ -782,7 +823,7 @@ describe('execute()', () => { "status": "error", } `); - expect(configurationUtilities.validateEmailAddresses.mock.calls).toMatchInlineSnapshot(` + expect(configUtils.validateEmailAddresses.mock.calls).toMatchInlineSnapshot(` Array [ Array [ Array [ 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 5f0b74d9c7208..fb9a01ac5e7cd 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.ts @@ -21,7 +21,12 @@ import { import { sendEmail, JSON_TRANSPORT_SERVICE, SendEmailOptions, Transport } from './lib/send_email'; import { portSchema } from './lib/schemas'; -import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { + ActionType, + ActionTypeExecutorOptions, + ActionTypeExecutorResult, + ValidatorServices, +} from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { renderMustacheString, renderMustacheObject } from '../lib/mustache_renderer'; @@ -66,13 +71,14 @@ const ConfigSchemaProps = { const ConfigSchema = schema.object(ConfigSchemaProps); function validateConfig( - configurationUtilities: ActionsConfigurationUtilities, - configObject: ActionTypeConfigType + configObject: ActionTypeConfigType, + validatorServices?: ValidatorServices ): string | void { const config = configObject; + const { configurationUtilities } = validatorServices || {}; const emails = [config.from]; - const invalidEmailsMessage = configurationUtilities.validateEmailAddresses(emails); + const invalidEmailsMessage = configurationUtilities?.validateEmailAddresses(emails); if (!!invalidEmailsMessage) { return `[from]: ${invalidEmailsMessage}`; } @@ -109,7 +115,7 @@ function validateConfig( return '[port] is required'; } - if (!configurationUtilities.isHostnameAllowed(config.host)) { + if (!configurationUtilities?.isHostnameAllowed(config.host)) { return `[host] value '${config.host}' is not in the allowedHosts configuration`; } } else { @@ -118,7 +124,7 @@ function validateConfig( if (host == null) { return `[service] value '${config.service}' is not valid`; } - if (!configurationUtilities.isHostnameAllowed(host)) { + if (!configurationUtilities?.isHostnameAllowed(host)) { return `[service] value '${config.service}' resolves to host '${host}' which is not in the allowedHosts configuration`; } } @@ -161,9 +167,11 @@ const ParamsSchemaProps = { const ParamsSchema = schema.object(ParamsSchemaProps); function validateParams( - configurationUtilities: ActionsConfigurationUtilities, - paramsObject: unknown + paramsObject: unknown, + validatorServices?: ValidatorServices ): string | void { + const { configurationUtilities } = validatorServices || {}; + // avoids circular reference ... const params = paramsObject as ActionParamsType; @@ -175,7 +183,7 @@ function validateParams( } const emails = withoutMustacheTemplate(to.concat(cc).concat(bcc)); - const invalidEmailsMessage = configurationUtilities.validateEmailAddresses(emails, { + const invalidEmailsMessage = configurationUtilities?.validateEmailAddresses(emails, { treatMustacheTemplatesAsValid: true, }); if (invalidEmailsMessage) { @@ -224,13 +232,17 @@ export function getActionType(params: GetActionTypeParams): EmailActionType { SecurityConnectorFeatureId, ], validate: { - config: schema.object(ConfigSchemaProps, { - validate: curry(validateConfig)(configurationUtilities), - }), - secrets: SecretsSchema, - params: schema.object(ParamsSchemaProps, { - validate: curry(validateParams)(configurationUtilities), - }), + config: { + validateSchema: ConfigSchema, + validate: validateConfig, + }, + secrets: { + validateSchema: SecretsSchema, + }, + params: { + validateSchema: ParamsSchema, + validate: validateParams, + }, connector: validateConnector, }, renderParameterTemplates, diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index a27cfcccef9c8..dd4f15894ee28 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -26,6 +26,7 @@ import { GetServicesFunction, PreConfiguredAction, RawAction, + ValidatorServices, } from '../types'; import { EVENT_LOG_ACTIONS } from '../constants/event_log'; import { ActionsClient } from '../actions_client'; @@ -206,13 +207,17 @@ export class ActionExecutor { let rawResult: ActionTypeExecutorRawResult; try { - const { validatedParams, validatedConfig, validatedSecrets } = validateAction({ - actionId, - actionType, - params, - config, - secrets, - }); + const configurationUtilities = actionTypeRegistry.getUtils(); + const { validatedParams, validatedConfig, validatedSecrets } = validateAction( + { + actionId, + actionType, + params, + config, + secrets, + }, + { configurationUtilities } + ); rawResult = await actionType.executor({ actionId, @@ -426,15 +431,18 @@ interface ValidateActionOpts { secrets: unknown; } -function validateAction({ actionId, actionType, params, config, secrets }: ValidateActionOpts) { +function validateAction( + { actionId, actionType, params, config, secrets }: ValidateActionOpts, + validatorServices: ValidatorServices +) { let validatedParams: Record; let validatedConfig: Record; let validatedSecrets: Record; try { - validatedParams = validateParams(actionType, params); - validatedConfig = validateConfig(actionType, config); - validatedSecrets = validateSecrets(actionType, secrets); + validatedParams = validateParams(actionType, params, validatorServices); + validatedConfig = validateConfig(actionType, config, validatorServices); + validatedSecrets = validateSecrets(actionType, secrets, validatorServices); if (actionType.validate?.connector) { validateConnector(actionType, { config, 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 b8a861566acd2..7d45888a17725 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.ts @@ -6,15 +6,25 @@ */ import Boom from '@hapi/boom'; -import { ActionType, ActionTypeConfig, ActionTypeSecrets, ActionTypeParams } from '../types'; +import { + ActionType, + ActionTypeConfig, + ActionTypeSecrets, + ActionTypeParams, + ValidatorServices, +} from '../types'; export function validateParams< Config extends ActionTypeConfig = ActionTypeConfig, Secrets extends ActionTypeSecrets = ActionTypeSecrets, Params extends ActionTypeParams = ActionTypeParams, ExecutorResultData = void ->(actionType: ActionType, value: unknown) { - return validateWithSchema(actionType, 'params', value); +>( + actionType: ActionType, + value: unknown, + validatorServices: ValidatorServices +) { + return validateWithSchema(actionType, 'params', value, validatorServices); } export function validateConfig< @@ -22,8 +32,12 @@ export function validateConfig< Secrets extends ActionTypeSecrets = ActionTypeSecrets, Params extends ActionTypeParams = ActionTypeParams, ExecutorResultData = void ->(actionType: ActionType, value: unknown) { - return validateWithSchema(actionType, 'config', value); +>( + actionType: ActionType, + value: unknown, + validatorServices: ValidatorServices +) { + return validateWithSchema(actionType, 'config', value, validatorServices); } export function validateSecrets< @@ -31,8 +45,12 @@ export function validateSecrets< Secrets extends ActionTypeSecrets = ActionTypeSecrets, Params extends ActionTypeParams = ActionTypeParams, ExecutorResultData = void ->(actionType: ActionType, value: unknown) { - return validateWithSchema(actionType, 'secrets', value); +>( + actionType: ActionType, + value: unknown, + validatorServices: ValidatorServices +) { + return validateWithSchema(actionType, 'secrets', value, validatorServices); } export function validateConnector< @@ -73,7 +91,8 @@ function validateWithSchema< >( actionType: ActionType, key: ValidKeys, - value: unknown | { config: unknown; secrets: unknown } + value: unknown | { config: unknown; secrets: unknown }, + validatorServices: ValidatorServices ): Record { if (actionType.validate) { let name; @@ -82,20 +101,41 @@ function validateWithSchema< case 'params': name = 'action params'; if (actionType.validate.params) { - return actionType.validate.params.validate(value); + const params = actionType.validate.params.validateSchema.validate(value); + if (actionType.validate.params.validate) { + const result = actionType.validate.params.validate(params, validatorServices); + if (result) { + throw new Error(result); + } + } + return params; } break; case 'config': name = 'action type config'; if (actionType.validate.config) { - return actionType.validate.config.validate(value); + const config = actionType.validate.config.validateSchema.validate(value); + if (actionType.validate.config.validate) { + const result = actionType.validate.config.validate(config, validatorServices); + if (result) { + throw new Error(result); + } + } + return config; } break; case 'secrets': name = 'action type secrets'; if (actionType.validate.secrets) { - return actionType.validate.secrets.validate(value); + const secrets = actionType.validate.secrets.validateSchema.validate(value); + if (actionType.validate.secrets.validate) { + const result = actionType.validate.secrets.validate(secrets, validatorServices); + if (result) { + throw new Error(result); + } + } + return secrets; } break; default: diff --git a/x-pack/plugins/actions/server/types.ts b/x-pack/plugins/actions/server/types.ts index 140754eeafe21..1facab829e244 100644 --- a/x-pack/plugins/actions/server/types.ts +++ b/x-pack/plugins/actions/server/types.ts @@ -21,6 +21,7 @@ import { ActionsClient } from './actions_client'; import { ActionTypeExecutorResult } from '../common'; import { TaskInfo } from './lib/action_executor'; import { ConnectorTokenClient } from './builtin_action_types/lib/connector_token_client'; +import { ActionsConfigurationUtilities } from './actions_config'; export type { ActionTypeExecutorResult, ActionTypeExecutorRawResult } from '../common'; export type { GetFieldsByIssueTypeResponse as JiraGetFieldsResponse } from './builtin_action_types/jira/types'; @@ -94,7 +95,14 @@ export type ExecutorType = ( ) => Promise>; interface ValidatorType { - validate(value: unknown): Type; + validateSchema: { + validate(value: unknown): Type; + }; + validate?: (value: Type, validatorServices?: ValidatorServices) => string | void; +} + +export interface ValidatorServices { + configurationUtilities: ActionsConfigurationUtilities; } export interface ActionValidationService { From e5a49e8f1cdca0064164a3eba48c8b3abf7af494 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Wed, 24 Aug 2022 16:53:10 -0400 Subject: [PATCH 2/7] Updating the other connector types --- .../cases_webhook/index.ts | 23 +-- .../cases_webhook/types.ts | 11 +- .../cases_webhook/validators.ts | 13 +- .../builtin_action_types/es_index.test.ts | 37 +++-- .../server/builtin_action_types/es_index.ts | 8 +- .../server/builtin_action_types/jira/index.ts | 24 +-- .../server/builtin_action_types/jira/types.ts | 6 +- .../builtin_action_types/jira/validators.ts | 13 +- .../builtin_action_types/pagerduty.test.ts | 137 +++++++++++---- .../server/builtin_action_types/pagerduty.ts | 29 +++- .../builtin_action_types/resilient/index.ts | 24 +-- .../builtin_action_types/resilient/types.ts | 6 +- .../resilient/validators.ts | 13 +- .../builtin_action_types/server_log.test.ts | 31 +++- .../server/builtin_action_types/server_log.ts | 4 +- .../builtin_action_types/servicenow/index.ts | 62 ++++--- .../builtin_action_types/servicenow/types.ts | 5 +- .../servicenow/validators.test.ts | 156 ++++++++++-------- .../servicenow/validators.ts | 14 +- .../server/builtin_action_types/slack.test.ts | 99 ++++++----- .../server/builtin_action_types/slack.ts | 19 ++- .../builtin_action_types/swimlane/index.ts | 23 +-- .../builtin_action_types/swimlane/types.ts | 6 +- .../swimlane/validators.ts | 20 ++- .../server/builtin_action_types/teams.test.ts | 67 +++++--- .../server/builtin_action_types/teams.ts | 25 ++- .../builtin_action_types/webhook.test.ts | 47 +++--- .../server/builtin_action_types/webhook.ts | 29 +++- .../builtin_action_types/xmatters.test.ts | 63 +++---- .../server/builtin_action_types/xmatters.ts | 39 +++-- 30 files changed, 664 insertions(+), 389 deletions(-) diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts index c9b6c41d478cf..54961cbc77562 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts @@ -6,7 +6,6 @@ */ import { curry } from 'lodash'; -import { schema } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; import { CasesConnectorFeatureId } from '../../../common'; import { @@ -22,8 +21,8 @@ import { ActionsConfigurationUtilities } from '../../actions_config'; import { createExternalService } from './service'; import { ExecutorParamsSchema, - ExternalIncidentServiceConfiguration, - ExternalIncidentServiceSecretConfiguration, + ExternalIncidentServiceConfigurationSchema, + ExternalIncidentServiceSecretConfigurationSchema, } from './schema'; import { api } from './api'; import { validate } from './validators'; @@ -50,13 +49,17 @@ export function getActionType({ minimumLicenseRequired: 'gold', name: i18n.NAME, validate: { - config: schema.object(ExternalIncidentServiceConfiguration, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(ExternalIncidentServiceSecretConfiguration, { - validate: curry(validate.secrets), - }), - params: ExecutorParamsSchema, + config: { + validateSchema: ExternalIncidentServiceConfigurationSchema, + validate: validate.config, + }, + secrets: { + validateSchema: ExternalIncidentServiceSecretConfigurationSchema, + validate: validate.secrets, + }, + params: { + validateSchema: ExecutorParamsSchema, + }, connector: validate.connector, }, executor: curry(executor)({ logger, configurationUtilities }), diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts index 095b07fd4c9f7..85d25b7cf5498 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts @@ -7,7 +7,7 @@ import { TypeOf } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; -import { ActionsConfigurationUtilities } from '../../actions_config'; +import { ValidatorServices } from '../../types'; import { ExecutorParamsSchema, ExecutorSubActionPushParamsSchema, @@ -40,10 +40,13 @@ export interface ExternalServiceCredentials { export interface ExternalServiceValidation { config: ( - configurationUtilities: ActionsConfigurationUtilities, - configObject: CasesWebhookPublicConfigurationType + configObject: CasesWebhookPublicConfigurationType, + validatorServices?: ValidatorServices + ) => void; + secrets: ( + secrets: CasesWebhookSecretConfigurationType, + validatorServices?: ValidatorServices ) => void; - secrets: (secrets: CasesWebhookSecretConfigurationType) => void; connector: ( configObject: CasesWebhookPublicConfigurationType, secrets: CasesWebhookSecretConfigurationType diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts index 91e5ccbeb8715..4c8d8643fce16 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts @@ -12,11 +12,13 @@ import { CasesWebhookSecretConfigurationType, ExternalServiceValidation, } from './types'; +import { ValidatorServices } from '../../types'; const validateConfig = ( - configurationUtilities: ActionsConfigurationUtilities, - configObject: CasesWebhookPublicConfigurationType + configObject: CasesWebhookPublicConfigurationType, + validatorServices?: ValidatorServices ) => { + const { configurationUtilities } = validatorServices || {}; const { createCommentUrl, createIncidentUrl, @@ -41,7 +43,7 @@ const validateConfig = ( return i18n.INVALID_URL(err, url); } try { - configurationUtilities.ensureUriAllowed(url); + configurationUtilities?.ensureUriAllowed(url); } catch (allowListError) { return i18n.CONFIG_ERR(allowListError.message); } @@ -59,7 +61,10 @@ export const validateConnector = ( return i18n.INVALID_USER_PW; }; -export const validateSecrets = (secrets: CasesWebhookSecretConfigurationType) => { +export const validateSecrets = ( + secrets: CasesWebhookSecretConfigurationType, + validatorServices?: ValidatorServices +) => { // user and password must be set together (or not at all) if (!secrets.password && !secrets.user) return; if (secrets.password && secrets.user) return; diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts index 4dfcccdbc13f7..8a87255dc1210 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts @@ -20,16 +20,19 @@ import { } from './es_index'; import { AlertHistoryEsIndexConnectorId } from '../../common'; import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks'; +import { ActionsConfigurationUtilities } from '../actions_config'; const ACTION_TYPE_ID = '.index'; const services = actionsMock.createServices(); let actionType: ESIndexActionType; +let configurationUtilities: ActionsConfigurationUtilities; beforeAll(() => { const { actionTypeRegistry } = createActionTypeRegistry(); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); + configurationUtilities = actionTypeRegistry.getUtils(); }); beforeEach(() => { @@ -50,7 +53,7 @@ describe('config validation', () => { refresh: false, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, index: 'testing-123', refresh: false, @@ -58,7 +61,7 @@ describe('config validation', () => { }); config.executionTimeField = 'field-123'; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, index: 'testing-123', refresh: false, @@ -66,7 +69,7 @@ describe('config validation', () => { }); config.executionTimeField = null; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...config, index: 'testing-123', refresh: false, @@ -76,14 +79,18 @@ describe('config validation', () => { delete config.index; expect(() => { - validateConfig(actionType, { index: 666 }); + validateConfig(actionType, { index: 666 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [index]: expected value of type [string] but got [number]"` ); delete config.executionTimeField; expect(() => { - validateConfig(actionType, { index: 'testing-123', executionTimeField: true }); + validateConfig( + actionType, + { index: 'testing-123', executionTimeField: true }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot(` "error validating action type config: [executionTimeField]: types that failed validation: - [executionTimeField.0]: expected value of type [string] but got [boolean] @@ -92,7 +99,11 @@ describe('config validation', () => { delete config.refresh; expect(() => { - validateConfig(actionType, { index: 'testing-123', refresh: 'foo' }); + validateConfig( + actionType, + { index: 'testing-123', refresh: 'foo' }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [refresh]: expected value of type [boolean] but got [string]"` ); @@ -104,7 +115,7 @@ describe('config validation', () => { }; expect(() => { - validateConfig(actionType, baseConfig); + validateConfig(actionType, baseConfig, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [index]: expected value of type [string] but got [undefined]"` ); @@ -117,7 +128,7 @@ describe('params validation', () => { documents: [{ rando: 'thing' }], indexOverride: null, }; - expect(validateParams(actionType, params)).toMatchInlineSnapshot(` + expect(validateParams(actionType, params, { configurationUtilities })).toMatchInlineSnapshot(` Object { "documents": Array [ Object { @@ -131,19 +142,23 @@ describe('params validation', () => { test('params validation fails when params is not valid', () => { expect(() => { - validateParams(actionType, { documents: [{}], jim: 'bob' }); + validateParams(actionType, { documents: [{}], jim: 'bob' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [jim]: definition for this key is missing"` ); expect(() => { - validateParams(actionType, {}); + validateParams(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [documents]: expected value of type [array] but got [undefined]"` ); expect(() => { - validateParams(actionType, { documents: ['should be an object'] }); + validateParams( + actionType, + { documents: ['should be an object'] }, + { configurationUtilities } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [documents.0]: could not parse record value from json input"` ); diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.ts index 24e24413cfd73..a8b6caffcca9a 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/es_index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/es_index.ts @@ -72,8 +72,12 @@ export function getActionType({ logger }: { logger: Logger }): ESIndexActionType SecurityConnectorFeatureId, ], validate: { - config: ConfigSchema, - params: ParamsSchema, + config: { + validateSchema: ConfigSchema, + }, + params: { + validateSchema: ParamsSchema, + }, }, executor: curry(executor)({ logger }), renderParameterTemplates, diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts index a20cecf617e67..ea9ce5124ccb3 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts @@ -6,13 +6,13 @@ */ import { curry } from 'lodash'; -import { schema, TypeOf } from '@kbn/config-schema'; +import { TypeOf } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; import { validate } from './validators'; import { - ExternalIncidentServiceConfiguration, - ExternalIncidentServiceSecretConfiguration, + ExternalIncidentServiceConfigurationSchema, + ExternalIncidentServiceSecretConfigurationSchema, ExecutorParamsSchema, } from './schema'; import { ActionsConfigurationUtilities } from '../../actions_config'; @@ -77,13 +77,17 @@ export function getActionType( SecurityConnectorFeatureId, ], validate: { - config: schema.object(ExternalIncidentServiceConfiguration, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(ExternalIncidentServiceSecretConfiguration, { - validate: curry(validate.secrets)(configurationUtilities), - }), - params: ExecutorParamsSchema, + config: { + validateSchema: ExternalIncidentServiceConfigurationSchema, + validate: validate.config, + }, + secrets: { + validateSchema: ExternalIncidentServiceSecretConfigurationSchema, + validate: validate.config, + }, + params: { + validateSchema: ExecutorParamsSchema, + }, }, executor: curry(executor)({ logger, configurationUtilities }), }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts index ad504585b6f4b..8f540b8194d6f 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts @@ -22,7 +22,7 @@ import { ExecutorSubActionGetIssueParamsSchema, ExecutorSubActionCommonFieldsParamsSchema, } from './schema'; -import { ActionsConfigurationUtilities } from '../../actions_config'; +import { ValidatorServices } from '../../types'; export type JiraPublicConfigurationType = TypeOf; export type JiraSecretConfigurationType = TypeOf< @@ -38,8 +38,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configurationUtilities: ActionsConfigurationUtilities, configObject: any) => void; - secrets: (configurationUtilities: ActionsConfigurationUtilities, secrets: any) => void; + config: (configObject: any, validatorServices?: ValidatorServices) => void; + secrets: (secrets: any, validatorServices?: ValidatorServices) => void; } export interface ExternalServiceIncidentResponse { diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts index 96c4b7204fd9d..0cbdae09edd28 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { ActionsConfigurationUtilities } from '../../actions_config'; import { JiraPublicConfigurationType, JiraSecretConfigurationType, @@ -13,21 +12,23 @@ import { } from './types'; import * as i18n from './translations'; +import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( - configurationUtilities: ActionsConfigurationUtilities, - configObject: JiraPublicConfigurationType + configObject: JiraPublicConfigurationType, + validatorServices?: ValidatorServices ) => { + const { configurationUtilities } = validatorServices || {}; try { - configurationUtilities.ensureUriAllowed(configObject.apiUrl); + configurationUtilities?.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } }; export const validateCommonSecrets = ( - configurationUtilities: ActionsConfigurationUtilities, - secrets: JiraSecretConfigurationType + secrets: JiraSecretConfigurationType, + validatorServices?: ValidatorServices ) => {}; export const validate: ExternalServiceValidation = { diff --git a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts index c3628acc43b2f..a601532366fc4 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts @@ -26,6 +26,7 @@ import { PagerDutyActionType, PagerDutyActionTypeExecutorOptions, } from './pagerduty'; +import { ActionsConfigurationUtilities } from '../actions_config'; const postPagerdutyMock = postPagerduty as jest.Mock; @@ -35,6 +36,7 @@ const services: Services = actionsMock.createServices(); let actionType: PagerDutyActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: ActionsConfigurationUtilities; beforeAll(() => { const { logger, actionTypeRegistry } = createActionTypeRegistry(); @@ -44,6 +46,7 @@ beforeAll(() => { ActionParamsType >(ACTION_TYPE_ID); mockedLogger = logger; + configurationUtilities = actionTypeRegistry.getUtils(); }); describe('get()', () => { @@ -55,47 +58,65 @@ describe('get()', () => { describe('validateConfig()', () => { test('should validate and pass when config is valid', () => { - expect(validateConfig(actionType, {})).toEqual({ apiUrl: null }); - expect(validateConfig(actionType, { apiUrl: 'bar' })).toEqual({ apiUrl: 'bar' }); + expect(validateConfig(actionType, {}, { configurationUtilities })).toEqual({ apiUrl: null }); + expect( + validateConfig( + actionType, + { + apiUrl: 'bar', + }, + { configurationUtilities } + ) + ).toEqual({ apiUrl: 'bar' }); }); test('should validate and throw error when config is invalid', () => { expect(() => { - validateConfig(actionType, { shouldNotBeHere: true }); + validateConfig(actionType, { shouldNotBeHere: true }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: [shouldNotBeHere]: definition for this key is missing"` ); }); test('should validate and pass when the pagerduty url is added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (url: string) => { + expect(url).toEqual('https://events.pagerduty.com/v2/enqueue'); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (url) => { - expect(url).toEqual('https://events.pagerduty.com/v2/enqueue'); - }, - }, + configurationUtilities: configUtils, }); expect( - validateConfig(actionType, { apiUrl: 'https://events.pagerduty.com/v2/enqueue' }) + validateConfig( + actionType, + { apiUrl: 'https://events.pagerduty.com/v2/enqueue' }, + { configurationUtilities: configUtils } + ) ).toEqual({ apiUrl: 'https://events.pagerduty.com/v2/enqueue' }); }); test('config validation returns an error if the specified URL isnt added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (_: string) => { + throw new Error(`target url is not added to allowedHosts`); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (_) => { - throw new Error(`target url is not added to allowedHosts`); - }, - }, + configurationUtilities: configUtils, }); expect(() => { - validateConfig(actionType, { apiUrl: 'https://events.pagerduty.com/v2/enqueue' }); + validateConfig( + actionType, + { apiUrl: 'https://events.pagerduty.com/v2/enqueue' }, + { configurationUtilities: configUtils } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: error configuring pagerduty action: target url is not added to allowedHosts"` ); @@ -105,20 +126,20 @@ describe('validateConfig()', () => { describe('validateSecrets()', () => { test('should validate and pass when secrets is valid', () => { const routingKey = 'super-secret'; - expect(validateSecrets(actionType, { routingKey })).toEqual({ + expect(validateSecrets(actionType, { routingKey }, { configurationUtilities })).toEqual({ routingKey, }); }); test('should validate and throw error when secrets is invalid', () => { expect(() => { - validateSecrets(actionType, { routingKey: false }); + validateSecrets(actionType, { routingKey: false }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [routingKey]: expected value of type [string] but got [boolean]"` ); expect(() => { - validateSecrets(actionType, {}); + validateSecrets(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [routingKey]: expected value of type [string] but got [undefined]"` ); @@ -127,7 +148,7 @@ describe('validateSecrets()', () => { describe('validateParams()', () => { test('should validate and pass when params is valid', () => { - expect(validateParams(actionType, {})).toEqual({}); + expect(validateParams(actionType, {}, { configurationUtilities })).toEqual({}); const params = { eventAction: 'trigger', @@ -140,12 +161,12 @@ describe('validateParams()', () => { group: 'a group', class: 'a class', }; - expect(validateParams(actionType, params)).toEqual(params); + expect(validateParams(actionType, params, { configurationUtilities })).toEqual(params); }); test('should validate and throw error when params is invalid', () => { expect(() => { - validateParams(actionType, { eventAction: 'ackynollage' }); + validateParams(actionType, { eventAction: 'ackynollage' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot(` "error validating action params: [eventAction]: types that failed validation: - [eventAction.0]: expected value to equal [trigger] @@ -157,12 +178,28 @@ describe('validateParams()', () => { test('should validate and pass when valid timestamp has spaces', () => { const randoDate = new Date('1963-09-23T01:23:45Z').toISOString(); const timestamp = ` ${randoDate}`; - expect(validateParams(actionType, { timestamp })).toEqual({ timestamp }); + expect( + validateParams( + actionType, + { + timestamp, + }, + { configurationUtilities } + ) + ).toEqual({ timestamp }); }); test('should validate and pass when timestamp is empty string', () => { const timestamp = ''; - expect(validateParams(actionType, { timestamp })).toEqual({ timestamp }); + expect( + validateParams( + actionType, + { + timestamp, + }, + { configurationUtilities } + ) + ).toEqual({ timestamp }); }); test('should validate and pass all the valid ISO-8601 formatted dates', () => { @@ -170,25 +207,57 @@ describe('validateParams()', () => { const date2 = '2011-05-06T03:30-07'; const date3 = '2011-05-06'; - expect(validateParams(actionType, { timestamp: date1 })).toEqual({ timestamp: date1 }); - expect(validateParams(actionType, { timestamp: date2 })).toEqual({ timestamp: date2 }); - expect(validateParams(actionType, { timestamp: date3 })).toEqual({ timestamp: date3 }); + expect( + validateParams( + actionType, + { + timestamp: date1, + }, + { configurationUtilities } + ) + ).toEqual({ timestamp: date1 }); + expect( + validateParams( + actionType, + { + timestamp: date2, + }, + { configurationUtilities } + ) + ).toEqual({ timestamp: date2 }); + expect( + validateParams( + actionType, + { + timestamp: date3, + }, + { configurationUtilities } + ) + ).toEqual({ timestamp: date3 }); }); test('should validate and throw error when timestamp is invalid', () => { const timestamp = `1963-09-55 90:23:45`; expect(() => { - validateParams(actionType, { - timestamp, - }); + validateParams( + actionType, + { + timestamp, + }, + { configurationUtilities } + ); }).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`); }); test('should validate and throw error when dedupKey is missing on resolve', () => { expect(() => { - validateParams(actionType, { - eventAction: 'resolve', - }); + validateParams( + actionType, + { + eventAction: 'resolve', + }, + { configurationUtilities } + ); }).toThrowError( `error validating action params: DedupKey is required when eventAction is "resolve"` ); diff --git a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts index b195f123917ec..347fdf536b7ac 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts @@ -11,7 +11,12 @@ import { schema, TypeOf } from '@kbn/config-schema'; import moment from 'moment'; import { Logger } from '@kbn/core/server'; import { postPagerduty } from './lib/post_pagerduty'; -import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { + ActionType, + ActionTypeExecutorOptions, + ActionTypeExecutorResult, + ValidatorServices, +} from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { AlertingConnectorFeatureId, @@ -153,22 +158,28 @@ export function getActionType({ SecurityConnectorFeatureId, ], validate: { - config: schema.object(configSchemaProps, { - validate: curry(validateActionTypeConfig)(configurationUtilities), - }), - secrets: SecretsSchema, - params: ParamsSchema, + config: { + validateSchema: ConfigSchema, + validate: validateActionTypeConfig, + }, + secrets: { + validateSchema: SecretsSchema, + }, + params: { + validateSchema: ParamsSchema, + }, }, executor: curry(executor)({ logger, configurationUtilities }), }; } function validateActionTypeConfig( - configurationUtilities: ActionsConfigurationUtilities, - configObject: ActionTypeConfigType + configObject: ActionTypeConfigType, + validatorServices?: ValidatorServices ) { + const { configurationUtilities } = validatorServices || {}; try { - configurationUtilities.ensureUriAllowed(getPagerDutyApiUrl(configObject)); + configurationUtilities?.ensureUriAllowed(getPagerDutyApiUrl(configObject)); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.pagerduty.pagerdutyConfigurationError', { defaultMessage: 'error configuring pagerduty action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts index c35bf558e5b8a..087687870337d 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts @@ -6,13 +6,13 @@ */ import { curry } from 'lodash'; -import { schema, TypeOf } from '@kbn/config-schema'; +import { TypeOf } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; import { validate } from './validators'; import { - ExternalIncidentServiceConfiguration, - ExternalIncidentServiceSecretConfiguration, + ExternalIncidentServiceConfigurationSchema, + ExternalIncidentServiceSecretConfigurationSchema, ExecutorParamsSchema, } from './schema'; import { ActionsConfigurationUtilities } from '../../actions_config'; @@ -66,13 +66,17 @@ export function getActionType( SecurityConnectorFeatureId, ], validate: { - config: schema.object(ExternalIncidentServiceConfiguration, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(ExternalIncidentServiceSecretConfiguration, { - validate: curry(validate.secrets)(configurationUtilities), - }), - params: ExecutorParamsSchema, + config: { + validateSchema: ExternalIncidentServiceConfigurationSchema, + validate: validate.config, + }, + secrets: { + validateSchema: ExternalIncidentServiceSecretConfigurationSchema, + validate: validate.secrets, + }, + params: { + validateSchema: ExecutorParamsSchema, + }, }, executor: curry(executor)({ logger, configurationUtilities }), }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts index da37e6c072ac5..f19bf4f87563f 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts @@ -21,7 +21,7 @@ import { ExternalIncidentServiceSecretConfigurationSchema, } from './schema'; -import { ActionsConfigurationUtilities } from '../../actions_config'; +import { ValidatorServices } from '../../types'; export type ResilientPublicConfigurationType = TypeOf< typeof ExternalIncidentServiceConfigurationSchema @@ -43,8 +43,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configurationUtilities: ActionsConfigurationUtilities, configObject: any) => void; - secrets: (configurationUtilities: ActionsConfigurationUtilities, secrets: any) => void; + config: (configObject: any, validatorServices?: ValidatorServices) => void; + secrets: (secrets: any, validatorServices?: ValidatorServices) => void; } export interface ExternalServiceIncidentResponse { diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts index 3e37e95c4bb69..a33b6c5742416 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { ActionsConfigurationUtilities } from '../../actions_config'; import { ResilientPublicConfigurationType, ResilientSecretConfigurationType, @@ -13,21 +12,23 @@ import { } from './types'; import * as i18n from './translations'; +import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( - configurationUtilities: ActionsConfigurationUtilities, - configObject: ResilientPublicConfigurationType + configObject: ResilientPublicConfigurationType, + validatorServices?: ValidatorServices ) => { + const { configurationUtilities } = validatorServices || {}; try { - configurationUtilities.ensureUriAllowed(configObject.apiUrl); + configurationUtilities?.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } }; export const validateCommonSecrets = ( - configurationUtilities: ActionsConfigurationUtilities, - secrets: ResilientSecretConfigurationType + secrets: ResilientSecretConfigurationType, + validatorServices?: ValidatorServices ) => {}; export const validate: ExternalServiceValidation = { diff --git a/x-pack/plugins/actions/server/builtin_action_types/server_log.test.ts b/x-pack/plugins/actions/server/builtin_action_types/server_log.test.ts index 776d412d84143..89db7e3d10b84 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/server_log.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/server_log.test.ts @@ -14,16 +14,19 @@ import { ServerLogActionType, ServerLogActionTypeExecutorOptions, } from './server_log'; +import { ActionsConfigurationUtilities } from '../actions_config'; const ACTION_TYPE_ID = '.server-log'; let actionType: ServerLogActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: ActionsConfigurationUtilities; beforeAll(() => { const { logger, actionTypeRegistry } = createActionTypeRegistry(); actionType = actionTypeRegistry.get<{}, {}, ActionParamsType>(ACTION_TYPE_ID); mockedLogger = logger; + configurationUtilities = actionTypeRegistry.getUtils(); expect(actionType).toBeTruthy(); }); @@ -36,15 +39,25 @@ describe('get()', () => { describe('validateParams()', () => { test('should validate and pass when params is valid', () => { - expect(validateParams(actionType, { message: 'a message', level: 'info' })).toEqual({ + expect( + validateParams( + actionType, + { message: 'a message', level: 'info' }, + { configurationUtilities } + ) + ).toEqual({ message: 'a message', level: 'info', }); expect( - validateParams(actionType, { - message: 'a message', - level: 'info', - }) + validateParams( + actionType, + { + message: 'a message', + level: 'info', + }, + { configurationUtilities } + ) ).toEqual({ message: 'a message', level: 'info', @@ -53,19 +66,19 @@ describe('validateParams()', () => { test('should validate and throw error when params is invalid', () => { expect(() => { - validateParams(actionType, {}); + validateParams(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [message]: expected value of type [string] but got [undefined]"` ); expect(() => { - validateParams(actionType, { message: 1 }); + validateParams(actionType, { message: 1 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [message]: expected value of type [string] but got [number]"` ); expect(() => { - validateParams(actionType, { message: 'x', level: 2 }); + validateParams(actionType, { message: 'x', level: 2 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot(` "error validating action params: [level]: types that failed validation: - [level.0]: expected value to equal [trace] @@ -77,7 +90,7 @@ describe('validateParams()', () => { `); expect(() => { - validateParams(actionType, { message: 'x', level: 'foo' }); + validateParams(actionType, { message: 'x', level: 'foo' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot(` "error validating action params: [level]: types that failed validation: - [level.0]: expected value to equal [trace] diff --git a/x-pack/plugins/actions/server/builtin_action_types/server_log.ts b/x-pack/plugins/actions/server/builtin_action_types/server_log.ts index b75fe17276e2c..aaee8824682e3 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/server_log.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/server_log.ts @@ -51,7 +51,9 @@ export function getActionType({ logger }: { logger: Logger }): ServerLogActionTy }), supportedFeatureIds: [AlertingConnectorFeatureId, UptimeConnectorFeatureId], validate: { - params: ParamsSchema, + params: { + validateSchema: ParamsSchema, + }, }, executor: curry(executor)({ logger }), }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts index 59e8d5594a1e3..f82967679970c 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts @@ -6,14 +6,14 @@ */ import { curry } from 'lodash'; -import { schema, TypeOf } from '@kbn/config-schema'; +import { TypeOf } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; import { validate } from './validators'; import { - ExternalIncidentServiceConfiguration, - ExternalIncidentServiceConfigurationBase, - ExternalIncidentServiceSecretConfiguration, + ExternalIncidentServiceConfigurationSchema, + ExternalIncidentServiceConfigurationBaseSchema, + ExternalIncidentServiceSecretConfigurationSchema, ExecutorParamsSchemaITSM, ExecutorParamsSchemaSIR, ExecutorParamsSchemaITOM, @@ -105,14 +105,18 @@ export function getServiceNowITSMActionType( SecurityConnectorFeatureId, ], validate: { - config: schema.object(ExternalIncidentServiceConfiguration, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(ExternalIncidentServiceSecretConfiguration, { - validate: curry(validate.secrets)(configurationUtilities), - }), + config: { + validateSchema: ExternalIncidentServiceConfigurationSchema, + validate: validate.config, + }, + secrets: { + validateSchema: ExternalIncidentServiceSecretConfigurationSchema, + validate: validate.secrets, + }, connector: validate.connector, - params: ExecutorParamsSchemaITSM, + params: { + validateSchema: ExecutorParamsSchemaITSM, + }, }, executor: curry(executor)({ logger, @@ -138,14 +142,18 @@ export function getServiceNowSIRActionType( SecurityConnectorFeatureId, ], validate: { - config: schema.object(ExternalIncidentServiceConfiguration, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(ExternalIncidentServiceSecretConfiguration, { - validate: curry(validate.secrets)(configurationUtilities), - }), + config: { + validateSchema: ExternalIncidentServiceConfigurationSchema, + validate: validate.config, + }, + secrets: { + validateSchema: ExternalIncidentServiceSecretConfigurationSchema, + validate: validate.secrets, + }, connector: validate.connector, - params: ExecutorParamsSchemaSIR, + params: { + validateSchema: ExecutorParamsSchemaSIR, + }, }, executor: curry(executor)({ logger, @@ -167,14 +175,18 @@ export function getServiceNowITOMActionType( name: i18n.SERVICENOW_ITOM, supportedFeatureIds: [AlertingConnectorFeatureId, SecurityConnectorFeatureId], validate: { - config: schema.object(ExternalIncidentServiceConfigurationBase, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(ExternalIncidentServiceSecretConfiguration, { - validate: curry(validate.secrets)(configurationUtilities), - }), + config: { + validateSchema: ExternalIncidentServiceConfigurationBaseSchema, + validate: validate.config, + }, + secrets: { + validateSchema: ExternalIncidentServiceSecretConfigurationSchema, + validate: validate.secrets, + }, connector: validate.connector, - params: ExecutorParamsSchemaITOM, + params: { + validateSchema: ExecutorParamsSchemaITOM, + }, }, executor: curry(executorITOM)({ logger, diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts index 63cb0195a14f4..6fe6578641bba 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts @@ -27,6 +27,7 @@ import { } from './schema'; import { ActionsConfigurationUtilities } from '../../actions_config'; import { SNProductsConfigValue } from '../../../common'; +import { ValidatorServices } from '../../types'; export type { SNProductsConfigValue, SNProductsConfig } from '../../../common'; @@ -76,8 +77,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configurationUtilities: ActionsConfigurationUtilities, configObject: any) => void; - secrets: (configurationUtilities: ActionsConfigurationUtilities, secrets: any) => void; + config: (configObject: any, validatorServices?: ValidatorServices) => void; + secrets: (secrets: any, validatorServices?: ValidatorServices) => void; connector: (config: any, secrets: any) => string | null; } diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts index 547c025fcdb61..ba8ac5befdfd7 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts @@ -14,12 +14,6 @@ describe('validateCommonConfig', () => { test('config validation fails when apiUrl is not allowed', () => { expect( validateCommonConfig( - { - ...configurationUtilities, - ensureUriAllowed: (_) => { - throw new Error(`target url is not present in allowedHosts`); - }, - }, { apiUrl: 'example.com/do-something', usesTableApi: true, @@ -27,6 +21,14 @@ describe('validateCommonConfig', () => { userIdentifierValue: null, clientId: null, jwtKeyId: null, + }, + { + configurationUtilities: { + ...configurationUtilities, + ensureUriAllowed: (_) => { + throw new Error(`target url is not present in allowedHosts`); + }, + }, } ) ).toEqual(`error configuring connector action: target url is not present in allowedHosts`); @@ -34,38 +36,47 @@ describe('validateCommonConfig', () => { describe('when isOAuth = true', () => { test('config validation fails when userIdentifierValue is null', () => { expect( - validateCommonConfig(configurationUtilities, { - apiUrl: 'https://url', - usesTableApi: true, - isOAuth: true, - userIdentifierValue: null, - clientId: 'clientId', - jwtKeyId: 'jwtKeyId', - }) + validateCommonConfig( + { + apiUrl: 'https://url', + usesTableApi: true, + isOAuth: true, + userIdentifierValue: null, + clientId: 'clientId', + jwtKeyId: 'jwtKeyId', + }, + { configurationUtilities } + ) ).toEqual(`userIdentifierValue must be provided when isOAuth = true`); }); test('config validation fails when clientId is null', () => { expect( - validateCommonConfig(configurationUtilities, { - apiUrl: 'https://url', - usesTableApi: true, - isOAuth: true, - userIdentifierValue: 'userIdentifierValue', - clientId: null, - jwtKeyId: 'jwtKeyId', - }) + validateCommonConfig( + { + apiUrl: 'https://url', + usesTableApi: true, + isOAuth: true, + userIdentifierValue: 'userIdentifierValue', + clientId: null, + jwtKeyId: 'jwtKeyId', + }, + { configurationUtilities } + ) ).toEqual(`clientId must be provided when isOAuth = true`); }); test('config validation fails when jwtKeyId is null', () => { expect( - validateCommonConfig(configurationUtilities, { - apiUrl: 'https://url', - usesTableApi: true, - isOAuth: true, - userIdentifierValue: 'userIdentifierValue', - clientId: 'clientId', - jwtKeyId: null, - }) + validateCommonConfig( + { + apiUrl: 'https://url', + usesTableApi: true, + isOAuth: true, + userIdentifierValue: 'userIdentifierValue', + clientId: 'clientId', + jwtKeyId: null, + }, + { configurationUtilities } + ) ).toEqual(`jwtKeyId must be provided when isOAuth = true`); }); }); @@ -142,61 +153,76 @@ describe('validateCommonConfig', () => { describe('validateCommonSecrets', () => { test('secrets validation fails when no credentials are defined', () => { expect( - validateCommonSecrets(configurationUtilities, { - password: null, - username: null, - clientSecret: null, - privateKey: null, - privateKeyPassword: null, - }) + validateCommonSecrets( + { + password: null, + username: null, + clientSecret: null, + privateKey: null, + privateKeyPassword: null, + }, + { configurationUtilities } + ) ).toEqual(`Either basic auth or OAuth credentials must be specified`); }); test('secrets validation fails when username is defined and password is not', () => { expect( - validateCommonSecrets(configurationUtilities, { - password: null, - username: 'admin', - clientSecret: null, - privateKey: null, - privateKeyPassword: null, - }) + validateCommonSecrets( + { + password: null, + username: 'admin', + clientSecret: null, + privateKey: null, + privateKeyPassword: null, + }, + { configurationUtilities } + ) ).toEqual(`username and password must both be specified`); }); test('secrets validation fails when password is defined and username is not', () => { expect( - validateCommonSecrets(configurationUtilities, { - password: 'password', - username: null, - clientSecret: null, - privateKey: null, - privateKeyPassword: null, - }) + validateCommonSecrets( + { + password: 'password', + username: null, + clientSecret: null, + privateKey: null, + privateKeyPassword: null, + }, + { configurationUtilities } + ) ).toEqual(`username and password must both be specified`); }); test('secrets validation fails when clientSecret is defined and privateKey is not', () => { expect( - validateCommonSecrets(configurationUtilities, { - password: null, - username: null, - clientSecret: 'secret', - privateKey: null, - privateKeyPassword: null, - }) + validateCommonSecrets( + { + password: null, + username: null, + clientSecret: 'secret', + privateKey: null, + privateKeyPassword: null, + }, + { configurationUtilities } + ) ).toEqual(`clientSecret and privateKey must both be specified`); }); test('secrets validation fails when privateKey is defined and clientSecret is not', () => { expect( - validateCommonSecrets(configurationUtilities, { - password: null, - username: null, - clientSecret: null, - privateKey: 'private', - privateKeyPassword: null, - }) + validateCommonSecrets( + { + password: null, + username: null, + clientSecret: null, + privateKey: 'private', + privateKeyPassword: null, + }, + { configurationUtilities } + ) ).toEqual(`clientSecret and privateKey must both be specified`); }); }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts index 87ea4922fa5cc..6758a62e1360d 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { ActionsConfigurationUtilities } from '../../actions_config'; import { ServiceNowPublicConfigurationType, ServiceNowSecretConfigurationType, @@ -13,15 +12,16 @@ import { } from './types'; import * as i18n from './translations'; +import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( - configurationUtilities: ActionsConfigurationUtilities, - config: ServiceNowPublicConfigurationType + config: ServiceNowPublicConfigurationType, + validatorServices?: ValidatorServices ) => { const { isOAuth, apiUrl, userIdentifierValue, clientId, jwtKeyId } = config; - + const { configurationUtilities } = validatorServices || {}; try { - configurationUtilities.ensureUriAllowed(apiUrl); + configurationUtilities?.ensureUriAllowed(apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } @@ -42,8 +42,8 @@ export const validateCommonConfig = ( }; export const validateCommonSecrets = ( - configurationUtilities: ActionsConfigurationUtilities, - secrets: ServiceNowSecretConfigurationType + secrets: ServiceNowSecretConfigurationType, + validatorServices?: ValidatorServices ) => { const { username, password, clientSecret, privateKey } = secrets; diff --git a/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts b/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts index bcff352180656..f400e00db52d6 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts @@ -12,6 +12,7 @@ import { getActionType, SlackActionType, SlackActionTypeExecutorOptions } from ' import { actionsConfigMock } from '../actions_config.mock'; import { actionsMock } from '../mocks'; import { createActionTypeRegistry } from './index.test'; +import { ActionsConfigurationUtilities } from '../actions_config'; jest.mock('@slack/webhook', () => { return { @@ -27,14 +28,16 @@ const services: Services = actionsMock.createServices(); let actionType: SlackActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: jest.Mocked; beforeAll(() => { const { logger } = createActionTypeRegistry(); + configurationUtilities = actionsConfigMock.create(); actionType = getActionType({ async executor(options) { return { status: 'ok', actionId: options.actionId }; }, - configurationUtilities: actionsConfigMock.create(), + configurationUtilities, logger, }); mockedLogger = logger; @@ -50,20 +53,22 @@ describe('action registeration', () => { describe('validateParams()', () => { test('should validate and pass when params is valid', () => { - expect(validateParams(actionType, { message: 'a message' })).toEqual({ + expect( + validateParams(actionType, { message: 'a message' }, { configurationUtilities }) + ).toEqual({ message: 'a message', }); }); test('should validate and throw error when params is invalid', () => { expect(() => { - validateParams(actionType, {}); + validateParams(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [message]: expected value of type [string] but got [undefined]"` ); expect(() => { - validateParams(actionType, { message: 1 }); + validateParams(actionType, { message: 1 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [message]: expected value of type [string] but got [number]"` ); @@ -72,60 +77,76 @@ describe('validateParams()', () => { describe('validateActionTypeSecrets()', () => { test('should validate and pass when config is valid', () => { - validateSecrets(actionType, { - webhookUrl: 'https://example.com', - }); + validateSecrets( + actionType, + { + webhookUrl: 'https://example.com', + }, + { configurationUtilities } + ); }); test('should validate and throw error when config is invalid', () => { expect(() => { - validateSecrets(actionType, {}); + validateSecrets(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [webhookUrl]: expected value of type [string] but got [undefined]"` ); expect(() => { - validateSecrets(actionType, { webhookUrl: 1 }); + validateSecrets(actionType, { webhookUrl: 1 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [webhookUrl]: expected value of type [string] but got [number]"` ); expect(() => { - validateSecrets(actionType, { webhookUrl: 'fee-fi-fo-fum' }); + validateSecrets(actionType, { webhookUrl: 'fee-fi-fo-fum' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: error configuring slack action: unable to parse host name from webhookUrl"` ); }); test('should validate and pass when the slack webhookUrl is added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (url: string) => { + expect(url).toEqual('https://api.slack.com/'); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (url) => { - expect(url).toEqual('https://api.slack.com/'); - }, - }, + configurationUtilities: configUtils, }); - expect(validateSecrets(actionType, { webhookUrl: 'https://api.slack.com/' })).toEqual({ + expect( + validateSecrets( + actionType, + { webhookUrl: 'https://api.slack.com/' }, + { configurationUtilities: configUtils } + ) + ).toEqual({ webhookUrl: 'https://api.slack.com/', }); }); test('config validation returns an error if the specified URL isnt added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: () => { + throw new Error(`target hostname is not added to allowedHosts`); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: () => { - throw new Error(`target hostname is not added to allowedHosts`); - }, - }, + configurationUtilities: configUtils, }); expect(() => { - validateSecrets(actionType, { webhookUrl: 'https://api.slack.com/' }); + validateSecrets( + actionType, + { webhookUrl: 'https://api.slack.com/' }, + { configurationUtilities: configUtils } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: error configuring slack action: target hostname is not added to allowedHosts"` ); @@ -191,8 +212,8 @@ describe('execute()', () => { }); test('calls the mock executor with success proxy', async () => { - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.getProxySettings.mockReturnValue({ + const configUtils = actionsConfigMock.create(); + configUtils.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxySSLSettings: { verificationMode: 'none', @@ -202,7 +223,7 @@ describe('execute()', () => { }); const actionTypeProxy = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); await actionTypeProxy.executor({ actionId: 'some-id', @@ -218,8 +239,8 @@ describe('execute()', () => { test('ensure proxy bypass will bypass when expected', async () => { mockedLogger.debug.mockReset(); - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.getProxySettings.mockReturnValue({ + const configUtils = actionsConfigMock.create(); + configUtils.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxySSLSettings: { verificationMode: 'none', @@ -229,7 +250,7 @@ describe('execute()', () => { }); const actionTypeProxy = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); await actionTypeProxy.executor({ actionId: 'some-id', @@ -245,8 +266,8 @@ describe('execute()', () => { test('ensure proxy bypass will not bypass when expected', async () => { mockedLogger.debug.mockReset(); - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.getProxySettings.mockReturnValue({ + const configUtils = actionsConfigMock.create(); + configUtils.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxySSLSettings: { verificationMode: 'none', @@ -256,7 +277,7 @@ describe('execute()', () => { }); const actionTypeProxy = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); await actionTypeProxy.executor({ actionId: 'some-id', @@ -272,8 +293,8 @@ describe('execute()', () => { test('ensure proxy only will proxy when expected', async () => { mockedLogger.debug.mockReset(); - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.getProxySettings.mockReturnValue({ + const configUtils = actionsConfigMock.create(); + configUtils.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxySSLSettings: { verificationMode: 'none', @@ -283,7 +304,7 @@ describe('execute()', () => { }); const actionTypeProxy = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); await actionTypeProxy.executor({ actionId: 'some-id', @@ -299,8 +320,8 @@ describe('execute()', () => { test('ensure proxy only will not proxy when expected', async () => { mockedLogger.debug.mockReset(); - const configurationUtilities = actionsConfigMock.create(); - configurationUtilities.getProxySettings.mockReturnValue({ + const configUtils = actionsConfigMock.create(); + configUtils.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxySSLSettings: { verificationMode: 'none', @@ -310,7 +331,7 @@ describe('execute()', () => { }); const actionTypeProxy = getActionType({ logger: mockedLogger, - configurationUtilities, + configurationUtilities: configUtils, }); await actionTypeProxy.executor({ actionId: 'some-id', diff --git a/x-pack/plugins/actions/server/builtin_action_types/slack.ts b/x-pack/plugins/actions/server/builtin_action_types/slack.ts index 59d1d6a789b17..ae5bb589adc32 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.ts @@ -23,6 +23,7 @@ import { ActionTypeExecutorOptions, ActionTypeExecutorResult, ExecutorType, + ValidatorServices, } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { getCustomAgents } from './lib/get_custom_agents'; @@ -81,10 +82,13 @@ export function getActionType({ SecurityConnectorFeatureId, ], validate: { - secrets: schema.object(secretsSchemaProps, { - validate: curry(validateActionTypeConfig)(configurationUtilities), - }), - params: ParamsSchema, + secrets: { + validateSchema: SecretsSchema, + validate: validateActionTypeConfig, + }, + params: { + validateSchema: ParamsSchema, + }, }, renderParameterTemplates, executor, @@ -101,9 +105,10 @@ function renderParameterTemplates( } function validateActionTypeConfig( - configurationUtilities: ActionsConfigurationUtilities, - secretsObject: ActionTypeSecretsType + secretsObject: ActionTypeSecretsType, + validatorServices?: ValidatorServices ) { + const { configurationUtilities } = validatorServices || {}; const configuredUrl = secretsObject.webhookUrl; try { new URL(configuredUrl); @@ -114,7 +119,7 @@ function validateActionTypeConfig( } try { - configurationUtilities.ensureUriAllowed(configuredUrl); + configurationUtilities?.ensureUriAllowed(configuredUrl); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.slack.slackConfigurationError', { defaultMessage: 'error configuring slack action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts index 9be469d978f2f..dd5a61a76de0a 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts @@ -7,7 +7,6 @@ import { curry } from 'lodash'; import { i18n } from '@kbn/i18n'; -import { schema } from '@kbn/config-schema'; import { Logger } from '@kbn/logging'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../../types'; import { ActionsConfigurationUtilities } from '../../actions_config'; @@ -21,8 +20,8 @@ import { import { validate } from './validators'; import { ExecutorParamsSchema, - SwimlaneSecretsConfiguration, - SwimlaneServiceConfiguration, + SwimlaneSecretsConfigurationSchema, + SwimlaneServiceConfigurationSchema, } from './schema'; import { createExternalService } from './service'; import { api } from './api'; @@ -61,13 +60,17 @@ export function getActionType( SecurityConnectorFeatureId, ], validate: { - config: schema.object(SwimlaneServiceConfiguration, { - validate: curry(validate.config)(configurationUtilities), - }), - secrets: schema.object(SwimlaneSecretsConfiguration, { - validate: curry(validate.secrets)(configurationUtilities), - }), - params: ExecutorParamsSchema, + config: { + validateSchema: SwimlaneServiceConfigurationSchema, + validate: validate.config, + }, + secrets: { + validateSchema: SwimlaneSecretsConfigurationSchema, + validate: validate.secrets, + }, + params: { + validateSchema: ExecutorParamsSchema, + }, }, executor: curry(executor)({ logger, configurationUtilities }), }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts index 5cb3b10989621..d832ff211c512 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts @@ -15,7 +15,7 @@ import { SwimlaneSecretsConfigurationSchema, SwimlaneServiceConfigurationSchema, } from './schema'; -import { ActionsConfigurationUtilities } from '../../actions_config'; +import { ValidatorServices } from '../../types'; export type SwimlanePublicConfigurationType = TypeOf; export type SwimlaneSecretConfigurationType = TypeOf; @@ -30,8 +30,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configurationUtilities: ActionsConfigurationUtilities, configObject: any) => void; - secrets: (configurationUtilities: ActionsConfigurationUtilities, secrets: any) => void; + config: (configObject: any, validatorServices?: ValidatorServices) => void; + secrets: (secrets: any, validatorServices?: ValidatorServices) => void; } export interface CreateRecordParams { diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts index 1972cd7e6af0b..5d77c765b6efd 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts @@ -5,22 +5,30 @@ * 2.0. */ -import { ActionsConfigurationUtilities } from '../../actions_config'; -import { ExternalServiceValidation, SwimlanePublicConfigurationType } from './types'; +import { + ExternalServiceValidation, + SwimlanePublicConfigurationType, + SwimlaneSecretConfigurationType, +} from './types'; import * as i18n from './translations'; +import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( - configurationUtilities: ActionsConfigurationUtilities, - configObject: SwimlanePublicConfigurationType + configObject: SwimlanePublicConfigurationType, + validatorServices?: ValidatorServices ) => { + const { configurationUtilities } = validatorServices || {}; try { - configurationUtilities.ensureUriAllowed(configObject.apiUrl); + configurationUtilities?.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } }; -export const validateCommonSecrets = () => {}; +export const validateCommonSecrets = ( + secrets: SwimlaneSecretConfigurationType, + validatorServices?: ValidatorServices +) => {}; export const validate: ExternalServiceValidation = { config: validateCommonConfig, diff --git a/x-pack/plugins/actions/server/builtin_action_types/teams.test.ts b/x-pack/plugins/actions/server/builtin_action_types/teams.test.ts index 23bc0fba603d4..a64e71e54a4d3 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/teams.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/teams.test.ts @@ -14,6 +14,7 @@ import { actionsConfigMock } from '../actions_config.mock'; import { actionsMock } from '../mocks'; import { createActionTypeRegistry } from './index.test'; import * as utils from './lib/axios_utils'; +import { ActionsConfigurationUtilities } from '../actions_config'; jest.mock('axios'); jest.mock('./lib/axios_utils', () => { @@ -34,11 +35,13 @@ const services: Services = actionsMock.createServices(); let actionType: TeamsActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: ActionsConfigurationUtilities; beforeAll(() => { const { logger, actionTypeRegistry } = createActionTypeRegistry(); actionType = actionTypeRegistry.get<{}, ActionTypeSecretsType, ActionParamsType>(ACTION_TYPE_ID); mockedLogger = logger; + configurationUtilities = actionTypeRegistry.getUtils(); }); describe('action registration', () => { @@ -50,20 +53,22 @@ describe('action registration', () => { describe('validateParams()', () => { test('should validate and pass when params is valid', () => { - expect(validateParams(actionType, { message: 'a message' })).toEqual({ + expect( + validateParams(actionType, { message: 'a message' }, { configurationUtilities }) + ).toEqual({ message: 'a message', }); }); test('should validate and throw error when params is invalid', () => { expect(() => { - validateParams(actionType, {}); + validateParams(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [message]: expected value of type [string] but got [undefined]"` ); expect(() => { - validateParams(actionType, { message: 1 }); + validateParams(actionType, { message: 1 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [message]: expected value of type [string] but got [number]"` ); @@ -72,60 +77,76 @@ describe('validateParams()', () => { describe('validateActionTypeSecrets()', () => { test('should validate and pass when config is valid', () => { - validateSecrets(actionType, { - webhookUrl: 'https://example.com', - }); + validateSecrets( + actionType, + { + webhookUrl: 'https://example.com', + }, + { configurationUtilities } + ); }); test('should validate and throw error when config is invalid', () => { expect(() => { - validateSecrets(actionType, {}); + validateSecrets(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [webhookUrl]: expected value of type [string] but got [undefined]"` ); expect(() => { - validateSecrets(actionType, { webhookUrl: 1 }); + validateSecrets(actionType, { webhookUrl: 1 }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [webhookUrl]: expected value of type [string] but got [number]"` ); expect(() => { - validateSecrets(actionType, { webhookUrl: 'fee-fi-fo-fum' }); + validateSecrets(actionType, { webhookUrl: 'fee-fi-fo-fum' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: error configuring teams action: unable to parse host name from webhookUrl"` ); }); test('should validate and pass when the teams webhookUrl is added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (url: string) => { + expect(url).toEqual('https://outlook.office.com/'); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (url) => { - expect(url).toEqual('https://outlook.office.com/'); - }, - }, + configurationUtilities: configUtils, }); - expect(validateSecrets(actionType, { webhookUrl: 'https://outlook.office.com/' })).toEqual({ + expect( + validateSecrets( + actionType, + { webhookUrl: 'https://outlook.office.com/' }, + { configurationUtilities: configUtils } + ) + ).toEqual({ webhookUrl: 'https://outlook.office.com/', }); }); test('config validation returns an error if the specified URL isnt added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: () => { + throw new Error(`target hostname is not added to allowedHosts`); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: () => { - throw new Error(`target hostname is not added to allowedHosts`); - }, - }, + configurationUtilities: configUtils, }); expect(() => { - validateSecrets(actionType, { webhookUrl: 'https://outlook.office.com/' }); + validateSecrets( + actionType, + { webhookUrl: 'https://outlook.office.com/' }, + { configurationUtilities: configUtils } + ); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: error configuring teams action: target hostname is not added to allowedHosts"` ); diff --git a/x-pack/plugins/actions/server/builtin_action_types/teams.ts b/x-pack/plugins/actions/server/builtin_action_types/teams.ts index 937e5b84a05f3..503e043d8eef8 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/teams.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/teams.ts @@ -16,7 +16,12 @@ import { Logger } from '@kbn/core/server'; import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_header'; import { isOk, promiseResult, Result } from './lib/result_type'; import { request } from './lib/axios_utils'; -import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { + ActionType, + ActionTypeExecutorOptions, + ActionTypeExecutorResult, + ValidatorServices, +} from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { AlertingConnectorFeatureId, @@ -69,19 +74,23 @@ export function getActionType({ SecurityConnectorFeatureId, ], validate: { - secrets: schema.object(secretsSchemaProps, { - validate: curry(validateActionTypeConfig)(configurationUtilities), - }), - params: ParamsSchema, + secrets: { + validateSchema: SecretsSchema, + validate: validateActionTypeConfig, + }, + params: { + validateSchema: ParamsSchema, + }, }, executor: curry(teamsExecutor)({ logger, configurationUtilities }), }; } function validateActionTypeConfig( - configurationUtilities: ActionsConfigurationUtilities, - secretsObject: ActionTypeSecretsType + secretsObject: ActionTypeSecretsType, + validatorServices?: ValidatorServices ) { + const { configurationUtilities } = validatorServices || {}; const configuredUrl = secretsObject.webhookUrl; try { new URL(configuredUrl); @@ -92,7 +101,7 @@ function validateActionTypeConfig( } try { - configurationUtilities.ensureUriAllowed(configuredUrl); + configurationUtilities?.ensureUriAllowed(configuredUrl); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.teams.teamsConfigurationError', { defaultMessage: 'error configuring teams action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts index 6b8a1a4e8447b..9e762f1442c48 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -22,6 +22,7 @@ import { } from './webhook'; import * as utils from './lib/axios_utils'; +import { ActionsConfigurationUtilities } from '../actions_config'; jest.mock('axios'); jest.mock('./lib/axios_utils', () => { @@ -44,6 +45,7 @@ const services: Services = actionsMock.createServices(); let actionType: WebhookActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: ActionsConfigurationUtilities; beforeAll(() => { const { logger, actionTypeRegistry } = createActionTypeRegistry(); @@ -53,6 +55,7 @@ beforeAll(() => { ActionParamsType >(ACTION_TYPE_ID); mockedLogger = logger; + configurationUtilities = actionTypeRegistry.getUtils(); }); describe('actionType', () => { @@ -68,19 +71,22 @@ describe('secrets validation', () => { user: 'bob', password: 'supersecret', }; - expect(validateSecrets(actionType, secrets)).toEqual(secrets); + expect(validateSecrets(actionType, secrets, { configurationUtilities })).toEqual(secrets); }); test('fails when secret user is provided, but password is omitted', () => { expect(() => { - validateSecrets(actionType, { user: 'bob' }); + validateSecrets(actionType, { user: 'bob' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: both user and password must be specified"` ); }); test('succeeds when basic authentication credentials are omitted', () => { - expect(validateSecrets(actionType, {})).toEqual({ password: null, user: null }); + expect(validateSecrets(actionType, {}, { configurationUtilities })).toEqual({ + password: null, + user: null, + }); }); }); @@ -95,7 +101,7 @@ describe('config validation', () => { url: 'http://mylisteningserver:9200/endpoint', hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...defaultValues, ...config, }); @@ -108,7 +114,7 @@ describe('config validation', () => { method, hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...defaultValues, ...config, }); @@ -121,7 +127,7 @@ describe('config validation', () => { method: 'https', }; expect(() => { - validateConfig(actionType, config); + validateConfig(actionType, config, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot(` "error validating action type config: [method]: types that failed validation: - [method.0]: expected value to equal [post] @@ -134,7 +140,7 @@ describe('config validation', () => { url: 'http://mylisteningserver:9200/endpoint', hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...defaultValues, ...config, }); @@ -145,7 +151,7 @@ describe('config validation', () => { url: 'example.com/do-something', }; expect(() => { - validateConfig(actionType, config); + validateConfig(actionType, config, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( '"error validating action type config: error configuring webhook action: unable to parse url: TypeError: Invalid URL: example.com/do-something"' ); @@ -161,7 +167,7 @@ describe('config validation', () => { }, hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...defaultValues, ...config, }); @@ -173,7 +179,7 @@ describe('config validation', () => { headers: 'application/json', }; expect(() => { - validateConfig(actionType, config); + validateConfig(actionType, config, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot(` "error validating action type config: [headers]: types that failed validation: - [headers.0]: could not parse record value from json input @@ -192,21 +198,22 @@ describe('config validation', () => { hasAuth: true, }; - expect(validateConfig(actionType, config)).toEqual({ + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual({ ...defaultValues, ...config, }); }); test('config validation returns an error if the specified URL isnt added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (_: string) => { + throw new Error(`target url is not present in allowedHosts`); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (_) => { - throw new Error(`target url is not present in allowedHosts`); - }, - }, + configurationUtilities: configUtils, }); // any for testing @@ -219,7 +226,7 @@ describe('config validation', () => { }; expect(() => { - validateConfig(actionType, config); + validateConfig(actionType, config, { configurationUtilities: configUtils }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: error configuring webhook action: target url is not present in allowedHosts"` ); @@ -229,14 +236,14 @@ describe('config validation', () => { describe('params validation', () => { test('param validation passes when no fields are provided as none are required', () => { const params: Record = {}; - expect(validateParams(actionType, params)).toEqual({}); + expect(validateParams(actionType, params, { configurationUtilities })).toEqual({}); }); test('params validation passes when a valid body is provided', () => { const params: Record = { body: 'count: {{ctx.payload.hits.total}}', }; - expect(validateParams(actionType, params)).toEqual({ + expect(validateParams(actionType, params, { configurationUtilities })).toEqual({ ...params, }); }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts index 377b0858a158c..890803836e196 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts @@ -15,7 +15,12 @@ import { Logger } from '@kbn/core/server'; import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_header'; import { nullableType } from './lib/nullable'; import { isOk, promiseResult, Result } from './lib/result_type'; -import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { + ActionType, + ActionTypeExecutorOptions, + ActionTypeExecutorResult, + ValidatorServices, +} from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { request } from './lib/axios_utils'; import { renderMustacheString } from '../lib/mustache_renderer'; @@ -99,11 +104,16 @@ export function getActionType({ SecurityConnectorFeatureId, ], validate: { - config: schema.object(configSchemaProps, { - validate: curry(validateActionTypeConfig)(configurationUtilities), - }), - secrets: SecretsSchema, - params: ParamsSchema, + config: { + validateSchema: ConfigSchema, + validate: validateActionTypeConfig, + }, + secrets: { + validateSchema: SecretsSchema, + }, + params: { + validateSchema: ParamsSchema, + }, }, renderParameterTemplates, executor: curry(executor)({ logger, configurationUtilities }), @@ -121,9 +131,10 @@ function renderParameterTemplates( } function validateActionTypeConfig( - configurationUtilities: ActionsConfigurationUtilities, - configObject: ActionTypeConfigType + configObject: ActionTypeConfigType, + validatorServices?: ValidatorServices ) { + const { configurationUtilities } = validatorServices || {}; const configuredUrl = configObject.url; try { new URL(configuredUrl); @@ -137,7 +148,7 @@ function validateActionTypeConfig( } try { - configurationUtilities.ensureUriAllowed(configuredUrl); + configurationUtilities?.ensureUriAllowed(configuredUrl); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { defaultMessage: 'error configuring webhook action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/xmatters.test.ts b/x-pack/plugins/actions/server/builtin_action_types/xmatters.test.ts index 1ea95b4bf0b44..83e43ee8b3ba2 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/xmatters.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/xmatters.test.ts @@ -23,6 +23,7 @@ import { getActionType, XmattersActionType, } from './xmatters'; +import { ActionsConfigurationUtilities } from '../actions_config'; const postxMattersMock = postXmatters as jest.Mock; @@ -32,6 +33,7 @@ const services: Services = actionsMock.createServices(); let actionType: XmattersActionType; let mockedLogger: jest.Mocked; +let configurationUtilities: jest.Mocked; beforeAll(() => { const { logger, actionTypeRegistry } = createActionTypeRegistry(); @@ -44,9 +46,10 @@ beforeAll(() => { }); beforeEach(() => { + configurationUtilities = actionsConfigMock.create(); actionType = getActionType({ logger: mockedLogger, - configurationUtilities: actionsConfigMock.create(), + configurationUtilities, }); }); @@ -63,7 +66,7 @@ describe('secrets validation', () => { user: 'bob', password: 'supersecret', }; - expect(validateSecrets(actionType, secrets)).toEqual({ + expect(validateSecrets(actionType, secrets, { configurationUtilities })).toEqual({ ...secrets, secretsUrl: null, }); @@ -73,7 +76,7 @@ describe('secrets validation', () => { const secrets: Record = { secretsUrl: 'http://mylisteningserver:9200/endpoint?apiKey=someKey', }; - expect(validateSecrets(actionType, secrets)).toEqual({ + expect(validateSecrets(actionType, secrets, { configurationUtilities })).toEqual({ ...secrets, user: null, password: null, @@ -86,7 +89,7 @@ describe('secrets validation', () => { secretsUrl: 'http://mylisteningserver:9200/endpoint?apiKey=someKey', }; expect(() => { - validateSecrets(actionType, secrets); + validateSecrets(actionType, secrets, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: Cannot use user/password for URL authentication. Provide valid secretsUrl or use Basic Authentication."` ); @@ -98,7 +101,7 @@ describe('secrets validation', () => { secretsUrl: 'http://mylisteningserver:9200/endpoint?apiKey=someKey', }; expect(() => { - validateSecrets(actionType, secrets); + validateSecrets(actionType, secrets, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: Cannot use user/password for URL authentication. Provide valid secretsUrl or use Basic Authentication."` ); @@ -111,7 +114,7 @@ describe('secrets validation', () => { secretsUrl: 'http://mylisteningserver:9200/endpoint?apiKey=someKey', }; expect(() => { - validateSecrets(actionType, secrets); + validateSecrets(actionType, secrets, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: Cannot use user/password for URL authentication. Provide valid secretsUrl or use Basic Authentication."` ); @@ -119,7 +122,7 @@ describe('secrets validation', () => { test('fails when secret user is provided, but password is omitted', () => { expect(() => { - validateSecrets(actionType, { user: 'bob' }); + validateSecrets(actionType, { user: 'bob' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: Both user and password must be specified."` ); @@ -127,7 +130,7 @@ describe('secrets validation', () => { test('fails when password is provided, but user is omitted', () => { expect(() => { - validateSecrets(actionType, { password: 'supersecret' }); + validateSecrets(actionType, { password: 'supersecret' }, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: Both user and password must be specified."` ); @@ -135,7 +138,7 @@ describe('secrets validation', () => { test('fails when user, password, and secretsUrl are omitted', () => { expect(() => { - validateSecrets(actionType, {}); + validateSecrets(actionType, {}, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: Provide either secretsUrl link or user/password to authenticate"` ); @@ -146,28 +149,29 @@ describe('secrets validation', () => { secretsUrl: 'example.com/do-something?apiKey=someKey', }; expect(() => { - validateSecrets(actionType, secrets); + validateSecrets(actionType, secrets, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( '"error validating action type secrets: Invalid secretsUrl: TypeError: Invalid URL: example.com/do-something?apiKey=someKey"' ); }); test('fails when url host is not in allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (_: string) => { + throw new Error(`target url is not present in allowedHosts`); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (_) => { - throw new Error(`target url is not present in allowedHosts`); - }, - }, + configurationUtilities: configUtils, }); const secrets: Record = { secretsUrl: 'http://mylisteningserver.com:9200/endpoint', }; expect(() => { - validateSecrets(actionType, secrets); + validateSecrets(actionType, secrets, { configurationUtilities: configUtils }); }).toThrowErrorMatchingInlineSnapshot( '"error validating action type secrets: target url is not present in allowedHosts"' ); @@ -180,7 +184,7 @@ describe('config validation', () => { configUrl: 'http://mylisteningserver:9200/endpoint', usesBasic: true, }; - expect(validateConfig(actionType, config)).toEqual(config); + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual(config); }); test('config validation failed when a url is invalid', () => { @@ -189,21 +193,22 @@ describe('config validation', () => { usesBasic: true, }; expect(() => { - validateConfig(actionType, config); + validateConfig(actionType, config, { configurationUtilities }); }).toThrowErrorMatchingInlineSnapshot( '"error validating action type config: Error configuring xMatters action: unable to parse url: TypeError: Invalid URL: example.com/do-something"' ); }); test('config validation returns an error if the specified URL isnt added to allowedHosts', () => { + const configUtils = { + ...actionsConfigMock.create(), + ensureUriAllowed: (_: string) => { + throw new Error(`target url is not present in allowedHosts`); + }, + }; actionType = getActionType({ logger: mockedLogger, - configurationUtilities: { - ...actionsConfigMock.create(), - ensureUriAllowed: (_) => { - throw new Error(`target url is not present in allowedHosts`); - }, - }, + configurationUtilities: configUtils, }); const config: Record = { @@ -212,7 +217,7 @@ describe('config validation', () => { }; expect(() => { - validateConfig(actionType, config); + validateConfig(actionType, config, { configurationUtilities: configUtils }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type config: Error configuring xMatters action: target url is not present in allowedHosts"` ); @@ -224,7 +229,7 @@ describe('config validation', () => { usesBasic: false, }; - expect(validateConfig(actionType, config)).toEqual(config); + expect(validateConfig(actionType, config, { configurationUtilities })).toEqual(config); }); }); @@ -233,7 +238,7 @@ describe('params validation', () => { const params: Record = { severity: 'high', }; - expect(validateParams(actionType, params)).toEqual({ + expect(validateParams(actionType, params, { configurationUtilities })).toEqual({ severity: 'high', }); }); @@ -248,7 +253,7 @@ describe('params validation', () => { spaceId: 'default', tags: 'test1, test2', }; - expect(validateParams(actionType, params)).toEqual({ + expect(validateParams(actionType, params, { configurationUtilities })).toEqual({ ...params, }); }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts b/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts index 2a0a186f0e319..54bd918fbe1c4 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts @@ -9,7 +9,12 @@ import { i18n } from '@kbn/i18n'; import { curry, isString } from 'lodash'; import { schema, TypeOf } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; -import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { + ActionType, + ActionTypeExecutorOptions, + ActionTypeExecutorResult, + ValidatorServices, +} from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { postXmatters } from './lib/post_xmatters'; import { AlertingConnectorFeatureId } from '../../common'; @@ -71,13 +76,17 @@ export function getActionType({ }), supportedFeatureIds: [AlertingConnectorFeatureId], validate: { - config: schema.object(configSchemaProps, { - validate: curry(validateActionTypeConfig)(configurationUtilities), - }), - secrets: schema.object(secretSchemaProps, { - validate: curry(validateActionTypeSecrets)(configurationUtilities), - }), - params: ParamsSchema, + config: { + validateSchema: ConfigSchema, + validate: validateActionTypeConfig, + }, + secrets: { + validateSchema: SecretsSchema, + validate: validateActionTypeSecrets, + }, + params: { + validateSchema: ParamsSchema, + }, connector: validateConnector, }, executor: curry(executor)({ logger, configurationUtilities }), @@ -85,9 +94,10 @@ export function getActionType({ } function validateActionTypeConfig( - configurationUtilities: ActionsConfigurationUtilities, - configObject: ActionTypeConfigType + configObject: ActionTypeConfigType, + validatorServices?: ValidatorServices ): string | undefined { + const { configurationUtilities } = validatorServices || {}; const configuredUrl = configObject.configUrl; const usesBasic = configObject.usesBasic; if (!usesBasic) return; @@ -106,7 +116,7 @@ function validateActionTypeConfig( try { if (configuredUrl) { - configurationUtilities.ensureUriAllowed(configuredUrl); + configurationUtilities?.ensureUriAllowed(configuredUrl); } } catch (allowListError) { return i18n.translate('xpack.actions.builtin.xmatters.xmattersConfigurationError', { @@ -167,9 +177,10 @@ function validateConnector( } function validateActionTypeSecrets( - configurationUtilities: ActionsConfigurationUtilities, - secretsObject: ActionTypeSecretsType + secretsObject: ActionTypeSecretsType, + validatorServices?: ValidatorServices ): string | undefined { + const { configurationUtilities } = validatorServices || {}; if (!secretsObject.secretsUrl && !secretsObject.user && !secretsObject.password) { return i18n.translate('xpack.actions.builtin.xmatters.noSecretsProvided', { defaultMessage: 'Provide either secretsUrl link or user/password to authenticate', @@ -203,7 +214,7 @@ function validateActionTypeSecrets( // Test that hostname is allowed try { if (secretsObject.secretsUrl) { - configurationUtilities.ensureUriAllowed(secretsObject.secretsUrl); + configurationUtilities?.ensureUriAllowed(secretsObject.secretsUrl); } } catch (allowListError) { return i18n.translate('xpack.actions.builtin.xmatters.xmattersHostnameNotAllowed', { From 2c8664528678b79df0c58fdcb85074058e1435ca Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Thu, 25 Aug 2022 08:54:51 -0400 Subject: [PATCH 3/7] Fixing types --- .../server/action_type_registry.mock.ts | 1 + .../actions/server/actions_client.test.ts | 16 ++- .../server/lib/action_executor.test.ts | 16 ++- .../server/lib/validate_with_schema.test.ts | 107 ++++++++++++------ .../transform_connectors_for_export.ts | 9 +- .../server/sub_action_framework/validators.ts | 32 +++--- 6 files changed, 117 insertions(+), 64 deletions(-) diff --git a/x-pack/plugins/actions/server/action_type_registry.mock.ts b/x-pack/plugins/actions/server/action_type_registry.mock.ts index c95fd7c817a81..5056e7842b3b4 100644 --- a/x-pack/plugins/actions/server/action_type_registry.mock.ts +++ b/x-pack/plugins/actions/server/action_type_registry.mock.ts @@ -16,6 +16,7 @@ const createActionTypeRegistryMock = () => { ensureActionTypeEnabled: jest.fn(), isActionTypeEnabled: jest.fn(), isActionExecutable: jest.fn(), + getUtils: jest.fn(), }; return mocked; }; diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index 44276a604896f..b807d73ee2f3b 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -376,9 +376,11 @@ describe('create()', () => { minimumLicenseRequired: 'basic', supportedFeatureIds: ['alerting'], validate: { - config: schema.object({ - param1: schema.string(), - }), + config: { + validateSchema: schema.object({ + param1: schema.string(), + }), + }, }, executor, }); @@ -1949,9 +1951,11 @@ describe('update()', () => { minimumLicenseRequired: 'basic', supportedFeatureIds: ['alerting'], validate: { - config: schema.object({ - param1: schema.string(), - }), + config: { + validateSchema: schema.object({ + param1: schema.string(), + }), + }, }, executor, }); 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 d5bf063f5daa9..df3095daea4d9 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -270,9 +270,11 @@ test('throws an error when config is invalid', async () => { minimumLicenseRequired: 'basic', supportedFeatureIds: ['alerting'], validate: { - config: schema.object({ - param1: schema.string(), - }), + config: { + validateSchema: schema.object({ + param1: schema.string(), + }), + }, }, executor: jest.fn(), }; @@ -352,9 +354,11 @@ test('throws an error when params is invalid', async () => { minimumLicenseRequired: 'basic', supportedFeatureIds: ['alerting'], validate: { - params: schema.object({ - param1: schema.string(), - }), + params: { + validateSchema: schema.object({ + param1: schema.string(), + }), + }, }, executor: jest.fn(), }; 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 703d8694fbd62..20225ed0d4da7 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 @@ -14,11 +14,14 @@ import { validateConnector, } from './validate_with_schema'; import { ActionType, ExecutorType } from '../types'; +import { actionsConfigMock } from '../actions_config.mock'; const executor: ExecutorType<{}, {}, {}, void> = async (options) => { return { status: 'ok', actionId: options.actionId }; }; +const configurationUtilities = actionsConfigMock.create(); + test('should validate when there are no validators', () => { const actionType: ActionType = { id: 'foo', @@ -29,7 +32,7 @@ test('should validate when there are no validators', () => { }; const testValue = { any: ['old', 'thing'] }; - const result = validateConfig(actionType, testValue); + const result = validateConfig(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); }); @@ -46,13 +49,13 @@ test('should validate when there are no individual validators', () => { let result; const testValue = { any: ['old', 'thing'] }; - result = validateParams(actionType, testValue); + result = validateParams(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); - result = validateConfig(actionType, testValue); + result = validateConfig(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); - result = validateSecrets(actionType, testValue); + result = validateSecrets(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); result = validateConnector(actionType, { config: testValue }); @@ -68,9 +71,15 @@ test('should validate when validators return incoming value', () => { supportedFeatureIds: ['alerting'], executor, validate: { - params: selfValidator, - config: selfValidator, - secrets: selfValidator, + params: { + validateSchema: selfValidator, + }, + config: { + validateSchema: selfValidator, + }, + secrets: { + validateSchema: selfValidator, + }, connector: () => null, }, }; @@ -78,13 +87,13 @@ test('should validate when validators return incoming value', () => { let result; const testValue = { any: ['old', 'thing'] }; - result = validateParams(actionType, testValue); + result = validateParams(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); - result = validateConfig(actionType, testValue); + result = validateConfig(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); - result = validateSecrets(actionType, testValue); + result = validateSecrets(actionType, testValue, { configurationUtilities }); expect(result).toEqual(testValue); result = validateConnector(actionType, { config: testValue, secrets: { user: 'test' } }); @@ -101,9 +110,15 @@ test('should validate when validators return different values', () => { supportedFeatureIds: ['alerting'], executor, validate: { - params: selfValidator, - config: selfValidator, - secrets: selfValidator, + params: { + validateSchema: selfValidator, + }, + config: { + validateSchema: selfValidator, + }, + secrets: { + validateSchema: selfValidator, + }, connector: () => null, }, }; @@ -111,13 +126,13 @@ test('should validate when validators return different values', () => { let result; const testValue = { any: ['old', 'thing'] }; - result = validateParams(actionType, testValue); + result = validateParams(actionType, testValue, { configurationUtilities }); expect(result).toEqual(returnedValue); - result = validateConfig(actionType, testValue); + result = validateConfig(actionType, testValue, { configurationUtilities }); expect(result).toEqual(returnedValue); - result = validateSecrets(actionType, testValue); + result = validateSecrets(actionType, testValue, { configurationUtilities }); expect(result).toEqual(returnedValue); result = validateConnector(actionType, { config: testValue, secrets: { user: 'test' } }); @@ -137,26 +152,32 @@ test('should throw with expected error when validators fail', () => { supportedFeatureIds: ['alerting'], executor, validate: { - params: erroringValidator, - config: erroringValidator, - secrets: erroringValidator, + params: { + validateSchema: erroringValidator, + }, + config: { + validateSchema: erroringValidator, + }, + secrets: { + validateSchema: erroringValidator, + }, connector: () => 'test error', }, }; const testValue = { any: ['old', 'thing'] }; - expect(() => validateParams(actionType, testValue)).toThrowErrorMatchingInlineSnapshot( - `"error validating action params: test error"` - ); + expect(() => + validateParams(actionType, testValue, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action params: test error"`); - expect(() => validateConfig(actionType, testValue)).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: test error"` - ); + expect(() => + validateConfig(actionType, testValue, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action type config: test error"`); - expect(() => validateSecrets(actionType, testValue)).toThrowErrorMatchingInlineSnapshot( - `"error validating action type secrets: test error"` - ); + expect(() => + validateSecrets(actionType, testValue, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action type secrets: test error"`); expect(() => validateConnector(actionType, { config: testValue, secrets: { user: 'test' } }) @@ -172,17 +193,25 @@ test('should work with @kbn/config-schema', () => { supportedFeatureIds: ['alerting'], executor, validate: { - params: testSchema, - config: testSchema, - secrets: testSchema, + params: { + validateSchema: testSchema, + }, + config: { + validateSchema: testSchema, + }, + secrets: { + validateSchema: testSchema, + }, connector: () => null, }, }; - const result = validateParams(actionType, { foo: 'bar' }); + const result = validateParams(actionType, { foo: 'bar' }, { configurationUtilities }); expect(result).toEqual({ foo: 'bar' }); - expect(() => validateParams(actionType, { bar: 2 })).toThrowErrorMatchingInlineSnapshot( + expect(() => + validateParams(actionType, { bar: 2 }, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot( `"error validating action params: [foo]: expected value of type [string] but got [undefined]"` ); }); @@ -197,9 +226,15 @@ describe('validateConnectors', () => { supportedFeatureIds: ['alerting'], executor, validate: { - params: selfValidator, - config: selfValidator, - secrets: selfValidator, + params: { + validateSchema: selfValidator, + }, + config: { + validateSchema: selfValidator, + }, + secrets: { + validateSchema: selfValidator, + }, connector: () => null, }, }; diff --git a/x-pack/plugins/actions/server/saved_objects/transform_connectors_for_export.ts b/x-pack/plugins/actions/server/saved_objects/transform_connectors_for_export.ts index 9c690885bfe3c..34ff241c36852 100644 --- a/x-pack/plugins/actions/server/saved_objects/transform_connectors_for_export.ts +++ b/x-pack/plugins/actions/server/saved_objects/transform_connectors_for_export.ts @@ -6,6 +6,7 @@ */ import { SavedObject } from '@kbn/core/server'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; import { validateSecrets } from '../lib'; import { RawAction, ActionType } from '../types'; @@ -18,20 +19,22 @@ export function transformConnectorsForExport( const connector = c as SavedObject; return transformConnectorForExport( connector, - actionTypeRegistry.get(connector.attributes.actionTypeId) + actionTypeRegistry.get(connector.attributes.actionTypeId), + actionTypeRegistry.getUtils() ); }); } function transformConnectorForExport( connector: SavedObject, - actionType: ActionType + actionType: ActionType, + configurationUtilities: ActionsConfigurationUtilities ): SavedObject { let isMissingSecrets = false; try { // If connector requires secrets, this will throw an error - validateSecrets(actionType, {}); + validateSecrets(actionType, {}, { configurationUtilities }); // If connector has optional (or no) secrets, set isMissingSecrets value to value of hasAuth // If connector doesn't have hasAuth value, default to isMissingSecrets: false diff --git a/x-pack/plugins/actions/server/sub_action_framework/validators.ts b/x-pack/plugins/actions/server/sub_action_framework/validators.ts index 2c272a7d858d6..b1e24156fc0c6 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/validators.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/validators.ts @@ -21,18 +21,24 @@ export const buildValidators = < connector: SubActionConnectorType; }) => { return { - config: connector.schema.config, - secrets: connector.schema.secrets, - params: schema.object({ - subAction: schema.string(), - /** - * With this validation we enforce the subActionParams to be an object. - * Each sub action has different parameters and they are validated inside the executor - * (x-pack/plugins/actions/server/sub_action_framework/executor.ts). For that reason, - * we allow all unknowns at this level of validation as they are not known at this - * time of execution. - */ - subActionParams: schema.object({}, { unknowns: 'allow' }), - }), + config: { + validateSchema: connector.schema.config, + }, + secrets: { + validateSchema: connector.schema.secrets, + }, + params: { + validateSchema: schema.object({ + subAction: schema.string(), + /** + * With this validation we enforce the subActionParams to be an object. + * Each sub action has different parameters and they are validated inside the executor + * (x-pack/plugins/actions/server/sub_action_framework/executor.ts). For that reason, + * we allow all unknowns at this level of validation as they are not known at this + * time of execution. + */ + subActionParams: schema.object({}, { unknowns: 'allow' }), + }), + }, }; }; From 3c34474fc43a48d7a8d60ac207ef65597e9041d4 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Thu, 25 Aug 2022 10:29:18 -0400 Subject: [PATCH 4/7] Fixing functional tests --- .../server/builtin_action_types/jira/index.ts | 2 +- .../sub_action_framework/validators.test.ts | 10 ++--- .../plugins/alerts/server/action_types.ts | 40 ++++++++++++++----- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts index ea9ce5124ccb3..fbee03e4032be 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts @@ -83,7 +83,7 @@ export function getActionType( }, secrets: { validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.config, + validate: validate.secrets, }, params: { validateSchema: ExecutorParamsSchema, diff --git a/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts b/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts index ae7f42cb75a9c..9024304e53a90 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts @@ -47,21 +47,21 @@ describe('Validators', () => { const validator = createValidator(TestSubActionConnector); const { config, secrets } = validator; - expect(config).toEqual(TestConfigSchema); - expect(secrets).toEqual(TestSecretsSchema); + expect(config).toEqual({ validateSchema: TestConfigSchema }); + expect(secrets).toEqual({ validateSchema: TestSecretsSchema }); }); it('should validate the params correctly', async () => { const validator = createValidator(TestSubActionConnector); const { params } = validator; - expect(params.validate({ subAction: 'test', subActionParams: {} })); + expect(params.validateSchema.validate({ subAction: 'test', subActionParams: {} })); }); it('should allow any field in subActionParams', async () => { const validator = createValidator(TestSubActionConnector); const { params } = validator; expect( - params.validate({ + params.validateSchema.validate({ subAction: 'test', subActionParams: { foo: 'foo', @@ -94,6 +94,6 @@ describe('Validators', () => { ])('should throw if the subAction is %p', async (subAction) => { const validator = createValidator(TestSubActionConnector); const { params } = validator; - expect(() => params.validate({ subAction, subActionParams: {} })).toThrow(); + expect(() => params.validateSchema.validate({ subAction, subActionParams: {} })).toThrow(); }); }); diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts index 228c71b65e437..4bce070eab1c9 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts @@ -87,9 +87,15 @@ function getIndexRecordActionType() { minimumLicenseRequired: 'gold', supportedFeatureIds: ['alerting'], validate: { - params: paramsSchema, - config: configSchema, - secrets: secretsSchema, + params: { + validateSchema: paramsSchema, + }, + config: { + validateSchema: configSchema, + }, + secrets: { + validateSchema: secretsSchema, + }, }, async executor({ config, secrets, params, services, actionId }) { await services.scopedClusterClient.index({ @@ -128,9 +134,15 @@ function getDelayedActionType() { minimumLicenseRequired: 'gold', supportedFeatureIds: ['alerting'], validate: { - params: paramsSchema, - config: configSchema, - secrets: secretsSchema, + params: { + validateSchema: paramsSchema, + }, + config: { + validateSchema: configSchema, + }, + secrets: { + validateSchema: secretsSchema, + }, }, async executor({ config, secrets, params, services, actionId }) { await new Promise((resolve) => { @@ -156,7 +168,9 @@ function getFailingActionType() { minimumLicenseRequired: 'gold', supportedFeatureIds: ['alerting'], validate: { - params: paramsSchema, + params: { + validateSchema: paramsSchema, + }, }, async executor({ config, secrets, params, services }) { await services.scopedClusterClient.index({ @@ -190,7 +204,9 @@ function getRateLimitedActionType() { supportedFeatureIds: ['alerting'], maxAttempts: 2, validate: { - params: paramsSchema, + params: { + validateSchema: paramsSchema, + }, }, async executor({ config, params, services }) { await services.scopedClusterClient.index({ @@ -227,7 +243,9 @@ function getNoAttemptsRateLimitedActionType() { supportedFeatureIds: ['alerting'], maxAttempts: 0, validate: { - params: paramsSchema, + params: { + validateSchema: paramsSchema, + }, }, async executor({ config, params, services }) { await services.scopedClusterClient.index({ @@ -266,7 +284,9 @@ function getAuthorizationActionType(core: CoreSetup) { minimumLicenseRequired: 'gold', supportedFeatureIds: ['alerting'], validate: { - params: paramsSchema, + params: { + validateSchema: paramsSchema, + }, }, async executor({ params, services, actionId }) { // Call cluster From 760984fe80fac6392fa5af66c80daad6d2fea6ed Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Fri, 26 Aug 2022 13:16:21 -0400 Subject: [PATCH 5/7] Updating validators --- .../actions/server/actions_client.test.ts | 4 +-- .../cases_webhook/index.ts | 10 +++---- .../cases_webhook/types.ts | 4 +-- .../cases_webhook/validators.ts | 8 ++--- .../server/builtin_action_types/email.ts | 26 ++++++++-------- .../server/builtin_action_types/es_index.ts | 4 +-- .../server/builtin_action_types/jira/index.ts | 10 +++---- .../server/builtin_action_types/jira/types.ts | 4 +-- .../builtin_action_types/jira/validators.ts | 8 ++--- .../server/builtin_action_types/pagerduty.ts | 14 ++++----- .../builtin_action_types/resilient/index.ts | 10 +++---- .../builtin_action_types/resilient/types.ts | 4 +-- .../resilient/validators.ts | 8 ++--- .../server/builtin_action_types/server_log.ts | 2 +- .../builtin_action_types/servicenow/index.ts | 30 +++++++++---------- .../builtin_action_types/servicenow/types.ts | 4 +-- .../servicenow/validators.ts | 8 ++--- .../server/builtin_action_types/slack.ts | 12 ++++---- .../builtin_action_types/swimlane/index.ts | 10 +++---- .../builtin_action_types/swimlane/types.ts | 4 +-- .../swimlane/validators.ts | 8 ++--- .../server/builtin_action_types/teams.ts | 12 ++++---- .../server/builtin_action_types/webhook.ts | 14 ++++----- .../server/builtin_action_types/xmatters.ts | 22 +++++++------- .../server/lib/action_executor.test.ts | 4 +-- .../server/lib/validate_with_schema.test.ts | 30 +++++++++---------- .../server/lib/validate_with_schema.ts | 21 +++++++------ .../sub_action_framework/validators.test.ts | 10 +++---- .../server/sub_action_framework/validators.ts | 6 ++-- x-pack/plugins/actions/server/types.ts | 4 +-- .../plugins/alerts/server/action_types.ts | 20 ++++++------- 31 files changed, 169 insertions(+), 166 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index b807d73ee2f3b..1c1a6f7167b8f 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -377,7 +377,7 @@ describe('create()', () => { supportedFeatureIds: ['alerting'], validate: { config: { - validateSchema: schema.object({ + schema: schema.object({ param1: schema.string(), }), }, @@ -1952,7 +1952,7 @@ describe('update()', () => { supportedFeatureIds: ['alerting'], validate: { config: { - validateSchema: schema.object({ + schema: schema.object({ param1: schema.string(), }), }, diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts index 54961cbc77562..821ab792308c1 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/index.ts @@ -50,15 +50,15 @@ export function getActionType({ name: i18n.NAME, validate: { config: { - validateSchema: ExternalIncidentServiceConfigurationSchema, - validate: validate.config, + schema: ExternalIncidentServiceConfigurationSchema, + customValidator: validate.config, }, secrets: { - validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.secrets, + schema: ExternalIncidentServiceSecretConfigurationSchema, + customValidator: validate.secrets, }, params: { - validateSchema: ExecutorParamsSchema, + schema: ExecutorParamsSchema, }, connector: validate.connector, }, diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts index 85d25b7cf5498..aae733d4ed94b 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/types.ts @@ -41,11 +41,11 @@ export interface ExternalServiceCredentials { export interface ExternalServiceValidation { config: ( configObject: CasesWebhookPublicConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => void; secrets: ( secrets: CasesWebhookSecretConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => void; connector: ( configObject: CasesWebhookPublicConfigurationType, diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts index 4c8d8643fce16..bd79ee093274f 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts @@ -16,9 +16,9 @@ import { ValidatorServices } from '../../types'; const validateConfig = ( configObject: CasesWebhookPublicConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; const { createCommentUrl, createIncidentUrl, @@ -43,7 +43,7 @@ const validateConfig = ( return i18n.INVALID_URL(err, url); } try { - configurationUtilities?.ensureUriAllowed(url); + configurationUtilities.ensureUriAllowed(url); } catch (allowListError) { return i18n.CONFIG_ERR(allowListError.message); } @@ -63,7 +63,7 @@ export const validateConnector = ( export const validateSecrets = ( secrets: CasesWebhookSecretConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { // user and password must be set together (or not at all) if (!secrets.password && !secrets.user) return; 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 fb9a01ac5e7cd..26755ea8d1f57 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.ts @@ -72,13 +72,13 @@ const ConfigSchema = schema.object(ConfigSchemaProps); function validateConfig( configObject: ActionTypeConfigType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ): string | void { const config = configObject; - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; const emails = [config.from]; - const invalidEmailsMessage = configurationUtilities?.validateEmailAddresses(emails); + const invalidEmailsMessage = configurationUtilities.validateEmailAddresses(emails); if (!!invalidEmailsMessage) { return `[from]: ${invalidEmailsMessage}`; } @@ -115,7 +115,7 @@ function validateConfig( return '[port] is required'; } - if (!configurationUtilities?.isHostnameAllowed(config.host)) { + if (!configurationUtilities.isHostnameAllowed(config.host)) { return `[host] value '${config.host}' is not in the allowedHosts configuration`; } } else { @@ -124,7 +124,7 @@ function validateConfig( if (host == null) { return `[service] value '${config.service}' is not valid`; } - if (!configurationUtilities?.isHostnameAllowed(host)) { + if (!configurationUtilities.isHostnameAllowed(host)) { return `[service] value '${config.service}' resolves to host '${host}' which is not in the allowedHosts configuration`; } } @@ -168,9 +168,9 @@ const ParamsSchema = schema.object(ParamsSchemaProps); function validateParams( paramsObject: unknown, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ): string | void { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; // avoids circular reference ... const params = paramsObject as ActionParamsType; @@ -183,7 +183,7 @@ function validateParams( } const emails = withoutMustacheTemplate(to.concat(cc).concat(bcc)); - const invalidEmailsMessage = configurationUtilities?.validateEmailAddresses(emails, { + const invalidEmailsMessage = configurationUtilities.validateEmailAddresses(emails, { treatMustacheTemplatesAsValid: true, }); if (invalidEmailsMessage) { @@ -233,15 +233,15 @@ export function getActionType(params: GetActionTypeParams): EmailActionType { ], validate: { config: { - validateSchema: ConfigSchema, - validate: validateConfig, + schema: ConfigSchema, + customValidator: validateConfig, }, secrets: { - validateSchema: SecretsSchema, + schema: SecretsSchema, }, params: { - validateSchema: ParamsSchema, - validate: validateParams, + schema: ParamsSchema, + customValidator: validateParams, }, connector: validateConnector, }, diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.ts index a8b6caffcca9a..cc5b58f211671 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/es_index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/es_index.ts @@ -73,10 +73,10 @@ export function getActionType({ logger }: { logger: Logger }): ESIndexActionType ], validate: { config: { - validateSchema: ConfigSchema, + schema: ConfigSchema, }, params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, }, executor: curry(executor)({ logger }), diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts index fbee03e4032be..86ed10f57d20b 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/index.ts @@ -78,15 +78,15 @@ export function getActionType( ], validate: { config: { - validateSchema: ExternalIncidentServiceConfigurationSchema, - validate: validate.config, + schema: ExternalIncidentServiceConfigurationSchema, + customValidator: validate.config, }, secrets: { - validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.secrets, + schema: ExternalIncidentServiceSecretConfigurationSchema, + customValidator: validate.secrets, }, params: { - validateSchema: ExecutorParamsSchema, + schema: ExecutorParamsSchema, }, }, executor: curry(executor)({ logger, configurationUtilities }), diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts index 8f540b8194d6f..75b5f5a2ef30a 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/types.ts @@ -38,8 +38,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configObject: any, validatorServices?: ValidatorServices) => void; - secrets: (secrets: any, validatorServices?: ValidatorServices) => void; + config: (configObject: any, validatorServices: ValidatorServices) => void; + secrets: (secrets: any, validatorServices: ValidatorServices) => void; } export interface ExternalServiceIncidentResponse { diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts index 0cbdae09edd28..a5a6ff80f586e 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts @@ -16,11 +16,11 @@ import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( configObject: JiraPublicConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; try { - configurationUtilities?.ensureUriAllowed(configObject.apiUrl); + configurationUtilities.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } @@ -28,7 +28,7 @@ export const validateCommonConfig = ( export const validateCommonSecrets = ( secrets: JiraSecretConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => {}; export const validate: ExternalServiceValidation = { diff --git a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts index 347fdf536b7ac..486bcdff9f1f9 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts @@ -159,14 +159,14 @@ export function getActionType({ ], validate: { config: { - validateSchema: ConfigSchema, - validate: validateActionTypeConfig, + schema: ConfigSchema, + customValidator: validateActionTypeConfig, }, secrets: { - validateSchema: SecretsSchema, + schema: SecretsSchema, }, params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, }, executor: curry(executor)({ logger, configurationUtilities }), @@ -175,11 +175,11 @@ export function getActionType({ function validateActionTypeConfig( configObject: ActionTypeConfigType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; try { - configurationUtilities?.ensureUriAllowed(getPagerDutyApiUrl(configObject)); + configurationUtilities.ensureUriAllowed(getPagerDutyApiUrl(configObject)); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.pagerduty.pagerdutyConfigurationError', { defaultMessage: 'error configuring pagerduty action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts index 087687870337d..b0553a63239c2 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/index.ts @@ -67,15 +67,15 @@ export function getActionType( ], validate: { config: { - validateSchema: ExternalIncidentServiceConfigurationSchema, - validate: validate.config, + schema: ExternalIncidentServiceConfigurationSchema, + customValidator: validate.config, }, secrets: { - validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.secrets, + schema: ExternalIncidentServiceSecretConfigurationSchema, + customValidator: validate.secrets, }, params: { - validateSchema: ExecutorParamsSchema, + schema: ExecutorParamsSchema, }, }, executor: curry(executor)({ logger, configurationUtilities }), diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts index f19bf4f87563f..2528e1742e1b9 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/types.ts @@ -43,8 +43,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configObject: any, validatorServices?: ValidatorServices) => void; - secrets: (secrets: any, validatorServices?: ValidatorServices) => void; + config: (configObject: any, validatorServices: ValidatorServices) => void; + secrets: (secrets: any, validatorServices: ValidatorServices) => void; } export interface ExternalServiceIncidentResponse { diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts index a33b6c5742416..abf244c31177b 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts @@ -16,11 +16,11 @@ import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( configObject: ResilientPublicConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; try { - configurationUtilities?.ensureUriAllowed(configObject.apiUrl); + configurationUtilities.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } @@ -28,7 +28,7 @@ export const validateCommonConfig = ( export const validateCommonSecrets = ( secrets: ResilientSecretConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => {}; export const validate: ExternalServiceValidation = { diff --git a/x-pack/plugins/actions/server/builtin_action_types/server_log.ts b/x-pack/plugins/actions/server/builtin_action_types/server_log.ts index aaee8824682e3..e33894e6099d7 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/server_log.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/server_log.ts @@ -52,7 +52,7 @@ export function getActionType({ logger }: { logger: Logger }): ServerLogActionTy supportedFeatureIds: [AlertingConnectorFeatureId, UptimeConnectorFeatureId], validate: { params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, }, executor: curry(executor)({ logger }), diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts index f82967679970c..cb008de5587d6 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/index.ts @@ -106,16 +106,16 @@ export function getServiceNowITSMActionType( ], validate: { config: { - validateSchema: ExternalIncidentServiceConfigurationSchema, - validate: validate.config, + schema: ExternalIncidentServiceConfigurationSchema, + customValidator: validate.config, }, secrets: { - validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.secrets, + schema: ExternalIncidentServiceSecretConfigurationSchema, + customValidator: validate.secrets, }, connector: validate.connector, params: { - validateSchema: ExecutorParamsSchemaITSM, + schema: ExecutorParamsSchemaITSM, }, }, executor: curry(executor)({ @@ -143,16 +143,16 @@ export function getServiceNowSIRActionType( ], validate: { config: { - validateSchema: ExternalIncidentServiceConfigurationSchema, - validate: validate.config, + schema: ExternalIncidentServiceConfigurationSchema, + customValidator: validate.config, }, secrets: { - validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.secrets, + schema: ExternalIncidentServiceSecretConfigurationSchema, + customValidator: validate.secrets, }, connector: validate.connector, params: { - validateSchema: ExecutorParamsSchemaSIR, + schema: ExecutorParamsSchemaSIR, }, }, executor: curry(executor)({ @@ -176,16 +176,16 @@ export function getServiceNowITOMActionType( supportedFeatureIds: [AlertingConnectorFeatureId, SecurityConnectorFeatureId], validate: { config: { - validateSchema: ExternalIncidentServiceConfigurationBaseSchema, - validate: validate.config, + schema: ExternalIncidentServiceConfigurationBaseSchema, + customValidator: validate.config, }, secrets: { - validateSchema: ExternalIncidentServiceSecretConfigurationSchema, - validate: validate.secrets, + schema: ExternalIncidentServiceSecretConfigurationSchema, + customValidator: validate.secrets, }, connector: validate.connector, params: { - validateSchema: ExecutorParamsSchemaITOM, + schema: ExecutorParamsSchemaITOM, }, }, executor: curry(executorITOM)({ diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts index 6fe6578641bba..069f123fae4ec 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts @@ -77,8 +77,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configObject: any, validatorServices?: ValidatorServices) => void; - secrets: (secrets: any, validatorServices?: ValidatorServices) => void; + config: (configObject: any, validatorServices: ValidatorServices) => void; + secrets: (secrets: any, validatorServices: ValidatorServices) => void; connector: (config: any, secrets: any) => string | null; } diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts index 6758a62e1360d..7c9964c1117ee 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts @@ -16,12 +16,12 @@ import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( config: ServiceNowPublicConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { const { isOAuth, apiUrl, userIdentifierValue, clientId, jwtKeyId } = config; - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; try { - configurationUtilities?.ensureUriAllowed(apiUrl); + configurationUtilities.ensureUriAllowed(apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } @@ -43,7 +43,7 @@ export const validateCommonConfig = ( export const validateCommonSecrets = ( secrets: ServiceNowSecretConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { const { username, password, clientSecret, privateKey } = secrets; diff --git a/x-pack/plugins/actions/server/builtin_action_types/slack.ts b/x-pack/plugins/actions/server/builtin_action_types/slack.ts index ae5bb589adc32..f6d9edb0b2456 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.ts @@ -83,11 +83,11 @@ export function getActionType({ ], validate: { secrets: { - validateSchema: SecretsSchema, - validate: validateActionTypeConfig, + schema: SecretsSchema, + customValidator: validateActionTypeConfig, }, params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, }, renderParameterTemplates, @@ -106,9 +106,9 @@ function renderParameterTemplates( function validateActionTypeConfig( secretsObject: ActionTypeSecretsType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; const configuredUrl = secretsObject.webhookUrl; try { new URL(configuredUrl); @@ -119,7 +119,7 @@ function validateActionTypeConfig( } try { - configurationUtilities?.ensureUriAllowed(configuredUrl); + configurationUtilities.ensureUriAllowed(configuredUrl); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.slack.slackConfigurationError', { defaultMessage: 'error configuring slack action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts index dd5a61a76de0a..38cf28edc1e09 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/index.ts @@ -61,15 +61,15 @@ export function getActionType( ], validate: { config: { - validateSchema: SwimlaneServiceConfigurationSchema, - validate: validate.config, + schema: SwimlaneServiceConfigurationSchema, + customValidator: validate.config, }, secrets: { - validateSchema: SwimlaneSecretsConfigurationSchema, - validate: validate.secrets, + schema: SwimlaneSecretsConfigurationSchema, + customValidator: validate.secrets, }, params: { - validateSchema: ExecutorParamsSchema, + schema: ExecutorParamsSchema, }, }, executor: curry(executor)({ logger, configurationUtilities }), diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts index d832ff211c512..b00f0d25ee2f7 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/types.ts @@ -30,8 +30,8 @@ export interface ExternalServiceCredentials { } export interface ExternalServiceValidation { - config: (configObject: any, validatorServices?: ValidatorServices) => void; - secrets: (secrets: any, validatorServices?: ValidatorServices) => void; + config: (configObject: any, validatorServices: ValidatorServices) => void; + secrets: (secrets: any, validatorServices: ValidatorServices) => void; } export interface CreateRecordParams { diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts index 5d77c765b6efd..ae29e75ffdfae 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts @@ -15,11 +15,11 @@ import { ValidatorServices } from '../../types'; export const validateCommonConfig = ( configObject: SwimlanePublicConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; try { - configurationUtilities?.ensureUriAllowed(configObject.apiUrl); + configurationUtilities.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); } @@ -27,7 +27,7 @@ export const validateCommonConfig = ( export const validateCommonSecrets = ( secrets: SwimlaneSecretConfigurationType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) => {}; export const validate: ExternalServiceValidation = { diff --git a/x-pack/plugins/actions/server/builtin_action_types/teams.ts b/x-pack/plugins/actions/server/builtin_action_types/teams.ts index 503e043d8eef8..bf4b608eed36e 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/teams.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/teams.ts @@ -75,11 +75,11 @@ export function getActionType({ ], validate: { secrets: { - validateSchema: SecretsSchema, - validate: validateActionTypeConfig, + schema: SecretsSchema, + customValidator: validateActionTypeConfig, }, params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, }, executor: curry(teamsExecutor)({ logger, configurationUtilities }), @@ -88,9 +88,9 @@ export function getActionType({ function validateActionTypeConfig( secretsObject: ActionTypeSecretsType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; const configuredUrl = secretsObject.webhookUrl; try { new URL(configuredUrl); @@ -101,7 +101,7 @@ function validateActionTypeConfig( } try { - configurationUtilities?.ensureUriAllowed(configuredUrl); + configurationUtilities.ensureUriAllowed(configuredUrl); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.teams.teamsConfigurationError', { defaultMessage: 'error configuring teams action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts index 890803836e196..0040b485731c5 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts @@ -105,14 +105,14 @@ export function getActionType({ ], validate: { config: { - validateSchema: ConfigSchema, - validate: validateActionTypeConfig, + schema: ConfigSchema, + customValidator: validateActionTypeConfig, }, secrets: { - validateSchema: SecretsSchema, + schema: SecretsSchema, }, params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, }, renderParameterTemplates, @@ -132,9 +132,9 @@ function renderParameterTemplates( function validateActionTypeConfig( configObject: ActionTypeConfigType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ) { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; const configuredUrl = configObject.url; try { new URL(configuredUrl); @@ -148,7 +148,7 @@ function validateActionTypeConfig( } try { - configurationUtilities?.ensureUriAllowed(configuredUrl); + configurationUtilities.ensureUriAllowed(configuredUrl); } catch (allowListError) { return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { defaultMessage: 'error configuring webhook action: {message}', diff --git a/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts b/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts index 54bd918fbe1c4..c42e349f4cc60 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts @@ -77,15 +77,15 @@ export function getActionType({ supportedFeatureIds: [AlertingConnectorFeatureId], validate: { config: { - validateSchema: ConfigSchema, - validate: validateActionTypeConfig, + schema: ConfigSchema, + customValidator: validateActionTypeConfig, }, secrets: { - validateSchema: SecretsSchema, - validate: validateActionTypeSecrets, + schema: SecretsSchema, + customValidator: validateActionTypeSecrets, }, params: { - validateSchema: ParamsSchema, + schema: ParamsSchema, }, connector: validateConnector, }, @@ -95,9 +95,9 @@ export function getActionType({ function validateActionTypeConfig( configObject: ActionTypeConfigType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ): string | undefined { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; const configuredUrl = configObject.configUrl; const usesBasic = configObject.usesBasic; if (!usesBasic) return; @@ -116,7 +116,7 @@ function validateActionTypeConfig( try { if (configuredUrl) { - configurationUtilities?.ensureUriAllowed(configuredUrl); + configurationUtilities.ensureUriAllowed(configuredUrl); } } catch (allowListError) { return i18n.translate('xpack.actions.builtin.xmatters.xmattersConfigurationError', { @@ -178,9 +178,9 @@ function validateConnector( function validateActionTypeSecrets( secretsObject: ActionTypeSecretsType, - validatorServices?: ValidatorServices + validatorServices: ValidatorServices ): string | undefined { - const { configurationUtilities } = validatorServices || {}; + const { configurationUtilities } = validatorServices; if (!secretsObject.secretsUrl && !secretsObject.user && !secretsObject.password) { return i18n.translate('xpack.actions.builtin.xmatters.noSecretsProvided', { defaultMessage: 'Provide either secretsUrl link or user/password to authenticate', @@ -214,7 +214,7 @@ function validateActionTypeSecrets( // Test that hostname is allowed try { if (secretsObject.secretsUrl) { - configurationUtilities?.ensureUriAllowed(secretsObject.secretsUrl); + configurationUtilities.ensureUriAllowed(secretsObject.secretsUrl); } } catch (allowListError) { return i18n.translate('xpack.actions.builtin.xmatters.xmattersHostnameNotAllowed', { 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 df3095daea4d9..54405b03e7c02 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.test.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.test.ts @@ -271,7 +271,7 @@ test('throws an error when config is invalid', async () => { supportedFeatureIds: ['alerting'], validate: { config: { - validateSchema: schema.object({ + schema: schema.object({ param1: schema.string(), }), }, @@ -355,7 +355,7 @@ test('throws an error when params is invalid', async () => { supportedFeatureIds: ['alerting'], validate: { params: { - validateSchema: schema.object({ + schema: schema.object({ param1: schema.string(), }), }, 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 20225ed0d4da7..172baf82ca6b9 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 @@ -72,13 +72,13 @@ test('should validate when validators return incoming value', () => { executor, validate: { params: { - validateSchema: selfValidator, + schema: selfValidator, }, config: { - validateSchema: selfValidator, + schema: selfValidator, }, secrets: { - validateSchema: selfValidator, + schema: selfValidator, }, connector: () => null, }, @@ -111,13 +111,13 @@ test('should validate when validators return different values', () => { executor, validate: { params: { - validateSchema: selfValidator, + schema: selfValidator, }, config: { - validateSchema: selfValidator, + schema: selfValidator, }, secrets: { - validateSchema: selfValidator, + schema: selfValidator, }, connector: () => null, }, @@ -153,13 +153,13 @@ test('should throw with expected error when validators fail', () => { executor, validate: { params: { - validateSchema: erroringValidator, + schema: erroringValidator, }, config: { - validateSchema: erroringValidator, + schema: erroringValidator, }, secrets: { - validateSchema: erroringValidator, + schema: erroringValidator, }, connector: () => 'test error', }, @@ -194,13 +194,13 @@ test('should work with @kbn/config-schema', () => { executor, validate: { params: { - validateSchema: testSchema, + schema: testSchema, }, config: { - validateSchema: testSchema, + schema: testSchema, }, secrets: { - validateSchema: testSchema, + schema: testSchema, }, connector: () => null, }, @@ -227,13 +227,13 @@ describe('validateConnectors', () => { executor, validate: { params: { - validateSchema: selfValidator, + schema: selfValidator, }, config: { - validateSchema: selfValidator, + schema: selfValidator, }, secrets: { - validateSchema: selfValidator, + schema: selfValidator, }, 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 7d45888a17725..d96e0c86ba32e 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.ts @@ -101,9 +101,9 @@ function validateWithSchema< case 'params': name = 'action params'; if (actionType.validate.params) { - const params = actionType.validate.params.validateSchema.validate(value); - if (actionType.validate.params.validate) { - const result = actionType.validate.params.validate(params, validatorServices); + const params = actionType.validate.params.schema.validate(value); + if (actionType.validate.params.customValidator) { + const result = actionType.validate.params.customValidator(params, validatorServices); if (result) { throw new Error(result); } @@ -114,9 +114,9 @@ function validateWithSchema< case 'config': name = 'action type config'; if (actionType.validate.config) { - const config = actionType.validate.config.validateSchema.validate(value); - if (actionType.validate.config.validate) { - const result = actionType.validate.config.validate(config, validatorServices); + const config = actionType.validate.config.schema.validate(value); + if (actionType.validate.config.customValidator) { + const result = actionType.validate.config.customValidator(config, validatorServices); if (result) { throw new Error(result); } @@ -128,9 +128,12 @@ function validateWithSchema< case 'secrets': name = 'action type secrets'; if (actionType.validate.secrets) { - const secrets = actionType.validate.secrets.validateSchema.validate(value); - if (actionType.validate.secrets.validate) { - const result = actionType.validate.secrets.validate(secrets, validatorServices); + const secrets = actionType.validate.secrets.schema.validate(value); + if (actionType.validate.secrets.customValidator) { + const result = actionType.validate.secrets.customValidator( + secrets, + validatorServices + ); if (result) { throw new Error(result); } diff --git a/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts b/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts index 9024304e53a90..6cae35141b498 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/validators.test.ts @@ -47,21 +47,21 @@ describe('Validators', () => { const validator = createValidator(TestSubActionConnector); const { config, secrets } = validator; - expect(config).toEqual({ validateSchema: TestConfigSchema }); - expect(secrets).toEqual({ validateSchema: TestSecretsSchema }); + expect(config).toEqual({ schema: TestConfigSchema }); + expect(secrets).toEqual({ schema: TestSecretsSchema }); }); it('should validate the params correctly', async () => { const validator = createValidator(TestSubActionConnector); const { params } = validator; - expect(params.validateSchema.validate({ subAction: 'test', subActionParams: {} })); + expect(params.schema.validate({ subAction: 'test', subActionParams: {} })); }); it('should allow any field in subActionParams', async () => { const validator = createValidator(TestSubActionConnector); const { params } = validator; expect( - params.validateSchema.validate({ + params.schema.validate({ subAction: 'test', subActionParams: { foo: 'foo', @@ -94,6 +94,6 @@ describe('Validators', () => { ])('should throw if the subAction is %p', async (subAction) => { const validator = createValidator(TestSubActionConnector); const { params } = validator; - expect(() => params.validateSchema.validate({ subAction, subActionParams: {} })).toThrow(); + expect(() => params.schema.validate({ subAction, subActionParams: {} })).toThrow(); }); }); diff --git a/x-pack/plugins/actions/server/sub_action_framework/validators.ts b/x-pack/plugins/actions/server/sub_action_framework/validators.ts index b1e24156fc0c6..be6dafed28163 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/validators.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/validators.ts @@ -22,13 +22,13 @@ export const buildValidators = < }) => { return { config: { - validateSchema: connector.schema.config, + schema: connector.schema.config, }, secrets: { - validateSchema: connector.schema.secrets, + schema: connector.schema.secrets, }, params: { - validateSchema: schema.object({ + schema: schema.object({ subAction: schema.string(), /** * With this validation we enforce the subActionParams to be an object. diff --git a/x-pack/plugins/actions/server/types.ts b/x-pack/plugins/actions/server/types.ts index 1facab829e244..684029a85a206 100644 --- a/x-pack/plugins/actions/server/types.ts +++ b/x-pack/plugins/actions/server/types.ts @@ -95,10 +95,10 @@ export type ExecutorType = ( ) => Promise>; interface ValidatorType { - validateSchema: { + schema: { validate(value: unknown): Type; }; - validate?: (value: Type, validatorServices?: ValidatorServices) => string | void; + customValidator?: (value: Type, validatorServices: ValidatorServices) => string | void; } export interface ValidatorServices { diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts index 4bce070eab1c9..40e5c7d1b3abc 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/action_types.ts @@ -88,13 +88,13 @@ function getIndexRecordActionType() { supportedFeatureIds: ['alerting'], validate: { params: { - validateSchema: paramsSchema, + schema: paramsSchema, }, config: { - validateSchema: configSchema, + schema: configSchema, }, secrets: { - validateSchema: secretsSchema, + schema: secretsSchema, }, }, async executor({ config, secrets, params, services, actionId }) { @@ -135,13 +135,13 @@ function getDelayedActionType() { supportedFeatureIds: ['alerting'], validate: { params: { - validateSchema: paramsSchema, + schema: paramsSchema, }, config: { - validateSchema: configSchema, + schema: configSchema, }, secrets: { - validateSchema: secretsSchema, + schema: secretsSchema, }, }, async executor({ config, secrets, params, services, actionId }) { @@ -169,7 +169,7 @@ function getFailingActionType() { supportedFeatureIds: ['alerting'], validate: { params: { - validateSchema: paramsSchema, + schema: paramsSchema, }, }, async executor({ config, secrets, params, services }) { @@ -205,7 +205,7 @@ function getRateLimitedActionType() { maxAttempts: 2, validate: { params: { - validateSchema: paramsSchema, + schema: paramsSchema, }, }, async executor({ config, params, services }) { @@ -244,7 +244,7 @@ function getNoAttemptsRateLimitedActionType() { maxAttempts: 0, validate: { params: { - validateSchema: paramsSchema, + schema: paramsSchema, }, }, async executor({ config, params, services }) { @@ -285,7 +285,7 @@ function getAuthorizationActionType(core: CoreSetup) { supportedFeatureIds: ['alerting'], validate: { params: { - validateSchema: paramsSchema, + schema: paramsSchema, }, }, async executor({ params, services, actionId }) { From a3a6548f661e305c10f0d33ca1dc3131a1354572 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Fri, 26 Aug 2022 14:02:19 -0400 Subject: [PATCH 6/7] Updating to throw errors --- .../cases_webhook/validators.ts | 6 +- .../server/builtin_action_types/email.ts | 36 ++++---- .../builtin_action_types/jira/validators.ts | 2 +- .../server/builtin_action_types/pagerduty.ts | 14 +-- .../resilient/validators.ts | 2 +- .../servicenow/validators.test.ts | 42 +++++---- .../servicenow/validators.ts | 14 +-- .../server/builtin_action_types/slack.ts | 22 +++-- .../swimlane/validators.ts | 2 +- .../server/builtin_action_types/teams.ts | 22 +++-- .../server/builtin_action_types/webhook.ts | 28 +++--- .../server/builtin_action_types/xmatters.ts | 86 +++++++++++-------- .../server/lib/validate_with_schema.ts | 38 ++++---- x-pack/plugins/actions/server/types.ts | 2 +- 14 files changed, 173 insertions(+), 143 deletions(-) diff --git a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts index bd79ee093274f..22113f0f7626e 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/cases_webhook/validators.ts @@ -40,12 +40,12 @@ const validateConfig = ( try { new URL(url); } catch (err) { - return i18n.INVALID_URL(err, url); + throw new Error(i18n.INVALID_URL(err, url)); } try { configurationUtilities.ensureUriAllowed(url); } catch (allowListError) { - return i18n.CONFIG_ERR(allowListError.message); + throw new Error(i18n.CONFIG_ERR(allowListError.message)); } } } @@ -68,7 +68,7 @@ export const validateSecrets = ( // user and password must be set together (or not at all) if (!secrets.password && !secrets.user) return; if (secrets.password && secrets.user) return; - return i18n.INVALID_USER_PW; + throw new Error(i18n.INVALID_USER_PW); }; export const validate: ExternalServiceValidation = { 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 26755ea8d1f57..8d70f1de208b6 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.ts @@ -70,17 +70,14 @@ const ConfigSchemaProps = { const ConfigSchema = schema.object(ConfigSchemaProps); -function validateConfig( - configObject: ActionTypeConfigType, - validatorServices: ValidatorServices -): string | void { +function validateConfig(configObject: ActionTypeConfigType, validatorServices: ValidatorServices) { const config = configObject; const { configurationUtilities } = validatorServices; const emails = [config.from]; const invalidEmailsMessage = configurationUtilities.validateEmailAddresses(emails); if (!!invalidEmailsMessage) { - return `[from]: ${invalidEmailsMessage}`; + throw new Error(`[from]: ${invalidEmailsMessage}`); } // If service is set as JSON_TRANSPORT_SERVICE or EXCHANGE, host/port are ignored, when the email is sent. @@ -91,41 +88,43 @@ function validateConfig( return; } else if (config.service === AdditionalEmailServices.EXCHANGE) { if (config.clientId == null && config.tenantId == null) { - return '[clientId]/[tenantId] is required'; + throw new Error('[clientId]/[tenantId] is required'); } if (config.clientId == null) { - return '[clientId] is required'; + throw new Error('[clientId] is required'); } if (config.tenantId == null) { - return '[tenantId] is required'; + throw new Error('[tenantId] is required'); } } else if (CUSTOM_HOST_PORT_SERVICES.indexOf(config.service) >= 0) { // If configured `service` requires custom host/port/secure settings, validate that they are set if (config.host == null && config.port == null) { - return '[host]/[port] is required'; + throw new Error('[host]/[port] is required'); } if (config.host == null) { - return '[host] is required'; + throw new Error('[host] is required'); } if (config.port == null) { - return '[port] is required'; + throw new Error('[port] is required'); } if (!configurationUtilities.isHostnameAllowed(config.host)) { - return `[host] value '${config.host}' is not in the allowedHosts configuration`; + throw new Error(`[host] value '${config.host}' is not in the allowedHosts configuration`); } } else { // Check configured `service` against nodemailer list of well known services + any custom ones allowed by Kibana const host = getServiceNameHost(config.service); if (host == null) { - return `[service] value '${config.service}' is not valid`; + throw new Error(`[service] value '${config.service}' is not valid`); } if (!configurationUtilities.isHostnameAllowed(host)) { - return `[service] value '${config.service}' resolves to host '${host}' which is not in the allowedHosts configuration`; + throw new Error( + `[service] value '${config.service}' resolves to host '${host}' which is not in the allowedHosts configuration` + ); } } } @@ -166,10 +165,7 @@ const ParamsSchemaProps = { const ParamsSchema = schema.object(ParamsSchemaProps); -function validateParams( - paramsObject: unknown, - validatorServices: ValidatorServices -): string | void { +function validateParams(paramsObject: unknown, validatorServices: ValidatorServices) { const { configurationUtilities } = validatorServices; // avoids circular reference ... @@ -179,7 +175,7 @@ function validateParams( const addrs = to.length + cc.length + bcc.length; if (addrs === 0) { - return 'no [to], [cc], or [bcc] entries'; + throw new Error('no [to], [cc], or [bcc] entries'); } const emails = withoutMustacheTemplate(to.concat(cc).concat(bcc)); @@ -187,7 +183,7 @@ function validateParams( treatMustacheTemplatesAsValid: true, }); if (invalidEmailsMessage) { - return `[to/cc/bcc]: ${invalidEmailsMessage}`; + throw new Error(`[to/cc/bcc]: ${invalidEmailsMessage}`); } } diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts index a5a6ff80f586e..4195920703248 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/validators.ts @@ -22,7 +22,7 @@ export const validateCommonConfig = ( try { configurationUtilities.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { - return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); + throw new Error(i18n.ALLOWED_HOSTS_ERROR(allowedListError.message)); } }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts index 486bcdff9f1f9..da30adf2b3672 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts @@ -181,12 +181,14 @@ function validateActionTypeConfig( try { configurationUtilities.ensureUriAllowed(getPagerDutyApiUrl(configObject)); } catch (allowListError) { - return i18n.translate('xpack.actions.builtin.pagerduty.pagerdutyConfigurationError', { - defaultMessage: 'error configuring pagerduty action: {message}', - values: { - message: allowListError.message, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.pagerduty.pagerdutyConfigurationError', { + defaultMessage: 'error configuring pagerduty action: {message}', + values: { + message: allowListError.message, + }, + }) + ); } } diff --git a/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts index abf244c31177b..5d70aea1660a1 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/resilient/validators.ts @@ -22,7 +22,7 @@ export const validateCommonConfig = ( try { configurationUtilities.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { - return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); + throw new Error(i18n.ALLOWED_HOSTS_ERROR(allowedListError.message)); } }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts index ba8ac5befdfd7..8b2f6b26e1a5a 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.test.ts @@ -12,7 +12,7 @@ const configurationUtilities = actionsConfigMock.create(); describe('validateCommonConfig', () => { test('config validation fails when apiUrl is not allowed', () => { - expect( + expect(() => validateCommonConfig( { apiUrl: 'example.com/do-something', @@ -31,11 +31,13 @@ describe('validateCommonConfig', () => { }, } ) - ).toEqual(`error configuring connector action: target url is not present in allowedHosts`); + ).toThrowErrorMatchingInlineSnapshot( + `"error configuring connector action: target url is not present in allowedHosts"` + ); }); describe('when isOAuth = true', () => { test('config validation fails when userIdentifierValue is null', () => { - expect( + expect(() => validateCommonConfig( { apiUrl: 'https://url', @@ -47,10 +49,12 @@ describe('validateCommonConfig', () => { }, { configurationUtilities } ) - ).toEqual(`userIdentifierValue must be provided when isOAuth = true`); + ).toThrowErrorMatchingInlineSnapshot( + `"userIdentifierValue must be provided when isOAuth = true"` + ); }); test('config validation fails when clientId is null', () => { - expect( + expect(() => validateCommonConfig( { apiUrl: 'https://url', @@ -62,10 +66,10 @@ describe('validateCommonConfig', () => { }, { configurationUtilities } ) - ).toEqual(`clientId must be provided when isOAuth = true`); + ).toThrowErrorMatchingInlineSnapshot(`"clientId must be provided when isOAuth = true"`); }); test('config validation fails when jwtKeyId is null', () => { - expect( + expect(() => validateCommonConfig( { apiUrl: 'https://url', @@ -77,7 +81,7 @@ describe('validateCommonConfig', () => { }, { configurationUtilities } ) - ).toEqual(`jwtKeyId must be provided when isOAuth = true`); + ).toThrowErrorMatchingInlineSnapshot(`"jwtKeyId must be provided when isOAuth = true"`); }); }); @@ -152,7 +156,7 @@ describe('validateCommonConfig', () => { describe('validateCommonSecrets', () => { test('secrets validation fails when no credentials are defined', () => { - expect( + expect(() => validateCommonSecrets( { password: null, @@ -163,11 +167,13 @@ describe('validateCommonSecrets', () => { }, { configurationUtilities } ) - ).toEqual(`Either basic auth or OAuth credentials must be specified`); + ).toThrowErrorMatchingInlineSnapshot( + `"Either basic auth or OAuth credentials must be specified"` + ); }); test('secrets validation fails when username is defined and password is not', () => { - expect( + expect(() => validateCommonSecrets( { password: null, @@ -178,11 +184,11 @@ describe('validateCommonSecrets', () => { }, { configurationUtilities } ) - ).toEqual(`username and password must both be specified`); + ).toThrowErrorMatchingInlineSnapshot(`"username and password must both be specified"`); }); test('secrets validation fails when password is defined and username is not', () => { - expect( + expect(() => validateCommonSecrets( { password: 'password', @@ -193,11 +199,11 @@ describe('validateCommonSecrets', () => { }, { configurationUtilities } ) - ).toEqual(`username and password must both be specified`); + ).toThrowErrorMatchingInlineSnapshot(`"username and password must both be specified"`); }); test('secrets validation fails when clientSecret is defined and privateKey is not', () => { - expect( + expect(() => validateCommonSecrets( { password: null, @@ -208,11 +214,11 @@ describe('validateCommonSecrets', () => { }, { configurationUtilities } ) - ).toEqual(`clientSecret and privateKey must both be specified`); + ).toThrowErrorMatchingInlineSnapshot(`"clientSecret and privateKey must both be specified"`); }); test('secrets validation fails when privateKey is defined and clientSecret is not', () => { - expect( + expect(() => validateCommonSecrets( { password: null, @@ -223,7 +229,7 @@ describe('validateCommonSecrets', () => { }, { configurationUtilities } ) - ).toEqual(`clientSecret and privateKey must both be specified`); + ).toThrowErrorMatchingInlineSnapshot(`"clientSecret and privateKey must both be specified"`); }); }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts index 7c9964c1117ee..6978a5335f8b0 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/servicenow/validators.ts @@ -23,20 +23,20 @@ export const validateCommonConfig = ( try { configurationUtilities.ensureUriAllowed(apiUrl); } catch (allowedListError) { - return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); + throw new Error(i18n.ALLOWED_HOSTS_ERROR(allowedListError.message)); } if (isOAuth) { if (userIdentifierValue == null) { - return i18n.VALIDATE_OAUTH_MISSING_FIELD_ERROR('userIdentifierValue', true); + throw new Error(i18n.VALIDATE_OAUTH_MISSING_FIELD_ERROR('userIdentifierValue', true)); } if (clientId == null) { - return i18n.VALIDATE_OAUTH_MISSING_FIELD_ERROR('clientId', true); + throw new Error(i18n.VALIDATE_OAUTH_MISSING_FIELD_ERROR('clientId', true)); } if (jwtKeyId == null) { - return i18n.VALIDATE_OAUTH_MISSING_FIELD_ERROR('jwtKeyId', true); + throw new Error(i18n.VALIDATE_OAUTH_MISSING_FIELD_ERROR('jwtKeyId', true)); } } }; @@ -48,18 +48,18 @@ export const validateCommonSecrets = ( const { username, password, clientSecret, privateKey } = secrets; if (!username && !password && !clientSecret && !privateKey) { - return i18n.CREDENTIALS_ERROR; + throw new Error(i18n.CREDENTIALS_ERROR); } if (username || password) { // Username and password must be set and set together if (!username || !password) { - return i18n.BASIC_AUTH_CREDENTIALS_ERROR; + throw new Error(i18n.BASIC_AUTH_CREDENTIALS_ERROR); } } else if (clientSecret || privateKey) { // Client secret and private key must be set and set together if (!clientSecret || !privateKey) { - return i18n.OAUTH_CREDENTIALS_ERROR; + throw new Error(i18n.OAUTH_CREDENTIALS_ERROR); } } }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/slack.ts b/x-pack/plugins/actions/server/builtin_action_types/slack.ts index f6d9edb0b2456..48f1fb5fc2a82 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.ts @@ -113,20 +113,24 @@ function validateActionTypeConfig( try { new URL(configuredUrl); } catch (err) { - return i18n.translate('xpack.actions.builtin.slack.slackConfigurationErrorNoHostname', { - defaultMessage: 'error configuring slack action: unable to parse host name from webhookUrl', - }); + throw new Error( + i18n.translate('xpack.actions.builtin.slack.slackConfigurationErrorNoHostname', { + defaultMessage: 'error configuring slack action: unable to parse host name from webhookUrl', + }) + ); } try { configurationUtilities.ensureUriAllowed(configuredUrl); } catch (allowListError) { - return i18n.translate('xpack.actions.builtin.slack.slackConfigurationError', { - defaultMessage: 'error configuring slack action: {message}', - values: { - message: allowListError.message, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.slack.slackConfigurationError', { + defaultMessage: 'error configuring slack action: {message}', + values: { + message: allowListError.message, + }, + }) + ); } } diff --git a/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts b/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts index ae29e75ffdfae..fc1d0a95d0d85 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/swimlane/validators.ts @@ -21,7 +21,7 @@ export const validateCommonConfig = ( try { configurationUtilities.ensureUriAllowed(configObject.apiUrl); } catch (allowedListError) { - return i18n.ALLOWED_HOSTS_ERROR(allowedListError.message); + throw new Error(i18n.ALLOWED_HOSTS_ERROR(allowedListError.message)); } }; diff --git a/x-pack/plugins/actions/server/builtin_action_types/teams.ts b/x-pack/plugins/actions/server/builtin_action_types/teams.ts index bf4b608eed36e..53bb8ca9066a5 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/teams.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/teams.ts @@ -95,20 +95,24 @@ function validateActionTypeConfig( try { new URL(configuredUrl); } catch (err) { - return i18n.translate('xpack.actions.builtin.teams.teamsConfigurationErrorNoHostname', { - defaultMessage: 'error configuring teams action: unable to parse host name from webhookUrl', - }); + throw new Error( + i18n.translate('xpack.actions.builtin.teams.teamsConfigurationErrorNoHostname', { + defaultMessage: 'error configuring teams action: unable to parse host name from webhookUrl', + }) + ); } try { configurationUtilities.ensureUriAllowed(configuredUrl); } catch (allowListError) { - return i18n.translate('xpack.actions.builtin.teams.teamsConfigurationError', { - defaultMessage: 'error configuring teams action: {message}', - values: { - message: allowListError.message, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.teams.teamsConfigurationError', { + defaultMessage: 'error configuring teams action: {message}', + values: { + message: allowListError.message, + }, + }) + ); } } diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts index 0040b485731c5..fe491470f1765 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts @@ -139,23 +139,27 @@ function validateActionTypeConfig( try { new URL(configuredUrl); } catch (err) { - return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationErrorNoHostname', { - defaultMessage: 'error configuring webhook action: unable to parse url: {err}', - values: { - err, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationErrorNoHostname', { + defaultMessage: 'error configuring webhook action: unable to parse url: {err}', + values: { + err, + }, + }) + ); } try { configurationUtilities.ensureUriAllowed(configuredUrl); } catch (allowListError) { - return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { - defaultMessage: 'error configuring webhook action: {message}', - values: { - message: allowListError.message, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { + defaultMessage: 'error configuring webhook action: {message}', + values: { + message: allowListError.message, + }, + }) + ); } } diff --git a/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts b/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts index c42e349f4cc60..8bd6350ada633 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/xmatters.ts @@ -96,7 +96,7 @@ export function getActionType({ function validateActionTypeConfig( configObject: ActionTypeConfigType, validatorServices: ValidatorServices -): string | undefined { +) { const { configurationUtilities } = validatorServices; const configuredUrl = configObject.configUrl; const usesBasic = configObject.usesBasic; @@ -106,12 +106,14 @@ function validateActionTypeConfig( new URL(configuredUrl); } } catch (err) { - return i18n.translate('xpack.actions.builtin.xmatters.xmattersConfigurationErrorNoHostname', { - defaultMessage: 'Error configuring xMatters action: unable to parse url: {err}', - values: { - err, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.xmattersConfigurationErrorNoHostname', { + defaultMessage: 'Error configuring xMatters action: unable to parse url: {err}', + values: { + err, + }, + }) + ); } try { @@ -119,12 +121,14 @@ function validateActionTypeConfig( configurationUtilities.ensureUriAllowed(configuredUrl); } } catch (allowListError) { - return i18n.translate('xpack.actions.builtin.xmatters.xmattersConfigurationError', { - defaultMessage: 'Error configuring xMatters action: {message}', - values: { - message: allowListError.message, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.xmattersConfigurationError', { + defaultMessage: 'Error configuring xMatters action: {message}', + values: { + message: allowListError.message, + }, + }) + ); } } @@ -179,22 +183,26 @@ function validateConnector( function validateActionTypeSecrets( secretsObject: ActionTypeSecretsType, validatorServices: ValidatorServices -): string | undefined { +) { const { configurationUtilities } = validatorServices; if (!secretsObject.secretsUrl && !secretsObject.user && !secretsObject.password) { - return i18n.translate('xpack.actions.builtin.xmatters.noSecretsProvided', { - defaultMessage: 'Provide either secretsUrl link or user/password to authenticate', - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.noSecretsProvided', { + defaultMessage: 'Provide either secretsUrl link or user/password to authenticate', + }) + ); } // Check for secrets URL first if (secretsObject.secretsUrl) { // Neither user/password should be defined if secretsUrl is specified if (secretsObject.user || secretsObject.password) { - return i18n.translate('xpack.actions.builtin.xmatters.noUserPassWhenSecretsUrl', { - defaultMessage: - 'Cannot use user/password for URL authentication. Provide valid secretsUrl or use Basic Authentication.', - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.noUserPassWhenSecretsUrl', { + defaultMessage: + 'Cannot use user/password for URL authentication. Provide valid secretsUrl or use Basic Authentication.', + }) + ); } // Test that URL is valid @@ -203,12 +211,14 @@ function validateActionTypeSecrets( new URL(secretsObject.secretsUrl); } } catch (err) { - return i18n.translate('xpack.actions.builtin.xmatters.xmattersInvalidUrlError', { - defaultMessage: 'Invalid secretsUrl: {err}', - values: { - err, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.xmattersInvalidUrlError', { + defaultMessage: 'Invalid secretsUrl: {err}', + values: { + err, + }, + }) + ); } // Test that hostname is allowed @@ -217,19 +227,23 @@ function validateActionTypeSecrets( configurationUtilities.ensureUriAllowed(secretsObject.secretsUrl); } } catch (allowListError) { - return i18n.translate('xpack.actions.builtin.xmatters.xmattersHostnameNotAllowed', { - defaultMessage: '{message}', - values: { - message: allowListError.message, - }, - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.xmattersHostnameNotAllowed', { + defaultMessage: '{message}', + values: { + message: allowListError.message, + }, + }) + ); } } else { // Username and password must both be set if (!secretsObject.user || !secretsObject.password) { - return i18n.translate('xpack.actions.builtin.xmatters.invalidUsernamePassword', { - defaultMessage: 'Both user and password must be specified.', - }); + throw new Error( + i18n.translate('xpack.actions.builtin.xmatters.invalidUsernamePassword', { + defaultMessage: 'Both user and password must be specified.', + }) + ); } } } 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 d96e0c86ba32e..55674ee3efba7 100644 --- a/x-pack/plugins/actions/server/lib/validate_with_schema.ts +++ b/x-pack/plugins/actions/server/lib/validate_with_schema.ts @@ -101,44 +101,44 @@ function validateWithSchema< case 'params': name = 'action params'; if (actionType.validate.params) { - const params = actionType.validate.params.schema.validate(value); + const validatedValue = actionType.validate.params.schema.validate(value); + if (actionType.validate.params.customValidator) { - const result = actionType.validate.params.customValidator(params, validatorServices); - if (result) { - throw new Error(result); - } + actionType.validate.params.customValidator( + validatedValue as Params, + validatorServices + ); } - return params; + return validatedValue; } break; case 'config': name = 'action type config'; if (actionType.validate.config) { - const config = actionType.validate.config.schema.validate(value); + const validatedValue = actionType.validate.config.schema.validate(value); + if (actionType.validate.config.customValidator) { - const result = actionType.validate.config.customValidator(config, validatorServices); - if (result) { - throw new Error(result); - } + actionType.validate.config.customValidator( + validatedValue as Config, + validatorServices + ); } - return config; + return validatedValue; } break; case 'secrets': name = 'action type secrets'; if (actionType.validate.secrets) { - const secrets = actionType.validate.secrets.schema.validate(value); + const validatedValue = actionType.validate.secrets.schema.validate(value); + if (actionType.validate.secrets.customValidator) { - const result = actionType.validate.secrets.customValidator( - secrets, + actionType.validate.secrets.customValidator( + validatedValue as Secrets, validatorServices ); - if (result) { - throw new Error(result); - } } - return secrets; + return validatedValue; } break; default: diff --git a/x-pack/plugins/actions/server/types.ts b/x-pack/plugins/actions/server/types.ts index 684029a85a206..d809228e4eeda 100644 --- a/x-pack/plugins/actions/server/types.ts +++ b/x-pack/plugins/actions/server/types.ts @@ -98,7 +98,7 @@ interface ValidatorType { schema: { validate(value: unknown): Type; }; - customValidator?: (value: Type, validatorServices: ValidatorServices) => string | void; + customValidator?: (value: Type, validatorServices: ValidatorServices) => void; } export interface ValidatorServices { From a336be2f741b6cddbf18ab9b8a47425e518a4e3d Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Mon, 29 Aug 2022 13:02:03 -0400 Subject: [PATCH 7/7] Adding new tests --- .../server/lib/validate_with_schema.test.ts | 100 +++++++++++++++++- 1 file changed, 99 insertions(+), 1 deletion(-) 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 172baf82ca6b9..24eb9a57a32f3 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 @@ -13,7 +13,14 @@ import { validateSecrets, validateConnector, } from './validate_with_schema'; -import { ActionType, ExecutorType } from '../types'; +import { + ActionType, + ActionTypeConfig, + ActionTypeParams, + ActionTypeSecrets, + ExecutorType, + ValidatorServices, +} from '../types'; import { actionsConfigMock } from '../actions_config.mock'; const executor: ExecutorType<{}, {}, {}, void> = async (options) => { @@ -216,6 +223,97 @@ test('should work with @kbn/config-schema', () => { ); }); +test('should validate when custom validator is defined', () => { + const schemaValidator = { + validate: (value: ActionTypeParams | ActionTypeConfig | ActionTypeSecrets) => value, + }; + const customValidator = jest.fn(); + + const actionType: ActionType = { + id: 'foo', + name: 'bar', + minimumLicenseRequired: 'basic', + supportedFeatureIds: ['alerting'], + executor, + validate: { + params: { + schema: schemaValidator, + customValidator, + }, + config: { + schema: schemaValidator, + customValidator, + }, + secrets: { + schema: schemaValidator, + customValidator, + }, + }, + }; + + let result; + const testValue = { any: ['old', 'thing'] }; + + result = validateParams(actionType, testValue, { configurationUtilities }); + expect(result).toEqual(testValue); + + result = validateConfig(actionType, testValue, { configurationUtilities }); + expect(result).toEqual(testValue); + + result = validateSecrets(actionType, testValue, { configurationUtilities }); + expect(result).toEqual(testValue); + + expect(customValidator).toBeCalledTimes(3); +}); + +test('should throw an error when custom validators fail', () => { + const schemaValidator = { + validate: (value: ActionTypeParams | ActionTypeConfig | ActionTypeSecrets) => value, + }; + const customValidator = ( + value: ActionTypeParams | ActionTypeConfig | ActionTypeSecrets, + services: ValidatorServices + ) => { + throw new Error('test error'); + }; + + const actionType: ActionType = { + id: 'foo', + name: 'bar', + minimumLicenseRequired: 'basic', + supportedFeatureIds: ['alerting'], + executor, + validate: { + params: { + schema: schemaValidator, + customValidator, + }, + config: { + schema: schemaValidator, + customValidator, + }, + secrets: { + schema: schemaValidator, + customValidator, + }, + }, + }; + + const testValue = { any: ['old', 'thing'] }; + + expect(() => + validateParams(actionType, testValue, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action params: test error"`); + + expect(() => + validateConfig(actionType, testValue, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action type config: test error"`); + + expect(() => + validateSecrets(actionType, testValue, { configurationUtilities }) + ).toThrowErrorMatchingInlineSnapshot(`"error validating action type secrets: test error"`); +}); + describe('validateConnectors', () => { const testValue = { any: ['old', 'thing'] }; const selfValidator = { validate: (value: Record) => value };