Skip to content
This repository has been archived by the owner on Mar 15, 2018. It is now read-only.

Commit

Permalink
add vulnerabilitystatus to blocklist (bug 778365)
Browse files Browse the repository at this point in the history
  • Loading branch information
wraithan committed Aug 29, 2012
1 parent 869f107 commit 533a80f
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 4 deletions.
8 changes: 7 additions & 1 deletion apps/blocklist/admin.py
@@ -1,8 +1,13 @@
from django.contrib import admin

from . import forms
from . import models


class PluginAdmin(admin.ModelAdmin):
form = forms.BlocklistPluginForm


ms = models.BlocklistItem, models.BlocklistPlugin, models.BlocklistGfx
inlines = [type(cls.__name__ + 'Inline', (admin.StackedInline,),
{'model': cls})
Expand All @@ -12,9 +17,10 @@
class DetailAdmin(admin.ModelAdmin):
inlines = inlines


admin.site.register(models.BlocklistApp)
admin.site.register(models.BlocklistCA)
admin.site.register(models.BlocklistItem)
admin.site.register(models.BlocklistPlugin)
admin.site.register(models.BlocklistPlugin, PluginAdmin)
admin.site.register(models.BlocklistGfx)
admin.site.register(models.BlocklistDetail, DetailAdmin)
18 changes: 18 additions & 0 deletions apps/blocklist/forms.py
@@ -0,0 +1,18 @@
from django import forms

from .models import BlocklistPlugin


class BlocklistPluginForm(forms.ModelForm):
class Meta:
model = BlocklistPlugin

def clean(self):
severity = self.cleaned_data.get('severity')
vulnerability = self.cleaned_data.get('vulnerability_status')

if severity and vulnerability:
raise forms.ValidationError(
'Vulnerability status must be blank if Severity is non zero')

return self.cleaned_data
14 changes: 14 additions & 0 deletions apps/blocklist/models.py
Expand Up @@ -91,6 +91,10 @@ class BlocklistPlugin(BlocklistBase, amo.models.ModelBase):
description = models.CharField(max_length=255, blank=True, null=True)
filename = models.CharField(max_length=255, blank=True, null=True)
severity = models.SmallIntegerField(blank=True, null=True)
vulnerability_status = models.SmallIntegerField(blank=True, null=True,
choices=
((1, 'update available'),
(2, 'update unavailable')))
details = models.OneToOneField(BlocklistDetail, null=True)

class Meta(amo.models.ModelBase.Meta):
Expand All @@ -100,6 +104,16 @@ def __unicode__(self):
return '%s: %s - %s' % (self.name or self.guid or self.filename,
self.min, self.max)

@property
def get_vulnerability_status(self):
"""Returns vulnerability status per bug 778365
Returns None when criteria aren't met so jinja2 excludes it from when
using the attrs filter.
"""
if self.severity == 0 and self.vulnerability_status in (1,2):

This comment has been minimized.

Copy link
@andymckay

andymckay Aug 29, 2012

Contributor

the in (1,2) seems superfluous

This comment has been minimized.

Copy link
@wraithan

wraithan Aug 29, 2012

Author Contributor

With how finicky things are about the blocklist, I wanted to make sure that it was only valid values being returned. I can remove the in (1,2) it doesn't matter much to me.

This comment has been minimized.

Copy link
@andymckay

andymckay Aug 30, 2012

Contributor

I'm cool either way, mostly I couldn't find much else to complain about.

return self.vulnerability_status

def flush_urls(self):
return ['/blocklist*'] # no lang/app

Expand Down
8 changes: 5 additions & 3 deletions apps/blocklist/templates/blocklist/blocklist.xml
Expand Up @@ -30,19 +30,21 @@
{% if plugin.name %}<match name="name" exp="{{ plugin.name }}" />{% endif %}
{% if plugin.description %}<match name="description" exp="{{ plugin.description }}" />{% endif %}
{% if plugin.filename %}<match name="filename" exp="{{ plugin.filename }}" />{% endif %}
{% if plugin.severity or plugin.min or plugin.max %}
{% if plugin.severity or plugin.min or plugin.max or plugin.get_vulnerability_status %}
{% if plugin.guid %}
<versionRange {{ attrs(severity=plugin.severity) }}>
<versionRange {{ attrs(severity=plugin.severity, vulnerabilitystatus=plugin.get_vulnerability_status) }}>
{% if apiver > 2 and plugin.min and plugin.max %}
<targetApplication id="{{ plugin.guid }}">
<versionRange {{ attrs(minVersion=plugin.min, maxVersion=plugin.max) }} />
</targetApplication>
{% endif %}
</versionRange>
{% elif apiver > 2 and plugin.min and plugin.max %}
<versionRange {{ attrs(severity=plugin.severity, minVersion=plugin.min, maxVersion=plugin.max) }}></versionRange>
<versionRange {{ attrs(severity=plugin.severity, minVersion=plugin.min, maxVersion=plugin.max, vulnerabilitystatus=plugin.get_vulnerability_status) }}></versionRange>
{% elif plugin.severity and not (plugin.min or plugin.max) %}
<versionRange {{ attrs(severity=plugin.severity) }}></versionRange>
{% elif plugin.vulnerability_status %}
<versionRange {{ attrs(severity=plugin.severity, vulnerabilitystatus=plugin.get_vulnerability_status) }}></versionRange>
{% endif %}
{% endif %}
</pluginItem>
Expand Down
33 changes: 33 additions & 0 deletions apps/blocklist/tests.py
Expand Up @@ -323,6 +323,21 @@ def test_plugin_with_target_app(self):
self.plugin.update(guid=amo.FIREFOX.guid, severity=1, min='1', max='2')
vr = self.dom().getElementsByTagName('versionRange')[0]
eq_(vr.getAttribute('severity'), '1')
assert not vr.getAttribute('vulnerabilitystatus')

app = vr.getElementsByTagName('targetApplication')[0]
eq_(app.getAttribute('id'), amo.FIREFOX.guid)

vr = app.getElementsByTagName('versionRange')[0]
eq_(vr.getAttribute('minVersion'), '1')
eq_(vr.getAttribute('maxVersion'), '2')

def test_plugin_with_target_app_with_vulnerability(self):
self.plugin.update(guid=amo.FIREFOX.guid, severity=0, min='1', max='2',
vulnerability_status=2)
vr = self.dom().getElementsByTagName('versionRange')[0]
eq_(vr.getAttribute('severity'), '0')
eq_(vr.getAttribute('vulnerabilitystatus'), '2')

app = vr.getElementsByTagName('targetApplication')[0]
eq_(app.getAttribute('id'), amo.FIREFOX.guid)
Expand All @@ -335,12 +350,30 @@ def test_plugin_with_severity_only(self):
self.plugin.update(guid=None, severity=1)
vr = self.dom().getElementsByTagName('versionRange')[0]
eq_(vr.getAttribute('severity'), '1')
assert not vr.getAttribute('vulnerabilitystatus')
eq_(vr.getAttribute('minVersion'), '')
eq_(vr.getAttribute('maxVersion'), '')

eq_(vr.getElementsByTagName('targetApplication'), [],
'There should not be a <targetApplication> if there was no app')

def test_plugin_without_severity_and_with_vulnerability(self):
self.plugin.update(guid=None, severity=0, vulnerability_status=1)
vr = self.dom().getElementsByTagName('versionRange')[0]
eq_(vr.getAttribute('severity'), '0')
eq_(vr.getAttribute('vulnerabilitystatus'), '1')
eq_(vr.getAttribute('minVersion'), '')
eq_(vr.getAttribute('maxVersion'), '')

def test_plugin_without_severity_and_with_vulnerability_and_minmax(self):
self.plugin.update(guid=None, severity=0, vulnerability_status=1,
min='2.0', max='3.0')
vr = self.dom().getElementsByTagName('versionRange')[0]
eq_(vr.getAttribute('severity'), '0')
eq_(vr.getAttribute('vulnerabilitystatus'), '1')
eq_(vr.getAttribute('minVersion'), '2.0')
eq_(vr.getAttribute('maxVersion'), '3.0')

def test_plugin_apiver_lt_3(self):
self.plugin.update(severity='2')
# No min & max so the app matches.
Expand Down
1 change: 1 addition & 0 deletions migrations/463-add-vulnerability-status-to-blocklist.sql
@@ -0,0 +1 @@
alter table blplugins add column vulnerability_status tinyint(1) unsigned default null;

0 comments on commit 533a80f

Please sign in to comment.