[Actions] Extended ActionTypeRegistry with connector validation to va…

…lidate config with secrets (#116079)

* [Actions] Extended ActionTypeRegistry with connector validation to validate config with secrets

* fixed typecheck

* added tests

* more tests

* email validation fix

* fixed typecheck

* fixed typecheck

* added tests

* fixed typecheck

* fixed test

* fixed due to comments

* fixed test

* fixed test

* fixed typecheck

x-pack/plugins/actions/server/actions_client.test.ts

@@ -353,6 +353,36 @@ describe('create()', () => {
     );
   });
 
+  test('validates connector: config and secrets', async () => {
+    const connectorValidator = ({}, secrets: { param1: '1' }) => {
+      if (secrets.param1 == null) {
+        return '[param1] is required';
+      }
+      return null;
+    };
+    actionTypeRegistry.register({
+      id: 'my-action-type',
+      name: 'My action type',
+      minimumLicenseRequired: 'basic',
+      validate: {
+        connector: connectorValidator,
+      },
+      executor,
+    });
+    await expect(
+      actionsClient.create({
+        action: {
+          name: 'my name',
+          actionTypeId: 'my-action-type',
+          config: {},
+          secrets: {},
+        },
+      })
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type connector: [param1] is required"`
+    );
+  });
+
   test(`throws an error when an action type doesn't exist`, async () => {
     await expect(
       actionsClient.create({
@@ -1539,6 +1569,40 @@ describe('update()', () => {
     );
   });
 
+  test('validates connector: config and secrets', async () => {
+    actionTypeRegistry.register({
+      id: 'my-action-type',
+      name: 'My action type',
+      minimumLicenseRequired: 'basic',
+      validate: {
+        connector: () => {
+          return '[param1] is required';
+        },
+      },
+      executor,
+    });
+    unsecuredSavedObjectsClient.get.mockResolvedValueOnce({
+      id: 'my-action',
+      type: 'action',
+      attributes: {
+        actionTypeId: 'my-action-type',
+      },
+      references: [],
+    });
+    await expect(
+      actionsClient.update({
+        id: 'my-action',
+        action: {
+          name: 'my name',
+          config: {},
+          secrets: {},
+        },
+      })
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type connector: [param1] is required"`
+    );
+  });
+
   test('encrypts action type options unless specified not to', async () => {
     actionTypeRegistry.register({
       id: 'my-action-type',

x-pack/plugins/actions/server/actions_client.ts

@@ -22,7 +22,7 @@ import {
 import { AuditLogger } from '../../security/server';
 import { ActionType } from '../common';
 import { ActionTypeRegistry } from './action_type_registry';
-import { validateConfig, validateSecrets, ActionExecutorContract } from './lib';
+import { validateConfig, validateSecrets, ActionExecutorContract, validateConnector } from './lib';
 import {
   ActionResult,
   FindActionResult,
@@ -150,7 +150,9 @@ export class ActionsClient {
     const actionType = this.actionTypeRegistry.get(actionTypeId);
     const validatedActionTypeConfig = validateConfig(actionType, config);
     const validatedActionTypeSecrets = validateSecrets(actionType, secrets);
-
+    if (actionType.validate?.connector) {
+      validateConnector(actionType, { config, secrets });
+    }
     this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
 
     this.auditLogger?.log(
@@ -221,6 +223,9 @@ export class ActionsClient {
     const actionType = this.actionTypeRegistry.get(actionTypeId);
     const validatedActionTypeConfig = validateConfig(actionType, config);
     const validatedActionTypeSecrets = validateSecrets(actionType, secrets);
+    if (actionType.validate?.connector) {
+      validateConnector(actionType, { config, secrets });
+    }
 
     this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
 

x-pack/plugins/actions/server/builtin_action_types/email.test.ts

@@ -12,7 +12,7 @@ jest.mock('./lib/send_email', () => ({
 import { Logger } from '../../../../../src/core/server';
 
 import { actionsConfigMock } from '../actions_config.mock';
-import { validateConfig, validateSecrets, validateParams } from '../lib';
+import { validateConfig, validateConnector, validateParams, validateSecrets } from '../lib';
 import { createActionTypeRegistry } from './index.test';
 import { sendEmail } from './lib/send_email';
 import { actionsMock } from '../mocks';
@@ -303,6 +303,75 @@ describe('secrets validation', () => {
   });
 });
 
+describe('connector validation: secrets with config', () => {
+  test('connector validation succeeds when username/password was populated for hasAuth true', () => {
+    const secrets: Record<string, unknown> = {
+      user: 'bob',
+      password: 'supersecret',
+    };
+    const config: Record<string, unknown> = {
+      hasAuth: true,
+    };
+    expect(validateConnector(actionType, { config, secrets })).toBeNull();
+  });
+
+  test('connector validation succeeds when username/password not filled for hasAuth false', () => {
+    const secrets: Record<string, unknown> = {
+      user: null,
+      password: null,
+      clientSecret: null,
+    };
+    const config: Record<string, unknown> = {
+      hasAuth: false,
+    };
+    expect(validateConnector(actionType, { config, secrets })).toBeNull();
+    expect(validateConnector(actionType, { config, secrets: {} })).toBeNull();
+    expect(validateConnector(actionType, { config, secrets: { user: null } })).toBeNull();
+    expect(validateConnector(actionType, { config, secrets: { password: null } })).toBeNull();
+  });
+
+  test('connector validation fails when username/password was populated for hasAuth true', () => {
+    const secrets: Record<string, unknown> = {
+      password: null,
+      user: null,
+    };
+    const config: Record<string, unknown> = {
+      hasAuth: true,
+    };
+    // invalid user
+    expect(() => {
+      validateConnector(actionType, { config, secrets });
+    }).toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type connector: [user] is required"`
+    );
+  });
+
+  test('connector validation succeeds when service is exchange_server and clientSecret is populated', () => {
+    const secrets: Record<string, unknown> = {
+      clientSecret: '12345678',
+    };
+    const config: Record<string, unknown> = {
+      service: 'exchange_server',
+    };
+    expect(validateConnector(actionType, { config, secrets })).toBeNull();
+  });
+
+  test('connector validation fails when service is exchange_server and clientSecret is not populated', () => {
+    const secrets: Record<string, unknown> = {
+      clientSecret: null,
+    };
+    const config: Record<string, unknown> = {
+      service: 'exchange_server',
+    };
+    // invalid user
+    expect(() => {
+      validateConnector(actionType, { config, secrets });
+    }).toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type connector: [clientSecret] is required"`
+    );
+  });
+});
+
 describe('params validation', () => {
   test('params validation succeeds when params is valid', () => {
     const params: Record<string, unknown> = {

x-pack/plugins/actions/server/builtin_action_types/email.ts

@@ -116,11 +116,13 @@ function validateConfig(
 
 export type ActionTypeSecretsType = TypeOf<typeof SecretsSchema>;
 
-const SecretsSchema = schema.object({
+const SecretsSchemaProps = {
   user: schema.nullable(schema.string()),
   password: schema.nullable(schema.string()),
   clientSecret: schema.nullable(schema.string()),
-});
+};
+
+const SecretsSchema = schema.object(SecretsSchemaProps);
 
 // params definition
 
@@ -167,6 +169,25 @@ interface GetActionTypeParams {
   configurationUtilities: ActionsConfigurationUtilities;
 }
 
+function validateConnector(
+  config: ActionTypeConfigType,
+  secrets: ActionTypeSecretsType
+): string | null {
+  if (config.service === AdditionalEmailServices.EXCHANGE) {
+    if (secrets.clientSecret == null) {
+      return '[clientSecret] is required';
+    }
+  } else if (config.hasAuth && (secrets.password == null || secrets.user == null)) {
+    if (secrets.user == null) {
+      return '[user] is required';
+    }
+    if (secrets.password == null) {
+      return '[password] is required';
+    }
+  }
+  return null;
+}
+
 // action type definition
 export const ActionTypeId = '.email';
 export function getActionType(params: GetActionTypeParams): EmailActionType {
@@ -183,6 +204,7 @@ export function getActionType(params: GetActionTypeParams): EmailActionType {
       }),
       secrets: SecretsSchema,
       params: ParamsSchema,
+      connector: validateConnector,
     },
     renderParameterTemplates,
     executor: curry(executor)({ logger, publicBaseUrl, configurationUtilities }),

x-pack/plugins/actions/server/lib/action_executor.test.ts

@@ -273,6 +273,45 @@ test('throws an error when config is invalid', async () => {
   });
 });
 
+test('throws an error when connector is invalid', async () => {
+  const actionType: jest.Mocked<ActionType> = {
+    id: 'test',
+    name: 'Test',
+    minimumLicenseRequired: 'basic',
+    validate: {
+      connector: () => {
+        return 'error';
+      },
+    },
+    executor: jest.fn(),
+  };
+  const actionSavedObject = {
+    id: '1',
+    type: 'action',
+    attributes: {
+      actionTypeId: 'test',
+    },
+    references: [],
+  };
+  const actionResult = {
+    id: actionSavedObject.id,
+    name: actionSavedObject.id,
+    actionTypeId: actionSavedObject.attributes.actionTypeId,
+    isPreconfigured: false,
+  };
+  actionsClient.get.mockResolvedValueOnce(actionResult);
+  encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(actionSavedObject);
+  actionTypeRegistry.get.mockReturnValueOnce(actionType);
+
+  const result = await actionExecutor.execute(executeParams);
+  expect(result).toEqual({
+    actionId: '1',
+    status: 'error',
+    retry: false,
+    message: `error validating action type connector: error`,
+  });
+});
+
 test('throws an error when params is invalid', async () => {
   const actionType: jest.Mocked<ActionType> = {
     id: 'test',

x-pack/plugins/actions/server/lib/action_executor.ts

@@ -9,7 +9,12 @@ import type { PublicMethodsOf } from '@kbn/utility-types';
 import { Logger, KibanaRequest } from 'src/core/server';
 import { cloneDeep } from 'lodash';
 import { withSpan } from '@kbn/apm-utils';
-import { validateParams, validateConfig, validateSecrets } from './validate_with_schema';
+import {
+  validateParams,
+  validateConfig,
+  validateSecrets,
+  validateConnector,
+} from './validate_with_schema';
 import {
   ActionTypeExecutorResult,
   ActionTypeRegistryContract,
@@ -142,11 +147,16 @@ export class ActionExecutor {
         let validatedParams: Record<string, unknown>;
         let validatedConfig: Record<string, unknown>;
         let validatedSecrets: Record<string, unknown>;
-
         try {
           validatedParams = validateParams(actionType, params);
           validatedConfig = validateConfig(actionType, config);
           validatedSecrets = validateSecrets(actionType, secrets);
+          if (actionType.validate?.connector) {
+            validateConnector(actionType, {
+              config,
+              secrets,
+            });
+          }
         } catch (err) {
           span?.setOutcome('failure');
           return { status: 'error', actionId, message: err.message, retry: false };

x-pack/plugins/actions/server/lib/index.ts

@@ -6,7 +6,12 @@
  */
 
 export { ExecutorError } from './executor_error';
-export { validateParams, validateConfig, validateSecrets } from './validate_with_schema';
+export {
+  validateParams,
+  validateConfig,
+  validateSecrets,
+  validateConnector,
+} from './validate_with_schema';
 export { TaskRunnerFactory } from './task_runner_factory';
 export { ActionExecutor, ActionExecutorContract } from './action_executor';
 export { ILicenseState, LicenseState } from './license_state';