[Connectors] Check connector's responses

x-pack/plugins/actions/server/builtin_action_types/jira/service.ts

@@ -27,7 +27,7 @@ import {
 } from './types';
 
 import * as i18n from './translations';
-import { request, getErrorMessage } from '../lib/axios_utils';
+import { request, getErrorMessage, throwIfRequestIsNotValid } from '../lib/axios_utils';
 import { ActionsConfigurationUtilities } from '../../actions_config';
 
 const VERSION = '2';
@@ -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';
     }
 
     const { errorMessages, errors } = errorResponse;
 
     if (errors == null) {
-      return '';
+      return 'unknown';
     }
 
     if (Array.isArray(errorMessages) && errorMessages.length > 0) {
@@ -185,9 +181,14 @@ export const createExternalService = (
         configurationUtilities,
       });
 
-      const { fields, ...rest } = res.data;
+      throwIfRequestIsNotValid({
+        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(
@@ -234,6 +235,11 @@ export const createExternalService = (
         configurationUtilities,
       });
 
+      throwIfRequestIsNotValid({
+        res,
+        requiredAttributesToBeInTheResponse: ['id'],
+      });
+
       const updatedIncident = await getIncident(res.data.id);
 
       return {
@@ -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}`,
@@ -275,6 +281,10 @@ export const createExternalService = (
         configurationUtilities,
       });
 
+      throwIfRequestIsNotValid({
+        res,
+      });
+
       const updatedIncident = await getIncident(incidentId as string);
 
       return {
@@ -309,6 +319,11 @@ export const createExternalService = (
         configurationUtilities,
       });
 
+      throwIfRequestIsNotValid({
+        res,
+        requiredAttributesToBeInTheResponse: ['id', 'created'],
+      });
+
       return {
         commentId: comment.commentId,
         externalCommentId: res.data.id,
@@ -336,6 +351,11 @@ export const createExternalService = (
         configurationUtilities,
       });
 
+      throwIfRequestIsNotValid({
+        res,
+        requiredAttributesToBeInTheResponse: ['capabilities'],
+      });
+
       return { ...res.data };
     } catch (error) {
       throw new Error(
@@ -362,6 +382,10 @@ export const createExternalService = (
           configurationUtilities,
         });
 
+        throwIfRequestIsNotValid({
+          res,
+        });
+
         const issueTypes = res.data.projects[0]?.issuetypes ?? [];
         return normalizeIssueTypes(issueTypes);
       } else {
@@ -373,6 +397,10 @@ export const createExternalService = (
           configurationUtilities,
         });
 
+        throwIfRequestIsNotValid({
+          res,
+        });
+
         const issueTypes = res.data.values;
         return normalizeIssueTypes(issueTypes);
       }
@@ -401,6 +429,10 @@ export const createExternalService = (
           configurationUtilities,
         });
 
+        throwIfRequestIsNotValid({
+          res,
+        });
+
         const fields = res.data.projects[0]?.issuetypes[0]?.fields || {};
         return normalizeFields(fields);
       } else {
@@ -412,6 +444,10 @@ export const createExternalService = (
           configurationUtilities,
         });
 
+        throwIfRequestIsNotValid({
+          res,
+        });
+
         const fields = res.data.values.reduce(
           (acc: { [x: string]: {} }, value: { fieldId: string }) => ({
             ...acc,
@@ -471,6 +507,10 @@ export const createExternalService = (
         configurationUtilities,
       });
 
+      throwIfRequestIsNotValid({
+        res,
+      });
+
       return normalizeSearchResults(res.data?.issues ?? []);
     } catch (error) {
       throw new Error(
@@ -495,6 +535,10 @@ export const createExternalService = (
         configurationUtilities,
       });
 
+      throwIfRequestIsNotValid({
+        res,
+      });
+
       return normalizeIssue(res.data ?? {});
     } catch (error) {
       throw new Error(

x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.test.ts

@@ -5,12 +5,18 @@
  * 2.0.
  */
 
-import axios from 'axios';
+import axios, { AxiosResponse } from 'axios';
 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,
+  throwIfRequestIsNotValid,
+} from './axios_utils';
 import { loggingSystemMock } from '../../../../../../src/core/server/mocks';
 import { actionsConfigMock } from '../../actions_config.mock';
 import { getCustomAgents } from './get_custom_agents';
@@ -292,3 +298,78 @@ describe('getErrorMessage', () => {
     expect(msg).toBe('[Action][My connector name]: An error has occurred');
   });
 });
+
+describe('throwIfRequestIsNotValid', () => {
+  const res = {
+    headers: { ['content-type']: 'application/json' },
+    data: { incident: { id: '1' } },
+  } as AxiosResponse;
+
+  test('it does NOT throw if the request is valid', () => {
+    expect(() => throwIfRequestIsNotValid({ res })).not.toThrow();
+  });
+
+  test('it does throw if the content-type is not json', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, headers: { ['content-type']: 'text/html' } },
+      })
+    ).toThrow();
+  });
+
+  test('it does throw if the content-type is defined', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, headers: {} },
+      })
+    ).toThrow();
+  });
+
+  test('it does throw if the data is not an object or array', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, data: 'string' },
+      })
+    ).toThrow();
+  });
+
+  test('it does NOT throw if the data is an array', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, data: ['test'] },
+      })
+    ).not.toThrow();
+  });
+
+  test.each(['', [], {}])('it does NOT throw if the data is %p', (data) => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, data },
+      })
+    ).not.toThrow();
+  });
+
+  test('it does throw if the required attribute is not in the response', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({ res, requiredAttributesToBeInTheResponse: ['not-exist'] })
+    ).toThrow();
+  });
+
+  test('it does NOT throw if the required attribute is not in the response and the data are an array', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, data: ['test'] },
+        requiredAttributesToBeInTheResponse: ['not-exist'],
+      })
+    ).not.toThrow();
+  });
+
+  test('it does NOT throw if the value of the required attribute is null', () => {
+    expect(() =>
+      throwIfRequestIsNotValid({
+        res: { ...res, data: { id: null } },
+        requiredAttributesToBeInTheResponse: ['id'],
+      })
+    ).not.toThrow();
+  });
+});

x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts

@@ -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';
@@ -76,3 +77,41 @@ export const addTimeZoneToDate = (date: string, timezone = 'GMT'): string => {
 export const getErrorMessage = (connector: string, msg: string) => {
   return `[Action][${connector}]: ${msg}`;
 };
+
+export const throwIfRequestIsNotValid = ({
+  res,
+  requiredAttributesToBeInTheResponse = [],
+}: {
+  res: AxiosResponse;
+  requiredAttributesToBeInTheResponse?: string[];
+}) => {
+  const requiredContentType = 'application/json';
+  const contentType = res.headers['content-type'] ?? '';
+  const data = res.data;
+  /**
+   * This check ensures that the response is a valid JSON.
+   * First we 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.
+   * Then we 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 (!contentType.includes(requiredContentType) || (!isEmpty(data) && !isObjectLike(data))) {
+    throw new Error('Response must be a valid JSON');
+  }
+
+  /**
+   * Check for required attributes only if provided and
+   * data is an object and not an array
+   */
+  if (requiredAttributesToBeInTheResponse.length > 0 && !Array.isArray(data)) {
+    requiredAttributesToBeInTheResponse.forEach((attr) => {
+      // Check only for undefined as null is a valid value
+      if (data[attr] === undefined) {
+        throw new Error('Response is missing expected fields');
+      }
+    });
+  }
+};