Skip to content

Commit

Permalink
[SIEM] Adds recursive exact key checks for validation and formatter (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
FrankHassanabad authored Apr 21, 2020
1 parent 695cda4 commit a10ae66
Show file tree
Hide file tree
Showing 27 changed files with 199 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
errors: t.Errors;
schema: T | {};
}

const onLeft = <T>(errors: t.Errors): Message<T> => {
return { schema: {}, errors };
};

const onRight = <T>(schema: T): Message<T> => {
return {
schema,
errors: [],
};
};

export const foldLeftRight = fold(onLeft, onRight);

export const ANCHOR_DATE = '2020-02-20T03:57:54.037Z';

Expand Down Expand Up @@ -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 = <A>(validation: t.Validation<A>): string[] => {
return pipe(
validation,
fold(
errors => formatErrors(errors),
() => ['no errors']
)
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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<ImportRulesSchema, 'success'> & { 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"']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> }/ids: Array<string>'
'Invalid value "undefined" supplied to "ids"'
);
});

Expand All @@ -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"'
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"'
);
});
});
Expand Down
Loading

0 comments on commit a10ae66

Please sign in to comment.