From 30b9a947d2855428a6dbc43a0d5048ee65d24fa3 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 14 Sep 2020 15:54:32 -0500 Subject: [PATCH 01/31] WIP: Adding new route for EQL Validation This is mostly boilerplate with some rough parameter definitions; the actual implementation of the validation is going to live in our validateEql function. A few tests are failing as the mocks haven't yet been implemented, I need to see the shape of the responses first. --- .../security_solution/common/constants.ts | 1 + .../request/eql_validation_schema.mock.ts | 12 ++++ .../request/eql_validation_schema.test.ts | 59 +++++++++++++++++++ .../schemas/request/eql_validation_schema.ts | 19 ++++++ .../routes/__mocks__/request_context.ts | 5 +- .../routes/__mocks__/request_responses.ts | 9 +++ .../detection_engine/routes/eql/helpers.ts | 24 ++++++++ .../routes/eql/validation_route.test.ts | 48 +++++++++++++++ .../routes/eql/validation_route.ts | 54 +++++++++++++++++ .../security_solution/server/routes/index.ts | 4 ++ 10 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.mock.ts create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts index 9321aa769423f..774bb709f490a 100644 --- a/x-pack/plugins/security_solution/common/constants.ts +++ b/x-pack/plugins/security_solution/common/constants.ts @@ -115,6 +115,7 @@ export const DETECTION_ENGINE_PREPACKAGED_URL = `${DETECTION_ENGINE_RULES_URL}/p export const DETECTION_ENGINE_PRIVILEGES_URL = `${DETECTION_ENGINE_URL}/privileges`; export const DETECTION_ENGINE_INDEX_URL = `${DETECTION_ENGINE_URL}/index`; export const DETECTION_ENGINE_TAGS_URL = `${DETECTION_ENGINE_URL}/tags`; +export const DETECTION_ENGINE_EQL_VALIDATION_URL = `${DETECTION_ENGINE_URL}/validate_eql`; export const DETECTION_ENGINE_RULES_STATUS_URL = `${DETECTION_ENGINE_RULES_URL}/_find_statuses`; export const DETECTION_ENGINE_PREPACKAGED_RULES_STATUS_URL = `${DETECTION_ENGINE_RULES_URL}/prepackaged/_status`; diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.mock.ts new file mode 100644 index 0000000000000..96afc0c85df44 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.mock.ts @@ -0,0 +1,12 @@ +/* + * 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 { EqlValidationSchema } from './eql_validation_schema'; + +export const getEqlValidationSchemaMock = (): EqlValidationSchema => ({ + index: ['index-123'], + query: 'process where process.name == "regsvr32.exe"', +}); diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts new file mode 100644 index 0000000000000..8def5cb7cc9e2 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts @@ -0,0 +1,59 @@ +/* + * 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 { exactCheck } from '../../../exact_check'; +import { foldLeftRight, getPaths } from '../../../test_utils'; +import { eqlValidationSchema, EqlValidationSchemaDecoded } from './eql_validation_schema'; +import { getEqlValidationSchemaMock } from './eql_validation_schema.mock'; + +describe('EQL validation schema', () => { + it('requires a value for index', () => { + const payload = { + ...getEqlValidationSchemaMock(), + index: undefined, + }; + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "index"', + ]); + expect(message.schema).toEqual({}); + }); + + it('requires a value for query', () => { + const payload = { + ...getEqlValidationSchemaMock(), + query: undefined, + }; + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "query"', + ]); + expect(message.schema).toEqual({}); + }); + + it('validates a payload with index and query', () => { + const payload = getEqlValidationSchemaMock(); + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + const expected: EqlValidationSchemaDecoded = { + index: ['index-123'], + query: 'process where process.name == "regsvr32.exe"', + }; + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(expected); + }); +}); diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts new file mode 100644 index 0000000000000..11ebf170f86d5 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts @@ -0,0 +1,19 @@ +/* + * 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 { index, query } from '../common/schemas'; + +export const eqlValidationSchema = t.exact( + t.type({ + index, + query, + }) +); + +export type EqlValidationSchema = t.TypeOf; +export type EqlValidationSchemaDecoded = EqlValidationSchema; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_context.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_context.ts index c45dd5bd8a281..8e379e5caa89e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_context.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_context.ts @@ -18,6 +18,7 @@ const createMockClients = () => ({ alertsClient: alertsClientMock.create(), clusterClient: elasticsearchServiceMock.createLegacyScopedClusterClient(), licensing: { license: licensingMock.createLicenseMock() }, + newClusterClient: elasticsearchServiceMock.createScopedClusterClient(), savedObjectsClient: savedObjectsClientMock.create(), appClient: siemMock.createClient(), }); @@ -31,7 +32,9 @@ const createRequestContextMock = ( core: { ...coreContext, elasticsearch: { - legacy: { ...coreContext.elasticsearch, client: clients.clusterClient }, + ...coreContext.elasticsearch, + client: clients.newClusterClient, + legacy: { ...coreContext.elasticsearch.legacy, client: clients.clusterClient }, }, savedObjects: { client: clients.savedObjectsClient }, }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts index 29c56e8ed80b1..62a725a77757b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts @@ -15,6 +15,7 @@ import { INTERNAL_RULE_ID_KEY, INTERNAL_IMMUTABLE_KEY, DETECTION_ENGINE_PREPACKAGED_URL, + DETECTION_ENGINE_EQL_VALIDATION_URL, } from '../../../../../common/constants'; import { ShardsResponse } from '../../../types'; import { @@ -28,6 +29,7 @@ import { QuerySignalsSchemaDecoded } from '../../../../../common/detection_engin import { SetSignalsStatusSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/set_signal_status_schema'; import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/create_rules_schema.mock'; import { getListArrayMock } from '../../../../../common/detection_engine/schemas/types/lists.mock'; +import { getEqlValidationSchemaMock } from '../../../../../common/detection_engine/schemas/request/eql_validation_schema.mock'; export const typicalSetStatusSignalByIdsPayload = (): SetSignalsStatusSchemaDecoded => ({ signal_ids: ['somefakeid1', 'somefakeid2'], @@ -145,6 +147,13 @@ export const getPrepackagedRulesStatusRequest = () => path: `${DETECTION_ENGINE_PREPACKAGED_URL}/_status`, }); +export const eqlValidationRequest = () => + requestMock.create({ + method: 'post', + path: DETECTION_ENGINE_EQL_VALIDATION_URL, + body: getEqlValidationSchemaMock(), + }); + export interface FindHit { page: number; perPage: number; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts new file mode 100644 index 0000000000000..b2ae3445db51e --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -0,0 +1,24 @@ +/* + * 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 interface Validation { + isValid: boolean; + errors: string[]; +} + +export interface ValidateEqlParams { + client: unknown; + index: string[]; + query: string; +} + +export const validateEql = async ({ + client, + index, + query, +}: ValidateEqlParams): Promise => { + return { isValid: true, errors: [] }; +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts new file mode 100644 index 0000000000000..135812633e1e2 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts @@ -0,0 +1,48 @@ +/* + * 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 { serverMock, requestContextMock } from '../__mocks__'; +import { eqlValidationRequest } from '../__mocks__/request_responses'; +import { eqlValidationRoute } from './validation_route'; + +describe('validate_eql route', () => { + let server: ReturnType; + let { clients, context } = requestContextMock.createTools(); + + beforeEach(() => { + server = serverMock.create(); + ({ clients, context } = requestContextMock.createTools()); + + clients.newClusterClient.asCurrentUser.eql.search.mockResolvedValue({}); + eqlValidationRoute(server.router); + }); + + describe('normal status codes', () => { + test('returns 200 when doing a normal request', async () => { + const response = await server.inject(eqlValidationRequest(), context); + expect(response.status).toEqual(200); + }); + + test('returns the payload when doing a normal request', async () => { + const response = await server.inject(eqlValidationRequest(), context); + const expectedBody = { + is_valid: true, + errors: [], + }; + expect(response.status).toEqual(200); + expect(response.body).toEqual(expectedBody); + }); + + test('returns 500 when bad response from cluster', async () => { + clients.newClusterClient.asCurrentUser.eql.search.mockImplementation(() => { + throw new Error('Test error'); + }); + const response = await server.inject(eqlValidationRequest(), context); + expect(response.status).toEqual(500); + expect(response.body).toEqual({ message: 'Test error', status_code: 500 }); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts new file mode 100644 index 0000000000000..f4c62864480e0 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts @@ -0,0 +1,54 @@ +/* + * 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 { IRouter } from '../../../../../../../../src/core/server'; +import { + eqlValidationSchema, + EqlValidationSchemaDecoded, +} from '../../../../../common/detection_engine/schemas/request/eql_validation_schema'; +import { DETECTION_ENGINE_EQL_VALIDATION_URL } from '../../../../../common/constants'; +import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; +import { transformError, buildSiemResponse } from '../utils'; +import { validateEql } from './helpers'; + +export const eqlValidationRoute = (router: IRouter) => { + router.post( + { + path: DETECTION_ENGINE_EQL_VALIDATION_URL, + validate: { + body: buildRouteValidation( + eqlValidationSchema + ), + }, + options: { + tags: ['access:securitySolution'], + }, + }, + async (context, request, response) => { + const siemResponse = buildSiemResponse(response); + const { query, index } = request.body; + const esClient = context.core.elasticsearch.client.asCurrentUser; + + try { + const validation = await validateEql({ + client: esClient, + query, + index, + }); + + return response.ok({ + body: { isValid: validation.isValid, errors: validation.errors }, + }); + } catch (err) { + const error = transformError(err); + return siemResponse.error({ + body: error.message, + statusCode: error.statusCode, + }); + } + } + ); +}; diff --git a/x-pack/plugins/security_solution/server/routes/index.ts b/x-pack/plugins/security_solution/server/routes/index.ts index 000bd875930f9..fff3cd4b2e391 100644 --- a/x-pack/plugins/security_solution/server/routes/index.ts +++ b/x-pack/plugins/security_solution/server/routes/index.ts @@ -34,6 +34,7 @@ import { createTimelinesRoute } from '../lib/timeline/routes/create_timelines_ro import { updateTimelinesRoute } from '../lib/timeline/routes/update_timelines_route'; import { getDraftTimelinesRoute } from '../lib/timeline/routes/get_draft_timelines_route'; import { cleanDraftTimelinesRoute } from '../lib/timeline/routes/clean_draft_timelines_route'; +import { eqlValidationRoute } from '../lib/detection_engine/routes/eql/validation_route'; import { SetupPlugins } from '../plugin'; import { ConfigType } from '../config'; import { installPrepackedTimelinesRoute } from '../lib/timeline/routes/install_prepacked_timelines_route'; @@ -94,4 +95,7 @@ export const initRoutes = ( // Privileges API to get the generic user privileges readPrivilegesRoute(router, security, usingEphemeralEncryptionKey); + + // Route used by the UI for form validation + eqlValidationRoute(router); }; From 01fc159d248f83481f048d67e1a0ba2a32ef6ca2 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 14 Sep 2020 21:17:52 -0500 Subject: [PATCH 02/31] Cherry-pick Marshall's EQL types --- .../security_solution/server/lib/types.ts | 55 +++++++++++++------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/types.ts b/x-pack/plugins/security_solution/server/lib/types.ts index ff89512124b66..4548786614a5d 100644 --- a/x-pack/plugins/security_solution/server/lib/types.ts +++ b/x-pack/plugins/security_solution/server/lib/types.ts @@ -26,6 +26,7 @@ import { PinnedEvent } from './pinned_event/saved_object'; import { Timeline } from './timeline/saved_object'; import { TLS } from './tls'; import { MatrixHistogram } from './matrix_histogram'; +import { SearchTypes } from './detection_engine/signals/types'; export * from './hosts'; @@ -64,6 +65,12 @@ export interface TotalValue { relation: string; } +export interface BaseHit { + _index: string; + _id: string; + _source: T; +} + export interface SearchResponse { took: number; timed_out: boolean; @@ -72,27 +79,43 @@ export interface SearchResponse { hits: { total: TotalValue | number; max_score: number; - hits: Array<{ - _index: string; - _type: string; - _id: string; - _score: number; - _source: T; - _version?: number; - _explanation?: Explanation; - fields?: string[]; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - highlight?: any; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - inner_hits?: any; - matched_queries?: string[]; - sort?: string[]; - }>; + hits: Array< + BaseHit & { + _type: string; + _score: number; + _version?: number; + _explanation?: Explanation; + fields?: string[]; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + highlight?: any; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + inner_hits?: any; + matched_queries?: string[]; + sort?: string[]; + } + >; }; // eslint-disable-next-line @typescript-eslint/no-explicit-any aggregations?: any; } +export interface EqlSequence { + join_keys: SearchTypes[]; + events: Array>; +} + +export interface EqlSearchResponse { + is_partial: boolean; + is_running: boolean; + took: number; + timed_out: boolean; + hits: { + total: TotalValue; + sequences?: Array>; + events?: Array>; + }; +} + export interface ShardsResponse { total: number; successful: number; From f81ed6c95f728579b821f4987e9e3d0d2afe4b7f Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 14 Sep 2020 21:38:58 -0500 Subject: [PATCH 03/31] Implements actual EQL validation * Performs an EQL search * filters out non-parsing errors, and returns what remains in the response * Adds mocks for empty EQL responses (we don't yet have a need for mocked data, but we will when we unit-test validateEql) --- .../routes/__mocks__/request_responses.ts | 18 +++++++- .../detection_engine/routes/eql/helpers.ts | 35 +++++++++------- .../routes/eql/validate_eql.ts | 42 +++++++++++++++++++ .../routes/eql/validation_route.test.ts | 11 +++-- .../routes/eql/validation_route.ts | 4 +- 5 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts index 62a725a77757b..464a5ed3a35cf 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts @@ -17,7 +17,7 @@ import { DETECTION_ENGINE_PREPACKAGED_URL, DETECTION_ENGINE_EQL_VALIDATION_URL, } from '../../../../../common/constants'; -import { ShardsResponse } from '../../../types'; +import { EqlSearchResponse, ShardsResponse } from '../../../types'; import { RuleAlertType, IRuleSavedAttributesSavedObjectAttributes, @@ -576,6 +576,22 @@ export const getEmptySignalsResponse = (): SignalSearchResponse => ({ }, }); +export const getEmptyEqlSearchResponse = (): EqlSearchResponse => ({ + hits: { total: { value: 0, relation: 'eq' }, events: [] }, + is_partial: false, + is_running: false, + took: 1, + timed_out: false, +}); + +export const getEmptyEqlSequencesResponse = (): EqlSearchResponse => ({ + hits: { total: { value: 0, relation: 'eq' }, sequences: [] }, + is_partial: false, + is_running: false, + took: 1, + timed_out: false, +}); + export const getSuccessfulSignalUpdateResponse = () => ({ took: 18, timed_out: false, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts index b2ae3445db51e..d5ce36dbb1571 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -4,21 +4,26 @@ * you may not use this file except in compliance with the Elastic License. */ -export interface Validation { - isValid: boolean; - errors: string[]; -} +import { ResponseError } from '@elastic/elasticsearch/lib/errors'; +import get from 'lodash/get'; + +const PARSING_ERROR_TYPE = 'parsing_exception'; -export interface ValidateEqlParams { - client: unknown; - index: string[]; - query: string; +interface ErrorCause { + type: string; + reason: string; } -export const validateEql = async ({ - client, - index, - query, -}: ValidateEqlParams): Promise => { - return { isValid: true, errors: [] }; -}; +export type ParsingError = ResponseError<{ + error: ErrorCause & { root_cause: ErrorCause[] }; +}>; + +export const isParsingErrorType = (type: unknown): boolean => type === PARSING_ERROR_TYPE; + +export const isParsingError = (error: unknown): error is ParsingError => + error instanceof ResponseError && isParsingErrorType(get(error, 'meta.body.error.type')); + +export const getParsingErrors = (error: ParsingError): string[] => + error.body.error.root_cause + .filter((cause) => isParsingErrorType(cause.type)) + .map((cause) => cause.reason); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts new file mode 100644 index 0000000000000..684d28ae6a33f --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts @@ -0,0 +1,42 @@ +/* + * 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 { ElasticsearchClient } from '../../../../../../../../src/core/server'; +import { getParsingErrors, isParsingError } from './helpers'; + +export interface Validation { + isValid: boolean; + errors: string[]; +} + +export interface ValidateEqlParams { + client: ElasticsearchClient; + index: string[]; + query: string; +} + +export const validateEql = async ({ + client, + index, + query, +}: ValidateEqlParams): Promise => { + try { + await client.eql.search({ + index: index.join(','), + body: { + query, + size: 1, + }, + }); + + return { isValid: true, errors: [] }; + } catch (error) { + if (isParsingError(error)) { + return { isValid: false, errors: getParsingErrors(error) }; + } + throw error; + } +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts index 135812633e1e2..9fe2c13cff71a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.test.ts @@ -4,8 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ +import { ApiResponse, TransportRequestPromise } from '@elastic/elasticsearch/lib/Transport'; import { serverMock, requestContextMock } from '../__mocks__'; -import { eqlValidationRequest } from '../__mocks__/request_responses'; +import { eqlValidationRequest, getEmptyEqlSearchResponse } from '../__mocks__/request_responses'; import { eqlValidationRoute } from './validation_route'; describe('validate_eql route', () => { @@ -16,7 +17,11 @@ describe('validate_eql route', () => { server = serverMock.create(); ({ clients, context } = requestContextMock.createTools()); - clients.newClusterClient.asCurrentUser.eql.search.mockResolvedValue({}); + clients.newClusterClient.asCurrentUser.eql.search.mockResolvedValue( + (getEmptyEqlSearchResponse() as unknown) as TransportRequestPromise< + ApiResponse + > + ); eqlValidationRoute(server.router); }); @@ -29,7 +34,7 @@ describe('validate_eql route', () => { test('returns the payload when doing a normal request', async () => { const response = await server.inject(eqlValidationRequest(), context); const expectedBody = { - is_valid: true, + valid: true, errors: [], }; expect(response.status).toEqual(200); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts index f4c62864480e0..c571a69f6817f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts @@ -12,7 +12,7 @@ import { import { DETECTION_ENGINE_EQL_VALIDATION_URL } from '../../../../../common/constants'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { transformError, buildSiemResponse } from '../utils'; -import { validateEql } from './helpers'; +import { validateEql } from './validate_eql'; export const eqlValidationRoute = (router: IRouter) => { router.post( @@ -40,7 +40,7 @@ export const eqlValidationRoute = (router: IRouter) => { }); return response.ok({ - body: { isValid: validation.isValid, errors: validation.errors }, + body: { valid: validation.isValid, errors: validation.errors }, }); } catch (err) { const error = transformError(err); From c45ca395a3f73b1262196c71c0bddc272572c327 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 16 Sep 2020 17:34:02 -0500 Subject: [PATCH 04/31] Adds validation calls to the EQL form input * Adds EQL Validation response schema,mocks,tests * Adds frontend function to call our validation endpoint * Adds hook, useEqlValidation, to call the above function and return state * Adds labels/help text for EQL Query bar * EqlQueryBar consumes useEqlValidation and marks the field as invalid, but does not yet report errors. --- .../response/eql_validation_schema.mock.ts | 12 ++++ .../response/eql_validation_schema.test.ts | 64 +++++++++++++++++++ .../schemas/response/eql_validation_schema.ts | 16 +++++ .../public/common/hooks/eql/api.ts | 31 +++++++++ .../common/hooks/eql/use_eql_validation.ts | 12 ++++ .../rules/eql_query_bar/eql_overview_link.tsx | 23 +++++++ .../rules/eql_query_bar/eql_query_bar.tsx | 32 +++++++++- .../components/rules/eql_query_bar/index.ts | 1 + .../rules/eql_query_bar/translations.ts | 28 ++++++++ .../rules/step_define_rule/index.tsx | 10 ++- .../rules/step_define_rule/schema.tsx | 6 -- .../rules/step_define_rule/translations.tsx | 21 ++++++ .../routes/eql/validate_eql.ts | 2 + 13 files changed, 248 insertions(+), 10 deletions(-) create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.ts create mode 100644 x-pack/plugins/security_solution/public/common/hooks/eql/api.ts create mode 100644 x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_overview_link.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts new file mode 100644 index 0000000000000..aa9bb6d16d877 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts @@ -0,0 +1,12 @@ +/* + * 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 { EqlValidationSchema } from './eql_validation_schema'; + +export const getEqlValidationResponseMock = (): EqlValidationSchema => ({ + valid: false, + errors: ['line 3:52: token recognition error at: '], +}); diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts new file mode 100644 index 0000000000000..c374f985caa60 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts @@ -0,0 +1,64 @@ +/* + * 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. + */ +/* + * 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 { exactCheck } from '../../../exact_check'; +import { foldLeftRight, getPaths } from '../../../test_utils'; +import { getEqlValidationResponseMock } from './eql_validation_schema.mock'; +import { eqlValidationSchema } from './eql_validation_schema'; + +describe('EQL validation response schema', () => { + it('validates a typical response', () => { + const payload = getEqlValidationResponseMock(); + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(getEqlValidationResponseMock()); + }); + + it('invalidates a response with extra properties', () => { + const payload = { ...getEqlValidationResponseMock(), extra: 'nope' }; + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual(['invalid keys "extra"']); + expect(message.schema).toEqual({}); + }); + + it('invalidates a response with missing properties', () => { + const payload = { ...getEqlValidationResponseMock(), valid: undefined }; + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "valid"', + ]); + expect(message.schema).toEqual({}); + }); + + it('invalidates a response with properties of the wrong type', () => { + const payload = { ...getEqlValidationResponseMock(), errors: 'should be an array' }; + const decoded = eqlValidationSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "should be an array" supplied to "errors"', + ]); + expect(message.schema).toEqual({}); + }); +}); diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.ts new file mode 100644 index 0000000000000..e999e1dd273f8 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.ts @@ -0,0 +1,16 @@ +/* + * 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'; + +export const eqlValidationSchema = t.exact( + t.type({ + valid: t.boolean, + errors: t.array(t.string), + }) +); + +export type EqlValidationSchema = t.TypeOf; diff --git a/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts b/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts new file mode 100644 index 0000000000000..11fe79910bc87 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts @@ -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 { HttpStart } from '../../../../../../../src/core/public'; +import { DETECTION_ENGINE_EQL_VALIDATION_URL } from '../../../../common/constants'; +import { EqlValidationSchema as EqlValidationRequest } from '../../../../common/detection_engine/schemas/request/eql_validation_schema'; +import { EqlValidationSchema as EqlValidationResponse } from '../../../../common/detection_engine/schemas/response/eql_validation_schema'; + +interface ApiParams { + http: HttpStart; + signal: AbortSignal; +} + +export const validateEql = async ({ + http, + query, + index, + signal, +}: ApiParams & EqlValidationRequest) => { + return http.fetch(DETECTION_ENGINE_EQL_VALIDATION_URL, { + method: 'POST', + body: JSON.stringify({ + query, + index, + }), + signal, + }); +}; diff --git a/x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts b/x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts new file mode 100644 index 0000000000000..f73cba915bdec --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts @@ -0,0 +1,12 @@ +/* + * 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 { useAsync, withOptionalSignal } from '../../../shared_imports'; +import { validateEql } from './api'; + +const validateEqlWithOptionalSignal = withOptionalSignal(validateEql); + +export const useEqlValidation = () => useAsync(validateEqlWithOptionalSignal); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_overview_link.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_overview_link.tsx new file mode 100644 index 0000000000000..22712b261b5a6 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_overview_link.tsx @@ -0,0 +1,23 @@ +/* + * 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 React from 'react'; +import styled from 'styled-components'; + +import { EuiLink, EuiText } from '@elastic/eui'; +import { EQL_OVERVIEW_LINK_TEXT } from './translations'; + +const EQL_OVERVIEW_URL = 'https://eql.readthedocs.io/en/latest/query-guide/index.html'; + +const InlineText = styled(EuiText)` + display: inline-block; +`; + +export const EqlOverviewLink = () => ( + + {EQL_OVERVIEW_LINK_TEXT} + +); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx index e3f33ea9b9b87..55323f82f8359 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx @@ -4,22 +4,47 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FC, useCallback, ChangeEvent } from 'react'; +import React, { FC, useCallback, ChangeEvent, useEffect } from 'react'; import { EuiFormRow, EuiTextArea } from '@elastic/eui'; import { FieldHook, getFieldValidityAndErrorMessage } from '../../../../shared_imports'; +import { useEqlValidation } from '../../../../common/hooks/eql/use_eql_validation'; +import { useAppToasts } from '../../../../common/hooks/use_app_toasts'; import { DefineStepRule } from '../../../pages/detection_engine/rules/types'; +import * as i18n from './translations'; +import { useKibana } from '../../../../common/lib/kibana'; export interface EqlQueryBarProps { dataTestSubj: string; field: FieldHook; idAria?: string; + index: string[]; } -export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria }) => { - const { setValue } = field; +export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, index }) => { + const { http } = useKibana().services; + const { addError } = useAppToasts(); + const { error, start, result } = useEqlValidation(); + const { setErrors, setValue } = field; const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field); const fieldValue = field.value.query.query as string; + const validationErrors = result?.errors; + + useEffect(() => { + if (error) { + addError(error, { title: i18n.EQL_VALIDATION_REQUEST_ERROR }); + } + }, [error, addError]); + + useEffect(() => { + if (validationErrors != null && validationErrors.length) { + setErrors([{ message: '' }]); + } + }, [setErrors, validationErrors]); + + const handleValidation = useCallback(() => { + start({ http, index, query: fieldValue }); + }, [fieldValue, http, index, start]); const handleChange = useCallback( (e: ChangeEvent) => { @@ -52,6 +77,7 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria fullWidth isInvalid={isInvalid} value={fieldValue} + onBlur={handleValidation} onChange={handleChange} /> diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts index fcbe069c1f57f..efdbf22ebc999 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts @@ -5,3 +5,4 @@ */ export { EqlQueryBar } from './eql_query_bar'; +export { EqlOverviewLink } from './eql_overview_link'; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts new file mode 100644 index 0000000000000..6314015f0cc82 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts @@ -0,0 +1,28 @@ +/* + * 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 { i18n } from '@kbn/i18n'; + +export const EQL_VALIDATION_REQUEST_ERROR = i18n.translate( + 'xpack.securitySolution.detectionEngine.eqlValidation.requestError', + { + defaultMessage: 'An error occurred while validating your EQL query', + } +); + +export const EQL_QUERY_BAR_LABEL = i18n.translate( + 'xpack.securitySolution.detectionEngine.eqlQueryBar.label', + { + defaultMessage: 'Enter an EQL Query', + } +); + +export const EQL_OVERVIEW_LINK_TEXT = i18n.translate( + 'xpack.securitySolution.detectionEngine.eqlOverViewLink.text', + { + defaultMessage: 'Event Query Language (EQL) Overview', + } +); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx index 7846f0c406668..936f21dbfa86f 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx @@ -47,7 +47,7 @@ import { import { schema } from './schema'; import * as i18n from './translations'; import { isEqlRule, isThresholdRule } from '../../../../../common/detection_engine/utils'; -import { EqlQueryBar } from '../eql_query_bar'; +import { EqlOverviewLink, EqlQueryBar } from '../eql_query_bar'; const CommonUseField = getUseField({ component: Field }); @@ -239,8 +239,15 @@ const StepDefineRuleComponent: FC = ({ idAria: 'detectionEngineStepDefineRuleEqlQueryBar', isDisabled: isLoading, isLoading: indexPatternsLoading, + index, dataTestSubj: 'detectionEngineStepDefineRuleEqlQueryBar', }} + config={{ + ...schema.queryBar, + helpText: i18n.EQL_QUERY_BAR_HELP_TEXT, + label: i18n.EQL_QUERY_BAR_LABEL, + labelAppend: , + }} /> ) : ( = ({ path="queryBar" config={{ ...schema.queryBar, + label: i18n.QUERY_BAR_LABEL, labelAppend: ( = { ], }, queryBar: { - label: i18n.translate( - 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldQuerBarLabel', - { - defaultMessage: 'Custom query', - } - ), validations: [ { validator: ( diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx index 860ed1831fdc6..6f4ba899a54fd 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx @@ -70,3 +70,24 @@ export const ENABLE_ML_JOB_WARNING = i18n.translate( 'This ML job is not currently running. Please set this job to run via "ML job settings" before activating this rule.', } ); + +export const QUERY_BAR_LABEL = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldQuerBarLabel', + { + defaultMessage: 'Custom query', + } +); + +export const EQL_QUERY_BAR_LABEL = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.EqlQueryBarLabel', + { + defaultMessage: 'EQL query', + } +); + +export const EQL_QUERY_BAR_HELP_TEXT = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.EqlQueryBarHelpText', + { + defaultMessage: 'Enter an EQL query', + } +); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts index 684d28ae6a33f..4a48a12f2d2a1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts @@ -25,6 +25,8 @@ export const validateEql = async ({ }: ValidateEqlParams): Promise => { try { await client.eql.search({ + // @ts-expect-error type is missing allow_no_indices + allow_no_indices: true, index: index.join(','), body: { query, From 587a60a03c4baaf2ae819e700891a3ccf36b7bbc Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 16 Sep 2020 17:40:53 -0500 Subject: [PATCH 05/31] Do not call the validation API if query is not present This causes a broader error that results in a 400 response; we can (and do) handle the case of a blank query in the form itself. --- .../components/rules/eql_query_bar/eql_query_bar.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx index 55323f82f8359..f857d85901ca0 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx @@ -43,7 +43,9 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, }, [setErrors, validationErrors]); const handleValidation = useCallback(() => { - start({ http, index, query: fieldValue }); + if (fieldValue) { + start({ http, index, query: fieldValue }); + } }, [fieldValue, http, index, start]); const handleChange = useCallback( From cfaa05c256861048f9b98de12d97f6ff7b1d648b Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 16 Sep 2020 18:24:36 -0500 Subject: [PATCH 06/31] Remove EQL Help Text It doesn't add any information for the user, and it currently looks bad when combined with validation errors. --- .../detections/components/rules/step_define_rule/index.tsx | 1 - .../components/rules/step_define_rule/translations.tsx | 7 ------- 2 files changed, 8 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx index 936f21dbfa86f..fed0b30caebe0 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx @@ -244,7 +244,6 @@ const StepDefineRuleComponent: FC = ({ }} config={{ ...schema.queryBar, - helpText: i18n.EQL_QUERY_BAR_HELP_TEXT, label: i18n.EQL_QUERY_BAR_LABEL, labelAppend: , }} diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx index 6f4ba899a54fd..6d17d5f61c18f 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/translations.tsx @@ -84,10 +84,3 @@ export const EQL_QUERY_BAR_LABEL = i18n.translate( defaultMessage: 'EQL query', } ); - -export const EQL_QUERY_BAR_HELP_TEXT = i18n.translate( - 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.EqlQueryBarHelpText', - { - defaultMessage: 'Enter an EQL query', - } -); From d68ff61dfffa7ae1e1b3111c51c7057ebd06ece1 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 16 Sep 2020 18:25:29 -0500 Subject: [PATCH 07/31] Flesh out and use our popover for displaying validation errors * Fixes issue where old errors were persisted after the user had made modifications --- .../rules/eql_query_bar/eql_query_bar.tsx | 41 +++++++++----- .../rules/eql_query_bar/errors_popover.tsx | 53 +++++++++++++++++++ .../rules/eql_query_bar/translations.ts | 14 +++++ 3 files changed, 95 insertions(+), 13 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx index f857d85901ca0..5ac6094b23871 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx @@ -4,8 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FC, useCallback, ChangeEvent, useEffect } from 'react'; -import { EuiFormRow, EuiTextArea } from '@elastic/eui'; +import React, { FC, useCallback, ChangeEvent, useEffect, useState } from 'react'; +import { EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiTextArea } from '@elastic/eui'; import { FieldHook, getFieldValidityAndErrorMessage } from '../../../../shared_imports'; import { useEqlValidation } from '../../../../common/hooks/eql/use_eql_validation'; @@ -13,6 +13,7 @@ import { useAppToasts } from '../../../../common/hooks/use_app_toasts'; import { DefineStepRule } from '../../../pages/detection_engine/rules/types'; import * as i18n from './translations'; import { useKibana } from '../../../../common/lib/kibana'; +import { ErrorsPopover } from './errors_popover'; export interface EqlQueryBarProps { dataTestSubj: string; @@ -24,11 +25,11 @@ export interface EqlQueryBarProps { export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, index }) => { const { http } = useKibana().services; const { addError } = useAppToasts(); + const [errorMessages, setErrorMessages] = useState([]); const { error, start, result } = useEqlValidation(); const { setErrors, setValue } = field; const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field); const fieldValue = field.value.query.query as string; - const validationErrors = result?.errors; useEffect(() => { if (error) { @@ -37,10 +38,11 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, }, [error, addError]); useEffect(() => { - if (validationErrors != null && validationErrors.length) { + if (result != null && result.valid === false && result.errors.length > 0) { setErrors([{ message: '' }]); + setErrorMessages(result.errors); } - }, [setErrors, validationErrors]); + }, [result, setErrors]); const handleValidation = useCallback(() => { if (fieldValue) { @@ -52,6 +54,7 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, (e: ChangeEvent) => { const newQuery = e.target.value; + setErrorMessages([]); setValue({ filters: [], query: { @@ -74,14 +77,26 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, data-test-subj={dataTestSubj} describedByIds={idAria ? [idAria] : undefined} > - + <> + + {errorMessages.length > 0 && ( + + + + + + )} + ); }; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx new file mode 100644 index 0000000000000..6b64adddce700 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx @@ -0,0 +1,53 @@ +/* + * 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 React, { FC, useCallback, useState } from 'react'; +import { EuiButtonEmpty, EuiPopover, EuiPopoverTitle, EuiText } from '@elastic/eui'; + +import * as i18n from './translations'; + +export interface ErrorsPopoverProps { + ariaLabel?: string; + errors: string[]; +} + +export const ErrorsPopover: FC = ({ ariaLabel, errors }) => { + const [isOpen, setIsOpen] = useState(false); + + const handleToggle = useCallback(() => { + setIsOpen(!isOpen); + }, [isOpen]); + + const handleClose = useCallback(() => { + setIsOpen(false); + }, []); + + return ( + + {errors.length} + + } + isOpen={isOpen} + closePopover={handleClose} + anchorPosition="downCenter" + > + <> + {i18n.EQL_VALIDATION_ERRORS_TITLE} + {errors.map((message, idx) => ( + {message} + ))} + + + ); +}; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts index 6314015f0cc82..8b016d9ad68cb 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/translations.ts @@ -13,6 +13,20 @@ export const EQL_VALIDATION_REQUEST_ERROR = i18n.translate( } ); +export const EQL_VALIDATION_ERRORS_TITLE = i18n.translate( + 'xpack.securitySolution.detectionEngine.eqlValidation.title', + { + defaultMessage: 'EQL Validation Errors', + } +); + +export const EQL_VALIDATION_ERROR_POPOVER_LABEL = i18n.translate( + 'xpack.securitySolution.detectionEngine.eqlValidation.showErrorsLabel', + { + defaultMessage: 'Show EQL Validation Errors', + } +); + export const EQL_QUERY_BAR_LABEL = i18n.translate( 'xpack.securitySolution.detectionEngine.eqlQueryBar.label', { From 15f955ee15493c5f16bb5b24b147d1eadefc21c7 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 17 Sep 2020 13:58:00 -0500 Subject: [PATCH 08/31] Include verification_exception errors as validation errors These include errors related to index fields and mappings. --- .../server/lib/detection_engine/routes/eql/helpers.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts index d5ce36dbb1571..94fc12d427a6a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -8,6 +8,7 @@ import { ResponseError } from '@elastic/elasticsearch/lib/errors'; import get from 'lodash/get'; const PARSING_ERROR_TYPE = 'parsing_exception'; +const VERIFICATION_ERROR_TYPE = 'verification_exception'; interface ErrorCause { type: string; @@ -18,7 +19,8 @@ export type ParsingError = ResponseError<{ error: ErrorCause & { root_cause: ErrorCause[] }; }>; -export const isParsingErrorType = (type: unknown): boolean => type === PARSING_ERROR_TYPE; +export const isParsingErrorType = (type: unknown): boolean => + type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE; export const isParsingError = (error: unknown): error is ParsingError => error instanceof ResponseError && isParsingErrorType(get(error, 'meta.body.error.type')); From 4cf2d550e59b0aa8bd516cf0de2555299bf94a96 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 17 Sep 2020 14:01:17 -0500 Subject: [PATCH 09/31] Generalize our validation helpers We're concerned with validation errors; the source of those errors is an implementation detail of these functions. --- .../lib/detection_engine/routes/eql/helpers.ts | 12 ++++++------ .../lib/detection_engine/routes/eql/validate_eql.ts | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts index 94fc12d427a6a..5012784d46f3b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -15,17 +15,17 @@ interface ErrorCause { reason: string; } -export type ParsingError = ResponseError<{ +export type ValidationError = ResponseError<{ error: ErrorCause & { root_cause: ErrorCause[] }; }>; -export const isParsingErrorType = (type: unknown): boolean => +export const isValidationErrorType = (type: unknown): boolean => type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE; -export const isParsingError = (error: unknown): error is ParsingError => - error instanceof ResponseError && isParsingErrorType(get(error, 'meta.body.error.type')); +export const isValidationError = (error: unknown): error is ValidationError => + error instanceof ResponseError && isValidationErrorType(get(error, 'meta.body.error.type')); -export const getParsingErrors = (error: ParsingError): string[] => +export const getValidationErrors = (error: ValidationError): string[] => error.body.error.root_cause - .filter((cause) => isParsingErrorType(cause.type)) + .filter((cause) => isValidationErrorType(cause.type)) .map((cause) => cause.reason); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts index 4a48a12f2d2a1..ae8168cb74983 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts @@ -5,7 +5,7 @@ */ import { ElasticsearchClient } from '../../../../../../../../src/core/server'; -import { getParsingErrors, isParsingError } from './helpers'; +import { getValidationErrors, isValidationError } from './helpers'; export interface Validation { isValid: boolean; @@ -36,8 +36,8 @@ export const validateEql = async ({ return { isValid: true, errors: [] }; } catch (error) { - if (isParsingError(error)) { - return { isValid: false, errors: getParsingErrors(error) }; + if (isValidationError(error)) { + return { isValid: false, errors: getValidationErrors(error) }; } throw error; } From 051db751e85d960a8c002a3afa628b42224aa2b5 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 17 Sep 2020 17:40:02 -0500 Subject: [PATCH 10/31] Move error popover and EQL reference link to footer This more closely resembles the new Eui Markdown editor, which places errors and doc links in a footer. --- .../rules/eql_query_bar/eql_query_bar.tsx | 25 ++++++----- .../components/rules/eql_query_bar/footer.tsx | 42 +++++++++++++++++++ .../components/rules/eql_query_bar/index.ts | 1 - .../rules/step_define_rule/index.tsx | 3 +- 4 files changed, 55 insertions(+), 16 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/footer.tsx diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx index 5ac6094b23871..01ecf963aebcd 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx @@ -5,7 +5,8 @@ */ import React, { FC, useCallback, ChangeEvent, useEffect, useState } from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiTextArea } from '@elastic/eui'; +import styled from 'styled-components'; +import { EuiFormRow, EuiTextArea } from '@elastic/eui'; import { FieldHook, getFieldValidityAndErrorMessage } from '../../../../shared_imports'; import { useEqlValidation } from '../../../../common/hooks/eql/use_eql_validation'; @@ -13,7 +14,14 @@ import { useAppToasts } from '../../../../common/hooks/use_app_toasts'; import { DefineStepRule } from '../../../pages/detection_engine/rules/types'; import * as i18n from './translations'; import { useKibana } from '../../../../common/lib/kibana'; -import { ErrorsPopover } from './errors_popover'; +import { EqlQueryBarFooter } from './footer'; + +const TextArea = styled(EuiTextArea)` + display: block; + border: ${({ theme }) => theme.eui.euiBorderThin}; + border-bottom: 0; + box-shadow: none; +`; export interface EqlQueryBarProps { dataTestSubj: string; @@ -78,7 +86,7 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, describedByIds={idAria ? [idAria] : undefined} > <> - = ({ dataTestSubj, field, idAria, onBlur={handleValidation} onChange={handleChange} /> - {errorMessages.length > 0 && ( - - - - - - )} + ); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/footer.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/footer.tsx new file mode 100644 index 0000000000000..19bab26f8aa58 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/footer.tsx @@ -0,0 +1,42 @@ +/* + * 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 React, { FC } from 'react'; +import styled from 'styled-components'; +import { EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui'; + +import * as i18n from './translations'; +import { ErrorsPopover } from './errors_popover'; +import { EqlOverviewLink } from './eql_overview_link'; + +export interface Props { + errors: string[]; +} + +const Container = styled(EuiPanel)` + border-radius: 0; + background: ${({ theme }) => theme.eui.euiPageBackgroundColor}; + padding: ${({ theme }) => theme.eui.euiSizeXS}; +`; + +const FlexGroup = styled(EuiFlexGroup)` + min-height: ${({ theme }) => theme.eui.euiSizeXL}; +`; + +export const EqlQueryBarFooter: FC = ({ errors }) => ( + + + + {errors.length > 0 && ( + + )} + + + + + + +); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts index efdbf22ebc999..fcbe069c1f57f 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/index.ts @@ -5,4 +5,3 @@ */ export { EqlQueryBar } from './eql_query_bar'; -export { EqlOverviewLink } from './eql_overview_link'; diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx index fed0b30caebe0..7f17d5c1cbc91 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx @@ -47,7 +47,7 @@ import { import { schema } from './schema'; import * as i18n from './translations'; import { isEqlRule, isThresholdRule } from '../../../../../common/detection_engine/utils'; -import { EqlOverviewLink, EqlQueryBar } from '../eql_query_bar'; +import { EqlQueryBar } from '../eql_query_bar'; const CommonUseField = getUseField({ component: Field }); @@ -245,7 +245,6 @@ const StepDefineRuleComponent: FC = ({ config={{ ...schema.queryBar, label: i18n.EQL_QUERY_BAR_LABEL, - labelAppend: , }} /> ) : ( From d3a6c751287b535b5a8a75f0918cd81cff5710e6 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 17 Sep 2020 17:41:09 -0500 Subject: [PATCH 11/31] Fix jest tests following additional prop --- .../rules/eql_query_bar/eql_query_bar.test.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx index 331c0ba4c4491..f31d2cf3f3ecf 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx @@ -13,21 +13,27 @@ import { EqlQueryBar, EqlQueryBarProps } from './eql_query_bar'; describe('EqlQueryBar', () => { let mockField: EqlQueryBarProps['field']; + let mockIndex: string[]; beforeEach(() => { + mockIndex = ['index-123*']; mockField = useFormFieldMock({ value: mockQueryBar, }); }); it('renders correctly', () => { - const wrapper = shallow(); + const wrapper = shallow( + + ); expect(wrapper.find('[data-test-subj="myQueryBar"]')).toHaveLength(1); }); it('sets the field value on input change', () => { - const wrapper = mount(); + const wrapper = mount( + + ); wrapper .find('[data-test-subj="eqlQueryBarTextInput"]') From 9ee7397ecc5eb2561c9b854ee192cb7910757a3f Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 18 Sep 2020 11:44:54 -0500 Subject: [PATCH 12/31] Add icon for EQL Rule card --- .../components/rules/select_rule_type/eql_search_icon.svg | 6 ++++++ .../detections/components/rules/select_rule_type/index.tsx | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/eql_search_icon.svg diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/eql_search_icon.svg b/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/eql_search_icon.svg new file mode 100644 index 0000000000000..716fff726293c --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/eql_search_icon.svg @@ -0,0 +1,6 @@ + + + + + + diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/index.tsx index 169e4f81d3498..5f36bd6ccd7b7 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/index.tsx @@ -7,6 +7,7 @@ import React, { useCallback, useMemo } from 'react'; import { EuiCard, EuiFlexGrid, EuiFlexItem, EuiFormRow, EuiIcon } from '@elastic/eui'; +import { Type } from '../../../../../common/detection_engine/schemas/common/schemas'; import { isMlRule } from '../../../../../common/machine_learning/helpers'; import { isThresholdRule, @@ -17,7 +18,7 @@ import { FieldHook } from '../../../../shared_imports'; import { useKibana } from '../../../../common/lib/kibana'; import * as i18n from './translations'; import { MlCardDescription } from './ml_card_description'; -import { Type } from '../../../../../common/detection_engine/schemas/common/schemas'; +import EqlSearchIcon from './eql_search_icon.svg'; interface SelectRuleTypeProps { describedByIds?: string[]; @@ -133,7 +134,7 @@ export const SelectRuleType: React.FC = ({ data-test-subj="eqlRuleType" title={i18n.EQL_TYPE_TITLE} description={i18n.EQL_TYPE_DESCRIPTION} - icon={} + icon={} isDisabled={eqlSelectableConfig.isDisabled && !eqlSelectableConfig.isSelected} selectable={eqlSelectableConfig} /> From 2662d77a6a394d80db5e6ff5034da4d0ceb6edc0 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 18 Sep 2020 13:23:36 -0500 Subject: [PATCH 13/31] Fixes existing EqlQueryBar tests These were broken by our use of useAppToasts and the EUI theme. --- .../components/rules/eql_query_bar/eql_query_bar.test.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx index f31d2cf3f3ecf..41f9aec7c7703 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx @@ -7,10 +7,12 @@ import React from 'react'; import { shallow, mount } from 'enzyme'; -import { useFormFieldMock } from '../../../../common/mock'; +import { TestProviders, useFormFieldMock } from '../../../../common/mock'; import { mockQueryBar } from '../../../pages/detection_engine/rules/all/__mocks__/mock'; import { EqlQueryBar, EqlQueryBarProps } from './eql_query_bar'; +jest.mock('../../../../common/lib/kibana'); + describe('EqlQueryBar', () => { let mockField: EqlQueryBarProps['field']; let mockIndex: string[]; @@ -32,7 +34,9 @@ describe('EqlQueryBar', () => { it('sets the field value on input change', () => { const wrapper = mount( - + + + ); wrapper From 490d2e32d0ce3607bac1c554a397cee5f45a7987 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Fri, 18 Sep 2020 13:44:09 -0500 Subject: [PATCH 14/31] Add unit tests around error rendering on EQL Query Bar --- .../response/eql_validation_schema.mock.ts | 5 +++ .../eql_query_bar/eql_query_bar.test.tsx | 35 +++++++++++++++++++ .../rules/eql_query_bar/errors_popover.tsx | 1 + 3 files changed, 41 insertions(+) diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts index aa9bb6d16d877..98e5db47253fb 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.mock.ts @@ -10,3 +10,8 @@ export const getEqlValidationResponseMock = (): EqlValidationSchema => ({ valid: false, errors: ['line 3:52: token recognition error at: '], }); + +export const getValidEqlValidationResponseMock = (): EqlValidationSchema => ({ + valid: true, + errors: [], +}); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx index 41f9aec7c7703..ade5a5552aab4 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx @@ -8,16 +8,25 @@ import React from 'react'; import { shallow, mount } from 'enzyme'; import { TestProviders, useFormFieldMock } from '../../../../common/mock'; +import { useEqlValidation } from '../../../../common/hooks/eql/use_eql_validation'; +import { + getEqlValidationResponseMock, + getValidEqlValidationResponseMock, +} from '../../../../../common/detection_engine/schemas/response/eql_validation_schema.mock'; import { mockQueryBar } from '../../../pages/detection_engine/rules/all/__mocks__/mock'; import { EqlQueryBar, EqlQueryBarProps } from './eql_query_bar'; jest.mock('../../../../common/lib/kibana'); +jest.mock('../../../../common/hooks/eql/use_eql_validation'); describe('EqlQueryBar', () => { let mockField: EqlQueryBarProps['field']; let mockIndex: string[]; beforeEach(() => { + (useEqlValidation as jest.Mock).mockReturnValue({ + result: getValidEqlValidationResponseMock(), + }); mockIndex = ['index-123*']; mockField = useFormFieldMock({ value: mockQueryBar, @@ -54,4 +63,30 @@ describe('EqlQueryBar', () => { expect(mockField.setValue).toHaveBeenCalledWith(expected); }); + + it('does not render errors for a valid query', () => { + const wrapper = mount( + + + + ); + + expect(wrapper.find('[data-test-subj="eql-validation-errors-popover"]').exists()).toEqual( + false + ); + }); + + it('renders errors for an invalid query', () => { + (useEqlValidation as jest.Mock).mockReturnValue({ + result: getEqlValidationResponseMock(), + }); + + const wrapper = mount( + + + + ); + + expect(wrapper.find('[data-test-subj="eql-validation-errors-popover"]').exists()).toEqual(true); + }); }); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx index 6b64adddce700..bf8c8c5718862 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx @@ -27,6 +27,7 @@ export const ErrorsPopover: FC = ({ ariaLabel, errors }) => return ( Date: Fri, 18 Sep 2020 13:56:49 -0500 Subject: [PATCH 15/31] Add tests for ErrorPopover --- .../eql_query_bar/errors_popover.test.tsx | 50 +++++++++++++++++++ .../rules/eql_query_bar/errors_popover.tsx | 5 +- 2 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.test.tsx diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.test.tsx new file mode 100644 index 0000000000000..01bd8afd0e4ab --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.test.tsx @@ -0,0 +1,50 @@ +/* + * 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 React from 'react'; +import { shallow, mount } from 'enzyme'; + +import { ErrorsPopover } from './errors_popover'; + +describe('ErrorsPopover', () => { + let mockErrors: string[]; + + beforeEach(() => { + mockErrors = []; + }); + + it('renders correctly', () => { + const wrapper = shallow(); + + expect(wrapper.find('[data-test-subj="eql-validation-errors-popover"]')).toHaveLength(1); + }); + + it('renders the number of errors by default', () => { + mockErrors = ['error', 'other', 'third']; + const wrapper = mount(); + expect( + wrapper.find('[data-test-subj="eql-validation-errors-popover"]').first().text() + ).toContain('3'); + }); + + it('renders the error messages if clicked', () => { + mockErrors = ['error', 'other']; + const wrapper = mount(); + wrapper + .find('[data-test-subj="eql-validation-errors-popover-button"]') + .first() + .simulate('click'); + + expect( + wrapper.find('[data-test-subj="eql-validation-errors-popover"]').first().text() + ).toContain('2'); + const messagesContent = wrapper + .find('[data-test-subj="eql-validation-errors-popover-content"]') + .text(); + expect(messagesContent).toContain('error'); + expect(messagesContent).toContain('other'); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx index bf8c8c5718862..a7122b7dc65d8 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/errors_popover.tsx @@ -30,6 +30,7 @@ export const ErrorsPopover: FC = ({ ariaLabel, errors }) => data-test-subj="eql-validation-errors-popover" button={ = ({ ariaLabel, errors }) => closePopover={handleClose} anchorPosition="downCenter" > - <> +
{i18n.EQL_VALIDATION_ERRORS_TITLE} {errors.map((message, idx) => ( {message} ))} - +
); }; From 7a6c8dcadb4ea38c1c6a052603aee94779ae6e1d Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 22 Sep 2020 15:04:12 -0500 Subject: [PATCH 16/31] Remove unused schema type Decode doesn't do any additional processing, so we can use t.TypeOf here (the default for buildRouteValidation). --- .../schemas/request/eql_validation_schema.test.ts | 4 ++-- .../schemas/request/eql_validation_schema.ts | 1 - .../lib/detection_engine/routes/eql/validation_route.ts | 9 ++------- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts index 8def5cb7cc9e2..84bb8e067bf75 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.test.ts @@ -9,7 +9,7 @@ import { left } from 'fp-ts/lib/Either'; import { exactCheck } from '../../../exact_check'; import { foldLeftRight, getPaths } from '../../../test_utils'; -import { eqlValidationSchema, EqlValidationSchemaDecoded } from './eql_validation_schema'; +import { eqlValidationSchema, EqlValidationSchema } from './eql_validation_schema'; import { getEqlValidationSchemaMock } from './eql_validation_schema.mock'; describe('EQL validation schema', () => { @@ -48,7 +48,7 @@ describe('EQL validation schema', () => { const decoded = eqlValidationSchema.decode(payload); const checked = exactCheck(payload, decoded); const message = pipe(checked, foldLeftRight); - const expected: EqlValidationSchemaDecoded = { + const expected: EqlValidationSchema = { index: ['index-123'], query: 'process where process.name == "regsvr32.exe"', }; diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts index 11ebf170f86d5..abbbe33a32258 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/eql_validation_schema.ts @@ -16,4 +16,3 @@ export const eqlValidationSchema = t.exact( ); export type EqlValidationSchema = t.TypeOf; -export type EqlValidationSchemaDecoded = EqlValidationSchema; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts index c571a69f6817f..478cefbc5ba8e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validation_route.ts @@ -5,10 +5,7 @@ */ import { IRouter } from '../../../../../../../../src/core/server'; -import { - eqlValidationSchema, - EqlValidationSchemaDecoded, -} from '../../../../../common/detection_engine/schemas/request/eql_validation_schema'; +import { eqlValidationSchema } from '../../../../../common/detection_engine/schemas/request/eql_validation_schema'; import { DETECTION_ENGINE_EQL_VALIDATION_URL } from '../../../../../common/constants'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { transformError, buildSiemResponse } from '../utils'; @@ -19,9 +16,7 @@ export const eqlValidationRoute = (router: IRouter) => { { path: DETECTION_ENGINE_EQL_VALIDATION_URL, validate: { - body: buildRouteValidation( - eqlValidationSchema - ), + body: buildRouteValidation(eqlValidationSchema), }, options: { tags: ['access:securitySolution'], From 2a97944b528da9b597c0b3ca51e60ac810b65c3f Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 22 Sep 2020 15:07:07 -0500 Subject: [PATCH 17/31] Remove duplicated header --- .../schemas/response/eql_validation_schema.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts index c374f985caa60..939238e340cff 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/eql_validation_schema.test.ts @@ -3,11 +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. */ -/* - * 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'; From 6655ce13247e8d42425029d19a849c8db6d8d75a Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 22 Sep 2020 17:05:21 -0500 Subject: [PATCH 18/31] Use ignore parameter to prevent EQL validations from logging errors Without `ignore: [400]` the ES client will log errors and then throw them. We can catch the error, but the log is undesirable. This updates the query to use the ignore parameter, along with updating the validation logic to work with the updated response. Adds some mocks and tests around these responses and helpers, since these will exist independent of the validation implementation. --- .../routes/eql/helpers.mock.ts | 69 +++++++++++++++++++ .../routes/eql/helpers.test.ts | 44 ++++++++++++ .../detection_engine/routes/eql/helpers.ts | 16 ++--- .../routes/eql/validate_eql.ts | 22 +++--- 4 files changed, 132 insertions(+), 19 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts new file mode 100644 index 0000000000000..ee99bd87ee324 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts @@ -0,0 +1,69 @@ +/* + * 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 { ApiResponse } from '@elastic/elasticsearch'; +import { ValidationErrorResponse } from './helpers'; + +export const getValidEqlResponse = (): ApiResponse['body'] => ({ + is_partial: false, + is_running: false, + took: 162, + timed_out: false, + hits: { + total: { + value: 1, + relation: 'eq', + }, + sequences: [], + }, +}); + +export const getEqlResponseWithValidationError = (): ValidationErrorResponse => ({ + error: { + root_cause: [ + { + type: 'verification_exception', + reason: + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + }, + ], + type: 'verification_exception', + reason: + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + }, +}); + +export const getEqlResponseWithValidationErrors = (): ValidationErrorResponse => ({ + error: { + root_cause: [ + { + type: 'verification_exception', + reason: + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + }, + { + type: 'parsing_exception', + reason: "line 1:4: mismatched input '' expecting 'where'", + }, + ], + type: 'verification_exception', + reason: + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + }, +}); + +export const getEqlResponseWithNonValidationError = (): ApiResponse['body'] => ({ + error: { + root_cause: [ + { + type: 'other_error', + reason: 'some other reason', + }, + ], + type: 'other_error', + reason: 'some other reason', + }, +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts new file mode 100644 index 0000000000000..6c6e34b3f5bdc --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts @@ -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 { getValidationErrors, isValidationErrorResponse } from './helpers'; +import { + getEqlResponseWithNonValidationError, + getEqlResponseWithValidationError, + getEqlResponseWithValidationErrors, + getValidEqlResponse, +} from './helpers.mock'; + +describe('eql validation helpers', () => { + describe('isValidationErrorResponse', () => { + it('is false for a regular response', () => { + expect(isValidationErrorResponse(getValidEqlResponse())).toEqual(false); + }); + + it('is false for a response with non-validation errors', () => { + expect(isValidationErrorResponse(getEqlResponseWithNonValidationError())).toEqual(false); + }); + + it('is true for a response with validation errors', () => { + expect(isValidationErrorResponse(getEqlResponseWithValidationError())).toEqual(true); + }); + }); + + describe('getValidationErrors', () => { + it('returns a single error for a single root cause', () => { + expect(getValidationErrors(getEqlResponseWithValidationError())).toEqual([ + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + ]); + }); + + it('returns multiple errors for multiple root causes', () => { + expect(getValidationErrors(getEqlResponseWithValidationErrors())).toEqual([ + 'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]', + "line 1:4: mismatched input '' expecting 'where'", + ]); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts index 5012784d46f3b..3183ff4241e5e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -4,8 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ResponseError } from '@elastic/elasticsearch/lib/errors'; import get from 'lodash/get'; +import has from 'lodash/has'; const PARSING_ERROR_TYPE = 'parsing_exception'; const VERIFICATION_ERROR_TYPE = 'verification_exception'; @@ -15,17 +15,17 @@ interface ErrorCause { reason: string; } -export type ValidationError = ResponseError<{ +export interface ValidationErrorResponse { error: ErrorCause & { root_cause: ErrorCause[] }; -}>; +} -export const isValidationErrorType = (type: unknown): boolean => +const isValidationErrorType = (type: unknown): boolean => type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE; -export const isValidationError = (error: unknown): error is ValidationError => - error instanceof ResponseError && isValidationErrorType(get(error, 'meta.body.error.type')); +export const isValidationErrorResponse = (response: unknown): response is ValidationErrorResponse => + has(response, 'error.type') && isValidationErrorType(get(response, 'error.type')); -export const getValidationErrors = (error: ValidationError): string[] => - error.body.error.root_cause +export const getValidationErrors = (response: ValidationErrorResponse): string[] => + response.error.root_cause .filter((cause) => isValidationErrorType(cause.type)) .map((cause) => cause.reason); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts index ae8168cb74983..e96dd485933d4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts @@ -5,7 +5,7 @@ */ import { ElasticsearchClient } from '../../../../../../../../src/core/server'; -import { getValidationErrors, isValidationError } from './helpers'; +import { getValidationErrors, isValidationErrorResponse } from './helpers'; export interface Validation { isValid: boolean; @@ -23,8 +23,8 @@ export const validateEql = async ({ index, query, }: ValidateEqlParams): Promise => { - try { - await client.eql.search({ + const response = await client.eql.search( + { // @ts-expect-error type is missing allow_no_indices allow_no_indices: true, index: index.join(','), @@ -32,13 +32,13 @@ export const validateEql = async ({ query, size: 1, }, - }); + }, + { ignore: [400] } + ); - return { isValid: true, errors: [] }; - } catch (error) { - if (isValidationError(error)) { - return { isValid: false, errors: getValidationErrors(error) }; - } - throw error; - } + const errors = isValidationErrorResponse(response.body) ? getValidationErrors(response.body) : []; + return { + errors, + isValid: errors.length === 0, + }; }; From 9c27650531d605c4c678ca20d9e997dfb1a83144 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 22 Sep 2020 17:15:04 -0500 Subject: [PATCH 19/31] Include mapping_exceptions during EQL query validation These include errors for inaccessible indexes, which should be useful to the rule writer in writing their EQL query. --- .../server/lib/detection_engine/routes/eql/helpers.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts index 3183ff4241e5e..d92ed3c468bea 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -9,6 +9,7 @@ import has from 'lodash/has'; const PARSING_ERROR_TYPE = 'parsing_exception'; const VERIFICATION_ERROR_TYPE = 'verification_exception'; +const MAPPING_ERROR_TYPE = 'mapping_exception'; interface ErrorCause { type: string; @@ -20,7 +21,7 @@ export interface ValidationErrorResponse { } const isValidationErrorType = (type: unknown): boolean => - type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE; + type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE || type === MAPPING_ERROR_TYPE; export const isValidationErrorResponse = (response: unknown): response is ValidationErrorResponse => has(response, 'error.type') && isValidationErrorType(get(response, 'error.type')); From 4ef8069d331402814bc1045b0aa72c740cc1e931 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 23 Sep 2020 20:55:58 -0500 Subject: [PATCH 20/31] Display toast messages for non-validation messages --- .../detection_engine/routes/eql/helpers.test.ts | 16 +++++++++++++++- .../lib/detection_engine/routes/eql/helpers.ts | 11 +++++++---- .../detection_engine/routes/eql/validate_eql.ts | 14 ++++++++------ 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts index 6c6e34b3f5bdc..41a1ef0faf69f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.test.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getValidationErrors, isValidationErrorResponse } from './helpers'; +import { getValidationErrors, isErrorResponse, isValidationErrorResponse } from './helpers'; import { getEqlResponseWithNonValidationError, getEqlResponseWithValidationError, @@ -13,6 +13,20 @@ import { } from './helpers.mock'; describe('eql validation helpers', () => { + describe('isErrorResponse', () => { + it('is false for a regular response', () => { + expect(isErrorResponse(getValidEqlResponse())).toEqual(false); + }); + + it('is true for a response with non-validation errors', () => { + expect(isErrorResponse(getEqlResponseWithNonValidationError())).toEqual(true); + }); + + it('is true for a response with validation errors', () => { + expect(isErrorResponse(getEqlResponseWithValidationError())).toEqual(true); + }); + }); + describe('isValidationErrorResponse', () => { it('is false for a regular response', () => { expect(isValidationErrorResponse(getValidEqlResponse())).toEqual(false); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts index d92ed3c468bea..71601574802ce 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.ts @@ -16,17 +16,20 @@ interface ErrorCause { reason: string; } -export interface ValidationErrorResponse { +export interface ErrorResponse { error: ErrorCause & { root_cause: ErrorCause[] }; } const isValidationErrorType = (type: unknown): boolean => type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE || type === MAPPING_ERROR_TYPE; -export const isValidationErrorResponse = (response: unknown): response is ValidationErrorResponse => - has(response, 'error.type') && isValidationErrorType(get(response, 'error.type')); +export const isErrorResponse = (response: unknown): response is ErrorResponse => + has(response, 'error.type'); -export const getValidationErrors = (response: ValidationErrorResponse): string[] => +export const isValidationErrorResponse = (response: unknown): response is ErrorResponse => + isErrorResponse(response) && isValidationErrorType(get(response, 'error.type')); + +export const getValidationErrors = (response: ErrorResponse): string[] => response.error.root_cause .filter((cause) => isValidationErrorType(cause.type)) .map((cause) => cause.reason); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts index e96dd485933d4..fd82f1d9453b4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts @@ -5,7 +5,7 @@ */ import { ElasticsearchClient } from '../../../../../../../../src/core/server'; -import { getValidationErrors, isValidationErrorResponse } from './helpers'; +import { getValidationErrors, isErrorResponse, isValidationErrorResponse } from './helpers'; export interface Validation { isValid: boolean; @@ -36,9 +36,11 @@ export const validateEql = async ({ { ignore: [400] } ); - const errors = isValidationErrorResponse(response.body) ? getValidationErrors(response.body) : []; - return { - errors, - isValid: errors.length === 0, - }; + if (isValidationErrorResponse(response.body)) { + return { isValid: false, errors: getValidationErrors(response.body) }; + } else if (isErrorResponse(response.body)) { + throw new Error(JSON.stringify(response.body)); + } else { + return { isValid: true, errors: [] }; + } }; From 34f43c88a75e6de2a7585d3776f398e5d957d038 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 24 Sep 2020 09:51:21 -0500 Subject: [PATCH 21/31] fix type errors This type was renamed. --- .../server/lib/detection_engine/routes/eql/helpers.mock.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts index ee99bd87ee324..4aa4c38802a92 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/helpers.mock.ts @@ -5,7 +5,7 @@ */ import { ApiResponse } from '@elastic/elasticsearch'; -import { ValidationErrorResponse } from './helpers'; +import { ErrorResponse } from './helpers'; export const getValidEqlResponse = (): ApiResponse['body'] => ({ is_partial: false, @@ -21,7 +21,7 @@ export const getValidEqlResponse = (): ApiResponse['body'] => ({ }, }); -export const getEqlResponseWithValidationError = (): ValidationErrorResponse => ({ +export const getEqlResponseWithValidationError = (): ErrorResponse => ({ error: { root_cause: [ { @@ -36,7 +36,7 @@ export const getEqlResponseWithValidationError = (): ValidationErrorResponse => }, }); -export const getEqlResponseWithValidationErrors = (): ValidationErrorResponse => ({ +export const getEqlResponseWithValidationErrors = (): ErrorResponse => ({ error: { root_cause: [ { From 9684414657458848f7e265dd6eae610a00aeffbd Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 28 Sep 2020 17:02:10 -0500 Subject: [PATCH 22/31] Do not request data in our validation request By not having the cluster retrieve/send any data, this should saves us a few CPU cycles. --- .../server/lib/detection_engine/routes/eql/validate_eql.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts index fd82f1d9453b4..ab3bbc7b06cc9 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/eql/validate_eql.ts @@ -30,7 +30,7 @@ export const validateEql = async ({ index: index.join(','), body: { query, - size: 1, + size: 0, }, }, { ignore: [400] } From 80ed453276ad88ace83e48fbd5e75c91acea2de8 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 30 Sep 2020 21:22:56 -0500 Subject: [PATCH 23/31] Move EQL validation to an async form validator Rather than invoking a custom validation hook (useEqlValidation) at custom times (onBlur) in our EqlQueryBar component, we can instead move this functionality to a form validation function and have it be invoked automatically by our form when values change. However, because we still need to handle the validation messages slightly differently (place them in a popover as opposed to an EuiFormRow), we also need custom error retrieval in the form of getValidationResults. After much pain, it was determined that the default behavior of _.debounce does not work with async validator functions, as a debounced call will not "wait" for the eventual invocation but will instead return the most recently resolved value. This leads to stale validation results and terrible UX, so I wrote a custom function (debounceAsync) that behaves like we want/need; see tests for details. --- .../common/hooks/eql/use_eql_validation.ts | 12 -- .../eql_query_bar/eql_query_bar.test.tsx | 18 +-- .../rules/eql_query_bar/eql_query_bar.tsx | 37 +++--- .../rules/eql_query_bar/validators.mock.ts | 19 ++++ .../rules/eql_query_bar/validators.test.ts | 52 +++++++++ .../rules/eql_query_bar/validators.ts | 105 ++++++++++++++++++ .../rules/step_define_rule/schema.tsx | 6 +- .../public/shared_imports.ts | 1 + 8 files changed, 200 insertions(+), 50 deletions(-) delete mode 100644 x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/validators.mock.ts create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/validators.test.ts create mode 100644 x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/validators.ts diff --git a/x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts b/x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts deleted file mode 100644 index f73cba915bdec..0000000000000 --- a/x-pack/plugins/security_solution/public/common/hooks/eql/use_eql_validation.ts +++ /dev/null @@ -1,12 +0,0 @@ -/* - * 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 { useAsync, withOptionalSignal } from '../../../shared_imports'; -import { validateEql } from './api'; - -const validateEqlWithOptionalSignal = withOptionalSignal(validateEql); - -export const useEqlValidation = () => useAsync(validateEqlWithOptionalSignal); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx index ade5a5552aab4..2acee0036dac2 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx @@ -8,25 +8,17 @@ import React from 'react'; import { shallow, mount } from 'enzyme'; import { TestProviders, useFormFieldMock } from '../../../../common/mock'; -import { useEqlValidation } from '../../../../common/hooks/eql/use_eql_validation'; -import { - getEqlValidationResponseMock, - getValidEqlValidationResponseMock, -} from '../../../../../common/detection_engine/schemas/response/eql_validation_schema.mock'; import { mockQueryBar } from '../../../pages/detection_engine/rules/all/__mocks__/mock'; import { EqlQueryBar, EqlQueryBarProps } from './eql_query_bar'; +import { getEqlValidationError } from './validators.mock'; jest.mock('../../../../common/lib/kibana'); -jest.mock('../../../../common/hooks/eql/use_eql_validation'); describe('EqlQueryBar', () => { let mockField: EqlQueryBarProps['field']; let mockIndex: string[]; beforeEach(() => { - (useEqlValidation as jest.Mock).mockReturnValue({ - result: getValidEqlValidationResponseMock(), - }); mockIndex = ['index-123*']; mockField = useFormFieldMock({ value: mockQueryBar, @@ -77,13 +69,13 @@ describe('EqlQueryBar', () => { }); it('renders errors for an invalid query', () => { - (useEqlValidation as jest.Mock).mockReturnValue({ - result: getEqlValidationResponseMock(), + const invalidMockField = useFormFieldMock({ + value: mockQueryBar, + errors: [getEqlValidationError()], }); - const wrapper = mount( - + ); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx index 01ecf963aebcd..d1300c521083d 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx @@ -8,13 +8,12 @@ import React, { FC, useCallback, ChangeEvent, useEffect, useState } from 'react' import styled from 'styled-components'; import { EuiFormRow, EuiTextArea } from '@elastic/eui'; -import { FieldHook, getFieldValidityAndErrorMessage } from '../../../../shared_imports'; -import { useEqlValidation } from '../../../../common/hooks/eql/use_eql_validation'; +import { FieldHook } from '../../../../shared_imports'; import { useAppToasts } from '../../../../common/hooks/use_app_toasts'; import { DefineStepRule } from '../../../pages/detection_engine/rules/types'; import * as i18n from './translations'; -import { useKibana } from '../../../../common/lib/kibana'; import { EqlQueryBarFooter } from './footer'; +import { getValidationResults } from './validators'; const TextArea = styled(EuiTextArea)` display: block; @@ -31,32 +30,23 @@ export interface EqlQueryBarProps { } export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, index }) => { - const { http } = useKibana().services; const { addError } = useAppToasts(); const [errorMessages, setErrorMessages] = useState([]); - const { error, start, result } = useEqlValidation(); - const { setErrors, setValue } = field; - const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field); + const { setValue } = field; + const { isValid, message, messages, error } = getValidationResults(field); const fieldValue = field.value.query.query as string; useEffect(() => { - if (error) { - addError(error, { title: i18n.EQL_VALIDATION_REQUEST_ERROR }); + if (messages && messages.length > 0) { + setErrorMessages(messages); } - }, [error, addError]); + }, [messages]); useEffect(() => { - if (result != null && result.valid === false && result.errors.length > 0) { - setErrors([{ message: '' }]); - setErrorMessages(result.errors); - } - }, [result, setErrors]); - - const handleValidation = useCallback(() => { - if (fieldValue) { - start({ http, index, query: fieldValue }); + if (error) { + addError(error, { title: i18n.EQL_VALIDATION_REQUEST_ERROR }); } - }, [fieldValue, http, index, start]); + }, [error, addError]); const handleChange = useCallback( (e: ChangeEvent) => { @@ -79,8 +69,8 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria, label={field.label} labelAppend={field.labelAppend} helpText={field.helpText} - error={errorMessage} - isInvalid={isInvalid} + error={message} + isInvalid={!isValid} fullWidth data-test-subj={dataTestSubj} describedByIds={idAria ? [idAria] : undefined} @@ -89,9 +79,8 @@ export const EqlQueryBar: FC = ({ dataTestSubj, field, idAria,