http: use credential API to get passwords

This patch converts the http code to use the new credential API, both for http authentication as well as for getting certificate passwords. Most of the code change is simply variable naming (the passwords are now contained inside the credential struct) or deletion of obsolete code (the credential code handles URL parsing and prompting for us). The behavior should be the same, with one exception: the credential code will prompt with a description based on the credential components. Therefore, the old prompt of: Username for 'example.com': Password for 'example.com': now looks like: Username for 'https://example.com/repo.git': Password for 'https://user@example.com/repo.git': Note that we include more information in each line, specifically: 1. We now include the protocol. While more noisy, this is an important part of knowing what you are accessing (especially if you care about http vs https). 2. We include the username in the password prompt. This is not a big deal when you have just been prompted for it, but the username may also come from the remote's URL (and after future patches, from configuration or credential helpers). In that case, it's a nice reminder of the user for which you're giving the password. 3. We include the path component of the URL. In many cases, the user won't care about this and it's simply noise (i.e., they'll use the same credential for a whole site). However, that is part of a larger question, which is whether path components should be part of credential context, both for prompting and for lookup by storage helpers. That issue will be addressed as a whole in a future patch. Similarly, for unlocking certificates, we used to say: Certificate Password for 'example.com': and we now say: Password for 'cert:///path/to/certificate': Showing the path to the client certificate makes more sense, as that is what you are unlocking, not "example.com". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 10, 2011 at 05:31 UTC 148bb6a7b4d82a6380c6a51951b870933564c115
2 files changed +52 -99
http.c
+25 -88
@@ -3,6 +3,7 @@
3 #include "sideband.h"
4 #include "run-command.h"
5 #include "url.h"
6 +#include "credential.h"
7
8 int data_received;
9 int active_requests;
@@ -42,7 +43,7 @@ static long curl_low_speed_time = -1;
43 static int curl_ftp_no_epsv;
44 static const char *curl_http_proxy;
45 static const char *curl_cookie_file;
45 -static char *user_name, *user_pass, *description;
46 +static struct credential http_auth = CREDENTIAL_INIT;
47 static const char *user_agent;
48
49 #if LIBCURL_VERSION_NUM >= 0x071700
@@ -53,7 +54,7 @@ static const char *user_agent;
54 #define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD
55 #endif
56
56 -static char *ssl_cert_password;
57 +static struct credential cert_auth = CREDENTIAL_INIT;
58 static int ssl_cert_password_required;
59
60 static struct curl_slist *pragma_header;
@@ -139,27 +140,6 @@ static void process_curl_messages(void)
140 }
141 #endif
142
142 -static char *git_getpass_with_description(const char *what, const char *desc)
143 -{
144 - struct strbuf prompt = STRBUF_INIT;
145 - char *r;
146 -
147 - if (desc)
148 - strbuf_addf(&prompt, "%s for '%s': ", what, desc);
149 - else
150 - strbuf_addf(&prompt, "%s: ", what);
151 - /*
152 - * NEEDSWORK: for usernames, we should do something less magical that
153 - * actually echoes the characters. However, we need to read from
154 - * /dev/tty and not stdio, which is not portable (but getpass will do
155 - * it for us). http.c uses the same workaround.
156 - */
157 - r = git_getpass(prompt.buf);
158 -
159 - strbuf_release(&prompt);
160 - return xstrdup(r);
161 -}
162 -
143 static int http_options(const char *var, const char *value, void *cb)
144 {
145 if (!strcmp("http.sslverify", var)) {
@@ -232,11 +212,11 @@ static int http_options(const char *var, const char *value, void *cb)
212
213 static void init_curl_http_auth(CURL *result)
214 {
235 - if (user_name) {
215 + if (http_auth.username) {
216 struct strbuf up = STRBUF_INIT;
237 - if (!user_pass)
238 - user_pass = xstrdup(git_getpass_with_description("Password", description));
239 - strbuf_addf(&up, "%s:%s", user_name, user_pass);
217 + credential_fill(&http_auth);
218 + strbuf_addf(&up, "%s:%s",
219 + http_auth.username, http_auth.password);
220 curl_easy_setopt(result, CURLOPT_USERPWD,
221 strbuf_detach(&up, NULL));
222 }
@@ -244,18 +224,14 @@ static void init_curl_http_auth(CURL *result)
224
225 static int has_cert_password(void)
226 {
247 - if (ssl_cert_password != NULL)
248 - return 1;
227 if (ssl_cert == NULL || ssl_cert_password_required != 1)
228 return 0;
251 - /* Only prompt the user once. */
252 - ssl_cert_password_required = -1;
253 - ssl_cert_password = git_getpass_with_description("Certificate Password", description);
254 - if (ssl_cert_password != NULL) {
255 - ssl_cert_password = xstrdup(ssl_cert_password);
256 - return 1;
257 - } else
258 - return 0;
229 + if (!cert_auth.password) {
230 + cert_auth.protocol = xstrdup("cert");
231 + cert_auth.path = xstrdup(ssl_cert);
232 + credential_fill(&cert_auth);
233 + }
234 + return 1;
235 }
236
237 static CURL *get_curl_handle(void)
@@ -282,7 +258,7 @@ static CURL *get_curl_handle(void)
258 if (ssl_cert != NULL)
259 curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
260 if (has_cert_password())
285 - curl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);
261 + curl_easy_setopt(result, CURLOPT_KEYPASSWD, cert_auth.password);
262 #if LIBCURL_VERSION_NUM >= 0x070903
263 if (ssl_key != NULL)
264 curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);
@@ -324,42 +300,6 @@ static CURL *get_curl_handle(void)
300 return result;
301 }
302
327 -static void http_auth_init(const char *url)
328 -{
329 - const char *at, *colon, *cp, *slash, *host;
330 -
331 - cp = strstr(url, "://");
332 - if (!cp)
333 - return;
334 -
335 - /*
336 - * Ok, the URL looks like "proto://something". Which one?
337 - * "proto://<user>:<pass>@<host>/...",
338 - * "proto://<user>@<host>/...", or just
339 - * "proto://<host>/..."?
340 - */
341 - cp += 3;
342 - at = strchr(cp, '@');
343 - colon = strchr(cp, ':');
344 - slash = strchrnul(cp, '/');
345 - if (!at || slash <= at) {
346 - /* No credentials, but we may have to ask for some later */
347 - host = cp;
348 - }
349 - else if (!colon || at <= colon) {
350 - /* Only username */
351 - user_name = url_decode_mem(cp, at - cp);
352 - user_pass = NULL;
353 - host = at + 1;
354 - } else {
355 - user_name = url_decode_mem(cp, colon - cp);
356 - user_pass = url_decode_mem(colon + 1, at - (colon + 1));
357 - host = at + 1;
358 - }
359 -
360 - description = url_decode_mem(host, slash - host);
361 -}
362 -
303 static void set_from_env(const char **var, const char *envname)
304 {
305 const char *val = getenv(envname);
@@ -432,7 +372,7 @@ void http_init(struct remote *remote, const char *url)
372 curl_ftp_no_epsv = 1;
373
374 if (url) {
435 - http_auth_init(url);
375 + credential_from_url(&http_auth, url);
376 if (!ssl_cert_password_required &&
377 getenv("GIT_SSL_CERT_PASSWORD_PROTECTED") &&
378 !prefixcmp(url, "https://"))
@@ -481,10 +421,10 @@ void http_cleanup(void)
421 curl_http_proxy = NULL;
422 }
423
484 - if (ssl_cert_password != NULL) {
485 - memset(ssl_cert_password, 0, strlen(ssl_cert_password));
486 - free(ssl_cert_password);
487 - ssl_cert_password = NULL;
424 + if (cert_auth.password != NULL) {
425 + memset(cert_auth.password, 0, strlen(cert_auth.password));
426 + free(cert_auth.password);
427 + cert_auth.password = NULL;
428 }
429 ssl_cert_password_required = 0;
430 }
@@ -836,17 +776,11 @@ static int http_request(const char *url, void *result, int target, int options)
776 else if (missing_target(&results))
777 ret = HTTP_MISSING_TARGET;
778 else if (results.http_code == 401) {
839 - if (user_name && user_pass) {
779 + if (http_auth.username && http_auth.password) {
780 + credential_reject(&http_auth);
781 ret = HTTP_NOAUTH;
782 } else {
842 - /*
843 - * git_getpass is needed here because its very likely stdin/stdout are
844 - * pipes to our parent process. So we instead need to use /dev/tty,
845 - * but that is non-portable. Using git_getpass() can at least be stubbed
846 - * on other platforms with a different implementation if/when necessary.
847 - */
848 - if (!user_name)
849 - user_name = xstrdup(git_getpass_with_description("Username", description));
783 + credential_fill(&http_auth);
784 init_curl_http_auth(slot->curl);
785 ret = HTTP_REAUTH;
786 }
@@ -866,6 +800,9 @@ static int http_request(const char *url, void *result, int target, int options)
800 curl_slist_free_all(headers);
801 strbuf_release(&buf);
802
803 + if (ret == HTTP_OK)
804 + credential_approve(&http_auth);
805 +
806 return ret;
807 }
808
t/t5550-http-fetch.sh
+27 -11
@@ -49,40 +49,56 @@ test_expect_success 'setup askpass helpers' '
49 EOF
50 chmod +x askpass &&
51 GIT_ASKPASS="$PWD/askpass" &&
52 - export GIT_ASKPASS &&
53 - >askpass-expect-none &&
54 - echo "askpass: Password for '\''$HTTPD_DEST'\'': " >askpass-expect-pass &&
55 - { echo "askpass: Username for '\''$HTTPD_DEST'\'': " &&
56 - cat askpass-expect-pass
57 - } >askpass-expect-both
58 -'
52 + export GIT_ASKPASS
53 +'
54 +
55 +expect_askpass() {
56 + dest=$HTTPD_DEST/auth/repo.git
57 + {
58 + case "$1" in
59 + none)
60 + ;;
61 + pass)
62 + echo "askpass: Password for 'http://$2@$dest': "
63 + ;;
64 + both)
65 + echo "askpass: Username for 'http://$dest': "
66 + echo "askpass: Password for 'http://$2@$dest': "
67 + ;;
68 + *)
69 + false
70 + ;;
71 + esac
72 + } >askpass-expect &&
73 + test_cmp askpass-expect askpass-query
74 +}
75
76 test_expect_success 'cloning password-protected repository can fail' '
77 >askpass-query &&
78 echo wrong >askpass-response &&
79 test_must_fail git clone "$HTTPD_URL/auth/repo.git" clone-auth-fail &&
64 - test_cmp askpass-expect-both askpass-query
80 + expect_askpass both wrong
81 '
82
83 test_expect_success 'http auth can use user/pass in URL' '
84 >askpass-query &&
85 echo wrong >askpass-response &&
86 git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
71 - test_cmp askpass-expect-none askpass-query
87 + expect_askpass none
88 '
89
90 test_expect_success 'http auth can use just user in URL' '
91 >askpass-query &&
92 echo user@host >askpass-response &&
93 git clone "$HTTPD_URL_USER/auth/repo.git" clone-auth-pass &&
78 - test_cmp askpass-expect-pass askpass-query
94 + expect_askpass pass user@host
95 '
96
97 test_expect_success 'http auth can request both user and pass' '
98 >askpass-query &&
99 echo user@host >askpass-response &&
100 git clone "$HTTPD_URL/auth/repo.git" clone-auth-both &&
85 - test_cmp askpass-expect-both askpass-query
101 + expect_askpass both user@host
102 '
103
104 test_expect_success 'fetch changes via http' '