Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIEM] Add license check to ML Rule form #60691

Merged
merged 10 commits into from
Mar 23, 2020
Merged

[SIEM] Add license check to ML Rule form #60691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
02ff875
Gate ML Rules behind a license check
rylnd Mar 19, 2020
95ce642
Add aria-describedby for new ML input fields
rylnd Mar 19, 2020
42eaba7
Add data-test-subj to new ML input fields
rylnd Mar 19, 2020
004ae91
Remove unused prop
rylnd Mar 19, 2020
4621085
Fix capitalization on translation id
rylnd Mar 19, 2020
55df0d2
Merge branch 'master' into ml_rule_license_check
rylnd Mar 20, 2020
fac289c
Declare defaulted props as optional
rylnd Mar 21, 2020
1b6b460
Gray out entire ML card when ML Rules are disabled
rylnd Mar 21, 2020
be56dc3
Merge branch 'master' into ml_rule_license_check
rylnd Mar 21, 2020
41217c7
Merge branch 'master' into ml_rule_license_check
rylnd Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions ...ns/siem/public/pages/detection_engine/rules/components/anomaly_threshold_slider/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@ import { EuiFlexGrid, EuiFlexItem, EuiRange, EuiFormRow } from '@elastic/eui';
import { FieldHook } from '../../../../../shared_imports';

interface AnomalyThresholdSliderProps {
describedByIds: string[];
field: FieldHook;
}
type Event = React.ChangeEvent<HTMLInputElement>;
type EventArg = Event | React.MouseEvent<HTMLButtonElement>;

export const AnomalyThresholdSlider: React.FC<AnomalyThresholdSliderProps> = ({ field }) => {
export const AnomalyThresholdSlider: React.FC<AnomalyThresholdSliderProps> = ({
describedByIds = [],
field,
}) => {
const threshold = field.value as number;
const onThresholdChange = useCallback(
(event: EventArg) => {
Expand All @@ -26,7 +30,12 @@ export const AnomalyThresholdSlider: React.FC<AnomalyThresholdSliderProps> = ({
);

return (
<EuiFormRow label={field.label} fullWidth>
<EuiFormRow
fullWidth
label={field.label}
data-test-subj="anomalyThresholdSlider"
describedByIds={describedByIds}
>
<EuiFlexGrid columns={2}>
<EuiFlexItem>
<EuiRange
Expand Down
12 changes: 10 additions & 2 deletions ...egacy/plugins/siem/public/pages/detection_engine/rules/components/ml_job_select/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ const JobDisplay = ({ title, description }: { title: string; description: string
);

interface MlJobSelectProps {
describedByIds: string[];
field: FieldHook;
}

export const MlJobSelect: React.FC<MlJobSelectProps> = ({ field }) => {
export const MlJobSelect: React.FC<MlJobSelectProps> = ({ describedByIds = [], field }) => {
const jobId = field.value as string;
const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field);
const [isLoading, siemJobs] = useSiemJobs(false);
Expand All @@ -41,7 +42,14 @@ export const MlJobSelect: React.FC<MlJobSelectProps> = ({ field }) => {
}));

return (
<EuiFormRow fullWidth label={field.label} isInvalid={isInvalid} error={errorMessage}>
<EuiFormRow
fullWidth
label={field.label}
isInvalid={isInvalid}
error={errorMessage}
data-test-subj="mlJobSelect"
describedByIds={describedByIds}
>
<EuiFlexGroup>
<EuiFlexItem>
<EuiSuperSelect
Expand Down
60 changes: 52 additions & 8 deletions ...cy/plugins/siem/public/pages/detection_engine/rules/components/select_rule_type/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,58 @@
*/

import React, { useCallback } from 'react';
import { EuiCard, EuiFlexGrid, EuiFlexItem, EuiIcon, EuiFormRow } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import {
EuiCard,
EuiFlexGrid,
EuiFlexItem,
EuiFormRow,
EuiIcon,
EuiLink,
EuiText,
} from '@elastic/eui';

import { FieldHook } from '../../../../../shared_imports';
import { RuleType } from '../../../../../containers/detection_engine/rules/types';
import * as i18n from './translations';
import { isMlRule } from '../../helpers';

const MlCardDescription = ({ hasValidLicense = false }: { hasValidLicense?: boolean }) => (
<EuiText size="s">
{hasValidLicense ? (
i18n.ML_TYPE_DESCRIPTION
) : (
<FormattedMessage
id="xpack.siem.detectionEngine.createRule.stepDefineRule.ruleTypeField.mlTypeDisabledDescription"
defaultMessage="Access to ML requires a {subscriptionsLink}."
values={{
subscriptionsLink: (
<EuiLink href="https://www.elastic.co/subscriptions" target="_blank">
<FormattedMessage
id="xpack.siem.components.stepDefineRule.ruleTypeField.subscriptionsLink"
defaultMessage="Platinum subscription"
/>
</EuiLink>
),
}}
/>
)}
</EuiText>
);

interface SelectRuleTypeProps {
describedByIds?: string[];
field: FieldHook;
isReadOnly: boolean;
hasValidLicense?: boolean;
isReadOnly?: boolean;
}

export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({ field, isReadOnly = false }) => {
export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
describedByIds = [],
field,
hasValidLicense = false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have these two taking values when they are defaulted by the SelectRuleTypeProps shows they cannot have defaults and are required?

If they are required I would just drop the defaulting or change the types to be ? variants where they are defaults.

isReadOnly = false,
}) => {
const ruleType = field.value as RuleType;
const setType = useCallback(
(type: RuleType) => {
Expand All @@ -27,10 +66,15 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({ field, isReadOnl
);
const setMl = useCallback(() => setType('machine_learning'), [setType]);
const setQuery = useCallback(() => setType('query'), [setType]);
const license = true; // TODO
const mlCardDisabled = isReadOnly || !hasValidLicense;

return (
<EuiFormRow label={field.label} fullWidth>
<EuiFormRow
fullWidth
data-test-subj="selectRuleType"
describedByIds={describedByIds}
label={field.label}
>
<EuiFlexGrid columns={4}>
<EuiFlexItem>
<EuiCard
Expand All @@ -47,11 +91,11 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({ field, isReadOnl
<EuiFlexItem>
<EuiCard
title={i18n.ML_TYPE_TITLE}
description={license ? i18n.ML_TYPE_DESCRIPTION : i18n.ML_TYPE_DISABLED_DESCRIPTION}
isDisabled={!license}
description={<MlCardDescription hasValidLicense={hasValidLicense} />}
icon={<EuiIcon size="l" type="machineLearningApp" />}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I played with adding:

isDisabled={isReadOnly || !hasValidLicense}

To see what it looked like if the whole thing was diasbled.

What you have now:
Screen Shot 2020-03-20 at 4 59 43 PM

vs if the whole thing was disabled:
Screen Shot 2020-03-20 at 5 03 55 PM

Up to you and design on which is better but I prefer the whole thing being disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up going with the full-disabling, as it is more visually striking and a better CTA

isDisabled={mlCardDisabled}
selectable={{
isDisabled: isReadOnly,
isDisabled: mlCardDisabled,
onClick: setMl,
isSelected: isMlRule(ruleType),
}}
Expand Down
7 changes: 0 additions & 7 deletions ...gins/siem/public/pages/detection_engine/rules/components/select_rule_type/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,3 @@ export const ML_TYPE_DESCRIPTION = i18n.translate(
defaultMessage: 'Select ML job to detect anomalous activity.',
}
);

export const ML_TYPE_DISABLED_DESCRIPTION = i18n.translate(
'xpack.siem.detectionEngine.createRule.stepDefineRule.ruleTypeField.mlTypeDisabledDescription',
{
defaultMessage: 'Access to ML requires a Platinum subscription.',
}
);
23 changes: 19 additions & 4 deletions ...cy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import {
EuiButton,
} from '@elastic/eui';
import { isEmpty } from 'lodash/fp';
import React, { FC, memo, useCallback, useState, useEffect } from 'react';
import React, { FC, memo, useCallback, useState, useEffect, useContext } from 'react';
import styled from 'styled-components';
import deepEqual from 'fast-deep-equal';

import { IIndexPattern } from '../../../../../../../../../../src/plugins/data/public';
import { useFetchIndexPatterns } from '../../../../../containers/detection_engine/rules';
import { DEFAULT_INDEX_KEY } from '../../../../../../common/constants';
import { MlCapabilitiesContext } from '../../../../../components/ml/permissions/ml_capabilities_provider';
import { useUiSetting$ } from '../../../../../lib/kibana';
import { setFieldValue, isMlRule } from '../../helpers';
import * as RuleI18n from '../../translations';
Expand Down Expand Up @@ -103,6 +104,7 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
setForm,
setStepData,
}) => {
const mlCapabilities = useContext(MlCapabilitiesContext);
const [openTimelineSearch, setOpenTimelineSearch] = useState(false);
const [localUseIndicesConfig, setLocalUseIndicesConfig] = useState(false);
const [localIsMlRule, setIsMlRule] = useState(false);
Expand Down Expand Up @@ -182,6 +184,8 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
path="ruleType"
component={SelectRuleType}
componentProps={{
describedByIds: ['detectionEngineStepDefineRuleType'],
hasValidLicense: mlCapabilities.isPlatinumOrTrialLicense,
isReadOnly: isUpdateView,
}}
/>
Expand Down Expand Up @@ -220,7 +224,6 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
component={QueryBarDefineRule}
componentProps={{
browserFields,
loading: indexPatternLoadingQueryBar,
idAria: 'detectionEngineStepDefineRuleQueryBar',
indexPattern: indexPatternQueryBar,
isDisabled: isLoading,
Expand All @@ -234,8 +237,20 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
</EuiFormRow>
<EuiFormRow fullWidth style={{ display: localIsMlRule ? 'flex' : 'none' }}>
<>
<UseField path="machineLearningJobId" component={MlJobSelect} />
<UseField path="anomalyThreshold" component={AnomalyThresholdSlider} />
<UseField
path="machineLearningJobId"
component={MlJobSelect}
componentProps={{
describedByIds: ['detectionEngineStepDefineRulemachineLearningJobId'],
}}
/>
<UseField
path="anomalyThreshold"
component={AnomalyThresholdSlider}
componentProps={{
describedByIds: ['detectionEngineStepDefineRuleAnomalyThreshold'],
}}
/>
</>
</EuiFormRow>
<FormDataProvider pathsToWatch={['index', 'ruleType']}>
Expand Down