-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
task/mac-eventing-form #62999
task/mac-eventing-form #62999
Changes from 10 commits
e11e19f
43c6a10
3d9e34c
d9c77ac
c12e150
d413a05
9fdfe77
0e8aa04
817c4bb
bf65411
47ac630
2e636ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
import { PolicyDetailsState } from '../../types'; | ||
import { createStore, Dispatch, Store } from 'redux'; | ||
import { policyDetailsReducer, PolicyDetailsAction } from './index'; | ||
import { policyConfig, windowsEventing } from './selectors'; | ||
import { policyConfig } from './selectors'; | ||
import { clone } from '../../models/policy_details_config'; | ||
import { generatePolicy } from '../../models/policy'; | ||
|
||
|
@@ -72,7 +72,8 @@ describe('policy details: ', () => { | |
}); | ||
|
||
it('windows process eventing is enabled', async () => { | ||
expect(windowsEventing(getState())!.process).toEqual(true); | ||
const config = policyConfig(getState()); | ||
expect(config!.windows.events.process).toEqual(true); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @parkiino - Looks good! Will we also be adding a test here for the network eventing for Windows. This PR says mac eventing, are we also going to add tests for Mac here? Or did you want me to execute those manually for now until we can automate them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i can add a test for mac as well |
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
*/ | ||
|
||
import { Reducer } from 'redux'; | ||
import { PolicyData, PolicyDetailsState, UIPolicyConfig } from '../../types'; | ||
import { PolicyDetailsState, UIPolicyConfig } from '../../types'; | ||
import { AppAction } from '../action'; | ||
import { fullPolicy, isOnPolicyDetailsPage } from './selectors'; | ||
|
||
|
@@ -89,10 +89,12 @@ export const policyDetailsReducer: Reducer<PolicyDetailsState, AppAction> = ( | |
} | ||
|
||
if (action.type === 'userChangedPolicyConfig') { | ||
const newState = { ...state, policyItem: { ...(state.policyItem as PolicyData) } }; | ||
const newPolicy = (newState.policyItem.inputs[0].config.policy.value = { | ||
...fullPolicy(state), | ||
}); | ||
if (!state.policyItem) { | ||
return state; | ||
} | ||
const newState = { ...state, policyItem: { ...state.policyItem } }; | ||
const newPolicy: any = { ...fullPolicy(state) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of any can't this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for some reason typecheck complains on lines 99-102 because it doesn't like how windows has a different number of events (process, network) than mac and linux (process, network and file). so any resolves that for now |
||
newState.policyItem.inputs[0].config.policy.value = newPolicy; | ||
|
||
Object.entries(action.payload.policyConfig).forEach(([section, newSettings]) => { | ||
newPolicy[section as keyof UIPolicyConfig] = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,34 +118,21 @@ export interface PolicyDetailsState { | |
* Endpoint Policy configuration | ||
*/ | ||
export interface PolicyConfig { | ||
windows: { | ||
events: { | ||
process: boolean; | ||
network: boolean; | ||
}; | ||
/** malware mode can be off, detect, prevent or prevent and notify user */ | ||
malware: MalwareFields; | ||
windows: UIPolicyConfig['windows'] & { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious to know why this was needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying random stuff. Don't think the change is needed |
||
logging: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see you use this same format 3 times here, could be a candidate to abstract
|
||
stdout: string; | ||
file: string; | ||
}; | ||
advanced: PolicyConfigAdvancedOptions; | ||
}; | ||
mac: { | ||
events: { | ||
process: boolean; | ||
}; | ||
malware: MalwareFields; | ||
mac: UIPolicyConfig['mac'] & { | ||
logging: { | ||
stdout: string; | ||
file: string; | ||
}; | ||
advanced: PolicyConfigAdvancedOptions; | ||
}; | ||
linux: { | ||
events: { | ||
process: boolean; | ||
}; | ||
linux: UIPolicyConfig['linux'] & { | ||
logging: { | ||
stdout: string; | ||
file: string; | ||
|
@@ -168,29 +155,39 @@ interface PolicyConfigAdvancedOptions { | |
}; | ||
} | ||
|
||
/** | ||
* Windows-specific policy configuration that is supported via the UI | ||
*/ | ||
type WindowsPolicyConfig = Pick<PolicyConfig['windows'], 'events' | 'malware'>; | ||
|
||
/** | ||
* Mac-specific policy configuration that is supported via the UI | ||
*/ | ||
type MacPolicyConfig = Pick<PolicyConfig['mac'], 'malware' | 'events'>; | ||
|
||
/** | ||
* Linux-specific policy configuration that is supported via the UI | ||
*/ | ||
type LinuxPolicyConfig = Pick<PolicyConfig['linux'], 'events'>; | ||
|
||
/** | ||
* The set of Policy configuration settings that are show/edited via the UI | ||
*/ | ||
export interface UIPolicyConfig { | ||
windows: WindowsPolicyConfig; | ||
mac: MacPolicyConfig; | ||
linux: LinuxPolicyConfig; | ||
} | ||
/* eslint-disable @typescript-eslint/consistent-type-definitions */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why disable this here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed. was just trying random stuff because typescript was beating us up |
||
export type UIPolicyConfig = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you ever use these fields all together and not just selecting one type from the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes as of now, there is one form that changes malware protections for both windows and mac. so it grabs the entire policyconfig and returns back the changed malware protection for both mac and windows. There will probably be other protections like this in the future |
||
windows: { | ||
events: { | ||
process: boolean; | ||
network: boolean; | ||
}; | ||
/** malware mode can be off, detect, prevent or prevent and notify user */ | ||
malware: MalwareFields; | ||
}; | ||
mac: { | ||
events: { | ||
file: boolean; | ||
process: boolean; | ||
network: boolean; | ||
}; | ||
malware: MalwareFields; | ||
}; | ||
|
||
/** | ||
* Linux-specific policy configuration that is supported via the UI | ||
*/ | ||
linux: { | ||
events: { | ||
file: boolean; | ||
process: boolean; | ||
network: boolean; | ||
}; | ||
}; | ||
}; | ||
|
||
/** OS used in Policy */ | ||
export enum OS { | ||
|
@@ -203,6 +200,7 @@ export enum OS { | |
export enum EventingFields { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is used anymore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i believe you are right! |
||
process = 'process', | ||
network = 'network', | ||
file = 'file', | ||
} | ||
|
||
/** | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React, { useCallback, useMemo } from 'react'; | ||
import { EuiCheckbox } from '@elastic/eui'; | ||
import { useDispatch } from 'react-redux'; | ||
import { htmlIdGenerator } from '@elastic/eui'; | ||
import { usePolicyDetailsSelector } from '../../policy_hooks'; | ||
import { policyConfig } from '../../../../store/policy_details/selectors'; | ||
import { PolicyDetailsAction } from '../../../../store/policy_details'; | ||
import { UIPolicyConfig } from '../../../../types'; | ||
|
||
export const EventsCheckbox = React.memo(function({ | ||
name, | ||
setter, | ||
getter, | ||
}: { | ||
name: string; | ||
setter: (config: UIPolicyConfig, checked: boolean) => UIPolicyConfig; | ||
getter: (config: UIPolicyConfig) => boolean; | ||
}) { | ||
const policyDetailsConfig = usePolicyDetailsSelector(policyConfig); | ||
const selected = getter(policyDetailsConfig); | ||
const dispatch = useDispatch<(action: PolicyDetailsAction) => void>(); | ||
|
||
const handleCheckboxChange = useCallback( | ||
(event: React.ChangeEvent<HTMLInputElement>) => { | ||
if (policyDetailsConfig) { | ||
dispatch({ | ||
type: 'userChangedPolicyConfig', | ||
payload: { policyConfig: setter(policyDetailsConfig, event.target.checked) }, | ||
}); | ||
} | ||
}, | ||
[dispatch, policyDetailsConfig, setter] | ||
); | ||
|
||
return ( | ||
<EuiCheckbox | ||
id={useMemo(() => htmlIdGenerator()(), [])} | ||
label={name} | ||
checked={selected} | ||
onChange={handleCheckboxChange} | ||
/> | ||
); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
export { WindowsEvents } from './windows'; | ||
export { MacEvents } from './mac'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works to keep us type compliant when we have different combinations of keys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this was to make sure the types were still getting passed through correctly