[SIEM] [Detection Engine] Increase unit test coverage (#58274)

* updates read privileges route tests

* fixes test for 404 when alertsClient is not present, adds new test for catching errors at the route level for prepackaged rules route

* fixes test for happy path on create rules bulk route (missing mock resulted in 200, but was still throwing an error), adds tests for covering case where a rule with a matching rule id is found to have already existed, adds a test covering the case where createRules throws an error and that error is caught in the catch block, and appears in the response

* adds more jest functions to beforeEach to ensure mockImplementation state used by spyOn is not carried over between tests, increases test coverage for create rules route

* updates unit test coverage for delete rules route and bulk delete

* increase unit test coverage for find_rules_route

* add test case to get_prepackaged_rules where findRules throws an error

* adds unit test for missing alertsClient in patch_rules_bulk route

* adds unit test coverage for transform error and pathRules throwing an error on patch rules route

* adds unit test coverage for rule not found, transform error, and readRules throws an error on read_rules_route

* adds unit test coverage for update rules (bulk) routes

* increases unit test coverage for open close signals route

* updates coverage for signals query route

* adds unit tests for rules status route, updates utils test coverage. Removed unreachable code.

* updates test coverage, removes usage of pipes from ndjson to stream conversion and returns arrays of tranformers to be used in createPromiseFromStreams so that unhandled promise rejections can be handled appropriately.

* fixes type errors

* fix bug on transform when rulestatus saved objects array is empty because we are no longer passing in the current status, we are passing in the whole saved object.

* adds unit test for when readRules throws an error inside of get export by object ids

* adds unit tests for catching errors on read rules and fixes property undefined bug in catch block

* removes unused function from utils

* adds the 'else' clause back to the getTupleDuplicateErrorsAndUniqueRules function and adds a test case for when an attempt to import rules with missing ruleIds is made, and the expected outcome.

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/request_responses.ts

@@ -427,13 +427,54 @@ export const getMockPrivileges = () => ({
   has_encryption_key: true,
 });
 
-export const getFindResultStatus = (): SavedObjectsFindResponse<IRuleSavedAttributesSavedObjectAttributes> => ({
+export const getFindResultStatusEmpty = (): SavedObjectsFindResponse<IRuleSavedAttributesSavedObjectAttributes> => ({
   page: 1,
   per_page: 1,
   total: 0,
   saved_objects: [],
 });
 
+export const getFindResultStatus = (): SavedObjectsFindResponse<IRuleSavedAttributesSavedObjectAttributes> => ({
+  page: 1,
+  per_page: 6,
+  total: 2,
+  saved_objects: [
+    {
+      type: 'my-type',
+      id: 'e0b86950-4e9f-11ea-bdbd-07b56aa159b3',
+      attributes: {
+        alertId: '1ea5a820-4da1-4e82-92a1-2b43a7bece08',
+        statusDate: '2020-02-18T15:26:49.783Z',
+        status: 'succeeded',
+        lastFailureAt: null,
+        lastSuccessAt: '2020-02-18T15:26:49.783Z',
+        lastFailureMessage: null,
+        lastSuccessMessage: 'succeeded',
+      },
+      references: [],
+      updated_at: '2020-02-18T15:26:51.333Z',
+      version: 'WzQ2LDFd',
+    },
+    {
+      type: 'my-type',
+      id: '91246bd0-5261-11ea-9650-33b954270f67',
+      attributes: {
+        alertId: '1ea5a820-4da1-4e82-92a1-2b43a7bece08',
+        statusDate: '2020-02-18T15:15:58.806Z',
+        status: 'failed',
+        lastFailureAt: '2020-02-18T15:15:58.806Z',
+        lastSuccessAt: '2020-02-13T20:31:59.855Z',
+        lastFailureMessage:
+          'Signal rule name: "Query with a rule id Number 1", id: "1ea5a820-4da1-4e82-92a1-2b43a7bece08", rule_id: "query-rule-id-1" has a time gap of 5 days (412682928ms), and could be missing signals within that time. Consider increasing your look behind time or adding more Kibana instances.',
+        lastSuccessMessage: 'succeeded',
+      },
+      references: [],
+      updated_at: '2020-02-18T15:15:58.860Z',
+      version: 'WzMyLDFd',
+    },
+  ],
+});
+
 export const getIndexName = () => 'index-name';
 export const getEmptyIndex = (): { _shards: Partial<ShardsResponse> } => ({
   _shards: { total: 0 },

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/utils.ts

@@ -26,6 +26,20 @@ export const getSimpleRule = (ruleId = 'rule-1'): Partial<OutputRuleAlertRest> =
   query: 'user.name: root or user.name: admin',
 });
 
+/**
+ * This is a typical simple rule for testing that is easy for most basic testing
+ * @param ruleId
+ */
+export const getSimpleRuleWithId = (id = 'rule-1'): Partial<OutputRuleAlertRest> => ({
+  name: 'Simple Rule Query',
+  description: 'Simple Rule Query',
+  risk_score: 1,
+  id,
+  severity: 'high',
+  type: 'query',
+  query: 'user.name: root or user.name: admin',
+});
+
 /**
  * Given an array of rule_id strings this will return a ndjson buffer which is useful
  * for testing uploads.
@@ -51,3 +65,26 @@ export const getSimpleRuleAsMultipartContent = (ruleIds: string[], isNdjson = tr
 
   return Buffer.from(resultingPayload);
 };
+
+/**
+ * Given an array of rule_id strings this will return a ndjson buffer which is useful
+ * for testing uploads.
+ * @param count Number of rules to generate
+ * @param isNdjson Boolean to determine file extension
+ */
+export const getSimpleRuleAsMultipartContentNoRuleId = (count: number, isNdjson = true): Buffer => {
+  const arrayOfRules = Array(count).fill(JSON.stringify(getSimpleRuleWithId()));
+  const stringOfRules = arrayOfRules.join('\r\n');
+
+  const resultingPayload =
+    `--${TEST_BOUNDARY}\r\n` +
+    `Content-Disposition: form-data; name="file"; filename="rules.${
+      isNdjson ? 'ndjson' : 'json'
+    }\r\n` +
+    'Content-Type: application/octet-stream\r\n' +
+    '\r\n' +
+    `${stringOfRules}\r\n` +
+    `--${TEST_BOUNDARY}--\r\n`;
+
+  return Buffer.from(resultingPayload);
+};

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/privileges/read_privileges_route.test.ts

@@ -5,6 +5,7 @@
  */
 
 import { readPrivilegesRoute } from './read_privileges_route';
+import * as readPrivileges from '../../privileges/read_privileges';
 import { createMockServer, createMockConfig, clientsServiceMock } from '../__mocks__';
 import { getPrivilegeRequest, getMockPrivileges } from '../__mocks__/request_responses';
 
@@ -38,5 +39,16 @@ describe('read_privileges', () => {
       const { payload } = await inject(getPrivilegeRequest());
       expect(JSON.parse(payload)).toEqual(getMockPrivileges());
     });
+
+    test('returns 500 when bad response from readPrivileges', async () => {
+      jest.spyOn(readPrivileges, 'readPrivileges').mockImplementation(() => {
+        throw new Error('Test error');
+      });
+      const { payload } = await inject(getPrivilegeRequest());
+      expect(JSON.parse(payload)).toEqual({
+        message: 'Test error',
+        status_code: 500,
+      });
+    });
   });
 });

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts

@@ -6,7 +6,6 @@
 
 import { omit } from 'lodash/fp';
 
-import { createRulesRoute } from './create_rules_route';
 import {
   getFindResult,
   getResult,
@@ -17,6 +16,7 @@ import {
   getNonEmptyIndex,
 } from '../__mocks__/request_responses';
 import { createMockServer, createMockConfig, clientsServiceMock } from '../__mocks__';
+import * as updatePrepackagedRules from '../../rules/update_prepacked_rules';
 
 jest.mock('../../rules/get_prepackaged_rules', () => {
   return {
@@ -54,7 +54,8 @@ describe('add_prepackaged_rules_route', () => {
 
   beforeEach(() => {
     jest.resetAllMocks();
-
+    jest.restoreAllMocks();
+    jest.clearAllMocks();
     server = createMockServer();
     config = createMockConfig();
     getClients = clientsServiceMock.createGetScoped();
@@ -78,9 +79,7 @@ describe('add_prepackaged_rules_route', () => {
 
     test('returns 404 if alertClient is not available on the route', async () => {
       getClients.mockResolvedValue(omit('alertsClient', clients));
-      const { inject, route } = createMockServer();
-      createRulesRoute(route, config, getClients);
-      const { statusCode } = await inject(addPrepackagedRulesRequest());
+      const { statusCode } = await server.inject(addPrepackagedRulesRequest());
       expect(statusCode).toBe(404);
     });
   });
@@ -126,5 +125,19 @@ describe('add_prepackaged_rules_route', () => {
         rules_updated: 1,
       });
     });
+    test('catches errors if payloads cause errors to be thrown', async () => {
+      jest.spyOn(updatePrepackagedRules, 'updatePrepackagedRules').mockImplementation(() => {
+        throw new Error('Test error');
+      });
+      clients.alertsClient.find.mockResolvedValue(getFindResultWithSingleHit());
+      clients.alertsClient.get.mockResolvedValue(getResult());
+      clients.actionsClient.create.mockResolvedValue(createActionResult());
+      clients.alertsClient.create.mockResolvedValue(getResult());
+      const { payload } = await server.inject(addPrepackagedRulesRequest());
+      expect(JSON.parse(payload)).toEqual({
+        message: 'Test error',
+        status_code: 500,
+      });
+    });
   });
 });

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/create_rules_bulk_route.test.ts

@@ -14,12 +14,15 @@ import {
   typicalPayload,
   getReadBulkRequest,
   getEmptyIndex,
+  getNonEmptyIndex,
 } from '../__mocks__/request_responses';
 import { createMockServer, createMockConfig, clientsServiceMock } from '../__mocks__';
 import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
 import { createRulesBulkRoute } from './create_rules_bulk_route';
 import { BulkError } from '../utils';
 import { OutputRuleAlertRest } from '../../types';
+import * as createRules from '../../rules/create_rules';
+import * as readRules from '../../rules/read_rules';
 
 describe('create_rules_bulk', () => {
   let server = createMockServer();
@@ -29,10 +32,14 @@ describe('create_rules_bulk', () => {
 
   beforeEach(() => {
     jest.resetAllMocks();
+    jest.restoreAllMocks();
+    jest.clearAllMocks();
     server = createMockServer();
     config = createMockConfig();
     getClients = clientsServiceMock.createGetScoped();
     clients = clientsServiceMock.createClients();
+    clients.clusterClient.callAsCurrentUser.mockResolvedValue(getNonEmptyIndex());
+
     getClients.mockResolvedValue(clients);
 
     createRulesBulkRoute(server.route, config, getClients);
@@ -44,8 +51,12 @@ describe('create_rules_bulk', () => {
       clients.alertsClient.get.mockResolvedValue(getResult());
       clients.actionsClient.create.mockResolvedValue(createActionResult());
       clients.alertsClient.create.mockResolvedValue(getResult());
-      const { statusCode } = await server.inject(getReadBulkRequest());
+      jest.spyOn(createRules, 'createRules').mockImplementation(async () => {
+        return getResult();
+      });
+      const { payload, statusCode } = await server.inject(getReadBulkRequest());
       expect(statusCode).toBe(200);
+      expect(JSON.parse(payload).error).toBeUndefined();
     });
 
     test('returns 404 if alertClient is not available on the route', async () => {
@@ -149,6 +160,24 @@ describe('create_rules_bulk', () => {
     expect(output.some(item => item.error?.status_code === 409)).toBeTruthy();
   });
 
+  test('returns 409 if duplicate rule_ids found in rule saved objects', async () => {
+    clients.alertsClient.find.mockResolvedValue(getFindResult());
+    clients.alertsClient.get.mockResolvedValue(getResult());
+    clients.actionsClient.create.mockResolvedValue(createActionResult());
+    clients.alertsClient.create.mockResolvedValue(getResult());
+    jest.spyOn(readRules, 'readRules').mockImplementation(async () => {
+      return getResult();
+    });
+    const request: ServerInjectOptions = {
+      method: 'POST',
+      url: `${DETECTION_ENGINE_RULES_URL}/_bulk_create`,
+      payload: [typicalPayload()],
+    };
+    const { payload } = await server.inject(request);
+    const output: Array<BulkError | Partial<OutputRuleAlertRest>> = JSON.parse(payload);
+    expect(output.some(item => item.error?.status_code === 409)).toBeTruthy();
+  });
+
   test('returns one error object in response when duplicate rule_ids found in request payload', async () => {
     clients.alertsClient.find.mockResolvedValue(getFindResult());
     clients.alertsClient.get.mockResolvedValue(getResult());
@@ -163,4 +192,22 @@ describe('create_rules_bulk', () => {
     const output: Array<BulkError | Partial<OutputRuleAlertRest>> = JSON.parse(payload);
     expect(output.length).toBe(1);
   });
+
+  test('catches error if createRules throws error', async () => {
+    clients.alertsClient.find.mockResolvedValue(getFindResult());
+    clients.alertsClient.get.mockResolvedValue(getResult());
+    clients.actionsClient.create.mockResolvedValue(createActionResult());
+    clients.alertsClient.create.mockResolvedValue(getResult());
+    jest.spyOn(createRules, 'createRules').mockImplementation(async () => {
+      throw new Error('Test error');
+    });
+    const request: ServerInjectOptions = {
+      method: 'POST',
+      url: `${DETECTION_ENGINE_RULES_URL}/_bulk_create`,
+      payload: [typicalPayload()],
+    };
+    const { payload } = await server.inject(request);
+    const output: Array<BulkError | Partial<OutputRuleAlertRest>> = JSON.parse(payload);
+    expect(output[0].error.message).toBe('Test error');
+  });
 });

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/create_rules_route.test.ts

@@ -9,6 +9,9 @@ import { omit } from 'lodash/fp';
 
 import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
 import { createRulesRoute } from './create_rules_route';
+import * as createRules from '../../rules/create_rules';
+import * as readRules from '../../rules/read_rules';
+import * as utils from './utils';
 
 import {
   getFindResult,
@@ -29,8 +32,12 @@ describe('create_rules', () => {
   let clients = clientsServiceMock.createClients();
 
   beforeEach(() => {
+    // jest carries state between mocked implementations when using
+    // spyOn. So now we're doing all three of these.
+    // https://github.com/facebook/jest/issues/7136#issuecomment-565976599
     jest.resetAllMocks();
-
+    jest.restoreAllMocks();
+    jest.clearAllMocks();
     server = createMockServer();
     config = createMockConfig();
     getClients = clientsServiceMock.createGetScoped();
@@ -130,5 +137,44 @@ describe('create_rules', () => {
       const { statusCode } = await server.inject(request);
       expect(statusCode).toBe(400);
     });
+
+    test('catches error if createRules throws error', async () => {
+      clients.actionsClient.create.mockResolvedValue(createActionResult());
+      clients.alertsClient.find.mockResolvedValue(getFindResult());
+      clients.alertsClient.get.mockResolvedValue(getResult());
+      clients.alertsClient.create.mockResolvedValue(getResult());
+      clients.savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
+      jest.spyOn(createRules, 'createRules').mockImplementation(async () => {
+        throw new Error('Test error');
+      });
+      const { payload, statusCode } = await server.inject(getCreateRequest());
+      expect(JSON.parse(payload).message).toBe('Test error');
+      expect(statusCode).toBe(500);
+    });
+
+    test('catches error if transform returns null', async () => {
+      clients.actionsClient.create.mockResolvedValue(createActionResult());
+      clients.alertsClient.find.mockResolvedValue(getFindResult());
+      clients.alertsClient.get.mockResolvedValue(getResult());
+      clients.alertsClient.create.mockResolvedValue(getResult());
+      clients.savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
+      jest.spyOn(utils, 'transform').mockReturnValue(null);
+      const { payload, statusCode } = await server.inject(getCreateRequest());
+      expect(JSON.parse(payload).message).toBe('Internal error transforming rules');
+      expect(statusCode).toBe(500);
+    });
+
+    test('returns 409 if duplicate rule_ids found in rule saved objects', async () => {
+      clients.alertsClient.find.mockResolvedValue(getFindResult());
+      clients.alertsClient.get.mockResolvedValue(getResult());
+      clients.actionsClient.create.mockResolvedValue(createActionResult());
+      clients.alertsClient.create.mockResolvedValue(getResult());
+      jest.spyOn(readRules, 'readRules').mockImplementation(async () => {
+        return getResult();
+      });
+      const { payload } = await server.inject(getCreateRequest());
+      const output = JSON.parse(payload);
+      expect(output.status_code).toEqual(409);
+    });
   });
 });

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.test.ts

@@ -48,6 +48,16 @@ describe('delete_rules', () => {
       expect(statusCode).toBe(200);
     });
 
+    test('resturns 200 when deleting a single rule and related rule status', async () => {
+      clients.savedObjectsClient.find.mockResolvedValue(getFindResultStatus());
+      clients.savedObjectsClient.delete.mockResolvedValue(true);
+      clients.alertsClient.find.mockResolvedValue(getFindResultWithSingleHit());
+      clients.alertsClient.get.mockResolvedValue(getResult());
+      clients.alertsClient.delete.mockResolvedValue({});
+      const { statusCode } = await server.inject(getDeleteBulkRequest());
+      expect(statusCode).toBe(200);
+    });
+
     test('returns 200 when deleting a single rule with a valid actionClient and alertClient by alertId using POST', async () => {
       clients.alertsClient.find.mockResolvedValue(getFindResultWithSingleHit());
       clients.alertsClient.get.mockResolvedValue(getResult());

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts

@@ -58,7 +58,7 @@ export const createDeleteRulesBulkRoute = (getClients: GetScopedClients): Hapi.S
               ruleStatuses.saved_objects.forEach(async obj =>
                 savedObjectsClient.delete(ruleStatusSavedObjectType, obj.id)
               );
-              return transformOrBulkError(idOrRuleIdOrUnknown, rule);
+              return transformOrBulkError(idOrRuleIdOrUnknown, rule, ruleStatuses);
             } else {
               return getIdBulkError({ id, ruleId });
             }