-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Started reviewing today but didn't get far, will finish tomorrow
...common/api/detection_engine/alert_assignees/set_alert_assignees/set_alert_assignees_route.ts
Outdated
Show resolved
Hide resolved
...ugins/security_solution/docs/testing/test_plans/detection_response/alerts/user_assignment.md
Outdated
Show resolved
Hide resolved
# Conflicts: # x-pack/test/security_solution_cypress/cypress/tasks/navigation.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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":"[email protected]"},"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 <[email protected]>
…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 <[email protected]>
…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 <[email protected]>
…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 <[email protected]>
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