Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.16] [Connectors] Check connector's responses (#115797) #116085

Merged
merged 1 commit into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
918 changes: 506 additions & 412 deletions x-pack/plugins/actions/server/builtin_action_types/jira/service.test.ts

Large diffs are not rendered by default.

66 changes: 55 additions & 11 deletions x-pack/plugins/actions/server/builtin_action_types/jira/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
} from './types';

import * as i18n from './translations';
import { request, getErrorMessage } from '../lib/axios_utils';
import { request, getErrorMessage, throwIfResponseIsNotValid } from '../lib/axios_utils';
import { ActionsConfigurationUtilities } from '../../actions_config';

const VERSION = '2';
Expand Down Expand Up @@ -111,19 +111,15 @@ export const createExternalService = (
.filter((item) => !isEmpty(item))
.join(', ');

const createErrorMessage = (errorResponse: ResponseError | string | null | undefined): string => {
const createErrorMessage = (errorResponse: ResponseError | null | undefined): string => {
if (errorResponse == null) {
return '';
}
if (typeof errorResponse === 'string') {
// Jira error.response.data can be string!!
return errorResponse;
return 'unknown: errorResponse was null';
}

const { errorMessages, errors } = errorResponse;

if (errors == null) {
return '';
return 'unknown: errorResponse.errors was null';
}

if (Array.isArray(errorMessages) && errorMessages.length > 0) {
Expand Down Expand Up @@ -185,9 +181,14 @@ export const createExternalService = (
configurationUtilities,
});

const { fields, ...rest } = res.data;
throwIfResponseIsNotValid({
res,
requiredAttributesToBeInTheResponse: ['id', 'key'],
});

const { fields, id: incidentId, key } = res.data;

return { ...rest, ...fields };
return { id: incidentId, key, created: fields.created, updated: fields.updated, ...fields };
} catch (error) {
throw new Error(
getErrorMessage(
Expand Down Expand Up @@ -234,6 +235,11 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
requiredAttributesToBeInTheResponse: ['id'],
});

const updatedIncident = await getIncident(res.data.id);

return {
Expand Down Expand Up @@ -266,7 +272,7 @@ export const createExternalService = (
const fields = createFields(projectKey, incidentWithoutNullValues);

try {
await request({
const res = await request({
axios: axiosInstance,
method: 'put',
url: `${incidentUrl}/${incidentId}`,
Expand All @@ -275,6 +281,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

const updatedIncident = await getIncident(incidentId as string);

return {
Expand Down Expand Up @@ -309,6 +319,11 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
requiredAttributesToBeInTheResponse: ['id', 'created'],
});

return {
commentId: comment.commentId,
externalCommentId: res.data.id,
Expand Down Expand Up @@ -336,6 +351,11 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
requiredAttributesToBeInTheResponse: ['capabilities'],
});

return { ...res.data };
} catch (error) {
throw new Error(
Expand All @@ -362,6 +382,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

const issueTypes = res.data.projects[0]?.issuetypes ?? [];
return normalizeIssueTypes(issueTypes);
} else {
Expand All @@ -373,6 +397,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

const issueTypes = res.data.values;
return normalizeIssueTypes(issueTypes);
}
Expand Down Expand Up @@ -401,6 +429,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

const fields = res.data.projects[0]?.issuetypes[0]?.fields || {};
return normalizeFields(fields);
} else {
Expand All @@ -412,6 +444,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

const fields = res.data.values.reduce(
(acc: { [x: string]: {} }, value: { fieldId: string }) => ({
...acc,
Expand Down Expand Up @@ -471,6 +507,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

return normalizeSearchResults(res.data?.issues ?? []);
} catch (error) {
throw new Error(
Expand All @@ -495,6 +535,10 @@ export const createExternalService = (
configurationUtilities,
});

throwIfResponseIsNotValid({
res,
});

return normalizeIssue(res.data ?? {});
} catch (error) {
throw new Error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ import { Agent as HttpsAgent } from 'https';
import HttpProxyAgent from 'http-proxy-agent';
import { HttpsProxyAgent } from 'https-proxy-agent';
import { Logger } from '../../../../../../src/core/server';
import { addTimeZoneToDate, request, patch, getErrorMessage } from './axios_utils';
import {
addTimeZoneToDate,
request,
patch,
getErrorMessage,
throwIfResponseIsNotValid,
createAxiosResponse,
} from './axios_utils';
import { loggingSystemMock } from '../../../../../../src/core/server/mocks';
import { actionsConfigMock } from '../../actions_config.mock';
import { getCustomAgents } from './get_custom_agents';
Expand Down Expand Up @@ -292,3 +299,82 @@ describe('getErrorMessage', () => {
expect(msg).toBe('[Action][My connector name]: An error has occurred');
});
});

describe('throwIfResponseIsNotValid', () => {
const res = createAxiosResponse({
headers: { ['content-type']: 'application/json' },
data: { incident: { id: '1' } },
});

test('it does NOT throw if the request is valid', () => {
expect(() => throwIfResponseIsNotValid({ res })).not.toThrow();
});

test('it does throw if the content-type is not json', () => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, headers: { ['content-type']: 'text/html' } },
})
).toThrow(
'Unsupported content type: text/html in GET https://example.com. Supported content types: application/json'
);
});

test('it does throw if the content-type is undefined', () => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, headers: {} },
})
).toThrow(
'Unsupported content type: undefined in GET https://example.com. Supported content types: application/json'
);
});

test('it does throw if the data is not an object or array', () => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, data: 'string' },
})
).toThrow('Response is not a valid JSON');
});

test('it does NOT throw if the data is an array', () => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, data: ['test'] },
})
).not.toThrow();
});

test.each(['', [], {}])('it does NOT throw if the data is %p', (data) => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, data },
})
).not.toThrow();
});

test('it does throw if the required attribute is not in the response', () => {
expect(() =>
throwIfResponseIsNotValid({ res, requiredAttributesToBeInTheResponse: ['not-exist'] })
).toThrow('Response is missing at least one of the expected fields: not-exist');
});

test('it does throw if the required attribute are defined and the data is an array', () => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, data: ['test'] },
requiredAttributesToBeInTheResponse: ['not-exist'],
})
).toThrow('Response is missing at least one of the expected fields: not-exist');
});

test('it does NOT throw if the value of the required attribute is null', () => {
expect(() =>
throwIfResponseIsNotValid({
res: { ...res, data: { id: null } },
requiredAttributesToBeInTheResponse: ['id'],
})
).not.toThrow();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { isObjectLike, isEmpty } from 'lodash';
import { AxiosInstance, Method, AxiosResponse, AxiosBasicCredentials } from 'axios';
import { Logger } from '../../../../../../src/core/server';
import { getCustomAgents } from './get_custom_agents';
Expand Down Expand Up @@ -76,3 +77,70 @@ export const addTimeZoneToDate = (date: string, timezone = 'GMT'): string => {
export const getErrorMessage = (connector: string, msg: string) => {
return `[Action][${connector}]: ${msg}`;
};

export const throwIfResponseIsNotValid = ({
res,
requiredAttributesToBeInTheResponse = [],
}: {
res: AxiosResponse;
requiredAttributesToBeInTheResponse?: string[];
}) => {
const requiredContentType = 'application/json';
const contentType = res.headers['content-type'] ?? 'undefined';
const data = res.data;

/**
* Check that the content-type of the response is application/json.
* Then includes is added because the header can be application/json;charset=UTF-8.
*/
if (!contentType.includes(requiredContentType)) {
throw new Error(
`Unsupported content type: ${contentType} in ${res.config.method} ${res.config.url}. Supported content types: ${requiredContentType}`
);
}

/**
* Check if the response is a JS object (data != null && typeof data === 'object')
* in case the content type is application/json but for some reason the response is not.
* Empty responses (204 No content) are ignored because the typeof data will be string and
* isObjectLike will fail.
* Axios converts automatically JSON to JS objects.
*/
if (!isEmpty(data) && !isObjectLike(data)) {
throw new Error('Response is not a valid JSON');
}

if (requiredAttributesToBeInTheResponse.length > 0) {
const requiredAttributesError = new Error(
`Response is missing at least one of the expected fields: ${requiredAttributesToBeInTheResponse.join(
','
)}`
);

/**
* If the response is an array and requiredAttributesToBeInTheResponse
* are not empty then we thrown an error assuming that the consumer
* expects an object response and not an array.
*/

if (Array.isArray(data)) {
throw requiredAttributesError;
}

requiredAttributesToBeInTheResponse.forEach((attr) => {
// Check only for undefined as null is a valid value
if (data[attr] === undefined) {
throw requiredAttributesError;
}
});
}
};

export const createAxiosResponse = (res: Partial<AxiosResponse>): AxiosResponse => ({
data: {},
status: 200,
statusText: 'OK',
headers: { ['content-type']: 'application/json' },
config: { method: 'GET', url: 'https://example.com' },
...res,
});
Loading