From 8548c1dd73da14eeb0b7a8dcfe9f0e0c5d7c8cff Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 13 Aug 2020 14:47:09 -0400 Subject: [PATCH 1/8] wip using shared types, NewRule --- .../{public => }/common/fp_utils.test.ts | 0 .../lists/{public => }/common/fp_utils.ts | 0 x-pack/plugins/lists/common/shared_exports.ts | 2 + x-pack/plugins/lists/public/lists/api.ts | 2 +- x-pack/plugins/lists/public/shared_exports.ts | 1 + .../schemas/common/schemas.ts | 2 +- .../detection_engine/schemas/request/index.ts | 18 ++++++ .../schemas/response/index.ts | 13 +++++ .../common/shared_imports.ts | 2 + ...se_fetch_or_create_rule_exception_list.tsx | 3 +- .../rules/all_rules_tables/index.tsx | 5 +- .../containers/detection_engine/rules/api.ts | 57 +++++++++++++++++-- .../detection_engine/rules/persist_rule.tsx | 12 +++- .../detection_engine/rules/types.ts | 57 ++++++------------- .../rules/create/helpers.test.ts | 26 ++++++--- .../detection_engine/rules/create/helpers.ts | 8 +-- .../pages/detection_engine/rules/types.ts | 3 +- 17 files changed, 145 insertions(+), 66 deletions(-) rename x-pack/plugins/lists/{public => }/common/fp_utils.test.ts (100%) rename x-pack/plugins/lists/{public => }/common/fp_utils.ts (100%) create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/request/index.ts create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts diff --git a/x-pack/plugins/lists/public/common/fp_utils.test.ts b/x-pack/plugins/lists/common/fp_utils.test.ts similarity index 100% rename from x-pack/plugins/lists/public/common/fp_utils.test.ts rename to x-pack/plugins/lists/common/fp_utils.test.ts diff --git a/x-pack/plugins/lists/public/common/fp_utils.ts b/x-pack/plugins/lists/common/fp_utils.ts similarity index 100% rename from x-pack/plugins/lists/public/common/fp_utils.ts rename to x-pack/plugins/lists/common/fp_utils.ts diff --git a/x-pack/plugins/lists/common/shared_exports.ts b/x-pack/plugins/lists/common/shared_exports.ts index 1f6c65919b063a..d6a6e76333ce85 100644 --- a/x-pack/plugins/lists/common/shared_exports.ts +++ b/x-pack/plugins/lists/common/shared_exports.ts @@ -45,3 +45,5 @@ export { } from './schemas'; export { ENDPOINT_LIST_ID } from './constants'; + +export { toPromise, toError } from './fp_utils'; diff --git a/x-pack/plugins/lists/public/lists/api.ts b/x-pack/plugins/lists/public/lists/api.ts index 2b123280474dfb..93319318501d07 100644 --- a/x-pack/plugins/lists/public/lists/api.ts +++ b/x-pack/plugins/lists/public/lists/api.ts @@ -30,7 +30,7 @@ import { } from '../../common/schemas'; import { LIST_INDEX, LIST_ITEM_URL, LIST_PRIVILEGES_URL, LIST_URL } from '../../common/constants'; import { validateEither } from '../../common/shared_imports'; -import { toError, toPromise } from '../common/fp_utils'; +import { toError, toPromise } from '../../common/fp_utils'; import { ApiParams, diff --git a/x-pack/plugins/lists/public/shared_exports.ts b/x-pack/plugins/lists/public/shared_exports.ts index 16026a436f1549..0557dcb567e5c1 100644 --- a/x-pack/plugins/lists/public/shared_exports.ts +++ b/x-pack/plugins/lists/public/shared_exports.ts @@ -34,3 +34,4 @@ export { Pagination, UseExceptionListSuccess, } from './exceptions/types'; +export { toPromise, toError } from './common/fp_utils'; diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts index 2a0d1ef8b9dfde..b72b6e15c5e11f 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts @@ -325,7 +325,7 @@ export const sortFieldOrUndefined = t.union([sort_field, t.undefined]); export type SortFieldOrUndefined = t.TypeOf; export const sort_order = t.keyof({ asc: null, desc: null }); -export type sortOrder = t.TypeOf; +export type SortOrder = t.TypeOf; export const sortOrderOrUndefined = t.union([sort_order, t.undefined]); export type SortOrderOrUndefined = t.TypeOf; diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/index.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/index.ts new file mode 100644 index 00000000000000..abfbc391896430 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +export * from './add_prepackaged_rules_schema'; +export * from './create_rules_bulk_schema'; +export * from './create_rules_schema'; +export * from './export_rules_schema'; +export * from './find_rules_schema'; +export * from './import_rules_schema'; +export * from './patch_rules_bulk_schema'; +export * from './patch_rules_schema'; +export * from './query_rules_schema'; +export * from './query_signals_index_schema'; +export * from './set_signal_status_schema'; +export * from './update_rules_bulk_schema'; +export * from './update_rules_schema'; diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts new file mode 100644 index 00000000000000..6c22b8140e738f --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +export * from './error_schema'; +export * from './find_rules_schema'; +export * from './import_rules_schema'; +export * from './prepackaged_rules_schema'; +export * from './prepackaged_rules_status_schema'; +export * from './rules_bulk_schema'; +export * from './rules_schema'; +export * from './type_timeline_only_schema'; diff --git a/x-pack/plugins/security_solution/common/shared_imports.ts b/x-pack/plugins/security_solution/common/shared_imports.ts index e28d1969b39763..3bc112101af149 100644 --- a/x-pack/plugins/security_solution/common/shared_imports.ts +++ b/x-pack/plugins/security_solution/common/shared_imports.ts @@ -43,4 +43,6 @@ export { ExceptionListType, Type, ENDPOINT_LIST_ID, + toPromise, + toError, } from '../../lists/common'; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx index 0d367e03a799f3..c5be89ead95d74 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx @@ -9,6 +9,7 @@ import { HttpStart } from '../../../../../../../src/core/public'; import { Rule } from '../../../detections/containers/detection_engine/rules/types'; import { List, ListArray } from '../../../../common/detection_engine/schemas/types'; +import { RulesSchema } from '../../../../common/detection_engine/schemas/response'; import { fetchRuleById, patchRule, @@ -28,7 +29,7 @@ export type ReturnUseFetchOrCreateRuleExceptionList = [boolean, ExceptionListSch export interface UseFetchOrCreateRuleExceptionListProps { http: HttpStart; - ruleId: Rule['id']; + ruleId: RulesSchema['id']; exceptionListType: ExceptionListSchema['type']; onError: (arg: Error) => void; onSuccess?: (ruleWasChanged: boolean) => void; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx index 8fd3f648bc812c..582490ced33d17 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx @@ -20,7 +20,8 @@ import { RulesColumns, RuleStatusRowItemType, } from '../../../pages/detection_engine/rules/all/columns'; -import { Rule, Rules } from '../../../containers/detection_engine/rules/types'; +import { RulesSchema } from '../../../../../common/detection_engine/schemas/response'; +import { Rules } from '../../../containers/detection_engine/rules/types'; import { AllRulesTabs } from '../../../pages/detection_engine/rules/all'; // EuiBasicTable give me a hardtime with adding the ref attributes so I went the easy way @@ -36,7 +37,7 @@ export interface SortingType { } interface AllRulesTablesProps { - euiBasicTableSelectionProps: EuiTableSelectionType; + euiBasicTableSelectionProps: EuiTableSelectionType; hasNoPermissions: boolean; monitoringColumns: Array>; pagination: { diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index 3538d8ec8c9b95..10b117625e3d2f 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -3,6 +3,9 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +import { chain, fromEither, tryCatch } from 'fp-ts/lib/TaskEither'; +import { flow } from 'fp-ts/lib/function'; +import { pipe } from 'fp-ts/lib/pipeable'; import { HttpStart } from '../../../../../../../../src/core/public'; import { @@ -14,12 +17,13 @@ import { } from '../../../../../common/constants'; import { AddRulesProps, + UpdateRulesProps, + CreateRulesProps, DeleteRulesProps, DuplicateRulesProps, EnableRulesProps, FetchRulesProps, FetchRulesResponse, - NewRule, Rule, FetchRuleProps, BasicFetchProps, @@ -33,6 +37,14 @@ import { } from './types'; import { KibanaServices } from '../../../../common/lib/kibana'; import * as i18n from '../../../pages/detection_engine/rules/translations'; +import { RulesSchema, rulesSchema } from '../../../../../common/detection_engine/schemas/response'; +import { validateEither } from '../../../../../common'; +import { + UpdateRulesSchema, + createRulesSchema, + updateRulesSchema, +} from '../../../../../common/detection_engine/schemas/request'; +import { toError, toPromise } from '../../../../../common/shared_imports'; /** * Add provided Rule @@ -42,13 +54,50 @@ import * as i18n from '../../../pages/detection_engine/rules/translations'; * * @throws An error if response is not OK */ -export const addRule = async ({ rule, signal }: AddRulesProps): Promise => - KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { - method: rule.id != null ? 'PUT' : 'POST', +export const addRule = async ({ rule, signal }: AddRulesProps): Promise => { + if ((rule as UpdateRulesSchema).id != null) { + return updateRuleWithValidation({ rule, signal }); + } else { + return createRuleWithValidation({ rule, signal }); + } +}; + +const createRule = async ({ rule, signal }: CreateRulesProps): Promise => + KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { + method: 'POST', + body: JSON.stringify(rule), + signal, + }); + +const createRuleWithValidation = async ({ rule, signal }: CreateRulesProps): Promise => + pipe( + rule, + (body) => fromEither(validateEither(createRulesSchema, body)), + chain((payload) => tryCatch(() => createRule({ signal, rule: { ...payload } }), toError)), + chain((response) => fromEither(validateEither(rulesSchema, response))), + flow(toPromise) + ); + +export { createRuleWithValidation as createRule }; + +const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise => + KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { + method: 'PUT', body: JSON.stringify(rule), signal, }); +const updateRuleWithValidation = async ({ rule, signal }: UpdateRulesProps): Promise => + pipe( + rule, + (body) => fromEither(validateEither(updateRulesSchema, body)), + chain((payload) => tryCatch(() => updateRule({ signal, rule: { ...payload } }), toError)), + chain((response) => fromEither(validateEither(rulesSchema, response))), + flow(toPromise) + ); + +export { updateRuleWithValidation as updateRule }; + /** * Patch provided Rule * diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx index fd139d59c0a271..233a6cdb414aa3 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx @@ -7,20 +7,26 @@ import { useEffect, useState, Dispatch } from 'react'; import { errorToToaster, useStateToaster } from '../../../../common/components/toasters'; +import { + UpdateRulesSchema, + CreateRulesSchema, +} from '../../../../../common/detection_engine/schemas/request'; import { addRule as persistRule } from './api'; import * as i18n from './translations'; -import { NewRule } from './types'; interface PersistRuleReturn { isLoading: boolean; isSaved: boolean; } -export type ReturnPersistRule = [PersistRuleReturn, Dispatch]; +export type ReturnPersistRule = [ + PersistRuleReturn, + Dispatch +]; export const usePersistRule = (): ReturnPersistRule => { - const [rule, setRule] = useState(null); + const [rule, setRule] = useState(null); const [isSaved, setIsSaved] = useState(false); const [isLoading, setIsLoading] = useState(false); const [, dispatchToaster] = useStateToaster(); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts index 78d2e2a5b0c2f6..f65212a0590af7 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts @@ -16,12 +16,17 @@ import { severity_mapping, timestamp_override, threshold, + SortOrder, } from '../../../../../common/detection_engine/schemas/common/schemas'; import { listArray, listArrayOrUndefined, } from '../../../../../common/detection_engine/schemas/types'; -import { PatchRulesSchema } from '../../../../../common/detection_engine/schemas/request/patch_rules_schema'; +import { + CreateRulesSchema, + PatchRulesSchema, + UpdateRulesSchema, +} from '../../../../../common/detection_engine/schemas/request'; /** * Params is an "record", since it is a type of AlertActionParams which is action templates. @@ -36,48 +41,18 @@ export const action = t.exact( }) ); -export const NewRuleSchema = t.intersection([ - t.type({ - description: t.string, - enabled: t.boolean, - interval: t.string, - name: t.string, - risk_score: t.number, - severity: t.string, - type: RuleTypeSchema, - }), - t.partial({ - actions: t.array(action), - anomaly_threshold: t.number, - created_by: t.string, - false_positives: t.array(t.string), - filters: t.array(t.unknown), - from: t.string, - id: t.string, - index: t.array(t.string), - language: t.string, - machine_learning_job_id: t.string, - max_signals: t.number, - query: t.string, - references: t.array(t.string), - rule_id: t.string, - saved_id: t.string, - tags: t.array(t.string), - threat: t.array(t.unknown), - threshold, - throttle: t.union([t.string, t.null]), - to: t.string, - updated_by: t.string, - note: t.string, - exceptions_list: listArrayOrUndefined, - }), -]); +export interface CreateRulesProps { + rule: CreateRulesSchema; + signal: AbortSignal; +} -export const NewRulesSchema = t.array(NewRuleSchema); -export type NewRule = t.TypeOf; +export interface UpdateRulesProps { + rule: UpdateRulesSchema; + signal: AbortSignal; +} export interface AddRulesProps { - rule: NewRule; + rule: UpdateRulesSchema | CreateRulesSchema; signal: AbortSignal; } @@ -185,7 +160,7 @@ export interface FetchRulesProps { export interface FilterOptions { filter: string; sortField: string; - sortOrder: 'asc' | 'desc'; + sortOrder: SortOrder; showCustomRules?: boolean; showElasticRules?: boolean; tags?: string[]; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts index 4f2055424ca61d..35b418308952b0 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts @@ -5,7 +5,7 @@ */ import { List } from '../../../../../../common/detection_engine/schemas/types'; -import { NewRule } from '../../../../containers/detection_engine/rules'; +import { CreateRulesSchema } from '../../../../../../common/detection_engine/schemas/request/create_rules_schema'; import { getListMock, getEndpointListMock, @@ -719,13 +719,18 @@ describe('helpers', () => { mockActions = mockActionsStepRule(); }); - test('returns NewRule with type of saved_query when saved_id exists', () => { - const result: NewRule = formatRule(mockDefine, mockAbout, mockSchedule, mockActions); + test('returns CreateRulesSchema with type of saved_query when saved_id exists', () => { + const result: CreateRulesSchema = formatRule( + mockDefine, + mockAbout, + mockSchedule, + mockActions + ); expect(result.type).toEqual('saved_query'); }); - test('returns NewRule with type of query when saved_id does not exist', () => { + test('returns CreateRulesSchema with type of query when saved_id does not exist', () => { const mockDefineStepRuleWithoutSavedId = { ...mockDefine, queryBar: { @@ -733,7 +738,7 @@ describe('helpers', () => { saved_id: '', }, }; - const result: NewRule = formatRule( + const result: CreateRulesSchema = formatRule( mockDefineStepRuleWithoutSavedId, mockAbout, mockSchedule, @@ -743,10 +748,15 @@ describe('helpers', () => { expect(result.type).toEqual('query'); }); - test('returns NewRule without id if ruleId does not exist', () => { - const result: NewRule = formatRule(mockDefine, mockAbout, mockSchedule, mockActions); + test('returns CreateRulesSchema without id if ruleId does not exist', () => { + const result: CreateRulesSchema = formatRule( + mockDefine, + mockAbout, + mockSchedule, + mockActions + ); - expect(result.id).toBeUndefined(); + expect(result).not.toHaveProperty('id'); }); }); diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts index 434a33ac377228..58320b1bef85f6 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts @@ -15,8 +15,8 @@ import { isMlRule } from '../../../../../../common/machine_learning/helpers'; import { isThresholdRule } from '../../../../../../common/detection_engine/utils'; import { List } from '../../../../../../common/detection_engine/schemas/types'; import { ENDPOINT_LIST_ID } from '../../../../../shared_imports'; -import { NewRule, Rule } from '../../../../containers/detection_engine/rules'; - +import { Rule } from '../../../../containers/detection_engine/rules'; +import { CreateRulesSchema } from '../../../../../../common/detection_engine/schemas/request/create_rules_schema'; import { AboutStepRule, DefineStepRule, @@ -243,10 +243,10 @@ export const formatRule = ( scheduleData: ScheduleStepRule, actionsData: ActionsStepRule, rule?: Rule | null -): NewRule => +): CreateRulesSchema => deepmerge.all([ formatDefineStepData(defineStepData), formatAboutStepData(aboutStepData, rule?.exceptions_list), formatScheduleStepData(scheduleData), formatActionsStepData(actionsData), - ]) as NewRule; + ]) as CreateRulesSchema; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts index e36f08703dae51..501e03d8e31596 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts @@ -19,12 +19,13 @@ import { RuleNameOverride, SeverityMapping, TimestampOverride, + SortOrder, } from '../../../../../common/detection_engine/schemas/common/schemas'; import { List } from '../../../../../common/detection_engine/schemas/types'; export interface EuiBasicTableSortTypes { field: string; - direction: 'asc' | 'desc'; + direction: SortOrder; } export interface EuiBasicTableOnChange { From 4e77ecf8d2dddf62b8c417e3a687fc168eba9f07 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 13 Aug 2020 16:49:03 -0400 Subject: [PATCH 2/8] cleaned up types --- x-pack/plugins/lists/public/shared_exports.ts | 2 +- ...se_fetch_or_create_rule_exception_list.tsx | 3 +- .../rules/all_rules_tables/index.tsx | 5 ++- .../detection_engine/rules/__mocks__/api.ts | 13 ++++---- .../detection_engine/rules/api.test.ts | 15 ++++----- .../containers/detection_engine/rules/api.ts | 26 ++++++++++++++-- .../containers/detection_engine/rules/mock.ts | 31 +------------------ .../rules/persist_rule.test.tsx | 6 ++-- .../detection_engine/rules/types.ts | 5 +-- 9 files changed, 47 insertions(+), 59 deletions(-) diff --git a/x-pack/plugins/lists/public/shared_exports.ts b/x-pack/plugins/lists/public/shared_exports.ts index 0557dcb567e5c1..320619b97cff07 100644 --- a/x-pack/plugins/lists/public/shared_exports.ts +++ b/x-pack/plugins/lists/public/shared_exports.ts @@ -34,4 +34,4 @@ export { Pagination, UseExceptionListSuccess, } from './exceptions/types'; -export { toPromise, toError } from './common/fp_utils'; +export { toPromise, toError } from '../common/fp_utils'; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx index c5be89ead95d74..0d367e03a799f3 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx @@ -9,7 +9,6 @@ import { HttpStart } from '../../../../../../../src/core/public'; import { Rule } from '../../../detections/containers/detection_engine/rules/types'; import { List, ListArray } from '../../../../common/detection_engine/schemas/types'; -import { RulesSchema } from '../../../../common/detection_engine/schemas/response'; import { fetchRuleById, patchRule, @@ -29,7 +28,7 @@ export type ReturnUseFetchOrCreateRuleExceptionList = [boolean, ExceptionListSch export interface UseFetchOrCreateRuleExceptionListProps { http: HttpStart; - ruleId: RulesSchema['id']; + ruleId: Rule['id']; exceptionListType: ExceptionListSchema['type']; onError: (arg: Error) => void; onSuccess?: (ruleWasChanged: boolean) => void; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx index 582490ced33d17..8fd3f648bc812c 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/all_rules_tables/index.tsx @@ -20,8 +20,7 @@ import { RulesColumns, RuleStatusRowItemType, } from '../../../pages/detection_engine/rules/all/columns'; -import { RulesSchema } from '../../../../../common/detection_engine/schemas/response'; -import { Rules } from '../../../containers/detection_engine/rules/types'; +import { Rule, Rules } from '../../../containers/detection_engine/rules/types'; import { AllRulesTabs } from '../../../pages/detection_engine/rules/all'; // EuiBasicTable give me a hardtime with adding the ref attributes so I went the easy way @@ -37,7 +36,7 @@ export interface SortingType { } interface AllRulesTablesProps { - euiBasicTableSelectionProps: EuiTableSelectionType; + euiBasicTableSelectionProps: EuiTableSelectionType; hasNoPermissions: boolean; monitoringColumns: Array>; pagination: { diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts index f12a5d523bade7..bdaf65c4b39765 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts @@ -7,7 +7,6 @@ import { AddRulesProps, PatchRuleProps, - NewRule, PrePackagedRulesStatusResponse, BasicFetchProps, RuleStatusResponse, @@ -16,13 +15,15 @@ import { FetchRulesResponse, FetchRulesProps, } from '../types'; -import { ruleMock, savedRuleMock, rulesMock } from '../mock'; +import { savedRuleMock, rulesMock } from '../mock'; +import { getRulesSchemaMock } from '../../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; +import { RulesSchema } from '../../../../../../common/detection_engine/schemas/response'; -export const addRule = async ({ rule, signal }: AddRulesProps): Promise => - Promise.resolve(ruleMock); +export const addRule = async ({ rule, signal }: AddRulesProps): Promise => + Promise.resolve(getRulesSchemaMock()); -export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => - Promise.resolve(ruleMock); +export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => + Promise.resolve(getRulesSchemaMock()); export const getPrePackagedRulesStatus = async ({ signal, diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts index f58c95ed71e29e..b5aa8774363881 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts @@ -19,7 +19,8 @@ import { fetchTags, getPrePackagedRulesStatus, } from './api'; -import { ruleMock, rulesMock } from './mock'; +import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; +import { rulesMock } from './mock'; import { buildEsQuery } from 'src/plugins/data/common'; const abortCtrl = new AbortController(); @@ -33,11 +34,11 @@ describe('Detections Rules API', () => { describe('addRule', () => { beforeEach(() => { fetchMock.mockClear(); - fetchMock.mockResolvedValue(ruleMock); + fetchMock.mockResolvedValue(getRulesSchemaMock()); }); test('check parameter url, body', async () => { - await addRule({ rule: ruleMock, signal: abortCtrl.signal }); + await addRule({ rule: getRulesSchemaMock(), signal: abortCtrl.signal }); expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', { body: '{"description":"some desc","enabled":true,"false_positives":[],"filters":[],"from":"now-360s","index":["apm-*-transaction*","auditbeat-*","endgame-*","filebeat-*","packetbeat-*","winlogbeat-*"],"interval":"5m","rule_id":"bbd3106e-b4b5-4d7c-a1a2-47531d6a2baf","language":"kuery","risk_score":75,"name":"Test rule","query":"user.email: \'root@elastic.co\'","references":[],"severity":"high","tags":["APM"],"to":"now","type":"query","threat":[],"throttle":null}', @@ -47,8 +48,8 @@ describe('Detections Rules API', () => { }); test('happy path', async () => { - const ruleResp = await addRule({ rule: ruleMock, signal: abortCtrl.signal }); - expect(ruleResp).toEqual(ruleMock); + const ruleResp = await addRule({ rule: getRulesSchemaMock(), signal: abortCtrl.signal }); + expect(ruleResp).toEqual(getRulesSchemaMock()); }); }); @@ -280,7 +281,7 @@ describe('Detections Rules API', () => { describe('fetchRuleById', () => { beforeEach(() => { fetchMock.mockClear(); - fetchMock.mockResolvedValue(ruleMock); + fetchMock.mockResolvedValue(getRulesSchemaMock()); }); test('check parameter url, query', async () => { @@ -296,7 +297,7 @@ describe('Detections Rules API', () => { test('happy path', async () => { const ruleResp = await fetchRuleById({ id: 'mySuperRuleId', signal: abortCtrl.signal }); - expect(ruleResp).toEqual(ruleMock); + expect(ruleResp).toEqual(getRulesSchemaMock()); }); }); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index 10b117625e3d2f..2c6cba662c4ca6 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -43,6 +43,7 @@ import { UpdateRulesSchema, createRulesSchema, updateRulesSchema, + patchRulesSchema, } from '../../../../../common/detection_engine/schemas/request'; import { toError, toPromise } from '../../../../../common/shared_imports'; @@ -99,20 +100,39 @@ const updateRuleWithValidation = async ({ rule, signal }: UpdateRulesProps): Pro export { updateRuleWithValidation as updateRule }; /** - * Patch provided Rule + * Patch provided rule + * NOTE: The rule edit flow does NOT use patch as it relies on the + * functionality of PUT to delete field values when not provided, if + * just expecting changes, use this `patchRule` * * @param ruleProperties to patch * @param signal to cancel request * * @throws An error if response is not OK */ -export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => - KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { +const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => + KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { method: 'PATCH', body: JSON.stringify(ruleProperties), signal, }); +const patchRuleWithValidation = async ({ + ruleProperties, + signal, +}: PatchRuleProps): Promise => + pipe( + ruleProperties, + (body) => fromEither(validateEither(patchRulesSchema, body)), + chain((payload) => + tryCatch(() => patchRule({ signal, ruleProperties: { ...payload } }), toError) + ), + chain((response) => fromEither(validateEither(rulesSchema, response))), + flow(toPromise) + ); + +export { patchRuleWithValidation as patchRule }; + /** * Fetches all rules from the Detection Engine API * diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/mock.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/mock.ts index fa11cfabcdf8ba..c0397b0af6db9f 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/mock.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/mock.ts @@ -4,36 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { NewRule, FetchRulesResponse, Rule } from './types'; - -export const ruleMock: NewRule = { - description: 'some desc', - enabled: true, - false_positives: [], - filters: [], - from: 'now-360s', - index: [ - 'apm-*-transaction*', - 'auditbeat-*', - 'endgame-*', - 'filebeat-*', - 'packetbeat-*', - 'winlogbeat-*', - ], - interval: '5m', - rule_id: 'bbd3106e-b4b5-4d7c-a1a2-47531d6a2baf', - language: 'kuery', - risk_score: 75, - name: 'Test rule', - query: "user.email: 'root@elastic.co'", - references: [], - severity: 'high', - tags: ['APM'], - to: 'now', - type: 'query', - threat: [], - throttle: null, -}; +import { FetchRulesResponse, Rule } from './types'; export const savedRuleMock: Rule = { author: [], diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx index 1bf21623992e61..056eb912a7d03d 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx @@ -7,7 +7,7 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { usePersistRule, ReturnPersistRule } from './persist_rule'; -import { ruleMock } from './mock'; +import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; jest.mock('./api'); @@ -24,7 +24,7 @@ describe('usePersistRule', () => { usePersistRule() ); await waitForNextUpdate(); - result.current[1](ruleMock); + result.current[1](getRulesSchemaMock()); rerender(); expect(result.current).toEqual([{ isLoading: true, isSaved: false }, result.current[1]]); }); @@ -36,7 +36,7 @@ describe('usePersistRule', () => { usePersistRule() ); await waitForNextUpdate(); - result.current[1](ruleMock); + result.current[1](getRulesSchemaMock()); await waitForNextUpdate(); expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); }); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts index f65212a0590af7..85cb0f2d6af3d2 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts @@ -18,10 +18,7 @@ import { threshold, SortOrder, } from '../../../../../common/detection_engine/schemas/common/schemas'; -import { - listArray, - listArrayOrUndefined, -} from '../../../../../common/detection_engine/schemas/types'; +import { listArray } from '../../../../../common/detection_engine/schemas/types'; import { CreateRulesSchema, PatchRulesSchema, From 9a7b05c2ac86144baab851de9c206e4fb5ef60e8 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 13 Aug 2020 18:26:37 -0400 Subject: [PATCH 3/8] updated some rule hooks and tests --- .../detection_engine/rules/__mocks__/api.ts | 8 +- .../detection_engine/rules/api.test.ts | 147 ++++++++++++++++-- .../containers/detection_engine/rules/api.ts | 22 ++- .../detection_engine/rules/index.ts | 3 +- .../detection_engine/rules/types.ts | 5 - ...rule.test.tsx => use_create_rule.test.tsx} | 20 +-- .../{persist_rule.tsx => use_create_rule.tsx} | 20 +-- .../rules/use_pre_packaged_rules.test.tsx | 2 +- .../rules/use_update_rule.test.tsx | 44 ++++++ .../rules/use_update_rule.tsx | 60 +++++++ .../detection_engine/rules/create/helpers.ts | 9 +- .../detection_engine/rules/create/index.tsx | 7 +- .../detection_engine/rules/edit/index.tsx | 7 +- 13 files changed, 290 insertions(+), 64 deletions(-) rename x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/{persist_rule.test.tsx => use_create_rule.test.tsx} (68%) rename x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/{persist_rule.tsx => use_create_rule.tsx} (72%) create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.test.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.tsx diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts index bdaf65c4b39765..0ed091f2c18a63 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/__mocks__/api.ts @@ -5,8 +5,9 @@ */ import { - AddRulesProps, PatchRuleProps, + CreateRulesProps, + UpdateRulesProps, PrePackagedRulesStatusResponse, BasicFetchProps, RuleStatusResponse, @@ -19,7 +20,10 @@ import { savedRuleMock, rulesMock } from '../mock'; import { getRulesSchemaMock } from '../../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; import { RulesSchema } from '../../../../../../common/detection_engine/schemas/response'; -export const addRule = async ({ rule, signal }: AddRulesProps): Promise => +export const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise => + Promise.resolve(getRulesSchemaMock()); + +export const createRule = async ({ rule, signal }: CreateRulesProps): Promise => Promise.resolve(getRulesSchemaMock()); export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts index b5aa8774363881..bce493d86c89b9 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts @@ -6,7 +6,9 @@ import { KibanaServices } from '../../../../common/lib/kibana'; import { - addRule, + createRule, + updateRule, + patchRule, fetchRules, fetchRuleById, enableRules, @@ -20,9 +22,16 @@ import { getPrePackagedRulesStatus, } from './api'; import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; +import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/create_rules_schema.mock'; +import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/update_rules_schema.mock'; +import { getPatchRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/patch_rules_schema.mock'; import { rulesMock } from './mock'; import { buildEsQuery } from 'src/plugins/data/common'; - +import { + UpdateRulesSchema, + CreateRulesSchema, + PatchRulesSchema, +} from '../../../../../common/detection_engine/schemas/request'; const abortCtrl = new AbortController(); const mockKibanaServices = KibanaServices.get as jest.Mock; jest.mock('../../../../common/lib/kibana'); @@ -31,25 +40,145 @@ const fetchMock = jest.fn(); mockKibanaServices.mockReturnValue({ http: { fetch: fetchMock } }); describe('Detections Rules API', () => { - describe('addRule', () => { + describe('createRule', () => { beforeEach(() => { fetchMock.mockClear(); fetchMock.mockResolvedValue(getRulesSchemaMock()); }); - test('check parameter url, body', async () => { - await addRule({ rule: getRulesSchemaMock(), signal: abortCtrl.signal }); + test('POSTs rule', async () => { + const payload = getCreateRulesSchemaMock(); + await createRule({ rule: payload, signal: abortCtrl.signal }); expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', { body: - '{"description":"some desc","enabled":true,"false_positives":[],"filters":[],"from":"now-360s","index":["apm-*-transaction*","auditbeat-*","endgame-*","filebeat-*","packetbeat-*","winlogbeat-*"],"interval":"5m","rule_id":"bbd3106e-b4b5-4d7c-a1a2-47531d6a2baf","language":"kuery","risk_score":75,"name":"Test rule","query":"user.email: \'root@elastic.co\'","references":[],"severity":"high","tags":["APM"],"to":"now","type":"query","threat":[],"throttle":null}', + '{"description":"Detecting root and admin users","name":"Query with a rule id","severity":"high","type":"query","risk_score":55,"query":"user.name: root or user.name: admin","language":"kuery","rule_id":"rule-1","actions":[],"author":[],"enabled":true,"false_positives":[],"from":"now-6m","interval":"5m","max_signals":100,"risk_score_mapping":[],"severity_mapping":[],"tags":[],"to":"now","threat":[],"throttle":null,"references":[],"version":1,"exceptions_list":[]}', method: 'POST', signal: abortCtrl.signal, }); }); - test('happy path', async () => { - const ruleResp = await addRule({ rule: getRulesSchemaMock(), signal: abortCtrl.signal }); - expect(ruleResp).toEqual(getRulesSchemaMock()); + it('rejects with an error if request payload is invalid (and does not make API call)', async () => { + const payload: Partial = { + rule_id: 'rule-1', + description: 'some description', + from: 'now-5m', + to: 'now', + name: 'some-name', + severity: 'low', + type: 'query', + interval: '5m', + index: ['index-1'], + }; + + await expect( + createRule({ + rule: (payload as unknown) as CreateRulesSchema, + signal: abortCtrl.signal, + }) + ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "risk_score"')); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('rejects with an error if response payload is invalid', async () => { + const payload: CreateRulesSchema = getCreateRulesSchemaMock(); + const badResponse = { ...getRulesSchemaMock(), id: undefined }; + fetchMock.mockResolvedValue(badResponse); + + await expect( + createRule({ + rule: payload, + signal: abortCtrl.signal, + }) + ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"')); + }); + }); + + describe('updateRule', () => { + beforeEach(() => { + fetchMock.mockClear(); + fetchMock.mockResolvedValue(getRulesSchemaMock()); + }); + + test('POSTs rule', async () => { + const payload = getUpdateRulesSchemaMock(); + await updateRule({ rule: payload, signal: abortCtrl.signal }); + expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', { + body: + '{"description":"some description","name":"Query with a rule id","severity":"high","type":"query","risk_score":55,"query":"user.name: root or user.name: admin","language":"kuery","rule_id":"rule-1","actions":[],"author":[],"enabled":true,"false_positives":[],"from":"now-6m","interval":"5m","max_signals":100,"risk_score_mapping":[],"severity_mapping":[],"tags":[],"to":"now","threat":[],"throttle":null,"references":[],"exceptions_list":[]}', + method: 'PUT', + signal: abortCtrl.signal, + }); + }); + + it('rejects with an error if request payload is invalid (and does not make API call)', async () => { + const payload: Omit & { + note: number; + } = { ...getUpdateRulesSchemaMock(), note: 23 }; + + await expect( + updateRule({ + rule: (payload as unknown) as UpdateRulesSchema, + signal: abortCtrl.signal, + }) + ).rejects.toEqual(new Error('Invalid value "23" supplied to "note"')); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('rejects with an error if response payload is invalid', async () => { + const payload: UpdateRulesSchema = getUpdateRulesSchemaMock(); + const badResponse = { ...getRulesSchemaMock(), id: undefined }; + fetchMock.mockResolvedValue(badResponse); + + await expect( + updateRule({ + rule: payload, + signal: abortCtrl.signal, + }) + ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"')); + }); + }); + + describe('patchRule', () => { + beforeEach(() => { + fetchMock.mockClear(); + fetchMock.mockResolvedValue(getRulesSchemaMock()); + }); + + test('PATCHs rule', async () => { + const payload = getPatchRulesSchemaMock(); + await patchRule({ ruleProperties: payload, signal: abortCtrl.signal }); + expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', { + body: JSON.stringify(payload), + method: 'PATCH', + signal: abortCtrl.signal, + }); + }); + + it('rejects with an error if request payload is invalid (and does not make API call)', async () => { + const payload: Omit & { + note: number; + } = { ...getPatchRulesSchemaMock(), note: 3 }; + + await expect( + patchRule({ + ruleProperties: (payload as unknown) as PatchRulesSchema, + signal: abortCtrl.signal, + }) + ).rejects.toEqual(new Error('Invalid value "3" supplied to "note"')); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('rejects with an error if response payload is invalid', async () => { + const payload: PatchRulesSchema = getPatchRulesSchemaMock(); + const badResponse = { ...getRulesSchemaMock(), id: undefined }; + fetchMock.mockResolvedValue(badResponse); + + await expect( + patchRule({ + ruleProperties: payload, + signal: abortCtrl.signal, + }) + ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"')); }); }); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index 2c6cba662c4ca6..11deb9002f75f6 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -16,7 +16,6 @@ import { DETECTION_ENGINE_TAGS_URL, } from '../../../../../common/constants'; import { - AddRulesProps, UpdateRulesProps, CreateRulesProps, DeleteRulesProps, @@ -40,7 +39,6 @@ import * as i18n from '../../../pages/detection_engine/rules/translations'; import { RulesSchema, rulesSchema } from '../../../../../common/detection_engine/schemas/response'; import { validateEither } from '../../../../../common'; import { - UpdateRulesSchema, createRulesSchema, updateRulesSchema, patchRulesSchema, @@ -48,21 +46,13 @@ import { import { toError, toPromise } from '../../../../../common/shared_imports'; /** - * Add provided Rule + * Create provided Rule * - * @param rule to add + * @param rule CreateRulesSchema to add * @param signal to cancel request * * @throws An error if response is not OK */ -export const addRule = async ({ rule, signal }: AddRulesProps): Promise => { - if ((rule as UpdateRulesSchema).id != null) { - return updateRuleWithValidation({ rule, signal }); - } else { - return createRuleWithValidation({ rule, signal }); - } -}; - const createRule = async ({ rule, signal }: CreateRulesProps): Promise => KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { method: 'POST', @@ -81,6 +71,14 @@ const createRuleWithValidation = async ({ rule, signal }: CreateRulesProps): Pro export { createRuleWithValidation as createRule }; +/** + * Update provided Rule using PUT + * + * @param rule UpdateRulesSchema to be updated + * @param signal to cancel request + * + * @throws An error if response is not OK + */ const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise => KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { method: 'PUT', diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts index c7ecfb33cd9052..a40ab2e4878519 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts @@ -6,7 +6,8 @@ export * from './api'; export * from './fetch_index_patterns'; -export * from './persist_rule'; +export * from './use_update_rule'; +export * from './use_create_rule'; export * from './types'; export * from './use_rule'; export * from './use_rules'; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts index 85cb0f2d6af3d2..05ce79d04dc8f9 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts @@ -48,11 +48,6 @@ export interface UpdateRulesProps { signal: AbortSignal; } -export interface AddRulesProps { - rule: UpdateRulesSchema | CreateRulesSchema; - signal: AbortSignal; -} - export interface PatchRuleProps { ruleProperties: PatchRulesSchema; signal: AbortSignal; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_create_rule.test.tsx similarity index 68% rename from x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx rename to x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_create_rule.test.tsx index 056eb912a7d03d..42d6a2a92a4c2f 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_create_rule.test.tsx @@ -6,25 +6,25 @@ import { renderHook, act } from '@testing-library/react-hooks'; -import { usePersistRule, ReturnPersistRule } from './persist_rule'; -import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; +import { useCreateRule, ReturnCreateRule } from './use_create_rule'; +import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/update_rules_schema.mock'; jest.mock('./api'); -describe('usePersistRule', () => { +describe('useCreateRule', () => { test('init', async () => { - const { result } = renderHook(() => usePersistRule()); + const { result } = renderHook(() => useCreateRule()); expect(result.current).toEqual([{ isLoading: false, isSaved: false }, result.current[1]]); }); test('saving rule with isLoading === true', async () => { await act(async () => { - const { result, rerender, waitForNextUpdate } = renderHook(() => - usePersistRule() + const { result, rerender, waitForNextUpdate } = renderHook(() => + useCreateRule() ); await waitForNextUpdate(); - result.current[1](getRulesSchemaMock()); + result.current[1](getUpdateRulesSchemaMock()); rerender(); expect(result.current).toEqual([{ isLoading: true, isSaved: false }, result.current[1]]); }); @@ -32,11 +32,11 @@ describe('usePersistRule', () => { test('saved rule with isSaved === true', async () => { await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - usePersistRule() + const { result, waitForNextUpdate } = renderHook(() => + useCreateRule() ); await waitForNextUpdate(); - result.current[1](getRulesSchemaMock()); + result.current[1](getUpdateRulesSchemaMock()); await waitForNextUpdate(); expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); }); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_create_rule.tsx similarity index 72% rename from x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx rename to x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_create_rule.tsx index 233a6cdb414aa3..2bbd27994fc771 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/persist_rule.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_create_rule.tsx @@ -7,26 +7,20 @@ import { useEffect, useState, Dispatch } from 'react'; import { errorToToaster, useStateToaster } from '../../../../common/components/toasters'; -import { - UpdateRulesSchema, - CreateRulesSchema, -} from '../../../../../common/detection_engine/schemas/request'; +import { CreateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; -import { addRule as persistRule } from './api'; +import { createRule } from './api'; import * as i18n from './translations'; -interface PersistRuleReturn { +interface CreateRuleReturn { isLoading: boolean; isSaved: boolean; } -export type ReturnPersistRule = [ - PersistRuleReturn, - Dispatch -]; +export type ReturnCreateRule = [CreateRuleReturn, Dispatch]; -export const usePersistRule = (): ReturnPersistRule => { - const [rule, setRule] = useState(null); +export const useCreateRule = (): ReturnCreateRule => { + const [rule, setRule] = useState(null); const [isSaved, setIsSaved] = useState(false); const [isLoading, setIsLoading] = useState(false); const [, dispatchToaster] = useStateToaster(); @@ -39,7 +33,7 @@ export const usePersistRule = (): ReturnPersistRule => { if (rule != null) { try { setIsLoading(true); - await persistRule({ rule, signal: abortCtrl.signal }); + await createRule({ rule, signal: abortCtrl.signal }); if (isSubscribed) { setIsSaved(true); } diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_pre_packaged_rules.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_pre_packaged_rules.test.tsx index 9a6ea4f60fdcc9..92d46a785b0349 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_pre_packaged_rules.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_pre_packaged_rules.test.tsx @@ -10,7 +10,7 @@ import * as api from './api'; jest.mock('./api'); -describe('usePersistRule', () => { +describe('usePrePackagedRules', () => { beforeEach(() => { jest.clearAllMocks(); jest.restoreAllMocks(); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.test.tsx new file mode 100644 index 00000000000000..9603a4151933a4 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.test.tsx @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { renderHook, act } from '@testing-library/react-hooks'; + +import { useUpdateRule, ReturnUpdateRule } from './use_update_rule'; +import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/update_rules_schema.mock'; + +jest.mock('./api'); + +describe('useUpdateRule', () => { + test('init', async () => { + const { result } = renderHook(() => useUpdateRule()); + + expect(result.current).toEqual([{ isLoading: false, isSaved: false }, result.current[1]]); + }); + + test('saving rule with isLoading === true', async () => { + await act(async () => { + const { result, rerender, waitForNextUpdate } = renderHook(() => + useUpdateRule() + ); + await waitForNextUpdate(); + result.current[1](getUpdateRulesSchemaMock()); + rerender(); + expect(result.current).toEqual([{ isLoading: true, isSaved: false }, result.current[1]]); + }); + }); + + test('saved rule with isSaved === true', async () => { + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useUpdateRule() + ); + await waitForNextUpdate(); + result.current[1](getUpdateRulesSchemaMock()); + await waitForNextUpdate(); + expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.tsx new file mode 100644 index 00000000000000..a437974e93ba30 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_update_rule.tsx @@ -0,0 +1,60 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { useEffect, useState, Dispatch } from 'react'; + +import { errorToToaster, useStateToaster } from '../../../../common/components/toasters'; +import { UpdateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; + +import { updateRule } from './api'; +import * as i18n from './translations'; + +interface UpdateRuleReturn { + isLoading: boolean; + isSaved: boolean; +} + +export type ReturnUpdateRule = [UpdateRuleReturn, Dispatch]; + +export const useUpdateRule = (): ReturnUpdateRule => { + const [rule, setRule] = useState(null); + const [isSaved, setIsSaved] = useState(false); + const [isLoading, setIsLoading] = useState(false); + const [, dispatchToaster] = useStateToaster(); + + useEffect(() => { + let isSubscribed = true; + const abortCtrl = new AbortController(); + setIsSaved(false); + async function saveRule() { + if (rule != null) { + try { + setIsLoading(true); + await updateRule({ rule, signal: abortCtrl.signal }); + if (isSubscribed) { + setIsSaved(true); + } + } catch (error) { + if (isSubscribed) { + errorToToaster({ title: i18n.RULE_ADD_FAILURE, error, dispatchToaster }); + } + } + if (isSubscribed) { + setIsLoading(false); + } + } + } + + saveRule(); + return () => { + isSubscribed = false; + abortCtrl.abort(); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [rule]); + + return [{ isLoading, isSaved }, setRule]; +}; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts index 58320b1bef85f6..19ff2244a65aa3 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts @@ -16,7 +16,6 @@ import { isThresholdRule } from '../../../../../../common/detection_engine/utils import { List } from '../../../../../../common/detection_engine/schemas/types'; import { ENDPOINT_LIST_ID } from '../../../../../shared_imports'; import { Rule } from '../../../../containers/detection_engine/rules'; -import { CreateRulesSchema } from '../../../../../../common/detection_engine/schemas/request/create_rules_schema'; import { AboutStepRule, DefineStepRule, @@ -237,16 +236,16 @@ export const formatActionsStepData = (actionsStepData: ActionsStepRule): Actions }; }; -export const formatRule = ( +export const formatRule = ( defineStepData: DefineStepRule, aboutStepData: AboutStepRule, scheduleData: ScheduleStepRule, actionsData: ActionsStepRule, rule?: Rule | null -): CreateRulesSchema => - deepmerge.all([ +): T => + (deepmerge.all([ formatDefineStepData(defineStepData), formatAboutStepData(aboutStepData, rule?.exceptions_list), formatScheduleStepData(scheduleData), formatActionsStepData(actionsData), - ]) as CreateRulesSchema; + ]) as unknown) as T; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx index 70f278197b0051..6cd989c7991703 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx @@ -9,7 +9,8 @@ import React, { useCallback, useRef, useState, useMemo } from 'react'; import { useHistory } from 'react-router-dom'; import styled, { StyledComponent } from 'styled-components'; -import { usePersistRule } from '../../../../containers/detection_engine/rules'; +import { useCreateRule } from '../../../../containers/detection_engine/rules'; +import { CreateRulesSchema } from '../../../../../../common/detection_engine/schemas/request'; import { useListsConfig } from '../../../../containers/detection_engine/lists/use_lists_config'; import { @@ -120,7 +121,7 @@ const CreateRulePageComponent: React.FC = () => { [RuleStep.scheduleRule]: false, [RuleStep.ruleActions]: false, }); - const [{ isLoading, isSaved }, setRule] = usePersistRule(); + const [{ isLoading, isSaved }, setRule] = useCreateRule(); const actionMessageParams = useMemo( () => getActionMessageParams((stepsData.current['define-rule'].data as DefineStepRule)?.ruleType), @@ -157,7 +158,7 @@ const CreateRulePageComponent: React.FC = () => { stepsData.current[RuleStep.scheduleRule].isValid ) { setRule( - formatRule( + formatRule( stepsData.current[RuleStep.defineRule].data as DefineStepRule, stepsData.current[RuleStep.aboutRule].data as AboutStepRule, stepsData.current[RuleStep.scheduleRule].data as ScheduleStepRule, diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx index 13855a4b81494f..a190668e6510af 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx @@ -17,7 +17,7 @@ import { FormattedMessage } from '@kbn/i18n/react'; import React, { FC, memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useParams, useHistory } from 'react-router-dom'; -import { useRule, usePersistRule } from '../../../../containers/detection_engine/rules'; +import { useRule, useUpdateRule } from '../../../../containers/detection_engine/rules'; import { useListsConfig } from '../../../../containers/detection_engine/lists/use_lists_config'; import { WrapperPage } from '../../../../../common/components/wrapper_page'; import { @@ -51,6 +51,7 @@ import { } from '../types'; import * as i18n from './translations'; import { SecurityPageName } from '../../../../../app/types'; +import { UpdateRulesSchema } from '../../../../../../common/detection_engine/schemas/request'; interface StepRuleForm { isValid: boolean; @@ -111,7 +112,7 @@ const EditRulePageComponent: FC = () => { [RuleStep.scheduleRule]: null, [RuleStep.ruleActions]: null, }); - const [{ isLoading, isSaved }, setRule] = usePersistRule(); + const [{ isLoading, isSaved }, setRule] = useUpdateRule(); const [tabHasError, setTabHasError] = useState([]); // eslint-disable-next-line react-hooks/exhaustive-deps const actionMessageParams = useMemo(() => getActionMessageParams(rule?.type), [rule]); @@ -259,7 +260,7 @@ const EditRulePageComponent: FC = () => { if (invalidForms.length === 0 && activeForm != null) { setTabHasError([]); setRule({ - ...formatRule( + ...formatRule( (activeFormId === RuleStep.defineRule ? activeForm.data : myDefineRuleForm.data) as DefineStepRule, From f713a287b278879d4b6856ec8518c089319f5ef0 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Wed, 2 Sep 2020 21:11:18 -0400 Subject: [PATCH 4/8] removed api io-ts validation, can be discussed separatly --- x-pack/plugins/lists/common/shared_exports.ts | 2 - .../{ => public}/common/fp_utils.test.ts | 0 .../lists/{ => public}/common/fp_utils.ts | 0 x-pack/plugins/lists/public/lists/api.ts | 2 +- x-pack/plugins/lists/public/shared_exports.ts | 1 - .../detection_engine/rules/api.test.ts | 95 +------------------ .../containers/detection_engine/rules/api.ts | 57 +---------- 7 files changed, 7 insertions(+), 150 deletions(-) rename x-pack/plugins/lists/{ => public}/common/fp_utils.test.ts (100%) rename x-pack/plugins/lists/{ => public}/common/fp_utils.ts (100%) diff --git a/x-pack/plugins/lists/common/shared_exports.ts b/x-pack/plugins/lists/common/shared_exports.ts index d6a6e76333ce85..1f6c65919b063a 100644 --- a/x-pack/plugins/lists/common/shared_exports.ts +++ b/x-pack/plugins/lists/common/shared_exports.ts @@ -45,5 +45,3 @@ export { } from './schemas'; export { ENDPOINT_LIST_ID } from './constants'; - -export { toPromise, toError } from './fp_utils'; diff --git a/x-pack/plugins/lists/common/fp_utils.test.ts b/x-pack/plugins/lists/public/common/fp_utils.test.ts similarity index 100% rename from x-pack/plugins/lists/common/fp_utils.test.ts rename to x-pack/plugins/lists/public/common/fp_utils.test.ts diff --git a/x-pack/plugins/lists/common/fp_utils.ts b/x-pack/plugins/lists/public/common/fp_utils.ts similarity index 100% rename from x-pack/plugins/lists/common/fp_utils.ts rename to x-pack/plugins/lists/public/common/fp_utils.ts diff --git a/x-pack/plugins/lists/public/lists/api.ts b/x-pack/plugins/lists/public/lists/api.ts index 93319318501d07..2b123280474dfb 100644 --- a/x-pack/plugins/lists/public/lists/api.ts +++ b/x-pack/plugins/lists/public/lists/api.ts @@ -30,7 +30,7 @@ import { } from '../../common/schemas'; import { LIST_INDEX, LIST_ITEM_URL, LIST_PRIVILEGES_URL, LIST_URL } from '../../common/constants'; import { validateEither } from '../../common/shared_imports'; -import { toError, toPromise } from '../../common/fp_utils'; +import { toError, toPromise } from '../common/fp_utils'; import { ApiParams, diff --git a/x-pack/plugins/lists/public/shared_exports.ts b/x-pack/plugins/lists/public/shared_exports.ts index 320619b97cff07..16026a436f1549 100644 --- a/x-pack/plugins/lists/public/shared_exports.ts +++ b/x-pack/plugins/lists/public/shared_exports.ts @@ -34,4 +34,3 @@ export { Pagination, UseExceptionListSuccess, } from './exceptions/types'; -export { toPromise, toError } from '../common/fp_utils'; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts index bce493d86c89b9..803c7c4433882a 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts @@ -27,11 +27,7 @@ import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine import { getPatchRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/patch_rules_schema.mock'; import { rulesMock } from './mock'; import { buildEsQuery } from 'src/plugins/data/common'; -import { - UpdateRulesSchema, - CreateRulesSchema, - PatchRulesSchema, -} from '../../../../../common/detection_engine/schemas/request'; +import { CreateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; const abortCtrl = new AbortController(); const mockKibanaServices = KibanaServices.get as jest.Mock; jest.mock('../../../../common/lib/kibana'); @@ -56,41 +52,6 @@ describe('Detections Rules API', () => { signal: abortCtrl.signal, }); }); - - it('rejects with an error if request payload is invalid (and does not make API call)', async () => { - const payload: Partial = { - rule_id: 'rule-1', - description: 'some description', - from: 'now-5m', - to: 'now', - name: 'some-name', - severity: 'low', - type: 'query', - interval: '5m', - index: ['index-1'], - }; - - await expect( - createRule({ - rule: (payload as unknown) as CreateRulesSchema, - signal: abortCtrl.signal, - }) - ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "risk_score"')); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - it('rejects with an error if response payload is invalid', async () => { - const payload: CreateRulesSchema = getCreateRulesSchemaMock(); - const badResponse = { ...getRulesSchemaMock(), id: undefined }; - fetchMock.mockResolvedValue(badResponse); - - await expect( - createRule({ - rule: payload, - signal: abortCtrl.signal, - }) - ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"')); - }); }); describe('updateRule', () => { @@ -109,33 +70,6 @@ describe('Detections Rules API', () => { signal: abortCtrl.signal, }); }); - - it('rejects with an error if request payload is invalid (and does not make API call)', async () => { - const payload: Omit & { - note: number; - } = { ...getUpdateRulesSchemaMock(), note: 23 }; - - await expect( - updateRule({ - rule: (payload as unknown) as UpdateRulesSchema, - signal: abortCtrl.signal, - }) - ).rejects.toEqual(new Error('Invalid value "23" supplied to "note"')); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - it('rejects with an error if response payload is invalid', async () => { - const payload: UpdateRulesSchema = getUpdateRulesSchemaMock(); - const badResponse = { ...getRulesSchemaMock(), id: undefined }; - fetchMock.mockResolvedValue(badResponse); - - await expect( - updateRule({ - rule: payload, - signal: abortCtrl.signal, - }) - ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"')); - }); }); describe('patchRule', () => { @@ -153,33 +87,6 @@ describe('Detections Rules API', () => { signal: abortCtrl.signal, }); }); - - it('rejects with an error if request payload is invalid (and does not make API call)', async () => { - const payload: Omit & { - note: number; - } = { ...getPatchRulesSchemaMock(), note: 3 }; - - await expect( - patchRule({ - ruleProperties: (payload as unknown) as PatchRulesSchema, - signal: abortCtrl.signal, - }) - ).rejects.toEqual(new Error('Invalid value "3" supplied to "note"')); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - it('rejects with an error if response payload is invalid', async () => { - const payload: PatchRulesSchema = getPatchRulesSchemaMock(); - const badResponse = { ...getRulesSchemaMock(), id: undefined }; - fetchMock.mockResolvedValue(badResponse); - - await expect( - patchRule({ - ruleProperties: payload, - signal: abortCtrl.signal, - }) - ).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"')); - }); }); describe('fetchRules', () => { diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index 11deb9002f75f6..51bff6f7b9d873 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -3,10 +3,6 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { chain, fromEither, tryCatch } from 'fp-ts/lib/TaskEither'; -import { flow } from 'fp-ts/lib/function'; -import { pipe } from 'fp-ts/lib/pipeable'; - import { HttpStart } from '../../../../../../../../src/core/public'; import { DETECTION_ENGINE_RULES_URL, @@ -36,13 +32,8 @@ import { } from './types'; import { KibanaServices } from '../../../../common/lib/kibana'; import * as i18n from '../../../pages/detection_engine/rules/translations'; -import { RulesSchema, rulesSchema } from '../../../../../common/detection_engine/schemas/response'; -import { validateEither } from '../../../../../common'; -import { - createRulesSchema, - updateRulesSchema, - patchRulesSchema, -} from '../../../../../common/detection_engine/schemas/request'; +import { RulesSchema } from '../../../../../common/detection_engine/schemas/response'; +import { updateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; import { toError, toPromise } from '../../../../../common/shared_imports'; /** @@ -53,24 +44,13 @@ import { toError, toPromise } from '../../../../../common/shared_imports'; * * @throws An error if response is not OK */ -const createRule = async ({ rule, signal }: CreateRulesProps): Promise => +export const createRule = async ({ rule, signal }: CreateRulesProps): Promise => KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { method: 'POST', body: JSON.stringify(rule), signal, }); -const createRuleWithValidation = async ({ rule, signal }: CreateRulesProps): Promise => - pipe( - rule, - (body) => fromEither(validateEither(createRulesSchema, body)), - chain((payload) => tryCatch(() => createRule({ signal, rule: { ...payload } }), toError)), - chain((response) => fromEither(validateEither(rulesSchema, response))), - flow(toPromise) - ); - -export { createRuleWithValidation as createRule }; - /** * Update provided Rule using PUT * @@ -79,24 +59,13 @@ export { createRuleWithValidation as createRule }; * * @throws An error if response is not OK */ -const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise => +export const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise => KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { method: 'PUT', body: JSON.stringify(rule), signal, }); -const updateRuleWithValidation = async ({ rule, signal }: UpdateRulesProps): Promise => - pipe( - rule, - (body) => fromEither(validateEither(updateRulesSchema, body)), - chain((payload) => tryCatch(() => updateRule({ signal, rule: { ...payload } }), toError)), - chain((response) => fromEither(validateEither(rulesSchema, response))), - flow(toPromise) - ); - -export { updateRuleWithValidation as updateRule }; - /** * Patch provided rule * NOTE: The rule edit flow does NOT use patch as it relies on the @@ -108,29 +77,13 @@ export { updateRuleWithValidation as updateRule }; * * @throws An error if response is not OK */ -const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => +export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise => KibanaServices.get().http.fetch(DETECTION_ENGINE_RULES_URL, { method: 'PATCH', body: JSON.stringify(ruleProperties), signal, }); -const patchRuleWithValidation = async ({ - ruleProperties, - signal, -}: PatchRuleProps): Promise => - pipe( - ruleProperties, - (body) => fromEither(validateEither(patchRulesSchema, body)), - chain((payload) => - tryCatch(() => patchRule({ signal, ruleProperties: { ...payload } }), toError) - ), - chain((response) => fromEither(validateEither(rulesSchema, response))), - flow(toPromise) - ); - -export { patchRuleWithValidation as patchRule }; - /** * Fetches all rules from the Detection Engine API * From 6a9f4327494dc81df205225b2c476f8af0a08336 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 3 Sep 2020 10:20:20 -0400 Subject: [PATCH 5/8] cleanup --- x-pack/plugins/security_solution/common/shared_imports.ts | 2 -- .../public/detections/containers/detection_engine/rules/api.ts | 1 - 2 files changed, 3 deletions(-) diff --git a/x-pack/plugins/security_solution/common/shared_imports.ts b/x-pack/plugins/security_solution/common/shared_imports.ts index 3bc112101af149..e28d1969b39763 100644 --- a/x-pack/plugins/security_solution/common/shared_imports.ts +++ b/x-pack/plugins/security_solution/common/shared_imports.ts @@ -43,6 +43,4 @@ export { ExceptionListType, Type, ENDPOINT_LIST_ID, - toPromise, - toError, } from '../../lists/common'; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index 51bff6f7b9d873..44d0626e24eb32 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -34,7 +34,6 @@ import { KibanaServices } from '../../../../common/lib/kibana'; import * as i18n from '../../../pages/detection_engine/rules/translations'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response'; import { updateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; -import { toError, toPromise } from '../../../../../common/shared_imports'; /** * Create provided Rule From 0b3773d6ccc62e2d4814050436c63677eed41b7f Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 3 Sep 2020 11:13:15 -0400 Subject: [PATCH 6/8] update missing mock in unit test --- .../detections/containers/detection_engine/rules/api.test.ts | 1 - .../detections/containers/detection_engine/rules/api.ts | 1 - .../rules/use_dissasociate_exception_list.test.tsx | 4 ++-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts index 803c7c4433882a..44f9d9de5a92cb 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts @@ -27,7 +27,6 @@ import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine import { getPatchRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/patch_rules_schema.mock'; import { rulesMock } from './mock'; import { buildEsQuery } from 'src/plugins/data/common'; -import { CreateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; const abortCtrl = new AbortController(); const mockKibanaServices = KibanaServices.get as jest.Mock; jest.mock('../../../../common/lib/kibana'); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index 44d0626e24eb32..e254516d110768 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -33,7 +33,6 @@ import { import { KibanaServices } from '../../../../common/lib/kibana'; import * as i18n from '../../../pages/detection_engine/rules/translations'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response'; -import { updateRulesSchema } from '../../../../../common/detection_engine/schemas/request'; /** * Create provided Rule diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx index 6721d89f2799b7..062bee741fdb53 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx @@ -9,7 +9,7 @@ import { act, renderHook } from '@testing-library/react-hooks'; import { coreMock } from '../../../../../../../../src/core/public/mocks'; import * as api from './api'; -import { ruleMock } from './mock'; +import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_bulk_schema.mocks'; import { ReturnUseDissasociateExceptionList, UseDissasociateExceptionListProps, @@ -23,7 +23,7 @@ describe('useDissasociateExceptionList', () => { const onSuccess = jest.fn(); beforeEach(() => { - jest.spyOn(api, 'patchRule').mockResolvedValue(ruleMock); + jest.spyOn(api, 'patchRule').mockResolvedValue(getRulesSchemaMock()); }); afterEach(() => { From 5e1fd07be257311c96b86b6c57b8a5076a339da8 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 3 Sep 2020 12:25:34 -0400 Subject: [PATCH 7/8] update unit test --- .../detection_engine/rules/api.test.ts | 4 ++-- .../use_dissasociate_exception_list.test.tsx | 2 +- .../rules/create/helpers.test.ts | 18 +++++++----------- .../detection_engine/rules/create/helpers.ts | 3 +++ 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts index 44f9d9de5a92cb..44cedb5570e89b 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts @@ -46,7 +46,7 @@ describe('Detections Rules API', () => { await createRule({ rule: payload, signal: abortCtrl.signal }); expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', { body: - '{"description":"Detecting root and admin users","name":"Query with a rule id","severity":"high","type":"query","risk_score":55,"query":"user.name: root or user.name: admin","language":"kuery","rule_id":"rule-1","actions":[],"author":[],"enabled":true,"false_positives":[],"from":"now-6m","interval":"5m","max_signals":100,"risk_score_mapping":[],"severity_mapping":[],"tags":[],"to":"now","threat":[],"throttle":null,"references":[],"version":1,"exceptions_list":[]}', + '{"description":"Detecting root and admin users","name":"Query with a rule id","query":"user.name: root or user.name: admin","severity":"high","type":"query","risk_score":55,"language":"kuery","rule_id":"rule-1"}', method: 'POST', signal: abortCtrl.signal, }); @@ -64,7 +64,7 @@ describe('Detections Rules API', () => { await updateRule({ rule: payload, signal: abortCtrl.signal }); expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', { body: - '{"description":"some description","name":"Query with a rule id","severity":"high","type":"query","risk_score":55,"query":"user.name: root or user.name: admin","language":"kuery","rule_id":"rule-1","actions":[],"author":[],"enabled":true,"false_positives":[],"from":"now-6m","interval":"5m","max_signals":100,"risk_score_mapping":[],"severity_mapping":[],"tags":[],"to":"now","threat":[],"throttle":null,"references":[],"exceptions_list":[]}', + '{"description":"some description","name":"Query with a rule id","query":"user.name: root or user.name: admin","severity":"high","type":"query","risk_score":55,"language":"kuery","rule_id":"rule-1"}', method: 'PUT', signal: abortCtrl.signal, }); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx index 062bee741fdb53..2ba78cd90cf9b0 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx @@ -9,7 +9,7 @@ import { act, renderHook } from '@testing-library/react-hooks'; import { coreMock } from '../../../../../../../../src/core/public/mocks'; import * as api from './api'; -import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_bulk_schema.mocks'; +import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks'; import { ReturnUseDissasociateExceptionList, UseDissasociateExceptionListProps, diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts index d557ddd9363b35..79488231b29ee6 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts @@ -6,6 +6,7 @@ import { List } from '../../../../../../common/detection_engine/schemas/types'; import { CreateRulesSchema } from '../../../../../../common/detection_engine/schemas/request/create_rules_schema'; +import { Rule } from '../../../../containers/detection_engine/rules'; import { getListMock, getEndpointListMock, @@ -721,18 +722,13 @@ describe('helpers', () => { mockActions = mockActionsStepRule(); }); - test('returns CreateRulesSchema with type of saved_query when saved_id exists', () => { - const result: CreateRulesSchema = formatRule( - mockDefine, - mockAbout, - mockSchedule, - mockActions - ); + test('returns rule with type of saved_query when saved_id exists', () => { + const result: Rule = formatRule(mockDefine, mockAbout, mockSchedule, mockActions); expect(result.type).toEqual('saved_query'); }); - test('returns CreateRulesSchema with type of query when saved_id does not exist', () => { + test('returns rule with type of query when saved_id does not exist', () => { const mockDefineStepRuleWithoutSavedId = { ...mockDefine, queryBar: { @@ -740,7 +736,7 @@ describe('helpers', () => { saved_id: '', }, }; - const result: CreateRulesSchema = formatRule( + const result: CreateRulesSchema = formatRule( mockDefineStepRuleWithoutSavedId, mockAbout, mockSchedule, @@ -750,8 +746,8 @@ describe('helpers', () => { expect(result.type).toEqual('query'); }); - test('returns CreateRulesSchema without id if ruleId does not exist', () => { - const result: CreateRulesSchema = formatRule( + test('returns rule without id if ruleId does not exist', () => { + const result: CreateRulesSchema = formatRule( mockDefine, mockAbout, mockSchedule, diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts index f637e912983a0d..874ef032b7c5ec 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts @@ -237,6 +237,9 @@ export const formatActionsStepData = (actionsStepData: ActionsStepRule): Actions }; }; +// Used to format form data in rule edit and +// create flows so "T" here would likely +// either be CreateRulesSchema or Rule export const formatRule = ( defineStepData: DefineStepRule, aboutStepData: AboutStepRule, From 639386218df870002dc37148550a39bb83e8b782 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 3 Sep 2020 15:10:45 -0400 Subject: [PATCH 8/8] update test description --- .../detections/containers/detection_engine/rules/api.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts index 44cedb5570e89b..cd1ded544cfe5a 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.test.ts @@ -59,7 +59,7 @@ describe('Detections Rules API', () => { fetchMock.mockResolvedValue(getRulesSchemaMock()); }); - test('POSTs rule', async () => { + test('PUTs rule', async () => { const payload = getUpdateRulesSchemaMock(); await updateRule({ rule: payload, signal: abortCtrl.signal }); expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules', {