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

Fix create notification for group of findings #10433

Merged

Conversation

ccronca
Copy link
Contributor

@ccronca ccronca commented Jun 19, 2024

Description

DefectDojo fails to process comments when closing a Jira issue associated with a group of findings, if the issue is closed with a comment. The error occurs because jissue.finding is not set when the Jira issue is associated with a group of findings. However, it is passed as a parameter to the create_notification function, which then fails when trying to get the product from it.

The proposed fix is to pass the first finding from the group, if it exists. This finding can be used as a parameter for the create_notification function, as the function uses it to retrieve notification parameters from the product configuration associated with the finding. Since all findings in the same group share the same product, using the first finding should work.

Additionally, I have updated the findings variable. It was previously created as a list with a Queryset, which is already an iterable object, so it does not need to be inside a list.

Test results

I don't believe there are tests for this

Documentation

This is a minor bug fix that shouldn't need any documentation updates

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Extra information

The following example demonstrates the error that occurs when closing a Jira issue associated with a group of findings, if the issue is closed with a comment:

[18/Jun/2024 13:04:08] ERROR [dojo.jira_link.views:124] 'NoneType' object has no attribute 'test'
Traceback (most recent call last):
  File "/app/dojo/jira_link/views.py", line 109, in webhook
    if (error_response := check_for_and_create_comment(parsed)) is not None:
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/dojo/jira_link/views.py", line 214, in check_for_and_create_comment
    create_notification(event='other', title=f'JIRA incoming comment - {jissue.finding}', finding=jissue.finding, url=reverse("view_finding_group", args=(jissue.finding_group.id,)), icon='check')
  File "/app/dojo/notifications/helper.py", line 64, in create_notification
    product = kwargs['finding'].test.engagement.product
              ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'test'
[18/Jun/2024 13:04:08] INFO [django.request:241] OK: /jira/webhook/....
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 56, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 55, in wrapped_view
    return view_func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/decorators/http.py", line 43, in inner
    return func(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/dojo/jira_link/views.py", line 109, in webhook
    if (error_response := check_for_and_create_comment(parsed)) is not None:
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/dojo/jira_link/views.py", line 214, in check_for_and_create_comment
    create_notification(event='other', title=f'JIRA incoming comment - {jissue.finding}', finding=jissue.finding, url=reverse("view_finding_group", args=(jissue.finding_group.id,)), icon='check')
  File "/app/dojo/notifications/helper.py", line 64, in create_notification
    product = kwargs['finding'].test.engagement.product
              ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'test'
 

Copy link

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Server-Side Request Forgery Analyzer 0 findings
Configured Codepaths Analyzer 3 findings
IDOR Analyzer 0 findings
Sensitive Files Analyzer 0 findings
SQL Injection Analyzer 0 findings
Authn/Authz Analyzer 0 findings
Secrets Analyzer 0 findings

Note

🔴 Risk threshold exceeded. Adding a reviewer if one is configured in .dryrunsecurity.yaml.

notification list: @mtesauro @grendel513

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The code changes in this pull request focus on improving the handling of JIRA webhook events in the DefectDojo application. The key changes include:

  1. Webhook Handling: The webhook function ensures that the JIRA integration is enabled and the webhook is configured correctly before processing the incoming JIRA webhook events, such as "comment_created" and "jira:issue_updated".

  2. Comment Handling: The check_for_and_create_comment function processes incoming JIRA comments and creates a new note in DefectDojo with the comment details, if the comment is associated with a DefectDojo finding or finding group.

  3. Issue Update Handling: When a JIRA issue is updated, the code processes the update and updates the corresponding DefectDojo finding or finding group with the new resolution status, assignee, and other relevant information.

From an application security perspective, the code appears to handle the incoming JIRA webhook events securely. It checks the request content type, ensures the JIRA integration is enabled, and verifies the webhook secret (if configured) before processing the request. This helps prevent unauthorized access and potential abuse of the webhook functionality. Additionally, the code handles exceptions and errors gracefully, logging relevant information and returning appropriate HTTP responses, which can improve debugging and monitoring.

Files Changed:

  • dojo/jira_link/views.py: This file contains the changes related to the handling of JIRA webhooks in the DefectDojo application. The key changes include:
    • The webhook function that processes incoming JIRA webhook events.
    • The check_for_and_create_comment function that processes incoming JIRA comments and creates new notes in DefectDojo.
    • The handling of JIRA issue updates, where the code updates the corresponding DefectDojo finding or finding group with the new resolution status, assignee, and other relevant information.

Powered by DryRun Security

@ccronca ccronca changed the title Fix create notification execution for group of findings Fix create notification for group of findings Jun 19, 2024
Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

@mtesauro mtesauro merged commit f9dfd29 into DefectDojo:bugfix Jun 20, 2024
123 of 124 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants