Browse Source

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
tags/1.0.0
Cedric Brandily 4 years ago
parent
commit
910ffddd1b
4 changed files with 83 additions and 7 deletions
  1. 5
    0
      doc/source/installation.rst
  2. 7
    0
      git-review.1
  3. 25
    7
      git_review/cmd.py
  4. 46
    0
      git_review/tests/test_unit.py

+ 5
- 0
doc/source/installation.rst View File

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

+ 7
- 0
git-review.1 View File

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

+ 25
- 7
git_review/cmd.py View File

@@ -120,7 +120,7 @@ def build_review_number(review, patchset):
120 120
     return review
121 121
 
122 122
 
123
-def run_command_status(*argv, **env):
123
+def run_command_status(*argv, **kwargs):
124 124
     if VERBOSE:
125 125
         print(datetime.datetime.now(), "Running:", " ".join(argv))
126 126
     if len(argv) == 1:
@@ -129,17 +129,21 @@ def run_command_status(*argv, **env):
129 129
             argv = shlex.split(argv[0].encode('utf-8'))
130 130
         else:
131 131
             argv = shlex.split(str(argv[0]))
132
+    stdin = kwargs.pop('stdin', None)
132 133
     newenv = os.environ.copy()
133
-    newenv.update(env)
134
-    p = subprocess.Popen(argv, stdout=subprocess.PIPE,
135
-                         stderr=subprocess.STDOUT, env=newenv)
136
-    (out, nothing) = p.communicate()
134
+    newenv.update(kwargs)
135
+    p = subprocess.Popen(argv,
136
+                         stdin=subprocess.PIPE if stdin else None,
137
+                         stdout=subprocess.PIPE,
138
+                         stderr=subprocess.STDOUT,
139
+                         env=newenv)
140
+    (out, nothing) = p.communicate(stdin)
137 141
     out = out.decode('utf-8', 'replace')
138 142
     return (p.returncode, out.strip())
139 143
 
140 144
 
141
-def run_command(*argv, **env):
142
-    (rc, output) = run_command_status(*argv, **env)
145
+def run_command(*argv, **kwargs):
146
+    (rc, output) = run_command_status(*argv, **kwargs)
143 147
     return output
144 148
 
145 149
 
@@ -154,6 +158,15 @@ def run_command_exc(klazz, *argv, **env):
154 158
     return output
155 159
 
156 160
 
161
+def git_credentials(klazz, url):
162
+    """Get credentials using git credential."""
163
+    cmd = 'git', 'credential', 'fill'
164
+    stdin = 'url=%s' % url
165
+    out = run_command_exc(klazz, *cmd, stdin=stdin)
166
+    data = dict(l.split('=', 1) for l in out.splitlines())
167
+    return data['username'], data['password']
168
+
169
+
157 170
 def http_code_2_return_code(code):
158 171
     """Tranform http status code to system return code."""
159 172
     return (code - 301) % 255 + 1
@@ -173,6 +186,11 @@ def run_http_exc(klazz, url, **env):
173 186
 
174 187
     try:
175 188
         res = requests.get(url, **env)
189
+        if res.status_code == 401:
190
+            env['auth'] = git_credentials(klazz, url)
191
+            res = requests.get(url, **env)
192
+    except klazz:
193
+        raise
176 194
     except Exception as err:
177 195
         raise klazz(255, str(err), ('GET', url), env)
178 196
     if not 200 <= res.status_code < 300:

+ 46
- 0
git_review/tests/test_unit.py View File

@@ -168,6 +168,14 @@ class FakeException(Exception):
168 168
         self.code = code
169 169
 
170 170
 
171
+FAKE_GIT_CREDENTIAL_FILL = """\
172
+protocol=http
173
+host=gerrit.example.com
174
+username=user
175
+password=pass
176
+"""
177
+
178
+
171 179
 class GitReviewUnitTest(testtools.TestCase):
172 180
     """Class for misc unit tests."""
173 181
 
@@ -190,3 +198,41 @@ class GitReviewUnitTest(testtools.TestCase):
190 198
         except FakeException as err:
191 199
             self.assertEqual(255, err.code)
192 200
             mock_get.assert_called_once_with(url)
201
+
202
+    @mock.patch('git_review.cmd.run_command_exc')
203
+    @mock.patch('requests.get', return_value=FakeResponse(200))
204
+    def test_run_http_exc_without_auth(self, mock_get, mock_run):
205
+        url = 'http://user@gerrit.example.com'
206
+
207
+        cmd.run_http_exc(FakeException, url)
208
+        self.assertFalse(mock_run.called)
209
+        mock_get.assert_called_once_with(url)
210
+
211
+    @mock.patch('git_review.cmd.run_command_exc',
212
+                return_value=FAKE_GIT_CREDENTIAL_FILL)
213
+    @mock.patch('requests.get',
214
+                side_effect=[FakeResponse(401), FakeResponse(200)])
215
+    def test_run_http_exc_with_auth(self, mock_get, mock_run):
216
+        url = 'http://user@gerrit.example.com'
217
+
218
+        cmd.run_http_exc(FakeException, url)
219
+        mock_run.assert_called_once_with(mock.ANY, 'git', 'credential', 'fill',
220
+                                         stdin='url=%s' % url)
221
+        calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
222
+        mock_get.assert_has_calls(calls)
223
+
224
+    @mock.patch('git_review.cmd.run_command_exc',
225
+                return_value=FAKE_GIT_CREDENTIAL_FILL)
226
+    @mock.patch('requests.get', return_value=FakeResponse(401))
227
+    def test_run_http_exc_with_failing_auth(self, mock_get, mock_run):
228
+        url = 'http://user@gerrit.example.com'
229
+
230
+        try:
231
+            cmd.run_http_exc(FakeException, url)
232
+            self.fails('Exception expected')
233
+        except FakeException as err:
234
+            self.assertEqual(cmd.http_code_2_return_code(401), err.code)
235
+        mock_run.assert_called_once_with(mock.ANY, 'git', 'credential', 'fill',
236
+                                         stdin='url=%s' % url)
237
+        calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
238
+        mock_get.assert_has_calls(calls)

Loading…
Cancel
Save