Skip to content

[SIEM] Detection engine timeline#53783

Merged
XavierM merged 23 commits intoelastic:masterfrom
XavierM:detection-engine-timeline
Jan 9, 2020
Merged

[SIEM] Detection engine timeline#53783
XavierM merged 23 commits intoelastic:masterfrom
XavierM:detection-engine-timeline

Conversation

@XavierM
Copy link
Copy Markdown
Contributor

@XavierM XavierM commented Dec 24, 2019 •
edited
Loading

Summary

  • Remove the accordion arrows that appear before each step number.
  • Don’t allow the user to collapse currently open step panels, unless they continue to next step or edit a previous step.
  • Don’t allow more than one step panel to be open at a time.
  • Circular step numbers/checks in each step panel should be colored blue when active or completed.
  • It looks like we're currently using compressed EUI form fields. I'd prefer if we switch to the standard sized EUI form fields in this scenario.
  • As the selection to use the SIEM advanced settings index pattern defaults isn't a permanent connection, the previously suggested use of a radio button to choose between default or custom makes less sense. Let's simplify it to use a restore defaults button/link, when it is detected that the user has altered the index patterns away from what is currently in SIEM advanced settings.
  • Add option to import the query from a saved timeline.
  • Add a color/health indicator to severity selector.
  • Add ticks/stops to risk score slider.
  • Add timeline template selector, which will allow the user to select an existing timeline to use as a template when opening a signal from this rule.
  • Reorganize MITRE ATT&CK threat form layout so that it plays nicer when the technique fields grow taller than one line.
  • Move and reword tags placeholder to bottom helper text.
  • Add Url Validation
  • Open signal from the selected timeline if selected Will do that in another PR since we are not completely sure about the requirement

here the issue

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@XavierM XavierM added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Dec 24, 2019
@XavierM XavierM self-assigned this Dec 24, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 24, 2019

Pinging @elastic/siem (Team:SIEM)

@elastic elastic deleted a comment from kibanamachine Jan 2, 2020
@elastic elastic deleted a comment from kibanamachine Jan 2, 2020
spong
spong reviewed Jan 8, 2020
View reviewed changes
...egacy/plugins/siem/public/pages/detection_engine/rules/components/step_about_rule/schema.tsx
Comment on lines +116 to +119
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined => {
const [{ value, path }] = args;
let hasError = false;
(value as string[]).forEach(url => {
Copy link
Copy Markdown
Member

@spong spong Jan 8, 2020

Choose a reason for hiding this comment

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

nit: Types are a little loose here -- potential improvement to the es_ui_shared ValidationFunc interface.

spong
spong reviewed Jan 8, 2020
View reviewed changes
x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/index.tsx
<EuiButton fill href="#/detection-engine/rules/create" iconType="plusInCircle">
<EuiButton
fill
href={`#${DETECTION_ENGINE_PAGE_NAME}/rules/create`}
Copy link
Copy Markdown
Member

@spong spong Jan 8, 2020

Choose a reason for hiding this comment

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

Consider using the route helpers (which appear to be out of date already... 😅 ):

kibana/x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_detection_engine.tsx

Line 48 in 59e0a1c

export const getCreateRuleUrl = () => `#/link-to/${DETECTION_ENGINE_PAGE_NAME}/rules/create-rule`;

Copy link
Copy Markdown
Contributor Author

@XavierM XavierM Jan 8, 2020

Choose a reason for hiding this comment

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

When we fix the state url, we will go back to it

spong
spong approved these changes Jan 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out locally and code reviewed all client-side code. Added a few comments for leftovers/cleanup + a minor bug or two, but everything else looks good here! Thanks for looping back around to clean things up and for also adding the Timeline Template as well. LGTM! 👍

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jan 8, 2020

💚 Build Succeeded

History

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

@XavierM XavierM merged commit 1e2cbb3 into elastic:master Jan 9, 2020
@XavierM XavierM mentioned this pull request Jan 9, 2020
Merged
XavierM added a commit to XavierM/kibana that referenced this pull request Jan 9, 2020
@XavierM
[SIEM] Detection engine timeline (elastic#53783)
5b04df5 
* change create to only have only one form to be open at the same time

* add tick to risk score

* remove compressed

* fix select in schedule

* fix bug to not  allow more than one step panel to be open at a time

* Add a color/health indicator to severity selector

* Move and reword tags placeholder to bottom helper text

* fix ux on the index patterns field

* Reorganize MITRE ATT&CK threat

* add url validation + some cleaning to prerp work for UT

* add feature to get back timeline + be able to disable action on timeline modal

* Add option to import the query from a saved timeline.

* wip

* Add timeline template selector

* fix few bugs from last commit

* review I

* fix unit test for timeline_title

* ui review

* fix truncation on timeline selectable
MichaelMarcialis
MichaelMarcialis reviewed Jan 9, 2020
View reviewed changes
...lugins/siem/public/pages/detection_engine/rules/components/step_define_rule/translations.tsx Outdated
'xpack.siem.detectionEngine.createRule.stepDefineRule.indicesHelperDescription',
{
defaultMessage:
'Enter the pattern of Elasticsearch indices where you would like this rule to run.By default, these will include index patterns defined in SIEM advanced settings.',
Copy link
Copy Markdown
Contributor

@MichaelMarcialis MichaelMarcialis Jan 8, 2020

Choose a reason for hiding this comment

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

Missing space between sentences.

...gacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/schema.tsx Outdated
'xpack.siem.detectionEngine.createRule.stepDefineRule.outputIndiceNameFieldRequiredError',
{
defaultMessage: 'An output indice name for signals is required.',
defaultMessage: 'An index patterns for signals is required.',
Copy link
Copy Markdown
Contributor

@MichaelMarcialis MichaelMarcialis Jan 8, 2020

Choose a reason for hiding this comment

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

The plural is incorrect here. I imagine this message should be reworded to one of these:

  • An index pattern is required.
  • Index patterns are required.
  • A minimum of one index pattern is required.

@MichaelMarcialis MichaelMarcialis mentioned this pull request Jan 9, 2020
Closed
39 tasks
@sebelga
Copy link
Copy Markdown
Contributor

sebelga commented Jan 9, 2020 •
edited
Loading

@XavierM Just seeing this PR as we had a merge conflict with the changes in the form library. Could you please next time ping our team whenever you make a change to the es_ui_shared folder so we can also review it? thanks! 😊

I see that we should have been pinged automatically but the CODEOWNERS file is outdated

[EDIT] It is not outdated. @cjcenizal can you have a look why we didn't get pinged by a change in the es_ui_shared folder?

gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 9, 2020
@gmmorris
Merge branch 'master' into task-manager/schema-config
3dd94d5 
* master: (23 commits)
  [Vis: Default editor] Reactify the timelion editor (elastic#52990)
  [Discover] fix histogram min interval (elastic#53979)
  [Telemetry] [Monitoring] Only retry fetching usage once monito… (elastic#54309)
  [docs][APM] Add runtime index config documentation (elastic#53907)
  [SIEM] Detection engine timeline (elastic#53783)
  Filter scripted fields preview field list to source fields (elastic#53826)
  Management - New platform api (elastic#52579)
  Reset region and Account when switching inventory (elastic#54287)
  [SIEM] [Case] Case workflow api schema (elastic#51535)
  Code coverage setup on CI (elastic#49003)
  [ML] DF Analytics Results: adds link to docs (elastic#54189)
  Update schemas boolean, byteSize, and duration to coerce strings (elastic#54177)
  [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage (elastic#49781)
  [Canvas] Fixes bugs with autoplay and refresh (elastic#53149)
  [ML] DF Analytics Classification: ensure confusion matrix can be fetched (elastic#53629)
  Fix Vega react eslint errors (elastic#54259)
  Remove non existing codeowners (elastic#54274)
  use correct type (elastic#54244)
  [Dashboard] Removing 100% as dshDashboardViewport height (elastic#54263)
  add `examples/` to no-restricted-path config (elastic#54252)
  ...
XavierM added a commit that referenced this pull request Jan 9, 2020
@XavierM
[SIEM] Detection engine timeline (#53783) (#54318)
0907022 
* change create to only have only one form to be open at the same time

* add tick to risk score

* remove compressed

* fix select in schedule

* fix bug to not  allow more than one step panel to be open at a time

* Add a color/health indicator to severity selector

* Move and reword tags placeholder to bottom helper text

* fix ux on the index patterns field

* Reorganize MITRE ATT&CK threat

* add url validation + some cleaning to prerp work for UT

* add feature to get back timeline + be able to disable action on timeline modal

* Add option to import the query from a saved timeline.

* wip

* Add timeline template selector

* fix few bugs from last commit

* review I

* fix unit test for timeline_title

* ui review

* fix truncation on timeline selectable
@cjcenizal cjcenizal mentioned this pull request Jan 10, 2020
Closed
@XavierM XavierM deleted the detection-engine-timeline branch June 4, 2020 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaelMarcialis MichaelMarcialis MichaelMarcialis left review comments

@spong spong spong approved these changes

+1 more reviewer

@FrankHassanabad FrankHassanabad FrankHassanabad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@XavierM XavierM

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.6.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@XavierM @elasticmachine @FrankHassanabad @spong @kibanamachine @sebelga @MichaelMarcialis