[Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS operator for trusted apps creation/editing #175356

parkiino · 2024-01-23T23:20:16Z

Summary

Adds updated warning messaging for trusted apps entries that use wildcards *? with the "IS" operator
Three different warnings: callout, individual entry item warnings and a final confirmation modal when the user tries to add a trusted app with ineffective IS / wildcard combination entry.
Unit tests

Screenshots

…d-warning

caitlinbetz · 2024-02-13T17:06:10Z

hey @parkiino this looks good to me, but wondering if we should keep the same blue color for the "Add" button (vs changing it red per screen shots). Let me know if you feel differently.

cc @dasansol92

dasansol92 · 2024-02-13T17:11:07Z

but wondering if we should keep the same blue color for the "Add" button (vs changing it red per screen shots). Let me know if you feel differently.

cc @dasansol92

I also think it should be blue for a save action confirmation.
Is the spacing between the callout and the form (AND button) and other warnings (soft warnings below) correct? It seems to me there should be more space around the warning callout.

let me know what do you think.

cc: @parkiino @caitlinbetz

…fix'

parkiino · 2024-02-13T20:55:47Z

but wondering if we should keep the same blue color for the "Add" button (vs changing it red per screen shots). Let me know if you feel differently.
cc @dasansol92

I also think it should be blue for a save action confirmation. Is the spacing between the callout and the form (AND button) and other warnings (soft warnings below) correct? It seems to me there should be more space around the warning callout.

let me know what do you think.

cc: @parkiino @caitlinbetz

i can adjust this to have more space

paul-tavares

The changes to the ArtifactListPage and Trusted apps looks good.

Are the changes done to kbn-securitysolution-* packages need for this PR which is only changing the TrustedApps form (which I don't think uses the forms from the packages you changed)

anyway, approving and deferring the review of those packages to the code owners of them

paul-tavares · 2024-02-13T20:59:21Z

...ecuritysolution-exception-list-components/src/wildcard_with_wrong_operator_callout/index.tsx

+            operator: <strong>Change the operator</strong>,
+            matches: <strong>matches</strong>,


these strings values need to be i18n as well

…d-warning

rylnd

I had one test fix that should be incorporated, and I agree with the i18n comments.

I had a nit about some react control flow stuff, but that isn't pressing.

Just fix that test and i18n and this should be good on DE's side!

rylnd · 2024-02-13T22:44:58Z

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

@@ -120,6 +125,13 @@ export const AutocompleteFieldMatchComponent: React.FC<AutocompleteFieldMatchPro
    [setError, onError]
  );

+  const handleWarning = useCallback(
+    (warn: Warning | undefined): void => {
+      onWarning(warn !== undefined);


So if one were to pass onWarning to this component, but not warning, the parent setWarningsExist is always going to be false and nothing will happen, right? Is that the intended behavior?

The control flow here is a little odd; if the component is accepting onWarning I would expect the rendering of the warning to be the parent's responsibility, but it seems like it's not (or is helpText={warning} something else?).

I guess this all works for now, so nothing's needed to be done, but I wonder if this architecture might be causing some extra renders 🤔

packages/kbn-securitysolution-utils/src/path_validations/index.test.ts

…d-warning

kibana-ci · 2024-02-14T04:43:25Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 632342f

Failed CI Steps

Test Failures

[job] [logs] Defend Workflows Cypress Tests on Serverless #5 / Form User with access can create and save an endpoint response action create and save endpoint response action inside of a rule create and save endpoint response action inside of a rule
[job] [logs] FTR Configs #73 / security APIs - Audit Log Audit Log logs audit events when reading and writing saved objects

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	4983	4985	+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-exception-list-components`	93	94	+1
`@kbn/securitysolution-utils`	39	43	+4
total			+5

Async chunks

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

id	before	after	diff
`securitySolution`	11.5MB	11.5MB	+3.7KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-exception-list-components`	104	105	+1
`@kbn/securitysolution-utils`	44	48	+4
total			+5

ESLint disabled line counts

id	before	after	diff
`securitySolution`	476	477	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	550	551	+1

History

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

… operator for trusted apps creation/editing (elastic#175356) ## Summary - [x] Adds updated warning messaging for trusted apps entries that use wildcards `*?` with the "IS" operator - [x] Three different warnings: callout, individual entry item warnings and a final confirmation modal when the user tries to add a trusted app with ineffective IS / wildcard combination etnry. - [x] Unit tests # Screenshots <img width="829" alt="image" src="https://github.com/elastic/kibana/assets/56409205/c7beec62-a249-4535-ac0b-34f9be57f542"> <img width="1649" alt="image" src="https://github.com/elastic/kibana/assets/56409205/22f38f1b-7e6b-4b69-8d03-4d74d8674fa6"> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

parkiino added 6 commits January 11, 2024 23:27

wip

a097dfd

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

b694154

…d-warning

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

8c2d8c3

…d-warning

show wildcard with is operator warning in red text

5587baf

fix up warning and add test for event filter warning

f2e0b31

missed these files in the last commit

8340cf1

parkiino mentioned this pull request Jan 23, 2024

Defend TrustedApps allow use of ineffective IS operator with wildcarded paths #172212

Closed

3 tasks

parkiino added 11 commits January 29, 2024 15:11

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

ea5a152

…d-warning

wip add callout to trusted apps

033d552

trusted app entry specific warning for wildcard with wrong operator

5e5b1d2

TA: add callout only once at the top, some unit tests

35149a0

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

075b64d

…d-warning

wip confirm modal component and connecting flyout

147d98c

setup modal, connect form modal info to flyout

c349639

clean up confirm modal, add correct wording, wip show modal

d65051f

have a working dummy modal

50c74fa

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

0369bae

…d-warning

working modals

32c4528

parkiino changed the title ~~Task/is and wildcard warning~~ [Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS operator for trusted apps creation/editing Feb 12, 2024

fix text, rename file

b56ecb5

parkiino marked this pull request as ready for review February 12, 2024 22:04

parkiino requested review from a team as code owners February 12, 2024 22:04

parkiino requested review from joeypoon, dasansol92 and e40pud February 12, 2024 22:04

parkiino added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.13.0 labels Feb 13, 2024

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

db6e2e3

…d-warning

parkiino and others added 3 commits February 13, 2024 14:24

change confirm modal button to blue

04bd256

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

0865a5c

…fix'

add flyout confirm modal unit test

e619e7e

paul-tavares approved these changes Feb 13, 2024

View reviewed changes

parkiino added 2 commits February 13, 2024 17:33

fix i18n errors

0b69735

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

fe6c31f

…d-warning

rylnd approved these changes Feb 13, 2024

View reviewed changes

joeypoon approved these changes Feb 13, 2024

View reviewed changes

parkiino added 8 commits February 13, 2024 18:10

fix path validations test

7de3ea6

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

8d79564

…d-warning

add missing onwarnigns to test

7f2939e

add spacer and switch posiition of callout

ee3e3ee

one more missing on warning

42e3255

last 2 unit tests

3d7f180

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

a1eb6bc

…d-warning

fix test text

391d41a

parkiino enabled auto-merge (squash) February 14, 2024 01:16

parkiino added 3 commits February 13, 2024 20:18

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

cc62417

…d-warning

remove files not related to trusted apps warnings

4880404

Merge remote-tracking branch 'upstream/main' into task/is-and-wildcar…

632342f

…d-warning

parkiino merged commit 9a73eb4 into elastic:main Feb 14, 2024
38 checks passed

parkiino deleted the task/is-and-wildcard-warning branch March 25, 2024 05:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS operator for trusted apps creation/editing #175356

[Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS operator for trusted apps creation/editing #175356

parkiino commented Jan 23, 2024 •

edited

Loading

caitlinbetz commented Feb 13, 2024

dasansol92 commented Feb 13, 2024

parkiino commented Feb 13, 2024

paul-tavares left a comment

paul-tavares Feb 13, 2024

rylnd left a comment

rylnd Feb 13, 2024

kibana-ci commented Feb 14, 2024

API count

ESLint disabled line counts

Total ESLint disabled count

		operator: <strong>Change the operator</strong>,
		matches: <strong>matches</strong>,

[Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS operator for trusted apps creation/editing #175356

[Security Solution][Artifacts][Trusted Apps] Wildcard warning with IS operator for trusted apps creation/editing #175356

Conversation

parkiino commented Jan 23, 2024 • edited Loading

Summary

Screenshots

caitlinbetz commented Feb 13, 2024

dasansol92 commented Feb 13, 2024

parkiino commented Feb 13, 2024

paul-tavares left a comment

Choose a reason for hiding this comment

paul-tavares Feb 13, 2024

Choose a reason for hiding this comment

rylnd left a comment

Choose a reason for hiding this comment

rylnd Feb 13, 2024

Choose a reason for hiding this comment

kibana-ci commented Feb 14, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

ESLint disabled line counts

Total ESLint disabled count

History

parkiino commented Jan 23, 2024 •

edited

Loading