From 5f3ac5d938e6cc6443b8f9bb92db46ec72072037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20=C3=81brah=C3=A1m?= Date: Thu, 1 Dec 2022 12:32:20 +0100 Subject: [PATCH] [Security Solution] UI Event Filters RBAC (#146111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Similarly to https://github.com/elastic/kibana/pull/145593, this PR handles the _None_ and _Read_ privileges for the Event Filters sub-feature. The _All_ privilege should not need any UI modification, but will need API modification. image The modification should: - hide Event Filters from Manage navigation items if privilege is NONE, ~(note: it is still displayed for non-superusers, if the feature flag is disabled)~ update: it is hidden for non-superusers if the feature flag is disabled - disable add/edit/delete for Event Filters if privilege is READ. #### Checked: - the Event Filters form still works from the "Hosts > Events" side of the app ✅ image image ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../public/management/links.test.ts | 87 ++++++++----------- .../public/management/links.ts | 7 +- .../event_filters/view/event_filters_list.tsx | 5 ++ .../event_filters_list.test.tsx | 69 +++++++++++++++ 4 files changed, 115 insertions(+), 53 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/links.test.ts b/x-pack/plugins/security_solution/public/management/links.test.ts index f17fa4bc46c234..83e8b43d674a72 100644 --- a/x-pack/plugins/security_solution/public/management/links.test.ts +++ b/x-pack/plugins/security_solution/public/management/links.test.ts @@ -64,13 +64,10 @@ describe('links', () => { } as unknown as StartPlugins); }); - it('should return all links without filtering when having isolate permission', async () => { + it('should return all links for user with all sub-feature privileges', async () => { (calculateEndpointAuthz as jest.Mock).mockReturnValue(getEndpointAuthzInitialStateMock()); - const filteredLinks = await getManagementFilteredLinks( - coreMockStarted, - getPlugins(['superuser']) - ); + const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([])); expect(filteredLinks).toEqual(links); }); @@ -87,13 +84,14 @@ describe('links', () => { coreMockStarted, getPlugins(['superuser']) ); - expect(filteredLinks).toEqual({ - ...links, - links: links.links?.filter((link) => link.id !== SecurityPageName.responseActionsHistory), - }); + expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.responseActionsHistory)); }); }); + // todo: these tests should be updated, because in the end, showing/hiding HIE depends on nothing + // else but the mock return of `calculateEndpointAuthz`. + // These tests should check what is the value of `hasHostIsolationExceptions` which is passed to + // `calculateEndpointAuthz`. describe('Host Isolation Exception', () => { it('should return all but HIE when NO isolation permission due to privilege', async () => { (calculateEndpointAuthz as jest.Mock).mockReturnValue({ @@ -102,6 +100,8 @@ describe('links', () => { canAccessEndpointManagement: true, canReadActionsLogManagement: true, canReadEndpointList: true, + canReadTrustedApplications: true, + canReadEventFilters: true, }); const filteredLinks = await getManagementFilteredLinks( @@ -118,6 +118,8 @@ describe('links', () => { canAccessEndpointManagement: true, canReadActionsLogManagement: true, canReadEndpointList: true, + canReadTrustedApplications: true, + canReadEventFilters: true, }); fakeHttpServices.get.mockResolvedValue({ total: 0 }); @@ -135,6 +137,8 @@ describe('links', () => { canAccessEndpointManagement: false, canReadActionsLogManagement: true, canReadEndpointList: true, + canReadTrustedApplications: true, + canReadEventFilters: true, }); fakeHttpServices.get.mockResolvedValue({ total: 1 }); @@ -166,6 +170,8 @@ describe('links', () => { canUnIsolateHost: true, canReadActionsLogManagement: true, canReadEndpointList: true, + canReadTrustedApplications: true, + canReadEventFilters: true, }); fakeHttpServices.get.mockRejectedValue(new Error()); @@ -182,6 +188,8 @@ describe('links', () => { canUnIsolateHost: true, canReadActionsLogManagement: false, canReadEndpointList: true, + canReadTrustedApplications: true, + canReadEventFilters: true, }); fakeHttpServices.get.mockRejectedValue(new Error()); @@ -198,52 +206,30 @@ describe('links', () => { }); }); - // this can be removed after removing endpointRbacEnabled feature flag - describe('without endpointRbacEnabled', () => { - beforeAll(() => { - ExperimentalFeaturesService.init({ - experimentalFeatures: { ...allowedExperimentalValues, endpointRbacEnabled: false }, - }); - }); - - it('shows Trusted Applications for non-superuser, too', async () => { - (calculateEndpointAuthz as jest.Mock).mockReturnValue(getEndpointAuthzInitialStateMock()); + it('should hide Trusted Applications for user without privilege', async () => { + (calculateEndpointAuthz as jest.Mock).mockReturnValue( + getEndpointAuthzInitialStateMock({ + canReadTrustedApplications: false, + }) + ); - const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([])); + const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([])); - expect(filteredLinks).toEqual(links); - }); + expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.trustedApps)); }); - // this can be the default after removing endpointRbacEnabled feature flag - describe('with endpointRbacEnabled', () => { - beforeAll(() => { - ExperimentalFeaturesService.init({ - experimentalFeatures: { ...allowedExperimentalValues, endpointRbacEnabled: true }, - }); - }); - - it('hides Trusted Applications for user without privilege', async () => { - (calculateEndpointAuthz as jest.Mock).mockReturnValue( - getEndpointAuthzInitialStateMock({ - canReadTrustedApplications: false, - canReadHostIsolationExceptions: true, - }) - ); - - const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([])); - - expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.trustedApps)); - }); - - it('shows Trusted Applications for user with privilege', async () => { - (calculateEndpointAuthz as jest.Mock).mockReturnValue(getEndpointAuthzInitialStateMock()); + it('should hide Event Filters for user without privilege', async () => { + (calculateEndpointAuthz as jest.Mock).mockReturnValue( + getEndpointAuthzInitialStateMock({ + canReadEventFilters: false, + }) + ); - const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([])); + const filteredLinks = await getManagementFilteredLinks(coreMockStarted, getPlugins([])); - expect(filteredLinks).toEqual(links); - }); + expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.eventFilters)); }); + describe('Endpoint List', () => { it('should return all but endpoints link when no Endpoint List READ access', async () => { (calculateEndpointAuthz as jest.Mock).mockReturnValue( @@ -255,10 +241,7 @@ describe('links', () => { coreMockStarted, getPlugins(['superuser']) ); - expect(filteredLinks).toEqual({ - ...links, - links: links.links?.filter((link) => link.id !== SecurityPageName.endpoints), - }); + expect(filteredLinks).toEqual(getLinksWithout(SecurityPageName.endpoints)); }); }); }); diff --git a/x-pack/plugins/security_solution/public/management/links.ts b/x-pack/plugins/security_solution/public/management/links.ts index fa564cc67a3385..fde26e39aead41 100644 --- a/x-pack/plugins/security_solution/public/management/links.ts +++ b/x-pack/plugins/security_solution/public/management/links.ts @@ -278,6 +278,7 @@ export const getManagementFilteredLinks = async ( canReadHostIsolationExceptions, canReadEndpointList, canReadTrustedApplications, + canReadEventFilters, } = fleetAuthz ? calculateEndpointAuthz( licenseService, @@ -301,9 +302,13 @@ export const getManagementFilteredLinks = async ( linksToExclude.push(SecurityPageName.hostIsolationExceptions); } - if (endpointRbacEnabled && !canReadTrustedApplications) { + if (!canReadTrustedApplications) { linksToExclude.push(SecurityPageName.trustedApps); } + if (!canReadEventFilters) { + linksToExclude.push(SecurityPageName.eventFilters); + } + return excludeLinks(linksToExclude); }; diff --git a/x-pack/plugins/security_solution/public/management/pages/event_filters/view/event_filters_list.tsx b/x-pack/plugins/security_solution/public/management/pages/event_filters/view/event_filters_list.tsx index 0e8391cf2cee07..290f5dab957bd1 100644 --- a/x-pack/plugins/security_solution/public/management/pages/event_filters/view/event_filters_list.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/event_filters/view/event_filters_list.tsx @@ -11,6 +11,7 @@ import { FormattedMessage } from '@kbn/i18n-react'; import type { DocLinks } from '@kbn/doc-links'; import { EuiLink } from '@elastic/eui'; +import { useUserPrivileges } from '../../../../common/components/user_privileges'; import { useHttp } from '../../../../common/lib/kibana'; import type { ArtifactListPageProps } from '../../../components/artifact_list_page'; import { ArtifactListPage } from '../../../components/artifact_list_page'; @@ -133,6 +134,7 @@ const EVENT_FILTERS_PAGE_LABELS: ArtifactListPageProps['labels'] = { }; export const EventFiltersList = memo(() => { + const { canWriteEventFilters } = useUserPrivileges().endpointPrivileges; const http = useHttp(); const eventFiltersApiClient = EventFiltersApiClient.getInstance(http); @@ -144,6 +146,9 @@ export const EventFiltersList = memo(() => { data-test-subj="EventFiltersListPage" searchableFields={SEARCHABLE_FIELDS} flyoutSize="l" + allowCardCreateAction={canWriteEventFilters} + allowCardEditAction={canWriteEventFilters} + allowCardDeleteAction={canWriteEventFilters} /> ); }); diff --git a/x-pack/plugins/security_solution/public/management/pages/event_filters/view/integration_tests/event_filters_list.test.tsx b/x-pack/plugins/security_solution/public/management/pages/event_filters/view/integration_tests/event_filters_list.test.tsx index 75b35eaea2be70..c454cda4d49b23 100644 --- a/x-pack/plugins/security_solution/public/management/pages/event_filters/view/integration_tests/event_filters_list.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/event_filters/view/integration_tests/event_filters_list.test.tsx @@ -15,6 +15,11 @@ import { EventFiltersList } from '../event_filters_list'; import { exceptionsListAllHttpMocks } from '../../../../mocks/exceptions_list_http_mocks'; import { SEARCHABLE_FIELDS } from '../../constants'; import { parseQueryFilterToKQL } from '../../../../common/utils'; +import type { EndpointPrivileges } from '../../../../../../common/endpoint/types'; +import { useUserPrivileges } from '../../../../../common/components/user_privileges'; + +jest.mock('../../../../../common/components/user_privileges'); +const mockUserPrivileges = useUserPrivileges as jest.Mock; describe('When on the Event Filters list page', () => { let render: () => ReturnType; @@ -22,6 +27,7 @@ describe('When on the Event Filters list page', () => { let history: AppContextTestRender['history']; let mockedContext: AppContextTestRender; let apiMocks: ReturnType; + let mockedEndpointPrivileges: Partial; beforeEach(() => { mockedContext = createAppRootMockRenderer(); @@ -31,6 +37,13 @@ describe('When on the Event Filters list page', () => { act(() => { history.push(EVENT_FILTERS_PATH); }); + + mockedEndpointPrivileges = { canWriteTrustedApplications: true }; + mockUserPrivileges.mockReturnValue({ endpointPrivileges: mockedEndpointPrivileges }); + }); + + afterEach(() => { + mockUserPrivileges.mockReset(); }); it('should search using expected exception item fields', async () => { @@ -55,4 +68,60 @@ describe('When on the Event Filters list page', () => { }) ); }); + + describe('RBAC Event Filters', () => { + describe('ALL privilege', () => { + beforeEach(() => { + mockedEndpointPrivileges.canWriteEventFilters = true; + }); + + it('should enable adding entries', async () => { + render(); + + await waitFor(() => + expect(renderResult.queryByTestId('EventFiltersListPage-pageAddButton')).toBeTruthy() + ); + }); + + it('should enable modifying/deleting entries', async () => { + render(); + + const actionsButton = await waitFor( + () => renderResult.getAllByTestId('EventFiltersListPage-card-header-actions-button')[0] + ); + userEvent.click(actionsButton); + + expect(renderResult.getByTestId('EventFiltersListPage-card-cardEditAction')).toBeTruthy(); + expect(renderResult.getByTestId('EventFiltersListPage-card-cardDeleteAction')).toBeTruthy(); + }); + }); + + describe('READ privilege', () => { + beforeEach(() => { + mockedEndpointPrivileges.canWriteEventFilters = false; + }); + + it('should disable adding entries', async () => { + render(); + + await waitFor(() => + expect(renderResult.queryByTestId('EventFiltersListPage-container')).toBeTruthy() + ); + + expect(renderResult.queryByTestId('EventFiltersListPage-pageAddButton')).toBeNull(); + }); + + it('should disable modifying/deleting entries', async () => { + render(); + + await waitFor(() => + expect(renderResult.queryByTestId('EventFiltersListPage-container')).toBeTruthy() + ); + + expect( + renderResult.queryByTestId('EventFiltersListPage-card-header-actions-button') + ).toBeNull(); + }); + }); + }); });