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.

Sign up for GitHub

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

[Security Solution][Detection Alerts] Fixes follow-up alert refresh bugs #112169

Merged
merged 12 commits into from
Oct 12, 2021
Merged

[Security Solution][Detection Alerts] Fixes follow-up alert refresh bugs #112169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e7941de
fixes take action exception options
dplumlee Sep 14, 2021
92ced82
updates snapshots
dplumlee Sep 14, 2021
0878902
fixes timeline bugs
dplumlee Sep 22, 2021
62a905f
fixes prettier stuff
dplumlee Sep 23, 2021
68bb7c4
updates tests
dplumlee Oct 4, 2021
acbe509
adds trend test
dplumlee Oct 6, 2021
b570bb0
adds comments
dplumlee Oct 6, 2021
8c03277
Merge branch 'master' into alert-status-refresh-pt2
kibanamachine Oct 11, 2021
ebd6140
Merge branch 'master' into alert-status-refresh-pt2
kibanamachine Oct 11, 2021
374c023
Merge branch 'master' into alert-status-refresh-pt2
kibanamachine Oct 11, 2021
9239483
Merge branch 'master' into alert-status-refresh-pt2
kibanamachine Oct 12, 2021
8cf6f6d
Merge branch 'master' into alert-status-refresh-pt2
kibanamachine Oct 12, 2021
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
11 changes: 10 additions & 1 deletion x-pack/plugins/security_solution/cypress/integration/detection_alerts/acknowledged.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
*/

import { getNewRule } from '../../objects/rule';
import { ALERTS_COUNT, TAKE_ACTION_POPOVER_BTN } from '../../screens/alerts';
import {
ALERTS_COUNT,
TAKE_ACTION_POPOVER_BTN,
ALERT_COUNT_TABLE_FIRST_ROW_COUNT,
} from '../../screens/alerts';

import {
selectNumberOfAlerts,
Expand Down Expand Up @@ -50,11 +54,16 @@ describe('Marking alerts as acknowledged', () => {
markAcknowledgedFirstAlert();
const expectedNumberOfAlerts = +numberOfAlerts - numberOfAlertsToBeMarkedAcknowledged;
cy.get(ALERTS_COUNT).should('have.text', `${expectedNumberOfAlerts} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should('have.text', `${expectedNumberOfAlerts}`);

goToAcknowledgedAlerts();
waitForAlerts();

cy.get(ALERTS_COUNT).should('have.text', `${numberOfAlertsToBeMarkedAcknowledged} alert`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${numberOfAlertsToBeMarkedAcknowledged}`
);
Comment on lines +57 to +66
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case for verifying the Trend histogram has refreshed as well? No explicit counts visible in that component, so we'd need a separate method for verification here -- perhaps setup test to only have one alert and then verify Rule Name is no longer in legend? Or could hover on a bar and verify counts in tooltip (though may be tough to sort out the flake)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment goes for opening/closing specs below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't verify the trend histogram yet, I was looking at some possible ways to do that since the data isn't as neat from a css selector standpoint for cypress. I think having the one alert would be a good way to do that, though, I'll add it to the test case here and in the other relevant files 👍

});
});
});
69 changes: 68 additions & 1 deletion x-pack/plugins/security_solution/cypress/integration/detection_alerts/closing.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
*/

import { getNewRule } from '../../objects/rule';
import { ALERTS_COUNT, SELECTED_ALERTS, TAKE_ACTION_POPOVER_BTN } from '../../screens/alerts';
import {
ALERTS_COUNT,
SELECTED_ALERTS,
TAKE_ACTION_POPOVER_BTN,
ALERT_COUNT_TABLE_FIRST_ROW_COUNT,
ALERTS_TREND_SIGNAL_RULE_NAME_PANEL,
} from '../../screens/alerts';

import {
closeFirstAlert,
Expand Down Expand Up @@ -46,6 +52,7 @@ describe('Closing alerts', () => {
.then((alertNumberString) => {
const numberOfAlerts = alertNumberString.split(' ')[0];
cy.get(ALERTS_COUNT).should('have.text', `${numberOfAlerts} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should('have.text', `${numberOfAlerts}`);

selectNumberOfAlerts(numberOfAlertsToBeClosed);

Expand All @@ -56,6 +63,10 @@ describe('Closing alerts', () => {

const expectedNumberOfAlertsAfterClosing = +numberOfAlerts - numberOfAlertsToBeClosed;
cy.get(ALERTS_COUNT).should('have.text', `${expectedNumberOfAlertsAfterClosing} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${expectedNumberOfAlertsAfterClosing}`
);

goToClosedAlerts();
waitForAlerts();
Expand All @@ -75,6 +86,10 @@ describe('Closing alerts', () => {
'have.text',
`${expectedNumberOfClosedAlertsAfterOpened} alerts`
);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${expectedNumberOfClosedAlertsAfterOpened}`
);

goToOpenedAlerts();
waitForAlerts();
Expand All @@ -83,6 +98,10 @@ describe('Closing alerts', () => {
+numberOfAlerts - expectedNumberOfClosedAlertsAfterOpened;

cy.get(ALERTS_COUNT).should('have.text', `${expectedNumberOfOpenedAlerts} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${expectedNumberOfOpenedAlerts}`
);
});
});

Expand All @@ -103,11 +122,59 @@ describe('Closing alerts', () => {

const expectedNumberOfAlerts = +numberOfAlerts - numberOfAlertsToBeClosed;
cy.get(ALERTS_COUNT).should('have.text', `${expectedNumberOfAlerts} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should('have.text', `${expectedNumberOfAlerts}`);

goToClosedAlerts();
waitForAlerts();

cy.get(ALERTS_COUNT).should('have.text', `${numberOfAlertsToBeClosed} alert`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${numberOfAlertsToBeClosed}`
);
});
});

it('Updates trend histogram whenever alert status is updated in table', () => {
const numberOfAlertsToBeClosed = 1;
cy.get(ALERTS_COUNT)
.invoke('text')
.then((alertNumberString) => {
const numberOfAlerts = alertNumberString.split(' ')[0];
cy.get(ALERTS_COUNT).should('have.text', `${numberOfAlerts} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should('have.text', `${numberOfAlerts}`);

selectNumberOfAlerts(numberOfAlertsToBeClosed);

cy.get(SELECTED_ALERTS).should('have.text', `Selected ${numberOfAlertsToBeClosed} alert`);

closeAlerts();
waitForAlerts();

const expectedNumberOfAlertsAfterClosing = +numberOfAlerts - numberOfAlertsToBeClosed;
cy.get(ALERTS_COUNT).should('have.text', `${expectedNumberOfAlertsAfterClosing} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${expectedNumberOfAlertsAfterClosing}`
);

goToClosedAlerts();
waitForAlerts();

cy.get(ALERTS_COUNT).should('have.text', `${numberOfAlertsToBeClosed} alert`);

const numberOfAlertsToBeOpened = 1;
selectNumberOfAlerts(numberOfAlertsToBeOpened);

cy.get(SELECTED_ALERTS).should('have.text', `Selected ${numberOfAlertsToBeOpened} alert`);
cy.get(ALERTS_TREND_SIGNAL_RULE_NAME_PANEL).should('exist');

openAlerts();
waitForAlerts();

cy.get(ALERTS_COUNT).should('not.exist');
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should('not.exist');
cy.get(ALERTS_TREND_SIGNAL_RULE_NAME_PANEL).should('not.exist');
});
});
});
15 changes: 14 additions & 1 deletion x-pack/plugins/security_solution/cypress/integration/detection_alerts/opening.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
*/

import { getNewRule } from '../../objects/rule';
import { ALERTS_COUNT, SELECTED_ALERTS, TAKE_ACTION_POPOVER_BTN } from '../../screens/alerts';
import {
ALERTS_COUNT,
SELECTED_ALERTS,
TAKE_ACTION_POPOVER_BTN,
ALERT_COUNT_TABLE_FIRST_ROW_COUNT,
} from '../../screens/alerts';

import {
closeAlerts,
Expand Down Expand Up @@ -74,6 +79,10 @@ describe('Opening alerts', () => {

const expectedNumberOfAlerts = +numberOfAlerts - numberOfAlertsToBeOpened;
cy.get(ALERTS_COUNT).should('have.text', `${expectedNumberOfAlerts} alerts`);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${expectedNumberOfAlerts}`
);

goToOpenedAlerts();
waitForAlerts();
Expand All @@ -82,6 +91,10 @@ describe('Opening alerts', () => {
'have.text',
`${numberOfOpenedAlerts + numberOfAlertsToBeOpened} alerts`.toString()
);
cy.get(ALERT_COUNT_TABLE_FIRST_ROW_COUNT).should(
'have.text',
`${numberOfOpenedAlerts + numberOfAlertsToBeOpened}`
);
});
});
});
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security_solution/cypress/screens/alerts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,9 @@ export const TAKE_ACTION_POPOVER_BTN = '[data-test-subj="selectedShowBulkActions
export const TIMELINE_CONTEXT_MENU_BTN = '[data-test-subj="timeline-context-menu-button"]';

export const ATTACH_ALERT_TO_CASE_BUTTON = '[data-test-subj="attach-alert-to-case-button"]';

export const ALERT_COUNT_TABLE_FIRST_ROW_COUNT =
'[data-test-subj="alertsCountTable"] tr:nth-child(1) td:nth-child(2) .euiTableCellContent__text';

export const ALERTS_TREND_SIGNAL_RULE_NAME_PANEL =
'[data-test-subj="render-content-signal.rule.name"]';
7 changes: 6 additions & 1 deletion ...olution/public/detections/components/alerts_table/timeline_actions/alert_context_menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { indexOf } from 'lodash';
import { connect, ConnectedProps } from 'react-redux';
import { ExceptionListType } from '@kbn/securitysolution-io-ts-list-types';
import { get } from 'lodash/fp';
import { useRouteSpy } from '../../../../common/utils/route/use_route_spy';
import { buildGetAlertByIdQuery } from '../../../../common/components/exceptions/helpers';
import { EventsTdContent } from '../../../../timelines/components/timeline/styles';
import { DEFAULT_ICON_BUTTON_WIDTH } from '../../../../timelines/components/timeline/helpers';
Expand Down Expand Up @@ -63,6 +64,7 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps & PropsFromRedux
timelineQuery,
}) => {
const [isPopoverOpen, setPopover] = useState(false);
const [routeProps] = useRouteSpy();

const afterItemSelection = useCallback(() => {
setPopover(false);
Expand Down Expand Up @@ -112,10 +114,13 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps & PropsFromRedux
const refetchAll = useCallback(() => {
if (timelineId === TimelineId.active) {
refetchQuery([timelineQuery]);
if (routeProps.pageName === 'alerts') {
refetchQuery(globalQuery);
}
Comment on lines +117 to +119
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I had explicitly tried closing an alert from within timeline elsewhere in the app to see if we'd still refetch, haha! 😅

🚀 🥮

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you could use https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/common/constants.ts#L72 instead of this string JUST IN CASE it ever changes

} else {
refetchQuery(globalQuery);
}
}, [timelineId, globalQuery, timelineQuery]);
}, [timelineId, globalQuery, timelineQuery, routeProps]);

const {
exceptionModalType,
Expand Down
43 changes: 4 additions & 39 deletions x-pack/plugins/security_solution/public/detections/components/take_action_dropdown/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import React, { useState, useCallback, useMemo } from 'react';
import { EuiContextMenuPanel, EuiButton, EuiPopover } from '@elastic/eui';
import type { ExceptionListType } from '@kbn/securitysolution-io-ts-list-types';
import { isEmpty } from 'lodash/fp';
import { connect, ConnectedProps } from 'react-redux';
import { TimelineEventsDetailsItem, TimelineId } from '../../../../common';
import { TimelineEventsDetailsItem } from '../../../../common';
import { TAKE_ACTION } from '../alerts_table/alerts_utility_bar/translations';
import { useExceptionActions } from '../alerts_table/timeline_actions/use_add_exception_actions';
import { useAlertsActions } from '../alerts_table/timeline_actions/use_alerts_actions';
Expand All @@ -24,8 +23,6 @@ import { Status } from '../../../../common/detection_engine/schemas/common/schem
import { isAlertFromEndpointAlert } from '../../../common/utils/endpoint_alert_check';
import { useIsExperimentalFeatureEnabled } from '../../../common/hooks/use_experimental_features';
import { useAddToCaseActions } from '../alerts_table/timeline_actions/use_add_to_case_actions';
import { inputsModel, inputsSelectors, State } from '../../../common/store';

interface ActionsData {
alertStatus: Status;
eventId: string;
Expand All @@ -48,7 +45,7 @@ export interface TakeActionDropdownProps {
timelineId: string;
}

export const TakeActionDropdownComponent = React.memo(
export const TakeActionDropdown = React.memo(
({
detailsData,
ecsData,
Expand All @@ -61,9 +58,7 @@ export const TakeActionDropdownComponent = React.memo(
onAddIsolationStatusClick,
refetch,
timelineId,
globalQuery,
timelineQuery,
}: TakeActionDropdownProps & PropsFromRedux) => {
}: TakeActionDropdownProps) => {
const tGridEnabled = useIsExperimentalFeatureEnabled('tGridEnabled');

const [isPopoverOpen, setIsPopoverOpen] = useState(false);
Expand Down Expand Up @@ -146,24 +141,12 @@ export const TakeActionDropdownComponent = React.memo(
closePopoverHandler();
}, [closePopoverHandler]);

const refetchQuery = (newQueries: inputsModel.GlobalQuery[]) => {
newQueries.forEach((q) => q.refetch && (q.refetch as inputsModel.Refetch)());
};

const refetchAll = useCallback(() => {
if (timelineId === TimelineId.active) {
refetchQuery([timelineQuery]);
} else {
refetchQuery(globalQuery);
}
}, [timelineId, globalQuery, timelineQuery]);

const { actionItems: statusActionItems } = useAlertsActions({
alertStatus: actionsData.alertStatus,
closePopover: closePopoverAndFlyout,
eventId: actionsData.eventId,
indexName,
refetch: refetchAll,
refetch,
timelineId,
});

Expand Down Expand Up @@ -233,21 +216,3 @@ export const TakeActionDropdownComponent = React.memo(
) : null;
}
);

const makeMapStateToProps = () => {
const getGlobalQueries = inputsSelectors.globalQuery();
const getTimelineQuery = inputsSelectors.timelineQueryByIdSelector();
const mapStateToProps = (state: State, { timelineId }: TakeActionDropdownProps) => {
return {
globalQuery: getGlobalQueries(state),
timelineQuery: getTimelineQuery(state, timelineId),
};
};
return mapStateToProps;
};

const connector = connect(makeMapStateToProps);

type PropsFromRedux = ConnectedProps<typeof connector>;

export const TakeActionDropdown = connector(React.memo(TakeActionDropdownComponent));
Loading