On Thu, Jan 01, 2015 at 07:56:27PM +0000, brian m. carlson wrote:
> +void disable_passwordless_auth(struct active_request_slot *slot)
> +{
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +#define HTTP_AUTH_PASSWORDLESS (CURLAUTH_GSSNEGOTIATE)
> + curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH,
> + CURLAUTH_ANY & ~HTTP_AUTH_PASSWORDLESS);
> +#endif
> +}
I like that you are trying to put a layer of abstraction around what
"passwordless" means here, but it seems like there are two layers. The
function itself abstracts the idea, and then there is an extra
HTTP_AUTH_PASSWORDLESS macro. Since the concept is already confined to
this function and used only once, it might be more readable to simply
get rid of HTTP_AUTH_PASSWORD.
> @@ -1035,6 +1047,9 @@ static int http_request(const char *url,
> curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
>
> + if (!http_passwordless_auth)
> + disable_passwordless_auth(slot);
> +
> ret = run_one_slot(slot, &results);
>
> if (options && options->content_type) {
> @@ -1139,6 +1154,7 @@ static int http_request_reauth(const char *url,
> }
>
> credential_fill(&http_auth);
> + http_passwordless_auth = 0;
>
> return http_request(url, result, target, options);
> }
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)?
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.
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;
+ 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).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
