| From: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Subject: | Re: Improve OAuth discovery logging |
| Date: | 2026-03-20 18:14:10 |
| Message-ID: | CAOYmi+mw0ss04usMi-ShJPdAi9tmF=aO6SyM8qrtd8WfZsS7ag@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Mar 17, 2026 at 2:19 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
> > That's not really true, because the caller hardcodes the mechanism
> > descriptor.
>
> I meant that the caller shouldn't depend on the implementation details
> of the mechanism. The abandoned comment says that '"Abandoned" is a
> SASL-specific state similar to STATUS_EOF ...', yet later it also
> depends on an implementation detail of which sasl mechanism actually
> use it.
I don't disagree, I'm just trying to point out that this coupling is
already part of CheckSASLAuth. See e.g. the handling of shadow_pass.
(I'm not very worried about this, because we're free to improve this
API at any time, and there are only two callers. Michael was very
receptive to prefactoring patches here prior to the addition of
OAUTHBEARER, and I expect we'll continue to refactor it if/when more
mechanisms show up. It's just hard to pull a general interface out of
two mechanisms as dissimilar as SCRAM and OAuth.)
> The patch is also good as-is, all these comments in the last few
> messages are just very minor details, I probably spent way too much
> time thinging about how to make this not oauth specific in the generic
> part of the code.
I appreciate the review!
I'm not in a rush to get this patch pushed, and I want to give Michael
ample time to weigh in. (Personally, I don't think anyone is likely to
argue against the behavior change here, only against how it's being
done. We have alternative implementations available if there are
strong opinions late in the cycle. So I feel pretty confident we can
land a fix for 19.)
Thanks,
--Jacob
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-03-20 18:56:28 | Re: Accounting for metapages in genericcostestimate() |
| Previous Message | Corey Huinker | 2026-03-20 18:13:13 | Re: meson: Make test output much more useful on failure (both in CI and locally) |