Add the ability to filter by index pattern to the rules management ta…

…ble (#128245)
elastic · Mar 29, 2022 · eb51ea6 · eb51ea6
1 parent 52f0bf0
commit eb51ea6
Show file tree

Hide file tree

Showing 22 changed files with 225 additions and 307 deletions.
diff --git a/x-pack/plugins/security_solution/cypress/integration/detection_rules/sorting.spec.ts b/x-pack/plugins/security_solution/cypress/integration/detection_rules/sorting.spec.ts
@@ -13,7 +13,7 @@ import {
   FOURTH_RULE,
   RULES_TABLE,
   pageSelector,
-  RULES_TABLE_REFRESH_INDICATOR,
+  RULES_ROW,
 } from '../../screens/alerts_detection_rules';
 
 import { goToManageAlertsDetectionRules, waitForAlertsPanelToBeLoaded } from '../../tasks/alerts';
@@ -90,14 +90,10 @@ describe('Alerts detection rules', () => {
       .invoke('text')
       .then((ruleNameFirstPage) => {
         goToPage(2);
-        cy.get(RULES_TABLE_REFRESH_INDICATOR).should('not.exist');
-        cy.get(RULES_TABLE)
-          .find(RULE_NAME)
-          .first()
-          .invoke('text')
-          .should((ruleNameSecondPage) => {
-            expect(ruleNameFirstPage).not.to.eq(ruleNameSecondPage);
-          });
+        // Check that the rules table shows at least one row
+        cy.get(RULES_TABLE).find(RULES_ROW).should('have.length.gte', 1);
+        // Check that the rules table doesn't show the rule from the first page
+        cy.get(RULES_TABLE).should('not.contain', ruleNameFirstPage);
       });
 
     cy.get(RULES_TABLE)

diff --git a/x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts b/x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts
@@ -154,7 +154,7 @@ export const goToRuleDetails = () => {
 };
 
 export const goToTheRuleDetailsOf = (ruleName: string) => {
-  cy.get(RULE_NAME).contains(ruleName).click();
+  cy.get(RULE_NAME).should('contain', ruleName).contains(ruleName).click();
 };
 
 export const loadPrebuiltDetectionRules = () => {

diff --git a/...solution/public/common/components/ml_popover/jobs_table/filters/toggle_selected_group.tsx b/...solution/public/common/components/ml_popover/jobs_table/filters/toggle_selected_group.tsx
@@ -5,12 +5,10 @@
  * 2.0.
  */
 
-import { Dispatch, SetStateAction } from 'react';
-
 export const toggleSelectedGroup = (
   group: string,
   selectedGroups: string[],
-  setSelectedGroups: Dispatch<SetStateAction<string[]>>
+  setSelectedGroups: (groups: string[]) => void
 ): void => {
   const selectedGroupIndex = selectedGroups.indexOf(group);
   const updatedSelectedGroups = [...selectedGroups];

diff --git a/...security_solution/public/detections/components/rules/rule_actions_overflow/index.test.tsx b/...security_solution/public/detections/components/rules/rule_actions_overflow/index.test.tsx
@@ -178,7 +178,7 @@ describe('RuleActionsOverflow', () => {
       ).toEqual(false);
     });
 
-    test('it calls duplicateRulesAction when rules-details-duplicate-rule is clicked', () => {
+    test('it calls duplicate action when rules-details-duplicate-rule is clicked', () => {
       const wrapper = mount(
         <RuleActionsOverflow
           rule={mockRule('id')}
@@ -195,7 +195,7 @@ describe('RuleActionsOverflow', () => {
       );
     });
 
-    test('it calls duplicateRulesAction with the rule and rule.id when rules-details-duplicate-rule is clicked', () => {
+    test('it calls duplicate action with the rule and rule.id when rules-details-duplicate-rule is clicked', () => {
       const rule = mockRule('id');
       const wrapper = mount(
         <RuleActionsOverflow rule={rule} userHasPermissions canDuplicateRuleWithActions={true} />
@@ -210,7 +210,7 @@ describe('RuleActionsOverflow', () => {
     });
   });
 
-  test('it calls editRuleAction after the rule is duplicated', async () => {
+  test('it navigates to edit page after the rule is duplicated', async () => {
     const rule = mockRule('id');
     const ruleDuplicate = mockRule('newRule');
     executeRulesBulkActionMock.mockImplementation(() =>

diff --git a/...plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/...plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts
@@ -142,7 +142,37 @@ describe('Detections Rules API', () => {
       expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules/_find', {
         method: 'GET',
         query: {
-          filter: 'alert.attributes.name: hello world',
+          filter:
+            '(alert.attributes.name: "hello world" OR alert.attributes.params.index: "hello world" OR alert.attributes.params.threat.tactic.id: "hello world" OR alert.attributes.params.threat.tactic.name: "hello world" OR alert.attributes.params.threat.technique.id: "hello world" OR alert.attributes.params.threat.technique.name: "hello world")',
+          page: 1,
+          per_page: 20,
+          sort_field: 'enabled',
+          sort_order: 'desc',
+        },
+        signal: abortCtrl.signal,
+      });
+    });
+
+    test('check parameter url, query with a filter get escaped correctly', async () => {
+      await fetchRules({
+        filterOptions: {
+          filter: '" OR (foo:bar)',
+          showCustomRules: false,
+          showElasticRules: false,
+          tags: [],
+        },
+        sortingOptions: {
+          field: 'enabled',
+          order: 'desc',
+        },
+        signal: abortCtrl.signal,
+      });
+
+      expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules/_find', {
+        method: 'GET',
+        query: {
+          filter:
+            '(alert.attributes.name: "\\" OR (foo:bar)" OR alert.attributes.params.index: "\\" OR (foo:bar)" OR alert.attributes.params.threat.tactic.id: "\\" OR (foo:bar)" OR alert.attributes.params.threat.tactic.name: "\\" OR (foo:bar)" OR alert.attributes.params.threat.technique.id: "\\" OR (foo:bar)" OR alert.attributes.params.threat.technique.name: "\\" OR (foo:bar)")',
           page: 1,
           per_page: 20,
           sort_field: 'enabled',
@@ -226,7 +256,7 @@ describe('Detections Rules API', () => {
       expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules/_find', {
         method: 'GET',
         query: {
-          filter: 'alert.attributes.tags: "hello" AND alert.attributes.tags: "world"',
+          filter: 'alert.attributes.tags:("hello" AND "world")',
           page: 1,
           per_page: 20,
           sort_field: 'enabled',
@@ -254,7 +284,7 @@ describe('Detections Rules API', () => {
       expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules/_find', {
         method: 'GET',
         query: {
-          filter: 'alert.attributes.tags: "hello" AND alert.attributes.tags: "world"',
+          filter: 'alert.attributes.tags:("hello" AND "world")',
           page: 1,
           per_page: 20,
           sort_field: 'updatedAt',
@@ -353,7 +383,7 @@ describe('Detections Rules API', () => {
         method: 'GET',
         query: {
           filter:
-            'alert.attributes.name: ruleName AND alert.attributes.tags: "__internal_immutable:false" AND alert.attributes.tags: "__internal_immutable:true" AND (alert.attributes.tags: "hello" AND alert.attributes.tags: "world")',
+            'alert.attributes.tags: "__internal_immutable:false" AND alert.attributes.tags: "__internal_immutable:true" AND alert.attributes.tags:("hello" AND "world") AND (alert.attributes.name: "ruleName" OR alert.attributes.params.index: "ruleName" OR alert.attributes.params.threat.tactic.id: "ruleName" OR alert.attributes.params.threat.tactic.name: "ruleName" OR alert.attributes.params.threat.technique.id: "ruleName" OR alert.attributes.params.threat.technique.name: "ruleName")',
           page: 1,
           per_page: 20,
           sort_field: 'enabled',

diff --git a/...ugins/security_solution/public/detections/containers/detection_engine/rules/utils.test.ts b/...ugins/security_solution/public/detections/containers/detection_engine/rules/utils.test.ts
@@ -26,7 +26,17 @@ describe('convertRulesFilterToKQL', () => {
   it('handles presence of "filter" properly', () => {
     const kql = convertRulesFilterToKQL({ ...filterOptions, filter: 'foo' });
 
-    expect(kql).toBe('alert.attributes.name: foo');
+    expect(kql).toBe(
+      '(alert.attributes.name: "foo" OR alert.attributes.params.index: "foo" OR alert.attributes.params.threat.tactic.id: "foo" OR alert.attributes.params.threat.tactic.name: "foo" OR alert.attributes.params.threat.technique.id: "foo" OR alert.attributes.params.threat.technique.name: "foo")'
+    );
+  });
+
+  it('escapes "filter" value properly', () => {
+    const kql = convertRulesFilterToKQL({ ...filterOptions, filter: '" OR (foo: bar)' });
+
+    expect(kql).toBe(
+      '(alert.attributes.name: "\\" OR (foo: bar)" OR alert.attributes.params.index: "\\" OR (foo: bar)" OR alert.attributes.params.threat.tactic.id: "\\" OR (foo: bar)" OR alert.attributes.params.threat.tactic.name: "\\" OR (foo: bar)" OR alert.attributes.params.threat.technique.id: "\\" OR (foo: bar)" OR alert.attributes.params.threat.technique.name: "\\" OR (foo: bar)")'
+    );
   });
 
   it('handles presence of "showCustomRules" properly', () => {
@@ -44,7 +54,7 @@ describe('convertRulesFilterToKQL', () => {
   it('handles presence of "tags" properly', () => {
     const kql = convertRulesFilterToKQL({ ...filterOptions, tags: ['tag1', 'tag2'] });
 
-    expect(kql).toBe('alert.attributes.tags: "tag1" AND alert.attributes.tags: "tag2"');
+    expect(kql).toBe('alert.attributes.tags:("tag1" AND "tag2")');
   });
 
   it('handles combination of different properties properly', () => {
@@ -56,7 +66,7 @@ describe('convertRulesFilterToKQL', () => {
     });
 
     expect(kql).toBe(
-      `alert.attributes.name: foo AND alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:true" AND (alert.attributes.tags: "tag1" AND alert.attributes.tags: "tag2")`
+      `alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:true" AND alert.attributes.tags:(\"tag1\" AND \"tag2\") AND (alert.attributes.name: \"foo\" OR alert.attributes.params.index: \"foo\" OR alert.attributes.params.threat.tactic.id: \"foo\" OR alert.attributes.params.threat.tactic.name: \"foo\" OR alert.attributes.params.threat.technique.id: \"foo\" OR alert.attributes.params.threat.technique.name: \"foo\")`
     );
   });
 });
diff --git a/...ck/plugins/security_solution/public/detections/containers/detection_engine/rules/utils.ts b/...ck/plugins/security_solution/public/detections/containers/detection_engine/rules/utils.ts
@@ -6,36 +6,54 @@
  */
 
 import { INTERNAL_IMMUTABLE_KEY } from '../../../../../common/constants';
+import { escapeKuery } from '../../../../common/lib/keury';
 import { FilterOptions } from './types';
 
+const SEARCHABLE_RULE_PARAMS = [
+  'alert.attributes.name',
+  'alert.attributes.params.index',
+  'alert.attributes.params.threat.tactic.id',
+  'alert.attributes.params.threat.tactic.name',
+  'alert.attributes.params.threat.technique.id',
+  'alert.attributes.params.threat.technique.name',
+];
+
 /**
  * Convert rules filter options object to KQL query
  *
  * @param filterOptions desired filters (e.g. filter/sortField/sortOrder)
  *
  * @returns KQL string
  */
-export const convertRulesFilterToKQL = (filterOptions: FilterOptions): string => {
-  const showCustomRuleFilter = filterOptions.showCustomRules
-    ? [`alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:false"`]
-    : [];
-  const showElasticRuleFilter = filterOptions.showElasticRules
-    ? [`alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:true"`]
-    : [];
-  const filtersWithoutTags = [
-    ...(filterOptions.filter.length ? [`alert.attributes.name: ${filterOptions.filter}`] : []),
-    ...showCustomRuleFilter,
-    ...showElasticRuleFilter,
-  ].join(' AND ');
-
-  const tags = filterOptions.tags
-    .map((t) => `alert.attributes.tags: "${t.replace(/"/g, '\\"')}"`)
-    .join(' AND ');
-
-  const filterString =
-    filtersWithoutTags !== '' && tags !== ''
-      ? `${filtersWithoutTags} AND (${tags})`
-      : filtersWithoutTags + tags;
-
-  return filterString;
+export const convertRulesFilterToKQL = ({
+  showCustomRules,
+  showElasticRules,
+  filter,
+  tags,
+}: FilterOptions): string => {
+  const filters: string[] = [];
+
+  if (showCustomRules) {
+    filters.push(`alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:false"`);
+  }
+
+  if (showElasticRules) {
+    filters.push(`alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:true"`);
+  }
+
+  if (tags.length > 0) {
+    filters.push(
+      `alert.attributes.tags:(${tags.map((tag) => `"${escapeKuery(tag)}"`).join(' AND ')})`
+    );
+  }
+
+  if (filter.length) {
+    const searchQuery = SEARCHABLE_RULE_PARAMS.map(
+      (param) => `${param}: "${escapeKuery(filter)}"`
+    ).join(' OR ');
+
+    filters.push(`(${searchQuery})`);
+  }
+
+  return filters.join(' AND ');
 };
diff --git a/...k/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.ts b/...k/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.ts
@@ -81,25 +81,6 @@ export const executeRulesBulkAction = async ({
   }
 };
 
-export const initRulesBulkAction = (params: Omit<ExecuteRulesBulkActionArgs, 'search'>) => {
-  const byQuery = (query: string) =>
-    executeRulesBulkAction({
-      ...params,
-      search: { query },
-    });
-
-  const byIds = (ids: string[]) =>
-    executeRulesBulkAction({
-      ...params,
-      search: { ids },
-    });
-
-  return {
-    byQuery,
-    byIds,
-  };
-};
-
 function defaultErrorHandler(toasts: UseAppToasts, action: BulkAction, error: HTTPError) {
   // if response doesn't have number of failed rules, it means the whole bulk action failed
   // and general error toast will be shown. Otherwise - error toast for partial failure

diff --git a/...tion/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx b/...tion/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx
@@ -30,7 +30,7 @@ import { canEditRuleWithActions } from '../../../../../../common/utils/privilege
 import { useRulesTableContext } from '../rules_table/rules_table_context';
 import * as detectionI18n from '../../../translations';
 import * as i18n from '../../translations';
-import { executeRulesBulkAction, initRulesBulkAction } from '../actions';
+import { executeRulesBulkAction } from '../actions';
 import { useHasActionsPrivileges } from '../use_has_actions_privileges';
 import { useHasMlPermissions } from '../use_has_ml_permissions';
 import { getCustomRulesCountFromCache } from './use_custom_rules_count';
@@ -239,26 +239,23 @@ export const useBulkActions = ({
           );
         }, 5 * 1000);
 
-        const rulesBulkAction = initRulesBulkAction({
+        await executeRulesBulkAction({
           visibleRuleIds: selectedRuleIds,
           action: BulkAction.edit,
           setLoadingRules,
           toasts,
           payload: { edit: [editPayload] },
           onFinish: () => hideWarningToast(),
+          search: isAllSelected
+            ? {
+                query: convertRulesFilterToKQL({
+                  ...filterOptions,
+                  showCustomRules: true, // only edit custom rules, as elastic rule are immutable
+                }),
+              }
+            : { ids: customSelectedRuleIds },
         });
 
-        // only edit custom rules, as elastic rule are immutable
-        if (isAllSelected) {
-          const customRulesOnlyFilterQuery = convertRulesFilterToKQL({
-            ...filterOptions,
-            showCustomRules: true,
-          });
-          await rulesBulkAction.byQuery(customRulesOnlyFilterQuery);
-        } else {
-          await rulesBulkAction.byIds(customSelectedRuleIds);
-        }
-
         isBulkEditFinished = true;
         invalidateRules();
         if (getIsMounted()) {