[PATCH] git-cvsserver: pserver-auth-script

DORMANTno replies

8 messages, 4 authors, 2016-06-15 · open the first message on its own page

[PATCH] git-cvsserver: pserver-auth-script

From: László ÁSHIN <hidden>
Date: 2016-06-15 22:49:04

Hi,

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.

-- 
Regards,
László Áshin

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
@@ -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;
+            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
     }

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:49:04

On Fri, Jul 2, 2010 at 07:54, László ÁSHIN [off-list ref] wrote:
quoted hunk
Hi,

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.

--
Regards,
László Áshin

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
@@ -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;
+            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
    }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:49:04

On Fri, Jul 2, 2010 at 07:54, László ÁSHIN [off-list ref] 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.
quoted hunk
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.
quoted hunk
@@ -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.

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:49:04

On Fri, Jul 2, 2010 at 14:39, Ævar Arnfjörð Bjarmason [off-list ref] wrote:
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.
Oh, and it also needs tests. See the existing authdb tests in
t9400-git-cvsserver-server.sh.

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:49:04

Ævar Arnfjörð Bjarmason [off-list ref] writes:
On Fri, Jul 2, 2010 at 07:54, László ÁSHIN [off-list ref] wrote:
quoted
+
+            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 $!;
    ...
This is two-argument open, not three-argument magic open.  There is
string concatenation operator '.' there, not a comma ',' delimiting
arguments.

It should be

   open my $script_fd, '|-', $authscript
   	or die "Couldn't open authentication script '$authscript': $!";
 
quoted
+        } else {
+            if (not exists $cfg->{gitcvs}->{authdb}) {
Why not elsif?
quoted
+                print "E the repo config file needs a [gitcvs] section with an 'authdb' parameter set to the filename of the authentication database\n";
Overly long line.  Perhaps it would be better to split it into
concatenated parts.

quoted
+            my $auth_ok;
+            open my $passwd, "<", $authdb or die $!;
And here you use three-argument form of (ordinary) open.
quoted
+            while (<$passwd>) {
+                if (m{^\Q$user\E:(.*)}) {
+                    if (crypt($user, descramble($password)) eq $1) {
Why nested if, and not short-circuit &&?

    +                if (/^\Q$user\E:(.*)/ &&
    +                    crypt($user, descramble($password)) eq $1) {
    
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:49:04

On Fri, Jul 2, 2010 at 21:31, Jakub Narebski [off-list ref] wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
On Fri, Jul 2, 2010 at 07:54, László ÁSHIN [off-list ref] wrote:
quoted
+
+            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 $!;
    ...
This is two-argument open, not three-argument magic open.  There is
string concatenation operator '.' there, not a comma ',' delimiting
arguments.
Well spotted, I misread that.
It should be

  open my $script_fd, '|-', $authscript
       or die "Couldn't open authentication script '$authscript': $!";
quoted
quoted
+        } else {
+            if (not exists $cfg->{gitcvs}->{authdb}) {
Why not elsif?
quoted
quoted
+                print "E the repo config file needs a [gitcvs] section with an 'authdb' parameter set to the filename of the authentication database\n";
Overly long line.  Perhaps it would be better to split it into
concatenated parts.

quoted
quoted
+            my $auth_ok;
+            open my $passwd, "<", $authdb or die $!;
And here you use three-argument form of (ordinary) open.
This and the code below were already part of the code. But the patch
would be better if it didn't move so much code around so that this
would be obvious.
quoted
quoted
+            while (<$passwd>) {
+                if (m{^\Q$user\E:(.*)}) {
+                    if (crypt($user, descramble($password)) eq $1) {
Why nested if, and not short-circuit &&?

   +                if (/^\Q$user\E:(.*)/ &&
   +                    crypt($user, descramble($password)) eq $1) {

--
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Áshin László <hidden>
Date: 2016-06-15 22:49:04

Hi,

On Fri, Jul 2, 2010 at 23:34, Ævar Arnfjörð Bjarmason [off-list ref] wrote:
On Fri, Jul 2, 2010 at 21:31, Jakub Narebski [off-list ref] wrote:
quoted
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
On Fri, Jul 2, 2010 at 07:54, László ÁSHIN [off-list ref] wrote:
quoted
+
+            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 $!;
    ...
This is two-argument open, not three-argument magic open.  There is
string concatenation operator '.' there, not a comma ',' delimiting
arguments.
Well spotted, I misread that.
quoted
It should be

  open my $script_fd, '|-', $authscript
       or die "Couldn't open authentication script '$authscript': $!";
It will be fixed, I will resend the patch.

quoted
quoted
quoted
+        } else {
+            if (not exists $cfg->{gitcvs}->{authdb}) {
Why not elsif?
Because the else branch continues below. But good point, it can be
done that way too. Will be fixed.

quoted
quoted
quoted
+                print "E the repo config file needs a [gitcvs] section with an 'authdb' parameter set to the filename of the authentication database\n";
Overly long line.  Perhaps it would be better to split it into
concatenated parts.
OK. Next patch will be fixed from this point of view as well.

quoted
quoted
quoted
+            my $auth_ok;
+            open my $passwd, "<", $authdb or die $!;
And here you use three-argument form of (ordinary) open.
This and the code below were already part of the code. But the patch
would be better if it didn't move so much code around so that this
would be obvious.
I wanted to keep the old functionality (authdb), so I moved all of its
code to this else branch. That move meant I had to increase
indentation level. But as far as I see now I could have solved it
somehow else to keep the indentation level. I will fix this too.

quoted
quoted
quoted
+            while (<$passwd>) {
+                if (m{^\Q$user\E:(.*)}) {
+                    if (crypt($user, descramble($password)) eq $1) {
Why nested if, and not short-circuit &&?

   +                if (/^\Q$user\E:(.*)/ &&
   +                    crypt($user, descramble($password)) eq $1) {
Good point, but it is not my code.

So, I will resend this patch - the corrected one without the mistakes
I taken in the first version of it.
Should I make one patch for each of the code, doc, and test case, or
can they all go into one patch?

Regards,
László ÁSHIN

Re: [PATCH] git-cvsserver: pserver-auth-script

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:49:04

On Sat, Jul 3, 2010 at 09:28, Áshin László [off-list ref] wrote:
So, I will resend this patch - the corrected one without the mistakes
I taken in the first version of it.
Good.
Should I make one patch for each of the code, doc, and test case, or
can they all go into one patch?
They should all be part of the same patch.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help