* [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
* 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 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
* [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
* 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
* [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
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).