Merge "Support authentication in run_http_exc"

doc/source/installation.rst

@@ -60,6 +60,11 @@ defaultremote (default: gerrit).
 * You can specify different values to be used as defaults in
   ~/.config/git-review/git-review.conf or /etc/git-review/git-review.conf.
 
+* Git-review will query git credential system for gerrit user/password when
+  authentication failed over http(s). Unlike git, git-review does not persist
+  gerrit user/password in git credential system for security purposes and git
+  credential system configuration stays under user responsibility.
+
 Hooks
 =====
 

git-review.1

@@ -235,6 +235,13 @@ If you want all output to use color
 If you wish not to use color for any output. (default with Git older than 1.8.4)
 .El
 .El
+.Pp
+.Nm
+will query git credential system for gerrit user/password when
+authentication failed over http(s). Unlike git,
+.Nm
+does not persist gerrit user/password in git credential system for security
+purposes and git credential system configuration stays under user responsibility.
 .Sh FILES
 To use
 .Nm

git_review/cmd.py

@@ -121,7 +121,7 @@ def build_review_number(review, patchset):
     return review
 
 
-def run_command_status(*argv, **env):
+def run_command_status(*argv, **kwargs):
     if VERBOSE:
         print(datetime.datetime.now(), "Running:", " ".join(argv))
     if len(argv) == 1:
@@ -130,17 +130,21 @@ def run_command_status(*argv, **env):
             argv = shlex.split(argv[0].encode('utf-8'))
         else:
             argv = shlex.split(str(argv[0]))
+    stdin = kwargs.pop('stdin', None)
     newenv = os.environ.copy()
-    newenv.update(env)
+    newenv.update(kwargs)
-    p = subprocess.Popen(argv, stdout=subprocess.PIPE,
+    p = subprocess.Popen(argv,
-                         stderr=subprocess.STDOUT, env=newenv)
+                         stdin=subprocess.PIPE if stdin else None,
-    (out, nothing) = p.communicate()
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.STDOUT,
+                         env=newenv)
+    (out, nothing) = p.communicate(stdin)
     out = out.decode('utf-8', 'replace')
     return (p.returncode, out.strip())
 
 
-def run_command(*argv, **env):
+def run_command(*argv, **kwargs):
-    (rc, output) = run_command_status(*argv, **env)
+    (rc, output) = run_command_status(*argv, **kwargs)
     return output
 
 
@@ -155,6 +159,15 @@ def run_command_exc(klazz, *argv, **env):
     return output
 
 
+def git_credentials(klazz, url):
+    """Get credentials using git credential."""
+    cmd = 'git', 'credential', 'fill'
+    stdin = 'url=%s' % url
+    out = run_command_exc(klazz, *cmd, stdin=stdin)
+    data = dict(l.split('=', 1) for l in out.splitlines())
+    return data['username'], data['password']
+
+
 def http_code_2_return_code(code):
     """Tranform http status code to system return code."""
     return (code - 301) % 255 + 1
@@ -174,6 +187,11 @@ def run_http_exc(klazz, url, **env):
 
     try:
         res = requests.get(url, **env)
+        if res.status_code == 401:
+            env['auth'] = git_credentials(klazz, url)
+            res = requests.get(url, **env)
+    except klazz:
+        raise
     except Exception as err:
         raise klazz(255, str(err), ('GET', url), env)
     if not 200 <= res.status_code < 300:

git_review/tests/test_unit.py

@@ -168,6 +168,14 @@ class FakeException(Exception):
         self.code = code
 
 
+FAKE_GIT_CREDENTIAL_FILL = """\
+protocol=http
+host=gerrit.example.com
+username=user
+password=pass
+"""
+
+
 class GitReviewUnitTest(testtools.TestCase):
     """Class for misc unit tests."""
 
@@ -190,3 +198,41 @@ class GitReviewUnitTest(testtools.TestCase):
         except FakeException as err:
             self.assertEqual(255, err.code)
             mock_get.assert_called_once_with(url)
+
+    @mock.patch('git_review.cmd.run_command_exc')
+    @mock.patch('requests.get', return_value=FakeResponse(200))
+    def test_run_http_exc_without_auth(self, mock_get, mock_run):
+        url = 'http://user@gerrit.example.com'
+
+        cmd.run_http_exc(FakeException, url)
+        self.assertFalse(mock_run.called)
+        mock_get.assert_called_once_with(url)
+
+    @mock.patch('git_review.cmd.run_command_exc',
+                return_value=FAKE_GIT_CREDENTIAL_FILL)
+    @mock.patch('requests.get',
+                side_effect=[FakeResponse(401), FakeResponse(200)])
+    def test_run_http_exc_with_auth(self, mock_get, mock_run):
+        url = 'http://user@gerrit.example.com'
+
+        cmd.run_http_exc(FakeException, url)
+        mock_run.assert_called_once_with(mock.ANY, 'git', 'credential', 'fill',
+                                         stdin='url=%s' % url)
+        calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
+        mock_get.assert_has_calls(calls)
+
+    @mock.patch('git_review.cmd.run_command_exc',
+                return_value=FAKE_GIT_CREDENTIAL_FILL)
+    @mock.patch('requests.get', return_value=FakeResponse(401))
+    def test_run_http_exc_with_failing_auth(self, mock_get, mock_run):
+        url = 'http://user@gerrit.example.com'
+
+        try:
+            cmd.run_http_exc(FakeException, url)
+            self.fails('Exception expected')
+        except FakeException as err:
+            self.assertEqual(cmd.http_code_2_return_code(401), err.code)
+        mock_run.assert_called_once_with(mock.ANY, 'git', 'credential', 'fill',
+                                         stdin='url=%s' % url)
+        calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
+        mock_get.assert_has_calls(calls)