Skip to content

Commit

Permalink
[7.12] [alerting] encode rule/connector ids in http requests made fro…
Browse files Browse the repository at this point in the history
…m alerting UI (#97854) (#98211)

* [alerting] encode rule/connector ids in http requests made from alerting UI (#97854)

resolves: #97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/delete.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/delete.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/disable.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/disable.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/enable.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/enable.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/get_rule.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/get_rule.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute_alert.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute_alert.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute_alert.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute_alert.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/update.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/update.ts
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts

* fix merge conflicts

In 7.13.0, the structure of the connector and rules API libraries in
triggers_actions_ui changed, where in 7.12 they were all in a single
file - one for connectors, one for rules - but in 7.13 they are split
out into separate files in a directory for connectors and one for rules.

To cut down on the noise, I decided to not use the `encodeURIComponent()`
wrappers on rule ids, just connector ids and alert ids, since it's not
possible in 7.12 to have rule ids which are not UUIDs, and so don't need
the encoding.

* fix prettier errors

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
pmuellr and kibanamachine committed Apr 26, 2021
1 parent b468ed7 commit 61f7cf9
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ describe('Jira API', () => {
test('should call get issue types API', async () => {
const abortCtrl = new AbortController();
http.post.mockResolvedValueOnce(issueTypesResponse);
const res = await getIssueTypes({ http, signal: abortCtrl.signal, connectorId: 'test' });
const res = await getIssueTypes({ http, signal: abortCtrl.signal, connectorId: 'te/st' });

expect(res).toEqual(issueTypesResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"issueTypes","subActionParams":{}}}',
signal: abortCtrl.signal,
});
Expand All @@ -116,12 +116,12 @@ describe('Jira API', () => {
const res = await getFieldsByIssueType({
http,
signal: abortCtrl.signal,
connectorId: 'test',
connectorId: 'te/st',
id: '10006',
});

expect(res).toEqual(fieldsResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"fieldsByIssueType","subActionParams":{"id":"10006"}}}',
signal: abortCtrl.signal,
});
Expand All @@ -135,12 +135,12 @@ describe('Jira API', () => {
const res = await getIssues({
http,
signal: abortCtrl.signal,
connectorId: 'test',
connectorId: 'te/st',
title: 'test issue',
});

expect(res).toEqual(issuesResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"issues","subActionParams":{"title":"test issue"}}}',
signal: abortCtrl.signal,
});
Expand All @@ -154,12 +154,12 @@ describe('Jira API', () => {
const res = await getIssue({
http,
signal: abortCtrl.signal,
connectorId: 'test',
connectorId: 'te/st',
id: 'RJ-107',
});

expect(res).toEqual(issuesResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"issue","subActionParams":{"id":"RJ-107"}}}',
signal: abortCtrl.signal,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ export async function getIssueTypes({
signal: AbortSignal;
connectorId: string;
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'issueTypes', subActionParams: {} },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'issueTypes', subActionParams: {} },
}),
signal,
}
);
}

export async function getFieldsByIssueType({
Expand All @@ -36,12 +39,15 @@ export async function getFieldsByIssueType({
connectorId: string;
id: string;
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'fieldsByIssueType', subActionParams: { id } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'fieldsByIssueType', subActionParams: { id } },
}),
signal,
}
);
}

export async function getIssues({
Expand All @@ -55,12 +61,15 @@ export async function getIssues({
connectorId: string;
title: string;
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'issues', subActionParams: { title } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'issues', subActionParams: { title } },
}),
signal,
}
);
}

export async function getIssue({
Expand All @@ -74,10 +83,13 @@ export async function getIssue({
connectorId: string;
id: string;
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'issue', subActionParams: { id } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'issue', subActionParams: { id } },
}),
signal,
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const incidentTypesResponse = {
{ id: 16, name: 'TBD / Unknown' },
{ id: 15, name: 'Vendor / 3rd party error' },
],
actionId: 'test',
actionId: 'te/st',
};

const severityResponse = {
Expand All @@ -42,7 +42,7 @@ const severityResponse = {
{ id: 5, name: 'Medium' },
{ id: 6, name: 'High' },
],
actionId: 'test',
actionId: 'te/st',
};

describe('Resilient API', () => {
Expand All @@ -57,11 +57,11 @@ describe('Resilient API', () => {
const res = await getIncidentTypes({
http,
signal: abortCtrl.signal,
connectorId: 'test',
connectorId: 'te/st',
});

expect(res).toEqual(incidentTypesResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"incidentTypes","subActionParams":{}}}',
signal: abortCtrl.signal,
});
Expand All @@ -75,11 +75,11 @@ describe('Resilient API', () => {
const res = await getSeverity({
http,
signal: abortCtrl.signal,
connectorId: 'test',
connectorId: 'te/st',
});

expect(res).toEqual(severityResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"severity","subActionParams":{}}}',
signal: abortCtrl.signal,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ export async function getIncidentTypes({
signal: AbortSignal;
connectorId: string;
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'incidentTypes', subActionParams: {} },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'incidentTypes', subActionParams: {} },
}),
signal,
}
);
}

export async function getSeverity({
Expand All @@ -34,10 +37,13 @@ export async function getSeverity({
signal: AbortSignal;
connectorId: string;
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'severity', subActionParams: {} },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'severity', subActionParams: {} },
}),
signal,
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ describe('ServiceNow API', () => {
const res = await getChoices({
http,
signal: abortCtrl.signal,
connectorId: 'test',
connectorId: 'te/st',
fields: ['priority'],
});

expect(res).toEqual(choicesResponse);
expect(http.post).toHaveBeenCalledWith('/api/actions/action/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/action/te%2Fst/_execute', {
body: '{"params":{"subAction":"getChoices","subActionParams":{"fields":["priority"]}}}',
signal: abortCtrl.signal,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ export async function getChoices({
connectorId: string;
fields: string[];
}): Promise<Record<string, any>> {
return await http.post(`${BASE_ACTION_API_PATH}/action/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'getChoices', subActionParams: { fields } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'getChoices', subActionParams: { fields } },
}),
signal,
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('loadActionTypes', () => {
test('should call get types API', async () => {
const resolvedValue: ActionType[] = [
{
id: 'test',
id: 'te/st',
name: 'Test',
enabled: true,
enabledInConfig: true,
Expand Down Expand Up @@ -67,7 +67,7 @@ describe('createActionConnector', () => {
config: {},
secrets: {},
};
const resolvedValue: ActionConnector = { ...connector, id: '123' };
const resolvedValue: ActionConnector = { ...connector, id: '12/3' };
http.post.mockResolvedValueOnce(resolvedValue);

const result = await createActionConnector({ http, connector });
Expand All @@ -85,7 +85,7 @@ describe('createActionConnector', () => {

describe('updateActionConnector', () => {
test('should call the update API', async () => {
const id = '123';
const id = '12/3';
const connector: ActionConnectorWithoutId<{}, {}> = {
actionTypeId: 'test',
isPreconfigured: false,
Expand All @@ -100,7 +100,7 @@ describe('updateActionConnector', () => {
expect(result).toEqual(resolvedValue);
expect(http.put.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/actions/action/123",
"/api/actions/action/12%2F3",
Object {
"body": "{\\"name\\":\\"My test\\",\\"config\\":{},\\"secrets\\":{}}",
},
Expand All @@ -111,7 +111,7 @@ describe('updateActionConnector', () => {

describe('deleteActions', () => {
test('should call delete API per action', async () => {
const ids = ['1', '2', '3'];
const ids = ['1', '2/', '3'];

const result = await deleteActions({ ids, http });
expect(result).toEqual({ errors: [], successes: [undefined, undefined, undefined] });
Expand All @@ -121,7 +121,7 @@ describe('deleteActions', () => {
"/api/actions/action/1",
],
Array [
"/api/actions/action/2",
"/api/actions/action/2%2F",
],
Array [
"/api/actions/action/3",
Expand All @@ -133,7 +133,7 @@ describe('deleteActions', () => {

describe('executeAction', () => {
test('should call execute API', async () => {
const id = '123';
const id = '12/3';
const params = {
stringParams: 'someString',
numericParams: 123,
Expand All @@ -151,7 +151,7 @@ describe('executeAction', () => {
});
expect(http.post.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/actions/action/123/_execute",
"/api/actions/action/12%2F3/_execute",
Object {
"body": "{\\"params\\":{\\"stringParams\\":\\"someString\\",\\"numericParams\\":123}}",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function updateActionConnector({
connector: Pick<ActionConnectorWithoutId, 'name' | 'config' | 'secrets'>;
id: string;
}): Promise<ActionConnector> {
return await http.put(`${BASE_ACTION_API_PATH}/action/${id}`, {
return await http.put(`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(id)}`, {
body: JSON.stringify({
name: connector.name,
config: connector.config,
Expand All @@ -57,7 +57,9 @@ export async function deleteActions({
}): Promise<{ successes: string[]; errors: string[] }> {
const successes: string[] = [];
const errors: string[] = [];
await Promise.all(ids.map((id) => http.delete(`${BASE_ACTION_API_PATH}/action/${id}`))).then(
await Promise.all(
ids.map((id) => http.delete(`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(id)}`))
).then(
function (fulfilled) {
successes.push(...fulfilled);
},
Expand All @@ -77,7 +79,7 @@ export async function executeAction({
http: HttpSetup;
params: Record<string, unknown>;
}): Promise<ActionTypeExecutorResult<unknown>> {
return http.post(`${BASE_ACTION_API_PATH}/action/${id}/_execute`, {
return http.post(`${BASE_ACTION_API_PATH}/action/${encodeURIComponent(id)}/_execute`, {
body: JSON.stringify({ params }),
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -664,12 +664,12 @@ describe('disableAlert', () => {

describe('muteAlertInstance', () => {
test('should call mute instance alert API', async () => {
const result = await muteAlertInstance({ http, id: '1', instanceId: '123' });
const result = await muteAlertInstance({ http, id: '1', instanceId: '12/3' });
expect(result).toEqual(undefined);
expect(http.post.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"/api/alerts/alert/1/alert_instance/123/_mute",
"/api/alerts/alert/1/alert_instance/12%2F3/_mute",
],
]
`);
Expand All @@ -678,12 +678,12 @@ describe('muteAlertInstance', () => {

describe('unmuteAlertInstance', () => {
test('should call mute instance alert API', async () => {
const result = await unmuteAlertInstance({ http, id: '1', instanceId: '123' });
const result = await unmuteAlertInstance({ http, id: '1', instanceId: '1/23' });
expect(result).toEqual(undefined);
expect(http.post.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"/api/alerts/alert/1/alert_instance/123/_unmute",
"/api/alerts/alert/1/alert_instance/1%2F23/_unmute",
],
]
`);
Expand Down
Loading

0 comments on commit 61f7cf9

Please sign in to comment.