MIME-Version: 1.0
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>
 <15434512-B3FB-4AB3-B6B3-5D85ED0B4BBE@yandex-team.ru>
 <CAN4CZFNscs=hiOkRJYF39r7AD7ef9+MR+O2BQdEtE_2Ajdo5qw@mail.gmail.com>
 <CAOYmi+nVzkoLjzNk_58e0NnUPi9uVXwmurK2QP6CzC2WOpqwbg@mail.gmail.com>
 <CAN4CZFPjiUQbKo2q+ovs--AHkjvaE8OJyncB9xu5b+1gp=HHPQ@mail.gmail.com>
 <CAOYmi+=SR_nJJBh7UXZzK8Zbs21L2RUNkW3d9aPRkQOHj1bBPA@mail.gmail.com>
 <CAN4CZFO7ju7fjjv+qwObP8_V-Tdx463zV8F7u_s6wtg9ANVWVg@mail.gmail.com>
 <CAOYmi+kEYA0Tp2son-+Ti1wvSAPov87AVFf4qXATTOHRX1F2gg@mail.gmail.com>
 <CAN4CZFOmym1BaV_U2V56aOyRp2JMrw5nfn6kwcAEcu_RWK-F3Q@mail.gmail.com>
 <99C6E80B-8770-41C2-8084-BF3C7F389FFF@yandex-team.ru>
 <CAN4CZFN7u1kX3_0cfyVvtfiWpORxnvZo=xCr9Ag-F5Onp-hpbA@mail.gmail.com>
 <1B58D836-6B8F-4B9A-9B84-08965E5AA06B@yandex-team.ru>
 <CAOYmi+kxfGEKw7frQPxWYEA6Qe4BLc683UCNPTYCLdCCV0b4Jw@mail.gmail.com>
 <CAN4CZFP--Ec8hMgpu7JojgK9qS08bNnev0c6goA++T4Ozy8bOQ@mail.gmail.com>
 <CAOYmi+nsK1dSXaB+oicoyA6kM9ymygCLhSiKtkg1ph_P1uhYOQ@mail.gmail.com>
 <1AB529D5-6F5D-426D-AB99-12BB7DD394D3@gmail.com>
 <CAN4CZFOwmgyv1002=EZTSM__97gX-1fG0Q3Q4Zy=XviVtZPRxg@mail.gmail.com>
In-Reply-To: 
 <CAN4CZFOwmgyv1002=EZTSM__97gX-1fG0Q3Q4Zy=XviVtZPRxg@mail.gmail.com>
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Tue, 17 Mar 2026 09:25:41 -0700
Message-ID: 
 <CAOYmi+=y=iAQ11E6jpUzOKsP8ARVK7g5=etaE3RsrtetFTN-+w@mail.gmail.com>
Subject: Re: Improve OAuth discovery logging
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: Chao Li <li.evan.chao@gmail.com>, Andrey Borodin <x4mmm@yandex-team.ru>,
	Daniel Gustafsson <daniel@yesql.se>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAOYmi%2B%3Dy%3DiAQ11E6jpUzOKsP8ARVK7g5%3DetaE3RsrtetFTN-%2Bw%40mail.gmail.com>
Precedence: bulk

On Mon, Mar 16, 2026 at 10:29=E2=80=AFPM Zsolt Parragi
<zsolt.parragi@percona.com> wrote:
> > As far as I understand, for a programmer error, Assert should be used. =
Why do we use elog(ERROR) here?
>
> +1

A couple reasons:
- to keep it parallel with the similar elog directly above it
- to strengthen the API boundary for an esoteric edge case without
requiring assertions to be enabled

I don't mind relying on assertions for (not an exhaustive list!)
well-exercised code, or when performance is critical, or when the
callers and callees are in the same source file, or when it's not a
security-critical context... This would seem to fail all of those
tests.

I imagine some of the existing assertions in OAuth should ideally be
strengthened into production checks, too. I've considered adding a
"production assert" variant for our security code in general, client-
and server-side.

> I would also say that for CheckSASLAuth, specifying abandoned is
> always required, since the caller can't know when it will result in an
> error.

That's not really true, because the caller hardcodes the mechanism
descriptor. The layers above and below CheckSASLAuth are coupled via
injection -- ClientAuthentication knows full well that a uaOAuth HBA
entry can't be satisfied by the SCRAM mechanisms, and vice-versa.

If that were to change in a meaningful way, then sure, the caller
would need to always provide the currently-OAuth-specific-edge-case
flag. (But in that case, if the caller somehow doesn't have to know
the mechanism in use, we could presumably also centralize a single
call to CheckSASLAuth().)

> Have you considered adding the error level here instead, the same way
> as in auth_failed, explicitly defaulted to normal fatal by the caller,
> so existing code don't have to change it? That wouldn't need an
> SASL-specific explanation or flag in the generic code.

I don't think I can visualize what you're proposing. If you mean that
CheckSASLAuth should set the elevel with an output parameter, I'd
rather not; that moves the responsibility for a very critical
assumption (we're ending the process now) across a bunch of different
files.

(If more things than OAuth need this eventually, maybe it becomes
STATUS_SILENT_ERROR or something, to make it even more generic?)

Thanks,
--Jacob