{"thread":{"id":"24280","subject":"[PATCH 1/4] git-cvsserver: implement script based pserver authentication","startedAt":"2010-07-03T21:21:44Z","lastAt":"2010-07-04T12:22:05Z","messageCount":4,"participants":["Áshin László","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"144757","messageId":"AANLkTikbPyZ-q8XgTBX1P9HQlFwUhiJSpiOgYG75mtiq@mail.gmail.com","threadId":"24280","inReplyTo":"1058578005711783867@unknownmsgid","subject":"[PATCH 1/4] git-cvsserver: implement script based pserver authentication","fromName":"Áshin László","fromEmail":"ashinlaszlo@gmail.com","sentAt":"2010-07-03T21:21:44Z","receivedAt":"2010-07-03T21:21:44Z","isPatch":true,"sender":{"key":"ashinlaszlo@gmail.com","avatar":null},"body":" Documentation/git-cvsserver.txt |   46 +++++++++++++++++++++++++++++++++++---\n git-cvsserver.perl              |   28 +++++++++++++++++++++++\n t/t9400-git-cvsserver-server.sh |   45 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 115 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex 7004dd2..b3c3122 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -100,10 +100,48 @@ looks like\n ------\n\n Only anonymous access is provided by pserve by default. To commit you\n-will have to create pserver accounts, simply add a gitcvs.authdb\n-setting in the config file of the repositories you want the cvsserver\n-to allow writes to, for example:\n+will have to specify an authentication option in the config file.\n+Currently there are two options are available for authentication through\n+pserver in git-cvsserver: one through an authenticator script and an other\n+through a textual authentication database. If both are specified in the config\n+file then the script based solution will be used.\n+\n+  a. To use the authentication script based method, simply add a\n+     gitcvs.authscript setting in the config file of the repositories you want\n+     the cvsserver to allow writes to, for example:\n++\n+--\n+------\n+\n+   [gitcvs]\n+    authscript = /usr/local/bin/cvsserver-auth.sh\n+\n+------\n+The file specified here must be executable by the user the git-cvsserver runs\n+under the name of. This script has to read exactly two lines from its standard\n+input as long as git-cvsserver passes the username and the password on separate\n+lines. After the script did its task by checking the username and password\n+given it has to return zero if the authentication was successful and return\n+other value if it was not.\n+\n+Here is an example for an authentication script which checks the users against\n+active directory:\n+------\n+#!/bin/sh\n+# /usr/local/bin/cvsserver-auth.sh\n\n+read username\n+read password\n+\n+wbinfo -a \"${username}%${password}\"\n+------\n+--\n+\n+  b. To use the authentication database based method, simply add a\n+     gitcvs.authdb setting in the config file of the repositories you want the\n+     cvsserver to allow writes to, for example:\n++\n+--\n ------\n\n   [gitcvs]\n@@ -125,7 +163,7 @@ Alternatively you can produce the password with\nperl's crypt() operator:\n -----\n   perl -e 'my ($user, $pass) = @ARGV; printf \"%s:%s\\n\", $user,\ncrypt($user, $pass)' $USER password\n -----\n-\n+--\n Then provide your password via the pserver method, for example:\n ------\n   cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex e9f3037..9664e86 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {\n        }\n\n        # Fall through to LOVE\n+    } elsif (exists $cfg->{gitcvs}->{authscript}) {\n+        my $authscript = $cfg->{gitcvs}->{authscript};\n+\n+        unless (-x $authscript) {\n+            print \"E The authentication script specified in \";\n+            print \"[gitcvs.authscript] cannot be executed\\n\";\n+            print \"I HATE YOU\\n\";\n+            exit 1;\n+        }\n+\n+        open my $script_fd, '|-', \"'$authscript'\"\n+            or die \"Couldn't open authentication script '$authscript': $!\";\n+\n+        if (length($password) > 0) {\n+            $password = descramble($password);\n+        }\n+\n+        print $script_fd \"$user\\n\";\n+        print $script_fd \"$password\\n\";\n+        close $script_fd;\n+\n+        unless ($? == 0) {\n+            print \"E External script authentication failed.\\n\";\n+            print \"I HATE YOU\\n\";\n+            exit 1;\n+        }\n+\n+        # Fall through to LOVE\n    } else {\n        # Trying to authenticate a user\n        if (not exists $cfg->{gitcvs}->{authdb}) {\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 8639506..0743e9a 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \\\n # PSERVER AUTHENTICATION\n #------------------------\n\n+cat >\"$SERVERDIR/authscript.sh\" <<EOF\n+#!/bin/sh\n+read username\n+read password\n+\n+test \"x\\$username\" = xcvsuser -a \"x\\$password\" = xcvspassword\n+EOF\n+\n+chmod a+x \"$SERVERDIR/authscript.sh\"\n+\n cat >request-anonymous  <<EOF\n BEGIN AUTH REQUEST\n $SERVERDIR\n@@ -134,6 +144,41 @@ test_expect_success 'pserver authentication\nfailure (login/non-anonymous user)'\n   fi &&\n   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n\n+GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authdb || exit 1\n+GIT_DIR=\"$SERVERDIR\" git config gitcvs.authscript\n\"$SERVERDIR/authscript.sh\" || exit 1\n+\n+test_expect_success 'pserver authentication (authscript)' \\\n+  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&\n+   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n+\n+test_expect_success 'pserver authentication failure (authscript,\nnon-anonymous user)' \\\n+  'if cat request-git | git-cvsserver pserver >log 2>&1\n+   then\n+       false\n+   else\n+       true\n+   fi &&\n+   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n+\n+test_expect_success 'pserver authentication success (authscript,\nnon-anonymous user with password)' \\\n+  'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&\n+   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n+\n+test_expect_success 'pserver authentication (authscript, login)' \\\n+  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&\n+   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n+\n+test_expect_success 'pserver authentication failure (authscript,\nlogin/non-anonymous user)' \\\n+  'if cat login-git | git-cvsserver pserver >log 2>&1\n+   then\n+       false\n+   else\n+       true\n+   fi &&\n+   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n+\n+GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authscript || exit 1\n+GIT_DIR=\"$SERVERDIR\" git config gitcvs.authdb \"$SERVERDIR/auth.db\" || exit 1\n\n # misuse pserver authentication for testing of req_Root\n\n--\n1.7.2.rc1.dirty\n"},{"id":"144762","messageId":"AANLkTil4C_oF972J5iRApUGXpwoFNqcYTgkzvorCseZZ@mail.gmail.com","threadId":"24280","inReplyTo":"AANLkTikbPyZ-q8XgTBX1P9HQlFwUhiJSpiOgYG75mtiq@mail.gmail.com","subject":"Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-03T22:14:53Z","receivedAt":"2010-07-03T22:14:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@gmail.com> wrote:\n>  Documentation/git-cvsserver.txt |   46 +++++++++++++++++++++++++++++++++++---\n>  git-cvsserver.perl              |   28 +++++++++++++++++++++++\n>  t/t9400-git-cvsserver-server.sh |   45 ++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 115 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\n> index 7004dd2..b3c3122 100644\n> --- a/Documentation/git-cvsserver.txt\n> +++ b/Documentation/git-cvsserver.txt\n> @@ -100,10 +100,48 @@ looks like\n>  ------\n>\n>  Only anonymous access is provided by pserve by default. To commit you\n\nI think it's always called \"pserver\" with an -r.\n\n> -will have to create pserver accounts, simply add a gitcvs.authdb\n> -setting in the config file of the repositories you want the cvsserver\n> -to allow writes to, for example:\n> +will have to specify an authentication option in the config file.\n> +Currently there are two options are available for authentication through\n> +pserver in git-cvsserver: one through an authenticator script and an other\n> +through a textual authentication database. If both are specified in the config\n> +file then the script based solution will be used.\n\nIf both are specified shouldn't we throw an error?\n\n> +  a. To use the authentication script based method, simply add a\n> +     gitcvs.authscript setting in the config file of the repositories you want\n> +     the cvsserver to allow writes to, for example:\n> ++\n> +--\n> +------\n> +\n> +   [gitcvs]\n> +    authscript = /usr/local/bin/cvsserver-auth.sh\n> +\n> +------\n> +The file specified here must be executable by the user the git-cvsserver runs\n> +under the name of. This script has to read exactly two lines from its standard\n\n\"the name of\" is redundant here.\n\n> +input as long as git-cvsserver passes the username and the password on separate\n> +lines. After the script did its task by checking the username and password\n> +given it has to return zero if the authentication was successful and return\n> +other value if it was not.\n\nBetter as: \"The script will receive two lines on standard input, the\nfirst is the username and the second is the password. It should return\n0 if the user was successfully authenticated, and a non-zero value if\nnot\".\n\n> +\n> +Here is an example for an authentication script which checks the users against\n> +active directory:\n> +------\n> +#!/bin/sh\n> +# /usr/local/bin/cvsserver-auth.sh\n>\n> +read username\n> +read password\n> +\n> +wbinfo -a \"${username}%${password}\"\n\nDo all POSIX shells implicitly exit with the return value of the last\nstatement they evaluate. I don't know.\n\n> +------\n> +--\n> +\n> +  b. To use the authentication database based method, simply add a\n> +     gitcvs.authdb setting in the config file of the repositories you want the\n> +     cvsserver to allow writes to, for example:\n> ++\n> +--\n>  ------\n>\n>    [gitcvs]\n> @@ -125,7 +163,7 @@ Alternatively you can produce the password with\n> perl's crypt() operator:\n>  -----\n>    perl -e 'my ($user, $pass) = @ARGV; printf \"%s:%s\\n\", $user,\n> crypt($user, $pass)' $USER password\n>  -----\n> -\n> +--\n>  Then provide your password via the pserver method, for example:\n>  ------\n>    cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index e9f3037..9664e86 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {\n>         }\n>\n>         # Fall through to LOVE\n> +    } elsif (exists $cfg->{gitcvs}->{authscript}) {\n> +        my $authscript = $cfg->{gitcvs}->{authscript};\n> +\n> +        unless (-x $authscript) {\n> +            print \"E The authentication script specified in \";\n> +            print \"[gitcvs.authscript] cannot be executed\\n\";\n\nI *think* you have to prefix anything that's not \"I (HATE|LOVE) YOU\"\nwith \"E \" if it's an error. I.e. this should probably be:\n\n    print \"E The authentication script specified in\";\n    print \"E [gitcvs.authscript] cannot be executed\\n\";\n\nBut that's just my hazy memory. Maybe CVS clients will report the\nerror anyway.\n\n> +            print \"I HATE YOU\\n\";\n> +            exit 1;\n> +        }\n> +\n> +        open my $script_fd, '|-', \"'$authscript'\"\n> +            or die \"Couldn't open authentication script '$authscript': $!\";\n> +\n> +        if (length($password) > 0) {\n> +            $password = descramble($password);\n> +        }\n> +\n> +        print $script_fd \"$user\\n\";\n> +        print $script_fd \"$password\\n\";\n> +        close $script_fd;\n> +\n> +        unless ($? == 0) {\n> +            print \"E External script authentication failed.\\n\";\n> +            print \"I HATE YOU\\n\";\n> +            exit 1;\n> +        }\n> +\n> +        # Fall through to LOVE\n>     } else {\n>         # Trying to authenticate a user\n>         if (not exists $cfg->{gitcvs}->{authdb}) {\n> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\n> index 8639506..0743e9a 100755\n> --- a/t/t9400-git-cvsserver-server.sh\n> +++ b/t/t9400-git-cvsserver-server.sh\n> @@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \\\n>  # PSERVER AUTHENTICATION\n>  #------------------------\n>\n> +cat >\"$SERVERDIR/authscript.sh\" <<EOF\n> +#!/bin/sh\n> +read username\n> +read password\n> +\n> +test \"x\\$username\" = xcvsuser -a \"x\\$password\" = xcvspassword\n\nThis is offtopic. But I've never figured out why you need to do\n\n    test \"x$foo\" = \"xbar\"\n\nAs opposed to:\n\n    test \"$foo\" = \"bar\"\n\nIn shellscript. Are there really some shells that get equality tests\nwhere one term is \"\" wrong?\n\n> +EOF\n> +\n> +chmod a+x \"$SERVERDIR/authscript.sh\"\n> +\n>  cat >request-anonymous  <<EOF\n>  BEGIN AUTH REQUEST\n>  $SERVERDIR\n> @@ -134,6 +144,41 @@ test_expect_success 'pserver authentication\n> failure (login/non-anonymous user)'\n>    fi &&\n>    sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>\n> +GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authdb || exit 1\n> +GIT_DIR=\"$SERVERDIR\" git config gitcvs.authscript\n> \"$SERVERDIR/authscript.sh\" || exit 1\n> +\n> +test_expect_success 'pserver authentication (authscript)' \\\n> +  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&\n> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n> +\n> +test_expect_success 'pserver authentication failure (authscript,\n> non-anonymous user)' \\\n> +  'if cat request-git | git-cvsserver pserver >log 2>&1\n> +   then\n> +       false\n> +   else\n> +       true\n> +   fi &&\n\nThis should probably be (untested):\n\n    test_must_fail cat request-git git-cvsserver pserver >log 2>&1 &&\n\n> +   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n> +\n> +test_expect_success 'pserver authentication success (authscript,\n> non-anonymous user with password)' \\\n> +  'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&\n> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n> +\n> +test_expect_success 'pserver authentication (authscript, login)' \\\n> +  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&\n> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n> +\n> +test_expect_success 'pserver authentication failure (authscript,\n> login/non-anonymous user)' \\\n> +  'if cat login-git | git-cvsserver pserver >log 2>&1\n> +   then\n> +       false\n> +   else\n> +       true\n> +   fi &&\n> +   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n> +\n> +GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authscript || exit 1\n> +GIT_DIR=\"$SERVERDIR\" git config gitcvs.authdb \"$SERVERDIR/auth.db\" || exit 1\n>\n>  # misuse pserver authentication for testing of req_Root\n\nOtherwise looking good.\n"},{"id":"144773","messageId":"AANLkTimVYRJOJ6MG1HDFnj56cLxWrP1gjmIi-dKOuE-7@mail.gmail.com","threadId":"24280","inReplyTo":"AANLkTil4C_oF972J5iRApUGXpwoFNqcYTgkzvorCseZZ@mail.gmail.com","subject":"Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication","fromName":"Áshin László","fromEmail":"ashinlaszlo@gmail.com","sentAt":"2010-07-04T08:51:10Z","receivedAt":"2010-07-04T08:51:10Z","isPatch":true,"sender":{"key":"ashinlaszlo@gmail.com","avatar":null},"body":"Hi,\n\nOn Sun, Jul 4, 2010 at 00:14, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@gmail.com> wrote:\n>>  Documentation/git-cvsserver.txt |   46 +++++++++++++++++++++++++++++++++++---\n>>  git-cvsserver.perl              |   28 +++++++++++++++++++++++\n>>  t/t9400-git-cvsserver-server.sh |   45 ++++++++++++++++++++++++++++++++++++++\n>>  3 files changed, 115 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\n>> index 7004dd2..b3c3122 100644\n>> --- a/Documentation/git-cvsserver.txt\n>> +++ b/Documentation/git-cvsserver.txt\n>> @@ -100,10 +100,48 @@ looks like\n>>  ------\n>>\n>>  Only anonymous access is provided by pserve by default. To commit you\n>\n> I think it's always called \"pserver\" with an -r.\n\nYes but it is not part of my changeset.\n\n\n>> -will have to create pserver accounts, simply add a gitcvs.authdb\n>> -setting in the config file of the repositories you want the cvsserver\n>> -to allow writes to, for example:\n>> +will have to specify an authentication option in the config file.\n>> +Currently there are two options are available for authentication through\n>> +pserver in git-cvsserver: one through an authenticator script and an other\n>> +through a textual authentication database. If both are specified in the config\n>> +file then the script based solution will be used.\n>\n> If both are specified shouldn't we throw an error?\n\nActually a check can be added to the code. OK, I will do that.\n\n\n>> +  a. To use the authentication script based method, simply add a\n>> +     gitcvs.authscript setting in the config file of the repositories you want\n>> +     the cvsserver to allow writes to, for example:\n>> ++\n>> +--\n>> +------\n>> +\n>> +   [gitcvs]\n>> +    authscript = /usr/local/bin/cvsserver-auth.sh\n>> +\n>> +------\n>> +The file specified here must be executable by the user the git-cvsserver runs\n>> +under the name of. This script has to read exactly two lines from its standard\n>\n> \"the name of\" is redundant here.\n>\n>> +input as long as git-cvsserver passes the username and the password on separate\n>> +lines. After the script did its task by checking the username and password\n>> +given it has to return zero if the authentication was successful and return\n>> +other value if it was not.\n>\n> Better as: \"The script will receive two lines on standard input, the\n> first is the username and the second is the password. It should return\n> 0 if the user was successfully authenticated, and a non-zero value if\n> not\".\n\nOK, a little bit I overcomplicated it :)\n\n\n>> +\n>> +Here is an example for an authentication script which checks the users against\n>> +active directory:\n>> +------\n>> +#!/bin/sh\n>> +# /usr/local/bin/cvsserver-auth.sh\n>>\n>> +read username\n>> +read password\n>> +\n>> +wbinfo -a \"${username}%${password}\"\n>\n> Do all POSIX shells implicitly exit with the return value of the last\n> statement they evaluate. I don't know.\n\n\"The exit status of a sequential list *shall* be the exit status of\nthe last command in the list.\"\n( After registration,\nhttp://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html#tag_02_08\n)\nGenerally, all kind of list constructions use this principle according\nto this page.\n\nTerminology of \"shall\":\n\"For an implementation that conforms to IEEE Std 1003.1-2001,\ndescribes a feature or behavior that is mandatory. *An application can\nrely on the existence of the feature or behavior*.\n\nFor an application or user, describes a behavior that is mandatory.\"\n\n\n>> +------\n>> +--\n>> +\n>> +  b. To use the authentication database based method, simply add a\n>> +     gitcvs.authdb setting in the config file of the repositories you want the\n>> +     cvsserver to allow writes to, for example:\n>> ++\n>> +--\n>>  ------\n>>\n>>    [gitcvs]\n>> @@ -125,7 +163,7 @@ Alternatively you can produce the password with\n>> perl's crypt() operator:\n>>  -----\n>>    perl -e 'my ($user, $pass) = @ARGV; printf \"%s:%s\\n\", $user,\n>> crypt($user, $pass)' $USER password\n>>  -----\n>> -\n>> +--\n>>  Then provide your password via the pserver method, for example:\n>>  ------\n>>    cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>\n>> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n>> index e9f3037..9664e86 100755\n>> --- a/git-cvsserver.perl\n>> +++ b/git-cvsserver.perl\n>> @@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {\n>>         }\n>>\n>>         # Fall through to LOVE\n>> +    } elsif (exists $cfg->{gitcvs}->{authscript}) {\n>> +        my $authscript = $cfg->{gitcvs}->{authscript};\n>> +\n>> +        unless (-x $authscript) {\n>> +            print \"E The authentication script specified in \";\n>> +            print \"[gitcvs.authscript] cannot be executed\\n\";\n>\n> I *think* you have to prefix anything that's not \"I (HATE|LOVE) YOU\"\n> with \"E \" if it's an error. I.e. this should probably be:\n>\n>    print \"E The authentication script specified in\";\n>    print \"E [gitcvs.authscript] cannot be executed\\n\";\n>\n> But that's just my hazy memory. Maybe CVS clients will report the\n> error anyway.\n\nPlease note that there is no \"\\n\" after the first line. This is only a\nline break in the source code.\nShould I have used the following?\n\n    print \"E The authentication script specified in\" .\n        \"E [gitcvs.authscript] cannot be executed\\n\";\n\n\n>> +            print \"I HATE YOU\\n\";\n>> +            exit 1;\n>> +        }\n>> +\n>> +        open my $script_fd, '|-', \"'$authscript'\"\n>> +            or die \"Couldn't open authentication script '$authscript': $!\";\n>> +\n>> +        if (length($password) > 0) {\n>> +            $password = descramble($password);\n>> +        }\n>> +\n>> +        print $script_fd \"$user\\n\";\n>> +        print $script_fd \"$password\\n\";\n>> +        close $script_fd;\n>> +\n>> +        unless ($? == 0) {\n>> +            print \"E External script authentication failed.\\n\";\n>> +            print \"I HATE YOU\\n\";\n>> +            exit 1;\n>> +        }\n>> +\n>> +        # Fall through to LOVE\n>>     } else {\n>>         # Trying to authenticate a user\n>>         if (not exists $cfg->{gitcvs}->{authdb}) {\n>> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\n>> index 8639506..0743e9a 100755\n>> --- a/t/t9400-git-cvsserver-server.sh\n>> +++ b/t/t9400-git-cvsserver-server.sh\n>> @@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \\\n>>  # PSERVER AUTHENTICATION\n>>  #------------------------\n>>\n>> +cat >\"$SERVERDIR/authscript.sh\" <<EOF\n>> +#!/bin/sh\n>> +read username\n>> +read password\n>> +\n>> +test \"x\\$username\" = xcvsuser -a \"x\\$password\" = xcvspassword\n>\n> This is offtopic. But I've never figured out why you need to do\n>\n>    test \"x$foo\" = \"xbar\"\n>\n> As opposed to:\n>\n>    test \"$foo\" = \"bar\"\n>\n> In shellscript. Are there really some shells that get equality tests\n> where one term is \"\" wrong?\n\nMaybe this is too much. Maybe x was used only where \" couldn't be:\n\n    test x$foo = xbar\n\nSo, if $foo is empty we have to make sure that test gets something to\nboth side of the equation mark. But if quotation marks can be used it\nis possibly unneeded:\n\n    test \"$foo\" = \"bar\"\n\nMy solution is an overkill in this case. But which of the above ones\nis preferred then?\nI think I can answer my own question. If $foo contains spaces only the\nquotation mark based version will do what we want. I will remove the x\ncharacters.\n\n\n>> +EOF\n>> +\n>> +chmod a+x \"$SERVERDIR/authscript.sh\"\n>> +\n>>  cat >request-anonymous  <<EOF\n>>  BEGIN AUTH REQUEST\n>>  $SERVERDIR\n>> @@ -134,6 +144,41 @@ test_expect_success 'pserver authentication\n>> failure (login/non-anonymous user)'\n>>    fi &&\n>>    sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>>\n>> +GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authdb || exit 1\n>> +GIT_DIR=\"$SERVERDIR\" git config gitcvs.authscript\n>> \"$SERVERDIR/authscript.sh\" || exit 1\n>> +\n>> +test_expect_success 'pserver authentication (authscript)' \\\n>> +  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&\n>> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n>> +\n>> +test_expect_success 'pserver authentication failure (authscript,\n>> non-anonymous user)' \\\n>> +  'if cat request-git | git-cvsserver pserver >log 2>&1\n>> +   then\n>> +       false\n>> +   else\n>> +       true\n>> +   fi &&\n>\n> This should probably be (untested):\n>\n>    test_must_fail cat request-git git-cvsserver pserver >log 2>&1 &&\n\nI copied this code from above so this was not my decision. These test\ncases are all can be find elsewhere in the code, the only difference\nis in their names and the fact that the script based authentication\nmethod is active while they are running.\n\n\n>> +   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>> +\n>> +test_expect_success 'pserver authentication success (authscript,\n>> non-anonymous user with password)' \\\n>> +  'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&\n>> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n>> +\n>> +test_expect_success 'pserver authentication (authscript, login)' \\\n>> +  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&\n>> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n>> +\n>> +test_expect_success 'pserver authentication failure (authscript,\n>> login/non-anonymous user)' \\\n>> +  'if cat login-git | git-cvsserver pserver >log 2>&1\n>> +   then\n>> +       false\n>> +   else\n>> +       true\n>> +   fi &&\n>> +   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>> +\n>> +GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authscript || exit 1\n>> +GIT_DIR=\"$SERVERDIR\" git config gitcvs.authdb \"$SERVERDIR/auth.db\" || exit 1\n>>\n>>  # misuse pserver authentication for testing of req_Root\n>\n> Otherwise looking good.\n>\n\nI am not familiar the way that a patch goes in this project. Where can\nI find the repository my patches will be commited? When will this kind\nof changes go to mainline?\n\nThanks,\nLászló ÁSHIN\n"},{"id":"144778","messageId":"AANLkTin3I8aKYTkfVD602TZoosHuUsFLv35WAqVyuKDi@mail.gmail.com","threadId":"24280","inReplyTo":"AANLkTimVYRJOJ6MG1HDFnj56cLxWrP1gjmIi-dKOuE-7@mail.gmail.com","subject":"Re: [PATCH 1/4] git-cvsserver: implement script based pserver authentication","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-04T12:22:05Z","receivedAt":"2010-07-04T12:22:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jul 4, 2010 at 08:51, Áshin László <ashinlaszlo@gmail.com> wrote:\n> Hi,\n>\n> On Sun, Jul 4, 2010 at 00:14, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@gmail.com> wrote:\n>>>  Documentation/git-cvsserver.txt |   46 +++++++++++++++++++++++++++++++++++---\n>>>  git-cvsserver.perl              |   28 +++++++++++++++++++++++\n>>>  t/t9400-git-cvsserver-server.sh |   45 ++++++++++++++++++++++++++++++++++++++\n>>>  3 files changed, 115 insertions(+), 4 deletions(-)\n>>>\n>>> diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\n>>> index 7004dd2..b3c3122 100644\n>>> --- a/Documentation/git-cvsserver.txt\n>>> +++ b/Documentation/git-cvsserver.txt\n>>> @@ -100,10 +100,48 @@ looks like\n>>>  ------\n>>>\n>>>  Only anonymous access is provided by pserve by default. To commit you\n>>\n>> I think it's always called \"pserver\" with an -r.\n>\n> Yes but it is not part of my changeset.\n\nSorry, misread the diff.\n\n>>> -will have to create pserver accounts, simply add a gitcvs.authdb\n>>> -setting in the config file of the repositories you want the cvsserver\n>>> -to allow writes to, for example:\n>>> +will have to specify an authentication option in the config file.\n>>> +Currently there are two options are available for authentication through\n>>> +pserver in git-cvsserver: one through an authenticator script and an other\n>>> +through a textual authentication database. If both are specified in the config\n>>> +file then the script based solution will be used.\n>>\n>> If both are specified shouldn't we throw an error?\n>\n> Actually a check can be added to the code. OK, I will do that.\n\nNice.\n\n>>> +  a. To use the authentication script based method, simply add a\n>>> +     gitcvs.authscript setting in the config file of the repositories you want\n>>> +     the cvsserver to allow writes to, for example:\n>>> ++\n>>> +--\n>>> +------\n>>> +\n>>> +   [gitcvs]\n>>> +    authscript = /usr/local/bin/cvsserver-auth.sh\n>>> +\n>>> +------\n>>> +The file specified here must be executable by the user the git-cvsserver runs\n>>> +under the name of. This script has to read exactly two lines from its standard\n>>\n>> \"the name of\" is redundant here.\n>>\n>>> +input as long as git-cvsserver passes the username and the password on separate\n>>> +lines. After the script did its task by checking the username and password\n>>> +given it has to return zero if the authentication was successful and return\n>>> +other value if it was not.\n>>\n>> Better as: \"The script will receive two lines on standard input, the\n>> first is the username and the second is the password. It should return\n>> 0 if the user was successfully authenticated, and a non-zero value if\n>> not\".\n>\n> OK, a little bit I overcomplicated it :)\n>\n>\n>>> +\n>>> +Here is an example for an authentication script which checks the users against\n>>> +active directory:\n>>> +------\n>>> +#!/bin/sh\n>>> +# /usr/local/bin/cvsserver-auth.sh\n>>>\n>>> +read username\n>>> +read password\n>>> +\n>>> +wbinfo -a \"${username}%${password}\"\n>>\n>> Do all POSIX shells implicitly exit with the return value of the last\n>> statement they evaluate. I don't know.\n>\n> \"The exit status of a sequential list *shall* be the exit status of\n> the last command in the list.\"\n> ( After registration,\n> http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html#tag_02_08\n> )\n> Generally, all kind of list constructions use this principle according\n> to this page.\n>\n> Terminology of \"shall\":\n> \"For an implementation that conforms to IEEE Std 1003.1-2001,\n> describes a feature or behavior that is mandatory. *An application can\n> rely on the existence of the feature or behavior*.\n>\n> For an application or user, describes a behavior that is mandatory.\"\n\nNice, good to have that cleared up in general :)\n\n>>> +------\n>>> +--\n>>> +\n>>> +  b. To use the authentication database based method, simply add a\n>>> +     gitcvs.authdb setting in the config file of the repositories you want the\n>>> +     cvsserver to allow writes to, for example:\n>>> ++\n>>> +--\n>>>  ------\n>>>\n>>>    [gitcvs]\n>>> @@ -125,7 +163,7 @@ Alternatively you can produce the password with\n>>> perl's crypt() operator:\n>>>  -----\n>>>    perl -e 'my ($user, $pass) = @ARGV; printf \"%s:%s\\n\", $user,\n>>> crypt($user, $pass)' $USER password\n>>>  -----\n>>> -\n>>> +--\n>>>  Then provide your password via the pserver method, for example:\n>>>  ------\n>>>    cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>\n>>> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n>>> index e9f3037..9664e86 100755\n>>> --- a/git-cvsserver.perl\n>>> +++ b/git-cvsserver.perl\n>>> @@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {\n>>>         }\n>>>\n>>>         # Fall through to LOVE\n>>> +    } elsif (exists $cfg->{gitcvs}->{authscript}) {\n>>> +        my $authscript = $cfg->{gitcvs}->{authscript};\n>>> +\n>>> +        unless (-x $authscript) {\n>>> +            print \"E The authentication script specified in \";\n>>> +            print \"[gitcvs.authscript] cannot be executed\\n\";\n>>\n>> I *think* you have to prefix anything that's not \"I (HATE|LOVE) YOU\"\n>> with \"E \" if it's an error. I.e. this should probably be:\n>>\n>>    print \"E The authentication script specified in\";\n>>    print \"E [gitcvs.authscript] cannot be executed\\n\";\n>>\n>> But that's just my hazy memory. Maybe CVS clients will report the\n>> error anyway.\n>\n> Please note that there is no \"\\n\" after the first line. This is only a\n> line break in the source code.\n> Should I have used the following?\n\nAh yes, I missed that there wasn't a \\n there.\n\n>     print \"E The authentication script specified in\" .\n>         \"E [gitcvs.authscript] cannot be executed\\n\";\n\nProbably yeah, at least I managed to misread it :)\n\n>>> +            print \"I HATE YOU\\n\";\n>>> +            exit 1;\n>>> +        }\n>>> +\n>>> +        open my $script_fd, '|-', \"'$authscript'\"\n>>> +            or die \"Couldn't open authentication script '$authscript': $!\";\n>>> +\n>>> +        if (length($password) > 0) {\n>>> +            $password = descramble($password);\n>>> +        }\n>>> +\n>>> +        print $script_fd \"$user\\n\";\n>>> +        print $script_fd \"$password\\n\";\n>>> +        close $script_fd;\n>>> +\n>>> +        unless ($? == 0) {\n>>> +            print \"E External script authentication failed.\\n\";\n>>> +            print \"I HATE YOU\\n\";\n>>> +            exit 1;\n>>> +        }\n>>> +\n>>> +        # Fall through to LOVE\n>>>     } else {\n>>>         # Trying to authenticate a user\n>>>         if (not exists $cfg->{gitcvs}->{authdb}) {\n>>> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\n>>> index 8639506..0743e9a 100755\n>>> --- a/t/t9400-git-cvsserver-server.sh\n>>> +++ b/t/t9400-git-cvsserver-server.sh\n>>> @@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \\\n>>>  # PSERVER AUTHENTICATION\n>>>  #------------------------\n>>>\n>>> +cat >\"$SERVERDIR/authscript.sh\" <<EOF\n>>> +#!/bin/sh\n>>> +read username\n>>> +read password\n>>> +\n>>> +test \"x\\$username\" = xcvsuser -a \"x\\$password\" = xcvspassword\n>>\n>> This is offtopic. But I've never figured out why you need to do\n>>\n>>    test \"x$foo\" = \"xbar\"\n>>\n>> As opposed to:\n>>\n>>    test \"$foo\" = \"bar\"\n>>\n>> In shellscript. Are there really some shells that get equality tests\n>> where one term is \"\" wrong?\n>\n> Maybe this is too much. Maybe x was used only where \" couldn't be:\n>\n>    test x$foo = xbar\n>\n> So, if $foo is empty we have to make sure that test gets something to\n> both side of the equation mark. But if quotation marks can be used it\n> is possibly unneeded:\n>\n>    test \"$foo\" = \"bar\"\n>\n> My solution is an overkill in this case. But which of the above ones\n> is preferred then?\n> I think I can answer my own question. If $foo contains spaces only the\n> quotation mark based version will do what we want. I will remove the x\n> characters.\n\nOk.\n\n>>> +EOF\n>>> +\n>>> +chmod a+x \"$SERVERDIR/authscript.sh\"\n>>> +\n>>>  cat >request-anonymous  <<EOF\n>>>  BEGIN AUTH REQUEST\n>>>  $SERVERDIR\n>>> @@ -134,6 +144,41 @@ test_expect_success 'pserver authentication\n>>> failure (login/non-anonymous user)'\n>>>    fi &&\n>>>    sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>>>\n>>> +GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authdb || exit 1\n>>> +GIT_DIR=\"$SERVERDIR\" git config gitcvs.authscript\n>>> \"$SERVERDIR/authscript.sh\" || exit 1\n>>> +\n>>> +test_expect_success 'pserver authentication (authscript)' \\\n>>> +  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&\n>>> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n>>> +\n>>> +test_expect_success 'pserver authentication failure (authscript,\n>>> non-anonymous user)' \\\n>>> +  'if cat request-git | git-cvsserver pserver >log 2>&1\n>>> +   then\n>>> +       false\n>>> +   else\n>>> +       true\n>>> +   fi &&\n>>\n>> This should probably be (untested):\n>>\n>>    test_must_fail cat request-git git-cvsserver pserver >log 2>&1 &&\n>\n> I copied this code from above so this was not my decision. These test\n> cases are all can be find elsewhere in the code, the only difference\n> is in their names and the fact that the script based authentication\n> method is active while they are running.\n\nYeah, a lot of the test code is ancient and unoptimal. Just leave it\nlike that I guess.\n\n>>> +   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>>> +\n>>> +test_expect_success 'pserver authentication success (authscript,\n>>> non-anonymous user with password)' \\\n>>> +  'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&\n>>> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n>>> +\n>>> +test_expect_success 'pserver authentication (authscript, login)' \\\n>>> +  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&\n>>> +   sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n>>> +\n>>> +test_expect_success 'pserver authentication failure (authscript,\n>>> login/non-anonymous user)' \\\n>>> +  'if cat login-git | git-cvsserver pserver >log 2>&1\n>>> +   then\n>>> +       false\n>>> +   else\n>>> +       true\n>>> +   fi &&\n>>> +   sed -ne \\$p log | grep \"^I HATE YOU\\$\"'\n>>> +\n>>> +GIT_DIR=\"$SERVERDIR\" git config --unset gitcvs.authscript || exit 1\n>>> +GIT_DIR=\"$SERVERDIR\" git config gitcvs.authdb \"$SERVERDIR/auth.db\" || exit 1\n>>>\n>>>  # misuse pserver authentication for testing of req_Root\n>>\n>> Otherwise looking good.\n>>\n>\n> I am not familiar the way that a patch goes in this project. Where can\n> I find the repository my patches will be commited? When will this kind\n> of changes go to mainline?\n\nPatches are:\n\n * Discussed on the mailing list (you are here)\n\n * Submitted for inclusion (see Documentation/SubmittingPatches). You\n   didn't CC Junio so it isn't there yet.\n\n * Maybe show up in a \"What's cooking\" post if they're more complex\n   and warrant further discussion.\n\n * Get merged into git://git.kernel.org/pub/scm/git/git.git, either\n   pu, next, master, maint or some combo of that.\n"}]}