Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes UX issues with edit detector pages #404

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cypress/fixtures/sample_index_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
},
"ServiceName": {
"type": "text"
},
"DnsQuestionName": {
"type": "text"
}
}
},
Expand Down
63 changes: 20 additions & 43 deletions cypress/integration/1_detectors.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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');
});

Expand Down Expand Up @@ -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 });
});
});

Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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');

Expand Down Expand Up @@ -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();
Expand All @@ -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 });
});
Expand Down
4 changes: 3 additions & 1 deletion cypress/integration/4_findings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -45,6 +47,8 @@ interface ConfigureAlertsState {
}

export default class ConfigureAlerts extends Component<ConfigureAlertsProps, ConfigureAlertsState> {
static contextType = CoreServicesContext;

constructor(props: ConfigureAlertsProps) {
super(props);
this.state = {
Expand All @@ -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();
Expand Down Expand Up @@ -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>

Expand All @@ -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'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }> {
Expand All @@ -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<
Expand All @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }> {
Expand All @@ -35,6 +36,8 @@ export default class UpdateFieldMappings extends Component<
UpdateFieldMappingsProps,
UpdateFieldMappingsState
> {
static contextType = CoreServicesContext;

constructor(props: UpdateFieldMappingsProps) {
super(props);
const { location } = props;
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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 && (
Expand Down
Loading