[Security Solution][Alerts] Alert (+Investigation) User Assignment (#2504)#170579
[Security Solution][Alerts] Alert (+Investigation) User Assignment (#2504)#170579
Conversation
…he list of assigned users (#7647) (#166845) ## Summary Closes elastic/security-team#7647 This PR extends alert's schema. We add a new field `kibana.alert.workflow_assignee_ids` where assignees will live.
# Conflicts: # packages/kbn-alerts-as-data-utils/src/schemas/generated/alert_schema.ts # packages/kbn-alerts-as-data-utils/src/schemas/generated/security_schema.ts
…7661) (#167079) ## Summary Closes elastic/security-team#7661 This PR adds Alert user assignment UI within alerts table. https://github.com/elastic/kibana/assets/2700761/df81928a-b0c7-46ba-98b3-3803774ed239
# Conflicts: # x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_alerts/alert_assignees.cy.ts
…#169367) ## Summary Fixes the bug where we would apply only visible user profile selections instead of taking into account those which are not visible during the search within component.
## Summary Fix broken tests introduced in #169367
… flyout component (#7662) (#169508) ## Summary Closes elastic/security-team#7662 This PR adds Alert user assignment UI within alert's details flyout component. https://github.com/elastic/kibana/assets/2700761/b84299d7-5d65-4e9a-8836-807f51c0bbc7 This PR is a replacement to #168467 since I broke that one with wrong merges from main. cc @PhilippeOberti
## Summary Closes elastic/security-team#7820 This PR adds "filter by assignees" component to the Alerts page. https://github.com/elastic/kibana/assets/2700761/3f2c27bd-503f-4f0d-86ce-4e1f949db182
…folder (#169645) ## Summary These changes move user profiles hooks into a separate folder. Before it was part of the `containers/detection_engine/alerts/`.
# Conflicts: # x-pack/plugins/security_solution/public/timelines/components/side_panel/event_details/flyout/index.tsx
# Conflicts: # x-pack/test/security_solution_cypress/cypress/tasks/navigation.ts
marshallmain
left a comment
There was a problem hiding this comment.
Looks pretty good overall - I think the assignees panel would benefit from some refactoring to make the data model and intended usage clearer.
x-pack/plugins/security_solution/public/common/components/filter_group/filter_by_assignees.tsx
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/common/components/filter_group/filter_by_assignees.tsx
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/common/components/assignees/assignees_apply_panel.tsx
Show resolved
Hide resolved
yctercero
left a comment
There was a problem hiding this comment.
Thank you for this huge effort, including a test plan, testing party and the automated tests.
Once #172285 is merged to add in the openAPI specs, I think it's good to go! Please ensure that one is merged in before merging this one. It looks like maybe there's some further component refactoring that is suggested. As long as @marshallmain is ok with it, I think the refactor can be followed up on.
Given the timezone differences, I'm going ahead and LGTM-ing.
## Summary With these changes we specify the schemas for new alert assignments APIs with OpenAPI. cc @yctercero @marshallmain
# Conflicts: # x-pack/plugins/security_solution/server/routes/index.ts
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @e40pud |
marshallmain
left a comment
There was a problem hiding this comment.
LGTM, component refactoring improvements to be done in a follow up
## Summary With this changes I make "suggestions user profiles" API to be internal instead of public. We did not reveal it via documentation and it is better to mark it as an internal API. This API was introduced in this PR #170579 and was not released yet. I also realised that currently the route does not reflect the fact that it is finding user profiles: `/api/detection_engine/signals/_find` The new version will have `users` as part of the path: `/internal/detection_engine/users/_find`
## Summary With this changes I make "suggestions user profiles" API to be internal instead of public. We did not reveal it via documentation and it is better to mark it as an internal API. This API was introduced in this PR elastic#170579 and was not released yet. I also realised that currently the route does not reflect the fact that it is finding user profiles: `/api/detection_engine/signals/_find` The new version will have `users` as part of the path: `/internal/detection_engine/users/_find` (cherry picked from commit 7e168c7)
…173249) # Backport This will backport the following commits from `main` to `8.12`: - [Switch "suggest user profiles" API to internal use (#173141)](#173141) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ievgen Sorokopud","email":"ievgen.sorokopud@elastic.co"},"sourceCommit":{"committedDate":"2023-12-13T10:10:10Z","message":"Switch \"suggest user profiles\" API to internal use (#173141)\n\n## Summary\r\n\r\nWith this changes I make \"suggestions user profiles\" API to be internal\r\ninstead of public. We did not reveal it via documentation and it is\r\nbetter to mark it as an internal API.\r\n\r\nThis API was introduced in this PR\r\nhttps://github.com//pull/170579 and was not released yet.\r\n\r\nI also realised that currently the route does not reflect the fact that\r\nit is finding user profiles:\r\n\r\n`/api/detection_engine/signals/_find`\r\n\r\nThe new version will have `users` as part of the path:\r\n\r\n`/internal/detection_engine/users/_find`","sha":"7e168c7fa9af17f80d3daa53a632754efb553c36","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team: SecuritySolution","backport:prev-minor","Team:Detection Engine","v8.13.0"],"number":173141,"url":"https://github.com/elastic/kibana/pull/173141","mergeCommit":{"message":"Switch \"suggest user profiles\" API to internal use (#173141)\n\n## Summary\r\n\r\nWith this changes I make \"suggestions user profiles\" API to be internal\r\ninstead of public. We did not reveal it via documentation and it is\r\nbetter to mark it as an internal API.\r\n\r\nThis API was introduced in this PR\r\nhttps://github.com//pull/170579 and was not released yet.\r\n\r\nI also realised that currently the route does not reflect the fact that\r\nit is finding user profiles:\r\n\r\n`/api/detection_engine/signals/_find`\r\n\r\nThe new version will have `users` as part of the path:\r\n\r\n`/internal/detection_engine/users/_find`","sha":"7e168c7fa9af17f80d3daa53a632754efb553c36"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/173141","number":173141,"mergeCommit":{"message":"Switch \"suggest user profiles\" API to internal use (#173141)\n\n## Summary\r\n\r\nWith this changes I make \"suggestions user profiles\" API to be internal\r\ninstead of public. We did not reveal it via documentation and it is\r\nbetter to mark it as an internal API.\r\n\r\nThis API was introduced in this PR\r\nhttps://github.com//pull/170579 and was not released yet.\r\n\r\nI also realised that currently the route does not reflect the fact that\r\nit is finding user profiles:\r\n\r\n`/api/detection_engine/signals/_find`\r\n\r\nThe new version will have `users` as part of the path:\r\n\r\n`/internal/detection_engine/users/_find`","sha":"7e168c7fa9af17f80d3daa53a632754efb553c36"}}]}] BACKPORT--> Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>
…o make the data model and intended usage clearer (#8164) (#176442) ## Summary These changes are followup for [alert assignments feature](elastic/security-team#2504) and addresses feedback described in elastic/security-team#8164 Addressed requests: 1. Clearer data model within filter [filter_by_assignees.tsx](#170579 (comment)) 2. [Decouple](#170579 (comment)) `AssigneesApplyPanel` and `Apply` button As part of this PR, I also fixed the issue where user was able to trigger apply assignments action even when there were no changes done to the list of assignees #173262. Apply button will be disabled as long as there are no changes. https://github.com/elastic/kibana/assets/2700761/45b02fb5-f85e-42d6-9411-5e040c99af68 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ESS 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5157) - [Serverless 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5135) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
…o make the data model and intended usage clearer (elastic#8164) (elastic#176442) ## Summary These changes are followup for [alert assignments feature](elastic/security-team#2504) and addresses feedback described in elastic/security-team#8164 Addressed requests: 1. Clearer data model within filter [filter_by_assignees.tsx](elastic#170579 (comment)) 2. [Decouple](elastic#170579 (comment)) `AssigneesApplyPanel` and `Apply` button As part of this PR, I also fixed the issue where user was able to trigger apply assignments action even when there were no changes done to the list of assignees elastic#173262. Apply button will be disabled as long as there are no changes. https://github.com/elastic/kibana/assets/2700761/45b02fb5-f85e-42d6-9411-5e040c99af68 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ESS 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5157) - [Serverless 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5135) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
…o make the data model and intended usage clearer (elastic#8164) (elastic#176442) ## Summary These changes are followup for [alert assignments feature](elastic/security-team#2504) and addresses feedback described in elastic/security-team#8164 Addressed requests: 1. Clearer data model within filter [filter_by_assignees.tsx](elastic#170579 (comment)) 2. [Decouple](elastic#170579 (comment)) `AssigneesApplyPanel` and `Apply` button As part of this PR, I also fixed the issue where user was able to trigger apply assignments action even when there were no changes done to the list of assignees elastic#173262. Apply button will be disabled as long as there are no changes. https://github.com/elastic/kibana/assets/2700761/45b02fb5-f85e-42d6-9411-5e040c99af68 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ESS 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5157) - [Serverless 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5135) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
…ssignees (#219460) ## Summary This PR implements a similar change that was just merged a few hours ago. While [that change](#219410) was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in [this PR](#170579), and I believe the code was using the similar logic of [the alert tag PR](#157786). The issue is related to the fact that we have a `useRef` for a function that is returned before the `useEffect` in the same hook runs, and setting the value of the function returned is happening within that `useEffect`. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function. This PR fixes that by returning the function directly. Here's a video showing that the functionality still works correctly for bulk actions: https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea And also for normal row actions: https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…ssignees (elastic#219460) ## Summary This PR implements a similar change that was just merged a few hours ago. While [that change](elastic#219410) was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in [this PR](elastic#170579), and I believe the code was using the similar logic of [the alert tag PR](elastic#157786). The issue is related to the fact that we have a `useRef` for a function that is returned before the `useEffect` in the same hook runs, and setting the value of the function returned is happening within that `useEffect`. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function. This PR fixes that by returning the function directly. Here's a video showing that the functionality still works correctly for bulk actions: https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea And also for normal row actions: https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…ssignees (elastic#219460) ## Summary This PR implements a similar change that was just merged a few hours ago. While [that change](elastic#219410) was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in [this PR](elastic#170579), and I believe the code was using the similar logic of [the alert tag PR](elastic#157786). The issue is related to the fact that we have a `useRef` for a function that is returned before the `useEffect` in the same hook runs, and setting the value of the function returned is happening within that `useEffect`. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function. This PR fixes that by returning the function directly. Here's a video showing that the functionality still works correctly for bulk actions: https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea And also for normal row actions: https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 254477e)
…ssignees (elastic#219460) ## Summary This PR implements a similar change that was just merged a few hours ago. While [that change](elastic#219410) was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in [this PR](elastic#170579), and I believe the code was using the similar logic of [the alert tag PR](elastic#157786). The issue is related to the fact that we have a `useRef` for a function that is returned before the `useEffect` in the same hook runs, and setting the value of the function returned is happening within that `useEffect`. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function. This PR fixes that by returning the function directly. Here's a video showing that the functionality still works correctly for bulk actions: https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea And also for normal row actions: https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…ssignees (elastic#219460) ## Summary This PR implements a similar change that was just merged a few hours ago. While [that change](elastic#219410) was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in [this PR](elastic#170579), and I believe the code was using the similar logic of [the alert tag PR](elastic#157786). The issue is related to the fact that we have a `useRef` for a function that is returned before the `useEffect` in the same hook runs, and setting the value of the function returned is happening within that `useEffect`. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function. This PR fixes that by returning the function directly. Here's a video showing that the functionality still works correctly for bulk actions: https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea And also for normal row actions: https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 254477e)
…ssignees (elastic#219460) ## Summary This PR implements a similar change that was just merged a few hours ago. While [that change](elastic#219410) was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in [this PR](elastic#170579), and I believe the code was using the similar logic of [the alert tag PR](elastic#157786). The issue is related to the fact that we have a `useRef` for a function that is returned before the `useEffect` in the same hook runs, and setting the value of the function returned is happening within that `useEffect`. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function. This PR fixes that by returning the function directly. Here's a video showing that the functionality still works correctly for bulk actions: https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea And also for normal row actions: https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 254477e)
Summary
With this PR we introduce a new Alert User Assignment feature:
We decided to develop this feature on a separate branch. This gives us ability to make sure that it is thoroughly tested and we did not break anything in production. Since there is a data scheme changes involved we decided that it will be a better approach. cc @yctercero
Testing notes
In order to test assignments you need to create a few users. Then for users to appear in user profiles dropdown menu you need to activate them by login into those account at least once.
user-assignments-720.mov
Main ticket https://github.com/elastic/security-team/issues/2504
Bugfixes
Enhancements
Checklist