launchpad-reviewers team mailing list archive

[Colin Watson <mp+430310@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-bpb-librarian-auth-condition into launchpad:master.

Commit message:
Fix the condition for dispatching tokens for private source files

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/430310

If a librarian request has a token, then the librarian will only serve restricted files.  In the case of source package release files, this isn't quite synonymous with files in private archives: source files in private archives may be unrestricted if the same source package is also published in a public archive.  Only dispatch tokens for source files that are actually restricted.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-bpb-librarian-auth-condition into launchpad:master.

diff --git a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
index 15f374d..3878a0f 100644
--- a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
+++ b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
@@ -76,7 +76,7 @@ class BinaryPackageBuildBehaviour(BuildFarmJobBehaviourBase):
                 "sha1": lfa.content.sha1,
                 "url": lfa.getURL(),
             }
-            if self.build.archive.private:
+            if lfa.restricted:
                 if macaroon_raw is None:
                     macaroon_raw = yield self.issueMacaroon()
                 filemap[lfa.filename].update(
diff --git a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
index 8f4d199..4d6ca14 100644
--- a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
+++ b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
@@ -141,7 +141,7 @@ class TestBinaryBuildPackageBehaviour(StatsMixin, TestCaseWithFactory):
 
         uploads = [(chroot.http_url, "", "")]
         for sprf in files:
-            if build.archive.private:
+            if sprf.libraryfile.restricted:
                 password = MacaroonVerifies(
                     "binary-package-build", build.archive
                 )
@@ -323,7 +323,48 @@ class TestBinaryBuildPackageBehaviour(StatsMixin, TestCaseWithFactory):
         )
 
     @defer.inlineCallbacks
-    def test_private_source_dispatch(self):
+    def test_private_source_file_dispatch(self):
+        self.useFixture(InProcessAuthServerFixture())
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret"
+        )
+        archive = self.factory.makeArchive(private=True)
+        worker = OkWorker()
+        builder = self.factory.makeBuilder()
+        builder.setCleanStatus(BuilderCleanStatus.CLEAN)
+        vitals = extract_vitals_from_db(builder)
+        build = self.factory.makeBinaryPackageBuild(
+            builder=builder, archive=archive
+        )
+        build.source_package_release.addFile(
+            self.factory.makeLibraryFileAlias(restricted=True, db_only=True),
+            filetype=SourcePackageFileType.ORIG_TARBALL,
+        )
+        lf = self.factory.makeLibraryFileAlias(db_only=True)
+        build.distro_arch_series.addOrUpdateChroot(lf)
+        bq = build.queueBuild()
+        bq.markAsBuilding(builder)
+        interactor = BuilderInteractor()
+        behaviour = interactor.getBuildBehaviour(bq, builder, worker)
+        yield interactor._startBuild(
+            bq, vitals, builder, worker, behaviour, BufferLogger()
+        )
+        yield self.assertExpectedInteraction(
+            worker.call_log,
+            builder,
+            build,
+            behaviour,
+            lf,
+            archive,
+            ArchivePurpose.PPA,
+        )
+
+    @defer.inlineCallbacks
+    def test_public_source_file_in_private_archive_dispatch(self):
+        # A source file in a private archive might be unrestricted if the
+        # same source package is also published in a public archive.  The
+        # librarian will only serve restricted files if given a token, so
+        # make sure that we don't send a token for unrestricted files.
         self.useFixture(InProcessAuthServerFixture())
         self.pushConfig(
             "launchpad", internal_macaroon_secret_key="some-secret"
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 3ebd99c..96bef5a 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -3255,7 +3255,11 @@ class LaunchpadObjectFactory(ObjectFactory):
                 md5=hashlib.md5(content).hexdigest(),
             )
             lfa = LibraryFileAlias(
-                content=lfc, filename=filename, mimetype=content_type
+                content=lfc,
+                filename=filename,
+                mimetype=content_type,
+                expires=expires,
+                restricted=restricted,
             )
         else:
             lfa = getUtility(ILibraryFileAliasSet).create(