Re: [PoC] Federated Authn/z with OAUTHBEARER

From: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Christoph Berg <myon(at)debian(dot)org>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, Antonin Houska <ah(at)cybertec(dot)at>, Wolfgang Walther <walther(at)technowledgy(dot)de>, Devrim Gündüz <devrim(at)gunduz(dot)org>
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Date: 2025-04-22 23:41:50
Message-ID: CAOYmi+=ka9dTDtFhHjnL7jLd-rA1Q+VuU6=vjMM=jjm6_yCrpg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 22, 2025 at 3:02 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> + if oauth_flow_supported
> + cdata.set('USE_LIBCURL', 1)
> + elif libcurlopt.enabled()
> + error('client OAuth is not supported on this platform')
> + endif
> We already know that libcurlopt.enabled() is true here so maybe just doing
> if-else-endif would make it more readable and save readers thinking it might
> have changed?

Features are tri-state, so libcurlopt.disabled() and
libcurlopt.enabled() can both be false. :( My intent is to fall
through nicely in the case where -Dlibcurl=auto.

(Our minimum version of Meson is too old to switch to syntax that
makes this more readable, like .allowed(), .require(), .disable_if(),
etc...)

> Also, "client OAuth" reads a bit strange, how about "client-side
> OAuth" or "OAuth flow module"?
> ...
> I think we should take this opportunity to turn this into a appendPQExpBuffer()
> with a format string instead of two calls.
> ...
> Now that the actual variable, errbuf->len, is short and very descriptive I
> wonder if we shouldn't just use this as it makes the code even clearer IMO.

All three done in v9, attached.

Thanks!
--Jacob

Attachment Content-Type Size
since-v8.diff.txt text/plain 2.7 KB
v9-0001-Add-minor-version-counterpart-to-PG_-MAJORVERSION.patch application/octet-stream 3.5 KB
v9-0002-oauth-Move-the-builtin-flow-into-a-separate-modul.patch application/octet-stream 56.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2025-04-22 23:49:36 DOCS - create publication (tweak for generated columns)
Previous Message Jacob Champion 2025-04-22 22:42:57 Re: [PATCH] Support older Pythons in oauth_server.py