Subject: Re: [PATCH 8/8] http: set specific auth scheme depending on credential

From: Derrick Stolee <[email-masked]>
Date: Mon, 19 Sep 2022 12:42:12 -0400

On 9/13/2022 3:25 PM, Matthew John Cheetham via GitGitGadget wrote:
> From: Matthew John Cheetham <[email-masked]>
> 
> Introduce a new credential field `authtype` that can be used by
> credential helpers to indicate the type of the credential or
> authentication mechanism to use for a request.
> 
> Modify http.c to now specify the correct authentication scheme or
> credential type when authenticating the curl handle. If the new
> `authtype` field in the credential structure is `NULL` or "Basic" then
> use the existing username/password options. If the field is "Bearer"
> then use the OAuth bearer token curl option. Otherwise, the `authtype`
> field is the authentication scheme and the `password` field is the
> raw, unencoded value.

> @@ -524,8 +525,25 @@ static void init_curl_http_auth(struct active_request_slot *slot)
>  
>  	credential_fill(&http_auth);
>  
> -	curl_easy_setopt(slot->curl, CURLOPT_USERNAME, http_auth.username);
> -	curl_easy_setopt(slot->curl, CURLOPT_PASSWORD, http_auth.password);
> +	if (!http_auth.authtype || !strcasecmp(http_auth.authtype, "basic")
> +				|| !strcasecmp(http_auth.authtype, "digest")) {
> +		curl_easy_setopt(slot->curl, CURLOPT_USERNAME,
> +			http_auth.username);
> +		curl_easy_setopt(slot->curl, CURLOPT_PASSWORD,
> +			http_auth.password);
> +#ifdef GIT_CURL_HAVE_CURLAUTH_BEARER
> +	} else if (!strcasecmp(http_auth.authtype, "bearer")) {
> +		curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, CURLAUTH_BEARER);
> +		curl_easy_setopt(slot->curl, CURLOPT_XOAUTH2_BEARER,
> +			http_auth.password);
> +#endif
> +	} else {
> +		struct strbuf auth = STRBUF_INIT;
> +		strbuf_addf(&auth, "Authorization: %s %s",
> +			http_auth.authtype, http_auth.password);
> +		slot->headers = curl_slist_append(slot->headers, auth.buf);
> +		strbuf_release(&auth);
> +	}
>  }

It would be good to have a test here, and the only way I can think
to add it would be to modify one of the test credential helpers to
indicate that OAuth is being used.

The test would somehow need to be careful about the curl version,
though, and I'm not sure if we have prior work for writing prereqs
based on the linked curl version.

Thanks,
-Stolee