-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker #210585
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.
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
Changes from all commits
6a1d3e3
f26acdf
cc44f6c
d0c1970
99f434a
85d7233
73db16d
eadb7e6
a0b30c7
73ee667
df8d885
6cf58d6
c39eb3b
7020ef4
3d47008
83667ba
28aa4f0
f92eab5
0805163
aeeaf71
f49c643
1f4aebf
63054f5
c7e0665
f261ab8
8c3e9a0
23439aa
770993c
fa80256
2baf50b
40b395c
c26acc7
1de086e
b088e6b
c85dec4
09daf62
829f255
639fee5
8e1dee0
10c8e8e
1b8774f
d58905c
0fc0db5
77be180
19a8011
83b194a
3b25db5
815ba6e
ac38c51
0e1e7a0
d3814a0
0506aac
ed274ee
1ec5817
d503cb6
3e0c647
90c5561
0f95d1e
4540f96
abf09aa
b3ecd5b
61e0787
787a836
871e2a3
eaabb1a
ce3e2b6
90fd3ae
128bfb9
517b20b
79868ca
742c340
1e82688
dd84f99
bec10ae
ddf2573
5f68a90
baecafd
38df1eb
119d6a2
6ff0437
d31acdc
5ac74b7
bac953a
7b9b2df
c96766b
f5beeca
a6381fd
b804725
23f23d6
b904a03
8eb497e
33406cf
082fc42
069393d
ded2d6e
1cf669e
02f5ee4
2906cf3
4f8da74
8dea4ad
6697cd5
14eaf77
5489009
196e6a5
ef8b236
a62eb7c
4bf4202
06ca822
2174b42
7abf93b
8dd9476
4c52692
0cb4d7b
6404e01
8bb6819
983ef71
2c2d8bc
4b95150
a1604fe
38ef963
c00d6aa
e19c4db
e8e4454
ae2da08
30aba34
b10f648
2b72f56
e98f170
40ad20f
af736fb
0b05550
1e8f9d4
1afb06a
2102945
9ff1816
4fa2b9c
4e953e1
4c515c9
8b71caf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,11 @@ import React from 'react'; | |
| import type { ComponentType } from 'react'; | ||
| import type { ReactElement } from 'react-markdown'; | ||
| import type { DataView } from '@kbn/data-views-plugin/common'; | ||
| import { DataViewManagerScopeName } from '../../../data_view_manager/constants'; | ||
| import { useFullDataView } from '../../../data_view_manager/hooks/use_full_data_view'; | ||
| import { DataViewErrorComponent } from './data_view_error'; | ||
| import { useEnableExperimental } from '../../hooks/use_experimental_features'; | ||
|
|
||
| import { useGetScopedSourcererDataView } from '../../../sourcerer/components/use_get_sourcerer_data_view'; | ||
| import { SourcererScopeName } from '../../../sourcerer/store/model'; | ||
|
|
||
|
|
@@ -30,15 +34,21 @@ export const withDataView = <P extends WithDataViewArg>( | |
| fallback?: ReactElement | ||
| ) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing wrong with this pattern, but do you think using something like context would be better? Just a quick note, but still working through the code
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HOCs in general are much harder to reason about imo compared to hooks, and introduce unique to this pattern issues. ComponentWithDataView can and should be memoized, we should also make it a pattern to preserve the displayName of the passed in component, the amount of unknowns popping up in the tree is beginning to be a little confusing. Full suggestion: |
||
| const ComponentWithDataView = (props: OmitDataView<P>) => { | ||
| const { dataView: experimentalDataView } = useFullDataView(DataViewManagerScopeName.timeline); | ||
|
|
||
| const dataView = useGetScopedSourcererDataView({ | ||
| sourcererScope: SourcererScopeName.timeline, | ||
| }); | ||
|
|
||
| if (!dataView) { | ||
| const { newDataViewPickerEnabled } = useEnableExperimental(); | ||
|
|
||
| const dataViewToUse = newDataViewPickerEnabled ? experimentalDataView : dataView; | ||
|
|
||
| if (!dataViewToUse) { | ||
| return fallback ?? <DataViewErrorComponent />; | ||
| } | ||
|
|
||
| return <Component {...(props as unknown as P)} dataView={dataView} />; | ||
| return <Component {...(props as unknown as P)} dataView={dataViewToUse} />; | ||
| }; | ||
|
|
||
| return ComponentWithDataView; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,17 +8,19 @@ | |
| import { useCallback } from 'react'; | ||
| import { useDispatch, useSelector } from 'react-redux'; | ||
| import type { Filter, Query } from '@kbn/es-query'; | ||
| import { useSelectDataView } from '../../../data_view_manager/hooks/use_select_data_view'; | ||
| import { useCreateTimeline } from '../../../timelines/hooks/use_create_timeline'; | ||
| import { updateProviders, setFilters, applyKqlFilterQuery } from '../../../timelines/store/actions'; | ||
| import { SourcererScopeName } from '../../../sourcerer/store/model'; | ||
| import type { DataProvider } from '../../../../common/types'; | ||
| import { sourcererSelectors } from '../../store'; | ||
| import { sourcererActions } from '../../store/actions'; | ||
| import { inputsActions } from '../../store/inputs'; | ||
| import { InputsModelId } from '../../store/inputs/constants'; | ||
| import type { TimeRange } from '../../store/inputs/model'; | ||
| import { TimelineId } from '../../../../common/types/timeline'; | ||
| import { TimelineTypeEnum } from '../../../../common/api/timeline'; | ||
| import { sourcererActions } from '../../store/actions'; | ||
| import { useEnableExperimental } from '../use_experimental_features'; | ||
|
|
||
| interface InvestigateInTimelineArgs { | ||
| /** | ||
|
|
@@ -57,6 +59,7 @@ export const useInvestigateInTimeline = () => { | |
|
|
||
| const signalIndexName = useSelector(sourcererSelectors.signalIndexName); | ||
| const defaultDataView = useSelector(sourcererSelectors.defaultDataView); | ||
| const { newDataViewPickerEnabled } = useEnableExperimental(); | ||
|
|
||
| const clearTimelineTemplate = useCreateTimeline({ | ||
| timelineId: TimelineId.active, | ||
|
|
@@ -68,6 +71,8 @@ export const useInvestigateInTimeline = () => { | |
| timelineType: TimelineTypeEnum.default, | ||
| }); | ||
|
|
||
| const setSelectedDataView = useSelectDataView(); | ||
|
|
||
| const investigateInTimeline = useCallback( | ||
| async ({ | ||
| query, | ||
|
|
@@ -124,19 +129,35 @@ export const useInvestigateInTimeline = () => { | |
| // Only show detection alerts | ||
| // (This is required so the timeline event count matches the prevalence count) | ||
| if (!keepDataView) { | ||
| dispatch( | ||
| sourcererActions.setSelectedDataView({ | ||
| id: SourcererScopeName.timeline, | ||
| selectedDataViewId: defaultDataView.id, | ||
| selectedPatterns: [signalIndexName || ''], | ||
| }) | ||
| ); | ||
| if (newDataViewPickerEnabled) { | ||
| setSelectedDataView({ | ||
| scope: [SourcererScopeName.timeline], | ||
| id: defaultDataView.id, | ||
| fallbackPatterns: [signalIndexName || ''], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand it for current compatibility, but in the long run, we shouldn't need fallbackPatterns anymore?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah and since this is for timelines specifically maybe I should include that in the name somehow? |
||
| }); | ||
| } else { | ||
| dispatch( | ||
| sourcererActions.setSelectedDataView({ | ||
| id: SourcererScopeName.timeline, | ||
| selectedDataViewId: defaultDataView.id, | ||
| selectedPatterns: [signalIndexName || ''], | ||
| }) | ||
| ); | ||
| } | ||
| } | ||
| // Unlock the time range from the global time range | ||
| dispatch(inputsActions.removeLinkTo([InputsModelId.timeline, InputsModelId.global])); | ||
| } | ||
| }, | ||
| [clearTimelineTemplate, clearTimelineDefault, dispatch, defaultDataView.id, signalIndexName] | ||
| [ | ||
| clearTimelineTemplate, | ||
| clearTimelineDefault, | ||
| dispatch, | ||
| newDataViewPickerEnabled, | ||
| setSelectedDataView, | ||
| defaultDataView.id, | ||
| signalIndexName, | ||
| ] | ||
| ); | ||
|
|
||
| return { investigateInTimeline }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,8 +13,11 @@ import { appLinks } from '../../../app_links'; | |
| import { useUserPrivileges } from '../../components/user_privileges'; | ||
| import { useShowTimeline } from './use_show_timeline'; | ||
| import { uiSettingsServiceMock } from '@kbn/core-ui-settings-browser-mocks'; | ||
| import { TestProviders } from '../../mock'; | ||
| import { hasAccessToSecuritySolution } from '../../../helpers_access'; | ||
|
|
||
| jest.mock('../../components/user_privileges'); | ||
| jest.mock('../../../helpers_access', () => ({ hasAccessToSecuritySolution: jest.fn(() => true) })); | ||
|
|
||
| const mockUseLocation = jest.fn().mockReturnValue({ pathname: '/overview' }); | ||
| jest.mock('react-router-dom', () => { | ||
|
|
@@ -35,30 +38,11 @@ jest.mock('../../../sourcerer/containers', () => ({ | |
| useSourcererDataView: () => mockUseSourcererDataView(), | ||
| })); | ||
|
|
||
| const mockSiemUserCanRead = jest.fn(() => true); | ||
| jest.mock('../../lib/kibana', () => { | ||
| const original = jest.requireActual('../../lib/kibana'); | ||
|
|
||
| return { | ||
| ...original, | ||
| useKibana: () => ({ | ||
| services: { | ||
| ...original.useKibana().services, | ||
| application: { | ||
| capabilities: { | ||
| siemV2: { | ||
| show: mockSiemUserCanRead(), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| }; | ||
| }); | ||
|
|
||
| const mockUpselling = new UpsellingService(); | ||
| const mockUiSettingsClient = uiSettingsServiceMock.createStartContract(); | ||
|
|
||
| const renderUseShowTimeline = () => renderHook(() => useShowTimeline(), { wrapper: TestProviders }); | ||
|
|
||
| describe('use show timeline', () => { | ||
| beforeAll(() => { | ||
| (useUserPrivileges as unknown as jest.Mock).mockReturnValue({ | ||
|
|
@@ -84,33 +68,33 @@ describe('use show timeline', () => { | |
| }); | ||
|
|
||
| it('shows timeline for routes on default', async () => { | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| await waitFor(() => expect(result.current).toEqual([true])); | ||
| }); | ||
|
|
||
| it('hides timeline for blacklist routes', async () => { | ||
| mockUseLocation.mockReturnValueOnce({ pathname: '/rules/add_rules' }); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| await waitFor(() => expect(result.current).toEqual([false])); | ||
| }); | ||
|
|
||
| it('shows timeline for partial blacklist routes', async () => { | ||
| mockUseLocation.mockReturnValueOnce({ pathname: '/rules' }); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| await waitFor(() => expect(result.current).toEqual([true])); | ||
| }); | ||
|
|
||
| it('hides timeline for sub blacklist routes', async () => { | ||
| mockUseLocation.mockReturnValueOnce({ pathname: '/administration/policy' }); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| await waitFor(() => expect(result.current).toEqual([false])); | ||
| }); | ||
| it('hides timeline for users without timeline access', async () => { | ||
| (useUserPrivileges as unknown as jest.Mock).mockReturnValue({ | ||
| timelinePrivileges: { read: false }, | ||
| }); | ||
|
|
||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| const showTimeline = result.current; | ||
| expect(showTimeline).toEqual([false]); | ||
| }); | ||
|
|
@@ -120,41 +104,40 @@ it('shows timeline for users with timeline read access', async () => { | |
| timelinePrivileges: { read: true }, | ||
| }); | ||
|
|
||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| const showTimeline = result.current; | ||
| expect(showTimeline).toEqual([true]); | ||
| }); | ||
|
|
||
| describe('sourcererDataView', () => { | ||
| it('should show timeline when indices exist', () => { | ||
| mockUseSourcererDataView.mockReturnValueOnce({ indicesExist: true, dataViewId: 'test' }); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| expect(result.current).toEqual([true]); | ||
| }); | ||
|
|
||
| it('should show timeline when dataViewId is null', () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per the last comment, if you can add a comment for this and the below test why if you don't have indices and no dataview you would still want to show the timeline? |
||
| mockUseSourcererDataView.mockReturnValueOnce({ indicesExist: false, dataViewId: null }); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| expect(result.current).toEqual([true]); | ||
| }); | ||
|
|
||
| it('should not show timeline when dataViewId is not null and indices does not exist', () => { | ||
| mockUseSourcererDataView.mockReturnValueOnce({ indicesExist: false, dataViewId: 'test' }); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| expect(result.current).toEqual([false]); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Security solution capabilities', () => { | ||
| it('should show timeline when user has read capabilities', () => { | ||
| mockSiemUserCanRead.mockReturnValueOnce(true); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| const { result } = renderUseShowTimeline(); | ||
| expect(result.current).toEqual([true]); | ||
| }); | ||
|
|
||
| it('should not show timeline when user does not have read capabilities', () => { | ||
| mockSiemUserCanRead.mockReturnValueOnce(false); | ||
| const { result } = renderHook(() => useShowTimeline()); | ||
| jest.mocked(hasAccessToSecuritySolution).mockReturnValueOnce(false); | ||
| const { result } = renderUseShowTimeline(); | ||
| expect(result.current).toEqual([false]); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.