From: Ævar Arnfjörð Bjarmason Date: Sat, 03 Jul 2010 22:14:53 GMT Subject: Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication Message-ID: In-Reply-To: On Sat, Jul 3, 2010 at 21:21, Áshin László wrote: >  Documentation/git-cvsserver.txt |   46 +++++++++++++++++++++++++++++++++++--- >  git-cvsserver.perl              |   28 +++++++++++++++++++++++ >  t/t9400-git-cvsserver-server.sh |   45 ++++++++++++++++++++++++++++++++++++++ >  3 files changed, 115 insertions(+), 4 deletions(-) > > diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt > index 7004dd2..b3c3122 100644 > --- a/Documentation/git-cvsserver.txt > +++ b/Documentation/git-cvsserver.txt > @@ -100,10 +100,48 @@ looks like >  ------ > >  Only anonymous access is provided by pserve by default. To commit you I think it's always called "pserver" with an -r. > -will have to create pserver accounts, simply add a gitcvs.authdb > -setting in the config file of the repositories you want the cvsserver > -to allow writes to, for example: > +will have to specify an authentication option in the config file. > +Currently there are two options are available for authentication through > +pserver in git-cvsserver: one through an authenticator script and an other > +through a textual authentication database. If both are specified in the config > +file then the script based solution will be used. If both are specified shouldn't we throw an error? > +  a. To use the authentication script based method, simply add a > +     gitcvs.authscript setting in the config file of the repositories you want > +     the cvsserver to allow writes to, for example: > ++ > +-- > +------ > + > +   [gitcvs] > +    authscript = /usr/local/bin/cvsserver-auth.sh > + > +------ > +The file specified here must be executable by the user the git-cvsserver runs > +under the name of. This script has to read exactly two lines from its standard "the name of" is redundant here. > +input as long as git-cvsserver passes the username and the password on separate > +lines. After the script did its task by checking the username and password > +given it has to return zero if the authentication was successful and return > +other value if it was not. Better as: "The script will receive two lines on standard input, the first is the username and the second is the password. It should return 0 if the user was successfully authenticated, and a non-zero value if not". > + > +Here is an example for an authentication script which checks the users against > +active directory: > +------ > +#!/bin/sh > +# /usr/local/bin/cvsserver-auth.sh > > +read username > +read password > + > +wbinfo -a "${username}%${password}" Do all POSIX shells implicitly exit with the return value of the last statement they evaluate. I don't know. > +------ > +-- > + > +  b. To use the authentication database based method, simply add a > +     gitcvs.authdb setting in the config file of the repositories you want the > +     cvsserver to allow writes to, for example: > ++ > +-- >  ------ > >    [gitcvs] > @@ -125,7 +163,7 @@ Alternatively you can produce the password with > perl's crypt() operator: >  ----- >    perl -e 'my ($user, $pass) = @ARGV; printf "%s:%s\n", $user, > crypt($user, $pass)' $USER password >  ----- > - > +-- >  Then provide your password via the pserver method, for example: >  ------ >    cvs -d:pserver:someuser:somepassword server/path/repo.git co > diff --git a/git-cvsserver.perl b/git-cvsserver.perl > index e9f3037..9664e86 100755 > --- a/git-cvsserver.perl > +++ b/git-cvsserver.perl > @@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') { >         } > >         # Fall through to LOVE > +    } elsif (exists $cfg->{gitcvs}->{authscript}) { > +        my $authscript = $cfg->{gitcvs}->{authscript}; > + > +        unless (-x $authscript) { > +            print "E The authentication script specified in "; > +            print "[gitcvs.authscript] cannot be executed\n"; I *think* you have to prefix anything that's not "I (HATE|LOVE) YOU" with "E " if it's an error. I.e. this should probably be: print "E The authentication script specified in"; print "E [gitcvs.authscript] cannot be executed\n"; But that's just my hazy memory. Maybe CVS clients will report the error anyway. > +            print "I HATE YOU\n"; > +            exit 1; > +        } > + > +        open my $script_fd, '|-', "'$authscript'" > +            or die "Couldn't open authentication script '$authscript': $!"; > + > +        if (length($password) > 0) { > +            $password = descramble($password); > +        } > + > +        print $script_fd "$user\n"; > +        print $script_fd "$password\n"; > +        close $script_fd; > + > +        unless ($? == 0) { > +            print "E External script authentication failed.\n"; > +            print "I HATE YOU\n"; > +            exit 1; > +        } > + > +        # Fall through to LOVE >     } else { >         # Trying to authenticate a user >         if (not exists $cfg->{gitcvs}->{authdb}) { > diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh > index 8639506..0743e9a 100755 > --- a/t/t9400-git-cvsserver-server.sh > +++ b/t/t9400-git-cvsserver-server.sh > @@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \ >  # PSERVER AUTHENTICATION >  #------------------------ > > +cat >"$SERVERDIR/authscript.sh" < +#!/bin/sh > +read username > +read password > + > +test "x\$username" = xcvsuser -a "x\$password" = xcvspassword This is offtopic. But I've never figured out why you need to do test "x$foo" = "xbar" As opposed to: test "$foo" = "bar" In shellscript. Are there really some shells that get equality tests where one term is "" wrong? > +EOF > + > +chmod a+x "$SERVERDIR/authscript.sh" > + >  cat >request-anonymous  <  BEGIN AUTH REQUEST >  $SERVERDIR > @@ -134,6 +144,41 @@ test_expect_success 'pserver authentication > failure (login/non-anonymous user)' >    fi && >    sed -ne \$p log | grep "^I HATE YOU\$"' > > +GIT_DIR="$SERVERDIR" git config --unset gitcvs.authdb || exit 1 > +GIT_DIR="$SERVERDIR" git config gitcvs.authscript > "$SERVERDIR/authscript.sh" || exit 1 > + > +test_expect_success 'pserver authentication (authscript)' \ > +  'cat request-anonymous | git-cvsserver pserver >log 2>&1 && > +   sed -ne \$p log | grep "^I LOVE YOU\$"' > + > +test_expect_success 'pserver authentication failure (authscript, > non-anonymous user)' \ > +  'if cat request-git | git-cvsserver pserver >log 2>&1 > +   then > +       false > +   else > +       true > +   fi && This should probably be (untested): test_must_fail cat request-git git-cvsserver pserver >log 2>&1 && > +   sed -ne \$p log | grep "^I HATE YOU\$"' > + > +test_expect_success 'pserver authentication success (authscript, > non-anonymous user with password)' \ > +  'cat login-git-ok | git-cvsserver pserver >log 2>&1 && > +   sed -ne \$p log | grep "^I LOVE YOU\$"' > + > +test_expect_success 'pserver authentication (authscript, login)' \ > +  'cat login-anonymous | git-cvsserver pserver >log 2>&1 && > +   sed -ne \$p log | grep "^I LOVE YOU\$"' > + > +test_expect_success 'pserver authentication failure (authscript, > login/non-anonymous user)' \ > +  'if cat login-git | git-cvsserver pserver >log 2>&1 > +   then > +       false > +   else > +       true > +   fi && > +   sed -ne \$p log | grep "^I HATE YOU\$"' > + > +GIT_DIR="$SERVERDIR" git config --unset gitcvs.authscript || exit 1 > +GIT_DIR="$SERVERDIR" git config gitcvs.authdb "$SERVERDIR/auth.db" || exit 1 > >  # misuse pserver authentication for testing of req_Root Otherwise looking good.