From 9d7b4590e23f3693b41dd05a5e0b4a418ec1d865 Mon Sep 17 00:00:00 2001 From: Jordan <51442161+JordanSh@users.noreply.github.com> Date: Tue, 3 May 2022 22:07:15 +0300 Subject: [PATCH] [Cloud Posture] Display and save rules per benchmark (#131412) --- .../pages/rules/rules_container.test.tsx | 112 ++++++++++++++---- .../public/pages/rules/rules_container.tsx | 17 ++- .../public/pages/rules/rules_flyout.tsx | 2 +- .../public/pages/rules/rules_table.tsx | 2 +- .../public/pages/rules/use_csp_rules.ts | 35 ++++-- .../update_rules_configuration.ts | 4 +- 6 files changed, 130 insertions(+), 42 deletions(-) diff --git a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.test.tsx b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.test.tsx index 7782be9c4917e..0787bc37c45e7 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.test.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.test.tsx @@ -13,6 +13,7 @@ import { useFindCspRules, useBulkUpdateCspRules, type RuleSavedObject } from './ import * as TEST_SUBJECTS from './test_subjects'; import { Chance } from 'chance'; import { TestProvider } from '../../test/test_provider'; +import { useParams } from 'react-router-dom'; const chance = new Chance(); @@ -21,6 +22,11 @@ jest.mock('./use_csp_rules', () => ({ useBulkUpdateCspRules: jest.fn(), })); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: jest.fn(), +})); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, @@ -32,21 +38,54 @@ const getWrapper = ({ children }) => {children}; -const getRuleMock = ({ id = chance.guid(), enabled }: { id?: string; enabled: boolean }) => +const getRuleMock = ({ + packagePolicyId = chance.guid(), + policyId = chance.guid(), + savedObjectId = chance.guid(), + id = chance.guid(), + enabled, +}: { + packagePolicyId?: string; + policyId?: string; + savedObjectId?: string; + id?: string; + enabled: boolean; +}) => ({ - id, + id: savedObjectId, updatedAt: chance.date().toISOString(), attributes: { id, name: chance.sentence(), + package_policy_id: packagePolicyId, + policy_id: policyId, enabled, }, } as RuleSavedObject); +const getSavedObjectRuleEnable = ( + rule: RuleSavedObject, + enabled: RuleSavedObject['attributes']['enabled'] +) => ({ + ...rule, + attributes: { + ...rule.attributes, + enabled, + }, +}); + +const params = { + policyId: chance.guid(), + packagePolicyId: chance.guid(), +}; + describe('', () => { beforeEach(() => { queryClient.clear(); jest.clearAllMocks(); + + (useParams as jest.Mock).mockReturnValue(params); + (useBulkUpdateCspRules as jest.Mock).mockReturnValue({ status: 'idle', mutate: jest.fn(), @@ -102,6 +141,13 @@ describe('', () => { const switchId1 = TEST_SUBJECTS.getCspRulesTableItemSwitchTestId(rule1.id); const switchId2 = TEST_SUBJECTS.getCspRulesTableItemSwitchTestId(rule2.id); + expect(screen.getByTestId(switchId1).getAttribute('aria-checked')).toEqual( + rule1.attributes.enabled.toString() + ); + expect(screen.getByTestId(switchId2).getAttribute('aria-checked')).toEqual( + rule2.attributes.enabled.toString() + ); + fireEvent.click(screen.getByTestId(switchId1)); fireEvent.click(screen.getByTestId(switchId2)); @@ -154,6 +200,7 @@ describe('', () => { it('updates rules with local changes done by non-bulk toggles', () => { const Wrapper = getWrapper(); + const rule1 = getRuleMock({ enabled: false }); const rule2 = getRuleMock({ enabled: true }); const rule3 = getRuleMock({ enabled: true }); @@ -184,10 +231,13 @@ describe('', () => { fireEvent.click(screen.getByTestId(TEST_SUBJECTS.CSP_RULES_SAVE_BUTTON)); expect(mutate).toHaveBeenCalledTimes(1); - expect(mutate).toHaveBeenCalledWith([ - { ...rule1.attributes, enabled: !rule1.attributes.enabled }, - { ...rule2.attributes, enabled: !rule2.attributes.enabled }, - ]); + expect(mutate).toHaveBeenCalledWith({ + packagePolicyId: params.packagePolicyId, + savedObjectRules: [ + getSavedObjectRuleEnable(rule1, !rule1.attributes.enabled), + getSavedObjectRuleEnable(rule2, !rule2.attributes.enabled), + ], + }); }); it('updates rules with local changes done by bulk toggles', () => { @@ -217,9 +267,10 @@ describe('', () => { fireEvent.click(screen.getByTestId(TEST_SUBJECTS.CSP_RULES_SAVE_BUTTON)); expect(mutate).toHaveBeenCalledTimes(1); - expect(mutate).toHaveBeenCalledWith([ - { ...rule1.attributes, enabled: !rule1.attributes.enabled }, - ]); + expect(mutate).toHaveBeenCalledWith({ + packagePolicyId: params.packagePolicyId, + savedObjectRules: [getSavedObjectRuleEnable(rule1, !rule1.attributes.enabled)], + }); }); it('only changes selected rules in bulk operations', () => { @@ -254,11 +305,14 @@ describe('', () => { fireEvent.click(screen.getByTestId(TEST_SUBJECTS.CSP_RULES_SAVE_BUTTON)); expect(mutate).toHaveBeenCalledTimes(1); - expect(mutate).toHaveBeenCalledWith([ - { ...rule1.attributes, enabled: !rule1.attributes.enabled }, - { ...rule4.attributes, enabled: !rule4.attributes.enabled }, - { ...rule5.attributes, enabled: !rule5.attributes.enabled }, - ]); + expect(mutate).toHaveBeenCalledWith({ + packagePolicyId: params.packagePolicyId, + savedObjectRules: [ + getSavedObjectRuleEnable(rule1, !rule1.attributes.enabled), + getSavedObjectRuleEnable(rule4, !rule4.attributes.enabled), + getSavedObjectRuleEnable(rule5, !rule5.attributes.enabled), + ], + }); }); it('updates rules with changes of both bulk/non-bulk toggles', () => { @@ -295,11 +349,14 @@ describe('', () => { fireEvent.click(screen.getByTestId(TEST_SUBJECTS.CSP_RULES_SAVE_BUTTON)); expect(mutate).toHaveBeenCalledTimes(1); - expect(mutate).toHaveBeenCalledWith([ - { ...rule1.attributes, enabled: !rule1.attributes.enabled }, - { ...rule4.attributes, enabled: !rule4.attributes.enabled }, - { ...rule5.attributes, enabled: !rule5.attributes.enabled }, - ]); + expect(mutate).toHaveBeenCalledWith({ + packagePolicyId: params.packagePolicyId, + savedObjectRules: [ + getSavedObjectRuleEnable(rule1, !rule1.attributes.enabled), + getSavedObjectRuleEnable(rule4, !rule4.attributes.enabled), + getSavedObjectRuleEnable(rule5, !rule5.attributes.enabled), + ], + }); }); it('selects and updates all rules', async () => { @@ -329,12 +386,12 @@ describe('', () => { fireEvent.click(screen.getByTestId(TEST_SUBJECTS.CSP_RULES_SAVE_BUTTON)); expect(mutate).toHaveBeenCalledTimes(1); - expect(mutate).toHaveBeenCalledWith( - rules.map((rule) => ({ - ...rule.attributes, - enabled: !enabled, - })) - ); + expect(mutate).toHaveBeenCalledWith({ + packagePolicyId: params.packagePolicyId, + savedObjectRules: rules.map((rule) => + getSavedObjectRuleEnable(rule, !rule.attributes.enabled) + ), + }); }); it('updates the rules from within the flyout', () => { @@ -370,6 +427,9 @@ describe('', () => { const { mutate } = useBulkUpdateCspRules(); expect(mutate).toHaveBeenCalledTimes(1); - expect(mutate).toHaveBeenCalledWith([{ ...rule.attributes, enabled: !enabled }]); + expect(mutate).toHaveBeenCalledWith({ + packagePolicyId: params.packagePolicyId, + savedObjectRules: [getSavedObjectRuleEnable(rule, !rule.attributes.enabled)], + }); }); }); diff --git a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.tsx b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.tsx index cf77fba3c513f..e7bc2d5c1b344 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_container.tsx @@ -16,6 +16,7 @@ import { import { useParams } from 'react-router-dom'; import { FormattedMessage } from '@kbn/i18n-react'; import { pagePathGetters } from '@kbn/fleet-plugin/public'; +import { cspRuleAssetSavedObjectType } from '../../../common/schemas/csp_rule'; import { extractErrorMessage, isNonNullable } from '../../../common/utils/helpers'; import { RulesTable } from './rules_table'; import { RulesBottomBar } from './rules_bottom_bar'; @@ -81,7 +82,7 @@ const getRulesPageData = ( error: error ? extractErrorMessage(error) : undefined, all_rules: rules, rules_map: new Map(rules.map((rule) => [rule.id, rule])), - rules_page: page.map((rule) => changedRules.get(rule.attributes.id) || rule), + rules_page: page.map((rule) => changedRules.get(rule.id) || rule), total: data?.total || 0, lastModified: getLastModified(rules) || null, }; @@ -107,9 +108,15 @@ export const RulesContainer = () => { const [selectedRuleId, setSelectedRuleId] = useState(null); const [isAllSelected, setIsAllSelected] = useState(false); const [visibleSelectedRulesIds, setVisibleSelectedRulesIds] = useState([]); - const [rulesQuery, setRulesQuery] = useState({ page: 0, perPage: 5, search: '' }); + const [rulesQuery, setRulesQuery] = useState({ + filter: `${cspRuleAssetSavedObjectType}.attributes.policy_id: "${params.policyId}" and ${cspRuleAssetSavedObjectType}.attributes.package_policy_id: "${params.packagePolicyId}"`, + search: '', + page: 0, + perPage: 5, + }); const { data, status, error, refetch } = useFindCspRules({ + filter: rulesQuery.filter, search: getSimpleQueryString(rulesQuery.search), page: 1, perPage: MAX_ITEMS_PER_PAGE, @@ -152,7 +159,11 @@ export const RulesContainer = () => { const toggleRule = (rule: RuleSavedObject) => toggleRules([rule], !rule.attributes.enabled); - const bulkUpdateRules = () => bulkUpdate([...changedRules].map(([, rule]) => rule.attributes)); + const bulkUpdateRules = () => + bulkUpdate({ + savedObjectRules: [...changedRules].map(([, savedObjectRule]) => savedObjectRule), + packagePolicyId: params.packagePolicyId, + }); const discardChanges = useCallback(() => setChangedRules(new Map()), []); diff --git a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_flyout.tsx b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_flyout.tsx index 54657660c7aa4..f21ca53b76649 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_flyout.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_flyout.tsx @@ -146,7 +146,7 @@ const RuleOverviewTab = ({ rule, toggleRule }: { rule: RuleSavedObject; toggleRu label={TEXT.ACTIVATED} checked={rule.attributes.enabled} onChange={toggleRule} - data-test-subj={TEST_SUBJECTS.getCspRulesTableItemSwitchTestId(rule.attributes.id)} + data-test-subj={TEST_SUBJECTS.getCspRulesTableItemSwitchTestId(rule.id)} /> diff --git a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_table.tsx b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_table.tsx index c577daebb58a6..704b87697094c 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_table.tsx +++ b/x-pack/plugins/cloud_security_posture/public/pages/rules/rules_table.tsx @@ -148,7 +148,7 @@ const getColumns = ({ label={enabled ? TEXT.DISABLE : TEXT.ENABLE} checked={enabled} onChange={() => toggleRule(rule)} - data-test-subj={TEST_SUBJECTS.getCspRulesTableItemSwitchTestId(rule.attributes.id)} + data-test-subj={TEST_SUBJECTS.getCspRulesTableItemSwitchTestId(rule.id)} /> ), diff --git a/x-pack/plugins/cloud_security_posture/public/pages/rules/use_csp_rules.ts b/x-pack/plugins/cloud_security_posture/public/pages/rules/use_csp_rules.ts index 87f06fe70df76..2ce088cd5c29b 100644 --- a/x-pack/plugins/cloud_security_posture/public/pages/rules/use_csp_rules.ts +++ b/x-pack/plugins/cloud_security_posture/public/pages/rules/use_csp_rules.ts @@ -7,6 +7,7 @@ import { useQuery, useMutation, useQueryClient } from 'react-query'; import { FunctionKeys } from 'utility-types'; import type { SavedObjectsFindOptions, SimpleSavedObject } from '@kbn/core/public'; +import { UPDATE_RULES_CONFIG_ROUTE_PATH } from '../../../common/constants'; import { cspRuleAssetSavedObjectType, type CspRuleSchema } from '../../../common/schemas/csp_rule'; import { useKibana } from '../../common/hooks/use_kibana'; import { UPDATE_FAILED } from './translations'; @@ -16,11 +17,14 @@ export type RuleSavedObject = Omit< FunctionKeys >; -export type RulesQuery = Required>; +export type RulesQuery = Required< + Pick +>; export type RulesQueryResult = ReturnType; -export const useFindCspRules = ({ search, page, perPage }: RulesQuery) => { +export const useFindCspRules = ({ search, page, perPage, filter }: RulesQuery) => { const { savedObjects } = useKibana().services; + return useQuery( [cspRuleAssetSavedObjectType, { search, page, perPage }], () => @@ -32,24 +36,37 @@ export const useFindCspRules = ({ search, page, perPage }: RulesQuery) => { // NOTE: 'name.raw' is a field mapping we defined on 'name' sortField: 'name.raw', perPage, + filter, }), { refetchOnWindowFocus: false } ); }; export const useBulkUpdateCspRules = () => { - const { savedObjects, notifications } = useKibana().services; + const { savedObjects, notifications, http } = useKibana().services; const queryClient = useQueryClient(); return useMutation( - (rules: CspRuleSchema[]) => - savedObjects.client.bulkUpdate( - rules.map((rule) => ({ + async ({ + savedObjectRules, + packagePolicyId, + }: { + savedObjectRules: RuleSavedObject[]; + packagePolicyId: CspRuleSchema['package_policy_id']; + }) => { + await savedObjects.client.bulkUpdate( + savedObjectRules.map((savedObjectRule) => ({ type: cspRuleAssetSavedObjectType, - id: rule.id, - attributes: rule, + id: savedObjectRule.id, + attributes: savedObjectRule.attributes, })) - ), + ); + await http.post(UPDATE_RULES_CONFIG_ROUTE_PATH, { + body: JSON.stringify({ + package_policy_id: packagePolicyId, + }), + }); + }, { onError: (err) => { if (err instanceof Error) notifications.toasts.addError(err, { title: UPDATE_FAILED }); diff --git a/x-pack/plugins/cloud_security_posture/server/routes/configuration/update_rules_configuration.ts b/x-pack/plugins/cloud_security_posture/server/routes/configuration/update_rules_configuration.ts index e9b9401d1373b..f0e0e3b8a2bb3 100644 --- a/x-pack/plugins/cloud_security_posture/server/routes/configuration/update_rules_configuration.ts +++ b/x-pack/plugins/cloud_security_posture/server/routes/configuration/update_rules_configuration.ts @@ -106,7 +106,7 @@ export const defineUpdateRulesConfigRoute = (router: CspRouter, cspContext: CspA router.post( { path: UPDATE_RULES_CONFIG_ROUTE_PATH, - validate: { query: configurationUpdateInputSchema }, + validate: { body: configurationUpdateInputSchema }, }, async (context, request, response) => { if (!(await context.fleet).authz.fleet.all) { @@ -118,7 +118,7 @@ export const defineUpdateRulesConfigRoute = (router: CspRouter, cspContext: CspA const esClient = coreContext.elasticsearch.client.asCurrentUser; const soClient = coreContext.savedObjects.client; const packagePolicyService = cspContext.service.packagePolicyService; - const packagePolicyId = request.query.package_policy_id; + const packagePolicyId = request.body.package_policy_id; if (!packagePolicyService) { throw new Error(`Failed to get Fleet services`);