Hi,
On Sun, Jul 4, 2010 at 00:14, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
> On Sat, Jul 3, 2010 at 21:21, Áshin László <ashinlaszlo@xxxxxxxxx> 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
--
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