[Security Solution][Endpoint] Allow wildcard in trusted app paths #97623

ashokaditya · 2021-04-20T12:25:06Z

Summary

Adds a dropdown for the path field to enable adding wildcards as well as adding specific paths for creating trusted apps.
See elastic/security-team/issues/543 for more details.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

refs elastic/security-team/issues/543

x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts

refs elastic/security-team/issues/543

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx

dasansol92 · 2021-04-20T16:17:32Z

Few comments added but it looks awesome! 🔥 🔥

refs elastic/security-team/issues/543

elasticmachine · 2021-04-21T07:47:21Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

refs elastic/security-team/issues/543

x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts

refs 1deab39 refs elastic/security-team/issues/543

refs elastic/security-team/issues/543

kevinlog · 2021-04-21T13:19:42Z

@ashokaditya

checked it out and it looks great so far!

A couple things to call out:

We're saving the entry with matches, however the Endpoint should be reading the field as wildcard_caseless - does this value get set during artifact creation? cc/ @paul-tavares
In addition, I see that matches is being saved in the SO, but the UI still reflects is for the operator in the saved entry. I would imagine this should reflect the input and say matches

Similarly if I go to edit the entry, the operator is set to is where it should be matches

Let me know if you have any questions. The requirements in the original ticket may not have been clear enough on item 2, happy to adjust!

ashokaditya · 2021-04-21T16:13:31Z

@elasticmachine merge upstream

refs elastic/security-team/issues/543

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx

paul-tavares

Left some comments. Looking good :)

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx

refs elastic/security-team/issues/543

refs f9cb7ed refs b3f5dc4

x-pack/plugins/lists/common/schemas/types/entries.ts

marshallmain · 2021-04-27T22:36:08Z

Changes to the shared exception list types look great! Thanks for all the hard work on this 😄

@ashokaditya, @yctercero, and I met today to discuss how the detection engine exception UI should handle the addition of wildcard entries. Until now, the exceptions UI has been built on the assumption that it supports all entry types, however the addition of wildcard entries means this is no longer true. We hope to add wildcard entry support in the detection exceptions UI at some point, so we settled on continuing to use the complete entry-type-union (EntriesArray) as a static type in the exceptions UI and we'll simply hide the wildcard option until we implement it.

The alternative option was decoupling the exceptions UI from the list of all entry types by adding a new union that contains only the subset of entry types supported by detection exceptions. However, that refactoring effort was deemed to be too significant to include in this PR. Long term, if we need entry types that are specific to trusted apps (i.e. detections will never support them) we should consider decoupling the exceptions UI to make it easier to add new entry types.

x-pack/plugins/lists/public/exceptions/components/builder/helpers.ts

x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx

yctercero

LGTM! Just had some small comments about removing some type casts. I don't know the ins and outs of the trusted apps logic, so focused mainly on the exception type changes and testing basic functionality.

Not sure if a ticket already exists, but would be great to have a ticket to reference for the work needed detections side to hide this operator for now in UI and add a check API side to throw if a wildcard entry is trying to be added to a detection type exception list.

Just to note: I think @marshallmain point of decoupling types and creating unions could be a really good idea. I know we discussed it over zoom and it's definitely something to keep in mind for the larger discussions around exceptions refactoring, but I think it would add a whole lot of changes were we to jump to that right now.

review changes refs dbd3532

review changes refs bcf615a refs b3f5dc4

review changes /pull/97623#discussion_r621673881 refs 6f2d0d7

review changes

CI check suggestion

paul-tavares

🔥 🔥
Left just a minor comment, but it should not hold merging this in.

Awesome job on this

paul-tavares · 2021-04-28T19:32:20Z

x-pack/plugins/security_solution/common/utils/path_placeholder.ts

+
+import { ConditionEntryField, OperatingSystem, TrustedAppEntryTypes } from '../endpoint/types';
+
+export const placeholderText = {


I suggest that this be a function instead of object definition in order to ensure they are not mutated.
Also - are these for testing? if so, I would also suggest moving them to a file (or directory) whose name as "test" in it. like test_utils.ts

Ah, I see your point with the mutation. I would change this to a function. Although, I'm just reusing the same text values within the getPlaceholderText function and also within the unit tests and thus I believe it is not necessary to move this to a test_utils.ts kind of file.

ashokaditya · 2021-04-29T08:35:26Z

@elasticmachine merge upstream

review suggestion /pull/97623/commits/2dc4fd390cf5ea0e4fa67b3f5fc2561cbb29555e

x-pack/plugins/security_solution/common/utils/path_placeholder.ts

dasansol92

LGTM!!🔥 🔥
Just a function naming comment but not needs to be changed now.

refs 330731e

kibanamachine · 2021-04-29T12:24:45Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lists`	165	166	+1
`securitySolution`	2181	2182	+1
total			+2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`lists`	237	239	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	7.0MB	7.0MB	+1.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`lists`	210.2KB	210.7KB	+521.0B

Unknown metric groups

API count

id	before	after	diff
`lists`	258	260	+2

History

💚 Build #123171 succeeded e8e9ce5
💔 Build #123184 failed ab49010e404be6dc2955102a7608ec2bb29a5413
💚 Build #122951 succeeded d7cb47b
💚 Build #122932 succeeded eb8c572
💚 Build #122911 succeeded 2dc4fd3

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

kibanamachine · 2021-04-29T12:57:19Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 97623

…7623) (#98749) * show operator dropdown for path field refs elastic/security-team/issues/543 * update translation to use consistent values refs elastic/security-team/issues/543 * update schema to validate path values refs elastic/security-team/issues/543 * add tests for field and operator values refs elastic/security-team/issues/543 * review changes refs elastic/security-team/issues/543 * update schema to enforce dropdown validation for PATH field refs elastic/security-team/issues/543 * add tests for schema updates refs 1deab39 refs elastic/security-team/issues/543 * optimise dropdown list for re-renders refs elastic/security-team/issues/543 * align input fields and keep alignments when resized refs elastic/security-team/issues/543 * correctly enter operator data on trusted app CRUD refs elastic/security-team/issues/543 * update tests refs 2ac56ee refs elastic/security-team/issues/543 * remove redundant code review changes * better type assertion review changes * move operator options out of component - these do not depend on component props and thus no need to have it within a useMemo callback. - review changes * derive keys from operator entry field review changes * update type * use custom styles for aligning input fields review changes * add a custom type for trusted_apps operator undo changes from list plugin and server/lib/detection_engine refs 2ac56ee refs elastic/security-team/issues/543 * add wildcard entry type refs elastic/security-team/issues/543 refs #97623 (review) * use the new entry type refs elastic/security-team/issues/543 refs #97623 (review) * update tests refs elastic/security-team/issues/543 refs #97623 (review) * update name for wildcard type so that it can be used also for cased inputs refs elastic/security-team/issues/543 refs f9cb7ed * update artifacts to support wildcard entries refs elastic/security-team/issues/543 * add tests for list schemas refs f9cb7ed refs elastic/security-team/issues/543 * add placeholders for path values review changes /pull/97623#discussion_r620617999 * ignore type check for now * add type assertion refs 284352e * remove unnecessary test refs 2ac56ee * fix types refs f9cb7ed refs b3f5dc4 * add a note to entries review changes refs dbd3532 * remove redundant type assertions review changes refs bcf615a refs b3f5dc4 * move placeholder text logic to utils review changes /pull/97623#discussion_r621673881 refs 6f2d0d7 * pass the style as prop review changes * update api doc CI check suggestion * make placeholderText a function expression review suggestion /pull/97623/commits/2dc4fd390cf5ea0e4fa67b3f5fc2561cbb29555e * use semantic names for functions refs 330731e Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # api_docs/security_solution.json

ashokaditya added 2 commits April 20, 2021 12:34

show operator dropdown for path field

57bf657

refs elastic/security-team/issues/543

update translation to use consistent values

dd84507

refs elastic/security-team/issues/543

update schema to validate path values

9f72250

refs elastic/security-team/issues/543

ashokaditya commented Apr 20, 2021

View reviewed changes

x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts Outdated Show resolved Hide resolved

add tests for field and operator values

fa9e3ce

refs elastic/security-team/issues/543

dasansol92 reviewed Apr 20, 2021

View reviewed changes

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Outdated Show resolved Hide resolved

dasansol92 reviewed Apr 20, 2021

View reviewed changes

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Outdated Show resolved Hide resolved

dasansol92 reviewed Apr 20, 2021

View reviewed changes

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Outdated Show resolved Hide resolved

review changes

8c19290

refs elastic/security-team/issues/543

ashokaditya marked this pull request as ready for review April 21, 2021 07:47

ashokaditya requested a review from a team as a code owner April 21, 2021 07:47

update schema to enforce dropdown validation for PATH field

1deab39

refs elastic/security-team/issues/543

dasansol92 reviewed Apr 21, 2021

View reviewed changes

x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts Outdated Show resolved Hide resolved

ashokaditya added 2 commits April 21, 2021 11:17

add tests for schema updates

baac452

refs 1deab39 refs elastic/security-team/issues/543

optimise dropdown list for re-renders

5d9e848

refs elastic/security-team/issues/543

kibanamachine and others added 2 commits April 21, 2021 12:13

Merge branch 'master' into sec-team-543/allow-wildcard-for-paths

9badfcc

align input fields and keep alignments when resized

f657b80

refs elastic/security-team/issues/543

ashokaditya commented Apr 22, 2021

View reviewed changes

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Show resolved Hide resolved

paul-tavares reviewed Apr 22, 2021

View reviewed changes

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Outdated Show resolved Hide resolved

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Outdated Show resolved Hide resolved

correctly enter operator data on trusted app CRUD

2ac56ee

refs elastic/security-team/issues/543

fix types

dbd3532

refs f9cb7ed refs b3f5dc4

ashokaditya requested a review from yctercero April 27, 2021 20:30

marshallmain approved these changes Apr 27, 2021

View reviewed changes

yctercero reviewed Apr 27, 2021

View reviewed changes

x-pack/plugins/lists/common/schemas/types/entries.ts Show resolved Hide resolved

yctercero reviewed Apr 27, 2021

View reviewed changes

x-pack/plugins/lists/public/exceptions/components/builder/helpers.ts Outdated Show resolved Hide resolved

yctercero reviewed Apr 27, 2021

View reviewed changes

x-pack/plugins/security_solution/common/endpoint/schema/trusted_apps.ts Outdated Show resolved Hide resolved

yctercero reviewed Apr 27, 2021

View reviewed changes

...olution/public/management/pages/trusted_apps/view/components/condition_entry_input/index.tsx Outdated Show resolved Hide resolved

yctercero approved these changes Apr 27, 2021

View reviewed changes

ashokaditya added 3 commits April 28, 2021 08:39

add a note to entries

ddc3d72

review changes refs dbd3532

remove redundant type assertions

0bf8199

review changes refs bcf615a refs b3f5dc4

move placeholder text logic to utils

2dc4fd3

review changes /pull/97623#discussion_r621673881 refs 6f2d0d7

ashokaditya added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 28, 2021

ashokaditya added 2 commits April 28, 2021 13:15

pass the style as prop

eb8c572

review changes

update api doc

d7cb47b

CI check suggestion

paul-tavares approved these changes Apr 28, 2021

View reviewed changes

kibanamachine and others added 2 commits April 29, 2021 04:35

Merge branch 'master' into sec-team-543/allow-wildcard-for-paths

e8e9ce5

make placeholderText a function expression

330731e

review suggestion /pull/97623/commits/2dc4fd390cf5ea0e4fa67b3f5fc2561cbb29555e

dasansol92 reviewed Apr 29, 2021

View reviewed changes

x-pack/plugins/security_solution/common/utils/path_placeholder.ts Outdated Show resolved Hide resolved

dasansol92 reviewed Apr 29, 2021

View reviewed changes

x-pack/plugins/security_solution/common/utils/path_placeholder.ts Outdated Show resolved Hide resolved

dasansol92 approved these changes Apr 29, 2021

View reviewed changes

use semantic names for functions

4e7dce9

refs 330731e

ashokaditya merged commit c93e028 into elastic:master Apr 29, 2021

ashokaditya deleted the sec-team-543/allow-wildcard-for-paths branch April 29, 2021 12:54

ashokaditya mentioned this pull request Apr 29, 2021

[7.x] [Security Solution][Endpoint] Allow wildcard in trusted app paths (#97623) #98749

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Endpoint] Allow wildcard in trusted app paths #97623

[Security Solution][Endpoint] Allow wildcard in trusted app paths #97623

ashokaditya commented Apr 20, 2021 •

edited

Loading

dasansol92 commented Apr 20, 2021

elasticmachine commented Apr 21, 2021

kevinlog commented Apr 21, 2021

ashokaditya commented Apr 21, 2021

paul-tavares left a comment

marshallmain commented Apr 27, 2021

yctercero left a comment •

edited

Loading

paul-tavares left a comment

paul-tavares Apr 28, 2021

ashokaditya Apr 29, 2021 •

edited

Loading

ashokaditya commented Apr 29, 2021

dasansol92 left a comment

kibanamachine commented Apr 29, 2021

API count

kibanamachine commented Apr 29, 2021


		import { ConditionEntryField, OperatingSystem, TrustedAppEntryTypes } from '../endpoint/types';

		export const placeholderText = {

[Security Solution][Endpoint] Allow wildcard in trusted app paths #97623

[Security Solution][Endpoint] Allow wildcard in trusted app paths #97623

Conversation

ashokaditya commented Apr 20, 2021 • edited Loading

Summary

Checklist

dasansol92 commented Apr 20, 2021

elasticmachine commented Apr 21, 2021

kevinlog commented Apr 21, 2021

ashokaditya commented Apr 21, 2021

paul-tavares left a comment

Choose a reason for hiding this comment

marshallmain commented Apr 27, 2021

yctercero left a comment • edited Loading

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

paul-tavares Apr 28, 2021

Choose a reason for hiding this comment

ashokaditya Apr 29, 2021 • edited Loading

Choose a reason for hiding this comment

ashokaditya commented Apr 29, 2021

dasansol92 left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 29, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

kibanamachine commented Apr 29, 2021

💔 Backport failed

ashokaditya commented Apr 20, 2021 •

edited

Loading

yctercero left a comment •

edited

Loading

ashokaditya Apr 29, 2021 •

edited

Loading