On Sat, Jan 03, 2015 at 06:19:23AM -0500, Jeff King wrote:
This pattern gets repeated in several places. Now that
http_passwordless_auth is a global, can we handle it automatically for
the callers, as below (which, aside from compiling, is completely
untested by me)?

This looks good (although I haven't tested it).

Note that this is in a slightly different boat than credential_fill.
Ideally we would also handle picking up credentials on behalf of the
callers of get_curl_handle/handle_curl_result. But that may involve
significant work and/or prompting the user, which we _must_ avoid if we
do not know if we are going to retry the request (and only the caller
knows that for sure). However, in the case of http_passwordless_auth, we
are just setting a flag, so it's OK to do it preemptively.

Right. We already prompt the user for a username and password in that case, so we already have the credentials that we need.

diff --git a/http.c b/http.c
index 040f362..2bbcdf1 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,8 @@ static const char *user_agent;

static struct credential cert_auth = CREDENTIAL_INIT;
static int ssl_cert_password_required;
+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
+static int http_passwordless_auth = 1;

static struct curl_slist *pragma_header;
static struct curl_slist *no_pragma_header;
@@ -318,7 +320,12 @@ static CURL *get_curl_handle(void)
        curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-       curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+       {
+               int flags = CURLAUTH_ANY;

I think this needs to be unsigned long or it can cause undefined behavior, since libcurl uses unsigned long in the flags. I'll fix that up when I reroll. I'll need your sign-off since it will essentially be your work.

+               if (!http_passwordless_auth)
+                       flags &= ~CURLAUTH_GSSNEGOTIATE;
+               curl_easy_setopt(result, CURLOPT_HTTPAUTH, flags);
+       }
#endif

        if (http_proactive_auth)
@@ -870,6 +877,7 @@ int handle_curl_result(struct slot_results *results)
                        credential_reject(&http_auth);
                        return HTTP_NOAUTH;
                } else {
+                       http_passwordless_auth = 0;
                        return HTTP_REAUTH;
                }
        } else {


Note that you could probably drop http_passwordless_auth completely, and
just keep a:

 static int http_auth_methods = CURLAUTH_ANY;

and then drop CURLAUTH_GSSNEGOTIATE from it instead of setting the
passwordless_auth flag to 0 (again, it happens in one place, so I don't
know that it needs an extra layer of abstraction).

This does seem to handle both cases well. It also has the pleasant side effect of being static.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

Attachment: signature.asc
Description: Digital signature

Reply via email to