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.

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 SOLUTIONS] Bugs overview page + investigate eql in timeline #81550

Merged
merged 10 commits into from
Oct 27, 2020

Conversation

XavierM
Copy link
Contributor

@XavierM XavierM commented Oct 23, 2020

Summary

Checklist

@XavierM XavierM added bug Fixes for quality problems that affect the customer experience v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.10.0 Team:Threat Hunting Security Solution Threat Hunting Team labels Oct 23, 2020
@XavierM XavierM requested review from a team as code owners October 23, 2020 00:55
@XavierM XavierM self-assigned this Oct 23, 2020
@XavierM
Copy link
Contributor Author

XavierM commented Oct 26, 2020

@elasticmachine merge upstream

Comment on lines +154 to +155
ecsData.signal?.rule?.type?.length &&
ecsData.signal?.rule?.type[0] === 'eql' &&
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could consolidate to the following using array item access with optional chaining if you'd like:

Suggested change
ecsData.signal?.rule?.type?.length &&
ecsData.signal?.rule?.type[0] === 'eql' &&
ecsData.signal?.rule?.type?.[0] === 'eql' &&

]);
const resultingTimeline: TimelineResult = getOr({}, 'data.getOneTimeline', responseTimeline);
const eventData: TimelineEventsDetailsItem[] = getOr([], 'data', eventDataResp);
const eventData: TimelineEventsDetailsItem[] = eventDataResp.data ?? [];
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for cleaning up more getOr's! 🙇‍♂️

Copy link
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, tested locally, and verified EQL Rules with sequences use signal.group.id and EQL Rules without sequences use _id when Investigating in Timeline. LGTM! Thanks @XavierM! 👍

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

Tested "investigate in timeline" with both sequence alerts and non-sequence alerts. LGTM!

ecsData.signal?.rule?.type?.length && ecsData.signal?.rule?.type[0] === 'eql';
export const isEqlRuleWithGroupId = (ecsData: Ecs) =>
ecsData.signal?.rule?.type?.length &&
ecsData.signal?.rule?.type[0] === 'eql' &&
Copy link
Contributor

Choose a reason for hiding this comment

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

We've also got those rule type helpers that could be leveraged here!

Copy link
Contributor

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

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

Thanks for these fixes @XavierM!
Desk tested locally for (only) the following issue:

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
securitySolution 8.1MB 8.1MB +2.2KB

page load bundle size

id before after diff
securitySolution 263.1KB 263.1KB -1.0B

History

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

@XavierM XavierM merged commit b304051 into elastic:master Oct 27, 2020
XavierM added a commit to XavierM/kibana that referenced this pull request Oct 27, 2020
…elastic#81550)

* fix overview query to be connected to sourcerer

* investigate eql in timeline

* keep timeline indices

* trusting what is coming from timeline saved object for index pattern at initialization

* fix type + initialize old timeline to sourcerer

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
XavierM added a commit to XavierM/kibana that referenced this pull request Oct 27, 2020
…elastic#81550)

* fix overview query to be connected to sourcerer

* investigate eql in timeline

* keep timeline indices

* trusting what is coming from timeline saved object for index pattern at initialization

* fix type + initialize old timeline to sourcerer

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 27, 2020
* master: (37 commits)
  [ILM] Migrate Warm phase to Form Lib (elastic#81323)
  [Security Solutions][Detection Engine] Fixes critical bug with error reporting that was doing a throw (elastic#81549)
  [Detection Rules] Add 7.10 rules (elastic#81676)
  [kbn/optimizer] ignore missing metrics when updating limits with --focus (elastic#81696)
  [SECURITY SOLUTIONS] Bugs overview page + investigate eql in timeline (elastic#81550)
  [Maps] fix unable to edit cluster vector styles styled by count when switching to super fine grid resolution (elastic#81525)
  Fixed migration issue for case specific actions, by extending email action migrator checks (elastic#81673)
  [CI] Preparation for APM tracking on CI (elastic#80399)
  [Home] Fixes Kibana app description order on home page and updates Canvas copy (elastic#80057)
  Make sure `to` is 'now' and not the same as `from` (elastic#81524)
  Nitpicking the 8.0 Breaking Change issue template (elastic#81678)
  [SECURITY_SOLUTION] Fix text on onboarding screen (elastic#81672)
  [data.search] Skip async search tests in build candidates and production builds (elastic#81547)
  Fix previousStartedAt by not changing when execution fails (elastic#81388)
  [Monitoring] Fix a couple of issues with the cpu usage alert (elastic#80737)
  Telemetry collection xpack to ts project references (elastic#81269)
  Elasticsearch: don't use url authentication for new client (elastic#81564)
  [App Search] Credentials: implement working flyout form (elastic#81541)
  Properly encode links to edit user page (elastic#81562)
  [Alerting UI] Don't wait for health check before showing Create Alert flyout (elastic#80996)
  ...
XavierM added a commit that referenced this pull request Oct 27, 2020
…#81550) (#81708)

* fix overview query to be connected to sourcerer

* investigate eql in timeline

* keep timeline indices

* trusting what is coming from timeline saved object for index pattern at initialization

* fix type + initialize old timeline to sourcerer

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 27, 2020
XavierM added a commit that referenced this pull request Oct 27, 2020
…#81550) (#81712)

* fix overview query to be connected to sourcerer

* investigate eql in timeline

* keep timeline indices

* trusting what is coming from timeline saved object for index pattern at initialization

* fix type + initialize old timeline to sourcerer

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants