launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28462
[Merge] ~lgp171188/launchpad:vulnerability-api-fixes into launchpad:master
Guruprasad has proposed merging ~lgp171188/launchpad:vulnerability-api-fixes into launchpad:master.
Commit message:
Fix the vulnerability attributes editing in the web service
Also add the missing `privacy` attribute in the `Vulnerability` model.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/422726
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:vulnerability-api-fixes into launchpad:master.
diff --git a/lib/lp/bugs/interfaces/vulnerability.py b/lib/lp/bugs/interfaces/vulnerability.py
index 79a0312..b74b71a 100644
--- a/lib/lp/bugs/interfaces/vulnerability.py
+++ b/lib/lp/bugs/interfaces/vulnerability.py
@@ -96,27 +96,6 @@ class IVulnerabilityView(Interface):
as_of="devel"
)
- distribution = exported(
- Reference(
- IDistribution,
- title=_("Distribution"),
- required=True,
- readonly=True
- ),
- as_of="devel"
- )
-
- cve = exported(
- Reference(
- ICve,
- title=_('External CVE reference corresponding'
- ' to this vulnerability, if any.'),
- required=False,
- readonly=True
- ),
- as_of="devel"
- )
-
date_created = exported(
Datetime(
title=_("The date this vulnerability was created."),
@@ -142,6 +121,26 @@ class IVulnerabilityEditableAttributes(Interface):
These attributes need launchpad.View to see, and launchpad.Edit to change.
"""
+ distribution = exported(
+ Reference(
+ IDistribution,
+ title=_("Distribution"),
+ required=True,
+ readonly=False,
+ ),
+ as_of="devel"
+ )
+
+ cve = exported(
+ Reference(
+ ICve,
+ title=_('External CVE reference corresponding'
+ ' to this vulnerability, if any.'),
+ required=False,
+ readonly=False,
+ ),
+ as_of="devel"
+ )
status = exported(
Choice(
diff --git a/lib/lp/bugs/model/tests/test_vulnerability.py b/lib/lp/bugs/model/tests/test_vulnerability.py
index a9d7bc2..f2cac72 100644
--- a/lib/lp/bugs/model/tests/test_vulnerability.py
+++ b/lib/lp/bugs/model/tests/test_vulnerability.py
@@ -4,7 +4,9 @@
"""Tests for the vulnerability and related models."""
from zope.component import getUtility
+from lp.bugs.interfaces.buglink import IBugLinkTarget
from lp.bugs.interfaces.vulnerability import (
+ IVulnerability,
IVulnerabilitySet,
VulnerabilityChange,
)
@@ -29,6 +31,14 @@ class TestVulnerability(TestCaseWithFactory):
self.vulnerability = self.factory.makeVulnerability(
distribution=self.distribution)
+ def test_Vulnerability_implements_IVulnerability(self):
+ vulnerability = self.factory.makeVulnerability()
+ self.assertTrue(verifyObject(IVulnerability, vulnerability))
+
+ def test_Vulnerability_implements_IBugLinkTarget(self):
+ vulnerability = self.factory.makeVulnerability()
+ self.assertTrue(verifyObject(IBugLinkTarget, vulnerability))
+
def test_random_user(self):
with person_logged_in(self.factory.makePerson()):
self.assertTrue(
diff --git a/lib/lp/bugs/model/vulnerability.py b/lib/lp/bugs/model/vulnerability.py
index f10ece1..e10b2d5 100644
--- a/lib/lp/bugs/model/vulnerability.py
+++ b/lib/lp/bugs/model/vulnerability.py
@@ -19,6 +19,7 @@ from zope.component import getUtility
from zope.interface import implementer
from lp.app.enums import InformationType
+from lp.app.model.launchpad import InformationTypeMixin
from lp.bugs.enums import VulnerabilityStatus
from lp.bugs.interfaces.buglink import IBugLinkTarget
from lp.bugs.interfaces.bugtask import BugTaskImportance
@@ -40,7 +41,7 @@ from lp.services.xref.interfaces import IXRefSet
@implementer(IVulnerability, IBugLinkTarget)
-class Vulnerability(StormBase, BugLinkTargetMixin):
+class Vulnerability(StormBase, BugLinkTargetMixin, InformationTypeMixin):
__storm_table__ = 'Vulnerability'
id = Int(primary=True)
diff --git a/lib/lp/bugs/tests/test_vulnerability.py b/lib/lp/bugs/tests/test_vulnerability.py
new file mode 100644
index 0000000..9334bb3
--- /dev/null
+++ b/lib/lp/bugs/tests/test_vulnerability.py
@@ -0,0 +1,184 @@
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for lp.bugs.interface.IVulnerability exposed via the web service."""
+
+import datetime
+import json
+
+import pytz
+from testtools.matchers import MatchesStructure
+from zope.security.proxy import removeSecurityProxy
+
+from lp.app.enums import InformationType
+from lp.bugs.enums import VulnerabilityStatus
+from lp.bugs.interfaces.bugtask import BugTaskImportance
+from lp.services.webapp.interfaces import OAuthPermission
+from lp.testing import (
+ api_url,
+ TestCaseWithFactory,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.pages import webservice_for_person
+
+
+class TestVulnerabilityWebService(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_editing_an_existing_vulnerability_attributes(self):
+ vulnerability = removeSecurityProxy(
+ self.factory.makeVulnerability()
+ )
+ owner = vulnerability.distribution.owner
+
+ self.assertThat(
+ vulnerability,
+ MatchesStructure.byEquality(
+ status=VulnerabilityStatus.NEEDS_TRIAGE,
+ importance=BugTaskImportance.UNDECIDED,
+ information_type=InformationType.PUBLIC,
+ cve=None,
+ description=None,
+ notes=None,
+ mitigation=None,
+ date_made_public=None,
+ )
+ )
+ vulnerability_url = api_url(vulnerability)
+ webservice = webservice_for_person(
+ owner,
+ permission=OAuthPermission.WRITE_PRIVATE,
+ default_api_version="devel"
+ )
+ now = datetime.datetime.now(pytz.UTC)
+
+ response = webservice.patch(
+ vulnerability_url,
+ "application/json",
+ json.dumps(
+ {
+ "status": "Active",
+ "information_type": "Private",
+ "importance": "Critical",
+ "description": "Foo bar",
+ "notes": "lgp171188> Foo bar",
+ "mitigation": "Foo bar baz",
+ "importance_explanation": "Foo bar bazz",
+ "date_made_public": now.isoformat(),
+ }
+ )
+ )
+ self.assertEqual(209, response.status)
+ self.assertThat(
+ vulnerability,
+ MatchesStructure.byEquality(
+ status=VulnerabilityStatus.ACTIVE,
+ importance=BugTaskImportance.CRITICAL,
+ description="Foo bar",
+ notes="lgp171188> Foo bar",
+ mitigation="Foo bar baz",
+ importance_explanation="Foo bar bazz",
+ date_made_public=now,
+ )
+ )
+
+ def test_creator_of_a_vulnerability_read_only(self):
+ person = self.factory.makePerson()
+ distribution = self.factory.makeDistribution()
+ vulnerability = removeSecurityProxy(
+ self.factory.makeVulnerability(
+ distribution=distribution,
+ creator=distribution.owner
+ )
+ )
+ vulnerability_url = api_url(vulnerability)
+ person_url = api_url(person)
+ webservice = webservice_for_person(
+ person,
+ permission=OAuthPermission.WRITE_PRIVATE,
+ default_api_version="devel"
+ )
+
+ response = webservice.patch(
+ vulnerability_url,
+ "application/json",
+ json.dumps(
+ {
+ "creator_link": person_url,
+ }
+ )
+ )
+ self.assertEqual(400, response.status)
+ self.assertEqual(
+ b'creator_link: You tried to modify a read-only attribute.',
+ response.body
+ )
+
+ def test_editing_an_existing_vulnerability_associated_objects(self):
+ person = self.factory.makePerson()
+ distribution = removeSecurityProxy(
+ self.factory.makeDistribution(owner=person)
+ )
+ cve = self.factory.makeCVE("2022-1234")
+ vulnerability = removeSecurityProxy(
+ self.factory.makeVulnerability(
+ distribution=distribution,
+ creator=person,
+ cve=cve
+ )
+ )
+ vulnerability_url = api_url(vulnerability)
+ api_base = "http://api.launchpad.test/devel"
+ another_distribution = removeSecurityProxy(
+ self.factory.makeDistribution(owner=person)
+ )
+ distribution_url = api_base + api_url(another_distribution)
+ another_cve = self.factory.makeCVE('2022-1235')
+ cve_url = api_base + api_url(another_cve)
+
+ self.assertThat(
+ vulnerability,
+ MatchesStructure.byEquality(
+ distribution=distribution,
+ cve=cve,
+ creator=person,
+ )
+ )
+ self.assertIn(distribution.name, vulnerability_url)
+
+ webservice = webservice_for_person(
+ person,
+ permission=OAuthPermission.WRITE_PRIVATE,
+ default_api_version="devel"
+ )
+
+ response = webservice.patch(
+ vulnerability_url,
+ "application/json",
+ json.dumps(
+ {
+ "cve_link": cve_url,
+ "distribution_link": distribution_url,
+ }
+ )
+ )
+ # There is a redirect because the vulnerability is now
+ # associated with a different distribution and hence its
+ # URL will no longer be the same.
+ self.assertEqual(301, response.status)
+ new_url = response.headers['Location']
+ new_vulnerability_url = api_base + api_url(vulnerability)
+ self.assertIn(another_distribution.name, new_vulnerability_url)
+
+ self.assertEqual(
+ api_base + api_url(vulnerability),
+ new_url
+ )
+ self.assertThat(
+ vulnerability,
+ MatchesStructure.byEquality(
+ cve=another_cve,
+ distribution=another_distribution
+ )
+ )