From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Fri, 02 Jul 2010 14:39:40 GMT
Subject: Re: [PATCH] git-cvsserver: pserver-auth-script
Message-ID: <AANLkTinGZsCLF8CFKgQJaB63Jh9anGfQK4ZSHFPl4R4N@mail.gmail.com>
In-Reply-To: <20100702095406.7af15d20@ashinlaszlo.pta.neti.hu>

On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@neti.hu> wrote:

Disregard the last E-Mail. I bumped into the wrong button on my
keyboard.

> The following patch makes git-cvsserver capable of authenticating users through an external executable script using pserver method.
> The script can be specified in the gitcvs section of the config file:
> [gitcvs]
>        enabled = 1
>        authscript = /some/where/script.sh
>
> The script, itself will get username and password on its standard input, so it can look like something like this:
>
> #!/bin/sh
> read username
> read password
>
> wbinfo -a "$username%$password"
>
> --
> Only a return value of zero means a successful authentication.
>
> Please comment and keep me on cc.

Good to see someone use the pserver auth code I added, even though I'm
not doing so.

The idea looks good, please send another patch that adds documentation
to git-cvsserver.txt too.

> diff -ruN a/git-cvsserver b/git-cvsserver
> --- a/git-cvsserver     2010-07-01 15:31:18.000000000 +0200
> +++ b/git-cvsserver     2010-07-01 15:35:41.000000000 +0200

Why isn't this a patch against git-cvsserver.perl? Presumably you made
it without using the Git tools. It doesn't apply like this.

> @@ -200,35 +200,54 @@
>         # Fall through to LOVE
>     } else {
>         # Trying to authenticate a user
> -        if (not exists $cfg->{gitcvs}->{authdb}) {
> -            print "E the repo config file needs a [gitcvs] section with an 'authdb' parameter set to the filename of the authentication database\n";
> -            print "I HATE YOU\n";
> -            exit 1;
> -        }
> -
> -        my $authdb = $cfg->{gitcvs}->{authdb};
> -
> -        unless (-e $authdb) {
> -            print "E The authentication database specified in [gitcvs.authdb] does not exist\n";
> -            print "I HATE YOU\n";
> -            exit 1;
> -        }
> -
> -        my $auth_ok;
> -        open my $passwd, "<", $authdb or die $!;
> -        while (<$passwd>) {
> -            if (m{^\Q$user\E:(.*)}) {
> -                if (crypt($user, descramble($password)) eq $1) {
> -                    $auth_ok = 1;
> -                }
> -            };
> -        }
> -        close $passwd;
> +        if (exists $cfg->{gitcvs}->{authscript}) {
> +            my $authscript = $cfg->{gitcvs}->{authscript};
> +            unless (-x $authscript) {
> +                print "E The authentication script specified in [gitcvs.authscript] cannot be executed\n";
> +                print "I HATE YOU\n";
> +                exit 1;
> +            }
> +
> +            open SCRIPTIN, '|' . $authscript or die $!;
> +            print SCRIPTIN $user . "\n";
> +            print SCRIPTIN descramble($password) . "\n";
> +            close SCRIPTIN;

Nit: Nice use of three-arg open, but you should use lexical
filehandles instead. I.e.:

    open my $script, '|' . $authscript or die $!;
    ...

> +            if ($? != 0) {
> +                print "E External script authentication failed.\n";
> +                print "I HATE YOU\n";
> +                exit 1;
> +            }
> +        } else {
> +            if (not exists $cfg->{gitcvs}->{authdb}) {
> +                print "E the repo config file needs a [gitcvs] section with an 'authdb' parameter set to the filename of the authentication database\n";
> +                print "I HATE YOU\n";
> +                exit 1;
> +            }
> +
> +            my $authdb = $cfg->{gitcvs}->{authdb};
> +
> +            unless (-e $authdb) {
> +                print "E The authentication database specified in [gitcvs.authdb] does not exist\n";
> +                print "I HATE YOU\n";
> +                exit 1;
> +            }
> +
> +            my $auth_ok;
> +            open my $passwd, "<", $authdb or die $!;
> +            while (<$passwd>) {
> +                if (m{^\Q$user\E:(.*)}) {
> +                    if (crypt($user, descramble($password)) eq $1) {
> +                        $auth_ok = 1;
> +                    }
> +                };
> +            }
> +            close $passwd;
>
> -        unless ($auth_ok) {
> -            print "I HATE YOU\n";
> -            exit 1;
> -        }
> +            unless ($auth_ok) {
> +                print "I HATE YOU\n";
> +                exit 1;
> +            }
> +       }
>
>         # Fall through to LOVE
>     }

Otherwise this looks good. Submit something that's against the *.perl
(and uses git format-patch / git send-email .. ) & has docs and I'll
ack it.