Skip to content

Commit

Permalink
[alerting] encode rule/connector ids in http requests made from alert…
Browse files Browse the repository at this point in the history 
…ing UI (elastic#97854)

resolves: elastic#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.
  • Loading branch information
@pmuellr
pmuellr committed Apr 23, 2021
1 parent fd17880 commit d521eb9
Show file tree
Hide file tree
Showing 33 changed files with 168 additions and 125 deletions.
16 changes: 8 additions & 8 deletions ...s/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.test.ts
View file
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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/te%2Fst/_execute', {
body: '{"params":{"subAction":"issue","subActionParams":{"id":"RJ-107"}}}',
signal: abortCtrl.signal,
});
Expand Down
60 changes: 36 additions & 24 deletions ...lugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.ts
View file
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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'issueTypes', subActionParams: {} },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'fieldsByIssueType', subActionParams: { id } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'issues', subActionParams: { title } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'issue', subActionParams: { id } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'issue', subActionParams: { id } },
}),
signal,
}
);
}
12 changes: 6 additions & 6 deletions ...ggers_actions_ui/public/application/components/builtin_action_types/resilient/api.test.ts
View file
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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/te%2Fst/_execute', {
body: '{"params":{"subAction":"severity","subActionParams":{}}}',
signal: abortCtrl.signal,
});
Expand Down
30 changes: 18 additions & 12 deletions ...s/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.ts
View file
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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'incidentTypes', subActionParams: {} },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'severity', subActionParams: {} },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'severity', subActionParams: {} },
}),
signal,
}
);
}
4 changes: 2 additions & 2 deletions ...gers_actions_ui/public/application/components/builtin_action_types/servicenow/api.test.ts
View file
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/connector/test/_execute', {
expect(http.post).toHaveBeenCalledWith('/api/actions/connector/te%2Fst/_execute', {
body: '{"params":{"subAction":"getChoices","subActionParams":{"fields":["priority"]}}}',
signal: abortCtrl.signal,
});
Expand Down
15 changes: 9 additions & 6 deletions .../triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.ts
View file
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}/connector/${connectorId}/_execute`, {
body: JSON.stringify({
params: { subAction: 'getChoices', subActionParams: { fields } },
}),
signal,
});
return await http.post(
`${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(connectorId)}/_execute`,
{
body: JSON.stringify({
params: { subAction: 'getChoices', subActionParams: { fields } },
}),
signal,
}
);
}
4 changes: 2 additions & 2 deletions ...ck/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ beforeEach(() => jest.resetAllMocks());

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

const result = await deleteActions({ ids, http });
expect(result).toEqual({ errors: [], successes: [undefined, undefined, undefined] });
Expand All @@ -27,7 +27,7 @@ describe('deleteActions', () => {
"/api/actions/connector/2",
],
Array [
"/api/actions/connector/3",
"/api/actions/connector/%2F",
],
]
`);
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,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}/connector/${id}`))).then(
await Promise.all(
ids.map((id) => http.delete(`${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(id)}`))
).then(
function (fulfilled) {
successes.push(...fulfilled);
},
Expand Down
4 changes: 2 additions & 2 deletions ...k/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ beforeEach(() => jest.resetAllMocks());

describe('executeAction', () => {
test('should call execute API', async () => {
const id = '123';
const id = '12/3';
const params = {
stringParams: 'someString',
numericParams: 123,
Expand All @@ -32,7 +32,7 @@ describe('executeAction', () => {
});
expect(http.post.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/actions/connector/123/_execute",
"/api/actions/connector/12%2F3/_execute",
Object {
"body": "{\\"params\\":{\\"stringParams\\":\\"someString\\",\\"numericParams\\":123}}",
},
Expand Down
9 changes: 6 additions & 3 deletions x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ export async function executeAction({
http: HttpSetup;
params: Record<string, unknown>;
}): Promise<ActionTypeExecutorResult<unknown>> {
const res = await http.post(`${BASE_ACTION_API_PATH}/connector/${id}/_execute`, {
body: JSON.stringify({ params }),
});
const res = await http.post(
`${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(id)}/_execute`,
{
body: JSON.stringify({ params }),
}
);
return rewriteBodyRes(res);
}
8 changes: 4 additions & 4 deletions ...ck/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ beforeEach(() => jest.resetAllMocks());

describe('updateActionConnector', () => {
test('should call the update API', async () => {
const id = '123';
const id = '12/3';
const apiResponse = {
connector_type_id: 'test',
connector_type_id: 'te/st',
is_preconfigured: false,
name: 'My test',
config: {},
Expand All @@ -27,7 +27,7 @@ describe('updateActionConnector', () => {
http.put.mockResolvedValueOnce(apiResponse);

const connector: ActionConnectorWithoutId<{}, {}> = {
actionTypeId: 'test',
actionTypeId: 'te/st',
isPreconfigured: false,
name: 'My test',
config: {},
Expand All @@ -39,7 +39,7 @@ describe('updateActionConnector', () => {
expect(result).toEqual(resolvedValue);
expect(http.put.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/actions/connector/123",
"/api/actions/connector/12%2F3",
Object {
"body": "{\\"name\\":\\"My test\\",\\"config\\":{},\\"secrets\\":{}}",
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export async function updateActionConnector({
connector: Pick<ActionConnectorWithoutId, 'name' | 'config' | 'secrets'>;
id: string;
}): Promise<ActionConnector> {
const res = await http.put(`${BASE_ACTION_API_PATH}/connector/${id}`, {
const res = await http.put(`${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(id)}`, {
body: JSON.stringify({
name: connector.name,
config: connector.config,
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('loadAlertInstanceSummary', () => {
consumer: 'alerts',
enabled: true,
errorMessages: [],
id: 'test',
id: 'te/st',
lastRun: '2021-04-01T22:18:27.609Z',
muteAll: false,
name: 'test',
Expand All @@ -35,7 +35,7 @@ describe('loadAlertInstanceSummary', () => {
consumer: 'alerts',
enabled: true,
error_messages: [],
id: 'test',
id: 'te/st',
last_run: '2021-04-01T22:18:27.609Z',
mute_all: false,
name: 'test',
Expand All @@ -47,11 +47,11 @@ describe('loadAlertInstanceSummary', () => {
throttle: null,
});

const result = await loadAlertInstanceSummary({ http, alertId: 'test' });
const result = await loadAlertInstanceSummary({ http, alertId: 'te/st' });
expect(result).toEqual(resolvedValue);
expect(http.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/internal/alerting/rule/test/_alert_summary",
"/internal/alerting/rule/te%2Fst/_alert_summary",
]
`);
});
Expand Down
Loading

0 comments on commit d521eb9

Please sign in to comment.