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

[Security Solution][Exceptions] - Make esTypes and subType available to index patterns #72336

Merged
merged 12 commits into from
Jul 20, 2020
Merged

[Security Solution][Exceptions] - Make esTypes and subType available to index patterns #72336

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
952a105
making estypes and subtype available in indexPatterns
yctercero Jul 17, 2020
4448734
updating graphql types
yctercero Jul 17, 2020
c56a96d
Merge branch 'master' of github.com:yctercero/kibana into index_es_types
yctercero Jul 17, 2020
f059d70
update source graphql type to match expected
yctercero Jul 17, 2020
42de990
cleanup
yctercero Jul 17, 2020
ebcb6d3
tests
yctercero Jul 19, 2020
2861cfa
cleanup
yctercero Jul 19, 2020
760ed31
updated tests, reverted changes to modals, to make sure they were sti…
yctercero Jul 20, 2020
548ead4
Merge branch 'master' of github.com:yctercero/kibana into index_es_types
yctercero Jul 20, 2020
1e57353
Merge branch 'master' of github.com:yctercero/kibana into index_es_types
yctercero Jul 20, 2020
f667704
applying feedback
yctercero Jul 20, 2020
7d5744f
Merge branch 'master' of github.com:yctercero/kibana into index_es_types
yctercero Jul 20, 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
122 changes: 92 additions & 30 deletions ...lugins/security_solution/public/common/components/autocomplete/field_value_lists.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,22 @@ import { ThemeProvider } from 'styled-components';
import { mount } from 'enzyme';
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui';
import { act } from 'react-dom/test-utils';

import { getField } from '../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks.ts';
import { AutocompleteFieldListsComponent } from './field_value_lists';
import { getFoundListSchemaMock } from '../../../../../lists/common/schemas/response/found_list_schema.mock';
import { getListResponseMock } from '../../../../../lists/common/schemas/response/list_schema.mock';
import { wait } from '../../../common/lib/helpers';

jest.mock('../../../common/lib/kibana');
const mockStart = jest.fn();
const mockKeywordList = getListResponseMock();
mockKeywordList.id = 'keyword_list';
mockKeywordList.type = 'keyword';
mockKeywordList.name = 'keyword list';
yctercero marked this conversation as resolved.
Show resolved Hide resolved
const mockResult = getFoundListSchemaMock();
jest.mock('../../../common/lib/kibana');
mockResult.data = [...mockResult.data, mockKeywordList];
jest.mock('../../../lists_plugin_deps', () => {
const originalModule = jest.requireActual('../../../lists_plugin_deps');

Expand All @@ -31,78 +39,130 @@ jest.mock('../../../lists_plugin_deps', () => {
});

describe('AutocompleteFieldListsComponent', () => {
test('it renders disabled if "isDisabled" is true', () => {
test('it renders disabled if "isDisabled" is true', async () => {
await act(async () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<AutocompleteFieldListsComponent
placeholder="Placeholder text"
selectedField={getField('ip')}
selectedValue="some-list-id"
isLoading={false}
isClearable={false}
isDisabled={true}
onChange={jest.fn()}
/>
</ThemeProvider>
);
await wait();
expect(
wrapper
.find(`[data-test-subj="valuesAutocompleteComboBox listsComboxBox"] input`)
.prop('disabled')
).toBeTruthy();
});
});

test('it renders loading if "isLoading" is true', async () => {
await act(async () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<AutocompleteFieldListsComponent
placeholder="Placeholder text"
selectedField={getField('ip')}
selectedValue="some-list-id"
isLoading={true}
isClearable={false}
isDisabled={false}
onChange={jest.fn()}
/>
</ThemeProvider>
);
await wait();
wrapper
.find(`[data-test-subj="valuesAutocompleteComboBox listsComboxBox"] button`)
.at(0)
.simulate('click');
expect(
wrapper
.find(
`EuiComboBoxOptionsList[data-test-subj="valuesAutocompleteComboBox listsComboxBox-optionsList"]`
)
.prop('isLoading')
).toBeTruthy();
});
});

test('it allows user to clear values if "isClearable" is true', async () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<AutocompleteFieldListsComponent
placeholder="Placeholder text"
selectedField={getField('ip')}
selectedValue="some-list-id"
isLoading={false}
isClearable={false}
isDisabled={true}
isClearable={true}
isDisabled={false}
onChange={jest.fn()}
/>
</ThemeProvider>
);

expect(
wrapper
.find(`[data-test-subj="valuesAutocompleteComboBox listsComboxBox"] input`)
.prop('disabled')
.find(`[data-test-subj="comboBoxInput"]`)
.hasClass('euiComboBox__inputWrap-isClearable')
).toBeTruthy();
});

test('it renders loading if "isLoading" is true', () => {
test('it correctly displays lists that match the selected "keyword" field esType', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<AutocompleteFieldListsComponent
placeholder="Placeholder text"
selectedField={getField('ip')}
selectedValue="some-list-id"
isLoading={true}
selectedField={getField('@tags')}
selectedValue=""
isLoading={false}
isClearable={false}
isDisabled={false}
onChange={jest.fn()}
/>
</ThemeProvider>
);
wrapper
.find(`[data-test-subj="valuesAutocompleteComboBox listsComboxBox"] button`)
.at(0)
.simulate('click');

wrapper.find('[data-test-subj="comboBoxToggleListButton"] button').simulate('click');

expect(
wrapper
.find(
`EuiComboBoxOptionsList[data-test-subj="valuesAutocompleteComboBox listsComboxBox-optionsList"]`
)
.prop('isLoading')
).toBeTruthy();
.find('EuiComboBox[data-test-subj="valuesAutocompleteComboBox listsComboxBox"]')
.prop('options')
).toEqual([{ label: 'keyword list' }]);
});

test('it allows user to clear values if "isClearable" is true', () => {
test('it correctly displays lists that match the selected "ip" field esType', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<AutocompleteFieldListsComponent
placeholder="Placeholder text"
selectedField={getField('ip')}
selectedValue="some-list-id"
selectedValue=""
isLoading={false}
isClearable={true}
isClearable={false}
isDisabled={false}
onChange={jest.fn()}
/>
</ThemeProvider>
);

wrapper.find('[data-test-subj="comboBoxToggleListButton"] button').simulate('click');

expect(
wrapper
.find(`[data-test-subj="comboBoxInput"]`)
.hasClass('euiComboBox__inputWrap-isClearable')
).toBeTruthy();
.find('EuiComboBox[data-test-subj="valuesAutocompleteComboBox listsComboxBox"]')
.prop('options')
).toEqual([{ label: 'some name' }]);
});

test('it correctly displays selected list', () => {
test('it correctly displays selected list', async () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<AutocompleteFieldListsComponent
Expand Down Expand Up @@ -141,9 +201,11 @@ describe('AutocompleteFieldListsComponent', () => {
</ThemeProvider>
);

((wrapper.find(EuiComboBox).props() as unknown) as {
onChange: (a: EuiComboBoxOptionOption[]) => void;
}).onChange([{ label: 'some name' }]);
await act(async () => {
((wrapper.find(EuiComboBox).props() as unknown) as {
onChange: (a: EuiComboBoxOptionOption[]) => void;
}).onChange([{ label: 'some name' }]);
});

expect(mockOnChange).toHaveBeenCalledWith({
created_at: '2020-04-20T15:25:31.830Z',
yctercero marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_lists.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ export const AutocompleteFieldListsComponent: React.FC<AutocompleteFieldListsPro
const getLabel = useCallback(({ name }) => name, []);

const optionsMemo = useMemo(() => {
if (selectedField != null) {
return lists.filter(({ type }) => type === selectedField.type);
if (
selectedField != null &&
selectedField.esTypes != null &&
selectedField.esTypes.length > 0
) {
return lists.filter(({ type }) => selectedField.esTypes?.includes(type));
} else {
return [];
}
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ export const AutocompleteFieldMatchComponent: React.FC<AutocompleteFieldMatchPro
});
};

const isValid = useMemo(
(): boolean => validateParams(selectedValue, selectedField ? selectedField.type : ''),
[selectedField, selectedValue]
);
const isValid = useMemo((): boolean => validateParams(selectedValue, selectedField), [
selectedField,
selectedValue,
]);

return (
<EuiComboBox
Expand Down
2 changes: 1 addition & 1 deletion ...plugins/security_solution/public/common/components/autocomplete/field_value_match_any.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const AutocompleteFieldMatchAnyComponent: React.FC<AutocompleteFieldMatch

const isValid = useMemo((): boolean => {
const areAnyInvalid = selectedComboOptions.filter(
({ label }) => !validateParams(label, selectedField ? selectedField.type : '')
({ label }) => !validateParams(label, selectedField)
);
return areAnyInvalid.length === 0;
}, [selectedComboOptions, selectedField]);
Expand Down
24 changes: 6 additions & 18 deletions x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,49 +55,37 @@ describe('helpers', () => {

describe('#validateParams', () => {
test('returns true if value is undefined', () => {
const isValid = validateParams(undefined, 'date');
const isValid = validateParams(undefined, getField('@timestamp'));

expect(isValid).toBeTruthy();
});

test('returns true if value is empty string', () => {
const isValid = validateParams('', 'date');
const isValid = validateParams('', getField('@timestamp'));

expect(isValid).toBeTruthy();
});

test('returns true if type is "date" and value is valid', () => {
const isValid = validateParams('1994-11-05T08:15:30-05:00', 'date');
const isValid = validateParams('1994-11-05T08:15:30-05:00', getField('@timestamp'));

expect(isValid).toBeTruthy();
});

test('returns false if type is "date" and value is not valid', () => {
const isValid = validateParams('1593478826', 'date');
const isValid = validateParams('1593478826', getField('@timestamp'));

expect(isValid).toBeFalsy();
});

test('returns true if type is "ip" and value is valid', () => {
const isValid = validateParams('126.45.211.34', 'ip');
const isValid = validateParams('126.45.211.34', getField('ip'));

expect(isValid).toBeTruthy();
});

test('returns false if type is "ip" and value is not valid', () => {
const isValid = validateParams('hellooo', 'ip');

expect(isValid).toBeFalsy();
});

test('returns true if type is "number" and value is valid', () => {
const isValid = validateParams('123', 'number');

expect(isValid).toBeTruthy();
});

test('returns false if type is "number" and value is not valid', () => {
const isValid = validateParams('not a number', 'number');
const isValid = validateParams('hellooo', getField('ip'));

expect(isValid).toBeFalsy();
});
Expand Down
40 changes: 22 additions & 18 deletions x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,33 @@ export const getOperators = (field: IFieldType | undefined): OperatorOption[] =>
}
};

export function validateParams(params: string | undefined, type: string) {
export const validateParams = (
params: string | undefined,
field: IFieldType | undefined
): boolean => {
// Box would show error state if empty otherwise
if (params == null || params === '') {
return true;
}

switch (type) {
case 'date':
const moment = dateMath.parse(params);
return Boolean(moment && moment.isValid());
case 'ip':
try {
return Boolean(new Ipv4Address(params));
} catch (e) {
return false;
}
case 'number':
const val = parseFloat(params);
return typeof val === 'number' && !isNaN(val);
default:
return true;
}
}
const types = field != null && field.esTypes != null ? field.esTypes : [];

return types.reduce<boolean>((acc, type) => {
switch (type) {
case 'date':
const moment = dateMath.parse(params);
return Boolean(moment && moment.isValid());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, can you just do: return moment && moment.isValid() ? Won't that work as well without introducing the capital boolean boxing here?

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 tried removing it and the test yelled at me saying expected boolean not string. Leaving for now.

case 'ip':
try {
return Boolean(new Ipv4Address(params));
Copy link
Contributor

Choose a reason for hiding this comment

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

This new object seems weird to me...Does this mean we only support ipv4 and not ipv6?

If this is only validating ipv4 but allowing ipv6 I am fine with it. If we begin hindering users or disallowing ipv6 though we are going to start to have issue relatively quickly.

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 did a bit of research and there isn't really great consensus on a good regex for ipv6. I didn't find a similar util like the one above for ipv4 in our code so I removed the validation here. I don't want to block users from adding exceptions. This can be something we can circle back on.

} catch (e) {
return false;
}
default:
return acc;
}
}, true);
};

export function getGenericComboBoxProps<T>({
options,
Expand Down
Loading