[Cases] Include rule registry client for updating alert statuses (#10…

…8588) * Trying to get import to work * Plumbed alerts client through and logging errors * No longer need the ES cluster client * Fixing types * Fixing imports * Fixing integration tests and refactoring * Throwing an error when rule registry is disabled * Reworking alert update and get to catch errors * Adding tests and fixing errors
elastic · Aug 19, 2021 · 1fd7038 · 1fd7038
1 parent 7369bdf
commit 1fd7038
Show file tree

Hide file tree

Showing 28 changed files with 5,794 additions and 542 deletions.
diff --git a/x-pack/plugins/cases/kibana.json b/x-pack/plugins/cases/kibana.json
@@ -10,6 +10,7 @@
   "id":"cases",
   "kibanaVersion":"kibana",
   "optionalPlugins":[
+    "ruleRegistry",
      "security",
      "spaces"
   ],

diff --git a/x-pack/plugins/cases/server/client/alerts/get.ts b/x-pack/plugins/cases/server/client/alerts/get.ts
@@ -12,19 +12,11 @@ export const get = async (
   { alertsInfo }: AlertGet,
   clientArgs: CasesClientArgs
 ): Promise<CasesClientGetAlertsResponse> => {
-  const { alertsService, scopedClusterClient, logger } = clientArgs;
+  const { alertsService, logger } = clientArgs;
   if (alertsInfo.length === 0) {
     return [];
   }
 
-  const alerts = await alertsService.getAlerts({ alertsInfo, scopedClusterClient, logger });
-  if (!alerts) {
-    return [];
-  }
-
-  return alerts.docs.map((alert) => ({
-    id: alert._id,
-    index: alert._index,
-    ...alert._source,
-  }));
+  const alerts = await alertsService.getAlerts({ alertsInfo, logger });
+  return alerts ?? [];
 };
diff --git a/x-pack/plugins/cases/server/client/alerts/types.ts b/x-pack/plugins/cases/server/client/alerts/types.ts
@@ -7,17 +7,7 @@
 
 import { CaseStatuses } from '../../../common/api';
 import { AlertInfo } from '../../common';
-
-interface Alert {
-  id: string;
-  index: string;
-  destination?: {
-    ip: string;
-  };
-  source?: {
-    ip: string;
-  };
-}
+import { Alert } from '../../services/alerts/types';
 
 export type CasesClientGetAlertsResponse = Alert[];
 

diff --git a/x-pack/plugins/cases/server/client/alerts/update_status.ts b/x-pack/plugins/cases/server/client/alerts/update_status.ts
@@ -16,6 +16,6 @@ export const updateStatus = async (
   { alerts }: UpdateAlertsStatusArgs,
   clientArgs: CasesClientArgs
 ): Promise<void> => {
-  const { alertsService, scopedClusterClient, logger } = clientArgs;
-  await alertsService.updateAlertsStatus({ alerts, scopedClusterClient, logger });
+  const { alertsService, logger } = clientArgs;
+  await alertsService.updateAlertsStatus({ alerts, logger });
 };
diff --git a/x-pack/plugins/cases/server/client/attachments/add.ts b/x-pack/plugins/cases/server/client/attachments/add.ts
@@ -40,12 +40,7 @@ import {
 } from '../../services/user_actions/helpers';
 
 import { AttachmentService, CasesService, CaseUserActionService } from '../../services';
-import {
-  createCaseError,
-  CommentableCase,
-  createAlertUpdateRequest,
-  isCommentRequestTypeGenAlert,
-} from '../../common';
+import { createCaseError, CommentableCase, isCommentRequestTypeGenAlert } from '../../common';
 import { CasesClientArgs, CasesClientInternal } from '..';
 
 import { decodeCommentRequest } from '../utils';
@@ -195,22 +190,9 @@ const addGeneratedAlerts = async (
       user: userDetails,
       commentReq: query,
       id: savedObjectID,
+      casesClientInternal,
     });
 
-    if (
-      (newComment.attributes.type === CommentType.alert ||
-        newComment.attributes.type === CommentType.generatedAlert) &&
-      caseInfo.attributes.settings.syncAlerts
-    ) {
-      const alertsToUpdate = createAlertUpdateRequest({
-        comment: query,
-        status: subCase.attributes.status,
-      });
-      await casesClientInternal.alerts.updateStatus({
-        alerts: alertsToUpdate,
-      });
-    }
-
     await userActionService.bulkCreate({
       unsecuredSavedObjectsClient,
       actions: [
@@ -386,19 +368,9 @@ export const addComment = async (
       user: userInfo,
       commentReq: query,
       id: savedObjectID,
+      casesClientInternal,
     });
 
-    if (newComment.attributes.type === CommentType.alert && updatedCase.settings.syncAlerts) {
-      const alertsToUpdate = createAlertUpdateRequest({
-        comment: query,
-        status: updatedCase.status,
-      });
-
-      await casesClientInternal.alerts.updateStatus({
-        alerts: alertsToUpdate,
-      });
-    }
-
     await userActionService.bulkCreate({
       unsecuredSavedObjectsClient,
       actions: [

diff --git a/x-pack/plugins/cases/server/client/cases/push.ts b/x-pack/plugins/cases/server/client/cases/push.ts
@@ -6,7 +6,7 @@
  */
 
 import Boom from '@hapi/boom';
-import { SavedObjectsFindResponse, SavedObject } from 'kibana/server';
+import { SavedObjectsFindResponse, SavedObject, Logger } from 'kibana/server';
 
 import {
   ActionConnector,
@@ -22,26 +22,16 @@ import {
 import { buildCaseUserActionItem } from '../../services/user_actions/helpers';
 
 import { createIncident, getCommentContextFromAttributes } from './utils';
-import { createCaseError, flattenCaseSavedObject, getAlertInfoFromComments } from '../../common';
+import {
+  AlertInfo,
+  createCaseError,
+  flattenCaseSavedObject,
+  getAlertInfoFromComments,
+} from '../../common';
 import { CasesClient, CasesClientArgs, CasesClientInternal } from '..';
 import { Operations } from '../../authorization';
 import { casesConnectors } from '../../connectors';
-
-/**
- * Returns true if the case should be closed based on the configuration settings and whether the case
- * is a collection. Collections are not closable because we aren't allowing their status to be changed.
- * In the future we could allow push to close all the sub cases of a collection but that's not currently supported.
- */
-function shouldCloseByPush(
-  configureSettings: SavedObjectsFindResponse<CasesConfigureAttributes>,
-  caseInfo: SavedObject<CaseAttributes>
-): boolean {
-  return (
-    configureSettings.total > 0 &&
-    configureSettings.saved_objects[0].attributes.closure_type === 'close-by-pushing' &&
-    caseInfo.attributes.type !== CaseType.collection
-  );
-}
+import { CasesClientGetAlertsResponse } from '../alerts/types';
 
 /**
  * Parameters for pushing a case to an external system
@@ -106,9 +96,7 @@ export const push = async (
 
     const alertsInfo = getAlertInfoFromComments(theCase?.comments);
 
-    const alerts = await casesClientInternal.alerts.get({
-      alertsInfo,
-    });
+    const alerts = await getAlertsCatchErrors({ casesClientInternal, alertsInfo, logger });
 
     const getMappingsResponse = await casesClientInternal.configuration.getMappings({
       connector: theCase.connector,
@@ -278,3 +266,38 @@ export const push = async (
     throw createCaseError({ message: `Failed to push case: ${error}`, error, logger });
   }
 };
+
+async function getAlertsCatchErrors({
+  casesClientInternal,
+  alertsInfo,
+  logger,
+}: {
+  casesClientInternal: CasesClientInternal;
+  alertsInfo: AlertInfo[];
+  logger: Logger;
+}): Promise<CasesClientGetAlertsResponse> {
+  try {
+    return await casesClientInternal.alerts.get({
+      alertsInfo,
+    });
+  } catch (error) {
+    logger.error(`Failed to retrieve alerts during push: ${error}`);
+    return [];
+  }
+}
+
+/**
+ * Returns true if the case should be closed based on the configuration settings and whether the case
+ * is a collection. Collections are not closable because we aren't allowing their status to be changed.
+ * In the future we could allow push to close all the sub cases of a collection but that's not currently supported.
+ */
+function shouldCloseByPush(
+  configureSettings: SavedObjectsFindResponse<CasesConfigureAttributes>,
+  caseInfo: SavedObject<CaseAttributes>
+): boolean {
+  return (
+    configureSettings.total > 0 &&
+    configureSettings.saved_objects[0].attributes.closure_type === 'close-by-pushing' &&
+    caseInfo.attributes.type !== CaseType.collection
+  );
+}
diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts
@@ -12,6 +12,7 @@ import { fold } from 'fp-ts/lib/Either';
 import { identity } from 'fp-ts/lib/function';
 
 import {
+  Logger,
   SavedObject,
   SavedObjectsClientContract,
   SavedObjectsFindResponse,
@@ -307,12 +308,14 @@ async function updateAlerts({
   caseService,
   unsecuredSavedObjectsClient,
   casesClientInternal,
+  logger,
 }: {
   casesWithSyncSettingChangedToOn: UpdateRequestWithOriginalCase[];
   casesWithStatusChangedAndSynced: UpdateRequestWithOriginalCase[];
   caseService: CasesService;
   unsecuredSavedObjectsClient: SavedObjectsClientContract;
   casesClientInternal: CasesClientInternal;
+  logger: Logger;
 }) {
   /**
    * It's possible that a case ID can appear multiple times in each array. I'm intentionally placing the status changes
@@ -361,7 +364,9 @@ async function updateAlerts({
     []
   );
 
-  await casesClientInternal.alerts.updateStatus({ alerts: alertsToUpdate });
+  await casesClientInternal.alerts.updateStatus({
+    alerts: alertsToUpdate,
+  });
 }
 
 function partitionPatchRequest(
@@ -562,15 +567,6 @@ export const update = async (
       );
     });
 
-    // Update the alert's status to match any case status or sync settings changes
-    await updateAlerts({
-      casesWithStatusChangedAndSynced,
-      casesWithSyncSettingChangedToOn,
-      caseService,
-      unsecuredSavedObjectsClient,
-      casesClientInternal,
-    });
-
     const returnUpdatedCase = myCases.saved_objects
       .filter((myCase) =>
         updatedCases.saved_objects.some((updatedCase) => updatedCase.id === myCase.id)
@@ -598,6 +594,17 @@ export const update = async (
       }),
     });
 
+    // Update the alert's status to match any case status or sync settings changes
+    // Attempt to do this after creating/changing the other entities just in case it fails
+    await updateAlerts({
+      casesWithStatusChangedAndSynced,
+      casesWithSyncSettingChangedToOn,
+      caseService,
+      unsecuredSavedObjectsClient,
+      casesClientInternal,
+      logger,
+    });
+
     return CasesResponseRt.encode(returnUpdatedCase);
   } catch (error) {
     const idVersions = cases.cases.map((caseInfo) => ({

diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts
@@ -5,12 +5,7 @@
  * 2.0.
  */
 
-import {
-  KibanaRequest,
-  SavedObjectsServiceStart,
-  Logger,
-  ElasticsearchClient,
-} from 'kibana/server';
+import { KibanaRequest, SavedObjectsServiceStart, Logger } from 'kibana/server';
 import { SecurityPluginSetup, SecurityPluginStart } from '../../../security/server';
 import { SAVED_OBJECT_TYPES } from '../../common';
 import { Authorization } from '../authorization/authorization';
@@ -25,8 +20,8 @@ import {
 } from '../services';
 import { PluginStartContract as FeaturesPluginStart } from '../../../features/server';
 import { PluginStartContract as ActionsPluginStart } from '../../../actions/server';
+import { RuleRegistryPluginStartContract } from '../../../rule_registry/server';
 import { LensServerPluginSetup } from '../../../lens/server';
-
 import { AuthorizationAuditLogger } from '../authorization';
 import { CasesClient, createCasesClient } from '.';
 
@@ -36,6 +31,7 @@ interface CasesClientFactoryArgs {
   getSpace: GetSpaceFn;
   featuresPluginStart: FeaturesPluginStart;
   actionsPluginStart: ActionsPluginStart;
+  ruleRegistryPluginStart?: RuleRegistryPluginStartContract;
   lensEmbeddableFactory: LensServerPluginSetup['lensEmbeddableFactory'];
 }
 
@@ -69,12 +65,10 @@ export class CasesClientFactory {
    */
   public async create({
     request,
-    scopedClusterClient,
     savedObjectsService,
   }: {
     request: KibanaRequest;
     savedObjectsService: SavedObjectsServiceStart;
-    scopedClusterClient: ElasticsearchClient;
   }): Promise<CasesClient> {
     if (!this.isInitialized || !this.options) {
       throw new Error('CasesClientFactory must be initialized before calling create');
@@ -94,9 +88,12 @@ export class CasesClientFactory {
     const caseService = new CasesService(this.logger, this.options?.securityPluginStart?.authc);
     const userInfo = caseService.getUser({ request });
 
+    const alertsClient = await this.options.ruleRegistryPluginStart?.getRacClientWithRequest(
+      request
+    );
+
     return createCasesClient({
-      alertsService: new AlertService(),
-      scopedClusterClient,
+      alertsService: new AlertService(alertsClient),
       unsecuredSavedObjectsClient: savedObjectsService.getScopedClient(request, {
         includedHiddenTypes: SAVED_OBJECT_TYPES,
         // this tells the security plugin to not perform SO authorization and audit logging since we are handling

diff --git a/x-pack/plugins/cases/server/client/sub_cases/update.ts b/x-pack/plugins/cases/server/client/sub_cases/update.ts
@@ -246,7 +246,9 @@ async function updateAlerts({
       []
     );
 
-    await casesClientInternal.alerts.updateStatus({ alerts: alertsToUpdate });
+    await casesClientInternal.alerts.updateStatus({
+      alerts: alertsToUpdate,
+    });
   } catch (error) {
     throw createCaseError({
       message: `Failed to update alert status while updating sub cases: ${JSON.stringify(
@@ -355,14 +357,6 @@ export async function update({
       );
     });
 
-    await updateAlerts({
-      caseService,
-      unsecuredSavedObjectsClient,
-      casesClientInternal,
-      subCasesToSync: subCasesToSyncAlertsFor,
-      logger: clientArgs.logger,
-    });
-
     const returnUpdatedSubCases = updatedCases.saved_objects.reduce<SubCaseResponse[]>(
       (acc, updatedSO) => {
         const originalSubCase = subCasesMap.get(updatedSO.id);
@@ -394,6 +388,15 @@ export async function update({
       }),
     });
 
+    // attempt to update the status of the alerts after creating all the user actions just in case it fails
+    await updateAlerts({
+      caseService,
+      unsecuredSavedObjectsClient,
+      casesClientInternal,
+      subCasesToSync: subCasesToSyncAlertsFor,
+      logger: clientArgs.logger,
+    });
+
     return SubCasesResponseRt.encode(returnUpdatedSubCases);
   } catch (error) {
     const idVersions = query.subCases.map((subCase) => ({

diff --git a/x-pack/plugins/cases/server/client/types.ts b/x-pack/plugins/cases/server/client/types.ts
@@ -6,7 +6,7 @@
  */
 
 import type { PublicMethodsOf } from '@kbn/utility-types';
-import { ElasticsearchClient, SavedObjectsClientContract, Logger } from 'kibana/server';
+import { SavedObjectsClientContract, Logger } from 'kibana/server';
 import { User } from '../../common';
 import { Authorization } from '../authorization/authorization';
 import {
@@ -24,7 +24,6 @@ import { LensServerPluginSetup } from '../../../lens/server';
  * Parameters for initializing a cases client
  */
 export interface CasesClientArgs {
-  readonly scopedClusterClient: ElasticsearchClient;
   readonly caseConfigureService: CaseConfigureService;
   readonly caseService: CasesService;
   readonly connectorMappingsService: ConnectorMappingsService;