[PATCH 05/13] credential: gate new fields on capability

["brian m. carlson" <sandals@crustytoothpaste.net>]

We support the new credential and authtype fields, but we lack a way to
indicate to a credential helper that we'd like them to be used.  Without
some sort of indication, the credential helper doesn't know if it should
try to provide us a username and password, or a pre-encoded credential.
For example, the helper might prefer a more restricted Bearer token if
pre-encoded credentials are possible, but might have to fall back to
more general username and password if not.

Let's provide a simple way to indicate whether Git (or, for that matter,
the helper) is capable of understanding the authtype and credential
fields.  We send this capability when we generate a request, and the
other side may reply to indicate to us that it does, too.

For now, don't enable sending capabilities for the HTTP code.  In a
future commit, we'll introduce appropriate handling for that code,
which requires more in-depth work.

The logic for determining whether a capability is supported may seem
complex, but it is not.  At each stage, we emit the capability to the
following stage if all preceding stages have declared it.  Thus, if the
caller to git credential fill didn't declare it, then we won't send it
to the helper, and if fill's caller did send but the helper doesn't
understand it, then we won't send it on in the response.  If we're an
internal user, then we know about all capabilities and will request
them.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/credential-cache--daemon.c |   2 +-
 builtin/credential-store.c         |   2 +-
 builtin/credential.c               |   6 +-
 credential.c                       |  58 ++++++++++++++--
 credential.h                       |  30 +++++++-
 http.c                             |  10 +--
 imap-send.c                        |   2 +-
 remote-curl.c                      |   4 +-
 t/t0300-credentials.sh             | 107 ++++++++++++++++++++++++++++-
 9 files changed, 197 insertions(+), 24 deletions(-)

diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
index 3a6a750a8e..ccbcf99ac1 100644
--- a/builtin/credential-cache--daemon.c
+++ b/builtin/credential-cache--daemon.c
@@ -115,7 +115,7 @@ static int read_request(FILE *fh, struct credential *c,
 		return error("client sent bogus timeout line: %s", item.buf);
 	*timeout = atoi(p);
 
-	if (credential_read(c, fh) < 0)
+	if (credential_read(c, fh, CREDENTIAL_OP_HELPER) < 0)
 		return -1;
 	return 0;
 }
diff --git a/builtin/credential-store.c b/builtin/credential-store.c
index 4a492411bb..494c809332 100644
--- a/builtin/credential-store.c
+++ b/builtin/credential-store.c
@@ -205,7 +205,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)
 	if (!fns.nr)
 		die("unable to set up default path; use --file");
 
-	if (credential_read(&c, stdin) < 0)
+	if (credential_read(&c, stdin, CREDENTIAL_OP_HELPER) < 0)
 		die("unable to read credential");
 
 	if (!strcmp(op, "get"))
diff --git a/builtin/credential.c b/builtin/credential.c
index 7010752987..5123dabcf1 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -17,12 +17,12 @@ int cmd_credential(int argc, const char **argv, const char *prefix UNUSED)
 		usage(usage_msg);
 	op = argv[1];
 
-	if (credential_read(&c, stdin) < 0)
+	if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
 		die("unable to read credential from stdin");
 
 	if (!strcmp(op, "fill")) {
-		credential_fill(&c);
-		credential_write(&c, stdout);
+		credential_fill(&c, 0);
+		credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
 	} else if (!strcmp(op, "approve")) {
 		credential_approve(&c);
 	} else if (!strcmp(op, "reject")) {
diff --git a/credential.c b/credential.c
index c521822e5a..f2a26b8672 100644
--- a/credential.c
+++ b/credential.c
@@ -34,6 +34,11 @@ void credential_clear(struct credential *c)
 	credential_init(c);
 }
 
+static void credential_set_all_capabilities(struct credential *c)
+{
+	c->capa_authtype.request_initial = 1;
+}
+
 int credential_match(const struct credential *want,
 		     const struct credential *have, int match_password)
 {
@@ -210,7 +215,39 @@ static void credential_getpass(struct credential *c)
 						 PROMPT_ASKPASS);
 }
 
-int credential_read(struct credential *c, FILE *fp)
+static void credential_set_capability(struct credential_capability *capa, int op_type)
+{
+	switch (op_type) {
+	case CREDENTIAL_OP_INITIAL:
+		capa->request_initial = 1;
+		break;
+	case CREDENTIAL_OP_HELPER:
+		capa->request_helper = 1;
+		break;
+	case CREDENTIAL_OP_RESPONSE:
+		capa->response = 1;
+		break;
+	}
+}
+
+static int credential_has_capability(const struct credential_capability *capa, int op_type)
+{
+	/*
+	 * We're checking here if each previous step indicated that we had the
+	 * capability.  If it did, then we want to pass it along; conversely, if
+	 * it did not, we don't want to report that to our caller.
+	 */
+	switch (op_type) {
+	case CREDENTIAL_OP_HELPER:
+		return capa->request_initial;
+	case CREDENTIAL_OP_RESPONSE:
+		return capa->request_initial && capa->request_helper;
+	default:
+		return 0;
+	}
+}
+
+int credential_read(struct credential *c, FILE *fp, int op_type)
 {
 	struct strbuf line = STRBUF_INIT;
 
@@ -249,6 +286,8 @@ int credential_read(struct credential *c, FILE *fp)
 			c->path = xstrdup(value);
 		} else if (!strcmp(key, "wwwauth[]")) {
 			strvec_push(&c->wwwauth_headers, value);
+		} else if (!strcmp(key, "capability[]") && !strcmp(value, "authtype")) {
+			credential_set_capability(&c->capa_authtype, op_type);
 		} else if (!strcmp(key, "password_expiry_utc")) {
 			errno = 0;
 			c->password_expiry_utc = parse_timestamp(value, NULL, 10);
@@ -288,14 +327,18 @@ static void credential_write_item(FILE *fp, const char *key, const char *value,
 	fprintf(fp, "%s=%s\n", key, value);
 }
 
-void credential_write(const struct credential *c, FILE *fp)
+void credential_write(const struct credential *c, FILE *fp, int op_type)
 {
+	if (credential_has_capability(&c->capa_authtype, op_type)) {
+		credential_write_item(fp, "capability[]", "authtype", 0);
+		credential_write_item(fp, "authtype", c->authtype, 0);
+		credential_write_item(fp, "credential", c->credential, 0);
+	}
 	credential_write_item(fp, "protocol", c->protocol, 1);
 	credential_write_item(fp, "host", c->host, 1);
 	credential_write_item(fp, "path", c->path, 0);
 	credential_write_item(fp, "username", c->username, 0);
 	credential_write_item(fp, "password", c->password, 0);
-	credential_write_item(fp, "credential", c->credential, 0);
 	credential_write_item(fp, "oauth_refresh_token", c->oauth_refresh_token, 0);
 	if (c->password_expiry_utc != TIME_MAX) {
 		char *s = xstrfmt("%"PRItime, c->password_expiry_utc);
@@ -304,7 +347,6 @@ void credential_write(const struct credential *c, FILE *fp)
 	}
 	for (size_t i = 0; i < c->wwwauth_headers.nr; i++)
 		credential_write_item(fp, "wwwauth[]", c->wwwauth_headers.v[i], 0);
-	credential_write_item(fp, "authtype", c->authtype, 0);
 }
 
 static int run_credential_helper(struct credential *c,
@@ -327,14 +369,14 @@ static int run_credential_helper(struct credential *c,
 
 	fp = xfdopen(helper.in, "w");
 	sigchain_push(SIGPIPE, SIG_IGN);
-	credential_write(c, fp);
+	credential_write(c, fp, want_output ? CREDENTIAL_OP_HELPER : CREDENTIAL_OP_RESPONSE);
 	fclose(fp);
 	sigchain_pop(SIGPIPE);
 
 	if (want_output) {
 		int r;
 		fp = xfdopen(helper.out, "r");
-		r = credential_read(c, fp);
+		r = credential_read(c, fp, CREDENTIAL_OP_HELPER);
 		fclose(fp);
 		if (r < 0) {
 			finish_command(&helper);
@@ -367,7 +409,7 @@ static int credential_do(struct credential *c, const char *helper,
 	return r;
 }
 
-void credential_fill(struct credential *c)
+void credential_fill(struct credential *c, int all_capabilities)
 {
 	int i;
 
@@ -375,6 +417,8 @@ void credential_fill(struct credential *c)
 		return;
 
 	credential_apply_config(c);
+	if (all_capabilities)
+		credential_set_all_capabilities(c);
 
 	for (i = 0; i < c->helpers.nr; i++) {
 		credential_do(c, c->helpers.items[i].string, "get");
diff --git a/credential.h b/credential.h
index 9db892cf4d..2051d04c5a 100644
--- a/credential.h
+++ b/credential.h
@@ -93,6 +93,25 @@
  * -----------------------------------------------------------------------
  */
 
+/*
+ * These values define the kind of operation we're performing and the
+ * capabilities at each stage.  The first is either an external request (via git
+ * credential fill) or an internal request (e.g., via the HTTP) code.  The
+ * second is the call to the credential helper, and the third is the response
+ * we're providing.
+ *
+ * At each stage, we will emit the capability only if the previous stage
+ * supported it.
+ */
+#define CREDENTIAL_OP_INITIAL  1
+#define CREDENTIAL_OP_HELPER   2
+#define CREDENTIAL_OP_RESPONSE 3
+
+struct credential_capability {
+	unsigned request_initial:1,
+		 request_helper:1,
+		 response:1;
+};
 
 /**
  * This struct represents a single username/password combination
@@ -136,6 +155,8 @@ struct credential {
 		 use_http_path:1,
 		 username_from_proto:1;
 
+	struct credential_capability capa_authtype;
+
 	char *username;
 	char *password;
 	char *credential;
@@ -174,8 +195,11 @@ void credential_clear(struct credential *);
  * returns, the username and password fields of the credential are
  * guaranteed to be non-NULL. If an error occurs, the function will
  * die().
+ *
+ * If all_capabilities is set, this is an internal user that is prepared
+ * to deal with all known capabilities, and we should advertise that fact.
  */
-void credential_fill(struct credential *);
+void credential_fill(struct credential *, int all_capabilities);
 
 /**
  * Inform the credential subsystem that the provided credentials
@@ -198,8 +222,8 @@ void credential_approve(struct credential *);
  */
 void credential_reject(struct credential *);
 
-int credential_read(struct credential *, FILE *);
-void credential_write(const struct credential *, FILE *);
+int credential_read(struct credential *, FILE *, int);
+void credential_write(const struct credential *, FILE *, int);
 
 /*
  * Parse a url into a credential struct, replacing any existing contents.
diff --git a/http.c b/http.c
index 1c2200da77..4f5df6fb14 100644
--- a/http.c
+++ b/http.c
@@ -569,7 +569,7 @@ static void init_curl_http_auth(CURL *result)
 		return;
 	}
 
-	credential_fill(&http_auth);
+	credential_fill(&http_auth, 0);
 
 	curl_easy_setopt(result, CURLOPT_USERNAME, http_auth.username);
 	curl_easy_setopt(result, CURLOPT_PASSWORD, http_auth.password);
@@ -596,7 +596,7 @@ static void init_curl_proxy_auth(CURL *result)
 {
 	if (proxy_auth.username) {
 		if (!proxy_auth.password)
-			credential_fill(&proxy_auth);
+			credential_fill(&proxy_auth, 0);
 		set_proxyauth_name_password(result);
 	}
 
@@ -630,7 +630,7 @@ static int has_cert_password(void)
 		cert_auth.host = xstrdup("");
 		cert_auth.username = xstrdup("");
 		cert_auth.path = xstrdup(ssl_cert);
-		credential_fill(&cert_auth);
+		credential_fill(&cert_auth, 0);
 	}
 	return 1;
 }
@@ -645,7 +645,7 @@ static int has_proxy_cert_password(void)
 		proxy_cert_auth.host = xstrdup("");
 		proxy_cert_auth.username = xstrdup("");
 		proxy_cert_auth.path = xstrdup(http_proxy_ssl_cert);
-		credential_fill(&proxy_cert_auth);
+		credential_fill(&proxy_cert_auth, 0);
 	}
 	return 1;
 }
@@ -2191,7 +2191,7 @@ static int http_request_reauth(const char *url,
 		BUG("Unknown http_request target");
 	}
 
-	credential_fill(&http_auth);
+	credential_fill(&http_auth, 0);
 
 	return http_request(url, result, target, options);
 }
diff --git a/imap-send.c b/imap-send.c
index f2e1947e63..8c89e866b6 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -944,7 +944,7 @@ static void server_fill_credential(struct imap_server_conf *srvc, struct credent
 	cred->username = xstrdup_or_null(srvc->user);
 	cred->password = xstrdup_or_null(srvc->pass);
 
-	credential_fill(cred);
+	credential_fill(cred, 1);
 
 	if (!srvc->user)
 		srvc->user = xstrdup(cred->username);
diff --git a/remote-curl.c b/remote-curl.c
index e37eaa17b7..f96bda2431 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -926,7 +926,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
 		do {
 			err = probe_rpc(rpc, &results);
 			if (err == HTTP_REAUTH)
-				credential_fill(&http_auth);
+				credential_fill(&http_auth, 0);
 		} while (err == HTTP_REAUTH);
 		if (err != HTTP_OK)
 			return -1;
@@ -1044,7 +1044,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
 	rpc->any_written = 0;
 	err = run_slot(slot, NULL);
 	if (err == HTTP_REAUTH && !large_request) {
-		credential_fill(&http_auth);
+		credential_fill(&http_auth, 0);
 		curl_slist_free_all(headers);
 		goto retry;
 	}
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 400f6bdbca..8477108b28 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -12,7 +12,13 @@ test_expect_success 'setup helper scripts' '
 	IFS==
 	while read key value; do
 		echo >&2 "$whoami: $key=$value"
-		eval "$key=$value"
+		if test -z "${key%%*\[\]}"
+		then
+			key=${key%%\[\]}
+			eval "$key=\"\$$key $value\""
+		else
+			eval "$key=$value"
+		fi
 	done
 	IFS=$OIFS
 	EOF
@@ -35,6 +41,16 @@ test_expect_success 'setup helper scripts' '
 	test -z "$pass" || echo password=$pass
 	EOF
 
+	write_script git-credential-verbatim-cred <<-\EOF &&
+	authtype=$1; shift
+	credential=$1; shift
+	. ./dump
+	echo capability[]=authtype
+	test -z "${capability##*authtype*}" || return
+	test -z "$authtype" || echo authtype=$authtype
+	test -z "$credential" || echo credential=$credential
+	EOF
+
 	write_script git-credential-verbatim-with-expiry <<-\EOF &&
 	user=$1; shift
 	pass=$1; shift
@@ -64,6 +80,26 @@ test_expect_success 'credential_fill invokes helper' '
 	EOF
 '
 
+test_expect_success 'credential_fill invokes helper with credential' '
+	check fill "verbatim-cred Bearer token" <<-\EOF
+	capability[]=authtype
+	protocol=http
+	host=example.com
+	--
+	capability[]=authtype
+	authtype=Bearer
+	credential=token
+	protocol=http
+	host=example.com
+	--
+	verbatim-cred: get
+	verbatim-cred: capability[]=authtype
+	verbatim-cred: protocol=http
+	verbatim-cred: host=example.com
+	EOF
+'
+
+
 test_expect_success 'credential_fill invokes multiple helpers' '
 	check fill useless "verbatim foo bar" <<-\EOF
 	protocol=http
@@ -83,6 +119,42 @@ test_expect_success 'credential_fill invokes multiple helpers' '
 	EOF
 '
 
+test_expect_success 'credential_fill response does not get capabilities when helpers are incapable' '
+	check fill useless "verbatim foo bar" <<-\EOF
+	capability[]=authtype
+	protocol=http
+	host=example.com
+	--
+	protocol=http
+	host=example.com
+	username=foo
+	password=bar
+	--
+	useless: get
+	useless: capability[]=authtype
+	useless: protocol=http
+	useless: host=example.com
+	verbatim: get
+	verbatim: capability[]=authtype
+	verbatim: protocol=http
+	verbatim: host=example.com
+	EOF
+'
+
+test_expect_success 'credential_fill response does not get capabilities when caller is incapable' '
+	check fill "verbatim-cred Bearer token" <<-\EOF
+	protocol=http
+	host=example.com
+	--
+	protocol=http
+	host=example.com
+	--
+	verbatim-cred: get
+	verbatim-cred: protocol=http
+	verbatim-cred: host=example.com
+	EOF
+'
+
 test_expect_success 'credential_fill stops when we get a full response' '
 	check fill "verbatim one two" "verbatim three four" <<-\EOF
 	protocol=http
@@ -99,6 +171,25 @@ test_expect_success 'credential_fill stops when we get a full response' '
 	EOF
 '
 
+test_expect_success 'credential_fill thinks a credential is a full response' '
+	check fill "verbatim-cred Bearer token" "verbatim three four" <<-\EOF
+	capability[]=authtype
+	protocol=http
+	host=example.com
+	--
+	capability[]=authtype
+	authtype=Bearer
+	credential=token
+	protocol=http
+	host=example.com
+	--
+	verbatim-cred: get
+	verbatim-cred: capability[]=authtype
+	verbatim-cred: protocol=http
+	verbatim-cred: host=example.com
+	EOF
+'
+
 test_expect_success 'credential_fill continues through partial response' '
 	check fill "verbatim one \"\"" "verbatim two three" <<-\EOF
 	protocol=http
@@ -175,6 +266,20 @@ test_expect_success 'credential_fill passes along metadata' '
 	EOF
 '
 
+test_expect_success 'credential_fill produces no credential without capability' '
+	check fill "verbatim-cred Bearer token" <<-\EOF
+	protocol=http
+	host=example.com
+	--
+	protocol=http
+	host=example.com
+	--
+	verbatim-cred: get
+	verbatim-cred: protocol=http
+	verbatim-cred: host=example.com
+	EOF
+'
+
 test_expect_success 'credential_approve calls all helpers' '
 	check approve useless "verbatim one two" <<-\EOF
 	protocol=http