All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add contrib/credentials/netrc with GPG support
@ 2013-02-04 19:54 Ted Zlatanov
  2013-02-04 21:17 ` Jeff King
  0 siblings, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 19:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King


Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/git-credential-netrc |  223 +++++++++++++++++++++++++
 1 files changed, 223 insertions(+), 0 deletions(-)
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..7b43aa9
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,223 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+               help => 0,
+               debug => 0,
+
+               # identical token maps, e.g. host -> host, will be inserted later
+               tmap => {
+                        port => 'protocol',
+                        machine => 'host',
+                        path => 'path',
+                        login => 'username',
+                        user => 'username',
+                        password => 'password',
+                       }
+              );
+
+# map each credential protocol token to itself on the netrc side
+$options{tmap}->{$_} = $_ foreach my $v (values %{$options{tmap}});
+
+foreach my $suffix ('.gpg', '') {
+	foreach my $base (qw/authinfo netrc/) {
+		my $file = glob("~/.$base$suffix");
+		next unless (defined $file && -f $file);
+		$options{file} = $file ;
+	}
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "file|f=s",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE] [-d] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+  -f AUTHFILE: specify a netrc-style file
+  -d: turn on debugging
+
+To enable (note that Git will prepend "git-credential-" to the helper
+name and look for it in the path):
+
+  git config credential.helper '$shortname -f AUTHFILE'
+
+And if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+Only "get" mode is supported by this credential helper.  It opens
+AUTHFILE and looks for entries that match the requested search
+criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get "protocol=https\nusername=tzz", this credential
+helper will look for lines in AUTHFILE that match
+
+port https login tzz
+
+OR
+
+protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the line,
+including "password" tokens, mapping e.g. "port" back to "protocol".
+
+The first matching line is used.  Tokens can be quoted as 'STRING' or
+"STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit;
+}
+
+my $mode = shift @ARGV;
+
+# credentials may get 'get', 'store', or 'erase' as parameters but
+# only acknowledge 'get'
+die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
+
+# only support 'get' mode
+exit unless $mode eq 'get';
+
+my $debug = $options{debug};
+my $file = $options{file};
+
+die "Sorry, you need to specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE"
+ unless defined $file;
+
+unless (-f $file) {
+	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
+	exit 0;
+}
+
+my @data;
+if ($file =~ m/\.gpg$/) {
+	@data = load('-|', qw(gpg --decrypt), $file)
+}
+else {
+	@data = load('<', $file);
+}
+
+chomp @data;
+
+unless (scalar @data) {
+	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
+	exit;
+}
+
+# the query: start with every token with no value
+my %q = map { $_ => undef } values(%{$options{tmap}});
+
+while (<STDIN>) {
+	next unless m/([^=]+)=(.+)/;
+
+	my ($token, $value) = ($1, $2);
+	die "Unknown search token $1" unless exists $q{$token};
+	$q{$token} = $value;
+}
+
+# build reverse token map
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+# there are CPAN modules to do this better, but we want to avoid
+# dependencies and generally, complex netrc-style files are rare
+
+if ($debug) {
+	printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
+	 foreach sort keys %q;
+}
+
+LINE: foreach my $line (@data) {
+
+	print STDERR "line [$line]\n" if $debug;
+	my @tok;
+	# gratefully stolen from Net::Netrc
+	while (length $line &&
+	       $line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+		(my $tok = $+) =~ s/\\(.)/$1/g;
+		push(@tok, $tok);
+	}
+
+	# skip blank lines, comments, etc.
+	next LINE unless scalar @tok;
+
+	my %tokens;
+	while (@tok) {
+		my ($k, $v) = (shift @tok, shift @tok);
+		next unless defined $v;
+		next unless exists $options{tmap}->{$k};
+		$tokens{$options{tmap}->{$k}} = $v;
+	}
+
+	foreach my $check (sort keys %q) {
+		if (exists $tokens{$check} && defined $q{$check}) {
+			print STDERR "comparing [$tokens{$check}] to [$q{$check}] in line [$line]\n" if $debug;
+			next LINE unless $tokens{$check} eq $q{$check};
+		}
+		else {
+			print STDERR "we could not find [$check] but it's OK\n" if $debug;
+		}
+	}
+
+	print STDERR "line has passed all the search checks\n" if $debug;
+ TOKEN:
+	foreach my $token (sort keys %rmap) {
+		print STDERR "looking for useful token $token\n" if $debug;
+		next unless exists $tokens{$token}; # did we match?
+
+		foreach my $rctoken (@{$rmap{$token}}) {
+			next TOKEN if defined $q{$rctoken};           # don't re-print given tokens
+		}
+
+		print STDERR "FOUND: $token=$tokens{$token}\n" if $debug;
+		printf "%s=%s\n", $token, $tokens{$token};
+	}
+
+	last;
+}
+
+sub load {
+	# this supports pipes too
+	my $io = new IO::File(@_) or die "Could not open [@_]: $!\n";
+	return <$io>;                          # whole file
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support
  2013-02-04 19:54 [PATCH] Add contrib/credentials/netrc with GPG support Ted Zlatanov
@ 2013-02-04 21:17 ` Jeff King
  2013-02-04 21:32   ` Ted Zlatanov
  2013-02-04 21:44   ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
  0 siblings, 2 replies; 38+ messages in thread
From: Jeff King @ 2013-02-04 21:17 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Junio C Hamano, git

On Mon, Feb 04, 2013 at 02:54:30PM -0500, Ted Zlatanov wrote:

> +	print <<EOHIPPUS;
> +
> +$0 [-f AUTHFILE] [-d] get
> +
> +Version $VERSION by tzz\@lifelogs.com.  License: BSD.

This here-doc is interpolated so you can use $0 and $VERSION, and
therefore have to quote the @-sign. But later in the here-doc...

> +Thus, when we get "protocol=https\nusername=tzz", this credential
> +helper will look for lines in AUTHFILE that match

Do you need to quote "\n" here?

> +die "Sorry, you need to specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE"
> + unless defined $file;
> +
> +unless (-f $file) {
> +	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
> +	exit 0;
> +}

Hmm, so it's not an error (just a warning) to say:

  git credential-netrc -f /does/not/exist

but it is an error to say:

  git credential-netrc

and have it fail to find any netrc files. Shouldn't the latter be a
lesser error than the former?

> +while (<STDIN>) {
> +	next unless m/([^=]+)=(.+)/;
> +
> +	my ($token, $value) = ($1, $2);
> +	die "Unknown search token $1" unless exists $q{$token};
> +	$q{$token} = $value;
> +}

Should this regex be anchored at the start of the string? I think the
left-to-right matching means we will correctly match:

  key=value with=in it

so it may be OK.

> +if ($debug) {
> +	printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
> +	 foreach sort keys %q;
> +}

Leftover one-char indent.

> [...]

The rest looks OK to me.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support
  2013-02-04 21:17 ` Jeff King
@ 2013-02-04 21:32   ` Ted Zlatanov
  2013-02-04 21:44   ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
  1 sibling, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 21:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Mon, 4 Feb 2013 16:17:26 -0500 Jeff King <peff@peff.net> wrote: 

JK> Do you need to quote "\n" here?

Fixed.

JK> Hmm, so it's not an error (just a warning) to say:

JK>   git credential-netrc -f /does/not/exist

JK> but it is an error to say:

JK>   git credential-netrc

JK> and have it fail to find any netrc files. Shouldn't the latter be a
JK> lesser error than the former?

Fixed, they should both exit(0).

>> +	next unless m/([^=]+)=(.+)/;

JK> Should this regex be anchored at the start of the string?

Fixed.

>> +	printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
>> +	 foreach sort keys %q;
JK> Leftover one-char indent.

Fixed.

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 21:17 ` Jeff King
  2013-02-04 21:32   ` Ted Zlatanov
@ 2013-02-04 21:44   ` Ted Zlatanov
  2013-02-04 22:56     ` Junio C Hamano
  2013-02-05 16:15     ` Junio C Hamano
  1 sibling, 2 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 21:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git



Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/git-credential-netrc |  236 +++++++++++++++++++++++++
 1 files changed, 236 insertions(+), 0 deletions(-)
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..d265fde
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,236 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+               help => 0,
+               debug => 0,
+
+               # identical token maps, e.g. host -> host, will be inserted later
+               tmap => {
+                        port => 'protocol',
+                        machine => 'host',
+                        path => 'path',
+                        login => 'username',
+                        user => 'username',
+                        password => 'password',
+                       }
+              );
+
+# map each credential protocol token to itself on the netrc side
+$options{tmap}->{$_} = $_ foreach values %{$options{tmap}};
+
+foreach my $suffix ('.gpg', '') {
+	foreach my $base (qw/authinfo netrc/) {
+		my $file = glob("~/.$base$suffix");
+		next unless (defined $file && -f $file);
+		$options{file} = $file ;
+	}
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "file|f=s",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE] [-d] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+  -f AUTHFILE: specify a netrc-style file
+  -d: turn on debugging
+
+To enable (note that Git will prepend "git-credential-" to the helper
+name and look for it in the path):
+
+  git config credential.helper '$shortname -f AUTHFILE'
+
+And if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+Only "get" mode is supported by this credential helper.  It opens
+AUTHFILE and looks for entries that match the requested search
+criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+protocol=https
+username=tzz
+
+this credential helper will look for lines in AUTHFILE that match
+
+port https login tzz
+
+OR
+
+protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the line,
+including "password" tokens, mapping e.g. "port" back to "protocol".
+
+The first matching line is used.  Tokens can be quoted as 'STRING' or
+"STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit;
+}
+
+my $mode = shift @ARGV;
+
+# credentials may get 'get', 'store', or 'erase' as parameters but
+# only acknowledge 'get'
+die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
+
+# only support 'get' mode
+exit unless $mode eq 'get';
+
+my $debug = $options{debug};
+my $file = $options{file};
+
+unless (defined $file) {
+	print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
+	exit 0;
+}
+
+unless (-f $file) {
+	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
+	exit 0;
+}
+
+my @data;
+if ($file =~ m/\.gpg$/) {
+	@data = load('-|', qw(gpg --decrypt), $file)
+}
+else {
+	@data = load('<', $file);
+}
+
+chomp @data;
+
+unless (scalar @data) {
+	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
+	exit;
+}
+
+# the query: start with every token with no value
+my %q = map { $_ => undef } values(%{$options{tmap}});
+
+while (<STDIN>) {
+	next unless m/^([^=]+)=(.+)/;
+
+	my ($token, $value) = ($1, $2);
+	die "Unknown search token $1" unless exists $q{$token};
+	$q{$token} = $value;
+}
+
+# build reverse token map
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+# there are CPAN modules to do this better, but we want to avoid
+# dependencies and generally, complex netrc-style files are rare
+
+if ($debug) {
+	printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
+		foreach sort keys %q;
+}
+
+LINE: foreach my $line (@data) {
+
+	print STDERR "line [$line]\n" if $debug;
+	my @tok;
+	# gratefully stolen from Net::Netrc
+	while (length $line &&
+	       $line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+		(my $tok = $+) =~ s/\\(.)/$1/g;
+		push(@tok, $tok);
+	}
+
+	# skip blank lines, comments, etc.
+	next LINE unless scalar @tok;
+
+	my %tokens;
+	my $num_port;
+	while (@tok) {
+		my ($k, $v) = (shift @tok, shift @tok);
+		next unless defined $v;
+		next unless exists $options{tmap}->{$k};
+		$tokens{$options{tmap}->{$k}} = $v;
+		$num_port = $v if $k eq 'port' && $v =~ m/^\d+$/;
+	}
+
+	# for "host X port Y" where Y is an integer (captured by
+	# $num_port above), set the host to "X:Y"
+	$tokens{host} = join(':', $tokens{host}, $num_port)
+		if defined $tokens{host} && defined $num_port;
+
+	foreach my $check (sort keys %q) {
+		if (exists $tokens{$check} && defined $q{$check}) {
+			print STDERR "comparing [$tokens{$check}] to [$q{$check}] in line [$line]\n" if $debug;
+			next LINE unless $tokens{$check} eq $q{$check};
+		}
+		else {
+			print STDERR "we could not find [$check] but it's OK\n" if $debug;
+		}
+	}
+
+	print STDERR "line has passed all the search checks\n" if $debug;
+ TOKEN:
+	foreach my $token (sort keys %rmap) {
+		print STDERR "looking for useful token $token\n" if $debug;
+		next unless exists $tokens{$token}; # did we match?
+
+		foreach my $rctoken (@{$rmap{$token}}) {
+			next TOKEN if defined $q{$rctoken};           # don't re-print given tokens
+		}
+
+		print STDERR "FOUND: $token=$tokens{$token}\n" if $debug;
+		printf "%s=%s\n", $token, $tokens{$token};
+	}
+
+	last;
+}
+
+sub load {
+	# this supports pipes too
+	my $io = new IO::File(@_) or die "Could not open [@_]: $!\n";
+	return <$io>;                          # whole file
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 21:44   ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
@ 2013-02-04 22:56     ` Junio C Hamano
  2013-02-04 23:23       ` Jeff King
                         ` (2 more replies)
  2013-02-05 16:15     ` Junio C Hamano
  1 sibling, 3 replies; 38+ messages in thread
From: Junio C Hamano @ 2013-02-04 22:56 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>

The space above your S-o-b: could be utilized a bit better ;-)

> @@ -0,0 +1,236 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;
> +
> +use Getopt::Long;
> +use File::Basename;
> +
> +my $VERSION = "0.1";
> +
> +my %options = (
> +               help => 0,
> +               debug => 0,
> +
> +               # identical token maps, e.g. host -> host, will be inserted later
> +               tmap => {
> +                        port => 'protocol',
> +                        machine => 'host',
> +                        path => 'path',
> +                        login => 'username',
> +                        user => 'username',
> +                        password => 'password',
> +                       }
> +              );
> +
> +# map each credential protocol token to itself on the netrc side
> +$options{tmap}->{$_} = $_ foreach values %{$options{tmap}};
> +
> +foreach my $suffix ('.gpg', '') {
> +	foreach my $base (qw/authinfo netrc/) {
> +		my $file = glob("~/.$base$suffix");
> +		next unless (defined $file && -f $file);
> +		$options{file} = $file ;
> +	}
> +}

This checks .gpg first and then unencrypted, and checks authinfo
first and netrc second, both of which makes sense.  It is good to
encourage use of encrypted files, and it is good to use newer
authinfo files over netrc files.

However, it is strange that you let the ones that are discovered
later in the loop to override the ones that are discovered earlier.
Perhaps you meant

	next unless (... exists there ...);
        $options{"file"} = $file;
        last;

instead?

> +Getopt::Long::Configure("bundling");

Hmm, OK.

> +# TODO: maybe allow the token map $options{tmap} to be configurable.
> +GetOptions(\%options,
> +           "help|h",
> +           "debug|d",
> +           "file|f=s",
> +          );
> +
> +if ($options{help}) {
> +	my $shortname = basename($0);
> +	$shortname =~ s/git-credential-//;
> +
> +	print <<EOHIPPUS;
> +
> +$0 [-f AUTHFILE] [-d] get
> +
> +Version $VERSION by tzz\@lifelogs.com.  License: BSD.
> +
> +Options:
> +  -f AUTHFILE: specify a netrc-style file
> +  -d: turn on debugging
> +
> +To enable (note that Git will prepend "git-credential-" to the helper
> +name and look for it in the path):
> +
> +  git config credential.helper '$shortname -f AUTHFILE'
> +
> +And if you want lots of debugging info:
> +
> +  git config credential.helper '$shortname -f AUTHFILE -d'
> +
> +Only "get" mode is supported by this credential helper.  It opens
> +AUTHFILE and looks for entries that match the requested search
> +criteria:
> +
> + 'port|protocol':
> +   The protocol that will be used (e.g., https). (protocol=X)
> +
> + 'machine|host':
> +   The remote hostname for a network credential. (host=X)
> +
> + 'path':
> +   The path with which the credential will be used. (path=X)
> +
> + 'login|user|username':
> +   The credential’s username, if we already have one. (username=X)
> +
> +Thus, when we get this query on STDIN:
> +
> +protocol=https
> +username=tzz
> +
> +this credential helper will look for lines in AUTHFILE that match
> +
> +port https login tzz
> +
> +OR
> +
> +protocol https login tzz
> +
> +OR... etc. acceptable tokens as listed above.  Any unknown tokens are
> +simply ignored.

I recall that netrc/authinfo files are _not_ line oriented.  Earlier
you said "looks for entries that match" which is a lot more correct,
but then we see "look for lines in authfile".

> +Then, the helper will print out whatever tokens it got from the line,
> +including "password" tokens, mapping e.g. "port" back to "protocol".

Again "line" is mentioned twice, above and below.

> +The first matching line is used.  Tokens can be quoted as 'STRING' or
> +"STRING".
> +
> +No caching is performed by this credential helper.
> +
> +EOHIPPUS
> +
> +	exit;
> +}

> +my $mode = shift @ARGV;
> +
> +# credentials may get 'get', 'store', or 'erase' as parameters but
> +# only acknowledge 'get'
> +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
> +
> +# only support 'get' mode
> +exit unless $mode eq 'get';

The above looks strange.  Why does the invoker get the error message
only when it runs this without arguments?  Did you mean to say more
like this?

	unless (defined $mode && $mode eq 'get') {
		die "...";
	}

By the way, I think statement modifiers tend to get overused and
make the resulting program harder to read.  die "..." at the
beginning of line makes the reader go "Whoa, it already is done and
existing on error", and then forces the eyes to scan the error
message to find "unless" and the condition.

It may be a cute syntax and some may find it even cool, but cuteness
or coolness is less valuable compared with the readability.

> +my $debug = $options{debug};
> +my $file = $options{file};
> +
> +unless (defined $file) {
> +	print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
> +	exit 0;
> +}
> +
> +unless (-f $file) {
> +	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
> +	exit 0;
> +}

Perhaps "-r $file", if you say "is not accessible"?

Is it sensible to squelch the error message by default and force
user to specify --debug?  You could argue that the option is to
debug the user's configuration, but the name of the option sounds
more like it is for debugging this script itself.

I saw Peff already pointed out error conditions, but I am not sure
why all of these exit with 0.  If the user has configured

	git config credential.helper 'netrc -f $HOME/.netcr'

shouldn't it be diagnosed as an error?  It is understandable to let
this go silently

	git config credential.helper 'netrc'

and let other credential helpers take over when no $HOME/.{netrc,authinfo}{,.gpg}
file exist, but in that case the user may still want to remove the
config item that is not doing anything useful and erroring out with
a message may be a way to help the user know about the situation.

> +my @data;
> +if ($file =~ m/\.gpg$/) {
> +	@data = load('-|', qw(gpg --decrypt), $file)
> +}
> +else {
> +	@data = load('<', $file);
> +}
> +
> +chomp @data;
> +
> +unless (scalar @data) {

Shouldn't this error check come logically before chomping?

> +	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
> +	exit;
> +}

Is this really an error?  The file perhaps was empty.  Shouldn't
that case treated the same way as the case where no entry that
matches the criteria invoker gave you was found?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 22:56     ` Junio C Hamano
@ 2013-02-04 23:23       ` Jeff King
  2013-02-04 23:36         ` Junio C Hamano
  2013-02-04 23:42         ` Ted Zlatanov
  2013-02-04 23:28       ` [PATCHv3] Add contrib/credentials/netrc with GPG support Ted Zlatanov
  2013-02-04 23:31       ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
  2 siblings, 2 replies; 38+ messages in thread
From: Jeff King @ 2013-02-04 23:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ted Zlatanov, git

On Mon, Feb 04, 2013 at 02:56:06PM -0800, Junio C Hamano wrote:

> > +my $mode = shift @ARGV;
> > +
> > +# credentials may get 'get', 'store', or 'erase' as parameters but
> > +# only acknowledge 'get'
> > +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
> > +
> > +# only support 'get' mode
> > +exit unless $mode eq 'get';
> 
> The above looks strange.  Why does the invoker get the error message
> only when it runs this without arguments?  Did you mean to say more
> like this?
> 
> 	unless (defined $mode && $mode eq 'get') {
> 		die "...";
> 	}

Not having a mode is an invocation error; the credential-helper
documentation indicates that the helper will always be invoked with an
action. The likely culprit for not having one is the user invoking it
manually, and showing the usage there is a sensible action.

Whereas invoking it with a mode other than "get" is not an error at all.
Git will run it with the "store" and "erase" actions, too. Those happen
to be no-ops for this helper, so it exits silently. The credential docs
specify that any other actions should be ignored, too, to allow for
future expansion.

> > +my $debug = $options{debug};
> > +my $file = $options{file};
> > +
> > +unless (defined $file) {
> > +	print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
> > +	exit 0;
> > +}
> > +
> > +unless (-f $file) {
> > +	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
> > +	exit 0;
> > +}
> 
> Perhaps "-r $file", if you say "is not accessible"?

Even better: look at whether opening the file was successful. Though I
guess that is complicated by the use of gpg, who will probably not
distinguish ENOENT from other failures for us.

> Is it sensible to squelch the error message by default and force
> user to specify --debug?  You could argue that the option is to
> debug the user's configuration, but the name of the option sounds
> more like it is for debugging this script itself.
> 
> I saw Peff already pointed out error conditions, but I am not sure
> why all of these exit with 0.  If the user has configured

It was from my suggestion to ignore missing files, which is that the
user might have the helper configured (e.g., via /etc/gitconfig, or by a
shared ~/.gitconfig) but not actually have a netrc.

It gets confusing because the contents of $file may have been
auto-detected, or it may have come from the command-line, and we do not
remember which at this point.

I was trying not to be too nit-picky with my review, but here is how I
would have written the outer logic of the script:

  my $tokens = read_credential_data_from_stdin();
  if ($options{file}) {
          my @entries = load_netrc($options{file})
                  or die "unable to open $options{file}: $!";
          check_netrc($tokens, @entries);
  }
  else {
          foreach my $ext ('.gpg', '') {
                  foreach my $base (qw(authinfo netrc)) {
                          my @entries = load_netrc("$base$ext")
                                  or next;
                          if (check_netrc($tokens, @entries)) {
                                  last;
                          }
                  }
          }
  }

I.e., to fail on "-f", but otherwise treat unreadable auto-selected
files as a no-op, for whatever reason. I'd also consider checking all
files if they are available, in case the user has multiple (e.g., they
keep low-quality junk unencrypted but some high-security passwords in a
.gpg file). Not that likely, but not any harder to implement.

-Peff

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv3] Add contrib/credentials/netrc with GPG support
  2013-02-04 22:56     ` Junio C Hamano
  2013-02-04 23:23       ` Jeff King
@ 2013-02-04 23:28       ` Ted Zlatanov
  2013-02-04 23:31       ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
  2 siblings, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 23:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

Changes since PATCHv2:

- don't keep looking at netrc candidates if one good one is found

- fixed wording of "line" to "entry" everywhere suitable

- many (but not all) statement modifiers changed to block format

- use -r everywhere instead of -f

- move chomp to when we know @data has contents

Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/git-credential-netrc |  243 +++++++++++++++++++++++++
 1 files changed, 243 insertions(+), 0 deletions(-)
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..99ab204
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,243 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+               help => 0,
+               debug => 0,
+
+               # identical token maps, e.g. host -> host, will be inserted later
+               tmap => {
+                        port => 'protocol',
+                        machine => 'host',
+                        path => 'path',
+                        login => 'username',
+                        user => 'username',
+                        password => 'password',
+                       }
+              );
+
+# map each credential protocol token to itself on the netrc side
+foreach (values %{$options{tmap}}) {
+	$options{tmap}->{$_} = $_;
+}
+
+FILE:
+foreach my $suffix ('.gpg', '') {
+	foreach my $base (qw/authinfo netrc/) {
+		my $file = glob("~/.$base$suffix");
+		next unless (defined $file && -r $file);
+		$options{file} = $file;
+		last FILE;
+	}
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "file|f=s",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE] [-d] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+  -f AUTHFILE: specify a netrc-style file
+  -d: turn on debugging
+
+To enable (note that Git will prepend "git-credential-" to the helper
+name and look for it in the path):
+
+  git config credential.helper '$shortname -f AUTHFILE'
+
+And if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+Only "get" mode is supported by this credential helper.  It opens
+AUTHFILE and looks for entries that match the requested search
+criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+protocol=https
+username=tzz
+
+this credential helper will look for entries in AUTHFILE that match
+
+port https login tzz
+
+OR
+
+protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry,
+including "password" tokens, mapping e.g. "port" back to "protocol".
+
+The first matching entry is used.  Tokens can be quoted as 'STRING' or
+"STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit;
+}
+
+my $mode = shift @ARGV;
+
+# credentials may get 'get', 'store', or 'erase' as parameters but
+# only acknowledge 'get'
+die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
+
+# only support 'get' mode
+exit unless $mode eq 'get';
+
+my $debug = $options{debug};
+my $file = $options{file};
+
+unless (defined $file) {
+	print STDERR "Please specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\n" if $debug;
+	exit 0;
+}
+
+unless (-r $file) {
+	print STDERR "Sorry, the specified netrc $file is not accessible\n" if $debug;
+	exit 0;
+}
+
+my @data;
+if ($file =~ m/\.gpg$/) {
+	@data = load('-|', qw(gpg --decrypt), $file)
+}
+else {
+	@data = load('<', $file);
+}
+
+unless (scalar @data) {
+	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
+	exit;
+}
+
+chomp @data;
+
+# the query: start with every token with no value
+my %q = map { $_ => undef } values(%{$options{tmap}});
+
+while (<STDIN>) {
+	next unless m/^([^=]+)=(.+)/;
+
+	my ($token, $value) = ($1, $2);
+	die "Unknown search token $1" unless exists $q{$token};
+	$q{$token} = $value;
+}
+
+# build reverse token map
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+# there are CPAN modules to do this better, but we want to avoid
+# dependencies and generally, complex netrc-style files are rare
+
+if ($debug) {
+	foreach (sort keys %q) {
+		printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)';
+	}
+}
+
+LINE: foreach my $line (@data) {
+
+	print STDERR "line [$line]\n" if $debug;
+	my @tok;
+	# gratefully stolen from Net::Netrc
+	while (length $line &&
+	       $line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+		(my $tok = $+) =~ s/\\(.)/$1/g;
+		push(@tok, $tok);
+	}
+
+	# skip blank lines, comments, etc.
+	next LINE unless scalar @tok;
+
+	my %tokens;
+	my $num_port;
+	while (@tok) {
+		my ($k, $v) = (shift @tok, shift @tok);
+		next unless defined $v;
+		next unless exists $options{tmap}->{$k};
+		$tokens{$options{tmap}->{$k}} = $v;
+		$num_port = ($k eq 'port' && $v =~ m/^\d+$/) ? $v : undef;
+	}
+
+	# for "host X port Y" where Y is an integer (captured by
+	# $num_port above), set the host to "X:Y"
+	if (defined $tokens{host} && defined $num_port) {
+		$tokens{host} = join(':', $tokens{host}, $num_port);
+	}
+
+	foreach my $check (sort keys %q) {
+		if (exists $tokens{$check} && defined $q{$check}) {
+			print STDERR "comparing [$tokens{$check}] to [$q{$check}] in entry [$line]\n" if $debug;
+			next LINE unless $tokens{$check} eq $q{$check};
+		}
+		else {
+			print STDERR "we could not find [$check] but it's OK\n" if $debug;
+		}
+	}
+
+	print STDERR "entry has passed all the search checks\n" if $debug;
+ TOKEN:
+	foreach my $token (sort keys %rmap) {
+		print STDERR "looking for useful token $token\n" if $debug;
+		next unless exists $tokens{$token}; # did we match?
+
+		foreach my $rctoken (@{$rmap{$token}}) {
+			# don't re-print given tokens
+			next TOKEN if defined $q{$rctoken};
+		}
+
+		print STDERR "FOUND: $token=$tokens{$token}\n" if $debug;
+		printf "%s=%s\n", $token, $tokens{$token};
+	}
+
+	last;
+}
+
+sub load {
+	# this supports pipes too
+	my $io = new IO::File(@_) or die "Could not open [@_]: $!\n";
+	return <$io>;                          # whole file
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 22:56     ` Junio C Hamano
  2013-02-04 23:23       ` Jeff King
  2013-02-04 23:28       ` [PATCHv3] Add contrib/credentials/netrc with GPG support Ted Zlatanov
@ 2013-02-04 23:31       ` Ted Zlatanov
  2013-02-04 23:40         ` Junio C Hamano
  2 siblings, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 23:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Mon, 04 Feb 2013 14:56:06 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> I recall that netrc/authinfo files are _not_ line oriented.  Earlier
JCH> you said "looks for entries that match" which is a lot more correct,
JCH> but then we see "look for lines in authfile".

Hmm, do you mean backslashed newlines?  I think the Net::Netrc parser
doesn't support them, and I haven't seen them in the wild, but I could
support them if you think that's useful.

>> +my $mode = shift @ARGV;
>> +
>> +# credentials may get 'get', 'store', or 'erase' as parameters but
>> +# only acknowledge 'get'
>> +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
>> +
>> +# only support 'get' mode
>> +exit unless $mode eq 'get';

JCH> The above looks strange.  Why does the invoker get the error message
JCH> only when it runs this without arguments?  Did you mean to say more
JCH> like this?

JCH> 	unless (defined $mode && $mode eq 'get') {
JCH> 		die "...";
JCH> 	}

I mean:

- if the mode is not given, exit badly (since it's required)

- if the mode is given but we don't support it, exit pleasantly

I thought that was the right thing, according to my reading of the
credentials API.  If not, I'll be glad to change it.

JCH> By the way, I think statement modifiers tend to get overused and
JCH> make the resulting program harder to read.  die "..." at the
JCH> beginning of line makes the reader go "Whoa, it already is done and
JCH> existing on error", and then forces the eyes to scan the error
JCH> message to find "unless" and the condition.

JCH> It may be a cute syntax and some may find it even cool, but cuteness
JCH> or coolness is less valuable compared with the readability.

Your coding guidelines said you prefer one-line if statements, and I
thought it would be OK to lean on modifiers.  I changed many of the
modifiers but not all; please let me know if you'd like me to change
them all.  It's no problem.

JCH> Is it sensible to squelch the error message by default and force
JCH> user to specify --debug?  You could argue that the option is to
JCH> debug the user's configuration, but the name of the option sounds
JCH> more like it is for debugging this script itself.

It's both... without a clear separation because it's such a small
script.  Let me know how you'd like to change it, if at all.

JCH> I saw Peff already pointed out error conditions, but I am not sure
JCH> why all of these exit with 0.  If the user has configured

JCH> 	git config credential.helper 'netrc -f $HOME/.netcr'

JCH> shouldn't it be diagnosed as an error?  It is understandable to let
JCH> this go silently

JCH> 	git config credential.helper 'netrc'

JCH> and let other credential helpers take over when no $HOME/.{netrc,authinfo}{,.gpg}
JCH> file exist, but in that case the user may still want to remove the
JCH> config item that is not doing anything useful and erroring out with
JCH> a message may be a way to help the user know about the situation.

You and Peff should tell me how it should behave, or perhaps make the
changes after it's in.  I'm happy to change it any way you like, but at
this point I'm just following instructions, not really contributing,
about the exit statuses.  I thought I knew what you wanted 2 iterations
ago :)

>> +	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
>> +	exit;

JCH> Is this really an error?  The file perhaps was empty.  Shouldn't
JCH> that case treated the same way as the case where no entry that
JCH> matches the criteria invoker gave you was found?

exit(0) is not an error, so the behavior is exactly the same, we just
don't print anything to STDOUT because there was no data, with a nicer
error message.  I think that's what we want?

PATCHv3 is out with the rest of your suggestions.  Thank you for the
thorough review.  I am happy to improve the script to meet your standards.

Thanks
Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 23:23       ` Jeff King
@ 2013-02-04 23:36         ` Junio C Hamano
  2013-02-04 23:42         ` Ted Zlatanov
  1 sibling, 0 replies; 38+ messages in thread
From: Junio C Hamano @ 2013-02-04 23:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Ted Zlatanov, git

Jeff King <peff@peff.net> writes:

> On Mon, Feb 04, 2013 at 02:56:06PM -0800, Junio C Hamano wrote:
>
>> > +my $mode = shift @ARGV;
>> > +
>> > +# credentials may get 'get', 'store', or 'erase' as parameters but
>> > +# only acknowledge 'get'
>> > +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
>> > +
>> > +# only support 'get' mode
>> > +exit unless $mode eq 'get';
>> 
>> The above looks strange.  Why does the invoker get the error message
>> only when it runs this without arguments?  Did you mean to say more
>> like this?
>> 
>> 	unless (defined $mode && $mode eq 'get') {
>> 		die "...";
>> 	}
>
> Not having a mode is an invocation error; the credential-helper
> documentation indicates that the helper will always be invoked with an
> action. The likely culprit for not having one is the user invoking it
> manually, and showing the usage there is a sensible action.
>
> Whereas invoking it with a mode other than "get" is not an error at all.
> Git will run it with the "store" and "erase" actions, too. Those happen
> to be no-ops for this helper, so it exits silently. The credential docs
> specify that any other actions should be ignored, too, to allow for
> future expansion.

OK.  The code didn't express the above reasoning clearly enough.

> I was trying not to be too nit-picky with my review,...

I wasn't either.  Mine was still at design level review to get the
semantics right (e.g. what to consider as errors, the input is _not_
one entry per line, etc.), before reviewing the details of the
implementation.

> but here is how I
> would have written the outer logic of the script:
>
>   my $tokens = read_credential_data_from_stdin();
>   if ($options{file}) {
>           my @entries = load_netrc($options{file})
>                   or die "unable to open $options{file}: $!";
>           check_netrc($tokens, @entries);
>   }
>   else {
>           foreach my $ext ('.gpg', '') {
>                   foreach my $base (qw(authinfo netrc)) {
>                           my @entries = load_netrc("$base$ext")
>                                   or next;
>                           if (check_netrc($tokens, @entries)) {
>                                   last;
>                           }
>                   }
>           }
>   }
>
> I.e., to fail on "-f", but otherwise treat unreadable auto-selected
> files as a no-op, for whatever reason. I'd also consider checking all
> files if they are available, in case the user has multiple (e.g., they
> keep low-quality junk unencrypted but some high-security passwords in a
> .gpg file). Not that likely, but not any harder to implement.

Yeah, I think that looks like the right top-level codeflow.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 23:31       ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
@ 2013-02-04 23:40         ` Junio C Hamano
  2013-02-04 23:54           ` Ted Zlatanov
  0 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-04 23:40 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

>>> +my $mode = shift @ARGV;
>>> +
>>> +# credentials may get 'get', 'store', or 'erase' as parameters but
>>> +# only acknowledge 'get'
>>> +die "Syntax: $0 [-f AUTHFILE] [-d] get" unless defined $mode;
>>> +
>>> +# only support 'get' mode
>>> +exit unless $mode eq 'get';
>
> JCH> The above looks strange.  Why does the invoker get the error message
> JCH> only when it runs this without arguments?  Did you mean to say more
> JCH> like this?
>
> JCH> 	unless (defined $mode && $mode eq 'get') {
> JCH> 		die "...";
> JCH> 	}
>
> I mean:
>
> - if the mode is not given, exit badly (since it's required)
>
> - if the mode is given but we don't support it, exit pleasantly
>
> I thought that was the right thing, according to my reading of the
> credentials API.  If not, I'll be glad to change it.

As Peff noted, I mistead what the code was doing, especially with
somewhat cryptic "only support x mode" comment, as if it is
rejecting other modes.

>>> +	print STDERR "Sorry, we could not load data from [$file]\n" if $debug;
>>> +	exit;
>
> JCH> Is this really an error?  The file perhaps was empty.  Shouldn't
> JCH> that case treated the same way as the case where no entry that
> JCH> matches the criteria invoker gave you was found?
>
> exit(0) is not an error, so the behavior is exactly the same, we just
> don't print anything to STDOUT because there was no data, with a nicer
> error message.  I think that's what we want?

"Sorry we couldn't" sounded like an error messag to me.  If this is
a normal exit, then please make sure it is a normal exit.

The review cycle is not like reviewers give you instructions and
designs and you blindly implement them.  It is a creative process
where you show the design and a clear implementation of that design.

Thanks.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 23:23       ` Jeff King
  2013-02-04 23:36         ` Junio C Hamano
@ 2013-02-04 23:42         ` Ted Zlatanov
  1 sibling, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 23:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Mon, 4 Feb 2013 18:23:17 -0500 Jeff King <peff@peff.net> wrote: 

>> Perhaps "-r $file", if you say "is not accessible"?

JK> Even better: look at whether opening the file was successful. Though I
JK> guess that is complicated by the use of gpg, who will probably not
JK> distinguish ENOENT from other failures for us.

Yup.  I think the outcome for the user will be the same, so this is
mostly for debugging, right?  And we do look at the outcome of opening
the file, and die if that failed (which would change if your suggestion
below is implemented).

JK> I was trying not to be too nit-picky with my review, but here is how I
JK> would have written the outer logic of the script:

JK>   my $tokens = read_credential_data_from_stdin();
JK>   if ($options{file}) {
JK>           my @entries = load_netrc($options{file})
JK>                   or die "unable to open $options{file}: $!";
JK>           check_netrc($tokens, @entries);
JK>   }
JK>   else {
JK>           foreach my $ext ('.gpg', '') {
JK>                   foreach my $base (qw(authinfo netrc)) {
JK>                           my @entries = load_netrc("$base$ext")
JK>                                   or next;
JK>                           if (check_netrc($tokens, @entries)) {
JK>                                   last;
JK>                           }
JK>                   }
JK>           }
JK>   }

JK> I.e., to fail on "-f", but otherwise treat unreadable auto-selected
JK> files as a no-op, for whatever reason.

JK> I'd also consider checking all files if they are available, in case
JK> the user has multiple (e.g., they keep low-quality junk unencrypted
JK> but some high-security passwords in a .gpg file). Not that likely,
JK> but not any harder to implement.

I think that makes everything more complicated, and the user can name a
specific netrc file in the helper spec if he wants it.  It's too
automagic for me.  But if you and Junio feel this is the right approach,
I'll rewrite to basically allow --file to take a list of filenames and
default that list to the base list of ~/.{authinfo,netrc}{,.gpg}

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 23:40         ` Junio C Hamano
@ 2013-02-04 23:54           ` Ted Zlatanov
  2013-02-05  0:15             ` Junio C Hamano
  0 siblings, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-04 23:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Mon, 04 Feb 2013 15:40:32 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> "Sorry we couldn't" sounded like an error messag to me.  If this is
JCH> a normal exit, then please make sure it is a normal exit.

OK; done in PATCHv4: removed all "Sorry" because they are not abnormal
exits.  I'll hold PATCHv4 until the below are known.

JCH> The review cycle is not like reviewers give you instructions and
JCH> designs and you blindly implement them.  It is a creative process
JCH> where you show the design and a clear implementation of that design.

OK.  I would like you to make the decisions I asked for, though:

- do you want to support backslashed newlines?
- do you want me to remove the statement modifiers?
- should all die() calls just print to STDERR and exit(0)?
- do you want to support multiple netrc files, as you and Peff suggested?

On all of those, I can go either way, it's just a little more work for me.

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 23:54           ` Ted Zlatanov
@ 2013-02-05  0:15             ` Junio C Hamano
  2013-02-05 13:39               ` Ted Zlatanov
  0 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05  0:15 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Mon, 04 Feb 2013 15:40:32 -0800 Junio C Hamano <gitster@pobox.com> wrote: 
>
> JCH> "Sorry we couldn't" sounded like an error messag to me.  If this is
> JCH> a normal exit, then please make sure it is a normal exit.
>
> OK; done in PATCHv4: removed all "Sorry" because they are not abnormal
> exits.  I'll hold PATCHv4 until the below are known.
>
> JCH> The review cycle is not like reviewers give you instructions and
> JCH> designs and you blindly implement them.  It is a creative process
> JCH> where you show the design and a clear implementation of that design.
>
> OK.  I would like you to make the decisions I asked for, though:
>
> - do you want to support backslashed newlines?

What for?  netrc/authinfo is not a line oriented file format at all,
and

	machine k.org
	        	login me
                        password mysecret

is a single entry; you do not need backslash at the end of any line.

Perhaps you are asking something different?

> - do you want me to remove the statement modifiers?

I do not think we are at that "implementation nitpick" level yet.

> - should all die() calls just print to STDERR and exit(0)?

Where "when unhandled, the helper should silently exit with 0" is
expected by the invoker, we shouldn't say anything to error stream,
and exit with zero.  Please leave a comment to make it easy to
understand to the readers that is what is going on there.

If on the other hand it diagnosed an error (not a bug in the
implementation but a misconfiguration on the user's side), I _think_
it should loudly die() so that the user can notice and take
corrective action.

> - do you want to support multiple netrc files, as you and Peff suggested?

I didn't even suggest such thing IIRC---I expected it to iterate
from the most desirable (.authinfo.gpg) to the least (.netrc) and
stop at the first found one.  There may be use cases people use more
than one and expect an entry to be found in any file, but I suspect
that might be more confusing than it is worth.  But I do not care
very deeply myself either way.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05  0:15             ` Junio C Hamano
@ 2013-02-05 13:39               ` Ted Zlatanov
  2013-02-05 16:07                 ` Junio C Hamano
  0 siblings, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 13:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Mon, 04 Feb 2013 16:15:47 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:

>> - do you want to support backslashed newlines?

JCH> What for?  netrc/authinfo is not a line oriented file format at all,
JCH> and

JCH> 	machine k.org
JCH> 	        	login me
JCH>                         password mysecret

JCH> is a single entry; you do not need backslash at the end of any line.

Hmm. The parser I implemented only does single-line parsing, and I
misunderstood the format to be single-line (partly because I have never
seen anyone using the multi-line format you show).  Looking at
Net::Netrc more carefully, it seems that the "machine" token is what
defines an entry, so a new entry starts with a new line that contains a
"machine" token.  Is that acceptable and does it match your
understanding of the format?  It matches Net::Netrc, at least.

I'll add this change to PATCHv4 with the assumption you agree.

>> - should all die() calls just print to STDERR and exit(0)?

JCH> Where "when unhandled, the helper should silently exit with 0" is
JCH> expected by the invoker, we shouldn't say anything to error stream,
JCH> and exit with zero.  Please leave a comment to make it easy to
JCH> understand to the readers that is what is going on there.

JCH> If on the other hand it diagnosed an error (not a bug in the
JCH> implementation but a misconfiguration on the user's side), I _think_
JCH> it should loudly die() so that the user can notice and take
JCH> corrective action.

OK, I'll review these for PATCHv4 (also see below).  Thanks.

>> - do you want to support multiple netrc files, as you and Peff suggested?

JCH> I didn't even suggest such thing IIRC---I expected it to iterate
JCH> from the most desirable (.authinfo.gpg) to the least (.netrc) and
JCH> stop at the first found one.  There may be use cases people use more
JCH> than one and expect an entry to be found in any file, but I suspect
JCH> that might be more confusing than it is worth.  But I do not care
JCH> very deeply myself either way.

After thinking about it, I agree with Peff multiple sources make sense
and will simplify the code flow (especially the default case, which
won't need to be handled separately).  And the functionality doesn't
have to be confusing with the right debugging messages.  So I'll add
them in PATCHv4.

The debugging messages will be fewer and simpler with this approach,
which makes it feel like the right track :)

Thanks
Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 13:39               ` Ted Zlatanov
@ 2013-02-05 16:07                 ` Junio C Hamano
  2013-02-05 16:18                   ` Junio C Hamano
  0 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 16:07 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Mon, 04 Feb 2013 16:15:47 -0800 Junio C Hamano <gitster@pobox.com> wrote: 
>
> JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>>> - do you want to support backslashed newlines?
>
> JCH> What for?  netrc/authinfo is not a line oriented file format at all,
> JCH> and
>
> JCH> 	machine k.org
> JCH> 	        	login me
> JCH>                         password mysecret
>
> JCH> is a single entry; you do not need backslash at the end of any line.
>
> Hmm. The parser I implemented only does single-line parsing, and I
> misunderstood the format to be single-line (partly because I have never
> seen anyone using the multi-line format you show).  Looking at
> Net::Netrc more carefully, it seems that the "machine" token is what
> defines an entry, so a new entry starts with a new line that contains a
> "machine" token.  Is that acceptable and does it match your
> understanding of the format?  It matches Net::Netrc, at least.

I thought I've given a more concrete outline than "I'll read
Net::Netrc and do whatever I think it does" in a separate message.

It would be better to read "man netrc" carefully at least once ;-)

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-04 21:44   ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
  2013-02-04 22:56     ` Junio C Hamano
@ 2013-02-05 16:15     ` Junio C Hamano
  2013-02-05 17:01       ` Ted Zlatanov
  1 sibling, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 16:15 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> +# build reverse token map
> +my %rmap;
> +foreach my $k (keys %{$options{tmap}}) {
> +	push @{$rmap{$options{tmap}->{$k}}}, $k;
> +}

Mental note: "$rmap{foo} -eq 'bar'" means that what Git calls 'bar'
is found as 'foo' in the netrc/authinfo file.  Keys in %rmap are
what we expect to read from the netrc/authinfo file.

> +# there are CPAN modules to do this better, but we want to avoid
> +# dependencies and generally, complex netrc-style files are rare
> +
> +if ($debug) {
> +	printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
> +		foreach sort keys %q;
> +}
> +
> +LINE: foreach my $line (@data) {
> +
> +	print STDERR "line [$line]\n" if $debug;
> +	my @tok;
> +	# gratefully stolen from Net::Netrc
> +	while (length $line &&
> +	       $line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
> +		(my $tok = $+) =~ s/\\(.)/$1/g;
> +		push(@tok, $tok);
> +	}
> +
> +	# skip blank lines, comments, etc.
> +	next LINE unless scalar @tok;
> +
> +	my %tokens;
> +	my $num_port;
> +	while (@tok) {
> +		my ($k, $v) = (shift @tok, shift @tok);
> +		next unless defined $v;
> +		next unless exists $options{tmap}->{$k};
> +		$tokens{$options{tmap}->{$k}} = $v;
> +		$num_port = $v if $k eq 'port' && $v =~ m/^\d+$/;
> +	}

So you grabbed one line of input, split them into token pairs, and
built %tokens = ('key Git may want to see' => 'value read from file')
mapping.

> +	# for "host X port Y" where Y is an integer (captured by
> +	# $num_port above), set the host to "X:Y"
> +	$tokens{host} = join(':', $tokens{host}, $num_port)
> +		if defined $tokens{host} && defined $num_port;

What happens when 'host' does not exist?  netrc/authinfo should be a
stream of SP/HT/LF delimited tokens and 'machine' token (or
'default') begins a new entry, so it would mean the input file is
corrupt if we do not have $tokens{host} when we get here, I think.

Oh, another thing. 'default' is like 'machine' followed by any
machine name, so the above while loop that reads two tokens
pair-wise needs to be aware that 'default' is not followed by a
value.  I think the loop will fail to parse this:

        default       login anonymous    password me@home
        machine k.org login me           password mysecret

> +	foreach my $check (sort keys %q) {

Hmph, aren't you checking what you read a bit too early?  This is a
valid input:

        default       
                login anonymous
                password me@home
        machine k.org
                login me
                password mysecret

but does this loop gives mysecret back to me when asked for
host=k.org and user=me? 

> +		if (exists $tokens{$check} && defined $q{$check}) {
> +			print STDERR "comparing [$tokens{$check}] to [$q{$check}] in line [$line]\n" if $debug;
> +			next LINE unless $tokens{$check} eq $q{$check};
> +		}
> +		else {
> +			print STDERR "we could not find [$check] but it's OK\n" if $debug;
> +		}
> +	}

I would probably structure this part like this:

	%pending = ();
        split the whole input into tokens, regardless of lines;
        iterate over the tokens {
		peek the token
		if (it is not "default") {
			take (token, value) pair;
		} else {
			take "default" as token; value does not matter.
		}
                if (token is "default" or "machine") {
			# finished reading one entry and we are
                        # at the beginning of the next entry.
                        # see if this entry matches
			if (%pending is not empty &&
                            %pending matches %q) {
				found a match; use %pending;
			}
                        # done with that entry. now start a new one.
                        %pending = ();
		}
		$pending{token} = value;
	}

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 16:07                 ` Junio C Hamano
@ 2013-02-05 16:18                   ` Junio C Hamano
  0 siblings, 0 replies; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 16:18 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Junio C Hamano <gitster@pobox.com> writes:

> I thought I've given a more concrete outline than "I'll read
> Net::Netrc and do whatever I think it does" in a separate message.

And it turns out that the message was sitting in my outbox.  Sorry.

I just told the outbox to send it out.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 16:15     ` Junio C Hamano
@ 2013-02-05 17:01       ` Ted Zlatanov
  2013-02-05 18:55         ` [PATCHv4] Add contrib/credentials/netrc with GPG support Ted Zlatanov
  2013-02-05 19:47         ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Junio C Hamano
  0 siblings, 2 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 17:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Tue, 05 Feb 2013 08:15:48 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> +# build reverse token map
>> +my %rmap;
>> +foreach my $k (keys %{$options{tmap}}) {
>> +	push @{$rmap{$options{tmap}->{$k}}}, $k;
>> +}

JCH> Mental note: "$rmap{foo} -eq 'bar'" means that what Git calls 'bar'
JCH> is found as 'foo' in the netrc/authinfo file.  Keys in %rmap are
JCH> what we expect to read from the netrc/authinfo file.

I'll document that better in PATCHv4.

JCH> So you grabbed one line of input, split them into token pairs, and
JCH> built %tokens = ('key Git may want to see' => 'value read from file')
JCH> mapping.

This will be fixed with PATCHv4 to do multiple lines (as defined by the
netrc manpage, etc.).

>> +	# for "host X port Y" where Y is an integer (captured by
>> +	# $num_port above), set the host to "X:Y"
>> +	$tokens{host} = join(':', $tokens{host}, $num_port)
>> +		if defined $tokens{host} && defined $num_port;

JCH> What happens when 'host' does not exist?  netrc/authinfo should be a
JCH> stream of SP/HT/LF delimited tokens and 'machine' token (or
JCH> 'default') begins a new entry, so it would mean the input file is
JCH> corrupt if we do not have $tokens{host} when we get here, I think.

Yes.  I'll make the host/machine token required, which will avoid this
issue.

JCH> Oh, another thing. 'default' is like 'machine' followed by any
JCH> machine name, so the above while loop that reads two tokens
JCH> pair-wise needs to be aware that 'default' is not followed by a
JCH> value.  I think the loop will fail to parse this:

JCH>         default       login anonymous    password me@home
JCH>         machine k.org login me           password mysecret

I'd prefer to ignore "default" because it should not be used for the Git
credential helpers (its only use case is for anonymous services AFAIK).
So I'll add a case to ignore it in PATCHv4, if that's OK.

JCH> Hmph, aren't you checking what you read a bit too early?  This is a
JCH> valid input:

JCH>         default       
JCH>                 login anonymous
JCH>                 password me@home
JCH>         machine k.org
JCH>                 login me
JCH>                 password mysecret

JCH> but does this loop gives mysecret back to me when asked for
JCH> host=k.org and user=me? 

To be fixed in PATCHv4, which will require the host/machine, use it as
the primary key, and only examine entries with it.

JCH> I would probably structure this part like this: [...]

I will do it like that, thank you for the suggestion.

I'll also add a simple testing Makefile for my own use, and you can
consider adding tests to the general framework later.

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv4] Add contrib/credentials/netrc with GPG support
  2013-02-05 17:01       ` Ted Zlatanov
@ 2013-02-05 18:55         ` Ted Zlatanov
  2013-02-05 19:53           ` Junio C Hamano
  2013-02-05 19:47         ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Junio C Hamano
  1 sibling, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 18:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

Changes since PATCHv3:

- simple tests in Makefile
- support multiple files, code refactored
- documentation and comments updated
- fix IO::File for GPG pipe
- exit peacefully in almost every situation, die on bad invocation or query
- use log_verbose() and -v for logging for the user
- use log_debug() and -d for logging for the developer
- use Net::Netrc parser and `man netrc' to improve parsing
- ignore 'default' and 'macdef' netrc entries
- require 'machine' token in netrc lines
- ignore netrc files with bad permissions or owner (from Net::Netrc)

Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/Makefile             |   10 +
 contrib/credential/netrc/git-credential-netrc |  423 +++++++++++++++++++++++++
 2 files changed, 433 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..ee8c5f0
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,10 @@
+test_netrc:
+	@(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
+	@echo "-> Silent invocation... nothing should show up here with a missing file"
+	@echo "bad data" | ./git-credential-netrc -f A get
+	@echo "-> Back to noisy: -v and -d used below, missing file"
+	echo "bad data" | ./git-credential-netrc -f A -d -v get
+	@echo "-> Look for any entry in the default file set"
+	echo "" | ./git-credential-netrc -d -v get
+	@echo "-> Look for github.com in the default file set"
+	echo "host=google.com" | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..6946217
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,423 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+               help => 0,
+               debug => 0,
+               verbose => 0,
+	       file => [],
+
+               # identical token maps, e.g. host -> host, will be inserted later
+               tmap => {
+                        port => 'protocol',
+                        machine => 'host',
+                        path => 'path',
+                        login => 'username',
+                        user => 'username',
+                        password => 'password',
+                       }
+              );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+	$options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "verbose|v",
+           "file|f=s@",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
+                       will be decrypted by GPG before parsing.  Multiple -f
+                       arguments are OK, and the order is respected.
+
+  -d|--debug         : turn on debugging (developer info)
+
+  -v|--verbose       : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper.  It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+port https login tzz
+
+OR
+
+protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol".
+
+Again, note that the first matching entry from all the AUTHFILEs is used.
+
+Tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files)
+{
+	my @candidates = qw[
+				   ~/.authinfo.gpg
+				   ~/.netrc.gpg
+				   ~/.authinfo
+				   ~/.netrc
+			  ];
+
+	$files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files)
+{
+	unless (-r $file)
+	{
+		log_verbose("Unable to read $file; skipping it");
+		next FILE;
+	}
+
+	# the following check is copied from Net::Netrc
+	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(
+	unless ($^O eq 'os2'
+		|| $^O eq 'MSWin32'
+		|| $^O eq 'MacOS'
+		|| $^O =~ /^cygwin/)
+	{
+		my @stat = stat($file);
+
+		if (@stat) {
+			if ($stat[2] & 077) {
+				log_verbose("Insecure $file (mode=%04o); skipping it",
+					    $stat[2] & 07777);
+				next FILE;
+			}
+			if ($stat[4] != $<) {
+				log_verbose("Not owner of $file; skipping it");
+				next FILE;
+			}
+		}
+	}
+
+	my $mode = (stat($file))[2];
+	if ($mode & 077)
+	{
+		log_verbose("Insecure $file (mode=%04o); skipping it",
+			    $mode & 07777);
+		next FILE;
+	}
+
+	my @entries = load_netrc($file);
+
+	unless (scalar @entries)
+	{
+		if ($!)
+		{
+			log_verbose("Unable to open $file: $!");
+		}
+		else
+		{
+			log_verbose("No netrc entries found in $file");
+		}
+
+		next FILE;
+	}
+
+	my $entry = find_netrc_entry($query, @entries);
+	if ($entry)
+	{
+		print_credential_data($entry, $query);
+		# we're done!
+		last FILE;
+	}
+}
+
+exit 0;
+
+sub load_netrc
+{
+	my $file = shift @_;
+
+	my $io;
+	if ($file =~ m/\.gpg$/) {
+		log_verbose("Using GPG to open $file");
+		# GPG doesn't work well with 2- or 3-argument open
+		$io = new IO::File("gpg --decrypt $file|");
+	}
+	else {
+		log_verbose("Opening $file...");
+		$io = new IO::File($file, '<');
+	}
+
+	# nothing to do if the open failed (we log the error later)
+	return unless $io;
+
+	# Net::Netrc does this, but the functionality is merged with the file
+	# detection logic, so we have to extract just the part we need
+	my @netrc_entries = net_netrc_loader($io);
+
+	# these entries will use the credential helper protocol token names
+	my @entries;
+
+	foreach my $nentry (@netrc_entries) {
+		my %entry;
+		my $num_port;
+
+		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+			$num_port = $nentry->{port};
+			delete $nentry->{port};
+		}
+
+		# create the new entry for the credential helper protocol
+		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+		# for "host X port Y" where Y is an integer (captured by
+		# $num_port above), set the host to "X:Y"
+		if (defined $entry{host} && defined $num_port) {
+			$entry{host} = join(':', $entry{host}, $num_port);
+		}
+
+		push @entries, \%entry;
+	}
+
+	return @entries;
+}
+
+sub net_netrc_loader
+{
+	my $fh = shift @_;
+	my @entries;
+	my ($mach, $macdef, $tok, @tok) = (0, 0);
+
+    LINE:
+	while (<$fh>) {
+		undef $macdef if /\A\n\Z/;
+
+		if ($macdef) {
+			push(@$macdef, $_);
+			next LINE;
+		}
+
+		s/^\s*//;
+		chomp;
+
+		while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+			(my $tok = $+) =~ s/\\(.)/$1/g;
+			push(@tok, $tok);
+		}
+
+	    TOKEN:
+		while (@tok) {
+			$tok = shift(@tok);
+
+			if ($tok eq "machine") {
+				my $host = shift @tok;
+				$mach = { machine => $host };
+				push @entries, $mach;
+			}
+			elsif (exists $options{tmap}->{$tok}) {
+				unless ($mach) {
+					log_debug("Skipping token $tok because no machine was given");
+					next TOKEN;
+				}
+
+				my $value = shift @tok;
+				unless (defined $value) {
+					log_debug("Token $tok had no value, skipping it.");
+					next TOKEN;
+				}
+
+				# Following line added by rmerrell to remove '/' escape char in .netrc
+				$value =~ s/\/\\/\\/g;
+				$mach->{$tok} = $value;
+			}
+			elsif ($tok eq "macdef") { # we ignore macros
+				next TOKEN unless $mach;
+				my $value = shift @tok;
+				$mach->{macdef} = {} unless exists $mach->{macdef};
+				$macdef = $mach->{machdef}{$value} = [];
+			}
+		}
+	}
+
+	return @entries;
+}
+
+sub read_credential_data_from_stdin
+{
+	# the query: start with every token with no value
+	my %q = map { $_ => undef } values(%{$options{tmap}});
+
+	while (<STDIN>) {
+		next unless m/^([^=]+)=(.+)/;
+
+		my ($token, $value) = ($1, $2);
+		die "Unknown search token $token" unless exists $q{$token};
+		$q{$token} = $value;
+		log_debug("We were given search token $token and value $value");
+	}
+
+	foreach (sort keys %q) {
+		log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+	}
+
+	return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry
+{
+	my $query = shift @_;
+
+    ENTRY:
+	foreach my $entry (@_)
+	{
+		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+		foreach my $check (sort keys %$query) {
+			if (defined $query->{$check}) {
+				log_debug("compare %s [%s] to [%s] (entry: %s)",
+					  $check,
+					  $entry->{$check},
+					  $query->{$check},
+					  $entry_text);
+				unless ($query->{$check} eq $entry->{$check}) {
+					next ENTRY;
+				}
+			}
+			else {
+				log_debug("OK: any value satisfies check $check");
+			}
+		}
+
+		return $entry;
+	}
+
+	# nothing was found
+	return;
+}
+
+sub print_credential_data
+{
+	my $entry = shift @_;
+	my $query = shift @_;
+
+	log_debug("entry has passed all the search checks");
+ TOKEN:
+	foreach my $git_token (sort keys %$entry) {
+		log_debug("looking for useful token $git_token");
+		# don't print unknown (to the credential helper protocol) tokens
+		next TOKEN unless exists $query->{$git_token};
+
+		# don't print things asked in the query (the entry matches them)
+		next TOKEN if defined $query->{$git_token};
+
+		log_debug("FOUND: $git_token=$entry->{$git_token}");
+		printf "%s=%s\n", $git_token, $entry->{$git_token};
+	}
+}
+sub log_verbose {
+	return unless $options{verbose};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
+
+sub log_debug {
+	return unless $options{debug};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 17:01       ` Ted Zlatanov
  2013-02-05 18:55         ` [PATCHv4] Add contrib/credentials/netrc with GPG support Ted Zlatanov
@ 2013-02-05 19:47         ` Junio C Hamano
  2013-02-05 20:03           ` Ted Zlatanov
  1 sibling, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 19:47 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> JCH> Oh, another thing. 'default' is like 'machine' followed by any
> JCH> machine name, so the above while loop that reads two tokens
> JCH> pair-wise needs to be aware that 'default' is not followed by a
> JCH> value.  I think the loop will fail to parse this:
>
> JCH>         default       login anonymous    password me@home
> JCH>         machine k.org login me           password mysecret
>
> I'd prefer to ignore "default" because it should not be used for the Git
> credential helpers (its only use case is for anonymous services AFAIK).
> So I'll add a case to ignore it in PATCHv4, if that's OK.

You still need to parse a file that has a "default" entry correctly;
otherwise the users won't be able to share existing .netrc files
with other applications e.g. ftp, which is the whole point of this
series.  Not using values from the "default" entry is probably fine,
though.

Thanks.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
  2013-02-05 18:55         ` [PATCHv4] Add contrib/credentials/netrc with GPG support Ted Zlatanov
@ 2013-02-05 19:53           ` Junio C Hamano
  2013-02-05 20:47             ` Ted Zlatanov
  2013-02-05 20:55             ` [PATCHv5] " Ted Zlatanov
  0 siblings, 2 replies; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 19:53 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> Changes since PATCHv3:
>
> - simple tests in Makefile
> - support multiple files, code refactored
> - documentation and comments updated
> - fix IO::File for GPG pipe
> - exit peacefully in almost every situation, die on bad invocation or query
> - use log_verbose() and -v for logging for the user
> - use log_debug() and -d for logging for the developer
> - use Net::Netrc parser and `man netrc' to improve parsing
> - ignore 'default' and 'macdef' netrc entries
> - require 'machine' token in netrc lines
> - ignore netrc files with bad permissions or owner (from Net::Netrc)

Please place the above _after_ the three-dashes.

The space here, above "---", is to justify why this change is a good
idea to people who see this patch for the first time who never saw
the earlier rounds of this patch, e.g. reading "git log" output 6
months down the road (see Documentation/SubmittingPatches "(2)
Describe your changes well").

>
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
> ---
>  contrib/credential/netrc/Makefile             |   10 +
>  contrib/credential/netrc/git-credential-netrc |  423 +++++++++++++++++++++++++
>  2 files changed, 433 insertions(+), 0 deletions(-)
>  create mode 100644 contrib/credential/netrc/Makefile
>  create mode 100755 contrib/credential/netrc/git-credential-netrc
>
> diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
> new file mode 100644
> index 0000000..ee8c5f0
> --- /dev/null
> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,10 @@
> +test_netrc:
> +	@(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
> +	@echo "-> Silent invocation... nothing should show up here with a missing file"

Avoid starting an argument to "echo" with a dash; some
implementations choke with "unknown option".

> +	@echo "bad data" | ./git-credential-netrc -f A get
> +	@echo "-> Back to noisy: -v and -d used below, missing file"
> +	echo "bad data" | ./git-credential-netrc -f A -d -v get
> +	@echo "-> Look for any entry in the default file set"
> +	echo "" | ./git-credential-netrc -d -v get
> +	@echo "-> Look for github.com in the default file set"
> +	echo "host=google.com" | ./git-credential-netrc -d -v get
> diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
> new file mode 100755
> index 0000000..6946217
> --- /dev/null
> +++ b/contrib/credential/netrc/git-credential-netrc
> @@ -0,0 +1,423 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;
> +
> +use Getopt::Long;
> +use File::Basename;
> +
> +my $VERSION = "0.1";
> +
> +my %options = (
> +               help => 0,
> +               debug => 0,
> +               verbose => 0,
> +	       file => [],

Looks like there is some funny indentation going on here.

> +
> +               # identical token maps, e.g. host -> host, will be inserted later
> +               tmap => {
> +                        port => 'protocol',
> +                        machine => 'host',
> +                        path => 'path',
> +                        login => 'username',
> +                        user => 'username',
> +                        password => 'password',
> +                       }
> +              );
> +
> +# Map each credential protocol token to itself on the netrc side.
> +foreach (values %{$options{tmap}}) {
> +	$options{tmap}->{$_} = $_;
> +}
> +
> +# Now, $options{tmap} has a mapping from the netrc format to the Git credential
> +# helper protocol.
> +
> +# Next, we build the reverse token map.
> +
> +# When $rmap{foo} contains 'bar', that means that what the Git credential helper
> +# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
> +# %rmap are what we expect to read from the netrc/authinfo file.
> +
> +my %rmap;
> +foreach my $k (keys %{$options{tmap}}) {
> +	push @{$rmap{$options{tmap}->{$k}}}, $k;
> +}
> +
> +Getopt::Long::Configure("bundling");
> +
> +# TODO: maybe allow the token map $options{tmap} to be configurable.
> +GetOptions(\%options,
> +           "help|h",
> +           "debug|d",
> +           "verbose|v",
> +           "file|f=s@",
> +          );
> +
> +if ($options{help}) {
> +	my $shortname = basename($0);
> +	$shortname =~ s/git-credential-//;
> +
> +	print <<EOHIPPUS;
> +
> +$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] get
> +
> +Version $VERSION by tzz\@lifelogs.com.  License: BSD.
> +
> +Options:
> +
> +  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
> +                       will be decrypted by GPG before parsing.  Multiple -f
> +                       arguments are OK, and the order is respected.

Saying "order is respected" without mentioning the collision
resolution rules is not helpful to the users when deciding in what
order they should give these files.  First one wins, or last one
wins?  Later you say "looks for the first entry", but it will be
much easier to read the above to mention it here as well.

> +  -d|--debug         : turn on debugging (developer info)
> +
> +  -v|--verbose       : be more verbose (show files and information found)
> +
> +To enable this credential helper:
> +
> +  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
> +
> +(Note that Git will prepend "git-credential-" to the helper name and look for it
> +in the path.)
> +
> +...and if you want lots of debugging info:
> +
> +  git config credential.helper '$shortname -f AUTHFILE -d'
> +
> +...or to see the files opened and data found:
> +
> +  git config credential.helper '$shortname -f AUTHFILE -v'
> +
> +Only "get" mode is supported by this credential helper.  It opens every AUTHFILE
> +and looks for the first entry that matches the requested search criteria:
> +
> + 'port|protocol':
> +   The protocol that will be used (e.g., https). (protocol=X)
> +
> + 'machine|host':
> +   The remote hostname for a network credential. (host=X)
> +
> + 'path':
> +   The path with which the credential will be used. (path=X)
> +
> + 'login|user|username':
> +   The credential’s username, if we already have one. (username=X)
> +
> +Thus, when we get this query on STDIN:
> +
> +protocol=https
> +username=tzz
> +
> +this credential helper will look for the first entry in every AUTHFILE that
> +matches
> +
> +port https login tzz
> +
> +OR
> +
> +protocol https login tzz
> +
> +OR... etc. acceptable tokens as listed above.  Any unknown tokens are
> +simply ignored.
> +
> +Then, the helper will print out whatever tokens it got from the entry, including
> +"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
> +back to "protocol".

Isn't "hostname" typically what users expect to see?  It is somewhat
unnerving to see an example that throws the same password back to
any host you happen to have an accoutn "tzz" on, even though that is
not technically an invalid way to use this helper.

> +Again, note that the first matching entry from all the AUTHFILEs is used.
> +
> +Tokens can be quoted as 'STRING' or "STRING".
> +
> +No caching is performed by this credential helper.
> +
> +EOHIPPUS

Otherwise, nice write-up.

> +my $mode = shift @ARGV;
> +
> +# Credentials must get a parameter, so die if it's missing.
> +die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
> +
> +# Only support 'get' mode; with any other unsupported ones we just exit.
> +exit 0 unless $mode eq 'get';
> +
> +my $files = $options{file};
> +
> +# if no files were given, use a predefined list.
> +# note that .gpg files come first
> +unless (scalar @$files)
> +{
> +	my @candidates = qw[
> +				   ~/.authinfo.gpg
> +				   ~/.netrc.gpg
> +				   ~/.authinfo
> +				   ~/.netrc
> +			  ];
> +
> +	$files = $options{file} = [ map { glob $_ } @candidates ];
> +}
> +
> +my $query = read_credential_data_from_stdin();
> +
> +FILE:
> +foreach my $file (@$files)
> +{
> +	unless (-r $file)
> +	{
> +		log_verbose("Unable to read $file; skipping it");
> +		next FILE;
> +	}
> +
> +	# the following check is copied from Net::Netrc
> +	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(
> +	unless ($^O eq 'os2'
> +		|| $^O eq 'MSWin32'
> +		|| $^O eq 'MacOS'
> +		|| $^O =~ /^cygwin/)
> +	{
> +		my @stat = stat($file);
> +
> +		if (@stat) {
> +			if ($stat[2] & 077) {
> +				log_verbose("Insecure $file (mode=%04o); skipping it",
> +					    $stat[2] & 07777);

Nice touch, although I am not sure rejecting world or group readable
encrypted file is absolutely necessary.

> +				next FILE;
> +			}
> +			if ($stat[4] != $<) {
> +				log_verbose("Not owner of $file; skipping it");
> +				next FILE;

OK.  A group of local users may share the same account at the
remote, but that would be unusual.

> +			}
> +		}
> +	}
> +
> +	my $mode = (stat($file))[2];
> +	if ($mode & 077)
> +	{
> +		log_verbose("Insecure $file (mode=%04o); skipping it",
> +			    $mode & 07777);

Again?  Didn't you just do this?

> +		next FILE;
> +	}
> +
> +	my @entries = load_netrc($file);
> +
> +	unless (scalar @entries)
> +	{
> +		if ($!)
> +		{
> +			log_verbose("Unable to open $file: $!");
> +		}
> +		else
> +		{
> +			log_verbose("No netrc entries found in $file");
> +		}

I think the prevalent style is to

	if (condition) {
        	do this;
	} elsif (another condition) {
		do that
	} else {
		do that other thing;
	}

(this comment applies to all if/elsif/else cascades in this patch).

> +
> +		next FILE;

Isn't this outermost loop, by the way?  What the motivation to have
an explicit label everywhere (not complaining---it could be your own
discipline thing---just wondering).

> +	}
> +
> +	my $entry = find_netrc_entry($query, @entries);
> +	if ($entry)
> +	{
> +		print_credential_data($entry, $query);
> +		# we're done!
> +		last FILE;
> +	}
> +}
> +
> +exit 0;
> +
> +sub load_netrc
> +{
> +	my $file = shift @_;
> +
> +	my $io;
> +	if ($file =~ m/\.gpg$/) {
> +		log_verbose("Using GPG to open $file");
> +		# GPG doesn't work well with 2- or 3-argument open

If that is the case, please quote $file properly against shell
munging it.

The only thing you do on $io is to read from it via "while (<$io>)",
so I would personally have written this part like this without
having to use IO::File(), though:

	$io = open("-|", qw(gpg --decrypt), $file);

Similarly for the plain file:

	$io = open("<", $file);

> +		$io = new IO::File("gpg --decrypt $file|");
> +	}
> +	else {
> +		log_verbose("Opening $file...");
> +		$io = new IO::File($file, '<');
> +	}
> +
> +	# nothing to do if the open failed (we log the error later)
> +	return unless $io;
> +
> +	# Net::Netrc does this, but the functionality is merged with the file
> +	# detection logic, so we have to extract just the part we need
> +	my @netrc_entries = net_netrc_loader($io);
> +
> +	# these entries will use the credential helper protocol token names
> +	my @entries;
> +
> +	foreach my $nentry (@netrc_entries) {
> +		my %entry;
> +		my $num_port;
> +
> +		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
> +			$num_port = $nentry->{port};
> +			delete $nentry->{port};
> +		}
> +
> +		# create the new entry for the credential helper protocol
> +		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
> +
> +		# for "host X port Y" where Y is an integer (captured by
> +		# $num_port above), set the host to "X:Y"
> +		if (defined $entry{host} && defined $num_port) {
> +			$entry{host} = join(':', $entry{host}, $num_port);
> +		}
> +
> +		push @entries, \%entry;
> +	}
> +
> +	return @entries;
> +}
> +
> +sub net_netrc_loader
> +{
> +	my $fh = shift @_;
> +	my @entries;
> +	my ($mach, $macdef, $tok, @tok) = (0, 0);

I think you meant to use $mach as a reference to a hash and $macdef
as a reference to an array; do you want to initialize them to
numeric zeros?

(The remainder of the patch unsnipped for others' reference).

Thanks.

> +    LINE:
> +	while (<$fh>) {
> +		undef $macdef if /\A\n\Z/;
> +
> +		if ($macdef) {
> +			push(@$macdef, $_);
> +			next LINE;
> +		}
> +
> +		s/^\s*//;
> +		chomp;
> +
> +		while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
> +			(my $tok = $+) =~ s/\\(.)/$1/g;
> +			push(@tok, $tok);
> +		}
> +
> +	    TOKEN:
> +		while (@tok) {
> +			$tok = shift(@tok);
> +
> +			if ($tok eq "machine") {
> +				my $host = shift @tok;
> +				$mach = { machine => $host };
> +				push @entries, $mach;
> +			}
> +			elsif (exists $options{tmap}->{$tok}) {
> +				unless ($mach) {
> +					log_debug("Skipping token $tok because no machine was given");
> +					next TOKEN;
> +				}
> +
> +				my $value = shift @tok;
> +				unless (defined $value) {
> +					log_debug("Token $tok had no value, skipping it.");
> +					next TOKEN;
> +				}
> +
> +				# Following line added by rmerrell to remove '/' escape char in .netrc
> +				$value =~ s/\/\\/\\/g;
> +				$mach->{$tok} = $value;
> +			}
> +			elsif ($tok eq "macdef") { # we ignore macros
> +				next TOKEN unless $mach;
> +				my $value = shift @tok;
> +				$mach->{macdef} = {} unless exists $mach->{macdef};
> +				$macdef = $mach->{machdef}{$value} = [];
> +			}
> +		}
> +	}
> +
> +	return @entries;
> +}
> +
> +sub read_credential_data_from_stdin
> +{
> +	# the query: start with every token with no value
> +	my %q = map { $_ => undef } values(%{$options{tmap}});
> +
> +	while (<STDIN>) {
> +		next unless m/^([^=]+)=(.+)/;
> +
> +		my ($token, $value) = ($1, $2);
> +		die "Unknown search token $token" unless exists $q{$token};
> +		$q{$token} = $value;
> +		log_debug("We were given search token $token and value $value");
> +	}
> +
> +	foreach (sort keys %q) {
> +		log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
> +	}
> +
> +	return \%q;
> +}
> +
> +# takes the search tokens and then a list of entries
> +# each entry is a hash reference
> +sub find_netrc_entry
> +{
> +	my $query = shift @_;
> +
> +    ENTRY:
> +	foreach my $entry (@_)
> +	{
> +		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
> +		foreach my $check (sort keys %$query) {
> +			if (defined $query->{$check}) {
> +				log_debug("compare %s [%s] to [%s] (entry: %s)",
> +					  $check,
> +					  $entry->{$check},
> +					  $query->{$check},
> +					  $entry_text);
> +				unless ($query->{$check} eq $entry->{$check}) {
> +					next ENTRY;
> +				}
> +			}
> +			else {
> +				log_debug("OK: any value satisfies check $check");
> +			}
> +		}
> +
> +		return $entry;
> +	}
> +
> +	# nothing was found
> +	return;
> +}
> +
> +sub print_credential_data
> +{
> +	my $entry = shift @_;
> +	my $query = shift @_;
> +
> +	log_debug("entry has passed all the search checks");
> + TOKEN:
> +	foreach my $git_token (sort keys %$entry) {
> +		log_debug("looking for useful token $git_token");
> +		# don't print unknown (to the credential helper protocol) tokens
> +		next TOKEN unless exists $query->{$git_token};
> +
> +		# don't print things asked in the query (the entry matches them)
> +		next TOKEN if defined $query->{$git_token};
> +
> +		log_debug("FOUND: $git_token=$entry->{$git_token}");
> +		printf "%s=%s\n", $git_token, $entry->{$git_token};
> +	}
> +}
> +sub log_verbose {
> +	return unless $options{verbose};
> +	printf STDERR @_;
> +	printf STDERR "\n";
> +}
> +
> +sub log_debug {
> +	return unless $options{debug};
> +	printf STDERR @_;
> +	printf STDERR "\n";
> +}

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 19:47         ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Junio C Hamano
@ 2013-02-05 20:03           ` Ted Zlatanov
  2013-02-05 20:23             ` Junio C Hamano
  0 siblings, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 20:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Tue, 05 Feb 2013 11:47:56 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
JCH> Oh, another thing. 'default' is like 'machine' followed by any
JCH> machine name, so the above while loop that reads two tokens
JCH> pair-wise needs to be aware that 'default' is not followed by a
JCH> value.  I think the loop will fail to parse this:
>> 
JCH> default       login anonymous    password me@home
JCH> machine k.org login me           password mysecret
>> 
>> I'd prefer to ignore "default" because it should not be used for the Git
>> credential helpers (its only use case is for anonymous services AFAIK).
>> So I'll add a case to ignore it in PATCHv4, if that's OK.

JCH> You still need to parse a file that has a "default" entry correctly;
JCH> otherwise the users won't be able to share existing .netrc files
JCH> with other applications e.g. ftp, which is the whole point of this
JCH> series.  Not using values from the "default" entry is probably fine,
JCH> though.

OK; done in PATCHv4.

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 20:03           ` Ted Zlatanov
@ 2013-02-05 20:23             ` Junio C Hamano
  2013-02-05 21:00               ` Ted Zlatanov
  0 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 20:23 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> JCH> You still need to parse a file that has a "default" entry correctly;
> JCH> otherwise the users won't be able to share existing .netrc files
> JCH> with other applications e.g. ftp, which is the whole point of this
> JCH> series.  Not using values from the "default" entry is probably fine,
> JCH> though.
>
> OK; done in PATCHv4.

Hmph.

Didn't you remove that from your version of net_netrc_loader when
you borrowed the bulk of the code from Net::Netrc::_readrc?  I see
"default" token handled at the beginning of "TOKEN: while (@tok)"
loop in the original but not in your version I see in v4.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
  2013-02-05 19:53           ` Junio C Hamano
@ 2013-02-05 20:47             ` Ted Zlatanov
  2013-02-05 22:09               ` Junio C Hamano
  2013-02-05 20:55             ` [PATCHv5] " Ted Zlatanov
  1 sibling, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 20:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Tue, 05 Feb 2013 11:53:20 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> Changes since PATCHv3:
>> 
>> - simple tests in Makefile
>> - support multiple files, code refactored
>> - documentation and comments updated
>> - fix IO::File for GPG pipe
>> - exit peacefully in almost every situation, die on bad invocation or query
>> - use log_verbose() and -v for logging for the user
>> - use log_debug() and -d for logging for the developer
>> - use Net::Netrc parser and `man netrc' to improve parsing
>> - ignore 'default' and 'macdef' netrc entries
>> - require 'machine' token in netrc lines
>> - ignore netrc files with bad permissions or owner (from Net::Netrc)

JCH> Please place the above _after_ the three-dashes.

JCH> The space here, above "---", is to justify why this change is a good
JCH> idea to people who see this patch for the first time who never saw
JCH> the earlier rounds of this patch, e.g. reading "git log" output 6
JCH> months down the road (see Documentation/SubmittingPatches "(2)
JCH> Describe your changes well").

Will do in PATCHv5.

JCH> Avoid starting an argument to "echo" with a dash; some
JCH> implementations choke with "unknown option".

Nice, thanks.  It's purely decorative so I use '=>' now.

>> +my %options = (
>> +               help => 0,
>> +               debug => 0,
>> +               verbose => 0,
>> +	       file => [],

JCH> Looks like there is some funny indentation going on here.

Fixed.

>> +  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
>> +                       will be decrypted by GPG before parsing.  Multiple -f
>> +                       arguments are OK, and the order is respected.

JCH> Saying "order is respected" without mentioning the collision
JCH> resolution rules is not helpful to the users when deciding in what
JCH> order they should give these files.  First one wins, or last one
JCH> wins?  Later you say "looks for the first entry", but it will be
JCH> much easier to read the above to mention it here as well.

Right.  Reworded.

>> +Thus, when we get this query on STDIN:
>> +
>> +protocol=https
>> +username=tzz
>> +
>> +this credential helper will look for the first entry in every AUTHFILE that
>> +matches
>> +
>> +port https login tzz
>> +
>> +OR
>> +
>> +protocol https login tzz
>> +
>> +OR... etc. acceptable tokens as listed above.  Any unknown tokens are
>> +simply ignored.
>> +
>> +Then, the helper will print out whatever tokens it got from the entry, including
>> +"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
>> +back to "protocol".

JCH> Isn't "hostname" typically what users expect to see?  It is somewhat
JCH> unnerving to see an example that throws the same password back to
JCH> any host you happen to have an accoutn "tzz" on, even though that is
JCH> not technically an invalid way to use this helper.

Yeah, I changed it to show "machine" in the query (which would be more typical).

>> +			if ($stat[2] & 077) {
>> +				log_verbose("Insecure $file (mode=%04o); skipping it",
>> +					    $stat[2] & 07777);

JCH> Nice touch, although I am not sure rejecting world or group readable
JCH> encrypted file is absolutely necessary.

Right.  Fixed.

>> +			if ($stat[4] != $<) {
>> +				log_verbose("Not owner of $file; skipping it");
>> +				next FILE;

JCH> OK.  A group of local users may share the same account at the
JCH> remote, but that would be unusual.

I added --insecure/-k to override this check.

>> +	if ($mode & 077)

JCH> Again?  Didn't you just do this?

Damn, sorry.

JCH> I think the prevalent style is to

JCH> 	if (condition) {
JCH>         	do this;
JCH> 	} elsif (another condition) {
JCH> 		do that
JCH> 	} else {
JCH> 		do that other thing;
JCH> 	}

JCH> (this comment applies to all if/elsif/else cascades in this patch).

Yup.  I was working with Net::Netrc code and forgot to reformat it.  Sorry.

>> +
>> +		next FILE;

JCH> Isn't this outermost loop, by the way?  What the motivation to have
JCH> an explicit label everywhere (not complaining---it could be your own
JCH> discipline thing---just wondering).

I think it's more readable with large loops, and it actually makes sense
when you read the code.  Not a big deal to me either, I just felt for
this particular script it was OK.

>> +	if ($file =~ m/\.gpg$/) {
>> +		log_verbose("Using GPG to open $file");
>> +		# GPG doesn't work well with 2- or 3-argument open

JCH> If that is the case, please quote $file properly against shell
JCH> munging it.

Ahhh that gets ugly.  OK, quoted.

JCH> The only thing you do on $io is to read from it via "while (<$io>)",
JCH> so I would personally have written this part like this without
JCH> having to use IO::File(), though:

JCH> 	$io = open("-|", qw(gpg --decrypt), $file);

That doesn't work for me, unfortunately.  I'm trying to avoid the IPC::*
modules and such.  Please test it yourself with GPG.  I'm on Perl
5.14.2.

I think it's OK with the quoting, as you'll see in PATCHv5.

JCH> Similarly for the plain file:

JCH> 	$io = open("<", $file);

You mean "open $io, '<', $file".  open() returns nonzero on success and
undef on failure.  OK, I'll use open() instead of IO::File.

>> +	my ($mach, $macdef, $tok, @tok) = (0, 0);

JCH> I think you meant to use $mach as a reference to a hash and $macdef
JCH> as a reference to an array; do you want to initialize them to
JCH> numeric zeros?

That actually came from Net::Netrc.  The $mach default is OK either way;
I left it undefined so it's clearer. I think the $macdef initial value
is a bug (which, I guess, shows macros are very rare); I just made it a
boolean for our purposes.

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv5] Add contrib/credentials/netrc with GPG support
  2013-02-05 19:53           ` Junio C Hamano
  2013-02-05 20:47             ` Ted Zlatanov
@ 2013-02-05 20:55             ` Ted Zlatanov
  2013-02-05 22:24               ` Junio C Hamano
  1 sibling, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 20:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

Add Git credential helper that can parse netrc/authinfo files.

This credential helper support multiple files, returning the first one
that matches.  It checks file permissions and owner.  For *.gpg files,
it will run GPG to decrypt the file.

Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Changes since PATCHv4:

- indentation and brace fixes
- test makefile uses "=>" as the decorative prefix
- documentation fixes about order and show query with "hostname"
- add --insecure to ignore owner and permission checks
- check permissions just once and only for unencrypted files
- change IO::File to simple open() and quote $file
- fixed macdef buglet from Net::Netrc
- ignore 'default' entries

 contrib/credential/netrc/Makefile             |   12 +
 contrib/credential/netrc/git-credential-netrc |  424 +++++++++++++++++++++++++
 2 files changed, 436 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..18a924f
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+	@(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
+	@echo "=> Silent invocation... nothing should show up here with a missing file"
+	@echo "bad data" | ./git-credential-netrc -f A get
+	@echo "=> Back to noisy: -v and -d used below, missing file"
+	echo "bad data" | ./git-credential-netrc -f A -d -v get
+	@echo "=> Look for any entry in the default file set"
+	echo "" | ./git-credential-netrc -d -v get
+	@echo "=> Look for github.com in the default file set"
+	echo "host=google.com" | ./git-credential-netrc -d -v get
+	@echo "=> Look for a nonexistent machine in the default file set"
+	echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..8298564
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,424 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+	       help => 0,
+	       debug => 0,
+	       verbose => 0,
+	       insecure => 0,
+	       file => [],
+
+	       # identical token maps, e.g. host -> host, will be inserted later
+	       tmap => {
+			port => 'protocol',
+			machine => 'host',
+			path => 'path',
+			login => 'username',
+			user => 'username',
+			password => 'password',
+		       }
+	      );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+	$options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "insecure|k",
+           "verbose|v",
+           "file|f=s@",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
+                       will be decrypted by GPG before parsing.  Multiple -f
+                       arguments are OK.  They are processed in order, and the
+                       first matching entry found is returned via the credential
+                       helper protocol (see below).
+
+  -k|--insecure      : ignore bad file ownership or permissions
+
+  -d|--debug         : turn on debugging (developer info)
+
+  -v|--verbose       : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper.  It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+host=github.com
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+machine github.com port https login tzz
+
+OR
+
+machine github.com protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol".  Any redundant entry tokens (part of the original query) are
+skipped.
+
+Again, note that only the first matching entry from all the AUTHFILEs, processed
+in the sequence given on the command line, is used.
+
+Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files) {
+	my @candidates = qw[
+				   ~/.authinfo.gpg
+				   ~/.netrc.gpg
+				   ~/.authinfo
+				   ~/.netrc
+			  ];
+
+	$files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files) {
+	my $gpgmode = $file =~ m/\.gpg$/;
+	unless (-r $file) {
+		log_verbose("Unable to read $file; skipping it");
+		next FILE;
+	}
+
+	# the following check is copied from Net::Netrc, for non-GPG files
+	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(
+	unless ($gpgmode || $options{insecure} ||
+		$^O eq 'os2'
+		|| $^O eq 'MSWin32'
+		|| $^O eq 'MacOS'
+		|| $^O =~ /^cygwin/) {
+		my @stat = stat($file);
+
+		if (@stat) {
+			if ($stat[2] & 077) {
+				log_verbose("Insecure $file (mode=%04o); skipping it",
+					    $stat[2] & 07777);
+				next FILE;
+			}
+
+			if ($stat[4] != $<) {
+				log_verbose("Not owner of $file; skipping it");
+				next FILE;
+			}
+		}
+	}
+
+	my @entries = load_netrc($file, $gpgmode);
+
+	unless (scalar @entries) {
+		if ($!) {
+			log_verbose("Unable to open $file: $!");
+		}
+		else {
+			log_verbose("No netrc entries found in $file");
+		}
+
+		next FILE;
+	}
+
+	my $entry = find_netrc_entry($query, @entries);
+	if ($entry) {
+		print_credential_data($entry, $query);
+		# we're done!
+		last FILE;
+	}
+}
+
+exit 0;
+
+sub load_netrc {
+	my $file = shift @_;
+	my $gpgmode = shift @_;
+
+	my $io;
+	if ($gpgmode) {
+		# typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
+		my $f = $file;
+		$f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
+		# GPG doesn't work well with 2- or 3-argument open
+		my $cmd = "gpg --decrypt $f";
+		log_verbose("Using GPG to open $file: [$cmd]");
+		open $io, "$cmd|";
+	}
+	else {
+		log_verbose("Opening $file...");
+		open $io, '<', $file;
+	}
+
+	# nothing to do if the open failed (we log the error later)
+	return unless $io;
+
+	# Net::Netrc does this, but the functionality is merged with the file
+	# detection logic, so we have to extract just the part we need
+	my @netrc_entries = net_netrc_loader($io);
+
+	# these entries will use the credential helper protocol token names
+	my @entries;
+
+	foreach my $nentry (@netrc_entries) {
+		my %entry;
+		my $num_port;
+
+		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+			$num_port = $nentry->{port};
+			delete $nentry->{port};
+		}
+
+		# create the new entry for the credential helper protocol
+		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+		# for "host X port Y" where Y is an integer (captured by
+		# $num_port above), set the host to "X:Y"
+		if (defined $entry{host} && defined $num_port) {
+			$entry{host} = join(':', $entry{host}, $num_port);
+		}
+
+		push @entries, \%entry;
+	}
+
+	return @entries;
+}
+
+sub net_netrc_loader {
+	my $fh = shift @_;
+	my @entries;
+	my ($mach, $macdef, $tok, @tok);
+
+    LINE:
+	while (<$fh>) {
+		undef $macdef if /\A\n\Z/;
+
+		if ($macdef) {
+			next LINE;
+		}
+
+		s/^\s*//;
+		chomp;
+
+		while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+			(my $tok = $+) =~ s/\\(.)/$1/g;
+			push(@tok, $tok);
+		}
+
+	    TOKEN:
+		while (@tok) {
+			if ($tok[0] eq "default") {
+				shift(@tok);
+				undef $mach; # ignore 'default' lines
+
+				next TOKEN;
+			}
+
+			$tok = shift(@tok);
+
+			if ($tok eq "machine") {
+				my $host = shift @tok;
+				$mach = { machine => $host };
+				push @entries, $mach;
+			}
+			elsif (exists $options{tmap}->{$tok}) {
+				unless ($mach) {
+					log_debug("Skipping token $tok because no machine was given");
+					next TOKEN;
+				}
+
+				my $value = shift @tok;
+				unless (defined $value) {
+					log_debug("Token $tok had no value, skipping it.");
+					next TOKEN;
+				}
+
+				# Following line added by rmerrell to remove '/' escape char in .netrc
+				$value =~ s/\/\\/\\/g;
+				$mach->{$tok} = $value;
+			}
+			elsif ($tok eq "macdef") { # we ignore macros
+				next TOKEN unless $mach;
+				my $value = shift @tok;
+				$macdef = 1;
+			}
+		}
+	}
+
+	return @entries;
+}
+
+sub read_credential_data_from_stdin {
+	# the query: start with every token with no value
+	my %q = map { $_ => undef } values(%{$options{tmap}});
+
+	while (<STDIN>) {
+		next unless m/^([^=]+)=(.+)/;
+
+		my ($token, $value) = ($1, $2);
+		die "Unknown search token $token" unless exists $q{$token};
+		$q{$token} = $value;
+		log_debug("We were given search token $token and value $value");
+	}
+
+	foreach (sort keys %q) {
+		log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+	}
+
+	return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry {
+	my $query = shift @_;
+
+    ENTRY:
+	foreach my $entry (@_)
+	{
+		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+		foreach my $check (sort keys %$query) {
+			if (defined $query->{$check}) {
+				log_debug("compare %s [%s] to [%s] (entry: %s)",
+					  $check,
+					  $entry->{$check},
+					  $query->{$check},
+					  $entry_text);
+				unless ($query->{$check} eq $entry->{$check}) {
+					next ENTRY;
+				}
+			}
+			else {
+				log_debug("OK: any value satisfies check $check");
+			}
+		}
+
+		return $entry;
+	}
+
+	# nothing was found
+	return;
+}
+
+sub print_credential_data {
+	my $entry = shift @_;
+	my $query = shift @_;
+
+	log_debug("entry has passed all the search checks");
+ TOKEN:
+	foreach my $git_token (sort keys %$entry) {
+		log_debug("looking for useful token $git_token");
+		# don't print unknown (to the credential helper protocol) tokens
+		next TOKEN unless exists $query->{$git_token};
+
+		# don't print things asked in the query (the entry matches them)
+		next TOKEN if defined $query->{$git_token};
+
+		log_debug("FOUND: $git_token=$entry->{$git_token}");
+		printf "%s=%s\n", $git_token, $entry->{$git_token};
+	}
+}
+sub log_verbose {
+	return unless $options{verbose};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
+
+sub log_debug {
+	return unless $options{debug};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
  2013-02-05 20:23             ` Junio C Hamano
@ 2013-02-05 21:00               ` Ted Zlatanov
  0 siblings, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 21:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Tue, 05 Feb 2013 12:23:00 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
JCH> You still need to parse a file that has a "default" entry correctly;
JCH> otherwise the users won't be able to share existing .netrc files
JCH> with other applications e.g. ftp, which is the whole point of this
JCH> series.  Not using values from the "default" entry is probably fine,
JCH> though.
>> 
>> OK; done in PATCHv4.

JCH> Hmph.

JCH> Didn't you remove that from your version of net_netrc_loader when
JCH> you borrowed the bulk of the code from Net::Netrc::_readrc?  I see
JCH> "default" token handled at the beginning of "TOKEN: while (@tok)"
JCH> loop in the original but not in your version I see in v4.

Damn, I accidentally moved that check to /dev/null.  OK, I added it in
PATCHv5.  Thanks for catching that.

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
  2013-02-05 20:47             ` Ted Zlatanov
@ 2013-02-05 22:09               ` Junio C Hamano
  2013-02-05 22:30                 ` Ted Zlatanov
  0 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 22:09 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Tue, 05 Feb 2013 11:53:20 -0800 Junio C Hamano <gitster@pobox.com> wrote: 
>
> I think it's more readable with large loops, and it actually makes sense
> when you read the code.  Not a big deal to me either, I just felt for
> this particular script it was OK.
>
>>> +	if ($file =~ m/\.gpg$/) {
>>> +		log_verbose("Using GPG to open $file");
>>> +		# GPG doesn't work well with 2- or 3-argument open
>
> JCH> If that is the case, please quote $file properly against shell
> JCH> munging it.
>
> Ahhh that gets ugly.  OK, quoted.
>
> JCH> The only thing you do on $io is to read from it via "while (<$io>)",
> JCH> so I would personally have written this part like this without
> JCH> having to use IO::File(), though:
>
> JCH> 	$io = open("-|", qw(gpg --decrypt), $file);
>
> That doesn't work for me, unfortunately.  I'm trying to avoid the IPC::*
> modules and such.  Please test it yourself with GPG.  I'm on Perl
> 5.14.2.

This works for me as expected (sorry for that open $io syntax
gotcha).

-- cut here -- >8 -- cut here --

#!/usr/bin/perl
my $io;
open $io, "-|", qw(gpg --decrypt), $ARGV[0]
        or die "$!: gpg open";
while (<$io>) {
        print;
}
close $io
        or die "$!: gpg close";

-- cut here -- 8< -- cut here --

$ perl --version
This is perl, v5.10.1 (*) built for x86_64-linux-gnu-thread-multi

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv5] Add contrib/credentials/netrc with GPG support
  2013-02-05 20:55             ` [PATCHv5] " Ted Zlatanov
@ 2013-02-05 22:24               ` Junio C Hamano
  2013-02-05 23:58                 ` Junio C Hamano
  2013-02-06  0:34                 ` [PATCHv5] " Ted Zlatanov
  0 siblings, 2 replies; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 22:24 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> +	unless (scalar @entries) {
> +		if ($!) {
> +			log_verbose("Unable to open $file: $!");
> +		}
> +		else {

	} else {

> +			log_verbose("No netrc entries found in $file");
> +		}
> +
> +	if ($gpgmode) {
> +		# typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
> +		my $f = $file;
> +		$f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;

Yuck.  If you really have to quote, it is often far simpler to take
advantage of the fact that quoting rule for shell is much simpler
inside '', i.e.

	sub sq {
		my ($string) = @_;
		$string =~ s|'|'\\''|g;
		return "'$string'";
	}

The only thing you have to worry about is a single quote, which
would close the single quote you want to be in, and you express it
by first closing the single quote you opened, put the single quote
by emitting a backslash and a single quote, and then immediately 
open another single quote.

> +		# GPG doesn't work well with 2- or 3-argument open

I think I commented on this in a separate message.

> +	# Net::Netrc does this, but the functionality is merged with the file
> +	# detection logic, so we have to extract just the part we need
> +	my @netrc_entries = net_netrc_loader($io);
> +
> +	# these entries will use the credential helper protocol token names
> +	my @entries;
> +
> +	foreach my $nentry (@netrc_entries) {
> +		my %entry;
> +		my $num_port;
> +
> +		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
> +			$num_port = $nentry->{port};
> +			delete $nentry->{port};
> +		}
> +
> +		# create the new entry for the credential helper protocol
> +		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
> +
> +		# for "host X port Y" where Y is an integer (captured by
> +		# $num_port above), set the host to "X:Y"
> +		if (defined $entry{host} && defined $num_port) {
> +			$entry{host} = join(':', $entry{host}, $num_port);
> +		}
> +
> +		push @entries, \%entry;
> +	}
> +
> +	return @entries;
> +}

I'll leave this part to Peff to comment on.

> +sub net_netrc_loader {
> +	my $fh = shift @_;
> +	my @entries;
> +	my ($mach, $macdef, $tok, @tok);
> +
> +    LINE:
> +	while (<$fh>) {
> +		undef $macdef if /\A\n\Z/;
> +
> +		if ($macdef) {
> +			next LINE;
> +		}
> +
> +		s/^\s*//;
> +		chomp;
> +
> +		while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
> +			(my $tok = $+) =~ s/\\(.)/$1/g;
> +			push(@tok, $tok);
> +		}
> +
> +	    TOKEN:
> +		while (@tok) {
> +			if ($tok[0] eq "default") {
> +				shift(@tok);
> +				undef $mach; # ignore 'default' lines

I think it is saner to do something like this instead here:

				$mach = { machine => undef }

Otherwise your log_debug() will be filled by the tokens used for the
default entry, and also this "undef $mach" here will break your
macdef skipping logic if the default entry has a macdef, I think.

You can ignore an entry with undefined "machine" in the loop at the
end of load_netrc.

> +				next TOKEN;
> +			}
> +
> +			$tok = shift(@tok);
> +
> +			if ($tok eq "machine") {
> +				my $host = shift @tok;
> +				$mach = { machine => $host };
> +				push @entries, $mach;
> +			}
> +			elsif (exists $options{tmap}->{$tok}) {
> +				unless ($mach) {
> +					log_debug("Skipping token $tok because no machine was given");
> +					next TOKEN;
> +				}
> +
> +				my $value = shift @tok;
> +				unless (defined $value) {
> +					log_debug("Token $tok had no value, skipping it.");
> +					next TOKEN;
> +				}
> +
> +				# Following line added by rmerrell to remove '/' escape char in .netrc
> +				$value =~ s/\/\\/\\/g;
> +				$mach->{$tok} = $value;
> +			}
> +			elsif ($tok eq "macdef") { # we ignore macros
> +				next TOKEN unless $mach;
> +				my $value = shift @tok;
> +				$macdef = 1;
> +			}
> +		}
> +	}
> +
> +	return @entries;
> +}
> +
> +sub read_credential_data_from_stdin {
> +	# the query: start with every token with no value
> +	my %q = map { $_ => undef } values(%{$options{tmap}});
> +
> +	while (<STDIN>) {
> +		next unless m/^([^=]+)=(.+)/;
> +
> +		my ($token, $value) = ($1, $2);
> +		die "Unknown search token $token" unless exists $q{$token};
> +		$q{$token} = $value;
> +		log_debug("We were given search token $token and value $value");
> +	}
> +
> +	foreach (sort keys %q) {
> +		log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
> +	}
> +
> +	return \%q;
> +}
> +
> +# takes the search tokens and then a list of entries
> +# each entry is a hash reference
> +sub find_netrc_entry {
> +	my $query = shift @_;
> +
> +    ENTRY:
> +	foreach my $entry (@_)
> +	{
> +		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
> +		foreach my $check (sort keys %$query) {
> +			if (defined $query->{$check}) {
> +				log_debug("compare %s [%s] to [%s] (entry: %s)",
> +					  $check,
> +					  $entry->{$check},
> +					  $query->{$check},
> +					  $entry_text);
> +				unless ($query->{$check} eq $entry->{$check}) {
> +					next ENTRY;
> +				}
> +			}
> +			else {
> +				log_debug("OK: any value satisfies check $check");
> +			}
> +		}
> +
> +		return $entry;
> +	}
> +
> +	# nothing was found
> +	return;
> +}

I'll leave this part to Peff to comment on.

> +
> +sub print_credential_data {
> +	my $entry = shift @_;
> +	my $query = shift @_;
> +
> +	log_debug("entry has passed all the search checks");
> + TOKEN:
> +	foreach my $git_token (sort keys %$entry) {
> +		log_debug("looking for useful token $git_token");
> +		# don't print unknown (to the credential helper protocol) tokens
> +		next TOKEN unless exists $query->{$git_token};
> +
> +		# don't print things asked in the query (the entry matches them)
> +		next TOKEN if defined $query->{$git_token};
> +
> +		log_debug("FOUND: $git_token=$entry->{$git_token}");
> +		printf "%s=%s\n", $git_token, $entry->{$git_token};
> +	}
> +}
> +sub log_verbose {
> +	return unless $options{verbose};
> +	printf STDERR @_;
> +	printf STDERR "\n";
> +}
> +
> +sub log_debug {
> +	return unless $options{debug};
> +	printf STDERR @_;
> +	printf STDERR "\n";
> +}

Otherwise, looks almost ready to me.

Thanks.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv4] Add contrib/credentials/netrc with GPG support
  2013-02-05 22:09               ` Junio C Hamano
@ 2013-02-05 22:30                 ` Ted Zlatanov
  0 siblings, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-05 22:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Tue, 05 Feb 2013 14:09:58 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> open $io, "-|", qw(gpg --decrypt), $ARGV[0]

OK, the below will be in PATCHv6 (I'll wait on mailing it until after
you've reviewed the rest of PATCHv5).  Thanks for checking... I must
have had a typo or a missing comma or something, I could swear I tried
exactly what you show.

Ted


diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
index 8298564..5f91630 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -230,13 +230,9 @@ sub load_netrc {
 
 	my $io;
 	if ($gpgmode) {
-		# typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
-		my $f = $file;
-		$f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
-		# GPG doesn't work well with 2- or 3-argument open
-		my $cmd = "gpg --decrypt $f";
-		log_verbose("Using GPG to open $file: [$cmd]");
-		open $io, "$cmd|";
+		my @cmd = (qw(gpg --decrypt), $file);
+		log_verbose("Using GPG to open $file: [@cmd]");
+		open $io, "-|", @cmd;
 	}
 	else {
 		log_verbose("Opening $file...");

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCHv5] Add contrib/credentials/netrc with GPG support
  2013-02-05 22:24               ` Junio C Hamano
@ 2013-02-05 23:58                 ` Junio C Hamano
  2013-02-06  0:38                   ` [PATCHv6] " Ted Zlatanov
  2013-02-06  0:34                 ` [PATCHv5] " Ted Zlatanov
  1 sibling, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-05 23:58 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Junio C Hamano <gitster@pobox.com> writes:

> Otherwise, looks almost ready to me.

For now, I've queued this as a minimum fix-up on top of your patch
and pushed the result out.  It is an equivalent of the previous
review comments in a patch form.  Please review and incorporate in
your reroll as appropriate.

I haven't looked at the part that interacts with the credential
subsystem itself, though.

 contrib/credential/netrc/git-credential-netrc | 35 ++++++++++++---------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
index 8298564..30e05fb 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -74,6 +74,10 @@ Options:
                        first matching entry found is returned via the credential
                        helper protocol (see below).
 
+                       When no -f option is given, .authinfo.gpg, .netrc.gpg,
+		       .authinfo, and .netrc files in your home directory are used
+		       in this order.
+
   -k|--insecure      : ignore bad file ownership or permissions
 
   -d|--debug         : turn on debugging (developer info)
@@ -206,8 +210,7 @@ foreach my $file (@$files) {
 	unless (scalar @entries) {
 		if ($!) {
 			log_verbose("Unable to open $file: $!");
-		}
-		else {
+		} else {
 			log_verbose("No netrc entries found in $file");
 		}
 
@@ -230,15 +233,10 @@ sub load_netrc {
 
 	my $io;
 	if ($gpgmode) {
-		# typical shell character escapes from http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
-		my $f = $file;
-		$f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;
-		# GPG doesn't work well with 2- or 3-argument open
-		my $cmd = "gpg --decrypt $f";
-		log_verbose("Using GPG to open $file: [$cmd]");
-		open $io, "$cmd|";
-	}
-	else {
+		my @cmd = (qw(gpg --decrypt), $file)
+		log_verbose("Using GPG to open $file: [@cmd]");
+		open $io, "-|", @cmd;
+	} else {
 		log_verbose("Opening $file...");
 		open $io, '<', $file;
 	}
@@ -257,6 +255,9 @@ sub load_netrc {
 		my %entry;
 		my $num_port;
 
+		if (!defined $nentry->{machine}) {
+			next;
+		}
 		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
 			$num_port = $nentry->{port};
 			delete $nentry->{port};
@@ -302,8 +303,7 @@ sub net_netrc_loader {
 		while (@tok) {
 			if ($tok[0] eq "default") {
 				shift(@tok);
-				undef $mach; # ignore 'default' lines
-
+				$mach = { machine => undef }
 				next TOKEN;
 			}
 
@@ -313,8 +313,7 @@ sub net_netrc_loader {
 				my $host = shift @tok;
 				$mach = { machine => $host };
 				push @entries, $mach;
-			}
-			elsif (exists $options{tmap}->{$tok}) {
+			} elsif (exists $options{tmap}->{$tok}) {
 				unless ($mach) {
 					log_debug("Skipping token $tok because no machine was given");
 					next TOKEN;
@@ -329,8 +328,7 @@ sub net_netrc_loader {
 				# Following line added by rmerrell to remove '/' escape char in .netrc
 				$value =~ s/\/\\/\\/g;
 				$mach->{$tok} = $value;
-			}
-			elsif ($tok eq "macdef") { # we ignore macros
+			} elsif ($tok eq "macdef") { # we ignore macros
 				next TOKEN unless $mach;
 				my $value = shift @tok;
 				$macdef = 1;
@@ -380,8 +378,7 @@ sub find_netrc_entry {
 				unless ($query->{$check} eq $entry->{$check}) {
 					next ENTRY;
 				}
-			}
-			else {
+			} else {
 				log_debug("OK: any value satisfies check $check");
 			}
 		}
-- 
1.8.1.2.641.g0b90ac4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCHv5] Add contrib/credentials/netrc with GPG support
  2013-02-05 22:24               ` Junio C Hamano
  2013-02-05 23:58                 ` Junio C Hamano
@ 2013-02-06  0:34                 ` Ted Zlatanov
  1 sibling, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-06  0:34 UTC (permalink / raw)
  To: git

On Tue, 05 Feb 2013 14:24:01 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> +		$f =~ s/([;<>\*\|`&\$!#\(\)\[\]\{\}:'"])/\\$1/g;

JCH> Yuck.  If you really have to quote, it is often far simpler to take
JCH> advantage of the fact that quoting rule for shell is much simpler
JCH> inside '', i.e.

JCH> 	sub sq {
JCH> 		my ($string) = @_;
JCH> 		$string =~ s|'|'\\''|g;
JCH> 		return "'$string'";
JCH> 	}

Oh, that's nice.  Thanks.  We don't need it anymore, but I'm sad to see
it go unused.

JCH> I think it is saner to do something like this instead here:
JCH> 				$mach = { machine => undef }

JCH> Otherwise your log_debug() will be filled by the tokens used for the
JCH> default entry, and also this "undef $mach" here will break your
JCH> macdef skipping logic if the default entry has a macdef, I think.

JCH> You can ignore an entry with undefined "machine" in the loop at the
JCH> end of load_netrc.

Cool, I merged your changes into PATCHv6.  I'll keep in mind about
merging the trailing else braces, too.  I forgot that setting for
cperl-mode (`cperl-merge-trailing-else . t').

Thanks
Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv6] Add contrib/credentials/netrc with GPG support
  2013-02-05 23:58                 ` Junio C Hamano
@ 2013-02-06  0:38                   ` Ted Zlatanov
  2013-02-07 23:52                     ` Junio C Hamano
                                       ` (2 more replies)
  0 siblings, 3 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-06  0:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

Add Git credential helper that can parse netrc/authinfo files.

This credential helper supports multiple files, returning the first one
that matches.  It checks file permissions and owner.  For *.gpg files,
it will run GPG to decrypt the file.

Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Changes since PATCHv5:

- reroll from tz/credential-authinfo:
 * brace fixes
 * list attempted default files in help doc
 * 3-arg open() for the GPG pipe
 * instead of skipping 'default' tokens, store them as machine => undef

 contrib/credential/netrc/Makefile             |   12 +
 contrib/credential/netrc/git-credential-netrc |  421 +++++++++++++++++++++++++
 2 files changed, 433 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..18a924f
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+	@(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
+	@echo "=> Silent invocation... nothing should show up here with a missing file"
+	@echo "bad data" | ./git-credential-netrc -f A get
+	@echo "=> Back to noisy: -v and -d used below, missing file"
+	echo "bad data" | ./git-credential-netrc -f A -d -v get
+	@echo "=> Look for any entry in the default file set"
+	echo "" | ./git-credential-netrc -d -v get
+	@echo "=> Look for github.com in the default file set"
+	echo "host=google.com" | ./git-credential-netrc -d -v get
+	@echo "=> Look for a nonexistent machine in the default file set"
+	echo "host=korovamilkbar" | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..30e05fb
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,421 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+	       help => 0,
+	       debug => 0,
+	       verbose => 0,
+	       insecure => 0,
+	       file => [],
+
+	       # identical token maps, e.g. host -> host, will be inserted later
+	       tmap => {
+			port => 'protocol',
+			machine => 'host',
+			path => 'path',
+			login => 'username',
+			user => 'username',
+			password => 'password',
+		       }
+	      );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+	$options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "insecure|k",
+           "verbose|v",
+           "file|f=s@",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
+                       will be decrypted by GPG before parsing.  Multiple -f
+                       arguments are OK.  They are processed in order, and the
+                       first matching entry found is returned via the credential
+                       helper protocol (see below).
+
+                       When no -f option is given, .authinfo.gpg, .netrc.gpg,
+		       .authinfo, and .netrc files in your home directory are used
+		       in this order.
+
+  -k|--insecure      : ignore bad file ownership or permissions
+
+  -d|--debug         : turn on debugging (developer info)
+
+  -v|--verbose       : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper.  It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+host=github.com
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+machine github.com port https login tzz
+
+OR
+
+machine github.com protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol".  Any redundant entry tokens (part of the original query) are
+skipped.
+
+Again, note that only the first matching entry from all the AUTHFILEs, processed
+in the sequence given on the command line, is used.
+
+Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files) {
+	my @candidates = qw[
+				   ~/.authinfo.gpg
+				   ~/.netrc.gpg
+				   ~/.authinfo
+				   ~/.netrc
+			  ];
+
+	$files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files) {
+	my $gpgmode = $file =~ m/\.gpg$/;
+	unless (-r $file) {
+		log_verbose("Unable to read $file; skipping it");
+		next FILE;
+	}
+
+	# the following check is copied from Net::Netrc, for non-GPG files
+	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(
+	unless ($gpgmode || $options{insecure} ||
+		$^O eq 'os2'
+		|| $^O eq 'MSWin32'
+		|| $^O eq 'MacOS'
+		|| $^O =~ /^cygwin/) {
+		my @stat = stat($file);
+
+		if (@stat) {
+			if ($stat[2] & 077) {
+				log_verbose("Insecure $file (mode=%04o); skipping it",
+					    $stat[2] & 07777);
+				next FILE;
+			}
+
+			if ($stat[4] != $<) {
+				log_verbose("Not owner of $file; skipping it");
+				next FILE;
+			}
+		}
+	}
+
+	my @entries = load_netrc($file, $gpgmode);
+
+	unless (scalar @entries) {
+		if ($!) {
+			log_verbose("Unable to open $file: $!");
+		} else {
+			log_verbose("No netrc entries found in $file");
+		}
+
+		next FILE;
+	}
+
+	my $entry = find_netrc_entry($query, @entries);
+	if ($entry) {
+		print_credential_data($entry, $query);
+		# we're done!
+		last FILE;
+	}
+}
+
+exit 0;
+
+sub load_netrc {
+	my $file = shift @_;
+	my $gpgmode = shift @_;
+
+	my $io;
+	if ($gpgmode) {
+		my @cmd = (qw(gpg --decrypt), $file)
+		log_verbose("Using GPG to open $file: [@cmd]");
+		open $io, "-|", @cmd;
+	} else {
+		log_verbose("Opening $file...");
+		open $io, '<', $file;
+	}
+
+	# nothing to do if the open failed (we log the error later)
+	return unless $io;
+
+	# Net::Netrc does this, but the functionality is merged with the file
+	# detection logic, so we have to extract just the part we need
+	my @netrc_entries = net_netrc_loader($io);
+
+	# these entries will use the credential helper protocol token names
+	my @entries;
+
+	foreach my $nentry (@netrc_entries) {
+		my %entry;
+		my $num_port;
+
+		if (!defined $nentry->{machine}) {
+			next;
+		}
+		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+			$num_port = $nentry->{port};
+			delete $nentry->{port};
+		}
+
+		# create the new entry for the credential helper protocol
+		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+		# for "host X port Y" where Y is an integer (captured by
+		# $num_port above), set the host to "X:Y"
+		if (defined $entry{host} && defined $num_port) {
+			$entry{host} = join(':', $entry{host}, $num_port);
+		}
+
+		push @entries, \%entry;
+	}
+
+	return @entries;
+}
+
+sub net_netrc_loader {
+	my $fh = shift @_;
+	my @entries;
+	my ($mach, $macdef, $tok, @tok);
+
+    LINE:
+	while (<$fh>) {
+		undef $macdef if /\A\n\Z/;
+
+		if ($macdef) {
+			next LINE;
+		}
+
+		s/^\s*//;
+		chomp;
+
+		while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+			(my $tok = $+) =~ s/\\(.)/$1/g;
+			push(@tok, $tok);
+		}
+
+	    TOKEN:
+		while (@tok) {
+			if ($tok[0] eq "default") {
+				shift(@tok);
+				$mach = { machine => undef }
+				next TOKEN;
+			}
+
+			$tok = shift(@tok);
+
+			if ($tok eq "machine") {
+				my $host = shift @tok;
+				$mach = { machine => $host };
+				push @entries, $mach;
+			} elsif (exists $options{tmap}->{$tok}) {
+				unless ($mach) {
+					log_debug("Skipping token $tok because no machine was given");
+					next TOKEN;
+				}
+
+				my $value = shift @tok;
+				unless (defined $value) {
+					log_debug("Token $tok had no value, skipping it.");
+					next TOKEN;
+				}
+
+				# Following line added by rmerrell to remove '/' escape char in .netrc
+				$value =~ s/\/\\/\\/g;
+				$mach->{$tok} = $value;
+			} elsif ($tok eq "macdef") { # we ignore macros
+				next TOKEN unless $mach;
+				my $value = shift @tok;
+				$macdef = 1;
+			}
+		}
+	}
+
+	return @entries;
+}
+
+sub read_credential_data_from_stdin {
+	# the query: start with every token with no value
+	my %q = map { $_ => undef } values(%{$options{tmap}});
+
+	while (<STDIN>) {
+		next unless m/^([^=]+)=(.+)/;
+
+		my ($token, $value) = ($1, $2);
+		die "Unknown search token $token" unless exists $q{$token};
+		$q{$token} = $value;
+		log_debug("We were given search token $token and value $value");
+	}
+
+	foreach (sort keys %q) {
+		log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+	}
+
+	return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry {
+	my $query = shift @_;
+
+    ENTRY:
+	foreach my $entry (@_)
+	{
+		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+		foreach my $check (sort keys %$query) {
+			if (defined $query->{$check}) {
+				log_debug("compare %s [%s] to [%s] (entry: %s)",
+					  $check,
+					  $entry->{$check},
+					  $query->{$check},
+					  $entry_text);
+				unless ($query->{$check} eq $entry->{$check}) {
+					next ENTRY;
+				}
+			} else {
+				log_debug("OK: any value satisfies check $check");
+			}
+		}
+
+		return $entry;
+	}
+
+	# nothing was found
+	return;
+}
+
+sub print_credential_data {
+	my $entry = shift @_;
+	my $query = shift @_;
+
+	log_debug("entry has passed all the search checks");
+ TOKEN:
+	foreach my $git_token (sort keys %$entry) {
+		log_debug("looking for useful token $git_token");
+		# don't print unknown (to the credential helper protocol) tokens
+		next TOKEN unless exists $query->{$git_token};
+
+		# don't print things asked in the query (the entry matches them)
+		next TOKEN if defined $query->{$git_token};
+
+		log_debug("FOUND: $git_token=$entry->{$git_token}");
+		printf "%s=%s\n", $git_token, $entry->{$git_token};
+	}
+}
+sub log_verbose {
+	return unless $options{verbose};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
+
+sub log_debug {
+	return unless $options{debug};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
  2013-02-06  0:38                   ` [PATCHv6] " Ted Zlatanov
@ 2013-02-07 23:52                     ` Junio C Hamano
  2013-02-08  1:53                       ` Ted Zlatanov
  2013-02-08  6:18                     ` Jeff King
  2013-02-25 15:49                     ` [PATCH v7] " Ted Zlatanov
  2 siblings, 1 reply; 38+ messages in thread
From: Junio C Hamano @ 2013-02-07 23:52 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> Add Git credential helper that can parse netrc/authinfo files.

I think this line is redundant; we already know it on the Subject: line.

> This credential helper supports multiple files, returning the first one
> that matches.  It checks file permissions and owner.  For *.gpg files,
> it will run GPG to decrypt the file.
>
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
> ---
> ...
> diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
> new file mode 100644
> index 0000000..18a924f
> --- /dev/null
> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,12 @@
> +test_netrc:
> +	@(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring failure"
> +	@echo "=> Silent invocation... nothing should show up here with a missing file"
> +	@echo "bad data" | ./git-credential-netrc -f A get
> +	@echo "=> Back to noisy: -v and -d used below, missing file"
> +	echo "bad data" | ./git-credential-netrc -f A -d -v get
> +	@echo "=> Look for any entry in the default file set"
> +	echo "" | ./git-credential-netrc -d -v get
> +	@echo "=> Look for github.com in the default file set"
> +	echo "host=google.com" | ./git-credential-netrc -d -v get
> +	@echo "=> Look for a nonexistent machine in the default file set"
> +	echo "host=korovamilkbar" | ./git-credential-netrc -d -v get

Whose netrc is this reading?

Don't we want all of them to have "-f A" and ship "A" (rename it to
something more reasonable), so that anybody can notice when he tries
to improve it and breaks it?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
  2013-02-07 23:52                     ` Junio C Hamano
@ 2013-02-08  1:53                       ` Ted Zlatanov
  2013-02-08  6:15                         ` Junio C Hamano
  0 siblings, 1 reply; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-08  1:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Thu, 07 Feb 2013 15:52:41 -0800 Junio C Hamano <gitster@pobox.com> wrote: 

>> +	@echo "=> Look for any entry in the default file set"
>> +	echo "" | ./git-credential-netrc -d -v get
>> +	@echo "=> Look for github.com in the default file set"
>> +	echo "host=google.com" | ./git-credential-netrc -d -v get
>> +	@echo "=> Look for a nonexistent machine in the default file set"
>> +	echo "host=korovamilkbar" | ./git-credential-netrc -d -v get

JCH> Whose netrc is this reading?

JCH> Don't we want all of them to have "-f A" and ship "A" (rename it to
JCH> something more reasonable), so that anybody can notice when he tries
JCH> to improve it and breaks it?

I agree this Makefile is not a good test to ship out.  It was my quickie
test rig that I should have reworked before adding to the patch.  Sorry.

I see contrib/subtree/t and contrib/mw-to-git/t that I could copy.  The
test will have a few files to parse, and will be able to compare the
expected to the actual output.  Does that sound like a good plan?

Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
  2013-02-08  1:53                       ` Ted Zlatanov
@ 2013-02-08  6:15                         ` Junio C Hamano
  0 siblings, 0 replies; 38+ messages in thread
From: Junio C Hamano @ 2013-02-08  6:15 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Jeff King, git

Ted Zlatanov <tzz@lifelogs.com> writes:

> I agree this Makefile is not a good test to ship out.  It was my quickie
> test rig that I should have reworked before adding to the patch.  Sorry.

Nothing to be sorry about.  Starting with quick-and-dirty and
polishing for public consumption is what the review cycle is about,
and we are here to help that process.

> I see contrib/subtree/t and contrib/mw-to-git/t that I could copy.  The
> test will have a few files to parse, and will be able to compare the
> expected to the actual output.  Does that sound like a good plan?

Yup.

Thanks.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
  2013-02-06  0:38                   ` [PATCHv6] " Ted Zlatanov
  2013-02-07 23:52                     ` Junio C Hamano
@ 2013-02-08  6:18                     ` Jeff King
  2013-02-25 16:24                       ` Ted Zlatanov
  2013-02-25 15:49                     ` [PATCH v7] " Ted Zlatanov
  2 siblings, 1 reply; 38+ messages in thread
From: Jeff King @ 2013-02-08  6:18 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: Junio C Hamano, git

On Tue, Feb 05, 2013 at 07:38:58PM -0500, Ted Zlatanov wrote:

> Add Git credential helper that can parse netrc/authinfo files.
> 
> This credential helper supports multiple files, returning the first one
> that matches.  It checks file permissions and owner.  For *.gpg files,
> it will run GPG to decrypt the file.
> 
> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>


> +	# the following check is copied from Net::Netrc, for non-GPG files
> +	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(

s/compatable/compatible/

You mention os/2 and Win32 here, but the check has more:

> +	unless ($gpgmode || $options{insecure} ||
> +		$^O eq 'os2'
> +		|| $^O eq 'MSWin32'
> +		|| $^O eq 'MacOS'
> +		|| $^O =~ /^cygwin/) {

Does MacOS really not handle stat? Or is this old MacOS, not OS X?

> +sub load_netrc {
> [...]
> +	foreach my $nentry (@netrc_entries) {
> +		my %entry;
> +		my $num_port;
> +
> +		if (!defined $nentry->{machine}) {
> +			next;
> +		}
> +		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
> +			$num_port = $nentry->{port};
> +			delete $nentry->{port};
> +		}
> +
> +		# create the new entry for the credential helper protocol
> +		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
> +
> +		# for "host X port Y" where Y is an integer (captured by
> +		# $num_port above), set the host to "X:Y"
> +		if (defined $entry{host} && defined $num_port) {
> +			$entry{host} = join(':', $entry{host}, $num_port);
> +		}

So this will convert:

  machine foo port smtp

in the netrc into (protocol => "smtp", host => "foo"), but:

  machine foo port 25

into (protocol => undef, host => "foo:25"), right? That makes sense to
me.

> +sub net_netrc_loader {
> [...]

I won't comment here, as I know very little about netrc (I always
thought it was line-oriented, too!) and Junio has covered it.

> +# takes the search tokens and then a list of entries
> +# each entry is a hash reference
> +sub find_netrc_entry {
> +	my $query = shift @_;
> +
> +    ENTRY:
> +	foreach my $entry (@_)
> +	{
> +		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
> +		foreach my $check (sort keys %$query) {
> +			if (defined $query->{$check}) {
> +				log_debug("compare %s [%s] to [%s] (entry: %s)",
> +					  $check,
> +					  $entry->{$check},
> +					  $query->{$check},
> +					  $entry_text);
> +				unless ($query->{$check} eq $entry->{$check}) {
> +					next ENTRY;
> +				}
> +			} else {
> +				log_debug("OK: any value satisfies check $check");
> +			}

This looks right to me.

> +sub print_credential_data {

I don't know if you want to take the hit of relying on Git.pm (it is
nice for the helper to be totally standalone and copy-able), but one
obvious possible refactor would be to use the credential read/write
functions recently added there. I'm OK with not doing that, though.

> +	my $entry = shift @_;
> +	my $query = shift @_;
> +
> +	log_debug("entry has passed all the search checks");
> + TOKEN:
> +	foreach my $git_token (sort keys %$entry) {
> +		log_debug("looking for useful token $git_token");
> +		# don't print unknown (to the credential helper protocol) tokens
> +		next TOKEN unless exists $query->{$git_token};
> +
> +		# don't print things asked in the query (the entry matches them)
> +		next TOKEN if defined $query->{$git_token};
> +
> +		log_debug("FOUND: $git_token=$entry->{$git_token}");
> +		printf "%s=%s\n", $git_token, $entry->{$git_token};
> +	}

Printf? Bleh, isn't this supposed to be perl? :P

I don't see anything wrong from the credential-handling side of things.
As I said, I didn't look closely at the netrc parsing bits. From my
reading of "perldoc macos", the answer to my question above is "yes,
stat doesn't work on MacOS Classic". So I think the script itself is
fine.

In your tests:

> +++ b/contrib/credential/netrc/Makefile
> @@ -0,0 +1,12 @@
> +test_netrc:
> +       @(echo "bad data" | ./git-credential-netrc -f A -d -v) || echo "Bad invocation test, ignoring
> failure"
> +       @echo "=> Silent invocation... nothing should show up here with a missing file"
> +       @echo "bad data" | ./git-credential-netrc -f A get
> +       @echo "=> Back to noisy: -v and -d used below, missing file"
> +       echo "bad data" | ./git-credential-netrc -f A -d -v get
> +       @echo "=> Look for any entry in the default file set"
> +       echo "" | ./git-credential-netrc -d -v get
> +       @echo "=> Look for github.com in the default file set"
> +       echo "host=google.com" | ./git-credential-netrc -d -v get
> +       @echo "=> Look for a nonexistent machine in the default file set"
> +       echo "host=korovamilkbar" | ./git-credential-netrc -d -v get

You are depending on whatever the user has in their ~/.netrc, no?
Wouldn't it make more sense to ship a sample netrc and run all of the
tests with "-f netrc.example"?

It may also be worth building on top of the regular git test harness.
It's more work, but the resulting code (and the output) will be much
more readable.

-Peff

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v7] Add contrib/credentials/netrc with GPG support
  2013-02-06  0:38                   ` [PATCHv6] " Ted Zlatanov
  2013-02-07 23:52                     ` Junio C Hamano
  2013-02-08  6:18                     ` Jeff King
@ 2013-02-25 15:49                     ` Ted Zlatanov
  2 siblings, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-25 15:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

This credential helper supports multiple files, returning the first one
that matches.  It checks file permissions and owner.  For *.gpg files,
it will run GPG to decrypt the file.

Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Changes since PATCHv6:

- change Makefile test to test.pl (using Perl Test module) + test.netrc
 * `make test' runs all the tests in the standard Test format
 * `make testverbose' runs the tests with -d -v to see what's happening
- fix missing semicolons and minor typos

 contrib/credential/netrc/Makefile             |    5 +
 contrib/credential/netrc/git-credential-netrc |  421 +++++++++++++++++++++++++
 contrib/credential/netrc/test.netrc           |   13 +
 contrib/credential/netrc/test.pl              |  106 +++++++
 4 files changed, 545 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc
 create mode 100644 contrib/credential/netrc/test.netrc
 create mode 100755 contrib/credential/netrc/test.pl

diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
new file mode 100644
index 0000000..51b7613
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,5 @@
+test:
+	./test.pl
+
+testverbose:
+	./test.pl -d -v
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 0000000..6c51c43
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,421 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = "0.1";
+
+my %options = (
+	       help => 0,
+	       debug => 0,
+	       verbose => 0,
+	       insecure => 0,
+	       file => [],
+
+	       # identical token maps, e.g. host -> host, will be inserted later
+	       tmap => {
+			port => 'protocol',
+			machine => 'host',
+			path => 'path',
+			login => 'username',
+			user => 'username',
+			password => 'password',
+		       }
+	      );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+	$options{tmap}->{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+	push @{$rmap{$options{tmap}->{$k}}}, $k;
+}
+
+Getopt::Long::Configure("bundling");
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+           "help|h",
+           "debug|d",
+           "insecure|k",
+           "verbose|v",
+           "file|f=s@",
+          );
+
+if ($options{help}) {
+	my $shortname = basename($0);
+	$shortname =~ s/git-credential-//;
+
+	print <<EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
+                       will be decrypted by GPG before parsing.  Multiple -f
+                       arguments are OK.  They are processed in order, and the
+                       first matching entry found is returned via the credential
+                       helper protocol (see below).
+
+                       When no -f option is given, .authinfo.gpg, .netrc.gpg,
+		       .authinfo, and .netrc files in your home directory are used
+		       in this order.
+
+  -k|--insecure      : ignore bad file ownership or permissions
+
+  -d|--debug         : turn on debugging (developer info)
+
+  -v|--verbose       : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend "git-credential-" to the helper name and look for it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only "get" mode is supported by this credential helper.  It opens every AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we already have one. (username=X)
+
+Thus, when we get this query on STDIN:
+
+host=github.com
+protocol=https
+username=tzz
+
+this credential helper will look for the first entry in every AUTHFILE that
+matches
+
+machine github.com port https login tzz
+
+OR
+
+machine github.com protocol https login tzz
+
+OR... etc. acceptable tokens as listed above.  Any unknown tokens are
+simply ignored.
+
+Then, the helper will print out whatever tokens it got from the entry, including
+"password" tokens, mapping back to Git's helper protocol; e.g. "port" is mapped
+back to "protocol".  Any redundant entry tokens (part of the original query) are
+skipped.
+
+Again, note that only the first matching entry from all the AUTHFILEs, processed
+in the sequence given on the command line, is used.
+
+Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
+
+No caching is performed by this credential helper.
+
+EOHIPPUS
+
+	exit 0;
+}
+
+my $mode = shift @ARGV;
+
+# Credentials must get a parameter, so die if it's missing.
+die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
+
+# Only support 'get' mode; with any other unsupported ones we just exit.
+exit 0 unless $mode eq 'get';
+
+my $files = $options{file};
+
+# if no files were given, use a predefined list.
+# note that .gpg files come first
+unless (scalar @$files) {
+	my @candidates = qw[
+				   ~/.authinfo.gpg
+				   ~/.netrc.gpg
+				   ~/.authinfo
+				   ~/.netrc
+			  ];
+
+	$files = $options{file} = [ map { glob $_ } @candidates ];
+}
+
+my $query = read_credential_data_from_stdin();
+
+FILE:
+foreach my $file (@$files) {
+	my $gpgmode = $file =~ m/\.gpg$/;
+	unless (-r $file) {
+		log_verbose("Unable to read $file; skipping it");
+		next FILE;
+	}
+
+	# the following check is copied from Net::Netrc, for non-GPG files
+	# OS/2 and Win32 do not handle stat in a way compatible with this check :-(
+	unless ($gpgmode || $options{insecure} ||
+		$^O eq 'os2'
+		|| $^O eq 'MSWin32'
+		|| $^O eq 'MacOS'
+		|| $^O =~ /^cygwin/) {
+		my @stat = stat($file);
+
+		if (@stat) {
+			if ($stat[2] & 077) {
+				log_verbose("Insecure $file (mode=%04o); skipping it",
+					    $stat[2] & 07777);
+				next FILE;
+			}
+
+			if ($stat[4] != $<) {
+				log_verbose("Not owner of $file; skipping it");
+				next FILE;
+			}
+		}
+	}
+
+	my @entries = load_netrc($file, $gpgmode);
+
+	unless (scalar @entries) {
+		if ($!) {
+			log_verbose("Unable to open $file: $!");
+		} else {
+			log_verbose("No netrc entries found in $file");
+		}
+
+		next FILE;
+	}
+
+	my $entry = find_netrc_entry($query, @entries);
+	if ($entry) {
+		print_credential_data($entry, $query);
+		# we're done!
+		last FILE;
+	}
+}
+
+exit 0;
+
+sub load_netrc {
+	my $file = shift @_;
+	my $gpgmode = shift @_;
+
+	my $io;
+	if ($gpgmode) {
+		my @cmd = (qw(gpg --decrypt), $file);
+		log_verbose("Using GPG to open $file: [@cmd]");
+		open $io, "-|", @cmd;
+	} else {
+		log_verbose("Opening $file...");
+		open $io, '<', $file;
+	}
+
+	# nothing to do if the open failed (we log the error later)
+	return unless $io;
+
+	# Net::Netrc does this, but the functionality is merged with the file
+	# detection logic, so we have to extract just the part we need
+	my @netrc_entries = net_netrc_loader($io);
+
+	# these entries will use the credential helper protocol token names
+	my @entries;
+
+	foreach my $nentry (@netrc_entries) {
+		my %entry;
+		my $num_port;
+
+		if (!defined $nentry->{machine}) {
+			next;
+		}
+		if (defined $nentry->{port} && $nentry->{port} =~ m/^\d+$/) {
+			$num_port = $nentry->{port};
+			delete $nentry->{port};
+		}
+
+		# create the new entry for the credential helper protocol
+		$entry{$options{tmap}->{$_}} = $nentry->{$_} foreach keys %$nentry;
+
+		# for "host X port Y" where Y is an integer (captured by
+		# $num_port above), set the host to "X:Y"
+		if (defined $entry{host} && defined $num_port) {
+			$entry{host} = join(':', $entry{host}, $num_port);
+		}
+
+		push @entries, \%entry;
+	}
+
+	return @entries;
+}
+
+sub net_netrc_loader {
+	my $fh = shift @_;
+	my @entries;
+	my ($mach, $macdef, $tok, @tok);
+
+    LINE:
+	while (<$fh>) {
+		undef $macdef if /\A\n\Z/;
+
+		if ($macdef) {
+			next LINE;
+		}
+
+		s/^\s*//;
+		chomp;
+
+		while (length && s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
+			(my $tok = $+) =~ s/\\(.)/$1/g;
+			push(@tok, $tok);
+		}
+
+	    TOKEN:
+		while (@tok) {
+			if ($tok[0] eq "default") {
+				shift(@tok);
+				$mach = { machine => undef };
+				next TOKEN;
+			}
+
+			$tok = shift(@tok);
+
+			if ($tok eq "machine") {
+				my $host = shift @tok;
+				$mach = { machine => $host };
+				push @entries, $mach;
+			} elsif (exists $options{tmap}->{$tok}) {
+				unless ($mach) {
+					log_debug("Skipping token $tok because no machine was given");
+					next TOKEN;
+				}
+
+				my $value = shift @tok;
+				unless (defined $value) {
+					log_debug("Token $tok had no value, skipping it.");
+					next TOKEN;
+				}
+
+				# Following line added by rmerrell to remove '/' escape char in .netrc
+				$value =~ s/\/\\/\\/g;
+				$mach->{$tok} = $value;
+			} elsif ($tok eq "macdef") { # we ignore macros
+				next TOKEN unless $mach;
+				my $value = shift @tok;
+				$macdef = 1;
+			}
+		}
+	}
+
+	return @entries;
+}
+
+sub read_credential_data_from_stdin {
+	# the query: start with every token with no value
+	my %q = map { $_ => undef } values(%{$options{tmap}});
+
+	while (<STDIN>) {
+		next unless m/^([^=]+)=(.+)/;
+
+		my ($token, $value) = ($1, $2);
+		die "Unknown search token $token" unless exists $q{$token};
+		$q{$token} = $value;
+		log_debug("We were given search token $token and value $value");
+	}
+
+	foreach (sort keys %q) {
+		log_debug("Searching for %s = %s", $_, $q{$_} || '(any value)');
+	}
+
+	return \%q;
+}
+
+# takes the search tokens and then a list of entries
+# each entry is a hash reference
+sub find_netrc_entry {
+	my $query = shift @_;
+
+    ENTRY:
+	foreach my $entry (@_)
+	{
+		my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
+		foreach my $check (sort keys %$query) {
+			if (defined $query->{$check}) {
+				log_debug("compare %s [%s] to [%s] (entry: %s)",
+					  $check,
+					  $entry->{$check},
+					  $query->{$check},
+					  $entry_text);
+				unless ($query->{$check} eq $entry->{$check}) {
+					next ENTRY;
+				}
+			} else {
+				log_debug("OK: any value satisfies check $check");
+			}
+		}
+
+		return $entry;
+	}
+
+	# nothing was found
+	return;
+}
+
+sub print_credential_data {
+	my $entry = shift @_;
+	my $query = shift @_;
+
+	log_debug("entry has passed all the search checks");
+ TOKEN:
+	foreach my $git_token (sort keys %$entry) {
+		log_debug("looking for useful token $git_token");
+		# don't print unknown (to the credential helper protocol) tokens
+		next TOKEN unless exists $query->{$git_token};
+
+		# don't print things asked in the query (the entry matches them)
+		next TOKEN if defined $query->{$git_token};
+
+		log_debug("FOUND: $git_token=$entry->{$git_token}");
+		printf "%s=%s\n", $git_token, $entry->{$git_token};
+	}
+}
+sub log_verbose {
+	return unless $options{verbose};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
+
+sub log_debug {
+	return unless $options{debug};
+	printf STDERR @_;
+	printf STDERR "\n";
+}
diff --git a/contrib/credential/netrc/test.netrc b/contrib/credential/netrc/test.netrc
new file mode 100644
index 0000000..ba119a9
--- /dev/null
+++ b/contrib/credential/netrc/test.netrc
@@ -0,0 +1,13 @@
+machine imap login tzz@lifelogs.com port imaps password letmeknow
+machine imap login bob port imaps password bobwillknow
+
+# comment test
+
+machine imap2 login tzz port 1099 password tzzknow
+machine imap2 login bob password bobwillknow
+
+# another command
+
+machine github.com
+  multilinetoken anothervalue
+  login carol password carolknows
diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl
new file mode 100755
index 0000000..169b646
--- /dev/null
+++ b/contrib/credential/netrc/test.pl
@@ -0,0 +1,106 @@
+#!/usr/bin/perl
+
+use warnings;
+use strict;
+use Test;
+use IPC::Open2;
+
+BEGIN { plan tests => 15 }
+
+my @global_credential_args = @ARGV;
+my $netrc = './test.netrc';
+print "# Testing insecure file, nothing should be found\n";
+chmod 0644, $netrc;
+my $cred = run_credential(['-f', $netrc, 'get'],
+			  { host => 'github.com' });
+
+ok(scalar keys %$cred, 0, "Got 0 keys from insecure file");
+
+print "# Testing missing file, nothing should be found\n";
+chmod 0644, $netrc;
+$cred = run_credential(['-f', '///nosuchfile///', 'get'],
+		       { host => 'github.com' });
+
+ok(scalar keys %$cred, 0, "Got 0 keys from missing file");
+
+chmod 0600, $netrc;
+
+print "# Testing with invalid data\n";
+$cred = run_credential(['-f', $netrc, 'get'],
+		       "bad data");
+ok(scalar keys %$cred, 4, "Got first found keys with bad data");
+
+print "# Testing netrc file for a missing corovamilkbar entry\n";
+$cred = run_credential(['-f', $netrc, 'get'],
+		       { host => 'corovamilkbar' });
+
+ok(scalar keys %$cred, 0, "Got no corovamilkbar keys");
+
+print "# Testing netrc file for a github.com entry\n";
+$cred = run_credential(['-f', $netrc, 'get'],
+		       { host => 'github.com' });
+
+ok(scalar keys %$cred, 2, "Got 2 Github keys");
+
+ok($cred->{password}, 'carolknows', "Got correct Github password");
+ok($cred->{username}, 'carol', "Got correct Github username");
+
+print "# Testing netrc file for a username-specific entry\n";
+$cred = run_credential(['-f', $netrc, 'get'],
+		       { host => 'imap', username => 'bob' });
+
+ok(scalar keys %$cred, 2, "Got 2 username-specific keys");
+
+ok($cred->{password}, 'bobwillknow', "Got correct user-specific password");
+ok($cred->{protocol}, 'imaps', "Got correct user-specific protocol");
+
+print "# Testing netrc file for a host:port-specific entry\n";
+$cred = run_credential(['-f', $netrc, 'get'],
+		       { host => 'imap2:1099' });
+
+ok(scalar keys %$cred, 2, "Got 2 host:port-specific keys");
+
+ok($cred->{password}, 'tzzknow', "Got correct host:port-specific password");
+ok($cred->{username}, 'tzz', "Got correct host:port-specific username");
+
+print "# Testing netrc file that 'host:port kills host' entry\n";
+$cred = run_credential(['-f', $netrc, 'get'],
+		       { host => 'imap2' });
+
+ok(scalar keys %$cred, 2, "Got 2 'host:port kills host' keys");
+
+ok($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
+ok($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
+
+sub run_credential
+{
+	my $args = shift @_;
+	my $data = shift @_;
+	my $pid = open2(my $chld_out, my $chld_in,
+			'./git-credential-netrc', @global_credential_args,
+			@$args);
+
+	die "Couldn't open pipe to netrc credential helper: $!" unless $pid;
+
+	if (ref $data eq 'HASH')
+	{
+		print $chld_in "$_=$data->{$_}\n" foreach sort keys %$data;
+	}
+	else
+	{
+		print $chld_in "$data\n";
+	}
+
+	close $chld_in;
+	my %ret;
+
+	while (<$chld_out>)
+	{
+		chomp;
+		next unless m/^([^=]+)=(.+)/;
+
+		$ret{$1} = $2;
+	}
+
+	return \%ret;
+}
-- 
1.7.9.rc2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
  2013-02-08  6:18                     ` Jeff King
@ 2013-02-25 16:24                       ` Ted Zlatanov
  0 siblings, 0 replies; 38+ messages in thread
From: Ted Zlatanov @ 2013-02-25 16:24 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Fri, 8 Feb 2013 01:18:55 -0500 Jeff King <peff@peff.net> wrote: 

>> +	# the following check is copied from Net::Netrc, for non-GPG files
>> +	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(

JK> s/compatable/compatible/

This is from the Net::Netrc module.  Fixed in my commit but eh...

JK> You mention os/2 and Win32 here, but the check has more:

>> +	unless ($gpgmode || $options{insecure} ||
>> +		$^O eq 'os2'
>> +		|| $^O eq 'MSWin32'
>> +		|| $^O eq 'MacOS'
>> +		|| $^O =~ /^cygwin/) {

JK> Does MacOS really not handle stat? Or is this old MacOS, not OS X?

This is all out of Net::Netrc, and yes, it's pre-Mac OS X.  I think it's
safe to leave as is, but I can remove OS/2 and MacOS if you prefer.

JK> So this will convert:
JK>   machine foo port smtp
JK> in the netrc into (protocol => "smtp", host => "foo"), but:
JK>   machine foo port 25
JK> into (protocol => undef, host => "foo:25"), right? That makes sense to
JK> me.

Yes.  test.pl checks that host=foo doesn't find the above, as well.

JK> I don't know if you want to take the hit of relying on Git.pm (it is
JK> nice for the helper to be totally standalone and copy-able), but one
JK> obvious possible refactor would be to use the credential read/write
JK> functions recently added there. I'm OK with not doing that, though.

JK> It may also be worth building on top of the regular git test harness.
JK> It's more work, but the resulting code (and the output) will be much
JK> more readable.

At least for now let's leave it standalone.  When and if it moves into
the core, we can change it to use the core's Git.pm and test suite.  The
code and the tests are small enough that I think using Perl's Test
module makes the most sense right now.

JK> Printf? Bleh, isn't this supposed to be perl? :P

What?  Was I supposed to use formats?!?!

JK> You are depending on whatever the user has in their ~/.netrc, no?
JK> Wouldn't it make more sense to ship a sample netrc and run all of the
JK> tests with "-f netrc.example"?

Yes.  See test.netrc.

Thanks
Ted

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2013-02-25 16:24 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 19:54 [PATCH] Add contrib/credentials/netrc with GPG support Ted Zlatanov
2013-02-04 21:17 ` Jeff King
2013-02-04 21:32   ` Ted Zlatanov
2013-02-04 21:44   ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
2013-02-04 22:56     ` Junio C Hamano
2013-02-04 23:23       ` Jeff King
2013-02-04 23:36         ` Junio C Hamano
2013-02-04 23:42         ` Ted Zlatanov
2013-02-04 23:28       ` [PATCHv3] Add contrib/credentials/netrc with GPG support Ted Zlatanov
2013-02-04 23:31       ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
2013-02-04 23:40         ` Junio C Hamano
2013-02-04 23:54           ` Ted Zlatanov
2013-02-05  0:15             ` Junio C Hamano
2013-02-05 13:39               ` Ted Zlatanov
2013-02-05 16:07                 ` Junio C Hamano
2013-02-05 16:18                   ` Junio C Hamano
2013-02-05 16:15     ` Junio C Hamano
2013-02-05 17:01       ` Ted Zlatanov
2013-02-05 18:55         ` [PATCHv4] Add contrib/credentials/netrc with GPG support Ted Zlatanov
2013-02-05 19:53           ` Junio C Hamano
2013-02-05 20:47             ` Ted Zlatanov
2013-02-05 22:09               ` Junio C Hamano
2013-02-05 22:30                 ` Ted Zlatanov
2013-02-05 20:55             ` [PATCHv5] " Ted Zlatanov
2013-02-05 22:24               ` Junio C Hamano
2013-02-05 23:58                 ` Junio C Hamano
2013-02-06  0:38                   ` [PATCHv6] " Ted Zlatanov
2013-02-07 23:52                     ` Junio C Hamano
2013-02-08  1:53                       ` Ted Zlatanov
2013-02-08  6:15                         ` Junio C Hamano
2013-02-08  6:18                     ` Jeff King
2013-02-25 16:24                       ` Ted Zlatanov
2013-02-25 15:49                     ` [PATCH v7] " Ted Zlatanov
2013-02-06  0:34                 ` [PATCHv5] " Ted Zlatanov
2013-02-05 19:47         ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Junio C Hamano
2013-02-05 20:03           ` Ted Zlatanov
2013-02-05 20:23             ` Junio C Hamano
2013-02-05 21:00               ` Ted Zlatanov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.