Hello Eric, all,
thanks for comments, the coding style will be fixed
in the next version (I cannot find a way how to set
vim to help me with those if<SPACE>( issues. I always/often
forget it when writing so I never do it to be consistent.).
Do I understand well that you are complaining about too
narrow commmit message?
I am trying to figure out how to write a test. It is
not very clear to me, what the testing suite does. My
attempt looks this way at the moment:
1657 do_smtp_auth_test() {
1658 git send-email \
1659 --from="Example <nobody@xxxxxxxxxxx>" \
1660 --to=someone@xxxxxxxxxxx \
1661 --smtp-server="$(pwd)/fake.sendmail" \
1662 --smtp-auth="$1" \
1663 -v \
1664 0001-*.patch \
1665 2>errors >out
1666 }
1667
1668 test_expect_success $PREREQ 'accepts SMTP AUTH mechanisms (see RFC-4422, p. 8)' '
1669 do_smtp_auth_test "PLAIN LOGIN CRAM-MD5 DIGEST-MD5 GSSAPI EXTERNAL ANONYMOUS" &&
1670 do_smtp_auth_test "ABCDEFGHIKLMNOPQRSTUVWXYZ 0123456789_-"
1671 '
1672
1673 test_expect_success $PREREQ 'does not accept non-RFC-4422 strings for SMTP AUTH' '
1674 test_must_fail do_smtp_auth_test "../ATTACK" &&
1675 test_must_fail do_smtp_auth_test "TOO-LONG-BUT-VALID-STRING" &&
1676 test_must_fail do_smtp_auth_test "no-lower-case-sorry"
1677 '
* I do not know yet, what to check after each do_smtp_auth_test call.
* Perhaps, each case should have its own test_expect_success call?
* Why send-email -v does not generate any output?
(I found a directory 'trash directory.t9001-send-email', however, the
errors file is always empty.)
* Is there any other place where the files out, errors are placed?
* I have no idea what the fake.sendmail does (I could see its contents
but still...). Is it suitable for my tests?
* Should I check the behaviour '--smtp-auth overrides
sendemail.smtpAuth'?
Regards
Jan
On Sun, 2 Aug 2015 14:57:19 -0400
Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sun, Aug 2, 2015 at 12:42 PM, Jan Viktorin
> <viktorin@xxxxxxxxxxxxxx> wrote:
> > When sending an e-mail, the client and server must
> > agree on an authentication mechanism. Some servers
> > (due to misconfiguration or a bug) deny valid
> > credentials for certain mechanisms. In this patch,
> > a new option --smtp-auth and configuration entry
> > smtpauth are introduced. If smtp_auth is defined,
> > it works as a whitelist of allowed mechanisms for
> > authentication selected from the ones supported by
> > the installed SASL perl library.
>
> Nit: This would read a bit more nicely if wrapped to 70-72 columns.
>
> > Signed-off-by: Jan Viktorin <viktorin@xxxxxxxxxxxxxx>
> > ---
> > diff --git a/Documentation/git-send-email.txt
> > b/Documentation/git-send-email.txt index 7ae467b..c237c80 100644
> > --- a/Documentation/git-send-email.txt
> > +++ b/Documentation/git-send-email.txt
> > @@ -171,6 +171,14 @@ Sending
> > +--smtp-auth=<mechs>::
> > + Specify allowed SMTP-AUTH mechanisms. This setting forces
> > using only
> > + the listed mechanisms. Separate allowed mechanisms by a
> > whitespace.
>
> Perhaps:
>
> Whitespace-separated list of allowed SMTP-AUTH mechanisms.
>
> > + Example: PLAIN LOGIN GSSAPI. If at least one of the
> > specified mechanisms
> > + matchs those advertised by the SMTP server and it is
> > supported by the SASL
>
> s/matchs/matches/
>
> > + library we use, it is used for authentication. If neither
> > of 'sendemail.smtpAuth'
> > + or '--smtp-auth' is specified, all mechanisms supported on
> > client can be used.
>
> s/neither of/neither/
> s/or/nor/
>
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index ae9f869..ebc1e90 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -75,6 +75,9 @@ git send-email [options] <file | directory |
> > rev-list options > Pass an empty string to disable certificate
> > verification.
> > --smtp-domain <str> * The domain name sent to
> > HELO/EHLO handshake
> > + --smtp-auth <str> * Space separated list of
> > allowed AUTH methods.
>
> s/Space separated/Space-separated/
>
> > + This setting forces to use
> > one of the listed methods.
> > + Supported: PLAIN LOGIN
> > CRAM-MD5 DIGEST-MD5.
>
> Since you're no longer checking explicitly for these mechanisms, you
> probably want to drop the "Supported:" line.
>
> > --smtp-debug <0|1> * Disable, enable Net::SMTP
> > debug.
> >
> > Automating:
> > @@ -1136,6 +1141,10 @@ sub smtp_auth_maybe {
> > Authen::SASL->import(qw(Perl));
> > };
> >
> > + if($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
> > + die "invalid smtp auth: '${smtp_auth}'";
> > + }
>
> Style: space after 'if'
>
> > # TODO: Authentication may fail not because credentials were
> > # invalid but due to other reasons, in which we should not
> > # reject credentials.
> > @@ -1148,6 +1157,20 @@ sub smtp_auth_maybe {
> > 'password' => $smtp_authpass
> > }, sub {
> > my $cred = shift;
> > +
> > + if($smtp_auth) {
>
> Style: space after 'if'
>
> > + my $sasl = Authen::SASL->new(
> > + mechanism => $smtp_auth,
> > + callback => {
> > + user => $cred->{'username'},
> > + pass => $cred->{'password'},
> > + authname =>
> > $cred->{'username'},
> > + }
> > + );
> > +
> > + return !!$smtp->auth($sasl);
> > + }
> > +
> > return !!$smtp->auth($cred->{'username'},
> > $cred->{'password'}); });
> >
> > --
> > 2.5.0
--
Jan Viktorin E-mail: Viktorin@xxxxxxxxxxxxxx
System Architect Web: www.RehiveTech.com
RehiveTech Phone: +420 606 201 868
Brno, Czech Republic
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html