[Security Solution] [Detections] Fix bug to allow lower privileged users 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
This commit is contained in:
Devin W. Hurley 2021-01-12 21:16:06 -05:00 committed by GitHub
parent 500edba975
commit e339018285
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 78 additions and 28 deletions

View file

@ -49,6 +49,7 @@ interface Index {
create: boolean;
manage_follow_index: boolean;
manage_leader_index: boolean;
maintenance: boolean;
write: boolean;
};
}
@ -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,
@ -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,

View file

@ -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}
@ -40,8 +40,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
@ -76,8 +76,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
@ -112,8 +112,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<TestProviders>
<AlertsUtilityBar
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}
@ -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}
@ -166,8 +166,8 @@ describe('AlertsUtilityBar', () => {
const wrapper = mount(
<Proxy
canUserCRUD={true}
hasIndexWrite={true}
hasIndexMaintenance={true}
areEventsLoading={false}
clearSelection={jest.fn()}
totalCount={100}

View file

@ -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;
@ -59,8 +59,8 @@ const BuildingBlockContainer = styled(EuiFlexItem)`
`;
const AlertsUtilityBarComponent: React.FC<AlertsUtilityBarProps> = ({
canUserCRUD,
hasIndexWrite,
hasIndexMaintenance,
areEventsLoading,
clearSelection,
totalCount,
@ -180,7 +180,7 @@ const AlertsUtilityBarComponent: React.FC<AlertsUtilityBarProps> = ({
</UtilityBarGroup>
<UtilityBarGroup grow={true}>
{canUserCRUD && hasIndexWrite && (
{hasIndexWrite && hasIndexMaintenance && (
<>
<UtilityBarText dataTestSubj="selectedAlerts">
{i18n.SELECTED_ALERTS(

View file

@ -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'}

View file

@ -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;
@ -65,7 +65,6 @@ type AlertsTableComponentProps = OwnProps & PropsFromRedux;
export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
timelineId,
canUserCRUD,
clearEventsDeleted,
clearEventsLoading,
clearSelected,
@ -74,6 +73,7 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
globalFilters,
globalQuery,
hasIndexWrite,
hasIndexMaintenance,
isSelectAllChecked,
loading,
loadingEventIds,
@ -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}
@ -275,8 +275,8 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
);
},
[
canUserCRUD,
hasIndexWrite,
hasIndexMaintenance,
clearSelectionCallback,
filterGroup,
showBuildingBlockAlerts,

View file

@ -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) {
@ -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>
@ -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>

View file

@ -37,6 +37,7 @@ describe('useUserInfo', () => {
canUserCRUD: null,
hasEncryptionKey: null,
hasIndexManage: null,
hasIndexMaintenance: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
isAuthenticated: null,

View file

@ -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;
@ -27,6 +28,7 @@ export interface State {
export const initialState: State = {
canUserCRUD: null,
hasIndexManage: null,
hasIndexMaintenance: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
isSignalIndexExists: null,
@ -43,6 +45,10 @@ export type Action =
type: 'updateHasIndexManage';
hasIndexManage: boolean | null;
}
| {
type: 'updateHasIndexMaintenance';
hasIndexMaintenance: boolean | null;
}
| {
type: 'updateHasIndexWrite';
hasIndexWrite: boolean | null;
@ -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,
@ -162,6 +174,7 @@ export const useUserInfo = (): State => {
{
canUserCRUD,
hasIndexManage,
hasIndexMaintenance,
hasIndexWrite,
hasIndexUpdateDelete,
isSignalIndexExists,
@ -178,6 +191,7 @@ export const useUserInfo = (): State => {
isAuthenticated: isApiAuthenticated,
hasEncryptionKey: isApiEncryptionKey,
hasIndexManage: hasApiIndexManage,
hasIndexMaintenance: hasApiIndexMaintenance,
hasIndexWrite: hasApiIndexWrite,
hasIndexUpdateDelete: hasApiIndexUpdateDelete,
} = usePrivilegeUser();
@ -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 &&
@ -298,6 +322,7 @@ export const useUserInfo = (): State => {
hasEncryptionKey,
canUserCRUD,
hasIndexManage,
hasIndexMaintenance,
hasIndexWrite,
hasIndexUpdateDelete,
signalIndexName,

View file

@ -1015,6 +1015,7 @@ export const mockUserPrivilege: Privilege = {
index: {
'.siem-signals-default': {
all: true,
maintenance: true,
manage_ilm: true,
read: true,
create_index: true,

View file

@ -79,6 +79,7 @@ export interface Privilege {
index: {
[indexName: string]: {
all: boolean;
maintenance: boolean;
manage_ilm: boolean;
read: boolean;
create_index: boolean;

View file

@ -20,6 +20,7 @@ describe('usePrivilegeUser', () => {
expect(result.current).toEqual({
hasEncryptionKey: null,
hasIndexManage: null,
hasIndexMaintenance: null,
hasIndexWrite: null,
hasIndexUpdateDelete: null,
isAuthenticated: null,
@ -38,6 +39,7 @@ describe('usePrivilegeUser', () => {
expect(result.current).toEqual({
hasEncryptionKey: true,
hasIndexManage: true,
hasIndexMaintenance: true,
hasIndexWrite: true,
hasIndexUpdateDelete: true,
isAuthenticated: true,
@ -60,6 +62,7 @@ describe('usePrivilegeUser', () => {
expect(result.current).toEqual({
hasEncryptionKey: false,
hasIndexManage: false,
hasIndexMaintenance: false,
hasIndexWrite: false,
hasIndexUpdateDelete: false,
isAuthenticated: false,

View file

@ -17,6 +17,7 @@ export interface ReturnPrivilegeUser {
hasIndexManage: boolean | null;
hasIndexWrite: boolean | null;
hasIndexUpdateDelete: boolean | null;
hasIndexMaintenance: boolean | null;
}
/**
* Hook to get user privilege from
@ -24,12 +25,23 @@ export interface ReturnPrivilegeUser {
*/
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();
@ -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 ||
@ -68,6 +81,7 @@ export const usePrivilegeUser = (): ReturnPrivilegeUser => {
hasIndexManage: false,
hasIndexWrite: false,
hasIndexUpdateDelete: false,
hasIndexMaintenance: false,
});
errorToToaster({ title: i18n.PRIVILEGE_FETCH_FAILURE, error, dispatchToaster });
}

View file

@ -75,9 +75,9 @@ const DetectionEnginePageComponent = () => {
isSignalIndexExists,
isAuthenticated: isUserAuthenticated,
hasEncryptionKey,
canUserCRUD,
signalIndexName,
hasIndexWrite,
hasIndexMaintenance,
},
] = useUserData();
const {
@ -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}

View file

@ -156,6 +156,7 @@ const RuleDetailsPageComponent = () => {
hasEncryptionKey,
canUserCRUD,
hasIndexWrite,
hasIndexMaintenance,
signalIndexName,
},
] = useUserData();
@ -591,9 +592,9 @@ const RuleDetailsPageComponent = () => {
{ruleId != null && (
<AlertsTable
timelineId={TimelineId.detectionsRulesDetailsPage}
canUserCRUD={canUserCRUD ?? false}
defaultFilters={alertDefaultFilters}
hasIndexWrite={hasIndexWrite ?? false}
hasIndexMaintenance={hasIndexMaintenance ?? false}
from={from}
loading={loading}
showBuildingBlockAlerts={showBuildingBlockAlerts}

View file

@ -53,6 +53,7 @@ export const readPrivileges = async (
'delete_index',
'index',
'manage',
'maintenance',
'manage_follow_index',
'manage_ilm',
'manage_leader_index',

View file

@ -14,7 +14,7 @@
"names": [
"*"
],
"privileges": ["read", "view_index_metadata"]
"privileges": ["read", "maintenance", "view_index_metadata"]
}
]
},

View file

@ -18,7 +18,7 @@
},
{
"names": [".siem-signals-*"],
"privileges": ["read", "write", "view_index_metadata"]
"privileges": ["read", "write", "maintenance", "view_index_metadata"]
}
]
},

View file

@ -2,7 +2,7 @@
"elasticsearch": {
"cluster": [],
"indices": [
{ "names": [".siem-signals-*"], "privileges": ["read", "write"] },
{ "names": [".siem-signals-*"], "privileges": ["read", "write", "maintenance"] },
{
"names": [
"apm-*-transaction*",

View file

@ -2,7 +2,7 @@
"elasticsearch": {
"cluster": [],
"indices": [
{ "names": [".siem-signals-*"], "privileges": ["read", "write"] },
{ "names": [".siem-signals-*"], "privileges": ["read", "write", "maintenance"] },
{
"names": [
".lists*",

View file

@ -166,7 +166,7 @@ export default ({ getService }: FtrProviderContext) => {
expect(everySignalClosed).to.eql(true);
});
it('should NOT be able to close signals with t1 analyst user', async () => {
it('should be able to close signals with t1 analyst user', async () => {
const rule = getRuleForSignalTesting(['auditbeat-*']);
const { id } = await createRule(supertest, rule);
await waitForRuleSuccessOrStatus(supertest, id);
@ -182,7 +182,7 @@ export default ({ getService }: FtrProviderContext) => {
.set('kbn-xsrf', 'true')
.auth(ROLES.t1_analyst, 'changeme')
.send(setSignalStatus({ signalIds, status: 'closed' }))
.expect(403);
.expect(200);
// query for the signals with the superuser
// to allow a check that the signals were NOT closed with t1 analyst
@ -199,7 +199,7 @@ export default ({ getService }: FtrProviderContext) => {
_source: {
signal: { status },
},
}) => status === 'open'
}) => status === 'closed'
);
expect(everySignalOpen).to.eql(true);
});