Skip to content

Commit

Permalink
[Security Solution] New Rules Installation and Upgrade UI Workflows (#…
Browse files Browse the repository at this point in the history
…158450)

Addresses: #154614
#154615

Figma designs:
https://www.figma.com/file/gLHm8LpTtSkAUQHrkG3RHU/%5B8.7%5D-%5BRules%5D-Rule-Immutability%2FCustomization?type=design&node-id=2935-577576&t=ziqgnlEJBpowqa7F-0

## Summary

- Removes `prebuiltRulesNewUpgradeAndInstallationWorkflowsEnabled`
feature flag. All new prebuilt endpoints now available by default.
- Creates the UI for the new **rules installation** and **rules
upgrade** workflows.
- Creates new **Add Rules** page, which lists rules available for
installation.
- Creates new **Rule Updates** page, which lists rules which have
available updates.
- Creates new, separate contexts for the **Add Rules** and the **Rule
Updates** page, and the hooks to use them
(`useAddPrebuiltRulesTableContext` and
`useUpgradePrebuiltRulesTableContext` respectively)
    - Creates prebuilt rule hooks, which consume new endpoints:
- `useFetchPrebuiltRulesStatusQuery` and `usePrebuiltRulesStatus`
consume the `/internal/detection_engine/prebuilt_rules/status` endpoint
and provide information about number of rules available for
installation, number of installed rules, and number of rules with
available updates.
- `useFetchPrebuiltRulesInstallReviewQuery` and
`usePrebuiltRulesInstallReview` consume the
`/internal/detection_engine/prebuilt_rules/installation/_review`
endpoint and return the rules available for installation which are
listed in the **Add Rules** page.
- `useFetchPrebuiltRulesUpgradeReviewQuery` and
`usePrebuiltRulesUpgradeReview` consume the
`/internal/detection_engine/prebuilt_rules/upgrade/_review` endpoint and
return the rules which have available updates, and are listed in the
**Rule Updates** page.
- `usePerformInstallAllRules`, `usePerformInstallSpecificRules`, and its
respective mutation hooks `usePerformAllRulesInstallMutation` and
`usePerformSpecificRulesInstallMutation` consume the
`/internal/detection_engine/prebuilt_rules/upgrade/_perform` endpoint in
order to install rules.
- `usePerformUpgradeAllRules`, `usePerformUpgradeSpecificRules` and its
respective mutation hooks `usePerformAllRulesUpgradeMutation` and
`usePerformSpecificRulesUpgradeMutation` consume the
`/internal/detection_engine/prebuilt_rules/upgrade/_perform` endpoint in
order to upgrade rules.

### Deprecated code

**Hooks:**
- `useCreatePrebuiltRulesMutation`
- `useInstallPrePackagedRules`
- `useCreatePrePackagedRules`
- `usePrePackagedRulesInstallationStatus`
- `usePrePackagedTimelinesInstallationStatus`

### Major points to resolve

- **Timeline templates installation**: Since this PR stops using the
`/api/detection_engine/rules/prepackaged` endpoint in favour of the new
ones, we are not currently installing timeline templates. Serverside, we
will need a new endpoint to install them separately from rules? In the
UI, how would this still work: would they get installed in the
background now? Or maybe have a new button for it somewhere?
- **ML Jobs warning**: when updating rules, we currently have a wrapper
to add confirmation modal for users who may be running older ML Jobs
that would be overridden by updating their rules. This PR removes that
code, but we'll need to reintroduce it for the cases of: upgrading
single rules, upgrading a selection of rules, upgrading all rules.


### Deviations from design

This PR includes a reduced scope to the final workflow shown in the
Figma designs.

Most notably, in Milestone 2, to be released in 8.9, we did not build
the flyout that, in the Add Rules page, shows the rule details when the
user clicks on it, so the user can review it before installing. The same
is true in the Rule Updates table, which does not allow, for now,
reviewing the rules. In both cases, the user can only click in "Install
Rule" and "Upgrade Rule".

There are other differences in the UI, for technical reasons:
- Both for the Add Rules page and the Rule Updates table we decided to
use **EUI's InMemoryTable**. Since the endpoint that return the data to
populate both of these tables do not allow for sorting, filtering and
paging, we decided to use the InMemoryTable for both cases, as all of
those functions are handled out-of-the-box by the EUI component. The
relatively low number of items that populate these tables means that we
won't face significant performance issues. However, this meant a number
of deviations from the designs:
- Since filtering, sorting and pagination are handled by the table, the
contexts for these table do not includes any internal state relating to
these functions. This makes it hard to recreate the RuleUtilityBar for
each of these components or make the existing one reusable. We therefore
decided to leave the Utility Bar for the new two tables out of scope,
and deviate from the design by moving the button that the user can click
on o install or upgrade the selected rules to beside the "Install all"
or "Upgrade all" buttons. This button is shown only when at least one
rule of the table is selected.
- The **tags filter box** that comes out-of-the-box with the
InMemoryTable can only be positioned to the right of the search bar,
instead of the left like we have in our main **Installed Rules** table.
Also, clicking on the tabs adds the text to the search bar, and the box
does not allow for negative selection of tabs (exclusion).
- The search bar filters on keystroke rather than on Enter. This
behaviour can be changed, but it feels more useful than the other
behaviour for these new two tables.
- The search bar filters by searching the user's input in any of the
string properties of first order within the rule object. This means that
the search bar can be used to look up rules according to their name,
description, rule_id, etc (but not for example for MITRE techniques,
which are an object.) This behaviour, however, is also customisable.
- Neither the Add Rules table nor the Rule Updates tables display the
_Last updated_ column which is shown in the design. Since the original
intent of the designers is to show when the rule asset (`security-rule`)
was created or updated, this is information we don't currently have
within the SO. After discussion with @ksevasilyeva and @ARWNightingale,
we decided, for now, to remove the column. In the meantime,
@terrancedejesus [created an issue to include `createdAt` and
`updatedAt`
fields](elastic/detection-rules#2826) within
the rule assets, that we can use to display in the table in later
iterations.

#### Other remaining work:

- Introduce confirmation modals when the user clicks on the "Install
all" or "Upgrade all" modal.
- Unit testing for new hooks and components.
- Other component redesign: Rule Filter, Tag Filter 

#### How to test rule upgrade

1. Have at least one rule installed
2. Find its `rule_id` from the Network tab.
3. Make a request to `PATCH /api/detection_engine/rules` with the
`rule_id` in the payload, and also set the `version` to a number lower
than the current version.
4. Reload the page.
5. The `/upgrade/_review` endpoint will now return that rule as
available for upgrade.

### Videos

#### Rule Installation Workflow



https://github.com/elastic/kibana/assets/5354282/5a219625-beb1-48ee-a9fc-ff48b69eeae0

#### Rule Upgrade Workflow



https://github.com/elastic/kibana/assets/5354282/b5f3c23b-004a-462c-bbdd-ed04321f5ce7

### TODO

- [x] Align copy, use "update" instead of "upgrade"
- [ ] Persist user's choice when they dismiss the upgrade/install rules
callouts till the next package release (create a separate task for that)
- [ ] Unify table controls (search bar and tags), use the ones we have
on the rules management table
- [ ] After rule installation, adjust copy, and display that all
available rules have been installed. Add a "Go Back" CTA
- [ ] Add links from the available rules table to docs
- [ ] Rule severity sorting should take semantics into consideration

---------

Co-authored-by: Dmitrii <[email protected]>
Co-authored-by: Dmitrii Shevchenko <[email protected]>
Co-authored-by: Sergi Massaneda <[email protected]>
  • Loading branch information
4 people authored Jun 14, 2023
1 parent 56e9ceb commit 17b5fd3
Show file tree
Hide file tree
Showing 95 changed files with 2,830 additions and 1,608 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export enum SecurityPageName {
policies = 'policy',
responseActionsHistory = 'response_actions_history',
rules = 'rules',
rulesAdd = 'rules-add',
rulesCreate = 'rules-create',
sessions = 'sessions',
/*
Expand Down Expand Up @@ -158,6 +159,7 @@ export const DETECTIONS_PATH = '/detections' as const;
export const ALERTS_PATH = '/alerts' as const;
export const ALERT_DETAILS_REDIRECT_PATH = `${ALERTS_PATH}/redirect` as const;
export const RULES_PATH = '/rules' as const;
export const RULES_ADD_PATH = `${RULES_PATH}/add_rules` as const;
export const RULES_CREATE_PATH = `${RULES_PATH}/create` as const;
export const EXCEPTIONS_PATH = '/exceptions' as const;
export const EXCEPTION_LIST_DETAIL_PATH = `${EXCEPTIONS_PATH}/details/:detailName` as const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ export const RuleVersionSpecifier = t.exact(
);
export type RuleVersionSpecifier = t.TypeOf<typeof RuleVersionSpecifier>;

export type InstallSpecificRulesRequest = t.TypeOf<typeof InstallSpecificRulesRequest>;

export const InstallSpecificRulesRequest = t.exact(
t.type({
mode: t.literal(`SPECIFIC_RULES`),
rules: t.array(RuleVersionSpecifier),
})
);

export type InstallAllRulesRequest = t.TypeOf<typeof InstallAllRulesRequest>;

export const InstallAllRulesRequest = t.exact(
t.type({
mode: t.literal(`ALL_RULES`),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ export const RuleUpgradeSpecifier = t.exact(
rule_id: t.string,
/**
* This parameter is needed for handling race conditions with Optimistic Concurrency Control.
* Two or more users can call installation/_review and installation/_perform endpoints concurrently.
* Two or more users can call upgrade/_review and upgrade/_perform endpoints concurrently.
* Also, in general the time between these two calls can be anything.
* The idea is to only allow the user to install a rule if the user has reviewed the exact version
* of it that had been returned from the _review endpoint. If the version changed on the BE,
* installation/_perform endpoint will return a version mismatch error for this rule.
* upgrade/_perform endpoint will return a version mismatch error for this rule.
*/
revision: t.number,
/**
Expand All @@ -41,6 +41,7 @@ export const RuleUpgradeSpecifier = t.exact(
);
export type RuleUpgradeSpecifier = t.TypeOf<typeof RuleUpgradeSpecifier>;

export type UpgradeSpecificRulesRequest = t.TypeOf<typeof UpgradeSpecificRulesRequest>;
export const UpgradeSpecificRulesRequest = t.exact(
t.intersection([
t.type({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ export interface RuleUpgradeInfoForReview {
rule_id: RuleSignatureId;
rule: DiffableRule;
diff: PartialRuleDiff;
revision: number;
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ export const allowedExperimentalValues = Object.freeze({
*/
extendedRuleExecutionLoggingEnabled: false,

/**
* Enables the new API and UI for https://github.com/elastic/security-team/issues/1974.
* It's a temporary feature flag that will be removed once the feature gets a basic production-ready implementation.
*/
prebuiltRulesNewUpgradeAndInstallationWorkflowsEnabled: false,

/**
* Enables the SOC trends timerange and stats on D&R page
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
testAllTagsBadges,
testTagsBadge,
testMultipleSelectedRulesLabel,
loadPrebuiltDetectionRulesFromHeaderBtn,
filterByElasticRules,
clickErrorToastBtn,
unselectRuleByName,
Expand Down Expand Up @@ -90,7 +89,10 @@ import {
} from '../../objects/rule';

import { esArchiverResetKibana } from '../../tasks/es_archiver';
import { getAvailablePrebuiltRulesCount } from '../../tasks/api_calls/prebuilt_rules';
import {
getAvailablePrebuiltRulesCount,
excessivelyInstallAllPrebuiltRules,
} from '../../tasks/api_calls/prebuilt_rules';
import { setRowsPerPageTo } from '../../tasks/table_pagination';

const RULE_NAME = 'Custom rule for bulk actions';
Expand Down Expand Up @@ -151,7 +153,7 @@ describe('Detection rules, bulk edit', () => {
it('Only prebuilt rules selected', () => {
const expectedNumberOfSelectedRules = 10;

loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

// select Elastic(prebuilt) rules, check if we can't proceed further, as Elastic rules are not editable
filterByElasticRules();
Expand All @@ -167,7 +169,7 @@ describe('Detection rules, bulk edit', () => {
});

it('Prebuilt and custom rules selected: user proceeds with custom rules editing', () => {
loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

// modal window should show how many rules can be edit, how many not
selectAllRules();
Expand All @@ -190,7 +192,7 @@ describe('Detection rules, bulk edit', () => {
});

it('Prebuilt and custom rules selected: user cancels action', () => {
loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

// modal window should show how many rules can be edit, how many not
selectAllRules();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {
import {
waitForRulesTableToBeLoaded,
selectNumberOfRules,
loadPrebuiltDetectionRulesFromHeaderBtn,
goToEditRuleActionsSettingsOf,
} from '../../tasks/alerts_detection_rules';
import {
Expand All @@ -58,6 +57,7 @@ import {
getMachineLearningRule,
getNewTermsRule,
} from '../../objects/rule';
import { excessivelyInstallAllPrebuiltRules } from '../../tasks/api_calls/prebuilt_rules';

const ruleNameToAssert = 'Custom rule name with actions';
const expectedNumberOfCustomRulesToBeEdited = 7;
Expand Down Expand Up @@ -136,7 +136,7 @@ describe.skip('Detection rules, bulk edit of rule actions', () => {
throttleUnit: 'd',
};

loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

// select both custom and prebuilt rules
selectNumberOfRules(expectedNumberOfRulesToBeEdited);
Expand Down Expand Up @@ -164,7 +164,7 @@ describe.skip('Detection rules, bulk edit of rule actions', () => {
});

it('Overwrite rule actions in rules', () => {
loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

// select both custom and prebuilt rules
selectNumberOfRules(expectedNumberOfRulesToBeEdited);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
TOASTER,
} from '../../screens/alerts_detection_rules';
import {
loadPrebuiltDetectionRulesFromHeaderBtn,
filterByElasticRules,
selectNumberOfRules,
bulkExportRules,
Expand All @@ -32,7 +31,10 @@ import { cleanKibana, resetRulesTableState, deleteAlertsAndRules } from '../../t
import { login, visitWithoutDateRange } from '../../tasks/login';

import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
import { getAvailablePrebuiltRulesCount } from '../../tasks/api_calls/prebuilt_rules';
import {
excessivelyInstallAllPrebuiltRules,
getAvailablePrebuiltRulesCount,
} from '../../tasks/api_calls/prebuilt_rules';

const EXPORTED_RULES_FILENAME = 'rules_export.ndjson';
const exceptionList = getExceptionList();
Expand Down Expand Up @@ -83,7 +85,7 @@ describe('Export rules', () => {
it('shows a modal saying that no rules can be exported if all the selected rules are prebuilt', function () {
const expectedElasticRulesCount = 7;

loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

filterByElasticRules();
selectNumberOfRules(expectedElasticRulesCount);
Expand All @@ -97,7 +99,7 @@ describe('Export rules', () => {
it('exports only custom rules', function () {
const expectedNumberCustomRulesToBeExported = 1;

loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

selectAllRules();
bulkExportRules();
Expand Down Expand Up @@ -149,7 +151,7 @@ describe('Export rules', () => {
// one rule with exception, one without it
const expectedNumberCustomRulesToBeExported = 2;

loadPrebuiltDetectionRulesFromHeaderBtn();
excessivelyInstallAllPrebuiltRules();

selectAllRules();
bulkExportRules();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,23 @@ import {
RULES_MANAGEMENT_TABLE,
RULE_SWITCH,
SELECT_ALL_RULES_ON_PAGE_CHECKBOX,
INSTALL_ALL_RULES_BUTTON,
} from '../../screens/alerts_detection_rules';
import {
confirmRulesDelete,
deleteFirstRule,
deleteSelectedRules,
disableSelectedRules,
enableSelectedRules,
loadPrebuiltDetectionRules,
reloadDeletedRules,
selectAllRules,
selectNumberOfRules,
waitForPrebuiltDetectionRulesToBeLoaded,
waitForRuleToUpdate,
} from '../../tasks/alerts_detection_rules';
import { getAvailablePrebuiltRulesCount } from '../../tasks/api_calls/prebuilt_rules';
import {
excessivelyInstallAllPrebuiltRules,
getAvailablePrebuiltRulesCount,
} from '../../tasks/api_calls/prebuilt_rules';
import { cleanKibana, deleteAlertsAndRules } from '../../tasks/common';
import { login, visitWithoutDateRange } from '../../tasks/login';
import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
Expand All @@ -43,7 +45,8 @@ describe('Prebuilt rules', () => {
login();
deleteAlertsAndRules();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
loadPrebuiltDetectionRules();
excessivelyInstallAllPrebuiltRules();
cy.reload();
waitForPrebuiltDetectionRulesToBeLoaded();
});

Expand Down Expand Up @@ -111,15 +114,19 @@ describe('Prebuilt rules', () => {
'have.text',
`Elastic rules (${expectedNumberOfRulesAfterDeletion})`
);
cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should('exist');
cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should(
'have.text',
'Install 1 Elastic prebuilt rule '
);
cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should('have.text', `Add Elastic rules1`);

reloadDeletedRules();
// Navigate to the prebuilt rule installation page
cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).click();

cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should('not.exist');
// Click the "Install all rules" button
cy.get(INSTALL_ALL_RULES_BUTTON).click();

// Wait for the rules to be installed
cy.get(INSTALL_ALL_RULES_BUTTON).should('be.disabled');

// Navigate back to the rules page
cy.go('back');

cy.get(ELASTIC_RULES_BTN).should(
'have.text',
Expand All @@ -137,20 +144,28 @@ describe('Prebuilt rules', () => {
selectNumberOfRules(numberOfRulesToBeSelected);
deleteSelectedRules();

cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should('exist');
cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should(
'have.text',
`Install ${numberOfRulesToBeSelected} Elastic prebuilt rules `
`Add Elastic rules${numberOfRulesToBeSelected}`
);
cy.get(ELASTIC_RULES_BTN).should(
'have.text',
`Elastic rules (${expectedNumberOfRulesAfterDeletion})`
);

reloadDeletedRules();
// Navigate to the prebuilt rule installation page
cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).click();

// Click the "Install all rules" button
cy.get(INSTALL_ALL_RULES_BUTTON).click();

// Wait for the rules to be installed
cy.get(INSTALL_ALL_RULES_BUTTON).should('be.disabled');

cy.get(LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN).should('not.exist');
// Navigate back to the rules page
cy.go('back');

// Check that the rules table contains all rules
cy.get(ELASTIC_RULES_BTN).should(
'have.text',
`Elastic rules (${expectedNumberOfRulesAfterRecovering})`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import {
SELECT_ALL_RULES_ON_PAGE_CHECKBOX,
} from '../../screens/alerts_detection_rules';
import {
loadPrebuiltDetectionRules,
selectNumberOfRules,
unselectNumberOfRules,
waitForPrebuiltDetectionRulesToBeLoaded,
} from '../../tasks/alerts_detection_rules';
import { getAvailablePrebuiltRulesCount } from '../../tasks/api_calls/prebuilt_rules';
import {
excessivelyInstallAllPrebuiltRules,
getAvailablePrebuiltRulesCount,
} from '../../tasks/api_calls/prebuilt_rules';
import { cleanKibana } from '../../tasks/common';
import { login, visitWithoutDateRange } from '../../tasks/login';
import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
Expand All @@ -29,7 +31,7 @@ describe.skip('Rules selection', () => {
});

it('should correctly update the selection label when rules are individually selected and unselected', () => {
loadPrebuiltDetectionRules();
excessivelyInstallAllPrebuiltRules();
waitForPrebuiltDetectionRulesToBeLoaded();

selectNumberOfRules(2);
Expand All @@ -42,7 +44,7 @@ describe.skip('Rules selection', () => {
});

it('should correctly update the selection label when rules are bulk selected and then bulk un-selected', () => {
loadPrebuiltDetectionRules();
excessivelyInstallAllPrebuiltRules();
waitForPrebuiltDetectionRulesToBeLoaded();

cy.get(SELECT_ALL_RULES_BTN).click();
Expand All @@ -63,7 +65,7 @@ describe.skip('Rules selection', () => {
});

it('should correctly update the selection label when rules are bulk selected and then unselected via the table select all checkbox', () => {
loadPrebuiltDetectionRules();
excessivelyInstallAllPrebuiltRules();
waitForPrebuiltDetectionRulesToBeLoaded();

cy.get(SELECT_ALL_RULES_BTN).click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export const LOAD_PREBUILT_RULES_BTN = '[data-test-subj="load-prebuilt-rules"]';

export const LOAD_PREBUILT_RULES_ON_PAGE_HEADER_BTN = '[data-test-subj="loadPrebuiltRulesBtn"]';

export const INSTALL_ALL_RULES_BUTTON = '[data-test-subj="installAllRulesButton"]';

export const RULES_TABLE_INITIAL_LOADING_INDICATOR =
'[data-test-subj="initialLoadingPanelAllRulesTable"]';

Expand Down
Loading

0 comments on commit 17b5fd3

Please sign in to comment.