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

[SecuritySolution][Threat Hunting] Fix a couple of field ids for highlighted fields #124941

Merged
merged 2 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,40 @@ describe('AlertSummaryView', () => {
expect(getByText(fieldId));
});
});

test('DNS event renders the correct summary rows', () => {
const renderProps = {
...props,
data: [
...(mockAlertDetailsData.map((item) => {
if (item.category === 'event' && item.field === 'event.category') {
return {
...item,
values: ['dns'],
originalValue: ['dns'],
};
}
return item;
}) as TimelineEventsDetailsItem[]),
{
category: 'dns',
field: 'dns.question.name',
Copy link
Contributor

@andrew-goldstein andrew-goldstein Feb 9, 2022

Choose a reason for hiding this comment

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

Does it make sense to also display dns.question.type in the flyout summary for parity with the DNS row renderer, shown in the screenshot below?

dns-row-renderer

CC @paulewing

values: ['www.example.com'],
originalValue: ['www.example.com'],
} as TimelineEventsDetailsItem,
],
};
const { getByText } = render(
<TestProvidersComponent>
<AlertSummaryView {...renderProps} />
</TestProvidersComponent>
);

['dns.question.name', 'process.name'].forEach((fieldId) => {
expect(getByText(fieldId));
});
});

test('Memory event code renders additional summary rows', () => {
const renderProps = {
...props,
Expand All @@ -140,32 +174,41 @@ describe('AlertSummaryView', () => {
});
});
test('Behavior event code renders additional summary rows', () => {
const actualRuleDescription = 'The actual rule description';
const renderProps = {
...props,
data: mockAlertDetailsData.map((item) => {
if (item.category === 'event' && item.field === 'event.code') {
return {
...item,
values: ['behavior'],
originalValue: ['behavior'],
};
}
if (item.category === 'event' && item.field === 'event.category') {
return {
...item,
values: ['malware', 'process', 'file'],
originalValue: ['malware', 'process', 'file'],
};
}
return item;
}) as TimelineEventsDetailsItem[],
data: [
...mockAlertDetailsData.map((item) => {
if (item.category === 'event' && item.field === 'event.code') {
return {
...item,
values: ['behavior'],
originalValue: ['behavior'],
};
}
if (item.category === 'event' && item.field === 'event.category') {
return {
...item,
values: ['malware', 'process', 'file'],
originalValue: ['malware', 'process', 'file'],
};
}
return item;
}),
{
category: 'rule',
field: 'rule.description',
values: [actualRuleDescription],
originalValue: [actualRuleDescription],
},
] as TimelineEventsDetailsItem[],
};
const { getByText } = render(
<TestProvidersComponent>
<AlertSummaryView {...renderProps} />
</TestProvidersComponent>
);
['host.name', 'user.name', 'process.name'].forEach((fieldId) => {
['host.name', 'user.name', 'process.name', actualRuleDescription].forEach((fieldId) => {
expect(getByText(fieldId));
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@
*/

import { find, isEmpty, uniqBy } from 'lodash/fp';
import {
ALERT_RULE_NAMESPACE,
ALERT_RULE_TYPE,
ALERT_RULE_DESCRIPTION,
} from '@kbn/rule-data-utils';
import { ALERT_RULE_NAMESPACE, ALERT_RULE_TYPE } from '@kbn/rule-data-utils';

import * as i18n from './translations';
import { BrowserFields } from '../../../../common/search_strategy/index_fields';
Expand Down Expand Up @@ -69,7 +65,7 @@ function getFieldsByCategory({
{ id: 'process.name' },
];
case EventCategory.DNS:
return [{ id: 'dns.query.name' }, { id: 'process.name' }];
return [{ id: 'dns.question.name' }, { id: 'process.name' }];
case EventCategory.REGISTRY:
return [{ id: 'registry.key' }, { id: 'registry.value' }, { id: 'process.name' }];
case EventCategory.MALWARE:
Expand Down Expand Up @@ -107,7 +103,7 @@ function getFieldsByEventCode(
switch (eventCode) {
case EventCode.BEHAVIOR:
return [
{ id: ALERT_RULE_DESCRIPTION, label: ALERTS_HEADERS_RULE_DESCRIPTION },
{ id: 'rule.description', label: ALERTS_HEADERS_RULE_DESCRIPTION },
Copy link
Contributor

Choose a reason for hiding this comment

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

When viewing the same alert in a branch based off main (before the change), the alert description appears in the flyout, because the alert is populated with kibana.alert.rule.description, per the before screenshot below:

before

before

When running this PR branch, the dns.question.name now shows up as expected, but the rule description no longer appears, because the event does not contain rule.description, per the after screenshot below:

after

after

Is it possible that I'm missing a change in my local environment that would populate rule.description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is something that I was also struggling with to understand. From what @paulewing explained to me I understood that kibana.alert.rule.description is not the same as rule.description. I am not sure though, how it gets populated. Maybe Paul has some more insights into that.

// Resolve more fields based on the source event
...getFieldsByCategory({ ...eventCategories, primaryEventCategory: undefined }),
];
Expand Down