Skip to content

Commit

Permalink
[Security Solution][Detections] Update read-only callouts from warnin…
Browse files Browse the repository at this point in the history
…g to info so they persist when dismissed (elastic#84904)

**Addresses:** elastic#76587

## Summary

In this PR I'm doing basically 2 things:

1. Making readonly callouts we have in Detections `primary` instead of `warning` and thus persistable in local storage (if a callout is dismissed, we remember it there)
2. Creating a reusable implementation for that.

TODO:

- [x] Adjust all callouts used in Detections to be of type `primary`
- [x] Implement the local storage logic (dumb)
- [x] Implement the local storage logic (reusable)
- [x] Add a new user role: "Reader" (read-only user)
- [x] Add Cypress tests

Out of scope:

- Add unit tests. I'll probably address this in a follow-up PR. Do you think it's worth it or better to wait until the rework (see the next point)?
- Rework callouts to standardise them across Detections, Cases and other parts of the Security app? See my comment below in this PR.

## Screen recordings

Quick demo of how this implementation works:

- [primary callouts](https://drive.google.com/file/d/1SYQd_ihKPvzlVUxELI8qNEqLBOkL18Gd/view?usp=sharing)
- [warning, danger](https://drive.google.com/file/d/1lrAFPyXNjOYSiEsUXxY_fjXsvmyDcdWY/view?usp=sharing) (callout types here were manually adjusted)

## Additional notes

Cypress tests are based on the work done in elastic#81866.

![](https://puu.sh/GXwOd/1c855cb03f.png)
  • Loading branch information
banderror committed Dec 15, 2020
1 parent 477b859 commit 68a99ca
Show file tree
Hide file tree
Showing 40 changed files with 644 additions and 142 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/security_solution/common/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

// For the source of these roles please consult the PR these were introduced https://github.com/elastic/kibana/pull/81866#issue-511165754
export enum ROLES {
reader = 'reader',
t1_analyst = 't1_analyst',
t2_analyst = 't2_analyst',
hunter = 'hunter',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { ROLES } from '../../common/test';
import { DETECTIONS_RULE_MANAGEMENT_URL, DETECTIONS_URL } from '../urls/navigation';
import { newRule } from '../objects/rule';
import { PAGE_TITLE } from '../screens/common/page';

import {
login,
loginAndWaitForPageWithoutDateRange,
waitForPageWithoutDateRange,
deleteRoleAndUser,
} from '../tasks/login';
import { waitForAlertsIndexToBeCreated } from '../tasks/alerts';
import { goToRuleDetails } from '../tasks/alerts_detection_rules';
import { createCustomRule, deleteCustomRule, removeSignalsIndex } from '../tasks/api_calls/rules';
import { getCallOut, waitForCallOutToBeShown, dismissCallOut } from '../tasks/common/callouts';

const loadPageAsReadOnlyUser = (url: string) => {
waitForPageWithoutDateRange(url, ROLES.reader);
waitForPageTitleToBeShown();
};

const reloadPage = () => {
cy.reload();
waitForPageTitleToBeShown();
};

const waitForPageTitleToBeShown = () => {
cy.get(PAGE_TITLE).should('be.visible');
};

describe('Detections > Callouts indicating read-only access to resources', () => {
const ALERTS_CALLOUT = 'read-only-access-to-alerts';
const RULES_CALLOUT = 'read-only-access-to-rules';

before(() => {
// First, we have to open the app on behalf of a priviledged user in order to initialize it.
// Otherwise the app will be disabled and show a "welcome"-like page.
loginAndWaitForPageWithoutDateRange(DETECTIONS_URL, ROLES.platform_engineer);
waitForAlertsIndexToBeCreated();

// After that we can login as a read-only user.
login(ROLES.reader);
});

after(() => {
deleteRoleAndUser(ROLES.reader);
deleteRoleAndUser(ROLES.platform_engineer);
removeSignalsIndex();
});

context('On Detections home page', () => {
beforeEach(() => {
loadPageAsReadOnlyUser(DETECTIONS_URL);
});

it('We show one primary callout', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
});

context('When a user clicks Dismiss on the callout', () => {
it('We hide it and persist the dismissal', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
dismissCallOut(ALERTS_CALLOUT);
reloadPage();
getCallOut(ALERTS_CALLOUT).should('not.exist');
});
});
});

context('On Rules Management page', () => {
beforeEach(() => {
loadPageAsReadOnlyUser(DETECTIONS_RULE_MANAGEMENT_URL);
});

it('We show one primary callout', () => {
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
});

context('When a user clicks Dismiss on the callout', () => {
it('We hide it and persist the dismissal', () => {
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
dismissCallOut(RULES_CALLOUT);
reloadPage();
getCallOut(RULES_CALLOUT).should('not.exist');
});
});
});

context('On Rule Details page', () => {
beforeEach(() => {
createCustomRule(newRule);

loadPageAsReadOnlyUser(DETECTIONS_RULE_MANAGEMENT_URL);
waitForPageTitleToBeShown();
goToRuleDetails();
});

afterEach(() => {
deleteCustomRule();
});

it('We show two primary callouts', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
});

context('When a user clicks Dismiss on the callouts', () => {
it('We hide them and persist the dismissal', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');

dismissCallOut(ALERTS_CALLOUT);
reloadPage();

getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('be.visible');

dismissCallOut(RULES_CALLOUT);
reloadPage();

getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('not.exist');
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export const CALLOUT = '[data-test-subj^="callout-"]';

export const callOutWithId = (id: string) => `[data-test-subj="callout-${id}"]`;

export const CALLOUT_DISMISS_BTN = '[data-test-subj^="callout-dismiss-"]';
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export const PAGE_TITLE = '[data-test-subj="header-page-title"]';
24 changes: 24 additions & 0 deletions x-pack/plugins/security_solution/cypress/tasks/common/callouts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { callOutWithId, CALLOUT_DISMISS_BTN } from '../../screens/common/callouts';

export const getCallOut = (id: string, options?: Cypress.Timeoutable) => {
return cy.get(callOutWithId(id), options);
};

export const waitForCallOutToBeShown = (id: string, color: string) => {
getCallOut(id, { timeout: 10000 })
.should('be.visible')
.should('have.class', `euiCallOut--${color}`);
};

export const dismissCallOut = (id: string) => {
getCallOut(id, { timeout: 10000 }).within(() => {
cy.get(CALLOUT_DISMISS_BTN).should('be.visible').click();
cy.root().should('not.exist');
});
};
5 changes: 5 additions & 0 deletions x-pack/plugins/security_solution/cypress/tasks/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,8 @@ export const loginAndWaitForTimeline = (timelineId: string, role?: RolesType) =>
cy.get('[data-test-subj="headerGlobalNav"]');
cy.get(TIMELINE_FLYOUT_BODY).should('be.visible');
};

export const waitForPageWithoutDateRange = (url: string, role?: RolesType) => {
cy.visit(role ? getUrlWithRoute(role, url) : url);
cy.get('[data-test-subj="headerGlobalNav"]', { timeout: 120000 });
};
4 changes: 4 additions & 0 deletions x-pack/plugins/security_solution/cypress/urls/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
*/

export const DETECTIONS_URL = 'app/security/detections';
export const DETECTIONS_RULE_MANAGEMENT_URL = 'app/security/detections/rules';
export const detectionsRuleDetailsUrl = (ruleId: string) =>
`app/security/detections/rules/id/${ruleId}`;

export const CASES_URL = '/app/security/cases';
export const DETECTIONS = '/app/siem#/detections';
export const HOSTS_URL = '/app/security/hosts/allHosts';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, memo } from 'react';
import { EuiCallOut } from '@elastic/eui';

import { CallOutType, CallOutMessage } from './callout_types';
import { CallOutDescription } from './callout_description';
import { CallOutDismissButton } from './callout_dismiss_button';

export interface CallOutProps {
message: CallOutMessage;
iconType?: string;
dismissButtonText?: string;
onDismiss?: (message: CallOutMessage) => void;
}

const CallOutComponent: FC<CallOutProps> = ({
message,
iconType,
dismissButtonText,
onDismiss,
}) => {
const { type, id, title } = message;
const finalIconType = iconType ?? getDefaultIconType(type);

return (
<EuiCallOut
color={type}
title={title}
iconType={finalIconType}
data-test-subj={`callout-${id}`}
data-test-messages={`[${id}]`}
>
<CallOutDescription messages={message} />
<CallOutDismissButton message={message} text={dismissButtonText} onClick={onDismiss} />
</EuiCallOut>
);
};

const getDefaultIconType = (type: CallOutType): string => {
switch (type) {
case 'primary':
return 'iInCircle';
case 'success':
return 'cheer';
case 'warning':
return 'help';
case 'danger':
return 'alert';
default:
return '';
}
};

export const CallOut = memo(CallOutComponent);
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC } from 'react';
import { EuiDescriptionList } from '@elastic/eui';
import { CallOutMessage } from './callout_types';

export interface CallOutDescriptionProps {
messages: CallOutMessage | CallOutMessage[];
}

export const CallOutDescription: FC<CallOutDescriptionProps> = ({ messages }) => {
if (!Array.isArray(messages)) {
return messages.description;
}

if (messages.length < 1) {
return null;
}

return <EuiDescriptionList listItems={messages} />;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, useCallback } from 'react';
import { EuiButton } from '@elastic/eui';
import { noop } from 'lodash/fp';
import { CallOutMessage } from './callout_types';
import * as i18n from './translations';

export interface CallOutDismissButtonProps {
message: CallOutMessage;
text?: string;
onClick?: (message: CallOutMessage) => void;
}

export const CallOutDismissButton: FC<CallOutDismissButtonProps> = ({
message,
text,
onClick = noop,
}) => {
const { type } = message;
const buttonColor = type === 'success' ? 'secondary' : type;
const buttonText = text ?? i18n.DISMISS_BUTTON;
const handleClick = useCallback(() => onClick(message), [onClick, message]);

return (
<EuiButton color={buttonColor} data-test-subj="callout-dismiss-btn" onClick={handleClick}>
{buttonText}
</EuiButton>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, memo } from 'react';

import { CallOutMessage } from './callout_types';
import { CallOut } from './callout';
import { useCallOutStorage } from './use_callout_storage';

export interface CallOutSwitcherProps {
namespace?: string;
condition: boolean;
message: CallOutMessage;
}

const CallOutSwitcherComponent: FC<CallOutSwitcherProps> = ({ namespace, condition, message }) => {
const { isVisible, dismiss } = useCallOutStorage([message], namespace);

const shouldRender = condition && isVisible(message);
return shouldRender ? <CallOut message={message} onDismiss={dismiss} /> : null;
};

export const CallOutSwitcher = memo(CallOutSwitcherComponent);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export type CallOutType = 'primary' | 'success' | 'warning' | 'danger';

export interface CallOutMessage {
type: CallOutType;
id: string;
title: string;
description: JSX.Element;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export * from './callout_switcher';
export * from './callout_types';
export * from './callout';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { i18n } from '@kbn/i18n';

export const DISMISS_BUTTON = i18n.translate('xpack.securitySolution.callouts.dismissButton', {
defaultMessage: 'Dismiss',
});
Loading

0 comments on commit 68a99ca

Please sign in to comment.