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

Allow the possibility to use rule and match filed in thehive alert description #855

Merged
merged 17 commits into from
May 24, 2022

Conversation

luffynextgen
Copy link
Contributor

@luffynextgen luffynextgen commented May 20, 2022

Description

No breaking change.
I used the model of alert_text and alert_text_args, to give the possibility to use rule fields and match fields in thehive alert description.

For this purpose I added an optional field in the rule 'description_args'.
if not used the description will follow the normal path (either default description or the string added by the user).

I was in need for it and saw I was the only who tried to use such a thing.

Checklist

  • I have reviewed the contributing guidelines.
  • I have included unit tests for my changes or additions.
  • I have successfully run make test-docker with my changes.
  • I have manually tested all relevant modes of the change in this PR.
  • I have updated the documentation.
  • I have updated the changelog.

Questions or Comments

I have tested this in my environment making elastalert run on a ELK Siem. I created data that would trigger my test alert and they were indexed in the elasticsearch cluster. It worked as expected, the alert was sent to TheHive with the necessary information in the description.
I tested the alert by also sending it to slack at the same time, and found no issue.
I used the last version of elastalert2.

ITER and others added 4 commits May 20, 2022 17:15
…llow the use of match and rule fields in the description for the alert in thehive, based on the model of alert_text and alert_text_args'
…llow the use of match and rule fields in the description for the alert in thehive, based on the model of alert_text and alert_text_args'
add documentation for description_args field in the hive
Add the possibility to use rule and match fileds in the description of TheHive alert
Copy link
Owner

@jertel jertel 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 your contribution! Please make the following changes:

  1. Document the new description_missing_value setting.
  2. Add unit tests to cover the following scenarios:
  • Rule does not contain a description subfield under the hive_alert_config block
  • Rule does contain a description subfield under the hive_alert_config block but does not contain a description_args subfield.
  • Rule contains both the description and description_args subfield, with 3 args, where arg1 value is correctly looked up in the match, arg2 value has no match lookup (None), and for arg3, whatever you need to do make it get a description_missing_value (I'm not seeing how this can ever be used, so a test case illustrating it will be helpful).
  1. Update the CHANGELOG entry you added to adhere to the proper changelog format. Notice that the other changelogs contain the PR link and the author username, separated by hyphens.

@luffynextgen
Copy link
Contributor Author

luffynextgen commented May 23, 2022

Hello @jertel,
Thank you for your remarks.
I tried to implement everything asked.
Regarding the description_missing_value it basically replace all None values by a predefined value (default being <MISSING VALUE>).

Let me know if anything else is needed.

Kind regards,

@jertel
Copy link
Owner

jertel commented May 23, 2022

Thank you for those changes. Why did you move thehive_test.py out of alerters/ and into the parent dir? Your new unit tests should have been added to that existing file. And there isn't a need for those 4 function call lines at the bottom of the unit test file. The unittest framework will automatically invoke the test_* functions.

@luffynextgen
Copy link
Contributor Author

the file move was copy and pate error, my bad, sorry.
I added the unit tests in thehive_test.py.

Kind regards,

Copy link
Owner

@jertel jertel left a comment

Choose a reason for hiding this comment

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

Couple more minor changes and we should be ready to merge this. I noticed there's no schema.yaml values for thehive alerter so eventually we'll need to get that fixed, but it doesn't need to be part of this PR.

elastalert/alerters/thehive.py Outdated Show resolved Hide resolved
tests/alerters/thehive_test.py Outdated Show resolved Hide resolved
@jertel
Copy link
Owner

jertel commented May 23, 2022

Looks like two of your new unit tests are failing. Once you fix those and see the build go green we will be set.

@jertel
Copy link
Owner

jertel commented May 24, 2022

The unit tests are now passing but there are some lint problems:

./alerters/thehive_test.py:308:1: E302 expected 2 blank lines, found 1
./alerters/thehive_test.py:330:1: W293 blank line contains whitespace
./alerters/thehive_test.py:344:13: E225 missing whitespace around operator
./alerters/thehive_test.py:347:1: E266 too many leading '#' for block comment
./alerters/thehive_test.py:348:1: E302 expected 2 blank lines, found 1
./alerters/thehive_test.py:371:1: W293 blank line contains whitespace
./alerters/thehive_test.py:388:1: E266 too many leading '#' for block comment
./alerters/thehive_test.py:403:56: E201 whitespace after '['
./alerters/thehive_test.py:403:83: E202 whitespace before ']'
./alerters/thehive_test.py:416:1: W293 blank line contains whitespace
./alerters/thehive_test.py:433:1: E266 too many leading '#' for block comment
./alerters/thehive_test.py:446:56: E201 whitespace after '['
./alerters/thehive_test.py:446:83: E202 whitespace before ']'
./alerters/thehive_test.py:459:1: W293 blank line contains whitespace
./alerters/thehive_test.py:473:30: W292 no newline at end of file

@luffynextgen
Copy link
Contributor Author

I tried to correct, following the output you gave me.
It should be good now

@jertel jertel merged commit 80fb1d0 into jertel:master May 24, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants