public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jacob Champion <[email protected]>
To: Zsolt Parragi <[email protected]>
Cc: Chao Li <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: Improve OAuth discovery logging
Date: Tue, 17 Mar 2026 09:25:41 -0700
Message-ID: <CAOYmi+=y=iAQ11E6jpUzOKsP8ARVK7g5=etaE3RsrtetFTN-+w@mail.gmail.com> (raw)
In-Reply-To: <CAN4CZFOwmgyv1002=EZTSM__97gX-1fG0Q3Q4Zy=XviVtZPRxg@mail.gmail.com>
References: <CAN4CZFPim7hUiyb7daNKQPSZ8CvQRBGkVhbvED7yZi8VktSn4Q@mail.gmail.com>
<[email protected]>
<CAN4CZFNNfhFCQdFWui5HWbQR60eM-cyndZ7YgSv7b5SKxB9C2A@mail.gmail.com>
<CAOYmi+mDSmh6RNizHRmMAwg4ZP2W=uai3Fr3-wm186NMypf_Pg@mail.gmail.com>
<CAN4CZFNJftK8NaREYaLi-wqpEz3=crQ=1+3f_XUVji=aOrDSWA@mail.gmail.com>
<[email protected]>
<CAOYmi+kjtmRMBdBU3_bGKGDoRSK2AErXbGtHkAjFRapcQNmjhA@mail.gmail.com>
<CAN4CZFNWBXtF-ML3yzdOvX3QEuUwVo5VrBzyWU3O=y-7SeDstA@mail.gmail.com>
<[email protected]>
<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>
<[email protected]>
<CAN4CZFN7u1kX3_0cfyVvtfiWpORxnvZo=xCr9Ag-F5Onp-hpbA@mail.gmail.com>
<[email protected]>
<CAOYmi+kxfGEKw7frQPxWYEA6Qe4BLc683UCNPTYCLdCCV0b4Jw@mail.gmail.com>
<CAN4CZFP--Ec8hMgpu7JojgK9qS08bNnev0c6goA++T4Ozy8bOQ@mail.gmail.com>
<CAOYmi+nsK1dSXaB+oicoyA6kM9ymygCLhSiKtkg1ph_P1uhYOQ@mail.gmail.com>
<[email protected]>
<CAN4CZFOwmgyv1002=EZTSM__97gX-1fG0Q3Q4Zy=XviVtZPRxg@mail.gmail.com>
On Mon, Mar 16, 2026 at 10:29 PM Zsolt Parragi
<[email protected]> 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
view thread (26+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Improve OAuth discovery logging
In-Reply-To: <CAOYmi+=y=iAQ11E6jpUzOKsP8ARVK7g5=etaE3RsrtetFTN-+w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox