[Cases] Remove case id from alerts when deleting a case (#154829)

## Summary

This PR removes the case id from all alerts attached to a case when
deleting a case. It also removes any alert authorization when removing
alerts from a case.

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

x-pack/plugins/cases/server/client/alerts/types.ts

@@ -43,7 +43,7 @@ export interface UpdateAlertCasesRequest {
   caseIds: string[];
 }
 
-export interface RemoveAlertsFromCaseRequest {
+export interface RemoveCaseIdFromAlertsRequest {
   alerts: AlertInfo[];
   caseId: string;
 }

x-pack/plugins/cases/server/client/attachments/delete.test.ts

@@ -25,10 +25,6 @@ describe('delete', () => {
       it('delete alerts correctly', async () => {
         await deleteComment({ caseID: 'mock-id-4', attachmentID: 'mock-comment-4' }, clientArgs);
 
-        expect(clientArgs.services.alertsService.ensureAlertsAuthorized).toHaveBeenCalledWith({
-          alerts: [{ id: 'test-id', index: 'test-index' }],
-        });
-
         expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).toHaveBeenCalledWith({
           alerts: [{ id: 'test-id', index: 'test-index' }],
           caseId: 'mock-id-4',
@@ -39,7 +35,6 @@ describe('delete', () => {
         clientArgs.services.attachmentService.getter.get.mockResolvedValue(commentSO);
         await deleteComment({ caseID: 'mock-id-1', attachmentID: 'mock-comment-1' }, clientArgs);
 
-        expect(clientArgs.services.alertsService.ensureAlertsAuthorized).not.toHaveBeenCalledWith();
         expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).not.toHaveBeenCalledWith();
       });
     });
@@ -66,14 +61,6 @@ describe('delete', () => {
       it('delete alerts correctly', async () => {
         await deleteAll({ caseID: 'mock-id-4' }, clientArgs);
 
-        expect(clientArgs.services.alertsService.ensureAlertsAuthorized).toHaveBeenCalledWith({
-          alerts: [
-            { id: 'test-id', index: 'test-index' },
-            { id: 'test-id-2', index: 'test-index-2' },
-            { id: 'test-id-3', index: 'test-index-3' },
-          ],
-        });
-
         expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).toHaveBeenCalledWith({
           alerts: [
             { id: 'test-id', index: 'test-index' },

x-pack/plugins/cases/server/client/attachments/delete.ts

@@ -151,6 +151,5 @@ const handleAlerts = async ({ alertsService, attachments, caseId }: HandleAlerts
   }
 
   const alerts = getAlertInfoFromComments(alertAttachments);
-  await alertsService.ensureAlertsAuthorized({ alerts });
   await alertsService.removeCaseIdFromAlerts({ alerts, caseId });
 };

x-pack/plugins/cases/server/client/cases/delete.test.ts

@@ -10,11 +10,14 @@ import { createFileServiceMock } from '@kbn/files-plugin/server/mocks';
 import type { FileJSON } from '@kbn/shared-ux-file-types';
 import type { CaseFileMetadataForDeletion } from '../../../common/files';
 import { constructFileKindIdByOwner } from '../../../common/files';
-import { getFileEntities } from './delete';
+import { getFileEntities, deleteCases } from './delete';
+import { createCasesClientMockArgs } from '../mocks';
+import { mockCases } from '../../mocks';
 
 const getCaseIds = (numIds: number) => {
   return Array.from(Array(numIds).keys()).map((key) => key.toString());
 };
+
 describe('delete', () => {
   describe('getFileEntities', () => {
     const numCaseIds = 1000;
@@ -66,6 +69,33 @@ describe('delete', () => {
       expect(entities).toEqual(expectedEntities);
     });
   });
+
+  describe('deleteCases', () => {
+    const clientArgs = createCasesClientMockArgs();
+    clientArgs.fileService.find.mockResolvedValue({ files: [], total: 0 });
+
+    clientArgs.services.caseService.getCases.mockResolvedValue({
+      saved_objects: [mockCases[0], mockCases[1]],
+    });
+
+    clientArgs.services.attachmentService.getter.getAttachmentIdsForCases.mockResolvedValue([]);
+    clientArgs.services.userActionService.getUserActionIdsForCases.mockResolvedValue([]);
+
+    beforeEach(() => {
+      jest.clearAllMocks();
+    });
+
+    describe('alerts', () => {
+      const caseIds = ['mock-id-1', 'mock-id-2'];
+
+      it('removes the case ids from all alerts', async () => {
+        await deleteCases(caseIds, clientArgs);
+        expect(clientArgs.services.alertsService.removeCaseIdsFromAllAlerts).toHaveBeenCalledWith({
+          caseIds,
+        });
+      });
+    });
+  });
 });
 
 const createMockFileJSON = (): FileJSON => {

x-pack/plugins/cases/server/client/cases/delete.ts

@@ -28,7 +28,7 @@ import { createFileEntities, deleteFiles } from '../files';
  */
 export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): Promise<void> {
   const {
-    services: { caseService, attachmentService, userActionService },
+    services: { caseService, attachmentService, userActionService, alertsService },
     logger,
     authorization,
     fileService,
@@ -75,6 +75,7 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P
         entities: bulkDeleteEntities,
         options: { refresh: 'wait_for' },
       }),
+      alertsService.removeCaseIdsFromAllAlerts({ caseIds: ids }),
     ]);
 
     await userActionService.creator.bulkAuditLogCaseDeletion(

x-pack/plugins/cases/server/services/alerts/index.test.ts

@@ -423,5 +423,47 @@ describe('updateAlertsStatus', () => {
 
       expect(alertsClient.removeCaseIdFromAlerts).not.toHaveBeenCalled();
     });
+
+    it('should not throw an error and log it', async () => {
+      alertsClient.removeCaseIdFromAlerts.mockRejectedValueOnce('An error');
+
+      await expect(
+        alertService.removeCaseIdFromAlerts({ alerts, caseId })
+      ).resolves.not.toThrowError();
+
+      expect(logger.error).toHaveBeenCalledWith(
+        'Failed removing case test-case from alerts: An error'
+      );
+    });
+  });
+
+  describe('removeCaseIdsFromAllAlerts', () => {
+    const caseIds = ['test-case-1', 'test-case-2'];
+
+    it('remove all case ids from alerts', async () => {
+      await alertService.removeCaseIdsFromAllAlerts({ caseIds });
+
+      expect(alertsClient.removeCaseIdsFromAllAlerts).toBeCalledWith({ caseIds });
+    });
+
+    it('does not call the alerts client with no case ids', async () => {
+      await alertService.removeCaseIdsFromAllAlerts({
+        caseIds: [],
+      });
+
+      expect(alertsClient.removeCaseIdsFromAllAlerts).not.toHaveBeenCalled();
+    });
+
+    it('should not throw an error and log it', async () => {
+      alertsClient.removeCaseIdsFromAllAlerts.mockRejectedValueOnce('An error');
+
+      await expect(
+        alertService.removeCaseIdsFromAllAlerts({ caseIds })
+      ).resolves.not.toThrowError();
+
+      expect(logger.error).toHaveBeenCalledWith(
+        'Failed removing cases test-case-1,test-case-2 for all alerts: An error'
+      );
+    });
   });
 });

x-pack/plugins/cases/server/services/alerts/index.ts

@@ -19,7 +19,7 @@ import { MAX_ALERTS_PER_CASE, MAX_CONCURRENT_SEARCHES } from '../../../common/co
 import { createCaseError } from '../../common/error';
 import type { AlertInfo } from '../../common/types';
 import type {
-  RemoveAlertsFromCaseRequest,
+  RemoveCaseIdFromAlertsRequest,
   UpdateAlertCasesRequest,
   UpdateAlertStatusRequest,
 } from '../../client/alerts/types';
@@ -237,7 +237,7 @@ export class AlertService {
   public async removeCaseIdFromAlerts({
     alerts,
     caseId,
-  }: RemoveAlertsFromCaseRequest): Promise<void> {
+  }: RemoveCaseIdFromAlertsRequest): Promise<void> {
     try {
       const nonEmptyAlerts = this.getNonEmptyAlerts(alerts);
 
@@ -250,11 +250,31 @@ export class AlertService {
         caseId,
       });
     } catch (error) {
-      throw createCaseError({
-        message: `Failed to remove case ${caseId} from alerts: ${error}`,
-        error,
-        logger: this.logger,
+      /**
+       * We intentionally do not throw an error.
+       * Users should be able to remove alerts from a case even
+       * in the event of an error produced by the alerts client
+       */
+      this.logger.error(`Failed removing case ${caseId} from alerts: ${error}`);
+    }
+  }
+
+  public async removeCaseIdsFromAllAlerts({ caseIds }: { caseIds: string[] }): Promise<void> {
+    try {
+      if (caseIds.length <= 0) {
+        return;
+      }
+
+      await this.alertsClient.removeCaseIdsFromAllAlerts({
+        caseIds,
       });
+    } catch (error) {
+      /**
+       * We intentionally do not throw an error.
+       * Users should be able to remove alerts from cases even
+       * in the event of an error produced by the alerts client
+       */
+      this.logger.error(`Failed removing cases ${caseIds} for all alerts: ${error}`);
     }
   }
 

x-pack/plugins/cases/server/services/mocks.ts

@@ -45,7 +45,7 @@ export type LicensingServiceMock = jest.Mocked<LicensingService>;
 export type NotificationServiceMock = jest.Mocked<EmailNotificationService>;
 
 export const createCaseServiceMock = (): CaseServiceMock => {
-  const service = {
+  const service: PublicMethodsOf<CaseServiceMock> = {
     deleteCase: jest.fn(),
     findCases: jest.fn(),
     getAllCaseComments: jest.fn(),
@@ -61,6 +61,7 @@ export const createCaseServiceMock = (): CaseServiceMock => {
     findCasesGroupedByID: jest.fn(),
     getCaseStatusStats: jest.fn(),
     executeAggregations: jest.fn(),
+    bulkDeleteCaseEntities: jest.fn(),
   };
 
   // the cast here is required because jest.Mocked tries to include private members and would throw an error
@@ -140,6 +141,7 @@ export const createAlertServiceMock = (): AlertServiceMock => {
     bulkUpdateCases: jest.fn(),
     ensureAlertsAuthorized: jest.fn(),
     removeCaseIdFromAlerts: jest.fn(),
+    removeCaseIdsFromAllAlerts: jest.fn(),
   };
 
   // the cast here is required because jest.Mocked tries to include private members and would throw an error

x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts

@@ -24,6 +24,7 @@ const createAlertsClientMock = () => {
     getAlertSummary: jest.fn(),
     ensureAllAlertsAuthorizedRead: jest.fn(),
     removeCaseIdFromAlerts: jest.fn(),
+    removeCaseIdsFromAllAlerts: jest.fn(),
   };
   return mocked;
 };

x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts

@@ -105,7 +105,7 @@ export interface BulkUpdateCasesOptions {
   caseIds: string[];
 }
 
-export interface RemoveAlertsFromCaseOptions {
+export interface RemoveCaseIdFromAlertsOptions {
   alerts: MgetAndAuditAlert[];
   caseId: string;
 }
@@ -851,33 +851,33 @@ export class AlertsClient {
     });
   }
 
-  public async removeCaseIdFromAlerts({ caseId, alerts }: RemoveAlertsFromCaseOptions) {
+  public async removeCaseIdFromAlerts({ caseId, alerts }: RemoveCaseIdFromAlertsOptions) {
+    /**
+     * We intentionally do not perform any authorization
+     * on the alerts. Users should be able to remove
+     * cases from alerts when deleting a case or an
+     * attachment
+     */
     try {
       if (alerts.length === 0) {
         return;
       }
 
-      const mgetRes = await this.ensureAllAlertsAuthorized({
-        alerts,
-        operation: ReadOperations.Get,
-      });
-
       const painlessScript = `if (ctx._source['${ALERT_CASE_IDS}'] != null) {
-        for (int i=0; i < ctx._source['${ALERT_CASE_IDS}'].length; i++) {
-            if (ctx._source['${ALERT_CASE_IDS}'][i].equals('${caseId}')) {
-                ctx._source['${ALERT_CASE_IDS}'].remove(i);
-            }
+        if (ctx._source['${ALERT_CASE_IDS}'].contains('${caseId}')) {
+          int index = ctx._source['${ALERT_CASE_IDS}'].indexOf('${caseId}');
+          ctx._source['${ALERT_CASE_IDS}'].remove(index);
         }
       }`;
 
       const bulkUpdateRequest = [];
 
-      for (const doc of mgetRes.docs) {
+      for (const alert of alerts) {
         bulkUpdateRequest.push(
           {
             update: {
-              _index: doc._index,
-              _id: doc._id,
+              _index: alert.index,
+              _id: alert.id,
             },
           },
           {
@@ -891,11 +891,61 @@ export class AlertsClient {
         body: bulkUpdateRequest,
       });
     } catch (error) {
-      this.logger.error(`Error to remove case ${caseId} from alerts: ${error}`);
+      this.logger.error(`Error removing case ${caseId} from alerts: ${error}`);
       throw error;
     }
   }
 
+  public async removeCaseIdsFromAllAlerts({ caseIds }: { caseIds: string[] }) {
+    /**
+     * We intentionally do not perform any authorization
+     * on the alerts. Users should be able to remove
+     * cases from alerts when deleting a case or an
+     * attachment
+     */
+    try {
+      if (caseIds.length === 0) {
+        return;
+      }
+
+      const index = `${this.ruleDataService.getResourcePrefix()}-*`;
+      const query = `${ALERT_CASE_IDS}: (${caseIds.join(' or ')})`;
+      const esQuery = buildEsQuery(undefined, { query, language: 'kuery' }, []);
+
+      const SCRIPT_PARAMS_ID = 'caseIds';
+
+      const painlessScript = `if (ctx._source['${ALERT_CASE_IDS}'] != null && ctx._source['${ALERT_CASE_IDS}'].length > 0 && params['${SCRIPT_PARAMS_ID}'] != null && params['${SCRIPT_PARAMS_ID}'].length > 0) {
+        List storedCaseIds = ctx._source['${ALERT_CASE_IDS}'];
+        List caseIdsToRemove = params['${SCRIPT_PARAMS_ID}'];
+
+        for (int i=0; i < caseIdsToRemove.length; i++) {
+          if (storedCaseIds.contains(caseIdsToRemove[i])) {
+            int index = storedCaseIds.indexOf(caseIdsToRemove[i]);
+            storedCaseIds.remove(index);
+          }
+        }
+      }`;
+
+      await this.esClient.updateByQuery({
+        index,
+        conflicts: 'proceed',
+        refresh: true,
+        body: {
+          script: {
+            source: painlessScript,
+            lang: 'painless',
+            params: { caseIds },
+          } as InlineScript,
+          query: esQuery,
+        },
+        ignore_unavailable: true,
+      });
+    } catch (err) {
+      this.logger.error(`Failed removing ${caseIds} from all alerts: ${err}`);
+      throw err;
+    }
+  }
+
   public async find<Params extends RuleTypeParams = never>({
     aggs,
     featureIds,