launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/widen-performLookup-account-for-grants into lp:launchpad.

Commit message:
Widen _performLookup to account for users with grants

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/widen-performLookup-account-for-grants/+merge/357706

Allow write access for users that have a RuleGrant to the repository, as well as the repository owner.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/widen-performLookup-account-for-grants into lp:launchpad.

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2018-10-22 10:37:03 +0000
+++ lib/lp/code/xmlrpc/git.py	2018-10-23 16:17:49 +0000
@@ -103,7 +103,7 @@
         else:
             return issuer.checkMacaroonIssuer(macaroon)
 
-    def _performLookup(self, path, auth_params):
+    def _performLookup(self, path, auth_params, requester):
         repository, extra_path = getUtility(IGitLookup).getByPath(path)
         if repository is None:
             return None
@@ -127,6 +127,13 @@
             writable = (
                 repository.repository_type == GitRepositoryType.HOSTED and
                 check_permission("launchpad.Edit", repository))
+            # If we have any grants to this user, they are declared to have
+            # write access at this point. `_checkRefPermissions` will
+            # sort out access to individual refs at a later point in the push.
+            if not writable:
+                grants = naked_repository.findRuleGrantsByGrantee(requester)
+                if not grants.is_empty():
+                    writable = True
             private = repository.private
         return {
             "path": hosting_path,
@@ -282,11 +289,11 @@
         if requester == LAUNCHPAD_ANONYMOUS:
             requester = None
         try:
-            result = self._performLookup(path, auth_params)
+            result = self._performLookup(path, auth_params, requester)
             if (result is None and requester is not None and
                 permission == "write"):
-                self._createRepository(requester, path)
-                result = self._performLookup(path, auth_params)
+                self._createRepository(requester, path, requester)
+                result = self._performLookup(path, auth_params, requester)
             if result is None:
                 raise faults.GitRepositoryNotFound(path)
             if permission != "read" and not result["writable"]:

=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py	2018-10-18 15:07:49 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py	2018-10-23 16:17:49 +0000
@@ -266,6 +266,34 @@
         self.assertEqual(
             initial_count, getUtility(IAllGitRepositories).count())
 
+    def test_translatePath_grant_to_other(self):
+        requester = self.factory.makePerson()
+        other_person = self.factory.makePerson()
+        repository = self.factory.makeGitRepository(owner=requester)
+        rule = self.factory.makeGitRule(
+            repository, ref_pattern=u'refs/heads/stable/next')
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=other_person,
+            can_force_push=True)
+        path = u"/%s" % repository.unique_name
+        self.assertTranslates(
+            other_person, path, repository, True, private=False)
+
+    def test_translatePath_grant_to_other_private(self):
+        requester = self.factory.makePerson()
+        other_person = self.factory.makePerson()
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=requester, information_type=InformationType.USERDATA))
+        rule = self.factory.makeGitRule(
+            repository, ref_pattern=u'refs/heads/stable/next')
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=other_person,
+            can_force_push=True)
+        path = u"/%s" % repository.unique_name
+        self.assertGitRepositoryNotFound(
+            other_person, path, can_authenticate=True)
+
     def _make_scenario_one_repository(self):
         user_a = self.factory.makePerson()
         user_b = self.factory.makePerson()