Skip to content
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

Btsymbala/registered av #81910

Merged
merged 23 commits into from
Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
12f6191
Moved out type for OperatingSystem and moved OS translations one leve…
efreeti Oct 26, 2020
0501c1d
Changed the translation to be consistent between trusted apps and pol…
efreeti Oct 28, 2020
56f8f44
Unified translations of OS types between trusted apps and policy.
efreeti Oct 28, 2020
7e216d7
Removed unused types.
efreeti Oct 28, 2020
0b4a7eb
Added registered AV form section.
efreeti Oct 28, 2020
62db297
Changed the property structure to match the format expected by endpoint.
efreeti Oct 30, 2020
509aace
Fixed the visual alignment of titles in the form and added responsive…
efreeti Oct 30, 2020
25d0d48
Updated snapshots.
efreeti Oct 30, 2020
4734757
Moved out type for OperatingSystem and moved OS translations one leve…
efreeti Oct 30, 2020
fbff194
Added config form heading component.
efreeti Oct 30, 2020
ecfc5b3
Added config form heading component.
efreeti Oct 30, 2020
7a1d007
Cleaned up translations.
efreeti Nov 2, 2020
ee19c19
Merge branch 'master' of https:/elastic/kibana into btsym…
efreeti Nov 2, 2020
7289449
Fixed type error with initialization.
efreeti Nov 2, 2020
8a07958
Fixed error in trusted app creation form test.
efreeti Nov 2, 2020
9e1fed6
Removed the guard for now in favour of better initialization.
efreeti Nov 2, 2020
623a91b
Fixed the store test.
efreeti Nov 2, 2020
df503b5
Fixing functional test data.
efreeti Nov 2, 2020
15c0265
Added functional test config option to account for a custom header wi…
efreeti Nov 2, 2020
98360a8
Merge branch 'master' of https:/elastic/kibana into btsym…
efreeti Nov 4, 2020
41b9750
Merge branch 'master' of https:/elastic/kibana into btsym…
efreeti Nov 4, 2020
f1b02f3
Merge branch 'master' of https:/elastic/kibana into btsym…
efreeti Nov 9, 2020
773204f
Merge branch 'master' into btsymbala/registered-av
kibanamachine Nov 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export const factory = (): PolicyConfig => {
logging: {
file: 'info',
},
antivirus_registration: {
enabled: false,
},
},
mac: {
events: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ApplicationStart } from 'kibana/public';
import { NewPackagePolicy, PackagePolicy } from '../../../../ingest_manager/common';
import { ManifestSchema } from '../schema/manifest';

export * from './os';
export * from './trusted_apps';

/**
Expand Down Expand Up @@ -880,6 +881,9 @@ export interface PolicyConfig {
enabled: boolean;
};
};
antivirus_registration: {
enabled: boolean;
};
};
mac: {
advanced?: {};
Expand Down Expand Up @@ -919,7 +923,10 @@ export interface UIPolicyConfig {
/**
* Windows-specific policy configuration that is supported via the UI
*/
windows: Pick<PolicyConfig['windows'], 'events' | 'malware' | 'popup' | 'advanced'>;
windows: Pick<
PolicyConfig['windows'],
'events' | 'malware' | 'popup' | 'antivirus_registration' | 'advanced'
>;
/**
* Mac-specific policy configuration that is supported via the UI
*/
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/security_solution/common/endpoint/types/os.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* 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 type Linux = 'linux';
export type MacOS = 'macos';
export type Windows = 'windows';
export type OperatingSystem = Linux | MacOS | Windows;
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
GetTrustedAppsRequestSchema,
PostTrustedAppCreateRequestSchema,
} from '../schema/trusted_apps';
import { Linux, MacOS, Windows } from './os';

/** API request params for deleting Trusted App entry */
export type DeleteTrustedAppsRequestParams = TypeOf<typeof DeleteTrustedAppsRequestSchema.params>;
Expand Down Expand Up @@ -51,11 +52,11 @@ export type NewTrustedApp = {
description?: string;
} & (
| {
os: 'linux' | 'macos';
os: Linux | MacOS;
entries: MacosLinuxConditionEntry[];
}
| {
os: 'windows';
os: Windows;
entries: WindowsConditionEntry[];
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import { i18n } from '@kbn/i18n';

import { OperatingSystem } from '../../../common/endpoint/types';

export const ENDPOINTS_TAB = i18n.translate('xpack.securitySolution.endpointsTab', {
defaultMessage: 'Endpoints',
});
Expand All @@ -21,3 +23,15 @@ export const TRUSTED_APPS_TAB = i18n.translate('xpack.securitySolution.trustedAp
export const BETA_BADGE_LABEL = i18n.translate('xpack.securitySolution.administration.list.beta', {
defaultMessage: 'Beta',
});

export const OS_TITLES: Readonly<{ [K in OperatingSystem]: string }> = {
windows: i18n.translate('xpack.securitySolution.administration.os.windows', {
defaultMessage: 'Windows',
}),
macos: i18n.translate('xpack.securitySolution.administration.os.macos', {
defaultMessage: 'Mac',
}),
linux: i18n.translate('xpack.securitySolution.administration.os.linux', {
defaultMessage: 'Linux',
}),
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ interface UserChangedPolicyConfig {
};
}

interface UserChangedAntivirusRegistration {
type: 'userChangedAntivirusRegistration';
payload: {
enabled: boolean;
};
}

interface ServerReturnedPolicyDetailsAgentSummaryData {
type: 'serverReturnedPolicyDetailsAgentSummaryData';
payload: {
Expand Down Expand Up @@ -62,4 +69,5 @@ export type PolicyDetailsAction =
| ServerReturnedPolicyDetailsUpdateFailure
| ServerReturnedUpdatedPolicyDetailsData
| ServerFailedToReturnPolicyDetailsData
| UserChangedPolicyConfig;
| UserChangedPolicyConfig
| UserChangedAntivirusRegistration;
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ describe('policy details: ', () => {
},
},
logging: { file: 'info' },
antivirus_registration: {
enabled: false,
},
},
mac: {
events: { process: true, file: true, network: true },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,33 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { fullPolicy, isOnPolicyDetailsPage } from './selectors';
import { Immutable, PolicyConfig, UIPolicyConfig } from '../../../../../../common/endpoint/types';
import {
Immutable,
PolicyConfig,
UIPolicyConfig,
PolicyData,
} from '../../../../../../common/endpoint/types';
import { ImmutableReducer } from '../../../../../common/store';
import { AppAction } from '../../../../../common/store/actions';
import { PolicyDetailsState } from '../../types';

const updatePolicyConfigInPolicyData = (
policyData: Immutable<PolicyData>,
policyConfig: Immutable<PolicyConfig>
) => ({
...policyData,
inputs: policyData.inputs.map((input) => ({
...input,
config: input.config && {
...input.config,
policy: {
...input.config.policy,
value: policyConfig,
},
},
})),
});

/**
* Return a fresh copy of initial state, since we mutate state in the reducer.
*/
Expand Down Expand Up @@ -126,5 +148,26 @@ export const policyDetailsReducer: ImmutableReducer<PolicyDetailsState, AppActio
return newState;
}

if (action.type === 'userChangedAntivirusRegistration') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you created a separate action for antivirus registration specifically? If I am reading this right, it seems like the antivirus_registration is just another key within the policy config. The action userChangedPolicyConfig should be able to do the same thing. ex) https:/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_forms/protections/malware.tsx#L53

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will bring this up during next week office hours. General thought was to avoid components being too smart and knowing how to update properties in policy config and rather keep the logic out (specifically move it to redux). What are your thoughts on this?

I noticed that due to this logic living in component we clone policy twice and have some ts-ignore directives because the code violates immutability.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh okie, i can see the value in moving logic into redux, i guess im just wondering if its possible to make it more robust since i feel like policy details will only get bigger in the future?

if (state.policyItem) {
const policyConfig = fullPolicy(state);

return {
...state,
policyItem: updatePolicyConfigInPolicyData(state.policyItem, {
...policyConfig,
windows: {
...policyConfig.windows,
antivirus_registration: {
enabled: action.payload.enabled,
},
},
}),
};
} else {
return state;
}
}

return state;
};
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export const policyConfig: (s: PolicyDetailsState) => UIPolicyConfig = createSel
events: windows.events,
malware: windows.malware,
popup: windows.popup,
antivirus_registration: windows.antivirus_registration,
},
mac: {
advanced: mac.advanced,
Expand All @@ -122,6 +123,10 @@ export const policyConfig: (s: PolicyDetailsState) => UIPolicyConfig = createSel
}
);

export const isAntivirusRegistrationEnabled = createSelector(policyConfig, (uiPolicyConfig) => {
return uiPolicyConfig.windows.antivirus_registration.enabled;
});

/** Returns the total number of possible windows eventing configurations */
export const totalWindowsEvents = (state: PolicyDetailsState): number => {
const config = policyConfig(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,68 +76,6 @@ export interface PolicyListUrlSearchParams {
page_size: number;
}

/**
* Endpoint Policy configuration
*/
export interface PolicyConfig {
windows: {
events: {
dll_and_driver_load: boolean;
dns: boolean;
file: boolean;
network: boolean;
process: boolean;
registry: boolean;
security: boolean;
};
malware: MalwareFields;
logging: {
stdout: string;
file: string;
};
advanced: PolicyConfigAdvancedOptions;
};
mac: {
events: {
file: boolean;
process: boolean;
network: boolean;
};
malware: MalwareFields;
logging: {
stdout: string;
file: string;
};
advanced: PolicyConfigAdvancedOptions;
};
linux: {
events: {
file: boolean;
process: boolean;
network: boolean;
};
logging: {
stdout: string;
file: string;
};
advanced: PolicyConfigAdvancedOptions;
};
}

interface PolicyConfigAdvancedOptions {
elasticsearch: {
indices: {
control: string;
event: string;
logging: string;
};
kernel: {
connect: boolean;
process: boolean;
};
};
}

export enum OS {
windows = 'windows',
mac = 'mac',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* 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 from 'react';
import { ThemeProvider } from 'styled-components';
import { storiesOf, addDecorator } from '@storybook/react';
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
import { EuiCheckbox, EuiSpacer, EuiSwitch, EuiText } from '@elastic/eui';

import { ConfigForm } from '.';

addDecorator((storyFn) => (
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>{storyFn()}</ThemeProvider>
));

storiesOf('PolicyDetails/ConfigForm', module)
.add('One OS', () => {
return (
<ConfigForm type="Type 1" supportedOss={['windows']}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use the os enums you made here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you refer to os.ts file then it contains only type definitions, unfortunately literal types can not be used as keys/symbols directly. I plan to later revisit this and add some constant to keep the keys.

{'Some content'}
</ConfigForm>
);
})
.add('Multiple OSs', () => {
return (
<ConfigForm type="Type 1" supportedOss={['windows', 'macos', 'linux']}>
{'Some content'}
</ConfigForm>
);
})
.add('Complex content', () => {
return (
<ConfigForm type="Type 1" supportedOss={['macos', 'linux']}>
<EuiText>
{'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore ' +
'et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut ' +
'aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum ' +
'dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia ' +
'deserunt mollit anim id est laborum.'}
</EuiText>
<EuiSpacer size="s" />
<EuiSwitch label={'Switch'} checked={true} onChange={() => {}} />
<EuiSpacer size="s" />
<EuiCheckbox id="1" label={'Checkbox 1'} checked={false} onChange={() => {}} />
<EuiCheckbox id="2" label={'Checkbox 2'} checked={true} onChange={() => {}} />
<EuiCheckbox id="3" label={'Checkbox 3'} checked={true} onChange={() => {}} />
</ConfigForm>
);
})
.add('Right corner content', () => {
const toggle = <EuiSwitch label={'Switch'} checked={true} onChange={() => {}} />;

return (
<ConfigForm type="Type 1" supportedOss={['linux']} rightCorner={toggle}>
{'Some content'}
</ConfigForm>
);
});
Loading