[Security Solution][Detections][Threshold Rules][7.12] Final Threshol…

…d Rule Fixes for 7.12 (#93553) * refactor * Add explicit to/from * cleanup * A bit more cleanup * Fix threshold signal history bug when rule is edited * Added comments * more cleanup, fix tests * Add tests later * Reverse the tuples array * Fix getThresholdBucketFilters test * Fix translations
elastic · Mar 5, 2021 · 840edac · 840edac
1 parent 8902035
commit 840edac
Show file tree

Hide file tree

Showing 25 changed files with 821 additions and 710 deletions.
diff --git a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/index.ts b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/index.ts
@@ -7,6 +7,7 @@
  */
 
 export * from './empty_field';
+export * from './max_length';
 export * from './min_length';
 export * from './min_selectable_selection';
 export * from './url';

diff --git a/src/plugins/es_ui_shared/static/forms/helpers/field_validators/max_length.ts b/src/plugins/es_ui_shared/static/forms/helpers/field_validators/max_length.ts
@@ -0,0 +1,42 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { ValidationFunc, ValidationError } from '../../hook_form_lib';
+import { hasMaxLengthString } from '../../../validators/string';
+import { hasMaxLengthArray } from '../../../validators/array';
+import { ERROR_CODE } from './types';
+
+export const maxLengthField = ({
+  length = 0,
+  message,
+}: {
+  length: number;
+  message: string | ((err: Partial<ValidationError>) => string);
+}) => (...args: Parameters<ValidationFunc>): ReturnType<ValidationFunc<any, ERROR_CODE>> => {
+  const [{ value }] = args;
+
+  // Validate for Arrays
+  if (Array.isArray(value)) {
+    return hasMaxLengthArray(length)(value)
+      ? undefined
+      : {
+          code: 'ERR_MAX_LENGTH',
+          length,
+          message: typeof message === 'function' ? message({ length }) : message,
+        };
+  }
+
+  // Validate for Strings
+  return hasMaxLengthString(length)((value as string).trim())
+    ? undefined
+    : {
+        code: 'ERR_MAX_LENGTH',
+        length,
+        message: typeof message === 'function' ? message({ length }) : message,
+      };
+};
diff --git a/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx b/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx
@@ -15,7 +15,11 @@ import { i18n } from '@kbn/i18n';
 import type { Filter } from '../../../../../../../src/plugins/data/common/es_query/filters';
 import { TimelineId, TimelineStatus, TimelineType } from '../../../../common/types/timeline';
 import { updateAlertStatus } from '../../containers/detection_engine/alerts/api';
-import { SendAlertToTimelineActionProps, UpdateAlertStatusActionProps } from './types';
+import {
+  SendAlertToTimelineActionProps,
+  ThresholdAggregationData,
+  UpdateAlertStatusActionProps,
+} from './types';
 import { Ecs } from '../../../../common/ecs';
 import { GetOneTimeline, TimelineResult } from '../../../graphql/types';
 import {
@@ -123,13 +127,13 @@ export const determineToAndFrom = ({ ecs }: { ecs: Ecs[] | Ecs }) => {
     };
   }
   const ecsData = ecs as Ecs;
-  const ellapsedTimeRule = moment.duration(
+  const elapsedTimeRule = moment.duration(
     moment().diff(
       dateMath.parse(ecsData?.signal?.rule?.from != null ? ecsData.signal?.rule?.from[0] : 'now-0s')
     )
   );
   const from = moment(ecsData?.timestamp ?? new Date())
-    .subtract(ellapsedTimeRule)
+    .subtract(elapsedTimeRule)
     .toISOString();
   const to = moment(ecsData?.timestamp ?? new Date()).toISOString();
 
@@ -146,83 +150,100 @@ const getFiltersFromRule = (filters: string[]): Filter[] =>
     }
   }, [] as Filter[]);
 
-export const getThresholdAggregationDataProvider = (
+export const getThresholdAggregationData = (
   ecsData: Ecs | Ecs[],
   nonEcsData: TimelineNonEcsData[]
-): DataProvider[] => {
+): ThresholdAggregationData => {
   const thresholdEcsData: Ecs[] = Array.isArray(ecsData) ? ecsData : [ecsData];
-  return thresholdEcsData.reduce<DataProvider[]>((outerAcc, thresholdData) => {
-    const threshold = thresholdData.signal?.rule?.threshold as string[];
-
-    let aggField: string[] = [];
-    let thresholdResult: {
-      terms?: Array<{
-        field?: string;
-        value: string;
-      }>;
-      count: number;
-    };
-
-    try {
-      thresholdResult = JSON.parse((thresholdData.signal?.threshold_result as string[])[0]);
-      aggField = JSON.parse(threshold[0]).field;
-    } catch (err) {
-      thresholdResult = {
-        terms: [
-          {
-            field: (thresholdData.rule?.threshold as { field: string }).field,
-            value: (thresholdData.signal?.threshold_result as { value: string }).value,
-          },
-        ],
-        count: (thresholdData.signal?.threshold_result as { count: number }).count,
+  return thresholdEcsData.reduce<ThresholdAggregationData>(
+    (outerAcc, thresholdData) => {
+      const threshold = thresholdData.signal?.rule?.threshold as string[];
+
+      let aggField: string[] = [];
+      let thresholdResult: {
+        terms?: Array<{
+          field?: string;
+          value: string;
+        }>;
+        count: number;
+        from: string;
       };
-    }
-
-    const aggregationFields = Array.isArray(aggField) ? aggField : [aggField];
-
-    return [
-      ...outerAcc,
-      ...aggregationFields.reduce<DataProvider[]>((acc, aggregationField, i) => {
-        const aggregationValue = (thresholdResult.terms ?? []).filter(
-          (term: { field?: string | undefined; value: string }) => term.field === aggregationField
-        )[0].value;
-        const dataProviderValue = Array.isArray(aggregationValue)
-          ? aggregationValue[0]
-          : aggregationValue;
-
-        if (!dataProviderValue) {
-          return acc;
-        }
-
-        const aggregationFieldId = aggregationField.replace('.', '-');
-        const dataProviderPartial = {
-          id: `send-alert-to-timeline-action-default-draggable-event-details-value-formatted-field-value-${TimelineId.active}-${aggregationFieldId}-${dataProviderValue}`,
-          name: aggregationField,
-          enabled: true,
-          excluded: false,
-          kqlQuery: '',
-          queryMatch: {
-            field: aggregationField,
-            value: dataProviderValue,
-            operator: ':' as QueryOperator,
-          },
-        };
 
-        if (i === 0) {
-          return [
-            ...acc,
+      try {
+        thresholdResult = JSON.parse((thresholdData.signal?.threshold_result as string[])[0]);
+        aggField = JSON.parse(threshold[0]).field;
+      } catch (err) {
+        thresholdResult = {
+          terms: [
             {
-              ...dataProviderPartial,
-              and: [],
+              field: (thresholdData.rule?.threshold as { field: string }).field,
+              value: (thresholdData.signal?.threshold_result as { value: string }).value,
             },
-          ];
-        } else {
-          acc[0].and.push(dataProviderPartial);
-          return acc;
-        }
-      }, []),
-    ];
-  }, []);
+          ],
+          count: (thresholdData.signal?.threshold_result as { count: number }).count,
+          from: (thresholdData.signal?.threshold_result as { from: string }).from,
+        };
+      }
+
+      const originalTime = moment(thresholdData.signal?.original_time![0]);
+      const now = moment();
+      const ruleFrom = dateMath.parse(thresholdData.signal?.rule?.from![0]!);
+      const ruleInterval = moment.duration(now.diff(ruleFrom));
+      const fromOriginalTime = originalTime.clone().subtract(ruleInterval); // This is the default... can overshoot
+      const aggregationFields = Array.isArray(aggField) ? aggField : [aggField];
+
+      return {
+        // Use `threshold_result.from` if available (it will always be available for new signals). Otherwise, use a calculated
+        // lower bound, which could result in the timeline showing a superset of the events that made up the threshold set.
+        thresholdFrom: thresholdResult.from ?? fromOriginalTime.toISOString(),
+        thresholdTo: originalTime.toISOString(),
+        dataProviders: [
+          ...outerAcc.dataProviders,
+          ...aggregationFields.reduce<DataProvider[]>((acc, aggregationField, i) => {
+            const aggregationValue = (thresholdResult.terms ?? []).filter(
+              (term: { field?: string | undefined; value: string }) =>
+                term.field === aggregationField
+            )[0].value;
+            const dataProviderValue = Array.isArray(aggregationValue)
+              ? aggregationValue[0]
+              : aggregationValue;
+
+            if (!dataProviderValue) {
+              return acc;
+            }
+
+            const aggregationFieldId = aggregationField.replace('.', '-');
+            const dataProviderPartial = {
+              id: `send-alert-to-timeline-action-default-draggable-event-details-value-formatted-field-value-${TimelineId.active}-${aggregationFieldId}-${dataProviderValue}`,
+              name: aggregationField,
+              enabled: true,
+              excluded: false,
+              kqlQuery: '',
+              queryMatch: {
+                field: aggregationField,
+                value: dataProviderValue,
+                operator: ':' as QueryOperator,
+              },
+            };
+
+            if (i === 0) {
+              return [
+                ...acc,
+                {
+                  ...dataProviderPartial,
+                  and: [],
+                },
+              ];
+            } else {
+              acc[0].and.push(dataProviderPartial);
+              return acc;
+            }
+          }, []),
+        ],
+      };
+    },
+    { dataProviders: [], thresholdFrom: '', thresholdTo: '' } as ThresholdAggregationData
+  );
 };
 
 export const isEqlRuleWithGroupId = (ecsData: Ecs) =>
@@ -446,19 +467,24 @@ export const sendAlertToTimelineAction = async ({
   }
 
   if (isThresholdRule(ecsData)) {
+    const { thresholdFrom, thresholdTo, dataProviders } = getThresholdAggregationData(
+      ecsData,
+      nonEcsData
+    );
+
     return createTimeline({
-      from,
+      from: thresholdFrom,
       notes: null,
       timeline: {
         ...timelineDefaults,
         description: `_id: ${ecsData._id}`,
         filters: getFiltersFromRule(ecsData.signal?.rule?.filters as string[]),
-        dataProviders: getThresholdAggregationDataProvider(ecsData, nonEcsData),
+        dataProviders,
         id: TimelineId.active,
         indexNames: [],
         dateRange: {
-          start: from,
-          end: to,
+          start: thresholdFrom,
+          end: thresholdTo,
         },
         eventType: 'all',
         kqlQuery: {
@@ -475,7 +501,7 @@ export const sendAlertToTimelineAction = async ({
           },
         },
       },
-      to,
+      to: thresholdTo,
       ruleNote: noteContent,
     });
   } else {

diff --git a/x-pack/plugins/security_solution/public/detections/components/alerts_table/types.ts b/x-pack/plugins/security_solution/public/detections/components/alerts_table/types.ts
@@ -11,6 +11,7 @@ import { ISearchStart } from '../../../../../../../src/plugins/data/public';
 import { Status } from '../../../../common/detection_engine/schemas/common/schemas';
 import { Ecs } from '../../../../common/ecs';
 import { TimelineNonEcsData } from '../../../../common/search_strategy/timeline';
+import { DataProvider } from '../../../timelines/components/timeline/data_providers/data_provider';
 import { NoteResult } from '../../../graphql/types';
 import { TimelineModel } from '../../../timelines/store/timeline/model';
 import { inputsModel } from '../../../common/store';
@@ -72,3 +73,9 @@ export interface CreateTimelineProps {
 }
 
 export type CreateTimeline = ({ from, timeline, to }: CreateTimelineProps) => void;
+
+export interface ThresholdAggregationData {
+  thresholdFrom: string;
+  thresholdTo: string;
+  dataProviders: DataProvider[];
+}
diff --git a/...plugins/security_solution/public/detections/components/rules/description_step/helpers.tsx b/...plugins/security_solution/public/detections/components/rules/description_step/helpers.tsx
@@ -461,7 +461,9 @@ export const buildThresholdDescription = (label: string, threshold: Threshold):
       <>
         {isEmpty(threshold.field[0])
           ? `${i18n.THRESHOLD_RESULTS_ALL} >= ${threshold.value}`
-          : `${i18n.THRESHOLD_RESULTS_AGGREGATED_BY} ${threshold.field[0]} >= ${threshold.value}`}
+          : `${i18n.THRESHOLD_RESULTS_AGGREGATED_BY} ${
+              Array.isArray(threshold.field) ? threshold.field.join(',') : threshold.field
+            } >= ${threshold.value}`}
       </>
     ),
   },

diff --git a/.../plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx b/.../plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx
@@ -206,6 +206,28 @@ export const schema: FormSchema<DefineStepRule> = {
           defaultMessage: "Select fields to group by. Fields are joined together with 'AND'",
         }
       ),
+      validations: [
+        {
+          validator: (
+            ...args: Parameters<ValidationFunc>
+          ): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined => {
+            const [{ formData }] = args;
+            const needsValidation = isThresholdRule(formData.ruleType);
+            if (!needsValidation) {
+              return;
+            }
+            return fieldValidators.maxLengthField({
+              length: 3,
+              message: i18n.translate(
+                'xpack.securitySolution.detectionEngine.validations.thresholdFieldFieldData.arrayLengthGreaterThanMaxErrorMessage',
+                {
+                  defaultMessage: 'Number of fields must be 3 or less.',
+                }
+              ),
+            })(...args);
+          },
+        },
+      ],
     },
     value: {
       type: FIELD_TYPES.NUMBER,
@@ -245,7 +267,7 @@ export const schema: FormSchema<DefineStepRule> = {
         fieldsToValidateOnChange: ['threshold.cardinality.field', 'threshold.cardinality.value'],
         type: FIELD_TYPES.COMBO_BOX,
         label: i18n.translate(
-          'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldThresholdCardinalityFieldLabel',
+          'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldThresholdCardinalityFieldLabel',
           {
             defaultMessage: 'Count',
           }
@@ -277,7 +299,7 @@ export const schema: FormSchema<DefineStepRule> = {
           },
         ],
         helpText: i18n.translate(
-          'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldThresholdFieldCardinalityFieldHelpText',
+          'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldThresholdFieldCardinalityFieldHelpText',
           {
             defaultMessage: 'Select a field to check cardinality',
           }
@@ -287,7 +309,7 @@ export const schema: FormSchema<DefineStepRule> = {
         fieldsToValidateOnChange: ['threshold.cardinality.field', 'threshold.cardinality.value'],
         type: FIELD_TYPES.NUMBER,
         label: i18n.translate(
-          'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldThresholdCardinalityValueFieldLabel',
+          'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldThresholdCardinalityValueFieldLabel',
           {
             defaultMessage: 'Unique values',
           }

diff --git a/...k/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json b/...k/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json
@@ -391,6 +391,9 @@
           },
           "threshold_result": {
             "properties": {
+              "from": {
+                "type": "date"
+              },
               "terms": {
                 "properties": {
                   "field": {