Skip to content

Commit

Permalink
[SecuritySolution][Detections] Resolves referential integrity issues …
Browse files Browse the repository at this point in the history
…when deleting value lists (#85925)

## Summary

Resolves #77324, #77325, resolves #77325, and resolves #81302


This PR addresses referential integrity issues when deleting value lists. Previously when deleting value lists, any references in Exception Lists/Items would be left behind. This PR introduces a new confirmation modal when deleting value lists that are referenced in either space aware (`simple`) or space `agnostic` exception lists.

Also includes:

* Fixed Lists plugin `quick_start.sh` as it was using endpoint exception list + value lists (unsupported)
* Adds `quick_start_value_list_references.sh` to create exception lists/items, value lists, and references to easily test
* Add support to `findExceptionList` for searching for both `simple` and `agnostic` list types
* Two new query params have been added to the `deleteListRoute`
  * `ignoreReferences` (default:false) when true, maintains pre-7.11 behavior of deleting value list without performing any additional checks. 
    * NOTE: As written, this becomes an API breaking change as existing existing calls to the same API will `409` conflict if references exist. cc @jmikell821 @Donnater 
  * `deleteReferences` (default:false) to perform dry run and identify referenced exception lists/items

## Testing
To test, run `quick_start_value_list_references.sh` and it will create all the necessary resources/references to easily exercise the above functionality. The below diagram details the resources created and how the references are wired up.

> Creates three different exception lists and value lists, and associates as
> below to test referential integrity functionality.
>
> NOTE: Endpoint lists don't support value lists, and are not tested here
>
> EL: Exception list
> ELI Exception list Item
> VL: Value list
>
>      EL1        EL2 (Agnostic)   EL3
>       |          |                |
>      ELI1       ELI2             ELI3
>       |\        /|                |
>       | \      / |                |
>       |  \    /  |                |
>       |   \  /   |                |
>       |    \/    |                |
>       |    /\    |                |
>       |   /  \   |                |
>       |  /    \  |                |
>       | /      \ |                |
>       |/        \|                |
>      VL1        VL2              VL3        VL4
>      ips.txt  ip_range.txt       text.txt   hosts.txt
>

Corner cases to be aware of:

* An exception item may have multiple value list entries -- only referenced value list entries should be removed
  * There is no API for removing individual entries. If all entries are references the entire item is deleted. If only some entries are references, the item is updated via a `PUT` (no `PATCH` support for exception items)
* It's not possible via the UI to create a space agnostic list that has value list exception items (only agnostic endpoint exception lists can be created and they do not support value lists). Please use above script to exercise this behavior.


Additional notes:
* Once the Exception List table is introduced (#85465), we can add an enhancement for deeplinking to exception lists from the reference error modal.
* The `deleteListRoute` response has been updated to include the responses from the reference checks to provide maximum flexibility
* There is no bulk API for deleting exception list items, and so they are iterated over via the `deleteExceptionListItem` API.


##### Reference error modal
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/102199153-813e1e80-3e80-11eb-8a9b-af116ca13df9.gif" />
</p>




##### Overflow example
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/102199032-5784f780-3e80-11eb-81c7-17283d002ce4.gif" />
</p>

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))

### For maintainers

- [X] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
spong authored Dec 16, 2020
1 parent 92a805f commit 4dccbca
Show file tree
Hide file tree
Showing 45 changed files with 1,267 additions and 53 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/lists/common/constants.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const NESTED_FIELD = 'parent.field';
// Exception List specific
export const ID = 'uuid_here';
export const ITEM_ID = 'some-list-item-id';
export const DETECTION_TYPE = 'detection';
export const ENDPOINT_TYPE = 'endpoint';
export const FIELD = 'host.name';
export const OPERATOR = 'included';
Expand Down
188 changes: 188 additions & 0 deletions x-pack/plugins/lists/common/format_errors.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
/*
* 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 { formatErrors } from './format_errors';

describe('utils', () => {
test('returns an empty error message string if there are no errors', () => {
const errors: t.Errors = [];
const output = formatErrors(errors);
expect(output).toEqual([]);
});

test('returns a single error message if given one', () => {
const validationError: t.ValidationError = {
context: [],
message: 'some error',
value: 'Some existing error',
};
const errors: t.Errors = [validationError];
const output = formatErrors(errors);
expect(output).toEqual(['some error']);
});

test('returns a two error messages if given two', () => {
const validationError1: t.ValidationError = {
context: [],
message: 'some error 1',
value: 'Some existing error 1',
};
const validationError2: t.ValidationError = {
context: [],
message: 'some error 2',
value: 'Some existing error 2',
};
const errors: t.Errors = [validationError1, validationError2];
const output = formatErrors(errors);
expect(output).toEqual(['some error 1', 'some error 2']);
});

test('it filters out duplicate error messages', () => {
const validationError1: t.ValidationError = {
context: [],
message: 'some error 1',
value: 'Some existing error 1',
};
const validationError2: t.ValidationError = {
context: [],
message: 'some error 1',
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1, validationError2];
const output = formatErrors(errors);
expect(output).toEqual(['some error 1']);
});

test('will use message before context if it is set', () => {
const context: t.Context = ([{ key: 'some string key' }] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
message: 'I should be used first',
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual(['I should be used first']);
});

test('will use context entry of a single string', () => {
const context: t.Context = ([{ key: 'some string key' }] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual(['Invalid value "Some existing error 1" supplied to "some string key"']);
});

test('will use two context entries of two strings', () => {
const context: t.Context = ([
{ key: 'some string key 1' },
{ key: 'some string key 2' },
] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual([
'Invalid value "Some existing error 1" supplied to "some string key 1,some string key 2"',
]);
});

test('will filter out and not use any strings of numbers', () => {
const context: t.Context = ([
{ key: '5' },
{ key: 'some string key 2' },
] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual([
'Invalid value "Some existing error 1" supplied to "some string key 2"',
]);
});

test('will filter out and not use null', () => {
const context: t.Context = ([
{ key: null },
{ key: 'some string key 2' },
] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual([
'Invalid value "Some existing error 1" supplied to "some string key 2"',
]);
});

test('will filter out and not use empty strings', () => {
const context: t.Context = ([
{ key: '' },
{ key: 'some string key 2' },
] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual([
'Invalid value "Some existing error 1" supplied to "some string key 2"',
]);
});

test('will use a name context if it cannot find a keyContext', () => {
const context: t.Context = ([
{ key: '' },
{ key: '', type: { name: 'someName' } },
] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual(['Invalid value "Some existing error 1" supplied to "someName"']);
});

test('will return an empty string if name does not exist but type does', () => {
const context: t.Context = ([{ key: '' }, { key: '', type: {} }] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: 'Some existing error 1',
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual(['Invalid value "Some existing error 1" supplied to ""']);
});

test('will stringify an error value', () => {
const context: t.Context = ([
{ key: '' },
{ key: 'some string key 2' },
] as unknown) as t.Context;
const validationError1: t.ValidationError = {
context,
value: { foo: 'some error' },
};
const errors: t.Errors = [validationError1];
const output = formatErrors(errors);
expect(output).toEqual([
'Invalid value "{"foo":"some error"}" supplied to "some string key 2"',
]);
});
});
31 changes: 31 additions & 0 deletions x-pack/plugins/lists/common/format_errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 { isObject } from 'lodash/fp';

export const formatErrors = (errors: t.Errors): string[] => {
const err = errors.map((error) => {
if (error.message != null) {
return error.message;
} else {
const keyContext = error.context
.filter(
(entry) => entry.key != null && !Number.isInteger(+entry.key) && entry.key.trim() !== ''
)
.map((entry) => entry.key)
.join(',');

const nameContext = error.context.find((entry) => entry.type?.name?.length > 0);
const suppliedValue =
keyContext !== '' ? keyContext : nameContext != null ? nameContext.type.name : '';
const value = isObject(error.value) ? JSON.stringify(error.value) : error.value;
return `Invalid value "${value}" supplied to "${suppliedValue}"`;
}
});

return [...new Set(err)];
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ import { LIST_ID } from '../../constants.mock';
import { DeleteListSchema } from './delete_list_schema';

export const getDeleteListSchemaMock = (): DeleteListSchema => ({
deleteReferences: false,
id: LIST_ID,
ignoreReferences: true,
});
19 changes: 14 additions & 5 deletions x-pack/plugins/lists/common/schemas/request/delete_list_schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@ import * as t from 'io-ts';

import { id } from '../common/schemas';
import { RequiredKeepUndefined } from '../../types';
import { DefaultStringBooleanFalse } from '../types/default_string_boolean_false';

export const deleteListSchema = t.exact(
t.type({
id,
})
);
export const deleteListSchema = t.intersection([
t.exact(
t.type({
id,
})
),
t.exact(
t.partial({
deleteReferences: DefaultStringBooleanFalse,
ignoreReferences: DefaultStringBooleanFalse,
})
),
]);

export type DeleteListSchema = RequiredKeepUndefined<t.TypeOf<typeof deleteListSchema>>;
export type DeleteListSchemaEncoded = t.OutputOf<typeof deleteListSchema>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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 { pipe } from 'fp-ts/lib/pipeable';
import { left } from 'fp-ts/lib/Either';

import { foldLeftRight, getPaths } from '../../test_utils';

import { DefaultStringBooleanFalse } from './default_string_boolean_false';

describe('default_string_boolean_false', () => {
test('it should validate a boolean false', () => {
const payload = false;
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(payload);
});

test('it should validate a boolean true', () => {
const payload = true;
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(payload);
});

test('it should not validate a number', () => {
const payload = 5;
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([
'Invalid value "5" supplied to "DefaultStringBooleanFalse"',
]);
expect(message.schema).toEqual({});
});

test('it should return a default false', () => {
const payload = null;
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(false);
});

test('it should return a default false when given a string of "false"', () => {
const payload = 'false';
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(false);
});

test('it should return a default true when given a string of "true"', () => {
const payload = 'true';
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(true);
});

test('it should return a default true when given a string of "TruE"', () => {
const payload = 'TruE';
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(true);
});

test('it should not work with a string of junk "junk"', () => {
const payload = 'junk';
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([
'Invalid value "junk" supplied to "DefaultStringBooleanFalse"',
]);
expect(message.schema).toEqual({});
});

test('it should not work with an empty string', () => {
const payload = '';
const decoded = DefaultStringBooleanFalse.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([
'Invalid value "" supplied to "DefaultStringBooleanFalse"',
]);
expect(message.schema).toEqual({});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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 { Either } from 'fp-ts/lib/Either';

/**
* Types the DefaultStringBooleanFalse as:
* - If a string this will convert the string to a boolean
* - If null or undefined, then a default false will be set
*/
export const DefaultStringBooleanFalse = new t.Type<boolean, boolean | undefined | string, unknown>(
'DefaultStringBooleanFalse',
t.boolean.is,
(input, context): Either<t.Errors, boolean> => {
if (input == null) {
return t.success(false);
} else if (typeof input === 'string' && input.toLowerCase() === 'true') {
return t.success(true);
} else if (typeof input === 'string' && input.toLowerCase() === 'false') {
return t.success(false);
} else {
return t.boolean.validate(input, context);
}
},
t.identity
);

export type DefaultStringBooleanFalseC = typeof DefaultStringBooleanFalse;
Loading

0 comments on commit 4dccbca

Please sign in to comment.