forked from elastic/kibana
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Security Solution] Fix infinite loading state on Add Rules page for …
…users with `Security: Read` permissions (elastic#178005) Fixes: elastic#161543 ## Summary Solves edge case of a `Security: Read` user visiting the Add Rules page before a user with permissions does (therefore the space has no permissions). This would cause the `/install/_review` call to never happen, and the page to get stuck in an infinite loading state. - Encapsulates logic to calculate if the `/install/_review` endpoint should be called - Allows `Security: Read` users to make the endpoint call `/install/_review` - The "All Elastic rules already installed" screen is shown to users in this edge case. - Adds frontend integration tests to Add Tables page ### Checklist - [ ] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed (cherry picked from commit b8396f4)
- Loading branch information
Showing
3 changed files
with
346 additions
and
5 deletions.
There are no files selected for viewing
306 changes: 306 additions & 0 deletions
306
...ment_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_table.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
...management_ui/components/rules_table/add_prebuilt_rules_table/add_prebuilt_rules_utils.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
}; |