ntop / ntopng

Web-based Traffic and Security Network Traffic Monitoring
http://www.ntop.org
GNU General Public License v3.0
6.31k stars 658 forks source link

Fix issue #8770: Update webhook endpoint functionality #8775

Closed marcoeg closed 1 month ago

marcoeg commented 1 month ago

Please sign (check) the below before submitting the Pull Request:

Link to the related issue: https://github.com/ntop/ntopng/issues/8770

Describe changes:

I have modified the webhook implementation to fix the JSON formatting issues.

Here are the key changes made:

Added a new formatAlert() function that standardizes each alert into a consistent structure:

  id = alert.alert_id,
  timestamp = alert.tstamp,
  timestamp_end = alert.tstamp_end,
  type = alert_consts.alertTypeLabel(...),
  severity = alert.alert_severity,
  score = alert.score,
  entity = {
    type = alert.entity_id,
    value = alert.entity_val
  },
  message = alert.alert,
  details = alert.info,
  source = {
    interface_id = alert.interface_id,
    probe_ip = alert.probe_ip
  },
  metadata = alert.metadata or {}
}
Enhanced the webhook message structure:
{
  version = webhook.API_VERSION,
  timestamp = os.time(),
  sharedsecret = settings.sharedsecret,
  alerts = formatted_alerts
}

Added consistent JSON encoding options:

These changes ensure:

Did not thoroughly test

MatteoBiscosi commented 1 month ago

Like i mentioned in the comment, it is not currently possible to merge the changes due to the fact that the formatAlert() function is going to add retrocompatibility issues (people that already matched the fields, by changing the name of the field, the matching is not going to work anymore). If you'd like to change some fields, please add new fields (possibly without duplicating the ones already existing). Moreover this PR does not address the issue #8770 , described as the JSON field not correctly formatted and the field not being consistent, however the fields you addressed should be present in all the alerts alwais. So please let me know if you found some cases where they were not. Meanwhile i'll talk with @DGabri directly to solve this issue.