{"thread":{"id":"23292","subject":"[PATCH] Prompt for a username when an HTTP request 401s","startedAt":"2010-04-01T22:14:35Z","lastAt":"2010-04-02T16:11:14Z","messageCount":4,"participants":["Scott Chacon","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"138367","messageId":"m2wd411cc4a1004011514w6d350ac7l15ab6bb1a6be8d89@mail.gmail.com","threadId":"23292","inReplyTo":null,"subject":"[PATCH] Prompt for a username when an HTTP request 401s","fromName":"Scott Chacon","fromEmail":"schacon@gmail.com","sentAt":"2010-04-01T22:14:35Z","receivedAt":"2010-04-01T22:14:35Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"When an HTTP request returns a 401, Git will currently fail with a\nconfusing message saying that it got a 401, which is not very\ndescriptive.\n\nCurrently if a user wants to use Git over HTTP, they have to use one\nURL with the username in the URL (e.g. \"http://user@host.com/repo.git\")\nfor write access and another without the username for unauthenticated\nread access (unless they want to be prompted for the password each\ntime). However, since the HTTP servers will return a 401 if an action\nrequires authentication, we can prompt for username and password if we\nsee this, allowing us to use a single URL for both purposes.\n\nThis patch changes http_request to prompt for the username and password,\nthen return HTTP_REAUTH so http_get_strbuf can try again. If it gets\na 401 even when a user/pass is supplied, http_request will now return\nHTTP_NOAUTH which remote_curl can then use to display a more\nintelligent error message that is less confusing.\n\nSigned-off-by: Scott Chacon \n---\n\nUpdated the comments style and the commit message for Junio.\n\n http.c | 22 ++++++++++++++++++++--\n http.h | 2 ++\n remote-curl.c | 2 ++\n 3 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 4814217..51253e1 100644\n--- a/http.c\n+++ b/http.c\n@@ -815,7 +815,21 @@ static int http_request(const char *url, void\n*result, int target, int options)\n \t\t\tret = HTTP_OK;\n \t\telse if (missing_target(&results))\n \t\t\tret = HTTP_MISSING_TARGET;\n-\t\telse\n+\t\telse if (results.http_code == 401) {\n+\t\t\tif (user_name) {\n+\t\t\t\tret = HTTP_NOAUTH;\n+\t\t\t} else {\n+\t\t\t\t/*\n+\t\t\t\t * git_getpass is needed here because its very likely stdin/stdout are\n+\t\t\t\t * pipes to our parent process. So we instead need to use /dev/tty,\n+\t\t\t\t * but that is non-portable. Using git_getpass() can at least be stubbed\n+\t\t\t\t * on other platforms with a different implementation if/when necessary.\n+\t\t\t\t */\n+\t\t\t\tuser_name = xstrdup(git_getpass(\"Username: \"));\n+\t\t\t\tinit_curl_http_auth(slot->curl);\n+\t\t\t\tret = HTTP_REAUTH;\n+\t\t\t}\n+\t\t} else\n \t\t\tret = HTTP_ERROR;\n \t} else {\n \t\terror(\"Unable to start HTTP request for %s\", url);\n@@ -831,7 +845,11 @@ static int http_request(const char *url, void\n*result, int target, int options)\n\n int http_get_strbuf(const char *url, struct strbuf *result, int options)\n {\n-\treturn http_request(url, result, HTTP_REQUEST_STRBUF, options);\n+\tint http_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);\n+\tif (http_ret == HTTP_REAUTH) {\n+\t\thttp_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);\n+\t}\n+\treturn http_ret;\n }\n\n /*\ndiff --git a/http.h b/http.h\nindex 5c9441c..2dd03e8 100644\n--- a/http.h\n+++ b/http.h\n@@ -126,6 +126,8 @@ extern char *get_remote_object_url(const char\n*url, const char *hex,\n #define HTTP_MISSING_TARGET\t1\n #define HTTP_ERROR\t\t2\n #define HTTP_START_FAILED\t3\n+#define HTTP_REAUTH\t4\n+#define HTTP_NOAUTH\t5\n\n /*\n * Requests an url and stores the result in a strbuf.\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b76bfcb..0782756 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -132,6 +132,8 @@ static struct discovery* discover_refs(const char *service)\n \tcase HTTP_MISSING_TARGET:\n \t\tdie(\"%s not found: did you run git update-server-info on the\"\n \t\t \" server?\", refs_url);\n+\tcase HTTP_NOAUTH:\n+\t\tdie(\"Authentication failed\");\n \tdefault:\n \t\thttp_error(refs_url, http_ret);\n \t\tdie(\"HTTP request failed\");\n-- \n1.7.0.1\n"},{"id":"138390","messageId":"7veiiymk75.fsf@alter.siamese.dyndns.org","threadId":"23292","inReplyTo":"m2wd411cc4a1004011514w6d350ac7l15ab6bb1a6be8d89@mail.gmail.com","subject":"Re: [PATCH] Prompt for a username when an HTTP request 401s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-02T06:39:58Z","receivedAt":"2010-04-02T06:39:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Scott Chacon writes:\n\n> When an HTTP request returns a 401, Git will currently fail with a\n> confusing message saying that it got a 401, which is not very\n> descriptive.\n>\n> Currently if a user wants to use Git over HTTP, they have to use one\n> URL with the username in the URL (e.g. \"http://user@host.com/repo.git\")\n> for write access and another without the username for unauthenticated\n> read access (unless they want to be prompted for the password each\n> time). However, since the HTTP servers will return a 401 if an action\n> requires authentication, we can prompt for username and password if we\n> see this, allowing us to use a single URL for both purposes.\n\nThanks; this illustrates the issue you are trying to solve much easier to\nsee, don't you agree?\n\nAn obvious enhancement could be to make \"http://user@host.com/repo.git\"\nask for password lazily. Then such a URL can be used even for an access\nthat does not need authentication and the user does not have to prompted\nfor the password each time, which was what you wanted to really solve, no?\n\nActually that could not just be an enhancement, but might be a better\nalternative solution to the problem, but I haven't thought things\nthrough.\n\n> Signed-off-by: Scott Chacon \n> ---\n>\n> Updated the comments style and the commit message for Junio.\n\nHeh, Message update is never _for_ me. It is to clarify the problem you\nare trying to solve, so that we can be certain that the proposed patch is\nthe best approach to solve it.\n\n> diff --git a/http.c b/http.c\n> index 4814217..51253e1 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -815,7 +815,21 @@ static int http_request(const char *url, void\n> *result, int target, int options)\n\nI fixed this up when I queued the previous version, and you have the same\nline wrapping problem in this version, which I have fixed, too, before\nreplacing what was queued to 'pu'.\n\nI mention this not as a complaint (but I would appreciate if you try to be\ncareful next time, especially if you plan to post more patches and to\nbecome a regular contributor), but primarily because it is curious that\nonly the hunk headers are wrapped but not these long lines we see below:\n\n> ...\n> +\t\t\t} else {\n> +\t\t\t\t/*\n> +\t\t\t\t * git_getpass is needed here because its very likely stdin/stdout are\n> ...\n"},{"id":"138432","messageId":"y2rd411cc4a1004020843we196537ak35ab6006ce28fefe@mail.gmail.com","threadId":"23292","inReplyTo":"7veiiymk75.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Prompt for a username when an HTTP request 401s","fromName":"Scott Chacon","fromEmail":"schacon@gmail.com","sentAt":"2010-04-02T15:43:00Z","receivedAt":"2010-04-02T15:43:00Z","isPatch":true,"sender":{"key":"schacon@gmail.com","avatar":"https://gravatar.com/avatar/9b13a8a078e1dcf8588c4eea9554445d51ebed6c41b51f56f4d96738130b05c6?d=mp&s=160"},"body":"Hey,\n\nOn Thu, Apr 1, 2010 at 11:39 PM, Junio C Hamano wrote:\n> An obvious enhancement could be to make \"http://user@host.com/repo.git\"\n> ask for password lazily.  Then such a URL can be used even for an access\n> that does not need authentication and the user does not have to prompted\n> for the password each time, which was what you wanted to really solve, no?\n>\n> Actually that could not just be an enhancement, but might be a better\n> alternative solution to the problem, but I haven't thought things\n> through.\n\nActually, what I want to do is be able to show a single URL that will\nwork for everyone - both read-only and read-write users, so I much\nprefer the way I wrote the patch. This way, GitHub and kernel.org and\nwhomever else can just publish the one url and it will prompt if they\nneed auth for some reason, and just work if not.\n\nI do however agree that if someone _does_ put their username in the\nurl that it should only prompt for the password if it 401s. That\nshould probably be a separate patch, though.\n\nScott\n"},{"id":"138434","messageId":"7viq89bzrx.fsf@alter.siamese.dyndns.org","threadId":"23292","inReplyTo":"y2rd411cc4a1004020843we196537ak35ab6006ce28fefe@mail.gmail.com","subject":"Re: [PATCH] Prompt for a username when an HTTP request 401s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-02T16:11:14Z","receivedAt":"2010-04-02T16:11:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Scott Chacon writes:\n\n> I do however agree that if someone _does_ put their username in the\n> url that it should only prompt for the password if it 401s. That\n> should probably be a separate patch, though.\n\nOh, absolutely. Thanks for a clear explanation.\n"}]}