Skip to content

Commit

Permalink
[Alerting] changes preconfigured actions config from array to object
Browse files Browse the repository at this point in the history 
resolves #63171

Previously, preconfigured actions were specified as an array of action
properties.  This ended up being problematic when using the kibana keystore
for secrets, as you'd have to reference specific actions via index.

This changes preconfigured actions to be specified as an object, where the
property key is the id, and the body is the remainder of the action properties.

As access to preconfigured actions has leaked across the code base, it's
probably time to consider changing the internal representation from an array
to a Map, to provide easier access by action id.  For a future PR.
  • Loading branch information
@pmuellr
pmuellr committed May 7, 2020
1 parent 74187fd commit f7a4801
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 73 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Built-In-Actions are configured using the _xpack.actions_ namespoace under _kiba
| _xpack.actions._**enabled** | Feature toggle which enabled Actions in Kibana. | boolean |
| _xpack.actions._**whitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" in place of a specific hostname to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | Array<String> |
| _xpack.actions._**enabledActionTypes** | A list of _actionTypes_ id's that are enabled. A "\*" may be used as an element to indicate all registered actionTypes should be enabled. The actionTypes registered for Kibana are `.server-log`, `.slack`, `.email`, `.index`, `.pagerduty`, `.webhook`. Default: `["*"]` | Array<String> |
| _xpack.actions._**preconfigured** | A list of preconfigured actions. Default: `[]` | Array<Object> |
| _xpack.actions._**preconfigured** | A object of action id / preconfigured actions. Default: `{}` | Array<Object> |

#### Whitelisting Built-in Action Types

Expand Down
53 changes: 44 additions & 9 deletions x-pack/plugins/actions/server/config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('config validation', () => {
"enabledActionTypes": Array [
"*",
],
"preconfigured": Array [],
"preconfigured": Object {},
"whitelistedHosts": Array [
"*",
],
Expand All @@ -24,38 +24,73 @@ describe('config validation', () => {

test('action with preconfigured actions', () => {
const config: Record<string, unknown> = {
preconfigured: [
{
id: 'my-slack1',
preconfigured: {
mySlack1: {
actionTypeId: '.slack',
name: 'Slack #xyz',
config: {
webhookUrl: 'https://hooks.slack.com/services/abcd/efgh/ijklmnopqrstuvwxyz',
},
},
],
},
};
expect(configSchema.validate(config)).toMatchInlineSnapshot(`
Object {
"enabled": true,
"enabledActionTypes": Array [
"*",
],
"preconfigured": Array [
Object {
"preconfigured": Object {
"mySlack1": Object {
"actionTypeId": ".slack",
"config": Object {
"webhookUrl": "https://hooks.slack.com/services/abcd/efgh/ijklmnopqrstuvwxyz",
},
"id": "my-slack1",
"name": "Slack #xyz",
"secrets": Object {},
},
],
},
"whitelistedHosts": Array [
"*",
],
}
`);
});

test('validates preconfigured action ids', () => {
expect(() =>
configSchema.validate(preConfiguredActionConfig(''))
).toThrowErrorMatchingInlineSnapshot(
`"[preconfigured]: invalid preconfigured action id \\"\\""`
);

expect(() =>
configSchema.validate(preConfiguredActionConfig('constructor'))
).toThrowErrorMatchingInlineSnapshot(
`"[preconfigured]: invalid preconfigured action id \\"constructor\\""`
);

expect(() =>
configSchema.validate(preConfiguredActionConfig('__proto__'))
).toThrowErrorMatchingInlineSnapshot(
`"[preconfigured]: invalid preconfigured action id \\"__proto__\\""`
);
});
});

// object creator that ensures we can create a property named __proto__ on an
// object, via JSON.parse()
function preConfiguredActionConfig(id: string) {
return JSON.parse(`{
"preconfigured": {
${JSON.stringify(id)}: {
"actionTypeId": ".server-log",
"name": "server log 1"
},
"serverLog": {
"actionTypeId": ".server-log",
"name": "server log 2"
}
}
}`);
}
39 changes: 27 additions & 12 deletions x-pack/plugins/actions/server/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
import { schema, TypeOf } from '@kbn/config-schema';
import { WhitelistedHosts, EnabledActionTypes } from './actions_config';

const preconfiguredActionSchema = schema.object({
name: schema.string({ minLength: 1 }),
actionTypeId: schema.string({ minLength: 1 }),
config: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
secrets: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
});

export const configSchema = schema.object({
enabled: schema.boolean({ defaultValue: true }),
whitelistedHosts: schema.arrayOf(
Expand All @@ -21,18 +28,26 @@ export const configSchema = schema.object({
defaultValue: [WhitelistedHosts.Any],
}
),
preconfigured: schema.arrayOf(
schema.object({
id: schema.string({ minLength: 1 }),
name: schema.string(),
actionTypeId: schema.string({ minLength: 1 }),
config: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
secrets: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
}),
{
defaultValue: [],
}
),
preconfigured: schema.recordOf(schema.string(), preconfiguredActionSchema, {
defaultValue: {},
validate: validatePreconfigured,
}),
});

export type ActionsConfig = TypeOf<typeof configSchema>;

const invalidActionIds = new Set(['', '__proto__', 'constructor']);

function validatePreconfigured(preconfigured: Record<string, unknown>): string | undefined {
// check for ids that should not be used
for (const id of Object.keys(preconfigured)) {
if (invalidActionIds.has(id)) {
return `invalid preconfigured action id "${id}"`;
}
}

// in case __proto__ was used as a preconfigured action id ...
if (Object.getPrototypeOf(preconfigured) !== Object.getPrototypeOf({})) {
return `invalid preconfigured action id "__proto__"`;
}
}
57 changes: 28 additions & 29 deletions x-pack/plugins/actions/server/plugin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { taskManagerMock } from '../../task_manager/server/mocks';
import { eventLogMock } from '../../event_log/server/mocks';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { ActionType } from './types';
import { ActionsConfig } from './config';
import {
ActionsPlugin,
ActionsPluginsSetup,
Expand All @@ -31,33 +32,11 @@ describe('Actions Plugin', () => {
let pluginsSetup: jest.Mocked<ActionsPluginsSetup>;

beforeEach(() => {
context = coreMock.createPluginInitializerContext({
preconfigured: [
{
id: 'my-slack1',
actionTypeId: '.slack',
name: 'Slack #xyz',
description: 'Send a message to the #xyz channel',
config: {
webhookUrl: 'https://hooks.slack.com/services/abcd/efgh/ijklmnopqrstuvwxyz',
},
},
{
id: 'custom-system-abc-connector',
actionTypeId: 'system-abc-action-type',
description: 'Send a notification to system ABC',
name: 'System ABC',
config: {
xyzConfig1: 'value1',
xyzConfig2: 'value2',
listOfThings: ['a', 'b', 'c', 'd'],
},
secrets: {
xyzSecret1: 'credential1',
xyzSecret2: 'credential2',
},
},
],
context = coreMock.createPluginInitializerContext<ActionsConfig>({
enabled: true,
enabledActionTypes: ['*'],
whitelistedHosts: ['*'],
preconfigured: {},
});
plugin = new ActionsPlugin(context);
coreSetup = coreMock.createSetup();
Expand Down Expand Up @@ -192,6 +171,7 @@ describe('Actions Plugin', () => {
});
});
});

describe('start()', () => {
let plugin: ActionsPlugin;
let coreSetup: ReturnType<typeof coreMock.createSetup>;
Expand All @@ -200,8 +180,18 @@ describe('Actions Plugin', () => {
let pluginsStart: jest.Mocked<ActionsPluginsStart>;

beforeEach(() => {
const context = coreMock.createPluginInitializerContext({
preconfigured: [],
const context = coreMock.createPluginInitializerContext<ActionsConfig>({
enabled: true,
enabledActionTypes: ['*'],
whitelistedHosts: ['*'],
preconfigured: {
preconfiguredServerLog: {
actionTypeId: '.server-log',
name: 'preconfigured-server-log',
config: {},
secrets: {},
},
},
});
plugin = new ActionsPlugin(context);
coreSetup = coreMock.createSetup();
Expand All @@ -220,6 +210,15 @@ describe('Actions Plugin', () => {
});

describe('getActionsClientWithRequest()', () => {
it('should handle preconfigured actions', async () => {
// coreMock.createSetup doesn't support Plugin generics
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await plugin.setup(coreSetup as any, pluginsSetup);
const pluginStart = plugin.start(coreStart, pluginsStart);

expect(pluginStart.isActionExecutable('preconfiguredServerLog', '.server-log')).toBe(true);
});

it('should not throw error when ESO plugin not using a generated key', async () => {
// coreMock.createSetup doesn't support Plugin generics
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
14 changes: 8 additions & 6 deletions x-pack/plugins/actions/server/plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,14 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
const actionsConfig = (await this.config) as ActionsConfig;
const actionsConfigUtils = getActionsConfigurationUtilities(actionsConfig);

this.preconfiguredActions.push(
...actionsConfig.preconfigured.map(
preconfiguredAction =>
({ ...preconfiguredAction, isPreconfigured: true } as PreConfiguredAction)
)
);
for (const preconfiguredId of Object.keys(actionsConfig.preconfigured)) {
this.preconfiguredActions.push({
...actionsConfig.preconfigured[preconfiguredId],
id: preconfiguredId,
isPreconfigured: true,
});
}

const actionTypeRegistry = new ActionTypeRegistry({
taskRunnerFactory,
taskManager: plugins.taskManager,
Expand Down
16 changes: 6 additions & 10 deletions x-pack/test/alerting_api_integration/common/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,15 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
])}`,
`--xpack.actions.enabledActionTypes=${JSON.stringify(enabledActionTypes)}`,
'--xpack.eventLog.logEntries=true',
`--xpack.actions.preconfigured=${JSON.stringify([
{
id: 'my-slack1',
`--xpack.actions.preconfigured=${JSON.stringify({
'my-slack1': {
actionTypeId: '.slack',
name: 'Slack#xyz',
config: {
webhookUrl: 'https://hooks.slack.com/services/abcd/efgh/ijklmnopqrstuvwxyz',
},
},
{
id: 'custom-system-abc-connector',
'custom-system-abc-connector': {
actionTypeId: 'system-abc-action-type',
name: 'SystemABC',
config: {
Expand All @@ -106,8 +104,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
xyzSecret2: 'credential2',
},
},
{
id: 'preconfigured-es-index-action',
'preconfigured-es-index-action': {
actionTypeId: '.index',
name: 'preconfigured_es_index_action',
config: {
Expand All @@ -116,8 +113,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
executionTimeField: 'timestamp',
},
},
{
id: 'preconfigured.test.index-record',
'preconfigured.test.index-record': {
actionTypeId: 'test.index-record',
name: 'Test:_Preconfigured_Index_Record',
config: {
Expand All @@ -127,7 +123,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
encrypted: 'this-is-also-ignored-and-also-required',
},
},
])}`,
})}`,
...disabledPlugins.map(key => `--xpack.${key}.enabled=false`),
...plugins.map(
pluginDir =>
Expand Down
10 changes: 4 additions & 6 deletions x-pack/test/functional_with_es_ssl/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,19 @@ export default async function({ readConfigFile }: FtrConfigProviderContext) {
`--elasticsearch.ssl.certificateAuthorities=${CA_CERT_PATH}`,
`--plugin-path=${join(__dirname, 'fixtures', 'plugins', 'alerts')}`,
`--xpack.actions.enabledActionTypes=${JSON.stringify(enabledActionTypes)}`,
`--xpack.actions.preconfigured=${JSON.stringify([
{
id: 'my-slack1',
`--xpack.actions.preconfigured=${JSON.stringify({
'my-slack1': {
actionTypeId: '.slack',
name: 'Slack#xyztest',
config: {
webhookUrl: 'https://hooks.slack.com/services/abcd/efgh/ijklmnopqrstuvwxyz',
},
},
{
id: 'my-server-log',
'my-server-log': {
actionTypeId: '.server-log',
name: 'Serverlog#xyz',
},
])}`,
})}`,
],
},
};
Expand Down

0 comments on commit f7a4801

Please sign in to comment.