[RFC/PATCH 0/14] less annoying http authentication

[RFC/PATCH 0/14] less annoying http authentication

[Jeff King]

Using smart-http with a server that does http authentication can involve
typing your username and password a lot. This is annoying, and is a
frequent complaint of people using smart-http at GitHub.

Storing passwords is tricky, of course, because of the security
concerns. Fortunately this is a problem that has been solved by a
multitude of password wallet and keychain systems. We just have to hook
into whatever secure storage the user is already using.

So the goals of the series are:

  1. Provide an interface to request usernames and passwords from
     external helpers. By making them an external program that we spawn,
     it's easy to write custom helpers for whatever wallet program you
     want. Somebody has already expressed interest in making a helper
     for freedesktop.org's "Secrets API" to do one. Somebody at GitHub
     will probably do an OS X Keychain one.

  2. For users on systems with no secure storage, provide a
     time-limited, in-memory cache that is persistent between program
     invocations. So if you do:

       $ git fetch https://user@example.com/repo.git
       $ hack hack hack
       $ git push https://user@example.com/repo.git

     or:

       $ git push
       $ hack hack hack
       $ git push

     we can use the same password both times.

  3. For users on systems with no secure storage who want to take the
     risk of plaintext storage, provide a persistent disk cache (i.e.,
     prompt once and then store it in a config file). This is what SVN
     does. I think it's a horrible security policy to have by default,
     but it is simple, and it's good enough for many people.

  4. Allow associating usernames with remote hosts. Even if you do want
     to type your password each time, typing your username every time is
     annoying. Two things you can already do are:

       a. Use user@host in your remote URL. This has a few downsides,
          though.

          One is that git assumes that a URL with a user-portion is
          going to want authentication. So now it will ask for your
          password to fetch, even if the server doesn't actually care
          (though we might want to change that, since handles HTTP 401s
          properly these days).

          Two is that it's much simpler to separate the authentication
          from the repo name. Then you never have to worry about adding
          in your username to a URL when cloning, or stripping out
          somebody else's username from the URL they've shared with you.

       b. Use netrc. We don't document anywhere that netrc exists, or
          that we respect it. It's just a feature which comes along with
          curl. And even if we did mention it, some systems don't
          actually have a manpage for netrc. So we'd need to document
          that we use it, and its format.

          It also isn't very git-ish. You can't have per-repo versus
          per-user values. You can't use "git config" to set and look at
          it. It isn't carted around with your .gitconfig when you copy
          it to a new system.

This is still in the RFC stage. I think the code is fine, and I've been
using it for a few weeks. I tried to keep the design of the credential
helper interface simple, but flexible enough to meet the needs of the
various keychain systems. Aside from the usual, what I'd most like
feedback on is whether the interface is sufficient to actually integrate
with those systems. And I suspect we won't really know until people try
to make helpers for their pet systems.

The patches are:

  [01/14]: parse-options: add OPT_STRING_LIST helper

This one is actually a cherry-pick from jk/clone-cmdline-config, which
was marked as "Will merge to master" in the last What's Cooking. So it
probably makes sense to drop this and just build the rest of the series
on top of c8ba1639.

  [02/14]: url: decode buffers that are not NUL-terminated
  [03/14]: improve httpd auth tests
  [04/14]: remote-curl: don't retry auth failures with dumb protocol
  [05/14]: http: retry authentication failures for all http requests
  [06/14]: introduce credentials API
  [07/14]: http: use credential API to get passwords
  [08/14]: look for credentials in config before prompting
  [09/14]: allow the user to configure credential helpers
  [10/14]: http: use hostname in credential description
  [11/14]: docs: end-user documentation for the credential subsystem
  [12/14]: credentials: add "cache" helper
  [13/14]: credentials: add "store" helper
  [14/14]: credentials: add "getpass" helper

-Peff

[PATCH 01/14] parse-options: add OPT_STRING_LIST helper

[Jeff King]

This just adds repeated invocations of an option to a list
of strings. Using the "--no-<var>" form will reset the list
to empty.

Signed-off-by: Jeff King <peff@peff.net>
---
As I mentioned elsewhere, this is a cherry-pick of c8ba163 and can be
dropped.

 parse-options.c          |   17 +++++++++++++++++
 parse-options.h          |    4 ++++
 t/t0040-parse-options.sh |   17 +++++++++++++++++
 test-parse-options.c     |    6 ++++++
 4 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 73bd28a..879ea82 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
 #include "cache.h"
 #include "commit.h"
 #include "color.h"
+#include "string-list.h"
 
 static int parse_options_usage(struct parse_opt_ctx_t *ctx,
 			       const char * const *usagestr,
@@ -687,3 +688,19 @@ int parse_options_concat(struct option *dst, size_t dst_size, struct option *src
 	}
 	return -1;
 }
+
+int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
+{
+	struct string_list *v = opt->value;
+
+	if (unset) {
+		string_list_clear(v, 0);
+		return 0;
+	}
+
+	if (!arg)
+		return -1;
+
+	string_list_append(v, xstrdup(arg));
+	return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index d1b12fe..05eb09b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -130,6 +130,9 @@ struct option {
 				      (h), PARSE_OPT_NOARG, NULL, (p) }
 #define OPT_INTEGER(s, l, v, h)     { OPTION_INTEGER, (s), (l), (v), "n", (h) }
 #define OPT_STRING(s, l, v, a, h)   { OPTION_STRING,  (s), (l), (v), (a), (h) }
+#define OPT_STRING_LIST(s, l, v, a, h) \
+				    { OPTION_CALLBACK, (s), (l), (v), (a), \
+				      (h), 0, &parse_opt_string_list }
 #define OPT_UYN(s, l, v, h)         { OPTION_CALLBACK, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG, &parse_opt_tertiary }
 #define OPT_DATE(s, l, v, h) \
@@ -204,6 +207,7 @@ extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
 extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 extern int parse_opt_with_commit(const struct option *, const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
+extern int parse_opt_string_list(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var, h)  OPT_BOOLEAN('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)    OPT_BOOLEAN('q', "quiet",   (var), (h))
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae26614..007f39d 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -28,6 +28,7 @@ String options
     --st <st>             get another string (pervert ordering)
     -o <str>              get another string
     --default-string      set string to default
+    --list <str>          add str to list
 
 Magic arguments
     --quux                means --quux
@@ -337,4 +338,20 @@ test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '
 	test_cmp expect output
 '
 
+cat >>expect <<'EOF'
+list: foo
+list: bar
+list: baz
+EOF
+test_expect_success '--list keeps list of strings' '
+	test-parse-options --list foo --list=bar --list=baz >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--no-list resets list' '
+	test-parse-options --list=other --list=irrelevant --list=options \
+		--no-list --list=foo --list=bar --list=baz >output &&
+	test_cmp expect output
+'
+
 test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 4e3710b..91a5701 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "parse-options.h"
+#include "string-list.h"
 
 static int boolean = 0;
 static int integer = 0;
@@ -9,6 +10,7 @@ static int verbose = 0, dry_run = 0, quiet = 0;
 static char *string = NULL;
 static char *file = NULL;
 static int ambiguous;
+static struct string_list list;
 
 static int length_callback(const struct option *opt, const char *arg, int unset)
 {
@@ -54,6 +56,7 @@ int main(int argc, const char **argv)
 		OPT_STRING('o', NULL, &string, "str", "get another string"),
 		OPT_SET_PTR(0, "default-string", &string,
 			"set string to default", (unsigned long)"default"),
+		OPT_STRING_LIST(0, "list", &list, "str", "add str to list"),
 		OPT_GROUP("Magic arguments"),
 		OPT_ARGUMENT("quux", "means --quux"),
 		OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
@@ -85,6 +88,9 @@ int main(int argc, const char **argv)
 	printf("dry run: %s\n", dry_run ? "yes" : "no");
 	printf("file: %s\n", file ? file : "(not set)");
 
+	for (i = 0; i < list.nr; i++)
+		printf("list: %s\n", list.items[i].string);
+
 	for (i = 0; i < argc; i++)
 		printf("arg %02d: %s\n", i, argv[i]);
 
-- 
1.7.6.rc1.12.g65e2

[PATCH 02/14] url: decode buffers that are not NUL-terminated

[Jeff King]

The url_decode function needs only minor tweaks to handle
arbitrary buffers. Let's do those tweaks, which cleans up an
unreadable mess of temporary strings in http.c.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c |   27 ++++-----------------------
 url.c  |   26 ++++++++++++++++++--------
 url.h  |    1 +
 3 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/http.c b/http.c
index a1ea3db..c93716c 100644
--- a/http.c
+++ b/http.c
@@ -307,8 +307,7 @@ static CURL *get_curl_handle(void)
 
 static void http_auth_init(const char *url)
 {
-	char *at, *colon, *cp, *slash, *decoded;
-	int len;
+	char *at, *colon, *cp, *slash;
 
 	cp = strstr(url, "://");
 	if (!cp)
@@ -328,29 +327,11 @@ static void http_auth_init(const char *url)
 		return; /* No credentials */
 	if (!colon || at <= colon) {
 		/* Only username */
-		len = at - cp;
-		user_name = xmalloc(len + 1);
-		memcpy(user_name, cp, len);
-		user_name[len] = '\0';
-		decoded = url_decode(user_name);
-		free(user_name);
-		user_name = decoded;
+		user_name = url_decode_mem(cp, at - cp);
 		user_pass = NULL;
 	} else {
-		len = colon - cp;
-		user_name = xmalloc(len + 1);
-		memcpy(user_name, cp, len);
-		user_name[len] = '\0';
-		decoded = url_decode(user_name);
-		free(user_name);
-		user_name = decoded;
-		len = at - (colon + 1);
-		user_pass = xmalloc(len + 1);
-		memcpy(user_pass, colon + 1, len);
-		user_pass[len] = '\0';
-		decoded = url_decode(user_pass);
-		free(user_pass);
-		user_pass = decoded;
+		user_name = url_decode_mem(cp, colon - cp);
+		user_pass = url_decode_mem(colon + 1, at - (colon + 1));
 	}
 }
 
diff --git a/url.c b/url.c
index 3e06fd3..389d9da 100644
--- a/url.c
+++ b/url.c
@@ -68,18 +68,20 @@ static int url_decode_char(const char *q)
 	return val;
 }
 
-static char *url_decode_internal(const char **query, const char *stop_at,
-				 struct strbuf *out, int decode_plus)
+static char *url_decode_internal(const char **query, int len,
+				 const char *stop_at, struct strbuf *out,
+				 int decode_plus)
 {
 	const char *q = *query;
 
-	do {
+	while (len) {
 		unsigned char c = *q;
 
 		if (!c)
 			break;
 		if (stop_at && strchr(stop_at, c)) {
 			q++;
+			len--;
 			break;
 		}
 
@@ -88,6 +90,7 @@ static char *url_decode_internal(const char **query, const char *stop_at,
 			if (0 <= val) {
 				strbuf_addch(out, val);
 				q += 3;
+				len -= 3;
 				continue;
 			}
 		}
@@ -97,34 +100,41 @@ static char *url_decode_internal(const char **query, const char *stop_at,
 		else
 			strbuf_addch(out, c);
 		q++;
-	} while (1);
+		len--;
+	}
 	*query = q;
 	return strbuf_detach(out, NULL);
 }
 
 char *url_decode(const char *url)
 {
+	return url_decode_mem(url, strlen(url));
+}
+
+char *url_decode_mem(const char *url, int len)
+{
 	struct strbuf out = STRBUF_INIT;
-	const char *colon = strchr(url, ':');
+	const char *colon = memchr(url, ':', len);
 
 	/* Skip protocol part if present */
 	if (colon && url < colon) {
 		strbuf_add(&out, url, colon - url);
+		len -= colon - url;
 		url = colon;
 	}
-	return url_decode_internal(&url, NULL, &out, 0);
+	return url_decode_internal(&url, len, NULL, &out, 0);
 }
 
 char *url_decode_parameter_name(const char **query)
 {
 	struct strbuf out = STRBUF_INIT;
-	return url_decode_internal(query, "&=", &out, 1);
+	return url_decode_internal(query, -1, "&=", &out, 1);
 }
 
 char *url_decode_parameter_value(const char **query)
 {
 	struct strbuf out = STRBUF_INIT;
-	return url_decode_internal(query, "&", &out, 1);
+	return url_decode_internal(query, -1, "&", &out, 1);
 }
 
 void end_url_with_slash(struct strbuf *buf, const char *url)
diff --git a/url.h b/url.h
index 7100e32..abdaf6f 100644
--- a/url.h
+++ b/url.h
@@ -4,6 +4,7 @@
 extern int is_url(const char *url);
 extern int is_urlschemechar(int first_flag, int ch);
 extern char *url_decode(const char *url);
+extern char *url_decode_mem(const char *url, int len);
 extern char *url_decode_parameter_name(const char **query);
 extern char *url_decode_parameter_value(const char **query);
 
-- 
1.7.6.rc1.12.g65e2

Re: [PATCH 02/14] url: decode buffers that are not NUL-terminated

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> The url_decode function needs only minor tweaks to handle
> arbitrary buffers. Let's do those tweaks, which cleans up an
> unreadable mess of temporary strings in http.c.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  http.c |   27 ++++-----------------------
>  url.c  |   26 ++++++++++++++++++--------
>  url.h  |    1 +
>  3 files changed, 23 insertions(+), 31 deletions(-)

Very nice.

[PATCH 03/14] improve httpd auth tests

[Jeff King]

These just checked that we could clone a repository when the
username and password were given in the URL; we should also
check that git will prompt when no or partial credentials
are given.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-httpd.sh        |   10 +++++---
 t/t5550-http-fetch.sh |   51 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index b8996a3..f7dc078 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -81,8 +81,7 @@ prepare_httpd() {
 
 	if test -n "$LIB_HTTPD_SSL"
 	then
-		HTTPD_URL=https://127.0.0.1:$LIB_HTTPD_PORT
-		AUTH_HTTPD_URL=https://user%40host:user%40host@127.0.0.1:$LIB_HTTPD_PORT
+		HTTPD_PROTO=https
 
 		RANDFILE_PATH="$HTTPD_ROOT_PATH"/.rnd openssl req \
 			-config "$TEST_PATH/ssl.cnf" \
@@ -93,9 +92,12 @@ prepare_httpd() {
 		export GIT_SSL_NO_VERIFY
 		HTTPD_PARA="$HTTPD_PARA -DSSL"
 	else
-		HTTPD_URL=http://127.0.0.1:$LIB_HTTPD_PORT
-		AUTH_HTTPD_URL=http://user%40host:user%40host@127.0.0.1:$LIB_HTTPD_PORT
+		HTTPD_PROTO=http
 	fi
+	HTTPD_DEST=127.0.0.1:$LIB_HTTPD_PORT
+	HTTPD_URL=$HTTPD_PROTO://$HTTPD_DEST
+	HTTPD_URL_USER=$HTTPD_PROTO://user%40host@$HTTPD_DEST
+	HTTPD_URL_USER_PASS=$HTTPD_PROTO://user%40host:user%40host@$HTTPD_DEST
 
 	if test -n "$LIB_HTTPD_DAV" -o -n "$LIB_HTTPD_SVN"
 	then
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index a1883ca..ed4db09 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -35,11 +35,54 @@ test_expect_success 'clone http repository' '
 	test_cmp file clone/file
 '
 
-test_expect_success 'clone http repository with authentication' '
+test_expect_success 'create password-protected repository' '
 	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/auth/" &&
-	cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" "$HTTPD_DOCUMENT_ROOT_PATH/auth/repo.git" &&
-	git clone $AUTH_HTTPD_URL/auth/repo.git clone-auth &&
-	test_cmp file clone-auth/file
+	cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
+	       "$HTTPD_DOCUMENT_ROOT_PATH/auth/repo.git"
+'
+
+test_expect_success 'setup askpass helpers' '
+	cat >askpass <<-EOF &&
+	#!/bin/sh
+	echo >>"$PWD/askpass-query" "askpass: \$*" &&
+	cat "$PWD/askpass-response"
+	EOF
+	chmod +x askpass &&
+	GIT_ASKPASS="$PWD/askpass" &&
+	export GIT_ASKPASS &&
+	>askpass-expect-none &&
+	echo "askpass: Password: " >askpass-expect-pass &&
+	{ echo "askpass: Username: " &&
+	  cat askpass-expect-pass
+	} >askpass-expect-both
+'
+
+test_expect_success 'cloning password-protected repository can fail' '
+	>askpass-query &&
+	echo wrong >askpass-response &&
+	test_must_fail git clone "$HTTPD_URL/auth/repo.git" clone-auth-fail &&
+	test_cmp askpass-expect-both askpass-query
+'
+
+test_expect_success 'http auth can use user/pass in URL' '
+	>askpass-query &&
+	echo wrong >askpass-reponse &&
+	git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
+	test_cmp askpass-expect-none askpass-query
+'
+
+test_expect_success 'http auth can use just user in URL' '
+	>askpass-query &&
+	echo user@host >askpass-response &&
+	git clone "$HTTPD_URL_USER/auth/repo.git" clone-auth-pass &&
+	test_cmp askpass-expect-pass askpass-query
+'
+
+test_expect_success 'http auth can request both user and pass' '
+	>askpass-query &&
+	echo user@host >askpass-response &&
+	git clone "$HTTPD_URL/auth/repo.git" clone-auth-both &&
+	test_cmp askpass-expect-both askpass-query
 '
 
 test_expect_success 'fetch changes via http' '
-- 
1.7.6.rc1.12.g65e2

[PATCH 04/14] remote-curl: don't retry auth failures with dumb protocol

[Jeff King]

When fetching an http URL, we first try fetching info/refs
with an extra "service" parameter. This will work for a
smart-http server, or a dumb server which ignores extra
parameters when fetching files. If that fails, we retry
without the extra parameter to remain compatible with dumb
servers which didn't like our first request.

If the server returned a "401 Unauthorized", indicating that
the credentials we provided were not good, there is not much
point in retrying. With the current code, we just waste an
extra round trip to the HTTP server before failing.

But as the http code becomes smarter about throwing away
rejected credentials and re-prompting the user for new ones
(which it will later in this series), this will become more
confusing. At some point we will stop asking for credentials
to retry smart http, and will be asking for credentials to
retry dumb http. So now we're not only wasting an extra HTTP
round trip for something that is unlikely to work, but we're
making the user re-type their password for it.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote-curl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index b5be25c..42b02af 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -115,7 +115,7 @@ static struct discovery* discover_refs(const char *service)
 	http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
 
 	/* try again with "plain" url (no ? or & appended) */
-	if (http_ret != HTTP_OK) {
+	if (http_ret != HTTP_OK && http_ret != HTTP_NOAUTH) {
 		free(refs_url);
 		strbuf_reset(&buffer);
 
-- 
1.7.6.rc1.12.g65e2

[PATCH 05/14] http: retry authentication failures for all http requests

[Jeff King]

Commit 42653c0 (Prompt for a username when an HTTP request
401s, 2010-04-01) changed http_get_strbuf to prompt for
credentials when we receive a 401, but didn't touch
http_get_file. The latter is called only for dumb http;
while it's usually the case that people don't use
authentication on top of dumb http, there is no reason not
to allow both types of requests to use this feature.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/http.c b/http.c
index c93716c..89e3cf4 100644
--- a/http.c
+++ b/http.c
@@ -846,13 +846,18 @@ static int http_request(const char *url, void *result, int target, int options)
 	return ret;
 }
 
+static int http_request_reauth(const char *url, void *result, int target,
+			       int options)
+{
+	int ret = http_request(url, result, target, options);
+	if (ret != HTTP_REAUTH)
+		return ret;
+	return http_request(url, result, target, options);
+}
+
 int http_get_strbuf(const char *url, struct strbuf *result, int options)
 {
-	int http_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);
-	if (http_ret == HTTP_REAUTH) {
-		http_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);
-	}
-	return http_ret;
+	return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
 }
 
 /*
@@ -875,7 +880,7 @@ static int http_get_file(const char *url, const char *filename, int options)
 		goto cleanup;
 	}
 
-	ret = http_request(url, result, HTTP_REQUEST_FILE, options);
+	ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
 	fclose(result);
 
 	if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
-- 
1.7.6.rc1.12.g65e2

[PATCH 06/14] introduce credentials API

[Jeff King]

There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.

Right now the only choices for providing credentials are to
put them plaintext into your ~/.netrc, or to have git prompt
you (either on the terminal or via an askpass program). The
former is not very secure, and the latter is not very
convenient.

Unfortunately, there is no "always best" solution for
password management. The details will depend on the tradeoff
you want between security and convenience, as well as how
git can integrate with other security systems (e.g., many
operating systems provide a keychain or password wallet for
single sign-on).

This patch abstracts the notion of gathering user
credentials into a few simple functions. These functions can
be backed by our internal git_getpass implementation (which
just prompts the user), or by external helpers which are
free to consult system-specific password wallets, make
custom policy decisions on password caching and storage, or
prompt the user in a non-traditional manner.

The helper protocol aims for simplicity of helper
implementation; see the newly added documentation for
details.

Signed-off-by: Jeff King <peff@peff.net>
---
 .gitignore                                  |    1 +
 Documentation/technical/api-credentials.txt |  113 ++++++++++++++++
 Makefile                                    |    3 +
 credential.c                                |  190 +++++++++++++++++++++++++++
 credential.h                                |   19 +++
 t/t0300-credentials.sh                      |  175 ++++++++++++++++++++++++
 test-credential.c                           |   47 +++++++
 7 files changed, 548 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/technical/api-credentials.txt
 create mode 100644 credential.c
 create mode 100644 credential.h
 create mode 100755 t/t0300-credentials.sh
 create mode 100644 test-credential.c

diff --git a/.gitignore b/.gitignore
index 8572c8c..7d2fefc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -167,6 +167,7 @@
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
 /test-chmtime
+/test-credential
 /test-ctype
 /test-date
 /test-delta
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
new file mode 100644
index 0000000..880db92
--- /dev/null
+++ b/Documentation/technical/api-credentials.txt
@@ -0,0 +1,113 @@
+credentials API
+===============
+
+The credentials API provides an abstracted way of gathering username and
+password credentials from the user (even though credentials in the wider
+world can take many forms, in this document the word "credential" always
+refers to a username and password pair).
+
+Data Structures
+---------------
+
+`struct credential`::
+
+	This struct represents a single username/password combination.
+	The `username` and `password` fields should be heap-allocated
+	strings (or NULL if they are not yet known). The `unique` field,
+	if non-NULL, should be a heap-allocated string indicating a
+	unique context for this credential (e.g., a protocol and server
+	name for a remote credential). The `description` field, if
+	non-NULL, should point to a string containing a human-readable
+	description of this credential.
+
+`struct string_list methods`::
+
+	The credential functions take a `string_list` of methods for
+	acquiring credentials. Each string specifies an external
+	helper which will be run, in order, to acquire credentials,
+	until both a username and password have been acquired. A NULL or
+	empty methods list indicates that the internal
+	`credential_getpass` function should be used.
+
+
+Functions
+---------
+
+`credential_fill_gently`::
+
+	Attempt to fill the username and password fields of the passed
+	credential struct. If they cannot be filled after trying each
+	available method, returns -1. Otherwise, returns 0.
+
+`credential_fill`::
+
+	Like `credential_fill_gently`, but `die()` if credentials cannot
+	be gathered.
+
+`credential_reject`::
+
+	Inform the credential subsystem that the provided credentials
+	have been rejected. This will clear the username and password
+	fields in `struct credential`, as well as notify any helpers of
+	the rejection (which may, for example, purge the invalid
+	credentials from storage).
+
+`credential_getpass`::
+
+	Fetch credentials from the user either using an "askpass" helper
+	(see the discussion of core.askpass and GIT_ASKPASS in
+	linkgit:git-config[1] and linkgit:git[1], respectively) or by
+	prompting the user via the terminal.
+
+
+Credential Helpers
+------------------
+
+Credential helpers are programs executed by git to fetch credentials
+from storage or from the user. The default behavior when no helpers are
+defined is to use the internal `credential_askpass` function.
+
+When a helper is executed, it may receive the following options on the
+command line:
+
+`--reject`::
+
+	Specify that the provided credential has been rejected; the
+	helper may take appropriate action to purge any credential
+	storage or cache. If this option is not given, the helper should
+	assume a credential is being requested.
+
+`--description=<X>`::
+
+	`<X>` will contain a human-readable description of the
+	credential being requested. If this option is not given, no
+	description is available.
+
+`--unique=<X>`::
+
+	`<X>` will contain a token to uniquely identify the context of
+	the credential (e.g., a host name for network authentication).
+	If this option is not given, no context is available.
+
+`--username=<X>`::
+
+	`<X>` will contain the username requested by the user. If this
+	option is not given, no username is available, and the helper
+	should provide both a username and password.
+
+The helper should produce a list of items on stdout, each followed by a
+newline character. Each item should consist of a key-value pair, separated
+by an `=` (equals) sign. The value may contain any bytes except a
+newline. When reading the response, git understands the following keys:
+
+`username`::
+
+	The username part of the credential. If a username was given to
+	the helper via `--username`, the new value will override it.
+
+`password`::
+
+	The password part of the credential.
+
+It is perfectly acceptable for a helper to provide only part of a
+credential, or nothing at all.
diff --git a/Makefile b/Makefile
index 46793d1..16515bf 100644
--- a/Makefile
+++ b/Makefile
@@ -421,6 +421,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-credential
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
@@ -511,6 +512,7 @@ LIB_H += compat/win32/pthread.h
 LIB_H += compat/win32/syslog.h
 LIB_H += compat/win32/sys/poll.h
 LIB_H += compat/win32/dirent.h
+LIB_H += credential.h
 LIB_H += csum-file.h
 LIB_H += decorate.h
 LIB_H += delta.h
@@ -590,6 +592,7 @@ LIB_OBJS += config.o
 LIB_OBJS += connect.o
 LIB_OBJS += convert.o
 LIB_OBJS += copy.o
+LIB_OBJS += credential.o
 LIB_OBJS += csum-file.o
 LIB_OBJS += ctype.o
 LIB_OBJS += date.o
diff --git a/credential.c b/credential.c
new file mode 100644
index 0000000..c403289
--- /dev/null
+++ b/credential.c
@@ -0,0 +1,190 @@
+#include "cache.h"
+#include "credential.h"
+#include "quote.h"
+#include "string-list.h"
+#include "run-command.h"
+
+static char *credential_ask_one(const char *what, const char *desc)
+{
+	struct strbuf prompt = STRBUF_INIT;
+	char *r;
+
+	if (desc)
+		strbuf_addf(&prompt, "%s for '%s': ", what, desc);
+	else
+		strbuf_addf(&prompt, "%s: ", what);
+
+	/* FIXME: for usernames, we should do something less magical that
+	 * actually echoes the characters. However, we need to read from
+	 * /dev/tty and not stdio, which is not portable (but getpass will do
+	 * it for us). http.c uses the same workaround. */
+	r = git_getpass(prompt.buf);
+
+	strbuf_release(&prompt);
+	return xstrdup(r);
+}
+
+int credential_getpass(struct credential *c)
+{
+
+	if (!c->username)
+		c->username = credential_ask_one("Username", c->description);
+	if (!c->password)
+		c->password = credential_ask_one("Password", c->description);
+	return 0;
+}
+
+static int read_credential_response(struct credential *c, FILE *fp)
+{
+	struct strbuf response = STRBUF_INIT;
+
+	while (strbuf_getline(&response, fp, '\n') != EOF) {
+		char *key = response.buf;
+		char *value = strchr(key, '=');
+
+		if (!value) {
+			warning("bad output from credential helper: %s", key);
+			strbuf_release(&response);
+			return -1;
+		}
+		*value++ = '\0';
+
+		if (!strcmp(key, "username")) {
+			free(c->username);
+			c->username = xstrdup(value);
+		}
+		else if (!strcmp(key, "password")) {
+			free(c->password);
+			c->password = xstrdup(value);
+		}
+		/* ignore other responses; we don't know what they mean */
+	}
+
+	strbuf_release(&response);
+	return 0;
+}
+
+static int run_credential_helper(struct credential *c, const char *cmd)
+{
+	struct child_process helper;
+	const char *argv[] = { NULL, NULL };
+	FILE *fp;
+	int r;
+
+	memset(&helper, 0, sizeof(helper));
+	argv[0] = cmd;
+	helper.argv = argv;
+	helper.use_shell = 1;
+	helper.no_stdin = 1;
+	helper.out = -1;
+
+	if (start_command(&helper))
+		return -1;
+	fp = xfdopen(helper.out, "r");
+
+	r = read_credential_response(c, fp);
+
+	fclose(fp);
+	if (finish_command(&helper))
+		r = -1;
+
+	return r;
+}
+
+static void add_item(struct strbuf *out, const char *key, const char *value)
+{
+	if (!value)
+		return;
+	strbuf_addf(out, " --%s=", key);
+	sq_quote_buf(out, value);
+}
+
+static int first_word_is_alnum(const char *s)
+{
+	for (; *s && *s != ' '; s++)
+		if (!isalnum(*s))
+			return 0;
+	return 1;
+}
+
+static int credential_do(struct credential *c, const char *method,
+			 const char *extra)
+{
+	struct strbuf cmd = STRBUF_INIT;
+	int r;
+
+	if (first_word_is_alnum(method))
+		strbuf_addf(&cmd, "git credential-%s", method);
+	else
+		strbuf_addstr(&cmd, method);
+
+	if (extra)
+		strbuf_addf(&cmd, " %s", extra);
+
+	add_item(&cmd, "description", c->description);
+	add_item(&cmd, "unique", c->unique);
+	add_item(&cmd, "username", c->username);
+
+	r = run_credential_helper(c, cmd.buf);
+
+	strbuf_release(&cmd);
+	return r;
+}
+
+void credential_fill(struct credential *c, const struct string_list *methods)
+{
+	struct strbuf err = STRBUF_INIT;
+
+	if (!credential_fill_gently(c, methods))
+		return;
+
+	strbuf_addstr(&err, "unable to get credentials");
+	if (c->description)
+		strbuf_addf(&err, "for '%s'", c->description);
+	if (methods && methods->nr == 1)
+		strbuf_addf(&err, "; tried '%s'", methods->items[0].string);
+	else if (methods) {
+		int i;
+		strbuf_addstr(&err, "; tried:");
+		for (i = 0; i < methods->nr; i++)
+			strbuf_addf(&err, "\n  %s", methods->items[i].string);
+	}
+	die(err.buf);
+}
+
+int credential_fill_gently(struct credential *c,
+			   const struct string_list *methods)
+{
+	int i;
+
+	if (c->username && c->password)
+		return 0;
+
+	if (!methods || !methods->nr)
+		return credential_getpass(c);
+
+	for (i = 0; i < methods->nr; i++) {
+		if (!credential_do(c, methods->items[i].string, NULL) &&
+		    c->username && c->password)
+			return 0;
+	}
+
+	return -1;
+}
+
+void credential_reject(struct credential *c, const struct string_list *methods)
+{
+	int i;
+
+	if (methods && c->username) {
+		for (i = 0; i < methods->nr; i++) {
+			/* ignore errors, there's nothing we can do */
+			credential_do(c, methods->items[i].string, "--reject");
+		}
+	}
+
+	free(c->username);
+	c->username = NULL;
+	free(c->password);
+	c->password = NULL;
+}
diff --git a/credential.h b/credential.h
new file mode 100644
index 0000000..383b720
--- /dev/null
+++ b/credential.h
@@ -0,0 +1,19 @@
+#ifndef CREDENTIAL_H
+#define CREDENTIAL_H
+
+struct credential {
+	char *description;
+	char *username;
+	char *password;
+	char *unique;
+};
+
+struct string_list;
+
+int credential_getpass(struct credential *);
+
+int credential_fill_gently(struct credential *, const struct string_list *methods);
+void credential_fill(struct credential *, const struct string_list *methods);
+void credential_reject(struct credential *, const struct string_list *methods);
+
+#endif /* CREDENTIAL_H */
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
new file mode 100755
index 0000000..447e983
--- /dev/null
+++ b/t/t0300-credentials.sh
@@ -0,0 +1,175 @@
+#!/bin/sh
+
+test_description='basic credential helper tests'
+. ./test-lib.sh
+
+# Try a set of credential helpers; the expected
+# stdout and stderr should be provided on stdin,
+# separated by "--".
+check() {
+	while read line; do
+		case "$line" in
+		--) break ;;
+		*) echo "$line" ;;
+		esac
+	done >expect-stdout &&
+	cat >expect-stderr &&
+	test-credential "$@" >stdout 2>stderr &&
+	test_cmp expect-stdout stdout &&
+	test_cmp expect-stderr stderr
+}
+
+test_expect_success 'setup helper scripts' '
+	cat >dump <<-\EOF &&
+	whoami=$1; shift
+	if test $# = 0; then
+		echo >&2 "$whoami: <empty>"
+	else
+		for i in "$@"; do
+			echo >&2 "$whoami: $i"
+		done
+	fi
+	EOF
+	chmod +x dump &&
+
+	cat >git-credential-useless <<-\EOF &&
+	#!/bin/sh
+	dump useless "$@"
+	exit 0
+	EOF
+	chmod +x git-credential-useless &&
+
+	cat >git-credential-verbatim <<-\EOF &&
+	#!/bin/sh
+	user=$1; shift
+	pass=$1; shift
+	dump verbatim "$@"
+	test -z "$user" || echo username=$user
+	test -z "$pass" || echo password=$pass
+	EOF
+	chmod +x git-credential-verbatim &&
+
+	cat >askpass <<-\EOF &&
+	#!/bin/sh
+	echo >&2 askpass: $*
+	echo askpass-result
+	EOF
+	chmod +x askpass &&
+	GIT_ASKPASS=askpass &&
+	export GIT_ASKPASS &&
+
+	PATH="$PWD:$PATH"
+'
+
+test_expect_success 'credential_fill invokes helper' '
+	check "verbatim foo bar" <<-\EOF
+	username=foo
+	password=bar
+	--
+	verbatim: <empty>
+	EOF
+'
+
+test_expect_success 'credential_fill invokes multiple helpers' '
+	check useless "verbatim foo bar" <<-\EOF
+	username=foo
+	password=bar
+	--
+	useless: <empty>
+	verbatim: <empty>
+	EOF
+'
+
+test_expect_success 'credential_fill stops when we get a full response' '
+	check "verbatim one two" "verbatim three four" <<-\EOF
+	username=one
+	password=two
+	--
+	verbatim: <empty>
+	EOF
+'
+
+test_expect_success 'credential_fill continues through partial response' '
+	check "verbatim one \"\"" "verbatim two three" <<-\EOF
+	username=two
+	password=three
+	--
+	verbatim: <empty>
+	verbatim: --username=one
+	EOF
+'
+
+test_expect_success 'credential_fill passes along metadata' '
+	check --description=foo --unique=bar "verbatim one two" <<-\EOF
+	username=one
+	password=two
+	--
+	verbatim: --description=foo
+	verbatim: --unique=bar
+	EOF
+'
+
+test_expect_success 'credential_reject calls all helpers' '
+	check --reject --username=foo useless "verbatim one two" <<-\EOF
+	--
+	useless: --reject
+	useless: --username=foo
+	verbatim: --reject
+	verbatim: --username=foo
+	EOF
+'
+
+test_expect_success 'do not bother rejecting empty credential' '
+	check --reject useless <<-\EOF
+	--
+	EOF
+'
+
+test_expect_success 'usernames can be preserved' '
+	check --username=one "verbatim \"\" three" <<-\EOF
+	username=one
+	password=three
+	--
+	verbatim: --username=one
+'
+
+test_expect_success 'usernames can be overridden' '
+	check --username=one "verbatim two three" <<-\EOF
+	username=two
+	password=three
+	--
+	verbatim: --username=one
+	EOF
+'
+
+test_expect_success 'do not bother completing already-full credential' '
+	check --username=one --password=two "verbatim three four" <<-\EOF
+	username=one
+	password=two
+	--
+	EOF
+'
+
+# We can't test the basic terminal password prompt here because
+# getpass() tries too hard to find the real terminal. But if our
+# askpass helper is run, we know the internal getpass is working.
+test_expect_success 'empty methods falls back to internal getpass' '
+	check <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
+test_expect_success 'internal getpass does not ask for known username' '
+	check --username=foo <<-\EOF
+	username=foo
+	password=askpass-result
+	--
+	askpass: Password:
+	EOF
+'
+
+test_done
diff --git a/test-credential.c b/test-credential.c
new file mode 100644
index 0000000..3929efd
--- /dev/null
+++ b/test-credential.c
@@ -0,0 +1,47 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "parse-options.h"
+
+int main(int argc, const char **argv)
+{
+	int reject = 0;
+	struct credential c = { NULL };
+	struct string_list methods = STRING_LIST_INIT_NODUP;
+	const char *const usage[] = {
+		"test-credential [options] [method...]",
+		NULL
+	};
+	struct option options[] = {
+		OPT_BOOLEAN(0, "reject", &reject, "reject"),
+		OPT_STRING(0, "description", &c.description, "desc",
+			   "description"),
+		OPT_STRING(0, "unique", &c.unique, "token",
+			   "unique"),
+		OPT_STRING(0, "username", &c.username, "name", "username"),
+		OPT_STRING(0, "password", &c.password, "pass", "password"),
+		OPT_END()
+	};
+	int i;
+
+	argc = parse_options(argc, argv, NULL, options, usage, 0);
+	for (i = 0; i < argc; i++)
+		string_list_append(&methods, argv[i]);
+	/* credential_reject will try to free() */
+	if (c.username)
+		c.username = xstrdup(c.username);
+	if (c.password)
+		c.password = xstrdup(c.password);
+
+	if (reject)
+		credential_reject(&c, &methods);
+	else
+		credential_fill(&c, &methods);
+
+	if (c.username)
+		printf("username=%s\n", c.username);
+	if (c.password)
+		printf("password=%s\n", c.password);
+
+	return 0;
+}
-- 
1.7.6.rc1.12.g65e2

Re: [PATCH 06/14] introduce credentials API

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> +`struct credential`::
> +
> +	This struct represents a single username/password combination.
> +	The `username` and `password` fields should be heap-allocated
> +	strings (or NULL if they are not yet known). The `unique` field,
> +	if non-NULL, should be a heap-allocated string indicating a
> +	unique context for this credential (e.g., a protocol and server
> +	name for a remote credential). The `description` field, if
> +	non-NULL, should point to a string containing a human-readable
> +	description of this credential.

Am I confused to say "unique" is for code/machine and "description" is for
human to identify what remote resource is being accessed using the
credential? Can two credentials have the same description but different
unique, and if so what happens? A plausible answer may be "the user cannot
tell what username/password pair to give, but it is expected that such a
nondescriptive context is only for certificate and it also is expected
that only one certificate is used at a time", perhaps?

I think "unique" vs "description" are secondary aspects of these things,
and the primary aspect that they share is that they are about "context",
as [11/14] describes.  Perhaps "context-id" vs "context-description" may
be better names to reduce confusion?  I dunno.

> +`credential_fill`::
> +
> +	Like `credential_fill_gently`, but `die()` if credentials cannot
> +	be gathered.
> +
> +`credential_reject`::
> +
> +	Inform the credential subsystem that the provided credentials
> +	have been rejected. This will clear the username and password
> +	fields in `struct credential`, as well as notify any helpers of
> +	the rejection (which may, for example, purge the invalid
> +	credentials from storage).

What hints do helpers get when this is called? Do they get username,
unique and description to allow them to selectively purge the bad ones
while keeping good ones, or the username is already cleared by the time
the helpers are notified and they have no clue?

> +`credential_getpass`::
> +
> +	Fetch credentials from the user either using an "askpass" helper
> +	(see the discussion of core.askpass and GIT_ASKPASS in
> +	linkgit:git-config[1] and linkgit:git[1], respectively) or by
> +	prompting the user via the terminal.

It sounds like that users of the API should call fill_credential() and let
the machinery fall back to this function as needed. Does this need to be
part of the public API functions?

> +static int read_credential_response(struct credential *c, FILE *fp)
> +{
> +	struct strbuf response = STRBUF_INIT;
> +
> +	while (strbuf_getline(&response, fp, '\n') != EOF) {
> +		char *key = response.buf;
> +		char *value = strchr(key, '=');
> +
> +		if (!value) {
> +			warning("bad output from credential helper: %s", key);
> +			strbuf_release(&response);
> +			return -1;
> +		}
> +		*value++ = '\0';
> +
> +		if (!strcmp(key, "username")) {
> +			free(c->username);
> +			c->username = xstrdup(value);

The document did not say anything about escaping/quoting of values, but it
may not be a bad idea to make it more explicit over there.

Re: [PATCH 06/14] introduce credentials API

[Jeff King]

On Wed, Jul 20, 2011 at 03:17:02PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > +`struct credential`::
> > +
> > +	This struct represents a single username/password combination.
> > +	The `username` and `password` fields should be heap-allocated
> > +	strings (or NULL if they are not yet known). The `unique` field,
> > +	if non-NULL, should be a heap-allocated string indicating a
> > +	unique context for this credential (e.g., a protocol and server
> > +	name for a remote credential). The `description` field, if
> > +	non-NULL, should point to a string containing a human-readable
> > +	description of this credential.
> 
> Am I confused to say "unique" is for code/machine and "description" is for
> human to identify what remote resource is being accessed using the
> credential?

No, you're not confused. That is the intent.

> Can two credentials have the same description but different unique,
> and if so what happens?

Yes, they could. In fact, you can do it with the code in my series. The
URL "http://example.com" will have unique context "http:example.com",
and a description of "example.com". The ssl-enabled version,
"https://example.com", will have the same description, but a different
unique context ("https:example.com").

I wanted the machine-readable token to be complete and unambiguous,
since it may be sent automatically. Deciding to send a password over
unencrypted http versus https is a policy decision that only the user
can make, and we should err on the side of caution.

The human-readable portion, on the other hand, can assume that the user
has some context for the request, and can make the right decision. So we
can afford to put less information it, if it makes the output prettier
to human eyes.

One could argue that we still need to provide as much information as
possible to a human user. For example, if I cut-and-paste a URL with
"http" instead of "https", I might fail to notice and provide my
password accidentally over an unencrypted link. Including the missing
information in the human-readable part could help with that case, at the
cost of making the prompt a little longer than necessary for other
cases.

Another example is that we say only "Password for 'certificate':" when
unlocking a certificate, even though the certificate's unique token
mentions the filename. This is the same case: the user has the extra
context to know which certificate we mean, so we can make the message
prettier. Again, I think one could argue that it's better to provide the
context to the user, in case they don't have it.

In both of the above paragraphs, my "one could argue..." statements can
be translated as "I argued with myself about this, and am on the fence.
If you feel strongly, it's easy to make it so."

> A plausible answer may be "the user cannot tell what username/password
> pair to give, but it is expected that such a nondescriptive context is
> only for certificate and it also is expected that only one certificate
> is used at a time", perhaps?

Right.

> I think "unique" vs "description" are secondary aspects of these things,
> and the primary aspect that they share is that they are about "context",
> as [11/14] describes.  Perhaps "context-id" vs "context-description" may
> be better names to reduce confusion?  I dunno.

Yeah, it might be better to unify the concepts by giving them similar
names. It might even make sense to call them "context_id" and
"context_human" to make the difference more clear.

Another option would be to drop the concept entirely, and simply give
the helpers as many descriptive tokens as we can. So instead of:

  unique=https:example.com
  description=example.com

and

  unique=cert:/path/to/cert.pem
  description=certificate

we would give:

  type=network
  proto=https
  host=example.com
  path=repo.git

and

  type=certificate
  file=/path/to/cert.pem

and then let the helper generate an appropriate token or description
from those fields. That provides more flexibility for a helper to make a
decision. I avoided doing that in the first place for two reasons:

  1. It pushes more work onto the helpers to generate a token, or
     to generate a nice human-readable prompt. So we may end up with
     inconsistency between helpers, both in terms of messages shown to
     the user and in security properties (e.g., helpers may decide
     differently on policy issues like "are http and https similar
     enough to share passwords? What about imaps and https?"). Not to
     mention the duplicated effort among helper writers.

  2. I didn't know where it would end. Right now, those examples above
     are enough for what git uses. But when we add new credential
     locations (certainly imap-send is a place we could use it; probably
     send-email could also use it for smtp auth), will it be enough? How
     will already-written credential helpers cope with the new fields?

The Gnome Keyring API seems to just provide space for a free-form set of
key/value pairs. But I haven't found a micro-format which says which
fields should be used, so I guess it's up to the application to use them
consistently. The OS X keychain API has very specific fields for
protocol, hostname, etc (and even has different classes of "this is an
internet password" versus "this is a generic password").

So maybe it is simply better to break the information down as much as
possible and let the password wallet helpers do what they will with it.

> > +`credential_reject`::
> > +
> > +	Inform the credential subsystem that the provided credentials
> > +	have been rejected. This will clear the username and password
> > +	fields in `struct credential`, as well as notify any helpers of
> > +	the rejection (which may, for example, purge the invalid
> > +	credentials from storage).
> 
> What hints do helpers get when this is called? Do they get username,
> unique and description to allow them to selectively purge the bad ones
> while keeping good ones, or the username is already cleared by the time
> the helpers are notified and they have no clue?

They get username, unique, and description, to let them purge the
minimal amount (they are always free to ignore the username and purge
more, of course, if they are backed by less-capable storage).

We could also hold open a connection to the helper that gave us
information, try the credential, and then say either "OK" or "FAIL".
I specifically tried to avoid bi-directional interactive communication
in this protocol, though, in the name of simplicity. The helper gets
everything from git in one shot via the command line, and then dumps
everything back in one shot via stdout.

>From looking at a few APIs of system password wallets, it seems that
many expose functionality that is just "get" and "put". So normal use
would be something like:

  while (1) {
    if (!wallet_get(&cred))
      ask_user(&cred);
    if (request_with_cred(&cred))
      break;
  }
  wallet_put(&cred);

and it is up to the program to implement "ask_user".  So another option
would be to just expose "get" and "put" via credential helpers, which
keeps things pretty simple.  In my proposal, helpers are actually
responsible for asking the user.

I did that intentionally to provide a mechanism for system-specific
prompts. For example, you could pop up a single dialog with both
username and password fields. The username field might be filled in
already, but the user could override it.  You can't do anything quite as
nice with the askpass interface, which receives the questions one at a
time.

I also wanted to make sure we were flexible to systems which do
incorporate the "ask the user" step automatically. From my brief reading
of the freedesktop Secrets API, it does so.

> > +`credential_getpass`::
> > +
> > +	Fetch credentials from the user either using an "askpass" helper
> > +	(see the discussion of core.askpass and GIT_ASKPASS in
> > +	linkgit:git-config[1] and linkgit:git[1], respectively) or by
> > +	prompting the user via the terminal.
> 
> It sounds like that users of the API should call fill_credential() and let
> the machinery fall back to this function as needed. Does this need to be
> part of the public API functions?

The "getpass" helper calls it, though I suppose it could also just use
credential_fill with an empty list. I mainly left it as a convenience
function if there is something that doesn't want to use the regular
"fill" process, but I don't think it's critical.

> > +static int read_credential_response(struct credential *c, FILE *fp)
> > +{
> > +	struct strbuf response = STRBUF_INIT;
> > +
> > +	while (strbuf_getline(&response, fp, '\n') != EOF) {
> > +		char *key = response.buf;
> > +		char *value = strchr(key, '=');
> > +
> > +		if (!value) {
> > +			warning("bad output from credential helper: %s", key);
> > +			strbuf_release(&response);
> > +			return -1;
> > +		}
> > +		*value++ = '\0';
> > +
> > +		if (!strcmp(key, "username")) {
> > +			free(c->username);
> > +			c->username = xstrdup(value);
> 
> The document did not say anything about escaping/quoting of values, but it
> may not be a bad idea to make it more explicit over there.

There is no quoting or escaping. As the document says, "the value may
contain any bytes except a newline". It doesn't mention that keys cannot
contain an "=" or a newline, though that is also true.

Again, I went for simplicity of helper implementation, in the hope that
usernames and passwords could get by without newline characters. We
could switch it to "\0" if it matters. That makes shell implementations
of helpers a little tougher. But it's much easier than quoting.

-Peff

Re: [PATCH 06/14] introduce credentials API

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> > +`credential_reject`::
>> > +
>> > +	Inform the credential subsystem that the provided credentials
>> > +	have been rejected. This will clear the username and password
>> > +	fields in `struct credential`, as well as notify any helpers of
>> > +	the rejection (which may, for example, purge the invalid
>> > +	credentials from storage).
>> 
>> What hints do helpers get when this is called? Do they get username,
>> unique and description to allow them to selectively purge the bad ones
>> while keeping good ones, or the username is already cleared by the time
>> the helpers are notified and they have no clue?
>
> They get username, unique, and description, to let them purge the
> minimal amount (they are always free to ignore the username and purge
> more, of course, if they are backed by less-capable storage).
>
> We could also...

Ahh, that wasn't what I was getting at. I was only reacting to the order
of events described in "clear them, as well as notify" (as if these
cleared information is no longer available to be used to nitify) and
asking for clarification. As long as enough information is sent to the
helper to selectively purge stale/mistyped information so that it can ask
again, I am perfectly happy.
>> The document did not say anything about escaping/quoting of values, but it
>> may not be a bad idea to make it more explicit over there.
>
> There is no quoting or escaping. As the document says, "the value may
> contain any bytes except a newline". It doesn't mention that keys cannot
> contain an "=" or a newline, though that is also true.

This also was a documentation comment; "may contain any bytes" can be
followed with "and used as-is" and I wouldn't have been confused.

Thanks.

Re: [PATCH 06/14] introduce credentials API

[Jeff King]

On Fri, Jul 22, 2011 at 02:42:09PM -0700, Junio C Hamano wrote:

> >> The document did not say anything about escaping/quoting of values, but it
> >> may not be a bad idea to make it more explicit over there.
> >
> > There is no quoting or escaping. As the document says, "the value may
> > contain any bytes except a newline". It doesn't mention that keys cannot
> > contain an "=" or a newline, though that is also true.
> 
> This also was a documentation comment; "may contain any bytes" can be
> followed with "and used as-is" and I wouldn't have been confused.

Makes sense. I'll tweak for the next version.

-Peff

Re: [PATCH 06/14] introduce credentials API

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> diff --git a/credential.c b/credential.c
> new file mode 100644
> index 0000000..c403289
> --- /dev/null
> +++ b/credential.c
> @@ -0,0 +1,190 @@
> ...
> +void credential_fill(struct credential *c, const struct string_list *methods)
> +{
> +	...
> +	}
> +	die(err.buf);

credential.c:195: error: format not a string literal and no format arguments

This needs to be

	die("%s", err.buf);

Re: [PATCH 06/14] introduce credentials API

[Jeff King]

On Thu, Jul 21, 2011 at 02:59:31PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > diff --git a/credential.c b/credential.c
> > new file mode 100644
> > index 0000000..c403289
> > --- /dev/null
> > +++ b/credential.c
> > @@ -0,0 +1,190 @@
> > ...
> > +void credential_fill(struct credential *c, const struct string_list *methods)
> > +{
> > +	...
> > +	}
> > +	die(err.buf);
> 
> credential.c:195: error: format not a string literal and no format arguments
> 
> This needs to be
> 
> 	die("%s", err.buf);

Thanks, silly me. I'm surprised I didn't get a warning, as I compile
with -Wall. Is there an extra flag you need to give gcc?

-Peff

[PATCH 07/14] http: use credential API to get passwords

[Jeff King]

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 a struct). The biggest
change is determining a "unique" context to pass to the
credential API.  This patch uses "http:$host" for http
authentication and "cert:$file" for opening certificate
files.

We pass an empty list of methods to the credential API,
which means that we will use the internal credential_getpass
function. This should yield no behavior change, except that
we now print "Password for 'certificate':" instead of
"Certificate Password:" when asking for certificate
passwords.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c                |   90 +++++++++++++++++++++++++++----------------------
 t/t5550-http-fetch.sh |    2 +-
 2 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/http.c b/http.c
index 89e3cf4..4c047be 100644
--- a/http.c
+++ b/http.c
@@ -3,6 +3,7 @@
 #include "sideband.h"
 #include "run-command.h"
 #include "url.h"
+#include "credential.h"
 
 int data_received;
 int active_requests;
@@ -42,7 +43,7 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
-static char *user_name, *user_pass;
+static struct credential http_auth;
 static const char *user_agent;
 
 #if LIBCURL_VERSION_NUM >= 0x071700
@@ -53,7 +54,7 @@ static const char *user_agent;
 #define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD
 #endif
 
-static char *ssl_cert_password;
+static struct credential cert_auth;
 static int ssl_cert_password_required;
 
 static struct curl_slist *pragma_header;
@@ -211,11 +212,11 @@ static int http_options(const char *var, const char *value, void *cb)
 
 static void init_curl_http_auth(CURL *result)
 {
-	if (user_name) {
+	if (http_auth.username) {
 		struct strbuf up = STRBUF_INIT;
-		if (!user_pass)
-			user_pass = xstrdup(git_getpass("Password: "));
-		strbuf_addf(&up, "%s:%s", user_name, user_pass);
+		credential_fill(&http_auth, NULL);
+		strbuf_addf(&up, "%s:%s",
+			    http_auth.username, http_auth.password);
 		curl_easy_setopt(result, CURLOPT_USERPWD,
 				 strbuf_detach(&up, NULL));
 	}
@@ -223,18 +224,19 @@ static void init_curl_http_auth(CURL *result)
 
 static int has_cert_password(void)
 {
-	if (ssl_cert_password != NULL)
-		return 1;
 	if (ssl_cert == NULL || ssl_cert_password_required != 1)
 		return 0;
-	/* Only prompt the user once. */
-	ssl_cert_password_required = -1;
-	ssl_cert_password = git_getpass("Certificate Password: ");
-	if (ssl_cert_password != NULL) {
-		ssl_cert_password = xstrdup(ssl_cert_password);
-		return 1;
-	} else
-		return 0;
+	if (!cert_auth.description)
+		cert_auth.description = "certificate";
+	if (!cert_auth.unique) {
+		struct strbuf unique = STRBUF_INIT;
+		strbuf_addf(&unique, "cert:%s", ssl_cert);
+		cert_auth.unique = strbuf_detach(&unique, NULL);
+	}
+	if (!cert_auth.username)
+		cert_auth.username = xstrdup("");
+	credential_fill(&cert_auth, NULL);
+	return 1;
 }
 
 static CURL *get_curl_handle(void)
@@ -263,7 +265,7 @@ static CURL *get_curl_handle(void)
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
-		curl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);
+		curl_easy_setopt(result, CURLOPT_KEYPASSWD, cert_auth.password);
 #if LIBCURL_VERSION_NUM >= 0x070903
 	if (ssl_key != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);
@@ -307,10 +309,12 @@ static CURL *get_curl_handle(void)
 
 static void http_auth_init(const char *url)
 {
-	char *at, *colon, *cp, *slash;
+	const char *at, *colon, *cp, *slash, *host, *proto_end;
+	char *decoded;
+	struct strbuf unique = STRBUF_INIT;
 
-	cp = strstr(url, "://");
-	if (!cp)
+	proto_end = strstr(url, "://");
+	if (!proto_end)
 		return;
 
 	/*
@@ -319,20 +323,31 @@ static void http_auth_init(const char *url)
 	 * "proto://<user>@<host>/...", or just
 	 * "proto://<host>/..."?
 	 */
-	cp += 3;
+	cp = proto_end + 3;
 	at = strchr(cp, '@');
 	colon = strchr(cp, ':');
 	slash = strchrnul(cp, '/');
-	if (!at || slash <= at)
-		return; /* No credentials */
-	if (!colon || at <= colon) {
+
+	if (!at || slash <= at) {
+		/* No credentials, but we may have to ask for some later */
+		host = cp;
+	}
+	else if (!colon || at <= colon) {
 		/* Only username */
-		user_name = url_decode_mem(cp, at - cp);
-		user_pass = NULL;
+		http_auth.username = url_decode_mem(cp, at - cp);
+		host = at + 1;
 	} else {
-		user_name = url_decode_mem(cp, colon - cp);
-		user_pass = url_decode_mem(colon + 1, at - (colon + 1));
+		http_auth.username = url_decode_mem(cp, colon - cp);
+		http_auth.password = url_decode_mem(colon + 1, at - (colon + 1));
+		host = at + 1;
 	}
+
+	strbuf_add(&unique, url, proto_end - url);
+	strbuf_addch(&unique, ':');
+	decoded = url_decode_mem(host, slash - host);
+	strbuf_addstr(&unique, decoded);
+	free(decoded);
+	http_auth.unique = strbuf_detach(&unique, NULL);
 }
 
 static void set_from_env(const char **var, const char *envname)
@@ -456,10 +471,10 @@ void http_cleanup(void)
 		curl_http_proxy = NULL;
 	}
 
-	if (ssl_cert_password != NULL) {
-		memset(ssl_cert_password, 0, strlen(ssl_cert_password));
-		free(ssl_cert_password);
-		ssl_cert_password = NULL;
+	if (cert_auth.password) {
+		memset(cert_auth.password, 0, strlen(cert_auth.password));
+		free(cert_auth.password);
+		cert_auth.password = NULL;
 	}
 	ssl_cert_password_required = 0;
 }
@@ -819,16 +834,11 @@ static int http_request(const char *url, void *result, int target, int options)
 		else if (missing_target(&results))
 			ret = HTTP_MISSING_TARGET;
 		else if (results.http_code == 401) {
-			if (user_name) {
+			if (http_auth.username) {
+				credential_reject(&http_auth, NULL);
 				ret = HTTP_NOAUTH;
 			} else {
-				/*
-				 * git_getpass is needed here because its very likely stdin/stdout are
-				 * pipes to our parent process.  So we instead need to use /dev/tty,
-				 * but that is non-portable.  Using git_getpass() can at least be stubbed
-				 * on other platforms with a different implementation if/when necessary.
-				 */
-				user_name = xstrdup(git_getpass("Username: "));
+				credential_fill(&http_auth, NULL);
 				init_curl_http_auth(slot->curl);
 				ret = HTTP_REAUTH;
 			}
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index ed4db09..af3bc6b 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -66,7 +66,7 @@ test_expect_success 'cloning password-protected repository can fail' '
 
 test_expect_success 'http auth can use user/pass in URL' '
 	>askpass-query &&
-	echo wrong >askpass-reponse &&
+	echo wrong >askpass-response &&
 	git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
 	test_cmp askpass-expect-none askpass-query
 '
-- 
1.7.6.rc1.12.g65e2

[PATCH 08/14] look for credentials in config before prompting

[Jeff King]

When an http request receives a 401, we ask the user for
both a username and password. While it's generally not a
good idea for us to store the password in plaintext, having
to input the username each time is annoying, and can be
easily solved with a config variable.

This patch teaches the credential subsystem to look up items
in the git config file before prompting. Items are indexed
by the "unique" token passed to the credential system.

Signed-off-by: Jeff King <peff@peff.net>
---
 credential.c           |   38 ++++++++++++++++++++++++++++++++++++++
 credential.h           |    1 +
 t/t0300-credentials.sh |   10 ++++++++++
 t/t5550-http-fetch.sh  |    8 ++++++++
 4 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/credential.c b/credential.c
index c403289..3a4104c 100644
--- a/credential.c
+++ b/credential.c
@@ -4,6 +4,43 @@
 #include "string-list.h"
 #include "run-command.h"
 
+static int credential_config_callback(const char *var, const char *value,
+				      void *data)
+{
+	struct credential *c = data;
+
+	if (!value)
+		return 0;
+
+	var = skip_prefix(var, "credential.");
+	if (!var)
+		return 0;
+
+	var = skip_prefix(var, c->unique);
+	if (!var)
+		return 0;
+
+	if (*var != '.')
+		return 0;
+	var++;
+
+	if (!strcmp(var, "username")) {
+		if (!c->username)
+			c->username = xstrdup(value);
+	}
+	else if (!strcmp(var, "password")) {
+		free(c->password);
+		c->password = xstrdup(value);
+	}
+	return 0;
+}
+
+void credential_from_config(struct credential *c)
+{
+	if (c->unique)
+		git_config(credential_config_callback, c);
+}
+
 static char *credential_ask_one(const char *what, const char *desc)
 {
 	struct strbuf prompt = STRBUF_INIT;
@@ -26,6 +63,7 @@ static char *credential_ask_one(const char *what, const char *desc)
 
 int credential_getpass(struct credential *c)
 {
+	credential_from_config(c);
 
 	if (!c->username)
 		c->username = credential_ask_one("Username", c->description);
diff --git a/credential.h b/credential.h
index 383b720..30a0156 100644
--- a/credential.h
+++ b/credential.h
@@ -11,6 +11,7 @@ struct credential {
 struct string_list;
 
 int credential_getpass(struct credential *);
+void credential_from_config(struct credential *);
 
 int credential_fill_gently(struct credential *, const struct string_list *methods);
 void credential_fill(struct credential *, const struct string_list *methods);
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 447e983..68d838c 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -172,4 +172,14 @@ test_expect_success 'internal getpass does not ask for known username' '
 	EOF
 '
 
+test_expect_success 'internal getpass can pull from config' '
+	git config credential.foo.username configured-username
+	check --unique=foo <<-\EOF
+	username=configured-username
+	password=askpass-result
+	--
+	askpass: Password:
+	EOF
+'
+
 test_done
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index af3bc6b..c78baaf 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -85,6 +85,14 @@ test_expect_success 'http auth can request both user and pass' '
 	test_cmp askpass-expect-both askpass-query
 '
 
+test_expect_success 'http auth can pull user from config' '
+	>askpass-query &&
+	echo user@host >askpass-response &&
+	git config --global credential.$HTTPD_PROTO:$HTTPD_DEST.username user@host &&
+	git clone "$HTTPD_URL/auth/repo.git" clone-auth-config &&
+	test_cmp askpass-expect-pass askpass-query
+'
+
 test_expect_success 'fetch changes via http' '
 	echo content >>file &&
 	git commit -a -m two &&
-- 
1.7.6.rc1.12.g65e2

[PATCH 09/14] allow the user to configure credential helpers

[Jeff King]

The functionality for helpers is already there; we just need
to give the users a way to turn it on.

The new functionality is enabled whenever a caller of the
credentials API passes a NULL method list. This will enable
it for all current callers (i.e., the http code).

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/technical/api-credentials.txt |    5 ++-
 config.c                                    |    4 +++
 credential.c                                |   31 +++++++++++++++++++++++---
 credential.h                                |    2 +
 t/t5550-http-fetch.sh                       |   13 +++++++++++
 5 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index 880db92..335a007 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -25,8 +25,9 @@ Data Structures
 	The credential functions take a `string_list` of methods for
 	acquiring credentials. Each string specifies an external
 	helper which will be run, in order, to acquire credentials,
-	until both a username and password have been acquired. A NULL or
-	empty methods list indicates that the internal
+	until both a username and password have been acquired. A NULL
+	parameter means to use the default list (as configured by
+	`credential.helper`); an empty list indicates that the internal
 	`credential_getpass` function should be used.
 
 
diff --git a/config.c b/config.c
index 1fc063b..ee643eb 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,7 @@
 #include "exec_cmd.h"
 #include "strbuf.h"
 #include "quote.h"
+#include "credential.h"
 
 #define MAXNAME (256)
 
@@ -791,6 +792,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
 		return 0;
 	}
 
+	if (!prefixcmp(var, "credential."))
+		return git_default_credential_config(var, value);
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/credential.c b/credential.c
index 3a4104c..744d105 100644
--- a/credential.c
+++ b/credential.c
@@ -4,6 +4,8 @@
 #include "string-list.h"
 #include "run-command.h"
 
+static struct string_list default_methods;
+
 static int credential_config_callback(const char *var, const char *value,
 				      void *data)
 {
@@ -173,15 +175,18 @@ void credential_fill(struct credential *c, const struct string_list *methods)
 {
 	struct strbuf err = STRBUF_INIT;
 
+	if (!methods)
+		methods = &default_methods;
+
 	if (!credential_fill_gently(c, methods))
 		return;
 
 	strbuf_addstr(&err, "unable to get credentials");
 	if (c->description)
 		strbuf_addf(&err, "for '%s'", c->description);
-	if (methods && methods->nr == 1)
+	if (methods->nr == 1)
 		strbuf_addf(&err, "; tried '%s'", methods->items[0].string);
-	else if (methods) {
+	else {
 		int i;
 		strbuf_addstr(&err, "; tried:");
 		for (i = 0; i < methods->nr; i++)
@@ -198,7 +203,10 @@ int credential_fill_gently(struct credential *c,
 	if (c->username && c->password)
 		return 0;
 
-	if (!methods || !methods->nr)
+	if (!methods)
+		methods = &default_methods;
+
+	if (!methods->nr)
 		return credential_getpass(c);
 
 	for (i = 0; i < methods->nr; i++) {
@@ -214,7 +222,10 @@ void credential_reject(struct credential *c, const struct string_list *methods)
 {
 	int i;
 
-	if (methods && c->username) {
+	if (!methods)
+		methods = &default_methods;
+
+	if (c->username) {
 		for (i = 0; i < methods->nr; i++) {
 			/* ignore errors, there's nothing we can do */
 			credential_do(c, methods->items[i].string, "--reject");
@@ -226,3 +237,15 @@ void credential_reject(struct credential *c, const struct string_list *methods)
 	free(c->password);
 	c->password = NULL;
 }
+
+int git_default_credential_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "credential.helper")) {
+		if (!value)
+			return config_error_nonbool(var);
+		string_list_append(&default_methods, xstrdup(value));
+		return 0;
+	}
+
+	return 0;
+}
diff --git a/credential.h b/credential.h
index 30a0156..788ed8e 100644
--- a/credential.h
+++ b/credential.h
@@ -17,4 +17,6 @@ int credential_fill_gently(struct credential *, const struct string_list *method
 void credential_fill(struct credential *, const struct string_list *methods);
 void credential_reject(struct credential *, const struct string_list *methods);
 
+int git_default_credential_config(const char *var, const char *value);
+
 #endif /* CREDENTIAL_H */
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index c78baaf..407e1cb 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -93,6 +93,19 @@ test_expect_success 'http auth can pull user from config' '
 	test_cmp askpass-expect-pass askpass-query
 '
 
+test_expect_success 'http auth respects credential helpers' '
+	cat >credential-helper <<-\EOF &&
+	#!/bin/sh
+	echo username=user@host
+	echo password=user@host
+	EOF
+	chmod +x credential-helper &&
+	git config --global credential.helper "\"$PWD/credential-helper\"" &&
+	>askpass-query &&
+	git clone "$HTTPD_URL/auth/repo.git" clone-auth-helper &&
+	test_cmp askpass-expect-none askpass-query
+'
+
 test_expect_success 'fetch changes via http' '
 	echo content >>file &&
 	git commit -a -m two &&
-- 
1.7.6.rc1.12.g65e2

[PATCH 10/14] http: use hostname in credential description

[Jeff King]

Until now, a request for an http password looked like:

  Username:
  Password:

Now it will look like:

  Username for 'example.com':
  Password for 'example.com':

Signed-off-by: Jeff King <peff@peff.net>
---
This has been requested a few times. I think we could go even further
with:

  Username for 'example.com':
  Password for 'user@example.com':

It's not that big a deal if you just typed the username, obviously, but
if the username came out of the config file, it might be a helpful
reminder.

 http.c                |    7 +++----
 t/t5550-http-fetch.sh |    4 ++--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/http.c b/http.c
index 4c047be..d6b2d78 100644
--- a/http.c
+++ b/http.c
@@ -310,7 +310,6 @@ static CURL *get_curl_handle(void)
 static void http_auth_init(const char *url)
 {
 	const char *at, *colon, *cp, *slash, *host, *proto_end;
-	char *decoded;
 	struct strbuf unique = STRBUF_INIT;
 
 	proto_end = strstr(url, "://");
@@ -342,11 +341,11 @@ static void http_auth_init(const char *url)
 		host = at + 1;
 	}
 
+	http_auth.description = url_decode_mem(host, slash - host);
+
 	strbuf_add(&unique, url, proto_end - url);
 	strbuf_addch(&unique, ':');
-	decoded = url_decode_mem(host, slash - host);
-	strbuf_addstr(&unique, decoded);
-	free(decoded);
+	strbuf_addstr(&unique, http_auth.description);
 	http_auth.unique = strbuf_detach(&unique, NULL);
 }
 
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 407e1cb..b04261c 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -51,8 +51,8 @@ test_expect_success 'setup askpass helpers' '
 	GIT_ASKPASS="$PWD/askpass" &&
 	export GIT_ASKPASS &&
 	>askpass-expect-none &&
-	echo "askpass: Password: " >askpass-expect-pass &&
-	{ echo "askpass: Username: " &&
+	echo "askpass: Password for '\''$HTTPD_DEST'\'': " >askpass-expect-pass &&
+	{ echo "askpass: Username for '\''$HTTPD_DEST'\'': " &&
 	  cat askpass-expect-pass
 	} >askpass-expect-both
 '
-- 
1.7.6.rc1.12.g65e2

Re: [PATCH 10/14] http: use hostname in credential description

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> @@ -342,11 +341,11 @@ static void http_auth_init(const char *url)
>  		host = at + 1;
>  	}
>  
> +	http_auth.description = url_decode_mem(host, slash - host);

Could a hosting site like github host multiple repositories, each of which
the user may interact with under different identity?  Just wondering if it
is sufficient to say "http://example.com/" or it needs to be more
specific, e.g. "http://example.com/p/project.git/".

Re: [PATCH 10/14] http: use hostname in credential description

[Jeff King]

On Wed, Jul 20, 2011 at 03:17:08PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > @@ -342,11 +341,11 @@ static void http_auth_init(const char *url)
> >  		host = at + 1;
> >  	}
> >  
> > +	http_auth.description = url_decode_mem(host, slash - host);
> 
> Could a hosting site like github host multiple repositories, each of which
> the user may interact with under different identity?  Just wondering if it
> is sufficient to say "http://example.com/" or it needs to be more
> specific, e.g. "http://example.com/p/project.git/".

Yes, it's entirely possible. This is a policy decision I had to make
(which I really tried to avoid in this series, but at some point, you
have to make some if you don't want a million config options).

It means that you can get convenient credential handling (whether it's
because you've configured a username, or you're getting it from a
wallet, or caching, or whatever) with:

  git push https://github.com/peff/git.git &&
  git push https://github.com/peff/foo.git

which should hopefully just prompt you once (and a configured username
would have to be configured only once for the host).

And it comes at the cost that there's not a good way to use two
different identities for the same host.

I tried to optimize for the common case (many repos under one identity)
than the uncommon (many identities under one host). But I do wish there
were a better way to say "No, I want to be more specific". In the
current code, it would probably mean adding a config option to put the
repo path into the unique token.

If we go in the direction of dumping a list of key/value attributes to
the credential helper, then it becomes up to the helper to make that
policy decision. Which is extremely flexible (e.g., you could use the
same identity for all repos on host1, but per-repo identities on host2).
But configuring it becomes a matter of actually writing your own helper,
which is not something most people would (or should have to) do.

-Peff

Re: [PATCH 10/14] http: use hostname in credential description

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> It means that you can get convenient credential handling (whether it's
> because you've configured a username, or you're getting it from a
> wallet, or caching, or whatever) with:
>
>   git push https://github.com/peff/git.git &&
>   git push https://github.com/peff/foo.git
>
> which should hopefully just prompt you once (and a configured username
> would have to be configured only once for the host).
>
> And it comes at the cost that there's not a good way to use two
> different identities for the same host.

Yes, both "just prompt you once" and "would have to be configured only
once" cut both ways, and that is mildly disturbing to my taste.

If we annotate the remote in .git/config perhaps like this:

	[remote "foo"]
        	url = https://github.com/peff/foo.git
                auth_context = "foo project at github"
		...
	[remote "bar"]
        	url = https://github.com/peff/bar.git
                auth_context = "bar project at github"

and have "git push foo" pass the auth_context to the credential backend,
which can notice the two projects are in different context and cache two
identities under different contexts, would it be a good workaround for the
issue?  Then, a remote that does not have auth_context configured would
use "https:github.com" that is machine-generated (in http.c in your code),
but that can easily be overridden if/as necessary.

> I tried to optimize for the common case (many repos under one identity)
> than the uncommon (many identities under one host).

As I am not convinced if this statement is true.

Re: [PATCH 10/14] http: use hostname in credential description

[Jeff King]

On Fri, Jul 22, 2011 at 03:01:53PM -0700, Junio C Hamano wrote:

> Yes, both "just prompt you once" and "would have to be configured only
> once" cut both ways, and that is mildly disturbing to my taste.
> 
> If we annotate the remote in .git/config perhaps like this:
> 
> 	[remote "foo"]
>         	url = https://github.com/peff/foo.git
>                 auth_context = "foo project at github"
> 		...
> 	[remote "bar"]
>         	url = https://github.com/peff/bar.git
>                 auth_context = "bar project at github"
> 
> and have "git push foo" pass the auth_context to the credential backend,
> which can notice the two projects are in different context and cache two
> identities under different contexts, would it be a good workaround for the
> issue?  Then, a remote that does not have auth_context configured would
> use "https:github.com" that is machine-generated (in http.c in your code),
> but that can easily be overridden if/as necessary.

That has the minor downside of not handling one-off URLs.

Actually, though, I think even with the current code, you can do better
than that. The "username" is an implicit part of the context, as well. A
poorly-written helper might ignore it, of course, but you can already
say:

  [remote "foo"]
    url = https://peff@github.com/peff/foo.git
    ...
  [remote "bar"]
    url = https://otheruser@github.com/otheruser/foo.git"
    ...

The "cache" helper will match the username when looking up only a
password. The "store" helper is less exacting, and uses only the
opaque context. Mostly because it uses the config format as a backing
store, which makes pairing usernames and passwords more difficult (but
not impossible; it can be worked around by saving some context between
invocations of the config callback). So that's a good reason to improve
"store".

As a bonus, this technique actually works to access the exact same repo
as two different identities (whereas just including the path in the
context does not). E.g.:

  [remote "repo-as-me"]
    url = https://me@example.com/repo.git
    ...
  [remote "repo-as-other-role"]
    url = https://role@example.com/repo.git
    ...

I expect those cases to be even less common, of course, but it's nice
that it's straightforward to support them.

> > I tried to optimize for the common case (many repos under one identity)
> > than the uncommon (many identities under one host).
> 
> As I am not convinced if this statement is true.

I admit I don't have any data beyond my own experiences. GitHub tends
towards the concept of a single identity, and it has some group
management. I don't know about other sites, though. Do you have any
specific examples in mind?

-Peff

Re: [PATCH 10/14] http: use hostname in credential description

[Ted Zlatanov]

On Fri, 22 Jul 2011 16:13:38 -0600 Jeff King <peff@peff.net> wrote: 

JK> I admit I don't have any data beyond my own experiences. GitHub tends
JK> towards the concept of a single identity, and it has some group
JK> management. I don't know about other sites, though. Do you have any
JK> specific examples in mind?

I have not yet needed multiple user names on a single Git server, so I
think it's OK to make that case less convenient to favor the more common
single user name case.

Ted

Re: [PATCH 10/14] http: use hostname in credential description

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I expect those cases to be even less common, of course, but it's nice
> that it's straightforward to support them.
>
>> > I tried to optimize for the common case (many repos under one identity)
>> > than the uncommon (many identities under one host).
>> 
>> As I am not convinced if this statement is true.
>
> I admit I don't have any data beyond my own experiences. GitHub tends
> towards the concept of a single identity, and it has some group
> management. I don't know about other sites, though. Do you have any
> specific examples in mind?

I think I heard Shawn talk about Gerrit installations where multiple
instances that serve separate userbases are behind a shared Apache
front-end that would be problematic with this limitation.

Re: [PATCH 10/14] http: use hostname in credential description

[Ted Zlatanov]

I see some info in "What's Cooking" about this patch but it's unclear to
me whether the hostname issue (where it's hard to have multiple
identities on a single server, which I think is all right) is blocking
the inclusion of the patch into the next release, or if it will be
included eventually if no one complains about that issue, or something
else...

Thanks
Ted

Re: [PATCH 10/14] http: use hostname in credential description

[Jeff King]

On Fri, Aug 19, 2011 at 07:01:21AM -0500, Ted Zlatanov wrote:

> I see some info in "What's Cooking" about this patch but it's unclear to
> me whether the hostname issue (where it's hard to have multiple
> identities on a single server, which I think is all right) is blocking
> the inclusion of the patch into the next release, or if it will be
> included eventually if no one complains about that issue, or something
> else...

Junio and I discussed it a bit in another thread. I think the ability to
use "user@hostname" to disambiguate means the problem is dealt with at a
high level. And the "cache" helper handles that just fine. But the
"store" helper will conflate two entries for the same host. I'll see if
I can work on a patch for that.

It looks like Junio is planning to hold the series off until 1.7.8. Have
you been working on a Secrets API helper? If so, I'd love to get
feedback on how well the interface is serving your needs.

-Peff

Re: [PATCH 10/14] http: use hostname in credential description

[Ted Zlatanov]

On Thu, 25 Aug 2011 16:23:26 -0400 Jeff King <peff@peff.net> wrote: 

JK> On Fri, Aug 19, 2011 at 07:01:21AM -0500, Ted Zlatanov wrote:
>> I see some info in "What's Cooking" about this patch but it's unclear to
>> me whether the hostname issue (where it's hard to have multiple
>> identities on a single server, which I think is all right) is blocking
>> the inclusion of the patch into the next release, or if it will be
>> included eventually if no one complains about that issue, or something
>> else...

JK> Junio and I discussed it a bit in another thread. I think the ability to
JK> use "user@hostname" to disambiguate means the problem is dealt with at a
JK> high level. And the "cache" helper handles that just fine. But the
JK> "store" helper will conflate two entries for the same host. I'll see if
JK> I can work on a patch for that.

Cool, I hope this is the last wrinkle on the bundled helpers.

JK> It looks like Junio is planning to hold the series off until 1.7.8. Have
JK> you been working on a Secrets API helper? If so, I'd love to get
JK> feedback on how well the interface is serving your needs.

Work and Real Life have interfered with coding for fun, but I will have
time next week to try writing a few helpers.  This is high priority for
me because of various projects that require it; sorry for taking so long
to start using it.

Ted

[PATCH 11/14] docs: end-user documentation for the credential subsystem

[Jeff King]

The credential API and helper format is already defined in
technical/api-credentials.txt.  This presents the end-user
view.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/Makefile           |    1 +
 Documentation/config.txt         |   11 +++
 Documentation/gitcredentials.txt |  139 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 151 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/gitcredentials.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 36989b7..88e7f47 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -7,6 +7,7 @@ MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt githooks.txt \
 MAN7_TXT=gitcli.txt gittutorial.txt gittutorial-2.txt \
 	gitcvs-migration.txt gitcore-tutorial.txt gitglossary.txt \
 	gitdiffcore.txt gitrevisions.txt gitworkflows.txt
+MAN7_TXT += gitcredentials.txt
 
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
 MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT))
diff --git a/Documentation/config.txt b/Documentation/config.txt
index b56959b..13c13f4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -823,6 +823,17 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
+credential.helper::
+	Specify an external helper to be called when a username or
+	password credential is needed; the helper may consult external
+	storage to avoid prompting the user for the credentials. See
+	linkgit:gitcredentials[7] for details.
+
+credential.<context>.username::
+	Specify a default username to be used instead of prompting the
+	user when getting credentials for `<context>`. See
+	linkgit:gitcredentials[7] for details.
+
 include::diff-config.txt[]
 
 difftool.<tool>.path::
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
new file mode 100644
index 0000000..74136ee
--- /dev/null
+++ b/Documentation/gitcredentials.txt
@@ -0,0 +1,139 @@
+gitcredentials(7)
+=================
+
+NAME
+----
+gitcredentials - providing usernames and passwords to git
+
+SYNOPSIS
+--------
+------------------
+git config credential.https:example.com.username myusername
+git config credential.helper "$helper $options"
+------------------
+
+DESCRIPTION
+-----------
+
+Git will sometimes need credentials from the user in order to perform
+operations; for example, it may need to ask for a username and password
+in order to access a remote repository over HTTP. This manual describes
+the mechanisms git uses to request these credentials, as well as some
+features to avoid inputting these credentials repeatedly.
+
+REQUESTING CREDENTIALS
+----------------------
+
+Without any credential helpers defined, git will try the following
+strategies to ask the user for usernames and passwords:
+
+1. If the `GIT_ASKPASS` environment variable is set, the program
+   specified by the variable is invoked. A suitable prompt is provided
+   to the program on the command line, and the user's input is read
+   from its standard output.
+
+2. Otherwise, if the `core.askpass` configuration variable is set, its
+   value is used as above.
+
+3. Otherwise, if the `SSH_ASKPASS` environment variable is set, its
+   value is used as above.
+
+4. Otherwise, the user is prompted on the terminal.
+
+AVOIDING REPETITION
+-------------------
+
+It can be cumbersome to input the same credentials over and over.  Git
+provides two methods to reduce this annoyance:
+
+1. Static configuration of usernames for a given authentication context.
+
+2. Credential helpers to cache or store passwords, or to interact with
+   a system password wallet or keychain.
+
+STATIC CONFIGURATION
+--------------------
+
+Git can look for credential information in your git config files. Note
+that it only makes sense to store usernames, not passwords, as git
+config files are not encrypted or usually even protected by filesystem
+permissions.
+
+For a given credential request, git uses a unique token to represent the
+context of a request. For example, a request to
+`https://example.com/repo.git` would have the context
+`https:example.com`. See `CONTEXT TOKENS` below for a full list.
+
+To statically configure a username, set the configuration variable
+`credential.$token.username`. For example, in this instance git will
+prompt only for the password, not the username:
+
+--------------------------------------------------------------
+$ git config --global credential.https:example.com.username me
+$ git push https://example.com/repo.git
+Password:
+--------------------------------------------------------------
+
+CREDENTIAL HELPERS
+------------------
+
+Credential helpers are external programs from which git can request
+usernames and passwords.
+
+To use a helper, you must first select one to use.  Git does not yet
+include any credential helpers, but you may have third-party helpers
+installed; search for `credential-*` in the output of `git help -a`, and
+consult the documentation of individual helpers.  Once you have selected
+a helper, you can tell git to use it by putting its name into the
+credential.helper variable.
+
+1. Find a helper.
++
+-------------------------------------------
+$ git help -a | grep credential-
+credential-foo
+-------------------------------------------
+
+2. Read its description.
++
+-------------------------------------------
+$ git help credential-foo
+-------------------------------------------
+
+3. Tell git to use it.
++
+-------------------------------------------
+$ git config --global credential.helper foo
+-------------------------------------------
+
+If there are multiple instances of the `credential.helper` configuration
+variable, each helper will be tried in turn, and may provide a username,
+password, or nothing. Once git has acquired both a username and a
+password, no more helpers will be tried.
+
+CUSTOM HELPERS
+--------------
+
+You can write your own custom helpers to interface with any system in
+which you keep credentials. See the documentation for git's
+link:technical/api-credentials.html[credentials API] for details.
+
+CONTEXT TOKENS
+--------------
+
+The full set of unique context tokens provided by git to credential
+helpers is:
+
+`$protocol:$hostname`::
+
+	A network request to a specific host. `$protocol` is
+	either `http` or `https`, and `$hostname` is the hostname
+	provided to git (which may not be fully qualified).
+
+`cert:$filename`::
+
+	A password to decrypt a certificate on disk.
+
+GIT
+---
+Part of the linkgit:git[1] suite
-- 
1.7.6.rc1.12.g65e2

Re: [PATCH 11/14] docs: end-user documentation for the credential subsystem

[Junio C Hamano]

I've read up to this one, and felt that you must have had a lot of fun
while working on these ;-)  In general all of them looked good.

[PATCH 12/14] credentials: add "cache" helper

[Jeff King]

If you access repositories over smart-http using http
authentication, then it can be annoying to have git ask you
for your password repeatedly. We cache credentials in
memory, of course, but git is composed of many small
programs. Having to input your password for each one can be
frustrating.

This patch introduces a credential helper that will cache
passwords in memory for a short period of time.

Signed-off-by: Jeff King <peff@peff.net>
---
This uses unix sockets (protected by filesystem permissions) to talk to
the daemon. I have no idea if emulation is available on Windows, or if
there's some equivalent we can use.

 .gitignore                                     |    2 +
 Documentation/git-credential-cache--daemon.txt |   26 +++
 Documentation/git-credential-cache.txt         |   84 ++++++++
 Documentation/gitcredentials.txt               |   17 +-
 Makefile                                       |    3 +
 credential-cache--daemon.c                     |  268 ++++++++++++++++++++++++
 credential-cache.c                             |  163 ++++++++++++++
 git-compat-util.h                              |    1 +
 t/t0300-credentials.sh                         |   91 ++++++++
 unix-socket.c                                  |   58 +++++
 unix-socket.h                                  |    7 +
 11 files changed, 715 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/git-credential-cache--daemon.txt
 create mode 100644 Documentation/git-credential-cache.txt
 create mode 100644 credential-cache--daemon.c
 create mode 100644 credential-cache.c
 create mode 100644 unix-socket.c
 create mode 100644 unix-socket.h

diff --git a/.gitignore b/.gitignore
index 7d2fefc..a6b0bd4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,8 @@
 /git-commit-tree
 /git-config
 /git-count-objects
+/git-credential-cache
+/git-credential-cache--daemon
 /git-cvsexportcommit
 /git-cvsimport
 /git-cvsserver
diff --git a/Documentation/git-credential-cache--daemon.txt b/Documentation/git-credential-cache--daemon.txt
new file mode 100644
index 0000000..11edc5a
--- /dev/null
+++ b/Documentation/git-credential-cache--daemon.txt
@@ -0,0 +1,26 @@
+git-credential-cache--daemon(1)
+===============================
+
+NAME
+----
+git-credential-cache--daemon - temporarily store user credentials in memory
+
+SYNOPSIS
+--------
+[verse]
+git credential-cache--daemon <socket>
+
+DESCRIPTION
+-----------
+
+NOTE: You probably don't want to invoke this command yourself; it is
+started automatically when you use linkgit:git-credential-cache[1].
+
+This command listens on the Unix domain socket specified by `<socket>`
+for `git-credential-cache` clients. Clients may store and retrieve
+credentials. Each credential is held for a timeout specified by the
+client; once no credentials are held, the daemon exits.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/git-credential-cache.txt b/Documentation/git-credential-cache.txt
new file mode 100644
index 0000000..563fdae
--- /dev/null
+++ b/Documentation/git-credential-cache.txt
@@ -0,0 +1,84 @@
+git-credential-cache(1)
+=======================
+
+NAME
+----
+git-credential-cache - helper to temporarily store passwords in memory
+
+SYNOPSIS
+--------
+-----------------------------
+git config credential.helper 'cache [options]'
+-----------------------------
+
+DESCRIPTION
+-----------
+
+This command requests credentials from the user and caches them in
+memory for use by future git programs. The stored credentials never
+touch the disk, and are forgotten after a configurable timeout.  The
+cache is accessible over a Unix domain socket, restricted to the current
+user by filesystem permissions.
+
+You probably don't want to invoke this command directly; it is meant to
+be used as a credential helper by other parts of git. See
+linkgit:gitcredentials[7] or `EXAMPLES` below.
+
+OPTIONS
+-------
+
+--timeout::
+
+	Number of seconds to cache credentials (default: 900).
+
+--socket <path>::
+
+	Use `<path>` to contact a running cache daemon (or start a new
+	cache daemon if one is not started). Defaults to
+	`~/.git-credential-cache/socket`. If your home directory is on a
+	network-mounted filesystem, you may need to change this to a
+	local filesystem.
+
+--chain <helper>::
+
+	Specify an external helper to use for retrieving credentials
+	from the user, instead of the default method. The resulting
+	credentials are then cached as normal. This option can be
+	given multiple times; each chained helper will be tried until
+	credentials are received.
+
+--exit::
+
+	Tell a running daemon to exit, forgetting all cached
+	credentials.
+
+Git may provide other options to the program when it is called as a
+credential helper; see linkgit:gitcredentials[7].
+
+EXAMPLES
+--------
+
+The point of this helper is to reduce the number of times you must type
+your username or password. For example:
+
+------------------------------------
+$ git config credential.helper cache
+$ git push http://example.com/repo.git
+Username: <type your username>
+Password: <type your password>
+
+[work for 5 more minutes]
+$ git push http://example.com/repo.git
+[your credentials are used automatically]
+------------------------------------
+
+You can provide options via the credential.helper configuration
+variable (this example drops the cache time to 5 minutes):
+
+------------------------------------
+$ git config credential.helper 'cache --timeout=300'
+------------------------------------
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 74136ee..bd1a3b6 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -80,11 +80,18 @@ CREDENTIAL HELPERS
 Credential helpers are external programs from which git can request
 usernames and passwords.
 
-To use a helper, you must first select one to use.  Git does not yet
-include any credential helpers, but you may have third-party helpers
-installed; search for `credential-*` in the output of `git help -a`, and
-consult the documentation of individual helpers.  Once you have selected
-a helper, you can tell git to use it by putting its name into the
+To use a helper, you must first select one to use. Git currently
+includes the following helpers:
+
+cache::
+
+	Cache credentials in memory for a short period of time. See
+	linkgit:git-credential-cache[1] for details.
+
+You may may also have third-party helpers installed; search for
+`credential-*` in the output of `git help -a`, and consult the
+documentation of individual helpers.  Once you have selected a helper,
+you can tell git to use it by putting its name into the
 credential.helper variable.
 
 1. Find a helper.
diff --git a/Makefile b/Makefile
index 16515bf..2ecfd90 100644
--- a/Makefile
+++ b/Makefile
@@ -417,6 +417,8 @@ PROGRAM_OBJS += show-index.o
 PROGRAM_OBJS += upload-pack.o
 PROGRAM_OBJS += http-backend.o
 PROGRAM_OBJS += sh-i18n--envsubst.o
+PROGRAM_OBJS += credential-cache.o
+PROGRAM_OBJS += credential-cache--daemon.o
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
@@ -677,6 +679,7 @@ LIB_OBJS += transport-helper.o
 LIB_OBJS += tree-diff.o
 LIB_OBJS += tree.o
 LIB_OBJS += tree-walk.o
+LIB_OBJS += unix-socket.o
 LIB_OBJS += unpack-trees.o
 LIB_OBJS += url.o
 LIB_OBJS += usage.o
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
new file mode 100644
index 0000000..f520347
--- /dev/null
+++ b/credential-cache--daemon.c
@@ -0,0 +1,268 @@
+#include "cache.h"
+#include "credential.h"
+#include "unix-socket.h"
+
+struct credential_cache_entry {
+	struct credential item;
+	unsigned long expiration;
+};
+static struct credential_cache_entry *entries;
+static int entries_nr;
+static int entries_alloc;
+
+static void cache_credential(const struct credential *c, int timeout)
+{
+	struct credential_cache_entry *e;
+
+	ALLOC_GROW(entries, entries_nr + 1, entries_alloc);
+	e = &entries[entries_nr++];
+
+	memcpy(&e->item, c, sizeof(*c));
+	e->expiration = time(NULL) + timeout;
+}
+
+static struct credential_cache_entry *lookup_credential(const struct credential *c)
+{
+	int i;
+	for (i = 0; i < entries_nr; i++) {
+		struct credential *e = &entries[i].item;
+
+		/* We must either both have the same unique token,
+		 * or we must not be using unique tokens at all. */
+		if (e->unique) {
+			if (!c->unique || strcmp(e->unique, c->unique))
+				continue;
+		}
+		else if (c->unique)
+			continue;
+
+		/* If we have a username, it must match. Otherwise,
+		 * we will fill in the username. */
+		if (c->username && strcmp(e->username, c->username))
+			continue;
+
+		return &entries[i];
+	}
+	return NULL;
+}
+
+static void remove_credential(const struct credential *c)
+{
+	struct credential_cache_entry *e;
+
+	e = lookup_credential(c);
+	if (e)
+		e->expiration = 0;
+}
+
+static int check_expirations(void)
+{
+	int i = 0;
+	unsigned long now = time(NULL);
+	unsigned long next = (unsigned long)-1;
+
+	while (i < entries_nr) {
+		if (entries[i].expiration <= now) {
+			entries_nr--;
+			if (!entries_nr)
+				return 0;
+			free(entries[i].item.description);
+			free(entries[i].item.unique);
+			free(entries[i].item.username);
+			free(entries[i].item.password);
+			memcpy(&entries[i], &entries[entries_nr], sizeof(*entries));
+		}
+		else {
+			if (entries[i].expiration < next)
+				next = entries[i].expiration;
+			i++;
+		}
+	}
+
+	return next - now;
+}
+
+static int read_credential_request(FILE *fh, struct credential *c,
+				   char **action, int *timeout) {
+	struct strbuf item = STRBUF_INIT;
+
+	while (strbuf_getline(&item, fh, '\0') != EOF) {
+		char *key = item.buf;
+		char *value = strchr(key, '=');
+
+		if (!value) {
+			warning("cache client sent bogus input: %s", key);
+			strbuf_release(&item);
+			return -1;
+		}
+		*value++ = '\0';
+
+		if (!strcmp(key, "action"))
+			*action = xstrdup(value);
+		else if (!strcmp(key, "unique"))
+			c->unique = xstrdup(value);
+		else if (!strcmp(key, "username"))
+			c->username = xstrdup(value);
+		else if (!strcmp(key, "password"))
+			c->password = xstrdup(value);
+		else if (!strcmp(key, "timeout"))
+			*timeout = atoi(value);
+		else {
+			warning("cache client sent bogus key: %s", key);
+			strbuf_release(&item);
+			return -1;
+		}
+	}
+	strbuf_release(&item);
+	return 0;
+}
+
+static void serve_one_client(FILE *in, FILE *out)
+{
+	struct credential c = { NULL };
+	int timeout = -1;
+	char *action = NULL;
+
+	if (read_credential_request(in, &c, &action, &timeout) < 0)
+		return;
+
+	if (!action) {
+		warning("cache client didn't specify an action");
+		return;
+	}
+
+	if (!strcmp(action, "exit"))
+		exit(0);
+
+	if (!strcmp(action, "get")) {
+		struct credential_cache_entry *e = lookup_credential(&c);
+		if (e) {
+			fprintf(out, "username=%s\n", e->item.username);
+			fprintf(out, "password=%s\n", e->item.password);
+		}
+		return;
+	}
+
+	if (!strcmp(action, "erase")) {
+		remove_credential(&c);
+		return;
+	}
+
+	if (!strcmp(action, "store")) {
+		if (timeout < 0) {
+			warning("cache client didn't specify a timeout");
+			return;
+		}
+
+		remove_credential(&c);
+		cache_credential(&c, timeout);
+		return;
+	}
+
+	warning("cache client sent unknown action: %s", action);
+	return;
+}
+
+static int serve_cache_loop(int fd)
+{
+	struct pollfd pfd;
+	unsigned long wakeup;
+
+	wakeup = check_expirations();
+	if (!wakeup)
+		return 0;
+
+	pfd.fd = fd;
+	pfd.events = POLLIN;
+	if (poll(&pfd, 1, 1000 * wakeup) < 0) {
+		if (errno != EINTR)
+			die_errno("poll failed");
+		return 1;
+	}
+
+	if (pfd.revents & POLLIN) {
+		int client, client2;
+		FILE *in, *out;
+
+		client = accept(fd, NULL, NULL);
+		if (client < 0) {
+			warning("accept failed: %s", strerror(errno));
+			return 1;
+		}
+		client2 = dup(client);
+		if (client2 < 0) {
+			warning("dup failed: %s", strerror(errno));
+			close(client);
+			return 1;
+		}
+
+		in = xfdopen(client, "r");
+		out = xfdopen(client2, "w");
+		serve_one_client(in, out);
+		fclose(in);
+		fclose(out);
+	}
+	return 1;
+}
+
+static void serve_cache(const char *socket_path)
+{
+	int fd;
+
+	fd = unix_stream_listen(socket_path);
+	if (fd < 0)
+		die_errno("unable to bind to '%s'", socket_path);
+
+	printf("ok\n");
+	fclose(stdout);
+
+	while (serve_cache_loop(fd))
+		; /* nothing */
+
+	close(fd);
+	unlink(socket_path);
+}
+
+static const char permissions_advice[] =
+"The permissions on your socket directory are too loose; other\n"
+"users may be able to read your cached credentials. Consider running:\n"
+"\n"
+"	chmod 0700 %s";
+static void check_socket_directory(const char *path)
+{
+	struct stat st;
+	char *path_copy = xstrdup(path);
+	char *dir = dirname(path_copy);
+
+	if (!stat(dir, &st)) {
+		if (st.st_mode & 077)
+			die(permissions_advice, dir);
+		free(path_copy);
+		return;
+	}
+
+	/*
+	 * We must be sure to create the directory with the correct mode,
+	 * not just chmod it after the fact; otherwise, there is a race
+	 * condition in which somebody can chdir to it, sleep, then try to open
+	 * our protected socket.
+	 */
+	if (safe_create_leading_directories_const(dir) < 0)
+		die_errno("unable to create directories for '%s'", dir);
+	if (mkdir(dir, 0700) < 0)
+		die_errno("unable to mkdir '%s'", dir);
+	free(path_copy);
+}
+
+int main(int argc, const char **argv)
+{
+	const char *socket_path = argv[1];
+
+	if (!socket_path)
+		die("usage: git-credential-cache--daemon <socket_path>");
+	check_socket_directory(socket_path);
+
+	serve_cache(socket_path);
+
+	return 0;
+}
diff --git a/credential-cache.c b/credential-cache.c
new file mode 100644
index 0000000..f495043
--- /dev/null
+++ b/credential-cache.c
@@ -0,0 +1,163 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "parse-options.h"
+#include "unix-socket.h"
+#include "run-command.h"
+
+static int send_request(const char *socket, const struct strbuf *out)
+{
+	int got_data = 0;
+	int fd = unix_stream_connect(socket);
+
+	if (fd < 0)
+		return -1;
+
+	if (write_in_full(fd, out->buf, out->len) < 0)
+		die_errno("unable to write to cache daemon");
+	shutdown(fd, SHUT_WR);
+
+	while (1) {
+		char in[1024];
+		int r;
+
+		r = read_in_full(fd, in, sizeof(in));
+		if (r == 0)
+			break;
+		if (r < 0)
+			die_errno("read error from cache daemon");
+		write_or_die(1, in, r);
+		got_data = 1;
+	}
+	return got_data;
+}
+
+static void out_str(struct strbuf *out, const char *key, const char *value)
+{
+	if (!value)
+		return;
+	strbuf_addf(out, "%s=%s", key, value);
+	strbuf_addch(out, '\0');
+}
+
+static void out_int(struct strbuf *out, const char *key, int value)
+{
+	strbuf_addf(out, "%s=%d", key, value);
+	strbuf_addch(out, '\0');
+}
+
+static int do_cache(const char *socket, const char *action,
+		    const struct credential *c, int timeout)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	out_str(&buf, "action", action);
+	if (c) {
+		out_str(&buf, "unique", c->unique);
+		out_str(&buf, "username", c->username);
+		out_str(&buf, "password", c->password);
+	}
+	if (timeout > 0)
+		out_int(&buf, "timeout", timeout);
+
+	ret = send_request(socket, &buf);
+
+	strbuf_release(&buf);
+	return ret;
+}
+
+static void spawn_daemon(const char *socket)
+{
+	struct child_process daemon;
+	const char *argv[] = { NULL, NULL, NULL };
+	char buf[128];
+	int r;
+
+	memset(&daemon, 0, sizeof(daemon));
+	argv[0] = "git-credential-cache--daemon";
+	argv[1] = socket;
+	daemon.argv = argv;
+	daemon.no_stdin = 1;
+	daemon.out = -1;
+
+	if (start_command(&daemon))
+		die_errno("unable to start cache daemon");
+	r = read_in_full(daemon.out, buf, sizeof(buf));
+	if (r < 0)
+		die_errno("unable to read result code from cache daemon");
+	if (r != 3 || memcmp(buf, "ok\n", 3))
+		die("cache daemon did not start: %.*s", r, buf);
+	close(daemon.out);
+}
+
+int main(int argc, const char **argv)
+{
+	struct credential c = { NULL };
+	char *socket_path = NULL;
+	int timeout = 900;
+	struct string_list chain = STRING_LIST_INIT_NODUP;
+	int exit_mode = 0;
+	int reject_mode = 0;
+	const char * const usage[] = {
+		"git credential-cache [options]",
+		NULL
+	};
+	struct option options[] = {
+		OPT_BOOLEAN(0, "exit", &exit_mode,
+			    "tell a running daemon to exit"),
+		OPT_BOOLEAN(0, "reject", &reject_mode,
+			    "reject a cached credential"),
+		OPT_INTEGER(0, "timeout", &timeout,
+			    "number of seconds to cache credentials"),
+		OPT_STRING(0, "socket", &socket_path, "path",
+			   "path of cache-daemon socket"),
+		OPT_STRING_LIST(0, "chain", &chain, "helper",
+				"use <helper> to get non-cached credentials"),
+		OPT_STRING(0, "username", &c.username, "name",
+			   "an existing username"),
+		OPT_STRING(0, "description", &c.description, "desc",
+			   "human-readable description of the credential"),
+		OPT_STRING(0, "unique", &c.unique, "token",
+			   "a unique context for the credential"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, NULL, options, usage, 0);
+	if (argc)
+		usage_with_options(usage, options);
+	/* credential_reject wants to free() these */
+	if (c.username)
+		c.username = xstrdup(c.username);
+	if (c.password)
+		c.password = xstrdup(c.password);
+
+	if (!socket_path)
+		socket_path = expand_user_path("~/.git-credential-cache/socket");
+	if (!socket_path)
+		die("unable to find a suitable socket path; use --socket");
+
+	if (exit_mode) {
+		do_cache(socket_path, "exit", NULL, -1);
+		return 0;
+	}
+
+	if (reject_mode) {
+		do_cache(socket_path, "erase", &c, -1);
+		credential_reject(&c, &chain);
+		return 0;
+	}
+
+	if (do_cache(socket_path, "get", &c, -1) > 0)
+		return 0;
+
+	credential_fill(&c, &chain);
+	printf("username=%s\n", c.username);
+	printf("password=%s\n", c.password);
+
+	if (do_cache(socket_path, "store", &c, timeout) < 0) {
+		spawn_daemon(socket_path);
+		do_cache(socket_path, "store", &c, timeout);
+	}
+	return 0;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index a75530d..8cced82 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -130,6 +130,7 @@
 #include <arpa/inet.h>
 #include <netdb.h>
 #include <pwd.h>
+#include <sys/un.h>
 #ifndef NO_INTTYPES_H
 #include <inttypes.h>
 #else
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 68d838c..994a0aa 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -182,4 +182,95 @@ test_expect_success 'internal getpass can pull from config' '
 	EOF
 '
 
+test_expect_success 'credential-cache caches password' '
+	test_when_finished "git credential-cache --exit" &&
+	check --unique=host cache <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --unique=host cache <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	EOF
+'
+
+test_expect_success 'credential-cache requires matching unique token' '
+	test_when_finished "git credential-cache --exit" &&
+	check --unique=host cache <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --unique=host2 cache <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
+test_expect_success 'credential-cache requires matching usernames' '
+	test_when_finished "git credential-cache --exit" &&
+	check --unique=host cache <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --unique=host --username=other cache <<-\EOF
+	username=other
+	password=askpass-result
+	--
+	askpass: Password:
+	EOF
+'
+
+test_expect_success 'credential-cache times out' '
+	test_when_finished "git credential-cache --exit || true" &&
+	check --unique=host "cache --timeout=1" <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	sleep 2 &&
+	check --unique=host cache <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
+test_expect_success 'credential-cache removes rejected credentials' '
+	test_when_finished "git credential-cache --exit || true" &&
+	check --unique=host cache <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --reject --unique=host --username=askpass-result cache <<-\EOF &&
+	--
+	EOF
+	check --unique=host cache <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
 test_done
diff --git a/unix-socket.c b/unix-socket.c
new file mode 100644
index 0000000..cf9890f
--- /dev/null
+++ b/unix-socket.c
@@ -0,0 +1,58 @@
+#include "cache.h"
+#include "unix-socket.h"
+
+static int unix_stream_socket(void)
+{
+	int fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	if (fd < 0)
+		die_errno("unable to create socket");
+	return fd;
+}
+
+static void unix_sockaddr_init(struct sockaddr_un *sa, const char *path)
+{
+	int size = strlen(path) + 1;
+	if (size > sizeof(sa->sun_path))
+		die("socket path is too long to fit in sockaddr");
+	memset(sa, 0, sizeof(*sa));
+	sa->sun_family = AF_UNIX;
+	memcpy(sa->sun_path, path, size);
+}
+
+int unix_stream_connect(const char *path)
+{
+	int fd;
+	struct sockaddr_un sa;
+
+	unix_sockaddr_init(&sa, path);
+	fd = unix_stream_socket();
+	if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+		close(fd);
+		return -1;
+	}
+	return fd;
+}
+
+int unix_stream_listen(const char *path)
+{
+	int fd;
+	struct sockaddr_un sa;
+
+	unix_sockaddr_init(&sa, path);
+	fd = unix_stream_socket();
+
+	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+		unlink(path);
+		if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+			close(fd);
+			return -1;
+		}
+	}
+
+	if (listen(fd, 5) < 0) {
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+}
diff --git a/unix-socket.h b/unix-socket.h
new file mode 100644
index 0000000..e271aee
--- /dev/null
+++ b/unix-socket.h
@@ -0,0 +1,7 @@
+#ifndef UNIX_SOCKET_H
+#define UNIX_SOCKET_H
+
+int unix_stream_connect(const char *path);
+int unix_stream_listen(const char *path);
+
+#endif /* UNIX_SOCKET_H */
-- 
1.7.6.rc1.12.g65e2

[PATCH 13/14] credentials: add "store" helper

[Jeff King]

This is like "cache", except that we actually put the
credentials on disk. This can be terribly insecure, of
course, but we do what we can to protect them by filesystem
permissions, and we warn the user in the documentation.

This is not unlike using .netrc to store entries, but it's a
little more user-friendly. Instead of putting credentials in
place ahead of time, we transparently store them after
prompting the user for them once.

Signed-off-by: Jeff King <peff@peff.net>
---
This reuses the gitconfig format for storage. Which is really kind of
limiting, because I want an extra layer of hierarchy:

  credential.$context.$username.password

So as it is now, it just looks up the username and password for a
context. But in theory, it should be able to handle multiple usernames
for a given context, and correctly look up passwords for
user1@example.com versus user2@example.com. In practice, I doubt it
really matters, though.

 .gitignore                             |    1 +
 Documentation/git-credential-store.txt |   69 +++++++++++++++++++++++++
 Documentation/gitcredentials.txt       |    5 ++
 Makefile                               |    1 +
 credential-store.c                     |   87 ++++++++++++++++++++++++++++++++
 t/t0300-credentials.sh                 |   55 ++++++++++++++++++++
 6 files changed, 218 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-credential-store.txt
 create mode 100644 credential-store.c

diff --git a/.gitignore b/.gitignore
index a6b0bd4..2b7a3f9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@
 /git-count-objects
 /git-credential-cache
 /git-credential-cache--daemon
+/git-credential-store
 /git-cvsexportcommit
 /git-cvsimport
 /git-cvsserver
diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
new file mode 100644
index 0000000..9fc0f76
--- /dev/null
+++ b/Documentation/git-credential-store.txt
@@ -0,0 +1,69 @@
+git-credential-store(1)
+=======================
+
+NAME
+----
+git-credential-store - helper to store credentials on disk
+
+SYNOPSIS
+--------
+-------------------
+git config credential.helper 'store [options]'
+-------------------
+
+DESCRIPTION
+-----------
+
+NOTE: Using this helper will store your passwords unencrypted on disk,
+protected only by filesystem permissions. If this is not an acceptable
+security tradeoff, try linkgit:git-credential-cache[1], or find a helper
+that integrates with secure storage provided by your operating system.
+
+This command requests credentials from the user and stores them
+indefinitely on disk for use by future git programs.
+
+You probably don't want to invoke this command directly; it is meant to
+be used as a credential helper by other parts of git. See
+linkgit:gitcredentials[7] or `EXAMPLES` below.
+
+OPTIONS
+-------
+
+--store=<path>::
+
+	Use `<path>` to store credentials. The file will have its
+	filesystem permissions set to prevent other users on the system
+	from reading it, but will not be encrypted or otherwise
+	protected.
+
+--chain <helper>::
+
+	Specify an external helper to use for retrieving credentials
+	from the user, instead of the default method. The resulting
+	credentials are then stored as normal. This option can be
+	given multiple times; each chained helper will be tried until
+	credentials are received.
+
+Git may provide other options to the program when it is called as a
+credential helper; see linkgit:gitcredentials[7].
+
+EXAMPLES
+--------
+
+The point of this helper is to reduce the number of times you must type
+your username or password. For example:
+
+------------------------------------
+$ git config credential.helper store
+$ git push http://example.com/repo.git
+Username: <type your username>
+Password: <type your password>
+
+[several days later]
+$ git push http://example.com/repo.git
+[your credentials are used automatically]
+------------------------------------
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index bd1a3b6..33ea56c 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -88,6 +88,11 @@ cache::
 	Cache credentials in memory for a short period of time. See
 	linkgit:git-credential-cache[1] for details.
 
+store::
+
+	Store credentials indefinitely on disk. See
+	linkgit:git-credential-store[1] for details.
+
 You may may also have third-party helpers installed; search for
 `credential-*` in the output of `git help -a`, and consult the
 documentation of individual helpers.  Once you have selected a helper,
diff --git a/Makefile b/Makefile
index 2ecfd90..e818257 100644
--- a/Makefile
+++ b/Makefile
@@ -419,6 +419,7 @@ PROGRAM_OBJS += http-backend.o
 PROGRAM_OBJS += sh-i18n--envsubst.o
 PROGRAM_OBJS += credential-cache.o
 PROGRAM_OBJS += credential-cache--daemon.o
+PROGRAM_OBJS += credential-store.o
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
diff --git a/credential-store.c b/credential-store.c
new file mode 100644
index 0000000..8ab8582
--- /dev/null
+++ b/credential-store.c
@@ -0,0 +1,87 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "parse-options.h"
+
+static int lookup_credential(const char *fn, struct credential *c)
+{
+	config_exclusive_filename = fn;
+	credential_from_config(c);
+	return c->username && c->password;
+}
+
+static void store_item(const char *fn, const char *unique,
+		       const char *item, const char *value)
+{
+	struct strbuf key = STRBUF_INIT;
+
+	if (!unique)
+		return;
+
+	config_exclusive_filename = fn;
+	umask(077);
+
+	strbuf_addf(&key, "credential.%s.%s", unique, item);
+	git_config_set(key.buf, value);
+	strbuf_release(&key);
+}
+
+static void store_credential(const char *fn, struct credential *c)
+{
+	store_item(fn, c->unique, "username", c->username);
+	store_item(fn, c->unique, "password", c->password);
+}
+
+static void remove_credential(const char *fn, struct credential *c)
+{
+	store_item(fn, c->unique, "username", NULL);
+	store_item(fn, c->unique, "password", NULL);
+}
+
+int main(int argc, const char **argv)
+{
+	const char * const usage[] = {
+		"git credential-store [options]",
+		NULL
+	};
+	struct credential c = { NULL };
+	struct string_list chain = STRING_LIST_INIT_NODUP;
+	char *store = NULL;
+	int reject = 0;
+	struct option options[] = {
+		OPT_STRING_LIST(0, "store", &store, "file",
+				"fetch and store credentials in <file>"),
+		OPT_STRING_LIST(0, "chain", &chain, "helper",
+				"use <helper> to get non-cached credentials"),
+		OPT_BOOLEAN(0, "reject", &reject,
+			    "reject a stored credential"),
+		OPT_STRING(0, "username", &c.username, "name",
+			   "an existing username"),
+		OPT_STRING(0, "description", &c.description, "desc",
+			   "human-readable description of the credential"),
+		OPT_STRING(0, "unique", &c.unique, "token",
+			   "a unique context for the credential"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, NULL, options, usage, 0);
+	if (argc)
+		usage_with_options(usage, options);
+
+	if (!store)
+		store = expand_user_path("~/.git-credentials");
+	if (!store)
+		die("unable to set up default store; use --store");
+
+	if (reject)
+		remove_credential(store, &c);
+	else {
+		if (!lookup_credential(store, &c)) {
+			credential_fill(&c, &chain);
+			store_credential(store, &c);
+		}
+		printf("username=%s\n", c.username);
+		printf("password=%s\n", c.password);
+	}
+	return 0;
+}
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 994a0aa..5d54976 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -273,4 +273,59 @@ test_expect_success 'credential-cache removes rejected credentials' '
 	EOF
 '
 
+test_expect_success 'credential-store stores password' '
+	test_when_finished "rm -f .git-credentials" &&
+	check --unique=host store <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --unique=host store <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	EOF
+'
+
+test_expect_success 'credential-store requires matching unique token' '
+	test_when_finished "rm -f .git-credentials" &&
+	check --unique=host store <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --unique=host2 store <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
+test_expect_success 'credential-store removes rejected credentials' '
+	test_when_finished "rm -f .git-credentials" &&
+	check --unique=host store <<-\EOF &&
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+	check --reject --unique=host --username=askpass-result store <<-\EOF &&
+	--
+	EOF
+	check --unique=host store <<-\EOF
+	username=askpass-result
+	password=askpass-result
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
 test_done
-- 
1.7.6.rc1.12.g65e2

[PATCH 14/14] credentials: add "getpass" helper

[Jeff King]

This just does the normal "ask on the terminal, or use
GIT_ASKPASS" logic that we already do. But it's useful for
writers of third-party helpers. See the documentation for an
example.

Signed-off-by: Jeff King <peff@peff.net>
---
 .gitignore                               |    1 +
 Documentation/git-credential-getpass.txt |   58 ++++++++++++++++++++++++++++++
 Makefile                                 |    1 +
 credential-getpass.c                     |   37 +++++++++++++++++++
 4 files changed, 97 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-credential-getpass.txt
 create mode 100644 credential-getpass.c

diff --git a/.gitignore b/.gitignore
index 2b7a3f9..a26ad00 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,6 +33,7 @@
 /git-credential-cache
 /git-credential-cache--daemon
 /git-credential-store
+/git-credential-getpass
 /git-cvsexportcommit
 /git-cvsimport
 /git-cvsserver
diff --git a/Documentation/git-credential-getpass.txt b/Documentation/git-credential-getpass.txt
new file mode 100644
index 0000000..a37c7a2
--- /dev/null
+++ b/Documentation/git-credential-getpass.txt
@@ -0,0 +1,58 @@
+git-credential-getpass(1)
+=========================
+
+NAME
+----
+git-credential-getpass - helper to request credentials from a user
+
+SYNOPSIS
+--------
+[verse]
+git credential-getpass
+
+DESCRIPTION
+-----------
+
+This command requests credentials from the user using git's "default"
+scheme, including asking via the terminal and respecting the
+`GIT_ASKPASS` environment variable; see linkgit:gitcredentials[7] for a
+complete description. The helpers are provided on stdout using git's
+credential helper protocol.
+
+There is no point in using this program as a credential helper by
+itself; it is exactly equivalent to git's behavior when no helper is
+configured.
+
+However, writers of third-party helpers may want to invoke this program
+to simulate git's behavior.
+
+EXAMPLES
+--------
+
+Here's a simple, silly example of a helper that stores credentials on
+disk (similar to linkgit:git-credential-store[1]), and how it could use
+the `getpass` helper.
+
+-------------------------------------------
+#!/bin/sh
+
+STORAGE=$HOME/.credentials
+
+for i in "$@"; do
+	case "$i" in
+	--unique=*)
+		unique=${i#--unique=} ;;
+	esac
+done
+
+if ! test -e "$STORAGE/$unique"; then
+	mkdir -m 0700 "$STORAGE"
+	git credential-getpass "$@" >"$STORAGE/$unique"
+fi
+
+cat "$STORAGE/$unique"
+-------------------------------------------
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index e818257..86abd91 100644
--- a/Makefile
+++ b/Makefile
@@ -420,6 +420,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
 PROGRAM_OBJS += credential-cache.o
 PROGRAM_OBJS += credential-cache--daemon.o
 PROGRAM_OBJS += credential-store.o
+PROGRAM_OBJS += credential-getpass.o
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
diff --git a/credential-getpass.c b/credential-getpass.c
new file mode 100644
index 0000000..6bfb8f8
--- /dev/null
+++ b/credential-getpass.c
@@ -0,0 +1,37 @@
+#include "cache.h"
+#include "credential.h"
+#include "parse-options.h"
+#include "string-list.h"
+
+int main(int argc, const char **argv)
+{
+	const char * const usage[] = {
+		"git credential-getpass [options]",
+		NULL
+	};
+	struct credential c = { NULL };
+	int reject = 0;
+	struct option options[] = {
+		OPT_BOOLEAN(0, "reject", &reject,
+			    "reject a stored credential"),
+		OPT_STRING(0, "username", &c.username, "name",
+			   "an existing username"),
+		OPT_STRING(0, "description", &c.description, "desc",
+			   "human-readable description of the credential"),
+		OPT_STRING(0, "unique", &c.unique, "token",
+			   "a unique context for the credential"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, NULL, options, usage, 0);
+	if (argc)
+		usage_with_options(usage, options);
+
+	if (reject)
+		return 0;
+
+	credential_getpass(&c);
+	printf("username=%s\n", c.username);
+	printf("password=%s\n", c.password);
+	return 0;
+}
-- 
1.7.6.rc1.12.g65e2

Re: [RFC/PATCH 0/14] less annoying http authentication

[Jeff King]

On Mon, Jul 18, 2011 at 03:46:42AM -0400, Jeff King wrote:

> This is still in the RFC stage. I think the code is fine, and I've been
> using it for a few weeks. I tried to keep the design of the credential
> helper interface simple, but flexible enough to meet the needs of the
> various keychain systems. Aside from the usual, what I'd most like
> feedback on is whether the interface is sufficient to actually integrate
> with those systems. And I suspect we won't really know until people try
> to make helpers for their pet systems.

BTW, I'll be traveling and have spotty time and email availability for
the next 3-5 days. So I hope to get lots of comments, but please don't
be discouraged if I don't respond immediately. :)

-Peff