launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+491804@xxxxxxxxxxxxxxxxxx>]

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-import-export-endpoints into launchpad:master.

Commit message:
Fix SOSSImport SecurityProxy

As celery jobs run without a logged in user, we need to remove the
security proxy to be able to view bug/vulnerability fields.
SOSS distribution currently only allows InformationType.PROPRIETARY bugs
and vulnerabilities.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/491804
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-import-export-endpoints into launchpad:master.

diff --git a/lib/lp/bugs/scripts/soss/sossexport.py b/lib/lp/bugs/scripts/soss/sossexport.py
index aae719f..348d4d9 100644
--- a/lib/lp/bugs/scripts/soss/sossexport.py
+++ b/lib/lp/bugs/scripts/soss/sossexport.py
@@ -49,7 +49,7 @@ class SOSSExporter:
 
     def __init__(
         self,
-        information_type: InformationType = InformationType.PRIVATESECURITY,
+        information_type: InformationType = InformationType.PROPRIETARY,
         dry_run: bool = False,
     ) -> None:
         self.dry_run = dry_run
diff --git a/lib/lp/bugs/scripts/soss/sossimport.py b/lib/lp/bugs/scripts/soss/sossimport.py
index bdc004c..f330a0f 100644
--- a/lib/lp/bugs/scripts/soss/sossimport.py
+++ b/lib/lp/bugs/scripts/soss/sossimport.py
@@ -171,23 +171,25 @@ class SOSSImporter:
         metadata = {"repositories": package.repositories}
 
         # Create the bug, only first bugtask
-        bug, _ = self.bug_set.createBug(
-            CreateBugParams(
-                comment=self._make_bug_description(soss_record),
-                title=lp_cve.sequence,
-                information_type=self.information_type,
-                owner=self.bug_importer,
-                target=externalpackage,
-                status=PACKAGE_STATUS_MAP[package.status],
-                status_explanation=package.note,
-                assignee=assignee,
-                validate_assignee=False,
-                importance=PRIORITY_ENUM_MAP[soss_record.priority],
-                cve=lp_cve,
-                metadata=metadata,
-                check_permissions=False,
-            ),
-            notify_event=False,
+        bug, _ = removeSecurityProxy(
+            self.bug_set.createBug(
+                CreateBugParams(
+                    comment=self._make_bug_description(soss_record),
+                    title=lp_cve.sequence,
+                    information_type=self.information_type,
+                    owner=self.bug_importer,
+                    target=externalpackage,
+                    status=PACKAGE_STATUS_MAP[package.status],
+                    status_explanation=package.note,
+                    assignee=assignee,
+                    validate_assignee=False,
+                    importance=PRIORITY_ENUM_MAP[soss_record.priority],
+                    cve=lp_cve,
+                    metadata=metadata,
+                    check_permissions=False,
+                ),
+                notify_event=False,
+            )
         )
 
         # Create next bugtasks
@@ -234,23 +236,25 @@ class SOSSImporter:
         :param distribution: a Distribution affected by the vulnerability
         :return: a Vulnerability
         """
-        vulnerability: Vulnerability = self.vulnerability_set.new(
-            distribution=distribution,
-            status=VulnerabilityStatus.NEEDS_TRIAGE,
-            importance=PRIORITY_ENUM_MAP[soss_record.priority],
-            creator=bug.owner,
-            information_type=self.information_type,
-            cve=lp_cve,
-            description=soss_record.description,
-            notes="\n".join(soss_record.notes),
-            mitigation=None,
-            importance_explanation=soss_record.priority_reason,
-            date_made_public=self._normalize_date_with_timezone(
-                soss_record.public_date
-            ),
-            date_notice_issued=None,
-            date_coordinated_release=None,
-            cvss=self._prepare_cvss_data(soss_record),
+        vulnerability: Vulnerability = removeSecurityProxy(
+            self.vulnerability_set.new(
+                distribution=distribution,
+                status=VulnerabilityStatus.NEEDS_TRIAGE,
+                importance=PRIORITY_ENUM_MAP[soss_record.priority],
+                creator=bug.owner,
+                information_type=self.information_type,
+                cve=lp_cve,
+                description=soss_record.description,
+                notes="\n".join(soss_record.notes),
+                mitigation=None,
+                importance_explanation=soss_record.priority_reason,
+                date_made_public=self._normalize_date_with_timezone(
+                    soss_record.public_date
+                ),
+                date_notice_issued=None,
+                date_coordinated_release=None,
+                cvss=self._prepare_cvss_data(soss_record),
+            )
         )
         vulnerability.linkBug(bug, bug.owner)
 
@@ -308,7 +312,7 @@ class SOSSImporter:
                 soss_record.sequence,
             )
         if bugs:
-            return bugs[0]
+            return removeSecurityProxy(bugs[0])
 
         return None
 
@@ -319,7 +323,8 @@ class SOSSImporter:
         if not lp_cve:
             return None
 
-        return lp_cve.getDistributionVulnerability(distribution)
+        vulnerability = lp_cve.getDistributionVulnerability(distribution)
+        return removeSecurityProxy(vulnerability)
 
     def _create_or_update_bugtasks(
         self, bug: BugModel, soss_record: SOSSRecord
@@ -340,7 +345,9 @@ class SOSSImporter:
         assignee = self._get_assignee(soss_record.assigned_to)
 
         # Build a lookup dict for existing bug tasks
-        bugtask_by_target = {task.target: task for task in bug.bugtasks}
+        bugtask_by_target = {
+            task.target: removeSecurityProxy(task) for task in bug.bugtasks
+        }
 
         for packagetype, package_list in packages:
             for package in package_list:
@@ -354,14 +361,16 @@ class SOSSImporter:
                 )
 
                 if target not in bugtask_by_target:
-                    bugtask = self.bugtask_set.createTask(
-                        bug,
-                        self.bug_importer,
-                        target,
-                        status=PACKAGE_STATUS_MAP[package.status],
-                        importance=PRIORITY_ENUM_MAP[soss_record.priority],
-                        assignee=assignee,
-                        metadata=metadata,
+                    bugtask = removeSecurityProxy(
+                        self.bugtask_set.createTask(
+                            bug,
+                            self.bug_importer,
+                            target,
+                            status=PACKAGE_STATUS_MAP[package.status],
+                            importance=PRIORITY_ENUM_MAP[soss_record.priority],
+                            assignee=assignee,
+                            metadata=metadata,
+                        )
                     )
                 else:
                     bugtask = bugtask_by_target[target]
diff --git a/lib/lp/bugs/scripts/soss/tests/test_sossexport.py b/lib/lp/bugs/scripts/soss/tests/test_sossexport.py
index c7f315f..02e813b 100644
--- a/lib/lp/bugs/scripts/soss/tests/test_sossexport.py
+++ b/lib/lp/bugs/scripts/soss/tests/test_sossexport.py
@@ -25,6 +25,7 @@ class TestSOSSExporter(TestCaseWithFactory):
         self.soss = self.factory.makeDistribution(
             name="soss",
             displayname="SOSS",
+            information_type=InformationType.PROPRIETARY,
         )
         transaction.commit()
 
@@ -69,7 +70,7 @@ class TestSOSSExporter(TestCaseWithFactory):
     def test_to_record(self):
         """Test that imported and exported SOSSRecords match."""
         soss_importer = SOSSImporter(
-            information_type=InformationType.PRIVATESECURITY
+            information_type=InformationType.PROPRIETARY
         )
 
         for file in self.sampledata.iterdir():
@@ -91,7 +92,7 @@ class TestSOSSExporter(TestCaseWithFactory):
         """Integration test that checks that cve files imported and exported
         match."""
         soss_importer = SOSSImporter(
-            information_type=InformationType.PRIVATESECURITY
+            information_type=InformationType.PROPRIETARY
         )
 
         for file in self.sampledata.iterdir():