Skip to content

Commit

Permalink
[8.8] [Security Solution] fix from-to values investigate in timeline …
Browse files Browse the repository at this point in the history
…pulled from timestamp instead of @timestamp field (#156884) (#157114)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] fix from-to values investigate in timeline pulled
from timestamp instead of @timestamp field
(#156884)](#156884)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-08T23:44:14Z","message":"[Security
Solution] fix from-to values investigate in timeline pulled from
timestamp instead of @timestamp field (#156884)\n\n## Summary\r\n\r\nIn
Kibana 8.8 we've done a huge refactor of the alert table
(see\r\n[PR](https://github.com/elastic/kibana/pull/149128)).\r\nThe new
table's trigger actions are no longer some of the Security\r\nSolution
server side methods that were adding a `timestamp` field to
the\r\nresponse
(see\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)).\r\nThat
`timestamp` field was basically a duplication of the
`@timestamp`\r\nfield (done via [this
helper\r\nfunction](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12))\r\n\r\nRunning
the stack locally, you would see that clicking on the\r\n`Investigate in
timeline` icon button or ANY alerts in the alerts table\r\n(pretty new
or months/years old) would bring the timeline flyout with\r\nalways the
`to` value being the `current date`, and the `from` value\r\nbeing the
`current date - kibana.alert.rule.from`.\r\nThe `timetamp`
field\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158)\r\ndoes
not exists, so we always fall back to `new Date()` (unless
you're\r\nlooking at an alert generated by a threshold rule, a new terms
rule or a\r\nsuppressed alert).\r\n\r\nThis PR fixes the issue by
retrieving the `@timestamp` field instead of\r\nthe unwanted `timestamp`
field.\r\n\r\nMore work will be done in the future to actually entirely
remove and\r\ncleanup the `timestamp` field (see [this
WIP\r\nticket](https://github.com/elastic/kibana/issues/156879)).\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\nBefore
(recorded on May 4th and we're looking at an alert generated on\r\nMay
3rd)\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov\r\n\r\nAfter\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov\r\n\r\nCloses
https://github.com/elastic/kibana/issues/126077","sha":"114c98d3b5df4252f500ea762879524540b2c50a","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Threat
Hunting","Team: SecuritySolution","Team:Threat
Hunting:Investigations","v8.8.0","v8.9.0"],"number":156884,"url":"https://github.com/elastic/kibana/pull/156884","mergeCommit":{"message":"[Security
Solution] fix from-to values investigate in timeline pulled from
timestamp instead of @timestamp field (#156884)\n\n## Summary\r\n\r\nIn
Kibana 8.8 we've done a huge refactor of the alert table
(see\r\n[PR](https://github.com/elastic/kibana/pull/149128)).\r\nThe new
table's trigger actions are no longer some of the Security\r\nSolution
server side methods that were adding a `timestamp` field to
the\r\nresponse
(see\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)).\r\nThat
`timestamp` field was basically a duplication of the
`@timestamp`\r\nfield (done via [this
helper\r\nfunction](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12))\r\n\r\nRunning
the stack locally, you would see that clicking on the\r\n`Investigate in
timeline` icon button or ANY alerts in the alerts table\r\n(pretty new
or months/years old) would bring the timeline flyout with\r\nalways the
`to` value being the `current date`, and the `from` value\r\nbeing the
`current date - kibana.alert.rule.from`.\r\nThe `timetamp`
field\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158)\r\ndoes
not exists, so we always fall back to `new Date()` (unless
you're\r\nlooking at an alert generated by a threshold rule, a new terms
rule or a\r\nsuppressed alert).\r\n\r\nThis PR fixes the issue by
retrieving the `@timestamp` field instead of\r\nthe unwanted `timestamp`
field.\r\n\r\nMore work will be done in the future to actually entirely
remove and\r\ncleanup the `timestamp` field (see [this
WIP\r\nticket](https://github.com/elastic/kibana/issues/156879)).\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\nBefore
(recorded on May 4th and we're looking at an alert generated on\r\nMay
3rd)\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov\r\n\r\nAfter\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov\r\n\r\nCloses
https://github.com/elastic/kibana/issues/126077","sha":"114c98d3b5df4252f500ea762879524540b2c50a"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156884","number":156884,"mergeCommit":{"message":"[Security
Solution] fix from-to values investigate in timeline pulled from
timestamp instead of @timestamp field (#156884)\n\n## Summary\r\n\r\nIn
Kibana 8.8 we've done a huge refactor of the alert table
(see\r\n[PR](https://github.com/elastic/kibana/pull/149128)).\r\nThe new
table's trigger actions are no longer some of the Security\r\nSolution
server side methods that were adding a `timestamp` field to
the\r\nresponse
(see\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)).\r\nThat
`timestamp` field was basically a duplication of the
`@timestamp`\r\nfield (done via [this
helper\r\nfunction](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12))\r\n\r\nRunning
the stack locally, you would see that clicking on the\r\n`Investigate in
timeline` icon button or ANY alerts in the alerts table\r\n(pretty new
or months/years old) would bring the timeline flyout with\r\nalways the
`to` value being the `current date`, and the `from` value\r\nbeing the
`current date - kibana.alert.rule.from`.\r\nThe `timetamp`
field\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158)\r\ndoes
not exists, so we always fall back to `new Date()` (unless
you're\r\nlooking at an alert generated by a threshold rule, a new terms
rule or a\r\nsuppressed alert).\r\n\r\nThis PR fixes the issue by
retrieving the `@timestamp` field instead of\r\nthe unwanted `timestamp`
field.\r\n\r\nMore work will be done in the future to actually entirely
remove and\r\ncleanup the `timestamp` field (see [this
WIP\r\nticket](https://github.com/elastic/kibana/issues/156879)).\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\nBefore
(recorded on May 4th and we're looking at an alert generated on\r\nMay
3rd)\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov\r\n\r\nAfter\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov\r\n\r\nCloses
https://github.com/elastic/kibana/issues/126077","sha":"114c98d3b5df4252f500ea762879524540b2c50a"}}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <[email protected]>
  • Loading branch information
kibanamachine and PhilippeOberti authored May 9, 2023
1 parent 57f0a68 commit 26c3fbe
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { EcsSecurityExtension as Ecs } from '@kbn/securitysolution-ecs';
export const getDetectionAlertMock = (overrides: Partial<Ecs> = {}): Ecs => ({
...{
_id: '1',
timestamp: '2018-11-05T19:03:25.937Z',
'@timestamp': '2018-11-05T19:03:25.937Z',
host: {
name: ['apache'],
ip: ['192.168.0.1'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1014,9 +1014,9 @@ describe('alert actions', () => {
});

test('it uses ecs.Data.timestamp if one is provided', () => {
const ecsDataMock: Ecs = {
const ecsDataMock = {
...mockEcsDataWithAlert,
timestamp: '2020-03-20T17:59:46.349Z',
'@timestamp': '2020-03-20T17:59:46.349Z',
};
const result = determineToAndFrom({ ecs: ecsDataMock });

Expand All @@ -1025,7 +1025,8 @@ describe('alert actions', () => {
});

test('it uses current time timestamp if ecsData.timestamp is not provided', () => {
const { timestamp, ...ecsDataMock } = mockEcsDataWithAlert;
// @ts-ignore // TODO remove when EcsSecurityExtension has been cleaned https://github.com/elastic/kibana/issues/156879
const { '@timestamp': timestamp, ...ecsDataMock } = mockEcsDataWithAlert;
const result = determineToAndFrom({ ecs: ecsDataMock });

expect(result.from).toEqual('2020-03-01T17:54:46.349Z');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
ALERT_SUPPRESSION_END,
ALERT_SUPPRESSION_DOCS_COUNT,
ALERT_SUPPRESSION_TERMS,
TIMESTAMP,
} from '@kbn/rule-data-utils';

import { lastValueFrom } from 'rxjs';
Expand Down Expand Up @@ -155,10 +156,13 @@ export const determineToAndFrom = ({ ecs }: { ecs: Ecs[] | Ecs }) => {
const elapsedTimeRule = moment.duration(
moment().diff(dateMath.parse(ruleFrom != null ? ruleFrom[0] : 'now-1d'))
);
const from = moment(ecsData.timestamp ?? new Date())
.subtract(elapsedTimeRule)
.toISOString();
const to = moment(ecsData.timestamp ?? new Date()).toISOString();

const alertTimestampEcsValue = getField(ecsData, TIMESTAMP);
const alertTimestamp = Array.isArray(alertTimestampEcsValue)
? alertTimestampEcsValue[0]
: alertTimestampEcsValue;
const to = moment(alertTimestamp ?? new Date()).toISOString();
const from = moment(to).subtract(elapsedTimeRule).toISOString();

return { to, from };
};
Expand Down

0 comments on commit 26c3fbe

Please sign in to comment.