Support authentication in run_http_exc

Authentication could be required when performing REST API authenticated
queries over http(s) (done by run_http_exc). Typically, it appends with
gerrit behind apache2.

This change queries git credential for gerrit password when http request
returns a 401.

Change-Id: Iad60eea938c42210ba8c5df4a1b76f8d2f4dd76d
This commit is contained in:
Cedric Brandily 2015-02-27 16:17:35 +01:00
parent 4158150c48
commit 910ffddd1b
4 changed files with 83 additions and 7 deletions

View File

@ -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
=====

View File

@ -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

View File

@ -120,7 +120,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:
@ -129,17 +129,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)
p = subprocess.Popen(argv, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT, env=newenv)
(out, nothing) = p.communicate()
newenv.update(kwargs)
p = subprocess.Popen(argv,
stdin=subprocess.PIPE if stdin else None,
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):
(rc, output) = run_command_status(*argv, **env)
def run_command(*argv, **kwargs):
(rc, output) = run_command_status(*argv, **kwargs)
return output
@ -154,6 +158,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
@ -173,6 +186,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:

View File

@ -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)