Re: [PoC] Federated Authn/z with OAUTHBEARER - Mailing list pgsql-hackers

On 05.04.25 02:27, Jacob Champion wrote:
> On Tue, Apr 1, 2025 at 3:40 PM Jacob Champion
> <jacob.champion@enterprisedb.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.