Skip to content

Commit

Permalink
[Security Solution][Detections] Rule Form: prevent creation of invali…
Browse files Browse the repository at this point in the history
…d scheduling parameters (#79577)

* Clean up our component types

* Clamp our Rule Schedule inputs to safe values

* If the user enters a value > Number.MAX_SAFE_INTEGER, it will be
  updated to Number.MAX_SAFE_INTEGER
* If the user enters non-numeric text, it will be updated to 0

* Ensure that we do not go below the default value

0 is not necessarily a reasonable default, but we already have a
  variable for the default value.

* Update cypress interaction with schedule fields

Now that we set defaults for bad values, it's no longer possible to
"clear" the numeric input. However, to get roughly the same behavior we
can instead select the current value and start typing.
  • Loading branch information
rylnd authored Oct 6, 2020
1 parent bd153c4 commit 7031ea4
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,16 @@ export const fillDefineCustomRuleWithImportedQueryAndContinue = (
) => {
cy.get(IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK).click();
cy.get(TIMELINE(rule.timelineId)).click();
cy.get(CUSTOM_QUERY_INPUT).invoke('text').should('eq', rule.customQuery);
cy.get(CUSTOM_QUERY_INPUT).should('have.text', rule.customQuery);
cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });

cy.get(CUSTOM_QUERY_INPUT).should('not.exist');
};

export const fillScheduleRuleAndContinue = (rule: CustomRule | MachineLearningRule) => {
cy.get(RUNS_EVERY_INTERVAL).clear().type(rule.runsEvery.interval);
cy.get(RUNS_EVERY_INTERVAL).type('{selectall}').type(rule.runsEvery.interval);
cy.get(RUNS_EVERY_TIME_TYPE).select(rule.runsEvery.timeType);
cy.get(LOOK_BACK_INTERVAL).clear().type(rule.lookBack.interval);
cy.get(LOOK_BACK_INTERVAL).type('{selectAll}').type(rule.lookBack.interval);
cy.get(LOOK_BACK_TIME_TYPE).select(rule.lookBack.timeType);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,91 @@
*/

import React from 'react';
import { shallow } from 'enzyme';
import { mount, shallow } from 'enzyme';

import { ScheduleItem } from './index';
import { useFormFieldMock } from '../../../../common/mock';
import { TestProviders, useFormFieldMock } from '../../../../common/mock';

describe('ScheduleItem', () => {
it('renders correctly', () => {
const Component = () => {
const field = useFormFieldMock();
const mockField = useFormFieldMock<string>();
const wrapper = shallow(
<ScheduleItem
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
field={mockField}
/>
);

return (
expect(wrapper.find('[data-test-subj="schedule-item"]')).toHaveLength(1);
});

it('accepts a large number via user input', () => {
const mockField = useFormFieldMock<string>();
const wrapper = mount(
<TestProviders>
<ScheduleItem
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
field={mockField}
/>
</TestProviders>
);

wrapper
.find('[data-test-subj="interval"]')
.last()
.simulate('change', { target: { value: '5000000' } });

expect(mockField.setValue).toHaveBeenCalledWith('5000000s');
});

it('clamps a number value greater than MAX_SAFE_INTEGER to MAX_SAFE_INTEGER', () => {
const unsafeInput = '99999999999999999999999';

const mockField = useFormFieldMock<string>();
const wrapper = mount(
<TestProviders>
<ScheduleItem
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
field={field}
field={mockField}
/>
);
};
const wrapper = shallow(<Component />);
</TestProviders>
);

wrapper
.find('[data-test-subj="interval"]')
.last()
.simulate('change', { target: { value: unsafeInput } });

const expectedValue = `${Number.MAX_SAFE_INTEGER}s`;
expect(mockField.setValue).toHaveBeenCalledWith(expectedValue);
});

it('converts a non-numeric value to 0', () => {
const unsafeInput = 'this is not a number';

const mockField = useFormFieldMock<string>();
const wrapper = mount(
<TestProviders>
<ScheduleItem
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
field={mockField}
/>
</TestProviders>
);

wrapper
.find('[data-test-subj="interval"]')
.last()
.simulate('change', { target: { value: unsafeInput } });

expect(wrapper.dive().find('[data-test-subj="schedule-item"]')).toHaveLength(1);
expect(mockField.setValue).toHaveBeenCalledWith('0s');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { FieldHook, getFieldValidityAndErrorMessage } from '../../../../shared_i
import * as I18n from './translations';

interface ScheduleItemProps {
field: FieldHook;
field: FieldHook<string>;
dataTestSubj: string;
idAria: string;
isDisabled: boolean;
Expand Down Expand Up @@ -62,6 +62,15 @@ const MyEuiSelect = styled(EuiSelect)`
width: auto;
`;

const getNumberFromUserInput = (input: string, defaultValue = 0): number => {
const number = parseInt(input, 10);
if (Number.isNaN(number)) {
return defaultValue;
} else {
return Math.min(number, Number.MAX_SAFE_INTEGER);
}
};

export const ScheduleItem = ({
dataTestSubj,
field,
Expand All @@ -72,30 +81,29 @@ export const ScheduleItem = ({
const [timeType, setTimeType] = useState('s');
const [timeVal, setTimeVal] = useState<number>(0);
const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field);
const { value, setValue } = field;

const onChangeTimeType = useCallback(
(e) => {
setTimeType(e.target.value);
field.setValue(`${timeVal}${e.target.value}`);
setValue(`${timeVal}${e.target.value}`);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[timeVal]
[setValue, timeVal]
);

const onChangeTimeVal = useCallback(
(e) => {
const sanitizedValue: number = parseInt(e.target.value, 10);
const sanitizedValue = getNumberFromUserInput(e.target.value, minimumValue);
setTimeVal(sanitizedValue);
field.setValue(`${sanitizedValue}${timeType}`);
setValue(`${sanitizedValue}${timeType}`);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[timeType]
[minimumValue, setValue, timeType]
);

useEffect(() => {
if (field.value !== `${timeVal}${timeType}`) {
const filterTimeVal = (field.value as string).match(/\d+/g);
const filterTimeType = (field.value as string).match(/[a-zA-Z]+/g);
if (value !== `${timeVal}${timeType}`) {
const filterTimeVal = value.match(/\d+/g);
const filterTimeType = value.match(/[a-zA-Z]+/g);
if (
!isEmpty(filterTimeVal) &&
filterTimeVal != null &&
Expand All @@ -113,8 +121,7 @@ export const ScheduleItem = ({
setTimeType(filterTimeType[0]);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [field.value]);
}, [timeType, timeVal, value]);

// EUI missing some props
const rest = { disabled: isDisabled };
Expand Down Expand Up @@ -157,6 +164,7 @@ export const ScheduleItem = ({
<EuiFieldNumber
fullWidth
min={minimumValue}
max={Number.MAX_SAFE_INTEGER}
onChange={onChangeTimeVal}
value={timeVal}
data-test-subj="interval"
Expand Down

0 comments on commit 7031ea4

Please sign in to comment.