> On 2 Jun 2025, at 5:57 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Aditya Garg <gargaditya08@xxxxxxxx> 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"?
No, there is a difference here. CRAM-MD5 works ONLY with OpenSSL.
OAuth2 and PLAIN work with BOTH OpenSSL and libcurl.
Anyways, taking comments from the OAuth2.0 reply, let me see if I can
remove these statements.
>> +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, "
