Skip to content

Commit

Permalink
[Security Solution] [Detections] Fix bug to allow lower privileged us…
Browse files Browse the repository at this point in the history
…ers to close alerts (#87761)

* remove canUserCRUD from signal actions and remove refresh param from open_close_signals route. 'refresh' requires maintenance / manage / all privileges for signals index

* adds 'maintenance' to privileges route

* fix unit teset typing

* update tests, updated lists e2e tests since it relies on the readPrivileges function of SIEM so any changes to the expected response from there must also be changed in the lists privileges route

* update scripts roles to include maintenance for roles that do not have privileges higher than 'maintenance'

* fix open-close signals integration test
  • Loading branch information
dhurley14 authored Jan 13, 2021
1 parent 500edba commit e339018
Show file tree
Hide file tree
Showing 20 changed files with 78 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ interface Index {
create: boolean;
manage_follow_index: boolean;
manage_leader_index: boolean;
maintenance: boolean;
write: boolean;
};
}
Expand Down Expand Up @@ -113,6 +114,7 @@ export const getReadPrivilegeMock = (
delete: booleanValues,
delete_index: booleanValues,
index: booleanValues,
maintenance: booleanValues,
manage: booleanValues,
manage_follow_index: booleanValues,
manage_ilm: booleanValues,
Expand Down Expand Up @@ -165,6 +167,7 @@ export const getReadPrivilegeMock = (
delete: booleanValues,
delete_index: booleanValues,
index: booleanValues,
maintenance: booleanValues,
manage: booleanValues,
manage_follow_index: booleanValues,
manage_ilm: booleanValues,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ describe('AlertsUtilityBar', () => {
test('renders correctly', () => {
const wrapper = shallow(
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
Expand All @@ -40,8 +40,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
Expand Down Expand Up @@ -76,8 +76,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
Expand Down Expand Up @@ -112,8 +112,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
Expand Down Expand Up @@ -148,8 +148,8 @@ describe('AlertsUtilityBar', () => {
const Proxy = (props: AlertsUtilityBarProps) => (
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
Expand All @@ -166,8 +166,8 @@ describe('AlertsUtilityBar', () => {

const wrapper = mount(
<Proxy
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import { UpdateAlertsStatus } from '../types';
import { FILTER_CLOSED, FILTER_IN_PROGRESS, FILTER_OPEN } from '../alerts_filter_group';

export interface AlertsUtilityBarProps {
canUserCRUD: boolean;
hasIndexWrite: boolean;
hasIndexMaintenance: boolean;
areEventsLoading: boolean;
clearSelection: () => void;
currentFilter: Status;
Expand Down Expand Up @@ -59,8 +59,8 @@ const BuildingBlockContainer = styled(EuiFlexItem)`
`;

const AlertsUtilityBarComponent: React.FC<AlertsUtilityBarProps> = ({
canUserCRUD,
hasIndexWrite,
hasIndexMaintenance,
areEventsLoading,
clearSelection,
totalCount,
Expand Down Expand Up @@ -180,7 +180,7 @@ const AlertsUtilityBarComponent: React.FC<AlertsUtilityBarProps> = ({
</UtilityBarGroup>

<UtilityBarGroup grow={true}>
{canUserCRUD && hasIndexWrite && (
{hasIndexWrite && hasIndexMaintenance && (
<>
<UtilityBarText dataTestSubj="selectedAlerts">
{i18n.SELECTED_ALERTS(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe('AlertsTableComponent', () => {
<TestProviders>
<AlertsTableComponent
timelineId={TimelineId.test}
canUserCRUD
hasIndexWrite
hasIndexMaintenance
from={'2020-07-07T08:20:18.966Z'}
loading
to={'2020-07-08T08:20:18.966Z'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ import { buildTimeRangeFilter } from './helpers';

interface OwnProps {
timelineId: TimelineIdLiteral;
canUserCRUD: boolean;
defaultFilters?: Filter[];
hasIndexWrite: boolean;
hasIndexMaintenance: boolean;
from: string;
loading: boolean;
onRuleChange?: () => void;
Expand All @@ -65,7 +65,6 @@ type AlertsTableComponentProps = OwnProps & PropsFromRedux;

export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
timelineId,
canUserCRUD,
clearEventsDeleted,
clearEventsLoading,
clearSelected,
Expand All @@ -74,6 +73,7 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
globalFilters,
globalQuery,
hasIndexWrite,
hasIndexMaintenance,
isSelectAllChecked,
loading,
loadingEventIds,
Expand Down Expand Up @@ -259,10 +259,10 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
(refetchQuery: inputsModel.Refetch, totalCount: number) => {
return (
<AlertsUtilityBar
canUserCRUD={canUserCRUD}
areEventsLoading={loadingEventIds.length > 0}
clearSelection={clearSelectionCallback}
hasIndexWrite={hasIndexWrite}
hasIndexMaintenance={hasIndexMaintenance}
currentFilter={filterGroup}
selectAll={selectAllOnAllPagesCallback}
selectedEventIds={selectedEventIds}
Expand All @@ -275,8 +275,8 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
);
},
[
canUserCRUD,
hasIndexWrite,
hasIndexMaintenance,
clearSelectionCallback,
filterGroup,
showBuildingBlockAlerts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
setPopover(false);
}, []);
const [exceptionModalType, setOpenAddExceptionModal] = useState<ExceptionListType | null>(null);
const [{ canUserCRUD, hasIndexWrite, hasIndexUpdateDelete }] = useUserData();
const [{ canUserCRUD, hasIndexWrite, hasIndexMaintenance, hasIndexUpdateDelete }] = useUserData();

const isEndpointAlert = useMemo((): boolean => {
if (ecsRowData == null) {
Expand Down Expand Up @@ -215,7 +215,7 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
data-test-subj="open-alert-status"
id={FILTER_OPEN}
onClick={openAlertActionOnClick}
disabled={!canUserCRUD || !hasIndexUpdateDelete}
disabled={!hasIndexUpdateDelete && !hasIndexMaintenance}
>
<EuiText size="m">{i18n.ACTION_OPEN_ALERT}</EuiText>
</EuiContextMenuItem>
Expand Down Expand Up @@ -248,7 +248,7 @@ const AlertContextMenuComponent: React.FC<AlertContextMenuProps> = ({
data-test-subj="close-alert-status"
id={FILTER_CLOSED}
onClick={closeAlertActionClick}
disabled={!canUserCRUD || !hasIndexUpdateDelete}
disabled={!hasIndexUpdateDelete && !hasIndexMaintenance}
>
<EuiText size="m">{i18n.ACTION_CLOSE_ALERT}</EuiText>
</EuiContextMenuItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('useUserInfo', () => {
canUserCRUD: null,
hasEncryptionKey: null,
hasIndexManage: null,
hasIndexMaintenance: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
isAuthenticated: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useKibana } from '../../../common/lib/kibana';
export interface State {
canUserCRUD: boolean | null;
hasIndexManage: boolean | null;
hasIndexMaintenance: boolean | null;
hasIndexWrite: boolean | null;
hasIndexUpdateDelete: boolean | null;
isSignalIndexExists: boolean | null;
Expand All @@ -27,6 +28,7 @@ export interface State {
export const initialState: State = {
canUserCRUD: null,
hasIndexManage: null,
hasIndexMaintenance: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
isSignalIndexExists: null,
Expand All @@ -43,6 +45,10 @@ export type Action =
type: 'updateHasIndexManage';
hasIndexManage: boolean | null;
}
| {
type: 'updateHasIndexMaintenance';
hasIndexMaintenance: boolean | null;
}
| {
type: 'updateHasIndexWrite';
hasIndexWrite: boolean | null;
Expand Down Expand Up @@ -90,6 +96,12 @@ export const userInfoReducer = (state: State, action: Action): State => {
hasIndexManage: action.hasIndexManage,
};
}
case 'updateHasIndexMaintenance': {
return {
...state,
hasIndexMaintenance: action.hasIndexMaintenance,
};
}
case 'updateHasIndexWrite': {
return {
...state,
Expand Down Expand Up @@ -162,6 +174,7 @@ export const useUserInfo = (): State => {
{
canUserCRUD,
hasIndexManage,
hasIndexMaintenance,
hasIndexWrite,
hasIndexUpdateDelete,
isSignalIndexExists,
Expand All @@ -178,6 +191,7 @@ export const useUserInfo = (): State => {
isAuthenticated: isApiAuthenticated,
hasEncryptionKey: isApiEncryptionKey,
hasIndexManage: hasApiIndexManage,
hasIndexMaintenance: hasApiIndexMaintenance,
hasIndexWrite: hasApiIndexWrite,
hasIndexUpdateDelete: hasApiIndexUpdateDelete,
} = usePrivilegeUser();
Expand Down Expand Up @@ -224,6 +238,16 @@ export const useUserInfo = (): State => {
}
}, [dispatch, loading, hasIndexUpdateDelete, hasApiIndexUpdateDelete]);

useEffect(() => {
if (
!loading &&
hasIndexMaintenance !== hasApiIndexMaintenance &&
hasApiIndexMaintenance != null
) {
dispatch({ type: 'updateHasIndexMaintenance', hasIndexMaintenance: hasApiIndexMaintenance });
}
}, [dispatch, loading, hasIndexMaintenance, hasApiIndexMaintenance]);

useEffect(() => {
if (
!loading &&
Expand Down Expand Up @@ -298,6 +322,7 @@ export const useUserInfo = (): State => {
hasEncryptionKey,
canUserCRUD,
hasIndexManage,
hasIndexMaintenance,
hasIndexWrite,
hasIndexUpdateDelete,
signalIndexName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ export const mockUserPrivilege: Privilege = {
index: {
'.siem-signals-default': {
all: true,
maintenance: true,
manage_ilm: true,
read: true,
create_index: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export interface Privilege {
index: {
[indexName: string]: {
all: boolean;
maintenance: boolean;
manage_ilm: boolean;
read: boolean;
create_index: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('usePrivilegeUser', () => {
expect(result.current).toEqual({
hasEncryptionKey: null,
hasIndexManage: null,
hasIndexMaintenance: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
isAuthenticated: null,
Expand All @@ -38,6 +39,7 @@ describe('usePrivilegeUser', () => {
expect(result.current).toEqual({
hasEncryptionKey: true,
hasIndexManage: true,
hasIndexMaintenance: true,
hasIndexWrite: true,
hasIndexUpdateDelete: true,
isAuthenticated: true,
Expand All @@ -60,6 +62,7 @@ describe('usePrivilegeUser', () => {
expect(result.current).toEqual({
hasEncryptionKey: false,
hasIndexManage: false,
hasIndexMaintenance: false,
hasIndexWrite: false,
hasIndexUpdateDelete: false,
isAuthenticated: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,31 @@ export interface ReturnPrivilegeUser {
hasIndexManage: boolean | null;
hasIndexWrite: boolean | null;
hasIndexUpdateDelete: boolean | null;
hasIndexMaintenance: boolean | null;
}
/**
* Hook to get user privilege from
*
*/
export const usePrivilegeUser = (): ReturnPrivilegeUser => {
const [loading, setLoading] = useState(true);
const [privilegeUser, setPrivilegeUser] = useState<Omit<ReturnPrivilegeUser, 'loading'>>({
const [privilegeUser, setPrivilegeUser] = useState<
Pick<
ReturnPrivilegeUser,
| 'isAuthenticated'
| 'hasEncryptionKey'
| 'hasIndexManage'
| 'hasIndexWrite'
| 'hasIndexUpdateDelete'
| 'hasIndexMaintenance'
>
>({
isAuthenticated: null,
hasEncryptionKey: null,
hasIndexManage: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
hasIndexMaintenance: null,
});
const [, dispatchToaster] = useStateToaster();

Expand All @@ -51,6 +63,7 @@ export const usePrivilegeUser = (): ReturnPrivilegeUser => {
isAuthenticated: privilege.is_authenticated,
hasEncryptionKey: privilege.has_encryption_key,
hasIndexManage: privilege.index[indexName].manage,
hasIndexMaintenance: privilege.index[indexName].maintenance,
hasIndexWrite:
privilege.index[indexName].create ||
privilege.index[indexName].create_doc ||
Expand All @@ -68,6 +81,7 @@ export const usePrivilegeUser = (): ReturnPrivilegeUser => {
hasIndexManage: false,
hasIndexWrite: false,
hasIndexUpdateDelete: false,
hasIndexMaintenance: false,
});
errorToToaster({ title: i18n.PRIVILEGE_FETCH_FAILURE, error, dispatchToaster });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ const DetectionEnginePageComponent = () => {
isSignalIndexExists,
isAuthenticated: isUserAuthenticated,
hasEncryptionKey,
canUserCRUD,
signalIndexName,
hasIndexWrite,
hasIndexMaintenance,
},
] = useUserData();
const {
Expand Down Expand Up @@ -232,7 +232,7 @@ const DetectionEnginePageComponent = () => {
timelineId={TimelineId.detectionsPage}
loading={loading}
hasIndexWrite={hasIndexWrite ?? false}
canUserCRUD={(canUserCRUD ?? false) && (hasEncryptionKey ?? false)}
hasIndexMaintenance={hasIndexMaintenance ?? false}
from={from}
defaultFilters={alertsTableDefaultFilters}
showBuildingBlockAlerts={showBuildingBlockAlerts}
Expand Down
Loading

0 comments on commit e339018

Please sign in to comment.