From ff402bf98e67ba1ef3532ec4280309960d144834 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 24 Jan 2024 19:00:49 +0200 Subject: [PATCH 01/12] Get KibanaRequest from framework --- .../server/sub_action_framework/executor.ts | 3 +- .../sub_action_connector.ts | 3 + .../server/sub_action_framework/types.ts | 2 + .../connectors/cases/cases_connector.test.ts | 66 +++++++++++++++---- .../connectors/cases/cases_connector.ts | 65 ++++++++++-------- .../cases/cases_connector_executor.ts | 4 -- 6 files changed, 97 insertions(+), 46 deletions(-) diff --git a/x-pack/plugins/actions/server/sub_action_framework/executor.ts b/x-pack/plugins/actions/server/sub_action_framework/executor.ts index 9ac7a63dc2e96..2a68a060e15c4 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/executor.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/executor.ts @@ -29,7 +29,7 @@ export const buildExecutor = < logger: Logger; configurationUtilities: ActionsConfigurationUtilities; }): ExecutorType => { - return async ({ actionId, params, config, secrets, services }) => { + return async ({ actionId, params, config, secrets, services, request }) => { const subAction = params.subAction; const subActionParams = params.subActionParams; @@ -40,6 +40,7 @@ export const buildExecutor = < configurationUtilities, logger, services, + request, }); const subActions = service.getSubActions(); diff --git a/x-pack/plugins/actions/server/sub_action_framework/sub_action_connector.ts b/x-pack/plugins/actions/server/sub_action_framework/sub_action_connector.ts index 7d3c6e51e844e..3d7e6540fd74f 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/sub_action_connector.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/sub_action_connector.ts @@ -21,6 +21,7 @@ import { ElasticsearchClient } from '@kbn/core-elasticsearch-server'; import { finished } from 'stream/promises'; import { IncomingMessage } from 'http'; import { PassThrough } from 'stream'; +import { KibanaRequest } from '@kbn/core-http-server'; import { assertURL } from './helpers/validators'; import { ActionsConfigurationUtilities } from '../actions_config'; import { SubAction, SubActionRequestParams } from './types'; @@ -39,6 +40,7 @@ export abstract class SubActionConnector { private axiosInstance: AxiosInstance; private subActions: Map = new Map(); private configurationUtilities: ActionsConfigurationUtilities; + protected readonly kibanaRequest?: KibanaRequest; protected logger: Logger; protected esClient: ElasticsearchClient; protected savedObjectsClient: SavedObjectsClientContract; @@ -55,6 +57,7 @@ export abstract class SubActionConnector { this.esClient = params.services.scopedClusterClient; this.configurationUtilities = params.configurationUtilities; this.axiosInstance = axios.create(); + this.kibanaRequest = params.request; } private normalizeURL(url: string) { diff --git a/x-pack/plugins/actions/server/sub_action_framework/types.ts b/x-pack/plugins/actions/server/sub_action_framework/types.ts index 2f5ae6bad769b..b9fc6c684beca 100644 --- a/x-pack/plugins/actions/server/sub_action_framework/types.ts +++ b/x-pack/plugins/actions/server/sub_action_framework/types.ts @@ -10,6 +10,7 @@ import type { Logger } from '@kbn/logging'; import type { LicenseType } from '@kbn/licensing-plugin/common/types'; import type { Method, AxiosRequestConfig } from 'axios'; +import { KibanaRequest } from '@kbn/core-http-server'; import type { ActionsConfigurationUtilities } from '../actions_config'; import type { ActionTypeParams, @@ -30,6 +31,7 @@ export interface ServiceParams { logger: Logger; secrets: Secrets; services: Services; + request?: KibanaRequest; } export type SubActionRequestParams = { diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts index e19b07bf327fb..2818a9b957334 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts @@ -16,6 +16,7 @@ import { CasesService } from './cases_service'; import { CasesConnectorError } from './cases_connector_error'; import { CaseError } from '../../common/error'; import { fullJitterBackoffFactory } from './full_jitter_backoff'; +import { CoreKibanaRequest } from '@kbn/core/server'; jest.mock('./cases_connector_executor'); jest.mock('./full_jitter_backoff'); @@ -26,6 +27,7 @@ const fullJitterBackoffFactoryMock = fullJitterBackoffFactory as jest.Mock; describe('CasesConnector', () => { const services = actionsMock.createServices(); const logger = loggingSystemMock.createLogger(); + const kibanaRequest = CoreKibanaRequest.from({ path: '/', headers: {} }); const groupingBy = ['host.name', 'dest.ip']; const rule = { @@ -38,6 +40,7 @@ describe('CasesConnector', () => { const owner = 'cases'; const timeWindow = '7d'; const reopenClosedCases = false; + const maximumCasesToOpen = 5; const mockExecute = jest.fn(); const getCasesClient = jest.fn().mockResolvedValue({ foo: 'bar' }); @@ -48,6 +51,18 @@ describe('CasesConnector', () => { create: () => ({ nextBackOff }), }; + const casesParams = { getCasesClient }; + + const connectorParams = { + configurationUtilities: actionsConfigMock.create(), + config: {}, + secrets: {}, + connector: { id: '1', type: CASES_CONNECTOR_ID }, + logger, + services, + request: kibanaRequest, + }; + let connector: CasesConnector; beforeEach(() => { @@ -63,15 +78,8 @@ describe('CasesConnector', () => { fullJitterBackoffFactoryMock.mockReturnValue(backOffFactory); connector = new CasesConnector({ - casesParams: { getCasesClient }, - connectorParams: { - configurationUtilities: actionsConfigMock.create(), - config: {}, - secrets: {}, - connector: { id: '1', type: CASES_CONNECTOR_ID }, - logger, - services, - }, + casesParams, + connectorParams, }); }); @@ -83,6 +91,7 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }); expect(CasesConnectorExecutorMock).toBeCalledWith({ @@ -101,6 +110,7 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }); expect(mockExecute).toBeCalledWith({ @@ -110,6 +120,7 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }); }); @@ -121,6 +132,7 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }); expect(getCasesClient).toBeCalled(); @@ -137,11 +149,12 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Bad request"`); expect(logger.error.mock.calls[0][0]).toBe( - '[CasesConnector][_run] Execution of case connector failed. Message: Bad request. Status code: 400' + '[CasesConnector][run] Execution of case connector failed. Message: Bad request. Status code: 400' ); }); @@ -156,11 +169,12 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Forbidden"`); expect(logger.error.mock.calls[0][0]).toBe( - '[CasesConnector][_run] Execution of case connector failed. Message: Forbidden. Status code: 500' + '[CasesConnector][run] Execution of case connector failed. Message: Forbidden. Status code: 500' ); }); @@ -175,11 +189,12 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Server error"`); expect(logger.error.mock.calls[0][0]).toBe( - '[CasesConnector][_run] Execution of case connector failed. Message: Server error. Status code: 500' + '[CasesConnector][run] Execution of case connector failed. Message: Server error. Status code: 500' ); }); @@ -196,9 +211,36 @@ describe('CasesConnector', () => { rule, timeWindow, reopenClosedCases, + maximumCasesToOpen, }); expect(nextBackOff).toBeCalledTimes(2); expect(mockExecute).toBeCalledTimes(3); }); + + it('throws if the kibana request is not defined', async () => { + connector = new CasesConnector({ + casesParams, + connectorParams: { ...connectorParams, request: undefined }, + }); + + await expect(() => + connector.run({ + alerts: [], + groupingBy, + owner, + rule, + timeWindow, + reopenClosedCases, + maximumCasesToOpen, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Kibana request is not defined"`); + + expect(logger.error.mock.calls[0][0]).toBe( + '[CasesConnector][run] Execution of case connector failed. Message: Kibana request is not defined. Status code: 400' + ); + + expect(nextBackOff).toBeCalledTimes(0); + expect(mockExecute).toBeCalledTimes(0); + }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts index aaae59a0c534a..c4a52b10be53d 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts @@ -8,7 +8,6 @@ import type { ServiceParams } from '@kbn/actions-plugin/server'; import { SubActionConnector } from '@kbn/actions-plugin/server'; import type { KibanaRequest } from '@kbn/core-http-server'; -import { CoreKibanaRequest } from '@kbn/core/server'; import { CASES_CONNECTOR_SUB_ACTION } from './constants'; import type { CasesConnectorConfig, CasesConnectorRunParams, CasesConnectorSecrets } from './types'; import { CasesConnectorRunParamsSchema } from './schema'; @@ -36,7 +35,6 @@ export class CasesConnector extends SubActionConnector< private readonly casesOracleService: CasesOracleService; private readonly casesService: CasesService; private readonly retryService: CaseConnectorRetryService; - private readonly kibanaRequest: KibanaRequest; private readonly casesParams: CasesConnectorParams['casesParams']; constructor({ connectorParams, casesParams }: CasesConnectorParams) { @@ -60,12 +58,6 @@ export class CasesConnector extends SubActionConnector< const backOffFactory = fullJitterBackoffFactory({ baseDelay: 5, maxBackoffTime: 2000 }); this.retryService = new CaseConnectorRetryService(this.logger, backOffFactory); - /** - * TODO: Get request from the actions framework. - * Should be set in the SubActionConnector's constructor - */ - this.kibanaRequest = CoreKibanaRequest.from({ path: '/', headers: {} }); - this.casesParams = casesParams; this.registerSubActions(); @@ -89,6 +81,11 @@ export class CasesConnector extends SubActionConnector< } public async run(params: CasesConnectorRunParams) { + if (!this.kibanaRequest) { + const error = new CasesConnectorError('Kibana request is not defined', 400); + this.handleError(error); + } + /** * TODO: Tell the task manager to not retry on non * retryable errors @@ -98,7 +95,13 @@ export class CasesConnector extends SubActionConnector< private async _run(params: CasesConnectorRunParams) { try { - const casesClient = await this.casesParams.getCasesClient(this.kibanaRequest); + const casesClient = await this.casesParams.getCasesClient( + /** + * The case connector will throw an error if the Kibana request + * is not define before executing the _run method + */ + this.kibanaRequest as KibanaRequest + ); const connectorExecutor = new CasesConnectorExecutor({ logger: this.logger, @@ -117,25 +120,7 @@ export class CasesConnector extends SubActionConnector< params ); } catch (error) { - if (isCasesConnectorError(error)) { - this.logError(error); - throw error; - } - - if (isCasesClientError(error)) { - const caseConnectorError = new CasesConnectorError( - error.message, - error.boomify().output.statusCode - ); - - this.logError(caseConnectorError); - throw caseConnectorError; - } - - const caseConnectorError = new CasesConnectorError(error.message, 500); - this.logError(caseConnectorError); - - throw caseConnectorError; + this.handleError(error); } finally { this.logDebugCurrentState( 'end', @@ -145,6 +130,28 @@ export class CasesConnector extends SubActionConnector< } } + private handleError(error: Error) { + if (isCasesConnectorError(error)) { + this.logError(error); + throw error; + } + + if (isCasesClientError(error)) { + const caseConnectorError = new CasesConnectorError( + error.message, + error.boomify().output.statusCode + ); + + this.logError(caseConnectorError); + throw caseConnectorError; + } + + const caseConnectorError = new CasesConnectorError(error.message, 500); + this.logError(caseConnectorError); + + throw caseConnectorError; + } + private logDebugCurrentState(state: string, message: string, params: CasesConnectorRunParams) { const alertIds = params.alerts.map(({ _id }) => _id); @@ -163,7 +170,7 @@ export class CasesConnector extends SubActionConnector< private logError(error: CasesConnectorError) { this.logger.error( - `[CasesConnector][_run] Execution of case connector failed. Message: ${error.message}. Status code: ${error.statusCode}`, + `[CasesConnector][run] Execution of case connector failed. Message: ${error.message}. Status code: ${error.statusCode}`, { error: { stack_trace: error.stack, diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts index e5befb1d7ab45..388f070ba07da 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts @@ -75,10 +75,6 @@ export class CasesConnectorExecutor { groupedAlertsWithCircuitBreakers ); - /** - * TODO: Add circuit breakers to the number of oracles they can be created or retrieved - */ - /** * Gets all records by the IDs that produces in generateOracleKeys. * If a record does not exist it will create the record. From 9cfe9f175e6fb19930d744aefe85793ff557add8 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 24 Jan 2024 19:57:29 +0200 Subject: [PATCH 02/12] Get space ID from the Kibana request --- .../connectors/cases/cases_connector.test.ts | 5 ++-- .../connectors/cases/cases_connector.ts | 24 +++++++++---------- .../cases/cases_connector_executor.test.ts | 2 ++ .../cases/cases_connector_executor.ts | 17 +++++-------- .../cases/server/connectors/cases/index.ts | 4 +++- .../plugins/cases/server/connectors/index.ts | 4 +++- x-pack/plugins/cases/server/plugin.ts | 14 +++++++++-- 7 files changed, 41 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts index 2818a9b957334..58c55ef2b6d07 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector.test.ts @@ -44,6 +44,7 @@ describe('CasesConnector', () => { const mockExecute = jest.fn(); const getCasesClient = jest.fn().mockResolvedValue({ foo: 'bar' }); + const getSpaceId = jest.fn().mockReturnValue('default'); // 1ms delay before retrying const nextBackOff = jest.fn().mockReturnValue(1); @@ -51,8 +52,7 @@ describe('CasesConnector', () => { create: () => ({ nextBackOff }), }; - const casesParams = { getCasesClient }; - + const casesParams = { getCasesClient, getSpaceId }; const connectorParams = { configurationUtilities: actionsConfigMock.create(), config: {}, @@ -99,6 +99,7 @@ describe('CasesConnector', () => { casesClient: { foo: 'bar' }, casesOracleService: expect.any(CasesOracleService), casesService: expect.any(CasesService), + spaceId: 'default', }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts index c4a52b10be53d..b7fc42f2054c9 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts @@ -25,7 +25,10 @@ import { fullJitterBackoffFactory } from './full_jitter_backoff'; interface CasesConnectorParams { connectorParams: ServiceParams; - casesParams: { getCasesClient: (request: KibanaRequest) => Promise }; + casesParams: { + getCasesClient: (request: KibanaRequest) => Promise; + getSpaceId: (request?: KibanaRequest) => string; + }; } export class CasesConnector extends SubActionConnector< @@ -86,28 +89,25 @@ export class CasesConnector extends SubActionConnector< this.handleError(error); } - /** - * TODO: Tell the task manager to not retry on non - * retryable errors - */ await this.retryService.retryWithBackoff(() => this._run(params)); } private async _run(params: CasesConnectorRunParams) { try { - const casesClient = await this.casesParams.getCasesClient( - /** - * The case connector will throw an error if the Kibana request - * is not define before executing the _run method - */ - this.kibanaRequest as KibanaRequest - ); + /** + * The case connector will throw an error if the Kibana request + * is not define before executing the _run method + */ + const kibanaRequest = this.kibanaRequest as KibanaRequest; + const casesClient = await this.casesParams.getCasesClient(kibanaRequest); + const spaceId = this.casesParams.getSpaceId(kibanaRequest); const connectorExecutor = new CasesConnectorExecutor({ logger: this.logger, casesOracleService: this.casesOracleService, casesService: this.casesService, casesClient, + spaceId, }); this.logDebugCurrentState('start', '[CasesConnector][_run] Executing case connector', params); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index 042e53c8ab295..4736d7fea3e7c 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -109,6 +109,7 @@ describe('CasesConnectorExecutor', () => { casesOracleService: new CasesOracleServiceMock(), casesService: new CasesServiceMock(), casesClient: casesClientMock, + spaceId: 'default', }); dateMathMock.parse.mockImplementation(() => moment('2023-10-09T10:23:42.769Z')); @@ -990,6 +991,7 @@ describe('CasesConnectorExecutor', () => { casesOracleService: new CasesOracleServiceMock(), casesService: new CasesServiceMock(), casesClient: casesClientMock, + spaceId: 'default', }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts index 388f070ba07da..c108baccfba4f 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts @@ -30,6 +30,7 @@ interface CasesConnectorExecutorParams { casesOracleService: CasesOracleService; casesService: CasesService; casesClient: CasesClient; + spaceId: string; } interface GroupedAlerts { @@ -47,17 +48,20 @@ export class CasesConnectorExecutor { private readonly casesOracleService: CasesOracleService; private readonly casesService: CasesService; private readonly casesClient: CasesClient; + private readonly spaceId: string; constructor({ logger, casesOracleService, casesService, casesClient, + spaceId, }: CasesConnectorExecutorParams) { this.logger = logger; this.casesOracleService = casesOracleService; this.casesService = casesService; this.casesClient = casesClient; + this.spaceId = spaceId; } public async execute(params: CasesConnectorRunParams) { @@ -209,10 +213,6 @@ export class CasesConnectorExecutor { ); const { rule, owner } = params; - /** - * TODO: Take spaceId from the actions framework - */ - const spaceId = 'default'; const oracleMap = new Map(); @@ -221,7 +221,7 @@ export class CasesConnectorExecutor { ruleId: rule.id, grouping, owner, - spaceId, + spaceId: this.spaceId, }; const oracleKey = this.casesOracleService.getRecordId(getRecordIdParams); @@ -494,11 +494,6 @@ export class CasesConnectorExecutor { const { rule, owner } = params; - /** - * TODO: Take spaceId from the actions framework - */ - const spaceId = 'default'; - const casesMap = new Map(); for (const [recordId, entry] of groupedAlertsWithOracleRecords.entries()) { @@ -506,7 +501,7 @@ export class CasesConnectorExecutor { ruleId: rule.id, grouping: entry.grouping, owner, - spaceId, + spaceId: this.spaceId, counter: entry.oracleRecord.counter, }; diff --git a/x-pack/plugins/cases/server/connectors/cases/index.ts b/x-pack/plugins/cases/server/connectors/cases/index.ts index b45a1b1bdffcb..f53c447742e53 100644 --- a/x-pack/plugins/cases/server/connectors/cases/index.ts +++ b/x-pack/plugins/cases/server/connectors/cases/index.ts @@ -16,10 +16,12 @@ import type { CasesClient } from '../../client'; interface GetCasesConnectorTypeArgs { getCasesClient: (request: KibanaRequest) => Promise; + getSpaceId: (request?: KibanaRequest) => string; } export const getCasesConnectorType = ({ getCasesClient, + getSpaceId, }: GetCasesConnectorTypeArgs): SubActionConnectorType< CasesConnectorConfig, CasesConnectorSecrets @@ -27,7 +29,7 @@ export const getCasesConnectorType = ({ id: CASES_CONNECTOR_ID, name: CASES_CONNECTOR_TITLE, getService: (params) => - new CasesConnector({ casesParams: { getCasesClient }, connectorParams: params }), + new CasesConnector({ casesParams: { getCasesClient, getSpaceId }, connectorParams: params }), schema: { config: CasesConnectorConfigSchema, secrets: CasesConnectorSecretsSchema, diff --git a/x-pack/plugins/cases/server/connectors/index.ts b/x-pack/plugins/cases/server/connectors/index.ts index 7e83e97b2e6e1..bf4e43ff3a49a 100644 --- a/x-pack/plugins/cases/server/connectors/index.ts +++ b/x-pack/plugins/cases/server/connectors/index.ts @@ -16,9 +16,11 @@ export { casesConnectors } from './factory'; export function registerConnectorTypes({ actions, getCasesClient, + getSpaceId, }: { actions: ActionsPluginSetupContract; getCasesClient: (request: KibanaRequest) => Promise; + getSpaceId: (request?: KibanaRequest) => string; }) { - actions.registerSubActionConnectorType(getCasesConnectorType({ getCasesClient })); + actions.registerSubActionConnectorType(getCasesConnectorType({ getCasesClient, getSpaceId })); } diff --git a/x-pack/plugins/cases/server/plugin.ts b/x-pack/plugins/cases/server/plugin.ts index 0ce4eb626cb35..3ebd9321f302c 100644 --- a/x-pack/plugins/cases/server/plugin.ts +++ b/x-pack/plugins/cases/server/plugin.ts @@ -20,7 +20,7 @@ import type { PluginSetupContract as ActionsPluginSetup, PluginStartContract as ActionsPluginStart, } from '@kbn/actions-plugin/server'; -import type { SpacesPluginStart } from '@kbn/spaces-plugin/server'; +import type { SpacesPluginSetup, SpacesPluginStart } from '@kbn/spaces-plugin/server'; import type { PluginStartContract as FeaturesPluginStart, PluginSetupContract as FeaturesPluginSetup, @@ -35,6 +35,7 @@ import type { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing- import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; import type { RuleRegistryPluginStartContract } from '@kbn/rule-registry-plugin/server'; +import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; import { APP_ID } from '../common/constants'; import { createCaseCommentSavedObjectType, @@ -72,6 +73,7 @@ export interface PluginsSetup { licensing: LicensingPluginSetup; taskManager?: TaskManagerSetupContract; usageCollection?: UsageCollectionSetup; + spaces?: SpacesPluginSetup; } export interface PluginsStart { @@ -184,7 +186,15 @@ export class CasePlugin { return this.getCasesClientWithRequest(coreStart)(request); }; - registerConnectorTypes({ actions: plugins.actions, getCasesClient }); + const getSpaceId = (request?: KibanaRequest) => { + if (!request) { + return DEFAULT_SPACE_ID; + } + + return plugins.spaces?.spacesService.getSpaceId(request) ?? DEFAULT_SPACE_ID; + }; + + registerConnectorTypes({ actions: plugins.actions, getCasesClient, getSpaceId }); return { attachmentFramework: { From f80150b96ceed3d2038a74d1f5f35ba987e40179 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 12:28:49 +0200 Subject: [PATCH 03/12] Improve case creation request --- .../cases/cases_connector_executor.test.ts | 243 +++++++++++++++++- .../cases/cases_connector_executor.ts | 57 ++-- .../connectors/cases/cases_oracle_service.ts | 3 +- .../server/connectors/cases/constants.ts | 1 + .../server/connectors/cases/utils.test.ts | 34 ++- .../cases/server/connectors/cases/utils.ts | 18 +- 6 files changed, 327 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index 4736d7fea3e7c..16b35163ef835 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -8,7 +8,12 @@ import dateMath from '@kbn/datemath'; import moment from 'moment'; import { CasesConnectorExecutor } from './cases_connector_executor'; -import { CASE_ORACLE_SAVED_OBJECT, MAX_ALERTS_PER_CASE } from '../../../common/constants'; +import { + CASE_ORACLE_SAVED_OBJECT, + MAX_ALERTS_PER_CASE, + MAX_LENGTH_PER_TAG, + MAX_TAGS_PER_CASE, +} from '../../../common/constants'; import { CasesOracleService } from './cases_oracle_service'; import { CasesService } from './cases_service'; import { createCasesClientMock } from '../../client/mocks'; @@ -231,7 +236,13 @@ describe('CasesConnectorExecutor', () => { settings: { syncAlerts: false, }, - tags: ['auto-generated', ...rule.tags], + tags: [ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:A', + 'dest.ip:0.0.0.1', + ...rule.tags, + ], connector: { fields: null, id: 'none', @@ -248,7 +259,13 @@ describe('CasesConnectorExecutor', () => { settings: { syncAlerts: false, }, - tags: ['auto-generated', ...rule.tags], + tags: [ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:B', + 'dest.ip:0.0.0.1', + ...rule.tags, + ], connector: { fields: null, id: 'none', @@ -265,7 +282,13 @@ describe('CasesConnectorExecutor', () => { settings: { syncAlerts: false, }, - tags: ['auto-generated', ...rule.tags], + tags: [ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:B', + 'dest.ip:0.0.0.3', + ...rule.tags, + ], connector: { fields: null, id: 'none', @@ -503,7 +526,13 @@ describe('CasesConnectorExecutor', () => { settings: { syncAlerts: false, }, - tags: ['auto-generated', ...rule.tags], + tags: [ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:B', + 'dest.ip:0.0.0.3', + ...rule.tags, + ], connector: { fields: null, id: 'none', @@ -515,6 +544,202 @@ describe('CasesConnectorExecutor', () => { }); }); + it('does not add the rule URL to the description if the ruleUrl is null', async () => { + mockBulkGetRecords.mockResolvedValue([oracleRecords[0]]); + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[0]] }); + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: 'mock-id-1', + }, + ], + }); + + await connectorExecutor.execute({ + ...params, + rule: { ...params.rule, ruleUrl: null }, + }); + + const description = + casesClientMock.cases.bulkCreate.mock.calls[0][0].cases[0].description; + + expect(description).toBe( + 'This case is auto-created by Test rule. \n\n Grouping: `host.name` equals `A` and `dest.ip` equals `0.0.0.1`' + ); + }); + + it('converts grouping values in the description correctly', async () => { + mockBulkGetRecords.mockResolvedValue([oracleRecords[0]]); + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[0]] }); + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: 'mock-id-1', + }, + ], + }); + + await connectorExecutor.execute({ + ...params, + alerts: [ + { + _id: 'test-id', + _index: 'test-index', + foo: ['bar', 1, true, {}], + bar: { foo: 'test' }, + baz: 'my value', + }, + ], + groupingBy: ['foo', 'bar', 'baz'], + }); + + const description = + casesClientMock.cases.bulkCreate.mock.calls[0][0].cases[0].description; + + expect(description).toBe( + 'This case is auto-created by [Test rule](https://example.com/rules/rule-test-id). \n\n Grouping: `foo` equals `["bar",1,true,{}]` and `bar` equals `{"foo":"test"}` and `baz` equals `my value`' + ); + }); + + it('adds the counter correctly if it is bigger than INITIAL_ORACLE_RECORD_COUNTER', async () => { + mockBulkGetRecords.mockResolvedValue([{ ...oracleRecords[0], counter: 2 }]); + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[0]] }); + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: 'mock-id-1', + }, + ], + }); + + await connectorExecutor.execute({ ...params, groupingBy: [] }); + const title = casesClientMock.cases.bulkCreate.mock.calls[0][0].cases[0].title; + + expect(title).toBe('Test rule (2) (Auto-created)'); + }); + + it(`trims tags that are bigger than ${MAX_LENGTH_PER_TAG} characters`, async () => { + mockBulkGetRecords.mockResolvedValue([oracleRecords[0]]); + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[0]] }); + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: 'mock-id-1', + }, + ], + }); + + await connectorExecutor.execute({ + ...params, + rule: { ...params.rule, tags: ['a'.repeat(MAX_LENGTH_PER_TAG * 2)] }, + }); + + const tags = casesClientMock.cases.bulkCreate.mock.calls[0][0].cases[0].tags; + + expect(tags).toEqual([ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:A', + 'dest.ip:0.0.0.1', + 'a'.repeat(MAX_LENGTH_PER_TAG), + ]); + }); + + it(`create cases with up to ${MAX_TAGS_PER_CASE} tags`, async () => { + mockBulkGetRecords.mockResolvedValue([oracleRecords[0]]); + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[0]] }); + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: 'mock-id-1', + }, + ], + }); + + await connectorExecutor.execute({ + ...params, + rule: { ...params.rule, tags: Array(MAX_TAGS_PER_CASE * 2).fill('foo') }, + }); + + const tags = casesClientMock.cases.bulkCreate.mock.calls[0][0].cases[0].tags; + const systemTags = [ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:A', + 'dest.ip:0.0.0.1', + ]; + + expect(tags).toEqual([ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:A', + 'dest.ip:0.0.0.1', + ...Array(MAX_TAGS_PER_CASE - systemTags.length).fill('foo'), + ]); + }); + + it('converts grouping values in tags correctly', async () => { + mockBulkGetRecords.mockResolvedValue([oracleRecords[0]]); + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[0]] }); + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: 'mock-id-1', + }, + ], + }); + + await connectorExecutor.execute({ + ...params, + alerts: [ + { + _id: 'test-id', + _index: 'test-index', + foo: ['bar', 1, true, {}], + bar: { foo: 'test' }, + baz: 'my value', + }, + ], + groupingBy: ['foo', 'bar', 'baz'], + }); + + const tags = casesClientMock.cases.bulkCreate.mock.calls[0][0].cases[0].tags; + + expect(tags).toEqual([ + 'auto-generated', + 'rule:rule-test-id', + 'foo:["bar",1,true,{}]', + 'bar:{"foo":"test"}', + 'baz:my value', + 'rule', + 'test', + ]); + }); + it('does not reopen closed cases if reopenClosedCases=false', async () => { casesClientMock.cases.bulkGet.mockResolvedValue({ cases: [{ ...cases[0], status: CaseStatuses.closed }], @@ -573,7 +798,13 @@ describe('CasesConnectorExecutor', () => { settings: { syncAlerts: false, }, - tags: ['auto-generated', ...rule.tags], + tags: [ + 'auto-generated', + 'rule:rule-test-id', + 'host.name:A', + 'dest.ip:0.0.0.1', + ...rule.tags, + ], connector: { fields: null, id: 'none', diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts index c108baccfba4f..1634809e717af 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts @@ -12,14 +12,22 @@ import dateMath from '@kbn/datemath'; import { CaseStatuses } from '@kbn/cases-components'; import type { SavedObjectError } from '@kbn/core-saved-objects-common'; import type { Logger } from '@kbn/core/server'; -import { MAX_ALERTS_PER_CASE } from '../../../common/constants'; +import { + MAX_ALERTS_PER_CASE, + MAX_LENGTH_PER_TAG, + MAX_TAGS_PER_CASE, +} from '../../../common/constants'; import type { BulkCreateCasesRequest } from '../../../common/types/api'; import type { Case } from '../../../common'; import { ConnectorTypes, AttachmentType } from '../../../common'; -import { MAX_CONCURRENT_ES_REQUEST, MAX_OPEN_CASES } from './constants'; +import { + INITIAL_ORACLE_RECORD_COUNTER, + MAX_CONCURRENT_ES_REQUEST, + MAX_OPEN_CASES, +} from './constants'; import type { BulkCreateOracleRecordRequest, CasesConnectorRunParams, OracleRecord } from './types'; import type { CasesOracleService } from './cases_oracle_service'; -import { partitionByNonFoundErrors, partitionRecordsByError } from './utils'; +import { convertValueToString, partitionByNonFoundErrors, partitionRecordsByError } from './utils'; import type { CasesService } from './cases_service'; import type { CasesClient } from '../../client'; import type { BulkCreateArgs as BulkCreateAlertsReq } from '../../client/attachments/types'; @@ -649,35 +657,27 @@ export class CasesConnectorExecutor { params: CasesConnectorRunParams, groupingData: GroupedAlertsWithCaseId ): Omit & { id: string } { - const { grouping, caseId } = groupingData; + const { grouping, caseId, oracleRecord } = groupingData; const ruleName = params.rule.ruleUrl ? `[${params.rule.name}](${params.rule.ruleUrl})` : params.rule.name; const groupingDescription = this.getGroupingDescription(grouping); - const description = `This case is auto-created by ${ruleName}. \n\n Grouping: ${groupingDescription}`; + const title = + oracleRecord.counter === INITIAL_ORACLE_RECORD_COUNTER + ? `${params.rule.name} (Auto-created)` + : `${params.rule.name} (${oracleRecord.counter}) (Auto-created)`; - const tags = Array.isArray(params.rule.tags) ? params.rule.tags : []; - - /** - * TODO: - * 1. Add grouping info to - * 2. Required custom fields will throw an error when creating a case. - * We should find a way to fill the custom fields with default values. - */ return { id: caseId, description, - tags: ['auto-generated', ...tags], - /** - * TODO: Append the counter to the name - */ - title: `${params.rule.name} (Auto-created)`, + tags: this.getCaseTags(params, grouping), + title, connector: { id: 'none', name: 'none', type: ConnectorTypes.none, fields: null }, /** - * Turn on for Security solution + * TODO: Turn on for Security solution */ settings: { syncAlerts: false }, owner: params.owner, @@ -691,13 +691,30 @@ export class CasesConnectorExecutor { return Object.entries(grouping) .map(([key, value]) => { const keyAsCodeBlock = `\`${key}\``; - const valueAsCodeBlock = `\`${value}\``; + const valueAsCodeBlock = `\`${convertValueToString(value)}\``; return `${keyAsCodeBlock} equals ${valueAsCodeBlock}`; }) .join(' and '); } + private getCaseTags(params: CasesConnectorRunParams, grouping: GroupedAlerts['grouping']) { + const ruleTags = Array.isArray(params.rule.tags) ? params.rule.tags : []; + + return [ + 'auto-generated', + `rule:${params.rule.id}`, + ...this.getGroupingAsTags(grouping), + ...ruleTags, + ] + .splice(0, MAX_TAGS_PER_CASE) + .map((tag) => tag.slice(0, MAX_LENGTH_PER_TAG)); + } + + private getGroupingAsTags(grouping: GroupedAlerts['grouping']) { + return Object.entries(grouping).map(([key, value]) => `${key}:${convertValueToString(value)}`); + } + private async handleClosedCases( params: CasesConnectorRunParams, casesMap: Map diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts index b17f7974863ed..4f36d8eeff4bc 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts @@ -9,6 +9,7 @@ import type { Logger, SavedObject, SavedObjectsClientContract } from '@kbn/core/ import { CASE_ORACLE_SAVED_OBJECT } from '../../../common/constants'; import type { SavedObjectsBulkResponseWithErrors } from '../../common/types'; import { isSOError } from '../../common/utils'; +import { INITIAL_ORACLE_RECORD_COUNTER } from './constants'; import { CryptoService } from './crypto_service'; import type { BulkCreateOracleRecordRequest, @@ -199,7 +200,7 @@ export class CasesOracleService { private getCreateRecordAttributes({ cases, rules, grouping }: OracleRecordCreateRequest) { return { - counter: 1, + counter: INITIAL_ORACLE_RECORD_COUNTER, cases, rules, grouping, diff --git a/x-pack/plugins/cases/server/connectors/cases/constants.ts b/x-pack/plugins/cases/server/connectors/cases/constants.ts index 01c2046c528b1..34b933b3dc013 100644 --- a/x-pack/plugins/cases/server/connectors/cases/constants.ts +++ b/x-pack/plugins/cases/server/connectors/cases/constants.ts @@ -9,6 +9,7 @@ export const CASES_CONNECTOR_ID = '.cases'; export const CASES_CONNECTOR_TITLE = 'Cases'; export const MAX_CONCURRENT_ES_REQUEST = 5; export const MAX_OPEN_CASES = 10; +export const INITIAL_ORACLE_RECORD_COUNTER = 1; export enum CASES_CONNECTOR_SUB_ACTION { RUN = 'run', diff --git a/x-pack/plugins/cases/server/connectors/cases/utils.test.ts b/x-pack/plugins/cases/server/connectors/cases/utils.test.ts index 9652414bff9c7..bed0d7c996e6b 100644 --- a/x-pack/plugins/cases/server/connectors/cases/utils.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/utils.test.ts @@ -6,7 +6,7 @@ */ import { oracleRecordError, oracleRecord } from './index.mock'; -import { isRecordError, partitionRecordsByError } from './utils'; +import { convertValueToString, isRecordError, partitionRecordsByError } from './utils'; describe('utils', () => { describe('isRecordError', () => { @@ -34,4 +34,36 @@ describe('utils', () => { ]); }); }); + + describe('convertValueToString', () => { + it('converts null correctly', () => { + expect(convertValueToString(null)).toBe(''); + }); + + it('converts undefined correctly', () => { + expect(convertValueToString(undefined)).toBe(''); + }); + + it('converts an array correctly', () => { + expect(convertValueToString([1, 2, 'foo', { foo: 'bar' }])).toBe('[1,2,"foo",{"foo":"bar"}]'); + }); + + it('converts an object correctly', () => { + expect(convertValueToString({ foo: 'bar', baz: 2, qux: [1, 2, 'foo'] })).toBe( + '{"foo":"bar","baz":2,"qux":[1,2,"foo"]}' + ); + }); + + it('converts a number correctly', () => { + expect(convertValueToString(5.2)).toBe('5.2'); + }); + + it('converts a string correctly', () => { + expect(convertValueToString('foo')).toBe('foo'); + }); + + it('converts a boolean correctly', () => { + expect(convertValueToString(true)).toBe('true'); + }); + }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/utils.ts b/x-pack/plugins/cases/server/connectors/cases/utils.ts index e1e5596b047af..ad2b46333b744 100644 --- a/x-pack/plugins/cases/server/connectors/cases/utils.ts +++ b/x-pack/plugins/cases/server/connectors/cases/utils.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { partition } from 'lodash'; +import { isPlainObject, partition, toString } from 'lodash'; import type { BulkGetOracleRecordsResponse, OracleRecord, OracleRecordError } from './types'; export const isRecordError = (so: OracleRecord | OracleRecordError): so is OracleRecordError => @@ -32,3 +32,19 @@ export const partitionByNonFoundErrors = { + if (value == null) { + return ''; + } + + if (Array.isArray(value) || isPlainObject(value)) { + try { + return JSON.stringify(value); + } catch (error) { + return ''; + } + } + + return toString(value); +}; From 011f973719a9fcaf327434cfed8a765db83a2663 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 12:32:03 +0200 Subject: [PATCH 04/12] Remove limits from tags --- .../server/connectors/cases/schema.test.ts | 22 ------------------- .../cases/server/connectors/cases/schema.ts | 8 +------ 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/schema.test.ts b/x-pack/plugins/cases/server/connectors/cases/schema.test.ts index efce618425678..bf910fae3eb6e 100644 --- a/x-pack/plugins/cases/server/connectors/cases/schema.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/schema.test.ts @@ -75,28 +75,6 @@ describe('CasesConnectorRunParamsSchema', () => { ).not.toThrow(); }); - it('does not accept more than 10 tags', () => { - const params = getParams(); - - expect(() => - CasesConnectorRunParamsSchema.validate({ - ...params, - rule: { ...params.rule, tags: Array(11).fill('test') }, - }) - ).toThrow(); - }); - - it('does not accept a tag that is more than 50 characters', () => { - const params = getParams(); - - expect(() => - CasesConnectorRunParamsSchema.validate({ - ...params, - rule: { ...params.rule, tags: ['x'.repeat(51)] }, - }) - ).toThrow(); - }); - it('does not accept an empty tag', () => { const params = getParams(); diff --git a/x-pack/plugins/cases/server/connectors/cases/schema.ts b/x-pack/plugins/cases/server/connectors/cases/schema.ts index 7a2bb8c26e2ac..ddea351e2ffe2 100644 --- a/x-pack/plugins/cases/server/connectors/cases/schema.ts +++ b/x-pack/plugins/cases/server/connectors/cases/schema.ts @@ -25,13 +25,7 @@ const GroupingSchema = schema.arrayOf(schema.string(), { minSize: 0, maxSize: 1 const RuleSchema = schema.object({ id: schema.string(), name: schema.string(), - /** - * TODO: Verify limits - */ - tags: schema.arrayOf(schema.string({ minLength: 1, maxLength: 50 }), { - minSize: 0, - maxSize: 10, - }), + tags: schema.arrayOf(schema.string(), { defaultValue: [] }), ruleUrl: schema.nullable(schema.string()), }); From a6991bdc48c216293d34e06f3e151ef21c43ab04 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 12:47:24 +0200 Subject: [PATCH 05/12] Add rule info when creating an oracle record --- .../cases/cases_connector_executor.test.ts | 30 +++++++++++++++---- .../cases/cases_connector_executor.ts | 13 ++++---- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index 16b35163ef835..f820cc0ed5b1a 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -198,7 +198,11 @@ describe('CasesConnectorExecutor', () => { payload: { cases: [], grouping: groupedAlertsWithOracleKey[0].grouping, - rules: [], + rules: [ + { + id: 'rule-test-id', + }, + ], }, }, { @@ -206,7 +210,11 @@ describe('CasesConnectorExecutor', () => { payload: { cases: [], grouping: groupedAlertsWithOracleKey[1].grouping, - rules: [], + rules: [ + { + id: 'rule-test-id', + }, + ], }, }, { @@ -214,7 +222,11 @@ describe('CasesConnectorExecutor', () => { payload: { cases: [], grouping: groupedAlertsWithOracleKey[2].grouping, - rules: [], + rules: [ + { + id: 'rule-test-id', + }, + ], }, }, ]); @@ -361,7 +373,11 @@ describe('CasesConnectorExecutor', () => { payload: { cases: [], grouping: groupedAlertsWithOracleKey[2].grouping, - rules: [], + rules: [ + { + id: 'rule-test-id', + }, + ], }, }, ]); @@ -397,7 +413,11 @@ describe('CasesConnectorExecutor', () => { payload: { cases: [], grouping: groupedAlertsWithOracleKey[2].grouping, - rules: [], + rules: [ + { + id: 'rule-test-id', + }, + ], }, }, ]); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts index 1634809e717af..c51084a47f885 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts @@ -93,7 +93,7 @@ export class CasesConnectorExecutor { * A record does not exist if it is the first time the connector run for a specific grouping. * The returned map will contain all records old and new. */ - const oracleRecordsMap = await this.upsertOracleRecords(groupedAlertsWithOracleKey); + const oracleRecordsMap = await this.upsertOracleRecords(params, groupedAlertsWithOracleKey); /** * If the time window has passed for a case we need to create a new case. @@ -254,6 +254,7 @@ export class CasesConnectorExecutor { } private async upsertOracleRecords( + params: CasesConnectorRunParams, groupedAlertsWithOracleKey: Map ): Promise> { this.logger.debug( @@ -325,8 +326,11 @@ export class CasesConnectorExecutor { const record = groupedAlertsWithOracleKey.get(error.id); bulkCreateReq.push({ recordId: error.id, - // TODO: Add the rule info - payload: { cases: [], rules: [], grouping: record?.grouping ?? {} }, + payload: { + cases: [], + rules: [{ id: params.rule.id }], + grouping: record?.grouping ?? {}, + }, }); } } @@ -685,9 +689,6 @@ export class CasesConnectorExecutor { } private getGroupingDescription(grouping: GroupedAlerts['grouping']) { - /** - * TODO: Handle multi values - */ return Object.entries(grouping) .map(([key, value]) => { const keyAsCodeBlock = `\`${key}\``; From 5846a53e527df1bd16d4e5e8d44616ea68f66365 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 13:02:48 +0200 Subject: [PATCH 06/12] Rename unsecuredSavedObjectsClient to savedObjectsClient --- .../connectors/cases/cases_connector.ts | 7 +--- .../cases/cases_oracle_service.test.ts | 34 +++++++++---------- .../connectors/cases/cases_oracle_service.ts | 30 ++++++++-------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts index b7fc42f2054c9..deab28a3c7c0b 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector.ts @@ -45,12 +45,7 @@ export class CasesConnector extends SubActionConnector< this.casesOracleService = new CasesOracleService({ logger: this.logger, - /** - * TODO: Think about permissions etc. - * Should we use our own savedObjectsClient as we do - * in the cases client? Should we so the createInternalRepository? - */ - unsecuredSavedObjectsClient: this.savedObjectsClient, + savedObjectsClient: this.savedObjectsClient, }); this.casesService = new CasesService(); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts index 6c8186f5fc221..b80c9b0d22138 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts @@ -15,14 +15,14 @@ import { CASE_ORACLE_SAVED_OBJECT } from '../../../common/constants'; import { isEmpty, set } from 'lodash'; describe('CasesOracleService', () => { - const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); + const savedObjectsClient = savedObjectsClientMock.create(); const logger = loggingSystemMock.createLogger(); let service: CasesOracleService; beforeEach(() => { jest.resetAllMocks(); - service = new CasesOracleService({ unsecuredSavedObjectsClient, logger }); + service = new CasesOracleService({ savedObjectsClient, logger }); }); describe('getRecordId', () => { @@ -166,7 +166,7 @@ describe('CasesOracleService', () => { }; beforeEach(() => { - unsecuredSavedObjectsClient.get.mockResolvedValue(oracleSO); + savedObjectsClient.get.mockResolvedValue(oracleSO); }); it('gets a record correctly', async () => { @@ -175,10 +175,10 @@ describe('CasesOracleService', () => { expect(record).toEqual({ ...oracleSO.attributes, id: 'so-id', version: 'so-version' }); }); - it('calls the unsecuredSavedObjectsClient.get method correctly', async () => { + it('calls the savedObjectsClient.get method correctly', async () => { await service.getRecord('so-id'); - expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith('cases-oracle', 'so-id'); + expect(savedObjectsClient.get).toHaveBeenCalledWith('cases-oracle', 'so-id'); }); }); @@ -215,7 +215,7 @@ describe('CasesOracleService', () => { beforeEach(() => { // @ts-expect-error: types of the SO client are wrong and they do not accept errors - unsecuredSavedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: bulkGetSOs }); + savedObjectsClient.bulkGet.mockResolvedValue({ saved_objects: bulkGetSOs }); }); it('formats the response correctly', async () => { @@ -227,10 +227,10 @@ describe('CasesOracleService', () => { ]); }); - it('calls the unsecuredSavedObjectsClient.bulkGet method correctly', async () => { + it('calls the savedObjectsClient.bulkGet method correctly', async () => { await service.bulkGetRecords(['so-id', 'so-id-2']); - expect(unsecuredSavedObjectsClient.bulkGet).toHaveBeenCalledWith([ + expect(savedObjectsClient.bulkGet).toHaveBeenCalledWith([ { id: 'so-id', type: 'cases-oracle' }, { id: 'so-id-2', type: 'cases-oracle' }, ]); @@ -258,7 +258,7 @@ describe('CasesOracleService', () => { }; beforeEach(() => { - unsecuredSavedObjectsClient.create.mockResolvedValue(oracleSO); + savedObjectsClient.create.mockResolvedValue(oracleSO); }); it('creates a record correctly', async () => { @@ -267,12 +267,12 @@ describe('CasesOracleService', () => { expect(record).toEqual({ ...oracleSO.attributes, id: 'so-id', version: 'so-version' }); }); - it('calls the unsecuredSavedObjectsClient.create method correctly', async () => { + it('calls the savedObjectsClient.create method correctly', async () => { const id = 'so-id'; await service.createRecord(id, { cases, rules, grouping }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + expect(savedObjectsClient.create).toHaveBeenCalledWith( 'cases-oracle', { cases, @@ -320,7 +320,7 @@ describe('CasesOracleService', () => { beforeEach(() => { // @ts-expect-error: types of the SO client are wrong and they do not accept errors - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: bulkCreateSOs }); + savedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: bulkCreateSOs }); }); it('formats the response correctly', async () => { @@ -341,7 +341,7 @@ describe('CasesOracleService', () => { { recordId: 'so-id-2', payload: { cases, rules, grouping } }, ]); - expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith([ + expect(savedObjectsClient.bulkCreate).toHaveBeenCalledWith([ { attributes: { cases, @@ -396,8 +396,8 @@ describe('CasesOracleService', () => { }; beforeEach(() => { - unsecuredSavedObjectsClient.get.mockResolvedValue(oracleSO); - unsecuredSavedObjectsClient.update.mockResolvedValue(oracleSOWithIncreasedCounter); + savedObjectsClient.get.mockResolvedValue(oracleSO); + savedObjectsClient.update.mockResolvedValue(oracleSOWithIncreasedCounter); }); it('increases the counter correctly', async () => { @@ -411,10 +411,10 @@ describe('CasesOracleService', () => { }); }); - it('calls the unsecuredSavedObjectsClient.update method correctly', async () => { + it('calls the savedObjectsClient.update method correctly', async () => { await service.increaseCounter('so-id'); - expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( + expect(savedObjectsClient.update).toHaveBeenCalledWith( 'cases-oracle', 'so-id', { diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts index 4f36d8eeff4bc..11e7caf9a2256 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts @@ -27,18 +27,18 @@ export class CasesOracleService { * TODO: Think about permissions etc. * Should we authorize based on the owner? */ - private readonly unsecuredSavedObjectsClient: SavedObjectsClientContract; + private readonly savedObjectsClient: SavedObjectsClientContract; private cryptoService: CryptoService; constructor({ logger, - unsecuredSavedObjectsClient, + savedObjectsClient, }: { logger: Logger; - unsecuredSavedObjectsClient: SavedObjectsClientContract; + savedObjectsClient: SavedObjectsClientContract; }) { this.logger = logger; - this.unsecuredSavedObjectsClient = unsecuredSavedObjectsClient; + this.savedObjectsClient = savedObjectsClient; this.cryptoService = new CryptoService(); } @@ -64,7 +64,7 @@ export class CasesOracleService { tags: ['cases-oracle-service', 'getRecord', recordId], }); - const oracleRecord = await this.unsecuredSavedObjectsClient.get( + const oracleRecord = await this.savedObjectsClient.get( CASE_ORACLE_SAVED_OBJECT, recordId ); @@ -77,7 +77,7 @@ export class CasesOracleService { tags: ['cases-oracle-service', 'bulkGetRecords', ...ids], }); - const oracleRecords = (await this.unsecuredSavedObjectsClient.bulkGet( + const oracleRecords = (await this.savedObjectsClient.bulkGet( ids.map((id) => ({ id, type: CASE_ORACLE_SAVED_OBJECT })) )) as SavedObjectsBulkResponseWithErrors; @@ -92,7 +92,7 @@ export class CasesOracleService { tags: ['cases-oracle-service', 'createRecord', recordId], }); - const oracleRecord = await this.unsecuredSavedObjectsClient.create( + const oracleRecord = await this.savedObjectsClient.create( CASE_ORACLE_SAVED_OBJECT, this.getCreateRecordAttributes(payload), { id: recordId } @@ -116,10 +116,9 @@ export class CasesOracleService { attributes: this.getCreateRecordAttributes(record.payload), })); - const oracleRecords = - (await this.unsecuredSavedObjectsClient.bulkCreate( - req - )) as SavedObjectsBulkResponseWithErrors; + const oracleRecords = (await this.savedObjectsClient.bulkCreate( + req + )) as SavedObjectsBulkResponseWithErrors; return this.getBulkRecordsResponse(oracleRecords); } @@ -135,7 +134,7 @@ export class CasesOracleService { } ); - const oracleRecord = await this.unsecuredSavedObjectsClient.update( + const oracleRecord = await this.savedObjectsClient.update( CASE_ORACLE_SAVED_OBJECT, recordId, { counter: newCounter }, @@ -165,10 +164,9 @@ export class CasesOracleService { attributes: { ...record.payload, updatedAt: new Date().toISOString() }, })); - const oracleRecords = - (await this.unsecuredSavedObjectsClient.bulkUpdate( - req - )) as SavedObjectsBulkResponseWithErrors; + const oracleRecords = (await this.savedObjectsClient.bulkUpdate( + req + )) as SavedObjectsBulkResponseWithErrors; return this.getBulkRecordsResponse(oracleRecords); } From 53a90aa3a10f901e096b788c103e2aa1c927d385 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 13:32:36 +0200 Subject: [PATCH 07/12] Log when reaching the limits of 1K alerts --- .../cases/cases_connector_executor.test.ts | 8 ++++++ .../cases/cases_connector_executor.ts | 28 +++++++++++++++---- .../connectors/cases/cases_oracle_service.ts | 4 --- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index f820cc0ed5b1a..eecdd6452a144 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -940,6 +940,10 @@ describe('CasesConnectorExecutor', () => { await connectorExecutor.execute(params); expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(0); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Cases with ids "mock-id-1" contain more than 1000 alerts. The new alerts will not be attached to the cases. Total new alerts: 1', + { tags: ['rule:rule-test-id'] } + ); }); it('does not attach alerts to cases when attaching the new alerts will surpass the limit', async () => { @@ -956,6 +960,10 @@ describe('CasesConnectorExecutor', () => { await connectorExecutor.execute(params); expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(0); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Cases with ids "mock-id-1" contain more than 1000 alerts. The new alerts will not be attached to the cases. Total new alerts: 1', + { tags: ['rule:rule-test-id'] } + ); }); it('attach alerts to cases when attaching the new alerts will be equal to the limit', async () => { diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts index c51084a47f885..56bf1b2484d70 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts @@ -7,7 +7,7 @@ import stringify from 'json-stable-stringify'; import pMap from 'p-map'; -import { pick } from 'lodash'; +import { partition, pick } from 'lodash'; import dateMath from '@kbn/datemath'; import { CaseStatuses } from '@kbn/cases-components'; import type { SavedObjectError } from '@kbn/core-saved-objects-common'; @@ -906,14 +906,23 @@ export class CasesConnectorExecutor { const { rule } = params; - /** - * TODO: Log that we could not attach the alerts to the cases - * that have reached out the limit - */ - const casesUnderAlertLimit = Array.from(groupedAlertsWithCases.values()).filter( + const [casesUnderAlertLimit, casesOverAlertLimit] = partition( + Array.from(groupedAlertsWithCases.values()), ({ theCase, alerts }) => theCase.totalAlerts + alerts.length <= MAX_ALERTS_PER_CASE ); + if (casesOverAlertLimit.length > 0) { + const ids = casesOverAlertLimit.map(({ theCase }) => theCase.id); + const totalAlerts = casesOverAlertLimit.map(({ alerts }) => alerts.length).flat().length; + + this.logger.warn( + `Cases with ids "${ids.join( + ',' + )}" contain more than ${MAX_ALERTS_PER_CASE} alerts. The new alerts will not be attached to the cases. Total new alerts: ${totalAlerts}`, + this.getLogMetadata(params) + ); + } + this.logger.debug( `[CasesConnector][CasesConnectorExecutor][attachAlertsToCases] Attaching alerts to ${casesUnderAlertLimit.length} cases that do not have reach the alert limit per case`, { @@ -981,4 +990,11 @@ export class CasesConnectorExecutor { throw new CasesConnectorError(message, firstError.statusCode); } + + private getLogMetadata( + params: CasesConnectorRunParams, + { extraTags = [] }: { extraTags?: string[] } = {} + ) { + return { tags: [...extraTags, `rule:${params.rule.id}`] }; + } } diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts index 11e7caf9a2256..1bd5b209c52c4 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts @@ -23,10 +23,6 @@ import type { export class CasesOracleService { private readonly logger: Logger; - /** - * TODO: Think about permissions etc. - * Should we authorize based on the owner? - */ private readonly savedObjectsClient: SavedObjectsClientContract; private cryptoService: CryptoService; From 4cb64eec4b49e7f666e0502164e3178e3d1e51ba Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 17:15:27 +0200 Subject: [PATCH 08/12] Add tests for missing data --- .../cases/cases_connector_executor.test.ts | 195 ++++++++++++++++++ 1 file changed, 195 insertions(+) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index eecdd6452a144..62ecf7cc9755e 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -2215,4 +2215,199 @@ describe('CasesConnectorExecutor', () => { }); }); }); + + describe('Sequence of executions with missing oracle or cases', () => { + const missingDataParams = { + ...params, + alerts: [ + { + _id: 'test-id', + _index: 'test-index', + foo: 'bar', + }, + ], + groupingBy: ['foo'], + }; + + it('oracle counter increases but some cases are missing', async () => { + mockGetRecordId.mockReturnValue(oracleRecords[0].id); + mockBulkGetRecords + .mockResolvedValueOnce([oracleRecords[0]]) + .mockResolvedValueOnce([{ ...oracleRecords[0], counter: 2 }]) + .mockResolvedValueOnce([{ ...oracleRecords[0], counter: 3 }]); + + mockGetCaseId + .mockReturnValueOnce('mock-id-1') + .mockReturnValueOnce('mock-id-2') + .mockReturnValueOnce('mock-id-3'); + + casesClientMock.cases.bulkGet + .mockResolvedValueOnce({ + cases: [cases[0]], + errors: [], + }) + .mockResolvedValueOnce({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: cases[1].id, + }, + ], + }) + .mockResolvedValueOnce({ + cases: [cases[2]], + errors: [], + }); + + casesClientMock.cases.bulkCreate.mockResolvedValue({ cases: [cases[1]] }); + + await connectorExecutor.execute(missingDataParams); + await connectorExecutor.execute(missingDataParams); + await connectorExecutor.execute(missingDataParams); + + expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(3); + + expect(casesClientMock.attachments.bulkCreate).nthCalledWith(1, { + caseId: 'mock-id-1', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + + expect(casesClientMock.attachments.bulkCreate).nthCalledWith(2, { + caseId: 'mock-id-2', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + + expect(casesClientMock.attachments.bulkCreate).nthCalledWith(3, { + caseId: 'mock-id-3', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + }); + + it('oracle record is missing but some cases exists', async () => { + mockGetRecordId.mockReturnValue(oracleRecords[0].id); + mockBulkGetRecords + .mockResolvedValueOnce([ + { + id: oracleRecords[0].id, + type: CASE_ORACLE_SAVED_OBJECT, + message: 'Not found', + statusCode: 404, + error: 'Not found', + }, + ]) + .mockResolvedValueOnce([oracleRecords[0]]) + .mockResolvedValueOnce([{ ...oracleRecords[0], counter: 2 }]); + + mockBulkCreateRecords.mockResolvedValue([oracleRecords[0]]); + + mockGetCaseId + .mockReturnValueOnce('mock-id-1') + .mockReturnValueOnce('mock-id-2') + .mockReturnValueOnce('mock-id-3'); + + casesClientMock.cases.bulkGet + .mockResolvedValueOnce({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: cases[0].id, + }, + ], + }) + .mockResolvedValueOnce({ + cases: [cases[1]], + errors: [], + }) + .mockResolvedValueOnce({ + cases: [], + errors: [ + { + error: 'Not found', + message: 'Not found', + status: 404, + caseId: cases[2].id, + }, + ], + }); + + casesClientMock.cases.bulkCreate + .mockResolvedValueOnce({ cases: [cases[0]] }) + .mockResolvedValueOnce({ cases: [cases[2]] }); + + await connectorExecutor.execute(missingDataParams); + await connectorExecutor.execute(missingDataParams); + await connectorExecutor.execute(missingDataParams); + + expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(3); + + expect(casesClientMock.attachments.bulkCreate).nthCalledWith(1, { + caseId: 'mock-id-1', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + + expect(casesClientMock.attachments.bulkCreate).nthCalledWith(2, { + caseId: 'mock-id-2', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + + expect(casesClientMock.attachments.bulkCreate).nthCalledWith(3, { + caseId: 'mock-id-3', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + }); + }); }); From 5e3820bc1b9b3b0e8e82aec9ac1f7587308975ae Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Thu, 25 Jan 2024 17:27:50 +0200 Subject: [PATCH 09/12] Use getLogMetadata util function --- .../cases/cases_connector_executor.test.ts | 16 +- .../cases/cases_connector_executor.ts | 163 ++++++++++-------- 2 files changed, 101 insertions(+), 78 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index 62ecf7cc9755e..d5ed8fc7813c9 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -942,7 +942,7 @@ describe('CasesConnectorExecutor', () => { expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(0); expect(mockLogger.warn).toHaveBeenCalledWith( 'Cases with ids "mock-id-1" contain more than 1000 alerts. The new alerts will not be attached to the cases. Total new alerts: 1', - { tags: ['rule:rule-test-id'] } + { tags: ['cases-connector', 'rule:rule-test-id'], labels: {} } ); }); @@ -962,7 +962,7 @@ describe('CasesConnectorExecutor', () => { expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(0); expect(mockLogger.warn).toHaveBeenCalledWith( 'Cases with ids "mock-id-1" contain more than 1000 alerts. The new alerts will not be attached to the cases. Total new alerts: 1', - { tags: ['rule:rule-test-id'] } + { tags: ['cases-connector', 'rule:rule-test-id'], labels: {} } ); }); @@ -2032,7 +2032,8 @@ describe('CasesConnectorExecutor', () => { }); expect(mockLogger.warn).toHaveBeenCalledWith( - '[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Parsing time window error. Parsing value: "invalid"' + '[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Parsing time window error. Parsing value: "invalid"', + { labels: {}, tags: ['cases-connector', 'rule:rule-test-id'] } ); }); @@ -2042,7 +2043,8 @@ describe('CasesConnectorExecutor', () => { await connectorExecutor.execute(params); expect(mockLogger.warn).toHaveBeenCalledWith( - '[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Timestamp "invalid" is not a valid date' + '[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Timestamp "invalid" is not a valid date', + { labels: {}, tags: ['cases-connector', 'rule:rule-test-id'] } ); }); }); @@ -2129,7 +2131,8 @@ describe('CasesConnectorExecutor', () => { }); expect(mockLogger.warn).toHaveBeenCalledWith( - `[CasesConnector][CasesConnectorExecutor][applyCircuitBreakers] Circuit breaker: Grouping definition would create more than the maximum number of allowed cases 1. Falling back to one case.` + `[CasesConnector][CasesConnectorExecutor][applyCircuitBreakers] Circuit breaker: Grouping definition would create more than the maximum number of allowed cases 1. Falling back to one case.`, + { labels: {}, tags: ['cases-connector', 'rule:rule-test-id'] } ); }); }); @@ -2210,7 +2213,8 @@ describe('CasesConnectorExecutor', () => { }); expect(mockLogger.warn).toHaveBeenCalledWith( - `[CasesConnector][CasesConnectorExecutor][applyCircuitBreakers] Circuit breaker: Grouping definition would create more than the maximum number of allowed cases 10. Falling back to one case.` + `[CasesConnector][CasesConnectorExecutor][applyCircuitBreakers] Circuit breaker: Grouping definition would create more than the maximum number of allowed cases 10. Falling back to one case.`, + { labels: {}, tags: ['cases-connector', 'rule:rule-test-id'] } ); }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts index 56bf1b2484d70..908fb92314f87 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.ts @@ -75,7 +75,7 @@ export class CasesConnectorExecutor { public async execute(params: CasesConnectorRunParams) { const { alerts, groupingBy } = params; - const groupedAlerts = this.groupAlerts({ alerts, groupingBy }); + const groupedAlerts = this.groupAlerts({ params, alerts, groupingBy }); const groupedAlertsWithCircuitBreakers = this.applyCircuitBreakers(params, groupedAlerts); /** @@ -144,12 +144,15 @@ export class CasesConnectorExecutor { } private groupAlerts({ + params, alerts, groupingBy, - }: Pick): GroupedAlerts[] { + }: Pick & { + params: CasesConnectorRunParams; + }): GroupedAlerts[] { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][groupAlerts] Grouping ${alerts.length} alerts`, - { labels: { groupingBy }, tags: ['case-connector:groupAlerts'] } + this.getLogMetadata(params, { labels: { groupingBy }, tags: ['case-connector:groupAlerts'] }) ); const uniqueGroupingByFields = Array.from(new Set(groupingBy)); @@ -166,7 +169,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][groupAlerts] Total alerts to be grouped: ${filteredAlerts.length} out of ${alerts.length}`, - { tags: ['case-connector:groupAlerts'] } + this.getLogMetadata(params, { tags: ['case-connector:groupAlerts'] }) ); for (const alert of filteredAlerts) { @@ -175,7 +178,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][groupAlerts] Alert ${alert._id} got grouped into bucket with ID ${groupingKey}`, - { tags: ['case-connector:groupAlerts', groupingKey] } + this.getLogMetadata(params, { tags: ['case-connector:groupAlerts', groupingKey] }) ); if (groupingMap.has(groupingKey)) { @@ -196,7 +199,8 @@ export class CasesConnectorExecutor { const maxCasesCircuitBreaker = Math.min(params.maximumCasesToOpen, MAX_OPEN_CASES); this.logger.warn( - `[CasesConnector][CasesConnectorExecutor][applyCircuitBreakers] Circuit breaker: Grouping definition would create more than the maximum number of allowed cases ${maxCasesCircuitBreaker}. Falling back to one case.` + `[CasesConnector][CasesConnectorExecutor][applyCircuitBreakers] Circuit breaker: Grouping definition would create more than the maximum number of allowed cases ${maxCasesCircuitBreaker}. Falling back to one case.`, + this.getLogMetadata(params) ); return this.removeGrouping(groupedAlerts); @@ -217,7 +221,7 @@ export class CasesConnectorExecutor { ): Map { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][generateOracleKeys] Generating ${groupedAlerts.length} oracle keys`, - { tags: ['case-connector:generateOracleKeys'] } + this.getLogMetadata(params, { tags: ['case-connector:generateOracleKeys'] }) ); const { rule, owner } = params; @@ -236,10 +240,10 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][generateOracleKeys] Oracle key ${oracleKey} generated`, - { + this.getLogMetadata(params, { labels: { params: getRecordIdParams }, tags: ['case-connector:generateOracleKeys', oracleKey], - } + }) ); oracleMap.set(oracleKey, { oracleKey, grouping, alerts }); @@ -247,7 +251,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][generateOracleKeys] Total of oracles keys generated ${oracleMap.size}`, - { tags: ['case-connector:generateOracleKeys'] } + this.getLogMetadata(params, { tags: ['case-connector:generateOracleKeys'] }) ); return oracleMap; @@ -259,7 +263,7 @@ export class CasesConnectorExecutor { ): Promise> { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] Upserting ${groupedAlertsWithOracleKey.size} oracle records`, - { tags: ['case-connector:upsertOracleRecords'] } + this.getLogMetadata(params, { tags: ['case-connector:upsertOracleRecords'] }) ); const bulkCreateReq: BulkCreateOracleRecordRequest = []; @@ -278,7 +282,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] Getting oracle records with ids ${ids}`, - { tags: ['case-connector:upsertOracleRecords', ...ids] } + this.getLogMetadata(params, { tags: ['case-connector:upsertOracleRecords', ...ids] }) ); const bulkGetRes = await this.casesOracleService.bulkGetRecords(ids); @@ -286,14 +290,14 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] The total number of valid oracle records is ${bulkGetValidRecords.length} and the total number of errors while getting the records is ${bulkGetRecordsErrors.length}`, - { + this.getLogMetadata(params, { labels: { total: ids.length, success: bulkGetValidRecords.length, errors: bulkGetRecordsErrors.length, }, tags: ['case-connector:upsertOracleRecords'], - } + }) ); addRecordToMap(bulkGetValidRecords); @@ -306,13 +310,13 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] The total number of non found oracle records is ${nonFoundErrors.length} and the total number of the rest of errors while getting the records is ${restOfErrors.length}`, - { + this.getLogMetadata(params, { labels: { nonFoundErrors: nonFoundErrors.length, restOfErrors: restOfErrors.length, }, tags: ['case-connector:upsertOracleRecords'], - } + }) ); this.handleAndThrowErrors(restOfErrors); @@ -339,7 +343,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] Creating oracle records with ids ${idsToCreate}`, - { tags: ['case-connector:upsertOracleRecords', ...idsToCreate] } + this.getLogMetadata(params, { tags: ['case-connector:upsertOracleRecords', ...idsToCreate] }) ); const bulkCreateRes = await this.casesOracleService.bulkCreateRecord(bulkCreateReq); @@ -347,14 +351,14 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] The total number of created oracle records is ${bulkCreateValidRecords.length} and the total number of errors while creating the records is ${bulkCreateErrors.length}`, - { + this.getLogMetadata(params, { labels: { total: idsToCreate.length, success: bulkCreateValidRecords.length, errors: bulkCreateErrors.length, }, tags: ['case-connector:upsertOracleRecords'], - } + }) ); this.handleAndThrowErrors(bulkCreateErrors); @@ -372,32 +376,36 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][handleTimeWindow] Handling time window ${timeWindow}`, - { tags: ['case-connector:handleTimeWindow'] } + this.getLogMetadata(params, { tags: ['case-connector:handleTimeWindow'] }) ); const oracleRecordMapWithIncreasedCounters = new Map(oracleRecordMap); const recordsToIncreaseCounter = Array.from(oracleRecordMap.values()) .filter(({ oracleRecord }) => - this.isTimeWindowPassed(timeWindow, oracleRecord.updatedAt ?? oracleRecord.createdAt) + this.isTimeWindowPassed( + params, + timeWindow, + oracleRecord.updatedAt ?? oracleRecord.createdAt + ) ) .map(({ oracleRecord }) => oracleRecord); this.logger.debug( `[CasesConnector][CasesConnectorExecutor][handleTimeWindow] Total oracle records where the time window has passed and their counter will be increased ${recordsToIncreaseCounter.length}`, - { tags: ['case-connector:handleTimeWindow', ...recordsToIncreaseCounter.map(({ id }) => id)] } + this.getLogMetadata(params, { + tags: ['case-connector:handleTimeWindow', ...recordsToIncreaseCounter.map(({ id }) => id)], + }) ); - const bulkUpdateValidRecords = await this.increaseOracleRecordCounter(recordsToIncreaseCounter); + const bulkUpdateValidRecords = await this.increaseOracleRecordCounter( + params, + recordsToIncreaseCounter + ); this.logger.debug( `[CasesConnector][CasesConnectorExecutor][handleTimeWindow] Total oracle records where their counter got increased ${bulkUpdateValidRecords.length}`, - { - labels: { - total: recordsToIncreaseCounter.length, - }, - tags: ['case-connector:handleTimeWindow'], - } + this.getLogMetadata(params, { tags: ['case-connector:handleTimeWindow'] }) ); for (const res of bulkUpdateValidRecords) { @@ -411,11 +419,12 @@ export class CasesConnectorExecutor { } private async increaseOracleRecordCounter( + params: CasesConnectorRunParams, oracleRecords: OracleRecord[] ): Promise { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][increaseOracleRecordCounter] Increasing the counters of ${oracleRecords.length} oracle records`, - { tags: ['case-connector:increaseOracleRecordCounter'] } + this.getLogMetadata(params, { tags: ['case-connector:increaseOracleRecordCounter'] }) ); if (oracleRecords.length === 0) { @@ -438,14 +447,14 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertOracleRecords] The total number of updated oracle records is ${bulkUpdateValidRecords.length} and the total number of errors while updating is ${bulkUpdateErrors.length}`, - { + this.getLogMetadata(params, { labels: { total: idsToUpdate.length, success: bulkUpdateValidRecords.length, errors: bulkUpdateErrors.length, }, tags: ['case-connector:increaseOracleRecordCounter', ...idsToUpdate], - } + }) ); this.handleAndThrowErrors(bulkUpdateErrors); @@ -453,10 +462,14 @@ export class CasesConnectorExecutor { return bulkUpdateValidRecords; } - private isTimeWindowPassed(timeWindow: string, counterLastUpdatedAt: string) { + private isTimeWindowPassed( + params: CasesConnectorRunParams, + timeWindow: string, + counterLastUpdatedAt: string + ) { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Validating the time window ${timeWindow} against the timestamp of the last update of the oracle record ${counterLastUpdatedAt}`, - { tags: ['case-connector:isTimeWindowPassed'] } + this.getLogMetadata(params, { tags: ['case-connector:isTimeWindowPassed'] }) ); const parsedDate = dateMath.parse(`now-${timeWindow}`); @@ -466,7 +479,8 @@ export class CasesConnectorExecutor { */ if (!parsedDate || !parsedDate.isValid()) { this.logger.warn( - `[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Parsing time window error. Parsing value: "${timeWindow}"` + `[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Parsing time window error. Parsing value: "${timeWindow}"`, + this.getLogMetadata(params) ); return false; @@ -479,7 +493,8 @@ export class CasesConnectorExecutor { */ if (isNaN(counterLastUpdatedAtAsDate.getTime())) { this.logger.warn( - `[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Timestamp "${counterLastUpdatedAt}" is not a valid date` + `[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Timestamp "${counterLastUpdatedAt}" is not a valid date`, + this.getLogMetadata(params) ); return false; @@ -489,7 +504,7 @@ export class CasesConnectorExecutor { `[CasesConnector][CasesConnectorExecutor][isTimeWindowPassed] Time window has passed ${ counterLastUpdatedAtAsDate < parsedDate.toDate() }`, - { tags: ['case-connector:isTimeWindowPassed'] } + this.getLogMetadata(params, { tags: ['case-connector:isTimeWindowPassed'] }) ); return counterLastUpdatedAtAsDate < parsedDate.toDate(); @@ -501,7 +516,7 @@ export class CasesConnectorExecutor { ): Map { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][generateCaseIds] Generating ${groupedAlertsWithOracleRecords.size} case IDs`, - { tags: ['case-connector:generateCaseIds'] } + this.getLogMetadata(params, { tags: ['case-connector:generateCaseIds'] }) ); const { rule, owner } = params; @@ -523,10 +538,10 @@ export class CasesConnectorExecutor { `[CasesConnector][CasesConnectorExecutor][generateCaseIds] Case ID ${caseId} generated with params ${JSON.stringify( getCaseIdParams )}`, - { + this.getLogMetadata(params, { labels: { params: getCaseIdParams }, tags: ['case-connector:generateCaseIds', caseId], - } + }) ); casesMap.set(caseId, { @@ -540,7 +555,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][generateCaseIds] Total of case IDs generated ${casesMap.size}`, - { tags: ['case-connector:generateCaseIds'] } + this.getLogMetadata(params, { tags: ['case-connector:generateCaseIds'] }) ); return casesMap; @@ -552,7 +567,7 @@ export class CasesConnectorExecutor { ): Promise> { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertCases] Upserting ${groupedAlertsWithCaseId.size} cases`, - { tags: ['case-connector:upsertCases'] } + this.getLogMetadata(params, { tags: ['case-connector:upsertCases'] }) ); const bulkCreateReq = []; @@ -562,21 +577,21 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertCases] Getting cases with ids ${ids}`, - { tags: ['case-connector:upsertCases', ...ids] } + this.getLogMetadata(params, { tags: ['case-connector:upsertCases', ...ids] }) ); const { cases, errors } = await this.casesClient.cases.bulkGet({ ids }); this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertCases] The total number of cases is ${cases.length} and the total number of errors while getting the cases is ${errors.length}`, - { + this.getLogMetadata(params, { labels: { total: ids.length, success: cases.length, errors: errors.length, }, tags: ['case-connector:upsertCases'], - } + }) ); for (const theCase of cases) { @@ -600,13 +615,13 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertCases] The total number of non found cases is ${nonFoundErrors.length} and the total number of the rest of errors while getting the cases is ${restOfErrors.length}`, - { + this.getLogMetadata(params, { labels: { nonFoundErrors: nonFoundErrors.length, restOfErrors: restOfErrors.length, }, tags: ['case-connector:upsertCases'], - } + }) ); this.handleAndThrowErrors(restOfErrors); @@ -627,7 +642,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertCases] Creating cases with ids ${idsToCreate}`, - { tags: ['case-connector:upsertCases', ...idsToCreate] } + this.getLogMetadata(params, { tags: ['case-connector:upsertCases', ...idsToCreate] }) ); /** @@ -639,12 +654,12 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][upsertCases] The total number of created cases is ${bulkCreateCasesResponse.cases.length}`, - { + this.getLogMetadata(params, { labels: { total: bulkCreateReq.length, }, tags: ['case-connector:upsertCases'], - } + }) ); for (const theCase of bulkCreateCasesResponse.cases) { @@ -722,7 +737,7 @@ export class CasesConnectorExecutor { ) { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][handleClosedCases] Handling closed cases with reopenClosedCases set to ${params.reopenClosedCases}`, - { tags: ['case-connector:handleClosedCases'] } + this.getLogMetadata(params, { tags: ['case-connector:handleClosedCases'] }) ); const entriesWithClosedCases = Array.from(casesMap.values()).filter( @@ -731,7 +746,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][handleClosedCases] Closed cases ${entriesWithClosedCases.length}`, - { tags: ['case-connector:handleClosedCases'] } + this.getLogMetadata(params, { tags: ['case-connector:handleClosedCases'] }) ); if (entriesWithClosedCases.length === 0) { @@ -739,7 +754,7 @@ export class CasesConnectorExecutor { } const res = params.reopenClosedCases - ? await this.reopenClosedCases(entriesWithClosedCases, casesMap) + ? await this.reopenClosedCases(params, entriesWithClosedCases, casesMap) : await this.createNewCasesOutOfClosedCases(params, entriesWithClosedCases, casesMap); /** @@ -750,12 +765,13 @@ export class CasesConnectorExecutor { } private async reopenClosedCases( + params: CasesConnectorRunParams, closedCasesEntries: GroupedAlertsWithCases[], casesMap: Map ): Promise> { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][reopenClosedCases] Total closed cases to reopen ${closedCasesEntries.length}`, - { tags: ['case-connector:reopenClosedCases'] } + this.getLogMetadata(params, { tags: ['case-connector:reopenClosedCases'] }) ); const casesMapWithClosedCasesOpened = new Map(casesMap); @@ -770,7 +786,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][reopenClosedCases] Reopening total ${bulkUpdateReq.length} closed cases with ids ${idsToReopen}`, - { tags: ['case-connector:reopenClosedCases', ...idsToReopen] } + this.getLogMetadata(params, { tags: ['case-connector:reopenClosedCases', ...idsToReopen] }) ); /** @@ -789,12 +805,12 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][reopenClosedCases] The total number of cases that got reopened is ${bulkUpdateCasesResponse.length}`, - { + this.getLogMetadata(params, { labels: { total: bulkUpdateCasesResponse.length, }, tags: ['case-connector:reopenClosedCases'], - } + }) ); return casesMapWithClosedCasesOpened; @@ -807,7 +823,7 @@ export class CasesConnectorExecutor { ): Promise> { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][createNewCasesOutOfClosedCases] Creating new cases for closed cases ${closedCasesEntries.length}`, - { tags: ['case-connector:createNewCasesOutOfClosedCases'] } + this.getLogMetadata(params, { tags: ['case-connector:createNewCasesOutOfClosedCases'] }) ); const casesMapWithNewCases = new Map(casesMap); @@ -819,21 +835,22 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][createNewCasesOutOfClosedCases] Total oracle records where their corresponding case is closed and their counter will be increased ${closedCasesEntries.length}`, - { tags: ['case-connector:createNewCasesOutOfClosedCases'] } + this.getLogMetadata(params, { tags: ['case-connector:createNewCasesOutOfClosedCases'] }) ); const bulkUpdateOracleValidRecords = await this.increaseOracleRecordCounter( + params, closedCasesEntries.map((entry) => entry.oracleRecord) ); this.logger.debug( `[CasesConnector][CasesConnectorExecutor][createNewCasesOutOfClosedCases] Total oracle records where their corresponding case is closed and their counter got increased ${bulkUpdateOracleValidRecords.length}`, - { + this.getLogMetadata(params, { tags: [ 'case-connector:createNewCasesOutOfClosedCases', ...closedCasesEntries.map(({ oracleKey }) => oracleKey), ], - } + }) ); const groupedAlertsWithOracleRecords = new Map(); @@ -853,7 +870,7 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][createNewCasesOutOfClosedCases] Generating ${groupedAlertsWithOracleRecords.size} case IDs`, - { tags: ['case-connector:createNewCasesOutOfClosedCases'] } + this.getLogMetadata(params, { tags: ['case-connector:createNewCasesOutOfClosedCases'] }) ); const groupedAlertsWithCaseId = this.generateCaseIds(params, groupedAlertsWithOracleRecords); @@ -865,7 +882,9 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][createNewCasesOutOfClosedCases] Creating cases with ids ${idsToCreate}`, - { tags: ['case-connector:createNewCasesOutOfClosedCases', ...idsToCreate] } + this.getLogMetadata(params, { + tags: ['case-connector:createNewCasesOutOfClosedCases', ...idsToCreate], + }) ); /** @@ -877,12 +896,12 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][createNewCasesOutOfClosedCases] The total number of created cases is ${bulkCreateCasesResponse.cases.length}`, - { + this.getLogMetadata(params, { labels: { total: bulkCreateCasesResponse.cases.length, }, tags: ['case-connector:createNewCasesOutOfClosedCases'], - } + }) ); for (const theCase of bulkCreateCasesResponse.cases) { @@ -901,7 +920,7 @@ export class CasesConnectorExecutor { ): Promise { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][attachAlertsToCases] Attaching alerts to ${groupedAlertsWithCases.size} cases`, - { tags: ['case-connector:attachAlertsToCases'] } + this.getLogMetadata(params, { tags: ['case-connector:attachAlertsToCases'] }) ); const { rule } = params; @@ -925,12 +944,12 @@ export class CasesConnectorExecutor { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][attachAlertsToCases] Attaching alerts to ${casesUnderAlertLimit.length} cases that do not have reach the alert limit per case`, - { + this.getLogMetadata(params, { tags: [ 'case-connector:attachAlertsToCases', ...casesUnderAlertLimit.map(({ caseId }) => caseId), ], - } + }) ); const bulkCreateAlertsRequest: BulkCreateAlertsReq[] = casesUnderAlertLimit.map( @@ -954,14 +973,14 @@ export class CasesConnectorExecutor { async (req: BulkCreateAlertsReq) => { this.logger.debug( `[CasesConnector][CasesConnectorExecutor][attachAlertsToCases] Attaching ${req.attachments.length} alerts to case with ID ${req.caseId}`, - { + this.getLogMetadata(params, { labels: { caseId: req.caseId }, tags: [ 'case-connector:attachAlertsToCases', req.caseId, ...(req.attachments as Array<{ alertId: string }>).map(({ alertId }) => alertId), ], - } + }) ); await this.casesClient.attachments.bulkCreate(req); @@ -993,8 +1012,8 @@ export class CasesConnectorExecutor { private getLogMetadata( params: CasesConnectorRunParams, - { extraTags = [] }: { extraTags?: string[] } = {} + { tags = [], labels = {} }: { tags?: string[]; labels?: Record } = {} ) { - return { tags: [...extraTags, `rule:${params.rule.id}`] }; + return { tags: ['cases-connector', `rule:${params.rule.id}`, ...tags], labels }; } } From 16a79b4f03dce5f0a61708bcdaaaa26f429066e0 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 29 Jan 2024 11:24:40 +0200 Subject: [PATCH 10/12] Add tests for special characters --- .../cases/cases_oracle_service.test.ts | 16 ++++++++++++++++ .../connectors/cases/cases_service.test.ts | 17 +++++++++++++++++ .../connectors/cases/crypto_service.test.ts | 10 ++++++++++ 3 files changed, 43 insertions(+) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts index b80c9b0d22138..72463fb863b74 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts @@ -143,6 +143,22 @@ describe('CasesOracleService', () => { expect(service.getRecordId({ ...params, grouping })).toEqual(hex); } ); + + it('constructs a record ID with special characters correctly', async () => { + const ruleId = `{}=:&".'/{}}`; + const spaceId = 'default:'; + const owner = 'cases{'; + const grouping = { '{:}': `{}=:&".'/{}}` }; + + const payload = `${ruleId}:${spaceId}:${owner}:${stringify(grouping)}`; + const hash = createHash('sha256'); + + hash.update(payload); + + const hex = hash.digest('hex'); + + expect(service.getRecordId({ ruleId, spaceId, owner, grouping })).toEqual(hex); + }); }); describe('getRecord', () => { diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_service.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_service.test.ts index 5ea9b51bad3ab..848d3fa276236 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_service.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_service.test.ts @@ -144,5 +144,22 @@ describe('CasesService', () => { expect(service.getCaseId({ ...params, grouping, counter })).toEqual(hex); } ); + + it('constructs a record ID with special characters correctly', async () => { + const ruleId = `{}=:&".'/{}}`; + const spaceId = 'default'; + const owner = 'cases'; + const grouping = { '{:}': `{}=:&".'/{}}` }; + const counter = 1; + + const payload = `${ruleId}:${spaceId}:${owner}:${stringify(grouping)}:${counter}`; + const hash = createHash('sha256'); + + hash.update(payload); + + const hex = hash.digest('hex'); + + expect(service.getCaseId({ ruleId, spaceId, owner, grouping, counter })).toEqual(hex); + }); }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/crypto_service.test.ts b/x-pack/plugins/cases/server/connectors/cases/crypto_service.test.ts index bf8a9f946ab58..6ea5b32542ea5 100644 --- a/x-pack/plugins/cases/server/connectors/cases/crypto_service.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/crypto_service.test.ts @@ -47,5 +47,15 @@ describe('CryptoService', () => { service.stringifyDeterministically({ 'host.ip': '0.0.0.1', 'agent.id': '8a4f500d' }) ).toEqual('{"agent.id":"8a4f500d","host.ip":"0.0.0.1"}'); }); + + it('returns null if the object is not defined', async () => { + expect(service.stringifyDeterministically()).toEqual(null); + }); + + it('handles special characters correctly', async () => { + expect(service.stringifyDeterministically({ [`{}=:&".'/{}}`]: `{}=:&".'{}}` })).toEqual( + `{\"{}=:&\\\".'/{}}\":\"{}=:&\\\".'{}}\"}` + ); + }); }); }); From b52f16bb87e14a7095ae7034328ec353f9abd40f Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 29 Jan 2024 11:40:54 +0200 Subject: [PATCH 11/12] Do not call the SO client if the data is an empty array --- .../cases/cases_oracle_service.test.ts | 91 +++++++++++++++++++ .../connectors/cases/cases_oracle_service.ts | 12 +++ 2 files changed, 103 insertions(+) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts index 72463fb863b74..cf9930410208d 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.test.ts @@ -251,6 +251,12 @@ describe('CasesOracleService', () => { { id: 'so-id-2', type: 'cases-oracle' }, ]); }); + + it('does not call the savedObjectsClient if the input is an empty array', async () => { + await service.bulkGetRecords([]); + + expect(savedObjectsClient.bulkGet).not.toHaveBeenCalledWith(); + }); }); describe('createRecord', () => { @@ -384,6 +390,12 @@ describe('CasesOracleService', () => { }, ]); }); + + it('does not call the savedObjectsClient if the input is an empty array', async () => { + await service.bulkCreateRecord([]); + + expect(savedObjectsClient.bulkCreate).not.toHaveBeenCalledWith(); + }); }); describe('increaseCounter', () => { @@ -440,4 +452,83 @@ describe('CasesOracleService', () => { ); }); }); + + describe('bulkUpdateRecord', () => { + const bulkUpdateSOs = [ + { + id: 'so-id', + version: 'so-version', + attributes: { + counter: 1, + cases: [], + rules: [], + grouping: {}, + createdAt: '2023-10-10T10:23:42.769Z', + updatedAt: '2023-10-10T10:23:42.769Z', + }, + type: CASE_ORACLE_SAVED_OBJECT, + references: [], + }, + { + id: 'so-id-2', + type: CASE_ORACLE_SAVED_OBJECT, + error: { + message: 'Conflict', + statusCode: 409, + error: 'Conflict', + }, + }, + ]; + + beforeEach(() => { + // @ts-expect-error: types of the SO client are wrong and they do not accept errors + savedObjectsClient.bulkUpdate.mockResolvedValue({ saved_objects: bulkUpdateSOs }); + }); + + it('formats the response correctly', async () => { + const res = await service.bulkUpdateRecord([ + { recordId: 'so-id', version: 'so-version-1', payload: { counter: 2 } }, + { recordId: 'so-id-2', version: 'so-version-22', payload: { counter: 3 } }, + ]); + + expect(res).toEqual([ + { ...bulkUpdateSOs[0].attributes, id: 'so-id', version: 'so-version' }, + { ...bulkUpdateSOs[1].error, id: 'so-id-2' }, + ]); + }); + + it('calls the bulkUpdateRecord correctly', async () => { + await service.bulkUpdateRecord([ + { recordId: 'so-id', version: 'so-version-1', payload: { counter: 2 } }, + { recordId: 'so-id-2', version: 'so-version-2', payload: { counter: 3 } }, + ]); + + expect(savedObjectsClient.bulkUpdate).toHaveBeenCalledWith([ + { + attributes: { + counter: 2, + updatedAt: expect.anything(), + }, + id: 'so-id', + version: 'so-version-1', + type: 'cases-oracle', + }, + { + attributes: { + counter: 3, + updatedAt: expect.anything(), + }, + id: 'so-id-2', + version: 'so-version-2', + type: 'cases-oracle', + }, + ]); + }); + + it('does not call the savedObjectsClient if the input is an empty array', async () => { + await service.bulkUpdateRecord([]); + + expect(savedObjectsClient.bulkUpdate).not.toHaveBeenCalledWith(); + }); + }); }); diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts index 1bd5b209c52c4..42f45beb2f111 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_oracle_service.ts @@ -73,6 +73,10 @@ export class CasesOracleService { tags: ['cases-oracle-service', 'bulkGetRecords', ...ids], }); + if (ids.length === 0) { + return []; + } + const oracleRecords = (await this.savedObjectsClient.bulkGet( ids.map((id) => ({ id, type: CASE_ORACLE_SAVED_OBJECT })) )) as SavedObjectsBulkResponseWithErrors; @@ -106,6 +110,10 @@ export class CasesOracleService { tags: ['cases-oracle-service', 'bulkCreateRecord', ...recordIds], }); + if (records.length === 0) { + return []; + } + const req = records.map((record) => ({ id: record.recordId, type: CASE_ORACLE_SAVED_OBJECT, @@ -153,6 +161,10 @@ export class CasesOracleService { tags: ['cases-oracle-service', 'bulkUpdateRecord', ...recordIds], }); + if (records.length === 0) { + return []; + } + const req = records.map((record) => ({ id: record.recordId, type: CASE_ORACLE_SAVED_OBJECT, From d43295514a395727f32e00543325a2621ae0a4b6 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 29 Jan 2024 12:17:22 +0200 Subject: [PATCH 12/12] Add tests for 404 when updating oracle records --- .../cases/cases_connector_executor.test.ts | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts index d5ed8fc7813c9..7a28e60338c6a 100644 --- a/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts +++ b/x-pack/plugins/cases/server/connectors/cases/cases_connector_executor.test.ts @@ -2413,5 +2413,76 @@ describe('CasesConnectorExecutor', () => { ], }); }); + + it('increase oracle counter but is missing', async () => { + const nonFoundRecord = { + id: oracleRecords[0].id, + type: CASE_ORACLE_SAVED_OBJECT, + message: 'Not found', + statusCode: 404, + error: 'Not found', + }; + + dateMathMock.parse + // time window has passed. should increase the counter + .mockImplementationOnce(() => moment('2023-11-10T10:23:42.769Z')) + // time window has not passed. counter should not be increased + .mockImplementationOnce(() => moment('2023-10-09T10:23:42.769Z')); + + mockGetRecordId.mockReturnValue(oracleRecords[0].id); + mockBulkGetRecords + .mockResolvedValueOnce([oracleRecords[0]]) + .mockResolvedValueOnce([nonFoundRecord]); + + mockBulkCreateRecords.mockResolvedValueOnce(oracleRecords[0]); + mockBulkUpdateRecord.mockResolvedValueOnce(nonFoundRecord); + + mockGetCaseId.mockReturnValueOnce('mock-id-1'); + + casesClientMock.cases.bulkGet.mockResolvedValue({ + cases: [cases[0]], + errors: [], + }); + + await connectorExecutor.execute(missingDataParams); + await connectorExecutor.execute(missingDataParams); + + expect(mockBulkUpdateRecord).toBeCalledTimes(1); + expect(mockBulkUpdateRecord).toHaveBeenCalledWith([ + { payload: { counter: 2 }, recordId: 'so-oracle-record-0', version: 'so-version-0' }, + ]); + + expect(mockBulkCreateRecords).toBeCalledTimes(1); + expect(mockBulkCreateRecords).toHaveBeenCalledWith([ + { + payload: { + cases: [], + grouping: { + foo: 'bar', + }, + rules: [ + { + id: 'rule-test-id', + }, + ], + }, + recordId: 'so-oracle-record-0', + }, + ]); + + expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledTimes(1); + expect(casesClientMock.attachments.bulkCreate).toHaveBeenCalledWith({ + caseId: 'mock-id-1', + attachments: [ + { + type: 'alert', + alertId: 'test-id', + index: 'test-index', + rule: { id: 'rule-test-id', name: 'Test rule' }, + owner: 'securitySolution', + }, + ], + }); + }); }); });