Fixes UX issues with edit detector pages (#404)

* [FEATURE] Add edit detector links into breadcrumbs #393 Signed-off-by: Jovan Cvetkovic <[email protected]> * [FEATURE] Add edit detector links into breadcrumbs #393 Signed-off-by: Jovan Cvetkovic <[email protected]> * [FEATURE] Add edit detector links into breadcrumbs #393 Signed-off-by: Jovan Cvetkovic <[email protected]> * Unit tests for public components #383 [BUG] Detector Edit | Custom rule are not selected on update rules #406 Signed-off-by: Jovan Cvetkovic <[email protected]> --------- Signed-off-by: Jovan Cvetkovic <[email protected]> (cherry picked from commit 85f2ee4)
opensearch-project · Feb 14, 2023 · 21cd339 · 21cd339
1 parent cbaf2a3
commit 21cd339
Show file tree

Hide file tree

Showing 10 changed files with 146 additions and 80 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<ConfigureAlertsProps, ConfigureAlertsState> {
+  static contextType = CoreServicesContext;
+
   constructor(props: ConfigureAlertsProps) {
     super(props);
     this.state = {
@@ -55,8 +59,21 @@ export default class ConfigureAlerts extends Component<ConfigureAlertsProps, Con
 
   componentDidMount = async () => {
     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,15 +117,18 @@ export default class ConfigureAlerts extends Component<ConfigureAlertsProps, Con
 
   render() {
     const {
+      isEdit,
       detector: { triggers },
     } = this.props;
     const { loading, notificationChannels } = this.state;
     return (
       <div>
         <EuiTitle size={'m'}>
           <h3>
-            {createDetectorSteps[DetectorCreationStep.CONFIGURE_ALERTS].title +
-              ` (${triggers.length})`}
+            {isEdit
+              ? 'Edit alert triggers'
+              : createDetectorSteps[DetectorCreationStep.CONFIGURE_ALERTS].title +
+                ` (${triggers.length})`}
           </h3>
         </EuiTitle>
 
@@ -126,7 +146,7 @@ export default class ConfigureAlerts extends Component<ConfigureAlertsProps, Con
                 id={`alert-condition-${index}`}
                 buttonContent={
                   <EuiTitle>
-                    <h4>Alert trigger</h4>
+                    <h4>{alertCondition.name}</h4>
                   </EuiTitle>
                 }
                 paddingSize={'none'}

diff --git 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
@@ -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<any, any, { detectorHit: DetectorHit }> {
@@ -41,6 +42,8 @@ export const UpdateDetectorBasicDetails: React.FC<UpdateDetectorBasicDetailsProp
   const description = inputs[0].detector_input.description;
   const detectorId = props.location.pathname.replace(`${ROUTES.EDIT_DETECTOR_DETAILS}/`, '');
 
+  const context = useContext(CoreServicesContext);
+
   useEffect(() => {
     const getDetector = async () => {
       const response = (await services?.detectorsService.getDetectors()) as ServerResponse<
@@ -51,6 +54,15 @@ export const UpdateDetectorBasicDetails: React.FC<UpdateDetectorBasicDetailsProp
           (detectorHit) => 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
@@ -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<any, any, { detectorHit: DetectorHit }> {
@@ -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<
         <EuiTitle size={'m'}>
           <h3>Edit detector details</h3>
         </EuiTitle>
+
+        <EuiText size="s" color="subdued">
+          To perform threat detections, your data source will need to be in a common schema format.
+          <br />
+          Rule field names are automatically mapped to the most common fields in your log data
+          source.
+        </EuiText>
+
         <EuiSpacer size={'xxl'} />
 
         {!loading && (