Skip to content

Commit

Permalink
[Security Solution][Investigations] - fix opening flyout, and clear f…
Browse files Browse the repository at this point in the history
…ilters (#156702)

## Summary

This PR fixes the following issue
#156590 where a users
configuration for their alert page filters can prevent them from seeing
the alert in the table. In this PR we clear the filters to only show the
required status filter, with no selection, to rely primarily on the kql
query filter for the alert id.

Demo: 


https://user-images.githubusercontent.com/17211684/236332095-ac5583ec-6b5f-4ee2-9c23-9391b54263ba.mov


It also fixes a bug where the flyout when opened wasn't unmounted when
navigating away from a page and back to it as seen here:

The bug:


https://user-images.githubusercontent.com/17211684/236333550-b41925f9-a06c-4aa9-931b-beecf4c9ae9b.mov

With the fix:


https://user-images.githubusercontent.com/17211684/236334123-a0e3169e-2ec8-4754-92d2-65f2f369fa4f.mov


### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
michaelolo24 and kibanamachine authored May 8, 2023
1 parent 6ad8c42 commit 1b40216
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import type { DataTableModel } from '@kbn/securitysolution-data-table';
import {
ALERT_FLYOUT,
CELL_TEXT,
Expand All @@ -24,6 +25,8 @@ import { login, visit, visitWithoutDateRange } from '../../tasks/login';
import { getUnmappedRule } from '../../objects/rule';
import { ALERTS_URL } from '../../urls/navigation';
import { tablePageSelector } from '../../screens/table_pagination';
import { getLocalstorageEntryAsObject } from '../../helpers/common';
import { goToRuleDetails } from '../../tasks/alerts_detection_rules';

describe('Alert details flyout', () => {
describe('With unmapped fields', { testIsolation: false }, () => {
Expand Down Expand Up @@ -124,11 +127,81 @@ describe('Alert details flyout', () => {
});

it('should have the `kibana.alert.url` field set', () => {
const alertUrl =
'http://localhost:5601/app/security/alerts/redirect/eabbdefc23da981f2b74ab58b82622a97bb9878caa11bc914e2adfacc94780f1?index=.alerts-security.alerts-default&timestamp=2023-04-27T11:03:57.906Z';
openTable();
filterBy('kibana.alert.url');
cy.get('[data-test-subj="formatted-field-kibana.alert.url"]').should('have.text', alertUrl);
cy.get('[data-test-subj="formatted-field-kibana.alert.url"]').should(
'have.text',
'http://localhost:5601/app/security/alerts/redirect/eabbdefc23da981f2b74ab58b82622a97bb9878caa11bc914e2adfacc94780f1?index=.alerts-security.alerts-default&timestamp=2023-04-27T11:03:57.906Z'
);
});
});

describe('Localstorage management', { testIsolation: false }, () => {
before(() => {
cleanKibana();
esArchiverLoad('query_alert');
login();
visit(ALERTS_URL);
waitForAlertsToPopulate();
});

beforeEach(() => {
expandFirstAlert();
});

const alertTableKey = 'alerts-page';
const getFlyoutConfig = (dataTable: { [alertTableKey]: DataTableModel }) =>
dataTable?.[alertTableKey]?.expandedDetail?.query;

/**
* Localstorage is updated after a delay here x-pack/plugins/security_solution/public/common/store/data_table/epic_local_storage.ts
* We create this config to re-check localStorage 3 times, every 500ms to avoid any potential flakyness from that delay
*/
const storageCheckRetryConfig = {
timeout: 1500,
interval: 500,
};

it('should store the flyout state in localstorage', () => {
cy.get(OVERVIEW_RULE).should('be.visible');
const localStorageCheck = () =>
cy.getAllLocalStorage().then((storage) => {
const securityDataTable = getLocalstorageEntryAsObject(storage, 'securityDataTable');
return getFlyoutConfig(securityDataTable)?.panelView === 'eventDetail';
});

cy.waitUntil(localStorageCheck, storageCheckRetryConfig);
});

it('should remove the flyout details from local storage when closed', () => {
cy.get(OVERVIEW_RULE).should('be.visible');
closeAlertFlyout();
const localStorageCheck = () =>
cy.getAllLocalStorage().then((storage) => {
const securityDataTable = getLocalstorageEntryAsObject(storage, 'securityDataTable');
return getFlyoutConfig(securityDataTable)?.panelView === undefined;
});

cy.waitUntil(localStorageCheck, storageCheckRetryConfig);
});

it('should remove the flyout state from localstorage when navigating away without closing the flyout', () => {
cy.get(OVERVIEW_RULE).should('be.visible');
goToRuleDetails();
const localStorageCheck = () =>
cy.getAllLocalStorage().then((storage) => {
const securityDataTable = getLocalstorageEntryAsObject(storage, 'securityDataTable');
return getFlyoutConfig(securityDataTable)?.panelView === undefined;
});

cy.waitUntil(localStorageCheck, storageCheckRetryConfig);
});

it('should not reopen the flyout when navigating away from the alerts page and returning to it', () => {
cy.get(OVERVIEW_RULE).should('be.visible');
goToRuleDetails();
visit(ALERTS_URL);
cy.get(OVERVIEW_RULE).should('not.exist');
});
});
});
14 changes: 14 additions & 0 deletions x-pack/plugins/security_solution/cypress/helpers/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,17 @@ export const getDataTestSubjectSelectorStartWith = (dataTestSubjectValue: string
* @param className the value passed to class property of the DOM element
*/
export const getClassSelector = (className: string) => `.${className}`;

export const getLocalstorageEntryAsObject = (storage: Cypress.StorageByOrigin, field: string) => {
// baseUrl value from x-pack/plugins/security_solution/cypress/cypress.config.ts
const envLocalstorage = storage?.['http://localhost:5620'];
let result;
if (envLocalstorage && envLocalstorage[field]) {
try {
result = JSON.parse(envLocalstorage[field] as string);
} catch {
result = undefined;
}
}
return result;
};
3 changes: 2 additions & 1 deletion x-pack/plugins/security_solution/cypress/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"@kbn/rison",
"@kbn/datemath",
"@kbn/guided-onboarding-plugin",
"@kbn/alerting-plugin"
"@kbn/alerting-plugin",
"@kbn/securitysolution-data-table"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('AlertDetailsRedirect', () => {
expect(historyMock.replace).toHaveBeenCalledWith({
hash: '',
pathname: ALERTS_PATH,
search: `?query=(language:kuery,query:'_id: ${testAlertId}')&timerange=(global:(linkTo:!(timeline,socTrends),timerange:(from:'${testTimestamp}',kind:absolute,to:'2023-04-20T12:05:00.000Z')),timeline:(linkTo:!(global,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',fromStr:now/d,kind:relative,to:'2020-07-08T08:20:18.966Z',toStr:now/d)))&eventFlyout=(panelView:eventDetail,params:(eventId:${testAlertId},indexName:${testIndex}))`,
search: `?query=(language:kuery,query:'_id: ${testAlertId}')&timerange=(global:(linkTo:!(timeline,socTrends),timerange:(from:'${testTimestamp}',kind:absolute,to:'2023-04-20T12:05:00.000Z')),timeline:(linkTo:!(global,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',fromStr:now/d,kind:relative,to:'2020-07-08T08:20:18.966Z',toStr:now/d)))&pageFilters=!((exclude:!f,existsSelected:!f,fieldName:kibana.alert.workflow_status,selectedOptions:!(),title:Status))&eventFlyout=(panelView:eventDetail,params:(eventId:${testAlertId},indexName:${testIndex}))`,
state: undefined,
});
});
Expand Down Expand Up @@ -96,7 +96,7 @@ describe('AlertDetailsRedirect', () => {
expect(historyMock.replace).toHaveBeenCalledWith({
hash: '',
pathname: ALERTS_PATH,
search: `?query=(language:kuery,query:'_id: ${testAlertId}')&timerange=(global:(linkTo:!(timeline,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',kind:absolute,to:'2020-07-08T08:25:18.966Z')),timeline:(linkTo:!(global,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',fromStr:now/d,kind:relative,to:'2020-07-08T08:20:18.966Z',toStr:now/d)))&eventFlyout=(panelView:eventDetail,params:(eventId:${testAlertId},indexName:${testIndex}))`,
search: `?query=(language:kuery,query:'_id: ${testAlertId}')&timerange=(global:(linkTo:!(timeline,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',kind:absolute,to:'2020-07-08T08:25:18.966Z')),timeline:(linkTo:!(global,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',fromStr:now/d,kind:relative,to:'2020-07-08T08:20:18.966Z',toStr:now/d)))&pageFilters=!((exclude:!f,existsSelected:!f,fieldName:kibana.alert.workflow_status,selectedOptions:!(),title:Status))&eventFlyout=(panelView:eventDetail,params:(eventId:${testAlertId},indexName:${testIndex}))`,
state: undefined,
});
});
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('AlertDetailsRedirect', () => {
expect(historyMock.replace).toHaveBeenCalledWith({
hash: '',
pathname: ALERTS_PATH,
search: `?query=(language:kuery,query:'_id: ${testAlertId}')&timerange=(global:(linkTo:!(timeline,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',kind:absolute,to:'2020-07-08T08:25:18.966Z')),timeline:(linkTo:!(global,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',fromStr:now/d,kind:relative,to:'2020-07-08T08:20:18.966Z',toStr:now/d)))&eventFlyout=(panelView:eventDetail,params:(eventId:${testAlertId},indexName:.internal${DEFAULT_ALERTS_INDEX}-default))`,
search: `?query=(language:kuery,query:'_id: ${testAlertId}')&timerange=(global:(linkTo:!(timeline,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',kind:absolute,to:'2020-07-08T08:25:18.966Z')),timeline:(linkTo:!(global,socTrends),timerange:(from:'2020-07-07T08:20:18.966Z',fromStr:now/d,kind:relative,to:'2020-07-08T08:20:18.966Z',toStr:now/d)))&pageFilters=!((exclude:!f,existsSelected:!f,fieldName:kibana.alert.workflow_status,selectedOptions:!(),title:Status))&eventFlyout=(panelView:eventDetail,params:(eventId:${testAlertId},indexName:.internal${DEFAULT_ALERTS_INDEX}-default))`,
state: undefined,
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ import { Redirect, useLocation, useParams } from 'react-router-dom';

import moment from 'moment';
import { encode } from '@kbn/rison';
import { ALERT_WORKFLOW_STATUS } from '@kbn/rule-data-utils';
import type { FilterItemObj } from '../../../common/components/filter_group/types';
import { ALERTS_PATH, DEFAULT_ALERTS_INDEX } from '../../../../common/constants';
import { URL_PARAM_KEY } from '../../../common/hooks/use_url_state';
import { inputsSelectors } from '../../../common/store';
import { formatPageFilterSearchParam } from '../../../../common/utils/format_page_filter_search_param';

export const AlertDetailsRedirect = () => {
const { alertId } = useParams<{ alertId: string }>();
Expand Down Expand Up @@ -61,7 +64,16 @@ export const AlertDetailsRedirect = () => {

const kqlAppQuery = encode({ language: 'kuery', query: `_id: ${alertId}` });

const url = `${ALERTS_PATH}?${URL_PARAM_KEY.appQuery}=${kqlAppQuery}&${URL_PARAM_KEY.timerange}=${timerange}&${URL_PARAM_KEY.eventFlyout}=${flyoutString}`;
const statusPageFilter: FilterItemObj = {
fieldName: ALERT_WORKFLOW_STATUS,
title: 'Status',
selectedOptions: [],
existsSelected: false,
};

const pageFiltersQuery = encode(formatPageFilterSearchParam([statusPageFilter]));

const url = `${ALERTS_PATH}?${URL_PARAM_KEY.appQuery}=${kqlAppQuery}&${URL_PARAM_KEY.timerange}=${timerange}&${URL_PARAM_KEY.pageFilter}=${pageFiltersQuery}&${URL_PARAM_KEY.eventFlyout}=${flyoutString}`;

return <Redirect to={url} />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
* 2.0.
*/

import React, { useCallback, useMemo } from 'react';
import React, { useCallback, useEffect, useMemo } from 'react';
import { useDispatch } from 'react-redux';
import type { EuiFlyoutProps } from '@elastic/eui';
import { EuiFlyout } from '@elastic/eui';

import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { EntityType } from '@kbn/timelines-plugin/common';
import { dataTableSelectors } from '@kbn/securitysolution-data-table';
import { dataTableActions, dataTableSelectors } from '@kbn/securitysolution-data-table';
import { getScopedActions, isInTableScope, isTimelineScope } from '../../../helpers';
import { timelineSelectors } from '../../store/timeline';
import { timelineDefaults } from '../../store/timeline/defaults';
Expand Down Expand Up @@ -66,6 +66,21 @@ export const DetailsPanel = React.memo(
(state) => ((getScope && getScope(state, scopeId)) ?? timelineDefaults)?.expandedDetail
);

useEffect(() => {
/**
* Removes the flyout from redux when it is unmounted as it's also stored in localStorage
* This only works when navigating within the app, if navigating via the url bar,
* the localStorage state will be maintained
* */
return () => {
dispatch(
dataTableActions.toggleDetailPanel({
id: scopeId,
})
);
};
}, [dispatch, scopeId]);

// To be used primarily in the flyout scenario where we don't want to maintain the tabType
const defaultOnPanelClose = useCallback(() => {
const scopedActions = getScopedActions(scopeId);
Expand Down

0 comments on commit 1b40216

Please sign in to comment.