Skip to content

Commit

Permalink
[7.7] [SIEM] [Detection Engine] Remove has manage api keys req… (#62535)
Browse files Browse the repository at this point in the history
Alerting no longer requires the manage_api_keys privilege, so we are removing it from the detection engine code. Fixes #62387

* removes hasManageApiKeys since alerting is using the internal user api calls, manage_api_keys privilege is no longer necessary

* linting error

* fixes types and removes a test for manage api keys

* removes manage api key reducer and updates leftover tests

* moves userHasNoPermissions repeated code into a function in helpers, adds a few test cases, updated references to new function

* fix test title

* remove userHasNoPermissions function and remove tests, replace with just not canUserCRUD

* Revert "remove userHasNoPermissions function and remove tests, replace with just not canUserCRUD"

This reverts commit 93912e7.

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
dhurley14 and elasticmachine authored Apr 4, 2020
1 parent 22cb4e7 commit 868cf86
Show file tree
Hide file tree
Showing 13 changed files with 51 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: null,
hasIndexWrite: null,
hasManageApiKey: null,
isAuthenticated: null,
hasEncryptionKey: null,
isSignalIndexExists: null,
Expand Down Expand Up @@ -50,7 +49,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: null,
hasIndexWrite: null,
hasManageApiKey: null,
isAuthenticated: null,
hasEncryptionKey: null,
isSignalIndexExists: null,
Expand Down Expand Up @@ -79,7 +77,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: true,
hasEncryptionKey: true,
isSignalIndexExists: true,
Expand Down Expand Up @@ -116,7 +113,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: true,
hasEncryptionKey: true,
isSignalIndexExists: true,
Expand All @@ -139,7 +135,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: false,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: true,
hasEncryptionKey: true,
isSignalIndexExists: true,
Expand All @@ -161,29 +156,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: false,
hasManageApiKey: true,
isAuthenticated: true,
hasEncryptionKey: true,
isSignalIndexExists: true,
})
);
await waitForNextUpdate();
await waitForNextUpdate();
let resp = null;
if (result.current.createPrePackagedRules) {
resp = await result.current.createPrePackagedRules();
}
expect(resp).toEqual(false);
});
});

test('can NOT createPrePackagedRules because hasManageApiKey === false', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<unknown, ReturnPrePackagedRules>(() =>
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: true,
hasManageApiKey: false,
isAuthenticated: true,
hasEncryptionKey: true,
isSignalIndexExists: true,
Expand All @@ -205,7 +177,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: false,
hasEncryptionKey: true,
isSignalIndexExists: true,
Expand All @@ -227,7 +198,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: true,
hasEncryptionKey: false,
isSignalIndexExists: true,
Expand All @@ -249,7 +219,6 @@ describe('usePersistRule', () => {
usePrePackagedRules({
canUserCRUD: true,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: true,
hasEncryptionKey: true,
isSignalIndexExists: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export interface ReturnPrePackagedRules {
interface UsePrePackagedRuleProps {
canUserCRUD: boolean | null;
hasIndexWrite: boolean | null;
hasManageApiKey: boolean | null;
isAuthenticated: boolean | null;
hasEncryptionKey: boolean | null;
isSignalIndexExists: boolean | null;
Expand All @@ -36,7 +35,6 @@ interface UsePrePackagedRuleProps {
* Hook for using to get status about pre-packaged Rules from the Detection Engine API
*
* @param hasIndexWrite boolean
* @param hasManageApiKey boolean
* @param isAuthenticated boolean
* @param hasEncryptionKey boolean
* @param isSignalIndexExists boolean
Expand All @@ -45,7 +43,6 @@ interface UsePrePackagedRuleProps {
export const usePrePackagedRules = ({
canUserCRUD,
hasIndexWrite,
hasManageApiKey,
isAuthenticated,
hasEncryptionKey,
isSignalIndexExists,
Expand Down Expand Up @@ -117,7 +114,6 @@ export const usePrePackagedRules = ({
if (
canUserCRUD &&
hasIndexWrite &&
hasManageApiKey &&
isAuthenticated &&
hasEncryptionKey &&
isSignalIndexExists
Expand Down Expand Up @@ -185,14 +181,7 @@ export const usePrePackagedRules = ({
isSubscribed = false;
abortCtrl.abort();
};
}, [
canUserCRUD,
hasIndexWrite,
hasManageApiKey,
isAuthenticated,
hasEncryptionKey,
isSignalIndexExists,
]);
}, [canUserCRUD, hasIndexWrite, isAuthenticated, hasEncryptionKey, isSignalIndexExists]);

return {
loading,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,6 @@ export const mockUserPrivilege: Privilege = {
monitor_watcher: true,
monitor_transform: true,
read_ilm: true,
manage_api_key: true,
manage_security: true,
manage_own_api_key: false,
manage_saml: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export interface Privilege {
monitor_watcher: boolean;
monitor_transform: boolean;
read_ilm: boolean;
manage_api_key: boolean;
manage_security: boolean;
manage_own_api_key: boolean;
manage_saml: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ describe('usePrivilegeUser', () => {
hasEncryptionKey: null,
hasIndexManage: null,
hasIndexWrite: null,
hasManageApiKey: null,
isAuthenticated: null,
loading: true,
});
Expand All @@ -39,7 +38,6 @@ describe('usePrivilegeUser', () => {
hasEncryptionKey: true,
hasIndexManage: true,
hasIndexWrite: true,
hasManageApiKey: true,
isAuthenticated: true,
loading: false,
});
Expand All @@ -61,7 +59,6 @@ describe('usePrivilegeUser', () => {
hasEncryptionKey: false,
hasIndexManage: false,
hasIndexWrite: false,
hasManageApiKey: false,
isAuthenticated: false,
loading: false,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export interface ReturnPrivilegeUser {
isAuthenticated: boolean | null;
hasEncryptionKey: boolean | null;
hasIndexManage: boolean | null;
hasManageApiKey: boolean | null;
hasIndexWrite: boolean | null;
}
/**
Expand All @@ -27,17 +26,12 @@ export const usePrivilegeUser = (): ReturnPrivilegeUser => {
const [privilegeUser, setPrivilegeUser] = useState<
Pick<
ReturnPrivilegeUser,
| 'isAuthenticated'
| 'hasEncryptionKey'
| 'hasIndexManage'
| 'hasManageApiKey'
| 'hasIndexWrite'
'isAuthenticated' | 'hasEncryptionKey' | 'hasIndexManage' | 'hasIndexWrite'
>
>({
isAuthenticated: null,
hasEncryptionKey: null,
hasIndexManage: null,
hasManageApiKey: null,
hasIndexWrite: null,
});
const [, dispatchToaster] = useStateToaster();
Expand Down Expand Up @@ -65,10 +59,6 @@ export const usePrivilegeUser = (): ReturnPrivilegeUser => {
privilege.index[indexName].create_doc ||
privilege.index[indexName].index ||
privilege.index[indexName].write,
hasManageApiKey:
privilege.cluster.manage_security ||
privilege.cluster.manage_api_key ||
privilege.cluster.manage_own_api_key,
});
}
}
Expand All @@ -78,7 +68,6 @@ export const usePrivilegeUser = (): ReturnPrivilegeUser => {
isAuthenticated: false,
hasEncryptionKey: false,
hasIndexManage: false,
hasManageApiKey: false,
hasIndexWrite: false,
});
errorToToaster({ title: i18n.PRIVILEGE_FETCH_FAILURE, error, dispatchToaster });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export interface State {
canUserCRUD: boolean | null;
hasIndexManage: boolean | null;
hasIndexWrite: boolean | null;
hasManageApiKey: boolean | null;
isSignalIndexExists: boolean | null;
isAuthenticated: boolean | null;
hasEncryptionKey: boolean | null;
Expand All @@ -27,7 +26,6 @@ const initialState: State = {
canUserCRUD: null,
hasIndexManage: null,
hasIndexWrite: null,
hasManageApiKey: null,
isSignalIndexExists: null,
isAuthenticated: null,
hasEncryptionKey: null,
Expand All @@ -37,10 +35,6 @@ const initialState: State = {

export type Action =
| { type: 'updateLoading'; loading: boolean }
| {
type: 'updateHasManageApiKey';
hasManageApiKey: boolean | null;
}
| {
type: 'updateHasIndexManage';
hasIndexManage: boolean | null;
Expand Down Expand Up @@ -90,12 +84,6 @@ export const userInfoReducer = (state: State, action: Action): State => {
hasIndexWrite: action.hasIndexWrite,
};
}
case 'updateHasManageApiKey': {
return {
...state,
hasManageApiKey: action.hasManageApiKey,
};
}
case 'updateIsSignalIndexExists': {
return {
...state,
Expand Down Expand Up @@ -151,7 +139,6 @@ export const useUserInfo = (): State => {
canUserCRUD,
hasIndexManage,
hasIndexWrite,
hasManageApiKey,
isSignalIndexExists,
isAuthenticated,
hasEncryptionKey,
Expand All @@ -166,7 +153,6 @@ export const useUserInfo = (): State => {
hasEncryptionKey: isApiEncryptionKey,
hasIndexManage: hasApiIndexManage,
hasIndexWrite: hasApiIndexWrite,
hasManageApiKey: hasApiManageApiKey,
} = usePrivilegeUser();
const {
loading: indexNameLoading,
Expand Down Expand Up @@ -197,12 +183,6 @@ export const useUserInfo = (): State => {
}
}, [loading, hasIndexWrite, hasApiIndexWrite]);

useEffect(() => {
if (!loading && hasManageApiKey !== hasApiManageApiKey && hasApiManageApiKey != null) {
dispatch({ type: 'updateHasManageApiKey', hasManageApiKey: hasApiManageApiKey });
}
}, [loading, hasManageApiKey, hasApiManageApiKey]);

useEffect(() => {
if (
!loading &&
Expand Down Expand Up @@ -258,7 +238,6 @@ export const useUserInfo = (): State => {
canUserCRUD,
hasIndexManage,
hasIndexWrite,
hasManageApiKey,
signalIndexName,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { StepScheduleRule } from '../components/step_schedule_rule';
import { StepRuleActions } from '../components/step_rule_actions';
import { DetectionEngineHeaderPage } from '../../components/detection_engine_header_page';
import * as RuleI18n from '../translations';
import { redirectToDetections, getActionMessageParams } from '../helpers';
import { redirectToDetections, getActionMessageParams, userHasNoPermissions } from '../helpers';
import {
AboutStepRule,
DefineStepRule,
Expand Down Expand Up @@ -85,7 +85,6 @@ const CreateRulePageComponent: React.FC = () => {
isAuthenticated,
hasEncryptionKey,
canUserCRUD,
hasManageApiKey,
} = useUserInfo();
const [, dispatchToaster] = useStateToaster();
const [openAccordionId, setOpenAccordionId] = useState<RuleStep>(RuleStep.defineRule);
Expand Down Expand Up @@ -117,8 +116,6 @@ const CreateRulePageComponent: React.FC = () => {
getActionMessageParams((stepsData.current['define-rule'].data as DefineStepRule).ruleType),
[stepsData.current['define-rule'].data]
);
const userHasNoPermissions =
canUserCRUD != null && hasManageApiKey != null ? !canUserCRUD || !hasManageApiKey : false;

const setStepData = useCallback(
(step: RuleStep, data: unknown, isValid: boolean) => {
Expand Down Expand Up @@ -274,7 +271,7 @@ const CreateRulePageComponent: React.FC = () => {

if (redirectToDetections(isSignalIndexExists, isAuthenticated, hasEncryptionKey)) {
return <Redirect to={`/${DETECTION_ENGINE_PAGE_NAME}`} />;
} else if (userHasNoPermissions) {
} else if (userHasNoPermissions(canUserCRUD)) {
return <Redirect to={`/${DETECTION_ENGINE_PAGE_NAME}/rules`} />;
}

Expand Down
Loading

0 comments on commit 868cf86

Please sign in to comment.