Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
64 changes: 64 additions & 0 deletions x-pack/plugins/actions/server/actions_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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',
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);

Expand Down
71 changes: 70 additions & 1 deletion x-pack/plugins/actions/server/builtin_action_types/email.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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> = {
Expand Down
26 changes: 24 additions & 2 deletions x-pack/plugins/actions/server/builtin_action_types/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 {
Expand All @@ -183,6 +204,7 @@ export function getActionType(params: GetActionTypeParams): EmailActionType {
}),
secrets: SecretsSchema,
params: ParamsSchema,
connector: validateConnector,
},
renderParameterTemplates,
executor: curry(executor)({ logger, publicBaseUrl, configurationUtilities }),
Expand Down
39 changes: 39 additions & 0 deletions x-pack/plugins/actions/server/lib/action_executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
14 changes: 12 additions & 2 deletions x-pack/plugins/actions/server/lib/action_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 };
Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/actions/server/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading