[Security Solution][Endpoint][Admin] Malware user notification checkbox #78084

parkiino · 2020-09-21T20:46:53Z

Summary

adds checkbox for Endpoint policy malware user notification
migration for new malware user notification field
migrated policy does not automatically have user notification checked off even if prevent is selected
if a user makes a new policy, user notification on is the default
migration unit test

Issue: https://github.com/elastic/security-team/issues/220

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-notification

elasticmachine · 2020-10-15T15:56:14Z

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

elasticmachine · 2020-10-15T15:56:14Z

Pinging @elastic/endpoint-management (Team:Endpoint Management)

elasticmachine · 2020-10-15T15:59:10Z

Pinging @elastic/ingest-management (Team:Ingest Management)

jfsiii · 2020-10-15T16:05:53Z

x-pack/plugins/ingest_manager/server/saved_objects/index.ts

@@ -26,6 +26,7 @@ import {
  migrateSettingsToV7100,
  migrateAgentActionToV7100,
 } from './migrations/to_v7_10_0';
+import { migratePackagePolicyToV7110 } from './migrations/to_v7_11_0';


Can this reside and be run outside Fleet? e.g. inside security?

It seems limited to Endpoint and I'd like to limit how much Fleet "knows" about the specifics of a package

we could store store this file outside of Fleet, but doesn't the SO migration have to run as a part of the SO mapping definition? Or could we import a mapping in security and then add an additional migration?

EDIT:
agreed on the Endpoint-only nature, happy to look at other alternatives

kevinlog · 2020-10-15T22:09:27Z

x-pack/plugins/security_solution/server/endpoint/lib/policy/saved_object_mappings.ts

+import { PACKAGE_POLICY_SAVED_OBJECT_TYPE } from '../../../../../ingest_manager/common';
+import { migratePackagePolicyToV7110 } from './to_v7_11.0';
+
+export const packagePolicyType: SavedObjectsType = {


is it a problem that we define this here as well as in Ingest? If it is OK, should we import the mapping, clone it, and add our own migration? What happens if there are 2 migrations for 7.11? One in Ingest and one in Security?

cc @jfsiii @jen-huang

judging from the test failures, it certainly looks like this is an issue

https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/82038/execution/node/636/log/

hmmm so then should i just have the code for the migration in endpoint, but then add our migration line to the one in ingest for package policy saved objects then?

hmmm so then should i just have the code for the migration in endpoint, but then add our migration line to the one in ingest for package policy saved objects then?

This seems like what needs to happen. We can't define a duplicate mapping for an existing SO. Unless there's a way that we import the SO mapping from ingest in our app and simply add another migration to it.

…-notification

jfsiii

👍 🚀 Thanks for moving the migration to security_solution

jen-huang

I would like to see another test case added but rest of migration code LGTM

jen-huang · 2020-10-20T16:25:21Z

x-pack/plugins/security_solution/common/endpoint/policy/migrations/to_v7_11.0.test.ts

+import { PackagePolicy } from '../../../../../ingest_manager/common';
+import { migratePackagePolicyToV7110 } from './to_v7_11.0';
+
+describe('7.11.0 Endpoint Package Policy migration', () => {


could you add another test case that ensures package policies are not modified for non-endpoint package policies?

yesss will do!

paul-tavares

Some minor comments, but I'm good if you merge

paul-tavares · 2020-10-20T18:56:09Z

x-pack/plugins/security_solution/common/endpoint/policy/migrations/to_v7_11.0.test.ts

+describe('7.11.0 Endpoint Package Policy migration', () => {
+  const migration = migratePackagePolicyToV7110;
+  it('adds malware notification checkbox and optional message', () => {
+    const doc = {


maybe add a Type to doc? so that in the future the structure changes, we're reminded (error) to come revisit this.

paul-tavares · 2020-10-20T18:58:25Z

x-pack/plugins/security_solution/common/endpoint/policy/migrations/to_v7_11.0.ts

+import { cloneDeep } from 'lodash';
+import { PackagePolicy } from '../../../../../ingest_manager/common';
+
+export const migratePackagePolicyToV7110: SavedObjectMigrationFn<PackagePolicy, PackagePolicy> = (


This is only used by server code correct?
perhaps it would be more appropriate to store it under ./server directory and have it exposed via ./server/index?

@nnamdifrankie what do you think? ⬆️

jen-huang · 2020-10-20T20:28:59Z

x-pack/plugins/security_solution/common/endpoint/policy/migrations/to_v7_11.0.test.ts

+          },
+        ],
+      },
+      type: ' nested',


small spacing typo here and on the rest of the type: lines in this file

kibanamachine · 2020-10-20T21:12:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6b1801c

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`securitySolution`	2057	2058	+1

async chunks size

id	before	after	diff
`securitySolution`	8.1MB	8.1MB	+2.0KB

distributable file count

id	before	after	diff
`default`	48035	48036	+1

page load bundle size

id	before	after	diff
`securitySolution`	262.0KB	262.8KB	+813.0B

History

💚 Build #82625 succeeded aee43b3
💔 Build #82603 failed a7add0d
💚 Build #82505 succeeded 32de69d
💔 Build #82498 failed c195a63
💔 Build #82038 failed 8bcd5b9

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…ox (#78084) (#81271) includes migration for endpoint package policy 7.11

…arm-phase-to-formlib * 'master' of github.com:elastic/kibana: (55 commits) [UX] Fix map color variance and apply proper filter for extended stats (elastic#81106) [User Experience] Use EuiSelect for percentiles instead of SuperSelect (elastic#81082) [DOCS] Add link for monitoring ssl settings (elastic#81057) [test] Await loading indicator in monitoring test (elastic#81279) [ILM] Minor copy and link additions to cloud CTA for cold phase (elastic#80512) [Mappings editor] Add scaled_float and date_range comp integration tests (elastic#81287) [Discover] Deangularize context.app (elastic#80851) [O11y Overview] Add code to display/hide UX section when appropriate (elastic#80873) [Discover] Extend DiscoverNoResults component to show different message on error (elastic#79671) Fix tagcloud word overlapping (elastic#81161) [Security Solution] Fixes flaky test rules (elastic#81040) Changed the code to avoid tech debt with hacky solutions after receiving comments on EUI issue reported about this problem. (elastic#81183) [Security Solution][All] Replace old markdown renderer with the new one (elastic#80301) Add namespaced version of the API call (elastic#81278) [ML] Data Frame Analytics: Fix race condition and support for feature influence legacy format. (elastic#81123) [Fleet] Fix POLICY_CHANGE action creation for new policy (elastic#81236) [Security Solution][Endpoint][Admin] Malware user notification checkbox (elastic#78084) [SecuritySolution][Unit Tests] - fix flakey unit test (elastic#81239) skip flaky suite (elastic#81264) [Maps] fix top-level Map page is called 'Kibana' (elastic#81238) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/forcemerge_field.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase.tsx

parkiino added 10 commits September 21, 2020 16:43

basic user notification checkbox

4267f8c

Merge remote-tracking branch 'upstream/master' into task/malware-user…

72a4be0

…-notification

Merge remote-tracking branch 'upstream/master' into task/malware-user…

c9af419

…-notification

Merge remote-tracking branch 'upstream/master' into task/malware-user…

bc20a81

…-notification

working migration, user notification tied to prevent or detect

b9ce8c1

fix existing jest test

f652794

fixed existing functional test

6d5c35e

add mac migration

1c1f11a

migration unit test

0cd6e0a

Merge remote-tracking branch 'upstream/master' into task/malware-user…

872fcf3

…-notification

parkiino marked this pull request as ready for review October 15, 2020 15:55

parkiino requested a review from a team as a code owner October 15, 2020 15:55

parkiino requested a review from a team October 15, 2020 15:55

parkiino requested a review from a team as a code owner October 15, 2020 15:55

parkiino added Feature:Endpoint Elastic Endpoint feature release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Management v7.11.0 v8.0.0 labels Oct 15, 2020

add mac stuff

190ecac

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Oct 15, 2020

jfsiii reviewed Oct 15, 2020

View reviewed changes

move migration from ingest manager to endpoint

8bcd5b9

kevinlog reviewed Oct 15, 2020

View reviewed changes

parkiino added 3 commits October 19, 2020 12:03

moving to common folder

c195a63

didn't even push the actual changes...

dad57cc

Merge remote-tracking branch 'upstream/master' into task/malware-user…

32de69d

…-notification

parkiino added 3 commits October 19, 2020 16:21

migration works for sure now

57b8993

completed ui

a7add0d

fix test

aee43b3

jfsiii self-requested a review October 20, 2020 14:42

jfsiii approved these changes Oct 20, 2020

View reviewed changes

jen-huang approved these changes Oct 20, 2020

View reviewed changes

kevinlog approved these changes Oct 20, 2020

View reviewed changes

paul-tavares approved these changes Oct 20, 2020

View reviewed changes

parkiino added 2 commits October 20, 2020 15:22

2nd migration test

ee04d14

add type to test variable

6b1801c

jen-huang reviewed Oct 20, 2020

View reviewed changes

parkiino merged commit b4864ab into elastic:master Oct 21, 2020

parkiino deleted the task/malware-user-notification branch October 21, 2020 02:42

parkiino mentioned this pull request Oct 21, 2020

[7.x] [Security Solution][Endpoint][Admin] Malware user notification checkbox (#78084) #81271

Merged

parkiino added a commit that referenced this pull request Oct 21, 2020

[Security Solution][Endpoint][Admin] Malware user notification checkb…

5d3e7c9

…ox (#78084) (#81271) includes migration for endpoint package policy 7.11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Endpoint][Admin] Malware user notification checkbox #78084

[Security Solution][Endpoint][Admin] Malware user notification checkbox #78084

parkiino commented Sep 21, 2020 •

edited

Loading

elasticmachine commented Oct 15, 2020

elasticmachine commented Oct 15, 2020

elasticmachine commented Oct 15, 2020

jfsiii Oct 15, 2020

kevinlog Oct 15, 2020 •

edited

Loading

kevinlog Oct 15, 2020

kevinlog Oct 15, 2020

parkiino Oct 16, 2020

kevinlog Oct 19, 2020

jfsiii left a comment

jen-huang left a comment

jen-huang Oct 20, 2020

parkiino Oct 20, 2020

paul-tavares left a comment

paul-tavares Oct 20, 2020

paul-tavares Oct 20, 2020

jen-huang Oct 20, 2020

kibanamachine commented Oct 20, 2020

[Security Solution][Endpoint][Admin] Malware user notification checkbox #78084

[Security Solution][Endpoint][Admin] Malware user notification checkbox #78084

Conversation

parkiino commented Sep 21, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Oct 15, 2020

elasticmachine commented Oct 15, 2020

elasticmachine commented Oct 15, 2020

Choose a reason for hiding this comment

kevinlog Oct 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfsiii left a comment

Choose a reason for hiding this comment

jen-huang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Oct 20, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

distributable file count

page load bundle size

History

parkiino commented Sep 21, 2020 •

edited

Loading

kevinlog Oct 15, 2020 •

edited

Loading