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

Digital guardian Bug #34920

Merged
merged 22 commits into from
Jun 19, 2024
Merged

Digital guardian Bug #34920

merged 22 commits into from
Jun 19, 2024

Conversation

merit-maita
Copy link
Contributor

@merit-maita merit-maita commented Jun 18, 2024

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: https://jira-dc.paloaltonetworks.com/browse/XSUP-38319

Description

fixed an issue where fetch events failed when some fields from the api were empty

Must have

  • Tests
  • Documentation

Copy link

Your contributed DigitalGuardian pack has been modified on files:

Packs/DigitalGuardian/ReleaseNotes/1_1_5.md
Packs/DigitalGuardian/Integrations/DigitalGuardianARCEventCollector/DigitalGuardianARCEventCollector.py
Packs/DigitalGuardian/Integrations/DigitalGuardian/DigitalGuardian.py
Packs/DigitalGuardian/pack_metadata.json
Please review the changes here

Copy link

github-actions bot commented Jun 18, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/DigitalGuardian/Integrations/DigitalGuardian
   DigitalGuardian.py26919726%44, 55, 57–58, 67–74, 76, 78–79, 81, 88–95, 97, 99–100, 102, 114–115, 117–118, 120–122, 124–129, 131, 138–139, 141–142, 144–149, 151–152, 155, 160, 169–172, 174–179, 181–184, 186, 188–189, 192, 200–203, 205–209, 211–212, 215, 225–228, 230–233, 235–236, 238, 249–252, 254, 256–257, 260, 268–276, 278–279, 282, 291–292, 294, 296–297, 299–301, 303–304, 306–311, 313, 315–317, 319–322, 324–337, 339, 342, 385, 397, 406–407, 460–469, 471–487, 495–500, 530, 538–539
Packs/DigitalGuardian/Integrations/DigitalGuardianARCEventCollector
   DigitalGuardianARCEventCollector.py1133370%35–40, 42–46, 52–53, 57–58, 61–63, 66, 71, 86–88, 94–96, 98–99, 117–118, 146, 175, 177
TOTAL38223039% 

Tests Skipped Failures Errors Time
5 0 💤 0 ❌ 0 🔥 1.518s ⏱️

@@ -180,8 +180,8 @@ def check_componentlist_entry():

if 200 <= r.status_code <= 299:
for jText in json_text:
if str(jText['content_value']).lower() == componentlist_entry.lower():
componentlist = jText['content_value']
if str(jText.get('content_value')).lower() == componentlist_entry.lower():
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps return a default empty string jText.get('content_value', '')
because we are doing lower on that and it will throw exception if returns None

@@ -125,8 +125,8 @@ def get_watchlist_entry_id(watchlist_name: str, watchlist_entry: str) -> str:
if r.status_code != requests.codes.ok:
return_error('Unable to retrieve watchlist entries')
for jText in json_text:
if str(jText['value_name']).lower() == watchlist_entry.lower():
watchlist_entry_id = jText['value_id']
if str(jText.get('value_name')).lower() == watchlist_entry.lower():
Copy link
Contributor

Choose a reason for hiding this comment

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

same here:
jText.get('content_value', '')


DEBUG("cef: " + json.dumps(cef))
for artifact_key, artifact_tuple in specific_alert_mapping.get(CATEGORY).items(): # type: ignore
if alert.get(artifact_tuple[0]):
cef[artifact_key] = alert[artifact_tuple[0]]
cef_types[artifact_key] = artifact_tuple[1]
if cef:
comm = alert['dg_alarm_name'].find(',')
comm = alert.get('dg_alarm_name').find(',')
Copy link
Contributor

Choose a reason for hiding this comment

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

same here:
alert.get('dg_alarm_name', '')

result = dict(zip(outcome, event))
event_list.append(result)
event_list.sort(key=lambda item: (item["inc_mtime"], item["dg_guid"]))
event_list.sort(key=lambda item: (item.get("inc_mtime"), item.get("dg_guid")))
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a unit test that tests this method were item does not have inc_mtime and dg_guid and see that it is working.

@ShirleyDenkberg ShirleyDenkberg self-assigned this Jun 19, 2024
@ShirleyDenkberg
Copy link
Contributor

@thefrieddan1 Doc review completed.

@merit-maita merit-maita merged commit 043480d into master Jun 19, 2024
18 checks passed
@merit-maita merit-maita deleted the digitalGuardian_bug branch June 19, 2024 15:12
amshamah419 pushed a commit that referenced this pull request Jun 20, 2024
* fixed parsing events from api

* fix

* added rn

* fix

* fix

* fixes

* fixes

* added rn

* updated do

* pre-commit edits

* Update Packs/DigitalGuardian/ReleaseNotes/1_1_5.md

Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>

* Update Packs/DigitalGuardian/ReleaseNotes/1_1_5.md

Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>

* fixes

* made a change for the unit test

* edit

* added fixes for unittests

* fixed do

---------

Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants