Skip to content

Commit

Permalink
[Security Solution] Fix exporting all rules (#152900)
Browse files Browse the repository at this point in the history
**Relates to:** elastic/security-team#5339, #150097, #150553

## Summary

This PR fixes all rules exporting functionality which started exporting unintentionally runtime fields like `execution_summary`. This way it lead to inability to import just exported rules if as minimum one of them executed just once.

On top of this the PR contains functional and Cypress tests to cover the fix.

## TODO

- [ ] get rid of `await waitForEventLogExecuteComplete()` in functional tests
- [ ] allow `getNewRule()` to rewrite its defaults

### Checklist

- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
maximpn authored Mar 10, 2023
1 parent 2fa877b commit 6b62ae2
Show file tree
Hide file tree
Showing 10 changed files with 234 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,25 @@
* 2.0.
*/

import { expectedExportedRule, getNewRule } from '../../objects/rule';
import path from 'path';

import { expectedExportedRule, getNewRule } from '../../objects/rule';
import {
TOASTER_BODY,
MODAL_CONFIRMATION_BODY,
MODAL_CONFIRMATION_BTN,
TOASTER,
} from '../../screens/alerts_detection_rules';

import {
exportFirstRule,
loadPrebuiltDetectionRulesFromHeaderBtn,
filterByElasticRules,
selectNumberOfRules,
bulkExportRules,
selectAllRules,
waitForRuleExecution,
exportRule,
importRules,
expectManagementTableRules,
} from '../../tasks/alerts_detection_rules';
import { createExceptionList, deleteExceptionList } from '../../tasks/api_calls/exceptions';
import { getExceptionList } from '../../objects/exception';
Expand All @@ -30,9 +34,12 @@ import { login, visitWithoutDateRange } from '../../tasks/login';
import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
import { getAvailablePrebuiltRulesCount } from '../../tasks/api_calls/prebuilt_rules';

const EXPORTED_RULES_FILENAME = 'rules_export.ndjson';
const exceptionList = getExceptionList();

describe('Export rules', () => {
const downloadsFolder = Cypress.config('downloadsFolder');

before(() => {
cleanKibana();
login();
Expand All @@ -45,17 +52,34 @@ describe('Export rules', () => {
// Rules get exported via _bulk_action endpoint
cy.intercept('POST', '/api/detection_engine/rules/_bulk_action').as('bulk_action');
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
createRule(getNewRule()).as('ruleResponse');
createRule({ ...getNewRule(), name: 'Rule to export' }).as('ruleResponse');
});

it('Exports a custom rule', function () {
exportFirstRule();
it('exports a custom rule', function () {
exportRule('Rule to export');
cy.wait('@bulk_action').then(({ response }) => {
cy.wrap(response?.body).should('eql', expectedExportedRule(this.ruleResponse));
cy.get(TOASTER_BODY).should('have.text', 'Successfully exported 1 of 1 rule.');
});
});

it('creates an importable file from executed rule', () => {
// Rule needs to be enabled to make sure it has been executed so rule's SO contains runtime fields like `execution_summary`
createRule({ ...getNewRule(), name: 'Enabled rule to export', enabled: true });
waitForRuleExecution('Enabled rule to export');

exportRule('Enabled rule to export');

cy.get(TOASTER).should('have.text', 'Rules exported');
cy.get(TOASTER_BODY).should('have.text', 'Successfully exported 1 of 1 rule.');

deleteAlertsAndRules();
importRules(path.join(downloadsFolder, EXPORTED_RULES_FILENAME));

cy.get(TOASTER).should('have.text', 'Successfully imported 1 rule');
expectManagementTableRules(['Enabled rule to export']);
});

it('shows a modal saying that no rules can be exported if all the selected rules are prebuilt', function () {
const expectedElasticRulesCount = 7;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
* 2.0.
*/

import { RULES_MANAGEMENT_TABLE, TOASTER } from '../../screens/alerts_detection_rules';
import { TOASTER } from '../../screens/alerts_detection_rules';
import {
expectNumberOfRules,
expectToContainRule,
expectManagementTableRules,
importRules,
importRulesWithOverwriteAll,
} from '../../tasks/alerts_detection_rules';
import { cleanKibana, deleteAlertsAndRules, reload } from '../../tasks/common';
import { login, visitWithoutDateRange } from '../../tasks/login';

import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
const RULES_TO_IMPORT_FILENAME = 'cypress/fixtures/7_16_rules.ndjson';

describe('Import rules', () => {
before(() => {
Expand All @@ -29,10 +29,7 @@ describe('Import rules', () => {
});

it('Imports a custom rule with exceptions', function () {
const expectedNumberOfRules = 1;
const expectedImportedRuleName = 'Test Custom Rule';

importRules('7_16_rules.ndjson');
importRules(RULES_TO_IMPORT_FILENAME);

cy.wait('@import').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
Expand All @@ -41,20 +38,19 @@ describe('Import rules', () => {
'Successfully imported 1 ruleSuccessfully imported 1 exception.'
);

expectNumberOfRules(RULES_MANAGEMENT_TABLE, expectedNumberOfRules);
expectToContainRule(RULES_MANAGEMENT_TABLE, expectedImportedRuleName);
expectManagementTableRules(['Test Custom Rule']);
});
});

it('Shows error toaster when trying to import rule and exception list that already exist', function () {
importRules('7_16_rules.ndjson');
importRules(RULES_TO_IMPORT_FILENAME);

cy.wait('@import').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
});

reload();
importRules('7_16_rules.ndjson');
importRules(RULES_TO_IMPORT_FILENAME);

cy.wait('@import').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
Expand All @@ -63,14 +59,14 @@ describe('Import rules', () => {
});

it('Does not show error toaster when trying to import rule and exception list that already exist when overwrite is true', function () {
importRules('7_16_rules.ndjson');
importRules(RULES_TO_IMPORT_FILENAME);

cy.wait('@import').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
});

reload();
importRulesWithOverwriteAll('7_16_rules.ndjson');
importRulesWithOverwriteAll(RULES_TO_IMPORT_FILENAME);

cy.wait('@import').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import {
} from '../../urls/navigation';
import { getNewRule } from '../../objects/rule';
import {
expectNumberOfRules,
expectToContainRule,
filterByCustomRules,
filterBySearchTerm,
filterByTags,
Expand All @@ -35,8 +33,8 @@ import {
filterByDisabledRules,
expectFilterByPrebuiltRules,
expectFilterByEnabledRules,
expectManagementTableRules,
} from '../../tasks/alerts_detection_rules';
import { RULES_MANAGEMENT_TABLE } from '../../screens/alerts_detection_rules';
import { createRule } from '../../tasks/api_calls/rules';
import {
expectRowsPerPage,
Expand Down Expand Up @@ -108,14 +106,6 @@ function expectDefaultRulesTableState(): void {
expectTablePage(1);
}

function expectManagementTableRules(ruleNames: string[]): void {
expectNumberOfRules(RULES_MANAGEMENT_TABLE, ruleNames.length);

for (const ruleName of ruleNames) {
expectToContainRule(RULES_MANAGEMENT_TABLE, ruleName);
}
}

describe('Persistent rules table state', () => {
before(() => {
cleanKibana();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export const RULE_CHECKBOX = '.euiTableRow .euiCheckbox__input';

export const RULE_NAME = '[data-test-subj="ruleName"]';

export const RULE_LAST_RUN = '[data-test-subj="ruleLastRun"]';

export const RULE_SWITCH = '[data-test-subj="ruleSwitch"]';

export const RULE_SWITCH_LOADER = '[data-test-subj="ruleSwitchLoader"]';
Expand Down Expand Up @@ -143,6 +145,8 @@ export const SELECTED_RULES_NUMBER_LABEL = '[data-test-subj="selectedRules"]';

export const REFRESH_SETTINGS_POPOVER = '[data-test-subj="refreshSettings-popover"]';

export const REFRESH_RULES_TABLE_BUTTON = '[data-test-subj="refreshRulesAction-linkIcon"]';

export const REFRESH_SETTINGS_SWITCH = '[data-test-subj="refreshSettingsSwitch"]';

export const REFRESH_SETTINGS_SELECTION_NOTE = '[data-test-subj="refreshSettingsSelectionNote"]';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ import {
RULES_MONITORING_TAB,
ENABLED_RULES_BTN,
DISABLED_RULES_BTN,
REFRESH_RULES_TABLE_BUTTON,
RULE_LAST_RUN,
} from '../screens/alerts_detection_rules';
import type { RULES_MONITORING_TABLE } from '../screens/alerts_detection_rules';
import { EUI_CHECKBOX } from '../screens/common/controls';
Expand Down Expand Up @@ -162,8 +164,10 @@ export const disableSelectedRules = () => {
cy.get(DISABLE_RULE_BULK_BTN).click();
};

export const exportFirstRule = () => {
cy.get(COLLAPSED_ACTION_BTN).first().click({ force: true });
export const exportRule = (name: string) => {
cy.log(`Export rule "${name}"`);

cy.contains(RULE_NAME, name).parents(RULES_ROW).find(COLLAPSED_ACTION_BTN).click();
cy.get(EXPORT_ACTION_BTN).click();
cy.get(EXPORT_ACTION_BTN).should('not.exist');
};
Expand All @@ -183,6 +187,19 @@ export const filterByTags = (tags: string[]) => {
}
};

export const waitForRuleExecution = (name: string) => {
cy.log(`Wait for rule "${name}" to be executed`);
cy.waitUntil(() => {
cy.get(REFRESH_RULES_TABLE_BUTTON).click();

return cy
.contains(RULE_NAME, name)
.parents(RULES_ROW)
.find(RULE_LAST_RUN)
.then(($el) => $el.text().endsWith('ago'));
});
};

export const filterByElasticRules = () => {
cy.get(ELASTIC_RULES_BTN).click();
waitForRulesTableToBeRefreshed();
Expand Down Expand Up @@ -358,7 +375,7 @@ export const checkAutoRefresh = (ms: number, condition: string) => {
export const importRules = (rulesFile: string) => {
cy.get(RULE_IMPORT_MODAL).click();
cy.get(INPUT_FILE).should('exist');
cy.get(INPUT_FILE).trigger('click', { force: true }).attachFile(rulesFile).trigger('change');
cy.get(INPUT_FILE).trigger('click', { force: true }).selectFile(rulesFile).trigger('change');
cy.get(RULE_IMPORT_MODAL_BUTTON).last().click({ force: true });
cy.get(INPUT_FILE).should('not.exist');
};
Expand Down Expand Up @@ -455,6 +472,14 @@ const selectOverwriteRulesImport = () => {
.should('be.checked');
};

export const expectManagementTableRules = (ruleNames: string[]): void => {
expectNumberOfRules(RULES_MANAGEMENT_TABLE, ruleNames.length);

for (const ruleName of ruleNames) {
expectToContainRule(RULES_MANAGEMENT_TABLE, ruleName);
}
};

const selectOverwriteExceptionsRulesImport = () => {
cy.get(RULE_IMPORT_OVERWRITE_EXCEPTIONS_CHECKBOX)
.pipe(($el) => $el.trigger('click'))
Expand All @@ -468,7 +493,7 @@ const selectOverwriteConnectorsRulesImport = () => {
export const importRulesWithOverwriteAll = (rulesFile: string) => {
cy.get(RULE_IMPORT_MODAL).click();
cy.get(INPUT_FILE).should('exist');
cy.get(INPUT_FILE).trigger('click', { force: true }).attachFile(rulesFile).trigger('change');
cy.get(INPUT_FILE).trigger('click', { force: true }).selectFile(rulesFile).trigger('change');
selectOverwriteRulesImport();
selectOverwriteExceptionsRulesImport();
selectOverwriteConnectorsRulesImport();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,15 +278,19 @@ export const useRulesColumns = ({
field: 'execution_summary.last_execution.date',
name: i18n.COLUMN_LAST_COMPLETE_RUN,
render: (value: RuleExecutionSummary['last_execution']['date'] | undefined) => {
return value == null ? (
getEmptyTagValue()
) : (
<FormattedRelativePreferenceDate
tooltipFieldName={i18n.COLUMN_LAST_COMPLETE_RUN}
relativeThresholdInHrs={DEFAULT_RELATIVE_DATE_THRESHOLD}
value={value}
tooltipAnchorClassName="eui-textTruncate"
/>
return (
<EuiFlexGroup data-test-subj="ruleLastRun">
{value == null ? (
getEmptyTagValue()
) : (
<FormattedRelativePreferenceDate
tooltipFieldName={i18n.COLUMN_LAST_COMPLETE_RUN}
relativeThresholdInHrs={DEFAULT_RELATIVE_DATE_THRESHOLD}
value={value}
tooltipAnchorClassName="eui-textTruncate"
/>
)}
</EuiFlexGroup>
);
},
sortable: true,
Expand Down Expand Up @@ -438,15 +442,19 @@ export const useMonitoringColumns = ({
field: 'execution_summary.last_execution.date',
name: i18n.COLUMN_LAST_COMPLETE_RUN,
render: (value: RuleExecutionSummary['last_execution']['date'] | undefined) => {
return value == null ? (
getEmptyTagValue()
) : (
<FormattedRelativePreferenceDate
tooltipFieldName={i18n.COLUMN_LAST_COMPLETE_RUN}
relativeThresholdInHrs={DEFAULT_RELATIVE_DATE_THRESHOLD}
value={value}
tooltipAnchorClassName="eui-textTruncate"
/>
return (
<EuiFlexGroup data-test-subj="ruleLastRun">
{value == null ? (
getEmptyTagValue()
) : (
<FormattedRelativePreferenceDate
tooltipFieldName={i18n.COLUMN_LAST_COMPLETE_RUN}
relativeThresholdInHrs={DEFAULT_RELATIVE_DATE_THRESHOLD}
value={value}
tooltipAnchorClassName="eui-textTruncate"
/>
)}
</EuiFlexGroup>
);
},
sortable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { ExceptionListClient } from '@kbn/lists-plugin/server';
import type { RulesClient, RuleExecutorServices } from '@kbn/alerting-plugin/server';
import { getNonPackagedRules } from '../search/get_existing_prepackaged_rules';
import { getExportDetailsNdjson } from './get_export_details_ndjson';
import { transformAlertsToRules } from '../../utils/utils';
import { transformAlertsToRules, transformRuleToExportableFormat } from '../../utils/utils';
import { getRuleExceptionsForExport } from './get_export_rule_exceptions';
import { getRuleActionConnectorsForExport } from './get_export_rule_action_connectors';

Expand Down Expand Up @@ -42,6 +42,7 @@ export const getExportAll = async (
logger,
});
const rules = transformAlertsToRules(ruleAlertTypes, legacyActions);
const exportRules = rules.map((r) => transformRuleToExportableFormat(r));

// Gather exceptions
const exceptions = rules.flatMap((rule) => rule.exceptions_list ?? []);
Expand All @@ -55,7 +56,7 @@ export const getExportAll = async (
request
);

const rulesNdjson = transformDataToNdjson(rules);
const rulesNdjson = transformDataToNdjson(exportRules);
const exportDetails = getExportDetailsNdjson(rules, [], exceptionDetails, actionConnectorDetails);

return { rulesNdjson, exportDetails, exceptionLists, actionConnectors };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { getExportDetailsNdjson } from './get_export_details_ndjson';

import { isAlertType } from '../../../rule_schema';
import { findRules } from '../search/find_rules';
import { transformRuleToExportableFormat } from '../../utils/utils';
import { getRuleExceptionsForExport } from './get_export_rule_exceptions';
import { getRuleActionConnectorsForExport } from './get_export_rule_action_connectors';

Expand Down Expand Up @@ -133,14 +134,11 @@ export const getRulesFromObjects = async (
isAlertType(matchingRule) &&
matchingRule.params.immutable !== true
) {
const rule = internalRuleToAPIResponse(matchingRule, legacyActions[matchingRule.id]);

// Fields containing runtime information shouldn't be exported. It causes import failures.
delete rule.execution_summary;

return {
statusCode: 200,
rule,
rule: transformRuleToExportableFormat(
internalRuleToAPIResponse(matchingRule, legacyActions[matchingRule.id])
),
};
} else {
return {
Expand Down
Loading

0 comments on commit 6b62ae2

Please sign in to comment.