Skip to content

Commit

Permalink
[SIEM] Fixes UX issues around prebuilt ML Rules (elastic#62396)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes a number of UX issues around the new prebuilt `machine_learning` rules when the user does not have the necessary permissions to manage the backing ML Job. Along with elastic#62383, this ensures there is adequate information for the user determine if a rule is not working because the backing job is not running (and helping to prevent this from occurring). This also includes some requested copy changes, including:

* Renames `Anomaly Detection`  dropdown to `ML job settings`
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320279-57c5a880-7526-11ea-8350-647cbba263a4.png" />
</p>

* Updates copy in `ML job settings` dropdown
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320473-cc98e280-7526-11ea-8871-e97661ff5f78.png" />
</p>

* Only shows `ML job settings` UI when on `/detections/` routes 
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320401-922f4580-7526-11ea-9f97-0ec06526b273.png" />
</p>


### All Rules Changes

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320892-d3742500-7527-11ea-90bb-91fd203480bd.png" />
</p>

* Adds warning toast when attempting to activate via bulk actions (if user does not have permission to enable/disable jobs)
<p align="center">
  <img width="300" src="https://user-images.githubusercontent.com/2946766/78321015-1a621a80-7528-11ea-8ab0-f9fef19240f7.png" />
</p>

### Rule Details Changes
* `Machine Learning job` link now links to ML App with table filtered to the relevant job

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321277-c277e380-7528-11ea-99e9-034970a5054e.png" />
</p>

### Create/Edit Rule Changes

* If the job selected _is not running_, a warning will be displayed to remind the user to enable the job before running the rule. cc @benskelker @MikePaquette -- this okay copy here?
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321498-63ff3500-7529-11ea-9b09-a87186cbe0ce.png" />
</p>

Resolves elastic/siem-team#575
Resolves elastic/siem-team#519

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials 
  - Scheduled time with @benskelker to update docs
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
  • Loading branch information
spong committed Apr 6, 2020
1 parent 34028fd commit d7ce199
Show file tree
Hide file tree
Showing 17 changed files with 305 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ import React from 'react';
import '../../mock/match_media';
import { HeaderGlobal } from './index';

jest.mock('react-router-dom', () => ({
useLocation: () => ({
pathname: '/app/siem#/hosts/allHosts',
hash: '',
search: '',
state: '',
}),
withRouter: () => jest.fn(),
}));

jest.mock('ui/new_platform');

// Test will fail because we will to need to mock some core services to make the test work
Expand All @@ -19,6 +29,10 @@ jest.mock('../search_bar', () => ({
}));

describe('HeaderGlobal', () => {
beforeEach(() => {
jest.resetAllMocks();
});

test('it renders', () => {
const wrapper = shallow(<HeaderGlobal />);

Expand Down
116 changes: 61 additions & 55 deletions x-pack/legacy/plugins/siem/public/components/header_global/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { pickBy } from 'lodash/fp';
import React from 'react';
import styled, { css } from 'styled-components';

import { useLocation } from 'react-router-dom';
import { gutterTimeline } from '../../lib/helpers';
import { navTabs } from '../../pages/home/home_navigations';
import { SiemPageName } from '../../pages/home/types';
Expand Down Expand Up @@ -36,63 +37,68 @@ FlexItem.displayName = 'FlexItem';
interface HeaderGlobalProps {
hideDetectionEngine?: boolean;
}
export const HeaderGlobal = React.memo<HeaderGlobalProps>(({ hideDetectionEngine = false }) => (
<Wrapper className="siemHeaderGlobal">
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween" wrap>
<WithSource sourceId="default">
{({ indicesExist }) => (
<>
<FlexItem>
<EuiFlexGroup alignItems="center" responsive={false}>
<FlexItem grow={false}>
<EuiLink href={getOverviewUrl()}>
<EuiIcon aria-label={i18n.SIEM} type="securityAnalyticsApp" size="l" />
</EuiLink>
</FlexItem>
export const HeaderGlobal = React.memo<HeaderGlobalProps>(({ hideDetectionEngine = false }) => {
const currentLocation = useLocation();

<FlexItem component="nav">
{indicesExistOrDataTemporarilyUnavailable(indicesExist) ? (
<SiemNavigation
display="condensed"
navTabs={
hideDetectionEngine
? pickBy((_, key) => key !== SiemPageName.detections, navTabs)
: navTabs
}
/>
) : (
<SiemNavigation
display="condensed"
navTabs={pickBy((_, key) => key === SiemPageName.overview, navTabs)}
/>
)}
</FlexItem>
</EuiFlexGroup>
</FlexItem>

<FlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="s" responsive={false} wrap>
{indicesExistOrDataTemporarilyUnavailable(indicesExist) && (
return (
<Wrapper className="siemHeaderGlobal">
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween" wrap>
<WithSource sourceId="default">
{({ indicesExist }) => (
<>
<FlexItem>
<EuiFlexGroup alignItems="center" responsive={false}>
<FlexItem grow={false}>
<MlPopover />
<EuiLink href={getOverviewUrl()}>
<EuiIcon aria-label={i18n.SIEM} type="securityAnalyticsApp" size="l" />
</EuiLink>
</FlexItem>

<FlexItem component="nav">
{indicesExistOrDataTemporarilyUnavailable(indicesExist) ? (
<SiemNavigation
display="condensed"
navTabs={
hideDetectionEngine
? pickBy((_, key) => key !== SiemPageName.detections, navTabs)
: navTabs
}
/>
) : (
<SiemNavigation
display="condensed"
navTabs={pickBy((_, key) => key === SiemPageName.overview, navTabs)}
/>
)}
</FlexItem>
)}
</EuiFlexGroup>
</FlexItem>

<FlexItem grow={false}>
<EuiButtonEmpty
data-test-subj="add-data"
href="kibana#home/tutorial_directory/siem"
iconType="plusInCircle"
>
{i18n.BUTTON_ADD_DATA}
</EuiButtonEmpty>
</FlexItem>
</EuiFlexGroup>
</FlexItem>
</>
)}
</WithSource>
</EuiFlexGroup>
</Wrapper>
));
<FlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="s" responsive={false} wrap>
{indicesExistOrDataTemporarilyUnavailable(indicesExist) &&
currentLocation.pathname.includes(`/${SiemPageName.detections}/`) && (
<FlexItem grow={false}>
<MlPopover />
</FlexItem>
)}

<FlexItem grow={false}>
<EuiButtonEmpty
data-test-subj="add-data"
href="kibana#home/tutorial_directory/siem"
iconType="plusInCircle"
>
{i18n.BUTTON_ADD_DATA}
</EuiButtonEmpty>
</FlexItem>
</EuiFlexGroup>
</FlexItem>
</>
)}
</WithSource>
</EuiFlexGroup>
</Wrapper>
);
});
HeaderGlobal.displayName = 'HeaderGlobal';

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export const MlPopover = React.memo(() => {
iconSide="right"
onClick={() => setIsPopoverOpen(!isPopoverOpen)}
>
{i18n.ANOMALY_DETECTION}
{i18n.ML_JOB_SETTINGS}
</EuiButtonEmpty>
}
isOpen={isPopoverOpen}
Expand All @@ -142,14 +142,14 @@ export const MlPopover = React.memo(() => {
dispatch({ type: 'refresh' });
}}
>
{i18n.ANOMALY_DETECTION}
{i18n.ML_JOB_SETTINGS}
</EuiButtonEmpty>
}
isOpen={isPopoverOpen}
closePopover={() => setIsPopoverOpen(!isPopoverOpen)}
>
<PopoverContentsDiv data-test-subj="ml-popover-contents">
<EuiPopoverTitle>{i18n.ANOMALY_DETECTION_TITLE}</EuiPopoverTitle>
<EuiPopoverTitle>{i18n.ML_JOB_SETTINGS}</EuiPopoverTitle>
<PopoverDescription />

<EuiSpacer />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const PopoverDescriptionComponent = () => (
<EuiText size="s">
<FormattedMessage
id="xpack.siem.components.mlPopup.anomalyDetectionDescription"
defaultMessage="Run any of the Machine Learning jobs below to view anomalous events throughout the SIEM application. We’ve provided a few common detection jobs to get you started. If you wish to add your own custom jobs, simply create and tag them with “SIEM” from the {machineLearning} application for inclusion here."
defaultMessage="Run any of the Machine Learning jobs below to prepare for creating signal detection rules that produce signals for detected anomalies, and to view anomalous events throughout the SIEM application. We’ve provided a collection of common detection jobs to get you started. If you wish to add your own custom ML jobs, create and add them to the “SIEM” group from the {machineLearning} application."
values={{
machineLearning: (
<EuiLink href={`${useBasePath()}/app/ml`} target="_blank">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,10 @@

import { i18n } from '@kbn/i18n';

export const ANOMALY_DETECTION = i18n.translate(
'xpack.siem.components.mlPopup.anomalyDetectionButtonLabel',
export const ML_JOB_SETTINGS = i18n.translate(
'xpack.siem.components.mlPopup.mlJobSettingsButtonLabel',
{
defaultMessage: 'Anomaly detection',
}
);

export const ANOMALY_DETECTION_TITLE = i18n.translate(
'xpack.siem.components.mlPopup.anomalyDetectionTitle',
{
defaultMessage: 'Anomaly detection settings',
defaultMessage: 'ML job settings',
}
);

Expand Down
24 changes: 24 additions & 0 deletions x-pack/legacy/plugins/siem/public/components/toasters/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,30 @@ export const displayErrorToast = (
});
};

/**
* Displays a warning toast for the provided title and message
*
* @param title warning message to display in toaster and modal
* @param dispatchToaster provided by useStateToaster()
* @param id unique ID if necessary
*/
export const displayWarningToast = (
title: string,
dispatchToaster: React.Dispatch<ActionToaster>,
id: string = uuid.v4()
): void => {
const toast: AppToast = {
id,
title,
color: 'warning',
iconType: 'help',
};
dispatchToaster({
type: 'addToaster',
toast,
});
};

/**
* Displays a success toast for the provided title and message
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import {
enableRulesAction,
exportRulesAction,
} from './actions';
import { ActionToaster } from '../../../../components/toasters';
import { ActionToaster, displayWarningToast } from '../../../../components/toasters';
import { Rule } from '../../../../containers/detection_engine/rules';
import * as detectionI18n from '../../translations';

interface GetBatchItems {
closePopover: () => void;
dispatch: Dispatch<Action>;
dispatchToaster: Dispatch<ActionToaster>;
hasMlPermissions: boolean;
loadingRuleIds: string[];
reFetchRules: (refreshPrePackagedRule?: boolean) => void;
rules: Rule[];
Expand All @@ -31,6 +33,7 @@ export const getBatchItems = ({
closePopover,
dispatch,
dispatchToaster,
hasMlPermissions,
loadingRuleIds,
reFetchRules,
rules,
Expand All @@ -57,7 +60,22 @@ export const getBatchItems = ({
const deactivatedIds = selectedRuleIds.filter(
id => !rules.find(r => r.id === id)?.enabled ?? false
);
await enableRulesAction(deactivatedIds, true, dispatch, dispatchToaster);

const deactivatedIdsNoML = deactivatedIds.filter(
id => rules.find(r => r.id === id)?.type !== 'machine_learning' ?? false
);

const mlRuleCount = deactivatedIds.length - deactivatedIdsNoML.length;
if (!hasMlPermissions && mlRuleCount > 0) {
displayWarningToast(detectionI18n.ML_RULES_UNAVAILABLE(mlRuleCount), dispatchToaster);
}

await enableRulesAction(
hasMlPermissions ? deactivatedIds : deactivatedIdsNoML,
true,
dispatch,
dispatchToaster
);
}}
>
{i18n.BATCH_ACTION_ACTIVATE_SELECTED}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
EuiTableActionsColumnType,
EuiText,
EuiHealth,
EuiToolTip,
} from '@elastic/eui';
import { FormattedRelative } from '@kbn/i18n/react';
import * as H from 'history';
Expand All @@ -36,6 +37,8 @@ import {
} from './actions';
import { Action } from './reducer';
import { LocalizedDateTooltip } from '../../../../components/localized_date_tooltip';
import * as detectionI18n from '../../translations';
import { isMlRule } from '../../../../../common/detection_engine/ml_helpers';

export const getActions = (
dispatch: React.Dispatch<Action>,
Expand Down Expand Up @@ -92,6 +95,7 @@ interface GetColumns {
dispatch: React.Dispatch<Action>;
dispatchToaster: Dispatch<ActionToaster>;
history: H.History;
hasMlPermissions: boolean;
hasNoPermissions: boolean;
loadingRuleIds: string[];
reFetchRules: (refreshPrePackagedRule?: boolean) => void;
Expand All @@ -102,6 +106,7 @@ export const getColumns = ({
dispatch,
dispatchToaster,
history,
hasMlPermissions,
hasNoPermissions,
loadingRuleIds,
reFetchRules,
Expand Down Expand Up @@ -186,14 +191,25 @@ export const getColumns = ({
field: 'enabled',
name: i18n.COLUMN_ACTIVATE,
render: (value: Rule['enabled'], item: Rule) => (
<RuleSwitch
data-test-subj="enabled"
dispatch={dispatch}
id={item.id}
enabled={item.enabled}
isDisabled={hasNoPermissions}
isLoading={loadingRuleIds.includes(item.id)}
/>
<EuiToolTip
position="top"
content={
isMlRule(item.type) && !hasMlPermissions
? detectionI18n.ML_RULES_DISABLED_MESSAGE
: undefined
}
>
<RuleSwitch
data-test-subj="enabled"
dispatch={dispatch}
id={item.id}
enabled={item.enabled}
isDisabled={
hasNoPermissions || (isMlRule(item.type) && !hasMlPermissions && !item.enabled)
}
isLoading={loadingRuleIds.includes(item.id)}
/>
</EuiToolTip>
),
sortable: true,
width: '95px',
Expand Down
Loading

0 comments on commit d7ce199

Please sign in to comment.