[RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of using

From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: git@vger.kernel.org
Cc: dirk@ed4u.de, sunshine@sunshineco.com, peff@peff.net,
	gitster@pobox.com, jrnieder@gmail.com,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Subject: [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of using
Date: Wed, 29 Apr 2020 13:35:46 -0700	[thread overview]
Message-ID: <20200429203546.56753-3-carenas@gmail.com> (raw)
In-Reply-To: <20200429203546.56753-1-carenas@gmail.com>

originally any credential found was tried for matching as far as it had
a username and password, but that resulted in fatal errors as the rules
were harden.

now that we have a way to report malformed credentials, use it to notify
the user when username/password was missing, instead of just silently
skipping.

do the same for credentials that are missing host (or had one that is
empty) or that are missing a path (for supporting cert://) as well.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 credential-store.c          |  7 ++++---
 t/t0302-credential-store.sh | 38 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/credential-store.c b/credential-store.c
index 1cc5ca081a..53f77ff6f5 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -26,9 +26,10 @@ static int parse_credential_file(const char *fn,
 
 	while (strbuf_getline_lf(&line, fh) != EOF) {
 		lineno++;
-		if (!credential_from_url_gently(&entry, line.buf, 1)) {
-			if (entry.username && entry.password &&
-				credential_match(c, &entry)) {
+		if (!credential_from_url_gently(&entry, line.buf, 1) &&
+			((entry.host && *entry.host) || entry.path) &&
+			entry.username && entry.password) {
+			if (credential_match(c, &entry)) {
 				found_credential = 1;
 				if (match_cb) {
 					match_cb(&entry);
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index 801c1eb200..3150f304cb 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -139,6 +139,44 @@ test_expect_success 'get: credentials without scheme are invalid' '
 	test_i18ngrep "ignoring invalid credential" stderr
 '
 
+test_expect_success 'get: credentials without valid host/path are invalid' '
+	echo "https://user:pass@" >"$HOME/.git-credentials" &&
+	cat >expect-stdout <<-\STDOUT &&
+	protocol=https
+	host=example.com
+	username=askpass-username
+	password=askpass-password
+	STDOUT
+	test_config credential.helper store &&
+	git credential fill <<-\EOF >stdout 2>stderr &&
+	protocol=https
+	host=example.com
+	EOF
+	test_cmp expect-stdout stdout &&
+	grep "askpass: Username for '\''https://example.com'\'':" stderr &&
+	grep "askpass: Password for '\''https://askpass-username@example.com'\'':" stderr &&
+	test_i18ngrep "ignoring invalid credential" stderr
+'
+
+test_expect_success 'get: credentials without username/password are invalid' '
+	echo "https://pass@example.com" >"$HOME/.git-credentials" &&
+	cat >expect-stdout <<-\STDOUT &&
+	protocol=https
+	host=example.com
+	username=askpass-username
+	password=askpass-password
+	STDOUT
+	test_config credential.helper store &&
+	git credential fill <<-\EOF >stdout 2>stderr &&
+	protocol=https
+	host=example.com
+	EOF
+	test_cmp expect-stdout stdout &&
+	grep "askpass: Username for '\''https://example.com'\'':" stderr &&
+	grep "askpass: Password for '\''https://askpass-username@example.com'\'':" stderr &&
+	test_i18ngrep "ignoring invalid credential" stderr
+'
+
 test_expect_success 'get: store file can contain empty/bogus lines' '
 	echo "" > "$HOME/.git-credentials" &&
 	q_to_tab <<-\CONFIG >>"$HOME/.git-credentials" &&
-- 
2.26.2.569.g1d74ac4d14

