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

Finding groups: (Auto) Group By and more enhancements #4353

Merged
merged 28 commits into from
May 10, 2021

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented Apr 24, 2021

Finding Group enhancements:

  • Allow to auto group on import/reimport (by component_name for example)
  • Fix sla display bug for empty groups
  • Allow edit of finding group on edit_finding page
  • Allow group by component name/version/filepath via bulk edit

Main feature is automatic grouping, so in the UI or API a group_by field has been added. Setting group_by to component_name on import or reimport will add all new findings automatically to a group based on the component_name field.
If push_to_jira or push_all is enabled, the groups will be pushed to JIRA.

There's also the possibility on the edit finding page now to modify the group for a finding.

While doing this PR I thought of some more things we could/should do for the finding groups, but ideally we get some more feedback first :-)

@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@valentijnscholten valentijnscholten changed the title WIP: Finding groups: (Auto) Group By and more enhancements Finding groups: (Auto) Group By and more enhancements Apr 28, 2021
@valentijnscholten valentijnscholten marked this pull request as ready for review April 28, 2021 13:31
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2021

Conflicts have been resolved. A maintainer will review the pull request shortly.

@madchap madchap mentioned this pull request May 2, 2021
@madchap
Copy link
Contributor

madchap commented May 2, 2021

I could make it crash.

I select like some 180 findings, some active, others inactive, yet other dups.. selected them all, and tried to create group by "component version".

Files I used I uploaded at DefectDojo/sample-scan-files@23cf6e6

Template error:
In template /app/dojo/templates/base.html, error at line 341
   '<' not supported between instances of 'NoneType' and 'int'
   331 :                                         <span>Calendar</span>
   332 :                                     </a>
   333 :                                 </li>
   334 :                             {% endif %}
   335 :                             {% if request.user.is_staff %}
   336 :                                 <li>
   337 :                                     <a href="#"><i class="fa fa-hospital-o fa-fw"> </i> <span>Questionnaires</span><span
   338 :                                             class="glyphicon arrow"></span></a>
   339 :                                     <ul class="nav nav-second-level">
   340 :                                         <li><a href="{% url 'questionnaire' %}">All Questionnaires </a></li>
   341 :                                          <li><a href="{% url  'questions' %}">All Questions </a></li>
   342 : 
   343 :                                     </ul>
   344 :                                     <!-- /.nav-second-level -->
   345 :                                 </li>
   346 :                             {% endif %}
   347 :                                 <li id="menu_configuration">
   348 : 				                    <a href="#" aria-expanded="false" aria-label="Configuration">
   349 :                                         <i class="fa fa-cog fa-fw"></i>
   350 :                                         <span>Configuration</span>
   351 :                                     </a>


Traceback:


File "./dojo/test/views.py" in view_test
  169.     return render(request, 'dojo/view_test.html',

File "/usr/local/lib/python3.8/site-packages/django/shortcuts.py" in render
  36.     content = loader.render_to_string(template_name, context, request, using=using)

File "/usr/local/lib/python3.8/site-packages/django/template/loader.py" in render_to_string
  62.     return template.render(context, request)

...

File "/usr/local/lib/python3.8/site-packages/django/template/base.py" in render_annotated
  904.             return self.render(context)

File "/usr/local/lib/python3.8/site-packages/django/template/base.py" in render
  987.             output = self.filter_expression.resolve(context)

File "/usr/local/lib/python3.8/site-packages/django/template/base.py" in resolve
  698.                 new_obj = func(obj, *arg_vals)

File "./dojo/templatetags/display_tags.py" in group_sla
  246.     return finding_sla(group)

File "./dojo/templatetags/display_tags.py" in finding_sla
  256.     find_sla = finding.sla_days_remaining()

File "./dojo/models.py" in sla_days_remaining
  2370.         return self.sla_days_remaining_internal

File "/usr/local/lib/python3.8/site-packages/django/utils/functional.py" in __get__
  80.         res = instance.__dict__[self.name] = self.func(instance)

File "./dojo/models.py" in sla_days_remaining_internal
  2367.         return min([find.sla_days_remaining() for find in self.findings.all()])

Exception Type: TypeError at /test/2
Exception Value: '<' not supported between instances of 'NoneType' and 'int'

@valentijnscholten
Copy link
Member Author

I could make it crash.

He who break its, shall fix it

I'll take a look, thanks for the sample files.

@valentijnscholten
Copy link
Member Author

Thanks @madchap, could reproduce. Problem was with findings of Info severity, which don't have an "SLA Days Remaining value" (and SLA Start Date). This is now fixed, also added some missed prefetches. Could you retest / confirm?

@madchap
Copy link
Contributor

madchap commented May 3, 2021

Well done

image

Question about groups of 1 finding. Should we maybe just pass on creating a group?

image

@valentijnscholten
Copy link
Member Author

The main usecase, I think, is to report findings per component to JIRA. So even if there's only one now, it makes sense to report as a group. On a next reimport there might be 2 vulns inside a component and then we could just update the existing jira issues for the group.

@madchap madchap requested a review from a team May 7, 2021 09:01
@valentijnscholten
Copy link
Member Author

Merging this as we need to deploy from dev and dogfood it. It's behind a feature flag, so should be good!

@valentijnscholten valentijnscholten merged commit c385871 into DefectDojo:dev May 10, 2021
dsever pushed a commit to dsever/django-DefectDojo that referenced this pull request May 18, 2021
* bulk edit: allow group by

* finding groups enhancements

* make auto_group_by optional, add help_text

* add auto_group_by to api

* finding groups: push groups only once on import/reimport

* jira: fix silent epic error during unit tests

* finding groups: push groups only once on import/reimport

* fix tests

* add unit tests for jira finding groups

* remove token

* rerecord JIRA

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* rename auto_group_by to group_by

* rename auto_group_by to group_by

* fix SLA for Info findings bug, prefetch

* fix SLA for Info findings bug, prefetch
madchap pushed a commit to madchap/django-DefectDojo that referenced this pull request May 30, 2023
* bulk edit: allow group by

* finding groups enhancements

* make auto_group_by optional, add help_text

* add auto_group_by to api

* finding groups: push groups only once on import/reimport

* jira: fix silent epic error during unit tests

* finding groups: push groups only once on import/reimport

* fix tests

* add unit tests for jira finding groups

* remove token

* rerecord JIRA

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* add unit tests for jira finding groups

* rename auto_group_by to group_by

* rename auto_group_by to group_by

* fix SLA for Info findings bug, prefetch

* fix SLA for Info findings bug, prefetch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants