Skip to content

Commit

Permalink
[8.8] [Security Solution] [Fix] Alert Page Controls do not recover fr…
Browse files Browse the repository at this point in the history
…om invalid query. (#156542) (#156567)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] [Fix] Alert Page Controls do not recover from
invalid query. (#156542)](#156542)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jatin
Kathuria","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-03T14:57:11Z","message":"[Security
Solution] [Fix] Alert Page Controls do not recover from invalid query.
(#156542)\n\n## Summary\r\n\r\nThis PR handles #156016
.\r\n\r\nPreviously, if user supplied an invalid kql or lucene query,
Alert Page\r\ncontrols will go in error state and not recover until user
reloaded the\r\npage or navigated away and back to the Alert
Page.\r\n\r\nThis PR prevents Alert Page Controls going in that error
state.\r\n\r\n| Before | After |\r\n|--|--|\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/235931286-6da23567-4ae8-454a-92b8-a595a20f5655.mov\"\r\n/>
|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/235930584-485df881-d22c-44f3-9d53-f673820eb673.mov\"\r\n/>
|\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable
to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"ff65ca42f2f09589b09a8e61abe22a4a69885170","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Threat
Hunting:Investigations","v8.8.0","v8.9.0"],"number":156542,"url":"https://github.com/elastic/kibana/pull/156542","mergeCommit":{"message":"[Security
Solution] [Fix] Alert Page Controls do not recover from invalid query.
(#156542)\n\n## Summary\r\n\r\nThis PR handles #156016
.\r\n\r\nPreviously, if user supplied an invalid kql or lucene query,
Alert Page\r\ncontrols will go in error state and not recover until user
reloaded the\r\npage or navigated away and back to the Alert
Page.\r\n\r\nThis PR prevents Alert Page Controls going in that error
state.\r\n\r\n| Before | After |\r\n|--|--|\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/235931286-6da23567-4ae8-454a-92b8-a595a20f5655.mov\"\r\n/>
|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/235930584-485df881-d22c-44f3-9d53-f673820eb673.mov\"\r\n/>
|\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable
to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"ff65ca42f2f09589b09a8e61abe22a4a69885170"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156542","number":156542,"mergeCommit":{"message":"[Security
Solution] [Fix] Alert Page Controls do not recover from invalid query.
(#156542)\n\n## Summary\r\n\r\nThis PR handles #156016
.\r\n\r\nPreviously, if user supplied an invalid kql or lucene query,
Alert Page\r\ncontrols will go in error state and not recover until user
reloaded the\r\npage or navigated away and back to the Alert
Page.\r\n\r\nThis PR prevents Alert Page Controls going in that error
state.\r\n\r\n| Before | After |\r\n|--|--|\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/235931286-6da23567-4ae8-454a-92b8-a595a20f5655.mov\"\r\n/>
|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/235930584-485df881-d22c-44f3-9d53-f673820eb673.mov\"\r\n/>
|\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable
to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"ff65ca42f2f09589b09a8e61abe22a4a69885170"}}]}]
BACKPORT-->

Co-authored-by: Jatin Kathuria <[email protected]>
  • Loading branch information
kibanamachine and logeekal authored May 3, 2023
1 parent a9958bc commit 92ced17
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,19 @@ const getStoreWithCustomState = (newState: typeof state = state) => {
return createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);
};

const TestComponent: FC<ComponentProps<typeof TestProviders>> = (props) => (
const TestComponent: FC<
ComponentProps<typeof TestProviders> & {
filterGroupProps?: Partial<ComponentProps<typeof FilterGroup>>;
}
> = (props) => (
<TestProviders store={getStoreWithCustomState()} {...props}>
<FilterGroup
initialControls={DEFAULT_DETECTION_PAGE_FILTERS}
dataViewId="security-solution-default"
chainingSystem="HIERARCHICAL"
onFilterChange={onFilterChangeMock}
onInit={onInitMock}
{...props.filterGroupProps}
/>
</TestProviders>
);
Expand Down Expand Up @@ -522,6 +527,36 @@ describe(' Filter Group Component ', () => {
expect(screen.queryByTestId(TEST_IDS.SAVE_CHANGE_POPOVER)).toBeVisible();
});
});
it('should update controlGroup with new filters and queries when valid query is supplied', async () => {
const validQuery = { query: { language: 'kuery', query: '' } };
// pass an invalid query
render(<TestComponent filterGroupProps={validQuery} />);

await waitFor(() => {
expect(controlGroupMock.updateInput).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
filters: undefined,
query: validQuery.query,
})
);
});
});

it('should not update controlGroup with new filters and queries when invalid query is supplied', async () => {
const invalidQuery = { query: { language: 'kuery', query: '\\' } };
// pass an invalid query
render(<TestComponent filterGroupProps={invalidQuery} />);

await waitFor(() => {
expect(controlGroupMock.updateInput).toHaveBeenCalledWith(
expect.objectContaining({
filters: [],
query: undefined,
})
);
});
});
});

describe('Filter Changed Banner', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { FilterGroupContext } from './filter_group_context';
import { NUM_OF_CONTROLS } from './config';
import { TEST_IDS } from './constants';
import { URL_PARAM_ARRAY_EXCEPTION_MSG } from './translations';
import { convertToBuildEsQuery } from '../../lib/kuery';

const FilterWrapper = styled.div.attrs((props) => ({
className: props.className,
Expand Down Expand Up @@ -149,14 +150,41 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
return cleanup;
}, []);

const { filters: validatedFilters, query: validatedQuery } = useMemo(() => {
const [_, kqlError] = convertToBuildEsQuery({
config: {},
queries: query ? [query] : [],
filters: filters ?? [],
indexPattern: { fields: [], title: '' },
});

// we only need to handle kqlError because control group can handle Lucene error
if (kqlError) {
/*
* Based on the behaviour from other components,
* ignore all filters and queries if there is some error
* in the input filters and queries
*
* */
return {
filters: [],
query: undefined,
};
}
return {
filters,
query,
};
}, [filters, query]);

useEffect(() => {
controlGroup?.updateInput({
filters: validatedFilters,
query: validatedQuery,
timeRange,
filters,
query,
chainingSystem,
});
}, [timeRange, filters, query, chainingSystem, controlGroup]);
}, [timeRange, chainingSystem, controlGroup, validatedQuery, validatedFilters]);

const handleInputUpdates = useCallback(
(newInput: ControlGroupInput) => {
Expand All @@ -171,7 +199,7 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
[setControlGroupInputUpdates, getStoredControlInput, isViewMode, setHasPendingChanges]
);

const handleFilterUpdates = useCallback(
const handleOutputFilterUpdates = useCallback(
({ filters: newFilters }: ControlGroupOutput) => {
if (isEqual(currentFiltersRef.current, newFilters)) return;
if (onFilterChange) onFilterChange(newFilters ?? []);
Expand All @@ -181,8 +209,8 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
);

const debouncedFilterUpdates = useMemo(
() => debounce(handleFilterUpdates, 500),
[handleFilterUpdates]
() => debounce(handleOutputFilterUpdates, 500),
[handleOutputFilterUpdates]
);

useEffect(() => {
Expand Down

0 comments on commit 92ced17

Please sign in to comment.