[Security Solution][Exceptions] - Update add/edit exception flyouts #143127

yctercero · 2022-10-11T19:55:16Z

Summary

Addresses #131668, #131671, #143014, #143015.

Updates the exception item add/edit flyout to match new designs and flows. While this can only be tested in our current UI from the rule details page, alerts, alert details flyout, it's been built out for the other cases where it will be used - rule bulk actions and exception list management.

To Do

Some of the hooks need unit tests
Update to use the components from the exception package, to start deleting them from the rule_exceptions/components
Add back in 2 error states - when version conflict (not super common) and list does not exist and user needs to remove association from rule, again, shouldn't be super common error

Breaking down the changes

packages/kbn-securitysolution-autocomplete/
- Updates the field combo box to allow user to input a custom option, no longer restricted to selecting a prefilled value
- There is a boolean added, that is set to default false, so this custom option functionality is optional
x-pack/plugins/lists/public/exceptions/components/builder/
- Update UI to show text under field combo box if it accepts a custom option to let user know that this is the case
- Updates the builder to take an exception item name from user input
x-pack/plugins/security_solution/cypress/e2e/exceptions/
- Updates and adds cypress tests related to adding/editing exceptions from the different existing flows
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/
- Update the add exception flyout to use the new components added in PR
- Tried to move out submission logic into hook to clean things up a bit
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/
- Update the add exception flyout to use the new components added in PR
- Tried to move out submission logic into hook to clean things up a bit
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/all_exception_items_table/
- Updates the all items view to take in an array of listTypes since now the table isn't just endpoint exceptions or detection exceptions, it can include rule_default list type
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/exception_item_card
- Now includes affected rules and affected lists link in the card, so it's easier to tell which items are rule only vs shared list
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/logic/
- Updated the hooks to try and simplify them and separate concerns a bit
- TODO: these need updated tests
x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/logic/use_exception_flyout_data.tsx
- New hook to try and manage the logic around the add/edit exception flyout indices
  - If a single rule is passed into the flyout, it uses the data_view_id if one is defined or the index to fetch the fields
  - If multiple rules are passed in, it defaults to using the default security data view (right now there's no UI supporting this use case, but it would be part of the bulk rule action flow)
  - If no rules are passed in, it defaults to using the default security data view (right now there's no UI supporting this use case, but it would be part of the create exception item from the list management flow)
x-pack/plugins/security_solution/public/detections/components/alerts_table/timeline_actions/
- Updates logic around adding exception from alert and alert detail flyout to fit with new add exception modal

Existing questions

When add modal is dealing with multiple rules (bulk action or when adding an exception from the list management page where you can choose what rules to add the exception to) - should large value lists be allowed with some sort of warning text that any rules of type x, y, z will ignore any exceptions of type list?
- In this PR, I am allowing large value lists unless a single rule is passed in where we can be sure of whether large value lists are supported by it

Addresses

#127696, #99597, #117289, #99022, #135254, #102460, #116586

Screenshots

Shows exception item that is part of a shared list

Add endpoint exception

Edit endpoint exception

Add rule exception

Edit rule exception

Checklist

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
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

…ypress tests and unit tests

elasticmachine · 2022-10-11T22:57:42Z

Pinging @elastic/security-solution (Team: SecuritySolution)

maximpn

@yctercero Rules area LGTM

dhurley14 · 2022-10-17T14:37:32Z

@elasticmachine merge upstream

dhurley14

this is excellent! a few comments for us to look at down the line. LGTM!

dhurley14 · 2022-10-18T19:29:59Z

packages/kbn-securitysolution-autocomplete/src/field/index.tsx

@@ -43,6 +45,29 @@ export const FieldComponent: React.FC<FieldProps> = ({
    fieldInputWidth,
    onChange,
  });
+
+  if (acceptsCustomOptions) {


nit: might be possible to remove this if statement and either conditionally pass the props or conditionally modify a props object.

dhurley14 · 2022-10-18T19:52:34Z

x-pack/plugins/lists/public/exceptions/components/builder/exception_items_renderer.tsx

+    return filterExceptionItems(exceptions);
+  }, [exceptions]);
+
+  // useEffect(() => {


nit: could probably remove this commented out code.

dhurley14 · 2022-10-18T19:54:42Z

...y_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx

+      // We'll only block this when we know what rule we're dealing with.
+      // When dealing with numerous rules that can be a mix of those that do and
+      // don't work with large value lists we'll need to communicate that to the
+      // user but not block.


good to know thanks for commenting this here.

dhurley14 · 2022-10-18T21:34:28Z

...ecurity_solution/public/detection_engine/rule_exceptions/logic/use_exception_flyout_data.tsx

+  // If data view is defined, it superceeds use of rule defined index patterns.
+  // If no rule is available, use fields from default data view id.


This is not great that we have to make sure to remember data views supercede index patterns on rules. If we don't see customers using data views more then we should look into adding a boolean variable on the rule saying which data source to use.

dhurley14 · 2022-10-18T21:35:55Z

...ecurity_solution/public/detection_engine/rule_exceptions/logic/use_exception_flyout_data.tsx

+  // We only want to provide a non empty array if it's an ML rule and we were able to fetch
+  // the index patterns, or if it's a rule not using data views. Otherwise, return an empty
+  // empty array to avoid making the `useFetchIndex` call


better yet if we could encapsulate all this logic determining to select a data view vs an index pattern or if it's an ml rule use an empty array in another hook which could be used here...

…-fix'

kibana-ci · 2022-10-19T19:10:19Z

💚 Build Succeeded

Buildkite Build
Commit: e6a4c20

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3170	3196	+26

Async chunks

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

id	before	after	diff
`lists`	191.2KB	192.3KB	+1.1KB
`securitySolution`	9.3MB	10.0MB	⚠️ +662.6KB
total			+663.7KB

Page load bundle

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

id	before	after	diff
`securitySolution`	51.1KB	50.9KB	-191.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-list-utils`	192	194	+2

async chunk count

id	before	after	diff
`securitySolution`	39	36	-3

ESLint disabled in files

id	before	after	diff
`securitySolution`	73	72	-1

ESLint disabled line counts

id	before	after	diff
`lists`	13	18	+5

Total ESLint disabled count

id	before	after	diff
`lists`	18	23	+5
`securitySolution`	484	483	-1
total			+4

History

💔 Build #81476 failed 5a4452d
💔 Build #81451 failed 813fe8e
💚 Build #80767 succeeded 4b998a1
💛 Build #79595 was flaky 980403b
💔 Build #79580 failed fe11114

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

cc @yctercero

squashed commit of updates to add/edit flyouts for exception, added c…

fe11114

…ypress tests and unit tests

yctercero self-assigned this Oct 11, 2022

yctercero added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. release_note:feature Makes this part of the condensed release notes Team:Security Solution Platform Security Solution Platform Team v8.6.0 labels Oct 11, 2022

fix up some check type issues

980403b

yctercero marked this pull request as ready for review October 11, 2022 22:57

yctercero requested review from a team as code owners October 11, 2022 22:57

yctercero requested a review from maximpn October 11, 2022 22:57

maximpn approved these changes Oct 13, 2022

View reviewed changes

Merge branch 'main' into add_edit_exception_flyouts

4b998a1

dhurley14 approved these changes Oct 18, 2022

View reviewed changes

WafaaNasr approved these changes Oct 19, 2022

View reviewed changes

WafaaNasr mentioned this pull request Oct 19, 2022

[Security Solution][Exceptions] - Create/ refactor Exception-List common's components in @kbn/securitysolution-exception-list-components #143363

Merged

3 tasks

dhurley14 and others added 3 commits October 19, 2022 10:47

Merge branch 'main' into add_edit_exception_flyouts

7215f1b

Merge branch 'main' into add_edit_exception_flyouts

813fe8e

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

5a4452d

…-fix'

andrew-goldstein approved these changes Oct 19, 2022

View reviewed changes

fixes type errors

e6a4c20

dhurley14 merged commit 6c5d816 into elastic:main Oct 19, 2022

dhurley14 deleted the add_edit_exception_flyouts branch October 19, 2022 19:13

kibanamachine added the backport:skip This commit does not require backporting label Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Exceptions] - Update add/edit exception flyouts #143127

[Security Solution][Exceptions] - Update add/edit exception flyouts #143127

yctercero commented Oct 11, 2022

elasticmachine commented Oct 11, 2022

maximpn left a comment

dhurley14 commented Oct 17, 2022

dhurley14 left a comment

dhurley14 Oct 18, 2022

dhurley14 Oct 18, 2022

dhurley14 Oct 18, 2022

dhurley14 Oct 18, 2022

dhurley14 Oct 18, 2022

kibana-ci commented Oct 19, 2022

API count

async chunk count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

		// If data view is defined, it superceeds use of rule defined index patterns.
		// If no rule is available, use fields from default data view id.

[Security Solution][Exceptions] - Update add/edit exception flyouts #143127

[Security Solution][Exceptions] - Update add/edit exception flyouts #143127

Conversation

yctercero commented Oct 11, 2022

Summary

To Do

Breaking down the changes

Existing questions

Addresses

Screenshots

Add endpoint exception

Edit endpoint exception

Add rule exception

Edit rule exception

Checklist

For maintainers

elasticmachine commented Oct 11, 2022

maximpn left a comment

Choose a reason for hiding this comment

dhurley14 commented Oct 17, 2022

dhurley14 left a comment

Choose a reason for hiding this comment

dhurley14 Oct 18, 2022

Choose a reason for hiding this comment

dhurley14 Oct 18, 2022

Choose a reason for hiding this comment

dhurley14 Oct 18, 2022

Choose a reason for hiding this comment

dhurley14 Oct 18, 2022

Choose a reason for hiding this comment

dhurley14 Oct 18, 2022

Choose a reason for hiding this comment

kibana-ci commented Oct 19, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

API count

async chunk count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History