| From: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [oauth] Split and extend PGOAUTHDEBUG |
| Date: | 2026-03-30 23:26:08 |
| Message-ID: | CAOYmi+kivcSnazEJA=KWknd3azGYnU3mMq9SUvht5Zq74qNcYQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Mar 30, 2026 at 2:41 PM Jacob Champion
<jacob(dot)champion(at)enterprisedb(dot)com> wrote:
> v2, attached, rebases this over 993368113. The big change is the
> removal of `custom-ca`; there were a couple of other tweaks to get
> both commits compiling independently.
Now for review. 0001:
I like how the UNSAFE: split works in practice. Implementation-wise, I
think I'd prefer that the debug flags be implemented as bits in a
uint32, and then we can cut down on the boilerplate.
fe-auth-oauth-debug.c is IMO too much code for what the feature is
providing. We should ideally be able to include this in a header from
both locations it's used.
I think `fast-retry` needs to be moved under UNSAFE and renamed to
something that doesn't sound "good". `dos-interval` maybe?
nitpick: `poll-counts` and `print-plugin-errors` choose different
naming conventions, and we're not referring to the poll() API for the
former. `call-count`? `dlopen`?
We need to test that ignored unsafe options are in fact ignored (the
parsed flag is currently being accumulated anyway, in contradiction to
the warning message).
I have a sample patch locally for these suggestions, if you'd like.
--
I'm not a fan of 0002; I don't think it provides enough useful
functionality to offset the cost. You said
> validators authors should be able to verify that mismatched
> configurations are rejected properly by the validator.
but "mismatched configuration" is kind of meaningless here: the
validator interacts with a token, not a client. Real-world validator
implementations already need to test against all manner of broken
tokens -- creating one that's signed by the wrong party shouldn't
really be harder to create than one that's signed by the right party
-- and the code to send a custom token from libpq is minimal (<40
lines).
--Jacob
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Rowley | 2026-03-30 23:31:19 | Re: scale parallel_tuple_cost by tuple width |
| Previous Message | Tatsuo Ishii | 2026-03-30 23:17:24 | Re: Add GoAway protocol message for graceful but fast server shutdown/switchover |