* [PATCH 1/4] git-cvsserver: implement script based pserver authentication [not found] <1058578005711783867@unknownmsgid> @ 2010-07-03 21:21 ` Áshin László 2010-07-03 22:14 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 4+ messages in thread From: Áshin László @ 2010-07-03 21:21 UTC (permalink / raw) To: git 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 -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. + + 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 +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. + +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}" +------ +-- + + 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 <at> server/path/repo.git co <HEAD_name> 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"; + 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" <<EOF +#!/bin/sh +read username +read password + +test "x\$username" = xcvsuser -a "x\$password" = xcvspassword +EOF + +chmod a+x "$SERVERDIR/authscript.sh" + cat >request-anonymous <<EOF 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 && + 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 -- 1.7.2.rc1.dirty ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication 2010-07-03 21:21 ` [PATCH 1/4] git-cvsserver: implement script based pserver authentication Áshin László @ 2010-07-03 22:14 ` Ævar Arnfjörð Bjarmason 2010-07-04 8:51 ` Áshin László 0 siblings, 1 reply; 4+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-03 22:14 UTC (permalink / raw) To: Áshin László; +Cc: git On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@gmail.com> 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 <at> server/path/repo.git co <HEAD_name> > 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" <<EOF > +#!/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 <<EOF > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication 2010-07-03 22:14 ` Ævar Arnfjörð Bjarmason @ 2010-07-04 8:51 ` Áshin László 2010-07-04 12:22 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 4+ messages in thread From: Áshin László @ 2010-07-04 8:51 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Hi, On Sun, Jul 4, 2010 at 00:14, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@gmail.com> 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. Yes but it is not part of my changeset. >> -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? Actually a check can be added to the code. OK, I will do that. >> + 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". OK, a little bit I overcomplicated it :) >> + >> +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. "The exit status of a sequential list *shall* be the exit status of the last command in the list." ( After registration, http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html#tag_02_08 ) Generally, all kind of list constructions use this principle according to this page. Terminology of "shall": "For an implementation that conforms to IEEE Std 1003.1-2001, describes a feature or behavior that is mandatory. *An application can rely on the existence of the feature or behavior*. For an application or user, describes a behavior that is mandatory." >> +------ >> +-- >> + >> + 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 <at> server/path/repo.git co <HEAD_name> >> 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. Please note that there is no "\n" after the first line. This is only a line break in the source code. Should I have used the following? print "E The authentication script specified in" . "E [gitcvs.authscript] cannot be executed\n"; >> + 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" <<EOF >> +#!/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? Maybe this is too much. Maybe x was used only where " couldn't be: test x$foo = xbar So, if $foo is empty we have to make sure that test gets something to both side of the equation mark. But if quotation marks can be used it is possibly unneeded: test "$foo" = "bar" My solution is an overkill in this case. But which of the above ones is preferred then? I think I can answer my own question. If $foo contains spaces only the quotation mark based version will do what we want. I will remove the x characters. >> +EOF >> + >> +chmod a+x "$SERVERDIR/authscript.sh" >> + >> cat >request-anonymous <<EOF >> 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 && I copied this code from above so this was not my decision. These test cases are all can be find elsewhere in the code, the only difference is in their names and the fact that the script based authentication method is active while they are running. >> + 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. > I am not familiar the way that a patch goes in this project. Where can I find the repository my patches will be commited? When will this kind of changes go to mainline? Thanks, László ÁSHIN ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication 2010-07-04 8:51 ` Áshin László @ 2010-07-04 12:22 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 4+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-04 12:22 UTC (permalink / raw) To: Áshin László; +Cc: git On Sun, Jul 4, 2010 at 08:51, Áshin László <ashinlaszlo@gmail.com> wrote: > Hi, > > On Sun, Jul 4, 2010 at 00:14, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@gmail.com> 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. > > Yes but it is not part of my changeset. Sorry, misread the diff. >>> -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? > > Actually a check can be added to the code. OK, I will do that. Nice. >>> + 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". > > OK, a little bit I overcomplicated it :) > > >>> + >>> +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. > > "The exit status of a sequential list *shall* be the exit status of > the last command in the list." > ( After registration, > http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html#tag_02_08 > ) > Generally, all kind of list constructions use this principle according > to this page. > > Terminology of "shall": > "For an implementation that conforms to IEEE Std 1003.1-2001, > describes a feature or behavior that is mandatory. *An application can > rely on the existence of the feature or behavior*. > > For an application or user, describes a behavior that is mandatory." Nice, good to have that cleared up in general :) >>> +------ >>> +-- >>> + >>> + 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 <at> server/path/repo.git co <HEAD_name> >>> 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. > > Please note that there is no "\n" after the first line. This is only a > line break in the source code. > Should I have used the following? Ah yes, I missed that there wasn't a \n there. > print "E The authentication script specified in" . > "E [gitcvs.authscript] cannot be executed\n"; Probably yeah, at least I managed to misread it :) >>> + 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" <<EOF >>> +#!/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? > > Maybe this is too much. Maybe x was used only where " couldn't be: > > test x$foo = xbar > > So, if $foo is empty we have to make sure that test gets something to > both side of the equation mark. But if quotation marks can be used it > is possibly unneeded: > > test "$foo" = "bar" > > My solution is an overkill in this case. But which of the above ones > is preferred then? > I think I can answer my own question. If $foo contains spaces only the > quotation mark based version will do what we want. I will remove the x > characters. Ok. >>> +EOF >>> + >>> +chmod a+x "$SERVERDIR/authscript.sh" >>> + >>> cat >request-anonymous <<EOF >>> 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 && > > I copied this code from above so this was not my decision. These test > cases are all can be find elsewhere in the code, the only difference > is in their names and the fact that the script based authentication > method is active while they are running. Yeah, a lot of the test code is ancient and unoptimal. Just leave it like that I guess. >>> + 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. >> > > I am not familiar the way that a patch goes in this project. Where can > I find the repository my patches will be commited? When will this kind > of changes go to mainline? Patches are: * Discussed on the mailing list (you are here) * Submitted for inclusion (see Documentation/SubmittingPatches). You didn't CC Junio so it isn't there yet. * Maybe show up in a "What's cooking" post if they're more complex and warrant further discussion. * Get merged into git://git.kernel.org/pub/scm/git/git.git, either pu, next, master, maint or some combo of that. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-07-04 12:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1058578005711783867@unknownmsgid>
2010-07-03 21:21 ` [PATCH 1/4] git-cvsserver: implement script based pserver authentication Áshin László
2010-07-03 22:14 ` Ævar Arnfjörð Bjarmason
2010-07-04 8:51 ` Áshin László
2010-07-04 12:22 ` Ævar Arnfjörð Bjarmason
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).