From de7bc89de1171e89e4d22575c676765ddc87175a Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Wed, 11 Sep 2024 12:28:55 +0200 Subject: [PATCH 1/7] depreciated const removal --- .../pages/blocklist/services/blocklists_api_client.ts | 6 +++--- .../pages/event_filters/service/api_client.ts | 6 +++--- .../host_isolation_exceptions_api_client.ts | 10 +++------- .../pages/trusted_apps/service/api_client.ts | 6 +++--- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/blocklist/services/blocklists_api_client.ts b/x-pack/plugins/security_solution/public/management/pages/blocklist/services/blocklists_api_client.ts index 017301c55a018..eb47a4d22ae2c 100644 --- a/x-pack/plugins/security_solution/public/management/pages/blocklist/services/blocklists_api_client.ts +++ b/x-pack/plugins/security_solution/public/management/pages/blocklist/services/blocklists_api_client.ts @@ -10,7 +10,7 @@ import type { ExceptionListItemSchema, UpdateExceptionListItemSchema, } from '@kbn/securitysolution-io-ts-list-types'; -import { ENDPOINT_BLOCKLISTS_LIST_ID } from '@kbn/securitysolution-list-constants'; +import { ENDPOINT_ARTIFACT_LISTS } from '@kbn/securitysolution-list-constants'; import type { HttpStart } from '@kbn/core/public'; import type { ConditionEntry } from '../../../../../common/endpoint/types'; @@ -46,7 +46,7 @@ export class BlocklistsApiClient extends ExceptionsListApiClient { constructor(http: HttpStart) { super( http, - ENDPOINT_BLOCKLISTS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.blocklists.id, BLOCKLISTS_LIST_DEFINITION, readTransform, writeTransform @@ -56,7 +56,7 @@ export class BlocklistsApiClient extends ExceptionsListApiClient { public static getInstance(http: HttpStart): ExceptionsListApiClient { return super.getInstance( http, - ENDPOINT_BLOCKLISTS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.blocklists.id, BLOCKLISTS_LIST_DEFINITION, readTransform, writeTransform diff --git a/x-pack/plugins/security_solution/public/management/pages/event_filters/service/api_client.ts b/x-pack/plugins/security_solution/public/management/pages/event_filters/service/api_client.ts index a69e54bf7776d..e7c5e53e34274 100644 --- a/x-pack/plugins/security_solution/public/management/pages/event_filters/service/api_client.ts +++ b/x-pack/plugins/security_solution/public/management/pages/event_filters/service/api_client.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { ENDPOINT_EVENT_FILTERS_LIST_ID } from '@kbn/securitysolution-list-constants'; +import { ENDPOINT_ARTIFACT_LISTS } from '@kbn/securitysolution-list-constants'; import type { HttpStart } from '@kbn/core/public'; import type { CreateExceptionListItemSchema, @@ -33,7 +33,7 @@ export class EventFiltersApiClient extends ExceptionsListApiClient { constructor(http: HttpStart) { super( http, - ENDPOINT_EVENT_FILTERS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.eventFilters.id, EVENT_FILTER_LIST_DEFINITION, undefined, writeTransform @@ -43,7 +43,7 @@ export class EventFiltersApiClient extends ExceptionsListApiClient { public static getInstance(http: HttpStart): ExceptionsListApiClient { return super.getInstance( http, - ENDPOINT_EVENT_FILTERS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.eventFilters.id, EVENT_FILTER_LIST_DEFINITION, undefined, writeTransform diff --git a/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts b/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts index eea09ceaf91ca..4d8bcaba4469d 100644 --- a/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts +++ b/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { ENDPOINT_HOST_ISOLATION_EXCEPTIONS_LIST_ID } from '@kbn/securitysolution-list-constants'; +import { ENDPOINT_ARTIFACT_LISTS } from '@kbn/securitysolution-list-constants'; import type { HttpStart } from '@kbn/core/public'; import { ExceptionsListApiClient } from '../../services/exceptions_list/exceptions_list_api_client'; import { HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION } from './constants'; @@ -17,17 +17,13 @@ import { HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION } from './constants'; */ export class HostIsolationExceptionsApiClient extends ExceptionsListApiClient { constructor(http: HttpStart) { - super( - http, - ENDPOINT_HOST_ISOLATION_EXCEPTIONS_LIST_ID, - HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION - ); + super(http, ENDPOINT_ARTIFACT_LISTS.eventFilters.id, HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION); } public static getInstance(http: HttpStart): ExceptionsListApiClient { return super.getInstance( http, - ENDPOINT_HOST_ISOLATION_EXCEPTIONS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.eventFilters.id, HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION ); } diff --git a/x-pack/plugins/security_solution/public/management/pages/trusted_apps/service/api_client.ts b/x-pack/plugins/security_solution/public/management/pages/trusted_apps/service/api_client.ts index 9ace94955d30f..85672ea0c8b03 100644 --- a/x-pack/plugins/security_solution/public/management/pages/trusted_apps/service/api_client.ts +++ b/x-pack/plugins/security_solution/public/management/pages/trusted_apps/service/api_client.ts @@ -10,7 +10,7 @@ import type { ExceptionListItemSchema, UpdateExceptionListItemSchema, } from '@kbn/securitysolution-io-ts-list-types'; -import { ENDPOINT_TRUSTED_APPS_LIST_ID } from '@kbn/securitysolution-list-constants'; +import { ENDPOINT_ARTIFACT_LISTS } from '@kbn/securitysolution-list-constants'; import type { HttpStart } from '@kbn/core/public'; import type { ConditionEntry } from '../../../../../common/endpoint/types'; @@ -46,7 +46,7 @@ export class TrustedAppsApiClient extends ExceptionsListApiClient { constructor(http: HttpStart) { super( http, - ENDPOINT_TRUSTED_APPS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.trustedApps.id, TRUSTED_APPS_EXCEPTION_LIST_DEFINITION, readTransform, writeTransform @@ -56,7 +56,7 @@ export class TrustedAppsApiClient extends ExceptionsListApiClient { public static getInstance(http: HttpStart): ExceptionsListApiClient { return super.getInstance( http, - ENDPOINT_TRUSTED_APPS_LIST_ID, + ENDPOINT_ARTIFACT_LISTS.trustedApps.id, TRUSTED_APPS_EXCEPTION_LIST_DEFINITION, readTransform, writeTransform From 21740a0cca0597a480d1aee6c58630361ebac85d Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Wed, 11 Sep 2024 12:56:12 +0200 Subject: [PATCH 2/7] depreciated const removal --- .../host_isolation_exceptions_api_client.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts b/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts index 4d8bcaba4469d..94bdab6af651e 100644 --- a/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts +++ b/x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/host_isolation_exceptions_api_client.ts @@ -17,13 +17,17 @@ import { HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION } from './constants'; */ export class HostIsolationExceptionsApiClient extends ExceptionsListApiClient { constructor(http: HttpStart) { - super(http, ENDPOINT_ARTIFACT_LISTS.eventFilters.id, HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION); + super( + http, + ENDPOINT_ARTIFACT_LISTS.hostIsolationExceptions.id, + HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION + ); } public static getInstance(http: HttpStart): ExceptionsListApiClient { return super.getInstance( http, - ENDPOINT_ARTIFACT_LISTS.eventFilters.id, + ENDPOINT_ARTIFACT_LISTS.hostIsolationExceptions.id, HOST_ISOLATION_EXCEPTIONS_LIST_DEFINITION ); } From e65294eb8383fd3f8fd24f7358712d258949e12b Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Wed, 11 Sep 2024 15:12:47 +0200 Subject: [PATCH 3/7] do not display the tab under certain conditions --- ..._host_isolation_exceptions_access.test.tsx | 74 +++++++++++ .../use_host_isolation_exceptions_access.tsx | 36 ++++++ .../pages/policy/view/policy_details.test.tsx | 116 ++++++++++-------- .../pages/policy/view/tabs/policy_tabs.tsx | 40 ++++-- 4 files changed, 205 insertions(+), 61 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx create mode 100644 x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx diff --git a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx new file mode 100644 index 0000000000000..a4d2953203a14 --- /dev/null +++ b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { useHostIsolationExceptionsAccess } from './use_host_isolation_exceptions_access'; +import { checkArtifactHasData } from '../../services/exceptions_list/check_artifact_has_data'; + +jest.mock('../../services/exceptions_list/check_artifact_has_data', () => ({ + checkArtifactHasData: jest.fn(), +})); + +const mockArtifactHasData = (hasData = true) => { + (checkArtifactHasData as jest.Mock).mockResolvedValueOnce(hasData); +}; + +describe('useHostIsolationExceptionsAccess', () => { + const mockApiClient = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const setupHook = (canAccess: boolean, canRead: boolean) => { + return renderHook(() => useHostIsolationExceptionsAccess(canAccess, canRead, mockApiClient)); + }; + + test('should set access to true if canAccessHostIsolationExceptions is true', async () => { + const { result, waitFor } = setupHook(true, false); + + await waitFor(() => expect(result.current).toBe(true)); + }); + + test('should check for artifact data if canReadHostIsolationExceptions is true and canAccessHostIsolationExceptions is false', async () => { + mockArtifactHasData(); + + const { result, waitFor } = setupHook(false, true); + + await waitFor(() => { + expect(checkArtifactHasData).toHaveBeenCalledWith(mockApiClient()); + expect(result.current).toBe(true); + }); + }); + + test('should set access to false if canReadHostIsolationExceptions is true but no artifact data exists', async () => { + mockArtifactHasData(false); + + const { result, waitFor } = setupHook(false, true); + + await waitFor(() => { + expect(checkArtifactHasData).toHaveBeenCalledWith(mockApiClient()); + expect(result.current).toBe(false); + }); + }); + + test('should set access to false if neither canAccessHostIsolationExceptions nor canReadHostIsolationExceptions is true', async () => { + const { result, waitFor } = setupHook(false, false); + await waitFor(() => { + expect(result.current).toBe(false); + }); + }); + + test('should not call checkArtifactHasData if canAccessHostIsolationExceptions is true', async () => { + const { result, waitFor } = setupHook(true, true); + + await waitFor(() => { + expect(checkArtifactHasData).not.toHaveBeenCalled(); + expect(result.current).toBe(true); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx new file mode 100644 index 0000000000000..3829dc00929af --- /dev/null +++ b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useEffect, useState } from 'react'; +import { checkArtifactHasData } from '../../services/exceptions_list/check_artifact_has_data'; +import type { ExceptionsListApiClient } from '../../services/exceptions_list/exceptions_list_api_client'; + +export const useHostIsolationExceptionsAccess = ( + canAccessHostIsolationExceptions: boolean, + canReadHostIsolationExceptions: boolean, + apiClient: () => ExceptionsListApiClient +) => { + const [hasAccess, setHasAccess] = useState(null); + + useEffect(() => { + (async () => { + // canAccessHostIsolationExceptions is a paid feature, so the tab should always be displayed. + // canReadHostIsolationExceptions, however, is not a paid feature, which allows users to view and delete exceptions in case of a downgrade. + // In such cases, the tab should be visible only if there is existing data. + if (canAccessHostIsolationExceptions) { + setHasAccess(true); + } else if (canReadHostIsolationExceptions) { + const result = await checkArtifactHasData(apiClient()); + setHasAccess(result); + } else { + setHasAccess(false); + } + })(); + }, [canAccessHostIsolationExceptions, canReadHostIsolationExceptions, apiClient]); + + return hasAccess; +}; diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx index a41cd65f9db3c..67a867908545e 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx @@ -30,12 +30,15 @@ import { PolicyDetails } from './policy_details'; import { APP_UI_ID } from '../../../../../common/constants'; import { createLicenseServiceMock } from '../../../../../common/license/mocks'; import { licenseService as licenseServiceMocked } from '../../../../common/hooks/__mocks__/use_license'; +import { useHostIsolationExceptionsAccess } from '../../../hooks/artifacts/use_host_isolation_exceptions_access'; jest.mock('../../../../common/components/user_privileges'); jest.mock('../../../../common/hooks/use_license'); +jest.mock('../../../hooks/artifacts/use_host_isolation_exceptions_access'); const useUserPrivilegesMock = useUserPrivileges as jest.Mock; const useLicenseMock = _useLicense as jest.Mock; +const useHostIsolationExceptionsAccessMock = useHostIsolationExceptionsAccess as jest.Mock; describe('Policy Details', () => { const policyDetailsPathUrl = getPolicyDetailPath('1'); @@ -104,7 +107,7 @@ describe('Policy Details', () => { http.get.mockImplementation((...args) => { const [path] = args; if (typeof path === 'string') { - // GET datasouce + // GET datasource if (path === `${PACKAGE_POLICY_API_ROOT}/1`) { asyncActions = asyncActions.then(async (): Promise => sleep()); return Promise.resolve({ @@ -210,13 +213,22 @@ describe('Policy Details', () => { expect(eventFiltersTab.text()).toBe('Event filters'); }); - it('should display the host isolation exceptions tab', async () => { + it('should display the host isolation exceptions tab if user have access', async () => { + useHostIsolationExceptionsAccessMock.mockReturnValue(true); policyView = render(); await asyncActions; policyView.update(); - const tab = policyView.find('button#hostIsolationExceptions'); + const tab = policyView.find('button[data-test-subj="policyHostIsolationExceptionsTab"]'); expect(tab).toHaveLength(1); - expect(tab.text()).toBe('Host isolation exceptions'); + }); + + it("shouldn't display when user doesn't have access", async () => { + useHostIsolationExceptionsAccessMock.mockReturnValue(false); + policyView = render(); + await asyncActions; + policyView.update(); + const tab = policyView.find('button[data-test-subj="policyHostIsolationExceptionsTab"]'); + expect(tab).toHaveLength(0); }); it('should display the protection updates tab', async () => { @@ -247,55 +259,55 @@ describe('Policy Details', () => { const tab = policyView.find('button#protectionUpdates'); expect(tab).toHaveLength(0); }); - }); - describe('without required permissions', () => { - const renderWithPrivilege = async (privilege: string) => { - useUserPrivilegesMock.mockReturnValue({ - endpointPrivileges: { - loading: false, - [privilege]: false, - }, - }); - policyView = render(); - await asyncActions; - policyView.update(); - }; - - it.each([ - ['trusted apps', 'canReadTrustedApplications', 'trustedApps'], - ['event filters', 'canReadEventFilters', 'eventFilters'], - ['host isolation exeptions', 'canReadHostIsolationExceptions', 'hostIsolationExceptions'], - ['blocklist', 'canReadBlocklist', 'blocklists'], - ])( - 'should not display the %s tab with no privileges', - async (_: string, privilege: string, selector: string) => { - await renderWithPrivilege(privilege); - expect(policyView.find(`button#${selector}`)).toHaveLength(0); - } - ); - - it.each([ - ['trusted apps', 'canReadTrustedApplications', getPolicyTrustedAppsPath('1')], - ['event filters', 'canReadEventFilters', getPolicyEventFiltersPath('1')], - [ - 'host isolation exeptions', - 'canReadHostIsolationExceptions', - getPolicyHostIsolationExceptionsPath('1'), - ], - ['blocklist', 'canReadBlocklist', getPolicyBlocklistsPath('1')], - ])( - 'should redirect to policy details when no %s required privileges', - async (_: string, privilege: string, path: string) => { - history.push(path); - await renderWithPrivilege(privilege); - expect(history.location.pathname).toBe(policyDetailsPathUrl); - expect(coreStart.notifications.toasts.addDanger).toHaveBeenCalledTimes(1); - expect(coreStart.notifications.toasts.addDanger).toHaveBeenCalledWith( - 'You do not have the required Kibana permissions to use the given artifact.' - ); - } - ); + describe('without required permissions', () => { + const renderWithoutPrivilege = async (privilege: string) => { + useUserPrivilegesMock.mockReturnValue({ + endpointPrivileges: { + loading: false, + [privilege]: false, + }, + }); + policyView = render(); + await asyncActions; + policyView.update(); + }; + + it.each([ + ['trusted apps', 'canReadTrustedApplications', 'trustedApps'], + ['event filters', 'canReadEventFilters', 'eventFilters'], + ['host isolation exeptions', 'canReadHostIsolationExceptions', 'hostIsolationExceptions'], + ['blocklist', 'canReadBlocklist', 'blocklists'], + ])( + 'should not display the %s tab with no privileges', + async (_: string, privilege: string, selector: string) => { + await renderWithoutPrivilege(privilege); + expect(policyView.find(`button#${selector}`)).toHaveLength(0); + } + ); + + it.each([ + ['trusted apps', 'canReadTrustedApplications', getPolicyTrustedAppsPath('1')], + ['event filters', 'canReadEventFilters', getPolicyEventFiltersPath('1')], + [ + 'host isolation exeptions', + 'canReadHostIsolationExceptions', + getPolicyHostIsolationExceptionsPath('1'), + ], + ['blocklist', 'canReadBlocklist', getPolicyBlocklistsPath('1')], + ])( + 'should redirect to policy details when no %s required privileges', + async (_: string, privilege: string, path: string) => { + history.push(path); + await renderWithoutPrivilege(privilege); + expect(history.location.pathname).toBe(policyDetailsPathUrl); + expect(coreStart.notifications.toasts.addDanger).toHaveBeenCalledTimes(1); + expect(coreStart.notifications.toasts.addDanger).toHaveBeenCalledWith( + 'You do not have the required Kibana permissions to use the given artifact.' + ); + } + ); + }); }); }); }); diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx index cb480615d27a5..a7eac40da90e6 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx @@ -58,6 +58,7 @@ import { SEARCHABLE_FIELDS as EVENT_FILTERS_SEARCHABLE_FIELDS } from '../../../e import { SEARCHABLE_FIELDS as HOST_ISOLATION_EXCEPTIONS_SEARCHABLE_FIELDS } from '../../../host_isolation_exceptions/constants'; import { SEARCHABLE_FIELDS as BLOCKLISTS_SEARCHABLE_FIELDS } from '../../../blocklist/constants'; import type { PolicyDetailsRouteState } from '../../../../../../common/endpoint/types'; +import { useHostIsolationExceptionsAccess } from '../../../../hooks/artifacts/use_host_isolation_exceptions_access'; enum PolicyTabKeys { SETTINGS = 'settings', @@ -118,6 +119,7 @@ export const PolicyTabs = React.memo(() => { canWriteTrustedApplications, canReadEventFilters, canWriteEventFilters, + canAccessHostIsolationExceptions, canReadHostIsolationExceptions, canWriteHostIsolationExceptions, canReadBlocklist, @@ -131,12 +133,35 @@ export const PolicyTabs = React.memo(() => { ); const isEnterprise = useLicense().isEnterprise(); const isProtectionUpdatesEnabled = isEnterprise && isProtectionUpdatesFeatureEnabled; + + const getHostIsolationExceptionsApiClientInstance = useCallback( + () => HostIsolationExceptionsApiClient.getInstance(http), + [http] + ); + + const hasAccessToHostIsolationExceptions = useHostIsolationExceptionsAccess( + canAccessHostIsolationExceptions, + canReadHostIsolationExceptions, + getHostIsolationExceptionsApiClientInstance + ); + + const hostIsolationExceptionsAccessLoading = hasAccessToHostIsolationExceptions === null; + // move the user out of this route if they can't access it useEffect(() => { + if (hostIsolationExceptionsAccessLoading) { + return; + } + + const redirectHostIsolationException = + isInHostIsolationExceptionsTab && + (!canReadHostIsolationExceptions || + (!hostIsolationExceptionsAccessLoading && !hasAccessToHostIsolationExceptions)); + if ( (isInTrustedAppsTab && !canReadTrustedApplications) || (isInEventFiltersTab && !canReadEventFilters) || - (isInHostIsolationExceptionsTab && !canReadHostIsolationExceptions) || + redirectHostIsolationException || (isInBlocklistsTab && !canReadBlocklist) ) { history.replace(getPolicyDetailPath(policyId)); @@ -152,7 +177,9 @@ export const PolicyTabs = React.memo(() => { canReadEventFilters, canReadHostIsolationExceptions, canReadTrustedApplications, + hasAccessToHostIsolationExceptions, history, + hostIsolationExceptionsAccessLoading, isInBlocklistsTab, isInEventFiltersTab, isInHostIsolationExceptionsTab, @@ -172,11 +199,6 @@ export const PolicyTabs = React.memo(() => { [http] ); - const getHostIsolationExceptionsApiClientInstance = useCallback( - () => HostIsolationExceptionsApiClient.getInstance(http), - [http] - ); - const getBlocklistsApiClientInstance = useCallback( () => BlocklistsApiClient.getInstance(http), [http] @@ -298,7 +320,7 @@ export const PolicyTabs = React.memo(() => { 'data-test-subj': 'policyEventFiltersTab', } : undefined, - [PolicyTabKeys.HOST_ISOLATION_EXCEPTIONS]: canReadHostIsolationExceptions + [PolicyTabKeys.HOST_ISOLATION_EXCEPTIONS]: hasAccessToHostIsolationExceptions ? { id: PolicyTabKeys.HOST_ISOLATION_EXCEPTIONS, name: i18n.translate( @@ -379,7 +401,7 @@ export const PolicyTabs = React.memo(() => { canReadEventFilters, getEventFiltersApiClientInstance, canWriteEventFilters, - canReadHostIsolationExceptions, + hasAccessToHostIsolationExceptions, getHostIsolationExceptionsApiClientInstance, canWriteHostIsolationExceptions, canReadBlocklist, @@ -485,7 +507,7 @@ export const PolicyTabs = React.memo(() => { }, [changeTab, unsavedChangesModal.nextTab]); // show loader for privileges validation - if (privilegesLoading) { + if (privilegesLoading || hostIsolationExceptionsAccessLoading) { return ; } From 0d736cd51a0e33c321e17c419413278c8357fa71 Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Mon, 16 Sep 2024 11:07:56 +0200 Subject: [PATCH 4/7] cr --- .../use_host_isolation_exceptions_access.tsx | 21 ++++++++++------ .../pages/policy/view/tabs/policy_tabs.tsx | 24 +++++++++---------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx index 3829dc00929af..daccba91bccf0 100644 --- a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx +++ b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.tsx @@ -12,25 +12,32 @@ import type { ExceptionsListApiClient } from '../../services/exceptions_list/exc export const useHostIsolationExceptionsAccess = ( canAccessHostIsolationExceptions: boolean, canReadHostIsolationExceptions: boolean, - apiClient: () => ExceptionsListApiClient -) => { + getApiClient: () => ExceptionsListApiClient +): { + hasAccessToHostIsolationExceptions: boolean; + isHostIsolationExceptionsAccessLoading: boolean; +} => { const [hasAccess, setHasAccess] = useState(null); useEffect(() => { (async () => { - // canAccessHostIsolationExceptions is a paid feature, so the tab should always be displayed. - // canReadHostIsolationExceptions, however, is not a paid feature, which allows users to view and delete exceptions in case of a downgrade. + // Host isolation exceptions is a paid feature and therefore: + // canAccessHostIsolationExceptions signifies if the user has required license to access the feature. + // canReadHostIsolationExceptions, however, is a privilege that allows the user to read and delete the data even if the license is not sufficient (downgrade scenario). // In such cases, the tab should be visible only if there is existing data. if (canAccessHostIsolationExceptions) { setHasAccess(true); } else if (canReadHostIsolationExceptions) { - const result = await checkArtifactHasData(apiClient()); + const result = await checkArtifactHasData(getApiClient()); setHasAccess(result); } else { setHasAccess(false); } })(); - }, [canAccessHostIsolationExceptions, canReadHostIsolationExceptions, apiClient]); + }, [canAccessHostIsolationExceptions, canReadHostIsolationExceptions, getApiClient]); - return hasAccess; + return { + hasAccessToHostIsolationExceptions: !!hasAccess, + isHostIsolationExceptionsAccessLoading: hasAccess === null, + }; }; diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx index a7eac40da90e6..c350f91c914d6 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/tabs/policy_tabs.tsx @@ -124,7 +124,7 @@ export const PolicyTabs = React.memo(() => { canWriteHostIsolationExceptions, canReadBlocklist, canWriteBlocklist, - loading: privilegesLoading, + loading: isPrivilegesLoading, } = useUserPrivileges().endpointPrivileges; const { state: routeState = {} } = useLocation(); @@ -139,24 +139,23 @@ export const PolicyTabs = React.memo(() => { [http] ); - const hasAccessToHostIsolationExceptions = useHostIsolationExceptionsAccess( - canAccessHostIsolationExceptions, - canReadHostIsolationExceptions, - getHostIsolationExceptionsApiClientInstance - ); - - const hostIsolationExceptionsAccessLoading = hasAccessToHostIsolationExceptions === null; + const { hasAccessToHostIsolationExceptions, isHostIsolationExceptionsAccessLoading } = + useHostIsolationExceptionsAccess( + canAccessHostIsolationExceptions, + canReadHostIsolationExceptions, + getHostIsolationExceptionsApiClientInstance + ); // move the user out of this route if they can't access it useEffect(() => { - if (hostIsolationExceptionsAccessLoading) { + if (isHostIsolationExceptionsAccessLoading || isPrivilegesLoading) { return; } const redirectHostIsolationException = isInHostIsolationExceptionsTab && (!canReadHostIsolationExceptions || - (!hostIsolationExceptionsAccessLoading && !hasAccessToHostIsolationExceptions)); + (!isHostIsolationExceptionsAccessLoading && !hasAccessToHostIsolationExceptions)); if ( (isInTrustedAppsTab && !canReadTrustedApplications) || @@ -179,12 +178,13 @@ export const PolicyTabs = React.memo(() => { canReadTrustedApplications, hasAccessToHostIsolationExceptions, history, - hostIsolationExceptionsAccessLoading, + isHostIsolationExceptionsAccessLoading, isInBlocklistsTab, isInEventFiltersTab, isInHostIsolationExceptionsTab, isInProtectionUpdatesTab, isInTrustedAppsTab, + isPrivilegesLoading, policyId, toasts, ]); @@ -507,7 +507,7 @@ export const PolicyTabs = React.memo(() => { }, [changeTab, unsavedChangesModal.nextTab]); // show loader for privileges validation - if (privilegesLoading || hostIsolationExceptionsAccessLoading) { + if (isPrivilegesLoading || isHostIsolationExceptionsAccessLoading) { return ; } From 7dcf947ea730e5bd9dbb2db87ea8de183c258db0 Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Mon, 16 Sep 2024 17:14:24 +0200 Subject: [PATCH 5/7] tests aligned --- .../use_host_isolation_exceptions_access.test.tsx | 10 +++++----- .../pages/policy/view/policy_details.test.tsx | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx index a4d2953203a14..aa886d7b0b752 100644 --- a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx +++ b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx @@ -31,7 +31,7 @@ describe('useHostIsolationExceptionsAccess', () => { test('should set access to true if canAccessHostIsolationExceptions is true', async () => { const { result, waitFor } = setupHook(true, false); - await waitFor(() => expect(result.current).toBe(true)); + await waitFor(() => expect(result.current.hasAccessToHostIsolationExceptions).toBe(true)); }); test('should check for artifact data if canReadHostIsolationExceptions is true and canAccessHostIsolationExceptions is false', async () => { @@ -41,7 +41,7 @@ describe('useHostIsolationExceptionsAccess', () => { await waitFor(() => { expect(checkArtifactHasData).toHaveBeenCalledWith(mockApiClient()); - expect(result.current).toBe(true); + expect(result.current.hasAccessToHostIsolationExceptions).toBe(true); }); }); @@ -52,14 +52,14 @@ describe('useHostIsolationExceptionsAccess', () => { await waitFor(() => { expect(checkArtifactHasData).toHaveBeenCalledWith(mockApiClient()); - expect(result.current).toBe(false); + expect(result.current.hasAccessToHostIsolationExceptions).toBe(false); }); }); test('should set access to false if neither canAccessHostIsolationExceptions nor canReadHostIsolationExceptions is true', async () => { const { result, waitFor } = setupHook(false, false); await waitFor(() => { - expect(result.current).toBe(false); + expect(result.current.hasAccessToHostIsolationExceptions).toBe(false); }); }); @@ -68,7 +68,7 @@ describe('useHostIsolationExceptionsAccess', () => { await waitFor(() => { expect(checkArtifactHasData).not.toHaveBeenCalled(); - expect(result.current).toBe(true); + expect(result.current.hasAccessToHostIsolationExceptions).toBe(true); }); }); }); diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx index 67a867908545e..22c7ae7bfcc26 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx @@ -103,6 +103,10 @@ describe('Policy Details', () => { policyPackagePolicy.policy_id = policyPackagePolicy.policy_ids[0]; const policyListApiHandlers = policyListApiPathHandlers(); + useHostIsolationExceptionsAccessMock.mockReturnValue({ + hasAccessToHostIsolationExceptions: true, + isHostIsolationExceptionsAccessLoading: false, + }); http.get.mockImplementation((...args) => { const [path] = args; @@ -214,7 +218,6 @@ describe('Policy Details', () => { }); it('should display the host isolation exceptions tab if user have access', async () => { - useHostIsolationExceptionsAccessMock.mockReturnValue(true); policyView = render(); await asyncActions; policyView.update(); @@ -223,7 +226,10 @@ describe('Policy Details', () => { }); it("shouldn't display when user doesn't have access", async () => { - useHostIsolationExceptionsAccessMock.mockReturnValue(false); + useHostIsolationExceptionsAccessMock.mockReturnValue({ + hasAccessToHostIsolationExceptions: false, + isHostIsolationExceptionsAccessLoading: false, + }); policyView = render(); await asyncActions; policyView.update(); @@ -246,6 +252,10 @@ describe('Policy Details', () => { licenseServiceMock.isEnterprise.mockReturnValue(false); useLicenseMock.mockReturnValue(licenseServiceMock); + useHostIsolationExceptionsAccessMock.mockReturnValue({ + hasAccessToHostIsolationExceptions: false, + isHostIsolationExceptionsAccessLoading: false, + }); }); afterEach(() => { From c554f89c6d10759ff86a1674feec4d9701cdd937 Mon Sep 17 00:00:00 2001 From: Konrad Szwarc Date: Fri, 20 Sep 2024 13:01:24 +0200 Subject: [PATCH 6/7] Update x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx Co-authored-by: Ash <1849116+ashokaditya@users.noreply.github.com> --- .../public/management/pages/policy/view/policy_details.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx index 22c7ae7bfcc26..45e7e56a337eb 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx @@ -225,7 +225,7 @@ describe('Policy Details', () => { expect(tab).toHaveLength(1); }); - it("shouldn't display when user doesn't have access", async () => { + it("shouldn't display the host isolation exceptions tab when user doesn't have access", async () => { useHostIsolationExceptionsAccessMock.mockReturnValue({ hasAccessToHostIsolationExceptions: false, isHostIsolationExceptionsAccessLoading: false, From 5dc3427cd22692d9e46eb886f0bc123e6d09402f Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Fri, 20 Sep 2024 13:44:16 +0200 Subject: [PATCH 7/7] added test coverage for loading state --- ..._host_isolation_exceptions_access.test.tsx | 10 +++++++ .../pages/policy/view/policy_details.test.tsx | 28 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx index aa886d7b0b752..2bd673f68095f 100644 --- a/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx +++ b/x-pack/plugins/security_solution/public/management/hooks/artifacts/use_host_isolation_exceptions_access.test.tsx @@ -71,4 +71,14 @@ describe('useHostIsolationExceptionsAccess', () => { expect(result.current.hasAccessToHostIsolationExceptions).toBe(true); }); }); + + test('should set loading state correctly while checking access', async () => { + const { result, waitFor } = setupHook(false, true); + + expect(result.current.isHostIsolationExceptionsAccessLoading).toBe(true); + + await waitFor(() => { + expect(result.current.isHostIsolationExceptionsAccessLoading).toBe(false); + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx index 45e7e56a337eb..ffcee81a3cb8f 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_details.test.tsx @@ -142,6 +142,34 @@ describe('Policy Details', () => { history.push(policyDetailsPathUrl); }); + it('should NOT display tabs when Host Isolation Exceptions access is loading', async () => { + useHostIsolationExceptionsAccessMock.mockReturnValue({ + hasAccessToHostIsolationExceptions: false, + isHostIsolationExceptionsAccessLoading: true, + }); + policyView = render(); + await asyncActions; + policyView.update(); + const tab = policyView.find('[data-test-subj="policyTabs"]'); + expect(tab).toHaveLength(0); + const loader = policyView.find('span[data-test-subj="privilegesLoading"]'); + expect(loader).toHaveLength(1); + }); + + it('should display tabs when Host Isolation Exceptions access is not loading', async () => { + useHostIsolationExceptionsAccessMock.mockReturnValue({ + hasAccessToHostIsolationExceptions: true, + isHostIsolationExceptionsAccessLoading: false, + }); + policyView = render(); + await asyncActions; + policyView.update(); + const tab = policyView.find('div[data-test-subj="policyTabs"]'); + expect(tab).toHaveLength(1); + const loader = policyView.find('div[data-test-subj="privilegesLoading"]'); + expect(loader).toHaveLength(0); + }); + it('should NOT display timeline', async () => { policyView = render(); await asyncActions;