Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Security Solution][Endpoint][Response Actions] Do fuzzy search on given usernames for Actions Log #141239

Merged
merged 3 commits into from
Sep 22, 2022
Merged

[Security Solution][Endpoint][Response Actions] Do fuzzy search on given usernames for Actions Log #141239

merged 3 commits into from
Sep 22, 2022

Conversation

ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Sep 21, 2022 •
edited by kibanamachine
Loading

Summary

Allows fuzzy searcing on given usernames in the user filter input for Actions Log.

refs /pull/140975

Checklist

For maintainers

@ashokaditya ashokaditya added release_note:fix Team:Defend Workflows “EDR Workflows” sub-team of Security Solution OLM Sprint v8.5.0 labels Sep 21, 2022
@ashokaditya ashokaditya self-assigned this Sep 21, 2022
@ashokaditya ashokaditya force-pushed the fix/olm-4724-actions-log-do-fuzzy-search-usernames branch 3 times, most recently from f7a440f to c8c77b0 Compare September 21, 2022 16:28
@ashokaditya ashokaditya force-pushed the fix/olm-4724-actions-log-do-fuzzy-search-usernames branch from c8c77b0 to 756221f Compare September 21, 2022 16:30
@ashokaditya ashokaditya marked this pull request as ready for review September 21, 2022 16:30
@ashokaditya ashokaditya requested a review from a team as a code owner September 21, 2022 16:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

@kevinlog
Copy link
Contributor

Checked it out and tried it - works great!

I can fuzzy search usernames

image

I can also use a comma separated list

image

paul-tavares
paul-tavares approved these changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving, but I did leave a comment with (maybe) a concern with making this the default behaviour for the API

x-pack/plugins/security_solution/server/endpoint/utils/action_list_helpers.ts Outdated
];

if (userIds?.length) {
const kql = userIds.map((userId) => `user_id:*${userId}*`).join(' or ');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so that we are all clear: this implies that the API does not allow for searching on exact names. It will always do a wildcard search. Is that the intent?

Would it be better if we allowed the user to enter values (user names) along with the wildcards?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the UI perspective, I think it's easier to just let users fuzzy search so that they don't have to remember full usernames, but I see your point regarding the API.

@ashokaditya I see you add the * in this line of code. Could we just implicitly add them on the client side after they enter the values before we submit to the API? That way we still have the usability of users being able to just type in a partial name, but we don't tie the API to always being a fuzzy search. Then, in the future, if we ever wanted an exact match option it the UI, it's much easier to change that implementation in a non-breaking way.

If the above is hard, then let's just require * characters in the UI for a fuzzy search.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 069ee75

@ashokaditya
Allow match searches on the API
069ee75 
Do fuzzy search via the request

review suggestions
@kevinlog
Copy link
Contributor

Tried it again, still works great! Thanks for the changes @ashokaditya

image

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #2 / Alerts detection rules table auto-refresh should disable auto refresh when any rule selected and enable it after rules unselected
  • [job] [logs] Security Solution Tests #2 / Related integrations installed integrations: Amazon CloudFront, AWS CloudTrail, System, enabled integrations: Amazon CloudFront, Aws Cloudfront, System "before all" hook for "should display a badge with the installed integrations on the rule management page"

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 6.6MB 6.6MB +112.0B

History

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

cc @ashokaditya

@ashokaditya ashokaditya merged commit 15fc5b7 into elastic:main Sep 22, 2022
@ashokaditya ashokaditya deleted the fix/olm-4724-actions-log-do-fuzzy-search-usernames branch September 22, 2022 13:30
@kibanamachine kibanamachine mentioned this pull request Sep 22, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 22, 2022
@ashokaditya
[Security Solution][Endpoint][Response Actions] Do fuzzy search on gi…
685eb7c 
…ven usernames for Actions Log (elastic#141239)

* Do fuzzy search on given usernames

fixes elastic/security-team/issues/4724
refs elastic/pull/140975

* Allow `match` searches on the API

Do fuzzy search via the request

review suggestions

(cherry picked from commit 15fc5b7)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 22, 2022
@jloleysens
Merge branch 'main' into files-upload-use-self-destruct-flag
f178af8 
* main: (33 commits)
  Fix Next and Previous button on step screenshot carousel. (elastic#141422)
  [journeys] add a short delay after loading data (elastic#141437)
  skip failing test suite (elastic#140797)
  skip failing test suite (elastic#138776)
  Documents saved objects bulk delete API (elastic#141164)
  [Response Ops][Alerting] Defining default action params on connector type (elastic#141226)
  [ML] Explain Log Rate Spikes: Adds jest tests for query_utils/buildBaseFilterCriteria. (elastic#141213)
  [APM] Fix search bar suggestions (elastic#141101)
  close popover on click (elastic#141272)
  [Fleet] Update to use savedObjects bulkDelete for package policy delete (elastic#141276)
  [Security Solution][Endpoint][Response Actions] Do fuzzy search on given usernames for Actions Log (elastic#141239)
  [Graph] Fix guidance panel appearing for a moment when saving Graph (elastic#141228)
  [Fleet] Add experimental data stream features support to simplified package policy API (elastic#141288)
  Shameless copy of the retryTransientEsErrors from fleet (elastic#141246)
  [Security Solution][Fix]-Issue with disabled dataProvider (elastic#140735)
  Changing triggers actions ui routes to internal (elastic#141149)
  skip flaky test suit elastic#141356
  [SharedUX] Removing TODOs from KibanaPageTemplate (elastic#141043)
  [dashboard controls] skip failing test on cloud (elastic#141291)
  [Synthetics] unskip edit_monitor api integration tests (elastic#141277)
  ...
spalger pushed a commit that referenced this pull request Sep 23, 2022
@kibanamachine @ashokaditya
[8.5] [Security Solution][Endpoint][Response Actions] Do fuzzy search…
2973b22 
… on given usernames for Actions Log (#141239) (#141429)

Co-authored-by: Ashokaditya <[email protected]>
Co-authored-by: spalger <[email protected]>
@ashokaditya ashokaditya added release_note:skip Skip the PR/issue when compiling release notes release_note:fix and removed release_note:fix release_note:skip Skip the PR/issue when compiling release notes labels Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinlog kevinlog kevinlog approved these changes

@parkiino parkiino Awaiting requested review from parkiino parkiino was automatically assigned from elastic/security-onboarding-and-lifecycle-mgt

@paul-tavares paul-tavares Awaiting requested review from paul-tavares

@joeypoon joeypoon Awaiting requested review from joeypoon

Assignees

@ashokaditya ashokaditya

Labels
OLM Sprint release_note:fix Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.5.0 v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ashokaditya @elasticmachine @kevinlog @kibana-ci @kibanamachine @joeypoon @paul-tavares