launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-xmlrpc-auth-params into lp:launchpad.

Commit message:
Support passing GitAPI.translatePath authentication parameters as a dict.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1469459 in Launchpad itself: "import external code into a LP git repo (natively)"
  https://bugs.launchpad.net/launchpad/+bug/1469459

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-xmlrpc-auth-params/+merge/307694

Support passing GitAPI.translatePath authentication parameters as a dict.  Once this is deployed, turnip will be able to pass additional data from the authentication stage through to translatePath; this will be needed for code imports, where we want to be able to say that a client has successfully authenticated but is only authorised to push to a certain repository.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-xmlrpc-auth-params into lp:launchpad.

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2016-10-05 09:15:05 +0000
+++ lib/lp/code/xmlrpc/git.py	2016-10-05 11:19:51 +0000
@@ -252,8 +252,14 @@
             else:
                 raise
 
-    def translatePath(self, path, permission, requester_id, can_authenticate):
+    def translatePath(self, path, permission, *auth_args):
         """See `IGitAPI`."""
+        if len(auth_args) == 1:
+            auth_params = auth_args[0]
+            requester_id = auth_params.get("id")
+            can_authenticate = auth_params.get("can-authenticate", False)
+        else:
+            requester_id, can_authenticate = auth_args
         if requester_id is None:
             requester_id = LAUNCHPAD_ANONYMOUS
         if isinstance(path, str):

=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py	2016-10-05 09:15:05 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py	2016-10-05 11:19:51 +0000
@@ -5,6 +5,10 @@
 
 __metaclass__ = type
 
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -39,22 +43,36 @@
 from lp.xmlrpc import faults
 
 
-class TestGitAPIMixin:
+class TestGitAPIMixin(WithScenarios):
     """Helper methods for `IGitAPI` tests, and security-relevant tests."""
 
+    scenarios = [
+        ("auth_params_flat", {"auth_params_dict": False}),
+        ("auth_params_dict", {"auth_params_dict": True}),
+        ]
+
     def setUp(self):
         super(TestGitAPIMixin, self).setUp()
         self.git_api = GitAPI(None, None)
         self.hosting_fixture = self.useFixture(GitHostingFixture())
         self.repository_set = getUtility(IGitRepositorySet)
 
+    def _translatePath(self, path, permission, auth_params):
+        if self.auth_params_dict:
+            return self.git_api.translatePath(path, permission, auth_params)
+        else:
+            return self.git_api.translatePath(
+                path, permission, auth_params.get("id"),
+                auth_params.get("can-authenticate", False))
+
     def assertGitRepositoryNotFound(self, requester, path, permission="read",
                                     can_authenticate=False):
         """Assert that the given path cannot be translated."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertEqual(
             faults.GitRepositoryNotFound(path.strip("/")), fault)
 
@@ -64,8 +82,9 @@
         """Assert that looking at the given path returns PermissionDenied."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertEqual(faults.PermissionDenied(message), fault)
 
     def assertUnauthorized(self, requester, path,
@@ -74,8 +93,9 @@
         """Assert that looking at the given path returns Unauthorized."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertEqual(faults.Unauthorized(message), fault)
 
     def assertNotFound(self, requester, path, message, permission="read",
@@ -83,8 +103,9 @@
         """Assert that looking at the given path returns NotFound."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertEqual(faults.NotFound(message), fault)
 
     def assertInvalidSourcePackageName(self, requester, path, name,
@@ -94,8 +115,9 @@
         InvalidSourcePackageName."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertEqual(faults.InvalidSourcePackageName(name), fault)
 
     def assertInvalidBranchName(self, requester, path, message,
@@ -103,8 +125,9 @@
         """Assert that looking at the given path returns InvalidBranchName."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertEqual(faults.InvalidBranchName(Exception(message)), fault)
 
     def assertOopsOccurred(self, requester, path,
@@ -112,8 +135,9 @@
         """Assert that looking at the given path OOPSes."""
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        fault = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        fault = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         self.assertIsInstance(fault, faults.OopsOccurred)
         prefix = (
             "An unexpected error has occurred while creating a Git "
@@ -126,8 +150,9 @@
                          trailing="", private=False):
         if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
             requester = requester.id
-        translation = self.git_api.translatePath(
-            path, permission, requester, can_authenticate)
+        translation = self._translatePath(
+            path, permission,
+            {"id": requester, "can-authenticate": can_authenticate})
         login(ANONYMOUS)
         self.assertEqual(
             {"path": repository.getInternalPath(), "writable": writable,
@@ -140,8 +165,9 @@
             requester_id = requester
         else:
             requester_id = requester.id
-        translation = self.git_api.translatePath(
-            path, "write", requester_id, can_authenticate)
+        translation = self._translatePath(
+            path, "write",
+            {"id": requester_id, "can-authenticate": can_authenticate})
         login(ANONYMOUS)
         repository = getUtility(IGitRepositorySet).getByPath(
             requester, path.lstrip("/"))
@@ -657,3 +683,6 @@
     """
 
     layer = AppServerLayer
+
+
+load_tests = load_tests_apply_scenarios