Launchpad itself

Merge ~lgp171188/launchpad:vulnerability-api-fixes into launchpad:master

  1. Git
  2. lp:~lgp171188/launchpad
  3. vulnerability-api-fixes
  4. Merge into master
Proposed by Guruprasad
Status: Merged
Approved by: Guruprasad
Approved revision: 17bb3a7422b67545012fd5ba1c151bc4d4d5ffa5
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~lgp171188/launchpad:vulnerability-api-fixes
Merge into: launchpad:master
Diff against target: 324 lines (+251/-12)
4 files modified
lib/lp/bugs/interfaces/vulnerability.py (+10/-11)
lib/lp/bugs/model/tests/test_vulnerability.py (+10/-0)
lib/lp/bugs/model/vulnerability.py (+2/-1)
lib/lp/bugs/tests/test_vulnerability.py (+229/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Andrey Fedoseev (community) Approve
Review via email: mp+422726@code.launchpad.net

Commit message

Fix the vulnerability attributes editing in the web service

Also add the missing `privacy` attribute in the `Vulnerability` model.

To post a comment you must log in.
Revision history for this message
Andrey Fedoseev (andrey-fedoseev) wrote :

LGTM

review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

Can you explain why it was necessary to make `distribution` and `cve` writable?

review: Needs Information
Revision history for this message
Guruprasad (lgp171188) wrote :

Colin, I wanted `cve` to be writeable since it is an optional field that can be set later. While doing that, I also thought if it made sense to allow changing the distribution (not sure if this is a valid use-case) and added it.

I will now make it read-only. 👍

Revision history for this message
Guruprasad (lgp171188) wrote :

Colin, I have made `distribution` read-only now and updated the MP.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Guruprasad (lgp171188) :
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/bugs/interfaces/vulnerability.py b/lib/lp/bugs/interfaces/vulnerability.py
2index 79a0312..7e76c8c 100644
3--- a/lib/lp/bugs/interfaces/vulnerability.py
4+++ b/lib/lp/bugs/interfaces/vulnerability.py
5@@ -106,17 +106,6 @@ class IVulnerabilityView(Interface):
6 as_of="devel"
7 )
8
9- cve = exported(
10- Reference(
11- ICve,
12- title=_('External CVE reference corresponding'
13- ' to this vulnerability, if any.'),
14- required=False,
15- readonly=True
16- ),
17- as_of="devel"
18- )
19-
20 date_created = exported(
21 Datetime(
22 title=_("The date this vulnerability was created."),
23@@ -142,6 +131,16 @@ class IVulnerabilityEditableAttributes(Interface):
24
25 These attributes need launchpad.View to see, and launchpad.Edit to change.
26 """
27+ cve = exported(
28+ Reference(
29+ ICve,
30+ title=_('External CVE reference corresponding'
31+ ' to this vulnerability, if any.'),
32+ required=False,
33+ readonly=False,
34+ ),
35+ as_of="devel"
36+ )
37
38 status = exported(
39 Choice(
40diff --git a/lib/lp/bugs/model/tests/test_vulnerability.py b/lib/lp/bugs/model/tests/test_vulnerability.py
41index a9d7bc2..f2cac72 100644
42--- a/lib/lp/bugs/model/tests/test_vulnerability.py
43+++ b/lib/lp/bugs/model/tests/test_vulnerability.py
44@@ -4,7 +4,9 @@
45 """Tests for the vulnerability and related models."""
46 from zope.component import getUtility
47
48+from lp.bugs.interfaces.buglink import IBugLinkTarget
49 from lp.bugs.interfaces.vulnerability import (
50+ IVulnerability,
51 IVulnerabilitySet,
52 VulnerabilityChange,
53 )
54@@ -29,6 +31,14 @@ class TestVulnerability(TestCaseWithFactory):
55 self.vulnerability = self.factory.makeVulnerability(
56 distribution=self.distribution)
57
58+ def test_Vulnerability_implements_IVulnerability(self):
59+ vulnerability = self.factory.makeVulnerability()
60+ self.assertTrue(verifyObject(IVulnerability, vulnerability))
61+
62+ def test_Vulnerability_implements_IBugLinkTarget(self):
63+ vulnerability = self.factory.makeVulnerability()
64+ self.assertTrue(verifyObject(IBugLinkTarget, vulnerability))
65+
66 def test_random_user(self):
67 with person_logged_in(self.factory.makePerson()):
68 self.assertTrue(
69diff --git a/lib/lp/bugs/model/vulnerability.py b/lib/lp/bugs/model/vulnerability.py
70index f10ece1..e10b2d5 100644
71--- a/lib/lp/bugs/model/vulnerability.py
72+++ b/lib/lp/bugs/model/vulnerability.py
73@@ -19,6 +19,7 @@ from zope.component import getUtility
74 from zope.interface import implementer
75
76 from lp.app.enums import InformationType
77+from lp.app.model.launchpad import InformationTypeMixin
78 from lp.bugs.enums import VulnerabilityStatus
79 from lp.bugs.interfaces.buglink import IBugLinkTarget
80 from lp.bugs.interfaces.bugtask import BugTaskImportance
81@@ -40,7 +41,7 @@ from lp.services.xref.interfaces import IXRefSet
82
83
84 @implementer(IVulnerability, IBugLinkTarget)
85-class Vulnerability(StormBase, BugLinkTargetMixin):
86+class Vulnerability(StormBase, BugLinkTargetMixin, InformationTypeMixin):
87 __storm_table__ = 'Vulnerability'
88
89 id = Int(primary=True)
90diff --git a/lib/lp/bugs/tests/test_vulnerability.py b/lib/lp/bugs/tests/test_vulnerability.py
91new file mode 100644
92index 0000000..be3457a
93--- /dev/null
94+++ b/lib/lp/bugs/tests/test_vulnerability.py
95@@ -0,0 +1,229 @@
96+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
97+# GNU Affero General Public License version 3 (see the file LICENSE).
98+
99+"""Tests for lp.bugs.interface.IVulnerability exposed via the web service."""
100+
101+import datetime
102+import json
103+
104+import pytz
105+from testtools.matchers import MatchesStructure
106+from zope.security.proxy import removeSecurityProxy
107+
108+from lp.app.enums import InformationType
109+from lp.bugs.enums import VulnerabilityStatus
110+from lp.bugs.interfaces.bugtask import BugTaskImportance
111+from lp.services.webapp.interfaces import OAuthPermission
112+from lp.testing import (
113+ api_url,
114+ TestCaseWithFactory,
115+ )
116+from lp.testing.layers import DatabaseFunctionalLayer
117+from lp.testing.pages import webservice_for_person
118+
119+
120+class TestVulnerabilityWebService(TestCaseWithFactory):
121+
122+ layer = DatabaseFunctionalLayer
123+
124+ def test_editing_an_existing_vulnerability_attributes(self):
125+ vulnerability = removeSecurityProxy(
126+ self.factory.makeVulnerability()
127+ )
128+ owner = vulnerability.distribution.owner
129+
130+ self.assertThat(
131+ vulnerability,
132+ MatchesStructure.byEquality(
133+ status=VulnerabilityStatus.NEEDS_TRIAGE,
134+ importance=BugTaskImportance.UNDECIDED,
135+ information_type=InformationType.PUBLIC,
136+ cve=None,
137+ description=None,
138+ notes=None,
139+ mitigation=None,
140+ date_made_public=None,
141+ )
142+ )
143+ vulnerability_url = api_url(vulnerability)
144+ webservice = webservice_for_person(
145+ owner,
146+ permission=OAuthPermission.WRITE_PRIVATE,
147+ default_api_version="devel"
148+ )
149+ now = datetime.datetime.now(pytz.UTC)
150+
151+ response = webservice.patch(
152+ vulnerability_url,
153+ "application/json",
154+ json.dumps(
155+ {
156+ "status": "Active",
157+ "information_type": "Private",
158+ "importance": "Critical",
159+ "description": "Foo bar",
160+ "notes": "lgp171188> Foo bar",
161+ "mitigation": "Foo bar baz",
162+ "importance_explanation": "Foo bar bazz",
163+ "date_made_public": now.isoformat(),
164+ }
165+ )
166+ )
167+ self.assertEqual(209, response.status)
168+ self.assertThat(
169+ vulnerability,
170+ MatchesStructure.byEquality(
171+ status=VulnerabilityStatus.ACTIVE,
172+ importance=BugTaskImportance.CRITICAL,
173+ description="Foo bar",
174+ notes="lgp171188> Foo bar",
175+ mitigation="Foo bar baz",
176+ importance_explanation="Foo bar bazz",
177+ date_made_public=now,
178+ )
179+ )
180+
181+ def test_creator_of_a_vulnerability_read_only(self):
182+ person = self.factory.makePerson()
183+ distribution = self.factory.makeDistribution()
184+ vulnerability = removeSecurityProxy(
185+ self.factory.makeVulnerability(
186+ distribution=distribution,
187+ creator=distribution.owner
188+ )
189+ )
190+ vulnerability_url = api_url(vulnerability)
191+ person_url = api_url(person)
192+ webservice = webservice_for_person(
193+ person,
194+ permission=OAuthPermission.WRITE_PRIVATE,
195+ default_api_version="devel"
196+ )
197+
198+ response = webservice.patch(
199+ vulnerability_url,
200+ "application/json",
201+ json.dumps(
202+ {
203+ "creator_link": person_url,
204+ }
205+ )
206+ )
207+ self.assertEqual(400, response.status)
208+ self.assertEqual(
209+ b'creator_link: You tried to modify a read-only attribute.',
210+ response.body
211+ )
212+
213+ def test_distribution_of_a_vulnerability_read_only(self):
214+ distribution = self.factory.makeDistribution()
215+ vulnerability = removeSecurityProxy(
216+ self.factory.makeVulnerability(
217+ distribution=distribution,
218+ creator=distribution.owner
219+ )
220+ )
221+ another_distribution = removeSecurityProxy(
222+ self.factory.makeDistribution(owner=distribution.owner)
223+ )
224+ vulnerability_url = api_url(vulnerability)
225+ distribution_url = api_url(another_distribution)
226+ webservice = webservice_for_person(
227+ distribution.owner,
228+ permission=OAuthPermission.WRITE_PRIVATE,
229+ default_api_version="devel"
230+ )
231+
232+ response = webservice.patch(
233+ vulnerability_url,
234+ "application/json",
235+ json.dumps(
236+ {
237+ "distribution_link": distribution_url,
238+ }
239+ )
240+ )
241+ self.assertEqual(400, response.status)
242+ self.assertEqual(
243+ b'distribution_link: You tried to modify a read-only attribute.',
244+ response.body
245+ )
246+
247+ def test_editing_the_cve_associated_with_a_vulnerability(self):
248+ person = self.factory.makePerson()
249+ distribution = removeSecurityProxy(
250+ self.factory.makeDistribution(owner=person)
251+ )
252+ cve = self.factory.makeCVE("2022-1234")
253+ vulnerability = removeSecurityProxy(
254+ self.factory.makeVulnerability(
255+ distribution=distribution,
256+ creator=person,
257+ cve=cve
258+ )
259+ )
260+ vulnerability_url = api_url(vulnerability)
261+ api_base = "http://api.launchpad.test/devel"
262+ another_cve = self.factory.makeCVE('2022-1235')
263+ cve_url = api_base + api_url(another_cve)
264+
265+ self.assertThat(
266+ vulnerability,
267+ MatchesStructure.byEquality(
268+ distribution=distribution,
269+ cve=cve,
270+ creator=person,
271+ )
272+ )
273+ self.assertIn(distribution.name, vulnerability_url)
274+
275+ webservice = webservice_for_person(
276+ person,
277+ permission=OAuthPermission.WRITE_PRIVATE,
278+ default_api_version="devel"
279+ )
280+
281+ response = webservice.patch(
282+ vulnerability_url,
283+ "application/json",
284+ json.dumps(
285+ {
286+ "cve_link": cve_url,
287+ }
288+ )
289+ )
290+ self.assertEqual(209, response.status)
291+ self.assertEqual(vulnerability.cve, another_cve)
292+
293+ def test_user_without_edit_permissions_cannot_edit_vulnerability(self):
294+ vulnerability = removeSecurityProxy(
295+ self.factory.makeVulnerability()
296+ )
297+ random_user = self.factory.makePerson()
298+ now = datetime.datetime.now(pytz.UTC)
299+ vulnerability_url = api_url(vulnerability)
300+
301+ webservice = webservice_for_person(
302+ random_user,
303+ permission=OAuthPermission.WRITE_PRIVATE,
304+ default_api_version="devel"
305+ )
306+
307+ response = webservice.patch(
308+ vulnerability_url,
309+ "application/json",
310+ json.dumps(
311+ {
312+ "status": "Active",
313+ "information_type": "Private",
314+ "importance": "Critical",
315+ "description": "Foo bar",
316+ "notes": "lgp171188> Foo bar",
317+ "mitigation": "Foo bar baz",
318+ "importance_explanation": "Foo bar bazz",
319+ "date_made_public": now.isoformat(),
320+ }
321+ )
322+ )
323+ self.assertEqual(401, response.status)
324+ self.assertIn('launchpad.Edit', response.body.decode())

Subscribers

People subscribed via source and target branches

to status/vote changes: