* [PATCH] git-cvsserver: pserver-auth-script
@ 2010-07-02 7:54 László ÁSHIN
2010-07-02 14:33 ` Ævar Arnfjörð Bjarmason
2010-07-02 14:39 ` Ævar Arnfjörð Bjarmason
0 siblings, 2 replies; 8+ messages in thread
From: László ÁSHIN @ 2010-07-02 7:54 UTC (permalink / raw)
To: git
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
}
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-02 7:54 [PATCH] git-cvsserver: pserver-auth-script László ÁSHIN @ 2010-07-02 14:33 ` Ævar Arnfjörð Bjarmason 2010-07-02 14:39 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 8+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-02 14:33 UTC (permalink / raw) To: László ÁSHIN; +Cc: git On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@neti.hu> wrote: > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-02 7:54 [PATCH] git-cvsserver: pserver-auth-script László ÁSHIN 2010-07-02 14:33 ` Ævar Arnfjörð Bjarmason @ 2010-07-02 14:39 ` Ævar Arnfjörð Bjarmason 2010-07-02 15:04 ` Ævar Arnfjörð Bjarmason 2010-07-02 21:31 ` Jakub Narebski 1 sibling, 2 replies; 8+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-02 14:39 UTC (permalink / raw) To: László ÁSHIN; +Cc: git On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@neti.hu> 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. > 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. > @@ -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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-02 14:39 ` Ævar Arnfjörð Bjarmason @ 2010-07-02 15:04 ` Ævar Arnfjörð Bjarmason 2010-07-02 21:31 ` Jakub Narebski 1 sibling, 0 replies; 8+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-02 15:04 UTC (permalink / raw) To: László ÁSHIN; +Cc: git On Fri, Jul 2, 2010 at 14:39, Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-02 14:39 ` Ævar Arnfjörð Bjarmason 2010-07-02 15:04 ` Ævar Arnfjörð Bjarmason @ 2010-07-02 21:31 ` Jakub Narebski 2010-07-02 21:34 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 8+ messages in thread From: Jakub Narebski @ 2010-07-02 21:31 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: László ÁSHIN, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@neti.hu> wrote: > > > + > > + 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': $!"; > > + } else { > > + if (not exists $cfg->{gitcvs}->{authdb}) { Why not elsif? > > + 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. > > + my $auth_ok; > > + open my $passwd, "<", $authdb or die $!; And here you use three-argument form of (ordinary) open. > > + 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-02 21:31 ` Jakub Narebski @ 2010-07-02 21:34 ` Ævar Arnfjörð Bjarmason 2010-07-03 9:28 ` Áshin László 0 siblings, 1 reply; 8+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-02 21:34 UTC (permalink / raw) To: Jakub Narebski; +Cc: László ÁSHIN, git On Fri, Jul 2, 2010 at 21:31, Jakub Narebski <jnareb@gmail.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@neti.hu> wrote: >> >> > + >> > + 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': $!"; > >> > + } else { >> > + if (not exists $cfg->{gitcvs}->{authdb}) { > > Why not elsif? > >> > + 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. > > >> > + 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. >> > + 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-02 21:34 ` Ævar Arnfjörð Bjarmason @ 2010-07-03 9:28 ` Áshin László 2010-07-03 10:49 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 8+ messages in thread From: Áshin László @ 2010-07-03 9:28 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Jakub Narebski, László ÁSHIN, git Hi, On Fri, Jul 2, 2010 at 23:34, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Fri, Jul 2, 2010 at 21:31, Jakub Narebski <jnareb@gmail.com> wrote: >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >>> On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@neti.hu> wrote: >>> >>> > + >>> > + 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': $!"; >> It will be fixed, I will resend the patch. >>> > + } 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. >> >>> > + 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. >> >>> > + 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. >>> > + 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] git-cvsserver: pserver-auth-script 2010-07-03 9:28 ` Áshin László @ 2010-07-03 10:49 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 8+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-03 10:49 UTC (permalink / raw) To: Áshin László Cc: Jakub Narebski, László ÁSHIN, git On Sat, Jul 3, 2010 at 09:28, Áshin László <ashinlaszlo@gmail.com> 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-07-03 10:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-02 7:54 [PATCH] git-cvsserver: pserver-auth-script László ÁSHIN 2010-07-02 14:33 ` Ævar Arnfjörð Bjarmason 2010-07-02 14:39 ` Ævar Arnfjörð Bjarmason 2010-07-02 15:04 ` Ævar Arnfjörð Bjarmason 2010-07-02 21:31 ` Jakub Narebski 2010-07-02 21:34 ` Ævar Arnfjörð Bjarmason 2010-07-03 9:28 ` Áshin László 2010-07-03 10:49 ` Æ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).