launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1451107 into lp:launchpad.

Commit message:
Fix GitAPI.notify for private branches.

Requested reviews:
  William Grant (wgrant): code
Related bugs:
  Bug #1451107 in Launchpad itself: "Pushing to a private git repository hangs"
  https://bugs.launchpad.net/launchpad/+bug/1451107

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1451107/+merge/258328

The XMLRPC GitAPI.notify method runs as anonymous with web security, but needs to function for private branches too. This branch strips the security proxy from the branch before creating the scan job for it, which is fine as the method is idempotent, returns nothing, and takes no compromising input.
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2015-04-28 13:29:38 +0000
+++ lib/lp/code/xmlrpc/git.py	2015-05-06 00:48:17 +0000
@@ -16,6 +16,7 @@
 from zope.error.interfaces import IErrorReportingUtility
 from zope.interface import implements
 from zope.security.interfaces import Unauthorized
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NameLookupFailed
 from lp.app.validators import LaunchpadValidationError
@@ -265,7 +266,8 @@
         if repository is None:
             return faults.NotFound(
                 "No repository found for '%s'." % translated_path)
-        getUtility(IGitRefScanJobSource).create(repository)
+        getUtility(IGitRefScanJobSource).create(
+            removeSecurityProxy(repository))
 
     def authenticateWithPassword(self, username, password):
         """See `IGitAPI`."""

=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py	2015-04-28 13:29:38 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py	2015-05-06 00:48:17 +0000
@@ -25,6 +25,7 @@
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.escaping import html_escape
 from lp.testing import (
+    admin_logged_in,
     ANONYMOUS,
     login,
     person_logged_in,
@@ -687,6 +688,17 @@
         job_source = getUtility(IGitRefScanJobSource)
         self.assertEqual([], list(job_source.iterReady()))
 
+    def test_notify_private(self):
+        # notify works on private repos.
+        with admin_logged_in():
+            repository = self.factory.makeGitRepository(
+                information_type=InformationType.PRIVATESECURITY)
+            path = repository.getInternalPath()
+        self.assertIsNone(self.git_api.notify(path))
+        job_source = getUtility(IGitRefScanJobSource)
+        [job] = list(job_source.iterReady())
+        self.assertEqual(repository, job.repository)
+
     def test_authenticateWithPassword(self):
         self.assertIsInstance(
             self.git_api.authenticateWithPassword('foo', 'bar'),