← Back to team overview

launchpad-reviewers team mailing list archive

[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
+            )
+        )