Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: Improve OAuth discovery logging
From: Andrey Borodin <x4mmm@yandex-team.ru>
In-Reply-To: <CAN4CZFNWBXtF-ML3yzdOvX3QEuUwVo5VrBzyWU3O=y-7SeDstA@mail.gmail.com>
Date: Thu, 26 Feb 2026 10:51:16 +0500
Cc: Jacob Champion <jacob.champion@enterprisedb.com>,
 Chao Li <li.evan.chao@gmail.com>,
 Daniel Gustafsson <daniel@yesql.se>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 Michael Paquier <michael@paquier.xyz>
Content-Transfer-Encoding: 7bit
Message-Id: <15434512-B3FB-4AB3-B6B3-5D85ED0B4BBE@yandex-team.ru>
References: <CAN4CZFPim7hUiyb7daNKQPSZ8CvQRBGkVhbvED7yZi8VktSn4Q@mail.gmail.com>
 <7DB528BA-C7A0-4B23-890C-5332FB35A16E@yesql.se>
 <CAN4CZFNNfhFCQdFWui5HWbQR60eM-cyndZ7YgSv7b5SKxB9C2A@mail.gmail.com>
 <CAOYmi+mDSmh6RNizHRmMAwg4ZP2W=uai3Fr3-wm186NMypf_Pg@mail.gmail.com>
 <CAN4CZFNJftK8NaREYaLi-wqpEz3=crQ=1+3f_XUVji=aOrDSWA@mail.gmail.com>
 <7094F798-8DD1-4974-9A04-10E147B29581@gmail.com>
 <CAOYmi+kjtmRMBdBU3_bGKGDoRSK2AErXbGtHkAjFRapcQNmjhA@mail.gmail.com>
 <CAN4CZFNWBXtF-ML3yzdOvX3QEuUwVo5VrBzyWU3O=y-7SeDstA@mail.gmail.com>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Archived-At: <https://www.postgresql.org/message-id/15434512-B3FB-4AB3-B6B3-5D85ED0B4BBE%40yandex-team.ru>
Precedence: bulk

This looks like nice patch addressing real issue in log analyzing.
Basic idea seems correct to me WRT OAuth, but I'm not a real expert in auth.

> On 25 Feb 2026, at 18:14, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
> 
> It can't, because the if is based on ctx->state. If I move it to
> before the if, I have to save the previous value, which just makes the
> code longer.

Well, you can do something in a line with

bool was_discovery = (ctx->state == OAUTH_STATE_ERROR_DISCOVERY);
ctx->state = OAUTH_STATE_FINISHED;
if (was_discovery)
{
}

But it's a matter of taste. Your code is correct anyway.

We can tweak comments a bit in sasl.h:

/*---------
 * exchange()
 *
 * Produces a server challenge to be sent to the client. The callback
 * must return one of the PG_SASL_EXCHANGE_* values, depending on
 * whether the exchange continues, has finished successfully, or has
 * failed.  <---- , or was abandoned by the client.

 * a successful outcome). The callback should set this to
 * NULL if the exchange is over and no output should be sent,
 * which should correspond to either PG_SASL_EXCHANGE_FAILURE
 * or a PG_SASL_EXCHANGE_SUCCESS with no outcome data.   <----- or ABANDONED

 * failure message.) Ignored if the exchange is completed
 * with PG_SASL_EXCHANGE_SUCCESS.  <------ or ABANDONED


That's all what I could grep.

And thanks for your review in my thread!


Best regards, Andrey Borodin.