Re: [PATCH v11 3/9] imap-send: add PLAIN authentication method to OpenSSL

[Junio C Hamano <gitster@pobox.com>]

Aditya Garg <gargaditya08@live.com> writes:

>  #else
>  
> +static char *plain_base64(const char *user UNUSED,
> +		  const char *access_token UNUSED)
> +{
> +	die("You are trying to use PLAIN authenticate method "
> +	    "with OpenSSL library, but its support has not been compiled in.");
> +}

This comment may apply also to the earlier OAuth related two stub
functions, but this is the "#else" side of "#ifndef NO_OPENSSL";
double negation always makes all our spin, but in short, this is
"You are not building with OpenSSL".  We cannot quite look at the
post context of this hunk for sanity check, but inside the cram()
stub function ...

>  static char *cram(const char *challenge_64 UNUSED,
>  		  const char *user UNUSED,
>  		  const char *pass UNUSED)

	die("If you want to use CRAM-MD5 authenticate method, "
	    "you have to build git-imap-send with OpenSSL library.");

... is the message it dies with.  So, shouldn't the error from the
new stub function also say "If you want to use PLAIN, you have to
build with OpenSSL"?

> +static int auth_plain(struct imap_store *ctx, const char *prompt UNUSED)
> +{
> +	int ret;
> +	char *b64;
> +
> +	b64 = plain_base64(ctx->cfg->user, ctx->cfg->pass);
> +	if (!b64)
> +		return error("PLAIN: base64 encoding failed");
> +
> +	/* Send the base64-encoded response */
> +	ret = socket_write(&ctx->imap->buf.sock, b64, strlen(b64));
> +	if (ret != (int)strlen(b64)) {
> +		free(b64);
> +		return error("IMAP error: sending PLAIN response failed");
> +	}
> +
> +	free(b64);
> +	return 0;
> +}

And the same comment about not gracefully failing when our side lack
support, even though we gracefully fail when the other side lacks
support, given to an earlier step also applies here.

> @@ -1209,7 +1273,22 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
>  		if (srvc->auth_method) {
>  			struct imap_cmd_cb cb;
>  
> -			if (!strcmp(srvc->auth_method, "CRAM-MD5")) {
> +			if (!strcmp(srvc->auth_method, "PLAIN")) {
> +				if (!CAP(AUTH_PLAIN)) {
> +					fprintf(stderr, "You specified "
> +						"PLAIN as authentication method, "
> +						"but %s doesn't support it.\n", srvc->host);
> +					goto bail;
> +				}
> +				/* PLAIN */
> +
> +				memset(&cb, 0, sizeof(cb));
> +				cb.cont = auth_plain;
> +				if (imap_exec(ctx, &cb, "AUTHENTICATE PLAIN") != RESP_OK) {
> +					fprintf(stderr, "IMAP error: AUTHENTICATE PLAIN failed\n");
> +					goto bail;
> +				}
> +			} else if (!strcmp(srvc->auth_method, "CRAM-MD5")) {
>  				if (!CAP(AUTH_CRAM_MD5)) {
>  					fprintf(stderr, "You specified "
>  						"CRAM-MD5 as authentication method, "