From: M Hickford <mirth.hickford@xxxxxxxxx>
A credential helper may return a bad credential if the user's password
has changed or a personal access token has expired. The user gets
an HTTP 401 Unauthorized error. The user invariably retries the command.
To spare the user from retrying the command, in case of HTTP 401
Unauthorized, call `credential fill` again and reauthenticate. This will
succeed if a helper generates a fresh credential or the user enters a
valid password.
Keep current behaviour of asking user for username and password at
most once. Sanity check that second credential differs from first before
trying it.
Alternatives considered: add a string 'source' field to credential
struct that records which helper (or getpass) populated credential.
Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx>
---
[RFC] http: reauthenticate on 401 Unauthorized
cc. Jeff King peff@xxxxxxxx
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1521%2Fhickford%2Freauth-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1521/hickford/reauth-v1
Pull-Request: https://github.com/git/git/pull/1521
credential.c | 1 +
credential.h | 4 +++-
http.c | 30 +++++++++++++++++++++++++++---
t/t5551-http-fetch-smart.sh | 10 ++--------
t/t5563-simple-http-auth.sh | 3 +++
5 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/credential.c b/credential.c
index 023b59d5711..00fea51800c 100644
--- a/credential.c
+++ b/credential.c
@@ -379,6 +379,7 @@ void credential_fill(struct credential *c)
c->helpers.items[i].string);
}
+ c->getpass = 1;
credential_getpass(c);
if (!c->username && !c->password)
die("unable to get password from user");
diff --git a/credential.h b/credential.h
index b8e2936d1dc..c176b05981a 100644
--- a/credential.h
+++ b/credential.h
@@ -134,7 +134,9 @@ struct credential {
configured:1,
quit:1,
use_http_path:1,
- username_from_proto:1;
+ username_from_proto:1,
+ /* Whether the user has been prompted for username or password. */
+ getpass:1;
char *username;
char *password;
diff --git a/http.c b/http.c
index bb58bb3e6a3..d2897c4d9d1 100644
--- a/http.c
+++ b/http.c
@@ -1732,7 +1732,11 @@ static int handle_curl_result(struct slot_results *results)
else if (results->http_code == 401) {
if (http_auth.username && http_auth.password) {
credential_reject(&http_auth);
- return HTTP_NOAUTH;
+ if (http_auth.getpass) {
+ /* Previously prompted user, don't prompt again. */
+ return HTTP_NOAUTH;
+ }
+ return HTTP_REAUTH;
} else {
http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
if (results->auth_avail) {
@@ -2125,6 +2129,9 @@ static int http_request_reauth(const char *url,
struct http_get_options *options)
{
int ret = http_request(url, result, target, options);
+ int reauth = 0;
+ char* first_username;
+ char* first_password;
if (ret != HTTP_OK && ret != HTTP_REAUTH)
return ret;
@@ -2140,6 +2147,9 @@ static int http_request_reauth(const char *url,
if (ret != HTTP_REAUTH)
return ret;
+ credential_fill(&http_auth);
+
+reauth:
/*
* The previous request may have put cruft into our output stream; we
* should clear it out before making our next request.
@@ -2163,9 +2173,23 @@ static int http_request_reauth(const char *url,
BUG("Unknown http_request target");
}
- credential_fill(&http_auth);
+ first_username = xstrdup(http_auth.username);
+ first_password = xstrdup(http_auth.password);
+ ret = http_request(url, result, target, options);
+ if (ret == HTTP_REAUTH && reauth++ == 0) {
+ credential_fill(&http_auth);
+ /* Sanity check that second credential differs from first. */
+ if (strcmp(first_username, http_auth.username)
+ || strcmp(first_password, http_auth.password)) {
+ free(first_username);
+ free(first_password);
+ goto reauth;
+ }
+ }
- return http_request(url, result, target, options);
+ free(first_username);
+ free(first_password);
+ return ret;
}
int http_get_strbuf(const char *url,
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 21b7767cbd3..be2e76133c1 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -589,14 +589,8 @@ test_expect_success 'http auth forgets bogus credentials' '
echo "password=bogus"
} | git credential approve &&
- # we expect this to use the bogus values and fail, never even
- # prompting the user...
- set_askpass user@host pass@host &&
- test_must_fail git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
- expect_askpass none &&
-
- # ...but now we should have forgotten the bad value, causing
- # us to prompt the user again.
+ # we expect this to use the bogus values and fail, forget the bad value,
+ # then reauthenticate, prompting the user
set_askpass user@host pass@host &&
git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
expect_askpass both user@host
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index ab8a721ccc7..1e7e426bc65 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -111,6 +111,9 @@ test_expect_success 'access using basic auth invalid credentials' '
protocol=http
host=$HTTPD_DEST
wwwauth[]=Basic realm="example.com"
+ protocol=http
+ host=$HTTPD_DEST
+ wwwauth[]=Basic realm="example.com"
EOF
expect_credential_query erase <<-EOF
base-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09
--
gitgitgadget
