Skip to content

Commit

Permalink
[8.13] [Security Solution] Fix infinite loading state on Add Rules pa…
Browse files Browse the repository at this point in the history
…ge for users with `Security: Read` permissions (#178005) (#178603)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Security Solution] Fix infinite loading state on Add Rules page for
users with `Security: Read` permissions
(#178005)](#178005)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Juan Pablo
Djeredjian","email":"jpdjeredjian@gmail.com"},"sourceCommit":{"committedDate":"2024-03-12T23:16:20Z","message":"[Security
Solution] Fix infinite loading state on Add Rules page for users with
`Security: Read` permissions (#178005)\n\nFixes:
https://github.com/elastic/kibana/issues/161543\r\n\r\n##
Summary\r\n\r\nSolves edge case of a `Security: Read` user visiting the
Add Rules page\r\nbefore a user with permissions does (therefore the
space has no\r\npermissions). This would cause the `/install/_review`
call to never\r\nhappen, and the page to get stuck in an infinite
loading state.\r\n\r\n- Encapsulates logic to calculate if the
`/install/_review` endpoint\r\nshould be called\r\n- Allows `Security:
Read` users to make the endpoint call\r\n`/install/_review`\r\n- The
\"All Elastic rules already installed\" screen is shown to users
in\r\nthis edge case.\r\n- Adds frontend integration tests to Add Tables
page\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"b8396f48ce05f2c16d2fd9890f921260dd6a5a7d","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","v8.13.0","v8.14.0"],"number":178005,"url":"https://github.com/elastic/kibana/pull/178005","mergeCommit":{"message":"[Security
Solution] Fix infinite loading state on Add Rules page for users with
`Security: Read` permissions (#178005)\n\nFixes:
https://github.com/elastic/kibana/issues/161543\r\n\r\n##
Summary\r\n\r\nSolves edge case of a `Security: Read` user visiting the
Add Rules page\r\nbefore a user with permissions does (therefore the
space has no\r\npermissions). This would cause the `/install/_review`
call to never\r\nhappen, and the page to get stuck in an infinite
loading state.\r\n\r\n- Encapsulates logic to calculate if the
`/install/_review` endpoint\r\nshould be called\r\n- Allows `Security:
Read` users to make the endpoint call\r\n`/install/_review`\r\n- The
\"All Elastic rules already installed\" screen is shown to users
in\r\nthis edge case.\r\n- Adds frontend integration tests to Add Tables
page\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"b8396f48ce05f2c16d2fd9890f921260dd6a5a7d"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/178588","number":178588,"state":"OPEN"},{"branch":"main","label":"v8.14.0","labelRegex":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/178005","number":178005,"mergeCommit":{"message":"[Security
Solution] Fix infinite loading state on Add Rules page for users with
`Security: Read` permissions (#178005)\n\nFixes:
https://github.com/elastic/kibana/issues/161543\r\n\r\n##
Summary\r\n\r\nSolves edge case of a `Security: Read` user visiting the
Add Rules page\r\nbefore a user with permissions does (therefore the
space has no\r\npermissions). This would cause the `/install/_review`
call to never\r\nhappen, and the page to get stuck in an infinite
loading state.\r\n\r\n- Encapsulates logic to calculate if the
`/install/_review` endpoint\r\nshould be called\r\n- Allows `Security:
Read` users to make the endpoint call\r\n`/install/_review`\r\n- The
\"All Elastic rules already installed\" screen is shown to users
in\r\nthis edge case.\r\n- Adds frontend integration tests to Add Tables
page\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"b8396f48ce05f2c16d2fd9890f921260dd6a5a7d"}}]}]
BACKPORT-->

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jpdjere and kibanamachine authored Mar 13, 2024
1 parent 6f8e044 commit 4117bf9
Show file tree
Hide file tree
Showing 3 changed files with 346 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,306 @@
/*
* 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 React from 'react';
import { render, screen } from '@testing-library/react';
import { AddPrebuiltRulesTable } from './add_prebuilt_rules_table';
import { AddPrebuiltRulesHeaderButtons } from './add_prebuilt_rules_header_buttons';
import { AddPrebuiltRulesTableContextProvider } from './add_prebuilt_rules_table_context';

import { useUserData } from '../../../../../detections/components/user_info';
import { usePrebuiltRulesInstallReview } from '../../../../rule_management/logic/prebuilt_rules/use_prebuilt_rules_install_review';
import { useFetchPrebuiltRulesStatusQuery } from '../../../../rule_management/api/hooks/prebuilt_rules/use_fetch_prebuilt_rules_status_query';
import { useIsUpgradingSecurityPackages } from '../../../../rule_management/logic/use_upgrade_security_packages';

// Mock components not needed in this test suite
jest.mock('../../../../rule_management/components/rule_details/rule_details_flyout', () => ({
RuleDetailsFlyout: jest.fn().mockReturnValue(<></>),
}));
jest.mock('../rules_changelog_link', () => ({
RulesChangelogLink: jest.fn().mockReturnValue(<></>),
}));
jest.mock('./add_prebuilt_rules_table_filters', () => ({
AddPrebuiltRulesTableFilters: jest.fn().mockReturnValue(<></>),
}));

jest.mock('../../../../rule_management/logic/prebuilt_rules/use_perform_rule_install', () => ({
usePerformInstallAllRules: () => ({
performInstallAll: jest.fn(),
isLoading: false,
}),
usePerformInstallSpecificRules: () => ({
performInstallSpecific: jest.fn(),
isLoading: false,
}),
}));

jest.mock('../../../../../common/lib/kibana', () => ({
useUiSetting$: jest.fn().mockReturnValue([false]),
useKibana: jest.fn().mockReturnValue({
services: {
docLinks: { links: { siem: { ruleChangeLog: '' } } },
},
}),
}));

jest.mock('../../../../../common/components/links', () => ({
useGetSecuritySolutionLinkProps: () =>
jest.fn().mockReturnValue({
onClick: jest.fn(),
}),
}));

jest.mock(
'../../../../rule_management/api/hooks/prebuilt_rules/use_fetch_prebuilt_rules_status_query',
() => ({
useFetchPrebuiltRulesStatusQuery: jest.fn().mockReturnValue({
data: {
prebuiltRulesStatus: {
num_prebuilt_rules_total_in_package: 1,
},
},
}),
})
);

jest.mock('../../../../rule_management/logic/use_upgrade_security_packages', () => ({
useIsUpgradingSecurityPackages: jest.fn().mockImplementation(() => false),
}));

jest.mock(
'../../../../rule_management/logic/prebuilt_rules/use_prebuilt_rules_install_review',
() => ({
usePrebuiltRulesInstallReview: jest.fn().mockReturnValue({
data: {
rules: [
{
id: 'rule-1',
name: 'rule-1',
tags: [],
risk_score: 1,
severity: 'low',
},
],
stats: {
num_rules_to_install: 1,
tags: [],
},
},
isLoading: false,
isFetched: true,
}),
})
);

jest.mock('../../../../../detections/components/user_info', () => ({
useUserData: jest.fn(),
}));

describe('AddPrebuiltRulesTable', () => {
it('disables `Install all` button if user has no write permissions', async () => {
(useUserData as jest.Mock).mockReturnValue([
{
loading: false,
canUserCRUD: false,
},
]);

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
);

const installAllButton = screen.getByTestId('installAllRulesButton');

expect(installAllButton).toHaveTextContent('Install all');
expect(installAllButton).toBeDisabled();
});

it('disables `Install all` button if prebuilt package is being installed', async () => {
(useUserData as jest.Mock).mockReturnValue([
{
loading: false,
canUserCRUD: true,
},
]);

(useIsUpgradingSecurityPackages as jest.Mock).mockReturnValueOnce(true);

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
);

const installAllButton = screen.getByTestId('installAllRulesButton');

expect(installAllButton).toHaveTextContent('Install all');
expect(installAllButton).toBeDisabled();
});

it('enables Install all` button when user has permissions', async () => {
(useUserData as jest.Mock).mockReturnValue([
{
loading: false,
canUserCRUD: true,
},
]);

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesHeaderButtons />
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
);

const installAllButton = screen.getByTestId('installAllRulesButton');

expect(installAllButton).toHaveTextContent('Install all');
expect(installAllButton).toBeEnabled();
});

it.each([
['Security:Read', true],
['Security:Write', false],
])(
`renders "No rules available for install" when there are no rules to install and user has %s`,
async (_permissions, canUserCRUD) => {
(useUserData as jest.Mock).mockReturnValue([
{
loading: false,
canUserCRUD,
},
]);

(usePrebuiltRulesInstallReview as jest.Mock).mockReturnValueOnce({
data: {
rules: [],
stats: {
num_rules_to_install: 0,
tags: [],
},
},
isLoading: false,
isFetched: true,
});
(useFetchPrebuiltRulesStatusQuery as jest.Mock).mockReturnValueOnce({
data: {
prebuiltRulesStatus: {
num_prebuilt_rules_total_in_package: 0,
},
},
});

const { findByText } = render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
);

expect(await findByText('All Elastic rules have been installed')).toBeInTheDocument();
}
);

it('does not render `Install rule` on rule rows for users with no write permissions', async () => {
(useUserData as jest.Mock).mockReturnValue([
{
loading: false,
canUserCRUD: false,
},
]);

const id = 'rule-1';
(usePrebuiltRulesInstallReview as jest.Mock).mockReturnValueOnce({
data: {
rules: [
{
id,
rule_id: id,
name: 'rule-1',
tags: [],
risk_score: 1,
severity: 'low',
},
],
stats: {
num_rules_to_install: 1,
tags: [],
},
},
isLoading: false,
isFetched: true,
});
(useFetchPrebuiltRulesStatusQuery as jest.Mock).mockReturnValueOnce({
data: {
prebuiltRulesStatus: {
num_prebuilt_rules_total_in_package: 1,
},
},
});

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
);

const installRuleButton = screen.queryByTestId(`installSinglePrebuiltRuleButton-${id}`);

expect(installRuleButton).not.toBeInTheDocument();
});

it('renders `Install rule` on rule rows for users with write permissions', async () => {
(useUserData as jest.Mock).mockReturnValue([
{
loading: false,
canUserCRUD: true,
},
]);

const id = 'rule-1';
(usePrebuiltRulesInstallReview as jest.Mock).mockReturnValueOnce({
data: {
rules: [
{
id,
rule_id: id,
name: 'rule-1',
tags: [],
risk_score: 1,
severity: 'low',
},
],
stats: {
num_rules_to_install: 1,
tags: [],
},
},
isLoading: false,
isFetched: true,
});
(useFetchPrebuiltRulesStatusQuery as jest.Mock).mockReturnValueOnce({
data: {
prebuiltRulesStatus: {
num_prebuilt_rules_total_in_package: 1,
},
},
});

render(
<AddPrebuiltRulesTableContextProvider>
<AddPrebuiltRulesTable />
</AddPrebuiltRulesTableContextProvider>
);

const installRuleButton = screen.queryByTestId(`installSinglePrebuiltRuleButton-${id}`);

expect(installRuleButton).toBeInTheDocument();
expect(installRuleButton).toBeEnabled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { useRuleDetailsFlyout } from '../../../../rule_management/components/rul
import type { RuleResponse } from '../../../../../../common/api/detection_engine/model/rule_schema';
import { RuleDetailsFlyout } from '../../../../rule_management/components/rule_details/rule_details_flyout';
import * as i18n from './translations';
import { isUpgradeReviewRequestEnabled } from './add_prebuilt_rules_utils';

export interface AddPrebuiltRulesTableState {
/**
Expand Down Expand Up @@ -125,11 +126,11 @@ export const AddPrebuiltRulesTableContextProvider = ({
refetchInterval: 60000, // Refetch available rules for installation every minute
keepPreviousData: true, // Use this option so that the state doesn't jump between "success" and "loading" on page change
// Fetch rules to install only after background installation of security_detection_rules package is complete
enabled: Boolean(
!isUpgradingSecurityPackages &&
prebuiltRulesStatus &&
prebuiltRulesStatus.num_prebuilt_rules_total_in_package > 0
),
enabled: isUpgradeReviewRequestEnabled({
canUserCRUD,
isUpgradingSecurityPackages,
prebuiltRulesStatus,
}),
});

const { mutateAsync: installAllRulesRequest } = usePerformInstallAllRules();
Expand Down
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
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { PrebuiltRulesStatusStats } from '../../../../../../common/api/detection_engine';

interface UpgradeReviewEnabledProps {
canUserCRUD: boolean | null;
isUpgradingSecurityPackages: boolean;
prebuiltRulesStatus?: PrebuiltRulesStatusStats;
}

export const isUpgradeReviewRequestEnabled = ({
canUserCRUD,
isUpgradingSecurityPackages,
prebuiltRulesStatus,
}: UpgradeReviewEnabledProps) => {
// Wait until security package is updated
if (isUpgradingSecurityPackages) {
return false;
}

// If user is read-only, allow request to proceed even though the Prebuilt
// Rules might not be installed. For these users, the Fleet endpoint quickly
// fails with 403 so isUpgradingSecurityPackages is false
if (canUserCRUD === false) {
return true;
}

return prebuiltRulesStatus && prebuiltRulesStatus.num_prebuilt_rules_total_in_package > 0;
};

0 comments on commit 4117bf9

Please sign in to comment.