Skip to content

Commit

Permalink
[Defend Workflows][Bug] Case flyout z-index (#153219)
Browse files Browse the repository at this point in the history
Fixes elastic/security-team#6228

5000 `z-index` set in `create-case-flyout-mask-overlay` is being
overwritten by `euiOverlayMask-belowHeader` with a value of 1000. This
causes **Case flyout** to be under the already opened **Osquery flyout**

This PR includes cleanup in flyouts renders - we removed unnecessary
`maskProps` as well as z-indexes that were explicitly set even though
flyout component manages them itself.

![test](https://user-images.githubusercontent.com/29123534/225292177-a08d3fb8-aad3-487b-a054-6cde6aec94d7.gif)

---------

Co-authored-by: Tomasz Ciecierski <[email protected]>
Co-authored-by: Tomasz Ciecierski <[email protected]>
Co-authored-by: Patryk Kopyciński <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit d8fe39c)
  • Loading branch information
szwarckonrad committed May 3, 2023
1 parent cdd9cb4 commit 445842f
Show file tree
Hide file tree
Showing 28 changed files with 245 additions and 239 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ export const SolutionSideNavPanel: React.FC<SolutionSideNavPanelProps> = React.m

return (
<>
{/* <GlobalPanelStyle /> */}
<EuiWindowEvent event="keydown" handler={onKeyDown} />
<EuiPortal>
<EuiFocusTrap autoFocus>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React from 'react';
import styled, { createGlobalStyle } from 'styled-components';
import styled from 'styled-components';
import { EuiFlyout, EuiFlyoutHeader, EuiTitle, EuiFlyoutBody } from '@elastic/eui';

import { QueryClientProvider } from '@tanstack/react-query';
Expand Down Expand Up @@ -38,22 +38,6 @@ const StyledFlyout = styled(EuiFlyout)`
`}
`;

const maskOverlayClassName = 'create-case-flyout-mask-overlay';

/**
* We need to target the mask overlay which is a parent element
* of the flyout.
* A global style is needed to target a parent element.
*/

const GlobalStyle = createGlobalStyle<{ theme: { eui: { euiZLevel5: number } } }>`
.${maskOverlayClassName} {
${({ theme }) => `
z-index: ${theme.eui.euiZLevel5};
`}
}
`;

// Adding bottom padding because timeline's
// bottom bar gonna hide the submit button.
const StyledEuiFlyoutBody = styled(EuiFlyoutBody)`
Expand Down Expand Up @@ -83,13 +67,10 @@ export const CreateCaseFlyout = React.memo<CreateCaseFlyoutProps>(
return (
<QueryClientProvider client={casesQueryClient}>
<ReactQueryDevtools initialIsOpen={false} />
<GlobalStyle />
<StyledFlyout
onClose={onClose}
tour-step="create-case-flyout"
data-test-subj="create-case-flyout"
// maskProps is needed in order to apply the z-index to the parent overlay element, not to the flyout only
maskProps={{ className: maskOverlayClassName }}
>
<EuiFlyoutHeader data-test-subj="create-case-flyout-header" hasBorder>
<EuiTitle size="m">
Expand Down
62 changes: 60 additions & 2 deletions x-pack/plugins/osquery/cypress/e2e/all/alerts.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ import {
viewRecentCaseAndCheckResults,
} from '../../tasks/live_query';
import { preparePack } from '../../tasks/packs';
import { closeModalIfVisible, closeToastIfVisible } from '../../tasks/integrations';
import {
closeModalIfVisible,
closeToastIfVisible,
generateRandomStringName,
interceptCaseId,
} from '../../tasks/integrations';
import { navigateTo } from '../../tasks/navigation';
import { RESULTS_TABLE, RESULTS_TABLE_BUTTON } from '../../screens/live_query';
import { OSQUERY_POLICY } from '../../screens/fleet';
Expand Down Expand Up @@ -359,6 +364,59 @@ describe('Alert Event Details', () => {
});
});

describe('Case creation', () => {
let ruleId: string;
let ruleName: string;
let packId: string;
let packName: string;
let caseId: string;
const packData = packFixture();

before(() => {
loadPack(packData).then((data) => {
packId = data.id;
packName = data.attributes.name;
});
loadRule(true).then((data) => {
ruleId = data.id;
ruleName = data.name;
});
interceptCaseId((id) => {
caseId = id;
});
});

after(() => {
cleanupPack(packId);
cleanupRule(ruleId);
cleanupCase(caseId);
});

it('runs osquery against alert and creates a new case', () => {
const [caseName, caseDescription] = generateRandomStringName(2);
loadRuleAlerts(ruleName);
cy.getBySel('expand-event').first().click({ force: true });
cy.getBySel('take-action-dropdown-btn').click();
cy.getBySel('osquery-action-item').click();
cy.contains('Run a set of queries in a pack').wait(500).click();
cy.getBySel('select-live-pack').within(() => {
cy.getBySel('comboBoxInput').type(`${packName}{downArrow}{enter}`);
});
submitQuery();
cy.get('[aria-label="Add to Case"]').first().click();
cy.getBySel('cases-table-add-case-filter-bar').click();
cy.getBySel('create-case-flyout').should('be.visible');
cy.getBySel('caseTitle').within(() => {
cy.getBySel('input').type(caseName);
});
cy.getBySel('caseDescription').within(() => {
cy.getBySel('euiMarkdownEditorTextArea').type(caseDescription);
});
cy.getBySel('create-case-submit').click();
cy.contains(`An alert was added to "${caseName}"`);
});
});

describe('Case', () => {
let ruleId: string;
let ruleName: string;
Expand Down Expand Up @@ -541,7 +599,7 @@ describe('Alert Event Details', () => {
});
});
cy.contains(timelineRegex);
cy.contains('Untitled timeline').click();
cy.getBySel('flyoutBottomBar').contains('Untitled timeline').click();
cy.contains(filterRegex);
});
});
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/osquery/cypress/tasks/integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ export const interceptAgentPolicyId = (cb: (policyId: string) => void) => {
});
};

export const interceptCaseId = (cb: (caseId: string) => void) => {
cy.intercept('POST', '**/api/cases', (req) => {
req.continue((res) => {
cb(res.body.id);

return res.send(res.body);
});
});
};

export const interceptPackId = (cb: (packId: string) => void) => {
cy.intercept('POST', '**/api/osquery/packs', (req) => {
req.continue((res) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export const GLOBAL_SEARCH_BAR_FILTER_ITEM_DELETE = '#popoverFor_filter0 button[

export const GLOBAL_SEARCH_BAR_PINNED_FILTER = '.globalFilterItem-isPinned';

export const GLOBAL_KQL_WRAPPER = '[data-test-subj="filters-global-container"]';

export const GLOBAL_KQL_INPUT =
'[data-test-subj="filters-global-container"] [data-test-subj="unifiedQueryInput"] textarea';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export const TIMELINE_FILTER_OPERATOR = '[data-test-subj="filterOperatorList"]';
export const TIMELINE_FILTER_VALUE =
'[data-test-subj="filterParamsComboBox phraseParamsComboxBox"]';

export const TIMELINE_FLYOUT = '[data-test-subj="eui-flyout"]';
export const TIMELINE_FLYOUT = '[data-test-subj="timeline-flyout"]';

export const TIMELINE_FLYOUT_HEADER = '[data-test-subj="query-tab-flyout-header"]';

Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/cypress/tasks/search_bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export const fillKqlQueryBar = (query: string) => {
export const clearKqlQueryBar = () => {
cy.get(GLOBAL_KQL_INPUT).should('be.visible');
cy.get(GLOBAL_KQL_INPUT).clear();
// clicks outside of the input to close the autocomplete
cy.get('body').click(0, 0);
};

export const removeKqlFilter = () => {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/cypress/tasks/timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ export const createNewTimelineTemplate = () => {
};

export const executeTimelineKQL = (query: string) => {
cy.get(`${SEARCH_OR_FILTER_CONTAINER} textarea`).type(`${query} {enter}`);
cy.get(`${SEARCH_OR_FILTER_CONTAINER} textarea`).clear().type(`${query} {enter}`);
};

export const executeTimelineSearch = (query: string) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,62 +35,22 @@ export const FULL_SCREEN_CONTENT_OVERRIDES_CSS_STYLESHEET = () => css`
}
`;

/** The `z-index` for EuiPopover Panels that are displayed from inside of Timeline page */
export const TIMELINE_EUI_POPOVER_PANEL_ZINDEX = 9900;

/**
* Stylesheet with Eui class overrides in order to address display issues caused when
* the Timeline overlay is opened. These are normally adjustments to ensure that the
* z-index of other EUI components continues to work with the z-index used by timeline
* overlay.
*/
export const TIMELINE_OVERRIDES_CSS_STYLESHEET = () => css`
.euiPopover__panel[data-popover-open] {
z-index: ${TIMELINE_EUI_POPOVER_PANEL_ZINDEX} !important;
min-width: 24px;
}
.euiPopover__panel[data-popover-open].sourcererPopoverPanel {
// needs to appear under modal
z-index: 5900 !important;
}
.euiToolTip {
z-index: 9950 !important;
}
/*
overrides the default styling of euiComboBoxOptionsList because it's implemented
as a popover, so it's not selectable as a child of the styled component
*/
.euiComboBoxOptionsList {
z-index: 9999;
}
/* ensure elastic charts tooltips appear above open euiPopovers */
.echTooltip {
z-index: 9950;
}
`;

/*
SIDE EFFECT: the following `createGlobalStyle` overrides default styling in angular code that was not theme-friendly
and `EuiPopover`, `EuiToolTip` global styles
*/
export const AppGlobalStyle = createGlobalStyle<{
theme: { eui: { euiColorPrimary: string; euiColorLightShade: string; euiSizeS: string } };
}>`
${TIMELINE_OVERRIDES_CSS_STYLESHEET}
/*
overrides the default styling of EuiDataGrid expand popover footer to
make it a column of actions instead of the default actions row
*/
.euiDataGridRowCell__popover {
max-width: 815px !important;
max-height: none !important;
overflow: hidden;
.expandable-top-value-button {
&.euiButtonEmpty--primary:enabled:focus,
.euiButtonEmpty--primary:focus {
Expand All @@ -111,8 +71,8 @@ export const AppGlobalStyle = createGlobalStyle<{
}
}
.euiText + .euiPopoverFooter {
border-top: 1px solid ${({ theme }) => theme.eui.euiColorLightShade};
.euiText + .euiPopoverFooter {
border-top: 1px solid ${({ theme }) => theme.eui.euiColorLightShade};
margin-top: ${({ theme }) => theme.eui.euiSizeS};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ const MyEuiModal = styled(EuiModal)`
height: auto !important;
max-width: 718px;
}
z-index: 99999999;
`;

export const UpdateDefaultDataViewModal = React.memo<Props>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ const TopNContainer = styled.div`
`;

const CloseButton = styled(EuiButtonIcon)`
z-index: 999999;
position: absolute;
right: 4px;
top: 4px;
`;

const ViewSelect = styled(EuiSuperSelect)`
z-index: 999999;
width: 170px;
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export interface AddExceptionFlyoutProps {
sharedListToAddTo?: ExceptionListSchema[];
onCancel: (didRuleChange: boolean) => void;
onConfirm: (didRuleChange: boolean, didCloseAlert: boolean, didBulkCloseAlert: boolean) => void;
isNonTimeline?: boolean;
}

const FlyoutBodySection = styled(EuiFlyoutBody)`
Expand Down Expand Up @@ -114,7 +113,6 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
sharedListToAddTo,
onCancel,
onConfirm,
isNonTimeline = false,
}: AddExceptionFlyoutProps) {
const { isLoading, indexPatterns } = useFetchIndexPatterns(rules);
const [isSubmitting, submitNewExceptionItems] = useAddNewExceptionItems();
Expand Down Expand Up @@ -462,13 +460,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
}, [listType]);

return (
<EuiFlyout
ownFocus
maskProps={{ style: isNonTimeline === false ? 'z-index: 5000' : 'z-index: 1000' }} // For an edge case to display above the timeline flyout
size="l"
onClose={handleCloseFlyout}
data-test-subj="addExceptionFlyout"
>
<EuiFlyout size="l" onClose={handleCloseFlyout} data-test-subj="addExceptionFlyout">
<FlyoutHeader>
<EuiTitle>
<h2 data-test-subj="exceptionFlyoutTitle">{addExceptionMessage}</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,6 @@ const ExceptionsViewerComponent = ({
onConfirm={handleConfirmExceptionFlyout}
data-test-subj="addExceptionItemFlyout"
showAlertCloseOptions
isNonTimeline={true}
/>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import type {
ExceptionsBuilderReturnExceptionItem,
} from '@kbn/securitysolution-list-utils';
import type { DataViewBase } from '@kbn/es-query';
import styled, { css, createGlobalStyle } from 'styled-components';
import styled, { css } from 'styled-components';
import { ENDPOINT_LIST_ID } from '@kbn/securitysolution-list-constants';
import { hasEqlSequenceQuery, isEqlRule } from '../../../../../../common/detection_engine/utils';
import type { Rule } from '../../../../rule_management/logic/types';
Expand Down Expand Up @@ -56,15 +56,6 @@ const SectionHeader = styled(EuiTitle)`
font-weight: ${({ theme }) => theme.eui.euiFontWeightSemiBold};
`}
`;
// EuiCombox doesn't support change of z-index, or providing any class to portal
// This fix ovveride z-index for EuiFlyout, which conflict with EuiComboBox on this flyout
// fix x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx#L429
// TODO: should be fixed on Component level
const EuiComboboxZIndexGlobalStyle = createGlobalStyle`
[data-test-subj="comboBoxOptionsList osSelectionDropdown-optionsList"] {
z-index: 6000 !important;
}
`;

interface ExceptionsFlyoutConditionsComponentProps {
/* Exception list item field value for "name" */
Expand Down Expand Up @@ -242,7 +233,6 @@ const ExceptionsConditionsComponent: React.FC<ExceptionsFlyoutConditionsComponen
data-test-subj="osSelectionDropdown"
/>
</EuiFormRow>
<EuiComboboxZIndexGlobalStyle />
<EuiSpacer size="l" />
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,7 @@ const OsqueryFlyoutComponent: React.FC<OsqueryFlyoutProps> = ({

if (osquery?.OsqueryAction) {
return (
<EuiFlyout
ownFocus
maskProps={{ style: 'z-index: 5000' }} // For an edge case to display above the timeline flyout
size="m"
onClose={onClose}
>
<EuiFlyout size="m" onClose={onClose}>
<EuiFlyoutHeader hasBorder data-test-subj="flyout-header-osquery">
<EuiTitle>
<h2>{ACTION_OSQUERY}</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ export const ExceptionsListCard = memo<ExceptionsListCardProps>(
onConfirm={handleConfirmExceptionFlyout}
data-test-subj="addExceptionItemFlyoutInSharedLists"
showAlertCloseOptions={false}
isNonTimeline={true}
/>
) : null}
{showEditExceptionFlyout && exceptionToEdit ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const ListWithSearchComponent: FC<ListWithSearchComponentProps> = ({
onConfirm={handleConfirmExceptionFlyout}
data-test-subj="addExceptionItemFlyoutInList"
showAlertCloseOptions={false} // TODO ask if we need it
isNonTimeline={true}
// ask if we need the add to rule/list section and which list should we link the exception here
/>
) : viewerStatus === ViewerStatus.EMPTY || viewerStatus === ViewerStatus.LOADING ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,6 @@ export const SharedLists = React.memo(() => {
setDisplayAddExceptionItemFlyout(false);
if (didRuleChange) handleRefresh();
}}
isNonTimeline={true}
/>
)}

Expand Down
Loading

0 comments on commit 445842f

Please sign in to comment.