launchpad-reviewers team mailing list archive

[Colin Watson <mp+431534@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:public-archive-auth into launchpad:master.

Commit message:
Allow anonymous authorization to public archives

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

The current ppa.launchpadcontent.net has no need for any authorization arrangements to access public archives: it already knows that archives are public because of where they live on its file system.  However, a diskless archive service doesn't know this, and must perform an authorization check for public and private archives alike.

To support this, allow calling `ArchiveAPI.checkArchiveAuthToken` with any authentication information, even None to indicate anonymous access.  The archive service can then call this over XML-RPC without needing to first discover whether the archive is public or private.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:public-archive-auth into launchpad:master.

diff --git a/lib/lp/soyuz/xmlrpc/archive.py b/lib/lp/soyuz/xmlrpc/archive.py
index a23b7fa..53ebf1a 100644
--- a/lib/lp/soyuz/xmlrpc/archive.py
+++ b/lib/lp/soyuz/xmlrpc/archive.py
@@ -65,7 +65,17 @@ class ArchiveAPI(LaunchpadXMLRPCView):
                 message="No archive found for '%s'." % archive_reference
             )
         archive = removeSecurityProxy(archive)
-        token_set = getUtility(IArchiveAuthTokenSet)
+
+        # Public archives do not require authorization.
+        if not archive.private:
+            log.info("%s: Authorized (public)", archive_reference)
+            return
+        elif username is None:
+            log.info(
+                "<anonymous>@%s: Private archive requires authorization",
+                archive_reference,
+            )
+            raise faults.Unauthorized()
 
         # If the password is a serialized macaroon for the buildd user, then
         # try macaroon authentication.
@@ -83,6 +93,7 @@ class ArchiveAPI(LaunchpadXMLRPCView):
                 raise faults.Unauthorized()
 
         # Fall back to checking archive auth tokens.
+        token_set = getUtility(IArchiveAuthTokenSet)
         if username.startswith("+"):
             token = token_set.getActiveNamedTokenForArchive(
                 archive, username[1:]
diff --git a/lib/lp/soyuz/xmlrpc/tests/test_archive.py b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
index cdb7d7d..33fd516 100644
--- a/lib/lp/soyuz/xmlrpc/tests/test_archive.py
+++ b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
@@ -57,6 +57,26 @@ class TestArchiveAPI(TestCaseWithFactory):
             "",
         )
 
+    def test_checkArchiveAuthToken_anonymous_private(self):
+        archive = self.factory.makeArchive(private=True)
+        self.assertUnauthorized(
+            "checkArchiveAuthToken",
+            "<anonymous>@%s: Private archive requires authorization"
+            % archive.reference,
+            archive.reference,
+            None,
+            None,
+        )
+
+    def test_checkArchiveAuthToken_anonymous_public(self):
+        archive = self.factory.makeArchive()
+        self.assertIsNone(
+            self.archive_api.checkArchiveAuthToken(
+                archive.reference, None, None
+            )
+        )
+        self.assertLogs("%s: Authorized (public)" % archive.reference)
+
     def test_checkArchiveAuthToken_no_tokens(self):
         archive = removeSecurityProxy(self.factory.makeArchive(private=True))
         self.assertNotFound(