[Security Solution][Exceptions] - Add exception list duplication options with and without expired items #154991

yctercero · 2023-04-15T16:35:31Z

Summary

Adds the following:

Add the option to duplicate from the shared exception list management actions dropdowns
- User can select to include exception items with expired TTL
- User can select to not include exception items with expired TTL
- Cypress tests added for both options

Add the option to duplicate from the shared exception list details header actions dropdowns
- User can select to include exception items with expired TTL
- User can select to not include exception items with expired TTL
- On duplicate, user is redirected to newly created list
- Cypress tests added for both options

Add the option to include or exclude exception items with expired TTL when bulk duplicating rules from rules management page
- Cypress tests added for both options

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

…r duplication

…plication - unit tests added

…m list details header

…_8.8

…fix'

…o exceptions_dupe_8.8

elasticmachine · 2023-04-16T03:47:24Z

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

…_8.8

dplumlee

Pulled down and tested all three scenarios, looks good from an alert area pov. Great test coverage!

dplumlee · 2023-04-20T20:06:14Z

x-pack/plugins/lists/server/services/exception_lists/duplicate_exception_list.ts

+  const filter = includeExpiredExceptions
+    ? []
+    : [
+        `(${savedObjectPrefix}.attributes.expire_time > "${new Date().toISOString()}" OR NOT ${savedObjectPrefix}.attributes.expire_time: *)`,


This isn't necessarily a comment for your PR but we should perhaps centralize this filter string since it's used just about every place we have to filter for/against expired exceptions and probably will be going forward. I know we have this for an array of SavedObjectType's but nothing else.

Good point! I can probably do a follow up with this. My only hesitation doing it here is trying not to increase the number of files touched since it's already much more than anticipated.

… for ttl items option

nastasha-solomon · 2023-04-20T21:14:58Z

Thanks for tagging the docs team for the UI copy, @yctercero! Please find my suggestions below.

Confirmation prompt to duplicate expired exception items

Does this prompt only appear after users choose to duplicate a shared exception list with expired exception items? If it does, I recommend some minor tweaks:

Title: Duplicate expired exceptions?
Body text: This exception list has expired exceptions. Switch the toggle off if you don't want to duplicate them.
Button: Duplicate

If this prompt appears after users choose to duplicate a shared exception list or it appears regardless of whether the list contains expired exception items, I'd suggest something different. I'll hold off on that for now though.

Confirmation prompt to duplicate rule

Title
To clarify, does this prompt appear regardless of whether the rule has an exception? If that's the case, I think the title should reflect the primary action the user is taking, which is duplicating the rule. With that in mind, here's my suggestion for the title:
Duplicate the rule?

Body
My recommendations for the body text if the user is only duplicating one rule:
You're duplicating 1 rule. Choose what to duplicate:

If the user is duplicating multiple rules:
You're duplicating 2 rules. Choose what to duplicate:

Options
My recs for the options if the user is only duplicating one rule:

Only the rule
The rule and its active exceptions
The rule and all of its exceptions (active and expired)

If the user is duplicating multiple rules:

Only the rules
The rules and their active exceptions
The rules and all of their exceptions (active and expired)

…y docs

spong · 2023-04-20T22:04:00Z

...detection_engine/rule_management_ui/components/rules_table/bulk_actions/use_bulk_actions.tsx

+              modalDuplicationConfirmationResult === DuplicateOptions.withExceptions ||
+              modalDuplicationConfirmationResult ===
+                DuplicateOptions.withExceptionsExcludeExpiredExceptions,
+            include_expired_exceptions: !(
+              modalDuplicationConfirmationResult ===
+              DuplicateOptions.withExceptionsExcludeExpiredExceptions
+            ),


nit: Maybe have useBulkDuplicateExceptionsConfirmation's showBulkDuplicateConfirmation return object also include include_exceptions/include_expired_exceptions bools thus encapsulating this logic?

Was just thinking how to DRY this out since it's used a few places, no biggie either way.

...y_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_exceptions.ts

spong

Reviewed rules area code changes, checked out, tested locally and LGTM! 👍 🚀

Great stuff here @yctercero 🎉 I left a few nits, but nothing major. Really appreciate the thorough test coverage you added here .

I tested both the Rules Table/Details bulk_editing paths, along with the Shared Exceptions UI duplication as well. Didn't find any issues, and everything looked as expected (even when duplicating lists with only expired exception items :).

nkhristinin · 2023-04-21T12:03:42Z

LGTM! Nice work @yctercero

I only have one question, currently the rule duplication modal window looks like that.

And for me, options is more confusing than it was here

michaelolo24 · 2023-04-21T17:43:31Z

x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts

+        .send()
+        .expect(200);
+
+      // Item should have been duplicated, even if expired


Thanks for the comments. Really helpful!

x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts

michaelolo24

Investigations changes - Seems like they were only cypress tests, so might be worth updating codeowners or re-organizing the files later on as most of these don't affect us

kibana-ci · 2023-04-21T23:01:45Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 68042a4

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #8 / AIOps POST /internal/aiops/explain_log_rate_spikes - groups only with ecommerce should return group only in chunks with streaming without compression with flushFix
[job] [logs] Security Solution Tests #3 / timeline flyout button the (+) button popover menu owns focus

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lists`	259	260	+1
`securitySolution`	3831	3832	+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
`@kbn/securitysolution-io-ts-list-types`	503	515	+12
`@kbn/securitysolution-list-api`	64	65	+1
`@kbn/securitysolution-list-hooks`	47	49	+2
total			+15

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`@kbn/securitysolution-io-ts-list-types`	1	0	-1

Async chunks

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

id	before	after	diff
`lists`	157.2KB	157.3KB	+78.0B
`securitySolution`	9.1MB	9.1MB	+7.0KB
total			+7.1KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-io-ts-list-types`	516	528	+12
`@kbn/securitysolution-list-api`	67	69	+2
`@kbn/securitysolution-list-hooks`	60	62	+2
total			+16

ESLint disabled line counts

id	before	after	diff
`securitySolution`	394	397	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	474	477	+3

History

💔 Build #122190 failed 68d49ef
💔 Build #121791 failed 0539797
💔 Build #121761 failed 3dfab5a
💚 Build #120072 succeeded c6e0949
💔 Build #120067 failed ff0e13c

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

cc @yctercero

…ons with and without expired items (elastic#154991) ## Summary Adds the following: - Add the option to duplicate from the shared exception list management actions dropdowns - User can select to include exception items with expired TTL - User can select to not include exception items with expired TTL - Cypress tests added for both options

yctercero added 7 commits April 11, 2023 12:43

add duplication route and integration tests

2408f81

Perform bulk actions - backend updates with tests

e177290

Perform bulk actions, duplicate - front end changes and cypress tests

d95f8c1

Updated shared exception list management export modal to also work fo…

19ce08f

…r duplication

Updated shared exception lists management overflow menu to include du…

3b1cb04

…plication - unit tests added

Shared exception lists management - cypress updates

89a59fd

Shared exception list details management - add logic to duplicate fro…

03dccf2

…m list details header

yctercero self-assigned this Apr 15, 2023

yctercero added 2 commits April 15, 2023 09:38

undo unit test comment

2846149

Merge branch 'main' of github.com:elastic/kibana into exceptions_dupe…

1d87929

…_8.8

yctercero requested a review from a team April 15, 2023 16:40

kibanamachine and others added 3 commits April 15, 2023 16:43

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

ff0e13c

…fix'

clean up some type issues

04f511a

Merge branch 'exceptions_dupe_8.8' of github.com:yctercero/kibana int…

c6e0949

…o exceptions_dupe_8.8

yctercero marked this pull request as ready for review April 16, 2023 03:47

yctercero requested review from a team as code owners April 16, 2023 03:47

yctercero requested a review from spong April 16, 2023 03:47

Merge branch 'main' of github.com:elastic/kibana into exceptions_dupe…

2831573

…_8.8

dplumlee approved these changes Apr 20, 2023

View reviewed changes

updating manage bulk duplicate confirmation modal to include checkbox…

3dfab5a

… for ttl items option

updated cypress tests per new UI and applied text updates suggested b…

0539797

…y docs

spong reviewed Apr 20, 2023

View reviewed changes

...y_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_exceptions.ts Outdated Show resolved Hide resolved

spong reviewed Apr 20, 2023

View reviewed changes

...y_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_exceptions.ts Outdated Show resolved Hide resolved

spong approved these changes Apr 20, 2023

View reviewed changes

nkhristinin approved these changes Apr 21, 2023

View reviewed changes

michaelolo24 reviewed Apr 21, 2023

View reviewed changes

x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts Outdated Show resolved Hide resolved

michaelolo24 approved these changes Apr 21, 2023

View reviewed changes

yctercero added 2 commits April 21, 2023 13:16

updating cypress tests and edits from feedback

68d49ef

fix jest missing param

68042a4

yctercero enabled auto-merge (squash) April 21, 2023 22:41

yctercero merged commit 1115532 into elastic:main Apr 21, 2023
19 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Apr 21, 2023

yctercero deleted the exceptions_dupe_8.8 branch April 24, 2023 04:43

yctercero mentioned this pull request Apr 24, 2023

[Security Solution][Exceptions] - Add shared exception list duplication UX #154101

Closed

4 tasks

This was referenced May 4, 2023

Shared exception lists can be duplicated elastic/security-docs#3219

Closed

Options in the duplicate rule modal updated elastic/security-docs#3220

Closed

banderror mentioned this pull request May 5, 2023

[Security Solution] Rule counter is not displaying properly the number of rules to be duplicated when duplicating a rule #156655

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Exceptions] - Add exception list duplication options with and without expired items #154991

[Security Solution][Exceptions] - Add exception list duplication options with and without expired items #154991

yctercero commented Apr 15, 2023 •

edited

elasticmachine commented Apr 16, 2023

dplumlee left a comment

dplumlee Apr 20, 2023

yctercero Apr 20, 2023

nastasha-solomon commented Apr 20, 2023 •

edited

spong Apr 20, 2023

spong left a comment

nkhristinin commented Apr 21, 2023

michaelolo24 Apr 21, 2023

michaelolo24 left a comment

kibana-ci commented Apr 21, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

[Security Solution][Exceptions] - Add exception list duplication options with and without expired items #154991

[Security Solution][Exceptions] - Add exception list duplication options with and without expired items #154991

Conversation

yctercero commented Apr 15, 2023 • edited

Summary

Checklist

elasticmachine commented Apr 16, 2023

dplumlee left a comment

Choose a reason for hiding this comment

dplumlee Apr 20, 2023

Choose a reason for hiding this comment

yctercero Apr 20, 2023

Choose a reason for hiding this comment

nastasha-solomon commented Apr 20, 2023 • edited

Confirmation prompt to duplicate expired exception items

Confirmation prompt to duplicate rule

spong Apr 20, 2023

Choose a reason for hiding this comment

spong left a comment

Choose a reason for hiding this comment

nkhristinin commented Apr 21, 2023

michaelolo24 Apr 21, 2023

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 21, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Any counts in public APIs

Async chunks

API count

ESLint disabled line counts

Total ESLint disabled count

History

yctercero commented Apr 15, 2023 •

edited

nastasha-solomon commented Apr 20, 2023 •

edited