Re: [PATCH] [RFC] http: reauthenticate on 401 Unauthorized
From: Junio C Hamano <hidden>
Date: 2023-06-12 21:59:21
"M Hickford via GitGitGadget" [off-list ref] writes:
From: M Hickford <redacted> 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.
... and no matter how many times the user retries, the command will never succeed? Is that the problem the patch tries to solve?
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.
Soon after changing the password is probably the time it is more likely that you would mistype your password, than after you got used to typing it over and over again. I can understand the wish to avoid asking for correct password forever, but giving just one attempt feels a bit cruel for that reason.
quoted hunk
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;
Mental note: the comment here says "prompted".
quoted hunk
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;
And here we also see "prompted" again. Perhaps it will help make the result easier to read if we renamed the new member from "getpass" to another phrase that contains "prompt"?
quoted hunk
} 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;
In our codebase, asterisk sticks to the variable, not type. Thanks.