git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCHv2 08/13] credential: make relevance of http path configurable
Date: Tue, 6 Dec 2011 01:22:52 -0500	[thread overview]
Message-ID: <20111206062252.GH29233@sigill.intra.peff.net> (raw)
In-Reply-To: <20111206062127.GA29046@sigill.intra.peff.net>

When parsing a URL into a credential struct, we carefully
record each part of the URL, including the path on the
remote host, and use the result as part of the credential
context.

This had two practical implications:

  1. Credential helpers which store a credential for later
     access are likely to use the "path" portion as part of
     the storage key. That means that a request to

       https://example.com/foo.git

     would not use the same credential that was stored in an
     earlier request for:

       https://example.com/bar.git

  2. The prompt shown to the user includes all relevant
     context, including the path.

In most cases, however, users will have a single password
per host. The behavior in (1) will be inconvenient, and the
prompt in (2) will be overly long.

This patch introduces a config option to toggle the
relevance of http paths. When turned on, we use the path as
before. When turned off, we drop the path component from the
context: helpers don't see it, and it does not appear in the
prompt.

This is nothing you couldn't do with a clever credential
helper at the start of your stack, like:

  [credential "http://"]
	helper = "f() { grep -v ^path= ; }; f"
	helper = your_real_helper

But doing this:

  [credential]
	useHttpPath = false

is way easier and more readable. Furthermore, since most
users will want the "off" behavior, that is the new default.
Users who want it "on" can set the variable (either for all
credentials, or just for a subset using
credential.*.useHttpPath).

Signed-off-by: Jeff King <peff@peff.net>
---
 credential.c           |   14 ++++++++++++++
 credential.h           |    3 ++-
 t/t0300-credentials.sh |   29 +++++++++++++++++++++++++++++
 t/t5550-http-fetch.sh  |    2 +-
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/credential.c b/credential.c
index 3c17ea1..a17eafe 100644
--- a/credential.c
+++ b/credential.c
@@ -69,16 +69,30 @@ static int credential_config_callback(const char *var, const char *value,
 		if (!c->username)
 			c->username = xstrdup(value);
 	}
+	else if (!strcmp(key, "usehttppath"))
+		c->use_http_path = git_config_bool(var, value);
 
 	return 0;
 }
 
+static int proto_is_http(const char *s)
+{
+	if (!s)
+		return 0;
+	return !strcmp(s, "https") || !strcmp(s, "http");
+}
+
 static void credential_apply_config(struct credential *c)
 {
 	if (c->configured)
 		return;
 	git_config(credential_config_callback, c);
 	c->configured = 1;
+
+	if (!c->use_http_path && proto_is_http(c->protocol)) {
+		free(c->path);
+		c->path = NULL;
+	}
 }
 
 static void credential_describe(struct credential *c, struct strbuf *out)
diff --git a/credential.h b/credential.h
index e504272..96ea41b 100644
--- a/credential.h
+++ b/credential.h
@@ -6,7 +6,8 @@
 struct credential {
 	struct string_list helpers;
 	unsigned approved:1,
-		 configured:1;
+		 configured:1,
+		 use_http_path:1;
 
 	char *username;
 	char *password;
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 57751a1..89a8531 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -247,4 +247,33 @@ test_expect_success 'pull username from config' '
 	EOF
 '
 
+test_expect_success 'http paths can be part of context' '
+	check fill "verbatim foo bar" <<-\EOF &&
+	protocol=https
+	host=example.com
+	path=foo.git
+	--
+	username=foo
+	password=bar
+	--
+	verbatim: get
+	verbatim: protocol=https
+	verbatim: host=example.com
+	EOF
+	test_config credential.https://example.com.useHttpPath true &&
+	check fill "verbatim foo bar" <<-\EOF
+	protocol=https
+	host=example.com
+	path=foo.git
+	--
+	username=foo
+	password=bar
+	--
+	verbatim: get
+	verbatim: protocol=https
+	verbatim: host=example.com
+	verbatim: path=foo.git
+	EOF
+'
+
 test_done
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 18f4d59..dd073d1 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -53,7 +53,7 @@ test_expect_success 'setup askpass helpers' '
 '
 
 expect_askpass() {
-	dest=$HTTPD_DEST/auth/repo.git
+	dest=$HTTPD_DEST
 	{
 		case "$1" in
 		none)
-- 
1.7.8.rc4.4.g884ec

  parent reply	other threads:[~2011-12-06  6:23 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-06  6:21 [PATCHv2 0/13] credential helpers Jeff King
2011-12-06  6:22 ` [PATCHv2 01/13] test-lib: add test_config_global variant Jeff King
2011-12-06  6:22 ` [PATCHv2 02/13] t5550: fix typo Jeff King
2011-12-06  6:22 ` [PATCHv2 03/13] introduce credentials API Jeff King
2011-12-06  6:22 ` [PATCHv2 04/13] credential: add function for parsing url components Jeff King
2011-12-06  6:22 ` [PATCHv2 05/13] http: use credential API to get passwords Jeff King
2011-12-06  6:22 ` [PATCHv2 06/13] credential: apply helper config Jeff King
2011-12-06 23:58   ` Junio C Hamano
2011-12-07  0:45     ` Jeff King
2011-12-07  0:49       ` Jeff King
2011-12-06  6:22 ` [PATCHv2 07/13] credential: add credential.*.username Jeff King
2011-12-06  6:22 ` Jeff King [this message]
2011-12-06  6:22 ` [PATCHv2 09/13] docs: end-user documentation for the credential subsystem Jeff King
2011-12-06  6:22 ` [PATCHv2 10/13] credentials: add "cache" helper Jeff King
2011-12-06  6:23 ` [PATCHv2 11/13] strbuf: add strbuf_add*_urlencode Jeff King
2011-12-06  6:23 ` [PATCHv2 12/13] credentials: add "store" helper Jeff King
2011-12-06 21:50   ` Junio C Hamano
2011-12-09 23:19   ` Jeff King
2011-12-06  6:23 ` [PATCHv2 13/13] t: add test harness for external credential helpers Jeff King
2011-12-06 21:51   ` Junio C Hamano
2011-12-06 22:08     ` Jeff King
2011-12-06 21:40 ` [PATCHv2 0/13] " Junio C Hamano
2011-12-07  6:42   ` Jeff King
2011-12-08 21:34     ` Junio C Hamano
2011-12-09  2:29       ` Jeff King
2011-12-09 18:00         ` Junio C Hamano
2011-12-09 23:18           ` Jeff King
2011-12-09 23:34             ` Junio C Hamano
2011-12-09 23:39               ` Jeff King
2011-12-09 23:56                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111206062252.GH29233@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).