Skip to content

Commit

Permalink
[Security Solution][Detections][Tech Debt] - Move to using common io-…
Browse files Browse the repository at this point in the history
…ts types (#75009) (#76780)

## Summary

Part of the DE tech debt includes moving to use the common io-ts types in the UI. Currently, we are using some of them in some places and other times we're using the front end defined typescript types. This means that changes often have to be done on both ends, validation isn't being done at the UI boundary on request or response. Using the common types could help us avoid bugs moving forward. Obviously, there's a lot of code to touch so trying to just do it piece by piece. This PR addresses the following:

- Replaced uses of `NewRule` with common type `CreateRuleSchema` and `UpdateRuleSchema`. These were being combined a bit
- Split `usePersistRule` which was used for both `POST` and `PUT` into two hooks - `useCreateRule` and `useUpdateRule`. 
  - The logic for combining these two relied on checking for the existence of an `id` - if present it would `PUT`, otherwise it would `POST`.
  - However, `id` is not a required property for either of these, so it's not reliable
- Updated the rule edit flow to use `useUpdateRule`
- Updated the rule creation flow to use `useCreateRule`
  • Loading branch information
yctercero authored Sep 4, 2020
1 parent f56fcc6 commit 568e4cd
Show file tree
Hide file tree
Showing 20 changed files with 294 additions and 149 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ export const sortFieldOrUndefined = t.union([sort_field, t.undefined]);
export type SortFieldOrUndefined = t.TypeOf<typeof sortFieldOrUndefined>;

export const sort_order = t.keyof({ asc: null, desc: null });
export type sortOrder = t.TypeOf<typeof sort_order>;
export type SortOrder = t.TypeOf<typeof sort_order>;

export const sortOrderOrUndefined = t.union([sort_order, t.undefined]);
export type SortOrderOrUndefined = t.TypeOf<typeof sortOrderOrUndefined>;
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
*/

import {
AddRulesProps,
PatchRuleProps,
NewRule,
CreateRulesProps,
UpdateRulesProps,
PrePackagedRulesStatusResponse,
BasicFetchProps,
RuleStatusResponse,
Expand All @@ -16,13 +16,18 @@ 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<NewRule> =>
Promise.resolve(ruleMock);
export const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise<RulesSchema> =>
Promise.resolve(getRulesSchemaMock());

export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise<NewRule> =>
Promise.resolve(ruleMock);
export const createRule = async ({ rule, signal }: CreateRulesProps): Promise<RulesSchema> =>
Promise.resolve(getRulesSchemaMock());

export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise<RulesSchema> =>
Promise.resolve(getRulesSchemaMock());

export const getPrePackagedRulesStatus = async ({
signal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

import { KibanaServices } from '../../../../common/lib/kibana';
import {
addRule,
createRule,
updateRule,
patchRule,
fetchRules,
fetchRuleById,
enableRules,
Expand All @@ -19,9 +21,12 @@ import {
fetchTags,
getPrePackagedRulesStatus,
} from './api';
import { ruleMock, rulesMock } from './mock';
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';

const abortCtrl = new AbortController();
const mockKibanaServices = KibanaServices.get as jest.Mock;
jest.mock('../../../../common/lib/kibana');
Expand All @@ -30,25 +35,56 @@ const fetchMock = jest.fn();
mockKibanaServices.mockReturnValue({ http: { fetch: fetchMock } });

describe('Detections Rules API', () => {
describe('addRule', () => {
describe('createRule', () => {
beforeEach(() => {
fetchMock.mockClear();
fetchMock.mockResolvedValue(ruleMock);
fetchMock.mockResolvedValue(getRulesSchemaMock());
});

test('check parameter url, body', async () => {
await addRule({ rule: ruleMock, 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","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,
});
});
});

test('happy path', async () => {
const ruleResp = await addRule({ rule: ruleMock, signal: abortCtrl.signal });
expect(ruleResp).toEqual(ruleMock);
describe('updateRule', () => {
beforeEach(() => {
fetchMock.mockClear();
fetchMock.mockResolvedValue(getRulesSchemaMock());
});

test('PUTs 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","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,
});
});
});

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,
});
});
});

Expand Down Expand Up @@ -280,7 +316,7 @@ describe('Detections Rules API', () => {
describe('fetchRuleById', () => {
beforeEach(() => {
fetchMock.mockClear();
fetchMock.mockResolvedValue(ruleMock);
fetchMock.mockResolvedValue(getRulesSchemaMock());
});

test('check parameter url, query', async () => {
Expand All @@ -296,7 +332,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());
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +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 { HttpStart } from '../../../../../../../../src/core/public';
import {
DETECTION_ENGINE_RULES_URL,
Expand All @@ -13,13 +12,13 @@ import {
DETECTION_ENGINE_TAGS_URL,
} from '../../../../../common/constants';
import {
AddRulesProps,
UpdateRulesProps,
CreateRulesProps,
DeleteRulesProps,
DuplicateRulesProps,
EnableRulesProps,
FetchRulesProps,
FetchRulesResponse,
NewRule,
Rule,
FetchRuleProps,
BasicFetchProps,
Expand All @@ -33,32 +32,51 @@ import {
} from './types';
import { KibanaServices } from '../../../../common/lib/kibana';
import * as i18n from '../../../pages/detection_engine/rules/translations';
import { RulesSchema } from '../../../../../common/detection_engine/schemas/response';

/**
* 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<NewRule> =>
KibanaServices.get().http.fetch<NewRule>(DETECTION_ENGINE_RULES_URL, {
method: rule.id != null ? 'PUT' : 'POST',
export const createRule = async ({ rule, signal }: CreateRulesProps): Promise<RulesSchema> =>
KibanaServices.get().http.fetch<RulesSchema>(DETECTION_ENGINE_RULES_URL, {
method: 'POST',
body: JSON.stringify(rule),
signal,
});

/**
* Update provided Rule using PUT
*
* @param rule UpdateRulesSchema to be updated
* @param signal to cancel request
*
* @throws An error if response is not OK
*/
export const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise<RulesSchema> =>
KibanaServices.get().http.fetch<RulesSchema>(DETECTION_ENGINE_RULES_URL, {
method: 'PUT',
body: JSON.stringify(rule),
signal,
});

/**
* 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<NewRule> =>
KibanaServices.get().http.fetch<NewRule>(DETECTION_ENGINE_RULES_URL, {
export const patchRule = async ({ ruleProperties, signal }: PatchRuleProps): Promise<RulesSchema> =>
KibanaServices.get().http.fetch<RulesSchema>(DETECTION_ENGINE_RULES_URL, {
method: 'PATCH',
body: JSON.stringify(ruleProperties),
signal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down
Loading

0 comments on commit 568e4cd

Please sign in to comment.