threads / patch / 38900

v5, 3 partsgit-credential-store: support multiple credential files

Subject: [PATCH v5 1/3] git-credential-store: support multiple credential files

## tl;dr

11 messages between Mar 24, 2015 and Mar 26, 2015. Diffs are folded; open one to read it.

replies: 10people: 5as markdown or json

Paul Tan· Mar 24, 2015, 05:20 UTC · lore

Previously, git-credential-store only supported storing credentials in a single file: ~/.git-credentials. In order to support the XDG base directory specification[1], git-credential-store needs to be able to lookup and erase credentials from multiple files, as well as to pick the appropriate file to write to so that the credentials can be found on subsequent lookups.

[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html

Note that some credential storage files may not be owned, readable or writable by the user, as they may be system-wide files that are meant to apply to every user.

Instead of a single file path, lookup_credential(), remove_credential() and store_credential() now take a precedence-ordered string_list of file paths. lookup_credential() expects both user-specific and system-wide credential files to be provided to support the use case of system administrators setting default credentials for users. remove_credential() and store_credential() expect only the user-specific credential files to be provided as usually the only config files that users are allowed to edit are their own user-specific ones.

lookup_credential() will read these (user-specific and system-wide) file paths in order until it finds the 1st matching credential and print it. As some files may be private and thus unreadable, any file which cannot be read will be ignored silently.

remove_credential() will erase credentials from all (user-specific) files in the list. This is because if credentials are only erased from the file with the highest precedence, a matching credential may still be found in a file further down the list. (Note that due to the lockfile code, this requires the directory to be writable, which should be so for user-specific config files)

store_credential() will write the credentials to the first existing (user-specific) file in the list. If none of the files in the list exist, store_credential() will write to the filename specified by the first item of the filename list. For backwards compatibility, this filename should be "~/.git-credentials".

Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
The previous version can be found at [1].
[1] http://thread.gmane.org/gmane.comp.version-control.git/265683
The changes are as follows:
* Fix comment formatting error. Thanks Matthieu.
 credential-store.c | 81 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 25 deletions(-)
Show changes to credential-store.c +56 −25
diff --git a/credential-store.c b/credential-store.c
index 925d3f4..d700a93 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -6,7 +6,7 @@
 
 static struct lock_file credential_lock;
 
-static void parse_credential_file(const char *fn,
+static int parse_credential_file(const char *fn,
 				  struct credential *c,
 				  void (*match_cb)(struct credential *),
 				  void (*other_cb)(struct strbuf *))
@@ -14,18 +14,20 @@ static void parse_credential_file(const char *fn,
 	FILE *fh;
 	struct strbuf line = STRBUF_INIT;
 	struct credential entry = CREDENTIAL_INIT;
+	int found_credential = 0;
 
 	fh = fopen(fn, "r");
 	if (!fh) {
-		if (errno != ENOENT)
+		if (errno != ENOENT && errno != EACCES)
 			die_errno("unable to open %s", fn);
-		return;
+		return found_credential;
 	}
 
 	while (strbuf_getline(&line, fh, '\n') != EOF) {
 		credential_from_url(&entry, line.buf);
 		if (entry.username && entry.password &&
 		    credential_match(c, &entry)) {
+			found_credential = 1;
 			if (match_cb) {
 				match_cb(&entry);
 				break;
@@ -38,6 +40,7 @@ static void parse_credential_file(const char *fn,
 	credential_clear(&entry);
 	strbuf_release(&line);
 	fclose(fh);
+	return found_credential;
 }
 
 static void print_entry(struct credential *c)
@@ -64,21 +67,10 @@ static void rewrite_credential_file(const char *fn, struct credential *c,
 		die_errno("unable to commit credential store");
 }
 
-static void store_credential(const char *fn, struct credential *c)
+static void store_credential_file(const char *fn, struct credential *c)
 {
 	struct strbuf buf = STRBUF_INIT;
 
-	/*
-	 * Sanity check that what we are storing is actually sensible.
-	 * In particular, we can't make a URL without a protocol field.
-	 * Without either a host or pathname (depending on the scheme),
-	 * we have no primary key. And without a username and password,
-	 * we are not actually storing a credential.
-	 */
-	if (!c->protocol || !(c->host || c->path) ||
-	    !c->username || !c->password)
-		return;
-
 	strbuf_addf(&buf, "%s://", c->protocol);
 	strbuf_addstr_urlencode(&buf, c->username, 1);
 	strbuf_addch(&buf, ':');
@@ -95,8 +87,37 @@ static void store_credential(const char *fn, struct credential *c)
 	strbuf_release(&buf);
 }
 
-static void remove_credential(const char *fn, struct credential *c)
+static void store_credential(const struct string_list *fns, struct credential *c)
 {
+	struct string_list_item *fn;
+
+	/*
+	 * Sanity check that what we are storing is actually sensible.
+	 * In particular, we can't make a URL without a protocol field.
+	 * Without either a host or pathname (depending on the scheme),
+	 * we have no primary key. And without a username and password,
+	 * we are not actually storing a credential.
+	 */
+	if (!c->protocol || !(c->host || c->path) || !c->username || !c->password)
+		return;
+
+	for_each_string_list_item(fn, fns)
+		if (!access(fn->string, F_OK)) {
+			store_credential_file(fn->string, c);
+			return;
+		}
+	/*
+	 * Write credential to the filename specified by fns->items[0], thus
+	 * creating it
+	 */
+	if (fns->nr)
+		store_credential_file(fns->items[0].string, c);
+}
+
+static void remove_credential(const struct string_list *fns, struct credential *c)
+{
+	struct string_list_item *fn;
+
 	/*
 	 * Sanity check that we actually have something to match
 	 * against. The input we get is a restrictive pattern,
@@ -105,14 +126,20 @@ static void remove_credential(const char *fn, struct credential *c)
 	 * to empty input. So explicitly disallow it, and require that the
 	 * pattern have some actual content to match.
 	 */
-	if (c->protocol || c->host || c->path || c->username)
-		rewrite_credential_file(fn, c, NULL);
+	if (!c->protocol && !c->host && !c->path && !c->username)
+		return;
+	for_each_string_list_item(fn, fns)
+		if (!access(fn->string, F_OK))
+			rewrite_credential_file(fn->string, c, NULL);
 }
 
-static int lookup_credential(const char *fn, struct credential *c)
+static void lookup_credential(const struct string_list *fns, struct credential *c)
 {
-	parse_credential_file(fn, c, print_entry, NULL);
-	return c->username && c->password;
+	struct string_list_item *fn;
+
+	for_each_string_list_item(fn, fns)
+		if (parse_credential_file(fn->string, c, print_entry, NULL))
+			return; /* Found credential */
 }
 
 int main(int argc, char **argv)
@@ -123,6 +150,7 @@ int main(int argc, char **argv)
 	};
 	const char *op;
 	struct credential c = CREDENTIAL_INIT;
+	struct string_list fns = STRING_LIST_INIT_DUP;
 	char *file = NULL;
 	struct option options[] = {
 		OPT_STRING(0, "file", &file, "path",
@@ -139,20 +167,23 @@ int main(int argc, char **argv)
 
 	if (!file)
 		file = expand_user_path("~/.git-credentials");
-	if (!file)
+	if (file)
+		string_list_append(&fns, file);
+	else
 		die("unable to set up default path; use --file");
 
 	if (credential_read(&c, stdin) < 0)
 		die("unable to read credential");
 
 	if (!strcmp(op, "get"))
-		lookup_credential(file, &c);
+		lookup_credential(&fns, &c);
 	else if (!strcmp(op, "erase"))
-		remove_credential(file, &c);
+		remove_credential(&fns, &c);
 	else if (!strcmp(op, "store"))
-		store_credential(file, &c);
+		store_credential(&fns, &c);
 	else
 		; /* Ignore unknown operation. */
 
+	string_list_clear(&fns, 0);
 	return 0;
 }
-- 
2.1.4
Paul Tan· Mar 24, 2015, 05:20 UTC · re: Paul Tan · lore

[PATCH v5 2/3] git-credential-store: support XDG_CONFIG_HOME

Add $XDG_CONFIG_HOME/git/credentials to the default credential search path of git-credential-store. This allows git-credential-store to support user-specific configuration files in accordance with the XDG base directory specification[1].

[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html

~/.git-credentials has a higher precedence than $XDG_CONFIG_HOME/git/credentials when looking up credentials. This means that if any duplicate matching credentials are found in the xdg file (due to ~/.git-credentials being updated by old versions of git or outdated tools), they will not be used at all. This is to give the user some leeway in switching to old versions of git while keeping the xdg directory. This is consistent with the behavior of git-config.

However, the higher precedence of ~/.git-credentials means that as long as ~/.git-credentials exist, all credentials will be written to the ~/.git-credentials file even if the user has an xdg file as having a ~/.git-credentials file indicates that the user wants to preserve backwards-compatibility. This is also consistent with the behavior of git-config.

To make this precedence explicit in docs/git-credential-store, add a new section FILES that lists out the credential file paths in their order of precedence, and explain how the ordering affects the lookup, storage and erase operations.

Also, update the documentation for --file to briefly explain the operations on multiple files if the --file option is not provided.

Since the xdg file will not be used unless it actually exists, to prevent the situation where some credentials are present in the xdg file while some are present in the home file, users are recommended to not create the xdg file if they require compatibility with old versions of git or outdated tools. Note, though, that "erase" can be used to explicitly erase matching credentials from all files.

Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
This patch is a combination of 2 patches [1][2].
[1] http://thread.gmane.org/gmane.comp.version-control.git/265683/focus=265686
[2] http://thread.gmane.org/gmane.comp.version-control.git/265683/focus=265685
The changes are as follows:
* s/per default/by default/ in the documentation. Thanks Matthieu.
 Documentation/git-credential-store.txt | 35 ++++++++++++++++++++++++++++++++--
 credential-store.c                     | 13 +++++++++----
 2 files changed, 42 insertions(+), 6 deletions(-)
Show changes to 2 files +42 −6

Documentation/git-credential-store.txt, credential-store.c

diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
index bc97071..e3c8f27 100644
--- a/Documentation/git-credential-store.txt
+++ b/Documentation/git-credential-store.txt
@@ -31,10 +31,41 @@ OPTIONS
 
 --file=<path>::
 
-	Use `<path>` to store credentials. The file will have its
+	Use `<path>` to lookup and 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. Defaults to `~/.git-credentials`.
+	protected. If not specified, credentials will be searched for from
+	`~/.git-credentials` and `$XDG_CONFIG_HOME/git/credentials`, and
+	credentials will be written to `~/.git-credentials` if it exists, or
+	`$XDG_CONFIG_HOME/git/credentials` if it exists and the former does
+	not. See also <<FILES>>.
+
+[[FILES]]
+FILES
+-----
+
+If not set explicitly with '--file', there are two files where
+git-credential-store will search for credentials in order of precedence:
+
+~/.git-credentials::
+	User-specific credentials file.
+
+$XDG_CONFIG_HOME/git/credentials::
+	Second user-specific credentials file. If '$XDG_CONFIG_HOME' is not set
+	or empty, `$HOME/.config/git/credentials` will be used. Any credentials
+	stored in this file will not be used if `~/.git-credentials` has a
+	matching credential as well. It is a good idea not to create this file
+	if you sometimes use older versions of Git that do not support it.
+
+For credential lookups, the files are read in the order given above, with the
+first matching credential found taking precedence over credentials found in
+files further down the list.
+
+Credential storage will by default write to the first existing file in the
+list. If none of these files exist, `~/.git-credentials` will be created and
+written to.
+
+When erasing credentials, matching credentials will be erased from all files.
 
 EXAMPLES
 --------
diff --git a/credential-store.c b/credential-store.c
index d700a93..8b22251 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -165,11 +165,16 @@ int main(int argc, char **argv)
 		usage_with_options(usage, options);
 	op = argv[0];
 
-	if (!file)
-		file = expand_user_path("~/.git-credentials");
-	if (file)
+	if (file) {
 		string_list_append(&fns, file);
-	else
+	} else {
+		if ((file = expand_user_path("~/.git-credentials")))
+			string_list_append_nodup(&fns, file);
+		home_config_paths(NULL, &file, "credentials");
+		if (file)
+			string_list_append_nodup(&fns, file);
+	}
+	if (!fns.nr)
 		die("unable to set up default path; use --file");
 
 	if (credential_read(&c, stdin) < 0)
-- 
2.1.4
Paul Tan· Mar 24, 2015, 05:20 UTC · re: Paul Tan · lore

[PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

t0302 now tests git-credential-store's support for the XDG user-specific configuration file $XDG_CONFIG_HOME/git/credentials. Specifically:

* Ensure that the XDG file is strictly opt-in. It should not be created
  by git at all times if it does not exist.
* Conversely, if the XDG file exists, ~/.git-credentials should
  not be created at all times.
* If both the XDG file and ~/.git-credentials exists, then both files
  should be used for credential lookups. However, credentials should
  only be written to ~/.git-credentials.
* Credentials must be erased from both files.
* $XDG_CONFIG_HOME can be a custom directory set by the user as per the
  XDG base directory specification. Test that git-credential-store
  respects that, but defaults to "~/.config/git/credentials" if it does
  not exist or is empty.
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
The previous version can be found at [1].
[1] http://thread.gmane.org/gmane.comp.version-control.git/265683/focus=265684
The changes are as follows:
* Fix broken && chains
* Rewrite test descriptions to be shorter
* Code formatting
* Fixed test "get: return credentials from home file if xdg files are
unreadable". It should actually be the reverse to follow the actual
precedence order.
Thanks Eric and Matthieu for the code review.

Matthieu and Eric: I know I said I will try to re-order the patches to put the tests before the implementation, but after thinking and trying to rewrite the commit messages I realised it seems really weird to me. In this patch series, the implementation is split across the first two patches. The first patch should use the old tests, and ideally, the new tests should be squashed with the second patch because it seems more logical to me to implement the tests at the same time as the new feature. However, since the tests patch is very long, to make it easier to review it is split into a separate patch which is applied after the implementation patches.

 t/t0302-credential-store.sh | 114 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)
Show changes to t/t0302-credential-store.sh +114 −0
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index f61b40c..4e1f8ec 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -6,4 +6,118 @@ test_description='credential-store tests'
 
 helper_test store
 
+test_expect_success 'when xdg file does not exist, xdg file not created' '
+	test_path_is_missing "$HOME/.config/git/credentials" &&
+	test -s "$HOME/.git-credentials"
+'
+
+test_expect_success 'setup xdg file' '
+	rm -f "$HOME/.git-credentials" &&
+	mkdir -p "$HOME/.config/git" &&
+	>"$HOME/.config/git/credentials"
+'
+
+helper_test store
+
+test_expect_success 'when xdg file exists, home file not created' '
+	test -s "$HOME/.config/git/credentials" &&
+	test_path_is_missing "$HOME/.git-credentials"
+'
+
+test_expect_success 'setup custom xdg file' '
+	rm -f "$HOME/.git-credentials" &&
+	rm -f "$HOME/.config/git/credentials" &&
+	mkdir -p "$HOME/xdg/git" &&
+	>"$HOME/xdg/git/credentials"
+'
+
+XDG_CONFIG_HOME="$HOME/xdg"
+export XDG_CONFIG_HOME
+helper_test store
+unset XDG_CONFIG_HOME
+
+test_expect_success 'if custom xdg file exists, home and xdg files not created' '
+	test_when_finished "rm -f $HOME/xdg/git/credentials" &&
+	test -s "$HOME/xdg/git/credentials" &&
+	test_path_is_missing "$HOME/.git-credentials" &&
+	test_path_is_missing "$HOME/.config/git/credentials"
+'
+
+test_expect_success 'get: use home file if both home and xdg files have matches' '
+	echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
+	mkdir -p "$HOME/.config/git" &&
+	echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+	check fill store <<-\EOF
+	protocol=https
+	host=example.com
+	--
+	protocol=https
+	host=example.com
+	username=home-user
+	password=home-pass
+	--
+	EOF
+'
+
+test_expect_success 'get: use xdg file if home file has no matches' '
+	>"$HOME/.git-credentials" &&
+	mkdir -p "$HOME/.config/git" &&
+	echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+	check fill store <<-\EOF
+	protocol=https
+	host=example.com
+	--
+	protocol=https
+	host=example.com
+	username=xdg-user
+	password=xdg-pass
+	--
+	EOF
+'
+
+test_expect_success 'get: use xdg file if home file is unreadable' '
+	echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
+	chmod -r "$HOME/.git-credentials" &&
+	mkdir -p "$HOME/.config/git" &&
+	echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+	check fill store <<-\EOF
+	protocol=https
+	host=example.com
+	--
+	protocol=https
+	host=example.com
+	username=xdg-user
+	password=xdg-pass
+	--
+	EOF
+'
+
+test_expect_success 'store: if both xdg and home files exist, only store in home file' '
+	>"$HOME/.git-credentials" &&
+	mkdir -p "$HOME/.config/git" &&
+	>"$HOME/.config/git/credentials" &&
+	check approve store <<-\EOF &&
+	protocol=https
+	host=example.com
+	username=store-user
+	password=store-pass
+	EOF
+	echo "https://store-user:store-pass@example.com" >expected &&
+	test_cmp expected "$HOME/.git-credentials" &&
+	test_must_be_empty "$HOME/.config/git/credentials"
+'
+
+
+test_expect_success 'erase: erase matching credentials from both xdg and home files' '
+	echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
+	mkdir -p "$HOME/.config/git" &&
+	echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+	check reject store <<-\EOF &&
+	protocol=https
+	host=example.com
+	EOF
+	test_must_be_empty "$HOME/.git-credentials" &&
+	test_must_be_empty "$HOME/.config/git/credentials"
+'
+
 test_done
-- 
2.1.4
Matthieu Moy· Mar 24, 2015, 09:52 UTC · re: Paul Tan · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

Paul Tan <pyokagan@gmail.com> writes:
Show 10 quoted lines
> Matthieu and Eric: I know I said I will try to re-order the patches to
> put the tests before the implementation, but after thinking and trying
> to rewrite the commit messages I realised it seems really weird to me.
> In this patch series, the implementation is split across the first two
> patches. The first patch should use the old tests, and ideally, the new
> tests should be squashed with the second patch because it seems more
> logical to me to implement the tests at the same time as the new
> feature. However, since the tests patch is very long, to make it easier
> to review it is split into a separate patch which is applied after the
> implementation patches.

No problem, your version is very good. I was pointing out alternatives, but not requesting a change, and your reasoning makes perfect sense.

I had reviewed v4 in details, and checked the diff between v4 and v5. The whole series is now

Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Thanks,
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Eric Sunshine· Mar 25, 2015, 06:54 UTC · re: Matthieu Moy · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

On Tue, Mar 24, 2015 at 5:52 AM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 20 quoted lines
> Paul Tan <pyokagan@gmail.com> writes:
>
>> Matthieu and Eric: I know I said I will try to re-order the patches to
>> put the tests before the implementation, but after thinking and trying
>> to rewrite the commit messages I realised it seems really weird to me.
>> In this patch series, the implementation is split across the first two
>> patches. The first patch should use the old tests, and ideally, the new
>> tests should be squashed with the second patch because it seems more
>> logical to me to implement the tests at the same time as the new
>> feature. However, since the tests patch is very long, to make it easier
>> to review it is split into a separate patch which is applied after the
>> implementation patches.
>
> No problem, your version is very good. I was pointing out alternatives,
> but not requesting a change, and your reasoning makes perfect sense.
>
> I had reviewed v4 in details, and checked the diff between v4 and v5.
> The whole series is now
>
> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
With the POSIXPERM issue[1] addressed (if necessary), patch 3/3 is also:
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
[1]: http://article.gmane.org/gmane.comp.version-control.git/266265
Junio C Hamano· Mar 25, 2015, 16:17 UTC · re: Eric Sunshine · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 7 quoted lines
> On Tue, Mar 24, 2015 at 5:52 AM, Matthieu Moy
> ...
>> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> With the POSIXPERM issue[1] addressed (if necessary), patch 3/3 is also:
>
> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

THanks for a review. Paul, the 3-patch series is already in 'next', so please fix these up with a follow-up patch.

Thanks.
Junio C Hamano· Mar 25, 2015, 20:25 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> On Tue, Mar 24, 2015 at 5:52 AM, Matthieu Moy
>> ...
>>> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>>
>> With the POSIXPERM issue[1] addressed (if necessary), patch 3/3 is also:
>>
>> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
>
> THanks for a review.  Paul, the 3-patch series is already in 'next',
> so please fix these up with a follow-up patch.
>
> Thanks.
I've already queued the following and merged it to 'next'.
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 25 Mar 2015 13:23:21 -0700
Subject: [PATCH] t0302: "unreadable" test needs POSIXPERM
Noticed and fixed by Eric Sunshine, confirmed by Johannes Sixt.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0302-credential-store.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/t0302-credential-store.sh +1 −1
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index 4e1f8ec..0979df9 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -75,7 +75,7 @@ test_expect_success 'get: use xdg file if home file has no matches' '
 	EOF
 '
 
-test_expect_success 'get: use xdg file if home file is unreadable' '
+test_expect_success POSIXPERM 'get: use xdg file if home file is unreadable' '
 	echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
 	chmod -r "$HOME/.git-credentials" &&
 	mkdir -p "$HOME/.config/git" &&
-- 
2.3.4-462-gc8eeafd
Paul Tan· Mar 26, 2015, 05:20 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

On Wed, Mar 25, 2015 at 01:25:07PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> I've already queued the following and merged it to 'next'.

Thanks Matthieu and Eric for your reviews, and Johannes for following up on this.

Will keep in view XDG support for ~/.git-credential-cache next because I don't like to leave things unfinished, unless we want to keep it around as a microproject. Perhaps home_config_paths() can also be simplified/removed because it hardcodes '~/.gitconfig'.

Regards, Paul

Matthieu Moy· Mar 26, 2015, 13:27 UTC · re: Paul Tan · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

Paul Tan <pyokagan@gmail.com> writes:
Show 11 quoted lines
> On Wed, Mar 25, 2015 at 01:25:07PM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> I've already queued the following and merged it to 'next'.
>
> Thanks Matthieu and Eric for your reviews, and Johannes for following up
> on this.
>
> Will keep in view XDG support for ~/.git-credential-cache next because I
> don't like to leave things unfinished, unless we want to keep it around
> as a microproject. Perhaps home_config_paths() can also be
> simplified/removed because it hardcodes '~/.gitconfig'.

I may be able to get some students to work on it in May-June (I teach in Ensimag, a french computer-science school and offer "contribute to open-source" as an end-of-year project), but no guarantee. If you want to do it before, just do it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Eric Sunshine· Mar 25, 2015, 06:42 UTC · re: Paul Tan · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

On Tue, Mar 24, 2015 at 1:20 AM, Paul Tan <pyokagan@gmail.com> wrote:
Show 34 quoted lines
> t0302 now tests git-credential-store's support for the XDG user-specific
> configuration file $XDG_CONFIG_HOME/git/credentials. Specifically:
>
> * Ensure that the XDG file is strictly opt-in. It should not be created
>   by git at all times if it does not exist.
>
> * Conversely, if the XDG file exists, ~/.git-credentials should
>   not be created at all times.
>
> * If both the XDG file and ~/.git-credentials exists, then both files
>   should be used for credential lookups. However, credentials should
>   only be written to ~/.git-credentials.
>
> * Credentials must be erased from both files.
>
> * $XDG_CONFIG_HOME can be a custom directory set by the user as per the
>   XDG base directory specification. Test that git-credential-store
>   respects that, but defaults to "~/.config/git/credentials" if it does
>   not exist or is empty.
>
> Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
> index f61b40c..4e1f8ec 100755
> --- a/t/t0302-credential-store.sh
> +++ b/t/t0302-credential-store.sh
> @@ -6,4 +6,118 @@ test_description='credential-store tests'
>
>  helper_test store
>
> +test_expect_success 'get: use xdg file if home file is unreadable' '

I meant to mention this earlier. Does this test need to be protected by the POSIXPERM prerequisite since it's using chmod?

    test_expect_success POSIXPERM 'get: ... unreadable' '
Otherwise, the test will likely fail on Windows.
Show 15 quoted lines
> +       echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
> +       chmod -r "$HOME/.git-credentials" &&
> +       mkdir -p "$HOME/.config/git" &&
> +       echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
> +       check fill store <<-\EOF
> +       protocol=https
> +       host=example.com
> +       --
> +       protocol=https
> +       host=example.com
> +       username=xdg-user
> +       password=xdg-pass
> +       --
> +       EOF
> +'
Johannes Sixt· Mar 25, 2015, 19:03 UTC · re: Eric Sunshine · lore

Re: [PATCH v5 3/3] t0302: test credential-store support for XDG_CONFIG_HOME

Am 25.03.2015 um 07:42 schrieb Eric Sunshine:
Show 42 quoted lines
> On Tue, Mar 24, 2015 at 1:20 AM, Paul Tan <pyokagan@gmail.com> wrote:
>> t0302 now tests git-credential-store's support for the XDG user-specific
>> configuration file $XDG_CONFIG_HOME/git/credentials. Specifically:
>>
>> * Ensure that the XDG file is strictly opt-in. It should not be created
>>    by git at all times if it does not exist.
>>
>> * Conversely, if the XDG file exists, ~/.git-credentials should
>>    not be created at all times.
>>
>> * If both the XDG file and ~/.git-credentials exists, then both files
>>    should be used for credential lookups. However, credentials should
>>    only be written to ~/.git-credentials.
>>
>> * Credentials must be erased from both files.
>>
>> * $XDG_CONFIG_HOME can be a custom directory set by the user as per the
>>    XDG base directory specification. Test that git-credential-store
>>    respects that, but defaults to "~/.config/git/credentials" if it does
>>    not exist or is empty.
>>
>> Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
>> Signed-off-by: Paul Tan <pyokagan@gmail.com>
>> ---
>> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
>> index f61b40c..4e1f8ec 100755
>> --- a/t/t0302-credential-store.sh
>> +++ b/t/t0302-credential-store.sh
>> @@ -6,4 +6,118 @@ test_description='credential-store tests'
>>
>>   helper_test store
>>
>> +test_expect_success 'get: use xdg file if home file is unreadable' '
>
> I meant to mention this earlier. Does this test need to be protected
> by the POSIXPERM prerequisite since it's using chmod?
>
>      test_expect_success POSIXPERM 'get: ... unreadable' '
>
> Otherwise, the test will likely fail on Windows.
Well spotted! The test indeed fails on Windows. POSIXPERM is required.
Show 15 quoted lines
>> +       echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
>> +       chmod -r "$HOME/.git-credentials" &&
>> +       mkdir -p "$HOME/.config/git" &&
>> +       echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
>> +       check fill store <<-\EOF
>> +       protocol=https
>> +       host=example.com
>> +       --
>> +       protocol=https
>> +       host=example.com
>> +       username=xdg-user
>> +       password=xdg-pass
>> +       --
>> +       EOF
>> +'
-- Hannes

← back to recent threads