git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] git credential plumbing command: implementation and use
@ 2012-06-22 16:07 Matthieu Moy
  2012-06-22 16:07 ` [PATCH 1/3] add 'git credential' plumbing command Matthieu Moy
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Matthieu Moy @ 2012-06-22 16:07 UTC (permalink / raw)
  To: git, gitster; +Cc: Javier.Roucher-Iglesias, Matthieu Moy

This is a rework of an earlier patch serie:

  http://thread.gmane.org/gmane.comp.version-control.git/199794/focus=199802
  http://thread.gmane.org/gmane.comp.version-control.git/199789

We first add a new "git credential" command, that replaces
test-credential.c and wraps the C API, and then use it in the
git-remote-mediawiki interface.

PATCH 3 is actually a rewrite of Javier's patch, so I took the
ownership of the patch, but mentionned Javier in the commit message.

Javier Roucher Iglesias (1):
  add 'git credential' plumbing command

Matthieu Moy (2):
  git credential fill: output the whole 'struct credential'
  git-remote-mediawiki: add credential support

 .gitignore                                  |   2 +-
 Documentation/git-credential.txt            | 133 ++++++++++++++++++++++++++++
 Documentation/technical/api-credentials.txt |  39 +-------
 Makefile                                    |   2 +-
 builtin.h                                   |   1 +
 builtin/credential.c                        |  31 +++++++
 contrib/mw-to-git/git-remote-mediawiki      | 106 +++++++++++++++++++---
 credential.c                                |   2 +-
 credential.h                                |   1 +
 git.c                                       |   1 +
 t/lib-credential.sh                         |  39 +++++++-
 t/t0300-credentials.sh                      |  14 +++
 test-credential.c                           |  38 --------
 13 files changed, 318 insertions(+), 91 deletions(-)
 create mode 100644 Documentation/git-credential.txt
 create mode 100644 builtin/credential.c
 delete mode 100644 test-credential.c

-- 
1.7.11.5.g0c7e058.dirty

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

* [PATCH 1/3] add 'git credential' plumbing command
  2012-06-22 16:07 [PATCH 0/3] git credential plumbing command: implementation and use Matthieu Moy
@ 2012-06-22 16:07 ` Matthieu Moy
  2012-06-22 20:24   ` Junio C Hamano
  2012-06-22 16:07 ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
  2012-06-22 16:07 ` [PATCH 3/3] git-remote-mediawiki: add credential support Matthieu Moy
  2 siblings, 1 reply; 11+ messages in thread
From: Matthieu Moy @ 2012-06-22 16:07 UTC (permalink / raw)
  To: git, gitster
  Cc: Javier.Roucher-Iglesias, Pavel Volek, Kim Thuat Nguyen,
	Matthieu Moy

From: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>

The credential API is in C, and not available to scripting languages.
Expose the functionalities of the API by wrapping them into a new
plumbing command "git credentials".

In other words, replace the internal "test-credential" by an official Git
command.

Most documentation writen by: Jeff King <peff@peff.net>
Signed-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>
Signed-off-by: Kim Thuat Nguyen <Kim-Thuat.Nguyen@ensimag.imag.fr>
Signed-off-by: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 .gitignore                                  |   2 +-
 Documentation/git-credential.txt            | 126 ++++++++++++++++++++++++++++
 Documentation/technical/api-credentials.txt |  39 +--------
 Makefile                                    |   2 +-
 builtin.h                                   |   1 +
 test-credential.c => builtin/credential.c   |  20 ++---
 git.c                                       |   1 +
 t/lib-credential.sh                         |  14 +++-
 8 files changed, 153 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/git-credential.txt
 rename test-credential.c => builtin/credential.c (63%)

diff --git a/.gitignore b/.gitignore
index bf66648..c188d0b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,6 +31,7 @@
 /git-commit-tree
 /git-config
 /git-count-objects
+/git-credential
 /git-credential-cache
 /git-credential-cache--daemon
 /git-credential-store
@@ -172,7 +173,6 @@
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
 /test-chmtime
-/test-credential
 /test-ctype
 /test-date
 /test-delta
diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
new file mode 100644
index 0000000..b64ac30
--- /dev/null
+++ b/Documentation/git-credential.txt
@@ -0,0 +1,126 @@
+git-credential(7)
+=================
+
+NAME
+----
+git-credential - retrieve and store user credentials
+
+SYNOPSIS
+--------
+------------------
+git credential <fill|approve|reject>
+------------------
+
+DESCRIPTION
+-----------
+
+Git has an internal interface for storing and retrieving credentials
+from system-specific helpers, as well as prompting the user for
+usernames and passwords. The git-credential command exposes this
+interface to scripts which may want to retrieve, store, or prompt for
+credentials in the same manner as git. The design of this scriptable
+interface models the internal C API; see
+link:technical/api-credentials.txt[the git credential API] for more
+background on the concepts.
+
+git-credential takes an "action" option on the command-line (one of
+`fill`, `approve`, or `reject`) and reads a credential description
+on stdin (see <<IOFMT,INPUT/OUTPUT FORMAT>>).
+
+If the action is `fill`, git-credential will attempt to add "username"
+and "password" attributes to the description by reading config files,
+by contacting any configured credential helpers, or by prompting the
+user. The username and password attributes of the credential
+description are then printed to stdout together with the attributes
+already provided.
+
+If the action is `approve`, git-credential will send the description
+to any configured credential helpers, which may store the credential
+for later use. 
+
+If the action is `reject`, git-credential will send the description to
+any configured credential helpers, which may erase any stored
+credential matching the description.
+
+If the action is `approve` or `reject`, no output should be emitted.
+
+TYPICAL WORKFLOW
+----------------
+
+An application using git-credential will typically follow this
+workflow:
+
+  1. Generate a credential description based on the context.
++
+For example, if we want a password for
+`https://example.com/foo.git`, we might generate the following
+credential description (don't forget the blank line at the end):
+
+         protocol=https
+         host=example.com
+         path=foo.git
+
+  2. Ask git-credential to give us a username and password for this
+     description. This is done by running `git credential fill`,
+     feeding the description from step (1) to its stdin. The username
+     and password will be produced on stdout, like:
+
+	username=bob
+	password=secr3t
+
+  3. Try to use the credential (e.g., by accessing the URL with the
+     username and password from step (2)).
+
+  4. Report on the success or failure of the password. If the
+     credential allowed the operation to complete successfully, then
+     it can be marked with an "approve" action. If the credential was
+     rejected during the operation, use the "reject" action. In either
+     case, `git credential` should be fed with the credential
+     description obtained from step (2) together with the ones already
+     provided in step (1).
+
+[[IOFMT]]
+INPUT/OUTPUT FORMAT
+-------------------
+
+`git credential` reads and/or writes (depending on the action used)
+credential information in its standard input/output. These information
+can correspond either to keys from which `git credential` will obtain
+the login/password information (e.g. host, protocol, path), or to the
+actual credential data to be obtained (login/password).
+
+The credential is split into a set of named attributes.
+Attributes are provided to the helper, one per line. Each attribute is
+specified by a key-value pair, separated by an `=` (equals) sign,
+followed by a newline. The key may contain any bytes except `=`,
+newline, or NUL. The value may contain any bytes except newline or NUL.
+In both cases, all bytes are treated as-is (i.e., there is no quoting,
+and one cannot transmit a value with newline or NUL in it). The list of
+attributes is terminated by a blank line or end-of-file.
+Git will send the following attributes (but may not send all of
+them for a given credential; for example, a `host` attribute makes no
+sense when dealing with a non-network protocol):
+
+`protocol`::
+
+	The protocol over which the credential will be used (e.g.,
+	`https`).
+
+`host`::
+
+	The remote hostname for a network credential.
+
+`path`::
+
+	The path with which the credential will be used. E.g., for
+	accessing a remote https repository, this will be the
+	repository's path on the server.
+
+`username`::
+
+	The credential's username, if we already have one (e.g., from a
+	URL, from the user, or from a previously run helper).
+
+`password`::
+
+	The credential's password, if we are asking it to be stored.
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index adb6f0c..5977b58 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -241,42 +241,9 @@ appended to its command line, which is one of:
 	Remove a matching credential, if any, from the helper's storage.
 
 The details of the credential will be provided on the helper's stdin
-stream. The credential is split into a set of named attributes.
-Attributes are provided to the helper, one per line. Each attribute is
-specified by a key-value pair, separated by an `=` (equals) sign,
-followed by a newline. The key may contain any bytes except `=`,
-newline, or NUL. The value may contain any bytes except newline or NUL.
-In both cases, all bytes are treated as-is (i.e., there is no quoting,
-and one cannot transmit a value with newline or NUL in it). The list of
-attributes is terminated by a blank line or end-of-file.
-
-Git will send the following attributes (but may not send all of
-them for a given credential; for example, a `host` attribute makes no
-sense when dealing with a non-network protocol):
-
-`protocol`::
-
-	The protocol over which the credential will be used (e.g.,
-	`https`).
-
-`host`::
-
-	The remote hostname for a network credential.
-
-`path`::
-
-	The path with which the credential will be used. E.g., for
-	accessing a remote https repository, this will be the
-	repository's path on the server.
-
-`username`::
-
-	The credential's username, if we already have one (e.g., from a
-	URL, from the user, or from a previously run helper).
-
-`password`::
-
-	The credential's password, if we are asking it to be stored.
+stream. The exact format is the same as the input/output format of the
+`git credential` plumbing command (see the section `INPUT/OUTPUT
+FORMAT` in linkgit:git-credential[7] for a detailed specification).
 
 For a `get` operation, the helper should produce a list of attributes
 on stdout in the same format. A helper is free to produce a subset, or
diff --git a/Makefile b/Makefile
index 4592f1f..670366b 100644
--- a/Makefile
+++ b/Makefile
@@ -480,7 +480,6 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
-TEST_PROGRAMS_NEED_X += test-credential
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
@@ -827,6 +826,7 @@ BUILTIN_OBJS += builtin/commit-tree.o
 BUILTIN_OBJS += builtin/commit.o
 BUILTIN_OBJS += builtin/config.o
 BUILTIN_OBJS += builtin/count-objects.o
+BUILTIN_OBJS += builtin/credential.o
 BUILTIN_OBJS += builtin/describe.o
 BUILTIN_OBJS += builtin/diff-files.o
 BUILTIN_OBJS += builtin/diff-index.o
diff --git a/builtin.h b/builtin.h
index 338f540..48feddc 100644
--- a/builtin.h
+++ b/builtin.h
@@ -66,6 +66,7 @@ extern int cmd_commit(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_config(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
+extern int cmd_credential(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_files(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_index(int argc, const char **argv, const char *prefix);
diff --git a/test-credential.c b/builtin/credential.c
similarity index 63%
rename from test-credential.c
rename to builtin/credential.c
index dee200e..4147314 100644
--- a/test-credential.c
+++ b/builtin/credential.c
@@ -1,21 +1,18 @@
-#include "cache.h"
+#include <stdio.h>
 #include "credential.h"
-#include "string-list.h"
+#include "builtin.h"
 
 static const char usage_msg[] =
-"test-credential <fill|approve|reject> [helper...]";
+	"credential <fill|approve|reject>";
 
-int main(int argc, const char **argv)
+int cmd_credential (int argc, const char **argv, const char *prefix)
 {
 	const char *op;
 	struct credential c = CREDENTIAL_INIT;
-	int i;
 
 	op = argv[1];
 	if (!op)
 		usage(usage_msg);
-	for (i = 2; i < argc; i++)
-		string_list_append(&c.helpers, argv[i]);
 
 	if (credential_read(&c, stdin) < 0)
 		die("unable to read credential from stdin");
@@ -26,13 +23,12 @@ int main(int argc, const char **argv)
 			printf("username=%s\n", c.username);
 		if (c.password)
 			printf("password=%s\n", c.password);
-	}
-	else if (!strcmp(op, "approve"))
+	} else if (!strcmp(op, "approve")) {
 		credential_approve(&c);
-	else if (!strcmp(op, "reject"))
+	} else if (!strcmp(op, "reject")) {
 		credential_reject(&c);
-	else
+	} else {
 		usage(usage_msg);
-
+	}
 	return 0;
 }
diff --git a/git.c b/git.c
index d232de9..660c926 100644
--- a/git.c
+++ b/git.c
@@ -353,6 +353,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config, RUN_SETUP_GENTLY },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
+		{ "credential", cmd_credential, RUN_SETUP_GENTLY },
 		{ "describe", cmd_describe, RUN_SETUP },
 		{ "diff", cmd_diff },
 		{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 4a37cd7..7c4826e 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -4,10 +4,20 @@
 # stdout and stderr should be provided on stdin,
 # separated by "--".
 check() {
+	credential_opts=
+	credential_cmd=$1
+	shift
+	for arg in "$@"; do
+		credential_opts="$credential_opts -c credential.helper='$arg'"
+	done
 	read_chunk >stdin &&
 	read_chunk >expect-stdout &&
 	read_chunk >expect-stderr &&
-	test-credential "$@" <stdin >stdout 2>stderr &&
+	if ! eval "git $credential_opts credential $credential_cmd <stdin >stdout 2>stderr"; then
+		echo "git credential failed with code $?" &&
+		cat stderr &&
+		false
+	fi &&
 	test_cmp expect-stdout stdout &&
 	test_cmp expect-stderr stderr
 }
@@ -41,7 +51,7 @@ reject() {
 		echo protocol=$2
 		echo host=$3
 		echo username=$4
-	) | test-credential reject $1
+	) | git -c credential.helper=$1 credential reject
 }
 
 helper_test() {
-- 
1.7.11.5.g0c7e058.dirty

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

* [PATCH 2/3] git credential fill: output the whole 'struct credential'
  2012-06-22 16:07 [PATCH 0/3] git credential plumbing command: implementation and use Matthieu Moy
  2012-06-22 16:07 ` [PATCH 1/3] add 'git credential' plumbing command Matthieu Moy
@ 2012-06-22 16:07 ` Matthieu Moy
  2012-06-22 20:26   ` Junio C Hamano
  2012-06-22 16:07 ` [PATCH 3/3] git-remote-mediawiki: add credential support Matthieu Moy
  2 siblings, 1 reply; 11+ messages in thread
From: Matthieu Moy @ 2012-06-22 16:07 UTC (permalink / raw)
  To: git, gitster; +Cc: Javier.Roucher-Iglesias, Matthieu Moy

Instead of outputing only the username and password, print all the
attributes, even those that already appeared in the input.

This is closer to what the C API does, and allows one to take the exact
output of "git credential fill" as input to "git credential approve" or
"git credential reject".
---
Suggested by Jeff, and will be very nice to use from perl in the next
patch.

 Documentation/git-credential.txt | 15 +++++++++++----
 builtin/credential.c             |  5 +----
 credential.c                     |  2 +-
 credential.h                     |  1 +
 t/lib-credential.sh              | 25 +++++++++++++++++++++++++
 t/t0300-credentials.sh           | 14 ++++++++++++++
 6 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index b64ac30..67737de 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -62,11 +62,18 @@ credential description (don't forget the blank line at the end):
 
   2. Ask git-credential to give us a username and password for this
      description. This is done by running `git credential fill`,
-     feeding the description from step (1) to its stdin. The username
-     and password will be produced on stdout, like:
+     feeding the description from step (1) to its stdin. The complete
+     credential description will be produced on stdout, like:
 
+	protocol=https
+	host=example.com
 	username=bob
 	password=secr3t
++
+In most cases, this means the attributes given in the input will be
+repeated in the output, but git may also modify the credential
+description, for example by removing the `path` attribute when the
+protocol is HTTP(s) and `credential.useHttpPath` is false.
 
   3. Try to use the credential (e.g., by accessing the URL with the
      username and password from step (2)).
@@ -76,8 +83,8 @@ credential description (don't forget the blank line at the end):
      it can be marked with an "approve" action. If the credential was
      rejected during the operation, use the "reject" action. In either
      case, `git credential` should be fed with the credential
-     description obtained from step (2) together with the ones already
-     provided in step (1).
+     description obtained from step (2) (which also contain the ones
+     provided in step (1)).
 
 [[IOFMT]]
 INPUT/OUTPUT FORMAT
diff --git a/builtin/credential.c b/builtin/credential.c
index 4147314..b956059 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -19,10 +19,7 @@ int cmd_credential (int argc, const char **argv, const char *prefix)
 
 	if (!strcmp(op, "fill")) {
 		credential_fill(&c);
-		if (c.username)
-			printf("username=%s\n", c.username);
-		if (c.password)
-			printf("password=%s\n", c.password);
+		credential_write(&c, stdout);
 	} else if (!strcmp(op, "approve")) {
 		credential_approve(&c);
 	} else if (!strcmp(op, "reject")) {
diff --git a/credential.c b/credential.c
index 62d1c56..2c40007 100644
--- a/credential.c
+++ b/credential.c
@@ -191,7 +191,7 @@ static void credential_write_item(FILE *fp, const char *key, const char *value)
 	fprintf(fp, "%s=%s\n", key, value);
 }
 
-static void credential_write(const struct credential *c, FILE *fp)
+void credential_write(const struct credential *c, FILE *fp)
 {
 	credential_write_item(fp, "protocol", c->protocol);
 	credential_write_item(fp, "host", c->host);
diff --git a/credential.h b/credential.h
index 96ea41b..0c3e85e 100644
--- a/credential.h
+++ b/credential.h
@@ -26,6 +26,7 @@ void credential_approve(struct credential *);
 void credential_reject(struct credential *);
 
 int credential_read(struct credential *, FILE *);
+void credential_write(const struct credential *, FILE *);
 void credential_from_url(struct credential *, const char *url);
 int credential_match(const struct credential *have,
 		     const struct credential *want);
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 7c4826e..957ae93 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -62,6 +62,8 @@ helper_test() {
 		protocol=https
 		host=example.com
 		--
+		protocol=https
+		host=example.com
 		username=askpass-username
 		password=askpass-password
 		--
@@ -84,6 +86,8 @@ helper_test() {
 		protocol=https
 		host=example.com
 		--
+		protocol=https
+		host=example.com
 		username=store-user
 		password=store-pass
 		--
@@ -95,6 +99,8 @@ helper_test() {
 		protocol=http
 		host=example.com
 		--
+		protocol=http
+		host=example.com
 		username=askpass-username
 		password=askpass-password
 		--
@@ -108,6 +114,8 @@ helper_test() {
 		protocol=https
 		host=other.tld
 		--
+		protocol=https
+		host=other.tld
 		username=askpass-username
 		password=askpass-password
 		--
@@ -122,6 +130,8 @@ helper_test() {
 		host=example.com
 		username=other
 		--
+		protocol=https
+		host=example.com
 		username=other
 		password=askpass-password
 		--
@@ -143,6 +153,9 @@ helper_test() {
 		host=path.tld
 		path=bar.git
 		--
+		protocol=http
+		host=path.tld
+		path=bar.git
 		username=askpass-username
 		password=askpass-password
 		--
@@ -160,6 +173,8 @@ helper_test() {
 		protocol=https
 		host=example.com
 		--
+		protocol=https
+		host=example.com
 		username=askpass-username
 		password=askpass-password
 		--
@@ -186,6 +201,8 @@ helper_test() {
 		host=example.com
 		username=user1
 		--
+		protocol=https
+		host=example.com
 		username=user1
 		password=pass1
 		EOF
@@ -194,6 +211,8 @@ helper_test() {
 		host=example.com
 		username=user2
 		--
+		protocol=https
+		host=example.com
 		username=user2
 		password=pass2
 		EOF
@@ -210,6 +229,8 @@ helper_test() {
 		host=example.com
 		username=user1
 		--
+		protocol=https
+		host=example.com
 		username=user1
 		password=askpass-password
 		--
@@ -223,6 +244,8 @@ helper_test() {
 		host=example.com
 		username=user2
 		--
+		protocol=https
+		host=example.com
 		username=user2
 		password=pass2
 		EOF
@@ -244,6 +267,8 @@ helper_test_timeout() {
 		protocol=https
 		host=timeout.tld
 		--
+		protocol=https
+		host=timeout.tld
 		username=askpass-username
 		password=askpass-password
 		--
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 20e28e3..538ea5f 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -82,6 +82,9 @@ test_expect_success 'credential_fill passes along metadata' '
 	host=example.com
 	path=foo.git
 	--
+	protocol=ftp
+	host=example.com
+	path=foo.git
 	username=one
 	password=two
 	--
@@ -213,6 +216,8 @@ test_expect_success 'match configured credential' '
 	host=example.com
 	path=repo.git
 	--
+	protocol=https
+	host=example.com
 	username=foo
 	password=bar
 	--
@@ -225,6 +230,8 @@ test_expect_success 'do not match configured credential' '
 	protocol=https
 	host=bar
 	--
+	protocol=https
+	host=bar
 	username=askpass-username
 	password=askpass-password
 	--
@@ -239,6 +246,8 @@ test_expect_success 'pull username from config' '
 	protocol=https
 	host=example.com
 	--
+	protocol=https
+	host=example.com
 	username=foo
 	password=askpass-password
 	--
@@ -252,6 +261,8 @@ test_expect_success 'http paths can be part of context' '
 	host=example.com
 	path=foo.git
 	--
+	protocol=https
+	host=example.com
 	username=foo
 	password=bar
 	--
@@ -265,6 +276,9 @@ test_expect_success 'http paths can be part of context' '
 	host=example.com
 	path=foo.git
 	--
+	protocol=https
+	host=example.com
+	path=foo.git
 	username=foo
 	password=bar
 	--
-- 
1.7.11.5.g0c7e058.dirty

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

* [PATCH 3/3] git-remote-mediawiki: add credential support
  2012-06-22 16:07 [PATCH 0/3] git credential plumbing command: implementation and use Matthieu Moy
  2012-06-22 16:07 ` [PATCH 1/3] add 'git credential' plumbing command Matthieu Moy
  2012-06-22 16:07 ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
@ 2012-06-22 16:07 ` Matthieu Moy
  2 siblings, 0 replies; 11+ messages in thread
From: Matthieu Moy @ 2012-06-22 16:07 UTC (permalink / raw)
  To: git, gitster; +Cc: Javier.Roucher-Iglesias, Matthieu Moy

The previous version implemented the possibility to log in a wiki, but
the username and password had to be provided as configuration variables.
We add the possibility to use the Git credential system to prompt
the password.

The support if implemented with generic functions that mimic the C API,
designed to be usable from other contexts in the future (i.e. they may
migrate to Git.pm if someone is interested).

While we're there, do a bit of refactoring in mw_connect_maybe.

Based on patch by: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
---
 contrib/mw-to-git/git-remote-mediawiki | 106 +++++++++++++++++++++++++++++----
 1 file changed, 94 insertions(+), 12 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki
index c18bfa1..a34f53f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki
+++ b/contrib/mw-to-git/git-remote-mediawiki
@@ -43,6 +43,8 @@ use encoding 'utf8';
 binmode STDERR, ":utf8";
 
 use URI::Escape;
+use IPC::Open2;
+
 use warnings;
 
 # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced
@@ -151,28 +153,108 @@ while (<STDIN>) {
 
 ########################## Functions ##############################
 
+## credential API management (generic functions)
+
+sub credential_from_url {
+	my $url = shift;
+	my $parsed = URI->new($url);
+	my %credential;
+
+	if ($parsed->scheme) {
+		$credential{protocol} = $parsed->scheme;
+	}
+	if ($parsed->host) {
+		$credential{host} = $parsed->host;
+	}
+	if ($parsed->path) {
+		$credential{path} = $parsed->path;
+	}
+	if ($parsed->userinfo) {
+		if ($parsed->userinfo =~ /([^:]*):(.*)/) {
+			$credential{username} = $1;
+			$credential{password} = $2;
+		} else {
+			$credential{username} = $parsed->userinfo;
+		}
+	}
+
+	return %credential;
+}
+
+sub credential_read {
+	my %credential;
+	my $reader = shift;
+	my $op = shift;
+	while (<$reader>) {
+		my ($key, $value) = /([^=]*)=(.*)/;
+		if (not defined $key) {
+			die "ERROR receiving response from git credential $op:\n$_\n";
+		}
+		$credential{$key} = $value;
+	}
+	return %credential;
+}
+
+sub credential_write {
+	my $credential = shift;
+	my $writer = shift;
+	while (my ($key, $value) = each(%$credential) ) {
+		if ($value) {
+			print $writer "$key=$value\n";
+		}
+	}
+}
+
+sub credential_run {
+	my $op = shift;
+	my $credential = shift;
+	my $pid = open2(my $reader, my $writer, "git credential $op");
+	credential_write($credential, $writer);
+	print $writer "\n";
+	close($writer);
+	
+	if ($op eq "fill") {
+		%$credential = credential_read($reader, $op);
+	} else {
+		if (<$reader>) {
+			die "ERROR while running git credential $op:\n$_";
+		}
+	}
+	close($reader);
+	waitpid($pid, 0);
+	my $child_exit_status = $? >> 8;
+	if ($child_exit_status != 0) {
+		die "'git credential $op' failed with code $child_exit_status.";
+	}
+}
+
 # MediaWiki API instance, created lazily.
 my $mediawiki;
 
 sub mw_connect_maybe {
 	if ($mediawiki) {
-	    return;
+		return;
 	}
 	$mediawiki = MediaWiki::API->new;
 	$mediawiki->{config}->{api_url} = "$url/api.php";
 	if ($wiki_login) {
-		if (!$mediawiki->login({
-			lgname => $wiki_login,
-			lgpassword => $wiki_passwd,
-			lgdomain => $wiki_domain,
-		})) {
-			print STDERR "Failed to log in mediawiki user \"$wiki_login\" on $url\n";
-			print STDERR "(error " .
-			    $mediawiki->{error}->{code} . ': ' .
-			    $mediawiki->{error}->{details} . ")\n";
-			exit 1;
+		my %credential = credential_from_url($url);
+		$credential{username} = $wiki_login;
+		$credential{password} = $wiki_passwd;
+		credential_run("fill", \%credential);
+		my $request = {lgname => $credential{username},
+			       lgpassword => $credential{password},
+			       lgdomain => $wiki_domain};
+		if ($mediawiki->login($request)) {
+			credential_run("approve", \%credential);
+			print STDERR "Logged in mediawiki user \"$credential{username}\".\n";
 		} else {
-			print STDERR "Logged in with user \"$wiki_login\".\n";
+			print STDERR "Failed to log in mediawiki user \"$credential{username}\" on $url\n";
+			print STDERR "  (error " .
+				$mediawiki->{error}->{code} . ': ' .
+				$mediawiki->{error}->{details} . ")\n";
+			credential_run("reject", \%credential);
+			exit 1;
 		}
 	}
 }
-- 
1.7.11.5.g0c7e058.dirty

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

* Re: [PATCH 1/3] add 'git credential' plumbing command
  2012-06-22 16:07 ` [PATCH 1/3] add 'git credential' plumbing command Matthieu Moy
@ 2012-06-22 20:24   ` Junio C Hamano
  2012-06-24 11:37     ` Matthieu Moy
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2012-06-22 20:24 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Javier.Roucher-Iglesias, Pavel Volek, Kim Thuat Nguyen

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> From: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
>
> The credential API is in C, and not available to scripting languages.
> Expose the functionalities of the API by wrapping them into a new
> plumbing command "git credentials".
>
> In other words, replace the internal "test-credential" by an official Git
> command.
>
> Most documentation writen by: Jeff King <peff@peff.net>
> Signed-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>
> Signed-off-by: Kim Thuat Nguyen <Kim-Thuat.Nguyen@ensimag.imag.fr>
> Signed-off-by: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  .gitignore                                  |   2 +-
>  Documentation/git-credential.txt            | 126 ++++++++++++++++++++++++++++
>  Documentation/technical/api-credentials.txt |  39 +--------
>  Makefile                                    |   2 +-
>  builtin.h                                   |   1 +
>  test-credential.c => builtin/credential.c   |  20 ++---

Nice ;-)

> diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
> new file mode 100644
> index 0000000..b64ac30
> --- /dev/null
> +++ b/Documentation/git-credential.txt
> @@ -0,0 +1,126 @@
> ...
> +DESCRIPTION
> +-----------
> +...

Very clearly written.

> +TYPICAL WORKFLOW
> +----------------
> +
> +An application using git-credential will typically follow this
> +workflow:

Again, who does what in what order is very clearly described below.

One minor question I had was "is this _workflow_?".  "An application
using ... typically follow this workflow:" might want to be reworded
to "An application uses `git credential` like this:" or something like
that.

I am not sure about the section title, though.

> +  1. Generate a credential description based on the context.
> ++
> +For example, if we want a password for
> +`https://example.com/foo.git`, we might generate the following
> +credential description (don't forget the blank line at the end):

Add "; it tells `git credential` that the application finished feeding
all the infomation it has" or something after "blank line at the
end" to justify why the user must not forget it.

> +
> +         protocol=https
> +         host=example.com
> +         path=foo.git

I also thought that we discussed optionally removing the burden of
parsing the incoming URL (e.g. https://exmaple.com/foo.git) into its
components by giving them a way to feed a single line

	url=https://example.com/foo.git

in place of the above three?  Perhaps it will come as an enhancement
in a later patch in the series?

> +  2. Ask git-credential to give us a username and password for this
> +     description. This is done by running `git credential fill`,
> +     feeding the description from step (1) to its stdin. The username
> +     and password will be produced on stdout, like:
> +
> +	username=bob
> +	password=secr3t

I may be ahead of myself, but the following untold paragraph came to
my mind while I was reading this:

    If the "git credential" knew about the password, this step may
    not have involved the user actually typing this password (the
    user may have typed a password to unlock the keychain instead,
    or no user interaction was done if the keychain was already
    unlocked) before it returned "password=secr3t".

> +  3. Try to use the credential (e.g., by accessing the URL with the
> +     username and password from step (2)).

OK.  Drop "Try to use" and just say "Use"; it's clearer.  Similarly,
s/by accessing/access/.

> +  4. Report on the success or failure of the password. If the
> +     credential allowed the operation to complete successfully, then
> +     it can be marked with an "approve" action. If the credential was
> +     rejected during the operation, use the "reject" action. In either
> +     case, `git credential` should be fed with the credential
> +     description obtained from step (2) together with the ones already
> +     provided in step (1).

A question that came to my mind after the first sentence was "Why
should I report it?  What benefit am I getting, if I successfully
accessed the resource I wanted to in the previous step already?"

    4. Tell the git credential if the credential was good, so that
       git credential can ask the user the correct password upon the
       next invocation, if the one returned in step 2. did not work
       (e.g. a bad password came from a keychain), by using "approve"
       to tell it that it was good, or "reject" to tell it that it
       was bad.  In either case, ...

The term "credential" is used here without definition (I think you
meant a <username, password> pair with the word).  I think it makes
the description shorter to say "credential" instead of "username and
password", but then we would want to define the term upfront and use
it throughout the document.  E.g. the end of step 2. would read like
this if we did so:

	... to its standard input.  The credential will be produced
	on the standard output, like so:

		username=bob
                password=secr3t

Oh, another thing.  Please avoid using "stdin", etc., when you are
not discussing variables in the program text but you are referring
to them as mechanism names, and instead spell it out.

> +[[IOFMT]]
> +INPUT/OUTPUT FORMAT
> +-------------------
> +
> +`git credential` reads and/or writes (depending on the action used)
> +credential information in its standard input/output. These information
> +can correspond either to keys from which `git credential` will obtain
> +the login/password information (e.g. host, protocol, path), or to the
> +actual credential data to be obtained (login/password).

Shouldn't "keys from which ..." be "keys for which ..."?  You ask
for a data item from somebody who has the data _for_ a key, or you
query a data item from a database _with_ a key.  The data source
could be human in this context, so I'd prefer not get my brain
queried with a key (asking me politely for data for a key is OK ;-).

> +The credential is split into a set of named attributes.
> +Attributes are provided to the helper, one per line. Each attribute is
> ...

I didn't have any issue with the remainder of the paragraph.

> diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
> index adb6f0c..5977b58 100644
> --- a/Documentation/technical/api-credentials.txt
> +++ b/Documentation/technical/api-credentials.txt
> @@ -241,42 +241,9 @@ appended to its command line, which is one of:
>  	Remove a matching credential, if any, from the helper's storage.
>  
>  The details of the credential will be provided on the helper's stdin
> +stream. The exact format is the same as the input/output format of the
> +`git credential` plumbing command (see the section `INPUT/OUTPUT
> +FORMAT` in linkgit:git-credential[7] for a detailed specification).

OK.

> diff --git a/test-credential.c b/builtin/credential.c
> similarity index 63%
> rename from test-credential.c
> rename to builtin/credential.c
> index dee200e..4147314 100644
> --- a/test-credential.c
> +++ b/builtin/credential.c
> @@ -1,21 +1,18 @@
> -#include "cache.h"
> +#include <stdio.h>
>  #include "credential.h"
> -#include "string-list.h"
> +#include "builtin.h"

I understand that you do not want the entire cache.h, but could you
include "git-compat-util.h" instead?  We try to absorb platform
dependencies in that header file and avoid including system headers
directly to C sources.

>  static const char usage_msg[] =
> -"test-credential <fill|approve|reject> [helper...]";
> +	"credential <fill|approve|reject>";
>  
> -int main(int argc, const char **argv)
> +int cmd_credential (int argc, const char **argv, const char *prefix)

Style.

> diff --git a/git.c b/git.c
> index d232de9..660c926 100644
> --- a/git.c
> +++ b/git.c
> @@ -353,6 +353,7 @@ static void handle_internal_command(int argc, const char **argv)
>  		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
>  		{ "config", cmd_config, RUN_SETUP_GENTLY },
>  		{ "count-objects", cmd_count_objects, RUN_SETUP },
> +		{ "credential", cmd_credential, RUN_SETUP_GENTLY },

Good.

> diff --git a/t/lib-credential.sh b/t/lib-credential.sh
> index 4a37cd7..7c4826e 100755
> --- a/t/lib-credential.sh
> +++ b/t/lib-credential.sh
> @@ -4,10 +4,20 @@
>  # stdout and stderr should be provided on stdin,
>  # separated by "--".
>  check() {
> +	credential_opts=
> +	credential_cmd=$1
> +	shift
> +	for arg in "$@"; do
> +		credential_opts="$credential_opts -c credential.helper='$arg'"
> +	done
>  	read_chunk >stdin &&
>  	read_chunk >expect-stdout &&
>  	read_chunk >expect-stderr &&
> -	test-credential "$@" <stdin >stdout 2>stderr &&
> +	if ! eval "git $credential_opts credential $credential_cmd <stdin >stdout 2>stderr"; then
> +		echo "git credential failed with code $?" &&
> +		cat stderr &&
> +		false
> +	fi &&
>  	test_cmp expect-stdout stdout &&
>  	test_cmp expect-stderr stderr
>  }
> @@ -41,7 +51,7 @@ reject() {
>  		echo protocol=$2
>  		echo host=$3
>  		echo username=$4
> -	) | test-credential reject $1
> +	) | git -c credential.helper=$1 credential reject
>  }
>  
>  helper_test() {

Nice.

Thanks.

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

* Re: [PATCH 2/3] git credential fill: output the whole 'struct credential'
  2012-06-22 16:07 ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
@ 2012-06-22 20:26   ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2012-06-22 20:26 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Javier.Roucher-Iglesias

Looks very straightforward and clean.  Thanks.

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

* Re: [PATCH 1/3] add 'git credential' plumbing command
  2012-06-22 20:24   ` Junio C Hamano
@ 2012-06-24 11:37     ` Matthieu Moy
  2012-06-24 11:39       ` Matthieu Moy
  0 siblings, 1 reply; 11+ messages in thread
From: Matthieu Moy @ 2012-06-24 11:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Javier.Roucher-Iglesias, Pavel Volek, Kim Thuat Nguyen,
	Jeff King

[ Ccing Peff. I should have done it when sending the patch ]

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

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> From: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
>>
>> The credential API is in C, and not available to scripting languages.
>> Expose the functionalities of the API by wrapping them into a new
>> plumbing command "git credentials".
>>
>> In other words, replace the internal "test-credential" by an official Git
>> command.
>>
>> Most documentation writen by: Jeff King <peff@peff.net>
>> Signed-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>
>> Signed-off-by: Kim Thuat Nguyen <Kim-Thuat.Nguyen@ensimag.imag.fr>
>> Signed-off-by: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>>  .gitignore                                  |   2 +-
>>  Documentation/git-credential.txt            | 126 ++++++++++++++++++++++++++++
>>  Documentation/technical/api-credentials.txt |  39 +--------
>>  Makefile                                    |   2 +-
>>  builtin.h                                   |   1 +
>>  test-credential.c => builtin/credential.c   |  20 ++---
>
> Nice ;-)
>
>> diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
>> new file mode 100644
>> index 0000000..b64ac30
>> --- /dev/null
>> +++ b/Documentation/git-credential.txt
>> @@ -0,0 +1,126 @@
>> ...
>> +DESCRIPTION
>> +-----------
>> +...
>
> Very clearly written.

... Thanks to Peff ;-).

>> +TYPICAL WORKFLOW
>> +----------------
>> +
>> +An application using git-credential will typically follow this
>> +workflow:
>
> Again, who does what in what order is very clearly described below.
>
> One minor question I had was "is this _workflow_?".  "An application
> using ... typically follow this workflow:" might want to be reworded
> to "An application uses `git credential` like this:" or something like
> that.

I made that "will typically use `git credential` following these steps:

> I am not sure about the section title, though.

I changed it to "TYPICAL USE OF GIT CREDENTIAL".

>> +  1. Generate a credential description based on the context.
>> ++
>> +For example, if we want a password for
>> +`https://example.com/foo.git`, we might generate the following
>> +credential description (don't forget the blank line at the end):
>
> Add "; it tells `git credential` that the application finished feeding
> all the infomation it has" or something after "blank line at the
> end" to justify why the user must not forget it.

Done.

>> +
>> +         protocol=https
>> +         host=example.com
>> +         path=foo.git
>
> I also thought that we discussed optionally removing the burden of
> parsing the incoming URL (e.g. https://exmaple.com/foo.git) into its
> components by giving them a way to feed a single line
>
> 	url=https://example.com/foo.git
>
> in place of the above three?  Perhaps it will come as an enhancement
> in a later patch in the series?

Not in this serie. The conclusion was that it would be easy to add
afterwards if needed. Actually, I think it would make sense to add a new
command "git credential parse-url" or so, that would read a credential
description on stdin and output its canonical form on stdin. That would
allow

$ echo 'http://foo.com' | git credential parse-url
protocol=http
host=foo.com
...

and that would allow the caller to sort-of call credential_from_url
(in credential.c) from another language.

>> +  2. Ask git-credential to give us a username and password for this
>> +     description. This is done by running `git credential fill`,
>> +     feeding the description from step (1) to its stdin. The username
>> +     and password will be produced on stdout, like:
>> +
>> +	username=bob
>> +	password=secr3t
>
> I may be ahead of myself, but the following untold paragraph came to
> my mind while I was reading this:
>
>     If the "git credential" knew about the password, this step may
>     not have involved the user actually typing this password (the
>     user may have typed a password to unlock the keychain instead,
>     or no user interaction was done if the keychain was already
>     unlocked) before it returned "password=secr3t".

Added.

>> +  3. Try to use the credential (e.g., by accessing the URL with the
>> +     username and password from step (2)).
>
> OK.  Drop "Try to use" and just say "Use"; it's clearer.  Similarly,
> s/by accessing/access/.

I took that, but added "and see if it's accepted." at the end of the
sentense to insist on the fact that "git credential" is not the one who
knows whether the authentication failed or not.

>> +  4. Report on the success or failure of the password. If the
>> +     credential allowed the operation to complete successfully, then
>> +     it can be marked with an "approve" action. If the credential was
>> +     rejected during the operation, use the "reject" action. In either
>> +     case, `git credential` should be fed with the credential
>> +     description obtained from step (2) together with the ones already
>> +     provided in step (1).
>
> A question that came to my mind after the first sentence was "Why
> should I report it?  What benefit am I getting, if I successfully
> accessed the resource I wanted to in the previous step already?"
>
>     4. Tell the git credential if the credential was good, so that
>        git credential can ask the user the correct password upon the
>        next invocation, if the one returned in step 2. did not work
>        (e.g. a bad password came from a keychain), by using "approve"
>        to tell it that it was good, or "reject" to tell it that it
>        was bad.  In either case, ...

I like the idea, but I think your wording has an overly long sentence. I
grabbed some pieces and inserted them into the existing paragraph.

> The term "credential" is used here without definition (I think you
> meant a <username, password> pair with the word).  I think it makes
> the description shorter to say "credential" instead of "username and
> password", but then we would want to define the term upfront and use
> it throughout the document.  E.g. the end of step 2. would read like
> this if we did so:
>
> 	... to its standard input.  The credential will be produced
> 	on the standard output, like so:
>
> 		username=bob
>                 password=secr3t
>
> Oh, another thing.  Please avoid using "stdin", etc., when you are
> not discussing variables in the program text but you are referring
> to them as mechanism names, and instead spell it out.

Done, but the next patch is changing that to "The complete credential
description" anyway. There's a little ambiguity between "credential" ==
<username,password> and "credential description" == struct credential ==
the whole set of attributes, so patch 2 does now

     The complete credential description (including the credential per
     se, i.e. the login and password) will be produced on standard
     output

>> +[[IOFMT]]
>> +INPUT/OUTPUT FORMAT
>> +-------------------
>> +
>> +`git credential` reads and/or writes (depending on the action used)
>> +credential information in its standard input/output. These information
>> +can correspond either to keys from which `git credential` will obtain
>> +the login/password information (e.g. host, protocol, path), or to the
>> +actual credential data to be obtained (login/password).
>
> Shouldn't "keys from which ..." be "keys for which ..."?  You ask
> for a data item from somebody who has the data _for_ a key, or you
> query a data item from a database _with_ a key.  The data source
> could be human in this context, so I'd prefer not get my brain
> queried with a key (asking me politely for data for a key is OK ;-).

I guess your english is better than mine, so I take this ;-).

>> diff --git a/test-credential.c b/builtin/credential.c
>> similarity index 63%
>> rename from test-credential.c
>> rename to builtin/credential.c
>> index dee200e..4147314 100644
>> --- a/test-credential.c
>> +++ b/builtin/credential.c
>> @@ -1,21 +1,18 @@
>> -#include "cache.h"
>> +#include <stdio.h>
>>  #include "credential.h"
>> -#include "string-list.h"
>> +#include "builtin.h"
>
> I understand that you do not want the entire cache.h, but could you
> include "git-compat-util.h" instead?  We try to absorb platform
> dependencies in that header file and avoid including system headers
> directly to C sources.

Done.

>>  static const char usage_msg[] =
>> -"test-credential <fill|approve|reject> [helper...]";
>> +	"credential <fill|approve|reject>";
>>  
>> -int main(int argc, const char **argv)
>> +int cmd_credential (int argc, const char **argv, const char *prefix)
>
> Style.

Removed space before "(".

In summary, I'm squashing this into the patch:

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index b64ac30..d5228a3 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -44,17 +44,19 @@ credential matching the description.
 
 If the action is `approve` or `reject`, no output should be emitted.
 
-TYPICAL WORKFLOW
-----------------
+TYPICAL USE OF GIT CREDENTIAL
+-----------------------------
 
-An application using git-credential will typically follow this
-workflow:
+An application using git-credential will typically use `git
+credential` following these steps:
 
   1. Generate a credential description based on the context.
 +
 For example, if we want a password for
 `https://example.com/foo.git`, we might generate the following
-credential description (don't forget the blank line at the end):
+credential description (don't forget the blank line at the end; it
+tells `git credential` that the application finished feeding all the
+infomation it has):
 
          protocol=https
          host=example.com
@@ -62,22 +64,30 @@ credential description (don't forget the blank line at the end):
 
   2. Ask git-credential to give us a username and password for this
      description. This is done by running `git credential fill`,
-     feeding the description from step (1) to its stdin. The username
-     and password will be produced on stdout, like:
+     feeding the description from step (1) to its standard input. The
+     credential will be produced on standard output, like:
 
 	username=bob
 	password=secr3t
++
+If the `git credential` knew about the password, this step may
+not have involved the user actually typing this password (the
+user may have typed a password to unlock the keychain instead,
+or no user interaction was done if the keychain was already
+unlocked) before it returned `password=secr3t`.
 
-  3. Try to use the credential (e.g., by accessing the URL with the
-     username and password from step (2)).
+  3. Use the credential (e.g., access the URL with the username and
+     password from step (2)), and see if it's accepted.
 
   4. Report on the success or failure of the password. If the
      credential allowed the operation to complete successfully, then
-     it can be marked with an "approve" action. If the credential was
-     rejected during the operation, use the "reject" action. In either
-     case, `git credential` should be fed with the credential
-     description obtained from step (2) together with the ones already
-     provided in step (1).
+     it can be marked with an "approve" action to tell `git
+     credential` to reuse it in its next invocation. If the credential
+     was rejected during the operation, use the "reject" action so
+     that `git credential` will ask for a new password in its next
+     invocation. In either case, `git credential` should be fed with
+     the credential description obtained from step (2) together with
+     the ones already provided in step (1).
 
 [[IOFMT]]
 INPUT/OUTPUT FORMAT
@@ -85,7 +95,7 @@ INPUT/OUTPUT FORMAT
 
 `git credential` reads and/or writes (depending on the action used)
 credential information in its standard input/output. These information
-can correspond either to keys from which `git credential` will obtain
+can correspond either to keys for which `git credential` will obtain
 the login/password information (e.g. host, protocol, path), or to the
 actual credential data to be obtained (login/password).
 
diff --git a/builtin/credential.c b/builtin/credential.c
index 4147314..c185c07 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -1,11 +1,11 @@
-#include <stdio.h>
+#include "git-compat-util.h"
 #include "credential.h"
 #include "builtin.h"
 
 static const char usage_msg[] =
-	"credential <fill|approve|reject>";
+	"git credential [fill|approve|reject]";
 
-int cmd_credential (int argc, const char **argv, const char *prefix)
+int cmd_credential(int argc, const char **argv, const char *prefix)
 {
 	const char *op;
 	struct credential c = CREDENTIAL_INIT;


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

* [PATCH 1/3] add 'git credential' plumbing command
  2012-06-24 11:37     ` Matthieu Moy
@ 2012-06-24 11:39       ` Matthieu Moy
  2012-06-24 11:40         ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
                           ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Matthieu Moy @ 2012-06-24 11:39 UTC (permalink / raw)
  To: git, gitster
  Cc: peff, Javier Roucher Iglesias, Pavel Volek, Kim Thuat Nguyen,
	Matthieu Moy

From: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>

The credential API is in C, and not available to scripting languages.
Expose the functionalities of the API by wrapping them into a new
plumbing command "git credentials".

In other words, replace the internal "test-credential" by an official Git
command.

Most documentation writen by: Jeff King <peff@peff.net>
Signed-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>
Signed-off-by: Kim Thuat Nguyen <Kim-Thuat.Nguyen@ensimag.imag.fr>
Signed-off-by: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 .gitignore                                  |   2 +-
 Documentation/git-credential.txt            | 136 ++++++++++++++++++++++++++++
 Documentation/technical/api-credentials.txt |  39 +-------
 Makefile                                    |   2 +-
 builtin.h                                   |   1 +
 test-credential.c => builtin/credential.c   |  20 ++--
 git.c                                       |   1 +
 t/lib-credential.sh                         |  14 ++-
 8 files changed, 163 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/git-credential.txt
 rename test-credential.c => builtin/credential.c (63%)

diff --git a/.gitignore b/.gitignore
index bf66648..c188d0b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,6 +31,7 @@
 /git-commit-tree
 /git-config
 /git-count-objects
+/git-credential
 /git-credential-cache
 /git-credential-cache--daemon
 /git-credential-store
@@ -172,7 +173,6 @@
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
 /test-chmtime
-/test-credential
 /test-ctype
 /test-date
 /test-delta
diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
new file mode 100644
index 0000000..d5228a3
--- /dev/null
+++ b/Documentation/git-credential.txt
@@ -0,0 +1,136 @@
+git-credential(7)
+=================
+
+NAME
+----
+git-credential - retrieve and store user credentials
+
+SYNOPSIS
+--------
+------------------
+git credential <fill|approve|reject>
+------------------
+
+DESCRIPTION
+-----------
+
+Git has an internal interface for storing and retrieving credentials
+from system-specific helpers, as well as prompting the user for
+usernames and passwords. The git-credential command exposes this
+interface to scripts which may want to retrieve, store, or prompt for
+credentials in the same manner as git. The design of this scriptable
+interface models the internal C API; see
+link:technical/api-credentials.txt[the git credential API] for more
+background on the concepts.
+
+git-credential takes an "action" option on the command-line (one of
+`fill`, `approve`, or `reject`) and reads a credential description
+on stdin (see <<IOFMT,INPUT/OUTPUT FORMAT>>).
+
+If the action is `fill`, git-credential will attempt to add "username"
+and "password" attributes to the description by reading config files,
+by contacting any configured credential helpers, or by prompting the
+user. The username and password attributes of the credential
+description are then printed to stdout together with the attributes
+already provided.
+
+If the action is `approve`, git-credential will send the description
+to any configured credential helpers, which may store the credential
+for later use. 
+
+If the action is `reject`, git-credential will send the description to
+any configured credential helpers, which may erase any stored
+credential matching the description.
+
+If the action is `approve` or `reject`, no output should be emitted.
+
+TYPICAL USE OF GIT CREDENTIAL
+-----------------------------
+
+An application using git-credential will typically use `git
+credential` following these steps:
+
+  1. Generate a credential description based on the context.
++
+For example, if we want a password for
+`https://example.com/foo.git`, we might generate the following
+credential description (don't forget the blank line at the end; it
+tells `git credential` that the application finished feeding all the
+infomation it has):
+
+         protocol=https
+         host=example.com
+         path=foo.git
+
+  2. Ask git-credential to give us a username and password for this
+     description. This is done by running `git credential fill`,
+     feeding the description from step (1) to its standard input. The
+     credential will be produced on standard output, like:
+
+	username=bob
+	password=secr3t
++
+If the `git credential` knew about the password, this step may
+not have involved the user actually typing this password (the
+user may have typed a password to unlock the keychain instead,
+or no user interaction was done if the keychain was already
+unlocked) before it returned `password=secr3t`.
+
+  3. Use the credential (e.g., access the URL with the username and
+     password from step (2)), and see if it's accepted.
+
+  4. Report on the success or failure of the password. If the
+     credential allowed the operation to complete successfully, then
+     it can be marked with an "approve" action to tell `git
+     credential` to reuse it in its next invocation. If the credential
+     was rejected during the operation, use the "reject" action so
+     that `git credential` will ask for a new password in its next
+     invocation. In either case, `git credential` should be fed with
+     the credential description obtained from step (2) together with
+     the ones already provided in step (1).
+
+[[IOFMT]]
+INPUT/OUTPUT FORMAT
+-------------------
+
+`git credential` reads and/or writes (depending on the action used)
+credential information in its standard input/output. These information
+can correspond either to keys for which `git credential` will obtain
+the login/password information (e.g. host, protocol, path), or to the
+actual credential data to be obtained (login/password).
+
+The credential is split into a set of named attributes.
+Attributes are provided to the helper, one per line. Each attribute is
+specified by a key-value pair, separated by an `=` (equals) sign,
+followed by a newline. The key may contain any bytes except `=`,
+newline, or NUL. The value may contain any bytes except newline or NUL.
+In both cases, all bytes are treated as-is (i.e., there is no quoting,
+and one cannot transmit a value with newline or NUL in it). The list of
+attributes is terminated by a blank line or end-of-file.
+Git will send the following attributes (but may not send all of
+them for a given credential; for example, a `host` attribute makes no
+sense when dealing with a non-network protocol):
+
+`protocol`::
+
+	The protocol over which the credential will be used (e.g.,
+	`https`).
+
+`host`::
+
+	The remote hostname for a network credential.
+
+`path`::
+
+	The path with which the credential will be used. E.g., for
+	accessing a remote https repository, this will be the
+	repository's path on the server.
+
+`username`::
+
+	The credential's username, if we already have one (e.g., from a
+	URL, from the user, or from a previously run helper).
+
+`password`::
+
+	The credential's password, if we are asking it to be stored.
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index adb6f0c..5977b58 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -241,42 +241,9 @@ appended to its command line, which is one of:
 	Remove a matching credential, if any, from the helper's storage.
 
 The details of the credential will be provided on the helper's stdin
-stream. The credential is split into a set of named attributes.
-Attributes are provided to the helper, one per line. Each attribute is
-specified by a key-value pair, separated by an `=` (equals) sign,
-followed by a newline. The key may contain any bytes except `=`,
-newline, or NUL. The value may contain any bytes except newline or NUL.
-In both cases, all bytes are treated as-is (i.e., there is no quoting,
-and one cannot transmit a value with newline or NUL in it). The list of
-attributes is terminated by a blank line or end-of-file.
-
-Git will send the following attributes (but may not send all of
-them for a given credential; for example, a `host` attribute makes no
-sense when dealing with a non-network protocol):
-
-`protocol`::
-
-	The protocol over which the credential will be used (e.g.,
-	`https`).
-
-`host`::
-
-	The remote hostname for a network credential.
-
-`path`::
-
-	The path with which the credential will be used. E.g., for
-	accessing a remote https repository, this will be the
-	repository's path on the server.
-
-`username`::
-
-	The credential's username, if we already have one (e.g., from a
-	URL, from the user, or from a previously run helper).
-
-`password`::
-
-	The credential's password, if we are asking it to be stored.
+stream. The exact format is the same as the input/output format of the
+`git credential` plumbing command (see the section `INPUT/OUTPUT
+FORMAT` in linkgit:git-credential[7] for a detailed specification).
 
 For a `get` operation, the helper should produce a list of attributes
 on stdout in the same format. A helper is free to produce a subset, or
diff --git a/Makefile b/Makefile
index f62ca2a..d5d46d9 100644
--- a/Makefile
+++ b/Makefile
@@ -487,7 +487,6 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
-TEST_PROGRAMS_NEED_X += test-credential
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
@@ -835,6 +834,7 @@ BUILTIN_OBJS += builtin/commit-tree.o
 BUILTIN_OBJS += builtin/commit.o
 BUILTIN_OBJS += builtin/config.o
 BUILTIN_OBJS += builtin/count-objects.o
+BUILTIN_OBJS += builtin/credential.o
 BUILTIN_OBJS += builtin/describe.o
 BUILTIN_OBJS += builtin/diff-files.o
 BUILTIN_OBJS += builtin/diff-index.o
diff --git a/builtin.h b/builtin.h
index dea1643..a945a4a 100644
--- a/builtin.h
+++ b/builtin.h
@@ -65,6 +65,7 @@ extern int cmd_commit(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_config(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
+extern int cmd_credential(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_files(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_index(int argc, const char **argv, const char *prefix);
diff --git a/test-credential.c b/builtin/credential.c
similarity index 63%
rename from test-credential.c
rename to builtin/credential.c
index dee200e..c185c07 100644
--- a/test-credential.c
+++ b/builtin/credential.c
@@ -1,21 +1,18 @@
-#include "cache.h"
+#include "git-compat-util.h"
 #include "credential.h"
-#include "string-list.h"
+#include "builtin.h"
 
 static const char usage_msg[] =
-"test-credential <fill|approve|reject> [helper...]";
+	"git credential [fill|approve|reject]";
 
-int main(int argc, const char **argv)
+int cmd_credential(int argc, const char **argv, const char *prefix)
 {
 	const char *op;
 	struct credential c = CREDENTIAL_INIT;
-	int i;
 
 	op = argv[1];
 	if (!op)
 		usage(usage_msg);
-	for (i = 2; i < argc; i++)
-		string_list_append(&c.helpers, argv[i]);
 
 	if (credential_read(&c, stdin) < 0)
 		die("unable to read credential from stdin");
@@ -26,13 +23,12 @@ int main(int argc, const char **argv)
 			printf("username=%s\n", c.username);
 		if (c.password)
 			printf("password=%s\n", c.password);
-	}
-	else if (!strcmp(op, "approve"))
+	} else if (!strcmp(op, "approve")) {
 		credential_approve(&c);
-	else if (!strcmp(op, "reject"))
+	} else if (!strcmp(op, "reject")) {
 		credential_reject(&c);
-	else
+	} else {
 		usage(usage_msg);
-
+	}
 	return 0;
 }
diff --git a/git.c b/git.c
index 4da3db5..8788b32 100644
--- a/git.c
+++ b/git.c
@@ -351,6 +351,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config, RUN_SETUP_GENTLY },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
+		{ "credential", cmd_credential, RUN_SETUP_GENTLY },
 		{ "describe", cmd_describe, RUN_SETUP },
 		{ "diff", cmd_diff },
 		{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 4a37cd7..7c4826e 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -4,10 +4,20 @@
 # stdout and stderr should be provided on stdin,
 # separated by "--".
 check() {
+	credential_opts=
+	credential_cmd=$1
+	shift
+	for arg in "$@"; do
+		credential_opts="$credential_opts -c credential.helper='$arg'"
+	done
 	read_chunk >stdin &&
 	read_chunk >expect-stdout &&
 	read_chunk >expect-stderr &&
-	test-credential "$@" <stdin >stdout 2>stderr &&
+	if ! eval "git $credential_opts credential $credential_cmd <stdin >stdout 2>stderr"; then
+		echo "git credential failed with code $?" &&
+		cat stderr &&
+		false
+	fi &&
 	test_cmp expect-stdout stdout &&
 	test_cmp expect-stderr stderr
 }
@@ -41,7 +51,7 @@ reject() {
 		echo protocol=$2
 		echo host=$3
 		echo username=$4
-	) | test-credential reject $1
+	) | git -c credential.helper=$1 credential reject
 }
 
 helper_test() {
-- 
1.7.11.5.g0c7e058.dirty

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

* [PATCH 2/3] git credential fill: output the whole 'struct credential'
  2012-06-24 11:39       ` Matthieu Moy
@ 2012-06-24 11:40         ` Matthieu Moy
  2012-06-24 11:40         ` [PATCH 3/3] git-remote-mediawiki: add credential support Matthieu Moy
  2012-06-26 21:58         ` [PATCH 1/3] add 'git credential' plumbing command Junio C Hamano
  2 siblings, 0 replies; 11+ messages in thread
From: Matthieu Moy @ 2012-06-24 11:40 UTC (permalink / raw)
  To: git, gitster; +Cc: peff, Matthieu Moy

Instead of outputing only the username and password, print all the
attributes, even those that already appeared in the input.

This is closer to what the C API does, and allows one to take the exact
output of "git credential fill" as input to "git credential approve" or
"git credential reject".
---
 Documentation/git-credential.txt | 16 ++++++++++++----
 builtin/credential.c             |  5 +----
 credential.c                     |  2 +-
 credential.h                     |  1 +
 t/lib-credential.sh              | 25 +++++++++++++++++++++++++
 t/t0300-credentials.sh           | 14 ++++++++++++++
 6 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index d5228a3..7489427 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -64,12 +64,20 @@ infomation it has):
 
   2. Ask git-credential to give us a username and password for this
      description. This is done by running `git credential fill`,
-     feeding the description from step (1) to its standard input. The
-     credential will be produced on standard output, like:
+     feeding the description from step (1) to its standard input. The complete
+     credential description (including the credential per se, i.e. the
+     login and password) will be produced on standard output, like:
 
+	protocol=https
+	host=example.com
 	username=bob
 	password=secr3t
 +
+In most cases, this means the attributes given in the input will be
+repeated in the output, but git may also modify the credential
+description, for example by removing the `path` attribute when the
+protocol is HTTP(s) and `credential.useHttpPath` is false.
++
 If the `git credential` knew about the password, this step may
 not have involved the user actually typing this password (the
 user may have typed a password to unlock the keychain instead,
@@ -86,8 +94,8 @@ unlocked) before it returned `password=secr3t`.
      was rejected during the operation, use the "reject" action so
      that `git credential` will ask for a new password in its next
      invocation. In either case, `git credential` should be fed with
-     the credential description obtained from step (2) together with
-     the ones already provided in step (1).
+     the credential description obtained from step (2) (which also
+     contain the ones provided in step (1)).
 
 [[IOFMT]]
 INPUT/OUTPUT FORMAT
diff --git a/builtin/credential.c b/builtin/credential.c
index c185c07..0412fa0 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -19,10 +19,7 @@ int cmd_credential(int argc, const char **argv, const char *prefix)
 
 	if (!strcmp(op, "fill")) {
 		credential_fill(&c);
-		if (c.username)
-			printf("username=%s\n", c.username);
-		if (c.password)
-			printf("password=%s\n", c.password);
+		credential_write(&c, stdout);
 	} else if (!strcmp(op, "approve")) {
 		credential_approve(&c);
 	} else if (!strcmp(op, "reject")) {
diff --git a/credential.c b/credential.c
index 62d1c56..2c40007 100644
--- a/credential.c
+++ b/credential.c
@@ -191,7 +191,7 @@ static void credential_write_item(FILE *fp, const char *key, const char *value)
 	fprintf(fp, "%s=%s\n", key, value);
 }
 
-static void credential_write(const struct credential *c, FILE *fp)
+void credential_write(const struct credential *c, FILE *fp)
 {
 	credential_write_item(fp, "protocol", c->protocol);
 	credential_write_item(fp, "host", c->host);
diff --git a/credential.h b/credential.h
index 96ea41b..0c3e85e 100644
--- a/credential.h
+++ b/credential.h
@@ -26,6 +26,7 @@ void credential_approve(struct credential *);
 void credential_reject(struct credential *);
 
 int credential_read(struct credential *, FILE *);
+void credential_write(const struct credential *, FILE *);
 void credential_from_url(struct credential *, const char *url);
 int credential_match(const struct credential *have,
 		     const struct credential *want);
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 7c4826e..957ae93 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -62,6 +62,8 @@ helper_test() {
 		protocol=https
 		host=example.com
 		--
+		protocol=https
+		host=example.com
 		username=askpass-username
 		password=askpass-password
 		--
@@ -84,6 +86,8 @@ helper_test() {
 		protocol=https
 		host=example.com
 		--
+		protocol=https
+		host=example.com
 		username=store-user
 		password=store-pass
 		--
@@ -95,6 +99,8 @@ helper_test() {
 		protocol=http
 		host=example.com
 		--
+		protocol=http
+		host=example.com
 		username=askpass-username
 		password=askpass-password
 		--
@@ -108,6 +114,8 @@ helper_test() {
 		protocol=https
 		host=other.tld
 		--
+		protocol=https
+		host=other.tld
 		username=askpass-username
 		password=askpass-password
 		--
@@ -122,6 +130,8 @@ helper_test() {
 		host=example.com
 		username=other
 		--
+		protocol=https
+		host=example.com
 		username=other
 		password=askpass-password
 		--
@@ -143,6 +153,9 @@ helper_test() {
 		host=path.tld
 		path=bar.git
 		--
+		protocol=http
+		host=path.tld
+		path=bar.git
 		username=askpass-username
 		password=askpass-password
 		--
@@ -160,6 +173,8 @@ helper_test() {
 		protocol=https
 		host=example.com
 		--
+		protocol=https
+		host=example.com
 		username=askpass-username
 		password=askpass-password
 		--
@@ -186,6 +201,8 @@ helper_test() {
 		host=example.com
 		username=user1
 		--
+		protocol=https
+		host=example.com
 		username=user1
 		password=pass1
 		EOF
@@ -194,6 +211,8 @@ helper_test() {
 		host=example.com
 		username=user2
 		--
+		protocol=https
+		host=example.com
 		username=user2
 		password=pass2
 		EOF
@@ -210,6 +229,8 @@ helper_test() {
 		host=example.com
 		username=user1
 		--
+		protocol=https
+		host=example.com
 		username=user1
 		password=askpass-password
 		--
@@ -223,6 +244,8 @@ helper_test() {
 		host=example.com
 		username=user2
 		--
+		protocol=https
+		host=example.com
 		username=user2
 		password=pass2
 		EOF
@@ -244,6 +267,8 @@ helper_test_timeout() {
 		protocol=https
 		host=timeout.tld
 		--
+		protocol=https
+		host=timeout.tld
 		username=askpass-username
 		password=askpass-password
 		--
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 20e28e3..538ea5f 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -82,6 +82,9 @@ test_expect_success 'credential_fill passes along metadata' '
 	host=example.com
 	path=foo.git
 	--
+	protocol=ftp
+	host=example.com
+	path=foo.git
 	username=one
 	password=two
 	--
@@ -213,6 +216,8 @@ test_expect_success 'match configured credential' '
 	host=example.com
 	path=repo.git
 	--
+	protocol=https
+	host=example.com
 	username=foo
 	password=bar
 	--
@@ -225,6 +230,8 @@ test_expect_success 'do not match configured credential' '
 	protocol=https
 	host=bar
 	--
+	protocol=https
+	host=bar
 	username=askpass-username
 	password=askpass-password
 	--
@@ -239,6 +246,8 @@ test_expect_success 'pull username from config' '
 	protocol=https
 	host=example.com
 	--
+	protocol=https
+	host=example.com
 	username=foo
 	password=askpass-password
 	--
@@ -252,6 +261,8 @@ test_expect_success 'http paths can be part of context' '
 	host=example.com
 	path=foo.git
 	--
+	protocol=https
+	host=example.com
 	username=foo
 	password=bar
 	--
@@ -265,6 +276,9 @@ test_expect_success 'http paths can be part of context' '
 	host=example.com
 	path=foo.git
 	--
+	protocol=https
+	host=example.com
+	path=foo.git
 	username=foo
 	password=bar
 	--
-- 
1.7.11.5.g0c7e058.dirty

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

* [PATCH 3/3] git-remote-mediawiki: add credential support
  2012-06-24 11:39       ` Matthieu Moy
  2012-06-24 11:40         ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
@ 2012-06-24 11:40         ` Matthieu Moy
  2012-06-26 21:58         ` [PATCH 1/3] add 'git credential' plumbing command Junio C Hamano
  2 siblings, 0 replies; 11+ messages in thread
From: Matthieu Moy @ 2012-06-24 11:40 UTC (permalink / raw)
  To: git, gitster; +Cc: peff, Matthieu Moy

The previous version implemented the possibility to log in a wiki, but
the username and password had to be provided as configuration variables.
We add the possibility to use the Git credential system to prompt
the password.

The support if implemented with generic functions that mimic the C API,
designed to be usable from other contexts in the future (i.e. they may
migrate to Git.pm if someone is interested).

While we're there, do a bit of refactoring in mw_connect_maybe.

Based on patch by: Javier Roucher Iglesias <Javier.Roucher-Iglesias@ensimag.imag.fr>
---
 contrib/mw-to-git/git-remote-mediawiki | 106 +++++++++++++++++++++++++++++----
 1 file changed, 94 insertions(+), 12 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki
index c18bfa1..a34f53f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki
+++ b/contrib/mw-to-git/git-remote-mediawiki
@@ -43,6 +43,8 @@ use encoding 'utf8';
 binmode STDERR, ":utf8";
 
 use URI::Escape;
+use IPC::Open2;
+
 use warnings;
 
 # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced
@@ -151,28 +153,108 @@ while (<STDIN>) {
 
 ########################## Functions ##############################
 
+## credential API management (generic functions)
+
+sub credential_from_url {
+	my $url = shift;
+	my $parsed = URI->new($url);
+	my %credential;
+
+	if ($parsed->scheme) {
+		$credential{protocol} = $parsed->scheme;
+	}
+	if ($parsed->host) {
+		$credential{host} = $parsed->host;
+	}
+	if ($parsed->path) {
+		$credential{path} = $parsed->path;
+	}
+	if ($parsed->userinfo) {
+		if ($parsed->userinfo =~ /([^:]*):(.*)/) {
+			$credential{username} = $1;
+			$credential{password} = $2;
+		} else {
+			$credential{username} = $parsed->userinfo;
+		}
+	}
+
+	return %credential;
+}
+
+sub credential_read {
+	my %credential;
+	my $reader = shift;
+	my $op = shift;
+	while (<$reader>) {
+		my ($key, $value) = /([^=]*)=(.*)/;
+		if (not defined $key) {
+			die "ERROR receiving response from git credential $op:\n$_\n";
+		}
+		$credential{$key} = $value;
+	}
+	return %credential;
+}
+
+sub credential_write {
+	my $credential = shift;
+	my $writer = shift;
+	while (my ($key, $value) = each(%$credential) ) {
+		if ($value) {
+			print $writer "$key=$value\n";
+		}
+	}
+}
+
+sub credential_run {
+	my $op = shift;
+	my $credential = shift;
+	my $pid = open2(my $reader, my $writer, "git credential $op");
+	credential_write($credential, $writer);
+	print $writer "\n";
+	close($writer);
+	
+	if ($op eq "fill") {
+		%$credential = credential_read($reader, $op);
+	} else {
+		if (<$reader>) {
+			die "ERROR while running git credential $op:\n$_";
+		}
+	}
+	close($reader);
+	waitpid($pid, 0);
+	my $child_exit_status = $? >> 8;
+	if ($child_exit_status != 0) {
+		die "'git credential $op' failed with code $child_exit_status.";
+	}
+}
+
 # MediaWiki API instance, created lazily.
 my $mediawiki;
 
 sub mw_connect_maybe {
 	if ($mediawiki) {
-	    return;
+		return;
 	}
 	$mediawiki = MediaWiki::API->new;
 	$mediawiki->{config}->{api_url} = "$url/api.php";
 	if ($wiki_login) {
-		if (!$mediawiki->login({
-			lgname => $wiki_login,
-			lgpassword => $wiki_passwd,
-			lgdomain => $wiki_domain,
-		})) {
-			print STDERR "Failed to log in mediawiki user \"$wiki_login\" on $url\n";
-			print STDERR "(error " .
-			    $mediawiki->{error}->{code} . ': ' .
-			    $mediawiki->{error}->{details} . ")\n";
-			exit 1;
+		my %credential = credential_from_url($url);
+		$credential{username} = $wiki_login;
+		$credential{password} = $wiki_passwd;
+		credential_run("fill", \%credential);
+		my $request = {lgname => $credential{username},
+			       lgpassword => $credential{password},
+			       lgdomain => $wiki_domain};
+		if ($mediawiki->login($request)) {
+			credential_run("approve", \%credential);
+			print STDERR "Logged in mediawiki user \"$credential{username}\".\n";
 		} else {
-			print STDERR "Logged in with user \"$wiki_login\".\n";
+			print STDERR "Failed to log in mediawiki user \"$credential{username}\" on $url\n";
+			print STDERR "  (error " .
+				$mediawiki->{error}->{code} . ': ' .
+				$mediawiki->{error}->{details} . ")\n";
+			credential_run("reject", \%credential);
+			exit 1;
 		}
 	}
 }
-- 
1.7.11.5.g0c7e058.dirty

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

* Re: [PATCH 1/3] add 'git credential' plumbing command
  2012-06-24 11:39       ` Matthieu Moy
  2012-06-24 11:40         ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
  2012-06-24 11:40         ` [PATCH 3/3] git-remote-mediawiki: add credential support Matthieu Moy
@ 2012-06-26 21:58         ` Junio C Hamano
  2 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2012-06-26 21:58 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: git, peff, Javier Roucher Iglesias, Pavel Volek, Kim Thuat Nguyen

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
> new file mode 100644
> index 0000000..d5228a3
> --- /dev/null
> +++ b/Documentation/git-credential.txt
> @@ -0,0 +1,136 @@
> +git-credential(7)
> +=================

Thanks.  I'll update this with s/7/1/ to make it consistent with
what the Makefile expects (my build of 'pu' was failing without the
fixup).

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

end of thread, other threads:[~2012-06-26 21:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-22 16:07 [PATCH 0/3] git credential plumbing command: implementation and use Matthieu Moy
2012-06-22 16:07 ` [PATCH 1/3] add 'git credential' plumbing command Matthieu Moy
2012-06-22 20:24   ` Junio C Hamano
2012-06-24 11:37     ` Matthieu Moy
2012-06-24 11:39       ` Matthieu Moy
2012-06-24 11:40         ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
2012-06-24 11:40         ` [PATCH 3/3] git-remote-mediawiki: add credential support Matthieu Moy
2012-06-26 21:58         ` [PATCH 1/3] add 'git credential' plumbing command Junio C Hamano
2012-06-22 16:07 ` [PATCH 2/3] git credential fill: output the whole 'struct credential' Matthieu Moy
2012-06-22 20:26   ` Junio C Hamano
2012-06-22 16:07 ` [PATCH 3/3] git-remote-mediawiki: add credential support Matthieu Moy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).