[Security Solution][Endpoint] Response Console framework support for argument value selectors #148693

paul-tavares · 2023-01-10T23:05:26Z

Summary

PR adds the ability for Commands to define custom value selectors - components that will be rendered as the value when the Argument name is entered in the Console. These Argument Selectors can then provide the user with a better UX for selecting data that is not easily entered in the console via text input.
Introduces a File picker Argument Selector (not yet being used by real commands) that will be used in upcoming features.
Introduces a new mustHaveValue property to the definition of a command's argument. Usage (from JSDocs):

/**
   * If argument (when used) should have a value defined by the user.
   * Default is `false` which mean that argument can be entered without any value - internally the
   * value for the argument will be a boolean `true`.
   * When set to `true` the argument is expected to have a value that is non-boolean
   * In addition, the following options can be used with this parameter to further validate the user's input:
   *
   * - `non-empty-string`: user's value must be a string whose length is greater than zero. Note that
   *   the value entered will first be `trim()`'d.
   * - `number`: user's value will be converted to a Number and ensured to be a `safe integer`
   * - `number-greater-than-zero`: user's value must be a number greater than zero
   */

PR Testing

Since there are currently no commands that use this new framework, the following can be used access a test command in the console:

On the Endpoint List page, add the following to the URL: &show_upload=1. Example: http://localhost:5601/app/security/administration/endpoints?page_index=0&page_size=10&show_upload=1
use the upload command for dev testing purposes only. It is defined with two arguments that have ArgumentSelector: --file and --mock

** 🔴 NOTE:**: this dev command will be deleted from the code prior to merging this PR or soon after it is merged.

More about Argument Selectors

When an argument, having an SelectorComponent defined, is typed into the console, the UI provided by that selector component will be inserted as the value to the argument name. It is up to the Argument Selector to provide the desired UI to the user. Argument Selectors have the following behaviors:

The argument name typed by the user will be replaced with a UI that show the argument name as readonly and inserts the argument selector component into the value section. Example: user types --file, which is replaced in the console's input with --file=[argument selector UI here]
[Left] / [Right] keys will jump over the entire argument. It does not move through each letter once the argument name has been replaced with the UI for displaying the argument selector
[Delete] / [Backspace] keys will delete the entire argument from the input area
Display content of Arguments that are displayed with Argument Selectors can not be selected with the mouse (click and highlight)
Command input history (Menu shown when [ArrowUP] is clicked) and the console's command output area (after a command has been [Enter]'d) will display a text representation of the value provided by the Argument Selector.
When selecting a previously entered command from the input history ([ArrowUp]), the textual representation of any argument with Argument Selector will not be applied to the console's input area. Only the argument name will be entered (triggering the selector to be used again)

Some limitations

Some current limitation that may be addressed in subsequent PR(s):

Highlight of text and replacement (via keyboard key click) still works - as long as the selection does not traverse an argument that is using an argument selector.
When multiples of the same argument (that have value selectors) are used, and one instance of this argument already exists, and the user attempts to enter another BEFORE the existing instance, the newly entered argument will "steal" the value from the now second instance
Pasting data into the console's input area that includes argument's that are defined with an Argument selector will work as expected with the exception that if the pasted data had a value for that argument, it will be displayed as well (not removed). It is up to the user to delete it.

NOTE
The screen capture below shows argument selectors for two arguments - file and mock. These are meant to just show the interaction and usage of the argument selectors. The file argument selector UX could changed once we get feedback from the UX team.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…selectors (not working yet)

…EnteredState` action

… on command input

…mand + fix parseInput in store

…ipulating strings

…alue selectors

…electors is updated correctly

… data value validations

elasticmachine · 2023-01-18T20:24:45Z

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

…port

ashokaditya

Took an initial pass at reviewing this. The file selector popup UX looks awesome when I tried it out. 🤩

I noticed something that you probably already know but I thought I'll mention it since it's not listed in the limitations section. That is, whenever the file selector is empty (no files are selected) the popup opens up on every cursor move (left or right) no matter where the cursor is and even if I close the popup. I would expect that once I close the popup it should not show up unless I click on the file argument value ("click to select file"). See this clip. Once a file is selected the popup shows up again only when the file argument value ("click to select file") is clicked.

For later UX consideration,

Once a file is selected and the file selector popup is opened again I see the same message as when no file was selected. Maybe show a different message like "Select a file to replace the current one" or similar.
When the command is executed, the --file argument value shows just the file name. Should we show the absolute path of the file instead if that's useful info?

ashokaditya · 2023-01-24T16:06:58Z

.../security_solution/public/management/components/console_argument_selectors/file_selector.tsx

+  .popoverAnchor {
+    display: block;
+  }


Curios why the display style is needed?

at some point I needed this, but looks like with the most current code, it's no longer needed. I will deleted it. Thanks for the feedback.

ashokaditya · 2023-01-24T16:31:04Z

...components/console/components/console_state/state_update_handlers/handle_execute_command.tsx

@@ -381,6 +323,70 @@ export const handleExecuteCommand: ConsoleStoreReducer<
        );
      }

+      if (argDefinition.mustHaveValue !== undefined && argDefinition.mustHaveValue !== false) {


This file is now showing me cyclomatic complexity warning now and that it is at 32 now and exceeds 20 which is the limit. Consider adding an eslint ignore.

It's a warning right? so no need to ignore it. I'll have to come back at some point and break it up

ashokaditya · 2023-01-24T16:44:06Z

x-pack/plugins/security_solution/public/management/components/console/types.ts

+  /**
+   * If the argument is required to be entered by the user. NOTE that this will only validate that
+   * the user has entered the argument name - it does not validate that the argument must have a
+   * value. Argument's that have no value entered by the user have (by default) a value of


nit: typo Arguments...

gergoabraham

Awesome modification! I got nothing major with my current understanding, only small findings/questions - but those can be managed in subsequent PRs.

gergoabraham · 2023-01-24T11:37:19Z

x-pack/plugins/security_solution/public/management/components/console/types.ts

+  /**
+   * The instance of the argument in the current command. This is a zero-based number indicating
+   * which instance of the argument is being rendered.
+   */
+  argInstance: number;


the name for this field is a bit misleading to me: should it be maybe argIndex?

I guess I could have renamed it argIndex. I went with argInstance to kind of match the value I was already using internally and I just exposed it to the argument value component. "Instance" to indicate which instance of the argument usage is being rendered.

I see, thanks! would you consider to rename it? reading instance indicates to me that this is an object, an object instance of a class, and definitely not a number.

Ohh. I see what you meant by instance. Yes, I will rename it

.../security_solution/public/management/components/console_argument_selectors/file_selector.tsx

gergoabraham · 2023-01-24T13:20:42Z

...anagement/components/console/components/console_state/state_update_handlers/translations.tsx

+    return i18n.translate(
+      'xpack.securitySolution.console.commandValidation.mustHaveNonBlankValue',
+      {
+        defaultMessage: 'Argument --${argName} must have a value',


is this intentionally the same as the string for mustHaveValue?

🤦

No. It should not be a duplicate. I will change to use the one above.

gergoabraham · 2023-01-24T13:47:31Z

...components/console/components/console_state/state_update_handlers/handle_input_area_state.ts

 type InputAreaStateAction = ConsoleDataAction & {
  type:
    | 'updateInputPopoverState'
    | 'updateInputHistoryState'
    | 'clearInputHistoryState'
    | 'updateInputTextEnteredState'
    | 'updateInputPlaceholderState'
-    | 'setInputState';
+    | 'setInputState'
+    | 'updateInputCommandArgState';
 };

 export const handleInputAreaState: ConsoleStoreReducer<InputAreaStateAction> = (


for this function I'm seeing the eslint(complexity) warning: Arrow function has a complexity of 31. Maximum allowed is 20.

I think this could be refactored in a subsequent PR

correct. This one also needs to be refactored and broken down

gergoabraham · 2023-01-24T13:48:58Z

...components/console/components/console_state/state_update_handlers/handle_execute_command.tsx

@@ -131,16 +129,13 @@ const createCommandHistoryEntry = (
 export const handleExecuteCommand: ConsoleStoreReducer<


this also has too large cyclomatic complexity according to the eslint rule

yeah, I been meaning to break it out into separate functions to do the validations.

...lic/management/components/console/components/console_state/hooks/use_stored_input_history.ts

...solution/public/management/components/console/components/command_input/lib/entered_input.tsx

…le-arg-value-selector-support

…selector-support' into task/olm-5711-console-arg-value-selector-support

paul-tavares · 2023-01-24T20:32:43Z

Thanks @ashokaditya for the feedback.

Re:

"...the file selector is empty (no files are selected) the popup opens up on every cursor move (left or right) no matter where the cursor is and even if I close the popup.

Yeah, I noticed this as well. I did not spent too much time on the UX of the file picker as part of this PR and I do know that particular selector does need to be adjusted to be "smarter" about the states it supports. I might even have to introduce a "state store" per argument selector so that they can keep state for their own use cases. I added this item to the internal issue that is tracking this work for consideration

**re: **

"1. Once a file is selected and the file selector popup is opened again I see the same message as when no file was selected. Maybe show a different message like "Select a file to replace the current one" or similar.

I did not see the need to change the text since it is generic. Do you find that it could be confusing to the user as to what the outcome will be by selecting another file?

Re:

When the command is executed, the --file argument value shows just the file name. Should we show the absolute path of the file instead if that's useful info?

I don't think we have access to the full file path from the File DOM interface.

paul-tavares · 2023-01-24T22:19:14Z

@ashokaditya

FYI - for this item:

"...the file selector is empty (no files are selected) the popup opens up on every cursor move (left or right) no matter where the cursor is and even if I close the popup.

I have made the change to the framework and the File picker to keep state, so the "I'm always open when you move the cursor" will not happen anymore.

Thansk again for the feedback.

…le-arg-value-selector-support

kibana-ci · 2023-01-25T15:44:41Z

💛 Build succeeded, but was flaky

Failed CI Steps

FTR Configs #33

Test Failures

[job] [logs] FTR Configs #33 / spaces api with security copy to spaces user with no access from the default space single-namespace types "after each" hook for "should return 403 when copying to space without conflicts or references"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3557	3563	+6

Async chunks

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

id	before	after	diff
`securitySolution`	12.8MB	12.8MB	+11.0KB

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`securitySolution`	75	76	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	500	501	+1

History

💚 Build #102372 succeeded 304bea4
💛 Build #102330 was flaky 55816db
💚 Build #102250 succeeded 8cab164
💔 Build #102182 failed 3a2a7be
💔 Build #102002 failed 26f78a9
💔 Build #101858 failed 19772a7

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

cc @paul-tavares

ashokaditya

🚢 it!

...k/plugins/security_solution/public/management/components/endpoint_responder/lib/dev_only.tsx

paul-tavares added 18 commits January 4, 2023 16:09

Type definitions for Command Argument Value selector component

49bd6b7

Argument File Selector component

b7a5fe7

Add test command to console (upload)

52db251

Add enteredCommand structure to the console's input store state

a124c8f

Refactor input state properties of left/right of cursor text

4f20a11

refactor the types for parsing user input into a types.ts file

4cce1cd

add parsedInput to the input store

61cc541

EnteredInput class and initial implementation for displaying value …

5cdbe93

…selectors (not working yet)

Add complete input state to the callback argument of `updateInputText…

3cfbd6d

…EnteredState` action

Add methods to EnteredInput to retrieve rendering values and use them…

56fae04

… on command input

save argument value selector state to store

7e9d23b

Change execute command action to include parsed input and entered com…

217a405

…mand + fix parseInput in store

Adjust output of the upload execution dev component

1f5a3ea

Fix hints for known commands

a31b35a

Refactor EnteredInput to use an array of input items instead of man…

db80987

…ipulating strings

EnteredInput support for keybaord movement around argument's with v…

5f82e7b

…alue selectors

Ensure Entered Command is reset in store when command is entered

5815f82

Better handling of text selection replacement (but not done yet)

01c796e

paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.7.0 labels Jan 10, 2023

paul-tavares self-assigned this Jan 10, 2023

paul-tavares added 8 commits January 11, 2023 12:13

Fix EnteredInput to handle when cursor is between replacement value

123f1b2

Test file for EnteredInput

f0f6009

Fix setArgSelectorValueToParsedArgs() to ensure that any arg with s…

dd2b2eb

…electors is updated correctly

add new argument definition property (mustHaveValue) and associated…

fd70ff9

… data value validations

update dev command to use new argument value validations

dfc997b

Fix some types

9cd92a1

Fix text replacement for when argument value selectors are present

5fda48a

extracted i18n translations out of command execution module

61d1e91

paul-tavares marked this pull request as ready for review January 18, 2023 20:24

paul-tavares requested a review from a team as a code owner January 18, 2023 20:24

paul-tavares requested review from pzl and gergoabraham January 18, 2023 20:24

paul-tavares requested a review from ashokaditya January 18, 2023 20:24

paul-tavares and others added 6 commits January 18, 2023 16:43

Fix tests

9199c53

Merge branch 'main' into task/olm-5711-console-arg-value-selector-sup…

cc563c1

…port

Merge branch 'main' into task/olm-5711-console-arg-value-selector-sup…

19772a7

…port

Merge branch 'main' into task/olm-5711-console-arg-value-selector-sup…

26f78a9

…port

Merge branch 'main' into task/olm-5711-console-arg-value-selector-sup…

3a2a7be

…port

Merge branch 'main' into task/olm-5711-console-arg-value-selector-sup…

8cab164

…port

ashokaditya reviewed Jan 24, 2023

View reviewed changes

gergoabraham approved these changes Jan 24, 2023

View reviewed changes

paul-tavares added 3 commits January 24, 2023 13:57

Merge remote-tracking branch 'upstream/main' into task/olm-5711-conso…

13e957f

…le-arg-value-selector-support

Fixes from PR reviews

9eac198

Merge remote-tracking branch 'origin/task/olm-5711-console-arg-value-…

55816db

…selector-support' into task/olm-5711-console-arg-value-selector-support

paul-tavares requested a review from ashokaditya January 24, 2023 20:32

Add store data to ArgumentSelector components

304bea4

paul-tavares added 2 commits January 25, 2023 09:09

rename argInstance to argIndex

e3e7413

Merge remote-tracking branch 'upstream/main' into task/olm-5711-conso…

c3c1606

…le-arg-value-selector-support

ashokaditya approved these changes Jan 25, 2023

View reviewed changes

ashokaditya reviewed Jan 25, 2023

View reviewed changes

...k/plugins/security_solution/public/management/components/endpoint_responder/lib/dev_only.tsx Show resolved Hide resolved

paul-tavares merged commit 9ac065a into elastic:main Jan 25, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jan 25, 2023

paul-tavares deleted the task/olm-5711-console-arg-value-selector-support branch January 25, 2023 21:06

paul-tavares mentioned this pull request May 10, 2023

[Security Solution][Endpoint] add upload console response action #157208

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Endpoint] Response Console framework support for argument value selectors #148693

[Security Solution][Endpoint] Response Console framework support for argument value selectors #148693

paul-tavares commented Jan 10, 2023 •

edited

Loading

elasticmachine commented Jan 18, 2023

ashokaditya left a comment

ashokaditya Jan 24, 2023

paul-tavares Jan 24, 2023

ashokaditya Jan 24, 2023

paul-tavares Jan 24, 2023

ashokaditya Jan 24, 2023

gergoabraham left a comment

gergoabraham Jan 24, 2023

paul-tavares Jan 24, 2023

gergoabraham Jan 25, 2023

paul-tavares Jan 25, 2023

gergoabraham Jan 24, 2023

paul-tavares Jan 24, 2023

gergoabraham Jan 24, 2023

paul-tavares Jan 24, 2023

gergoabraham Jan 24, 2023

paul-tavares Jan 24, 2023

paul-tavares commented Jan 24, 2023

paul-tavares commented Jan 24, 2023

kibana-ci commented Jan 25, 2023

ESLint disabled in files

Total ESLint disabled count

ashokaditya left a comment

		@@ -131,16 +129,13 @@ const createCommandHistoryEntry = (
		export const handleExecuteCommand: ConsoleStoreReducer<

[Security Solution][Endpoint] Response Console framework support for argument value selectors #148693

[Security Solution][Endpoint] Response Console framework support for argument value selectors #148693

Conversation

paul-tavares commented Jan 10, 2023 • edited Loading

Summary

PR Testing

More about Argument Selectors

Checklist

elasticmachine commented Jan 18, 2023

ashokaditya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gergoabraham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares commented Jan 24, 2023

paul-tavares commented Jan 24, 2023

kibana-ci commented Jan 25, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

ESLint disabled in files

Total ESLint disabled count

History

ashokaditya left a comment

Choose a reason for hiding this comment

paul-tavares commented Jan 10, 2023 •

edited

Loading