git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Albert <danalbert@google.com>
To: git@vger.kernel.org
Cc: Dan Albert <danalbert@google.com>, peff@peff.net
Subject: [PATCH] Uses git-credential for git-imap-send
Date: Mon, 28 Apr 2014 20:00:04 -0700	[thread overview]
Message-ID: <c3bb0fb7f87e6ada5c73923b14d66c743a76caa6.1398739667.git.danalbert@google.com> (raw)
In-Reply-To: <20140428192349.GC25993@sigill.intra.peff.net>

git-imap-send was directly prompting for a password rather than using
git-credential. git-send-email, on the other hand, supports git-credential.

This is a necessary improvement for users that use two factor authentication, as
they should not be expected to remember all of their app specific passwords.

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

> I noticed that we are just filling in the password here, since we'll
> always fill cred.username from srvc->user. The lines directly above are:
> 
>         if (!srvc->user) {
>                 fprintf(stderr, "Skipping server %s, no user\n", srvc->host);
>                 goto bail;
>         }
> 
> That comes from the imap.user config variable. I wonder if we should
> just pass it off to credential_fill() in this case, too, which will fill
> in the username if necessary.
> 
> It probably doesn't matter much, though, as nobody is complaining. And
> if we were designing from scratch, I would say that "imap.user" and
> "imap.pass" would not need to exist, as you can configure
> "credential.imaps://host/.*" for the same purpose. But since we would
> have to keep supporting them anyway for compatibility, it's not worth
> trying to transition.

Yeah, doubtful anyone cares, but it's simple enough to do.

 imap-send.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 0bc6f7f..5c4f336 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;
@@ -1096,25 +1097,23 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
 		}
 #endif
 		imap_info("Logging in...\n");
-		if (!srvc->user) {
-			fprintf(stderr, "Skipping server %s, no user\n", srvc->host);
-			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);
+		if (!srvc->user || !srvc->pass) {
+			cred.protocol = xstrdup(srvc->use_ssl ? "imaps" : "imap");
+			cred.host = xstrdup(srvc->host);
+
+			if (srvc->user)
+				cred.username = xstrdup(srvc->user);
+			if (srvc->pass)
+				cred.password = xstrdup(srvc->pass);
+
+			credential_fill(&cred);
+
+			if (!srvc->user)
+				srvc->user = xstrdup(cred.username);
+			if (!srvc->pass)
+				srvc->pass = xstrdup(cred.password);
 		}
+
 		if (CAP(NOLOGIN)) {
 			fprintf(stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host);
 			goto bail;
@@ -1153,10 +1152,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

  reply	other threads:[~2014-04-29  3:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-26 17:50 [PATCH] Uses git-credential for git-imap-send Dan Albert
2014-04-26 18:08 ` Jeff King
     [not found]   ` <CAFVaGhuXvZhRCHRFurKNOC4tsiQ7WZnGb2CbRnoSSYg=XknJtg@mail.gmail.com>
2014-04-27  7:51     ` Jeff King
2014-04-27 17:58   ` Dan Albert
2014-04-28 19:23     ` Jeff King
2014-04-29  3:00       ` Dan Albert [this message]
2014-04-29  3:05         ` Jeff King
2014-04-29 17:19           ` 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=c3bb0fb7f87e6ada5c73923b14d66c743a76caa6.1398739667.git.danalbert@google.com \
    --to=danalbert@google.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).