Skip to content

Commit

Permalink
[Alerting] allow email action to not require auth (#60839)
Browse files Browse the repository at this point in the history
resolves #57143

Currently, the built-in email action requires user/password properties to be
set in it's secrets parameters.  This PR changes that requirement, so they
are no longer required.
  • Loading branch information
pmuellr authored Mar 23, 2020
1 parent dc31736 commit 72bc0ea
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 43 deletions.
14 changes: 8 additions & 6 deletions x-pack/plugins/actions/server/builtin_action_types/email.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,14 @@ describe('secrets validation', () => {
expect(validateSecrets(actionType, secrets)).toEqual(secrets);
});

test('secrets validation fails when secrets is not valid', () => {
expect(() => {
validateSecrets(actionType, {});
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: [user]: expected value of type [string] but got [undefined]"`
);
test('secrets validation succeeds when secrets props are null/undefined', () => {
const secrets: Record<string, any> = {
user: null,
password: null,
};
expect(validateSecrets(actionType, {})).toEqual(secrets);
expect(validateSecrets(actionType, { user: null })).toEqual(secrets);
expect(validateSecrets(actionType, { password: null })).toEqual(secrets);
});
});

Expand Down
25 changes: 14 additions & 11 deletions x-pack/plugins/actions/server/builtin_action_types/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { schema, TypeOf } from '@kbn/config-schema';
import nodemailerGetService from 'nodemailer/lib/well-known';

import { sendEmail, JSON_TRANSPORT_SERVICE } from './lib/send_email';
import { nullableType } from './lib/nullable';
import { portSchema } from './lib/schemas';
import { Logger } from '../../../../../src/core/server';
import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types';
Expand All @@ -20,10 +19,10 @@ import { ActionsConfigurationUtilities } from '../actions_config';
export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;

const ConfigSchemaProps = {
service: nullableType(schema.string()),
host: nullableType(schema.string()),
port: nullableType(portSchema()),
secure: nullableType(schema.boolean()),
service: schema.nullable(schema.string()),
host: schema.nullable(schema.string()),
port: schema.nullable(portSchema()),
secure: schema.nullable(schema.boolean()),
from: schema.string(),
};

Expand Down Expand Up @@ -75,8 +74,8 @@ function validateConfig(
export type ActionTypeSecretsType = TypeOf<typeof SecretsSchema>;

const SecretsSchema = schema.object({
user: schema.string(),
password: schema.string(),
user: schema.nullable(schema.string()),
password: schema.nullable(schema.string()),
});

// params definition
Expand Down Expand Up @@ -144,10 +143,14 @@ async function executor(
const secrets = execOptions.secrets as ActionTypeSecretsType;
const params = execOptions.params as ActionParamsType;

const transport: any = {
user: secrets.user,
password: secrets.password,
};
const transport: any = {};

if (secrets.user != null) {
transport.user = secrets.user;
}
if (secrets.password != null) {
transport.password = secrets.password;
}

if (config.service !== null) {
transport.service = config.service;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,33 @@ describe('connector validation', () => {
});
});

test('connector validation succeeds when connector config is valid with empty user/password', () => {
const actionConnector = {
secrets: {
user: null,
password: null,
},
id: 'test',
actionTypeId: '.email',
name: 'email',
config: {
from: 'test@test.com',
port: 2323,
host: 'localhost',
test: 'test',
},
} as EmailActionConnector;

expect(actionTypeModel.validateConnector(actionConnector)).toEqual({
errors: {
from: [],
port: [],
host: [],
user: [],
password: [],
},
});
});
test('connector validation fails when connector config is not valid', () => {
const actionConnector = {
secrets: {
Expand All @@ -82,6 +109,60 @@ describe('connector validation', () => {
},
});
});
test('connector validation fails when user specified but not password', () => {
const actionConnector = {
secrets: {
user: 'user',
password: null,
},
id: 'test',
actionTypeId: '.email',
name: 'email',
config: {
from: 'test@test.com',
port: 2323,
host: 'localhost',
test: 'test',
},
} as EmailActionConnector;

expect(actionTypeModel.validateConnector(actionConnector)).toEqual({
errors: {
from: [],
port: [],
host: [],
user: [],
password: ['Password is required when username is used.'],
},
});
});
test('connector validation fails when password specified but not user', () => {
const actionConnector = {
secrets: {
user: null,
password: 'password',
},
id: 'test',
actionTypeId: '.email',
name: 'email',
config: {
from: 'test@test.com',
port: 2323,
host: 'localhost',
test: 'test',
},
} as EmailActionConnector;

expect(actionTypeModel.validateConnector(actionConnector)).toEqual({
errors: {
from: [],
port: [],
host: [],
user: ['Username is required when password is used.'],
password: [],
},
});
});
});

describe('action params validation', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,22 @@ export function getActionType(): ActionTypeModel {
)
);
}
if (!action.secrets.user) {
errors.user.push(
if (action.secrets.user && !action.secrets.password) {
errors.password.push(
i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredUserText',
'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredPasswordText',
{
defaultMessage: 'Username is required.',
defaultMessage: 'Password is required when username is used.',
}
)
);
}
if (!action.secrets.password) {
errors.password.push(
if (!action.secrets.user && action.secrets.password) {
errors.user.push(
i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredPasswordText',
'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredUserText',
{
defaultMessage: 'Password is required.',
defaultMessage: 'Username is required when password is used.',
}
)
);
Expand Down Expand Up @@ -303,7 +303,7 @@ const EmailActionConnectorFields: React.FunctionComponent<ActionConnectorFieldsP
id="emailUser"
fullWidth
error={errors.user}
isInvalid={errors.user.length > 0 && user !== undefined}
isInvalid={errors.user.length > 0}
label={i18n.translate(
'xpack.triggersActionsUI.sections.builtinActionTypes.emailAction.userTextFieldLabel',
{
Expand All @@ -313,17 +313,12 @@ const EmailActionConnectorFields: React.FunctionComponent<ActionConnectorFieldsP
>
<EuiFieldText
fullWidth
isInvalid={errors.user.length > 0 && user !== undefined}
isInvalid={errors.user.length > 0}
name="user"
value={user || ''}
data-test-subj="emailUserInput"
onChange={e => {
editActionSecrets('user', e.target.value);
}}
onBlur={() => {
if (!user) {
editActionSecrets('user', '');
}
editActionSecrets('user', nullableString(e.target.value));
}}
/>
</EuiFormRow>
Expand All @@ -333,7 +328,7 @@ const EmailActionConnectorFields: React.FunctionComponent<ActionConnectorFieldsP
id="emailPassword"
fullWidth
error={errors.password}
isInvalid={errors.password.length > 0 && password !== undefined}
isInvalid={errors.password.length > 0}
label={i18n.translate(
'xpack.triggersActionsUI.sections.builtinActionTypes.emailAction.passwordFieldLabel',
{
Expand All @@ -343,17 +338,12 @@ const EmailActionConnectorFields: React.FunctionComponent<ActionConnectorFieldsP
>
<EuiFieldPassword
fullWidth
isInvalid={errors.password.length > 0 && password !== undefined}
isInvalid={errors.password.length > 0}
name="password"
value={password || ''}
data-test-subj="emailPasswordInput"
onChange={e => {
editActionSecrets('password', e.target.value);
}}
onBlur={() => {
if (!password) {
editActionSecrets('password', '');
}
editActionSecrets('password', nullableString(e.target.value));
}}
/>
</EuiFormRow>
Expand Down Expand Up @@ -624,3 +614,9 @@ const EmailParamsFields: React.FunctionComponent<ActionParamsProps<EmailActionPa
</Fragment>
);
};

// if the string == null or is empty, return null, else return string
function nullableString(str: string | null | undefined) {
if (str == null || str.trim() === '') return null;
return str;
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ interface EmailConfig {
}

interface EmailSecrets {
user: string;
password: string;
user: string | null;
password: string | null;
}

export interface EmailActionConnector extends ActionConnector {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,5 +227,61 @@ export default function emailTest({ getService }: FtrProviderContext) {
.expect(200);
expect(typeof createdAction.id).to.be('string');
});

it('should handle an email action with no auth', async () => {
const { body: createdAction } = await supertest
.post('/api/action')
.set('kbn-xsrf', 'foo')
.send({
name: 'An email action with no auth',
actionTypeId: '.email',
config: {
service: '__json',
from: 'jim@example.com',
},
})
.expect(200);

await supertest
.post(`/api/action/${createdAction.id}/_execute`)
.set('kbn-xsrf', 'foo')
.send({
params: {
to: ['kibana-action-test@elastic.co'],
subject: 'email-subject',
message: 'email-message',
},
})
.expect(200)
.then((resp: any) => {
expect(resp.body.data.message.messageId).to.be.a('string');
expect(resp.body.data.messageId).to.be.a('string');

delete resp.body.data.message.messageId;
delete resp.body.data.messageId;

expect(resp.body.data).to.eql({
envelope: {
from: 'jim@example.com',
to: ['kibana-action-test@elastic.co'],
},
message: {
from: { address: 'jim@example.com', name: '' },
to: [
{
address: 'kibana-action-test@elastic.co',
name: '',
},
],
cc: null,
bcc: null,
subject: 'email-subject',
html: '<p>email-message</p>\n',
text: 'email-message',
headers: {},
},
});
});
});
});
}

0 comments on commit 72bc0ea

Please sign in to comment.