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

[Security Solutions][Detection Engine] Adds a warning banner when the alerts data has not been migrated yet. #90258

Merged
merged 14 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 9 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

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 {
loginAndWaitForPageWithoutDateRange,
waitForPageWithoutDateRange,
} from '../../tasks/login';
import { waitForAlertsIndexToBeCreated } from '../../tasks/alerts';
import { goToRuleDetails } from '../../tasks/alerts_detection_rules';
import { createCustomRule, deleteCustomRule } from '../../tasks/api_calls/rules';
import { getCallOut } from '../../tasks/common/callouts';
import { cleanKibana } from '../../tasks/common';

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

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

describe('Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set', () => {
const NEED_ADMIN_FOR_UPDATE_CALLOUT = 'need-admin-for-update-rules';
const ALERTS_CALLOUT = 'read-only-access-to-alerts';
const RULES_CALLOUT = 'read-only-access-to-rules';

before(() => {
cleanKibana();
loginAndWaitForPageWithoutDateRange(DETECTIONS_URL, ROLES.platform_engineer);
waitForAlertsIndexToBeCreated();
});

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

it('We show no callouts', () => {
getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('not.exist');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});
});

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

it('We show no callouts', () => {
getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('not.exist');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});
});

context('On Rule Details page', () => {
beforeEach(() => {
createCustomRule(newRule);
loadPageAsPlatformEngineerUser(DETECTIONS_RULE_MANAGEMENT_URL);
waitForPageTitleToBeShown();
goToRuleDetails();
});

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

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

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 {
loginAndWaitForPageWithoutDateRange,
waitForPageWithoutDateRange,
} from '../../tasks/login';
import { waitForAlertsIndexToBeCreated } from '../../tasks/alerts';
import { goToRuleDetails } from '../../tasks/alerts_detection_rules';
import { createCustomRule, deleteCustomRule } from '../../tasks/api_calls/rules';
import { getCallOut } from '../../tasks/common/callouts';
import { cleanKibana } from '../../tasks/common';

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

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

describe('Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set', () => {
const NEED_ADMIN_FOR_UPDATE_CALLOUT = 'need-admin-for-update-rules';
const ALERTS_CALLOUT = 'read-only-access-to-alerts';
const RULES_CALLOUT = 'read-only-access-to-rules';
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could omit checking these guys in all tests which check the behaviour of NEED_ADMIN_FOR_UPDATE_CALLOUT?

Copy link
Contributor Author

@FrankHassanabad FrankHassanabad Feb 9, 2021

Choose a reason for hiding this comment

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

I think these are missing tests for the callouts since so far what we have been testing is that callouts will appear when they're supposed to appear based on a lack of permissions. However, the other flip side is that we need tests based on when the user has sufficient permissions and some if not all the callouts should not appear.

So, without tests that are checking for the non-existence of callouts when users have sufficient privileges we could introduce bugs where callouts show up on pages when users have sufficient privileges and we wouldn't catch it. With these additional tests in place we should hopefully catch all if not most of the test cases where:

  • Users have such sufficient privileges that no callouts appear.
  • Users do not have sufficient privileges and 1 or more of the callouts should appear.
  • Repeat the same two tests but now with index_mapping being outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, without tests that are checking for the non-existence of callouts when users have sufficient privileges we could introduce bugs where callouts show up on pages when users have sufficient privileges and we wouldn't catch it

That's a great point, I agree! However after re-reading all my comments, and the answers, and checking the code I think I just failed to explain what I meant. Would you have some time to chat on that stuff specifically?


before(() => {
cleanKibana();
loginAndWaitForPageWithoutDateRange(DETECTIONS_URL, ROLES.platform_engineer);
waitForAlertsIndexToBeCreated();
});

beforeEach(() => {
// Index mapping outdated is forced to return true as being outdated so that we get the
// need admin callouts being shown.
cy.intercept('GET', '/api/detection_engine/index', {
index_mapping_outdated: true,
name: '.siem-signals-default',
});
});

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

it('We show the need admin primary callout', () => {
getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('not.exist');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});
});

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

it('We show 1 primary callout of need admin', () => {
getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('not.exist');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});
});

context('On Rule Details page', () => {
beforeEach(() => {
createCustomRule(newRule);
loadPageAsPlatformEngineerUser(DETECTIONS_RULE_MANAGEMENT_URL);
waitForPageTitleToBeShown();
goToRuleDetails();
});

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

it('We show 1 primary callout', () => {
getCallOut(ALERTS_CALLOUT).should('not.exist');
getCallOut(RULES_CALLOUT).should('not.exist');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const waitForPageTitleToBeShown = () => {
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';
const NEED_ADMIN_FOR_UPDATE_CALLOUT = 'need-admin-for-update-rules';
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, do you think this test should check NEED_ADMIN_FOR_UPDATE_CALLOUT as well? I'd check this one only in the newly added tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, because with this role I do not want the NEED_ADMIN_FOR_UPDATE_CALLOUT to end up showing per the user's role and the fact that this test will return index_mapping_outdated: false.

The other test that is the companion to this one is:

alerts_detection_callouts_readonly_need_admin.spec.ts

Where it returns index_mapping_outdated: true and then I test to ensure that it shows up next to each of these callouts.


before(() => {
// First, we have to open the app on behalf of a privileged user in order to initialize it.
Expand All @@ -57,6 +58,7 @@ describe('Detections > Callouts indicating read-only access to resources', () =>

it('We show one primary callout', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});

context('When a user clicks Dismiss on the callout', () => {
Expand All @@ -76,6 +78,7 @@ describe('Detections > Callouts indicating read-only access to resources', () =>

it('We show one primary callout', () => {
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});

context('When a user clicks Dismiss on the callout', () => {
Expand Down Expand Up @@ -103,24 +106,28 @@ describe('Detections > Callouts indicating read-only access to resources', () =>
it('We show two primary callouts', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');
});

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');
getCallOut(NEED_ADMIN_FOR_UPDATE_CALLOUT).should('not.exist');

dismissCallOut(ALERTS_CALLOUT);
reloadPage();

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

dismissCallOut(RULES_CALLOUT);
reloadPage();

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

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,
} from '../../tasks/login';
import { waitForAlertsIndexToBeCreated } from '../../tasks/alerts';
import { goToRuleDetails } from '../../tasks/alerts_detection_rules';
import { createCustomRule, deleteCustomRule } from '../../tasks/api_calls/rules';
import { getCallOut, waitForCallOutToBeShown, dismissCallOut } from '../../tasks/common/callouts';
import { cleanKibana } from '../../tasks/common';

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 > Need Admin Callouts indicating an admin is needed to migrate the alert data set', () => {
const ALERTS_CALLOUT = 'read-only-access-to-alerts';
const RULES_CALLOUT = 'read-only-access-to-rules';
const NEED_ADMIN_FOR_UPDATE_CALLOUT = 'need-admin-for-update-rules';

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

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

beforeEach(() => {
// Index mapping outdated is forced to return true as being outdated so that we get the
// need admin callouts being shown.
cy.intercept('GET', '/api/detection_engine/index', {
index_mapping_outdated: true,
name: '.siem-signals-default',
});
});

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

it('We show the alerts and admin primary callout', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
waitForCallOutToBeShown(NEED_ADMIN_FOR_UPDATE_CALLOUT, 'primary');
});

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

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

it('We show two primary callouts of the alert and the admin', () => {
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
waitForCallOutToBeShown(NEED_ADMIN_FOR_UPDATE_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');
waitForCallOutToBeShown(NEED_ADMIN_FOR_UPDATE_CALLOUT, 'primary');
});
});
});

context('On Rule Details page', () => {
beforeEach(() => {
createCustomRule(newRule);
loadPageAsReadOnlyUser(DETECTIONS_RULE_MANAGEMENT_URL);
waitForPageTitleToBeShown();
goToRuleDetails();
});

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

it('We show three primary callouts', () => {
waitForCallOutToBeShown(ALERTS_CALLOUT, 'primary');
waitForCallOutToBeShown(RULES_CALLOUT, 'primary');
waitForCallOutToBeShown(NEED_ADMIN_FOR_UPDATE_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');
waitForCallOutToBeShown(NEED_ADMIN_FOR_UPDATE_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');
waitForCallOutToBeShown(NEED_ADMIN_FOR_UPDATE_CALLOUT, 'primary');
});
});
});
});
Loading