From 7b218e8ca930f33ad879bdd4c8ad077a4ead17bc Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 29 Mar 2023 19:18:53 +0300 Subject: [PATCH 1/6] Remove alerts from case --- .../plugins/cases/common/utils/attachments.ts | 6 +++ .../cases/server/client/alerts/types.ts | 5 ++ .../cases/server/client/attachments/delete.ts | 34 ++++++++++--- .../cases/server/services/alerts/index.ts | 30 ++++++++++- .../server/alert_data_client/alerts_client.ts | 50 +++++++++++++++++++ 5 files changed, 117 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/cases/common/utils/attachments.ts b/x-pack/plugins/cases/common/utils/attachments.ts index e98df664ed2d4..b251ffdcd5a76 100644 --- a/x-pack/plugins/cases/common/utils/attachments.ts +++ b/x-pack/plugins/cases/common/utils/attachments.ts @@ -6,6 +6,8 @@ */ import type { + AttributesTypeAlerts, + CommentAttributes, CommentRequest, CommentRequestExternalReferenceType, CommentRequestPersistableStateType, @@ -29,3 +31,7 @@ export const isCommentRequestTypePersistableState = ( ): context is CommentRequestPersistableStateType => { return context.type === CommentType.persistableState; }; + +export const isAlertAttachment = ( + attachment: CommentAttributes +): attachment is AttributesTypeAlerts => attachment.type === CommentType.alert; diff --git a/x-pack/plugins/cases/server/client/alerts/types.ts b/x-pack/plugins/cases/server/client/alerts/types.ts index ed2b6626427a9..a1317106e6fe7 100644 --- a/x-pack/plugins/cases/server/client/alerts/types.ts +++ b/x-pack/plugins/cases/server/client/alerts/types.ts @@ -42,3 +42,8 @@ export interface UpdateAlertCasesRequest { alerts: AlertInfo[]; caseIds: string[]; } + +export interface RemoveAlertsFromCaseRequest { + alerts: AlertInfo[]; + caseId: string; +} diff --git a/x-pack/plugins/cases/server/client/attachments/delete.ts b/x-pack/plugins/cases/server/client/attachments/delete.ts index c1a8e019ff0f8..0d0705ad88a87 100644 --- a/x-pack/plugins/cases/server/client/attachments/delete.ts +++ b/x-pack/plugins/cases/server/client/attachments/delete.ts @@ -11,6 +11,8 @@ import pMap from 'p-map'; import type { SavedObject } from '@kbn/core/server'; import type { CommentAttributes } from '../../../common/api'; import { Actions, ActionTypes } from '../../../common/api'; +import { isAlertAttachment } from '../../../common/utils/attachments'; +import { getAlertInfoFromComments } from '../../common/utils'; import { CASE_SAVED_OBJECT, MAX_CONCURRENT_SEARCHES } from '../../../common/constants'; import type { CasesClientArgs } from '../types'; import { createCaseError } from '../../common/error'; @@ -90,29 +92,29 @@ export async function deleteComment( ) { const { user, - services: { attachmentService, userActionService }, + services: { attachmentService, userActionService, alertsService }, logger, authorization, } = clientArgs; try { - const myComment = await attachmentService.getter.get({ + const attachment = await attachmentService.getter.get({ attachmentId: attachmentID, }); - if (myComment == null) { + if (attachment == null) { throw Boom.notFound(`This comment ${attachmentID} does not exist anymore.`); } await authorization.ensureAuthorized({ - entities: [{ owner: myComment.attributes.owner, id: myComment.id }], + entities: [{ owner: attachment.attributes.owner, id: attachment.id }], operation: Operations.deleteComment, }); const type = CASE_SAVED_OBJECT; const id = caseID; - const caseRef = myComment.references.find((c) => c.type === type); + const caseRef = attachment.references.find((c) => c.type === type); if (caseRef == null || (caseRef != null && caseRef.id !== id)) { throw Boom.notFound(`This comment ${attachmentID} does not exist in ${id}.`); } @@ -127,10 +129,12 @@ export async function deleteComment( action: Actions.delete, caseId: id, attachmentId: attachmentID, - payload: { attachment: { ...myComment.attributes } }, + payload: { attachment: { ...attachment.attributes } }, user, - owner: myComment.attributes.owner, + owner: attachment.attributes.owner, }); + + await handleAlerts({ alertsService, attachment: attachment.attributes, caseId: id }); } catch (error) { throw createCaseError({ message: `Failed to delete comment: ${caseID} comment id: ${attachmentID}: ${error}`, @@ -139,3 +143,19 @@ export async function deleteComment( }); } } + +interface HandleAlertsArgs { + alertsService: CasesClientArgs['services']['alertsService']; + attachment: CommentAttributes; + caseId: string; +} + +const handleAlerts = async ({ alertsService, attachment, caseId }: HandleAlertsArgs) => { + if (!isAlertAttachment(attachment)) { + return; + } + + const alerts = getAlertInfoFromComments([attachment]); + await alertsService.ensureAlertsAuthorized({ alerts }); + await alertsService.removeAlertsFromCase({ alerts, caseId }); +}; diff --git a/x-pack/plugins/cases/server/services/alerts/index.ts b/x-pack/plugins/cases/server/services/alerts/index.ts index c79587aa57fb1..675dafe450efb 100644 --- a/x-pack/plugins/cases/server/services/alerts/index.ts +++ b/x-pack/plugins/cases/server/services/alerts/index.ts @@ -18,7 +18,11 @@ import { CaseStatuses } from '../../../common/api'; import { MAX_ALERTS_PER_CASE, MAX_CONCURRENT_SEARCHES } from '../../../common/constants'; import { createCaseError } from '../../common/error'; import type { AlertInfo } from '../../common/types'; -import type { UpdateAlertCasesRequest, UpdateAlertStatusRequest } from '../../client/alerts/types'; +import type { + RemoveAlertsFromCaseRequest, + UpdateAlertCasesRequest, + UpdateAlertStatusRequest, +} from '../../client/alerts/types'; import type { AggregationBuilder, AggregationResponse } from '../../client/metrics/types'; export class AlertService { @@ -230,6 +234,30 @@ export class AlertService { } } + public async removeAlertsFromCase({ + alerts, + caseId, + }: RemoveAlertsFromCaseRequest): Promise { + try { + const nonEmptyAlerts = this.getNonEmptyAlerts(alerts); + + if (nonEmptyAlerts.length <= 0) { + return; + } + + await this.alertsClient.removeAlertsFromCase({ + alerts: nonEmptyAlerts, + caseId, + }); + } catch (error) { + throw createCaseError({ + message: `Failed to remove alerts from case ${caseId}: ${error}`, + error, + logger: this.logger, + }); + } + } + public async ensureAlertsAuthorized({ alerts }: { alerts: AlertInfo[] }): Promise { try { const nonEmptyAlerts = this.getNonEmptyAlerts(alerts); diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts index a5389b013129f..cc5d3b10d5bba 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts @@ -105,6 +105,11 @@ export interface BulkUpdateCasesOptions { caseIds: string[]; } +export interface RemoveAlertsFromCaseOptions { + alerts: MgetAndAuditAlert[]; + caseId: string; +} + interface GetAlertParams { id: string; index?: string; @@ -846,6 +851,51 @@ export class AlertsClient { }); } + public async removeAlertsFromCase({ caseId, alerts }: RemoveAlertsFromCaseOptions) { + try { + if (alerts.length === 0) { + return; + } + + const mgetRes = await this.ensureAllAlertsAuthorized({ + alerts, + operation: ReadOperations.Get, + }); + + const painlessScript = `if (ctx._source['${ALERT_CASE_IDS}'] != null && ctx._source['${ALERT_CASE_IDS}'].length > 0) { + for (int i=0; i < ctx._source['${ALERT_CASE_IDS}'].length; i++) { + if (ctx._source['${ALERT_CASE_IDS}'][i] == '${caseId}') { + ctx._source['${ALERT_CASE_IDS}'].remove(i); + } + } + }`; + + const bulkUpdateRequest = []; + + for (const doc of mgetRes.docs) { + bulkUpdateRequest.push( + { + update: { + _index: doc._index, + _id: doc._id, + }, + }, + { + script: { source: painlessScript, lang: 'painless' }, + } + ); + } + + await this.esClient.bulk({ + refresh: 'wait_for', + body: bulkUpdateRequest, + }); + } catch (error) { + this.logger.error(`Error removing alerts from case ${caseId}: ${error}`); + throw error; + } + } + public async find({ aggs, featureIds, From c7e2d82c8e05d1101b5289459baf58acf2ffbe1b Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 30 Mar 2023 12:50:29 +0300 Subject: [PATCH 2/6] Add tests --- .../server/client/attachments/delete.test.ts | 46 +++ .../server/services/alerts/index.test.ts | 43 +++ x-pack/plugins/cases/server/services/mocks.ts | 1 + .../alert_data_client/alerts_client.mock.ts | 1 + .../tests/bulk_update_cases.test.ts | 304 ++++++++++++++++++ .../tests/remove_alerts_from_case.test.ts | 109 +++++++ 6 files changed, 504 insertions(+) create mode 100644 x-pack/plugins/cases/server/client/attachments/delete.test.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update_cases.test.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts diff --git a/x-pack/plugins/cases/server/client/attachments/delete.test.ts b/x-pack/plugins/cases/server/client/attachments/delete.test.ts new file mode 100644 index 0000000000000..89db8b4f6f2b7 --- /dev/null +++ b/x-pack/plugins/cases/server/client/attachments/delete.test.ts @@ -0,0 +1,46 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { mockCaseComments } from '../../mocks'; +import { createCasesClientMockArgs } from '../mocks'; +import { deleteComment } from './delete'; + +describe('deleteComment', () => { + const clientArgs = createCasesClientMockArgs(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('Alerts', () => { + const commentSO = mockCaseComments[0]; + const alertsSO = mockCaseComments[3]; + clientArgs.services.attachmentService.getter.get.mockResolvedValue(alertsSO); + + 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.removeAlertsFromCase).toHaveBeenCalledWith({ + alerts: [{ id: 'test-id', index: 'test-index' }], + caseId: 'mock-id-4', + }); + }); + + it('does not call the alert service when the attachment is not an alert', async () => { + 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.removeAlertsFromCase).not.toHaveBeenCalledWith(); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/services/alerts/index.test.ts b/x-pack/plugins/cases/server/services/alerts/index.test.ts index 6bc943ab3f0d5..2d34ea0dec6f8 100644 --- a/x-pack/plugins/cases/server/services/alerts/index.test.ts +++ b/x-pack/plugins/cases/server/services/alerts/index.test.ts @@ -381,4 +381,47 @@ describe('updateAlertsStatus', () => { expect(alertsClient.bulkUpdateCases).not.toHaveBeenCalled(); }); }); + + describe('removeAlertsFromCase', () => { + const alerts = [ + { + id: 'c3869d546717e8c581add9cbf7d24578f34cd3e72cbc8d8b8e9a9330a899f70f', + index: '.internal.alerts-security.alerts-default-000001', + }, + ]; + const caseId = 'test-case'; + + it('update case info', async () => { + await alertService.removeAlertsFromCase({ alerts, caseId }); + + expect(alertsClient.removeAlertsFromCase).toBeCalledWith({ alerts, caseId }); + }); + + it('filters out alerts with empty id', async () => { + await alertService.removeAlertsFromCase({ + alerts: [{ id: '', index: 'test-index' }, ...alerts], + caseId, + }); + + expect(alertsClient.removeAlertsFromCase).toBeCalledWith({ alerts, caseId }); + }); + + it('filters out alerts with empty index', async () => { + await alertService.removeAlertsFromCase({ + alerts: [{ id: 'test-id', index: '' }, ...alerts], + caseId, + }); + + expect(alertsClient.removeAlertsFromCase).toBeCalledWith({ alerts, caseId }); + }); + + it('does not call the alerts client with no alerts', async () => { + await alertService.removeAlertsFromCase({ + alerts: [{ id: '', index: 'test-index' }], + caseId, + }); + + expect(alertsClient.removeAlertsFromCase).not.toHaveBeenCalled(); + }); + }); }); diff --git a/x-pack/plugins/cases/server/services/mocks.ts b/x-pack/plugins/cases/server/services/mocks.ts index 380e10745b903..7517ca2fc205d 100644 --- a/x-pack/plugins/cases/server/services/mocks.ts +++ b/x-pack/plugins/cases/server/services/mocks.ts @@ -139,6 +139,7 @@ export const createAlertServiceMock = (): AlertServiceMock => { executeAggregations: jest.fn(), bulkUpdateCases: jest.fn(), ensureAlertsAuthorized: jest.fn(), + removeAlertsFromCase: jest.fn(), }; // the cast here is required because jest.Mocked tries to include private members and would throw an error diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts index 74ea0225614b0..a25b5a1b36ef5 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts @@ -23,6 +23,7 @@ const createAlertsClientMock = () => { getBrowserFields: jest.fn(), getAlertSummary: jest.fn(), ensureAllAlertsAuthorizedRead: jest.fn(), + removeAlertsFromCase: jest.fn(), }; return mocked; }; diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update_cases.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update_cases.test.ts new file mode 100644 index 0000000000000..f8d4b1a2c3e69 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update_cases.test.ts @@ -0,0 +1,304 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { loggingSystemMock } from '@kbn/core/server/mocks'; +import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks'; +import { alertingAuthorizationMock } from '@kbn/alerting-plugin/server/authorization/alerting_authorization.mock'; +import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; +import { + ALERT_CASE_IDS, + ALERT_RULE_CONSUMER, + ALERT_RULE_TYPE_ID, + MAX_CASES_PER_ALERT, +} from '@kbn/rule-data-utils'; +import { AlertsClient, ConstructorOptions } from '../alerts_client'; +import { ruleDataServiceMock } from '../../rule_data_plugin_service/rule_data_plugin_service.mock'; + +describe('bulkUpdateCases', () => { + const alertingAuthMock = alertingAuthorizationMock.create(); + const esClientMock = elasticsearchClientMock.createElasticsearchClient(); + const auditLogger = auditLoggerMock.create(); + const caseIds = ['test-case']; + const alerts = [ + { + id: 'alert-id', + index: 'alert-index', + }, + ]; + + const alertsClientParams: jest.Mocked = { + logger: loggingSystemMock.create().get(), + authorization: alertingAuthMock, + esClient: esClientMock, + auditLogger, + ruleDataService: ruleDataServiceMock.create(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + + esClientMock.mget.mockResponse({ + docs: [ + { + found: true, + _id: 'alert-id', + _index: 'alert-index', + _source: { + [ALERT_RULE_TYPE_ID]: 'apm.error_rate', + [ALERT_RULE_CONSUMER]: 'apm', + [ALERT_CASE_IDS]: caseIds, + }, + }, + ], + }); + }); + + it('bulks update the alerts with case info correctly', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + await alertsClient.bulkUpdateCases({ caseIds, alerts }); + + expect(esClientMock.mget).toHaveBeenCalledWith({ + docs: [{ _id: 'alert-id', _index: 'alert-index' }], + }); + + expect(esClientMock.bulk.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "body": Array [ + Object { + "update": Object { + "_id": "alert-id", + "_index": "alert-index", + }, + }, + Object { + "doc": Object { + "kibana.alert.case_ids": Array [ + "test-case", + ], + }, + }, + ], + "refresh": "wait_for", + } + `); + }); + + it('bulks update correctly with multiple cases and alerts', async () => { + const multipleAlerts = [ + ...alerts, + { + id: 'alert-id-2', + index: 'alert-index-2', + }, + ]; + + const multipleCases = [...caseIds, 'another-case']; + + esClientMock.mget.mockResponse({ + docs: multipleAlerts.map((alert) => ({ + found: true, + _id: alert.id, + _index: alert.index, + _source: { + [ALERT_RULE_TYPE_ID]: 'apm.error_rate', + [ALERT_RULE_CONSUMER]: 'apm', + [ALERT_CASE_IDS]: multipleCases, + }, + })), + }); + + const alertsClient = new AlertsClient(alertsClientParams); + + await alertsClient.bulkUpdateCases({ + caseIds: multipleCases, + alerts: multipleAlerts, + }); + + expect(esClientMock.mget).toHaveBeenCalledWith({ + docs: [ + { _id: 'alert-id', _index: 'alert-index' }, + { _id: 'alert-id-2', _index: 'alert-index-2' }, + ], + }); + + expect(esClientMock.bulk.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "body": Array [ + Object { + "update": Object { + "_id": "alert-id", + "_index": "alert-index", + }, + }, + Object { + "doc": Object { + "kibana.alert.case_ids": Array [ + "test-case", + "another-case", + ], + }, + }, + Object { + "update": Object { + "_id": "alert-id-2", + "_index": "alert-index-2", + }, + }, + Object { + "doc": Object { + "kibana.alert.case_ids": Array [ + "test-case", + "another-case", + ], + }, + }, + ], + "refresh": "wait_for", + } + `); + }); + + it('removes duplicated cases', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + + await alertsClient.bulkUpdateCases({ + caseIds: [...caseIds, ...caseIds], + alerts, + }); + + expect(esClientMock.bulk.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "body": Array [ + Object { + "update": Object { + "_id": "alert-id", + "_index": "alert-index", + }, + }, + Object { + "doc": Object { + "kibana.alert.case_ids": Array [ + "test-case", + ], + }, + }, + ], + "refresh": "wait_for", + } + `); + }); + + it('calls ensureAllAlertsAuthorized correctly', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + await alertsClient.bulkUpdateCases({ caseIds, alerts }); + + expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ + consumer: 'apm', + entity: 'alert', + operation: 'get', + ruleTypeId: 'apm.error_rate', + }); + }); + + it(`throws an error when adding a case to an alert with ${MAX_CASES_PER_ALERT} cases`, async () => { + esClientMock.mget.mockResponse({ + docs: [ + { + found: true, + _id: 'alert-id', + _index: 'alert-index', + _source: { + [ALERT_RULE_TYPE_ID]: 'apm.error_rate', + [ALERT_RULE_CONSUMER]: 'apm', + [ALERT_CASE_IDS]: Array.from(Array(10).keys()), + }, + }, + ], + }); + + const alertsClient = new AlertsClient(alertsClientParams); + + await expect( + alertsClient.bulkUpdateCases({ caseIds, alerts }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"You cannot attach more than 10 cases to an alert"` + ); + }); + + it(`throws an error when adding more than ${MAX_CASES_PER_ALERT} cases to an alert`, async () => { + esClientMock.mget.mockResponse({ + docs: [ + { + found: true, + _id: 'alert-id', + _index: 'alert-index', + _source: { + [ALERT_RULE_TYPE_ID]: 'apm.error_rate', + [ALERT_RULE_CONSUMER]: 'apm', + [ALERT_CASE_IDS]: [], + }, + }, + ], + }); + + const alertsClient = new AlertsClient(alertsClientParams); + const multipleCases = Array.from(Array(11).values()); + + await expect( + alertsClient.bulkUpdateCases({ caseIds: multipleCases, alerts }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"You cannot attach more than 10 cases to an alert"` + ); + }); + + it(`throws an error when the sum of cases extends the limit of ${MAX_CASES_PER_ALERT} cases per alert`, async () => { + esClientMock.mget.mockResponse({ + docs: [ + { + found: true, + _id: 'alert-id', + _index: 'alert-index', + _source: { + [ALERT_RULE_TYPE_ID]: 'apm.error_rate', + [ALERT_RULE_CONSUMER]: 'apm', + [ALERT_CASE_IDS]: Array.from(Array(5).keys()), + }, + }, + ], + }); + + const alertsClient = new AlertsClient(alertsClientParams); + const multipleCases = Array.from(Array(6).values()); + + await expect( + alertsClient.bulkUpdateCases({ caseIds: multipleCases, alerts }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"You cannot attach more than 10 cases to an alert"` + ); + }); + + it('throws an error when no alerts are provided', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + + await expect( + alertsClient.bulkUpdateCases({ caseIds, alerts: [] }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"You need to define at least one alert to update case ids"` + ); + }); + + it(`throws an error when the provided case ids are more than ${MAX_CASES_PER_ALERT}`, async () => { + const alertsClient = new AlertsClient(alertsClientParams); + const multipleCases = Array.from(Array(11).values()); + + await expect( + alertsClient.bulkUpdateCases({ caseIds: multipleCases, alerts }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"You cannot attach more than 10 cases to an alert"` + ); + }); +}); diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts new file mode 100644 index 0000000000000..52d67315e23ea --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts @@ -0,0 +1,109 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { loggingSystemMock } from '@kbn/core/server/mocks'; +import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks'; +import { alertingAuthorizationMock } from '@kbn/alerting-plugin/server/authorization/alerting_authorization.mock'; +import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; +import { ALERT_CASE_IDS, ALERT_RULE_CONSUMER, ALERT_RULE_TYPE_ID } from '@kbn/rule-data-utils'; +import { AlertsClient, ConstructorOptions } from '../alerts_client'; +import { ruleDataServiceMock } from '../../rule_data_plugin_service/rule_data_plugin_service.mock'; + +describe('removeAlertsFromCase', () => { + const alertingAuthMock = alertingAuthorizationMock.create(); + const esClientMock = elasticsearchClientMock.createElasticsearchClient(); + const auditLogger = auditLoggerMock.create(); + const caseId = 'test-case'; + const alerts = [ + { + id: 'alert-id', + index: 'alert-index', + }, + ]; + + const alertsClientParams: jest.Mocked = { + logger: loggingSystemMock.create().get(), + authorization: alertingAuthMock, + esClient: esClientMock, + auditLogger, + ruleDataService: ruleDataServiceMock.create(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + + esClientMock.mget.mockResponseOnce({ + docs: [ + { + found: true, + _id: 'alert-id', + _index: 'alert-index', + _source: { + [ALERT_RULE_TYPE_ID]: 'apm.error_rate', + [ALERT_RULE_CONSUMER]: 'apm', + [ALERT_CASE_IDS]: [caseId], + }, + }, + ], + }); + }); + + it('removes alerts from a case', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + await alertsClient.removeAlertsFromCase({ caseId, alerts }); + + expect(esClientMock.mget).toHaveBeenCalledWith({ + docs: [{ _id: 'alert-id', _index: 'alert-index' }], + }); + + expect(esClientMock.bulk.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "body": Array [ + Object { + "update": Object { + "_id": "alert-id", + "_index": "alert-index", + }, + }, + Object { + "script": Object { + "lang": "painless", + "source": "if (ctx._source['kibana.alert.case_ids'] != null && ctx._source['kibana.alert.case_ids'].length > 0) { + for (int i=0; i < ctx._source['kibana.alert.case_ids'].length; i++) { + if (ctx._source['kibana.alert.case_ids'][i] == 'test-case') { + ctx._source['kibana.alert.case_ids'].remove(i); + } + } + }", + }, + }, + ], + "refresh": "wait_for", + } + `); + }); + + it('calls ensureAllAlertsAuthorized correctly', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + await alertsClient.removeAlertsFromCase({ caseId, alerts }); + + expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ + consumer: 'apm', + entity: 'alert', + operation: 'get', + ruleTypeId: 'apm.error_rate', + }); + }); + + it('does not do any calls if there are no alerts', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + await alertsClient.removeAlertsFromCase({ caseId, alerts: [] }); + + expect(alertingAuthMock.ensureAuthorized).not.toHaveBeenCalled(); + expect(esClientMock.bulk).not.toHaveBeenCalled(); + }); +}); From 47358684b5ff9d9f95836a820b54ee6ce84fba53 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 30 Mar 2023 17:25:58 +0300 Subject: [PATCH 3/6] Add integration tests --- .../tests/common/comments/delete_comment.ts | 314 +++++++++++++++++- .../tests/common/comments/post_comment.ts | 4 +- 2 files changed, 314 insertions(+), 4 deletions(-) diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts index 68a52c13ecbf0..e68727ab8d25a 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts @@ -6,7 +6,20 @@ */ import expect from '@kbn/expect'; +import { CommentType } from '@kbn/cases-plugin/common'; +import { ALERT_CASE_IDS } from '@kbn/rule-data-utils'; +import { + createSecuritySolutionAlerts, + getAlertById, + getSecuritySolutionAlerts, +} from '../../../../common/lib/alerts'; +import { + createSignalsIndex, + deleteSignalsIndex, + deleteAllRules, +} from '../../../../../detection_engine_api_integration/utils'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +import { User } from '../../../../common/lib/authentication/types'; import { getPostCaseRequest, postCaseReq, postCommentUserReq } from '../../../../common/lib/mock'; import { @@ -25,9 +38,13 @@ import { noKibanaPrivileges, obsOnly, obsOnlyRead, + obsOnlyReadAlerts, + obsSec, obsSecRead, secOnly, secOnlyRead, + secOnlyReadAlerts, + secSolutionOnlyReadNoIndexAlerts, superUser, } from '../../../../common/lib/authentication/users'; @@ -35,6 +52,9 @@ import { export default ({ getService }: FtrProviderContext): void => { const supertest = getService('supertest'); const es = getService('es'); + const esArchiver = getService('esArchiver'); + const log = getService('log'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); describe('delete_comment', () => { afterEach(async () => { @@ -91,9 +111,299 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - describe('rbac', () => { - const supertestWithoutAuth = getService('supertestWithoutAuth'); + describe('alerts', () => { + describe('security_solution', () => { + const createCaseAttachAlertAndDeleteAlert = async ({ + totalCases, + indexOfCaseToDelete, + expectedHttpCode = 204, + auth = { user: superUser, space: 'space1' }, + }: { + totalCases: number; + indexOfCaseToDelete: number; + expectedHttpCode?: number; + auth?: { user: User; space: string | null }; + }) => { + const cases = await Promise.all( + [...Array(totalCases).keys()].map((index) => + createCase( + supertestWithoutAuth, + { + ...postCaseReq, + settings: { syncAlerts: false }, + }, + 200, + { user: superUser, space: 'space1' } + ) + ) + ); + + const signals = await createSecuritySolutionAlerts(supertest, log); + const alerts = [signals.hits.hits[0], signals.hits.hits[1]]; + const updatedCases = []; + + for (const theCase of cases) { + const updatedCase = await createComment({ + supertest: supertestWithoutAuth, + caseId: theCase.id, + params: { + alertId: alerts.map((alert) => alert._id), + index: alerts.map((alert) => alert._index), + rule: { + id: 'id', + name: 'name', + }, + owner: 'securitySolutionFixture', + type: CommentType.alert, + }, + auth: { user: superUser, space: 'space1' }, + }); + + updatedCases.push(updatedCase); + } + + const caseIds = updatedCases.map((theCase) => theCase.id); + + await es.indices.refresh({ index: alerts.map((alert) => alert._index) }); + const updatedAlerts = await getSecuritySolutionAlerts( + supertest, + alerts.map((alert) => alert._id) + ); + + for (const alert of updatedAlerts.hits.hits) { + expect(alert._source?.[ALERT_CASE_IDS]).eql(caseIds); + } + + const caseToDelete = updatedCases[indexOfCaseToDelete]; + const commentId = caseToDelete.comments![0].id; + + await deleteComment({ + supertest: supertestWithoutAuth, + caseId: caseToDelete.id, + commentId, + expectedHttpCode, + auth, + }); + + await es.indices.refresh({ index: updatedAlerts.hits.hits.map((alert) => alert._index) }); + + const alertAfterDeletion = await getSecuritySolutionAlerts( + supertest, + updatedAlerts.hits.hits.map((alert) => alert._id) + ); + + const caseIdsWithoutRemovedCase = + expectedHttpCode === 204 + ? updatedCases + .filter((theCase) => theCase.id !== caseToDelete.id) + .map((theCase) => theCase.id) + : updatedCases.map((theCase) => theCase.id); + + for (const alert of alertAfterDeletion.hits.hits) { + expect(alert._source?.[ALERT_CASE_IDS]).eql(caseIdsWithoutRemovedCase); + } + }; + + beforeEach(async () => { + await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts'); + await createSignalsIndex(supertest, log); + }); + + afterEach(async () => { + await deleteSignalsIndex(supertest, log); + await deleteAllRules(supertest, log); + await esArchiver.unload('x-pack/test/functional/es_archives/auditbeat/hosts'); + }); + + it('removes a case from the alert schema when deleting an alert attachment', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + }); + }); + + it('should remove only one case', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 3, + indexOfCaseToDelete: 1, + }); + }); + + it('should delete case ID from the alert schema when the user has write access to the indices and only read access to the siem solution', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + expectedHttpCode: 204, + auth: { user: secOnlyReadAlerts, space: 'space1' }, + }); + }); + + it('should NOT delete case ID from the alert schema when the user does NOT have access to the alert', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + expectedHttpCode: 403, + auth: { user: obsSec, space: 'space1' }, + }); + }); + + it('should delete the case ID from the alert schema when the user has read access to the kibana feature but no read access to the ES index', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + expectedHttpCode: 204, + auth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, + }); + }); + }); + + describe('observability', () => { + const createCaseAttachAlertAndDeleteAlert = async ({ + totalCases, + indexOfCaseToDelete, + expectedHttpCode = 204, + auth = { user: superUser, space: 'space1' }, + }: { + totalCases: number; + indexOfCaseToDelete: number; + expectedHttpCode?: number; + auth?: { user: User; space: string | null }; + }) => { + const cases = await Promise.all( + [...Array(totalCases).keys()].map((index) => + createCase( + supertestWithoutAuth, + { + ...postCaseReq, + owner: 'observabilityFixture', + settings: { syncAlerts: false }, + }, + 200, + { user: superUser, space: 'space1' } + ) + ) + ); + + const alerts = [ + { _id: 'NoxgpHkBqbdrfX07MqXV', _index: '.alerts-observability.apm.alerts' }, + { _id: 'space1alert', _index: '.alerts-observability.apm.alerts' }, + ]; + const updatedCases = []; + + for (const theCase of cases) { + const updatedCase = await createComment({ + supertest: supertestWithoutAuth, + caseId: theCase.id, + params: { + alertId: alerts.map((alert) => alert._id), + index: alerts.map((alert) => alert._index), + rule: { + id: 'id', + name: 'name', + }, + owner: 'observabilityFixture', + type: CommentType.alert, + }, + auth: { user: superUser, space: 'space1' }, + }); + + updatedCases.push(updatedCase); + } + + const caseIds = updatedCases.map((theCase) => theCase.id); + + const updatedAlerts = await Promise.all( + alerts.map((alert) => + getAlertById({ + supertest: supertestWithoutAuth, + id: alert._id, + index: alert._index, + auth: { user: superUser, space: 'space1' }, + }) + ) + ); + + for (const alert of updatedAlerts) { + expect(alert[ALERT_CASE_IDS]).eql(caseIds); + } + + const caseToDelete = updatedCases[indexOfCaseToDelete]; + const commentId = caseToDelete.comments![0].id; + + await deleteComment({ + supertest: supertestWithoutAuth, + caseId: caseToDelete.id, + commentId, + expectedHttpCode, + auth, + }); + + const alertAfterDeletion = await Promise.all( + alerts.map((alert) => + getAlertById({ + supertest: supertestWithoutAuth, + id: alert._id, + index: alert._index, + auth: { user: superUser, space: 'space1' }, + }) + ) + ); + + const caseIdsWithoutRemovedCase = + expectedHttpCode === 204 + ? updatedCases + .filter((theCase) => theCase.id !== caseToDelete.id) + .map((theCase) => theCase.id) + : updatedCases.map((theCase) => theCase.id); + for (const alert of alertAfterDeletion) { + expect(alert[ALERT_CASE_IDS]).eql(caseIdsWithoutRemovedCase); + } + }; + + beforeEach(async () => { + await esArchiver.load('x-pack/test/functional/es_archives/rule_registry/alerts'); + }); + + afterEach(async () => { + await esArchiver.unload('x-pack/test/functional/es_archives/rule_registry/alerts'); + }); + + it('removes a case from the alert schema when deleting an alert attachment', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + }); + }); + + it('should remove only one case', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 3, + indexOfCaseToDelete: 1, + }); + }); + + it('should delete case ID from the alert schema when the user has read access only', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + expectedHttpCode: 204, + auth: { user: obsOnlyReadAlerts, space: 'space1' }, + }); + }); + + it('should NOT delete case ID from the alert schema when the user does NOT have access to the alert', async () => { + await createCaseAttachAlertAndDeleteAlert({ + totalCases: 1, + indexOfCaseToDelete: 0, + expectedHttpCode: 403, + auth: { user: obsSec, space: 'space1' }, + }); + }); + }); + }); + + describe('rbac', () => { afterEach(async () => { await deleteAllCaseItems(es); }); diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts index d3e864374d31e..b04ee89675472 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts @@ -812,7 +812,7 @@ export default ({ getService }: FtrProviderContext): void => { const caseIds = cases.map((theCase) => theCase.id); - expect(alert['kibana.alert.case_ids']).eql(caseIds); + expect(alert[ALERT_CASE_IDS]).eql(caseIds); return { alert, cases }; }; @@ -851,7 +851,7 @@ export default ({ getService }: FtrProviderContext): void => { auth: { user: superUser, space: 'space1' }, }); - expect(alert['kibana.alert.case_ids']).eql([postedCase.id]); + expect(alert[ALERT_CASE_IDS]).eql([postedCase.id]); }); it('should not add more than 10 cases to an alert', async () => { From 476502bab6491bfb81604ca314807fc8e62df907 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 30 Mar 2023 18:00:11 +0300 Subject: [PATCH 4/6] Show a success toaster when deleting attachments --- .../components/user_actions/comment/alert.tsx | 7 +- .../user_actions/comment/comment.test.tsx | 21 ++++-- .../comment/registered_attachments.tsx | 8 ++- .../user_actions/comment/translations.tsx | 21 ++++++ .../components/user_actions/comment/user.tsx | 2 +- .../public/components/user_actions/types.ts | 2 +- .../user_actions/use_user_actions_handler.tsx | 4 +- .../containers/use_delete_comment.test.tsx | 68 ++++++++++++------- .../public/containers/use_delete_comment.tsx | 6 +- 9 files changed, 98 insertions(+), 41 deletions(-) diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/alert.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/alert.tsx index 7dd9f6283ce2a..3cc06cceab921 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/alert.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/alert.tsx @@ -21,6 +21,7 @@ import { ShowAlertTableLink } from './show_alert_table_link'; import { HoverableUserWithAvatarResolver } from '../../user_profiles/hoverable_user_with_avatar_resolver'; import { UserActionContentToolbar } from '../content_toolbar'; import { AlertPropertyActions } from '../property_actions/alert_property_actions'; +import { DELETE_ALERTS_SUCCESS_TITLE } from './translations'; type BuilderArgs = Pick< UserActionBuilderArgs, @@ -88,7 +89,7 @@ const getSingleAlertUserAction = ({ /> handleDeleteComment(comment.id)} + onDelete={() => handleDeleteComment(comment.id, DELETE_ALERTS_SUCCESS_TITLE(1))} isLoading={loadingCommentIds.includes(comment.id)} totalAlerts={1} /> @@ -142,7 +143,9 @@ const getMultipleAlertsUserAction = ({ handleDeleteComment(comment.id)} + onDelete={() => + handleDeleteComment(comment.id, DELETE_ALERTS_SUCCESS_TITLE(totalAlerts)) + } isLoading={loadingCommentIds.includes(comment.id)} totalAlerts={totalAlerts} /> diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/comment.test.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/comment.test.tsx index 2ecc4b32d0837..92942caf936ac 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/comment.test.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/comment.test.tsx @@ -201,7 +201,10 @@ describe('createCommentUserActionBuilder', () => { await deleteAttachment(result, 'trash', 'Delete'); await waitFor(() => { - expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith('basic-comment-id'); + expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith( + 'basic-comment-id', + 'Deleted comment' + ); }); }); @@ -319,7 +322,10 @@ describe('createCommentUserActionBuilder', () => { await deleteAttachment(res, 'minusInCircle', 'Remove'); await waitFor(() => { - expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith('alert-comment-id'); + expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith( + 'alert-comment-id', + 'Deleted one alert' + ); }); }); @@ -414,7 +420,10 @@ describe('createCommentUserActionBuilder', () => { await deleteAttachment(res, 'minusInCircle', 'Remove'); await waitFor(() => { - expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith('alert-comment-id'); + expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith( + 'alert-comment-id', + 'Deleted 2 alerts' + ); }); }); @@ -590,7 +599,8 @@ describe('createCommentUserActionBuilder', () => { await waitFor(() => { expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith( - 'external-reference-comment-id' + 'external-reference-comment-id', + 'Deleted attachment' ); }); }); @@ -761,7 +771,8 @@ describe('createCommentUserActionBuilder', () => { await waitFor(() => { expect(builderArgs.handleDeleteComment).toHaveBeenCalledWith( - 'persistable-state-comment-id' + 'persistable-state-comment-id', + 'Deleted attachment' ); }); }); diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/registered_attachments.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/registered_attachments.tsx index 0b314aeb0de1e..732afcd988f49 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/registered_attachments.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/registered_attachments.tsx @@ -21,7 +21,11 @@ import type { CommentResponse } from '../../../../common/api'; import type { UserActionBuilder, UserActionBuilderArgs } from '../types'; import { UserActionTimestamp } from '../timestamp'; import type { SnakeToCamelCase } from '../../../../common/types'; -import { ATTACHMENT_NOT_REGISTERED_ERROR, DEFAULT_EVENT_ATTACHMENT_TITLE } from './translations'; +import { + ATTACHMENT_NOT_REGISTERED_ERROR, + DEFAULT_EVENT_ATTACHMENT_TITLE, + DELETE_REGISTERED_ATTACHMENT, +} from './translations'; import { UserActionContentToolbar } from '../content_toolbar'; import { HoverableUserWithAvatarResolver } from '../../user_profiles/hoverable_user_with_avatar_resolver'; import { RegisteredAttachmentsPropertyActions } from '../property_actions/registered_attachments_property_actions'; @@ -127,7 +131,7 @@ export const createRegisteredAttachmentUserActionBuilder = < {attachmentViewObject.actions} handleDeleteComment(comment.id)} + onDelete={() => handleDeleteComment(comment.id, DELETE_REGISTERED_ATTACHMENT)} /> ), diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/translations.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/translations.tsx index 43f04127a88c3..6c57e7fd173bf 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/translations.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/translations.tsx @@ -39,3 +39,24 @@ export const UNSAVED_DRAFT_COMMENT = i18n.translate( defaultMessage: 'You have unsaved edits for this comment', } ); + +export const DELETE_ALERTS_SUCCESS_TITLE = (totalAlerts: number) => + i18n.translate('xpack.cases.userActions.attachments.alerts.successToasterTitle', { + defaultMessage: + 'Deleted {totalAlerts, plural, =1 {one} other {{totalAlerts}}} {totalAlerts, plural, =1 {alert} other {alerts}}', + values: { totalAlerts }, + }); + +export const DELETE_COMMENT_SUCCESS_TITLE = i18n.translate( + 'xpack.cases.userActions.attachments.comment.successToasterTitle', + { + defaultMessage: 'Deleted comment', + } +); + +export const DELETE_REGISTERED_ATTACHMENT = i18n.translate( + 'xpack.cases.userActions.attachments.registeredAttachment.successToasterTitle', + { + defaultMessage: 'Deleted attachment', + } +); diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx index 76ce8e4070b48..0c40f738c56b6 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx @@ -115,7 +115,7 @@ export const createUserAttachmentUserActionBuilder = ({ isLoading={isLoading} commentContent={comment.comment} onEdit={() => handleManageMarkdownEditId(comment.id)} - onDelete={() => handleDeleteComment(comment.id)} + onDelete={() => handleDeleteComment(comment.id, i18n.DELETE_COMMENT_SUCCESS_TITLE)} onQuote={() => handleManageQuote(comment.comment)} /> diff --git a/x-pack/plugins/cases/public/components/user_actions/types.ts b/x-pack/plugins/cases/public/components/user_actions/types.ts index c1e1562b8233a..a1e02fb90d9a1 100644 --- a/x-pack/plugins/cases/public/components/user_actions/types.ts +++ b/x-pack/plugins/cases/public/components/user_actions/types.ts @@ -69,7 +69,7 @@ export interface UserActionBuilderArgs { handleOutlineComment: (id: string) => void; handleManageMarkdownEditId: (id: string) => void; handleSaveComment: ({ id, version }: { id: string; version: string }, content: string) => void; - handleDeleteComment: (id: string) => void; + handleDeleteComment: (id: string, successToasterTitle: string) => void; handleManageQuote: (quote: string) => void; onShowAlertDetails: (alertId: string, index: string) => void; getRuleDetailsHref?: RuleDetailsNavigation['href']; diff --git a/x-pack/plugins/cases/public/components/user_actions/use_user_actions_handler.tsx b/x-pack/plugins/cases/public/components/user_actions/use_user_actions_handler.tsx index f6bf5372eedc0..e19dbea584fc8 100644 --- a/x-pack/plugins/cases/public/components/user_actions/use_user_actions_handler.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/use_user_actions_handler.tsx @@ -76,8 +76,8 @@ export const useUserActionsHandler = (): UseUserActionsHandler => { ); const handleDeleteComment = useCallback( - (id: string) => { - deleteComment({ caseId, commentId: id }); + (id: string, successToasterTitle: string) => { + deleteComment({ caseId, commentId: id, successToasterTitle }); }, [caseId, deleteComment] ); diff --git a/x-pack/plugins/cases/public/containers/use_delete_comment.test.tsx b/x-pack/plugins/cases/public/containers/use_delete_comment.test.tsx index 2fe3a2f21eaa1..6f2063864a837 100644 --- a/x-pack/plugins/cases/public/containers/use_delete_comment.test.tsx +++ b/x-pack/plugins/cases/public/containers/use_delete_comment.test.tsx @@ -5,23 +5,21 @@ * 2.0. */ -import React from 'react'; import { act, renderHook } from '@testing-library/react-hooks'; -import type { UseDeleteComment } from './use_delete_comment'; import { useDeleteComment } from './use_delete_comment'; import * as api from './api'; import { basicCaseId } from './mock'; -import { TestProviders } from '../common/mock'; import { useRefreshCaseViewPage } from '../components/case_view/use_on_refresh_case_view_page'; import { useToasts } from '../common/lib/kibana'; +import type { AppMockRenderer } from '../common/mock'; +import { createAppMockRenderer } from '../common/mock'; jest.mock('../common/lib/kibana'); jest.mock('./api'); jest.mock('../components/case_view/use_on_refresh_case_view_page'); const commentId = 'ab124'; - -const wrapper: React.FC = ({ children }) => {children}; +const successToasterTitle = 'Deleted'; describe('useDeleteComment', () => { const addSuccess = jest.fn(); @@ -29,13 +27,16 @@ describe('useDeleteComment', () => { (useToasts as jest.Mock).mockReturnValue({ addSuccess, addError }); + let appMockRender: AppMockRenderer; + beforeEach(() => { + appMockRender = createAppMockRenderer(); jest.clearAllMocks(); }); it('init', async () => { - const { result } = renderHook(() => useDeleteComment(), { - wrapper, + const { result } = renderHook(() => useDeleteComment(), { + wrapper: appMockRender.AppWrapper, }); expect(result.current).toBeTruthy(); @@ -44,17 +45,15 @@ describe('useDeleteComment', () => { it('calls deleteComment with correct arguments - case', async () => { const spyOnDeleteComment = jest.spyOn(api, 'deleteComment'); - const { waitForNextUpdate, result } = renderHook( - () => useDeleteComment(), - { - wrapper, - } - ); + const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), { + wrapper: appMockRender.AppWrapper, + }); act(() => { result.current.mutate({ caseId: basicCaseId, commentId, + successToasterTitle, }); }); @@ -68,16 +67,14 @@ describe('useDeleteComment', () => { }); it('refreshes the case page view after delete', async () => { - const { waitForNextUpdate, result } = renderHook( - () => useDeleteComment(), - { - wrapper, - } - ); + const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), { + wrapper: appMockRender.AppWrapper, + }); result.current.mutate({ caseId: basicCaseId, commentId, + successToasterTitle, }); await waitForNextUpdate(); @@ -85,20 +82,39 @@ describe('useDeleteComment', () => { expect(useRefreshCaseViewPage()).toBeCalled(); }); + it('shows a success toaster correctly', async () => { + const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), { + wrapper: appMockRender.AppWrapper, + }); + + act(() => { + result.current.mutate({ + caseId: basicCaseId, + commentId, + successToasterTitle, + }); + }); + + await waitForNextUpdate(); + + expect(addSuccess).toHaveBeenCalledWith({ + title: 'Deleted', + className: 'eui-textBreakWord', + }); + }); + it('sets isError when fails to delete a case', async () => { const spyOnDeleteComment = jest.spyOn(api, 'deleteComment'); - spyOnDeleteComment.mockRejectedValue(new Error('Not possible :O')); + spyOnDeleteComment.mockRejectedValue(new Error('Error')); - const { waitForNextUpdate, result } = renderHook( - () => useDeleteComment(), - { - wrapper, - } - ); + const { waitForNextUpdate, result } = renderHook(() => useDeleteComment(), { + wrapper: appMockRender.AppWrapper, + }); result.current.mutate({ caseId: basicCaseId, commentId, + successToasterTitle, }); await waitForNextUpdate(); diff --git a/x-pack/plugins/cases/public/containers/use_delete_comment.tsx b/x-pack/plugins/cases/public/containers/use_delete_comment.tsx index 58de861baf383..ffe8bebedab34 100644 --- a/x-pack/plugins/cases/public/containers/use_delete_comment.tsx +++ b/x-pack/plugins/cases/public/containers/use_delete_comment.tsx @@ -16,10 +16,11 @@ import * as i18n from './translations'; interface MutationArgs { caseId: string; commentId: string; + successToasterTitle: string; } export const useDeleteComment = () => { - const { showErrorToast } = useCasesToast(); + const { showErrorToast, showSuccessToast } = useCasesToast(); const refreshCaseViewPage = useRefreshCaseViewPage(); return useMutation( @@ -29,7 +30,8 @@ export const useDeleteComment = () => { }, { mutationKey: casesMutationsKeys.deleteComment, - onSuccess: () => { + onSuccess: (_, { successToasterTitle }) => { + showSuccessToast(successToasterTitle); refreshCaseViewPage(); }, onError: (error: ServerError) => { From 7aaa9c28123bf945c6898078f3c9425946ad79b4 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 31 Mar 2023 18:45:36 +0300 Subject: [PATCH 5/6] PR feedback --- .../cases/common/utils/attachments.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 x-pack/plugins/cases/common/utils/attachments.test.ts diff --git a/x-pack/plugins/cases/common/utils/attachments.test.ts b/x-pack/plugins/cases/common/utils/attachments.test.ts new file mode 100644 index 0000000000000..dfd3c491f0968 --- /dev/null +++ b/x-pack/plugins/cases/common/utils/attachments.test.ts @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { CommentAttributes } from '../api'; +import { CommentType } from '../api'; +import { isAlertAttachment } from './attachments'; + +describe('isAlertAttachment', () => { + const alert = { + type: CommentType.alert as const, + } as CommentAttributes; + + const commentTypeWithoutAlert = Object.values(CommentType).filter( + (type) => type !== CommentType.alert + ); + + it('returns false for type: alert', () => { + expect(isAlertAttachment(alert)).toBe(true); + }); + + it.each(commentTypeWithoutAlert)('returns false for type: %s', (type) => { + const attachment = { + type, + } as CommentAttributes; + + expect(isAlertAttachment(attachment)).toBe(false); + }); +}); From 0a62126790805dd2a24e828c6f44641f5ad8875e Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Sat, 1 Apr 2023 13:28:34 +0300 Subject: [PATCH 6/6] PR feedback --- .../cases/common/utils/attachments.test.ts | 55 +++- .../plugins/cases/common/utils/attachments.ts | 6 - .../server/client/attachments/delete.test.ts | 4 +- .../cases/server/client/attachments/delete.ts | 7 +- .../server/services/alerts/index.test.ts | 18 +- .../cases/server/services/alerts/index.ts | 6 +- x-pack/plugins/cases/server/services/mocks.ts | 2 +- .../alert_data_client/alerts_client.mock.ts | 2 +- .../server/alert_data_client/alerts_client.ts | 8 +- .../tests/remove_alerts_from_case.test.ts | 12 +- .../tests/common/comments/delete_comment.ts | 298 ++++++++---------- 11 files changed, 197 insertions(+), 221 deletions(-) diff --git a/x-pack/plugins/cases/common/utils/attachments.test.ts b/x-pack/plugins/cases/common/utils/attachments.test.ts index dfd3c491f0968..eec0ecaef5b74 100644 --- a/x-pack/plugins/cases/common/utils/attachments.test.ts +++ b/x-pack/plugins/cases/common/utils/attachments.test.ts @@ -7,26 +7,53 @@ import type { CommentAttributes } from '../api'; import { CommentType } from '../api'; -import { isAlertAttachment } from './attachments'; +import { + isCommentRequestTypeExternalReference, + isCommentRequestTypePersistableState, +} from './attachments'; -describe('isAlertAttachment', () => { - const alert = { - type: CommentType.alert as const, - } as CommentAttributes; +describe('attachments utils', () => { + describe('isCommentRequestTypeExternalReference', () => { + const externalReferenceAttachment = { + type: CommentType.externalReference as const, + } as CommentAttributes; + + const commentTypeWithoutAlert = Object.values(CommentType).filter( + (type) => type !== CommentType.externalReference + ); + + it('returns false for type: externalReference', () => { + expect(isCommentRequestTypeExternalReference(externalReferenceAttachment)).toBe(true); + }); - const commentTypeWithoutAlert = Object.values(CommentType).filter( - (type) => type !== CommentType.alert - ); + it.each(commentTypeWithoutAlert)('returns false for type: %s', (type) => { + const attachment = { + type, + } as CommentAttributes; - it('returns false for type: alert', () => { - expect(isAlertAttachment(alert)).toBe(true); + expect(isCommentRequestTypeExternalReference(attachment)).toBe(false); + }); }); - it.each(commentTypeWithoutAlert)('returns false for type: %s', (type) => { - const attachment = { - type, + describe('isCommentRequestTypePersistableState', () => { + const persistableStateAttachment = { + type: CommentType.persistableState as const, } as CommentAttributes; - expect(isAlertAttachment(attachment)).toBe(false); + const commentTypeWithoutAlert = Object.values(CommentType).filter( + (type) => type !== CommentType.persistableState + ); + + it('returns false for type: persistableState', () => { + expect(isCommentRequestTypePersistableState(persistableStateAttachment)).toBe(true); + }); + + it.each(commentTypeWithoutAlert)('returns false for type: %s', (type) => { + const attachment = { + type, + } as CommentAttributes; + + expect(isCommentRequestTypePersistableState(attachment)).toBe(false); + }); }); }); diff --git a/x-pack/plugins/cases/common/utils/attachments.ts b/x-pack/plugins/cases/common/utils/attachments.ts index b251ffdcd5a76..e98df664ed2d4 100644 --- a/x-pack/plugins/cases/common/utils/attachments.ts +++ b/x-pack/plugins/cases/common/utils/attachments.ts @@ -6,8 +6,6 @@ */ import type { - AttributesTypeAlerts, - CommentAttributes, CommentRequest, CommentRequestExternalReferenceType, CommentRequestPersistableStateType, @@ -31,7 +29,3 @@ export const isCommentRequestTypePersistableState = ( ): context is CommentRequestPersistableStateType => { return context.type === CommentType.persistableState; }; - -export const isAlertAttachment = ( - attachment: CommentAttributes -): attachment is AttributesTypeAlerts => attachment.type === CommentType.alert; diff --git a/x-pack/plugins/cases/server/client/attachments/delete.test.ts b/x-pack/plugins/cases/server/client/attachments/delete.test.ts index 89db8b4f6f2b7..c74f621346728 100644 --- a/x-pack/plugins/cases/server/client/attachments/delete.test.ts +++ b/x-pack/plugins/cases/server/client/attachments/delete.test.ts @@ -28,7 +28,7 @@ describe('deleteComment', () => { alerts: [{ id: 'test-id', index: 'test-index' }], }); - expect(clientArgs.services.alertsService.removeAlertsFromCase).toHaveBeenCalledWith({ + expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).toHaveBeenCalledWith({ alerts: [{ id: 'test-id', index: 'test-index' }], caseId: 'mock-id-4', }); @@ -40,7 +40,7 @@ describe('deleteComment', () => { expect(clientArgs.services.alertsService.ensureAlertsAuthorized).not.toHaveBeenCalledWith(); - expect(clientArgs.services.alertsService.removeAlertsFromCase).not.toHaveBeenCalledWith(); + expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).not.toHaveBeenCalledWith(); }); }); }); diff --git a/x-pack/plugins/cases/server/client/attachments/delete.ts b/x-pack/plugins/cases/server/client/attachments/delete.ts index 0d0705ad88a87..1a11620d9998e 100644 --- a/x-pack/plugins/cases/server/client/attachments/delete.ts +++ b/x-pack/plugins/cases/server/client/attachments/delete.ts @@ -11,8 +11,7 @@ import pMap from 'p-map'; import type { SavedObject } from '@kbn/core/server'; import type { CommentAttributes } from '../../../common/api'; import { Actions, ActionTypes } from '../../../common/api'; -import { isAlertAttachment } from '../../../common/utils/attachments'; -import { getAlertInfoFromComments } from '../../common/utils'; +import { getAlertInfoFromComments, isCommentRequestTypeAlert } from '../../common/utils'; import { CASE_SAVED_OBJECT, MAX_CONCURRENT_SEARCHES } from '../../../common/constants'; import type { CasesClientArgs } from '../types'; import { createCaseError } from '../../common/error'; @@ -151,11 +150,11 @@ interface HandleAlertsArgs { } const handleAlerts = async ({ alertsService, attachment, caseId }: HandleAlertsArgs) => { - if (!isAlertAttachment(attachment)) { + if (!isCommentRequestTypeAlert(attachment)) { return; } const alerts = getAlertInfoFromComments([attachment]); await alertsService.ensureAlertsAuthorized({ alerts }); - await alertsService.removeAlertsFromCase({ alerts, caseId }); + await alertsService.removeCaseIdFromAlerts({ alerts, caseId }); }; diff --git a/x-pack/plugins/cases/server/services/alerts/index.test.ts b/x-pack/plugins/cases/server/services/alerts/index.test.ts index 2d34ea0dec6f8..5cc5e6de9650d 100644 --- a/x-pack/plugins/cases/server/services/alerts/index.test.ts +++ b/x-pack/plugins/cases/server/services/alerts/index.test.ts @@ -382,7 +382,7 @@ describe('updateAlertsStatus', () => { }); }); - describe('removeAlertsFromCase', () => { + describe('removeCaseIdFromAlerts', () => { const alerts = [ { id: 'c3869d546717e8c581add9cbf7d24578f34cd3e72cbc8d8b8e9a9330a899f70f', @@ -392,36 +392,36 @@ describe('updateAlertsStatus', () => { const caseId = 'test-case'; it('update case info', async () => { - await alertService.removeAlertsFromCase({ alerts, caseId }); + await alertService.removeCaseIdFromAlerts({ alerts, caseId }); - expect(alertsClient.removeAlertsFromCase).toBeCalledWith({ alerts, caseId }); + expect(alertsClient.removeCaseIdFromAlerts).toBeCalledWith({ alerts, caseId }); }); it('filters out alerts with empty id', async () => { - await alertService.removeAlertsFromCase({ + await alertService.removeCaseIdFromAlerts({ alerts: [{ id: '', index: 'test-index' }, ...alerts], caseId, }); - expect(alertsClient.removeAlertsFromCase).toBeCalledWith({ alerts, caseId }); + expect(alertsClient.removeCaseIdFromAlerts).toBeCalledWith({ alerts, caseId }); }); it('filters out alerts with empty index', async () => { - await alertService.removeAlertsFromCase({ + await alertService.removeCaseIdFromAlerts({ alerts: [{ id: 'test-id', index: '' }, ...alerts], caseId, }); - expect(alertsClient.removeAlertsFromCase).toBeCalledWith({ alerts, caseId }); + expect(alertsClient.removeCaseIdFromAlerts).toBeCalledWith({ alerts, caseId }); }); it('does not call the alerts client with no alerts', async () => { - await alertService.removeAlertsFromCase({ + await alertService.removeCaseIdFromAlerts({ alerts: [{ id: '', index: 'test-index' }], caseId, }); - expect(alertsClient.removeAlertsFromCase).not.toHaveBeenCalled(); + expect(alertsClient.removeCaseIdFromAlerts).not.toHaveBeenCalled(); }); }); }); diff --git a/x-pack/plugins/cases/server/services/alerts/index.ts b/x-pack/plugins/cases/server/services/alerts/index.ts index 675dafe450efb..9ce651dce105d 100644 --- a/x-pack/plugins/cases/server/services/alerts/index.ts +++ b/x-pack/plugins/cases/server/services/alerts/index.ts @@ -234,7 +234,7 @@ export class AlertService { } } - public async removeAlertsFromCase({ + public async removeCaseIdFromAlerts({ alerts, caseId, }: RemoveAlertsFromCaseRequest): Promise { @@ -245,13 +245,13 @@ export class AlertService { return; } - await this.alertsClient.removeAlertsFromCase({ + await this.alertsClient.removeCaseIdFromAlerts({ alerts: nonEmptyAlerts, caseId, }); } catch (error) { throw createCaseError({ - message: `Failed to remove alerts from case ${caseId}: ${error}`, + message: `Failed to remove case ${caseId} from alerts: ${error}`, error, logger: this.logger, }); diff --git a/x-pack/plugins/cases/server/services/mocks.ts b/x-pack/plugins/cases/server/services/mocks.ts index 7517ca2fc205d..5480d79a2c8d6 100644 --- a/x-pack/plugins/cases/server/services/mocks.ts +++ b/x-pack/plugins/cases/server/services/mocks.ts @@ -139,7 +139,7 @@ export const createAlertServiceMock = (): AlertServiceMock => { executeAggregations: jest.fn(), bulkUpdateCases: jest.fn(), ensureAlertsAuthorized: jest.fn(), - removeAlertsFromCase: jest.fn(), + removeCaseIdFromAlerts: jest.fn(), }; // the cast here is required because jest.Mocked tries to include private members and would throw an error diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts index a25b5a1b36ef5..a0c903ca7b4e1 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts @@ -23,7 +23,7 @@ const createAlertsClientMock = () => { getBrowserFields: jest.fn(), getAlertSummary: jest.fn(), ensureAllAlertsAuthorizedRead: jest.fn(), - removeAlertsFromCase: jest.fn(), + removeCaseIdFromAlerts: jest.fn(), }; return mocked; }; diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts index cc5d3b10d5bba..08b4dd5738791 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts @@ -851,7 +851,7 @@ export class AlertsClient { }); } - public async removeAlertsFromCase({ caseId, alerts }: RemoveAlertsFromCaseOptions) { + public async removeCaseIdFromAlerts({ caseId, alerts }: RemoveAlertsFromCaseOptions) { try { if (alerts.length === 0) { return; @@ -862,9 +862,9 @@ export class AlertsClient { operation: ReadOperations.Get, }); - const painlessScript = `if (ctx._source['${ALERT_CASE_IDS}'] != null && ctx._source['${ALERT_CASE_IDS}'].length > 0) { + 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] == '${caseId}') { + if (ctx._source['${ALERT_CASE_IDS}'][i].equals('${caseId}')) { ctx._source['${ALERT_CASE_IDS}'].remove(i); } } @@ -891,7 +891,7 @@ export class AlertsClient { body: bulkUpdateRequest, }); } catch (error) { - this.logger.error(`Error removing alerts from case ${caseId}: ${error}`); + this.logger.error(`Error to remove case ${caseId} from alerts: ${error}`); throw error; } } diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts index 52d67315e23ea..c938dd8d592fb 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/remove_alerts_from_case.test.ts @@ -13,7 +13,7 @@ import { ALERT_CASE_IDS, ALERT_RULE_CONSUMER, ALERT_RULE_TYPE_ID } from '@kbn/ru import { AlertsClient, ConstructorOptions } from '../alerts_client'; import { ruleDataServiceMock } from '../../rule_data_plugin_service/rule_data_plugin_service.mock'; -describe('removeAlertsFromCase', () => { +describe('removeCaseIdFromAlerts', () => { const alertingAuthMock = alertingAuthorizationMock.create(); const esClientMock = elasticsearchClientMock.createElasticsearchClient(); const auditLogger = auditLoggerMock.create(); @@ -54,7 +54,7 @@ describe('removeAlertsFromCase', () => { it('removes alerts from a case', async () => { const alertsClient = new AlertsClient(alertsClientParams); - await alertsClient.removeAlertsFromCase({ caseId, alerts }); + await alertsClient.removeCaseIdFromAlerts({ caseId, alerts }); expect(esClientMock.mget).toHaveBeenCalledWith({ docs: [{ _id: 'alert-id', _index: 'alert-index' }], @@ -72,9 +72,9 @@ describe('removeAlertsFromCase', () => { Object { "script": Object { "lang": "painless", - "source": "if (ctx._source['kibana.alert.case_ids'] != null && ctx._source['kibana.alert.case_ids'].length > 0) { + "source": "if (ctx._source['kibana.alert.case_ids'] != null) { for (int i=0; i < ctx._source['kibana.alert.case_ids'].length; i++) { - if (ctx._source['kibana.alert.case_ids'][i] == 'test-case') { + if (ctx._source['kibana.alert.case_ids'][i].equals('test-case')) { ctx._source['kibana.alert.case_ids'].remove(i); } } @@ -89,7 +89,7 @@ describe('removeAlertsFromCase', () => { it('calls ensureAllAlertsAuthorized correctly', async () => { const alertsClient = new AlertsClient(alertsClientParams); - await alertsClient.removeAlertsFromCase({ caseId, alerts }); + await alertsClient.removeCaseIdFromAlerts({ caseId, alerts }); expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ consumer: 'apm', @@ -101,7 +101,7 @@ describe('removeAlertsFromCase', () => { it('does not do any calls if there are no alerts', async () => { const alertsClient = new AlertsClient(alertsClientParams); - await alertsClient.removeAlertsFromCase({ caseId, alerts: [] }); + await alertsClient.removeCaseIdFromAlerts({ caseId, alerts: [] }); expect(alertingAuthMock.ensureAuthorized).not.toHaveBeenCalled(); expect(esClientMock.bulk).not.toHaveBeenCalled(); diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts index e68727ab8d25a..b0c4d670855a3 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts @@ -112,101 +112,113 @@ export default ({ getService }: FtrProviderContext): void => { }); describe('alerts', () => { - describe('security_solution', () => { - const createCaseAttachAlertAndDeleteAlert = async ({ - totalCases, - indexOfCaseToDelete, - expectedHttpCode = 204, - auth = { user: superUser, space: 'space1' }, - }: { - totalCases: number; - indexOfCaseToDelete: number; - expectedHttpCode?: number; - auth?: { user: User; space: string | null }; - }) => { - const cases = await Promise.all( - [...Array(totalCases).keys()].map((index) => - createCase( - supertestWithoutAuth, - { - ...postCaseReq, - settings: { syncAlerts: false }, - }, - 200, - { user: superUser, space: 'space1' } - ) + type Alerts = Array<{ _id: string; _index: string }>; + + const createCaseAttachAlertAndDeleteAlert = async ({ + totalCases, + indexOfCaseToDelete, + owner, + expectedHttpCode = 204, + deleteCommentAuth = { user: superUser, space: 'space1' }, + alerts, + getAlerts, + }: { + totalCases: number; + indexOfCaseToDelete: number; + owner: string; + expectedHttpCode?: number; + deleteCommentAuth?: { user: User; space: string | null }; + alerts: Alerts; + getAlerts: (alerts: Alerts) => Promise>>; + }) => { + const cases = await Promise.all( + [...Array(totalCases).keys()].map((index) => + createCase( + supertestWithoutAuth, + { + ...postCaseReq, + owner, + settings: { syncAlerts: false }, + }, + 200, + { user: superUser, space: 'space1' } ) - ); + ) + ); - const signals = await createSecuritySolutionAlerts(supertest, log); - const alerts = [signals.hits.hits[0], signals.hits.hits[1]]; - const updatedCases = []; - - for (const theCase of cases) { - const updatedCase = await createComment({ - supertest: supertestWithoutAuth, - caseId: theCase.id, - params: { - alertId: alerts.map((alert) => alert._id), - index: alerts.map((alert) => alert._index), - rule: { - id: 'id', - name: 'name', - }, - owner: 'securitySolutionFixture', - type: CommentType.alert, + const updatedCases = []; + + for (const theCase of cases) { + const updatedCase = await createComment({ + supertest: supertestWithoutAuth, + caseId: theCase.id, + params: { + alertId: alerts.map((alert) => alert._id), + index: alerts.map((alert) => alert._index), + rule: { + id: 'id', + name: 'name', }, - auth: { user: superUser, space: 'space1' }, - }); + owner, + type: CommentType.alert, + }, + auth: { user: superUser, space: 'space1' }, + }); - updatedCases.push(updatedCase); - } + updatedCases.push(updatedCase); + } - const caseIds = updatedCases.map((theCase) => theCase.id); + const caseIds = updatedCases.map((theCase) => theCase.id); - await es.indices.refresh({ index: alerts.map((alert) => alert._index) }); - const updatedAlerts = await getSecuritySolutionAlerts( - supertest, - alerts.map((alert) => alert._id) - ); + const updatedAlerts = await getAlerts(alerts); - for (const alert of updatedAlerts.hits.hits) { - expect(alert._source?.[ALERT_CASE_IDS]).eql(caseIds); - } + for (const alert of updatedAlerts) { + expect(alert[ALERT_CASE_IDS]).eql(caseIds); + } - const caseToDelete = updatedCases[indexOfCaseToDelete]; - const commentId = caseToDelete.comments![0].id; + const caseToDelete = updatedCases[indexOfCaseToDelete]; + const commentId = caseToDelete.comments![0].id; - await deleteComment({ - supertest: supertestWithoutAuth, - caseId: caseToDelete.id, - commentId, - expectedHttpCode, - auth, - }); + await deleteComment({ + supertest: supertestWithoutAuth, + caseId: caseToDelete.id, + commentId, + expectedHttpCode, + auth: deleteCommentAuth, + }); - await es.indices.refresh({ index: updatedAlerts.hits.hits.map((alert) => alert._index) }); + const alertAfterDeletion = await getAlerts(alerts); - const alertAfterDeletion = await getSecuritySolutionAlerts( + const caseIdsWithoutRemovedCase = + expectedHttpCode === 204 + ? updatedCases + .filter((theCase) => theCase.id !== caseToDelete.id) + .map((theCase) => theCase.id) + : updatedCases.map((theCase) => theCase.id); + + for (const alert of alertAfterDeletion) { + expect(alert[ALERT_CASE_IDS]).eql(caseIdsWithoutRemovedCase); + } + }; + + describe('security_solution', () => { + let alerts: Alerts = []; + + const getAlerts = async (_alerts: Alerts) => { + await es.indices.refresh({ index: _alerts.map((alert) => alert._index) }); + const updatedAlerts = await getSecuritySolutionAlerts( supertest, - updatedAlerts.hits.hits.map((alert) => alert._id) + alerts.map((alert) => alert._id) ); - const caseIdsWithoutRemovedCase = - expectedHttpCode === 204 - ? updatedCases - .filter((theCase) => theCase.id !== caseToDelete.id) - .map((theCase) => theCase.id) - : updatedCases.map((theCase) => theCase.id); - - for (const alert of alertAfterDeletion.hits.hits) { - expect(alert._source?.[ALERT_CASE_IDS]).eql(caseIdsWithoutRemovedCase); - } + return updatedAlerts.hits.hits.map((alert) => ({ ...alert._source })); }; beforeEach(async () => { await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts'); await createSignalsIndex(supertest, log); + const signals = await createSecuritySolutionAlerts(supertest, log); + alerts = [signals.hits.hits[0], signals.hits.hits[1]]; }); afterEach(async () => { @@ -219,6 +231,9 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 1, indexOfCaseToDelete: 0, + owner: 'securitySolutionFixture', + alerts, + getAlerts, }); }); @@ -226,6 +241,9 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 3, indexOfCaseToDelete: 1, + owner: 'securitySolutionFixture', + alerts, + getAlerts, }); }); @@ -233,8 +251,11 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 1, indexOfCaseToDelete: 0, + owner: 'securitySolutionFixture', + alerts, + getAlerts, expectedHttpCode: 204, - auth: { user: secOnlyReadAlerts, space: 'space1' }, + deleteCommentAuth: { user: secOnlyReadAlerts, space: 'space1' }, }); }); @@ -242,8 +263,11 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 1, indexOfCaseToDelete: 0, + owner: 'securitySolutionFixture', + alerts, + getAlerts, expectedHttpCode: 403, - auth: { user: obsSec, space: 'space1' }, + deleteCommentAuth: { user: obsSec, space: 'space1' }, }); }); @@ -251,95 +275,24 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 1, indexOfCaseToDelete: 0, + owner: 'securitySolutionFixture', + alerts, + getAlerts, expectedHttpCode: 204, - auth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, + deleteCommentAuth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, }); }); }); describe('observability', () => { - const createCaseAttachAlertAndDeleteAlert = async ({ - totalCases, - indexOfCaseToDelete, - expectedHttpCode = 204, - auth = { user: superUser, space: 'space1' }, - }: { - totalCases: number; - indexOfCaseToDelete: number; - expectedHttpCode?: number; - auth?: { user: User; space: string | null }; - }) => { - const cases = await Promise.all( - [...Array(totalCases).keys()].map((index) => - createCase( - supertestWithoutAuth, - { - ...postCaseReq, - owner: 'observabilityFixture', - settings: { syncAlerts: false }, - }, - 200, - { user: superUser, space: 'space1' } - ) - ) - ); - - const alerts = [ - { _id: 'NoxgpHkBqbdrfX07MqXV', _index: '.alerts-observability.apm.alerts' }, - { _id: 'space1alert', _index: '.alerts-observability.apm.alerts' }, - ]; - const updatedCases = []; - - for (const theCase of cases) { - const updatedCase = await createComment({ - supertest: supertestWithoutAuth, - caseId: theCase.id, - params: { - alertId: alerts.map((alert) => alert._id), - index: alerts.map((alert) => alert._index), - rule: { - id: 'id', - name: 'name', - }, - owner: 'observabilityFixture', - type: CommentType.alert, - }, - auth: { user: superUser, space: 'space1' }, - }); - - updatedCases.push(updatedCase); - } - - const caseIds = updatedCases.map((theCase) => theCase.id); + const alerts = [ + { _id: 'NoxgpHkBqbdrfX07MqXV', _index: '.alerts-observability.apm.alerts' }, + { _id: 'space1alert', _index: '.alerts-observability.apm.alerts' }, + ]; + const getAlerts = async (_alerts: Alerts) => { const updatedAlerts = await Promise.all( - alerts.map((alert) => - getAlertById({ - supertest: supertestWithoutAuth, - id: alert._id, - index: alert._index, - auth: { user: superUser, space: 'space1' }, - }) - ) - ); - - for (const alert of updatedAlerts) { - expect(alert[ALERT_CASE_IDS]).eql(caseIds); - } - - const caseToDelete = updatedCases[indexOfCaseToDelete]; - const commentId = caseToDelete.comments![0].id; - - await deleteComment({ - supertest: supertestWithoutAuth, - caseId: caseToDelete.id, - commentId, - expectedHttpCode, - auth, - }); - - const alertAfterDeletion = await Promise.all( - alerts.map((alert) => + _alerts.map((alert) => getAlertById({ supertest: supertestWithoutAuth, id: alert._id, @@ -349,16 +302,7 @@ export default ({ getService }: FtrProviderContext): void => { ) ); - const caseIdsWithoutRemovedCase = - expectedHttpCode === 204 - ? updatedCases - .filter((theCase) => theCase.id !== caseToDelete.id) - .map((theCase) => theCase.id) - : updatedCases.map((theCase) => theCase.id); - - for (const alert of alertAfterDeletion) { - expect(alert[ALERT_CASE_IDS]).eql(caseIdsWithoutRemovedCase); - } + return updatedAlerts as Array>; }; beforeEach(async () => { @@ -373,6 +317,9 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 1, indexOfCaseToDelete: 0, + owner: 'observabilityFixture', + alerts, + getAlerts, }); }); @@ -380,6 +327,9 @@ export default ({ getService }: FtrProviderContext): void => { await createCaseAttachAlertAndDeleteAlert({ totalCases: 3, indexOfCaseToDelete: 1, + owner: 'observabilityFixture', + alerts, + getAlerts, }); }); @@ -388,7 +338,10 @@ export default ({ getService }: FtrProviderContext): void => { totalCases: 1, indexOfCaseToDelete: 0, expectedHttpCode: 204, - auth: { user: obsOnlyReadAlerts, space: 'space1' }, + owner: 'observabilityFixture', + alerts, + getAlerts, + deleteCommentAuth: { user: obsOnlyReadAlerts, space: 'space1' }, }); }); @@ -397,7 +350,10 @@ export default ({ getService }: FtrProviderContext): void => { totalCases: 1, indexOfCaseToDelete: 0, expectedHttpCode: 403, - auth: { user: obsSec, space: 'space1' }, + owner: 'observabilityFixture', + alerts, + getAlerts, + deleteCommentAuth: { user: obsSec, space: 'space1' }, }); }); });