mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
[SecuritySolution] Fix Risk score Insufficient privileges warning missing cluster privileges (#212405)
## Summary ### * Fixes Bug: User with no cluster privileges should not be able to enable the risk score When users with no cluster privileges open the risk score page, they don't see any errors and are able to click the install button. This happened because we were only checking for index privileges in the UI, but for the enablement flow we also need to check cluster privileges. I also introduced a new parameter to the missing privileges hook so pages that only need to check for `read` privileges can work as before. https://github.com/user-attachments/assets/fe162005-ee2b-497d-8744-6262e4511d2d * Fixed Bug: The install button was enabled when all toggles were disabled There were too many booleans in the panel, which was confusing and led me to introduce more bugs while trying to fix this one, so I refactored the code to understand it before fixing it. I also simplified the logic to display the modal. Now, it only shows when one of the engines' status is "not_installed" <img width="300" src="https://github.com/user-attachments/assets/a2e8fbba-ac64-4c97-9ef0-ef6fe61e60cd" /> ### To Reproduce 1. Create a user with security privileges and index privileges but no cluster privileges 2. Go to the risk score page and enable the toggle ### Checklist Check the PR satisfies following conditions. - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or
This commit is contained in:
parent
f277497d7b
commit
b69b696e7f
8 changed files with 122 additions and 86 deletions
|
@ -172,7 +172,7 @@ const EntityAnalyticsRiskScoresComponent = <T extends EntityType>({
|
|||
setUpdatedAt(Date.now());
|
||||
}, [isTableLoading, isKpiLoading]); // Update the time when data loads
|
||||
|
||||
const privileges = useMissingRiskEnginePrivileges(['read']);
|
||||
const privileges = useMissingRiskEnginePrivileges({ readonly: true });
|
||||
|
||||
if (!isAuthorized) {
|
||||
return null;
|
||||
|
|
|
@ -187,7 +187,7 @@ export const EnablementPanel: React.FC<EnableEntityStorePanelProps> = ({ state }
|
|||
|
||||
if (
|
||||
riskEngineStatus !== RiskEngineStatusEnum.NOT_INSTALLED &&
|
||||
(entityStoreStatus === 'running' || entityStoreStatus === 'stopped')
|
||||
entityStoreStatus !== 'not_installed'
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
@ -224,14 +224,8 @@ export const EnablementPanel: React.FC<EnableEntityStorePanelProps> = ({ state }
|
|||
visible={modal.visible}
|
||||
toggle={(visible) => setModalState({ visible })}
|
||||
enableStore={enableEntityStore}
|
||||
riskScore={{
|
||||
canToggle: riskEngineStatus === RiskEngineStatusEnum.NOT_INSTALLED,
|
||||
checked: riskEngineStatus === RiskEngineStatusEnum.NOT_INSTALLED,
|
||||
}}
|
||||
entityStore={{
|
||||
canToggle: entityStoreStatus !== 'running',
|
||||
checked: entityStoreStatus === 'not_installed',
|
||||
}}
|
||||
riskEngineStatus={riskEngineStatus}
|
||||
entityStoreStatus={entityStoreStatus}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
|
|
|
@ -8,9 +8,14 @@
|
|||
import React from 'react';
|
||||
import { act } from 'react-dom/test-utils';
|
||||
import { fireEvent, render, screen } from '@testing-library/react';
|
||||
import type { EntityStoreEnablementModalProps } from './enablement_modal';
|
||||
import { EntityStoreEnablementModal } from './enablement_modal';
|
||||
import { TestProviders } from '../../../../common/mock';
|
||||
import type { EntityAnalyticsPrivileges } from '../../../../../common/api/entity_analytics';
|
||||
import {
|
||||
type EntityAnalyticsPrivileges,
|
||||
RiskEngineStatusEnum,
|
||||
StoreStatusEnum,
|
||||
} from '../../../../../common/api/entity_analytics';
|
||||
import type { RiskEngineMissingPrivilegesResponse } from '../../../hooks/use_missing_risk_engine_privileges';
|
||||
|
||||
const mockToggle = jest.fn();
|
||||
|
@ -35,8 +40,8 @@ const defaultProps = {
|
|||
visible: true,
|
||||
toggle: mockToggle,
|
||||
enableStore: mockEnableStore,
|
||||
riskScore: { canToggle: false, checked: false },
|
||||
entityStore: { canToggle: false, checked: false },
|
||||
riskEngineStatus: RiskEngineStatusEnum.NOT_INSTALLED,
|
||||
entityStoreStatus: StoreStatusEnum.not_installed,
|
||||
};
|
||||
|
||||
const allEntityEnginePrivileges: EntityAnalyticsPrivileges = {
|
||||
|
@ -83,7 +88,7 @@ const missingRiskEnginePrivileges: RiskEngineMissingPrivilegesResponse = {
|
|||
},
|
||||
};
|
||||
|
||||
const renderComponent = async (props = defaultProps) => {
|
||||
const renderComponent = async (props: EntityStoreEnablementModalProps = defaultProps) => {
|
||||
await act(async () => {
|
||||
return render(<EntityStoreEnablementModal {...props} />, { wrapper: TestProviders });
|
||||
});
|
||||
|
@ -121,46 +126,45 @@ describe('EntityStoreEnablementModal', () => {
|
|||
});
|
||||
|
||||
it('should call enableStore function when enable button is clicked', () => {
|
||||
renderComponent({
|
||||
...defaultProps,
|
||||
riskScore: { ...defaultProps.riskScore, checked: true },
|
||||
entityStore: { ...defaultProps.entityStore, checked: true },
|
||||
});
|
||||
renderComponent(defaultProps);
|
||||
fireEvent.click(screen.getByText('Enable'));
|
||||
expect(mockEnableStore).toHaveBeenCalledWith({ riskScore: true, entityStore: true });
|
||||
});
|
||||
|
||||
it('should display proceed warning when no enablement options are selected', () => {
|
||||
renderComponent();
|
||||
fireEvent.click(screen.getByTestId('enablementEntityStoreSwitch')); // unselect entity store
|
||||
fireEvent.click(screen.getByTestId('enablementRiskScoreSwitch')); // unselect risk engine
|
||||
expect(screen.getByText('Please enable at least one option to proceed.')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('should disable the enable button when enablementOptions are false', () => {
|
||||
renderComponent({
|
||||
...defaultProps,
|
||||
riskScore: { ...defaultProps.riskScore, checked: false },
|
||||
entityStore: { ...defaultProps.entityStore, checked: false },
|
||||
});
|
||||
renderComponent();
|
||||
fireEvent.click(screen.getByTestId('enablementEntityStoreSwitch')); // unselect entity store
|
||||
fireEvent.click(screen.getByTestId('enablementRiskScoreSwitch')); // unselect risk engine
|
||||
|
||||
const enableButton = screen.getByRole('button', { name: /Enable/i });
|
||||
expect(enableButton).toBeDisabled();
|
||||
});
|
||||
|
||||
it('should show proceed warning when riskScore is enabled but entityStore is disabled and unchecked', () => {
|
||||
it('should show proceed warning when riskScore is not installed and unchecked but entityStore is already running', () => {
|
||||
renderComponent({
|
||||
...defaultProps,
|
||||
riskScore: { canToggle: false, checked: false }, // Enabled & Checked
|
||||
entityStore: { canToggle: true, checked: false }, // Disabled & Unchecked
|
||||
riskEngineStatus: RiskEngineStatusEnum.NOT_INSTALLED,
|
||||
entityStoreStatus: StoreStatusEnum.running,
|
||||
});
|
||||
fireEvent.click(screen.getByTestId('enablementRiskScoreSwitch')); // unselect risk engine
|
||||
expect(screen.getByText('Please enable at least one option to proceed.')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('should show proceed warning when entityStore is enabled but riskScore is disabled and unchecked', () => {
|
||||
it('should show proceed warning when entityStore is not installed and unchecked but riskScore is already installed', () => {
|
||||
renderComponent({
|
||||
...defaultProps,
|
||||
entityStore: { canToggle: false, checked: false }, // Enabled & Checked
|
||||
riskScore: { canToggle: true, checked: false }, // Disabled & Unchecked
|
||||
riskEngineStatus: RiskEngineStatusEnum.ENABLED,
|
||||
entityStoreStatus: StoreStatusEnum.not_installed,
|
||||
});
|
||||
fireEvent.click(screen.getByTestId('enablementEntityStoreSwitch')); // unselect risk engine
|
||||
|
||||
expect(screen.getByText('Please enable at least one option to proceed.')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
|
@ -209,5 +213,13 @@ describe('EntityStoreEnablementModal', () => {
|
|||
|
||||
expect(screen.queryByTestId('enablement-modal-test')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('should disabled the "enable" button', async () => {
|
||||
renderComponent();
|
||||
expect(screen.getByTestId('callout-missing-entity-store-privileges')).toBeInTheDocument();
|
||||
|
||||
const enableButton = screen.getByRole('button', { name: /Enable/i });
|
||||
expect(enableButton).toBeDisabled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -22,73 +22,94 @@ import {
|
|||
useEuiTheme,
|
||||
} from '@elastic/eui';
|
||||
import { css } from '@emotion/react';
|
||||
import React, { useState } from 'react';
|
||||
import React, { useEffect, useState } from 'react';
|
||||
import { FormattedMessage } from '@kbn/i18n-react';
|
||||
import type { RiskEngineStatus, StoreStatus } from '../../../../../common/api/entity_analytics';
|
||||
import { RiskEngineStatusEnum } from '../../../../../common/api/entity_analytics';
|
||||
import { useContractComponents } from '../../../../common/hooks/use_contract_component';
|
||||
import {
|
||||
ENABLEMENT_DESCRIPTION_RISK_ENGINE_ONLY,
|
||||
ENABLEMENT_DESCRIPTION_ENTITY_STORE_ONLY,
|
||||
ENABLEMENT_WARNING_SELECT_TO_PROCEED,
|
||||
} from '../translations';
|
||||
import { useEntityEnginePrivileges } from '../hooks/use_entity_engine_privileges';
|
||||
import { MissingPrivilegesCallout } from './missing_privileges_callout';
|
||||
import { useMissingRiskEnginePrivileges } from '../../../hooks/use_missing_risk_engine_privileges';
|
||||
import { RiskEnginePrivilegesCallOut } from '../../risk_engine_privileges_callout';
|
||||
import { useEntityEnginePrivileges } from '../hooks/use_entity_engine_privileges';
|
||||
|
||||
export interface Enablements {
|
||||
riskScore: boolean;
|
||||
entityStore: boolean;
|
||||
}
|
||||
|
||||
interface EntityStoreEnablementModalProps {
|
||||
export interface EntityStoreEnablementModalProps {
|
||||
visible: boolean;
|
||||
toggle: (visible: boolean) => void;
|
||||
enableStore: (enablements: Enablements) => () => void;
|
||||
riskScore: {
|
||||
canToggle?: boolean;
|
||||
checked?: boolean;
|
||||
};
|
||||
entityStore: {
|
||||
canToggle?: boolean;
|
||||
checked?: boolean;
|
||||
};
|
||||
riskEngineStatus?: RiskEngineStatus;
|
||||
entityStoreStatus?: StoreStatus;
|
||||
}
|
||||
|
||||
const shouldAllowEnablement = (
|
||||
riskScoreEnabled: boolean,
|
||||
entityStoreEnabled: boolean,
|
||||
const isInstallButtonEnabled = (
|
||||
canInstallRiskScore: boolean,
|
||||
canInstallEntityStore: boolean,
|
||||
userHasEnabled: Enablements
|
||||
) => {
|
||||
if (riskScoreEnabled) {
|
||||
return userHasEnabled.entityStore;
|
||||
if (canInstallRiskScore || canInstallEntityStore) {
|
||||
return userHasEnabled.riskScore || userHasEnabled.entityStore;
|
||||
}
|
||||
if (entityStoreEnabled) {
|
||||
return userHasEnabled.riskScore;
|
||||
}
|
||||
return userHasEnabled.riskScore || userHasEnabled.entityStore;
|
||||
|
||||
return false;
|
||||
};
|
||||
|
||||
export const EntityStoreEnablementModal: React.FC<EntityStoreEnablementModalProps> = ({
|
||||
visible,
|
||||
toggle,
|
||||
enableStore,
|
||||
riskScore,
|
||||
entityStore,
|
||||
riskEngineStatus,
|
||||
entityStoreStatus,
|
||||
}) => {
|
||||
const { euiTheme } = useEuiTheme();
|
||||
const [enablements, setEnablements] = useState({
|
||||
riskScore: !!riskScore.checked,
|
||||
entityStore: !!entityStore.checked,
|
||||
});
|
||||
const riskEnginePrivileges = useMissingRiskEnginePrivileges();
|
||||
const { data: entityEnginePrivileges, isLoading: isLoadingEntityEnginePrivileges } =
|
||||
useEntityEnginePrivileges();
|
||||
const riskEnginePrivileges = useMissingRiskEnginePrivileges();
|
||||
|
||||
const enablementOptions = shouldAllowEnablement(
|
||||
!riskScore.canToggle,
|
||||
!entityStore.canToggle,
|
||||
enablements
|
||||
const hasRiskScorePrivileges = !(
|
||||
riskEnginePrivileges.isLoading || !riskEnginePrivileges?.hasAllRequiredPrivileges
|
||||
);
|
||||
|
||||
const canInstallRiskScore =
|
||||
hasRiskScorePrivileges && riskEngineStatus === RiskEngineStatusEnum.NOT_INSTALLED;
|
||||
|
||||
const hasEntityStorePrivileges = !(
|
||||
isLoadingEntityEnginePrivileges || !entityEnginePrivileges?.has_all_required
|
||||
);
|
||||
|
||||
const canInstallEntityStore = hasEntityStorePrivileges && entityStoreStatus === 'not_installed';
|
||||
|
||||
const { euiTheme } = useEuiTheme();
|
||||
const [toggleState, setToggleState] = useState({
|
||||
riskScore: false,
|
||||
entityStore: false,
|
||||
});
|
||||
|
||||
/**
|
||||
* Update the toggle state when the install status changes because privileges are async.
|
||||
* We automatically toggle the switch when the user can enable the engine.
|
||||
*
|
||||
*/
|
||||
useEffect(() => {
|
||||
setToggleState({
|
||||
riskScore: canInstallRiskScore,
|
||||
entityStore: canInstallEntityStore,
|
||||
});
|
||||
}, [canInstallRiskScore, canInstallEntityStore]);
|
||||
|
||||
const isInstallButtonDisabled = !isInstallButtonEnabled(
|
||||
canInstallRiskScore,
|
||||
canInstallEntityStore,
|
||||
toggleState
|
||||
);
|
||||
|
||||
const { AdditionalChargesMessage } = useContractComponents();
|
||||
|
||||
if (!visible) {
|
||||
|
@ -127,12 +148,9 @@ export const EntityStoreEnablementModal: React.FC<EntityStoreEnablementModalProp
|
|||
defaultMessage="Risk Score"
|
||||
/>
|
||||
}
|
||||
checked={enablements.riskScore}
|
||||
disabled={
|
||||
!riskScore.canToggle ||
|
||||
(!riskEnginePrivileges.isLoading && !riskEnginePrivileges?.hasAllRequiredPrivileges)
|
||||
}
|
||||
onChange={() => setEnablements((prev) => ({ ...prev, riskScore: !prev.riskScore }))}
|
||||
checked={toggleState.riskScore}
|
||||
disabled={!canInstallRiskScore}
|
||||
onChange={() => setToggleState((prev) => ({ ...prev, riskScore: !prev.riskScore }))}
|
||||
data-test-subj="enablementRiskScoreSwitch"
|
||||
/>
|
||||
</EuiFlexItem>
|
||||
|
@ -154,13 +172,10 @@ export const EntityStoreEnablementModal: React.FC<EntityStoreEnablementModalProp
|
|||
defaultMessage="Entity Store"
|
||||
/>
|
||||
}
|
||||
checked={enablements.entityStore}
|
||||
disabled={
|
||||
!entityStore.canToggle ||
|
||||
(!isLoadingEntityEnginePrivileges && !entityEnginePrivileges?.has_all_required)
|
||||
}
|
||||
checked={toggleState.entityStore}
|
||||
disabled={!canInstallEntityStore}
|
||||
onChange={() =>
|
||||
setEnablements((prev) => ({ ...prev, entityStore: !prev.entityStore }))
|
||||
setToggleState((prev) => ({ ...prev, entityStore: !prev.entityStore }))
|
||||
}
|
||||
data-test-subj="enablementEntityStoreSwitch"
|
||||
/>
|
||||
|
@ -179,15 +194,17 @@ export const EntityStoreEnablementModal: React.FC<EntityStoreEnablementModalProp
|
|||
|
||||
<EuiModalFooter>
|
||||
<EuiFlexGroup justifyContent="flexEnd" alignItems="center">
|
||||
{!enablementOptions ? <EuiFlexItem>{proceedWarning}</EuiFlexItem> : null}
|
||||
{isInstallButtonDisabled && (canInstallRiskScore || canInstallEntityStore) ? (
|
||||
<EuiFlexItem>{proceedWarning}</EuiFlexItem>
|
||||
) : null}
|
||||
<EuiFlexItem grow={false}>
|
||||
<EuiFlexGroup direction="row" justifyContent="flexEnd">
|
||||
<EuiButtonEmpty onClick={() => toggle(false)}>{'Cancel'}</EuiButtonEmpty>
|
||||
<EuiButton
|
||||
onClick={enableStore(enablements)}
|
||||
onClick={enableStore(toggleState)}
|
||||
fill
|
||||
isDisabled={!enablementOptions}
|
||||
aria-disabled={!enablementOptions}
|
||||
isDisabled={isInstallButtonDisabled}
|
||||
aria-disabled={isInstallButtonDisabled}
|
||||
data-test-subj="entityStoreEnablementModalButton"
|
||||
>
|
||||
<FormattedMessage
|
||||
|
|
|
@ -93,7 +93,7 @@ const RiskDetailsTabBodyComponent: React.FC<
|
|||
[setContributorsToggleStatus]
|
||||
);
|
||||
|
||||
const privileges = useMissingRiskEnginePrivileges();
|
||||
const privileges = useMissingRiskEnginePrivileges({ readonly: true });
|
||||
|
||||
const RiskScoreUpsell = useUpsellingComponent('entity_analytics_panel');
|
||||
if (RiskScoreUpsell) {
|
||||
|
|
|
@ -64,7 +64,7 @@ export const UserRiskScoreQueryTabBody = ({
|
|||
|
||||
const timerange = useMemo(() => ({ from, to }), [from, to]);
|
||||
|
||||
const privileges = useMissingRiskEnginePrivileges();
|
||||
const privileges = useMissingRiskEnginePrivileges({ readonly: true });
|
||||
|
||||
const { data, inspect, isInspected, hasEngineBeenInstalled, loading, refetch, totalCount } =
|
||||
useRiskScore({
|
||||
|
|
|
@ -22,8 +22,14 @@ export type RiskEngineMissingPrivilegesResponse =
|
|||
hasAllRequiredPrivileges: false;
|
||||
};
|
||||
|
||||
interface UseMissingRiskEnginePrivilegesParams {
|
||||
/**
|
||||
* If `true`, only read privileges are required.
|
||||
*/
|
||||
readonly: boolean;
|
||||
}
|
||||
export const useMissingRiskEnginePrivileges = (
|
||||
required: NonEmptyArray<RiskEngineIndexPrivilege> = ['read', 'write']
|
||||
{ readonly }: UseMissingRiskEnginePrivilegesParams = { readonly: false }
|
||||
): RiskEngineMissingPrivilegesResponse => {
|
||||
const { data: privilegesResponse, isLoading } = useRiskEnginePrivileges();
|
||||
|
||||
|
@ -41,14 +47,21 @@ export const useMissingRiskEnginePrivileges = (
|
|||
};
|
||||
}
|
||||
|
||||
const requiredIndexPrivileges: NonEmptyArray<RiskEngineIndexPrivilege> = readonly
|
||||
? ['read']
|
||||
: ['read', 'write'];
|
||||
|
||||
const { indexPrivileges, clusterPrivileges } = getMissingRiskEnginePrivileges(
|
||||
privilegesResponse.privileges,
|
||||
required
|
||||
requiredIndexPrivileges
|
||||
);
|
||||
|
||||
// privilegesResponse.has_all_required` is slightly misleading, it checks if it has *all* default required privileges.
|
||||
// Here we check if there are no missing privileges of the provided set of required privileges
|
||||
if (indexPrivileges.every(([_, missingPrivileges]) => missingPrivileges.length === 0)) {
|
||||
if (
|
||||
indexPrivileges.every(([_, missingPrivileges]) => missingPrivileges.length === 0) &&
|
||||
(readonly || clusterPrivileges.length === 0) // cluster privileges check is required for write operations
|
||||
) {
|
||||
return {
|
||||
isLoading: false,
|
||||
hasAllRequiredPrivileges: true,
|
||||
|
@ -60,8 +73,8 @@ export const useMissingRiskEnginePrivileges = (
|
|||
hasAllRequiredPrivileges: false,
|
||||
missingPrivileges: {
|
||||
indexPrivileges,
|
||||
clusterPrivileges,
|
||||
clusterPrivileges: readonly ? [] : clusterPrivileges, // cluster privileges are not required for readonly
|
||||
},
|
||||
};
|
||||
}, [isLoading, privilegesResponse, required]);
|
||||
}, [isLoading, privilegesResponse, readonly]);
|
||||
};
|
||||
|
|
|
@ -63,7 +63,7 @@ export const HostRiskScoreQueryTabBody = ({
|
|||
}, [toggleStatus]);
|
||||
const timerange = useMemo(() => ({ from, to }), [from, to]);
|
||||
|
||||
const privileges = useMissingRiskEnginePrivileges();
|
||||
const privileges = useMissingRiskEnginePrivileges({ readonly: true });
|
||||
const { data, inspect, isInspected, hasEngineBeenInstalled, loading, refetch, totalCount } =
|
||||
useRiskScore({
|
||||
filterQuery,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue