-
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] Fix IP/geo location in Top N Flow Table #149369
Conversation
@@ -118,6 +118,7 @@ export const mockSearchStrategyResponse: IEsSearchResponse<unknown> = { | |||
_id: 'dd4fa2d4bd-1526378075029582', | |||
_score: 0, | |||
fields: { | |||
'@timestamp': ['2022-07-18T15:08:48.064Z'], |
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.
the bug surfaced because some returned geo data included the timestamp. Adding this field here would break tests in main
and proves the fix
result.location.top_geo.hits.hits.length > 0 && result.location.top_geo.hits.hits[0].fields | ||
? { | ||
geo: formatResponseObjectValues( | ||
getOr( | ||
'', | ||
`${Object.keys(result.location.top_geo.hits.hits[0].fields)[0].split('.geo')[0]}.geo`, |
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.
This was very silly. If for some reason a non-geo field is in there (ahem, @timestamp
), the code breaks. We are able to simply pass flowTarget
from where the function is called.
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.
Now, it looks ten times better 🤩
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
result.location.top_geo.hits.hits.length > 0 && result.location.top_geo.hits.hits[0].fields | ||
? { | ||
geo: formatResponseObjectValues( | ||
getOr( | ||
'', | ||
`${Object.keys(result.location.top_geo.hits.hits[0].fields)[0].split('.geo')[0]}.geo`, |
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.
Now, it looks ten times better 🤩
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
Summary
I found a bug where the country flag was not showing in the top n table on the network page even though the IP address was valid. I've explained the problematic code inline.
Before
After