From b3ddb5836308231aeb664506ff508a48f1570a61 Mon Sep 17 00:00:00 2001 From: Amardeepsingh Siglani Date: Tue, 21 Nov 2023 13:21:53 -0800 Subject: [PATCH] Support more cases for detection; use code editor for condition (#783) * support more cases for detection; use code editor for condition Signed-off-by: Amardeepsingh Siglani * updated snapshots Signed-off-by: Amardeepsingh Siglani * fixed cypress tests Signed-off-by: Amardeepsingh Siglani --------- Signed-off-by: Amardeepsingh Siglani --- babel.config.js | 4 +- cypress/integration/1_detectors.spec.js | 5 +- cypress/integration/2_rules.spec.js | 20 +- cypress/integration/3_alerts.spec.js | 7 +- package.json | 2 + .../RuleEditor/DetectionVisualEditor.tsx | 80 +-- .../DetectionVisualEditor.test.tsx.snap | 250 ++++++-- .../RuleEditorContainer.test.tsx.snap | 250 ++++++-- .../RuleEditorForm.test.tsx.snap | 250 ++++++-- public/utils/validation.ts | 4 +- yarn.lock | 553 ++++++++++++++++-- 11 files changed, 1156 insertions(+), 269 deletions(-) diff --git a/babel.config.js b/babel.config.js index 838132a9b..449368657 100644 --- a/babel.config.js +++ b/babel.config.js @@ -13,7 +13,7 @@ module.exports = { ], plugins: [ [require('@babel/plugin-transform-runtime'), { regenerator: true }], - require('@babel/plugin-proposal-class-properties'), - require('@babel/plugin-proposal-object-rest-spread'), + require('@babel/plugin-transform-class-properties'), + require('@babel/plugin-transform-object-rest-spread'), ], }; diff --git a/cypress/integration/1_detectors.spec.js b/cypress/integration/1_detectors.spec.js index a01dc4c8c..31941e236 100644 --- a/cypress/integration/1_detectors.spec.js +++ b/cypress/integration/1_detectors.spec.js @@ -17,6 +17,7 @@ const cypressIndexWindows = 'cypress-index-windows'; const detectorName = 'test detector'; const cypressLogTypeDns = 'dns'; const sampleNotificationChannel = 'sample_chime_channel'; +const creationFailedMessage = 'Create detector failed.'; const cypressDNSRule = dns_name_rule_data.title; @@ -381,12 +382,12 @@ describe('Detectors', () => { it('...can fail creation', () => { createDetector(`${detectorName}_fail`, '.kibana_1', true); - cy.getElementByText('.euiCallOut', 'Create detector failed.'); + cy.getElementByText('.euiCallOut', creationFailedMessage); }); it('...can be created', () => { createDetector(detectorName, cypressIndexDns, false); - cy.getElementByText('.euiCallOut', 'Detector created successfully'); + cy.contains(creationFailedMessage).should('not.exist'); }); it('...basic details can be edited', () => { diff --git a/cypress/integration/2_rules.spec.js b/cypress/integration/2_rules.spec.js index b09a9d385..18b18f6ca 100644 --- a/cypress/integration/2_rules.spec.js +++ b/cypress/integration/2_rules.spec.js @@ -176,8 +176,6 @@ const fillCreateForm = () => { getMapValueField().type('FieldValue'); }); - getConditionAddButton().click({ force: true }); - // rule additional details SAMPLE_RULE.tags.forEach((tag, idx) => { getTagField(idx).type(tag); @@ -336,7 +334,7 @@ describe('Rules', () => { getMapKeyField() .parentsUntil('.euiFormRow__fieldWrapper') .siblings() - .contains('Key name is required'); + .contains('Invalid key name'); getMapKeyField().type('FieldKey'); getMapKeyField() @@ -388,13 +386,10 @@ describe('Rules', () => { getConditionField().scrollIntoView(); getConditionField().find('.euiFormErrorText').should('not.exist'); getRuleSubmitButton().click({ force: true }); - getConditionField().parents('.euiFormRow__fieldWrapper').contains('Condition is required'); - - getConditionAddButton().click({ force: true }); - getConditionField().find('.euiFormErrorText').should('not.exist'); - - getConditionRemoveButton(0).click({ force: true }); - getConditionField().parents('.euiFormRow__fieldWrapper').contains('Condition is required'); + getConditionField() + .parents('.euiFormRow__fieldWrapper') + .contains('Condition is required') + .should('not.exist'); }); it('...should validate tag field', () => { @@ -472,11 +467,6 @@ describe('Rules', () => { getMapListField().type('FieldValue'); }); - // condition field - getConditionRemoveButton(0).click({ force: true }); - toastShouldExist(); - getConditionAddButton().click({ force: true }); - // tags field getTagField(0).clearValue().type('wrong.tag'); toastShouldExist(); diff --git a/cypress/integration/3_alerts.spec.js b/cypress/integration/3_alerts.spec.js index 23a752a1e..a0018485e 100644 --- a/cypress/integration/3_alerts.spec.js +++ b/cypress/integration/3_alerts.spec.js @@ -112,12 +112,12 @@ describe('Alerts', () => { // Wait for the findings table to finish loading cy.contains('Findings (1)'); - cy.contains('Cypress USB Rule'); + cy.contains('Detection rules'); // Confirm alert findings contain expected values cy.get('tbody > tr').should(($tr) => { expect($tr, `timestamp`).to.contain(date); - expect($tr, `rule name`).to.contain('Cypress USB Rule'); + expect($tr, `detection`).to.contain('Detection rules'); expect($tr, `detector name`).to.contain(testDetector.name); expect($tr, `log type`).to.contain( `System Activity: ${getLogTypeLabel(testDetector.detector_type)}` @@ -143,7 +143,8 @@ describe('Alerts', () => { cy.get('[data-test-subj="alert-details-flyout"]').within(() => { // Wait for findings table to finish loading - cy.contains('Cypress USB Rule'); + cy.wait(3000); + cy.contains('Detection rules'); // Click the details button for the first finding cy.get('tbody > tr') diff --git a/package.json b/package.json index b25b37817..86e734f51 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,8 @@ "@cypress/request": "^3.0.0" }, "devDependencies": { + "@babel/plugin-transform-class-properties": "^7.22.9", + "@babel/plugin-transform-object-rest-spread": "^7.22.9", "@elastic/elastic-eslint-config-kibana": "link:../../packages/opensearch-eslint-config-opensearch-dashboards", "@elastic/eslint-import-resolver-kibana": "link:../../packages/osd-eslint-import-resolver-opensearch-dashboards", "@testing-library/dom": "^8.11.3", diff --git a/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx b/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx index 2c20e2392..9ea8b145a 100644 --- a/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx +++ b/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx @@ -29,10 +29,10 @@ import { EuiFilePicker, EuiButtonEmpty, EuiCallOut, + EuiCodeEditor, } from '@elastic/eui'; import _ from 'lodash'; import { validateCondition, validateDetectionFieldName } from '../../../../utils/validation'; -import { SelectionExpField } from './components/SelectionExpField'; export interface DetectionVisualEditorProps { detectionYml: string; @@ -90,7 +90,7 @@ const detectionModifierOptions = [ ]; const defaultDetectionObj: DetectionObject = { - condition: '', + condition: 'Selection_1', selections: [ { name: 'Selection_1', @@ -178,7 +178,18 @@ export class DetectionVisualEditor extends React.Component< const selectionMapJSON = detectionJSON[selectionKey]; const selectionDataEntries: SelectionData[] = []; - if (typeof selectionMapJSON === 'object') { + if (Array.isArray(selectionMapJSON)) { + selectionDataEntries.push({ + field: '', + modifier: 'all', + values: selectionMapJSON, + selectedRadioId: `${ + selectionMapJSON.length <= 1 + ? SelectionMapValueRadioId.VALUE + : SelectionMapValueRadioId.LIST + }-${selectionIdx}-0`, + }); + } else if (typeof selectionMapJSON === 'object') { Object.keys(selectionMapJSON).forEach((fieldKey, dataIdx) => { const [field, modifier] = fieldKey.split('|'); const val = selectionMapJSON[fieldKey]; @@ -212,11 +223,15 @@ export class DetectionVisualEditor extends React.Component< }; selections.forEach((selection) => { - const selectionMaps: any = {}; + let selectionMaps: any = {}; selection.data.forEach((datum) => { - const key = `${datum.field}${datum.modifier ? `|${datum.modifier}` : ''}`; - selectionMaps[key] = datum.values; + if (datum.field) { + const key = `${datum.field}${datum.modifier ? `|${datum.modifier}` : ''}`; + selectionMaps[key] = datum.values; + } else { + selectionMaps = datum.values; + } }); compiledDetection[selection.name] = selectionMaps; @@ -228,21 +243,16 @@ export class DetectionVisualEditor extends React.Component< private validateData = (selections: Selection[]) => { const { errors } = this.state; selections.map((selection, selIdx) => { - const fieldNames = new Set(); selection.data.map((data, idx) => { if ('field' in data) { const fieldName = `field_${selIdx}_${idx}`; delete errors.fields[fieldName]; - if (!data.field) { - errors.fields[fieldName] = 'Key name is required'; - } else if (fieldNames.has(data.field)) { - errors.fields[fieldName] = 'Key name already used'; - } else { - fieldNames.add(data.field); - if (!validateDetectionFieldName(data.field)) { - errors.fields[fieldName] = 'Invalid key name.'; - } + + if (!validateDetectionFieldName(data.field)) { + errors.fields[fieldName] = + 'Invalid key name. Valid characters are a-z, A-Z, 0-9, hyphens, dots, and underscores'; } + errors.touched[fieldName] = true; } @@ -343,10 +353,7 @@ export class DetectionVisualEditor extends React.Component< }; private validateCondition = (value: string) => { - const { - errors, - detectionObj: { selections }, - } = this.state; + const { errors } = this.state; value = value.trim(); delete errors.fields['condition']; if (!value) { @@ -354,18 +361,6 @@ export class DetectionVisualEditor extends React.Component< } else { if (!validateCondition(value)) { errors.fields['condition'] = 'Invalid condition.'; - } else { - const selectionNames = _.map(selections, 'name'); - const conditions = _.pull(value.split(' '), ...['and', 'or', 'not']); - conditions.map((selection) => { - if (_.indexOf(selectionNames, selection) === -1) { - errors.fields[ - 'condition' - ] = `Invalid selection name ${selection}. Allowed names: "${selectionNames.join( - ', ' - )}"`; - } - }); } } @@ -376,8 +371,6 @@ export class DetectionVisualEditor extends React.Component< }; private updateCondition = (value: string) => { - value = value.trim(); - const detectionObj: DetectionObject = { ...this.state.detectionObj, condition: value }; this.setState( { @@ -729,6 +722,7 @@ export class DetectionVisualEditor extends React.Component< { const newData = [ ...selection.data, @@ -754,14 +748,15 @@ export class DetectionVisualEditor extends React.Component< fullWidth iconType={'plusInCircle'} onClick={() => { + const selectionName = `Selection_${selections.length + 1}`; this.setState({ detectionObj: { - condition, + condition: `${condition} and ${selectionName}`, selections: [ ...selections, { ...defaultDetectionObj.selections[0], - name: `Selection_${selections.length + 1}`, + name: selectionName, }, ], }, @@ -796,11 +791,16 @@ export class DetectionVisualEditor extends React.Component< } > - this.updateCondition(value)} + onBlur={(e) => { + this.updateCondition(this.state.detectionObj.condition); + }} + data-test-subj={'rule_detection_field'} /> diff --git a/public/pages/Rules/components/RuleEditor/__snapshots__/DetectionVisualEditor.test.tsx.snap b/public/pages/Rules/components/RuleEditor/__snapshots__/DetectionVisualEditor.test.tsx.snap index afcdacc96..ec4313442 100644 --- a/public/pages/Rules/components/RuleEditor/__snapshots__/DetectionVisualEditor.test.tsx.snap +++ b/public/pages/Rules/components/RuleEditor/__snapshots__/DetectionVisualEditor.test.tsx.snap @@ -333,7 +333,8 @@ Object { class="euiSpacer euiSpacer--m" />
+