Skip to content

Commit

Permalink
[Security Solution] Security RAC migration follow up bug fixes (#116386)
Browse files Browse the repository at this point in the history
* Add compatibility aliases to alerts as data indices

* Fix dupe mitigation, allow more fields in mapping

* Remove legacy signals fields from new RAC alerts

* Fix cypress test

* Remove outdated comment

* Reduce flakiness in time based test

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
marshallmain and kibanamachine authored Oct 28, 2021
1 parent 2f3d4fc commit d280f12
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ export const technicalRuleFieldMap = {
array: false,
required: false,
},
[Fields.ALERT_RISK_SCORE]: {
type: 'float',
array: false,
required: false,
},
[Fields.ALERT_WORKFLOW_STATUS]: {
type: 'keyword',
array: false,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/rule_registry/common/field_map/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
*/

export interface FieldMap {
[key: string]: { type: string; required?: boolean; array?: boolean };
[key: string]: { type: string; required?: boolean; array?: boolean; path?: string };
}
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ export class ResourceInstaller {
// @ts-expect-error
rollover_alias: primaryNamespacedAlias,
},
'index.mapping.total_fields.limit': 1100,
},
mappings: {
dynamic: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
.getWriter({ namespace: options.spaceId })
.bulk({
body: alerts.flatMap((alert) => [
{ index: {} },
{ index: { _id: alert.id } },
{
[VERSION]: ruleDataClient.kibanaVersion,
...commonRuleFields,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ describe('Alert details with unmapped fields', () => {
});
});

// This test needs to be updated to not look for the field in a specific row, as it prevents us from adding/removing fields
it('Displays the unmapped field on the table', () => {
const expectedUnmmappedField = {
row: 54,
row: 82,
field: 'unmapped',
text: 'This is the unmapped field',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { bulkCreateFactory, wrapHitsFactory, wrapSequencesFactory } from './fact
import { RuleExecutionLogClient, truncateMessageList } from '../rule_execution_log';
import { RuleExecutionStatus } from '../../../../common/detection_engine/schemas/common/schemas';
import { scheduleThrottledNotificationActions } from '../notifications/schedule_throttle_notification_actions';
import aadFieldConversion from '../routes/index/signal_aad_mapping.json';

/* eslint-disable complexity */
export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
Expand Down Expand Up @@ -225,16 +226,17 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
refresh
);

const legacySignalFields: string[] = Object.keys(aadFieldConversion);
const wrapHits = wrapHitsFactory({
ignoreFields,
ignoreFields: [...ignoreFields, ...legacySignalFields],
mergeStrategy,
completeRule,
spaceId,
});

const wrapSequences = wrapSequencesFactory({
logger,
ignoreFields,
ignoreFields: [...ignoreFields, ...legacySignalFields],
mergeStrategy,
completeRule,
spaceId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,5 @@ export const wrapHitsFactory =
};
});

return filterDuplicateSignals(completeRule.alertId, wrappedDocs, false);
return filterDuplicateSignals(completeRule.alertId, wrappedDocs, true);
};
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ export const alertsFieldMap: FieldMap = {
array: false,
required: true,
},
'kibana.alert.original_event.severity': {
type: 'long',
array: false,
required: false,
},
'kibana.alert.original_event.start': {
type: 'date',
array: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,17 @@ export const rulesFieldMap = {
array: true,
required: false,
},
'kibana.alert.rule.threat_mapping.field': {
'kibana.alert.rule.threat_mapping.entries.field': {
type: 'keyword',
array: true,
required: false,
},
'kibana.alert.rule.threat_mapping.value': {
'kibana.alert.rule.threat_mapping.entries.value': {
type: 'keyword',
array: true,
required: false,
},
'kibana.alert.rule.threat_mapping.type': {
'kibana.alert.rule.threat_mapping.entries.type': {
type: 'keyword',
array: true,
required: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ describe('utils', () => {
it('throws an error if the validator is called after the specified interval', async () => {
const validator = buildExecutionIntervalValidator('1s');

await new Promise((r) => setTimeout(r, 1001));
await new Promise((r) => setTimeout(r, 2000));
expect(() => validator()).toThrowError(
'Current rule execution has exceeded its allotted interval (1s) and has been stopped.'
);
Expand Down
16 changes: 8 additions & 8 deletions x-pack/plugins/security_solution/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import { Observable } from 'rxjs';
import LRU from 'lru-cache';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import {
SIGNALS_ID,
QUERY_RULE_TYPE_ID,
Expand All @@ -22,6 +21,8 @@ import { Logger, SavedObjectsClient } from '../../../../src/core/server';
import { UsageCounter } from '../../../../src/plugins/usage_collection/server';

import { ECS_COMPONENT_TEMPLATE_NAME } from '../../rule_registry/common/assets';
import { FieldMap } from '../../rule_registry/common/field_map';
import { technicalRuleFieldMap } from '../../rule_registry/common/assets/field_maps/technical_rule_field_map';
import { mappingFromFieldMap } from '../../rule_registry/common/mapping_from_field_map';
import { IRuleDataClient, Dataset } from '../../rule_registry/server';
import { ListPluginSetup } from '../../lists/server';
Expand Down Expand Up @@ -188,13 +189,9 @@ export class Plugin implements ISecuritySolutionPlugin {
};

if (isRuleRegistryEnabled) {
// NOTE: this is not used yet
// TODO: convert the aliases to FieldMaps. Requires enhancing FieldMap to support alias path.
// Split aliases by component template since we need to alias some fields in technical field mappings,
// some fields in security solution specific component template.
const aliases: Record<string, estypes.MappingProperty> = {};
const aliasesFieldMap: FieldMap = {};
Object.entries(aadFieldConversion).forEach(([key, value]) => {
aliases[key] = {
aliasesFieldMap[key] = {
type: 'alias',
path: value,
};
Expand All @@ -208,7 +205,10 @@ export class Plugin implements ISecuritySolutionPlugin {
componentTemplates: [
{
name: 'mappings',
mappings: mappingFromFieldMap({ ...alertsFieldMap, ...rulesFieldMap }, false),
mappings: mappingFromFieldMap(
{ ...technicalRuleFieldMap, ...alertsFieldMap, ...rulesFieldMap, ...aliasesFieldMap },
false
),
},
],
secondaryAlias: config.signalsIndex,
Expand Down

0 comments on commit d280f12

Please sign in to comment.