[PATCH] Uses git-credential for git-imap-send

From

DADan Albert <danalbert@google.com>

Date

Apr 27, 2014, 17:58 UTC

Message-ID

<907d1444a8e17a43c03ee0c2bb04038bdd3372b8.1398620231.git.danalbert@google.com>

In-Reply-To

<20140426180835.GE21493@sigill.intra.peff.net>

Signed-off-by: Dan Albert <danalbert@google.com>
---

>> About imap vs. imaps: I actually had your exact line in before, but decided
>> that as long as its for the same host the user probably wants to use the
>> same credentials for both imap and imaps (if they for some reason had both
>> configured). Hard coding "imap" allows them to use either protocol with
>> only one keychain entry. The use case is a stretch, but it doesn't do any
>> harm to implement it this way.
>
> My concerns with conflating the two are:
> 
>   1. The system helper might care about the distinction and prefer imaps
>      (e.g., it might already have the credential stored for your regular
>      mail client, which uses imaps). But osxkeychain is the only helper
>      that makes the distinction, and I don't really know how OS X's
>      keychain code handles the distinction.
> 
>   2. With http and https, we are careful to make the distinction,
>      because we would not want to accidentally share a credential over http
>      that was stored via https. But it's pretty easy to use an http URL
>      rather than an https one. It's probably pretty rare to accidentally
>      turn off imap SSL.
> 
> So I'd be OK with leaving it as "imap" for now, and waiting for somebody
> to actually come up with a real case where the distinction matters.

These are good points. I've made the change.

 imap-send.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 0bc6f7f..112fc83 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -23,9 +23,9 @@
  */
 
 #include "cache.h"
+#include "credential.h"
 #include "exec_cmd.h"
 #include "run-command.h"
-#include "prompt.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #endif
@@ -946,6 +946,7 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha
 
 static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
 {
+	struct credential cred = CREDENTIAL_INIT;
 	struct imap_store *ctx;
 	struct imap *imap;
 	char *arg, *rsp;
@@ -1101,19 +1102,11 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
 			goto bail;
 		}
 		if (!srvc->pass) {
-			struct strbuf prompt = STRBUF_INIT;
-			strbuf_addf(&prompt, "Password (%s@%s): ", srvc->user, srvc->host);
-			arg = git_getpass(prompt.buf);
-			strbuf_release(&prompt);
-			if (!*arg) {
-				fprintf(stderr, "Skipping account %s@%s, no password\n", srvc->user, srvc->host);
-				goto bail;
-			}
-			/*
-			 * getpass() returns a pointer to a static buffer.  make a copy
-			 * for long term storage.
-			 */
-			srvc->pass = xstrdup(arg);
+			cred.username = xstrdup(srvc->user);
+			cred.protocol = xstrdup(srvc->use_ssl ? "imaps" : "imap");
+			cred.host = xstrdup(srvc->host);
+			credential_fill(&cred);
+			srvc->pass = xstrdup(cred.password);
 		}
 		if (CAP(NOLOGIN)) {
 			fprintf(stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host);
@@ -1153,10 +1146,18 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
 		}
 	} /* !preauth */
 
+	if (cred.username)
+		credential_approve(&cred);
+	credential_clear(&cred);
+
 	ctx->prefix = "";
 	return ctx;
 
 bail:
+	if (cred.username)
+		credential_reject(&cred);
+	credential_clear(&cred);
+
 	imap_close_store(ctx);
 	return NULL;
 }

-- 
2.0.0.rc1.1.gce060f5