[Security Solution][Case][Bug] Only update alert status in its specific index (#92530)

* Writing failing test for duplicate ids

* Test is correctly failing prior to bug fix

* Working jest tests

* Adding more jest tests

* Fixing jest tests

* Adding await and gzip

* Fixing type errors

* Updating log message

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Jonathan Buttner 2021-03-03 13:28:59 -05:00 committed by GitHub
parent 7cf2284580
commit 9dd395b452
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
27 changed files with 7281 additions and 311 deletions

View file

@ -6,34 +6,33 @@
*/
import { ElasticsearchClient, Logger } from 'kibana/server';
import { AlertInfo } from '../../common';
import { AlertServiceContract } from '../../services';
import { CaseClientGetAlertsResponse } from './types';
interface GetParams {
alertsService: AlertServiceContract;
ids: string[];
indices: Set<string>;
alertsInfo: AlertInfo[];
scopedClusterClient: ElasticsearchClient;
logger: Logger;
}
export const get = async ({
alertsService,
ids,
indices,
alertsInfo,
scopedClusterClient,
logger,
}: GetParams): Promise<CaseClientGetAlertsResponse> => {
if (ids.length === 0 || indices.size <= 0) {
if (alertsInfo.length === 0) {
return [];
}
const alerts = await alertsService.getAlerts({ ids, indices, scopedClusterClient, logger });
const alerts = await alertsService.getAlerts({ alertsInfo, scopedClusterClient, logger });
if (!alerts) {
return [];
}
return alerts.hits.hits.map((alert) => ({
return alerts.docs.map((alert) => ({
id: alert._id,
index: alert._index,
...alert._source,

View file

@ -15,17 +15,13 @@ describe('updateAlertsStatus', () => {
const caseClient = await createCaseClientWithMockSavedObjectsClient({ savedObjectsClient });
await caseClient.client.updateAlertsStatus({
ids: ['alert-id-1'],
status: CaseStatuses.closed,
indices: new Set<string>(['.siem-signals']),
alerts: [{ id: 'alert-id-1', index: '.siem-signals', status: CaseStatuses.closed }],
});
expect(caseClient.services.alertsService.updateAlertsStatus).toHaveBeenCalledWith({
scopedClusterClient: expect.anything(),
logger: expect.anything(),
ids: ['alert-id-1'],
indices: new Set<string>(['.siem-signals']),
status: CaseStatuses.closed,
scopedClusterClient: expect.anything(),
alerts: [{ id: 'alert-id-1', index: '.siem-signals', status: CaseStatuses.closed }],
});
});
});

View file

@ -6,25 +6,21 @@
*/
import { ElasticsearchClient, Logger } from 'src/core/server';
import { CaseStatuses } from '../../../common/api';
import { AlertServiceContract } from '../../services';
import { UpdateAlertRequest } from '../types';
interface UpdateAlertsStatusArgs {
alertsService: AlertServiceContract;
ids: string[];
status: CaseStatuses;
indices: Set<string>;
alerts: UpdateAlertRequest[];
scopedClusterClient: ElasticsearchClient;
logger: Logger;
}
export const updateAlertsStatus = async ({
alertsService,
ids,
status,
indices,
alerts,
scopedClusterClient,
logger,
}: UpdateAlertsStatusArgs): Promise<void> => {
await alertsService.updateAlertsStatus({ ids, status, indices, scopedClusterClient, logger });
await alertsService.updateAlertsStatus({ alerts, scopedClusterClient, logger });
};

View file

@ -15,7 +15,7 @@ import {
SavedObject,
} from 'kibana/server';
import { ActionResult, ActionsClient } from '../../../../actions/server';
import { flattenCaseSavedObject, getAlertIndicesAndIDs } from '../../routes/api/utils';
import { flattenCaseSavedObject, getAlertInfoFromComments } from '../../routes/api/utils';
import {
ActionConnector,
@ -108,12 +108,11 @@ export const push = async ({
);
}
const { ids, indices } = getAlertIndicesAndIDs(theCase?.comments);
const alertsInfo = getAlertInfoFromComments(theCase?.comments);
try {
alerts = await caseClient.getAlerts({
ids,
indices,
alertsInfo,
});
} catch (e) {
throw createCaseError({

View file

@ -430,9 +430,13 @@ describe('update', () => {
await caseClient.client.update(patchCases);
expect(caseClient.client.updateAlertsStatus).toHaveBeenCalledWith({
ids: ['test-id'],
status: 'closed',
indices: new Set<string>(['test-index']),
alerts: [
{
id: 'test-id',
index: 'test-index',
status: 'closed',
},
],
});
});
@ -458,11 +462,10 @@ describe('update', () => {
});
const caseClient = await createCaseClientWithMockSavedObjectsClient({ savedObjectsClient });
caseClient.client.updateAlertsStatus = jest.fn();
await caseClient.client.update(patchCases);
expect(caseClient.client.updateAlertsStatus).not.toHaveBeenCalled();
expect(caseClient.esClient.bulk).not.toHaveBeenCalled();
});
test('it updates alert status when syncAlerts is turned on', async () => {
@ -492,9 +495,7 @@ describe('update', () => {
await caseClient.client.update(patchCases);
expect(caseClient.client.updateAlertsStatus).toHaveBeenCalledWith({
ids: ['test-id'],
status: 'open',
indices: new Set<string>(['test-index']),
alerts: [{ id: 'test-id', index: 'test-index', status: 'open' }],
});
});
@ -515,11 +516,10 @@ describe('update', () => {
});
const caseClient = await createCaseClientWithMockSavedObjectsClient({ savedObjectsClient });
caseClient.client.updateAlertsStatus = jest.fn();
await caseClient.client.update(patchCases);
expect(caseClient.client.updateAlertsStatus).not.toHaveBeenCalled();
expect(caseClient.esClient.bulk).not.toHaveBeenCalled();
});
test('it updates alert status for multiple cases', async () => {
@ -576,22 +576,12 @@ describe('update', () => {
caseClient.client.updateAlertsStatus = jest.fn();
await caseClient.client.update(patchCases);
/**
* the update code will put each comment into a status bucket and then make at most 1 call
* to ES for each status bucket
* Now instead of doing a call per case to get the comments, it will do a single call with all the cases
* and sub cases and get all the comments in one go
*/
expect(caseClient.client.updateAlertsStatus).toHaveBeenNthCalledWith(1, {
ids: ['test-id'],
status: 'open',
indices: new Set<string>(['test-index']),
});
expect(caseClient.client.updateAlertsStatus).toHaveBeenNthCalledWith(2, {
ids: ['test-id-2'],
status: 'closed',
indices: new Set<string>(['test-index-2']),
expect(caseClient.client.updateAlertsStatus).toHaveBeenCalledWith({
alerts: [
{ id: 'test-id', index: 'test-index', status: 'open' },
{ id: 'test-id-2', index: 'test-index-2', status: 'closed' },
],
});
});
@ -611,11 +601,10 @@ describe('update', () => {
});
const caseClient = await createCaseClientWithMockSavedObjectsClient({ savedObjectsClient });
caseClient.client.updateAlertsStatus = jest.fn();
await caseClient.client.update(patchCases);
expect(caseClient.client.updateAlertsStatus).not.toHaveBeenCalled();
expect(caseClient.esClient.bulk).not.toHaveBeenCalled();
});
});

View file

@ -18,7 +18,6 @@ import {
Logger,
} from 'kibana/server';
import {
AlertInfo,
flattenCaseSavedObject,
isCommentRequestTypeAlertOrGenAlert,
} from '../../routes/api/utils';
@ -53,7 +52,8 @@ import {
SUB_CASE_SAVED_OBJECT,
} from '../../saved_object_types';
import { CaseClientHandler } from '..';
import { addAlertInfoToStatusMap } from '../../common';
import { createAlertUpdateRequest } from '../../common';
import { UpdateAlertRequest } from '../types';
import { createCaseError } from '../../common/error';
/**
@ -291,33 +291,25 @@ async function updateAlerts({
// get a map of sub case id to the sub case status
const subCasesToStatus = await getSubCasesToStatus({ totalAlerts, client, caseService });
// create a map of the case statuses to the alert information that we need to update for that status
// This allows us to make at most 3 calls to ES, one for each status type that we need to update
// One potential improvement here is to do a tick (set timeout) to reduce the memory footprint if that becomes an issue
const alertsToUpdate = totalAlerts.saved_objects.reduce((acc, alertComment) => {
if (isCommentRequestTypeAlertOrGenAlert(alertComment.attributes)) {
const status = getSyncStatusForComment({
alertComment,
casesToSyncToStatus,
subCasesToStatus,
});
// create an array of requests that indicate the id, index, and status to update an alert
const alertsToUpdate = totalAlerts.saved_objects.reduce(
(acc: UpdateAlertRequest[], alertComment) => {
if (isCommentRequestTypeAlertOrGenAlert(alertComment.attributes)) {
const status = getSyncStatusForComment({
alertComment,
casesToSyncToStatus,
subCasesToStatus,
});
addAlertInfoToStatusMap({ comment: alertComment.attributes, statusMap: acc, status });
}
acc.push(...createAlertUpdateRequest({ comment: alertComment.attributes, status }));
}
return acc;
}, new Map<CaseStatuses, AlertInfo>());
return acc;
},
[]
);
// This does at most 3 calls to Elasticsearch to update the status of the alerts to either open, closed, or in-progress
for (const [status, alertInfo] of alertsToUpdate.entries()) {
if (alertInfo.ids.length > 0 && alertInfo.indices.size > 0) {
caseClient.updateAlertsStatus({
ids: alertInfo.ids,
status,
indices: alertInfo.indices,
});
}
}
await caseClient.updateAlertsStatus({ alerts: alertsToUpdate });
}
interface UpdateArgs {

View file

@ -169,9 +169,9 @@ export class CaseClientHandler implements CaseClient {
});
} catch (error) {
throw createCaseError({
message: `Failed to update alerts status using client ids: ${JSON.stringify(
args.ids
)} \nindices: ${JSON.stringify([...args.indices])} \nstatus: ${args.status}: ${error}`,
message: `Failed to update alerts status using client alerts: ${JSON.stringify(
args.alerts
)}: ${error}`,
error,
logger: this.logger,
});
@ -218,9 +218,9 @@ export class CaseClientHandler implements CaseClient {
});
} catch (error) {
throw createCaseError({
message: `Failed to get alerts using client ids: ${JSON.stringify(
args.ids
)} \nindices: ${JSON.stringify([...args.indices])}: ${error}`,
message: `Failed to get alerts using client requested alerts: ${JSON.stringify(
args.alertsInfo
)}: ${error}`,
error,
logger: this.logger,
});

View file

@ -15,6 +15,8 @@ import {
} from '../../routes/api/__fixtures__';
import { createCaseClientWithMockSavedObjectsClient } from '../mocks';
type AlertComment = CommentType.alert | CommentType.generatedAlert;
describe('addComment', () => {
beforeEach(async () => {
jest.restoreAllMocks();
@ -248,9 +250,7 @@ describe('addComment', () => {
});
expect(caseClient.client.updateAlertsStatus).toHaveBeenCalledWith({
ids: ['test-alert'],
status: 'open',
indices: new Set<string>(['test-index']),
alerts: [{ id: 'test-alert', index: 'test-index', status: 'open' }],
});
});
@ -517,5 +517,77 @@ describe('addComment', () => {
expect(boomErr.output.statusCode).toBe(400);
});
});
describe('alert format', () => {
it.each([
['1', ['index1', 'index2'], CommentType.alert],
[['1', '2'], 'index', CommentType.alert],
['1', ['index1', 'index2'], CommentType.generatedAlert],
[['1', '2'], 'index', CommentType.generatedAlert],
])(
'throws an error with an alert comment with contents id: %p indices: %p type: %s',
async (alertId, index, type) => {
expect.assertions(1);
const savedObjectsClient = createMockSavedObjectsRepository({
caseSavedObject: mockCases,
caseCommentSavedObject: mockCaseComments,
});
const caseClient = await createCaseClientWithMockSavedObjectsClient({
savedObjectsClient,
});
await expect(
caseClient.client.addComment({
caseId: 'mock-id-4',
comment: {
// casting because type must be either alert or generatedAlert but type is CommentType
type: type as AlertComment,
alertId,
index,
rule: {
id: 'test-rule1',
name: 'test-rule',
},
},
})
).rejects.toThrow();
}
);
it.each([
['1', ['index1'], CommentType.alert],
[['1', '2'], ['index', 'other-index'], CommentType.alert],
])(
'does not throw an error with an alert comment with contents id: %p indices: %p type: %s',
async (alertId, index, type) => {
expect.assertions(1);
const savedObjectsClient = createMockSavedObjectsRepository({
caseSavedObject: mockCases,
caseCommentSavedObject: mockCaseComments,
});
const caseClient = await createCaseClientWithMockSavedObjectsClient({
savedObjectsClient,
});
await expect(
caseClient.client.addComment({
caseId: 'mock-id-1',
comment: {
// casting because type must be either alert or generatedAlert but type is CommentType
type: type as AlertComment,
alertId,
index,
rule: {
id: 'test-rule1',
name: 'test-rule',
},
},
})
).resolves.not.toBeUndefined();
}
);
});
});
});

View file

@ -11,11 +11,7 @@ import { fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';
import { SavedObject, SavedObjectsClientContract, Logger } from 'src/core/server';
import {
decodeCommentRequest,
getAlertIds,
isCommentRequestTypeGenAlert,
} from '../../routes/api/utils';
import { decodeCommentRequest, isCommentRequestTypeGenAlert } from '../../routes/api/utils';
import {
throwErrors,
@ -36,7 +32,7 @@ import {
} from '../../services/user_actions/helpers';
import { CaseServiceSetup, CaseUserActionServiceSetup } from '../../services';
import { CommentableCase } from '../../common';
import { CommentableCase, createAlertUpdateRequest } from '../../common';
import { CaseClientHandler } from '..';
import { createCaseError } from '../../common/error';
import { CASE_COMMENT_SAVED_OBJECT } from '../../saved_object_types';
@ -177,15 +173,12 @@ const addGeneratedAlerts = async ({
newComment.attributes.type === CommentType.generatedAlert) &&
caseInfo.attributes.settings.syncAlerts
) {
const ids = getAlertIds(query);
await caseClient.updateAlertsStatus({
ids,
const alertsToUpdate = createAlertUpdateRequest({
comment: query,
status: subCase.attributes.status,
indices: new Set([
...(Array.isArray(newComment.attributes.index)
? newComment.attributes.index
: [newComment.attributes.index]),
]),
});
await caseClient.updateAlertsStatus({
alerts: alertsToUpdate,
});
}
@ -331,15 +324,13 @@ export const addComment = async ({
});
if (newComment.attributes.type === CommentType.alert && updatedCase.settings.syncAlerts) {
const ids = getAlertIds(query);
await caseClient.updateAlertsStatus({
ids,
const alertsToUpdate = createAlertUpdateRequest({
comment: query,
status: updatedCase.status,
indices: new Set([
...(Array.isArray(newComment.attributes.index)
? newComment.attributes.index
: [newComment.attributes.index]),
]),
});
await caseClient.updateAlertsStatus({
alerts: alertsToUpdate,
});
}

View file

@ -5,6 +5,8 @@
* 2.0.
*/
import { ElasticsearchClient } from 'kibana/server';
import { DeeplyMockedKeys } from 'packages/kbn-utility-types/target/jest';
import { loggingSystemMock, elasticsearchServiceMock } from '../../../../../src/core/server/mocks';
import {
AlertServiceContract,
@ -45,6 +47,7 @@ export const createCaseClientWithMockSavedObjectsClient = async ({
userActionService: jest.Mocked<CaseUserActionServiceSetup>;
alertsService: jest.Mocked<AlertServiceContract>;
};
esClient: DeeplyMockedKeys<ElasticsearchClient>;
}> => {
const esClient = elasticsearchServiceMock.createElasticsearchClient();
const log = loggingSystemMock.create().get('case');
@ -82,5 +85,6 @@ export const createCaseClientWithMockSavedObjectsClient = async ({
return {
client: caseClient,
services: { userActionService, alertsService },
esClient,
};
};

View file

@ -19,6 +19,7 @@ import {
CaseUserActionsResponse,
User,
} from '../../common/api';
import { AlertInfo } from '../common';
import {
CaseConfigureServiceSetup,
CaseServiceSetup,
@ -46,14 +47,11 @@ export interface CaseClientAddComment {
}
export interface CaseClientUpdateAlertsStatus {
ids: string[];
status: CaseStatuses;
indices: Set<string>;
alerts: UpdateAlertRequest[];
}
export interface CaseClientGetAlerts {
ids: string[];
indices: Set<string>;
alertsInfo: AlertInfo[];
}
export interface CaseClientGetUserActions {
@ -85,6 +83,15 @@ export interface ConfigureFields {
connectorType: string;
}
/**
* Defines the fields necessary to update an alert's status.
*/
export interface UpdateAlertRequest {
id: string;
index: string;
status: CaseStatuses;
}
/**
* This represents the interface that other plugins can access.
*/

View file

@ -7,3 +7,4 @@
export * from './models';
export * from './utils';
export * from './types';

View file

@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
/**
* This structure holds the alert ID and index from an alert comment
*/
export interface AlertInfo {
id: string;
index: string;
}

View file

@ -6,8 +6,15 @@
*/
import { SavedObjectsFindResult, SavedObjectsFindResponse } from 'kibana/server';
import { CaseStatuses, CommentAttributes, CommentType, User } from '../../common/api';
import { AlertInfo, getAlertIndicesAndIDs } from '../routes/api/utils';
import {
CaseStatuses,
CommentAttributes,
CommentRequest,
CommentType,
User,
} from '../../common/api';
import { UpdateAlertRequest } from '../client/types';
import { getAlertInfoFromComments } from '../routes/api/utils';
/**
* Default sort field for querying saved objects.
@ -22,27 +29,14 @@ export const nullUser: User = { username: null, full_name: null, email: null };
/**
* Adds the ids and indices to a map of statuses
*/
export function addAlertInfoToStatusMap({
export function createAlertUpdateRequest({
comment,
statusMap,
status,
}: {
comment: CommentAttributes;
statusMap: Map<CaseStatuses, AlertInfo>;
comment: CommentRequest;
status: CaseStatuses;
}) {
const newAlertInfo = getAlertIndicesAndIDs([comment]);
// combine the already accumulated ids and indices with the new ones from this alert comment
if (newAlertInfo.ids.length > 0 && newAlertInfo.indices.size > 0) {
const accAlertInfo = statusMap.get(status) ?? { ids: [], indices: new Set<string>() };
accAlertInfo.ids.push(...newAlertInfo.ids);
accAlertInfo.indices = new Set<string>([
...accAlertInfo.indices.values(),
...newAlertInfo.indices.values(),
]);
statusMap.set(status, accAlertInfo);
}
}): UpdateAlertRequest[] {
return getAlertInfoFromComments([comment]).map((alert) => ({ ...alert, status }));
}
/**

View file

@ -404,6 +404,43 @@ export const mockCaseComments: Array<SavedObject<CommentAttributes>> = [
updated_at: '2019-11-25T22:32:30.608Z',
version: 'WzYsMV0=',
},
{
type: 'cases-comment',
id: 'mock-comment-6',
attributes: {
associationType: AssociationType.case,
type: CommentType.generatedAlert,
index: 'test-index',
alertId: 'test-id',
created_at: '2019-11-25T22:32:30.608Z',
created_by: {
full_name: 'elastic',
email: 'testemail@elastic.co',
username: 'elastic',
},
pushed_at: null,
pushed_by: null,
rule: {
id: 'rule-id-1',
name: 'rule-name-1',
},
updated_at: '2019-11-25T22:32:30.608Z',
updated_by: {
full_name: 'elastic',
email: 'testemail@elastic.co',
username: 'elastic',
},
},
references: [
{
type: 'cases',
name: 'associated-cases',
id: 'mock-id-4',
},
],
updated_at: '2019-11-25T22:32:30.608Z',
version: 'WzYsMV0=',
},
];
export const mockCaseConfigure: Array<SavedObject<ESCasesConfigureAttributes>> = [

View file

@ -296,4 +296,83 @@ describe('PATCH comment', () => {
expect(response.status).toEqual(404);
expect(response.payload.isBoom).toEqual(true);
});
describe('alert format', () => {
it.each([
['1', ['index1', 'index2'], CommentType.alert, 'mock-comment-4'],
[['1', '2'], 'index', CommentType.alert, 'mock-comment-4'],
['1', ['index1', 'index2'], CommentType.generatedAlert, 'mock-comment-6'],
[['1', '2'], 'index', CommentType.generatedAlert, 'mock-comment-6'],
])(
'returns an error with an alert comment with contents id: %p indices: %p type: %s comment id: %s',
async (alertId, index, type, commentID) => {
const request = httpServerMock.createKibanaRequest({
path: CASE_COMMENTS_URL,
method: 'patch',
params: {
case_id: 'mock-id-4',
},
body: {
type,
alertId,
index,
rule: {
id: 'rule-id',
name: 'rule',
},
id: commentID,
version: 'WzYsMV0=',
},
});
const { context } = await createRouteContext(
createMockSavedObjectsRepository({
caseSavedObject: mockCases,
caseCommentSavedObject: mockCaseComments,
})
);
const response = await routeHandler(context, request, kibanaResponseFactory);
expect(response.status).toEqual(400);
}
);
it.each([
['1', ['index1'], CommentType.alert],
[['1', '2'], ['index', 'other-index'], CommentType.alert],
])(
'does not return an error with an alert comment with contents id: %p indices: %p type: %s',
async (alertId, index, type) => {
const request = httpServerMock.createKibanaRequest({
path: CASE_COMMENTS_URL,
method: 'patch',
params: {
case_id: 'mock-id-4',
},
body: {
type,
alertId,
index,
rule: {
id: 'rule-id',
name: 'rule',
},
id: 'mock-comment-4',
// this version is different than the one in mockCaseComments because it gets updated in place
version: 'WzE3LDFd',
},
});
const { context } = await createRouteContext(
createMockSavedObjectsRepository({
caseSavedObject: mockCases,
caseCommentSavedObject: mockCaseComments,
})
);
const response = await routeHandler(context, request, kibanaResponseFactory);
expect(response.status).toEqual(200);
}
);
});
});

View file

@ -105,7 +105,7 @@ describe('GET case', () => {
const response = await routeHandler(context, request, kibanaResponseFactory);
expect(response.status).toEqual(200);
expect(response.payload.comments).toHaveLength(5);
expect(response.payload.comments).toHaveLength(6);
});
it(`returns an error when thrown from getAllCaseComments`, async () => {

View file

@ -132,8 +132,7 @@ describe('Push case', () => {
const response = await routeHandler(context, request, kibanaResponseFactory);
expect(response.status).toEqual(200);
expect(caseClient.getAlerts).toHaveBeenCalledWith({
ids: ['test-id'],
indices: new Set<string>(['test-index']),
alertsInfo: [{ id: 'test-id', index: 'test-index' }],
});
});

View file

@ -39,7 +39,6 @@ import {
import { SUB_CASES_PATCH_DEL_URL } from '../../../../../common/constants';
import { RouteDeps } from '../../types';
import {
AlertInfo,
escapeHatch,
flattenSubCaseSavedObject,
isCommentRequestTypeAlertOrGenAlert,
@ -47,7 +46,8 @@ import {
} from '../../utils';
import { getCaseToUpdate } from '../helpers';
import { buildSubCaseUserActions } from '../../../../services/user_actions/helpers';
import { addAlertInfoToStatusMap } from '../../../../common';
import { createAlertUpdateRequest } from '../../../../common';
import { UpdateAlertRequest } from '../../../../client/types';
import { createCaseError } from '../../../../common/error';
interface UpdateArgs {
@ -235,29 +235,23 @@ async function updateAlerts({
// get all the alerts for all sub cases that need to be synced
const totalAlerts = await getAlertComments({ caseService, client, subCasesToSync });
// create a map of the status (open, closed, etc) to alert info that needs to be updated
const alertsToUpdate = totalAlerts.saved_objects.reduce((acc, alertComment) => {
if (isCommentRequestTypeAlertOrGenAlert(alertComment.attributes)) {
const id = getID(alertComment);
const status =
id !== undefined
? subCasesToSyncMap.get(id)?.status ?? CaseStatuses.open
: CaseStatuses.open;
const alertsToUpdate = totalAlerts.saved_objects.reduce(
(acc: UpdateAlertRequest[], alertComment) => {
if (isCommentRequestTypeAlertOrGenAlert(alertComment.attributes)) {
const id = getID(alertComment);
const status =
id !== undefined
? subCasesToSyncMap.get(id)?.status ?? CaseStatuses.open
: CaseStatuses.open;
addAlertInfoToStatusMap({ comment: alertComment.attributes, statusMap: acc, status });
}
return acc;
}, new Map<CaseStatuses, AlertInfo>());
acc.push(...createAlertUpdateRequest({ comment: alertComment.attributes, status }));
}
return acc;
},
[]
);
// This does at most 3 calls to Elasticsearch to update the status of the alerts to either open, closed, or in-progress
for (const [status, alertInfo] of alertsToUpdate.entries()) {
if (alertInfo.ids.length > 0 && alertInfo.indices.size > 0) {
caseClient.updateAlertsStatus({
ids: alertInfo.ids,
status,
indices: alertInfo.indices,
});
}
}
await caseClient.updateAlertsStatus({ alerts: alertsToUpdate });
} catch (error) {
throw createCaseError({
message: `Failed to update alert status while updating sub cases: ${JSON.stringify(

View file

@ -45,6 +45,7 @@ import {
import { transformESConnectorToCaseConnector } from './cases/helpers';
import { SortFieldCase } from './types';
import { AlertInfo } from '../../common';
import { isCaseError } from '../../common/error';
export const transformNewSubCase = ({
@ -111,55 +112,50 @@ export const getAlertIds = (comment: CommentRequest): string[] => {
return [];
};
/**
* This structure holds the alert IDs and indices found from multiple alert comments
*/
export interface AlertInfo {
ids: string[];
indices: Set<string>;
}
const getIDsAndIndicesAsArrays = (
comment: CommentRequestAlertType
): { ids: string[]; indices: string[] } => {
return {
ids: Array.isArray(comment.alertId) ? comment.alertId : [comment.alertId],
indices: Array.isArray(comment.index) ? comment.index : [comment.index],
};
};
const accumulateIndicesAndIDs = (comment: CommentAttributes, acc: AlertInfo): AlertInfo => {
if (isCommentRequestTypeAlertOrGenAlert(comment)) {
acc.ids.push(...getAlertIds(comment));
const indices = Array.isArray(comment.index) ? comment.index : [comment.index];
indices.forEach((index) => acc.indices.add(index));
/**
* This functions extracts the ids and indices from an alert comment. It enforces that the alertId and index are either
* both strings or string arrays that are the same length. If they are arrays they represent a 1-to-1 mapping of
* id existing in an index at each position in the array. This is not ideal. Ideally an alert comment request would
* accept an array of objects like this: Array<{id: string; index: string; ruleName: string ruleID: string}> instead.
*
* To reformat the alert comment request requires a migration and a breaking API change.
*/
const getAndValidateAlertInfoFromComment = (comment: CommentRequest): AlertInfo[] => {
if (!isCommentRequestTypeAlertOrGenAlert(comment)) {
return [];
}
return acc;
const { ids, indices } = getIDsAndIndicesAsArrays(comment);
if (ids.length !== indices.length) {
return [];
}
return ids.map((id, index) => ({ id, index: indices[index] }));
};
/**
* Builds an AlertInfo object accumulating the alert IDs and indices for the passed in alerts.
*/
export const getAlertIndicesAndIDs = (comments: CommentAttributes[] | undefined): AlertInfo => {
export const getAlertInfoFromComments = (comments: CommentRequest[] | undefined): AlertInfo[] => {
if (comments === undefined) {
return { ids: [], indices: new Set<string>() };
return [];
}
return comments.reduce(
(acc: AlertInfo, comment) => {
return accumulateIndicesAndIDs(comment, acc);
},
{ ids: [], indices: new Set<string>() }
);
};
/**
* Builds an AlertInfo object accumulating the alert IDs and indices for the passed in alert saved objects.
*/
export const getAlertIndicesAndIDsFromSO = (
comments: SavedObjectsFindResponse<CommentAttributes> | undefined
): AlertInfo => {
if (comments === undefined) {
return { ids: [], indices: new Set<string>() };
}
return comments.saved_objects.reduce(
(acc: AlertInfo, comment) => {
return accumulateIndicesAndIDs(comment.attributes, acc);
},
{ ids: [], indices: new Set<string>() }
);
return comments.reduce((acc: AlertInfo[], comment) => {
const alertInfo = getAndValidateAlertInfoFromComment(comment);
acc.push(...alertInfo);
return acc;
}, []);
};
export const transformNewComment = ({
@ -378,5 +374,47 @@ export const decodeCommentRequest = (comment: CommentRequest) => {
pipe(excess(ContextTypeUserRt).decode(comment), fold(throwErrors(badRequest), identity));
} else if (isCommentRequestTypeAlertOrGenAlert(comment)) {
pipe(excess(AlertCommentRequestRt).decode(comment), fold(throwErrors(badRequest), identity));
const { ids, indices } = getIDsAndIndicesAsArrays(comment);
/**
* The alertId and index field must either be both of type string or they must both be string[] and be the same length.
* Having a one-to-one relationship between the id and index of an alert avoids accidentally updating or
* retrieving the wrong alert. Elasticsearch only guarantees that the _id (the field we use for alertId) to be
* unique within a single index. So if we attempt to update or get a specific alert across multiple indices we could
* update or receive the wrong one.
*
* Consider the situation where we have a alert1 with _id = '100' in index 'my-index-awesome' and also in index
* 'my-index-hi'.
* If we attempt to update the status of alert1 using an index pattern like `my-index-*` or even providing multiple
* indices, there's a chance we'll accidentally update too many alerts.
*
* This check doesn't enforce that the API request has the correct alert ID to index relationship it just guards
* against accidentally making a request like:
* {
* alertId: [1,2,3],
* index: awesome,
* }
*
* Instead this requires the requestor to provide:
* {
* alertId: [1,2,3],
* index: [awesome, awesome, awesome]
* }
*
* Ideally we'd change the format of the comment request to be an array of objects like:
* {
* alerts: [{id: 1, index: awesome}, {id: 2, index: awesome}]
* }
*
* But we'd need to also implement a migration because the saved object document currently stores the id and index
* in separate fields.
*/
if (ids.length !== indices.length) {
throw badRequest(
`Received an alert comment with ids and indices arrays of different lengths ids: ${JSON.stringify(
ids
)} indices: ${JSON.stringify(indices)}`
);
}
}
};

View file

@ -17,10 +17,8 @@ describe('updateAlertsStatus', () => {
describe('happy path', () => {
let alertService: AlertServiceContract;
const args = {
ids: ['alert-id-1'],
indices: new Set<string>(['.siem-signals']),
alerts: [{ id: 'alert-id-1', index: '.siem-signals', status: CaseStatuses.closed }],
request: {} as KibanaRequest,
status: CaseStatuses.closed,
scopedClusterClient: esClient,
logger,
};
@ -33,14 +31,17 @@ describe('updateAlertsStatus', () => {
test('it update the status of the alert correctly', async () => {
await alertService.updateAlertsStatus(args);
expect(esClient.updateByQuery).toHaveBeenCalledWith({
body: {
query: { ids: { values: args.ids } },
script: { lang: 'painless', source: `ctx._source.signal.status = '${args.status}'` },
},
conflicts: 'abort',
ignore_unavailable: true,
index: [...args.indices],
expect(esClient.bulk).toHaveBeenCalledWith({
body: [
{ update: { _id: 'alert-id-1', _index: '.siem-signals' } },
{
doc: {
signal: {
status: CaseStatuses.closed,
},
},
},
],
});
});
@ -48,9 +49,7 @@ describe('updateAlertsStatus', () => {
it('ignores empty indices', async () => {
expect(
await alertService.updateAlertsStatus({
ids: ['alert-id-1'],
status: CaseStatuses.closed,
indices: new Set<string>(['']),
alerts: [{ id: 'alert-id-1', index: '', status: CaseStatuses.closed }],
scopedClusterClient: esClient,
logger,
})

View file

@ -5,28 +5,26 @@
* 2.0.
*/
import _ from 'lodash';
import { isEmpty } from 'lodash';
import type { PublicMethodsOf } from '@kbn/utility-types';
import { ElasticsearchClient, Logger } from 'kibana/server';
import { CaseStatuses } from '../../../common/api';
import { MAX_ALERTS_PER_SUB_CASE } from '../../../common/constants';
import { UpdateAlertRequest } from '../../client/types';
import { AlertInfo } from '../../common';
import { createCaseError } from '../../common/error';
export type AlertServiceContract = PublicMethodsOf<AlertService>;
interface UpdateAlertsStatusArgs {
ids: string[];
status: CaseStatuses;
indices: Set<string>;
alerts: UpdateAlertRequest[];
scopedClusterClient: ElasticsearchClient;
logger: Logger;
}
interface GetAlertsArgs {
ids: string[];
indices: Set<string>;
alertsInfo: AlertInfo[];
scopedClusterClient: ElasticsearchClient;
logger: Logger;
}
@ -38,54 +36,33 @@ interface Alert {
}
interface AlertsResponse {
hits: {
hits: Alert[];
};
docs: Alert[];
}
/**
* remove empty strings from the indices, I'm not sure how likely this is but in the case that
* the document doesn't have _index set the security_solution code sets the value to an empty string
* instead
*/
function getValidIndices(indices: Set<string>): string[] {
return [...indices].filter((index) => !_.isEmpty(index));
function isEmptyAlert(alert: AlertInfo): boolean {
return isEmpty(alert.id) || isEmpty(alert.index);
}
export class AlertService {
constructor() {}
public async updateAlertsStatus({
ids,
status,
indices,
scopedClusterClient,
logger,
}: UpdateAlertsStatusArgs) {
const sanitizedIndices = getValidIndices(indices);
if (sanitizedIndices.length <= 0) {
logger.warn(`Empty alert indices when updateAlertsStatus ids: ${JSON.stringify(ids)}`);
return;
}
public async updateAlertsStatus({ alerts, scopedClusterClient, logger }: UpdateAlertsStatusArgs) {
try {
const result = await scopedClusterClient.updateByQuery({
index: sanitizedIndices,
conflicts: 'abort',
body: {
script: {
source: `ctx._source.signal.status = '${status}'`,
lang: 'painless',
},
query: { ids: { values: ids } },
},
ignore_unavailable: true,
});
const body = alerts
.filter((alert) => !isEmptyAlert(alert))
.flatMap((alert) => [
{ update: { _id: alert.id, _index: alert.index } },
{ doc: { signal: { status: alert.status } } },
]);
return result;
if (body.length <= 0) {
return;
}
return scopedClusterClient.bulk({ body });
} catch (error) {
throw createCaseError({
message: `Failed to update alert status ids: ${JSON.stringify(ids)}: ${error}`,
message: `Failed to update alert status ids: ${JSON.stringify(alerts)}: ${error}`,
error,
logger,
});
@ -94,38 +71,25 @@ export class AlertService {
public async getAlerts({
scopedClusterClient,
ids,
indices,
alertsInfo,
logger,
}: GetAlertsArgs): Promise<AlertsResponse | undefined> {
const index = getValidIndices(indices);
if (index.length <= 0) {
logger.warn(`Empty alert indices when retrieving alerts ids: ${JSON.stringify(ids)}`);
return;
}
try {
const result = await scopedClusterClient.search<AlertsResponse>({
index,
body: {
query: {
bool: {
filter: {
ids: {
values: ids,
},
},
},
},
},
size: MAX_ALERTS_PER_SUB_CASE,
ignore_unavailable: true,
});
const docs = alertsInfo
.filter((alert) => !isEmptyAlert(alert))
.slice(0, MAX_ALERTS_PER_SUB_CASE)
.map((alert) => ({ _id: alert.id, _index: alert.index }));
return result.body;
if (docs.length <= 0) {
return;
}
const results = await scopedClusterClient.mget<AlertsResponse>({ body: { docs } });
return results.body;
} catch (error) {
throw createCaseError({
message: `Failed to retrieve alerts ids: ${JSON.stringify(ids)}: ${error}`,
message: `Failed to retrieve alerts ids: ${JSON.stringify(alertsInfo)}: ${error}`,
error,
logger,
});

View file

@ -438,8 +438,12 @@ export default ({ getService }: FtrProviderContext): void => {
});
// There should be no change in their status since syncing is disabled
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
const updatedIndWithStatus: CasesResponse = (await setStatus({
supertest,
@ -467,8 +471,12 @@ export default ({ getService }: FtrProviderContext): void => {
});
// There should still be no change in their status since syncing is disabled
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
// turn on the sync settings
await supertest
@ -492,8 +500,139 @@ export default ({ getService }: FtrProviderContext): void => {
});
// alerts should be updated now that the
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.closed);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses['in-progress']);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.closed
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses['in-progress']
);
});
});
describe('esArchiver', () => {
const defaultSignalsIndex = '.siem-signals-default-000001';
beforeEach(async () => {
await esArchiver.load('cases/signals/duplicate_ids');
});
afterEach(async () => {
await esArchiver.unload('cases/signals/duplicate_ids');
await deleteAllCaseItems(es);
});
it('should not update the status of duplicate alert ids in separate indices', async () => {
const getSignals = async () => {
return getSignalsWithES({
es,
indices: [defaultSignalsIndex, signalsIndex2],
ids: [signalIDInFirstIndex, signalIDInSecondIndex],
});
};
// this id exists only in .siem-signals-default-000001
const signalIDInFirstIndex =
'cae78067e65582a3b277c1ad46ba3cb29044242fe0d24bbf3fcde757fdd31d1c';
// This id exists in both .siem-signals-default-000001 and .siem-signals-default-000002
const signalIDInSecondIndex = 'duplicate-signal-id';
const signalsIndex2 = '.siem-signals-default-000002';
const { body: individualCase } = await supertest
.post(CASES_URL)
.set('kbn-xsrf', 'true')
.send({
...postCaseReq,
settings: {
syncAlerts: false,
},
});
const { body: updatedIndWithComment } = await supertest
.post(`${CASES_URL}/${individualCase.id}/comments`)
.set('kbn-xsrf', 'true')
.send({
alertId: signalIDInFirstIndex,
index: defaultSignalsIndex,
rule: { id: 'test-rule-id', name: 'test-index-id' },
type: CommentType.alert,
})
.expect(200);
const { body: updatedIndWithComment2 } = await supertest
.post(`${CASES_URL}/${updatedIndWithComment.id}/comments`)
.set('kbn-xsrf', 'true')
.send({
alertId: signalIDInSecondIndex,
index: signalsIndex2,
rule: { id: 'test-rule-id', name: 'test-index-id' },
type: CommentType.alert,
})
.expect(200);
await es.indices.refresh({ index: defaultSignalsIndex });
let signals = await getSignals();
// There should be no change in their status since syncing is disabled
expect(
signals.get(defaultSignalsIndex)?.get(signalIDInFirstIndex)?._source.signal.status
).to.be(CaseStatuses.open);
expect(
signals.get(signalsIndex2)?.get(signalIDInSecondIndex)?._source.signal.status
).to.be(CaseStatuses.open);
const updatedIndWithStatus: CasesResponse = (await setStatus({
supertest,
cases: [
{
id: updatedIndWithComment2.id,
version: updatedIndWithComment2.version,
status: CaseStatuses.closed,
},
],
type: 'case',
})) as CasesResponse;
await es.indices.refresh({ index: defaultSignalsIndex });
signals = await getSignals();
// There should still be no change in their status since syncing is disabled
expect(
signals.get(defaultSignalsIndex)?.get(signalIDInFirstIndex)?._source.signal.status
).to.be(CaseStatuses.open);
expect(
signals.get(signalsIndex2)?.get(signalIDInSecondIndex)?._source.signal.status
).to.be(CaseStatuses.open);
// turn on the sync settings
await supertest
.patch(CASES_URL)
.set('kbn-xsrf', 'true')
.send({
cases: [
{
id: updatedIndWithStatus[0].id,
version: updatedIndWithStatus[0].version,
settings: { syncAlerts: true },
},
],
})
.expect(200);
await es.indices.refresh({ index: defaultSignalsIndex });
signals = await getSignals();
// alerts should be updated now that the
expect(
signals.get(defaultSignalsIndex)?.get(signalIDInFirstIndex)?._source.signal.status
).to.be(CaseStatuses.closed);
expect(
signals.get(signalsIndex2)?.get(signalIDInSecondIndex)?._source.signal.status
).to.be(CaseStatuses.closed);
// the duplicate signal id in the other index should not be affect (so its status should be open)
expect(
signals.get(defaultSignalsIndex)?.get(signalIDInSecondIndex)?._source.signal.status
).to.be(CaseStatuses.open);
});
});

View file

@ -96,7 +96,9 @@ export default function ({ getService }: FtrProviderContext) {
let signals = await getSignalsWithES({ es, indices: defaultSignalsIndex, ids: signalID });
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
await setStatus({
supertest,
@ -114,7 +116,9 @@ export default function ({ getService }: FtrProviderContext) {
signals = await getSignalsWithES({ es, indices: defaultSignalsIndex, ids: signalID });
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses['in-progress']);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses['in-progress']
);
});
it('should update the status of multiple alerts attached to a sub case', async () => {
@ -152,8 +156,12 @@ export default function ({ getService }: FtrProviderContext) {
ids: [signalID, signalID2],
});
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
await setStatus({
supertest,
@ -175,8 +183,12 @@ export default function ({ getService }: FtrProviderContext) {
ids: [signalID, signalID2],
});
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses['in-progress']);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses['in-progress']);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses['in-progress']
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses['in-progress']
);
});
it('should update the status of multiple alerts attached to multiple sub cases in one collection', async () => {
@ -232,8 +244,12 @@ export default function ({ getService }: FtrProviderContext) {
});
// There should be no change in their status since syncing is disabled
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
await setStatus({
supertest,
@ -256,8 +272,12 @@ export default function ({ getService }: FtrProviderContext) {
});
// There still should be no change in their status since syncing is disabled
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
// Turn sync alerts on
await supertest
@ -282,8 +302,12 @@ export default function ({ getService }: FtrProviderContext) {
ids: [signalID, signalID2],
});
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.closed);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses['in-progress']);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.closed
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses['in-progress']
);
});
it('should update the status of alerts attached to a case and sub case when sync settings is turned on', async () => {
@ -342,8 +366,12 @@ export default function ({ getService }: FtrProviderContext) {
});
// There should be no change in their status since syncing is disabled
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
await setStatus({
supertest,
@ -380,8 +408,12 @@ export default function ({ getService }: FtrProviderContext) {
});
// There should still be no change in their status since syncing is disabled
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.open);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses.open
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.open
);
// Turn sync alerts on
await supertest
@ -421,8 +453,12 @@ export default function ({ getService }: FtrProviderContext) {
});
// alerts should be updated now that the
expect(signals.get(signalID)?._source.signal.status).to.be(CaseStatuses['in-progress']);
expect(signals.get(signalID2)?._source.signal.status).to.be(CaseStatuses.closed);
expect(signals.get(defaultSignalsIndex)?.get(signalID)?._source.signal.status).to.be(
CaseStatuses['in-progress']
);
expect(signals.get(defaultSignalsIndex)?.get(signalID2)?._source.signal.status).to.be(
CaseStatuses.closed
);
});
it('404s when sub case id is invalid', async () => {

View file

@ -54,11 +54,11 @@ export const getSignalsWithES = async ({
es: Client;
indices: string | string[];
ids: string | string[];
}): Promise<Map<string, Hit<SignalHit>>> => {
}): Promise<Map<string, Map<string, Hit<SignalHit>>>> => {
const signals = await es.search<SearchResponse<SignalHit>>({
index: indices,
body: {
size: ids.length,
size: 10000,
query: {
bool: {
filter: [
@ -72,10 +72,17 @@ export const getSignalsWithES = async ({
},
},
});
return signals.body.hits.hits.reduce((acc, hit) => {
acc.set(hit._id, hit);
let indexMap = acc.get(hit._index);
if (indexMap === undefined) {
indexMap = new Map<string, Hit<SignalHit>>([[hit._id, hit]]);
} else {
indexMap.set(hit._id, hit);
}
acc.set(hit._index, indexMap);
return acc;
}, new Map<string, Hit<SignalHit>>());
}, new Map<string, Map<string, Hit<SignalHit>>>());
};
interface SetStatusCasesParams {

File diff suppressed because it is too large Load diff