Re: [PoC] Federated Authn/z with OAUTHBEARER

On 05.04.25 02:27, Jacob Champion wrote:
> On Tue, Apr 1, 2025 at 3:40 PM Jacob Champion
> <jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>> Maybe a better idea would be to ship an SONAME of
>> `libpq-oauth.so.0.<major>`, without any symlinks, so that there's
>> never any ambiguity about which module belongs with which libpq.
>
> While I was looking into this I found that Debian's going to use the
> existence of an SONAME to check other things, which I assume will make
> Christoph's life harder. I have switched over to
> 'libpq-oauth-<major>.so', without any SONAME or symlinks.

Yes, this is correct. We want a shared module, not a shared library, in
meson parlance.

But the installation directory of a shared module should not be directly
libdir. That is reserved for libraries that you can link at build-time.
Here are some examples I found of other packages that have a library
that itself has plugins:

https://packages.debian.org/bookworm/amd64/libaspell15/filelist
https://packages.debian.org/bookworm/amd64/libkrb5-3/filelist
https://packages.debian.org/bookworm/amd64/libmagickcore-6.q16-6/filelist

> v2 simplifies quite a few things and breaks out the new duplicated
> code into its own file. I pared down the exports from libpq, by having
> it push the pg_g_threadlock pointer directly into the module when
> needed. I think a future improvement would be to combine the dlopen
> with the libcurl initialization, so that everything is done exactly
> once and the module doesn't need to know about threadlocks at all.

Looks mostly ok. I need the following patch to get it to build on macOS:

diff --git a/src/interfaces/libpq-oauth/Makefile
b/src/interfaces/libpq-oauth/Makefile
index 461c44b59c1..d5469ca0e11 100644
--- a/src/interfaces/libpq-oauth/Makefile
+++ b/src/interfaces/libpq-oauth/Makefile
@@ -28,8 +28,9 @@ OBJS = \
oauth-utils.o

SHLIB_LINK_INTERNAL = $(libpq_pgport_shlib)
-SHLIB_LINK = -lcurl
+SHLIB_LINK = -lcurl $(filter -lintl, $(LIBS))
SHLIB_PREREQS = submake-libpq
+BE_DLLLIBS =

SHLIB_EXPORTS = exports.txt

(The second change is not strictly required, but it disables the use of
-bundle_loader postgres, since this is not a backend-loadable module.)

I don't know whether we need an exports file for libpq-oauth. The other
shared modules don't provide an export file, and I'm not sure whether
this is even supported for shared modules. I guess it doesn't hurt?

The PKG_CONFIG_REQUIRES_PRIVATE setting in libpq-oauth/Makefile is
meaningless and can be removed.

In fe-auth-oauth.c, I note that dlerror() is not necessarily
thread-safe. Since there isn't really an alternative, I guess it's ok
to leave it like that, but I figured it should be mentioned.

> i18n is still not working correctly on my machine. I've gotten `make
> init-po` to put the files into the right places now, but if I fake a
> .po file and install the generated .mo, the translations still don't
> seem to be found at runtime. Is anyone able to take a quick look to
> see if I'm missing something obvious?

Not sure, the code looks correct at first glance. However, you could
also just keep the libpq-oauth strings in the libpq catalog. There
isn't really a need to make a separate one, since the versions you end
up installing are locked to each other. So you could for example in
libpq's nls.mk just add

../libpq-oauth/oauth-curl.c

etc. to the files.

Maybe it would also make sense to make libpq-oauth a subdirectory of the
libpq directory instead of a peer.