From a10ae66e077648adfd491a8b0f24a2a54b0b7388 Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Mon, 20 Apr 2020 20:00:19 -0600 Subject: [PATCH] [SIEM] Adds recursive exact key checks for validation and formatter (#64023) ## Summary * Adds exact check to cause failures whenever a schema is using t.exact({}) and that schema has extra keys at any level of the schema hierarchy. * Fixes the exact check to work with unknown in a safe way and only do recursive checks for keys only when the unknown is an Object. * Changes the output to use a format error mechanism which pushes all the errors onto one line to be consistent with the response errors. We can change this as a team to whatever we want I just put it to a comma separator as that is what the responses type checks were using downstream. * Moves the downstream code up higher to be used within SIEM for timeline and detection engine. * Adds tests in TDD/red light/green light fashion where I fail the extra key checks first and then pass them second. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios --- .../detection_engine/routes/rules/validate.ts | 4 +- .../schemas/response/__mocks__/utils.ts | 37 ----------- .../response/check_type_dependents.test.ts | 10 +-- .../schemas/response/error_schema.test.ts | 5 +- .../response/find_rules_schema.test.ts | 10 +-- .../response/import_rules_schema.test.ts | 27 ++++++-- .../response/prepackaged_rules_schema.test.ts | 4 +- .../prepackaged_rules_status_schema.test.ts | 4 +- .../response/rules_bulk_schema.test.ts | 10 +-- .../schemas/response/rules_schema.test.ts | 5 +- .../type_timeline_only_schema.test.ts | 4 +- .../schemas/types/iso_date_string.test.ts | 2 +- .../schemas/types/lists_default_array.test.ts | 2 +- ...positive_integer_greater_than_zero.test.ts | 2 +- .../schemas/types/postive_integer.test.ts | 2 +- .../types/references_default_array.test.ts | 2 +- .../routes/schemas/types/risk_score.test.ts | 2 +- .../routes/schemas/types/uuid.test.ts | 2 +- .../routes/export_timelines_route.test.ts | 8 +-- .../routes/import_timelines_route.test.ts | 5 +- .../utils/build_validation/__mocks__/utils.ts | 43 +++++++++++++ .../build_validation}/exact_check.test.ts | 21 +++---- .../build_validation}/exact_check.ts | 59 ++++++++--------- .../build_validation/format_errors.test.ts} | 11 +--- .../build_validation/format_errors.ts} | 0 .../build_validation/route_validation.test.ts | 63 ++++++++++++++++--- .../build_validation/route_validation.ts | 6 +- 27 files changed, 199 insertions(+), 151 deletions(-) create mode 100644 x-pack/plugins/siem/server/utils/build_validation/__mocks__/utils.ts rename x-pack/plugins/siem/server/{lib/detection_engine/routes/schemas/response => utils/build_validation}/exact_check.test.ts (94%) rename x-pack/plugins/siem/server/{lib/detection_engine/routes/schemas/response => utils/build_validation}/exact_check.ts (59%) rename x-pack/plugins/siem/server/{lib/detection_engine/routes/schemas/response/utils.test.ts => utils/build_validation/format_errors.test.ts} (94%) rename x-pack/plugins/siem/server/{lib/detection_engine/routes/schemas/response/utils.ts => utils/build_validation/format_errors.ts} (100%) diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/rules/validate.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/rules/validate.ts index c207d075331b6..cfba40fc225a2 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/rules/validate.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/rules/validate.ts @@ -9,8 +9,9 @@ import { fold } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; import * as t from 'io-ts'; +import { formatErrors } from '../../../../utils/build_validation/format_errors'; +import { exactCheck } from '../../../../utils/build_validation/exact_check'; import { PartialAlert, FindResult } from '../../../../../../alerting/server'; -import { formatErrors } from '../schemas/response/utils'; import { isAlertType, IRuleSavedAttributesSavedObjectAttributes, @@ -19,7 +20,6 @@ import { import { OutputRuleAlertRest } from '../../types'; import { createBulkErrorObject, BulkError } from '../utils'; import { rulesSchema, RulesSchema } from '../schemas/response/rules_schema'; -import { exactCheck } from '../schemas/response/exact_check'; import { transformFindAlerts, transform, transformAlertToRule } from './utils'; import { findRulesSchema } from '../schemas/response/find_rules_schema'; import { RuleActions } from '../../rule_actions/types'; diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/__mocks__/utils.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/__mocks__/utils.ts index 21f18f9db55fb..fef6bcf42e49f 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/__mocks__/utils.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/__mocks__/utils.ts @@ -4,32 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import * as t from 'io-ts'; -import { fold } from 'fp-ts/lib/Either'; import { RulesSchema } from '../rules_schema'; import { RulesBulkSchema } from '../rules_bulk_schema'; import { ErrorSchema } from '../error_schema'; import { FindRulesSchema } from '../find_rules_schema'; -import { formatErrors } from '../utils'; -import { pipe } from 'fp-ts/lib/pipeable'; - -interface Message { - errors: t.Errors; - schema: T | {}; -} - -const onLeft = (errors: t.Errors): Message => { - return { schema: {}, errors }; -}; - -const onRight = (schema: T): Message => { - return { - schema, - errors: [], - }; -}; - -export const foldLeftRight = fold(onLeft, onRight); export const ANCHOR_DATE = '2020-02-20T03:57:54.037Z'; @@ -127,18 +105,3 @@ export const getFindResponseSingle = (): FindRulesSchema => ({ total: 1, data: [getBaseResponsePayload()], }); - -/** - * Convenience utility to keep the error message handling within tests to be - * very concise. - * @param validation The validation to get the errors from - */ -export const getPaths = (validation: t.Validation): string[] => { - return pipe( - validation, - fold( - errors => formatErrors(errors), - () => ['no errors'] - ) - ); -}; diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/check_type_dependents.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/check_type_dependents.test.ts index 0eda2a7a13d96..fbd2382e2826d 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/check_type_dependents.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/check_type_dependents.test.ts @@ -15,17 +15,13 @@ import { addQueryFields, addMlFields, } from './check_type_dependents'; -import { - foldLeftRight, - getBaseResponsePayload, - getPaths, - getMlRuleResponsePayload, -} from './__mocks__/utils'; +import { getBaseResponsePayload, getMlRuleResponsePayload } from './__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; -import { exactCheck } from './exact_check'; import { RulesSchema } from './rules_schema'; import { TypeAndTimelineOnly } from './type_timeline_only_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; describe('check_type_dependents', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/error_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/error_schema.test.ts index 11d8b85f25920..6e159a792edb6 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/error_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/error_schema.test.ts @@ -7,10 +7,11 @@ import { left } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; -import { exactCheck } from './exact_check'; -import { foldLeftRight, getErrorPayload, getPaths } from './__mocks__/utils'; +import { getErrorPayload } from './__mocks__/utils'; import { errorSchema, ErrorSchema } from './error_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('error_schema', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/find_rules_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/find_rules_schema.test.ts index f5c1970ee8c55..68b67db595d76 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/find_rules_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/find_rules_schema.test.ts @@ -5,17 +5,13 @@ */ import { findRulesSchema, FindRulesSchema } from './find_rules_schema'; -import { exactCheck } from './exact_check'; import { pipe } from 'fp-ts/lib/pipeable'; -import { - foldLeftRight, - getFindResponseSingle, - getBaseResponsePayload, - getPaths, -} from './__mocks__/utils'; +import { getFindResponseSingle, getBaseResponsePayload } from './__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; import { RulesSchema } from './rules_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { getPaths, foldLeftRight } from '../../../../../utils/build_validation/__mocks__/utils'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; describe('find_rules_schema', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/import_rules_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/import_rules_schema.test.ts index ce4bbf420a634..b0b863ebbbc0b 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/import_rules_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/import_rules_schema.test.ts @@ -4,13 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { exactCheck } from './exact_check'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from './__mocks__/utils'; -import { left } from 'fp-ts/lib/Either'; +import { left, Either } from 'fp-ts/lib/Either'; import { ImportRulesSchema, importRulesSchema } from './import_rules_schema'; import { ErrorSchema } from './error_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { getPaths, foldLeftRight } from '../../../../../utils/build_validation/__mocks__/utils'; +import { Errors } from 'io-ts'; describe('import_rules_schema', () => { beforeAll(() => { @@ -79,13 +80,31 @@ describe('import_rules_schema', () => { }); test('it should NOT validate a success that is not a boolean', () => { + type UnsafeCastForTest = Either< + Errors, + { + success: string; + success_count: number; + errors: Array< + { + id?: string | undefined; + rule_id?: string | undefined; + } & { + error: { + status_code: number; + message: string; + }; + } + >; + } + >; const payload: Omit & { success: string } = { success: 'hello', success_count: 0, errors: [], }; const decoded = importRulesSchema.decode(payload); - const checked = exactCheck(payload, decoded); + const checked = exactCheck(payload, decoded as UnsafeCastForTest); const message = pipe(checked, foldLeftRight); expect(getPaths(left(message.errors))).toEqual(['Invalid value "hello" supplied to "success"']); diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_schema.test.ts index 46667826416e1..827167c63fd58 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_schema.test.ts @@ -4,12 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { exactCheck } from './exact_check'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from './__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; import { PrePackagedRulesSchema, prePackagedRulesSchema } from './prepackaged_rules_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('prepackaged_rules_schema', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_status_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_status_schema.test.ts index 1c270ff402f75..a864667583c0a 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_status_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/prepackaged_rules_status_schema.test.ts @@ -4,15 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ -import { exactCheck } from './exact_check'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from './__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; import { PrePackagedRulesStatusSchema, prePackagedRulesStatusSchema, } from './prepackaged_rules_status_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('prepackaged_rules_schema', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_bulk_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_bulk_schema.test.ts index 8dc97d727c4d1..9a7cf5e2c2871 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_bulk_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_bulk_schema.test.ts @@ -7,17 +7,13 @@ import { left } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; -import { exactCheck } from './exact_check'; -import { - foldLeftRight, - getBaseResponsePayload, - getErrorPayload, - getPaths, -} from './__mocks__/utils'; +import { getBaseResponsePayload, getErrorPayload } from './__mocks__/utils'; import { RulesBulkSchema, rulesBulkSchema } from './rules_bulk_schema'; import { RulesSchema } from './rules_schema'; import { ErrorSchema } from './error_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('prepackaged_rule_schema', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_schema.test.ts index 4bfc51c1a66aa..82a6682e6461f 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/rules_schema.test.ts @@ -7,10 +7,11 @@ import { left } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; -import { exactCheck } from './exact_check'; import { rulesSchema, RulesSchema, removeList } from './rules_schema'; -import { foldLeftRight, getBaseResponsePayload, getPaths } from './__mocks__/utils'; +import { getBaseResponsePayload } from './__mocks__/utils'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; export const ANCHOR_DATE = '2020-02-20T03:57:54.037Z'; diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/type_timeline_only_schema.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/type_timeline_only_schema.test.ts index 68a3c8b303823..85fb124464487 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/type_timeline_only_schema.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/type_timeline_only_schema.test.ts @@ -7,10 +7,10 @@ import { left } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; -import { exactCheck } from './exact_check'; -import { foldLeftRight, getPaths } from './__mocks__/utils'; import { TypeAndTimelineOnly, typeAndTimelineOnlySchema } from './type_timeline_only_schema'; import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { exactCheck } from '../../../../../utils/build_validation/exact_check'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('prepackaged_rule_schema', () => { beforeAll(() => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/iso_date_string.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/iso_date_string.test.ts index fbafaf7f52ecb..ff62ea4443f3f 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/iso_date_string.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/iso_date_string.test.ts @@ -6,8 +6,8 @@ import { IsoDateString } from './iso_date_string'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('ios_date_string', () => { test('it should validate a iso string', () => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/lists_default_array.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/lists_default_array.test.ts index e8a9c7b0886a1..2a97c8a4a143e 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/lists_default_array.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/lists_default_array.test.ts @@ -6,8 +6,8 @@ import { ListsDefaultArray } from './lists_default_array'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('lists_default_array', () => { test('it should validate an empty array', () => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/positive_integer_greater_than_zero.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/positive_integer_greater_than_zero.test.ts index bc17303f24203..d6f21681df88f 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/positive_integer_greater_than_zero.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/positive_integer_greater_than_zero.test.ts @@ -6,8 +6,8 @@ import { PositiveIntegerGreaterThanZero } from './positive_integer_greater_than_zero'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('positive_integer_greater_than_zero', () => { test('it should validate a positive number', () => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/postive_integer.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/postive_integer.test.ts index cee451279663a..26441745a7f29 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/postive_integer.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/postive_integer.test.ts @@ -6,8 +6,8 @@ import { PositiveInteger } from './positive_integer'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('positive_integer_greater_than_zero', () => { test('it should validate a positive number', () => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/references_default_array.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/references_default_array.test.ts index 3ae8415b4f170..76f722274ce23 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/references_default_array.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/references_default_array.test.ts @@ -6,8 +6,8 @@ import { ReferencesDefaultArray } from './references_default_array'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('references_default_array', () => { test('it should validate an empty array', () => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/risk_score.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/risk_score.test.ts index ab3f80944489f..76e3445358dd9 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/risk_score.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/risk_score.test.ts @@ -6,8 +6,8 @@ import { RiskScore } from './risk_score'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('risk_score', () => { test('it should validate a positive number', () => { diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/uuid.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/uuid.test.ts index 342e6f2db2e16..7b68dbcef2d7e 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/uuid.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/types/uuid.test.ts @@ -6,8 +6,8 @@ import { UUID } from './uuid'; import { pipe } from 'fp-ts/lib/pipeable'; -import { foldLeftRight, getPaths } from '../response/__mocks__/utils'; import { left } from 'fp-ts/lib/Either'; +import { foldLeftRight, getPaths } from '../../../../../utils/build_validation/__mocks__/utils'; describe('uuid', () => { test('it should validate a uuid', () => { diff --git a/x-pack/plugins/siem/server/lib/timeline/routes/export_timelines_route.test.ts b/x-pack/plugins/siem/server/lib/timeline/routes/export_timelines_route.test.ts index 47ca25e16bd50..2bccb7c393837 100644 --- a/x-pack/plugins/siem/server/lib/timeline/routes/export_timelines_route.test.ts +++ b/x-pack/plugins/siem/server/lib/timeline/routes/export_timelines_route.test.ts @@ -85,7 +85,7 @@ describe('export timelines', () => { const result = server.validate(request); expect(result.badRequest.mock.calls[0][0]).toEqual( - 'Invalid value undefined supplied to : { ids: Array }/ids: Array' + 'Invalid value "undefined" supplied to "ids"' ); }); @@ -98,11 +98,7 @@ describe('export timelines', () => { const result = server.validate(request); expect(result.badRequest.mock.calls[1][0]).toEqual( - [ - 'Invalid value undefined supplied to : { file_name: string, exclude_export_details: ("true" | "false") }/file_name: string', - 'Invalid value undefined supplied to : { file_name: string, exclude_export_details: ("true" | "false") }/exclude_export_details: ("true" | "false")/0: "true"', - 'Invalid value undefined supplied to : { file_name: string, exclude_export_details: ("true" | "false") }/exclude_export_details: ("true" | "false")/1: "false"', - ].join('\n') + 'Invalid value "undefined" supplied to "file_name",Invalid value "undefined" supplied to "exclude_export_details",Invalid value "undefined" supplied to "exclude_export_details"' ); }); }); diff --git a/x-pack/plugins/siem/server/lib/timeline/routes/import_timelines_route.test.ts b/x-pack/plugins/siem/server/lib/timeline/routes/import_timelines_route.test.ts index 3931bf0e5bea5..9f41943cfa27f 100644 --- a/x-pack/plugins/siem/server/lib/timeline/routes/import_timelines_route.test.ts +++ b/x-pack/plugins/siem/server/lib/timeline/routes/import_timelines_route.test.ts @@ -332,10 +332,7 @@ describe('import timelines', () => { const result = server.validate(request); expect(result.badRequest).toHaveBeenCalledWith( - [ - 'Invalid value undefined supplied to : { file: (ReadableRt & { hapi: { filename: string } }) }/file: (ReadableRt & { hapi: { filename: string } })/0: ReadableRt', - 'Invalid value undefined supplied to : { file: (ReadableRt & { hapi: { filename: string } }) }/file: (ReadableRt & { hapi: { filename: string } })/1: { hapi: { filename: string } }', - ].join('\n') + 'Invalid value "undefined" supplied to "file",Invalid value "undefined" supplied to "file"' ); }); }); diff --git a/x-pack/plugins/siem/server/utils/build_validation/__mocks__/utils.ts b/x-pack/plugins/siem/server/utils/build_validation/__mocks__/utils.ts new file mode 100644 index 0000000000000..578972dda5aef --- /dev/null +++ b/x-pack/plugins/siem/server/utils/build_validation/__mocks__/utils.ts @@ -0,0 +1,43 @@ +/* + * 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 * as t from 'io-ts'; +import { fold } from 'fp-ts/lib/Either'; +import { pipe } from 'fp-ts/lib/pipeable'; +import { formatErrors } from '../format_errors'; + +interface Message { + errors: t.Errors; + schema: T | {}; +} + +const onLeft = (errors: t.Errors): Message => { + return { schema: {}, errors }; +}; + +const onRight = (schema: T): Message => { + return { + schema, + errors: [], + }; +}; + +export const foldLeftRight = fold(onLeft, onRight); + +/** + * Convenience utility to keep the error message handling within tests to be + * very concise. + * @param validation The validation to get the errors from + */ +export const getPaths = (validation: t.Validation): string[] => { + return pipe( + validation, + fold( + errors => formatErrors(errors), + () => ['no errors'] + ) + ); +}; diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/exact_check.test.ts b/x-pack/plugins/siem/server/utils/build_validation/exact_check.test.ts similarity index 94% rename from x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/exact_check.test.ts rename to x-pack/plugins/siem/server/utils/build_validation/exact_check.test.ts index cae4365d06856..1e70deaeed438 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/exact_check.test.ts +++ b/x-pack/plugins/siem/server/utils/build_validation/exact_check.test.ts @@ -5,22 +5,13 @@ */ import * as t from 'io-ts'; -import { left, right } from 'fp-ts/lib/Either'; +import { left, right, Either } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; import { foldLeftRight, getPaths } from './__mocks__/utils'; import { exactCheck, findDifferencesRecursive } from './exact_check'; -import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; describe('exact_check', () => { - beforeAll(() => { - setFeatureFlagsForTestsOnly(); - }); - - afterAll(() => { - unSetFeatureFlagsForTestsOnly(); - }); - test('it returns an error if given extra object properties', () => { const someType = t.exact( t.type({ @@ -36,14 +27,22 @@ describe('exact_check', () => { }); test('it returns an error if the data type is not as expected', () => { + type UnsafeCastForTest = Either< + t.Errors, + { + a: number; + } + >; + const someType = t.exact( t.type({ a: t.string, }) ); + const payload = { a: 1 }; const decoded = someType.decode(payload); - const checked = exactCheck(payload, decoded); + const checked = exactCheck(payload, decoded as UnsafeCastForTest); const message = pipe(checked, foldLeftRight); expect(getPaths(left(message.errors))).toEqual(['Invalid value "1" supplied to "a"']); expect(message.schema).toEqual({}); diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/exact_check.ts b/x-pack/plugins/siem/server/utils/build_validation/exact_check.ts similarity index 59% rename from x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/exact_check.ts rename to x-pack/plugins/siem/server/utils/build_validation/exact_check.ts index 6fa0472950189..9484765f9973d 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/exact_check.ts +++ b/x-pack/plugins/siem/server/utils/build_validation/exact_check.ts @@ -25,10 +25,7 @@ import { isObject, get } from 'lodash/fp'; * @param decoded The decoded either which has either an existing error or the * decoded object which could have additional keys stripped from it. */ -export const exactCheck = ( - original: object, - decoded: Either -): Either => { +export const exactCheck = (original: T, decoded: Either): Either => { const onLeft = (errors: t.Errors): Either => left(errors); const onRight = (decodedValue: T): Either => { const differences = findDifferencesRecursive(original, decodedValue); @@ -47,7 +44,7 @@ export const exactCheck = ( return pipe(decoded, fold(onLeft, onRight)); }; -export const findDifferencesRecursive = (original: object, decodedValue: T): string[] => { +export const findDifferencesRecursive = (original: T, decodedValue: T): string[] => { if (decodedValue == null) { try { // It is null and painful when the original contains an object or an array @@ -56,29 +53,33 @@ export const findDifferencesRecursive = (original: object, decodedValue: T): } catch (err) { return ['circular reference']; } + } else if (typeof original !== 'object' || original == null) { + // We are not an object or null so do not report differences + return []; + } else { + const decodedKeys = Object.keys(decodedValue); + const differences = Object.keys(original).flatMap(originalKey => { + const foundKey = decodedKeys.some(key => key === originalKey); + const topLevelKey = foundKey ? [] : [originalKey]; + // I use lodash to cheat and get an any (not going to lie ;-)) + const valueObjectOrArrayOriginal = get(originalKey, original); + const valueObjectOrArrayDecoded = get(originalKey, decodedValue); + if (isObject(valueObjectOrArrayOriginal)) { + return [ + ...topLevelKey, + ...findDifferencesRecursive(valueObjectOrArrayOriginal, valueObjectOrArrayDecoded), + ]; + } else if (Array.isArray(valueObjectOrArrayOriginal)) { + return [ + ...topLevelKey, + ...valueObjectOrArrayOriginal.flatMap((arrayElement, index) => + findDifferencesRecursive(arrayElement, get(index, valueObjectOrArrayDecoded)) + ), + ]; + } else { + return topLevelKey; + } + }); + return differences; } - const decodedKeys = Object.keys(decodedValue); - const differences = Object.keys(original).flatMap(originalKey => { - const foundKey = decodedKeys.some(key => key === originalKey); - const topLevelKey = foundKey ? [] : [originalKey]; - // I use lodash to cheat and get an any (not going to lie ;-)) - const valueObjectOrArrayOriginal = get(originalKey, original); - const valueObjectOrArrayDecoded = get(originalKey, decodedValue); - if (isObject(valueObjectOrArrayOriginal)) { - return [ - ...topLevelKey, - ...findDifferencesRecursive(valueObjectOrArrayOriginal, valueObjectOrArrayDecoded), - ]; - } else if (Array.isArray(valueObjectOrArrayOriginal)) { - return [ - ...topLevelKey, - ...valueObjectOrArrayOriginal.flatMap((arrayElement, index) => - findDifferencesRecursive(arrayElement, get(index, valueObjectOrArrayDecoded)) - ), - ]; - } else { - return topLevelKey; - } - }); - return differences; }; diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/utils.test.ts b/x-pack/plugins/siem/server/utils/build_validation/format_errors.test.ts similarity index 94% rename from x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/utils.test.ts rename to x-pack/plugins/siem/server/utils/build_validation/format_errors.test.ts index c1eb32be4895c..f9dd9e76a1d9c 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/utils.test.ts +++ b/x-pack/plugins/siem/server/utils/build_validation/format_errors.test.ts @@ -5,18 +5,9 @@ */ import * as t from 'io-ts'; -import { formatErrors } from './utils'; -import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../../feature_flags'; +import { formatErrors } from './format_errors'; describe('utils', () => { - beforeAll(() => { - setFeatureFlagsForTestsOnly(); - }); - - afterAll(() => { - unSetFeatureFlagsForTestsOnly(); - }); - test('returns an empty error message string if there are no errors', () => { const errors: t.Errors = []; const output = formatErrors(errors); diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/utils.ts b/x-pack/plugins/siem/server/utils/build_validation/format_errors.ts similarity index 100% rename from x-pack/plugins/siem/server/lib/detection_engine/routes/schemas/response/utils.ts rename to x-pack/plugins/siem/server/utils/build_validation/format_errors.ts diff --git a/x-pack/plugins/siem/server/utils/build_validation/route_validation.test.ts b/x-pack/plugins/siem/server/utils/build_validation/route_validation.test.ts index d17a8457ff81b..866dbe9480ca5 100644 --- a/x-pack/plugins/siem/server/utils/build_validation/route_validation.test.ts +++ b/x-pack/plugins/siem/server/utils/build_validation/route_validation.test.ts @@ -9,9 +9,33 @@ import * as rt from 'io-ts'; import { RouteValidationResultFactory } from '../../../../../../src/core/server/http'; describe('buildRouteValidation', () => { - const schema = rt.type({ - ids: rt.array(rt.string), - }); + const schema = rt.exact( + rt.type({ + ids: rt.array(rt.string), + }) + ); + type Schema = rt.TypeOf; + + /** + * If your schema is using exact all the way down then the validation will + * catch any additional keys that should not be present within the validation + * when the route_validation uses the exact check. + */ + const deepSchema = rt.exact( + rt.type({ + topLevel: rt.exact( + rt.type({ + secondLevel: rt.exact( + rt.type({ + thirdLevel: rt.string, + }) + ), + }) + ), + }) + ); + type DeepSchema = rt.TypeOf; + const validationResult: RouteValidationResultFactory = { ok: jest.fn().mockImplementation(validatedInput => validatedInput), badRequest: jest.fn().mockImplementation(e => e), @@ -22,18 +46,41 @@ describe('buildRouteValidation', () => { }); test('return validation error', () => { - const input = { id: 'someId' }; + const input: Omit & { id: string } = { id: 'someId' }; const result = buildRouteValidation(schema)(input, validationResult); - expect(result).toEqual( - 'Invalid value undefined supplied to : { ids: Array }/ids: Array' - ); + expect(result).toEqual('Invalid value "undefined" supplied to "ids"'); }); test('return validated input', () => { - const input = { ids: ['someId'] }; + const input: Schema = { ids: ['someId'] }; const result = buildRouteValidation(schema)(input, validationResult); expect(result).toEqual(input); }); + + test('returns validation error if given extra keys on input for an array', () => { + const input: Schema & { somethingExtra: string } = { + ids: ['someId'], + somethingExtra: 'hello', + }; + const result = buildRouteValidation(schema)(input, validationResult); + expect(result).toEqual('invalid keys "somethingExtra"'); + }); + + test('return validation input for a deep 3rd level object', () => { + const input: DeepSchema = { topLevel: { secondLevel: { thirdLevel: 'hello' } } }; + const result = buildRouteValidation(deepSchema)(input, validationResult); + expect(result).toEqual(input); + }); + + test('return validation error for a deep 3rd level object that has an extra key value of "somethingElse"', () => { + const input: DeepSchema & { + topLevel: { secondLevel: { thirdLevel: string; somethingElse: string } }; + } = { + topLevel: { secondLevel: { thirdLevel: 'hello', somethingElse: 'extraKey' } }, + }; + const result = buildRouteValidation(deepSchema)(input, validationResult); + expect(result).toEqual('invalid keys "somethingElse"'); + }); }); diff --git a/x-pack/plugins/siem/server/utils/build_validation/route_validation.ts b/x-pack/plugins/siem/server/utils/build_validation/route_validation.ts index bfcd0998fe690..30b95dcfa94ee 100644 --- a/x-pack/plugins/siem/server/utils/build_validation/route_validation.ts +++ b/x-pack/plugins/siem/server/utils/build_validation/route_validation.ts @@ -7,12 +7,13 @@ import { fold } from 'fp-ts/lib/Either'; import { pipe } from 'fp-ts/lib/pipeable'; import * as rt from 'io-ts'; -import { failure } from 'io-ts/lib/PathReporter'; import { RouteValidationFunction, RouteValidationResultFactory, RouteValidationError, } from '../../../../../../src/core/server'; +import { exactCheck } from './exact_check'; +import { formatErrors } from './format_errors'; type RequestValidationResult = | { @@ -32,8 +33,9 @@ export const buildRouteValidation = >( ) => pipe( schema.decode(inputValue), + decoded => exactCheck(inputValue, decoded), fold>( - (errors: rt.Errors) => validationResult.badRequest(failure(errors).join('\n')), + (errors: rt.Errors) => validationResult.badRequest(formatErrors(errors).join()), (validatedInput: A) => validationResult.ok(validatedInput) ) );