git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michal Nazarewicz <mpn@google.com>
Cc: Jeff King <peff@peff.net>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	git@vger.kernel.org
Subject: Re: [PATCHv2 5/5] git-send-email: use git credential to obtain password
Date: Thu, 07 Feb 2013 10:42:24 -0800	[thread overview]
Message-ID: <7vd2wc7ypb.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <0b3c9b66ccb6c8343dafd210b82c7765891d3785.1360242782.git.mina86@mina86.com> (Michal Nazarewicz's message of "Thu, 7 Feb 2013 15:01:21 +0100")

Michal Nazarewicz <mpn@google.com> writes:

> From: Michal Nazarewicz <mina86@mina86.com>
>
> If smtp_user is provided but smtp_pass is not, instead of
> prompting for password, make git-send-email use git
> credential command instead.
>
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> ---

Nice ;-)

I'd expect reviews on 4/5 from Peff and Matthiew which may result in
either Reviewed-by:'s or another round, but everything else looks in
good order.

Thanks to all three of you for working on this.

>  Documentation/git-send-email.txt |  4 +--
>  git-send-email.perl              | 59 +++++++++++++++++++++++-----------------
>  2 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 44a1f7c..0cffef8 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -164,8 +164,8 @@ Sending
>  Furthermore, passwords need not be specified in configuration files
>  or on the command line. If a username has been specified (with
>  '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
> -specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
> -user is prompted for a password while the input is masked for privacy.
> +specified (with '--smtp-pass' or 'sendemail.smtppass'), then
> +a password is obtained using 'git-credential'.
>  
>  --smtp-server=<host>::
>  	If set, specifies the outgoing SMTP server to use (e.g.
> diff --git a/git-send-email.perl b/git-send-email.perl
> index be809e5..76bbfc3 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1045,6 +1045,39 @@ sub maildomain {
>  	return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
>  }
>  
> +# Returns 1 if authentication succeeded or was not necessary
> +# (smtp_user was not specified), and 0 otherwise.
> +
> +sub smtp_auth_maybe {
> +	if (!defined $smtp_authuser || $auth) {
> +		return 1;
> +	}
> +
> +	# Workaround AUTH PLAIN/LOGIN interaction defect
> +	# with Authen::SASL::Cyrus
> +	eval {
> +		require Authen::SASL;
> +		Authen::SASL->import(qw(Perl));
> +	};
> +
> +	# TODO: Authentication may fail not because credentials were
> +	# invalid but due to other reasons, in which we should not
> +	# reject credentials.
> +	$auth = Git::credential({
> +		'protocol' => 'smtp',
> +		'host' => join(':', $smtp_server, $smtp_server_port),
> +		'username' => $smtp_authuser,
> +		# if there's no password, "git credential fill" will
> +		# give us one, otherwise it'll just pass this one.
> +		'password' => $smtp_authpass
> +	}, sub {
> +		my $cred = shift;
> +		return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
> +	});
> +
> +	return $auth;
> +}
> +
>  # Returns 1 if the message was sent, and 0 otherwise.
>  # In actuality, the whole program dies when there
>  # is an error sending a message.
> @@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
>  			    defined $smtp_server_port ? " port=$smtp_server_port" : "";
>  		}
>  
> -		if (defined $smtp_authuser) {
> -			# Workaround AUTH PLAIN/LOGIN interaction defect
> -			# with Authen::SASL::Cyrus
> -			eval {
> -				require Authen::SASL;
> -				Authen::SASL->import(qw(Perl));
> -			};
> -
> -			if (!defined $smtp_authpass) {
> -
> -				system "stty -echo";
> -
> -				do {
> -					print "Password: ";
> -					$_ = <STDIN>;
> -					print "\n";
> -				} while (!defined $_);
> -
> -				chomp($smtp_authpass = $_);
> -
> -				system "stty echo";
> -			}
> -
> -			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> -		}
> +		smtp_auth_maybe or die $smtp->message;
>  
>  		$smtp->mail( $raw_from ) or die $smtp->message;
>  		$smtp->to( @recipients ) or die $smtp->message;

      reply	other threads:[~2013-02-07 18:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-07 14:01 [PATCHv2 0/5] Make git-send-email use git-credential Michal Nazarewicz
2013-02-07 14:01 ` [PATCHv2 1/5] Git.pm: allow command_close_bidi_pipe to be called as method Michal Nazarewicz
2013-02-07 14:01 ` [PATCHv2 2/5] Git.pm: fix example in command_close_bidi_pipe documentation Michal Nazarewicz
2013-02-07 14:01 ` [PATCHv2 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe Michal Nazarewicz
2013-02-07 14:01 ` [PATCHv2 4/5] Git.pm: add interface for git credential command Michal Nazarewicz
2013-02-07 18:46   ` Matthieu Moy
2013-02-07 23:38     ` Junio C Hamano
2013-02-08  1:37       ` Michal Nazarewicz
2013-02-08  5:11   ` Jeff King
2013-02-07 14:01 ` [PATCHv2 5/5] git-send-email: use git credential to obtain password Michal Nazarewicz
2013-02-07 18:42   ` Junio C Hamano [this message]

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=7vd2wc7ypb.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=mpn@google.com \
    --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).