From 0222d93c38093e19a11b73bc3f60bcadf7363f0a Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 15 Feb 2023 11:01:09 -0800 Subject: [PATCH] Fixes UX issues with edit detector pages (#404) (#411) * [FEATURE] Add edit detector links into breadcrumbs #393 Signed-off-by: Jovan Cvetkovic * [FEATURE] Add edit detector links into breadcrumbs #393 Signed-off-by: Jovan Cvetkovic * [FEATURE] Add edit detector links into breadcrumbs #393 Signed-off-by: Jovan Cvetkovic * Unit tests for public components #383 [BUG] Detector Edit | Custom rule are not selected on update rules #406 Signed-off-by: Jovan Cvetkovic --------- Signed-off-by: Jovan Cvetkovic (cherry picked from commit 85f2ee4c3ac7deb64f44183082ab403c213429f5) Co-authored-by: Jovan Cvetkovic --- cypress/fixtures/sample_index_settings.json | 3 + cypress/integration/1_detectors.spec.js | 63 +++++---------- cypress/integration/4_findings.spec.js | 4 +- .../containers/ConfigureAlerts.tsx | 26 +++++- .../UpdateAlertConditions.tsx | 6 +- .../UpdateBasicDetails/UpdateBasicDetails.tsx | 14 +++- .../UpdateFieldMappings.tsx | 24 +++++- .../components/UpdateRules/UpdateRules.tsx | 80 ++++++++++++------- public/utils/constants.ts | 4 + public/utils/helpers.tsx | 2 +- 10 files changed, 146 insertions(+), 80 deletions(-) diff --git a/cypress/fixtures/sample_index_settings.json b/cypress/fixtures/sample_index_settings.json index f794e671e..a8a5294a7 100644 --- a/cypress/fixtures/sample_index_settings.json +++ b/cypress/fixtures/sample_index_settings.json @@ -18,6 +18,9 @@ }, "ServiceName": { "type": "text" + }, + "DnsQuestionName": { + "type": "text" } } }, diff --git a/cypress/integration/1_detectors.spec.js b/cypress/integration/1_detectors.spec.js index 4f5a80090..3d0ff2f0e 100644 --- a/cypress/integration/1_detectors.spec.js +++ b/cypress/integration/1_detectors.spec.js @@ -5,34 +5,21 @@ import { OPENSEARCH_DASHBOARDS_URL } from '../support/constants'; import sample_index_settings from '../fixtures/sample_index_settings.json'; +import dns_rule_data from '../fixtures/integration_tests/rule/create_dns_rule.json'; const testMappings = { properties: { - 'host-hostname': { - type: 'alias', - path: 'HostName', - }, - 'windows-message': { - type: 'alias', - path: 'Message', - }, - 'winlog-provider_name': { - type: 'alias', - path: 'Provider_Name', - }, - 'winlog-event_data-ServiceName': { - type: 'alias', - path: 'ServiceName', - }, - 'winlog-event_id': { - path: 'EventID', + 'dns-question-name': { type: 'alias', + path: 'DnsQuestionName', }, }, }; +const cypressDNSRule = 'Cypress DNS Rule'; + describe('Detectors', () => { - const indexName = 'cypress-test-windows'; + const indexName = 'cypress-test-dns'; const detectorName = 'test detector'; before(() => { @@ -46,13 +33,15 @@ describe('Detectors', () => { query: { nested: { path: 'rule', - query: { bool: { must: [{ match: { 'rule.category': 'windows' } }] } }, + query: { bool: { must: [{ match: { 'rule.category': 'dns' } }] } }, }, }, }) .should('have.property', 'status', 200) ); + cy.createRule(dns_rule_data); + cy.contains(detectorName).should('not.exist'); }); @@ -94,28 +83,28 @@ describe('Detectors', () => { }).as('getSigmaRules'); // Select threat detector type (Windows logs) - cy.get(`input[id="windows"]`).click({ force: true }); + cy.get(`input[id="dns"]`).click({ force: true }); cy.wait('@getSigmaRules').then(() => { // Open Detection rules accordion cy.get('[data-test-subj="detection-rules-btn"]').click({ force: true, timeout: 5000 }); - cy.contains('table tr', 'Windows', { + cy.contains('table tr', 'DNS', { timeout: 120000, }); // find search, type USB - cy.get(`input[placeholder="Search..."]`).ospSearch('USB Device Plugged'); + cy.get(`input[placeholder="Search..."]`).ospSearch(cypressDNSRule); // Disable all rules - cy.contains('tr', 'USB Device Plugged', { timeout: 1000 }); + cy.contains('tr', cypressDNSRule, { timeout: 1000 }); cy.get('table th').within(() => { cy.get('button').first().click({ force: true }); }); // Enable single rule - cy.contains('table tr', 'USB Device Plugged').within(() => { - cy.get('button').eq(1).click({ force: true }); + cy.contains('table tr', cypressDNSRule).within(() => { + cy.get('button').eq(1).click({ force: true, timeout: 2000 }); }); }); @@ -125,14 +114,6 @@ describe('Detectors', () => { // Check that correct page now showing cy.contains('Configure field mapping'); - // Show 50 rows per page - cy.contains('Rows per page').click({ force: true }); - cy.contains('50 rows').click({ force: true }); - - // Show 50 rows per page - cy.contains('Rows per page').click({ force: true }); - cy.contains('50 rows').click({ force: true }); - // Select appropriate names to map fields to for (let field_name in testMappings.properties) { const mappedTo = testMappings.properties[field_name].path; @@ -173,10 +154,6 @@ describe('Detectors', () => { // Confirm field mappings registered cy.contains('Field mapping'); - // Show 50 rows per page - cy.contains('Rows per page').click({ force: true }); - cy.contains('50 rows').click({ force: true }); - for (let field in testMappings.properties) { const mappedTo = testMappings.properties[field].path; @@ -187,7 +164,7 @@ describe('Detectors', () => { // Confirm entries user has made cy.contains('Detector details'); cy.contains(detectorName); - cy.contains('windows'); + cy.contains('dns'); cy.contains(indexName); cy.contains('Alert on test_trigger'); @@ -288,10 +265,10 @@ describe('Detectors', () => { }); // Search for specific rule - cy.get(`input[placeholder="Search..."]`).ospSearch('USB Device'); + cy.get(`input[placeholder="Search..."]`).ospSearch(cypressDNSRule); // Toggle single search result to unchecked - cy.contains('table tr', 'USB Device Plugged').within(() => { + cy.contains('table tr', cypressDNSRule).within(() => { // Of note, timeout can sometimes work instead of wait here, but is very unreliable from case to case. cy.wait(1000); cy.get('button').eq(1).click(); @@ -312,10 +289,10 @@ describe('Detectors', () => { }); // Search for specific rule - cy.get(`input[placeholder="Search..."]`).ospSearch('USB'); + cy.get(`input[placeholder="Search..."]`).ospSearch(cypressDNSRule); // Toggle single search result to checked - cy.contains('table tr', 'USB Device Plugged').within(() => { + cy.contains('table tr', cypressDNSRule).within(() => { cy.wait(2000); cy.get('button').eq(1).click({ force: true }); }); diff --git a/cypress/integration/4_findings.spec.js b/cypress/integration/4_findings.spec.js index 670423071..afcc744e3 100644 --- a/cypress/integration/4_findings.spec.js +++ b/cypress/integration/4_findings.spec.js @@ -88,7 +88,9 @@ describe('Findings', () => { cy.get('.euiFlexItem--flexGrowZero > .euiButtonIcon').click({ force: true }); }); - it('displays finding details and create an index pattern from flyout', () => { + // TODO: this one triggers a button handler which goes throw condition and therefor is flaky + // find a better way to test this dialog, condition is based on `indexPatternId` + xit('displays finding details and create an index pattern from flyout', () => { // filter table to show only sample_detector findings cy.get(`input[placeholder="Search findings"]`).ospSearch('sample_detector'); diff --git a/public/pages/CreateDetector/components/ConfigureAlerts/containers/ConfigureAlerts.tsx b/public/pages/CreateDetector/components/ConfigureAlerts/containers/ConfigureAlerts.tsx index f8499056d..9acc179db 100644 --- a/public/pages/CreateDetector/components/ConfigureAlerts/containers/ConfigureAlerts.tsx +++ b/public/pages/CreateDetector/components/ConfigureAlerts/containers/ConfigureAlerts.tsx @@ -28,6 +28,8 @@ import { } from '../utils/helpers'; import { NotificationsService } from '../../../../../services'; import { validateName } from '../../../../../utils/validation'; +import { CoreServicesContext } from '../../../../../components/core_services'; +import { BREADCRUMBS } from '../../../../../utils/constants'; interface ConfigureAlertsProps extends RouteComponentProps { detector: Detector; @@ -45,6 +47,8 @@ interface ConfigureAlertsState { } export default class ConfigureAlerts extends Component { + static contextType = CoreServicesContext; + constructor(props: ConfigureAlertsProps) { super(props); this.state = { @@ -55,8 +59,21 @@ export default class ConfigureAlerts extends Component { const { + isEdit, + detector, detector: { triggers }, } = this.props; + + isEdit && + this.context.chrome.setBreadcrumbs([ + BREADCRUMBS.SECURITY_ANALYTICS, + BREADCRUMBS.DETECTORS, + BREADCRUMBS.DETECTORS_DETAILS(detector.name, detector.id), + { + text: 'Edit alert triggers', + }, + ]); + this.getNotificationChannels(); if (triggers.length === 0) { this.addCondition(); @@ -100,6 +117,7 @@ export default class ConfigureAlerts extends Component

- {createDetectorSteps[DetectorCreationStep.CONFIGURE_ALERTS].title + - ` (${triggers.length})`} + {isEdit + ? 'Edit alert triggers' + : createDetectorSteps[DetectorCreationStep.CONFIGURE_ALERTS].title + + ` (${triggers.length})`}

@@ -126,7 +146,7 @@ export default class ConfigureAlerts extends Component -

Alert trigger

+

{alertCondition.name}

} paddingSize={'none'} diff --git a/public/pages/Detectors/components/UpdateAlertConditions/UpdateAlertConditions.tsx b/public/pages/Detectors/components/UpdateAlertConditions/UpdateAlertConditions.tsx index 55bc57056..aa000c957 100644 --- a/public/pages/Detectors/components/UpdateAlertConditions/UpdateAlertConditions.tsx +++ b/public/pages/Detectors/components/UpdateAlertConditions/UpdateAlertConditions.tsx @@ -49,8 +49,12 @@ export default class UpdateAlertConditions extends Component< > { constructor(props: UpdateAlertConditionsProps) { super(props); + const detector = { + ...props.location.state.detectorHit._source, + id: props.location.state.detectorHit._id, + }; this.state = { - detector: props.location.state.detectorHit._source, + detector, rules: {}, rulesOptions: [], submitting: false, diff --git a/public/pages/Detectors/components/UpdateBasicDetails/UpdateBasicDetails.tsx b/public/pages/Detectors/components/UpdateBasicDetails/UpdateBasicDetails.tsx index b473143f0..ad8fed8c4 100644 --- a/public/pages/Detectors/components/UpdateBasicDetails/UpdateBasicDetails.tsx +++ b/public/pages/Detectors/components/UpdateBasicDetails/UpdateBasicDetails.tsx @@ -20,10 +20,11 @@ import { IndexService, ServicesContext } from '../../../../services'; import { DetectorSchedule } from '../../../CreateDetector/components/DefineDetector/components/DetectorSchedule/DetectorSchedule'; import { useCallback } from 'react'; import { DetectorHit, SearchDetectorsResponse } from '../../../../../server/models/interfaces'; -import { EMPTY_DEFAULT_DETECTOR, ROUTES } from '../../../../utils/constants'; +import { BREADCRUMBS, EMPTY_DEFAULT_DETECTOR, ROUTES } from '../../../../utils/constants'; import { ServerResponse } from '../../../../../server/models/types'; import { NotificationsStart } from 'opensearch-dashboards/public'; import { errorNotificationToast, successNotificationToast } from '../../../../utils/helpers'; +import { CoreServicesContext } from '../../../../components/core_services'; export interface UpdateDetectorBasicDetailsProps extends RouteComponentProps { @@ -41,6 +42,8 @@ export const UpdateDetectorBasicDetails: React.FC { const getDetector = async () => { const response = (await services?.detectorsService.getDetectors()) as ServerResponse< @@ -51,6 +54,15 @@ export const UpdateDetectorBasicDetails: React.FC detectorHit._id === detectorId ) as DetectorHit; setDetector(detectorHit._source); + + context?.chrome.setBreadcrumbs([ + BREADCRUMBS.SECURITY_ANALYTICS, + BREADCRUMBS.DETECTORS, + BREADCRUMBS.DETECTORS_DETAILS(detectorHit._source.name, detectorHit._id), + { + text: 'Edit detector details', + }, + ]); props.history.replace({ pathname: `${ROUTES.EDIT_DETECTOR_DETAILS}/${detectorId}`, state: { diff --git a/public/pages/Detectors/components/UpdateFieldMappings/UpdateFieldMappings.tsx b/public/pages/Detectors/components/UpdateFieldMappings/UpdateFieldMappings.tsx index 1ee5ec8af..f135efb03 100644 --- a/public/pages/Detectors/components/UpdateFieldMappings/UpdateFieldMappings.tsx +++ b/public/pages/Detectors/components/UpdateFieldMappings/UpdateFieldMappings.tsx @@ -5,16 +5,17 @@ import React, { Component } from 'react'; import { RouteComponentProps } from 'react-router-dom'; -import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiTitle } from '@elastic/eui'; +import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiTitle, EuiText } from '@elastic/eui'; import { Detector, FieldMapping } from '../../../../../models/interfaces'; import FieldMappingService from '../../../../services/FieldMappingService'; import { DetectorHit, SearchDetectorsResponse } from '../../../../../server/models/interfaces'; -import { EMPTY_DEFAULT_DETECTOR, ROUTES } from '../../../../utils/constants'; +import { BREADCRUMBS, EMPTY_DEFAULT_DETECTOR, ROUTES } from '../../../../utils/constants'; import { DetectorsService } from '../../../../services'; import { ServerResponse } from '../../../../../server/models/types'; import { NotificationsStart } from 'opensearch-dashboards/public'; import { errorNotificationToast, successNotificationToast } from '../../../../utils/helpers'; import EditFieldMappings from '../../containers/FieldMappings/EditFieldMapping'; +import { CoreServicesContext } from '../../../../components/core_services'; export interface UpdateFieldMappingsProps extends RouteComponentProps { @@ -35,6 +36,8 @@ export default class UpdateFieldMappings extends Component< UpdateFieldMappingsProps, UpdateFieldMappingsState > { + static contextType = CoreServicesContext; + constructor(props: UpdateFieldMappingsProps) { super(props); const { location } = props; @@ -66,6 +69,15 @@ export default class UpdateFieldMappings extends Component< const detector = detectorHit._source; detector.detector_type = detector.detector_type.toLowerCase(); + this.context.chrome.setBreadcrumbs([ + BREADCRUMBS.SECURITY_ANALYTICS, + BREADCRUMBS.DETECTORS, + BREADCRUMBS.DETECTORS_DETAILS(detectorHit._source.name, detectorHit._id), + { + text: 'Edit field mapping', + }, + ]); + history.replace({ pathname: `${ROUTES.EDIT_FIELD_MAPPINGS}/${detectorId}`, state: { @@ -156,6 +168,14 @@ export default class UpdateFieldMappings extends Component<

Edit detector details

 + + + To perform threat detections, your data source will need to be in a common schema format. +
+ Rule field names are automatically mapped to the most common fields in your log data + source. + + {!loading && ( diff --git a/public/pages/Detectors/components/UpdateRules/UpdateRules.tsx b/public/pages/Detectors/components/UpdateRules/UpdateRules.tsx index a6806ecba..9e7193758 100644 --- a/public/pages/Detectors/components/UpdateRules/UpdateRules.tsx +++ b/public/pages/Detectors/components/UpdateRules/UpdateRules.tsx @@ -14,14 +14,16 @@ import { RouteComponentProps } from 'react-router-dom'; import { RuleItem } from '../../../CreateDetector/components/DefineDetector/components/DetectionRules/types/interfaces'; import { Detector } from '../../../../../models/interfaces'; import { DetectionRulesTable } from '../../../CreateDetector/components/DefineDetector/components/DetectionRules/DetectionRulesTable'; -import { EMPTY_DEFAULT_DETECTOR, ROUTES } from '../../../../utils/constants'; +import { BREADCRUMBS, EMPTY_DEFAULT_DETECTOR, ROUTES } from '../../../../utils/constants'; import { ServicesContext } from '../../../../services'; import { ServerResponse } from '../../../../../server/models/types'; import { NotificationsStart } from 'opensearch-dashboards/public'; +import { CoreServicesContext } from '../../../../components/core_services'; import { errorNotificationToast, successNotificationToast } from '../../../../utils/helpers'; import { RuleTableItem } from '../../../Rules/utils/helpers'; import { RuleViewerFlyout } from '../../../Rules/components/RuleViewerFlyout/RuleViewerFlyout'; import { RulesViewModelActor } from '../../../Rules/models/RulesViewModelActor'; +import { ContentPanel } from '../../../../components/ContentPanel'; export interface UpdateDetectorRulesProps extends RouteComponentProps< @@ -47,6 +49,8 @@ export const UpdateDetectorRules: React.FC = (props) = [services] ); + const context = useContext(CoreServicesContext); + useEffect(() => { const getDetector = async () => { setLoading(true); @@ -59,6 +63,15 @@ export const UpdateDetectorRules: React.FC = (props) = ) as DetectorHit; const newDetector = { ...detectorHit._source, id: detectorId }; setDetector(newDetector); + + context?.chrome.setBreadcrumbs([ + BREADCRUMBS.SECURITY_ANALYTICS, + BREADCRUMBS.DETECTORS, + BREADCRUMBS.DETECTORS_DETAILS(detectorHit._source.name, detectorHit._id), + { + text: 'Edit detector rules', + }, + ]); await getRules(newDetector); } else { errorNotificationToast(props.notifications, 'retrieve', 'detector', response.error); @@ -67,9 +80,13 @@ export const UpdateDetectorRules: React.FC = (props) = }; const getRules = async (detector: Detector) => { - const enabledRuleIds = detector.inputs[0].detector_input.pre_packaged_rules.map( + let enabledRuleIds = detector.inputs[0].detector_input.pre_packaged_rules.map( + (rule) => rule.id + ); + const enabledCustomRuleIds = detector.inputs[0].detector_input.custom_rules.map( (rule) => rule.id ); + enabledRuleIds = enabledRuleIds.concat(enabledCustomRuleIds); const allRules = await rulesViewModelActor?.fetchRules(undefined, { bool: { @@ -205,34 +222,41 @@ export const UpdateDetectorRules: React.FC = (props) = - + item.active).length + })`} + > + - + + + + + + Cancel + + - - - - Cancel - - - - - Save changes - - - + + + Save changes + + + + ); }; diff --git a/public/utils/constants.ts b/public/utils/constants.ts index df7f10d49..9b5600a36 100644 --- a/public/utils/constants.ts +++ b/public/utils/constants.ts @@ -55,6 +55,10 @@ export const BREADCRUMBS = Object.freeze({ text: `${name}`, href: `#${ROUTES.DETECTOR_DETAILS}/${detectorId}`, }), + DETECTORS_EDIT_DETAILS: (name: string, detectorId: string) => ({ + text: `${name}`, + href: `#${ROUTES.EDIT_DETECTOR_DETAILS}/${detectorId}`, + }), RULES: { text: 'Rules', href: `#${ROUTES.RULES}` }, ALERTS: { text: 'Alerts', href: `#${ROUTES.ALERTS}` }, RULES_CREATE: { text: 'Create rule', href: `#${ROUTES.RULES_CREATE}` }, diff --git a/public/utils/helpers.tsx b/public/utils/helpers.tsx index cf3cf2ca1..d7b297ff2 100644 --- a/public/utils/helpers.tsx +++ b/public/utils/helpers.tsx @@ -45,7 +45,7 @@ export function createTextDetailsGroup( columnNum?: number ) { const createFormRow = (label: string, content: string, url?: string) => { - const dataTestSubj = label.toLowerCase().replaceAll(' ', '-'); + const dataTestSubj = label.toLowerCase().replace(/ /g, '-'); return ( {label}}> {url ? (