From dd109814b19e35f28b4d37187bf753320353ebd4 Mon Sep 17 00:00:00 2001 From: Christos Nasikas <christos.nasikas@elastic.co> Date: Sat, 6 Feb 2021 13:33:22 +0200 Subject: [PATCH 1/4] Do not allow labels with spaces --- x-pack/plugins/actions/README.md | 2 +- .../actions/server/builtin_action_types/jira/schema.ts | 9 ++++++++- .../components/builtin_action_types/jira/jira.tsx | 7 +++++++ .../components/builtin_action_types/jira/jira_params.tsx | 8 ++++++++ .../components/builtin_action_types/jira/translations.ts | 7 +++++++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/actions/README.md b/x-pack/plugins/actions/README.md index 1eb94af4dddf8..1d50bc7e05807 100644 --- a/x-pack/plugins/actions/README.md +++ b/x-pack/plugins/actions/README.md @@ -657,7 +657,7 @@ The following table describes the properties of the `incident` object. | externalId | The id of the issue in Jira. If presented the incident will be update. Otherwise a new incident will be created. | string _(optional)_ | | issueType | The id of the issue type in Jira. | string _(optional)_ | | priority | The name of the priority in Jira. Example: `Medium`. | string _(optional)_ | -| labels | An array of labels. | string[] _(optional)_ | +| labels | An array of labels. Labels cannot contain spaces. | string[] _(optional)_ | | parent | The parent issue id or key. Only for `Sub-task` issue types. | string _(optional)_ | #### `subActionParams (getIncident)` diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts index 552053bdd7651..179ab97846bb1 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts @@ -40,7 +40,14 @@ export const ExecutorSubActionPushParamsSchema = schema.object({ externalId: schema.nullable(schema.string()), issueType: schema.nullable(schema.string()), priority: schema.nullable(schema.string()), - labels: schema.nullable(schema.arrayOf(schema.string())), + labels: schema.nullable( + schema.arrayOf( + schema.string({ + validate: (label) => + label.includes(' ') ? `The label ${label} cannot contain spaces` : undefined, + }) + ) + ), parent: schema.nullable(schema.string()), }), comments: schema.nullable( diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx index 5cb8a76d09bee..ded02256bd2fe 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx @@ -72,6 +72,7 @@ export function getActionType(): ActionTypeModel<JiraConfig, JiraSecrets, JiraAc validateParams: (actionParams: JiraActionParams): GenericValidationResult<unknown> => { const errors = { 'subActionParams.incident.summary': new Array<string>(), + 'subActionParams.incident.labels': new Array<string>(), }; const validationResult = { errors, @@ -83,6 +84,12 @@ export function getActionType(): ActionTypeModel<JiraConfig, JiraSecrets, JiraAc ) { errors['subActionParams.incident.summary'].push(i18n.SUMMARY_REQUIRED); } + + if (actionParams.subActionParams?.incident?.labels?.length) { + // Jira do not allows empty spaces on labels. If the label includes a whitespace show an error. + if (actionParams.subActionParams.incident.labels.some((label) => label.includes(' '))) + errors['subActionParams.incident.labels'].push(i18n.LABELS_WHITE_SPACES); + } return validationResult; }, actionParamsFields: lazy(() => import('./jira_params')), diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx index 75930482797a2..cb2d637972cb8 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx @@ -184,6 +184,11 @@ const JiraParamsFields: React.FunctionComponent<ActionParamsProps<JiraActionPara // eslint-disable-next-line react-hooks/exhaustive-deps }, [actionParams]); + const areLabelsInvalid = + errors['subActionParams.incident.labels'] != null && + errors['subActionParams.incident.labels'].length > 0 && + incident.labels !== undefined; + return ( <Fragment> <> @@ -304,6 +309,8 @@ const JiraParamsFields: React.FunctionComponent<ActionParamsProps<JiraActionPara defaultMessage: 'Labels', } )} + error={errors['subActionParams.incident.labels'] as string[]} + isInvalid={areLabelsInvalid} > <EuiComboBox noSuggestions @@ -331,6 +338,7 @@ const JiraParamsFields: React.FunctionComponent<ActionParamsProps<JiraActionPara }} isClearable={true} data-test-subj="labelsComboBox" + isInvalid={areLabelsInvalid} /> </EuiFormRow> </EuiFlexItem> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts index 3c8bda7792f0a..5a8bdbbaa7a98 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts @@ -199,3 +199,10 @@ export const SEARCH_ISSUES_LOADING = i18n.translate( defaultMessage: 'Loading...', } ); + +export const LABELS_WHITE_SPACES = i18n.translate( + 'xpack.triggersActionsUI.components.builtinActionTypes.jira.requiredSummaryTextField', + { + defaultMessage: 'Labels cannot contain spaces.', + } +); From 2702efb6c38b7bd550f7a9b968b6d3036d7c0742 Mon Sep 17 00:00:00 2001 From: Christos Nasikas <christos.nasikas@elastic.co> Date: Mon, 8 Feb 2021 15:50:47 +0200 Subject: [PATCH 2/4] Fix i18n --- .../components/builtin_action_types/jira/translations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts index 5a8bdbbaa7a98..fe7ea61e68193 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts @@ -201,7 +201,7 @@ export const SEARCH_ISSUES_LOADING = i18n.translate( ); export const LABELS_WHITE_SPACES = i18n.translate( - 'xpack.triggersActionsUI.components.builtinActionTypes.jira.requiredSummaryTextField', + 'xpack.triggersActionsUI.components.builtinActionTypes.jira.labelsSpacesErrorMessage', { defaultMessage: 'Labels cannot contain spaces.', } From ad932e4b4addfeb64ce4ac5c415bda5edbf8428c Mon Sep 17 00:00:00 2001 From: Christos Nasikas <christos.nasikas@elastic.co> Date: Mon, 8 Feb 2021 16:02:42 +0200 Subject: [PATCH 3/4] Add tests --- .../builtin_action_types/jira/jira.test.tsx | 19 ++++++++++++- .../actions/builtin_action_types/jira.ts | 28 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.test.tsx index 2d47740a581b8..ea1bcf82c314c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.test.tsx @@ -96,7 +96,7 @@ describe('jira action params validation', () => { }; expect(actionTypeModel.validateParams(actionParams)).toEqual({ - errors: { 'subActionParams.incident.summary': [] }, + errors: { 'subActionParams.incident.summary': [], 'subActionParams.incident.labels': [] }, }); }); @@ -108,6 +108,23 @@ describe('jira action params validation', () => { expect(actionTypeModel.validateParams(actionParams)).toEqual({ errors: { 'subActionParams.incident.summary': ['Summary is required.'], + 'subActionParams.incident.labels': [], + }, + }); + }); + + test('params validation fails when labels contain spaces', () => { + const actionParams = { + subActionParams: { + incident: { summary: 'some title', labels: ['label with spaces'] }, + comments: [], + }, + }; + + expect(actionTypeModel.validateParams(actionParams)).toEqual({ + errors: { + 'subActionParams.incident.summary': [], + 'subActionParams.incident.labels': ['Labels cannot contain spaces.'], }, }); }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/jira.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/jira.ts index 6cc5e2eaefb94..8bd0b8a790d40 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/jira.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/jira.ts @@ -375,6 +375,34 @@ export default function jiraTest({ getService }: FtrProviderContext) { }); }); }); + + it('should handle failing with a simulated success when labels containing a space', async () => { + await supertest + .post(`/api/actions/action/${simulatedActionId}/_execute`) + .set('kbn-xsrf', 'foo') + .send({ + params: { + ...mockJira.params, + subActionParams: { + incident: { + ...mockJira.params.subActionParams.incident, + issueType: '10006', + labels: ['label with spaces'], + }, + comments: [], + }, + }, + }) + .then((resp: any) => { + expect(resp.body).to.eql({ + actionId: simulatedActionId, + status: 'error', + retry: false, + message: + 'error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [getFields]\n- [1.subAction]: expected value to equal [getIncident]\n- [2.subAction]: expected value to equal [handshake]\n- [3.subActionParams.incident.labels]: types that failed validation:\n - [subActionParams.incident.labels.0.0]: The label label with spaces cannot contain spaces\n - [subActionParams.incident.labels.1]: expected value to equal [null]\n- [4.subAction]: expected value to equal [issueTypes]\n- [5.subAction]: expected value to equal [fieldsByIssueType]\n- [6.subAction]: expected value to equal [issues]\n- [7.subAction]: expected value to equal [issue]', + }); + }); + }); }); describe('Execution', () => { From 058a9bc8e45fc177b70d9147d5b6f3a41e35fdc9 Mon Sep 17 00:00:00 2001 From: Christos Nasikas <christos.nasikas@elastic.co> Date: Mon, 8 Feb 2021 18:54:06 +0200 Subject: [PATCH 4/4] Change to RegExp --- .../plugins/actions/server/builtin_action_types/jira/schema.ts | 3 ++- .../application/components/builtin_action_types/jira/jira.tsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts index 179ab97846bb1..a81dfaeef8175 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/jira/schema.ts @@ -44,7 +44,8 @@ export const ExecutorSubActionPushParamsSchema = schema.object({ schema.arrayOf( schema.string({ validate: (label) => - label.includes(' ') ? `The label ${label} cannot contain spaces` : undefined, + // Matches any space, tab or newline character. + label.match(/\s/g) ? `The label ${label} cannot contain spaces` : undefined, }) ) ), diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx index ded02256bd2fe..26b37278003c3 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx @@ -87,7 +87,7 @@ export function getActionType(): ActionTypeModel<JiraConfig, JiraSecrets, JiraAc if (actionParams.subActionParams?.incident?.labels?.length) { // Jira do not allows empty spaces on labels. If the label includes a whitespace show an error. - if (actionParams.subActionParams.incident.labels.some((label) => label.includes(' '))) + if (actionParams.subActionParams.incident.labels.some((label) => label.match(/\s/g))) errors['subActionParams.incident.labels'].push(i18n.LABELS_WHITE_SPACES); } return validationResult;