[Security Solution][Exceptions] - Initial updates to exceptions viewer UX #138770

yctercero · 2022-08-12T20:40:16Z

Summary

This does not complete the planned rule details exceptions tab changes planned, but it is a shippable start:

API changes

Adds API for determining the list-rule references. New endpoint returns something like:

{
        references: [
          {
            'my-list-id': [
              {
                exception_lists: [
                  {
                    id: 'myListId',
                    list_id: 'my-list-id',
                    namespace_type: 'single',
                    type: 'detection',
                  },
                ],
                id: '4656dc92-5832-11ea-8e2d-0242ac130003',
                name: 'My rule',
                rule_id: 'my-rule-id',
              },
            ],
          },
        ],
      }

Updates the exception items find api to include the search param which allows for simple search queries - used with the EUI search bar

UI updates

Moved the exception components into new rule_exceptions folder per suggested folder structure updates listed here
Updates the rule details tabs to split endpoint and rule exceptions into their own tabs
Updates the viewer utilities header now that these different exception types are split
Updates exception item UI to match new designs
Updates the UI for when there are no items
Removes use_exception_list_items hook as it is no longer in use
Flyouts (add/edit) remain untouched

Things to test

Any regressions, all existing functionality should continue to work
- Can add an exception from an alert
- Can add an exception from rule details exceptions tab
  - Success toast or error toast on add
  - On success, user is still on the exceptions tab and new item is shown
- Can edit an exception item from rule details exceptions tab
  - Success toast or error toast on edit
  - On success, user is still on the exceptions tab
- Can delete an exception item from rule details exceptions tab
  - While deleting, exception item action buttons are disabled
  - Success toast or error toast on delete
Exception items are separated into rule exceptions and endpoint exceptions
- Appropriate flyout shows when selecting to add a new endpoint exception vs rule exception
- Only endpoint exceptions show on endpoint exceptions tab
Can search items using simple query search
Exception items show user how many rules each exception item affects

Addresses:

Changes to come:

Ability to see all exception list containers attached to rule (this would be new functionality, not taking anything away from current feature). Right now you can only see the individual items, can't group by parent list.
Known issue - with the separation of endpoint/rule exception tabs, hitting issues on add/edit exceptions, it doesn't stay on that tab - this is fixed by [Security Solution][Rule details] - Updates rule details page to allow tabs to be a part of url #139592

Screenshots

Loading screen

No exceptions exist screen

No search results found screen

Search in progress screen

Error screen

Detections exceptions

Endpoint exceptions

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

…-fix'

…te_viewer

…-fix'

…te_viewer

joepeeples

Thanks for tagging the docs team! The screenshots above look great, just a small edit suggested for the empty state text.

x-pack/plugins/security_solution/public/common/components/exceptions/viewer/translations.ts

yctercero · 2022-08-25T15:26:30Z

...-securitysolution-io-ts-list-types/src/request/find_exception_list_item_schema/index.test.ts

@@ -123,6 +124,18 @@ describe('find_list_item_schema', () => {
    expect(message.schema).toEqual(expected);
  });

+  test('it should validate with search missing', () => {


Non-breaking change. It's a new field that is optional.

yctercero · 2022-08-25T15:29:27Z

packages/kbn-securitysolution-list-api/src/api/index.ts

@@ -324,7 +322,7 @@ export { fetchExceptionListByIdWithValidation as fetchExceptionListById };
 * @param http Kibana http service
 * @param listIds ExceptionList list_ids (not ID)
 * @param namespaceTypes ExceptionList namespace_types
- * @param filterOptions optional - filter by field or tags
+ * @param filter optional


Unit test updated here: x-pack/plugins/lists/public/exceptions/api.test.ts

MadameSheema · 2022-09-07T08:13:57Z

x-pack/plugins/security_solution/cypress/tasks/rule_details.ts

  cy.get(EXCEPTIONS_TAB).click();
 };

 export const editException = () => {
-  cy.get(EXCEPTION_ITEM_ACTIONS_BUTTON).click({ force: true });
+  cy.get(EXCEPTION_ITEM_ACTIONS_BUTTON).eq(0).click({ force: true });


Do we have more than one element now with the same locator?

We do, there's a test with multiple exception items in the items table. Updated to take an index.

...ins/security_solution/cypress/integration/exceptions/rule_details_flow/add_exception.spec.ts

...ns/security_solution/cypress/integration/exceptions/rule_details_flow/edit_exception.spec.ts

...y_solution/cypress/integration/exceptions/rule_details_flow/edit_exception_data_view.spec.ts

MadameSheema · 2022-09-07T08:53:29Z

x-pack/test/security_solution_cypress/es_archives/exceptions_2/data.json

+
+{
+  "type": "doc",


Do we strictly need 2 documents? If not, I would suggest extending the current document :)

I added 2 because I wanted to update the tests to more explicitly show that the matching alert is not created and the non matching one is. Let me know if you think that's not necessary and I can update.

MadameSheema

security-engineering-productivity changes reviewed!!

Left some nit comments and improvements that do not block the PR to be merged (but would be niche to be addressed/taken into consideration) :)

yctercero · 2022-09-07T16:00:21Z

@elasticmachine merge upstream

yctercero · 2022-09-07T16:01:00Z

security-engineering-productivity changes reviewed!!

Left some nit comments and improvements that do not block the PR to be merged (but would be niche to be addressed/taken into consideration) :)

Thanks @MadameSheema ! I'll be sure to follow up - with the size, I'd like to just get this in and follow up on any non-blocker feedback.

marshallmain

Alerts area changes LGTM

...ity_solution/public/detection_engine/rule_exceptions/components/exception_item_card/meta.tsx

michaelolo24 · 2022-09-08T15:56:38Z

A couple things:

Might be weirdness with the test data being endpoint data without an endpoint installed, but the endpoint exception tab wasn't visible for a non Endpoint Security rule even though I could add an endpoint exception. It show's up if I refresh the page though.
Just that one minor change and for posterity, I think it might be good to have the search functionality auto-update? Was tempted to click on the add exceptions button to run the search. That can be done in a follow up PR though 👍🏾

yctercero · 2022-09-08T17:10:53Z

@elasticmachine merge upstream

kibana-ci · 2022-09-08T17:51:01Z

💛 Build succeeded, but was flaky

Failed CI Steps

FTR Configs #9

Test Failures

[job] [logs] FTR Configs #9 / apis uptime uptime REST endpoints uptime CRUD routes [PUT] /internal/uptime/service/monitors handles private location errors and does not update the monitor if integration policy is unable to be updated

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3057	3063	+6

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`	452	457	+5
`@kbn/securitysolution-list-hooks`	44	42	-2
total			+3

Async chunks

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

id	before	after	diff
`lists`	148.8KB	148.9KB	+48.0B
`securitySolution`	6.4MB	6.4MB	-1.7KB
total			-1.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`	259.9KB	260.0KB	+102.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-io-ts-list-types`	465	470	+5
`@kbn/securitysolution-list-hooks`	56	53	-3
total			+2

ESLint disabled line counts

id	before	after	diff
`@kbn/securitysolution-list-hooks`	2	1	-1

miscellaneous assets size

id	before	after	diff
`securitySolution`	4.1MB	4.2MB	+126.3KB

Total ESLint disabled count

id	before	after	diff
`@kbn/securitysolution-list-hooks`	2	1	-1

History

💚 Build #70161 succeeded 18b0d18
💚 Build #69502 succeeded 7d253fe
💛 Build #69284 was flaky 0b59305
💔 Build #69280 failed f6dc695
💔 Build #69248 failed a665aad

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

cc @yctercero

michaelolo24

LGTM!

yctercero · 2022-09-09T19:55:30Z

@kqualters-elastic @michaelolo24 @MadameSheema

Thank you thank you for your feedback. I've opened a follow up PR addressing it here.

…r UX (elastic#138770) ## Summary **API changes** - Adds API for determining the list-rule references. - Updates the exception items find api to include the `search` param which allows for simple search queries - used with the EUI search bar **UI updates** - Moved the exception components into new `rule_exceptions` folder per suggested folder structure updates listed [here](elastic#138600) - Updates the rule details tabs to split endpoint and rule exceptions into their own tabs - Updates the viewer utilities header now that these different exception types are split - Updates exception item UI to match new designs - Updates the UI for when there are no items - Removes `use_exception_list_items` hook as it is no longer in use - Flyouts (add/edit) remain untouched

initial changes done, need to add tests, fix types, cleanup

363e011

yctercero self-assigned this Aug 12, 2022

yctercero requested a review from yiyangliu9286 August 12, 2022 20:42

yctercero mentioned this pull request Aug 12, 2022

[DOCS][Security Solution][Exceptions] - New exceptions UI/UX elastic/security-docs#2304

Closed

yctercero requested a review from a team August 12, 2022 20:59

kibanamachine and others added 8 commits August 12, 2022 21:07

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

091cbab

…-fix'

continued cleanup, updated jest tests, added some tests

2489fda

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

d12ddd2

Merge branch 'update_viewer' of github.com:yctercero/kibana into upda…

8c519ad

…te_viewer

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

c06e497

…-fix'

passing down wrong rule value

d501eb0

Merge branch 'update_viewer' of github.com:yctercero/kibana into upda…

7b093d5

…te_viewer

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

90c191c

joepeeples reviewed Aug 16, 2022

View reviewed changes

x-pack/plugins/security_solution/public/common/components/exceptions/viewer/translations.ts Outdated Show resolved Hide resolved

yctercero added 4 commits August 16, 2022 10:09

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

bfe7776

addressing some lint/checks issues and starting cleanup

4444e5f

moving components under new folder structure

07cbd36

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

3bb8970

yctercero force-pushed the update_viewer branch from 208529c to 3bb8970 Compare August 25, 2022 15:20

yctercero commented Aug 25, 2022

View reviewed changes

yctercero added 3 commits August 25, 2022 11:16

cleanup

5626524

cleanup unused translations

6a31a42

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

6bdacea

yctercero marked this pull request as ready for review August 25, 2022 18:37