Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

CVE page: optimise, fix popovers #628

Closed
wants to merge 4 commits into from
Closed

CVE page: optimise, fix popovers #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99891d9
Don't fetch all devices for every package.
a-martynovich Jan 17, 2020
c3e9fbe
Optimize CVEView by using window queries.
a-martynovich Jan 21, 2020
eea540e
Fix tests, bring back Affected Nodes popovers.
a-martynovich Jan 21, 2020
c832677
Don't scroll to the top when clicking on popover links.
a-martynovich Jan 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions backend/device_registry/templates/cve.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ <h1 style="margin-bottom: 0">CVE list{% if device_name %} for {{ device_name }}{
<td>
{% for p in row.packages %}
<a href="#" class="wott-popover">
{{ p.device_urls|length }}
{{ p.devices_count }}
<template>
{% for du in p.device_urls %}
{% for du in p.devices %}
<a href="{{ du.href }}">{{ du.text }}</a><br>
{% endfor %}
</template>
Expand Down Expand Up @@ -76,16 +76,36 @@ <h1 style="margin-bottom: 0">CVE list{% if device_name %} for {{ device_name }}{
{{ block.super }}

<script>
let popoverShown = null;

$(function () {
$('[data-toggle="popover"]').popover()
});

$('.wott-popover').popover({
$('.wott-popover').click((e) => {
e.preventDefault(); // prevent scrolling to the top (because <a href="#">)
if(popoverShown) // hide previous popover
$(popoverShown).popover('hide');
$(e.target).popover('show');
popoverShown = e.target;
}).popover({
html: true,
trigger: 'click',
trigger: 'manual',
title: 'Details',
content: function() {
return $(this).children('template').html();
return $(this).children('template').html();
}
});

$('body').click((e) => {
if(e.target == popoverShown || $(e.target).parents('.popover').length) {
// clicked on popover link or inside popover
return;
}
if(popoverShown) {
// clicked anywhere outside the popover while it's shown -> hide it
$(popoverShown).popover('hide');
popoverShown = null;
}
})
</script>
Expand Down
32 changes: 18 additions & 14 deletions backend/device_registry/tests/test_all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,10 @@ def setUp(self):
self.device0.deb_packages.set(self.packages)
self.device_unrelated.deb_packages.set(self.packages)

def _hyperlinks(self, devices):
return [CVEView.Hyperlink(text=device.get_name(), href=reverse('device_cve', kwargs={'device_pk': device.pk}))
for device in devices]

def test_sort_package_hosts_affected(self):
self.packages[0].vulnerabilities.set(self.vulns)
self.packages[1].vulnerabilities.set(self.vulns[1:])
Expand All @@ -1423,11 +1427,11 @@ def test_sort_package_hosts_affected(self):
self.assertListEqual(response.context_data['table_rows'], [
CVEView.TableRow(cve_name='CVE-2018-2', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
# These two AffectedPackage's should be sorted by hosts_affected
CVEView.AffectedPackage('one_second', [self.device0, self.device1]),
CVEView.AffectedPackage('one_first', [self.device0])
CVEView.AffectedPackage('one_second', 2, self._hyperlinks([self.device0, self.device1])),
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0]))
], cve_date=self.today),
CVEView.TableRow(cve_name='CVE-2018-1', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
CVEView.AffectedPackage('one_first', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0]))
])
])

Expand All @@ -1441,15 +1445,15 @@ def test_sort_urgency(self):
self.assertListEqual(response.context_data['table_rows'], [
# These two TableRow's should be sorted by urgency
CVEView.TableRow(cve_name='CVE-2018-2', cve_url='', urgency=Vulnerability.Urgency.HIGH, packages=[
CVEView.AffectedPackage('one_first', [self.device0]),
CVEView.AffectedPackage('one_second', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0])),
CVEView.AffectedPackage('one_second', 1, self._hyperlinks([self.device0]))
], cve_date=self.today),
CVEView.TableRow(cve_name='CVE-2018-1', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
CVEView.AffectedPackage('one_first', [self.device0]),
CVEView.AffectedPackage('one_second', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0])),
CVEView.AffectedPackage('one_second', 1, self._hyperlinks([self.device0]))
])
])
self.assertEqual(len(response.context_data['table_rows'][0].packages[0].device_urls), 1)
# self.assertEqual(len(response.context_data['table_rows'][0].packages[0].device_urls), 1)

def test_sort_total_hosts_affected(self):
self.packages[0].vulnerabilities.set(self.vulns)
Expand All @@ -1460,11 +1464,11 @@ def test_sort_total_hosts_affected(self):
self.assertListEqual(response.context_data['table_rows'], [
# These two TableRow's should be sorted by the sum of hosts_affected
CVEView.TableRow(cve_name='CVE-2018-2', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
CVEView.AffectedPackage('one_first', [self.device0]),
CVEView.AffectedPackage('one_second', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0])),
CVEView.AffectedPackage('one_second', 1, self._hyperlinks([self.device0]))
], cve_date=self.today),
CVEView.TableRow(cve_name='CVE-2018-1', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
CVEView.AffectedPackage('one_first', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0]))
])
])

Expand All @@ -1479,11 +1483,11 @@ def test_filter_device(self):
self.assertEqual(response.status_code, 200)
self.assertListEqual(response.context_data['table_rows'], [
CVEView.TableRow(cve_name='CVE-2018-2', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
CVEView.AffectedPackage('one_first', [self.device0]),
CVEView.AffectedPackage('one_second', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0])),
CVEView.AffectedPackage('one_second', 1, self._hyperlinks([self.device0]))
], cve_date=self.today),
CVEView.TableRow(cve_name='CVE-2018-1', cve_url='', urgency=Vulnerability.Urgency.LOW, packages=[
CVEView.AffectedPackage('one_first', [self.device0])
CVEView.AffectedPackage('one_first', 1, self._hyperlinks([self.device0]))
])
])

Expand Down
74 changes: 40 additions & 34 deletions backend/device_registry/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.contrib.auth.decorators import login_required
from django.contrib.auth.mixins import LoginRequiredMixin
from django.db import transaction
from django.db.models import Case, When, Count, F
from django.db.models import Case, When, Count, Window
from django.db.models import Q, Sum, Avg, IntegerField, Max
from django.http import HttpResponseRedirect, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
from django.shortcuts import get_object_or_404
Expand Down Expand Up @@ -597,14 +597,8 @@ class Hyperlink(NamedTuple):

class AffectedPackage(NamedTuple):
name: str
devices: List[Device]

@property
def device_urls(self):
return [CVEView.Hyperlink(
d.get_name(),
reverse('device_cve', kwargs={'device_pk': d.pk})
) for d in self.devices]
devices_count: int
devices: List[NamedTuple]

@property
def upgrade_command(self):
Expand All @@ -626,7 +620,7 @@ class TableRow(NamedTuple):

@property
def key(self):
return self.urgency, sum([len(p.devices) for p in self.packages])
return self.urgency, sum([p.devices_count for p in self.packages])

@property
def severity(self):
Expand All @@ -644,36 +638,48 @@ def get_context_data(self, **kwargs):
if device_pk is not None:
device = get_object_or_404(Device, pk=device_pk, owner=user)
vuln_query = Q(debpackage__device__pk=device_pk)
packages_query = Q(device__pk=device_pk)
else:
device = None
vuln_query = Q(debpackage__device__owner=user)
packages_query = Q(device__owner=user)

vulns = Vulnerability.objects.filter(vuln_query, fix_available=True) \
.values('name').distinct().annotate(max_urgency=Max('urgency'),
pubdate=Max('pub_date'))
vuln_info = {v['name']: (v['max_urgency'], v['pubdate']) for v in vulns}
packages = DebPackage.objects.filter(packages_query,
vulnerabilities__isnull=False,
vulnerabilities__fix_available=True)\
.annotate(cve_name=F('vulnerabilities__name'))

packages_by_cve = defaultdict(set)
for p in packages:
packages_by_cve[p.cve_name].add(p)
vuln_names = Vulnerability.objects.only('name').filter(debpackage__device__owner=user,
fix_available=True).values('name')
vuln_pub_dates_qs = Vulnerability.objects.only('name').filter(name__in=vuln_names) \
.values('name').annotate(pubdate=Max('pub_date')).distinct()
vuln_pub_dates = {v['name']: v['pubdate'] for v in vuln_pub_dates_qs}

qs0 = Vulnerability.objects.filter(fix_available=True) \
.values('name').annotate(max_urgency=Max('urgency')) \
.filter(vuln_query) \
.values('name', 'max_urgency', 'debpackage__pk', 'debpackage__device__pk', 'debpackage__name', 'debpackage__device__name', 'debpackage__device__deviceinfo__fqdn')
qs1 = qs0.annotate(devcnt=Window(expression=Count('debpackage__name'),
partition_by=['name', 'debpackage__name']),
cvecnt=Window(expression=Count('debpackage__name'),
partition_by=['name'])) \
.order_by('-max_urgency', '-cvecnt', 'name', '-devcnt', 'debpackage__name')

table_rows = []
for cve_name, cve_packages in packages_by_cve.items():
plist = sorted([self.AffectedPackage(package.name,
# FIXME: this line may need additional optimisation to avoid calling
# device_set.filter() every time.
[device] if device else list(package.device_set.filter(owner=user)))
for package in cve_packages],
key=lambda package: len(package.devices), reverse=True)
urgency, cve_date = vuln_info[cve_name]
table_rows.append(self.TableRow(cve_name=cve_name, cve_url='', urgency=urgency,
cve_date=cve_date, packages=plist))
current_row = None
current_package = None
for device_package_cve in qs1:
cve_name, package_name, urgency, devices_count,\
device_pk, device_name, device_fqdn = (device_package_cve[k] for k in [
'name', 'debpackage__name', 'max_urgency', 'devcnt',
'debpackage__device__pk', 'debpackage__device__name',
'debpackage__device__deviceinfo__fqdn'])
if not current_row or current_row.cve_name != cve_name:
current_row = self.TableRow(cve_name, urgency, [], '', vuln_pub_dates[cve_name])
table_rows.append(current_row)
current_package = None
if not current_package or current_package.name != package_name:
current_package = self.AffectedPackage(package_name, devices_count, [])
current_row.packages.append(current_package)
if device_name or device_fqdn:
device_pretty_name = device_name or device_fqdn[:36]
else:
device_pretty_name = f"device_{device_pk}"
current_package.devices.append(self.Hyperlink(href=reverse('device_cve', kwargs={'device_pk': device_pk}),
text=device_pretty_name))

context['table_rows'] = sorted(table_rows,
key=lambda r: r.key, reverse=True)
Expand Down